From b80c77230ad59704e8c773dd1dcf99aa957353ef Mon Sep 17 00:00:00 2001 From: Graham Forest Date: Fri, 18 Mar 2022 09:45:39 -0700 Subject: [PATCH] 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 --- localfs/store.go | 22 ++++++-------- localfs/store_test.go | 71 ++++++++++++++++++++++++++++--------------- 2 files changed, 57 insertions(+), 36 deletions(-) diff --git a/localfs/store.go b/localfs/store.go index 898b367..a2ce337 100644 --- a/localfs/store.go +++ b/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 diff --git a/localfs/store_test.go b/localfs/store_test.go index d0e7b4c..38c0848 100644 --- a/localfs/store_test.go +++ b/localfs/store_test.go @@ -2,10 +2,12 @@ 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" @@ -13,15 +15,18 @@ import ( ) 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) }