Add support for string ids; fix ObjectId caching

Author: lorensr

index.d.ts

@@ -31,15 +31,15 @@ declare module 'apollo-datasource-mongodb' {
     constructor(modelOrCollection: ModelOrCollection<TData>)
 
     findOneById(
-      id: ObjectId,
+      id: ObjectId | string,
       options?: Options
     ): Promise<TData | null | undefined>
 
     findManyByIds(
-      ids: ObjectId[],
+      ids: (ObjectId | string)[],
       options?: Options
     ): Promise<(TData | null | undefined)[]>
 
-    deleteFromCacheById(id: ObjectId): Promise<void>
+    deleteFromCacheById(id: ObjectId | string): Promise<void>
   }
 }

package-lock.json

@@ -21,6 +21,7 @@
     "apollo-datasource": "^0.3.1",
     "apollo-server-caching": "^0.3.1",
     "apollo-server-errors": "^2.4.1",
+    "bson": "^4.1.0",
     "dataloader": "^1.4.0"
   },
   "devDependencies": {

package.json

@@ -1,22 +1,22 @@
 import { InMemoryLRUCache } from 'apollo-server-caching'
 import wait from 'waait'
+import { ObjectId } from 'mongodb'
+import { EJSON } from 'bson'
 
-import { createCachingMethods } from '../cache'
+import { createCachingMethods, idToString } from '../cache'
 
 const docs = {
   id1: {
     _id: 'id1'
+    // _id: ObjectId()
   },
   id2: {
-    _id: 'id2'
-  },
-  id3: {
-    _id: 'id3'
+    _id: ObjectId()
   }
 }
 
 const collectionName = 'test'
-const cacheKey = id => `mongo-${collectionName}-${id}`
+const cacheKey = id => `mongo-${collectionName}-${idToString(id)}`
 
 describe('createCachingMethods', () => {
   let collection
@@ -29,7 +29,13 @@ describe('createCachingMethods', () => {
       find: jest.fn(({ _id: { $in: ids } }) => ({
         toArray: () =>
           new Promise(resolve => {
-            setTimeout(() => resolve(ids.map(id => docs[id])), 0)
+            setTimeout(
+              () =>
+                resolve(
+                  ids.map(id => (id === docs.id1._id ? docs.id1 : docs.id2))
+                ),
+              0
+            )
           })
       }))
     }
@@ -46,61 +52,61 @@ describe('createCachingMethods', () => {
   })
 
   it('finds one', async () => {
-    const doc = await api.findOneById('id1')
+    const doc = await api.findOneById(docs.id1._id)
     expect(doc).toBe(docs.id1)
     expect(collection.find.mock.calls.length).toBe(1)
   })
 
   it('finds two with batching', async () => {
-    const foundDocs = await api.findManyByIds(['id2', 'id3'])
+    const foundDocs = await api.findManyByIds([docs.id1._id, docs.id2._id])
 
-    expect(foundDocs[0]).toBe(docs.id2)
-    expect(foundDocs[1]).toBe(docs.id3)
+    expect(foundDocs[0]).toBe(docs.id1)
+    expect(foundDocs[1]).toBe(docs.id2)
 
     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('id1')
-  //   await api.findOneById('id1')
+  //   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('id1')
+    await api.findOneById(docs.id1._id)
 
-    const value = await cache.get(cacheKey('id1'))
+    const value = await cache.get(cacheKey(docs.id1._id))
     expect(value).toBeUndefined()
   })
 
   it(`caches`, async () => {
-    await api.findOneById('id1', { ttl: 1 })
-    const value = await cache.get(cacheKey('id1'))
-    expect(JSON.parse(value)).toEqual(docs.id1)
+    await api.findOneById(docs.id1._id, { ttl: 1 })
+    const value = await cache.get(cacheKey(docs.id1._id))
+    expect(value).toEqual(EJSON.stringify(docs.id1))
 
-    await api.findOneById('id1')
+    await api.findOneById(docs.id1._id)
     expect(collection.find.mock.calls.length).toBe(1)
   })
 
   it(`caches with ttl`, async () => {
-    await api.findOneById('id1', { ttl: 1 })
+    await api.findOneById(docs.id1._id, { ttl: 1 })
     await wait(1001)
 
-    const value = await cache.get(cacheKey('id1'))
+    const value = await cache.get(cacheKey(docs.id1._id))
     expect(value).toBeUndefined()
   })
 
   it(`deletes from cache`, async () => {
-    await api.findOneById('id1', { ttl: 1 })
+    await api.findOneById(docs.id1._id, { ttl: 1 })
 
-    const valueBefore = await cache.get(cacheKey('id1'))
-    expect(JSON.parse(valueBefore)).toEqual(docs.id1)
+    const valueBefore = await cache.get(cacheKey(docs.id1._id))
+    expect(valueBefore).toEqual(EJSON.stringify(docs.id1))
 
-    await api.deleteFromCacheById('id1')
+    await api.deleteFromCacheById(docs.id1._id)
 
-    const valueAfter = await cache.get(cacheKey('id1'))
+    const valueAfter = await cache.get(cacheKey(docs.id1._id))
     expect(valueAfter).toBeUndefined()
   })
 })

src/__tests__/cache.test.js

@@ -1,14 +1,18 @@
 import DataLoader from 'dataloader'
+import { ObjectId } from 'mongodb'
+import { EJSON } from 'bson'
+
+import { getCollection } from './helpers'
+
+export const idToString = id => (id instanceof ObjectId ? id.toHexString() : id)
 
-import { getCollection, isModel } from './helpers'
-import { ObjectId } from 'bson'
 // https://github.com/graphql/dataloader#batch-function
 const orderDocs = ids => docs => {
   const idMap = {}
   docs.forEach(doc => {
-    idMap[doc._id] = doc
+    idMap[idToString(doc._id)] = doc
   })
-  return ids.map(id => idMap[id])
+  return ids.map(id => idMap[idToString(id)])
 }
 
 export const createCachingMethods = ({ collection, model, cache }) => {
@@ -29,18 +33,17 @@ export const createCachingMethods = ({ collection, model, cache }) => {
 
   const methods = {
     findOneById: async (id, { ttl } = {}) => {
-      const _id = id instanceof ObjectId ? id.toHexString() : id
-      const key = cachePrefix + _id
+      const key = cachePrefix + idToString(id)
 
       const cacheDoc = await cache.get(key)
       if (cacheDoc) {
-        return JSON.parse(cacheDoc)
+        return EJSON.parse(cacheDoc)
       }
 
-      const doc = await loader.load(_id)
+      const doc = await loader.load(id)
       if (Number.isInteger(ttl)) {
         // https://github.com/apollographql/apollo-server/tree/master/packages/apollo-server-caching#apollo-server-caching
-        cache.set(key, JSON.stringify(doc), { ttl })
+        cache.set(key, EJSON.stringify(doc), { ttl })
       }
 
       return doc
@@ -49,9 +52,9 @@ export const createCachingMethods = ({ collection, model, cache }) => {
       return Promise.all(ids.map(id => methods.findOneById(id, { ttl })))
     },
     deleteFromCacheById: async id => {
-      const _id = id instanceof ObjectId ? id.toHexString() : id
-      loader.clear(_id)
-      await cache.delete(cachePrefix + _id)
+      const stringId = idToString(id)
+      loader.clear(stringId)
+      await cache.delete(cachePrefix + stringId)
     }
   }