Allow infinite TTL (time-to-live) on KeyValueCache's set.

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
 

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<void> {
     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<string | undefined> {

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<string> {
   readonly client: any;
-  readonly defaultSetOptions = {
+  readonly defaultSetOptions: KeyValueCacheSetOptions = {
     ttl: 300,
   };
 
@@ -22,10 +25,16 @@ export class RedisCache implements TestableKeyValueCache<string> {
   async set(
     key: string,
     value: string,
-    options?: { ttl?: number },
+    options?: KeyValueCacheSetOptions,
   ): Promise<void> {
     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<string | undefined> {

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<void> {
     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<string | undefined> {

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);
 };
 

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<V = string> {

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<V = string> implements KeyValueCache<V> {
   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) {

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');
+    });
   });
 }
 

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';

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

packages/apollo-server-core/src/graphqlOptions.ts

@@ -69,6 +69,14 @@ export type DataSources<TContext> = {
 
 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;

packages/apollo-server-core/src/requestPipeline.ts

@@ -330,9 +330,18 @@ export async function processGraphQLRequest<TContext>(
     // 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(

packages/apollo-server-integration-testsuite/src/index.ts

@@ -1226,16 +1226,20 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => {
         Parameters<GraphQLRequestListener['didEncounterErrors']>
       >;
 
-      beforeEach(async () => {
+      function createMockCache() {
         const map = new Map<string, string>();
-        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')