Skip to content

Commit aba7a2a

Browse files
TulsishahTulsishah
committed
review comments
1 parent 4690cc0 commit aba7a2a

File tree

3 files changed

+253
-3
lines changed

3 files changed

+253
-3
lines changed

‎internal/fs/inode/dir.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -598,6 +598,8 @@ func (d *dirInode) LookUpChild(ctx context.Context, name string) (*Core, error)
598598

599599
var fileResult *Core
600600
var dirResult *Core
601+
var err error
602+
var result *Core
601603
lookUpFile := func() (err error) {
602604
fileResult, err = findExplicitInode(ctx, d.Bucket(), NewFileName(d.Name(), name), false)
603605
return
@@ -617,8 +619,6 @@ func (d *dirInode) LookUpChild(ctx context.Context, name string) (*Core, error)
617619

618620
var cachedType metadata.Type
619621
var cacheMissErr *caching.CacheMissError
620-
var err error
621-
var result *Core
622622
if d.IsTypeCacheDeprecated() {
623623
if d.metadataCacheTtlSecs != 0 {
624624
fileResult, err = findExplicitInode(ctx, d.Bucket(), NewFileName(d.Name(), name), true)
@@ -694,7 +694,7 @@ func (d *dirInode) LookUpChild(ctx context.Context, name string) (*Core, error)
694694
}
695695
}
696696

697-
if err := group.Wait(); err != nil {
697+
if err = group.Wait(); err != nil {
698698
return nil, err
699699
}
700700

‎internal/fs/inode/dir_test.go‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,14 @@ import (
2727
"github.com/googlecloudplatform/gcsfuse/v3/cfg"
2828
"github.com/googlecloudplatform/gcsfuse/v3/internal/cache/metadata"
2929
"github.com/googlecloudplatform/gcsfuse/v3/internal/contentcache"
30+
"github.com/googlecloudplatform/gcsfuse/v3/internal/storage/caching"
3031
"github.com/googlecloudplatform/gcsfuse/v3/internal/storage/fake"
3132
"github.com/googlecloudplatform/gcsfuse/v3/internal/storage/gcs"
33+
storagemock "github.com/googlecloudplatform/gcsfuse/v3/internal/storage/mock"
3234
"github.com/googlecloudplatform/gcsfuse/v3/internal/storage/storageutil"
3335
"github.com/googlecloudplatform/gcsfuse/v3/internal/util"
3436
"github.com/stretchr/testify/assert"
37+
"github.com/stretchr/testify/mock"
3538
"github.com/stretchr/testify/require"
3639
"github.com/stretchr/testify/suite"
3740
"golang.org/x/net/context"
@@ -2155,3 +2158,122 @@ func (t *DirTest) Test_IsTypeCacheDeprecated_true() {
21552158

21562159
assert.True(t.T(), dInode.IsTypeCacheDeprecated())
21572160
}
2161+
2162+
func (t *DirTest) TestLookUpChild_TypeCacheDeprecated_CacheMiss() {
2163+
mockBucket := new(storagemock.TestifyMockBucket)
2164+
mockBucket.On("BucketType").Return(gcs.BucketType{})
2165+
syncerBucket := gcsx.NewSyncerBucket(1, ChunkTransferTimeoutSecs, ".gcsfuse_tmp/", mockBucket)
2166+
config := &cfg.Config{
2167+
MetadataCache: cfg.MetadataCacheConfig{
2168+
TtlSecs: 60,
2169+
TypeCacheMaxSizeMb: 4,
2170+
},
2171+
EnableTypeCacheDeprecation: true,
2172+
}
2173+
in := NewDirInode(
2174+
dirInodeID,
2175+
NewDirName(NewRootName(""), dirInodeName),
2176+
fuseops.InodeAttributes{
2177+
Uid: uid,
2178+
Gid: gid,
2179+
Mode: dirMode,
2180+
},
2181+
false, // implicitDirs
2182+
false, // enableNonexistentTypeCache
2183+
typeCacheTTL,
2184+
&syncerBucket,
2185+
&t.clock,
2186+
&t.clock,
2187+
config,
2188+
)
2189+
const name = "file"
2190+
objName := path.Join(dirInodeName, name)
2191+
dirObjName := objName + "/"
2192+
cacheMissErr := &caching.CacheMissError{}
2193+
// Expect cache lookup for file -> CacheMiss
2194+
mockBucket.On("StatObject", mock.Anything, mock.MatchedBy(func(req *gcs.StatObjectRequest) bool {
2195+
return req.Name == objName && req.FetchOnlyFromCache == true
2196+
})).Return(nil, nil, cacheMissErr).Once()
2197+
// Expect cache lookup for dir -> CacheMiss
2198+
mockBucket.On("StatObject", mock.Anything, mock.MatchedBy(func(req *gcs.StatObjectRequest) bool {
2199+
return req.Name == dirObjName && req.FetchOnlyFromCache == true
2200+
})).Return(nil, nil, cacheMissErr).Once()
2201+
// Expect actual lookup for file -> Success
2202+
minObject := &gcs.MinObject{
2203+
Name: objName,
2204+
Generation: 1,
2205+
MetaGeneration: 1,
2206+
Size: 100,
2207+
}
2208+
mockBucket.On("StatObject", mock.Anything, mock.MatchedBy(func(req *gcs.StatObjectRequest) bool {
2209+
return req.Name == objName && req.FetchOnlyFromCache == false
2210+
})).Return(minObject, &gcs.ExtendedObjectAttributes{}, nil).Once()
2211+
// Expect actual lookup for dir -> NotFound
2212+
mockBucket.On("StatObject", mock.Anything, mock.MatchedBy(func(req *gcs.StatObjectRequest) bool {
2213+
return req.Name == dirObjName && req.FetchOnlyFromCache == false
2214+
})).Return(nil, nil, &gcs.NotFoundError{}).Once()
2215+
2216+
in.Lock()
2217+
entry, err := in.LookUpChild(t.ctx, name)
2218+
in.Unlock()
2219+
2220+
require.NoError(t.T(), err)
2221+
require.NotNil(t.T(), entry)
2222+
assert.Equal(t.T(), objName, entry.FullName.GcsObjectName())
2223+
mockBucket.AssertExpectations(t.T())
2224+
}
2225+
2226+
func (t *DirTest) TestLookUpChild_TypeCacheDeprecated_CacheHit() {
2227+
mockBucket := new(storagemock.TestifyMockBucket)
2228+
mockBucket.On("BucketType").Return(gcs.BucketType{})
2229+
syncerBucket := gcsx.NewSyncerBucket(1, ChunkTransferTimeoutSecs, ".gcsfuse_tmp/", mockBucket)
2230+
config := &cfg.Config{
2231+
MetadataCache: cfg.MetadataCacheConfig{
2232+
TtlSecs: 60,
2233+
TypeCacheMaxSizeMb: 4,
2234+
},
2235+
EnableTypeCacheDeprecation: true,
2236+
}
2237+
in := NewDirInode(
2238+
dirInodeID,
2239+
NewDirName(NewRootName(""), dirInodeName),
2240+
fuseops.InodeAttributes{
2241+
Uid: uid,
2242+
Gid: gid,
2243+
Mode: dirMode,
2244+
},
2245+
false, // implicitDirs
2246+
false, // enableNonexistentTypeCache
2247+
typeCacheTTL,
2248+
&syncerBucket,
2249+
&t.clock,
2250+
&t.clock,
2251+
config,
2252+
)
2253+
const name = "file"
2254+
objName := path.Join(dirInodeName, name)
2255+
dirObjName := objName + "/"
2256+
// Expect cache lookup for file -> Success
2257+
minObject := &gcs.MinObject{
2258+
Name: objName,
2259+
Generation: 1,
2260+
MetaGeneration: 1,
2261+
Size: 100,
2262+
}
2263+
mockBucket.On("StatObject", mock.Anything, mock.MatchedBy(func(req *gcs.StatObjectRequest) bool {
2264+
return req.Name == objName && req.FetchOnlyFromCache == true
2265+
})).Return(minObject, &gcs.ExtendedObjectAttributes{}, nil).Once()
2266+
// Expect cache lookup for dir -> NotFound (nil, nil)
2267+
mockBucket.On("StatObject", mock.Anything, mock.MatchedBy(func(req *gcs.StatObjectRequest) bool {
2268+
return req.Name == dirObjName && req.FetchOnlyFromCache == true
2269+
})).Return(nil, nil, &gcs.NotFoundError{}).Once()
2270+
2271+
in.Lock()
2272+
entry, err := in.LookUpChild(t.ctx, name)
2273+
in.Unlock()
2274+
2275+
require.NoError(t.T(), err)
2276+
require.NotNil(t.T(), entry)
2277+
assert.Equal(t.T(), objName, entry.FullName.GcsObjectName())
2278+
mockBucket.AssertExpectations(t.T())
2279+
}

‎internal/fs/inode/hns_dir_test.go‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424

2525
"github.com/googlecloudplatform/gcsfuse/v3/cfg"
2626
"github.com/googlecloudplatform/gcsfuse/v3/internal/cache/metadata"
27+
"github.com/googlecloudplatform/gcsfuse/v3/internal/storage/caching"
2728
"github.com/googlecloudplatform/gcsfuse/v3/internal/storage/gcs"
2829
storagemock "github.com/googlecloudplatform/gcsfuse/v3/internal/storage/mock"
2930
"github.com/jacobsa/fuse/fuseops"
@@ -888,3 +889,130 @@ func (t *HNSDirTest) TestLookUpChild_TypeCacheDeprecated_Folder() {
888889
assert.Equal(t.T(), folderName, entry.FullName.GcsObjectName())
889890
assert.Equal(t.T(), metadata.ExplicitDirType, entry.Type())
890891
}
892+
893+
func (t *HNSDirTest) TestLookUpChild_TypeCacheDeprecated_CacheMiss() {
894+
config := &cfg.Config{
895+
List: cfg.ListConfig{EnableEmptyManagedFolders: true},
896+
MetadataCache: cfg.MetadataCacheConfig{
897+
TtlSecs: 60,
898+
TypeCacheMaxSizeMb: 4,
899+
},
900+
EnableHns: true,
901+
EnableUnsupportedPathSupport: true,
902+
EnableTypeCacheDeprecation: true,
903+
}
904+
t.in.Unlock()
905+
t.in = NewDirInode(
906+
dirInodeID,
907+
NewDirName(NewRootName(""), dirInodeName),
908+
fuseops.InodeAttributes{
909+
Uid: uid,
910+
Gid: gid,
911+
Mode: dirMode,
912+
},
913+
false, // implicitDirs
914+
false, // enableNonexistentTypeCache
915+
typeCacheTTL,
916+
&t.bucket,
917+
&t.fixedTime,
918+
&t.fixedTime,
919+
config,
920+
)
921+
t.in.Lock()
922+
923+
const name = "file"
924+
objName := path.Join(dirInodeName, name)
925+
dirObjName := objName + "/"
926+
927+
cacheMissErr := &caching.CacheMissError{}
928+
929+
// Expect cache lookup for file -> CacheMiss
930+
t.mockBucket.On("StatObject", mock.Anything, mock.MatchedBy(func(req *gcs.StatObjectRequest) bool {
931+
return req.Name == objName && req.FetchOnlyFromCache == true
932+
})).Return(nil, nil, cacheMissErr).Once()
933+
934+
// Expect cache lookup for dir -> CacheMiss
935+
t.mockBucket.On("GetFolder", mock.Anything, mock.MatchedBy(func(req *gcs.GetFolderRequest) bool {
936+
return req.Name == dirObjName && req.FetchOnlyFromCache == true
937+
})).Return(nil, cacheMissErr).Once()
938+
939+
// Expect actual lookup for file -> Success
940+
minObject := &gcs.MinObject{
941+
Name: objName,
942+
Generation: 1,
943+
MetaGeneration: 1,
944+
Size: 100,
945+
}
946+
t.mockBucket.On("StatObject", mock.Anything, mock.MatchedBy(func(req *gcs.StatObjectRequest) bool {
947+
return req.Name == objName && req.FetchOnlyFromCache == false
948+
})).Return(minObject, &gcs.ExtendedObjectAttributes{}, nil).Once()
949+
950+
// Expect actual lookup for dir -> NotFound
951+
t.mockBucket.On("GetFolder", mock.Anything, mock.MatchedBy(func(req *gcs.GetFolderRequest) bool {
952+
return req.Name == dirObjName && req.FetchOnlyFromCache == false
953+
})).Return(nil, &gcs.NotFoundError{}).Once()
954+
955+
entry, err := t.in.LookUpChild(t.ctx, name)
956+
957+
require.NoError(t.T(), err)
958+
require.NotNil(t.T(), entry)
959+
assert.Equal(t.T(), objName, entry.FullName.GcsObjectName())
960+
t.mockBucket.AssertExpectations(t.T())
961+
}
962+
963+
func (t *HNSDirTest) TestLookUpChild_TypeCacheDeprecated_CacheHit() {
964+
config := &cfg.Config{
965+
List: cfg.ListConfig{EnableEmptyManagedFolders: true},
966+
MetadataCache: cfg.MetadataCacheConfig{
967+
TtlSecs: 60,
968+
TypeCacheMaxSizeMb: 4,
969+
},
970+
EnableHns: true,
971+
EnableUnsupportedPathSupport: true,
972+
EnableTypeCacheDeprecation: true,
973+
}
974+
t.in.Unlock()
975+
t.in = NewDirInode(
976+
dirInodeID,
977+
NewDirName(NewRootName(""), dirInodeName),
978+
fuseops.InodeAttributes{
979+
Uid: uid,
980+
Gid: gid,
981+
Mode: dirMode,
982+
},
983+
false, // implicitDirs
984+
false, // enableNonexistentTypeCache
985+
typeCacheTTL,
986+
&t.bucket,
987+
&t.fixedTime,
988+
&t.fixedTime,
989+
config,
990+
)
991+
t.in.Lock()
992+
const name = "file"
993+
objName := path.Join(dirInodeName, name)
994+
dirObjName := objName + "/"
995+
996+
// Expect cache lookup for file -> Success
997+
minObject := &gcs.MinObject{
998+
Name: objName,
999+
Generation: 1,
1000+
MetaGeneration: 1,
1001+
Size: 100,
1002+
}
1003+
t.mockBucket.On("StatObject", mock.Anything, mock.MatchedBy(func(req *gcs.StatObjectRequest) bool {
1004+
return req.Name == objName && req.FetchOnlyFromCache == true
1005+
})).Return(minObject, &gcs.ExtendedObjectAttributes{}, nil).Once()
1006+
1007+
// Expect cache lookup for dir -> NotFound (nil, nil)
1008+
t.mockBucket.On("GetFolder", mock.Anything, mock.MatchedBy(func(req *gcs.GetFolderRequest) bool {
1009+
return req.Name == dirObjName && req.FetchOnlyFromCache == true
1010+
})).Return(nil, &gcs.NotFoundError{}).Once()
1011+
1012+
entry, err := t.in.LookUpChild(t.ctx, name)
1013+
1014+
require.NoError(t.T(), err)
1015+
require.NotNil(t.T(), entry)
1016+
assert.Equal(t.T(), objName, entry.FullName.GcsObjectName())
1017+
t.mockBucket.AssertExpectations(t.T())
1018+
}

0 commit comments

Comments
 (0)