Skip to content

Commit

Permalink
fix(datastore): remove connections with model field as undefined (#10983
Browse files Browse the repository at this point in the history
)
  • Loading branch information
dpilch authored Mar 1, 2023
1 parent 37b9c23 commit 91e0c8f
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 24 deletions.
63 changes: 63 additions & 0 deletions packages/datastore/__tests__/DataStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1733,6 +1733,69 @@ describe('Model behavior', () => {
expect(await disconnectedParent.child).toBeUndefined();
});

[null, undefined].forEach(value => {
test(`model field can be set to ${value} to remove connection hasOne parent`, async () => {
const { DataStore, HasOneChild, HasOneParent } = getDataStore();

const child = await DataStore.save(
new HasOneChild({ content: 'child content' })
);
const parent = await DataStore.save(
new HasOneParent({
child,
})
);

const parentWithoutChild = HasOneParent.copyOf(parent, draft => {
draft.child = value;
});

expect(parentWithoutChild.hasOneParentChildId).toBeUndefined();
expect(
(await DataStore.save(parentWithoutChild)).hasOneParentChildId
).toBeUndefined();
expect(
(await DataStore.query(HasOneParent, parent.id))!.hasOneParentChildId
).toBeUndefined();
});

test(`model field can be set to ${value} to remove connection on child hasMany`, async () => {
const { DataStore, CompositePKParent, CompositePKChild } = getDataStore();

const parent = await DataStore.save(
new CompositePKParent({
customId: 'customId',
content: 'content',
})
);

const child = await DataStore.save(
new CompositePKChild({ childId: 'childId', content: 'content', parent })
);

const childWithoutParent = CompositePKChild.copyOf(child, draft => {
draft.parent = value;
});

expect(await childWithoutParent.parent).toBeUndefined();
expect(
await DataStore.save(childWithoutParent).then(c => c.parent)
).toBeUndefined();
expect(
await DataStore.query(CompositePKChild, {
childId: child.childId,
content: child.content,
}).then(c => c!.parent)
).toBeUndefined();
expect(
await DataStore.query(CompositePKParent, {
customId: parent.customId,
content: parent.content,
}).then(c => c!.children.toArray())
).toEqual([]);
});
});

test('removes no-longer-matching items from the snapshot when using an eq() predicate on boolean field', done => {
(async () => {
const { DataStore, ModelWithBoolean } = getDataStore();
Expand Down
48 changes: 26 additions & 22 deletions packages/datastore/src/datastore/datastore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -908,43 +908,47 @@ const createModelClass = <T extends PersistentModel>(
);

Object.defineProperty(clazz.prototype, modelDefinition.fields[field].name, {
set(model: PersistentModel) {
if (!model || !(typeof model === 'object')) return;
set(model: PersistentModel | undefined | null) {
if (!(typeof model === 'object' || typeof model === 'undefined'))
return;

// Avoid validation error when processing AppSync response with nested
// selection set. Nested entitites lack version field and can not be validated
// TODO: explore a more reliable method to solve this
if (model.hasOwnProperty('_version')) {
const modelConstructor = Object.getPrototypeOf(model || {})
.constructor as PersistentModelConstructor<T>;
// if model is undefined or null, the connection should be removed
if (model) {
// Avoid validation error when processing AppSync response with nested
// selection set. Nested entitites lack version field and can not be validated
// TODO: explore a more reliable method to solve this
if (model.hasOwnProperty('_version')) {
const modelConstructor = Object.getPrototypeOf(model || {})
.constructor as PersistentModelConstructor<T>;

if (!isValidModelConstructor(modelConstructor)) {
const msg = `Value passed to ${modelDefinition.name}.${field} is not a valid instance of a model`;
logger.error(msg, { model });
if (!isValidModelConstructor(modelConstructor)) {
const msg = `Value passed to ${modelDefinition.name}.${field} is not a valid instance of a model`;
logger.error(msg, { model });

throw new Error(msg);
}
throw new Error(msg);
}

if (
modelConstructor.name.toLowerCase() !==
relationship.remoteModelConstructor.name.toLowerCase()
) {
const msg = `Value passed to ${modelDefinition.name}.${field} is not an instance of ${relationship.remoteModelConstructor.name}`;
logger.error(msg, { model });
if (
modelConstructor.name.toLowerCase() !==
relationship.remoteModelConstructor.name.toLowerCase()
) {
const msg = `Value passed to ${modelDefinition.name}.${field} is not an instance of ${relationship.remoteModelConstructor.name}`;
logger.error(msg, { model });

throw new Error(msg);
throw new Error(msg);
}
}
}

if (relationship.isComplete) {
for (let i = 0; i < relationship.localJoinFields.length; i++) {
this[relationship.localJoinFields[i]] =
model[relationship.remoteJoinFields[i]];
model?.[relationship.remoteJoinFields[i]];
}
const instanceMemos = modelInstanceAssociationsMap.has(this)
? modelInstanceAssociationsMap.get(this)!
: modelInstanceAssociationsMap.set(this, {}).get(this)!;
instanceMemos[field] = model;
instanceMemos[field] = model || undefined;
}
},
get() {
Expand Down
6 changes: 4 additions & 2 deletions packages/datastore/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -597,9 +597,11 @@ type DeepWritable<T> = {
-readonly [P in keyof T]: T[P] extends TypeName<T[P]>
? T[P]
: T[P] extends Promise<infer InnerPromiseType>
? InnerPromiseType
? undefined extends InnerPromiseType
? InnerPromiseType | null
: InnerPromiseType
: T[P] extends AsyncCollection<infer InnerCollectionType>
? InnerCollectionType[] | undefined
? InnerCollectionType[] | undefined | null
: DeepWritable<T[P]>;
};

Expand Down

0 comments on commit 91e0c8f

Please sign in to comment.