Skip to content

Commit

Permalink
refactor(httpclientx): use better naming
Browse files Browse the repository at this point in the history
Prodded by @ainghazal to avoid confusion when discussing.
  • Loading branch information
bassosimone committed Jun 25, 2024
1 parent f10bad6 commit c369bb3
Show file tree
Hide file tree
Showing 29 changed files with 183 additions and 183 deletions.
2 changes: 1 addition & 1 deletion internal/enginelocate/cloudflare.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func cloudflareIPLookup(
// get the raw response body
data, err := httpclientx.GetRaw(
ctx,
httpclientx.NewEndpoint("https://www.cloudflare.com/cdn-cgi/trace"),
httpclientx.NewBaseURL("https://www.cloudflare.com/cdn-cgi/trace"),
&httpclientx.Config{
Authorization: "", // not needed
Client: httpClient,
Expand Down
2 changes: 1 addition & 1 deletion internal/enginelocate/ubuntu.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func ubuntuIPLookup(
// read the HTTP response and parse as XML
v, err := httpclientx.GetXML[*ubuntuResponse](
ctx,
httpclientx.NewEndpoint("https://geoip.ubuntu.com/lookup"),
httpclientx.NewBaseURL("https://geoip.ubuntu.com/lookup"),
&httpclientx.Config{
Authorization: "", // not needed
Client: httpClient,
Expand Down
32 changes: 16 additions & 16 deletions internal/httpclientx/DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,18 +76,18 @@ type Config struct {
UserAgent string
}

type Endpoint struct {
URL string
Host string // optional for cloudfronting
type BaseURL struct {
Value string // mandatory base-URL value
HostOverride string // optional for cloudfronting
}

func GetJSON[Output any](ctx context.Context, epnt *Endpoint, config *Config) (Output, error)
func GetJSON[Output any](ctx context.Context, base *BaseURL, config *Config) (Output, error)

func GetRaw(ctx context.Context, epnt *Endpoint, config *Config) ([]byte, error)
func GetRaw(ctx context.Context, base *BaseURL, config *Config) ([]byte, error)

func GetXML[Output any](ctx context.Context, epnt *Endpoint, config *Config) (Output, error)
func GetXML[Output any](ctx context.Context, base *BaseURL, config *Config) (Output, error)

func PostJSON[Input, Output any](ctx context.Context, epnt *Endpoint, input Input, config *Config) (Output, error)
func PostJSON[Input, Output any](ctx context.Context, base *BaseURL, input Input, config *Config) (Output, error)
```

(The `*Config` is the last argument because it is handy to create it inline when calling
Expand Down Expand Up @@ -124,9 +124,9 @@ To avoid logging bodies, one just needs to pass `model.DiscardLogger` as the
The code at `./internal/httpapi` performs sequential function calls. This design
does not interact well with the `enginenetx` package and its dial tactics. A better
strategy is to allow calls to be overlapped. This means that, if the `enginenetx`
is busy trying tactics for a given API endpoint, we eventually try to use the
subsequent (semantically-equivalent) endpoint after a given time, without waiting
for the first endpoint to complete.
is busy trying tactics for a given API base URL, we eventually try to use the
subsequent (semantically-equivalent) base URL after a given time, without waiting
for the first base URL to complete.

We allow for overlapped operations by defining these constructors:

Expand All @@ -144,7 +144,7 @@ They all construct the same `*Overlapped` struct, which looks like this:

```Go
type Overlapped[Output any] struct {
RunFunc func(ctx context.Context, epnt *Endpoint) (Output, error)
RunFunc func(ctx context.Context, base *BaseURL) (Output, error)

ScheduleInterval time.Duration
}
Expand All @@ -156,15 +156,15 @@ name (i.e., `NewOverlappedGetXML` configures `RunFunc` to run `GetXML`).
Then, we define the following method:

```Go
func (ovx *Overlapped[Output]) Run(ctx context.Context, epnts ...*Endpoint) (Output, error)
func (ovx *Overlapped[Output]) Run(ctx context.Context, bases ...*BaseURL) (Output, error)
```

This method starts N goroutines to issue the API calls with each endpoint URL. (A classic example
This method starts N goroutines to issue the API calls with each base URL. (A classic example
is for the URLs to be `https://0.th.ooni.org/`, `https://1.th.ooni.org/` and so on.)

By default, `ScheduleInterval` is 15 seconds. If the first endpoint URL does not provide a result
By default, `ScheduleInterval` is 15 seconds. If the first base URL does not provide a result
within 15 seconds, we try the second one. That is, every 15 seconds, we will attempt using
another endpoint URL, until there's a successful response or we run out of URLs.
another base URL, until there's a successful response or we run out of URLs.

As soon as we have a successful response, we cancel all the other pending operations
that may exist. Once all operations have terminated, we return to the caller.
Expand Down Expand Up @@ -329,7 +329,7 @@ that is part of the same package, while in this package we marshal in `PostJSON`
Consider the following code snippet:

```Go
resp, err := httpclientx.GetJSON[*APIResponse](ctx, epnt, config)
resp, err := httpclientx.GetJSON[*APIResponse](ctx, base, config)
runtimex.Assert((resp == nil && err != nil) || (resp != nil && err == nil), "ouch")
```

Expand Down
49 changes: 49 additions & 0 deletions internal/httpclientx/baseurl.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package httpclientx

import "github.com/ooni/probe-cli/v3/internal/model"

// BaseURL is an HTTP-endpoint base URL.
//
// The zero value is invalid; construct using [NewBaseURL].
type BaseURL struct {
// Value is the MANDATORY base-URL Value.
Value string

// HostOverride is the OPTIONAL host header to use for cloudfronting.
HostOverride string
}

// NewBaseURL constructs a new [*BaseURL] instance using the given URL.
func NewBaseURL(URL string) *BaseURL {
return &BaseURL{
Value: URL,
HostOverride: "",
}
}

// WithHostOverride returns a copy of the [*BaseURL] using the given host header override.
func (e *BaseURL) WithHostOverride(host string) *BaseURL {
return &BaseURL{
Value: e.Value,
HostOverride: host,
}
}

// NewBaseURLsFromModelOOAPIServices constructs new [*BaseURL] instances from the
// given [model.OOAPIService] instances, assigning the host header if "cloudfront", and
// skipping all the entries that are neither "https" not "cloudfront".
func NewBaseURLsFromModelOOAPIServices(svcs ...model.OOAPIService) (epnts []*BaseURL) {
for _, svc := range svcs {
epnt := NewBaseURL(svc.Address)
switch svc.Type {
case "cloudfront":
epnt = epnt.WithHostOverride(svc.Front)
fallthrough
case "https":
epnts = append(epnts, epnt)
default:
// skip entry
}
}
return
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,23 @@ import (
"github.com/ooni/probe-cli/v3/internal/model"
)

func TestEndpoint(t *testing.T) {
func TestBaseURL(t *testing.T) {
t.Run("the constructor only assigns the URL", func(t *testing.T) {
epnt := NewEndpoint("https://www.example.com/")
if epnt.URL != "https://www.example.com/" {
epnt := NewBaseURL("https://www.example.com/")
if epnt.Value != "https://www.example.com/" {
t.Fatal("unexpected URL")
}
if epnt.Host != "" {
if epnt.HostOverride != "" {
t.Fatal("unexpected host")
}
})

t.Run("we can optionally get a copy with an assigned host header", func(t *testing.T) {
epnt := NewEndpoint("https://www.example.com/").WithHostOverride("www.cloudfront.com")
if epnt.URL != "https://www.example.com/" {
epnt := NewBaseURL("https://www.example.com/").WithHostOverride("www.cloudfront.com")
if epnt.Value != "https://www.example.com/" {
t.Fatal("unexpected URL")
}
if epnt.Host != "www.cloudfront.com" {
if epnt.HostOverride != "www.cloudfront.com" {
t.Fatal("unexpected host")
}
})
Expand All @@ -50,14 +50,14 @@ func TestEndpoint(t *testing.T) {
Front: "",
}}

expect := []*Endpoint{{
URL: "https://www.example.com/",
expect := []*BaseURL{{
Value: "https://www.example.com/",
}, {
URL: "https://www.example.com/",
Host: "www.cloudfront.com",
Value: "https://www.example.com/",
HostOverride: "www.cloudfront.com",
}}

got := NewEndpointFromModelOOAPIServices(services...)
got := NewBaseURLsFromModelOOAPIServices(services...)
if diff := cmp.Diff(expect, got); diff != "" {
t.Fatal(diff)
}
Expand Down
49 changes: 0 additions & 49 deletions internal/httpclientx/endpoint.go

This file was deleted.

6 changes: 3 additions & 3 deletions internal/httpclientx/getjson.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,16 @@ import (
//
// - ctx is the cancellable context;
//
// - epnt is the HTTP [*Endpoint] to use;
// - epnt is the HTTP [*BaseURL] to use;
//
// - config contains the config.
//
// This function either returns an error or a valid Output.
func GetJSON[Output any](ctx context.Context, epnt *Endpoint, config *Config) (Output, error) {
func GetJSON[Output any](ctx context.Context, epnt *BaseURL, config *Config) (Output, error) {
return OverlappedIgnoreIndex(NewOverlappedGetJSON[Output](config).Run(ctx, epnt))
}

func getJSON[Output any](ctx context.Context, epnt *Endpoint, config *Config) (Output, error) {
func getJSON[Output any](ctx context.Context, epnt *BaseURL, config *Config) (Output, error) {
// read the raw body
rawrespbody, err := GetRaw(ctx, epnt, config)

Expand Down
16 changes: 8 additions & 8 deletions internal/httpclientx/getjson_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func TestGetJSON(t *testing.T) {
// invoke the API
resp, err := GetJSON[*apiResponse](
context.Background(),
NewEndpoint(server.URL),
NewBaseURL(server.URL),
&Config{
Client: http.DefaultClient,
Logger: model.DiscardLogger,
Expand Down Expand Up @@ -59,7 +59,7 @@ func TestGetJSON(t *testing.T) {
// invoke the API
resp, err := GetJSON[*apiResponse](
context.Background(),
NewEndpoint(server.URL),
NewBaseURL(server.URL),
&Config{
Client: http.DefaultClient,
Logger: model.DiscardLogger,
Expand Down Expand Up @@ -90,7 +90,7 @@ func TestGetJSON(t *testing.T) {
// invoke the API
resp, err := GetJSON[*apiResponse](
context.Background(),
NewEndpoint(server.URL),
NewBaseURL(server.URL),
&Config{
Client: http.DefaultClient,
Logger: model.DiscardLogger,
Expand Down Expand Up @@ -137,7 +137,7 @@ func TestGetJSONHeadersOkay(t *testing.T) {
// send the request and receive the response
apiresp, err := GetJSON[*apiResponse](
context.Background(),
NewEndpoint(server.URL).WithHostOverride("www.cloudfront.com"),
NewBaseURL(server.URL).WithHostOverride("www.cloudfront.com"),
&Config{
Authorization: "scribai",
Client: http.DefaultClient,
Expand Down Expand Up @@ -194,7 +194,7 @@ func TestGetJSONLoggingOkay(t *testing.T) {
// invoke the API
resp, err := GetJSON[*apiResponse](
context.Background(),
NewEndpoint(server.URL),
NewBaseURL(server.URL),
&Config{
Client: http.DefaultClient,
Logger: logger,
Expand Down Expand Up @@ -238,7 +238,7 @@ func TestGetJSONCorrectlyRejectsNilValues(t *testing.T) {
// invoke the API
resp, err := GetJSON[map[string]string](
context.Background(),
NewEndpoint(server.URL),
NewBaseURL(server.URL),
&Config{
Client: http.DefaultClient,
Logger: model.DiscardLogger,
Expand Down Expand Up @@ -268,7 +268,7 @@ func TestGetJSONCorrectlyRejectsNilValues(t *testing.T) {
// invoke the API
resp, err := GetJSON[*apiResponse](
context.Background(),
NewEndpoint(server.URL),
NewBaseURL(server.URL),
&Config{
Client: http.DefaultClient,
Logger: model.DiscardLogger,
Expand Down Expand Up @@ -298,7 +298,7 @@ func TestGetJSONCorrectlyRejectsNilValues(t *testing.T) {
// invoke the API
resp, err := GetJSON[[]string](
context.Background(),
NewEndpoint(server.URL),
NewBaseURL(server.URL),
&Config{
Client: http.DefaultClient,
Logger: model.DiscardLogger,
Expand Down
8 changes: 4 additions & 4 deletions internal/httpclientx/getraw.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,18 @@ import (
//
// - ctx is the cancellable context;
//
// - epnt is the HTTP [*Endpoint] to use;
// - epnt is the HTTP [*BaseURL] to use;
//
// - config is the config to use.
//
// This function either returns an error or a valid Output.
func GetRaw(ctx context.Context, epnt *Endpoint, config *Config) ([]byte, error) {
func GetRaw(ctx context.Context, epnt *BaseURL, config *Config) ([]byte, error) {
return OverlappedIgnoreIndex(NewOverlappedGetRaw(config).Run(ctx, epnt))
}

func getRaw(ctx context.Context, epnt *Endpoint, config *Config) ([]byte, error) {
func getRaw(ctx context.Context, epnt *BaseURL, config *Config) ([]byte, error) {
// construct the request to use
req, err := http.NewRequestWithContext(ctx, "GET", epnt.URL, nil)
req, err := http.NewRequestWithContext(ctx, "GET", epnt.Value, nil)
if err != nil {
return nil, err
}
Expand Down
8 changes: 4 additions & 4 deletions internal/httpclientx/getraw_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func TestGetRaw(t *testing.T) {

rawrespbody, err := GetRaw(
context.Background(),
NewEndpoint("\t"), // <- invalid URL that we cannot parse
NewBaseURL("\t"), // <- invalid URL that we cannot parse
&Config{
Client: http.DefaultClient,
Logger: model.DiscardLogger,
Expand Down Expand Up @@ -49,7 +49,7 @@ func TestGetRaw(t *testing.T) {

rawrespbody, err := GetRaw(
context.Background(),
NewEndpoint(server.URL),
NewBaseURL(server.URL),
&Config{
Client: http.DefaultClient,
Logger: model.DiscardLogger,
Expand Down Expand Up @@ -93,7 +93,7 @@ func TestGetRawHeadersOkay(t *testing.T) {
// send the request and receive the response
rawresp, err := GetRaw(
context.Background(),
NewEndpoint(server.URL).WithHostOverride("www.cloudfront.com"),
NewBaseURL(server.URL).WithHostOverride("www.cloudfront.com"),
&Config{
Authorization: "scribai",
Client: http.DefaultClient,
Expand Down Expand Up @@ -151,7 +151,7 @@ func TestGetRawLoggingOkay(t *testing.T) {

rawrespbody, err := GetRaw(
context.Background(),
NewEndpoint(server.URL),
NewBaseURL(server.URL),
&Config{
Client: http.DefaultClient,
Logger: logger,
Expand Down
Loading

0 comments on commit c369bb3

Please sign in to comment.