From d521d17c45a246c63c02a29e103e8a3db374c11e Mon Sep 17 00:00:00 2001 From: Ivan Artemiev <29709626+iartemiev@users.noreply.github.com> Date: Wed, 17 Nov 2021 14:33:07 -0500 Subject: [PATCH] feat(@aws-amplify/datastore): hasOne CRUD improvements (#9239) --- .prettierrc.js | 1 + .../src/SQLiteAdapter/SQLiteAdapter.ts | 26 ++++----- .../src/SQLiteAdapter/SQLiteUtils.ts | 12 ++-- .../src/SQLiteAdapter/types.ts | 4 +- .../__tests__/AsyncStorageAdapter.test.ts | 54 +++++++++++++++-- .../__tests__/IndexedDBAdapter.test.ts | 46 +++++++++++++++ packages/datastore/__tests__/helpers.ts | 3 +- .../storage/adapter/AsyncStorageAdapter.ts | 38 ++++++------ .../src/storage/adapter/IndexedDBAdapter.ts | 58 ++++++++++--------- packages/datastore/src/util.ts | 20 +++++-- 10 files changed, 186 insertions(+), 76 deletions(-) diff --git a/.prettierrc.js b/.prettierrc.js index d82995ee3b6..1ddacd85aeb 100644 --- a/.prettierrc.js +++ b/.prettierrc.js @@ -2,4 +2,5 @@ module.exports = { trailingComma: 'es5', singleQuote: true, useTabs: true, + arrowParens: 'avoid', }; diff --git a/packages/datastore-storage-adapter/src/SQLiteAdapter/SQLiteAdapter.ts b/packages/datastore-storage-adapter/src/SQLiteAdapter/SQLiteAdapter.ts index 1ec312e4c10..d933cb7d024 100644 --- a/packages/datastore-storage-adapter/src/SQLiteAdapter/SQLiteAdapter.ts +++ b/packages/datastore-storage-adapter/src/SQLiteAdapter/SQLiteAdapter.ts @@ -194,21 +194,19 @@ export class SQLiteAdapter implements StorageAdapter { switch (relationType) { case 'HAS_ONE': for await (const recordItem of records) { - if (recordItem[fieldName]) { - const [queryStatement, params] = queryByIdStatement( - recordItem[fieldName], - tableName - ); + const getByfield = recordItem[targetName] ? targetName : fieldName; + if (!recordItem[getByfield]) break; - const connectionRecord = await this.db.get( - queryStatement, - params - ); + const [queryStatement, params] = queryByIdStatement( + recordItem[getByfield], + tableName + ); - recordItem[fieldName] = - connectionRecord && - this.modelInstanceCreator(modelConstructor, connectionRecord); - } + const connectionRecord = await this.db.get(queryStatement, params); + + recordItem[fieldName] = + connectionRecord && + this.modelInstanceCreator(modelConstructor, connectionRecord); } break; @@ -236,7 +234,7 @@ export class SQLiteAdapter implements StorageAdapter { // TODO: Lazy loading break; default: - const _: never = relationType; + const _: never = relationType as never; throw new Error(`invalid relation type ${relationType}`); break; } diff --git a/packages/datastore-storage-adapter/src/SQLiteAdapter/SQLiteUtils.ts b/packages/datastore-storage-adapter/src/SQLiteAdapter/SQLiteUtils.ts index 06de71f6947..e7d667de87c 100644 --- a/packages/datastore-storage-adapter/src/SQLiteAdapter/SQLiteUtils.ts +++ b/packages/datastore-storage-adapter/src/SQLiteAdapter/SQLiteUtils.ts @@ -90,9 +90,8 @@ export const implicitAuthFieldsForModel: (model: SchemaModel) => string[] = ( return []; } - const authRules: ModelAttributeAuth = model.attributes.find( - isModelAttributeAuth - ); + const authRules: ModelAttributeAuth = + model.attributes.find(isModelAttributeAuth); if (!authRules) { return []; @@ -307,7 +306,7 @@ export function whereClauseFromPredicate( filterType = 'OR'; break; default: - const _: never = groupType; + const _: never = groupType as never; throw new Error(`Invalid ${groupType}`); } @@ -319,9 +318,8 @@ export function whereClauseFromPredicate( `${isNegation ? 'NOT' : ''}(${groupResult.join(` ${filterType} `)})` ); } else if (isPredicateObj(predicate)) { - const [condition, conditionParams] = whereConditionFromPredicateObject( - predicate - ); + const [condition, conditionParams] = + whereConditionFromPredicateObject(predicate); result.push(condition); params.push(...conditionParams); diff --git a/packages/datastore-storage-adapter/src/SQLiteAdapter/types.ts b/packages/datastore-storage-adapter/src/SQLiteAdapter/types.ts index dd781657c57..38e6cc96365 100644 --- a/packages/datastore-storage-adapter/src/SQLiteAdapter/types.ts +++ b/packages/datastore-storage-adapter/src/SQLiteAdapter/types.ts @@ -25,7 +25,7 @@ export function getSQLiteType( case 'Float': return 'REAL'; default: - const _: never = scalar; - throw new Error(`unknown type ${scalar}`); + const _: never = scalar as never; + throw new Error(`unknown type ${scalar as string}`); } } diff --git a/packages/datastore/__tests__/AsyncStorageAdapter.test.ts b/packages/datastore/__tests__/AsyncStorageAdapter.test.ts index 8b141f671d0..0c8c4636b3c 100644 --- a/packages/datastore/__tests__/AsyncStorageAdapter.test.ts +++ b/packages/datastore/__tests__/AsyncStorageAdapter.test.ts @@ -13,6 +13,10 @@ let DataStore: typeof DataStoreType; const ASAdapter = AsyncStorageAdapter; describe('AsyncStorageAdapter tests', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + describe('Query', () => { let Model: PersistentModelConstructor; let model1Id: string; @@ -56,10 +60,6 @@ describe('AsyncStorageAdapter tests', () => { ); }); - beforeEach(() => { - jest.clearAllMocks(); - }); - it('Should call getById for query by id', async () => { const result = await DataStore.query(Model, model1Id); @@ -169,4 +169,50 @@ describe('AsyncStorageAdapter tests', () => { expect(profile).toBeUndefined; }); }); + + describe('Save', () => { + let User: PersistentModelConstructor; + let Profile: PersistentModelConstructor; + let profile: Profile; + + beforeAll(async () => { + ({ initSchema, DataStore } = require('../src/datastore/datastore')); + + const classes = initSchema(testSchema()); + + ({ User } = classes as { + User: PersistentModelConstructor; + }); + + ({ Profile } = classes as { + Profile: PersistentModelConstructor; + }); + + profile = await DataStore.save( + new Profile({ firstName: 'Rick', lastName: 'Bob' }) + ); + }); + + it('should allow linking model via model field', async () => { + const savedUser = await DataStore.save( + new User({ name: 'test', profile }) + ); + const user1Id = savedUser.id; + + const user = await DataStore.query(User, user1Id); + expect(user.profileID).toEqual(profile.id); + expect(user.profile).toEqual(profile); + }); + + it('should allow linking model via FK', async () => { + const savedUser = await DataStore.save( + new User({ name: 'test', profileID: profile.id }) + ); + const user1Id = savedUser.id; + + const user = await DataStore.query(User, user1Id); + expect(user.profileID).toEqual(profile.id); + expect(user.profile).toEqual(profile); + }); + }); }); diff --git a/packages/datastore/__tests__/IndexedDBAdapter.test.ts b/packages/datastore/__tests__/IndexedDBAdapter.test.ts index 9532835bb8f..3325df17705 100644 --- a/packages/datastore/__tests__/IndexedDBAdapter.test.ts +++ b/packages/datastore/__tests__/IndexedDBAdapter.test.ts @@ -155,4 +155,50 @@ describe('IndexedDBAdapter tests', () => { expect(profile).toBeUndefined; }); }); + + describe('Save', () => { + let User: PersistentModelConstructor; + let Profile: PersistentModelConstructor; + let profile: Profile; + + beforeAll(async () => { + ({ initSchema, DataStore } = require('../src/datastore/datastore')); + + const classes = initSchema(testSchema()); + + ({ User } = classes as { + User: PersistentModelConstructor; + }); + + ({ Profile } = classes as { + Profile: PersistentModelConstructor; + }); + + profile = await DataStore.save( + new Profile({ firstName: 'Rick', lastName: 'Bob' }) + ); + }); + + it('should allow linking model via model field', async () => { + const savedUser = await DataStore.save( + new User({ name: 'test', profile }) + ); + const user1Id = savedUser.id; + + const user = await DataStore.query(User, user1Id); + expect(user.profileID).toEqual(profile.id); + expect(user.profile).toEqual(profile); + }); + + it('should allow linking model via FK', async () => { + const savedUser = await DataStore.save( + new User({ name: 'test', profileID: profile.id }) + ); + const user1Id = savedUser.id; + + const user = await DataStore.query(User, user1Id); + expect(user.profileID).toEqual(profile.id); + expect(user.profile).toEqual(profile); + }); + }); }); diff --git a/packages/datastore/__tests__/helpers.ts b/packages/datastore/__tests__/helpers.ts index 5fcfe24f138..752ac56a86f 100644 --- a/packages/datastore/__tests__/helpers.ts +++ b/packages/datastore/__tests__/helpers.ts @@ -48,7 +48,8 @@ export declare class Comment { export declare class User { public readonly id: string; public readonly name: string; - public readonly profileID: string; + public readonly profile?: Profile; + public readonly profileID?: string; } export declare class Profile { public readonly id: string; diff --git a/packages/datastore/src/storage/adapter/AsyncStorageAdapter.ts b/packages/datastore/src/storage/adapter/AsyncStorageAdapter.ts index f461fee0099..2729379533a 100644 --- a/packages/datastore/src/storage/adapter/AsyncStorageAdapter.ts +++ b/packages/datastore/src/storage/adapter/AsyncStorageAdapter.ts @@ -184,16 +184,17 @@ export class AsyncStorageAdapter implements Adapter { switch (relationType) { case 'HAS_ONE': for await (const recordItem of records) { - if (recordItem[fieldName]) { - const connectionRecord = await this.db.get( - recordItem[fieldName], - storeName - ); + const getByfield = recordItem[targetName] ? targetName : fieldName; + if (!recordItem[getByfield]) break; - recordItem[fieldName] = - connectionRecord && - this.modelInstanceCreator(modelConstructor, connectionRecord); - } + const connectionRecord = await this.db.get( + recordItem[getByfield], + storeName + ); + + recordItem[fieldName] = + connectionRecord && + this.modelInstanceCreator(modelConstructor, connectionRecord); } break; @@ -352,9 +353,9 @@ export class AsyncStorageAdapter implements Adapter { // models to be deleted. const models = await this.query(modelConstructor, condition); // TODO: refactor this to use a function like getRelations() - const relations = this.schema.namespaces[nameSpace].relationships[ - modelConstructor.name - ].relationTypes; + const relations = + this.schema.namespaces[nameSpace].relationships[modelConstructor.name] + .relationTypes; if (condition !== undefined) { await this.deleteTraverse( @@ -419,9 +420,9 @@ export class AsyncStorageAdapter implements Adapter { throw new Error(msg); } - const relations = this.schema.namespaces[nameSpace].relationships[ - modelConstructor.name - ].relationTypes; + const relations = + this.schema.namespaces[nameSpace].relationships[modelConstructor.name] + .relationTypes; await this.deleteTraverse( relations, [model], @@ -430,9 +431,9 @@ export class AsyncStorageAdapter implements Adapter { deleteQueue ); } else { - const relations = this.schema.namespaces[nameSpace].relationships[ - modelConstructor.name - ].relationTypes; + const relations = + this.schema.namespaces[nameSpace].relationships[modelConstructor.name] + .relationTypes; await this.deleteTraverse( relations, @@ -510,6 +511,7 @@ export class AsyncStorageAdapter implements Adapter { const hasOneCustomField = targetName in model; const value = hasOneCustomField ? model[targetName] : model.id; + if (!value) break; const allRecords = await this.db.getAll(storeName); const recordToDelete = allRecords.filter( diff --git a/packages/datastore/src/storage/adapter/IndexedDBAdapter.ts b/packages/datastore/src/storage/adapter/IndexedDBAdapter.ts index b5a6dd3f152..7d607802704 100644 --- a/packages/datastore/src/storage/adapter/IndexedDBAdapter.ts +++ b/packages/datastore/src/storage/adapter/IndexedDBAdapter.ts @@ -121,8 +121,10 @@ class IndexedDBAdapter implements Adapter { autoIncrement: true, }); - const indexes = this.schema.namespaces[namespaceName] - .relationships[modelName].indexes; + const indexes = + this.schema.namespaces[namespaceName].relationships[ + modelName + ].indexes; indexes.forEach(index => store.createIndex(index, index)); store.createIndex('byId', 'id', { unique: true }); @@ -310,16 +312,17 @@ class IndexedDBAdapter implements Adapter { switch (relation.relationType) { case 'HAS_ONE': for await (const recordItem of records) { - if (recordItem[fieldName]) { - const connectionRecord = await this._get( - store, - recordItem[fieldName] - ); + const getByfield = recordItem[targetName] ? targetName : fieldName; + if (!recordItem[getByfield]) break; - recordItem[fieldName] = - connectionRecord && - this.modelInstanceCreator(modelConstructor, connectionRecord); - } + const connectionRecord = await this._get( + store, + recordItem[getByfield] + ); + + recordItem[fieldName] = + connectionRecord && + this.modelInstanceCreator(modelConstructor, connectionRecord); } break; @@ -533,9 +536,9 @@ class IndexedDBAdapter implements Adapter { const storeName = this.getStorenameForModel(modelConstructor); const models = await this.query(modelConstructor, condition); - const relations = this.schema.namespaces[nameSpace].relationships[ - modelConstructor.name - ].relationTypes; + const relations = + this.schema.namespaces[nameSpace].relationships[modelConstructor.name] + .relationTypes; if (condition !== undefined) { await this.deleteTraverse( @@ -611,9 +614,9 @@ class IndexedDBAdapter implements Adapter { } await tx.done; - const relations = this.schema.namespaces[nameSpace].relationships[ - modelConstructor.name - ].relationTypes; + const relations = + this.schema.namespaces[nameSpace].relationships[modelConstructor.name] + .relationTypes; await this.deleteTraverse( relations, @@ -623,9 +626,9 @@ class IndexedDBAdapter implements Adapter { deleteQueue ); } else { - const relations = this.schema.namespaces[nameSpace].relationships[ - modelConstructor.name - ].relationTypes; + const relations = + this.schema.namespaces[nameSpace].relationships[modelConstructor.name] + .relationTypes; await this.deleteTraverse( relations, @@ -709,12 +712,15 @@ class IndexedDBAdapter implements Adapter { const hasOneCustomField = targetName in model; const value = hasOneCustomField ? model[targetName] : model.id; - - const recordToDelete = await this.db - .transaction(storeName, 'readwrite') - .objectStore(storeName) - .index(hasOneIndex) - .get(value); + if (!value) break; + + const recordToDelete = ( + await this.db + .transaction(storeName, 'readwrite') + .objectStore(storeName) + .index(hasOneIndex) + .get(value) + ); await this.deleteTraverse( this.schema.namespaces[nameSpace].relationships[modelName] diff --git a/packages/datastore/src/util.ts b/packages/datastore/src/util.ts index f15c630006e..0e5364281ce 100644 --- a/packages/datastore/src/util.ts +++ b/packages/datastore/src/util.ts @@ -337,6 +337,7 @@ export const traverseModel = ( ); } catch (error) { // Do nothing + console.log(error); } result.push({ @@ -345,9 +346,20 @@ export const traverseModel = ( instance: modelInstance, }); - (draftInstance)[rItem.fieldName] = (( - draftInstance[rItem.fieldName] - )).id; + // targetName will be defined for Has One if feature flag + // https://docs.amplify.aws/cli/reference/feature-flags/#useAppsyncModelgenPlugin + // is true (default as of 5/7/21) + // Making this conditional for backward-compatibility + if (rItem.targetName) { + (draftInstance)[rItem.targetName] = (( + draftInstance[rItem.fieldName] + )).id; + delete draftInstance[rItem.fieldName]; + } else { + (draftInstance)[rItem.fieldName] = (( + draftInstance[rItem.fieldName] + )).id; + } } break; @@ -491,7 +503,7 @@ export const isPrivateMode = () => { }); }; -const randomBytes = function(nBytes: number): Buffer { +const randomBytes = (nBytes: number): Buffer => { return Buffer.from(new WordArray().random(nBytes).toString(), 'hex'); }; const prng = () => randomBytes(1).readUInt8(0) / 0xff;