From ca638ddc516bc3c465114ca8ac9a5fe33d93f3e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Fri, 28 Apr 2023 13:50:59 +0200 Subject: [PATCH 1/8] cache special drive items until space root changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../pkg/config/defaults/defaultconfig.go | 2 + services/graph/pkg/service/v0/driveitems.go | 37 ++++++++++++++++++- services/graph/pkg/service/v0/drives.go | 2 +- services/graph/pkg/service/v0/graph.go | 2 +- services/graph/pkg/service/v0/service.go | 2 +- 5 files changed, 40 insertions(+), 5 deletions(-) diff --git a/services/graph/pkg/config/defaults/defaultconfig.go b/services/graph/pkg/config/defaults/defaultconfig.go index eff3b84772..42545f19a3 100644 --- a/services/graph/pkg/config/defaults/defaultconfig.go +++ b/services/graph/pkg/config/defaults/defaultconfig.go @@ -54,6 +54,8 @@ func DefaultConfig() *config.Config { WebDavPath: "/dav/spaces/", DefaultQuota: "1000000000", // 30 minutes + ExtendedSpacePropertiesCacheTTL: 1800, + // 30 minutes GroupsCacheTTL: 1800, // 30 minutes UsersCacheTTL: 1800, diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index 58b87cf38b..86fb03a313 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/services/graph/pkg/service/v0/driveitems.go @@ -215,8 +215,20 @@ func (g Graph) getPathForResource(ctx context.Context, id storageprovider.Resour return res.Path, err } -// getExtendedSpaceProperties reads properties from the opaque and transforms them into driveItems -func (g Graph) getExtendedSpaceProperties(ctx context.Context, baseURL *url.URL, space *storageprovider.StorageSpace) []libregraph.DriveItem { +// getSpecialDriveItems reads properties from the opaque and transforms them into driveItems +func (g Graph) getSpecialDriveItems(ctx context.Context, baseURL *url.URL, space *storageprovider.StorageSpace) []libregraph.DriveItem { + + // if the root is older or equal to our cache we can reuse the cached extended spaces properties + if entry := g.specialDriveItemsCache.Get(spaceRootStatKey(space.Root)); entry != nil { + if spe, ok := entry.Value().(specialDriveItemEntry); ok { + if spe.rootMtime != nil && space.Mtime != nil { + if spe.rootMtime.Seconds >= space.Mtime.Seconds { // second precision is good enough + return spe.specialDriveItems + } + } + } + } + var spaceItems []libregraph.DriveItem if space.Opaque == nil { return nil @@ -235,9 +247,30 @@ func (g Graph) getExtendedSpaceProperties(ctx context.Context, baseURL *url.URL, } } } + + // cache properties + spacePropertiesEntry := specialDriveItemEntry{ + specialDriveItems: spaceItems, + rootMtime: space.Mtime, + } + g.specialDriveItemsCache.Set(spaceRootStatKey(space.Root), spacePropertiesEntry, time.Duration(g.config.Spaces.ExtendedSpacePropertiesCacheTTL)) + return spaceItems } +// generates a space root stat cache key used to detect changes in a space +func spaceRootStatKey(id *storageprovider.ResourceId) string { + if id == nil { + return "" + } + return id.StorageId + "$" + id.SpaceId + "!" + id.OpaqueId +} + +type specialDriveItemEntry struct { + specialDriveItems []libregraph.DriveItem + rootMtime *types.Timestamp +} + func (g Graph) getSpecialDriveItem(ctx context.Context, id storageprovider.ResourceId, itemName string, baseURL *url.URL, space *storageprovider.StorageSpace) *libregraph.DriveItem { var spaceItem *libregraph.DriveItem if id.SpaceId == "" && id.OpaqueId == "" { diff --git a/services/graph/pkg/service/v0/drives.go b/services/graph/pkg/service/v0/drives.go index 50d8215465..f3df77b44c 100644 --- a/services/graph/pkg/service/v0/drives.go +++ b/services/graph/pkg/service/v0/drives.go @@ -536,7 +536,7 @@ func (g Graph) formatDrives(ctx context.Context, baseURL *url.URL, storageSpaces // can't access disabled space if utils.ReadPlainFromOpaque(storageSpace.Opaque, "trashed") != "trashed" { - res.Special = g.getExtendedSpaceProperties(ctx, baseURL, storageSpace) + res.Special = g.getSpecialDriveItems(ctx, baseURL, storageSpace) quota, err := g.getDriveQuota(ctx, storageSpace) res.Quota = "a if err != nil { diff --git a/services/graph/pkg/service/v0/graph.go b/services/graph/pkg/service/v0/graph.go index 4ab673ddc2..eab4903d40 100644 --- a/services/graph/pkg/service/v0/graph.go +++ b/services/graph/pkg/service/v0/graph.go @@ -65,7 +65,7 @@ type Graph struct { gatewayClient gateway.GatewayAPIClient roleService RoleService permissionsService Permissions - spacePropertiesCache *ttlcache.Cache[string, interface{}] + specialDriveItemsCache *ttlcache.Cache[string, interface{}] usersCache *ttlcache.Cache[string, libregraph.User] groupsCache *ttlcache.Cache[string, libregraph.Group] eventsPublisher events.Publisher diff --git a/services/graph/pkg/service/v0/service.go b/services/graph/pkg/service/v0/service.go index 8984685d37..234e182fbe 100644 --- a/services/graph/pkg/service/v0/service.go +++ b/services/graph/pkg/service/v0/service.go @@ -136,7 +136,7 @@ func NewService(opts ...Option) (Graph, error) { config: options.Config, mux: m, logger: &options.Logger, - spacePropertiesCache: spacePropertiesCache, + specialDriveItemsCache: spacePropertiesCache, usersCache: usersCache, groupsCache: groupsCache, eventsPublisher: options.EventsPublisher, From f2b86d8115c620d8c84637a94dac32b209db7cdb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Tue, 2 May 2023 16:40:58 +0200 Subject: [PATCH 2/8] stat based on default paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- services/graph/pkg/service/v0/driveitems.go | 56 ++++++++++++++------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index 86fb03a313..2af1d7e174 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/services/graph/pkg/service/v0/driveitems.go @@ -77,18 +77,16 @@ func (g Graph) GetRootDriveChildren(w http.ResponseWriter, r *http.Request) { render.JSON(w, r, &ListResponse{Value: files}) } -func (g Graph) getDriveItem(ctx context.Context, root storageprovider.ResourceId) (*libregraph.DriveItem, error) { +func (g Graph) getDriveItem(ctx context.Context, ref storageprovider.Reference) (*libregraph.DriveItem, error) { client := g.GetGatewayClient() - ref := &storageprovider.Reference{ - ResourceId: &root, - } - res, err := client.Stat(ctx, &storageprovider.StatRequest{Ref: ref}) + res, err := client.Stat(ctx, &storageprovider.StatRequest{Ref: &ref}) if err != nil { return nil, err } if res.Status.Code != cs3rpc.Code_CODE_OK { - return nil, fmt.Errorf("could not stat %s: %s", ref, res.Status.Message) + refStr, _ := storagespace.FormatReference(&ref) + return nil, fmt.Errorf("could not stat %s: %s", refStr, res.Status.Message) } return cs3ResourceToDriveItem(res.Info) } @@ -234,17 +232,30 @@ func (g Graph) getSpecialDriveItems(ctx context.Context, baseURL *url.URL, space return nil } metadata := space.Opaque.Map - names := [2]string{SpaceImageSpecialFolderName, ReadmeSpecialFolderName} + names := map[string]string{ + SpaceImageSpecialFolderName: "/.space/logo.png", + ReadmeSpecialFolderName: "/.space/readme.md", + } - for _, itemName := range names { + for itemName, itemPath := range names { + // The default is a path relative to the space root + var ref storageprovider.Reference if itemID, ok := metadata[itemName]; ok { rid, _ := storagespace.ParseID(string(itemID.Value)) // add the storageID of the space, all drive items of this space belong to the same storageID rid.StorageId = space.GetRoot().GetStorageId() - spaceItem := g.getSpecialDriveItem(ctx, rid, itemName, baseURL, space) - if spaceItem != nil { - spaceItems = append(spaceItems, *spaceItem) + ref = storageprovider.Reference{ + ResourceId: &rid, } + } else { + ref = storageprovider.Reference{ + ResourceId: space.GetRoot(), + Path: itemPath, + } + } + spaceItem := g.getSpecialDriveItem(ctx, ref, itemName, baseURL, space) + if spaceItem != nil { + spaceItems = append(spaceItems, *spaceItem) } } @@ -271,21 +282,28 @@ type specialDriveItemEntry struct { rootMtime *types.Timestamp } -func (g Graph) getSpecialDriveItem(ctx context.Context, id storageprovider.ResourceId, itemName string, baseURL *url.URL, space *storageprovider.StorageSpace) *libregraph.DriveItem { +func (g Graph) getSpecialDriveItem(ctx context.Context, ref storageprovider.Reference, itemName string, baseURL *url.URL, space *storageprovider.StorageSpace) *libregraph.DriveItem { var spaceItem *libregraph.DriveItem - if id.SpaceId == "" && id.OpaqueId == "" { + if ref.GetResourceId().GetSpaceId() == "" && ref.GetResourceId().GetOpaqueId() == "" { return nil } - spaceItem, err := g.getDriveItem(ctx, id) + // FIXME we should send a fieldmask 'path' and return it as the Path property to save an additional call to the storage. + // To do that we need to align the useg of ResourceInfo.Name vs ResourceInfo.Path. By default, only the name should be set + // and Path should always be relative to the space root OR the resource the current user can access ... + spaceItem, err := g.getDriveItem(ctx, ref) if err != nil { - g.logger.Error().Err(err).Str("ID", id.OpaqueId).Str("name", itemName).Msg("Could not get item info") + g.logger.Error().Err(err).Str("ID", ref.GetResourceId().GetOpaqueId()).Str("name", itemName).Msg("Could not get item info") return nil } - itemPath, err := g.getPathForResource(ctx, id) - if err != nil { - g.logger.Error().Err(err).Str("ID", id.OpaqueId).Str("name", itemName).Msg("Could not get item path") - return nil + itemPath := ref.Path + if itemPath == "" { + // lookup by id + itemPath, err = g.getPathForResource(ctx, *ref.ResourceId) + if err != nil { + g.logger.Error().Err(err).Str("ID", ref.GetResourceId().GetOpaqueId()).Str("name", itemName).Msg("Could not get item path") + return nil + } } spaceItem.SpecialFolder = &libregraph.SpecialFolder{Name: libregraph.PtrString(itemName)} webdavURL := *baseURL From 40eba0909d07bc9a942b55b1a29698c752c4e2e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Tue, 2 May 2023 22:04:04 +0200 Subject: [PATCH 3/8] fix mocks for stat based special resource handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- services/graph/pkg/service/v0/graph_test.go | 38 ++++++++++++++++----- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/services/graph/pkg/service/v0/graph_test.go b/services/graph/pkg/service/v0/graph_test.go index aace93a770..0b244d5057 100644 --- a/services/graph/pkg/service/v0/graph_test.go +++ b/services/graph/pkg/service/v0/graph_test.go @@ -202,6 +202,9 @@ var _ = Describe("Graph", func() { gatewayClient.On("GetQuota", mock.Anything, mock.Anything).Return(&provider.GetQuotaResponse{ Status: status.NewUnimplemented(ctx, fmt.Errorf("not supported"), "not supported"), }, nil) + gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(&provider.StatResponse{ + Status: status.NewNotFound(ctx, "no special files found"), + }, nil) r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/me/drives?$orderby=name%20asc", nil) r = r.WithContext(ctx) @@ -891,16 +894,30 @@ var _ = Describe("Graph", func() { Status: status.NewOK(ctx), Path: "thepath", }, nil) - gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(&provider.StatResponse{ - Status: status.NewOK(ctx), - Info: &provider.ResourceInfo{ - Id: &provider.ResourceId{ - StorageId: "pro-1", - SpaceId: "spaceID", - OpaqueId: "specialID", + // no stat for the image + gatewayClient.On("Stat", + mock.Anything, + mock.MatchedBy( + func(req *provider.StatRequest) bool { + return req.Ref.Path == "/.space/logo.png" + })). + Return(&provider.StatResponse{ + Status: status.NewNotFound(ctx, "not found"), + }, nil) + // mock readme stats + gatewayClient.On("Stat", + mock.Anything, + mock.Anything). + Return(&provider.StatResponse{ + Status: status.NewOK(ctx), + Info: &provider.ResourceInfo{ + Id: &provider.ResourceId{ + StorageId: "pro-1", + SpaceId: "spaceID", + OpaqueId: "specialID", + }, }, - }, - }, nil) + }, nil) gatewayClient.On("ListStorageSpaces", mock.Anything, mock.MatchedBy( @@ -994,6 +1011,9 @@ var _ = Describe("Graph", func() { StorageSpace: req.StorageSpace, } }, nil) + gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(&provider.StatResponse{ + Status: status.NewNotFound(ctx, "no special files found"), + }, nil) r := httptest.NewRequest(http.MethodPatch, "/graph/v1.0/drives/{driveID}/", bytes.NewBuffer(driveJson)) rctx := chi.NewRouteContext() From 92533895c5b3aedf731fc901abd374e7926072de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 3 May 2023 00:21:57 +0200 Subject: [PATCH 4/8] some fixes for the tests and noisy logs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- services/graph/pkg/service/v0/driveitems.go | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index 2af1d7e174..69832e4ee0 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/services/graph/pkg/service/v0/driveitems.go @@ -13,6 +13,7 @@ import ( storageprovider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" "github.com/cs3org/reva/v2/pkg/storagespace" + "github.com/cs3org/reva/v2/pkg/utils" "github.com/go-chi/render" libregraph "github.com/owncloud/libre-graph-api-go" "github.com/owncloud/ocis/v2/services/graph/pkg/service/v0/errorcode" @@ -215,13 +216,17 @@ func (g Graph) getPathForResource(ctx context.Context, id storageprovider.Resour // getSpecialDriveItems reads properties from the opaque and transforms them into driveItems func (g Graph) getSpecialDriveItems(ctx context.Context, baseURL *url.URL, space *storageprovider.StorageSpace) []libregraph.DriveItem { + if space.GetRoot().GetStorageId() == utils.ShareStorageProviderID { + return nil // no point in stating the ShareStorageProvider + } // if the root is older or equal to our cache we can reuse the cached extended spaces properties if entry := g.specialDriveItemsCache.Get(spaceRootStatKey(space.Root)); entry != nil { - if spe, ok := entry.Value().(specialDriveItemEntry); ok { - if spe.rootMtime != nil && space.Mtime != nil { - if spe.rootMtime.Seconds >= space.Mtime.Seconds { // second precision is good enough - return spe.specialDriveItems + if cached, ok := entry.Value().(specialDriveItemEntry); ok { + if cached.rootMtime != nil && space.Mtime != nil { + // beware, LaterTS does not handle equalness. it returns t1 if t1 > t2, else t2, so a >= check looks like this + if utils.LaterTS(space.Mtime, cached.rootMtime) == cached.rootMtime { + return cached.specialDriveItems } } } @@ -293,7 +298,7 @@ func (g Graph) getSpecialDriveItem(ctx context.Context, ref storageprovider.Refe // and Path should always be relative to the space root OR the resource the current user can access ... spaceItem, err := g.getDriveItem(ctx, ref) if err != nil { - g.logger.Error().Err(err).Str("ID", ref.GetResourceId().GetOpaqueId()).Str("name", itemName).Msg("Could not get item info") + g.logger.Debug().Err(err).Str("ID", ref.GetResourceId().GetOpaqueId()).Str("name", itemName).Msg("Could not get item info") return nil } itemPath := ref.Path @@ -301,7 +306,7 @@ func (g Graph) getSpecialDriveItem(ctx context.Context, ref storageprovider.Refe // lookup by id itemPath, err = g.getPathForResource(ctx, *ref.ResourceId) if err != nil { - g.logger.Error().Err(err).Str("ID", ref.GetResourceId().GetOpaqueId()).Str("name", itemName).Msg("Could not get item path") + g.logger.Debug().Err(err).Str("ID", ref.GetResourceId().GetOpaqueId()).Str("name", itemName).Msg("Could not get item path") return nil } } From 13369f8367e91a817017da96b93fefefdf461a75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 3 May 2023 09:18:19 +0200 Subject: [PATCH 5/8] only stat if property is set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- services/graph/pkg/service/v0/driveitems.go | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index 69832e4ee0..e23ceb3ad2 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/services/graph/pkg/service/v0/driveitems.go @@ -237,12 +237,9 @@ func (g Graph) getSpecialDriveItems(ctx context.Context, baseURL *url.URL, space return nil } metadata := space.Opaque.Map - names := map[string]string{ - SpaceImageSpecialFolderName: "/.space/logo.png", - ReadmeSpecialFolderName: "/.space/readme.md", - } + names := [2]string{SpaceImageSpecialFolderName, ReadmeSpecialFolderName} - for itemName, itemPath := range names { + for _, itemName := range names { // The default is a path relative to the space root var ref storageprovider.Reference if itemID, ok := metadata[itemName]; ok { @@ -252,16 +249,11 @@ func (g Graph) getSpecialDriveItems(ctx context.Context, baseURL *url.URL, space ref = storageprovider.Reference{ ResourceId: &rid, } - } else { - ref = storageprovider.Reference{ - ResourceId: space.GetRoot(), - Path: itemPath, + spaceItem := g.getSpecialDriveItem(ctx, ref, itemName, baseURL, space) + if spaceItem != nil { + spaceItems = append(spaceItems, *spaceItem) } } - spaceItem := g.getSpecialDriveItem(ctx, ref, itemName, baseURL, space) - if spaceItem != nil { - spaceItems = append(spaceItems, *spaceItem) - } } // cache properties From c7bbdd2eae8ea78e17031ff54efc06f65ba7605d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 3 May 2023 11:09:55 +0200 Subject: [PATCH 6/8] add special nodes to cache key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- services/graph/pkg/service/v0/driveitems.go | 40 ++++++++++++++------- 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index e23ceb3ad2..7cfedd51e2 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/services/graph/pkg/service/v0/driveitems.go @@ -17,6 +17,7 @@ import ( "github.com/go-chi/render" libregraph "github.com/owncloud/libre-graph-api-go" "github.com/owncloud/ocis/v2/services/graph/pkg/service/v0/errorcode" + "golang.org/x/crypto/sha3" ) // GetRootDriveChildren implements the Service interface. @@ -219,9 +220,16 @@ func (g Graph) getSpecialDriveItems(ctx context.Context, baseURL *url.URL, space if space.GetRoot().GetStorageId() == utils.ShareStorageProviderID { return nil // no point in stating the ShareStorageProvider } + if space.Opaque == nil { + return nil + } + imageNode := utils.ReadPlainFromOpaque(space.Opaque, SpaceImageSpecialFolderName) + readmeNode := utils.ReadPlainFromOpaque(space.Opaque, ReadmeSpecialFolderName) + + cachekey := spaceRootStatKey(space.Root, imageNode, readmeNode) // if the root is older or equal to our cache we can reuse the cached extended spaces properties - if entry := g.specialDriveItemsCache.Get(spaceRootStatKey(space.Root)); entry != nil { + if entry := g.specialDriveItemsCache.Get(cachekey); entry != nil { if cached, ok := entry.Value().(specialDriveItemEntry); ok { if cached.rootMtime != nil && space.Mtime != nil { // beware, LaterTS does not handle equalness. it returns t1 if t1 > t2, else t2, so a >= check looks like this @@ -233,17 +241,14 @@ func (g Graph) getSpecialDriveItems(ctx context.Context, baseURL *url.URL, space } var spaceItems []libregraph.DriveItem - if space.Opaque == nil { - return nil - } - metadata := space.Opaque.Map - names := [2]string{SpaceImageSpecialFolderName, ReadmeSpecialFolderName} - for _, itemName := range names { - // The default is a path relative to the space root + for itemName, itemNode := range map[string]string{ + SpaceImageSpecialFolderName: imageNode, + ReadmeSpecialFolderName: readmeNode, + } { var ref storageprovider.Reference - if itemID, ok := metadata[itemName]; ok { - rid, _ := storagespace.ParseID(string(itemID.Value)) + if itemNode != "" { + rid, _ := storagespace.ParseID(itemNode) // add the storageID of the space, all drive items of this space belong to the same storageID rid.StorageId = space.GetRoot().GetStorageId() ref = storageprovider.Reference{ @@ -261,17 +266,26 @@ func (g Graph) getSpecialDriveItems(ctx context.Context, baseURL *url.URL, space specialDriveItems: spaceItems, rootMtime: space.Mtime, } - g.specialDriveItemsCache.Set(spaceRootStatKey(space.Root), spacePropertiesEntry, time.Duration(g.config.Spaces.ExtendedSpacePropertiesCacheTTL)) + g.specialDriveItemsCache.Set(cachekey, spacePropertiesEntry, time.Duration(g.config.Spaces.ExtendedSpacePropertiesCacheTTL)) return spaceItems } // generates a space root stat cache key used to detect changes in a space -func spaceRootStatKey(id *storageprovider.ResourceId) string { +// takes into account the special nodes because changing metadata does not affect the etag / mtime +func spaceRootStatKey(id *storageprovider.ResourceId, imagenode, readmeNode string) string { if id == nil { return "" } - return id.StorageId + "$" + id.SpaceId + "!" + id.OpaqueId + sha3 := sha3.NewShake256() + sha3.Write([]byte(id.GetStorageId())) + sha3.Write([]byte(id.GetSpaceId())) + sha3.Write([]byte(id.GetOpaqueId())) + sha3.Write([]byte(imagenode)) + sha3.Write([]byte(readmeNode)) + h := make([]byte, 64) + sha3.Read(h) + return fmt.Sprintf("%x", h) } type specialDriveItemEntry struct { From 024e66ffd01ca0b8bd2c3b6fcc177258792cc508 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 3 May 2023 11:26:46 +0200 Subject: [PATCH 7/8] ignore errors when building the cache key hash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- services/graph/pkg/service/v0/driveitems.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index 7cfedd51e2..0d34e1559c 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/services/graph/pkg/service/v0/driveitems.go @@ -278,11 +278,11 @@ func spaceRootStatKey(id *storageprovider.ResourceId, imagenode, readmeNode stri return "" } sha3 := sha3.NewShake256() - sha3.Write([]byte(id.GetStorageId())) - sha3.Write([]byte(id.GetSpaceId())) - sha3.Write([]byte(id.GetOpaqueId())) - sha3.Write([]byte(imagenode)) - sha3.Write([]byte(readmeNode)) + _, _ = sha3.Write([]byte(id.GetStorageId())) + _, _ = sha3.Write([]byte(id.GetSpaceId())) + _, _ = sha3.Write([]byte(id.GetOpaqueId())) + _, _ = sha3.Write([]byte(imagenode)) + _, _ = sha3.Write([]byte(readmeNode)) h := make([]byte, 64) sha3.Read(h) return fmt.Sprintf("%x", h) From 4eaa90af541917aa4bd08d3b19bd744b9c9bb785 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 3 May 2023 12:15:02 +0200 Subject: [PATCH 8/8] address code smells MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- services/graph/pkg/service/v0/driveitems.go | 39 +++++++++++---------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index 0d34e1559c..3c9d412b62 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/services/graph/pkg/service/v0/driveitems.go @@ -242,24 +242,8 @@ func (g Graph) getSpecialDriveItems(ctx context.Context, baseURL *url.URL, space var spaceItems []libregraph.DriveItem - for itemName, itemNode := range map[string]string{ - SpaceImageSpecialFolderName: imageNode, - ReadmeSpecialFolderName: readmeNode, - } { - var ref storageprovider.Reference - if itemNode != "" { - rid, _ := storagespace.ParseID(itemNode) - // add the storageID of the space, all drive items of this space belong to the same storageID - rid.StorageId = space.GetRoot().GetStorageId() - ref = storageprovider.Reference{ - ResourceId: &rid, - } - spaceItem := g.getSpecialDriveItem(ctx, ref, itemName, baseURL, space) - if spaceItem != nil { - spaceItems = append(spaceItems, *spaceItem) - } - } - } + spaceItems = g.fetchSpecialDriveItem(ctx, spaceItems, SpaceImageSpecialFolderName, imageNode, space, baseURL) + spaceItems = g.fetchSpecialDriveItem(ctx, spaceItems, ReadmeSpecialFolderName, readmeNode, space, baseURL) // cache properties spacePropertiesEntry := specialDriveItemEntry{ @@ -271,6 +255,23 @@ func (g Graph) getSpecialDriveItems(ctx context.Context, baseURL *url.URL, space return spaceItems } +func (g Graph) fetchSpecialDriveItem(ctx context.Context, spaceItems []libregraph.DriveItem, itemName string, itemNode string, space *storageprovider.StorageSpace, baseURL *url.URL) []libregraph.DriveItem { + var ref storageprovider.Reference + if itemNode != "" { + rid, _ := storagespace.ParseID(itemNode) + + rid.StorageId = space.GetRoot().GetStorageId() + ref = storageprovider.Reference{ + ResourceId: &rid, + } + spaceItem := g.getSpecialDriveItem(ctx, ref, itemName, baseURL, space) + if spaceItem != nil { + spaceItems = append(spaceItems, *spaceItem) + } + } + return spaceItems +} + // generates a space root stat cache key used to detect changes in a space // takes into account the special nodes because changing metadata does not affect the etag / mtime func spaceRootStatKey(id *storageprovider.ResourceId, imagenode, readmeNode string) string { @@ -284,7 +285,7 @@ func spaceRootStatKey(id *storageprovider.ResourceId, imagenode, readmeNode stri _, _ = sha3.Write([]byte(imagenode)) _, _ = sha3.Write([]byte(readmeNode)) h := make([]byte, 64) - sha3.Read(h) + _, _ = sha3.Read(h) return fmt.Sprintf("%x", h) }