From 4ea29ff336e489eda529744a9f1eeabef56c45ec Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Thu, 6 Jun 2024 13:51:37 +0200 Subject: [PATCH] refactor: make TargetLoader using ExperimentBuilder This diff completes the set of preliminary richer input diffs. We build the TargetLoader using the ExperimentBuilder, which in turn uses a registry.Factory under the hood. This means that we can load targets for each experiment. Part of https://github.com/ooni/probe/issues/2607 --- cmd/ooniprobe/internal/nettests/dnscheck.go | 16 ++++---- .../internal/nettests/stunreachability.go | 16 ++++---- .../internal/nettests/web_connectivity.go | 21 +++++----- internal/engine/experimentbuilder.go | 7 ++++ internal/mocks/experimentbuilder.go | 8 ++++ internal/model/experiment.go | 40 ++++++++++++++++++- internal/oonirun/experiment.go | 21 +++++----- internal/oonirun/experiment_test.go | 12 +++--- internal/registry/dash.go | 4 +- internal/registry/dnscheck.go | 4 +- internal/registry/dnsping.go | 4 +- internal/registry/dslxtutorial.go | 8 ++-- internal/registry/echcheck.go | 8 ++-- internal/registry/example.go | 4 +- internal/registry/factory.go | 20 ++++++++++ internal/registry/fbmessenger.go | 4 +- internal/registry/hhfm.go | 4 +- internal/registry/hirl.go | 4 +- internal/registry/httphostheader.go | 4 +- internal/registry/ndt.go | 4 +- internal/registry/openvpn.go | 4 +- internal/registry/portfiltering.go | 4 +- internal/registry/psiphon.go | 4 +- internal/registry/quicping.go | 4 +- internal/registry/riseupvpn.go | 8 ++-- internal/registry/signal.go | 4 +- internal/registry/simplequicping.go | 4 +- internal/registry/sniblocking.go | 4 +- internal/registry/stunreachability.go | 4 +- internal/registry/tcpping.go | 4 +- internal/registry/telegram.go | 4 +- internal/registry/tlsmiddlebox.go | 4 +- internal/registry/tlsping.go | 4 +- internal/registry/tlstool.go | 4 +- internal/registry/tor.go | 4 +- internal/registry/torsf.go | 4 +- internal/registry/urlgetter.go | 4 +- internal/registry/vanillator.go | 6 ++- internal/registry/webconnectivity.go | 4 +- internal/registry/webconnectivityv05.go | 4 +- internal/registry/whatsapp.go | 4 +- internal/targetloading/targetloading.go | 13 ++---- internal/targetloading/targetloading_test.go | 11 ++++- 43 files changed, 229 insertions(+), 98 deletions(-) diff --git a/cmd/ooniprobe/internal/nettests/dnscheck.go b/cmd/ooniprobe/internal/nettests/dnscheck.go index cb53560155..2c8e798622 100644 --- a/cmd/ooniprobe/internal/nettests/dnscheck.go +++ b/cmd/ooniprobe/internal/nettests/dnscheck.go @@ -4,23 +4,21 @@ import ( "context" "github.com/ooni/probe-cli/v3/internal/model" - "github.com/ooni/probe-cli/v3/internal/targetloading" ) // DNSCheck nettest implementation. type DNSCheck struct{} -func (n DNSCheck) lookupURLs(ctl *Controller) ([]model.ExperimentTarget, error) { - targetloader := &targetloading.Loader{ +func (n DNSCheck) lookupURLs(ctl *Controller, builder model.ExperimentBuilder) ([]model.ExperimentTarget, error) { + config := &model.ExperimentTargetLoaderConfig{ CheckInConfig: &model.OOAPICheckInConfig{ // not needed because we have default static input in the engine }, - ExperimentName: "dnscheck", - InputPolicy: model.InputOrStaticDefault, - Session: ctl.Session, - SourceFiles: ctl.InputFiles, - StaticInputs: ctl.Inputs, + Session: ctl.Session, + SourceFiles: ctl.InputFiles, + StaticInputs: ctl.Inputs, } + targetloader := builder.NewTargetLoader(config) testlist, err := targetloader.Load(context.Background()) if err != nil { return nil, err @@ -34,7 +32,7 @@ func (n DNSCheck) Run(ctl *Controller) error { if err != nil { return err } - urls, err := n.lookupURLs(ctl) + urls, err := n.lookupURLs(ctl, builder) if err != nil { return err } diff --git a/cmd/ooniprobe/internal/nettests/stunreachability.go b/cmd/ooniprobe/internal/nettests/stunreachability.go index 743d53e8f2..c6313d96c8 100644 --- a/cmd/ooniprobe/internal/nettests/stunreachability.go +++ b/cmd/ooniprobe/internal/nettests/stunreachability.go @@ -4,23 +4,21 @@ import ( "context" "github.com/ooni/probe-cli/v3/internal/model" - "github.com/ooni/probe-cli/v3/internal/targetloading" ) // STUNReachability nettest implementation. type STUNReachability struct{} -func (n STUNReachability) lookupURLs(ctl *Controller) ([]model.ExperimentTarget, error) { - targetloader := &targetloading.Loader{ +func (n STUNReachability) lookupURLs(ctl *Controller, builder model.ExperimentBuilder) ([]model.ExperimentTarget, error) { + config := &model.ExperimentTargetLoaderConfig{ CheckInConfig: &model.OOAPICheckInConfig{ // not needed because we have default static input in the engine }, - ExperimentName: "stunreachability", - InputPolicy: model.InputOrStaticDefault, - Session: ctl.Session, - SourceFiles: ctl.InputFiles, - StaticInputs: ctl.Inputs, + Session: ctl.Session, + SourceFiles: ctl.InputFiles, + StaticInputs: ctl.Inputs, } + targetloader := builder.NewTargetLoader(config) testlist, err := targetloader.Load(context.Background()) if err != nil { return nil, err @@ -34,7 +32,7 @@ func (n STUNReachability) Run(ctl *Controller) error { if err != nil { return err } - urls, err := n.lookupURLs(ctl) + urls, err := n.lookupURLs(ctl, builder) if err != nil { return err } diff --git a/cmd/ooniprobe/internal/nettests/web_connectivity.go b/cmd/ooniprobe/internal/nettests/web_connectivity.go index 7e4bcad996..0da3be56d1 100644 --- a/cmd/ooniprobe/internal/nettests/web_connectivity.go +++ b/cmd/ooniprobe/internal/nettests/web_connectivity.go @@ -5,11 +5,11 @@ import ( "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/model" - "github.com/ooni/probe-cli/v3/internal/targetloading" ) -func (n WebConnectivity) lookupURLs(ctl *Controller, categories []string) ([]model.ExperimentTarget, error) { - targetloader := &targetloading.Loader{ +func (n WebConnectivity) lookupURLs( + ctl *Controller, builder model.ExperimentBuilder, categories []string) ([]model.ExperimentTarget, error) { + config := &model.ExperimentTargetLoaderConfig{ CheckInConfig: &model.OOAPICheckInConfig{ // Setting Charging and OnWiFi to true causes the CheckIn // API to return to us as much URL as possible with the @@ -21,12 +21,11 @@ func (n WebConnectivity) lookupURLs(ctl *Controller, categories []string) ([]mod CategoryCodes: categories, }, }, - ExperimentName: "web_connectivity", - InputPolicy: model.InputOrQueryBackend, - Session: ctl.Session, - SourceFiles: ctl.InputFiles, - StaticInputs: ctl.Inputs, + Session: ctl.Session, + SourceFiles: ctl.InputFiles, + StaticInputs: ctl.Inputs, } + targetloader := builder.NewTargetLoader(config) testlist, err := targetloader.Load(context.Background()) if err != nil { return nil, err @@ -39,12 +38,12 @@ type WebConnectivity struct{} // Run starts the test func (n WebConnectivity) Run(ctl *Controller) error { - log.Debugf("Enabled category codes are the following %v", ctl.Probe.Config().Nettests.WebsitesEnabledCategoryCodes) - urls, err := n.lookupURLs(ctl, ctl.Probe.Config().Nettests.WebsitesEnabledCategoryCodes) + builder, err := ctl.Session.NewExperimentBuilder("web_connectivity") if err != nil { return err } - builder, err := ctl.Session.NewExperimentBuilder("web_connectivity") + log.Debugf("Enabled category codes are the following %v", ctl.Probe.Config().Nettests.WebsitesEnabledCategoryCodes) + urls, err := n.lookupURLs(ctl, builder, ctl.Probe.Config().Nettests.WebsitesEnabledCategoryCodes) if err != nil { return err } diff --git a/internal/engine/experimentbuilder.go b/internal/engine/experimentbuilder.go index a60d0a50d5..330777957b 100644 --- a/internal/engine/experimentbuilder.go +++ b/internal/engine/experimentbuilder.go @@ -22,6 +22,8 @@ type experimentBuilder struct { session *Session } +var _ model.ExperimentBuilder = &experimentBuilder{} + // Interruptible implements ExperimentBuilder.Interruptible. func (b *experimentBuilder) Interruptible() bool { return b.factory.Interruptible() @@ -60,6 +62,11 @@ func (b *experimentBuilder) NewExperiment() model.Experiment { return experiment } +// NewTargetLoader creates a new [model.ExperimentTargetLoader] instance. +func (b *experimentBuilder) NewTargetLoader(config *model.ExperimentTargetLoaderConfig) model.ExperimentTargetLoader { + return b.factory.NewTargetLoader(config) +} + // newExperimentBuilder creates a new experimentBuilder instance. func newExperimentBuilder(session *Session, name string) (*experimentBuilder, error) { factory, err := registry.NewFactory(name, session.kvStore, session.logger) diff --git a/internal/mocks/experimentbuilder.go b/internal/mocks/experimentbuilder.go index 16763d471d..1f6a27187f 100644 --- a/internal/mocks/experimentbuilder.go +++ b/internal/mocks/experimentbuilder.go @@ -17,8 +17,12 @@ type ExperimentBuilder struct { MockSetCallbacks func(callbacks model.ExperimentCallbacks) MockNewExperiment func() model.Experiment + + MockNewTargetLoader func(config *model.ExperimentTargetLoaderConfig) model.ExperimentTargetLoader } +var _ model.ExperimentBuilder = &ExperimentBuilder{} + func (eb *ExperimentBuilder) Interruptible() bool { return eb.MockInterruptible() } @@ -46,3 +50,7 @@ func (eb *ExperimentBuilder) SetCallbacks(callbacks model.ExperimentCallbacks) { func (eb *ExperimentBuilder) NewExperiment() model.Experiment { return eb.MockNewExperiment() } + +func (eb *ExperimentBuilder) NewTargetLoader(config *model.ExperimentTargetLoaderConfig) model.ExperimentTargetLoader { + return eb.MockNewTargetLoader(config) +} diff --git a/internal/model/experiment.go b/internal/model/experiment.go index 9ae3dbcce6..8bd309f326 100644 --- a/internal/model/experiment.go +++ b/internal/model/experiment.go @@ -232,8 +232,46 @@ type ExperimentBuilder interface { // SetCallbacks sets the experiment's interactive callbacks. SetCallbacks(callbacks ExperimentCallbacks) - // NewExperiment creates the experiment instance. + // NewExperiment creates the [Experiment] instance. NewExperiment() Experiment + + // NewTargetLoader creates the [ExperimentTargetLoader] instance. + NewTargetLoader(config *ExperimentTargetLoaderConfig) ExperimentTargetLoader +} + +// ExperimentTargetLoaderConfig is the configuration to create a new [ExperimentTargetLoader]. +// +// The zero value is not ready to use; please, init the MANDATORY fields. +type ExperimentTargetLoaderConfig struct { + // CheckInConfig contains OPTIONAL options for the CheckIn API. If not set, then we'll create a + // default config. If set but there are fields inside it that are not set, then we will set them + // to a default value. + CheckInConfig *OOAPICheckInConfig + + // Session is the MANDATORY current measurement session. + Session ExperimentTargetLoaderSession + + // StaticInputs contains OPTIONAL input to be added + // to the resulting input list if possible. + StaticInputs []string + + // SourceFiles contains OPTIONAL files to read input + // from. Each file should contain a single input string + // per line. We will fail if any file is unreadable + // as well as if any file is empty. + SourceFiles []string +} + +// ExperimentTargetLoaderSession is the session according to [ExperimentTargetLoader]. +type ExperimentTargetLoaderSession interface { + // CheckIn invokes the check-in API. + CheckIn(ctx context.Context, config *OOAPICheckInConfig) (*OOAPICheckInResult, error) + + // FetchOpenVPNConfig fetches the OpenVPN experiment configuration. + FetchOpenVPNConfig(ctx context.Context, provider, cc string) (*OOAPIVPNProviderConfig, error) + + // Logger returns the logger to use. + Logger() Logger } // ExperimentOptionInfo contains info about an experiment option. diff --git a/internal/oonirun/experiment.go b/internal/oonirun/experiment.go index 09deedaedc..758db9e0c5 100644 --- a/internal/oonirun/experiment.go +++ b/internal/oonirun/experiment.go @@ -14,7 +14,6 @@ import ( "github.com/ooni/probe-cli/v3/internal/humanize" "github.com/ooni/probe-cli/v3/internal/model" - "github.com/ooni/probe-cli/v3/internal/targetloading" ) // experimentShuffledInputs counts how many times we shuffled inputs @@ -61,7 +60,7 @@ type Experiment struct { newExperimentBuilderFn func(experimentName string) (model.ExperimentBuilder, error) // newTargetLoaderFn is OPTIONAL and used for testing. - newTargetLoaderFn func(inputPolicy model.InputPolicy) targetLoader + newTargetLoaderFn func(builder model.ExperimentBuilder) targetLoader // newSubmitterFn is OPTIONAL and used for testing. newSubmitterFn func(ctx context.Context) (model.Submitter, error) @@ -84,7 +83,7 @@ func (ed *Experiment) Run(ctx context.Context) error { } // 2. create input loader and load input for this experiment - targetLoader := ed.newTargetLoader(builder.InputPolicy()) + targetLoader := ed.newTargetLoader(builder) targetList, err := targetLoader.Load(ctx) if err != nil { return err @@ -196,22 +195,20 @@ func (ed *Experiment) newExperimentBuilder(experimentName string) (model.Experim type targetLoader = model.ExperimentTargetLoader // newTargetLoader creates a new [model.ExperimentTargetLoader]. -func (ed *Experiment) newTargetLoader(inputPolicy model.InputPolicy) targetLoader { +func (ed *Experiment) newTargetLoader(builder model.ExperimentBuilder) targetLoader { if ed.newTargetLoaderFn != nil { - return ed.newTargetLoaderFn(inputPolicy) + return ed.newTargetLoaderFn(builder) } - return &targetloading.Loader{ + return builder.NewTargetLoader(&model.ExperimentTargetLoaderConfig{ CheckInConfig: &model.OOAPICheckInConfig{ RunType: model.RunTypeManual, OnWiFi: true, // meaning: not on 4G Charging: true, }, - ExperimentName: ed.Name, - InputPolicy: inputPolicy, - StaticInputs: ed.Inputs, - SourceFiles: ed.InputFilePaths, - Session: ed.Session, - } + StaticInputs: ed.Inputs, + SourceFiles: ed.InputFilePaths, + Session: ed.Session, + }) } // experimentOptionsToStringList convers the options to []string, which is diff --git a/internal/oonirun/experiment_test.go b/internal/oonirun/experiment_test.go index 29764d9c8c..b93fd25ae2 100644 --- a/internal/oonirun/experiment_test.go +++ b/internal/oonirun/experiment_test.go @@ -165,7 +165,7 @@ func TestExperimentRun(t *testing.T) { ReportFile string Session Session newExperimentBuilderFn func(experimentName string) (model.ExperimentBuilder, error) - newTargetLoaderFn func(inputPolicy model.InputPolicy) targetLoader + newTargetLoaderFn func(builder model.ExperimentBuilder) targetLoader newSubmitterFn func(ctx context.Context) (model.Submitter, error) newSaverFn func() (model.Saver, error) newInputProcessorFn func(experiment model.Experiment, @@ -199,7 +199,7 @@ func TestExperimentRun(t *testing.T) { } return eb, nil }, - newTargetLoaderFn: func(inputPolicy model.InputPolicy) targetLoader { + newTargetLoaderFn: func(builder model.ExperimentBuilder) targetLoader { return &mocks.ExperimentTargetLoader{ MockLoad: func(ctx context.Context) ([]model.ExperimentTarget, error) { return nil, errMocked @@ -223,7 +223,7 @@ func TestExperimentRun(t *testing.T) { } return eb, nil }, - newTargetLoaderFn: func(inputPolicy model.InputPolicy) targetLoader { + newTargetLoaderFn: func(builder model.ExperimentBuilder) targetLoader { return &mocks.ExperimentTargetLoader{ MockLoad: func(ctx context.Context) ([]model.ExperimentTarget, error) { return []model.ExperimentTarget{}, nil @@ -263,7 +263,7 @@ func TestExperimentRun(t *testing.T) { } return eb, nil }, - newTargetLoaderFn: func(inputPolicy model.InputPolicy) targetLoader { + newTargetLoaderFn: func(builder model.ExperimentBuilder) targetLoader { return &mocks.ExperimentTargetLoader{ MockLoad: func(ctx context.Context) ([]model.ExperimentTarget, error) { return []model.ExperimentTarget{}, nil @@ -306,7 +306,7 @@ func TestExperimentRun(t *testing.T) { } return eb, nil }, - newTargetLoaderFn: func(inputPolicy model.InputPolicy) targetLoader { + newTargetLoaderFn: func(builder model.ExperimentBuilder) targetLoader { return &mocks.ExperimentTargetLoader{ MockLoad: func(ctx context.Context) ([]model.ExperimentTarget, error) { return []model.ExperimentTarget{}, nil @@ -352,7 +352,7 @@ func TestExperimentRun(t *testing.T) { } return eb, nil }, - newTargetLoaderFn: func(inputPolicy model.InputPolicy) targetLoader { + newTargetLoaderFn: func(builder model.ExperimentBuilder) targetLoader { return &mocks.ExperimentTargetLoader{ MockLoad: func(ctx context.Context) ([]model.ExperimentTarget, error) { return []model.ExperimentTarget{}, nil diff --git a/internal/registry/dash.go b/internal/registry/dash.go index 812619361b..e8a2ad1165 100644 --- a/internal/registry/dash.go +++ b/internal/registry/dash.go @@ -10,13 +10,15 @@ import ( ) func init() { - AllExperiments["dash"] = func() *Factory { + const canonicalName = "dash" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config interface{}) model.ExperimentMeasurer { return dash.NewExperimentMeasurer( *config.(*dash.Config), ) }, + canonicalName: canonicalName, config: &dash.Config{}, enabledByDefault: true, interruptible: true, diff --git a/internal/registry/dnscheck.go b/internal/registry/dnscheck.go index b9355773fc..2890ca19a5 100644 --- a/internal/registry/dnscheck.go +++ b/internal/registry/dnscheck.go @@ -10,13 +10,15 @@ import ( ) func init() { - AllExperiments["dnscheck"] = func() *Factory { + const canonicalName = "dnscheck" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config interface{}) model.ExperimentMeasurer { return dnscheck.NewExperimentMeasurer( *config.(*dnscheck.Config), ) }, + canonicalName: canonicalName, config: &dnscheck.Config{}, enabledByDefault: true, inputPolicy: model.InputOrStaticDefault, diff --git a/internal/registry/dnsping.go b/internal/registry/dnsping.go index 92d52456ed..fb01d1ab80 100644 --- a/internal/registry/dnsping.go +++ b/internal/registry/dnsping.go @@ -10,13 +10,15 @@ import ( ) func init() { - AllExperiments["dnsping"] = func() *Factory { + const canonicalName = "dnsping" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config interface{}) model.ExperimentMeasurer { return dnsping.NewExperimentMeasurer( *config.(*dnsping.Config), ) }, + canonicalName: canonicalName, config: &dnsping.Config{}, enabledByDefault: true, inputPolicy: model.InputOrStaticDefault, diff --git a/internal/registry/dslxtutorial.go b/internal/registry/dslxtutorial.go index 41aef7d40c..199e36077c 100644 --- a/internal/registry/dslxtutorial.go +++ b/internal/registry/dslxtutorial.go @@ -10,15 +10,17 @@ import ( ) func init() { - AllExperiments["simple_sni"] = func() *Factory { + const canonicalName = "simple_sni" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config interface{}) model.ExperimentMeasurer { return chapter02.NewExperimentMeasurer( *config.(*chapter02.Config), ) }, - config: &chapter02.Config{}, - inputPolicy: model.InputOrQueryBackend, + canonicalName: canonicalName, + config: &chapter02.Config{}, + inputPolicy: model.InputOrQueryBackend, } } } diff --git a/internal/registry/echcheck.go b/internal/registry/echcheck.go index b091d58af9..939cf3d15d 100644 --- a/internal/registry/echcheck.go +++ b/internal/registry/echcheck.go @@ -10,15 +10,17 @@ import ( ) func init() { - AllExperiments["echcheck"] = func() *Factory { + const canonicalName = "echcheck" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config interface{}) model.ExperimentMeasurer { return echcheck.NewExperimentMeasurer( *config.(*echcheck.Config), ) }, - config: &echcheck.Config{}, - inputPolicy: model.InputOptional, + canonicalName: canonicalName, + config: &echcheck.Config{}, + inputPolicy: model.InputOptional, } } } diff --git a/internal/registry/example.go b/internal/registry/example.go index aa2d13ee11..94c55ca59f 100644 --- a/internal/registry/example.go +++ b/internal/registry/example.go @@ -12,13 +12,15 @@ import ( ) func init() { - AllExperiments["example"] = func() *Factory { + const canonicalName = "example" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config interface{}) model.ExperimentMeasurer { return example.NewExperimentMeasurer( *config.(*example.Config), "example", ) }, + canonicalName: canonicalName, config: &example.Config{ Message: "Good day from the example experiment!", SleepTime: int64(time.Second), diff --git a/internal/registry/factory.go b/internal/registry/factory.go index f0baa34806..aba0946d64 100644 --- a/internal/registry/factory.go +++ b/internal/registry/factory.go @@ -15,6 +15,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/checkincache" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/strcasex" + "github.com/ooni/probe-cli/v3/internal/targetloading" ) // Factory allows to construct an experiment measurer. @@ -22,6 +23,9 @@ type Factory struct { // build is the constructor that build an experiment with the given config. build func(config interface{}) model.ExperimentMeasurer + // canonicalName is the canonical name of the experiment. + canonicalName string + // config contains the experiment's config. config any @@ -218,6 +222,22 @@ func (b *Factory) NewExperimentMeasurer() model.ExperimentMeasurer { return b.build(b.config) } +// Session is the session definition according to this package. +type Session = model.ExperimentTargetLoaderSession + +// NewTargetLoader creates a new [model.ExperimentTargetLoader] instance. +func (b *Factory) NewTargetLoader(config *model.ExperimentTargetLoaderConfig) model.ExperimentTargetLoader { + return &targetloading.Loader{ + CheckInConfig: config.CheckInConfig, // OPTIONAL + ExperimentName: b.canonicalName, + InputPolicy: b.inputPolicy, + Logger: config.Session.Logger(), + Session: config.Session, + StaticInputs: config.StaticInputs, + SourceFiles: config.SourceFiles, + } +} + // CanonicalizeExperimentName allows code to provide experiment names // in a more flexible way, where we have aliases. // diff --git a/internal/registry/fbmessenger.go b/internal/registry/fbmessenger.go index d3f3233e3a..cd46519826 100644 --- a/internal/registry/fbmessenger.go +++ b/internal/registry/fbmessenger.go @@ -10,13 +10,15 @@ import ( ) func init() { - AllExperiments["facebook_messenger"] = func() *Factory { + const canonicalName = "facebook_messenger" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config interface{}) model.ExperimentMeasurer { return fbmessenger.NewExperimentMeasurer( *config.(*fbmessenger.Config), ) }, + canonicalName: canonicalName, config: &fbmessenger.Config{}, enabledByDefault: true, inputPolicy: model.InputNone, diff --git a/internal/registry/hhfm.go b/internal/registry/hhfm.go index 0820a86476..a86ce98192 100644 --- a/internal/registry/hhfm.go +++ b/internal/registry/hhfm.go @@ -10,13 +10,15 @@ import ( ) func init() { - AllExperiments["http_header_field_manipulation"] = func() *Factory { + const canonicalName = "http_header_field_manipulation" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config interface{}) model.ExperimentMeasurer { return hhfm.NewExperimentMeasurer( *config.(*hhfm.Config), ) }, + canonicalName: canonicalName, config: &hhfm.Config{}, enabledByDefault: true, inputPolicy: model.InputNone, diff --git a/internal/registry/hirl.go b/internal/registry/hirl.go index 846493bc8f..84dbea7ccd 100644 --- a/internal/registry/hirl.go +++ b/internal/registry/hirl.go @@ -10,13 +10,15 @@ import ( ) func init() { - AllExperiments["http_invalid_request_line"] = func() *Factory { + const canonicalName = "http_invalid_request_line" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config interface{}) model.ExperimentMeasurer { return hirl.NewExperimentMeasurer( *config.(*hirl.Config), ) }, + canonicalName: canonicalName, config: &hirl.Config{}, enabledByDefault: true, inputPolicy: model.InputNone, diff --git a/internal/registry/httphostheader.go b/internal/registry/httphostheader.go index c1ecffd76f..c3a02b655c 100644 --- a/internal/registry/httphostheader.go +++ b/internal/registry/httphostheader.go @@ -10,13 +10,15 @@ import ( ) func init() { - AllExperiments["http_host_header"] = func() *Factory { + const canonicalName = "http_host_header" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config interface{}) model.ExperimentMeasurer { return httphostheader.NewExperimentMeasurer( *config.(*httphostheader.Config), ) }, + canonicalName: canonicalName, config: &httphostheader.Config{}, enabledByDefault: true, inputPolicy: model.InputOrQueryBackend, diff --git a/internal/registry/ndt.go b/internal/registry/ndt.go index e6891c7c7b..71ea5562a6 100644 --- a/internal/registry/ndt.go +++ b/internal/registry/ndt.go @@ -10,13 +10,15 @@ import ( ) func init() { - AllExperiments["ndt"] = func() *Factory { + const canonicalName = "ndt" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config interface{}) model.ExperimentMeasurer { return ndt7.NewExperimentMeasurer( *config.(*ndt7.Config), ) }, + canonicalName: canonicalName, config: &ndt7.Config{}, enabledByDefault: true, interruptible: true, diff --git a/internal/registry/openvpn.go b/internal/registry/openvpn.go index cf9244786f..8bf107630c 100644 --- a/internal/registry/openvpn.go +++ b/internal/registry/openvpn.go @@ -10,13 +10,15 @@ import ( ) func init() { - AllExperiments["openvpn"] = func() *Factory { + const canonicalName = "openvpn" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config interface{}) model.ExperimentMeasurer { return openvpn.NewExperimentMeasurer( *config.(*openvpn.Config), "openvpn", ) }, + canonicalName: canonicalName, config: &openvpn.Config{}, enabledByDefault: true, interruptible: true, diff --git a/internal/registry/portfiltering.go b/internal/registry/portfiltering.go index 56937eaca1..8c6a857df8 100644 --- a/internal/registry/portfiltering.go +++ b/internal/registry/portfiltering.go @@ -10,13 +10,15 @@ import ( ) func init() { - AllExperiments["portfiltering"] = func() *Factory { + const canonicalName = "portfiltering" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config any) model.ExperimentMeasurer { return portfiltering.NewExperimentMeasurer( config.(portfiltering.Config), ) }, + canonicalName: canonicalName, config: portfiltering.Config{}, enabledByDefault: true, interruptible: false, diff --git a/internal/registry/psiphon.go b/internal/registry/psiphon.go index 48dc7888a2..517bb22053 100644 --- a/internal/registry/psiphon.go +++ b/internal/registry/psiphon.go @@ -10,13 +10,15 @@ import ( ) func init() { - AllExperiments["psiphon"] = func() *Factory { + const canonicalName = "psiphon" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config interface{}) model.ExperimentMeasurer { return psiphon.NewExperimentMeasurer( *config.(*psiphon.Config), ) }, + canonicalName: canonicalName, config: &psiphon.Config{}, enabledByDefault: true, inputPolicy: model.InputOptional, diff --git a/internal/registry/quicping.go b/internal/registry/quicping.go index 090037ad78..77a1d3ebf2 100644 --- a/internal/registry/quicping.go +++ b/internal/registry/quicping.go @@ -10,13 +10,15 @@ import ( ) func init() { - AllExperiments["quicping"] = func() *Factory { + const canonicalName = "quicping" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config interface{}) model.ExperimentMeasurer { return quicping.NewExperimentMeasurer( *config.(*quicping.Config), ) }, + canonicalName: canonicalName, config: &quicping.Config{}, enabledByDefault: true, inputPolicy: model.InputStrictlyRequired, diff --git a/internal/registry/riseupvpn.go b/internal/registry/riseupvpn.go index 950a86b79e..6527f3a04f 100644 --- a/internal/registry/riseupvpn.go +++ b/internal/registry/riseupvpn.go @@ -10,15 +10,17 @@ import ( ) func init() { - AllExperiments["riseupvpn"] = func() *Factory { + const canonicalName = "riseupvpn" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config interface{}) model.ExperimentMeasurer { return riseupvpn.NewExperimentMeasurer( *config.(*riseupvpn.Config), ) }, - config: &riseupvpn.Config{}, - inputPolicy: model.InputNone, + canonicalName: canonicalName, + config: &riseupvpn.Config{}, + inputPolicy: model.InputNone, } } } diff --git a/internal/registry/signal.go b/internal/registry/signal.go index 615d44b732..5890936b31 100644 --- a/internal/registry/signal.go +++ b/internal/registry/signal.go @@ -10,13 +10,15 @@ import ( ) func init() { - AllExperiments["signal"] = func() *Factory { + const canonicalName = "signal" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config interface{}) model.ExperimentMeasurer { return signal.NewExperimentMeasurer( *config.(*signal.Config), ) }, + canonicalName: canonicalName, config: &signal.Config{}, enabledByDefault: true, inputPolicy: model.InputNone, diff --git a/internal/registry/simplequicping.go b/internal/registry/simplequicping.go index 8eb3e7c54e..c83b8cec88 100644 --- a/internal/registry/simplequicping.go +++ b/internal/registry/simplequicping.go @@ -10,13 +10,15 @@ import ( ) func init() { - AllExperiments["simplequicping"] = func() *Factory { + const canonicalName = "simplequicping" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config interface{}) model.ExperimentMeasurer { return simplequicping.NewExperimentMeasurer( *config.(*simplequicping.Config), ) }, + canonicalName: canonicalName, config: &simplequicping.Config{}, enabledByDefault: true, inputPolicy: model.InputStrictlyRequired, diff --git a/internal/registry/sniblocking.go b/internal/registry/sniblocking.go index 8af8214d68..a433de3dc2 100644 --- a/internal/registry/sniblocking.go +++ b/internal/registry/sniblocking.go @@ -10,13 +10,15 @@ import ( ) func init() { - AllExperiments["sni_blocking"] = func() *Factory { + const canonicalName = "sni_blocking" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config interface{}) model.ExperimentMeasurer { return sniblocking.NewExperimentMeasurer( *config.(*sniblocking.Config), ) }, + canonicalName: canonicalName, config: &sniblocking.Config{}, enabledByDefault: true, inputPolicy: model.InputOrQueryBackend, diff --git a/internal/registry/stunreachability.go b/internal/registry/stunreachability.go index dfa331c3fc..db35b59845 100644 --- a/internal/registry/stunreachability.go +++ b/internal/registry/stunreachability.go @@ -10,13 +10,15 @@ import ( ) func init() { - AllExperiments["stunreachability"] = func() *Factory { + const canonicalName = "stunreachability" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config interface{}) model.ExperimentMeasurer { return stunreachability.NewExperimentMeasurer( *config.(*stunreachability.Config), ) }, + canonicalName: canonicalName, config: &stunreachability.Config{}, enabledByDefault: true, inputPolicy: model.InputOrStaticDefault, diff --git a/internal/registry/tcpping.go b/internal/registry/tcpping.go index d0ca932494..029e6e2d1c 100644 --- a/internal/registry/tcpping.go +++ b/internal/registry/tcpping.go @@ -10,13 +10,15 @@ import ( ) func init() { - AllExperiments["tcpping"] = func() *Factory { + const canonicalName = "tcpping" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config interface{}) model.ExperimentMeasurer { return tcpping.NewExperimentMeasurer( *config.(*tcpping.Config), ) }, + canonicalName: canonicalName, config: &tcpping.Config{}, enabledByDefault: true, inputPolicy: model.InputStrictlyRequired, diff --git a/internal/registry/telegram.go b/internal/registry/telegram.go index 8f61d78baa..987e640935 100644 --- a/internal/registry/telegram.go +++ b/internal/registry/telegram.go @@ -10,13 +10,15 @@ import ( ) func init() { - AllExperiments["telegram"] = func() *Factory { + const canonicalName = "telegram" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config any) model.ExperimentMeasurer { return telegram.NewExperimentMeasurer( config.(telegram.Config), ) }, + canonicalName: canonicalName, config: telegram.Config{}, enabledByDefault: true, interruptible: false, diff --git a/internal/registry/tlsmiddlebox.go b/internal/registry/tlsmiddlebox.go index a97de82c89..73e1ecfa33 100644 --- a/internal/registry/tlsmiddlebox.go +++ b/internal/registry/tlsmiddlebox.go @@ -10,13 +10,15 @@ import ( ) func init() { - AllExperiments["tlsmiddlebox"] = func() *Factory { + const canonicalName = "tlsmiddlebox" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config interface{}) model.ExperimentMeasurer { return tlsmiddlebox.NewExperimentMeasurer( *config.(*tlsmiddlebox.Config), ) }, + canonicalName: canonicalName, config: &tlsmiddlebox.Config{}, enabledByDefault: true, inputPolicy: model.InputStrictlyRequired, diff --git a/internal/registry/tlsping.go b/internal/registry/tlsping.go index a40723d20b..6ec5048d52 100644 --- a/internal/registry/tlsping.go +++ b/internal/registry/tlsping.go @@ -10,13 +10,15 @@ import ( ) func init() { - AllExperiments["tlsping"] = func() *Factory { + const canonicalName = "tlsping" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config interface{}) model.ExperimentMeasurer { return tlsping.NewExperimentMeasurer( *config.(*tlsping.Config), ) }, + canonicalName: canonicalName, config: &tlsping.Config{}, enabledByDefault: true, inputPolicy: model.InputStrictlyRequired, diff --git a/internal/registry/tlstool.go b/internal/registry/tlstool.go index 8bb70983f2..0fe2625a55 100644 --- a/internal/registry/tlstool.go +++ b/internal/registry/tlstool.go @@ -10,13 +10,15 @@ import ( ) func init() { - AllExperiments["tlstool"] = func() *Factory { + const canonicalName = "tlstool" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config interface{}) model.ExperimentMeasurer { return tlstool.NewExperimentMeasurer( *config.(*tlstool.Config), ) }, + canonicalName: canonicalName, config: &tlstool.Config{}, enabledByDefault: true, inputPolicy: model.InputOrQueryBackend, diff --git a/internal/registry/tor.go b/internal/registry/tor.go index 73098bf770..2e2a613265 100644 --- a/internal/registry/tor.go +++ b/internal/registry/tor.go @@ -10,13 +10,15 @@ import ( ) func init() { - AllExperiments["tor"] = func() *Factory { + const canonicalName = "tor" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config interface{}) model.ExperimentMeasurer { return tor.NewExperimentMeasurer( *config.(*tor.Config), ) }, + canonicalName: canonicalName, config: &tor.Config{}, enabledByDefault: true, inputPolicy: model.InputNone, diff --git a/internal/registry/torsf.go b/internal/registry/torsf.go index 178f01d5e4..3090900445 100644 --- a/internal/registry/torsf.go +++ b/internal/registry/torsf.go @@ -10,13 +10,15 @@ import ( ) func init() { - AllExperiments["torsf"] = func() *Factory { + const canonicalName = "torsf" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config interface{}) model.ExperimentMeasurer { return torsf.NewExperimentMeasurer( *config.(*torsf.Config), ) }, + canonicalName: canonicalName, config: &torsf.Config{}, enabledByDefault: false, inputPolicy: model.InputNone, diff --git a/internal/registry/urlgetter.go b/internal/registry/urlgetter.go index 63dba20f9e..ae3391bd67 100644 --- a/internal/registry/urlgetter.go +++ b/internal/registry/urlgetter.go @@ -10,13 +10,15 @@ import ( ) func init() { - AllExperiments["urlgetter"] = func() *Factory { + const canonicalName = "urlgetter" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config interface{}) model.ExperimentMeasurer { return urlgetter.NewExperimentMeasurer( *config.(*urlgetter.Config), ) }, + canonicalName: canonicalName, config: &urlgetter.Config{}, enabledByDefault: true, inputPolicy: model.InputStrictlyRequired, diff --git a/internal/registry/vanillator.go b/internal/registry/vanillator.go index 1af548660a..a1835e22ce 100644 --- a/internal/registry/vanillator.go +++ b/internal/registry/vanillator.go @@ -10,14 +10,16 @@ import ( ) func init() { - AllExperiments["vanilla_tor"] = func() *Factory { + const canonicalName = "vanilla_tor" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config interface{}) model.ExperimentMeasurer { return vanillator.NewExperimentMeasurer( *config.(*vanillator.Config), ) }, - config: &vanillator.Config{}, + canonicalName: canonicalName, + config: &vanillator.Config{}, // We discussed this topic with @aanorbel. On Android this experiment crashes // frequently because of https://github.com/ooni/probe/issues/2406. So, it seems // more cautious to disable it by default and let the check-in API decide. diff --git a/internal/registry/webconnectivity.go b/internal/registry/webconnectivity.go index 1ea720d581..470bad802b 100644 --- a/internal/registry/webconnectivity.go +++ b/internal/registry/webconnectivity.go @@ -10,13 +10,15 @@ import ( ) func init() { - AllExperiments["web_connectivity"] = func() *Factory { + const canonicalName = "web_connectivity" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config any) model.ExperimentMeasurer { return webconnectivity.NewExperimentMeasurer( config.(webconnectivity.Config), ) }, + canonicalName: canonicalName, config: webconnectivity.Config{}, enabledByDefault: true, interruptible: false, diff --git a/internal/registry/webconnectivityv05.go b/internal/registry/webconnectivityv05.go index 0881b4448f..3a56511a69 100644 --- a/internal/registry/webconnectivityv05.go +++ b/internal/registry/webconnectivityv05.go @@ -12,13 +12,15 @@ import ( ) func init() { - AllExperiments["web_connectivity@v0.5"] = func() *Factory { + const canonicalName = "web_connectivity@v0.5" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config any) model.ExperimentMeasurer { return webconnectivitylte.NewExperimentMeasurer( config.(*webconnectivitylte.Config), ) }, + canonicalName: canonicalName, config: &webconnectivitylte.Config{}, enabledByDefault: true, interruptible: false, diff --git a/internal/registry/whatsapp.go b/internal/registry/whatsapp.go index 84acb57898..24a27a2acc 100644 --- a/internal/registry/whatsapp.go +++ b/internal/registry/whatsapp.go @@ -10,13 +10,15 @@ import ( ) func init() { - AllExperiments["whatsapp"] = func() *Factory { + const canonicalName = "whatsapp" + AllExperiments[canonicalName] = func() *Factory { return &Factory{ build: func(config interface{}) model.ExperimentMeasurer { return whatsapp.NewExperimentMeasurer( *config.(*whatsapp.Config), ) }, + canonicalName: canonicalName, config: &whatsapp.Config{}, enabledByDefault: true, inputPolicy: model.InputNone, diff --git a/internal/targetloading/targetloading.go b/internal/targetloading/targetloading.go index 6590c2c4a7..66a22bc88e 100644 --- a/internal/targetloading/targetloading.go +++ b/internal/targetloading/targetloading.go @@ -13,7 +13,6 @@ import ( "github.com/ooni/probe-cli/v3/internal/experiment/openvpn" "github.com/ooni/probe-cli/v3/internal/fsx" "github.com/ooni/probe-cli/v3/internal/model" - "github.com/ooni/probe-cli/v3/internal/registry" "github.com/ooni/probe-cli/v3/internal/stuninput" ) @@ -26,12 +25,8 @@ var ( ErrNoStaticInput = errors.New("no static input for this experiment") ) -// Session is the session according to a [*Loader]. -type Session interface { - CheckIn(ctx context.Context, config *model.OOAPICheckInConfig) (*model.OOAPICheckInResult, error) - FetchOpenVPNConfig(ctx context.Context, - provider, cc string) (*model.OOAPIVPNProviderConfig, error) -} +// Session is the session according to a [*Loader] instance. +type Session = model.ExperimentTargetLoaderSession // Logger is the [model.Logger] according to a [*Loader]. type Logger interface { @@ -230,7 +225,7 @@ func StaticBareInputForExperiment(name string) ([]string, error) { // // TODO(https://github.com/ooni/probe/issues/2557): server STUNReachability // inputs using richer input (aka check-in v2). - switch registry.CanonicalizeExperimentName(name) { + switch name { case "dnscheck": return dnsCheckDefaultInput, nil case "stunreachability": @@ -305,7 +300,7 @@ func (il *Loader) readfile(filepath string, open openFunc) ([]model.ExperimentTa // loadRemote loads inputs from a remote source. func (il *Loader) loadRemote(ctx context.Context) ([]model.ExperimentTarget, error) { - switch registry.CanonicalizeExperimentName(il.ExperimentName) { + switch il.ExperimentName { case "openvpn": // TODO(ainghazal): given the semantics of the current API call, in an ideal world we'd need to pass // the desired provider here, if known. We only have one provider for now, so I'm acting like YAGNI. Another diff --git a/internal/targetloading/targetloading_test.go b/internal/targetloading/targetloading_test.go index 9710ce1b34..82684a8205 100644 --- a/internal/targetloading/targetloading_test.go +++ b/internal/targetloading/targetloading_test.go @@ -11,6 +11,7 @@ import ( "testing" "time" + "github.com/apex/log" "github.com/google/go-cmp/cmp" "github.com/ooni/probe-cli/v3/internal/mocks" "github.com/ooni/probe-cli/v3/internal/model" @@ -535,7 +536,7 @@ type TargetLoaderMockableSession struct { Error error } -// CheckIn implements TargetLoaderSession.CheckIn. +// CheckIn implements [Session]. func (sess *TargetLoaderMockableSession) CheckIn( ctx context.Context, config *model.OOAPICheckInConfig) (*model.OOAPICheckInResult, error) { if sess.Output == nil && sess.Error == nil { @@ -544,13 +545,19 @@ func (sess *TargetLoaderMockableSession) CheckIn( return sess.Output, sess.Error } -// FetchOpenVPNConfig implements TargetLoaderSession.FetchOpenVPNConfig. +// FetchOpenVPNConfig implements [Session]. func (sess *TargetLoaderMockableSession) FetchOpenVPNConfig( ctx context.Context, provider, cc string) (*model.OOAPIVPNProviderConfig, error) { runtimex.Assert(!(sess.Error == nil && sess.FetchOpenVPNConfigOutput == nil), "both FetchOpenVPNConfig and Error are nil") return sess.FetchOpenVPNConfigOutput, sess.Error } +// Logger implements [Session]. +func (sess *TargetLoaderMockableSession) Logger() model.Logger { + // Such that we see some logs when running tests + return log.Log +} + func TestTargetLoaderCheckInFailure(t *testing.T) { il := &Loader{ Session: &TargetLoaderMockableSession{