fix(datastore): Update outbox comparison logic to fix update versioni…

…ng problems (#12813)

packages/datastore/__tests__/conflictResolutionBehavior.test.ts

@@ -88,13 +88,13 @@ describe('DataStore sync engine', () => {
 						['post title 0', 1],
 						['post title 1', 1],
 						['post title 2', 1],
-						['post title 0', 3],
-						['post title 0', 3],
+						['post title 2', 3],
+						['post title 2', 3],
 					]);
 
 					expect(await postHarness.currentContents).toMatchObject({
 						_version: 3,
-						title: 'post title 0',
+						title: 'post title 2',
 					});
 				});
 				test('delayed input, low latency where we wait for the create to clear the outbox', async () => {
@@ -118,12 +118,12 @@ describe('DataStore sync engine', () => {
 						['post title 0', 1],
 						['post title 1', 1],
 						['post title 2', 1],
-						['post title 0', 4],
+						['post title 2', 4],
 					]);
 
 					expect(await postHarness.currentContents).toMatchObject({
 						_version: 4,
-						title: 'post title 0',
+						title: 'post title 2',
 					});
 				});
 				test("no input delay, high latency where we don't wait for the create to clear the outbox", async () => {
@@ -313,12 +313,17 @@ describe('DataStore sync engine', () => {
 						harness.latency = 'low';
 
 						await postHarness.revise('post title 0');
+						// Time warping is causing the 'post title 1' revision to wait
+						// until after 'post title 0' returns from the server (the setTimeout ticks are doing strange things)
 						await postHarness.revise('post title 1');
 						await harness.externalPostUpdate({
 							originalPostId: postHarness.original.id,
 							updatedFields: { title: 'update from second client' },
+							// The fake service version will be 2, having received the 'post title 0' update
+							// Because of this, the title will not be updated, but the version number will increment to 3
 							version: 1,
 						});
+						// The 'post title 1' change is in flight before this happens so that they don't merge
 						await postHarness.revise('post title 2');
 
 						await harness.fullSettle();
@@ -329,6 +334,9 @@ describe('DataStore sync engine', () => {
 							['post title 0', 1],
 							['post title 1', 1],
 							['post title 2', 1],
+							// Every change starting with 'post title 1' including the external
+							// update fails to automerge update the title, but increments
+							// the version.
 							['post title 0', 5],
 						]);
 
@@ -717,11 +725,7 @@ describe('DataStore sync engine', () => {
 					await postHarness.revise('post title 2');
 
 					await harness.fullSettle();
-					await harness.expectGraphqlSettledWithEventCount({
-						update: 3,
-						updateSubscriptionMessage: 2,
-						updateError: 1,
-					});
+					await harness.expectGraphqlSettledWithEventCount(2);
 
 					expect(harness.subscriptionLogs()).toEqual([
 						['original title', 1],
@@ -751,11 +755,7 @@ describe('DataStore sync engine', () => {
 					await postHarness.revise('post title 2');
 
 					await harness.fullSettle();
-					await harness.expectGraphqlSettledWithEventCount({
-						update: 4,
-						updateSubscriptionMessage: 3,
-						updateError: 1,
-					});
+					await harness.expectGraphqlSettledWithEventCount(3);
 
 					expect(harness.subscriptionLogs()).toEqual([
 						['original title', 1],

packages/datastore/src/sync/outbox.ts

@@ -15,7 +15,7 @@ import {
 	QueryOne,
 	SchemaModel,
 } from '../types';
-import { USER, SYNC, valuesEqual } from '../util';
+import { USER, SYNC, directedValueEquality } from '../util';
 import { getIdentifierValue, TransformerMutationType } from './utils';
 
 // TODO: Persist deleted ids
@@ -193,10 +193,12 @@ class MutationEventOutbox {
 		// Don't sync the version when the data in the response does not match the data
 		// in the request, i.e., when there's a handled conflict
 		//
-		// NOTE: `incomingData` contains all the fields in the record, and `outgoingData`
-		// only contains updated fields, resulting in an error when doing a comparison
-		// of two equal mutations. Fix this, or mitigate otherwise.
-		if (!valuesEqual(incomingData, outgoingData, true)) {
+		// NOTE: `incomingData` contains all the fields in the record received from AppSync
+		// and `outgoingData` only contains updated fields sent to AppSync
+		// If all send data isn't matched in the returned data then the update was rejected
+		// by AppSync and we should not update the version on other outbox entries for this
+		// object
+		if (!directedValueEquality(outgoingData, incomingData, true)) {
 			return;
 		}
 

packages/datastore/src/util.ts

@@ -473,6 +473,35 @@ export function sortCompareFunction<T extends PersistentModel>(
 	};
 }
 
+/* deep directed comparison ensuring that all fields on "from" object exist and
+ * are equal to values on an "against" object
+ *
+ * Note: This same guarauntee is not applied for values on "against" that aren't on "from"
+ *
+ * @param fromObject - The object that may be an equal subset of the againstObject.
+ * @param againstObject - The object that may be an equal superset of the fromObject.
+ *
+ * @returns True if fromObject is a equal subset of againstObject and False otherwise.
+ */
+export function directedValueEquality(
+	fromObject: object,
+	againstObject: object,
+	nullish: boolean = false
+) {
+	const aKeys = Object.keys(fromObject);
+
+	for (const key of aKeys) {
+		const fromValue = fromObject[key];
+		const againstValue = againstObject[key];
+
+		if (!valuesEqual(fromValue, againstValue, nullish)) {
+			return false;
+		}
+	}
+
+	return true;
+}
+
 // deep compare any 2 values
 // primitives or object types (including arrays, Sets, and Maps)
 // returns true if equal by value