refactor: move engine.InputLoader to targetloading (#1616)

This commit moves the engine.InputLoader type to a new package called
inputloading and adapts the naming to avoid stuttering. We therefore
have engine.InputLoaderSession => targetloading.Session and other
similar renames.

The reason for moving InputLoader is that the engine package depends on
registry, and, per the plan described by the first richer input PR,
#1615, we want to move target
loading directly inside the registry. To this end, we need to move the
target loading feature outside of engine to avoid creating import loops,
which prevent the code from compiling because Go does not support them.

While there, name the package targetloading rather than inputloading
since richer input is all about targets, where a target is defined by
the (input, options) tuple. Also, try to consistently rename types to
mention targets.

We keep an integration test inside the engine package because it seems
such an integration test was checking both engine and the Loader
together. We may further refactor this test in the future.

Part of #1612

---------

Co-authored-by: DecFox <[email protected]>

cmd/ooniprobe/internal/nettests/dnscheck.go

@@ -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
 		},
@@ -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
 	}

cmd/ooniprobe/internal/nettests/stunreachability.go

@@ -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
 		},
@@ -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
 	}

cmd/ooniprobe/internal/nettests/web_connectivity.go

@@ -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
@@ -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
 	}

internal/engine/inputloader_network_test.go → internal/engine/inputloader_integration_test.go

@@ -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")
 	}
@@ -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,
 	}

internal/mocks/experimenttargetloader.go

@@ -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)
+}

internal/mocks/experimentinputloader_test.go → internal/mocks/experimenttargetloader_test.go

@@ -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
 			},

internal/model/experiment.go

@@ -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)
 }
 

internal/oonirun/experiment.go

@@ -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
@@ -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)
@@ -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)
 	}
@@ -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)
@@ -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

internal/oonirun/experiment_test.go

@@ -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 {
@@ -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,
@@ -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
 					},
@@ -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
 					},
@@ -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
 					},
@@ -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
 					},
@@ -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
 					},
@@ -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,

internal/oonirun/session.go

@@ -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

internal/oonirun/v1.go

@@ -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,

internal/oonirun/v2.go

@@ -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,