fix(datastore): prevent version cross-contamination between different model types (#13412)

All MutationEvent queries in the outbox (enqueue, getForModel,
syncOutboxVersionsOnDequeue) now filter by both `model` and `modelId`
to prevent _version cross-contamination when different model types
share the same primary key value.

Added test verifying that dequeuing a mutation for one model type does
not update the _version on queued mutations for a different model type
with the same ID.

Author: anivar

packages/datastore/__tests__/outbox.test.ts

@@ -131,7 +131,7 @@ describe('Outbox tests', () => {
 			expect(head.modelId).toEqual(modelId);
 			expect(head.operation).toEqual(TransformerMutationType.UPDATE);
 			expect(modelData.field1).toEqual('another value');
-			const modelDefinition = getModelDefinition(last);
+			const modelDefinition = getModelDefinition(Model);
 			const mutationsForModel = await outbox.getForModel(
 				s,
 				last,
@@ -155,7 +155,7 @@ describe('Outbox tests', () => {
 
 		await outbox.enqueue(Storage, await createMutationEvent(updatedModel3));
 
-		const modelDefinition = getModelDefinition(last);
+		const modelDefinition = getModelDefinition(Model);
 
 		// model2 should get deleted when model3 is enqueued, so we're expecting to see
 		// 2 items in the queue for this Model total (including the in progress record - updatedModel1)
@@ -243,7 +243,7 @@ describe('Outbox tests', () => {
 			expect(head.operation).toEqual(TransformerMutationType.UPDATE);
 			expect(modelData.field1).toEqual('another value');
 
-			const modelDefinition = getModelDefinition(last);
+			const modelDefinition = getModelDefinition(Model);
 			const mutationsForModel = await outbox.getForModel(
 				s,
 				last,
@@ -259,7 +259,7 @@ describe('Outbox tests', () => {
 		});
 
 		await outbox.enqueue(Storage, await createMutationEvent(updatedModel2));
-		const modelDefinition = getModelDefinition(last);
+		const modelDefinition = getModelDefinition(Model);
 
 		// 2 items in the queue for this Model total (including the in progress record - updatedModel1)
 		const mutationsForModel = await outbox.getForModel(
@@ -348,6 +348,73 @@ describe('Outbox tests', () => {
 			expect(headData.optionalField1).toEqual(optionalField1);
 		});
 	});
+
+	it('Should NOT sync the _version across different model types with the same ID', async () => {
+		// Verifies fix for #13412: mutations for different model types sharing the
+		// same primary key must not have their _version cross-contaminated.
+
+		const model1 = new Model({
+			field1: 'model1 value',
+			dateCreated: new Date().toISOString(),
+		});
+
+		await DataStore.save(model1);
+
+		const updatedModel1 = Model.copyOf(model1, updated => {
+			updated.field1 = 'updated model1 value';
+		});
+
+		const mutationEvent1 = await createMutationEvent(updatedModel1);
+		await outbox.enqueue(Storage, mutationEvent1);
+
+		// Insert a mutation for a different model type sharing the same modelId
+		const MutationEventConstructor = syncClasses[
+			'MutationEvent'
+		] as PersistentModelConstructor<MutationEvent>;
+
+		await Storage.save(
+			new MutationEventConstructor({
+				id: 'diff-model-mutation',
+				model: 'DifferentModel',
+				modelId: model1.id,
+				operation: TransformerMutationType.UPDATE,
+				data: JSON.stringify({
+					id: model1.id,
+					someField: 'different model value',
+					_version: 5,
+				}),
+				condition: JSON.stringify(null),
+			}),
+		);
+
+		const response1 = {
+			...updatedModel1,
+			_version: 20,
+			_lastChangedAt: Date.now(),
+			_deleted: false,
+		};
+
+		await Storage.runExclusive(async s => {
+			await processMutationResponse(
+				s,
+				response1,
+				TransformerMutationType.UPDATE,
+			);
+
+			const allMutations = await s.query(MutationEventConstructor);
+			const differentModelMutations = allMutations.filter(
+				m => m.model === 'DifferentModel',
+			);
+
+			expect(differentModelMutations.length).toBeGreaterThan(0);
+
+			const differentModelData = JSON.parse(
+				differentModelMutations[0].data,
+			);
+			expect(differentModelData._version).toEqual(5);
+		});
+	});
+
 });
 
 // performs all the required dependency injection
@@ -415,6 +482,6 @@ async function processMutationResponse(
 
 	const modelConstructor = Model as unknown as PersistentModelConstructor<any>;
 	const model = modelInstanceCreator(modelConstructor, record);
-	const modelDefinition = getModelDefinition(model);
+	const modelDefinition = getModelDefinition(modelConstructor);
 	await merger.merge(storage, model, modelDefinition);
 }

packages/datastore/src/sync/outbox.ts

@@ -46,6 +46,7 @@ class MutationEventOutbox {
 				mutationEventModelDefinition,
 				{
 					and: [
+						{ model: { eq: mutationEvent.model } },
 						{ modelId: { eq: mutationEvent.modelId } },
 						{ id: { ne: this.inProgressMutationEventId } },
 					],
@@ -148,7 +149,10 @@ class MutationEventOutbox {
 		const mutationEvents = await storage.query(
 			this._MutationEvent,
 			ModelPredicateCreator.createFromAST(mutationEventModelDefinition, {
-				and: { modelId: { eq: modelId } },
+				and: [
+					{ model: { eq: userModelDefinition.name } },
+					{ modelId: { eq: modelId } },
+				],
 			}),
 		);
 
@@ -218,6 +222,7 @@ class MutationEventOutbox {
 			mutationEventModelDefinition,
 			{
 				and: [
+					{ model: { eq: head.model } },
 					{ modelId: { eq: recordId } },
 					{ id: { ne: this.inProgressMutationEventId } },
 				],