diff --git a/internal/storage/caching/fast_stat_bucket.go b/internal/storage/caching/fast_stat_bucket.go index c8b6597b2f..e78834d631 100644 --- a/internal/storage/caching/fast_stat_bucket.go +++ b/internal/storage/caching/fast_stat_bucket.go @@ -115,6 +115,14 @@ func (b *fastStatBucket) lookUp(name string) (hit bool, m *gcs.MinObject) { return } +func (b *fastStatBucket) lookUpFolder(name string) (bool, *gcs.Folder) { + b.mu.Lock() + defer b.mu.Unlock() + + hit, f := b.cache.LookUpFolder(name, b.clock.Now()) + return hit, f +} + //////////////////////////////////////////////////////////////////////// // Bucket interface //////////////////////////////////////////////////////////////////////// @@ -298,17 +306,23 @@ func (b *fastStatBucket) StatObjectFromGcs(ctx context.Context, func (b *fastStatBucket) GetFolder( ctx context.Context, - prefix string) (folder *gcs.Folder, err error) { - // Fetch the listing. - folder, err = b.wrapped.GetFolder(ctx, prefix) - if err != nil { - return - } + prefix string) (*gcs.Folder, error) { - // TODO: add folder metadata in stat cache - // b.insertFolder(folder) + if hit, entry := b.lookUpFolder(prefix); hit { + // Negative entries result in NotFoundError. + if entry == nil { + err := &gcs.NotFoundError{ + Err: fmt.Errorf("negative cache entry for folder %v", prefix), + } - return + return nil, err + } + + return entry, nil + } + + // Fetch the Folder + return b.wrapped.GetFolder(ctx, prefix) } func (b *fastStatBucket) RenameFolder(ctx context.Context, folderName string, destinationFolderId string) (o *control.RenameFolderOperation, err error) { diff --git a/internal/storage/caching/fast_stat_bucket_test.go b/internal/storage/caching/fast_stat_bucket_test.go index e1639fd28f..c50d539081 100644 --- a/internal/storage/caching/fast_stat_bucket_test.go +++ b/internal/storage/caching/fast_stat_bucket_test.go @@ -725,3 +725,46 @@ func (t *DeleteObjectTest) WrappedSucceeds() { err = t.deleteObject(name) AssertEq(nil, err) } + +func (t *StatObjectTest) TestShouldReturnFromCacheWhenEntryIsPresent() { + const name = "some-name" + folder := &gcs.Folder{ + Name: name, + } + ExpectCall(t.cache, "LookUpFolder")(name, Any()). + WillOnce(Return(true, folder)) + + result, err := t.bucket.GetFolder(context.TODO(), name) + + AssertEq(nil, err) + ExpectThat(result, Pointee(DeepEquals(*folder))) +} + +func (t *StatObjectTest) TestShouldReturnNotFoundErrorWhenNilEntryIsReturned() { + const name = "some-name" + + ExpectCall(t.cache, "LookUpFolder")(name, Any()). + WillOnce(Return(true, nil)) + + result, err := t.bucket.GetFolder(context.TODO(), name) + + ExpectThat(err, HasSameTypeAs(&gcs.NotFoundError{})) + AssertEq(nil, result) +} + +func (t *StatObjectTest) TestShouldCallGetFolderWhenEntryIsNotPresent() { + const name = "some-name" + folder := &gcs.Folder{ + Name: name, + } + + ExpectCall(t.cache, "LookUpFolder")(name, Any()). + WillOnce(Return(false, nil)) + ExpectCall(t.wrapped, "GetFolder")(Any(), name). + WillOnce(Return(folder, nil)) + + result, err := t.bucket.GetFolder(context.TODO(), name) + + AssertEq(nil, err) + ExpectThat(result, Pointee(DeepEquals(*folder))) +} diff --git a/internal/storage/mock_bucket.go b/internal/storage/mock_bucket.go index 7faa630995..3124eb53f7 100644 --- a/internal/storage/mock_bucket.go +++ b/internal/storage/mock_bucket.go @@ -359,7 +359,7 @@ func (m *mockBucket) GetFolder( "GetFolder", file, line, - []interface{}{}) + []interface{}{ctx, prefix}) if len(retVals) != 2 { panic(fmt.Sprintf("mockBucket.GetFolder: invalid return values: %v", retVals))