diff --git a/.github/workflows/go1.19.yml b/.github/workflows/go1.19.yml index 34ed31053f..1cb6a2eb68 100644 --- a/.github/workflows/go1.19.yml +++ b/.github/workflows/go1.19.yml @@ -18,11 +18,12 @@ jobs: steps: - uses: actions/checkout@v2 - - run: go install golang.org/dl/go1.19.1@latest + - uses: actions/setup-go@v3 + with: + go-version: "1.19" - - run: $(go env GOPATH)/bin/go1.19.1 download + - run: go version - - run: $(go env GOPATH)/bin/go1.19.1 build -v ./... - - - run: $(go env GOPATH)/bin/go1.19.1 test -short -race -tags shaping ./... + - run: go build -v ./... + - run: go test -short -race -tags shaping ./... diff --git a/internal/engine/experiment/webconnectivity/control.go b/internal/engine/experiment/webconnectivity/control.go index bd953a1e7a..39c600908f 100644 --- a/internal/engine/experiment/webconnectivity/control.go +++ b/internal/engine/experiment/webconnectivity/control.go @@ -24,8 +24,8 @@ func Control( ctx context.Context, sess model.ExperimentSession, testhelpers []model.OOAPIService, creq ControlRequest) (ControlResponse, *model.OOAPIService, error) { seqCaller := httpapi.NewSequenceCaller( - httpapi.MustNewPOSTJSONWithJSONResponseDescriptor(sess.Logger(), "/", creq).WithBodyLogging(true), - httpapi.NewEndpointList(sess.DefaultHTTPClient(), sess.UserAgent(), testhelpers...)..., + httpapi.MustNewPOSTJSONWithJSONResponseDescriptor("/", creq).WithBodyLogging(true), + httpapi.NewEndpointList(sess.DefaultHTTPClient(), sess.Logger(), sess.UserAgent(), testhelpers...)..., ) sess.Logger().Infof("control for %s...", creq.HTTPRequest) var out ControlResponse diff --git a/internal/experiment/webconnectivity/control.go b/internal/experiment/webconnectivity/control.go index bf7003af60..16adbb161c 100644 --- a/internal/experiment/webconnectivity/control.go +++ b/internal/experiment/webconnectivity/control.go @@ -110,8 +110,8 @@ func (c *Control) Run(parentCtx context.Context) { // create an httpapi sequence caller seqCaller := httpapi.NewSequenceCaller( - httpapi.MustNewPOSTJSONWithJSONResponseDescriptor(c.Logger, "/", creq).WithBodyLogging(true), - httpapi.NewEndpointList(c.Session.DefaultHTTPClient(), c.Session.UserAgent(), c.TestHelpers...)..., + httpapi.MustNewPOSTJSONWithJSONResponseDescriptor("/", creq).WithBodyLogging(true), + httpapi.NewEndpointList(c.Session.DefaultHTTPClient(), c.Logger, c.Session.UserAgent(), c.TestHelpers...)..., ) // issue the control request and wait for the response diff --git a/internal/httpapi/call.go b/internal/httpapi/call.go index 1228080fd7..8a9a74f4e5 100644 --- a/internal/httpapi/call.go +++ b/internal/httpapi/call.go @@ -48,9 +48,9 @@ func newRequest(ctx context.Context, endpoint *Endpoint, desc *Descriptor) (*htt var reqBody io.Reader if len(desc.RequestBody) > 0 { reqBody = bytes.NewReader(desc.RequestBody) - desc.Logger.Debugf("httpapi: request body length: %d", len(desc.RequestBody)) + endpoint.Logger.Debugf("httpapi: request body length: %d", len(desc.RequestBody)) if desc.LogBody { - desc.Logger.Debugf("httpapi: request body: %s", string(desc.RequestBody)) + endpoint.Logger.Debugf("httpapi: request body: %s", string(desc.RequestBody)) } } request, err := http.NewRequestWithContext(ctx, desc.Method, URL.String(), reqBody) @@ -120,9 +120,9 @@ func docall(endpoint *Endpoint, desc *Descriptor, request *http.Request) (*http. if err != nil { return response, nil, &errMaybeCensorship{err} } - desc.Logger.Debugf("httpapi: response body length: %d bytes", len(data)) + endpoint.Logger.Debugf("httpapi: response body length: %d bytes", len(data)) if desc.LogBody { - desc.Logger.Debugf("httpapi: response body: %s", string(data)) + endpoint.Logger.Debugf("httpapi: response body: %s", string(data)) } if response.StatusCode >= 400 { return response, nil, &ErrHTTPRequestFailed{response.StatusCode} @@ -174,7 +174,7 @@ func CallWithJSONResponse(ctx context.Context, desc *Descriptor, endpoint *Endpo return err } if ctype := httpResp.Header.Get("Content-Type"); !goodContentTypeForJSON[ctype] { - desc.Logger.Warnf("httpapi: unexpected content-type: %s", ctype) + endpoint.Logger.Warnf("httpapi: unexpected content-type: %s", ctype) // fallthrough } return json.Unmarshal(rawRespBody, response) diff --git a/internal/httpapi/call_test.go b/internal/httpapi/call_test.go index 8fd45d7be7..f088346930 100644 --- a/internal/httpapi/call_test.go +++ b/internal/httpapi/call_test.go @@ -90,6 +90,7 @@ func Test_newRequest(t *testing.T) { BaseURL: "\t\t\t", // does not parse! HTTPClient: nil, Host: "", + Logger: nil, UserAgent: "", }, desc: &Descriptor{ @@ -97,7 +98,6 @@ func Test_newRequest(t *testing.T) { Authorization: "", ContentType: "", LogBody: false, - Logger: nil, MaxBodySize: 0, Method: "", RequestBody: nil, @@ -116,6 +116,7 @@ func Test_newRequest(t *testing.T) { BaseURL: "https://example.com/", HTTPClient: nil, Host: "", + Logger: nil, UserAgent: "", }, desc: &Descriptor{ @@ -123,7 +124,6 @@ func Test_newRequest(t *testing.T) { Authorization: "", ContentType: "", LogBody: false, - Logger: nil, MaxBodySize: 0, Method: "", RequestBody: nil, @@ -142,6 +142,7 @@ func Test_newRequest(t *testing.T) { BaseURL: "https://example.com/", HTTPClient: nil, Host: "", + Logger: nil, UserAgent: "", }, desc: &Descriptor{ @@ -149,7 +150,6 @@ func Test_newRequest(t *testing.T) { Authorization: "", ContentType: "", LogBody: false, - Logger: nil, MaxBodySize: 0, Method: http.MethodGet, RequestBody: nil, @@ -181,6 +181,7 @@ func Test_newRequest(t *testing.T) { BaseURL: "https://example.com/", HTTPClient: nil, Host: "", + Logger: model.DiscardLogger, UserAgent: "", }, desc: &Descriptor{ @@ -188,7 +189,6 @@ func Test_newRequest(t *testing.T) { Authorization: "", ContentType: "", LogBody: false, - Logger: model.DiscardLogger, MaxBodySize: 0, Method: http.MethodPost, RequestBody: []byte("deadbeef"), @@ -224,6 +224,7 @@ func Test_newRequest(t *testing.T) { BaseURL: "https://example.com/", HTTPClient: nil, Host: "antani.org", + Logger: nil, UserAgent: "httpclient/1.0.1", }, desc: &Descriptor{ @@ -231,7 +232,6 @@ func Test_newRequest(t *testing.T) { Authorization: "deafbeef", ContentType: "text/plain", LogBody: false, - Logger: nil, MaxBodySize: 0, Method: http.MethodPut, RequestBody: nil, @@ -275,6 +275,7 @@ func Test_newRequest(t *testing.T) { BaseURL: "https://www.example.com/api/v1", HTTPClient: nil, Host: "", + Logger: nil, UserAgent: "", }, desc: &Descriptor{ @@ -282,7 +283,6 @@ func Test_newRequest(t *testing.T) { Authorization: "", ContentType: "", LogBody: false, - Logger: nil, MaxBodySize: 0, Method: http.MethodGet, RequestBody: nil, @@ -311,6 +311,7 @@ func Test_newRequest(t *testing.T) { BaseURL: "https://example.org/api/v1/?probe_cc=IT", HTTPClient: nil, Host: "", + Logger: nil, UserAgent: "", }, desc: &Descriptor{ @@ -318,7 +319,6 @@ func Test_newRequest(t *testing.T) { Authorization: "", ContentType: "", LogBody: false, - Logger: nil, MaxBodySize: 0, Method: http.MethodGet, RequestBody: nil, @@ -347,6 +347,7 @@ func Test_newRequest(t *testing.T) { BaseURL: "https://www.example.com/api/v1/", HTTPClient: nil, Host: "", + Logger: nil, UserAgent: "", }, desc: &Descriptor{ @@ -354,7 +355,6 @@ func Test_newRequest(t *testing.T) { Authorization: "", ContentType: "", LogBody: false, - Logger: nil, MaxBodySize: 0, Method: http.MethodGet, RequestBody: nil, @@ -446,7 +446,6 @@ func TestCall(t *testing.T) { Authorization: "", ContentType: "", LogBody: false, - Logger: nil, MaxBodySize: 0, Method: "", RequestBody: nil, @@ -458,6 +457,7 @@ func TestCall(t *testing.T) { BaseURL: "\t\t\t", // causes newRequest to fail HTTPClient: nil, Host: "", + Logger: nil, UserAgent: "", }, }, @@ -469,7 +469,6 @@ func TestCall(t *testing.T) { args: args{ ctx: context.Background(), desc: &Descriptor{ - Logger: model.DiscardLogger, Method: http.MethodGet, }, endpoint: &Endpoint{ @@ -479,6 +478,7 @@ func TestCall(t *testing.T) { return nil, io.EOF }, }, + Logger: model.DiscardLogger, }, }, want: nil, @@ -494,7 +494,6 @@ func TestCall(t *testing.T) { args: args{ ctx: context.Background(), desc: &Descriptor{ - Logger: model.DiscardLogger, Method: http.MethodGet, }, endpoint: &Endpoint{ @@ -511,6 +510,7 @@ func TestCall(t *testing.T) { return resp, nil }, }, + Logger: model.DiscardLogger, }, }, want: nil, @@ -526,7 +526,6 @@ func TestCall(t *testing.T) { args: args{ ctx: context.Background(), desc: &Descriptor{ - Logger: model.DiscardLogger, Method: http.MethodGet, }, endpoint: &Endpoint{ @@ -540,6 +539,7 @@ func TestCall(t *testing.T) { return resp, nil }, }, + Logger: model.DiscardLogger, }, }, want: nil, @@ -556,7 +556,6 @@ func TestCall(t *testing.T) { ctx: context.Background(), desc: &Descriptor{ LogBody: true, // as documented by this test's name - Logger: model.DiscardLogger, Method: http.MethodGet, }, endpoint: &Endpoint{ @@ -570,6 +569,7 @@ func TestCall(t *testing.T) { return resp, nil }, }, + Logger: model.DiscardLogger, }, }, want: []byte("deadbeef"), @@ -620,12 +620,11 @@ func TestCallWithJSONResponse(t *testing.T) { }{{ name: "call fails", args: args{ - ctx: context.Background(), - desc: &Descriptor{ - Logger: model.DiscardLogger, - }, + ctx: context.Background(), + desc: &Descriptor{}, endpoint: &Endpoint{ BaseURL: "\t\t\t\t", // causes failure + Logger: model.DiscardLogger, }, }, wantErr: errors.New(`parse "\t\t\t\t": net/url: invalid control character in URL`), @@ -633,10 +632,8 @@ func TestCallWithJSONResponse(t *testing.T) { }, { name: "with error during httpClient.Do", args: args{ - ctx: context.Background(), - desc: &Descriptor{ - Logger: model.DiscardLogger, - }, + ctx: context.Background(), + desc: &Descriptor{}, endpoint: &Endpoint{ BaseURL: "https://www.example.com/a", HTTPClient: &mocks.HTTPClient{ @@ -644,6 +641,7 @@ func TestCallWithJSONResponse(t *testing.T) { return nil, io.EOF }, }, + Logger: model.DiscardLogger, }, }, wantErr: io.EOF, @@ -656,10 +654,8 @@ func TestCallWithJSONResponse(t *testing.T) { }, { name: "with error when reading the response body", args: args{ - ctx: context.Background(), - desc: &Descriptor{ - Logger: model.DiscardLogger, - }, + ctx: context.Background(), + desc: &Descriptor{}, endpoint: &Endpoint{ BaseURL: "https://www.example.com/a", HTTPClient: &mocks.HTTPClient{ @@ -675,6 +671,7 @@ func TestCallWithJSONResponse(t *testing.T) { return resp, nil }, }, + Logger: model.DiscardLogger, }, }, wantErr: errors.New(netxlite.FailureConnectionReset), @@ -687,10 +684,8 @@ func TestCallWithJSONResponse(t *testing.T) { }, { name: "with HTTP failure", args: args{ - ctx: context.Background(), - desc: &Descriptor{ - Logger: model.DiscardLogger, - }, + ctx: context.Background(), + desc: &Descriptor{}, endpoint: &Endpoint{ BaseURL: "https://www.example.com/a", HTTPClient: &mocks.HTTPClient{ @@ -702,6 +697,7 @@ func TestCallWithJSONResponse(t *testing.T) { return resp, nil }, }, + Logger: model.DiscardLogger, }, }, wantErr: errors.New("httpapi: http request failed: 400"), @@ -714,10 +710,8 @@ func TestCallWithJSONResponse(t *testing.T) { }, { name: "with good response and missing header", args: args{ - ctx: context.Background(), - desc: &Descriptor{ - Logger: model.DiscardLogger, - }, + ctx: context.Background(), + desc: &Descriptor{}, endpoint: &Endpoint{ BaseURL: "https://www.example.com/a", HTTPClient: &mocks.HTTPClient{ @@ -729,6 +723,7 @@ func TestCallWithJSONResponse(t *testing.T) { return resp, nil }, }, + Logger: model.DiscardLogger, }, }, wantErr: nil, @@ -736,10 +731,8 @@ func TestCallWithJSONResponse(t *testing.T) { }, { name: "with good response and good header", args: args{ - ctx: context.Background(), - desc: &Descriptor{ - Logger: model.DiscardLogger, - }, + ctx: context.Background(), + desc: &Descriptor{}, endpoint: &Endpoint{ BaseURL: "https://www.example.com/a", HTTPClient: &mocks.HTTPClient{ @@ -754,6 +747,7 @@ func TestCallWithJSONResponse(t *testing.T) { return resp, nil }, }, + Logger: model.DiscardLogger, }, }, wantErr: nil, @@ -764,7 +758,6 @@ func TestCallWithJSONResponse(t *testing.T) { ctx: context.Background(), desc: &Descriptor{ LogBody: false, - Logger: model.DiscardLogger, Method: http.MethodGet, }, endpoint: &Endpoint{ @@ -781,6 +774,7 @@ func TestCallWithJSONResponse(t *testing.T) { return resp, nil }, }, + Logger: model.DiscardLogger, }, }, wantErr: errors.New("unexpected end of JSON input"), @@ -816,13 +810,13 @@ func TestCallHonoursContext(t *testing.T) { cancel() // should fail HTTP request immediately desc := &Descriptor{ LogBody: false, - Logger: model.DiscardLogger, Method: http.MethodGet, URLPath: "/robots.txt", } endpoint := &Endpoint{ BaseURL: "https://www.example.com/", HTTPClient: http.DefaultClient, + Logger: model.DiscardLogger, UserAgent: model.HTTPHeaderUserAgent, } body, err := Call(ctx, desc, endpoint) @@ -839,13 +833,13 @@ func TestCallWithJSONResponseHonoursContext(t *testing.T) { cancel() // should fail HTTP request immediately desc := &Descriptor{ LogBody: false, - Logger: model.DiscardLogger, Method: http.MethodGet, URLPath: "/robots.txt", } endpoint := &Endpoint{ BaseURL: "https://www.example.com/", HTTPClient: http.DefaultClient, + Logger: model.DiscardLogger, UserAgent: model.HTTPHeaderUserAgent, } var resp url.URL @@ -861,11 +855,12 @@ func TestDescriptorLogging(t *testing.T) { // by keeping the ~same implementation with a custom callx function that converts // the previous semantics of httpx to the new semantics of httpapi. callx := func(baseURL string, logBody bool, logger model.Logger, request, response any) error { - desc := MustNewPOSTJSONWithJSONResponseDescriptor(logger, "/", request).WithBodyLogging(logBody) + desc := MustNewPOSTJSONWithJSONResponseDescriptor("/", request).WithBodyLogging(logBody) runtimex.Assert(desc.LogBody == logBody, "desc.LogBody should be equal to logBody here") endpoint := &Endpoint{ BaseURL: baseURL, HTTPClient: http.DefaultClient, + Logger: logger, } return CallWithJSONResponse(context.Background(), desc, endpoint, response) } diff --git a/internal/httpapi/descriptor.go b/internal/httpapi/descriptor.go index ed35e85f08..e088aab50a 100644 --- a/internal/httpapi/descriptor.go +++ b/internal/httpapi/descriptor.go @@ -10,7 +10,6 @@ import ( "net/url" "time" - "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/runtimex" ) @@ -32,11 +31,6 @@ type Descriptor struct { // LogBody OPTIONALLY enables logging bodies. LogBody bool - // Logger is the MANDATORY logger to use. - // - // For example, model.DiscardLogger. - Logger model.Logger - // MaxBodySize is the OPTIONAL maximum response body size. If // not set, we use the |DefaultMaxBodySize| constant. MaxBodySize int64 @@ -76,8 +70,8 @@ const DefaultCallTimeout = 60 * time.Second // NewGETJSONDescriptor is a convenience factory for creating a new descriptor // that uses the GET method and expects a JSON response. -func NewGETJSONDescriptor(logger model.Logger, urlPath string) *Descriptor { - return NewGETJSONWithQueryDescriptor(logger, urlPath, url.Values{}) +func NewGETJSONDescriptor(urlPath string) *Descriptor { + return NewGETJSONWithQueryDescriptor(urlPath, url.Values{}) } // applicationJSON is the content-type for JSON @@ -86,13 +80,12 @@ const applicationJSON = "application/json" // NewGETJSONWithQueryDescriptor is like NewGETJSONDescriptor but it also // allows you to provide |query| arguments. Leaving |query| nil or empty // is equivalent to calling NewGETJSONDescriptor directly. -func NewGETJSONWithQueryDescriptor(logger model.Logger, urlPath string, query url.Values) *Descriptor { +func NewGETJSONWithQueryDescriptor(urlPath string, query url.Values) *Descriptor { return &Descriptor{ Accept: applicationJSON, Authorization: "", ContentType: "", LogBody: false, - Logger: logger, MaxBodySize: DefaultMaxBodySize, Method: http.MethodGet, RequestBody: nil, @@ -107,7 +100,7 @@ func NewGETJSONWithQueryDescriptor(logger model.Logger, urlPath string, query ur // // This function ONLY fails if we cannot serialize the |request| to JSON. So, if you know // that |request| is JSON-serializable, you can safely call MustNewPostJSONWithJSONResponseDescriptor instead. -func NewPOSTJSONWithJSONResponseDescriptor(logger model.Logger, urlPath string, request any) (*Descriptor, error) { +func NewPOSTJSONWithJSONResponseDescriptor(urlPath string, request any) (*Descriptor, error) { rawRequest, err := json.Marshal(request) if err != nil { return nil, err @@ -117,7 +110,6 @@ func NewPOSTJSONWithJSONResponseDescriptor(logger model.Logger, urlPath string, Authorization: "", ContentType: applicationJSON, LogBody: false, - Logger: logger, MaxBodySize: DefaultMaxBodySize, Method: http.MethodPost, RequestBody: rawRequest, @@ -130,21 +122,20 @@ func NewPOSTJSONWithJSONResponseDescriptor(logger model.Logger, urlPath string, // MustNewPOSTJSONWithJSONResponseDescriptor is like NewPOSTJSONWithJSONResponseDescriptor except that // it panics in case it's not possible to JSON serialize the |request|. -func MustNewPOSTJSONWithJSONResponseDescriptor(logger model.Logger, urlPath string, request any) *Descriptor { - desc, err := NewPOSTJSONWithJSONResponseDescriptor(logger, urlPath, request) +func MustNewPOSTJSONWithJSONResponseDescriptor(urlPath string, request any) *Descriptor { + desc, err := NewPOSTJSONWithJSONResponseDescriptor(urlPath, request) runtimex.PanicOnError(err, "NewPOSTJSONWithJSONResponseDescriptor failed") return desc } // NewGETResourceDescriptor creates a generic descriptor for GETting a // resource of unspecified type using the given |urlPath|. -func NewGETResourceDescriptor(logger model.Logger, urlPath string) *Descriptor { +func NewGETResourceDescriptor(urlPath string) *Descriptor { return &Descriptor{ Accept: "", Authorization: "", ContentType: "", LogBody: false, - Logger: logger, MaxBodySize: DefaultMaxBodySize, Method: http.MethodGet, RequestBody: nil, diff --git a/internal/httpapi/descriptor_test.go b/internal/httpapi/descriptor_test.go index c3c58c0107..46bef8d069 100644 --- a/internal/httpapi/descriptor_test.go +++ b/internal/httpapi/descriptor_test.go @@ -8,7 +8,6 @@ import ( "time" "github.com/google/go-cmp/cmp" - "github.com/ooni/probe-cli/v3/internal/model" ) func TestDescriptor_WithBodyLogging(t *testing.T) { @@ -17,7 +16,6 @@ func TestDescriptor_WithBodyLogging(t *testing.T) { Authorization string ContentType string LogBody bool - Logger model.Logger MaxBodySize int64 Method string RequestBody []byte @@ -42,7 +40,6 @@ func TestDescriptor_WithBodyLogging(t *testing.T) { Authorization: "y", ContentType: "zzz", LogBody: false, // obviously must be false - Logger: model.DiscardLogger, MaxBodySize: 123, Method: "POST", RequestBody: []byte("123"), @@ -57,7 +54,6 @@ func TestDescriptor_WithBodyLogging(t *testing.T) { Authorization: "y", ContentType: "zzz", LogBody: true, - Logger: model.DiscardLogger, MaxBodySize: 123, Method: "POST", RequestBody: []byte("123"), @@ -75,7 +71,6 @@ func TestDescriptor_WithBodyLogging(t *testing.T) { Authorization: tt.fields.Authorization, ContentType: tt.fields.ContentType, LogBody: tt.fields.LogBody, - Logger: tt.fields.Logger, MaxBodySize: tt.fields.MaxBodySize, Method: tt.fields.Method, RequestBody: tt.fields.RequestBody, @@ -97,7 +92,6 @@ func TestNewGetJSONDescriptor(t *testing.T) { Authorization: "", ContentType: "", LogBody: false, - Logger: model.DiscardLogger, MaxBodySize: DefaultMaxBodySize, Method: http.MethodGet, RequestBody: nil, @@ -105,7 +99,7 @@ func TestNewGetJSONDescriptor(t *testing.T) { URLPath: "/robots.txt", URLQuery: url.Values{}, } - got := NewGETJSONDescriptor(model.DiscardLogger, "/robots.txt") + got := NewGETJSONDescriptor("/robots.txt") if diff := cmp.Diff(expected, got); diff != "" { t.Fatal(diff) } @@ -121,7 +115,6 @@ func TestNewGetJSONWithQueryDescriptor(t *testing.T) { Authorization: "", ContentType: "", LogBody: false, - Logger: model.DiscardLogger, MaxBodySize: DefaultMaxBodySize, Method: http.MethodGet, RequestBody: nil, @@ -129,7 +122,7 @@ func TestNewGetJSONWithQueryDescriptor(t *testing.T) { URLPath: "/robots.txt", URLQuery: query, } - got := NewGETJSONWithQueryDescriptor(model.DiscardLogger, "/robots.txt", query) + got := NewGETJSONWithQueryDescriptor("/robots.txt", query) if diff := cmp.Diff(expected, got); diff != "" { t.Fatal(diff) } @@ -143,7 +136,7 @@ func TestNewPOSTJSONWithJSONResponseDescriptor(t *testing.T) { t.Run("with failure", func(t *testing.T) { request := make(chan int64) - got, err := NewPOSTJSONWithJSONResponseDescriptor(model.DiscardLogger, "/robots.txt", request) + got, err := NewPOSTJSONWithJSONResponseDescriptor("/robots.txt", request) if err == nil || err.Error() != "json: unsupported type: chan int64" { log.Fatal("unexpected err", err) } @@ -162,7 +155,6 @@ func TestNewPOSTJSONWithJSONResponseDescriptor(t *testing.T) { Authorization: "", ContentType: "application/json", LogBody: false, - Logger: model.DiscardLogger, MaxBodySize: DefaultMaxBodySize, Method: http.MethodPost, RequestBody: []byte(`{"Name":"sbs","Age":99}`), @@ -170,7 +162,7 @@ func TestNewPOSTJSONWithJSONResponseDescriptor(t *testing.T) { URLPath: "/robots.txt", URLQuery: nil, } - got, err := NewPOSTJSONWithJSONResponseDescriptor(model.DiscardLogger, "/robots.txt", request) + got, err := NewPOSTJSONWithJSONResponseDescriptor("/robots.txt", request) if err != nil { log.Fatal(err) } @@ -195,7 +187,7 @@ func TestMustNewPOSTJSONWithJSONResponseDescriptor(t *testing.T) { } }() request := make(chan int64) - _ = MustNewPOSTJSONWithJSONResponseDescriptor(model.DiscardLogger, "/robots.txt", request) + _ = MustNewPOSTJSONWithJSONResponseDescriptor("/robots.txt", request) }() if !panicked { t.Fatal("did not panic") @@ -212,7 +204,6 @@ func TestMustNewPOSTJSONWithJSONResponseDescriptor(t *testing.T) { Authorization: "", ContentType: "application/json", LogBody: false, - Logger: model.DiscardLogger, MaxBodySize: DefaultMaxBodySize, Method: http.MethodPost, RequestBody: []byte(`{"Name":"sbs","Age":99}`), @@ -220,7 +211,7 @@ func TestMustNewPOSTJSONWithJSONResponseDescriptor(t *testing.T) { URLPath: "/robots.txt", URLQuery: nil, } - got := MustNewPOSTJSONWithJSONResponseDescriptor(model.DiscardLogger, "/robots.txt", request) + got := MustNewPOSTJSONWithJSONResponseDescriptor("/robots.txt", request) if diff := cmp.Diff(expected, got); diff != "" { t.Fatal(diff) } @@ -233,7 +224,6 @@ func TestNewGetResourceDescriptor(t *testing.T) { Authorization: "", ContentType: "", LogBody: false, - Logger: model.DiscardLogger, MaxBodySize: DefaultMaxBodySize, Method: http.MethodGet, RequestBody: nil, @@ -241,7 +231,7 @@ func TestNewGetResourceDescriptor(t *testing.T) { URLPath: "/robots.txt", URLQuery: url.Values{}, } - got := NewGETResourceDescriptor(model.DiscardLogger, "/robots.txt") + got := NewGETResourceDescriptor("/robots.txt") if diff := cmp.Diff(expected, got); diff != "" { t.Fatal(diff) } diff --git a/internal/httpapi/endpoint.go b/internal/httpapi/endpoint.go index acfc4d8996..cc3a8d4ede 100644 --- a/internal/httpapi/endpoint.go +++ b/internal/httpapi/endpoint.go @@ -35,6 +35,11 @@ type Endpoint struct { // host header may be needed when using cloudfronting. Host string + // Logger is the MANDATORY logger to use. + // + // For example, model.DiscardLogger. + Logger model.Logger + // User-Agent is the OPTIONAL user-agent to use. If empty, // we'll use the stdlib's default user-agent string. UserAgent string @@ -47,11 +52,17 @@ type Endpoint struct { // // - httpClient is the HTTP client to use for accessing the endpoints; // +// - logger is the logger to use; +// // - userAgent is the user agent you would like to use; // // - service is the list of services gathered from the backend. -func NewEndpointList(httpClient model.HTTPClient, - userAgent string, services ...model.OOAPIService) (out []*Endpoint) { +func NewEndpointList( + httpClient model.HTTPClient, + logger model.Logger, + userAgent string, + services ...model.OOAPIService, +) (out []*Endpoint) { for _, svc := range services { switch svc.Type { case "https": @@ -59,6 +70,7 @@ func NewEndpointList(httpClient model.HTTPClient, BaseURL: svc.Address, HTTPClient: httpClient, Host: "", + Logger: logger, UserAgent: userAgent, }) case "cloudfront": @@ -66,6 +78,7 @@ func NewEndpointList(httpClient model.HTTPClient, BaseURL: svc.Address, HTTPClient: httpClient, Host: svc.Front, + Logger: logger, UserAgent: userAgent, }) default: diff --git a/internal/httpapi/endpoint_test.go b/internal/httpapi/endpoint_test.go index 7077e1480f..113915d96d 100644 --- a/internal/httpapi/endpoint_test.go +++ b/internal/httpapi/endpoint_test.go @@ -50,17 +50,24 @@ func TestNewEndpointList(t *testing.T) { BaseURL: "https://www.example.com/", HTTPClient: defaultHTTPClient, Host: "", + Logger: model.DiscardLogger, UserAgent: model.HTTPHeaderUserAgent, }, { BaseURL: "https://www.example.org/", HTTPClient: defaultHTTPClient, Host: "example.org.it", + Logger: model.DiscardLogger, UserAgent: model.HTTPHeaderUserAgent, }}, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - gotOut := NewEndpointList(tt.args.httpClient, tt.args.userAgent, tt.args.services...) + gotOut := NewEndpointList( + tt.args.httpClient, + model.DiscardLogger, + tt.args.userAgent, + tt.args.services..., + ) if diff := cmp.Diff(tt.wantOut, gotOut); diff != "" { t.Fatal(diff) } diff --git a/internal/httpapi/sequence_test.go b/internal/httpapi/sequence_test.go index 13cc50fb9d..3a88cec2bb 100644 --- a/internal/httpapi/sequence_test.go +++ b/internal/httpapi/sequence_test.go @@ -18,7 +18,6 @@ func TestSequenceCaller(t *testing.T) { t.Run("first success", func(t *testing.T) { sc := NewSequenceCaller( &Descriptor{ - Logger: model.DiscardLogger, Method: http.MethodGet, URLPath: "/", }, @@ -33,6 +32,7 @@ func TestSequenceCaller(t *testing.T) { return resp, nil }, }, + Logger: model.DiscardLogger, }, &Endpoint{ BaseURL: "https://b.example.com/", @@ -41,6 +41,7 @@ func TestSequenceCaller(t *testing.T) { return nil, io.EOF }, }, + Logger: model.DiscardLogger, }, ) data, idx, err := sc.Call(context.Background()) @@ -58,7 +59,6 @@ func TestSequenceCaller(t *testing.T) { t.Run("first HTTP failure and we immediately stop", func(t *testing.T) { sc := NewSequenceCaller( &Descriptor{ - Logger: model.DiscardLogger, Method: http.MethodGet, URLPath: "/", }, @@ -73,6 +73,7 @@ func TestSequenceCaller(t *testing.T) { return resp, nil }, }, + Logger: model.DiscardLogger, }, &Endpoint{ BaseURL: "https://b.example.com/", @@ -81,6 +82,7 @@ func TestSequenceCaller(t *testing.T) { return nil, io.EOF }, }, + Logger: model.DiscardLogger, }, ) data, idx, err := sc.Call(context.Background()) @@ -99,7 +101,6 @@ func TestSequenceCaller(t *testing.T) { t.Run("first network failure, second success", func(t *testing.T) { sc := NewSequenceCaller( &Descriptor{ - Logger: model.DiscardLogger, Method: http.MethodGet, URLPath: "/", }, @@ -110,6 +111,7 @@ func TestSequenceCaller(t *testing.T) { return nil, io.EOF // should cause us to cycle to the second entry }, }, + Logger: model.DiscardLogger, }, &Endpoint{ BaseURL: "https://b.example.com/", @@ -122,6 +124,7 @@ func TestSequenceCaller(t *testing.T) { return resp, nil }, }, + Logger: model.DiscardLogger, }, ) data, idx, err := sc.Call(context.Background()) @@ -139,7 +142,6 @@ func TestSequenceCaller(t *testing.T) { t.Run("all network failure", func(t *testing.T) { sc := NewSequenceCaller( &Descriptor{ - Logger: model.DiscardLogger, Method: http.MethodGet, URLPath: "/", }, @@ -150,6 +152,7 @@ func TestSequenceCaller(t *testing.T) { return nil, io.EOF // should cause us to cycle to the next entry }, }, + Logger: model.DiscardLogger, }, &Endpoint{ BaseURL: "https://b.example.com/", @@ -158,6 +161,7 @@ func TestSequenceCaller(t *testing.T) { return nil, io.EOF // should cause us to cycle to the next entry }, }, + Logger: model.DiscardLogger, }, ) data, idx, err := sc.Call(context.Background()) @@ -182,7 +186,6 @@ func TestSequenceCaller(t *testing.T) { t.Run("first success", func(t *testing.T) { sc := NewSequenceCaller( &Descriptor{ - Logger: model.DiscardLogger, Method: http.MethodGet, URLPath: "/", }, @@ -197,6 +200,7 @@ func TestSequenceCaller(t *testing.T) { return resp, nil }, }, + Logger: model.DiscardLogger, }, &Endpoint{ BaseURL: "https://b.example.com/", @@ -209,6 +213,7 @@ func TestSequenceCaller(t *testing.T) { return resp, nil }, }, + Logger: model.DiscardLogger, }, ) expect := response{ @@ -231,7 +236,6 @@ func TestSequenceCaller(t *testing.T) { t.Run("first HTTP failure and we immediately stop", func(t *testing.T) { sc := NewSequenceCaller( &Descriptor{ - Logger: model.DiscardLogger, Method: http.MethodGet, URLPath: "/", }, @@ -246,6 +250,7 @@ func TestSequenceCaller(t *testing.T) { return resp, nil }, }, + Logger: model.DiscardLogger, }, &Endpoint{ BaseURL: "https://b.example.com/", @@ -254,6 +259,7 @@ func TestSequenceCaller(t *testing.T) { return nil, io.EOF }, }, + Logger: model.DiscardLogger, }, ) // even though there is a JSON body we don't care about reading it @@ -279,7 +285,6 @@ func TestSequenceCaller(t *testing.T) { t.Run("first network failure, second success", func(t *testing.T) { sc := NewSequenceCaller( &Descriptor{ - Logger: model.DiscardLogger, Method: http.MethodGet, URLPath: "/", }, @@ -290,6 +295,7 @@ func TestSequenceCaller(t *testing.T) { return nil, io.EOF // should cause us to try the next entry }, }, + Logger: model.DiscardLogger, }, &Endpoint{ BaseURL: "https://b.example.com/", @@ -302,6 +308,7 @@ func TestSequenceCaller(t *testing.T) { return resp, nil }, }, + Logger: model.DiscardLogger, }, ) expect := response{ @@ -324,7 +331,6 @@ func TestSequenceCaller(t *testing.T) { t.Run("all network failure", func(t *testing.T) { sc := NewSequenceCaller( &Descriptor{ - Logger: model.DiscardLogger, Method: http.MethodGet, URLPath: "/", }, @@ -335,6 +341,7 @@ func TestSequenceCaller(t *testing.T) { return nil, io.EOF // should cause us to try the next entry }, }, + Logger: model.DiscardLogger, }, &Endpoint{ BaseURL: "https://b.example.com/", @@ -343,6 +350,7 @@ func TestSequenceCaller(t *testing.T) { return nil, io.EOF // should cause us to try the next entry }, }, + Logger: model.DiscardLogger, }, ) var got response