richer input: expose "probeservice.Client" from Session

[bassosimone]

This issue documents a possible code cleanup identified by @ainghazal in ooni/probe-cli#1625. Basically, rather than exposing a function for fetching each "richer input" type from a "Session", like we do now:

type ExperimentTargetLoaderSession interface {
	CheckIn(ctx context.Context, config *OOAPICheckInConfig) (*OOAPICheckInResult, error)

	FetchOpenVPNConfig(ctx context.Context, provider, cc string) (*OOAPIVPNProviderConfig, error)

	Logger() Logger
}

We can instead have something like the following:

type ExperimentTargetLoaderSession interface {
	ExperimentProbeServicesClient(ctx context.Context) (ExperimentProbeServicesClient, error)

	Logger() Logger
}

type ExperimentProbeServicesClient interface {
	// NewConfig returns a new [*httpclientx.Config] to communicate with the backend.
	NewConfig() *httpclientx.Config

	// NewProbeServicesEndpoints returns a copy of the list of known probe-services endpoints.
	NewProbeServicesEndpoints() []*httpclientx.Endpoint
}

The above code would not compile because of circular dependency, but this could be solved by moving the definition of *httpclientx.Config and *httpclientx.Endpoint to ./internal/model.

By doing that, communicating with the backend becomes an experiment-specific task and we can make lots of code experiment-private rather than having to implement fetches inside the ./internal/engine package.

Other solutions, would be possible, and we should discuss. The really important concept here is to move complexity from the ./internal/engine package to as close as possible to the experiments needing it.

The solution I presented above is just the first one that came to my mind.

[ainghazal]

to me NewProbeServicesEndpoints semantics is confusing. Is this supposed to return a list of all known probe services domains?

If we assume that at the point of passing control to experiment we have already selected one, perhaps it's enough with having a method for BaseURL()? Or perhaps I'm missing something here.

[bassosimone]

Let me expand the types we're going to use here:

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

type Config struct {
	// Authorization contains the OPTIONAL Authorization header value to use.
	Authorization string

	// Client is the MANDATORY [model.HTTPClient] to use.
	Client model.HTTPClient

	// Logger is the MANDATORY [model.Logger] to use.
	Logger model.Logger

	// MaxResponseBodySize OPTIONALLY limits the maximum body size. If not set, we
	// use the [DefaultMaxResponseBodySize] value.
	MaxResponseBodySize int64

	// UserAgent is the MANDATORY User-Agent header value to use.
	UserAgent string
}

type Endpoint struct {
	// URL is the MANDATORY endpoint URL.
	URL string

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

type Overlapped[Output any] struct { /* ... */ }

// usage:

// create a probe services client
psc, err := sess.NewProbeServicesClient(ctx)
if err != nil { /* ... */ }

// create config
config := psc.NewConfig()
// edit config...

// get available endpoints
epnts := psc.NewEndpoints()

// create overlapped function call
ovr := httpclientx.NewOverlappedPostJSON[*APIReq, *APIResp](apiReq, config)

// issue the overlapped function call
resp, _, err := ovr.Run(ctx, endpoints...)
if err != nil { /* ... */ }

[bassosimone]

From an initial back-and-forth with @ainghazal, it seems my proposal is, for now, quite unclear/obscure. I would circle back with @ainghazal, hammer down what is unclear and what could improved, then write a summary here below.