From df06a7c624da010d7853e0de8fc2a7d972460edf Mon Sep 17 00:00:00 2001 From: David Glasser Date: Tue, 6 Apr 2021 14:47:22 -0700 Subject: [PATCH] apollo-server-cache-redis: follow-up to #5034 (#5088) Add docs and changelog. As part of doing this, realize that the implementation in #5034 makes it annoying to inject a cluster client, so switch the new API to decide about get vs mget based on which kind of client is passed to the base class. --- CHANGELOG.md | 1 + docs/source/data/data-sources.md | 10 +-- docs/source/performance/apq.md | 60 +++++++++------- packages/apollo-server-cache-redis/README.md | 56 +++++++++------ .../src/BaseRedisCache.ts | 70 +++++++++++++++---- .../src/RedisCache.ts | 2 +- .../src/RedisClusterCache.ts | 12 +--- .../src/__tests__/BaseRedisCache.test.ts | 29 +++++--- 8 files changed, 151 insertions(+), 89 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bb4558df46..1e27b455a16 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ The version headers in this history reflect the versions of Apollo Server itself > The changes noted within this `vNEXT` section have not been released yet. New PRs and commits which introduce changes should include an entry in this `vNEXT` section as part of their development. With few exceptions, the format of the entry should follow convention (i.e., prefix with package name, use markdown `backtick formatting` for package names and code, suffix with a link to the change-set à la `[PR #YYY](https://link/pull/YYY)`, etc.). When a release is being prepared, a new header will be (manually) created below and the appropriate changes within that release will be moved into the new section. - `apollo-server-core`: Add optional argument to `ApolloServer.executeOperation` allowing the caller to manually specify an argument to the `config` function analogous to that provided by integration packages. [PR #4166](https://github.com/apollographql/apollo-server/pull/4166) [Issue #2886](https://github.com/apollographql/apollo-server/issues/2886) +- `apollo-server-cache-redis`: New `BaseRedisCache` class which takes an `ioredis`-compatible Redis client as an argument. The existing classes `RedisCache` and `RedisClusterCache` (which pass their arguments to `ioredis` constructors) are now implemented in terms of this class. This allows you to use any of the `ioredis` constructor forms rather than just the ones recognized by our classes. This also fixes a long-standing bug where the Redis cache implementations returned a number from `delete()`; it now returns a number, matching what the `KeyValueCache` interface and the TypeScript types expect. [PR #5034](https://github.com/apollographql/apollo-server/pull/5034) [PR #5088](https://github.com/apollographql/apollo-server/pull/5088) [Issue #4870](https://github.com/apollographql/apollo-server/issues/4870) [Issue #5006](https://github.com/apollographql/apollo-server/issues/5006) ## v2.22.2 diff --git a/docs/source/data/data-sources.md b/docs/source/data/data-sources.md index 6dd84e813ee..f679f443ee5 100644 --- a/docs/source/data/data-sources.md +++ b/docs/source/data/data-sources.md @@ -257,14 +257,16 @@ const server = new ApolloServer({ For documentation of the options you can pass to the underlying Memcached client, look [here](https://github.com/3rd-Eden/memcached). ```js -const { RedisCache } = require('apollo-server-cache-redis'); +const { BaseRedisCache } = require('apollo-server-cache-redis'); +const Redis = require('ioredis'); const server = new ApolloServer({ typeDefs, resolvers, - cache: new RedisCache({ - host: 'redis-server', - // Options are passed through to the Redis client + cache: new BaseRedisCache({ + client: new Redis({ + host: 'redis-server', + }), }), dataSources: () => ({ moviesAPI: new MoviesAPI(), diff --git a/docs/source/performance/apq.md b/docs/source/performance/apq.md index 2ecf90be74e..3d30c9e354d 100644 --- a/docs/source/performance/apq.md +++ b/docs/source/performance/apq.md @@ -217,20 +217,22 @@ const server = new ApolloServer({ ### Redis (single instance) ```shell -$ npm install apollo-server-cache-redis +$ npm install apollo-server-cache-redis ioredis ``` ```javascript -const { RedisCache } = require('apollo-server-cache-redis'); +const { BaseRedisCache } = require('apollo-server-cache-redis'); +const Redis = require('ioredis'); const server = new ApolloServer({ typeDefs, resolvers, // highlight-start persistedQueries: { - cache: new RedisCache({ - host: 'redis-server', - // Options are passed through to the Redis client + cache: new BaseRedisCache({ + client: new Redis({ + host: 'redis-server', + }), }), }, // highlight-end @@ -240,25 +242,27 @@ const server = new ApolloServer({ ### Redis (Sentinel) ```shell -$ npm install apollo-server-cache-redis +$ npm install apollo-server-cache-redis ioredis ``` ```javascript -const { RedisCache } = require('apollo-server-cache-redis'); +const { BaseRedisCache } = require('apollo-server-cache-redis'); +const Redis = require('ioredis'); const server = new ApolloServer({ typeDefs, resolvers, // highlight-start persistedQueries: { - cache: new RedisCache({ - sentinels: [{ - host: 'sentinel-host-01', - port: 26379 - }], - password: 'my_password', - name: 'service_name', - // Options are passed through to the Redis client + cache: new BaseRedisCache({ + client: new Redis({ + sentinels: [{ + host: 'sentinel-host-01', + port: 26379 + }], + password: 'my_password', + name: 'service_name', + }), }), }, // highlight-end @@ -268,26 +272,30 @@ const server = new ApolloServer({ ### Redis Cluster ```shell -$ npm install apollo-server-cache-redis +$ npm install apollo-server-cache-redis ioredis ``` ```javascript -const { RedisClusterCache } = require('apollo-server-cache-redis'); +const { BaseRedisCache } = require('apollo-server-cache-redis'); +const Redis = require('ioredis'); const server = new ApolloServer({ typeDefs, resolvers, // highlight-start persistedQueries: { - cache: new RedisClusterCache( - [{ - host: 'redis-node-01-host', - // Options are passed through to the Redis cluster client - }], - { - // Cluster options are passed through to the Redis cluster client - } - ), + cache: new BaseRedisCache({ + // Note that this uses the "clusterClient" option rather than "client", + // which avoids using the mget command which doesn't work in cluster mode. + clusterClient: new Redis.Cluster( + [{ + host: 'redis-node-01-host', + }], + { + // Other Redis cluster client options + } + ), + }), }, // highlight-end }); diff --git a/packages/apollo-server-cache-redis/README.md b/packages/apollo-server-cache-redis/README.md index 1f9f0814681..c0bc72c8313 100644 --- a/packages/apollo-server-cache-redis/README.md +++ b/packages/apollo-server-cache-redis/README.md @@ -9,17 +9,22 @@ It currently supports a single instance of Redis, [Cluster](http://redis.io/topi ## Usage +This package is built to be compatible with the [ioredis](https://www.npmjs.com/package/ioredis) Redis client. The recommended usage is to use the `BaseRedisCache` class which takes either a `client` option (a client that talks to a single server) or a `clusterClient` option (a client that talks to Redis Cluster). (The difference is that ioredis [only supports the `mget` multi-get command in non-cluster mode](https://github.com/luin/ioredis/issues/811), so using `clusterClient` tells `BaseRedisCache` to use parallel `get` commands instead.) + +You may also use the older `RedisCache` and `RedisClusterCache` classes, which allow you to pass the ioredis constructor arguments directly to the cache class's constructor. ### Single instance ```js -const { RedisCache } = require('apollo-server-cache-redis'); +const { BaseRedisCache } = require('apollo-server-cache-redis'); +const Redis = require('ioredis'); const server = new ApolloServer({ typeDefs, resolvers, - cache: new RedisCache({ - host: 'redis-server', - // Options are passed through to the Redis client + cache: new BaseRedisCache({ + client: new Redis({ + host: 'redis-server', + }), }), dataSources: () => ({ moviesAPI: new MoviesAPI(), @@ -30,19 +35,21 @@ const server = new ApolloServer({ ### Sentinels ```js -const { RedisCache } = require('apollo-server-cache-redis'); +const { BaseRedisCache } = require('apollo-server-cache-redis'); +const Redis = require('ioredis'); const server = new ApolloServer({ typeDefs, resolvers, - cache: new RedisCache({ - sentinels: [{ - host: 'sentinel-host-01', - port: 26379 - }], - password: 'my_password', - name: 'service_name', - // Options are passed through to the Redis client + cache: new BaseRedisCache({ + client: new Redis({ + sentinels: [{ + host: 'sentinel-host-01', + port: 26379 + }], + password: 'my_password', + name: 'service_name', + }), }), dataSources: () => ({ moviesAPI: new MoviesAPI(), @@ -53,20 +60,23 @@ const server = new ApolloServer({ ### Cluster ```js -const { RedisClusterCache } = require('apollo-server-cache-redis'); +const { BaseRedisCache } = require('apollo-server-cache-redis'); +const Redis = require('ioredis'); const server = new ApolloServer({ typeDefs, resolvers, - cache: new RedisClusterCache( - [{ - host: 'redis-node-01-host', - // Options are passed through to the Redis cluster client - }], - { - // Cluster options are passed through to the Redis cluster client - } - ), + cache: new BaseRedisCache({ + clusterClient: new Redis.Cluster( + [{ + host: 'redis-node-01-host', + // Options are passed through to the Redis cluster client + }], + { + // Redis cluster client options + } + ), + }), dataSources: () => ({ moviesAPI: new MoviesAPI(), }), diff --git a/packages/apollo-server-cache-redis/src/BaseRedisCache.ts b/packages/apollo-server-cache-redis/src/BaseRedisCache.ts index 116302f92c3..dc9892dbeeb 100644 --- a/packages/apollo-server-cache-redis/src/BaseRedisCache.ts +++ b/packages/apollo-server-cache-redis/src/BaseRedisCache.ts @@ -4,28 +4,70 @@ import { } from 'apollo-server-caching'; import DataLoader from 'dataloader'; -export interface RedisClient { - set: (key: string, value: string, option?: string, optionValue?: number) => Promise - mget: (...key: Array) => Promise> - flushdb: () => Promise - del: (key: string) => Promise - quit: () => Promise +interface BaseRedisClient { + set: ( + key: string, + value: string, + option?: string, + optionValue?: number, + ) => Promise; + flushdb: () => Promise; + del: (key: string) => Promise; + quit: () => Promise; +} + +export interface RedisClient extends BaseRedisClient { + mget: (...key: Array) => Promise>; +} + +export interface RedisClusterClient extends BaseRedisClient { + get: (key: string) => Promise; +} + +/** + * Provide exactly one of the options `client` and `clusterClient`. `client` is + * a client that supports the `mget` multiple-get command. + * + * ioredis does not support `mget` for cluster mode (see + * https://github.com/luin/ioredis/issues/811), so if you're using cluster mode, + * pass `clusterClient` instead, which has a `get` method instead of `mget`; + * this package will issue parallel `get` commands instead of a single `mget` + * command if `clusterClient` is provided. + */ +export interface BaseRedisCacheOptions { + client?: RedisClient; + clusterClient?: RedisClusterClient; } export class BaseRedisCache implements TestableKeyValueCache { - readonly client: RedisClient; + readonly client: BaseRedisClient; readonly defaultSetOptions: KeyValueCacheSetOptions = { ttl: 300, }; private loader: DataLoader; - constructor(client: RedisClient) { - this.client = client; - - this.loader = new DataLoader(keys => client.mget(...keys), { - cache: false, - }); + constructor(options: BaseRedisCacheOptions) { + const { client, clusterClient } = options; + if (client && clusterClient) { + throw Error('You may only provide one of `client` and `clusterClient`'); + } else if (client) { + this.client = client; + this.loader = new DataLoader((keys) => client.mget(...keys), { + cache: false, + }); + } else if (clusterClient) { + this.client = clusterClient; + this.loader = new DataLoader( + (keys) => + Promise.all(keys.map((key) => clusterClient.get(key).catch(() => null))), + { + cache: false, + }, + ); + } else { + throw Error('You must provide `client` or `clusterClient`'); + } } async set( @@ -52,7 +94,7 @@ export class BaseRedisCache implements TestableKeyValueCache { } async delete(key: string): Promise { - return await this.client.del(key) > 0; + return (await this.client.del(key)) > 0; } // Drops all data from Redis. This should only be used by test suites --- diff --git a/packages/apollo-server-cache-redis/src/RedisCache.ts b/packages/apollo-server-cache-redis/src/RedisCache.ts index 4732327efde..d7642f9320a 100644 --- a/packages/apollo-server-cache-redis/src/RedisCache.ts +++ b/packages/apollo-server-cache-redis/src/RedisCache.ts @@ -3,6 +3,6 @@ import { BaseRedisCache } from './BaseRedisCache'; export class RedisCache extends BaseRedisCache { constructor(options?: RedisOptions) { - super(new Redis(options)); + super({ client: new Redis(options) }); } } diff --git a/packages/apollo-server-cache-redis/src/RedisClusterCache.ts b/packages/apollo-server-cache-redis/src/RedisClusterCache.ts index 26ccc2b63a9..fd8544c2ee7 100644 --- a/packages/apollo-server-cache-redis/src/RedisClusterCache.ts +++ b/packages/apollo-server-cache-redis/src/RedisClusterCache.ts @@ -9,16 +9,8 @@ export class RedisClusterCache extends BaseRedisCache { private readonly clusterClient: Redis.Cluster; constructor(nodes: ClusterNode[], options?: ClusterOptions) { - const clusterClient = new Redis.Cluster(nodes, options) - super({ - del: clusterClient.del.bind(clusterClient), - flushdb: clusterClient.flushdb.bind(clusterClient), - mget(...keys: Array): Promise> { - return Promise.all(keys.map(key => clusterClient.get(key).catch(() => null))) - }, - quit: clusterClient.quit.bind(clusterClient), - set: clusterClient.set.bind(clusterClient), - }); + const clusterClient = new Redis.Cluster(nodes, options); + super({ clusterClient }); this.clusterClient = clusterClient; } diff --git a/packages/apollo-server-cache-redis/src/__tests__/BaseRedisCache.test.ts b/packages/apollo-server-cache-redis/src/__tests__/BaseRedisCache.test.ts index 978f18fff5c..4a41e38c364 100644 --- a/packages/apollo-server-cache-redis/src/__tests__/BaseRedisCache.test.ts +++ b/packages/apollo-server-cache-redis/src/__tests__/BaseRedisCache.test.ts @@ -1,21 +1,28 @@ -jest.mock('ioredis'); - import { BaseRedisCache, RedisClient } from '../index'; import { testKeyValueCache_Basics, testKeyValueCache_Expiration, } from '../../../apollo-server-caching/src/__tests__/testsuite'; - describe('BaseRedisCacheTest', () => { const store: { [key: string]: string } = {}; + const timeouts: NodeJS.Timer[] = []; + afterAll(() => { + timeouts.forEach((t) => clearTimeout(t)); + }); const testRedisClient: RedisClient = { - set: jest.fn((key: string, value: string, option?: string, ttl?: number) => { - store[key] = value; - option === 'EX' && ttl && setTimeout(() => delete store[key], ttl * 1000); - return Promise.resolve(); - }), - mget: jest.fn((...keys) => Promise.resolve(keys.map((key: string) => store[key]))), + set: jest.fn( + (key: string, value: string, option?: string, ttl?: number) => { + store[key] = value; + if (option === 'EX' && ttl) { + timeouts.push(setTimeout(() => delete store[key], ttl * 1000)); + } + return Promise.resolve(); + }, + ), + mget: jest.fn((...keys) => + Promise.resolve(keys.map((key: string) => store[key])), + ), flushdb: jest.fn(() => Promise.resolve()), del: jest.fn((key: string) => { const keysDeleted = store.hasOwnProperty(key) ? 1 : 0; @@ -23,9 +30,9 @@ describe('BaseRedisCacheTest', () => { return Promise.resolve(keysDeleted); }), quit: jest.fn(() => Promise.resolve()), - } + }; - const cache = new BaseRedisCache(testRedisClient); + const cache = new BaseRedisCache({ client: testRedisClient }); testKeyValueCache_Basics(cache); testKeyValueCache_Expiration(cache); });