From cf91ef649d634b99332dd4c3033fa209bbd36ed4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Fri, 25 Feb 2022 11:25:02 +0100 Subject: [PATCH 01/22] Add support for the userid claim --- pkg/user/manager/json/json.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/user/manager/json/json.go b/pkg/user/manager/json/json.go index 6badc23559..a0cc6f1d96 100644 --- a/pkg/user/manager/json/json.go +++ b/pkg/user/manager/json/json.go @@ -118,6 +118,8 @@ func extractClaim(u *userpb.User, claim string) (string, error) { return u.Mail, nil case "username": return u.Username, nil + case "userid": + return u.Id.OpaqueId, nil case "uid": if u.UidNumber != 0 { return strconv.FormatInt(u.UidNumber, 10), nil From d8034eb4335912ac80af0257b5b583cadf5fd783 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Fri, 25 Feb 2022 11:25:14 +0100 Subject: [PATCH 02/22] Add initial of the (still incomplete) cs3 share manager --- pkg/share/manager/cs3/cs3.go | 334 ++++++++++++++++++++++++ pkg/share/manager/cs3/cs3_suite_test.go | 31 +++ pkg/share/manager/cs3/cs3_test.go | 274 +++++++++++++++++++ pkg/share/manager/cs3/mocks/Indexer.go | 89 +++++++ pkg/share/manager/cs3/mocks/Storage.go | 165 ++++++++++++ pkg/share/manager/loader/loader.go | 1 + 6 files changed, 894 insertions(+) create mode 100644 pkg/share/manager/cs3/cs3.go create mode 100644 pkg/share/manager/cs3/cs3_suite_test.go create mode 100644 pkg/share/manager/cs3/cs3_test.go create mode 100644 pkg/share/manager/cs3/mocks/Indexer.go create mode 100644 pkg/share/manager/cs3/mocks/Storage.go diff --git a/pkg/share/manager/cs3/cs3.go b/pkg/share/manager/cs3/cs3.go new file mode 100644 index 0000000000..0836e7f398 --- /dev/null +++ b/pkg/share/manager/cs3/cs3.go @@ -0,0 +1,334 @@ +// Copyright 2018-2022 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 cs3 + +import ( + "context" + "encoding/json" + "fmt" + "net/url" + "path" + "time" + + collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1" + provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + typespb "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/share" + "github.com/cs3org/reva/pkg/share/manager/registry" + "github.com/cs3org/reva/pkg/storage/utils/indexer" + "github.com/cs3org/reva/pkg/storage/utils/indexer/option" + "github.com/cs3org/reva/pkg/storage/utils/metadata" + "github.com/google/uuid" + "github.com/mitchellh/mapstructure" + "github.com/pkg/errors" + "google.golang.org/genproto/protobuf/field_mask" +) + +//go:generate mockery -name Storage +//go:generate mockery -name Indexer +type Storage interface { + metadata.Storage +} + +type Indexer interface { + AddIndex(t interface{}, indexBy option.IndexBy, pkName, entityDirName, indexType string, bound *option.Bound, caseInsensitive bool) error + Add(t interface{}) ([]indexer.IdxAddResult, error) + FindBy(t interface{}, field string, val string) ([]string, error) + Delete(t interface{}) error +} + +// Manager implements a share manager using a cs3 storage backend +type Manager struct { + storage Storage + indexer Indexer + + initialized bool +} + +func init() { + registry.Register("cs3", NewDefault) +} + +type config struct { + GatewayAddr string `mapstructure:"gateway_addr"` + ProviderAddr string `mapstructure:"provider_addr"` + ServiceUserID string `mapstructure:"service_user_id"` + MachineAuthAPIKey string `mapstructure:"machine_auth_apikey"` +} + +func indexOwnerFunc(v interface{}) (string, error) { + share, ok := v.(*collaboration.Share) + if !ok { + return "", fmt.Errorf("given entity is not a share") + } + return url.QueryEscape(share.Owner.Idp + ":" + share.Owner.OpaqueId), nil +} + +func indexGranteeFunc(v interface{}) (string, error) { + share, ok := v.(*collaboration.Share) + if !ok { + return "", fmt.Errorf("given entity is not a share") + } + if share.Grantee.Type == provider.GranteeType_GRANTEE_TYPE_USER { + return "user:" + share.Grantee.GetUserId().Idp + ":" + share.Grantee.GetUserId().OpaqueId, nil + } else if share.Grantee.Type == provider.GranteeType_GRANTEE_TYPE_GROUP { + return "group:" + share.Grantee.GetGroupId().Idp + ":" + share.Grantee.GetGroupId().OpaqueId, nil + } else { + return "", fmt.Errorf("unknown grantee type") + } +} + +// NewDefault returns a new manager instance with default dependencies +func NewDefault(m map[string]interface{}) (share.Manager, error) { + c := &config{} + if err := mapstructure.Decode(m, c); err != nil { + err = errors.Wrap(err, "error creating a new manager") + return nil, err + } + + s, err := metadata.NewCS3Storage(c.GatewayAddr, c.ProviderAddr, c.ServiceUserID, c.MachineAuthAPIKey) + if err != nil { + return nil, err + } + indexer := indexer.CreateIndexer(s) + + return New(s, indexer) +} + +// New returns a new manager instance +func New(s Storage, indexer Indexer) (*Manager, error) { + return &Manager{ + storage: s, + indexer: indexer, + initialized: false, + }, nil +} + +func (m *Manager) initialize() error { + if m.initialized { + return nil + } + + err := m.storage.Init("cs3-share-manager-metadata", context.Background()) + if err != nil { + return err + } + if err := m.storage.MakeDirIfNotExist(context.Background(), "shares"); err != nil { + return err + } + err = m.indexer.AddIndex(&collaboration.Share{}, option.IndexByFunc{ + Name: "OwnerId", + Func: indexOwnerFunc, + }, "Id.OpaqueId", "shares", "non_unique", nil, true) + if err != nil { + return err + } + err = m.indexer.AddIndex(&collaboration.Share{}, option.IndexByFunc{ + Name: "GranteeId", + Func: indexGranteeFunc, + }, "Id.OpaqueId", "shares", "non_unique", nil, true) + if err != nil { + return err + } + m.initialized = true + return nil +} + +// Share creates a new share +func (m *Manager) Share(ctx context.Context, md *provider.ResourceInfo, g *collaboration.ShareGrant) (*collaboration.Share, error) { + if err := m.initialize(); err != nil { + return nil, err + } + user := ctxpkg.ContextMustGetUser(ctx) + now := time.Now().UnixNano() + ts := &typespb.Timestamp{ + Seconds: uint64(now / 1000000000), + Nanos: uint32(now % 1000000000), + } + + //// do not allow share to myself or the owner if share is for a user + //// TODO(labkode): should not this be caught already at the gw level? + //if g.Grantee.Type == provider.GranteeType_GRANTEE_TYPE_USER && + // (utils.UserEqual(g.Grantee.GetUserId(), user.Id) || utils.UserEqual(g.Grantee.GetUserId(), md.Owner)) { + // return nil, errors.New("json: owner/creator and grantee are the same") + //} + // + //// check if share already exists. + //key := &collaboration.ShareKey{ + // Owner: md.Owner, + // ResourceId: md.Id, + // Grantee: g.Grantee, + //} + //_, err := m.getByKey(ctx, key) + // + //// share already exists + //if err == nil { + // return nil, errtypes.AlreadyExists(key.String()) + //} + + share := &collaboration.Share{ + Id: &collaboration.ShareId{ + OpaqueId: uuid.NewString(), + }, + ResourceId: md.Id, + Permissions: g.Permissions, + Grantee: g.Grantee, + Owner: md.Owner, + Creator: user.Id, + Ctime: ts, + Mtime: ts, + } + + data, err := json.Marshal(share) + if err != nil { + return nil, err + } + + fn := path.Join("shares", share.Id.OpaqueId) + err = m.storage.SimpleUpload(ctx, fn, data) + if err != nil { + return nil, err + } + + _, err = m.indexer.Add(share) + if err != nil { + return nil, err + } + + return share, nil +} + +// GetShare gets the information for a share by the given ref. +func (m *Manager) GetShare(ctx context.Context, ref *collaboration.ShareReference) (*collaboration.Share, error) { + if err := m.initialize(); err != nil { + return nil, err + } + fn := path.Join("shares", ref.GetId().OpaqueId) + data, err := m.storage.SimpleDownload(ctx, fn) + if err != nil { + return nil, err + } + + return unmarshalShareData(data) +} + +// Unshare deletes the share pointed by ref. +func (m *Manager) Unshare(ctx context.Context, ref *collaboration.ShareReference) error { + if err := m.initialize(); err != nil { + return err + } + share, err := m.GetShare(ctx, ref) + if err != nil { + return err + } + + fn := path.Join("shares", ref.GetId().OpaqueId) + err = m.storage.Delete(ctx, fn) + if err != nil { + return err + } + + return m.indexer.Delete(share) +} + +// ListShares returns the shares created by the user. If md is provided is not nil, +// it returns only shares attached to the given resource. +func (m *Manager) ListShares(ctx context.Context, filters []*collaboration.Filter) ([]*collaboration.Share, error) { + if err := m.initialize(); err != nil { + return nil, err + } + + user, ok := ctxpkg.ContextGetUser(ctx) + if !ok { + return nil, errtypes.UserRequired("error getting user from context") + } + + allShareIds, err := m.indexer.FindBy(&collaboration.Share{}, "OwnerId", url.QueryEscape(user.GetId().Idp+":"+user.GetId().OpaqueId)) + if err != nil { + return nil, err + } + result := []*collaboration.Share{} + for _, id := range allShareIds { + data, err := m.storage.SimpleDownload(ctx, path.Join("shares", id)) + if err != nil { + return nil, err + } + + s, err := unmarshalShareData(data) + if err != nil { + return nil, err + } + if share.MatchesFilters(s, filters) { + result = append(result, s) + } + } + return result, nil +} + +// UpdateShare updates the mode of the given share. +func (m *Manager) UpdateShare(ctx context.Context, ref *collaboration.ShareReference, p *collaboration.SharePermissions) (*collaboration.Share, error) { + if err := m.initialize(); err != nil { + return nil, err + } + return nil, nil +} + +// ListReceivedShares returns the list of shares the user has access to. +func (m *Manager) ListReceivedShares(ctx context.Context, filters []*collaboration.Filter) ([]*collaboration.ReceivedShare, error) { + if err := m.initialize(); err != nil { + return nil, err + } + return nil, nil +} + +// GetReceivedShare returns the information for a received share. +func (m *Manager) GetReceivedShare(ctx context.Context, ref *collaboration.ShareReference) (*collaboration.ReceivedShare, error) { + if err := m.initialize(); err != nil { + return nil, err + } + return nil, nil +} + +// UpdateReceivedShare updates the received share with share state. +func (m *Manager) UpdateReceivedShare(ctx context.Context, share *collaboration.ReceivedShare, fieldMask *field_mask.FieldMask) (*collaboration.ReceivedShare, error) { + if err := m.initialize(); err != nil { + return nil, err + } + return nil, nil +} + +func unmarshalShareData(data []byte) (*collaboration.Share, error) { + userShare := &collaboration.Share{ + Grantee: &provider.Grantee{Id: &provider.Grantee_UserId{}}, + } + groupShare := &collaboration.Share{ + Grantee: &provider.Grantee{Id: &provider.Grantee_GroupId{}}, + } + err := json.Unmarshal(data, userShare) + if err == nil && userShare.Grantee.GetUserId() != nil { + return userShare, nil + } + err = json.Unmarshal(data, groupShare) // try to unmarshal to a group share if the user share unmarshalling failed + if err == nil && groupShare.Grantee.GetGroupId() != nil { + return groupShare, nil + } + return nil, errtypes.InternalError("failed to unmarshal share data") +} diff --git a/pkg/share/manager/cs3/cs3_suite_test.go b/pkg/share/manager/cs3/cs3_suite_test.go new file mode 100644 index 0000000000..30a29e6f26 --- /dev/null +++ b/pkg/share/manager/cs3/cs3_suite_test.go @@ -0,0 +1,31 @@ +// 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 cs3_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestCs3(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Cs3 Suite") +} diff --git a/pkg/share/manager/cs3/cs3_test.go b/pkg/share/manager/cs3/cs3_test.go new file mode 100644 index 0000000000..25f6d06f45 --- /dev/null +++ b/pkg/share/manager/cs3/cs3_test.go @@ -0,0 +1,274 @@ +// Copyright 2018-2022 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 cs3_test + +import ( + "context" + "encoding/json" + "path" + + groupv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1" + userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" + collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1" + provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + ctxpkg "github.com/cs3org/reva/pkg/ctx" + "github.com/cs3org/reva/pkg/share/manager/cs3" + "github.com/cs3org/reva/pkg/share/manager/cs3/mocks" + indexerpkg "github.com/cs3org/reva/pkg/storage/utils/indexer" + "github.com/stretchr/testify/mock" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Manager", func() { + var ( + storage *mocks.Storage + indexer *mocks.Indexer + user *userpb.User + grantee *userpb.User + share *collaboration.Share + share2 *collaboration.Share + grant *collaboration.ShareGrant + ctx context.Context + ) + + BeforeEach(func() { + storage = &mocks.Storage{} + storage.On("Init", mock.Anything, mock.Anything).Return(nil) + storage.On("MakeDirIfNotExist", mock.Anything, mock.Anything).Return(nil) + storage.On("Delete", mock.Anything, mock.Anything).Return(nil) + storage.On("SimpleUpload", mock.Anything, mock.Anything, mock.Anything).Return(nil) + indexer = &mocks.Indexer{} + indexer.On("AddIndex", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) + indexer.On("Add", mock.Anything).Return([]indexerpkg.IdxAddResult{}, nil) + indexer.On("Delete", mock.Anything).Return(nil) + + user = &userpb.User{ + Id: &userpb.UserId{ + OpaqueId: "1", + }, + } + grantee = &userpb.User{ + Id: &userpb.UserId{ + OpaqueId: "2", + }, + } + + grant = &collaboration.ShareGrant{ + Grantee: &provider.Grantee{ + Type: provider.GranteeType_GRANTEE_TYPE_USER, + Id: &provider.Grantee_UserId{UserId: grantee.GetId()}, + }, + Permissions: &collaboration.SharePermissions{ + Permissions: &provider.ResourcePermissions{ + GetPath: true, + InitiateFileDownload: true, + ListFileVersions: true, + ListContainer: true, + Stat: true, + }, + }, + } + share = &collaboration.Share{ + Id: &collaboration.ShareId{OpaqueId: "1"}, + ResourceId: &provider.ResourceId{OpaqueId: "abcd"}, + Owner: user.GetId(), + Grantee: &provider.Grantee{ + Type: provider.GranteeType_GRANTEE_TYPE_USER, + Id: &provider.Grantee_UserId{UserId: grantee.GetId()}, + }, + Permissions: &collaboration.SharePermissions{ + Permissions: &provider.ResourcePermissions{ + GetPath: true, + InitiateFileDownload: true, + ListFileVersions: true, + ListContainer: true, + Stat: true, + }, + }, + } + share2 = &collaboration.Share{ + Id: &collaboration.ShareId{OpaqueId: "2"}, + ResourceId: &provider.ResourceId{OpaqueId: "efgh"}, + Owner: user.GetId(), + Grantee: &provider.Grantee{ + Type: provider.GranteeType_GRANTEE_TYPE_USER, + Id: &provider.Grantee_UserId{UserId: grantee.GetId()}, + }, + Permissions: &collaboration.SharePermissions{ + Permissions: &provider.ResourcePermissions{ + GetPath: true, + InitiateFileDownload: true, + ListFileVersions: true, + ListContainer: true, + Stat: true, + }, + }, + } + ctx = ctxpkg.ContextSetUser(context.Background(), user) + }) + + Describe("New", func() { + JustBeforeEach(func() { + m, err := cs3.New(storage, indexer) + Expect(err).ToNot(HaveOccurred()) + Expect(m).ToNot(BeNil()) + }) + + It("does not initialize the storage yet", func() { + storage.AssertNotCalled(GinkgoT(), "Init", mock.Anything, mock.Anything) + }) + }) + + Context("with a manager instance and a share", func() { + var ( + m *cs3.Manager + sharedResource = &provider.ResourceInfo{ + Id: &provider.ResourceId{ + StorageId: "storageid", + OpaqueId: "opaqueid", + }, + } + ) + + JustBeforeEach(func() { + var err error + m, err = cs3.New(storage, indexer) + Expect(err).ToNot(HaveOccurred()) + data, err := json.Marshal(share) + Expect(err).ToNot(HaveOccurred()) + storage.On("SimpleDownload", mock.Anything, path.Join("shares", share.Id.OpaqueId)).Return(data, nil) + data2, err := json.Marshal(share2) + Expect(err).ToNot(HaveOccurred()) + storage.On("SimpleDownload", mock.Anything, path.Join("shares", share2.Id.OpaqueId)).Return(data2, nil) + }) + + Describe("Share", func() { + var ( + share *collaboration.Share + ) + + JustBeforeEach(func() { + var err error + share, err = m.Share(ctx, sharedResource, grant) + Expect(err).ToNot(HaveOccurred()) + }) + + It("returns a share holding the share information", func() { + Expect(share).ToNot(BeNil()) + Expect(share.ResourceId).To(Equal(sharedResource.Id)) + Expect(share.Creator).To(Equal(user.Id)) + Expect(share.Grantee).To(Equal(grant.Grantee)) + Expect(share.Permissions).To(Equal(grant.Permissions)) + }) + + It("stores the share in the storage using the id as the filename", func() { + expectedPath := path.Join("shares", share.Id.OpaqueId) + storage.AssertCalled(GinkgoT(), "SimpleUpload", mock.Anything, expectedPath, mock.Anything) + }) + + It("indexes the share", func() { + indexer.AssertCalled(GinkgoT(), "Add", mock.AnythingOfType("*collaborationv1beta1.Share")) + }) + }) + + Describe("Unshare", func() { + JustBeforeEach(func() { + err := m.Unshare(ctx, &collaboration.ShareReference{Spec: &collaboration.ShareReference_Id{Id: share.Id}}) + Expect(err).ToNot(HaveOccurred()) + }) + + It("deletes the share", func() { + expectedPath := path.Join("shares", share.Id.OpaqueId) + storage.AssertCalled(GinkgoT(), "Delete", mock.Anything, expectedPath) + }) + + It("removes the share from the index", func() { + indexer.AssertCalled(GinkgoT(), "Delete", mock.AnythingOfType("*collaborationv1beta1.Share")) + }) + }) + + Describe("GetShare", func() { + Context("when the share is a user share ", func() { + It("returns a user share", func() { + returnedShare, err := m.GetShare(ctx, &collaboration.ShareReference{ + Spec: &collaboration.ShareReference_Id{Id: &collaboration.ShareId{OpaqueId: "1"}}, + }) + Expect(err).ToNot(HaveOccurred()) + Expect(returnedShare).ToNot(BeNil()) + Expect(returnedShare.Id.OpaqueId).To(Equal(share.Id.OpaqueId)) + Expect(returnedShare.Owner).To(Equal(share.Owner)) + Expect(returnedShare.Grantee).To(Equal(share.Grantee)) + Expect(returnedShare.Permissions).To(Equal(share.Permissions)) + }) + }) + + Context("when the share is a group share ", func() { + BeforeEach(func() { + share.Grantee = &provider.Grantee{ + Type: provider.GranteeType_GRANTEE_TYPE_GROUP, + Id: &provider.Grantee_GroupId{ + GroupId: &groupv1beta1.GroupId{OpaqueId: "1000"}, + }, + } + }) + + It("returns a group share", func() { + returnedShare, err := m.GetShare(ctx, &collaboration.ShareReference{ + Spec: &collaboration.ShareReference_Id{Id: &collaboration.ShareId{OpaqueId: "1"}}, + }) + Expect(err).ToNot(HaveOccurred()) + Expect(returnedShare).ToNot(BeNil()) + Expect(returnedShare.Id.OpaqueId).To(Equal(share.Id.OpaqueId)) + Expect(returnedShare.Owner).To(Equal(share.Owner)) + Expect(returnedShare.Grantee).To(Equal(share.Grantee)) + Expect(returnedShare.Permissions).To(Equal(share.Permissions)) + }) + }) + }) + + Describe("ListShares", func() { + JustBeforeEach(func() { + indexer.On("FindBy", mock.Anything, "OwnerId", mock.Anything).Return([]string{share.Id.OpaqueId, share2.Id.OpaqueId}, nil) + }) + It("uses the index to get the owned shares", func() { + shares, err := m.ListShares(ctx, []*collaboration.Filter{}) + Expect(err).ToNot(HaveOccurred()) + Expect(len(shares)).To(Equal(2)) + Expect(shares[0].Id.OpaqueId).To(Equal("1")) + Expect(shares[1].Id.OpaqueId).To(Equal("2")) + }) + + It("applies resource id filters", func() { + shares, err := m.ListShares(ctx, []*collaboration.Filter{ + &collaboration.Filter{ + Type: collaboration.Filter_TYPE_RESOURCE_ID, + Term: &collaboration.Filter_ResourceId{ + ResourceId: share.ResourceId, + }, + }, + }) + Expect(err).ToNot(HaveOccurred()) + Expect(len(shares)).To(Equal(1)) + Expect(shares[0].Id.OpaqueId).To(Equal("1")) + }) + }) + }) +}) diff --git a/pkg/share/manager/cs3/mocks/Indexer.go b/pkg/share/manager/cs3/mocks/Indexer.go new file mode 100644 index 0000000000..b53695381f --- /dev/null +++ b/pkg/share/manager/cs3/mocks/Indexer.go @@ -0,0 +1,89 @@ +// Code generated by mockery v1.0.0. DO NOT EDIT. + +package mocks + +import ( + indexer "github.com/cs3org/reva/pkg/storage/utils/indexer" + mock "github.com/stretchr/testify/mock" + + option "github.com/cs3org/reva/pkg/storage/utils/indexer/option" +) + +// Indexer is an autogenerated mock type for the Indexer type +type Indexer struct { + mock.Mock +} + +// Add provides a mock function with given fields: t +func (_m *Indexer) Add(t interface{}) ([]indexer.IdxAddResult, error) { + ret := _m.Called(t) + + var r0 []indexer.IdxAddResult + if rf, ok := ret.Get(0).(func(interface{}) []indexer.IdxAddResult); ok { + r0 = rf(t) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]indexer.IdxAddResult) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(interface{}) error); ok { + r1 = rf(t) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// AddIndex provides a mock function with given fields: t, indexBy, pkName, entityDirName, indexType, bound, caseInsensitive +func (_m *Indexer) AddIndex(t interface{}, indexBy option.IndexBy, pkName string, entityDirName string, indexType string, bound *option.Bound, caseInsensitive bool) error { + ret := _m.Called(t, indexBy, pkName, entityDirName, indexType, bound, caseInsensitive) + + var r0 error + if rf, ok := ret.Get(0).(func(interface{}, option.IndexBy, string, string, string, *option.Bound, bool) error); ok { + r0 = rf(t, indexBy, pkName, entityDirName, indexType, bound, caseInsensitive) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// Delete provides a mock function with given fields: t +func (_m *Indexer) Delete(t interface{}) error { + ret := _m.Called(t) + + var r0 error + if rf, ok := ret.Get(0).(func(interface{}) error); ok { + r0 = rf(t) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// FindBy provides a mock function with given fields: t, field, val +func (_m *Indexer) FindBy(t interface{}, field string, val string) ([]string, error) { + ret := _m.Called(t, field, val) + + var r0 []string + if rf, ok := ret.Get(0).(func(interface{}, string, string) []string); ok { + r0 = rf(t, field, val) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]string) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(interface{}, string, string) error); ok { + r1 = rf(t, field, val) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} diff --git a/pkg/share/manager/cs3/mocks/Storage.go b/pkg/share/manager/cs3/mocks/Storage.go new file mode 100644 index 0000000000..80d547a927 --- /dev/null +++ b/pkg/share/manager/cs3/mocks/Storage.go @@ -0,0 +1,165 @@ +// Code generated by mockery v1.0.0. DO NOT EDIT. + +package mocks + +import ( + context "context" + + mock "github.com/stretchr/testify/mock" +) + +// Storage is an autogenerated mock type for the Storage type +type Storage struct { + mock.Mock +} + +// Backend provides a mock function with given fields: +func (_m *Storage) Backend() string { + ret := _m.Called() + + var r0 string + if rf, ok := ret.Get(0).(func() string); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(string) + } + + return r0 +} + +// CreateSymlink provides a mock function with given fields: ctx, oldname, newname +func (_m *Storage) CreateSymlink(ctx context.Context, oldname string, newname string) error { + ret := _m.Called(ctx, oldname, newname) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, string) error); ok { + r0 = rf(ctx, oldname, newname) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// Delete provides a mock function with given fields: ctx, path +func (_m *Storage) Delete(ctx context.Context, path string) error { + ret := _m.Called(ctx, path) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string) error); ok { + r0 = rf(ctx, path) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// Init provides a mock function with given fields: name, ctx +func (_m *Storage) Init(name string, ctx context.Context) error { + ret := _m.Called(name, ctx) + + var r0 error + if rf, ok := ret.Get(0).(func(string, context.Context) error); ok { + r0 = rf(name, ctx) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// MakeDirIfNotExist provides a mock function with given fields: ctx, name +func (_m *Storage) MakeDirIfNotExist(ctx context.Context, name string) error { + ret := _m.Called(ctx, name) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string) error); ok { + r0 = rf(ctx, name) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// ReadDir provides a mock function with given fields: ctx, path +func (_m *Storage) ReadDir(ctx context.Context, path string) ([]string, error) { + ret := _m.Called(ctx, path) + + var r0 []string + if rf, ok := ret.Get(0).(func(context.Context, string) []string); ok { + r0 = rf(ctx, path) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]string) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { + r1 = rf(ctx, path) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// ResolveSymlink provides a mock function with given fields: ctx, name +func (_m *Storage) ResolveSymlink(ctx context.Context, name string) (string, error) { + ret := _m.Called(ctx, name) + + var r0 string + if rf, ok := ret.Get(0).(func(context.Context, string) string); ok { + r0 = rf(ctx, name) + } else { + r0 = ret.Get(0).(string) + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { + r1 = rf(ctx, name) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// SimpleDownload provides a mock function with given fields: ctx, path +func (_m *Storage) SimpleDownload(ctx context.Context, path string) ([]byte, error) { + ret := _m.Called(ctx, path) + + var r0 []byte + if rf, ok := ret.Get(0).(func(context.Context, string) []byte); ok { + r0 = rf(ctx, path) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]byte) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { + r1 = rf(ctx, path) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// SimpleUpload provides a mock function with given fields: ctx, uploadpath, content +func (_m *Storage) SimpleUpload(ctx context.Context, uploadpath string, content []byte) error { + ret := _m.Called(ctx, uploadpath, content) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, []byte) error); ok { + r0 = rf(ctx, uploadpath, content) + } else { + r0 = ret.Error(0) + } + + return r0 +} diff --git a/pkg/share/manager/loader/loader.go b/pkg/share/manager/loader/loader.go index 5f2da40695..50c5f3fa29 100644 --- a/pkg/share/manager/loader/loader.go +++ b/pkg/share/manager/loader/loader.go @@ -20,6 +20,7 @@ package loader import ( // Load core share manager drivers. + _ "github.com/cs3org/reva/v2/pkg/share/manager/cs3" _ "github.com/cs3org/reva/v2/pkg/share/manager/json" _ "github.com/cs3org/reva/v2/pkg/share/manager/memory" _ "github.com/cs3org/reva/v2/pkg/share/manager/sql" From cc297062d722a183bb2fa57d7a234400ac8f46d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Mon, 28 Feb 2022 09:26:52 +0100 Subject: [PATCH 03/22] Add support for getting shares by key --- pkg/share/manager/cs3/cs3.go | 114 +++++++++++++++++++------ pkg/share/manager/cs3/cs3_test.go | 66 ++++++++++++++ pkg/share/manager/cs3/mocks/Storage.go | 10 +-- 3 files changed, 160 insertions(+), 30 deletions(-) diff --git a/pkg/share/manager/cs3/cs3.go b/pkg/share/manager/cs3/cs3.go index 0836e7f398..673765609e 100644 --- a/pkg/share/manager/cs3/cs3.go +++ b/pkg/share/manager/cs3/cs3.go @@ -26,6 +26,7 @@ import ( "path" "time" + userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" typespb "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" @@ -36,6 +37,7 @@ import ( "github.com/cs3org/reva/pkg/storage/utils/indexer" "github.com/cs3org/reva/pkg/storage/utils/indexer/option" "github.com/cs3org/reva/pkg/storage/utils/metadata" + "github.com/cs3org/reva/pkg/utils" "github.com/google/uuid" "github.com/mitchellh/mapstructure" "github.com/pkg/errors" @@ -74,28 +76,6 @@ type config struct { MachineAuthAPIKey string `mapstructure:"machine_auth_apikey"` } -func indexOwnerFunc(v interface{}) (string, error) { - share, ok := v.(*collaboration.Share) - if !ok { - return "", fmt.Errorf("given entity is not a share") - } - return url.QueryEscape(share.Owner.Idp + ":" + share.Owner.OpaqueId), nil -} - -func indexGranteeFunc(v interface{}) (string, error) { - share, ok := v.(*collaboration.Share) - if !ok { - return "", fmt.Errorf("given entity is not a share") - } - if share.Grantee.Type == provider.GranteeType_GRANTEE_TYPE_USER { - return "user:" + share.Grantee.GetUserId().Idp + ":" + share.Grantee.GetUserId().OpaqueId, nil - } else if share.Grantee.Type == provider.GranteeType_GRANTEE_TYPE_GROUP { - return "group:" + share.Grantee.GetGroupId().Idp + ":" + share.Grantee.GetGroupId().OpaqueId, nil - } else { - return "", fmt.Errorf("unknown grantee type") - } -} - // NewDefault returns a new manager instance with default dependencies func NewDefault(m map[string]interface{}) (share.Manager, error) { c := &config{} @@ -127,7 +107,7 @@ func (m *Manager) initialize() error { return nil } - err := m.storage.Init("cs3-share-manager-metadata", context.Background()) + err := m.storage.Init(context.Background(), "cs3-share-manager-metadata") if err != nil { return err } @@ -221,15 +201,54 @@ func (m *Manager) GetShare(ctx context.Context, ref *collaboration.ShareReferenc if err := m.initialize(); err != nil { return nil, err } - fn := path.Join("shares", ref.GetId().OpaqueId) + + switch { + case ref.GetId() != nil: + return m.getShareById(ctx, ref.GetId().OpaqueId) + case ref.GetKey() != nil: + return m.getShareByKey(ctx, ref.GetKey()) + default: + return nil, errtypes.BadRequest("neither share id nor key was given") + } + +} + +func (m *Manager) getShareById(ctx context.Context, id string) (*collaboration.Share, error) { + fn := path.Join("shares", id) data, err := m.storage.SimpleDownload(ctx, fn) if err != nil { return nil, err } - return unmarshalShareData(data) } +func (m *Manager) getShareByKey(ctx context.Context, key *collaboration.ShareKey) (*collaboration.Share, error) { + ownerIds, err := m.indexer.FindBy(&collaboration.Share{}, "OwnerId", userIdToIndex(key.Owner)) + if err != nil { + return nil, err + } + granteeIndex, err := granteeToIndex(key.Grantee) + if err != nil { + return nil, err + } + granteeIds, err := m.indexer.FindBy(&collaboration.Share{}, "GranteeId", granteeIndex) + if err != nil { + return nil, err + } + + ids := intersectSlices(ownerIds, granteeIds) + for _, id := range ids { + share, err := m.getShareById(ctx, id) + if err != nil { + return nil, err + } + if utils.ResourceIDEqual(share.ResourceId, key.ResourceId) { + return share, nil + } + } + return nil, errtypes.NotFound("share not found") +} + // Unshare deletes the share pointed by ref. func (m *Manager) Unshare(ctx context.Context, ref *collaboration.ShareReference) error { if err := m.initialize(); err != nil { @@ -332,3 +351,48 @@ func unmarshalShareData(data []byte) (*collaboration.Share, error) { } return nil, errtypes.InternalError("failed to unmarshal share data") } + +func indexOwnerFunc(v interface{}) (string, error) { + share, ok := v.(*collaboration.Share) + if !ok { + return "", fmt.Errorf("given entity is not a share") + } + return userIdToIndex(share.Owner), nil +} + +func userIdToIndex(id *userpb.UserId) string { + return url.QueryEscape(id.Idp + ":" + id.OpaqueId) +} + +func indexGranteeFunc(v interface{}) (string, error) { + share, ok := v.(*collaboration.Share) + if !ok { + return "", fmt.Errorf("given entity is not a share") + } + return granteeToIndex(share.Grantee) +} + +func granteeToIndex(grantee *provider.Grantee) (string, error) { + switch { + case grantee.Type == provider.GranteeType_GRANTEE_TYPE_USER: + return url.QueryEscape("user:" + grantee.GetUserId().Idp + ":" + grantee.GetUserId().OpaqueId), nil + case grantee.Type == provider.GranteeType_GRANTEE_TYPE_GROUP: + return url.QueryEscape("group:" + grantee.GetGroupId().Idp + ":" + grantee.GetGroupId().OpaqueId), nil + default: + return "", fmt.Errorf("unknown grantee type") + } +} + +func intersectSlices(a, b []string) []string { + aMap := map[string]bool{} + for _, s := range a { + aMap[s] = true + } + result := []string{} + for _, s := range b { + if _, ok := aMap[s]; ok { + result = append(result, s) + } + } + return result +} diff --git a/pkg/share/manager/cs3/cs3_test.go b/pkg/share/manager/cs3/cs3_test.go index 25f6d06f45..cb62791fe8 100644 --- a/pkg/share/manager/cs3/cs3_test.go +++ b/pkg/share/manager/cs3/cs3_test.go @@ -21,6 +21,7 @@ package cs3_test import ( "context" "encoding/json" + "net/url" "path" groupv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1" @@ -62,11 +63,13 @@ var _ = Describe("Manager", func() { user = &userpb.User{ Id: &userpb.UserId{ + Idp: "localhost:1111", OpaqueId: "1", }, } grantee = &userpb.User{ Id: &userpb.UserId{ + Idp: "localhost:1111", OpaqueId: "2", }, } @@ -218,6 +221,69 @@ var _ = Describe("Manager", func() { Expect(returnedShare.Grantee).To(Equal(share.Grantee)) Expect(returnedShare.Permissions).To(Equal(share.Permissions)) }) + + It("gets the share by key", func() { + indexer.On("FindBy", mock.Anything, "OwnerId", url.QueryEscape(share.Owner.Idp+":"+share.Owner.OpaqueId)). + Return([]string{share.Id.OpaqueId, share2.Id.OpaqueId}, nil) + indexer.On("FindBy", mock.Anything, "GranteeId", url.QueryEscape("user:"+grantee.Id.Idp+":"+grantee.Id.OpaqueId)). + Return([]string{share2.Id.OpaqueId}, nil) + returnedShare, err := m.GetShare(ctx, &collaboration.ShareReference{ + Spec: &collaboration.ShareReference_Key{ + Key: &collaboration.ShareKey{ + Owner: share2.Owner, + ResourceId: share2.ResourceId, + Grantee: share2.Grantee, + }, + }, + }) + Expect(err).ToNot(HaveOccurred()) + Expect(returnedShare).ToNot(BeNil()) + Expect(returnedShare.Id.OpaqueId).To(Equal(share2.Id.OpaqueId)) + Expect(returnedShare.Owner).To(Equal(share2.Owner)) + Expect(returnedShare.Grantee).To(Equal(share2.Grantee)) + Expect(returnedShare.Permissions).To(Equal(share2.Permissions)) + }) + + Context("when requesting the share by key", func() { + It("returns NotFound", func() { + indexer.On("FindBy", mock.Anything, "OwnerId", url.QueryEscape(share.Owner.Idp+":"+share.Owner.OpaqueId)). + Return([]string{share.Id.OpaqueId, share2.Id.OpaqueId}, nil) + indexer.On("FindBy", mock.Anything, "GranteeId", url.QueryEscape("user:"+grantee.Id.Idp+":"+grantee.Id.OpaqueId)). + Return([]string{}, nil) + returnedShare, err := m.GetShare(ctx, &collaboration.ShareReference{ + Spec: &collaboration.ShareReference_Key{ + Key: &collaboration.ShareKey{ + Owner: share2.Owner, + ResourceId: share2.ResourceId, + Grantee: share2.Grantee, + }, + }, + }) + Expect(err).To(HaveOccurred()) + Expect(returnedShare).To(BeNil()) + }) + It("gets the share", func() { + indexer.On("FindBy", mock.Anything, "OwnerId", url.QueryEscape(share.Owner.Idp+":"+share.Owner.OpaqueId)). + Return([]string{share.Id.OpaqueId, share2.Id.OpaqueId}, nil) + indexer.On("FindBy", mock.Anything, "GranteeId", url.QueryEscape("user:"+grantee.Id.Idp+":"+grantee.Id.OpaqueId)). + Return([]string{share2.Id.OpaqueId}, nil) + returnedShare, err := m.GetShare(ctx, &collaboration.ShareReference{ + Spec: &collaboration.ShareReference_Key{ + Key: &collaboration.ShareKey{ + Owner: share2.Owner, + ResourceId: share2.ResourceId, + Grantee: share2.Grantee, + }, + }, + }) + Expect(err).ToNot(HaveOccurred()) + Expect(returnedShare).ToNot(BeNil()) + Expect(returnedShare.Id.OpaqueId).To(Equal(share2.Id.OpaqueId)) + Expect(returnedShare.Owner).To(Equal(share2.Owner)) + Expect(returnedShare.Grantee).To(Equal(share2.Grantee)) + Expect(returnedShare.Permissions).To(Equal(share2.Permissions)) + }) + }) }) Context("when the share is a group share ", func() { diff --git a/pkg/share/manager/cs3/mocks/Storage.go b/pkg/share/manager/cs3/mocks/Storage.go index 80d547a927..14d9b8d00e 100644 --- a/pkg/share/manager/cs3/mocks/Storage.go +++ b/pkg/share/manager/cs3/mocks/Storage.go @@ -55,13 +55,13 @@ func (_m *Storage) Delete(ctx context.Context, path string) error { return r0 } -// Init provides a mock function with given fields: name, ctx -func (_m *Storage) Init(name string, ctx context.Context) error { - ret := _m.Called(name, ctx) +// Init provides a mock function with given fields: ctx, name +func (_m *Storage) Init(ctx context.Context, name string) error { + ret := _m.Called(ctx, name) var r0 error - if rf, ok := ret.Get(0).(func(string, context.Context) error); ok { - r0 = rf(name, ctx) + if rf, ok := ret.Get(0).(func(context.Context, string) error); ok { + r0 = rf(ctx, name) } else { r0 = ret.Error(0) } From a79267b84710e3c54cac84e0d65f5a6d0b56c69a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Mon, 28 Feb 2022 09:30:17 +0100 Subject: [PATCH 04/22] Cleanup, increase test coverage --- pkg/share/manager/cs3/cs3.go | 22 +------------- pkg/share/manager/cs3/cs3_test.go | 48 ++++++++++++------------------- 2 files changed, 20 insertions(+), 50 deletions(-) diff --git a/pkg/share/manager/cs3/cs3.go b/pkg/share/manager/cs3/cs3.go index 673765609e..22bb48654d 100644 --- a/pkg/share/manager/cs3/cs3.go +++ b/pkg/share/manager/cs3/cs3.go @@ -144,26 +144,6 @@ func (m *Manager) Share(ctx context.Context, md *provider.ResourceInfo, g *colla Nanos: uint32(now % 1000000000), } - //// do not allow share to myself or the owner if share is for a user - //// TODO(labkode): should not this be caught already at the gw level? - //if g.Grantee.Type == provider.GranteeType_GRANTEE_TYPE_USER && - // (utils.UserEqual(g.Grantee.GetUserId(), user.Id) || utils.UserEqual(g.Grantee.GetUserId(), md.Owner)) { - // return nil, errors.New("json: owner/creator and grantee are the same") - //} - // - //// check if share already exists. - //key := &collaboration.ShareKey{ - // Owner: md.Owner, - // ResourceId: md.Id, - // Grantee: g.Grantee, - //} - //_, err := m.getByKey(ctx, key) - // - //// share already exists - //if err == nil { - // return nil, errtypes.AlreadyExists(key.String()) - //} - share := &collaboration.Share{ Id: &collaboration.ShareId{ OpaqueId: uuid.NewString(), @@ -280,7 +260,7 @@ func (m *Manager) ListShares(ctx context.Context, filters []*collaboration.Filte return nil, errtypes.UserRequired("error getting user from context") } - allShareIds, err := m.indexer.FindBy(&collaboration.Share{}, "OwnerId", url.QueryEscape(user.GetId().Idp+":"+user.GetId().OpaqueId)) + allShareIds, err := m.indexer.FindBy(&collaboration.Share{}, "OwnerId", userIdToIndex(user.GetId())) if err != nil { return nil, err } diff --git a/pkg/share/manager/cs3/cs3_test.go b/pkg/share/manager/cs3/cs3_test.go index cb62791fe8..248895bcb4 100644 --- a/pkg/share/manager/cs3/cs3_test.go +++ b/pkg/share/manager/cs3/cs3_test.go @@ -29,6 +29,7 @@ import ( collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" ctxpkg "github.com/cs3org/reva/pkg/ctx" + "github.com/cs3org/reva/pkg/errtypes" "github.com/cs3org/reva/pkg/share/manager/cs3" "github.com/cs3org/reva/pkg/share/manager/cs3/mocks" indexerpkg "github.com/cs3org/reva/pkg/storage/utils/indexer" @@ -161,6 +162,7 @@ var _ = Describe("Manager", func() { data2, err := json.Marshal(share2) Expect(err).ToNot(HaveOccurred()) storage.On("SimpleDownload", mock.Anything, path.Join("shares", share2.Id.OpaqueId)).Return(data2, nil) + storage.On("SimpleDownload", mock.Anything, mock.Anything).Return(nil, errtypes.NotFound("")) }) Describe("Share", func() { @@ -210,38 +212,26 @@ var _ = Describe("Manager", func() { Describe("GetShare", func() { Context("when the share is a user share ", func() { - It("returns a user share", func() { - returnedShare, err := m.GetShare(ctx, &collaboration.ShareReference{ - Spec: &collaboration.ShareReference_Id{Id: &collaboration.ShareId{OpaqueId: "1"}}, + Context("when requesting the share by id", func() { + It("returns NotFound", func() { + returnedShare, err := m.GetShare(ctx, &collaboration.ShareReference{ + Spec: &collaboration.ShareReference_Id{Id: &collaboration.ShareId{OpaqueId: "1000"}}, + }) + Expect(err).To(HaveOccurred()) + Expect(returnedShare).To(BeNil()) }) - Expect(err).ToNot(HaveOccurred()) - Expect(returnedShare).ToNot(BeNil()) - Expect(returnedShare.Id.OpaqueId).To(Equal(share.Id.OpaqueId)) - Expect(returnedShare.Owner).To(Equal(share.Owner)) - Expect(returnedShare.Grantee).To(Equal(share.Grantee)) - Expect(returnedShare.Permissions).To(Equal(share.Permissions)) - }) - It("gets the share by key", func() { - indexer.On("FindBy", mock.Anything, "OwnerId", url.QueryEscape(share.Owner.Idp+":"+share.Owner.OpaqueId)). - Return([]string{share.Id.OpaqueId, share2.Id.OpaqueId}, nil) - indexer.On("FindBy", mock.Anything, "GranteeId", url.QueryEscape("user:"+grantee.Id.Idp+":"+grantee.Id.OpaqueId)). - Return([]string{share2.Id.OpaqueId}, nil) - returnedShare, err := m.GetShare(ctx, &collaboration.ShareReference{ - Spec: &collaboration.ShareReference_Key{ - Key: &collaboration.ShareKey{ - Owner: share2.Owner, - ResourceId: share2.ResourceId, - Grantee: share2.Grantee, - }, - }, + It("returns the share", func() { + returnedShare, err := m.GetShare(ctx, &collaboration.ShareReference{ + Spec: &collaboration.ShareReference_Id{Id: &collaboration.ShareId{OpaqueId: "1"}}, + }) + Expect(err).ToNot(HaveOccurred()) + Expect(returnedShare).ToNot(BeNil()) + Expect(returnedShare.Id.OpaqueId).To(Equal(share.Id.OpaqueId)) + Expect(returnedShare.Owner).To(Equal(share.Owner)) + Expect(returnedShare.Grantee).To(Equal(share.Grantee)) + Expect(returnedShare.Permissions).To(Equal(share.Permissions)) }) - Expect(err).ToNot(HaveOccurred()) - Expect(returnedShare).ToNot(BeNil()) - Expect(returnedShare.Id.OpaqueId).To(Equal(share2.Id.OpaqueId)) - Expect(returnedShare.Owner).To(Equal(share2.Owner)) - Expect(returnedShare.Grantee).To(Equal(share2.Grantee)) - Expect(returnedShare.Permissions).To(Equal(share2.Permissions)) }) Context("when requesting the share by key", func() { From 1ed9666b8d53d02d3c534fbde0e02b1def6b7e70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Thu, 3 Mar 2022 16:12:02 +0100 Subject: [PATCH 05/22] Implement listing received shares --- .../mocks/SharesProviderClient.go | 2 +- .../ocdav/propfind/mocks/GatewayClient.go | 4 +- .../sharing/shares/mocks/GatewayClient.go | 2 +- pkg/share/manager/cs3/cs3.go | 98 ++++++++++++++++++- pkg/share/manager/cs3/cs3_test.go | 80 ++++++++++++++- pkg/share/manager/cs3/mocks/Indexer.go | 18 ++++ pkg/share/manager/cs3/mocks/Storage.go | 18 ++++ pkg/share/manager/sql/mocks/UserConverter.go | 2 +- pkg/share/mocks/Manager.go | 2 +- .../spaces/mocks/StorageProviderClient.go | 2 +- 10 files changed, 216 insertions(+), 12 deletions(-) diff --git a/internal/grpc/services/sharesstorageprovider/mocks/SharesProviderClient.go b/internal/grpc/services/sharesstorageprovider/mocks/SharesProviderClient.go index 747d497de1..5d16110bf8 100644 --- a/internal/grpc/services/sharesstorageprovider/mocks/SharesProviderClient.go +++ b/internal/grpc/services/sharesstorageprovider/mocks/SharesProviderClient.go @@ -1,4 +1,4 @@ -// Copyright 2018-2021 CERN +// Copyright 2018-2022 CERN // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. diff --git a/internal/http/services/owncloud/ocdav/propfind/mocks/GatewayClient.go b/internal/http/services/owncloud/ocdav/propfind/mocks/GatewayClient.go index 3856899566..9fb26407af 100644 --- a/internal/http/services/owncloud/ocdav/propfind/mocks/GatewayClient.go +++ b/internal/http/services/owncloud/ocdav/propfind/mocks/GatewayClient.go @@ -1,4 +1,4 @@ -// Copyright 2018-2021 CERN +// Copyright 2018-2022 CERN // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -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 v120.0. DO NOT EDIT. +// Code generated by mockery v1.0.0. DO NOT EDIT. package mocks 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 b0aaf7962e..2d45a1bb4e 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 @@ -1,4 +1,4 @@ -// Copyright 2018-2021 CERN +// Copyright 2018-2022 CERN // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. diff --git a/pkg/share/manager/cs3/cs3.go b/pkg/share/manager/cs3/cs3.go index 22bb48654d..36e088717e 100644 --- a/pkg/share/manager/cs3/cs3.go +++ b/pkg/share/manager/cs3/cs3.go @@ -26,6 +26,7 @@ import ( "path" "time" + groupv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" @@ -65,6 +66,11 @@ type Manager struct { initialized bool } +type ReceivedShareMetadata struct { + State collaboration.ShareState `json:"state"` + MountPoint *provider.Reference `json:"mountpoint"` +} + func init() { registry.Register("cs3", NewDefault) } @@ -194,8 +200,7 @@ func (m *Manager) GetShare(ctx context.Context, ref *collaboration.ShareReferenc } func (m *Manager) getShareById(ctx context.Context, id string) (*collaboration.Share, error) { - fn := path.Join("shares", id) - data, err := m.storage.SimpleDownload(ctx, fn) + data, err := m.storage.SimpleDownload(ctx, shareFilename(id)) if err != nil { return nil, err } @@ -295,7 +300,73 @@ func (m *Manager) ListReceivedShares(ctx context.Context, filters []*collaborati if err := m.initialize(); err != nil { return nil, err } - return nil, nil + + user, ok := ctxpkg.ContextGetUser(ctx) + if !ok { + return nil, errtypes.UserRequired("error getting user from context") + } + + result := []*collaboration.ReceivedShare{} + + ids, err := granteeToIndex(&provider.Grantee{ + Type: provider.GranteeType_GRANTEE_TYPE_USER, + Id: &provider.Grantee_UserId{UserId: user.Id}, + }) + if err != nil { + return nil, err + } + receivedIds, err := m.indexer.FindBy(&collaboration.Share{}, "GranteeId", ids) + if err != nil { + return nil, err + } + for _, group := range user.Groups { + index, err := granteeToIndex(&provider.Grantee{ + Type: provider.GranteeType_GRANTEE_TYPE_GROUP, + Id: &provider.Grantee_GroupId{GroupId: &groupv1beta1.GroupId{OpaqueId: group}}, + }) + if err != nil { + return nil, err + } + groupIds, err := m.indexer.FindBy(&collaboration.Share{}, "GranteeId", index) + if err != nil { + return nil, err + } + receivedIds = append(receivedIds, groupIds...) + } + + for _, id := range receivedIds { + shareFn := shareFilename(id) + data, err := m.storage.SimpleDownload(ctx, shareFn) + if err != nil { + return nil, err + } + + s, err := unmarshalShareData(data) + if err != nil { + return nil, err + } + + metadataFn, err := metadataFilename(s, user) + if err != nil { + return nil, err + } + data, err = m.storage.SimpleDownload(ctx, metadataFn) + if err != nil { + return nil, err + } + metadata := &ReceivedShareMetadata{} + err = json.Unmarshal(data, metadata) + if err != nil { + return nil, err + } + + result = append(result, &collaboration.ReceivedShare{ + Share: s, + State: metadata.State, + MountPoint: metadata.MountPoint, + }) + } + return result, nil } // GetReceivedShare returns the information for a received share. @@ -332,6 +403,25 @@ func unmarshalShareData(data []byte) (*collaboration.Share, error) { return nil, errtypes.InternalError("failed to unmarshal share data") } +func shareFilename(id string) string { + return path.Join("shares", id) +} + +func metadataFilename(s *collaboration.Share, g interface{}) (string, error) { + var granteePart string + switch v := g.(type) { + case *userpb.User: + granteePart = url.QueryEscape("user:" + v.Id.Idp + ":" + v.Id.OpaqueId) + case *provider.Grantee: + var err error + granteePart, err = granteeToIndex(v) + if err != nil { + return "", err + } + } + return path.Join("metadata", s.Id.OpaqueId, granteePart), nil +} + func indexOwnerFunc(v interface{}) (string, error) { share, ok := v.(*collaboration.Share) if !ok { @@ -357,7 +447,7 @@ func granteeToIndex(grantee *provider.Grantee) (string, error) { case grantee.Type == provider.GranteeType_GRANTEE_TYPE_USER: return url.QueryEscape("user:" + grantee.GetUserId().Idp + ":" + grantee.GetUserId().OpaqueId), nil case grantee.Type == provider.GranteeType_GRANTEE_TYPE_GROUP: - return url.QueryEscape("group:" + grantee.GetGroupId().Idp + ":" + grantee.GetGroupId().OpaqueId), nil + return url.QueryEscape("group:" + grantee.GetGroupId().OpaqueId), nil default: return "", fmt.Errorf("unknown grantee type") } diff --git a/pkg/share/manager/cs3/cs3_test.go b/pkg/share/manager/cs3/cs3_test.go index 248895bcb4..27ee9540fe 100644 --- a/pkg/share/manager/cs3/cs3_test.go +++ b/pkg/share/manager/cs3/cs3_test.go @@ -67,6 +67,7 @@ var _ = Describe("Manager", func() { Idp: "localhost:1111", OpaqueId: "1", }, + Groups: []string{"admins"}, } grantee = &userpb.User{ Id: &userpb.UserId{ @@ -314,7 +315,7 @@ var _ = Describe("Manager", func() { It("applies resource id filters", func() { shares, err := m.ListShares(ctx, []*collaboration.Filter{ - &collaboration.Filter{ + { Type: collaboration.Filter_TYPE_RESOURCE_ID, Term: &collaboration.Filter_ResourceId{ ResourceId: share.ResourceId, @@ -326,5 +327,82 @@ var _ = Describe("Manager", func() { Expect(shares[0].Id.OpaqueId).To(Equal("1")) }) }) + + Describe("ListReceivedShares", func() { + Context("with a received user share", func() { + BeforeEach(func() { + userFn := url.QueryEscape("user:" + user.Id.Idp + ":" + user.Id.OpaqueId) + indexer.On("FindBy", mock.Anything, "GranteeId", userFn). + Return([]string{share2.Id.OpaqueId}, nil) + indexer.On("FindBy", mock.Anything, "GranteeId", mock.Anything). + Return([]string{}, nil) + + data, err := json.Marshal(&cs3.ReceivedShareMetadata{ + State: collaboration.ShareState_SHARE_STATE_PENDING, + MountPoint: &provider.Reference{ + ResourceId: &provider.ResourceId{ + StorageId: "storageid", + OpaqueId: "opaqueid", + }, + Path: "path", + }, + }) + Expect(err).ToNot(HaveOccurred()) + storage.On("SimpleDownload", mock.Anything, path.Join("metadata", share2.Id.OpaqueId, userFn)). + Return(data, nil) + }) + + It("list the user shares", func() { + rshares, err := m.ListReceivedShares(ctx, []*collaboration.Filter{}) + Expect(err).ToNot(HaveOccurred()) + Expect(rshares).ToNot(BeNil()) + Expect(len(rshares)).To(Equal(1)) + + rshare := rshares[0] + Expect(rshare.State).To(Equal(collaboration.ShareState_SHARE_STATE_PENDING)) + Expect(rshare.MountPoint.ResourceId.StorageId).To(Equal("storageid")) + Expect(rshare.MountPoint.ResourceId.OpaqueId).To(Equal("opaqueid")) + Expect(rshare.MountPoint.Path).To(Equal("path")) + }) + }) + + Context("with a received group share", func() { + BeforeEach(func() { + userFn := url.QueryEscape("user:" + user.Id.Idp + ":" + user.Id.OpaqueId) + groupFn := url.QueryEscape("group:admins") + indexer.On("FindBy", mock.Anything, "GranteeId", groupFn). + Return([]string{share2.Id.OpaqueId}, nil) + indexer.On("FindBy", mock.Anything, "GranteeId", mock.Anything). + Return([]string{}, nil) + + data, err := json.Marshal(&cs3.ReceivedShareMetadata{ + State: collaboration.ShareState_SHARE_STATE_PENDING, + MountPoint: &provider.Reference{ + ResourceId: &provider.ResourceId{ + StorageId: "storageid", + OpaqueId: "opaqueid", + }, + Path: "path", + }, + }) + Expect(err).ToNot(HaveOccurred()) + storage.On("SimpleDownload", mock.Anything, path.Join("metadata", share2.Id.OpaqueId, userFn)). + Return(data, nil) + }) + + It("list the group share", func() { + rshares, err := m.ListReceivedShares(ctx, []*collaboration.Filter{}) + Expect(err).ToNot(HaveOccurred()) + Expect(rshares).ToNot(BeNil()) + Expect(len(rshares)).To(Equal(1)) + + rshare := rshares[0] + Expect(rshare.State).To(Equal(collaboration.ShareState_SHARE_STATE_PENDING)) + Expect(rshare.MountPoint.ResourceId.StorageId).To(Equal("storageid")) + Expect(rshare.MountPoint.ResourceId.OpaqueId).To(Equal("opaqueid")) + Expect(rshare.MountPoint.Path).To(Equal("path")) + }) + }) + }) }) }) diff --git a/pkg/share/manager/cs3/mocks/Indexer.go b/pkg/share/manager/cs3/mocks/Indexer.go index b53695381f..b3a213ad26 100644 --- a/pkg/share/manager/cs3/mocks/Indexer.go +++ b/pkg/share/manager/cs3/mocks/Indexer.go @@ -1,3 +1,21 @@ +// Copyright 2018-2022 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. + // Code generated by mockery v1.0.0. DO NOT EDIT. package mocks diff --git a/pkg/share/manager/cs3/mocks/Storage.go b/pkg/share/manager/cs3/mocks/Storage.go index 14d9b8d00e..8b77b1b3e6 100644 --- a/pkg/share/manager/cs3/mocks/Storage.go +++ b/pkg/share/manager/cs3/mocks/Storage.go @@ -1,3 +1,21 @@ +// Copyright 2018-2022 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. + // Code generated by mockery v1.0.0. DO NOT EDIT. package mocks diff --git a/pkg/share/manager/sql/mocks/UserConverter.go b/pkg/share/manager/sql/mocks/UserConverter.go index ba7660c64d..3bb0249d93 100644 --- a/pkg/share/manager/sql/mocks/UserConverter.go +++ b/pkg/share/manager/sql/mocks/UserConverter.go @@ -1,4 +1,4 @@ -// Copyright 2018-2021 CERN +// Copyright 2018-2022 CERN // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. diff --git a/pkg/share/mocks/Manager.go b/pkg/share/mocks/Manager.go index 5a80d362cf..3d7f54b988 100644 --- a/pkg/share/mocks/Manager.go +++ b/pkg/share/mocks/Manager.go @@ -1,4 +1,4 @@ -// Copyright 2018-2021 CERN +// Copyright 2018-2022 CERN // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. diff --git a/pkg/storage/registry/spaces/mocks/StorageProviderClient.go b/pkg/storage/registry/spaces/mocks/StorageProviderClient.go index 4e9a910712..60ed404aa8 100644 --- a/pkg/storage/registry/spaces/mocks/StorageProviderClient.go +++ b/pkg/storage/registry/spaces/mocks/StorageProviderClient.go @@ -1,4 +1,4 @@ -// Copyright 2018-2021 CERN +// Copyright 2018-2022 CERN // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. From e44385bb865ba9eb424d640d5f387b983167bbea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Thu, 3 Mar 2022 16:42:07 +0100 Subject: [PATCH 06/22] Implement GetReceivedShare, DRY up code --- pkg/share/manager/cs3/cs3.go | 62 +++++++++-------- pkg/share/manager/cs3/cs3_test.go | 109 ++++++++++++++++++------------ 2 files changed, 99 insertions(+), 72 deletions(-) diff --git a/pkg/share/manager/cs3/cs3.go b/pkg/share/manager/cs3/cs3.go index 36e088717e..cd1cee5632 100644 --- a/pkg/share/manager/cs3/cs3.go +++ b/pkg/share/manager/cs3/cs3.go @@ -271,12 +271,7 @@ func (m *Manager) ListShares(ctx context.Context, filters []*collaboration.Filte } result := []*collaboration.Share{} for _, id := range allShareIds { - data, err := m.storage.SimpleDownload(ctx, path.Join("shares", id)) - if err != nil { - return nil, err - } - - s, err := unmarshalShareData(data) + s, err := m.getShareById(ctx, id) if err != nil { return nil, err } @@ -335,33 +330,16 @@ func (m *Manager) ListReceivedShares(ctx context.Context, filters []*collaborati } for _, id := range receivedIds { - shareFn := shareFilename(id) - data, err := m.storage.SimpleDownload(ctx, shareFn) - if err != nil { - return nil, err - } - - s, err := unmarshalShareData(data) - if err != nil { - return nil, err - } - - metadataFn, err := metadataFilename(s, user) - if err != nil { - return nil, err - } - data, err = m.storage.SimpleDownload(ctx, metadataFn) + share, err := m.getShareById(ctx, id) if err != nil { return nil, err } - metadata := &ReceivedShareMetadata{} - err = json.Unmarshal(data, metadata) + metadata, err := m.downloadMetadata(ctx, share) if err != nil { return nil, err } - result = append(result, &collaboration.ReceivedShare{ - Share: s, + Share: share, State: metadata.State, MountPoint: metadata.MountPoint, }) @@ -374,7 +352,18 @@ func (m *Manager) GetReceivedShare(ctx context.Context, ref *collaboration.Share if err := m.initialize(); err != nil { return nil, err } - return nil, nil + + share, err := m.GetShare(ctx, ref) + if err != nil { + return nil, err + } + + metadata, err := m.downloadMetadata(ctx, share) + return &collaboration.ReceivedShare{ + Share: share, + State: metadata.State, + MountPoint: metadata.MountPoint, + }, nil } // UpdateReceivedShare updates the received share with share state. @@ -385,6 +374,25 @@ func (m *Manager) UpdateReceivedShare(ctx context.Context, share *collaboration. return nil, nil } +func (m *Manager) downloadMetadata(ctx context.Context, share *collaboration.Share) (ReceivedShareMetadata, error) { + user, ok := ctxpkg.ContextGetUser(ctx) + if !ok { + return ReceivedShareMetadata{}, errtypes.UserRequired("error getting user from context") + } + + metadataFn, err := metadataFilename(share, user) + if err != nil { + return ReceivedShareMetadata{}, err + } + data, err := m.storage.SimpleDownload(ctx, metadataFn) + if err != nil { + return ReceivedShareMetadata{}, err + } + metadata := ReceivedShareMetadata{} + err = json.Unmarshal(data, &metadata) + return metadata, err +} + func unmarshalShareData(data []byte) (*collaboration.Share, error) { userShare := &collaboration.Share{ Grantee: &provider.Grantee{Id: &provider.Grantee_UserId{}}, diff --git a/pkg/share/manager/cs3/cs3_test.go b/pkg/share/manager/cs3/cs3_test.go index 27ee9540fe..5b51535234 100644 --- a/pkg/share/manager/cs3/cs3_test.go +++ b/pkg/share/manager/cs3/cs3_test.go @@ -41,14 +41,18 @@ import ( var _ = Describe("Manager", func() { var ( - storage *mocks.Storage - indexer *mocks.Indexer - user *userpb.User - grantee *userpb.User - share *collaboration.Share - share2 *collaboration.Share - grant *collaboration.ShareGrant - ctx context.Context + storage *mocks.Storage + indexer *mocks.Indexer + user *userpb.User + grantee *userpb.User + share *collaboration.Share + share2 *collaboration.Share + grant *collaboration.ShareGrant + ctx context.Context + granteeCtx context.Context + + granteeFn string + groupFn string ) BeforeEach(func() { @@ -67,14 +71,16 @@ var _ = Describe("Manager", func() { Idp: "localhost:1111", OpaqueId: "1", }, - Groups: []string{"admins"}, } grantee = &userpb.User{ Id: &userpb.UserId{ Idp: "localhost:1111", OpaqueId: "2", }, + Groups: []string{"users"}, } + granteeFn = url.QueryEscape("user:" + grantee.Id.Idp + ":" + grantee.Id.OpaqueId) + groupFn = url.QueryEscape("group:users") grant = &collaboration.ShareGrant{ Grantee: &provider.Grantee{ @@ -128,6 +134,7 @@ var _ = Describe("Manager", func() { }, } ctx = ctxpkg.ContextSetUser(context.Background(), user) + granteeCtx = ctxpkg.ContextSetUser(context.Background(), grantee) }) Describe("New", func() { @@ -160,9 +167,22 @@ var _ = Describe("Manager", func() { data, err := json.Marshal(share) Expect(err).ToNot(HaveOccurred()) storage.On("SimpleDownload", mock.Anything, path.Join("shares", share.Id.OpaqueId)).Return(data, nil) - data2, err := json.Marshal(share2) + data, err = json.Marshal(share2) Expect(err).ToNot(HaveOccurred()) - storage.On("SimpleDownload", mock.Anything, path.Join("shares", share2.Id.OpaqueId)).Return(data2, nil) + storage.On("SimpleDownload", mock.Anything, path.Join("shares", share2.Id.OpaqueId)).Return(data, nil) + data, err = json.Marshal(&cs3.ReceivedShareMetadata{ + State: collaboration.ShareState_SHARE_STATE_PENDING, + MountPoint: &provider.Reference{ + ResourceId: &provider.ResourceId{ + StorageId: "storageid", + OpaqueId: "opaqueid", + }, + Path: "path", + }, + }) + Expect(err).ToNot(HaveOccurred()) + storage.On("SimpleDownload", mock.Anything, path.Join("metadata", share2.Id.OpaqueId, granteeFn)). + Return(data, nil) storage.On("SimpleDownload", mock.Anything, mock.Anything).Return(nil, errtypes.NotFound("")) }) @@ -331,29 +351,14 @@ var _ = Describe("Manager", func() { Describe("ListReceivedShares", func() { Context("with a received user share", func() { BeforeEach(func() { - userFn := url.QueryEscape("user:" + user.Id.Idp + ":" + user.Id.OpaqueId) - indexer.On("FindBy", mock.Anything, "GranteeId", userFn). + indexer.On("FindBy", mock.Anything, "GranteeId", granteeFn). Return([]string{share2.Id.OpaqueId}, nil) indexer.On("FindBy", mock.Anything, "GranteeId", mock.Anything). Return([]string{}, nil) - - data, err := json.Marshal(&cs3.ReceivedShareMetadata{ - State: collaboration.ShareState_SHARE_STATE_PENDING, - MountPoint: &provider.Reference{ - ResourceId: &provider.ResourceId{ - StorageId: "storageid", - OpaqueId: "opaqueid", - }, - Path: "path", - }, - }) - Expect(err).ToNot(HaveOccurred()) - storage.On("SimpleDownload", mock.Anything, path.Join("metadata", share2.Id.OpaqueId, userFn)). - Return(data, nil) }) It("list the user shares", func() { - rshares, err := m.ListReceivedShares(ctx, []*collaboration.Filter{}) + rshares, err := m.ListReceivedShares(granteeCtx, []*collaboration.Filter{}) Expect(err).ToNot(HaveOccurred()) Expect(rshares).ToNot(BeNil()) Expect(len(rshares)).To(Equal(1)) @@ -368,30 +373,14 @@ var _ = Describe("Manager", func() { Context("with a received group share", func() { BeforeEach(func() { - userFn := url.QueryEscape("user:" + user.Id.Idp + ":" + user.Id.OpaqueId) - groupFn := url.QueryEscape("group:admins") indexer.On("FindBy", mock.Anything, "GranteeId", groupFn). Return([]string{share2.Id.OpaqueId}, nil) indexer.On("FindBy", mock.Anything, "GranteeId", mock.Anything). Return([]string{}, nil) - - data, err := json.Marshal(&cs3.ReceivedShareMetadata{ - State: collaboration.ShareState_SHARE_STATE_PENDING, - MountPoint: &provider.Reference{ - ResourceId: &provider.ResourceId{ - StorageId: "storageid", - OpaqueId: "opaqueid", - }, - Path: "path", - }, - }) - Expect(err).ToNot(HaveOccurred()) - storage.On("SimpleDownload", mock.Anything, path.Join("metadata", share2.Id.OpaqueId, userFn)). - Return(data, nil) }) It("list the group share", func() { - rshares, err := m.ListReceivedShares(ctx, []*collaboration.Filter{}) + rshares, err := m.ListReceivedShares(granteeCtx, []*collaboration.Filter{}) Expect(err).ToNot(HaveOccurred()) Expect(rshares).ToNot(BeNil()) Expect(len(rshares)).To(Equal(1)) @@ -404,5 +393,35 @@ var _ = Describe("Manager", func() { }) }) }) + + Describe("GetReceivedShare", func() { + Context("when the share is a user share ", func() { + Context("when requesting the share by id", func() { + It("returns NotFound", func() { + rshare, err := m.GetReceivedShare(granteeCtx, &collaboration.ShareReference{ + Spec: &collaboration.ShareReference_Id{Id: &collaboration.ShareId{OpaqueId: "1000"}}, + }) + Expect(err).To(HaveOccurred()) + Expect(rshare).To(BeNil()) + }) + + It("returns the share", func() { + rshare, err := m.GetReceivedShare(granteeCtx, &collaboration.ShareReference{ + Spec: &collaboration.ShareReference_Id{Id: &collaboration.ShareId{OpaqueId: share2.Id.OpaqueId}}, + }) + Expect(err).ToNot(HaveOccurred()) + Expect(rshare).ToNot(BeNil()) + Expect(rshare.Share.Id.OpaqueId).To(Equal(share2.Id.OpaqueId)) + Expect(rshare.Share.Owner).To(Equal(share2.Owner)) + Expect(rshare.Share.Grantee).To(Equal(share2.Grantee)) + Expect(rshare.Share.Permissions).To(Equal(share2.Permissions)) + Expect(rshare.State).To(Equal(collaboration.ShareState_SHARE_STATE_PENDING)) + Expect(rshare.MountPoint.ResourceId.StorageId).To(Equal("storageid")) + Expect(rshare.MountPoint.ResourceId.OpaqueId).To(Equal("opaqueid")) + Expect(rshare.MountPoint.Path).To(Equal("path")) + }) + }) + }) + }) }) }) From 30f0d64d60fc296221563bf2ecdc240294f7bace Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Thu, 3 Mar 2022 17:01:20 +0100 Subject: [PATCH 07/22] Refactor --- pkg/share/manager/cs3/cs3.go | 37 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/pkg/share/manager/cs3/cs3.go b/pkg/share/manager/cs3/cs3.go index cd1cee5632..04ccf77ddd 100644 --- a/pkg/share/manager/cs3/cs3.go +++ b/pkg/share/manager/cs3/cs3.go @@ -204,7 +204,24 @@ func (m *Manager) getShareById(ctx context.Context, id string) (*collaboration.S if err != nil { return nil, err } - return unmarshalShareData(data) + + userShare := &collaboration.Share{ + Grantee: &provider.Grantee{Id: &provider.Grantee_UserId{}}, + } + err = json.Unmarshal(data, userShare) + if err == nil && userShare.Grantee.GetUserId() != nil { + return userShare, nil + } + + groupShare := &collaboration.Share{ + Grantee: &provider.Grantee{Id: &provider.Grantee_GroupId{}}, + } + err = json.Unmarshal(data, groupShare) // try to unmarshal to a group share if the user share unmarshalling failed + if err == nil && groupShare.Grantee.GetGroupId() != nil { + return groupShare, nil + } + + return nil, errtypes.InternalError("failed to unmarshal share data") } func (m *Manager) getShareByKey(ctx context.Context, key *collaboration.ShareKey) (*collaboration.Share, error) { @@ -393,24 +410,6 @@ func (m *Manager) downloadMetadata(ctx context.Context, share *collaboration.Sha return metadata, err } -func unmarshalShareData(data []byte) (*collaboration.Share, error) { - userShare := &collaboration.Share{ - Grantee: &provider.Grantee{Id: &provider.Grantee_UserId{}}, - } - groupShare := &collaboration.Share{ - Grantee: &provider.Grantee{Id: &provider.Grantee_GroupId{}}, - } - err := json.Unmarshal(data, userShare) - if err == nil && userShare.Grantee.GetUserId() != nil { - return userShare, nil - } - err = json.Unmarshal(data, groupShare) // try to unmarshal to a group share if the user share unmarshalling failed - if err == nil && groupShare.Grantee.GetGroupId() != nil { - return groupShare, nil - } - return nil, errtypes.InternalError("failed to unmarshal share data") -} - func shareFilename(id string) string { return path.Join("shares", id) } From e1d432dc749cfde7bfd5375672c5c01f557f7083 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Fri, 4 Mar 2022 09:02:58 +0100 Subject: [PATCH 08/22] Implement UpdateShare and UpdateReceivedShare --- pkg/share/manager/cs3/cs3.go | 155 +++++++++++++++++++----------- pkg/share/manager/cs3/cs3_test.go | 29 ++++++ 2 files changed, 128 insertions(+), 56 deletions(-) diff --git a/pkg/share/manager/cs3/cs3.go b/pkg/share/manager/cs3/cs3.go index 04ccf77ddd..2de308d675 100644 --- a/pkg/share/manager/cs3/cs3.go +++ b/pkg/share/manager/cs3/cs3.go @@ -196,59 +196,6 @@ func (m *Manager) GetShare(ctx context.Context, ref *collaboration.ShareReferenc default: return nil, errtypes.BadRequest("neither share id nor key was given") } - -} - -func (m *Manager) getShareById(ctx context.Context, id string) (*collaboration.Share, error) { - data, err := m.storage.SimpleDownload(ctx, shareFilename(id)) - if err != nil { - return nil, err - } - - userShare := &collaboration.Share{ - Grantee: &provider.Grantee{Id: &provider.Grantee_UserId{}}, - } - err = json.Unmarshal(data, userShare) - if err == nil && userShare.Grantee.GetUserId() != nil { - return userShare, nil - } - - groupShare := &collaboration.Share{ - Grantee: &provider.Grantee{Id: &provider.Grantee_GroupId{}}, - } - err = json.Unmarshal(data, groupShare) // try to unmarshal to a group share if the user share unmarshalling failed - if err == nil && groupShare.Grantee.GetGroupId() != nil { - return groupShare, nil - } - - return nil, errtypes.InternalError("failed to unmarshal share data") -} - -func (m *Manager) getShareByKey(ctx context.Context, key *collaboration.ShareKey) (*collaboration.Share, error) { - ownerIds, err := m.indexer.FindBy(&collaboration.Share{}, "OwnerId", userIdToIndex(key.Owner)) - if err != nil { - return nil, err - } - granteeIndex, err := granteeToIndex(key.Grantee) - if err != nil { - return nil, err - } - granteeIds, err := m.indexer.FindBy(&collaboration.Share{}, "GranteeId", granteeIndex) - if err != nil { - return nil, err - } - - ids := intersectSlices(ownerIds, granteeIds) - for _, id := range ids { - share, err := m.getShareById(ctx, id) - if err != nil { - return nil, err - } - if utils.ResourceIDEqual(share.ResourceId, key.ResourceId) { - return share, nil - } - } - return nil, errtypes.NotFound("share not found") } // Unshare deletes the share pointed by ref. @@ -304,7 +251,24 @@ func (m *Manager) UpdateShare(ctx context.Context, ref *collaboration.ShareRefer if err := m.initialize(); err != nil { return nil, err } - return nil, nil + share, err := m.GetShare(ctx, ref) + if err != nil { + return nil, err + } + share.Permissions = p + + data, err := json.Marshal(share) + if err != nil { + return nil, err + } + + fn := path.Join("shares", share.Id.OpaqueId) + err = m.storage.SimpleUpload(ctx, fn, data) + if err != nil { + return nil, err + } + + return share, nil } // ListReceivedShares returns the list of shares the user has access to. @@ -376,6 +340,9 @@ func (m *Manager) GetReceivedShare(ctx context.Context, ref *collaboration.Share } metadata, err := m.downloadMetadata(ctx, share) + if err != nil { + return nil, err + } return &collaboration.ReceivedShare{ Share: share, State: metadata.State, @@ -384,11 +351,35 @@ func (m *Manager) GetReceivedShare(ctx context.Context, ref *collaboration.Share } // UpdateReceivedShare updates the received share with share state. -func (m *Manager) UpdateReceivedShare(ctx context.Context, share *collaboration.ReceivedShare, fieldMask *field_mask.FieldMask) (*collaboration.ReceivedShare, error) { +func (m *Manager) UpdateReceivedShare(ctx context.Context, rshare *collaboration.ReceivedShare, fieldMask *field_mask.FieldMask) (*collaboration.ReceivedShare, error) { if err := m.initialize(); err != nil { return nil, err } - return nil, nil + + user, ok := ctxpkg.ContextGetUser(ctx) + if !ok { + return nil, errtypes.UserRequired("error getting user from context") + } + + meta := ReceivedShareMetadata{ + State: rshare.State, + MountPoint: rshare.MountPoint, + } + data, err := json.Marshal(meta) + if err != nil { + return nil, err + } + + fn, err := metadataFilename(rshare.Share, user) + if err != nil { + return nil, err + } + err = m.storage.SimpleUpload(ctx, fn, data) + if err != nil { + return nil, err + } + + return rshare, nil } func (m *Manager) downloadMetadata(ctx context.Context, share *collaboration.Share) (ReceivedShareMetadata, error) { @@ -410,6 +401,58 @@ func (m *Manager) downloadMetadata(ctx context.Context, share *collaboration.Sha return metadata, err } +func (m *Manager) getShareById(ctx context.Context, id string) (*collaboration.Share, error) { + data, err := m.storage.SimpleDownload(ctx, shareFilename(id)) + if err != nil { + return nil, err + } + + userShare := &collaboration.Share{ + Grantee: &provider.Grantee{Id: &provider.Grantee_UserId{}}, + } + err = json.Unmarshal(data, userShare) + if err == nil && userShare.Grantee.GetUserId() != nil { + return userShare, nil + } + + groupShare := &collaboration.Share{ + Grantee: &provider.Grantee{Id: &provider.Grantee_GroupId{}}, + } + err = json.Unmarshal(data, groupShare) // try to unmarshal to a group share if the user share unmarshalling failed + if err == nil && groupShare.Grantee.GetGroupId() != nil { + return groupShare, nil + } + + return nil, errtypes.InternalError("failed to unmarshal share data") +} + +func (m *Manager) getShareByKey(ctx context.Context, key *collaboration.ShareKey) (*collaboration.Share, error) { + ownerIds, err := m.indexer.FindBy(&collaboration.Share{}, "OwnerId", userIdToIndex(key.Owner)) + if err != nil { + return nil, err + } + granteeIndex, err := granteeToIndex(key.Grantee) + if err != nil { + return nil, err + } + granteeIds, err := m.indexer.FindBy(&collaboration.Share{}, "GranteeId", granteeIndex) + if err != nil { + return nil, err + } + + ids := intersectSlices(ownerIds, granteeIds) + for _, id := range ids { + share, err := m.getShareById(ctx, id) + if err != nil { + return nil, err + } + if utils.ResourceIDEqual(share.ResourceId, key.ResourceId) { + return share, nil + } + } + return nil, errtypes.NotFound("share not found") +} + func shareFilename(id string) string { return path.Join("shares", id) } diff --git a/pkg/share/manager/cs3/cs3_test.go b/pkg/share/manager/cs3/cs3_test.go index 5b51535234..42dfb436f9 100644 --- a/pkg/share/manager/cs3/cs3_test.go +++ b/pkg/share/manager/cs3/cs3_test.go @@ -34,6 +34,7 @@ import ( "github.com/cs3org/reva/pkg/share/manager/cs3/mocks" indexerpkg "github.com/cs3org/reva/pkg/storage/utils/indexer" "github.com/stretchr/testify/mock" + "google.golang.org/protobuf/types/known/fieldmaskpb" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -231,6 +232,17 @@ var _ = Describe("Manager", func() { }) }) + Describe("UpdateShare", func() { + It("updates the share", func() { + Expect(share.Permissions.Permissions.AddGrant).To(BeFalse()) + s, err := m.UpdateShare(ctx, + &collaboration.ShareReference{Spec: &collaboration.ShareReference_Id{Id: share.Id}}, + &collaboration.SharePermissions{Permissions: &provider.ResourcePermissions{AddGrant: true}}) + Expect(err).ToNot(HaveOccurred()) + Expect(s).ToNot(BeNil()) + Expect(s.Permissions.Permissions.AddGrant).To(BeTrue()) + }) + }) Describe("GetShare", func() { Context("when the share is a user share ", func() { Context("when requesting the share by id", func() { @@ -423,5 +435,22 @@ var _ = Describe("Manager", func() { }) }) }) + + Describe("UpdateReceivedShare", func() { + It("updates the share", func() { + rs, err := m.GetReceivedShare(granteeCtx, &collaboration.ShareReference{Spec: &collaboration.ShareReference_Id{Id: share2.Id}}) + Expect(err).ToNot(HaveOccurred()) + + Expect(rs.State).To(Equal(collaboration.ShareState_SHARE_STATE_PENDING)) + rs.State = collaboration.ShareState_SHARE_STATE_ACCEPTED + + rrs, err := m.UpdateReceivedShare(granteeCtx, + rs, &fieldmaskpb.FieldMask{Paths: []string{"state"}}) + Expect(err).ToNot(HaveOccurred()) + Expect(rrs).ToNot(BeNil()) + Expect(rrs.State).To(Equal(collaboration.ShareState_SHARE_STATE_ACCEPTED)) + storage.AssertCalled(GinkgoT(), "SimpleUpload", mock.Anything, "metadata/2/"+granteeFn, mock.Anything) + }) + }) }) }) From d75ff44da5849318d7883c114c182b9930370da3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Mon, 7 Mar 2022 08:44:31 +0100 Subject: [PATCH 09/22] Fix accepting/declining received shares --- pkg/share/manager/cs3/cs3.go | 27 +++++++++++++++++++++++++-- pkg/storage/utils/indexer/indexer.go | 3 ++- pkg/storage/utils/metadata/cs3.go | 2 +- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/pkg/share/manager/cs3/cs3.go b/pkg/share/manager/cs3/cs3.go index 2de308d675..d26112ef85 100644 --- a/pkg/share/manager/cs3/cs3.go +++ b/pkg/share/manager/cs3/cs3.go @@ -120,6 +120,9 @@ func (m *Manager) initialize() error { if err := m.storage.MakeDirIfNotExist(context.Background(), "shares"); err != nil { return err } + if err := m.storage.MakeDirIfNotExist(context.Background(), "metadata"); err != nil { + return err + } err = m.indexer.AddIndex(&collaboration.Share{}, option.IndexByFunc{ Name: "OwnerId", Func: indexOwnerFunc, @@ -174,6 +177,12 @@ func (m *Manager) Share(ctx context.Context, md *provider.ResourceInfo, g *colla return nil, err } + metadataPath := path.Join("metadata", share.Id.OpaqueId) + err = m.storage.MakeDirIfNotExist(ctx, metadataPath) + if err != nil { + return nil, err + } + _, err = m.indexer.Add(share) if err != nil { return nil, err @@ -317,7 +326,14 @@ func (m *Manager) ListReceivedShares(ctx context.Context, filters []*collaborati } metadata, err := m.downloadMetadata(ctx, share) if err != nil { - return nil, err + if _, ok := err.(errtypes.NotFound); ok { + // use default values if the grantee didn't configure anything yet + metadata = ReceivedShareMetadata{ + State: collaboration.ShareState_SHARE_STATE_PENDING, + } + } else { + return nil, err + } } result = append(result, &collaboration.ReceivedShare{ Share: share, @@ -341,7 +357,14 @@ func (m *Manager) GetReceivedShare(ctx context.Context, ref *collaboration.Share metadata, err := m.downloadMetadata(ctx, share) if err != nil { - return nil, err + if _, ok := err.(errtypes.NotFound); ok { + // use default values if the grantee didn't configure anything yet + metadata = ReceivedShareMetadata{ + State: collaboration.ShareState_SHARE_STATE_PENDING, + } + } else { + return nil, err + } } return &collaboration.ReceivedShare{ Share: share, diff --git a/pkg/storage/utils/indexer/indexer.go b/pkg/storage/utils/indexer/indexer.go index 5ae1e2dfe3..9806aa5a4f 100644 --- a/pkg/storage/utils/indexer/indexer.go +++ b/pkg/storage/utils/indexer/indexer.go @@ -29,6 +29,7 @@ import ( "github.com/CiscoM31/godata" "github.com/iancoleman/strcase" + "github.com/cs3org/reva/v2/pkg/errtypes" errorspkg "github.com/cs3org/reva/v2/pkg/storage/utils/indexer/errors" "github.com/cs3org/reva/v2/pkg/storage/utils/indexer/index" "github.com/cs3org/reva/v2/pkg/storage/utils/indexer/option" @@ -148,7 +149,7 @@ func (i *Indexer) FindBy(t interface{}, findBy, val string) ([]string, error) { idxVal := val res, err := idx.Lookup(idxVal) if err != nil { - if errorspkg.IsNotFoundErr(err) { + if _, ok := err.(errtypes.NotFound); ok { continue } diff --git a/pkg/storage/utils/metadata/cs3.go b/pkg/storage/utils/metadata/cs3.go index ca522ca842..5e3ba503c7 100644 --- a/pkg/storage/utils/metadata/cs3.go +++ b/pkg/storage/utils/metadata/cs3.go @@ -277,7 +277,7 @@ func (cs3 *CS3) ReadDir(ctx context.Context, path string) ([]string, error) { return nil, err } if res.Status.Code != rpc.Code_CODE_OK { - return nil, fmt.Errorf("error listing directory: %v", path) + return nil, errtypes.NewErrtypeFromStatus(res.Status) } entries := []string{} From 340fd36066193cb77a8a37f99abf063fa1d83aca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Tue, 8 Mar 2022 14:54:25 +0100 Subject: [PATCH 10/22] Adapt to changed reva code --- pkg/share/manager/cs3/cs3.go | 19 ++++++++++--------- pkg/share/manager/cs3/cs3_test.go | 10 +++++----- pkg/share/manager/cs3/mocks/Indexer.go | 4 ++-- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/pkg/share/manager/cs3/cs3.go b/pkg/share/manager/cs3/cs3.go index d26112ef85..d115b80f0f 100644 --- a/pkg/share/manager/cs3/cs3.go +++ b/pkg/share/manager/cs3/cs3.go @@ -31,14 +31,14 @@ import ( collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" typespb "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/share" - "github.com/cs3org/reva/pkg/share/manager/registry" - "github.com/cs3org/reva/pkg/storage/utils/indexer" - "github.com/cs3org/reva/pkg/storage/utils/indexer/option" - "github.com/cs3org/reva/pkg/storage/utils/metadata" - "github.com/cs3org/reva/pkg/utils" + ctxpkg "github.com/cs3org/reva/v2/pkg/ctx" + "github.com/cs3org/reva/v2/pkg/errtypes" + "github.com/cs3org/reva/v2/pkg/share" + "github.com/cs3org/reva/v2/pkg/share/manager/registry" + "github.com/cs3org/reva/v2/pkg/storage/utils/indexer" + "github.com/cs3org/reva/v2/pkg/storage/utils/indexer/option" + "github.com/cs3org/reva/v2/pkg/storage/utils/metadata" + "github.com/cs3org/reva/v2/pkg/utils" "github.com/google/uuid" "github.com/mitchellh/mapstructure" "github.com/pkg/errors" @@ -79,6 +79,7 @@ type config struct { GatewayAddr string `mapstructure:"gateway_addr"` ProviderAddr string `mapstructure:"provider_addr"` ServiceUserID string `mapstructure:"service_user_id"` + ServiceUserIdp string `mapstructure:"service_user_idp"` MachineAuthAPIKey string `mapstructure:"machine_auth_apikey"` } @@ -90,7 +91,7 @@ func NewDefault(m map[string]interface{}) (share.Manager, error) { return nil, err } - s, err := metadata.NewCS3Storage(c.GatewayAddr, c.ProviderAddr, c.ServiceUserID, c.MachineAuthAPIKey) + s, err := metadata.NewCS3Storage(c.GatewayAddr, c.ProviderAddr, c.ServiceUserID, c.ServiceUserIdp, c.MachineAuthAPIKey) if err != nil { return nil, err } diff --git a/pkg/share/manager/cs3/cs3_test.go b/pkg/share/manager/cs3/cs3_test.go index 42dfb436f9..c2913be9f2 100644 --- a/pkg/share/manager/cs3/cs3_test.go +++ b/pkg/share/manager/cs3/cs3_test.go @@ -28,11 +28,11 @@ import ( userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" - ctxpkg "github.com/cs3org/reva/pkg/ctx" - "github.com/cs3org/reva/pkg/errtypes" - "github.com/cs3org/reva/pkg/share/manager/cs3" - "github.com/cs3org/reva/pkg/share/manager/cs3/mocks" - indexerpkg "github.com/cs3org/reva/pkg/storage/utils/indexer" + ctxpkg "github.com/cs3org/reva/v2/pkg/ctx" + "github.com/cs3org/reva/v2/pkg/errtypes" + "github.com/cs3org/reva/v2/pkg/share/manager/cs3" + "github.com/cs3org/reva/v2/pkg/share/manager/cs3/mocks" + indexerpkg "github.com/cs3org/reva/v2/pkg/storage/utils/indexer" "github.com/stretchr/testify/mock" "google.golang.org/protobuf/types/known/fieldmaskpb" diff --git a/pkg/share/manager/cs3/mocks/Indexer.go b/pkg/share/manager/cs3/mocks/Indexer.go index b3a213ad26..ca207fad23 100644 --- a/pkg/share/manager/cs3/mocks/Indexer.go +++ b/pkg/share/manager/cs3/mocks/Indexer.go @@ -21,10 +21,10 @@ package mocks import ( - indexer "github.com/cs3org/reva/pkg/storage/utils/indexer" + indexer "github.com/cs3org/reva/v2/pkg/storage/utils/indexer" mock "github.com/stretchr/testify/mock" - option "github.com/cs3org/reva/pkg/storage/utils/indexer/option" + option "github.com/cs3org/reva/v2/pkg/storage/utils/indexer/option" ) // Indexer is an autogenerated mock type for the Indexer type From 48ddfccb4074e04476716624e2584834ebd686a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Tue, 8 Mar 2022 14:54:51 +0100 Subject: [PATCH 11/22] Finish UpdateReceivedShare --- pkg/share/manager/cs3/cs3.go | 24 +++++++++++++++++--- pkg/share/manager/cs3/cs3_test.go | 37 ++++++++++++++++++++++++++++--- 2 files changed, 55 insertions(+), 6 deletions(-) diff --git a/pkg/share/manager/cs3/cs3.go b/pkg/share/manager/cs3/cs3.go index d115b80f0f..6517dd5e0c 100644 --- a/pkg/share/manager/cs3/cs3.go +++ b/pkg/share/manager/cs3/cs3.go @@ -385,10 +385,28 @@ func (m *Manager) UpdateReceivedShare(ctx context.Context, rshare *collaboration return nil, errtypes.UserRequired("error getting user from context") } + rs, err := m.GetReceivedShare(ctx, &collaboration.ShareReference{Spec: &collaboration.ShareReference_Id{Id: rshare.Share.Id}}) + if err != nil { + return nil, err + } + meta := ReceivedShareMetadata{ - State: rshare.State, - MountPoint: rshare.MountPoint, + State: rs.State, + MountPoint: rs.MountPoint, + } + for i := range fieldMask.Paths { + switch fieldMask.Paths[i] { + case "state": + meta.State = rshare.State + rs.State = rshare.State + case "mount_point": + meta.MountPoint = rshare.MountPoint + rs.MountPoint = rshare.MountPoint + default: + return nil, errtypes.NotSupported("updating " + fieldMask.Paths[i] + " is not supported") + } } + data, err := json.Marshal(meta) if err != nil { return nil, err @@ -403,7 +421,7 @@ func (m *Manager) UpdateReceivedShare(ctx context.Context, rshare *collaboration return nil, err } - return rshare, nil + return rs, nil } func (m *Manager) downloadMetadata(ctx context.Context, share *collaboration.Share) (ReceivedShareMetadata, error) { diff --git a/pkg/share/manager/cs3/cs3_test.go b/pkg/share/manager/cs3/cs3_test.go index c2913be9f2..2b2aad3fa2 100644 --- a/pkg/share/manager/cs3/cs3_test.go +++ b/pkg/share/manager/cs3/cs3_test.go @@ -443,13 +443,44 @@ var _ = Describe("Manager", func() { Expect(rs.State).To(Equal(collaboration.ShareState_SHARE_STATE_PENDING)) rs.State = collaboration.ShareState_SHARE_STATE_ACCEPTED + rs.MountPoint.Path = "newPath/" rrs, err := m.UpdateReceivedShare(granteeCtx, - rs, &fieldmaskpb.FieldMask{Paths: []string{"state"}}) + rs, &fieldmaskpb.FieldMask{Paths: []string{"state", "mount_point"}}) Expect(err).ToNot(HaveOccurred()) Expect(rrs).ToNot(BeNil()) - Expect(rrs.State).To(Equal(collaboration.ShareState_SHARE_STATE_ACCEPTED)) - storage.AssertCalled(GinkgoT(), "SimpleUpload", mock.Anything, "metadata/2/"+granteeFn, mock.Anything) + Expect(rrs.Share.ResourceId).ToNot(BeNil()) + storage.AssertCalled(GinkgoT(), "SimpleUpload", mock.Anything, "metadata/2/"+granteeFn, mock.MatchedBy(func(data []byte) bool { + meta := cs3.ReceivedShareMetadata{} + err := json.Unmarshal(data, &meta) + Expect(err).ToNot(HaveOccurred()) + Expect(meta.State).To(Equal(collaboration.ShareState_SHARE_STATE_ACCEPTED)) + Expect(meta.MountPoint.Path).To(Equal("newPath/")) + return true + })) + }) + + It("does not update fields that aren't part of the fieldmask", func() { + rs, err := m.GetReceivedShare(granteeCtx, &collaboration.ShareReference{Spec: &collaboration.ShareReference_Id{Id: share2.Id}}) + Expect(err).ToNot(HaveOccurred()) + + Expect(rs.State).To(Equal(collaboration.ShareState_SHARE_STATE_PENDING)) + rs.State = collaboration.ShareState_SHARE_STATE_ACCEPTED + rs.MountPoint.Path = "newPath/" + + rrs, err := m.UpdateReceivedShare(granteeCtx, + rs, &fieldmaskpb.FieldMask{Paths: []string{"mount_point"}}) + Expect(err).ToNot(HaveOccurred()) + Expect(rrs).ToNot(BeNil()) + Expect(rrs.Share.ResourceId).ToNot(BeNil()) + storage.AssertCalled(GinkgoT(), "SimpleUpload", mock.Anything, "metadata/2/"+granteeFn, mock.MatchedBy(func(data []byte) bool { + meta := cs3.ReceivedShareMetadata{} + err := json.Unmarshal(data, &meta) + Expect(err).ToNot(HaveOccurred()) + Expect(meta.State).To(Equal(collaboration.ShareState_SHARE_STATE_PENDING)) + Expect(meta.MountPoint.Path).To(Equal("newPath/")) + return true + })) }) }) }) From 45dc5e85533f7675e6f22d8fc6dde2b61a922b23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Tue, 8 Mar 2022 14:55:08 +0100 Subject: [PATCH 12/22] Return the proper error --- pkg/storage/utils/metadata/cs3.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/storage/utils/metadata/cs3.go b/pkg/storage/utils/metadata/cs3.go index 5e3ba503c7..ec702120fe 100644 --- a/pkg/storage/utils/metadata/cs3.go +++ b/pkg/storage/utils/metadata/cs3.go @@ -326,7 +326,7 @@ func (cs3 *CS3) MakeDirIfNotExist(ctx context.Context, folder string) error { } if r.Status.Code != rpc.Code_CODE_OK { - return errtypes.NewErrtypeFromStatus(resp.Status) + return errtypes.NewErrtypeFromStatus(r.Status) } default: return errtypes.NewErrtypeFromStatus(resp.Status) From 53ab1e5af4078e267da0afff61c44af19b325871 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Wed, 9 Mar 2022 14:12:12 +0100 Subject: [PATCH 13/22] Add changelog --- changelog/unreleased/cs3-share-manager.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelog/unreleased/cs3-share-manager.md diff --git a/changelog/unreleased/cs3-share-manager.md b/changelog/unreleased/cs3-share-manager.md new file mode 100644 index 0000000000..ae583755dd --- /dev/null +++ b/changelog/unreleased/cs3-share-manager.md @@ -0,0 +1,6 @@ +Enhancement: add new share manager + +We added a new share manager which uses the new metadata storage backend for +persisting the share information. + +https://github.com/cs3org/reva/pull/2626 \ No newline at end of file From c6e00d35fc95f8b51eb7e6bb5699f12209919987 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Wed, 9 Mar 2022 14:16:59 +0100 Subject: [PATCH 14/22] Make hound happy (woof woof) --- pkg/share/manager/cs3/cs3.go | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/pkg/share/manager/cs3/cs3.go b/pkg/share/manager/cs3/cs3.go index 6517dd5e0c..009270ef4a 100644 --- a/pkg/share/manager/cs3/cs3.go +++ b/pkg/share/manager/cs3/cs3.go @@ -47,10 +47,13 @@ import ( //go:generate mockery -name Storage //go:generate mockery -name Indexer + +// Storage is the interface to the metadata storage backend type Storage interface { metadata.Storage } +// Indexer is the interface to the indexer being used for indexing shares type Indexer interface { AddIndex(t interface{}, indexBy option.IndexBy, pkName, entityDirName, indexType string, bound *option.Bound, caseInsensitive bool) error Add(t interface{}) ([]indexer.IdxAddResult, error) @@ -66,6 +69,7 @@ type Manager struct { initialized bool } +// ReceivedShareMetadata hold the state information or a received share type ReceivedShareMetadata struct { State collaboration.ShareState `json:"state"` MountPoint *provider.Reference `json:"mountpoint"` @@ -200,7 +204,7 @@ func (m *Manager) GetShare(ctx context.Context, ref *collaboration.ShareReferenc switch { case ref.GetId() != nil: - return m.getShareById(ctx, ref.GetId().OpaqueId) + return m.getShareByID(ctx, ref.GetId().OpaqueId) case ref.GetKey() != nil: return m.getShareByKey(ctx, ref.GetKey()) default: @@ -239,13 +243,13 @@ func (m *Manager) ListShares(ctx context.Context, filters []*collaboration.Filte return nil, errtypes.UserRequired("error getting user from context") } - allShareIds, err := m.indexer.FindBy(&collaboration.Share{}, "OwnerId", userIdToIndex(user.GetId())) + allShareIds, err := m.indexer.FindBy(&collaboration.Share{}, "OwnerId", userIDToIndex(user.GetId())) if err != nil { return nil, err } result := []*collaboration.Share{} for _, id := range allShareIds { - s, err := m.getShareById(ctx, id) + s, err := m.getShareByID(ctx, id) if err != nil { return nil, err } @@ -321,7 +325,7 @@ func (m *Manager) ListReceivedShares(ctx context.Context, filters []*collaborati } for _, id := range receivedIds { - share, err := m.getShareById(ctx, id) + share, err := m.getShareByID(ctx, id) if err != nil { return nil, err } @@ -443,7 +447,7 @@ func (m *Manager) downloadMetadata(ctx context.Context, share *collaboration.Sha return metadata, err } -func (m *Manager) getShareById(ctx context.Context, id string) (*collaboration.Share, error) { +func (m *Manager) getShareByID(ctx context.Context, id string) (*collaboration.Share, error) { data, err := m.storage.SimpleDownload(ctx, shareFilename(id)) if err != nil { return nil, err @@ -469,7 +473,7 @@ func (m *Manager) getShareById(ctx context.Context, id string) (*collaboration.S } func (m *Manager) getShareByKey(ctx context.Context, key *collaboration.ShareKey) (*collaboration.Share, error) { - ownerIds, err := m.indexer.FindBy(&collaboration.Share{}, "OwnerId", userIdToIndex(key.Owner)) + ownerIds, err := m.indexer.FindBy(&collaboration.Share{}, "OwnerId", userIDToIndex(key.Owner)) if err != nil { return nil, err } @@ -484,7 +488,7 @@ func (m *Manager) getShareByKey(ctx context.Context, key *collaboration.ShareKey ids := intersectSlices(ownerIds, granteeIds) for _, id := range ids { - share, err := m.getShareById(ctx, id) + share, err := m.getShareByID(ctx, id) if err != nil { return nil, err } @@ -519,10 +523,10 @@ func indexOwnerFunc(v interface{}) (string, error) { if !ok { return "", fmt.Errorf("given entity is not a share") } - return userIdToIndex(share.Owner), nil + return userIDToIndex(share.Owner), nil } -func userIdToIndex(id *userpb.UserId) string { +func userIDToIndex(id *userpb.UserId) string { return url.QueryEscape(id.Idp + ":" + id.OpaqueId) } From 7bceda9359baea32637c903158b307ae4bc49ad1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Wed, 9 Mar 2022 14:51:28 +0100 Subject: [PATCH 15/22] Fix error type checking --- pkg/storage/utils/indexer/errors/errors.go | 14 ++++---------- pkg/storage/utils/indexer/index/unique_test.go | 5 ----- pkg/storage/utils/indexer/indexer.go | 5 ++--- 3 files changed, 6 insertions(+), 18 deletions(-) diff --git a/pkg/storage/utils/indexer/errors/errors.go b/pkg/storage/utils/indexer/errors/errors.go index 91cc3b78b9..d7bb66e339 100644 --- a/pkg/storage/utils/indexer/errors/errors.go +++ b/pkg/storage/utils/indexer/errors/errors.go @@ -34,11 +34,8 @@ func (e *AlreadyExistsErr) Error() string { return fmt.Sprintf("%s with %s=%s does already exist", e.TypeName, e.IndexBy.String(), e.Value) } -// IsAlreadyExistsErr checks whether an error is of type AlreadyExistsErr. -func IsAlreadyExistsErr(e error) bool { - _, ok := e.(*AlreadyExistsErr) - return ok -} +// IsAlreadyExists implements the IsAlreadyExists interface. +func (e *AlreadyExistsErr) IsAlreadyExists() {} // NotFoundErr implements the Error interface. type NotFoundErr struct { @@ -50,8 +47,5 @@ func (e *NotFoundErr) Error() string { return fmt.Sprintf("%s with %s=%s not found", e.TypeName, e.IndexBy.String(), e.Value) } -// IsNotFoundErr checks whether an error is of type IsNotFoundErr. -func IsNotFoundErr(e error) bool { - _, ok := e.(*NotFoundErr) - return ok -} +// IsNotFound implements the IsNotFound interface. +func (e *NotFoundErr) IsNotFound() {} diff --git a/pkg/storage/utils/indexer/index/unique_test.go b/pkg/storage/utils/indexer/index/unique_test.go index 1110274b5f..b08e089061 100644 --- a/pkg/storage/utils/indexer/index/unique_test.go +++ b/pkg/storage/utils/indexer/index/unique_test.go @@ -107,11 +107,6 @@ func TestUniqueIndexSearch(t *testing.T) { _ = os.RemoveAll(dataDir) } -func TestErrors(t *testing.T) { - assert.True(t, errors.IsAlreadyExistsErr(&errors.AlreadyExistsErr{})) - assert.True(t, errors.IsNotFoundErr(&errors.NotFoundErr{})) -} - func getUniqueIdxSut(t *testing.T, indexBy option.IndexBy, entityType interface{}) (index.Index, string) { dataPath, _ := WriteIndexTestData(Data, "ID", "") storage, err := metadata.NewDiskStorage(dataPath) diff --git a/pkg/storage/utils/indexer/indexer.go b/pkg/storage/utils/indexer/indexer.go index 9806aa5a4f..8b48b12960 100644 --- a/pkg/storage/utils/indexer/indexer.go +++ b/pkg/storage/utils/indexer/indexer.go @@ -30,7 +30,6 @@ import ( "github.com/iancoleman/strcase" "github.com/cs3org/reva/v2/pkg/errtypes" - errorspkg "github.com/cs3org/reva/v2/pkg/storage/utils/indexer/errors" "github.com/cs3org/reva/v2/pkg/storage/utils/indexer/index" "github.com/cs3org/reva/v2/pkg/storage/utils/indexer/option" "github.com/cs3org/reva/v2/pkg/storage/utils/metadata" @@ -149,7 +148,7 @@ func (i *Indexer) FindBy(t interface{}, findBy, val string) ([]string, error) { idxVal := val res, err := idx.Lookup(idxVal) if err != nil { - if _, ok := err.(errtypes.NotFound); ok { + if _, ok := err.(errtypes.IsNotFound); ok { continue } @@ -211,7 +210,7 @@ func (i *Indexer) FindByPartial(t interface{}, field string, pattern string) ([] for _, idx := range fields.IndicesByField[strcase.ToCamel(field)] { res, err := idx.Search(pattern) if err != nil { - if errorspkg.IsNotFoundErr(err) { + if _, ok := err.(errtypes.IsNotFound); ok { continue } From 65660b274dee9e5f4ddacaade506da715d0c521b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Fri, 11 Mar 2022 10:32:26 +0100 Subject: [PATCH 16/22] Use existing helper function for getting the current timestamp --- pkg/share/manager/cs3/cs3.go | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/pkg/share/manager/cs3/cs3.go b/pkg/share/manager/cs3/cs3.go index 009270ef4a..199e7d51cd 100644 --- a/pkg/share/manager/cs3/cs3.go +++ b/pkg/share/manager/cs3/cs3.go @@ -24,13 +24,11 @@ import ( "fmt" "net/url" "path" - "time" groupv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" - typespb "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" ctxpkg "github.com/cs3org/reva/v2/pkg/ctx" "github.com/cs3org/reva/v2/pkg/errtypes" "github.com/cs3org/reva/v2/pkg/share" @@ -152,11 +150,7 @@ func (m *Manager) Share(ctx context.Context, md *provider.ResourceInfo, g *colla return nil, err } user := ctxpkg.ContextMustGetUser(ctx) - now := time.Now().UnixNano() - ts := &typespb.Timestamp{ - Seconds: uint64(now / 1000000000), - Nanos: uint32(now % 1000000000), - } + ts := utils.TSNow() share := &collaboration.Share{ Id: &collaboration.ShareId{ From 5e93560e6d0ad2003595624b93eaf4980265426b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Fri, 11 Mar 2022 11:18:28 +0100 Subject: [PATCH 17/22] Return proper errors when deleting fails --- pkg/storage/utils/metadata/cs3.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/storage/utils/metadata/cs3.go b/pkg/storage/utils/metadata/cs3.go index ec702120fe..fcb41025ad 100644 --- a/pkg/storage/utils/metadata/cs3.go +++ b/pkg/storage/utils/metadata/cs3.go @@ -22,7 +22,6 @@ import ( "bytes" "context" "errors" - "fmt" "io/ioutil" "net/http" "os" @@ -248,7 +247,7 @@ func (cs3 *CS3) Delete(ctx context.Context, path string) error { return err } if res.Status.Code != rpc.Code_CODE_OK { - return fmt.Errorf("error deleting path: %v", path) + return errtypes.NewErrtypeFromStatus(res.Status) } return nil From 26404c70a4028920d47b75bacc9cefd9ba0389eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Fri, 11 Mar 2022 11:19:10 +0100 Subject: [PATCH 18/22] Still delete the share from the index when the share couldn't be found --- pkg/share/manager/cs3/cs3.go | 4 +++- pkg/share/manager/cs3/cs3_test.go | 17 ++++++++++++----- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/pkg/share/manager/cs3/cs3.go b/pkg/share/manager/cs3/cs3.go index 199e7d51cd..327b7edc21 100644 --- a/pkg/share/manager/cs3/cs3.go +++ b/pkg/share/manager/cs3/cs3.go @@ -219,7 +219,9 @@ func (m *Manager) Unshare(ctx context.Context, ref *collaboration.ShareReference fn := path.Join("shares", ref.GetId().OpaqueId) err = m.storage.Delete(ctx, fn) if err != nil { - return err + if _, ok := err.(errtypes.NotFound); !ok { + return err + } } return m.indexer.Delete(share) diff --git a/pkg/share/manager/cs3/cs3_test.go b/pkg/share/manager/cs3/cs3_test.go index 2b2aad3fa2..214dbe833a 100644 --- a/pkg/share/manager/cs3/cs3_test.go +++ b/pkg/share/manager/cs3/cs3_test.go @@ -60,7 +60,6 @@ var _ = Describe("Manager", func() { storage = &mocks.Storage{} storage.On("Init", mock.Anything, mock.Anything).Return(nil) storage.On("MakeDirIfNotExist", mock.Anything, mock.Anything).Return(nil) - storage.On("Delete", mock.Anything, mock.Anything).Return(nil) storage.On("SimpleUpload", mock.Anything, mock.Anything, mock.Anything).Return(nil) indexer = &mocks.Indexer{} indexer.On("AddIndex", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) @@ -217,17 +216,25 @@ var _ = Describe("Manager", func() { }) Describe("Unshare", func() { - JustBeforeEach(func() { + It("deletes the share", func() { + storage.On("Delete", mock.Anything, mock.Anything).Return(nil) err := m.Unshare(ctx, &collaboration.ShareReference{Spec: &collaboration.ShareReference_Id{Id: share.Id}}) Expect(err).ToNot(HaveOccurred()) - }) - - It("deletes the share", func() { expectedPath := path.Join("shares", share.Id.OpaqueId) storage.AssertCalled(GinkgoT(), "Delete", mock.Anything, expectedPath) }) It("removes the share from the index", func() { + storage.On("Delete", mock.Anything, mock.Anything).Return(nil) + err := m.Unshare(ctx, &collaboration.ShareReference{Spec: &collaboration.ShareReference_Id{Id: share.Id}}) + Expect(err).ToNot(HaveOccurred()) + indexer.AssertCalled(GinkgoT(), "Delete", mock.AnythingOfType("*collaborationv1beta1.Share")) + }) + + It("still tries to delete the share from the index when the share couldn't be found", func() { + storage.On("Delete", mock.Anything, mock.Anything).Return(errtypes.NotFound("")) + err := m.Unshare(ctx, &collaboration.ShareReference{Spec: &collaboration.ShareReference_Id{Id: share.Id}}) + Expect(err).ToNot(HaveOccurred()) indexer.AssertCalled(GinkgoT(), "Delete", mock.AnythingOfType("*collaborationv1beta1.Share")) }) }) From eb58b8c3cb71ffa15bb9cb548dddd8d3ad8ca125 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Fri, 11 Mar 2022 11:26:45 +0100 Subject: [PATCH 19/22] Incorporate review suggestions --- pkg/share/manager/cs3/cs3.go | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/pkg/share/manager/cs3/cs3.go b/pkg/share/manager/cs3/cs3.go index 327b7edc21..a86ce40dbb 100644 --- a/pkg/share/manager/cs3/cs3.go +++ b/pkg/share/manager/cs3/cs3.go @@ -227,8 +227,7 @@ func (m *Manager) Unshare(ctx context.Context, ref *collaboration.ShareReference return m.indexer.Delete(share) } -// ListShares returns the shares created by the user. If md is provided is not nil, -// it returns only shares attached to the given resource. +// ListShares returns the shares created by the user func (m *Manager) ListShares(ctx context.Context, filters []*collaboration.Filter) ([]*collaboration.Share, error) { if err := m.initialize(); err != nil { return nil, err @@ -327,14 +326,13 @@ func (m *Manager) ListReceivedShares(ctx context.Context, filters []*collaborati } metadata, err := m.downloadMetadata(ctx, share) if err != nil { - if _, ok := err.(errtypes.NotFound); ok { - // use default values if the grantee didn't configure anything yet - metadata = ReceivedShareMetadata{ - State: collaboration.ShareState_SHARE_STATE_PENDING, - } - } else { + if _, ok := err.(errtypes.NotFound); !ok { return nil, err } + // use default values if the grantee didn't configure anything yet + metadata = ReceivedShareMetadata{ + State: collaboration.ShareState_SHARE_STATE_PENDING, + } } result = append(result, &collaboration.ReceivedShare{ Share: share, @@ -358,14 +356,13 @@ func (m *Manager) GetReceivedShare(ctx context.Context, ref *collaboration.Share metadata, err := m.downloadMetadata(ctx, share) if err != nil { - if _, ok := err.(errtypes.NotFound); ok { - // use default values if the grantee didn't configure anything yet - metadata = ReceivedShareMetadata{ - State: collaboration.ShareState_SHARE_STATE_PENDING, - } - } else { + if _, ok := err.(errtypes.NotFound); !ok { return nil, err } + // use default values if the grantee didn't configure anything yet + metadata = ReceivedShareMetadata{ + State: collaboration.ShareState_SHARE_STATE_PENDING, + } } return &collaboration.ReceivedShare{ Share: share, From 8c5cc7f2ca093213446024648ed0e467db5a5539 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Fri, 11 Mar 2022 11:41:07 +0100 Subject: [PATCH 20/22] Make sure to initialize the manager atomically --- pkg/share/manager/cs3/cs3.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pkg/share/manager/cs3/cs3.go b/pkg/share/manager/cs3/cs3.go index a86ce40dbb..25339e09a6 100644 --- a/pkg/share/manager/cs3/cs3.go +++ b/pkg/share/manager/cs3/cs3.go @@ -24,6 +24,7 @@ import ( "fmt" "net/url" "path" + "sync" groupv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" @@ -61,6 +62,8 @@ type Indexer interface { // Manager implements a share manager using a cs3 storage backend type Manager struct { + sync.RWMutex + storage Storage indexer Indexer @@ -116,6 +119,13 @@ func (m *Manager) initialize() error { return nil } + m.Lock() + defer m.Unlock() + + if m.initialized { // check if initialization happened while grabbing the lock + return nil + } + err := m.storage.Init(context.Background(), "cs3-share-manager-metadata") if err != nil { return err From b7995c153ff3ac7a21ea4bdce379ede6cf784ac4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Mon, 14 Mar 2022 08:47:25 +0100 Subject: [PATCH 21/22] Spare some error checks --- pkg/share/manager/cs3/cs3.go | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/pkg/share/manager/cs3/cs3.go b/pkg/share/manager/cs3/cs3.go index 25339e09a6..ae670929b3 100644 --- a/pkg/share/manager/cs3/cs3.go +++ b/pkg/share/manager/cs3/cs3.go @@ -193,11 +193,8 @@ func (m *Manager) Share(ctx context.Context, md *provider.ResourceInfo, g *colla } _, err = m.indexer.Add(share) - if err != nil { - return nil, err - } - return share, nil + return share, err } // GetShare gets the information for a share by the given ref. @@ -283,11 +280,8 @@ func (m *Manager) UpdateShare(ctx context.Context, ref *collaboration.ShareRefer fn := path.Join("shares", share.Id.OpaqueId) err = m.storage.SimpleUpload(ctx, fn, data) - if err != nil { - return nil, err - } - return share, nil + return share, err } // ListReceivedShares returns the list of shares the user has access to. @@ -424,11 +418,8 @@ func (m *Manager) UpdateReceivedShare(ctx context.Context, rshare *collaboration return nil, err } err = m.storage.SimpleUpload(ctx, fn, data) - if err != nil { - return nil, err - } - return rs, nil + return rs, err } func (m *Manager) downloadMetadata(ctx context.Context, share *collaboration.Share) (ReceivedShareMetadata, error) { From de69ac210f7620feba60aaf6f2a1a5b06468bb3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Tue, 15 Mar 2022 08:35:09 +0100 Subject: [PATCH 22/22] Make use of `shareFilename` everywhere --- pkg/share/manager/cs3/cs3.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/pkg/share/manager/cs3/cs3.go b/pkg/share/manager/cs3/cs3.go index ae670929b3..9a541d09c2 100644 --- a/pkg/share/manager/cs3/cs3.go +++ b/pkg/share/manager/cs3/cs3.go @@ -180,8 +180,7 @@ func (m *Manager) Share(ctx context.Context, md *provider.ResourceInfo, g *colla return nil, err } - fn := path.Join("shares", share.Id.OpaqueId) - err = m.storage.SimpleUpload(ctx, fn, data) + err = m.storage.SimpleUpload(ctx, shareFilename(share.Id.OpaqueId), data) if err != nil { return nil, err } @@ -223,8 +222,7 @@ func (m *Manager) Unshare(ctx context.Context, ref *collaboration.ShareReference return err } - fn := path.Join("shares", ref.GetId().OpaqueId) - err = m.storage.Delete(ctx, fn) + err = m.storage.Delete(ctx, shareFilename(ref.GetId().OpaqueId)) if err != nil { if _, ok := err.(errtypes.NotFound); !ok { return err @@ -278,8 +276,7 @@ func (m *Manager) UpdateShare(ctx context.Context, ref *collaboration.ShareRefer return nil, err } - fn := path.Join("shares", share.Id.OpaqueId) - err = m.storage.SimpleUpload(ctx, fn, data) + err = m.storage.SimpleUpload(ctx, shareFilename(share.Id.OpaqueId), data) return share, err }