feat(datastore): custom pk support

Co-authored-by: Ivan Artemiev <29709626+iartemiev@users.noreply.github.com>
Co-authored-by: David McAfee <mcafd@amazon.com>
Co-authored-by: Dane Pilcher <dppilche@amazon.com>
Co-authored-by: Jon Wire <iambipedal@gmail.com>

Author: 5 people

.circleci/config.yml

@@ -369,6 +369,7 @@ jobs:
           command: |
             cd packages/datastore-storage-adapter
             npm install --build-from-source
+            rm -rf node_modules/@aws-amplify node_modules/@aws-sdk
       - run:
           name: 'Run Amplify JS unit tests'
           command: |
@@ -1218,6 +1219,7 @@ releasable_branches: &releasable_branches
       - ui-components/main
       - 1.0-stable
       - geo/main
+      - ds-custom-pk
 
 test_browsers: &test_browsers
   browser: [chrome, firefox]

.envrc

@@ -0,0 +1 @@
+use node 14

.vscode/launch.json

@@ -0,0 +1,28 @@
+// A launch configuration that compiles the extension and then opens it inside a new window
+// Use IntelliSense to learn about possible attributes.
+// Hover to view descriptions of existing attributes.
+// For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
+{
+	"version": "0.2.0",
+    "configurations": [
+        {
+			"name": "debug tests",
+			"type": "node",
+			"request": "launch",
+			// The debugger will only run tests for the package specified here:
+			"cwd": "${workspaceFolder}/packages/datastore",
+			"runtimeArgs": [
+				"--inspect-brk",
+				"${workspaceRoot}/node_modules/.bin/jest",
+				// Optionally specify a single test file to run/debug:
+				"storage.test.ts",
+				"--runInBand",
+				"--testTimeout",
+				"600000", // 10 min timeout so jest doesn't error while we're stepping through code
+				"false"
+			],
+			"console": "integratedTerminal",
+			"internalConsoleOptions": "neverOpen"
+		}
+    ]
+}

package.json

@@ -27,7 +27,9 @@
 		"publish:1.0-stable": "lerna publish --conventional-commits --yes --dist-tag=stable-1.0 --message 'chore(release): Publish [ci skip]' --no-verify-access",
 		"publish:ui-components/main": "lerna publish --canary --force-publish \"*\" --yes --dist-tag=ui-preview --preid=ui-preview --exact --no-verify-access",
 		"publish:verdaccio": "lerna publish --no-push --canary minor --dist-tag=unstable --preid=unstable --exact --force-publish --yes --no-verify-access",
-		"publish:geo/main": "lerna publish --canary --force-publish \"*\" --yes --dist-tag=geo --preid=geo --exact --no-verify-access"
+		"publish:geo/main": "lerna publish --canary --force-publish \"*\" --yes --dist-tag=geo --preid=geo --exact --no-verify-access",
+		"publish:ds-custom-pk": "lerna publish --canary --force-publish \"*\" --yes --dist-tag=custom-pk --preid=custom-pk --exact --no-verify-access",
+		"temp-ds-safe-push": "yarn build --scope @aws-amplify/datastore && yarn test --scope @aws-amplify/datastore && git push origin"
 	},
 	"husky": {
 		"hooks": {

packages/datastore-storage-adapter/__tests__/SQLiteAdapter.test.ts

@@ -76,6 +76,10 @@ class InnerSQLiteDatabase {
 					statement,
 					params,
 					async (err, row) => {
+						if (err) {
+							console.error('SQLite ERROR', new Error(err));
+							console.warn(statement, params);
+						}
 						rows.push(row);
 					},
 					() => {
@@ -86,7 +90,14 @@ class InnerSQLiteDatabase {
 				if (callback) await callback(this, resultSet);
 			});
 		} else {
-			return await this.innerDB.run(statement, params, callback);
+			return await this.innerDB.run(statement, params, err => {
+				if (typeof callback === 'function') {
+					callback(err);
+				} else if (err) {
+					console.error('calback', err);
+					throw err;
+				}
+			});
 		}
 	}
 

packages/datastore-storage-adapter/package.json

@@ -51,7 +51,8 @@
 						"es5",
 						"es2015",
 						"esnext.asynciterable",
-						"es2019"
+						"es2019",
+						"dom"
 					],
 					"allowJs": true,
 					"esModuleInterop": true,

packages/datastore-storage-adapter/src/common/CommonSQLiteAdapter.ts

@@ -17,7 +17,6 @@ import {
 	ModelSortPredicateCreator,
 	InternalSchema,
 	isPredicateObj,
-	ModelInstanceMetadata,
 	ModelPredicate,
 	NamespaceResolver,
 	OpType,
@@ -29,7 +28,7 @@ import {
 	QueryOne,
 	utils,
 } from '@aws-amplify/datastore';
-import { CommonSQLiteDatabase, ParameterizedStatement } from './types';
+import { CommonSQLiteDatabase, ParameterizedStatement, ModelInstanceMetadataWithId } from './types';
 
 const { traverseModel, validatePredicate, isModelConstructor } = utils;
 
@@ -407,7 +406,7 @@ export class CommonSQLiteAdapter implements StorageAdapter {
 
 	async batchSave<T extends PersistentModel>(
 		modelConstructor: PersistentModelConstructor<any>,
-		items: ModelInstanceMetadata[]
+		items: ModelInstanceMetadataWithId[]
 	): Promise<[T, OpType][]> {
 		const { name: tableName } = modelConstructor;
 		const result: [T, OpType][] = [];

packages/datastore-storage-adapter/src/common/SQLiteUtils.ts

@@ -148,7 +148,7 @@ export function modelCreateTableStatement(
 	let fields = Object.values(model.fields).reduce((acc, field: ModelField) => {
 		if (isGraphQLScalarType(field.type)) {
 			if (field.name === 'id') {
-				return acc + '"id" PRIMARY KEY NOT NULL';
+				return [...acc, '"id" PRIMARY KEY NOT NULL'];
 			}
 
 			let columnParam = `"${field.name}" ${getSQLiteType(field.type)}`;
@@ -157,7 +157,7 @@ export function modelCreateTableStatement(
 				columnParam += ' NOT NULL';
 			}
 
-			return acc + `, ${columnParam}`;
+			return [...acc, `${columnParam}`];
 		}
 
 		if (isModelFieldType(field.type)) {
@@ -167,7 +167,7 @@ export function modelCreateTableStatement(
 			if (isTargetNameAssociation(field.association)) {
 				// check if this field has been explicitly defined in the model
 				const fkDefinedInModel = Object.values(model.fields).find(
-					(f: ModelField) => f.name === field.association.targetName
+					(f: ModelField) => f.name === field?.association?.targetName
 				);
 
 				// if the FK is not explicitly defined in the model, we have to add it here
@@ -179,7 +179,7 @@ export function modelCreateTableStatement(
 
 			// ignore isRequired param for model fields, since they will not contain
 			// the related data locally
-			return acc + `, ${columnParam}`;
+			return [...acc, `${columnParam}`];
 		}
 
 		// default to TEXT
@@ -189,19 +189,25 @@ export function modelCreateTableStatement(
 			columnParam += ' NOT NULL';
 		}
 
-		return acc + `, ${columnParam}`;
-	}, '');
+		return [...acc, `${columnParam}`];
+	}, [] as string[]);
 
 	implicitAuthFields.forEach((authField: string) => {
-		fields += `, ${authField} TEXT`;
+		fields.push(`${authField} TEXT`);
 	});
 
 	if (userModel) {
-		fields +=
-			', "_version" INTEGER, "_lastChangedAt" INTEGER, "_deleted" INTEGER';
+		fields = [
+			...fields,
+			`"_version" INTEGER`,
+			`"_lastChangedAt" INTEGER`,
+			`"_deleted" INTEGER`,
+		];
 	}
 
-	const createTableStatement = `CREATE TABLE IF NOT EXISTS "${model.name}" (${fields});`;
+	const createTableStatement = `CREATE TABLE IF NOT EXISTS "${
+		model.name
+	}" (${fields.join(', ')});`;
 	return createTableStatement;
 }
 

packages/datastore-storage-adapter/src/common/types.ts

@@ -1,4 +1,4 @@
-import { PersistentModel } from '@aws-amplify/datastore';
+import { PersistentModel, ModelInstanceMetadata } from '@aws-amplify/datastore';
 
 export interface CommonSQLiteDatabase {
 	init(): Promise<void>;
@@ -27,3 +27,8 @@ export interface CommonSQLiteDatabase {
 }
 
 export type ParameterizedStatement = [string, any[]];
+
+// TODO: remove once we implement CPK for this adapter
+export type ModelInstanceMetadataWithId = ModelInstanceMetadata & {
+	id: string;
+};

packages/datastore/__tests__/AsyncStorage.ts

@@ -168,6 +168,7 @@ describe('AsyncStorage tests', () => {
 
 	test('save function 1:1 insert', async () => {
 		await DataStore.save(blog);
+
 		await DataStore.save(owner);
 
 		const get1 = JSON.parse(

packages/datastore/__tests__/AsyncStorageAdapter.test.ts

@@ -5,7 +5,15 @@ import {
 	syncClasses,
 } from '../src/datastore/datastore';
 import { PersistentModelConstructor, SortDirection } from '../src/types';
-import { pause, Model, User, Profile, testSchema } from './helpers';
+import {
+	Model,
+	User,
+	Profile,
+	Post,
+	Comment,
+	testSchema,
+	pause,
+} from './helpers';
 import { Predicates } from '../src/predicates';
 import { addCommonQueryTests } from './commonAdapterTests';
 
@@ -41,7 +49,7 @@ describe('AsyncStorageAdapter tests', () => {
 	describe('Query', () => {
 		let Model: PersistentModelConstructor<Model>;
 		let model1Id: string;
-		const spyOnGetOne = jest.spyOn(ASAdapter, 'getById');
+		const spyOnGetOne = jest.spyOn(ASAdapter, 'getByKey');
 		const spyOnGetAll = jest.spyOn(ASAdapter, 'getAll');
 		const spyOnMemory = jest.spyOn(ASAdapter, 'inMemoryPagination');
 
@@ -92,9 +100,8 @@ describe('AsyncStorageAdapter tests', () => {
 			await DataStore.clear();
 		});
 
-		it('Should call getById for query by id', async () => {
+		it('Should call getById for query by key', async () => {
 			const result = await DataStore.query(Model, model1Id);
-
 			expect(result.field1).toEqual('Some value');
 			expect(spyOnGetOne).toHaveBeenCalled();
 			expect(spyOnGetAll).not.toHaveBeenCalled();
@@ -155,11 +162,16 @@ describe('AsyncStorageAdapter tests', () => {
 			expect(spyOnMemory).not.toHaveBeenCalled();
 		});
 	});
+
 	describe('Delete', () => {
 		let User: PersistentModelConstructor<User>;
 		let Profile: PersistentModelConstructor<Profile>;
 		let profile1Id: string;
 		let user1Id: string;
+		let Post: PersistentModelConstructor<Post>;
+		let Comment: PersistentModelConstructor<Comment>;
+		let post1Id: string;
+		let comment1Id: string;
 
 		beforeAll(async () => {
 			({ initSchema, DataStore } = require('../src/datastore/datastore'));
@@ -183,6 +195,25 @@ describe('AsyncStorageAdapter tests', () => {
 			));
 		});
 
+		beforeEach(async () => {
+			const classes = initSchema(testSchema());
+
+			({ Post } = classes as {
+				Post: PersistentModelConstructor<Post>;
+			});
+
+			({ Comment } = classes as {
+				Comment: PersistentModelConstructor<Comment>;
+			});
+
+			const post = await DataStore.save(new Post({ title: 'Test' }));
+			({ id: post1Id } = post);
+
+			({ id: comment1Id } = await DataStore.save(
+				new Comment({ content: 'Test Content', post })
+			));
+		});
+
 		it('Should perform a cascading delete on a record with a Has One relationship', async () => {
 			let user = await DataStore.query(User, user1Id);
 			let profile = await DataStore.query(Profile, profile1Id);
@@ -197,8 +228,26 @@ describe('AsyncStorageAdapter tests', () => {
 			profile = await DataStore.query(Profile, profile1Id);
 
 			// both should be undefined, even though we only explicitly deleted the user
-			expect(user).toBeUndefined;
-			expect(profile).toBeUndefined;
+			expect(user).toBeUndefined();
+			expect(profile).toBeUndefined();
+		});
+
+		it('Should perform a cascading delete on a record with a Has Many relationship', async () => {
+			let post = await DataStore.query(Post, post1Id);
+			let comment = await DataStore.query(Comment, comment1Id);
+
+			// double-checking that both of the records exist at first
+			expect(post.id).toEqual(post1Id);
+			expect(comment.id).toEqual(comment1Id);
+
+			await DataStore.delete(Post, post.id);
+
+			post = await DataStore.query(Post, post1Id);
+			comment = await DataStore.query(Comment, comment1Id);
+
+			// both should be undefined, even though we only explicitly deleted the post
+			expect(post).toBeUndefined();
+			expect(comment).toBeUndefined();
 		});
 	});