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 {