diff --git a/modules/storage/azureblob.go b/modules/storage/azureblob.go index acf60972582..2981b48177c 100644 --- a/modules/storage/azureblob.go +++ b/modules/storage/azureblob.go @@ -32,21 +32,21 @@ var _ Object = &azureBlobObject{} type azureBlobObject struct { blobClient *blob.Client - Context context.Context - Name string - Size int64 - ModTime *time.Time + ctx context.Context + name string + size int64 + modTime *time.Time offset int64 } func (a *azureBlobObject) Read(p []byte) (int, error) { // TODO: improve the performance, we can implement another interface, maybe implement io.WriteTo - if a.offset >= a.Size { + if a.offset >= a.size { return 0, io.EOF } - count := min(int64(len(p)), a.Size-a.offset) + count := min(int64(len(p)), a.size-a.offset) - res, err := a.blobClient.DownloadBuffer(a.Context, p, &blob.DownloadBufferOptions{ + res, err := a.blobClient.DownloadBuffer(a.ctx, p, &blob.DownloadBufferOptions{ Range: blob.HTTPRange{ Offset: a.offset, Count: count, @@ -71,12 +71,12 @@ func (a *azureBlobObject) Seek(offset int64, whence int) (int64, error) { case io.SeekCurrent: offset += a.offset case io.SeekEnd: - offset = a.Size + offset + offset = a.size + offset default: return 0, errors.New("Seek: invalid whence") } - if offset > a.Size { + if offset > a.size { return 0, errors.New("Seek: invalid offset") } else if offset < 0 { return 0, errors.New("Seek: invalid offset") @@ -87,15 +87,14 @@ func (a *azureBlobObject) Seek(offset int64, whence int) (int64, error) { func (a *azureBlobObject) Stat() (os.FileInfo, error) { return &azureBlobFileInfo{ - a.Name, - a.Size, - *a.ModTime, + a.name, + a.size, + *a.modTime, }, nil } var _ ObjectStorage = &AzureBlobStorage{} -// AzureStorage returns a azure blob storage type AzureBlobStorage struct { cfg *setting.AzureBlobStorageConfig ctx context.Context @@ -150,11 +149,7 @@ func NewAzureBlobStorage(ctx context.Context, cfg *setting.Storage) (ObjectStora } func (a *AzureBlobStorage) buildAzureBlobPath(p string) string { - p = util.PathJoinRelX(a.cfg.BasePath, p) - if p == "." || p == "/" { - p = "" // azure uses prefix, so path should be empty as relative path - } - return p + return buildObjectStorePath(a.cfg.BasePath, p) } func (a *AzureBlobStorage) getObjectNameFromPath(path string) string { @@ -170,11 +165,11 @@ func (a *AzureBlobStorage) Open(path string) (Object, error) { return nil, convertAzureBlobErr(err) } return &azureBlobObject{ - Context: a.ctx, + ctx: a.ctx, blobClient: blobClient, - Name: a.getObjectNameFromPath(path), - Size: *res.ContentLength, - ModTime: res.LastModified, + name: a.getObjectNameFromPath(path), + size: *res.ContentLength, + modTime: res.LastModified, }, nil } @@ -302,33 +297,32 @@ func (a *AzureBlobStorage) ServeDirectURL(storePath, name, method string, reqPar return url.Parse(u) } -// IterateObjects iterates across the objects in the azureblobstorage func (a *AzureBlobStorage) IterateObjects(dirName string, fn func(path string, obj Object) error) error { - dirName = a.buildAzureBlobPath(dirName) - if dirName != "" { - dirName += "/" - } + basePrefix := buildObjectStorePathPrefix(a.cfg.BasePath, "") + dirPrefix := buildObjectStorePathPrefix(a.cfg.BasePath, dirName) pager := a.client.NewListBlobsFlatPager(a.cfg.Container, &container.ListBlobsFlatOptions{ - Prefix: &dirName, + Prefix: &dirPrefix, }) + + callback := func(object *azureBlobObject, objPath string) error { + defer object.Close() + return fn(objPath, object) + } for pager.More() { resp, err := pager.NextPage(a.ctx) if err != nil { return convertAzureBlobErr(err) } - for _, object := range resp.Segment.BlobItems { - blobClient := a.getBlobClient(*object.Name) - object := &azureBlobObject{ - Context: a.ctx, - blobClient: blobClient, - Name: *object.Name, - Size: *object.Properties.ContentLength, - ModTime: object.Properties.LastModified, + for _, azureObj := range resp.Segment.BlobItems { + objPath := strings.TrimPrefix(*azureObj.Name, basePrefix) + objWrap := &azureBlobObject{ + ctx: a.ctx, + blobClient: a.getBlobClient(objPath), + name: *azureObj.Name, + size: *azureObj.Properties.ContentLength, + modTime: azureObj.Properties.LastModified, } - if err := func(object *azureBlobObject, fn func(path string, obj Object) error) error { - defer object.Close() - return fn(strings.TrimPrefix(object.Name, a.cfg.BasePath), object) - }(object, fn); err != nil { + if err := callback(objWrap, objPath); err != nil { return convertAzureBlobErr(err) } } @@ -336,7 +330,6 @@ func (a *AzureBlobStorage) IterateObjects(dirName string, fn func(path string, o return nil } -// Delete delete a file func (a *AzureBlobStorage) getBlobClient(path string) *blob.Client { return a.client.ServiceClient().NewContainerClient(a.cfg.Container).NewBlobClient(a.buildAzureBlobPath(path)) } diff --git a/modules/storage/azureblob_test.go b/modules/storage/azureblob_test.go index 6f184ea0058..5a1ce4ef6c8 100644 --- a/modules/storage/azureblob_test.go +++ b/modules/storage/azureblob_test.go @@ -10,82 +10,45 @@ import ( "gitea.dev/modules/setting" "gitea.dev/modules/test" + "gitea.dev/modules/util" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) -func TestAzureBlobStorage(t *testing.T) { +func prepareAzureStorageConfig(t *testing.T, basePath ...string) *setting.Storage { endpoint := test.ExternalServiceHTTP(t, "TEST_AZURESTORAGE_ENDPOINT", "http://devstoreaccount1.azurite.local:10000") - storageType := setting.AzureBlobStorageType - config := &setting.Storage{ + return &setting.Storage{ AzureBlobConfig: setting.AzureBlobStorageConfig{ // https://learn.microsoft.com/azure/storage/common/storage-use-azurite?tabs=visual-studio-code#ip-style-url Endpoint: endpoint, // https://learn.microsoft.com/azure/storage/common/storage-use-azurite?tabs=visual-studio-code#well-known-storage-account-and-key AccountName: "devstoreaccount1", AccountKey: "Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==", - Container: "test", + Container: "test-container", + BasePath: util.OptionalArg(basePath), }, } - table := []struct { - name string - test func(t *testing.T, typStr Type, cfg *setting.Storage) - }{ - { - name: "iterator", - test: testStorageIterator, - }, - { - name: "testBlobStorageURLContentTypeAndDisposition", - test: testBlobStorageURLContentTypeAndDisposition, - }, - } - for _, entry := range table { - t.Run(entry.name, func(t *testing.T) { - entry.test(t, storageType, config) - }) - } } -func TestAzureBlobStoragePath(t *testing.T) { - m := &AzureBlobStorage{cfg: &setting.AzureBlobStorageConfig{BasePath: ""}} - assert.Empty(t, m.buildAzureBlobPath("/")) - assert.Empty(t, m.buildAzureBlobPath(".")) - assert.Equal(t, "a", m.buildAzureBlobPath("/a")) - assert.Equal(t, "a/b", m.buildAzureBlobPath("/a/b/")) - - m = &AzureBlobStorage{cfg: &setting.AzureBlobStorageConfig{BasePath: "/"}} - assert.Empty(t, m.buildAzureBlobPath("/")) - assert.Empty(t, m.buildAzureBlobPath(".")) - assert.Equal(t, "a", m.buildAzureBlobPath("/a")) - assert.Equal(t, "a/b", m.buildAzureBlobPath("/a/b/")) - - m = &AzureBlobStorage{cfg: &setting.AzureBlobStorageConfig{BasePath: "/base"}} - assert.Equal(t, "base", m.buildAzureBlobPath("/")) - assert.Equal(t, "base", m.buildAzureBlobPath(".")) - assert.Equal(t, "base/a", m.buildAzureBlobPath("/a")) - assert.Equal(t, "base/a/b", m.buildAzureBlobPath("/a/b/")) - - m = &AzureBlobStorage{cfg: &setting.AzureBlobStorageConfig{BasePath: "/base/"}} - assert.Equal(t, "base", m.buildAzureBlobPath("/")) - assert.Equal(t, "base", m.buildAzureBlobPath(".")) - assert.Equal(t, "base/a", m.buildAzureBlobPath("/a")) - assert.Equal(t, "base/a/b", m.buildAzureBlobPath("/a/b/")) +func TestAzureBlobStorage(t *testing.T) { + t.Run("NoBasePath", func(t *testing.T) { + config := prepareAzureStorageConfig(t) + objStore, err := NewStorage(setting.AzureBlobStorageType, config) + require.NoError(t, err) + testStorageGeneral(t, objStore) + }) + t.Run("WithBasePath", func(t *testing.T) { + config := prepareAzureStorageConfig(t, "test-base-path") + objStore, err := NewStorage(setting.AzureBlobStorageType, config) + require.NoError(t, err) + testStorageGeneral(t, objStore) + }) } func Test_azureBlobObject(t *testing.T) { - endpoint := test.ExternalServiceHTTP(t, "TEST_AZURESTORAGE_ENDPOINT", "http://devstoreaccount1.azurite.local:10000") - s, err := NewStorage(setting.AzureBlobStorageType, &setting.Storage{ - AzureBlobConfig: setting.AzureBlobStorageConfig{ - // https://learn.microsoft.com/azure/storage/common/storage-use-azurite?tabs=visual-studio-code#ip-style-url - Endpoint: endpoint, - // https://learn.microsoft.com/azure/storage/common/storage-use-azurite?tabs=visual-studio-code#well-known-storage-account-and-key - AccountName: "devstoreaccount1", - AccountKey: "Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==", - Container: "test", - }, - }) - assert.NoError(t, err) + s, err := NewStorage(setting.AzureBlobStorageType, prepareAzureStorageConfig(t)) + require.NoError(t, err) data := "Q2xTckt6Y1hDOWh0" _, err = s.Save("test.txt", strings.NewReader(data), int64(len(data))) diff --git a/modules/storage/local_test.go b/modules/storage/local_test.go index 86fbb20ae9c..a12b1cda03b 100644 --- a/modules/storage/local_test.go +++ b/modules/storage/local_test.go @@ -14,6 +14,12 @@ import ( "github.com/stretchr/testify/require" ) +func TestLocalStorage(t *testing.T) { + objStore, err := NewStorage(setting.LocalStorageType, &setting.Storage{Path: t.TempDir()}) + require.NoError(t, err) + testStorageGeneral(t, objStore) +} + func TestBuildLocalPath(t *testing.T) { kases := []struct { localDir string @@ -98,7 +104,3 @@ func TestLocalStorageDelete(t *testing.T) { assertExists(t, ".", true) assertExists(t, "dir", false) } - -func TestLocalStorageIterator(t *testing.T) { - testStorageIterator(t, setting.LocalStorageType, &setting.Storage{Path: t.TempDir()}) -} diff --git a/modules/storage/minio.go b/modules/storage/minio.go index c447be2170f..db43ba2484e 100644 --- a/modules/storage/minio.go +++ b/modules/storage/minio.go @@ -158,20 +158,11 @@ func NewMinioStorage(ctx context.Context, cfg *setting.Storage) (ObjectStorage, } func (m *MinioStorage) buildMinioPath(p string) string { - p = strings.TrimPrefix(util.PathJoinRelX(m.basePath, p), "/") // object store doesn't use slash for root path - if p == "." { - p = "" // object store doesn't use dot as relative path - } - return p + return buildObjectStorePath(m.basePath, p) } func (m *MinioStorage) buildMinioDirPrefix(p string) string { - // ending slash is required for avoiding matching like "foo/" and "foobar/" with prefix "foo" - p = m.buildMinioPath(p) + "/" - if p == "/" { - p = "" // object store doesn't use slash for root path - } - return p + return buildObjectStorePathPrefix(m.basePath, p) } func buildMinioCredentials(config setting.MinioStorageConfig) *credentials.Credentials { @@ -312,22 +303,26 @@ func (m *MinioStorage) ServeDirectURL(storePath, name, method string, opt *Serve return u, convertMinioErr(err) } -// IterateObjects iterates across the objects in the miniostorage func (m *MinioStorage) IterateObjects(dirName string, fn func(path string, obj Object) error) error { - opts := minio.GetObjectOptions{} - // FIXME: this loop is not right and causes resource leaking, see the comment of ListObjects - for mObjInfo := range m.client.ListObjects(m.ctx, m.bucket, minio.ListObjectsOptions{ - Prefix: m.buildMinioDirPrefix(dirName), - Recursive: true, - }) { - object, err := m.client.GetObject(m.ctx, m.bucket, mObjInfo.Key, opts) + basePrefix := m.buildMinioDirPrefix("") + dirPrefix := m.buildMinioDirPrefix(dirName) + callback := func(object *minio.Object, objPath string) error { + defer object.Close() + return fn(objPath, &minioObject{object}) + } + + ctxList, ctxListCancel := context.WithCancel(m.ctx) + defer ctxListCancel() // ListObjectsIter: make sure to cancel the passed context, without that you might leak coroutines + + getOpts := minio.GetObjectOptions{} + listOpts := minio.ListObjectsOptions{Prefix: dirPrefix, Recursive: true} + for mObjInfo := range m.client.ListObjectsIter(ctxList, m.bucket, listOpts) { + object, err := m.client.GetObject(m.ctx, m.bucket, mObjInfo.Key, getOpts) if err != nil { return convertMinioErr(err) } - if err := func(object *minio.Object, fn func(path string, obj Object) error) error { - defer object.Close() - return fn(strings.TrimPrefix(mObjInfo.Key, m.basePath), &minioObject{object}) - }(object, fn); err != nil { + objPath := strings.TrimPrefix(mObjInfo.Key, basePrefix) + if err := callback(object, objPath); err != nil { return convertMinioErr(err) } } diff --git a/modules/storage/minio_test.go b/modules/storage/minio_test.go index 67c4d189330..81c71f73eb9 100644 --- a/modules/storage/minio_test.go +++ b/modules/storage/minio_test.go @@ -10,89 +10,45 @@ import ( "gitea.dev/modules/setting" "gitea.dev/modules/test" + "gitea.dev/modules/util" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) -func TestMinioStorage(t *testing.T) { - endpoint := test.ExternalServiceHTTP(t, "TEST_MINIO_ENDPOINT", "minio:9000") - storageType := setting.MinioStorageType - config := &setting.Storage{ +func prepareMinioStorageConfig(t *testing.T, basePath ...string) *setting.Storage { + return &setting.Storage{ MinioConfig: setting.MinioStorageConfig{ - Endpoint: endpoint, + Endpoint: test.ExternalServiceHTTP(t, "TEST_MINIO_ENDPOINT", "minio:9000"), AccessKeyID: "123456", SecretAccessKey: "12345678", Bucket: "gitea", Location: "us-east-1", + BasePath: util.OptionalArg(basePath), }, } - table := []struct { - name string - test func(t *testing.T, typStr Type, cfg *setting.Storage) - }{ - { - name: "iterator", - test: testStorageIterator, - }, - { - name: "testBlobStorageURLContentTypeAndDisposition", - test: testBlobStorageURLContentTypeAndDisposition, - }, - } - for _, entry := range table { - t.Run(entry.name, func(t *testing.T) { - entry.test(t, storageType, config) - }) - } } -func TestMinioStoragePath(t *testing.T) { - m := &MinioStorage{basePath: ""} - assert.Empty(t, m.buildMinioPath("/")) - assert.Empty(t, m.buildMinioPath(".")) - assert.Equal(t, "a", m.buildMinioPath("/a")) - assert.Equal(t, "a/b", m.buildMinioPath("/a/b/")) - assert.Empty(t, m.buildMinioDirPrefix("")) - assert.Equal(t, "a/", m.buildMinioDirPrefix("/a/")) - - m = &MinioStorage{basePath: "/"} - assert.Empty(t, m.buildMinioPath("/")) - assert.Empty(t, m.buildMinioPath(".")) - assert.Equal(t, "a", m.buildMinioPath("/a")) - assert.Equal(t, "a/b", m.buildMinioPath("/a/b/")) - assert.Empty(t, m.buildMinioDirPrefix("")) - assert.Equal(t, "a/", m.buildMinioDirPrefix("/a/")) - - m = &MinioStorage{basePath: "/base"} - assert.Equal(t, "base", m.buildMinioPath("/")) - assert.Equal(t, "base", m.buildMinioPath(".")) - assert.Equal(t, "base/a", m.buildMinioPath("/a")) - assert.Equal(t, "base/a/b", m.buildMinioPath("/a/b/")) - assert.Equal(t, "base/", m.buildMinioDirPrefix("")) - assert.Equal(t, "base/a/", m.buildMinioDirPrefix("/a/")) - - m = &MinioStorage{basePath: "/base/"} - assert.Equal(t, "base", m.buildMinioPath("/")) - assert.Equal(t, "base", m.buildMinioPath(".")) - assert.Equal(t, "base/a", m.buildMinioPath("/a")) - assert.Equal(t, "base/a/b", m.buildMinioPath("/a/b/")) - assert.Equal(t, "base/", m.buildMinioDirPrefix("")) - assert.Equal(t, "base/a/", m.buildMinioDirPrefix("/a/")) +func TestMinioStorage(t *testing.T) { + t.Run("NoBasePath", func(t *testing.T) { + config := prepareMinioStorageConfig(t) + objStore, err := NewStorage(setting.MinioStorageType, config) + require.NoError(t, err) + testStorageGeneral(t, objStore) + }) + t.Run("WithBasePath", func(t *testing.T) { + config := prepareMinioStorageConfig(t, "test-base-path") + objStore, err := NewStorage(setting.MinioStorageType, config) + require.NoError(t, err) + testStorageGeneral(t, objStore) + }) } func TestS3StorageBadRequest(t *testing.T) { - endpoint := test.ExternalServiceHTTP(t, "TEST_MINIO_ENDPOINT", "minio:9000") - cfg := &setting.Storage{ - MinioConfig: setting.MinioStorageConfig{ - Endpoint: endpoint, - AccessKeyID: "123456", - SecretAccessKey: "invalid-secret", - Bucket: "bucket", - Location: "us-east-1", - }, - } + cfg := prepareMinioStorageConfig(t) + cfg.MinioConfig.SecretAccessKey = "invalid-secret" _, err := NewStorage(setting.MinioStorageType, cfg) - assert.ErrorContains(t, err, "ObjectStorage.BucketExists: endpoint="+endpoint) + assert.ErrorContains(t, err, "ObjectStorage.BucketExists: endpoint="+cfg.MinioConfig.Endpoint) } func TestMinioCredentials(t *testing.T) { diff --git a/modules/storage/storage.go b/modules/storage/storage.go index 43c79f5b35c..ba3058d333a 100644 --- a/modules/storage/storage.go +++ b/modules/storage/storage.go @@ -11,11 +11,13 @@ import ( "net/url" "os" "path" + "strings" "gitea.dev/modules/httplib" "gitea.dev/modules/log" "gitea.dev/modules/public" "gitea.dev/modules/setting" + "gitea.dev/modules/util" ) // ErrURLNotSupported represents url is not supported @@ -139,6 +141,23 @@ func SaveFrom(objStorage ObjectStorage, path string, callback func(w io.Writer) return err } +func buildObjectStorePath(base, p string) string { + p = strings.TrimPrefix(util.PathJoinRelX(base, p), "/") // object store doesn't use slash for root path + if p == "." { + p = "" // object store doesn't use dot as relative path + } + return p +} + +func buildObjectStorePathPrefix(base, p string) string { + // ending slash is required for avoiding matching like "foo/" and "foobar/" with prefix "foo" + p = buildObjectStorePath(base, p) + "/" + if p == "/" { + p = "" // object store doesn't use slash for root path + } + return p +} + var ( // Attachments represents attachments storage Attachments ObjectStorage = uninitializedStorage diff --git a/modules/storage/storage_test.go b/modules/storage/storage_test.go index 003b1a9e8a8..7960279b89c 100644 --- a/modules/storage/storage_test.go +++ b/modules/storage/storage_test.go @@ -4,20 +4,50 @@ package storage import ( + "io" "net/http" "strings" "testing" - "gitea.dev/modules/setting" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func testStorageIterator(t *testing.T, typStr Type, cfg *setting.Storage) { - l, err := NewStorage(typStr, cfg) - assert.NoError(t, err) +func TestObjectStoragePath(t *testing.T) { + base := "" + assert.Empty(t, buildObjectStorePath(base, "/")) + assert.Empty(t, buildObjectStorePath(base, ".")) + assert.Equal(t, "a", buildObjectStorePath(base, "/a")) + assert.Equal(t, "a/b", buildObjectStorePath(base, "/a/b/")) + assert.Empty(t, buildObjectStorePathPrefix(base, "")) + assert.Equal(t, "a/", buildObjectStorePathPrefix(base, "/a/")) + base = "/" + assert.Empty(t, buildObjectStorePath(base, "/")) + assert.Empty(t, buildObjectStorePath(base, ".")) + assert.Equal(t, "a", buildObjectStorePath(base, "/a")) + assert.Equal(t, "a/b", buildObjectStorePath(base, "/a/b/")) + assert.Empty(t, buildObjectStorePathPrefix(base, "")) + assert.Equal(t, "a/", buildObjectStorePathPrefix(base, "/a/")) + + base = "/base" + assert.Equal(t, "base", buildObjectStorePath(base, "/")) + assert.Equal(t, "base", buildObjectStorePath(base, ".")) + assert.Equal(t, "base/a", buildObjectStorePath(base, "/a")) + assert.Equal(t, "base/a/b", buildObjectStorePath(base, "/a/b/")) + assert.Equal(t, "base/", buildObjectStorePathPrefix(base, "")) + assert.Equal(t, "base/a/", buildObjectStorePathPrefix(base, "/a/")) + + base = "/base/" + assert.Equal(t, "base", buildObjectStorePath(base, "/")) + assert.Equal(t, "base", buildObjectStorePath(base, ".")) + assert.Equal(t, "base/a", buildObjectStorePath(base, "/a")) + assert.Equal(t, "base/a/b", buildObjectStorePath(base, "/a/b/")) + assert.Equal(t, "base/", buildObjectStorePathPrefix(base, "")) + assert.Equal(t, "base/a/", buildObjectStorePathPrefix(base, "/a/")) +} + +func testStorageIterator(t *testing.T, objStore ObjectStorage) { testFiles := [][]string{ {"a/1.txt", "a1"}, {"/a/1.txt", "aa1"}, // same as above, but with leading slash that will be trim @@ -28,12 +58,19 @@ func testStorageIterator(t *testing.T, typStr Type, cfg *setting.Storage) { {"b/x 4.txt", "bx4"}, } for _, f := range testFiles { - _, err = l.Save(f[0], strings.NewReader(f[1]), -1) + _, err := objStore.Save(f[0], strings.NewReader(f[1]), -1) assert.NoError(t, err) } + defer func() { + for _, f := range testFiles { + _ = objStore.Delete(f[0]) + } + }() expectedList := map[string][]string{ "a": {"a/1.txt"}, + "a/": {"a/1.txt"}, + "/a/": {"a/1.txt"}, "b": {"b/1.txt", "b/2.txt", "b/3.txt", "b/x 4.txt"}, "": {"a/1.txt", "b/1.txt", "b/2.txt", "b/3.txt", "b/x 4.txt", "ab/1.txt"}, "/": {"a/1.txt", "b/1.txt", "b/2.txt", "b/3.txt", "b/x 4.txt", "ab/1.txt"}, @@ -42,8 +79,10 @@ func testStorageIterator(t *testing.T, typStr Type, cfg *setting.Storage) { } for dir, expected := range expectedList { count := 0 - err = l.IterateObjects(dir, func(path string, f Object) error { - defer f.Close() + err := objStore.IterateObjects(dir, func(path string, f Object) error { + content, err := io.ReadAll(f) + assert.NoError(t, err) + assert.NotEmpty(t, content) assert.Contains(t, expected, path) count++ return nil @@ -53,52 +92,57 @@ func testStorageIterator(t *testing.T, typStr Type, cfg *setting.Storage) { } } -type expectedServeDirectHeaders struct { - ContentType string - ContentDisposition string -} - -func testSingleBlobStorageURLContentTypeAndDisposition(t *testing.T, s ObjectStorage, path, name string, expected expectedServeDirectHeaders, reqParams *ServeDirectOptions) { - u, err := s.ServeDirectURL(path, name, http.MethodGet, reqParams) - require.NoError(t, err) - resp, err := http.Get(u.String()) - require.NoError(t, err) - defer resp.Body.Close() - if expected.ContentType != "" { - assert.Equal(t, expected.ContentType, resp.Header.Get("Content-Type")) +func testStorageURLContentTypeAndDisposition(t *testing.T, objStore ObjectStorage) { + type expectedServeDirectHeaders struct { + ContentType string + ContentDisposition string } - if expected.ContentDisposition != "" { - assert.Equal(t, expected.ContentDisposition, resp.Header.Get("Content-Disposition")) + test := func(t *testing.T, s ObjectStorage, path, name string, expected expectedServeDirectHeaders, reqParams *ServeDirectOptions) { + u, err := s.ServeDirectURL(path, name, http.MethodGet, reqParams) + require.NoError(t, err) + resp, err := http.Get(u.String()) + require.NoError(t, err) + defer resp.Body.Close() + if expected.ContentType != "" { + assert.Equal(t, expected.ContentType, resp.Header.Get("Content-Type")) + } + if expected.ContentDisposition != "" { + assert.Equal(t, expected.ContentDisposition, resp.Header.Get("Content-Disposition")) + } } -} - -func testBlobStorageURLContentTypeAndDisposition(t *testing.T, typStr Type, cfg *setting.Storage) { - s, err := NewStorage(typStr, cfg) - assert.NoError(t, err) testFilename := "test.txt" - _, err = s.Save(testFilename, strings.NewReader("dummy-content"), -1) + _, err := objStore.Save(testFilename, strings.NewReader("dummy-content"), -1) assert.NoError(t, err) - testSingleBlobStorageURLContentTypeAndDisposition(t, s, testFilename, "test.txt", expectedServeDirectHeaders{ + test(t, objStore, testFilename, "test.txt", expectedServeDirectHeaders{ ContentType: "text/plain; charset=utf-8", ContentDisposition: `inline; filename=test.txt`, }, nil) - testSingleBlobStorageURLContentTypeAndDisposition(t, s, testFilename, "test.pdf", expectedServeDirectHeaders{ + test(t, objStore, testFilename, "test.pdf", expectedServeDirectHeaders{ ContentType: "application/pdf", ContentDisposition: `inline; filename=test.pdf`, }, nil) - testSingleBlobStorageURLContentTypeAndDisposition(t, s, testFilename, "test.wasm", expectedServeDirectHeaders{ + test(t, objStore, testFilename, "test.wasm", expectedServeDirectHeaders{ ContentDisposition: `inline; filename=test.wasm`, }, nil) - testSingleBlobStorageURLContentTypeAndDisposition(t, s, testFilename, "test.wasm", expectedServeDirectHeaders{ + test(t, objStore, testFilename, "test.wasm", expectedServeDirectHeaders{ ContentType: "application/wasm", ContentDisposition: `inline; filename=test.wasm`, }, &ServeDirectOptions{ ContentType: "application/wasm", }) - assert.NoError(t, s.Delete(testFilename)) + assert.NoError(t, objStore.Delete(testFilename)) +} + +func testStorageGeneral(t *testing.T, objStore ObjectStorage) { + t.Run("StorageIterator", func(t *testing.T) { testStorageIterator(t, objStore) }) + + if _, ok := objStore.(*LocalStorage); ok { + t.Skipf("Skipping tests for local storage") + } + t.Run("StorageURLContentTypeAndDisposition", func(t *testing.T) { testStorageURLContentTypeAndDisposition(t, objStore) }) }