From 9e238c27ddbe3955711ea1171807f9f43c194e85 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Fri, 13 Nov 2020 19:01:06 +0100 Subject: [PATCH] refactor(internal/ooni): Context => Probe (#170) Closes https://github.com/ooni/probe/issues/939 --- internal/cli/onboard/onboard.go | 22 ++++----- internal/cli/root/root.go | 12 ++--- internal/cli/run/run.go | 22 ++++----- internal/nettests/nettests.go | 24 ++++----- internal/nettests/nettests_test.go | 20 ++++---- internal/nettests/web_connectivity.go | 6 +-- internal/ooni/ooni.go | 70 +++++++++++++-------------- internal/ooni/ooni_test.go | 4 +- 8 files changed, 90 insertions(+), 90 deletions(-) diff --git a/internal/cli/onboard/onboard.go b/internal/cli/onboard/onboard.go index 1b8274c..dba3591 100644 --- a/internal/cli/onboard/onboard.go +++ b/internal/cli/onboard/onboard.go @@ -137,12 +137,12 @@ func Onboarding(config *config.Config) error { // MaybeOnboarding will run the onboarding process only if the informed consent // config option is set to false -func MaybeOnboarding(c *ooni.Context) error { - if c.Config.InformedConsent == false { - if c.IsBatch == true { +func MaybeOnboarding(probe *ooni.Probe) error { + if probe.Config.InformedConsent == false { + if probe.IsBatch == true { return errors.New("cannot run onboarding in batch mode") } - if err := Onboarding(c.Config); err != nil { + if err := Onboarding(probe.Config); err != nil { return errors.Wrap(err, "onboarding") } } @@ -155,26 +155,26 @@ func init() { yes := cmd.Flag("yes", "Answer yes to all the onboarding questions.").Bool() cmd.Action(func(_ *kingpin.ParseContext) error { - ctx, err := root.Init() + probe, err := root.Init() if err != nil { return err } if *yes == true { - ctx.Config.Lock() - ctx.Config.InformedConsent = true - ctx.Config.Unlock() + probe.Config.Lock() + probe.Config.InformedConsent = true + probe.Config.Unlock() - if err := ctx.Config.Write(); err != nil { + if err := probe.Config.Write(); err != nil { log.WithError(err).Error("failed to write config file") return err } return nil } - if ctx.IsBatch == true { + if probe.IsBatch == true { return errors.New("cannot do onboarding in batch mode") } - return Onboarding(ctx.Config) + return Onboarding(probe.Config) }) } diff --git a/internal/cli/root/root.go b/internal/cli/root/root.go index dca4c99..d6e059e 100644 --- a/internal/cli/root/root.go +++ b/internal/cli/root/root.go @@ -17,7 +17,7 @@ var Cmd = kingpin.New("ooniprobe", "") var Command = Cmd.Command // Init should be called by all subcommand that care to have a ooni.Context instance -var Init func() (*ooni.Context, error) +var Init func() (*ooni.Probe, error) func init() { configPath := Cmd.Flag("config", "Set a custom config file path").Short('c').String() @@ -43,7 +43,7 @@ func init() { log.Debugf("ooni version %s", version.Version) } - Init = func() (*ooni.Context, error) { + Init = func() (*ooni.Probe, error) { var err error homePath, err := utils.GetOONIHome() @@ -51,16 +51,16 @@ func init() { return nil, err } - ctx := ooni.NewContext(*configPath, homePath) - err = ctx.Init(*softwareName, *softwareVersion) + probe := ooni.NewProbe(*configPath, homePath) + err = probe.Init(*softwareName, *softwareVersion) if err != nil { return nil, err } if *isBatch { - ctx.IsBatch = true + probe.IsBatch = true } - return ctx, nil + return probe, nil } return nil diff --git a/internal/cli/run/run.go b/internal/cli/run/run.go index 621a867..95c04a5 100644 --- a/internal/cli/run/run.go +++ b/internal/cli/run/run.go @@ -13,7 +13,7 @@ import ( "github.com/ooni/probe-cli/internal/ooni" ) -func runNettestGroup(tg string, ctx *ooni.Context, network *database.Network) error { +func runNettestGroup(tg string, ctx *ooni.Probe, network *database.Network) error { if ctx.IsTerminated() == true { log.Debugf("context is terminated, stopping runNettestGroup early") return nil @@ -79,7 +79,7 @@ func init() { cmd := root.Command("run", "Run a test group or OONI Run link") var nettestGroupNamesBlue []string - var ctx *ooni.Context + var probe *ooni.Probe var network *database.Network for name := range nettests.NettestGroups { @@ -90,48 +90,48 @@ func init() { cmd.Action(func(_ *kingpin.ParseContext) error { var err error - ctx, err = root.Init() + probe, err = root.Init() if err != nil { log.Errorf("%s", err) return err } - if err = onboard.MaybeOnboarding(ctx); err != nil { + if err = onboard.MaybeOnboarding(probe); err != nil { log.WithError(err).Error("failed to perform onboarding") return err } if *noCollector == true { - ctx.Config.Sharing.UploadResults = false + probe.Config.Sharing.UploadResults = false } return nil }) websitesCmd := cmd.Command("websites", "") websitesCmd.Action(func(_ *kingpin.ParseContext) error { - return runNettestGroup("websites", ctx, network) + return runNettestGroup("websites", probe, network) }) imCmd := cmd.Command("im", "") imCmd.Action(func(_ *kingpin.ParseContext) error { - return runNettestGroup("im", ctx, network) + return runNettestGroup("im", probe, network) }) performanceCmd := cmd.Command("performance", "") performanceCmd.Action(func(_ *kingpin.ParseContext) error { - return runNettestGroup("performance", ctx, network) + return runNettestGroup("performance", probe, network) }) middleboxCmd := cmd.Command("middlebox", "") middleboxCmd.Action(func(_ *kingpin.ParseContext) error { - return runNettestGroup("middlebox", ctx, network) + return runNettestGroup("middlebox", probe, network) }) circumventionCmd := cmd.Command("circumvention", "") circumventionCmd.Action(func(_ *kingpin.ParseContext) error { - return runNettestGroup("circumvention", ctx, network) + return runNettestGroup("circumvention", probe, network) }) allCmd := cmd.Command("all", "").Default() allCmd.Action(func(_ *kingpin.ParseContext) error { log.Infof("Running %s tests", color.BlueString("all")) for tg := range nettests.NettestGroups { - if err := runNettestGroup(tg, ctx, network); err != nil { + if err := runNettestGroup(tg, probe, network); err != nil { log.WithError(err).Errorf("failed to run %s", tg) } } diff --git a/internal/nettests/nettests.go b/internal/nettests/nettests.go index 1c9159c..cfadf55 100644 --- a/internal/nettests/nettests.go +++ b/internal/nettests/nettests.go @@ -24,9 +24,9 @@ type Nettest interface { // NewController creates a nettest controller func NewController( - nt Nettest, ctx *ooni.Context, res *database.Result, sess *engine.Session) *Controller { + nt Nettest, probe *ooni.Probe, res *database.Result, sess *engine.Session) *Controller { return &Controller{ - Ctx: ctx, + Probe: probe, nt: nt, res: res, Session: sess, @@ -36,7 +36,7 @@ func NewController( // Controller is passed to the run method of every Nettest // each nettest instance has one controller type Controller struct { - Ctx *ooni.Context + Probe *ooni.Probe Session *engine.Session res *database.Result nt Nettest @@ -93,7 +93,7 @@ func (c *Controller) Run(builder *engine.ExperimentBuilder, inputs []string) err log.Debug(color.RedString("status.queued")) log.Debug(color.RedString("status.started")) - if c.Ctx.Config.Sharing.UploadResults { + if c.Probe.Config.Sharing.UploadResults { if err := exp.OpenReport(); err != nil { log.Debugf( "%s: %s", color.RedString("failure.report_create"), err.Error(), @@ -107,7 +107,7 @@ func (c *Controller) Run(builder *engine.ExperimentBuilder, inputs []string) err c.ntStartTime = time.Now() for idx, input := range inputs { - if c.Ctx.IsTerminated() == true { + if c.Probe.IsTerminated() == true { log.Debug("isTerminated == true, breaking the input loop") break } @@ -120,7 +120,7 @@ func (c *Controller) Run(builder *engine.ExperimentBuilder, inputs []string) err } msmt, err := database.CreateMeasurement( - c.Ctx.DB, reportID, exp.Name(), c.res.MeasurementDir, idx, resultID, urlID, + c.Probe.DB, reportID, exp.Name(), c.res.MeasurementDir, idx, resultID, urlID, ) if err != nil { return errors.Wrap(err, "failed to create measurement") @@ -133,7 +133,7 @@ func (c *Controller) Run(builder *engine.ExperimentBuilder, inputs []string) err measurement, err := exp.Measure(input) if err != nil { log.WithError(err).Debug(color.RedString("failure.measurement")) - if err := c.msmts[idx64].Failed(c.Ctx.DB, err.Error()); err != nil { + if err := c.msmts[idx64].Failed(c.Probe.DB, err.Error()); err != nil { return errors.Wrap(err, "failed to mark measurement as failed") } // Even with a failed measurement, we want to continue. We want to @@ -142,16 +142,16 @@ func (c *Controller) Run(builder *engine.ExperimentBuilder, inputs []string) err // undertsand what went wrong (censorship? bug? anomaly?). } - if c.Ctx.Config.Sharing.UploadResults { + if c.Probe.Config.Sharing.UploadResults { // Implementation note: SubmitMeasurement will fail here if we did fail // to open the report but we still want to continue. There will be a // bit of a spew in the logs, perhaps, but stopping seems less efficient. if err := exp.SubmitAndUpdateMeasurement(measurement); err != nil { log.Debug(color.RedString("failure.measurement_submission")) - if err := c.msmts[idx64].UploadFailed(c.Ctx.DB, err.Error()); err != nil { + if err := c.msmts[idx64].UploadFailed(c.Probe.DB, err.Error()); err != nil { return errors.Wrap(err, "failed to mark upload as failed") } - } else if err := c.msmts[idx64].UploadSucceeded(c.Ctx.DB); err != nil { + } else if err := c.msmts[idx64].UploadSucceeded(c.Probe.DB); err != nil { return errors.Wrap(err, "failed to mark upload as succeeded") } } @@ -159,7 +159,7 @@ func (c *Controller) Run(builder *engine.ExperimentBuilder, inputs []string) err if err := exp.SaveMeasurement(measurement, msmt.MeasurementFilePath.String); err != nil { return errors.Wrap(err, "failed to save measurement on disk") } - if err := c.msmts[idx64].Done(c.Ctx.DB); err != nil { + if err := c.msmts[idx64].Done(c.Probe.DB); err != nil { return errors.Wrap(err, "failed to mark measurement as done") } @@ -179,7 +179,7 @@ func (c *Controller) Run(builder *engine.ExperimentBuilder, inputs []string) err continue } log.Debugf("Fetching: %d %v", idx, c.msmts[idx64]) - if err := database.AddTestKeys(c.Ctx.DB, c.msmts[idx64], tk); err != nil { + if err := database.AddTestKeys(c.Probe.DB, c.msmts[idx64], tk); err != nil { return errors.Wrap(err, "failed to add test keys to summary") } } diff --git a/internal/nettests/nettests_test.go b/internal/nettests/nettests_test.go index fb13d5d..7710a15 100644 --- a/internal/nettests/nettests_test.go +++ b/internal/nettests/nettests_test.go @@ -10,7 +10,7 @@ import ( "github.com/ooni/probe-cli/internal/utils/shutil" ) -func newTestingContext(t *testing.T) *ooni.Context { +func newOONIProbe(t *testing.T) *ooni.Probe { homePath, err := ioutil.TempDir("", "ooniprobetests") if err != nil { t.Fatal(err) @@ -18,35 +18,35 @@ func newTestingContext(t *testing.T) *ooni.Context { configPath := path.Join(homePath, "config.json") testingConfig := path.Join("..", "..", "testdata", "testing-config.json") shutil.Copy(testingConfig, configPath, false) - ctx := ooni.NewContext(configPath, homePath) + probe := ooni.NewProbe(configPath, homePath) swName := "ooniprobe-cli-tests" swVersion := "3.0.0-alpha" - err = ctx.Init(swName, swVersion) + err = probe.Init(swName, swVersion) if err != nil { t.Fatal(err) } - return ctx + return probe } func TestCreateContext(t *testing.T) { - newTestingContext(t) + newOONIProbe(t) } func TestRun(t *testing.T) { - ctx := newTestingContext(t) - sess, err := ctx.NewSession() + probe := newOONIProbe(t) + sess, err := probe.NewSession() if err != nil { t.Fatal(err) } - network, err := database.CreateNetwork(ctx.DB, sess) + network, err := database.CreateNetwork(probe.DB, sess) if err != nil { t.Fatal(err) } - res, err := database.CreateResult(ctx.DB, ctx.Home, "middlebox", network.ID) + res, err := database.CreateResult(probe.DB, probe.Home, "middlebox", network.ID) if err != nil { t.Fatal(err) } nt := HTTPInvalidRequestLine{} - ctl := NewController(nt, ctx, res, sess) + ctl := NewController(nt, probe, res, sess) nt.Run(ctl) } diff --git a/internal/nettests/web_connectivity.go b/internal/nettests/web_connectivity.go index f16f3c7..7a8e793 100644 --- a/internal/nettests/web_connectivity.go +++ b/internal/nettests/web_connectivity.go @@ -19,7 +19,7 @@ func lookupURLs(ctl *Controller, limit int64, categories []string) ([]string, ma for idx, url := range testlist.Result { log.Debugf("Going over URL %d", idx) urlID, err := database.CreateOrUpdateURL( - ctl.Ctx.DB, url.URL, url.CategoryCode, url.CountryCode, + ctl.Probe.DB, url.URL, url.CategoryCode, url.CountryCode, ) if err != nil { log.Error("failed to add to the URL table") @@ -38,8 +38,8 @@ type WebConnectivity struct { // Run starts the test func (n WebConnectivity) Run(ctl *Controller) error { - log.Debugf("Enabled category codes are the following %v", ctl.Ctx.Config.Nettests.WebsitesEnabledCategoryCodes) - urls, urlIDMap, err := lookupURLs(ctl, ctl.Ctx.Config.Nettests.WebsitesURLLimit, ctl.Ctx.Config.Nettests.WebsitesEnabledCategoryCodes) + log.Debugf("Enabled category codes are the following %v", ctl.Probe.Config.Nettests.WebsitesEnabledCategoryCodes) + urls, urlIDMap, err := lookupURLs(ctl, ctl.Probe.Config.Nettests.WebsitesURLLimit, ctl.Probe.Config.Nettests.WebsitesEnabledCategoryCodes) if err != nil { return err } diff --git a/internal/ooni/ooni.go b/internal/ooni/ooni.go index 466a5f8..550073a 100644 --- a/internal/ooni/ooni.go +++ b/internal/ooni/ooni.go @@ -19,8 +19,8 @@ import ( "upper.io/db.v3/lib/sqlbuilder" ) -// Context for OONI Probe -type Context struct { +// Probe contains the ooniprobe CLI context. +type Probe struct { Config *config.Config DB sqlbuilder.Database IsBatch bool @@ -43,14 +43,14 @@ type Context struct { // 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 (c *Context) IsTerminated() bool { - i := atomic.LoadInt32(&c.isTerminatedAtomicInt) +func (p *Probe) IsTerminated() bool { + i := atomic.LoadInt32(&p.isTerminatedAtomicInt) return i != 0 } // Terminate interrupts the running context -func (c *Context) Terminate() { - atomic.AddInt32(&c.isTerminatedAtomicInt, 1) +func (p *Probe) Terminate() { + atomic.AddInt32(&p.isTerminatedAtomicInt, 1) } // ListenForSignals will listen for SIGINT and SIGTERM. When it receives those @@ -59,13 +59,13 @@ func (c *Context) Terminate() { // // TODO refactor this to use a cancellable context.Context instead of a bool // flag, probably as part of: https://github.com/ooni/probe-cli/issues/45 -func (c *Context) ListenForSignals() { +func (p *Probe) ListenForSignals() { s := make(chan os.Signal, 1) signal.Notify(s, os.Interrupt, syscall.SIGINT, syscall.SIGTERM) go func() { <-s log.Info("caught a stop signal, shutting down cleanly") - c.Terminate() + p.Terminate() }() } @@ -82,12 +82,12 @@ func (c *Context) ListenForSignals() { // // TODO refactor this to use a cancellable context.Context instead of a bool // flag, probably as part of: https://github.com/ooni/probe-cli/issues/45 -func (c *Context) MaybeListenForStdinClosed() { +func (p *Probe) MaybeListenForStdinClosed() { if os.Getenv("OONI_STDIN_EOF_IMPLIES_SIGTERM") != "true" { return } go func() { - defer c.Terminate() + defer p.Terminate() defer log.Info("stdin closed, shutting down cleanly") b := make([]byte, 1<<10) for { @@ -99,74 +99,74 @@ func (c *Context) MaybeListenForStdinClosed() { } // Init the OONI manager -func (c *Context) Init(softwareName, softwareVersion string) error { +func (p *Probe) Init(softwareName, softwareVersion string) error { var err error - if err = MaybeInitializeHome(c.Home); err != nil { + if err = MaybeInitializeHome(p.Home); err != nil { return err } - if c.configPath != "" { - log.Debugf("Reading config file from %s", c.configPath) - c.Config, err = config.ReadConfig(c.configPath) + if p.configPath != "" { + log.Debugf("Reading config file from %s", p.configPath) + p.Config, err = config.ReadConfig(p.configPath) } else { log.Debug("Reading default config file") - c.Config, err = InitDefaultConfig(c.Home) + p.Config, err = InitDefaultConfig(p.Home) } if err != nil { return err } - if err = c.Config.MaybeMigrate(); err != nil { + if err = p.Config.MaybeMigrate(); err != nil { return errors.Wrap(err, "migrating config") } - c.dbPath = utils.DBDir(c.Home, "main") - log.Debugf("Connecting to database sqlite3://%s", c.dbPath) - db, err := database.Connect(c.dbPath) + p.dbPath = utils.DBDir(p.Home, "main") + log.Debugf("Connecting to database sqlite3://%s", p.dbPath) + db, err := database.Connect(p.dbPath) if err != nil { return err } - c.DB = db + p.DB = db tempDir, err := ioutil.TempDir("", "ooni") if err != nil { return errors.Wrap(err, "creating TempDir") } - c.TempDir = tempDir + p.TempDir = tempDir - c.softwareName = softwareName - c.softwareVersion = softwareVersion + p.softwareName = softwareName + p.softwareVersion = softwareVersion return nil } // NewSession creates a new ooni/probe-engine session using the // current configuration inside the context. The caller must close // the session when done using it, by calling sess.Close(). -func (c *Context) NewSession() (*engine.Session, error) { +func (p *Probe) NewSession() (*engine.Session, error) { kvstore, err := engine.NewFileSystemKVStore( - utils.EngineDir(c.Home), + utils.EngineDir(p.Home), ) if err != nil { return nil, errors.Wrap(err, "creating engine's kvstore") } return engine.NewSession(engine.SessionConfig{ - AssetsDir: utils.AssetsDir(c.Home), + AssetsDir: utils.AssetsDir(p.Home), KVStore: kvstore, Logger: enginex.Logger, PrivacySettings: model.PrivacySettings{ - IncludeASN: c.Config.Sharing.IncludeASN, + IncludeASN: p.Config.Sharing.IncludeASN, IncludeCountry: true, - IncludeIP: c.Config.Sharing.IncludeIP, + IncludeIP: p.Config.Sharing.IncludeIP, }, - SoftwareName: c.softwareName, - SoftwareVersion: c.softwareVersion, - TempDir: c.TempDir, + SoftwareName: p.softwareName, + SoftwareVersion: p.softwareVersion, + TempDir: p.TempDir, }) } -// NewContext creates a new context instance. -func NewContext(configPath string, homePath string) *Context { - return &Context{ +// NewProbe creates a new probe instance. +func NewProbe(configPath string, homePath string) *Probe { + return &Probe{ Home: homePath, Config: &config.Config{}, configPath: configPath, diff --git a/internal/ooni/ooni_test.go b/internal/ooni/ooni_test.go index a1f5344..f46a386 100644 --- a/internal/ooni/ooni_test.go +++ b/internal/ooni/ooni_test.go @@ -14,10 +14,10 @@ func TestInit(t *testing.T) { } defer os.RemoveAll(ooniHome) - ctx := NewContext("", ooniHome) + probe := NewProbe("", ooniHome) swName := "ooniprobe-cli-tests" swVersion := "3.0.0-alpha" - if err := ctx.Init(swName, swVersion); err != nil { + if err := probe.Init(swName, swVersion); err != nil { t.Error(err) t.Fatal("failed to init the context") }