Skip to content

Commit

Permalink
cleanup(engine): remove redundant APIs (ooni#1536)
Browse files Browse the repository at this point in the history
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 ooni/probe#2700
  • Loading branch information
bassosimone authored Apr 3, 2024
1 parent 08ae37f commit 725c466
Show file tree
Hide file tree
Showing 8 changed files with 16 additions and 27 deletions.
2 changes: 1 addition & 1 deletion cmd/ooniprobe/internal/cli/geoip/geoip.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/ooniprobe/internal/nettests/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/ooniprobe/internal/ooni/ooni.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion cmd/ooniprobe/internal/oonitest/oonitest.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
4 changes: 2 additions & 2 deletions internal/engine/inputloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
13 changes: 1 addition & 12 deletions internal/engine/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"errors"
"fmt"
"io/ioutil"
"net/url"
"os"
"sync"
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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(
Expand Down
12 changes: 6 additions & 6 deletions internal/engine/session_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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
Expand Down Expand Up @@ -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")
}
}
Expand All @@ -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")
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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")
}
Expand Down
4 changes: 2 additions & 2 deletions internal/experiment/webconnectivity/webconnectivity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 725c466

Please sign in to comment.