fix(@aws-amplify/datastore): check read-only at instance level

aws-amplify · Aug 23, 2021 · 5399132 · 5399132
1 parent da8ef23
commit 5399132
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 73 deletions.
diff --git 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> };
 
-			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
@@ -374,13 +374,18 @@ const createModelClass = <T extends PersistentModel>(
 						_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 = <T extends PersistentModel>(
 				source,
 				draft => {
 					fn(<MutableModel<T>>(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 = <T extends PersistentModel>(
 
 			if (patches.length) {
 				modelPatchesMap.set(model, [patches, source]);
+				checkReadOnlyPropertyOnUpdate(patches, modelDefinition);
 			}
 
 			return model;
@@ -449,6 +457,36 @@ const createModelClass = <T extends PersistentModel>(
 	return clazz;
 };
 
+const checkReadOnlyPropertyOnCreate = <T extends PersistentModel>(
+	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 = <T>(typeDefinition: SchemaNonModel) => {
 	const clazz = <NonModelTypeConstructor<T>>(<unknown>class Model {
 		constructor(init: ModelInit<T>) {
@@ -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<string, any>,
-		patchesTuple: [
-			Patch[],
-			Readonly<
-				{
-					id: string;
-				} & Record<string, any>
-			>
-		]
-	) {
-		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;