refactor: rename geolocate enginelocate (#1263)

I am soon going to start addressing the underlying issue described by ooni/probe#2531. But, before doing that, I have noticed that the packages I need to edit to this end are the following: ``` ./internal/engine ./internal/geolocate ./internal/sessionresolver ``` Now, these three packages work in unison to provide an `engine.Session` and they should *sort* together. However, I would like also to avoid nesting because I think all of them servers an ~independent purpose and geolocate and sessionresolver are possibly building blocks to refactor or reimplement the engine. For this reason, I have chosen to rename them such that it is clear they are the engine and supporting packages. This diff addresses the first half of the change by renaming the geolocate package. While there acknowledge that the script to rename packages was broken and decide to ditch it for good rather than entering into the quest of fixing it. I would probably have spent lots of time trying in doing that, and my time is better spent otherwise. (I remember the official package renaming tool did not support go modules, but probably it does and I should look into giving it another spin.)
ooni · Sep 14, 2023 · 8c0646a · 8c0646a
1 parent 90366e5
commit 8c0646a
Show file tree

Hide file tree

Showing 20 changed files with 40 additions and 76 deletions.
diff --git a/internal/engine/experiment_test.go b/internal/engine/experiment_test.go
@@ -3,12 +3,12 @@ package engine
 import (
 	"testing"
 
-	"github.com/ooni/probe-cli/v3/internal/geolocate"
+	"github.com/ooni/probe-cli/v3/internal/enginelocate"
 	"github.com/ooni/probe-cli/v3/internal/model"
 )
 
 func TestExperimentHonoursSharingDefaults(t *testing.T) {
-	measure := func(info *geolocate.Results) *model.Measurement {
+	measure := func(info *enginelocate.Results) *model.Measurement {
 		sess := &Session{location: info}
 		builder, err := sess.NewExperimentBuilder("example")
 		if err != nil {
@@ -19,48 +19,48 @@ func TestExperimentHonoursSharingDefaults(t *testing.T) {
 	}
 	type spec struct {
 		name         string
-		locationInfo *geolocate.Results
+		locationInfo *enginelocate.Results
 		expect       func(*model.Measurement) bool
 	}
 	allspecs := []spec{{
 		name:         "probeIP",
-		locationInfo: &geolocate.Results{ProbeIP: "8.8.8.8"},
+		locationInfo: &enginelocate.Results{ProbeIP: "8.8.8.8"},
 		expect: func(m *model.Measurement) bool {
 			return m.ProbeIP == model.DefaultProbeIP
 		},
 	}, {
 		name:         "probeASN",
-		locationInfo: &geolocate.Results{ASN: 30722},
+		locationInfo: &enginelocate.Results{ASN: 30722},
 		expect: func(m *model.Measurement) bool {
 			return m.ProbeASN == "AS30722"
 		},
 	}, {
 		name:         "probeCC",
-		locationInfo: &geolocate.Results{CountryCode: "IT"},
+		locationInfo: &enginelocate.Results{CountryCode: "IT"},
 		expect: func(m *model.Measurement) bool {
 			return m.ProbeCC == "IT"
 		},
 	}, {
 		name:         "probeNetworkName",
-		locationInfo: &geolocate.Results{NetworkName: "Vodafone Italia"},
+		locationInfo: &enginelocate.Results{NetworkName: "Vodafone Italia"},
 		expect: func(m *model.Measurement) bool {
 			return m.ProbeNetworkName == "Vodafone Italia"
 		},
 	}, {
 		name:         "resolverIP",
-		locationInfo: &geolocate.Results{ResolverIP: "9.9.9.9"},
+		locationInfo: &enginelocate.Results{ResolverIP: "9.9.9.9"},
 		expect: func(m *model.Measurement) bool {
 			return m.ResolverIP == "9.9.9.9"
 		},
 	}, {
 		name:         "resolverASN",
-		locationInfo: &geolocate.Results{ResolverASN: 44},
+		locationInfo: &enginelocate.Results{ResolverASN: 44},
 		expect: func(m *model.Measurement) bool {
 			return m.ResolverASN == "AS44"
 		},
 	}, {
 		name:         "resolverNetworkName",
-		locationInfo: &geolocate.Results{ResolverNetworkName: "Google LLC"},
+		locationInfo: &enginelocate.Results{ResolverNetworkName: "Google LLC"},
 		expect: func(m *model.Measurement) bool {
 			return m.ResolverNetworkName == "Google LLC"
 		},

diff --git a/internal/engine/session.go b/internal/engine/session.go
@@ -13,7 +13,7 @@ import (
 
 	"github.com/ooni/probe-cli/v3/internal/bytecounter"
 	"github.com/ooni/probe-cli/v3/internal/checkincache"
-	"github.com/ooni/probe-cli/v3/internal/geolocate"
+	"github.com/ooni/probe-cli/v3/internal/enginelocate"
 	"github.com/ooni/probe-cli/v3/internal/kvstore"
 	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/ooni/probe-cli/v3/internal/netxlite"
@@ -60,7 +60,7 @@ type Session struct {
 	byteCounter              *bytecounter.Counter
 	httpDefaultTransport     model.HTTPTransport
 	kvStore                  model.KeyValueStore
-	location                 *geolocate.Results
+	location                 *enginelocate.Results
 	logger                   model.Logger
 	proxyURL                 *url.URL
 	queryProbeServicesCount  *atomic.Int64
@@ -79,7 +79,7 @@ type Session struct {
 
 	// testLookupLocationContext is a an optional hook for testing
 	// allowing us to mock LookupLocationContext.
-	testLookupLocationContext func(ctx context.Context) (*geolocate.Results, error)
+	testLookupLocationContext func(ctx context.Context) (*enginelocate.Results, error)
 
 	// testMaybeLookupBackendsContext is an optional hook for testing
 	// allowing us to mock MaybeLookupBackendsContext.
@@ -676,8 +676,8 @@ func (s *Session) MaybeLookupBackendsContext(ctx context.Context) error {
 
 // LookupLocationContext performs a location lookup. If you want memoisation
 // of the results, you should use MaybeLookupLocationContext.
-func (s *Session) LookupLocationContext(ctx context.Context) (*geolocate.Results, error) {
-	task := geolocate.NewTask(geolocate.Config{
+func (s *Session) LookupLocationContext(ctx context.Context) (*enginelocate.Results, error) {
+	task := enginelocate.NewTask(enginelocate.Config{
 		Logger:    s.Logger(),
 		Resolver:  s.resolver,
 		UserAgent: s.UserAgent(),
@@ -687,7 +687,7 @@ func (s *Session) LookupLocationContext(ctx context.Context) (*geolocate.Results
 
 // lookupLocationContext calls testLookupLocationContext if set and
 // otherwise calls LookupLocationContext.
-func (s *Session) lookupLocationContext(ctx context.Context) (*geolocate.Results, error) {
+func (s *Session) lookupLocationContext(ctx context.Context) (*enginelocate.Results, error) {
 	if s.testLookupLocationContext != nil {
 		return s.testLookupLocationContext(ctx)
 	}

diff --git a/internal/engine/session_internal_test.go b/internal/engine/session_internal_test.go
@@ -11,9 +11,9 @@ import (
 	"github.com/apex/log"
 	"github.com/google/go-cmp/cmp"
 	"github.com/ooni/probe-cli/v3/internal/checkincache"
+	"github.com/ooni/probe-cli/v3/internal/enginelocate"
 	"github.com/ooni/probe-cli/v3/internal/experiment/webconnectivity"
 	"github.com/ooni/probe-cli/v3/internal/experiment/webconnectivitylte"
-	"github.com/ooni/probe-cli/v3/internal/geolocate"
 	"github.com/ooni/probe-cli/v3/internal/kvstore"
 	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/ooni/probe-cli/v3/internal/registry"
@@ -85,7 +85,7 @@ func TestSessionCheckInSuccessful(t *testing.T) {
 		},
 	}
 	s := &Session{
-		location: &geolocate.Results{
+		location: &enginelocate.Results{
 			ASN:         137,
 			CountryCode: "IT",
 		},
@@ -136,7 +136,7 @@ func TestSessionCheckInNetworkError(t *testing.T) {
 		Error: expect,
 	}
 	s := &Session{
-		location: &geolocate.Results{
+		location: &enginelocate.Results{
 			ASN:         137,
 			CountryCode: "IT",
 		},
@@ -178,7 +178,7 @@ func TestSessionCheckInCannotLookupLocation(t *testing.T) {
 func TestSessionCheckInCannotCreateProbeServicesClient(t *testing.T) {
 	errMocked := errors.New("mocked error")
 	s := &Session{
-		location: &geolocate.Results{
+		location: &enginelocate.Results{
 			ASN:         137,
 			CountryCode: "IT",
 		},
@@ -240,7 +240,7 @@ func TestSessionNewSubmitterWithCancelledContext(t *testing.T) {
 func TestSessionMaybeLookupLocationContextLookupLocationContextFailure(t *testing.T) {
 	errMocked := errors.New("mocked error")
 	sess := newSessionForTestingNoLookups(t)
-	sess.testLookupLocationContext = func(ctx context.Context) (*geolocate.Results, error) {
+	sess.testLookupLocationContext = func(ctx context.Context) (*enginelocate.Results, error) {
 		return nil, errMocked
 	}
 	err := sess.MaybeLookupLocationContext(context.Background())

diff --git a/internal/geolocate/cloudflare.go → internal/enginelocate/cloudflare.go b/internal/geolocate/cloudflare.go → internal/enginelocate/cloudflare.go
@@ -1,4 +1,4 @@
-package geolocate
+package enginelocate
 
 import (
 	"context"

diff --git a/internal/geolocate/cloudflare_test.go → internal/enginelocate/cloudflare_test.go b/internal/geolocate/cloudflare_test.go → internal/enginelocate/cloudflare_test.go
@@ -1,4 +1,4 @@
-package geolocate
+package enginelocate
 
 import (
 	"context"

diff --git a/internal/geolocate/fake_test.go → internal/enginelocate/fake_test.go b/internal/geolocate/fake_test.go → internal/enginelocate/fake_test.go
@@ -1,4 +1,4 @@
-package geolocate
+package enginelocate
 
 import (
 	"net/http"

diff --git a/internal/geolocate/geolocate.go → internal/enginelocate/geolocate.go b/internal/geolocate/geolocate.go → internal/enginelocate/geolocate.go
@@ -1,5 +1,5 @@
-// Package geolocate implements IP lookup, resolver lookup, and geolocation.
-package geolocate
+// Package enginelocate implements IP lookup, resolver lookup, and geolocation.
+package enginelocate
 
 import (
 	"context"

diff --git a/internal/geolocate/geolocate_test.go → internal/enginelocate/geolocate_test.go b/internal/geolocate/geolocate_test.go → internal/enginelocate/geolocate_test.go
@@ -1,4 +1,4 @@
-package geolocate
+package enginelocate
 
 import (
 	"context"

diff --git a/internal/geolocate/invalid_test.go → internal/enginelocate/invalid_test.go b/internal/geolocate/invalid_test.go → internal/enginelocate/invalid_test.go
@@ -1,4 +1,4 @@
-package geolocate
+package enginelocate
 
 import (
 	"context"

diff --git a/internal/geolocate/iplookup.go → internal/enginelocate/iplookup.go b/internal/geolocate/iplookup.go → internal/enginelocate/iplookup.go
@@ -1,4 +1,4 @@
-package geolocate
+package enginelocate
 
 import (
 	"context"

diff --git a/internal/geolocate/iplookup_test.go → internal/enginelocate/iplookup_test.go b/internal/geolocate/iplookup_test.go → internal/enginelocate/iplookup_test.go
@@ -1,4 +1,4 @@
-package geolocate
+package enginelocate
 
 import (
 	"context"

diff --git a/internal/geolocate/mmdblookup.go → internal/enginelocate/mmdblookup.go b/internal/geolocate/mmdblookup.go → internal/enginelocate/mmdblookup.go
@@ -1,4 +1,4 @@
-package geolocate
+package enginelocate
 
 import (
 	"github.com/ooni/probe-cli/v3/internal/geoipx"

diff --git a/internal/geolocate/resolverlookup.go → internal/enginelocate/resolverlookup.go b/internal/geolocate/resolverlookup.go → internal/enginelocate/resolverlookup.go
@@ -1,4 +1,4 @@
-package geolocate
+package enginelocate
 
 import (
 	"context"

diff --git a/internal/geolocate/resolverlookup_test.go → internal/enginelocate/resolverlookup_test.go b/internal/geolocate/resolverlookup_test.go → internal/enginelocate/resolverlookup_test.go
@@ -1,4 +1,4 @@
-package geolocate
+package enginelocate
 
 import (
 	"context"

diff --git a/internal/geolocate/stun.go → internal/enginelocate/stun.go b/internal/geolocate/stun.go → internal/enginelocate/stun.go
@@ -1,4 +1,4 @@
-package geolocate
+package enginelocate
 
 import (
 	"context"

diff --git a/internal/geolocate/stun_test.go → internal/enginelocate/stun_test.go b/internal/geolocate/stun_test.go → internal/enginelocate/stun_test.go
@@ -1,4 +1,4 @@
-package geolocate
+package enginelocate
 
 import (
 	"context"

diff --git a/internal/geolocate/ubuntu.go → internal/enginelocate/ubuntu.go b/internal/geolocate/ubuntu.go → internal/enginelocate/ubuntu.go
@@ -1,4 +1,4 @@
-package geolocate
+package enginelocate
 
 import (
 	"context"

diff --git a/internal/geolocate/ubuntu_test.go → internal/enginelocate/ubuntu_test.go b/internal/geolocate/ubuntu_test.go → internal/enginelocate/ubuntu_test.go
@@ -1,4 +1,4 @@
-package geolocate
+package enginelocate
 
 import (
 	"context"

diff --git a/pkg/oonimkall/session_integration_test.go b/pkg/oonimkall/session_integration_test.go
@@ -11,7 +11,7 @@ import (
 	"testing"
 	"time"
 
-	"github.com/ooni/probe-cli/v3/internal/geolocate"
+	"github.com/ooni/probe-cli/v3/internal/enginelocate"
 	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/ooni/probe-cli/v3/pkg/oonimkall"
 )
@@ -50,7 +50,7 @@ func ReduceErrorForGeolocate(err error) error {
 	if errors.Is(err, context.Canceled) {
 		return nil // when we have not downloaded the resources yet
 	}
-	if !errors.Is(err, geolocate.ErrAllIPLookuppersFailed) {
+	if !errors.Is(err, enginelocate.ErrAllIPLookuppersFailed) {
 		return nil // otherwise
 	}
 	return fmt.Errorf("not the error we expected: %w", err)
@@ -295,7 +295,7 @@ func TestCheckInLookupLocationFailure(t *testing.T) {
 	config.WebConnectivity.AddCategory("CULTR")
 	ctx.Cancel() // immediate failure
 	result, err := sess.CheckIn(ctx, &config)
-	if !errors.Is(err, geolocate.ErrAllIPLookuppersFailed) {
+	if !errors.Is(err, enginelocate.ErrAllIPLookuppersFailed) {
 		t.Fatalf("not the error we expected: %+v", err)
 	}
 	if result != nil {

diff --git a/script/gorenamepkg.bash b/script/gorenamepkg.bash