diff --git a/cmd/ooniprobe/internal/nettests/nettests.go b/cmd/ooniprobe/internal/nettests/nettests.go index 711c544..2a0a275 100644 --- a/cmd/ooniprobe/internal/nettests/nettests.go +++ b/cmd/ooniprobe/internal/nettests/nettests.go @@ -1,6 +1,7 @@ package nettests import ( + "context" "database/sql" "fmt" "time" @@ -122,7 +123,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 *engine.ExperimentBuilder, inputs []string) error { +func (c *Controller) Run(builder engine.ExperimentBuilder, inputs []string) error { // This will configure the controller as handler for the callbacks // called by ooni/probe-engine/experiment.Experiment. builder.SetCallbacks(model.ExperimentCallbacks(c)) @@ -143,7 +144,7 @@ func (c *Controller) Run(builder *engine.ExperimentBuilder, inputs []string) err log.Debug(color.RedString("status.started")) if c.Probe.Config().Sharing.UploadResults { - if err := exp.OpenReport(); err != nil { + if err := exp.OpenReportContext(context.Background()); err != nil { log.Debugf( "%s: %s", color.RedString("failure.report_create"), err.Error(), ) @@ -197,7 +198,7 @@ func (c *Controller) Run(builder *engine.ExperimentBuilder, inputs []string) err if input != "" { c.OnProgress(0, fmt.Sprintf("processing input: %s", input)) } - measurement, err := exp.Measure(input) + measurement, err := exp.MeasureWithContext(context.Background(), input) if err != nil { log.WithError(err).Debug(color.RedString("failure.measurement")) if err := c.msmts[idx64].Failed(c.Probe.DB(), err.Error()); err != nil { @@ -218,7 +219,7 @@ func (c *Controller) Run(builder *engine.ExperimentBuilder, inputs []string) err // Implementation note: SubmitMeasurement will fail here if we did fail // to open the report but we still want to continue. There will be a // bit of a spew in the logs, perhaps, but stopping seems less efficient. - if err := exp.SubmitAndUpdateMeasurement(measurement); err != nil { + if err := exp.SubmitAndUpdateMeasurementContext(context.Background(), measurement); err != nil { log.Debug(color.RedString("failure.measurement_submission")) if err := c.msmts[idx64].UploadFailed(c.Probe.DB(), err.Error()); err != nil { return errors.Wrap(err, "failed to mark upload as failed") diff --git a/internal/engine/allexperiments.go b/internal/engine/allexperiments.go index f702118..d3471d7 100644 --- a/internal/engine/allexperiments.go +++ b/internal/engine/allexperiments.go @@ -1,5 +1,9 @@ package engine +// +// List of all implemented experiments +// + import ( "time" @@ -32,11 +36,11 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/experiment/whatsapp" ) -var experimentsByName = map[string]func(*Session) *ExperimentBuilder{ - "dash": func(session *Session) *ExperimentBuilder { - return &ExperimentBuilder{ - build: func(config interface{}) *Experiment { - return NewExperiment(session, dash.NewExperimentMeasurer( +var experimentsByName = map[string]func(*Session) *experimentBuilder{ + "dash": func(session *Session) *experimentBuilder { + return &experimentBuilder{ + build: func(config interface{}) *experiment { + return newExperiment(session, dash.NewExperimentMeasurer( *config.(*dash.Config), )) }, @@ -46,10 +50,10 @@ var experimentsByName = map[string]func(*Session) *ExperimentBuilder{ } }, - "dnscheck": func(session *Session) *ExperimentBuilder { - return &ExperimentBuilder{ - build: func(config interface{}) *Experiment { - return NewExperiment(session, dnscheck.NewExperimentMeasurer( + "dnscheck": func(session *Session) *experimentBuilder { + return &experimentBuilder{ + build: func(config interface{}) *experiment { + return newExperiment(session, dnscheck.NewExperimentMeasurer( *config.(*dnscheck.Config), )) }, @@ -58,10 +62,10 @@ var experimentsByName = map[string]func(*Session) *ExperimentBuilder{ } }, - "dnsping": func(session *Session) *ExperimentBuilder { - return &ExperimentBuilder{ - build: func(config interface{}) *Experiment { - return NewExperiment(session, dnsping.NewExperimentMeasurer( + "dnsping": func(session *Session) *experimentBuilder { + return &experimentBuilder{ + build: func(config interface{}) *experiment { + return newExperiment(session, dnsping.NewExperimentMeasurer( *config.(*dnsping.Config), )) }, @@ -70,10 +74,10 @@ var experimentsByName = map[string]func(*Session) *ExperimentBuilder{ } }, - "example": func(session *Session) *ExperimentBuilder { - return &ExperimentBuilder{ - build: func(config interface{}) *Experiment { - return NewExperiment(session, example.NewExperimentMeasurer( + "example": func(session *Session) *experimentBuilder { + return &experimentBuilder{ + build: func(config interface{}) *experiment { + return newExperiment(session, example.NewExperimentMeasurer( *config.(*example.Config), "example", )) }, @@ -86,10 +90,10 @@ var experimentsByName = map[string]func(*Session) *ExperimentBuilder{ } }, - "facebook_messenger": func(session *Session) *ExperimentBuilder { - return &ExperimentBuilder{ - build: func(config interface{}) *Experiment { - return NewExperiment(session, fbmessenger.NewExperimentMeasurer( + "facebook_messenger": func(session *Session) *experimentBuilder { + return &experimentBuilder{ + build: func(config interface{}) *experiment { + return newExperiment(session, fbmessenger.NewExperimentMeasurer( *config.(*fbmessenger.Config), )) }, @@ -98,10 +102,10 @@ var experimentsByName = map[string]func(*Session) *ExperimentBuilder{ } }, - "http_header_field_manipulation": func(session *Session) *ExperimentBuilder { - return &ExperimentBuilder{ - build: func(config interface{}) *Experiment { - return NewExperiment(session, hhfm.NewExperimentMeasurer( + "http_header_field_manipulation": func(session *Session) *experimentBuilder { + return &experimentBuilder{ + build: func(config interface{}) *experiment { + return newExperiment(session, hhfm.NewExperimentMeasurer( *config.(*hhfm.Config), )) }, @@ -110,10 +114,10 @@ var experimentsByName = map[string]func(*Session) *ExperimentBuilder{ } }, - "http_host_header": func(session *Session) *ExperimentBuilder { - return &ExperimentBuilder{ - build: func(config interface{}) *Experiment { - return NewExperiment(session, httphostheader.NewExperimentMeasurer( + "http_host_header": func(session *Session) *experimentBuilder { + return &experimentBuilder{ + build: func(config interface{}) *experiment { + return newExperiment(session, httphostheader.NewExperimentMeasurer( *config.(*httphostheader.Config), )) }, @@ -122,10 +126,10 @@ var experimentsByName = map[string]func(*Session) *ExperimentBuilder{ } }, - "http_invalid_request_line": func(session *Session) *ExperimentBuilder { - return &ExperimentBuilder{ - build: func(config interface{}) *Experiment { - return NewExperiment(session, hirl.NewExperimentMeasurer( + "http_invalid_request_line": func(session *Session) *experimentBuilder { + return &experimentBuilder{ + build: func(config interface{}) *experiment { + return newExperiment(session, hirl.NewExperimentMeasurer( *config.(*hirl.Config), )) }, @@ -134,10 +138,10 @@ var experimentsByName = map[string]func(*Session) *ExperimentBuilder{ } }, - "ndt": func(session *Session) *ExperimentBuilder { - return &ExperimentBuilder{ - build: func(config interface{}) *Experiment { - return NewExperiment(session, ndt7.NewExperimentMeasurer( + "ndt": func(session *Session) *experimentBuilder { + return &experimentBuilder{ + build: func(config interface{}) *experiment { + return newExperiment(session, ndt7.NewExperimentMeasurer( *config.(*ndt7.Config), )) }, @@ -147,10 +151,10 @@ var experimentsByName = map[string]func(*Session) *ExperimentBuilder{ } }, - "psiphon": func(session *Session) *ExperimentBuilder { - return &ExperimentBuilder{ - build: func(config interface{}) *Experiment { - return NewExperiment(session, psiphon.NewExperimentMeasurer( + "psiphon": func(session *Session) *experimentBuilder { + return &experimentBuilder{ + build: func(config interface{}) *experiment { + return newExperiment(session, psiphon.NewExperimentMeasurer( *config.(*psiphon.Config), )) }, @@ -159,10 +163,10 @@ var experimentsByName = map[string]func(*Session) *ExperimentBuilder{ } }, - "quicping": func(session *Session) *ExperimentBuilder { - return &ExperimentBuilder{ - build: func(config interface{}) *Experiment { - return NewExperiment(session, quicping.NewExperimentMeasurer( + "quicping": func(session *Session) *experimentBuilder { + return &experimentBuilder{ + build: func(config interface{}) *experiment { + return newExperiment(session, quicping.NewExperimentMeasurer( *config.(*quicping.Config), )) }, @@ -171,10 +175,10 @@ var experimentsByName = map[string]func(*Session) *ExperimentBuilder{ } }, - "riseupvpn": func(session *Session) *ExperimentBuilder { - return &ExperimentBuilder{ - build: func(config interface{}) *Experiment { - return NewExperiment(session, riseupvpn.NewExperimentMeasurer( + "riseupvpn": func(session *Session) *experimentBuilder { + return &experimentBuilder{ + build: func(config interface{}) *experiment { + return newExperiment(session, riseupvpn.NewExperimentMeasurer( *config.(*riseupvpn.Config), )) }, @@ -183,10 +187,10 @@ var experimentsByName = map[string]func(*Session) *ExperimentBuilder{ } }, - "run": func(session *Session) *ExperimentBuilder { - return &ExperimentBuilder{ - build: func(config interface{}) *Experiment { - return NewExperiment(session, run.NewExperimentMeasurer( + "run": func(session *Session) *experimentBuilder { + return &experimentBuilder{ + build: func(config interface{}) *experiment { + return newExperiment(session, run.NewExperimentMeasurer( *config.(*run.Config), )) }, @@ -195,10 +199,10 @@ var experimentsByName = map[string]func(*Session) *ExperimentBuilder{ } }, - "simplequicping": func(session *Session) *ExperimentBuilder { - return &ExperimentBuilder{ - build: func(config interface{}) *Experiment { - return NewExperiment(session, simplequicping.NewExperimentMeasurer( + "simplequicping": func(session *Session) *experimentBuilder { + return &experimentBuilder{ + build: func(config interface{}) *experiment { + return newExperiment(session, simplequicping.NewExperimentMeasurer( *config.(*simplequicping.Config), )) }, @@ -207,10 +211,10 @@ var experimentsByName = map[string]func(*Session) *ExperimentBuilder{ } }, - "signal": func(session *Session) *ExperimentBuilder { - return &ExperimentBuilder{ - build: func(config interface{}) *Experiment { - return NewExperiment(session, signal.NewExperimentMeasurer( + "signal": func(session *Session) *experimentBuilder { + return &experimentBuilder{ + build: func(config interface{}) *experiment { + return newExperiment(session, signal.NewExperimentMeasurer( *config.(*signal.Config), )) }, @@ -219,10 +223,10 @@ var experimentsByName = map[string]func(*Session) *ExperimentBuilder{ } }, - "sni_blocking": func(session *Session) *ExperimentBuilder { - return &ExperimentBuilder{ - build: func(config interface{}) *Experiment { - return NewExperiment(session, sniblocking.NewExperimentMeasurer( + "sni_blocking": func(session *Session) *experimentBuilder { + return &experimentBuilder{ + build: func(config interface{}) *experiment { + return newExperiment(session, sniblocking.NewExperimentMeasurer( *config.(*sniblocking.Config), )) }, @@ -231,10 +235,10 @@ var experimentsByName = map[string]func(*Session) *ExperimentBuilder{ } }, - "stunreachability": func(session *Session) *ExperimentBuilder { - return &ExperimentBuilder{ - build: func(config interface{}) *Experiment { - return NewExperiment(session, stunreachability.NewExperimentMeasurer( + "stunreachability": func(session *Session) *experimentBuilder { + return &experimentBuilder{ + build: func(config interface{}) *experiment { + return newExperiment(session, stunreachability.NewExperimentMeasurer( *config.(*stunreachability.Config), )) }, @@ -243,10 +247,10 @@ var experimentsByName = map[string]func(*Session) *ExperimentBuilder{ } }, - "tcpping": func(session *Session) *ExperimentBuilder { - return &ExperimentBuilder{ - build: func(config interface{}) *Experiment { - return NewExperiment(session, tcpping.NewExperimentMeasurer( + "tcpping": func(session *Session) *experimentBuilder { + return &experimentBuilder{ + build: func(config interface{}) *experiment { + return newExperiment(session, tcpping.NewExperimentMeasurer( *config.(*tcpping.Config), )) }, @@ -255,10 +259,10 @@ var experimentsByName = map[string]func(*Session) *ExperimentBuilder{ } }, - "tlsping": func(session *Session) *ExperimentBuilder { - return &ExperimentBuilder{ - build: func(config interface{}) *Experiment { - return NewExperiment(session, tlsping.NewExperimentMeasurer( + "tlsping": func(session *Session) *experimentBuilder { + return &experimentBuilder{ + build: func(config interface{}) *experiment { + return newExperiment(session, tlsping.NewExperimentMeasurer( *config.(*tlsping.Config), )) }, @@ -267,10 +271,10 @@ var experimentsByName = map[string]func(*Session) *ExperimentBuilder{ } }, - "telegram": func(session *Session) *ExperimentBuilder { - return &ExperimentBuilder{ - build: func(config interface{}) *Experiment { - return NewExperiment(session, telegram.NewExperimentMeasurer( + "telegram": func(session *Session) *experimentBuilder { + return &experimentBuilder{ + build: func(config interface{}) *experiment { + return newExperiment(session, telegram.NewExperimentMeasurer( *config.(*telegram.Config), )) }, @@ -279,10 +283,10 @@ var experimentsByName = map[string]func(*Session) *ExperimentBuilder{ } }, - "tlstool": func(session *Session) *ExperimentBuilder { - return &ExperimentBuilder{ - build: func(config interface{}) *Experiment { - return NewExperiment(session, tlstool.NewExperimentMeasurer( + "tlstool": func(session *Session) *experimentBuilder { + return &experimentBuilder{ + build: func(config interface{}) *experiment { + return newExperiment(session, tlstool.NewExperimentMeasurer( *config.(*tlstool.Config), )) }, @@ -291,10 +295,10 @@ var experimentsByName = map[string]func(*Session) *ExperimentBuilder{ } }, - "tor": func(session *Session) *ExperimentBuilder { - return &ExperimentBuilder{ - build: func(config interface{}) *Experiment { - return NewExperiment(session, tor.NewExperimentMeasurer( + "tor": func(session *Session) *experimentBuilder { + return &experimentBuilder{ + build: func(config interface{}) *experiment { + return newExperiment(session, tor.NewExperimentMeasurer( *config.(*tor.Config), )) }, @@ -303,10 +307,10 @@ var experimentsByName = map[string]func(*Session) *ExperimentBuilder{ } }, - "torsf": func(session *Session) *ExperimentBuilder { - return &ExperimentBuilder{ - build: func(config interface{}) *Experiment { - return NewExperiment(session, torsf.NewExperimentMeasurer( + "torsf": func(session *Session) *experimentBuilder { + return &experimentBuilder{ + build: func(config interface{}) *experiment { + return newExperiment(session, torsf.NewExperimentMeasurer( *config.(*torsf.Config), )) }, @@ -315,10 +319,10 @@ var experimentsByName = map[string]func(*Session) *ExperimentBuilder{ } }, - "urlgetter": func(session *Session) *ExperimentBuilder { - return &ExperimentBuilder{ - build: func(config interface{}) *Experiment { - return NewExperiment(session, urlgetter.NewExperimentMeasurer( + "urlgetter": func(session *Session) *experimentBuilder { + return &experimentBuilder{ + build: func(config interface{}) *experiment { + return newExperiment(session, urlgetter.NewExperimentMeasurer( *config.(*urlgetter.Config), )) }, @@ -327,10 +331,10 @@ var experimentsByName = map[string]func(*Session) *ExperimentBuilder{ } }, - "vanilla_tor": func(session *Session) *ExperimentBuilder { - return &ExperimentBuilder{ - build: func(config interface{}) *Experiment { - return NewExperiment(session, vanillator.NewExperimentMeasurer( + "vanilla_tor": func(session *Session) *experimentBuilder { + return &experimentBuilder{ + build: func(config interface{}) *experiment { + return newExperiment(session, vanillator.NewExperimentMeasurer( *config.(*vanillator.Config), )) }, @@ -339,10 +343,10 @@ var experimentsByName = map[string]func(*Session) *ExperimentBuilder{ } }, - "web_connectivity": func(session *Session) *ExperimentBuilder { - return &ExperimentBuilder{ - build: func(config interface{}) *Experiment { - return NewExperiment(session, webconnectivity.NewExperimentMeasurer( + "web_connectivity": func(session *Session) *experimentBuilder { + return &experimentBuilder{ + build: func(config interface{}) *experiment { + return newExperiment(session, webconnectivity.NewExperimentMeasurer( *config.(*webconnectivity.Config), )) }, @@ -351,10 +355,10 @@ var experimentsByName = map[string]func(*Session) *ExperimentBuilder{ } }, - "whatsapp": func(session *Session) *ExperimentBuilder { - return &ExperimentBuilder{ - build: func(config interface{}) *Experiment { - return NewExperiment(session, whatsapp.NewExperimentMeasurer( + "whatsapp": func(session *Session) *experimentBuilder { + return &experimentBuilder{ + build: func(config interface{}) *experiment { + return newExperiment(session, whatsapp.NewExperimentMeasurer( *config.(*whatsapp.Config), )) }, diff --git a/internal/engine/experiment.go b/internal/engine/experiment.go index e271075..be5a21a 100644 --- a/internal/engine/experiment.go +++ b/internal/engine/experiment.go @@ -1,5 +1,9 @@ package engine +// +// Experiment definition and implementation. +// + import ( "context" "encoding/json" @@ -23,7 +27,77 @@ func formatTimeNowUTC() string { } // Experiment is an experiment instance. -type Experiment struct { +type Experiment interface { + // KibiBytesReceived accounts for the KibiBytes received by the experiment. + KibiBytesReceived() float64 + + // KibiBytesSent is like KibiBytesReceived but for the bytes sent. + KibiBytesSent() float64 + + // Name returns the experiment name. + Name() string + + // GetSummaryKeys returns a data structure containing a + // summary of the test keys for ooniprobe. + GetSummaryKeys(m *model.Measurement) (any, error) + + // ReportID returns the open report's ID, if we have opened a report + // successfully before, or an empty string, otherwise. + // + // Deprecated: new code should use a Submitter. + ReportID() string + + // MeasureAsync runs an async measurement. This operation could post + // one or more measurements onto the returned channel. We'll close the + // channel when we've emitted all the measurements. + // + // Arguments: + // + // - ctx is the context for deadline/cancellation/timeout; + // + // - input is the input (typically a URL but it could also be + // just an endpoint or an empty string for input-less experiments + // such as, e.g., ndt7 and dash). + // + // Return value: + // + // - on success, channel where to post measurements (the channel + // will be closed when done) and nil error; + // + // - on failure, nil channel and non-nil error. + MeasureAsync(ctx context.Context, input string) (<-chan *model.Measurement, error) + + // MeasureWithContext performs a synchronous measurement. + // + // Return value: strictly either a non-nil measurement and + // a nil error or a nil measurement and a non-nil error. + // + // CAVEAT: while this API is perfectly fine for experiments that + // return a single measurement, it will only return the first measurement + // when used with an asynchronous experiment. + MeasureWithContext(ctx context.Context, input string) (measurement *model.Measurement, err error) + + // SaveMeasurement saves a measurement on the specified file path. + // + // Deprecated: new code should use a Saver. + SaveMeasurement(measurement *model.Measurement, filePath string) error + + // SubmitAndUpdateMeasurementContext submits a measurement and updates the + // fields whose value has changed as part of the submission. + // + // Deprecated: new code should use a Submitter. + SubmitAndUpdateMeasurementContext( + ctx context.Context, measurement *model.Measurement) error + + // OpenReportContext will open a report using the given context + // to possibly limit the lifetime of this operation. + // + // Deprecated: new code should use a Submitter. + OpenReportContext(ctx context.Context) error +} + +// experiment implements Experiment. +type experiment struct { byteCounter *bytecounter.Counter callbacks model.ExperimentCallbacks measurer model.ExperimentMeasurer @@ -34,11 +108,9 @@ type Experiment struct { testVersion string } -// NewExperiment creates a new experiment given a measurer. The preferred -// way to create an experiment is the ExperimentBuilder. Though this function -// allows the programmer to create a custom, external experiment. -func NewExperiment(sess *Session, measurer model.ExperimentMeasurer) *Experiment { - return &Experiment{ +// newExperiment creates a new experiment given a measurer. +func newExperiment(sess *Session, measurer model.ExperimentMeasurer) *experiment { + return &experiment{ byteCounter: bytecounter.New(), callbacks: model.NewPrinterCallbacks(sess.Logger()), measurer: measurer, @@ -49,64 +121,37 @@ func NewExperiment(sess *Session, measurer model.ExperimentMeasurer) *Experiment } } -// KibiBytesReceived accounts for the KibiBytes received by the HTTP clients -// managed by this session so far, including experiments. -func (e *Experiment) KibiBytesReceived() float64 { +// KibiBytesReceived implements Experiment.KibiBytesReceived. +func (e *experiment) KibiBytesReceived() float64 { return e.byteCounter.KibiBytesReceived() } -// KibiBytesSent is like KibiBytesReceived but for the bytes sent. -func (e *Experiment) KibiBytesSent() float64 { +// KibiBytesSent implements Experiment.KibiBytesSent. +func (e *experiment) KibiBytesSent() float64 { return e.byteCounter.KibiBytesSent() } -// Name returns the experiment name. -func (e *Experiment) Name() string { +// Name implements Experiment.Name. +func (e *experiment) Name() string { return e.testName } -// GetSummaryKeys returns a data structure containing a -// summary of the test keys for probe-cli. -func (e *Experiment) GetSummaryKeys(m *model.Measurement) (interface{}, error) { +// GetSummaryKeys implements Experiment.GetSummaryKeys. +func (e *experiment) GetSummaryKeys(m *model.Measurement) (interface{}, error) { return e.measurer.GetSummaryKeys(m) } -// OpenReport is an idempotent method to open a report. We assume that -// you have configured the available probe services, either manually or -// through using the session's MaybeLookupBackends method. -func (e *Experiment) OpenReport() (err error) { - return e.OpenReportContext(context.Background()) -} - -// ReportID returns the open reportID, if we have opened a report -// successfully before, or an empty string, otherwise. -func (e *Experiment) ReportID() string { +// ReportID implements Experiment.ReportID. +func (e *experiment) ReportID() string { if e.report == nil { return "" } return e.report.ReportID() } -// Measure performs a measurement with input. We assume that you have -// configured the available test helpers, either manually or by calling -// the session's MaybeLookupBackends() method. -// -// Return value: strictly either a non-nil measurement and -// a nil error or a nil measurement and a non-nil error. -// -// CAVEAT: while this API is perfectly fine for experiments that -// return a single measurement, it will only return the first measurement -// when used with an asynchronous experiment. We plan on eventually -// migrating all experiments to run in asynchronous fashion. -// -// Deprecated: use MeasureWithContext instead, please. -func (e *Experiment) Measure(input string) (*model.Measurement, error) { - return e.MeasureWithContext(context.Background(), input) -} - // experimentAsyncWrapper makes a sync experiment behave like it was async type experimentAsyncWrapper struct { - *Experiment + *experiment } var _ model.ExperimentMeasurerAsync = &experimentAsyncWrapper{} @@ -116,9 +161,9 @@ func (eaw *experimentAsyncWrapper) RunAsync( ctx context.Context, sess model.ExperimentSession, input string, callbacks model.ExperimentCallbacks) (<-chan *model.ExperimentAsyncTestKeys, error) { out := make(chan *model.ExperimentAsyncTestKeys) - measurement := eaw.Experiment.newMeasurement(input) + measurement := eaw.experiment.newMeasurement(input) start := time.Now() - err := eaw.Experiment.measurer.Run(ctx, eaw.session, measurement, eaw.callbacks) + err := eaw.experiment.measurer.Run(ctx, eaw.session, measurement, eaw.callbacks) stop := time.Now() if err != nil { return nil, err @@ -136,25 +181,8 @@ func (eaw *experimentAsyncWrapper) RunAsync( return out, nil } -// MeasureAsync runs an async measurement. This operation could post -// one or more measurements onto the returned channel. We'll close the -// channel when we've emitted all the measurements. -// -// Arguments: -// -// - ctx is the context for deadline/cancellation/timeout; -// -// - input is the input (typically a URL but it could also be -// just an endpoint or an empty string for input-less experiments -// such as, e.g., ndt7 and dash). -// -// Return value: -// -// - on success, channel where to post measurements (the channel -// will be closed when done) and nil error; -// -// - on failure, nil channel and non-nil error. -func (e *Experiment) MeasureAsync( +// MeasureAsync implements Experiment.MeasureAsync. +func (e *experiment) MeasureAsync( ctx context.Context, input string) (<-chan *model.Measurement, error) { err := e.session.MaybeLookupLocationContext(ctx) // this already tracks session bytes if err != nil { @@ -195,16 +223,8 @@ func (e *Experiment) MeasureAsync( return out, nil } -// MeasureWithContext is like Measure but with context. -// -// Return value: strictly either a non-nil measurement and -// a nil error or a nil measurement and a non-nil error. -// -// CAVEAT: while this API is perfectly fine for experiments that -// return a single measurement, it will only return the first measurement -// when used with an asynchronous experiment. We plan on eventually -// migrating all experiments to run in asynchronous fashion. -func (e *Experiment) MeasureWithContext( +// MeasureWithContext implements Experiment.MeasureWithContext. +func (e *experiment) MeasureWithContext( ctx context.Context, input string, ) (measurement *model.Measurement, err error) { out, err := e.MeasureAsync(ctx, input) @@ -222,8 +242,8 @@ func (e *Experiment) MeasureWithContext( return } -// SaveMeasurement saves a measurement on the specified file path. -func (e *Experiment) SaveMeasurement(measurement *model.Measurement, filePath string) error { +// SaveMeasurement implements Experiment.SaveMeasurement. +func (e *experiment) SaveMeasurement(measurement *model.Measurement, filePath string) error { return e.saveMeasurement( measurement, filePath, json.Marshal, os.OpenFile, func(fp *os.File, b []byte) (int, error) { @@ -232,15 +252,8 @@ func (e *Experiment) SaveMeasurement(measurement *model.Measurement, filePath st ) } -// SubmitAndUpdateMeasurement submits a measurement and updates the -// fields whose value has changed as part of the submission. -func (e *Experiment) SubmitAndUpdateMeasurement(measurement *model.Measurement) error { - return e.SubmitAndUpdateMeasurementContext(context.Background(), measurement) -} - -// SubmitAndUpdateMeasurementContext submits a measurement and updates the -// fields whose value has changed as part of the submission. -func (e *Experiment) SubmitAndUpdateMeasurementContext( +// SubmitAndUpdateMeasurementContext implements Experiment.SubmitAndUpdateMeasurementContext. +func (e *experiment) SubmitAndUpdateMeasurementContext( ctx context.Context, measurement *model.Measurement) error { if e.report == nil { return errors.New("report is not open") @@ -249,7 +262,7 @@ func (e *Experiment) SubmitAndUpdateMeasurementContext( } // newMeasurement creates a new measurement for this experiment with the given input. -func (e *Experiment) newMeasurement(input string) *model.Measurement { +func (e *experiment) newMeasurement(input string) *model.Measurement { utctimenow := time.Now().UTC() m := &model.Measurement{ DataFormatVersion: probeservices.DefaultDataFormatVersion, @@ -277,9 +290,8 @@ func (e *Experiment) newMeasurement(input string) *model.Measurement { return m } -// OpenReportContext will open a report using the given context -// to possibly limit the lifetime of this operation. -func (e *Experiment) OpenReportContext(ctx context.Context) error { +// OpenReportContext implements Experiment.OpenReportContext. +func (e *experiment) OpenReportContext(ctx context.Context) error { if e.report != nil { return nil // already open } @@ -305,7 +317,7 @@ func (e *Experiment) OpenReportContext(ctx context.Context) error { return nil } -func (e *Experiment) newReportTemplate() probeservices.ReportTemplate { +func (e *experiment) newReportTemplate() probeservices.ReportTemplate { return probeservices.ReportTemplate{ DataFormatVersion: probeservices.DefaultDataFormatVersion, Format: probeservices.DefaultFormat, @@ -319,7 +331,7 @@ func (e *Experiment) newReportTemplate() probeservices.ReportTemplate { } } -func (e *Experiment) saveMeasurement( +func (e *experiment) saveMeasurement( measurement *model.Measurement, filePath string, marshal func(v interface{}) ([]byte, error), openFile func(name string, flag int, perm os.FileMode) (*os.File, error), diff --git a/internal/engine/experiment_integration_test.go b/internal/engine/experiment_integration_test.go index 2114d6d..dd91b7f 100644 --- a/internal/engine/experiment_integration_test.go +++ b/internal/engine/experiment_integration_test.go @@ -162,7 +162,7 @@ func TestSetCallbacks(t *testing.T) { } register := ®isterCallbacksCalled{} builder.SetCallbacks(register) - if _, err := builder.NewExperiment().Measure(""); err != nil { + if _, err := builder.NewExperiment().MeasureWithContext(context.Background(), ""); err != nil { t.Fatal(err) } if register.onProgressCalled == false { @@ -206,7 +206,7 @@ func TestMeasurementFailure(t *testing.T) { if err := builder.SetOptionAny("ReturnError", true); err != nil { t.Fatal(err) } - measurement, err := builder.NewExperiment().Measure("") + measurement, err := builder.NewExperiment().MeasureWithContext(context.Background(), "") if err == nil { t.Fatal("expected an error here") } @@ -288,7 +288,7 @@ func TestUseOptions(t *testing.T) { if err := builder.SetOptionAny("Message", "antani"); err != nil { t.Fatal("cannot set Message field") } - config := builder.config.(*example.Config) + config := builder.(*experimentBuilder).config.(*example.Config) if config.ReturnError != true { t.Fatal("config.ReturnError was not changed") } @@ -313,15 +313,16 @@ func TestRunHHFM(t *testing.T) { runexperimentflow(t, builder.NewExperiment(), "") } -func runexperimentflow(t *testing.T, experiment *Experiment, input string) { - err := experiment.OpenReport() +func runexperimentflow(t *testing.T, experiment Experiment, input string) { + ctx := context.Background() + err := experiment.OpenReportContext(ctx) if err != nil { t.Fatal(err) } if experiment.ReportID() == "" { t.Fatal("reportID should not be empty here") } - measurement, err := experiment.Measure(input) + measurement, err := experiment.MeasureWithContext(ctx, input) if err != nil { t.Fatal(err) } @@ -335,7 +336,7 @@ func runexperimentflow(t *testing.T, experiment *Experiment, input string) { if data == nil { t.Fatal("data is nil") } - err = experiment.SubmitAndUpdateMeasurement(measurement) + err = experiment.SubmitAndUpdateMeasurementContext(ctx, measurement) if err != nil { t.Fatal(err) } @@ -364,14 +365,14 @@ func TestSaveMeasurementErrors(t *testing.T) { if err != nil { t.Fatal(err) } - exp := builder.NewExperiment() + exp := builder.NewExperiment().(*experiment) dirname, err := ioutil.TempDir("", "ooniprobe-engine-save-measurement") if err != nil { t.Fatal(err) } filename := filepath.Join(dirname, "report.jsonl") m := new(model.Measurement) - err = exp.SaveMeasurementEx( + err = exp.saveMeasurement( m, filename, func(v interface{}) ([]byte, error) { return nil, errors.New("mocked error") }, os.OpenFile, func(fp *os.File, b []byte) (int, error) { @@ -381,7 +382,7 @@ func TestSaveMeasurementErrors(t *testing.T) { if err == nil { t.Fatal("expected an error here") } - err = exp.SaveMeasurementEx( + err = exp.saveMeasurement( m, filename, json.Marshal, func(name string, flag int, perm os.FileMode) (*os.File, error) { return nil, errors.New("mocked error") @@ -392,7 +393,7 @@ func TestSaveMeasurementErrors(t *testing.T) { if err == nil { t.Fatal("expected an error here") } - err = exp.SaveMeasurementEx( + err = exp.saveMeasurement( m, filename, json.Marshal, os.OpenFile, func(fp *os.File, b []byte) (int, error) { return 0, errors.New("mocked error") @@ -417,10 +418,11 @@ func TestOpenReportIdempotent(t *testing.T) { if exp.ReportID() != "" { t.Fatal("unexpected initial report ID") } - if err := exp.SubmitAndUpdateMeasurement(&model.Measurement{}); err == nil { + ctx := context.Background() + if err := exp.SubmitAndUpdateMeasurementContext(ctx, &model.Measurement{}); err == nil { t.Fatal("we should not be able to submit before OpenReport") } - err = exp.OpenReport() + err = exp.OpenReportContext(ctx) if err != nil { t.Fatal(err) } @@ -428,7 +430,7 @@ func TestOpenReportIdempotent(t *testing.T) { if rid == "" { t.Fatal("invalid report ID") } - err = exp.OpenReport() + err = exp.OpenReportContext(ctx) if err != nil { t.Fatal(err) } @@ -453,12 +455,12 @@ func TestOpenReportFailure(t *testing.T) { if err != nil { t.Fatal(err) } - exp := builder.NewExperiment() + exp := builder.NewExperiment().(*experiment) exp.session.selectedProbeService = &model.OOAPIService{ Address: server.URL, Type: "https", } - err = exp.OpenReport() + err = exp.OpenReportContext(context.Background()) if !strings.HasPrefix(err.Error(), "httpx: request failed") { t.Fatal("not the error we expected") } @@ -474,12 +476,12 @@ func TestOpenReportNewClientFailure(t *testing.T) { if err != nil { t.Fatal(err) } - exp := builder.NewExperiment() + exp := builder.NewExperiment().(*experiment) exp.session.selectedProbeService = &model.OOAPIService{ Address: "antani:///", Type: "antani", } - err = exp.OpenReport() + err = exp.OpenReportContext(context.Background()) if err.Error() != "probe services: unsupported endpoint type" { t.Fatal(err) } @@ -497,7 +499,7 @@ func TestSubmitAndUpdateMeasurementWithClosedReport(t *testing.T) { } exp := builder.NewExperiment() m := new(model.Measurement) - err = exp.SubmitAndUpdateMeasurement(m) + err = exp.SubmitAndUpdateMeasurementContext(context.Background(), m) if err == nil { t.Fatal("expected an error here") } @@ -509,7 +511,7 @@ func TestMeasureLookupLocationFailure(t *testing.T) { } sess := newSessionForTestingNoLookups(t) defer sess.Close() - exp := NewExperiment(sess, new(antaniMeasurer)) + exp := newExperiment(sess, new(antaniMeasurer)) ctx, cancel := context.WithCancel(context.Background()) cancel() // so we fail immediately if _, err := exp.MeasureWithContext(ctx, "xx"); err == nil { @@ -529,8 +531,8 @@ func TestOpenReportNonHTTPS(t *testing.T) { Type: "mascetti", }, } - exp := NewExperiment(sess, new(antaniMeasurer)) - if err := exp.OpenReport(); err == nil { + exp := newExperiment(sess, new(antaniMeasurer)) + if err := exp.OpenReportContext(context.Background()); err == nil { t.Fatal("expected an error here") } } diff --git a/internal/engine/experiment_internal_test.go b/internal/engine/experiment_internal_test.go deleted file mode 100644 index 6adc11c..0000000 --- a/internal/engine/experiment_internal_test.go +++ /dev/null @@ -1,16 +0,0 @@ -package engine - -import ( - "os" - - "github.com/ooni/probe-cli/v3/internal/model" -) - -func (e *Experiment) SaveMeasurementEx( - measurement *model.Measurement, filePath string, - marshal func(v interface{}) ([]byte, error), - openFile func(name string, flag int, perm os.FileMode) (*os.File, error), - write func(fp *os.File, b []byte) (n int, err error), -) error { - return e.saveMeasurement(measurement, filePath, marshal, openFile, write) -} diff --git a/internal/engine/experiment_test.go b/internal/engine/experiment_test.go index fe126d7..7860d57 100644 --- a/internal/engine/experiment_test.go +++ b/internal/engine/experiment_test.go @@ -14,7 +14,7 @@ func TestExperimentHonoursSharingDefaults(t *testing.T) { if err != nil { t.Fatal(err) } - exp := builder.NewExperiment() + exp := builder.NewExperiment().(*experiment) return exp.newMeasurement("") } type spec struct { diff --git a/internal/engine/experimentbuilder.go b/internal/engine/experimentbuilder.go index 2b5f651..64dc7c4 100644 --- a/internal/engine/experimentbuilder.go +++ b/internal/engine/experimentbuilder.go @@ -1,5 +1,9 @@ package engine +// +// ExperimentBuilder definition and implementation +// + import ( "errors" "fmt" @@ -42,23 +46,60 @@ const ( InputOrStaticDefault = InputPolicy("or_static_default") ) -// ExperimentBuilder is an experiment builder. -type ExperimentBuilder struct { - build func(interface{}) *Experiment - callbacks model.ExperimentCallbacks - config interface{} - inputPolicy InputPolicy +// ExperimentBuilder builds an experiment. +type ExperimentBuilder interface { + // Interruptible tells you whether this is an interruptible experiment. This kind + // of experiments (e.g. ndt7) may be interrupted mid way. + Interruptible() bool + + // InputPolicy returns the experiment input policy. + InputPolicy() InputPolicy + + // Options returns information about the experiment's options. + Options() (map[string]OptionInfo, error) + + // SetOptionAny sets an option whose value is an any value. We will use reasonable + // heuristics to convert the any value to the proper type of the field whose name is + // contained by the key variable. If we cannot convert the provided any value to + // the proper type, then this function returns an error. + SetOptionAny(key string, value any) error + + // SetOptionsAny sets options from a map[string]any. See the documentation of + // the SetOptionAny method for more information. + SetOptionsAny(options map[string]any) error + + // SetCallbacks sets the experiment's interactive callbacks. + SetCallbacks(callbacks model.ExperimentCallbacks) + + // NewExperiment creates the experiment instance. + NewExperiment() Experiment +} + +// experimentBuilder implements ExperimentBuilder. +type experimentBuilder struct { + // build is the constructor that build an experiment with the given config. + build func(config interface{}) *experiment + + // callbacks contains callbacks for the new experiment. + callbacks model.ExperimentCallbacks + + // config contains the experiment's config. + config interface{} + + // inputPolicy contains the experiment's InputPolicy. + inputPolicy InputPolicy + + // interruptible indicates whether the experiment is interruptible. interruptible bool } -// Interruptible tells you whether this is an interruptible experiment. This kind -// of experiments (e.g. ndt7) may be interrupted mid way. -func (b *ExperimentBuilder) Interruptible() bool { +// Interruptible implements ExperimentBuilder.Interruptible. +func (b *experimentBuilder) Interruptible() bool { return b.interruptible } -// InputPolicy returns the experiment input policy -func (b *ExperimentBuilder) InputPolicy() InputPolicy { +// InputPolicy implements ExperimentBuilder.InputPolicy. +func (b *experimentBuilder) InputPolicy() InputPolicy { return b.inputPolicy } @@ -96,8 +137,8 @@ var ( ErrUnsupportedOptionType = errors.New("unsupported option type") ) -// Options returns info about all options -func (b *ExperimentBuilder) Options() (map[string]OptionInfo, error) { +// Options implements ExperimentBuilder.Options. +func (b *experimentBuilder) Options() (map[string]OptionInfo, error) { result := make(map[string]OptionInfo) ptrinfo := reflect.ValueOf(b.config) if ptrinfo.Kind() != reflect.Ptr { @@ -118,7 +159,7 @@ func (b *ExperimentBuilder) Options() (map[string]OptionInfo, error) { } // setOptionBool sets a bool option. -func (b *ExperimentBuilder) setOptionBool(field reflect.Value, value any) error { +func (b *experimentBuilder) setOptionBool(field reflect.Value, value any) error { switch v := value.(type) { case bool: field.SetBool(v) @@ -135,7 +176,7 @@ func (b *ExperimentBuilder) setOptionBool(field reflect.Value, value any) error } // setOptionInt sets an int option -func (b *ExperimentBuilder) setOptionInt(field reflect.Value, value any) error { +func (b *experimentBuilder) setOptionInt(field reflect.Value, value any) error { switch v := value.(type) { case int64: field.SetInt(v) @@ -165,7 +206,7 @@ func (b *ExperimentBuilder) setOptionInt(field reflect.Value, value any) error { } // setOptionString sets a string option -func (b *ExperimentBuilder) setOptionString(field reflect.Value, value any) error { +func (b *experimentBuilder) setOptionString(field reflect.Value, value any) error { switch v := value.(type) { case string: field.SetString(v) @@ -175,11 +216,8 @@ func (b *ExperimentBuilder) setOptionString(field reflect.Value, value any) erro } } -// SetOptionAny sets an option whose value is an any value. We will use reasonable -// heuristics to convert the any value to the proper type of the field whose name is -// contained by the key variable. If we cannot convert the provided any value to -// the proper type, then this function returns an error. -func (b *ExperimentBuilder) SetOptionAny(key string, value any) error { +// SetOptionAny implements ExperimentBuilder.SetOptionAny. +func (b *experimentBuilder) SetOptionAny(key string, value any) error { field, err := b.fieldbyname(b.config, key) if err != nil { return err @@ -196,9 +234,8 @@ func (b *ExperimentBuilder) SetOptionAny(key string, value any) error { } } -// SetOptionsAny sets options from a map[string]any. See the documentation of -// the SetOptionAny function for more information. -func (b *ExperimentBuilder) SetOptionsAny(options map[string]any) error { +// SetOptionsAny implements ExperimentBuilder.SetOptionsAny. +func (b *experimentBuilder) SetOptionsAny(options map[string]any) error { for key, value := range options { if err := b.SetOptionAny(key, value); err != nil { return err @@ -207,12 +244,13 @@ func (b *ExperimentBuilder) SetOptionsAny(options map[string]any) error { return nil } -// SetCallbacks sets the interactive callbacks -func (b *ExperimentBuilder) SetCallbacks(callbacks model.ExperimentCallbacks) { +// SetCallbacks implements ExperimentBuilder.SetCallbacks. +func (b *experimentBuilder) SetCallbacks(callbacks model.ExperimentCallbacks) { b.callbacks = callbacks } -func (b *ExperimentBuilder) fieldbyname(v interface{}, key string) (reflect.Value, error) { +// fieldbyname return v's field whose name is equal to the given key. +func (b *experimentBuilder) fieldbyname(v interface{}, key string) (reflect.Value, error) { // See https://stackoverflow.com/a/6396678/4354461 ptrinfo := reflect.ValueOf(v) if ptrinfo.Kind() != reflect.Ptr { @@ -230,7 +268,7 @@ func (b *ExperimentBuilder) fieldbyname(v interface{}, key string) (reflect.Valu } // NewExperiment creates the experiment -func (b *ExperimentBuilder) NewExperiment() *Experiment { +func (b *experimentBuilder) NewExperiment() Experiment { experiment := b.build(b.config) experiment.callbacks = b.callbacks return experiment @@ -255,7 +293,8 @@ func canonicalizeExperimentName(name string) string { return name } -func newExperimentBuilder(session *Session, name string) (*ExperimentBuilder, error) { +// newExperimentBuilder creates a new experimentBuilder instance. +func newExperimentBuilder(session *Session, name string) (*experimentBuilder, error) { factory := experimentsByName[canonicalizeExperimentName(name)] if factory == nil { return nil, fmt.Errorf("no such experiment: %s", name) diff --git a/internal/engine/experimentbuilder_test.go b/internal/engine/experimentbuilder_test.go index 39a439e..9197032 100644 --- a/internal/engine/experimentbuilder_test.go +++ b/internal/engine/experimentbuilder_test.go @@ -16,7 +16,7 @@ type fakeExperimentConfig struct { func TestExperimentBuilderOptions(t *testing.T) { t.Run("when config is not a pointer", func(t *testing.T) { - b := &ExperimentBuilder{ + b := &experimentBuilder{ config: 17, } options, err := b.Options() @@ -30,7 +30,7 @@ func TestExperimentBuilderOptions(t *testing.T) { t.Run("when config is not a struct", func(t *testing.T) { number := 17 - b := &ExperimentBuilder{ + b := &experimentBuilder{ config: &number, } options, err := b.Options() @@ -44,7 +44,7 @@ func TestExperimentBuilderOptions(t *testing.T) { t.Run("when config is a pointer to struct", func(t *testing.T) { config := &fakeExperimentConfig{} - b := &ExperimentBuilder{ + b := &experimentBuilder{ config: config, } options, err := b.Options() @@ -291,7 +291,7 @@ func TestExperimentBuilderSetOptionAny(t *testing.T) { for _, input := range inputs { t.Run(input.TestCaseName, func(t *testing.T) { ec := input.InitialConfig - b := &ExperimentBuilder{config: ec} + b := &experimentBuilder{config: ec} err := b.SetOptionAny(input.FieldName, input.FieldValue) if !errors.Is(err, input.ExpectErr) { t.Fatal(err) @@ -304,7 +304,7 @@ func TestExperimentBuilderSetOptionAny(t *testing.T) { } func TestExperimentBuilderSetOptionsAny(t *testing.T) { - b := &ExperimentBuilder{config: &fakeExperimentConfig{}} + b := &experimentBuilder{config: &fakeExperimentConfig{}} t.Run("we correctly handle an empty map", func(t *testing.T) { if err := b.SetOptionsAny(nil); err != nil { @@ -314,7 +314,7 @@ func TestExperimentBuilderSetOptionsAny(t *testing.T) { t.Run("we correctly handle a map containing options", func(t *testing.T) { f := &fakeExperimentConfig{} - privateb := &ExperimentBuilder{config: f} + privateb := &experimentBuilder{config: f} opts := map[string]any{ "String": "yoloyolo", "Value": "174", diff --git a/internal/engine/session.go b/internal/engine/session.go index ee7aa70..5bdc969 100644 --- a/internal/engine/session.go +++ b/internal/engine/session.go @@ -390,7 +390,7 @@ var ErrAlreadyUsingProxy = errors.New( // NewExperimentBuilder returns a new experiment builder // for the experiment with the given name, or an error if // there's no such experiment with the given name -func (s *Session) NewExperimentBuilder(name string) (*ExperimentBuilder, error) { +func (s *Session) NewExperimentBuilder(name string) (ExperimentBuilder, error) { return newExperimentBuilder(s, name) } diff --git a/pkg/oonimkall/experiment.go b/pkg/oonimkall/experiment.go index 117a3d0..4f7077c 100644 --- a/pkg/oonimkall/experiment.go +++ b/pkg/oonimkall/experiment.go @@ -67,7 +67,7 @@ type experimentBuilder interface { // experimentBuilderWrapper wraps *ExperimentBuilder type experimentBuilderWrapper struct { - eb *engine.ExperimentBuilder + eb engine.ExperimentBuilder } // newExperiment implements experimentBuilder.newExperiment diff --git a/pkg/oonimkall/tasksession.go b/pkg/oonimkall/tasksession.go index 1a785d9..86d2e08 100644 --- a/pkg/oonimkall/tasksession.go +++ b/pkg/oonimkall/tasksession.go @@ -59,7 +59,7 @@ func (sess *taskSessionEngine) NewExperimentBuilderByName( // taskExperimentBuilderEngine wraps ./internal/engine's // ExperimentBuilder type. type taskExperimentBuilderEngine struct { - *engine.ExperimentBuilder + engine.ExperimentBuilder } var _ taskExperimentBuilder = &taskExperimentBuilderEngine{} @@ -72,7 +72,7 @@ func (b *taskExperimentBuilderEngine) NewExperimentInstance() taskExperiment { // taskExperimentEngine wraps ./internal/engine's Experiment. type taskExperimentEngine struct { - *engine.Experiment + engine.Experiment } var _ taskExperiment = &taskExperimentEngine{}