From dd93cd840486d8d31706533e96959a1fa31dd023 Mon Sep 17 00:00:00 2001 From: Kevin Atkinson Date: Mon, 12 Jun 2017 21:43:17 -0400 Subject: [PATCH] address p.r. comments License: MIT Signed-off-by: Kevin Atkinson --- repo/config/init.go | 6 ++--- repo/fsrepo/datastores.go | 54 ++++++++++++++++++++++++++------------ repo/fsrepo/fsrepo_test.go | 10 +++---- 3 files changed, 45 insertions(+), 25 deletions(-) diff --git a/repo/config/init.go b/repo/config/init.go index f48914d50e0..97a12cf7f63 100644 --- a/repo/config/init.go +++ b/repo/config/init.go @@ -37,7 +37,7 @@ func Init(out io.Writer, nBitsForKeypair int) (*Config, error) { Gateway: "/ip4/127.0.0.1/tcp/8080", }, - Datastore: *datastore, + Datastore: datastore, Bootstrap: BootstrapPeerStrings(bootstrapPeers), Identity: identity, Discovery: Discovery{MDNS{ @@ -74,8 +74,8 @@ func Init(out io.Writer, nBitsForKeypair int) (*Config, error) { } // DatastoreConfig is an internal function exported to aid in testing. -func DefaultDatastoreConfig() *Datastore { - return &Datastore{ +func DefaultDatastoreConfig() Datastore { + return Datastore{ StorageMax: "10GB", StorageGCWatermark: 90, // 90% GCPeriod: "1h", diff --git a/repo/fsrepo/datastores.go b/repo/fsrepo/datastores.go index 3b1de881627..09581516cfe 100644 --- a/repo/fsrepo/datastores.go +++ b/repo/fsrepo/datastores.go @@ -1,7 +1,6 @@ package fsrepo import ( - "encoding/json" "fmt" "path/filepath" @@ -22,7 +21,7 @@ func (r *FSRepo) constructDatastore(params map[string]interface{}) (repo.Datasto case "mount": mounts, ok := params["mounts"].([]interface{}) if !ok { - return nil, fmt.Errorf("mounts field wasnt an array") + return nil, fmt.Errorf("'mounts' field is missing or not an array") } return r.openMountDatastore(mounts) @@ -31,19 +30,33 @@ func (r *FSRepo) constructDatastore(params map[string]interface{}) (repo.Datasto case "mem": return ds.NewMapDatastore(), nil case "log": - child, err := r.constructDatastore(params["child"].(map[string]interface{})) + childField, ok := params["child"].(map[string]interface{}) + if !ok { + return nil, fmt.Errorf("'child' field is missing or not a map") + } + child, err := r.constructDatastore(childField) if err != nil { return nil, err } - - return ds.NewLogDatastore(child, params["name"].(string)), nil + nameField, ok := params["name"].(string) + if !ok { + return nil, fmt.Errorf("'name' field was missing or not a string") + } + return ds.NewLogDatastore(child, nameField), nil case "measure": - child, err := r.constructDatastore(params["child"].(map[string]interface{})) + childField, ok := params["child"].(map[string]interface{}) + if !ok { + return nil, fmt.Errorf("'child' field was missing or not a map") + } + child, err := r.constructDatastore(childField) if err != nil { return nil, err } - prefix := params["prefix"].(string) + prefix, ok := params["prefix"].(string) + if !ok { + return nil, fmt.Errorf("'prefix' field was missing or not a string") + } return r.openMeasureDB(prefix, child) @@ -54,12 +67,6 @@ func (r *FSRepo) constructDatastore(params map[string]interface{}) (repo.Datasto } } -type mountConfig struct { - Path string - ChildType string - Child *json.RawMessage -} - func (r *FSRepo) openMountDatastore(mountcfg []interface{}) (repo.Datastore, error) { var mounts []mount.Mount for _, iface := range mountcfg { @@ -85,22 +92,35 @@ func (r *FSRepo) openMountDatastore(mountcfg []interface{}) (repo.Datastore, err } func (r *FSRepo) openFlatfsDatastore(params map[string]interface{}) (repo.Datastore, error) { - p := params["path"].(string) + p, ok := params["path"].(string) + if !ok { + return nil, fmt.Errorf("'path' field is missing or not boolean") + } if !filepath.IsAbs(p) { p = filepath.Join(r.path, p) } - sshardFun := params["shardFunc"].(string) + sshardFun, ok := params["shardFunc"].(string) + if !ok { + return nil, fmt.Errorf("'shardFunc' field is missing or not a string") + } shardFun, err := flatfs.ParseShardFunc(sshardFun) if err != nil { return nil, err } - return flatfs.CreateOrOpen(p, shardFun, params["sync"].(bool)) + syncField, ok := params["sync"].(bool) + if !ok { + return nil, fmt.Errorf("'sync' field is missing or not boolean") + } + return flatfs.CreateOrOpen(p, shardFun, syncField) } func (r *FSRepo) openLeveldbDatastore(params map[string]interface{}) (repo.Datastore, error) { - p := params["path"].(string) + p, ok := params["path"].(string) + if !ok { + return nil, fmt.Errorf("'path' field is missing or not string") + } if !filepath.IsAbs(p) { p = filepath.Join(r.path, p) } diff --git a/repo/fsrepo/fsrepo_test.go b/repo/fsrepo/fsrepo_test.go index b048036ff5f..a2065a3a5d7 100644 --- a/repo/fsrepo/fsrepo_test.go +++ b/repo/fsrepo/fsrepo_test.go @@ -40,8 +40,8 @@ func TestCanManageReposIndependently(t *testing.T) { pathB := testRepoPath("b", t) t.Log("initialize two repos") - assert.Nil(Init(pathA, &config.Config{Datastore: *config.DefaultDatastoreConfig()}), t, "a", "should initialize successfully") - assert.Nil(Init(pathB, &config.Config{Datastore: *config.DefaultDatastoreConfig()}), t, "b", "should initialize successfully") + assert.Nil(Init(pathA, &config.Config{Datastore: config.DefaultDatastoreConfig()}), t, "a", "should initialize successfully") + assert.Nil(Init(pathB, &config.Config{Datastore: config.DefaultDatastoreConfig()}), t, "b", "should initialize successfully") t.Log("ensure repos initialized") assert.True(IsInitialized(pathA), t, "a should be initialized") @@ -67,7 +67,7 @@ func TestDatastoreGetNotAllowedAfterClose(t *testing.T) { path := testRepoPath("test", t) assert.True(!IsInitialized(path), t, "should NOT be initialized") - assert.Nil(Init(path, &config.Config{Datastore: *config.DefaultDatastoreConfig()}), t, "should initialize successfully") + assert.Nil(Init(path, &config.Config{Datastore: config.DefaultDatastoreConfig()}), t, "should initialize successfully") r, err := Open(path) assert.Nil(err, t, "should open successfully") @@ -84,7 +84,7 @@ func TestDatastorePersistsFromRepoToRepo(t *testing.T) { t.Parallel() path := testRepoPath("test", t) - assert.Nil(Init(path, &config.Config{Datastore: *config.DefaultDatastoreConfig()}), t) + assert.Nil(Init(path, &config.Config{Datastore: config.DefaultDatastoreConfig()}), t) r1, err := Open(path) assert.Nil(err, t) @@ -106,7 +106,7 @@ func TestDatastorePersistsFromRepoToRepo(t *testing.T) { func TestOpenMoreThanOnceInSameProcess(t *testing.T) { t.Parallel() path := testRepoPath("", t) - assert.Nil(Init(path, &config.Config{Datastore: *config.DefaultDatastoreConfig()}), t) + assert.Nil(Init(path, &config.Config{Datastore: config.DefaultDatastoreConfig()}), t) r1, err := Open(path) assert.Nil(err, t, "first repo should open successfully")