From 0106bb8842724ede76a2aacea87dabe8b9a3ad41 Mon Sep 17 00:00:00 2001 From: Ryan Goree Date: Thu, 4 Mar 2021 07:41:07 -0600 Subject: [PATCH 01/10] Add findByFields method --- src/cache.js | 98 ++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 79 insertions(+), 19 deletions(-) diff --git a/src/cache.js b/src/cache.js index ffcb19a..7f16c36 100644 --- a/src/cache.js +++ b/src/cache.js @@ -16,28 +16,62 @@ const stringToId = str => { return str } +const fieldToDocField = key => key === 'id' ? '_id' : key // https://github.com/graphql/dataloader#batch-function -const orderDocs = ids => docs => { - const idMap = {} - docs.forEach(doc => { - idMap[idToString(doc._id)] = doc +// "The Array of values must be the same length as the Array of keys." +// "Each index in the Array of values must correspond to the same index in the Array of keys." +const orderDocs = fieldsArray => docs => fieldsArray.map(fields => + docs.filter(doc => { + for (let fieldName of Object.keys(fields)) { + const fieldValue = fields[fieldName] + if (typeof fieldValue === 'undefined') continue + const filterValuesArr = Array.isArray(fieldValue) + ? fieldValue.map(val => idToString(val)) + : [idToString(fieldValue)] + const docValue = doc[fieldToDocField(fieldName)] + const docValuesArr = Array.isArray(docValue) + ? docValue.map(val => idToString(val)) + : [idToString(docValue)] + let isMatch = false + for (const filterVal of filterValuesArr) { + if (docValuesArr.includes(filterVal)) { + isMatch = true + } + } + if (!isMatch) return false + } + return true }) - return ids.map(id => idMap[idToString(id)]) -} +) export const createCachingMethods = ({ collection, model, cache }) => { - const loader = new DataLoader(ids => { - const filter = { - _id: { - $in: ids.map(stringToId) + const loader = new DataLoader(JSONArray => { + let fieldsArray = JSONArray.map(JSON.parse); + const filter = fieldsArray.reduce((filter, fields) => { + + for (const fieldName of Object.keys(fields)) { + if (typeof fields[fieldName] === 'undefined') continue + const docFieldName = fieldToDocField(fieldName) + if (!filter[docFieldName]) filter[docFieldName] = { $in: [] } + let newVals = Array.isArray(fields[fieldName]) + ? fields[fieldName] + : [fields[fieldName]] + if (docFieldName === '_id') newVals = newVals.map(stringToId) + filter[docFieldName].$in = [ + ...filter[docFieldName].$in, + ...newVals.filter(val => !filter[docFieldName].$in.includes(val)) + ] } - } + + return filter + }, {}) + const promise = model ? model.find(filter).exec() : collection.find(filter).toArray() - return promise.then(orderDocs(ids)) + return promise.then(orderDocs(fieldsArray)) }) const cachePrefix = `mongo-${getCollection(collection).collectionName}-` @@ -51,23 +85,49 @@ export const createCachingMethods = ({ collection, model, cache }) => { return EJSON.parse(cacheDoc) } - const doc = await loader.load(idToString(id)) + const doc = await loader.load(JSON.stringify({ id: id })) if (Number.isInteger(ttl)) { // https://github.com/apollographql/apollo-server/tree/master/packages/apollo-server-caching#apollo-server-caching - cache.set(key, EJSON.stringify(doc), { ttl }) + cache.set(key, EJSON.stringify(doc[0]), { ttl }) } - return doc + return doc[0] }, findManyByIds: (ids, { ttl } = {}) => { return Promise.all(ids.map(id => methods.findOneById(id, { ttl }))) }, + findByFields: async (fields, { ttl } = {}) => { + + const cleanedFields = {}; + + Object.keys(fields).forEach(key => { + if (typeof key !== 'undefined') { + cleanedFields[key] = fields[key] + } + }) + + const loaderJSON = JSON.stringify(cleanedFields); + + const key = cachePrefix + loaderJSON + + const cacheDoc = await cache.get(key) + if (cacheDoc) { + return EJSON.parse(cacheDoc) + } + + const doc = await loader.load(loaderJSON) + if (Number.isInteger(ttl)) { + // https://github.com/apollographql/apollo-server/tree/master/packages/apollo-server-caching#apollo-server-caching + cache.set(key, EJSON.stringify(doc), { ttl }) + } + + return doc + }, deleteFromCacheById: async id => { - const stringId = idToString(id) - loader.clear(stringId) - await cache.delete(cachePrefix + stringId) + loader.clear(JSON.stringify({ id: id })) + await cache.delete(cachePrefix + idToString(id)) } } return methods -} +} \ No newline at end of file From c3459a1c7d761bd760b9d3176a978e918132e323 Mon Sep 17 00:00:00 2001 From: Ryan Goree Date: Thu, 4 Mar 2021 07:42:38 -0600 Subject: [PATCH 02/10] Prettify, fix filters mixing, and improve caching --- src/cache.js | 105 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 66 insertions(+), 39 deletions(-) diff --git a/src/cache.js b/src/cache.js index 7f16c36..705cc64 100644 --- a/src/cache.js +++ b/src/cache.js @@ -16,41 +16,47 @@ const stringToId = str => { return str } -const fieldToDocField = key => key === 'id' ? '_id' : key +const fieldToDocField = key => (key === 'id' ? '_id' : key) // https://github.com/graphql/dataloader#batch-function // "The Array of values must be the same length as the Array of keys." // "Each index in the Array of values must correspond to the same index in the Array of keys." -const orderDocs = fieldsArray => docs => fieldsArray.map(fields => - docs.filter(doc => { - for (let fieldName of Object.keys(fields)) { - const fieldValue = fields[fieldName] - if (typeof fieldValue === 'undefined') continue - const filterValuesArr = Array.isArray(fieldValue) - ? fieldValue.map(val => idToString(val)) - : [idToString(fieldValue)] - const docValue = doc[fieldToDocField(fieldName)] - const docValuesArr = Array.isArray(docValue) - ? docValue.map(val => idToString(val)) - : [idToString(docValue)] - let isMatch = false - for (const filterVal of filterValuesArr) { - if (docValuesArr.includes(filterVal)) { - isMatch = true +const orderDocs = fieldsArray => docs => + fieldsArray.map(fields => + docs.filter(doc => { + for (let fieldName of Object.keys(fields)) { + const fieldValue = fields[fieldName] + if (typeof fieldValue === 'undefined') continue + const filterValuesArr = Array.isArray(fieldValue) + ? fieldValue.map(val => idToString(val)) + : [idToString(fieldValue)] + const docValue = doc[fieldToDocField(fieldName)] + const docValuesArr = Array.isArray(docValue) + ? docValue.map(val => idToString(val)) + : [idToString(docValue)] + let isMatch = false + for (const filterVal of filterValuesArr) { + if (docValuesArr.includes(filterVal)) { + isMatch = true + } } + if (!isMatch) return false } - if (!isMatch) return false - } - return true - }) -) + return true + }) + ) export const createCachingMethods = ({ collection, model, cache }) => { const loader = new DataLoader(JSONArray => { - let fieldsArray = JSONArray.map(JSON.parse); - const filter = fieldsArray.reduce((filter, fields) => { - - for (const fieldName of Object.keys(fields)) { + const fieldsArray = JSONArray.map(JSON.parse) + const filterArray = fieldsArray.reduce((filterArray, fields) => { + const existingFieldsFilter = filterArray.find( + filter => + [...Object.keys(filter)].sort().join() === + [...Object.keys(fields)].sort().join() + ) + const filter = existingFieldsFilter || {} + for (const fieldName in fields) { if (typeof fields[fieldName] === 'undefined') continue const docFieldName = fieldToDocField(fieldName) if (!filter[docFieldName]) filter[docFieldName] = { $in: [] } @@ -63,10 +69,15 @@ export const createCachingMethods = ({ collection, model, cache }) => { ...newVals.filter(val => !filter[docFieldName].$in.includes(val)) ] } - - return filter - }, {}) - + if (existingFieldsFilter) return filterArray + return [...filterArray, filter] + }, []) + const filter = + filterArray.length === 1 + ? filterArray[0] + : { + $or: filterArray + } const promise = model ? model.find(filter).exec() : collection.find(filter).toArray() @@ -85,20 +96,19 @@ export const createCachingMethods = ({ collection, model, cache }) => { return EJSON.parse(cacheDoc) } - const doc = await loader.load(JSON.stringify({ id: id })) + const docs = await loader.load(JSON.stringify({ id: id })) if (Number.isInteger(ttl)) { // https://github.com/apollographql/apollo-server/tree/master/packages/apollo-server-caching#apollo-server-caching - cache.set(key, EJSON.stringify(doc[0]), { ttl }) + cache.set(key, EJSON.stringify(docs[0]), { ttl }) } - return doc[0] + return docs[0] }, findManyByIds: (ids, { ttl } = {}) => { return Promise.all(ids.map(id => methods.findOneById(id, { ttl }))) }, findByFields: async (fields, { ttl } = {}) => { - - const cleanedFields = {}; + const cleanedFields = {} Object.keys(fields).forEach(key => { if (typeof key !== 'undefined') { @@ -106,7 +116,7 @@ export const createCachingMethods = ({ collection, model, cache }) => { } }) - const loaderJSON = JSON.stringify(cleanedFields); + const loaderJSON = JSON.stringify(cleanedFields) const key = cachePrefix + loaderJSON @@ -115,13 +125,30 @@ export const createCachingMethods = ({ collection, model, cache }) => { return EJSON.parse(cacheDoc) } - const doc = await loader.load(loaderJSON) + const fieldNames = Object.keys(cleanedFields) + let docs + + if (fieldNames.length === 1) { + const field = cleanedFields[fieldNames[0]] + const fieldArray = Array.isArray(field) ? field : [field] + const docsArray = await Promise.all( + fieldArray.map(value => { + const filter = {} + filter[fieldNames[0]] = value + return loader.load(JSON.stringify(filter)) + }) + ) + docs = [].concat(...docsArray) + } else { + docs = await loader.load(loaderJSON) + } + if (Number.isInteger(ttl)) { // https://github.com/apollographql/apollo-server/tree/master/packages/apollo-server-caching#apollo-server-caching - cache.set(key, EJSON.stringify(doc), { ttl }) + cache.set(key, EJSON.stringify(docs), { ttl }) } - return doc + return docs }, deleteFromCacheById: async id => { loader.clear(JSON.stringify({ id: id })) From 6be5fbb35b8419e1975f9722eb820afa558c0a89 Mon Sep 17 00:00:00 2001 From: Ryan Goree Date: Thu, 4 Mar 2021 07:43:43 -0600 Subject: [PATCH 03/10] Remove bad test While the key didn't get cached in the new InMemoryLRUCache that's passed to createCachingMethods, it did get cached by the DataLoader's memoization cache since .load() is called twice with the same key. See https://github.com/graphql/dataloader#caching --- src/__tests__/cache.test.js | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/__tests__/cache.test.js b/src/__tests__/cache.test.js index 40e94e0..3a539ff 100644 --- a/src/__tests__/cache.test.js +++ b/src/__tests__/cache.test.js @@ -89,14 +89,6 @@ describe('createCachingMethods', () => { expect(collection.find.mock.calls.length).toBe(1) }) - // TODO why doesn't this pass? - // it.only(`doesn't cache without ttl`, async () => { - // await api.findOneById(docs.id1._id) - // await api.findOneById(docs.id1._id) - - // expect(collection.find.mock.calls.length).toBe(2) - // }) - it(`doesn't cache without ttl`, async () => { await api.findOneById(docs.one._id) From dd75571d81796e8f7ae5601ae06ee941f5b49716 Mon Sep 17 00:00:00 2001 From: Ryan Goree Date: Thu, 4 Mar 2021 07:44:46 -0600 Subject: [PATCH 04/10] Add findByFields tests --- src/__tests__/cache.test.js | 144 ++++++++++++++++++++++++++++-------- 1 file changed, 115 insertions(+), 29 deletions(-) diff --git a/src/__tests__/cache.test.js b/src/__tests__/cache.test.js index 3a539ff..1df6d42 100644 --- a/src/__tests__/cache.test.js +++ b/src/__tests__/cache.test.js @@ -9,15 +9,19 @@ const hexId = 'aaaa0000bbbb0000cccc0000' const docs = { one: { - _id: ObjectId(hexId) + _id: ObjectId(hexId), + foo: 'bar', + tags: ['foo', 'bar'] }, two: { - _id: ObjectId() + _id: ObjectId(), + foo: 'bar' } } const stringDoc = { - _id: 's2QBCnv6fXv5YbjAP' + _id: 's2QBCnv6fXv5YbjAP', + tags: ['bar', 'baz'] } const collectionName = 'test' @@ -31,30 +35,55 @@ describe('createCachingMethods', () => { beforeEach(() => { collection = { collectionName, - find: jest.fn(({ _id: { $in: ids } }) => ({ - toArray: () => - new Promise(resolve => { - setTimeout( - () => - resolve( - ids.map(id => { - if (id === stringDoc._id) { - return stringDoc - } - - if (id.equals(docs.one._id)) { - return docs.one - } - - if (id.equals(docs.two._id)) { - return docs.two - } - }) - ), - 0 - ) - }) - })) + find: jest.fn(filter => { + return { + toArray: () => + new Promise(resolve => { + setTimeout( + () => + resolve( + [docs.one, docs.two, stringDoc].filter(doc => { + for (const orFilter of filter.$or || [filter]) { + for (const field in orFilter) { + if (field === '_id') { + for (const id of orFilter._id.$in) { + if (id.equals && !id.equals(doc._id)) { + break + } else if ( + doc._id.equals && + !doc._id.equals(id) + ) { + break + } else if ( + !id.equals && + !doc._id.equals && + id !== doc._id + ) { + break + } + } + } else if (Array.isArray(doc[field])) { + for (const value of orFilter[field].$in) { + if (!doc[field].includes(value)) { + break + } + } + } else if ( + !orFilter[field].$in.includes(doc[field]) + ) { + break + } + } + return true + } + return false + }) + ), + 0 + ) + }) + } + }) } cache = new InMemoryLRUCache() @@ -65,10 +94,11 @@ describe('createCachingMethods', () => { it('adds the right methods', () => { expect(api.findOneById).toBeDefined() expect(api.findManyByIds).toBeDefined() + expect(api.findByFields).toBeDefined() expect(api.deleteFromCacheById).toBeDefined() }) - it('finds one', async () => { + it('finds one with ObjectId', async () => { const doc = await api.findOneById(docs.one._id) expect(doc).toBe(docs.one) expect(collection.find.mock.calls.length).toBe(1) @@ -89,6 +119,62 @@ describe('createCachingMethods', () => { expect(collection.find.mock.calls.length).toBe(1) }) + it('finds by field', async () => { + const foundDocs = await api.findByFields({ foo: 'bar' }) + + expect(foundDocs[0]).toBe(docs.one) + expect(foundDocs[1]).toBe(docs.two) + expect(foundDocs.length).toBe(2) + + expect(collection.find.mock.calls.length).toBe(1) + }) + + it('finds by array field', async () => { + const foundDocs = await api.findByFields({ tags: 'bar' }) + + expect(foundDocs[0]).toBe(docs.one) + expect(foundDocs[1]).toBe(stringDoc) + expect(foundDocs.length).toBe(2) + + expect(collection.find.mock.calls.length).toBe(1) + }) + + it('finds by mutiple fields', async () => { + const foundDocs = await api.findByFields({ + tags: ['foo', 'bar'], + foo: 'bar' + }) + + expect(foundDocs[0]).toBe(docs.one) + expect(foundDocs.length).toBe(1) + + expect(collection.find.mock.calls.length).toBe(1) + }) + + it(`doesn't mix filters of pending calls for different fields`, async () => { + const pendingDocs1 = api.findByFields({ foo: 'bar' }) + const pendingDocs2 = api.findByFields({ tags: 'baz' }) + const [foundDocs1, foundDocs2] = await Promise.all([ + pendingDocs1, + pendingDocs2 + ]) + + expect(foundDocs1[0]).toBe(docs.one) + expect(foundDocs1[1]).toBe(docs.two) + expect(foundDocs1.length).toBe(2) + expect(foundDocs2[0]).toBe(stringDoc) + expect(foundDocs2.length).toBe(1) + + expect(collection.find.mock.calls.length).toBe(1) + }) + + it(`caches each value individually when finding by a single field`, async () => { + await api.findByFields({ tags: ['foo', 'baz'] }) + await api.findByFields({ tags: 'foo' }) + + expect(collection.find.mock.calls.length).toBe(1) + }) + it(`doesn't cache without ttl`, async () => { await api.findOneById(docs.one._id) @@ -141,4 +227,4 @@ describe('createCachingMethods', () => { expect(collection.find).toHaveBeenCalled() } }) -}) +}) \ No newline at end of file From e8f12f92adfedc2813cc3a10dc06901006bac900 Mon Sep 17 00:00:00 2001 From: Ryan Goree Date: Thu, 4 Mar 2021 07:45:39 -0600 Subject: [PATCH 05/10] Add findByFields to ts module --- index.d.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/index.d.ts b/index.d.ts index 783c07a..7c7c14b 100644 --- a/index.d.ts +++ b/index.d.ts @@ -16,6 +16,10 @@ declare module 'apollo-datasource-mongodb' { export type ModelOrCollection = T extends Document ? Model : Collection + + export interface Fields { + [fieldName: string]: string | number | boolean | [string | number | boolean] + } export interface Options { ttl: number @@ -40,6 +44,11 @@ declare module 'apollo-datasource-mongodb' { options?: Options ): Promise<(TData | null | undefined)[]> + findByFields( + fields: Fields, + options?: Options + ): Promise<(TData | null | undefined)[]> + deleteFromCacheById(id: ObjectId | string): Promise } -} +} \ No newline at end of file From 92d6ee99b279d6e7d8130289c17c65620f615606 Mon Sep 17 00:00:00 2001 From: Ryan Goree Date: Thu, 4 Mar 2021 09:27:30 -0600 Subject: [PATCH 06/10] Add findByFields to README --- README.md | 39 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 005bee9..1a99bd5 100644 --- a/README.md +++ b/README.md @@ -6,10 +6,11 @@ Apollo [data source](https://www.apollographql.com/docs/apollo-server/features/d npm i apollo-datasource-mongodb ``` -This package uses [DataLoader](https://github.com/graphql/dataloader) for batching and per-request memoization caching. It also optionally (if you provide a `ttl`) does shared application-level caching (using either the default Apollo `InMemoryLRUCache` or the [cache you provide to ApolloServer()](https://www.apollographql.com/docs/apollo-server/features/data-sources#using-memcachedredis-as-a-cache-storage-backend)). It does this only for these two methods: +This package uses [DataLoader](https://github.com/graphql/dataloader) for batching and per-request memoization caching. It also optionally (if you provide a `ttl`) does shared application-level caching (using either the default Apollo `InMemoryLRUCache` or the [cache you provide to ApolloServer()](https://www.apollographql.com/docs/apollo-server/features/data-sources#using-memcachedredis-as-a-cache-storage-backend)). It does this for the following methods: - [`findOneById(id, options)`](#findonebyid) - [`findManyByIds(ids, options)`](#findmanybyids) +- [`findByFields(fields, options)`](#findbyfields) @@ -24,6 +25,8 @@ This package uses [DataLoader](https://github.com/graphql/dataloader) for batchi - [API](#api) - [findOneById](#findonebyid) - [findManyByIds](#findmanybyids) + - [findByFields](#findbyfields) + - [Examples:](#examples) - [deleteFromCacheById](#deletefromcachebyid) @@ -183,6 +186,7 @@ interface UserDocument { username: string password: string email: string + interests: [string] } // This is optional @@ -236,6 +240,39 @@ Resolves to the found document. Uses DataLoader to load `id`. DataLoader uses `c Calls [`findOneById()`](#findonebyid) for each id. Resolves to an array of documents. +### findByFields + +`this.findByFields(fields, { ttl })` + +Resolves to an array of documents matching the passed fields. + +fields has type `{ [fieldName: string]: string | number | boolean | [string | number | boolean] }`. + +#### Examples: + +```js + + // get user by username + // `collection.find({ username: $in: ['testUser'] })` + this.getByFields({ + username: 'testUser' + }) + + // get all users with either the "gaming" OR "games" interest + // `collection.find({ interests: $in: ['gaming', 'games'] })` + this.getByFields({ + interests: ['gaming', 'games'] + }) + + // get user by username AND with either the "gaming" OR "games" interest + // `collection.find({ username: $in: ['testUser'], interests: $in: ['gaming', 'games'] })` + this.getByFields({ + username: 'testUser', + interests: ['gaming', 'games'] + }) + +``` + ### deleteFromCacheById `this.deleteFromCacheById(id)` From 3a7d5e52f24156b467568e79604b7b8a9231aaa1 Mon Sep 17 00:00:00 2001 From: Ryan Goree Date: Thu, 4 Mar 2021 11:10:47 -0600 Subject: [PATCH 07/10] Ensure non-array field value cache keys are saved as arrays --- src/cache.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/cache.js b/src/cache.js index 705cc64..b224c6c 100644 --- a/src/cache.js +++ b/src/cache.js @@ -112,7 +112,9 @@ export const createCachingMethods = ({ collection, model, cache }) => { Object.keys(fields).forEach(key => { if (typeof key !== 'undefined') { - cleanedFields[key] = fields[key] + cleanedFields[key] = Array.isArray(fields[key]) + ? fields[key] + : [fields[key]] } }) @@ -157,4 +159,4 @@ export const createCachingMethods = ({ collection, model, cache }) => { } return methods -} \ No newline at end of file +} From bc4533c9a7ea318afe8135cc80c0f773b8b01737 Mon Sep 17 00:00:00 2001 From: Ryan Goree Date: Thu, 4 Mar 2021 11:15:23 -0600 Subject: [PATCH 08/10] Add more cache testing for findByFields --- src/__tests__/cache.test.js | 51 +++++++++++++++++++++++++++++-------- 1 file changed, 41 insertions(+), 10 deletions(-) diff --git a/src/__tests__/cache.test.js b/src/__tests__/cache.test.js index 1df6d42..3113b23 100644 --- a/src/__tests__/cache.test.js +++ b/src/__tests__/cache.test.js @@ -25,7 +25,20 @@ const stringDoc = { } const collectionName = 'test' -const cacheKey = id => `mongo-${collectionName}-${idToString(id)}` +const cacheKeyById = id => `mongo-${collectionName}-${idToString(id)}` +const cacheKeyByFields = fields => { + const cleanedFields = {} + + Object.keys(fields).forEach(key => { + if (typeof key !== 'undefined') { + cleanedFields[key] = Array.isArray(fields[key]) + ? fields[key] + : [fields[key]] + } + }) + + return `mongo-${collectionName}-${JSON.stringify(cleanedFields)}` +} describe('createCachingMethods', () => { let collection @@ -168,34 +181,52 @@ describe('createCachingMethods', () => { expect(collection.find.mock.calls.length).toBe(1) }) - it(`caches each value individually when finding by a single field`, async () => { - await api.findByFields({ tags: ['foo', 'baz'] }) - await api.findByFields({ tags: 'foo' }) + it(`dataloader caches each value individually when finding by a single field`, async () => { + await api.findByFields({ tags: ['foo', 'baz'] }, { ttl: 1 }) + + const fooBazCacheValue = await cache.get( + cacheKeyByFields({ tags: ['foo', 'baz'] }) + ) + const fooCacheValue = await cache.get(cacheKeyByFields({ tags: 'foo' })) + expect(fooBazCacheValue).toBeDefined() + expect(fooCacheValue).toBeUndefined() + + await api.findByFields({ tags: 'foo' }) expect(collection.find.mock.calls.length).toBe(1) }) it(`doesn't cache without ttl`, async () => { await api.findOneById(docs.one._id) - const value = await cache.get(cacheKey(docs.one._id)) + const value = await cache.get(cacheKeyById(docs.one._id)) expect(value).toBeUndefined() }) it(`caches`, async () => { await api.findOneById(docs.one._id, { ttl: 1 }) - const value = await cache.get(cacheKey(docs.one._id)) + const value = await cache.get(cacheKeyById(docs.one._id)) expect(value).toEqual(EJSON.stringify(docs.one)) await api.findOneById(docs.one._id) expect(collection.find.mock.calls.length).toBe(1) }) + it(`caches non-array field values as arrays`, async () => { + const fields = { tags: 'foo' } + await api.findByFields(fields, { ttl: 1 }) + const value = await cache.get(cacheKeyByFields(fields)) + expect(value).toEqual(EJSON.stringify([docs.one])) + + await api.findByFields({ tags: ['foo'] }) + expect(collection.find.mock.calls.length).toBe(1) + }) + it(`caches with ttl`, async () => { await api.findOneById(docs.one._id, { ttl: 1 }) await wait(1001) - const value = await cache.get(cacheKey(docs.one._id)) + const value = await cache.get(cacheKeyById(docs.one._id)) expect(value).toBeUndefined() }) @@ -203,12 +234,12 @@ describe('createCachingMethods', () => { for (const doc of [docs.one, docs.two, stringDoc]) { await api.findOneById(doc._id, { ttl: 1 }) - const valueBefore = await cache.get(cacheKey(doc._id)) + const valueBefore = await cache.get(cacheKeyById(doc._id)) expect(valueBefore).toEqual(EJSON.stringify(doc)) await api.deleteFromCacheById(doc._id) - const valueAfter = await cache.get(cacheKey(doc._id)) + const valueAfter = await cache.get(cacheKeyById(doc._id)) expect(valueAfter).toBeUndefined() } }) @@ -227,4 +258,4 @@ describe('createCachingMethods', () => { expect(collection.find).toHaveBeenCalled() } }) -}) \ No newline at end of file +}) From 75ad46f0e0d6cb310d7407d1a1bfa2ec300122c1 Mon Sep 17 00:00:00 2001 From: Ryan Goree Date: Fri, 12 Mar 2021 03:02:12 -0600 Subject: [PATCH 09/10] Clean up code --- src/cache.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cache.js b/src/cache.js index b224c6c..a23f46e 100644 --- a/src/cache.js +++ b/src/cache.js @@ -47,8 +47,8 @@ const orderDocs = fieldsArray => docs => ) export const createCachingMethods = ({ collection, model, cache }) => { - const loader = new DataLoader(JSONArray => { - const fieldsArray = JSONArray.map(JSON.parse) + const loader = new DataLoader(jsonArray => { + const fieldsArray = jsonArray.map(JSON.parse) const filterArray = fieldsArray.reduce((filterArray, fields) => { const existingFieldsFilter = filterArray.find( filter => @@ -96,7 +96,7 @@ export const createCachingMethods = ({ collection, model, cache }) => { return EJSON.parse(cacheDoc) } - const docs = await loader.load(JSON.stringify({ id: id })) + const docs = await loader.load(JSON.stringify({ id })) if (Number.isInteger(ttl)) { // https://github.com/apollographql/apollo-server/tree/master/packages/apollo-server-caching#apollo-server-caching cache.set(key, EJSON.stringify(docs[0]), { ttl }) @@ -153,7 +153,7 @@ export const createCachingMethods = ({ collection, model, cache }) => { return docs }, deleteFromCacheById: async id => { - loader.clear(JSON.stringify({ id: id })) + loader.clear(JSON.stringify({ id })) await cache.delete(cachePrefix + idToString(id)) } } From 2a6e36555fa67070e2bbd89992309708b751504a Mon Sep 17 00:00:00 2001 From: Ryan Goree Date: Fri, 12 Mar 2021 03:55:59 -0600 Subject: [PATCH 10/10] Sort fields before caching or calling loader Co-authored-by: Aditya Thakral <9at8@users.noreply.github.com> --- src/cache.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cache.js b/src/cache.js index a23f46e..02aaffe 100644 --- a/src/cache.js +++ b/src/cache.js @@ -110,7 +110,7 @@ export const createCachingMethods = ({ collection, model, cache }) => { findByFields: async (fields, { ttl } = {}) => { const cleanedFields = {} - Object.keys(fields).forEach(key => { + Object.keys(fields).sort().forEach(key => { if (typeof key !== 'undefined') { cleanedFields[key] = Array.isArray(fields[key]) ? fields[key]