diff --git a/.changeset/fix-datastore-version-cross-contamination.md b/.changeset/fix-datastore-version-cross-contamination.md new file mode 100644 index 00000000000..777ca189dce --- /dev/null +++ b/.changeset/fix-datastore-version-cross-contamination.md @@ -0,0 +1,5 @@ +--- +'@aws-amplify/datastore': patch +--- + +fix(datastore): prevent version cross-contamination between different model types sharing the same primary key (#13412) diff --git a/packages/datastore/__tests__/outbox.test.ts b/packages/datastore/__tests__/outbox.test.ts index 20cf7a46efc..0e1280529a1 100644 --- a/packages/datastore/__tests__/outbox.test.ts +++ b/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; + + 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; const model = modelInstanceCreator(modelConstructor, record); - const modelDefinition = getModelDefinition(model); + const modelDefinition = getModelDefinition(modelConstructor); await merger.merge(storage, model, modelDefinition); } diff --git a/packages/datastore/src/sync/outbox.ts b/packages/datastore/src/sync/outbox.ts index b555e47b5dd..820588d847e 100644 --- a/packages/datastore/src/sync/outbox.ts +++ b/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 } }, ],