diff --git a/cmd/ooniprobe/internal/nettests/dash.go b/cmd/ooniprobe/internal/nettests/dash.go index 80f8618dff..89e073e120 100644 --- a/cmd/ooniprobe/internal/nettests/dash.go +++ b/cmd/ooniprobe/internal/nettests/dash.go @@ -1,5 +1,7 @@ package nettests +import "github.com/ooni/probe-cli/v3/internal/model" + // Dash test implementation type Dash struct { } @@ -10,5 +12,5 @@ func (d Dash) Run(ctl *Controller) error { if err != nil { return err } - return ctl.Run(builder, []string{""}) + return ctl.Run(builder, []model.ExperimentTarget{model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("")}) } diff --git a/cmd/ooniprobe/internal/nettests/dnscheck.go b/cmd/ooniprobe/internal/nettests/dnscheck.go index beebcd306d..4fe7de5fbd 100644 --- a/cmd/ooniprobe/internal/nettests/dnscheck.go +++ b/cmd/ooniprobe/internal/nettests/dnscheck.go @@ -10,7 +10,7 @@ import ( // DNSCheck nettest implementation. type DNSCheck struct{} -func (n DNSCheck) lookupURLs(ctl *Controller) ([]string, error) { +func (n DNSCheck) lookupURLs(ctl *Controller) ([]model.ExperimentTarget, error) { inputloader := &engine.InputLoader{ CheckInConfig: &model.OOAPICheckInConfig{ // not needed because we have default static input in the engine diff --git a/cmd/ooniprobe/internal/nettests/echcheck.go b/cmd/ooniprobe/internal/nettests/echcheck.go index 7d6ff526e8..15a45de713 100644 --- a/cmd/ooniprobe/internal/nettests/echcheck.go +++ b/cmd/ooniprobe/internal/nettests/echcheck.go @@ -1,5 +1,7 @@ package nettests +import "github.com/ooni/probe-cli/v3/internal/model" + // ECHCheck nettest implementation. type ECHCheck struct{} @@ -11,5 +13,5 @@ func (n ECHCheck) Run(ctl *Controller) error { } // providing an input containing an empty string causes the experiment // to recognize the empty string and use the default URL - return ctl.Run(builder, []string{""}) + return ctl.Run(builder, []model.ExperimentTarget{model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("")}) } diff --git a/cmd/ooniprobe/internal/nettests/facebook_messenger.go b/cmd/ooniprobe/internal/nettests/facebook_messenger.go index 1316babee5..7ecbd1e198 100644 --- a/cmd/ooniprobe/internal/nettests/facebook_messenger.go +++ b/cmd/ooniprobe/internal/nettests/facebook_messenger.go @@ -1,5 +1,7 @@ package nettests +import "github.com/ooni/probe-cli/v3/internal/model" + // FacebookMessenger test implementation type FacebookMessenger struct { } @@ -12,5 +14,5 @@ func (h FacebookMessenger) Run(ctl *Controller) error { if err != nil { return err } - return ctl.Run(builder, []string{""}) + return ctl.Run(builder, []model.ExperimentTarget{model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("")}) } diff --git a/cmd/ooniprobe/internal/nettests/http_header_field_manipulation.go b/cmd/ooniprobe/internal/nettests/http_header_field_manipulation.go index 6fdd39688f..09332c20a4 100644 --- a/cmd/ooniprobe/internal/nettests/http_header_field_manipulation.go +++ b/cmd/ooniprobe/internal/nettests/http_header_field_manipulation.go @@ -1,5 +1,7 @@ package nettests +import "github.com/ooni/probe-cli/v3/internal/model" + // HTTPHeaderFieldManipulation test implementation type HTTPHeaderFieldManipulation struct { } @@ -12,5 +14,5 @@ func (h HTTPHeaderFieldManipulation) Run(ctl *Controller) error { if err != nil { return err } - return ctl.Run(builder, []string{""}) + return ctl.Run(builder, []model.ExperimentTarget{model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("")}) } diff --git a/cmd/ooniprobe/internal/nettests/http_invalid_request_line.go b/cmd/ooniprobe/internal/nettests/http_invalid_request_line.go index fb87e462d6..d107d2e8b2 100644 --- a/cmd/ooniprobe/internal/nettests/http_invalid_request_line.go +++ b/cmd/ooniprobe/internal/nettests/http_invalid_request_line.go @@ -1,5 +1,7 @@ package nettests +import "github.com/ooni/probe-cli/v3/internal/model" + // HTTPInvalidRequestLine test implementation type HTTPInvalidRequestLine struct { } @@ -12,5 +14,5 @@ func (h HTTPInvalidRequestLine) Run(ctl *Controller) error { if err != nil { return err } - return ctl.Run(builder, []string{""}) + return ctl.Run(builder, []model.ExperimentTarget{model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("")}) } diff --git a/cmd/ooniprobe/internal/nettests/ndt.go b/cmd/ooniprobe/internal/nettests/ndt.go index b8848b0056..a862dde106 100644 --- a/cmd/ooniprobe/internal/nettests/ndt.go +++ b/cmd/ooniprobe/internal/nettests/ndt.go @@ -1,5 +1,7 @@ package nettests +import "github.com/ooni/probe-cli/v3/internal/model" + // NDT test implementation. We use v7 of NDT since 2020-03-12. type NDT struct { } @@ -11,5 +13,5 @@ func (n NDT) Run(ctl *Controller) error { if err != nil { return err } - return ctl.Run(builder, []string{""}) + return ctl.Run(builder, []model.ExperimentTarget{model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("")}) } diff --git a/cmd/ooniprobe/internal/nettests/nettests.go b/cmd/ooniprobe/internal/nettests/nettests.go index 15122ab416..e3cf272510 100644 --- a/cmd/ooniprobe/internal/nettests/nettests.go +++ b/cmd/ooniprobe/internal/nettests/nettests.go @@ -88,24 +88,22 @@ type Controller struct { // - on success, a list of strings containing URLs to test; // // - on failure, an error. -func (c *Controller) BuildAndSetInputIdxMap(testlist []model.OOAPIURLInfo) ([]string, error) { - var urls []string +func (c *Controller) BuildAndSetInputIdxMap(testlist []model.ExperimentTarget) ([]model.ExperimentTarget, error) { urlIDMap := make(map[int64]int64) for idx, url := range testlist { log.Debugf("Going over URL %d", idx) urlID, err := c.Probe.DB().CreateOrUpdateURL( - url.URL, url.CategoryCode, url.CountryCode, + url.Input(), url.Category(), url.Country(), ) if err != nil { log.Error("failed to add to the URL table") return nil, err } - log.Debugf("Mapped URL %s to idx %d and urlID %d", url.URL, idx, urlID) + log.Debugf("Mapped URL %s to idx %d and urlID %d", url.Input(), idx, urlID) urlIDMap[int64(idx)] = urlID - urls = append(urls, url.URL) } c.inputIdxMap = urlIDMap - return urls, nil + return testlist, nil } // SetNettestIndex is used to set the current nettest index and total nettest @@ -120,7 +118,7 @@ func (c *Controller) SetNettestIndex(i, n int) { // // This function will continue to run in most cases but will // immediately halt if something's wrong with the file system. -func (c *Controller) Run(builder model.ExperimentBuilder, inputs []string) error { +func (c *Controller) Run(builder model.ExperimentBuilder, inputs []model.ExperimentTarget) error { db := c.Probe.DB() // This will configure the controller as handler for the callbacks // called by ooni/probe-engine/experiment.Experiment. @@ -193,7 +191,7 @@ func (c *Controller) Run(builder model.ExperimentBuilder, inputs []string) error } c.msmts[idx64] = msmt - if input != "" { + if input.Input() != "" { c.OnProgress(0, fmt.Sprintf("processing input: %s", input)) } measurement, err := exp.MeasureWithContext(context.Background(), input) diff --git a/cmd/ooniprobe/internal/nettests/psiphon.go b/cmd/ooniprobe/internal/nettests/psiphon.go index 940340cc4e..ffba2bbf33 100644 --- a/cmd/ooniprobe/internal/nettests/psiphon.go +++ b/cmd/ooniprobe/internal/nettests/psiphon.go @@ -1,5 +1,7 @@ package nettests +import "github.com/ooni/probe-cli/v3/internal/model" + // Psiphon test implementation type Psiphon struct { } @@ -12,5 +14,5 @@ func (h Psiphon) Run(ctl *Controller) error { if err != nil { return err } - return ctl.Run(builder, []string{""}) + return ctl.Run(builder, []model.ExperimentTarget{model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("")}) } diff --git a/cmd/ooniprobe/internal/nettests/riseupvpn.go b/cmd/ooniprobe/internal/nettests/riseupvpn.go index 185fbcefe4..0f3566df34 100644 --- a/cmd/ooniprobe/internal/nettests/riseupvpn.go +++ b/cmd/ooniprobe/internal/nettests/riseupvpn.go @@ -1,5 +1,7 @@ package nettests +import "github.com/ooni/probe-cli/v3/internal/model" + // RiseupVPN test implementation type RiseupVPN struct { } @@ -12,6 +14,5 @@ func (h RiseupVPN) Run(ctl *Controller) error { if err != nil { return err } - - return ctl.Run(builder, []string{""}) + return ctl.Run(builder, []model.ExperimentTarget{model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("")}) } diff --git a/cmd/ooniprobe/internal/nettests/signal.go b/cmd/ooniprobe/internal/nettests/signal.go index 3a2df6b874..ceca5f311d 100644 --- a/cmd/ooniprobe/internal/nettests/signal.go +++ b/cmd/ooniprobe/internal/nettests/signal.go @@ -1,5 +1,7 @@ package nettests +import "github.com/ooni/probe-cli/v3/internal/model" + // Signal nettest implementation. type Signal struct{} @@ -11,5 +13,5 @@ func (h Signal) Run(ctl *Controller) error { if err != nil { return err } - return ctl.Run(builder, []string{""}) + return ctl.Run(builder, []model.ExperimentTarget{model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("")}) } diff --git a/cmd/ooniprobe/internal/nettests/stunreachability.go b/cmd/ooniprobe/internal/nettests/stunreachability.go index 186bb7bb98..c09ae23cb2 100644 --- a/cmd/ooniprobe/internal/nettests/stunreachability.go +++ b/cmd/ooniprobe/internal/nettests/stunreachability.go @@ -10,7 +10,7 @@ import ( // STUNReachability nettest implementation. type STUNReachability struct{} -func (n STUNReachability) lookupURLs(ctl *Controller) ([]string, error) { +func (n STUNReachability) lookupURLs(ctl *Controller) ([]model.ExperimentTarget, error) { inputloader := &engine.InputLoader{ CheckInConfig: &model.OOAPICheckInConfig{ // not needed because we have default static input in the engine diff --git a/cmd/ooniprobe/internal/nettests/telegram.go b/cmd/ooniprobe/internal/nettests/telegram.go index 82d75d82c2..87c6099f89 100644 --- a/cmd/ooniprobe/internal/nettests/telegram.go +++ b/cmd/ooniprobe/internal/nettests/telegram.go @@ -1,5 +1,7 @@ package nettests +import "github.com/ooni/probe-cli/v3/internal/model" + // Telegram test implementation type Telegram struct { } @@ -12,5 +14,5 @@ func (h Telegram) Run(ctl *Controller) error { if err != nil { return err } - return ctl.Run(builder, []string{""}) + return ctl.Run(builder, []model.ExperimentTarget{model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("")}) } diff --git a/cmd/ooniprobe/internal/nettests/tor.go b/cmd/ooniprobe/internal/nettests/tor.go index 96bfbb7d2f..9f229c31ce 100644 --- a/cmd/ooniprobe/internal/nettests/tor.go +++ b/cmd/ooniprobe/internal/nettests/tor.go @@ -1,5 +1,7 @@ package nettests +import "github.com/ooni/probe-cli/v3/internal/model" + // Tor test implementation type Tor struct { } @@ -12,5 +14,5 @@ func (h Tor) Run(ctl *Controller) error { if err != nil { return err } - return ctl.Run(builder, []string{""}) + return ctl.Run(builder, []model.ExperimentTarget{model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("")}) } diff --git a/cmd/ooniprobe/internal/nettests/torsf.go b/cmd/ooniprobe/internal/nettests/torsf.go index 494dae4743..ef3b6425fb 100644 --- a/cmd/ooniprobe/internal/nettests/torsf.go +++ b/cmd/ooniprobe/internal/nettests/torsf.go @@ -1,5 +1,7 @@ package nettests +import "github.com/ooni/probe-cli/v3/internal/model" + // TorSf test implementation type TorSf struct { } @@ -10,7 +12,7 @@ func (h TorSf) Run(ctl *Controller) error { if err != nil { return err } - return ctl.Run(builder, []string{""}) + return ctl.Run(builder, []model.ExperimentTarget{model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("")}) } func (h TorSf) onlyBackground() {} diff --git a/cmd/ooniprobe/internal/nettests/vanillator.go b/cmd/ooniprobe/internal/nettests/vanillator.go index 11d7266549..1e154181c7 100644 --- a/cmd/ooniprobe/internal/nettests/vanillator.go +++ b/cmd/ooniprobe/internal/nettests/vanillator.go @@ -1,5 +1,7 @@ package nettests +import "github.com/ooni/probe-cli/v3/internal/model" + // VanillaTor test implementation type VanillaTor struct { } @@ -10,7 +12,7 @@ func (h VanillaTor) Run(ctl *Controller) error { if err != nil { return err } - return ctl.Run(builder, []string{""}) + return ctl.Run(builder, []model.ExperimentTarget{model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("")}) } func (h VanillaTor) onlyBackground() {} diff --git a/cmd/ooniprobe/internal/nettests/web_connectivity.go b/cmd/ooniprobe/internal/nettests/web_connectivity.go index fc643c0091..5d2fc939ea 100644 --- a/cmd/ooniprobe/internal/nettests/web_connectivity.go +++ b/cmd/ooniprobe/internal/nettests/web_connectivity.go @@ -8,7 +8,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/model" ) -func (n WebConnectivity) lookupURLs(ctl *Controller, categories []string) ([]string, error) { +func (n WebConnectivity) lookupURLs(ctl *Controller, categories []string) ([]model.ExperimentTarget, error) { inputloader := &engine.InputLoader{ CheckInConfig: &model.OOAPICheckInConfig{ // Setting Charging and OnWiFi to true causes the CheckIn diff --git a/cmd/ooniprobe/internal/nettests/whatsapp.go b/cmd/ooniprobe/internal/nettests/whatsapp.go index 4660abe006..d46953b262 100644 --- a/cmd/ooniprobe/internal/nettests/whatsapp.go +++ b/cmd/ooniprobe/internal/nettests/whatsapp.go @@ -1,5 +1,7 @@ package nettests +import "github.com/ooni/probe-cli/v3/internal/model" + // WhatsApp test implementation type WhatsApp struct { } @@ -12,5 +14,5 @@ func (h WhatsApp) Run(ctl *Controller) error { if err != nil { return err } - return ctl.Run(builder, []string{""}) + return ctl.Run(builder, []model.ExperimentTarget{model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("")}) } diff --git a/internal/engine/experiment.go b/internal/engine/experiment.go index 41afdddebf..61197c0d8d 100644 --- a/internal/engine/experiment.go +++ b/internal/engine/experiment.go @@ -58,6 +58,8 @@ type experiment struct { testVersion string } +var _ model.Experiment = &experiment{} + // newExperiment creates a new [*experiment] given a [model.ExperimentMeasurer]. func newExperiment(sess *Session, measurer model.ExperimentMeasurer) *experiment { return &experiment{ @@ -180,7 +182,8 @@ func (e *experiment) OpenReportContext(ctx context.Context) error { } // MeasureWithContext implements [model.Experiment]. -func (e *experiment) MeasureWithContext(ctx context.Context, input string) (*model.Measurement, error) { +func (e *experiment) MeasureWithContext( + ctx context.Context, target model.ExperimentTarget) (*model.Measurement, error) { // Here we ensure that we have already looked up the probe location // information such that we correctly populate the measurement and also // VERY IMPORTANTLY to scrub the IP address from the measurement. @@ -204,7 +207,7 @@ func (e *experiment) MeasureWithContext(ctx context.Context, input string) (*mod // by adding the test keys etc. Please, note that, as of 2024-06-05, we're using // the measurement Input to provide input to an experiment. We'll probably // change this, when we'll have finished implementing richer input. - measurement := e.newMeasurement(input) + measurement := e.newMeasurement(target.Input()) // Record when we started the experiment, to compute the runtime. start := time.Now() diff --git a/internal/engine/experiment_integration_test.go b/internal/engine/experiment_integration_test.go index a9906b6d84..1d9a7f91a3 100644 --- a/internal/engine/experiment_integration_test.go +++ b/internal/engine/experiment_integration_test.go @@ -177,7 +177,8 @@ func TestSetCallbacks(t *testing.T) { } register := ®isterCallbacksCalled{} builder.SetCallbacks(register) - if _, err := builder.NewExperiment().MeasureWithContext(context.Background(), ""); err != nil { + target := model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("") + if _, err := builder.NewExperiment().MeasureWithContext(context.Background(), target); err != nil { t.Fatal(err) } if register.onProgressCalled == false { @@ -221,7 +222,8 @@ func TestMeasurementFailure(t *testing.T) { if err := builder.SetOptionAny("ReturnError", true); err != nil { t.Fatal(err) } - measurement, err := builder.NewExperiment().MeasureWithContext(context.Background(), "") + target := model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("") + measurement, err := builder.NewExperiment().MeasureWithContext(context.Background(), target) if err == nil { t.Fatal("expected an error here") } @@ -255,7 +257,8 @@ func runexperimentflow(t *testing.T, experiment model.Experiment, input string) if experiment.ReportID() == "" { t.Fatal("reportID should not be empty here") } - measurement, err := experiment.MeasureWithContext(ctx, input) + target := model.NewOOAPIURLInfoWithDefaultCategoryAndCountry(input) + measurement, err := experiment.MeasureWithContext(ctx, target) if err != nil { t.Fatal(err) } @@ -413,7 +416,8 @@ func TestMeasureLookupLocationFailure(t *testing.T) { exp := newExperiment(sess, new(antaniMeasurer)) ctx, cancel := context.WithCancel(context.Background()) cancel() // so we fail immediately - if _, err := exp.MeasureWithContext(ctx, "xx"); err == nil { + target := model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("xx") + if _, err := exp.MeasureWithContext(ctx, target); err == nil { t.Fatal("expected an error here") } } diff --git a/internal/engine/inputloader.go b/internal/engine/inputloader.go index d338703203..06c31b5d57 100644 --- a/internal/engine/inputloader.go +++ b/internal/engine/inputloader.go @@ -113,7 +113,7 @@ type InputLoader struct { // Load attempts to load input using the specified input loader. We will // return a list of URLs because this is the only input we support. -func (il *InputLoader) Load(ctx context.Context) ([]model.OOAPIURLInfo, error) { +func (il *InputLoader) Load(ctx context.Context) ([]model.ExperimentTarget, error) { switch il.InputPolicy { case model.InputOptional: return il.loadOptional() @@ -129,26 +129,29 @@ func (il *InputLoader) Load(ctx context.Context) ([]model.OOAPIURLInfo, error) { } // loadNone implements the InputNone policy. -func (il *InputLoader) loadNone() ([]model.OOAPIURLInfo, error) { +func (il *InputLoader) loadNone() ([]model.ExperimentTarget, error) { if len(il.StaticInputs) > 0 || len(il.SourceFiles) > 0 { return nil, ErrNoInputExpected } - // Note that we need to return a single empty entry. - return []model.OOAPIURLInfo{{}}, nil + // Implementation note: the convention for input-less experiments is that + // they require a single entry containing an empty input. + entry := model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("") + return []model.ExperimentTarget{entry}, nil } // loadOptional implements the InputOptional policy. -func (il *InputLoader) loadOptional() ([]model.OOAPIURLInfo, error) { +func (il *InputLoader) loadOptional() ([]model.ExperimentTarget, error) { inputs, err := il.loadLocal() if err == nil && len(inputs) <= 0 { - // Note that we need to return a single empty entry. - inputs = []model.OOAPIURLInfo{{}} + // Implementation note: the convention for input-less experiments is that + // they require a single entry containing an empty input. + inputs = []model.ExperimentTarget{model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("")} } return inputs, err } // loadStrictlyRequired implements the InputStrictlyRequired policy. -func (il *InputLoader) loadStrictlyRequired(_ context.Context) ([]model.OOAPIURLInfo, error) { +func (il *InputLoader) loadStrictlyRequired(_ context.Context) ([]model.ExperimentTarget, error) { inputs, err := il.loadLocal() if err != nil || len(inputs) > 0 { return inputs, err @@ -157,7 +160,7 @@ func (il *InputLoader) loadStrictlyRequired(_ context.Context) ([]model.OOAPIURL } // loadOrQueryBackend implements the InputOrQueryBackend policy. -func (il *InputLoader) loadOrQueryBackend(ctx context.Context) ([]model.OOAPIURLInfo, error) { +func (il *InputLoader) loadOrQueryBackend(ctx context.Context) ([]model.ExperimentTarget, error) { inputs, err := il.loadLocal() if err != nil || len(inputs) > 0 { return inputs, err @@ -239,12 +242,12 @@ func StaticBareInputForExperiment(name string) ([]string, error) { // staticInputForExperiment returns the static input for the given experiment // or an error if there's no static input for the experiment. -func staticInputForExperiment(name string) ([]model.OOAPIURLInfo, error) { - return stringListToModelURLInfo(StaticBareInputForExperiment(name)) +func staticInputForExperiment(name string) ([]model.ExperimentTarget, error) { + return inputLoaderStringListToModelExperimentTarget(StaticBareInputForExperiment(name)) } // loadOrStaticDefault implements the InputOrStaticDefault policy. -func (il *InputLoader) loadOrStaticDefault(_ context.Context) ([]model.OOAPIURLInfo, error) { +func (il *InputLoader) loadOrStaticDefault(_ context.Context) ([]model.ExperimentTarget, error) { inputs, err := il.loadLocal() if err != nil || len(inputs) > 0 { return inputs, err @@ -253,10 +256,10 @@ func (il *InputLoader) loadOrStaticDefault(_ context.Context) ([]model.OOAPIURLI } // loadLocal loads inputs from StaticInputs and SourceFiles. -func (il *InputLoader) loadLocal() ([]model.OOAPIURLInfo, error) { - inputs := []model.OOAPIURLInfo{} +func (il *InputLoader) loadLocal() ([]model.ExperimentTarget, error) { + inputs := []model.ExperimentTarget{} for _, input := range il.StaticInputs { - inputs = append(inputs, model.OOAPIURLInfo{URL: input}) + inputs = append(inputs, model.NewOOAPIURLInfoWithDefaultCategoryAndCountry(input)) } for _, filepath := range il.SourceFiles { extra, err := il.readfile(filepath, fsx.OpenFile) @@ -277,8 +280,8 @@ type inputLoaderOpenFn func(filepath string) (fs.File, error) // readfile reads inputs from the specified file. The open argument should be // compatible with stdlib's fs.Open and helps us with unit testing. -func (il *InputLoader) readfile(filepath string, open inputLoaderOpenFn) ([]model.OOAPIURLInfo, error) { - inputs := []model.OOAPIURLInfo{} +func (il *InputLoader) readfile(filepath string, open inputLoaderOpenFn) ([]model.ExperimentTarget, error) { + inputs := []model.ExperimentTarget{} filep, err := open(filepath) if err != nil { return nil, err @@ -291,7 +294,7 @@ func (il *InputLoader) readfile(filepath string, open inputLoaderOpenFn) ([]mode for scanner.Scan() { line := scanner.Text() if line != "" { - inputs = append(inputs, model.OOAPIURLInfo{URL: line}) + inputs = append(inputs, model.NewOOAPIURLInfoWithDefaultCategoryAndCountry(line)) } } if scanner.Err() != nil { @@ -301,7 +304,7 @@ func (il *InputLoader) readfile(filepath string, open inputLoaderOpenFn) ([]mode } // loadRemote loads inputs from a remote source. -func (il *InputLoader) loadRemote(ctx context.Context) ([]model.OOAPIURLInfo, error) { +func (il *InputLoader) loadRemote(ctx context.Context) ([]model.ExperimentTarget, error) { switch registry.CanonicalizeExperimentName(il.ExperimentName) { case "openvpn": // TODO(ainghazal): given the semantics of the current API call, in an ideal world we'd need to pass @@ -316,7 +319,7 @@ func (il *InputLoader) loadRemote(ctx context.Context) ([]model.OOAPIURLInfo, er } // loadRemoteWebConnectivity loads webconnectivity inputs from a remote source. -func (il *InputLoader) loadRemoteWebConnectivity(ctx context.Context) ([]model.OOAPIURLInfo, error) { +func (il *InputLoader) loadRemoteWebConnectivity(ctx context.Context) ([]model.ExperimentTarget, error) { config := il.CheckInConfig if config == nil { // Note: Session.CheckIn documentation says it will fill in @@ -332,11 +335,28 @@ func (il *InputLoader) loadRemoteWebConnectivity(ctx context.Context) ([]model.O if reply.WebConnectivity == nil || len(reply.WebConnectivity.URLs) <= 0 { return nil, ErrNoURLsReturned } - return reply.WebConnectivity.URLs, nil + output := inputLoaderModelOOAPIURLInfoToModelExperimentTarget(reply.WebConnectivity.URLs) + return output, nil +} + +func inputLoaderModelOOAPIURLInfoToModelExperimentTarget( + inputs []model.OOAPIURLInfo) (outputs []model.ExperimentTarget) { + for _, input := range inputs { + // Note: Dammit! Before we switch to go1.22 we need to continue to + // stay careful about the variable over which we're looping! + // + // See https://go.dev/blog/loopvar-preview for more information. + outputs = append(outputs, &model.OOAPIURLInfo{ + CategoryCode: input.CategoryCode, + CountryCode: input.CountryCode, + URL: input.URL, + }) + } + return } // loadRemoteOpenVPN loads openvpn inputs from a remote source. -func (il *InputLoader) loadRemoteOpenVPN(ctx context.Context) ([]model.OOAPIURLInfo, error) { +func (il *InputLoader) loadRemoteOpenVPN(ctx context.Context) ([]model.ExperimentTarget, error) { // VPN Inputs do not match exactly the semantics expected from [model.OOAPIURLInfo], // since OOAPIURLInfo is oriented towards webconnectivity, // but we force VPN targets in the URL and ignore all the other fields. @@ -352,7 +372,8 @@ func (il *InputLoader) loadRemoteOpenVPN(ctx context.Context) ([]model.OOAPIURLI // hitting the API too many times. reply, err := il.fetchOpenVPNConfig(ctx, provider) if err != nil { - return urls, err + output := inputLoaderModelOOAPIURLInfoToModelExperimentTarget(urls) + return output, err } for _, input := range reply.Inputs { urls = append(urls, model.OOAPIURLInfo{URL: input}) @@ -365,7 +386,8 @@ func (il *InputLoader) loadRemoteOpenVPN(ctx context.Context) ([]model.OOAPIURLI // the experiment on the backend. return nil, ErrNoURLsReturned } - return urls, nil + output := inputLoaderModelOOAPIURLInfoToModelExperimentTarget(urls) + return output, nil } // checkIn executes the check-in and filters the returned URLs to exclude @@ -427,25 +449,21 @@ func (il *InputLoader) logger() InputLoaderLogger { return log.Log } -// stringListToModelURLInfo is an utility function to convert -// a list of strings containing URLs into a list of model.URLInfo +// inputLoaderStringListToModelExperimentTarget is an utility function to convert +// a list of strings containing URLs into a list of model.ExperimentTarget // which would have been returned by an hypothetical backend // API serving input for a test for which we don't have an API // yet (e.g., stunreachability and dnscheck). -func stringListToModelURLInfo(input []string, err error) ([]model.OOAPIURLInfo, error) { +func inputLoaderStringListToModelExperimentTarget(input []string, err error) ([]model.ExperimentTarget, error) { if err != nil { return nil, err } - var output []model.OOAPIURLInfo + var output []model.ExperimentTarget for _, URL := range input { if _, err := url.Parse(URL); err != nil { return nil, err } - output = append(output, model.OOAPIURLInfo{ - CategoryCode: "MISC", // hard to find a category - CountryCode: "XX", // representing no country - URL: URL, - }) + output = append(output, model.NewOOAPIURLInfoWithDefaultCategoryAndCountry(URL)) } return output, nil } diff --git a/internal/engine/inputloader_test.go b/internal/engine/inputloader_test.go index bc8b9686d5..70f3f86cfd 100644 --- a/internal/engine/inputloader_test.go +++ b/internal/engine/inputloader_test.go @@ -79,7 +79,7 @@ func TestInputLoaderInputNoneWithNoInput(t *testing.T) { if err != nil { t.Fatal(err) } - if len(out) != 1 || out[0].URL != "" { + if len(out) != 1 || out[0].Input() != "" { t.Fatal("not the output we expected") } } @@ -93,7 +93,7 @@ func TestInputLoaderInputOptionalWithNoInput(t *testing.T) { if err != nil { t.Fatal(err) } - if len(out) != 1 || out[0].URL != "" { + if len(out) != 1 || out[0].Input() != "" { t.Fatal("not the output we expected") } } @@ -115,12 +115,32 @@ func TestInputLoaderInputOptionalWithInput(t *testing.T) { if len(out) != 5 { t.Fatal("not the output length we expected") } - expect := []model.OOAPIURLInfo{ - {URL: "https://www.google.com/"}, - {URL: "https://www.x.org/"}, - {URL: "https://www.slashdot.org/"}, - {URL: "https://abc.xyz/"}, - {URL: "https://run.ooni.io/"}, + expect := []model.ExperimentTarget{ + &model.OOAPIURLInfo{ + CountryCode: model.DefaultCountryCode, + CategoryCode: model.DefaultCategoryCode, + URL: "https://www.google.com/", + }, + &model.OOAPIURLInfo{ + CountryCode: model.DefaultCountryCode, + CategoryCode: model.DefaultCategoryCode, + URL: "https://www.x.org/", + }, + &model.OOAPIURLInfo{ + CountryCode: model.DefaultCountryCode, + CategoryCode: model.DefaultCategoryCode, + URL: "https://www.slashdot.org/", + }, + &model.OOAPIURLInfo{ + CountryCode: model.DefaultCountryCode, + CategoryCode: model.DefaultCategoryCode, + URL: "https://abc.xyz/", + }, + &model.OOAPIURLInfo{ + CountryCode: model.DefaultCountryCode, + CategoryCode: model.DefaultCategoryCode, + URL: "https://run.ooni.io/", + }, } if diff := cmp.Diff(out, expect); diff != "" { t.Fatal(diff) @@ -164,12 +184,32 @@ func TestInputLoaderInputStrictlyRequiredWithInput(t *testing.T) { if len(out) != 5 { t.Fatal("not the output length we expected") } - expect := []model.OOAPIURLInfo{ - {URL: "https://www.google.com/"}, - {URL: "https://www.x.org/"}, - {URL: "https://www.slashdot.org/"}, - {URL: "https://abc.xyz/"}, - {URL: "https://run.ooni.io/"}, + expect := []model.ExperimentTarget{ + &model.OOAPIURLInfo{ + CountryCode: model.DefaultCountryCode, + CategoryCode: model.DefaultCategoryCode, + URL: "https://www.google.com/", + }, + &model.OOAPIURLInfo{ + CountryCode: model.DefaultCountryCode, + CategoryCode: model.DefaultCategoryCode, + URL: "https://www.x.org/", + }, + &model.OOAPIURLInfo{ + CountryCode: model.DefaultCountryCode, + CategoryCode: model.DefaultCategoryCode, + URL: "https://www.slashdot.org/", + }, + &model.OOAPIURLInfo{ + CountryCode: model.DefaultCountryCode, + CategoryCode: model.DefaultCategoryCode, + URL: "https://abc.xyz/", + }, + &model.OOAPIURLInfo{ + CountryCode: model.DefaultCountryCode, + CategoryCode: model.DefaultCategoryCode, + URL: "https://run.ooni.io/", + }, } if diff := cmp.Diff(out, expect); diff != "" { t.Fatal(diff) @@ -227,12 +267,32 @@ func TestInputLoaderInputOrStaticDefaultWithInput(t *testing.T) { if len(out) != 5 { t.Fatal("not the output length we expected") } - expect := []model.OOAPIURLInfo{ - {URL: "https://www.google.com/"}, - {URL: "https://www.x.org/"}, - {URL: "https://www.slashdot.org/"}, - {URL: "https://abc.xyz/"}, - {URL: "https://run.ooni.io/"}, + expect := []model.ExperimentTarget{ + &model.OOAPIURLInfo{ + CountryCode: model.DefaultCountryCode, + CategoryCode: model.DefaultCategoryCode, + URL: "https://www.google.com/", + }, + &model.OOAPIURLInfo{ + CountryCode: model.DefaultCountryCode, + CategoryCode: model.DefaultCategoryCode, + URL: "https://www.x.org/", + }, + &model.OOAPIURLInfo{ + CountryCode: model.DefaultCountryCode, + CategoryCode: model.DefaultCategoryCode, + URL: "https://www.slashdot.org/", + }, + &model.OOAPIURLInfo{ + CountryCode: model.DefaultCountryCode, + CategoryCode: model.DefaultCategoryCode, + URL: "https://abc.xyz/", + }, + &model.OOAPIURLInfo{ + CountryCode: model.DefaultCountryCode, + CategoryCode: model.DefaultCategoryCode, + URL: "https://run.ooni.io/", + }, } if diff := cmp.Diff(out, expect); diff != "" { t.Fatal(diff) @@ -274,13 +334,13 @@ func TestInputLoaderInputOrStaticDefaultWithoutInputDNSCheck(t *testing.T) { } for idx := 0; idx < len(dnsCheckDefaultInput); idx++ { e := out[idx] - if e.CategoryCode != "MISC" { + if e.Category() != model.DefaultCategoryCode { t.Fatal("invalid category code") } - if e.CountryCode != "XX" { + if e.Country() != model.DefaultCountryCode { t.Fatal("invalid country code") } - if e.URL != dnsCheckDefaultInput[idx] { + if e.Input() != dnsCheckDefaultInput[idx] { t.Fatal("invalid URL") } } @@ -301,13 +361,13 @@ func TestInputLoaderInputOrStaticDefaultWithoutInputStunReachability(t *testing. } for idx := 0; idx < len(stunReachabilityDefaultInput); idx++ { e := out[idx] - if e.CategoryCode != "MISC" { + if e.Category() != model.DefaultCategoryCode { t.Fatal("invalid category code") } - if e.CountryCode != "XX" { + if e.Country() != model.DefaultCountryCode { t.Fatal("invalid country code") } - if e.URL != stunReachabilityDefaultInput[idx] { + if e.Input() != stunReachabilityDefaultInput[idx] { t.Fatal("invalid URL") } } @@ -354,12 +414,32 @@ func TestInputLoaderInputOrQueryBackendWithInput(t *testing.T) { if len(out) != 5 { t.Fatal("not the output length we expected") } - expect := []model.OOAPIURLInfo{ - {URL: "https://www.google.com/"}, - {URL: "https://www.x.org/"}, - {URL: "https://www.slashdot.org/"}, - {URL: "https://abc.xyz/"}, - {URL: "https://run.ooni.io/"}, + expect := []model.ExperimentTarget{ + &model.OOAPIURLInfo{ + CountryCode: model.DefaultCountryCode, + CategoryCode: model.DefaultCategoryCode, + URL: "https://www.google.com/", + }, + &model.OOAPIURLInfo{ + CountryCode: model.DefaultCountryCode, + CategoryCode: model.DefaultCategoryCode, + URL: "https://www.x.org/", + }, + &model.OOAPIURLInfo{ + CountryCode: model.DefaultCountryCode, + CategoryCode: model.DefaultCategoryCode, + URL: "https://www.slashdot.org/", + }, + &model.OOAPIURLInfo{ + CountryCode: model.DefaultCountryCode, + CategoryCode: model.DefaultCategoryCode, + URL: "https://abc.xyz/", + }, + &model.OOAPIURLInfo{ + CountryCode: model.DefaultCountryCode, + CategoryCode: model.DefaultCategoryCode, + URL: "https://run.ooni.io/", + }, } if diff := cmp.Diff(out, expect); diff != "" { t.Fatal(diff) @@ -527,21 +607,24 @@ func TestInputLoaderCheckInSuccessWithNoURLs(t *testing.T) { } func TestInputLoaderCheckInSuccessWithSomeURLs(t *testing.T) { - expect := []model.OOAPIURLInfo{{ + inputs0 := model.OOAPIURLInfo{ CategoryCode: "NEWS", CountryCode: "IT", URL: "https://repubblica.it", - }, { + } + inputs1 := model.OOAPIURLInfo{ CategoryCode: "NEWS", CountryCode: "IT", URL: "https://corriere.it", - }} + } + inputs := []model.OOAPIURLInfo{inputs0, inputs1} + expect := []model.ExperimentTarget{&inputs0, &inputs1} il := &InputLoader{ Session: &InputLoaderMockableSession{ Output: &model.OOAPICheckInResult{ Tests: model.OOAPICheckInResultNettests{ WebConnectivity: &model.OOAPICheckInInfoWebConnectivity{ - URLs: expect, + URLs: inputs, }, }, }, @@ -711,7 +794,7 @@ func TestStringListToModelURLInfoWithValidInput(t *testing.T) { "stun://stun.voip.blackberry.com:3478", "stun://stun.altar.com.pl:3478", } - output, err := stringListToModelURLInfo(input, nil) + output, err := inputLoaderStringListToModelExperimentTarget(input, nil) if err != nil { t.Fatal(err) } @@ -719,13 +802,13 @@ func TestStringListToModelURLInfoWithValidInput(t *testing.T) { t.Fatal("unexpected output length") } for idx := 0; idx < len(input); idx++ { - if input[idx] != output[idx].URL { + if input[idx] != output[idx].Input() { t.Fatal("unexpected entry") } - if output[idx].CategoryCode != "MISC" { + if output[idx].Category() != model.DefaultCategoryCode { t.Fatal("unexpected category") } - if output[idx].CountryCode != "XX" { + if output[idx].Country() != model.DefaultCountryCode { t.Fatal("unexpected country") } } @@ -737,7 +820,7 @@ func TestStringListToModelURLInfoWithInvalidInput(t *testing.T) { "\t", // <- not a valid URL "stun://stun.altar.com.pl:3478", } - output, err := stringListToModelURLInfo(input, nil) + output, err := inputLoaderStringListToModelExperimentTarget(input, nil) if err == nil || !strings.HasSuffix(err.Error(), "invalid control character in URL") { t.Fatal("no the error we expected", err) } @@ -753,7 +836,7 @@ func TestStringListToModelURLInfoWithError(t *testing.T) { "stun://stun.altar.com.pl:3478", } expected := errors.New("mocked error") - output, err := stringListToModelURLInfo(input, expected) + output, err := inputLoaderStringListToModelExperimentTarget(input, expected) if !errors.Is(err, expected) { t.Fatal("not the error we expected", err) } diff --git a/internal/mocks/experiment.go b/internal/mocks/experiment.go index 3c833b8cb8..3063ab3ae1 100644 --- a/internal/mocks/experiment.go +++ b/internal/mocks/experiment.go @@ -17,7 +17,7 @@ type Experiment struct { MockReportID func() string MockMeasureWithContext func( - ctx context.Context, input string) (measurement *model.Measurement, err error) + ctx context.Context, target model.ExperimentTarget) (measurement *model.Measurement, err error) MockSaveMeasurement func(measurement *model.Measurement, filePath string) error @@ -44,8 +44,8 @@ func (e *Experiment) ReportID() string { } func (e *Experiment) MeasureWithContext( - ctx context.Context, input string) (measurement *model.Measurement, err error) { - return e.MockMeasureWithContext(ctx, input) + ctx context.Context, target model.ExperimentTarget) (measurement *model.Measurement, err error) { + return e.MockMeasureWithContext(ctx, target) } func (e *Experiment) SaveMeasurement(measurement *model.Measurement, filePath string) error { diff --git a/internal/mocks/experiment_test.go b/internal/mocks/experiment_test.go index 017752d178..ecd1f157b9 100644 --- a/internal/mocks/experiment_test.go +++ b/internal/mocks/experiment_test.go @@ -60,11 +60,15 @@ func TestExperiment(t *testing.T) { t.Run("MeasureWithContext", func(t *testing.T) { expected := errors.New("mocked err") e := &Experiment{ - MockMeasureWithContext: func(ctx context.Context, input string) (measurement *model.Measurement, err error) { + MockMeasureWithContext: func( + ctx context.Context, target model.ExperimentTarget) (measurement *model.Measurement, err error) { return nil, expected }, } - out, err := e.MeasureWithContext(context.Background(), "xo") + out, err := e.MeasureWithContext( + context.Background(), + model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("https://www.example.com/"), + ) if !errors.Is(err, expected) { t.Fatal("unexpected err", err) } diff --git a/internal/mocks/experimentinputloader.go b/internal/mocks/experimentinputloader.go index 5c7234348d..c46db13bef 100644 --- a/internal/mocks/experimentinputloader.go +++ b/internal/mocks/experimentinputloader.go @@ -8,12 +8,12 @@ import ( // ExperimentInputLoader mocks model.ExperimentInputLoader type ExperimentInputLoader struct { - MockLoad func(ctx context.Context) ([]model.OOAPIURLInfo, error) + MockLoad func(ctx context.Context) ([]model.ExperimentTarget, error) } var _ model.ExperimentInputLoader = &ExperimentInputLoader{} // Load calls MockLoad -func (eil *ExperimentInputLoader) Load(ctx context.Context) ([]model.OOAPIURLInfo, error) { +func (eil *ExperimentInputLoader) Load(ctx context.Context) ([]model.ExperimentTarget, error) { return eil.MockLoad(ctx) } diff --git a/internal/mocks/experimentinputloader_test.go b/internal/mocks/experimentinputloader_test.go index a22f456c47..a9d872e4bd 100644 --- a/internal/mocks/experimentinputloader_test.go +++ b/internal/mocks/experimentinputloader_test.go @@ -12,7 +12,7 @@ func TestExperimentInputLoader(t *testing.T) { t.Run("Load", func(t *testing.T) { expected := errors.New("mocked error") eil := &ExperimentInputLoader{ - MockLoad: func(ctx context.Context) ([]model.OOAPIURLInfo, error) { + MockLoad: func(ctx context.Context) ([]model.ExperimentTarget, error) { return nil, expected }, } diff --git a/internal/model/experiment.go b/internal/model/experiment.go index e83caa0bce..bac0ce0a43 100644 --- a/internal/model/experiment.go +++ b/internal/model/experiment.go @@ -8,6 +8,7 @@ package model import ( "context" "errors" + "fmt" ) // ErrNoAvailableTestHelpers is emitted when there are no available test helpers. @@ -80,6 +81,37 @@ func (d PrinterCallbacks) OnProgress(percentage float64, message string) { d.Logger.Infof("[%5.1f%%] %s", percentage*100, message) } +// ExperimentTarget contains a target for the experiment to measure. +type ExperimentTarget interface { + // Category returns the github.com/citizenlab/test-lists category + // code for this piece of richer input. + // + // Return [DefaultCategoryCode] if there's no applicable category code. + Category() string + + // Country returns the country code for this + // piece of richer input. + // + // Return [DefaultCountryCode] if there's not applicable country code. + Country() string + + // Input returns the experiment input, which is typically a URL. + Input() string + + // String MUST return the experiment input. + // + // Implementation note: previously existing code often times treated + // the input as a string and, crucially, printed it using %s. To be + // robust with respect to introducing richer input, we would like the + // code to print in output the same value as before, which possibly + // is processed by the desktop app. This is the reason why we are + // introducing an explicit String() method and why we say that this + // method MUST return the experiment input. + String() string +} + +var _ fmt.Stringer = ExperimentTarget(nil) + // ExperimentArgs contains the arguments passed to an experiment. type ExperimentArgs struct { // Callbacks contains MANDATORY experiment callbacks. @@ -127,11 +159,11 @@ type Experiment interface { // successfully before, or an empty string, otherwise. ReportID() string - // MeasureWithContext performs a synchronous measurement. + // MeasureWithContext measures the given experiment target. // - // Return value: strictly either a non-nil measurement and - // a nil error or a nil measurement and a non-nil error. - MeasureWithContext(ctx context.Context, input string) (measurement *Measurement, err error) + // Return value: either a non-nil measurement and a nil error + // or a nil measurement and a non-nil error. + MeasureWithContext(ctx context.Context, target ExperimentTarget) (measurement *Measurement, err error) // SubmitAndUpdateMeasurementContext submits a measurement and updates the // fields whose value has changed as part of the submission. @@ -215,7 +247,7 @@ type ExperimentOptionInfo struct { // ExperimentInputLoader loads inputs from local or remote sources. type ExperimentInputLoader interface { - Load(ctx context.Context) ([]OOAPIURLInfo, error) + Load(ctx context.Context) ([]ExperimentTarget, error) } // Submitter submits a measurement to the OONI collector. diff --git a/internal/model/ooapi.go b/internal/model/ooapi.go index 0442e8369a..3750cece17 100644 --- a/internal/model/ooapi.go +++ b/internal/model/ooapi.go @@ -182,6 +182,53 @@ type OOAPIURLInfo struct { URL string `json:"url"` } +const ( + // DefaultCategoryCode is the default category code to use + // when a URL's category code is unknown. + DefaultCategoryCode = "MISC" + + // DefaultCountryCode is the default country code to use + // when a URL's country code is unknown. + // + // We use XX because it is the same string that the URL + // prioritization code would return. + // + // See https://github.com/ooni/backend/blob/f7a93f477111c7278424996815b91e6300d66b83/api/ooniapi/prio.py#L182 + DefaultCountryCode = "XX" +) + +// NewOOAPIURLInfoWithDefaultCategoryAndCountry constructs a new instance +// of [*OOAPIURLInfo] with default category and country code. +func NewOOAPIURLInfoWithDefaultCategoryAndCountry(URL string) *OOAPIURLInfo { + return &OOAPIURLInfo{ + CategoryCode: DefaultCategoryCode, + CountryCode: DefaultCountryCode, + URL: URL, + } +} + +var _ ExperimentTarget = &OOAPIURLInfo{} + +// Category implements [ExperimentTarget]. +func (o *OOAPIURLInfo) Category() string { + return o.CategoryCode +} + +// Country implements [ExperimentTarget]. +func (o *OOAPIURLInfo) Country() string { + return o.CountryCode +} + +// Input implements [ExperimentTarget]. +func (o *OOAPIURLInfo) Input() string { + return o.URL +} + +// String implements [ExperimentTarget]. +func (o *OOAPIURLInfo) String() string { + return o.URL +} + const ( // OOAPIReportDefaultDataFormatVersion is the default data format version. // diff --git a/internal/model/ooapi_test.go b/internal/model/ooapi_test.go index 65cf1a623b..aee509e859 100644 --- a/internal/model/ooapi_test.go +++ b/internal/model/ooapi_test.go @@ -108,3 +108,27 @@ func TestOOAPIProbeMetadataValid(t *testing.T) { } }) } + +func TestOOAPIURLInfo(t *testing.T) { + info := &OOAPIURLInfo{ + CategoryCode: "SOCIAL", + CountryCode: "IT", + URL: "https://www.facebook.com/", + } + + if info.Category() != "SOCIAL" { + t.Fatal("invalid Category") + } + + if info.Country() != "IT" { + t.Fatal("invalid Country") + } + + if info.Input() != "https://www.facebook.com/" { + t.Fatal("invalid Input") + } + + if info.String() != "https://www.facebook.com/" { + t.Fatal("invalid String") + } +} diff --git a/internal/oonirun/experiment.go b/internal/oonirun/experiment.go index 611dd23fbe..7558d71fc8 100644 --- a/internal/oonirun/experiment.go +++ b/internal/oonirun/experiment.go @@ -70,7 +70,7 @@ type Experiment struct { newSaverFn func() (model.Saver, error) // newInputProcessorFn is OPTIONAL and used for testing. - newInputProcessorFn func(experiment model.Experiment, inputList []model.OOAPIURLInfo, + newInputProcessorFn func(experiment model.Experiment, inputList []model.ExperimentTarget, saver model.Saver, submitter model.Submitter) inputProcessor } @@ -138,7 +138,7 @@ type inputProcessor = model.ExperimentInputProcessor // newInputProcessor creates a new inputProcessor instance. func (ed *Experiment) newInputProcessor(experiment model.Experiment, - inputList []model.OOAPIURLInfo, saver model.Saver, submitter model.Submitter) inputProcessor { + inputList []model.ExperimentTarget, saver model.Saver, submitter model.Submitter) inputProcessor { if ed.newInputProcessorFn != nil { return ed.newInputProcessorFn(experiment, inputList, saver, submitter) } @@ -243,11 +243,11 @@ type experimentWrapper struct { } func (ew *experimentWrapper) MeasureWithContext( - ctx context.Context, input string, idx int) (*model.Measurement, error) { - if input != "" { - ew.logger.Infof("[%d/%d] running with input: %s", idx+1, ew.total, input) + ctx context.Context, target model.ExperimentTarget, idx int) (*model.Measurement, error) { + if target.Input() != "" { + ew.logger.Infof("[%d/%d] running with input: %s", idx+1, ew.total, target) } - return ew.child.MeasureWithContext(ctx, input, idx) + return ew.child.MeasureWithContext(ctx, target, idx) } // experimentSubmitterWrapper implements a submission policy where we don't diff --git a/internal/oonirun/experiment_test.go b/internal/oonirun/experiment_test.go index cd84e89b23..16d6d16bb4 100644 --- a/internal/oonirun/experiment_test.go +++ b/internal/oonirun/experiment_test.go @@ -49,7 +49,8 @@ func TestExperimentRunWithFailureToSubmitAndShuffle(t *testing.T) { }, MockNewExperiment: func() model.Experiment { exp := &mocks.Experiment{ - MockMeasureWithContext: func(ctx context.Context, input string) (*model.Measurement, error) { + MockMeasureWithContext: func( + ctx context.Context, target model.ExperimentTarget) (*model.Measurement, error) { ff := &testingx.FakeFiller{} var meas model.Measurement ff.Fill(&meas) @@ -167,7 +168,8 @@ func TestExperimentRun(t *testing.T) { newInputLoaderFn func(inputPolicy model.InputPolicy) inputLoader newSubmitterFn func(ctx context.Context) (model.Submitter, error) newSaverFn func() (model.Saver, error) - newInputProcessorFn func(experiment model.Experiment, inputList []model.OOAPIURLInfo, saver model.Saver, submitter model.Submitter) inputProcessor + newInputProcessorFn func(experiment model.Experiment, + inputList []model.ExperimentTarget, saver model.Saver, submitter model.Submitter) inputProcessor } type args struct { ctx context.Context @@ -199,7 +201,7 @@ func TestExperimentRun(t *testing.T) { }, newInputLoaderFn: func(inputPolicy model.InputPolicy) inputLoader { return &mocks.ExperimentInputLoader{ - MockLoad: func(ctx context.Context) ([]model.OOAPIURLInfo, error) { + MockLoad: func(ctx context.Context) ([]model.ExperimentTarget, error) { return nil, errMocked }, } @@ -223,8 +225,8 @@ func TestExperimentRun(t *testing.T) { }, newInputLoaderFn: func(inputPolicy model.InputPolicy) inputLoader { return &mocks.ExperimentInputLoader{ - MockLoad: func(ctx context.Context) ([]model.OOAPIURLInfo, error) { - return []model.OOAPIURLInfo{}, nil + MockLoad: func(ctx context.Context) ([]model.ExperimentTarget, error) { + return []model.ExperimentTarget{}, nil }, } }, @@ -263,8 +265,8 @@ func TestExperimentRun(t *testing.T) { }, newInputLoaderFn: func(inputPolicy model.InputPolicy) inputLoader { return &mocks.ExperimentInputLoader{ - MockLoad: func(ctx context.Context) ([]model.OOAPIURLInfo, error) { - return []model.OOAPIURLInfo{}, nil + MockLoad: func(ctx context.Context) ([]model.ExperimentTarget, error) { + return []model.ExperimentTarget{}, nil }, } }, @@ -306,8 +308,8 @@ func TestExperimentRun(t *testing.T) { }, newInputLoaderFn: func(inputPolicy model.InputPolicy) inputLoader { return &mocks.ExperimentInputLoader{ - MockLoad: func(ctx context.Context) ([]model.OOAPIURLInfo, error) { - return []model.OOAPIURLInfo{}, nil + MockLoad: func(ctx context.Context) ([]model.ExperimentTarget, error) { + return []model.ExperimentTarget{}, nil }, } }, @@ -352,8 +354,8 @@ func TestExperimentRun(t *testing.T) { }, newInputLoaderFn: func(inputPolicy model.InputPolicy) inputLoader { return &mocks.ExperimentInputLoader{ - MockLoad: func(ctx context.Context) ([]model.OOAPIURLInfo, error) { - return []model.OOAPIURLInfo{}, nil + MockLoad: func(ctx context.Context) ([]model.ExperimentTarget, error) { + return []model.ExperimentTarget{}, nil }, } }, @@ -363,7 +365,7 @@ func TestExperimentRun(t *testing.T) { newSaverFn: func() (model.Saver, error) { return &mocks.Saver{}, nil }, - newInputProcessorFn: func(experiment model.Experiment, inputList []model.OOAPIURLInfo, + newInputProcessorFn: func(experiment model.Experiment, inputList []model.ExperimentTarget, saver model.Saver, submitter model.Submitter) inputProcessor { return &mocks.ExperimentInputProcessor{ MockRun: func(ctx context.Context) error { diff --git a/internal/oonirun/inputprocessor.go b/internal/oonirun/inputprocessor.go index 08e0574b18..d4e292fd63 100644 --- a/internal/oonirun/inputprocessor.go +++ b/internal/oonirun/inputprocessor.go @@ -10,13 +10,13 @@ import ( // InputProcessorExperiment is the Experiment // according to InputProcessor. type InputProcessorExperiment interface { - MeasureWithContext(ctx context.Context, input string) (*model.Measurement, error) + MeasureWithContext(ctx context.Context, target model.ExperimentTarget) (*model.Measurement, error) } // InputProcessorExperimentWrapper is a wrapper for an // Experiment that also allow to pass around the input index. type InputProcessorExperimentWrapper interface { - MeasureWithContext(ctx context.Context, input string, idx int) (*model.Measurement, error) + MeasureWithContext(ctx context.Context, target model.ExperimentTarget, idx int) (*model.Measurement, error) } // NewInputProcessorExperimentWrapper creates a new @@ -31,8 +31,8 @@ type inputProcessorExperimentWrapper struct { } func (ipew inputProcessorExperimentWrapper) MeasureWithContext( - ctx context.Context, input string, idx int) (*model.Measurement, error) { - return ipew.exp.MeasureWithContext(ctx, input) + ctx context.Context, target model.ExperimentTarget, idx int) (*model.Measurement, error) { + return ipew.exp.MeasureWithContext(ctx, target) } var _ InputProcessorExperimentWrapper = inputProcessorExperimentWrapper{} @@ -47,7 +47,7 @@ type InputProcessor struct { Experiment InputProcessorExperimentWrapper // Inputs is the list of inputs to measure. - Inputs []model.OOAPIURLInfo + Inputs []model.ExperimentTarget // MaxRuntime is the optional maximum runtime // when looping over a list of inputs (e.g. when @@ -135,12 +135,11 @@ const ( // also returns the reason why we stopped. func (ip *InputProcessor) run(ctx context.Context) (int, error) { start := time.Now() - for idx, url := range ip.Inputs { + for idx, target := range ip.Inputs { if ip.MaxRuntime > 0 && time.Since(start) > ip.MaxRuntime { return stopMaxRuntime, nil } - input := url.URL - meas, err := ip.Experiment.MeasureWithContext(ctx, input, idx) + meas, err := ip.Experiment.MeasureWithContext(ctx, target, idx) if err != nil { return 0, err } diff --git a/internal/oonirun/inputprocessor_test.go b/internal/oonirun/inputprocessor_test.go index 83d32e9f09..a427b5126f 100644 --- a/internal/oonirun/inputprocessor_test.go +++ b/internal/oonirun/inputprocessor_test.go @@ -16,7 +16,7 @@ type FakeInputProcessorExperiment struct { } func (fipe *FakeInputProcessorExperiment) MeasureWithContext( - ctx context.Context, input string) (*model.Measurement, error) { + ctx context.Context, target model.ExperimentTarget) (*model.Measurement, error) { if fipe.Err != nil { return nil, fipe.Err } @@ -28,7 +28,7 @@ func (fipe *FakeInputProcessorExperiment) MeasureWithContext( // is MERGING annotations as opposed to overwriting them. m.AddAnnotation("antani", "antani") m.AddAnnotation("foo", "baz") // would be bar below - m.Input = model.MeasurementInput(input) + m.Input = model.MeasurementInput(target.Input()) fipe.M = append(fipe.M, m) return m, nil } @@ -39,9 +39,9 @@ func TestInputProcessorMeasurementFailed(t *testing.T) { Experiment: NewInputProcessorExperimentWrapper( &FakeInputProcessorExperiment{Err: expected}, ), - Inputs: []model.OOAPIURLInfo{{ - URL: "https://www.kernel.org/", - }}, + Inputs: []model.ExperimentTarget{ + model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("https://www.kernel.org/"), + }, } ctx := context.Background() if err := ip.Run(ctx); !errors.Is(err, expected) { @@ -68,9 +68,9 @@ func TestInputProcessorSubmissionFailed(t *testing.T) { "foo": "bar", }, Experiment: NewInputProcessorExperimentWrapper(fipe), - Inputs: []model.OOAPIURLInfo{{ - URL: "https://www.kernel.org/", - }}, + Inputs: []model.ExperimentTarget{ + model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("https://www.kernel.org/"), + }, Options: []string{"fake=true"}, Submitter: NewInputProcessorSubmitterWrapper( &FakeInputProcessorSubmitter{Err: expected}, @@ -117,9 +117,9 @@ func TestInputProcessorSaveOnDiskFailed(t *testing.T) { Experiment: NewInputProcessorExperimentWrapper( &FakeInputProcessorExperiment{}, ), - Inputs: []model.OOAPIURLInfo{{ - URL: "https://www.kernel.org/", - }}, + Inputs: []model.ExperimentTarget{ + model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("https://www.kernel.org/"), + }, Options: []string{"fake=true"}, Saver: NewInputProcessorSaverWrapper( &FakeInputProcessorSaver{Err: expected}, @@ -140,11 +140,10 @@ func TestInputProcessorGood(t *testing.T) { submitter := &FakeInputProcessorSubmitter{Err: nil} ip := &InputProcessor{ Experiment: NewInputProcessorExperimentWrapper(fipe), - Inputs: []model.OOAPIURLInfo{{ - URL: "https://www.kernel.org/", - }, { - URL: "https://www.slashdot.org/", - }}, + Inputs: []model.ExperimentTarget{ + model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("https://www.kernel.org/"), + model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("https://www.slashdot.org/"), + }, Options: []string{"fake=true"}, Saver: NewInputProcessorSaverWrapper(saver), Submitter: NewInputProcessorSubmitterWrapper(submitter), @@ -182,11 +181,10 @@ func TestInputProcessorMaxRuntime(t *testing.T) { submitter := &FakeInputProcessorSubmitter{Err: nil} ip := &InputProcessor{ Experiment: NewInputProcessorExperimentWrapper(fipe), - Inputs: []model.OOAPIURLInfo{{ - URL: "https://www.kernel.org/", - }, { - URL: "https://www.slashdot.org/", - }}, + Inputs: []model.ExperimentTarget{ + model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("https://www.kernel.org/"), + model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("https://www.slashdot.org/"), + }, MaxRuntime: 1 * time.Nanosecond, Options: []string{"fake=true"}, Saver: NewInputProcessorSaverWrapper(saver), diff --git a/internal/oonirun/v1_test.go b/internal/oonirun/v1_test.go index 6e6e473e25..c23455a9fc 100644 --- a/internal/oonirun/v1_test.go +++ b/internal/oonirun/v1_test.go @@ -28,7 +28,8 @@ func newMinimalFakeSession() *mocks.Session { }, MockNewExperiment: func() model.Experiment { exp := &mocks.Experiment{ - MockMeasureWithContext: func(ctx context.Context, input string) (*model.Measurement, error) { + MockMeasureWithContext: func( + ctx context.Context, target model.ExperimentTarget) (*model.Measurement, error) { ff := &testingx.FakeFiller{} var meas model.Measurement ff.Fill(&meas) diff --git a/internal/oonirun/v2_test.go b/internal/oonirun/v2_test.go index f2eab58f5b..c4afa60e96 100644 --- a/internal/oonirun/v2_test.go +++ b/internal/oonirun/v2_test.go @@ -379,7 +379,8 @@ func TestV2MeasureDescriptor(t *testing.T) { }, MockNewExperiment: func() model.Experiment { exp := &mocks.Experiment{ - MockMeasureWithContext: func(ctx context.Context, input string) (*model.Measurement, error) { + MockMeasureWithContext: func( + ctx context.Context, target model.ExperimentTarget) (*model.Measurement, error) { return nil, expected }, MockKibiBytesReceived: func() float64 { diff --git a/pkg/oonimkall/experiment.go b/pkg/oonimkall/experiment.go index b8c58f5a20..71250c5221 100644 --- a/pkg/oonimkall/experiment.go +++ b/pkg/oonimkall/experiment.go @@ -84,7 +84,7 @@ func (eb *experimentBuilderWrapper) setCallbacks(cb ExperimentCallbacks) { type experiment interface { // MeasureWithContext runs the measurement with the given input // and context. It returns a measurement or an error. - MeasureWithContext(ctx context.Context, input string) ( + MeasureWithContext(ctx context.Context, target model.ExperimentTarget) ( measurement *model.Measurement, err error) // KibiBytesSent returns the number of KiB sent. diff --git a/pkg/oonimkall/experiment_test.go b/pkg/oonimkall/experiment_test.go index f6fa6a4f68..ecb0936306 100644 --- a/pkg/oonimkall/experiment_test.go +++ b/pkg/oonimkall/experiment_test.go @@ -81,7 +81,7 @@ type FakeExperiment struct { } // MeasureWithContext implements experiment.MeasureWithContext. -func (e *FakeExperiment) MeasureWithContext(ctx context.Context, input string) ( +func (e *FakeExperiment) MeasureWithContext(ctx context.Context, target model.ExperimentTarget) ( measurement *model.Measurement, err error) { return e.Measurement, e.Err } diff --git a/pkg/oonimkall/taskmocks_test.go b/pkg/oonimkall/taskmocks_test.go index af58a82dd9..839f519e36 100644 --- a/pkg/oonimkall/taskmocks_test.go +++ b/pkg/oonimkall/taskmocks_test.go @@ -97,7 +97,7 @@ type MockableTaskRunnerDependencies struct { MockableKibiBytesSent func() float64 MockableOpenReportContext func(ctx context.Context) error MockableReportID func() string - MockableMeasureWithContext func(ctx context.Context, input string) ( + MockableMeasureWithContext func(ctx context.Context, target model.ExperimentTarget) ( measurement *model.Measurement, err error) MockableSubmitAndUpdateMeasurementContext func( ctx context.Context, measurement *model.Measurement) error @@ -200,9 +200,9 @@ func (dep *MockableTaskRunnerDependencies) ReportID() string { return dep.MockableReportID() } -func (dep *MockableTaskRunnerDependencies) MeasureWithContext(ctx context.Context, input string) ( - measurement *model.Measurement, err error) { - return dep.MockableMeasureWithContext(ctx, input) +func (dep *MockableTaskRunnerDependencies) MeasureWithContext( + ctx context.Context, target model.ExperimentTarget) (measurement *model.Measurement, err error) { + return dep.MockableMeasureWithContext(ctx, target) } func (dep *MockableTaskRunnerDependencies) SubmitAndUpdateMeasurementContext( diff --git a/pkg/oonimkall/taskmodel.go b/pkg/oonimkall/taskmodel.go index cd60ee9b4f..86f534e01b 100644 --- a/pkg/oonimkall/taskmodel.go +++ b/pkg/oonimkall/taskmodel.go @@ -252,7 +252,7 @@ type taskExperiment interface { ReportID() string // MeasureWithContext runs the measurement. - MeasureWithContext(ctx context.Context, input string) ( + MeasureWithContext(ctx context.Context, target model.ExperimentTarget) ( measurement *model.Measurement, err error) // SubmitAndUpdateMeasurementContext submits the measurement diff --git a/pkg/oonimkall/taskrunner.go b/pkg/oonimkall/taskrunner.go index 161d23ad87..5702ed6499 100644 --- a/pkg/oonimkall/taskrunner.go +++ b/pkg/oonimkall/taskrunner.go @@ -282,10 +282,20 @@ func (r *runnerForTask) Run(rootCtx context.Context) { "processing %s", input, )) } + + // Richer input implementation note: in mobile, we only observe richer input + // for Web Connectivity and only store this kind of input into the database and + // otherwise we ignore richer input for other experiments, which are just + // treated as experimental. As such, the thinking here is that we do not care + // about *passing* richer input from desktop to mobile for some time. When + // we will care, it would most likely suffice to require the Inputs field to + // implement in Java the [model.ExperimentTarget] interface, which is something + // we can always do, since it only has string accessors. m, err := experiment.MeasureWithContext( r.contextForExperiment(measCtx, builder), - input, + model.NewOOAPIURLInfoWithDefaultCategoryAndCountry(input), ) + if builder.Interruptible() && measCtx.Err() != nil { // We want to stop here only if interruptible otherwise we want to // submit measurement and stop at beginning of next iteration diff --git a/pkg/oonimkall/taskrunner_test.go b/pkg/oonimkall/taskrunner_test.go index e9f8f4920d..1a3e732062 100644 --- a/pkg/oonimkall/taskrunner_test.go +++ b/pkg/oonimkall/taskrunner_test.go @@ -196,7 +196,7 @@ func TestTaskRunnerRun(t *testing.T) { MockableReportID: func() string { return "20211202T074907Z_example_IT_30722_n1_axDLHNUfJaV1IbuU" }, - MockableMeasureWithContext: func(ctx context.Context, input string) (*model.Measurement, error) { + MockableMeasureWithContext: func(ctx context.Context, target model.ExperimentTarget) (*model.Measurement, error) { return &model.Measurement{}, nil }, MockableSubmitAndUpdateMeasurementContext: func(ctx context.Context, measurement *model.Measurement) error { @@ -447,7 +447,7 @@ func TestTaskRunnerRun(t *testing.T) { fake.MockableInputPolicy = func() model.InputPolicy { return model.InputNone } - fake.MockableMeasureWithContext = func(ctx context.Context, input string) (measurement *model.Measurement, err error) { + fake.MockableMeasureWithContext = func(ctx context.Context, target model.ExperimentTarget) (measurement *model.Measurement, err error) { return nil, errors.New("preconditions error") } runner.sessionBuilder = fake @@ -477,7 +477,7 @@ func TestTaskRunnerRun(t *testing.T) { fake.MockableInputPolicy = func() model.InputPolicy { return model.InputNone } - fake.MockableMeasureWithContext = func(ctx context.Context, input string) (measurement *model.Measurement, err error) { + fake.MockableMeasureWithContext = func(ctx context.Context, target model.ExperimentTarget) (measurement *model.Measurement, err error) { return nil, errors.New("preconditions error") } runner.sessionBuilder = fake @@ -669,7 +669,7 @@ func TestTaskRunnerRun(t *testing.T) { fake.MockableInputPolicy = func() model.InputPolicy { return model.InputStrictlyRequired } - fake.MockableMeasureWithContext = func(ctx context.Context, input string) (measurement *model.Measurement, err error) { + fake.MockableMeasureWithContext = func(ctx context.Context, target model.ExperimentTarget) (measurement *model.Measurement, err error) { time.Sleep(1 * time.Second) return &model.Measurement{}, nil } @@ -714,7 +714,7 @@ func TestTaskRunnerRun(t *testing.T) { return true } ctx, cancel := context.WithCancel(context.Background()) - fake.MockableMeasureWithContext = func(ctx context.Context, input string) (measurement *model.Measurement, err error) { + fake.MockableMeasureWithContext = func(ctx context.Context, target model.ExperimentTarget) (measurement *model.Measurement, err error) { cancel() return &model.Measurement{}, nil } @@ -778,7 +778,7 @@ func TestTaskRunnerRun(t *testing.T) { fake.MockableSetCallbacks = func(cbs model.ExperimentCallbacks) { callbacks = cbs } - fake.MockableMeasureWithContext = func(ctx context.Context, input string) (measurement *model.Measurement, err error) { + fake.MockableMeasureWithContext = func(ctx context.Context, target model.ExperimentTarget) (measurement *model.Measurement, err error) { callbacks.OnProgress(1, "hello from the fake experiment") return &model.Measurement{}, nil } diff --git a/pkg/oonimkall/webconnectivity.go b/pkg/oonimkall/webconnectivity.go index e96df38d4c..b1c44691d5 100644 --- a/pkg/oonimkall/webconnectivity.go +++ b/pkg/oonimkall/webconnectivity.go @@ -4,10 +4,13 @@ import ( "context" "encoding/json" + "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/runtimex" ) // WebConnectivityConfig contains settings for WebConnectivity. +// +// Deprecated: this code was just an experiment that we never ended up using. type WebConnectivityConfig struct { // Callbacks contains the experiment callbacks. This field is // optional. Leave it empty and we'll use a default set of @@ -20,6 +23,8 @@ type WebConnectivityConfig struct { } // WebConnectivityResults contains the results of WebConnectivity. +// +// Deprecated: this code was just an experiment that we never ended up using. type WebConnectivityResults struct { // KibiBytesReceived contains the KiB received. KibiBytesReceived float64 @@ -65,7 +70,8 @@ func (r *webConnectivityRunner) run(ctx context.Context, config *WebConnectivity builder.setCallbacks(config.Callbacks) } exp := builder.newExperiment() - measurement, err := exp.MeasureWithContext(ctx, config.Input) + target := model.NewOOAPIURLInfoWithDefaultCategoryAndCountry(config.Input) + measurement, err := exp.MeasureWithContext(ctx, target) if err != nil { return nil, err } @@ -86,6 +92,8 @@ func (r *webConnectivityRunner) run(ctx context.Context, config *WebConnectivity // // This API is currently experimental. We do not promise that we will bump // the major version number when changing it. +// +// Deprecated: this code was just an experiment that we never ended up using. func (sess *Session) WebConnectivity(ctx *Context, config *WebConnectivityConfig) (*WebConnectivityResults, error) { return (&webConnectivityRunner{sess: sess}).run(ctx.ctx, config) }