diff --git a/CHANGELOG.md b/CHANGELOG.md index be83dbb8c92..6d98c2a3238 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,10 +6,11 @@ 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. When a release is being prepared, a new header will be (manually) created below and the the appropriate changes within that release will be moved into the new section. +- `apollo-engine-reporting`: Fix regression introduced by [#3614](https://github.com/apollographql/apollo-server/pull/3614) which caused `PersistedQueryNotFoundError`, `PersistedQueryNotSupportedError` and `InvalidGraphQLRequestError` errors to be triggered before the `requestDidStart` handler triggered `treeBuilder`'s `startTiming` method. This fix preserves the existing behavior by special-casing these specific errors. [PR #3638](https://github.com/apollographql/apollo-server/pull/3638) [Issue #3627](https://github.com/apollographql/apollo-server/issues/3627) - `apollo-server-core`: Upgrade TS to 3.7.3 [#3618](https://github.com/apollographql/apollo-server/pull/3618) - `apollo-server-cloud-functions`: Transmit CORS headers on `OPTIONS` request. [PR #3557](https://github.com/apollographql/apollo-server/pull/3557) - `apollo-server-caching`: De-compose options interface for `KeyValueCache.prototype.set` to accommodate better TSDoc annotations for its properties (e.g. to specify that `ttl` is defined in _seconds_). [PR #3619](https://github.com/apollographql/apollo-server/pull/3619) -- `apollo-engine-reporting`: Fix regression introduced by [#3614](https://github.com/apollographql/apollo-server/pull/3614) which caused `PersistedQueryNotFoundError`, `PersistedQueryNotSupportedError` and `InvalidGraphQLRequestError` errors to be triggered before the `requestDidStart` handler triggered `treeBuilder`'s `startTiming` method. This fix preserves the existing behavior by special-casing these specific errors. [PR #3638](https://github.com/apollographql/apollo-server/pull/3638) [Issue #3627](https://github.com/apollographql/apollo-server/issues/3627) +- `apollo-server-core`, `apollo-server-caching`: Introduce a `ttl` property, specified in seconds, on the options for automated persisted queries (APQ) which applies specific TTL settings to the cache `set`s during APQ registration. Previously, all APQ cache records were set to 300 seconds. Additionally, this adds support (to the underlying `apollo-server-caching` mechanisms) for a time-to-live (TTL) value of `null` which, when supported by the cache implementation, skips the assignment of a TTL value altogether. This allows the cache's controller to determine when eviction happens (e.g. cache forever, and purge least recently used when the cache is full), which may be desireable for network cache stores (e.g. Memcached, Redis). [PR #3623](https://github.com/apollographql/apollo-server/pull/3623) ### v2.9.14 diff --git a/packages/apollo-server-cache-memcached/src/index.ts b/packages/apollo-server-cache-memcached/src/index.ts index 5df0de37b85..25d0f5ced98 100644 --- a/packages/apollo-server-cache-memcached/src/index.ts +++ b/packages/apollo-server-cache-memcached/src/index.ts @@ -1,11 +1,14 @@ -import { TestableKeyValueCache } from 'apollo-server-caching'; +import { + TestableKeyValueCache, + KeyValueCacheSetOptions, +} from 'apollo-server-caching'; import Memcached from 'memcached'; import { promisify } from 'util'; export class MemcachedCache implements TestableKeyValueCache { // FIXME: Replace any with proper promisified type readonly client: any; - readonly defaultSetOptions = { + readonly defaultSetOptions: KeyValueCacheSetOptions = { ttl: 300, }; @@ -23,10 +26,16 @@ export class MemcachedCache implements TestableKeyValueCache { async set( key: string, value: string, - options?: { ttl?: number }, + options?: KeyValueCacheSetOptions, ): Promise { const { ttl } = Object.assign({}, this.defaultSetOptions, options); - await this.client.set(key, value, ttl); + if (typeof ttl === 'number') { + await this.client.set(key, value, ttl); + } else { + // In Memcached, zero indicates no specific expiration time. Of course, + // it may be purged from the cache for other reasons as deemed necessary. + await this.client.set(key, value, 0); + } } async get(key: string): Promise { diff --git a/packages/apollo-server-cache-redis/src/RedisCache.ts b/packages/apollo-server-cache-redis/src/RedisCache.ts index 56e58562015..b45998dea98 100644 --- a/packages/apollo-server-cache-redis/src/RedisCache.ts +++ b/packages/apollo-server-cache-redis/src/RedisCache.ts @@ -1,10 +1,13 @@ -import { TestableKeyValueCache } from 'apollo-server-caching'; +import { + TestableKeyValueCache, + KeyValueCacheSetOptions, +} from 'apollo-server-caching'; import Redis, { RedisOptions } from 'ioredis'; import DataLoader from 'dataloader'; export class RedisCache implements TestableKeyValueCache { readonly client: any; - readonly defaultSetOptions = { + readonly defaultSetOptions: KeyValueCacheSetOptions = { ttl: 300, }; @@ -22,10 +25,16 @@ export class RedisCache implements TestableKeyValueCache { async set( key: string, value: string, - options?: { ttl?: number }, + options?: KeyValueCacheSetOptions, ): Promise { const { ttl } = Object.assign({}, this.defaultSetOptions, options); - await this.client.set(key, value, 'EX', ttl); + if (typeof ttl === 'number') { + await this.client.set(key, value, 'EX', ttl); + } else { + // We'll leave out the EXpiration when no value is specified. Of course, + // it may be purged from the cache for other reasons as deemed necessary. + await this.client.set(key, value); + } } async get(key: string): Promise { diff --git a/packages/apollo-server-cache-redis/src/RedisClusterCache.ts b/packages/apollo-server-cache-redis/src/RedisClusterCache.ts index d265e193db0..79bd73c83f0 100644 --- a/packages/apollo-server-cache-redis/src/RedisClusterCache.ts +++ b/packages/apollo-server-cache-redis/src/RedisClusterCache.ts @@ -1,4 +1,4 @@ -import { KeyValueCache } from 'apollo-server-caching'; +import { KeyValueCache, KeyValueCacheSetOptions } from 'apollo-server-caching'; import Redis, { ClusterOptions, ClusterNode, @@ -8,7 +8,7 @@ import DataLoader from 'dataloader'; export class RedisClusterCache implements KeyValueCache { readonly client: any; - readonly defaultSetOptions = { + readonly defaultSetOptions: KeyValueCacheSetOptions = { ttl: 300, }; @@ -27,10 +27,16 @@ export class RedisClusterCache implements KeyValueCache { async set( key: string, data: string, - options?: { ttl?: number }, + options?: KeyValueCacheSetOptions, ): Promise { const { ttl } = Object.assign({}, this.defaultSetOptions, options); - await this.client.set(key, data, 'EX', ttl); + if (typeof ttl === 'number') { + await this.client.set(key, data, 'EX', ttl); + } else { + // We'll leave out the EXpiration when no value is specified. Of course, + // it may be purged from the cache for other reasons as deemed necessary. + await this.client.set(key, data); + } } async get(key: string): Promise { diff --git a/packages/apollo-server-cache-redis/src/__mocks__/ioredis.ts b/packages/apollo-server-cache-redis/src/__mocks__/ioredis.ts index d5c2c7a98ef..61a38a55f19 100644 --- a/packages/apollo-server-cache-redis/src/__mocks__/ioredis.ts +++ b/packages/apollo-server-cache-redis/src/__mocks__/ioredis.ts @@ -22,9 +22,11 @@ const setKey = (key, value, type, ttl) => { value, ttl, }; - setTimeout(() => { - delete keyValue[key]; - }, ttl * 1000); + if (ttl) { + setTimeout(() => { + delete keyValue[key]; + }, ttl * 1000); + } return Promise.resolve(true); }; diff --git a/packages/apollo-server-caching/src/KeyValueCache.ts b/packages/apollo-server-caching/src/KeyValueCache.ts index 3dd1616379b..88c580ea50c 100644 --- a/packages/apollo-server-caching/src/KeyValueCache.ts +++ b/packages/apollo-server-caching/src/KeyValueCache.ts @@ -1,10 +1,10 @@ /** Options for {@link KeyValueCache.set} */ -interface KeyValueCacheSetOptions { +export interface KeyValueCacheSetOptions { /** * Specified in **seconds**, the time-to-live (TTL) value limits the lifespan * of the data being stored in the cache. */ - ttl?: number + ttl?: number | null }; export interface KeyValueCache { diff --git a/packages/apollo-server-caching/src/PrefixingKeyValueCache.ts b/packages/apollo-server-caching/src/PrefixingKeyValueCache.ts index ffa4c8f04a0..039f4a07679 100644 --- a/packages/apollo-server-caching/src/PrefixingKeyValueCache.ts +++ b/packages/apollo-server-caching/src/PrefixingKeyValueCache.ts @@ -1,4 +1,4 @@ -import { KeyValueCache } from './KeyValueCache'; +import { KeyValueCache, KeyValueCacheSetOptions } from './KeyValueCache'; // PrefixingKeyValueCache wraps another cache and adds a prefix to all keys used // by all operations. This allows multiple features to share the same @@ -16,7 +16,7 @@ export class PrefixingKeyValueCache implements KeyValueCache { get(key: string) { return this.wrapped.get(this.prefix + key); } - set(key: string, value: V, options?: { ttl?: number }) { + set(key: string, value: V, options?: KeyValueCacheSetOptions) { return this.wrapped.set(this.prefix + key, value, options); } delete(key: string) { diff --git a/packages/apollo-server-caching/src/__tests__/testsuite.ts b/packages/apollo-server-caching/src/__tests__/testsuite.ts index c4f8bbd67df..722b6e36923 100644 --- a/packages/apollo-server-caching/src/__tests__/testsuite.ts +++ b/packages/apollo-server-caching/src/__tests__/testsuite.ts @@ -54,6 +54,17 @@ export function testKeyValueCache_Expiration( expect(await keyValueCache.get('short')).toBeUndefined(); expect(await keyValueCache.get('long')).toBeUndefined(); }); + + it('does not expire when ttl is null', async () => { + await keyValueCache.set('forever', 'yours', { ttl: null }); + expect(await keyValueCache.get('forever')).toBe('yours'); + advanceTimeBy(1500); + jest.advanceTimersByTime(1500); + expect(await keyValueCache.get('forever')).toBe('yours'); + advanceTimeBy(4000); + jest.advanceTimersByTime(4000); + expect(await keyValueCache.get('forever')).toBe('yours'); + }); }); } diff --git a/packages/apollo-server-caching/src/index.ts b/packages/apollo-server-caching/src/index.ts index bd49bc740cf..cd94a872b38 100644 --- a/packages/apollo-server-caching/src/index.ts +++ b/packages/apollo-server-caching/src/index.ts @@ -1,3 +1,7 @@ -export { KeyValueCache, TestableKeyValueCache } from './KeyValueCache'; +export { + KeyValueCache, + TestableKeyValueCache, + KeyValueCacheSetOptions, +} from './KeyValueCache'; export { InMemoryLRUCache } from './InMemoryLRUCache'; export { PrefixingKeyValueCache } from './PrefixingKeyValueCache'; diff --git a/packages/apollo-server-core/src/ApolloServer.ts b/packages/apollo-server-core/src/ApolloServer.ts index 1d5ad694a0f..9da7462c125 100644 --- a/packages/apollo-server-core/src/ApolloServer.ts +++ b/packages/apollo-server-core/src/ApolloServer.ts @@ -243,13 +243,14 @@ export class ApolloServerBase { } if (requestOptions.persistedQueries !== false) { + const { + cache: apqCache = requestOptions.cache!, + ...apqOtherOptions + } = requestOptions.persistedQueries || Object.create(null); + requestOptions.persistedQueries = { - cache: new PrefixingKeyValueCache( - (requestOptions.persistedQueries && - requestOptions.persistedQueries.cache) || - requestOptions.cache!, - APQ_CACHE_PREFIX, - ), + cache: new PrefixingKeyValueCache(apqCache, APQ_CACHE_PREFIX), + ...apqOtherOptions, }; } else { // the user does not want to use persisted queries, so we remove the field diff --git a/packages/apollo-server-core/src/graphqlOptions.ts b/packages/apollo-server-core/src/graphqlOptions.ts index 19b0574469c..19015c93bb2 100644 --- a/packages/apollo-server-core/src/graphqlOptions.ts +++ b/packages/apollo-server-core/src/graphqlOptions.ts @@ -69,6 +69,14 @@ export type DataSources = { export interface PersistedQueryOptions { cache: KeyValueCache; + /** + * Specified in **seconds**, this time-to-live (TTL) value limits the lifespan + * of how long the persisted query should be cached. To specify a desired + * lifespan of "infinite", set this to `null`, in which case the eviction will + * be determined by the cache's eviction policy, but the record will never + * simply expire. + */ + ttl?: number | null; } export default GraphQLServerOptions; diff --git a/packages/apollo-server-core/src/requestPipeline.ts b/packages/apollo-server-core/src/requestPipeline.ts index 0986c741682..b40f8bdfdde 100644 --- a/packages/apollo-server-core/src/requestPipeline.ts +++ b/packages/apollo-server-core/src/requestPipeline.ts @@ -330,9 +330,18 @@ export async function processGraphQLRequest( // an error) and not actually write, we'll write to the cache if it was // determined earlier in the request pipeline that we should do so. if (metrics.persistedQueryRegister && persistedQueryCache) { - Promise.resolve(persistedQueryCache.set(queryHash, query)).catch( - console.warn, - ); + Promise.resolve( + persistedQueryCache.set( + queryHash, + query, + config.persistedQueries && + typeof config.persistedQueries.ttl !== 'undefined' + ? { + ttl: config.persistedQueries.ttl, + } + : Object.create(null), + ), + ).catch(console.warn); } let response: GraphQLResponse | null = await dispatcher.invokeHooksUntilNonNull( diff --git a/packages/apollo-server-integration-testsuite/src/index.ts b/packages/apollo-server-integration-testsuite/src/index.ts index 94be1b7dc33..21a65212ca9 100644 --- a/packages/apollo-server-integration-testsuite/src/index.ts +++ b/packages/apollo-server-integration-testsuite/src/index.ts @@ -1226,16 +1226,20 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { Parameters >; - beforeEach(async () => { + function createMockCache() { const map = new Map(); - const cache = { - set: async (key, val) => { + return { + set: jest.fn(async (key, val) => { await map.set(key, val); - }, - get: async key => map.get(key), - delete: async key => map.delete(key), + }), + get: jest.fn(async key => map.get(key)), + delete: jest.fn(async key => map.delete(key)), }; + } + + beforeEach(async () => { didEncounterErrors = jest.fn(); + const cache = createMockCache(); app = await createApp({ graphqlOptions: { schema, @@ -1253,6 +1257,63 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { }); }); + it('when ttlSeconds is set, passes ttl to the apq cache set call', async () => { + const cache = createMockCache(); + app = await createApp({ + graphqlOptions: { + schema, + persistedQueries: { + cache: cache, + ttl: 900, + }, + }, + }); + + await request(app) + .post('/graphql') + .send({ + extensions, + query, + }); + + expect(cache.set).toHaveBeenCalledWith( + expect.stringMatching(/^apq:/), + '{testString}', + expect.objectContaining({ + ttl: 900, + }), + ); + }); + + it('when ttlSeconds is unset, ttl is not passed to apq cache', + async () => { + const cache = createMockCache(); + app = await createApp({ + graphqlOptions: { + schema, + persistedQueries: { + cache: cache, + }, + }, + }); + + await request(app) + .post('/graphql') + .send({ + extensions, + query, + }); + + expect(cache.set).toHaveBeenCalledWith( + expect.stringMatching(/^apq:/), + '{testString}', + expect.not.objectContaining({ + ttl: 900, + }), + ); + } + ); + it('errors when version is not specified', async () => { const result = await request(app) .get('/graphql')