From fd08d6cd5d5b4523dcdb6c4100502fdf34494382 Mon Sep 17 00:00:00 2001
From: Ankita Luthra <lankita@google.com>
Date: Wed, 26 Jun 2024 08:13:14 +0000
Subject: [PATCH 01/17] running code cov with new token


From 823b6a44f73b41716ae8627670afd556ee3065c9 Mon Sep 17 00:00:00 2001
From: Ankita Luthra <lankita@google.com>
Date: Thu, 27 Jun 2024 08:52:37 +0000
Subject: [PATCH 02/17] adds insert folder in stat cache

---
 internal/cache/metadata/stat_cache.go | 34 +++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/internal/cache/metadata/stat_cache.go b/internal/cache/metadata/stat_cache.go
index cbc4b30920..596a6ce42c 100644
--- a/internal/cache/metadata/stat_cache.go
+++ b/internal/cache/metadata/stat_cache.go
@@ -18,6 +18,7 @@ import (
 	"math"
 	"time"
 
+	"cloud.google.com/go/storage/control/apiv2/controlpb"
 	"github.com/googlecloudplatform/gcsfuse/v2/internal/cache/lru"
 	"github.com/googlecloudplatform/gcsfuse/v2/internal/storage/gcs"
 	"github.com/googlecloudplatform/gcsfuse/v2/internal/util"
@@ -49,6 +50,15 @@ type StatCache interface {
 	// entry. Return hit == false when there is neither a positive nor a negative
 	// entry, or the entry has expired according to the supplied current time.
 	LookUp(name string, now time.Time) (hit bool, m *gcs.MinObject)
+
+	// Insert an entry for the given folder resource.
+	//
+	// In order to help cope with caching of arbitrarily out of date (i.e.
+	// inconsistent) object listings, entry will not replace any positive entry
+	// with a newer meta generation number.
+	//
+	// The entry will expire after the supplied time.
+	InsertFolder(f *controlpb.Folder, expiration time.Time)
 }
 
 // Create a new bucket-view to the passed shared-cache object.
@@ -80,6 +90,7 @@ type statCacheBucketView struct {
 // entry. Nil object means negative entry.
 type entry struct {
 	m          *gcs.MinObject
+	f          *controlpb.Folder
 	expiration time.Time
 	key        string
 }
@@ -203,3 +214,26 @@ func (sc *statCacheBucketView) LookUp(
 
 	return
 }
+
+func (sc *statCacheBucketView) InsertFolder(f *controlpb.Folder, expiration time.Time) {
+	name := sc.key(f.Name)
+
+	// Return if there is already a better entry?
+	existing := sc.sharedCache.LookUp(name)
+	if existing != nil && existing.(entry).f != nil {
+		existingFolder := existing.(entry).f
+		if f.Metageneration != existingFolder.Metageneration && f.Metageneration < existingFolder.Metageneration {
+			return
+		}
+	}
+
+	e := entry{
+		f:          f,
+		expiration: expiration,
+		key:        name,
+	}
+
+	if _, err := sc.sharedCache.Insert(name, e); err != nil {
+		panic(err)
+	}
+}

From 40d89a685749c4178ddca0c9b448710f43a4c1ef Mon Sep 17 00:00:00 2001
From: Ankita Luthra <lankita@google.com>
Date: Thu, 27 Jun 2024 11:30:16 +0000
Subject: [PATCH 03/17] adds Get folder impl in stat cache

---
 internal/storage/caching/fast_stat_bucket.go | 28 +++++++++++++++-----
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/internal/storage/caching/fast_stat_bucket.go b/internal/storage/caching/fast_stat_bucket.go
index b82a00aac4..d1740b519f 100644
--- a/internal/storage/caching/fast_stat_bucket.go
+++ b/internal/storage/caching/fast_stat_bucket.go
@@ -298,15 +298,29 @@ func (b *fastStatBucket) StatObjectFromGcs(ctx context.Context,
 
 func (b *fastStatBucket) GetFolder(
 	ctx context.Context,
-	prefix string) (folder *controlpb.Folder, err error) {
-	// Fetch the listing.
-	folder, err = b.wrapped.GetFolder(ctx, prefix)
+	prefix string) (*controlpb.Folder, error) {
+	// Do we have an entry in the cache?
+	if hit, entry := b.lookUp(prefix); hit {
+		// Negative entries result in NotFoundError.
+		if entry == nil {
+			err := &gcs.NotFoundError{
+				Err: fmt.Errorf("Negative cache entry for %v", prefix),
+			}
+			return nil, err
+		}
+
+		// TODO: return folder from here, create new lookup for folder
+		return nil, nil
+	}
+
+	folder, err := b.wrapped.GetFolder(ctx, prefix)
 	if err != nil {
-		return
+		return nil, err
 	}
 
-	// TODO: add folder metadata in stat cache
-	// b.insertFolder(folder)
+	// Add folder metadata in stat cache
+	expiration := b.clock.Now().Add(b.ttl)
+	b.cache.InsertFolder(folder, expiration)
 
-	return
+	return folder, nil
 }

From deb4ca4dde425bc01604219f7466525b421cdf6b Mon Sep 17 00:00:00 2001
From: Ankita Luthra <lankita@google.com>
Date: Wed, 3 Jul 2024 15:00:17 +0000
Subject: [PATCH 04/17] adds lookup for folder

---
 internal/cache/metadata/stat_cache.go      | 40 ++++++++++++++++------
 internal/cache/metadata/stat_cache_test.go | 18 +++++++++-
 2 files changed, 47 insertions(+), 11 deletions(-)

diff --git a/internal/cache/metadata/stat_cache.go b/internal/cache/metadata/stat_cache.go
index 596a6ce42c..a6b347dc79 100644
--- a/internal/cache/metadata/stat_cache.go
+++ b/internal/cache/metadata/stat_cache.go
@@ -65,19 +65,19 @@ type StatCache interface {
 // For dynamic-mount (mount for multiple buckets), pass bn as bucket-name.
 // For static-mout (mount for single bucket), pass bn as "".
 func NewStatCacheBucketView(sc *lru.Cache, bn string) StatCache {
-	return &statCacheBucketView{
+	return &StatCacheBucketView{
 		sharedCache: sc,
 		bucketName:  bn,
 	}
 }
 
-// statCacheBucketView is a special type of StatCache which
+// StatCacheBucketView is a special type of StatCache which
 // shares its underlying cache map object with other
-// statCacheBucketView objects (for dynamically mounts) through
+// StatCacheBucketView objects (for dynamically mounts) through
 // a specific bucket-name. It does so by prepending its
 // bucket-name to its entry keys to make them unique
 // to it.
-type statCacheBucketView struct {
+type StatCacheBucketView struct {
 	sharedCache *lru.Cache
 	// bucketName is the unique identifier for this
 	// statCache object among all statCache objects
@@ -139,7 +139,7 @@ func shouldReplace(m *gcs.MinObject, existing entry) bool {
 	return true
 }
 
-func (sc *statCacheBucketView) key(objectName string) string {
+func (sc *StatCacheBucketView) key(objectName string) string {
 	// path.Join(sc.bucketName, objectName) does not work
 	// because that normalizes the trailing "/"
 	// which breaks functionality by removing
@@ -150,7 +150,7 @@ func (sc *statCacheBucketView) key(objectName string) string {
 	return objectName
 }
 
-func (sc *statCacheBucketView) Insert(m *gcs.MinObject, expiration time.Time) {
+func (sc *StatCacheBucketView) Insert(m *gcs.MinObject, expiration time.Time) {
 	name := sc.key(m.Name)
 
 	// Is there already a better entry?
@@ -172,7 +172,7 @@ func (sc *statCacheBucketView) Insert(m *gcs.MinObject, expiration time.Time) {
 	}
 }
 
-func (sc *statCacheBucketView) AddNegativeEntry(objectName string, expiration time.Time) {
+func (sc *StatCacheBucketView) AddNegativeEntry(objectName string, expiration time.Time) {
 	name := sc.key(objectName)
 
 	// Insert a negative entry.
@@ -187,12 +187,12 @@ func (sc *statCacheBucketView) AddNegativeEntry(objectName string, expiration ti
 	}
 }
 
-func (sc *statCacheBucketView) Erase(objectName string) {
+func (sc *StatCacheBucketView) Erase(objectName string) {
 	name := sc.key(objectName)
 	sc.sharedCache.Erase(name)
 }
 
-func (sc *statCacheBucketView) LookUp(
+func (sc *StatCacheBucketView) LookUp(
 	objectName string,
 	now time.Time) (hit bool, m *gcs.MinObject) {
 	// Look up in the LRU cache.
@@ -215,7 +215,27 @@ func (sc *statCacheBucketView) LookUp(
 	return
 }
 
-func (sc *statCacheBucketView) InsertFolder(f *controlpb.Folder, expiration time.Time) {
+func (sc *StatCacheBucketView) LookUpFolder(
+	folderName string,
+	now time.Time) (bool, *controlpb.Folder) {
+	// Look up in the LRU cache.
+	value := sc.sharedCache.LookUp(sc.key(folderName))
+	if value == nil {
+		return false, nil
+	}
+
+	e := value.(entry)
+
+	// Has this entry expired?
+	if e.expiration.Before(now) {
+		sc.Erase(folderName)
+		return false, nil
+	}
+
+	return true, e.f
+}
+
+func (sc *StatCacheBucketView) InsertFolder(f *controlpb.Folder, expiration time.Time) {
 	name := sc.key(f.Name)
 
 	// Return if there is already a better entry?
diff --git a/internal/cache/metadata/stat_cache_test.go b/internal/cache/metadata/stat_cache_test.go
index fe0a8ee492..c68737b535 100644
--- a/internal/cache/metadata/stat_cache_test.go
+++ b/internal/cache/metadata/stat_cache_test.go
@@ -18,6 +18,7 @@ import (
 	"testing"
 	"time"
 
+	"cloud.google.com/go/storage/control/apiv2/controlpb"
 	"github.com/googlecloudplatform/gcsfuse/v2/internal/cache/lru"
 	"github.com/googlecloudplatform/gcsfuse/v2/internal/cache/metadata"
 	"github.com/googlecloudplatform/gcsfuse/v2/internal/mount"
@@ -100,7 +101,8 @@ var expiration = someTime.Add(time.Second)
 
 type StatCacheTest struct {
 	suite.Suite
-	cache testHelperCache
+	cache               testHelperCache
+	statCacheBucketView metadata.StatCacheBucketView
 }
 
 type MultiBucketStatCacheTest struct {
@@ -408,3 +410,17 @@ func (t *MultiBucketStatCacheTest) Test_ExpiresLeastRecentlyUsed() {
 	assert.Equal(t.T(), cardamom, spices.LookUpOrNil("cardamom", someTime))
 	assert.Equal(t.T(), saffron, spices.LookUpOrNil("saffron", someTime))
 }
+
+func (t *StatCacheTest) Test_Create_Entry_When_No_Entry_Is_Present() {
+	const name = "key1"
+	newEntry := &controlpb.Folder{
+		Name:           name,
+		Metageneration: 1,
+	}
+
+	t.statCacheBucketView.InsertFolder(newEntry, expiration)
+
+	hit, entry := t.statCacheBucketView.LookUpFolder(name, someTime)
+	assert.True(t.T(), hit)
+	assert.Equal(t.T(), "test", entry.Name)
+}

From 3091a89bebbc585c3cce1018ae3aee68c80c27a8 Mon Sep 17 00:00:00 2001
From: Ankita Luthra <lankita@google.com>
Date: Thu, 4 Jul 2024 06:37:12 +0000
Subject: [PATCH 05/17] adds unit tests for new methods

---
 internal/cache/metadata/stat_cache.go        | 29 +++++----
 internal/cache/metadata/stat_cache_test.go   | 66 ++++++++++++++++++--
 internal/storage/caching/fast_stat_bucket.go | 28 +++------
 3 files changed, 85 insertions(+), 38 deletions(-)

diff --git a/internal/cache/metadata/stat_cache.go b/internal/cache/metadata/stat_cache.go
index a6b347dc79..c4720b2030 100644
--- a/internal/cache/metadata/stat_cache.go
+++ b/internal/cache/metadata/stat_cache.go
@@ -46,7 +46,7 @@ type StatCache interface {
 	// Erase the entry for the given object name, if any.
 	Erase(name string)
 
-	// Return the current entry for the given name, or nil if there is a negative
+	// Return the current object entry for the given name, or nil if there is a negative
 	// entry. Return hit == false when there is neither a positive nor a negative
 	// entry, or the entry has expired according to the supplied current time.
 	LookUp(name string, now time.Time) (hit bool, m *gcs.MinObject)
@@ -59,25 +59,30 @@ type StatCache interface {
 	//
 	// The entry will expire after the supplied time.
 	InsertFolder(f *controlpb.Folder, expiration time.Time)
+
+	// Return the current folder entry for the given name, or nil if there is a negative
+	// entry. Return hit == false when there is neither a positive nor a negative
+	// entry, or the entry has expired according to the supplied current time.
+	LookUpFolder(folderName string, now time.Time) (bool, *controlpb.Folder)
 }
 
 // Create a new bucket-view to the passed shared-cache object.
 // For dynamic-mount (mount for multiple buckets), pass bn as bucket-name.
 // For static-mout (mount for single bucket), pass bn as "".
 func NewStatCacheBucketView(sc *lru.Cache, bn string) StatCache {
-	return &StatCacheBucketView{
+	return &statCacheBucketView{
 		sharedCache: sc,
 		bucketName:  bn,
 	}
 }
 
-// StatCacheBucketView is a special type of StatCache which
+// statCacheBucketView is a special type of StatCache which
 // shares its underlying cache map object with other
-// StatCacheBucketView objects (for dynamically mounts) through
+// statCacheBucketView objects (for dynamically mounts) through
 // a specific bucket-name. It does so by prepending its
 // bucket-name to its entry keys to make them unique
 // to it.
-type StatCacheBucketView struct {
+type statCacheBucketView struct {
 	sharedCache *lru.Cache
 	// bucketName is the unique identifier for this
 	// statCache object among all statCache objects
@@ -139,7 +144,7 @@ func shouldReplace(m *gcs.MinObject, existing entry) bool {
 	return true
 }
 
-func (sc *StatCacheBucketView) key(objectName string) string {
+func (sc *statCacheBucketView) key(objectName string) string {
 	// path.Join(sc.bucketName, objectName) does not work
 	// because that normalizes the trailing "/"
 	// which breaks functionality by removing
@@ -150,7 +155,7 @@ func (sc *StatCacheBucketView) key(objectName string) string {
 	return objectName
 }
 
-func (sc *StatCacheBucketView) Insert(m *gcs.MinObject, expiration time.Time) {
+func (sc *statCacheBucketView) Insert(m *gcs.MinObject, expiration time.Time) {
 	name := sc.key(m.Name)
 
 	// Is there already a better entry?
@@ -172,7 +177,7 @@ func (sc *StatCacheBucketView) Insert(m *gcs.MinObject, expiration time.Time) {
 	}
 }
 
-func (sc *StatCacheBucketView) AddNegativeEntry(objectName string, expiration time.Time) {
+func (sc *statCacheBucketView) AddNegativeEntry(objectName string, expiration time.Time) {
 	name := sc.key(objectName)
 
 	// Insert a negative entry.
@@ -187,12 +192,12 @@ func (sc *StatCacheBucketView) AddNegativeEntry(objectName string, expiration ti
 	}
 }
 
-func (sc *StatCacheBucketView) Erase(objectName string) {
+func (sc *statCacheBucketView) Erase(objectName string) {
 	name := sc.key(objectName)
 	sc.sharedCache.Erase(name)
 }
 
-func (sc *StatCacheBucketView) LookUp(
+func (sc *statCacheBucketView) LookUp(
 	objectName string,
 	now time.Time) (hit bool, m *gcs.MinObject) {
 	// Look up in the LRU cache.
@@ -215,7 +220,7 @@ func (sc *StatCacheBucketView) LookUp(
 	return
 }
 
-func (sc *StatCacheBucketView) LookUpFolder(
+func (sc *statCacheBucketView) LookUpFolder(
 	folderName string,
 	now time.Time) (bool, *controlpb.Folder) {
 	// Look up in the LRU cache.
@@ -235,7 +240,7 @@ func (sc *StatCacheBucketView) LookUpFolder(
 	return true, e.f
 }
 
-func (sc *StatCacheBucketView) InsertFolder(f *controlpb.Folder, expiration time.Time) {
+func (sc *statCacheBucketView) InsertFolder(f *controlpb.Folder, expiration time.Time) {
 	name := sc.key(f.Name)
 
 	// Return if there is already a better entry?
diff --git a/internal/cache/metadata/stat_cache_test.go b/internal/cache/metadata/stat_cache_test.go
index c68737b535..c52198ab5e 100644
--- a/internal/cache/metadata/stat_cache_test.go
+++ b/internal/cache/metadata/stat_cache_test.go
@@ -101,8 +101,8 @@ var expiration = someTime.Add(time.Second)
 
 type StatCacheTest struct {
 	suite.Suite
-	cache               testHelperCache
-	statCacheBucketView metadata.StatCacheBucketView
+	cache     testHelperCache
+	statCache metadata.StatCache
 }
 
 type MultiBucketStatCacheTest struct {
@@ -113,6 +113,7 @@ type MultiBucketStatCacheTest struct {
 func (t *StatCacheTest) SetupTest() {
 	cache := lru.NewCache(uint64((mount.AverageSizeOfPositiveStatCacheEntry + mount.AverageSizeOfNegativeStatCacheEntry) * capacity))
 	t.cache.wrapped = metadata.NewStatCacheBucketView(cache, "") // this demonstrates
+	t.statCache = metadata.NewStatCacheBucketView(cache, "")     // this demonstrates
 	// that if you are using a cache for a single bucket, then
 	// its prepending bucketName can be left empty("") without any problem.
 }
@@ -418,9 +419,64 @@ func (t *StatCacheTest) Test_Create_Entry_When_No_Entry_Is_Present() {
 		Metageneration: 1,
 	}
 
-	t.statCacheBucketView.InsertFolder(newEntry, expiration)
+	t.statCache.InsertFolder(newEntry, expiration)
 
-	hit, entry := t.statCacheBucketView.LookUpFolder(name, someTime)
+	hit, entry := t.statCache.LookUpFolder(name, someTime)
 	assert.True(t.T(), hit)
-	assert.Equal(t.T(), "test", entry.Name)
+	assert.Equal(t.T(), "key1", entry.Name)
+	assert.Equal(t.T(), int64(1), entry.Metageneration)
+}
+
+func (t *StatCacheTest) Test_Override_Entry_Old_Entry_Is_Already_Present() {
+	const name = "key1"
+	existingEntry := &controlpb.Folder{
+		Name:           name,
+		Metageneration: 1,
+	}
+	t.statCache.InsertFolder(existingEntry, expiration)
+	newEntry := &controlpb.Folder{
+		Name:           name,
+		Metageneration: 2,
+	}
+
+	t.statCache.InsertFolder(newEntry, expiration)
+
+	hit, entry := t.statCache.LookUpFolder(name, someTime)
+	assert.True(t.T(), hit)
+	assert.Equal(t.T(), "key1", entry.Name)
+	assert.Equal(t.T(), int64(2), entry.Metageneration)
+}
+
+func (t *StatCacheTest) Test_Lookup_Return_False_If_Expiration_Is_Passed() {
+	const name = "key1"
+	entry := &controlpb.Folder{
+		Name:           name,
+		Metageneration: 1,
+	}
+	t.statCache.InsertFolder(entry, expiration)
+
+	hit, result := t.statCache.LookUpFolder(name, expiration.Add(time.Second))
+
+	assert.False(t.T(), hit)
+	assert.Nil(t.T(), result)
+}
+
+func (t *StatCacheTest) Test_Should_Not_Override_Entry_If_Metageneration_Is_Old() {
+	const name = "key1"
+	existingEntry := &controlpb.Folder{
+		Name:           name,
+		Metageneration: 2,
+	}
+	t.statCache.InsertFolder(existingEntry, expiration)
+	newEntry := &controlpb.Folder{
+		Name:           name,
+		Metageneration: 1,
+	}
+
+	t.statCache.InsertFolder(newEntry, expiration)
+
+	hit, entry := t.statCache.LookUpFolder(name, someTime)
+	assert.True(t.T(), hit)
+	assert.Equal(t.T(), "key1", entry.Name)
+	assert.Equal(t.T(), int64(2), entry.Metageneration)
 }
diff --git a/internal/storage/caching/fast_stat_bucket.go b/internal/storage/caching/fast_stat_bucket.go
index d1740b519f..b82a00aac4 100644
--- a/internal/storage/caching/fast_stat_bucket.go
+++ b/internal/storage/caching/fast_stat_bucket.go
@@ -298,29 +298,15 @@ func (b *fastStatBucket) StatObjectFromGcs(ctx context.Context,
 
 func (b *fastStatBucket) GetFolder(
 	ctx context.Context,
-	prefix string) (*controlpb.Folder, error) {
-	// Do we have an entry in the cache?
-	if hit, entry := b.lookUp(prefix); hit {
-		// Negative entries result in NotFoundError.
-		if entry == nil {
-			err := &gcs.NotFoundError{
-				Err: fmt.Errorf("Negative cache entry for %v", prefix),
-			}
-			return nil, err
-		}
-
-		// TODO: return folder from here, create new lookup for folder
-		return nil, nil
-	}
-
-	folder, err := b.wrapped.GetFolder(ctx, prefix)
+	prefix string) (folder *controlpb.Folder, err error) {
+	// Fetch the listing.
+	folder, err = b.wrapped.GetFolder(ctx, prefix)
 	if err != nil {
-		return nil, err
+		return
 	}
 
-	// Add folder metadata in stat cache
-	expiration := b.clock.Now().Add(b.ttl)
-	b.cache.InsertFolder(folder, expiration)
+	// TODO: add folder metadata in stat cache
+	// b.insertFolder(folder)
 
-	return folder, nil
+	return
 }

From 821011433d89a0110731f863c0e883f760203997 Mon Sep 17 00:00:00 2001
From: Ankita Luthra <lankita@google.com>
Date: Thu, 4 Jul 2024 09:35:36 +0000
Subject: [PATCH 06/17] refactors lookup

---
 internal/cache/metadata/stat_cache.go | 44 +++++++++++++--------------
 1 file changed, 21 insertions(+), 23 deletions(-)

diff --git a/internal/cache/metadata/stat_cache.go b/internal/cache/metadata/stat_cache.go
index c4720b2030..a9d7a0fb6e 100644
--- a/internal/cache/metadata/stat_cache.go
+++ b/internal/cache/metadata/stat_cache.go
@@ -198,46 +198,44 @@ func (sc *statCacheBucketView) Erase(objectName string) {
 }
 
 func (sc *statCacheBucketView) LookUp(
-	objectName string,
-	now time.Time) (hit bool, m *gcs.MinObject) {
+		objectName string,
+		now time.Time) (bool, *gcs.MinObject) {
 	// Look up in the LRU cache.
-	value := sc.sharedCache.LookUp(sc.key(objectName))
-	if value == nil {
-		return
+	entry := sc.lruCacheLookup(objectName, now)
+	if entry == nil {
+		return false, nil
 	}
 
-	e := value.(entry)
+	return true, entry.m
+}
 
-	// Has this entry expired?
-	if e.expiration.Before(now) {
-		sc.Erase(objectName)
-		return
+func (sc *statCacheBucketView) LookUpFolder(
+		folderName string,
+		now time.Time) (bool, *controlpb.Folder) {
+	// Look up in the LRU cache.
+	entry := sc.lruCacheLookup(folderName, now)
+	if entry == nil {
+		return false, nil
 	}
 
-	hit = true
-	m = e.m
-
-	return
+	return true, entry.f
 }
 
-func (sc *statCacheBucketView) LookUpFolder(
-	folderName string,
-	now time.Time) (bool, *controlpb.Folder) {
-	// Look up in the LRU cache.
-	value := sc.sharedCache.LookUp(sc.key(folderName))
+func (sc *statCacheBucketView) lruCacheLookup(key string, now time.Time) *entry {
+	value := sc.sharedCache.LookUp(sc.key(key))
 	if value == nil {
-		return false, nil
+		return nil
 	}
 
 	e := value.(entry)
 
 	// Has this entry expired?
 	if e.expiration.Before(now) {
-		sc.Erase(folderName)
-		return false, nil
+		sc.Erase(key)
+		return nil
 	}
 
-	return true, e.f
+	return &e
 }
 
 func (sc *statCacheBucketView) InsertFolder(f *controlpb.Folder, expiration time.Time) {

From 8c2c8099ec948093dbee5eb9047fb1209fbca59b Mon Sep 17 00:00:00 2001
From: Ankita Luthra <lankita@google.com>
Date: Thu, 4 Jul 2024 09:41:18 +0000
Subject: [PATCH 07/17] adds missing case

---
 internal/cache/metadata/stat_cache_test.go | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/internal/cache/metadata/stat_cache_test.go b/internal/cache/metadata/stat_cache_test.go
index c52198ab5e..028e3264cb 100644
--- a/internal/cache/metadata/stat_cache_test.go
+++ b/internal/cache/metadata/stat_cache_test.go
@@ -461,6 +461,15 @@ func (t *StatCacheTest) Test_Lookup_Return_False_If_Expiration_Is_Passed() {
 	assert.Nil(t.T(), result)
 }
 
+func (t *StatCacheTest) Test_Lookup_Return_False_When_Is_Not_Present() {
+	const name = "key1"
+
+	hit, result := t.statCache.LookUpFolder(name, expiration.Add(time.Second))
+
+	assert.False(t.T(), hit)
+	assert.Nil(t.T(), result)
+}
+
 func (t *StatCacheTest) Test_Should_Not_Override_Entry_If_Metageneration_Is_Old() {
 	const name = "key1"
 	existingEntry := &controlpb.Folder{

From 5f2eb90e6af87cea77b83f7c54a92012932e7cef Mon Sep 17 00:00:00 2001
From: Ankita Luthra <lankita@google.com>
Date: Thu, 4 Jul 2024 09:47:12 +0000
Subject: [PATCH 08/17] implements mock stat cache

---
 .../mock_gcscaching/mock_stat_cache.go        | 47 +++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/internal/storage/caching/mock_gcscaching/mock_stat_cache.go b/internal/storage/caching/mock_gcscaching/mock_stat_cache.go
index e8b5ec0cf8..48e0a57c08 100644
--- a/internal/storage/caching/mock_gcscaching/mock_stat_cache.go
+++ b/internal/storage/caching/mock_gcscaching/mock_stat_cache.go
@@ -12,6 +12,7 @@ import (
 	time "time"
 	unsafe "unsafe"
 
+	"cloud.google.com/go/storage/control/apiv2/controlpb"
 	"github.com/googlecloudplatform/gcsfuse/v2/internal/cache/metadata"
 	"github.com/googlecloudplatform/gcsfuse/v2/internal/storage/gcs"
 	oglemock "github.com/jacobsa/oglemock"
@@ -123,3 +124,49 @@ func (m *mockStatCache) LookUp(p0 string, p1 time.Time) (o0 bool, o1 *gcs.MinObj
 
 	return
 }
+
+func (m *mockStatCache) InsertFolder(p0 *controlpb.Folder, p1 time.Time) {
+	// Get a file name and line number for the caller.
+	_, file, line, _ := runtime.Caller(1)
+
+	// Hand the call off to the controller, which does most of the work.
+	retVals := m.controller.HandleMethodCall(
+		m,
+		"InsertFolder",
+		file,
+		line,
+		[]interface{}{p0, p1})
+
+	if len(retVals) != 0 {
+		panic(fmt.Sprintf("mockStatCache.InsertFolder: invalid return values: %v", retVals))
+	}
+}
+
+func (m *mockStatCache) LookUpFolder(p0 string, p1 time.Time) (o0 bool, o1 *controlpb.Folder) {
+	// Get a file name and line number for the caller.
+	_, file, line, _ := runtime.Caller(1)
+
+	// Hand the call off to the controller, which does most of the work.
+	retVals := m.controller.HandleMethodCall(
+		m,
+		"LookUpFolder",
+		file,
+		line,
+		[]interface{}{p0, p1})
+
+	if len(retVals) != 2 {
+		panic(fmt.Sprintf("mockStatCache.LookUpFolder: invalid return values: %v", retVals))
+	}
+
+	// o0 bool
+	if retVals[0] != nil {
+		o0 = retVals[0].(bool)
+	}
+
+	// o1 *controlpb.Folder
+	if retVals[1] != nil {
+		o1 = retVals[1].(*controlpb.Folder)
+	}
+
+	return
+}

From dfa53c58e8970a95157085190e2e23c74948b752 Mon Sep 17 00:00:00 2001
From: Ankita Luthra <lankita@google.com>
Date: Thu, 4 Jul 2024 09:48:45 +0000
Subject: [PATCH 09/17] fix lint errors

---
 internal/cache/metadata/stat_cache.go | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/internal/cache/metadata/stat_cache.go b/internal/cache/metadata/stat_cache.go
index a9d7a0fb6e..577c8e9b00 100644
--- a/internal/cache/metadata/stat_cache.go
+++ b/internal/cache/metadata/stat_cache.go
@@ -198,8 +198,8 @@ func (sc *statCacheBucketView) Erase(objectName string) {
 }
 
 func (sc *statCacheBucketView) LookUp(
-		objectName string,
-		now time.Time) (bool, *gcs.MinObject) {
+	objectName string,
+	now time.Time) (bool, *gcs.MinObject) {
 	// Look up in the LRU cache.
 	entry := sc.lruCacheLookup(objectName, now)
 	if entry == nil {
@@ -210,8 +210,8 @@ func (sc *statCacheBucketView) LookUp(
 }
 
 func (sc *statCacheBucketView) LookUpFolder(
-		folderName string,
-		now time.Time) (bool, *controlpb.Folder) {
+	folderName string,
+	now time.Time) (bool, *controlpb.Folder) {
 	// Look up in the LRU cache.
 	entry := sc.lruCacheLookup(folderName, now)
 	if entry == nil {

From 114331d77e5dabd6b71e92a38f1c82f99079ff59 Mon Sep 17 00:00:00 2001
From: Ankita Luthra <lankita@google.com>
Date: Fri, 5 Jul 2024 05:11:07 +0000
Subject: [PATCH 10/17] updates mock stat cache with new implementation

---
 internal/cache/metadata/stat_cache.go         | 24 +++++++++++++++++--
 internal/cache/metadata/stat_cache_test.go    | 15 ++++++++++++
 .../mock_gcscaching/mock_stat_cache.go        | 17 +++++++++++++
 3 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/internal/cache/metadata/stat_cache.go b/internal/cache/metadata/stat_cache.go
index 577c8e9b00..ba1a7bda6f 100644
--- a/internal/cache/metadata/stat_cache.go
+++ b/internal/cache/metadata/stat_cache.go
@@ -64,6 +64,11 @@ type StatCache interface {
 	// entry. Return hit == false when there is neither a positive nor a negative
 	// entry, or the entry has expired according to the supplied current time.
 	LookUpFolder(folderName string, now time.Time) (bool, *controlpb.Folder)
+
+	// Set up a negative entry for the given folder name, indicating that the name
+	// doesn't exist. Overwrite any existing entry for the name, positive or
+	// negative.
+	AddNegativeEntryForFolder(folderName string, expiration time.Time)
 }
 
 // Create a new bucket-view to the passed shared-cache object.
@@ -108,10 +113,10 @@ type entry struct {
 // benchmark runs) to heap-size per positive stat-cache entry
 // to calculate a size closer to the actual memory utilization.
 func (e entry) Size() (size uint64) {
-	// First, calculate size on heap.
+	// First, calculate size on heap (including folder size also in case of hns buckets, in case of non-hns buckets 0 will be added as e.f will be Nil ).
 	// Additional 2*util.UnsafeSizeOf(&e.key) is to account for the copies of string
 	// struct stored in the cache map and in the cache linked-list.
-	size = uint64(util.UnsafeSizeOf(&e) + len(e.key) + 2*util.UnsafeSizeOf(&e.key) + util.NestedSizeOfGcsMinObject(e.m))
+	size = uint64(util.UnsafeSizeOf(&e) + len(e.key) + 2*util.UnsafeSizeOf(&e.key) + util.NestedSizeOfGcsMinObject(e.m) + util.UnsafeSizeOf(&e.f))
 	if e.m != nil {
 		size += 515
 	}
@@ -192,6 +197,21 @@ func (sc *statCacheBucketView) AddNegativeEntry(objectName string, expiration ti
 	}
 }
 
+func (sc *statCacheBucketView) AddNegativeEntryForFolder(folderName string, expiration time.Time) {
+	name := sc.key(folderName)
+
+	// Insert a negative entry.
+	e := entry{
+		f:          nil,
+		expiration: expiration,
+		key:        name,
+	}
+
+	if _, err := sc.sharedCache.Insert(name, e); err != nil {
+		panic(err)
+	}
+}
+
 func (sc *statCacheBucketView) Erase(objectName string) {
 	name := sc.key(objectName)
 	sc.sharedCache.Erase(name)
diff --git a/internal/cache/metadata/stat_cache_test.go b/internal/cache/metadata/stat_cache_test.go
index 028e3264cb..690cef6e2b 100644
--- a/internal/cache/metadata/stat_cache_test.go
+++ b/internal/cache/metadata/stat_cache_test.go
@@ -489,3 +489,18 @@ func (t *StatCacheTest) Test_Should_Not_Override_Entry_If_Metageneration_Is_Old(
 	assert.Equal(t.T(), "key1", entry.Name)
 	assert.Equal(t.T(), int64(2), entry.Metageneration)
 }
+
+func (t *StatCacheTest) Test_Should_Add_Negative_Entry_For_Folder() {
+	const name = "key1"
+	existingEntry := &controlpb.Folder{
+		Name:           name,
+		Metageneration: 2,
+	}
+	t.statCache.InsertFolder(existingEntry, expiration)
+
+	t.statCache.AddNegativeEntryForFolder(name, expiration)
+
+	hit, entry := t.statCache.LookUpFolder(name, someTime)
+	assert.True(t.T(), hit)
+	assert.Nil(t.T(), entry)
+}
diff --git a/internal/storage/caching/mock_gcscaching/mock_stat_cache.go b/internal/storage/caching/mock_gcscaching/mock_stat_cache.go
index 48e0a57c08..dbd42cf2fb 100644
--- a/internal/storage/caching/mock_gcscaching/mock_stat_cache.go
+++ b/internal/storage/caching/mock_gcscaching/mock_stat_cache.go
@@ -28,6 +28,23 @@ type mockStatCache struct {
 	description string
 }
 
+func (m *mockStatCache) AddNegativeEntryForFolder(p0 string, p1 time.Time) {
+	// Get a folder name and line number for the caller.
+	_, name, line, _ := runtime.Caller(1)
+
+	// Hand the call off to the controller, which does most of the work.
+	retVals := m.controller.HandleMethodCall(
+		m,
+		"AddNegativeEntryForFolder",
+		name,
+		line,
+		[]interface{}{p0, p1})
+
+	if len(retVals) != 0 {
+		panic(fmt.Sprintf("mockStatCache.AddNegativeEntryforFolder: invalid return values: %v", retVals))
+	}
+}
+
 func NewMockStatCache(
 	c oglemock.Controller,
 	desc string) MockStatCache {

From cce825a2e0d0a2fe4f63c240527397f184531665 Mon Sep 17 00:00:00 2001
From: Ankita Luthra <lankita@google.com>
Date: Fri, 5 Jul 2024 05:41:21 +0000
Subject: [PATCH 11/17] fixes capacity test

---
 internal/cache/metadata/stat_cache.go | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/internal/cache/metadata/stat_cache.go b/internal/cache/metadata/stat_cache.go
index ba1a7bda6f..f2dea84823 100644
--- a/internal/cache/metadata/stat_cache.go
+++ b/internal/cache/metadata/stat_cache.go
@@ -116,11 +116,15 @@ func (e entry) Size() (size uint64) {
 	// First, calculate size on heap (including folder size also in case of hns buckets, in case of non-hns buckets 0 will be added as e.f will be Nil ).
 	// Additional 2*util.UnsafeSizeOf(&e.key) is to account for the copies of string
 	// struct stored in the cache map and in the cache linked-list.
-	size = uint64(util.UnsafeSizeOf(&e) + len(e.key) + 2*util.UnsafeSizeOf(&e.key) + util.NestedSizeOfGcsMinObject(e.m) + util.UnsafeSizeOf(&e.f))
+	size = uint64(util.UnsafeSizeOf(&e) + len(e.key) + 2*util.UnsafeSizeOf(&e.key) + util.NestedSizeOfGcsMinObject(e.m))
 	if e.m != nil {
 		size += 515
 	}
 
+	if e.f != nil {
+		size = size + uint64(util.UnsafeSizeOf(&e.f))
+	}
+
 	// Convert heap-size to RSS (resident set size).
 	size = uint64(math.Ceil(util.HeapSizeToRssConversionFactor * float64(size)))
 

From da73c35bb0cef47ea8ca6a9678c00af9581d6d84 Mon Sep 17 00:00:00 2001
From: Ankita Luthra <lankita@google.com>
Date: Fri, 5 Jul 2024 06:11:49 +0000
Subject: [PATCH 12/17] adds test for size

---
 internal/cache/metadata/stat_cache_test.go | 32 ++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/internal/cache/metadata/stat_cache_test.go b/internal/cache/metadata/stat_cache_test.go
index 690cef6e2b..22d6285b80 100644
--- a/internal/cache/metadata/stat_cache_test.go
+++ b/internal/cache/metadata/stat_cache_test.go
@@ -504,3 +504,35 @@ func (t *StatCacheTest) Test_Should_Add_Negative_Entry_For_Folder() {
 	assert.True(t.T(), hit)
 	assert.Nil(t.T(), entry)
 }
+
+func (t *StatCacheTest) Test_Should_Evict_Entry_On_Full_Capacity_Including_Folder_Size() {
+	localCache := lru.NewCache(uint64(3000))
+	t.statCache = metadata.NewStatCacheBucketView(localCache, "local_bucket")
+	objectEntry1 := &gcs.MinObject{Name: "1"}
+	objectEntry2 := &gcs.MinObject{Name: "2"}
+	folderEntry := &controlpb.Folder{
+		Name:           "3",
+		Metageneration: 1,
+	}
+	t.statCache.Insert(objectEntry1, expiration) // adds size of 1428
+	t.statCache.Insert(objectEntry2, expiration) // adds size of 1428
+
+	hit1, entry1 := t.statCache.LookUp("1", someTime)
+	hit2, entry2 := t.statCache.LookUp("2", someTime)
+
+	assert.True(t.T(), hit1)
+	assert.Equal(t.T(), "1", entry1.Name)
+	assert.True(t.T(), hit2)
+	assert.Equal(t.T(), "2", entry2.Name)
+
+	t.statCache.InsertFolder(folderEntry, expiration) //adds size of 220 and exceeds capacity
+
+	hit1, entry1 = t.statCache.LookUp("1", someTime)
+	hit3, entry3 := t.statCache.LookUpFolder("3", someTime)
+
+	assert.False(t.T(), hit1)
+	assert.Nil(t.T(), entry1)
+	assert.True(t.T(), hit3)
+	assert.Equal(t.T(), "3", entry3.Name)
+
+}

From e46ab93c4c5c5e96cff087bf24b8bda6de752acc Mon Sep 17 00:00:00 2001
From: Ankita Luthra <lankita@google.com>
Date: Fri, 5 Jul 2024 07:51:27 +0000
Subject: [PATCH 13/17] adds comment for refactoring in test

---
 internal/cache/metadata/stat_cache_test.go | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/internal/cache/metadata/stat_cache_test.go b/internal/cache/metadata/stat_cache_test.go
index 22d6285b80..8b572f4f35 100644
--- a/internal/cache/metadata/stat_cache_test.go
+++ b/internal/cache/metadata/stat_cache_test.go
@@ -101,7 +101,14 @@ var expiration = someTime.Add(time.Second)
 
 type StatCacheTest struct {
 	suite.Suite
-	cache     testHelperCache
+	cache testHelperCache
+	//t.cache is the wrapper class on top of metadata.StatCache.
+	//This approach tests wrapper methods instead of directly testing actual functionality, compromising the safety net.
+	//For instance: If the helper class changes internally and stops calling stat cache methods, tests won't fail,
+	//hence tests are not being safety net capturing behaviour change of actual functionality.
+	//added stat cache to test metadata.StatCache directly, removing unnecessary wrappers for accurate unit testing.
+	//Changing every test will increase the scope and actual hns work will be affected, so taking cautious call to just add new test in refactored way in first go,
+	//we can update rest of tests slowly later.
 	statCache metadata.StatCache
 }
 

From d22d1bfc16b6260cbf65ee496d2e65bc390cee1e Mon Sep 17 00:00:00 2001
From: Ankita Luthra <lankita@google.com>
Date: Fri, 5 Jul 2024 10:31:19 +0000
Subject: [PATCH 14/17] resolves PR comments

---
 internal/cache/metadata/stat_cache.go      |  8 ++++----
 internal/cache/metadata/stat_cache_test.go | 11 +++++++----
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/internal/cache/metadata/stat_cache.go b/internal/cache/metadata/stat_cache.go
index f2dea84823..68367c9b9a 100644
--- a/internal/cache/metadata/stat_cache.go
+++ b/internal/cache/metadata/stat_cache.go
@@ -122,7 +122,7 @@ func (e entry) Size() (size uint64) {
 	}
 
 	if e.f != nil {
-		size = size + uint64(util.UnsafeSizeOf(&e.f))
+		size += uint64(util.UnsafeSizeOf(&e.f))
 	}
 
 	// Convert heap-size to RSS (resident set size).
@@ -225,7 +225,7 @@ func (sc *statCacheBucketView) LookUp(
 	objectName string,
 	now time.Time) (bool, *gcs.MinObject) {
 	// Look up in the LRU cache.
-	entry := sc.lruCacheLookup(objectName, now)
+	entry := sc.sharedcachelookup(objectName, now)
 	if entry == nil {
 		return false, nil
 	}
@@ -237,7 +237,7 @@ func (sc *statCacheBucketView) LookUpFolder(
 	folderName string,
 	now time.Time) (bool, *controlpb.Folder) {
 	// Look up in the LRU cache.
-	entry := sc.lruCacheLookup(folderName, now)
+	entry := sc.sharedcachelookup(folderName, now)
 	if entry == nil {
 		return false, nil
 	}
@@ -245,7 +245,7 @@ func (sc *statCacheBucketView) LookUpFolder(
 	return true, entry.f
 }
 
-func (sc *statCacheBucketView) lruCacheLookup(key string, now time.Time) *entry {
+func (sc *statCacheBucketView) sharedcachelookup(key string, now time.Time) *entry {
 	value := sc.sharedCache.LookUp(sc.key(key))
 	if value == nil {
 		return nil
diff --git a/internal/cache/metadata/stat_cache_test.go b/internal/cache/metadata/stat_cache_test.go
index 8b572f4f35..062e2b8ae9 100644
--- a/internal/cache/metadata/stat_cache_test.go
+++ b/internal/cache/metadata/stat_cache_test.go
@@ -419,7 +419,7 @@ func (t *MultiBucketStatCacheTest) Test_ExpiresLeastRecentlyUsed() {
 	assert.Equal(t.T(), saffron, spices.LookUpOrNil("saffron", someTime))
 }
 
-func (t *StatCacheTest) Test_Create_Entry_When_No_Entry_Is_Present() {
+func (t *StatCacheTest) Test_InsertFolder_Create_Entry_When_No_Entry_Is_Present() {
 	const name = "key1"
 	newEntry := &controlpb.Folder{
 		Name:           name,
@@ -434,7 +434,7 @@ func (t *StatCacheTest) Test_Create_Entry_When_No_Entry_Is_Present() {
 	assert.Equal(t.T(), int64(1), entry.Metageneration)
 }
 
-func (t *StatCacheTest) Test_Override_Entry_Old_Entry_Is_Already_Present() {
+func (t *StatCacheTest) Test_InsertFolder_Override_Entry_Old_Entry_Is_Already_Present() {
 	const name = "key1"
 	existingEntry := &controlpb.Folder{
 		Name:           name,
@@ -477,7 +477,7 @@ func (t *StatCacheTest) Test_Lookup_Return_False_When_Is_Not_Present() {
 	assert.Nil(t.T(), result)
 }
 
-func (t *StatCacheTest) Test_Should_Not_Override_Entry_If_Metageneration_Is_Old() {
+func (t *StatCacheTest) Test_InsertFolder_Should_Not_Override_Entry_If_Metageneration_Is_Old() {
 	const name = "key1"
 	existingEntry := &controlpb.Folder{
 		Name:           name,
@@ -497,7 +497,7 @@ func (t *StatCacheTest) Test_Should_Not_Override_Entry_If_Metageneration_Is_Old(
 	assert.Equal(t.T(), int64(2), entry.Metageneration)
 }
 
-func (t *StatCacheTest) Test_Should_Add_Negative_Entry_For_Folder() {
+func (t *StatCacheTest) Test_AddNegativeEntryForFolder_Should_Add_Negative_Entry_For_Folder() {
 	const name = "key1"
 	existingEntry := &controlpb.Folder{
 		Name:           name,
@@ -535,10 +535,13 @@ func (t *StatCacheTest) Test_Should_Evict_Entry_On_Full_Capacity_Including_Folde
 	t.statCache.InsertFolder(folderEntry, expiration) //adds size of 220 and exceeds capacity
 
 	hit1, entry1 = t.statCache.LookUp("1", someTime)
+	hit2, entry2 = t.statCache.LookUp("2", someTime)
 	hit3, entry3 := t.statCache.LookUpFolder("3", someTime)
 
 	assert.False(t.T(), hit1)
 	assert.Nil(t.T(), entry1)
+	assert.True(t.T(), hit2)
+	assert.Equal(t.T(), "2", entry2.Name)
 	assert.True(t.T(), hit3)
 	assert.Equal(t.T(), "3", entry3.Name)
 

From 97718945a05cf2f6bc63fd747312a318ef03bec3 Mon Sep 17 00:00:00 2001
From: Ankita Luthra <lankita@google.com>
Date: Fri, 5 Jul 2024 12:47:19 +0000
Subject: [PATCH 15/17] minor refactoring

---
 internal/cache/metadata/stat_cache.go | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/internal/cache/metadata/stat_cache.go b/internal/cache/metadata/stat_cache.go
index 68367c9b9a..ab6ff3207c 100644
--- a/internal/cache/metadata/stat_cache.go
+++ b/internal/cache/metadata/stat_cache.go
@@ -225,30 +225,30 @@ func (sc *statCacheBucketView) LookUp(
 	objectName string,
 	now time.Time) (bool, *gcs.MinObject) {
 	// Look up in the LRU cache.
-	entry := sc.sharedcachelookup(objectName, now)
-	if entry == nil {
-		return false, nil
+	hit, entry := sc.sharedcachelookup(objectName, now)
+	if hit {
+		return hit, entry.m
 	}
 
-	return true, entry.m
+	return false, nil
 }
 
 func (sc *statCacheBucketView) LookUpFolder(
 	folderName string,
 	now time.Time) (bool, *controlpb.Folder) {
 	// Look up in the LRU cache.
-	entry := sc.sharedcachelookup(folderName, now)
-	if entry == nil {
-		return false, nil
+	hit, entry := sc.sharedcachelookup(folderName, now)
+	if hit {
+		return hit, entry.f
 	}
 
-	return true, entry.f
+	return false, nil
 }
 
-func (sc *statCacheBucketView) sharedcachelookup(key string, now time.Time) *entry {
+func (sc *statCacheBucketView) sharedcachelookup(key string, now time.Time) (bool, *entry) {
 	value := sc.sharedCache.LookUp(sc.key(key))
 	if value == nil {
-		return nil
+		return false, nil
 	}
 
 	e := value.(entry)
@@ -256,10 +256,10 @@ func (sc *statCacheBucketView) sharedcachelookup(key string, now time.Time) *ent
 	// Has this entry expired?
 	if e.expiration.Before(now) {
 		sc.Erase(key)
-		return nil
+		return false, nil
 	}
 
-	return &e
+	return true, &e
 }
 
 func (sc *statCacheBucketView) InsertFolder(f *controlpb.Folder, expiration time.Time) {

From bd0789947221a93f4f7213bb063eb14556ec77f4 Mon Sep 17 00:00:00 2001
From: Ankita Luthra <lankita@google.com>
Date: Fri, 5 Jul 2024 12:51:38 +0000
Subject: [PATCH 16/17] running code cov for flaky test failure


From 82b639bc9bd9cb0a86db9154168195f7885dbdae Mon Sep 17 00:00:00 2001
From: Ankita Luthra <lankita@google.com>
Date: Mon, 8 Jul 2024 04:48:16 +0000
Subject: [PATCH 17/17] minor refactoring with renaming of tests

---
 internal/cache/metadata/stat_cache.go      |  6 +++---
 internal/cache/metadata/stat_cache_test.go | 14 +++++++-------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/internal/cache/metadata/stat_cache.go b/internal/cache/metadata/stat_cache.go
index ab6ff3207c..adc9d8a71a 100644
--- a/internal/cache/metadata/stat_cache.go
+++ b/internal/cache/metadata/stat_cache.go
@@ -225,7 +225,7 @@ func (sc *statCacheBucketView) LookUp(
 	objectName string,
 	now time.Time) (bool, *gcs.MinObject) {
 	// Look up in the LRU cache.
-	hit, entry := sc.sharedcachelookup(objectName, now)
+	hit, entry := sc.sharedCacheLookup(objectName, now)
 	if hit {
 		return hit, entry.m
 	}
@@ -237,7 +237,7 @@ func (sc *statCacheBucketView) LookUpFolder(
 	folderName string,
 	now time.Time) (bool, *controlpb.Folder) {
 	// Look up in the LRU cache.
-	hit, entry := sc.sharedcachelookup(folderName, now)
+	hit, entry := sc.sharedCacheLookup(folderName, now)
 	if hit {
 		return hit, entry.f
 	}
@@ -245,7 +245,7 @@ func (sc *statCacheBucketView) LookUpFolder(
 	return false, nil
 }
 
-func (sc *statCacheBucketView) sharedcachelookup(key string, now time.Time) (bool, *entry) {
+func (sc *statCacheBucketView) sharedCacheLookup(key string, now time.Time) (bool, *entry) {
 	value := sc.sharedCache.LookUp(sc.key(key))
 	if value == nil {
 		return false, nil
diff --git a/internal/cache/metadata/stat_cache_test.go b/internal/cache/metadata/stat_cache_test.go
index 062e2b8ae9..6eb8753eb2 100644
--- a/internal/cache/metadata/stat_cache_test.go
+++ b/internal/cache/metadata/stat_cache_test.go
@@ -419,7 +419,7 @@ func (t *MultiBucketStatCacheTest) Test_ExpiresLeastRecentlyUsed() {
 	assert.Equal(t.T(), saffron, spices.LookUpOrNil("saffron", someTime))
 }
 
-func (t *StatCacheTest) Test_InsertFolder_Create_Entry_When_No_Entry_Is_Present() {
+func (t *StatCacheTest) Test_InsertFolderCreateEntryWhenNoEntryIsPresent() {
 	const name = "key1"
 	newEntry := &controlpb.Folder{
 		Name:           name,
@@ -434,7 +434,7 @@ func (t *StatCacheTest) Test_InsertFolder_Create_Entry_When_No_Entry_Is_Present(
 	assert.Equal(t.T(), int64(1), entry.Metageneration)
 }
 
-func (t *StatCacheTest) Test_InsertFolder_Override_Entry_Old_Entry_Is_Already_Present() {
+func (t *StatCacheTest) Test_InsertFolderOverrideEntryOldEntryIsAlreadyPresent() {
 	const name = "key1"
 	existingEntry := &controlpb.Folder{
 		Name:           name,
@@ -454,7 +454,7 @@ func (t *StatCacheTest) Test_InsertFolder_Override_Entry_Old_Entry_Is_Already_Pr
 	assert.Equal(t.T(), int64(2), entry.Metageneration)
 }
 
-func (t *StatCacheTest) Test_Lookup_Return_False_If_Expiration_Is_Passed() {
+func (t *StatCacheTest) Test_LookupReturnFalseIfExpirationIsPassed() {
 	const name = "key1"
 	entry := &controlpb.Folder{
 		Name:           name,
@@ -468,7 +468,7 @@ func (t *StatCacheTest) Test_Lookup_Return_False_If_Expiration_Is_Passed() {
 	assert.Nil(t.T(), result)
 }
 
-func (t *StatCacheTest) Test_Lookup_Return_False_When_Is_Not_Present() {
+func (t *StatCacheTest) Test_LookupReturnFalseWhenIsNotPresent() {
 	const name = "key1"
 
 	hit, result := t.statCache.LookUpFolder(name, expiration.Add(time.Second))
@@ -477,7 +477,7 @@ func (t *StatCacheTest) Test_Lookup_Return_False_When_Is_Not_Present() {
 	assert.Nil(t.T(), result)
 }
 
-func (t *StatCacheTest) Test_InsertFolder_Should_Not_Override_Entry_If_Metageneration_Is_Old() {
+func (t *StatCacheTest) Test_InsertFolderShouldNotOverrideEntryIfMetagenerationIsOld() {
 	const name = "key1"
 	existingEntry := &controlpb.Folder{
 		Name:           name,
@@ -497,7 +497,7 @@ func (t *StatCacheTest) Test_InsertFolder_Should_Not_Override_Entry_If_Metagener
 	assert.Equal(t.T(), int64(2), entry.Metageneration)
 }
 
-func (t *StatCacheTest) Test_AddNegativeEntryForFolder_Should_Add_Negative_Entry_For_Folder() {
+func (t *StatCacheTest) Test_AddNegativeEntryForFolderShouldAddNegativeEntryForFolder() {
 	const name = "key1"
 	existingEntry := &controlpb.Folder{
 		Name:           name,
@@ -512,7 +512,7 @@ func (t *StatCacheTest) Test_AddNegativeEntryForFolder_Should_Add_Negative_Entry
 	assert.Nil(t.T(), entry)
 }
 
-func (t *StatCacheTest) Test_Should_Evict_Entry_On_Full_Capacity_Including_Folder_Size() {
+func (t *StatCacheTest) Test_ShouldEvictEntryOnFullCapacityIncludingFolderSize() {
 	localCache := lru.NewCache(uint64(3000))
 	t.statCache = metadata.NewStatCacheBucketView(localCache, "local_bucket")
 	objectEntry1 := &gcs.MinObject{Name: "1"}