From 25b472e3dbe5fc95f83b5e976a7f1123c41acea4 Mon Sep 17 00:00:00 2001
From: Tomasz Palys <tomasz.palys@swmansion.com>
Date: Tue, 17 Sep 2024 17:36:40 +0200
Subject: [PATCH] [lib] Update the conditions we check before processing the
 operations

Summary:
Queue operations when their dependencies aren't satisfied

https://linear.app/comm/issue/ENG-9189/introduce-additional-operation-queues

Depends on D13365

Test Plan:
Tested the whole stack at once:
1. Created an operation with a reaction that was received before an operation creating the message
2. Created an operation with an edit that was received before an operation creating the message
3. Created an operation with an entry edit that was received before an operation creating the entry
4. Created an operation with thread subscription change that was received before the user became a member
In each of the cases verified that the result was correct and that the operation was removed from the queue.

Reviewers: kamil, will

Reviewed By: kamil

Subscribers: ashoat

Differential Revision: https://phab.comm.dev/D13366
---
 lib/shared/dm-ops/add-members-spec.js         |  4 ++--
 .../add-viewer-to-thread-members-spec.js      |  4 ++--
 .../dm-ops/change-thread-read-status-spec.js  |  4 ++--
 .../dm-ops/change-thread-settings-spec.js     |  4 ++--
 .../dm-ops/change-thread-subscription.js      |  9 ++++---
 lib/shared/dm-ops/create-entry-spec.js        |  4 ++--
 lib/shared/dm-ops/create-sidebar-spec.js      | 18 +++++++++++++-
 lib/shared/dm-ops/create-thread-spec.js       |  2 +-
 lib/shared/dm-ops/delete-entry-spec.js        | 20 +++++++++-------
 lib/shared/dm-ops/dm-op-spec.js               | 24 ++++++++++---------
 lib/shared/dm-ops/edit-entry-spec.js          | 20 +++++++++-------
 lib/shared/dm-ops/join-thread-spec.js         |  4 ++--
 lib/shared/dm-ops/leave-thread-spec.js        |  4 ++--
 lib/shared/dm-ops/process-dm-ops.js           |  2 +-
 lib/shared/dm-ops/remove-members-spec.js      |  4 ++--
 lib/shared/dm-ops/send-edit-message-spec.js   | 21 +++++++++-------
 .../dm-ops/send-multimedia-message-spec.js    |  4 ++--
 .../dm-ops/send-reaction-message-spec.js      | 21 +++++++++-------
 lib/shared/dm-ops/send-text-message-spec.js   |  4 ++--
 19 files changed, 104 insertions(+), 73 deletions(-)

diff --git a/lib/shared/dm-ops/add-members-spec.js b/lib/shared/dm-ops/add-members-spec.js
index 73399fdebc..100f7ec6d4 100644
--- a/lib/shared/dm-ops/add-members-spec.js
+++ b/lib/shared/dm-ops/add-members-spec.js
@@ -122,11 +122,11 @@ const addMembersSpec: DMOperationSpec<DMAddMembersOperation> = Object.freeze({
       updateInfos,
     };
   },
-  canBeProcessed(
+  canBeProcessed: async (
     dmOperation: DMAddMembersOperation,
     viewerID: string,
     utilities: ProcessDMOperationUtilities,
-  ) {
+  ) => {
     if (utilities.threadInfos[dmOperation.threadID]) {
       return { isProcessingPossible: true };
     }
diff --git a/lib/shared/dm-ops/add-viewer-to-thread-members-spec.js b/lib/shared/dm-ops/add-viewer-to-thread-members-spec.js
index b1d1461e4e..938bea0a98 100644
--- a/lib/shared/dm-ops/add-viewer-to-thread-members-spec.js
+++ b/lib/shared/dm-ops/add-viewer-to-thread-members-spec.js
@@ -120,10 +120,10 @@ const addViewerToThreadMembersSpec: DMOperationSpec<DMAddViewerToThreadMembersOp
       ];
       return { rawMessageInfos, updateInfos };
     },
-    canBeProcessed(
+    canBeProcessed: async (
       dmOperation: DMAddViewerToThreadMembersOperation,
       viewerID: string,
-    ) {
+    ) => {
       // We expect the viewer to be in the added users when the DM op
       // is processed. An exception is for ops generated
       // by InitialStateSharingHandler, which won't contain a messageID
diff --git a/lib/shared/dm-ops/change-thread-read-status-spec.js b/lib/shared/dm-ops/change-thread-read-status-spec.js
index 5c1b0df0fa..762637213e 100644
--- a/lib/shared/dm-ops/change-thread-read-status-spec.js
+++ b/lib/shared/dm-ops/change-thread-read-status-spec.js
@@ -63,11 +63,11 @@ const changeThreadReadStatusSpec: DMOperationSpec<DMChangeThreadReadStatusOperat
         updateInfos,
       };
     },
-    canBeProcessed(
+    canBeProcessed: async (
       dmOperation: DMChangeThreadReadStatusOperation,
       viewerID: string,
       utilities: ProcessDMOperationUtilities,
-    ) {
+    ) => {
       const { creatorID, threadID } = dmOperation;
       if (viewerID !== creatorID) {
         return { isProcessingPossible: false, reason: { type: 'invalid' } };
diff --git a/lib/shared/dm-ops/change-thread-settings-spec.js b/lib/shared/dm-ops/change-thread-settings-spec.js
index 6d27a91791..1f26d5b0a0 100644
--- a/lib/shared/dm-ops/change-thread-settings-spec.js
+++ b/lib/shared/dm-ops/change-thread-settings-spec.js
@@ -161,11 +161,11 @@ const changeThreadSettingsSpec: DMOperationSpec<DMChangeThreadSettingsOperation>
         updateInfos,
       };
     },
-    canBeProcessed(
+    canBeProcessed: async (
       dmOperation: DMChangeThreadSettingsOperation,
       viewerID: string,
       utilities: ProcessDMOperationUtilities,
-    ) {
+    ) => {
       if (utilities.threadInfos[dmOperation.threadID]) {
         return { isProcessingPossible: true };
       }
diff --git a/lib/shared/dm-ops/change-thread-subscription.js b/lib/shared/dm-ops/change-thread-subscription.js
index 2897956b0e..4897912f14 100644
--- a/lib/shared/dm-ops/change-thread-subscription.js
+++ b/lib/shared/dm-ops/change-thread-subscription.js
@@ -77,11 +77,11 @@ const changeThreadSubscriptionSpec: DMOperationSpec<DMChangeThreadSubscriptionOp
 
       return { updateInfos, rawMessageInfos: [] };
     },
-    canBeProcessed(
+    canBeProcessed: async (
       dmOperation: DMChangeThreadSubscriptionOperation,
       viewerID: string,
       utilities: ProcessDMOperationUtilities,
-    ) {
+    ) => {
       const { threadID, creatorID } = dmOperation;
       if (!utilities.threadInfos[threadID]) {
         return {
@@ -95,7 +95,10 @@ const changeThreadSubscriptionSpec: DMOperationSpec<DMChangeThreadSubscriptionOp
           memberInfo => memberInfo.id === creatorID,
         )
       ) {
-        return { isProcessingPossible: false, reason: { type: 'invalid' } };
+        return {
+          isProcessingPossible: false,
+          reason: { type: 'missing_membership', threadID, userID: creatorID },
+        };
       }
 
       return { isProcessingPossible: true };
diff --git a/lib/shared/dm-ops/create-entry-spec.js b/lib/shared/dm-ops/create-entry-spec.js
index f63e5b840c..27f46d5e01 100644
--- a/lib/shared/dm-ops/create-entry-spec.js
+++ b/lib/shared/dm-ops/create-entry-spec.js
@@ -68,11 +68,11 @@ const createEntrySpec: DMOperationSpec<DMCreateEntryOperation> = Object.freeze({
       updateInfos: [entryUpdateInfo],
     };
   },
-  canBeProcessed(
+  canBeProcessed: async (
     dmOperation: DMCreateEntryOperation,
     viewerID: string,
     utilities: ProcessDMOperationUtilities,
-  ) {
+  ) => {
     if (utilities.threadInfos[dmOperation.threadID]) {
       return { isProcessingPossible: true };
     }
diff --git a/lib/shared/dm-ops/create-sidebar-spec.js b/lib/shared/dm-ops/create-sidebar-spec.js
index 01985e6622..05ce0e7503 100644
--- a/lib/shared/dm-ops/create-sidebar-spec.js
+++ b/lib/shared/dm-ops/create-sidebar-spec.js
@@ -166,7 +166,23 @@ const createSidebarSpec: DMOperationSpec<DMCreateSidebarOperation> =
         updateInfos: [threadJoinUpdateInfo],
       };
     },
-    canBeProcessed() {
+    canBeProcessed: async (
+      dmOperation: DMCreateSidebarOperation,
+      viewerID: string,
+      utilities: ProcessDMOperationUtilities,
+    ) => {
+      const sourceMessage = await utilities.fetchMessage(
+        dmOperation.sourceMessageID,
+      );
+      if (!sourceMessage) {
+        return {
+          isProcessingPossible: false,
+          reason: {
+            type: 'missing_message',
+            messageID: dmOperation.sourceMessageID,
+          },
+        };
+      }
       return { isProcessingPossible: true };
     },
     supportsAutoRetry: true,
diff --git a/lib/shared/dm-ops/create-thread-spec.js b/lib/shared/dm-ops/create-thread-spec.js
index 2806a9ddc1..7646f46f35 100644
--- a/lib/shared/dm-ops/create-thread-spec.js
+++ b/lib/shared/dm-ops/create-thread-spec.js
@@ -206,7 +206,7 @@ const createThreadSpec: DMOperationSpec<DMCreateThreadOperation> =
         updateInfos: [threadJoinUpdateInfo],
       };
     },
-    canBeProcessed() {
+    canBeProcessed: async () => {
       return { isProcessingPossible: true };
     },
     supportsAutoRetry: true,
diff --git a/lib/shared/dm-ops/delete-entry-spec.js b/lib/shared/dm-ops/delete-entry-spec.js
index 90048958c4..39ebbbd273 100644
--- a/lib/shared/dm-ops/delete-entry-spec.js
+++ b/lib/shared/dm-ops/delete-entry-spec.js
@@ -92,20 +92,22 @@ const deleteEntrySpec: DMOperationSpec<DMDeleteEntryOperation> = Object.freeze({
       updateInfos: [entryUpdateInfo],
     };
   },
-  canBeProcessed(
+  canBeProcessed: async (
     dmOperation: DMDeleteEntryOperation,
     viewerID: string,
     utilities: ProcessDMOperationUtilities,
-  ) {
-    if (utilities.threadInfos[dmOperation.threadID]) {
-      return { isProcessingPossible: true };
+  ) => {
+    if (!utilities.entryInfos[dmOperation.entryID]) {
+      return {
+        isProcessingPossible: false,
+        reason: {
+          type: 'missing_entry',
+          entryID: dmOperation.entryID,
+        },
+      };
     }
     return {
-      isProcessingPossible: false,
-      reason: {
-        type: 'missing_thread',
-        threadID: dmOperation.threadID,
-      },
+      isProcessingPossible: true,
     };
   },
   supportsAutoRetry: true,
diff --git a/lib/shared/dm-ops/dm-op-spec.js b/lib/shared/dm-ops/dm-op-spec.js
index c45bc1789f..5ee466ad7e 100644
--- a/lib/shared/dm-ops/dm-op-spec.js
+++ b/lib/shared/dm-ops/dm-op-spec.js
@@ -13,6 +13,18 @@ export type ProcessDMOperationUtilities = {
   +entryInfos: RawEntryInfos,
 };
 
+type ProcessingPossibilityCheckResult =
+  | { +isProcessingPossible: true }
+  | {
+      +isProcessingPossible: false,
+      +reason:
+        | { +type: 'missing_thread', +threadID: string }
+        | { +type: 'missing_entry', +entryID: string }
+        | { +type: 'missing_message', +messageID: string }
+        | { +type: 'missing_membership', +threadID: string, +userID: string }
+        | { +type: 'invalid' },
+    };
+
 export type DMOperationSpec<DMOp: DMOperation> = {
   +notificationsCreationData?: (
     dmOp: DMOp,
@@ -27,16 +39,6 @@ export type DMOperationSpec<DMOp: DMOperation> = {
     dmOp: DMOp,
     viewerID: string,
     utilities: ProcessDMOperationUtilities,
-  ) =>
-    | { +isProcessingPossible: true }
-    | {
-        +isProcessingPossible: false,
-        +reason:
-          | { +type: 'missing_thread', +threadID: string }
-          | { +type: 'missing_entry', +entryID: string }
-          | { +type: 'missing_message', +messageID: string }
-          | { +type: 'missing_membership', +threadID: string, +userID: string }
-          | { +type: 'invalid' },
-      },
+  ) => Promise<ProcessingPossibilityCheckResult>,
   +supportsAutoRetry: boolean,
 };
diff --git a/lib/shared/dm-ops/edit-entry-spec.js b/lib/shared/dm-ops/edit-entry-spec.js
index 36ab0b9d40..b928b22741 100644
--- a/lib/shared/dm-ops/edit-entry-spec.js
+++ b/lib/shared/dm-ops/edit-entry-spec.js
@@ -92,20 +92,22 @@ const editEntrySpec: DMOperationSpec<DMEditEntryOperation> = Object.freeze({
       updateInfos: [entryUpdateInfo],
     };
   },
-  canBeProcessed(
+  canBeProcessed: async (
     dmOperation: DMEditEntryOperation,
     viewerID: string,
     utilities: ProcessDMOperationUtilities,
-  ) {
-    if (utilities.threadInfos[dmOperation.threadID]) {
-      return { isProcessingPossible: true };
+  ) => {
+    if (!utilities.entryInfos[dmOperation.entryID]) {
+      return {
+        isProcessingPossible: false,
+        reason: {
+          type: 'missing_entry',
+          entryID: dmOperation.entryID,
+        },
+      };
     }
     return {
-      isProcessingPossible: false,
-      reason: {
-        type: 'missing_thread',
-        threadID: dmOperation.threadID,
-      },
+      isProcessingPossible: true,
     };
   },
   supportsAutoRetry: true,
diff --git a/lib/shared/dm-ops/join-thread-spec.js b/lib/shared/dm-ops/join-thread-spec.js
index 0addfcb7f4..66da751151 100644
--- a/lib/shared/dm-ops/join-thread-spec.js
+++ b/lib/shared/dm-ops/join-thread-spec.js
@@ -162,11 +162,11 @@ const joinThreadSpec: DMOperationSpec<DMJoinThreadOperation> = Object.freeze({
       updateInfos,
     };
   },
-  canBeProcessed(
+  canBeProcessed: async (
     dmOperation: DMJoinThreadOperation,
     viewerID: string,
     utilities: ProcessDMOperationUtilities,
-  ) {
+  ) => {
     if (
       utilities.threadInfos[dmOperation.existingThreadDetails.threadID] ||
       dmOperation.joinerID === viewerID
diff --git a/lib/shared/dm-ops/leave-thread-spec.js b/lib/shared/dm-ops/leave-thread-spec.js
index 98facdeb8f..576ef0ceb5 100644
--- a/lib/shared/dm-ops/leave-thread-spec.js
+++ b/lib/shared/dm-ops/leave-thread-spec.js
@@ -135,11 +135,11 @@ const leaveThreadSpec: DMOperationSpec<DMLeaveThreadOperation> = Object.freeze({
       ],
     };
   },
-  canBeProcessed(
+  canBeProcessed: async (
     dmOperation: DMLeaveThreadOperation,
     viewerID: string,
     utilities: ProcessDMOperationUtilities,
-  ) {
+  ) => {
     if (utilities.threadInfos[dmOperation.threadID]) {
       return { isProcessingPossible: true };
     }
diff --git a/lib/shared/dm-ops/process-dm-ops.js b/lib/shared/dm-ops/process-dm-ops.js
index f4017aae2b..a31b1f4e1e 100644
--- a/lib/shared/dm-ops/process-dm-ops.js
+++ b/lib/shared/dm-ops/process-dm-ops.js
@@ -142,7 +142,7 @@ function useProcessDMOperation(): (
         return;
       }
 
-      const processingCheckResult = dmOpSpecs[dmOp.type].canBeProcessed(
+      const processingCheckResult = await dmOpSpecs[dmOp.type].canBeProcessed(
         dmOp,
         viewerID,
         utilities,
diff --git a/lib/shared/dm-ops/remove-members-spec.js b/lib/shared/dm-ops/remove-members-spec.js
index 0df2008518..63f51ba63a 100644
--- a/lib/shared/dm-ops/remove-members-spec.js
+++ b/lib/shared/dm-ops/remove-members-spec.js
@@ -109,11 +109,11 @@ const removeMembersSpec: DMOperationSpec<DMRemoveMembersOperation> =
         updateInfos,
       };
     },
-    canBeProcessed(
+    canBeProcessed: async (
       dmOperation: DMRemoveMembersOperation,
       viewerID: string,
       utilities: ProcessDMOperationUtilities,
-    ) {
+    ) => {
       if (utilities.threadInfos[dmOperation.threadID]) {
         return { isProcessingPossible: true };
       }
diff --git a/lib/shared/dm-ops/send-edit-message-spec.js b/lib/shared/dm-ops/send-edit-message-spec.js
index b41a24bb83..4eacf81f4e 100644
--- a/lib/shared/dm-ops/send-edit-message-spec.js
+++ b/lib/shared/dm-ops/send-edit-message-spec.js
@@ -42,20 +42,23 @@ const sendEditMessageSpec: DMOperationSpec<DMSendEditMessageOperation> =
         updateInfos: [],
       };
     },
-    canBeProcessed(
+    canBeProcessed: async (
       dmOperation: DMSendEditMessageOperation,
       viewerID: string,
       utilities: ProcessDMOperationUtilities,
-    ) {
-      if (utilities.threadInfos[dmOperation.threadID]) {
-        return { isProcessingPossible: true };
+    ) => {
+      const message = await utilities.fetchMessage(dmOperation.targetMessageID);
+      if (!message) {
+        return {
+          isProcessingPossible: false,
+          reason: {
+            type: 'missing_message',
+            messageID: dmOperation.targetMessageID,
+          },
+        };
       }
       return {
-        isProcessingPossible: false,
-        reason: {
-          type: 'missing_thread',
-          threadID: dmOperation.threadID,
-        },
+        isProcessingPossible: true,
       };
     },
     supportsAutoRetry: true,
diff --git a/lib/shared/dm-ops/send-multimedia-message-spec.js b/lib/shared/dm-ops/send-multimedia-message-spec.js
index 3497434677..ae54eb16c5 100644
--- a/lib/shared/dm-ops/send-multimedia-message-spec.js
+++ b/lib/shared/dm-ops/send-multimedia-message-spec.js
@@ -45,11 +45,11 @@ const sendMultimediaMessageSpec: DMOperationSpec<DMSendMultimediaMessageOperatio
         updateInfos,
       };
     },
-    canBeProcessed(
+    canBeProcessed: async (
       dmOperation: DMSendMultimediaMessageOperation,
       viewerID: string,
       utilities: ProcessDMOperationUtilities,
-    ) {
+    ) => {
       if (utilities.threadInfos[dmOperation.threadID]) {
         return { isProcessingPossible: true };
       }
diff --git a/lib/shared/dm-ops/send-reaction-message-spec.js b/lib/shared/dm-ops/send-reaction-message-spec.js
index e7920d2944..a4b2320384 100644
--- a/lib/shared/dm-ops/send-reaction-message-spec.js
+++ b/lib/shared/dm-ops/send-reaction-message-spec.js
@@ -44,20 +44,23 @@ const sendReactionMessageSpec: DMOperationSpec<DMSendReactionMessageOperation> =
         updateInfos: [],
       };
     },
-    canBeProcessed(
+    canBeProcessed: async (
       dmOperation: DMSendReactionMessageOperation,
       viewerID: string,
       utilities: ProcessDMOperationUtilities,
-    ) {
-      if (utilities.threadInfos[dmOperation.threadID]) {
-        return { isProcessingPossible: true };
+    ) => {
+      const message = await utilities.fetchMessage(dmOperation.targetMessageID);
+      if (!message) {
+        return {
+          isProcessingPossible: false,
+          reason: {
+            type: 'missing_message',
+            messageID: dmOperation.targetMessageID,
+          },
+        };
       }
       return {
-        isProcessingPossible: false,
-        reason: {
-          type: 'missing_thread',
-          threadID: dmOperation.threadID,
-        },
+        isProcessingPossible: true,
       };
     },
     supportsAutoRetry: true,
diff --git a/lib/shared/dm-ops/send-text-message-spec.js b/lib/shared/dm-ops/send-text-message-spec.js
index 42851cca4e..7536b4c305 100644
--- a/lib/shared/dm-ops/send-text-message-spec.js
+++ b/lib/shared/dm-ops/send-text-message-spec.js
@@ -42,11 +42,11 @@ const sendTextMessageSpec: DMOperationSpec<DMSendTextMessageOperation> =
         updateInfos,
       };
     },
-    canBeProcessed(
+    canBeProcessed: async (
       dmOperation: DMSendTextMessageOperation,
       viewerID: string,
       utilities: ProcessDMOperationUtilities,
-    ) {
+    ) => {
       if (utilities.threadInfos[dmOperation.threadID]) {
         return { isProcessingPossible: true };
       }