fix(sharesstorageprovider): Merge shares of the same resourceid

When listing the sharejail root, merge shares that target the same resource
into a single resource. In order to avoid the resourceId changing randomly the
id will be composed from the oldest accepted share that exist for the resource.

Ideally we'd compose the resourceId based on the id of the shared resource, but
that is currently not possible in a backwards compatible way. Some clients seem
to rely on the fact that the resource ids in the sharejail contain valid shareids.

Fixes: owncloud/ocis#8080

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

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

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"))
+			})
+		})
+
 	})
 })