Skip to content

Commit

Permalink
refactoring; remove Do()
Browse files Browse the repository at this point in the history
  • Loading branch information
oke11o committed Mar 7, 2024
1 parent 27255c7 commit 6960b80
Show file tree
Hide file tree
Showing 8 changed files with 118 additions and 73 deletions.
9 changes: 4 additions & 5 deletions components/guns/http/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,9 @@ func DefaultBaseGunConfig() BaseGunConfig {
type BaseGun struct {
DebugLog bool // Automaticaly set in Bind if Log accepts debug messages.
Config BaseGunConfig
Do func(r *http.Request) (*http.Response, error) // Required.
Connect func(ctx context.Context) error // Optional hook.
OnClose func() error // Optional. Called on Close().
Aggregator netsample.Aggregator // Lazy set via BindResultTo.
Connect func(ctx context.Context) error // Optional hook.
OnClose func() error // Optional. Called on Close().
Aggregator netsample.Aggregator // Lazy set via BindResultTo.
AnswLog *zap.Logger

scheme string
Expand Down Expand Up @@ -168,7 +167,7 @@ func (b *BaseGun) Shoot(ammo Ammo) {
}
}
var res *http.Response
res, err = b.Do(req)
res, err = b.client.Do(req)
if b.Config.HTTPTrace.TraceEnabled && timings != nil {
sample.SetReceiveTime(timings.GetReceiveTime())
}
Expand Down
55 changes: 46 additions & 9 deletions components/guns/http/base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,38 @@ func (a *ammoMock) IsInvalid() bool {
return false
}

type testDecoratedClient struct {
client Client
t *testing.T
before func(req *http.Request)
after func(req *http.Request, res *http.Response, err error)
returnRes *http.Response
returnErr error
}

func (c *testDecoratedClient) Do(req *http.Request) (*http.Response, error) {
if c.before != nil {
c.before(req)
}
if c.client == nil {
return c.returnRes, c.returnErr
}
res, err := c.client.Do(req)
if c.after != nil {
c.after(req, res, err)
}
return res, err
}

func (c *testDecoratedClient) CloseIdleConnections() {
c.client.CloseIdleConnections()
}

func (s *BaseGunSuite) Test_Shoot_BeforeBindPanics() {
s.base.Do = func(*http.Request) (_ *http.Response, _ error) {
panic("should not be called")
s.base.client = &testDecoratedClient{
client: s.base.client,
before: func(req *http.Request) { panic("should not be called\"") },
after: nil,
}
am := &ammoMock{}

Expand Down Expand Up @@ -152,9 +181,15 @@ func (s *BaseGunSuite) Test_Shoot() {
beforeEachDoOk := func() {
body = ioutil.NopCloser(strings.NewReader("aaaaaaa"))
s.base.AnswLog = zap.NewNop()
s.base.Do = func(doReq *http.Request) (*http.Response, error) {
s.Require().Equal(req, doReq)
return res, nil
s.base.client = &testDecoratedClient{
before: func(doReq *http.Request) {
s.Require().Equal(req, doReq)
},
returnRes: &http.Response{
StatusCode: http.StatusNotFound,
Body: ioutil.NopCloser(body),
Request: req,
},
}
}
s.Run("ammo sample sent to results", func() {
Expand Down Expand Up @@ -232,10 +267,12 @@ func (s *BaseGunSuite) Test_Shoot() {
connectCalled = true
return nil
}
oldDo := s.base.Do
s.base.Do = func(r *http.Request) (*http.Response, error) {
doCalled = true
return oldDo(r)

s.base.client = &testDecoratedClient{
client: s.base.client,
before: func(doReq *http.Request) {
doCalled = true
},
}
}
s.Run("Connect called", func() {
Expand Down
23 changes: 23 additions & 0 deletions components/guns/http/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,29 @@ func (c *panicOnHTTP1Client) Do(req *http.Request) (*http.Response, error) {
return res, nil
}

type httpDecoratedClient struct {
client Client
scheme string
hostname string
targetResolved string
}

func (c *httpDecoratedClient) Do(req *http.Request) (*http.Response, error) {
if req.Host == "" {
req.Host = c.hostname
}

if c.targetResolved != "" {
req.URL.Host = c.targetResolved
}
req.URL.Scheme = c.scheme
return c.client.Do(req)
}

func (c *httpDecoratedClient) CloseIdleConnections() {
c.client.CloseIdleConnections()
}

func checkHTTP2(state *tls.ConnectionState) error {
if state == nil {
return errors.New("http2: non TLS connection")
Expand Down
28 changes: 11 additions & 17 deletions components/guns/http/connect.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,25 +22,28 @@ type ConnectGunConfig struct {
SSL bool // As in HTTP gun, defines scheme for http requests.
}

func NewConnectGun(conf ConnectGunConfig, answLog *zap.Logger) *ConnectGun {
func NewConnectGun(cfg ConnectGunConfig, answLog *zap.Logger) *ConnectGun {
scheme := "http"
if conf.SSL {
if cfg.SSL {
scheme = "https"
}
client := newConnectClient(conf.Client, conf.Target)
client := newConnectClient(cfg.Client, cfg.Target)
wrappedClient := &httpDecoratedClient{
client: client,
scheme: scheme,
hostname: "",
targetResolved: cfg.Target,
}
var g ConnectGun
g = ConnectGun{
BaseGun: BaseGun{
Config: conf.Base,
Do: g.Do,
Config: cfg.Base,
OnClose: func() error {
client.CloseIdleConnections()
return nil
},
AnswLog: answLog,

scheme: scheme,
client: client,
client: wrappedClient,
},
}
return &g
Expand All @@ -56,15 +59,6 @@ func (g *ConnectGun) WarmUp(opts *warmup.Options) (any, error) {
return nil, nil
}

func (g *ConnectGun) Do(req *http.Request) (*http.Response, error) {
req.URL.Scheme = g.scheme
if req.URL.Host == "" {
req.URL.Host = "127.0.0.1"
}

return g.client.Do(req)
}

func DefaultConnectGunConfig() ConnectGunConfig {
return ConnectGunConfig{
SSL: false,
Expand Down
12 changes: 7 additions & 5 deletions components/guns/http/connect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@ import (
"go.uber.org/zap"
)

var tunnelHandler = func(t *testing.T, originURL string) http.Handler {
var tunnelHandler = func(t *testing.T, originURL string, compareURI bool) http.Handler {
u, err := url.Parse(originURL)
require.NoError(t, err)
originHost := u.Host
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
require.Equal(t, originHost, r.RequestURI)
if compareURI {
require.Equal(t, originHost, r.RequestURI)
}

toOrigin, err := net.Dial("tcp", originHost)
require.NoError(t, err)
Expand Down Expand Up @@ -60,9 +62,9 @@ func TestDo(t *testing.T) {

var proxy *httptest.Server
if tunnelSSL {
proxy = httptest.NewTLSServer(tunnelHandler(t, origin.URL))
proxy = httptest.NewTLSServer(tunnelHandler(t, origin.URL, true))
} else {
proxy = httptest.NewServer(tunnelHandler(t, origin.URL))
proxy = httptest.NewServer(tunnelHandler(t, origin.URL, true))
}
defer proxy.Close()

Expand Down Expand Up @@ -91,7 +93,7 @@ func TestNewConnectGun(t *testing.T) {
rw.WriteHeader(http.StatusOK)
}))
defer origin.Close()
proxy := httptest.NewServer(tunnelHandler(t, origin.URL))
proxy := httptest.NewServer(tunnelHandler(t, origin.URL, false))
defer proxy.Close()

log := zap.NewNop()
Expand Down
51 changes: 20 additions & 31 deletions components/guns/http/http.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package phttp

import (
"net/http"

"github.com/pkg/errors"
"github.com/yandex/pandora/core/warmup"
"go.uber.org/zap"
Expand All @@ -15,7 +13,7 @@ type HTTPGunConfig struct {
SSL bool
}

func NewHTTPGun(conf HTTPGunConfig, answLog *zap.Logger, targetResolved string) *HTTPGun {
func NewHTTPGun(conf HTTPGunConfig, answLog *zap.Logger, targetResolved string) *BaseGun {
return NewClientGun(HTTP1ClientConstructor, conf, answLog, targetResolved)
}

Expand All @@ -26,7 +24,7 @@ var HTTP1ClientConstructor clientConstructor = func(clientConfig ClientConfig, t
}

// NewHTTP2Gun return simple HTTP/2 gun that can shoot sequentially through one connection.
func NewHTTP2Gun(conf HTTPGunConfig, answLog *zap.Logger, targetResolved string) (*HTTPGun, error) {
func NewHTTP2Gun(conf HTTPGunConfig, answLog *zap.Logger, targetResolved string) (*BaseGun, error) {
if !conf.SSL {
// Open issue on github if you really need this feature.
return nil, errors.New("HTTP/2.0 over TCP is not supported. Please leave SSL option true by default.")
Expand All @@ -41,28 +39,29 @@ var HTTP2ClientConstructor clientConstructor = func(clientConfig ClientConfig, t
return &panicOnHTTP1Client{Client: client}
}

func NewClientGun(clientConstructor clientConstructor, cfg HTTPGunConfig, answLog *zap.Logger, targetResolved string) *HTTPGun {
client := clientConstructor(cfg.Client, cfg.Target)
func NewClientGun(clientConstructor clientConstructor, cfg HTTPGunConfig, answLog *zap.Logger, targetResolved string) *BaseGun {
scheme := "http"
if cfg.SSL {
scheme = "https"
}
var g HTTPGun
g = HTTPGun{
BaseGun: BaseGun{
Config: cfg.Base,
Do: g.Do,
OnClose: func() error {
client.CloseIdleConnections()
return nil
},
AnswLog: answLog,

scheme: scheme,
hostname: getHostWithoutPort(cfg.Target),
targetResolved: targetResolved,
client: client,
client := clientConstructor(cfg.Client, cfg.Target)
wrappedClient := &httpDecoratedClient{
client: client,
hostname: getHostWithoutPort(cfg.Target),
targetResolved: targetResolved,
scheme: scheme,
}
g := BaseGun{
Config: cfg.Base,
OnClose: func() error {
client.CloseIdleConnections()
return nil
},
AnswLog: answLog,

hostname: getHostWithoutPort(cfg.Target),
targetResolved: targetResolved,
client: wrappedClient,
}
return &g
}
Expand All @@ -77,16 +76,6 @@ func (g *HTTPGun) WarmUp(opts *warmup.Options) (any, error) {
return nil, nil
}

func (g *HTTPGun) Do(req *http.Request) (*http.Response, error) {
if req.Host == "" {
req.Host = g.hostname
}

req.URL.Host = g.targetResolved
req.URL.Scheme = g.scheme
return g.client.Do(req)
}

func DefaultHTTPGunConfig() HTTPGunConfig {
return HTTPGunConfig{

Expand Down
12 changes: 6 additions & 6 deletions tests/acceptance/connect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,12 @@ func (s *ConnectGunSuite) Test_Connect() {
wantCnt: 15,
},
// TODO: first record does not look like a TLS handshake. Check https://go.dev/src/crypto/tls/conn.go
//{
// name: "connect-ssl",
// filecfg: "testdata/connect/connect-ssl.yaml",
// isTLS: true,
// wantCnt: 4,
//},
{
name: "connect-ssl",
filecfg: "testdata/connect/connect-ssl.yaml",
isTLS: true,
wantCnt: 4,
},
}
for _, tt := range tests {
s.Run(tt.name, func() {
Expand Down
1 change: 1 addition & 0 deletions tests/acceptance/testdata/connect/connect-ssl.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ pools:
target: {{.target}}
type: connect
ssl: true
connect-ssl: true
answlog:
enabled: false
rps-per-instance: false
Expand Down

0 comments on commit 6960b80

Please sign in to comment.