richer input: expose "probeservice.Client" from Session

[bassosimone]

This issue documents a possible code cleanup identified by @ainghazal in https://github.com/ooni/probe-cli/pull/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.