diff --git a/internal/engine/savemeasurement.go b/internal/engine/savemeasurement.go new file mode 100644 index 0000000000..4cf71e78c6 --- /dev/null +++ b/internal/engine/savemeasurement.go @@ -0,0 +1,39 @@ +package engine + +import ( + "encoding/json" + "os" + + "github.com/ooni/probe-cli/v3/internal/model" +) + +// SaveMeasurement saves a measurement on the specified file path. +func SaveMeasurement(measurement *model.Measurement, filePath string) error { + return saveMeasurement( + measurement, filePath, json.Marshal, os.OpenFile, + func(fp *os.File, b []byte) (int, error) { + return fp.Write(b) + }, + ) +} + +func saveMeasurement( + 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 { + data, err := marshal(measurement) + if err != nil { + return err + } + data = append(data, byte('\n')) + filep, err := openFile(filePath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0600) + if err != nil { + return err + } + if _, err := write(filep, data); err != nil { + return err + } + return filep.Close() +} diff --git a/internal/engine/saver_test.go b/internal/engine/savemeasurement_test.go similarity index 56% rename from internal/engine/saver_test.go rename to internal/engine/savemeasurement_test.go index 18f655eaa8..fe17d30c6d 100644 --- a/internal/engine/saver_test.go +++ b/internal/engine/savemeasurement_test.go @@ -7,7 +7,6 @@ import ( "path/filepath" "testing" - "github.com/apex/log" "github.com/google/go-cmp/cmp" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/must" @@ -15,72 +14,6 @@ import ( "github.com/ooni/probe-cli/v3/internal/testingx" ) -func TestNewSaverDisabled(t *testing.T) { - saver, err := NewSaver(SaverConfig{ - Enabled: false, - }) - if err != nil { - t.Fatal(err) - } - if _, ok := saver.(fakeSaver); !ok { - t.Fatal("not the type of Saver we expected") - } - m := new(model.Measurement) - if err := saver.SaveMeasurement(m); err != nil { - t.Fatal(err) - } -} - -func TestNewSaverWithEmptyFilePath(t *testing.T) { - saver, err := NewSaver(SaverConfig{ - Enabled: true, - FilePath: "", - }) - if err == nil || err.Error() != "saver: passed an empty filepath" { - t.Fatalf("not the error we expected: %+v", err) - } - if saver != nil { - t.Fatal("saver should be nil here") - } -} - -func TestNewSaverWithFailureWhenSaving(t *testing.T) { - filep := runtimex.Try1(os.CreateTemp("", "")) - filename := filep.Name() - filep.Close() - expected := errors.New("mocked error") - saver, err := NewSaver(SaverConfig{ - Enabled: true, - FilePath: filename, - Logger: log.Log, - }) - if err != nil { - t.Fatal(err) - } - realSaver, ok := saver.(*realSaver) - if !ok { - t.Fatal("not the type of saver we expected") - } - var ( - gotMeasurement *model.Measurement - gotFilePath string - ) - realSaver.savefunc = func(measurement *model.Measurement, filePath string) error { - gotMeasurement, gotFilePath = measurement, filePath - return expected - } - m := &model.Measurement{Input: "www.kernel.org"} - if err := saver.SaveMeasurement(m); !errors.Is(err, expected) { - t.Fatalf("not the error we expected: %+v", err) - } - if diff := cmp.Diff(m, gotMeasurement); diff != "" { - t.Fatal(diff) - } - if gotFilePath != filename { - t.Fatal("passed invalid filepath") - } -} - func TestSaveMeasurementSuccess(t *testing.T) { // get temporary file where to write the measurement filep, err := os.CreateTemp("", "") diff --git a/internal/engine/saver.go b/internal/engine/saver.go deleted file mode 100644 index bc5c9e4c5e..0000000000 --- a/internal/engine/saver.go +++ /dev/null @@ -1,92 +0,0 @@ -package engine - -import ( - "encoding/json" - "errors" - "os" - - "github.com/ooni/probe-cli/v3/internal/model" -) - -// Saver is an alias for model.Saver. -type Saver = model.Saver - -// SaverConfig is the configuration for creating a new Saver. -type SaverConfig struct { - // Enabled is true if saving is enabled. - Enabled bool - - // FilePath is the filepath where to append the measurement as a - // serialized JSON followed by a newline character. - FilePath string - - // Logger is the logger used by the saver. - Logger model.Logger -} - -// NewSaver creates a new instance of Saver. -func NewSaver(config SaverConfig) (Saver, error) { - if !config.Enabled { - return fakeSaver{}, nil - } - if config.FilePath == "" { - return nil, errors.New("saver: passed an empty filepath") - } - return &realSaver{ - FilePath: config.FilePath, - Logger: config.Logger, - savefunc: SaveMeasurement, - }, nil -} - -type fakeSaver struct{} - -func (fs fakeSaver) SaveMeasurement(m *model.Measurement) error { - return nil -} - -var _ Saver = fakeSaver{} - -type realSaver struct { - FilePath string - Logger model.Logger - savefunc func(measurement *model.Measurement, filePath string) error -} - -func (rs *realSaver) SaveMeasurement(m *model.Measurement) error { - rs.Logger.Info("saving measurement to disk") - return rs.savefunc(m, rs.FilePath) -} - -var _ Saver = &realSaver{} - -// SaveMeasurement saves a measurement on the specified file path. -func SaveMeasurement(measurement *model.Measurement, filePath string) error { - return saveMeasurement( - measurement, filePath, json.Marshal, os.OpenFile, - func(fp *os.File, b []byte) (int, error) { - return fp.Write(b) - }, - ) -} - -func saveMeasurement( - 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 { - data, err := marshal(measurement) - if err != nil { - return err - } - data = append(data, byte('\n')) - filep, err := openFile(filePath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0600) - if err != nil { - return err - } - if _, err := write(filep, data); err != nil { - return err - } - return filep.Close() -} diff --git a/internal/engine/session.go b/internal/engine/session.go index 491c1e95f6..855191f95a 100644 --- a/internal/engine/session.go +++ b/internal/engine/session.go @@ -420,7 +420,7 @@ func (s *Session) newProbeServicesClient(ctx context.Context) (*probeservices.Cl } // NewSubmitter creates a new submitter instance. -func (s *Session) NewSubmitter(ctx context.Context) (Submitter, error) { +func (s *Session) NewSubmitter(ctx context.Context) (model.Submitter, error) { psc, err := s.newProbeServicesClient(ctx) if err != nil { return nil, err diff --git a/internal/oonirun/experiment.go b/internal/oonirun/experiment.go index 0227002eea..f597cbb53e 100644 --- a/internal/oonirun/experiment.go +++ b/internal/oonirun/experiment.go @@ -64,14 +64,14 @@ type Experiment struct { newInputLoaderFn func(inputPolicy model.InputPolicy) inputLoader // newSubmitterFn is OPTIONAL and used for testing. - newSubmitterFn func(ctx context.Context) (engine.Submitter, error) + newSubmitterFn func(ctx context.Context) (model.Submitter, error) // newSaverFn is OPTIONAL and used for testing. - newSaverFn func(experiment model.Experiment) (engine.Saver, error) + newSaverFn func(experiment model.Experiment) (model.Saver, error) // newInputProcessorFn is OPTIONAL and used for testing. newInputProcessorFn func(experiment model.Experiment, inputList []model.OOAPIURLInfo, - saver engine.Saver, submitter engine.Submitter) inputProcessor + saver model.Saver, submitter model.Submitter) inputProcessor } // Run runs the given experiment. @@ -138,34 +138,34 @@ type inputProcessor = model.ExperimentInputProcessor // newInputProcessor creates a new inputProcessor instance. func (ed *Experiment) newInputProcessor(experiment model.Experiment, - inputList []model.OOAPIURLInfo, saver engine.Saver, submitter engine.Submitter) inputProcessor { + inputList []model.OOAPIURLInfo, saver model.Saver, submitter model.Submitter) inputProcessor { if ed.newInputProcessorFn != nil { return ed.newInputProcessorFn(experiment, inputList, saver, submitter) } - return &engine.InputProcessor{ + return &InputProcessor{ Annotations: ed.Annotations, Experiment: &experimentWrapper{ - child: engine.NewInputProcessorExperimentWrapper(experiment), + child: NewInputProcessorExperimentWrapper(experiment), logger: ed.Session.Logger(), total: len(inputList), }, Inputs: inputList, MaxRuntime: time.Duration(ed.MaxRuntime) * time.Second, Options: experimentOptionsToStringList(ed.ExtraOptions), - Saver: engine.NewInputProcessorSaverWrapper(saver), + Saver: NewInputProcessorSaverWrapper(saver), Submitter: &experimentSubmitterWrapper{ - child: engine.NewInputProcessorSubmitterWrapper(submitter), + child: NewInputProcessorSubmitterWrapper(submitter), logger: ed.Session.Logger(), }, } } // newSaver creates a new engine.Saver instance. -func (ed *Experiment) newSaver(experiment model.Experiment) (engine.Saver, error) { +func (ed *Experiment) newSaver(experiment model.Experiment) (model.Saver, error) { if ed.newSaverFn != nil { return ed.newSaverFn(experiment) } - return engine.NewSaver(engine.SaverConfig{ + return NewSaver(SaverConfig{ Enabled: !ed.NoJSON, FilePath: ed.ReportFile, Logger: ed.Session.Logger(), @@ -173,11 +173,11 @@ func (ed *Experiment) newSaver(experiment model.Experiment) (engine.Saver, error } // newSubmitter creates a new engine.Submitter instance. -func (ed *Experiment) newSubmitter(ctx context.Context) (engine.Submitter, error) { +func (ed *Experiment) newSubmitter(ctx context.Context) (model.Submitter, error) { if ed.newSubmitterFn != nil { return ed.newSubmitterFn(ctx) } - return engine.NewSubmitter(ctx, engine.SubmitterConfig{ + return NewSubmitter(ctx, SubmitterConfig{ Enabled: !ed.NoCollector, Session: ed.Session, Logger: ed.Session.Logger(), @@ -233,7 +233,7 @@ func experimentOptionsToStringList(options map[string]any) (out []string) { // experimentWrapper wraps an experiment and logs progress type experimentWrapper struct { // child is the child experiment wrapper - child engine.InputProcessorExperimentWrapper + child InputProcessorExperimentWrapper // logger is the logger to use logger model.Logger @@ -254,7 +254,7 @@ func (ew *experimentWrapper) MeasureAsync( // fail if we cannot submit a measurement type experimentSubmitterWrapper struct { // child is the child submitter wrapper - child engine.InputProcessorSubmitterWrapper + child InputProcessorSubmitterWrapper // logger is the logger to use logger model.Logger diff --git a/internal/oonirun/experiment_test.go b/internal/oonirun/experiment_test.go index 3c4b35389b..2a3455e3ab 100644 --- a/internal/oonirun/experiment_test.go +++ b/internal/oonirun/experiment_test.go @@ -8,7 +8,6 @@ import ( "testing" "time" - "github.com/ooni/probe-cli/v3/internal/engine" "github.com/ooni/probe-cli/v3/internal/mocks" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/testingx" @@ -81,7 +80,7 @@ func TestExperimentRunWithFailureToSubmitAndShuffle(t *testing.T) { }, newExperimentBuilderFn: nil, newInputLoaderFn: nil, - newSubmitterFn: func(ctx context.Context) (engine.Submitter, error) { + newSubmitterFn: func(ctx context.Context) (model.Submitter, error) { subm := &mocks.Submitter{ MockSubmit: func(ctx context.Context, m *model.Measurement) error { failedToSubmit++ @@ -171,9 +170,9 @@ func TestExperimentRun(t *testing.T) { Session Session newExperimentBuilderFn func(experimentName string) (model.ExperimentBuilder, error) newInputLoaderFn func(inputPolicy model.InputPolicy) inputLoader - newSubmitterFn func(ctx context.Context) (engine.Submitter, error) - newSaverFn func(experiment model.Experiment) (engine.Saver, error) - newInputProcessorFn func(experiment model.Experiment, inputList []model.OOAPIURLInfo, saver engine.Saver, submitter engine.Submitter) inputProcessor + newSubmitterFn func(ctx context.Context) (model.Submitter, error) + newSaverFn func(experiment model.Experiment) (model.Saver, error) + newInputProcessorFn func(experiment model.Experiment, inputList []model.OOAPIURLInfo, saver model.Saver, submitter model.Submitter) inputProcessor } type args struct { ctx context.Context @@ -274,7 +273,7 @@ func TestExperimentRun(t *testing.T) { }, } }, - newSubmitterFn: func(ctx context.Context) (engine.Submitter, error) { + newSubmitterFn: func(ctx context.Context) (model.Submitter, error) { return nil, errMocked }, }, @@ -317,10 +316,10 @@ func TestExperimentRun(t *testing.T) { }, } }, - newSubmitterFn: func(ctx context.Context) (engine.Submitter, error) { + newSubmitterFn: func(ctx context.Context) (model.Submitter, error) { return &mocks.Submitter{}, nil }, - newSaverFn: func(experiment model.Experiment) (engine.Saver, error) { + newSaverFn: func(experiment model.Experiment) (model.Saver, error) { return nil, errMocked }, }, @@ -363,14 +362,14 @@ func TestExperimentRun(t *testing.T) { }, } }, - newSubmitterFn: func(ctx context.Context) (engine.Submitter, error) { + newSubmitterFn: func(ctx context.Context) (model.Submitter, error) { return &mocks.Submitter{}, nil }, - newSaverFn: func(experiment model.Experiment) (engine.Saver, error) { + newSaverFn: func(experiment model.Experiment) (model.Saver, error) { return &mocks.Saver{}, nil }, newInputProcessorFn: func(experiment model.Experiment, inputList []model.OOAPIURLInfo, - saver engine.Saver, submitter engine.Submitter) inputProcessor { + saver model.Saver, submitter model.Submitter) inputProcessor { return &mocks.ExperimentInputProcessor{ MockRun: func(ctx context.Context) error { return errMocked diff --git a/internal/engine/inputprocessor.go b/internal/oonirun/inputprocessor.go similarity index 97% rename from internal/engine/inputprocessor.go rename to internal/oonirun/inputprocessor.go index ed33b3e9b3..c764d61df9 100644 --- a/internal/engine/inputprocessor.go +++ b/internal/oonirun/inputprocessor.go @@ -1,4 +1,4 @@ -package engine +package oonirun import ( "context" @@ -76,11 +76,11 @@ type InputProcessorSaverWrapper interface { } type inputProcessorSaverWrapper struct { - saver Saver + saver model.Saver } // NewInputProcessorSaverWrapper wraps a Saver for InputProcessor. -func NewInputProcessorSaverWrapper(saver Saver) InputProcessorSaverWrapper { +func NewInputProcessorSaverWrapper(saver model.Saver) InputProcessorSaverWrapper { return inputProcessorSaverWrapper{saver: saver} } diff --git a/internal/engine/inputprocessor_test.go b/internal/oonirun/inputprocessor_test.go similarity index 99% rename from internal/engine/inputprocessor_test.go rename to internal/oonirun/inputprocessor_test.go index ec1912a352..2aa57b9e17 100644 --- a/internal/engine/inputprocessor_test.go +++ b/internal/oonirun/inputprocessor_test.go @@ -1,4 +1,4 @@ -package engine +package oonirun import ( "context" diff --git a/internal/oonirun/saver.go b/internal/oonirun/saver.go new file mode 100644 index 0000000000..3f75392b29 --- /dev/null +++ b/internal/oonirun/saver.go @@ -0,0 +1,57 @@ +package oonirun + +import ( + "errors" + + "github.com/ooni/probe-cli/v3/internal/engine" + "github.com/ooni/probe-cli/v3/internal/model" +) + +// SaverConfig is the configuration for creating a new Saver. +type SaverConfig struct { + // Enabled is true if saving is enabled. + Enabled bool + + // FilePath is the filepath where to append the measurement as a + // serialized JSON followed by a newline character. + FilePath string + + // Logger is the logger used by the saver. + Logger model.Logger +} + +// NewSaver creates a new instance of Saver. +func NewSaver(config SaverConfig) (model.Saver, error) { + if !config.Enabled { + return fakeSaver{}, nil + } + if config.FilePath == "" { + return nil, errors.New("saver: passed an empty filepath") + } + return &realSaver{ + FilePath: config.FilePath, + Logger: config.Logger, + savefunc: engine.SaveMeasurement, + }, nil +} + +type fakeSaver struct{} + +func (fs fakeSaver) SaveMeasurement(m *model.Measurement) error { + return nil +} + +var _ model.Saver = fakeSaver{} + +type realSaver struct { + FilePath string + Logger model.Logger + savefunc func(measurement *model.Measurement, filePath string) error +} + +func (rs *realSaver) SaveMeasurement(m *model.Measurement) error { + rs.Logger.Info("saving measurement to disk") + return rs.savefunc(m, rs.FilePath) +} + +var _ model.Saver = &realSaver{} diff --git a/internal/oonirun/saver_test.go b/internal/oonirun/saver_test.go new file mode 100644 index 0000000000..0ceafccbea --- /dev/null +++ b/internal/oonirun/saver_test.go @@ -0,0 +1,78 @@ +package oonirun + +import ( + "errors" + "os" + "testing" + + "github.com/apex/log" + "github.com/google/go-cmp/cmp" + "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/runtimex" +) + +func TestNewSaverDisabled(t *testing.T) { + saver, err := NewSaver(SaverConfig{ + Enabled: false, + }) + if err != nil { + t.Fatal(err) + } + if _, ok := saver.(fakeSaver); !ok { + t.Fatal("not the type of Saver we expected") + } + m := new(model.Measurement) + if err := saver.SaveMeasurement(m); err != nil { + t.Fatal(err) + } +} + +func TestNewSaverWithEmptyFilePath(t *testing.T) { + saver, err := NewSaver(SaverConfig{ + Enabled: true, + FilePath: "", + }) + if err == nil || err.Error() != "saver: passed an empty filepath" { + t.Fatalf("not the error we expected: %+v", err) + } + if saver != nil { + t.Fatal("saver should be nil here") + } +} + +func TestNewSaverWithFailureWhenSaving(t *testing.T) { + filep := runtimex.Try1(os.CreateTemp("", "")) + filename := filep.Name() + filep.Close() + expected := errors.New("mocked error") + saver, err := NewSaver(SaverConfig{ + Enabled: true, + FilePath: filename, + Logger: log.Log, + }) + if err != nil { + t.Fatal(err) + } + realSaver, ok := saver.(*realSaver) + if !ok { + t.Fatal("not the type of saver we expected") + } + var ( + gotMeasurement *model.Measurement + gotFilePath string + ) + realSaver.savefunc = func(measurement *model.Measurement, filePath string) error { + gotMeasurement, gotFilePath = measurement, filePath + return expected + } + m := &model.Measurement{Input: "www.kernel.org"} + if err := saver.SaveMeasurement(m); !errors.Is(err, expected) { + t.Fatalf("not the error we expected: %+v", err) + } + if diff := cmp.Diff(m, gotMeasurement); diff != "" { + t.Fatal(diff) + } + if gotFilePath != filename { + t.Fatal("passed invalid filepath") + } +} diff --git a/internal/oonirun/session.go b/internal/oonirun/session.go index 6075c8db78..64566dbeca 100644 --- a/internal/oonirun/session.go +++ b/internal/oonirun/session.go @@ -20,7 +20,7 @@ type Session interface { engine.InputLoaderSession // A Session is also a SubmitterSession. - engine.SubmitterSession + SubmitterSession // DefaultHTTPClient returns the session's default HTTPClient. DefaultHTTPClient() model.HTTPClient diff --git a/internal/engine/submitter.go b/internal/oonirun/submitter.go similarity index 99% rename from internal/engine/submitter.go rename to internal/oonirun/submitter.go index 4c3991614b..ee0ddc7565 100644 --- a/internal/engine/submitter.go +++ b/internal/oonirun/submitter.go @@ -1,4 +1,4 @@ -package engine +package oonirun import ( "context" diff --git a/internal/engine/submitter_test.go b/internal/oonirun/submitter_test.go similarity index 99% rename from internal/engine/submitter_test.go rename to internal/oonirun/submitter_test.go index 1b7903338f..e1217fa207 100644 --- a/internal/engine/submitter_test.go +++ b/internal/oonirun/submitter_test.go @@ -1,4 +1,4 @@ -package engine +package oonirun import ( "context"