From 3809bf20c2e8af596c92e62f71dad31a79fe40b8 Mon Sep 17 00:00:00 2001 From: Sergey Yuferev Date: Tue, 3 Nov 2015 17:21:28 +0300 Subject: [PATCH 1/3] added serializeKey option --- src/__tests__/dataloader-test.js | 38 ++++++++++++++++++++++++++++++++ src/index.js | 25 ++++++++++++++------- 2 files changed, 55 insertions(+), 8 deletions(-) diff --git a/src/__tests__/dataloader-test.js b/src/__tests__/dataloader-test.js index b86e73b..aac9afc 100644 --- a/src/__tests__/dataloader-test.js +++ b/src/__tests__/dataloader-test.js @@ -378,6 +378,44 @@ describe('Accepts any kind of key', () => { }); +describe('Accepts object key in custom serializeKey function', () => { + function serializeKey(key) { + var result; + if (typeof key === 'object' && key !== null) { + result = Object.keys(key).sort().map(k => k + ':' + key[k]).join('-'); + } else { + result = key; + } + return result; + } + + it('Accepts objects with different order of keys', async () => { + var keyA = { a: 123, b: 321 }; + var keyB = { b: 321, a: 123 }; + + var identityLoadCalls = []; + var identityLoader = new DataLoader(keys => { + identityLoadCalls.push(keys); + return Promise.resolve(keys); + }, {serializeKey}); + + // Fetches as expected + + var [ valueA, valueB ] = await Promise.all([ + identityLoader.load(keyA), + identityLoader.load(keyB), + ]); + + expect(valueA).to.equal(keyA); + expect(valueB).to.equal(valueA); + + expect(identityLoadCalls).to.have.length(1); + expect(identityLoadCalls[0]).to.have.length(1); + expect(identityLoadCalls[0][0]).to.equal(keyA); + }); + +}); + describe('Accepts options', () => { // Note: mirrors 'batches multiple requests' above. diff --git a/src/index.js b/src/index.js index b8ca692..87c6cac 100644 --- a/src/index.js +++ b/src/index.js @@ -12,8 +12,9 @@ // of values or Errors. type BatchLoadFn = (keys: Array) => Promise> +type SerializeKeyFn = (key: K) => K // Optionally turn off batching or caching. -type Options = { batch?: boolean, cache?: boolean } +type Options = { batch?: boolean, cache?: boolean, keyFn?: SerializeKeyFn } /** * A `DataLoader` creates a public API for loading data from a particular @@ -39,14 +40,22 @@ export default class DataLoader { this._batchLoadFn = batchLoadFn; this._options = options; this._promiseCache = new Map(); + this._serializeKey = options && options.serializeKey ? + options.serializeKey : + this._defaultSerializeKey; this._queue = []; } // Private _batchLoadFn: BatchLoadFn; _options: ?Options; - _promiseCache: Map>; + _promiseCache: Map>; _queue: LoaderQueue; + _serializeKey: SerializeKeyFn; + + _defaultSerializeKey(key: K): K { + return key; + } /** * Loads a key, returning a `Promise` for the value represented by that key. @@ -58,15 +67,14 @@ export default class DataLoader { `but got: ${key}.` ); } - // Determine options var options = this._options; var shouldBatch = !options || options.batch !== false; var shouldCache = !options || options.cache !== false; - + var cacheKey = this._serializeKey(key); // If caching and there is a cache-hit, return cached Promise. - if (shouldCache && this._promiseCache.has(key)) { - return this._promiseCache.get(key); + if (shouldCache && this._promiseCache.has(cacheKey)) { + return this._promiseCache.get(cacheKey); } // Otherwise, produce a new Promise for this value. @@ -90,7 +98,7 @@ export default class DataLoader { // If caching, cache this promise. if (shouldCache) { - this._promiseCache.set(key, promise); + this._promiseCache.set(cacheKey, promise); } return promise; @@ -124,7 +132,7 @@ export default class DataLoader { * method chaining. */ clear(key: K): DataLoader { - this._promiseCache.delete(key); + this._promiseCache.delete(this._serializeKey(key)); return this; } @@ -204,6 +212,7 @@ function dispatchQueue(loader: DataLoader) { `not return a Promise of an Array: ${values}.` ); } + if (values.length !== keys.length) { throw new Error( 'DataLoader must be constructed with a function which accepts ' + From 574eb39479e2ebe44cd07fcf5b29baf6a1b734b4 Mon Sep 17 00:00:00 2001 From: Sergey Yuferev Date: Tue, 3 Nov 2015 17:24:51 +0300 Subject: [PATCH 2/3] fix --- src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index 87c6cac..7c8a98e 100644 --- a/src/index.js +++ b/src/index.js @@ -49,7 +49,7 @@ export default class DataLoader { // Private _batchLoadFn: BatchLoadFn; _options: ?Options; - _promiseCache: Map>; + _promiseCache: Map>; _queue: LoaderQueue; _serializeKey: SerializeKeyFn; From 13c57b6e78f8b8d43ac86f38828aa6c36e3fa11f Mon Sep 17 00:00:00 2001 From: Sergey Yuferev Date: Thu, 19 Nov 2015 13:53:37 +0300 Subject: [PATCH 3/3] added test: fixed coverage regression --- src/__tests__/dataloader-test.js | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/__tests__/dataloader-test.js b/src/__tests__/dataloader-test.js index aac9afc..9e6ef7f 100644 --- a/src/__tests__/dataloader-test.js +++ b/src/__tests__/dataloader-test.js @@ -381,14 +381,26 @@ describe('Accepts any kind of key', () => { describe('Accepts object key in custom serializeKey function', () => { function serializeKey(key) { var result; - if (typeof key === 'object' && key !== null) { + if (typeof key === 'object') { result = Object.keys(key).sort().map(k => k + ':' + key[k]).join('-'); } else { - result = key; + result = String(key); } return result; } + it('Accepts objects with simple key', async () => { + var keyA = '1234'; + var identityLoadCalls = []; + var identityLoader = new DataLoader(keys => { + identityLoadCalls.push(keys); + return Promise.resolve(keys); + }, {serializeKey}); + + var valueA = await identityLoader.load(keyA); + expect(valueA).to.equal(keyA); + }); + it('Accepts objects with different order of keys', async () => { var keyA = { a: 123, b: 321 }; var keyB = { b: 321, a: 123 };