diff --git a/changelog/unreleased/fix-duplicate-sharejail-items.md b/changelog/unreleased/fix-duplicate-sharejail-items.md new file mode 100644 index 00000000000..7f20e5e1fd2 --- /dev/null +++ b/changelog/unreleased/fix-duplicate-sharejail-items.md @@ -0,0 +1,7 @@ +Bugfix: Fix duplicated items in the sharejail root + +We fixed a bug, that caused duplicate items to listed in the sharejail, when +a user received multiple shares for the same resource. + +https://github.com/cs3org/reva/pull/4517 +https://github.com/owncloud/ocis/issues/8080 diff --git a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go index 49ffb87df33..8d5316c6d93 100644 --- a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go +++ b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go @@ -810,12 +810,28 @@ func (s *service) ListContainer(ctx context.Context, req *provider.ListContainer return nil, errors.Wrap(err, "sharesstorageprovider: error calling ListReceivedSharesRequest") } - infos := []*provider.ResourceInfo{} - for _, share := range receivedShares { - if share.GetState() != collaboration.ShareState_SHARE_STATE_ACCEPTED { + // Create map of shares that contains only the oldest share per shared resource. This is to avoid + // returning multiple resourceInfos for the same resource. But still be able to maintain a + // "somewhat" stable resourceID + oldestReceivedSharesByResourceID := make(map[string]*collaboration.ReceivedShare, len(receivedShares)) + for _, receivedShare := range receivedShares { + if receivedShare.GetState() != collaboration.ShareState_SHARE_STATE_ACCEPTED { continue } + rIDStr := storagespace.FormatResourceID(*receivedShare.GetShare().GetResourceId()) + if oldest, ok := oldestReceivedSharesByResourceID[rIDStr]; ok { + // replace if older than current oldest + if utils.TSToTime(receivedShare.GetShare().GetCtime()).Before(utils.TSToTime(oldest.GetShare().GetCtime())) { + oldestReceivedSharesByResourceID[rIDStr] = receivedShare + } + } else { + oldestReceivedSharesByResourceID[rIDStr] = receivedShare + } + } + // now compose the resourceInfos for the unified list of shares + infos := []*provider.ResourceInfo{} + for _, share := range oldestReceivedSharesByResourceID { info := shareMd[share.GetShare().GetId().GetOpaqueId()] if info == nil { appctx.GetLogger(ctx).Debug(). @@ -828,7 +844,7 @@ func (s *service) ListContainer(ctx context.Context, req *provider.ListContainer info.Id = &provider.ResourceId{ StorageId: utils.ShareStorageProviderID, SpaceId: utils.ShareStorageSpaceID, - OpaqueId: share.Share.Id.OpaqueId, + OpaqueId: share.GetShare().GetId().GetOpaqueId(), } info.Path = filepath.Base(share.MountPoint.Path) info.Name = info.Path diff --git a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go index 21508b41b38..9ff1f8592a4 100644 --- a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go +++ b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go @@ -261,6 +261,26 @@ var _ = Describe("Sharesstorageprovider", func() { }, } default: + if req.Ref.ResourceId.OpaqueId == "shareddir-merged" { + permissionSet := &sprovider.ResourcePermissions{ + Stat: true, + ListContainer: true, + } + return &sprovider.StatResponse{ + Status: status.NewOK(context.Background()), + Info: &sprovider.ResourceInfo{ + Type: sprovider.ResourceType_RESOURCE_TYPE_CONTAINER, + Path: "share1-shareddir", + Id: &sprovider.ResourceId{ + StorageId: "share1-storageid", + SpaceId: "share1-storageid", + OpaqueId: req.Ref.ResourceId.OpaqueId, + }, + PermissionSet: permissionSet, + Size: 100, + }, + } + } return &sprovider.StatResponse{ Status: status.NewNotFound(context.Background(), "not found"), } @@ -942,6 +962,7 @@ var _ = Describe("Sharesstorageprovider", func() { Stat: true, }, } + BaseShare.Share.Ctime = utils.TSNow() BaseShare.MountPoint = &sprovider.Reference{Path: "share1-shareddir"} BaseShareTwo.Share.Id.OpaqueId = "multishare2" BaseShareTwo.Share.ResourceId = BaseShare.Share.ResourceId @@ -951,6 +972,7 @@ var _ = Describe("Sharesstorageprovider", func() { }, } BaseShareTwo.MountPoint = BaseShare.MountPoint + BaseShareTwo.Share.Ctime = utils.TSNow() sharingCollaborationClient.On("ListReceivedShares", mock.Anything, mock.Anything).Return(&collaboration.ListReceivedSharesResponse{ Status: status.NewOK(context.Background()), @@ -990,5 +1012,21 @@ var _ = Describe("Sharesstorageprovider", func() { Expect(res.Info.PermissionSet.ListContainer).To(BeTrue()) }) }) + Describe("ListContainer", func() { + It("Returns just one item", func() { + req := BaseListContainerRequest + req.Ref.ResourceId.OpaqueId = utils.ShareStorageSpaceID + req.Ref.Path = "" + res, err := s.ListContainer(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res).ToNot(BeNil()) + Expect(res.Status.Code).To(Equal(rpc.Code_CODE_OK)) + Expect(len(res.Infos)).To(Equal(1)) + + entry := res.Infos[0] + Expect(entry.Path).To(Equal("share1-shareddir")) + }) + }) + }) })