From f3981b76a47373a318b221f68a5ec0c56ec91fee Mon Sep 17 00:00:00 2001 From: Jesse Rosenberger Date: Mon, 17 Dec 2018 16:26:34 +0200 Subject: [PATCH 1/5] Adjust `InMemoryLRUCache` to allow non-string values via generics. While it is certainly our current implementation, I'm not sure that the true spirit of an in-memory key-value store is to only map strings to other strings. While enforcing `` might be necessary for some distributed cache stores, it seems we shouldn't enforce it for `InMemoryLRUCache`. By changing the `KeyValueCache` to default to `string` but allow other options, we can allow the user to decide exactly how the RHS of this store should be typed. This does require a bit more flexible implementation of the `length` calculator which is used for cache ejection, but that implementation will become immediately useful when we start storing parsed ASTs in this KV store. --- .../apollo-server-cache-redis/src/index.ts | 2 +- .../src/InMemoryLRUCache.ts | 26 ++++++++++++++++--- .../src/KeyValueCache.ts | 6 ++--- 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/packages/apollo-server-cache-redis/src/index.ts b/packages/apollo-server-cache-redis/src/index.ts index 8116c939e22..c95f43662bf 100644 --- a/packages/apollo-server-cache-redis/src/index.ts +++ b/packages/apollo-server-cache-redis/src/index.ts @@ -3,7 +3,7 @@ import Redis from 'redis'; import { promisify } from 'util'; import DataLoader from 'dataloader'; -export class RedisCache implements KeyValueCache { +export class RedisCache implements KeyValueCache { // FIXME: Replace any with proper promisified type readonly client: any; readonly defaultSetOptions = { diff --git a/packages/apollo-server-caching/src/InMemoryLRUCache.ts b/packages/apollo-server-caching/src/InMemoryLRUCache.ts index 719db2ba34a..83b0232c6ac 100644 --- a/packages/apollo-server-caching/src/InMemoryLRUCache.ts +++ b/packages/apollo-server-caching/src/InMemoryLRUCache.ts @@ -1,21 +1,39 @@ import LRU from 'lru-cache'; import { KeyValueCache } from './KeyValueCache'; -export class InMemoryLRUCache implements KeyValueCache { - private store: LRU.Cache; +export class InMemoryLRUCache implements KeyValueCache { + private store: LRU.Cache; // FIXME: Define reasonable default max size of the cache constructor({ maxSize = Infinity }: { maxSize?: number } = {}) { this.store = new LRU({ max: maxSize, - length: item => item.length, + length(item) { + if (Array.isArray(item) || typeof item === 'string') { + return item.length; + } + + // If it's an object, we'll use the length to get an approximate, + // relative size of what it would take to store it. It's certainly not + // 100% accurate, but it's a very, very fast implementation and it + // doesn't require bringing in other dependencies or logic which we need + // to maintain. In the future, we might consider something like: + // npm.im/object-sizeof, but this should be sufficient for now. + if (typeof item === 'object') { + return JSON.stringify(item).length; + } + + // Go with the lru-cache default "naive" size, in lieu anything better: + // https://github.com/isaacs/node-lru-cache/blob/a71be6cd/index.js#L17 + return 1; + }, }); } async get(key: string) { return this.store.get(key); } - async set(key: string, value: string) { + async set(key: string, value: V) { this.store.set(key, value); } async delete(key: string) { diff --git a/packages/apollo-server-caching/src/KeyValueCache.ts b/packages/apollo-server-caching/src/KeyValueCache.ts index ff0a0d2ceb5..86c3e136273 100644 --- a/packages/apollo-server-caching/src/KeyValueCache.ts +++ b/packages/apollo-server-caching/src/KeyValueCache.ts @@ -1,5 +1,5 @@ -export interface KeyValueCache { - get(key: string): Promise; - set(key: string, value: string, options?: { ttl?: number }): Promise; +export interface KeyValueCache { + get(key: string): Promise; + set(key: string, value: V, options?: { ttl?: number }): Promise; delete(key: string): Promise; } From d53f361c54202b30c1c9b50f17c3fd2309760ab5 Mon Sep 17 00:00:00 2001 From: Jesse Rosenberger Date: Mon, 17 Dec 2018 18:25:04 +0200 Subject: [PATCH 2/5] cache: Implement `flush` method for `InMemoryLRUCache`. This implements the `flush` method, which is currently supported by both Memcached and Redis, into the `InMemoryLRUCCache` cache. --- packages/apollo-server-caching/src/InMemoryLRUCache.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/apollo-server-caching/src/InMemoryLRUCache.ts b/packages/apollo-server-caching/src/InMemoryLRUCache.ts index 83b0232c6ac..f8613373534 100644 --- a/packages/apollo-server-caching/src/InMemoryLRUCache.ts +++ b/packages/apollo-server-caching/src/InMemoryLRUCache.ts @@ -39,4 +39,7 @@ export class InMemoryLRUCache implements KeyValueCache { async delete(key: string) { this.store.del(key); } + async flush(): Promise { + this.store.reset(); + } } From c7f6b43234802dce6e91fed8ce96c5986b4a2ea2 Mon Sep 17 00:00:00 2001 From: Jesse Rosenberger Date: Mon, 17 Dec 2018 18:20:58 +0200 Subject: [PATCH 3/5] tests: Change `apollo-server-caching` test-suite to support InMemoryLRUCache. --- .../src/__tests__/Memcached.test.ts | 11 +++++- .../src/__tests__/Redis.test.ts | 11 +++++- packages/apollo-server-caching/README.md | 15 ++++++- .../src/__tests__/InMemoryLRUCache.test.ts | 7 ++++ .../src/__tests__/testsuite.ts | 39 +++++++++++++------ 5 files changed, 66 insertions(+), 17 deletions(-) create mode 100644 packages/apollo-server-caching/src/__tests__/InMemoryLRUCache.test.ts diff --git a/packages/apollo-server-cache-memcached/src/__tests__/Memcached.test.ts b/packages/apollo-server-cache-memcached/src/__tests__/Memcached.test.ts index cc769325efb..d3d7d20b47e 100644 --- a/packages/apollo-server-cache-memcached/src/__tests__/Memcached.test.ts +++ b/packages/apollo-server-cache-memcached/src/__tests__/Memcached.test.ts @@ -2,6 +2,13 @@ jest.mock('memcached', () => require('memcached-mock')); import { MemcachedCache } from '../index'; -import { testKeyValueCache } from '../../../apollo-server-caching/src/__tests__/testsuite'; +import { + testKeyValueCache_Basics, + testKeyValueCache_Expiration, +} from '../../../apollo-server-caching/src/__tests__/testsuite'; -testKeyValueCache(new MemcachedCache('localhost')); +describe('Memcached', () => { + const cache = new MemcachedCache('localhost'); + testKeyValueCache_Basics(cache); + testKeyValueCache_Expiration(cache); +}); diff --git a/packages/apollo-server-cache-redis/src/__tests__/Redis.test.ts b/packages/apollo-server-cache-redis/src/__tests__/Redis.test.ts index 4e0ea455af3..d13f0d30ccd 100644 --- a/packages/apollo-server-cache-redis/src/__tests__/Redis.test.ts +++ b/packages/apollo-server-cache-redis/src/__tests__/Redis.test.ts @@ -3,6 +3,13 @@ jest.mock('redis', () => require('redis-mock')); jest.useFakeTimers(); // mocks out setTimeout that is used in redis-mock import { RedisCache } from '../index'; -import { testKeyValueCache } from '../../../apollo-server-caching/src/__tests__/testsuite'; +import { + testKeyValueCache_Basics, + testKeyValueCache_Expiration, +} from '../../../apollo-server-caching/src/__tests__/testsuite'; -testKeyValueCache(new RedisCache({ host: 'localhost' })); +describe('Redis', () => { + const cache = new RedisCache({ host: 'localhost' }); + testKeyValueCache_Basics(cache); + testKeyValueCache_Expiration(cache); +}); diff --git a/packages/apollo-server-caching/README.md b/packages/apollo-server-caching/README.md index 5b9aaac8349..26980ea60ef 100644 --- a/packages/apollo-server-caching/README.md +++ b/packages/apollo-server-caching/README.md @@ -16,7 +16,9 @@ export interface KeyValueCache { } ``` -## Running test suite +## Testing cache implementations + +### Test helpers You can export and run a jest test suite from `apollo-server-caching` to test your implementation: @@ -28,4 +30,15 @@ import { testKeyValueCache } from 'apollo-server-caching'; testKeyValueCache(new MemcachedCache('localhost')); ``` +The default `testKeyValueCache` helper will run all key-value store tests on the specified store, including basic `get` and `set` functionality, along with time-based expunging rules. + +Some key-value cache implementations may not be able to support the full suite of tests (for example, some tests might not be able to expire based on time). For those cases, there are more granular implementations which can be used: + +* `testKeyValueCache_Basic` +* `testKeyValueCache_Expiration` + +For more details, consult the [source for `apollo-server-caching`](./src/__tests__/testsuite.ts). + +### Running tests + Run tests with `jest --verbose` diff --git a/packages/apollo-server-caching/src/__tests__/InMemoryLRUCache.test.ts b/packages/apollo-server-caching/src/__tests__/InMemoryLRUCache.test.ts new file mode 100644 index 00000000000..fff1d75c38b --- /dev/null +++ b/packages/apollo-server-caching/src/__tests__/InMemoryLRUCache.test.ts @@ -0,0 +1,7 @@ +import { testKeyValueCache_Basics } from '../../../apollo-server-caching/src/__tests__/testsuite'; +import { InMemoryLRUCache } from '../InMemoryLRUCache'; + +describe('InMemoryLRUCache', () => { + const cache = new InMemoryLRUCache(); + testKeyValueCache_Basics(cache); +}); diff --git a/packages/apollo-server-caching/src/__tests__/testsuite.ts b/packages/apollo-server-caching/src/__tests__/testsuite.ts index 81dcb24c3b5..3ee62aa22e0 100644 --- a/packages/apollo-server-caching/src/__tests__/testsuite.ts +++ b/packages/apollo-server-caching/src/__tests__/testsuite.ts @@ -1,21 +1,11 @@ import { advanceTimeBy, mockDate, unmockDate } from '__mocks__/date'; -export function testKeyValueCache(keyValueCache: any) { - describe('KeyValueCache Test Suite', () => { - beforeAll(() => { - mockDate(); - jest.useFakeTimers(); - }); - +export function testKeyValueCache_Basics(keyValueCache: any) { + describe('basic cache functionality', () => { beforeEach(() => { keyValueCache.flush(); }); - afterAll(() => { - unmockDate(); - keyValueCache.close(); - }); - it('can do a basic get and set', async () => { await keyValueCache.set('hello', 'world'); expect(await keyValueCache.get('hello')).toBe('world'); @@ -28,6 +18,24 @@ export function testKeyValueCache(keyValueCache: any) { await keyValueCache.delete('hello'); expect(await keyValueCache.get('hello')).toBeUndefined(); }); + }); +} + +export function testKeyValueCache_Expiration(keyValueCache: any) { + describe('time-based cache expunging', () => { + beforeAll(() => { + mockDate(); + jest.useFakeTimers(); + }); + + beforeEach(() => { + keyValueCache.flush(); + }); + + afterAll(() => { + unmockDate(); + keyValueCache.close(); + }); it('is able to expire keys based on ttl', async () => { await keyValueCache.set('short', 's', { ttl: 1 }); @@ -45,3 +53,10 @@ export function testKeyValueCache(keyValueCache: any) { }); }); } + +export function testKeyValueCache(keyValueCache: any) { + describe('KeyValueCache Test Suite', () => { + testKeyValueCache_Basics(keyValueCache); + testKeyValueCache_Expiration(keyValueCache); + }); +} From 098c89399075b589f2dda12eb50a8c17aa9ae217 Mon Sep 17 00:00:00 2001 From: Jesse Rosenberger Date: Tue, 18 Dec 2018 10:12:01 +0200 Subject: [PATCH 4/5] nit: Use the term `value` rather than `data` for Key + *Value* stores. Super nitty, but they are KeyValue stores, not KeyData stores. --- packages/apollo-server-cache-memcached/src/index.ts | 4 ++-- packages/apollo-server-cache-redis/src/index.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/apollo-server-cache-memcached/src/index.ts b/packages/apollo-server-cache-memcached/src/index.ts index 45d53aacfba..e369be35d63 100644 --- a/packages/apollo-server-cache-memcached/src/index.ts +++ b/packages/apollo-server-cache-memcached/src/index.ts @@ -22,11 +22,11 @@ export class MemcachedCache implements KeyValueCache { async set( key: string, - data: string, + value: string, options?: { ttl?: number }, ): Promise { const { ttl } = Object.assign({}, this.defaultSetOptions, options); - await this.client.set(key, data, ttl); + await this.client.set(key, value, ttl); } async get(key: string): Promise { diff --git a/packages/apollo-server-cache-redis/src/index.ts b/packages/apollo-server-cache-redis/src/index.ts index c95f43662bf..7c0b1b61330 100644 --- a/packages/apollo-server-cache-redis/src/index.ts +++ b/packages/apollo-server-cache-redis/src/index.ts @@ -31,11 +31,11 @@ export class RedisCache implements KeyValueCache { async set( key: string, - data: string, + value: string, options?: { ttl?: number }, ): Promise { const { ttl } = Object.assign({}, this.defaultSetOptions, options); - await this.client.set(key, data, 'EX', ttl); + await this.client.set(key, value, 'EX', ttl); } async get(key: string): Promise { From 2b50e149b0a2653a68acc90278eca7d8a4689da2 Mon Sep 17 00:00:00 2001 From: Jesse Rosenberger Date: Tue, 18 Dec 2018 10:13:08 +0200 Subject: [PATCH 5/5] caching: InMemoryLRUCache support for `ttl`, much like Redis/Memcached. This also re-enables the Expiration tests for this library, though I think I will continue to leave the testsuite decomposed into more granular methods for readability and future growth. Note that this doesn't implement the same default `300` second TTL as the other Memcache and Redis implementations currently do (a very debatable default we should re-consider in the future). --- packages/apollo-server-caching/src/InMemoryLRUCache.ts | 5 +++-- .../src/__tests__/InMemoryLRUCache.test.ts | 6 +++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/apollo-server-caching/src/InMemoryLRUCache.ts b/packages/apollo-server-caching/src/InMemoryLRUCache.ts index f8613373534..b75dfcb37d0 100644 --- a/packages/apollo-server-caching/src/InMemoryLRUCache.ts +++ b/packages/apollo-server-caching/src/InMemoryLRUCache.ts @@ -33,8 +33,9 @@ export class InMemoryLRUCache implements KeyValueCache { async get(key: string) { return this.store.get(key); } - async set(key: string, value: V) { - this.store.set(key, value); + async set(key: string, value: V, options?: { ttl?: number }) { + const maxAge = options && options.ttl && options.ttl * 1000; + this.store.set(key, value, maxAge); } async delete(key: string) { this.store.del(key); diff --git a/packages/apollo-server-caching/src/__tests__/InMemoryLRUCache.test.ts b/packages/apollo-server-caching/src/__tests__/InMemoryLRUCache.test.ts index fff1d75c38b..f6dd0a1dfaf 100644 --- a/packages/apollo-server-caching/src/__tests__/InMemoryLRUCache.test.ts +++ b/packages/apollo-server-caching/src/__tests__/InMemoryLRUCache.test.ts @@ -1,7 +1,11 @@ -import { testKeyValueCache_Basics } from '../../../apollo-server-caching/src/__tests__/testsuite'; +import { + testKeyValueCache_Basics, + testKeyValueCache_Expiration, +} from '../../../apollo-server-caching/src/__tests__/testsuite'; import { InMemoryLRUCache } from '../InMemoryLRUCache'; describe('InMemoryLRUCache', () => { const cache = new InMemoryLRUCache(); testKeyValueCache_Basics(cache); + testKeyValueCache_Expiration(cache); });