Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-datastore-version-cross-contamination.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@aws-amplify/datastore': patch
---

fix(datastore): prevent version cross-contamination between different model types sharing the same primary key (#13412)
77 changes: 72 additions & 5 deletions packages/datastore/__tests__/outbox.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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,
Expand All @@ -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(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
7 changes: 6 additions & 1 deletion packages/datastore/src/sync/outbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ class MutationEventOutbox {
mutationEventModelDefinition,
{
and: [
{ model: { eq: mutationEvent.model } },
{ modelId: { eq: mutationEvent.modelId } },
{ id: { ne: this.inProgressMutationEventId } },
],
Expand Down Expand Up @@ -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 } },
],
}),
);

Expand Down Expand Up @@ -218,6 +222,7 @@ class MutationEventOutbox {
mutationEventModelDefinition,
{
and: [
{ model: { eq: head.model } },
{ modelId: { eq: recordId } },
{ id: { ne: this.inProgressMutationEventId } },
],
Expand Down