make fs object client windows compatible and add a method to get path…

…separator from object clients (grafana#2865) Signed-off-by: Sandeep Sukhani <sandeep.d.sukhani@gmail.com>
cyriltovena · Jul 21, 2020 · a673f74 · a673f74
1 parent 6e3cbe2
commit a673f74
Show file tree

Hide file tree

Showing 8 changed files with 41 additions and 14 deletions.
diff --git a/aws/s3_storage_client.go b/aws/s3_storage_client.go
@@ -325,3 +325,7 @@ func (a *S3ObjectClient) List(ctx context.Context, prefix string) ([]chunk.Stora
 
 	return storageObjects, commonPrefixes, nil
 }
+
+func (a *S3ObjectClient) PathSeparator() string {
+	return a.delimiter
+}
diff --git a/azure/blob_storage_client.go b/azure/blob_storage_client.go
@@ -210,3 +210,7 @@ func (b *BlobStorage) DeleteObject(ctx context.Context, blobID string) error {
 	_, err = blockBlobURL.Delete(ctx, azblob.DeleteSnapshotsOptionInclude, azblob.BlobAccessConditions{})
 	return err
 }
+
+func (b *BlobStorage) PathSeparator() string {
+	return b.delimiter
+}
diff --git a/gcp/gcs_object_client.go b/gcp/gcs_object_client.go
@@ -155,3 +155,7 @@ func (s *GCSObjectClient) DeleteObject(ctx context.Context, objectKey string) er
 
 	return nil
 }
+
+func (s *GCSObjectClient) PathSeparator() string {
+	return s.delimiter
+}
diff --git a/inmemory_storage_client.go b/inmemory_storage_client.go
@@ -390,6 +390,10 @@ func (m *MockStorage) List(ctx context.Context, prefix string) ([]StorageObject,
 	return storageObjects, []StorageCommonPrefix{}, nil
 }
 
+func (m *MockStorage) PathSeparator() string {
+	return DirDelim
+}
+
 type mockWriteBatch struct {
 	inserts []struct {
 		tableName, hashValue string

diff --git a/local/fs_object_client.go b/local/fs_object_client.go
@@ -34,7 +34,8 @@ func (cfg *FSConfig) RegisterFlagsWithPrefix(prefix string, f *flag.FlagSet) {
 
 // FSObjectClient holds config for filesystem as object store
 type FSObjectClient struct {
-	cfg FSConfig
+	cfg           FSConfig
+	pathSeparator string
 }
 
 // NewFSObjectClient makes a chunk.Client which stores chunks as files in the local filesystem.
@@ -48,7 +49,8 @@ func NewFSObjectClient(cfg FSConfig) (*FSObjectClient, error) {
 	}
 
 	return &FSObjectClient{
-		cfg: cfg,
+		cfg:           cfg,
+		pathSeparator: string(os.PathSeparator),
 	}, nil
 }
 
@@ -124,7 +126,7 @@ func (f *FSObjectClient) List(ctx context.Context, prefix string) ([]chunk.Stora
 
 			// add the directory only if it is not empty
 			if !empty {
-				commonPrefixes = append(commonPrefixes, chunk.StorageCommonPrefix(nameWithPrefix+chunk.DirDelim))
+				commonPrefixes = append(commonPrefixes, chunk.StorageCommonPrefix(nameWithPrefix+f.pathSeparator))
 			}
 			continue
 		}
@@ -173,6 +175,10 @@ func (f *FSObjectClient) DeleteChunksBefore(ctx context.Context, ts time.Time) e
 	})
 }
 
+func (f *FSObjectClient) PathSeparator() string {
+	return f.pathSeparator
+}
+
 // copied from https://github.com/thanos-io/thanos/blob/55cb8ca38b3539381dc6a781e637df15c694e50a/pkg/objstore/filesystem/filesystem.go#L181
 func isDirEmpty(name string) (ok bool, err error) {
 	f, err := os.Open(name)

diff --git a/local/fs_object_client_test.go b/local/fs_object_client_test.go
@@ -75,12 +75,12 @@ func TestFSObjectClient_List(t *testing.T) {
 	}()
 
 	foldersWithFiles := make(map[string][]string)
-	foldersWithFiles["folder1/"] = []string{"file1", "file2"}
-	foldersWithFiles["folder2/"] = []string{"file3", "file4", "file5"}
+	foldersWithFiles["folder1"] = []string{"file1", "file2"}
+	foldersWithFiles["folder2"] = []string{"file3", "file4", "file5"}
 
 	for folder, files := range foldersWithFiles {
 		for _, filename := range files {
-			err := bucketClient.PutObject(context.Background(), folder+filename, bytes.NewReader([]byte(filename)))
+			err := bucketClient.PutObject(context.Background(), filepath.Join(folder, filename), bytes.NewReader([]byte(filename)))
 			require.NoError(t, err)
 		}
 	}
@@ -105,7 +105,7 @@ func TestFSObjectClient_List(t *testing.T) {
 
 	require.Len(t, commonPrefixes, len(foldersWithFiles))
 	for _, commonPrefix := range commonPrefixes {
-		_, ok := foldersWithFiles[string(commonPrefix)]
+		_, ok := foldersWithFiles[string(commonPrefix)[:len(commonPrefix)-len(bucketClient.PathSeparator())]]
 		require.True(t, ok)
 	}
 
@@ -115,7 +115,7 @@ func TestFSObjectClient_List(t *testing.T) {
 
 		require.Len(t, storageObjects, len(files))
 		for i := range storageObjects {
-			require.Equal(t, storageObjects[i].Key, folder+files[i])
+			require.Equal(t, storageObjects[i].Key, filepath.Join(folder, files[i]))
 		}
 
 		require.Len(t, commonPrefixes, 0)
@@ -136,11 +136,11 @@ func TestFSObjectClient_DeleteObject(t *testing.T) {
 	}()
 
 	foldersWithFiles := make(map[string][]string)
-	foldersWithFiles["folder1/"] = []string{"file1", "file2"}
+	foldersWithFiles["folder1"] = []string{"file1", "file2"}
 
 	for folder, files := range foldersWithFiles {
 		for _, filename := range files {
-			err := bucketClient.PutObject(context.Background(), folder+filename, bytes.NewReader([]byte(filename)))
+			err := bucketClient.PutObject(context.Background(), filepath.Join(folder, filename), bytes.NewReader([]byte(filename)))
 			require.NoError(t, err)
 		}
 	}
@@ -151,15 +151,15 @@ func TestFSObjectClient_DeleteObject(t *testing.T) {
 	require.Len(t, commonPrefixes, len(foldersWithFiles))
 
 	// let us delete file1 from folder1 and check that file1 is gone but folder1 with file2 is still there
-	require.NoError(t, bucketClient.DeleteObject(context.Background(), "folder1/file1"))
-	_, err = os.Stat(filepath.Join(fsObjectsDir, "folder1/file1"))
+	require.NoError(t, bucketClient.DeleteObject(context.Background(), filepath.Join("folder1", "file1")))
+	_, err = os.Stat(filepath.Join(fsObjectsDir, filepath.Join("folder1", "file1")))
 	require.True(t, os.IsNotExist(err))
 
-	_, err = os.Stat(filepath.Join(fsObjectsDir, "folder1/file2"))
+	_, err = os.Stat(filepath.Join(fsObjectsDir, filepath.Join("folder1", "file2")))
 	require.NoError(t, err)
 
 	// let us delete second file as well and check that folder1 also got removed
-	require.NoError(t, bucketClient.DeleteObject(context.Background(), "folder1/file2"))
+	require.NoError(t, bucketClient.DeleteObject(context.Background(), filepath.Join("folder1", "file2")))
 	_, err = os.Stat(filepath.Join(fsObjectsDir, "folder1"))
 	require.True(t, os.IsNotExist(err))
 

diff --git a/openstack/swift_object_client.go b/openstack/swift_object_client.go
@@ -176,3 +176,7 @@ func (s *SwiftObjectClient) DeleteObject(ctx context.Context, objectKey string)
 	}
 	return err
 }
+
+func (s *SwiftObjectClient) PathSeparator() string {
+	return string(s.delimiter)
+}
diff --git a/storage_client.go b/storage_client.go
@@ -67,6 +67,7 @@ type ObjectClient interface {
 	GetObject(ctx context.Context, objectKey string) (io.ReadCloser, error)
 	List(ctx context.Context, prefix string) ([]StorageObject, []StorageCommonPrefix, error)
 	DeleteObject(ctx context.Context, objectKey string) error
+	PathSeparator() string
 	Stop()
 }