Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: move engine.InputLoader to targetloading #1616

Merged
merged 5 commits into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions cmd/ooniprobe/internal/nettests/dnscheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@ package nettests
import (
"context"

engine "github.com/ooni/probe-cli/v3/internal/engine"
"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/targetloading"
)

// DNSCheck nettest implementation.
type DNSCheck struct{}

func (n DNSCheck) lookupURLs(ctl *Controller) ([]model.ExperimentTarget, error) {
inputloader := &engine.InputLoader{
targetloader := &targetloading.Loader{
CheckInConfig: &model.OOAPICheckInConfig{
// not needed because we have default static input in the engine
},
Expand All @@ -21,7 +21,7 @@ func (n DNSCheck) lookupURLs(ctl *Controller) ([]model.ExperimentTarget, error)
SourceFiles: ctl.InputFiles,
StaticInputs: ctl.Inputs,
}
testlist, err := inputloader.Load(context.Background())
testlist, err := targetloader.Load(context.Background())
if err != nil {
return nil, err
}
Expand Down
6 changes: 3 additions & 3 deletions cmd/ooniprobe/internal/nettests/stunreachability.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@ package nettests
import (
"context"

engine "github.com/ooni/probe-cli/v3/internal/engine"
"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/targetloading"
)

// STUNReachability nettest implementation.
type STUNReachability struct{}

func (n STUNReachability) lookupURLs(ctl *Controller) ([]model.ExperimentTarget, error) {
inputloader := &engine.InputLoader{
targetloader := &targetloading.Loader{
CheckInConfig: &model.OOAPICheckInConfig{
// not needed because we have default static input in the engine
},
Expand All @@ -21,7 +21,7 @@ func (n STUNReachability) lookupURLs(ctl *Controller) ([]model.ExperimentTarget,
SourceFiles: ctl.InputFiles,
StaticInputs: ctl.Inputs,
}
testlist, err := inputloader.Load(context.Background())
testlist, err := targetloader.Load(context.Background())
if err != nil {
return nil, err
}
Expand Down
6 changes: 3 additions & 3 deletions cmd/ooniprobe/internal/nettests/web_connectivity.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ import (
"context"

"github.com/apex/log"
engine "github.com/ooni/probe-cli/v3/internal/engine"
"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/targetloading"
)

func (n WebConnectivity) lookupURLs(ctl *Controller, categories []string) ([]model.ExperimentTarget, error) {
inputloader := &engine.InputLoader{
targetloader := &targetloading.Loader{
CheckInConfig: &model.OOAPICheckInConfig{
// Setting Charging and OnWiFi to true causes the CheckIn
// API to return to us as much URL as possible with the
Expand All @@ -27,7 +27,7 @@ func (n WebConnectivity) lookupURLs(ctl *Controller, categories []string) ([]mod
SourceFiles: ctl.InputFiles,
StaticInputs: ctl.Inputs,
}
testlist, err := inputloader.Load(context.Background())
testlist, err := targetloader.Load(context.Background())
if err != nil {
return nil, err
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,16 @@ import (
"github.com/ooni/probe-cli/v3/internal/engine"
"github.com/ooni/probe-cli/v3/internal/kvstore"
"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/targetloading"
)

func TestInputLoaderInputOrQueryBackendWithNoInput(t *testing.T) {
// This historical integration test ensures that we're able to fetch URLs from
// the dev infrastructure. We say this test's historical because the targetloading.Loader
// belonged to the engine package before we introduced richer input. It kind of feels
// good to keep this integration test here since we want to use a real session and a real
// Loader and double check whether we can get inputs. In a more distant future it would
// kind of make sense to have a broader package with this kind of integration tests.
func TestTargetLoaderInputOrQueryBackendWithNoInput(t *testing.T) {
if testing.Short() {
t.Skip("skip test in short mode")
}
Expand All @@ -29,7 +36,7 @@ func TestInputLoaderInputOrQueryBackendWithNoInput(t *testing.T) {
t.Fatal(err)
}
defer sess.Close()
il := &engine.InputLoader{
il := &targetloading.Loader{
InputPolicy: model.InputOrQueryBackend,
Session: sess,
}
Expand Down
19 changes: 0 additions & 19 deletions internal/mocks/experimentinputloader.go

This file was deleted.

19 changes: 19 additions & 0 deletions internal/mocks/experimenttargetloader.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package mocks

import (
"context"

"github.com/ooni/probe-cli/v3/internal/model"
)

// ExperimentTargetLoader mocks model.ExperimentTargetLoader
type ExperimentTargetLoader struct {
MockLoad func(ctx context.Context) ([]model.ExperimentTarget, error)
}

var _ model.ExperimentTargetLoader = &ExperimentTargetLoader{}

// Load calls MockLoad
func (eil *ExperimentTargetLoader) Load(ctx context.Context) ([]model.ExperimentTarget, error) {
return eil.MockLoad(ctx)
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
func TestExperimentInputLoader(t *testing.T) {
t.Run("Load", func(t *testing.T) {
expected := errors.New("mocked error")
eil := &ExperimentInputLoader{
eil := &ExperimentTargetLoader{
MockLoad: func(ctx context.Context) ([]model.ExperimentTarget, error) {
return nil, expected
},
Expand Down
4 changes: 2 additions & 2 deletions internal/model/experiment.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,8 @@ type ExperimentOptionInfo struct {
Type string
}

// ExperimentInputLoader loads inputs from local or remote sources.
type ExperimentInputLoader interface {
// ExperimentTargetLoader loads targets from local or remote sources.
type ExperimentTargetLoader interface {
Load(ctx context.Context) ([]ExperimentTarget, error)
}

Expand Down
30 changes: 15 additions & 15 deletions internal/oonirun/experiment.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ import (
"sync/atomic"
"time"

"github.com/ooni/probe-cli/v3/internal/engine"
"github.com/ooni/probe-cli/v3/internal/humanize"
"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/targetloading"
)

// experimentShuffledInputs counts how many times we shuffled inputs
Expand Down Expand Up @@ -60,8 +60,8 @@ type Experiment struct {
// newExperimentBuilderFn is OPTIONAL and used for testing.
newExperimentBuilderFn func(experimentName string) (model.ExperimentBuilder, error)

// newInputLoaderFn is OPTIONAL and used for testing.
newInputLoaderFn func(inputPolicy model.InputPolicy) inputLoader
// newTargetLoaderFn is OPTIONAL and used for testing.
newTargetLoaderFn func(inputPolicy model.InputPolicy) targetLoader

// newSubmitterFn is OPTIONAL and used for testing.
newSubmitterFn func(ctx context.Context) (model.Submitter, error)
Expand All @@ -84,17 +84,17 @@ func (ed *Experiment) Run(ctx context.Context) error {
}

// 2. create input loader and load input for this experiment
inputLoader := ed.newInputLoader(builder.InputPolicy())
inputList, err := inputLoader.Load(ctx)
targetLoader := ed.newTargetLoader(builder.InputPolicy())
targetList, err := targetLoader.Load(ctx)
if err != nil {
return err
}

// 3. randomize input, if needed
if ed.Random {
rnd := rand.New(rand.NewSource(time.Now().UnixNano())) // #nosec G404 -- not really important
rnd.Shuffle(len(inputList), func(i, j int) {
inputList[i], inputList[j] = inputList[j], inputList[i]
rnd.Shuffle(len(targetList), func(i, j int) {
targetList[i], targetList[j] = targetList[j], targetList[i]
})
experimentShuffledInputs.Add(1)
}
Expand Down Expand Up @@ -127,7 +127,7 @@ func (ed *Experiment) Run(ctx context.Context) error {
}

// 8. create an input processor
inputProcessor := ed.newInputProcessor(experiment, inputList, saver, submitter)
inputProcessor := ed.newInputProcessor(experiment, targetList, saver, submitter)

// 9. process input and generate measurements
return inputProcessor.Run(ctx)
Expand Down Expand Up @@ -192,15 +192,15 @@ func (ed *Experiment) newExperimentBuilder(experimentName string) (model.Experim
return ed.Session.NewExperimentBuilder(ed.Name)
}

// inputLoader is an alias for model.ExperimentInputLoader
type inputLoader = model.ExperimentInputLoader
// targetLoader is an alias for [model.ExperimentTargetLoader].
type targetLoader = model.ExperimentTargetLoader

// newInputLoader creates a new inputLoader.
func (ed *Experiment) newInputLoader(inputPolicy model.InputPolicy) inputLoader {
if ed.newInputLoaderFn != nil {
return ed.newInputLoaderFn(inputPolicy)
// newTargetLoader creates a new [model.ExperimentTargetLoader].
func (ed *Experiment) newTargetLoader(inputPolicy model.InputPolicy) targetLoader {
if ed.newTargetLoaderFn != nil {
return ed.newTargetLoaderFn(inputPolicy)
}
return &engine.InputLoader{
return &targetloading.Loader{
CheckInConfig: &model.OOAPICheckInConfig{
RunType: model.RunTypeManual,
OnWiFi: true, // meaning: not on 4G
Expand Down
26 changes: 13 additions & 13 deletions internal/oonirun/experiment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func TestExperimentRunWithFailureToSubmitAndShuffle(t *testing.T) {
},
},
newExperimentBuilderFn: nil,
newInputLoaderFn: nil,
newTargetLoaderFn: nil,
newSubmitterFn: func(ctx context.Context) (model.Submitter, error) {
subm := &mocks.Submitter{
MockSubmit: func(ctx context.Context, m *model.Measurement) error {
Expand Down Expand Up @@ -165,7 +165,7 @@ func TestExperimentRun(t *testing.T) {
ReportFile string
Session Session
newExperimentBuilderFn func(experimentName string) (model.ExperimentBuilder, error)
newInputLoaderFn func(inputPolicy model.InputPolicy) inputLoader
newTargetLoaderFn func(inputPolicy model.InputPolicy) targetLoader
newSubmitterFn func(ctx context.Context) (model.Submitter, error)
newSaverFn func() (model.Saver, error)
newInputProcessorFn func(experiment model.Experiment,
Expand Down Expand Up @@ -199,8 +199,8 @@ func TestExperimentRun(t *testing.T) {
}
return eb, nil
},
newInputLoaderFn: func(inputPolicy model.InputPolicy) inputLoader {
return &mocks.ExperimentInputLoader{
newTargetLoaderFn: func(inputPolicy model.InputPolicy) targetLoader {
return &mocks.ExperimentTargetLoader{
MockLoad: func(ctx context.Context) ([]model.ExperimentTarget, error) {
return nil, errMocked
},
Expand All @@ -223,8 +223,8 @@ func TestExperimentRun(t *testing.T) {
}
return eb, nil
},
newInputLoaderFn: func(inputPolicy model.InputPolicy) inputLoader {
return &mocks.ExperimentInputLoader{
newTargetLoaderFn: func(inputPolicy model.InputPolicy) targetLoader {
return &mocks.ExperimentTargetLoader{
MockLoad: func(ctx context.Context) ([]model.ExperimentTarget, error) {
return []model.ExperimentTarget{}, nil
},
Expand Down Expand Up @@ -263,8 +263,8 @@ func TestExperimentRun(t *testing.T) {
}
return eb, nil
},
newInputLoaderFn: func(inputPolicy model.InputPolicy) inputLoader {
return &mocks.ExperimentInputLoader{
newTargetLoaderFn: func(inputPolicy model.InputPolicy) targetLoader {
return &mocks.ExperimentTargetLoader{
MockLoad: func(ctx context.Context) ([]model.ExperimentTarget, error) {
return []model.ExperimentTarget{}, nil
},
Expand Down Expand Up @@ -306,8 +306,8 @@ func TestExperimentRun(t *testing.T) {
}
return eb, nil
},
newInputLoaderFn: func(inputPolicy model.InputPolicy) inputLoader {
return &mocks.ExperimentInputLoader{
newTargetLoaderFn: func(inputPolicy model.InputPolicy) targetLoader {
return &mocks.ExperimentTargetLoader{
MockLoad: func(ctx context.Context) ([]model.ExperimentTarget, error) {
return []model.ExperimentTarget{}, nil
},
Expand Down Expand Up @@ -352,8 +352,8 @@ func TestExperimentRun(t *testing.T) {
}
return eb, nil
},
newInputLoaderFn: func(inputPolicy model.InputPolicy) inputLoader {
return &mocks.ExperimentInputLoader{
newTargetLoaderFn: func(inputPolicy model.InputPolicy) targetLoader {
return &mocks.ExperimentTargetLoader{
MockLoad: func(ctx context.Context) ([]model.ExperimentTarget, error) {
return []model.ExperimentTarget{}, nil
},
Expand Down Expand Up @@ -392,7 +392,7 @@ func TestExperimentRun(t *testing.T) {
ReportFile: tt.fields.ReportFile,
Session: tt.fields.Session,
newExperimentBuilderFn: tt.fields.newExperimentBuilderFn,
newInputLoaderFn: tt.fields.newInputLoaderFn,
newTargetLoaderFn: tt.fields.newTargetLoaderFn,
newSubmitterFn: tt.fields.newSubmitterFn,
newSaverFn: tt.fields.newSaverFn,
newInputProcessorFn: tt.fields.newInputProcessorFn,
Expand Down
6 changes: 3 additions & 3 deletions internal/oonirun/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ package oonirun
//

import (
"github.com/ooni/probe-cli/v3/internal/engine"
"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/targetloading"
)

// Session is the definition of Session used by this package.
type Session interface {
// A Session is also an InputLoaderSession.
engine.InputLoaderSession
// A Session is also an [targetloading.Session].
targetloading.Session

// A Session is also a SubmitterSession.
SubmitterSession
Expand Down
2 changes: 1 addition & 1 deletion internal/oonirun/v1.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func v1Measure(ctx context.Context, config *LinkConfig, URL string) error {
ReportFile: config.ReportFile,
Session: config.Session,
newExperimentBuilderFn: nil,
newInputLoaderFn: nil,
newTargetLoaderFn: nil,
newSubmitterFn: nil,
newSaverFn: nil,
newInputProcessorFn: nil,
Expand Down
2 changes: 1 addition & 1 deletion internal/oonirun/v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func V2MeasureDescriptor(ctx context.Context, config *LinkConfig, desc *V2Descri
ReportFile: config.ReportFile,
Session: config.Session,
newExperimentBuilderFn: nil,
newInputLoaderFn: nil,
newTargetLoaderFn: nil,
newSubmitterFn: nil,
newSaverFn: nil,
newInputProcessorFn: nil,
Expand Down
Loading
Loading