From 33de7012636bbfd68a7dfc9c9312a76b0c40354e Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Fri, 4 Jun 2021 10:34:18 +0200 Subject: [PATCH] refactor: flatten and separate (#353) * refactor(atomicx): move outside the engine package After merging probe-engine into probe-cli, my impression is that we have too much unnecessary nesting of packages in this repository. The idea of this commit and of a bunch of following commits will instead be to reduce the nesting and simplify the structure. While there, improve the documentation. * fix: always use the atomicx package For consistency, never use sync/atomic and always use ./internal/atomicx so we can just grep and make sure we're not risking to crash if we make a subtle mistake on a 32 bit platform. While there, mention in the contributing guidelines that we want to always prefer the ./internal/atomicx package over sync/atomic. * fix(atomicx): remove unnecessary constructor We don't need a constructor here. The default constructed `&Int64{}` instance is already usable and the constructor does not add anything to what we are doing, rather it just creates extra confusion. * cleanup(atomicx): we are not using Float64 Because atomicx.Float64 is unused, we can safely zap it. * cleanup(atomicx): simplify impl and improve tests We can simplify the implementation by using defer and by letting the Load() method call Add(0). We can improve tests by making many goroutines updated the atomic int64 value concurrently. * refactor(fsx): can live in the ./internal pkg Let us reduce the amount of nesting. While there, ensure that the package only exports the bare minimum, and improve the documentation of the tests, to ease reading the code. * refactor: move runtimex to ./internal * refactor: move shellx into the ./internal package While there, remove unnecessary dependency between packages. While there, specify in the contributing guidelines that one should use x/sys/execabs instead of os/exec. * refactor: move ooapi into the ./internal pkg * refactor(humanize): move to ./internal and better docs * refactor: move platform to ./internal * refactor(randx): move to ./internal * refactor(multierror): move into the ./internal pkg * refactor(kvstore): all kvstores in ./internal Rather than having part of the kvstore inside ./internal/engine/kvstore and part in ./internal/engine/kvstore.go, let us put every piece of code that is kvstore related into the ./internal/kvstore package. * fix(kvstore): always return ErrNoSuchKey on Get() error It should help to use the kvstore everywhere removing all the copies that are lingering around the tree. * sessionresolver: make KVStore mandatory Simplifies implementation. While there, use the ./internal/kvstore package rather than having our private implementation. * fix(ooapi): use the ./internal/kvstore package * fix(platform): better documentation --- CONTRIBUTING.md | 6 + .../internal/autorun/autorun_darwin.go | 2 +- cmd/ooniprobe/internal/nettests/dnscheck.go | 2 +- cmd/ooniprobe/internal/ooni/ooni.go | 26 +- internal/atomicx/atomicx.go | 43 +++ internal/atomicx/atomicx_test.go | 28 ++ internal/cmd/apitool/main.go | 10 +- internal/cmd/jafar/iptables/iptables.go | 2 +- .../iptables/iptables_integration_test.go | 2 +- internal/cmd/jafar/iptables/iptables_linux.go | 4 +- internal/cmd/jafar/main.go | 4 +- internal/cmd/jafar/main_test.go | 2 +- internal/cmd/jafar/uncensored/uncensored.go | 2 +- internal/cmd/miniooni/libminiooni.go | 13 +- internal/cmd/oohelper/internal/client.go | 2 +- internal/cmd/oohelper/internal/fake_test.go | 6 +- internal/cmd/oohelper/oohelper.go | 2 +- internal/cmd/oohelperd/internal/fake_test.go | 6 +- internal/engine/atomicx/README.md | 3 - internal/engine/atomicx/atomicx.go | 68 ---- internal/engine/atomicx/atomicx_test.go | 50 --- internal/engine/experiment.go | 3 +- internal/engine/experiment/dash/dash.go | 4 +- .../engine/experiment/dnscheck/dnscheck.go | 11 +- .../experiment/dnscheck/dnscheck_test.go | 3 +- internal/engine/experiment/hhfm/hhfm.go | 2 +- internal/engine/experiment/hirl/hirl.go | 2 +- internal/engine/experiment/ndt7/ndt7.go | 6 +- .../engine/experiment/psiphon/psiphon_test.go | 4 +- .../experiment/telegram/telegram_test.go | 8 +- .../tlstool/internal/splitter_test.go | 2 +- internal/engine/experiment/tlstool/tlstool.go | 2 +- internal/engine/experiment/tor/tor.go | 6 +- .../engine/experiment/urlgetter/runner.go | 2 +- .../experiment/urlgetter/runner_test.go | 6 +- .../experiment/webconnectivity/endpoints.go | 2 +- .../webconnectivity/endpoints_test.go | 6 +- .../webconnectivity/httpanalysis_test.go | 2 +- .../engine/experiment/whatsapp/whatsapp.go | 2 +- .../experiment/whatsapp/whatsapp_test.go | 4 +- internal/engine/geolocate/geolocate.go | 2 +- internal/engine/geolocate/iplookup.go | 2 +- internal/engine/humanizex/humanizex_test.go | 34 -- internal/engine/inputloader.go | 4 +- internal/engine/inputloader_network_test.go | 4 +- internal/engine/inputloader_test.go | 4 +- internal/engine/inputprocessor.go | 30 +- internal/engine/inputprocessor_test.go | 10 +- internal/engine/internal/fsx/fsx_test.go | 76 ----- internal/engine/internal/mockable/mockable.go | 4 +- .../sessionresolver/integration_test.go | 5 +- .../internal/sessionresolver/memkvstore.go | 43 --- .../sessionresolver/memkvstore_test.go | 47 --- .../sessionresolver/sessionresolver.go | 12 +- .../sessionresolver/sessionresolver_test.go | 25 +- .../engine/internal/sessionresolver/state.go | 13 +- .../internal/sessionresolver/state_test.go | 44 ++- internal/engine/kvstore.go | 35 --- internal/engine/kvstore/kvstore.go | 44 --- internal/engine/kvstore_test.go | 31 -- internal/engine/legacy/netx/dialid/dialid.go | 4 +- .../engine/legacy/netx/handlers/handlers.go | 2 +- .../netx/oldhttptransport/tracetripper.go | 4 +- .../netx/transactionid/transactionid.go | 4 +- .../legacy/oonitemplates/oonitemplates.go | 6 +- .../engine/netx/bytecounter/bytecounter.go | 4 +- internal/engine/netx/netx.go | 2 +- internal/engine/netx/resolver/bogon.go | 2 +- internal/engine/netx/resolver/fake_test.go | 6 +- internal/engine/netx/resolver/serial.go | 4 +- internal/engine/netx/selfcensor/selfcensor.go | 6 +- internal/engine/ooapi/README.md | 5 - internal/engine/ooapi/kvstore_test.go | 34 -- .../probeservices/checkreportid_test.go | 16 +- .../probeservices/measurementmeta_test.go | 16 +- .../engine/probeservices/probeservices.go | 6 +- internal/engine/probeservices/register.go | 2 +- .../engine/probeservices/statefile_test.go | 10 +- internal/engine/session.go | 12 +- internal/engine/submitter_test.go | 15 +- internal/{engine/internal => }/fsx/fsx.go | 14 +- internal/fsx/fsx_test.go | 84 +++++ .../internal => }/fsx/testdata/.gitignore | 0 .../internal => }/fsx/testdata/testfile.txt | 0 .../humanizex => humanize}/humanizex.go | 9 +- internal/humanize/humanizex_test.go | 30 ++ internal/kvstore/fs.go | 53 ++++ internal/kvstore/fs_test.go | 67 ++++ internal/kvstore/memory.go | 42 +++ .../memory_test.go} | 11 +- .../internal => }/multierror/multierror.go | 11 +- .../multierror/multierror_test.go | 2 +- .../{engine => }/ooapi/apimodel/checkin.go | 0 .../ooapi/apimodel/checkreportid.go | 0 internal/{engine => }/ooapi/apimodel/doc.go | 0 internal/{engine => }/ooapi/apimodel/login.go | 0 .../ooapi/apimodel/measurementmeta.go | 0 .../{engine => }/ooapi/apimodel/openreport.go | 0 .../ooapi/apimodel/psiphonconfig.go | 0 .../{engine => }/ooapi/apimodel/register.go | 0 .../ooapi/apimodel/submitmeasurement.go | 0 .../ooapi/apimodel/testhelpers.go | 0 .../{engine => }/ooapi/apimodel/tortargets.go | 0 internal/{engine => }/ooapi/apimodel/urls.go | 0 internal/{engine => }/ooapi/apis.go | 2 +- internal/{engine => }/ooapi/apis_test.go | 2 +- internal/{engine => }/ooapi/caching.go | 2 +- internal/{engine => }/ooapi/caching_test.go | 15 +- internal/{engine => }/ooapi/callers.go | 2 +- internal/{engine => }/ooapi/client.go | 0 internal/{engine => }/ooapi/clientcall.go | 2 +- .../{engine => }/ooapi/clientcall_test.go | 21 +- internal/{engine => }/ooapi/cloners.go | 0 internal/{engine => }/ooapi/default.go | 0 internal/{engine => }/ooapi/default_test.go | 0 internal/{engine => }/ooapi/dependencies.go | 0 internal/{engine => }/ooapi/doc.go | 0 internal/{engine => }/ooapi/errors.go | 0 internal/{engine => }/ooapi/fake_test.go | 0 internal/{engine => }/ooapi/fakeapi_test.go | 70 +++-- internal/{engine => }/ooapi/fakefill_test.go | 2 +- .../{engine => }/ooapi/httpclient_test.go | 0 .../{engine => }/ooapi/integration_test.go | 19 +- .../ooapi/internal/generator/apis.go | 2 +- .../ooapi/internal/generator/apistest.go | 2 +- .../ooapi/internal/generator/caching.go | 2 +- .../ooapi/internal/generator/cachingtest.go | 15 +- .../ooapi/internal/generator/callers.go | 2 +- .../ooapi/internal/generator/clientcall.go | 2 +- .../internal/generator/clientcalltest.go | 5 +- .../ooapi/internal/generator/cloners.go | 0 .../ooapi/internal/generator/fakeapitest.go | 10 +- .../ooapi/internal/generator/generator.go | 0 .../ooapi/internal/generator/login.go | 2 +- .../ooapi/internal/generator/logintest.go | 143 +++++---- .../ooapi/internal/generator/reflect.go | 0 .../ooapi/internal/generator/requests.go | 2 +- .../ooapi/internal/generator/responses.go | 2 +- .../ooapi/internal/generator/spec.go | 2 +- .../ooapi/internal/generator/swaggertest.go | 2 +- .../ooapi/internal/generator/writefile.go | 0 .../ooapi/internal/openapi/openapi.go | 0 internal/{engine => }/ooapi/login.go | 2 +- internal/{engine => }/ooapi/login_test.go | 294 +++++++++++------- .../{engine => }/ooapi/loginhandler_test.go | 16 +- internal/{engine => }/ooapi/loginmodel.go | 4 +- internal/{engine => }/ooapi/requests.go | 2 +- internal/{engine => }/ooapi/responses.go | 2 +- internal/{engine => }/ooapi/swagger_test.go | 0 .../{engine => }/ooapi/swaggerdiff_test.go | 2 +- internal/{engine => }/ooapi/utils.go | 0 internal/{engine => }/ooapi/utils_test.go | 0 .../internal => }/platform/platform.go | 5 + .../internal => }/platform/platform_cgo.go | 0 .../platform/platform_otherwise.go | 0 .../internal => }/platform/platform_test.go | 0 internal/{engine/internal => }/randx/randx.go | 10 +- .../{engine/internal => }/randx/randx_test.go | 16 +- internal/{engine => }/runtimex/runtimex.go | 6 +- .../{engine => }/runtimex/runtimex_test.go | 2 +- internal/{engine => }/shellx/shellx.go | 43 ++- internal/{engine => }/shellx/shellx_test.go | 0 pkg/oonimkall/experiment_test.go | 14 +- pkg/oonimkall/internal/tasks/runner.go | 7 +- pkg/oonimkall/session.go | 11 +- pkg/oonimkall/sessioncontext_test.go | 12 +- pkg/oonimkall/task.go | 8 +- pkg/oonimkall/webconnectivity.go | 2 +- pkg/oonimkall/webconnectivity_test.go | 41 ++- 169 files changed, 1137 insertions(+), 1004 deletions(-) create mode 100644 internal/atomicx/atomicx.go create mode 100644 internal/atomicx/atomicx_test.go delete mode 100644 internal/engine/atomicx/README.md delete mode 100644 internal/engine/atomicx/atomicx.go delete mode 100644 internal/engine/atomicx/atomicx_test.go delete mode 100644 internal/engine/humanizex/humanizex_test.go delete mode 100644 internal/engine/internal/fsx/fsx_test.go delete mode 100644 internal/engine/internal/sessionresolver/memkvstore.go delete mode 100644 internal/engine/internal/sessionresolver/memkvstore_test.go delete mode 100644 internal/engine/kvstore/kvstore.go delete mode 100644 internal/engine/kvstore_test.go delete mode 100644 internal/engine/ooapi/README.md delete mode 100644 internal/engine/ooapi/kvstore_test.go rename internal/{engine/internal => }/fsx/fsx.go (55%) create mode 100644 internal/fsx/fsx_test.go rename internal/{engine/internal => }/fsx/testdata/.gitignore (100%) rename internal/{engine/internal => }/fsx/testdata/testfile.txt (100%) rename internal/{engine/humanizex => humanize}/humanizex.go (57%) create mode 100644 internal/humanize/humanizex_test.go create mode 100644 internal/kvstore/fs.go create mode 100644 internal/kvstore/fs_test.go create mode 100644 internal/kvstore/memory.go rename internal/{engine/kvstore/kvstore_test.go => kvstore/memory_test.go} (81%) rename internal/{engine/internal => }/multierror/multierror.go (85%) rename internal/{engine/internal => }/multierror/multierror_test.go (96%) rename internal/{engine => }/ooapi/apimodel/checkin.go (100%) rename internal/{engine => }/ooapi/apimodel/checkreportid.go (100%) rename internal/{engine => }/ooapi/apimodel/doc.go (100%) rename internal/{engine => }/ooapi/apimodel/login.go (100%) rename internal/{engine => }/ooapi/apimodel/measurementmeta.go (100%) rename internal/{engine => }/ooapi/apimodel/openreport.go (100%) rename internal/{engine => }/ooapi/apimodel/psiphonconfig.go (100%) rename internal/{engine => }/ooapi/apimodel/register.go (100%) rename internal/{engine => }/ooapi/apimodel/submitmeasurement.go (100%) rename internal/{engine => }/ooapi/apimodel/testhelpers.go (100%) rename internal/{engine => }/ooapi/apimodel/tortargets.go (100%) rename internal/{engine => }/ooapi/apimodel/urls.go (100%) rename internal/{engine => }/ooapi/apis.go (99%) rename internal/{engine => }/ooapi/apis_test.go (99%) rename internal/{engine => }/ooapi/caching.go (97%) rename internal/{engine => }/ooapi/caching_test.go (94%) rename internal/{engine => }/ooapi/callers.go (97%) rename internal/{engine => }/ooapi/client.go (100%) rename internal/{engine => }/ooapi/clientcall.go (98%) rename internal/{engine => }/ooapi/clientcall_test.go (96%) rename internal/{engine => }/ooapi/cloners.go (100%) rename internal/{engine => }/ooapi/default.go (100%) rename internal/{engine => }/ooapi/default_test.go (100%) rename internal/{engine => }/ooapi/dependencies.go (100%) rename internal/{engine => }/ooapi/doc.go (100%) rename internal/{engine => }/ooapi/errors.go (100%) rename internal/{engine => }/ooapi/fake_test.go (100%) rename internal/{engine => }/ooapi/fakeapi_test.go (80%) rename internal/{engine => }/ooapi/fakefill_test.go (98%) rename internal/{engine => }/ooapi/httpclient_test.go (100%) rename internal/{engine => }/ooapi/integration_test.go (85%) rename internal/{engine => }/ooapi/internal/generator/apis.go (98%) rename internal/{engine => }/ooapi/internal/generator/apistest.go (99%) rename internal/{engine => }/ooapi/internal/generator/caching.go (98%) rename internal/{engine => }/ooapi/internal/generator/cachingtest.go (95%) rename internal/{engine => }/ooapi/internal/generator/callers.go (91%) rename internal/{engine => }/ooapi/internal/generator/clientcall.go (97%) rename internal/{engine => }/ooapi/internal/generator/clientcalltest.go (96%) rename internal/{engine => }/ooapi/internal/generator/cloners.go (100%) rename internal/{engine => }/ooapi/internal/generator/fakeapitest.go (83%) rename internal/{engine => }/ooapi/internal/generator/generator.go (100%) rename internal/{engine => }/ooapi/internal/generator/login.go (98%) rename internal/{engine => }/ooapi/internal/generator/logintest.go (86%) rename internal/{engine => }/ooapi/internal/generator/reflect.go (100%) rename internal/{engine => }/ooapi/internal/generator/requests.go (97%) rename internal/{engine => }/ooapi/internal/generator/responses.go (96%) rename internal/{engine => }/ooapi/internal/generator/spec.go (98%) rename internal/{engine => }/ooapi/internal/generator/swaggertest.go (98%) rename internal/{engine => }/ooapi/internal/generator/writefile.go (100%) rename internal/{engine => }/ooapi/internal/openapi/openapi.go (100%) rename internal/{engine => }/ooapi/login.go (99%) rename internal/{engine => }/ooapi/login_test.go (84%) rename internal/{engine => }/ooapi/loginhandler_test.go (94%) rename internal/{engine => }/ooapi/loginmodel.go (93%) rename internal/{engine => }/ooapi/requests.go (98%) rename internal/{engine => }/ooapi/responses.go (99%) rename internal/{engine => }/ooapi/swagger_test.go (100%) rename internal/{engine => }/ooapi/swaggerdiff_test.go (98%) rename internal/{engine => }/ooapi/utils.go (100%) rename internal/{engine => }/ooapi/utils_test.go (100%) rename internal/{engine/internal => }/platform/platform.go (98%) rename internal/{engine/internal => }/platform/platform_cgo.go (100%) rename internal/{engine/internal => }/platform/platform_otherwise.go (100%) rename internal/{engine/internal => }/platform/platform_test.go (100%) rename internal/{engine/internal => }/randx/randx.go (79%) rename internal/{engine/internal => }/randx/randx_test.go (63%) rename internal/{engine => }/runtimex/runtimex.go (60%) rename internal/{engine => }/runtimex/runtimex_test.go (88%) rename internal/{engine => }/shellx/shellx.go (55%) rename internal/{engine => }/shellx/shellx_test.go (100%) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d0f9597..9634de3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -55,6 +55,12 @@ is documented. At the minimum document all the exported symbols. Make sure you commit `go.mod` and `go.sum` changes. Make sure you run `go mod tidy` to minimize such changes. +## Implementation requirements + +Please, use `./internal/atomicx` rather than `atomic/sync`. + +Do now use `os/exec`, use `x/sys/execabs`. + ## Code testing requirements Make sure all tests pass with `go test -race ./...` run from the diff --git a/cmd/ooniprobe/internal/autorun/autorun_darwin.go b/cmd/ooniprobe/internal/autorun/autorun_darwin.go index 80225f3..4f2499b 100644 --- a/cmd/ooniprobe/internal/autorun/autorun_darwin.go +++ b/cmd/ooniprobe/internal/autorun/autorun_darwin.go @@ -13,7 +13,7 @@ import ( "github.com/apex/log" "github.com/ooni/probe-cli/v3/cmd/ooniprobe/internal/utils" - "github.com/ooni/probe-cli/v3/internal/engine/shellx" + "github.com/ooni/probe-cli/v3/internal/shellx" "golang.org/x/sys/execabs" "golang.org/x/sys/unix" ) diff --git a/cmd/ooniprobe/internal/nettests/dnscheck.go b/cmd/ooniprobe/internal/nettests/dnscheck.go index 02cf963..5fba9a9 100644 --- a/cmd/ooniprobe/internal/nettests/dnscheck.go +++ b/cmd/ooniprobe/internal/nettests/dnscheck.go @@ -5,7 +5,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/experiment/dnscheck" "github.com/ooni/probe-cli/v3/internal/engine/experiment/run" - "github.com/ooni/probe-cli/v3/internal/engine/runtimex" + "github.com/ooni/probe-cli/v3/internal/runtimex" ) // DNSCheck nettest implementation. diff --git a/cmd/ooniprobe/internal/ooni/ooni.go b/cmd/ooniprobe/internal/ooni/ooni.go index 41fd682..34658dd 100644 --- a/cmd/ooniprobe/internal/ooni/ooni.go +++ b/cmd/ooniprobe/internal/ooni/ooni.go @@ -6,7 +6,6 @@ import ( "io/ioutil" "os" "os/signal" - "sync/atomic" "syscall" "github.com/apex/log" @@ -14,8 +13,10 @@ import ( "github.com/ooni/probe-cli/v3/cmd/ooniprobe/internal/database" "github.com/ooni/probe-cli/v3/cmd/ooniprobe/internal/enginex" "github.com/ooni/probe-cli/v3/cmd/ooniprobe/internal/utils" - engine "github.com/ooni/probe-cli/v3/internal/engine" + "github.com/ooni/probe-cli/v3/internal/atomicx" + "github.com/ooni/probe-cli/v3/internal/engine" "github.com/ooni/probe-cli/v3/internal/engine/legacy/assetsdir" + "github.com/ooni/probe-cli/v3/internal/kvstore" "github.com/pkg/errors" "upper.io/db.v3/lib/sqlbuilder" ) @@ -53,11 +54,7 @@ type Probe struct { dbPath string configPath string - // We need to use a int32 in order to use the atomic.AddInt32/LoadInt32 - // operations to ensure consistent reads of the variables. We do not use - // a 64 bit integer here because that may lead to crashes with 32 bit - // OSes as documented in https://golang.org/pkg/sync/atomic/#pkg-note-BUG. - isTerminatedAtomicInt int32 + isTerminated *atomicx.Int64 softwareName string softwareVersion string @@ -96,13 +93,12 @@ func (p *Probe) TempDir() string { // IsTerminated checks to see if the isTerminatedAtomicInt is set to a non zero // value and therefore we have received the signal to shutdown the running test func (p *Probe) IsTerminated() bool { - i := atomic.LoadInt32(&p.isTerminatedAtomicInt) - return i != 0 + return p.isTerminated.Load() != 0 } // Terminate interrupts the running context func (p *Probe) Terminate() { - atomic.AddInt32(&p.isTerminatedAtomicInt, 1) + p.isTerminated.Add(1) } // ListenForSignals will listen for SIGINT and SIGTERM. When it receives those @@ -203,7 +199,7 @@ func (p *Probe) Init(softwareName, softwareVersion string) error { // current configuration inside the context. The caller must close // the session when done using it, by calling sess.Close(). func (p *Probe) NewSession(ctx context.Context) (*engine.Session, error) { - kvstore, err := engine.NewFileSystemKVStore( + kvstore, err := kvstore.NewFS( utils.EngineDir(p.home), ) if err != nil { @@ -234,10 +230,10 @@ func (p *Probe) NewProbeEngine(ctx context.Context) (ProbeEngine, error) { // NewProbe creates a new probe instance. func NewProbe(configPath string, homePath string) *Probe { return &Probe{ - home: homePath, - config: &config.Config{}, - configPath: configPath, - isTerminatedAtomicInt: 0, + home: homePath, + config: &config.Config{}, + configPath: configPath, + isTerminated: &atomicx.Int64{}, } } diff --git a/internal/atomicx/atomicx.go b/internal/atomicx/atomicx.go new file mode 100644 index 0000000..33ddf1a --- /dev/null +++ b/internal/atomicx/atomicx.go @@ -0,0 +1,43 @@ +// Package atomicx extends sync/atomic. +// +// Sync/atomic fails when using int64 atomic operations on 32 bit platforms +// when the access is not aligned. As specified in the documentation, in +// fact, "it is the caller's responsibility to arrange for 64-bit alignment +// of 64-bit words accessed atomically". For more information on this +// issue, see https://golang.org/pkg/sync/atomic/#pkg-note-BUG. +// +// As explained in CONTRIBUTING.md, probe-cli SHOULD use this package rather +// than sync/atomic to avoid these alignment issues on 32 bit. +// +// It is of course possible to write atomic code using 64 bit variables on a +// 32 bit platform, but that's difficult to do correctly. This package +// provides an easier-to-use interface. We use allocated +// structures protected by a mutex that encapsulate a int64 value. +// +// While there we also added support for atomic float64 operations, again +// by using structures protected by a mutex variable. +package atomicx + +import "sync" + +// Int64 is an int64 with atomic semantics. +type Int64 struct { + // mu provides mutual exclusion. + mu sync.Mutex + + // v is the underlying value. + v int64 +} + +// Add behaves like atomic.AddInt64. +func (i64 *Int64) Add(delta int64) int64 { + i64.mu.Lock() + defer i64.mu.Unlock() + i64.v += delta + return i64.v +} + +// Load behaves like atomic.LoadInt64. +func (i64 *Int64) Load() (v int64) { + return i64.Add(0) +} diff --git a/internal/atomicx/atomicx_test.go b/internal/atomicx/atomicx_test.go new file mode 100644 index 0000000..fec8d87 --- /dev/null +++ b/internal/atomicx/atomicx_test.go @@ -0,0 +1,28 @@ +package atomicx_test + +import ( + "sync" + "testing" + + "github.com/ooni/probe-cli/v3/internal/atomicx" +) + +func TestInt64(t *testing.T) { + v := &atomicx.Int64{} + var wg sync.WaitGroup + // many goroutines update the value in parallel + for i := 0; i < 31; i++ { + wg.Add(1) + go func() { + defer wg.Done() + v.Add(1) + }() + } + wg.Wait() + if v.Add(3) != 34 { + t.Fatal("unexpected result") + } + if v.Load() != 34 { + t.Fatal("unexpected result") + } +} diff --git a/internal/cmd/apitool/main.go b/internal/cmd/apitool/main.go index 61e1e44..69a8eff 100644 --- a/internal/cmd/apitool/main.go +++ b/internal/cmd/apitool/main.go @@ -16,11 +16,11 @@ import ( "os" "github.com/apex/log" - "github.com/ooni/probe-cli/v3/internal/engine/atomicx" + "github.com/ooni/probe-cli/v3/internal/atomicx" "github.com/ooni/probe-cli/v3/internal/engine/httpx" - "github.com/ooni/probe-cli/v3/internal/engine/kvstore" "github.com/ooni/probe-cli/v3/internal/engine/netx" "github.com/ooni/probe-cli/v3/internal/engine/probeservices" + "github.com/ooni/probe-cli/v3/internal/kvstore" "github.com/ooni/probe-cli/v3/internal/version" ) @@ -34,9 +34,9 @@ func newclient() probeservices.Client { Logger: log.Log, UserAgent: ua, }, - LoginCalls: atomicx.NewInt64(), - RegisterCalls: atomicx.NewInt64(), - StateFile: probeservices.NewStateFile(kvstore.NewMemoryKeyValueStore()), + LoginCalls: &atomicx.Int64{}, + RegisterCalls: &atomicx.Int64{}, + StateFile: probeservices.NewStateFile(&kvstore.Memory{}), } } diff --git a/internal/cmd/jafar/iptables/iptables.go b/internal/cmd/jafar/iptables/iptables.go index 7743eea..580e8bf 100644 --- a/internal/cmd/jafar/iptables/iptables.go +++ b/internal/cmd/jafar/iptables/iptables.go @@ -4,7 +4,7 @@ package iptables import ( - "github.com/ooni/probe-cli/v3/internal/engine/runtimex" + "github.com/ooni/probe-cli/v3/internal/runtimex" ) type shell interface { diff --git a/internal/cmd/jafar/iptables/iptables_integration_test.go b/internal/cmd/jafar/iptables/iptables_integration_test.go index 2d10337..659fec2 100644 --- a/internal/cmd/jafar/iptables/iptables_integration_test.go +++ b/internal/cmd/jafar/iptables/iptables_integration_test.go @@ -17,7 +17,7 @@ import ( "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/cmd/jafar/resolver" "github.com/ooni/probe-cli/v3/internal/cmd/jafar/uncensored" - "github.com/ooni/probe-cli/v3/internal/engine/shellx" + "github.com/ooni/probe-cli/v3/internal/shellx" ) func init() { diff --git a/internal/cmd/jafar/iptables/iptables_linux.go b/internal/cmd/jafar/iptables/iptables_linux.go index 058c9b6..1664a66 100644 --- a/internal/cmd/jafar/iptables/iptables_linux.go +++ b/internal/cmd/jafar/iptables/iptables_linux.go @@ -3,8 +3,8 @@ package iptables import ( - "github.com/ooni/probe-cli/v3/internal/engine/runtimex" - "github.com/ooni/probe-cli/v3/internal/engine/shellx" + "github.com/ooni/probe-cli/v3/internal/runtimex" + "github.com/ooni/probe-cli/v3/internal/shellx" ) type linuxShell struct{} diff --git a/internal/cmd/jafar/main.go b/internal/cmd/jafar/main.go index eb4bb41..f7b1941 100644 --- a/internal/cmd/jafar/main.go +++ b/internal/cmd/jafar/main.go @@ -26,8 +26,8 @@ import ( "github.com/ooni/probe-cli/v3/internal/cmd/jafar/resolver" "github.com/ooni/probe-cli/v3/internal/cmd/jafar/tlsproxy" "github.com/ooni/probe-cli/v3/internal/cmd/jafar/uncensored" - "github.com/ooni/probe-cli/v3/internal/engine/runtimex" - "github.com/ooni/probe-cli/v3/internal/engine/shellx" + "github.com/ooni/probe-cli/v3/internal/runtimex" + "github.com/ooni/probe-cli/v3/internal/shellx" ) var ( diff --git a/internal/cmd/jafar/main_test.go b/internal/cmd/jafar/main_test.go index 5c356f1..6d4ac62 100644 --- a/internal/cmd/jafar/main_test.go +++ b/internal/cmd/jafar/main_test.go @@ -7,7 +7,7 @@ import ( "testing" "github.com/ooni/probe-cli/v3/internal/cmd/jafar/iptables" - "github.com/ooni/probe-cli/v3/internal/engine/shellx" + "github.com/ooni/probe-cli/v3/internal/shellx" ) func ensureWeStartOverWithIPTables() { diff --git a/internal/cmd/jafar/uncensored/uncensored.go b/internal/cmd/jafar/uncensored/uncensored.go index 9fce757..ab43125 100644 --- a/internal/cmd/jafar/uncensored/uncensored.go +++ b/internal/cmd/jafar/uncensored/uncensored.go @@ -10,7 +10,7 @@ import ( "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/engine/experiment/urlgetter" "github.com/ooni/probe-cli/v3/internal/engine/netx" - "github.com/ooni/probe-cli/v3/internal/engine/runtimex" + "github.com/ooni/probe-cli/v3/internal/runtimex" ) // Client is DNS, HTTP, and TCP client. diff --git a/internal/cmd/miniooni/libminiooni.go b/internal/cmd/miniooni/libminiooni.go index d023e40..9106883 100644 --- a/internal/cmd/miniooni/libminiooni.go +++ b/internal/cmd/miniooni/libminiooni.go @@ -17,10 +17,11 @@ import ( "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/engine" - "github.com/ooni/probe-cli/v3/internal/engine/humanizex" "github.com/ooni/probe-cli/v3/internal/engine/legacy/assetsdir" "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/netx/selfcensor" + "github.com/ooni/probe-cli/v3/internal/humanize" + "github.com/ooni/probe-cli/v3/internal/kvstore" "github.com/ooni/probe-cli/v3/internal/version" "github.com/pborman/getopt/v2" ) @@ -348,7 +349,7 @@ func MainWithConfiguration(experimentName string, currentOptions Options) { } kvstore2dir := filepath.Join(miniooniDir, "kvstore2") - kvstore, err := engine.NewFileSystemKVStore(kvstore2dir) + kvstore, err := kvstore.NewFS(kvstore2dir) fatalOnError(err, "cannot create kvstore2 directory") tunnelDir := filepath.Join(miniooniDir, "tunnel") @@ -377,8 +378,8 @@ func MainWithConfiguration(experimentName string, currentOptions Options) { defer func() { sess.Close() log.Infof("whole session: recv %s, sent %s", - humanizex.SI(sess.KibiBytesReceived()*1024, "byte"), - humanizex.SI(sess.KibiBytesSent()*1024, "byte"), + humanize.SI(sess.KibiBytesReceived()*1024, "byte"), + humanize.SI(sess.KibiBytesSent()*1024, "byte"), ) }() log.Debugf("miniooni temporary directory: %s", sess.TempDir()) @@ -426,8 +427,8 @@ func MainWithConfiguration(experimentName string, currentOptions Options) { experiment := builder.NewExperiment() defer func() { log.Infof("experiment: recv %s, sent %s", - humanizex.SI(experiment.KibiBytesReceived()*1024, "byte"), - humanizex.SI(experiment.KibiBytesSent()*1024, "byte"), + humanize.SI(experiment.KibiBytesReceived()*1024, "byte"), + humanize.SI(experiment.KibiBytesSent()*1024, "byte"), ) }() diff --git a/internal/cmd/oohelper/internal/client.go b/internal/cmd/oohelper/internal/client.go index 60e98ac..74d665f 100644 --- a/internal/cmd/oohelper/internal/client.go +++ b/internal/cmd/oohelper/internal/client.go @@ -15,7 +15,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/experiment/webconnectivity" "github.com/ooni/probe-cli/v3/internal/engine/httpheader" "github.com/ooni/probe-cli/v3/internal/engine/netx" - "github.com/ooni/probe-cli/v3/internal/engine/runtimex" + "github.com/ooni/probe-cli/v3/internal/runtimex" "github.com/ooni/probe-cli/v3/internal/version" ) diff --git a/internal/cmd/oohelper/internal/fake_test.go b/internal/cmd/oohelper/internal/fake_test.go index dd09aaa..13bcdb9 100644 --- a/internal/cmd/oohelper/internal/fake_test.go +++ b/internal/cmd/oohelper/internal/fake_test.go @@ -8,7 +8,7 @@ import ( "net/http" "time" - "github.com/ooni/probe-cli/v3/internal/engine/atomicx" + "github.com/ooni/probe-cli/v3/internal/atomicx" "github.com/ooni/probe-cli/v3/internal/engine/netx" ) @@ -19,11 +19,11 @@ type FakeResolver struct { } func NewFakeResolverThatFails() FakeResolver { - return FakeResolver{NumFailures: atomicx.NewInt64(), Err: ErrNotFound} + return FakeResolver{NumFailures: &atomicx.Int64{}, Err: ErrNotFound} } func NewFakeResolverWithResult(r []string) FakeResolver { - return FakeResolver{NumFailures: atomicx.NewInt64(), Result: r} + return FakeResolver{NumFailures: &atomicx.Int64{}, Result: r} } var ErrNotFound = &net.DNSError{ diff --git a/internal/cmd/oohelper/oohelper.go b/internal/cmd/oohelper/oohelper.go index 914ea8e..46977bc 100644 --- a/internal/cmd/oohelper/oohelper.go +++ b/internal/cmd/oohelper/oohelper.go @@ -12,7 +12,7 @@ import ( "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/cmd/oohelper/internal" "github.com/ooni/probe-cli/v3/internal/engine/netx" - "github.com/ooni/probe-cli/v3/internal/engine/runtimex" + "github.com/ooni/probe-cli/v3/internal/runtimex" ) var ( diff --git a/internal/cmd/oohelperd/internal/fake_test.go b/internal/cmd/oohelperd/internal/fake_test.go index 4eb77aa..95abe2c 100644 --- a/internal/cmd/oohelperd/internal/fake_test.go +++ b/internal/cmd/oohelperd/internal/fake_test.go @@ -8,7 +8,7 @@ import ( "net/http" "time" - "github.com/ooni/probe-cli/v3/internal/engine/atomicx" + "github.com/ooni/probe-cli/v3/internal/atomicx" "github.com/ooni/probe-cli/v3/internal/engine/netx" ) @@ -19,11 +19,11 @@ type FakeResolver struct { } func NewFakeResolverThatFails() FakeResolver { - return FakeResolver{NumFailures: atomicx.NewInt64(), Err: ErrNotFound} + return FakeResolver{NumFailures: &atomicx.Int64{}, Err: ErrNotFound} } func NewFakeResolverWithResult(r []string) FakeResolver { - return FakeResolver{NumFailures: atomicx.NewInt64(), Result: r} + return FakeResolver{NumFailures: &atomicx.Int64{}, Result: r} } var ErrNotFound = &net.DNSError{ diff --git a/internal/engine/atomicx/README.md b/internal/engine/atomicx/README.md deleted file mode 100644 index 8a8fb95..0000000 --- a/internal/engine/atomicx/README.md +++ /dev/null @@ -1,3 +0,0 @@ -# Package github.com/ooni/probe-engine/atomicx - -Atomic int64/float64 that works also on 32 bit platforms. diff --git a/internal/engine/atomicx/atomicx.go b/internal/engine/atomicx/atomicx.go deleted file mode 100644 index 3c02d88..0000000 --- a/internal/engine/atomicx/atomicx.go +++ /dev/null @@ -1,68 +0,0 @@ -// Package atomicx contains atomic int64/float64 that work also on 32 bit -// platforms. The main reason for rolling out this package is to avoid potential -// crashes when using 32 bit devices where we are atomically accessing a 64 bit -// variable that is not aligned. The solution to this issue is rather crude: use -// a normal variable and protect it using a normal mutex. While this could be -// disappointing in general, it seems fine to be done in our context where -// we mainly use atomic semantics for counting. -package atomicx - -import ( - "sync" -) - -// Int64 is an int64 with atomic semantics. -type Int64 struct { - mu sync.Mutex - v int64 -} - -// NewInt64 creates a new int64 with atomic semantics. -func NewInt64() *Int64 { - return new(Int64) -} - -// Add behaves like atomic.AddInt64 -func (i64 *Int64) Add(delta int64) (newvalue int64) { - i64.mu.Lock() - i64.v += delta - newvalue = i64.v - i64.mu.Unlock() - return -} - -// Load behaves like atomic.LoadInt64 -func (i64 *Int64) Load() (v int64) { - i64.mu.Lock() - v = i64.v - i64.mu.Unlock() - return -} - -// Float64 is an float64 with atomic semantics. -type Float64 struct { - mu sync.Mutex - v float64 -} - -// NewFloat64 creates a new float64 with atomic semantics. -func NewFloat64() *Float64 { - return new(Float64) -} - -// Add behaves like AtomicInt64.Add but for float64 -func (f64 *Float64) Add(delta float64) (newvalue float64) { - f64.mu.Lock() - f64.v += delta - newvalue = f64.v - f64.mu.Unlock() - return -} - -// Load behaves like LoadInt64.Load buf for float64 -func (f64 *Float64) Load() (v float64) { - f64.mu.Lock() - v = f64.v - f64.mu.Unlock() - return -} diff --git a/internal/engine/atomicx/atomicx_test.go b/internal/engine/atomicx/atomicx_test.go deleted file mode 100644 index baab7ff..0000000 --- a/internal/engine/atomicx/atomicx_test.go +++ /dev/null @@ -1,50 +0,0 @@ -package atomicx_test - -import ( - "testing" - "time" - - "github.com/ooni/probe-cli/v3/internal/engine/atomicx" -) - -func TestInt64(t *testing.T) { - // TODO(bassosimone): how to write tests with race conditions - // and be confident that they're WAI? Here I hope this test is - // run with `-race` and I'm doing something that AFAICT will - // be flagged as race if we were not be using mutexes. - v := atomicx.NewInt64() - go func() { - v.Add(17) - }() - go func() { - v.Add(14) - }() - time.Sleep(1 * time.Second) - if v.Add(3) != 34 { - t.Fatal("unexpected result") - } - if v.Load() != 34 { - t.Fatal("unexpected result") - } -} - -func TestFloat64(t *testing.T) { - // TODO(bassosimone): how to write tests with race conditions - // and be confident that they're WAI? Here I hope this test is - // run with `-race` and I'm doing something that AFAICT will - // be flagged as race if we were not be using mutexes. - v := atomicx.NewFloat64() - go func() { - v.Add(17.0) - }() - go func() { - v.Add(14.0) - }() - time.Sleep(1 * time.Second) - if r := v.Add(3); r < 33.9 && r > 34.1 { - t.Fatal("unexpected result") - } - if v.Load() < 33.9 && v.Load() > 34.1 { - t.Fatal("unexpected result") - } -} diff --git a/internal/engine/experiment.go b/internal/engine/experiment.go index cd6ad89..4035af1 100644 --- a/internal/engine/experiment.go +++ b/internal/engine/experiment.go @@ -10,7 +10,6 @@ import ( "time" "github.com/ooni/probe-cli/v3/internal/engine/geolocate" - "github.com/ooni/probe-cli/v3/internal/engine/internal/platform" "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/netx/bytecounter" "github.com/ooni/probe-cli/v3/internal/engine/netx/dialer" @@ -168,7 +167,7 @@ func (e *Experiment) newMeasurement(input string) *model.Measurement { } m.AddAnnotation("engine_name", "ooniprobe-engine") m.AddAnnotation("engine_version", version.Version) - m.AddAnnotation("platform", platform.Name()) + m.AddAnnotation("platform", e.session.Platform()) return m } diff --git a/internal/engine/experiment/dash/dash.go b/internal/engine/experiment/dash/dash.go index 5fdfd14..5b285bd 100644 --- a/internal/engine/experiment/dash/dash.go +++ b/internal/engine/experiment/dash/dash.go @@ -15,11 +15,11 @@ import ( "time" "github.com/montanaflynn/stats" - "github.com/ooni/probe-cli/v3/internal/engine/humanizex" "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/netx" "github.com/ooni/probe-cli/v3/internal/engine/netx/errorx" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" + "github.com/ooni/probe-cli/v3/internal/humanize" ) const ( @@ -181,7 +181,7 @@ func (r runner) measure( total += current.Received avgspeed := 8 * float64(total) / time.Now().Sub(begin).Seconds() percentage := float64(current.Iteration) / float64(numIterations) - message := fmt.Sprintf("streaming: speed: %s", humanizex.SI(avgspeed, "bit/s")) + message := fmt.Sprintf("streaming: speed: %s", humanize.SI(avgspeed, "bit/s")) r.callbacks.OnProgress(percentage, message) current.Iteration++ speed := float64(current.Received) / float64(current.Elapsed) diff --git a/internal/engine/experiment/dnscheck/dnscheck.go b/internal/engine/experiment/dnscheck/dnscheck.go index 6e8906c..197ce09 100644 --- a/internal/engine/experiment/dnscheck/dnscheck.go +++ b/internal/engine/experiment/dnscheck/dnscheck.go @@ -11,15 +11,15 @@ import ( "net/url" "strings" "sync" - "sync/atomic" "time" + "github.com/ooni/probe-cli/v3/internal/atomicx" "github.com/ooni/probe-cli/v3/internal/engine/experiment/urlgetter" "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/netx" "github.com/ooni/probe-cli/v3/internal/engine/netx/archival" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" - "github.com/ooni/probe-cli/v3/internal/engine/runtimex" + "github.com/ooni/probe-cli/v3/internal/runtimex" ) const ( @@ -31,7 +31,7 @@ const ( // Endpoints keeps track of repeatedly measured endpoints. type Endpoints struct { WaitTime time.Duration - count uint32 + count *atomicx.Int64 nextVisit map[string]time.Time mu sync.Mutex } @@ -48,7 +48,10 @@ func (e *Endpoints) maybeSleep(resolverURL string, logger model.Logger) { return } sleepTime := nextTime.Sub(now) - atomic.AddUint32(&e.count, 1) + if e.count == nil { + e.count = &atomicx.Int64{} + } + e.count.Add(1) logger.Infof("waiting %v before testing %s again", sleepTime, resolverURL) time.Sleep(sleepTime) } diff --git a/internal/engine/experiment/dnscheck/dnscheck_test.go b/internal/engine/experiment/dnscheck/dnscheck_test.go index 09de9d5..daec92d 100644 --- a/internal/engine/experiment/dnscheck/dnscheck_test.go +++ b/internal/engine/experiment/dnscheck/dnscheck_test.go @@ -4,7 +4,6 @@ import ( "context" "errors" "net/url" - "sync/atomic" "testing" "time" @@ -221,7 +220,7 @@ func TestDNSCheckWait(t *testing.T) { } run("dot://one.one.one.one") run("dot://1dot1dot1dot1.cloudflare-dns.com") - if atomic.LoadUint32(&endpoints.count) < 1 { + if endpoints.count.Load() < 1 { t.Fatal("did not sleep") } } diff --git a/internal/engine/experiment/hhfm/hhfm.go b/internal/engine/experiment/hhfm/hhfm.go index dce7924..2015a9f 100644 --- a/internal/engine/experiment/hhfm/hhfm.go +++ b/internal/engine/experiment/hhfm/hhfm.go @@ -17,7 +17,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/experiment/urlgetter" "github.com/ooni/probe-cli/v3/internal/engine/httpheader" - "github.com/ooni/probe-cli/v3/internal/engine/internal/randx" + "github.com/ooni/probe-cli/v3/internal/randx" "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/netx" "github.com/ooni/probe-cli/v3/internal/engine/netx/archival" diff --git a/internal/engine/experiment/hirl/hirl.go b/internal/engine/experiment/hirl/hirl.go index 6fa3f5c..8ec1134 100644 --- a/internal/engine/experiment/hirl/hirl.go +++ b/internal/engine/experiment/hirl/hirl.go @@ -11,7 +11,7 @@ import ( "strings" "time" - "github.com/ooni/probe-cli/v3/internal/engine/internal/randx" + "github.com/ooni/probe-cli/v3/internal/randx" "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/netx" "github.com/ooni/probe-cli/v3/internal/engine/netx/archival" diff --git a/internal/engine/experiment/ndt7/ndt7.go b/internal/engine/experiment/ndt7/ndt7.go index 5fe3982..0c39336 100644 --- a/internal/engine/experiment/ndt7/ndt7.go +++ b/internal/engine/experiment/ndt7/ndt7.go @@ -11,10 +11,10 @@ import ( "net/http" "time" - "github.com/ooni/probe-cli/v3/internal/engine/humanizex" "github.com/ooni/probe-cli/v3/internal/engine/internal/mlablocatev2" "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/netx" + "github.com/ooni/probe-cli/v3/internal/humanize" ) const ( @@ -127,7 +127,7 @@ func (m *Measurer) doDownload( // 50% of the whole experiment, hence the `/2.0`. percentage := elapsed / paramMaxRuntimeUpperBound / 2.0 speed := float64(count) * 8.0 / elapsed - message := fmt.Sprintf(" download: speed %s", humanizex.SI( + message := fmt.Sprintf(" download: speed %s", humanize.SI( float64(speed), "bit/s")) tk.Summary.Download = speed / 1e03 /* bit/s => kbit/s */ callbacks.OnProgress(percentage, message) @@ -197,7 +197,7 @@ func (m *Measurer) doUpload( // the whole experiment, hence `0.5 +` and `/2.0`. percentage := 0.5 + elapsed/paramMaxRuntimeUpperBound/2.0 speed := float64(count) * 8.0 / elapsed - message := fmt.Sprintf(" upload: speed %s", humanizex.SI( + message := fmt.Sprintf(" upload: speed %s", humanize.SI( float64(speed), "bit/s")) tk.Summary.Upload = speed / 1e03 /* bit/s => kbit/s */ callbacks.OnProgress(percentage, message) diff --git a/internal/engine/experiment/psiphon/psiphon_test.go b/internal/engine/experiment/psiphon/psiphon_test.go index d136db6..c33e8ef 100644 --- a/internal/engine/experiment/psiphon/psiphon_test.go +++ b/internal/engine/experiment/psiphon/psiphon_test.go @@ -8,7 +8,7 @@ import ( "time" "github.com/apex/log" - "github.com/ooni/probe-cli/v3/internal/engine/atomicx" + "github.com/ooni/probe-cli/v3/internal/atomicx" "github.com/ooni/probe-cli/v3/internal/engine/experiment/psiphon" "github.com/ooni/probe-cli/v3/internal/engine/experiment/urlgetter" "github.com/ooni/probe-cli/v3/internal/engine/internal/mockable" @@ -83,7 +83,7 @@ func TestRunWillPrintSomethingWithCancelledContext(t *testing.T) { time.Sleep(2 * time.Second) cancel() // fail after we've given the printer a chance to run } - observer := observerCallbacks{progress: atomicx.NewInt64()} + observer := observerCallbacks{progress: &atomicx.Int64{}} err := measurer.Run(ctx, newfakesession(), measurement, observer) if !errors.Is(err, context.Canceled) { t.Fatal("expected another error here") diff --git a/internal/engine/experiment/telegram/telegram_test.go b/internal/engine/experiment/telegram/telegram_test.go index f1afcc3..3a83813 100644 --- a/internal/engine/experiment/telegram/telegram_test.go +++ b/internal/engine/experiment/telegram/telegram_test.go @@ -7,7 +7,7 @@ import ( "testing" "github.com/apex/log" - "github.com/ooni/probe-cli/v3/internal/engine/atomicx" + "github.com/ooni/probe-cli/v3/internal/atomicx" "github.com/ooni/probe-cli/v3/internal/engine/experiment/telegram" "github.com/ooni/probe-cli/v3/internal/engine/experiment/urlgetter" "github.com/ooni/probe-cli/v3/internal/engine/internal/mockable" @@ -271,9 +271,9 @@ func TestUpdateWebWithMixedResults(t *testing.T) { } func TestWeConfigureWebChecksToFailOnHTTPError(t *testing.T) { - called := atomicx.NewInt64() - failOnErrorHTTPS := atomicx.NewInt64() - failOnErrorHTTP := atomicx.NewInt64() + called := &atomicx.Int64{} + failOnErrorHTTPS := &atomicx.Int64{} + failOnErrorHTTP := &atomicx.Int64{} measurer := telegram.Measurer{ Config: telegram.Config{}, Getter: func(ctx context.Context, g urlgetter.Getter) (urlgetter.TestKeys, error) { diff --git a/internal/engine/experiment/tlstool/internal/splitter_test.go b/internal/engine/experiment/tlstool/internal/splitter_test.go index f564880..aa318a9 100644 --- a/internal/engine/experiment/tlstool/internal/splitter_test.go +++ b/internal/engine/experiment/tlstool/internal/splitter_test.go @@ -4,7 +4,7 @@ import ( "testing" "github.com/ooni/probe-cli/v3/internal/engine/experiment/tlstool/internal" - "github.com/ooni/probe-cli/v3/internal/engine/internal/randx" + "github.com/ooni/probe-cli/v3/internal/randx" ) func TestSplitter84restSmall(t *testing.T) { diff --git a/internal/engine/experiment/tlstool/tlstool.go b/internal/engine/experiment/tlstool/tlstool.go index cf965f2..3eee7e1 100644 --- a/internal/engine/experiment/tlstool/tlstool.go +++ b/internal/engine/experiment/tlstool/tlstool.go @@ -19,7 +19,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/netx" "github.com/ooni/probe-cli/v3/internal/engine/netx/archival" - "github.com/ooni/probe-cli/v3/internal/engine/runtimex" + "github.com/ooni/probe-cli/v3/internal/runtimex" ) const ( diff --git a/internal/engine/experiment/tor/tor.go b/internal/engine/experiment/tor/tor.go index 09bc51f..cbe9f32 100644 --- a/internal/engine/experiment/tor/tor.go +++ b/internal/engine/experiment/tor/tor.go @@ -12,14 +12,14 @@ import ( "sync" "time" - "github.com/ooni/probe-cli/v3/internal/engine/atomicx" + "github.com/ooni/probe-cli/v3/internal/atomicx" "github.com/ooni/probe-cli/v3/internal/engine/httpheader" "github.com/ooni/probe-cli/v3/internal/engine/legacy/netxlogger" "github.com/ooni/probe-cli/v3/internal/engine/legacy/oonidatamodel" "github.com/ooni/probe-cli/v3/internal/engine/legacy/oonitemplates" "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/netx/errorx" - "github.com/ooni/probe-cli/v3/internal/engine/runtimex" + "github.com/ooni/probe-cli/v3/internal/runtimex" ) const ( @@ -264,7 +264,7 @@ func newResultsCollector( ) *resultsCollector { rc := &resultsCollector{ callbacks: callbacks, - completed: atomicx.NewInt64(), + completed: &atomicx.Int64{}, measurement: measurement, sess: sess, targetresults: make(map[string]TargetResults), diff --git a/internal/engine/experiment/urlgetter/runner.go b/internal/engine/experiment/urlgetter/runner.go index f02600d..3d7522d 100644 --- a/internal/engine/experiment/urlgetter/runner.go +++ b/internal/engine/experiment/urlgetter/runner.go @@ -13,7 +13,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/httpheader" "github.com/ooni/probe-cli/v3/internal/engine/netx" "github.com/ooni/probe-cli/v3/internal/engine/netx/errorx" - "github.com/ooni/probe-cli/v3/internal/engine/runtimex" + "github.com/ooni/probe-cli/v3/internal/runtimex" ) const httpRequestFailed = "http_request_failed" diff --git a/internal/engine/experiment/urlgetter/runner_test.go b/internal/engine/experiment/urlgetter/runner_test.go index fa02c4e..9f9947b 100644 --- a/internal/engine/experiment/urlgetter/runner_test.go +++ b/internal/engine/experiment/urlgetter/runner_test.go @@ -9,7 +9,7 @@ import ( "strings" "testing" - "github.com/ooni/probe-cli/v3/internal/engine/atomicx" + "github.com/ooni/probe-cli/v3/internal/atomicx" "github.com/ooni/probe-cli/v3/internal/engine/experiment/urlgetter" "github.com/ooni/probe-cli/v3/internal/engine/httpheader" ) @@ -235,7 +235,7 @@ func TestRunnerHTTPCannotReadBodyWinsOver400(t *testing.T) { func TestRunnerWeCanForceUserAgent(t *testing.T) { expected := "antani/1.23.4-dev" - found := atomicx.NewInt64() + found := &atomicx.Int64{} server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.Header.Get("User-Agent") == expected { found.Add(1) @@ -262,7 +262,7 @@ func TestRunnerWeCanForceUserAgent(t *testing.T) { func TestRunnerDefaultUserAgent(t *testing.T) { expected := httpheader.UserAgent() - found := atomicx.NewInt64() + found := &atomicx.Int64{} server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.Header.Get("User-Agent") == expected { found.Add(1) diff --git a/internal/engine/experiment/webconnectivity/endpoints.go b/internal/engine/experiment/webconnectivity/endpoints.go index a228798..ac7d072 100644 --- a/internal/engine/experiment/webconnectivity/endpoints.go +++ b/internal/engine/experiment/webconnectivity/endpoints.go @@ -4,7 +4,7 @@ import ( "net" "net/url" - "github.com/ooni/probe-cli/v3/internal/engine/runtimex" + "github.com/ooni/probe-cli/v3/internal/runtimex" ) // EndpointInfo describes a TCP/TLS endpoint. diff --git a/internal/engine/experiment/webconnectivity/endpoints_test.go b/internal/engine/experiment/webconnectivity/endpoints_test.go index 9baffbf..2fe7da8 100644 --- a/internal/engine/experiment/webconnectivity/endpoints_test.go +++ b/internal/engine/experiment/webconnectivity/endpoints_test.go @@ -6,12 +6,12 @@ import ( "testing" "github.com/google/go-cmp/cmp" - "github.com/ooni/probe-cli/v3/internal/engine/atomicx" + "github.com/ooni/probe-cli/v3/internal/atomicx" "github.com/ooni/probe-cli/v3/internal/engine/experiment/webconnectivity" ) func TestNewEndpointPortPanicsWithInvalidScheme(t *testing.T) { - counter := atomicx.NewInt64() + counter := &atomicx.Int64{} var wg sync.WaitGroup wg.Add(1) go func() { @@ -30,7 +30,7 @@ func TestNewEndpointPortPanicsWithInvalidScheme(t *testing.T) { } func TestNewEndpointPortPanicsWithInvalidHost(t *testing.T) { - counter := atomicx.NewInt64() + counter := &atomicx.Int64{} var wg sync.WaitGroup wg.Add(1) go func() { diff --git a/internal/engine/experiment/webconnectivity/httpanalysis_test.go b/internal/engine/experiment/webconnectivity/httpanalysis_test.go index fc69b1b..4c0c0f0 100644 --- a/internal/engine/experiment/webconnectivity/httpanalysis_test.go +++ b/internal/engine/experiment/webconnectivity/httpanalysis_test.go @@ -6,7 +6,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/ooni/probe-cli/v3/internal/engine/experiment/urlgetter" "github.com/ooni/probe-cli/v3/internal/engine/experiment/webconnectivity" - "github.com/ooni/probe-cli/v3/internal/engine/internal/randx" + "github.com/ooni/probe-cli/v3/internal/randx" "github.com/ooni/probe-cli/v3/internal/engine/netx/archival" ) diff --git a/internal/engine/experiment/whatsapp/whatsapp.go b/internal/engine/experiment/whatsapp/whatsapp.go index eac221d..cb560a9 100644 --- a/internal/engine/experiment/whatsapp/whatsapp.go +++ b/internal/engine/experiment/whatsapp/whatsapp.go @@ -15,7 +15,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/experiment/urlgetter" "github.com/ooni/probe-cli/v3/internal/engine/internal/httpfailure" "github.com/ooni/probe-cli/v3/internal/engine/model" - "github.com/ooni/probe-cli/v3/internal/engine/runtimex" + "github.com/ooni/probe-cli/v3/internal/runtimex" ) const ( diff --git a/internal/engine/experiment/whatsapp/whatsapp_test.go b/internal/engine/experiment/whatsapp/whatsapp_test.go index 89f13eb..abbc544 100644 --- a/internal/engine/experiment/whatsapp/whatsapp_test.go +++ b/internal/engine/experiment/whatsapp/whatsapp_test.go @@ -9,7 +9,7 @@ import ( "github.com/apex/log" "github.com/google/go-cmp/cmp" - "github.com/ooni/probe-cli/v3/internal/engine/atomicx" + "github.com/ooni/probe-cli/v3/internal/atomicx" "github.com/ooni/probe-cli/v3/internal/engine/experiment/urlgetter" "github.com/ooni/probe-cli/v3/internal/engine/experiment/whatsapp" "github.com/ooni/probe-cli/v3/internal/engine/internal/httpfailure" @@ -555,7 +555,7 @@ func TestTestKeysOnlyWebHTTPFailureTooManyURLs(t *testing.T) { } func TestWeConfigureWebChecksCorrectly(t *testing.T) { - called := atomicx.NewInt64() + called := &atomicx.Int64{} emptyConfig := urlgetter.Config{} configWithFailOnHTTPError := urlgetter.Config{FailOnHTTPError: true} configWithNoFollowRedirects := urlgetter.Config{NoFollowRedirects: true} diff --git a/internal/engine/geolocate/geolocate.go b/internal/engine/geolocate/geolocate.go index 443eaed..d951e58 100644 --- a/internal/engine/geolocate/geolocate.go +++ b/internal/engine/geolocate/geolocate.go @@ -7,7 +7,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/netx" - "github.com/ooni/probe-cli/v3/internal/engine/runtimex" + "github.com/ooni/probe-cli/v3/internal/runtimex" "github.com/ooni/probe-cli/v3/internal/version" ) diff --git a/internal/engine/geolocate/iplookup.go b/internal/engine/geolocate/iplookup.go index 01462bd..994055a 100644 --- a/internal/engine/geolocate/iplookup.go +++ b/internal/engine/geolocate/iplookup.go @@ -9,7 +9,7 @@ import ( "net/http" "time" - "github.com/ooni/probe-cli/v3/internal/engine/internal/multierror" + "github.com/ooni/probe-cli/v3/internal/multierror" "github.com/ooni/probe-cli/v3/internal/engine/netx" ) diff --git a/internal/engine/humanizex/humanizex_test.go b/internal/engine/humanizex/humanizex_test.go deleted file mode 100644 index 75d0c2d..0000000 --- a/internal/engine/humanizex/humanizex_test.go +++ /dev/null @@ -1,34 +0,0 @@ -package humanizex_test - -import ( - "testing" - - "github.com/ooni/probe-cli/v3/internal/engine/humanizex" -) - -func TestGood(t *testing.T) { - if humanizex.SI(128, "bit/s") != "128 bit/s" { - t.Fatal("unexpected result") - } - if humanizex.SI(1280, "bit/s") != " 1 kbit/s" { - t.Fatal("unexpected result") - } - if humanizex.SI(12800, "bit/s") != " 13 kbit/s" { - t.Fatal("unexpected result") - } - if humanizex.SI(128000, "bit/s") != "128 kbit/s" { - t.Fatal("unexpected result") - } - if humanizex.SI(1280000, "bit/s") != " 1 Mbit/s" { - t.Fatal("unexpected result") - } - if humanizex.SI(12800000, "bit/s") != " 13 Mbit/s" { - t.Fatal("unexpected result") - } - if humanizex.SI(128000000, "bit/s") != "128 Mbit/s" { - t.Fatal("unexpected result") - } - if humanizex.SI(1280000000, "bit/s") != " 1 Gbit/s" { - t.Fatal("unexpected result") - } -} diff --git a/internal/engine/inputloader.go b/internal/engine/inputloader.go index d3d3d6e..3e4640d 100644 --- a/internal/engine/inputloader.go +++ b/internal/engine/inputloader.go @@ -8,8 +8,8 @@ import ( "io/fs" "github.com/apex/log" - "github.com/ooni/probe-cli/v3/internal/engine/internal/fsx" "github.com/ooni/probe-cli/v3/internal/engine/model" + "github.com/ooni/probe-cli/v3/internal/fsx" ) // These errors are returned by the InputLoader. @@ -154,7 +154,7 @@ func (il *InputLoader) loadLocal() ([]model.URLInfo, error) { inputs = append(inputs, model.URLInfo{URL: input}) } for _, filepath := range il.SourceFiles { - extra, err := il.readfile(filepath, fsx.Open) + extra, err := il.readfile(filepath, fsx.OpenFile) if err != nil { return nil, err } diff --git a/internal/engine/inputloader_network_test.go b/internal/engine/inputloader_network_test.go index 729a869..5fba6e4 100644 --- a/internal/engine/inputloader_network_test.go +++ b/internal/engine/inputloader_network_test.go @@ -6,8 +6,8 @@ import ( "github.com/apex/log" engine "github.com/ooni/probe-cli/v3/internal/engine" - "github.com/ooni/probe-cli/v3/internal/engine/kvstore" "github.com/ooni/probe-cli/v3/internal/engine/model" + "github.com/ooni/probe-cli/v3/internal/kvstore" ) func TestInputLoaderInputOrQueryBackendWithNoInput(t *testing.T) { @@ -19,7 +19,7 @@ func TestInputLoaderInputOrQueryBackendWithNoInput(t *testing.T) { Address: "https://ams-pg-test.ooni.org/", Type: "https", }}, - KVStore: kvstore.NewMemoryKeyValueStore(), + KVStore: &kvstore.Memory{}, Logger: log.Log, SoftwareName: "miniooni", SoftwareVersion: "0.1.0-dev", diff --git a/internal/engine/inputloader_test.go b/internal/engine/inputloader_test.go index 9b2ee35..fb3e940 100644 --- a/internal/engine/inputloader_test.go +++ b/internal/engine/inputloader_test.go @@ -11,8 +11,8 @@ import ( "github.com/apex/log" "github.com/google/go-cmp/cmp" - "github.com/ooni/probe-cli/v3/internal/engine/kvstore" "github.com/ooni/probe-cli/v3/internal/engine/model" + "github.com/ooni/probe-cli/v3/internal/kvstore" ) func TestInputLoaderInputNoneWithStaticInputs(t *testing.T) { @@ -237,7 +237,7 @@ func TestInputLoaderInputOrQueryBackendWithInput(t *testing.T) { func TestInputLoaderInputOrQueryBackendWithNoInputAndCancelledContext(t *testing.T) { sess, err := NewSession(context.Background(), SessionConfig{ - KVStore: kvstore.NewMemoryKeyValueStore(), + KVStore: &kvstore.Memory{}, Logger: log.Log, SoftwareName: "miniooni", SoftwareVersion: "0.1.0-dev", diff --git a/internal/engine/inputprocessor.go b/internal/engine/inputprocessor.go index 2e890c1..c828cac 100644 --- a/internal/engine/inputprocessor.go +++ b/internal/engine/inputprocessor.go @@ -2,7 +2,6 @@ package engine import ( "context" - "sync/atomic" "time" "github.com/ooni/probe-cli/v3/internal/engine/model" @@ -68,11 +67,6 @@ type InputProcessor struct { // Submitter is the code that will submit measurements // to the OONI collector. Submitter InputProcessorSubmitterWrapper - - // terminatedByMaxRuntime is an internal atomic variabile - // incremented when we're terminated by MaxRuntime. We - // only use this variable when testing. - terminatedByMaxRuntime int32 } // InputProcessorSaverWrapper is InputProcessor's @@ -129,29 +123,41 @@ func (ipsw inputProcessorSubmitterWrapper) Submit( // though is free to choose different policies by configuring // the Experiment, Submitter, and Saver fields properly. func (ip *InputProcessor) Run(ctx context.Context) error { + _, err := ip.run(ctx) + return err +} + +// These are the reasons why run could stop. +const ( + stopNormal = (1 << iota) + stopMaxRuntime +) + +// run is like Run but, in addition to returning an error, it +// also returns the reason why we stopped. +func (ip *InputProcessor) run(ctx context.Context) (int, error) { start := time.Now() for idx, url := range ip.Inputs { if ip.MaxRuntime > 0 && time.Since(start) > ip.MaxRuntime { - atomic.AddInt32(&ip.terminatedByMaxRuntime, 1) - return nil + return stopMaxRuntime, nil } input := url.URL meas, err := ip.Experiment.MeasureWithContext(ctx, idx, input) if err != nil { - return err + return 0, err } meas.AddAnnotations(ip.Annotations) meas.Options = ip.Options err = ip.Submitter.Submit(ctx, idx, meas) if err != nil { - return err + return 0, err } // Note: must be after submission because submission modifies // the measurement to include the report ID. err = ip.Saver.SaveMeasurement(idx, meas) if err != nil { - return err + return 0, err } } - return nil + return stopNormal, nil } diff --git a/internal/engine/inputprocessor_test.go b/internal/engine/inputprocessor_test.go index 461c3bb..abb2beb 100644 --- a/internal/engine/inputprocessor_test.go +++ b/internal/engine/inputprocessor_test.go @@ -150,10 +150,11 @@ func TestInputProcessorGood(t *testing.T) { Submitter: NewInputProcessorSubmitterWrapper(submitter), } ctx := context.Background() - if err := ip.Run(ctx); err != nil { + reason, err := ip.run(ctx) + if err != nil { t.Fatal(err) } - if ip.terminatedByMaxRuntime > 0 { + if reason != stopNormal { t.Fatal("terminated by max runtime!?") } if len(fipe.M) != 2 || len(saver.M) != 2 || len(submitter.M) != 2 { @@ -192,10 +193,11 @@ func TestInputProcessorMaxRuntime(t *testing.T) { Submitter: NewInputProcessorSubmitterWrapper(submitter), } ctx := context.Background() - if err := ip.Run(ctx); err != nil { + reason, err := ip.run(ctx) + if err != nil { t.Fatal(err) } - if ip.terminatedByMaxRuntime <= 0 { + if reason != stopMaxRuntime { t.Fatal("not terminated by max runtime") } } diff --git a/internal/engine/internal/fsx/fsx_test.go b/internal/engine/internal/fsx/fsx_test.go deleted file mode 100644 index e38da71..0000000 --- a/internal/engine/internal/fsx/fsx_test.go +++ /dev/null @@ -1,76 +0,0 @@ -package fsx_test - -import ( - "errors" - "io/fs" - "os" - "sync/atomic" - "syscall" - "testing" - - "github.com/ooni/probe-cli/v3/internal/engine/internal/fsx" -) - -var StateBaseDir = "./testdata/" - -type FailingStatFS struct { - CloseCount *int32 -} - -type FailingStatFile struct { - CloseCount *int32 -} - -var errStatFailed = errors.New("stat failed") - -func (FailingStatFile) Stat() (os.FileInfo, error) { - return nil, errStatFailed -} - -func (f FailingStatFS) Open(pathname string) (fs.File, error) { - return FailingStatFile(f), nil -} - -func (fs FailingStatFile) Close() error { - if fs.CloseCount != nil { - atomic.AddInt32(fs.CloseCount, 1) - } - return nil -} - -func (FailingStatFile) Read([]byte) (int, error) { - return 0, nil -} - -func TestOpenWithFailingStat(t *testing.T) { - var count int32 - _, err := fsx.OpenWithFS(FailingStatFS{CloseCount: &count}, StateBaseDir+"testfile.txt") - if !errors.Is(err, errStatFailed) { - t.Errorf("expected error with invalid FS: %+v", err) - } - if count != 1 { - t.Error("expected counter to be equal to 1") - } -} - -func TestOpenNonexistentFile(t *testing.T) { - _, err := fsx.Open(StateBaseDir + "invalidtestfile.txt") - if !errors.Is(err, syscall.ENOENT) { - t.Errorf("not the error we expected") - } -} - -func TestOpenDirectoryShouldFail(t *testing.T) { - _, err := fsx.Open(StateBaseDir) - if !errors.Is(err, syscall.EISDIR) { - t.Fatalf("not the error we expected: %+v", err) - } -} - -func TestOpeningExistingFileShouldWork(t *testing.T) { - file, err := fsx.Open(StateBaseDir + "testfile.txt") - if err != nil { - t.Fatal(err) - } - defer file.Close() -} diff --git a/internal/engine/internal/mockable/mockable.go b/internal/engine/internal/mockable/mockable.go index 60f256a..f63f50d 100644 --- a/internal/engine/internal/mockable/mockable.go +++ b/internal/engine/internal/mockable/mockable.go @@ -6,9 +6,9 @@ import ( "net/http" "net/url" - "github.com/ooni/probe-cli/v3/internal/engine/kvstore" "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/probeservices" + "github.com/ooni/probe-cli/v3/internal/kvstore" ) // Session allows to mock sessions. @@ -67,7 +67,7 @@ func (sess *Session) FetchURLList( // KeyValueStore returns the configured key-value store. func (sess *Session) KeyValueStore() model.KeyValueStore { - return kvstore.NewMemoryKeyValueStore() + return &kvstore.Memory{} } // Logger implements ExperimentSession.Logger diff --git a/internal/engine/internal/sessionresolver/integration_test.go b/internal/engine/internal/sessionresolver/integration_test.go index 52d9fe0..d9c729a 100644 --- a/internal/engine/internal/sessionresolver/integration_test.go +++ b/internal/engine/internal/sessionresolver/integration_test.go @@ -5,13 +5,16 @@ import ( "testing" "github.com/ooni/probe-cli/v3/internal/engine/internal/sessionresolver" + "github.com/ooni/probe-cli/v3/internal/kvstore" ) func TestSessionResolverGood(t *testing.T) { if testing.Short() { t.Skip("skip test in short mode") } - reso := &sessionresolver.Resolver{} + reso := &sessionresolver.Resolver{ + KVStore: &kvstore.Memory{}, + } defer reso.CloseIdleConnections() if reso.Network() != "sessionresolver" { t.Fatal("unexpected Network") diff --git a/internal/engine/internal/sessionresolver/memkvstore.go b/internal/engine/internal/sessionresolver/memkvstore.go deleted file mode 100644 index daacc1c..0000000 --- a/internal/engine/internal/sessionresolver/memkvstore.go +++ /dev/null @@ -1,43 +0,0 @@ -package sessionresolver - -import ( - "errors" - "fmt" - "sync" -) - -func (r *Resolver) kvstore() KVStore { - defer r.mu.Unlock() - r.mu.Lock() - if r.KVStore == nil { - r.KVStore = &memkvstore{} - } - return r.KVStore -} - -var errMemkvstoreNotFound = errors.New("memkvstore: not found") - -type memkvstore struct { - m map[string][]byte - mu sync.Mutex -} - -func (kvs *memkvstore) Get(key string) ([]byte, error) { - defer kvs.mu.Unlock() - kvs.mu.Lock() - out, good := kvs.m[key] - if !good { - return nil, fmt.Errorf("%w: %s", errMemkvstoreNotFound, key) - } - return out, nil -} - -func (kvs *memkvstore) Set(key string, value []byte) error { - defer kvs.mu.Unlock() - kvs.mu.Lock() - if kvs.m == nil { - kvs.m = make(map[string][]byte) - } - kvs.m[key] = value - return nil -} diff --git a/internal/engine/internal/sessionresolver/memkvstore_test.go b/internal/engine/internal/sessionresolver/memkvstore_test.go deleted file mode 100644 index b924ef4..0000000 --- a/internal/engine/internal/sessionresolver/memkvstore_test.go +++ /dev/null @@ -1,47 +0,0 @@ -package sessionresolver - -import ( - "errors" - "testing" - - "github.com/google/go-cmp/cmp" -) - -func TestKVStoreCustom(t *testing.T) { - kvs := &memkvstore{} - reso := &Resolver{KVStore: kvs} - o := reso.kvstore() - if o != kvs { - t.Fatal("not the kvstore we expected") - } -} - -func TestMemkvstoreGetNotFound(t *testing.T) { - reso := &Resolver{} - key := "antani" - out, err := reso.kvstore().Get(key) - if !errors.Is(err, errMemkvstoreNotFound) { - t.Fatal("not the error we expected", err) - } - if out != nil { - t.Fatal("expected nil here") - } -} - -func TestMemkvstoreRoundTrip(t *testing.T) { - reso := &Resolver{} - key := []string{"antani", "mascetti"} - value := [][]byte{[]byte(`mascetti`), []byte(`antani`)} - for idx := 0; idx < 2; idx++ { - if err := reso.kvstore().Set(key[idx], value[idx]); err != nil { - t.Fatal(err) - } - out, err := reso.kvstore().Get(key[idx]) - if err != nil { - t.Fatal(err) - } - if diff := cmp.Diff(value[idx], out); diff != "" { - t.Fatal(diff) - } - } -} diff --git a/internal/engine/internal/sessionresolver/sessionresolver.go b/internal/engine/internal/sessionresolver/sessionresolver.go index 8e7be14..9180cd2 100644 --- a/internal/engine/internal/sessionresolver/sessionresolver.go +++ b/internal/engine/internal/sessionresolver/sessionresolver.go @@ -33,9 +33,9 @@ import ( "sync" "time" - "github.com/ooni/probe-cli/v3/internal/engine/internal/multierror" "github.com/ooni/probe-cli/v3/internal/engine/netx/bytecounter" - "github.com/ooni/probe-cli/v3/internal/engine/runtimex" + "github.com/ooni/probe-cli/v3/internal/multierror" + "github.com/ooni/probe-cli/v3/internal/runtimex" ) // Resolver is the session resolver. Resolver will try to use @@ -45,6 +45,9 @@ import ( // and therefore we can generally give preference to underlying // DoT/DoH resolvers that work better. // +// Make sure you fill the mandatory fields (indicated below) +// before using this data structure. +// // You MUST NOT modify public fields of this structure once it // has been created, because that MAY lead to data races. // @@ -57,10 +60,9 @@ type Resolver struct { // field is not set, then we won't count the bytes. ByteCounter *bytecounter.Counter - // KVStore is the optional key-value store where you + // KVStore is the MANDATORY key-value store where you // want us to write statistics about which resolver is - // working better in your network. If this field is - // not set, then we'll use a in-memory store. + // working better in your network. KVStore KVStore // Logger is the optional logger you want us to use diff --git a/internal/engine/internal/sessionresolver/sessionresolver_test.go b/internal/engine/internal/sessionresolver/sessionresolver_test.go index 8b46c1f..f62dd2f 100644 --- a/internal/engine/internal/sessionresolver/sessionresolver_test.go +++ b/internal/engine/internal/sessionresolver/sessionresolver_test.go @@ -6,11 +6,12 @@ import ( "net" "net/url" "strings" - "sync/atomic" "testing" "github.com/google/go-cmp/cmp" - "github.com/ooni/probe-cli/v3/internal/engine/internal/multierror" + "github.com/ooni/probe-cli/v3/internal/atomicx" + "github.com/ooni/probe-cli/v3/internal/kvstore" + "github.com/ooni/probe-cli/v3/internal/multierror" ) func TestNetworkWorks(t *testing.T) { @@ -30,7 +31,7 @@ func TestAddressWorks(t *testing.T) { func TestTypicalUsageWithFailure(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) cancel() // fail immediately - reso := &Resolver{} + reso := &Resolver{KVStore: &kvstore.Memory{}} addrs, err := reso.LookupHost(ctx, "ooni.org") if !errors.Is(err, ErrLookupHost) { t.Fatal("not the error we expected", err) @@ -82,6 +83,7 @@ func TestTypicalUsageWithSuccess(t *testing.T) { expected := []string{"8.8.8.8", "8.8.4.4"} ctx := context.Background() reso := &Resolver{ + KVStore: &kvstore.Memory{}, dnsClientMaker: &fakeDNSClientMaker{ reso: &FakeResolver{Data: expected}, }, @@ -252,7 +254,7 @@ func TestMaybeConfusionManyEntries(t *testing.T) { func TestResolverWorksWithProxy(t *testing.T) { var ( - works int32 + works = &atomicx.Int64{} startuperr = make(chan error) listench = make(chan net.Listener) done = make(chan interface{}) @@ -273,7 +275,7 @@ func TestResolverWorksWithProxy(t *testing.T) { // shutdown by the main goroutine. return } - atomic.AddInt32(&works, 1) + works.Add(1) conn.Close() } }() @@ -283,10 +285,13 @@ func TestResolverWorksWithProxy(t *testing.T) { } listener := <-listench // use the proxy - reso := &Resolver{ProxyURL: &url.URL{ - Scheme: "socks5", - Host: listener.Addr().String(), - }} + reso := &Resolver{ + ProxyURL: &url.URL{ + Scheme: "socks5", + Host: listener.Addr().String(), + }, + KVStore: &kvstore.Memory{}, + } ctx := context.Background() addrs, err := reso.LookupHost(ctx, "ooni.org") // cleanly shutdown the listener @@ -299,7 +304,7 @@ func TestResolverWorksWithProxy(t *testing.T) { if addrs != nil { t.Fatal("expected nil addrs") } - if works < 1 { + if works.Load() < 1 { t.Fatal("expected to see a positive number of entries here") } } diff --git a/internal/engine/internal/sessionresolver/state.go b/internal/engine/internal/sessionresolver/state.go index b725f9a..19970c3 100644 --- a/internal/engine/internal/sessionresolver/state.go +++ b/internal/engine/internal/sessionresolver/state.go @@ -18,9 +18,15 @@ type resolverinfo struct { Score float64 } +// ErrNilKVStore indicates that the KVStore is nil. +var ErrNilKVStore = errors.New("sessionresolver: kvstore is nil") + // readstate reads the resolver state from disk func (r *Resolver) readstate() ([]*resolverinfo, error) { - data, err := r.kvstore().Get(storekey) + if r.KVStore == nil { + return nil, ErrNilKVStore + } + data, err := r.KVStore.Get(storekey) if err != nil { return nil, err } @@ -85,9 +91,12 @@ func (r *Resolver) readstatedefault() []*resolverinfo { // writestate writes the state on the kvstore. func (r *Resolver) writestate(ri []*resolverinfo) error { + if r.KVStore == nil { + return ErrNilKVStore + } data, err := r.getCodec().Encode(ri) if err != nil { return err } - return r.kvstore().Set(storekey, data) + return r.KVStore.Set(storekey, data) } diff --git a/internal/engine/internal/sessionresolver/state_test.go b/internal/engine/internal/sessionresolver/state_test.go index 168ee5c..0046325 100644 --- a/internal/engine/internal/sessionresolver/state_test.go +++ b/internal/engine/internal/sessionresolver/state_test.go @@ -3,12 +3,25 @@ package sessionresolver import ( "errors" "testing" + + "github.com/ooni/probe-cli/v3/internal/kvstore" ) -func TestReadStateNothingInKVStore(t *testing.T) { - reso := &Resolver{KVStore: &memkvstore{}} +func TestReadStateNoKVStore(t *testing.T) { + reso := &Resolver{} out, err := reso.readstate() - if !errors.Is(err, errMemkvstoreNotFound) { + if !errors.Is(err, ErrNilKVStore) { + t.Fatal("not the error we expected", err) + } + if out != nil { + t.Fatal("expected nil here") + } +} + +func TestReadStateNothingInKVStore(t *testing.T) { + reso := &Resolver{KVStore: &kvstore.Memory{}} + out, err := reso.readstate() + if !errors.Is(err, kvstore.ErrNoSuchKey) { t.Fatal("not the error we expected", err) } if out != nil { @@ -19,7 +32,7 @@ func TestReadStateNothingInKVStore(t *testing.T) { func TestReadStateDecodeError(t *testing.T) { errMocked := errors.New("mocked error") reso := &Resolver{ - KVStore: &memkvstore{}, + KVStore: &kvstore.Memory{}, codec: &FakeCodec{DecodeErr: errMocked}, } if err := reso.KVStore.Set(storekey, []byte(`[]`)); err != nil { @@ -35,9 +48,9 @@ func TestReadStateDecodeError(t *testing.T) { } func TestReadStateAndPruneReadStateError(t *testing.T) { - reso := &Resolver{KVStore: &memkvstore{}} + reso := &Resolver{KVStore: &kvstore.Memory{}} out, err := reso.readstateandprune() - if !errors.Is(err, errMemkvstoreNotFound) { + if !errors.Is(err, kvstore.ErrNoSuchKey) { t.Fatal("not the error we expected", err) } if out != nil { @@ -46,7 +59,7 @@ func TestReadStateAndPruneReadStateError(t *testing.T) { } func TestReadStateAndPruneWithUnsupportedEntries(t *testing.T) { - reso := &Resolver{KVStore: &memkvstore{}} + reso := &Resolver{KVStore: &kvstore.Memory{}} var in []*resolverinfo in = append(in, &resolverinfo{}) if err := reso.writestate(in); err != nil { @@ -62,7 +75,7 @@ func TestReadStateAndPruneWithUnsupportedEntries(t *testing.T) { } func TestReadStateDefaultWithMissingEntries(t *testing.T) { - reso := &Resolver{KVStore: &memkvstore{}} + reso := &Resolver{KVStore: &kvstore.Memory{}} // let us simulate that we have just one entry here existingURL := "https://dns.google/dns-query" existingScore := 0.88 @@ -100,12 +113,27 @@ func TestReadStateDefaultWithMissingEntries(t *testing.T) { } } +func TestWriteStateNoKVStore(t *testing.T) { + reso := &Resolver{} + existingURL := "https://dns.google/dns-query" + existingScore := 0.88 + var in []*resolverinfo + in = append(in, &resolverinfo{ + URL: existingURL, + Score: existingScore, + }) + if err := reso.writestate(in); !errors.Is(err, ErrNilKVStore) { + t.Fatal("not the error we expected", err) + } +} + func TestWriteStateCannotSerialize(t *testing.T) { errMocked := errors.New("mocked error") reso := &Resolver{ codec: &FakeCodec{ EncodeErr: errMocked, }, + KVStore: &kvstore.Memory{}, } existingURL := "https://dns.google/dns-query" existingScore := 0.88 diff --git a/internal/engine/kvstore.go b/internal/engine/kvstore.go index 2e20229..c744de6 100644 --- a/internal/engine/kvstore.go +++ b/internal/engine/kvstore.go @@ -1,13 +1,5 @@ package engine -import ( - "bytes" - "os" - "path/filepath" - - "github.com/rogpeppe/go-internal/lockedfile" -) - // KVStore is a simple, atomic key-value store. The user of // probe-engine should supply an implementation of this interface, // which will be used by probe-engine to store specific data. @@ -15,30 +7,3 @@ type KVStore interface { Get(key string) (value []byte, err error) Set(key string, value []byte) (err error) } - -// FileSystemKVStore is a directory based KVStore -type FileSystemKVStore struct { - basedir string -} - -// NewFileSystemKVStore creates a new FileSystemKVStore. -func NewFileSystemKVStore(basedir string) (kvs *FileSystemKVStore, err error) { - if err = os.MkdirAll(basedir, 0700); err == nil { - kvs = &FileSystemKVStore{basedir: basedir} - } - return -} - -func (kvs *FileSystemKVStore) filename(key string) string { - return filepath.Join(kvs.basedir, key) -} - -// Get returns the specified key's value -func (kvs *FileSystemKVStore) Get(key string) ([]byte, error) { - return lockedfile.Read(kvs.filename(key)) -} - -// Set sets the value of a specific key -func (kvs *FileSystemKVStore) Set(key string, value []byte) error { - return lockedfile.Write(kvs.filename(key), bytes.NewReader(value), 0600) -} diff --git a/internal/engine/kvstore/kvstore.go b/internal/engine/kvstore/kvstore.go deleted file mode 100644 index c20334c..0000000 --- a/internal/engine/kvstore/kvstore.go +++ /dev/null @@ -1,44 +0,0 @@ -// Package kvstore contains key-value stores -package kvstore - -import ( - "errors" - "sync" -) - -// MemoryKeyValueStore is an in-memory key-value store -type MemoryKeyValueStore struct { - m map[string][]byte - mu sync.Mutex -} - -// NewMemoryKeyValueStore creates a new in-memory key-value store -func NewMemoryKeyValueStore() *MemoryKeyValueStore { - return &MemoryKeyValueStore{ - m: make(map[string][]byte), - } -} - -// Get returns a key from the key value store -func (kvs *MemoryKeyValueStore) Get(key string) ([]byte, error) { - var ( - err error - ok bool - value []byte - ) - kvs.mu.Lock() - defer kvs.mu.Unlock() - value, ok = kvs.m[key] - if !ok { - err = errors.New("no such key") - } - return value, err -} - -// Set sets a key into the key value store -func (kvs *MemoryKeyValueStore) Set(key string, value []byte) error { - kvs.mu.Lock() - defer kvs.mu.Unlock() - kvs.m[key] = value - return nil -} diff --git a/internal/engine/kvstore_test.go b/internal/engine/kvstore_test.go deleted file mode 100644 index 75e48af..0000000 --- a/internal/engine/kvstore_test.go +++ /dev/null @@ -1,31 +0,0 @@ -package engine - -import ( - "bytes" - "path/filepath" - "testing" -) - -func TestKVStoreIntegration(t *testing.T) { - var ( - err error - kvstore KVStore - ) - kvstore, err = NewFileSystemKVStore( - filepath.Join("testdata", "kvstore2"), - ) - if err != nil { - t.Fatal(err) - } - value := []byte("foobar") - if err := kvstore.Set("antani", value); err != nil { - t.Fatal(err) - } - ovalue, err := kvstore.Get("antani") - if err != nil { - t.Fatal(err) - } - if !bytes.Equal(ovalue, value) { - t.Fatal("invalid value") - } -} diff --git a/internal/engine/legacy/netx/dialid/dialid.go b/internal/engine/legacy/netx/dialid/dialid.go index 6cfd464..6c5185b 100644 --- a/internal/engine/legacy/netx/dialid/dialid.go +++ b/internal/engine/legacy/netx/dialid/dialid.go @@ -3,12 +3,12 @@ package dialid import ( "context" - "github.com/ooni/probe-cli/v3/internal/engine/atomicx" + "github.com/ooni/probe-cli/v3/internal/atomicx" ) type contextkey struct{} -var id = atomicx.NewInt64() +var id = &atomicx.Int64{} // WithDialID returns a copy of ctx with DialID func WithDialID(ctx context.Context) context.Context { diff --git a/internal/engine/legacy/netx/handlers/handlers.go b/internal/engine/legacy/netx/handlers/handlers.go index 48f79b2..4ad7248 100644 --- a/internal/engine/legacy/netx/handlers/handlers.go +++ b/internal/engine/legacy/netx/handlers/handlers.go @@ -7,7 +7,7 @@ import ( "sync" "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/modelx" - "github.com/ooni/probe-cli/v3/internal/engine/runtimex" + "github.com/ooni/probe-cli/v3/internal/runtimex" ) type stdoutHandler struct{} diff --git a/internal/engine/legacy/netx/oldhttptransport/tracetripper.go b/internal/engine/legacy/netx/oldhttptransport/tracetripper.go index 8280d5f..baad6dd 100644 --- a/internal/engine/legacy/netx/oldhttptransport/tracetripper.go +++ b/internal/engine/legacy/netx/oldhttptransport/tracetripper.go @@ -10,7 +10,7 @@ import ( "sync" "time" - "github.com/ooni/probe-cli/v3/internal/engine/atomicx" + "github.com/ooni/probe-cli/v3/internal/atomicx" "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/connid" "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/dialid" "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/modelx" @@ -28,7 +28,7 @@ type TraceTripper struct { // NewTraceTripper creates a new Transport. func NewTraceTripper(roundTripper http.RoundTripper) *TraceTripper { return &TraceTripper{ - readAllErrs: atomicx.NewInt64(), + readAllErrs: &atomicx.Int64{}, readAll: ioutil.ReadAll, roundTripper: roundTripper, } diff --git a/internal/engine/legacy/netx/transactionid/transactionid.go b/internal/engine/legacy/netx/transactionid/transactionid.go index 91f5168..6fab055 100644 --- a/internal/engine/legacy/netx/transactionid/transactionid.go +++ b/internal/engine/legacy/netx/transactionid/transactionid.go @@ -4,12 +4,12 @@ package transactionid import ( "context" - "github.com/ooni/probe-cli/v3/internal/engine/atomicx" + "github.com/ooni/probe-cli/v3/internal/atomicx" ) type contextkey struct{} -var id = atomicx.NewInt64() +var id = &atomicx.Int64{} // WithTransactionID returns a copy of ctx with TransactionID func WithTransactionID(ctx context.Context) context.Context { diff --git a/internal/engine/legacy/oonitemplates/oonitemplates.go b/internal/engine/legacy/oonitemplates/oonitemplates.go index 16e7277..cb7ba09 100644 --- a/internal/engine/legacy/oonitemplates/oonitemplates.go +++ b/internal/engine/legacy/oonitemplates/oonitemplates.go @@ -20,11 +20,11 @@ import ( "time" goptlib "git.torproject.org/pluggable-transports/goptlib.git" - "github.com/ooni/probe-cli/v3/internal/engine/atomicx" + "github.com/ooni/probe-cli/v3/internal/atomicx" "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx" "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/handlers" "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/modelx" - "github.com/ooni/probe-cli/v3/internal/engine/runtimex" + "github.com/ooni/probe-cli/v3/internal/runtimex" "gitlab.com/yawning/obfs4.git/transports" obfs4base "gitlab.com/yawning/obfs4.git/transports/base" ) @@ -37,7 +37,7 @@ type channelHandler struct { func newChannelHandler(ch chan<- modelx.Measurement) *channelHandler { return &channelHandler{ ch: ch, - lateWrites: atomicx.NewInt64(), + lateWrites: &atomicx.Int64{}, } } diff --git a/internal/engine/netx/bytecounter/bytecounter.go b/internal/engine/netx/bytecounter/bytecounter.go index 426b874..b1c9a02 100644 --- a/internal/engine/netx/bytecounter/bytecounter.go +++ b/internal/engine/netx/bytecounter/bytecounter.go @@ -1,6 +1,6 @@ package bytecounter -import "github.com/ooni/probe-cli/v3/internal/engine/atomicx" +import "github.com/ooni/probe-cli/v3/internal/atomicx" // Counter counts bytes sent and received. type Counter struct { @@ -10,7 +10,7 @@ type Counter struct { // New creates a new Counter. func New() *Counter { - return &Counter{Received: atomicx.NewInt64(), Sent: atomicx.NewInt64()} + return &Counter{Received: &atomicx.Int64{}, Sent: &atomicx.Int64{}} } // CountBytesSent adds count to the bytes sent counter. diff --git a/internal/engine/netx/netx.go b/internal/engine/netx/netx.go index 25f2365..5a2175a 100644 --- a/internal/engine/netx/netx.go +++ b/internal/engine/netx/netx.go @@ -39,7 +39,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/netx/resolver" "github.com/ooni/probe-cli/v3/internal/engine/netx/selfcensor" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" - "github.com/ooni/probe-cli/v3/internal/engine/runtimex" + "github.com/ooni/probe-cli/v3/internal/runtimex" ) // Logger is the logger assumed by this package diff --git a/internal/engine/netx/resolver/bogon.go b/internal/engine/netx/resolver/bogon.go index 2b4fe50..8103cc3 100644 --- a/internal/engine/netx/resolver/bogon.go +++ b/internal/engine/netx/resolver/bogon.go @@ -5,7 +5,7 @@ import ( "net" "github.com/ooni/probe-cli/v3/internal/engine/netx/errorx" - "github.com/ooni/probe-cli/v3/internal/engine/runtimex" + "github.com/ooni/probe-cli/v3/internal/runtimex" ) var privateIPBlocks []*net.IPNet diff --git a/internal/engine/netx/resolver/fake_test.go b/internal/engine/netx/resolver/fake_test.go index e406381..576e738 100644 --- a/internal/engine/netx/resolver/fake_test.go +++ b/internal/engine/netx/resolver/fake_test.go @@ -6,7 +6,7 @@ import ( "net" "time" - "github.com/ooni/probe-cli/v3/internal/engine/atomicx" + "github.com/ooni/probe-cli/v3/internal/atomicx" ) type FakeDialer struct { @@ -109,11 +109,11 @@ type FakeResolver struct { } func NewFakeResolverThatFails() FakeResolver { - return FakeResolver{NumFailures: atomicx.NewInt64(), Err: errNotFound} + return FakeResolver{NumFailures: &atomicx.Int64{}, Err: errNotFound} } func NewFakeResolverWithResult(r []string) FakeResolver { - return FakeResolver{NumFailures: atomicx.NewInt64(), Result: r} + return FakeResolver{NumFailures: &atomicx.Int64{}, Result: r} } var errNotFound = &net.DNSError{ diff --git a/internal/engine/netx/resolver/serial.go b/internal/engine/netx/resolver/serial.go index 6f82caa..6844df7 100644 --- a/internal/engine/netx/resolver/serial.go +++ b/internal/engine/netx/resolver/serial.go @@ -6,7 +6,7 @@ import ( "net" "github.com/miekg/dns" - "github.com/ooni/probe-cli/v3/internal/engine/atomicx" + "github.com/ooni/probe-cli/v3/internal/atomicx" ) // RoundTripper represents an abstract DNS transport. @@ -38,7 +38,7 @@ func NewSerialResolver(t RoundTripper) SerialResolver { return SerialResolver{ Encoder: MiekgEncoder{}, Decoder: MiekgDecoder{}, - NumTimeouts: atomicx.NewInt64(), + NumTimeouts: &atomicx.Int64{}, Txp: t, } } diff --git a/internal/engine/netx/selfcensor/selfcensor.go b/internal/engine/netx/selfcensor/selfcensor.go index 406ded4..bbe0147 100644 --- a/internal/engine/netx/selfcensor/selfcensor.go +++ b/internal/engine/netx/selfcensor/selfcensor.go @@ -31,7 +31,7 @@ import ( "sync" "time" - "github.com/ooni/probe-cli/v3/internal/engine/atomicx" + "github.com/ooni/probe-cli/v3/internal/atomicx" ) // Spec indicates what self censorship techniques to implement. @@ -61,8 +61,8 @@ type Spec struct { } var ( - attempts *atomicx.Int64 = atomicx.NewInt64() - enabled *atomicx.Int64 = atomicx.NewInt64() + attempts *atomicx.Int64 = &atomicx.Int64{} + enabled *atomicx.Int64 = &atomicx.Int64{} mu sync.Mutex spec *Spec ) diff --git a/internal/engine/ooapi/README.md b/internal/engine/ooapi/README.md deleted file mode 100644 index 63de899..0000000 --- a/internal/engine/ooapi/README.md +++ /dev/null @@ -1,5 +0,0 @@ -# Package ./internal/engine/ooapi - -Automatically generated API clients for speaking with OONI servers. - -Please, run `go doc ./internal/engine/ooapi` to see API documentation. diff --git a/internal/engine/ooapi/kvstore_test.go b/internal/engine/ooapi/kvstore_test.go deleted file mode 100644 index 625c9d7..0000000 --- a/internal/engine/ooapi/kvstore_test.go +++ /dev/null @@ -1,34 +0,0 @@ -package ooapi - -import ( - "errors" - "fmt" - "sync" -) - -var errMemkvstoreNotFound = errors.New("memkvstore: not found") - -type MemKVStore struct { - m map[string][]byte - mu sync.Mutex -} - -func (kvs *MemKVStore) Get(key string) ([]byte, error) { - defer kvs.mu.Unlock() - kvs.mu.Lock() - out, good := kvs.m[key] - if !good { - return nil, fmt.Errorf("%w: %s", errMemkvstoreNotFound, key) - } - return out, nil -} - -func (kvs *MemKVStore) Set(key string, value []byte) error { - defer kvs.mu.Unlock() - kvs.mu.Lock() - if kvs.m == nil { - kvs.m = make(map[string][]byte) - } - kvs.m[key] = value - return nil -} diff --git a/internal/engine/probeservices/checkreportid_test.go b/internal/engine/probeservices/checkreportid_test.go index d3fce64..631713d 100644 --- a/internal/engine/probeservices/checkreportid_test.go +++ b/internal/engine/probeservices/checkreportid_test.go @@ -7,10 +7,10 @@ import ( "testing" "github.com/apex/log" - "github.com/ooni/probe-cli/v3/internal/engine/atomicx" + "github.com/ooni/probe-cli/v3/internal/atomicx" "github.com/ooni/probe-cli/v3/internal/engine/httpx" - "github.com/ooni/probe-cli/v3/internal/engine/kvstore" "github.com/ooni/probe-cli/v3/internal/engine/probeservices" + "github.com/ooni/probe-cli/v3/internal/kvstore" ) func TestCheckReportIDWorkingAsIntended(t *testing.T) { @@ -21,9 +21,9 @@ func TestCheckReportIDWorkingAsIntended(t *testing.T) { Logger: log.Log, UserAgent: "miniooni/0.1.0-dev", }, - LoginCalls: atomicx.NewInt64(), - RegisterCalls: atomicx.NewInt64(), - StateFile: probeservices.NewStateFile(kvstore.NewMemoryKeyValueStore()), + LoginCalls: &atomicx.Int64{}, + RegisterCalls: &atomicx.Int64{}, + StateFile: probeservices.NewStateFile(&kvstore.Memory{}), } reportID := `20201209T052225Z_urlgetter_IT_30722_n1_E1VUhMz08SEkgYFU` ctx := context.Background() @@ -44,9 +44,9 @@ func TestCheckReportIDWorkingWithCancelledContext(t *testing.T) { Logger: log.Log, UserAgent: "miniooni/0.1.0-dev", }, - LoginCalls: atomicx.NewInt64(), - RegisterCalls: atomicx.NewInt64(), - StateFile: probeservices.NewStateFile(kvstore.NewMemoryKeyValueStore()), + LoginCalls: &atomicx.Int64{}, + RegisterCalls: &atomicx.Int64{}, + StateFile: probeservices.NewStateFile(&kvstore.Memory{}), } reportID := `20201209T052225Z_urlgetter_IT_30722_n1_E1VUhMz08SEkgYFU` ctx, cancel := context.WithCancel(context.Background()) diff --git a/internal/engine/probeservices/measurementmeta_test.go b/internal/engine/probeservices/measurementmeta_test.go index c777d5d..6613bf0 100644 --- a/internal/engine/probeservices/measurementmeta_test.go +++ b/internal/engine/probeservices/measurementmeta_test.go @@ -8,10 +8,10 @@ import ( "testing" "github.com/apex/log" - "github.com/ooni/probe-cli/v3/internal/engine/atomicx" + "github.com/ooni/probe-cli/v3/internal/atomicx" "github.com/ooni/probe-cli/v3/internal/engine/httpx" - "github.com/ooni/probe-cli/v3/internal/engine/kvstore" "github.com/ooni/probe-cli/v3/internal/engine/probeservices" + "github.com/ooni/probe-cli/v3/internal/kvstore" ) func TestGetMeasurementMetaWorkingAsIntended(t *testing.T) { @@ -22,9 +22,9 @@ func TestGetMeasurementMetaWorkingAsIntended(t *testing.T) { Logger: log.Log, UserAgent: "miniooni/0.1.0-dev", }, - LoginCalls: atomicx.NewInt64(), - RegisterCalls: atomicx.NewInt64(), - StateFile: probeservices.NewStateFile(kvstore.NewMemoryKeyValueStore()), + LoginCalls: &atomicx.Int64{}, + RegisterCalls: &atomicx.Int64{}, + StateFile: probeservices.NewStateFile(&kvstore.Memory{}), } config := probeservices.MeasurementMetaConfig{ ReportID: `20201209T052225Z_urlgetter_IT_30722_n1_E1VUhMz08SEkgYFU`, @@ -90,9 +90,9 @@ func TestGetMeasurementMetaWorkingWithCancelledContext(t *testing.T) { Logger: log.Log, UserAgent: "miniooni/0.1.0-dev", }, - LoginCalls: atomicx.NewInt64(), - RegisterCalls: atomicx.NewInt64(), - StateFile: probeservices.NewStateFile(kvstore.NewMemoryKeyValueStore()), + LoginCalls: &atomicx.Int64{}, + RegisterCalls: &atomicx.Int64{}, + StateFile: probeservices.NewStateFile(&kvstore.Memory{}), } config := probeservices.MeasurementMetaConfig{ ReportID: `20201209T052225Z_urlgetter_IT_30722_n1_E1VUhMz08SEkgYFU`, diff --git a/internal/engine/probeservices/probeservices.go b/internal/engine/probeservices/probeservices.go index ecbecdc..3a8b477 100644 --- a/internal/engine/probeservices/probeservices.go +++ b/internal/engine/probeservices/probeservices.go @@ -28,7 +28,7 @@ import ( "net/http" "net/url" - "github.com/ooni/probe-cli/v3/internal/engine/atomicx" + "github.com/ooni/probe-cli/v3/internal/atomicx" "github.com/ooni/probe-cli/v3/internal/engine/httpx" "github.com/ooni/probe-cli/v3/internal/engine/model" ) @@ -98,8 +98,8 @@ func NewClient(sess Session, endpoint model.Service) (*Client, error) { ProxyURL: sess.ProxyURL(), UserAgent: sess.UserAgent(), }, - LoginCalls: atomicx.NewInt64(), - RegisterCalls: atomicx.NewInt64(), + LoginCalls: &atomicx.Int64{}, + RegisterCalls: &atomicx.Int64{}, StateFile: NewStateFile(sess.KeyValueStore()), } switch endpoint.Type { diff --git a/internal/engine/probeservices/register.go b/internal/engine/probeservices/register.go index 44dd16a..7b2c31e 100644 --- a/internal/engine/probeservices/register.go +++ b/internal/engine/probeservices/register.go @@ -3,7 +3,7 @@ package probeservices import ( "context" - "github.com/ooni/probe-cli/v3/internal/engine/internal/randx" + "github.com/ooni/probe-cli/v3/internal/randx" ) type registerRequest struct { diff --git a/internal/engine/probeservices/statefile_test.go b/internal/engine/probeservices/statefile_test.go index d274fbf..272b798 100644 --- a/internal/engine/probeservices/statefile_test.go +++ b/internal/engine/probeservices/statefile_test.go @@ -7,8 +7,8 @@ import ( "time" "github.com/google/go-cmp/cmp" - "github.com/ooni/probe-cli/v3/internal/engine/kvstore" "github.com/ooni/probe-cli/v3/internal/engine/probeservices" + "github.com/ooni/probe-cli/v3/internal/kvstore" ) func TestStateAuth(t *testing.T) { @@ -67,7 +67,7 @@ func TestStateCredentials(t *testing.T) { func TestStateFileMemoryIntegration(t *testing.T) { // Does the StateFile have the property that we can write // values into it and then read again the same files? - sf := probeservices.NewStateFile(kvstore.NewMemoryKeyValueStore()) + sf := probeservices.NewStateFile(&kvstore.Memory{}) s := probeservices.State{ Expire: time.Now(), Password: "xy", @@ -85,7 +85,7 @@ func TestStateFileMemoryIntegration(t *testing.T) { } func TestStateFileSetMarshalError(t *testing.T) { - sf := probeservices.NewStateFile(kvstore.NewMemoryKeyValueStore()) + sf := probeservices.NewStateFile(&kvstore.Memory{}) s := probeservices.State{ Expire: time.Now(), Password: "xy", @@ -102,7 +102,7 @@ func TestStateFileSetMarshalError(t *testing.T) { } func TestStateFileGetKVStoreGetError(t *testing.T) { - sf := probeservices.NewStateFile(kvstore.NewMemoryKeyValueStore()) + sf := probeservices.NewStateFile(&kvstore.Memory{}) expected := errors.New("mocked error") failingfunc := func(string) ([]byte, error) { return nil, expected @@ -126,7 +126,7 @@ func TestStateFileGetKVStoreGetError(t *testing.T) { } func TestStateFileGetUnmarshalError(t *testing.T) { - sf := probeservices.NewStateFile(kvstore.NewMemoryKeyValueStore()) + sf := probeservices.NewStateFile(&kvstore.Memory{}) if err := sf.Set(probeservices.State{}); err != nil { t.Fatal(err) } diff --git a/internal/engine/session.go b/internal/engine/session.go index 11e42b9..6db35c2 100644 --- a/internal/engine/session.go +++ b/internal/engine/session.go @@ -10,16 +10,16 @@ import ( "os" "sync" - "github.com/ooni/probe-cli/v3/internal/engine/atomicx" + "github.com/ooni/probe-cli/v3/internal/atomicx" "github.com/ooni/probe-cli/v3/internal/engine/geolocate" - "github.com/ooni/probe-cli/v3/internal/engine/internal/platform" "github.com/ooni/probe-cli/v3/internal/engine/internal/sessionresolver" - "github.com/ooni/probe-cli/v3/internal/engine/kvstore" "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/netx" "github.com/ooni/probe-cli/v3/internal/engine/netx/bytecounter" "github.com/ooni/probe-cli/v3/internal/engine/probeservices" "github.com/ooni/probe-cli/v3/internal/engine/tunnel" + "github.com/ooni/probe-cli/v3/internal/kvstore" + "github.com/ooni/probe-cli/v3/internal/platform" "github.com/ooni/probe-cli/v3/internal/version" ) @@ -52,7 +52,7 @@ type Session struct { availableTestHelpers map[string][]model.Service byteCounter *bytecounter.Counter httpDefaultTransport netx.HTTPRoundTripper - kvStore model.KeyValueStore + kvStore KVStore location *geolocate.Results logger model.Logger proxyURL *url.URL @@ -142,7 +142,7 @@ func NewSession(ctx context.Context, config SessionConfig) (*Session, error) { return nil, errors.New("SoftwareVersion is empty") } if config.KVStore == nil { - config.KVStore = kvstore.NewMemoryKeyValueStore() + config.KVStore = &kvstore.Memory{} } // Implementation note: if config.TempDir is empty, then Go will // use the temporary directory on the current system. This should @@ -157,7 +157,7 @@ func NewSession(ctx context.Context, config SessionConfig) (*Session, error) { byteCounter: bytecounter.New(), kvStore: config.KVStore, logger: config.Logger, - queryProbeServicesCount: atomicx.NewInt64(), + queryProbeServicesCount: &atomicx.Int64{}, softwareName: config.SoftwareName, softwareVersion: config.SoftwareVersion, tempDir: tempDir, diff --git a/internal/engine/submitter_test.go b/internal/engine/submitter_test.go index d436856..a2f0795 100644 --- a/internal/engine/submitter_test.go +++ b/internal/engine/submitter_test.go @@ -3,10 +3,10 @@ package engine import ( "context" "errors" - "sync/atomic" "testing" "github.com/apex/log" + "github.com/ooni/probe-cli/v3/internal/atomicx" "github.com/ooni/probe-cli/v3/internal/engine/model" ) @@ -28,12 +28,14 @@ func TestSubmitterNotEnabled(t *testing.T) { } type FakeSubmitter struct { - Calls uint32 + Calls *atomicx.Int64 Error error } func (fs *FakeSubmitter) Submit(ctx context.Context, m *model.Measurement) error { - atomic.AddUint32(&fs.Calls, 1) + if fs.Calls != nil { + fs.Calls.Add(1) + } return fs.Error } @@ -68,7 +70,10 @@ func TestNewSubmitterFails(t *testing.T) { func TestNewSubmitterWithFailedSubmission(t *testing.T) { expected := errors.New("mocked error") ctx := context.Background() - fakeSubmitter := &FakeSubmitter{Error: expected} + fakeSubmitter := &FakeSubmitter{ + Calls: &atomicx.Int64{}, + Error: expected, + } submitter, err := NewSubmitter(ctx, SubmitterConfig{ Enabled: true, Logger: log.Log, @@ -82,7 +87,7 @@ func TestNewSubmitterWithFailedSubmission(t *testing.T) { if !errors.Is(err, expected) { t.Fatalf("not the error we expected: %+v", err) } - if fakeSubmitter.Calls != 1 { + if fakeSubmitter.Calls.Load() != 1 { t.Fatal("unexpected number of calls") } } diff --git a/internal/engine/internal/fsx/fsx.go b/internal/fsx/fsx.go similarity index 55% rename from internal/engine/internal/fsx/fsx.go rename to internal/fsx/fsx.go index d5a8bf7..89b888f 100644 --- a/internal/engine/internal/fsx/fsx.go +++ b/internal/fsx/fsx.go @@ -1,4 +1,4 @@ -// Package fsx contains file system extension +// Package fsx contains io/fs extensions. package fsx import ( @@ -8,13 +8,15 @@ import ( "syscall" ) -// Open is a wrapper for os.Open that ensures that we're opening a file. -func Open(pathname string) (fs.File, error) { - return OpenWithFS(filesystem{}, pathname) +// OpenFile is a wrapper for os.OpenFile that ensures that +// we're opening a file rather than a directory. If you are +// opening a directory, this func will return an error. +func OpenFile(pathname string) (fs.File, error) { + return openWithFS(filesystem{}, pathname) } -// OpenWithFS is like Open but with explicit file system argument. -func OpenWithFS(fs fs.FS, pathname string) (fs.File, error) { +// openWithFS is like Open but with explicit file system argument. +func openWithFS(fs fs.FS, pathname string) (fs.File, error) { file, err := fs.Open(pathname) if err != nil { return nil, err diff --git a/internal/fsx/fsx_test.go b/internal/fsx/fsx_test.go new file mode 100644 index 0000000..787f30e --- /dev/null +++ b/internal/fsx/fsx_test.go @@ -0,0 +1,84 @@ +package fsx + +import ( + "errors" + "io/fs" + "os" + "syscall" + "testing" + + "github.com/ooni/probe-cli/v3/internal/atomicx" +) + +// baseDir is the base directory we use for testing. +var baseDir = "./testdata/" + +// failingStatFS is a fs.FS returning a file where stat() fails. +type failingStatFS struct { + CloseCount *atomicx.Int64 +} + +// failingStatFile is a fs.File where stat() fails. +type failingStatFile struct { + CloseCount *atomicx.Int64 +} + +// errStatFailed is the internal error indicating that stat() failed. +var errStatFailed = errors.New("stat failed") + +// Stat is a stat implementation that fails. +func (failingStatFile) Stat() (os.FileInfo, error) { + return nil, errStatFailed +} + +// Open opens a fake file whose Stat fails. +func (f failingStatFS) Open(pathname string) (fs.File, error) { + return failingStatFile(f), nil +} + +// Close closes the failingStatFile. +func (fs failingStatFile) Close() error { + if fs.CloseCount != nil { + fs.CloseCount.Add(1) + } + return nil +} + +// Read implements fs.File.Read. +func (failingStatFile) Read([]byte) (int, error) { + return 0, errors.New("shouldn't be called") +} + +func TestOpenWithFailingStat(t *testing.T) { + count := &atomicx.Int64{} + _, err := openWithFS( + failingStatFS{CloseCount: count}, baseDir+"testfile.txt") + if !errors.Is(err, errStatFailed) { + t.Error("expected error with invalid FS", err) + } + if count.Load() != 1 { + t.Error("expected close counter to be equal to 1") + } +} + +func TestOpenNonexistentFile(t *testing.T) { + _, err := OpenFile(baseDir + "invalidtestfile.txt") + if !errors.Is(err, syscall.ENOENT) { + t.Errorf("not the error we expected") + } +} + +func TestOpenDirectoryShouldFail(t *testing.T) { + _, err := OpenFile(baseDir) + if !errors.Is(err, syscall.EISDIR) { + t.Fatalf("not the error we expected: %+v", err) + } +} + +func TestOpeningExistingFileShouldWork(t *testing.T) { + file, err := OpenFile(baseDir + "testfile.txt") + if err != nil { + t.Fatal(err) + } + defer file.Close() +} diff --git a/internal/engine/internal/fsx/testdata/.gitignore b/internal/fsx/testdata/.gitignore similarity index 100% rename from internal/engine/internal/fsx/testdata/.gitignore rename to internal/fsx/testdata/.gitignore diff --git a/internal/engine/internal/fsx/testdata/testfile.txt b/internal/fsx/testdata/testfile.txt similarity index 100% rename from internal/engine/internal/fsx/testdata/testfile.txt rename to internal/fsx/testdata/testfile.txt diff --git a/internal/engine/humanizex/humanizex.go b/internal/humanize/humanizex.go similarity index 57% rename from internal/engine/humanizex/humanizex.go rename to internal/humanize/humanizex.go index a41aa77..5a5934b 100644 --- a/internal/engine/humanizex/humanizex.go +++ b/internal/humanize/humanizex.go @@ -1,14 +1,17 @@ -// Package humanizex is like dustin/go-humanize -package humanizex +// Package humanize is like dustin/go-humanize. +package humanize import "fmt" -// SI is like dustin/go-humanize.SI +// SI is like dustin/go-humanize.SI but its implementation is +// specially tailored for printing download speeds. func SI(value float64, unit string) string { value, prefix := reduce(value) return fmt.Sprintf("%3.0f %s%s", value, prefix, unit) } +// reduce reduces value to a base value and a unit prefix. For +// example, reduce(1055) returns (1.055, "k"). func reduce(value float64) (float64, string) { if value < 1e03 { return value, " " diff --git a/internal/humanize/humanizex_test.go b/internal/humanize/humanizex_test.go new file mode 100644 index 0000000..c7286ac --- /dev/null +++ b/internal/humanize/humanizex_test.go @@ -0,0 +1,30 @@ +package humanize + +import "testing" + +func TestGood(t *testing.T) { + if SI(128, "bit/s") != "128 bit/s" { + t.Fatal("unexpected result") + } + if SI(1280, "bit/s") != " 1 kbit/s" { + t.Fatal("unexpected result") + } + if SI(12800, "bit/s") != " 13 kbit/s" { + t.Fatal("unexpected result") + } + if SI(128000, "bit/s") != "128 kbit/s" { + t.Fatal("unexpected result") + } + if SI(1280000, "bit/s") != " 1 Mbit/s" { + t.Fatal("unexpected result") + } + if SI(12800000, "bit/s") != " 13 Mbit/s" { + t.Fatal("unexpected result") + } + if SI(128000000, "bit/s") != "128 Mbit/s" { + t.Fatal("unexpected result") + } + if SI(1280000000, "bit/s") != " 1 Gbit/s" { + t.Fatal("unexpected result") + } +} diff --git a/internal/kvstore/fs.go b/internal/kvstore/fs.go new file mode 100644 index 0000000..483a625 --- /dev/null +++ b/internal/kvstore/fs.go @@ -0,0 +1,53 @@ +package kvstore + +import ( + "bytes" + "fmt" + "io/fs" + "os" + "path/filepath" + + "github.com/rogpeppe/go-internal/lockedfile" +) + +// FS is a file-system based KVStore. +type FS struct { + basedir string +} + +// NewFS creates a new kvstore.FileSystem. +func NewFS(basedir string) (kvs *FS, err error) { + return newFileSystem(basedir, os.MkdirAll) +} + +// osMkdirAll is the type of os.MkdirAll. +type osMkdirAll func(path string, perm fs.FileMode) error + +// newFileSystem is like NewFileSystem with a customizable +// osMkdirAll function for creating the kvstore dir. +func newFileSystem(basedir string, mkdir osMkdirAll) (*FS, error) { + if err := mkdir(basedir, 0700); err != nil { + return nil, err + } + return &FS{basedir: basedir}, nil +} + +// filename returns the filename for a given key. +func (kvs *FS) filename(key string) string { + return filepath.Join(kvs.basedir, key) +} + +// Get returns the specified key's value. In case of error, the +// error type is such that errors.Is(err, ErrNoSuchKey). +func (kvs *FS) Get(key string) ([]byte, error) { + data, err := lockedfile.Read(kvs.filename(key)) + if err != nil { + return nil, fmt.Errorf("%w: %s", ErrNoSuchKey, err.Error()) + } + return data, nil +} + +// Set sets the value of a specific key. +func (kvs *FS) Set(key string, value []byte) error { + return lockedfile.Write(kvs.filename(key), bytes.NewReader(value), 0600) +} diff --git a/internal/kvstore/fs_test.go b/internal/kvstore/fs_test.go new file mode 100644 index 0000000..462e5cb --- /dev/null +++ b/internal/kvstore/fs_test.go @@ -0,0 +1,67 @@ +package kvstore + +import ( + "bytes" + "errors" + "io/fs" + "os" + "path/filepath" + "testing" +) + +func TestFileSystemGood(t *testing.T) { + dirpath := filepath.Join("testdata", "kvstore2") + if err := os.RemoveAll(dirpath); err != nil { + t.Fatal(err) + } + kvstore, err := NewFS(dirpath) + if err != nil { + t.Fatal(err) + } + value := []byte("foobar") + if err := kvstore.Set("antani", value); err != nil { + t.Fatal(err) + } + ovalue, err := kvstore.Get("antani") + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(ovalue, value) { + t.Fatal("invalid value") + } +} + +func TestFileSystemNoSuchKey(t *testing.T) { + dirpath := filepath.Join("testdata", "kvstore2") + if err := os.RemoveAll(dirpath); err != nil { + t.Fatal(err) + } + kvstore, err := NewFS(dirpath) + if err != nil { + t.Fatal(err) + } + value, err := kvstore.Get("antani") + if !errors.Is(err, ErrNoSuchKey) { + t.Fatal("not the error we expected", err) + } + if value != nil { + t.Fatal("expected nil value") + } +} + +func TestFileSystemWithFailure(t *testing.T) { + expect := errors.New("mocked error") + mkdir := func(path string, perm fs.FileMode) error { + return expect + } + kvstore, err := newFileSystem( + filepath.Join("testdata", "kvstore2"), + mkdir, + ) + if !errors.Is(err, expect) { + t.Fatal("not the error we expected", err) + } + if kvstore != nil { + t.Fatal("expected nil here") + } +} diff --git a/internal/kvstore/memory.go b/internal/kvstore/memory.go new file mode 100644 index 0000000..9129e5e --- /dev/null +++ b/internal/kvstore/memory.go @@ -0,0 +1,42 @@ +// Package kvstore contains key-value stores. +package kvstore + +import ( + "errors" + "sync" +) + +// ErrNoSuchKey indicates that there's no value for the given key. +var ErrNoSuchKey = errors.New("no such key") + +// Memory is an in-memory key-value store. +type Memory struct { + // m is the underlying map. + m map[string][]byte + + // mu provides mutual exclusion + mu sync.Mutex +} + +// Get returns the specified key's value. In case of error, the +// error type is such that errors.Is(err, ErrNoSuchKey). +func (kvs *Memory) Get(key string) ([]byte, error) { + kvs.mu.Lock() + defer kvs.mu.Unlock() + value, ok := kvs.m[key] + if !ok { + return nil, ErrNoSuchKey + } + return value, nil +} + +// Set sets a key into the key-value store +func (kvs *Memory) Set(key string, value []byte) error { + kvs.mu.Lock() + defer kvs.mu.Unlock() + if kvs.m == nil { + kvs.m = make(map[string][]byte) + } + kvs.m[key] = value + return nil +} diff --git a/internal/engine/kvstore/kvstore_test.go b/internal/kvstore/memory_test.go similarity index 81% rename from internal/engine/kvstore/kvstore_test.go rename to internal/kvstore/memory_test.go index b8acfe7..dff34e0 100644 --- a/internal/engine/kvstore/kvstore_test.go +++ b/internal/kvstore/memory_test.go @@ -1,11 +1,14 @@ package kvstore -import "testing" +import ( + "errors" + "testing" +) func TestNoSuchKey(t *testing.T) { - kvs := NewMemoryKeyValueStore() + kvs := &Memory{} value, err := kvs.Get("nonexistent") - if err == nil { + if !errors.Is(err, ErrNoSuchKey) { t.Fatal("expected an error here") } if value != nil { @@ -14,7 +17,7 @@ func TestNoSuchKey(t *testing.T) { } func TestExistingKey(t *testing.T) { - kvs := NewMemoryKeyValueStore() + kvs := &Memory{} if err := kvs.Set("antani", []byte("mascetti")); err != nil { t.Fatal(err) } diff --git a/internal/engine/internal/multierror/multierror.go b/internal/multierror/multierror.go similarity index 85% rename from internal/engine/internal/multierror/multierror.go rename to internal/multierror/multierror.go index 2440633..560a56b 100644 --- a/internal/engine/internal/multierror/multierror.go +++ b/internal/multierror/multierror.go @@ -9,11 +9,14 @@ import ( // Union is the logical union of several errors. The Union will // appear to be the Root error, except that it will actually -// be possible to look deeper and see specific sub errors that +// be possible to look deeper and see specific child errors that // occurred using errors.As and errors.Is. type Union struct { + // Children contains the underlying errors. Children []error - Root error + + // Root is the root error. + Root error } // New creates a new Union error instance. @@ -37,8 +40,8 @@ func (err *Union) AddWithPrefix(prefix string, child error) { err.Add(fmt.Errorf("%s: %w", prefix, child)) } -// Is returns whether the Union error contains at least one child -// error that is exactly the specified target error. +// Is returns true (1) if the err.Root error is target or (2) if +// any err.Children error is target. func (err Union) Is(target error) bool { if errors.Is(err.Root, target) { return true diff --git a/internal/engine/internal/multierror/multierror_test.go b/internal/multierror/multierror_test.go similarity index 96% rename from internal/engine/internal/multierror/multierror_test.go rename to internal/multierror/multierror_test.go index bb973db..5dfaed9 100644 --- a/internal/engine/internal/multierror/multierror_test.go +++ b/internal/multierror/multierror_test.go @@ -8,7 +8,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" - "github.com/ooni/probe-cli/v3/internal/engine/internal/multierror" + "github.com/ooni/probe-cli/v3/internal/multierror" ) func TestEmpty(t *testing.T) { diff --git a/internal/engine/ooapi/apimodel/checkin.go b/internal/ooapi/apimodel/checkin.go similarity index 100% rename from internal/engine/ooapi/apimodel/checkin.go rename to internal/ooapi/apimodel/checkin.go diff --git a/internal/engine/ooapi/apimodel/checkreportid.go b/internal/ooapi/apimodel/checkreportid.go similarity index 100% rename from internal/engine/ooapi/apimodel/checkreportid.go rename to internal/ooapi/apimodel/checkreportid.go diff --git a/internal/engine/ooapi/apimodel/doc.go b/internal/ooapi/apimodel/doc.go similarity index 100% rename from internal/engine/ooapi/apimodel/doc.go rename to internal/ooapi/apimodel/doc.go diff --git a/internal/engine/ooapi/apimodel/login.go b/internal/ooapi/apimodel/login.go similarity index 100% rename from internal/engine/ooapi/apimodel/login.go rename to internal/ooapi/apimodel/login.go diff --git a/internal/engine/ooapi/apimodel/measurementmeta.go b/internal/ooapi/apimodel/measurementmeta.go similarity index 100% rename from internal/engine/ooapi/apimodel/measurementmeta.go rename to internal/ooapi/apimodel/measurementmeta.go diff --git a/internal/engine/ooapi/apimodel/openreport.go b/internal/ooapi/apimodel/openreport.go similarity index 100% rename from internal/engine/ooapi/apimodel/openreport.go rename to internal/ooapi/apimodel/openreport.go diff --git a/internal/engine/ooapi/apimodel/psiphonconfig.go b/internal/ooapi/apimodel/psiphonconfig.go similarity index 100% rename from internal/engine/ooapi/apimodel/psiphonconfig.go rename to internal/ooapi/apimodel/psiphonconfig.go diff --git a/internal/engine/ooapi/apimodel/register.go b/internal/ooapi/apimodel/register.go similarity index 100% rename from internal/engine/ooapi/apimodel/register.go rename to internal/ooapi/apimodel/register.go diff --git a/internal/engine/ooapi/apimodel/submitmeasurement.go b/internal/ooapi/apimodel/submitmeasurement.go similarity index 100% rename from internal/engine/ooapi/apimodel/submitmeasurement.go rename to internal/ooapi/apimodel/submitmeasurement.go diff --git a/internal/engine/ooapi/apimodel/testhelpers.go b/internal/ooapi/apimodel/testhelpers.go similarity index 100% rename from internal/engine/ooapi/apimodel/testhelpers.go rename to internal/ooapi/apimodel/testhelpers.go diff --git a/internal/engine/ooapi/apimodel/tortargets.go b/internal/ooapi/apimodel/tortargets.go similarity index 100% rename from internal/engine/ooapi/apimodel/tortargets.go rename to internal/ooapi/apimodel/tortargets.go diff --git a/internal/engine/ooapi/apimodel/urls.go b/internal/ooapi/apimodel/urls.go similarity index 100% rename from internal/engine/ooapi/apimodel/urls.go rename to internal/ooapi/apimodel/urls.go diff --git a/internal/engine/ooapi/apis.go b/internal/ooapi/apis.go similarity index 99% rename from internal/engine/ooapi/apis.go rename to internal/ooapi/apis.go index cbb69ae..dfe60c9 100644 --- a/internal/engine/ooapi/apis.go +++ b/internal/ooapi/apis.go @@ -9,7 +9,7 @@ import ( "context" "net/http" - "github.com/ooni/probe-cli/v3/internal/engine/ooapi/apimodel" + "github.com/ooni/probe-cli/v3/internal/ooapi/apimodel" ) // simpleCheckReportIDAPI implements the CheckReportID API. diff --git a/internal/engine/ooapi/apis_test.go b/internal/ooapi/apis_test.go similarity index 99% rename from internal/engine/ooapi/apis_test.go rename to internal/ooapi/apis_test.go index 792df41..6171074 100644 --- a/internal/engine/ooapi/apis_test.go +++ b/internal/ooapi/apis_test.go @@ -18,7 +18,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" - "github.com/ooni/probe-cli/v3/internal/engine/ooapi/apimodel" + "github.com/ooni/probe-cli/v3/internal/ooapi/apimodel" ) func TestCheckReportIDInvalidURL(t *testing.T) { diff --git a/internal/engine/ooapi/caching.go b/internal/ooapi/caching.go similarity index 97% rename from internal/engine/ooapi/caching.go rename to internal/ooapi/caching.go index e3171d5..640cdcc 100644 --- a/internal/engine/ooapi/caching.go +++ b/internal/ooapi/caching.go @@ -9,7 +9,7 @@ import ( "context" "reflect" - "github.com/ooni/probe-cli/v3/internal/engine/ooapi/apimodel" + "github.com/ooni/probe-cli/v3/internal/ooapi/apimodel" ) // withCacheMeasurementMetaAPI implements caching for simpleMeasurementMetaAPI. diff --git a/internal/engine/ooapi/caching_test.go b/internal/ooapi/caching_test.go similarity index 94% rename from internal/engine/ooapi/caching_test.go rename to internal/ooapi/caching_test.go index 746df68..0c6c1c7 100644 --- a/internal/engine/ooapi/caching_test.go +++ b/internal/ooapi/caching_test.go @@ -11,7 +11,8 @@ import ( "testing" "github.com/google/go-cmp/cmp" - "github.com/ooni/probe-cli/v3/internal/engine/ooapi/apimodel" + "github.com/ooni/probe-cli/v3/internal/kvstore" + "github.com/ooni/probe-cli/v3/internal/ooapi/apimodel" ) func TestCachesimpleMeasurementMetaAPISuccess(t *testing.T) { @@ -22,7 +23,7 @@ func TestCachesimpleMeasurementMetaAPISuccess(t *testing.T) { API: &FakeMeasurementMetaAPI{ Response: expect, }, - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, } var req *apimodel.MeasurementMetaRequest ff.fill(&req) @@ -69,7 +70,7 @@ func TestCachesimpleMeasurementMetaAPIFailureWithNoCache(t *testing.T) { API: &FakeMeasurementMetaAPI{ Err: errMocked, }, - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, } var req *apimodel.MeasurementMetaRequest ff.fill(&req) @@ -92,7 +93,7 @@ func TestCachesimpleMeasurementMetaAPIFailureWithPreviousCache(t *testing.T) { } cache := &withCacheMeasurementMetaAPI{ API: fakeapi, - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, } var req *apimodel.MeasurementMetaRequest ff.fill(&req) @@ -146,7 +147,7 @@ func TestCachesimpleMeasurementMetaAPIReadCacheNotFound(t *testing.T) { var incache []cacheEntryForMeasurementMetaAPI ff.fill(&incache) cache := &withCacheMeasurementMetaAPI{ - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, } err := cache.setcache(incache) if err != nil { @@ -172,7 +173,7 @@ func TestCachesimpleMeasurementMetaAPIWriteCacheDuplicate(t *testing.T) { var resp2 *apimodel.MeasurementMetaResponse ff.fill(&resp2) cache := &withCacheMeasurementMetaAPI{ - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, } err := cache.writecache(req, resp1) if err != nil { @@ -197,7 +198,7 @@ func TestCachesimpleMeasurementMetaAPIWriteCacheDuplicate(t *testing.T) { func TestCachesimpleMeasurementMetaAPICacheSizeLimited(t *testing.T) { ff := &fakeFill{} cache := &withCacheMeasurementMetaAPI{ - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, } var prev int for { diff --git a/internal/engine/ooapi/callers.go b/internal/ooapi/callers.go similarity index 97% rename from internal/engine/ooapi/callers.go rename to internal/ooapi/callers.go index 042addc..87a9fe8 100644 --- a/internal/engine/ooapi/callers.go +++ b/internal/ooapi/callers.go @@ -8,7 +8,7 @@ package ooapi import ( "context" - "github.com/ooni/probe-cli/v3/internal/engine/ooapi/apimodel" + "github.com/ooni/probe-cli/v3/internal/ooapi/apimodel" ) // callerForCheckReportIDAPI represents any type exposing a method diff --git a/internal/engine/ooapi/client.go b/internal/ooapi/client.go similarity index 100% rename from internal/engine/ooapi/client.go rename to internal/ooapi/client.go diff --git a/internal/engine/ooapi/clientcall.go b/internal/ooapi/clientcall.go similarity index 98% rename from internal/engine/ooapi/clientcall.go rename to internal/ooapi/clientcall.go index 2347207..6e094ea 100644 --- a/internal/engine/ooapi/clientcall.go +++ b/internal/ooapi/clientcall.go @@ -8,7 +8,7 @@ package ooapi import ( "context" - "github.com/ooni/probe-cli/v3/internal/engine/ooapi/apimodel" + "github.com/ooni/probe-cli/v3/internal/ooapi/apimodel" ) func (c *Client) newCheckReportIDCaller() callerForCheckReportIDAPI { diff --git a/internal/engine/ooapi/clientcall_test.go b/internal/ooapi/clientcall_test.go similarity index 96% rename from internal/engine/ooapi/clientcall_test.go rename to internal/ooapi/clientcall_test.go index f52a087..438b655 100644 --- a/internal/engine/ooapi/clientcall_test.go +++ b/internal/ooapi/clientcall_test.go @@ -16,7 +16,8 @@ import ( "testing" "github.com/google/go-cmp/cmp" - "github.com/ooni/probe-cli/v3/internal/engine/ooapi/apimodel" + "github.com/ooni/probe-cli/v3/internal/kvstore" + "github.com/ooni/probe-cli/v3/internal/ooapi/apimodel" ) type handleClientCallCheckReportID struct { @@ -72,7 +73,7 @@ func TestCheckReportIDClientCallRoundTrip(t *testing.T) { req := &apimodel.CheckReportIDRequest{} ff := &fakeFill{} ff.fill(&req) - clnt := &Client{KVStore: &MemKVStore{}, BaseURL: srvr.URL} + clnt := &Client{KVStore: &kvstore.Memory{}, BaseURL: srvr.URL} ff.fill(&clnt.UserAgent) // issue request ctx := context.Background() @@ -165,7 +166,7 @@ func TestCheckInClientCallRoundTrip(t *testing.T) { req := &apimodel.CheckInRequest{} ff := &fakeFill{} ff.fill(&req) - clnt := &Client{KVStore: &MemKVStore{}, BaseURL: srvr.URL} + clnt := &Client{KVStore: &kvstore.Memory{}, BaseURL: srvr.URL} ff.fill(&clnt.UserAgent) // issue request ctx := context.Background() @@ -257,7 +258,7 @@ func TestMeasurementMetaClientCallRoundTrip(t *testing.T) { req := &apimodel.MeasurementMetaRequest{} ff := &fakeFill{} ff.fill(&req) - clnt := &Client{KVStore: &MemKVStore{}, BaseURL: srvr.URL} + clnt := &Client{KVStore: &kvstore.Memory{}, BaseURL: srvr.URL} ff.fill(&clnt.UserAgent) // issue request ctx := context.Background() @@ -350,7 +351,7 @@ func TestTestHelpersClientCallRoundTrip(t *testing.T) { req := &apimodel.TestHelpersRequest{} ff := &fakeFill{} ff.fill(&req) - clnt := &Client{KVStore: &MemKVStore{}, BaseURL: srvr.URL} + clnt := &Client{KVStore: &kvstore.Memory{}, BaseURL: srvr.URL} ff.fill(&clnt.UserAgent) // issue request ctx := context.Background() @@ -465,7 +466,7 @@ func TestPsiphonConfigClientCallRoundTrip(t *testing.T) { req := &apimodel.PsiphonConfigRequest{} ff := &fakeFill{} ff.fill(&req) - clnt := &Client{KVStore: &MemKVStore{}, BaseURL: srvr.URL} + clnt := &Client{KVStore: &kvstore.Memory{}, BaseURL: srvr.URL} ff.fill(&clnt.UserAgent) // issue request ctx := context.Background() @@ -580,7 +581,7 @@ func TestTorTargetsClientCallRoundTrip(t *testing.T) { req := &apimodel.TorTargetsRequest{} ff := &fakeFill{} ff.fill(&req) - clnt := &Client{KVStore: &MemKVStore{}, BaseURL: srvr.URL} + clnt := &Client{KVStore: &kvstore.Memory{}, BaseURL: srvr.URL} ff.fill(&clnt.UserAgent) // issue request ctx := context.Background() @@ -673,7 +674,7 @@ func TestURLsClientCallRoundTrip(t *testing.T) { req := &apimodel.URLsRequest{} ff := &fakeFill{} ff.fill(&req) - clnt := &Client{KVStore: &MemKVStore{}, BaseURL: srvr.URL} + clnt := &Client{KVStore: &kvstore.Memory{}, BaseURL: srvr.URL} ff.fill(&clnt.UserAgent) // issue request ctx := context.Background() @@ -766,7 +767,7 @@ func TestOpenReportClientCallRoundTrip(t *testing.T) { req := &apimodel.OpenReportRequest{} ff := &fakeFill{} ff.fill(&req) - clnt := &Client{KVStore: &MemKVStore{}, BaseURL: srvr.URL} + clnt := &Client{KVStore: &kvstore.Memory{}, BaseURL: srvr.URL} ff.fill(&clnt.UserAgent) // issue request ctx := context.Background() @@ -858,7 +859,7 @@ func TestSubmitMeasurementClientCallRoundTrip(t *testing.T) { req := &apimodel.SubmitMeasurementRequest{} ff := &fakeFill{} ff.fill(&req) - clnt := &Client{KVStore: &MemKVStore{}, BaseURL: srvr.URL} + clnt := &Client{KVStore: &kvstore.Memory{}, BaseURL: srvr.URL} ff.fill(&clnt.UserAgent) // issue request ctx := context.Background() diff --git a/internal/engine/ooapi/cloners.go b/internal/ooapi/cloners.go similarity index 100% rename from internal/engine/ooapi/cloners.go rename to internal/ooapi/cloners.go diff --git a/internal/engine/ooapi/default.go b/internal/ooapi/default.go similarity index 100% rename from internal/engine/ooapi/default.go rename to internal/ooapi/default.go diff --git a/internal/engine/ooapi/default_test.go b/internal/ooapi/default_test.go similarity index 100% rename from internal/engine/ooapi/default_test.go rename to internal/ooapi/default_test.go diff --git a/internal/engine/ooapi/dependencies.go b/internal/ooapi/dependencies.go similarity index 100% rename from internal/engine/ooapi/dependencies.go rename to internal/ooapi/dependencies.go diff --git a/internal/engine/ooapi/doc.go b/internal/ooapi/doc.go similarity index 100% rename from internal/engine/ooapi/doc.go rename to internal/ooapi/doc.go diff --git a/internal/engine/ooapi/errors.go b/internal/ooapi/errors.go similarity index 100% rename from internal/engine/ooapi/errors.go rename to internal/ooapi/errors.go diff --git a/internal/engine/ooapi/fake_test.go b/internal/ooapi/fake_test.go similarity index 100% rename from internal/engine/ooapi/fake_test.go rename to internal/ooapi/fake_test.go diff --git a/internal/engine/ooapi/fakeapi_test.go b/internal/ooapi/fakeapi_test.go similarity index 80% rename from internal/engine/ooapi/fakeapi_test.go rename to internal/ooapi/fakeapi_test.go index cf617e3..12d2e14 100644 --- a/internal/engine/ooapi/fakeapi_test.go +++ b/internal/ooapi/fakeapi_test.go @@ -7,19 +7,21 @@ package ooapi import ( "context" - "sync/atomic" - "github.com/ooni/probe-cli/v3/internal/engine/ooapi/apimodel" + "github.com/ooni/probe-cli/v3/internal/atomicx" + "github.com/ooni/probe-cli/v3/internal/ooapi/apimodel" ) type FakeCheckReportIDAPI struct { Err error Response *apimodel.CheckReportIDResponse - CountCall int32 + CountCall *atomicx.Int64 } func (fapi *FakeCheckReportIDAPI) Call(ctx context.Context, req *apimodel.CheckReportIDRequest) (*apimodel.CheckReportIDResponse, error) { - atomic.AddInt32(&fapi.CountCall, 1) + if fapi.CountCall != nil { + fapi.CountCall.Add(1) + } return fapi.Response, fapi.Err } @@ -30,11 +32,13 @@ var ( type FakeCheckInAPI struct { Err error Response *apimodel.CheckInResponse - CountCall int32 + CountCall *atomicx.Int64 } func (fapi *FakeCheckInAPI) Call(ctx context.Context, req *apimodel.CheckInRequest) (*apimodel.CheckInResponse, error) { - atomic.AddInt32(&fapi.CountCall, 1) + if fapi.CountCall != nil { + fapi.CountCall.Add(1) + } return fapi.Response, fapi.Err } @@ -45,11 +49,13 @@ var ( type FakeLoginAPI struct { Err error Response *apimodel.LoginResponse - CountCall int32 + CountCall *atomicx.Int64 } func (fapi *FakeLoginAPI) Call(ctx context.Context, req *apimodel.LoginRequest) (*apimodel.LoginResponse, error) { - atomic.AddInt32(&fapi.CountCall, 1) + if fapi.CountCall != nil { + fapi.CountCall.Add(1) + } return fapi.Response, fapi.Err } @@ -60,11 +66,13 @@ var ( type FakeMeasurementMetaAPI struct { Err error Response *apimodel.MeasurementMetaResponse - CountCall int32 + CountCall *atomicx.Int64 } func (fapi *FakeMeasurementMetaAPI) Call(ctx context.Context, req *apimodel.MeasurementMetaRequest) (*apimodel.MeasurementMetaResponse, error) { - atomic.AddInt32(&fapi.CountCall, 1) + if fapi.CountCall != nil { + fapi.CountCall.Add(1) + } return fapi.Response, fapi.Err } @@ -75,11 +83,13 @@ var ( type FakeRegisterAPI struct { Err error Response *apimodel.RegisterResponse - CountCall int32 + CountCall *atomicx.Int64 } func (fapi *FakeRegisterAPI) Call(ctx context.Context, req *apimodel.RegisterRequest) (*apimodel.RegisterResponse, error) { - atomic.AddInt32(&fapi.CountCall, 1) + if fapi.CountCall != nil { + fapi.CountCall.Add(1) + } return fapi.Response, fapi.Err } @@ -90,11 +100,13 @@ var ( type FakeTestHelpersAPI struct { Err error Response apimodel.TestHelpersResponse - CountCall int32 + CountCall *atomicx.Int64 } func (fapi *FakeTestHelpersAPI) Call(ctx context.Context, req *apimodel.TestHelpersRequest) (apimodel.TestHelpersResponse, error) { - atomic.AddInt32(&fapi.CountCall, 1) + if fapi.CountCall != nil { + fapi.CountCall.Add(1) + } return fapi.Response, fapi.Err } @@ -106,11 +118,13 @@ type FakePsiphonConfigAPI struct { WithResult callerForPsiphonConfigAPI Err error Response apimodel.PsiphonConfigResponse - CountCall int32 + CountCall *atomicx.Int64 } func (fapi *FakePsiphonConfigAPI) Call(ctx context.Context, req *apimodel.PsiphonConfigRequest) (apimodel.PsiphonConfigResponse, error) { - atomic.AddInt32(&fapi.CountCall, 1) + if fapi.CountCall != nil { + fapi.CountCall.Add(1) + } return fapi.Response, fapi.Err } @@ -127,11 +141,13 @@ type FakeTorTargetsAPI struct { WithResult callerForTorTargetsAPI Err error Response apimodel.TorTargetsResponse - CountCall int32 + CountCall *atomicx.Int64 } func (fapi *FakeTorTargetsAPI) Call(ctx context.Context, req *apimodel.TorTargetsRequest) (apimodel.TorTargetsResponse, error) { - atomic.AddInt32(&fapi.CountCall, 1) + if fapi.CountCall != nil { + fapi.CountCall.Add(1) + } return fapi.Response, fapi.Err } @@ -147,11 +163,13 @@ var ( type FakeURLsAPI struct { Err error Response *apimodel.URLsResponse - CountCall int32 + CountCall *atomicx.Int64 } func (fapi *FakeURLsAPI) Call(ctx context.Context, req *apimodel.URLsRequest) (*apimodel.URLsResponse, error) { - atomic.AddInt32(&fapi.CountCall, 1) + if fapi.CountCall != nil { + fapi.CountCall.Add(1) + } return fapi.Response, fapi.Err } @@ -162,11 +180,13 @@ var ( type FakeOpenReportAPI struct { Err error Response *apimodel.OpenReportResponse - CountCall int32 + CountCall *atomicx.Int64 } func (fapi *FakeOpenReportAPI) Call(ctx context.Context, req *apimodel.OpenReportRequest) (*apimodel.OpenReportResponse, error) { - atomic.AddInt32(&fapi.CountCall, 1) + if fapi.CountCall != nil { + fapi.CountCall.Add(1) + } return fapi.Response, fapi.Err } @@ -177,11 +197,13 @@ var ( type FakeSubmitMeasurementAPI struct { Err error Response *apimodel.SubmitMeasurementResponse - CountCall int32 + CountCall *atomicx.Int64 } func (fapi *FakeSubmitMeasurementAPI) Call(ctx context.Context, req *apimodel.SubmitMeasurementRequest) (*apimodel.SubmitMeasurementResponse, error) { - atomic.AddInt32(&fapi.CountCall, 1) + if fapi.CountCall != nil { + fapi.CountCall.Add(1) + } return fapi.Response, fapi.Err } diff --git a/internal/engine/ooapi/fakefill_test.go b/internal/ooapi/fakefill_test.go similarity index 98% rename from internal/engine/ooapi/fakefill_test.go rename to internal/ooapi/fakefill_test.go index 5ee7776..32f1078 100644 --- a/internal/engine/ooapi/fakefill_test.go +++ b/internal/ooapi/fakefill_test.go @@ -7,7 +7,7 @@ import ( "testing" "time" - "github.com/ooni/probe-cli/v3/internal/engine/ooapi/apimodel" + "github.com/ooni/probe-cli/v3/internal/ooapi/apimodel" ) // fakeFill fills specific data structures with random data. The only diff --git a/internal/engine/ooapi/httpclient_test.go b/internal/ooapi/httpclient_test.go similarity index 100% rename from internal/engine/ooapi/httpclient_test.go rename to internal/ooapi/httpclient_test.go diff --git a/internal/engine/ooapi/integration_test.go b/internal/ooapi/integration_test.go similarity index 85% rename from internal/engine/ooapi/integration_test.go rename to internal/ooapi/integration_test.go index 59e0eea..3a7c1a0 100644 --- a/internal/engine/ooapi/integration_test.go +++ b/internal/ooapi/integration_test.go @@ -4,8 +4,9 @@ import ( "context" "testing" - "github.com/ooni/probe-cli/v3/internal/engine/ooapi" - "github.com/ooni/probe-cli/v3/internal/engine/ooapi/apimodel" + "github.com/ooni/probe-cli/v3/internal/kvstore" + "github.com/ooni/probe-cli/v3/internal/ooapi" + "github.com/ooni/probe-cli/v3/internal/ooapi/apimodel" ) func TestWithRealServerDoCheckIn(t *testing.T) { @@ -26,7 +27,7 @@ func TestWithRealServerDoCheckIn(t *testing.T) { }, } httpClnt := &ooapi.VerboseHTTPClient{T: t} - clnt := &ooapi.Client{HTTPClient: httpClnt, KVStore: &ooapi.MemKVStore{}} + clnt := &ooapi.Client{HTTPClient: httpClnt, KVStore: &kvstore.Memory{}} ctx := context.Background() resp, err := clnt.CheckIn(ctx, req) if err != nil { @@ -50,7 +51,7 @@ func TestWithRealServerDoCheckReportID(t *testing.T) { req := &apimodel.CheckReportIDRequest{ ReportID: "20210223T093606Z_ndt_JO_8376_n1_kDYToqrugDY54Soy", } - clnt := &ooapi.Client{KVStore: &ooapi.MemKVStore{}} + clnt := &ooapi.Client{KVStore: &kvstore.Memory{}} ctx := context.Background() resp, err := clnt.CheckReportID(ctx, req) if err != nil { @@ -69,7 +70,7 @@ func TestWithRealServerDoMeasurementMeta(t *testing.T) { req := &apimodel.MeasurementMetaRequest{ ReportID: "20210223T093606Z_ndt_JO_8376_n1_kDYToqrugDY54Soy", } - clnt := &ooapi.Client{KVStore: &ooapi.MemKVStore{}} + clnt := &ooapi.Client{KVStore: &kvstore.Memory{}} ctx := context.Background() resp, err := clnt.MeasurementMeta(ctx, req) if err != nil { @@ -96,7 +97,7 @@ func TestWithRealServerDoOpenReport(t *testing.T) { TestStartTime: "2018-11-01 15:33:20", TestVersion: "0.1.0", } - clnt := &ooapi.Client{KVStore: &ooapi.MemKVStore{}} + clnt := &ooapi.Client{KVStore: &kvstore.Memory{}} ctx := context.Background() resp, err := clnt.OpenReport(ctx, req) if err != nil { @@ -114,7 +115,7 @@ func TestWithRealServerDoPsiphonConfig(t *testing.T) { } req := &apimodel.PsiphonConfigRequest{} httpClnt := &ooapi.VerboseHTTPClient{T: t} - clnt := &ooapi.Client{HTTPClient: httpClnt, KVStore: &ooapi.MemKVStore{}} + clnt := &ooapi.Client{HTTPClient: httpClnt, KVStore: &kvstore.Memory{}} ctx := context.Background() resp, err := clnt.PsiphonConfig(ctx, req) if err != nil { @@ -132,7 +133,7 @@ func TestWithRealServerDoTorTargets(t *testing.T) { } req := &apimodel.TorTargetsRequest{} httpClnt := &ooapi.VerboseHTTPClient{T: t} - clnt := &ooapi.Client{HTTPClient: httpClnt, KVStore: &ooapi.MemKVStore{}} + clnt := &ooapi.Client{HTTPClient: httpClnt, KVStore: &kvstore.Memory{}} ctx := context.Background() resp, err := clnt.TorTargets(ctx, req) if err != nil { @@ -152,7 +153,7 @@ func TestWithRealServerDoURLs(t *testing.T) { CountryCode: "IT", Limit: 3, } - clnt := &ooapi.Client{KVStore: &ooapi.MemKVStore{}} + clnt := &ooapi.Client{KVStore: &kvstore.Memory{}} ctx := context.Background() resp, err := clnt.URLs(ctx, req) if err != nil { diff --git a/internal/engine/ooapi/internal/generator/apis.go b/internal/ooapi/internal/generator/apis.go similarity index 98% rename from internal/engine/ooapi/internal/generator/apis.go rename to internal/ooapi/internal/generator/apis.go index 56536db..1d15cda 100644 --- a/internal/engine/ooapi/internal/generator/apis.go +++ b/internal/ooapi/internal/generator/apis.go @@ -171,7 +171,7 @@ func GenAPIsGo(file string) { fmt.Fprint(&sb, "\t\"context\"\n") fmt.Fprint(&sb, "\t\"net/http\"\n") fmt.Fprint(&sb, "\n") - fmt.Fprint(&sb, "\t\"github.com/ooni/probe-cli/v3/internal/engine/ooapi/apimodel\"\n") + fmt.Fprint(&sb, "\t\"github.com/ooni/probe-cli/v3/internal/ooapi/apimodel\"\n") fmt.Fprint(&sb, ")\n") for _, desc := range Descriptors { desc.genNewAPI(&sb) diff --git a/internal/engine/ooapi/internal/generator/apistest.go b/internal/ooapi/internal/generator/apistest.go similarity index 99% rename from internal/engine/ooapi/internal/generator/apistest.go rename to internal/ooapi/internal/generator/apistest.go index 31cacbe..feeb7c8 100644 --- a/internal/engine/ooapi/internal/generator/apistest.go +++ b/internal/ooapi/internal/generator/apistest.go @@ -452,7 +452,7 @@ func GenAPIsTestGo(file string) { fmt.Fprint(&sb, "\t\"sync\"\n") fmt.Fprint(&sb, "\n") fmt.Fprint(&sb, "\t\"github.com/google/go-cmp/cmp\"\n") - fmt.Fprint(&sb, "\t\"github.com/ooni/probe-cli/v3/internal/engine/ooapi/apimodel\"\n") + fmt.Fprint(&sb, "\t\"github.com/ooni/probe-cli/v3/internal/ooapi/apimodel\"\n") fmt.Fprint(&sb, ")\n") for _, desc := range Descriptors { desc.genAPITests(&sb) diff --git a/internal/engine/ooapi/internal/generator/caching.go b/internal/ooapi/internal/generator/caching.go similarity index 98% rename from internal/engine/ooapi/internal/generator/caching.go rename to internal/ooapi/internal/generator/caching.go index 75a2bc9..1ceff42 100644 --- a/internal/engine/ooapi/internal/generator/caching.go +++ b/internal/ooapi/internal/generator/caching.go @@ -118,7 +118,7 @@ func GenCachingGo(file string) { fmt.Fprint(&sb, "\t\"context\"\n") fmt.Fprint(&sb, "\t\"reflect\"\n") fmt.Fprint(&sb, "\n") - fmt.Fprint(&sb, "\t\"github.com/ooni/probe-cli/v3/internal/engine/ooapi/apimodel\"\n") + fmt.Fprint(&sb, "\t\"github.com/ooni/probe-cli/v3/internal/ooapi/apimodel\"\n") fmt.Fprint(&sb, ")\n") for _, desc := range Descriptors { if desc.CachePolicy == CacheNone { diff --git a/internal/engine/ooapi/internal/generator/cachingtest.go b/internal/ooapi/internal/generator/cachingtest.go similarity index 95% rename from internal/engine/ooapi/internal/generator/cachingtest.go rename to internal/ooapi/internal/generator/cachingtest.go index c56459d..02c0ec9 100644 --- a/internal/engine/ooapi/internal/generator/cachingtest.go +++ b/internal/ooapi/internal/generator/cachingtest.go @@ -15,7 +15,7 @@ func (d *Descriptor) genTestCacheSuccess(sb *strings.Builder) { fmt.Fprintf(sb, "\t\tAPI: &%s{\n", d.FakeAPIStructName()) fmt.Fprint(sb, "\t\t\tResponse: expect,\n") fmt.Fprint(sb, "\t\t},\n") - fmt.Fprint(sb, "\t\tKVStore: &MemKVStore{},\n") + fmt.Fprint(sb, "\t\tKVStore: &kvstore.Memory{},\n") fmt.Fprint(sb, "\t}\n") fmt.Fprintf(sb, "\tvar req %s\n", d.RequestTypeName()) fmt.Fprint(sb, "\tff.fill(&req)\n") @@ -66,7 +66,7 @@ func (d *Descriptor) genTestFailureWithNoCache(sb *strings.Builder) { fmt.Fprintf(sb, "\t\tAPI: &%s{\n", d.FakeAPIStructName()) fmt.Fprint(sb, "\t\t\tErr: errMocked,\n") fmt.Fprint(sb, "\t\t},\n") - fmt.Fprint(sb, "\t\tKVStore: &MemKVStore{},\n") + fmt.Fprint(sb, "\t\tKVStore: &kvstore.Memory{},\n") fmt.Fprint(sb, "\t}\n") fmt.Fprintf(sb, "\tvar req %s\n", d.RequestTypeName()) fmt.Fprint(sb, "\tff.fill(&req)\n") @@ -92,7 +92,7 @@ func (d *Descriptor) genTestFailureWithPreviousCache(sb *strings.Builder) { fmt.Fprint(sb, "\t}\n") fmt.Fprintf(sb, "\tcache := &%s{\n", d.WithCacheAPIStructName()) fmt.Fprint(sb, "\t\tAPI: fakeapi,\n") - fmt.Fprint(sb, "\t\tKVStore: &MemKVStore{},\n") + fmt.Fprint(sb, "\t\tKVStore: &kvstore.Memory{},\n") fmt.Fprint(sb, "\t}\n") fmt.Fprintf(sb, "\tvar req %s\n", d.RequestTypeName()) fmt.Fprint(sb, "\tff.fill(&req)\n") @@ -156,7 +156,7 @@ func (d *Descriptor) genTestReadCacheNotFound(sb *strings.Builder) { fmt.Fprintf(sb, "\tvar incache []%s\n", d.CacheEntryName()) fmt.Fprint(sb, "\tff.fill(&incache)\n") fmt.Fprintf(sb, "\tcache := &%s{\n", d.WithCacheAPIStructName()) - fmt.Fprint(sb, "\t\tKVStore: &MemKVStore{},\n") + fmt.Fprint(sb, "\t\tKVStore: &kvstore.Memory{},\n") fmt.Fprint(sb, "\t}\n") fmt.Fprintf(sb, "\terr := cache.setcache(incache)\n") fmt.Fprintf(sb, "\tif err != nil {\n") @@ -184,7 +184,7 @@ func (d *Descriptor) genTestWriteCacheDuplicate(sb *strings.Builder) { fmt.Fprintf(sb, "\tvar resp2 %s\n", d.ResponseTypeName()) fmt.Fprint(sb, "\tff.fill(&resp2)\n") fmt.Fprintf(sb, "\tcache := &%s{\n", d.WithCacheAPIStructName()) - fmt.Fprint(sb, "\t\tKVStore: &MemKVStore{},\n") + fmt.Fprint(sb, "\t\tKVStore: &kvstore.Memory{},\n") fmt.Fprint(sb, "\t}\n") fmt.Fprintf(sb, "\terr := cache.writecache(req, resp1)\n") fmt.Fprintf(sb, "\tif err != nil {\n") @@ -217,7 +217,7 @@ func (d *Descriptor) genTestCachSizeLimited(sb *strings.Builder) { fmt.Fprintf(sb, "func TestCache%sCacheSizeLimited(t *testing.T) {\n", d.APIStructName()) fmt.Fprint(sb, "\tff := &fakeFill{}\n") fmt.Fprintf(sb, "\tcache := &%s{\n", d.WithCacheAPIStructName()) - fmt.Fprint(sb, "\t\tKVStore: &MemKVStore{},\n") + fmt.Fprint(sb, "\t\tKVStore: &kvstore.Memory{},\n") fmt.Fprint(sb, "\t}\n") fmt.Fprintf(sb, "\tvar prev int\n") fmt.Fprintf(sb, "\tfor {\n") @@ -255,7 +255,8 @@ func GenCachingTestGo(file string) { fmt.Fprint(&sb, "\t\"testing\"\n") fmt.Fprint(&sb, "\n") fmt.Fprint(&sb, "\t\"github.com/google/go-cmp/cmp\"\n") - fmt.Fprint(&sb, "\t\"github.com/ooni/probe-cli/v3/internal/engine/ooapi/apimodel\"\n") + fmt.Fprint(&sb, "\t\"github.com/ooni/probe-cli/v3/internal/kvstore\"\n") + fmt.Fprint(&sb, "\t\"github.com/ooni/probe-cli/v3/internal/ooapi/apimodel\"\n") fmt.Fprint(&sb, ")\n") for _, desc := range Descriptors { if desc.CachePolicy == CacheNone { diff --git a/internal/engine/ooapi/internal/generator/callers.go b/internal/ooapi/internal/generator/callers.go similarity index 91% rename from internal/engine/ooapi/internal/generator/callers.go rename to internal/ooapi/internal/generator/callers.go index 4ba8ea8..1ce873b 100644 --- a/internal/engine/ooapi/internal/generator/callers.go +++ b/internal/ooapi/internal/generator/callers.go @@ -26,7 +26,7 @@ func GenCallersGo(file string) { fmt.Fprint(&sb, "import (\n") fmt.Fprint(&sb, "\t\"context\"\n") fmt.Fprint(&sb, "\n") - fmt.Fprint(&sb, "\t\"github.com/ooni/probe-cli/v3/internal/engine/ooapi/apimodel\"\n") + fmt.Fprint(&sb, "\t\"github.com/ooni/probe-cli/v3/internal/ooapi/apimodel\"\n") fmt.Fprint(&sb, ")\n") for _, desc := range Descriptors { desc.genNewCaller(&sb) diff --git a/internal/engine/ooapi/internal/generator/clientcall.go b/internal/ooapi/internal/generator/clientcall.go similarity index 97% rename from internal/engine/ooapi/internal/generator/clientcall.go rename to internal/ooapi/internal/generator/clientcall.go index eeb688d..3b36063 100644 --- a/internal/engine/ooapi/internal/generator/clientcall.go +++ b/internal/ooapi/internal/generator/clientcall.go @@ -89,7 +89,7 @@ func GenClientCallGo(file string) { fmt.Fprint(&sb, "import (\n") fmt.Fprint(&sb, "\t\"context\"\n") fmt.Fprint(&sb, "\n") - fmt.Fprint(&sb, "\t\"github.com/ooni/probe-cli/v3/internal/engine/ooapi/apimodel\"\n") + fmt.Fprint(&sb, "\t\"github.com/ooni/probe-cli/v3/internal/ooapi/apimodel\"\n") fmt.Fprint(&sb, ")\n") for _, desc := range Descriptors { switch desc.Name { diff --git a/internal/engine/ooapi/internal/generator/clientcalltest.go b/internal/ooapi/internal/generator/clientcalltest.go similarity index 96% rename from internal/engine/ooapi/internal/generator/clientcalltest.go rename to internal/ooapi/internal/generator/clientcalltest.go index f840eff..0443d80 100644 --- a/internal/engine/ooapi/internal/generator/clientcalltest.go +++ b/internal/ooapi/internal/generator/clientcalltest.go @@ -90,7 +90,7 @@ func (d *Descriptor) genTestClientCallRoundTrip(sb *strings.Builder) { fmt.Fprintf(sb, "\treq := &%s{}\n", d.RequestTypeNameAsStruct()) fmt.Fprint(sb, "\tff := &fakeFill{}\n") fmt.Fprint(sb, "\tff.fill(&req)\n") - fmt.Fprint(sb, "\tclnt := &Client{KVStore: &MemKVStore{}, BaseURL: srvr.URL}\n") + fmt.Fprint(sb, "\tclnt := &Client{KVStore: &kvstore.Memory{}, BaseURL: srvr.URL}\n") fmt.Fprint(sb, "\tff.fill(&clnt.UserAgent)\n") fmt.Fprint(sb, "\t// issue request\n") @@ -169,7 +169,8 @@ func GenClientCallTestGo(file string) { fmt.Fprint(&sb, "\t\"sync\"\n") fmt.Fprint(&sb, "\n") fmt.Fprint(&sb, "\t\"github.com/google/go-cmp/cmp\"\n") - fmt.Fprint(&sb, "\t\"github.com/ooni/probe-cli/v3/internal/engine/ooapi/apimodel\"\n") + fmt.Fprint(&sb, "\t\"github.com/ooni/probe-cli/v3/internal/kvstore\"\n") + fmt.Fprint(&sb, "\t\"github.com/ooni/probe-cli/v3/internal/ooapi/apimodel\"\n") fmt.Fprint(&sb, ")\n") for _, desc := range Descriptors { if desc.Name == "Login" || desc.Name == "Register" { diff --git a/internal/engine/ooapi/internal/generator/cloners.go b/internal/ooapi/internal/generator/cloners.go similarity index 100% rename from internal/engine/ooapi/internal/generator/cloners.go rename to internal/ooapi/internal/generator/cloners.go diff --git a/internal/engine/ooapi/internal/generator/fakeapitest.go b/internal/ooapi/internal/generator/fakeapitest.go similarity index 83% rename from internal/engine/ooapi/internal/generator/fakeapitest.go rename to internal/ooapi/internal/generator/fakeapitest.go index 48b12c2..fa40441 100644 --- a/internal/engine/ooapi/internal/generator/fakeapitest.go +++ b/internal/ooapi/internal/generator/fakeapitest.go @@ -13,12 +13,14 @@ func (d *Descriptor) genNewFakeAPI(sb *strings.Builder) { } fmt.Fprint(sb, "\tErr error\n") fmt.Fprintf(sb, "\tResponse %s\n", d.ResponseTypeName()) - fmt.Fprint(sb, "\tCountCall int32\n") + fmt.Fprint(sb, "\tCountCall *atomicx.Int64\n") fmt.Fprint(sb, "}\n\n") fmt.Fprintf(sb, "func (fapi *%s) Call(ctx context.Context, req %s) (%s, error) {\n", d.FakeAPIStructName(), d.RequestTypeName(), d.ResponseTypeName()) - fmt.Fprint(sb, "\tatomic.AddInt32(&fapi.CountCall, 1)\n") + fmt.Fprint(sb, "\tif fapi.CountCall != nil {\n") + fmt.Fprint(sb, "\t\tfapi.CountCall.Add(1)\n") + fmt.Fprint(sb, "\t}\n") fmt.Fprint(sb, "\treturn fapi.Response, fapi.Err\n") fmt.Fprint(sb, "}\n\n") @@ -48,9 +50,9 @@ func GenFakeAPITestGo(file string) { fmt.Fprintf(&sb, "//go:generate go run ./internal/generator -file %s\n\n", file) fmt.Fprint(&sb, "import (\n") fmt.Fprint(&sb, "\t\"context\"\n") - fmt.Fprint(&sb, "\t\"sync/atomic\"\n") fmt.Fprint(&sb, "\n") - fmt.Fprint(&sb, "\t\"github.com/ooni/probe-cli/v3/internal/engine/ooapi/apimodel\"\n") + fmt.Fprint(&sb, "\t\"github.com/ooni/probe-cli/v3/internal/atomicx\"\n") + fmt.Fprint(&sb, "\t\"github.com/ooni/probe-cli/v3/internal/ooapi/apimodel\"\n") fmt.Fprint(&sb, ")\n") for _, desc := range Descriptors { desc.genNewFakeAPI(&sb) diff --git a/internal/engine/ooapi/internal/generator/generator.go b/internal/ooapi/internal/generator/generator.go similarity index 100% rename from internal/engine/ooapi/internal/generator/generator.go rename to internal/ooapi/internal/generator/generator.go diff --git a/internal/engine/ooapi/internal/generator/login.go b/internal/ooapi/internal/generator/login.go similarity index 98% rename from internal/engine/ooapi/internal/generator/login.go rename to internal/ooapi/internal/generator/login.go index 61dedba..c58ee3d 100644 --- a/internal/engine/ooapi/internal/generator/login.go +++ b/internal/ooapi/internal/generator/login.go @@ -170,7 +170,7 @@ func GenLoginGo(file string) { fmt.Fprint(&sb, "\t\"context\"\n") fmt.Fprint(&sb, "\t\"errors\"\n") fmt.Fprint(&sb, "\n") - fmt.Fprint(&sb, "\t\"github.com/ooni/probe-cli/v3/internal/engine/ooapi/apimodel\"\n") + fmt.Fprint(&sb, "\t\"github.com/ooni/probe-cli/v3/internal/ooapi/apimodel\"\n") fmt.Fprint(&sb, ")\n") for _, desc := range Descriptors { if !desc.RequiresLogin { diff --git a/internal/engine/ooapi/internal/generator/logintest.go b/internal/ooapi/internal/generator/logintest.go similarity index 86% rename from internal/engine/ooapi/internal/generator/logintest.go rename to internal/ooapi/internal/generator/logintest.go index 7960f3c..58122a6 100644 --- a/internal/engine/ooapi/internal/generator/logintest.go +++ b/internal/ooapi/internal/generator/logintest.go @@ -16,6 +16,7 @@ func (d *Descriptor) genTestRegisterAndLoginSuccess(sb *strings.Builder) { fmt.Fprint(sb, "\t\tResponse: &apimodel.RegisterResponse{\n") fmt.Fprint(sb, "\t\t\tClientID: \"antani-antani\",\n") fmt.Fprint(sb, "\t\t},\n") + fmt.Fprint(sb, "\t\tCountCall: &atomicx.Int64{},\n") fmt.Fprint(sb, "\t}\n") fmt.Fprint(sb, "\t\tloginAPI := &FakeLoginAPI{\n") @@ -23,6 +24,7 @@ func (d *Descriptor) genTestRegisterAndLoginSuccess(sb *strings.Builder) { fmt.Fprint(sb, "\t\t\t\tExpire: time.Now().Add(3600*time.Second),\n") fmt.Fprint(sb, "\t\t\t\tToken: \"antani-antani-token\",\n") fmt.Fprint(sb, "\t\t\t},\n") + fmt.Fprint(sb, "\t\t\tCountCall: &atomicx.Int64{},\n") fmt.Fprint(sb, "\t\t}\n") fmt.Fprintf(sb, "\tlogin := &%s{\n", d.WithLoginAPIStructName()) @@ -33,7 +35,7 @@ func (d *Descriptor) genTestRegisterAndLoginSuccess(sb *strings.Builder) { fmt.Fprint(sb, "\t\t},\n") fmt.Fprint(sb, "\t\tRegisterAPI: registerAPI,\n") fmt.Fprint(sb, "\t\tLoginAPI: loginAPI,\n") - fmt.Fprint(sb, "\t\tKVStore: &MemKVStore{},\n") + fmt.Fprint(sb, "\t\tKVStore: &kvstore.Memory{},\n") fmt.Fprint(sb, "\t}\n") fmt.Fprintf(sb, "\tvar req %s\n", d.RequestTypeName()) @@ -50,11 +52,11 @@ func (d *Descriptor) genTestRegisterAndLoginSuccess(sb *strings.Builder) { fmt.Fprint(sb, "\t\tt.Fatal(diff)\n") fmt.Fprint(sb, "\t}\n") - fmt.Fprint(sb, "\tif loginAPI.CountCall != 1 {\n") + fmt.Fprint(sb, "\tif loginAPI.CountCall.Load() != 1 {\n") fmt.Fprint(sb, "\t\tt.Fatal(\"invalid loginAPI.CountCall\")\n") fmt.Fprint(sb, "\t}\n") - fmt.Fprint(sb, "\tif registerAPI.CountCall != 1 {\n") + fmt.Fprint(sb, "\tif registerAPI.CountCall.Load() != 1 {\n") fmt.Fprint(sb, "\t\tt.Fatal(\"invalid registerAPI.CountCall\")\n") fmt.Fprint(sb, "\t}\n") @@ -71,6 +73,7 @@ func (d *Descriptor) genTestContinueUsingToken(sb *strings.Builder) { fmt.Fprint(sb, "\t\tResponse: &apimodel.RegisterResponse{\n") fmt.Fprint(sb, "\t\t\tClientID: \"antani-antani\",\n") fmt.Fprint(sb, "\t\t},\n") + fmt.Fprint(sb, "\t\tCountCall: &atomicx.Int64{},\n") fmt.Fprint(sb, "\t}\n") fmt.Fprint(sb, "\t\tloginAPI := &FakeLoginAPI{\n") @@ -78,6 +81,7 @@ func (d *Descriptor) genTestContinueUsingToken(sb *strings.Builder) { fmt.Fprint(sb, "\t\t\t\tExpire: time.Now().Add(3600*time.Second),\n") fmt.Fprint(sb, "\t\t\t\tToken: \"antani-antani-token\",\n") fmt.Fprint(sb, "\t\t\t},\n") + fmt.Fprint(sb, "\t\t\tCountCall: &atomicx.Int64{},\n") fmt.Fprint(sb, "\t\t}\n") fmt.Fprintf(sb, "\tlogin := &%s{\n", d.WithLoginAPIStructName()) @@ -88,7 +92,7 @@ func (d *Descriptor) genTestContinueUsingToken(sb *strings.Builder) { fmt.Fprint(sb, "\t\t},\n") fmt.Fprint(sb, "\t\tRegisterAPI: registerAPI,\n") fmt.Fprint(sb, "\t\tLoginAPI: loginAPI,\n") - fmt.Fprint(sb, "\t\tKVStore: &MemKVStore{},\n") + fmt.Fprint(sb, "\t\tKVStore: &kvstore.Memory{},\n") fmt.Fprint(sb, "\t}\n") fmt.Fprintf(sb, "\tvar req %s\n", d.RequestTypeName()) @@ -110,11 +114,11 @@ func (d *Descriptor) genTestContinueUsingToken(sb *strings.Builder) { fmt.Fprint(sb, "\t\t\tt.Fatal(diff)\n") fmt.Fprint(sb, "\t\t}\n") - fmt.Fprint(sb, "\t\tif loginAPI.CountCall != 1 {\n") + fmt.Fprint(sb, "\t\tif loginAPI.CountCall.Load() != 1 {\n") fmt.Fprint(sb, "\t\t\tt.Fatal(\"invalid loginAPI.CountCall\")\n") fmt.Fprint(sb, "\t\t}\n") - fmt.Fprint(sb, "\t\tif registerAPI.CountCall != 1 {\n") + fmt.Fprint(sb, "\t\tif registerAPI.CountCall.Load() != 1 {\n") fmt.Fprint(sb, "\t\t\tt.Fatal(\"invalid registerAPI.CountCall\")\n") fmt.Fprint(sb, "\t\t}\n") fmt.Fprint(sb, "\t}\n") @@ -138,11 +142,11 @@ func (d *Descriptor) genTestContinueUsingToken(sb *strings.Builder) { fmt.Fprint(sb, "\t\tt.Fatal(diff)\n") fmt.Fprint(sb, "\t}\n") - fmt.Fprint(sb, "\tif loginAPI.CountCall != 1 {\n") + fmt.Fprint(sb, "\tif loginAPI.CountCall.Load() != 1 {\n") fmt.Fprint(sb, "\t\tt.Fatal(\"invalid loginAPI.CountCall\")\n") fmt.Fprint(sb, "\t}\n") - fmt.Fprint(sb, "\tif registerAPI.CountCall != 1 {\n") + fmt.Fprint(sb, "\tif registerAPI.CountCall.Load() != 1 {\n") fmt.Fprint(sb, "\t\tt.Fatal(\"invalid registerAPI.CountCall\")\n") fmt.Fprint(sb, "\t}\n") @@ -158,6 +162,7 @@ func (d *Descriptor) genTestWithValidButExpiredToken(sb *strings.Builder) { fmt.Fprint(sb, "\terrMocked := errors.New(\"mocked error\")\n") fmt.Fprint(sb, "\tregisterAPI := &FakeRegisterAPI{\n") fmt.Fprint(sb, "\t\tErr: errMocked,\n") + fmt.Fprint(sb, "\t\tCountCall: &atomicx.Int64{},\n") fmt.Fprint(sb, "\t}\n") fmt.Fprint(sb, "\t\tloginAPI := &FakeLoginAPI{\n") @@ -165,6 +170,7 @@ func (d *Descriptor) genTestWithValidButExpiredToken(sb *strings.Builder) { fmt.Fprint(sb, "\t\t\t\tExpire: time.Now().Add(3600*time.Second),\n") fmt.Fprint(sb, "\t\t\t\tToken: \"antani-antani-token\",\n") fmt.Fprint(sb, "\t\t\t},\n") + fmt.Fprint(sb, "\t\t\tCountCall: &atomicx.Int64{},\n") fmt.Fprint(sb, "\t\t}\n") fmt.Fprintf(sb, "\tlogin := &%s{\n", d.WithLoginAPIStructName()) @@ -175,7 +181,7 @@ func (d *Descriptor) genTestWithValidButExpiredToken(sb *strings.Builder) { fmt.Fprint(sb, "\t\t},\n") fmt.Fprint(sb, "\t\tRegisterAPI: registerAPI,\n") fmt.Fprint(sb, "\t\tLoginAPI: loginAPI,\n") - fmt.Fprint(sb, "\t\tKVStore: &MemKVStore{},\n") + fmt.Fprint(sb, "\t\tKVStore: &kvstore.Memory{},\n") fmt.Fprint(sb, "\t}\n") fmt.Fprintf(sb, "\tls := &loginState{\n") @@ -202,11 +208,11 @@ func (d *Descriptor) genTestWithValidButExpiredToken(sb *strings.Builder) { fmt.Fprint(sb, "\t\tt.Fatal(diff)\n") fmt.Fprint(sb, "\t}\n") - fmt.Fprint(sb, "\tif loginAPI.CountCall != 1 {\n") + fmt.Fprint(sb, "\tif loginAPI.CountCall.Load() != 1 {\n") fmt.Fprint(sb, "\t\tt.Fatal(\"invalid loginAPI.CountCall\")\n") fmt.Fprint(sb, "\t}\n") - fmt.Fprint(sb, "\tif registerAPI.CountCall != 0 {\n") + fmt.Fprint(sb, "\tif registerAPI.CountCall.Load() != 0 {\n") fmt.Fprint(sb, "\t\tt.Fatal(\"invalid registerAPI.CountCall\")\n") fmt.Fprint(sb, "\t}\n") @@ -222,6 +228,7 @@ func (d *Descriptor) genTestWithRegisterAPIError(sb *strings.Builder) { fmt.Fprint(sb, "\terrMocked := errors.New(\"mocked error\")\n") fmt.Fprint(sb, "\tregisterAPI := &FakeRegisterAPI{\n") fmt.Fprint(sb, "\t\tErr: errMocked,\n") + fmt.Fprint(sb, "\t\tCountCall: &atomicx.Int64{},\n") fmt.Fprint(sb, "\t}\n") fmt.Fprintf(sb, "\tlogin := &%s{\n", d.WithLoginAPIStructName()) @@ -231,7 +238,7 @@ func (d *Descriptor) genTestWithRegisterAPIError(sb *strings.Builder) { fmt.Fprint(sb, "\t\t\t},\n") fmt.Fprint(sb, "\t\t},\n") fmt.Fprint(sb, "\t\tRegisterAPI: registerAPI,\n") - fmt.Fprint(sb, "\t\tKVStore: &MemKVStore{},\n") + fmt.Fprint(sb, "\t\tKVStore: &kvstore.Memory{},\n") fmt.Fprint(sb, "\t}\n") fmt.Fprintf(sb, "\tvar req %s\n", d.RequestTypeName()) @@ -245,7 +252,7 @@ func (d *Descriptor) genTestWithRegisterAPIError(sb *strings.Builder) { fmt.Fprint(sb, "\t\tt.Fatal(\"expected nil response\")\n") fmt.Fprint(sb, "\t}\n") - fmt.Fprint(sb, "\tif registerAPI.CountCall != 1 {\n") + fmt.Fprint(sb, "\tif registerAPI.CountCall.Load() != 1 {\n") fmt.Fprint(sb, "\t\tt.Fatal(\"invalid registerAPI.CountCall\")\n") fmt.Fprint(sb, "\t}\n") @@ -262,11 +269,13 @@ func (d *Descriptor) genTestWithLoginFailure(sb *strings.Builder) { fmt.Fprint(sb, "\t\tResponse: &apimodel.RegisterResponse{\n") fmt.Fprint(sb, "\t\t\tClientID: \"antani-antani\",\n") fmt.Fprint(sb, "\t\t},\n") + fmt.Fprint(sb, "\t\tCountCall: &atomicx.Int64{},\n") fmt.Fprint(sb, "\t}\n") fmt.Fprint(sb, "\terrMocked := errors.New(\"mocked error\")\n") fmt.Fprint(sb, "\t\tloginAPI := &FakeLoginAPI{\n") fmt.Fprint(sb, "\t\t\tErr: errMocked,\n") + fmt.Fprint(sb, "\t\t\tCountCall: &atomicx.Int64{},\n") fmt.Fprint(sb, "\t\t}\n") fmt.Fprintf(sb, "\tlogin := &%s{\n", d.WithLoginAPIStructName()) @@ -277,7 +286,7 @@ func (d *Descriptor) genTestWithLoginFailure(sb *strings.Builder) { fmt.Fprint(sb, "\t\t},\n") fmt.Fprint(sb, "\t\tRegisterAPI: registerAPI,\n") fmt.Fprint(sb, "\t\tLoginAPI: loginAPI,\n") - fmt.Fprint(sb, "\t\tKVStore: &MemKVStore{},\n") + fmt.Fprint(sb, "\t\tKVStore: &kvstore.Memory{},\n") fmt.Fprint(sb, "\t}\n") fmt.Fprintf(sb, "\tvar req %s\n", d.RequestTypeName()) @@ -291,11 +300,11 @@ func (d *Descriptor) genTestWithLoginFailure(sb *strings.Builder) { fmt.Fprint(sb, "\t\tt.Fatal(\"expected nil response\")\n") fmt.Fprint(sb, "\t}\n") - fmt.Fprint(sb, "\tif loginAPI.CountCall != 1 {\n") + fmt.Fprint(sb, "\tif loginAPI.CountCall.Load() != 1 {\n") fmt.Fprint(sb, "\t\tt.Fatal(\"invalid loginAPI.CountCall\")\n") fmt.Fprint(sb, "\t}\n") - fmt.Fprint(sb, "\tif registerAPI.CountCall != 1 {\n") + fmt.Fprint(sb, "\tif registerAPI.CountCall.Load() != 1 {\n") fmt.Fprint(sb, "\t\tt.Fatal(\"invalid registerAPI.CountCall\")\n") fmt.Fprint(sb, "\t}\n") @@ -312,6 +321,7 @@ func (d *Descriptor) genTestRegisterAndLoginThenFail(sb *strings.Builder) { fmt.Fprint(sb, "\t\tResponse: &apimodel.RegisterResponse{\n") fmt.Fprint(sb, "\t\t\tClientID: \"antani-antani\",\n") fmt.Fprint(sb, "\t\t},\n") + fmt.Fprint(sb, "\t\tCountCall: &atomicx.Int64{},\n") fmt.Fprint(sb, "\t}\n") fmt.Fprint(sb, "\t\tloginAPI := &FakeLoginAPI{\n") @@ -319,6 +329,7 @@ func (d *Descriptor) genTestRegisterAndLoginThenFail(sb *strings.Builder) { fmt.Fprint(sb, "\t\t\t\tExpire: time.Now().Add(3600*time.Second),\n") fmt.Fprint(sb, "\t\t\t\tToken: \"antani-antani-token\",\n") fmt.Fprint(sb, "\t\t\t},\n") + fmt.Fprint(sb, "\t\t\tCountCall: &atomicx.Int64{},\n") fmt.Fprint(sb, "\t\t}\n") fmt.Fprint(sb, "\terrMocked := errors.New(\"mocked error\")\n") @@ -330,7 +341,7 @@ func (d *Descriptor) genTestRegisterAndLoginThenFail(sb *strings.Builder) { fmt.Fprint(sb, "\t\t},\n") fmt.Fprint(sb, "\t\tRegisterAPI: registerAPI,\n") fmt.Fprint(sb, "\t\tLoginAPI: loginAPI,\n") - fmt.Fprint(sb, "\t\tKVStore: &MemKVStore{},\n") + fmt.Fprint(sb, "\t\tKVStore: &kvstore.Memory{},\n") fmt.Fprint(sb, "\t}\n") fmt.Fprintf(sb, "\tvar req %s\n", d.RequestTypeName()) @@ -344,11 +355,11 @@ func (d *Descriptor) genTestRegisterAndLoginThenFail(sb *strings.Builder) { fmt.Fprint(sb, "\t\tt.Fatal(\"expected nil response\")\n") fmt.Fprint(sb, "\t}\n") - fmt.Fprint(sb, "\tif loginAPI.CountCall != 1 {\n") + fmt.Fprint(sb, "\tif loginAPI.CountCall.Load() != 1 {\n") fmt.Fprint(sb, "\t\tt.Fatal(\"invalid loginAPI.CountCall\")\n") fmt.Fprint(sb, "\t}\n") - fmt.Fprint(sb, "\tif registerAPI.CountCall != 1 {\n") + fmt.Fprint(sb, "\tif registerAPI.CountCall.Load() != 1 {\n") fmt.Fprint(sb, "\t\tt.Fatal(\"invalid registerAPI.CountCall\")\n") fmt.Fprint(sb, "\t}\n") @@ -358,7 +369,11 @@ func (d *Descriptor) genTestRegisterAndLoginThenFail(sb *strings.Builder) { func (d *Descriptor) genTestTheDatabaseIsReplaced(sb *strings.Builder) { fmt.Fprintf(sb, "func Test%sTheDatabaseIsReplaced(t *testing.T) {\n", d.Name) fmt.Fprint(sb, "\tff := &fakeFill{}\n") - fmt.Fprint(sb, "\thandler := &LoginHandler{t: t}\n") + fmt.Fprint(sb, "\thandler := &LoginHandler{\n") + fmt.Fprint(sb, "\t\tlogins: &atomicx.Int64{},\n") + fmt.Fprint(sb, "\t\tregisters: &atomicx.Int64{},\n") + fmt.Fprint(sb, "\t\tt: t,\n") + fmt.Fprint(sb, "\t}\n") fmt.Fprint(sb, "\tsrvr := httptest.NewServer(handler)\n") fmt.Fprint(sb, "\tdefer srvr.Close()\n") @@ -379,7 +394,7 @@ func (d *Descriptor) genTestTheDatabaseIsReplaced(sb *strings.Builder) { fmt.Fprintf(sb, "\tAPI : baseAPI,\n") fmt.Fprint(sb, "\tRegisterAPI: registerAPI,\n") fmt.Fprint(sb, "\tLoginAPI: loginAPI,\n") - fmt.Fprint(sb, "\tKVStore: &MemKVStore{},\n") + fmt.Fprint(sb, "\tKVStore: &kvstore.Memory{},\n") fmt.Fprint(sb, "\t}\n") fmt.Fprintf(sb, "\tvar req %s\n", d.RequestTypeName()) @@ -398,11 +413,11 @@ func (d *Descriptor) genTestTheDatabaseIsReplaced(sb *strings.Builder) { fmt.Fprint(sb, "\t\t\tt.Fatal(\"expected non-nil response\")\n") fmt.Fprint(sb, "\t\t}\n") - fmt.Fprint(sb, "\t\tif handler.logins != 1 {\n") + fmt.Fprint(sb, "\t\tif handler.logins.Load() != 1 {\n") fmt.Fprint(sb, "\t\t\tt.Fatal(\"invalid handler.logins\")\n") fmt.Fprint(sb, "\t\t}\n") - fmt.Fprint(sb, "\t\tif handler.registers != 1 {\n") + fmt.Fprint(sb, "\t\tif handler.registers.Load() != 1 {\n") fmt.Fprint(sb, "\t\t\tt.Fatal(\"invalid handler.registers\")\n") fmt.Fprint(sb, "\t\t}\n") fmt.Fprint(sb, "\t}\n") @@ -418,11 +433,11 @@ func (d *Descriptor) genTestTheDatabaseIsReplaced(sb *strings.Builder) { fmt.Fprint(sb, "\t\tt.Fatal(\"expected non-nil response\")\n") fmt.Fprint(sb, "\t}\n") - fmt.Fprint(sb, "\tif handler.logins != 3 {\n") + fmt.Fprint(sb, "\tif handler.logins.Load() != 3 {\n") fmt.Fprint(sb, "\t\tt.Fatal(\"invalid handler.logins\")\n") fmt.Fprint(sb, "\t}\n") - fmt.Fprint(sb, "\tif handler.registers != 2 {\n") + fmt.Fprint(sb, "\tif handler.registers.Load() != 2 {\n") fmt.Fprint(sb, "\t\tt.Fatal(\"invalid handler.registers\")\n") fmt.Fprint(sb, "\t}\n") @@ -432,7 +447,11 @@ func (d *Descriptor) genTestTheDatabaseIsReplaced(sb *strings.Builder) { func (d *Descriptor) genTestTheDatabaseIsReplacedThenFailure(sb *strings.Builder) { fmt.Fprintf(sb, "func Test%sTheDatabaseIsReplacedThenFailure(t *testing.T) {\n", d.Name) fmt.Fprint(sb, "\tff := &fakeFill{}\n") - fmt.Fprint(sb, "\thandler := &LoginHandler{t: t}\n") + fmt.Fprint(sb, "\thandler := &LoginHandler{\n") + fmt.Fprint(sb, "\t\tlogins: &atomicx.Int64{},\n") + fmt.Fprint(sb, "\t\tregisters: &atomicx.Int64{},\n") + fmt.Fprint(sb, "\t\tt: t,\n") + fmt.Fprint(sb, "\t}\n") fmt.Fprint(sb, "\tsrvr := httptest.NewServer(handler)\n") fmt.Fprint(sb, "\tdefer srvr.Close()\n") @@ -453,7 +472,7 @@ func (d *Descriptor) genTestTheDatabaseIsReplacedThenFailure(sb *strings.Builder fmt.Fprintf(sb, "\tAPI : baseAPI,\n") fmt.Fprint(sb, "\tRegisterAPI: registerAPI,\n") fmt.Fprint(sb, "\tLoginAPI: loginAPI,\n") - fmt.Fprint(sb, "\tKVStore: &MemKVStore{},\n") + fmt.Fprint(sb, "\tKVStore: &kvstore.Memory{},\n") fmt.Fprint(sb, "\t}\n") fmt.Fprintf(sb, "\tvar req %s\n", d.RequestTypeName()) @@ -472,11 +491,11 @@ func (d *Descriptor) genTestTheDatabaseIsReplacedThenFailure(sb *strings.Builder fmt.Fprint(sb, "\t\t\tt.Fatal(\"expected non-nil response\")\n") fmt.Fprint(sb, "\t\t}\n") - fmt.Fprint(sb, "\t\tif handler.logins != 1 {\n") + fmt.Fprint(sb, "\t\tif handler.logins.Load() != 1 {\n") fmt.Fprint(sb, "\t\t\tt.Fatal(\"invalid handler.logins\")\n") fmt.Fprint(sb, "\t\t}\n") - fmt.Fprint(sb, "\t\tif handler.registers != 1 {\n") + fmt.Fprint(sb, "\t\tif handler.registers.Load() != 1 {\n") fmt.Fprint(sb, "\t\t\tt.Fatal(\"invalid handler.registers\")\n") fmt.Fprint(sb, "\t\t}\n") fmt.Fprint(sb, "\t}\n") @@ -494,11 +513,11 @@ func (d *Descriptor) genTestTheDatabaseIsReplacedThenFailure(sb *strings.Builder fmt.Fprint(sb, "\t\tt.Fatal(\"expected nil response\")\n") fmt.Fprint(sb, "\t}\n") - fmt.Fprint(sb, "\tif handler.logins != 2 {\n") + fmt.Fprint(sb, "\tif handler.logins.Load() != 2 {\n") fmt.Fprint(sb, "\t\tt.Fatal(\"invalid handler.logins\")\n") fmt.Fprint(sb, "\t}\n") - fmt.Fprint(sb, "\tif handler.registers != 2 {\n") + fmt.Fprint(sb, "\tif handler.registers.Load() != 2 {\n") fmt.Fprint(sb, "\t\tt.Fatal(\"invalid handler.registers\")\n") fmt.Fprint(sb, "\t}\n") @@ -515,6 +534,7 @@ func (d *Descriptor) genTestRegisterAndLoginCannotWriteState(sb *strings.Builder fmt.Fprint(sb, "\t\tResponse: &apimodel.RegisterResponse{\n") fmt.Fprint(sb, "\t\t\tClientID: \"antani-antani\",\n") fmt.Fprint(sb, "\t\t},\n") + fmt.Fprint(sb, "\t\tCountCall: &atomicx.Int64{},\n") fmt.Fprint(sb, "\t}\n") fmt.Fprint(sb, "\t\tloginAPI := &FakeLoginAPI{\n") @@ -522,6 +542,7 @@ func (d *Descriptor) genTestRegisterAndLoginCannotWriteState(sb *strings.Builder fmt.Fprint(sb, "\t\t\t\tExpire: time.Now().Add(3600*time.Second),\n") fmt.Fprint(sb, "\t\t\t\tToken: \"antani-antani-token\",\n") fmt.Fprint(sb, "\t\t\t},\n") + fmt.Fprint(sb, "\t\t\tCountCall: &atomicx.Int64{},\n") fmt.Fprint(sb, "\t\t}\n") fmt.Fprint(sb, "\terrMocked := errors.New(\"mocked error\")\n") @@ -533,7 +554,7 @@ func (d *Descriptor) genTestRegisterAndLoginCannotWriteState(sb *strings.Builder fmt.Fprint(sb, "\t\t},\n") fmt.Fprint(sb, "\t\tRegisterAPI: registerAPI,\n") fmt.Fprint(sb, "\t\tLoginAPI: loginAPI,\n") - fmt.Fprint(sb, "\t\tKVStore: &MemKVStore{},\n") + fmt.Fprint(sb, "\t\tKVStore: &kvstore.Memory{},\n") fmt.Fprint(sb, "\t\tJSONCodec: &FakeCodec{\n") fmt.Fprint(sb, "\t\t\tEncodeErr: errMocked,\n") fmt.Fprint(sb, "\t\t},\n") @@ -550,11 +571,11 @@ func (d *Descriptor) genTestRegisterAndLoginCannotWriteState(sb *strings.Builder fmt.Fprint(sb, "\t\tt.Fatal(\"expected nil response\")\n") fmt.Fprint(sb, "\t}\n") - fmt.Fprint(sb, "\tif loginAPI.CountCall != 1 {\n") + fmt.Fprint(sb, "\tif loginAPI.CountCall.Load() != 1 {\n") fmt.Fprint(sb, "\t\tt.Fatal(\"invalid loginAPI.CountCall\")\n") fmt.Fprint(sb, "\t}\n") - fmt.Fprint(sb, "\tif registerAPI.CountCall != 1 {\n") + fmt.Fprint(sb, "\tif registerAPI.CountCall.Load() != 1 {\n") fmt.Fprint(sb, "\t\tt.Fatal(\"invalid registerAPI.CountCall\")\n") fmt.Fprint(sb, "\t}\n") @@ -570,7 +591,7 @@ func (d *Descriptor) genTestReadStateDecodeFailure(sb *strings.Builder) { fmt.Fprint(sb, "\terrMocked := errors.New(\"mocked error\")\n") fmt.Fprintf(sb, "\tlogin := &%s{\n", d.WithLoginAPIStructName()) - fmt.Fprint(sb, "\t\tKVStore: &MemKVStore{},\n") + fmt.Fprint(sb, "\t\tKVStore: &kvstore.Memory{},\n") fmt.Fprint(sb, "\t\tJSONCodec: &FakeCodec{DecodeErr: errMocked},\n") fmt.Fprint(sb, "\t}\n") @@ -598,7 +619,11 @@ func (d *Descriptor) genTestReadStateDecodeFailure(sb *strings.Builder) { func (d *Descriptor) genTestClockIsOffThenSuccess(sb *strings.Builder) { fmt.Fprintf(sb, "func Test%sClockIsOffThenSuccess(t *testing.T) {\n", d.Name) fmt.Fprint(sb, "\tff := &fakeFill{}\n") - fmt.Fprint(sb, "\thandler := &LoginHandler{t: t}\n") + fmt.Fprint(sb, "\thandler := &LoginHandler{\n") + fmt.Fprint(sb, "\t\tlogins: &atomicx.Int64{},\n") + fmt.Fprint(sb, "\t\tregisters: &atomicx.Int64{},\n") + fmt.Fprint(sb, "\t\tt: t,\n") + fmt.Fprint(sb, "\t}\n") fmt.Fprint(sb, "\tsrvr := httptest.NewServer(handler)\n") fmt.Fprint(sb, "\tdefer srvr.Close()\n") @@ -619,7 +644,7 @@ func (d *Descriptor) genTestClockIsOffThenSuccess(sb *strings.Builder) { fmt.Fprintf(sb, "\tAPI : baseAPI,\n") fmt.Fprint(sb, "\tRegisterAPI: registerAPI,\n") fmt.Fprint(sb, "\tLoginAPI: loginAPI,\n") - fmt.Fprint(sb, "\tKVStore: &MemKVStore{},\n") + fmt.Fprint(sb, "\tKVStore: &kvstore.Memory{},\n") fmt.Fprint(sb, "\t}\n") fmt.Fprintf(sb, "\tvar req %s\n", d.RequestTypeName()) @@ -638,11 +663,11 @@ func (d *Descriptor) genTestClockIsOffThenSuccess(sb *strings.Builder) { fmt.Fprint(sb, "\t\t\tt.Fatal(\"expected non-nil response\")\n") fmt.Fprint(sb, "\t\t}\n") - fmt.Fprint(sb, "\t\tif handler.logins != 1 {\n") + fmt.Fprint(sb, "\t\tif handler.logins.Load() != 1 {\n") fmt.Fprint(sb, "\t\t\tt.Fatal(\"invalid handler.logins\")\n") fmt.Fprint(sb, "\t\t}\n") - fmt.Fprint(sb, "\t\tif handler.registers != 1 {\n") + fmt.Fprint(sb, "\t\tif handler.registers.Load() != 1 {\n") fmt.Fprint(sb, "\t\t\tt.Fatal(\"invalid handler.registers\")\n") fmt.Fprint(sb, "\t\t}\n") fmt.Fprint(sb, "\t}\n") @@ -660,11 +685,11 @@ func (d *Descriptor) genTestClockIsOffThenSuccess(sb *strings.Builder) { fmt.Fprint(sb, "\t\tt.Fatal(\"expected non-nil response\")\n") fmt.Fprint(sb, "\t}\n") - fmt.Fprint(sb, "\tif handler.logins != 2 {\n") + fmt.Fprint(sb, "\tif handler.logins.Load() != 2 {\n") fmt.Fprint(sb, "\t\tt.Fatal(\"invalid handler.logins\")\n") fmt.Fprint(sb, "\t}\n") - fmt.Fprint(sb, "\tif handler.registers != 1 {\n") + fmt.Fprint(sb, "\tif handler.registers.Load() != 1 {\n") fmt.Fprint(sb, "\t\tt.Fatal(\"invalid handler.registers\")\n") fmt.Fprint(sb, "\t}\n") @@ -674,7 +699,11 @@ func (d *Descriptor) genTestClockIsOffThenSuccess(sb *strings.Builder) { func (d *Descriptor) genTestClockIsOffThen401(sb *strings.Builder) { fmt.Fprintf(sb, "func Test%sClockIsOffThen401(t *testing.T) {\n", d.Name) fmt.Fprint(sb, "\tff := &fakeFill{}\n") - fmt.Fprint(sb, "\thandler := &LoginHandler{t: t}\n") + fmt.Fprint(sb, "\thandler := &LoginHandler{\n") + fmt.Fprint(sb, "\t\tlogins: &atomicx.Int64{},\n") + fmt.Fprint(sb, "\t\tregisters: &atomicx.Int64{},\n") + fmt.Fprint(sb, "\t\tt: t,\n") + fmt.Fprint(sb, "\t}\n") fmt.Fprint(sb, "\tsrvr := httptest.NewServer(handler)\n") fmt.Fprint(sb, "\tdefer srvr.Close()\n") @@ -695,7 +724,7 @@ func (d *Descriptor) genTestClockIsOffThen401(sb *strings.Builder) { fmt.Fprintf(sb, "\tAPI : baseAPI,\n") fmt.Fprint(sb, "\tRegisterAPI: registerAPI,\n") fmt.Fprint(sb, "\tLoginAPI: loginAPI,\n") - fmt.Fprint(sb, "\tKVStore: &MemKVStore{},\n") + fmt.Fprint(sb, "\tKVStore: &kvstore.Memory{},\n") fmt.Fprint(sb, "\t}\n") fmt.Fprintf(sb, "\tvar req %s\n", d.RequestTypeName()) @@ -714,11 +743,11 @@ func (d *Descriptor) genTestClockIsOffThen401(sb *strings.Builder) { fmt.Fprint(sb, "\t\t\tt.Fatal(\"expected non-nil response\")\n") fmt.Fprint(sb, "\t\t}\n") - fmt.Fprint(sb, "\t\tif handler.logins != 1 {\n") + fmt.Fprint(sb, "\t\tif handler.logins.Load() != 1 {\n") fmt.Fprint(sb, "\t\t\tt.Fatal(\"invalid handler.logins\")\n") fmt.Fprint(sb, "\t\t}\n") - fmt.Fprint(sb, "\t\tif handler.registers != 1 {\n") + fmt.Fprint(sb, "\t\tif handler.registers.Load() != 1 {\n") fmt.Fprint(sb, "\t\t\tt.Fatal(\"invalid handler.registers\")\n") fmt.Fprint(sb, "\t\t}\n") fmt.Fprint(sb, "\t}\n") @@ -737,11 +766,11 @@ func (d *Descriptor) genTestClockIsOffThen401(sb *strings.Builder) { fmt.Fprint(sb, "\t\tt.Fatal(\"expected non-nil response\")\n") fmt.Fprint(sb, "\t}\n") - fmt.Fprint(sb, "\tif handler.logins != 3 {\n") + fmt.Fprint(sb, "\tif handler.logins.Load() != 3 {\n") fmt.Fprint(sb, "\t\tt.Fatal(\"invalid handler.logins\")\n") fmt.Fprint(sb, "\t}\n") - fmt.Fprint(sb, "\tif handler.registers != 2 {\n") + fmt.Fprint(sb, "\tif handler.registers.Load() != 2 {\n") fmt.Fprint(sb, "\t\tt.Fatal(\"invalid handler.registers\")\n") fmt.Fprint(sb, "\t}\n") @@ -751,7 +780,11 @@ func (d *Descriptor) genTestClockIsOffThen401(sb *strings.Builder) { func (d *Descriptor) genTestClockIsOffThen500(sb *strings.Builder) { fmt.Fprintf(sb, "func Test%sClockIsOffThen500(t *testing.T) {\n", d.Name) fmt.Fprint(sb, "\tff := &fakeFill{}\n") - fmt.Fprint(sb, "\thandler := &LoginHandler{t: t}\n") + fmt.Fprint(sb, "\thandler := &LoginHandler{\n") + fmt.Fprint(sb, "\t\tlogins: &atomicx.Int64{},\n") + fmt.Fprint(sb, "\t\tregisters: &atomicx.Int64{},\n") + fmt.Fprint(sb, "\t\tt: t,\n") + fmt.Fprint(sb, "\t}\n") fmt.Fprint(sb, "\tsrvr := httptest.NewServer(handler)\n") fmt.Fprint(sb, "\tdefer srvr.Close()\n") @@ -772,7 +805,7 @@ func (d *Descriptor) genTestClockIsOffThen500(sb *strings.Builder) { fmt.Fprintf(sb, "\tAPI : baseAPI,\n") fmt.Fprint(sb, "\tRegisterAPI: registerAPI,\n") fmt.Fprint(sb, "\tLoginAPI: loginAPI,\n") - fmt.Fprint(sb, "\tKVStore: &MemKVStore{},\n") + fmt.Fprint(sb, "\tKVStore: &kvstore.Memory{},\n") fmt.Fprint(sb, "\t}\n") fmt.Fprintf(sb, "\tvar req %s\n", d.RequestTypeName()) @@ -791,11 +824,11 @@ func (d *Descriptor) genTestClockIsOffThen500(sb *strings.Builder) { fmt.Fprint(sb, "\t\t\tt.Fatal(\"expected non-nil response\")\n") fmt.Fprint(sb, "\t\t}\n") - fmt.Fprint(sb, "\t\tif handler.logins != 1 {\n") + fmt.Fprint(sb, "\t\tif handler.logins.Load() != 1 {\n") fmt.Fprint(sb, "\t\t\tt.Fatal(\"invalid handler.logins\")\n") fmt.Fprint(sb, "\t\t}\n") - fmt.Fprint(sb, "\t\tif handler.registers != 1 {\n") + fmt.Fprint(sb, "\t\tif handler.registers.Load() != 1 {\n") fmt.Fprint(sb, "\t\t\tt.Fatal(\"invalid handler.registers\")\n") fmt.Fprint(sb, "\t\t}\n") fmt.Fprint(sb, "\t}\n") @@ -814,11 +847,11 @@ func (d *Descriptor) genTestClockIsOffThen500(sb *strings.Builder) { fmt.Fprint(sb, "\t\tt.Fatal(\"expected nil response\")\n") fmt.Fprint(sb, "\t}\n") - fmt.Fprint(sb, "\tif handler.logins != 2 {\n") + fmt.Fprint(sb, "\tif handler.logins.Load() != 2 {\n") fmt.Fprint(sb, "\t\tt.Fatal(\"invalid handler.logins\")\n") fmt.Fprint(sb, "\t}\n") - fmt.Fprint(sb, "\tif handler.registers != 1 {\n") + fmt.Fprint(sb, "\tif handler.registers.Load() != 1 {\n") fmt.Fprint(sb, "\t\tt.Fatal(\"invalid handler.registers\")\n") fmt.Fprint(sb, "\t}\n") @@ -840,7 +873,9 @@ func GenLoginTestGo(file string) { fmt.Fprint(&sb, "\t\"time\"\n") fmt.Fprint(&sb, "\n") fmt.Fprint(&sb, "\t\"github.com/google/go-cmp/cmp\"\n") - fmt.Fprint(&sb, "\t\"github.com/ooni/probe-cli/v3/internal/engine/ooapi/apimodel\"\n") + fmt.Fprint(&sb, "\t\"github.com/ooni/probe-cli/v3/internal/atomicx\"\n") + fmt.Fprint(&sb, "\t\"github.com/ooni/probe-cli/v3/internal/kvstore\"\n") + fmt.Fprint(&sb, "\t\"github.com/ooni/probe-cli/v3/internal/ooapi/apimodel\"\n") fmt.Fprint(&sb, ")\n") for _, desc := range Descriptors { if !desc.RequiresLogin { diff --git a/internal/engine/ooapi/internal/generator/reflect.go b/internal/ooapi/internal/generator/reflect.go similarity index 100% rename from internal/engine/ooapi/internal/generator/reflect.go rename to internal/ooapi/internal/generator/reflect.go diff --git a/internal/engine/ooapi/internal/generator/requests.go b/internal/ooapi/internal/generator/requests.go similarity index 97% rename from internal/engine/ooapi/internal/generator/requests.go rename to internal/ooapi/internal/generator/requests.go index 1e02fa2..2041e21 100644 --- a/internal/engine/ooapi/internal/generator/requests.go +++ b/internal/ooapi/internal/generator/requests.go @@ -132,7 +132,7 @@ func GenRequestsGo(file string) { fmt.Fprint(&sb, "\t\"net/http\"\n") fmt.Fprint(&sb, "\t\"net/url\"\n") fmt.Fprint(&sb, "\n") - fmt.Fprint(&sb, "\t\"github.com/ooni/probe-cli/v3/internal/engine/ooapi/apimodel\"\n") + fmt.Fprint(&sb, "\t\"github.com/ooni/probe-cli/v3/internal/ooapi/apimodel\"\n") fmt.Fprint(&sb, ")\n\n") for _, desc := range Descriptors { desc.genNewRequest(&sb) diff --git a/internal/engine/ooapi/internal/generator/responses.go b/internal/ooapi/internal/generator/responses.go similarity index 96% rename from internal/engine/ooapi/internal/generator/responses.go rename to internal/ooapi/internal/generator/responses.go index 18ce9e2..da81013 100644 --- a/internal/engine/ooapi/internal/generator/responses.go +++ b/internal/ooapi/internal/generator/responses.go @@ -71,7 +71,7 @@ func GenResponsesGo(file string) { fmt.Fprint(&sb, "\t\"io/ioutil\"\n") fmt.Fprint(&sb, "\t\"net/http\"\n") fmt.Fprint(&sb, "\n") - fmt.Fprint(&sb, "\t\"github.com/ooni/probe-cli/v3/internal/engine/ooapi/apimodel\"\n") + fmt.Fprint(&sb, "\t\"github.com/ooni/probe-cli/v3/internal/ooapi/apimodel\"\n") fmt.Fprint(&sb, ")\n\n") for _, desc := range Descriptors { desc.genNewResponse(&sb) diff --git a/internal/engine/ooapi/internal/generator/spec.go b/internal/ooapi/internal/generator/spec.go similarity index 98% rename from internal/engine/ooapi/internal/generator/spec.go rename to internal/ooapi/internal/generator/spec.go index 01bd0c8..a8a6cde 100644 --- a/internal/engine/ooapi/internal/generator/spec.go +++ b/internal/ooapi/internal/generator/spec.go @@ -1,6 +1,6 @@ package main -import "github.com/ooni/probe-cli/v3/internal/engine/ooapi/apimodel" +import "github.com/ooni/probe-cli/v3/internal/ooapi/apimodel" // URLPath describes a URLPath. type URLPath struct { diff --git a/internal/engine/ooapi/internal/generator/swaggertest.go b/internal/ooapi/internal/generator/swaggertest.go similarity index 98% rename from internal/engine/ooapi/internal/generator/swaggertest.go rename to internal/ooapi/internal/generator/swaggertest.go index 3fadc38..96ed826 100644 --- a/internal/engine/ooapi/internal/generator/swaggertest.go +++ b/internal/ooapi/internal/generator/swaggertest.go @@ -9,7 +9,7 @@ import ( "sync" "time" - "github.com/ooni/probe-cli/v3/internal/engine/ooapi/internal/openapi" + "github.com/ooni/probe-cli/v3/internal/ooapi/internal/openapi" ) const ( diff --git a/internal/engine/ooapi/internal/generator/writefile.go b/internal/ooapi/internal/generator/writefile.go similarity index 100% rename from internal/engine/ooapi/internal/generator/writefile.go rename to internal/ooapi/internal/generator/writefile.go diff --git a/internal/engine/ooapi/internal/openapi/openapi.go b/internal/ooapi/internal/openapi/openapi.go similarity index 100% rename from internal/engine/ooapi/internal/openapi/openapi.go rename to internal/ooapi/internal/openapi/openapi.go diff --git a/internal/engine/ooapi/login.go b/internal/ooapi/login.go similarity index 99% rename from internal/engine/ooapi/login.go rename to internal/ooapi/login.go index 5c8f2b4..2c52aca 100644 --- a/internal/engine/ooapi/login.go +++ b/internal/ooapi/login.go @@ -9,7 +9,7 @@ import ( "context" "errors" - "github.com/ooni/probe-cli/v3/internal/engine/ooapi/apimodel" + "github.com/ooni/probe-cli/v3/internal/ooapi/apimodel" ) // withLoginPsiphonConfigAPI implements login for simplePsiphonConfigAPI. diff --git a/internal/engine/ooapi/login_test.go b/internal/ooapi/login_test.go similarity index 84% rename from internal/engine/ooapi/login_test.go rename to internal/ooapi/login_test.go index a6e7624..8131618 100644 --- a/internal/engine/ooapi/login_test.go +++ b/internal/ooapi/login_test.go @@ -13,7 +13,9 @@ import ( "time" "github.com/google/go-cmp/cmp" - "github.com/ooni/probe-cli/v3/internal/engine/ooapi/apimodel" + "github.com/ooni/probe-cli/v3/internal/atomicx" + "github.com/ooni/probe-cli/v3/internal/kvstore" + "github.com/ooni/probe-cli/v3/internal/ooapi/apimodel" ) func TestRegisterAndLoginPsiphonConfigSuccess(t *testing.T) { @@ -24,12 +26,14 @@ func TestRegisterAndLoginPsiphonConfigSuccess(t *testing.T) { Response: &apimodel.RegisterResponse{ ClientID: "antani-antani", }, + CountCall: &atomicx.Int64{}, } loginAPI := &FakeLoginAPI{ Response: &apimodel.LoginResponse{ Expire: time.Now().Add(3600 * time.Second), Token: "antani-antani-token", }, + CountCall: &atomicx.Int64{}, } login := &withLoginPsiphonConfigAPI{ API: &FakePsiphonConfigAPI{ @@ -39,7 +43,7 @@ func TestRegisterAndLoginPsiphonConfigSuccess(t *testing.T) { }, RegisterAPI: registerAPI, LoginAPI: loginAPI, - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, } var req *apimodel.PsiphonConfigRequest ff.fill(&req) @@ -54,10 +58,10 @@ func TestRegisterAndLoginPsiphonConfigSuccess(t *testing.T) { if diff := cmp.Diff(expect, resp); diff != "" { t.Fatal(diff) } - if loginAPI.CountCall != 1 { + if loginAPI.CountCall.Load() != 1 { t.Fatal("invalid loginAPI.CountCall") } - if registerAPI.CountCall != 1 { + if registerAPI.CountCall.Load() != 1 { t.Fatal("invalid registerAPI.CountCall") } } @@ -70,12 +74,14 @@ func TestPsiphonConfigContinueUsingToken(t *testing.T) { Response: &apimodel.RegisterResponse{ ClientID: "antani-antani", }, + CountCall: &atomicx.Int64{}, } loginAPI := &FakeLoginAPI{ Response: &apimodel.LoginResponse{ Expire: time.Now().Add(3600 * time.Second), Token: "antani-antani-token", }, + CountCall: &atomicx.Int64{}, } login := &withLoginPsiphonConfigAPI{ API: &FakePsiphonConfigAPI{ @@ -85,7 +91,7 @@ func TestPsiphonConfigContinueUsingToken(t *testing.T) { }, RegisterAPI: registerAPI, LoginAPI: loginAPI, - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, } var req *apimodel.PsiphonConfigRequest ff.fill(&req) @@ -103,10 +109,10 @@ func TestPsiphonConfigContinueUsingToken(t *testing.T) { if diff := cmp.Diff(expect, resp); diff != "" { t.Fatal(diff) } - if loginAPI.CountCall != 1 { + if loginAPI.CountCall.Load() != 1 { t.Fatal("invalid loginAPI.CountCall") } - if registerAPI.CountCall != 1 { + if registerAPI.CountCall.Load() != 1 { t.Fatal("invalid registerAPI.CountCall") } } @@ -127,10 +133,10 @@ func TestPsiphonConfigContinueUsingToken(t *testing.T) { if diff := cmp.Diff(expect, resp); diff != "" { t.Fatal(diff) } - if loginAPI.CountCall != 1 { + if loginAPI.CountCall.Load() != 1 { t.Fatal("invalid loginAPI.CountCall") } - if registerAPI.CountCall != 1 { + if registerAPI.CountCall.Load() != 1 { t.Fatal("invalid registerAPI.CountCall") } } @@ -141,13 +147,15 @@ func TestPsiphonConfigWithValidButExpiredToken(t *testing.T) { ff.fill(&expect) errMocked := errors.New("mocked error") registerAPI := &FakeRegisterAPI{ - Err: errMocked, + Err: errMocked, + CountCall: &atomicx.Int64{}, } loginAPI := &FakeLoginAPI{ Response: &apimodel.LoginResponse{ Expire: time.Now().Add(3600 * time.Second), Token: "antani-antani-token", }, + CountCall: &atomicx.Int64{}, } login := &withLoginPsiphonConfigAPI{ API: &FakePsiphonConfigAPI{ @@ -157,7 +165,7 @@ func TestPsiphonConfigWithValidButExpiredToken(t *testing.T) { }, RegisterAPI: registerAPI, LoginAPI: loginAPI, - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, } ls := &loginState{ ClientID: "antani-antani", @@ -181,10 +189,10 @@ func TestPsiphonConfigWithValidButExpiredToken(t *testing.T) { if diff := cmp.Diff(expect, resp); diff != "" { t.Fatal(diff) } - if loginAPI.CountCall != 1 { + if loginAPI.CountCall.Load() != 1 { t.Fatal("invalid loginAPI.CountCall") } - if registerAPI.CountCall != 0 { + if registerAPI.CountCall.Load() != 0 { t.Fatal("invalid registerAPI.CountCall") } } @@ -195,7 +203,8 @@ func TestPsiphonConfigWithRegisterAPIError(t *testing.T) { ff.fill(&expect) errMocked := errors.New("mocked error") registerAPI := &FakeRegisterAPI{ - Err: errMocked, + Err: errMocked, + CountCall: &atomicx.Int64{}, } login := &withLoginPsiphonConfigAPI{ API: &FakePsiphonConfigAPI{ @@ -204,7 +213,7 @@ func TestPsiphonConfigWithRegisterAPIError(t *testing.T) { }, }, RegisterAPI: registerAPI, - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, } var req *apimodel.PsiphonConfigRequest ff.fill(&req) @@ -216,7 +225,7 @@ func TestPsiphonConfigWithRegisterAPIError(t *testing.T) { if resp != nil { t.Fatal("expected nil response") } - if registerAPI.CountCall != 1 { + if registerAPI.CountCall.Load() != 1 { t.Fatal("invalid registerAPI.CountCall") } } @@ -229,10 +238,12 @@ func TestPsiphonConfigWithLoginFailure(t *testing.T) { Response: &apimodel.RegisterResponse{ ClientID: "antani-antani", }, + CountCall: &atomicx.Int64{}, } errMocked := errors.New("mocked error") loginAPI := &FakeLoginAPI{ - Err: errMocked, + Err: errMocked, + CountCall: &atomicx.Int64{}, } login := &withLoginPsiphonConfigAPI{ API: &FakePsiphonConfigAPI{ @@ -242,7 +253,7 @@ func TestPsiphonConfigWithLoginFailure(t *testing.T) { }, RegisterAPI: registerAPI, LoginAPI: loginAPI, - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, } var req *apimodel.PsiphonConfigRequest ff.fill(&req) @@ -254,10 +265,10 @@ func TestPsiphonConfigWithLoginFailure(t *testing.T) { if resp != nil { t.Fatal("expected nil response") } - if loginAPI.CountCall != 1 { + if loginAPI.CountCall.Load() != 1 { t.Fatal("invalid loginAPI.CountCall") } - if registerAPI.CountCall != 1 { + if registerAPI.CountCall.Load() != 1 { t.Fatal("invalid registerAPI.CountCall") } } @@ -270,12 +281,14 @@ func TestRegisterAndLoginPsiphonConfigThenFail(t *testing.T) { Response: &apimodel.RegisterResponse{ ClientID: "antani-antani", }, + CountCall: &atomicx.Int64{}, } loginAPI := &FakeLoginAPI{ Response: &apimodel.LoginResponse{ Expire: time.Now().Add(3600 * time.Second), Token: "antani-antani-token", }, + CountCall: &atomicx.Int64{}, } errMocked := errors.New("mocked error") login := &withLoginPsiphonConfigAPI{ @@ -286,7 +299,7 @@ func TestRegisterAndLoginPsiphonConfigThenFail(t *testing.T) { }, RegisterAPI: registerAPI, LoginAPI: loginAPI, - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, } var req *apimodel.PsiphonConfigRequest ff.fill(&req) @@ -298,17 +311,21 @@ func TestRegisterAndLoginPsiphonConfigThenFail(t *testing.T) { if resp != nil { t.Fatal("expected nil response") } - if loginAPI.CountCall != 1 { + if loginAPI.CountCall.Load() != 1 { t.Fatal("invalid loginAPI.CountCall") } - if registerAPI.CountCall != 1 { + if registerAPI.CountCall.Load() != 1 { t.Fatal("invalid registerAPI.CountCall") } } func TestPsiphonConfigTheDatabaseIsReplaced(t *testing.T) { ff := &fakeFill{} - handler := &LoginHandler{t: t} + handler := &LoginHandler{ + logins: &atomicx.Int64{}, + registers: &atomicx.Int64{}, + t: t, + } srvr := httptest.NewServer(handler) defer srvr.Close() registerAPI := &simpleRegisterAPI{ @@ -327,7 +344,7 @@ func TestPsiphonConfigTheDatabaseIsReplaced(t *testing.T) { API: baseAPI, RegisterAPI: registerAPI, LoginAPI: loginAPI, - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, } var req *apimodel.PsiphonConfigRequest ff.fill(&req) @@ -342,10 +359,10 @@ func TestPsiphonConfigTheDatabaseIsReplaced(t *testing.T) { if resp == nil { t.Fatal("expected non-nil response") } - if handler.logins != 1 { + if handler.logins.Load() != 1 { t.Fatal("invalid handler.logins") } - if handler.registers != 1 { + if handler.registers.Load() != 1 { t.Fatal("invalid handler.registers") } } @@ -358,10 +375,10 @@ func TestPsiphonConfigTheDatabaseIsReplaced(t *testing.T) { if resp == nil { t.Fatal("expected non-nil response") } - if handler.logins != 3 { + if handler.logins.Load() != 3 { t.Fatal("invalid handler.logins") } - if handler.registers != 2 { + if handler.registers.Load() != 2 { t.Fatal("invalid handler.registers") } } @@ -374,12 +391,14 @@ func TestRegisterAndLoginPsiphonConfigCannotWriteState(t *testing.T) { Response: &apimodel.RegisterResponse{ ClientID: "antani-antani", }, + CountCall: &atomicx.Int64{}, } loginAPI := &FakeLoginAPI{ Response: &apimodel.LoginResponse{ Expire: time.Now().Add(3600 * time.Second), Token: "antani-antani-token", }, + CountCall: &atomicx.Int64{}, } errMocked := errors.New("mocked error") login := &withLoginPsiphonConfigAPI{ @@ -390,7 +409,7 @@ func TestRegisterAndLoginPsiphonConfigCannotWriteState(t *testing.T) { }, RegisterAPI: registerAPI, LoginAPI: loginAPI, - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, JSONCodec: &FakeCodec{ EncodeErr: errMocked, }, @@ -405,10 +424,10 @@ func TestRegisterAndLoginPsiphonConfigCannotWriteState(t *testing.T) { if resp != nil { t.Fatal("expected nil response") } - if loginAPI.CountCall != 1 { + if loginAPI.CountCall.Load() != 1 { t.Fatal("invalid loginAPI.CountCall") } - if registerAPI.CountCall != 1 { + if registerAPI.CountCall.Load() != 1 { t.Fatal("invalid registerAPI.CountCall") } } @@ -419,7 +438,7 @@ func TestPsiphonConfigReadStateDecodeFailure(t *testing.T) { ff.fill(&expect) errMocked := errors.New("mocked error") login := &withLoginPsiphonConfigAPI{ - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, JSONCodec: &FakeCodec{DecodeErr: errMocked}, } ls := &loginState{ @@ -442,7 +461,11 @@ func TestPsiphonConfigReadStateDecodeFailure(t *testing.T) { func TestPsiphonConfigTheDatabaseIsReplacedThenFailure(t *testing.T) { ff := &fakeFill{} - handler := &LoginHandler{t: t} + handler := &LoginHandler{ + logins: &atomicx.Int64{}, + registers: &atomicx.Int64{}, + t: t, + } srvr := httptest.NewServer(handler) defer srvr.Close() registerAPI := &simpleRegisterAPI{ @@ -461,7 +484,7 @@ func TestPsiphonConfigTheDatabaseIsReplacedThenFailure(t *testing.T) { API: baseAPI, RegisterAPI: registerAPI, LoginAPI: loginAPI, - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, } var req *apimodel.PsiphonConfigRequest ff.fill(&req) @@ -476,10 +499,10 @@ func TestPsiphonConfigTheDatabaseIsReplacedThenFailure(t *testing.T) { if resp == nil { t.Fatal("expected non-nil response") } - if handler.logins != 1 { + if handler.logins.Load() != 1 { t.Fatal("invalid handler.logins") } - if handler.registers != 1 { + if handler.registers.Load() != 1 { t.Fatal("invalid handler.registers") } } @@ -494,17 +517,21 @@ func TestPsiphonConfigTheDatabaseIsReplacedThenFailure(t *testing.T) { if resp != nil { t.Fatal("expected nil response") } - if handler.logins != 2 { + if handler.logins.Load() != 2 { t.Fatal("invalid handler.logins") } - if handler.registers != 2 { + if handler.registers.Load() != 2 { t.Fatal("invalid handler.registers") } } func TestPsiphonConfigClockIsOffThenSuccess(t *testing.T) { ff := &fakeFill{} - handler := &LoginHandler{t: t} + handler := &LoginHandler{ + logins: &atomicx.Int64{}, + registers: &atomicx.Int64{}, + t: t, + } srvr := httptest.NewServer(handler) defer srvr.Close() registerAPI := &simpleRegisterAPI{ @@ -523,7 +550,7 @@ func TestPsiphonConfigClockIsOffThenSuccess(t *testing.T) { API: baseAPI, RegisterAPI: registerAPI, LoginAPI: loginAPI, - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, } var req *apimodel.PsiphonConfigRequest ff.fill(&req) @@ -538,10 +565,10 @@ func TestPsiphonConfigClockIsOffThenSuccess(t *testing.T) { if resp == nil { t.Fatal("expected non-nil response") } - if handler.logins != 1 { + if handler.logins.Load() != 1 { t.Fatal("invalid handler.logins") } - if handler.registers != 1 { + if handler.registers.Load() != 1 { t.Fatal("invalid handler.registers") } } @@ -556,17 +583,21 @@ func TestPsiphonConfigClockIsOffThenSuccess(t *testing.T) { if resp == nil { t.Fatal("expected non-nil response") } - if handler.logins != 2 { + if handler.logins.Load() != 2 { t.Fatal("invalid handler.logins") } - if handler.registers != 1 { + if handler.registers.Load() != 1 { t.Fatal("invalid handler.registers") } } func TestPsiphonConfigClockIsOffThen401(t *testing.T) { ff := &fakeFill{} - handler := &LoginHandler{t: t} + handler := &LoginHandler{ + logins: &atomicx.Int64{}, + registers: &atomicx.Int64{}, + t: t, + } srvr := httptest.NewServer(handler) defer srvr.Close() registerAPI := &simpleRegisterAPI{ @@ -585,7 +616,7 @@ func TestPsiphonConfigClockIsOffThen401(t *testing.T) { API: baseAPI, RegisterAPI: registerAPI, LoginAPI: loginAPI, - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, } var req *apimodel.PsiphonConfigRequest ff.fill(&req) @@ -600,10 +631,10 @@ func TestPsiphonConfigClockIsOffThen401(t *testing.T) { if resp == nil { t.Fatal("expected non-nil response") } - if handler.logins != 1 { + if handler.logins.Load() != 1 { t.Fatal("invalid handler.logins") } - if handler.registers != 1 { + if handler.registers.Load() != 1 { t.Fatal("invalid handler.registers") } } @@ -619,17 +650,21 @@ func TestPsiphonConfigClockIsOffThen401(t *testing.T) { if resp == nil { t.Fatal("expected non-nil response") } - if handler.logins != 3 { + if handler.logins.Load() != 3 { t.Fatal("invalid handler.logins") } - if handler.registers != 2 { + if handler.registers.Load() != 2 { t.Fatal("invalid handler.registers") } } func TestPsiphonConfigClockIsOffThen500(t *testing.T) { ff := &fakeFill{} - handler := &LoginHandler{t: t} + handler := &LoginHandler{ + logins: &atomicx.Int64{}, + registers: &atomicx.Int64{}, + t: t, + } srvr := httptest.NewServer(handler) defer srvr.Close() registerAPI := &simpleRegisterAPI{ @@ -648,7 +683,7 @@ func TestPsiphonConfigClockIsOffThen500(t *testing.T) { API: baseAPI, RegisterAPI: registerAPI, LoginAPI: loginAPI, - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, } var req *apimodel.PsiphonConfigRequest ff.fill(&req) @@ -663,10 +698,10 @@ func TestPsiphonConfigClockIsOffThen500(t *testing.T) { if resp == nil { t.Fatal("expected non-nil response") } - if handler.logins != 1 { + if handler.logins.Load() != 1 { t.Fatal("invalid handler.logins") } - if handler.registers != 1 { + if handler.registers.Load() != 1 { t.Fatal("invalid handler.registers") } } @@ -682,10 +717,10 @@ func TestPsiphonConfigClockIsOffThen500(t *testing.T) { if resp != nil { t.Fatal("expected nil response") } - if handler.logins != 2 { + if handler.logins.Load() != 2 { t.Fatal("invalid handler.logins") } - if handler.registers != 1 { + if handler.registers.Load() != 1 { t.Fatal("invalid handler.registers") } } @@ -698,12 +733,14 @@ func TestRegisterAndLoginTorTargetsSuccess(t *testing.T) { Response: &apimodel.RegisterResponse{ ClientID: "antani-antani", }, + CountCall: &atomicx.Int64{}, } loginAPI := &FakeLoginAPI{ Response: &apimodel.LoginResponse{ Expire: time.Now().Add(3600 * time.Second), Token: "antani-antani-token", }, + CountCall: &atomicx.Int64{}, } login := &withLoginTorTargetsAPI{ API: &FakeTorTargetsAPI{ @@ -713,7 +750,7 @@ func TestRegisterAndLoginTorTargetsSuccess(t *testing.T) { }, RegisterAPI: registerAPI, LoginAPI: loginAPI, - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, } var req *apimodel.TorTargetsRequest ff.fill(&req) @@ -728,10 +765,10 @@ func TestRegisterAndLoginTorTargetsSuccess(t *testing.T) { if diff := cmp.Diff(expect, resp); diff != "" { t.Fatal(diff) } - if loginAPI.CountCall != 1 { + if loginAPI.CountCall.Load() != 1 { t.Fatal("invalid loginAPI.CountCall") } - if registerAPI.CountCall != 1 { + if registerAPI.CountCall.Load() != 1 { t.Fatal("invalid registerAPI.CountCall") } } @@ -744,12 +781,14 @@ func TestTorTargetsContinueUsingToken(t *testing.T) { Response: &apimodel.RegisterResponse{ ClientID: "antani-antani", }, + CountCall: &atomicx.Int64{}, } loginAPI := &FakeLoginAPI{ Response: &apimodel.LoginResponse{ Expire: time.Now().Add(3600 * time.Second), Token: "antani-antani-token", }, + CountCall: &atomicx.Int64{}, } login := &withLoginTorTargetsAPI{ API: &FakeTorTargetsAPI{ @@ -759,7 +798,7 @@ func TestTorTargetsContinueUsingToken(t *testing.T) { }, RegisterAPI: registerAPI, LoginAPI: loginAPI, - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, } var req *apimodel.TorTargetsRequest ff.fill(&req) @@ -777,10 +816,10 @@ func TestTorTargetsContinueUsingToken(t *testing.T) { if diff := cmp.Diff(expect, resp); diff != "" { t.Fatal(diff) } - if loginAPI.CountCall != 1 { + if loginAPI.CountCall.Load() != 1 { t.Fatal("invalid loginAPI.CountCall") } - if registerAPI.CountCall != 1 { + if registerAPI.CountCall.Load() != 1 { t.Fatal("invalid registerAPI.CountCall") } } @@ -801,10 +840,10 @@ func TestTorTargetsContinueUsingToken(t *testing.T) { if diff := cmp.Diff(expect, resp); diff != "" { t.Fatal(diff) } - if loginAPI.CountCall != 1 { + if loginAPI.CountCall.Load() != 1 { t.Fatal("invalid loginAPI.CountCall") } - if registerAPI.CountCall != 1 { + if registerAPI.CountCall.Load() != 1 { t.Fatal("invalid registerAPI.CountCall") } } @@ -815,13 +854,15 @@ func TestTorTargetsWithValidButExpiredToken(t *testing.T) { ff.fill(&expect) errMocked := errors.New("mocked error") registerAPI := &FakeRegisterAPI{ - Err: errMocked, + Err: errMocked, + CountCall: &atomicx.Int64{}, } loginAPI := &FakeLoginAPI{ Response: &apimodel.LoginResponse{ Expire: time.Now().Add(3600 * time.Second), Token: "antani-antani-token", }, + CountCall: &atomicx.Int64{}, } login := &withLoginTorTargetsAPI{ API: &FakeTorTargetsAPI{ @@ -831,7 +872,7 @@ func TestTorTargetsWithValidButExpiredToken(t *testing.T) { }, RegisterAPI: registerAPI, LoginAPI: loginAPI, - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, } ls := &loginState{ ClientID: "antani-antani", @@ -855,10 +896,10 @@ func TestTorTargetsWithValidButExpiredToken(t *testing.T) { if diff := cmp.Diff(expect, resp); diff != "" { t.Fatal(diff) } - if loginAPI.CountCall != 1 { + if loginAPI.CountCall.Load() != 1 { t.Fatal("invalid loginAPI.CountCall") } - if registerAPI.CountCall != 0 { + if registerAPI.CountCall.Load() != 0 { t.Fatal("invalid registerAPI.CountCall") } } @@ -869,7 +910,8 @@ func TestTorTargetsWithRegisterAPIError(t *testing.T) { ff.fill(&expect) errMocked := errors.New("mocked error") registerAPI := &FakeRegisterAPI{ - Err: errMocked, + Err: errMocked, + CountCall: &atomicx.Int64{}, } login := &withLoginTorTargetsAPI{ API: &FakeTorTargetsAPI{ @@ -878,7 +920,7 @@ func TestTorTargetsWithRegisterAPIError(t *testing.T) { }, }, RegisterAPI: registerAPI, - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, } var req *apimodel.TorTargetsRequest ff.fill(&req) @@ -890,7 +932,7 @@ func TestTorTargetsWithRegisterAPIError(t *testing.T) { if resp != nil { t.Fatal("expected nil response") } - if registerAPI.CountCall != 1 { + if registerAPI.CountCall.Load() != 1 { t.Fatal("invalid registerAPI.CountCall") } } @@ -903,10 +945,12 @@ func TestTorTargetsWithLoginFailure(t *testing.T) { Response: &apimodel.RegisterResponse{ ClientID: "antani-antani", }, + CountCall: &atomicx.Int64{}, } errMocked := errors.New("mocked error") loginAPI := &FakeLoginAPI{ - Err: errMocked, + Err: errMocked, + CountCall: &atomicx.Int64{}, } login := &withLoginTorTargetsAPI{ API: &FakeTorTargetsAPI{ @@ -916,7 +960,7 @@ func TestTorTargetsWithLoginFailure(t *testing.T) { }, RegisterAPI: registerAPI, LoginAPI: loginAPI, - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, } var req *apimodel.TorTargetsRequest ff.fill(&req) @@ -928,10 +972,10 @@ func TestTorTargetsWithLoginFailure(t *testing.T) { if resp != nil { t.Fatal("expected nil response") } - if loginAPI.CountCall != 1 { + if loginAPI.CountCall.Load() != 1 { t.Fatal("invalid loginAPI.CountCall") } - if registerAPI.CountCall != 1 { + if registerAPI.CountCall.Load() != 1 { t.Fatal("invalid registerAPI.CountCall") } } @@ -944,12 +988,14 @@ func TestRegisterAndLoginTorTargetsThenFail(t *testing.T) { Response: &apimodel.RegisterResponse{ ClientID: "antani-antani", }, + CountCall: &atomicx.Int64{}, } loginAPI := &FakeLoginAPI{ Response: &apimodel.LoginResponse{ Expire: time.Now().Add(3600 * time.Second), Token: "antani-antani-token", }, + CountCall: &atomicx.Int64{}, } errMocked := errors.New("mocked error") login := &withLoginTorTargetsAPI{ @@ -960,7 +1006,7 @@ func TestRegisterAndLoginTorTargetsThenFail(t *testing.T) { }, RegisterAPI: registerAPI, LoginAPI: loginAPI, - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, } var req *apimodel.TorTargetsRequest ff.fill(&req) @@ -972,17 +1018,21 @@ func TestRegisterAndLoginTorTargetsThenFail(t *testing.T) { if resp != nil { t.Fatal("expected nil response") } - if loginAPI.CountCall != 1 { + if loginAPI.CountCall.Load() != 1 { t.Fatal("invalid loginAPI.CountCall") } - if registerAPI.CountCall != 1 { + if registerAPI.CountCall.Load() != 1 { t.Fatal("invalid registerAPI.CountCall") } } func TestTorTargetsTheDatabaseIsReplaced(t *testing.T) { ff := &fakeFill{} - handler := &LoginHandler{t: t} + handler := &LoginHandler{ + logins: &atomicx.Int64{}, + registers: &atomicx.Int64{}, + t: t, + } srvr := httptest.NewServer(handler) defer srvr.Close() registerAPI := &simpleRegisterAPI{ @@ -1001,7 +1051,7 @@ func TestTorTargetsTheDatabaseIsReplaced(t *testing.T) { API: baseAPI, RegisterAPI: registerAPI, LoginAPI: loginAPI, - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, } var req *apimodel.TorTargetsRequest ff.fill(&req) @@ -1016,10 +1066,10 @@ func TestTorTargetsTheDatabaseIsReplaced(t *testing.T) { if resp == nil { t.Fatal("expected non-nil response") } - if handler.logins != 1 { + if handler.logins.Load() != 1 { t.Fatal("invalid handler.logins") } - if handler.registers != 1 { + if handler.registers.Load() != 1 { t.Fatal("invalid handler.registers") } } @@ -1032,10 +1082,10 @@ func TestTorTargetsTheDatabaseIsReplaced(t *testing.T) { if resp == nil { t.Fatal("expected non-nil response") } - if handler.logins != 3 { + if handler.logins.Load() != 3 { t.Fatal("invalid handler.logins") } - if handler.registers != 2 { + if handler.registers.Load() != 2 { t.Fatal("invalid handler.registers") } } @@ -1048,12 +1098,14 @@ func TestRegisterAndLoginTorTargetsCannotWriteState(t *testing.T) { Response: &apimodel.RegisterResponse{ ClientID: "antani-antani", }, + CountCall: &atomicx.Int64{}, } loginAPI := &FakeLoginAPI{ Response: &apimodel.LoginResponse{ Expire: time.Now().Add(3600 * time.Second), Token: "antani-antani-token", }, + CountCall: &atomicx.Int64{}, } errMocked := errors.New("mocked error") login := &withLoginTorTargetsAPI{ @@ -1064,7 +1116,7 @@ func TestRegisterAndLoginTorTargetsCannotWriteState(t *testing.T) { }, RegisterAPI: registerAPI, LoginAPI: loginAPI, - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, JSONCodec: &FakeCodec{ EncodeErr: errMocked, }, @@ -1079,10 +1131,10 @@ func TestRegisterAndLoginTorTargetsCannotWriteState(t *testing.T) { if resp != nil { t.Fatal("expected nil response") } - if loginAPI.CountCall != 1 { + if loginAPI.CountCall.Load() != 1 { t.Fatal("invalid loginAPI.CountCall") } - if registerAPI.CountCall != 1 { + if registerAPI.CountCall.Load() != 1 { t.Fatal("invalid registerAPI.CountCall") } } @@ -1093,7 +1145,7 @@ func TestTorTargetsReadStateDecodeFailure(t *testing.T) { ff.fill(&expect) errMocked := errors.New("mocked error") login := &withLoginTorTargetsAPI{ - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, JSONCodec: &FakeCodec{DecodeErr: errMocked}, } ls := &loginState{ @@ -1116,7 +1168,11 @@ func TestTorTargetsReadStateDecodeFailure(t *testing.T) { func TestTorTargetsTheDatabaseIsReplacedThenFailure(t *testing.T) { ff := &fakeFill{} - handler := &LoginHandler{t: t} + handler := &LoginHandler{ + logins: &atomicx.Int64{}, + registers: &atomicx.Int64{}, + t: t, + } srvr := httptest.NewServer(handler) defer srvr.Close() registerAPI := &simpleRegisterAPI{ @@ -1135,7 +1191,7 @@ func TestTorTargetsTheDatabaseIsReplacedThenFailure(t *testing.T) { API: baseAPI, RegisterAPI: registerAPI, LoginAPI: loginAPI, - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, } var req *apimodel.TorTargetsRequest ff.fill(&req) @@ -1150,10 +1206,10 @@ func TestTorTargetsTheDatabaseIsReplacedThenFailure(t *testing.T) { if resp == nil { t.Fatal("expected non-nil response") } - if handler.logins != 1 { + if handler.logins.Load() != 1 { t.Fatal("invalid handler.logins") } - if handler.registers != 1 { + if handler.registers.Load() != 1 { t.Fatal("invalid handler.registers") } } @@ -1168,17 +1224,21 @@ func TestTorTargetsTheDatabaseIsReplacedThenFailure(t *testing.T) { if resp != nil { t.Fatal("expected nil response") } - if handler.logins != 2 { + if handler.logins.Load() != 2 { t.Fatal("invalid handler.logins") } - if handler.registers != 2 { + if handler.registers.Load() != 2 { t.Fatal("invalid handler.registers") } } func TestTorTargetsClockIsOffThenSuccess(t *testing.T) { ff := &fakeFill{} - handler := &LoginHandler{t: t} + handler := &LoginHandler{ + logins: &atomicx.Int64{}, + registers: &atomicx.Int64{}, + t: t, + } srvr := httptest.NewServer(handler) defer srvr.Close() registerAPI := &simpleRegisterAPI{ @@ -1197,7 +1257,7 @@ func TestTorTargetsClockIsOffThenSuccess(t *testing.T) { API: baseAPI, RegisterAPI: registerAPI, LoginAPI: loginAPI, - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, } var req *apimodel.TorTargetsRequest ff.fill(&req) @@ -1212,10 +1272,10 @@ func TestTorTargetsClockIsOffThenSuccess(t *testing.T) { if resp == nil { t.Fatal("expected non-nil response") } - if handler.logins != 1 { + if handler.logins.Load() != 1 { t.Fatal("invalid handler.logins") } - if handler.registers != 1 { + if handler.registers.Load() != 1 { t.Fatal("invalid handler.registers") } } @@ -1230,17 +1290,21 @@ func TestTorTargetsClockIsOffThenSuccess(t *testing.T) { if resp == nil { t.Fatal("expected non-nil response") } - if handler.logins != 2 { + if handler.logins.Load() != 2 { t.Fatal("invalid handler.logins") } - if handler.registers != 1 { + if handler.registers.Load() != 1 { t.Fatal("invalid handler.registers") } } func TestTorTargetsClockIsOffThen401(t *testing.T) { ff := &fakeFill{} - handler := &LoginHandler{t: t} + handler := &LoginHandler{ + logins: &atomicx.Int64{}, + registers: &atomicx.Int64{}, + t: t, + } srvr := httptest.NewServer(handler) defer srvr.Close() registerAPI := &simpleRegisterAPI{ @@ -1259,7 +1323,7 @@ func TestTorTargetsClockIsOffThen401(t *testing.T) { API: baseAPI, RegisterAPI: registerAPI, LoginAPI: loginAPI, - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, } var req *apimodel.TorTargetsRequest ff.fill(&req) @@ -1274,10 +1338,10 @@ func TestTorTargetsClockIsOffThen401(t *testing.T) { if resp == nil { t.Fatal("expected non-nil response") } - if handler.logins != 1 { + if handler.logins.Load() != 1 { t.Fatal("invalid handler.logins") } - if handler.registers != 1 { + if handler.registers.Load() != 1 { t.Fatal("invalid handler.registers") } } @@ -1293,17 +1357,21 @@ func TestTorTargetsClockIsOffThen401(t *testing.T) { if resp == nil { t.Fatal("expected non-nil response") } - if handler.logins != 3 { + if handler.logins.Load() != 3 { t.Fatal("invalid handler.logins") } - if handler.registers != 2 { + if handler.registers.Load() != 2 { t.Fatal("invalid handler.registers") } } func TestTorTargetsClockIsOffThen500(t *testing.T) { ff := &fakeFill{} - handler := &LoginHandler{t: t} + handler := &LoginHandler{ + logins: &atomicx.Int64{}, + registers: &atomicx.Int64{}, + t: t, + } srvr := httptest.NewServer(handler) defer srvr.Close() registerAPI := &simpleRegisterAPI{ @@ -1322,7 +1390,7 @@ func TestTorTargetsClockIsOffThen500(t *testing.T) { API: baseAPI, RegisterAPI: registerAPI, LoginAPI: loginAPI, - KVStore: &MemKVStore{}, + KVStore: &kvstore.Memory{}, } var req *apimodel.TorTargetsRequest ff.fill(&req) @@ -1337,10 +1405,10 @@ func TestTorTargetsClockIsOffThen500(t *testing.T) { if resp == nil { t.Fatal("expected non-nil response") } - if handler.logins != 1 { + if handler.logins.Load() != 1 { t.Fatal("invalid handler.logins") } - if handler.registers != 1 { + if handler.registers.Load() != 1 { t.Fatal("invalid handler.registers") } } @@ -1356,10 +1424,10 @@ func TestTorTargetsClockIsOffThen500(t *testing.T) { if resp != nil { t.Fatal("expected nil response") } - if handler.logins != 2 { + if handler.logins.Load() != 2 { t.Fatal("invalid handler.logins") } - if handler.registers != 1 { + if handler.registers.Load() != 1 { t.Fatal("invalid handler.registers") } } diff --git a/internal/engine/ooapi/loginhandler_test.go b/internal/ooapi/loginhandler_test.go similarity index 94% rename from internal/engine/ooapi/loginhandler_test.go rename to internal/ooapi/loginhandler_test.go index f9e0274..3ceac37 100644 --- a/internal/engine/ooapi/loginhandler_test.go +++ b/internal/ooapi/loginhandler_test.go @@ -6,11 +6,11 @@ import ( "net/http" "strings" "sync" - "sync/atomic" "testing" "time" - "github.com/ooni/probe-cli/v3/internal/engine/ooapi/apimodel" + "github.com/ooni/probe-cli/v3/internal/atomicx" + "github.com/ooni/probe-cli/v3/internal/ooapi/apimodel" ) // LoginHandler is an http.Handler to test login @@ -20,8 +20,8 @@ type LoginHandler struct { noRegister bool state []*loginState t *testing.T - logins int32 - registers int32 + logins *atomicx.Int64 + registers *atomicx.Int64 } func (lh *LoginHandler) forgetLogins() { @@ -49,10 +49,14 @@ func (lh *LoginHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { // for simplicity since it's already tested. switch r.URL.Path { case "/api/v1/register": - atomic.AddInt32(&lh.registers, 1) + if lh.registers != nil { + lh.registers.Add(1) + } lh.register(w, r) case "/api/v1/login": - atomic.AddInt32(&lh.logins, 1) + if lh.logins != nil { + lh.logins.Add(1) + } lh.login(w, r) case "/api/v1/test-list/psiphon-config": lh.psiphon(w, r) diff --git a/internal/engine/ooapi/loginmodel.go b/internal/ooapi/loginmodel.go similarity index 93% rename from internal/engine/ooapi/loginmodel.go rename to internal/ooapi/loginmodel.go index 4337348..5d86a5c 100644 --- a/internal/engine/ooapi/loginmodel.go +++ b/internal/ooapi/loginmodel.go @@ -5,8 +5,8 @@ import ( "encoding/base64" "time" - "github.com/ooni/probe-cli/v3/internal/engine/ooapi/apimodel" - "github.com/ooni/probe-cli/v3/internal/engine/runtimex" + "github.com/ooni/probe-cli/v3/internal/ooapi/apimodel" + "github.com/ooni/probe-cli/v3/internal/runtimex" ) // loginState is the struct saved in the kvstore diff --git a/internal/engine/ooapi/requests.go b/internal/ooapi/requests.go similarity index 98% rename from internal/engine/ooapi/requests.go rename to internal/ooapi/requests.go index ab0c0af..7f23cdb 100644 --- a/internal/engine/ooapi/requests.go +++ b/internal/ooapi/requests.go @@ -11,7 +11,7 @@ import ( "net/http" "net/url" - "github.com/ooni/probe-cli/v3/internal/engine/ooapi/apimodel" + "github.com/ooni/probe-cli/v3/internal/ooapi/apimodel" ) func (api *simpleCheckReportIDAPI) newRequest(ctx context.Context, req *apimodel.CheckReportIDRequest) (*http.Request, error) { diff --git a/internal/engine/ooapi/responses.go b/internal/ooapi/responses.go similarity index 99% rename from internal/engine/ooapi/responses.go rename to internal/ooapi/responses.go index f843040..da6078c 100644 --- a/internal/engine/ooapi/responses.go +++ b/internal/ooapi/responses.go @@ -10,7 +10,7 @@ import ( "io/ioutil" "net/http" - "github.com/ooni/probe-cli/v3/internal/engine/ooapi/apimodel" + "github.com/ooni/probe-cli/v3/internal/ooapi/apimodel" ) func (api *simpleCheckReportIDAPI) newResponse(resp *http.Response, err error) (*apimodel.CheckReportIDResponse, error) { diff --git a/internal/engine/ooapi/swagger_test.go b/internal/ooapi/swagger_test.go similarity index 100% rename from internal/engine/ooapi/swagger_test.go rename to internal/ooapi/swagger_test.go diff --git a/internal/engine/ooapi/swaggerdiff_test.go b/internal/ooapi/swaggerdiff_test.go similarity index 98% rename from internal/engine/ooapi/swaggerdiff_test.go rename to internal/ooapi/swaggerdiff_test.go index 6715bc9..d805223 100644 --- a/internal/engine/ooapi/swaggerdiff_test.go +++ b/internal/ooapi/swaggerdiff_test.go @@ -13,7 +13,7 @@ import ( "github.com/hexops/gotextdiff" "github.com/hexops/gotextdiff/myers" "github.com/hexops/gotextdiff/span" - "github.com/ooni/probe-cli/v3/internal/engine/ooapi/internal/openapi" + "github.com/ooni/probe-cli/v3/internal/ooapi/internal/openapi" ) const ( diff --git a/internal/engine/ooapi/utils.go b/internal/ooapi/utils.go similarity index 100% rename from internal/engine/ooapi/utils.go rename to internal/ooapi/utils.go diff --git a/internal/engine/ooapi/utils_test.go b/internal/ooapi/utils_test.go similarity index 100% rename from internal/engine/ooapi/utils_test.go rename to internal/ooapi/utils_test.go diff --git a/internal/engine/internal/platform/platform.go b/internal/platform/platform.go similarity index 98% rename from internal/engine/internal/platform/platform.go rename to internal/platform/platform.go index 5641331..39d6fe2 100644 --- a/internal/engine/internal/platform/platform.go +++ b/internal/platform/platform.go @@ -7,10 +7,15 @@ import "runtime" // Name returns the platform name. The returned value is one of: // // 1. "android" +// // 2. "ios" +// // 3. "linux" +// // 5. "macos" +// // 4. "windows" +// // 5. "unknown" // // The android, ios, linux, macos, windows, and unknown strings are diff --git a/internal/engine/internal/platform/platform_cgo.go b/internal/platform/platform_cgo.go similarity index 100% rename from internal/engine/internal/platform/platform_cgo.go rename to internal/platform/platform_cgo.go diff --git a/internal/engine/internal/platform/platform_otherwise.go b/internal/platform/platform_otherwise.go similarity index 100% rename from internal/engine/internal/platform/platform_otherwise.go rename to internal/platform/platform_otherwise.go diff --git a/internal/engine/internal/platform/platform_test.go b/internal/platform/platform_test.go similarity index 100% rename from internal/engine/internal/platform/platform_test.go rename to internal/platform/platform_test.go diff --git a/internal/engine/internal/randx/randx.go b/internal/randx/randx.go similarity index 79% rename from internal/engine/internal/randx/randx.go rename to internal/randx/randx.go index 7d240e9..01af8ac 100644 --- a/internal/engine/internal/randx/randx.go +++ b/internal/randx/randx.go @@ -1,4 +1,4 @@ -// Package randx contains math/rand extensions +// Package randx contains math/rand extensions. package randx import ( @@ -7,14 +7,16 @@ import ( "unicode" ) +// These constants are used by lettersWithString. const ( uppercase = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" lowercase = "abcdefghijklmnopqrstuvwxyz" letters = uppercase + lowercase ) +// lettersWithString is a method for generating a random string +// described at https://stackoverflow.com/questions/22892120. func lettersWithString(n int, letterBytes string) string { - // See https://stackoverflow.com/questions/22892120 rnd := rand.New(rand.NewSource(time.Now().UnixNano())) b := make([]byte, n) for i := range b { @@ -23,12 +25,12 @@ func lettersWithString(n int, letterBytes string) string { return string(b) } -// Letters return a string composed of random letters +// Letters return a string composed of random letters. func Letters(n int) string { return lettersWithString(n, letters) } -// LettersUppercase return a string composed of random uppercase letters +// LettersUppercase return a string composed of random uppercase letters. func LettersUppercase(n int) string { return lettersWithString(n, uppercase) } diff --git a/internal/engine/internal/randx/randx_test.go b/internal/randx/randx_test.go similarity index 63% rename from internal/engine/internal/randx/randx_test.go rename to internal/randx/randx_test.go index e8c8417..71f8584 100644 --- a/internal/engine/internal/randx/randx_test.go +++ b/internal/randx/randx_test.go @@ -1,13 +1,9 @@ -package randx_test +package randx -import ( - "testing" - - "github.com/ooni/probe-cli/v3/internal/engine/internal/randx" -) +import "testing" func TestLetters(t *testing.T) { - str := randx.Letters(1024) + str := Letters(1024) for _, chr := range str { if (chr >= 'A' && chr <= 'Z') || (chr >= 'a' && chr <= 'z') { continue @@ -17,7 +13,7 @@ func TestLetters(t *testing.T) { } func TestLettersUppercase(t *testing.T) { - str := randx.LettersUppercase(1024) + str := LettersUppercase(1024) for _, chr := range str { if chr >= 'A' && chr <= 'Z' { continue @@ -27,8 +23,8 @@ func TestLettersUppercase(t *testing.T) { } func TestChangeCapitalization(t *testing.T) { - str := randx.Letters(2048) - if randx.ChangeCapitalization(str) == str { + str := Letters(2048) + if ChangeCapitalization(str) == str { t.Fatal("capitalization not changed") } } diff --git a/internal/engine/runtimex/runtimex.go b/internal/runtimex/runtimex.go similarity index 60% rename from internal/engine/runtimex/runtimex.go rename to internal/runtimex/runtimex.go index 79dc5eb..6d454e4 100644 --- a/internal/engine/runtimex/runtimex.go +++ b/internal/runtimex/runtimex.go @@ -1,10 +1,10 @@ -// Package runtimex contains runtime extensions. This package is inspired to the excellent -// github.com/m-lab/rtx package, except that it's simpler. +// Package runtimex contains runtime extensions. This package is inspired to +// https://pkg.go.dev/github.com/m-lab/go/rtx, except that it's simpler. package runtimex import "fmt" -// PanicOnError panics if err is not nil. +// PanicOnError calls panic() if err is not nil. func PanicOnError(err error, message string) { if err != nil { panic(fmt.Errorf("%s: %w", message, err)) diff --git a/internal/engine/runtimex/runtimex_test.go b/internal/runtimex/runtimex_test.go similarity index 88% rename from internal/engine/runtimex/runtimex_test.go rename to internal/runtimex/runtimex_test.go index 19a29c4..9b129ec 100644 --- a/internal/engine/runtimex/runtimex_test.go +++ b/internal/runtimex/runtimex_test.go @@ -4,7 +4,7 @@ import ( "errors" "testing" - "github.com/ooni/probe-cli/v3/internal/engine/runtimex" + "github.com/ooni/probe-cli/v3/internal/runtimex" ) func TestGood(t *testing.T) { diff --git a/internal/engine/shellx/shellx.go b/internal/shellx/shellx.go similarity index 55% rename from internal/engine/shellx/shellx.go rename to internal/shellx/shellx.go index f95e71f..94e6cd4 100644 --- a/internal/engine/shellx/shellx.go +++ b/internal/shellx/shellx.go @@ -1,4 +1,4 @@ -// Package shellx contains utilities to run external commands. +// Package shellx runs external commands. package shellx import ( @@ -6,24 +6,36 @@ import ( "os" "strings" - "golang.org/x/sys/execabs" - "github.com/apex/log" "github.com/google/shlex" - "github.com/ooni/probe-cli/v3/internal/engine/model" + "golang.org/x/sys/execabs" ) +// runconfig is the configuration for run. type runconfig struct { - args []string + // args contains the command line arguments. + args []string + + // command is the command to execute. + command string + + // loginfof is the logging function. loginfof func(format string, v ...interface{}) - name string - stdout *os.File - stderr *os.File + + // stdout is the standard output. + stdout *os.File + + // stderr is the standard error. + stderr *os.File } +// run is the internal function for running commands. func run(config runconfig) error { - config.loginfof("exec: %s %s", config.name, strings.Join(config.args, " ")) - cmd := execabs.Command(config.name, config.args...) + config.loginfof( + "exec: %s %s", config.command, strings.Join(config.args, " ")) + // implementation note: here we're using execabs because + // of https://blog.golang.org/path-security. + cmd := execabs.Command(config.command, config.args...) cmd.Stdout = config.stdout cmd.Stderr = config.stderr err := cmd.Run() @@ -35,26 +47,29 @@ func run(config runconfig) error { func Run(name string, arg ...string) error { return run(runconfig{ args: arg, + command: name, loginfof: log.Log.Infof, - name: name, stdout: os.Stdout, stderr: os.Stderr, }) } +// quietInfof is an infof function that does nothing. +func quietInfof(format string, v ...interface{}) {} + // RunQuiet is like Run but it does not emit any output. func RunQuiet(name string, arg ...string) error { return run(runconfig{ args: arg, - loginfof: model.DiscardLogger.Infof, - name: name, + command: name, + loginfof: quietInfof, stdout: nil, stderr: nil, }) } // RunCommandline is like Run but its only argument is a command -// line that will be splitted using the google/shlex package +// line that will be splitted using the google/shlex package. func RunCommandline(cmdline string) error { args, err := shlex.Split(cmdline) if err != nil { diff --git a/internal/engine/shellx/shellx_test.go b/internal/shellx/shellx_test.go similarity index 100% rename from internal/engine/shellx/shellx_test.go rename to internal/shellx/shellx_test.go diff --git a/pkg/oonimkall/experiment_test.go b/pkg/oonimkall/experiment_test.go index 3a9a732..8ff2779 100644 --- a/pkg/oonimkall/experiment_test.go +++ b/pkg/oonimkall/experiment_test.go @@ -3,8 +3,8 @@ package oonimkall import ( "context" "sync" - "sync/atomic" + "github.com/ooni/probe-cli/v3/internal/atomicx" "github.com/ooni/probe-cli/v3/internal/engine/model" ) @@ -17,16 +17,18 @@ func (cb *FakeExperimentCallbacks) OnProgress(percentage float64, message string // FakeExperimentSession is a fake experimentSession type FakeExperimentSession struct { ExperimentBuilder experimentBuilder - LockCount int32 + LockCount *atomicx.Int64 LookupBackendsErr error LookupLocationErr error NewExperimentBuilderErr error - UnlockCount int32 + UnlockCount *atomicx.Int64 } // lock implements experimentSession.lock func (sess *FakeExperimentSession) lock() { - atomic.AddInt32(&sess.LockCount, 1) + if sess.LockCount != nil { + sess.LockCount.Add(1) + } } // maybeLookupBackends implements experimentSession.maybeLookupBackends @@ -46,7 +48,9 @@ func (sess *FakeExperimentSession) newExperimentBuilder(name string) (experiment // unlock implements experimentSession.unlock func (sess *FakeExperimentSession) unlock() { - atomic.AddInt32(&sess.UnlockCount, 1) + if sess.UnlockCount != nil { + sess.UnlockCount.Add(1) + } } // FakeExperimentBuilder is a fake experimentBuilder diff --git a/pkg/oonimkall/internal/tasks/runner.go b/pkg/oonimkall/internal/tasks/runner.go index ebff330..d98a02b 100644 --- a/pkg/oonimkall/internal/tasks/runner.go +++ b/pkg/oonimkall/internal/tasks/runner.go @@ -8,9 +8,10 @@ import ( "net/url" "time" - engine "github.com/ooni/probe-cli/v3/internal/engine" + "github.com/ooni/probe-cli/v3/internal/engine" "github.com/ooni/probe-cli/v3/internal/engine/model" - "github.com/ooni/probe-cli/v3/internal/engine/runtimex" + "github.com/ooni/probe-cli/v3/internal/kvstore" + "github.com/ooni/probe-cli/v3/internal/runtimex" ) const ( @@ -71,7 +72,7 @@ func (r *Runner) hasUnsupportedSettings(logger *ChanLogger) bool { } func (r *Runner) newsession(ctx context.Context, logger *ChanLogger) (*engine.Session, error) { - kvstore, err := engine.NewFileSystemKVStore(r.settings.StateDir) + kvstore, err := kvstore.NewFS(r.settings.StateDir) if err != nil { return nil, err } diff --git a/pkg/oonimkall/session.go b/pkg/oonimkall/session.go index 059abae..da63b02 100644 --- a/pkg/oonimkall/session.go +++ b/pkg/oonimkall/session.go @@ -8,12 +8,13 @@ import ( "runtime" "sync" + "github.com/ooni/probe-cli/v3/internal/atomicx" "github.com/ooni/probe-cli/v3/internal/engine" - "github.com/ooni/probe-cli/v3/internal/engine/atomicx" "github.com/ooni/probe-cli/v3/internal/engine/legacy/assetsdir" "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/probeservices" - "github.com/ooni/probe-cli/v3/internal/engine/runtimex" + "github.com/ooni/probe-cli/v3/internal/kvstore" + "github.com/ooni/probe-cli/v3/internal/runtimex" ) // AtomicInt64 allows us to export atomicx.Int64 variables to @@ -25,8 +26,8 @@ type AtomicInt64 struct { // These two variables contain metrics pertaining to the number // of Sessions and Contexts that are currently being used. var ( - ActiveSessions = &AtomicInt64{atomicx.NewInt64()} - ActiveContexts = &AtomicInt64{atomicx.NewInt64()} + ActiveSessions = &AtomicInt64{&atomicx.Int64{}} + ActiveContexts = &AtomicInt64{&atomicx.Int64{}} ) // Logger is the logger used by a Session. You should implement a class @@ -147,7 +148,7 @@ func NewSessionWithContext(ctx *Context, config *SessionConfig) (*Session, error // newSessionWithContext implements NewSessionWithContext. func newSessionWithContext(ctx context.Context, config *SessionConfig) (*Session, error) { - kvstore, err := engine.NewFileSystemKVStore(config.StateDir) + kvstore, err := kvstore.NewFS(config.StateDir) if err != nil { return nil, err } diff --git a/pkg/oonimkall/sessioncontext_test.go b/pkg/oonimkall/sessioncontext_test.go index 0d3a96b..6945b8d 100644 --- a/pkg/oonimkall/sessioncontext_test.go +++ b/pkg/oonimkall/sessioncontext_test.go @@ -4,7 +4,7 @@ import ( "testing" "time" - "github.com/ooni/probe-cli/v3/internal/engine/atomicx" + "github.com/ooni/probe-cli/v3/internal/atomicx" ) func TestClampTimeout(t *testing.T) { @@ -26,7 +26,7 @@ func TestClampTimeout(t *testing.T) { } func TestNewContextWithZeroTimeout(t *testing.T) { - here := atomicx.NewInt64() + here := &atomicx.Int64{} ctx, cancel := newContext(0) defer cancel() go func() { @@ -41,7 +41,7 @@ func TestNewContextWithZeroTimeout(t *testing.T) { } func TestNewContextWithNegativeTimeout(t *testing.T) { - here := atomicx.NewInt64() + here := &atomicx.Int64{} ctx, cancel := newContext(-1) defer cancel() go func() { @@ -56,7 +56,7 @@ func TestNewContextWithNegativeTimeout(t *testing.T) { } func TestNewContextWithHugeTimeout(t *testing.T) { - here := atomicx.NewInt64() + here := &atomicx.Int64{} ctx, cancel := newContext(maxTimeout + 1) defer cancel() go func() { @@ -71,7 +71,7 @@ func TestNewContextWithHugeTimeout(t *testing.T) { } func TestNewContextWithReasonableTimeout(t *testing.T) { - here := atomicx.NewInt64() + here := &atomicx.Int64{} ctx, cancel := newContext(1) defer cancel() go func() { @@ -86,7 +86,7 @@ func TestNewContextWithReasonableTimeout(t *testing.T) { } func TestNewContextWithArtificiallyLowMaxTimeout(t *testing.T) { - here := atomicx.NewInt64() + here := &atomicx.Int64{} const maxTimeout = 2 ctx, cancel := newContextEx(maxTimeout+1, maxTimeout) defer cancel() diff --git a/pkg/oonimkall/task.go b/pkg/oonimkall/task.go index 1ad82db..07c3c9e 100644 --- a/pkg/oonimkall/task.go +++ b/pkg/oonimkall/task.go @@ -43,8 +43,8 @@ import ( "context" "encoding/json" - "github.com/ooni/probe-cli/v3/internal/engine/atomicx" - "github.com/ooni/probe-cli/v3/internal/engine/runtimex" + "github.com/ooni/probe-cli/v3/internal/atomicx" + "github.com/ooni/probe-cli/v3/internal/runtimex" "github.com/ooni/probe-cli/v3/pkg/oonimkall/internal/tasks" ) @@ -77,8 +77,8 @@ func StartTask(input string) (*Task, error) { ctx, cancel := context.WithCancel(context.Background()) task := &Task{ cancel: cancel, - isdone: atomicx.NewInt64(), - isstopped: atomicx.NewInt64(), + isdone: &atomicx.Int64{}, + isstopped: &atomicx.Int64{}, out: make(chan *tasks.Event, bufsiz), } go func() { diff --git a/pkg/oonimkall/webconnectivity.go b/pkg/oonimkall/webconnectivity.go index 4a393e2..e96df38 100644 --- a/pkg/oonimkall/webconnectivity.go +++ b/pkg/oonimkall/webconnectivity.go @@ -4,7 +4,7 @@ import ( "context" "encoding/json" - "github.com/ooni/probe-cli/v3/internal/engine/runtimex" + "github.com/ooni/probe-cli/v3/internal/runtimex" ) // WebConnectivityConfig contains settings for WebConnectivity. diff --git a/pkg/oonimkall/webconnectivity_test.go b/pkg/oonimkall/webconnectivity_test.go index 7326b13..795e3cc 100644 --- a/pkg/oonimkall/webconnectivity_test.go +++ b/pkg/oonimkall/webconnectivity_test.go @@ -7,12 +7,17 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/ooni/probe-cli/v3/internal/atomicx" "github.com/ooni/probe-cli/v3/internal/engine/model" ) func TestWebConnectivityRunnerWithMaybeLookupBackendsFailure(t *testing.T) { errMocked := errors.New("mocked error") - sess := &FakeExperimentSession{LookupBackendsErr: errMocked} + sess := &FakeExperimentSession{ + LockCount: &atomicx.Int64{}, + LookupBackendsErr: errMocked, + UnlockCount: &atomicx.Int64{}, + } runner := &webConnectivityRunner{sess: sess} ctx := context.Background() config := &WebConnectivityConfig{Input: "https://ooni.org"} @@ -23,14 +28,18 @@ func TestWebConnectivityRunnerWithMaybeLookupBackendsFailure(t *testing.T) { if out != nil { t.Fatal("expected nil here") } - if sess.LockCount != 1 || sess.UnlockCount != 1 { + if sess.LockCount.Load() != 1 || sess.UnlockCount.Load() != 1 { t.Fatal("invalid locking pattern") } } func TestWebConnectivityRunnerWithMaybeLookupLocationFailure(t *testing.T) { errMocked := errors.New("mocked error") - sess := &FakeExperimentSession{LookupLocationErr: errMocked} + sess := &FakeExperimentSession{ + LockCount: &atomicx.Int64{}, + LookupLocationErr: errMocked, + UnlockCount: &atomicx.Int64{}, + } runner := &webConnectivityRunner{sess: sess} ctx := context.Background() config := &WebConnectivityConfig{Input: "https://ooni.org"} @@ -41,14 +50,18 @@ func TestWebConnectivityRunnerWithMaybeLookupLocationFailure(t *testing.T) { if out != nil { t.Fatal("expected nil here") } - if sess.LockCount != 1 || sess.UnlockCount != 1 { + if sess.LockCount.Load() != 1 || sess.UnlockCount.Load() != 1 { t.Fatal("invalid locking pattern") } } func TestWebConnectivityRunnerWithNewExperimentBuilderFailure(t *testing.T) { errMocked := errors.New("mocked error") - sess := &FakeExperimentSession{NewExperimentBuilderErr: errMocked} + sess := &FakeExperimentSession{ + LockCount: &atomicx.Int64{}, + NewExperimentBuilderErr: errMocked, + UnlockCount: &atomicx.Int64{}, + } runner := &webConnectivityRunner{sess: sess} ctx := context.Background() config := &WebConnectivityConfig{Input: "https://ooni.org"} @@ -59,7 +72,7 @@ func TestWebConnectivityRunnerWithNewExperimentBuilderFailure(t *testing.T) { if out != nil { t.Fatal("expected nil here") } - if sess.LockCount != 1 || sess.UnlockCount != 1 { + if sess.LockCount.Load() != 1 || sess.UnlockCount.Load() != 1 { t.Fatal("invalid locking pattern") } } @@ -69,7 +82,11 @@ func TestWebConnectivityRunnerWithMeasureFailure(t *testing.T) { cbs := &FakeExperimentCallbacks{} e := &FakeExperiment{Err: errMocked} eb := &FakeExperimentBuilder{Experiment: e} - sess := &FakeExperimentSession{ExperimentBuilder: eb} + sess := &FakeExperimentSession{ + LockCount: &atomicx.Int64{}, + ExperimentBuilder: eb, + UnlockCount: &atomicx.Int64{}, + } runner := &webConnectivityRunner{sess: sess} ctx := context.Background() config := &WebConnectivityConfig{ @@ -83,7 +100,7 @@ func TestWebConnectivityRunnerWithMeasureFailure(t *testing.T) { if out != nil { t.Fatal("expected nil here") } - if sess.LockCount != 1 || sess.UnlockCount != 1 { + if sess.LockCount.Load() != 1 || sess.UnlockCount.Load() != 1 { t.Fatal("invalid locking pattern") } if eb.Callbacks != cbs { @@ -99,7 +116,11 @@ func TestWebConnectivityRunnerWithNoError(t *testing.T) { cbs := &FakeExperimentCallbacks{} e := &FakeExperiment{Measurement: m, Sent: 10, Received: 128} eb := &FakeExperimentBuilder{Experiment: e} - sess := &FakeExperimentSession{ExperimentBuilder: eb} + sess := &FakeExperimentSession{ + LockCount: &atomicx.Int64{}, + ExperimentBuilder: eb, + UnlockCount: &atomicx.Int64{}, + } runner := &webConnectivityRunner{sess: sess} ctx := context.Background() config := &WebConnectivityConfig{ @@ -113,7 +134,7 @@ func TestWebConnectivityRunnerWithNoError(t *testing.T) { if out == nil { t.Fatal("expected non-nil here") } - if sess.LockCount != 1 || sess.UnlockCount != 1 { + if sess.LockCount.Load() != 1 || sess.UnlockCount.Load() != 1 { t.Fatal("invalid locking pattern") } if eb.Callbacks != cbs {