Skip to content

Commit 135f8b8

Browse files
ryangoree9at8
andauthored
Add findByFields method (#50)
* Add findByFields method * Prettify, fix filters mixing, and improve caching * 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 * Add findByFields tests * Add findByFields to ts module * Add findByFields to README * Ensure non-array field value cache keys are saved as arrays * Add more cache testing for findByFields * Clean up code * Sort fields before caching or calling loader Co-authored-by: Aditya Thakral <[email protected]> Co-authored-by: Aditya Thakral <[email protected]>
1 parent 3b6008c commit 135f8b8

File tree

4 files changed

+305
-61
lines changed

4 files changed

+305
-61
lines changed

README.md

+38-1
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,11 @@ Apollo [data source](https://www.apollographql.com/docs/apollo-server/features/d
66
npm i apollo-datasource-mongodb
77
```
88

9-
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:
9+
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:
1010

1111
- [`findOneById(id, options)`](#findonebyid)
1212
- [`findManyByIds(ids, options)`](#findmanybyids)
13+
- [`findByFields(fields, options)`](#findbyfields)
1314

1415

1516
<!-- START doctoc generated TOC please keep comment here to allow auto update -->
@@ -24,6 +25,8 @@ This package uses [DataLoader](https://github.com/graphql/dataloader) for batchi
2425
- [API](#api)
2526
- [findOneById](#findonebyid)
2627
- [findManyByIds](#findmanybyids)
28+
- [findByFields](#findbyfields)
29+
- [Examples:](#examples)
2730
- [deleteFromCacheById](#deletefromcachebyid)
2831

2932
<!-- END doctoc generated TOC please keep comment here to allow auto update -->
@@ -183,6 +186,7 @@ interface UserDocument {
183186
username: string
184187
password: string
185188
email: string
189+
interests: [string]
186190
}
187191

188192
// This is optional
@@ -236,6 +240,39 @@ Resolves to the found document. Uses DataLoader to load `id`. DataLoader uses `c
236240

237241
Calls [`findOneById()`](#findonebyid) for each id. Resolves to an array of documents.
238242

243+
### findByFields
244+
245+
`this.findByFields(fields, { ttl })`
246+
247+
Resolves to an array of documents matching the passed fields.
248+
249+
fields has type `{ [fieldName: string]: string | number | boolean | [string | number | boolean] }`.
250+
251+
#### Examples:
252+
253+
```js
254+
255+
// get user by username
256+
// `collection.find({ username: $in: ['testUser'] })`
257+
this.getByFields({
258+
username: 'testUser'
259+
})
260+
261+
// get all users with either the "gaming" OR "games" interest
262+
// `collection.find({ interests: $in: ['gaming', 'games'] })`
263+
this.getByFields({
264+
interests: ['gaming', 'games']
265+
})
266+
267+
// get user by username AND with either the "gaming" OR "games" interest
268+
// `collection.find({ username: $in: ['testUser'], interests: $in: ['gaming', 'games'] })`
269+
this.getByFields({
270+
username: 'testUser',
271+
interests: ['gaming', 'games']
272+
})
273+
274+
```
275+
239276
### deleteFromCacheById
240277

241278
`this.deleteFromCacheById(id)`

index.d.ts

+10-1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ declare module 'apollo-datasource-mongodb' {
1616
export type ModelOrCollection<T> = T extends Document
1717
? Model<T>
1818
: Collection<T>
19+
20+
export interface Fields {
21+
[fieldName: string]: string | number | boolean | [string | number | boolean]
22+
}
1923

2024
export interface Options {
2125
ttl: number
@@ -40,6 +44,11 @@ declare module 'apollo-datasource-mongodb' {
4044
options?: Options
4145
): Promise<(TData | null | undefined)[]>
4246

47+
findByFields(
48+
fields: Fields,
49+
options?: Options
50+
): Promise<(TData | null | undefined)[]>
51+
4352
deleteFromCacheById(id: ObjectId | string): Promise<void>
4453
}
45-
}
54+
}

src/__tests__/cache.test.js

+149-40
Original file line numberDiff line numberDiff line change
@@ -9,19 +9,36 @@ const hexId = 'aaaa0000bbbb0000cccc0000'
99

1010
const docs = {
1111
one: {
12-
_id: ObjectId(hexId)
12+
_id: ObjectId(hexId),
13+
foo: 'bar',
14+
tags: ['foo', 'bar']
1315
},
1416
two: {
15-
_id: ObjectId()
17+
_id: ObjectId(),
18+
foo: 'bar'
1619
}
1720
}
1821

1922
const stringDoc = {
20-
_id: 's2QBCnv6fXv5YbjAP'
23+
_id: 's2QBCnv6fXv5YbjAP',
24+
tags: ['bar', 'baz']
2125
}
2226

2327
const collectionName = 'test'
24-
const cacheKey = id => `mongo-${collectionName}-${idToString(id)}`
28+
const cacheKeyById = id => `mongo-${collectionName}-${idToString(id)}`
29+
const cacheKeyByFields = fields => {
30+
const cleanedFields = {}
31+
32+
Object.keys(fields).forEach(key => {
33+
if (typeof key !== 'undefined') {
34+
cleanedFields[key] = Array.isArray(fields[key])
35+
? fields[key]
36+
: [fields[key]]
37+
}
38+
})
39+
40+
return `mongo-${collectionName}-${JSON.stringify(cleanedFields)}`
41+
}
2542

2643
describe('createCachingMethods', () => {
2744
let collection
@@ -31,30 +48,55 @@ describe('createCachingMethods', () => {
3148
beforeEach(() => {
3249
collection = {
3350
collectionName,
34-
find: jest.fn(({ _id: { $in: ids } }) => ({
35-
toArray: () =>
36-
new Promise(resolve => {
37-
setTimeout(
38-
() =>
39-
resolve(
40-
ids.map(id => {
41-
if (id === stringDoc._id) {
42-
return stringDoc
43-
}
44-
45-
if (id.equals(docs.one._id)) {
46-
return docs.one
47-
}
48-
49-
if (id.equals(docs.two._id)) {
50-
return docs.two
51-
}
52-
})
53-
),
54-
0
55-
)
56-
})
57-
}))
51+
find: jest.fn(filter => {
52+
return {
53+
toArray: () =>
54+
new Promise(resolve => {
55+
setTimeout(
56+
() =>
57+
resolve(
58+
[docs.one, docs.two, stringDoc].filter(doc => {
59+
for (const orFilter of filter.$or || [filter]) {
60+
for (const field in orFilter) {
61+
if (field === '_id') {
62+
for (const id of orFilter._id.$in) {
63+
if (id.equals && !id.equals(doc._id)) {
64+
break
65+
} else if (
66+
doc._id.equals &&
67+
!doc._id.equals(id)
68+
) {
69+
break
70+
} else if (
71+
!id.equals &&
72+
!doc._id.equals &&
73+
id !== doc._id
74+
) {
75+
break
76+
}
77+
}
78+
} else if (Array.isArray(doc[field])) {
79+
for (const value of orFilter[field].$in) {
80+
if (!doc[field].includes(value)) {
81+
break
82+
}
83+
}
84+
} else if (
85+
!orFilter[field].$in.includes(doc[field])
86+
) {
87+
break
88+
}
89+
}
90+
return true
91+
}
92+
return false
93+
})
94+
),
95+
0
96+
)
97+
})
98+
}
99+
})
58100
}
59101

60102
cache = new InMemoryLRUCache()
@@ -65,10 +107,11 @@ describe('createCachingMethods', () => {
65107
it('adds the right methods', () => {
66108
expect(api.findOneById).toBeDefined()
67109
expect(api.findManyByIds).toBeDefined()
110+
expect(api.findByFields).toBeDefined()
68111
expect(api.deleteFromCacheById).toBeDefined()
69112
})
70113

71-
it('finds one', async () => {
114+
it('finds one with ObjectId', async () => {
72115
const doc = await api.findOneById(docs.one._id)
73116
expect(doc).toBe(docs.one)
74117
expect(collection.find.mock.calls.length).toBe(1)
@@ -89,48 +132,114 @@ describe('createCachingMethods', () => {
89132
expect(collection.find.mock.calls.length).toBe(1)
90133
})
91134

92-
// TODO why doesn't this pass?
93-
// it.only(`doesn't cache without ttl`, async () => {
94-
// await api.findOneById(docs.id1._id)
95-
// await api.findOneById(docs.id1._id)
135+
it('finds by field', async () => {
136+
const foundDocs = await api.findByFields({ foo: 'bar' })
137+
138+
expect(foundDocs[0]).toBe(docs.one)
139+
expect(foundDocs[1]).toBe(docs.two)
140+
expect(foundDocs.length).toBe(2)
141+
142+
expect(collection.find.mock.calls.length).toBe(1)
143+
})
144+
145+
it('finds by array field', async () => {
146+
const foundDocs = await api.findByFields({ tags: 'bar' })
147+
148+
expect(foundDocs[0]).toBe(docs.one)
149+
expect(foundDocs[1]).toBe(stringDoc)
150+
expect(foundDocs.length).toBe(2)
151+
152+
expect(collection.find.mock.calls.length).toBe(1)
153+
})
154+
155+
it('finds by mutiple fields', async () => {
156+
const foundDocs = await api.findByFields({
157+
tags: ['foo', 'bar'],
158+
foo: 'bar'
159+
})
160+
161+
expect(foundDocs[0]).toBe(docs.one)
162+
expect(foundDocs.length).toBe(1)
163+
164+
expect(collection.find.mock.calls.length).toBe(1)
165+
})
166+
167+
it(`doesn't mix filters of pending calls for different fields`, async () => {
168+
const pendingDocs1 = api.findByFields({ foo: 'bar' })
169+
const pendingDocs2 = api.findByFields({ tags: 'baz' })
170+
const [foundDocs1, foundDocs2] = await Promise.all([
171+
pendingDocs1,
172+
pendingDocs2
173+
])
174+
175+
expect(foundDocs1[0]).toBe(docs.one)
176+
expect(foundDocs1[1]).toBe(docs.two)
177+
expect(foundDocs1.length).toBe(2)
178+
expect(foundDocs2[0]).toBe(stringDoc)
179+
expect(foundDocs2.length).toBe(1)
180+
181+
expect(collection.find.mock.calls.length).toBe(1)
182+
})
183+
184+
it(`dataloader caches each value individually when finding by a single field`, async () => {
185+
await api.findByFields({ tags: ['foo', 'baz'] }, { ttl: 1 })
186+
187+
const fooBazCacheValue = await cache.get(
188+
cacheKeyByFields({ tags: ['foo', 'baz'] })
189+
)
190+
const fooCacheValue = await cache.get(cacheKeyByFields({ tags: 'foo' }))
191+
192+
expect(fooBazCacheValue).toBeDefined()
193+
expect(fooCacheValue).toBeUndefined()
96194

97-
// expect(collection.find.mock.calls.length).toBe(2)
98-
// })
195+
await api.findByFields({ tags: 'foo' })
196+
expect(collection.find.mock.calls.length).toBe(1)
197+
})
99198

100199
it(`doesn't cache without ttl`, async () => {
101200
await api.findOneById(docs.one._id)
102201

103-
const value = await cache.get(cacheKey(docs.one._id))
202+
const value = await cache.get(cacheKeyById(docs.one._id))
104203
expect(value).toBeUndefined()
105204
})
106205

107206
it(`caches`, async () => {
108207
await api.findOneById(docs.one._id, { ttl: 1 })
109-
const value = await cache.get(cacheKey(docs.one._id))
208+
const value = await cache.get(cacheKeyById(docs.one._id))
110209
expect(value).toEqual(EJSON.stringify(docs.one))
111210

112211
await api.findOneById(docs.one._id)
113212
expect(collection.find.mock.calls.length).toBe(1)
114213
})
115214

215+
it(`caches non-array field values as arrays`, async () => {
216+
const fields = { tags: 'foo' }
217+
await api.findByFields(fields, { ttl: 1 })
218+
const value = await cache.get(cacheKeyByFields(fields))
219+
expect(value).toEqual(EJSON.stringify([docs.one]))
220+
221+
await api.findByFields({ tags: ['foo'] })
222+
expect(collection.find.mock.calls.length).toBe(1)
223+
})
224+
116225
it(`caches with ttl`, async () => {
117226
await api.findOneById(docs.one._id, { ttl: 1 })
118227
await wait(1001)
119228

120-
const value = await cache.get(cacheKey(docs.one._id))
229+
const value = await cache.get(cacheKeyById(docs.one._id))
121230
expect(value).toBeUndefined()
122231
})
123232

124233
it(`deletes from cache`, async () => {
125234
for (const doc of [docs.one, docs.two, stringDoc]) {
126235
await api.findOneById(doc._id, { ttl: 1 })
127236

128-
const valueBefore = await cache.get(cacheKey(doc._id))
237+
const valueBefore = await cache.get(cacheKeyById(doc._id))
129238
expect(valueBefore).toEqual(EJSON.stringify(doc))
130239

131240
await api.deleteFromCacheById(doc._id)
132241

133-
const valueAfter = await cache.get(cacheKey(doc._id))
242+
const valueAfter = await cache.get(cacheKeyById(doc._id))
134243
expect(valueAfter).toBeUndefined()
135244
}
136245
})

0 commit comments

Comments
 (0)