From 05c17fbfac79a0b57d0b419306c175a683c9a02b Mon Sep 17 00:00:00 2001
From: Laurent Cozic <laurent@cozic.net>
Date: Thu, 9 Feb 2023 15:56:44 +0000
Subject: [PATCH] Server: Fixed sharing issue for changes that are associated
 with deleted items

---
 packages/server/src/models/ShareModel.ts      | 42 ++++++++++---------
 .../src/routes/api/shares.folder.test.ts      | 29 +++++++++++++
 2 files changed, 52 insertions(+), 19 deletions(-)

diff --git a/packages/server/src/models/ShareModel.ts b/packages/server/src/models/ShareModel.ts
index 3799d6a98..9459a0a41 100644
--- a/packages/server/src/models/ShareModel.ts
+++ b/packages/server/src/models/ShareModel.ts
@@ -290,29 +290,33 @@ export default class ShareModel extends BaseModel<Share> {
 					for (const change of changes) {
 						const item = items.find(i => i.id === change.item_id);
 
-						// When a folder is unshared, the share object is
-						// deleted, then all items that were shared get their
-						// 'share_id' property set to an empty string. This is
-						// all done client side.
-						//
-						// However it means that if a share object is deleted
-						// but the items are not synced, we'll find items that
-						// are associated with a share that no longer exists.
-						// This is fine, but we need to handle it properly
-						// below, otherwise the share update process will fail.
+						// Item associated with the change may have been
+						// deleted, so take this into account.
+						if (item) {
+							// When a folder is unshared, the share object is
+							// deleted, then all items that were shared get their
+							// 'share_id' property set to an empty string. This is
+							// all done client side.
+							//
+							// However it means that if a share object is deleted
+							// but the items are not synced, we'll find items that
+							// are associated with a share that no longer exists.
+							// This is fine, but we need to handle it properly
+							// below, otherwise the share update process will fail.
 
-						const itemShare = shares.find(s => s.id === item.jop_share_id);
+							const itemShare = shares.find(s => s.id === item.jop_share_id);
 
-						if (change.type === ChangeType.Create) {
-							if (!itemShare) {
-								logger.warn(`Found an item (${item.id}) associated with a share that no longer exists (${item.jop_share_id}) - skipping it`);
-							} else {
-								await handleCreated(change, item, itemShare);
+							if (change.type === ChangeType.Create) {
+								if (!itemShare) {
+									logger.warn(`Found an item (${item.id}) associated with a share that no longer exists (${item.jop_share_id}) - skipping it`);
+								} else {
+									await handleCreated(change, item, itemShare);
+								}
 							}
-						}
 
-						if (change.type === ChangeType.Update) {
-							await handleUpdated(change, item, itemShare);
+							if (change.type === ChangeType.Update) {
+								await handleUpdated(change, item, itemShare);
+							}
 						}
 
 						// We don't need to handle ChangeType.Delete because when an
diff --git a/packages/server/src/routes/api/shares.folder.test.ts b/packages/server/src/routes/api/shares.folder.test.ts
index 8623ef6a0..70d790e64 100644
--- a/packages/server/src/routes/api/shares.folder.test.ts
+++ b/packages/server/src/routes/api/shares.folder.test.ts
@@ -385,6 +385,35 @@ describe('shares.folder', function() {
 		await expectNotThrow(async () => await models().share().updateSharedItems3());
 	});
 
+	test('should not throw an error if a change is associated with an item that no longer exists', async function() {
+		const { session: session1 } = await createUserAndSession(1);
+		const { session: session2 } = await createUserAndSession(2);
+
+		const { share: share1 } = await shareFolderWithUser(session1.id, session2.id, '000000000000000000000000000000F1', {
+			'000000000000000000000000000000F1': {},
+		});
+
+		const { share: share2 } = await shareFolderWithUser(session1.id, session2.id, '000000000000000000000000000000F2', {
+			'000000000000000000000000000000F2': {},
+		});
+
+		const item1 = await createNote(session1.id, {
+			id: '00000000000000000000000000000007',
+			share_id: share1.id,
+		});
+
+		await createNote(session1.id, {
+			id: '00000000000000000000000000000008',
+			share_id: share2.id,
+		});
+
+		await models().item().delete(item1.id);
+
+		await models().share().updateSharedItems3();
+
+		await expectNotThrow(async () => await models().share().updateSharedItems3());
+	});
+
 	test('should unshare a deleted item', async function() {
 		const { user: user1, session: session1 } = await createUserAndSession(1);
 		const { user: user2, session: session2 } = await createUserAndSession(2);