From 376754b59e59f843eb60bd3a28065c8f6744e459 Mon Sep 17 00:00:00 2001 From: Andre Duffeck Date: Fri, 28 Jan 2022 15:32:48 +0100 Subject: [PATCH] Make owncloudsql spaces aware (#2472) * Add ListStorages method * Implement ListStorageSpaces in owncloudsql * ResourceInfos do no longer contain the full path but only the basename * Handle references relative to a root * Fix space lookup, extract space functionality into a separate file * Use oc_mounts to find the storage roots This way it also works with storages with hashed IDs (that happens when the id exceeds a certain length). * Fix shares * Implement GetPath, fix GetPathById * Include the storage id when listing shares * Fix accepting declined shares * Ignore setting-grants-not-supported errors, storage grants are optional * Add changelog * Fix missing storage id from resource info * Fix field mask * Do not log error messages for unsupported grant calls * Fix hound issue * Fix changelog URL * Fix linter issue * Remove unfinished GetPath() code * Adapt expected failures * Cache user lookups in the oc10-sql share manager That leads to a massive performance boost. --- .../make-owncloudsql-spaces-aware.md | 5 + .../mocks/GatewayClient.go | 30 +++ .../sharesstorageprovider.go | 1 + .../storageprovider/storageprovider.go | 15 ++ .../sharing/shares/mocks/GatewayClient.go | 32 +++- .../handlers/apps/sharing/shares/pending.go | 16 +- .../apps/sharing/shares/pending_test.go | 6 +- .../handlers/apps/sharing/shares/shares.go | 3 +- pkg/share/manager/sql/conversions.go | 30 ++- pkg/share/manager/sql/sql.go | 24 ++- pkg/share/manager/sql/sql_test.go | 22 ++- .../fs/owncloudsql/filecache/filecache.go | 124 ++++++++---- .../owncloudsql/filecache/filecache_test.go | 52 +++++ pkg/storage/fs/owncloudsql/owncloudsql.go | 29 +-- pkg/storage/fs/owncloudsql/spaces.go | 179 ++++++++++++++++++ .../expected-failures-on-OCIS-storage.md | 6 + .../expected-failures-on-S3NG-storage.md | 6 + 17 files changed, 497 insertions(+), 83 deletions(-) create mode 100644 changelog/unreleased/make-owncloudsql-spaces-aware.md create mode 100644 pkg/storage/fs/owncloudsql/spaces.go diff --git a/changelog/unreleased/make-owncloudsql-spaces-aware.md b/changelog/unreleased/make-owncloudsql-spaces-aware.md new file mode 100644 index 0000000000..0039893dd7 --- /dev/null +++ b/changelog/unreleased/make-owncloudsql-spaces-aware.md @@ -0,0 +1,5 @@ +Bugfix: make owncloudsql work with the spaces registry + +owncloudsql now works properly with the spaces registry. + +https://github.com/cs3org/reva/pull/2372 diff --git a/internal/grpc/services/sharesstorageprovider/mocks/GatewayClient.go b/internal/grpc/services/sharesstorageprovider/mocks/GatewayClient.go index 3ebf3a3c1a..5538440905 100644 --- a/internal/grpc/services/sharesstorageprovider/mocks/GatewayClient.go +++ b/internal/grpc/services/sharesstorageprovider/mocks/GatewayClient.go @@ -96,6 +96,36 @@ func (_m *GatewayClient) Delete(ctx context.Context, in *providerv1beta1.DeleteR return r0, r1 } +// GetPath provides a mock function with given fields: ctx, in, opts +func (_m *GatewayClient) GetPath(ctx context.Context, in *providerv1beta1.GetPathRequest, opts ...grpc.CallOption) (*providerv1beta1.GetPathResponse, error) { + _va := make([]interface{}, len(opts)) + for _i := range opts { + _va[_i] = opts[_i] + } + var _ca []interface{} + _ca = append(_ca, ctx, in) + _ca = append(_ca, _va...) + ret := _m.Called(_ca...) + + var r0 *providerv1beta1.GetPathResponse + if rf, ok := ret.Get(0).(func(context.Context, *providerv1beta1.GetPathRequest, ...grpc.CallOption) *providerv1beta1.GetPathResponse); ok { + r0 = rf(ctx, in, opts...) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*providerv1beta1.GetPathResponse) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, *providerv1beta1.GetPathRequest, ...grpc.CallOption) error); ok { + r1 = rf(ctx, in, opts...) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // InitiateFileDownload provides a mock function with given fields: ctx, req, opts func (_m *GatewayClient) InitiateFileDownload(ctx context.Context, req *providerv1beta1.InitiateFileDownloadRequest, opts ...grpc.CallOption) (*gatewayv1beta1.InitiateFileDownloadResponse, error) { _va := make([]interface{}, len(opts)) diff --git a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go index d0a17f138f..2aae3fd8bf 100644 --- a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go +++ b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go @@ -50,6 +50,7 @@ import ( // GatewayClient describe the interface of a gateway client type GatewayClient interface { + GetPath(ctx context.Context, in *provider.GetPathRequest, opts ...grpc.CallOption) (*provider.GetPathResponse, error) Stat(ctx context.Context, in *provider.StatRequest, opts ...grpc.CallOption) (*provider.StatResponse, error) Move(ctx context.Context, in *provider.MoveRequest, opts ...grpc.CallOption) (*provider.MoveResponse, error) Delete(ctx context.Context, in *provider.DeleteRequest, opts ...grpc.CallOption) (*provider.DeleteResponse, error) diff --git a/internal/grpc/services/storageprovider/storageprovider.go b/internal/grpc/services/storageprovider/storageprovider.go index 6815669964..8219d2178e 100644 --- a/internal/grpc/services/storageprovider/storageprovider.go +++ b/internal/grpc/services/storageprovider/storageprovider.go @@ -1090,6 +1090,11 @@ func (s *service) DenyGrant(ctx context.Context, req *provider.DenyGrantRequest) if err != nil { var st *rpc.Status switch err.(type) { + case errtypes.NotSupported: + // ignore - setting storage grants is optional + return &provider.DenyGrantResponse{ + Status: status.NewOK(ctx), + }, nil case errtypes.IsNotFound: st = status.NewNotFound(ctx, "path not found when setting grants") case errtypes.PermissionDenied: @@ -1135,6 +1140,11 @@ func (s *service) AddGrant(ctx context.Context, req *provider.AddGrantRequest) ( if err != nil { var st *rpc.Status switch err.(type) { + case errtypes.NotSupported: + // ignore - setting storage grants is optional + return &provider.AddGrantResponse{ + Status: status.NewOK(ctx), + }, nil case errtypes.IsNotFound: st = status.NewNotFound(ctx, "path not found when setting grants") case errtypes.PermissionDenied: @@ -1170,6 +1180,11 @@ func (s *service) UpdateGrant(ctx context.Context, req *provider.UpdateGrantRequ if err := s.storage.UpdateGrant(ctx, req.Ref, req.Grant); err != nil { var st *rpc.Status switch err.(type) { + case errtypes.NotSupported: + // ignore - setting storage grants is optional + return &provider.UpdateGrantResponse{ + Status: status.NewOK(ctx), + }, nil case errtypes.IsNotFound: st = status.NewNotFound(ctx, "path not found when updating grant") case errtypes.PermissionDenied: diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/mocks/GatewayClient.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/mocks/GatewayClient.go index d802734e9e..b0aaf7962e 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/mocks/GatewayClient.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/mocks/GatewayClient.go @@ -16,7 +16,7 @@ // granted to it by virtue of its status as an Intergovernmental Organization // or submit itself to any jurisdiction. -// Code generated by mockery v1.1.2. DO NOT EDIT. +// Code generated by mockery v1.0.0. DO NOT EDIT. package mocks @@ -193,6 +193,36 @@ func (_m *GatewayClient) GetPath(ctx context.Context, in *providerv1beta1.GetPat return r0, r1 } +// GetReceivedShare provides a mock function with given fields: ctx, in, opts +func (_m *GatewayClient) GetReceivedShare(ctx context.Context, in *collaborationv1beta1.GetReceivedShareRequest, opts ...grpc.CallOption) (*collaborationv1beta1.GetReceivedShareResponse, error) { + _va := make([]interface{}, len(opts)) + for _i := range opts { + _va[_i] = opts[_i] + } + var _ca []interface{} + _ca = append(_ca, ctx, in) + _ca = append(_ca, _va...) + ret := _m.Called(_ca...) + + var r0 *collaborationv1beta1.GetReceivedShareResponse + if rf, ok := ret.Get(0).(func(context.Context, *collaborationv1beta1.GetReceivedShareRequest, ...grpc.CallOption) *collaborationv1beta1.GetReceivedShareResponse); ok { + r0 = rf(ctx, in, opts...) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*collaborationv1beta1.GetReceivedShareResponse) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, *collaborationv1beta1.GetReceivedShareRequest, ...grpc.CallOption) error); ok { + r1 = rf(ctx, in, opts...) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // GetShare provides a mock function with given fields: ctx, in, opts func (_m *GatewayClient) GetShare(ctx context.Context, in *collaborationv1beta1.GetShareRequest, opts ...grpc.CallOption) (*collaborationv1beta1.GetShareResponse, error) { _va := make([]interface{}, len(opts)) diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending.go index 9bc66874a2..8f74cb08c6 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending.go @@ -52,13 +52,13 @@ func (h *Handler) AcceptReceivedShare(w http.ResponseWriter, r *http.Request) { return } - share, ocsResponse := getShareFromID(ctx, client, shareID) + rs, ocsResponse := getReceivedShareFromID(ctx, client, shareID) if ocsResponse != nil { response.WriteOCSResponse(w, r, *ocsResponse, nil) return } - sharedResource, ocsResponse := getSharedResource(ctx, client, share) + sharedResource, ocsResponse := getSharedResource(ctx, client, rs.Share.Share.ResourceId) if ocsResponse != nil { response.WriteOCSResponse(w, r, *ocsResponse, nil) return @@ -76,7 +76,7 @@ func (h *Handler) AcceptReceivedShare(w http.ResponseWriter, r *http.Request) { var mountPoints []string sharesToAccept := map[string]bool{shareID: true} for _, s := range lrs.Shares { - if utils.ResourceIDEqual(s.Share.ResourceId, share.Share.GetResourceId()) { + if utils.ResourceIDEqual(s.Share.ResourceId, rs.Share.Share.GetResourceId()) { if s.State == collaboration.ShareState_SHARE_STATE_ACCEPTED { mount = s.MountPoint.Path } else { @@ -183,9 +183,9 @@ func (h *Handler) updateReceivedShare(w http.ResponseWriter, r *http.Request, sh response.WriteOCSSuccess(w, r, []*conversions.ShareData{data}) } -// getShareFromID uses a client to the gateway to fetch a share based on its ID. -func getShareFromID(ctx context.Context, client GatewayClient, shareID string) (*collaboration.GetShareResponse, *response.Response) { - s, err := client.GetShare(ctx, &collaboration.GetShareRequest{ +// getReceivedShareFromID uses a client to the gateway to fetch a share based on its ID. +func getReceivedShareFromID(ctx context.Context, client GatewayClient, shareID string) (*collaboration.GetReceivedShareResponse, *response.Response) { + s, err := client.GetReceivedShare(ctx, &collaboration.GetReceivedShareRequest{ Ref: &collaboration.ShareReference{ Spec: &collaboration.ShareReference_Id{ Id: &collaboration.ShareId{ @@ -213,10 +213,10 @@ func getShareFromID(ctx context.Context, client GatewayClient, shareID string) ( } // getSharedResource attempts to get a shared resource from the storage from the resource reference. -func getSharedResource(ctx context.Context, client GatewayClient, share *collaboration.GetShareResponse) (*provider.StatResponse, *response.Response) { +func getSharedResource(ctx context.Context, client GatewayClient, resID *provider.ResourceId) (*provider.StatResponse, *response.Response) { res, err := client.Stat(ctx, &provider.StatRequest{ Ref: &provider.Reference{ - ResourceId: share.Share.GetResourceId(), + ResourceId: resID, }, }) if err != nil { diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending_test.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending_test.go index 1412d19af5..8f130da31d 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending_test.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending_test.go @@ -115,9 +115,11 @@ var _ = Describe("The ocs API", func() { ) BeforeEach(func() { - client.On("GetShare", mock.Anything, mock.Anything).Return(&collaboration.GetShareResponse{ + client.On("GetReceivedShare", mock.Anything, mock.Anything).Return(&collaboration.GetReceivedShareResponse{ Status: status.NewOK(context.Background()), - Share: share, + Share: &collaboration.ReceivedShare{ + Share: share, + }, }, nil) client.On("Stat", mock.Anything, mock.Anything).Return(&provider.StatResponse{ diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go index a5f0ab4bf0..ab98033537 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go @@ -117,6 +117,7 @@ type GatewayClient interface { ListShares(ctx context.Context, in *collaboration.ListSharesRequest, opts ...grpc.CallOption) (*collaboration.ListSharesResponse, error) GetShare(ctx context.Context, in *collaboration.GetShareRequest, opts ...grpc.CallOption) (*collaboration.GetShareResponse, error) + GetReceivedShare(ctx context.Context, in *collaboration.GetReceivedShareRequest, opts ...grpc.CallOption) (*collaboration.GetReceivedShareResponse, error) CreateShare(ctx context.Context, in *collaboration.CreateShareRequest, opts ...grpc.CallOption) (*collaboration.CreateShareResponse, error) RemoveShare(ctx context.Context, in *collaboration.RemoveShareRequest, opts ...grpc.CallOption) (*collaboration.RemoveShareResponse, error) ListReceivedShares(ctx context.Context, in *collaboration.ListReceivedSharesRequest, opts ...grpc.CallOption) (*collaboration.ListReceivedSharesResponse, error) @@ -329,7 +330,7 @@ func (h *Handler) CreateShare(w http.ResponseWriter, r *http.Request) { MountPoint: s.MountPoint, State: collaboration.ShareState_SHARE_STATE_ACCEPTED, }, - UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"state, mount_point"}}, + UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"state", "mount_point"}}, } shareRes, err := client.UpdateReceivedShare(granteeCtx, updateRequest) diff --git a/pkg/share/manager/sql/conversions.go b/pkg/share/manager/sql/conversions.go index 28c28f3971..84772f658f 100644 --- a/pkg/share/manager/sql/conversions.go +++ b/pkg/share/manager/sql/conversions.go @@ -21,7 +21,9 @@ package sql import ( "context" "strings" + "time" + "github.com/ReneKroon/ttlcache/v2" grouppb "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" userprovider "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" @@ -65,17 +67,34 @@ type UserConverter interface { // GatewayUserConverter converts usernames and ids using the gateway type GatewayUserConverter struct { gwAddr string + + IDCache *ttlcache.Cache + NameCache *ttlcache.Cache } // NewGatewayUserConverter returns a instance of GatewayUserConverter func NewGatewayUserConverter(gwAddr string) *GatewayUserConverter { + IDCache := ttlcache.NewCache() + _ = IDCache.SetTTL(30 * time.Second) + IDCache.SkipTTLExtensionOnHit(true) + NameCache := ttlcache.NewCache() + _ = NameCache.SetTTL(30 * time.Second) + NameCache.SkipTTLExtensionOnHit(true) + return &GatewayUserConverter{ - gwAddr: gwAddr, + gwAddr: gwAddr, + IDCache: IDCache, + NameCache: NameCache, } } // UserIDToUserName converts a user ID to an username func (c *GatewayUserConverter) UserIDToUserName(ctx context.Context, userid *userpb.UserId) (string, error) { + username, err := c.NameCache.Get(userid.String()) + if err == nil { + return username.(string), nil + } + gwConn, err := pool.GetGatewayServiceClient(c.gwAddr) if err != nil { return "", err @@ -89,11 +108,17 @@ func (c *GatewayUserConverter) UserIDToUserName(ctx context.Context, userid *use if getUserResponse.Status.Code != rpc.Code_CODE_OK { return "", status.NewErrorFromCode(getUserResponse.Status.Code, "gateway") } + _ = c.NameCache.Set(userid.String(), getUserResponse.User.Username) return getUserResponse.User.Username, nil } // UserNameToUserID converts a username to an user ID func (c *GatewayUserConverter) UserNameToUserID(ctx context.Context, username string) (*userpb.UserId, error) { + id, err := c.IDCache.Get(username) + if err == nil { + return id.(*userpb.UserId), nil + } + gwConn, err := pool.GetGatewayServiceClient(c.gwAddr) if err != nil { return nil, err @@ -108,6 +133,7 @@ func (c *GatewayUserConverter) UserNameToUserID(ctx context.Context, username st if getUserResponse.Status.Code != rpc.Code_CODE_OK { return nil, status.NewErrorFromCode(getUserResponse.Status.Code, "gateway") } + _ = c.IDCache.Set(username, getUserResponse.User.Id) return getUserResponse.User.Id, nil } @@ -233,7 +259,7 @@ func (m *mgr) convertToCS3Share(ctx context.Context, s DBShare, storageMountID s OpaqueId: s.ID, }, ResourceId: &provider.ResourceId{ - StorageId: storageMountID + "!" + s.ItemStorage, + StorageId: s.ItemStorage, OpaqueId: s.FileSource, }, Permissions: &collaboration.SharePermissions{Permissions: permissions}, diff --git a/pkg/share/manager/sql/sql.go b/pkg/share/manager/sql/sql.go index 74b31d7bb8..06ac74bb52 100644 --- a/pkg/share/manager/sql/sql.go +++ b/pkg/share/manager/sql/sql.go @@ -277,7 +277,15 @@ func (m *mgr) UpdateShare(ctx context.Context, ref *collaboration.ShareReference func (m *mgr) ListShares(ctx context.Context, filters []*collaboration.Filter) ([]*collaboration.Share, error) { uid := ctxpkg.ContextMustGetUser(ctx).Username - query := "select coalesce(uid_owner, '') as uid_owner, coalesce(uid_initiator, '') as uid_initiator, coalesce(share_with, '') as share_with, coalesce(file_source, '') as file_source, file_target, id, stime, permissions, share_type FROM oc_share WHERE (uid_owner=? or uid_initiator=?)" + query := ` + SELECT + coalesce(s.uid_owner, '') as uid_owner, coalesce(s.uid_initiator, '') as uid_initiator, + coalesce(s.share_with, '') as share_with, coalesce(s.file_source, '') as file_source, + s.file_target, s.id, s.stime, s.permissions, s.share_type, fc.storage as storage + FROM oc_share s + LEFT JOIN oc_filecache fc ON fc.fileid = file_source + WHERE (uid_owner=? or uid_initiator=?) + ` params := []interface{}{uid, uid} var ( @@ -310,7 +318,7 @@ func (m *mgr) ListShares(ctx context.Context, filters []*collaboration.Filter) ( var s DBShare shares := []*collaboration.Share{} for rows.Next() { - if err := rows.Scan(&s.UIDOwner, &s.UIDInitiator, &s.ShareWith, &s.FileSource, &s.FileTarget, &s.ID, &s.STime, &s.Permissions, &s.ShareType); err != nil { + if err := rows.Scan(&s.UIDOwner, &s.UIDInitiator, &s.ShareWith, &s.FileSource, &s.FileTarget, &s.ID, &s.STime, &s.Permissions, &s.ShareType, &s.ItemStorage); err != nil { continue } share, err := m.convertToCS3Share(ctx, s, m.storageMountID) @@ -357,7 +365,7 @@ func (m *mgr) ListReceivedShares(ctx context.Context, filters []*collaboration.F ) SELECT COALESCE(r.uid_owner, '') AS uid_owner, COALESCE(r.uid_initiator, '') AS uid_initiator, COALESCE(r.share_with, '') AS share_with, COALESCE(r.file_source, '') AS file_source, COALESCE(r2.file_target, r.file_target), r.id, r.stime, r.permissions, r.share_type, COALESCE(r2.accepted, r.accepted), - r.numeric_id, COALESCE(r.parent, -1) AS parent FROM results r LEFT JOIN results r2 ON r.id = r2.parent WHERE r.parent IS NULL;` + r.numeric_id, COALESCE(r.parent, -1) AS parent FROM results r LEFT JOIN results r2 ON r.id = r2.parent WHERE r.parent IS NULL` filterQuery, filterParams, err := translateFilters(filters) if err != nil { @@ -368,7 +376,7 @@ func (m *mgr) ListReceivedShares(ctx context.Context, filters []*collaboration.F if filterQuery != "" { query = fmt.Sprintf("%s AND (%s)", query, filterQuery) } - + query += ";" rows, err := m.db.Query(query, params...) if err != nil { return nil, err @@ -543,7 +551,7 @@ func (m *mgr) getReceivedByID(ctx context.Context, id *collaboration.ShareId) (* SELECT s.*, storages.numeric_id FROM oc_share s LEFT JOIN oc_storages storages ON ` + homeConcat + ` - WHERE s.id=? OR s.parent=?` + userSelect + ` + WHERE s.id=? OR s.parent=? ` + userSelect + ` ) SELECT COALESCE(r.uid_owner, '') AS uid_owner, COALESCE(r.uid_initiator, '') AS uid_initiator, COALESCE(r.share_with, '') AS share_with, COALESCE(r.file_source, '') AS file_source, COALESCE(r2.file_target, r.file_target), r.id, r.stime, r.permissions, r.share_type, COALESCE(r2.accepted, r.accepted), @@ -621,7 +629,7 @@ func translateFilters(filters []*collaboration.Filter) (string, []interface{}, e case collaboration.Filter_TYPE_RESOURCE_ID: filterQuery += "(" for i, f := range filters { - filterQuery += "item_source=?" + filterQuery += "file_source=?" params = append(params, f.GetResourceId().OpaqueId) if i != len(filters)-1 { @@ -632,7 +640,7 @@ func translateFilters(filters []*collaboration.Filter) (string, []interface{}, e case collaboration.Filter_TYPE_GRANTEE_TYPE: filterQuery += "(" for i, f := range filters { - filterQuery += "share_type=?" + filterQuery += "r.share_type=?" params = append(params, granteeTypeToShareType(f.GetGranteeType())) if i != len(filters)-1 { @@ -642,7 +650,7 @@ func translateFilters(filters []*collaboration.Filter) (string, []interface{}, e filterQuery += ")" case collaboration.Filter_TYPE_EXCLUDE_DENIALS: // TODO this may change once the mapping of permission to share types is completed (cf. pkg/cbox/utils/conversions.go) - filterQuery += "permissions > 0" + filterQuery += "r.permissions > 0" default: return "", nil, fmt.Errorf("filter type is not supported") } diff --git a/pkg/share/manager/sql/sql_test.go b/pkg/share/manager/sql/sql_test.go index 24d20ede45..4f327ed6ad 100644 --- a/pkg/share/manager/sql/sql_test.go +++ b/pkg/share/manager/sql/sql_test.go @@ -201,12 +201,19 @@ var _ = Describe("SQL manager", func() { shares, err := mgr.ListShares(ctx, []*collaboration.Filter{}) Expect(err).ToNot(HaveOccurred()) Expect(len(shares)).To(Equal(1)) + share := shares[0] + Expect(share.ResourceId.StorageId).To(Equal("1")) shares, err = mgr.ListShares(ctx, []*collaboration.Filter{ - share.ResourceIDFilter(&provider.ResourceId{ - StorageId: "/", - OpaqueId: "somethingElse", - }), + { + Type: collaboration.Filter_TYPE_RESOURCE_ID, + Term: &collaboration.Filter_ResourceId{ + ResourceId: &provider.ResourceId{ + StorageId: "/", + OpaqueId: "somethingElse", + }, + }, + }, }) Expect(err).ToNot(HaveOccurred()) Expect(len(shares)).To(Equal(0)) @@ -338,6 +345,13 @@ var _ = Describe("SQL manager", func() { Expect(err).ToNot(HaveOccurred()) Expect(len(shares)).To(Equal(1)) }) + + It("works with filters", func() { + loginAs(otherUser) + shares, err := mgr.ListReceivedShares(ctx, []*collaboration.Filter{{Type: collaboration.Filter_TYPE_EXCLUDE_DENIALS}}) + Expect(err).ToNot(HaveOccurred()) + Expect(len(shares)).To(Equal(1)) + }) }) Describe("GetReceivedShare", func() { diff --git a/pkg/storage/fs/owncloudsql/filecache/filecache.go b/pkg/storage/fs/owncloudsql/filecache/filecache.go index 7e0c58a1d8..49cb044c8e 100644 --- a/pkg/storage/fs/owncloudsql/filecache/filecache.go +++ b/pkg/storage/fs/owncloudsql/filecache/filecache.go @@ -42,6 +42,46 @@ type Cache struct { db *sql.DB } +// Storage represents a storage entry in the database +type Storage struct { + ID string + NumericID int +} + +// File represents an entry of the file cache +type File struct { + ID int + Storage int + Parent int + MimePart int + MimeType int + MimeTypeString string + Size int + MTime int + StorageMTime int + UnencryptedSize int + Permissions int + Encrypted bool + Path string + Name string + Etag string + Checksum string +} + +// TrashItem represents a trash item of the file cache +type TrashItem struct { + ID int + Name string + User string + Path string + Timestamp int +} + +// Scannable describes the interface providing a Scan method +type Scannable interface { + Scan(...interface{}) error +} + // NewMysql returns a new Cache instance connecting to a MySQL database func NewMysql(dsn string) (*Cache, error) { sqldb, err := sql.Open("mysql", dsn) @@ -68,6 +108,56 @@ func New(driver string, sqldb *sql.DB) (*Cache, error) { }, nil } +// ListStorages returns the list of numeric ids of all storages +// Optionally only home storages are considered +func (c *Cache) ListStorages(onlyHome bool) ([]*Storage, error) { + query := "" + if onlyHome { + mountPointConcat := "" + if c.driver == "mysql" { + mountPointConcat = "m.mount_point = CONCAT('/', m.user_id, '/')" + } else { // sqlite3 + mountPointConcat = "m.mount_point = '/' || m.user_id || '/'" + } + + query = "SELECT s.id, s.numeric_id FROM oc_storages s JOIN oc_mounts m ON s.numeric_id = m.storage_id WHERE " + mountPointConcat + } else { + query = "SELECT id, numeric_id FROM oc_storages" + } + rows, err := c.db.Query(query) + if err != nil { + return nil, err + } + defer rows.Close() + + storages := []*Storage{} + for rows.Next() { + storage := &Storage{} + err := rows.Scan(&storage.ID, &storage.NumericID) + if err != nil { + return nil, err + } + storages = append(storages, storage) + } + return storages, nil +} + +// GetStorage returns the storage with the given numeric id +func (c *Cache) GetStorage(numeridID interface{}) (*Storage, error) { + numericID, err := toIntID(numeridID) + if err != nil { + return nil, err + } + row := c.db.QueryRow("SELECT id, numeric_id FROM oc_storages WHERE numeric_id = ?", numericID) + s := &Storage{} + switch err := row.Scan(&s.ID, &s.NumericID); err { + case nil: + return s, nil + default: + return nil, err + } +} + // GetNumericStorageID returns the database id for the given storage func (c *Cache) GetNumericStorageID(id string) (int, error) { row := c.db.QueryRow("SELECT numeric_id FROM oc_storages WHERE id = ?", id) @@ -153,40 +243,6 @@ func (c *Cache) GetStorageOwnerByFileID(numericID interface{}) (string, error) { } } -// File represents an entry of the file cache -type File struct { - ID int - Storage int - Parent int - MimePart int - MimeType int - MimeTypeString string - Size int - MTime int - StorageMTime int - UnencryptedSize int - Permissions int - Encrypted bool - Path string - Name string - Etag string - Checksum string -} - -// TrashItem represents a trash item of the file cache -type TrashItem struct { - ID int - Name string - User string - Path string - Timestamp int -} - -// Scannable describes the interface providing a Scan method -type Scannable interface { - Scan(...interface{}) error -} - func (c *Cache) rowToFile(row Scannable) (*File, error) { var fileid, storage, parent, mimetype, mimepart, size, mtime, storageMtime, encrypted, unencryptedSize int var permissions sql.NullInt32 diff --git a/pkg/storage/fs/owncloudsql/filecache/filecache_test.go b/pkg/storage/fs/owncloudsql/filecache/filecache_test.go index c1c383aa58..4a6eab0438 100644 --- a/pkg/storage/fs/owncloudsql/filecache/filecache_test.go +++ b/pkg/storage/fs/owncloudsql/filecache/filecache_test.go @@ -63,6 +63,58 @@ var _ = Describe("Filecache", func() { os.Remove(testDbFile.Name()) }) + Describe("ListStorages", func() { + It("returns all storages", func() { + storages, err := cache.ListStorages(false) + Expect(err).ToNot(HaveOccurred()) + Expect(len(storages)).To(Equal(2)) + ids := []string{} + numericIDs := []int{} + for _, s := range storages { + ids = append(ids, s.ID) + numericIDs = append(numericIDs, s.NumericID) + } + Expect(numericIDs).To(ConsistOf([]int{1, 2})) + Expect(ids).To(ConsistOf([]string{"home::admin", "local::/mnt/data/files/"})) + }) + + It("returns all home storages", func() { + storages, err := cache.ListStorages(true) + Expect(err).ToNot(HaveOccurred()) + Expect(len(storages)).To(Equal(1)) + Expect(storages[0].ID).To(Equal("home::admin")) + Expect(storages[0].NumericID).To(Equal(1)) + }) + }) + + Describe("GetStorage", func() { + It("returns an error when the id is invalid", func() { + s, err := cache.GetStorage("foo") + Expect(err).To(HaveOccurred()) + Expect(s).To(BeNil()) + }) + + It("returns an error when the id doesn't exist", func() { + s, err := cache.GetStorage(100) + Expect(err).To(HaveOccurred()) + Expect(s).To(BeNil()) + }) + + It("returns the storage", func() { + s, err := cache.GetStorage(1) + Expect(err).ToNot(HaveOccurred()) + Expect(s.ID).To(Equal("home::admin")) + Expect(s.NumericID).To(Equal(1)) + }) + + It("takes string ids", func() { + s, err := cache.GetStorage("1") + Expect(err).ToNot(HaveOccurred()) + Expect(s.ID).To(Equal("home::admin")) + Expect(s.NumericID).To(Equal(1)) + }) + }) + Describe("GetNumericStorageID", func() { It("returns the proper storage id", func() { nid, err := cache.GetNumericStorageID("home::admin") diff --git a/pkg/storage/fs/owncloudsql/owncloudsql.go b/pkg/storage/fs/owncloudsql/owncloudsql.go index b9579db28d..d0fe1d39cd 100644 --- a/pkg/storage/fs/owncloudsql/owncloudsql.go +++ b/pkg/storage/fs/owncloudsql/owncloudsql.go @@ -489,11 +489,6 @@ func (fs *owncloudsqlfs) getUserStorage(user string) (int, error) { return id, err } -// CreateStorageSpace creates a storage space -func (fs *owncloudsqlfs) CreateStorageSpace(ctx context.Context, req *provider.CreateStorageSpaceRequest) (*provider.CreateStorageSpaceResponse, error) { - return nil, errtypes.NotSupported("unimplemented: CreateStorageSpace") -} - func (fs *owncloudsqlfs) convertToResourceInfo(ctx context.Context, entry *filecache.File, ip string, mdKeys []string) (*provider.ResourceInfo, error) { mdKeysMap := make(map[string]struct{}) for _, k := range mdKeys { @@ -507,8 +502,8 @@ func (fs *owncloudsqlfs) convertToResourceInfo(ctx context.Context, entry *filec isDir := entry.MimeTypeString == "httpd/unix-directory" ri := &provider.ResourceInfo{ - Id: &provider.ResourceId{OpaqueId: strconv.Itoa(entry.ID)}, - Path: fs.toStoragePath(ctx, ip), + Id: &provider.ResourceId{StorageId: strconv.Itoa(entry.Storage), OpaqueId: strconv.Itoa(entry.ID)}, + Path: filepath.Base(ip), Type: getResourceType(isDir), Etag: entry.Etag, MimeType: entry.MimeTypeString, @@ -544,7 +539,7 @@ func (fs *owncloudsqlfs) convertToResourceInfo(ctx context.Context, entry *filec // GetPathByID returns the storage relative path for the file id, without the internal namespace func (fs *owncloudsqlfs) GetPathByID(ctx context.Context, id *provider.ResourceId) (string, error) { - ip, err := fs.filecache.Path(id.OpaqueId) + ip, err := fs.resolve(ctx, &provider.Reference{ResourceId: id}) if err != nil { return "", err } @@ -580,6 +575,9 @@ func (fs *owncloudsqlfs) resolve(ctx context.Context, ref *provider.Reference) ( } p = filepath.Join(owner, p) } + if ref.GetPath() != "" { + p = filepath.Join(p, ref.GetPath()) + } return fs.toInternalPath(ctx, p), nil } @@ -1950,21 +1948,6 @@ func (fs *owncloudsqlfs) HashFile(path string) (string, string, string, error) { } } -func (fs *owncloudsqlfs) ListStorageSpaces(ctx context.Context, filter []*provider.ListStorageSpacesRequest_Filter) ([]*provider.StorageSpace, error) { - // TODO(corby): Implement - return nil, errtypes.NotSupported("list storage spaces") -} - -// UpdateStorageSpace updates a storage space -func (fs *owncloudsqlfs) UpdateStorageSpace(ctx context.Context, req *provider.UpdateStorageSpaceRequest) (*provider.UpdateStorageSpaceResponse, error) { - return nil, errtypes.NotSupported("update storage space") -} - -// DeleteStorageSpace deletes a storage space -func (fs *owncloudsqlfs) DeleteStorageSpace(ctx context.Context, req *provider.DeleteStorageSpaceRequest) error { - return errtypes.NotSupported("delete storage space") -} - func readChecksumIntoResourceChecksum(ctx context.Context, checksums, algo string, ri *provider.ResourceInfo) { re := regexp.MustCompile(strings.ToUpper(algo) + `:(.*)`) matches := re.FindStringSubmatch(checksums) diff --git a/pkg/storage/fs/owncloudsql/spaces.go b/pkg/storage/fs/owncloudsql/spaces.go new file mode 100644 index 0000000000..f58d6e0ae1 --- /dev/null +++ b/pkg/storage/fs/owncloudsql/spaces.go @@ -0,0 +1,179 @@ +// Copyright 2018-2021 CERN +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// In applying this license, CERN does not waive the privileges and immunities +// granted to it by virtue of its status as an Intergovernmental Organization +// or submit itself to any jurisdiction. + +package owncloudsql + +import ( + "context" + "fmt" + "strconv" + "strings" + + userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" + provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" + ctxpkg "github.com/cs3org/reva/pkg/ctx" + "github.com/cs3org/reva/pkg/errtypes" + "github.com/cs3org/reva/pkg/storage/fs/owncloudsql/filecache" + "github.com/cs3org/reva/pkg/utils" +) + +// ListStorageSpaces lists storage spaces according to the provided filters +func (fs *owncloudsqlfs) ListStorageSpaces(ctx context.Context, filter []*provider.ListStorageSpacesRequest_Filter) ([]*provider.StorageSpace, error) { + var ( + spaceID = "*" + ) + + filteringUnsupportedSpaceTypes := false + + for i := range filter { + switch filter[i].Type { + case provider.ListStorageSpacesRequest_Filter_TYPE_SPACE_TYPE: + t := filter[i].GetSpaceType() + filteringUnsupportedSpaceTypes = (t != "personal" && !strings.HasPrefix(t, "+")) + case provider.ListStorageSpacesRequest_Filter_TYPE_ID: + spaceID, _, _ = utils.SplitStorageSpaceID(filter[i].GetId().OpaqueId) + } + } + if filteringUnsupportedSpaceTypes { + // owncloudsql only supports personal spaces, no need to search for something else + return []*provider.StorageSpace{}, nil + } + + spaces := []*provider.StorageSpace{} + if spaceID == "*" { + u, ok := ctxpkg.ContextGetUser(ctx) + if !ok { + return nil, errtypes.UserRequired("error getting user from context") + } + space, err := fs.getPersonalSpace(u) + if err != nil { + return nil, err + } + spaces = append(spaces, space) + } else { + id, err := strconv.Atoi(spaceID) + if err != nil { + // non-numeric space id -> this request is not for us + return []*provider.StorageSpace{}, nil + } + space, err := fs.getSpaceByNumericID(ctx, id) + if err != nil { + return nil, err + } + spaces = append(spaces, space) + } + return spaces, nil +} + +// CreateStorageSpace creates a storage space +func (fs *owncloudsqlfs) CreateStorageSpace(ctx context.Context, req *provider.CreateStorageSpaceRequest) (*provider.CreateStorageSpaceResponse, error) { + return nil, errtypes.NotSupported("unimplemented: CreateStorageSpace") +} + +// UpdateStorageSpace updates a storage space +func (fs *owncloudsqlfs) UpdateStorageSpace(ctx context.Context, req *provider.UpdateStorageSpaceRequest) (*provider.UpdateStorageSpaceResponse, error) { + return nil, errtypes.NotSupported("update storage space") +} + +// DeleteStorageSpace deletes a storage space +func (fs *owncloudsqlfs) DeleteStorageSpace(ctx context.Context, req *provider.DeleteStorageSpaceRequest) error { + return errtypes.NotSupported("delete storage space") +} + +// Note: currently unused but will be used later +// func (fs *owncloudsqlfs) listAllPersonalSpaces(ctx context.Context) ([]*provider.StorageSpace, error) { +// storages, err := fs.filecache.ListStorages(true) +// if err != nil { +// return nil, err +// } +// spaces := []*provider.StorageSpace{} +// for _, storage := range storages { +// space, err := fs.storageToSpace(ctx, storage) +// if err != nil { +// return nil, err +// } +// spaces = append(spaces, space) +// } +// return spaces, nil +// } + +func (fs *owncloudsqlfs) getPersonalSpace(owner *userpb.User) (*provider.StorageSpace, error) { + storageID, err := fs.filecache.GetNumericStorageID("home::" + owner.Username) + if err != nil { + return nil, err + } + storage, err := fs.filecache.GetStorage(storageID) + if err != nil { + return nil, err + } + root, err := fs.filecache.Get(storage.NumericID, "") + if err != nil { + return nil, err + } + + space := &provider.StorageSpace{ + Id: &provider.StorageSpaceId{OpaqueId: strconv.Itoa(storage.NumericID)}, + Root: &provider.ResourceId{ + StorageId: strconv.Itoa(storage.NumericID), + OpaqueId: strconv.Itoa(root.ID), + }, + Name: owner.Username, + SpaceType: "personal", + Mtime: &types.Timestamp{Seconds: uint64(root.MTime)}, + Owner: owner, + } + return space, nil +} + +func (fs *owncloudsqlfs) getSpaceByNumericID(ctx context.Context, spaceID int) (*provider.StorageSpace, error) { + storage, err := fs.filecache.GetStorage(spaceID) + if err != nil { + return nil, err + } + if !strings.HasPrefix(storage.ID, "home::") { + return nil, fmt.Errorf("only personal spaces are supported") + } + + return fs.storageToSpace(ctx, storage) +} + +func (fs *owncloudsqlfs) storageToSpace(ctx context.Context, storage *filecache.Storage) (*provider.StorageSpace, error) { + root, err := fs.filecache.Get(storage.NumericID, "") + if err != nil { + return nil, err + } + ownerName := strings.TrimPrefix(storage.ID, "home::") + owner, err := fs.getUser(ctx, ownerName) + if err != nil { + return nil, err + } + + space := &provider.StorageSpace{ + Id: &provider.StorageSpaceId{OpaqueId: strconv.Itoa(storage.NumericID)}, + Root: &provider.ResourceId{ + StorageId: strconv.Itoa(storage.NumericID), + OpaqueId: strconv.Itoa(root.ID), + }, + Name: owner.Username, + SpaceType: "personal", + Mtime: &types.Timestamp{Seconds: uint64(root.MTime)}, + Owner: owner, + } + return space, nil +} diff --git a/tests/acceptance/expected-failures-on-OCIS-storage.md b/tests/acceptance/expected-failures-on-OCIS-storage.md index 1b6ba79d18..08cc6b6bde 100644 --- a/tests/acceptance/expected-failures-on-OCIS-storage.md +++ b/tests/acceptance/expected-failures-on-OCIS-storage.md @@ -1156,6 +1156,12 @@ Not everything needs to be implemented for ocis. While the oc10 testsuite covers #### moving a share from the /Shares jail to a user home is no longer supported. - [apiShareManagementToShares/mergeShare.feature:89](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementToShares/mergeShare.feature#L89) +#### Additional shares to the same resource (e.g. user and group share) are now auto-accepted with the existing mountpoint +- [apiShareCreateSpecialToShares1/createShareReceivedInMultipleWays.feature:553](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareCreateSpecialToShares1/createShareReceivedInMultipleWays.feature:553) +- [apiShareCreateSpecialToShares1/createShareReceivedInMultipleWays.feature:554](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareCreateSpecialToShares1/createShareReceivedInMultipleWays.feature:554) +- [apiShareCreateSpecialToShares1/createShareReceivedInMultipleWays.feature:575](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareCreateSpecialToShares1/createShareReceivedInMultipleWays.feature:575) +- [apiShareCreateSpecialToShares1/createShareReceivedInMultipleWays.feature:576](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareCreateSpecialToShares1/createShareReceivedInMultipleWays.feature:576) + ### To triage _The below features have been added after I last categorized them. AFAICT they are bugs. @jfd_ diff --git a/tests/acceptance/expected-failures-on-S3NG-storage.md b/tests/acceptance/expected-failures-on-S3NG-storage.md index ac2b12f380..76d77eea0e 100644 --- a/tests/acceptance/expected-failures-on-S3NG-storage.md +++ b/tests/acceptance/expected-failures-on-S3NG-storage.md @@ -1174,6 +1174,12 @@ Not everything needs to be implemented for ocis. While the oc10 testsuite covers #### moving a share from the /Shares jail to a user home is no longer supported. - [apiShareManagementToShares/mergeShare.feature:89](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementToShares/mergeShare.feature#L89) +#### Additional shares to the same resource (e.g. user and group share) are now auto-accepted with the existing mountpoint +- [apiShareCreateSpecialToShares1/createShareReceivedInMultipleWays.feature:553](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareCreateSpecialToShares1/createShareReceivedInMultipleWays.feature:553) +- [apiShareCreateSpecialToShares1/createShareReceivedInMultipleWays.feature:554](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareCreateSpecialToShares1/createShareReceivedInMultipleWays.feature:554) +- [apiShareCreateSpecialToShares1/createShareReceivedInMultipleWays.feature:575](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareCreateSpecialToShares1/createShareReceivedInMultipleWays.feature:575) +- [apiShareCreateSpecialToShares1/createShareReceivedInMultipleWays.feature:576](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareCreateSpecialToShares1/createShareReceivedInMultipleWays.feature:576) + ### To triage _The below features have been added after I last categorized them. AFAICT they are bugs. @jfd_