Skip to content

Commit

Permalink
x
Browse files Browse the repository at this point in the history
  • Loading branch information
bassosimone committed Jun 6, 2024
1 parent 4ea29ff commit 4beba37
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 26 deletions.
25 changes: 25 additions & 0 deletions internal/experimentname/experimentname.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Package experimentname contains code to manipulate experiment names.
package experimentname

import "github.com/ooni/probe-cli/v3/internal/strcasex"

// Canonicalize allows code to provide experiment names
// in a more flexible way, where we have aliases.
//
// Because we allow for uppercase experiment names for backwards
// compatibility with MK, we need to add some exceptions here when
// mapping (e.g., DNSCheck => dnscheck).
func Canonicalize(name string) string {
switch name = strcasex.ToSnake(name); name {
case "ndt_7":
name = "ndt" // since 2020-03-18, we use ndt7 to implement ndt by default
case "dns_check":
name = "dnscheck"
case "stun_reachability":
name = "stunreachability"
case "web_connectivity@v_0_5":
name = "[email protected]"
default:
}
return name
}
25 changes: 2 additions & 23 deletions internal/registry/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import (
"strconv"

"github.com/ooni/probe-cli/v3/internal/checkincache"
"github.com/ooni/probe-cli/v3/internal/experimentname"
"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/strcasex"
"github.com/ooni/probe-cli/v3/internal/targetloading"
)

Expand Down Expand Up @@ -238,27 +238,6 @@ func (b *Factory) NewTargetLoader(config *model.ExperimentTargetLoaderConfig) mo
}
}

// CanonicalizeExperimentName allows code to provide experiment names
// in a more flexible way, where we have aliases.
//
// Because we allow for uppercase experiment names for backwards
// compatibility with MK, we need to add some exceptions here when
// mapping (e.g., DNSCheck => dnscheck).
func CanonicalizeExperimentName(name string) string {
switch name = strcasex.ToSnake(name); name {
case "ndt_7":
name = "ndt" // since 2020-03-18, we use ndt7 to implement ndt by default
case "dns_check":
name = "dnscheck"
case "stun_reachability":
name = "stunreachability"
case "web_connectivity@v_0_5":
name = "[email protected]"
default:
}
return name
}

// ErrNoSuchExperiment indicates a given experiment does not exist.
var ErrNoSuchExperiment = errors.New("no such experiment")

Expand Down Expand Up @@ -305,7 +284,7 @@ const OONI_FORCE_ENABLE_EXPERIMENT = "OONI_FORCE_ENABLE_EXPERIMENT"
func NewFactory(name string, kvStore model.KeyValueStore, logger model.Logger) (*Factory, error) {
// Make sure we are deadling with the canonical experiment name. Historically MK used
// names such as WebConnectivity and we want to continue supporting this use case.
name = CanonicalizeExperimentName(name)
name = experimentname.Canonicalize(name)

// Handle A/B testing where we dynamically choose LTE for some users. The current policy
// only relates to a few users to collect data.
Expand Down
3 changes: 2 additions & 1 deletion internal/registry/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/ooni/probe-cli/v3/internal/checkincache"
"github.com/ooni/probe-cli/v3/internal/experiment/webconnectivitylte"
"github.com/ooni/probe-cli/v3/internal/experimentname"
"github.com/ooni/probe-cli/v3/internal/kvstore"
"github.com/ooni/probe-cli/v3/internal/model"
)
Expand Down Expand Up @@ -691,7 +692,7 @@ func TestNewFactory(t *testing.T) {

// get experiment expectations -- note that here we must canonicalize the
// experiment name otherwise we won't find it into the map when testing non-canonical names
expectations := expectationsMap[CanonicalizeExperimentName(tc.experimentName)]
expectations := expectationsMap[experimentname.Canonicalize(tc.experimentName)]
if expectations == nil {
t.Fatal("no expectations for", tc.experimentName)
}
Expand Down
5 changes: 3 additions & 2 deletions internal/targetloading/targetloading.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/apex/log"
"github.com/ooni/probe-cli/v3/internal/experiment/openvpn"
"github.com/ooni/probe-cli/v3/internal/experimentname"
"github.com/ooni/probe-cli/v3/internal/fsx"
"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/stuninput"
Expand Down Expand Up @@ -225,7 +226,7 @@ func StaticBareInputForExperiment(name string) ([]string, error) {
//
// TODO(https://github.com/ooni/probe/issues/2557): server STUNReachability
// inputs using richer input (aka check-in v2).
switch name {
switch experimentname.Canonicalize(name) {
case "dnscheck":
return dnsCheckDefaultInput, nil
case "stunreachability":
Expand Down Expand Up @@ -300,7 +301,7 @@ func (il *Loader) readfile(filepath string, open openFunc) ([]model.ExperimentTa

// loadRemote loads inputs from a remote source.
func (il *Loader) loadRemote(ctx context.Context) ([]model.ExperimentTarget, error) {
switch il.ExperimentName {
switch experimentname.Canonicalize(il.ExperimentName) {
case "openvpn":
// TODO(ainghazal): given the semantics of the current API call, in an ideal world we'd need to pass
// the desired provider here, if known. We only have one provider for now, so I'm acting like YAGNI. Another
Expand Down

0 comments on commit 4beba37

Please sign in to comment.