Storage: add WithContents option to storage.Get() (#53105)

* Storage: add `WithContents` option to `storage.Get()`

* fix tests

Co-authored-by: Ryan McKinley <ryantxu@gmail.com>
This commit is contained in:
Artur Wierzbicki
2022-08-30 15:23:16 +02:00
committed by GitHub
parent fc348e6279
commit 7a340f486b
10 changed files with 103 additions and 41 deletions

View File

@@ -166,9 +166,14 @@ type DeleteFolderOptions struct {
AccessFilter PathFilter
}
type GetFileOptions struct {
// WithContents if set to false, the `Get` operation will return just the file metadata. Default is `true`
WithContents bool
}
//go:generate mockery --name FileStorage --structname MockFileStorage --inpackage --filename file_storage_mock.go
type FileStorage interface {
Get(ctx context.Context, path string) (*File, error)
Get(ctx context.Context, path string, options *GetFileOptions) (*File, bool, error)
Delete(ctx context.Context, path string) error
Upsert(ctx context.Context, command *UpsertFileCommand) error

View File

@@ -30,17 +30,27 @@ func NewCdkBlobStorage(log log.Logger, bucket *blob.Bucket, rootFolder string, f
}, filter, rootFolder)
}
func (c cdkBlobStorage) Get(ctx context.Context, filePath string) (*File, error) {
contents, err := c.bucket.ReadAll(ctx, strings.ToLower(filePath))
func (c cdkBlobStorage) Get(ctx context.Context, path string, options *GetFileOptions) (*File, bool, error) {
var err error
var contents []byte
if options.WithContents {
contents, err = c.bucket.ReadAll(ctx, strings.ToLower(path))
if err != nil {
if gcerrors.Code(err) == gcerrors.NotFound {
return nil, false, nil
}
return nil, false, err
}
} else {
contents = make([]byte, 0)
}
attributes, err := c.bucket.Attributes(ctx, strings.ToLower(path))
if err != nil {
if gcerrors.Code(err) == gcerrors.NotFound {
return nil, nil
return nil, false, nil
}
return nil, err
}
attributes, err := c.bucket.Attributes(ctx, strings.ToLower(filePath))
if err != nil {
return nil, err
return nil, false, err
}
var originalPath string
@@ -53,7 +63,7 @@ func (c cdkBlobStorage) Get(ctx context.Context, filePath string) (*File, error)
}
} else {
props = make(map[string]string)
originalPath = filePath
originalPath = path
}
return &File{
@@ -67,7 +77,7 @@ func (c cdkBlobStorage) Get(ctx context.Context, filePath string) (*File, error)
Size: attributes.Size,
MimeType: detectContentType(originalPath, attributes.ContentType),
},
}, nil
}, true, nil
}
func (c cdkBlobStorage) Delete(ctx context.Context, filePath string) error {
@@ -85,7 +95,7 @@ func (c cdkBlobStorage) Delete(ctx context.Context, filePath string) error {
}
func (c cdkBlobStorage) Upsert(ctx context.Context, command *UpsertFileCommand) error {
existing, err := c.Get(ctx, command.Path)
existing, _, err := c.Get(ctx, command.Path, &GetFileOptions{WithContents: true})
if err != nil {
return err
}

View File

@@ -89,16 +89,24 @@ func (s dbFileStorage) getProperties(sess *sqlstore.DBSession, pathHashes []stri
return attributesByPath, nil
}
func (s dbFileStorage) Get(ctx context.Context, filePath string) (*File, error) {
func (s dbFileStorage) Get(ctx context.Context, path string, options *GetFileOptions) (*File, bool, error) {
var result *File
pathHash, err := createPathHash(filePath)
pathHash, err := createPathHash(path)
if err != nil {
return nil, err
return nil, false, err
}
err = s.db.WithDbSession(ctx, func(sess *sqlstore.DBSession) error {
table := &file{}
exists, err := sess.Table("file").Where("path_hash = ?", pathHash).Get(table)
sess.Table("file")
if options.WithContents {
sess.Cols(allFileCols...)
} else {
sess.Cols(fileColsNoContents...)
}
exists, err := sess.Where("path_hash = ?", pathHash).Get(table)
if !exists {
return nil
}
@@ -134,7 +142,7 @@ func (s dbFileStorage) Get(ctx context.Context, filePath string) (*File, error)
return err
})
return result, err
return result, result != nil, err
}
func (s dbFileStorage) Delete(ctx context.Context, filePath string) error {

View File

@@ -55,27 +55,34 @@ func (_m *MockFileStorage) DeleteFolder(ctx context.Context, path string, option
return r0
}
// Get provides a mock function with given fields: ctx, path
func (_m *MockFileStorage) Get(ctx context.Context, path string) (*File, error) {
ret := _m.Called(ctx, path)
// Get provides a mock function with given fields: ctx, path, options
func (_m *MockFileStorage) Get(ctx context.Context, path string, options *GetFileOptions) (*File, bool, error) {
ret := _m.Called(ctx, path, options)
var r0 *File
if rf, ok := ret.Get(0).(func(context.Context, string) *File); ok {
r0 = rf(ctx, path)
if rf, ok := ret.Get(0).(func(context.Context, string, *GetFileOptions) *File); ok {
r0 = rf(ctx, path, options)
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).(*File)
}
}
var r1 error
if rf, ok := ret.Get(1).(func(context.Context, string) error); ok {
r1 = rf(ctx, path)
var r1 bool
if rf, ok := ret.Get(1).(func(context.Context, string, *GetFileOptions) bool); ok {
r1 = rf(ctx, path, options)
} else {
r1 = ret.Error(1)
r1 = ret.Get(1).(bool)
}
return r0, r1
var r2 error
if rf, ok := ret.Get(2).(func(context.Context, string, *GetFileOptions) error); ok {
r2 = rf(ctx, path, options)
} else {
r2 = ret.Error(2)
}
return r0, r1, r2
}
// List provides a mock function with given fields: ctx, folderPath, paging, options

View File

@@ -859,6 +859,19 @@ func TestIntegrationFsStorage(t *testing.T) {
fContents(pngImage),
),
},
queryGet{
input: queryGetInput{
path: "/file.png",
options: &GetFileOptions{WithContents: false},
},
checks: checks(
fName("FILE.png"),
fMimeType("image/png"),
fProperties(map[string]string{"a": "av", "b": "bv"}),
fSize(pngImageSize),
fContents(emptyContents),
),
},
},
},
{

View File

@@ -38,7 +38,8 @@ type cmdDeleteFolder struct {
}
type queryGetInput struct {
path string
path string
options *GetFileOptions
}
type fileNameCheck struct {
@@ -270,15 +271,18 @@ func handleQuery(t *testing.T, ctx context.Context, query interface{}, queryName
switch q := query.(type) {
case queryGet:
inputPath := q.input.path
file, err := fs.Get(ctx, inputPath)
options := q.input.options
file, fileFound, err := fs.Get(ctx, inputPath, options)
require.NoError(t, err, "%s: should be able to get file %s", queryName, inputPath)
if q.checks != nil && len(q.checks) > 0 {
require.NotNil(t, file, "%s %s", queryName, inputPath)
require.True(t, fileFound, "%s %s", queryName, inputPath)
require.Equal(t, strings.ToLower(inputPath), strings.ToLower(file.FullPath), "%s %s", queryName, inputPath)
runChecks(t, queryName, inputPath, file, q.checks)
} else {
require.Nil(t, file, "%s %s", queryName, inputPath)
require.False(t, fileFound, "%s %s", queryName, inputPath)
}
case queryListFiles:
inputPath := q.input.path

View File

@@ -114,25 +114,35 @@ func (b wrapper) removeRoot(path string) string {
return Join(Delimiter, strings.TrimPrefix(path, b.rootFolder))
}
func (b wrapper) Get(ctx context.Context, path string) (*File, error) {
func (b wrapper) getOptionsWithDefaults(options *GetFileOptions) *GetFileOptions {
if options == nil {
return &GetFileOptions{WithContents: true}
}
return options
}
func (b wrapper) Get(ctx context.Context, path string, options *GetFileOptions) (*File, bool, error) {
if err := b.validatePath(path); err != nil {
return nil, err
return nil, false, err
}
rootedPath := b.addRoot(path)
if !b.filter.IsAllowed(rootedPath) {
return nil, nil
return nil, false, nil
}
optionsWithDefaults := b.getOptionsWithDefaults(options)
if b.rootFolder == rootedPath {
return nil, nil
return nil, false, nil
}
file, err := b.wrapped.Get(ctx, rootedPath)
file, _, err := b.wrapped.Get(ctx, rootedPath, optionsWithDefaults)
if file != nil {
file.FullPath = b.removeRoot(file.FullPath)
}
return file, err
return file, file != nil, err
}
func (b wrapper) Delete(ctx context.Context, path string) error {
@@ -323,7 +333,11 @@ func (b wrapper) List(ctx context.Context, folderPath string, paging *Paging, op
go func() {
if options.WithFiles {
if f, err := b.Get(fileRetrievalCtx, folderPath); err == nil {
var getOptions *GetFileOptions
if options.WithContents {
getOptions = &GetFileOptions{WithContents: true}
}
if f, _, err := b.Get(fileRetrievalCtx, folderPath, getOptions); err == nil {
fileChan <- f
return
}

View File

@@ -334,7 +334,7 @@ func (s *standardStorageService) Upload(ctx context.Context, user *user.SignedIn
grafanaStorageLogger.Info("uploading a file", "path", req.Path)
if !req.OverwriteExistingFile {
file, err := root.Store().Get(ctx, storagePath)
file, _, err := root.Store().Get(ctx, storagePath, &filestorage.GetFileOptions{WithContents: false})
if err != nil {
grafanaStorageLogger.Error("failed while checking file existence", "err", err, "path", req.Path)
return ErrUploadInternalError

View File

@@ -128,7 +128,7 @@ func TestShouldUploadWhenNoFileAlreadyExists(t *testing.T) {
service, mockStorage, storageName := setupUploadStore(t, nil)
fileName := "/myFile.jpg"
mockStorage.On("Get", mock.Anything, fileName).Return(nil, nil)
mockStorage.On("Get", mock.Anything, fileName, &filestorage.GetFileOptions{WithContents: false}).Return(nil, false, nil)
mockStorage.On("Upsert", mock.Anything, &filestorage.UpsertFileCommand{
Path: fileName,
MimeType: "image/jpeg",
@@ -157,7 +157,7 @@ func TestShouldFailUploadWithoutAccess(t *testing.T) {
func TestShouldFailUploadWhenFileAlreadyExists(t *testing.T) {
service, mockStorage, storageName := setupUploadStore(t, nil)
mockStorage.On("Get", mock.Anything, "/myFile.jpg").Return(&filestorage.File{Contents: make([]byte, 0)}, nil)
mockStorage.On("Get", mock.Anything, "/myFile.jpg", &filestorage.GetFileOptions{WithContents: false}).Return(&filestorage.File{Contents: make([]byte, 0)}, true, nil)
err := service.Upload(context.Background(), dummyUser, &UploadRequest{
EntityType: EntityTypeImage,
@@ -213,7 +213,7 @@ func TestShouldUploadSvg(t *testing.T) {
service, mockStorage, storageName := setupUploadStore(t, nil)
fileName := "/myFile.svg"
mockStorage.On("Get", mock.Anything, fileName).Return(nil, nil)
mockStorage.On("Get", mock.Anything, fileName, &filestorage.GetFileOptions{WithContents: false}).Return(nil, false, nil)
mockStorage.On("Upsert", mock.Anything, &filestorage.UpsertFileCommand{
Path: fileName,
MimeType: "image/svg+xml",

View File

@@ -86,7 +86,8 @@ func (t *nestedTree) GetFile(ctx context.Context, orgId int64, path string) (*fi
if store == nil {
return nil, fmt.Errorf("store not ready")
}
return store.Get(ctx, path)
file, _, err := store.Get(ctx, path, nil)
return file, err
}
func (t *nestedTree) getStorages(orgId int64) []storageRuntime {