refactor(lib): require a context for NewInstance

While we're playing with the signature of NewInstance, it makes sense to also require a context.
library consumers should always be providing a context to the instance constructor, even if just
context.Background(). Adding this requirement signals to the user that they should first
understand how context works before using lib, and that library authors will need to repsect
the passed in context throughout packages below lib.

api/update_test.go

@@ -1,6 +1,7 @@
 package api
 
 import (
+ "context"
  "io/ioutil"
  "os"
  "testing"
@@ -20,7 +21,7 @@ func TestUpdateHandlers(t *testing.T) {
  cfg.Store.Type = "map"
  cfg.Repo.Type = "mem"
 
- inst, err := lib.NewInstance(tmpDir,
+ inst, err := lib.NewInstance(context.Background(), tmpDir,
  lib.OptConfig(cfg),
  )
  if err != nil {

cmd/qri.go

@@ -109,11 +109,10 @@ func (o *QriOptions) Init() (err error) {
  initBody := func() {
  opts := []lib.Option{
  lib.OptIOStreams(o.IOStreams), // transfer iostreams to instance
- lib.OptCtx(o.ctx), // transfer request context to instance
  lib.OptSetIPFSPath(o.ipfsFsPath),
  lib.OptCheckConfigMigrations(""),
  }
- o.inst, err = lib.NewInstance(o.qriRepoPath, opts...)
+ o.inst, err = lib.NewInstance(o.ctx, o.qriRepoPath, opts...)
  }
  o.initialized.Do(initBody)
  return

cmd/update_test.go

@@ -1,6 +1,7 @@
 package cmd
 
 import (
+ "context"
  "io/ioutil"
  "os"
  "strings"
@@ -78,7 +79,7 @@ func TestUpdateMethods(t *testing.T) {
  cfg.Repo = &config.Repo{Type: "mem", Middleware: []string{}}
  cfg.Store = &config.Store{Type: "map"}
 
- inst, err := lib.NewInstance(tmpDir, lib.OptConfig(cfg), lib.OptIOStreams(streams))
+ inst, err := lib.NewInstance(context.Background(), tmpDir, lib.OptConfig(cfg), lib.OptIOStreams(streams))
  if err != nil {
  t.Fatal(err)
  }

lib/config_test.go

@@ -8,22 +8,6 @@ import (
  "github.com/qri-io/qri/config"
 )
 
-// func TestLoadConfig(t *testing.T) {
-// path, err := ioutil.TempDir("", "config_tests")
-// if err != nil {
-// t.Fatal(err.Error())
-// }
-// defer os.RemoveAll(path)
-// cfgPath := path + "/config.yaml"
-
-// if err := config.DefaultConfigForTesting().WriteToFile(cfgPath); err != nil {
-// t.Fatal(err.Error())
-// }
-// if err := LoadConfig(ioes.NewDiscardIOStreams(), cfgPath); err != nil {
-// t.Error(err.Error())
-// }
-// }
-
 func TestGetConfig(t *testing.T) {
  cfg := config.DefaultConfigForTesting()
  // TODO (b5) - hack until we can get better test-instance allocation

lib/lib.go

@@ -90,22 +90,13 @@ type Methods interface {
 // * Config consists only of static values stored in a configuration file
 // Options may override config in specific cases to avoid undefined state
 type InstanceOptions struct {
- Ctx context.Context
  Cfg *config.Config
  Streams ioes.IOStreams
 }
 
 // Option is a function that manipulates config details when fed to New()
 type Option func(o *InstanceOptions) error
 
-// OptCtx sets the base context to use for this instance
-func OptCtx(ctx context.Context) Option {
- return func(o *InstanceOptions) error {
- o.Ctx = ctx
- return nil
- }
-}
-
 // OptConfig supplies a configuration directly
 func OptConfig(cfg *config.Config) Option {
  return func(o *InstanceOptions) error {
@@ -176,14 +167,12 @@ func OptCheckConfigMigrations(cfgPath string) Option {
 
 // NewInstance creates a new Qri Instance, if no Option funcs are provided,
 // New uses a default set of Option funcs
-func NewInstance(repoPath string, opts ...Option) (qri *Instance, err error) {
+func NewInstance(ctx context.Context, repoPath string, opts ...Option) (qri *Instance, err error) {
  if repoPath == "" {
  return nil, fmt.Errorf("repo path is required")
  }
 
- o := &InstanceOptions{
- Ctx: context.Background(),
- }
+ o := &InstanceOptions{}
 
  // attempt to load a base configuration from repoPath
  if o.Cfg, err = loadRepoConfig(repoPath); err != nil {
@@ -215,7 +204,7 @@ func NewInstance(repoPath string, opts ...Option) (qri *Instance, err error) {
  return
  }
 
- ctx, teardown := context.WithCancel(o.Ctx)
+ ctx, teardown := context.WithCancel(ctx)
  inst := &Instance{
  ctx: ctx,
  teardown: teardown,
@@ -248,7 +237,7 @@ func NewInstance(repoPath string, opts ...Option) (qri *Instance, err error) {
  }
  }
 
- if inst.store, err = newStore(o.Ctx, cfg); err != nil {
+ if inst.store, err = newStore(ctx, cfg); err != nil {
  return
  }
  if inst.qfs, err = newFilesystem(cfg, inst.store); err != nil {
@@ -271,7 +260,7 @@ func NewInstance(repoPath string, opts ...Option) (qri *Instance, err error) {
  return
 }
 
-// TODO (b5): this is a repo layout assertion, move to repo package
+// TODO (b5): this is a repo layout assertion, move to repo package?
 func loadRepoConfig(repoPath string) (*config.Config, error) {
  path := filepath.Join(repoPath, "config.yaml")
 

lib/lib_test.go

@@ -1,6 +1,7 @@
 package lib
 
 import (
+ "context"
  "encoding/base64"
  "fmt"
  "io/ioutil"
@@ -54,7 +55,7 @@ func TestNewInstance(t *testing.T) {
  cfg.Store.Type = "map"
  cfg.Repo.Type = "mem"
 
- got, err := NewInstance(os.TempDir(), OptConfig(cfg))
+ got, err := NewInstance(context.Background(), os.TempDir(), OptConfig(cfg))
  if err != nil {
  t.Error(err)
  return
@@ -68,7 +69,7 @@ func TestNewInstance(t *testing.T) {
  t.Error(err)
  }
 
- if _, err = NewInstance(""); err == nil {
+ if _, err = NewInstance(context.Background(), ""); err == nil {
  t.Error("expected NewInstance to error when provided no repo path")
  }
 }
@@ -100,7 +101,7 @@ func TestNewDefaultInstance(t *testing.T) {
  cfg.Store.Path = tempDir
  cfg.WriteToFile(filepath.Join(tempDir, "config.yaml"))
 
- _, err = NewInstance(tempDir)
+ _, err = NewInstance(context.Background(), tempDir)
  if err != nil {
  t.Fatal(err)
  }

lib/update_test.go

@@ -26,7 +26,7 @@ func TestUpdateMethods(t *testing.T) {
  cfg.Repo = &config.Repo{Type: "mem", Middleware: []string{}}
  cfg.Store = &config.Store{Type: "map"}
 
- inst, err := NewInstance(tmpDir, OptConfig(cfg), OptIOStreams(ioes.NewDiscardIOStreams()))
+ inst, err := NewInstance(context.Background(), tmpDir, OptConfig(cfg), OptIOStreams(ioes.NewDiscardIOStreams()))
  if err != nil {
  t.Fatal(err)
  }