From 725c466e40dd5941c75f987dbed3a5066830b905 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Wed, 3 Apr 2024 06:24:07 +0200 Subject: [PATCH] cleanup(engine): remove redundant APIs (#1536) Let's just always use MaybeLookupLocationContext and MaybeLookupBackendsContext. This change simplifies the interaction graph related to using the probeservices and more generally the engine. Part of https://github.com/ooni/probe/issues/2700 --- cmd/ooniprobe/internal/cli/geoip/geoip.go | 2 +- cmd/ooniprobe/internal/nettests/run.go | 4 ++-- cmd/ooniprobe/internal/ooni/ooni.go | 2 +- cmd/ooniprobe/internal/oonitest/oonitest.go | 2 +- internal/engine/inputloader.go | 4 ++-- internal/engine/session.go | 13 +------------ internal/engine/session_integration_test.go | 12 ++++++------ .../webconnectivity/webconnectivity_test.go | 4 ++-- 8 files changed, 16 insertions(+), 27 deletions(-) diff --git a/cmd/ooniprobe/internal/cli/geoip/geoip.go b/cmd/ooniprobe/internal/cli/geoip/geoip.go index 81395cdebe..641ecc73e7 100644 --- a/cmd/ooniprobe/internal/cli/geoip/geoip.go +++ b/cmd/ooniprobe/internal/cli/geoip/geoip.go @@ -43,7 +43,7 @@ func dogeoip(config dogeoipconfig) error { } defer engine.Close() - err = engine.MaybeLookupLocation() + err = engine.MaybeLookupLocationContext(context.Background()) if err != nil { return err } diff --git a/cmd/ooniprobe/internal/nettests/run.go b/cmd/ooniprobe/internal/nettests/run.go index 503124aaec..e4ec2fc254 100644 --- a/cmd/ooniprobe/internal/nettests/run.go +++ b/cmd/ooniprobe/internal/nettests/run.go @@ -66,7 +66,7 @@ func RunGroup(config RunGroupConfig) error { } defer sess.Close() - err = sess.MaybeLookupLocation() + err = sess.MaybeLookupLocationContext(context.Background()) if err != nil { log.WithError(err).Error("Failed to lookup the location of the probe") return err @@ -77,7 +77,7 @@ func RunGroup(config RunGroupConfig) error { log.WithError(err).Error("Failed to create the network row") return err } - if err := sess.MaybeLookupBackends(); err != nil { + if err := sess.MaybeLookupBackendsContext(context.Background()); err != nil { log.WithError(err).Errorf("Failed to discover OONI backends") return err } diff --git a/cmd/ooniprobe/internal/ooni/ooni.go b/cmd/ooniprobe/internal/ooni/ooni.go index dcffdab6c4..69235caef4 100644 --- a/cmd/ooniprobe/internal/ooni/ooni.go +++ b/cmd/ooniprobe/internal/ooni/ooni.go @@ -42,7 +42,7 @@ type ProbeCLI interface { // ProbeEngine is an instance of the OONI Probe engine. type ProbeEngine interface { Close() error - MaybeLookupLocation() error + MaybeLookupLocationContext(context.Context) error ProbeASNString() string ProbeCC() string ProbeIP() string diff --git a/cmd/ooniprobe/internal/oonitest/oonitest.go b/cmd/ooniprobe/internal/oonitest/oonitest.go index a915c8f83f..0d5fd79d10 100644 --- a/cmd/ooniprobe/internal/oonitest/oonitest.go +++ b/cmd/ooniprobe/internal/oonitest/oonitest.go @@ -84,7 +84,7 @@ func (eng *FakeProbeEngine) Close() error { } // MaybeLookupLocation implements ProbeEngine.MaybeLookupLocation -func (eng *FakeProbeEngine) MaybeLookupLocation() error { +func (eng *FakeProbeEngine) MaybeLookupLocationContext(_ context.Context) error { return eng.FakeMaybeLookupLocation } diff --git a/internal/engine/inputloader.go b/internal/engine/inputloader.go index 9bc8b0a86e..e9faf039b6 100644 --- a/internal/engine/inputloader.go +++ b/internal/engine/inputloader.go @@ -146,7 +146,7 @@ func (il *InputLoader) loadOptional() ([]model.OOAPIURLInfo, error) { } // loadStrictlyRequired implements the InputStrictlyRequired policy. -func (il *InputLoader) loadStrictlyRequired(ctx context.Context) ([]model.OOAPIURLInfo, error) { +func (il *InputLoader) loadStrictlyRequired(_ context.Context) ([]model.OOAPIURLInfo, error) { inputs, err := il.loadLocal() if err != nil || len(inputs) > 0 { return inputs, err @@ -242,7 +242,7 @@ func staticInputForExperiment(name string) ([]model.OOAPIURLInfo, error) { } // loadOrStaticDefault implements the InputOrStaticDefault policy. -func (il *InputLoader) loadOrStaticDefault(ctx context.Context) ([]model.OOAPIURLInfo, error) { +func (il *InputLoader) loadOrStaticDefault(_ context.Context) ([]model.OOAPIURLInfo, error) { inputs, err := il.loadLocal() if err != nil || len(inputs) > 0 { return inputs, err diff --git a/internal/engine/session.go b/internal/engine/session.go index 27e897d48c..9b9bbeae48 100644 --- a/internal/engine/session.go +++ b/internal/engine/session.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "io/ioutil" "net/url" "os" "sync" @@ -155,7 +154,7 @@ func NewSession(ctx context.Context, config SessionConfig) (*Session, error) { // use the temporary directory on the current system. This should // work on Desktop. We tested that it did also work on iOS, but // we have also seen on 2020-06-10 that it does not work on Android. - tempDir, err := ioutil.TempDir(config.TempDir, "ooniengine") + tempDir, err := os.MkdirTemp(config.TempDir, "ooniengine") if err != nil { return nil, err } @@ -384,16 +383,6 @@ func (s *Session) Logger() model.Logger { return s.logger } -// MaybeLookupLocation is a caching location lookup call. -func (s *Session) MaybeLookupLocation() error { - return s.MaybeLookupLocationContext(context.Background()) -} - -// MaybeLookupBackends is a caching OONI backends lookup call. -func (s *Session) MaybeLookupBackends() error { - return s.MaybeLookupBackendsContext(context.Background()) -} - // ErrAlreadyUsingProxy indicates that we cannot create a tunnel with // a specific name because we already configured a proxy. var ErrAlreadyUsingProxy = errors.New( diff --git a/internal/engine/session_integration_test.go b/internal/engine/session_integration_test.go index 7bea2abff1..4770a6a366 100644 --- a/internal/engine/session_integration_test.go +++ b/internal/engine/session_integration_test.go @@ -146,7 +146,7 @@ func newSessionForTestingNoLookups(t *testing.T) *Session { func newSessionForTestingNoBackendsLookup(t *testing.T) *Session { sess := newSessionForTestingNoLookups(t) - if err := sess.MaybeLookupLocation(); err != nil { + if err := sess.MaybeLookupLocationContext(context.Background()); err != nil { t.Fatal(err) } log.Infof("Platform: %s", sess.Platform()) @@ -164,7 +164,7 @@ func newSessionForTestingNoBackendsLookup(t *testing.T) *Session { func newSessionForTesting(t *testing.T) *Session { sess := newSessionForTestingNoBackendsLookup(t) - if err := sess.MaybeLookupBackends(); err != nil { + if err := sess.MaybeLookupBackendsContext(context.Background()); err != nil { t.Fatal(err) } return sess @@ -245,7 +245,7 @@ func TestBouncerError(t *testing.T) { if sess.ProxyURL() == nil { t.Fatal("expected to see explicit proxy here") } - if err := sess.MaybeLookupBackends(); err == nil { + if err := sess.MaybeLookupBackendsContext(context.Background()); err == nil { t.Fatal("expected an error here") } } @@ -260,7 +260,7 @@ func TestMaybeLookupBackendsNewClientError(t *testing.T) { Address: "httpo://jehhrikjjqrlpufu.onion", }} defer sess.Close() - err := sess.MaybeLookupBackends() + err := sess.MaybeLookupBackendsContext(context.Background()) if !errors.Is(err, ErrAllProbeServicesFailed) { t.Fatal("not the error we expected") } @@ -272,7 +272,7 @@ func TestSessionLocationLookup(t *testing.T) { } sess := newSessionForTestingNoLookups(t) defer sess.Close() - if err := sess.MaybeLookupLocation(); err != nil { + if err := sess.MaybeLookupLocationContext(context.Background()); err != nil { t.Fatal(err) } if sess.ProbeASNString() == model.DefaultProbeASNString { @@ -419,7 +419,7 @@ func TestAllProbeServicesUnsupported(t *testing.T) { Address: "mascetti", Type: "antani", }) - err = sess.MaybeLookupBackends() + err = sess.MaybeLookupBackendsContext(context.Background()) if !errors.Is(err, ErrAllProbeServicesFailed) { t.Fatal("unexpected error") } diff --git a/internal/experiment/webconnectivity/webconnectivity_test.go b/internal/experiment/webconnectivity/webconnectivity_test.go index 15db7634a7..a4ee0c8b7b 100644 --- a/internal/experiment/webconnectivity/webconnectivity_test.go +++ b/internal/experiment/webconnectivity/webconnectivity_test.go @@ -236,11 +236,11 @@ func newsession(t *testing.T, lookupBackends bool) model.ExperimentSession { t.Fatal(err) } if lookupBackends { - if err := sess.MaybeLookupBackends(); err != nil { + if err := sess.MaybeLookupBackendsContext(context.Background()); err != nil { t.Fatal(err) } } - if err := sess.MaybeLookupLocation(); err != nil { + if err := sess.MaybeLookupLocationContext(context.Background()); err != nil { t.Fatal(err) } return sess