backend-plugin-api: refactor CacheService to lift up client methods to the service

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>

.changeset/chilled-brooms-jog.md

@@ -0,0 +1,7 @@
+---
+'@backstage/backend-common': patch
+---
+
+**BREAKING**: The `CacheClient` interface must now also implement the `withOptions` method. The `.get()` method has also received a type parameter that helps ensure that `undefined` in the event of a cache miss is handled.
+
+Added a `cacheToPluginCacheManager` helper that converts a `CacheService` into a legacy `PluginCacheManager` instance.

.changeset/eighty-knives-repeat.md

@@ -0,0 +1,6 @@
+---
+'@backstage/backend-app-api': patch
+'@backstage/backend-common': patch
+---
+
+Updated to match the new `CacheService` interface.

.changeset/twenty-pugs-explain.md

@@ -0,0 +1,5 @@
+---
+'@backstage/backend-plugin-api': minor
+---
+
+**BREAKING**: The `CacheService` has been changed to remove the indirection of `getClient`, instead making the `CacheClient` methods directly available on the `CacheService`. In order to allow for the creation of clients with default options, there is now a new `.withOptions` method that must be implemented as part of the service interface.

packages/backend-app-api/api-report.md

@@ -6,6 +6,7 @@
 /// <reference types="node" />
 
 import { BackendFeature } from '@backstage/backend-plugin-api';
+import { CacheClient } from '@backstage/backend-common';
 import { Config } from '@backstage/config';
 import { ConfigService } from '@backstage/backend-plugin-api';
 import { CorsOptions } from 'cors';
@@ -22,7 +23,6 @@ import { LifecycleService } from '@backstage/backend-plugin-api';
 import { LoadConfigOptionsRemote } from '@backstage/config-loader';
 import { LoggerService } from '@backstage/backend-plugin-api';
 import { PermissionsService } from '@backstage/backend-plugin-api';
-import { PluginCacheManager } from '@backstage/backend-common';
 import { PluginDatabaseManager } from '@backstage/backend-common';
 import { PluginEndpointDiscovery } from '@backstage/backend-common';
 import { RequestHandler } from 'express';
@@ -48,10 +48,7 @@ export interface Backend {
 }
 
 // @public (undocumented)
-export const cacheServiceFactory: () => ServiceFactory<
-  PluginCacheManager,
-  'plugin'
->;
+export const cacheServiceFactory: () => ServiceFactory<CacheClient, 'plugin'>;
 
 // @public (undocumented)
 export interface ConfigFactoryOptions {

packages/backend-app-api/src/services/implementations/cache/cacheServiceFactory.ts

@@ -31,6 +31,6 @@ export const cacheServiceFactory = createServiceFactory({
     return CacheManager.fromConfig(config);
   },
   async factory({ plugin }, manager) {
-    return manager.forPlugin(plugin.getId());
+    return manager.forPlugin(plugin.getId()).getClient();
   },
 });

packages/backend-common/api-report.md

@@ -13,9 +13,9 @@ import { BackendFeature } from '@backstage/backend-plugin-api';
 import { BitbucketCloudIntegration } from '@backstage/integration';
 import { BitbucketIntegration } from '@backstage/integration';
 import { BitbucketServerIntegration } from '@backstage/integration';
-import { CacheClient } from '@backstage/backend-plugin-api';
-import { CacheClientOptions } from '@backstage/backend-plugin-api';
-import { CacheClientSetOptions } from '@backstage/backend-plugin-api';
+import { CacheService as CacheClient } from '@backstage/backend-plugin-api';
+import { CacheServiceOptions as CacheClientOptions } from '@backstage/backend-plugin-api';
+import { CacheServiceSetOptions as CacheClientSetOptions } from '@backstage/backend-plugin-api';
 import { Config } from '@backstage/config';
 import { ConfigService } from '@backstage/backend-plugin-api';
 import cors from 'cors';
@@ -38,7 +38,6 @@ import { Logger } from 'winston';
 import { LoggerService } from '@backstage/backend-plugin-api';
 import { MergeResult } from 'isomorphic-git';
 import { PermissionsService } from '@backstage/backend-plugin-api';
-import { CacheService as PluginCacheManager } from '@backstage/backend-plugin-api';
 import { DatabaseService as PluginDatabaseManager } from '@backstage/backend-plugin-api';
 import { DiscoveryService as PluginEndpointDiscovery } from '@backstage/backend-plugin-api';
 import { PluginMetadataService } from '@backstage/backend-plugin-api';
@@ -199,6 +198,11 @@ export type CacheManagerOptions = {
   onError?: (err: Error) => void;
 };
 
+// @public (undocumented)
+export function cacheToPluginCacheManager(
+  cache: CacheClient,
+): PluginCacheManager;
+
 // @public
 export const coloredFormat: winston.Logform.Format;
 
@@ -528,7 +532,7 @@ export const legacyPlugin: (
     default: LegacyCreateRouter<
       TransformedEnv<
         {
-          cache: PluginCacheManager;
+          cache: CacheClient;
           config: ConfigService;
           database: PluginDatabaseManager;
           discovery: PluginEndpointDiscovery;
@@ -541,6 +545,7 @@ export const legacyPlugin: (
         },
         {
           logger: (log: LoggerService) => Logger;
+          cache: (cache: CacheClient) => PluginCacheManager;
         }
       >
     >;
@@ -581,7 +586,11 @@ export function makeLegacyPlugin<
 // @public
 export function notFoundHandler(): RequestHandler;
 
-export { PluginCacheManager };
+// @public (undocumented)
+export interface PluginCacheManager {
+  // (undocumented)
+  getClient(options?: CacheClientOptions): CacheClient;
+}
 
 export { PluginDatabaseManager };
 

packages/backend-common/src/cache/CacheClient.test.ts

@@ -32,7 +32,7 @@ describe('CacheClient', () => {
 
   describe('CacheClient.get', () => {
     it('calls client with normalized key', async () => {
-      const sut = new DefaultCacheClient({ client });
+      const sut = new DefaultCacheClient(client, () => client, {});
       const key = 'somekey';
 
       await sut.get(key);
@@ -41,7 +41,7 @@ describe('CacheClient', () => {
     });
 
     it('calls client with normalized key (very long key)', async () => {
-      const sut = new DefaultCacheClient({ client });
+      const sut = new DefaultCacheClient(client, () => client, {});
       const key = 'x'.repeat(251);
 
       await sut.get(key);
@@ -53,7 +53,7 @@ describe('CacheClient', () => {
     });
 
     it('rejects on underlying error', async () => {
-      const sut = new DefaultCacheClient({ client });
+      const sut = new DefaultCacheClient(client, () => client, {});
       const expectedError = new Error('Some runtime error');
       client.get = jest.fn().mockRejectedValue(expectedError);
 
@@ -61,7 +61,7 @@ describe('CacheClient', () => {
     });
 
     it('resolves what underlying client resolves', async () => {
-      const sut = new DefaultCacheClient({ client });
+      const sut = new DefaultCacheClient(client, () => client, {});
       const expectedValue = 'some value';
       client.get = jest.fn().mockResolvedValue(expectedValue);
 
@@ -73,7 +73,7 @@ describe('CacheClient', () => {
 
   describe('CacheClient.set', () => {
     it('calls client with normalized key', async () => {
-      const sut = new DefaultCacheClient({ client });
+      const sut = new DefaultCacheClient(client, () => client, {});
       const key = 'somekey';
 
       await sut.set(key, {});
@@ -84,7 +84,7 @@ describe('CacheClient', () => {
     });
 
     it('passes ttl to client when given', async () => {
-      const sut = new DefaultCacheClient({ client });
+      const sut = new DefaultCacheClient(client, () => client, {});
       const expectedTtl = 3600;
 
       await sut.set('someKey', {}, { ttl: expectedTtl });
@@ -95,7 +95,7 @@ describe('CacheClient', () => {
     });
 
     it('rejects on underlying error if configured', async () => {
-      const sut = new DefaultCacheClient({ client });
+      const sut = new DefaultCacheClient(client, () => client, {});
       const expectedError = new Error('Some runtime error');
       client.set = jest.fn().mockRejectedValue(expectedError);
 
@@ -105,7 +105,7 @@ describe('CacheClient', () => {
 
   describe('CacheClient.delete', () => {
     it('calls client with normalized key', async () => {
-      const sut = new DefaultCacheClient({ client });
+      const sut = new DefaultCacheClient(client, () => client, {});
       const key = 'somekey';
 
       await sut.delete(key);
@@ -116,11 +116,35 @@ describe('CacheClient', () => {
     });
 
     it('rejects on underlying error if configured', async () => {
-      const sut = new DefaultCacheClient({ client });
+      const sut = new DefaultCacheClient(client, () => client, {});
       const expectedError = new Error('Some runtime error');
       client.delete = jest.fn().mockRejectedValue(expectedError);
 
       return expect(sut.delete('someKey')).rejects.toEqual(expectedError);
     });
   });
+
+  describe('CacheClient.withOptions', () => {
+    it('merges together options to create a new instance', async () => {
+      const factory = jest.fn();
+      const sut = new DefaultCacheClient(client, factory, { foo: 1 } as any);
+      expect(factory).not.toHaveBeenCalled();
+
+      const sutA = await sut.withOptions({});
+      expect(factory).toHaveBeenLastCalledWith({ foo: 1 });
+
+      const sutA2 = await sutA.withOptions({ bar: 2 } as any);
+      expect(factory).toHaveBeenCalledWith({ foo: 1, bar: 2 });
+
+      await sutA2.withOptions({ foo: 3 } as any);
+      expect(factory).toHaveBeenCalledWith({ foo: 3, bar: 2 });
+
+      // calling .withOptions should not mutate state
+      const sutB = await sut.withOptions({ foo: 2 } as any);
+      expect(factory).toHaveBeenLastCalledWith({ foo: 2 });
+
+      await sutB.withOptions({ bar: 3 } as any);
+      expect(factory).toHaveBeenLastCalledWith({ foo: 2, bar: 3 });
+    });
+  });
 });

packages/backend-common/src/cache/CacheClient.ts

@@ -15,50 +15,64 @@
  */
 
 import {
-  CacheClient,
-  CacheClientSetOptions,
+  CacheService,
+  CacheServiceOptions,
+  CacheServiceSetOptions,
 } from '@backstage/backend-plugin-api';
 import { JsonValue } from '@backstage/types';
 import { createHash } from 'crypto';
 import Keyv from 'keyv';
 
-export type {
-  CacheClient,
-  CacheClientSetOptions,
-} from '@backstage/backend-plugin-api';
-
-type CacheClientArgs = {
-  client: Keyv;
-};
+export type CacheClientFactory = (options: CacheServiceOptions) => Keyv;
 
 /**
- * A basic, concrete implementation of the CacheClient, suitable for almost
+ * A basic, concrete implementation of the CacheService, suitable for almost
  * all uses in Backstage.
  */
-export class DefaultCacheClient implements CacheClient {
-  private readonly client: Keyv;
+export class DefaultCacheClient implements CacheService {
+  #client: Keyv;
+  #clientFactory: CacheClientFactory;
+  #options: CacheServiceOptions;
 
-  constructor({ client }: CacheClientArgs) {
-    this.client = client;
+  constructor(
+    client: Keyv,
+    clientFactory: CacheClientFactory,
+    options: CacheServiceOptions,
+  ) {
+    this.#client = client;
+    this.#clientFactory = clientFactory;
+    this.#options = options;
   }
 
-  async get(key: string): Promise<JsonValue | undefined> {
+  async get<TValue extends JsonValue>(
+    key: string,
+  ): Promise<TValue | undefined> {
     const k = this.getNormalizedKey(key);
-    return await this.client.get(k);
+    const value = await this.#client.get(k);
+    return value as TValue | undefined;
   }
 
   async set(
     key: string,
     value: JsonValue,
-    opts: CacheClientSetOptions = {},
+    opts: CacheServiceSetOptions = {},
   ): Promise<void> {
     const k = this.getNormalizedKey(key);
-    await this.client.set(k, value, opts.ttl);
+    await this.#client.set(k, value, opts.ttl);
   }
 
   async delete(key: string): Promise<void> {
     const k = this.getNormalizedKey(key);
-    await this.client.delete(k);
+    await this.#client.delete(k);
+  }
+
+  withOptions(options: CacheServiceOptions): CacheService {
+    const newOptions = { ...this.#options, ...options };
+    return new DefaultCacheClient(
+      this.#clientFactory(newOptions),
+      this.#clientFactory,
+      newOptions,
+    );
   }
 
   /**

packages/backend-common/src/cache/CacheManager.test.ts

@@ -94,7 +94,7 @@ describe('CacheManager', () => {
 
       const client = DefaultCacheClient as jest.Mock;
       const mockCalls = client.mock.calls.splice(-1);
-      const realClient = mockCalls[0][0].client as Keyv;
+      const realClient = mockCalls[0][0] as Keyv;
       expect(realClient.on).toHaveBeenCalledWith('error', expect.any(Function));
     });
 
@@ -233,7 +233,7 @@ describe('CacheManager', () => {
       // Retrieve the error handler attached to the cache client.
       const client = DefaultCacheClient as jest.Mock;
       const mockCalls = client.mock.calls.splice(-1);
-      const realClient = mockCalls[0][0].client as Keyv;
+      const realClient = mockCalls[0][0] as Keyv;
       const realOnError = realClient.on as jest.Mock;
       const realHandler = realOnError.mock.calls.splice(-1)[0][1];
 
@@ -259,7 +259,7 @@ describe('CacheManager', () => {
       // Retrieve the error handler attached to the cache client.
       const client = DefaultCacheClient as jest.Mock;
       const mockCalls = client.mock.calls.splice(-1);
-      const realClient = mockCalls[0][0].client as Keyv;
+      const realClient = mockCalls[0][0] as Keyv;
       const realOnError = realClient.on as jest.Mock;
       const realHandler = realOnError.mock.calls.splice(-1)[0][1];
 

packages/backend-common/src/cache/CacheManager.ts

@@ -18,9 +18,13 @@ import { Config } from '@backstage/config';
 import Keyv from 'keyv';
 import KeyvMemcache from '@keyv/memcache';
 import KeyvRedis from '@keyv/redis';
-import { LoggerService } from '@backstage/backend-plugin-api';
+import {
+  CacheService,
+  CacheServiceOptions,
+  LoggerService,
+} from '@backstage/backend-plugin-api';
 import { getRootLogger } from '../logging';
-import { DefaultCacheClient, CacheClient } from './CacheClient';
+import { DefaultCacheClient } from './CacheClient';
 import { NoStore } from './NoStore';
 import { CacheManagerOptions, PluginCacheManager } from './types';
 
@@ -99,23 +103,32 @@ export class CacheManager {
    */
   forPlugin(pluginId: string): PluginCacheManager {
     return {
-      getClient: (opts = {}): CacheClient => {
-        const concreteClient = this.getClientWithTtl(pluginId, opts.defaultTtl);
+      getClient: (defaultOptions = {}) => {
+        const clientFactory = (options: CacheServiceOptions) => {
+          const concreteClient = this.getClientWithTtl(
+            pluginId,
+            options.defaultTtl,
+          );
 
-        // Always provide an error handler to avoid stopping the process.
-        concreteClient.on('error', (err: Error) => {
-          // In all cases, just log the error.
-          this.logger.error('Failed to create cache client', err);
+          // Always provide an error handler to avoid stopping the process.
+          concreteClient.on('error', (err: Error) => {
+            // In all cases, just log the error.
+            this.logger.error('Failed to create cache client', err);
 
-          // Invoke any custom error handler if provided.
-          if (typeof this.errorHandler === 'function') {
-            this.errorHandler(err);
-          }
-        });
+            // Invoke any custom error handler if provided.
+            if (typeof this.errorHandler === 'function') {
+              this.errorHandler(err);
+            }
+          });
 
-        return new DefaultCacheClient({
-          client: concreteClient,
-        });
+          return concreteClient;
+        };
+
+        return new DefaultCacheClient(
+          clientFactory(defaultOptions),
+          clientFactory,
+          defaultOptions,
+        );
       },
     };
   }
@@ -164,3 +177,12 @@ export class CacheManager {
     });
   }
 }
+
+/** @public */
+export function cacheToPluginCacheManager(
+  cache: CacheService,
+): PluginCacheManager {
+  return {
+    getClient: (opts: CacheServiceOptions) => cache.withOptions(opts),
+  };
+}

packages/backend-common/src/cache/index.ts

@@ -14,9 +14,10 @@
  * limitations under the License.
  */
 
-export type { CacheClient, CacheClientSetOptions } from './CacheClient';
-export { CacheManager } from './CacheManager';
+export { CacheManager, cacheToPluginCacheManager } from './CacheManager';
 export type {
+  CacheClient,
+  CacheClientSetOptions,
   PluginCacheManager,
   CacheManagerOptions,
   CacheClientOptions,

packages/backend-common/src/cache/types.ts

@@ -15,10 +15,15 @@
  */
 
 import { LoggerService } from '@backstage/backend-plugin-api';
+import {
+  CacheService,
+  CacheServiceOptions,
+} from '@backstage/backend-plugin-api';
 
 export type {
-  CacheService as PluginCacheManager,
-  CacheClientOptions,
+  CacheService as CacheClient,
+  CacheServiceSetOptions as CacheClientSetOptions,
+  CacheServiceOptions as CacheClientOptions,
 } from '@backstage/backend-plugin-api';
 
 /**
@@ -38,3 +43,10 @@ export type CacheManagerOptions = {
    */
   onError?: (err: Error) => void;
 };
+
+/**
+ * @public
+ */
+export interface PluginCacheManager {
+  getClient(options?: CacheServiceOptions): CacheService;
+}

packages/backend-common/src/legacy.ts

@@ -20,6 +20,7 @@ import {
   ServiceRef,
 } from '@backstage/backend-plugin-api';
 import { RequestHandler } from 'express';
+import { cacheToPluginCacheManager } from './cache';
 import { loggerToWinstonLogger } from './logging';
 
 /**
@@ -119,5 +120,6 @@ export const legacyPlugin = makeLegacyPlugin(
   },
   {
     logger: log => loggerToWinstonLogger(log),
+    cache: cache => cacheToPluginCacheManager(cache),
   },
 );

packages/backend-plugin-api/api-report.md

@@ -72,31 +72,27 @@ export interface BackendPluginRegistrationPoints {
 }
 
 // @public
-export interface CacheClient {
+export interface CacheService {
   delete(key: string): Promise<void>;
-  get(key: string): Promise<JsonValue | undefined>;
+  get<TValue extends JsonValue>(key: string): Promise<TValue | undefined>;
   set(
     key: string,
     value: JsonValue,
-    options?: CacheClientSetOptions,
+    options?: CacheServiceSetOptions,
   ): Promise<void>;
+  withOptions(options: CacheServiceOptions): CacheService;
 }
 
 // @public
-export type CacheClientOptions = {
+export type CacheServiceOptions = {
   defaultTtl?: number;
 };
 
 // @public
-export type CacheClientSetOptions = {
+export type CacheServiceSetOptions = {
   ttl?: number;
 };
 
-// @public
-export interface CacheService {
-  getClient: (options?: CacheClientOptions) => CacheClient;
-}
-
 // @public (undocumented)
 export interface ConfigService extends Config {}
 

packages/backend-plugin-api/src/services/definitions/CacheService.ts

@@ -17,29 +17,11 @@
 import { JsonValue } from '@backstage/types';
 
 /**
- * Manages access to cache stores that plugins get.
+ * Options passed to {@link CacheService.set}.
  *
  * @public
  */
-export interface CacheService {
-  /**
-   * Provides backend plugins cache connections for themselves.
-   *
-   * @remarks
-   *
-   * The purpose of this method is to allow plugins to get isolated data stores
-   * so that plugins are discouraged from cache-level integration and/or cache
-   * key collisions.
-   */
-  getClient: (options?: CacheClientOptions) => CacheClient;
-}
-
-/**
- * Options passed to {@link CacheClient.set}.
- *
- * @public
- */
-export type CacheClientSetOptions = {
+export type CacheServiceSetOptions = {
   /**
    * Optional TTL in milliseconds. Defaults to the TTL provided when the client
    * was set up (or no TTL if none are provided).
@@ -48,17 +30,31 @@ export type CacheClientSetOptions = {
 };
 
 /**
- * A pre-configured, storage agnostic cache client suitable for use by
+ * Options passed to {@link CacheService.withOptions}.
+ *
+ * @public
+ */
+export type CacheServiceOptions = {
+  /**
+   * An optional default TTL (in milliseconds) to be set when getting a client
+   * instance. If not provided, data will persist indefinitely by default (or
+   * can be configured per entry at set-time).
+   */
+  defaultTtl?: number;
+};
+
+/**
+ * A pre-configured, storage agnostic cache service suitable for use by
  * Backstage plugins.
  *
  * @public
  */
-export interface CacheClient {
+export interface CacheService {
   /**
    * Reads data from a cache store for the given key. If no data was found,
    * returns undefined.
    */
-  get(key: string): Promise<JsonValue | undefined>;
+  get<TValue extends JsonValue>(key: string): Promise<TValue | undefined>;
 
   /**
    * Writes the given data to a cache store, associated with the given key. An
@@ -68,25 +64,16 @@ export interface CacheClient {
   set(
     key: string,
     value: JsonValue,
-    options?: CacheClientSetOptions,
+    options?: CacheServiceSetOptions,
   ): Promise<void>;
 
   /**
    * Removes the given key from the cache store.
    */
   delete(key: string): Promise<void>;
-}
 
-/**
- * Options given when constructing a {@link CacheClient}.
- *
- * @public
- */
-export type CacheClientOptions = {
   /**
-   * An optional default TTL (in milliseconds) to be set when getting a client
-   * instance. If not provided, data will persist indefinitely by default (or
-   * can be configured per entry at set-time).
+   * Creates a new {@link CacheService} instance with the given options.
    */
-  defaultTtl?: number;
-};
+  withOptions(options: CacheServiceOptions): CacheService;
+}

packages/backend-plugin-api/src/services/definitions/index.ts

@@ -17,9 +17,8 @@
 export { coreServices } from './coreServices';
 export type {
   CacheService,
-  CacheClient,
-  CacheClientOptions,
-  CacheClientSetOptions,
+  CacheServiceOptions,
+  CacheServiceSetOptions,
 } from './CacheService';
 export type { ConfigService } from './ConfigService';
 export type { DatabaseService } from './DatabaseService';

plugins/adr-backend/src/service/router.test.ts

@@ -107,6 +107,8 @@ class MockCacheClient implements CacheClient {
   async delete(key: string) {
     delete this.itemRegistry[key];
   }
+
+  withOptions = () => this;
 }
 
 describe('createRouter', () => {

plugins/auth-backend/api-report.md

@@ -7,7 +7,7 @@
 
 import { BackstageIdentityResponse } from '@backstage/plugin-auth-node';
 import { BackstageSignInResult } from '@backstage/plugin-auth-node';
-import { CacheClient } from '@backstage/backend-plugin-api';
+import { CacheService } from '@backstage/backend-plugin-api';
 import { CatalogApi } from '@backstage/catalog-client';
 import { Config } from '@backstage/config';
 import { Entity } from '@backstage/catalog-model';
@@ -486,7 +486,7 @@ export const providers: Readonly<{
       signIn: {
         resolver: SignInResolver<CloudflareAccessResult>;
       };
-      cache?: CacheClient | undefined;
+      cache?: CacheService | undefined;
     }) => AuthProviderFactory;
     resolvers: Readonly<{
       emailMatchingUserEntityProfileEmail: () => SignInResolver<unknown>;

plugins/auth-backend/src/providers/cloudflare-access/provider.test.ts

@@ -83,6 +83,7 @@ const mockCacheClient = {
   get: jest.fn(),
   set: jest.fn(),
   delete: jest.fn(),
+  withOptions: jest.fn(),
 };
 
 jest.mock('jose');

plugins/techdocs-backend/src/cache/TechDocsCache.test.ts

@@ -31,6 +31,7 @@ describe('TechDocsCache', () => {
       get: jest.fn(),
       set: jest.fn(),
       delete: jest.fn(),
+      withOptions: jest.fn(),
     };
     CacheUnderTest = TechDocsCache.fromConfig(new ConfigReader({}), {
       cache: MockClient,