From ff91400334ec213a2680739288f2a369ee289778 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Sat, 28 Dec 2019 13:23:20 +0000 Subject: [PATCH 1/3] Use identifiers in peek methods --- addon/-private/cache.ts | 64 +++++++++++++++++++++++++++++++++-------- addon/-private/model.ts | 2 +- addon/-private/store.ts | 13 +++++++-- 3 files changed, 64 insertions(+), 15 deletions(-) diff --git a/addon/-private/cache.ts b/addon/-private/cache.ts index c8aa09c6..2e6a386f 100644 --- a/addon/-private/cache.ts +++ b/addon/-private/cache.ts @@ -68,17 +68,42 @@ export default class Cache { */ retrieveRecordData(type: string, id: string): Record | undefined { deprecate( - '`Cache#retrieveRecordData(type, id)` is deprecated, use `Cache#peekRecordData(type, id)`.' + '`Cache#retrieveRecordData(type, id)` is deprecated, use `Cache#peekRecordData({ type, id })`.' ); - return this.peekRecordData(type, id); + return this.peekRecordData({ type, id }); } - peekRecordData(type: string, id: string): Record | undefined { - return this._sourceCache.getRecordSync({ type, id }); + peekRecordData( + identifier: RecordIdentity | string, + id?: string + ): Record | undefined { + if (typeof identifier === 'string' && id) { + deprecate( + '`Cache#peekRecordData(type, id)` is deprecated, use `Cache#peekRecordData({ type, id })`.' + ); + identifier = { type: identifier, id }; + } + if (typeof identifier !== 'object') { + throw new TypeError( + 'peekRecordData should be called with an identifier.' + ); + } + return this._sourceCache.getRecordSync(identifier); } - includesRecord(type: string, id: string): boolean { - return !!this.peekRecordData(type, id); + includesRecord(identifier: RecordIdentity | string, id?: string): boolean { + if (typeof identifier === 'string' && id) { + deprecate( + '`Cache#includesRecord(type, id)` is deprecated, use `Cache#includesRecord({ type, id })`.' + ); + identifier = { type: identifier, id }; + } + if (typeof identifier !== 'object') { + throw new TypeError( + 'includesRecord should be called with an identifier.' + ); + } + return !!this.peekRecordData(identifier); } /** @@ -86,14 +111,26 @@ export default class Cache { */ retrieveRecord(type: string, id: string): Model | undefined { deprecate( - '`Cache#retrieveRecord(type, id)` is deprecated, use `Cache#peekRecord(type, id)`.' + '`Cache#retrieveRecord(type, id)` is deprecated, use `Cache#peekRecord({ type, id })`.' ); - return this.peekRecord(type, id); + return this.peekRecord({ type, id }); } - peekRecord(type: string, id: string): Model | undefined { - if (this.includesRecord(type, id)) { - return this.lookup({ type, id }) as Model; + peekRecord( + identifier: RecordIdentity | string, + id?: string + ): Model | undefined { + if (typeof identifier === 'string' && id) { + deprecate( + '`Cache#peekRecord(type, id)` is deprecated, use `Cache#peekRecord({ type, id })`.' + ); + identifier = { type: identifier, id }; + } + if (typeof identifier !== 'object') { + throw new TypeError('peekRecord should be called with an identifier.'); + } + if (this.includesRecord(identifier)) { + return this.lookup(identifier) as Model; } return undefined; } @@ -108,7 +145,10 @@ export default class Cache { keyName: string, keyValue: string ): Model | undefined { - return this.peekRecord(type, this.recordIdFromKey(type, keyName, keyValue)); + return this.peekRecord({ + type, + id: this.recordIdFromKey(type, keyName, keyValue) + }); } recordIdFromKey(type: string, keyName: string, keyValue: string): string { diff --git a/addon/-private/model.ts b/addon/-private/model.ts index f4d82815..d2cdb4f7 100644 --- a/addon/-private/model.ts +++ b/addon/-private/model.ts @@ -38,7 +38,7 @@ export default class Model extends EmberObject { } getData(): Record | undefined { - return this.store.cache.peekRecordData(this.type, this.id); + return this.store.cache.peekRecordData({ type: this.type, id: this.id }); } getKey(field: string): string | undefined { diff --git a/addon/-private/store.ts b/addon/-private/store.ts index 3c0d5c9a..b6b1899a 100644 --- a/addon/-private/store.ts +++ b/addon/-private/store.ts @@ -184,8 +184,17 @@ export default class Store { ); } - peekRecord(type: string, id: string): Model | undefined { - return this.cache.peekRecord(type, id); + peekRecord( + identifier: RecordIdentity | string, + id?: string + ): Model | undefined { + if (typeof identifier === 'string' && id) { + deprecate( + '`Store#peekRecord(type, id)` is deprecated, use `Store#peekRecord({ type, id })`.' + ); + identifier = { type: identifier, id }; + } + return this.cache.peekRecord(identifier); } peekRecords(type: string): Model[] { From 4d06bec86aa8b11b6adc1fdc202b81c1f7ad7b1a Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Sat, 28 Dec 2019 13:25:02 +0000 Subject: [PATCH 2/3] Add chainable query builder interfaces --- addon/-private/cache.ts | 32 ++++- addon/-private/query-builders.ts | 198 +++++++++++++++++++++++++++++++ addon/-private/store.ts | 111 +++++++++++++++-- 3 files changed, 326 insertions(+), 15 deletions(-) create mode 100644 addon/-private/query-builders.ts diff --git a/addon/-private/cache.ts b/addon/-private/cache.ts index 2e6a386f..288a76da 100644 --- a/addon/-private/cache.ts +++ b/addon/-private/cache.ts @@ -293,26 +293,50 @@ export default class Cache { return liveQuery; } + /** + * @deprecated + */ find(type: string, id?: string): Model | Model[] { if (id === undefined) { - return this.findRecords(type); + deprecate( + '`Cache#find(type)` is deprecated, use `Store#findRecords(type).peek()`.' + ); + return this.query(q => q.findRecords(type)) as Model[]; } else { - return this.findRecord(type, id); + deprecate( + '`Cache#find(type, id)` is deprecated, use `Store#findRecord({ type, id }).peek()`.' + ); + return this.query(q => q.findRecord({ type, id })) as Model[]; } } + /** + * @deprecated + */ findAll(type: string, options?: object): Model[] { deprecate( - '`Cache.findAll(type)` is deprecated, use `Cache.findRecords(type)`.' + '`Cache#findAll(type)` is deprecated, use `Store#findRecords(type).peek()`.' ); - return this.findRecords(type, options); + return this.query(q => q.findRecords(type), options) as Model[]; } + /** + * @deprecated + */ findRecord(type: string, id: string, options?: object): Model { + deprecate( + '`Cache#findRecord(type, id)` is deprecated, use `Store#findRecord({ type, id }).peek()`.' + ); return this.query(q => q.findRecord({ type, id }), options) as Model; } + /** + * @deprecated + */ findRecords(type: string, options?: object): Model[] { + deprecate( + '`Cache#findRecords(type)` is deprecated, use `Store#findRecords(type).peek()`.' + ); return this.query(q => q.findRecords(type), options) as Model[]; } diff --git a/addon/-private/query-builders.ts b/addon/-private/query-builders.ts new file mode 100644 index 00000000..a326430f --- /dev/null +++ b/addon/-private/query-builders.ts @@ -0,0 +1,198 @@ +import { deepMerge } from '@orbit/utils'; +import { + RecordIdentity, + FindRecordTerm, + FindRecordsTerm, + FindRelatedRecordTerm, + FindRelatedRecordsTerm +} from '@orbit/data'; + +import Store from './store'; +import Model from './model'; + +export class FindRecordQueryBuilder extends FindRecordTerm { + store: Store; + _options = {}; + + constructor(store: Store, record: RecordIdentity) { + super(record); + this.store = store; + } + + options(options: object) { + deepMerge(this._options, options); + return this; + } + + then(...args: any[]): Promise { + return this._promise.then(...args); + } + + catch(cb: any) { + return this._promise.catch(cb); + } + + finally(cb: any) { + return this._promise.finally(cb); + } + + peek(): Model | undefined { + return this.store.cache.query( + this.toQueryExpression(), + this._options + ) as Model; + } + + private get _promise() { + return this.store.query(this.toQueryExpression(), this._options); + } +} + +export class FindRecordsQueryBuilder extends FindRecordsTerm { + store: Store; + _options = {}; + _live = false; + + constructor(store: Store, typeOrIdentities?: string | RecordIdentity[]) { + super(typeOrIdentities); + this.store = store; + } + + options(options: object) { + deepMerge(this._options, options); + return this; + } + + live() { + this._live = true; + return this; + } + + then(...args: any[]): Promise { + return this._promise.then(...args); + } + + catch(cb: any) { + return this._promise.catch(cb); + } + + finally(cb: any) { + return this._promise.finally(cb); + } + + peek(): Model[] | any { + if (this._live) { + return this.store.cache.liveQuery( + this.toQueryExpression(), + this._options + ); + } + return this.store.cache.query(this.toQueryExpression(), this._options); + } + + private get _promise() { + return this.store + .query(this.toQueryExpression(), this._options) + .then(result => { + if (this._live) { + return this.peek(); + } + return result; + }); + } +} + +export class FindRelatedRecordQueryBuilder extends FindRelatedRecordTerm { + store: Store; + _options = {}; + + constructor(store: Store, record: RecordIdentity, relationship: string) { + super(record, relationship); + this.store = store; + } + + options(options: object) { + deepMerge(this._options, options); + return this; + } + + then(...args: any[]): Promise { + return this._promise.then(...args); + } + + catch(cb: any) { + return this._promise.catch(cb); + } + + finally(cb: any) { + return this._promise.finally(cb); + } + + peek(): Model | null { + return this.store.cache.query( + this.toQueryExpression(), + this._options + ) as Model | null; + } + + private get _promise() { + return this.store.query(this.toQueryExpression(), this._options); + } +} + +export class FindRelatedRecordsQueryBuilder extends FindRelatedRecordsTerm { + store: Store; + _options = {}; + _live = false; + + constructor(store: Store, record: RecordIdentity, relationship: string) { + super(record, relationship); + this.store = store; + } + + options(options: object) { + deepMerge(this._options, options); + return this; + } + + live() { + this._live = true; + return this; + } + + then(...args: any[]): Promise { + return this._promise.then(...args); + } + + catch(cb: any) { + return this._promise.catch(cb); + } + + finally(cb: any) { + return this._promise.finally(cb); + } + + peek(): Model[] | any { + if (this._live) { + return this.store.cache.liveQuery( + this.toQueryExpression(), + this._options + ); + } + return this.store.cache.query( + this.toQueryExpression(), + this._options + ) as Model[]; + } + + private get _promise() { + return this.store + .query(this.toQueryExpression(), this._options) + .then(result => { + if (this._live) { + return this.peek(); + } + return result; + }); + } +} diff --git a/addon/-private/store.ts b/addon/-private/store.ts index b6b1899a..e081ee97 100644 --- a/addon/-private/store.ts +++ b/addon/-private/store.ts @@ -18,6 +18,12 @@ import Cache from './cache'; import Model from './model'; import ModelFactory from './model-factory'; import normalizeRecordProperties from './utils/normalize-record-properties'; +import { + FindRecordQueryBuilder, + FindRecordsQueryBuilder, + FindRelatedRecordQueryBuilder, + FindRelatedRecordsQueryBuilder +} from './query-builders'; const { deprecate } = Orbit; @@ -102,11 +108,17 @@ export default class Store { this.source.rebase(); } + /** + * @deprecated + */ liveQuery( queryOrExpression: QueryOrExpression, options?: object, id?: string ): Promise { + deprecate( + '`Store#liveQuery(type)` is deprecated, use `Store#findRecords(type).live()`.' + ); const query = buildQuery( queryOrExpression, options, @@ -148,27 +160,102 @@ export default class Store { await this.update(t => t.removeRecord(identity), options); } + /** + * @deprecated + */ findAll(type: string, options?: object): Promise { deprecate( - '`Store.findAll(type)` is deprecated, use `Store.findRecords(type)`.' + '`Store#findAll(type)` is deprecated, use `Store#findRecords(type)`.' ); - return this.findRecords(type, options); + return this.findRecords(type, options).then(result => result); } + /** + * @deprecated + */ find(type: string, id?: string | undefined): Promise { if (id === undefined) { - return this.findRecords(type); + deprecate( + '`Store#find(type)` is deprecated, use `Store#findRecords(type)`.' + ); + return this.findRecords(type).then(result => result); } else { - return this.findRecord(type, id); + deprecate( + '`Store#find(type, id)` is deprecated, use `Store#findRecord({ type, id })`.' + ); + return this.findRecord(type, id).then(result => result); + } + } + + findRecord( + identifier: RecordIdentity | string, + id?: string | object, + options?: object + ): FindRecordQueryBuilder { + if (typeof identifier === 'string' && typeof id === 'string') { + deprecate( + '`Store#findRecord(type, id)` is deprecated, use `Store#findRecord({ type, id })`.' + ); + identifier = { type: identifier, id }; + } else { + options = id as object; + } + if (typeof identifier !== 'object') { + throw new TypeError('findRecord should be called with an identifier.'); } + + const queryBuilder = new FindRecordQueryBuilder(this, identifier); + + if (options) { + return queryBuilder.options(options); + } + return queryBuilder; } - findRecord(type: string, id: string, options?: object): Promise { - return this.query(q => q.findRecord({ type, id }), options); + findRecords( + type: string | RecordIdentity[], + options?: object + ): FindRecordsQueryBuilder { + const queryBuilder = new FindRecordsQueryBuilder(this, type); + + if (options) { + return queryBuilder.options(options); + } + return queryBuilder; } - findRecords(type: string, options?: object): Promise { - return this.query(q => q.findRecords(type), options); + findRelatedRecord( + identifier: RecordIdentity, + relationship: string, + options?: object + ): FindRelatedRecordQueryBuilder { + const queryBuilder = new FindRelatedRecordQueryBuilder( + this, + identifier, + relationship + ); + + if (options) { + return queryBuilder.options(options); + } + return queryBuilder; + } + + findRelatedRecords( + identifier: RecordIdentity, + relationship: string, + options?: object + ): FindRelatedRecordsQueryBuilder { + const queryBuilder = new FindRelatedRecordsQueryBuilder( + this, + identifier, + relationship + ); + + if (options) { + return queryBuilder.options(options); + } + return queryBuilder; } findRecordByKey( @@ -178,10 +265,12 @@ export default class Store { options?: object ): Promise { return this.findRecord( - type, - this.cache.recordIdFromKey(type, keyName, keyValue), + { + type, + id: this.cache.recordIdFromKey(type, keyName, keyValue) + }, options - ); + ).then(result => result); } peekRecord( From 2e886d6d37d3d04b53bbfa129b64a4ba6dfffef7 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Sun, 5 Jan 2020 18:34:23 +0100 Subject: [PATCH 3/3] Fix deprecations in tests --- tests/integration/cache-test.js | 49 +++++++++------------------------ tests/integration/model-test.js | 2 +- tests/integration/store-test.js | 45 ++++++++++++++++-------------- 3 files changed, 38 insertions(+), 58 deletions(-) diff --git a/tests/integration/cache-test.js b/tests/integration/cache-test.js index e934a8be..4932def7 100644 --- a/tests/integration/cache-test.js +++ b/tests/integration/cache-test.js @@ -95,14 +95,17 @@ module('Integration - Cache', function(hooks) { test('#peekRecord - existing record', async function(assert) { const jupiter = await store.addRecord({ type: 'planet', name: 'Jupiter' }); assert.strictEqual( - cache.peekRecord('planet', jupiter.id), + cache.peekRecord({ type: 'planet', id: jupiter.id }), jupiter, 'retrieved record' ); }); test('#peekRecord - missing record', async function(assert) { - assert.strictEqual(cache.peekRecord('planet', 'fake'), undefined); + assert.strictEqual( + cache.peekRecord({ type: 'planet', id: 'fake' }), + undefined + ); }); test('#peekRecordByKey - existing record', async function(assert) { @@ -249,7 +252,10 @@ module('Integration - Cache', function(hooks) { test('#peekRecordData - existing record', async function(assert) { const jupiter = await store.addRecord({ type: 'planet', name: 'Jupiter' }); - const retrievedRecordData = cache.peekRecordData('planet', jupiter.id); + const retrievedRecordData = cache.peekRecordData({ + type: 'planet', + id: jupiter.id + }); assert.ok(retrievedRecordData, 'retrieved record data'); assert.equal( retrievedRecordData.attributes.name, @@ -259,7 +265,10 @@ module('Integration - Cache', function(hooks) { }); test('peekRecordData - missing record', async function(assert) { - assert.strictEqual(cache.peekRecordData('planet', 'fake'), undefined); + assert.strictEqual( + cache.peekRecordData({ type: 'planet', id: 'fake' }), + undefined + ); }); test('#recordIdFromKey - retrieves a record id based on a known key', async function(assert) { @@ -311,36 +320,4 @@ module('Integration - Cache', function(hooks) { assert.deepEqual(foundRecords, [earth]); assert.strictEqual(foundRecords[0], earth); }); - - test('#find - by type and id', async function(assert) { - const earth = await store.addRecord({ type: 'planet', name: 'Earth' }); - const foundRecord = cache.find('planet', earth.id); - assert.strictEqual(foundRecord, earth, 'exact match'); - }); - - test('#find - by type', async function(assert) { - const earth = await store.addRecord({ type: 'planet', name: 'Earth' }); - const jupiter = await store.addRecord({ type: 'planet', name: 'Jupiter' }); - - const foundRecords = cache.find('planet'); - assert.equal(foundRecords.length, 2, 'two records found'); - assert.ok(foundRecords.includes(earth), 'earth is included'); - assert.ok(foundRecords.includes(jupiter), 'jupiter is included'); - }); - - test('#findRecord', async function(assert) { - const earth = await store.addRecord({ type: 'planet', name: 'Earth' }); - const foundRecord = cache.findRecord('planet', earth.id); - assert.strictEqual(foundRecord, earth, 'exact match'); - }); - - test('#findRecords', async function(assert) { - const earth = await store.addRecord({ type: 'planet', name: 'Earth' }); - const jupiter = await store.addRecord({ type: 'planet', name: 'Jupiter' }); - - const foundRecords = cache.findRecords('planet'); - assert.equal(foundRecords.length, 2, 'two records found'); - assert.ok(foundRecords.includes(earth), 'earth is included'); - assert.ok(foundRecords.includes(jupiter), 'jupiter is included'); - }); }); diff --git a/tests/integration/model-test.js b/tests/integration/model-test.js index 69a2c6b9..ef559914 100644 --- a/tests/integration/model-test.js +++ b/tests/integration/model-test.js @@ -70,7 +70,7 @@ module('Integration - Model', function(hooks) { await record.remove(); assert.notOk( - cache.includesRecord('star', record.id), + cache.includesRecord({ type: 'star', id: record.id }), 'record does not exist in cache' ); assert.ok(record.disconnected, 'record has been disconnected from store'); diff --git a/tests/integration/store-test.js b/tests/integration/store-test.js index 789e8030..8f14d686 100644 --- a/tests/integration/store-test.js +++ b/tests/integration/store-test.js @@ -45,13 +45,13 @@ module('Integration - Store', function(hooks) { test('#findRecord', async function(assert) { const earth = await store.addRecord({ type: 'planet', name: 'Earth' }); - const planet = await store.findRecord('planet', earth.id); + const planet = await store.findRecord({ type: 'planet', id: earth.id }); assert.strictEqual(planet, earth); }); test('#findRecord - missing record', async function(assert) { try { - await store.findRecord('planet', 'jupiter'); + await store.findRecord({ type: 'planet', id: 'jupiter' }); } catch (e) { assert.equal(e.message, 'Record not found: planet:jupiter'); } @@ -87,14 +87,17 @@ module('Integration - Store', function(hooks) { test('#peekRecord - existing record', async function(assert) { const jupiter = await store.addRecord({ type: 'planet', name: 'Jupiter' }); assert.strictEqual( - store.peekRecord('planet', jupiter.id), + store.peekRecord({ type: 'planet', id: jupiter.id }), jupiter, 'retrieved record' ); }); test('#peekRecord - missing record', async function(assert) { - assert.strictEqual(store.peekRecord('planet', 'fake'), undefined); + assert.strictEqual( + store.peekRecord({ type: 'planet', id: 'fake' }), + undefined + ); }); test('#peekRecordByKey - existing record', async function(assert) { @@ -159,7 +162,7 @@ module('Integration - Store', function(hooks) { await store.removeRecord(record); try { - await store.findRecord('planet', record.id); + await store.findRecord({ type: 'planet', id: record.id }); } catch (error) { assert.ok(error.message.match(/Record not found/)); } @@ -186,7 +189,7 @@ module('Integration - Store', function(hooks) { await store.removeRecord({ type: 'planet', id: record.id }); try { - await store.findRecord('planet', record.id); + await store.findRecord({ type: 'planet', id: record.id }); } catch (error) { assert.ok(error.message.match(/Record not found/)); } @@ -398,27 +401,27 @@ module('Integration - Store', function(hooks) { assert.equal(liveQuery.length, 1); }); - test('#find - by type', async function(assert) { + test('#findRecords().peek()', async function(assert) { let earth = await store.addRecord({ type: 'planet', name: 'Earth' }); let jupiter = await store.addRecord({ type: 'planet', name: 'Jupiter' }); - let records = await store.find('planet'); + let records = store.findRecords('planet').peek(); assert.equal(records.length, 2); assert.ok(records.includes(earth)); assert.ok(records.includes(jupiter)); }); - test('#find - by type and id', async function(assert) { + test('#findRecord().peek()', async function(assert) { const earth = await store.addRecord({ type: 'planet', name: 'Earth' }); await store.addRecord({ type: 'planet', name: 'Jupiter' }); - const record = await store.find('planet', earth.id); + const record = store.findRecord({ type: 'planet', id: earth.id }).peek(); assert.strictEqual(record, earth); }); - test('#find - missing record', async function(assert) { + test('#findRecord().peek() - missing record', function(assert) { try { - await store.find('planet', 'jupiter'); + store.findRecord({ type: 'planet', id: 'jupiter' }).peek(); } catch (e) { assert.equal( e.message, @@ -437,11 +440,11 @@ module('Integration - Store', function(hooks) { }); assert.notOk( - store.cache.includesRecord('planet', jupiter.id), + store.cache.includesRecord({ type: 'planet', id: jupiter.id }), 'store does not contain record' ); assert.ok( - forkedStore.cache.includesRecord('planet', jupiter.id), + forkedStore.cache.includesRecord({ type: 'planet', id: jupiter.id }), 'fork includes record' ); }); @@ -456,11 +459,11 @@ module('Integration - Store', function(hooks) { await store.merge(forkedStore); assert.ok( - store.cache.includesRecord('planet', jupiter.id), + store.cache.includesRecord({ type: 'planet', id: jupiter.id }), 'store includes record' ); assert.ok( - forkedStore.cache.includesRecord('planet', jupiter.id), + forkedStore.cache.includesRecord({ type: 'planet', id: jupiter.id }), 'fork includes record' ); }); @@ -515,10 +518,10 @@ module('Integration - Store', function(hooks) { assert.deepEqual(fork.allTransforms(), [addRecordD, addRecordE]); assert.deepEqual(fork.cache.peekRecords('planet').length, 5); - assert.ok(fork.cache.includesRecord(recordA.type, recordA.id)); - assert.ok(fork.cache.includesRecord(recordB.type, recordB.id)); - assert.ok(fork.cache.includesRecord(recordC.type, recordC.id)); - assert.ok(fork.cache.includesRecord(recordD.type, recordD.id)); - assert.ok(fork.cache.includesRecord(recordE.type, recordE.id)); + assert.ok(fork.cache.includesRecord(recordA)); + assert.ok(fork.cache.includesRecord(recordB)); + assert.ok(fork.cache.includesRecord(recordC)); + assert.ok(fork.cache.includesRecord(recordD)); + assert.ok(fork.cache.includesRecord(recordE)); }); });