Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 3 commits into from
Aug 24, 2021
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c4cbea3
fix(@aws-amplify/datastore): check read-only at instance level
iartemiev Aug 23, 2021
0176bfd
Merge branch 'main' into ds-issue-8649
iartemiev Aug 24, 2021
8041e59
Merge branch 'main' into ds-issue-8649
iartemiev Aug 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 17 additions & 23 deletions packages/datastore/__tests__/DataStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
94 changes: 43 additions & 51 deletions packages/datastore/src/datastore/datastore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ import {
ErrorHandler,
SyncExpression,
AuthModeStrategyType,
ModelFields,
} from '../types';
import {
DATASTORE,
Expand Down Expand Up @@ -374,13 +373,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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for storing this value in a defined variable, makes the following code much more readable upon first glance.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you also needed it stored to reference later, but still, readability is a great added benefit :)


const id = isInternal
? _id
: modelDefinition.syncable
? uuid4()
: ulid();

if (!isInternal) {
checkReadOnlyPropertyOnCreate(draft, modelDefinition);
}

draft.id = id;

Expand Down Expand Up @@ -419,6 +423,7 @@ const createModelClass = <T extends PersistentModel>(

if (patches.length) {
modelPatchesMap.set(model, [patches, source]);
checkReadOnlyPropertyOnUpdate(patches, modelDefinition);
}

return model;
Expand Down Expand Up @@ -449,6 +454,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>) {
Expand Down Expand Up @@ -815,9 +850,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
Expand All @@ -835,46 +867,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;

Expand Down