From 84286be109d7f50eac83a9694e75b61500cc8a83 Mon Sep 17 00:00:00 2001 From: Manuel Iglesias <6154160+manueliglesias@users.noreply.github.com> Date: Tue, 21 Apr 2020 11:45:45 -0700 Subject: [PATCH] fix(@aws-amplify/datastore): Improve query and observe typings (#5468) * Handle indexeddb cursor when store is empty * Validate tests with tslint * Improve query and observe types and tests * Allow observing a model instance * Fix for observe() test case --- packages/datastore/__tests__/DataStore.ts | 204 +++++++++++++++++- .../datastore/__tests__/subscription.test.ts | 19 +- packages/datastore/package.json | 8 +- packages/datastore/src/datastore/datastore.ts | 56 +++-- .../src/storage/adapter/indexeddb.ts | 4 +- packages/datastore/src/types.ts | 2 +- 6 files changed, 254 insertions(+), 39 deletions(-) diff --git a/packages/datastore/__tests__/DataStore.ts b/packages/datastore/__tests__/DataStore.ts index b1cb7b1cf70..7fb8a4187f6 100644 --- a/packages/datastore/__tests__/DataStore.ts +++ b/packages/datastore/__tests__/DataStore.ts @@ -1,18 +1,20 @@ import 'fake-indexeddb/auto'; import uuidValidate from 'uuid-validate'; +import Observable from 'zen-observable-ts'; import { - initSchema as initSchemaType, DataStore as DataStoreType, + initSchema as initSchemaType, } from '../src/datastore/datastore'; +import { Predicates } from '../src/predicates'; +import { ExclusiveStorage as StorageType } from '../src/storage/storage'; import { ModelInit, MutableModel, + NonModelTypeConstructor, + PersistentModel, PersistentModelConstructor, Schema, - NonModelTypeConstructor, } from '../src/types'; -import { ExclusiveStorage as StorageType } from '../src/storage/storage'; -import Observable from 'zen-observable-ts'; let initSchema: typeof initSchemaType; let DataStore: typeof DataStoreType; @@ -34,6 +36,13 @@ beforeEach(() => { ({ initSchema, DataStore } = require('../src/datastore/datastore')); }); +const nameOf = (name: keyof T) => name; + +/** + * Does nothing intentionally, we care only about type checking + */ +const expectType: (param: T) => void = () => {}; + describe('DataStore tests', () => { describe('initSchema tests', () => { test('Model class is created', () => { @@ -43,8 +52,9 @@ describe('DataStore tests', () => { const { Model } = classes as { Model: PersistentModelConstructor }; - let property: keyof PersistentModelConstructor = 'copyOf'; - expect(Model).toHaveProperty(property); + expect(Model).toHaveProperty( + nameOf>('copyOf') + ); expect(typeof Model.copyOf).toBe('function'); }); @@ -79,7 +89,10 @@ describe('DataStore tests', () => { expect(model.id).toBeDefined(); - // local models use something like a uuid v1, see https://github.com/kelektiv/node-uuid/issues/75#issuecomment-483756623 + /** + * local models use something like a uuid v1 + * see https://github.com/kelektiv/node-uuid/issues/75#issuecomment-483756623 + */ expect( uuidValidate(model.id.replace(/^(.{4})-(.{4})-(.{8})/, '$3-$2-$1'), 1) ).toBe(true); @@ -100,8 +113,9 @@ describe('DataStore tests', () => { const { Metadata } = classes; - let property: keyof PersistentModelConstructor = 'copyOf'; - expect(Metadata).not.toHaveProperty(property); + expect(Metadata).not.toHaveProperty( + nameOf>('copyOf') + ); }); test('Non @model class can be instantiated', () => { @@ -268,6 +282,178 @@ describe('DataStore tests', () => { 'Object is not an instance of a valid model' ); }); + + describe('Type definitions', () => { + let Model: PersistentModelConstructor; + + beforeEach(() => { + let model: Model; + + jest.resetModules(); + jest.doMock('../src/storage/storage', () => { + const mock = jest.fn().mockImplementation(() => ({ + runExclusive: jest.fn(() => [model]), + query: jest.fn(() => [model]), + observe: jest.fn(() => Observable.from([])), + })); + + (mock).getNamespace = () => ({ models: {} }); + + return { ExclusiveStorage: mock }; + }); + ({ initSchema, DataStore } = require('../src/datastore/datastore')); + + const classes = initSchema(testSchema()); + + ({ Model } = classes as { Model: PersistentModelConstructor }); + + model = new Model({ + field1: 'Some value', + }); + }); + + describe('Query', () => { + test('all', async () => { + const allModels = await DataStore.query(Model); + expectType(allModels); + const [one] = allModels; + expect(one.field1).toBeDefined(); + expect(one).toBeInstanceOf(Model); + }); + test('one by id', async () => { + const oneModelById = await DataStore.query(Model, 'someid'); + expectType(oneModelById); + expect(oneModelById.field1).toBeDefined(); + expect(oneModelById).toBeInstanceOf(Model); + }); + test('with criteria', async () => { + const multiModelWithCriteria = await DataStore.query(Model, c => + c.field1('contains', 'something') + ); + expectType(multiModelWithCriteria); + const [one] = multiModelWithCriteria; + expect(one.field1).toBeDefined(); + expect(one).toBeInstanceOf(Model); + }); + test('with pagination', async () => { + const allModelsPaginated = await DataStore.query( + Model, + Predicates.ALL, + { page: 0, limit: 20 } + ); + expectType(allModelsPaginated); + const [one] = allModelsPaginated; + expect(one.field1).toBeDefined(); + expect(one).toBeInstanceOf(Model); + }); + }); + + describe('Query with generic type', () => { + test('all', async () => { + const allModels = await DataStore.query(Model); + expectType(allModels); + const [one] = allModels; + expect(one.field1).toBeDefined(); + expect(one).toBeInstanceOf(Model); + }); + test('one by id', async () => { + const oneModelById = await DataStore.query(Model, 'someid'); + expectType(oneModelById); + expect(oneModelById.field1).toBeDefined(); + expect(oneModelById).toBeInstanceOf(Model); + }); + test('with criteria', async () => { + const multiModelWithCriteria = await DataStore.query(Model, c => + c.field1('contains', 'something') + ); + expectType(multiModelWithCriteria); + const [one] = multiModelWithCriteria; + expect(one.field1).toBeDefined(); + expect(one).toBeInstanceOf(Model); + }); + test('with pagination', async () => { + const allModelsPaginated = await DataStore.query( + Model, + Predicates.ALL, + { page: 0, limit: 20 } + ); + expectType(allModelsPaginated); + const [one] = allModelsPaginated; + expect(one.field1).toBeDefined(); + expect(one).toBeInstanceOf(Model); + }); + }); + + describe('Observe', () => { + test('subscribe to all models', async () => { + DataStore.observe().subscribe(({ element, model }) => { + expectType>(model); + expectType(element); + }); + }); + test('subscribe to model instance', async () => { + const model = new Model({ field1: 'somevalue' }); + + DataStore.observe(model).subscribe(({ element, model }) => { + expectType>(model); + expectType(element); + }); + }); + test('subscribe to model', async () => { + DataStore.observe(Model).subscribe(({ element, model }) => { + expectType>(model); + expectType(element); + }); + }); + test('subscribe to model instance by id', async () => { + DataStore.observe(Model, 'some id').subscribe(({ element, model }) => { + expectType>(model); + expectType(element); + }); + }); + test('subscribe to model with criteria', async () => { + DataStore.observe(Model, c => c.field1('ne', 'somevalue')).subscribe( + ({ element, model }) => { + expectType>(model); + expectType(element); + } + ); + }); + }); + + describe('Observe with generic type', () => { + test('subscribe to model instance', async () => { + const model = new Model({ field1: 'somevalue' }); + + DataStore.observe(model).subscribe(({ element, model }) => { + expectType>(model); + expectType(element); + }); + }); + test('subscribe to model', async () => { + DataStore.observe(Model).subscribe(({ element, model }) => { + expectType>(model); + expectType(element); + }); + }); + test('subscribe to model instance by id', async () => { + DataStore.observe(Model, 'some id').subscribe( + ({ element, model }) => { + expectType>(model); + expectType(element); + } + ); + }); + test('subscribe to model with criteria', async () => { + DataStore.observe(Model, c => + c.field1('ne', 'somevalue') + ).subscribe(({ element, model }) => { + expectType>(model); + expectType(element); + }); + }); + }); + }); }); //#region Test helpers diff --git a/packages/datastore/__tests__/subscription.test.ts b/packages/datastore/__tests__/subscription.test.ts index 39b5f0a7d2a..d29915c2dcd 100644 --- a/packages/datastore/__tests__/subscription.test.ts +++ b/packages/datastore/__tests__/subscription.test.ts @@ -3,10 +3,11 @@ import { USER_CREDENTIALS, } from '../src/sync/processors/subscription'; import { TransformerMutationType } from '../src/sync/utils'; +import { SchemaModel } from '../src/types'; describe('sync engine subscription module', () => { test('owner authorization', () => { - const model = { + const model: SchemaModel = { syncable: true, name: 'Post', pluralName: 'Posts', @@ -75,6 +76,7 @@ describe('sync engine subscription module', () => { }; expect( + // @ts-ignore SubscriptionProcessor.prototype.getAuthorizationInfo( model, TransformerMutationType.CREATE, @@ -84,7 +86,7 @@ describe('sync engine subscription module', () => { ).toEqual(authInfo); }); test('group authorization', () => { - const model = { + const model: SchemaModel = { syncable: true, name: 'Post', pluralName: 'Posts', @@ -152,6 +154,7 @@ describe('sync engine subscription module', () => { }; expect( + // @ts-ignore SubscriptionProcessor.prototype.getAuthorizationInfo( model, TransformerMutationType.CREATE, @@ -161,7 +164,7 @@ describe('sync engine subscription module', () => { ).toEqual(authInfo); }); test('public iam authorization for unauth user', () => { - const model = { + const model: SchemaModel = { syncable: true, name: 'Post', pluralName: 'Posts', @@ -210,6 +213,7 @@ describe('sync engine subscription module', () => { }; expect( + // @ts-ignore SubscriptionProcessor.prototype.getAuthorizationInfo( model, TransformerMutationType.CREATE, @@ -218,7 +222,7 @@ describe('sync engine subscription module', () => { ).toEqual(authInfo); }); test('private iam authorization for unauth user', () => { - const model = { + const model: SchemaModel = { syncable: true, name: 'Post', pluralName: 'Posts', @@ -267,6 +271,7 @@ describe('sync engine subscription module', () => { }; expect( + // @ts-ignore SubscriptionProcessor.prototype.getAuthorizationInfo( model, TransformerMutationType.CREATE, @@ -275,7 +280,7 @@ describe('sync engine subscription module', () => { ).toEqual(null); }); test('private iam authorization for auth user', () => { - const model = { + const model: SchemaModel = { syncable: true, name: 'Post', pluralName: 'Posts', @@ -324,6 +329,7 @@ describe('sync engine subscription module', () => { }; expect( + // @ts-ignore SubscriptionProcessor.prototype.getAuthorizationInfo( model, TransformerMutationType.CREATE, @@ -332,7 +338,7 @@ describe('sync engine subscription module', () => { ).toEqual(authInfo); }); test('public apiKey authorization without credentials', () => { - const model = { + const model: SchemaModel = { syncable: true, name: 'Post', pluralName: 'Posts', @@ -381,6 +387,7 @@ describe('sync engine subscription module', () => { }; expect( + // @ts-ignore SubscriptionProcessor.prototype.getAuthorizationInfo( model, TransformerMutationType.CREATE, diff --git a/packages/datastore/package.json b/packages/datastore/package.json index ac0269eb69b..7d632b49e40 100644 --- a/packages/datastore/package.json +++ b/packages/datastore/package.json @@ -13,7 +13,7 @@ }, "sideEffects": false, "scripts": { - "test": "tslint 'src/**/*.ts' && jest -w 1 --coverage", + "test": "npm run lint && jest -w 1 --coverage", "build-with-test": "npm test && npm run build", "build:cjs": "node ./build es5 && webpack && webpack --config ./webpack.config.dev.js", "build:esm": "node ./build es6", @@ -22,7 +22,7 @@ "build": "npm run clean && npm run build:esm && npm run build:cjs", "clean": "rimraf lib-esm lib dist", "format": "echo \"Not implemented\"", - "lint": "tslint 'src/**/*.ts'" + "lint": "tslint '{__tests__,src}/**/*.ts'" }, "repository": { "type": "git", @@ -51,7 +51,7 @@ "jest": { "globals": { "ts-jest": { - "diagnostics": false, + "diagnostics": true, "tsConfig": { "lib": [ "es5", @@ -94,4 +94,4 @@ "/node_modules/" ] } -} +} \ No newline at end of file diff --git a/packages/datastore/src/datastore/datastore.ts b/packages/datastore/src/datastore/datastore.ts index faa7a9a4d74..3fd5777de1c 100644 --- a/packages/datastore/src/datastore/datastore.ts +++ b/packages/datastore/src/datastore/datastore.ts @@ -488,27 +488,49 @@ const remove: { return deleted; } }; - const observe: { - (): Observable>; - (obj: T): Observable>; - ( - modelConstructor: PersistentModelConstructor, - id: string - ): Observable>; - ( - modelConstructor: PersistentModelConstructor - ): Observable>; + (): Observable>; + + (model: T): Observable>; + ( modelConstructor: PersistentModelConstructor, - criteria: ProducerModelPredicate + criteria?: string | ProducerModelPredicate ): Observable>; -} = ( - modelConstructor?: PersistentModelConstructor, +} = ( + modelOrConstructor?: T | PersistentModelConstructor, idOrCriteria?: string | ProducerModelPredicate -) => { +): Observable> => { let predicate: ModelPredicate; + const modelConstructor: PersistentModelConstructor = + modelOrConstructor && isValidModelConstructor(modelOrConstructor) + ? modelOrConstructor + : undefined; + + if (modelOrConstructor && modelConstructor === undefined) { + const model = modelOrConstructor; + const modelConstructor = + model && (Object.getPrototypeOf(model)).constructor; + + if (isValidModelConstructor(modelConstructor)) { + if (idOrCriteria) { + logger.warn('idOrCriteria is ignored when using a model instance', { + model, + idOrCriteria, + }); + } + + return observe(modelConstructor, model.id); + } else { + const msg = + 'The model is not an instance of a PersistentModelConstructor'; + logger.error(msg, { model }); + + throw new Error(msg); + } + } + if (idOrCriteria !== undefined && modelConstructor === undefined) { const msg = 'Cannot provide criteria without a modelConstructor'; logger.error(msg, idOrCriteria); @@ -530,13 +552,13 @@ const observe: { } else { predicate = modelConstructor && - ModelPredicateCreator.createFromExisting( + ModelPredicateCreator.createFromExisting( getModelDefinition(modelConstructor), idOrCriteria ); } - return new Observable>(observer => { + return new Observable>(observer => { let handle: ZenObservable.Subscription; (async () => { @@ -570,7 +592,7 @@ const query: { modelConstructor: PersistentModelConstructor, idOrCriteria?: string | ProducerModelPredicate | typeof PredicateAll, pagination?: PaginationInput -) => { +): Promise => { await start(); if (!isValidModelConstructor(modelConstructor)) { const msg = 'Constructor is not for a valid model'; diff --git a/packages/datastore/src/storage/adapter/indexeddb.ts b/packages/datastore/src/storage/adapter/indexeddb.ts index e420a8fd566..d525e51efd0 100644 --- a/packages/datastore/src/storage/adapter/indexeddb.ts +++ b/packages/datastore/src/storage/adapter/indexeddb.ts @@ -343,7 +343,7 @@ class IndexedDBAdapter implements Adapter { } private async enginePagination( - storeName, + storeName: string, pagination?: PaginationInput ): Promise { let result: T[]; @@ -366,7 +366,7 @@ class IndexedDBAdapter implements Adapter { const hasLimit = typeof limit === 'number' && limit > 0; let moreRecords = true; let itemsLeft = limit; - while (moreRecords && cursor.value) { + while (moreRecords && cursor && cursor.value) { pageResults.push(cursor.value); cursor = await cursor.continue(); diff --git a/packages/datastore/src/types.ts b/packages/datastore/src/types.ts index 391b0000ae4..9f0a9b8e0bb 100644 --- a/packages/datastore/src/types.ts +++ b/packages/datastore/src/types.ts @@ -169,7 +169,7 @@ export type NonModelTypeConstructor = { }; export type PersistentModelConstructor = { new (init: ModelInit): T; - copyOf(src: T, mutator: (draft: MutableModel) => T | void): T; + copyOf(src: T, mutator: (draft: MutableModel) => void): T; }; export type TypeConstructorMap = Record< string,