[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
CommE2E · Sep 19, 2024 · 25b472e · 25b472e
1 parent bda7f89
commit 25b472e
Show file tree

Hide file tree

Showing 19 changed files with 104 additions and 73 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 };
       }