address p.r. comments

License: MIT Signed-off-by: Kevin Atkinson <[email protected]>
ipfs · Jun 13, 2017 · dd93cd8 · dd93cd8
1 parent bd082af
commit dd93cd8
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 25 deletions.
diff --git 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
@@ -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
@@ -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")