From 8c5af66b1f4072129764a85ead60f2041ff3b6c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 26 Mar 2025 13:47:15 +0100 Subject: [PATCH] graph: reduce memory allocations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../pkg/service/v0/api_driveitem_permissions.go | 1 + .../service/v0/api_driveitem_permissions_test.go | 7 +++++-- services/graph/pkg/service/v0/base.go | 15 ++++++--------- services/graph/pkg/service/v0/rolemanagement.go | 2 +- services/graph/pkg/service/v0/service.go | 2 ++ services/graph/pkg/service/v0/sharedwithme.go | 6 ++---- 6 files changed, 17 insertions(+), 16 deletions(-) diff --git a/services/graph/pkg/service/v0/api_driveitem_permissions.go b/services/graph/pkg/service/v0/api_driveitem_permissions.go index 4e013c5ada..ed37c9b3ab 100644 --- a/services/graph/pkg/service/v0/api_driveitem_permissions.go +++ b/services/graph/pkg/service/v0/api_driveitem_permissions.go @@ -83,6 +83,7 @@ func NewDriveItemPermissionsService(logger log.Logger, gatewaySelector pool.Sele gatewaySelector: gatewaySelector, identityCache: identityCache, config: config, + availableRoles: unifiedrole.GetRoles(unifiedrole.RoleFilterIDs(config.UnifiedRoles.AvailableRoles...)), }, }, nil } diff --git a/services/graph/pkg/service/v0/api_driveitem_permissions_test.go b/services/graph/pkg/service/v0/api_driveitem_permissions_test.go index 7a576be6d5..927067e91c 100644 --- a/services/graph/pkg/service/v0/api_driveitem_permissions_test.go +++ b/services/graph/pkg/service/v0/api_driveitem_permissions_test.go @@ -441,7 +441,6 @@ var _ = Describe("DriveItemPermissionsService", func() { }) It("returns role denied", func() { statResponse.Info.PermissionSet = roleconversions.NewManagerRole().CS3ResourcePermissions() - cfg.UnifiedRoles.AvailableRoles = []string{unifiedrole.UnifiedRoleViewerID, unifiedrole.UnifiedRoleDeniedID, unifiedrole.UnifiedRoleManagerID} listSharesResponse.Shares = []*collaboration.Share{ { Id: &collaboration.ShareId{OpaqueId: "1"}, @@ -465,11 +464,15 @@ var _ = Describe("DriveItemPermissionsService", func() { } listPublicSharesResponse.Share = []*link.PublicShare{} + cfg = defaults.FullDefaultConfig() + cfg.UnifiedRoles.AvailableRoles = []string{unifiedrole.UnifiedRoleViewerID, unifiedrole.UnifiedRoleDeniedID, unifiedrole.UnifiedRoleManagerID} + service, err := svc.NewDriveItemPermissionsService(log.NewLogger(), gatewaySelector, cache, cfg) + gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(statResponse, nil) gatewayClient.On("ListShares", mock.Anything, mock.Anything).Return(listSharesResponse, nil) gatewayClient.On("GetUser", mock.Anything, mock.Anything).Return(getUserResponse, nil) gatewayClient.On("ListPublicShares", mock.Anything, mock.Anything).Return(listPublicSharesResponse, nil) - permissions, err := driveItemPermissionsService.ListPermissions(context.Background(), itemID, false, false) + permissions, err := service.ListPermissions(context.Background(), itemID, false, false) Expect(err).ToNot(HaveOccurred()) Expect(len(permissions.LibreGraphPermissionsActionsAllowedValues)).ToNot(BeZero()) Expect(len(permissions.LibreGraphPermissionsRolesAllowedValues)).ToNot(BeZero()) diff --git a/services/graph/pkg/service/v0/base.go b/services/graph/pkg/service/v0/base.go index 05a6aee869..59a875c34a 100644 --- a/services/graph/pkg/service/v0/base.go +++ b/services/graph/pkg/service/v0/base.go @@ -46,6 +46,7 @@ type BaseGraphService struct { gatewaySelector pool.Selectable[gateway.GatewayAPIClient] identityCache identity.IdentityCache config *config.Config + availableRoles []*libregraph.UnifiedRoleDefinition } func (g BaseGraphService) getSpaceRootPermissions(ctx context.Context, spaceID *storageprovider.StorageSpaceId) ([]libregraph.Permission, error) { @@ -86,8 +87,7 @@ func (g BaseGraphService) CS3ReceivedSharesToDriveItems(ctx context.Context, rec return nil, err } - availableRoles := unifiedrole.GetRoles(unifiedrole.RoleFilterIDs(g.config.UnifiedRoles.AvailableRoles...)) - return cs3ReceivedSharesToDriveItems(ctx, g.logger, gatewayClient, g.identityCache, receivedShares, availableRoles) + return cs3ReceivedSharesToDriveItems(ctx, g.logger, gatewayClient, g.identityCache, receivedShares, g.availableRoles) } func (g BaseGraphService) CS3ReceivedOCMSharesToDriveItems(ctx context.Context, receivedShares []*ocm.ReceivedShare) ([]libregraph.DriveItem, error) { @@ -96,8 +96,7 @@ func (g BaseGraphService) CS3ReceivedOCMSharesToDriveItems(ctx context.Context, return nil, err } - availableRoles := unifiedrole.GetRoles(unifiedrole.RoleFilterIDs(g.config.UnifiedRoles.AvailableRoles...)) - return cs3ReceivedOCMSharesToDriveItems(ctx, g.logger, gatewayClient, g.identityCache, receivedShares, availableRoles) + return cs3ReceivedOCMSharesToDriveItems(ctx, g.logger, gatewayClient, g.identityCache, receivedShares, g.availableRoles) } func (g BaseGraphService) cs3SpacePermissionsToLibreGraph(ctx context.Context, space *storageprovider.StorageSpace, apiVersion APIVersion) []libregraph.Permission { @@ -196,9 +195,8 @@ func (g BaseGraphService) cs3SpacePermissionsToLibreGraph(ctx context.Context, s p.SetExpirationDateTime(time.Unix(int64(exp.GetSeconds()), int64(exp.GetNanos()))) } - availableRoles := unifiedrole.GetRoles(unifiedrole.RoleFilterIDs(g.config.UnifiedRoles.AvailableRoles...)) if role := unifiedrole.CS3ResourcePermissionsToRole( - availableRoles, + g.availableRoles, perm, unifiedrole.UnifiedRoleConditionDrive, false, @@ -601,7 +599,7 @@ func (g BaseGraphService) cs3UserShareToPermission(ctx context.Context, share *c perm.SetCreatedDateTime(cs3TimestampToTime(share.GetCtime())) } role := unifiedrole.CS3ResourcePermissionsToRole( - unifiedrole.GetRoles(unifiedrole.RoleFilterIDs(g.config.UnifiedRoles.AvailableRoles...)), + g.availableRoles, share.GetPermissions().GetPermissions(), roleCondition, false, @@ -689,9 +687,8 @@ func (g BaseGraphService) cs3OCMShareToPermission(ctx context.Context, share *oc } } - availableRoles := unifiedrole.GetRoles(unifiedrole.RoleFilterIDs(g.config.UnifiedRoles.AvailableRoles...)) role := unifiedrole.CS3ResourcePermissionsToRole( - availableRoles, + g.availableRoles, permissions, roleCondition, true, diff --git a/services/graph/pkg/service/v0/rolemanagement.go b/services/graph/pkg/service/v0/rolemanagement.go index c97f61a3a3..d5121e7140 100644 --- a/services/graph/pkg/service/v0/rolemanagement.go +++ b/services/graph/pkg/service/v0/rolemanagement.go @@ -14,7 +14,7 @@ import ( // GetRoleDefinitions a list of permission roles than can be used when sharing with users or groups func (g Graph) GetRoleDefinitions(w http.ResponseWriter, r *http.Request) { render.Status(r, http.StatusOK) - render.JSON(w, r, unifiedrole.GetRoles(unifiedrole.RoleFilterIDs(g.config.UnifiedRoles.AvailableRoles...))) + render.JSON(w, r, g.availableRoles) } // GetRoleDefinition a permission role than can be used when sharing with users or groups diff --git a/services/graph/pkg/service/v0/service.go b/services/graph/pkg/service/v0/service.go index 8bf29e4bfc..9881a9bdbd 100644 --- a/services/graph/pkg/service/v0/service.go +++ b/services/graph/pkg/service/v0/service.go @@ -31,6 +31,7 @@ import ( "github.com/opencloud-eu/opencloud/services/graph/pkg/identity" "github.com/opencloud-eu/opencloud/services/graph/pkg/identity/ldap" graphm "github.com/opencloud-eu/opencloud/services/graph/pkg/middleware" + "github.com/opencloud-eu/opencloud/services/graph/pkg/unifiedrole" ) const ( @@ -148,6 +149,7 @@ func NewService(opts ...Option) (Graph, error) { //nolint:maintidx identityCache: identityCache, gatewaySelector: options.GatewaySelector, config: options.Config, + availableRoles: unifiedrole.GetRoles(unifiedrole.RoleFilterIDs(options.Config.UnifiedRoles.AvailableRoles...)), }, mux: m, specialDriveItemsCache: spacePropertiesCache, diff --git a/services/graph/pkg/service/v0/sharedwithme.go b/services/graph/pkg/service/v0/sharedwithme.go index b6f24b027c..0e0076bc17 100644 --- a/services/graph/pkg/service/v0/sharedwithme.go +++ b/services/graph/pkg/service/v0/sharedwithme.go @@ -10,7 +10,6 @@ import ( libregraph "github.com/owncloud/libre-graph-api-go" "github.com/opencloud-eu/opencloud/services/graph/pkg/errorcode" - "github.com/opencloud-eu/opencloud/services/graph/pkg/unifiedrole" ) // ListSharedWithMe lists the files shared with the current user. @@ -40,8 +39,7 @@ func (g Graph) listSharedWithMe(ctx context.Context) ([]libregraph.DriveItem, er g.logger.Error().Err(err).Msg("listing shares failed") return nil, err } - availableRoles := unifiedrole.GetRoles(unifiedrole.RoleFilterIDs(g.config.UnifiedRoles.AvailableRoles...)) - driveItems, err := cs3ReceivedSharesToDriveItems(ctx, g.logger, gatewayClient, g.identityCache, listReceivedSharesResponse.GetShares(), availableRoles) + driveItems, err := cs3ReceivedSharesToDriveItems(ctx, g.logger, gatewayClient, g.identityCache, listReceivedSharesResponse.GetShares(), g.availableRoles) if err != nil { g.logger.Error().Err(err).Msg("could not convert received shares to drive items") return nil, err @@ -53,7 +51,7 @@ func (g Graph) listSharedWithMe(ctx context.Context) ([]libregraph.DriveItem, er g.logger.Error().Err(err).Msg("listing shares failed") return nil, err } - ocmDriveItems, err := cs3ReceivedOCMSharesToDriveItems(ctx, g.logger, gatewayClient, g.identityCache, listReceivedOCMSharesResponse.GetShares(), availableRoles) + ocmDriveItems, err := cs3ReceivedOCMSharesToDriveItems(ctx, g.logger, gatewayClient, g.identityCache, listReceivedOCMSharesResponse.GetShares(), g.availableRoles) if err != nil { g.logger.Error().Err(err).Msg("could not convert received ocm shares to drive items") return nil, err