graph/sharedWithMe: Fix 'parentReference' on 'driveItem'

The outer parentreference should refer to the drive containing the mountpoint.
In our case this is the storagespaceid of the virtual share jail.

Also 'CreatedBy' should be the same as on the wrapped remote item. Not the share creator.

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

@@ -84,17 +84,6 @@ func (g Graph) listSharedWithMe(ctx context.Context) ([]libregraph.DriveItem, er
 				return err
 			}
 
-			parentReference := libregraph.NewItemReference()
-			{
-				if spaceType := shareStat.GetInfo().GetSpace().GetSpaceType(); spaceType != "" {
-					parentReference.SetDriveType(spaceType)
-				}
-
-				if root := shareStat.GetInfo().GetSpace().GetRoot(); root != nil {
-					parentReference.SetDriveId(storagespace.FormatResourceID(*root))
-				}
-			}
-
 			shared := libregraph.NewShared()
 			{
 				if cTime := receivedShare.GetShare().GetCtime(); cTime != nil {
@@ -123,6 +112,19 @@ func (g Graph) listSharedWithMe(ctx context.Context) ([]libregraph.DriveItem, er
 				if size := shareStat.GetInfo().GetSize(); size != 0 {
 					remoteItem.SetSize(int64(size))
 				}
+
+				parentReference := libregraph.NewItemReference()
+				if spaceType := shareStat.GetInfo().GetSpace().GetSpaceType(); spaceType != "" {
+					parentReference.SetDriveType(spaceType)
+				}
+
+				if root := shareStat.GetInfo().GetSpace().GetRoot(); root != nil {
+					parentReference.SetDriveId(storagespace.FormatResourceID(*root))
+				}
+				if !reflect.ValueOf(*parentReference).IsZero() {
+					remoteItem.ParentReference = parentReference
+				}
+
 			}
 
 			driveItem := libregraph.NewDriveItem()
@@ -144,6 +146,18 @@ func (g Graph) listSharedWithMe(ctx context.Context) ([]libregraph.DriveItem, er
 				if etag := shareStat.GetInfo().GetEtag(); etag != "" {
 					driveItem.SetETag(etag)
 				}
+
+				// parentReference of the out driveItem should be the drive containing the mountpoint
+				// i.e. the share jail
+				driveItem.ParentReference = libregraph.NewItemReference()
+				driveItem.ParentReference.SetDriveType("virtual")
+				driveItem.ParentReference.SetDriveId(storagespace.FormatStorageID(utils.ShareStorageProviderID, utils.ShareStorageSpaceID))
+				driveItem.ParentReference.SetId(storagespace.FormatResourceID(storageprovider.ResourceId{
+					StorageId: utils.ShareStorageProviderID,
+					OpaqueId:  utils.ShareStorageSpaceID,
+					SpaceId:   utils.ShareStorageSpaceID,
+				}))
+
 			case collaboration.ShareState_SHARE_STATE_PENDING:
 				fallthrough
 			case collaboration.ShareState_SHARE_STATE_REJECTED:
@@ -183,6 +197,7 @@ func (g Graph) listSharedWithMe(ctx context.Context) ([]libregraph.DriveItem, er
 					}
 
 					remoteItem.SetCreatedBy(identitySet)
+					driveItem.SetCreatedBy(identitySet)
 				}
 
 				if userID := receivedShare.GetShare().GetOwner(); userID != nil {
@@ -216,7 +231,6 @@ func (g Graph) listSharedWithMe(ctx context.Context) ([]libregraph.DriveItem, er
 						},
 					}
 
-					driveItem.SetCreatedBy(identitySet)
 					shared.SetSharedBy(identitySet)
 
 				}
@@ -248,11 +262,6 @@ func (g Graph) listSharedWithMe(ctx context.Context) ([]libregraph.DriveItem, er
 					remoteItem.Permissions = permissions
 				}
 
-				if !reflect.ValueOf(*parentReference).IsZero() {
-					remoteItem.ParentReference = parentReference
-					driveItem.ParentReference = parentReference
-				}
-
 				if !reflect.ValueOf(*remoteItem).IsZero() {
 					driveItem.RemoteItem = remoteItem
 				}

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

@@ -72,9 +72,11 @@ var _ = Describe("SharedWithMe", func() {
 
 	Describe("ListSharedWithMe", func() {
 		var (
-			listReceivedSharesResponse *collaborationv1beta1.ListReceivedSharesResponse
+			listReceivedSharesResponse     *collaborationv1beta1.ListReceivedSharesResponse
-			statResponse               *providerv1beta1.StatResponse
+			statResponse                   *providerv1beta1.StatResponse
-			getUserResponse            *userv1beta1.GetUserResponse
+			getUserResponseDefault         *userv1beta1.GetUserResponse
+			getUserResponseResourceCreator *userv1beta1.GetUserResponse
+			getUserResponseShareCreator    *userv1beta1.GetUserResponse
 		)
 
 		toResourceID := func(in string) *providerv1beta1.ResourceId {
@@ -86,7 +88,7 @@ var _ = Describe("SharedWithMe", func() {
 
 		BeforeEach(func() {
 
-			getUserResponse = &userv1beta1.GetUserResponse{
+			getUserResponseDefault = &userv1beta1.GetUserResponse{
 				Status: status.NewOK(ctx),
 				User: &userv1beta1.User{
 					Id: &userv1beta1.UserId{
@@ -95,13 +97,51 @@ var _ = Describe("SharedWithMe", func() {
 					DisplayName: "John Romero",
 				},
 			}
+			getUserResponseResourceCreator = &userv1beta1.GetUserResponse{
+				Status: status.NewOK(ctx),
+				User: &userv1beta1.User{
+					Id: &userv1beta1.UserId{
+						OpaqueId: "resource-creator-id",
+					},
+					DisplayName: "Resource Creator",
+				},
+			}
+			getUserResponseShareCreator = &userv1beta1.GetUserResponse{
+				Status: status.NewOK(ctx),
+				User: &userv1beta1.User{
+					Id: &userv1beta1.UserId{
+						OpaqueId: "share-creator-id",
+					},
+					DisplayName: "Share Creator",
+				},
+			}
 
-			gatewayClient.On("GetUser", mock.Anything, mock.Anything).Return(getUserResponse, nil)
+			gatewayClient.On("GetUser", mock.Anything, mock.MatchedBy(
+				func(req *userv1beta1.GetUserRequest) bool {
+					return req.UserId.OpaqueId == "resource-creator-id"
+				})).
+				Return(getUserResponseResourceCreator, nil)
+			gatewayClient.On("GetUser", mock.Anything, mock.MatchedBy(
+				func(req *userv1beta1.GetUserRequest) bool {
+					return req.UserId.OpaqueId == "share-creator-id"
+				})).
+				Return(getUserResponseShareCreator, nil)
+			gatewayClient.On("GetUser", mock.Anything, mock.Anything).Return(getUserResponseDefault, nil)
 
 			listReceivedSharesResponse = &collaborationv1beta1.ListReceivedSharesResponse{
 				Status: status.NewOK(ctx),
 				Shares: []*collaborationv1beta1.ReceivedShare{
-					{Share: &collaborationv1beta1.Share{ResourceId: toResourceID("1$2!3")}},
+					{
+						Share: &collaborationv1beta1.Share{ResourceId: toResourceID("1$2!3")},
+						MountPoint: &providerv1beta1.Reference{
+							ResourceId: &providerv1beta1.ResourceId{
+								StorageId: utils.ShareStorageProviderID,
+								SpaceId:   utils.ShareStorageSpaceID,
+							},
+							Path: "some folder",
+						},
+						State: collaborationv1beta1.ShareState_SHARE_STATE_ACCEPTED,
+					},
 				},
 			}
 
@@ -161,24 +201,23 @@ var _ = Describe("SharedWithMe", func() {
 			Expect(jsonData.Get("#").Num).To(Equal(float64(2)))
 		})
 
-		// TODO clarify which of the various properties should actually be set on the driveItem and what
+		It("populates the driveItem properties", func() {
-		//      semantics those properties have
-		// createdDateTime: is this the share creation time or the creation time of the shared item (there is not ctime in the stat response)?
-		// lastModifiedDateTime: see above
-		// owner: Should this be the owner of the shared item, the creator of the share, or the owner of the sharejail?
-		// etag: should this just equal the etag of the inner remote item?
-		PIt("populates the driveItem properties", func() {
 
 			share := listReceivedSharesResponse.Shares[0].Share
 			share.Id = &collaborationv1beta1.ShareId{OpaqueId: "1:2:3"}
 			share.Ctime = &typesv1beta1.Timestamp{Seconds: 4001}
-			share.Mtime = &typesv1beta1.Timestamp{Seconds: 40002}
+			share.Mtime = &typesv1beta1.Timestamp{Seconds: 4002}
-
+			share.Creator = &userv1beta1.UserId{
-			etag := "5ffb8e4bec7026050af7fde9482b289a"
+				OpaqueId: "share-creator-id",
+			}
 
 			resourceInfo := statResponse.Info
 			resourceInfo.Name = "some folder"
-			resourceInfo.Etag = "\"" + etag + "\""
+			resourceInfo.Etag = "\"5ffb8e4bec7026050af7fde9482b289a\""
+			resourceInfo.Owner = &userv1beta1.UserId{
+				OpaqueId: "resource-creator-id",
+			}
+			resourceInfo.Mtime = &typesv1beta1.Timestamp{Seconds: 40000}
 
 			svc.ListSharedWithMe(
 				tape,
@@ -187,14 +226,21 @@ var _ = Describe("SharedWithMe", func() {
 
 			jsonData := gjson.Get(tape.Body.String(), "value.0")
 
-			Expect(jsonData.Get("createdDateTime").String()).To(Equal(utils.TSToTime(share.Ctime).Format(time.RFC3339Nano)))
+			Expect(jsonData.Get("eTag").String()).To(Equal(resourceInfo.Etag))
-			Expect(jsonData.Get("eTag").String()).To(Equal(etag))
+			Expect(jsonData.Get("id").String()).To(Equal(storagespace.FormatResourceID(
-			Expect(jsonData.Get("id").String()).To(Equal(share.Id.OpaqueId))
+				providerv1beta1.ResourceId{
-			Expect(jsonData.Get("lastModifiedDateTime").String()).To(Equal(utils.TSToTime(share.Mtime).Format(time.RFC3339Nano)))
+					StorageId: utils.ShareStorageProviderID,
+					SpaceId:   utils.ShareStorageSpaceID,
+					OpaqueId:  share.Id.OpaqueId,
+				},
+			)))
+			Expect(jsonData.Get("lastModifiedDateTime").String()).To(Equal(utils.TSToTime(resourceInfo.Mtime).Format(time.RFC3339Nano)))
+			Expect(jsonData.Get("createdBy.user.id").String()).To(Equal(resourceInfo.Owner.OpaqueId))
 			Expect(jsonData.Get("name").String()).To(Equal(resourceInfo.Name))
+
 		})
 
-		PIt("populates the driveItem parentReference properties", func() {
+		It("populates the driveItem parentReference properties", func() {
 			share := listReceivedSharesResponse.Shares[0].Share
 			share.Id = &collaborationv1beta1.ShareId{OpaqueId: "1:2:3"}
 
@@ -205,7 +251,7 @@ var _ = Describe("SharedWithMe", func() {
 
 			jsonData := gjson.Get(tape.Body.String(), "value.0.parentReference")
 
-			Expect(jsonData.Get("driveId").String()).To(Equal(share.Id.OpaqueId))
+			Expect(jsonData.Get("driveId").String()).To(Equal(storagespace.FormatStorageID(utils.ShareStorageProviderID, utils.ShareStorageSpaceID)))
 		})
 
 		It("populates the driveItem remoteItem properties", func() {
@@ -235,7 +281,7 @@ var _ = Describe("SharedWithMe", func() {
 		})
 
 		It("populates the driveItem.remoteItem.createdBy properties", func() {
-			driveOwner := getUserResponse.User
+			driveOwner := getUserResponseDefault.User
 
 			resourceInfo := statResponse.Info
 			resourceInfo.Owner = driveOwner.Id
@@ -295,7 +341,7 @@ var _ = Describe("SharedWithMe", func() {
 		})
 
 		It("populates the driveItem.remoteItem.shared.owner properties", func() {
-			shareOwner := getUserResponse.User
+			shareOwner := getUserResponseDefault.User
 
 			share := listReceivedSharesResponse.Shares[0].Share
 			share.Owner = shareOwner.Id
@@ -312,7 +358,7 @@ var _ = Describe("SharedWithMe", func() {
 		})
 
 		It("populates the driveItem.remoteItem.shared.sharedBy properties", func() {
-			shareCreator := getUserResponse.User
+			shareCreator := getUserResponseDefault.User
 
 			share := listReceivedSharesResponse.Shares[0].Share
 			share.Creator = shareCreator.Id