Segregate optionally based on bucket, fix tests (#99)

- If we have a bucket, use it in the path so we don't stomp other
  buckets' files
- Also make localfs tests parallel compatible
- Use require more

Author: Graham Forest

localfs/store.go

@@ -26,7 +26,7 @@ func init() {
 	cloudstorage.Register(StoreType, localProvider)
 }
 func localProvider(conf *cloudstorage.Config) (cloudstorage.Store, error) {
-	store, err := NewLocalStore(conf.LocalFS, conf.TmpDir)
+	store, err := NewLocalStore(conf.Bucket, conf.LocalFS, conf.TmpDir)
 	if err != nil {
 		return nil, err
 	}
@@ -48,14 +48,13 @@ const (
 
 // LocalStore is client to local-filesystem store.
 type LocalStore struct {
-	storepath   string // possibly is relative  ./tables
-	pathCleaned string // cleaned removing  ./ = "tables"
-	cachepath   string
-	Id          string
+	storepath string // possibly is relative  ./tables
+	cachepath string
+	Id        string
 }
 
 // NewLocalStore create local store from storage path on local filesystem, and cachepath.
-func NewLocalStore(storepath, cachepath string) (*LocalStore, error) {
+func NewLocalStore(bucket, storepath, cachepath string) (*LocalStore, error) {
 
 	if storepath == "" {
 		return nil, fmt.Errorf("storepath=%q cannot be empty", storepath)
@@ -65,7 +64,7 @@ func NewLocalStore(storepath, cachepath string) (*LocalStore, error) {
 		return nil, fmt.Errorf("storepath=%q cannot be the same as cachepath=%q", storepath, cachepath)
 	}
 
-	pathCleaned := strings.TrimPrefix(storepath, "./")
+	storepath = filepath.Join(storepath, bucket)
 
 	err := os.MkdirAll(storepath, 0775)
 	if err != nil {
@@ -81,10 +80,9 @@ func NewLocalStore(storepath, cachepath string) (*LocalStore, error) {
 	uid = strings.Replace(uid, "-", "", -1)
 
 	return &LocalStore{
-		storepath:   storepath,
-		pathCleaned: pathCleaned,
-		cachepath:   cachepath,
-		Id:          uid,
+		storepath: storepath,
+		cachepath: cachepath,
+		Id:        uid,
 	}, nil
 }
 
@@ -137,7 +135,7 @@ func (l *LocalStore) List(ctx context.Context, query cloudstorage.Query) (*cloud
 			return err
 		}
 
-		obj := strings.Replace(fo, l.pathCleaned, "", 1)
+		obj := strings.Replace(fo, l.storepath, "", 1)
 
 		if f.IsDir() {
 			return nil

localfs/store_test.go

@@ -2,26 +2,31 @@ package localfs_test
 
 import (
 	"context"
+	"io/ioutil"
 	"os"
+	"path/filepath"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/lytics/cloudstorage"
 	"github.com/lytics/cloudstorage/localfs"
 	"github.com/lytics/cloudstorage/testutils"
 )
 
 func TestAll(t *testing.T) {
+	t.Parallel()
 
-	os.RemoveAll("/tmp/mockcloud")
-	os.RemoveAll("/tmp/localcache")
+	tmpDir, err := ioutil.TempDir("/tmp", "all")
+	require.NoError(t, err)
+	defer os.RemoveAll(tmpDir)
 
 	localFsConf := &cloudstorage.Config{
 		Type:       localfs.StoreType,
 		AuthMethod: localfs.AuthFileSystem,
-		LocalFS:    "/tmp/mockcloud",
-		TmpDir:     "/tmp/localcache",
+		LocalFS:    filepath.Join(tmpDir, "mockcloud"),
+		TmpDir:     filepath.Join(tmpDir, "localcache"),
+		Bucket:     "all",
 	}
 
 	store, err := cloudstorage.NewStore(localFsConf)
@@ -30,16 +35,20 @@ func TestAll(t *testing.T) {
 		return
 	}
 	testutils.RunTests(t, store, localFsConf)
+}
+
+func TestBrusted(t *testing.T) {
+	t.Parallel()
 
 	// invalid config:  empty/missing LocalFS
-	localFsConf = &cloudstorage.Config{
+	localFsConf := &cloudstorage.Config{
 		Type:       localfs.StoreType,
 		AuthMethod: localfs.AuthFileSystem,
 		LocalFS:    "",
 	}
-	store, err = cloudstorage.NewStore(localFsConf)
-	assert.NotEqual(t, nil, err)
-	assert.Equal(t, nil, store)
+	store, err := cloudstorage.NewStore(localFsConf)
+	require.Error(t, err)
+	require.Equal(t, nil, store)
 
 	// invalid config:  LocalFS = TempDir
 	localFsConf = &cloudstorage.Config{
@@ -49,42 +58,56 @@ func TestAll(t *testing.T) {
 		TmpDir:     "/tmp/invalid",
 	}
 	store, err = cloudstorage.NewStore(localFsConf)
-	assert.NotEqual(t, nil, err)
-	assert.Equal(t, nil, store)
+	require.Error(t, err)
+	require.Equal(t, nil, store)
 }
 
 func TestNewReaderDir(t *testing.T) {
+	t.Parallel()
+
+	tmpDir, err := ioutil.TempDir("/tmp", "newreaderdir")
+	require.NoError(t, err)
+	defer os.RemoveAll(tmpDir)
+
 	// When a dir is requested, serve the index.html file instead
 	localFsConf := &cloudstorage.Config{
 		Type:       localfs.StoreType,
 		AuthMethod: localfs.AuthFileSystem,
-		LocalFS:    "/tmp/mockcloud",
-		TmpDir:     "/tmp/localcache",
+		LocalFS:    filepath.Join(tmpDir, "mockcloud"),
+		TmpDir:     filepath.Join(tmpDir, "localcache"),
+		Bucket:     "newreaderdir",
 	}
 	store, err := cloudstorage.NewStore(localFsConf)
 	testutils.MockFile(store, "test/index.html", "test")
-	assert.Equal(t, nil, err)
-	assert.Equal(t, nil, err)
+	require.NoError(t, err)
+	require.Equal(t, nil, err)
 	_, err = store.NewReader("test")
-	assert.Equal(t, err, cloudstorage.ErrObjectNotFound)
+	require.Equal(t, err, cloudstorage.ErrObjectNotFound)
 	err = store.Delete(context.Background(), "test/index.html")
-	assert.Equal(t, nil, err)
+	require.NoError(t, err)
 }
 
 func TestGetDir(t *testing.T) {
+	t.Parallel()
+
+	tmpDir, err := ioutil.TempDir("/tmp", "getdir")
+	require.NoError(t, err)
+	defer os.RemoveAll(tmpDir)
+
 	// When a dir is requested, serve the index.html file instead
 	localFsConf := &cloudstorage.Config{
 		Type:       localfs.StoreType,
 		AuthMethod: localfs.AuthFileSystem,
-		LocalFS:    "/tmp/mockcloud",
-		TmpDir:     "/tmp/localcache",
+		LocalFS:    filepath.Join(tmpDir, "mockcloud"),
+		TmpDir:     filepath.Join(tmpDir, "localcache"),
+		Bucket:     "getdir",
 	}
 	store, err := cloudstorage.NewStore(localFsConf)
-	testutils.MockFile(store, "test/index.html", "test")
-	assert.Equal(t, nil, err)
-	assert.Equal(t, nil, err)
+	require.NoError(t, err)
+	err = testutils.MockFile(store, "test/index.html", "test")
+	require.NoError(t, err)
 	_, err = store.Get(context.Background(), "test")
-	assert.Equal(t, err, cloudstorage.ErrObjectNotFound)
+	require.Equal(t, err, cloudstorage.ErrObjectNotFound)
 	err = store.Delete(context.Background(), "test/index.html")
-	assert.Equal(t, nil, err)
+	require.NoError(t, err)
 }