diff --git a/cmd/ooniprobe/internal/nettests/nettests.go b/cmd/ooniprobe/internal/nettests/nettests.go index bee5be39f7..ecea88c078 100644 --- a/cmd/ooniprobe/internal/nettests/nettests.go +++ b/cmd/ooniprobe/internal/nettests/nettests.go @@ -240,18 +240,9 @@ func (c *Controller) Run(builder model.ExperimentBuilder, inputs []string) error return errors.Wrap(err, "failed to mark measurement as done") } - // We're not sure whether it's enough to log the error or we should - // instead also mark the measurement as failed. Strictly speaking this - // is an inconsistency between the code that generate the measurement - // and the code that process the measurement. We do have some data - // but we're not gonna have a summary. To be reconsidered. - tk, err := exp.GetSummaryKeys(measurement) - if err != nil { - log.WithError(err).Error("failed to obtain testKeys") - continue - } + sk := engine.MeasurementSummaryKeys(measurement) log.Debugf("Fetching: %d %v", idx, c.msmts[idx64]) - if err := db.AddTestKeys(c.msmts[idx64], tk); err != nil { + if err := db.AddTestKeys(c.msmts[idx64], sk); err != nil { return errors.Wrap(err, "failed to add test keys to summary") } } diff --git a/internal/database/actions.go b/internal/database/actions.go index ac6bfa390d..984d25904a 100644 --- a/internal/database/actions.go +++ b/internal/database/actions.go @@ -8,10 +8,10 @@ import ( "net/url" "os" "path/filepath" - "reflect" "time" "github.com/apex/log" + "github.com/ooni/probe-cli/v3/internal/engine" "github.com/ooni/probe-cli/v3/internal/model" "github.com/pkg/errors" "github.com/upper/db/v4" @@ -349,27 +349,25 @@ func (d *Database) CreateOrUpdateURL(urlStr string, categoryCode string, country return url.ID.Int64, nil } -// AddTestKeys implements WritableDatabase.AddTestKeys -func (d *Database) AddTestKeys(msmt *model.DatabaseMeasurement, tk any) error { - var ( - isAnomaly bool - isAnomalyValid bool - ) - tkBytes, err := json.Marshal(tk) +func updateDatabaseMeasurementWithSummaryKeys(msmt *model.DatabaseMeasurement, sk model.MeasurementSummaryKeys) error { + skBytes, err := json.Marshal(sk) if err != nil { log.WithError(err).Error("failed to serialize summary") + return err } - // This is necessary so that we can extract from the the opaque testKeys just - // the IsAnomaly field of bool type. - // Maybe generics are not so bad after-all, heh golang? - isAnomalyValue := reflect.ValueOf(tk).FieldByName("IsAnomaly") - if isAnomalyValue.IsValid() && isAnomalyValue.Kind() == reflect.Bool { - isAnomaly = isAnomalyValue.Bool() - isAnomalyValid = true + msmt.TestKeys = string(skBytes) + _, isNotImplemented := sk.(*engine.ExperimentMeasurementSummaryKeysNotImplemented) + msmt.IsAnomaly = sql.NullBool{Bool: sk.Anomaly(), Valid: !isNotImplemented} + return nil +} + +// AddTestKeys implements WritableDatabase.AddTestKeys +func (d *Database) AddTestKeys(msmt *model.DatabaseMeasurement, sk model.MeasurementSummaryKeys) error { + if err := updateDatabaseMeasurementWithSummaryKeys(msmt, sk); err != nil { + // error message already printed + return err } - msmt.TestKeys = string(tkBytes) - msmt.IsAnomaly = sql.NullBool{Bool: isAnomaly, Valid: isAnomalyValid} - err = d.sess.Collection("measurements").Find("measurement_id", msmt.ID).Update(msmt) + err := d.sess.Collection("measurements").Find("measurement_id", msmt.ID).Update(msmt) if err != nil { log.WithError(err).Error("failed to update measurement") return errors.Wrap(err, "updating measurement") diff --git a/internal/database/actions_test.go b/internal/database/actions_test.go index 8e9ae7ac48..be9b2612ef 100644 --- a/internal/database/actions_test.go +++ b/internal/database/actions_test.go @@ -8,7 +8,12 @@ import ( "os" "testing" + "github.com/google/go-cmp/cmp" + "github.com/ooni/probe-cli/v3/internal/engine" + "github.com/ooni/probe-cli/v3/internal/experiment/signal" "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/must" + "github.com/ooni/probe-cli/v3/internal/testingx" "github.com/upper/db/v4" ) @@ -427,3 +432,95 @@ func TestGetMeasurementJSON(t *testing.T) { t.Error("inconsistent measurement downloaded") } } + +// chansummary is used to test that we handle summary serialization errors. +type chansummary chan int + +var _ model.MeasurementSummaryKeys = make(chansummary) + +// Anomaly implements model.MeasurementSummaryKeys. +func (chansummary) Anomaly() bool { + return false +} + +func TestUpdateDatabaseMeasurementWithSummaryKeys(t *testing.T) { + t.Run("we update the .TestKeys field", func(t *testing.T) { + meas := &model.DatabaseMeasurement{} + sk := &signal.SummaryKeys{} + ffiller := &testingx.FakeFiller{} + ffiller.Fill(sk) + + if err := updateDatabaseMeasurementWithSummaryKeys(meas, sk); err != nil { + t.Fatal(err) + } + + if len(meas.TestKeys) <= 0 { + t.Fatal("no meas.TestKeys") + } + + var got signal.SummaryKeys + must.UnmarshalJSON([]byte(meas.TestKeys), &got) + if diff := cmp.Diff(sk, &got); diff != "" { + t.Fatal(diff) + } + }) + + t.Run("we set .Anomaly.Bool to true and .Anomaly.Valid to true when needed", func(t *testing.T) { + meas := &model.DatabaseMeasurement{} + sk := &signal.SummaryKeys{ + IsAnomaly: true, + } + + if err := updateDatabaseMeasurementWithSummaryKeys(meas, sk); err != nil { + t.Fatal(err) + } + + if meas.IsAnomaly.Bool != sk.IsAnomaly { + t.Fatal("unexpected value") + } + if meas.IsAnomaly.Valid != true { + t.Fatal("unexpected value") + } + }) + + t.Run("we set .Anomaly.Bool to false and .Anomaly.Valid to true when needed", func(t *testing.T) { + meas := &model.DatabaseMeasurement{} + sk := &signal.SummaryKeys{ + IsAnomaly: false, + } + + if err := updateDatabaseMeasurementWithSummaryKeys(meas, sk); err != nil { + t.Fatal(err) + } + + if meas.IsAnomaly.Bool != sk.IsAnomaly { + t.Fatal("unexpected value") + } + if meas.IsAnomaly.Valid != true { + t.Fatal("unexpected value") + } + }) + + t.Run("we set .Anomaly.Valid to false when needed", func(t *testing.T) { + meas := &model.DatabaseMeasurement{} + sk := &engine.ExperimentMeasurementSummaryKeysNotImplemented{} + + if err := updateDatabaseMeasurementWithSummaryKeys(meas, sk); err != nil { + t.Fatal(err) + } + + if meas.IsAnomaly.Valid != false { + t.Fatal("unexpected value") + } + }) + + t.Run("we handle the case where we cannot serialize the summary", func(t *testing.T) { + meas := &model.DatabaseMeasurement{} + sk := make(chansummary) + + err := updateDatabaseMeasurementWithSummaryKeys(meas, sk) + if err == nil || err.Error() != "json: unsupported type: database.chansummary" { + t.Fatal("unexpected error", err) + } + }) +} diff --git a/internal/engine/experiment.go b/internal/engine/experiment.go index 27dbb90c76..a9a4abff18 100644 --- a/internal/engine/experiment.go +++ b/internal/engine/experiment.go @@ -60,9 +60,23 @@ func (e *experiment) Name() string { return e.testName } -// GetSummaryKeys implements Experiment.GetSummaryKeys. -func (e *experiment) GetSummaryKeys(m *model.Measurement) (interface{}, error) { - return e.measurer.GetSummaryKeys(m) +// ExperimentMeasurementSummaryKeysNotImplemented is the [model.MeasurementSummary] we use when +// the experiment TestKeys do not provide an implementation of [model.MeasurementSummary]. +type ExperimentMeasurementSummaryKeysNotImplemented struct{} + +var _ model.MeasurementSummaryKeys = &ExperimentMeasurementSummaryKeysNotImplemented{} + +// IsAnomaly implements MeasurementSummary. +func (*ExperimentMeasurementSummaryKeysNotImplemented) Anomaly() bool { + return false +} + +// MeasurementSummaryKeys returns the [model.MeasurementSummaryKeys] associated with a given measurement. +func MeasurementSummaryKeys(m *model.Measurement) model.MeasurementSummaryKeys { + if tk, ok := m.TestKeys.(model.MeasurementSummaryKeysProvider); ok { + return tk.MeasurementSummaryKeys() + } + return &ExperimentMeasurementSummaryKeysNotImplemented{} } // ReportID implements Experiment.ReportID. diff --git a/internal/engine/experiment_integration_test.go b/internal/engine/experiment_integration_test.go index 81aeb28e09..a1578f8591 100644 --- a/internal/engine/experiment_integration_test.go +++ b/internal/engine/experiment_integration_test.go @@ -285,8 +285,9 @@ func runexperimentflow(t *testing.T, experiment model.Experiment, input string) if experiment.KibiBytesReceived() <= 0 { t.Fatal("no data received?!") } - if _, err := experiment.GetSummaryKeys(measurement); err != nil { - t.Fatal(err) + sk := MeasurementSummaryKeys(measurement) + if sk == nil { + t.Fatal("got nil summary keys") } } @@ -485,9 +486,3 @@ func (am *antaniMeasurer) ExperimentVersion() string { func (am *antaniMeasurer) Run(ctx context.Context, args *model.ExperimentArgs) error { return nil } - -func (am *antaniMeasurer) GetSummaryKeys(m *model.Measurement) (interface{}, error) { - return struct { - Failure *string `json:"failure"` - }{}, nil -} diff --git a/internal/engine/experiment_test.go b/internal/engine/experiment_test.go index e8f5112d3b..e2444306af 100644 --- a/internal/engine/experiment_test.go +++ b/internal/engine/experiment_test.go @@ -4,6 +4,8 @@ import ( "testing" "github.com/ooni/probe-cli/v3/internal/enginelocate" + "github.com/ooni/probe-cli/v3/internal/experiment/example" + "github.com/ooni/probe-cli/v3/internal/experiment/signal" "github.com/ooni/probe-cli/v3/internal/model" ) @@ -73,3 +75,33 @@ func TestExperimentHonoursSharingDefaults(t *testing.T) { }) } } + +func TestExperimentMeasurementSummaryKeysNotImplemented(t *testing.T) { + t.Run("the .Anomaly method returns false", func(t *testing.T) { + sk := &ExperimentMeasurementSummaryKeysNotImplemented{} + if sk.Anomaly() != false { + t.Fatal("expected false") + } + }) +} + +func TestExperimentMeasurementSummaryKeys(t *testing.T) { + t.Run("when the TestKeys implement MeasurementSummaryKeysProvider", func(t *testing.T) { + tk := &signal.TestKeys{} + meas := &model.Measurement{TestKeys: tk} + sk := MeasurementSummaryKeys(meas) + if _, good := sk.(*signal.SummaryKeys); !good { + t.Fatal("not the expected type") + } + }) + + t.Run("otherwise", func(t *testing.T) { + // note: example does not implement SummaryKeys + tk := &example.TestKeys{} + meas := &model.Measurement{TestKeys: tk} + sk := MeasurementSummaryKeys(meas) + if _, good := sk.(*ExperimentMeasurementSummaryKeysNotImplemented); !good { + t.Fatal("not the expected type") + } + }) +} diff --git a/internal/experiment/dash/measurer.go b/internal/experiment/dash/measurer.go index 9174cb79c2..e8a19cc9e8 100644 --- a/internal/experiment/dash/measurer.go +++ b/internal/experiment/dash/measurer.go @@ -6,7 +6,6 @@ package dash import ( "context" - "errors" "net/http" "github.com/ooni/probe-cli/v3/internal/legacy/netx" @@ -125,10 +124,9 @@ func NewExperimentMeasurer(config Config) model.ExperimentMeasurer { return &Measurer{config: config} } +var _ model.MeasurementSummaryKeysProvider = &TestKeys{} + // SummaryKeys contains summary keys for this experiment. -// -// Note that this structure is part of the ABI contract with ooniprobe -// therefore we should be careful when changing it. type SummaryKeys struct { Latency float64 `json:"connect_latency"` Bitrate float64 `json:"median_bitrate"` @@ -136,15 +134,16 @@ type SummaryKeys struct { IsAnomaly bool `json:"-"` } -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (any, error) { - sk := SummaryKeys{IsAnomaly: false} - tk, ok := measurement.TestKeys.(*TestKeys) - if !ok { - return sk, errors.New("invalid test keys type") - } +// MeasurementSummaryKeys implements model.MeasurementSummaryKeysProvider. +func (tk *TestKeys) MeasurementSummaryKeys() model.MeasurementSummaryKeys { + sk := &SummaryKeys{IsAnomaly: false} sk.Latency = tk.Simple.ConnectLatency sk.Bitrate = float64(tk.Simple.MedianBitrate) sk.Delay = tk.Simple.MinPlayoutDelay - return sk, nil + return sk +} + +// Anomaly implements model.MeasurementSummary. +func (sk *SummaryKeys) Anomaly() bool { + return sk.IsAnomaly } diff --git a/internal/experiment/dash/measurer_test.go b/internal/experiment/dash/measurer_test.go index c553d9cfa4..f36450bcad 100644 --- a/internal/experiment/dash/measurer_test.go +++ b/internal/experiment/dash/measurer_test.go @@ -105,22 +105,6 @@ func TestMeasureWithCancelledContext(t *testing.T) { if !errors.Is(err, nil) { t.Fatal("unexpected error value") } - sk, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - if _, ok := sk.(SummaryKeys); !ok { - t.Fatal("invalid type for summary keys") - } -} - -func TestSummaryKeysInvalidType(t *testing.T) { - measurement := new(model.Measurement) - m := &Measurer{} - _, err := m.GetSummaryKeys(measurement) - if err.Error() != "invalid test keys type" { - t.Fatal("not the error we expected") - } } func TestSummaryKeysGood(t *testing.T) { @@ -129,12 +113,8 @@ func TestSummaryKeysGood(t *testing.T) { MedianBitrate: 123, MinPlayoutDelay: 12, }}} - m := &Measurer{} - osk, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - sk := osk.(SummaryKeys) + osk := measurement.TestKeys.(*TestKeys).MeasurementSummaryKeys() + sk := osk.(*SummaryKeys) if sk.Latency != 1234 { t.Fatal("invalid latency") } @@ -147,4 +127,7 @@ func TestSummaryKeysGood(t *testing.T) { if sk.IsAnomaly { t.Fatal("invalid isAnomaly") } + if sk.Anomaly() != sk.IsAnomaly { + t.Fatal("sk.Anomaly() does not return sk.IsAnomaly's value") + } } diff --git a/internal/experiment/dnscheck/dnscheck.go b/internal/experiment/dnscheck/dnscheck.go index abbe4ee33f..4c9d5fd17c 100644 --- a/internal/experiment/dnscheck/dnscheck.go +++ b/internal/experiment/dnscheck/dnscheck.go @@ -317,16 +317,3 @@ func NewExperimentMeasurer(config Config) model.ExperimentMeasurer { Endpoints: nil, // disabled by default } } - -// SummaryKeys contains summary keys for this experiment. -// -// Note that this structure is part of the ABI contract with ooniprobe -// therefore we should be careful when changing it. -type SummaryKeys struct { - IsAnomaly bool `json:"-"` -} - -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m *Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - return SummaryKeys{IsAnomaly: false}, nil -} diff --git a/internal/experiment/dnscheck/dnscheck_test.go b/internal/experiment/dnscheck/dnscheck_test.go index a7fb9b336b..aeb0e8eee2 100644 --- a/internal/experiment/dnscheck/dnscheck_test.go +++ b/internal/experiment/dnscheck/dnscheck_test.go @@ -109,13 +109,6 @@ func TestWithCancelledContext(t *testing.T) { if err != nil { t.Fatal(err) } - sk, err := measurer.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - if _, ok := sk.(SummaryKeys); !ok { - t.Fatal("invalid type for summary keys") - } } func TestMakeResolverURL(t *testing.T) { @@ -179,19 +172,6 @@ func newsession() model.ExperimentSession { return &mockable.Session{MockableLogger: log.Log} } -func TestSummaryKeysGeneric(t *testing.T) { - measurement := &model.Measurement{TestKeys: &TestKeys{}} - m := &Measurer{} - osk, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - sk := osk.(SummaryKeys) - if sk.IsAnomaly { - t.Fatal("invalid isAnomaly") - } -} - func TestDNSCheckWait(t *testing.T) { if testing.Short() { t.Skip("skip test in short mode") diff --git a/internal/experiment/dnsping/dnsping.go b/internal/experiment/dnsping/dnsping.go index eb976cf90b..06851667ce 100644 --- a/internal/experiment/dnsping/dnsping.go +++ b/internal/experiment/dnsping/dnsping.go @@ -226,16 +226,3 @@ func stopOperationLogger(ol stoppableOperationLogger, addrs []string, err error) func NewExperimentMeasurer(config Config) model.ExperimentMeasurer { return &Measurer{config: config} } - -// SummaryKeys contains summary keys for this experiment. -// -// Note that this structure is part of the ABI contract with ooniprobe -// therefore we should be careful when changing it. -type SummaryKeys struct { - IsAnomaly bool `json:"-"` -} - -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - return SummaryKeys{IsAnomaly: false}, nil -} diff --git a/internal/experiment/dnsping/dnsping_test.go b/internal/experiment/dnsping/dnsping_test.go index 56c7efedbd..4320b94a05 100644 --- a/internal/experiment/dnsping/dnsping_test.go +++ b/internal/experiment/dnsping/dnsping_test.go @@ -112,7 +112,7 @@ func TestMeasurer_run(t *testing.T) { ) env.Do(func() { - meas, m, err := runHelper("udp://8.8.8.8:53") + meas, _, err := runHelper("udp://8.8.8.8:53") if err != nil { t.Fatalf("Unexpected error: %s", err) } @@ -122,15 +122,6 @@ func TestMeasurer_run(t *testing.T) { t.Fatal("unexpected number of pings", len(tk.Pings)) } - ask, err := m.GetSummaryKeys(meas) - if err != nil { - t.Fatal("cannot obtain summary") - } - summary := ask.(SummaryKeys) - if summary.IsAnomaly { - t.Fatal("expected no anomaly") - } - for _, p := range tk.Pings { if p.Query == nil { t.Fatal("QUery should not be nil") @@ -170,7 +161,7 @@ func TestMeasurer_run(t *testing.T) { }) env.Do(func() { - meas, m, err := runHelper("udp://8.8.8.8:53") + meas, _, err := runHelper("udp://8.8.8.8:53") if err != nil { t.Fatalf("Unexpected error: %s", err) } @@ -180,17 +171,6 @@ func TestMeasurer_run(t *testing.T) { t.Fatal("unexpected number of pings", len(tk.Pings)) } - // note: this experiment does not set anomaly but we still want - // to have a test here for when we possibly will - ask, err := m.GetSummaryKeys(meas) - if err != nil { - t.Fatal("cannot obtain summary") - } - summary := ask.(SummaryKeys) - if summary.IsAnomaly { - t.Fatal("expected no anomaly") - } - for _, p := range tk.Pings { if p.Query == nil { t.Fatal("QUery should not be nil") diff --git a/internal/experiment/echcheck/measure.go b/internal/experiment/echcheck/measure.go index c33cd9b2dc..807a43d969 100644 --- a/internal/experiment/echcheck/measure.go +++ b/internal/experiment/echcheck/measure.go @@ -133,13 +133,3 @@ func (m *Measurer) Run( func NewExperimentMeasurer(config Config) model.ExperimentMeasurer { return &Measurer{config: config} } - -// SummaryKeys contains summary keys for this experiment. -type SummaryKeys struct { - IsAnomaly bool `json:"-"` -} - -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m *Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - return SummaryKeys{IsAnomaly: false}, nil -} diff --git a/internal/experiment/echcheck/measure_test.go b/internal/experiment/echcheck/measure_test.go index 2ba09707e4..2a53fbd938 100644 --- a/internal/experiment/echcheck/measure_test.go +++ b/internal/experiment/echcheck/measure_test.go @@ -113,13 +113,6 @@ func TestMeasurementSuccessRealWorld(t *testing.T) { } // check results - summary, err := measurer.GetSummaryKeys(&model.Measurement{}) - if err != nil { - t.Fatal("unexpected error: ", err) - } - if summary.(SummaryKeys).IsAnomaly != false { - t.Fatal("expected false") - } tk := msrmnt.TestKeys.(TestKeys) if tk.Control.Failure != nil { t.Fatal("unexpected control failure:", *tk.Control.Failure) diff --git a/internal/experiment/example/example.go b/internal/experiment/example/example.go index c2c1c01b7a..83ffbe90d4 100644 --- a/internal/experiment/example/example.go +++ b/internal/experiment/example/example.go @@ -84,16 +84,3 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { func NewExperimentMeasurer(config Config, testName string) model.ExperimentMeasurer { return Measurer{config: config, testName: testName} } - -// SummaryKeys contains summary keys for this experiment. -// -// Note that this structure is part of the ABI contract with ooniprobe -// therefore we should be careful when changing it. -type SummaryKeys struct { - IsAnomaly bool `json:"-"` -} - -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - return SummaryKeys{IsAnomaly: false}, nil -} diff --git a/internal/experiment/example/example_test.go b/internal/experiment/example/example_test.go index 6a9918347a..b4a66aafc1 100644 --- a/internal/experiment/example/example_test.go +++ b/internal/experiment/example/example_test.go @@ -35,13 +35,6 @@ func TestSuccess(t *testing.T) { if err != nil { t.Fatal(err) } - sk, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - if _, ok := sk.(example.SummaryKeys); !ok { - t.Fatal("invalid type for summary keys") - } } func TestFailure(t *testing.T) { @@ -62,16 +55,3 @@ func TestFailure(t *testing.T) { t.Fatal("expected an error here") } } - -func TestSummaryKeysGeneric(t *testing.T) { - measurement := &model.Measurement{TestKeys: &example.TestKeys{}} - m := &example.Measurer{} - osk, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - sk := osk.(example.SummaryKeys) - if sk.IsAnomaly { - t.Fatal("invalid isAnomaly") - } -} diff --git a/internal/experiment/fbmessenger/fbmessenger.go b/internal/experiment/fbmessenger/fbmessenger.go index d239ba925f..e039cfd000 100644 --- a/internal/experiment/fbmessenger/fbmessenger.go +++ b/internal/experiment/fbmessenger/fbmessenger.go @@ -5,7 +5,6 @@ package fbmessenger import ( "context" - "errors" "math/rand" "time" @@ -208,27 +207,27 @@ func NewExperimentMeasurer(config Config) model.ExperimentMeasurer { return Measurer{Config: config} } +var _ model.MeasurementSummaryKeysProvider = &TestKeys{} + // SummaryKeys contains summary keys for this experiment. -// -// Note that this structure is part of the ABI contract with ooniprobe -// therefore we should be careful when changing it. type SummaryKeys struct { DNSBlocking bool `json:"facebook_dns_blocking"` TCPBlocking bool `json:"facebook_tcp_blocking"` IsAnomaly bool `json:"-"` } -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - sk := SummaryKeys{IsAnomaly: false} - tk, ok := measurement.TestKeys.(*TestKeys) - if !ok { - return sk, errors.New("invalid test keys type") - } +// MeasurementSummaryKeys implements model.MeasurementSummaryKeysProvider. +func (tk *TestKeys) MeasurementSummaryKeys() model.MeasurementSummaryKeys { + sk := &SummaryKeys{IsAnomaly: false} dnsBlocking := tk.FacebookDNSBlocking != nil && *tk.FacebookDNSBlocking tcpBlocking := tk.FacebookTCPBlocking != nil && *tk.FacebookTCPBlocking sk.DNSBlocking = dnsBlocking sk.TCPBlocking = tcpBlocking sk.IsAnomaly = dnsBlocking || tcpBlocking - return sk, nil + return sk +} + +// Anomaly implements model.MeasurementSummary. +func (sk *SummaryKeys) Anomaly() bool { + return sk.IsAnomaly } diff --git a/internal/experiment/fbmessenger/fbmessenger_test.go b/internal/experiment/fbmessenger/fbmessenger_test.go index 5454923b35..f5f21efdb0 100644 --- a/internal/experiment/fbmessenger/fbmessenger_test.go +++ b/internal/experiment/fbmessenger/fbmessenger_test.go @@ -180,13 +180,6 @@ func TestMeasurerRun(t *testing.T) { if diff := cmp.Diff(expectAnalysis, tk.Analysis); diff != "" { t.Fatal(diff) } - sk, err := measurer.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - if _, ok := sk.(fbmessenger.SummaryKeys); !ok { - t.Fatal("invalid type for summary keys") - } }) }) @@ -393,23 +386,10 @@ func TestComputeEndpointStatsDNSIsLying(t *testing.T) { } } -func TestSummaryKeysInvalidType(t *testing.T) { - measurement := new(model.Measurement) - m := &fbmessenger.Measurer{} - _, err := m.GetSummaryKeys(measurement) - if err.Error() != "invalid test keys type" { - t.Fatal("not the error we expected") - } -} - func TestSummaryKeysWithNils(t *testing.T) { measurement := &model.Measurement{TestKeys: &fbmessenger.TestKeys{}} - m := &fbmessenger.Measurer{} - osk, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - sk := osk.(fbmessenger.SummaryKeys) + osk := measurement.TestKeys.(*fbmessenger.TestKeys).MeasurementSummaryKeys() + sk := osk.(*fbmessenger.SummaryKeys) if sk.DNSBlocking { t.Fatal("invalid dnsBlocking") } @@ -419,6 +399,9 @@ func TestSummaryKeysWithNils(t *testing.T) { if sk.IsAnomaly { t.Fatal("invalid isAnomaly") } + if sk.Anomaly() != sk.IsAnomaly { + t.Fatal("the values should match") + } } func TestSummaryKeysWithFalseFalse(t *testing.T) { @@ -428,12 +411,8 @@ func TestSummaryKeysWithFalseFalse(t *testing.T) { FacebookDNSBlocking: &falseValue, }, }} - m := &fbmessenger.Measurer{} - osk, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - sk := osk.(fbmessenger.SummaryKeys) + osk := measurement.TestKeys.(*fbmessenger.TestKeys).MeasurementSummaryKeys() + sk := osk.(*fbmessenger.SummaryKeys) if sk.DNSBlocking { t.Fatal("invalid dnsBlocking") } @@ -443,6 +422,9 @@ func TestSummaryKeysWithFalseFalse(t *testing.T) { if sk.IsAnomaly { t.Fatal("invalid isAnomaly") } + if sk.Anomaly() != sk.IsAnomaly { + t.Fatal("the values should match") + } } func TestSummaryKeysWithFalseTrue(t *testing.T) { @@ -452,12 +434,8 @@ func TestSummaryKeysWithFalseTrue(t *testing.T) { FacebookDNSBlocking: &trueValue, }, }} - m := &fbmessenger.Measurer{} - osk, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - sk := osk.(fbmessenger.SummaryKeys) + osk := measurement.TestKeys.(*fbmessenger.TestKeys).MeasurementSummaryKeys() + sk := osk.(*fbmessenger.SummaryKeys) if sk.DNSBlocking == false { t.Fatal("invalid dnsBlocking") } @@ -467,6 +445,9 @@ func TestSummaryKeysWithFalseTrue(t *testing.T) { if sk.IsAnomaly == false { t.Fatal("invalid isAnomaly") } + if sk.Anomaly() != sk.IsAnomaly { + t.Fatal("the values should match") + } } func TestSummaryKeysWithTrueFalse(t *testing.T) { @@ -476,12 +457,8 @@ func TestSummaryKeysWithTrueFalse(t *testing.T) { FacebookDNSBlocking: &falseValue, }, }} - m := &fbmessenger.Measurer{} - osk, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - sk := osk.(fbmessenger.SummaryKeys) + osk := measurement.TestKeys.(*fbmessenger.TestKeys).MeasurementSummaryKeys() + sk := osk.(*fbmessenger.SummaryKeys) if sk.DNSBlocking { t.Fatal("invalid dnsBlocking") } @@ -491,6 +468,9 @@ func TestSummaryKeysWithTrueFalse(t *testing.T) { if sk.IsAnomaly == false { t.Fatal("invalid isAnomaly") } + if sk.Anomaly() != sk.IsAnomaly { + t.Fatal("the values should match") + } } func TestSummaryKeysWithTrueTrue(t *testing.T) { @@ -500,12 +480,8 @@ func TestSummaryKeysWithTrueTrue(t *testing.T) { FacebookDNSBlocking: &trueValue, }, }} - m := &fbmessenger.Measurer{} - osk, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - sk := osk.(fbmessenger.SummaryKeys) + osk := measurement.TestKeys.(*fbmessenger.TestKeys).MeasurementSummaryKeys() + sk := osk.(*fbmessenger.SummaryKeys) if sk.DNSBlocking == false { t.Fatal("invalid dnsBlocking") } @@ -515,4 +491,7 @@ func TestSummaryKeysWithTrueTrue(t *testing.T) { if sk.IsAnomaly == false { t.Fatal("invalid isAnomaly") } + if sk.Anomaly() != sk.IsAnomaly { + t.Fatal("the values should match") + } } diff --git a/internal/experiment/hhfm/hhfm.go b/internal/experiment/hhfm/hhfm.go index a392497987..716115c444 100644 --- a/internal/experiment/hhfm/hhfm.go +++ b/internal/experiment/hhfm/hhfm.go @@ -340,26 +340,26 @@ func (c Conn) Write(b []byte) (int, error) { return c.Conn.Write(b) } +var _ model.MeasurementSummaryKeysProvider = &TestKeys{} + // SummaryKeys contains summary keys for this experiment. -// -// Note that this structure is part of the ABI contract with ooniprobe -// therefore we should be careful when changing it. type SummaryKeys struct { IsAnomaly bool `json:"-"` } -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - sk := SummaryKeys{IsAnomaly: false} - tk, ok := measurement.TestKeys.(*TestKeys) - if !ok { - return sk, errors.New("invalid test keys type") - } +// MeasurementSummaryKeys implements model.MeasurementSummaryKeysProvider. +func (tk *TestKeys) MeasurementSummaryKeys() model.MeasurementSummaryKeys { + sk := &SummaryKeys{IsAnomaly: false} sk.IsAnomaly = (tk.Tampering.HeaderFieldName || tk.Tampering.HeaderFieldNumber || tk.Tampering.HeaderFieldValue || tk.Tampering.HeaderNameCapitalization || tk.Tampering.RequestLineCapitalization || tk.Tampering.Total) - return sk, nil + return sk +} + +// Anomaly implements model.MeasurementSummaryKeys. +func (sk *SummaryKeys) Anomaly() bool { + return sk.IsAnomaly } diff --git a/internal/experiment/hhfm/hhfm_test.go b/internal/experiment/hhfm/hhfm_test.go index eb420066d1..9589c8f66a 100644 --- a/internal/experiment/hhfm/hhfm_test.go +++ b/internal/experiment/hhfm/hhfm_test.go @@ -258,13 +258,6 @@ func TestCancelledContext(t *testing.T) { if tk.Tampering.Total != true { t.Fatal("invalid Tampering.Total") } - sk, err := measurer.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - if _, ok := sk.(hhfm.SummaryKeys); !ok { - t.Fatal("invalid type for summary keys") - } } func TestNoHelpers(t *testing.T) { @@ -863,15 +856,6 @@ func TestDialerDialContext(t *testing.T) { } } -func TestSummaryKeysInvalidType(t *testing.T) { - measurement := new(model.Measurement) - m := &hhfm.Measurer{} - _, err := m.GetSummaryKeys(measurement) - if err.Error() != "invalid test keys type" { - t.Fatal("not the error we expected") - } -} - func TestSummaryKeysWorksAsIntended(t *testing.T) { tests := []struct { tampering hhfm.Tampering @@ -900,19 +884,17 @@ func TestSummaryKeysWorksAsIntended(t *testing.T) { }} for idx, tt := range tests { t.Run(fmt.Sprintf("%d", idx), func(t *testing.T) { - m := &hhfm.Measurer{} measurement := &model.Measurement{TestKeys: &hhfm.TestKeys{ Tampering: tt.tampering, }} - got, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - return - } - sk := got.(hhfm.SummaryKeys) + got := measurement.TestKeys.(*hhfm.TestKeys).MeasurementSummaryKeys() + sk := got.(*hhfm.SummaryKeys) if sk.IsAnomaly != tt.isAnomaly { t.Fatal("unexpected isAnomaly value") } + if sk.IsAnomaly != sk.Anomaly() { + t.Fatal("unexpected Anomaly() value") + } }) } } diff --git a/internal/experiment/hirl/hirl.go b/internal/experiment/hirl/hirl.go index d7b41a3eb9..bcf573f872 100644 --- a/internal/experiment/hirl/hirl.go +++ b/internal/experiment/hirl/hirl.go @@ -305,21 +305,21 @@ func RunMethod(ctx context.Context, config RunMethodConfig) { } } +var _ model.MeasurementSummaryKeysProvider = &TestKeys{} + // SummaryKeys contains summary keys for this experiment. -// -// Note that this structure is part of the ABI contract with ooniprobe -// therefore we should be careful when changing it. type SummaryKeys struct { IsAnomaly bool `json:"-"` } -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - sk := SummaryKeys{IsAnomaly: false} - tk, ok := measurement.TestKeys.(*TestKeys) - if !ok { - return sk, errors.New("invalid test keys type") - } +// MeasurementSummaryKeys implements model.MeasurementSummaryKeysProvider. +func (tk *TestKeys) MeasurementSummaryKeys() model.MeasurementSummaryKeys { + sk := &SummaryKeys{IsAnomaly: false} sk.IsAnomaly = tk.Tampering - return sk, nil + return sk +} + +// Anomaly implements model.MeasurementSummaryKeys. +func (sk *SummaryKeys) Anomaly() bool { + return sk.IsAnomaly } diff --git a/internal/experiment/hirl/hirl_test.go b/internal/experiment/hirl/hirl_test.go index 4a46f28c18..ec36e68205 100644 --- a/internal/experiment/hirl/hirl_test.go +++ b/internal/experiment/hirl/hirl_test.go @@ -141,13 +141,6 @@ func TestCancelledContext(t *testing.T) { if tk.Tampering != false { t.Fatal("overall there is tampering?!") } - sk, err := measurer.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - if _, ok := sk.(hirl.SummaryKeys); !ok { - t.Fatal("invalid type for summary keys") - } } type FakeMethodSuccessful struct{} @@ -602,41 +595,30 @@ func TestRunMethodReadEOFWithWrongData(t *testing.T) { } } -func TestSummaryKeysInvalidType(t *testing.T) { - measurement := new(model.Measurement) - m := &hirl.Measurer{} - _, err := m.GetSummaryKeys(measurement) - if err.Error() != "invalid test keys type" { - t.Fatal("not the error we expected") - } -} - func TestSummaryKeysFalse(t *testing.T) { measurement := &model.Measurement{TestKeys: &hirl.TestKeys{ Tampering: false, }} - m := &hirl.Measurer{} - osk, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - sk := osk.(hirl.SummaryKeys) + osk := measurement.TestKeys.(*hirl.TestKeys).MeasurementSummaryKeys() + sk := osk.(*hirl.SummaryKeys) if sk.IsAnomaly { t.Fatal("invalid isAnomaly") } + if sk.Anomaly() != sk.IsAnomaly { + t.Fatal("invalid Anomaly()") + } } func TestSummaryKeysTrue(t *testing.T) { measurement := &model.Measurement{TestKeys: &hirl.TestKeys{ Tampering: true, }} - m := &hirl.Measurer{} - osk, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - sk := osk.(hirl.SummaryKeys) + osk := measurement.TestKeys.(*hirl.TestKeys).MeasurementSummaryKeys() + sk := osk.(*hirl.SummaryKeys) if sk.IsAnomaly == false { t.Fatal("invalid isAnomaly") } + if sk.Anomaly() != sk.IsAnomaly { + t.Fatal("invalid Anomaly()") + } } diff --git a/internal/experiment/httphostheader/httphostheader.go b/internal/experiment/httphostheader/httphostheader.go index 7ac1fe2073..9f401ed19f 100644 --- a/internal/experiment/httphostheader/httphostheader.go +++ b/internal/experiment/httphostheader/httphostheader.go @@ -77,16 +77,3 @@ func (m *Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { func NewExperimentMeasurer(config Config) model.ExperimentMeasurer { return &Measurer{config: config} } - -// SummaryKeys contains summary keys for this experiment. -// -// Note that this structure is part of the ABI contract with ooniprobe -// therefore we should be careful when changing it. -type SummaryKeys struct { - IsAnomaly bool `json:"-"` -} - -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - return SummaryKeys{IsAnomaly: false}, nil -} diff --git a/internal/experiment/httphostheader/httphostheader_test.go b/internal/experiment/httphostheader/httphostheader_test.go index dd7a6c0365..82b7ce2a4a 100644 --- a/internal/experiment/httphostheader/httphostheader_test.go +++ b/internal/experiment/httphostheader/httphostheader_test.go @@ -53,13 +53,6 @@ func TestMeasurerMeasureNoTestHelper(t *testing.T) { if err != nil { t.Fatal(err) } - sk, err := measurer.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - if _, ok := sk.(SummaryKeys); !ok { - t.Fatal("invalid type for summary keys") - } } func TestRunnerHTTPSetHostHeader(t *testing.T) { @@ -92,16 +85,3 @@ func TestRunnerHTTPSetHostHeader(t *testing.T) { func newsession() model.ExperimentSession { return &mockable.Session{MockableLogger: log.Log} } - -func TestSummaryKeysGeneric(t *testing.T) { - measurement := &model.Measurement{TestKeys: &TestKeys{}} - m := &Measurer{} - osk, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - sk := osk.(SummaryKeys) - if sk.IsAnomaly { - t.Fatal("invalid isAnomaly") - } -} diff --git a/internal/experiment/ndt7/ndt7.go b/internal/experiment/ndt7/ndt7.go index cf8d772966..878a0aec67 100644 --- a/internal/experiment/ndt7/ndt7.go +++ b/internal/experiment/ndt7/ndt7.go @@ -3,7 +3,6 @@ package ndt7 import ( "context" "encoding/json" - "errors" "fmt" "time" @@ -264,10 +263,9 @@ func failureFromError(err error) (failure *string) { return } +var _ model.MeasurementSummaryKeysProvider = &TestKeys{} + // SummaryKeys contains summary keys for this experiment. -// -// Note that this structure is part of the ABI contract with ooniprobe -// therefore we should be careful when changing it. type SummaryKeys struct { Upload float64 `json:"upload"` Download float64 `json:"download"` @@ -280,13 +278,9 @@ type SummaryKeys struct { IsAnomaly bool `json:"-"` } -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - sk := SummaryKeys{IsAnomaly: false} - tk, ok := measurement.TestKeys.(*TestKeys) - if !ok { - return sk, errors.New("invalid test keys type") - } +// MeasurementSummaryKeys implements model.MeasurementSummaryKeysProvider. +func (tk *TestKeys) MeasurementSummaryKeys() model.MeasurementSummaryKeys { + sk := &SummaryKeys{IsAnomaly: false} sk.Upload = tk.Summary.Upload sk.Download = tk.Summary.Download sk.Ping = tk.Summary.Ping @@ -295,5 +289,10 @@ func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, e sk.MinRTT = tk.Summary.MinRTT sk.MSS = float64(tk.Summary.MSS) sk.RetransmitRate = tk.Summary.RetransmitRate - return sk, nil + return sk +} + +// Anomaly implements model.MeasurementSummaryKeys. +func (sk *SummaryKeys) Anomaly() bool { + return sk.IsAnomaly } diff --git a/internal/experiment/ndt7/ndt7_test.go b/internal/experiment/ndt7/ndt7_test.go index 371a289570..2692a34cd5 100644 --- a/internal/experiment/ndt7/ndt7_test.go +++ b/internal/experiment/ndt7/ndt7_test.go @@ -121,13 +121,6 @@ func TestGood(t *testing.T) { if err != nil { t.Fatal(err) } - sk, err := measurer.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - if _, ok := sk.(SummaryKeys); !ok { - t.Fatal("invalid type for summary keys") - } } func TestFailDownload(t *testing.T) { @@ -227,15 +220,6 @@ func TestDownloadJSONUnmarshalFail(t *testing.T) { } } -func TestSummaryKeysInvalidType(t *testing.T) { - measurement := new(model.Measurement) - m := &Measurer{} - _, err := m.GetSummaryKeys(measurement) - if err.Error() != "invalid test keys type" { - t.Fatal("not the error we expected") - } -} - func TestSummaryKeysGood(t *testing.T) { measurement := &model.Measurement{TestKeys: &TestKeys{Summary: Summary{ RetransmitRate: 1, @@ -247,12 +231,8 @@ func TestSummaryKeysGood(t *testing.T) { Download: 7, Upload: 8, }}} - m := &Measurer{} - osk, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - sk := osk.(SummaryKeys) + osk := measurement.TestKeys.(*TestKeys).MeasurementSummaryKeys() + sk := osk.(*SummaryKeys) if sk.RetransmitRate != 1 { t.Fatal("invalid retransmitRate") } @@ -280,4 +260,7 @@ func TestSummaryKeysGood(t *testing.T) { if sk.IsAnomaly { t.Fatal("invalid isAnomaly") } + if sk.IsAnomaly != sk.Anomaly() { + t.Fatal("invalid Anomaly()") + } } diff --git a/internal/experiment/portfiltering/measurer_test.go b/internal/experiment/portfiltering/measurer_test.go index 81eeb69568..465cc9d0cd 100644 --- a/internal/experiment/portfiltering/measurer_test.go +++ b/internal/experiment/portfiltering/measurer_test.go @@ -42,12 +42,4 @@ func TestMeasurer_run(t *testing.T) { if len(tk.TCPConnect) != len(Ports) { t.Fatal("unexpected number of ports") } - ask, err := m.GetSummaryKeys(meas) - if err != nil { - t.Fatal("cannot obtain summary") - } - summary := ask.(SummaryKeys) - if summary.IsAnomaly { - t.Fatal("expected no anomaly") - } } diff --git a/internal/experiment/portfiltering/summary.go b/internal/experiment/portfiltering/summary.go deleted file mode 100644 index f4bb007949..0000000000 --- a/internal/experiment/portfiltering/summary.go +++ /dev/null @@ -1,20 +0,0 @@ -package portfiltering - -// -// Summary -// - -import "github.com/ooni/probe-cli/v3/internal/model" - -// SummaryKeys contains summary keys for this experiment. -// -// Note that this structure is part of the ABI contract with ooniprobe -// therefore we should be careful when changing it. -type SummaryKeys struct { - IsAnomaly bool `json:"-"` -} - -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - return SummaryKeys{IsAnomaly: false}, nil -} diff --git a/internal/experiment/psiphon/psiphon.go b/internal/experiment/psiphon/psiphon.go index a70150907b..3907b8b4a9 100644 --- a/internal/experiment/psiphon/psiphon.go +++ b/internal/experiment/psiphon/psiphon.go @@ -6,7 +6,6 @@ package psiphon import ( "context" - "errors" "sync" "time" @@ -107,27 +106,27 @@ func NewExperimentMeasurer(config Config) model.ExperimentMeasurer { return &Measurer{Config: config} } +var _ model.MeasurementSummaryKeysProvider = &TestKeys{} + // SummaryKeys contains summary keys for this experiment. -// -// Note that this structure is part of the ABI contract with ooniprobe -// therefore we should be careful when changing it. type SummaryKeys struct { BootstrapTime float64 `json:"bootstrap_time"` Failure string `json:"failure"` IsAnomaly bool `json:"-"` } -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - sk := SummaryKeys{IsAnomaly: false} - tk, ok := measurement.TestKeys.(*TestKeys) - if !ok { - return sk, errors.New("invalid test keys type") - } +// MeasurementSummaryKeys implements model.MeasurementSummaryKeysProvider. +func (tk *TestKeys) MeasurementSummaryKeys() model.MeasurementSummaryKeys { + sk := &SummaryKeys{IsAnomaly: false} if tk.Failure != nil { sk.Failure = *tk.Failure sk.IsAnomaly = true } sk.BootstrapTime = tk.BootstrapTime - return sk, nil + return sk +} + +// Anomaly implements model.MeasurementSummaryKeys. +func (sk *SummaryKeys) Anomaly() bool { + return sk.IsAnomaly } diff --git a/internal/experiment/psiphon/psiphon_test.go b/internal/experiment/psiphon/psiphon_test.go index 28f8e0d894..cec0360b7c 100644 --- a/internal/experiment/psiphon/psiphon_test.go +++ b/internal/experiment/psiphon/psiphon_test.go @@ -46,13 +46,6 @@ func TestRunWithCancelledContext(t *testing.T) { if tk.MaxRuntime <= 0 { t.Fatal("you did not set the max runtime") } - sk, err := measurer.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - if _, ok := sk.(psiphon.SummaryKeys); !ok { - t.Fatal("invalid type for summary keys") - } } func TestRunWithCustomInputAndCancelledContext(t *testing.T) { @@ -122,25 +115,12 @@ func newfakesession() model.ExperimentSession { return &mockable.Session{MockableLogger: log.Log} } -func TestSummaryKeysInvalidType(t *testing.T) { - measurement := new(model.Measurement) - m := &psiphon.Measurer{} - _, err := m.GetSummaryKeys(measurement) - if err.Error() != "invalid test keys type" { - t.Fatal("not the error we expected") - } -} - func TestSummaryKeysGood(t *testing.T) { measurement := &model.Measurement{TestKeys: &psiphon.TestKeys{TestKeys: urlgetter.TestKeys{ BootstrapTime: 123, }}} - m := &psiphon.Measurer{} - osk, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - sk := osk.(psiphon.SummaryKeys) + osk := measurement.TestKeys.(*psiphon.TestKeys).MeasurementSummaryKeys() + sk := osk.(*psiphon.SummaryKeys) if sk.BootstrapTime != 123 { t.Fatal("invalid latency") } @@ -158,12 +138,8 @@ func TestSummaryKeysFailure(t *testing.T) { BootstrapTime: 123, Failure: &expected, }}} - m := &psiphon.Measurer{} - osk, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - sk := osk.(psiphon.SummaryKeys) + osk := measurement.TestKeys.(*psiphon.TestKeys).MeasurementSummaryKeys() + sk := osk.(*psiphon.SummaryKeys) if sk.BootstrapTime != 123 { t.Fatal("invalid latency") } @@ -173,4 +149,7 @@ func TestSummaryKeysFailure(t *testing.T) { if sk.IsAnomaly == false { t.Fatal("invalid isAnomaly") } + if sk.IsAnomaly != sk.Anomaly() { + t.Fatal("invalid Anomaly()") + } } diff --git a/internal/experiment/quicping/quicping.go b/internal/experiment/quicping/quicping.go index afc33d2c62..893f86e30d 100644 --- a/internal/experiment/quicping/quicping.go +++ b/internal/experiment/quicping/quicping.go @@ -347,16 +347,3 @@ L: func NewExperimentMeasurer(config Config) model.ExperimentMeasurer { return &Measurer{config: config} } - -// SummaryKeys contains summary keys for this experiment. -// -// Note that this structure is part of the ABI contract with ooniprobe -// therefore we should be careful when changing it. -type SummaryKeys struct { - IsAnomaly bool `json:"-"` -} - -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m *Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - return SummaryKeys{IsAnomaly: false}, nil -} diff --git a/internal/experiment/quicping/quicping_test.go b/internal/experiment/quicping/quicping_test.go index c8705e65c0..37e72ff456 100644 --- a/internal/experiment/quicping/quicping_test.go +++ b/internal/experiment/quicping/quicping_test.go @@ -113,13 +113,6 @@ func TestSuccess(t *testing.T) { } } } - sk, err := measurer.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - if _, ok := sk.(SummaryKeys); !ok { - t.Fatal("invalid type for summary keys") - } } func TestWithCancelledContext(t *testing.T) { diff --git a/internal/experiment/riseupvpn/riseupvpn.go b/internal/experiment/riseupvpn/riseupvpn.go index f89ff58904..282959e9f5 100644 --- a/internal/experiment/riseupvpn/riseupvpn.go +++ b/internal/experiment/riseupvpn/riseupvpn.go @@ -293,17 +293,3 @@ func DecodeEIPServiceV3(body string) (*EIPServiceV3, error) { func NewExperimentMeasurer(config Config) model.ExperimentMeasurer { return Measurer{Config: config} } - -// SummaryKeys contains summary keys for this experiment. -// -// Note that this structure is part of the ABI contract with ooniprobe -// therefore we should be careful when changing it. -type SummaryKeys struct { - IsAnomaly bool `json:"-"` -} - -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - sk := SummaryKeys{IsAnomaly: false} - return sk, nil -} diff --git a/internal/experiment/riseupvpn/riseupvpn_test.go b/internal/experiment/riseupvpn/riseupvpn_test.go index 9370aeb75f..f52a0fe8ac 100644 --- a/internal/experiment/riseupvpn/riseupvpn_test.go +++ b/internal/experiment/riseupvpn/riseupvpn_test.go @@ -495,18 +495,6 @@ func TestFailureGateway1TransportNOK(t *testing.T) { } } -func TestSummaryKeysAlwaysReturnIsAnomalyFalse(t *testing.T) { - measurement := new(model.Measurement) - m := &riseupvpn.Measurer{} - result, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal("GetSummaryKeys should never return an error") - } - if result.(riseupvpn.SummaryKeys).IsAnomaly { - t.Fatal("GetSummaryKeys should never return IsAnomaly true") - } -} - func generateMockGetter(requestResponse map[string]string, responseStatus map[string]bool) urlgetter.MultiGetter { return func(ctx context.Context, g urlgetter.Getter) (urlgetter.TestKeys, error) { url := g.Target diff --git a/internal/experiment/signal/signal.go b/internal/experiment/signal/signal.go index 807d18403c..1933c65017 100644 --- a/internal/experiment/signal/signal.go +++ b/internal/experiment/signal/signal.go @@ -204,25 +204,25 @@ func NewExperimentMeasurer(config Config) model.ExperimentMeasurer { return Measurer{Config: config} } +var _ model.MeasurementSummaryKeysProvider = &TestKeys{} + // SummaryKeys contains summary keys for this experiment. -// -// Note that this structure is part of the ABI contract with ooniprobe -// therefore we should be careful when changing it. type SummaryKeys struct { SignalBackendStatus string `json:"signal_backend_status"` SignalBackendFailure *string `json:"signal_backend_failure"` IsAnomaly bool `json:"-"` } -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - sk := SummaryKeys{IsAnomaly: false} - tk, ok := measurement.TestKeys.(*TestKeys) - if !ok { - return nil, errors.New("invalid test keys type") - } +// MeasurementSummaryKeys implements model.MeasurementSummaryKeysProvider. +func (tk *TestKeys) MeasurementSummaryKeys() model.MeasurementSummaryKeys { + sk := &SummaryKeys{IsAnomaly: false} sk.SignalBackendStatus = tk.SignalBackendStatus sk.SignalBackendFailure = tk.SignalBackendFailure sk.IsAnomaly = tk.SignalBackendStatus == "blocked" - return sk, nil + return sk +} + +// Anomaly implements model.MeasurementSummaryKeys. +func (tk *SummaryKeys) Anomaly() bool { + return tk.IsAnomaly } diff --git a/internal/experiment/signal/signal_test.go b/internal/experiment/signal/signal_test.go index 4170b98d3c..30111dcf1c 100644 --- a/internal/experiment/signal/signal_test.go +++ b/internal/experiment/signal/signal_test.go @@ -72,13 +72,6 @@ func TestGood(t *testing.T) { if tk.SignalBackendStatus != "ok" { t.Fatal("unexpected SignalBackendStatus") } - sk, err := measurer.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - if _, ok := sk.(signal.SummaryKeys); !ok { - t.Fatal("invalid type for summary keys") - } } func TestUpdate(t *testing.T) { @@ -121,14 +114,22 @@ func TestBadSignalCA(t *testing.T) { } } -func TestGetSummaryInvalidType(t *testing.T) { - measurer := signal.Measurer{} - in := make(chan int) - out, err := measurer.GetSummaryKeys(&model.Measurement{TestKeys: in}) - if err == nil || err.Error() != "invalid test keys type" { - t.Fatal("not the error we expected", err) - } - if out != nil { - t.Fatal("expected nil output here") - } +func TestSummaryKeys(t *testing.T) { + t.Run("without anomaly", func(t *testing.T) { + sk := &signal.SummaryKeys{ + IsAnomaly: false, + } + if sk.IsAnomaly != sk.Anomaly() { + t.Fatal("invalid Anomaly()") + } + }) + + t.Run("with anomaly", func(t *testing.T) { + sk := &signal.SummaryKeys{ + IsAnomaly: true, + } + if sk.IsAnomaly != sk.Anomaly() { + t.Fatal("invalid Anomaly()") + } + }) } diff --git a/internal/experiment/simplequicping/simplequicping.go b/internal/experiment/simplequicping/simplequicping.go index 4869d5ec45..095d0c2c85 100644 --- a/internal/experiment/simplequicping/simplequicping.go +++ b/internal/experiment/simplequicping/simplequicping.go @@ -195,16 +195,3 @@ func (m *Measurer) quicHandshake(ctx context.Context, index int64, func NewExperimentMeasurer(config Config) model.ExperimentMeasurer { return &Measurer{config: config} } - -// SummaryKeys contains summary keys for this experiment. -// -// Note that this structure is part of the ABI contract with ooniprobe -// therefore we should be careful when changing it. -type SummaryKeys struct { - IsAnomaly bool `json:"-"` -} - -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - return SummaryKeys{IsAnomaly: false}, nil -} diff --git a/internal/experiment/simplequicping/simplequicping_test.go b/internal/experiment/simplequicping/simplequicping_test.go index 156b8e73e4..ffdc0764e8 100644 --- a/internal/experiment/simplequicping/simplequicping_test.go +++ b/internal/experiment/simplequicping/simplequicping_test.go @@ -116,20 +116,11 @@ func TestMeasurerRun(t *testing.T) { defer env.Close() env.Do(func() { - meas, m, err := runHelper("quichandshake://8.8.8.8:443") + meas, _, err := runHelper("quichandshake://8.8.8.8:443") if err != nil { t.Fatalf("Unexpected error: %s", err) } - ask, err := m.GetSummaryKeys(meas) - if err != nil { - t.Fatal("cannot obtain summary") - } - summary := ask.(SummaryKeys) - if summary.IsAnomaly { - t.Fatal("expected no anomaly") - } - tk, _ := (meas.TestKeys).(*TestKeys) if len(tk.Pings) != NPINGS { t.Fatal("unexpected number of pings") @@ -169,24 +160,13 @@ func TestMeasurerRun(t *testing.T) { }) env.Do(func() { - meas, m, err := runHelper("quichandshake://8.8.8.8:443") + meas, _, err := runHelper("quichandshake://8.8.8.8:443") if err != nil { t.Fatalf("Unexpected error: %s", err) } tk, _ := (meas.TestKeys).(*TestKeys) - // note: this experiment does not set anomaly but we still want - // to have a test here for when we possibly will - ask, err := m.GetSummaryKeys(meas) - if err != nil { - t.Fatal("cannot obtain summary") - } - summary := ask.(SummaryKeys) - if summary.IsAnomaly { - t.Fatal("expected no anomaly") - } - for _, p := range tk.Pings { if p.QUICHandshake.Failure == nil { t.Fatal("expected failure here but found nil") diff --git a/internal/experiment/sniblocking/sniblocking.go b/internal/experiment/sniblocking/sniblocking.go index 91a2c70017..1ba6ac28ee 100644 --- a/internal/experiment/sniblocking/sniblocking.go +++ b/internal/experiment/sniblocking/sniblocking.go @@ -289,16 +289,3 @@ func asString(failure *string) (result string) { } return } - -// SummaryKeys contains summary keys for this experiment. -// -// Note that this structure is part of the ABI contract with ooniprobe -// therefore we should be careful when changing it. -type SummaryKeys struct { - IsAnomaly bool `json:"-"` -} - -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m *Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - return SummaryKeys{IsAnomaly: false}, nil -} diff --git a/internal/experiment/sniblocking/sniblocking_test.go b/internal/experiment/sniblocking/sniblocking_test.go index cf8e614fee..0519ba0624 100644 --- a/internal/experiment/sniblocking/sniblocking_test.go +++ b/internal/experiment/sniblocking/sniblocking_test.go @@ -185,19 +185,6 @@ func TestMaybeURLToSNI(t *testing.T) { }) } -func TestSummaryKeysGeneric(t *testing.T) { - measurement := &model.Measurement{TestKeys: &TestKeys{}} - m := &Measurer{} - osk, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - sk := osk.(SummaryKeys) - if sk.IsAnomaly { - t.Fatal("invalid isAnomaly") - } -} - // exampleOrgAddr is the IP address used for example.org in netem-based nettests. const exampleOrgAddr = "93.184.216.34" @@ -379,13 +366,6 @@ func TestMeasurerRun(t *testing.T) { if tk.Result != classAnomalyUnexpectedFailure { t.Fatalf("Unexpected result, expected: %s, got: %s", classAnomalyUnexpectedFailure, tk.Result) } - sk, err := measurer.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - if _, ok := sk.(SummaryKeys); !ok { - t.Fatal("invalid type for summary keys") - } target := tk.Target if target.Agent != "" { t.Fatal("not the expected Agent") diff --git a/internal/experiment/stunreachability/stunreachability.go b/internal/experiment/stunreachability/stunreachability.go index 9e4e602a2f..9fe308e339 100644 --- a/internal/experiment/stunreachability/stunreachability.go +++ b/internal/experiment/stunreachability/stunreachability.go @@ -169,16 +169,3 @@ func (tk *TestKeys) do( func NewExperimentMeasurer(config Config) model.ExperimentMeasurer { return &Measurer{config: config} } - -// SummaryKeys contains summary keys for this experiment. -// -// Note that this structure is part of the ABI contract with ooniprobe -// therefore we should be careful when changing it. -type SummaryKeys struct { - IsAnomaly bool `json:"-"` -} - -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - return SummaryKeys{IsAnomaly: false}, nil -} diff --git a/internal/experiment/stunreachability/stunreachability_test.go b/internal/experiment/stunreachability/stunreachability_test.go index 8336584aaa..4abecbdfd6 100644 --- a/internal/experiment/stunreachability/stunreachability_test.go +++ b/internal/experiment/stunreachability/stunreachability_test.go @@ -152,13 +152,6 @@ func TestCancelledContext(t *testing.T) { if len(tk.Queries) <= 0 { t.Fatal("no DNS queries?!") } - sk, err := measurer.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - if _, ok := sk.(SummaryKeys); !ok { - t.Fatal("invalid type for summary keys") - } } func TestNewClientFailure(t *testing.T) { @@ -276,16 +269,3 @@ func TestReadFailure(t *testing.T) { t.Fatal("DNS queries?!") } } - -func TestSummaryKeysGeneric(t *testing.T) { - measurement := &model.Measurement{TestKeys: &TestKeys{}} - m := &Measurer{} - osk, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - sk := osk.(SummaryKeys) - if sk.IsAnomaly { - t.Fatal("invalid isAnomaly") - } -} diff --git a/internal/experiment/tcpping/tcpping.go b/internal/experiment/tcpping/tcpping.go index 63e93e550b..2fd6d362de 100644 --- a/internal/experiment/tcpping/tcpping.go +++ b/internal/experiment/tcpping/tcpping.go @@ -149,16 +149,3 @@ func (m *Measurer) tcpConnect(ctx context.Context, index int64, func NewExperimentMeasurer(config Config) model.ExperimentMeasurer { return &Measurer{config: config} } - -// SummaryKeys contains summary keys for this experiment. -// -// Note that this structure is part of the ABI contract with ooniprobe -// therefore we should be careful when changing it. -type SummaryKeys struct { - IsAnomaly bool `json:"-"` -} - -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - return SummaryKeys{IsAnomaly: false}, nil -} diff --git a/internal/experiment/tcpping/tcpping_test.go b/internal/experiment/tcpping/tcpping_test.go index 1d3bddffc3..ebe197a7d5 100644 --- a/internal/experiment/tcpping/tcpping_test.go +++ b/internal/experiment/tcpping/tcpping_test.go @@ -102,7 +102,7 @@ func TestMeasurer_run(t *testing.T) { defer env.Close() env.Do(func() { - meas, m, err := runHelper("tcpconnect://8.8.8.8:443") + meas, _, err := runHelper("tcpconnect://8.8.8.8:443") if err != nil { t.Fatalf("Unexpected error: %s", err) } @@ -112,15 +112,6 @@ func TestMeasurer_run(t *testing.T) { t.Fatal("unexpected number of pings") } - ask, err := m.GetSummaryKeys(meas) - if err != nil { - t.Fatal("cannot obtain summary") - } - summary := ask.(SummaryKeys) - if summary.IsAnomaly { - t.Fatal("expected no anomaly") - } - for _, p := range tk.Pings { if p.TCPConnect == nil { t.Fatal("TCPConnect should not be nil") @@ -161,24 +152,13 @@ func TestMeasurer_run(t *testing.T) { }) env.Do(func() { - meas, m, err := runHelper("tcpconnect://8.8.8.8:443") + meas, _, err := runHelper("tcpconnect://8.8.8.8:443") if err != nil { t.Fatalf("Unexpected error: %s", err) } tk, _ := (meas.TestKeys).(*TestKeys) - // note: this experiment does not set anomaly but we still want - // to have a test here for when we possibly will - ask, err := m.GetSummaryKeys(meas) - if err != nil { - t.Fatal("cannot obtain summary") - } - summary := ask.(SummaryKeys) - if summary.IsAnomaly { - t.Fatal("expected no anomaly") - } - for _, p := range tk.Pings { if p.TCPConnect == nil { t.Fatal("TCPConnect should not be nil") diff --git a/internal/experiment/telegram/telegram.go b/internal/experiment/telegram/telegram.go index 67a9d05556..adf470b25e 100644 --- a/internal/experiment/telegram/telegram.go +++ b/internal/experiment/telegram/telegram.go @@ -5,7 +5,6 @@ package telegram import ( "context" - "errors" "time" "github.com/ooni/probe-cli/v3/internal/experiment/urlgetter" @@ -136,10 +135,9 @@ func NewExperimentMeasurer(config Config) model.ExperimentMeasurer { return Measurer{Config: config} } +var _ model.MeasurementSummaryKeysProvider = &TestKeys{} + // SummaryKeys contains summary keys for this experiment. -// -// Note that this structure is part of the ABI contract with ooniprobe -// therefore we should be careful when changing it. type SummaryKeys struct { HTTPBlocking bool `json:"telegram_http_blocking"` TCPBlocking bool `json:"telegram_tcp_blocking"` @@ -147,13 +145,9 @@ type SummaryKeys struct { IsAnomaly bool `json:"-"` } -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - sk := SummaryKeys{IsAnomaly: false} - tk, ok := measurement.TestKeys.(*TestKeys) - if !ok { - return sk, errors.New("invalid test keys type") - } +// MeasurementSummaryKeys implements model.MeasurementSummaryKeysProvider. +func (tk *TestKeys) MeasurementSummaryKeys() model.MeasurementSummaryKeys { + sk := &SummaryKeys{IsAnomaly: false} tcpBlocking := tk.TelegramTCPBlocking httpBlocking := tk.TelegramHTTPBlocking webBlocking := tk.TelegramWebFailure != nil @@ -161,5 +155,10 @@ func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, e sk.HTTPBlocking = httpBlocking sk.WebBlocking = webBlocking sk.IsAnomaly = webBlocking || httpBlocking || tcpBlocking - return sk, nil + return sk +} + +// Anomaly implements model.MeasurementSummaryKeys. +func (sk *SummaryKeys) Anomaly() bool { + return sk.IsAnomaly } diff --git a/internal/experiment/telegram/telegram_test.go b/internal/experiment/telegram/telegram_test.go index ed1ed43cca..767d286f70 100644 --- a/internal/experiment/telegram/telegram_test.go +++ b/internal/experiment/telegram/telegram_test.go @@ -219,15 +219,6 @@ func TestUpdateWithAllGood(t *testing.T) { } } -func TestSummaryKeysInvalidType(t *testing.T) { - measurement := new(model.Measurement) - m := &telegram.Measurer{} - _, err := m.GetSummaryKeys(measurement) - if err.Error() != "invalid test keys type" { - t.Fatal("not the error we expected") - } -} - func TestSummaryKeysWorksAsIntended(t *testing.T) { failure := io.EOF.Error() tests := []struct { @@ -248,17 +239,15 @@ func TestSummaryKeysWorksAsIntended(t *testing.T) { }} for idx, tt := range tests { t.Run(fmt.Sprintf("%d", idx), func(t *testing.T) { - m := &telegram.Measurer{} measurement := &model.Measurement{TestKeys: &tt.tk} - got, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - return - } - sk := got.(telegram.SummaryKeys) + got := measurement.TestKeys.(*telegram.TestKeys).MeasurementSummaryKeys() + sk := got.(*telegram.SummaryKeys) if sk.IsAnomaly != tt.isAnomaly { t.Fatal("unexpected isAnomaly value") } + if sk.IsAnomaly != sk.Anomaly() { + t.Fatal("invalid Anomaly()") + } }) } } @@ -374,13 +363,6 @@ func TestMeasurerRun(t *testing.T) { if tk.TelegramWebStatus != "ok" { t.Fatal("unexpected TelegramWebStatus") } - sk, err := measurer.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - if _, ok := sk.(telegram.SummaryKeys); !ok { - t.Fatal("invalid type for summary keys") - } }) }) diff --git a/internal/experiment/tlsmiddlebox/measurer_test.go b/internal/experiment/tlsmiddlebox/measurer_test.go index dadd78f375..8c21e8359c 100644 --- a/internal/experiment/tlsmiddlebox/measurer_test.go +++ b/internal/experiment/tlsmiddlebox/measurer_test.go @@ -95,18 +95,10 @@ func TestMeasurer_input_failure(t *testing.T) { t.Fatal(err) } URL.Scheme = "tlshandshake" - meas, m, err := runHelper(context.Background(), "tlstrace://google.com", URL.String(), "") + meas, _, err := runHelper(context.Background(), "tlstrace://google.com", URL.String(), "") if err != nil { t.Fatal(err) } - ask, err := m.GetSummaryKeys(meas) - if err != nil { - t.Fatal("cannot obtain summary") - } - summary := ask.(SummaryKeys) - if summary.IsAnomaly { - t.Fatal("expected no anomaly") - } t.Run("testkeys", func(t *testing.T) { tk := meas.TestKeys.(*TestKeys) @@ -150,18 +142,10 @@ func TestMeasurer_input_failure(t *testing.T) { if err != nil { t.Fatal(err) } - meas, m, err := runHelper(context.Background(), "tlstrace://google.com", URL.String(), "") + meas, _, err := runHelper(context.Background(), "tlstrace://google.com", URL.String(), "") if err != nil { t.Fatal(err) } - ask, err := m.GetSummaryKeys(meas) - if err != nil { - t.Fatal("cannot obtain summary") - } - summary := ask.(SummaryKeys) - if summary.IsAnomaly { - t.Fatal("expected no anomaly") - } t.Run("testkeys", func(t *testing.T) { tk := meas.TestKeys.(*TestKeys) @@ -205,18 +189,10 @@ func TestMeasurer_input_failure(t *testing.T) { if err != nil { t.Fatal(err) } - meas, m, err := runHelper(context.Background(), "tlstrace://google.com", URL.String(), "") + meas, _, err := runHelper(context.Background(), "tlstrace://google.com", URL.String(), "") if err != nil { t.Fatal(err) } - ask, err := m.GetSummaryKeys(meas) - if err != nil { - t.Fatal("cannot obtain summary") - } - summary := ask.(SummaryKeys) - if summary.IsAnomaly { - t.Fatal("expected no anomaly") - } t.Run("testkeys", func(t *testing.T) { tk := meas.TestKeys.(*TestKeys) diff --git a/internal/experiment/tlsmiddlebox/summary.go b/internal/experiment/tlsmiddlebox/summary.go deleted file mode 100644 index e42728f1f0..0000000000 --- a/internal/experiment/tlsmiddlebox/summary.go +++ /dev/null @@ -1,18 +0,0 @@ -package tlsmiddlebox - -// -// Summary -// - -import "github.com/ooni/probe-cli/v3/internal/model" - -// Summary contains the summary results -type SummaryKeys struct { - IsAnomaly bool `json:"-"` -} - -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - // TODO(DecFox, bassosimone): Add anomaly logic to generate summary keys for the experiment - return SummaryKeys{IsAnomaly: false}, nil -} diff --git a/internal/experiment/tlsping/tlsping.go b/internal/experiment/tlsping/tlsping.go index 23eeea46ff..d52c2b8653 100644 --- a/internal/experiment/tlsping/tlsping.go +++ b/internal/experiment/tlsping/tlsping.go @@ -200,16 +200,3 @@ func (m *Measurer) tlsConnectAndHandshake(ctx context.Context, index int64, func NewExperimentMeasurer(config Config) model.ExperimentMeasurer { return &Measurer{config: config} } - -// SummaryKeys contains summary keys for this experiment. -// -// Note that this structure is part of the ABI contract with ooniprobe -// therefore we should be careful when changing it. -type SummaryKeys struct { - IsAnomaly bool `json:"-"` -} - -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - return SummaryKeys{IsAnomaly: false}, nil -} diff --git a/internal/experiment/tlsping/tlsping_test.go b/internal/experiment/tlsping/tlsping_test.go index 8c7cc0fe6f..f0d4d7a2cb 100644 --- a/internal/experiment/tlsping/tlsping_test.go +++ b/internal/experiment/tlsping/tlsping_test.go @@ -117,7 +117,7 @@ func TestMeasurerRun(t *testing.T) { defer env.Close() env.Do(func() { - meas, m, err := runHelper(context.Background(), "tlshandshake://8.8.8.8:443") + meas, _, err := runHelper(context.Background(), "tlshandshake://8.8.8.8:443") if err != nil { t.Fatalf("Unexpected error: %s", err) } @@ -127,15 +127,6 @@ func TestMeasurerRun(t *testing.T) { t.Fatal("unexpected number of pings") } - ask, err := m.GetSummaryKeys(meas) - if err != nil { - t.Fatal("cannot obtain summary") - } - summary := ask.(SummaryKeys) - if summary.IsAnomaly { - t.Fatal("expected no anomaly") - } - for _, p := range tk.Pings { if p.TCPConnect == nil { t.Fatal("TCPConnect should not be nil") @@ -176,24 +167,13 @@ func TestMeasurerRun(t *testing.T) { }) env.Do(func() { - meas, m, err := runHelper(context.Background(), "tlshandshake://8.8.8.8:443") + meas, _, err := runHelper(context.Background(), "tlshandshake://8.8.8.8:443") if err != nil { t.Fatalf("Unexpected error: %s", err) } tk, _ := (meas.TestKeys).(*TestKeys) - // note: this experiment does not set anomaly but we still want - // to have a test here for when we possibly will - ask, err := m.GetSummaryKeys(meas) - if err != nil { - t.Fatal("cannot obtain summary") - } - summary := ask.(SummaryKeys) - if summary.IsAnomaly { - t.Fatal("expected no anomaly") - } - for _, p := range tk.Pings { if p.TCPConnect == nil { t.Fatal("TCPConnect should not be nil") @@ -240,23 +220,12 @@ func TestMeasurerRun(t *testing.T) { }) env.Do(func() { - meas, m, err := runHelper(context.Background(), "tlshandshake://8.8.8.8:443") + meas, _, err := runHelper(context.Background(), "tlshandshake://8.8.8.8:443") if err != nil { t.Fatalf("Unexpected error: %s", err) } tk, _ := (meas.TestKeys).(*TestKeys) - // note: this experiment does not set anomaly but we still want - // to have a test here for when we possibly will - ask, err := m.GetSummaryKeys(meas) - if err != nil { - t.Fatal("cannot obtain summary") - } - summary := ask.(SummaryKeys) - if summary.IsAnomaly { - t.Fatal("expected no anomaly") - } - for _, p := range tk.Pings { if p.TCPConnect == nil { t.Fatal("TCPConnect should not be nil") diff --git a/internal/experiment/tlstool/tlstool.go b/internal/experiment/tlstool/tlstool.go index 33a8b6f11b..b523ec06d1 100644 --- a/internal/experiment/tlstool/tlstool.go +++ b/internal/experiment/tlstool/tlstool.go @@ -161,16 +161,3 @@ func (m Measurer) pattern(address string) string { func NewExperimentMeasurer(config Config) model.ExperimentMeasurer { return Measurer{config: config} } - -// SummaryKeys contains summary keys for this experiment. -// -// Note that this structure is part of the ABI contract with ooniprobe -// therefore we should be careful when changing it. -type SummaryKeys struct { - IsAnomaly bool `json:"-"` -} - -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - return SummaryKeys{IsAnomaly: false}, nil -} diff --git a/internal/experiment/tlstool/tlstool_test.go b/internal/experiment/tlstool/tlstool_test.go index c2d96b9a62..7e9c56d7f1 100644 --- a/internal/experiment/tlstool/tlstool_test.go +++ b/internal/experiment/tlstool/tlstool_test.go @@ -69,24 +69,4 @@ func TestRunWithCancelledContext(t *testing.T) { if err != nil { t.Fatal(err) } - sk, err := measurer.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - if _, ok := sk.(tlstool.SummaryKeys); !ok { - t.Fatal("invalid type for summary keys") - } -} - -func TestSummaryKeysGeneric(t *testing.T) { - measurement := &model.Measurement{TestKeys: &tlstool.TestKeys{}} - m := &tlstool.Measurer{} - osk, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - sk := osk.(tlstool.SummaryKeys) - if sk.IsAnomaly { - t.Fatal("invalid isAnomaly") - } } diff --git a/internal/experiment/tor/tor.go b/internal/experiment/tor/tor.go index ca391365f8..af5501509c 100644 --- a/internal/experiment/tor/tor.go +++ b/internal/experiment/tor/tor.go @@ -6,7 +6,6 @@ package tor import ( "context" "encoding/json" - "errors" "fmt" "net/url" "sync" @@ -381,10 +380,9 @@ func failureString(failure *string) (s string) { return } +var _ model.MeasurementSummaryKeysProvider = &TestKeys{} + // SummaryKeys contains summary keys for this experiment. -// -// Note that this structure is part of the ABI contract with ooniprobe -// therefore we should be careful when changing it. type SummaryKeys struct { DirPortTotal int64 `json:"dir_port_total"` DirPortAccessible int64 `json:"dir_port_accessible"` @@ -397,13 +395,9 @@ type SummaryKeys struct { IsAnomaly bool `json:"-"` } -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - sk := SummaryKeys{IsAnomaly: false} - tk, ok := measurement.TestKeys.(*TestKeys) - if !ok { - return sk, errors.New("invalid test keys type") - } +// MeasurementSummaryKeys implements model.MeasurementSummaryKeysProvider. +func (tk *TestKeys) MeasurementSummaryKeys() model.MeasurementSummaryKeys { + sk := &SummaryKeys{IsAnomaly: false} sk.DirPortTotal = tk.DirPortTotal sk.DirPortAccessible = tk.DirPortAccessible sk.OBFS4Total = tk.OBFS4Total @@ -416,5 +410,10 @@ func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, e (sk.OBFS4Accessible <= 0 && sk.OBFS4Total > 0) || (sk.ORPortDirauthAccessible <= 0 && sk.ORPortDirauthTotal > 0) || (sk.ORPortAccessible <= 0 && sk.ORPortTotal > 0)) - return sk, nil + return sk +} + +// Anomaly implements model.MeasurementSummaryKeys. +func (sk *SummaryKeys) Anomaly() bool { + return sk.IsAnomaly } diff --git a/internal/experiment/tor/tor_test.go b/internal/experiment/tor/tor_test.go index db0e7018e9..2d63b85442 100644 --- a/internal/experiment/tor/tor_test.go +++ b/internal/experiment/tor/tor_test.go @@ -108,13 +108,6 @@ func TestMeasurerMeasureGood(t *testing.T) { if err != nil { t.Fatal(err) } - sk, err := measurer.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - if _, ok := sk.(SummaryKeys); !ok { - t.Fatal("invalid type for summary keys") - } } var staticPrivateTestingTargetEndpoint = "209.148.46.65:443" @@ -736,15 +729,6 @@ func TestMaybeScrubbingLogger(t *testing.T) { }) } -func TestSummaryKeysInvalidType(t *testing.T) { - measurement := new(model.Measurement) - m := &Measurer{} - _, err := m.GetSummaryKeys(measurement) - if err.Error() != "invalid test keys type" { - t.Fatal("not the error we expected") - } -} - func TestSummaryKeysWorksAsIntended(t *testing.T) { tests := []struct { tk TestKeys @@ -779,17 +763,15 @@ func TestSummaryKeysWorksAsIntended(t *testing.T) { }} for idx, tt := range tests { t.Run(fmt.Sprintf("%d", idx), func(t *testing.T) { - m := &Measurer{} measurement := &model.Measurement{TestKeys: &tt.tk} - got, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - return - } - sk := got.(SummaryKeys) + got := measurement.TestKeys.(*TestKeys).MeasurementSummaryKeys() + sk := got.(*SummaryKeys) if sk.IsAnomaly != tt.isAnomaly { t.Fatal("unexpected isAnomaly value") } + if sk.IsAnomaly != sk.Anomaly() { + t.Fatal("invalid Anomaly()") + } }) } } diff --git a/internal/experiment/torsf/torsf.go b/internal/experiment/torsf/torsf.go index f504899b47..f61d6b9781 100644 --- a/internal/experiment/torsf/torsf.go +++ b/internal/experiment/torsf/torsf.go @@ -303,30 +303,19 @@ func NewExperimentMeasurer(config Config) model.ExperimentMeasurer { return &Measurer{config: config} } +var _ model.MeasurementSummaryKeysProvider = &TestKeys{} + // SummaryKeys contains summary keys for this experiment. -// -// Note that this structure is part of the ABI contract with ooniprobe -// therefore we should be careful when changing it. type SummaryKeys struct { IsAnomaly bool `json:"-"` } -var ( - // errInvalidTestKeysType indicates the test keys type is invalid. - errInvalidTestKeysType = errors.New("torsf: invalid test keys type") - - //errNilTestKeys indicates that the test keys are nil. - errNilTestKeys = errors.New("torsf: nil test keys") -) +// MeasurementSummaryKeys implements model.MeasurementSummaryKeysProvider. +func (tk *TestKeys) MeasurementSummaryKeys() model.MeasurementSummaryKeys { + return &SummaryKeys{IsAnomaly: tk.Failure != nil} +} -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m *Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - testkeys, good := measurement.TestKeys.(*TestKeys) - if !good { - return nil, errInvalidTestKeysType - } - if testkeys == nil { - return nil, errNilTestKeys - } - return SummaryKeys{IsAnomaly: testkeys.Failure != nil}, nil +// Anomaly implements model.MeasurementSummaryKeys. +func (sk *SummaryKeys) Anomaly() bool { + return sk.IsAnomaly } diff --git a/internal/experiment/torsf/torsf_test.go b/internal/experiment/torsf/torsf_test.go index 120ae86499..f7c2301810 100644 --- a/internal/experiment/torsf/torsf_test.go +++ b/internal/experiment/torsf/torsf_test.go @@ -411,56 +411,21 @@ func TestBaseTunnelDir(t *testing.T) { }) } -func TestGetSummaryKeys(t *testing.T) { - t.Run("in case of untyped nil TestKeys", func(t *testing.T) { - measurement := &model.Measurement{ - TestKeys: nil, - } - m := &Measurer{} - _, err := m.GetSummaryKeys(measurement) - if !errors.Is(err, errInvalidTestKeysType) { - t.Fatal("unexpected error", err) - } - }) - - t.Run("in case of typed nil TestKeys", func(t *testing.T) { - var tk *TestKeys - measurement := &model.Measurement{ - TestKeys: tk, - } - m := &Measurer{} - _, err := m.GetSummaryKeys(measurement) - if !errors.Is(err, errNilTestKeys) { - t.Fatal("unexpected error", err) - } - }) - - t.Run("in case of invalid TestKeys type", func(t *testing.T) { - measurement := &model.Measurement{ - TestKeys: make(chan int), - } - m := &Measurer{} - _, err := m.GetSummaryKeys(measurement) - if !errors.Is(err, errInvalidTestKeysType) { - t.Fatal("unexpected error", err) - } - }) - +func TestMeasurementSummaryKeys(t *testing.T) { t.Run("in case of success", func(t *testing.T) { measurement := &model.Measurement{ TestKeys: &TestKeys{ Failure: nil, }, } - m := &Measurer{} - sk, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - rsk := sk.(SummaryKeys) + sk := measurement.TestKeys.(*TestKeys).MeasurementSummaryKeys() + rsk := sk.(*SummaryKeys) if rsk.IsAnomaly { t.Fatal("expected no anomaly here") } + if rsk.IsAnomaly != sk.Anomaly() { + t.Fatal("invalid Anomaly()") + } }) t.Run("in case of failure", func(t *testing.T) { @@ -470,14 +435,13 @@ func TestGetSummaryKeys(t *testing.T) { Failure: &failure, }, } - m := &Measurer{} - sk, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - rsk := sk.(SummaryKeys) + sk := measurement.TestKeys.(*TestKeys).MeasurementSummaryKeys() + rsk := sk.(*SummaryKeys) if !rsk.IsAnomaly { t.Fatal("expected anomaly here") } + if rsk.IsAnomaly != sk.Anomaly() { + t.Fatal("invalid Anomaly()") + } }) } diff --git a/internal/experiment/urlgetter/urlgetter.go b/internal/experiment/urlgetter/urlgetter.go index f42a4a8ac4..d1a18e7681 100644 --- a/internal/experiment/urlgetter/urlgetter.go +++ b/internal/experiment/urlgetter/urlgetter.go @@ -123,16 +123,3 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { func NewExperimentMeasurer(config Config) model.ExperimentMeasurer { return Measurer{Config: config} } - -// SummaryKeys contains summary keys for this experiment. -// -// Note that this structure is part of the ABI contract with ooniprobe -// therefore we should be careful when changing it. -type SummaryKeys struct { - IsAnomaly bool `json:"-"` -} - -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - return SummaryKeys{IsAnomaly: false}, nil -} diff --git a/internal/experiment/urlgetter/urlgetter_test.go b/internal/experiment/urlgetter/urlgetter_test.go index 1fbd44dde6..cb11179839 100644 --- a/internal/experiment/urlgetter/urlgetter_test.go +++ b/internal/experiment/urlgetter/urlgetter_test.go @@ -39,13 +39,6 @@ func TestMeasurer(t *testing.T) { if len(tk.DNSCache) != 0 { t.Fatal("not the DNSCache value we expected") } - sk, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - if _, ok := sk.(urlgetter.SummaryKeys); !ok { - t.Fatal("invalid type for summary keys") - } } func TestMeasurerDNSCache(t *testing.T) { @@ -79,16 +72,3 @@ func TestMeasurerDNSCache(t *testing.T) { t.Fatal("invalid tk.DNSCache") } } - -func TestSummaryKeysGeneric(t *testing.T) { - measurement := &model.Measurement{TestKeys: &urlgetter.TestKeys{}} - m := &urlgetter.Measurer{} - osk, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - sk := osk.(urlgetter.SummaryKeys) - if sk.IsAnomaly { - t.Fatal("invalid isAnomaly") - } -} diff --git a/internal/experiment/vanillator/vanillator.go b/internal/experiment/vanillator/vanillator.go index 8d342b1dc0..e774d77792 100644 --- a/internal/experiment/vanillator/vanillator.go +++ b/internal/experiment/vanillator/vanillator.go @@ -231,30 +231,19 @@ func NewExperimentMeasurer(config Config) model.ExperimentMeasurer { return &Measurer{config: config} } +var _ model.MeasurementSummaryKeysProvider = &TestKeys{} + // SummaryKeys contains summary keys for this experiment. -// -// Note that this structure is part of the ABI contract with ooniprobe -// therefore we should be careful when changing it. type SummaryKeys struct { IsAnomaly bool `json:"-"` } -var ( - // errInvalidTestKeysType indicates the test keys type is invalid. - errInvalidTestKeysType = errors.New("vanilla_tor: invalid test keys type") - - //errNilTestKeys indicates that the test keys are nil. - errNilTestKeys = errors.New("vanilla_tor: nil test keys") -) +// MeasurementSummaryKeys implements model.MeasurementSummaryKeysProvider. +func (tk *TestKeys) MeasurementSummaryKeys() model.MeasurementSummaryKeys { + return &SummaryKeys{IsAnomaly: tk.Failure != nil} +} -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m *Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - testkeys, good := measurement.TestKeys.(*TestKeys) - if !good { - return nil, errInvalidTestKeysType - } - if testkeys == nil { - return nil, errNilTestKeys - } - return SummaryKeys{IsAnomaly: testkeys.Failure != nil}, nil +// Anomaly implements model.MeasurementSummaryKeys. +func (sk *SummaryKeys) Anomaly() bool { + return sk.IsAnomaly } diff --git a/internal/experiment/vanillator/vanillator_test.go b/internal/experiment/vanillator/vanillator_test.go index c6c075f53b..9f0eda5bfd 100644 --- a/internal/experiment/vanillator/vanillator_test.go +++ b/internal/experiment/vanillator/vanillator_test.go @@ -294,56 +294,21 @@ func TestFailureNoTorBinary(t *testing.T) { }) } -func TestGetSummaryKeys(t *testing.T) { - t.Run("in case of untyped nil TestKeys", func(t *testing.T) { - measurement := &model.Measurement{ - TestKeys: nil, - } - m := &Measurer{} - _, err := m.GetSummaryKeys(measurement) - if !errors.Is(err, errInvalidTestKeysType) { - t.Fatal("unexpected error", err) - } - }) - - t.Run("in case of typed nil TestKeys", func(t *testing.T) { - var tk *TestKeys - measurement := &model.Measurement{ - TestKeys: tk, - } - m := &Measurer{} - _, err := m.GetSummaryKeys(measurement) - if !errors.Is(err, errNilTestKeys) { - t.Fatal("unexpected error", err) - } - }) - - t.Run("in case of invalid TestKeys type", func(t *testing.T) { - measurement := &model.Measurement{ - TestKeys: make(chan int), - } - m := &Measurer{} - _, err := m.GetSummaryKeys(measurement) - if !errors.Is(err, errInvalidTestKeysType) { - t.Fatal("unexpected error", err) - } - }) - +func TestMeasurementSummaryKeys(t *testing.T) { t.Run("in case of success", func(t *testing.T) { measurement := &model.Measurement{ TestKeys: &TestKeys{ Failure: nil, }, } - m := &Measurer{} - sk, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - rsk := sk.(SummaryKeys) + sk := measurement.TestKeys.(*TestKeys).MeasurementSummaryKeys() + rsk := sk.(*SummaryKeys) if rsk.IsAnomaly { t.Fatal("expected no anomaly here") } + if rsk.IsAnomaly != sk.Anomaly() { + t.Fatal("invalid Anomaly()") + } }) t.Run("in case of failure", func(t *testing.T) { @@ -353,14 +318,13 @@ func TestGetSummaryKeys(t *testing.T) { Failure: &failure, }, } - m := &Measurer{} - sk, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - rsk := sk.(SummaryKeys) + sk := measurement.TestKeys.(*TestKeys).MeasurementSummaryKeys() + rsk := sk.(*SummaryKeys) if !rsk.IsAnomaly { t.Fatal("expected anomaly here") } + if rsk.IsAnomaly != sk.Anomaly() { + t.Fatal("invalid Anomaly()") + } }) } diff --git a/internal/experiment/webconnectivity/webconnectivity.go b/internal/experiment/webconnectivity/webconnectivity.go index e19d202e25..60405a938d 100644 --- a/internal/experiment/webconnectivity/webconnectivity.go +++ b/internal/experiment/webconnectivity/webconnectivity.go @@ -249,27 +249,27 @@ func ComputeTCPBlocking(measurement []tracex.TCPConnectEntry, return } +var _ model.MeasurementSummaryKeysProvider = &TestKeys{} + // SummaryKeys contains summary keys for this experiment. -// -// Note that this structure is part of the ABI contract with ooniprobe -// therefore we should be careful when changing it. type SummaryKeys struct { Accessible bool `json:"accessible"` Blocking string `json:"blocking"` IsAnomaly bool `json:"-"` } -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - sk := SummaryKeys{IsAnomaly: false} - tk, ok := measurement.TestKeys.(*TestKeys) - if !ok { - return sk, errors.New("invalid test keys type") - } +// MeasurementSummaryKeys implements model.MeasurementSummaryKeysProvider. +func (tk *TestKeys) MeasurementSummaryKeys() model.MeasurementSummaryKeys { + sk := &SummaryKeys{} sk.IsAnomaly = tk.BlockingReason != nil if tk.BlockingReason != nil { sk.Blocking = *tk.BlockingReason } sk.Accessible = tk.Accessible != nil && *tk.Accessible - return sk, nil + return sk +} + +// Anomaly implements model.MeasurementSummaryKeys. +func (sk *SummaryKeys) Anomaly() bool { + return sk.IsAnomaly } diff --git a/internal/experiment/webconnectivity/webconnectivity_test.go b/internal/experiment/webconnectivity/webconnectivity_test.go index fad9fe82fa..15db7634a7 100644 --- a/internal/experiment/webconnectivity/webconnectivity_test.go +++ b/internal/experiment/webconnectivity/webconnectivity_test.go @@ -88,14 +88,6 @@ func TestMeasureWithCancelledContext(t *testing.T) { if tk.HTTPExperimentFailure != nil { t.Fatal("unexpected http_experiment_failure") } - // TODO(bassosimone): write further checks here? - sk, err := measurer.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - if _, ok := sk.(webconnectivity.SummaryKeys); !ok { - t.Fatal("invalid type for summary keys") - } } func TestMeasureWithNoInput(t *testing.T) { @@ -385,15 +377,6 @@ func TestComputeTCPBlocking(t *testing.T) { } } -func TestSummaryKeysInvalidType(t *testing.T) { - measurement := new(model.Measurement) - m := &webconnectivity.Measurer{} - _, err := m.GetSummaryKeys(measurement) - if err.Error() != "invalid test keys type" { - t.Fatal("not the error we expected") - } -} - func TestSummaryKeysWorksAsIntended(t *testing.T) { failure := io.EOF.Error() truy := true @@ -424,14 +407,9 @@ func TestSummaryKeysWorksAsIntended(t *testing.T) { }} for idx, tt := range tests { t.Run(fmt.Sprintf("%d", idx), func(t *testing.T) { - m := &webconnectivity.Measurer{} measurement := &model.Measurement{TestKeys: &tt.tk} - got, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - return - } - sk := got.(webconnectivity.SummaryKeys) + got := measurement.TestKeys.(*webconnectivity.TestKeys).MeasurementSummaryKeys() + sk := got.(*webconnectivity.SummaryKeys) if sk.IsAnomaly != tt.isAnomaly { t.Fatal("unexpected isAnomaly value") } @@ -441,6 +419,9 @@ func TestSummaryKeysWorksAsIntended(t *testing.T) { if sk.Blocking != tt.Blocking { t.Fatal("unexpected Accessible value") } + if sk.IsAnomaly != sk.Anomaly() { + t.Fatal("invalid Anomaly()") + } }) } } diff --git a/internal/experiment/webconnectivitylte/summary.go b/internal/experiment/webconnectivitylte/summary.go deleted file mode 100644 index a47bee822a..0000000000 --- a/internal/experiment/webconnectivitylte/summary.go +++ /dev/null @@ -1,23 +0,0 @@ -package webconnectivitylte - -// -// Summary -// - -import "github.com/ooni/probe-cli/v3/internal/model" - -// Summary contains the summary results. -// -// Note that this structure is part of the ABI contract with ooniprobe -// therefore we should be careful when changing it. -type SummaryKeys struct { - // TODO: add here additional summary fields. - isAnomaly bool -} - -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m *Measurer) GetSummaryKeys(measurement *model.Measurement) (any, error) { - // TODO(bassosimone): fill all the SummaryKeys - sk := SummaryKeys{isAnomaly: false} - return sk, nil -} diff --git a/internal/experiment/webconnectivityqa/throttling.go b/internal/experiment/webconnectivityqa/throttling.go index 77d4786696..4839d71da0 100644 --- a/internal/experiment/webconnectivityqa/throttling.go +++ b/internal/experiment/webconnectivityqa/throttling.go @@ -12,7 +12,7 @@ import ( func throttlingWithHTTP() *TestCase { return &TestCase{ Name: "throttlingWithHTTP", - Flags: TestCaseFlagNoV04, + Flags: TestCaseFlagNoV04 | TestCaseFlagNoLTE, // TODO(https://github.com/ooni/probe/issues/2668) Input: "http://largefile.com/", Configure: func(env *netemx.QAEnv) { diff --git a/internal/experiment/whatsapp/whatsapp.go b/internal/experiment/whatsapp/whatsapp.go index 94055b07dd..a96b48d305 100644 --- a/internal/experiment/whatsapp/whatsapp.go +++ b/internal/experiment/whatsapp/whatsapp.go @@ -5,7 +5,6 @@ package whatsapp import ( "context" - "errors" "fmt" "math/rand" "net/url" @@ -184,10 +183,9 @@ func NewExperimentMeasurer(config Config) model.ExperimentMeasurer { return Measurer{Config: config} } +var _ model.MeasurementSummaryKeysProvider = &TestKeys{} + // SummaryKeys contains summary keys for this experiment. -// -// Note that this structure is part of the ABI contract with ooniprobe -// therefore we should be careful when changing it. type SummaryKeys struct { RegistrationServerBlocking bool `json:"registration_server_blocking"` WebBlocking bool `json:"whatsapp_web_blocking"` @@ -195,13 +193,9 @@ type SummaryKeys struct { IsAnomaly bool `json:"-"` } -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - sk := SummaryKeys{IsAnomaly: false} - tk, ok := measurement.TestKeys.(*TestKeys) - if !ok { - return sk, errors.New("invalid test keys type") - } +// MeasurementSummaryKeys implements model.MeasurementSummaryKeysProvider. +func (tk *TestKeys) MeasurementSummaryKeys() model.MeasurementSummaryKeys { + sk := &SummaryKeys{} blocking := func(value string) bool { return value == "blocked" } @@ -209,5 +203,10 @@ func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, e sk.WebBlocking = blocking(tk.WhatsappWebStatus) sk.EndpointsBlocking = blocking(tk.WhatsappEndpointsStatus) sk.IsAnomaly = (sk.RegistrationServerBlocking || sk.WebBlocking || sk.EndpointsBlocking) - return sk, nil + return sk +} + +// Anomaly implements model.MeasurementSummaryKeys. +func (sk *SummaryKeys) Anomaly() bool { + return sk.IsAnomaly } diff --git a/internal/experiment/whatsapp/whatsapp_test.go b/internal/experiment/whatsapp/whatsapp_test.go index 7f32b08ed8..22d9f0da4c 100644 --- a/internal/experiment/whatsapp/whatsapp_test.go +++ b/internal/experiment/whatsapp/whatsapp_test.go @@ -286,13 +286,6 @@ func TestFailureAllEndpoints(t *testing.T) { if tk.WhatsappWebStatus != "blocked" { t.Fatal("invalid WhatsappWebStatus") } - sk, err := measurer.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - } - if _, ok := sk.(whatsapp.SummaryKeys); !ok { - t.Fatal("invalid type for summary keys") - } }) } @@ -561,15 +554,6 @@ func TestWeConfigureWebChecksCorrectly(t *testing.T) { } } -func TestSummaryKeysInvalidType(t *testing.T) { - measurement := new(model.Measurement) - m := &whatsapp.Measurer{} - _, err := m.GetSummaryKeys(measurement) - if err.Error() != "invalid test keys type" { - t.Fatal("not the error we expected") - } -} - func TestSummaryKeysWorksAsIntended(t *testing.T) { tests := []struct { tk whatsapp.TestKeys @@ -610,14 +594,9 @@ func TestSummaryKeysWorksAsIntended(t *testing.T) { }} for idx, tt := range tests { t.Run(fmt.Sprintf("%d", idx), func(t *testing.T) { - m := &whatsapp.Measurer{} measurement := &model.Measurement{TestKeys: &tt.tk} - got, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - return - } - sk := got.(whatsapp.SummaryKeys) + got := measurement.TestKeys.(*whatsapp.TestKeys).MeasurementSummaryKeys() + sk := got.(*whatsapp.SummaryKeys) if sk.IsAnomaly != tt.isAnomaly { t.Fatal("unexpected isAnomaly value") } @@ -630,6 +609,9 @@ func TestSummaryKeysWorksAsIntended(t *testing.T) { if sk.EndpointsBlocking != tt.EndpointsBlocking { t.Fatal("unexpected endpointsBlocking value") } + if sk.IsAnomaly != sk.Anomaly() { + t.Fatal("invalid Anomaly()") + } }) } } diff --git a/internal/mocks/database.go b/internal/mocks/database.go index 0a59b9fd35..834bd383e9 100644 --- a/internal/mocks/database.go +++ b/internal/mocks/database.go @@ -16,7 +16,7 @@ type Database struct { MockDeleteResult func(resultID int64) error MockCreateMeasurement func(reportID sql.NullString, testName string, measurementDir string, idx int, resultID int64, urlID sql.NullInt64) (*model.DatabaseMeasurement, error) - MockAddTestKeys func(msmt *model.DatabaseMeasurement, tk interface{}) error + MockAddTestKeys func(msmt *model.DatabaseMeasurement, sk model.MeasurementSummaryKeys) error MockDone func(msmt *model.DatabaseMeasurement) error MockUploadFailed func(msmt *model.DatabaseMeasurement, failure string) error MockUploadSucceeded func(msmt *model.DatabaseMeasurement) error @@ -65,8 +65,8 @@ func (d *Database) CreateMeasurement(reportID sql.NullString, testName string, m } // AddTestKeys calls MockAddTestKeys -func (d *Database) AddTestKeys(msmt *model.DatabaseMeasurement, tk interface{}) error { - return d.MockAddTestKeys(msmt, tk) +func (d *Database) AddTestKeys(msmt *model.DatabaseMeasurement, sk model.MeasurementSummaryKeys) error { + return d.MockAddTestKeys(msmt, sk) } // Done calls MockDone diff --git a/internal/mocks/database_test.go b/internal/mocks/database_test.go index c35f23fdf2..13bd47a241 100644 --- a/internal/mocks/database_test.go +++ b/internal/mocks/database_test.go @@ -119,12 +119,11 @@ func TestDatabase(t *testing.T) { t.Run("AddTestKeys", func(t *testing.T) { expected := errors.New("mocked") db := &Database{ - MockAddTestKeys: func(msmt *model.DatabaseMeasurement, tk interface{}) error { + MockAddTestKeys: func(msmt *model.DatabaseMeasurement, sk model.MeasurementSummaryKeys) error { return expected }, } - tk := make(map[string]string) // use a random type to pass as any in test keys - err := db.AddTestKeys(&model.DatabaseMeasurement{}, tk) + err := db.AddTestKeys(&model.DatabaseMeasurement{}, nil /* it's fine because we don't use it in the func above */) if !errors.Is(err, expected) { t.Fatal("not the error we expected") } diff --git a/internal/mocks/experiment.go b/internal/mocks/experiment.go index bf8c1f49d7..daa28eb0bd 100644 --- a/internal/mocks/experiment.go +++ b/internal/mocks/experiment.go @@ -14,8 +14,6 @@ type Experiment struct { MockName func() string - MockGetSummaryKeys func(m *model.Measurement) (any, error) - MockReportID func() string MockMeasureAsync func(ctx context.Context, input string) (<-chan *model.Measurement, error) @@ -43,10 +41,6 @@ func (e *Experiment) Name() string { return e.MockName() } -func (e *Experiment) GetSummaryKeys(m *model.Measurement) (any, error) { - return e.MockGetSummaryKeys(m) -} - func (e *Experiment) ReportID() string { return e.MockReportID() } diff --git a/internal/mocks/experiment_test.go b/internal/mocks/experiment_test.go index bb16b1553e..dc31b680bc 100644 --- a/internal/mocks/experiment_test.go +++ b/internal/mocks/experiment_test.go @@ -45,22 +45,6 @@ func TestExperiment(t *testing.T) { } }) - t.Run("GetSummaryKeys", func(t *testing.T) { - expected := errors.New("mocked err") - e := &Experiment{ - MockGetSummaryKeys: func(m *model.Measurement) (any, error) { - return nil, expected - }, - } - out, err := e.GetSummaryKeys(&model.Measurement{}) - if !errors.Is(err, expected) { - t.Fatal("unexpected err", err) - } - if out != nil { - t.Fatal("invalid out") - } - }) - t.Run("ReportID", func(t *testing.T) { expect := "xyz" e := &Experiment{ diff --git a/internal/mocks/experimentmeasurer.go b/internal/mocks/experimentmeasurer.go index 2af61f99ba..18013aebe0 100644 --- a/internal/mocks/experimentmeasurer.go +++ b/internal/mocks/experimentmeasurer.go @@ -9,7 +9,6 @@ import ( type ExperimentMeasurer struct { MockExperimentName func() string MockExperimentVersion func() string - MockGetSummaryKeys func(*model.Measurement) (any, error) MockRun func(ctx context.Context, args *model.ExperimentArgs) error } @@ -25,11 +24,6 @@ func (em *ExperimentMeasurer) ExperimentVersion() string { return em.MockExperimentVersion() } -// GetSummaryKeys implements model.ExperimentMeasurer. -func (em *ExperimentMeasurer) GetSummaryKeys(meas *model.Measurement) (any, error) { - return em.MockGetSummaryKeys(meas) -} - // Run implements model.ExperimentMeasurer. func (em *ExperimentMeasurer) Run(ctx context.Context, args *model.ExperimentArgs) error { return em.MockRun(ctx, args) diff --git a/internal/mocks/experimentmeasurer_test.go b/internal/mocks/experimentmeasurer_test.go index dfde84e8b1..38fb16d2a9 100644 --- a/internal/mocks/experimentmeasurer_test.go +++ b/internal/mocks/experimentmeasurer_test.go @@ -33,22 +33,6 @@ func TestExperimentMeasurer(t *testing.T) { } }) - t.Run("GetSummaryKeys", func(t *testing.T) { - expect := errors.New("mocked error") - em := &ExperimentMeasurer{ - MockGetSummaryKeys: func(*model.Measurement) (any, error) { - return nil, expect - }, - } - sk, err := em.GetSummaryKeys(&model.Measurement{}) - if !errors.Is(err, expect) { - t.Fatal("unexpected error", err) - } - if sk != nil { - t.Fatal("expected nil summary keys") - } - }) - t.Run("Run", func(t *testing.T) { expect := errors.New("mocked error") em := &ExperimentMeasurer{ diff --git a/internal/model/database.go b/internal/model/database.go index e28a48cf95..cdfcd764eb 100644 --- a/internal/model/database.go +++ b/internal/model/database.go @@ -106,7 +106,7 @@ type WritableDatabase interface { // - tk is the testkeys // // Returns a non-nil error if measurement update failed - AddTestKeys(msmt *DatabaseMeasurement, tk any) error + AddTestKeys(msmt *DatabaseMeasurement, tk MeasurementSummaryKeys) error // Done marks the measurement as completed // diff --git a/internal/model/experiment.go b/internal/model/experiment.go index 1cba6c1b84..29385fa2ea 100644 --- a/internal/model/experiment.go +++ b/internal/model/experiment.go @@ -147,9 +147,6 @@ type ExperimentMeasurer interface { // return nil. This is important because the caller WILL NOT submit // the measurement if this method returns an error. Run(ctx context.Context, args *ExperimentArgs) error - - // GetSummaryKeys returns summary keys expected by ooni/probe-cli. - GetSummaryKeys(*Measurement) (interface{}, error) } // Experiment is an experiment instance. @@ -163,10 +160,6 @@ type Experiment interface { // Name returns the experiment name. Name() string - // GetSummaryKeys returns a data structure containing a - // summary of the test keys for ooniprobe. - GetSummaryKeys(m *Measurement) (any, error) - // ReportID returns the open report's ID, if we have opened a report // successfully before, or an empty string, otherwise. // diff --git a/internal/model/measurement.go b/internal/model/measurement.go index 745d336638..3bb427e8f5 100644 --- a/internal/model/measurement.go +++ b/internal/model/measurement.go @@ -159,6 +159,20 @@ type Measurement struct { TestVersion string `json:"test_version"` } +// MeasurementSummaryKeysProvider is the interface that the experiment test keys should implement +// when they want to have a customized implementation of the [MeasurementSummaryKeys]. If there is +// no such implementation, common code for running experiments returns a default implementation +// whose Anomaly method always returns false when invoked. +type MeasurementSummaryKeysProvider interface { + MeasurementSummaryKeys() MeasurementSummaryKeys +} + +// MeasurementSummaryKeys is the measurement summary. +type MeasurementSummaryKeys interface { + // Anomaly returns whether there was a measurement anomaly. + Anomaly() bool +} + // AddAnnotations adds the annotations from input to m.Annotations. func (m *Measurement) AddAnnotations(input map[string]string) { for key, value := range input { diff --git a/internal/tutorial/dslx/chapter02/README.md b/internal/tutorial/dslx/chapter02/README.md index 8a0c1fa97a..339f8452a3 100644 --- a/internal/tutorial/dslx/chapter02/README.md +++ b/internal/tutorial/dslx/chapter02/README.md @@ -146,10 +146,6 @@ type SummaryKeys struct { IsAnomaly bool `json:"-"` } -func (m *Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - return SummaryKeys{IsAnomaly: false}, nil -} - ``` ## `Run`: The experiment code diff --git a/internal/tutorial/dslx/chapter02/main.go b/internal/tutorial/dslx/chapter02/main.go index 17e9502aa3..fcee1367a9 100644 --- a/internal/tutorial/dslx/chapter02/main.go +++ b/internal/tutorial/dslx/chapter02/main.go @@ -147,10 +147,6 @@ type SummaryKeys struct { IsAnomaly bool `json:"-"` } -func (m *Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - return SummaryKeys{IsAnomaly: false}, nil -} - // ``` // // ## `Run`: The experiment code diff --git a/internal/tutorial/experiment/torsf/chapter02/README.md b/internal/tutorial/experiment/torsf/chapter02/README.md index 36f3323f56..ffa7eae7ce 100644 --- a/internal/tutorial/experiment/torsf/chapter02/README.md +++ b/internal/tutorial/experiment/torsf/chapter02/README.md @@ -136,9 +136,9 @@ for one second and prints a logging message. Before concluding this chapter, we also need to create the `SummaryKeys` for this experiment. For historical reasons, the `TestKeys` of each -experiment is an `interface{}`. Every experiment also defines a `SummaryKeys` -data structure and a `GetSummaryKeys` method to convert the opaque -result of a measurement to the summary for such an experiment. +experiment is an `interface{}`. Many experiment also defines a `SummaryKeys` +data structure and implement `model.MeasurementSummaryKeysProvider` to +convert the opaque result of a measurement to the summary for such an experiment. The experiment summary is *only* used by the OONI Probe CLI. @@ -153,16 +153,10 @@ type SummaryKeys struct { IsAnomaly bool `json:"-"` } -``` - -GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. This -method just converts the `TestKeys` inside `measurement` to an instance of -the `SummaryKeys` structure. For now, we'll just implement a stub returning -fake `SummaryKeys` declaring there was no anomaly. +var _ model.MeasurementSummaryKeys = &SummaryKeys{} -```Go -func (m *Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - return &SummaryKeys{IsAnomaly: false}, nil +func (sk *SummaryKeys) Anomaly() bool { + return sk.IsAnomaly } ``` diff --git a/internal/tutorial/experiment/torsf/chapter02/torsf.go b/internal/tutorial/experiment/torsf/chapter02/torsf.go index 2547cb4ccc..a5799809f5 100644 --- a/internal/tutorial/experiment/torsf/chapter02/torsf.go +++ b/internal/tutorial/experiment/torsf/chapter02/torsf.go @@ -112,9 +112,9 @@ func (m *Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { // // Before concluding this chapter, we also need to create the `SummaryKeys` // for this experiment. For historical reasons, the `TestKeys` of each -// experiment is an `interface{}`. Every experiment also defines a `SummaryKeys` -// data structure and a `GetSummaryKeys` method to convert the opaque -// result of a measurement to the summary for such an experiment. +// experiment is an `interface{}`. Many experiment also defines a `SummaryKeys` +// data structure and implement `model.MeasurementSummaryKeysProvider` to +// convert the opaque result of a measurement to the summary for such an experiment. // // The experiment summary is *only* used by the OONI Probe CLI. @@ -129,16 +129,10 @@ type SummaryKeys struct { IsAnomaly bool `json:"-"` } -// ``` -// -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. This -// method just converts the `TestKeys` inside `measurement` to an instance of -// the `SummaryKeys` structure. For now, we'll just implement a stub returning -// fake `SummaryKeys` declaring there was no anomaly. -// -// ```Go -func (m *Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - return &SummaryKeys{IsAnomaly: false}, nil +var _ model.MeasurementSummaryKeys = &SummaryKeys{} + +func (sk *SummaryKeys) Anomaly() bool { + return sk.IsAnomaly } // ``` diff --git a/internal/tutorial/experiment/torsf/chapter03/README.md b/internal/tutorial/experiment/torsf/chapter03/README.md index c1aa8df3fe..4d216d9cbb 100644 --- a/internal/tutorial/experiment/torsf/chapter03/README.md +++ b/internal/tutorial/experiment/torsf/chapter03/README.md @@ -24,6 +24,20 @@ type TestKeys struct { ``` +Note that we need to tell the TestKeys how to produce +the SummaryKeys for producing a summary. + +```Go + +var _ model.MeasurementSummaryKeysProvider = &TestKeys{} + +MeasurementSummaryKeys implements model.MeasurementSummaryKeysProvider. +func (tk *TestKeys) MeasurementSummaryKeys() model.MeasurementSummaryKeys { + return &SummaryKeys{IsAnomaly: tk.Failure != nil} +} + +``` + ### Rewriting the Run method Next we will rewrite the Run method. We will arrange for this diff --git a/internal/tutorial/experiment/torsf/chapter03/torsf.go b/internal/tutorial/experiment/torsf/chapter03/torsf.go index 6f23d7e546..1edf82c933 100644 --- a/internal/tutorial/experiment/torsf/chapter03/torsf.go +++ b/internal/tutorial/experiment/torsf/chapter03/torsf.go @@ -55,6 +55,20 @@ type TestKeys struct { Failure *string `json:"failure"` } +// ``` +// +// Note that we need to tell the TestKeys how to produce +// the SummaryKeys for producing a summary. +// +// ```Go + +var _ model.MeasurementSummaryKeysProvider = &TestKeys{} + +// MeasurementSummaryKeys implements model.MeasurementSummaryKeysProvider. +func (tk *TestKeys) MeasurementSummaryKeys() model.MeasurementSummaryKeys { + return &SummaryKeys{IsAnomaly: tk.Failure != nil} +} + // ``` // // ### Rewriting the Run method @@ -204,7 +218,8 @@ type SummaryKeys struct { IsAnomaly bool `json:"-"` } -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m *Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - return &SummaryKeys{IsAnomaly: false}, nil +var _ model.MeasurementSummaryKeys = &SummaryKeys{} + +func (sk *SummaryKeys) Anomaly() bool { + return sk.IsAnomaly } diff --git a/internal/tutorial/experiment/torsf/chapter04/torsf.go b/internal/tutorial/experiment/torsf/chapter04/torsf.go index 60870808b2..28808460c4 100644 --- a/internal/tutorial/experiment/torsf/chapter04/torsf.go +++ b/internal/tutorial/experiment/torsf/chapter04/torsf.go @@ -98,6 +98,13 @@ type TestKeys struct { Failure *string `json:"failure"` } +var _ model.MeasurementSummaryKeysProvider = &TestKeys{} + +// MeasurementSummaryKeys implements model.MeasurementSummaryKeysProvider. +func (tk *TestKeys) MeasurementSummaryKeys() model.MeasurementSummaryKeys { + return &SummaryKeys{IsAnomaly: tk.Failure != nil} +} + // Run implements ExperimentMeasurer.Run. func (m *Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { callbacks := args.Callbacks @@ -271,7 +278,8 @@ type SummaryKeys struct { IsAnomaly bool `json:"-"` } -// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. -func (m *Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { - return &SummaryKeys{IsAnomaly: false}, nil +var _ model.MeasurementSummaryKeys = &SummaryKeys{} + +func (sk *SummaryKeys) Anomaly() bool { + return sk.IsAnomaly }