From a94ae7fdab5c35f911eecfac25b24d3db30daf68 Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Thu, 19 Jan 2023 13:50:13 -0600 Subject: [PATCH 01/11] Add chainable select method --- src/serviceLoader.js | 139 +++++++++++++++++++++++++----------- src/utils.js | 10 +++ tests/serviceLoader.test.js | 11 +++ 3 files changed, 118 insertions(+), 42 deletions(-) diff --git a/src/serviceLoader.js b/src/serviceLoader.js index f140bc2..f4d45a6 100644 --- a/src/serviceLoader.js +++ b/src/serviceLoader.js @@ -4,6 +4,7 @@ const { stableStringify, defaultCacheParamsFn, defaultCacheKeyFn, + defaultSelectFn, uniqueKeys, uniqueResults, uniqueResultsMulti @@ -38,6 +39,7 @@ module.exports = class ServiceLoader { app, serviceName, cacheParamsFn, + selectFn, cacheMap, ...loaderOptions }) { @@ -49,6 +51,7 @@ module.exports = class ServiceLoader { serviceName, service, key: service.options.id, + selectFn: selectFn || defaultSelectFn, cacheParamsFn: cacheParamsFn || defaultCacheParamsFn, loaderOptions: { cacheKeyFn: defaultCacheKeyFn, @@ -164,51 +167,15 @@ module.exports = class ServiceLoader { } key(key) { - return { - load: (id, params, cacheParamsFn) => { - return this.exec({ - method: 'load', - id, - key, - params, - cacheParamsFn - }) - }, - _load: (id, params, cacheParamsFn) => { - return this.exec({ - method: '_load', - id, - key, - params, - cacheParamsFn - }) - } - } + return new ChainedLoader({ loader: this, key }); } multi(key) { - return { - load: (id, params, cacheParamsFn) => { - return this.exec({ - method: 'load', - id, - key, - params, - cacheParamsFn, - multi: true - }) - }, - _load: (id, params, cacheParamsFn) => { - return this.exec({ - method: '_load', - id, - key, - params, - cacheParamsFn, - multi: true - }) - } - } + return new ChainedLoader({ loader: this, key, multi: true }); + } + + select(selection) { + return new ChainedLoader({ loader: this, selection }); } stringifyKey(options, cacheParamsFn = this.options.cacheParamsFn) { @@ -233,3 +200,91 @@ module.exports = class ServiceLoader { return this } } + +class ChainedLoader { + constructor({ loader, key, selection, multi }) { + this.loader = loader; + this.options = { loader, key, selection, multi } + } + + key(key) { + this.options = { + ...this.options, + key, + multi: false + } + return this; + } + + multi(key) { + this.options = { + ...this.options, + key, + multi: true + } + return this; + } + + select(selection) { + this.options = { + ...this.options, + selection + } + return this; + } + + _process(result) { + const { selection } = this.options; + const { selectFn, key } = this.loader.options; + if (selection) { + result = Array.isArray(result) + ? result.map(item => selectFn([key, ...selection], item)) + : selectFn([key, ...selection], result) + } + return result + } + + async get(id, params, cacheParamsFn) { + const result = await this.loader.get(id, params, cacheParamsFn) + return this._process(result) + } + + async _get(id, params, cacheParamsFn) { + const result = await this.loader._get(id, params, cacheParamsFn) + return this._process(result) + } + + async find(params, cacheParamsFn) { + const result = await this.loader.find(params, cacheParamsFn) + return this._process(result) + } + + async _find(params, cacheParamsFn) { + const result = await this.loader._find(params, cacheParamsFn) + return this._process(result) + } + + async load(id, params, cacheParamsFn) { + const result = await this.loader.exec({ + id, + params, + cacheParamsFn, + method: 'load', + key: this.options.key, + multi: this.options.multi + }) + return this._process(result) + } + + async _load(id, params, cacheParamsFn) { + const result = await this.loader.exec({ + id, + params, + cacheParamsFn, + method: '_load', + key: this.options.key, + multi: this.options.multi + }) + return this._process(result) + } +} diff --git a/src/utils.js b/src/utils.js index 1cbbaf7..8956735 100644 --- a/src/utils.js +++ b/src/utils.js @@ -48,6 +48,16 @@ module.exports.defaultCacheKeyFn = (id) => { return id.toString ? id.toString() : String(id) } +module.exports.defaultSelectFn = (selection, source) => { + return selection.reduce((result, key) => { + if (source[key] !== undefined) { + result[key] = source[key] + } + + return result + }, {}) +} + module.exports.uniqueKeys = (keys) => { const found = {} const unique = [] diff --git a/tests/serviceLoader.test.js b/tests/serviceLoader.test.js index 7afed47..1e9a799 100644 --- a/tests/serviceLoader.test.js +++ b/tests/serviceLoader.test.js @@ -116,6 +116,17 @@ describe('serviceLoader.test', () => { assert.deepEqual(result, defaultResult) }) + it('works with select("key").load(1)', async () => { + const serviceLoader = new ServiceLoader({ + app, + serviceName: 'posts', + }) + const defaultResult = await serviceLoader.load(1) + const selectedResult = await serviceLoader.select(['body']).load(1) + + console.log({ defaultResult, selectedResult }) + }) + it('works with get', async () => { const serviceLoader = new ServiceLoader({ app, From 0fddb1b4617baa257da6e681aa9de6bee6d7a49b Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Thu, 19 Jan 2023 15:37:14 -0600 Subject: [PATCH 02/11] Add tests --- src/serviceLoader.js | 89 ++++++++++++++++++++----------------- src/utils.js | 10 +++++ tests/serviceLoader.test.js | 79 ++++++++++++++++++++++++++++++-- 3 files changed, 133 insertions(+), 45 deletions(-) diff --git a/src/serviceLoader.js b/src/serviceLoader.js index f4d45a6..25c2247 100644 --- a/src/serviceLoader.js +++ b/src/serviceLoader.js @@ -7,10 +7,11 @@ const { defaultSelectFn, uniqueKeys, uniqueResults, - uniqueResultsMulti + uniqueResultsMulti, + assign } = require('./utils') -const createDataLoader = ({ service, key, loaderOptions, multi, method, params = {} }) => { +const createDataLoader = ({ service, key, loaderOptions, multi, method, params }) => { const serviceMethod = method === '_load' ? '_find' : 'find' if (!service[serviceMethod]) { @@ -53,24 +54,20 @@ module.exports = class ServiceLoader { key: service.options.id, selectFn: selectFn || defaultSelectFn, cacheParamsFn: cacheParamsFn || defaultCacheParamsFn, - loaderOptions: { - cacheKeyFn: defaultCacheKeyFn, - ...loaderOptions - } + loaderOptions: assign({ cacheKeyFn: defaultCacheKeyFn }, loaderOptions) } } async exec({ cacheParamsFn, ...options }) { const { service, loaderOptions } = this.options - options = { + options = assign({ id: null, key: this.options.key, - params: null, + params: {}, multi: false, - method: 'load', - ...options, - } + method: 'load' + }, options) if (['get', '_get', 'find', '_find'].includes(options.method)) { const cacheKey = this.stringifyKey(options, cacheParamsFn) @@ -167,15 +164,15 @@ module.exports = class ServiceLoader { } key(key) { - return new ChainedLoader({ loader: this, key }); + return new ChainedLoader(this, { key }); } multi(key) { - return new ChainedLoader({ loader: this, key, multi: true }); + return new ChainedLoader(this, { key, multi: true }); } select(selection) { - return new ChainedLoader({ loader: this, selection }); + return new ChainedLoader(this, { key: this.options.key, selection }); } stringifyKey(options, cacheParamsFn = this.options.cacheParamsFn) { @@ -202,66 +199,74 @@ module.exports = class ServiceLoader { } class ChainedLoader { - constructor({ loader, key, selection, multi }) { + constructor(loader, options) { this.loader = loader; - this.options = { loader, key, selection, multi } + this.options = options } key(key) { - this.options = { - ...this.options, - key, - multi: false - } + this.options = assign(this.options, { key, multi: false }) return this; } multi(key) { - this.options = { - ...this.options, - key, - multi: true - } + this.options = assign(this.options, { key, multi: true }) return this; } select(selection) { - this.options = { - ...this.options, - selection - } + this.options = assign(this.options, { selection }) return this; } - _process(result) { + handleResult(method, result) { const { selection } = this.options; const { selectFn, key } = this.loader.options; - if (selection) { - result = Array.isArray(result) - ? result.map(item => selectFn([key, ...selection], item)) - : selectFn([key, ...selection], result) + + if (!result || !selection) { + return result } - return result + + const convertResult = (result) => { + return selectFn([key, ...selection], result); + } + + if (method === 'find' && result.data) { + return { + ...result, + data: result.data.map(convertResult) + } + } + + if (Array.isArray(result)) { + return result.map((result) => { + return Array.isArray(result) + ? result.map(convertResult) + : convertResult(result) + }) + } + + return convertResult(result) } async get(id, params, cacheParamsFn) { const result = await this.loader.get(id, params, cacheParamsFn) - return this._process(result) + return this.handleResult('get', result) } async _get(id, params, cacheParamsFn) { const result = await this.loader._get(id, params, cacheParamsFn) - return this._process(result) + return this.handleResult('_get', result) } async find(params, cacheParamsFn) { const result = await this.loader.find(params, cacheParamsFn) - return this._process(result) + return this.handleResult('find', result) } async _find(params, cacheParamsFn) { const result = await this.loader._find(params, cacheParamsFn) - return this._process(result) + return this.handleResult('_find', result) } async load(id, params, cacheParamsFn) { @@ -273,7 +278,7 @@ class ChainedLoader { key: this.options.key, multi: this.options.multi }) - return this._process(result) + return this.handleResult('load', result) } async _load(id, params, cacheParamsFn) { @@ -285,6 +290,6 @@ class ChainedLoader { key: this.options.key, multi: this.options.multi }) - return this._process(result) + return this.handleResult('_load', result) } } diff --git a/src/utils.js b/src/utils.js index 8956735..b220558 100644 --- a/src/utils.js +++ b/src/utils.js @@ -58,6 +58,16 @@ module.exports.defaultSelectFn = (selection, source) => { }, {}) } +module.exports.assign = (target, source) => { + const result = { ...target } + Object.keys(source).forEach((key) => { + if (source[key] !== undefined) { + result[key] = source[key] + } + }) + return result; +} + module.exports.uniqueKeys = (keys) => { const found = {} const unique = [] diff --git a/tests/serviceLoader.test.js b/tests/serviceLoader.test.js index 1e9a799..2898cc1 100644 --- a/tests/serviceLoader.test.js +++ b/tests/serviceLoader.test.js @@ -116,7 +116,7 @@ describe('serviceLoader.test', () => { assert.deepEqual(result, defaultResult) }) - it('works with select("key").load(1)', async () => { + it('works with select(selection).load()', async () => { const serviceLoader = new ServiceLoader({ app, serviceName: 'posts', @@ -124,7 +124,81 @@ describe('serviceLoader.test', () => { const defaultResult = await serviceLoader.load(1) const selectedResult = await serviceLoader.select(['body']).load(1) - console.log({ defaultResult, selectedResult }) + assert.deepEqual(selectedResult, { + id: defaultResult.id, + body: defaultResult.body + }) + assert.deepEqual(serviceLoader.cacheMap.size, 1) + }) + + it('works with select(selection).get()', async () => { + const serviceLoader = new ServiceLoader({ + app, + serviceName: 'posts', + }) + const defaultResult = await serviceLoader.get(1) + const selectedResult = await serviceLoader.select(['body']).get(1) + + assert.deepEqual(selectedResult, { + id: defaultResult.id, + body: defaultResult.body + }) + assert.deepEqual(serviceLoader.cacheMap.size, 1) + }) + + it('works with select(selection).find()', async () => { + const serviceLoader = new ServiceLoader({ + app, + serviceName: 'posts', + }) + const defaultResult = await serviceLoader.find() + const selectedResult = await serviceLoader.select(['body']).find() + assert.deepEqual(selectedResult, defaultResult.map((result) => { + return { + id: result.id, + body: result.body + } + })) + assert.deepEqual(serviceLoader.cacheMap.size, 1) + }) + + it('works with select(selection).load([id1, id2])', async () => { + const serviceLoader = new ServiceLoader({ + app, + serviceName: 'posts', + }) + const defaultResult = await serviceLoader.load([1, 2]) + const selectedResult = await serviceLoader.select(['body']).load([1, 2]) + assert.deepEqual(selectedResult, defaultResult.map((result) => { + return { + id: result.id, + body: result.body + } + })) + assert.deepEqual(serviceLoader.cacheMap.size, 1) + }) + + it('works with select(selection).multi(key).load([id1, id2])', async () => { + const serviceLoader = new ServiceLoader({ + app, + serviceName: 'comments', + }) + const defaultResult = await serviceLoader + .multi('postId') + .load([1, 2]) + const selectedResult = await serviceLoader + .select(['text']) + .multi('postId') + .load([1, 2]) + assert.deepEqual(selectedResult, defaultResult.map((result) => { + return result.map(result => { + return { + id: result.id, + text: result.text + } + }) + })) + assert.deepEqual(serviceLoader.cacheMap.size, 1) }) it('works with get', async () => { @@ -155,7 +229,6 @@ describe('serviceLoader.test', () => { const methods = ['_get', '_find', '_load'] let hookCalled = false const hookCallback = (context) => { - console.log('hookCallback called') hookCalled = true return context } From c5d36d22c60542bd53390887daf4c9dd1f22bd56 Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Thu, 19 Jan 2023 15:42:53 -0600 Subject: [PATCH 03/11] npm run prettier --- src/serviceLoader.js | 50 +++++++++++++--------------- src/utils.js | 2 +- tests/serviceLoader.test.js | 66 ++++++++++++++++++++----------------- 3 files changed, 58 insertions(+), 60 deletions(-) diff --git a/src/serviceLoader.js b/src/serviceLoader.js index 1c7c002..05a28bc 100644 --- a/src/serviceLoader.js +++ b/src/serviceLoader.js @@ -38,14 +38,7 @@ const createDataLoader = ({ service, key, loaderOptions, multi, method, params } } module.exports = class ServiceLoader { - constructor({ - app, - serviceName, - cacheParamsFn, - selectFn, - cacheMap, - ...loaderOptions - }) { + constructor({ app, serviceName, cacheParamsFn, selectFn, cacheMap, ...loaderOptions }) { this.cacheMap = cacheMap || new Map() this.loaders = new Map() const service = app.service(serviceName) @@ -63,13 +56,16 @@ module.exports = class ServiceLoader { async exec({ cacheParamsFn, ...options }) { const { service, loaderOptions } = this.options - options = assign({ - id: null, - key: this.options.key, - params: {}, - multi: false, - method: 'load' - }, options) + options = assign( + { + id: null, + key: this.options.key, + params: {}, + multi: false, + method: 'load' + }, + options + ) if (['get', '_get', 'find', '_find'].includes(options.method)) { const cacheKey = this.stringifyKey(options, cacheParamsFn) @@ -166,15 +162,15 @@ module.exports = class ServiceLoader { } key(key) { - return new ChainedLoader(this, { key }); + return new ChainedLoader(this, { key }) } multi(key) { - return new ChainedLoader(this, { key, multi: true }); + return new ChainedLoader(this, { key, multi: true }) } select(selection) { - return new ChainedLoader(this, { key: this.options.key, selection }); + return new ChainedLoader(this, { key: this.options.key, selection }) } stringifyKey(options, cacheParamsFn = this.options.cacheParamsFn) { @@ -202,35 +198,35 @@ module.exports = class ServiceLoader { class ChainedLoader { constructor(loader, options) { - this.loader = loader; + this.loader = loader this.options = options } key(key) { this.options = assign(this.options, { key, multi: false }) - return this; + return this } multi(key) { this.options = assign(this.options, { key, multi: true }) - return this; + return this } select(selection) { this.options = assign(this.options, { selection }) - return this; + return this } handleResult(method, result) { - const { selection } = this.options; - const { selectFn, key } = this.loader.options; + const { selection } = this.options + const { selectFn, key } = this.loader.options if (!result || !selection) { return result } const convertResult = (result) => { - return selectFn([key, ...selection], result); + return selectFn([key, ...selection], result) } if (method === 'find' && result.data) { @@ -242,9 +238,7 @@ class ChainedLoader { if (Array.isArray(result)) { return result.map((result) => { - return Array.isArray(result) - ? result.map(convertResult) - : convertResult(result) + return Array.isArray(result) ? result.map(convertResult) : convertResult(result) }) } diff --git a/src/utils.js b/src/utils.js index a04c282..2912110 100644 --- a/src/utils.js +++ b/src/utils.js @@ -65,7 +65,7 @@ module.exports.assign = (target, source) => { result[key] = source[key] } }) - return result; + return result } module.exports.uniqueKeys = (keys) => { diff --git a/tests/serviceLoader.test.js b/tests/serviceLoader.test.js index ed76557..26189be 100644 --- a/tests/serviceLoader.test.js +++ b/tests/serviceLoader.test.js @@ -119,7 +119,7 @@ describe('serviceLoader.test', () => { it('works with select(selection).load()', async () => { const serviceLoader = new ServiceLoader({ app, - serviceName: 'posts', + serviceName: 'posts' }) const defaultResult = await serviceLoader.load(1) const selectedResult = await serviceLoader.select(['body']).load(1) @@ -134,7 +134,7 @@ describe('serviceLoader.test', () => { it('works with select(selection).get()', async () => { const serviceLoader = new ServiceLoader({ app, - serviceName: 'posts', + serviceName: 'posts' }) const defaultResult = await serviceLoader.get(1) const selectedResult = await serviceLoader.select(['body']).get(1) @@ -149,55 +149,59 @@ describe('serviceLoader.test', () => { it('works with select(selection).find()', async () => { const serviceLoader = new ServiceLoader({ app, - serviceName: 'posts', + serviceName: 'posts' }) const defaultResult = await serviceLoader.find() const selectedResult = await serviceLoader.select(['body']).find() - assert.deepEqual(selectedResult, defaultResult.map((result) => { - return { - id: result.id, - body: result.body - } - })) + assert.deepEqual( + selectedResult, + defaultResult.map((result) => { + return { + id: result.id, + body: result.body + } + }) + ) assert.deepEqual(serviceLoader.cacheMap.size, 1) }) it('works with select(selection).load([id1, id2])', async () => { const serviceLoader = new ServiceLoader({ app, - serviceName: 'posts', + serviceName: 'posts' }) const defaultResult = await serviceLoader.load([1, 2]) const selectedResult = await serviceLoader.select(['body']).load([1, 2]) - assert.deepEqual(selectedResult, defaultResult.map((result) => { - return { - id: result.id, - body: result.body - } - })) + assert.deepEqual( + selectedResult, + defaultResult.map((result) => { + return { + id: result.id, + body: result.body + } + }) + ) assert.deepEqual(serviceLoader.cacheMap.size, 1) }) it('works with select(selection).multi(key).load([id1, id2])', async () => { const serviceLoader = new ServiceLoader({ app, - serviceName: 'comments', + serviceName: 'comments' }) - const defaultResult = await serviceLoader - .multi('postId') - .load([1, 2]) - const selectedResult = await serviceLoader - .select(['text']) - .multi('postId') - .load([1, 2]) - assert.deepEqual(selectedResult, defaultResult.map((result) => { - return result.map(result => { - return { - id: result.id, - text: result.text - } + const defaultResult = await serviceLoader.multi('postId').load([1, 2]) + const selectedResult = await serviceLoader.select(['text']).multi('postId').load([1, 2]) + assert.deepEqual( + selectedResult, + defaultResult.map((result) => { + return result.map((result) => { + return { + id: result.id, + text: result.text + } + }) }) - })) + ) assert.deepEqual(serviceLoader.cacheMap.size, 1) }) From 3d2d0c89b77805b98302681028efb1e5d3245e13 Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Fri, 27 Jan 2023 15:00:25 -0600 Subject: [PATCH 04/11] Move cacheParamsFn to chainable method --- docs/loader/service-loader.md | 1 - src/serviceLoader.js | 135 ++++++++++++++++------------------ src/utils.js | 30 +++++++- 3 files changed, 90 insertions(+), 76 deletions(-) diff --git a/docs/loader/service-loader.md b/docs/loader/service-loader.md index 654cbd2..c6ec8bb 100644 --- a/docs/loader/service-loader.md +++ b/docs/loader/service-loader.md @@ -33,7 +33,6 @@ TODO: Provide Links to the DataLoader and FindLoader options. - **serviceName** `{String}` - The name of the service - **cacheMap** `{Object}` - Instance of Map (or an object with a similar API) to be used as cache. Defaults to `new Map()` - **cacheParamsFn** `{Function}` - A function that returns JSON.strinify-able params of a query to be used in the `cacheMap`. This function should return a set of params that will be used to identify this unique query and removes any non-serializable items. The default function returns traverses params and removes any functions. Defaults to `defaultCacheParamsFn` - - **cacheKeyFn** `{Function}` - Normalize keys. `(key) => key && key.toString ? key.toString() : String(key)` Defaults to `defaultCacheKeyFn` There are two ways to create `ServiceLoader` instances. diff --git a/src/serviceLoader.js b/src/serviceLoader.js index 05a28bc..d036b7c 100644 --- a/src/serviceLoader.js +++ b/src/serviceLoader.js @@ -37,6 +37,13 @@ const createDataLoader = ({ service, key, loaderOptions, multi, method, params } }, loaderOptions) } +const stringifyKey =(options, cacheParamsFn) =>{ + return stableStringify({ + ...options, + params: cacheParamsFn(options.params) + }) +} + module.exports = class ServiceLoader { constructor({ app, serviceName, cacheParamsFn, selectFn, cacheMap, ...loaderOptions }) { this.cacheMap = cacheMap || new Map() @@ -68,7 +75,7 @@ module.exports = class ServiceLoader { ) if (['get', '_get', 'find', '_find'].includes(options.method)) { - const cacheKey = this.stringifyKey(options, cacheParamsFn) + const cacheKey = stringifyKey(options, cacheParamsFn) const cachedResult = await this.cacheMap.get(cacheKey) @@ -91,7 +98,7 @@ module.exports = class ServiceLoader { // does to the cache key, thats why these are sorted. const sortedId = Array.isArray(options.id) ? [...options.id].sort() : options.id - const cacheKey = this.stringifyKey( + const cacheKey = stringifyKey( { ...options, id: sortedId @@ -105,7 +112,7 @@ module.exports = class ServiceLoader { return cachedResult } - const loaderKey = this.stringifyKey( + const loaderKey = stringifyKey( { key: options.key, multi: options.multi, @@ -137,28 +144,28 @@ module.exports = class ServiceLoader { return result } - get(id, params, cacheParamsFn) { - return this.exec({ method: 'get', id, params, cacheParamsFn }) + get(id, params) { + return this.exec({ method: 'get', id, params }) } - _get(id, params, cacheParamsFn) { - return this.exec({ method: '_get', id, params, cacheParamsFn }) + _get(id, params) { + return this.exec({ method: '_get', id, params }) } - find(params, cacheParamsFn) { - return this.exec({ method: 'find', params, cacheParamsFn }) + find(params) { + return this.exec({ method: 'find', params }) } - _find(params, cacheParamsFn) { - return this.exec({ method: '_find', params, cacheParamsFn }) + _find(params) { + return this.exec({ method: '_find', params }) } - load(id, params, cacheParamsFn) { - return this.exec({ method: 'load', id, params, cacheParamsFn }) + load(id, params) { + return this.exec({ method: 'load', id, params }) } - _load(id, params, cacheParamsFn) { - return this.exec({ method: '_load', id, params, cacheParamsFn }) + _load(id, params) { + return this.exec({ method: '_load', id, params }) } key(key) { @@ -173,18 +180,20 @@ module.exports = class ServiceLoader { return new ChainedLoader(this, { key: this.options.key, selection }) } - stringifyKey(options, cacheParamsFn = this.options.cacheParamsFn) { - return stableStringify({ - ...options, - serviceName: this.options.serviceName, - params: cacheParamsFn(options.params) - }) + params(cacheParamsFn) { + return new ChainedLoader(this, { key: this.options.key, cacheParamsFn }) } async clear() { const { serviceName } = this.options this.loaders.clear() const promises = [] + // TODO: This could be a redis store or some other + // async storage. That's why there is this for/await + // iterator used to step over all the keys in a collection. + // Is it a good idea to just be pushing all the deletes into + // one Promise.all() after? Instead, we may want to call + // the delete in the iteration. But that would be slower. for await (const cacheKey of this.cacheMap.keys()) { const parsedKey = JSON.parse(cacheKey) if (parsedKey.serviceName === serviceName) { @@ -217,75 +226,55 @@ class ChainedLoader { return this } - handleResult(method, result) { + params(cacheParamsFn) { + this.options = assign(this.options, { cacheParamsFn }) + return this + } + + async handleResult(result) { const { selection } = this.options - const { selectFn, key } = this.loader.options + const { selectFn } = this.loader.options if (!result || !selection) { return result } - const convertResult = (result) => { - return selectFn([key, ...selection], result) - } - - if (method === 'find' && result.data) { - return { - ...result, - data: result.data.map(convertResult) - } - } - - if (Array.isArray(result)) { - return result.map((result) => { - return Array.isArray(result) ? result.map(convertResult) : convertResult(result) - }) - } - - return convertResult(result) + return await selectFn(selection, result, this) } - async get(id, params, cacheParamsFn) { - const result = await this.loader.get(id, params, cacheParamsFn) - return this.handleResult('get', result) + async get(id, params) { + this.options = assign(this.options, { method: 'get' }) + const result = await this.loader.exec({ ...this.options, id, params }) + return this.handleResult(result) } - async _get(id, params, cacheParamsFn) { - const result = await this.loader._get(id, params, cacheParamsFn) - return this.handleResult('_get', result) + async _get(id, params) { + this.options = assign(this.options, { method: '_get' }) + const result = await this.loader.exec({ ...this.options, id, params }) + return this.handleResult(result) } - async find(params, cacheParamsFn) { - const result = await this.loader.find(params, cacheParamsFn) - return this.handleResult('find', result) + async find(params) { + this.options = assign(this.options, { method: 'find' }) + const result = await this.loader.exec({ ...this.options, params }) + return this.handleResult(result) } - async _find(params, cacheParamsFn) { - const result = await this.loader._find(params, cacheParamsFn) - return this.handleResult('_find', result) + async _find(params) { + this.options = assign(this.options, { method: '_find' }) + const result = await this.loader.exec({ ...this.options, id, params }) + return this.handleResult(result) } - async load(id, params, cacheParamsFn) { - const result = await this.loader.exec({ - id, - params, - cacheParamsFn, - method: 'load', - key: this.options.key, - multi: this.options.multi - }) - return this.handleResult('load', result) + async load(id, params) { + this.options = assign(this.options, { method: 'load' }) + const result = await this.loader.exec({ ...this.options, id, params }) + return this.handleResult(result) } - async _load(id, params, cacheParamsFn) { - const result = await this.loader.exec({ - id, - params, - cacheParamsFn, - method: '_load', - key: this.options.key, - multi: this.options.multi - }) - return this.handleResult('_load', result) + async _load(id, params) { + this.options = assign(this.options, { method: '_load' }) + const result = await this.loader.exec({ ...this.options, id, params }) + return this.handleResult(result) } } diff --git a/src/utils.js b/src/utils.js index 2912110..62f0416 100644 --- a/src/utils.js +++ b/src/utils.js @@ -48,16 +48,42 @@ module.exports.defaultCacheKeyFn = (id) => { return id.toString ? id.toString() : String(id) } -module.exports.defaultSelectFn = (selection, source) => { +const select = (selection, source) => { return selection.reduce((result, key) => { if (source[key] !== undefined) { result[key] = source[key] } - return result }, {}) } +module.exports.defaultSelectFn = (selection, result, chainedLoader) => { + if (!Array.isArray(selection)) { + throw new Error('selection must be an array'); + } + + const { key, method } = chainedLoader.options; + + const convertResult = (result) => { + return select([key, ...selection], result) + } + + if (['find', '_find'].includes(method) && result.data) { + return { + ...result, + data: result.data.map(convertResult) + } + } + + if (Array.isArray(result)) { + return result.map((result) => { + return Array.isArray(result) ? result.map(convertResult) : convertResult(result) + }) + } + + return convertResult(result) +} + module.exports.assign = (target, source) => { const result = { ...target } Object.keys(source).forEach((key) => { From eedbea38261df45c90aa9ebb3c4834b9275b78f0 Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Fri, 27 Jan 2023 15:21:48 -0600 Subject: [PATCH 05/11] Prettier --- src/serviceLoader.js | 2 +- src/utils.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/serviceLoader.js b/src/serviceLoader.js index d036b7c..b2bdc2c 100644 --- a/src/serviceLoader.js +++ b/src/serviceLoader.js @@ -37,7 +37,7 @@ const createDataLoader = ({ service, key, loaderOptions, multi, method, params } }, loaderOptions) } -const stringifyKey =(options, cacheParamsFn) =>{ +const stringifyKey = (options, cacheParamsFn) => { return stableStringify({ ...options, params: cacheParamsFn(options.params) diff --git a/src/utils.js b/src/utils.js index 62f0416..909ce12 100644 --- a/src/utils.js +++ b/src/utils.js @@ -59,10 +59,10 @@ const select = (selection, source) => { module.exports.defaultSelectFn = (selection, result, chainedLoader) => { if (!Array.isArray(selection)) { - throw new Error('selection must be an array'); + throw new Error('selection must be an array') } - const { key, method } = chainedLoader.options; + const { key, method } = chainedLoader.options const convertResult = (result) => { return select([key, ...selection], result) From d9081bb5b34ba32adb8b68de58c922d4023d0daa Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Fri, 27 Jan 2023 16:21:56 -0600 Subject: [PATCH 06/11] Pass tests, do some cleanup --- docs/index.md | 6 +- docs/loader/service-loader.md | 6 +- src/appLoader.js | 10 +-- src/serviceLoader.js | 128 ++++++++++++++++------------------ src/utils.js | 4 +- tests/appLoader.test.js | 5 +- tests/serviceLoader.test.js | 63 +++++++---------- 7 files changed, 103 insertions(+), 119 deletions(-) diff --git a/docs/index.md b/docs/index.md index e3557ca..0038fc2 100644 --- a/docs/index.md +++ b/docs/index.md @@ -89,7 +89,7 @@ The `AppLoader` lazily configures a new `ServiceLoader` per service as you use t const { ServiceLoader } = require('feathers-dataloader'); -const serviceLoader = new ServiceLoader({ app, serviceName: 'users' }); +const serviceLoader = new ServiceLoader({ app, service: 'users' }); const user = await serviceLoader.load(1, params); const user = await serviceLoader.get(1, params); const users = await serviceLoader.find(params); @@ -176,7 +176,7 @@ Create a new service-loader. This class lazily configures underlying `DataLoader | Argument | Type | Default | Description | | --------------- | :--------: | ---------- | ------------------------------------------------ | | `app` | `Object` | | A Feathers app` | -| `serviceName` | `String` | | The name of the service like "users"` | +| `service` | `String` | | The name of the service like "users"` | | `cacheMap` | `Map` | | A Map like object with methods get, set, and clear to serve as the cache of results.` | | `cacheParamsFn` | `Function` | defaultCacheParamsFn | A function that returns a JSON stringifiable set or params to be used in the cacheKey. The default function traverses the params and removes any functions` | | `loaderOptions` | `Object` | {} | See `DataLoader` | @@ -187,7 +187,7 @@ Create a new service-loader. This class lazily configures underlying `DataLoader const loader = new ServiceLoader({ app, - serviceName: 'users' + service: 'users' cacheParamsFn: (params) => { return { userId: params.user.id, diff --git a/docs/loader/service-loader.md b/docs/loader/service-loader.md index c6ec8bb..743f7bc 100644 --- a/docs/loader/service-loader.md +++ b/docs/loader/service-loader.md @@ -30,7 +30,7 @@ TODO: Provide Links to the DataLoader and FindLoader options. - **options** `{Object}` - **app** `{Object}` - A Feathers app, - - **serviceName** `{String}` - The name of the service + - **service** `{String}` - The name of the service - **cacheMap** `{Object}` - Instance of Map (or an object with a similar API) to be used as cache. Defaults to `new Map()` - **cacheParamsFn** `{Function}` - A function that returns JSON.strinify-able params of a query to be used in the `cacheMap`. This function should return a set of params that will be used to identify this unique query and removes any non-serializable items. The default function returns traverses params and removes any functions. Defaults to `defaultCacheParamsFn` @@ -57,7 +57,7 @@ You can also directly create an instance using the `ServiceLoader` class. const { ServiceLoader } = require('@feathersjs/loader') // This is our ServiceLoader instance -const userLoader = new ServiceLoader({ app, serviceName: 'users' }) +const userLoader = new ServiceLoader({ app, service: 'users' }) ``` ## Example @@ -70,7 +70,7 @@ const loaderOptions = {} const loader = new ServiceLoader({ app - serviceName: 'users', + service: 'users', ...loaderOptions }) diff --git a/src/appLoader.js b/src/appLoader.js index b367850..f4b3f61 100644 --- a/src/appLoader.js +++ b/src/appLoader.js @@ -6,14 +6,14 @@ module.exports = class AppLoader { this.loaders = new Map() } - service(serviceName) { + service(path) { const { app } = this.options const { ServiceLoader, ...loaderOptions } = { ServiceLoader: BaseServiceLoader, ...this.options.loaderOptions, - ...(this.options.services[serviceName] || {}) + ...(this.options.services[path] || {}) } - const cachedLoader = this.loaders.get(serviceName) + const cachedLoader = this.loaders.get(path) if (cachedLoader) { return cachedLoader @@ -21,11 +21,11 @@ module.exports = class AppLoader { const loader = new ServiceLoader({ ...loaderOptions, - serviceName, + service: path, app }) - this.loaders.set(serviceName, loader) + this.loaders.set(path, loader) return loader } diff --git a/src/serviceLoader.js b/src/serviceLoader.js index b2bdc2c..5ceaafa 100644 --- a/src/serviceLoader.js +++ b/src/serviceLoader.js @@ -11,10 +11,10 @@ const { assign } = require('./utils') -const createDataLoader = ({ service, key, loaderOptions, multi, method, params }) => { +const createDataLoader = ({ appService, key, loaderOptions, multi, method, params }) => { const serviceMethod = method === '_load' ? '_find' : 'find' - if (!service[serviceMethod]) { + if (!appService[serviceMethod]) { throw new GeneralError( `Cannot create a loader for a service that does not have a ${serviceMethod} method.` ) @@ -33,7 +33,7 @@ const createDataLoader = ({ service, key, loaderOptions, multi, method, params } } } delete loaderParams.query.$limit - return service[serviceMethod](loaderParams).then((result) => getResults(keys, result, key)) + return appService[serviceMethod](loaderParams).then((result) => getResults(keys, result, key)) }, loaderOptions) } @@ -45,15 +45,15 @@ const stringifyKey = (options, cacheParamsFn) => { } module.exports = class ServiceLoader { - constructor({ app, serviceName, cacheParamsFn, selectFn, cacheMap, ...loaderOptions }) { + constructor({ app, service, cacheParamsFn, selectFn, cacheMap, ...loaderOptions }) { this.cacheMap = cacheMap || new Map() this.loaders = new Map() - const service = app.service(serviceName) + const appService = app.service(service) this.options = { app, - serviceName, service, - key: service.options.id, + appService, + key: appService.options.id, selectFn: selectFn || defaultSelectFn, cacheParamsFn: cacheParamsFn || defaultCacheParamsFn, loaderOptions: assign({ cacheKeyFn: defaultCacheKeyFn }, loaderOptions) @@ -61,7 +61,8 @@ module.exports = class ServiceLoader { } async exec({ cacheParamsFn, ...options }) { - const { service, loaderOptions } = this.options + const { appService, loaderOptions, service } = this.options + cacheParamsFn = cacheParamsFn || this.options.cacheParamsFn options = assign( { @@ -71,7 +72,10 @@ module.exports = class ServiceLoader { multi: false, method: 'load' }, - options + { + ...options, + service + } ) if (['get', '_get', 'find', '_find'].includes(options.method)) { @@ -84,8 +88,8 @@ module.exports = class ServiceLoader { } const result = ['get', '_get'].includes(options.method) - ? await service[options.method](options.id, options.params) - : await service[options.method](options.params) + ? await appService[options.method](options.id, options.params) + : await appService[options.method](options.params) await this.cacheMap.set(cacheKey, result) @@ -112,25 +116,21 @@ module.exports = class ServiceLoader { return cachedResult } - const loaderKey = stringifyKey( - { - key: options.key, - multi: options.multi, - method: options.method, - params: options.params - }, - cacheParamsFn - ) + const loaderConfig = { + key: options.key, + multi: options.multi, + method: options.method, + params: options.params + } + + const loaderKey = stringifyKey(loaderConfig, cacheParamsFn) const dataLoader = this.loaders.get(loaderKey) || createDataLoader({ - key: options.key, - multi: options.multi, - method: options.method, - params: options.params, - service, - loaderOptions + appService, + loaderOptions, + ...loaderConfig }) this.loaders.set(loaderKey, dataLoader) @@ -185,7 +185,7 @@ module.exports = class ServiceLoader { } async clear() { - const { serviceName } = this.options + const { service } = this.options this.loaders.clear() const promises = [] // TODO: This could be a redis store or some other @@ -196,7 +196,7 @@ module.exports = class ServiceLoader { // the delete in the iteration. But that would be slower. for await (const cacheKey of this.cacheMap.keys()) { const parsedKey = JSON.parse(cacheKey) - if (parsedKey.serviceName === serviceName) { + if (parsedKey.service === service) { promises.push(this.cacheMap.delete(cacheKey)) } } @@ -207,74 +207,70 @@ module.exports = class ServiceLoader { class ChainedLoader { constructor(loader, options) { - this.loader = loader - this.options = options + this.options = { ...options, loader } } key(key) { - this.options = assign(this.options, { key, multi: false }) - return this + return this.set({ key, multi: false }) } multi(key) { - this.options = assign(this.options, { key, multi: true }) - return this + return this.set({ key, multi: true }) } select(selection) { - this.options = assign(this.options, { selection }) - return this + return this.set({ selection }) } params(cacheParamsFn) { - this.options = assign(this.options, { cacheParamsFn }) - return this - } - - async handleResult(result) { - const { selection } = this.options - const { selectFn } = this.loader.options - - if (!result || !selection) { - return result - } - - return await selectFn(selection, result, this) + return this.set({ cacheParamsFn }) } async get(id, params) { - this.options = assign(this.options, { method: 'get' }) - const result = await this.loader.exec({ ...this.options, id, params }) - return this.handleResult(result) + return this.set({ method: 'get', id, params }).exec() } async _get(id, params) { - this.options = assign(this.options, { method: '_get' }) - const result = await this.loader.exec({ ...this.options, id, params }) - return this.handleResult(result) + return this.set({ method: '_get', id, params }).exec() } async find(params) { - this.options = assign(this.options, { method: 'find' }) - const result = await this.loader.exec({ ...this.options, params }) - return this.handleResult(result) + return this.set({ method: 'find', params }).exec() } async _find(params) { - this.options = assign(this.options, { method: '_find' }) - const result = await this.loader.exec({ ...this.options, id, params }) - return this.handleResult(result) + return this.set({ method: '_find', params }).exec() } async load(id, params) { - this.options = assign(this.options, { method: 'load' }) - const result = await this.loader.exec({ ...this.options, id, params }) - return this.handleResult(result) + return this.set({ method: 'load', id, params }).exec() } async _load(id, params) { - this.options = assign(this.options, { method: '_load' }) - const result = await this.loader.exec({ ...this.options, id, params }) + return this.set({ method: '_load', id, params }).exec() + } + + set(options) { + this.options = assign(this.options, options) + return this + } + + async handleResult(result) { + const { selection } = this.options + const { selectFn } = this.options.loader.options + + if (!result || !selection) { + return result + } + + return await selectFn(selection, result, this.options) + } + + async exec() { + const options = { ...this.options } + delete options.selection + delete options.loader + const result = await this.options.loader.exec(options) return this.handleResult(result) } } diff --git a/src/utils.js b/src/utils.js index 909ce12..44f7d1e 100644 --- a/src/utils.js +++ b/src/utils.js @@ -57,12 +57,12 @@ const select = (selection, source) => { }, {}) } -module.exports.defaultSelectFn = (selection, result, chainedLoader) => { +module.exports.defaultSelectFn = (selection, result, options) => { if (!Array.isArray(selection)) { throw new Error('selection must be an array') } - const { key, method } = chainedLoader.options + const { key, method } = options const convertResult = (result) => { return select([key, ...selection], result) diff --git a/tests/appLoader.test.js b/tests/appLoader.test.js index 8538153..7730ae8 100644 --- a/tests/appLoader.test.js +++ b/tests/appLoader.test.js @@ -25,11 +25,12 @@ describe('appLoader.test', () => { assert.isFunction(serviceLoader._find) assert.isFunction(serviceLoader.load) assert.isFunction(serviceLoader._load) - assert.isFunction(serviceLoader.multi) assert.isFunction(serviceLoader.key) + assert.isFunction(serviceLoader.select) + assert.isFunction(serviceLoader.params) + assert.isFunction(serviceLoader.multi) assert.isFunction(serviceLoader.exec) assert.isFunction(serviceLoader.clear) - assert.isFunction(serviceLoader.stringifyKey) }) it('returns a cached ServiceLoader', () => { diff --git a/tests/serviceLoader.test.js b/tests/serviceLoader.test.js index 26189be..16ee987 100644 --- a/tests/serviceLoader.test.js +++ b/tests/serviceLoader.test.js @@ -10,7 +10,7 @@ describe('serviceLoader.test', () => { it('creates a serviceLoader', () => { const serviceLoader = new ServiceLoader({ app, - serviceName: 'posts' + service: 'posts' }) assert.isFunction(serviceLoader.get) assert.isFunction(serviceLoader._get) @@ -18,17 +18,18 @@ describe('serviceLoader.test', () => { assert.isFunction(serviceLoader._find) assert.isFunction(serviceLoader.load) assert.isFunction(serviceLoader._load) - assert.isFunction(serviceLoader.multi) assert.isFunction(serviceLoader.key) + assert.isFunction(serviceLoader.select) + assert.isFunction(serviceLoader.params) + assert.isFunction(serviceLoader.multi) assert.isFunction(serviceLoader.exec) assert.isFunction(serviceLoader.clear) - assert.isFunction(serviceLoader.stringifyKey) }) it('takes a cacheParamsFn option', () => { const serviceLoader = new ServiceLoader({ app, - serviceName: 'posts', + service: 'posts', cacheParamsFn: testFunc }) assert.deepEqual(serviceLoader.options.cacheParamsFn, testFunc) @@ -37,7 +38,7 @@ describe('serviceLoader.test', () => { it('passes loader options', async () => { const serviceLoader = new ServiceLoader({ app, - serviceName: 'posts', + service: 'posts', cacheKeyFn: testFunc }) await serviceLoader.load(1) @@ -48,7 +49,7 @@ describe('serviceLoader.test', () => { it('works with load(id)', async () => { const serviceLoader = new ServiceLoader({ app, - serviceName: 'posts' + service: 'posts' }) const defaultResult = await app.service('posts').get(1) const result = await serviceLoader.load(1) @@ -58,7 +59,7 @@ describe('serviceLoader.test', () => { it('works with load([id1, id2])', async () => { const serviceLoader = new ServiceLoader({ app, - serviceName: 'posts' + service: 'posts' }) const defaultResult = await Promise.all([app.service('posts').get(1), app.service('posts').get(2)]) const result = await serviceLoader.load([1, 2]) @@ -68,7 +69,7 @@ describe('serviceLoader.test', () => { it('works with key("key").load(id)', async () => { const serviceLoader = new ServiceLoader({ app, - serviceName: 'posts' + service: 'posts' }) const defaultResult = await app.service('posts').get(1) const result = await serviceLoader.key('body').load('John post') @@ -78,7 +79,7 @@ describe('serviceLoader.test', () => { it('works with key("key")._load(id)', async () => { const serviceLoader = new ServiceLoader({ app, - serviceName: 'posts' + service: 'posts' }) const defaultResult = await app.service('posts').get(1) const result = await serviceLoader.key('body')._load('John post') @@ -88,7 +89,7 @@ describe('serviceLoader.test', () => { it('works with multi("key").load(id)', async () => { const serviceLoader = new ServiceLoader({ app, - serviceName: 'comments' + service: 'comments' }) const result = await serviceLoader.multi('postId').load(1) assert.deepEqual(result.length, 3) @@ -97,7 +98,7 @@ describe('serviceLoader.test', () => { it('works with multi("key")._load(id)', async () => { const serviceLoader = new ServiceLoader({ app, - serviceName: 'comments' + service: 'comments' }) const result = await serviceLoader.multi('postId')._load(1) assert.deepEqual(result.length, 3) @@ -106,7 +107,7 @@ describe('serviceLoader.test', () => { it('works with multi("key").load([id1, id2])', async () => { const serviceLoader = new ServiceLoader({ app, - serviceName: 'comments' + service: 'comments' }) const defaultResult = await Promise.all([ app.service('comments').find({ paginate: false, query: { postId: 1 } }), @@ -119,7 +120,7 @@ describe('serviceLoader.test', () => { it('works with select(selection).load()', async () => { const serviceLoader = new ServiceLoader({ app, - serviceName: 'posts' + service: 'posts' }) const defaultResult = await serviceLoader.load(1) const selectedResult = await serviceLoader.select(['body']).load(1) @@ -134,7 +135,7 @@ describe('serviceLoader.test', () => { it('works with select(selection).get()', async () => { const serviceLoader = new ServiceLoader({ app, - serviceName: 'posts' + service: 'posts' }) const defaultResult = await serviceLoader.get(1) const selectedResult = await serviceLoader.select(['body']).get(1) @@ -149,7 +150,7 @@ describe('serviceLoader.test', () => { it('works with select(selection).find()', async () => { const serviceLoader = new ServiceLoader({ app, - serviceName: 'posts' + service: 'posts' }) const defaultResult = await serviceLoader.find() const selectedResult = await serviceLoader.select(['body']).find() @@ -168,7 +169,7 @@ describe('serviceLoader.test', () => { it('works with select(selection).load([id1, id2])', async () => { const serviceLoader = new ServiceLoader({ app, - serviceName: 'posts' + service: 'posts' }) const defaultResult = await serviceLoader.load([1, 2]) const selectedResult = await serviceLoader.select(['body']).load([1, 2]) @@ -187,16 +188,16 @@ describe('serviceLoader.test', () => { it('works with select(selection).multi(key).load([id1, id2])', async () => { const serviceLoader = new ServiceLoader({ app, - serviceName: 'comments' + service: 'comments' }) const defaultResult = await serviceLoader.multi('postId').load([1, 2]) - const selectedResult = await serviceLoader.select(['text']).multi('postId').load([1, 2]) + const selectedResult = await serviceLoader.multi('postId').select(['text']).load([1, 2]) assert.deepEqual( selectedResult, defaultResult.map((result) => { return result.map((result) => { return { - id: result.id, + postId: result.postId, text: result.text } }) @@ -208,7 +209,7 @@ describe('serviceLoader.test', () => { it('works with get', async () => { const serviceLoader = new ServiceLoader({ app, - serviceName: 'posts' + service: 'posts' }) const defaultResult = await app.service('posts').get(1) const result = await serviceLoader.get(1) @@ -218,7 +219,7 @@ describe('serviceLoader.test', () => { it('works with find', async () => { const serviceLoader = new ServiceLoader({ app, - serviceName: 'posts' + service: 'posts' }) const defaultResult = await app.service('posts').find() const result = await serviceLoader.find() @@ -228,7 +229,7 @@ describe('serviceLoader.test', () => { it('works with underscored methods', async () => { const serviceLoader = new ServiceLoader({ app, - serviceName: 'posts' + service: 'posts' }) const methods = ['_get', '_find', '_load'] let hookCalled = false @@ -247,30 +248,16 @@ describe('serviceLoader.test', () => { assert.deepEqual(hookCalled, false) }) - it('works with stringifyKey', async () => { - const serviceLoader = new ServiceLoader({ - app, - serviceName: 'posts' - }) - const cacheKey = serviceLoader.stringifyKey({ id: 1, key: 'id' }) - const stableKey = stableStringify({ - serviceName: 'posts', - id: 1, - key: 'id' - }) - assert.deepEqual(cacheKey, stableKey) - }) - it('clears', async () => { const cacheMap = new Map() const postsLoader = new ServiceLoader({ app, - serviceName: 'posts', + service: 'posts', cacheMap: cacheMap }) const commentsLoader = new ServiceLoader({ app, - serviceName: 'comments', + service: 'comments', cacheMap: cacheMap }) From 143d09aa3c832bee5fd6b3daaafe5d1cc1eda7b0 Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Fri, 27 Jan 2023 16:31:18 -0600 Subject: [PATCH 07/11] Add tests --- src/utils.js | 10 +++++++--- tests/serviceLoader.test.js | 27 +++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/src/utils.js b/src/utils.js index 44f7d1e..17b100f 100644 --- a/src/utils.js +++ b/src/utils.js @@ -62,7 +62,7 @@ module.exports.defaultSelectFn = (selection, result, options) => { throw new Error('selection must be an array') } - const { key, method } = options + const { key, method, multi } = options const convertResult = (result) => { return select([key, ...selection], result) @@ -75,12 +75,16 @@ module.exports.defaultSelectFn = (selection, result, options) => { } } - if (Array.isArray(result)) { + if (multi) { return result.map((result) => { - return Array.isArray(result) ? result.map(convertResult) : convertResult(result) + return result.map(convertResult) }) } + if (Array.isArray(result)) { + return result.map(convertResult) + } + return convertResult(result) } diff --git a/tests/serviceLoader.test.js b/tests/serviceLoader.test.js index 16ee987..ed0a82e 100644 --- a/tests/serviceLoader.test.js +++ b/tests/serviceLoader.test.js @@ -122,6 +122,7 @@ describe('serviceLoader.test', () => { app, service: 'posts' }) + const mainResult = await app.service('posts').get(1) const defaultResult = await serviceLoader.load(1) const selectedResult = await serviceLoader.select(['body']).load(1) @@ -129,6 +130,7 @@ describe('serviceLoader.test', () => { id: defaultResult.id, body: defaultResult.body }) + assert.deepEqual(mainResult, defaultResult) assert.deepEqual(serviceLoader.cacheMap.size, 1) }) @@ -137,6 +139,7 @@ describe('serviceLoader.test', () => { app, service: 'posts' }) + const mainResult = await app.service('posts').get(1) const defaultResult = await serviceLoader.get(1) const selectedResult = await serviceLoader.select(['body']).get(1) @@ -144,6 +147,7 @@ describe('serviceLoader.test', () => { id: defaultResult.id, body: defaultResult.body }) + assert.deepEqual(mainResult, defaultResult) assert.deepEqual(serviceLoader.cacheMap.size, 1) }) @@ -152,8 +156,10 @@ describe('serviceLoader.test', () => { app, service: 'posts' }) + const mainResult = await app.service('posts').find() const defaultResult = await serviceLoader.find() const selectedResult = await serviceLoader.select(['body']).find() + assert.deepEqual( selectedResult, defaultResult.map((result) => { @@ -163,6 +169,7 @@ describe('serviceLoader.test', () => { } }) ) + assert.deepEqual(mainResult, defaultResult) assert.deepEqual(serviceLoader.cacheMap.size, 1) }) @@ -171,8 +178,16 @@ describe('serviceLoader.test', () => { app, service: 'posts' }) + const mainResult = await app.service('posts').find({ + paginate: false, + query: { + id: { $in: [1, 2] }, + $sort: { id: 1 } + } + }) const defaultResult = await serviceLoader.load([1, 2]) const selectedResult = await serviceLoader.select(['body']).load([1, 2]) + assert.deepEqual( selectedResult, defaultResult.map((result) => { @@ -182,6 +197,7 @@ describe('serviceLoader.test', () => { } }) ) + assert.deepEqual(mainResult, defaultResult) assert.deepEqual(serviceLoader.cacheMap.size, 1) }) @@ -190,8 +206,18 @@ describe('serviceLoader.test', () => { app, service: 'comments' }) + const result1 = await app.service('comments').find({ + paginate: false, + query: { postId: 1 } + }) + const result2 = await app.service('comments').find({ + paginate: false, + query: { postId: 2 } + }) + const mainResult = [result1, result2] const defaultResult = await serviceLoader.multi('postId').load([1, 2]) const selectedResult = await serviceLoader.multi('postId').select(['text']).load([1, 2]) + assert.deepEqual( selectedResult, defaultResult.map((result) => { @@ -203,6 +229,7 @@ describe('serviceLoader.test', () => { }) }) ) + assert.deepEqual(mainResult, defaultResult) assert.deepEqual(serviceLoader.cacheMap.size, 1) }) From 9bb129b5a7b22abc6e3fdb480a014827005909cf Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Mon, 30 Jan 2023 14:48:05 -0600 Subject: [PATCH 08/11] Throw error on Feathers filters --- src/serviceLoader.js | 19 ++++++++------- src/utils.js | 48 ++++++++++++++++++++++++++----------- tests/serviceLoader.test.js | 1 - 3 files changed, 45 insertions(+), 23 deletions(-) diff --git a/src/serviceLoader.js b/src/serviceLoader.js index 5ceaafa..3b90448 100644 --- a/src/serviceLoader.js +++ b/src/serviceLoader.js @@ -8,9 +8,11 @@ const { uniqueKeys, uniqueResults, uniqueResultsMulti, - assign + _ } = require('./utils') +const filters = ['$limit', '$skip', '$select', '$sort']; + const createDataLoader = ({ appService, key, loaderOptions, multi, method, params }) => { const serviceMethod = method === '_load' ? '_find' : 'find' @@ -32,7 +34,6 @@ const createDataLoader = ({ appService, key, loaderOptions, multi, method, param [key]: { $in: uniqueKeys(keys) } } } - delete loaderParams.query.$limit return appService[serviceMethod](loaderParams).then((result) => getResults(keys, result, key)) }, loaderOptions) } @@ -56,7 +57,7 @@ module.exports = class ServiceLoader { key: appService.options.id, selectFn: selectFn || defaultSelectFn, cacheParamsFn: cacheParamsFn || defaultCacheParamsFn, - loaderOptions: assign({ cacheKeyFn: defaultCacheKeyFn }, loaderOptions) + loaderOptions: _.assign({ cacheKeyFn: defaultCacheKeyFn }, loaderOptions) } } @@ -64,7 +65,7 @@ module.exports = class ServiceLoader { const { appService, loaderOptions, service } = this.options cacheParamsFn = cacheParamsFn || this.options.cacheParamsFn - options = assign( + options = _.assign( { id: null, key: this.options.key, @@ -96,6 +97,10 @@ module.exports = class ServiceLoader { return result } + if (options.params.query && _.has(options.params.query, filters)) { + throw new GeneralError('Loader `load()` method cannot contain ${filters} in the query') + } + // stableStringify does not sort arrays on purpose because // array order matters in most cases. In this case, the // order of ids does not matter to the load function but @@ -251,7 +256,7 @@ class ChainedLoader { } set(options) { - this.options = assign(this.options, options) + this.options = _.assign(this.options, options) return this } @@ -267,9 +272,7 @@ class ChainedLoader { } async exec() { - const options = { ...this.options } - delete options.selection - delete options.loader + const options = _.omit(this.options, ['selection', 'loader']) const result = await this.options.loader.exec(options) return this.handleResult(result) } diff --git a/src/utils.js b/src/utils.js index 17b100f..bafca78 100644 --- a/src/utils.js +++ b/src/utils.js @@ -1,12 +1,42 @@ const { GeneralError } = require('@feathersjs/errors') -const isObject = (obj) => { - if (obj === null || typeof obj !== 'object' || Array.isArray(obj)) { +const _ = { + isObject: (obj) => { + if (obj === null || typeof obj !== 'object' || Array.isArray(obj)) { + return false + } + return Object.getPrototypeOf(obj) === Object.prototype + }, + assign: (target, source) => { + const result = { ...target } + const keys = Object.keys(source); + for (let index = 0; index < keys.length; index++) { + const key = keys[index]; + if (source[key] !== undefined) { + result[key] = source[key] + } + } + return result + }, + omit: (target, keys) => { + const result = { ...target } + for (let index = 0; index < keys.length; index++) { + delete result[keys[index]] + } + return result + }, + has: (target, keys) => { + for (let index = 0; index < keys.length; index++) { + if (Object.prototype.hasOwnProperty.call(target, keys[index])) { + return true + } + } return false } - return Object.getPrototypeOf(obj) === Object.prototype } +module.exports._ = _; + module.exports.stableStringify = (object) => { return JSON.stringify(object, (key, value) => { if (typeof value === 'function') { @@ -15,7 +45,7 @@ module.exports.stableStringify = (object) => { ) } - if (isObject(value)) { + if (_.isObject(value)) { const keys = Object.keys(value).sort() const result = {} for (let index = 0, length = keys.length; index < length; ++index) { @@ -88,16 +118,6 @@ module.exports.defaultSelectFn = (selection, result, options) => { return convertResult(result) } -module.exports.assign = (target, source) => { - const result = { ...target } - Object.keys(source).forEach((key) => { - if (source[key] !== undefined) { - result[key] = source[key] - } - }) - return result -} - module.exports.uniqueKeys = (keys) => { const found = {} const unique = [] diff --git a/tests/serviceLoader.test.js b/tests/serviceLoader.test.js index ed0a82e..0c348fa 100644 --- a/tests/serviceLoader.test.js +++ b/tests/serviceLoader.test.js @@ -1,6 +1,5 @@ const { assert } = require('chai') const { ServiceLoader } = require('../src') -const { stableStringify } = require('../src/utils') const { makeApp } = require('./utils') const testFunc = () => {} From 6b8d9e8583a1f34e23589abd2614e454f1460f7e Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Sat, 8 Jul 2023 16:15:15 -0500 Subject: [PATCH 09/11] Cleanup builder --- src/serviceLoader.js | 44 ++++++++++++++++++++++++-------------------- src/utils.js | 28 ++++++++++++++-------------- 2 files changed, 38 insertions(+), 34 deletions(-) diff --git a/src/serviceLoader.js b/src/serviceLoader.js index 3b90448..1bcbfc1 100644 --- a/src/serviceLoader.js +++ b/src/serviceLoader.js @@ -177,8 +177,8 @@ module.exports = class ServiceLoader { return new ChainedLoader(this, { key }) } - multi(key) { - return new ChainedLoader(this, { key, multi: true }) + multi(multi = true) { + return new ChainedLoader(this, { key: this.options.key, multi }) } select(selection) { @@ -212,68 +212,72 @@ module.exports = class ServiceLoader { class ChainedLoader { constructor(loader, options) { - this.options = { ...options, loader } + this.loader = loader; + this._selectFn = this.loader.options.selectFn; + this.options = { ...options } } key(key) { - return this.set({ key, multi: false }) + return this._set({ key }) } - multi(key) { - return this.set({ key, multi: true }) + multi(multi = true) { + return this._set({ multi }) } - select(selection) { - return this.set({ selection }) + select(selection, selectFn) { + this.selection = selection; + if (selectFn) { + this.selectFn = selectFn + } + return this; } params(cacheParamsFn) { - return this.set({ cacheParamsFn }) + return this._set({ cacheParamsFn }) } async get(id, params) { - return this.set({ method: 'get', id, params }).exec() + return this._set({ method: 'get', id, params }).exec() } async _get(id, params) { - return this.set({ method: '_get', id, params }).exec() + return this._set({ method: '_get', id, params }).exec() } async find(params) { - return this.set({ method: 'find', params }).exec() + return this._set({ method: 'find', params }).exec() } async _find(params) { - return this.set({ method: '_find', params }).exec() + return this._set({ method: '_find', params }).exec() } async load(id, params) { - return this.set({ method: 'load', id, params }).exec() + return this._set({ method: 'load', id, params }).exec() } async _load(id, params) { - return this.set({ method: '_load', id, params }).exec() + return this._set({ method: '_load', id, params }).exec() } - set(options) { + _set(options) { this.options = _.assign(this.options, options) return this } async handleResult(result) { const { selection } = this.options - const { selectFn } = this.options.loader.options if (!result || !selection) { return result } - return await selectFn(selection, result, this.options) + return await this._selectFn(selection, result, this.options) } async exec() { - const options = _.omit(this.options, ['selection', 'loader']) - const result = await this.options.loader.exec(options) + const result = await this.loader.exec(this.options) return this.handleResult(result) } } diff --git a/src/utils.js b/src/utils.js index bafca78..7b1663d 100644 --- a/src/utils.js +++ b/src/utils.js @@ -18,16 +18,24 @@ const _ = { } return result }, - omit: (target, keys) => { - const result = { ...target } + omit: (source, keys) => { + const result = { ...source } for (let index = 0; index < keys.length; index++) { delete result[keys[index]] } return result }, - has: (target, keys) => { + pick: (source, keys) => { + return keys.reduce((result, key) => { + if (source[key] !== undefined) { + result[key] = source[key] + } + return result + }, {}) + }, + has: (source, keys) => { for (let index = 0; index < keys.length; index++) { - if (Object.prototype.hasOwnProperty.call(target, keys[index])) { + if (Object.prototype.hasOwnProperty.call(source, keys[index])) { return true } } @@ -78,24 +86,16 @@ module.exports.defaultCacheKeyFn = (id) => { return id.toString ? id.toString() : String(id) } -const select = (selection, source) => { - return selection.reduce((result, key) => { - if (source[key] !== undefined) { - result[key] = source[key] - } - return result - }, {}) -} module.exports.defaultSelectFn = (selection, result, options) => { if (!Array.isArray(selection)) { - throw new Error('selection must be an array') + throw new Error('The argument to the `.select()` method must be an array when using the default `selectFn` option.') } const { key, method, multi } = options const convertResult = (result) => { - return select([key, ...selection], result) + return _.pick(result, [key, ...selection]) } if (['find', '_find'].includes(method) && result.data) { From 39247226237d275ab6f241ee0881da124e416026 Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Fri, 11 Aug 2023 19:14:28 -0500 Subject: [PATCH 10/11] Lint and cleanup --- src/serviceLoader.js | 75 +++++++++++++++++++++++--------------------- src/utils.js | 11 ++++--- 2 files changed, 46 insertions(+), 40 deletions(-) diff --git a/src/serviceLoader.js b/src/serviceLoader.js index 1bcbfc1..133f819 100644 --- a/src/serviceLoader.js +++ b/src/serviceLoader.js @@ -11,7 +11,7 @@ const { _ } = require('./utils') -const filters = ['$limit', '$skip', '$select', '$sort']; +const filters = ['$limit', '$skip', '$select', '$sort'] const createDataLoader = ({ appService, key, loaderOptions, multi, method, params }) => { const serviceMethod = method === '_load' ? '_find' : 'find' @@ -82,19 +82,19 @@ module.exports = class ServiceLoader { if (['get', '_get', 'find', '_find'].includes(options.method)) { const cacheKey = stringifyKey(options, cacheParamsFn) - const cachedResult = await this.cacheMap.get(cacheKey) + const cachedPromise = await this.cacheMap.get(cacheKey) - if (cachedResult) { - return cachedResult + if (cachedPromise) { + return cachedPromise } - const result = ['get', '_get'].includes(options.method) - ? await appService[options.method](options.id, options.params) - : await appService[options.method](options.params) + const promise = ['get', '_get'].includes(options.method) + ? appService[options.method](options.id, options.params) + : appService[options.method](options.params) - await this.cacheMap.set(cacheKey, result) + await this.cacheMap.set(cacheKey, promise) - return result + return promise } if (options.params.query && _.has(options.params.query, filters)) { @@ -115,10 +115,10 @@ module.exports = class ServiceLoader { cacheParamsFn ) - const cachedResult = await this.cacheMap.get(cacheKey) + const cachedPromise = await this.cacheMap.get(cacheKey) - if (cachedResult) { - return cachedResult + if (cachedPromise) { + return cachedPromise } const loaderConfig = { @@ -140,13 +140,11 @@ module.exports = class ServiceLoader { this.loaders.set(loaderKey, dataLoader) - const result = Array.isArray(sortedId) - ? await dataLoader.loadMany(sortedId) - : await dataLoader.load(sortedId) + const promise = Array.isArray(sortedId) ? dataLoader.loadMany(sortedId) : dataLoader.load(sortedId) - await this.cacheMap.set(cacheKey, result) + await this.cacheMap.set(cacheKey, promise) - return result + return promise } get(id, params) { @@ -177,12 +175,16 @@ module.exports = class ServiceLoader { return new ChainedLoader(this, { key }) } - multi(multi = true) { - return new ChainedLoader(this, { key: this.options.key, multi }) + multi(key) { + return new ChainedLoader(this, { key, multi: true }) } - select(selection) { - return new ChainedLoader(this, { key: this.options.key, selection }) + select(selection, selectFn) { + return new ChainedLoader(this, { + key: this.options.key, + selection, + selectFn + }) } params(cacheParamsFn) { @@ -211,26 +213,29 @@ module.exports = class ServiceLoader { } class ChainedLoader { - constructor(loader, options) { - this.loader = loader; - this._selectFn = this.loader.options.selectFn; - this.options = { ...options } + constructor(loader, { selectFn, selection, ...config }) { + this.loader = loader + this.options = { + selection, + selectFn: selectFn || this.loader.options.selectFn + } + this.config = config } key(key) { return this._set({ key }) } - multi(multi = true) { - return this._set({ multi }) + multi(key) { + return this._set({ multi: true, key }) } select(selection, selectFn) { - this.selection = selection; + this.options.selection = selection if (selectFn) { - this.selectFn = selectFn + this.options.selectFn = selectFn } - return this; + return this } params(cacheParamsFn) { @@ -261,23 +266,23 @@ class ChainedLoader { return this._set({ method: '_load', id, params }).exec() } - _set(options) { - this.options = _.assign(this.options, options) + _set(config) { + this.config = _.assign(this.config, config) return this } async handleResult(result) { - const { selection } = this.options + const { selection, selectFn } = this.options if (!result || !selection) { return result } - return await this._selectFn(selection, result, this.options) + return selectFn(selection, result, this.config) } async exec() { - const result = await this.loader.exec(this.options) + const result = await this.loader.exec(this.config) return this.handleResult(result) } } diff --git a/src/utils.js b/src/utils.js index 7b1663d..8395d22 100644 --- a/src/utils.js +++ b/src/utils.js @@ -9,9 +9,9 @@ const _ = { }, assign: (target, source) => { const result = { ...target } - const keys = Object.keys(source); + const keys = Object.keys(source) for (let index = 0; index < keys.length; index++) { - const key = keys[index]; + const key = keys[index] if (source[key] !== undefined) { result[key] = source[key] } @@ -43,7 +43,7 @@ const _ = { } } -module.exports._ = _; +module.exports._ = _ module.exports.stableStringify = (object) => { return JSON.stringify(object, (key, value) => { @@ -86,10 +86,11 @@ module.exports.defaultCacheKeyFn = (id) => { return id.toString ? id.toString() : String(id) } - module.exports.defaultSelectFn = (selection, result, options) => { if (!Array.isArray(selection)) { - throw new Error('The argument to the `.select()` method must be an array when using the default `selectFn` option.') + throw new Error( + 'The argument to the `.select()` method must be an array when using the default `selectFn` option.' + ) } const { key, method, multi } = options From cb790825c3acb1164fa002d57a8410a201912d1e Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Sat, 8 Jun 2024 18:00:01 -0500 Subject: [PATCH 11/11] Cleanup remove unused methods --- README.md | 44 +++++---- docs/guide.md | 2 +- docs/index.md | 24 ++--- src/appLoader.js | 21 ++-- src/serviceLoader.js | 186 +++++++++--------------------------- src/utils.js | 33 ------- tests/appLoader.test.js | 14 ++- tests/serviceLoader.test.js | 152 ++++------------------------- 8 files changed, 119 insertions(+), 357 deletions(-) diff --git a/README.md b/README.md index 6b9e3fb..f5f82f9 100755 --- a/README.md +++ b/README.md @@ -19,15 +19,15 @@ Please refer to the documentation for more information. ```js Promise.all([ app.service('posts').get(1), - app.service('posts').get(2), - app.service('posts').get(3) + app.service('posts').get(1), + app.service('posts').get(2) ]); ``` is slower than ```js -app.service('posts').find({ query: { id: { $in: [1, 2, 3] } } }); +app.service('posts').find({ query: { id: { $in: [1, 2] } } }); ``` Feathers Dataloader makes it easy and fast to write these kinds of queries. The loader handles coalescing all of the IDs into one request and mapping them back to the proper caller. @@ -37,15 +37,15 @@ const loader = new AppLoader({ app: context.app }); Promise.all([ loader.service('posts').load(1), - loader.service('posts').load(2), - loader.service('posts').load(3) + loader.service('posts').load(1), + loader.service('posts').load(2) ]); ``` is automatically converted to ```js -app.service('posts').find({ query: { id: { $in: [1, 2, 3] } } }); +app.service('posts').find({ query: { id: { $in: [1, 2] } } }); ``` @@ -54,8 +54,6 @@ app.service('posts').find({ query: { id: { $in: [1, 2, 3] } } }); ```js const { AppLoader } = require('feathers-dataloader'); -// See Guide for more information about how to better pass -// loaders from service to service. const initializeLoader = context => { if (context.params.loader) { return context; @@ -71,33 +69,37 @@ app.hooks({ all: [initializeLoader] } }) +``` +Loaders are most commonly used in resolvers like @feathersjs/schema, withResults, or fastJoin. See the Guide section for more information and common usecases. Pass the loader to any and all service/loader calls. This maximizes performance by allowing the loader to reuse its cache and batching mechanism as much as possible. -// Loaders are most commonly used in resolvers like @feathersjs/schema, -// withResults, or fastJoin. See the Guide section for more -// information and common usecases. -// Pass the loader to any and all service/loader calls. This maximizes -// performance by allowing the loader to reuse its cache and -// batching mechanism as much as possible. +```js const { resolveResult, resolve } = require('@feathersjs/schema'); const postResultsResolver = resolve({ properties: { user: async (value, post, context) => { - const { loader } = context.params; - return await loader.service('users').load(post.userId, { loader }); + return context.params.loader + .service('users') + .load(post.userId, { loader }); }, category: async (value, post, context) => { - const { loader } = context.params; - return await loader.service('categories').key('name').load(post.categoryName, { loader }); + return context.params.loader + .service('categories') + .key('name') + .load(post.categoryName, { loader }); }, tags: async (value, post, context) => { const { loader } = context.params; - return await loader.service('tags').load(post.tagIds, { loader }); + return context.params.loader + .service('tags') + .load(post.tagIds, { loader }); }, comments: async (value, post, context) => { - const { loader } = context.params; - return await loader.service('comments').multi('postId').load(post.id, { loader }); + return context.params.loader + .service('comments') + .multi('postId') + .load(post.id, { loader }); } } }); diff --git a/docs/guide.md b/docs/guide.md index 07b1934..8364b95 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -125,7 +125,7 @@ app.service('posts').hooks({ The ServiceLoader's underlying DataLoader takes a `maxBatchSize` option. This option limits the number of ids in the `$in` query. Huge arrays of ids can lead to performance issues and even database lockup. By using the `maxBatchSize` you can break those queries into smaller ones. You should monitor your own application to determine the best number to use for your batch size, but setting some maximum is recommended. ```js -const { AppLoader, ServiceLoader } = require('feathers-dataloader'); +const { AppLoader } = require('feathers-dataloader'); const loader = new AppLoader({ app, maxBatchSize: 100 }); diff --git a/docs/index.md b/docs/index.md index 0038fc2..d2b5105 100644 --- a/docs/index.md +++ b/docs/index.md @@ -83,13 +83,13 @@ app.service('posts').hooks({ }); ``` -The `AppLoader` lazily configures a new `ServiceLoader` per service as you use them. This means that you do not have to configure the lower level `ServiceLoader` classes. But, you can use these classes individually, although it is generally not needed. +The `AppLoader` lazily configures a new `ServiceLoader` per service as you use them. This means that you do not have to configure the lower level `ServiceLoader` classes. You can use these classes individually, although it is generally not needed. ```js const { ServiceLoader } = require('feathers-dataloader'); -const serviceLoader = new ServiceLoader({ app, service: 'users' }); +const serviceLoader = new ServiceLoader({ app, path: 'users' }); const user = await serviceLoader.load(1, params); const user = await serviceLoader.get(1, params); const users = await serviceLoader.find(params); @@ -131,7 +131,7 @@ Create a new app-loader. This is the most commonly used class. | Argument | Type | Default | Description | | --------------- | :--------: | ---------- | ------------------------------------------------ | | `app` | `Object` | | A Feathers app | -| `services` | `Object` | `{}`| An object where each property is a service name and the value is loader options for that service. These options override the `globalLoaderOptions` | +| `services` | `Object` | `{}`| An object where each property is a service name and the value is `loaderOptions` for that service. These options override the `globalLoaderOptions` | | `ServiceLoader` | `Class` | `ServiceLoader`| A base class that will be used to create each ServiceLoader instance | | `globalLoaderOptions` | `Object` | {} | Options that will be assigned to every new `ServiceLoader` | @@ -164,21 +164,23 @@ Create a new app-loader. This is the most commonly used class.

class ServiceLoader( [, options] )

-Create a new service-loader. This class lazily configures underlying `DataLoader` and `FindLoader` for a given service +Create a new service-loader. This class lazily configures underlying `DataLoader` and takes many `DataLoader` options. - **Arguments:** - `{Object} [ options ]` - - `{Object} service` + - `{Object} path` - `{Map} cacheMap` - - `{Map} cacheParamsFn` + - `{Function} cacheParamsFn` + - `{Function} selectFn` - ...loaderOptions | Argument | Type | Default | Description | | --------------- | :--------: | ---------- | ------------------------------------------------ | -| `app` | `Object` | | A Feathers app` | -| `service` | `String` | | The name of the service like "users"` | -| `cacheMap` | `Map` | | A Map like object with methods get, set, and clear to serve as the cache of results.` | +| `app` | `Object` | | A Feathers app | +| `path` | `String` | | The name of the service like "users" | +| `cacheMap` | `Map` | | A Map like object with methods get, set, and clear to serve as the cache of results. | | `cacheParamsFn` | `Function` | defaultCacheParamsFn | A function that returns a JSON stringifiable set or params to be used in the cacheKey. The default function traverses the params and removes any functions` | +| `selectFn` | `Function` | defaultSelectFn | A function that selects or resolves data after it is returned from the cache | | `loaderOptions` | `Object` | {} | See `DataLoader` | @@ -187,7 +189,7 @@ Create a new service-loader. This class lazily configures underlying `DataLoader const loader = new ServiceLoader({ app, - service: 'users' + path: 'users' cacheParamsFn: (params) => { return { userId: params.user.id, @@ -217,7 +219,7 @@ Create a new service-loader. This class lazily configures underlying `DataLoader

class DataLoader( batchLoadFunc [, options] )

-This library re-exports [Dataloader](https://www.npmjs.com/package/dataloader) from its original package. Please see its documentation for more information. `loaderOptions` given to `BatchLoader` will be used to configure Dataloaders. You can also import `Dataloader` along with some helpful utility functions to build custom loaders. +This library re-exports [Dataloader](https://www.npmjs.com/package/dataloader) from its original package. Please see its documentation for more information. `loaderOptions` given to `ServiceLoader` will be used to configure Dataloaders. You can also import `Dataloader` along with some helpful utility functions to build custom loaders. ```js const { DataLoader uniqueResults, uniqueKeys } = require("feathers-dataloader"); diff --git a/src/appLoader.js b/src/appLoader.js index f4b3f61..f502f07 100644 --- a/src/appLoader.js +++ b/src/appLoader.js @@ -2,8 +2,12 @@ const BaseServiceLoader = require('./serviceLoader') module.exports = class AppLoader { constructor({ app, services = {}, ...loaderOptions }) { - this.options = { app, services, loaderOptions } - this.loaders = new Map() + this.options = { + app, + services, + loaderOptions, + loaders: new Map() + } } service(path) { @@ -13,7 +17,7 @@ module.exports = class AppLoader { ...this.options.loaderOptions, ...(this.options.services[path] || {}) } - const cachedLoader = this.loaders.get(path) + const cachedLoader = this.options.loaders.get(path) if (cachedLoader) { return cachedLoader @@ -21,22 +25,23 @@ module.exports = class AppLoader { const loader = new ServiceLoader({ ...loaderOptions, - service: path, + path, app }) - this.loaders.set(path, loader) + this.options.loaders.set(path, loader) return loader } async clear() { + const { loaders } = this.options const promises = [] - this.loaders.forEach((loader) => { - promises.push(loader.cacheMap.clear()) + loaders.forEach((loader) => { + promises.push(loader.clear()) }) await Promise.all(promises) - this.loaders.clear() + loaders.clear() return this } } diff --git a/src/serviceLoader.js b/src/serviceLoader.js index 133f819..7219766 100644 --- a/src/serviceLoader.js +++ b/src/serviceLoader.js @@ -4,19 +4,18 @@ const { stableStringify, defaultCacheParamsFn, defaultCacheKeyFn, - defaultSelectFn, uniqueKeys, uniqueResults, uniqueResultsMulti, _ } = require('./utils') -const filters = ['$limit', '$skip', '$select', '$sort'] +const filters = ['$limit', '$skip', '$sort'] -const createDataLoader = ({ appService, key, loaderOptions, multi, method, params }) => { +const createDataLoader = ({ service, key, loaderOptions, multi, method, params }) => { const serviceMethod = method === '_load' ? '_find' : 'find' - if (!appService[serviceMethod]) { + if (!service[serviceMethod]) { throw new GeneralError( `Cannot create a loader for a service that does not have a ${serviceMethod} method.` ) @@ -34,7 +33,7 @@ const createDataLoader = ({ appService, key, loaderOptions, multi, method, param [key]: { $in: uniqueKeys(keys) } } } - return appService[serviceMethod](loaderParams).then((result) => getResults(keys, result, key)) + return service[serviceMethod](loaderParams).then((result) => getResults(keys, result, key)) }, loaderOptions) } @@ -46,23 +45,23 @@ const stringifyKey = (options, cacheParamsFn) => { } module.exports = class ServiceLoader { - constructor({ app, service, cacheParamsFn, selectFn, cacheMap, ...loaderOptions }) { - this.cacheMap = cacheMap || new Map() - this.loaders = new Map() - const appService = app.service(service) + constructor({ app, path, cacheParamsFn, cacheMap, ...loaderOptions }, state) { + const service = app.service(path) this.options = { app, + path, service, - appService, - key: appService.options.id, - selectFn: selectFn || defaultSelectFn, + key: service.options.id, cacheParamsFn: cacheParamsFn || defaultCacheParamsFn, - loaderOptions: _.assign({ cacheKeyFn: defaultCacheKeyFn }, loaderOptions) + loaderOptions: _.assign({ cacheKeyFn: defaultCacheKeyFn }, loaderOptions), + loaders: new Map(), + cacheMap: cacheMap || new Map() } + this.state = state || {} } async exec({ cacheParamsFn, ...options }) { - const { appService, loaderOptions, service } = this.options + const { path, service, loaderOptions, cacheMap, loaders } = this.options cacheParamsFn = cacheParamsFn || this.options.cacheParamsFn options = _.assign( @@ -75,24 +74,24 @@ module.exports = class ServiceLoader { }, { ...options, - service + path } ) if (['get', '_get', 'find', '_find'].includes(options.method)) { const cacheKey = stringifyKey(options, cacheParamsFn) - const cachedPromise = await this.cacheMap.get(cacheKey) + const cachedPromise = await cacheMap.get(cacheKey) if (cachedPromise) { return cachedPromise } const promise = ['get', '_get'].includes(options.method) - ? appService[options.method](options.id, options.params) - : appService[options.method](options.params) + ? service[options.method](options.id, options.params) + : service[options.method](options.params) - await this.cacheMap.set(cacheKey, promise) + await cacheMap.set(cacheKey, promise) return promise } @@ -101,21 +100,9 @@ module.exports = class ServiceLoader { throw new GeneralError('Loader `load()` method cannot contain ${filters} in the query') } - // stableStringify does not sort arrays on purpose because - // array order matters in most cases. In this case, the - // order of ids does not matter to the load function but - // does to the cache key, thats why these are sorted. - const sortedId = Array.isArray(options.id) ? [...options.id].sort() : options.id + const cacheKey = stringifyKey(options, cacheParamsFn) - const cacheKey = stringifyKey( - { - ...options, - id: sortedId - }, - cacheParamsFn - ) - - const cachedPromise = await this.cacheMap.get(cacheKey) + const cachedPromise = await cacheMap.get(cacheKey) if (cachedPromise) { return cachedPromise @@ -130,70 +117,60 @@ module.exports = class ServiceLoader { const loaderKey = stringifyKey(loaderConfig, cacheParamsFn) - const dataLoader = - this.loaders.get(loaderKey) || - createDataLoader({ - appService, + let dataLoader = loaders.get(loaderKey) + + if (!dataLoader) { + dataLoader = createDataLoader({ + service, loaderOptions, ...loaderConfig }) - this.loaders.set(loaderKey, dataLoader) + loaders.set(loaderKey, dataLoader) + } - const promise = Array.isArray(sortedId) ? dataLoader.loadMany(sortedId) : dataLoader.load(sortedId) + const promise = Array.isArray(options.id) ? dataLoader.loadMany(options.id) : dataLoader.load(options.id) - await this.cacheMap.set(cacheKey, promise) + await cacheMap.set(cacheKey, promise) return promise } get(id, params) { - return this.exec({ method: 'get', id, params }) + return this.exec({ ...this.state, method: 'get', id, params }) } _get(id, params) { - return this.exec({ method: '_get', id, params }) + return this.exec({ ...this.state, method: '_get', id, params }) } find(params) { - return this.exec({ method: 'find', params }) + return this.exec({ ...this.state, method: 'find', params }) } _find(params) { - return this.exec({ method: '_find', params }) + return this.exec({ ...this.state, method: '_find', params }) } load(id, params) { - return this.exec({ method: 'load', id, params }) + return this.exec({ ...this.state, method: 'load', id, params }) } _load(id, params) { - return this.exec({ method: '_load', id, params }) + return this.exec({ ...this.state, method: '_load', id, params }) } key(key) { - return new ChainedLoader(this, { key }) + return new ServiceLoader(this.options, { key }) } multi(key) { - return new ChainedLoader(this, { key, multi: true }) - } - - select(selection, selectFn) { - return new ChainedLoader(this, { - key: this.options.key, - selection, - selectFn - }) - } - - params(cacheParamsFn) { - return new ChainedLoader(this, { key: this.options.key, cacheParamsFn }) + return new ServiceLoader(this.options, { key, multi: true }) } async clear() { - const { service } = this.options - this.loaders.clear() + const { path, loaders, cacheMap } = this.options + loaders.clear() const promises = [] // TODO: This could be a redis store or some other // async storage. That's why there is this for/await @@ -201,88 +178,17 @@ module.exports = class ServiceLoader { // Is it a good idea to just be pushing all the deletes into // one Promise.all() after? Instead, we may want to call // the delete in the iteration. But that would be slower. - for await (const cacheKey of this.cacheMap.keys()) { + // Also note that we don't just clear the whole cache + // and only delete the keys that match the path because + // the user may have shared the cacheMap with other + // services. + for await (const cacheKey of cacheMap.keys()) { const parsedKey = JSON.parse(cacheKey) - if (parsedKey.service === service) { - promises.push(this.cacheMap.delete(cacheKey)) + if (parsedKey.path === path) { + promises.push(cacheMap.delete(cacheKey)) } } await Promise.all(promises) return this } } - -class ChainedLoader { - constructor(loader, { selectFn, selection, ...config }) { - this.loader = loader - this.options = { - selection, - selectFn: selectFn || this.loader.options.selectFn - } - this.config = config - } - - key(key) { - return this._set({ key }) - } - - multi(key) { - return this._set({ multi: true, key }) - } - - select(selection, selectFn) { - this.options.selection = selection - if (selectFn) { - this.options.selectFn = selectFn - } - return this - } - - params(cacheParamsFn) { - return this._set({ cacheParamsFn }) - } - - async get(id, params) { - return this._set({ method: 'get', id, params }).exec() - } - - async _get(id, params) { - return this._set({ method: '_get', id, params }).exec() - } - - async find(params) { - return this._set({ method: 'find', params }).exec() - } - - async _find(params) { - return this._set({ method: '_find', params }).exec() - } - - async load(id, params) { - return this._set({ method: 'load', id, params }).exec() - } - - async _load(id, params) { - return this._set({ method: '_load', id, params }).exec() - } - - _set(config) { - this.config = _.assign(this.config, config) - return this - } - - async handleResult(result) { - const { selection, selectFn } = this.options - - if (!result || !selection) { - return result - } - - return selectFn(selection, result, this.config) - } - - async exec() { - const result = await this.loader.exec(this.config) - return this.handleResult(result) - } -} diff --git a/src/utils.js b/src/utils.js index 8395d22..5766611 100644 --- a/src/utils.js +++ b/src/utils.js @@ -86,39 +86,6 @@ module.exports.defaultCacheKeyFn = (id) => { return id.toString ? id.toString() : String(id) } -module.exports.defaultSelectFn = (selection, result, options) => { - if (!Array.isArray(selection)) { - throw new Error( - 'The argument to the `.select()` method must be an array when using the default `selectFn` option.' - ) - } - - const { key, method, multi } = options - - const convertResult = (result) => { - return _.pick(result, [key, ...selection]) - } - - if (['find', '_find'].includes(method) && result.data) { - return { - ...result, - data: result.data.map(convertResult) - } - } - - if (multi) { - return result.map((result) => { - return result.map(convertResult) - }) - } - - if (Array.isArray(result)) { - return result.map(convertResult) - } - - return convertResult(result) -} - module.exports.uniqueKeys = (keys) => { const found = {} const unique = [] diff --git a/tests/appLoader.test.js b/tests/appLoader.test.js index 7730ae8..b3d389a 100644 --- a/tests/appLoader.test.js +++ b/tests/appLoader.test.js @@ -26,8 +26,6 @@ describe('appLoader.test', () => { assert.isFunction(serviceLoader.load) assert.isFunction(serviceLoader._load) assert.isFunction(serviceLoader.key) - assert.isFunction(serviceLoader.select) - assert.isFunction(serviceLoader.params) assert.isFunction(serviceLoader.multi) assert.isFunction(serviceLoader.exec) assert.isFunction(serviceLoader.clear) @@ -74,13 +72,13 @@ describe('appLoader.test', () => { const commentsLoader = appLoader.service('comments') await postsLoader.load(1) await commentsLoader.load(1) - assert.deepEqual(appLoader.loaders.size, 2) - assert.deepEqual(postsLoader.cacheMap.size, 1) - assert.deepEqual(commentsLoader.cacheMap.size, 1) + assert.deepEqual(appLoader.options.loaders.size, 2) + assert.deepEqual(postsLoader.options.cacheMap.size, 1) + assert.deepEqual(commentsLoader.options.cacheMap.size, 1) await appLoader.clear() - assert.deepEqual(appLoader.loaders.size, 0) - assert.deepEqual(postsLoader.cacheMap.size, 0) - assert.deepEqual(commentsLoader.cacheMap.size, 0) + assert.deepEqual(appLoader.options.loaders.size, 0) + assert.deepEqual(postsLoader.options.cacheMap.size, 0) + assert.deepEqual(commentsLoader.options.cacheMap.size, 0) }) it('takes a base class in global config', () => { diff --git a/tests/serviceLoader.test.js b/tests/serviceLoader.test.js index 0c348fa..520525f 100644 --- a/tests/serviceLoader.test.js +++ b/tests/serviceLoader.test.js @@ -9,7 +9,7 @@ describe('serviceLoader.test', () => { it('creates a serviceLoader', () => { const serviceLoader = new ServiceLoader({ app, - service: 'posts' + path: 'posts' }) assert.isFunction(serviceLoader.get) assert.isFunction(serviceLoader._get) @@ -18,8 +18,6 @@ describe('serviceLoader.test', () => { assert.isFunction(serviceLoader.load) assert.isFunction(serviceLoader._load) assert.isFunction(serviceLoader.key) - assert.isFunction(serviceLoader.select) - assert.isFunction(serviceLoader.params) assert.isFunction(serviceLoader.multi) assert.isFunction(serviceLoader.exec) assert.isFunction(serviceLoader.clear) @@ -28,7 +26,7 @@ describe('serviceLoader.test', () => { it('takes a cacheParamsFn option', () => { const serviceLoader = new ServiceLoader({ app, - service: 'posts', + path: 'posts', cacheParamsFn: testFunc }) assert.deepEqual(serviceLoader.options.cacheParamsFn, testFunc) @@ -37,18 +35,18 @@ describe('serviceLoader.test', () => { it('passes loader options', async () => { const serviceLoader = new ServiceLoader({ app, - service: 'posts', + path: 'posts', cacheKeyFn: testFunc }) await serviceLoader.load(1) - const [dataLoader] = serviceLoader.loaders.values() + const [dataLoader] = serviceLoader.options.loaders.values() assert.deepEqual(dataLoader._cacheKeyFn, testFunc) }) it('works with load(id)', async () => { const serviceLoader = new ServiceLoader({ app, - service: 'posts' + path: 'posts' }) const defaultResult = await app.service('posts').get(1) const result = await serviceLoader.load(1) @@ -58,7 +56,7 @@ describe('serviceLoader.test', () => { it('works with load([id1, id2])', async () => { const serviceLoader = new ServiceLoader({ app, - service: 'posts' + path: 'posts' }) const defaultResult = await Promise.all([app.service('posts').get(1), app.service('posts').get(2)]) const result = await serviceLoader.load([1, 2]) @@ -68,7 +66,7 @@ describe('serviceLoader.test', () => { it('works with key("key").load(id)', async () => { const serviceLoader = new ServiceLoader({ app, - service: 'posts' + path: 'posts' }) const defaultResult = await app.service('posts').get(1) const result = await serviceLoader.key('body').load('John post') @@ -78,7 +76,7 @@ describe('serviceLoader.test', () => { it('works with key("key")._load(id)', async () => { const serviceLoader = new ServiceLoader({ app, - service: 'posts' + path: 'posts' }) const defaultResult = await app.service('posts').get(1) const result = await serviceLoader.key('body')._load('John post') @@ -88,7 +86,7 @@ describe('serviceLoader.test', () => { it('works with multi("key").load(id)', async () => { const serviceLoader = new ServiceLoader({ app, - service: 'comments' + path: 'comments' }) const result = await serviceLoader.multi('postId').load(1) assert.deepEqual(result.length, 3) @@ -97,7 +95,7 @@ describe('serviceLoader.test', () => { it('works with multi("key")._load(id)', async () => { const serviceLoader = new ServiceLoader({ app, - service: 'comments' + path: 'comments' }) const result = await serviceLoader.multi('postId')._load(1) assert.deepEqual(result.length, 3) @@ -106,7 +104,7 @@ describe('serviceLoader.test', () => { it('works with multi("key").load([id1, id2])', async () => { const serviceLoader = new ServiceLoader({ app, - service: 'comments' + path: 'comments' }) const defaultResult = await Promise.all([ app.service('comments').find({ paginate: false, query: { postId: 1 } }), @@ -116,126 +114,10 @@ describe('serviceLoader.test', () => { assert.deepEqual(result, defaultResult) }) - it('works with select(selection).load()', async () => { - const serviceLoader = new ServiceLoader({ - app, - service: 'posts' - }) - const mainResult = await app.service('posts').get(1) - const defaultResult = await serviceLoader.load(1) - const selectedResult = await serviceLoader.select(['body']).load(1) - - assert.deepEqual(selectedResult, { - id: defaultResult.id, - body: defaultResult.body - }) - assert.deepEqual(mainResult, defaultResult) - assert.deepEqual(serviceLoader.cacheMap.size, 1) - }) - - it('works with select(selection).get()', async () => { - const serviceLoader = new ServiceLoader({ - app, - service: 'posts' - }) - const mainResult = await app.service('posts').get(1) - const defaultResult = await serviceLoader.get(1) - const selectedResult = await serviceLoader.select(['body']).get(1) - - assert.deepEqual(selectedResult, { - id: defaultResult.id, - body: defaultResult.body - }) - assert.deepEqual(mainResult, defaultResult) - assert.deepEqual(serviceLoader.cacheMap.size, 1) - }) - - it('works with select(selection).find()', async () => { - const serviceLoader = new ServiceLoader({ - app, - service: 'posts' - }) - const mainResult = await app.service('posts').find() - const defaultResult = await serviceLoader.find() - const selectedResult = await serviceLoader.select(['body']).find() - - assert.deepEqual( - selectedResult, - defaultResult.map((result) => { - return { - id: result.id, - body: result.body - } - }) - ) - assert.deepEqual(mainResult, defaultResult) - assert.deepEqual(serviceLoader.cacheMap.size, 1) - }) - - it('works with select(selection).load([id1, id2])', async () => { - const serviceLoader = new ServiceLoader({ - app, - service: 'posts' - }) - const mainResult = await app.service('posts').find({ - paginate: false, - query: { - id: { $in: [1, 2] }, - $sort: { id: 1 } - } - }) - const defaultResult = await serviceLoader.load([1, 2]) - const selectedResult = await serviceLoader.select(['body']).load([1, 2]) - - assert.deepEqual( - selectedResult, - defaultResult.map((result) => { - return { - id: result.id, - body: result.body - } - }) - ) - assert.deepEqual(mainResult, defaultResult) - assert.deepEqual(serviceLoader.cacheMap.size, 1) - }) - - it('works with select(selection).multi(key).load([id1, id2])', async () => { - const serviceLoader = new ServiceLoader({ - app, - service: 'comments' - }) - const result1 = await app.service('comments').find({ - paginate: false, - query: { postId: 1 } - }) - const result2 = await app.service('comments').find({ - paginate: false, - query: { postId: 2 } - }) - const mainResult = [result1, result2] - const defaultResult = await serviceLoader.multi('postId').load([1, 2]) - const selectedResult = await serviceLoader.multi('postId').select(['text']).load([1, 2]) - - assert.deepEqual( - selectedResult, - defaultResult.map((result) => { - return result.map((result) => { - return { - postId: result.postId, - text: result.text - } - }) - }) - ) - assert.deepEqual(mainResult, defaultResult) - assert.deepEqual(serviceLoader.cacheMap.size, 1) - }) - it('works with get', async () => { const serviceLoader = new ServiceLoader({ app, - service: 'posts' + path: 'posts' }) const defaultResult = await app.service('posts').get(1) const result = await serviceLoader.get(1) @@ -245,7 +127,7 @@ describe('serviceLoader.test', () => { it('works with find', async () => { const serviceLoader = new ServiceLoader({ app, - service: 'posts' + path: 'posts' }) const defaultResult = await app.service('posts').find() const result = await serviceLoader.find() @@ -255,7 +137,7 @@ describe('serviceLoader.test', () => { it('works with underscored methods', async () => { const serviceLoader = new ServiceLoader({ app, - service: 'posts' + path: 'posts' }) const methods = ['_get', '_find', '_load'] let hookCalled = false @@ -278,12 +160,12 @@ describe('serviceLoader.test', () => { const cacheMap = new Map() const postsLoader = new ServiceLoader({ app, - service: 'posts', + path: 'posts', cacheMap: cacheMap }) const commentsLoader = new ServiceLoader({ app, - service: 'comments', + path: 'comments', cacheMap: cacheMap }) @@ -296,6 +178,6 @@ describe('serviceLoader.test', () => { await postsLoader.clear() assert.deepEqual(cacheMap.size, 1) - assert.deepEqual(postsLoader.loaders.size, 0) + assert.deepEqual(postsLoader.options.loaders.size, 0) }) })