From 5399132c7bf396a5daff2e61201241a3b3365623 Mon Sep 17 00:00:00 2001 From: Ivan Artemiev <29709626+iartemiev@users.noreply.github.com> Date: Mon, 23 Aug 2021 15:06:17 -0400 Subject: [PATCH] fix(@aws-amplify/datastore): check read-only at instance level --- packages/datastore/__tests__/DataStore.ts | 40 ++++---- packages/datastore/src/datastore/datastore.ts | 95 +++++++++---------- 2 files changed, 62 insertions(+), 73 deletions(-) diff --git a/packages/datastore/__tests__/DataStore.ts b/packages/datastore/__tests__/DataStore.ts index 3339d7709e7..5a6d5f36f34 100644 --- a/packages/datastore/__tests__/DataStore.ts +++ b/packages/datastore/__tests__/DataStore.ts @@ -557,36 +557,30 @@ describe('DataStore tests', () => { const { Model } = classes as { Model: PersistentModelConstructor }; - model = new Model({ - field1: 'something', - dateCreated: new Date().toISOString(), - createdAt: '2021-06-03T20:56:23.201Z', - } as any); - - await expect(DataStore.save(model)).rejects.toThrowError( - 'createdAt is read-only.' - ); + expect(() => { + new Model({ + field1: 'something', + dateCreated: new Date().toISOString(), + createdAt: '2021-06-03T20:56:23.201Z', + } as any); + }).toThrow('createdAt is read-only.'); model = new Model({ field1: 'something', dateCreated: new Date().toISOString(), }); - model = Model.copyOf(model, draft => { - (draft as any).createdAt = '2021-06-03T20:56:23.201Z'; - }); - - await expect(DataStore.save(model)).rejects.toThrowError( - 'createdAt is read-only.' - ); - - model = Model.copyOf(model, draft => { - (draft as any).updatedAt = '2021-06-03T20:56:23.201Z'; - }); + expect(() => { + Model.copyOf(model, draft => { + (draft as any).createdAt = '2021-06-03T20:56:23.201Z'; + }); + }).toThrow('createdAt is read-only.'); - await expect(DataStore.save(model)).rejects.toThrowError( - 'updatedAt is read-only.' - ); + expect(() => { + Model.copyOf(model, draft => { + (draft as any).updatedAt = '2021-06-03T20:56:23.201Z'; + }); + }).toThrow('updatedAt is read-only.'); }); test('Instantiation validations', async () => { diff --git a/packages/datastore/src/datastore/datastore.ts b/packages/datastore/src/datastore/datastore.ts index 94e21b58205..17b987f50e8 100644 --- a/packages/datastore/src/datastore/datastore.ts +++ b/packages/datastore/src/datastore/datastore.ts @@ -374,13 +374,18 @@ const createModelClass = ( _deleted, } = modelInstanceMetadata; - const id = - // instancesIds is set by modelInstanceCreator, it is accessible only internally - _id !== null && _id !== undefined - ? _id - : modelDefinition.syncable - ? uuid4() - : ulid(); + // instancesIds are set by modelInstanceCreator, it is accessible only internally + const isInternal = _id !== null && _id !== undefined; + + const id = isInternal + ? _id + : modelDefinition.syncable + ? uuid4() + : ulid(); + + if (!isInternal) { + checkReadOnlyPropertyOnCreate(draft, modelDefinition); + } draft.id = id; @@ -408,7 +413,9 @@ const createModelClass = ( source, draft => { fn(>(draft as unknown)); + draft.id = source.id; + const modelValidator = validateModelFields(modelDefinition); Object.entries(draft).forEach(([k, v]) => { modelValidator(k, v); @@ -419,6 +426,7 @@ const createModelClass = ( if (patches.length) { modelPatchesMap.set(model, [patches, source]); + checkReadOnlyPropertyOnUpdate(patches, modelDefinition); } return model; @@ -449,6 +457,36 @@ const createModelClass = ( return clazz; }; +const checkReadOnlyPropertyOnCreate = ( + draft: T, + modelDefinition: SchemaModel +) => { + const modelKeys = Object.keys(draft); + const { fields } = modelDefinition; + + modelKeys.forEach(key => { + if (fields[key] && fields[key].isReadOnly) { + throw new Error(`${key} is read-only.`); + } + }); +}; + +const checkReadOnlyPropertyOnUpdate = ( + patches: Patch[], + modelDefinition: SchemaModel +) => { + const patchArray = patches.map(p => [p.path[0], p.value]); + const { fields } = modelDefinition; + + patchArray.forEach(([key, val]) => { + if (!val || !fields[key]) return; + + if (fields[key].isReadOnly) { + throw new Error(`${key} is read-only.`); + } + }); +}; + const createNonModelClass = (typeDefinition: SchemaNonModel) => { const clazz = >(class Model { constructor(init: ModelInit) { @@ -815,9 +853,6 @@ class DataStore { const modelDefinition = getModelDefinition(modelConstructor); - // ensuring "read-only" data isn't being overwritten - this.checkReadOnlyProperty(modelDefinition.fields, model, patchesTuple); - const producedCondition = ModelPredicateCreator.createFromExisting( modelDefinition, condition @@ -835,46 +870,6 @@ class DataStore { return savedModel; }; - private checkReadOnlyProperty( - fields: ModelFields, - model: Record, - patchesTuple: [ - Patch[], - Readonly< - { - id: string; - } & Record - > - ] - ) { - if (!patchesTuple) { - // saving a new model instance - const modelKeys = Object.keys(model); - modelKeys.forEach(key => { - if (fields[key] && fields[key].isReadOnly) { - throw new Error(`${key} is read-only.`); - } - }); - } else { - // * Updating an existing instance via 'patchesTuple' - // patchesTuple[0] is an object that contains the info we need - // like the 'path' (mapped to the model's key) and the 'value' of the patch - const patchArray = patchesTuple[0].map(p => [p.path[0], p.value]); - patchArray.forEach(patch => { - const [key, val] = [...patch]; - - // the value of a read-only field should be undefined - if so, no need to do the following check - if (!val || !fields[key]) return; - - // if the value is NOT undefined, we have to check the 'isReadOnly' property - // and throw an error to avoid persisting a mutation - if (fields[key].isReadOnly) { - throw new Error(`${key} is read-only.`); - } - }); - } - } - setConflictHandler = (config: DataStoreConfig): ConflictHandler => { const { DataStore: configDataStore } = config;