graph: Use existing helper to get $select values from request

services/graph/mocks/drive_item_permissions_provider.go

@@ -295,7 +295,7 @@ func (_c *DriveItemPermissionsProvider_Invite_Call) RunAndReturn(run func(contex
 }
 
 // ListPermissions provides a mock function with given fields: ctx, itemID, listFederatedRoles, selectedAttrs
-func (_m *DriveItemPermissionsProvider) ListPermissions(ctx context.Context, itemID *providerv1beta1.ResourceId, listFederatedRoles bool, selectedAttrs map[string]struct{}) (libregraph.CollectionOfPermissionsWithAllowedValues, error) {
+func (_m *DriveItemPermissionsProvider) ListPermissions(ctx context.Context, itemID *providerv1beta1.ResourceId, listFederatedRoles bool, selectedAttrs []string) (libregraph.CollectionOfPermissionsWithAllowedValues, error) {
 	ret := _m.Called(ctx, itemID, listFederatedRoles, selectedAttrs)
 
 	if len(ret) == 0 {
@@ -304,16 +304,16 @@ func (_m *DriveItemPermissionsProvider) ListPermissions(ctx context.Context, ite
 
 	var r0 libregraph.CollectionOfPermissionsWithAllowedValues
 	var r1 error
-	if rf, ok := ret.Get(0).(func(context.Context, *providerv1beta1.ResourceId, bool, map[string]struct{}) (libregraph.CollectionOfPermissionsWithAllowedValues, error)); ok {
+	if rf, ok := ret.Get(0).(func(context.Context, *providerv1beta1.ResourceId, bool, []string) (libregraph.CollectionOfPermissionsWithAllowedValues, error)); ok {
 		return rf(ctx, itemID, listFederatedRoles, selectedAttrs)
 	}
-	if rf, ok := ret.Get(0).(func(context.Context, *providerv1beta1.ResourceId, bool, map[string]struct{}) libregraph.CollectionOfPermissionsWithAllowedValues); ok {
+	if rf, ok := ret.Get(0).(func(context.Context, *providerv1beta1.ResourceId, bool, []string) libregraph.CollectionOfPermissionsWithAllowedValues); ok {
 		r0 = rf(ctx, itemID, listFederatedRoles, selectedAttrs)
 	} else {
 		r0 = ret.Get(0).(libregraph.CollectionOfPermissionsWithAllowedValues)
 	}
 
-	if rf, ok := ret.Get(1).(func(context.Context, *providerv1beta1.ResourceId, bool, map[string]struct{}) error); ok {
+	if rf, ok := ret.Get(1).(func(context.Context, *providerv1beta1.ResourceId, bool, []string) error); ok {
 		r1 = rf(ctx, itemID, listFederatedRoles, selectedAttrs)
 	} else {
 		r1 = ret.Error(1)
@@ -331,14 +331,14 @@ type DriveItemPermissionsProvider_ListPermissions_Call struct {
 //   - ctx context.Context
 //   - itemID *providerv1beta1.ResourceId
 //   - listFederatedRoles bool
-//   - selectedAttrs map[string]struct{}
+//   - selectedAttrs []string
 func (_e *DriveItemPermissionsProvider_Expecter) ListPermissions(ctx interface{}, itemID interface{}, listFederatedRoles interface{}, selectedAttrs interface{}) *DriveItemPermissionsProvider_ListPermissions_Call {
 	return &DriveItemPermissionsProvider_ListPermissions_Call{Call: _e.mock.On("ListPermissions", ctx, itemID, listFederatedRoles, selectedAttrs)}
 }
 
-func (_c *DriveItemPermissionsProvider_ListPermissions_Call) Run(run func(ctx context.Context, itemID *providerv1beta1.ResourceId, listFederatedRoles bool, selectedAttrs map[string]struct{})) *DriveItemPermissionsProvider_ListPermissions_Call {
+func (_c *DriveItemPermissionsProvider_ListPermissions_Call) Run(run func(ctx context.Context, itemID *providerv1beta1.ResourceId, listFederatedRoles bool, selectedAttrs []string)) *DriveItemPermissionsProvider_ListPermissions_Call {
 	_c.Call.Run(func(args mock.Arguments) {
-		run(args[0].(context.Context), args[1].(*providerv1beta1.ResourceId), args[2].(bool), args[3].(map[string]struct{}))
+		run(args[0].(context.Context), args[1].(*providerv1beta1.ResourceId), args[2].(bool), args[3].([]string))
 	})
 	return _c
 }
@@ -348,13 +348,13 @@ func (_c *DriveItemPermissionsProvider_ListPermissions_Call) Return(_a0 libregra
 	return _c
 }
 
-func (_c *DriveItemPermissionsProvider_ListPermissions_Call) RunAndReturn(run func(context.Context, *providerv1beta1.ResourceId, bool, map[string]struct{}) (libregraph.CollectionOfPermissionsWithAllowedValues, error)) *DriveItemPermissionsProvider_ListPermissions_Call {
+func (_c *DriveItemPermissionsProvider_ListPermissions_Call) RunAndReturn(run func(context.Context, *providerv1beta1.ResourceId, bool, []string) (libregraph.CollectionOfPermissionsWithAllowedValues, error)) *DriveItemPermissionsProvider_ListPermissions_Call {
 	_c.Call.Return(run)
 	return _c
 }
 
 // ListSpaceRootPermissions provides a mock function with given fields: ctx, driveID, selectedAttrs
-func (_m *DriveItemPermissionsProvider) ListSpaceRootPermissions(ctx context.Context, driveID *providerv1beta1.ResourceId, selectedAttrs map[string]struct{}) (libregraph.CollectionOfPermissionsWithAllowedValues, error) {
+func (_m *DriveItemPermissionsProvider) ListSpaceRootPermissions(ctx context.Context, driveID *providerv1beta1.ResourceId, selectedAttrs []string) (libregraph.CollectionOfPermissionsWithAllowedValues, error) {
 	ret := _m.Called(ctx, driveID, selectedAttrs)
 
 	if len(ret) == 0 {
@@ -363,16 +363,16 @@ func (_m *DriveItemPermissionsProvider) ListSpaceRootPermissions(ctx context.Con
 
 	var r0 libregraph.CollectionOfPermissionsWithAllowedValues
 	var r1 error
-	if rf, ok := ret.Get(0).(func(context.Context, *providerv1beta1.ResourceId, map[string]struct{}) (libregraph.CollectionOfPermissionsWithAllowedValues, error)); ok {
+	if rf, ok := ret.Get(0).(func(context.Context, *providerv1beta1.ResourceId, []string) (libregraph.CollectionOfPermissionsWithAllowedValues, error)); ok {
 		return rf(ctx, driveID, selectedAttrs)
 	}
-	if rf, ok := ret.Get(0).(func(context.Context, *providerv1beta1.ResourceId, map[string]struct{}) libregraph.CollectionOfPermissionsWithAllowedValues); ok {
+	if rf, ok := ret.Get(0).(func(context.Context, *providerv1beta1.ResourceId, []string) libregraph.CollectionOfPermissionsWithAllowedValues); ok {
 		r0 = rf(ctx, driveID, selectedAttrs)
 	} else {
 		r0 = ret.Get(0).(libregraph.CollectionOfPermissionsWithAllowedValues)
 	}
 
-	if rf, ok := ret.Get(1).(func(context.Context, *providerv1beta1.ResourceId, map[string]struct{}) error); ok {
+	if rf, ok := ret.Get(1).(func(context.Context, *providerv1beta1.ResourceId, []string) error); ok {
 		r1 = rf(ctx, driveID, selectedAttrs)
 	} else {
 		r1 = ret.Error(1)
@@ -389,14 +389,14 @@ type DriveItemPermissionsProvider_ListSpaceRootPermissions_Call struct {
 // ListSpaceRootPermissions is a helper method to define mock.On call
 //   - ctx context.Context
 //   - driveID *providerv1beta1.ResourceId
-//   - selectedAttrs map[string]struct{}
+//   - selectedAttrs []string
 func (_e *DriveItemPermissionsProvider_Expecter) ListSpaceRootPermissions(ctx interface{}, driveID interface{}, selectedAttrs interface{}) *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call {
 	return &DriveItemPermissionsProvider_ListSpaceRootPermissions_Call{Call: _e.mock.On("ListSpaceRootPermissions", ctx, driveID, selectedAttrs)}
 }
 
-func (_c *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call) Run(run func(ctx context.Context, driveID *providerv1beta1.ResourceId, selectedAttrs map[string]struct{})) *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call {
+func (_c *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call) Run(run func(ctx context.Context, driveID *providerv1beta1.ResourceId, selectedAttrs []string)) *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call {
 	_c.Call.Run(func(args mock.Arguments) {
-		run(args[0].(context.Context), args[1].(*providerv1beta1.ResourceId), args[2].(map[string]struct{}))
+		run(args[0].(context.Context), args[1].(*providerv1beta1.ResourceId), args[2].([]string))
 	})
 	return _c
 }
@@ -406,7 +406,7 @@ func (_c *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call) Return(_a0
 	return _c
 }
 
-func (_c *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call) RunAndReturn(run func(context.Context, *providerv1beta1.ResourceId, map[string]struct{}) (libregraph.CollectionOfPermissionsWithAllowedValues, error)) *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call {
+func (_c *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call) RunAndReturn(run func(context.Context, *providerv1beta1.ResourceId, []string) (libregraph.CollectionOfPermissionsWithAllowedValues, error)) *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call {
 	_c.Call.Return(run)
 	return _c
 }

services/graph/pkg/service/v0/api_driveitem_permissions.go

@@ -31,6 +31,7 @@ import (
 
 	"github.com/opencloud-eu/opencloud/pkg/l10n"
 	l10n_pkg "github.com/opencloud-eu/opencloud/services/graph/pkg/l10n"
+	"github.com/opencloud-eu/opencloud/services/graph/pkg/odata"
 
 	"github.com/opencloud-eu/opencloud/pkg/conversions"
 	"github.com/opencloud-eu/opencloud/pkg/log"
@@ -51,8 +52,8 @@ const (
 type DriveItemPermissionsProvider interface {
 	Invite(ctx context.Context, resourceId *storageprovider.ResourceId, invite libregraph.DriveItemInvite) (libregraph.Permission, error)
 	SpaceRootInvite(ctx context.Context, driveID *storageprovider.ResourceId, invite libregraph.DriveItemInvite) (libregraph.Permission, error)
-	ListPermissions(ctx context.Context, itemID *storageprovider.ResourceId, listFederatedRoles bool, selectedAttrs map[string]struct{}) (libregraph.CollectionOfPermissionsWithAllowedValues, error)
+	ListPermissions(ctx context.Context, itemID *storageprovider.ResourceId, listFederatedRoles bool, selectedAttrs []string) (libregraph.CollectionOfPermissionsWithAllowedValues, error)
-	ListSpaceRootPermissions(ctx context.Context, driveID *storageprovider.ResourceId, selectedAttrs map[string]struct{}) (libregraph.CollectionOfPermissionsWithAllowedValues, error)
+	ListSpaceRootPermissions(ctx context.Context, driveID *storageprovider.ResourceId, selectedAttrs []string) (libregraph.CollectionOfPermissionsWithAllowedValues, error)
 	DeletePermission(ctx context.Context, itemID *storageprovider.ResourceId, permissionID string) error
 	DeleteSpaceRootPermission(ctx context.Context, driveID *storageprovider.ResourceId, permissionID string) error
 	UpdatePermission(ctx context.Context, itemID *storageprovider.ResourceId, permissionID string, newPermission libregraph.Permission) (libregraph.Permission, error)
@@ -344,7 +345,7 @@ func (s DriveItemPermissionsService) SpaceRootInvite(ctx context.Context, driveI
 }
 
 // ListPermissions lists the permissions of a driveItem
-func (s DriveItemPermissionsService) ListPermissions(ctx context.Context, itemID *storageprovider.ResourceId, listFederatedRoles bool, selectedAttrs map[string]struct{}) (libregraph.CollectionOfPermissionsWithAllowedValues, error) {
+func (s DriveItemPermissionsService) ListPermissions(ctx context.Context, itemID *storageprovider.ResourceId, listFederatedRoles bool, selectedAttrs []string) (libregraph.CollectionOfPermissionsWithAllowedValues, error) {
 	collectionOfPermissions := libregraph.CollectionOfPermissionsWithAllowedValues{}
 	gatewayClient, err := s.gatewaySelector.Next()
 	if err != nil {
@@ -367,11 +368,11 @@ func (s DriveItemPermissionsService) ListPermissions(ctx context.Context, itemID
 
 	collectionOfPermissions = libregraph.CollectionOfPermissionsWithAllowedValues{}
 
-	if _, ok := selectedAttrs["@libre.graph.permissions.actions.allowedValues"]; ok || len(selectedAttrs) == 0 {
+	if len(selectedAttrs) == 0 || slices.Contains(selectedAttrs, "@libre.graph.permissions.actions.allowedValues") {
 		collectionOfPermissions.LibreGraphPermissionsActionsAllowedValues = allowedActions
 	}
 
-	if _, ok := selectedAttrs["@libre.graph.permissions.roles.allowedValues"]; ok || len(selectedAttrs) == 0 {
+	if len(selectedAttrs) == 0 || slices.Contains(selectedAttrs, "@libre.graph.permissions.roles.allowedValues") {
 		collectionOfPermissions.LibreGraphPermissionsRolesAllowedValues = conversions.ToValueSlice(
 			unifiedrole.GetRolesByPermissions(
 				unifiedrole.GetRoles(unifiedrole.RoleFilterIDs(s.config.UnifiedRoles.AvailableRoles...)),
@@ -444,7 +445,7 @@ func (s DriveItemPermissionsService) ListPermissions(ctx context.Context, itemID
 }
 
 // ListSpaceRootPermissions handles ListPermissions request on project spaces
-func (s DriveItemPermissionsService) ListSpaceRootPermissions(ctx context.Context, driveID *storageprovider.ResourceId, selectedAttrs map[string]struct{}) (libregraph.CollectionOfPermissionsWithAllowedValues, error) {
+func (s DriveItemPermissionsService) ListSpaceRootPermissions(ctx context.Context, driveID *storageprovider.ResourceId, selectedAttrs []string) (libregraph.CollectionOfPermissionsWithAllowedValues, error) {
 	collectionOfPermissions := libregraph.CollectionOfPermissionsWithAllowedValues{}
 	gatewayClient, err := s.gatewaySelector.Next()
 	if err != nil {
@@ -722,7 +723,7 @@ func (api DriveItemPermissionsApi) ListPermissions(w http.ResponseWriter, r *htt
 		}
 	}
 
-	selectRoles, err := api.listPermissionsQuerySelectValues(odataReq.Query)
+	selectAttrs, err := odata.GetSelectValues(odataReq.Query)
 	if err != nil {
 		api.logger.Debug().Err(err).Interface("query", r.URL.Query()).Msg("Error parsing ListPermissionRequest: query error")
 		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, err.Error())
@@ -731,7 +732,7 @@ func (api DriveItemPermissionsApi) ListPermissions(w http.ResponseWriter, r *htt
 
 	ctx := r.Context()
 
-	permissions, err := api.driveItemPermissionsService.ListPermissions(ctx, itemID, listFederatedRoles, selectRoles)
+	permissions, err := api.driveItemPermissionsService.ListPermissions(ctx, itemID, listFederatedRoles, selectAttrs)
 	if err != nil {
 		errorcode.RenderError(w, r, err)
 		return
@@ -772,7 +773,7 @@ func (api DriveItemPermissionsApi) ListSpaceRootPermissions(w http.ResponseWrite
 		return
 	}
 
-	selected, err := api.listPermissionsQuerySelectValues(odataReq.Query)
+	selected, err := odata.GetSelectValues(odataReq.Query)
 	if err != nil {
 		api.logger.Debug().Err(err).Interface("query", r.URL.Query()).Msg("Error parsing ListPermissionRequest: query error")
 		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, err.Error())
@@ -936,19 +937,3 @@ func (api DriveItemPermissionsApi) UpdateSpaceRootPermission(w http.ResponseWrit
 	render.Status(r, http.StatusOK)
 	render.JSON(w, r, &updatedPermission)
 }
-
-func (api DriveItemPermissionsApi) listPermissionsQuerySelectValues(odataQuery *godata.GoDataQuery) (map[string]struct{}, error) {
-	selectedAttrs := map[string]struct{}{}
-	if odataQuery.Select != nil {
-		for _, item := range odataQuery.Select.SelectItems {
-			// for now we only support a limitted set of $select attributes
-			if item.Segments[0].Value == "@libre.graph.permissions.roles.allowedValues" || item.Segments[0].Value == "@libre.graph.permissions.actions.allowedValues" {
-				selectedAttrs[item.Segments[0].Value] = struct{}{}
-			} else {
-				api.logger.Debug().Msg("Error parsing ListPermissionRequest: unsupported select item")
-				return selectedAttrs, errorcode.New(errorcode.InvalidRequest, "unsupported select item")
-			}
-		}
-	}
-	return selectedAttrs, nil
-}

services/graph/pkg/service/v0/api_driveitem_permissions_test.go

@@ -385,7 +385,7 @@ var _ = Describe("DriveItemPermissionsService", func() {
 			gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(statResponse, nil)
 			gatewayClient.On("ListShares", mock.Anything, mock.Anything).Return(listSharesResponse, nil)
 			gatewayClient.On("ListPublicShares", mock.Anything, mock.Anything).Return(listPublicSharesResponse, nil)
-			permissions, err := driveItemPermissionsService.ListPermissions(context.Background(), itemID, false, map[string]struct{}{})
+			permissions, err := driveItemPermissionsService.ListPermissions(context.Background(), itemID, false, []string{})
 			Expect(err).ToNot(HaveOccurred())
 			Expect(len(permissions.LibreGraphPermissionsActionsAllowedValues)).ToNot(BeZero())
 			Expect(len(permissions.LibreGraphPermissionsRolesAllowedValues)).ToNot(BeZero())
@@ -433,7 +433,7 @@ var _ = Describe("DriveItemPermissionsService", func() {
 			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, map[string]struct{}{})
+			permissions, err := driveItemPermissionsService.ListPermissions(context.Background(), itemID, false, []string{})
 			Expect(err).ToNot(HaveOccurred())
 			Expect(len(permissions.LibreGraphPermissionsActionsAllowedValues)).ToNot(BeZero())
 			Expect(len(permissions.LibreGraphPermissionsRolesAllowedValues)).ToNot(BeZero())
@@ -472,7 +472,7 @@ var _ = Describe("DriveItemPermissionsService", func() {
 			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 := service.ListPermissions(context.Background(), itemID, false, map[string]struct{}{})
+			permissions, err := service.ListPermissions(context.Background(), itemID, false, []string{})
 			Expect(err).ToNot(HaveOccurred())
 			Expect(len(permissions.LibreGraphPermissionsActionsAllowedValues)).ToNot(BeZero())
 			Expect(len(permissions.LibreGraphPermissionsRolesAllowedValues)).ToNot(BeZero())
@@ -508,7 +508,7 @@ var _ = Describe("DriveItemPermissionsService", func() {
 			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, map[string]struct{}{})
+			permissions, err := driveItemPermissionsService.ListPermissions(context.Background(), itemID, false, []string{})
 			Expect(err).ToNot(HaveOccurred())
 			Expect(len(permissions.LibreGraphPermissionsActionsAllowedValues)).ToNot(BeZero())
 			Expect(len(permissions.LibreGraphPermissionsRolesAllowedValues)).ToNot(BeZero())
@@ -555,7 +555,7 @@ var _ = Describe("DriveItemPermissionsService", func() {
 			gatewayClient.On("ListPublicShares", mock.Anything, mock.Anything).Return(listPublicSharesResponse, nil)
 			statResponse.Info.Id = listSpacesResponse.StorageSpaces[0].Root
 			gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(statResponse, nil)
-			permissions, err := driveItemPermissionsService.ListSpaceRootPermissions(context.Background(), driveId, map[string]struct{}{})
+			permissions, err := driveItemPermissionsService.ListSpaceRootPermissions(context.Background(), driveId, []string{})
 			Expect(err).ToNot(HaveOccurred())
 			Expect(len(permissions.LibreGraphPermissionsActionsAllowedValues)).ToNot(BeZero())
 		})
@@ -1268,7 +1268,7 @@ var _ = Describe("DriveItemPermissionsApi", func() {
 			Expect(err).ToNot(HaveOccurred())
 
 			mockProvider.On("ListPermissions", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
-				Return(func(ctx context.Context, itemid *provider.ResourceId, listFederatedRoles bool, selected map[string]struct{}) (libregraph.CollectionOfPermissionsWithAllowedValues, error) {
+				Return(func(ctx context.Context, itemid *provider.ResourceId, listFederatedRoles bool, selected []string) (libregraph.CollectionOfPermissionsWithAllowedValues, error) {
 					Expect(listFederatedRoles).To(Equal(false))
 					Expect(storagespace.FormatResourceID(itemid)).To(Equal("1$2!3"))
 					return libregraph.CollectionOfPermissionsWithAllowedValues{}, nil