Skip to content

Commit

Permalink
refactor: rename geolocate enginelocate (ooni#1263)
Browse files Browse the repository at this point in the history
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.)
  • Loading branch information
bassosimone authored and Murphy-OrangeMud committed Feb 13, 2024
1 parent 3fe5e32 commit 4c86cbc
Show file tree
Hide file tree
Showing 20 changed files with 40 additions and 76 deletions.
20 changes: 10 additions & 10 deletions internal/engine/experiment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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"
},
Expand Down
12 changes: 6 additions & 6 deletions internal/engine/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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(),
Expand All @@ -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)
}
Expand Down
10 changes: 5 additions & 5 deletions internal/engine/session_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -85,7 +85,7 @@ func TestSessionCheckInSuccessful(t *testing.T) {
},
}
s := &Session{
location: &geolocate.Results{
location: &enginelocate.Results{
ASN: 137,
CountryCode: "IT",
},
Expand Down Expand Up @@ -136,7 +136,7 @@ func TestSessionCheckInNetworkError(t *testing.T) {
Error: expect,
}
s := &Session{
location: &geolocate.Results{
location: &enginelocate.Results{
ASN: 137,
CountryCode: "IT",
},
Expand Down Expand Up @@ -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",
},
Expand Down Expand Up @@ -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())
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package geolocate
package enginelocate

import (
"context"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package geolocate
package enginelocate

import (
"context"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package geolocate
package enginelocate

import (
"net/http"
Expand Down
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package geolocate
package enginelocate

import (
"context"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package geolocate
package enginelocate

import (
"context"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package geolocate
package enginelocate

import (
"context"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package geolocate
package enginelocate

import (
"context"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package geolocate
package enginelocate

import (
"github.com/ooni/probe-cli/v3/internal/geoipx"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package geolocate
package enginelocate

import (
"context"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package geolocate
package enginelocate

import (
"context"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package geolocate
package enginelocate

import (
"context"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package geolocate
package enginelocate

import (
"context"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package geolocate
package enginelocate

import (
"context"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package geolocate
package enginelocate

import (
"context"
Expand Down
6 changes: 3 additions & 3 deletions pkg/oonimkall/session_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
36 changes: 0 additions & 36 deletions script/gorenamepkg.bash

This file was deleted.

0 comments on commit 4c86cbc

Please sign in to comment.