From 93ff8275dd050834972dda609723f2a9bf5b33c7 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Sat, 1 May 2021 21:23:50 +0200 Subject: [PATCH 01/19] WIP store-agnostic cache manager. Signed-off-by: Eric Peterson --- packages/backend-common/config.d.ts | 22 +++++ packages/backend-common/package.json | 4 + .../backend-common/src/cache/CacheManager.ts | 87 +++++++++++++++++++ packages/backend-common/src/cache/index.ts | 17 ++++ packages/backend-common/src/index.ts | 1 + packages/backend/src/index.ts | 4 +- packages/backend/src/types.ts | 2 + yarn.lock | 74 ++++++++++++++-- 8 files changed, 202 insertions(+), 9 deletions(-) create mode 100644 packages/backend-common/src/cache/CacheManager.ts create mode 100644 packages/backend-common/src/cache/index.ts diff --git a/packages/backend-common/config.d.ts b/packages/backend-common/config.d.ts index 41845f952e..b4cf8580d9 100644 --- a/packages/backend-common/config.d.ts +++ b/packages/backend-common/config.d.ts @@ -68,6 +68,28 @@ export interface Config { connection: string | object; }; + /** Cache connection configuration, select cache type using the `store` field */ + cache?: + | { + store: 'memory'; + } + | { + store: 'memcache'; + connection: { + /** + * An array of memcache hosts, optionally including a port number. + * e.g. 127.0.0.1:11211 + */ + hosts: string[]; + + /** + * Number of milliseconds to wait before assuming there is a + * network timeout. Defaults to 500ms. + */ + netTimeout?: number; + }; + }; + cors?: { origin?: string | string[]; methods?: string | string[]; diff --git a/packages/backend-common/package.json b/packages/backend-common/package.json index a144f1c23f..513fe2f985 100644 --- a/packages/backend-common/package.json +++ b/packages/backend-common/package.json @@ -36,10 +36,13 @@ "@backstage/integration": "^0.5.2", "@google-cloud/storage": "^5.8.0", "@octokit/rest": "^18.5.3", + "@types/cache-manager": "^3.4.0", "@types/cors": "^2.8.6", "@types/dockerode": "^3.2.1", "@types/express": "^4.17.6", "archiver": "^5.0.2", + "cache-manager": "^3.4.3", + "cache-manager-memcached-store": "^4.0.0", "compression": "^1.7.4", "concat-stream": "^2.0.0", "cors": "^2.8.5", @@ -54,6 +57,7 @@ "knex": "^0.95.1", "lodash": "^4.17.15", "logform": "^2.1.1", + "memcache-pp": "^0.3.3", "minimatch": "^3.0.4", "minimist": "^1.2.5", "morgan": "^1.10.0", diff --git a/packages/backend-common/src/cache/CacheManager.ts b/packages/backend-common/src/cache/CacheManager.ts new file mode 100644 index 0000000000..83e902a743 --- /dev/null +++ b/packages/backend-common/src/cache/CacheManager.ts @@ -0,0 +1,87 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Config } from '@backstage/config'; +import cacheManager from 'cache-manager'; +// @ts-expect-error +import Memcache from 'memcache-pp'; +// @ts-expect-error +import memcachedStore from 'cache-manager-memcached-store'; + +/** + * A Cache Manager, which is able to retrieve a `node-cache-manager`-compliant + * cache client, configured according to app configuration. If no cache is + * configured, or an unknown store is provided, a no-op cache instance will be + * returned. + */ +export class CacheManager { + private readonly storeGetterMap = { + memcache: this.getMemcacheClient, + memory: this.getMemoryClient, + none: this.getNoneClient, + }; + + /** + * Creates a new CacheManager instance by reading from the `backend` config + * section, specifically the `.cache` key. + * + * @param config The loaded application configuration. + */ + static fromConfig(config: Config): CacheManager { + return new CacheManager(config.getConfig('backend.cache')); + } + + private constructor(private readonly config: Config) {} + + getClientWithTtl(ttl: number): cacheManager.Cache { + const store = this.config.getOptionalString( + 'store', + ) as keyof CacheManager['storeGetterMap']; + + if (this.storeGetterMap.hasOwnProperty(store)) { + return this.storeGetterMap[store].call(this, ttl); + } + return this.storeGetterMap.none.call(this, ttl); + } + + private getMemcacheClient(ttl: number): cacheManager.Cache { + const hosts = this.config.getStringArray('connection.hosts'); + const netTimeout = this.config.getOptionalNumber('connection.netTimeout'); + return cacheManager.caching({ + store: memcachedStore, + driver: Memcache, + options: { + hosts, + ...(netTimeout && { netTimeout }), + }, + ttl, + }); + } + + private getMemoryClient(ttl: number): cacheManager.Cache { + return cacheManager.caching({ + store: 'memory', + ttl, + }); + } + + private getNoneClient(ttl: number): cacheManager.Cache { + return cacheManager.caching({ + store: 'none', + ttl, + }); + } +} diff --git a/packages/backend-common/src/cache/index.ts b/packages/backend-common/src/cache/index.ts new file mode 100644 index 0000000000..d405f02f46 --- /dev/null +++ b/packages/backend-common/src/cache/index.ts @@ -0,0 +1,17 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export * from './CacheManager'; diff --git a/packages/backend-common/src/index.ts b/packages/backend-common/src/index.ts index 29b1062750..dab981a824 100644 --- a/packages/backend-common/src/index.ts +++ b/packages/backend-common/src/index.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +export * from './cache'; export * from './config'; export * from './database'; export * from './discovery'; diff --git a/packages/backend/src/index.ts b/packages/backend/src/index.ts index 00ed248df5..82b8e1cb7b 100644 --- a/packages/backend/src/index.ts +++ b/packages/backend/src/index.ts @@ -24,6 +24,7 @@ import Router from 'express-promise-router'; import { + CacheManager, createServiceBuilder, getRootLogger, loadBackendConfig, @@ -59,11 +60,12 @@ function makeCreateEnv(config: Config) { root.info(`Created UrlReader ${reader}`); const databaseManager = SingleConnectionDatabaseManager.fromConfig(config); + const cache = CacheManager.fromConfig(config); return (plugin: string): PluginEnvironment => { const logger = root.child({ type: 'plugin', plugin }); const database = databaseManager.forPlugin(plugin); - return { logger, database, config, reader, discovery }; + return { logger, cache, database, config, reader, discovery }; }; } diff --git a/packages/backend/src/types.ts b/packages/backend/src/types.ts index f63b9dd780..a951562553 100644 --- a/packages/backend/src/types.ts +++ b/packages/backend/src/types.ts @@ -17,6 +17,7 @@ import { Logger } from 'winston'; import { Config } from '@backstage/config'; import { + CacheManager, PluginDatabaseManager, PluginEndpointDiscovery, UrlReader, @@ -24,6 +25,7 @@ import { export type PluginEnvironment = { logger: Logger; + cache: CacheManager; database: PluginDatabaseManager; config: Config; reader: UrlReader; diff --git a/yarn.lock b/yarn.lock index 77607f0637..4b1ebed024 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5737,6 +5737,11 @@ resolved "https://registry.npmjs.org/@types/btoa-lite/-/btoa-lite-1.0.0.tgz#e190a5a548e0b348adb0df9ac7fa5f1151c7cca4" integrity sha512-wJsiX1tosQ+J5+bY5LrSahHxr2wT+uME5UDwdN1kg4frt40euqA+wzECkmq4t5QbveHiJepfdThgQrPw6KiSlg== +"@types/cache-manager@^3.4.0": + version "3.4.0" + resolved "https://registry.npmjs.org/@types/cache-manager/-/cache-manager-3.4.0.tgz#414136ea3807a8cd071b8f20370c5df5dbffd382" + integrity sha512-XVbn2HS+O+Mk2SKRCjr01/8oD5p2Tv1fxxdBqJ0+Cl+UBNiz0WVY5rusHpMGx+qF6Vc2pnRwPVwSKbGaDApCpw== + "@types/cacheable-request@^6.0.1": version "6.0.1" resolved "https://registry.npmjs.org/@types/cacheable-request/-/cacheable-request-6.0.1.tgz#5d22f3dded1fd3a84c0bbeb5039a7419c2c91976" @@ -8152,6 +8157,11 @@ async@0.9.x: resolved "https://registry.npmjs.org/async/-/async-0.9.2.tgz#aea74d5e61c1f899613bf64bda66d4c78f2fd17d" integrity sha1-rqdNXmHB+JlhO/ZL2mbUx48v0X0= +async@3.2.0, async@^3.2.0: + version "3.2.0" + resolved "https://registry.npmjs.org/async/-/async-3.2.0.tgz#b3a2685c5ebb641d3de02d161002c60fc9f85720" + integrity sha512-TR2mEZFVOj2pLStYxLht7TyfuRzaydfpxr3k9RpHIzMgw7A64dzsdqCxH1WJyQdoe8T10nDXd9wnEigmiuHIZw== + async@^2.0.1, async@^2.6.1, async@^2.6.2: version "2.6.3" resolved "https://registry.npmjs.org/async/-/async-2.6.3.tgz#d72625e2344a3656e3a3ad4fa749fa83299d82ff" @@ -8826,7 +8836,7 @@ block-stream@*: dependencies: inherits "~2.0.0" -bluebird@3.7.2, bluebird@^3.3.5, bluebird@^3.5.1, bluebird@^3.5.5, bluebird@^3.7.2: +bluebird@3.7.2, bluebird@^3.3.5, bluebird@^3.4.1, bluebird@^3.5.1, bluebird@^3.5.5, bluebird@^3.7.2: version "3.7.2" resolved "https://registry.npmjs.org/bluebird/-/bluebird-3.7.2.tgz#9f229c15be272454ffa973ace0dbee79a1b0c36f" integrity sha512-XpNj6GDQzdfW+r2Wnn7xiSAd7TM3jzkxGXBGTtWKuSXv1xUV+azxAm8jdWZN06QTQk+2N2XB9jRDkvbmQmcRtg== @@ -9251,6 +9261,20 @@ cache-base@^1.0.1: union-value "^1.0.0" unset-value "^1.0.0" +cache-manager-memcached-store@^4.0.0: + version "4.0.0" + resolved "https://registry.npmjs.org/cache-manager-memcached-store/-/cache-manager-memcached-store-4.0.0.tgz#f68d00cdfaa73696d13137b7b0f39397167d9220" + integrity sha512-Lv1WMaRC1FQHHqxE8Xbi7YWInhgfxjOmLEE6u1K0A3Rwmq+XRLyekKDM7aW9WIzF+PuII/RDKqBK/Ezo5H2QVw== + +cache-manager@^3.4.3: + version "3.4.3" + resolved "https://registry.npmjs.org/cache-manager/-/cache-manager-3.4.3.tgz#c978d58f82b414ade08903d4d72b56a80a4978be" + integrity sha512-6+Hfzy1SNs/thUwo+07pV0ozgxc4sadrAN0eFVGvXl/X9nz3J0BqEnnEoyxEn8jnF+UkEo0MKpyk9BO80hMeiQ== + dependencies: + async "3.2.0" + lodash "^4.17.21" + lru-cache "6.0.0" + cacheable-lookup@^5.0.3: version "5.0.3" resolved "https://registry.npmjs.org/cacheable-lookup/-/cacheable-lookup-5.0.3.tgz#049fdc59dffdd4fc285e8f4f82936591bd59fec3" @@ -9457,6 +9481,11 @@ capture-exit@^2.0.0: dependencies: rsvp "^4.8.4" +carrier@^0.3.0: + version "0.3.0" + resolved "https://registry.npmjs.org/carrier/-/carrier-0.3.0.tgz#bd295d1d3a7524cac63dd779b929ee22a362bad4" + integrity sha1-vSldHTp1JMrGPdd5uSnuIqNiutQ= + case-sensitive-paths-webpack-plugin@^2.2.0: version "2.3.0" resolved "https://registry.npmjs.org/case-sensitive-paths-webpack-plugin/-/case-sensitive-paths-webpack-plugin-2.3.0.tgz#23ac613cc9a856e4f88ff8bb73bbb5e989825cf7" @@ -10273,6 +10302,11 @@ connect-history-api-fallback@^1.6.0: resolved "https://registry.npmjs.org/connect-history-api-fallback/-/connect-history-api-fallback-1.6.0.tgz#8b32089359308d111115d81cad3fceab888f97bc" integrity sha512-e54B99q/OUoH64zYYRf3HBP5z24G38h5D3qXu23JGRoigpX5Ss4r9ZnDk3g0Z8uQC2x2lPaJ+UlWBc1ZWBWdLg== +connection-parse@0.0.x: + version "0.0.7" + resolved "https://registry.npmjs.org/connection-parse/-/connection-parse-0.0.7.tgz#18e7318aab06a699267372b10c5226d25a1c9a69" + integrity sha1-GOcxiqsGppkmc3KxDFIm0locmmk= + console-browserify@^1.1.0: version "1.2.0" resolved "https://registry.npmjs.org/console-browserify/-/console-browserify-1.2.0.tgz#67063cef57ceb6cf4993a2ab3a55840ae8c49336" @@ -14746,6 +14780,14 @@ hash.js@^1.0.0, hash.js@^1.0.3: inherits "^2.0.3" minimalistic-assert "^1.0.1" +hashring@^3.2.0: + version "3.2.0" + resolved "https://registry.npmjs.org/hashring/-/hashring-3.2.0.tgz#fda4efde8aa22cdb97fb1d2a65e88401e1c144ce" + integrity sha1-/aTv3oqiLNuX+x0qZeiEAeHBRM4= + dependencies: + connection-parse "0.0.x" + simple-lru-cache "0.0.x" + hast-util-parse-selector@^2.0.0: version "2.2.4" resolved "https://registry.npmjs.org/hast-util-parse-selector/-/hast-util-parse-selector-2.2.4.tgz#60c99d0b519e12ab4ed32e58f150ec3f61ed1974" @@ -18023,6 +18065,13 @@ lru-cache@2: resolved "https://registry.npmjs.org/lru-cache/-/lru-cache-2.7.3.tgz#6d4524e8b955f95d4f5b58851ce21dd72fb4e952" integrity sha1-bUUk6LlV+V1PW1iFHOId1y+06VI= +lru-cache@6.0.0, lru-cache@^6.0.0: + version "6.0.0" + resolved "https://registry.yarnpkg.com/lru-cache/-/lru-cache-6.0.0.tgz#6d6fe6570ebd96aaf90fcad1dafa3b2566db3a94" + integrity sha512-Jo6dJ04CmSjuznwJSS3pUeWmd/H0ffTlkXXgwZi+eq1UCmqQwCh+eLsYOYCwY991i2Fah4h1BEMCx4qThGbsiA== + dependencies: + yallist "^4.0.0" + lru-cache@^4.0.1: version "4.1.5" resolved "https://registry.npmjs.org/lru-cache/-/lru-cache-4.1.5.tgz#8bbe50ea85bed59bc9e33dcab8235ee9bcf443cd" @@ -18038,13 +18087,6 @@ lru-cache@^5.0.0, lru-cache@^5.1.1: dependencies: yallist "^3.0.2" -lru-cache@^6.0.0: - version "6.0.0" - resolved "https://registry.yarnpkg.com/lru-cache/-/lru-cache-6.0.0.tgz#6d6fe6570ebd96aaf90fcad1dafa3b2566db3a94" - integrity sha512-Jo6dJ04CmSjuznwJSS3pUeWmd/H0ffTlkXXgwZi+eq1UCmqQwCh+eLsYOYCwY991i2Fah4h1BEMCx4qThGbsiA== - dependencies: - yallist "^4.0.0" - lru-queue@^0.1.0: version "0.1.0" resolved "https://registry.npmjs.org/lru-queue/-/lru-queue-0.1.0.tgz#2738bd9f0d3cf4f84490c5736c48699ac632cda3" @@ -18336,6 +18378,17 @@ media-typer@0.3.0: resolved "https://registry.npmjs.org/media-typer/-/media-typer-0.3.0.tgz#8710d7af0aa626f8fffa1ce00168545263255748" integrity sha1-hxDXrwqmJvj/+hzgAWhUUmMlV0g= +memcache-pp@^0.3.3: + version "0.3.3" + resolved "https://registry.npmjs.org/memcache-pp/-/memcache-pp-0.3.3.tgz#3124830e27d55a252499726510841590ac130c99" + integrity sha512-fMitetT5ulUs4DnFaoVqWHrTBiddLG/l8rMHXbV+ibuL7uWjBulaEf8koTBEweyrjmflqhbH4Uy9V709LadXSA== + dependencies: + bluebird "^3.4.1" + carrier "^0.3.0" + debug "^2.6.9" + hashring "^3.2.0" + immutable "^3.8.1" + memoize-one@^5.1.1: version "5.1.1" resolved "https://registry.npmjs.org/memoize-one/-/memoize-one-5.1.1.tgz#047b6e3199b508eaec03504de71229b8eb1d75c0" @@ -23780,6 +23833,11 @@ simple-get@^3.0.2, simple-get@^3.0.3: once "^1.3.1" simple-concat "^1.0.0" +simple-lru-cache@0.0.x: + version "0.0.2" + resolved "https://registry.npmjs.org/simple-lru-cache/-/simple-lru-cache-0.0.2.tgz#d59cc3a193c1a5d0320f84ee732f6e4713e511dd" + integrity sha1-1ZzDoZPBpdAyD4Tucy9uRxPlEd0= + simple-swizzle@^0.2.2: version "0.2.2" resolved "https://registry.npmjs.org/simple-swizzle/-/simple-swizzle-0.2.2.tgz#a4da6b635ffcccca33f70d17cb92592de95e557a" From 6e0be00e1d1a4a27bf314983f9dbc74c7557682c Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Sun, 2 May 2021 15:18:06 +0200 Subject: [PATCH 02/19] Wrap cache clients in a Backstage-specific CacheClient to handle key normalization and error handling + allow us the freedom to move away from cache-manager in the future if necessary. Signed-off-by: Eric Peterson --- app-config.yaml | 2 + .../backend-common/src/cache/CacheClient.ts | 94 +++++++++++++++++++ .../backend-common/src/cache/CacheManager.ts | 43 +++++++-- packages/backend-common/src/cache/index.ts | 1 + packages/backend/src/index.ts | 3 +- packages/backend/src/types.ts | 4 +- 6 files changed, 137 insertions(+), 10 deletions(-) create mode 100644 packages/backend-common/src/cache/CacheClient.ts diff --git a/app-config.yaml b/app-config.yaml index 3c4f2d7ea5..8224b2fb6a 100644 --- a/app-config.yaml +++ b/app-config.yaml @@ -26,6 +26,8 @@ backend: baseUrl: http://localhost:7000 listen: port: 7000 + cache: + store: memory database: client: sqlite3 connection: ':memory:' diff --git a/packages/backend-common/src/cache/CacheClient.ts b/packages/backend-common/src/cache/CacheClient.ts new file mode 100644 index 0000000000..9146131a10 --- /dev/null +++ b/packages/backend-common/src/cache/CacheClient.ts @@ -0,0 +1,94 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { JsonValue } from '@backstage/config'; +import cacheManager from 'cache-manager'; +import { createHash } from 'crypto'; + +type CacheClientArgs = { + client: cacheManager.Cache; + pluginId: string; +}; + +export interface CacheClient { + get(key: string): Promise; + set(key: string, value: JsonValue, ttl?: number): Promise; + delete(key: string): Promise; +} + +export class ConcreteCacheClient implements CacheClient { + private readonly client: cacheManager.Cache; + private readonly pluginId: string; + + constructor({ client, pluginId }: CacheClientArgs) { + this.client = client; + this.pluginId = pluginId; + } + + async get(key: string): Promise { + const k = this.getNormalizedKey(key); + try { + const data = (await this.client.get(k)) as string | undefined; + return this.unserializeData(data); + } catch (_e) { + return null; + } + } + + async set(key: string, value: JsonValue, ttl?: number): Promise { + const k = this.getNormalizedKey(key); + try { + const data = this.serializeData(value); + await this.client.set(k, data, ttl ? { ttl } : undefined); + } catch (_e) { + return; + } + } + + async delete(key: string): Promise { + const k = this.getNormalizedKey(key); + try { + await this.client.del(k); + } catch (_e) { + return; + } + } + + /** + * Namespaces key by plugin to discourage cross-plugin integration via the + * cache store. + */ + private getNormalizedKey(key: string): string { + // Namespace key by plugin ID and remove potentially invalid characters. + const candidateKey = `${this.pluginId}:${key}`; + const wellFormedKey = Buffer.from(candidateKey).toString('base64'); + + // Memcache in particular doesn't do well with keys > 250 bytes. + if (wellFormedKey.length < 250) { + return wellFormedKey; + } + + return createHash('md5').update(candidateKey).digest('hex'); + } + + private serializeData(data: JsonValue): string { + return JSON.stringify(data); + } + + private unserializeData(data: string | undefined): JsonValue { + return data ? JSON.parse(data) : null; + } +} diff --git a/packages/backend-common/src/cache/CacheManager.ts b/packages/backend-common/src/cache/CacheManager.ts index 83e902a743..f1c3b279fd 100644 --- a/packages/backend-common/src/cache/CacheManager.ts +++ b/packages/backend-common/src/cache/CacheManager.ts @@ -14,20 +14,28 @@ * limitations under the License. */ -import { Config } from '@backstage/config'; +import { Config, ConfigReader } from '@backstage/config'; import cacheManager from 'cache-manager'; // @ts-expect-error import Memcache from 'memcache-pp'; // @ts-expect-error import memcachedStore from 'cache-manager-memcached-store'; +import { ConcreteCacheClient, CacheClient } from './CacheClient'; + +export type PluginCacheManager = { + getClient: (ttl: number) => CacheClient; +}; /** - * A Cache Manager, which is able to retrieve a `node-cache-manager`-compliant - * cache client, configured according to app configuration. If no cache is - * configured, or an unknown store is provided, a no-op cache instance will be - * returned. + * Implements a Cache Manager which will automatically create new cache clients + * for plugins when requested. All requested cacheclients are created with the + * credentials provided. */ export class CacheManager { + /** + * Keys represented supported `backend.cache.store` values, mapped to getters + * that return cacheManager.Cache instances appropriate to the store. + */ private readonly storeGetterMap = { memcache: this.getMemcacheClient, memory: this.getMemoryClient, @@ -41,12 +49,33 @@ export class CacheManager { * @param config The loaded application configuration. */ static fromConfig(config: Config): CacheManager { - return new CacheManager(config.getConfig('backend.cache')); + // If no `backend.cache` config is provided, instantiate the CacheManager + // with empty config; allowing a "none" cache client will be returned. + return new CacheManager( + config.getOptionalConfig('backend.cache') || new ConfigReader(undefined), + ); } private constructor(private readonly config: Config) {} - getClientWithTtl(ttl: number): cacheManager.Cache { + /** + * Generates a CacheManagerInstance for consumption by plugins. + * + * @param pluginId The plugin that the cache manager should be created for. Plugin names should be unique. + */ + forPlugin(pluginId: string): PluginCacheManager { + return { + getClient: (ttl: number): CacheClient => { + const concreteClient = this.getClientWithTtl(ttl); + return new ConcreteCacheClient({ + client: concreteClient, + pluginId: pluginId, + }); + }, + }; + } + + private getClientWithTtl(ttl: number): cacheManager.Cache { const store = this.config.getOptionalString( 'store', ) as keyof CacheManager['storeGetterMap']; diff --git a/packages/backend-common/src/cache/index.ts b/packages/backend-common/src/cache/index.ts index d405f02f46..ba1625d043 100644 --- a/packages/backend-common/src/cache/index.ts +++ b/packages/backend-common/src/cache/index.ts @@ -14,4 +14,5 @@ * limitations under the License. */ +export * from './CacheClient'; export * from './CacheManager'; diff --git a/packages/backend/src/index.ts b/packages/backend/src/index.ts index 82b8e1cb7b..d43c8f0101 100644 --- a/packages/backend/src/index.ts +++ b/packages/backend/src/index.ts @@ -60,11 +60,12 @@ function makeCreateEnv(config: Config) { root.info(`Created UrlReader ${reader}`); const databaseManager = SingleConnectionDatabaseManager.fromConfig(config); - const cache = CacheManager.fromConfig(config); + const cacheManager = CacheManager.fromConfig(config); return (plugin: string): PluginEnvironment => { const logger = root.child({ type: 'plugin', plugin }); const database = databaseManager.forPlugin(plugin); + const cache = cacheManager.forPlugin(plugin); return { logger, cache, database, config, reader, discovery }; }; } diff --git a/packages/backend/src/types.ts b/packages/backend/src/types.ts index a951562553..356dd08d5f 100644 --- a/packages/backend/src/types.ts +++ b/packages/backend/src/types.ts @@ -17,7 +17,7 @@ import { Logger } from 'winston'; import { Config } from '@backstage/config'; import { - CacheManager, + PluginCacheManager, PluginDatabaseManager, PluginEndpointDiscovery, UrlReader, @@ -25,7 +25,7 @@ import { export type PluginEnvironment = { logger: Logger; - cache: CacheManager; + cache: PluginCacheManager; database: PluginDatabaseManager; config: Config; reader: UrlReader; From c7fa03cd760c03021b0b2f4e6fedfb83e1572832 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Sun, 2 May 2021 16:30:06 +0200 Subject: [PATCH 03/19] Test coverage for CacheManager and CacheClient Signed-off-by: Eric Peterson --- .../src/cache/CacheClient.test.ts | 141 ++++++++++++++++ .../src/cache/CacheManager.test.ts | 154 ++++++++++++++++++ 2 files changed, 295 insertions(+) create mode 100644 packages/backend-common/src/cache/CacheClient.test.ts create mode 100644 packages/backend-common/src/cache/CacheManager.test.ts diff --git a/packages/backend-common/src/cache/CacheClient.test.ts b/packages/backend-common/src/cache/CacheClient.test.ts new file mode 100644 index 0000000000..aadcee7386 --- /dev/null +++ b/packages/backend-common/src/cache/CacheClient.test.ts @@ -0,0 +1,141 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { ConcreteCacheClient } from './CacheClient'; +import cacheManager from 'cache-manager'; + +describe('CacheClient', () => { + const pluginId = 'test'; + let client: cacheManager.Cache; + const b64 = (k: string) => Buffer.from(k).toString('base64'); + + beforeEach(() => { + client = cacheManager.caching({ store: 'none', ttl: 0 }); + client.get = jest.fn(); + client.set = jest.fn(); + client.del = jest.fn(); + }); + + afterEach(() => jest.resetAllMocks()); + + describe('CacheClient.get', () => { + it('calls client with normalized key', async () => { + const sut = new ConcreteCacheClient({ client, pluginId }); + const keyPartial = 'somekey'; + + await sut.get(keyPartial); + + expect(client.get).toHaveBeenCalledWith(b64(`${pluginId}:${keyPartial}`)); + }); + + it('calls client with normalized key (very long key)', async () => { + const sut = new ConcreteCacheClient({ client, pluginId }); + const keyPartial = 'x'.repeat(251); + + await sut.get(keyPartial); + + const spy = client.get as jest.Mock; + const actualKey = spy.mock.calls[0][0]; + expect(actualKey).not.toEqual(b64(`${pluginId}:${keyPartial}`)); + expect(actualKey.length).toBeLessThan(250); + }); + + it('performs deserialization on returned data', async () => { + const expectedValue = { some: 'value' }; + const sut = new ConcreteCacheClient({ client, pluginId }); + client.get = jest.fn().mockResolvedValue(JSON.stringify(expectedValue)); + + const actualValue = await sut.get('someKey'); + + expect(actualValue).toMatchObject(expectedValue); + }); + + it('returns null on any underlying error', async () => { + const sut = new ConcreteCacheClient({ client, pluginId }); + client.get = jest.fn().mockRejectedValue(undefined); + + const actualValue = await sut.get('someKey'); + + expect(actualValue).toStrictEqual(null); + }); + }); + + describe('CacheClient.set', () => { + it('calls client with normalized key', async () => { + const sut = new ConcreteCacheClient({ client, pluginId }); + const keyPartial = 'somekey'; + + await sut.set(keyPartial, {}); + + const spy = client.set as jest.Mock; + const actualKey = spy.mock.calls[0][0]; + expect(actualKey).toEqual(b64(`${pluginId}:${keyPartial}`)); + }); + + it('performs serialization on given data', async () => { + const sut = new ConcreteCacheClient({ client, pluginId }); + const expectedData = { some: 'value' }; + + await sut.set('someKey', expectedData); + + const spy = client.set as jest.Mock; + const actualData = spy.mock.calls[0][1]; + expect(actualData).toEqual(JSON.stringify(expectedData)); + }); + + it('passes ttl to client when given', async () => { + const sut = new ConcreteCacheClient({ client, pluginId }); + const expectedTtl = 3600; + + await sut.set('someKey', {}, expectedTtl); + + const spy = client.set as jest.Mock; + const actualOptions = spy.mock.calls[0][2]; + expect(actualOptions.ttl).toEqual(expectedTtl); + }); + + it('does not throw errors on any client error', async () => { + const sut = new ConcreteCacheClient({ client, pluginId }); + client.set = jest.fn().mockRejectedValue(undefined); + + expect(async () => { + await sut.set('someKey', {}); + }).not.toThrow(); + }); + }); + + describe('CacheClient.delete', () => { + it('calls client with normalized key', async () => { + const sut = new ConcreteCacheClient({ client, pluginId }); + const keyPartial = 'somekey'; + + await sut.delete(keyPartial); + + const spy = client.del as jest.Mock; + const actualKey = spy.mock.calls[0][0]; + expect(actualKey).toEqual(b64(`${pluginId}:${keyPartial}`)); + }); + + it('does not throw errors on any client error', async () => { + const sut = new ConcreteCacheClient({ client, pluginId }); + client.del = jest.fn().mockRejectedValue(undefined); + + expect(async () => { + await sut.delete('someKey'); + }).not.toThrow(); + }); + }); +}); diff --git a/packages/backend-common/src/cache/CacheManager.test.ts b/packages/backend-common/src/cache/CacheManager.test.ts new file mode 100644 index 0000000000..af8450975a --- /dev/null +++ b/packages/backend-common/src/cache/CacheManager.test.ts @@ -0,0 +1,154 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { ConfigReader } from '@backstage/config'; +import cacheManager from 'cache-manager'; +import { ConcreteCacheClient } from './CacheClient'; +import { CacheManager } from './CacheManager'; + +cacheManager.caching = jest.fn() as jest.Mock; +jest.mock('./CacheClient', () => { + return { + ConcreteCacheClient: jest.fn(), + }; +}); + +describe('CacheManager', () => { + const defaultConfigOptions = { + backend: { + cache: { + store: 'memory', + }, + }, + }; + const defaultConfig = () => new ConfigReader(defaultConfigOptions); + + afterEach(() => jest.resetAllMocks()); + + describe('CacheManager.fromConfig', () => { + it('accesses the backend.cache key', () => { + const getOptionalConfig = jest.fn(); + const config = defaultConfig(); + config.getOptionalConfig = getOptionalConfig; + + CacheManager.fromConfig(config); + + expect(getOptionalConfig.mock.calls[0][0]).toEqual('backend.cache'); + }); + + it('does not require the backend.cache key', () => { + const config = new ConfigReader({ backend: {} }); + expect(() => { + CacheManager.fromConfig(config); + }).not.toThrowError(); + }); + }); + + describe('CacheManager.forPlugin', () => { + const manager = CacheManager.fromConfig(defaultConfig()); + + it('connects to a cache store scoped to the plugin', async () => { + const pluginId = 'test1'; + const expectedTtl = 3600; + manager.forPlugin(pluginId).getClient(expectedTtl); + + const client = ConcreteCacheClient as jest.Mock; + expect(client).toHaveBeenCalledTimes(1); + }); + + it('provides different plugins different cache clients', async () => { + const plugin1Id = 'test1'; + const plugin2Id = 'test2'; + const expectedTtl = 3600; + manager.forPlugin(plugin1Id).getClient(expectedTtl); + manager.forPlugin(plugin2Id).getClient(expectedTtl); + + const cache = cacheManager.caching as jest.Mock; + const client = ConcreteCacheClient as jest.Mock; + expect(cache).toHaveBeenCalledTimes(2); + expect(client).toHaveBeenCalledTimes(2); + + const plugin1CallArgs = client.mock.calls[0]; + const plugin2CallArgs = client.mock.calls[1]; + expect(plugin1CallArgs[0].pluginId).not.toEqual( + plugin2CallArgs[0].pluginId, + ); + }); + }); + + describe('CacheManager.forPlugin stores', () => { + it('returns none client when no cache is configured', () => { + const manager = CacheManager.fromConfig( + new ConfigReader({ backend: {} }), + ); + const expectedTtl = 3600; + manager.forPlugin('test').getClient(expectedTtl); + + const cache = cacheManager.caching as jest.Mock; + const mockCalls = cache.mock.calls.splice(-1); + const callArgs = mockCalls[0]; + expect(callArgs[0]).toMatchObject({ + store: 'none', + ttl: expectedTtl, + }); + }); + + it('returns memory client when configured', () => { + const manager = CacheManager.fromConfig(defaultConfig()); + const expectedTtl = 3600; + manager.forPlugin('test').getClient(expectedTtl); + + const cache = cacheManager.caching as jest.Mock; + const mockCalls = cache.mock.calls.splice(-1); + const callArgs = mockCalls[0]; + expect(callArgs[0]).toMatchObject({ + store: 'memory', + ttl: expectedTtl, + }); + }); + + it('returns a memcache client when configured', () => { + const expectedHost = '127.0.0.1:11211'; + const expectedTimeout = 1000; + const manager = CacheManager.fromConfig( + new ConfigReader({ + backend: { + cache: { + store: 'memcache', + connection: { + hosts: [expectedHost], + netTimeout: expectedTimeout, + }, + }, + }, + }), + ); + const expectedTtl = 3600; + manager.forPlugin('test').getClient(expectedTtl); + + const cache = cacheManager.caching as jest.Mock; + const mockCalls = cache.mock.calls.splice(-1); + const callArgs = mockCalls[0]; + expect(callArgs[0]).toMatchObject({ + options: { + hosts: [expectedHost], + netTimeout: expectedTimeout, + }, + ttl: expectedTtl, + }); + }); + }); +}); From b57fa279c5885e89ff25e757048c28a34724c6df Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Sun, 2 May 2021 16:45:47 +0200 Subject: [PATCH 04/19] Clean up and document exported types Signed-off-by: Eric Peterson --- .../backend-common/src/cache/CacheClient.ts | 21 +++++++++++++ .../backend-common/src/cache/CacheManager.ts | 5 +-- packages/backend-common/src/cache/index.ts | 3 +- packages/backend-common/src/cache/types.ts | 31 +++++++++++++++++++ 4 files changed, 55 insertions(+), 5 deletions(-) create mode 100644 packages/backend-common/src/cache/types.ts diff --git a/packages/backend-common/src/cache/CacheClient.ts b/packages/backend-common/src/cache/CacheClient.ts index 9146131a10..0716b159cb 100644 --- a/packages/backend-common/src/cache/CacheClient.ts +++ b/packages/backend-common/src/cache/CacheClient.ts @@ -23,12 +23,33 @@ type CacheClientArgs = { pluginId: string; }; +/** + * A pre-configured, storage agnostic cache client suitable for use by + * Backstage plugins. + */ export interface CacheClient { + /** + * Reads data from a cache store for the given key. + */ get(key: string): Promise; + + /** + * Writes the given data to a cache store, associated with the given key. An + * optional TTL may also be provided, otherwise it defaults to the TTL that + * was provided when the client was instantiated. + */ set(key: string, value: JsonValue, ttl?: number): Promise; + + /** + * Removes the given key from the cache store. + */ delete(key: string): Promise; } +/** + * A simple, concrete implementation of the CacheClient, suitable for almost + * all uses in Backstage. + */ export class ConcreteCacheClient implements CacheClient { private readonly client: cacheManager.Cache; private readonly pluginId: string; diff --git a/packages/backend-common/src/cache/CacheManager.ts b/packages/backend-common/src/cache/CacheManager.ts index f1c3b279fd..de27bb0e3d 100644 --- a/packages/backend-common/src/cache/CacheManager.ts +++ b/packages/backend-common/src/cache/CacheManager.ts @@ -21,10 +21,7 @@ import Memcache from 'memcache-pp'; // @ts-expect-error import memcachedStore from 'cache-manager-memcached-store'; import { ConcreteCacheClient, CacheClient } from './CacheClient'; - -export type PluginCacheManager = { - getClient: (ttl: number) => CacheClient; -}; +import { PluginCacheManager } from './types'; /** * Implements a Cache Manager which will automatically create new cache clients diff --git a/packages/backend-common/src/cache/index.ts b/packages/backend-common/src/cache/index.ts index ba1625d043..c233621bd4 100644 --- a/packages/backend-common/src/cache/index.ts +++ b/packages/backend-common/src/cache/index.ts @@ -14,5 +14,6 @@ * limitations under the License. */ -export * from './CacheClient'; +export type { CacheClient } from './CacheClient'; export * from './CacheManager'; +export * from './types'; diff --git a/packages/backend-common/src/cache/types.ts b/packages/backend-common/src/cache/types.ts new file mode 100644 index 0000000000..f1b9e96709 --- /dev/null +++ b/packages/backend-common/src/cache/types.ts @@ -0,0 +1,31 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { CacheClient } from './CacheClient'; + +/** + * The PluginCacheManager manages access to cache stores that Plugins get. + */ +export type PluginCacheManager = { + /** + * getClient provides backend plugins cache connections for itself. + * + * 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: (ttl: number) => CacheClient; +}; From 669ff5ee2ecca049c25008db86d2b93221831355 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Sun, 2 May 2021 16:46:12 +0200 Subject: [PATCH 05/19] Declare new public API Signed-off-by: Eric Peterson --- packages/backend-common/api-report.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/packages/backend-common/api-report.md b/packages/backend-common/api-report.md index f0cb066dc0..339a291ff3 100644 --- a/packages/backend-common/api-report.md +++ b/packages/backend-common/api-report.md @@ -16,6 +16,7 @@ import { GithubCredentialsProvider } from '@backstage/integration'; import { GitHubIntegration } from '@backstage/integration'; import { GitLabIntegration } from '@backstage/integration'; import * as http from 'http'; +import { JsonValue } from '@backstage/config'; import { Knex } from 'knex'; import { Logger } from 'winston'; import { MergeResult } from 'isomorphic-git'; @@ -62,6 +63,19 @@ export class BitbucketUrlReader implements UrlReader { toString(): string; } +// @public +export interface CacheClient { + delete(key: string): Promise; + get(key: string): Promise; + set(key: string, value: JsonValue, ttl?: number): Promise; +} + +// @public +export class CacheManager { + forPlugin(pluginId: string): PluginCacheManager; + static fromConfig(config: Config): CacheManager; + } + // @public (undocumented) export const coloredFormat: winston.Logform.Format; @@ -238,6 +252,11 @@ export function loadBackendConfig(options: Options): Promise; // @public export function notFoundHandler(): RequestHandler; +// @public +export type PluginCacheManager = { + getClient: (ttl: number) => CacheClient; +}; + // @public export interface PluginDatabaseManager { getClient(): Promise; From 22fd8ce2ac4cfccef1aa8d1af30a8af6c0011e60 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Sun, 2 May 2021 17:10:10 +0200 Subject: [PATCH 06/19] Changeset Signed-off-by: Eric Peterson --- .changeset/cool-cache-bro.md | 44 ++++++++++++++++++++++++++++++++++++ .github/styles/vocab.txt | 1 + 2 files changed, 45 insertions(+) create mode 100644 .changeset/cool-cache-bro.md diff --git a/.changeset/cool-cache-bro.md b/.changeset/cool-cache-bro.md new file mode 100644 index 0000000000..f86dc39339 --- /dev/null +++ b/.changeset/cool-cache-bro.md @@ -0,0 +1,44 @@ +--- +'@backstage/backend-common': minor +--- + +Introducing: a standard API for App Integrators to configure cache stores and Plugin Developers to interact with them. + +Two cache stores are currently supported. + +- `memory`, which is a very simple in-memory LRU cache, intended for local development. +- `memcache`, which can be used to connect to one or more memcache hosts. + +Configuring and working with cache stores is very similar to the process for database connections. + +```yaml +backend: + cache: + store: memcache + connection: + hosts: + - cache-a.example.com:11211 + - cache-b.example.com:11211 +``` + +```typescript +import { CacheManager } from '@backstage/backend-common'; + +// Instantiating a cache client for a plugin. +const cacheManager = CacheManager.fromConfig(config); +const somePluginCache = cacheManager.forPlugin('somePlugin'); +const defaultTtl = 3600; +const cacheClient = somePluginCache.getClient(defaultTtl); + +// Using the cache client: +const cachedValue = await cacheClient.get('someKey'); +if (cachedValue) { + return cachedValue; +} else { + const someValue = await someExpensiveProcess(); + await cacheClient.set('someKey', someValue); +} +await cacheClient.delete('someKey'); +``` + +Configuring a cache store is optional. Even when no cache store is configured, the cache manager will dutifully pass plugins a manager that resolves a cache client that does not actually write or read any data. diff --git a/.github/styles/vocab.txt b/.github/styles/vocab.txt index 694226b8cc..ff856239bf 100644 --- a/.github/styles/vocab.txt +++ b/.github/styles/vocab.txt @@ -167,6 +167,7 @@ mailto maintainership makefile md +memcache microsite middleware minikube From ac8f7e4666c9c8e856e141d1ac0b060079e1a747 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Sun, 2 May 2021 17:27:01 +0200 Subject: [PATCH 07/19] Dev dependency. Signed-off-by: Eric Peterson --- packages/backend-common/package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/backend-common/package.json b/packages/backend-common/package.json index 513fe2f985..0f331ccc6b 100644 --- a/packages/backend-common/package.json +++ b/packages/backend-common/package.json @@ -80,6 +80,7 @@ "@backstage/cli": "^0.6.10", "@backstage/test-utils": "^0.1.10", "@types/archiver": "^5.1.0", + "@types/cache-manager": "^3.4.0", "@types/compression": "^1.7.0", "@types/concat-stream": "^1.6.0", "@types/fs-extra": "^9.0.3", From b73dc5f46e19fcc45400279e25c0814d144f1850 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Mon, 3 May 2021 16:57:59 +0200 Subject: [PATCH 08/19] Clean up some types Signed-off-by: Eric Peterson --- .../src/cache/CacheClient.test.ts | 24 ++++++------- .../backend-common/src/cache/CacheClient.ts | 28 +++++++++------ .../src/cache/CacheManager.test.ts | 20 +++++------ .../backend-common/src/cache/CacheManager.ts | 35 ++++++++++--------- packages/backend-common/src/cache/index.ts | 4 +-- packages/backend-common/src/cache/types.ts | 6 +++- 6 files changed, 65 insertions(+), 52 deletions(-) diff --git a/packages/backend-common/src/cache/CacheClient.test.ts b/packages/backend-common/src/cache/CacheClient.test.ts index aadcee7386..5d56d84c61 100644 --- a/packages/backend-common/src/cache/CacheClient.test.ts +++ b/packages/backend-common/src/cache/CacheClient.test.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { ConcreteCacheClient } from './CacheClient'; +import { DefaultCacheClient } from './CacheClient'; import cacheManager from 'cache-manager'; describe('CacheClient', () => { @@ -33,7 +33,7 @@ describe('CacheClient', () => { describe('CacheClient.get', () => { it('calls client with normalized key', async () => { - const sut = new ConcreteCacheClient({ client, pluginId }); + const sut = new DefaultCacheClient({ client, pluginId }); const keyPartial = 'somekey'; await sut.get(keyPartial); @@ -42,7 +42,7 @@ describe('CacheClient', () => { }); it('calls client with normalized key (very long key)', async () => { - const sut = new ConcreteCacheClient({ client, pluginId }); + const sut = new DefaultCacheClient({ client, pluginId }); const keyPartial = 'x'.repeat(251); await sut.get(keyPartial); @@ -55,7 +55,7 @@ describe('CacheClient', () => { it('performs deserialization on returned data', async () => { const expectedValue = { some: 'value' }; - const sut = new ConcreteCacheClient({ client, pluginId }); + const sut = new DefaultCacheClient({ client, pluginId }); client.get = jest.fn().mockResolvedValue(JSON.stringify(expectedValue)); const actualValue = await sut.get('someKey'); @@ -64,7 +64,7 @@ describe('CacheClient', () => { }); it('returns null on any underlying error', async () => { - const sut = new ConcreteCacheClient({ client, pluginId }); + const sut = new DefaultCacheClient({ client, pluginId }); client.get = jest.fn().mockRejectedValue(undefined); const actualValue = await sut.get('someKey'); @@ -75,7 +75,7 @@ describe('CacheClient', () => { describe('CacheClient.set', () => { it('calls client with normalized key', async () => { - const sut = new ConcreteCacheClient({ client, pluginId }); + const sut = new DefaultCacheClient({ client, pluginId }); const keyPartial = 'somekey'; await sut.set(keyPartial, {}); @@ -86,7 +86,7 @@ describe('CacheClient', () => { }); it('performs serialization on given data', async () => { - const sut = new ConcreteCacheClient({ client, pluginId }); + const sut = new DefaultCacheClient({ client, pluginId }); const expectedData = { some: 'value' }; await sut.set('someKey', expectedData); @@ -97,10 +97,10 @@ describe('CacheClient', () => { }); it('passes ttl to client when given', async () => { - const sut = new ConcreteCacheClient({ client, pluginId }); + const sut = new DefaultCacheClient({ client, pluginId }); const expectedTtl = 3600; - await sut.set('someKey', {}, expectedTtl); + await sut.set('someKey', {}, { ttl: expectedTtl }); const spy = client.set as jest.Mock; const actualOptions = spy.mock.calls[0][2]; @@ -108,7 +108,7 @@ describe('CacheClient', () => { }); it('does not throw errors on any client error', async () => { - const sut = new ConcreteCacheClient({ client, pluginId }); + const sut = new DefaultCacheClient({ client, pluginId }); client.set = jest.fn().mockRejectedValue(undefined); expect(async () => { @@ -119,7 +119,7 @@ describe('CacheClient', () => { describe('CacheClient.delete', () => { it('calls client with normalized key', async () => { - const sut = new ConcreteCacheClient({ client, pluginId }); + const sut = new DefaultCacheClient({ client, pluginId }); const keyPartial = 'somekey'; await sut.delete(keyPartial); @@ -130,7 +130,7 @@ describe('CacheClient', () => { }); it('does not throw errors on any client error', async () => { - const sut = new ConcreteCacheClient({ client, pluginId }); + const sut = new DefaultCacheClient({ client, pluginId }); client.del = jest.fn().mockRejectedValue(undefined); expect(async () => { diff --git a/packages/backend-common/src/cache/CacheClient.ts b/packages/backend-common/src/cache/CacheClient.ts index 0716b159cb..68c762eb87 100644 --- a/packages/backend-common/src/cache/CacheClient.ts +++ b/packages/backend-common/src/cache/CacheClient.ts @@ -23,6 +23,10 @@ type CacheClientArgs = { pluginId: string; }; +type CacheSetOptions = { + ttl?: number; +}; + /** * A pre-configured, storage agnostic cache client suitable for use by * Backstage plugins. @@ -38,7 +42,7 @@ export interface CacheClient { * optional TTL may also be provided, otherwise it defaults to the TTL that * was provided when the client was instantiated. */ - set(key: string, value: JsonValue, ttl?: number): Promise; + set(key: string, value: JsonValue, options: CacheSetOptions): Promise; /** * Removes the given key from the cache store. @@ -50,7 +54,7 @@ export interface CacheClient { * A simple, concrete implementation of the CacheClient, suitable for almost * all uses in Backstage. */ -export class ConcreteCacheClient implements CacheClient { +export class DefaultCacheClient implements CacheClient { private readonly client: cacheManager.Cache; private readonly pluginId: string; @@ -63,18 +67,22 @@ export class ConcreteCacheClient implements CacheClient { const k = this.getNormalizedKey(key); try { const data = (await this.client.get(k)) as string | undefined; - return this.unserializeData(data); - } catch (_e) { + return this.deserializeData(data); + } catch { return null; } } - async set(key: string, value: JsonValue, ttl?: number): Promise { + async set( + key: string, + value: JsonValue, + opts: CacheSetOptions = {}, + ): Promise { const k = this.getNormalizedKey(key); try { const data = this.serializeData(value); - await this.client.set(k, data, ttl ? { ttl } : undefined); - } catch (_e) { + await this.client.set(k, data, opts.ttl ? { ttl: opts.ttl } : undefined); + } catch { return; } } @@ -83,7 +91,7 @@ export class ConcreteCacheClient implements CacheClient { const k = this.getNormalizedKey(key); try { await this.client.del(k); - } catch (_e) { + } catch { return; } } @@ -102,14 +110,14 @@ export class ConcreteCacheClient implements CacheClient { return wellFormedKey; } - return createHash('md5').update(candidateKey).digest('hex'); + return createHash('md5').update(candidateKey).digest('base64'); } private serializeData(data: JsonValue): string { return JSON.stringify(data); } - private unserializeData(data: string | undefined): JsonValue { + private deserializeData(data: string | undefined): JsonValue { return data ? JSON.parse(data) : null; } } diff --git a/packages/backend-common/src/cache/CacheManager.test.ts b/packages/backend-common/src/cache/CacheManager.test.ts index af8450975a..625c772dec 100644 --- a/packages/backend-common/src/cache/CacheManager.test.ts +++ b/packages/backend-common/src/cache/CacheManager.test.ts @@ -16,13 +16,13 @@ import { ConfigReader } from '@backstage/config'; import cacheManager from 'cache-manager'; -import { ConcreteCacheClient } from './CacheClient'; +import { DefaultCacheClient } from './CacheClient'; import { CacheManager } from './CacheManager'; cacheManager.caching = jest.fn() as jest.Mock; jest.mock('./CacheClient', () => { return { - ConcreteCacheClient: jest.fn(), + DefaultCacheClient: jest.fn(), }; }); @@ -63,9 +63,9 @@ describe('CacheManager', () => { it('connects to a cache store scoped to the plugin', async () => { const pluginId = 'test1'; const expectedTtl = 3600; - manager.forPlugin(pluginId).getClient(expectedTtl); + manager.forPlugin(pluginId).getClient({ defaultTtl: expectedTtl }); - const client = ConcreteCacheClient as jest.Mock; + const client = DefaultCacheClient as jest.Mock; expect(client).toHaveBeenCalledTimes(1); }); @@ -73,11 +73,11 @@ describe('CacheManager', () => { const plugin1Id = 'test1'; const plugin2Id = 'test2'; const expectedTtl = 3600; - manager.forPlugin(plugin1Id).getClient(expectedTtl); - manager.forPlugin(plugin2Id).getClient(expectedTtl); + manager.forPlugin(plugin1Id).getClient({ defaultTtl: expectedTtl }); + manager.forPlugin(plugin2Id).getClient({ defaultTtl: expectedTtl }); const cache = cacheManager.caching as jest.Mock; - const client = ConcreteCacheClient as jest.Mock; + const client = DefaultCacheClient as jest.Mock; expect(cache).toHaveBeenCalledTimes(2); expect(client).toHaveBeenCalledTimes(2); @@ -95,7 +95,7 @@ describe('CacheManager', () => { new ConfigReader({ backend: {} }), ); const expectedTtl = 3600; - manager.forPlugin('test').getClient(expectedTtl); + manager.forPlugin('test').getClient({ defaultTtl: expectedTtl }); const cache = cacheManager.caching as jest.Mock; const mockCalls = cache.mock.calls.splice(-1); @@ -109,7 +109,7 @@ describe('CacheManager', () => { it('returns memory client when configured', () => { const manager = CacheManager.fromConfig(defaultConfig()); const expectedTtl = 3600; - manager.forPlugin('test').getClient(expectedTtl); + manager.forPlugin('test').getClient({ defaultTtl: expectedTtl }); const cache = cacheManager.caching as jest.Mock; const mockCalls = cache.mock.calls.splice(-1); @@ -137,7 +137,7 @@ describe('CacheManager', () => { }), ); const expectedTtl = 3600; - manager.forPlugin('test').getClient(expectedTtl); + manager.forPlugin('test').getClient({ defaultTtl: expectedTtl }); const cache = cacheManager.caching as jest.Mock; const mockCalls = cache.mock.calls.splice(-1); diff --git a/packages/backend-common/src/cache/CacheManager.ts b/packages/backend-common/src/cache/CacheManager.ts index de27bb0e3d..065a0b1db5 100644 --- a/packages/backend-common/src/cache/CacheManager.ts +++ b/packages/backend-common/src/cache/CacheManager.ts @@ -20,7 +20,7 @@ import cacheManager from 'cache-manager'; import Memcache from 'memcache-pp'; // @ts-expect-error import memcachedStore from 'cache-manager-memcached-store'; -import { ConcreteCacheClient, CacheClient } from './CacheClient'; +import { DefaultCacheClient, CacheClient } from './CacheClient'; import { PluginCacheManager } from './types'; /** @@ -30,10 +30,11 @@ import { PluginCacheManager } from './types'; */ export class CacheManager { /** - * Keys represented supported `backend.cache.store` values, mapped to getters - * that return cacheManager.Cache instances appropriate to the store. + * Keys represented supported `backend.cache.store` values, mapped to + * factories that return cacheManager.Cache instances appropriate to the + * store. */ - private readonly storeGetterMap = { + private readonly storeFactories = { memcache: this.getMemcacheClient, memory: this.getMemoryClient, none: this.getNoneClient, @@ -62,9 +63,9 @@ export class CacheManager { */ forPlugin(pluginId: string): PluginCacheManager { return { - getClient: (ttl: number): CacheClient => { - const concreteClient = this.getClientWithTtl(ttl); - return new ConcreteCacheClient({ + getClient: ({ defaultTtl }): CacheClient => { + const concreteClient = this.getClientWithTtl(defaultTtl); + return new DefaultCacheClient({ client: concreteClient, pluginId: pluginId, }); @@ -75,15 +76,15 @@ export class CacheManager { private getClientWithTtl(ttl: number): cacheManager.Cache { const store = this.config.getOptionalString( 'store', - ) as keyof CacheManager['storeGetterMap']; + ) as keyof CacheManager['storeFactories']; - if (this.storeGetterMap.hasOwnProperty(store)) { - return this.storeGetterMap[store].call(this, ttl); + if (this.storeFactories.hasOwnProperty(store)) { + return this.storeFactories[store].call(this, ttl); } - return this.storeGetterMap.none.call(this, ttl); + return this.storeFactories.none.call(this, ttl); } - private getMemcacheClient(ttl: number): cacheManager.Cache { + private getMemcacheClient(defaultTtl: number): cacheManager.Cache { const hosts = this.config.getStringArray('connection.hosts'); const netTimeout = this.config.getOptionalNumber('connection.netTimeout'); return cacheManager.caching({ @@ -93,21 +94,21 @@ export class CacheManager { hosts, ...(netTimeout && { netTimeout }), }, - ttl, + ttl: defaultTtl, }); } - private getMemoryClient(ttl: number): cacheManager.Cache { + private getMemoryClient(defaultTtl: number): cacheManager.Cache { return cacheManager.caching({ store: 'memory', - ttl, + ttl: defaultTtl, }); } - private getNoneClient(ttl: number): cacheManager.Cache { + private getNoneClient(defaultTtl: number): cacheManager.Cache { return cacheManager.caching({ store: 'none', - ttl, + ttl: defaultTtl, }); } } diff --git a/packages/backend-common/src/cache/index.ts b/packages/backend-common/src/cache/index.ts index c233621bd4..0cc8178b1a 100644 --- a/packages/backend-common/src/cache/index.ts +++ b/packages/backend-common/src/cache/index.ts @@ -15,5 +15,5 @@ */ export type { CacheClient } from './CacheClient'; -export * from './CacheManager'; -export * from './types'; +export { CacheManager } from './CacheManager'; +export type { PluginCacheManager } from './types'; diff --git a/packages/backend-common/src/cache/types.ts b/packages/backend-common/src/cache/types.ts index f1b9e96709..c93455e6b2 100644 --- a/packages/backend-common/src/cache/types.ts +++ b/packages/backend-common/src/cache/types.ts @@ -16,6 +16,10 @@ import { CacheClient } from './CacheClient'; +type ClientOptions = { + defaultTtl: number; +}; + /** * The PluginCacheManager manages access to cache stores that Plugins get. */ @@ -27,5 +31,5 @@ export type PluginCacheManager = { * stores so that plugins are discouraged from cache-level integration * and/or cache key collisions. */ - getClient: (ttl: number) => CacheClient; + getClient: (options: ClientOptions) => CacheClient; }; From cc6047512793026b842d678a9d57e81dd247b846 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Mon, 3 May 2021 17:09:20 +0200 Subject: [PATCH 09/19] Clarify default TTL behavior. Signed-off-by: Eric Peterson --- .../src/cache/CacheClient.test.ts | 31 +++++++++++++------ .../backend-common/src/cache/CacheClient.ts | 9 ++++-- .../backend-common/src/cache/CacheManager.ts | 1 + 3 files changed, 29 insertions(+), 12 deletions(-) diff --git a/packages/backend-common/src/cache/CacheClient.test.ts b/packages/backend-common/src/cache/CacheClient.test.ts index 5d56d84c61..2e14fdbea1 100644 --- a/packages/backend-common/src/cache/CacheClient.test.ts +++ b/packages/backend-common/src/cache/CacheClient.test.ts @@ -19,6 +19,7 @@ import cacheManager from 'cache-manager'; describe('CacheClient', () => { const pluginId = 'test'; + const defaultTtl = 60; let client: cacheManager.Cache; const b64 = (k: string) => Buffer.from(k).toString('base64'); @@ -33,7 +34,7 @@ describe('CacheClient', () => { describe('CacheClient.get', () => { it('calls client with normalized key', async () => { - const sut = new DefaultCacheClient({ client, pluginId }); + const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); const keyPartial = 'somekey'; await sut.get(keyPartial); @@ -42,7 +43,7 @@ describe('CacheClient', () => { }); it('calls client with normalized key (very long key)', async () => { - const sut = new DefaultCacheClient({ client, pluginId }); + const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); const keyPartial = 'x'.repeat(251); await sut.get(keyPartial); @@ -55,7 +56,7 @@ describe('CacheClient', () => { it('performs deserialization on returned data', async () => { const expectedValue = { some: 'value' }; - const sut = new DefaultCacheClient({ client, pluginId }); + const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); client.get = jest.fn().mockResolvedValue(JSON.stringify(expectedValue)); const actualValue = await sut.get('someKey'); @@ -64,7 +65,7 @@ describe('CacheClient', () => { }); it('returns null on any underlying error', async () => { - const sut = new DefaultCacheClient({ client, pluginId }); + const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); client.get = jest.fn().mockRejectedValue(undefined); const actualValue = await sut.get('someKey'); @@ -75,7 +76,7 @@ describe('CacheClient', () => { describe('CacheClient.set', () => { it('calls client with normalized key', async () => { - const sut = new DefaultCacheClient({ client, pluginId }); + const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); const keyPartial = 'somekey'; await sut.set(keyPartial, {}); @@ -86,7 +87,7 @@ describe('CacheClient', () => { }); it('performs serialization on given data', async () => { - const sut = new DefaultCacheClient({ client, pluginId }); + const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); const expectedData = { some: 'value' }; await sut.set('someKey', expectedData); @@ -97,7 +98,7 @@ describe('CacheClient', () => { }); it('passes ttl to client when given', async () => { - const sut = new DefaultCacheClient({ client, pluginId }); + const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); const expectedTtl = 3600; await sut.set('someKey', {}, { ttl: expectedTtl }); @@ -107,8 +108,18 @@ describe('CacheClient', () => { expect(actualOptions.ttl).toEqual(expectedTtl); }); + it('passes defaultTtl to client when not', async () => { + const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); + + await sut.set('someKey', {}); + + const spy = client.set as jest.Mock; + const actualOptions = spy.mock.calls[0][2]; + expect(actualOptions.ttl).toEqual(defaultTtl); + }); + it('does not throw errors on any client error', async () => { - const sut = new DefaultCacheClient({ client, pluginId }); + const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); client.set = jest.fn().mockRejectedValue(undefined); expect(async () => { @@ -119,7 +130,7 @@ describe('CacheClient', () => { describe('CacheClient.delete', () => { it('calls client with normalized key', async () => { - const sut = new DefaultCacheClient({ client, pluginId }); + const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); const keyPartial = 'somekey'; await sut.delete(keyPartial); @@ -130,7 +141,7 @@ describe('CacheClient', () => { }); it('does not throw errors on any client error', async () => { - const sut = new DefaultCacheClient({ client, pluginId }); + const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); client.del = jest.fn().mockRejectedValue(undefined); expect(async () => { diff --git a/packages/backend-common/src/cache/CacheClient.ts b/packages/backend-common/src/cache/CacheClient.ts index 68c762eb87..52703a28d4 100644 --- a/packages/backend-common/src/cache/CacheClient.ts +++ b/packages/backend-common/src/cache/CacheClient.ts @@ -21,6 +21,7 @@ import { createHash } from 'crypto'; type CacheClientArgs = { client: cacheManager.Cache; pluginId: string; + defaultTtl: number; }; type CacheSetOptions = { @@ -56,10 +57,12 @@ export interface CacheClient { */ export class DefaultCacheClient implements CacheClient { private readonly client: cacheManager.Cache; + private readonly defaultTtl: number; private readonly pluginId: string; - constructor({ client, pluginId }: CacheClientArgs) { + constructor({ client, defaultTtl, pluginId }: CacheClientArgs) { this.client = client; + this.defaultTtl = defaultTtl; this.pluginId = pluginId; } @@ -81,7 +84,9 @@ export class DefaultCacheClient implements CacheClient { const k = this.getNormalizedKey(key); try { const data = this.serializeData(value); - await this.client.set(k, data, opts.ttl ? { ttl: opts.ttl } : undefined); + await this.client.set(k, data, { + ttl: opts.ttl || this.defaultTtl, + }); } catch { return; } diff --git a/packages/backend-common/src/cache/CacheManager.ts b/packages/backend-common/src/cache/CacheManager.ts index 065a0b1db5..ecccbcf712 100644 --- a/packages/backend-common/src/cache/CacheManager.ts +++ b/packages/backend-common/src/cache/CacheManager.ts @@ -67,6 +67,7 @@ export class CacheManager { const concreteClient = this.getClientWithTtl(defaultTtl); return new DefaultCacheClient({ client: concreteClient, + defaultTtl, pluginId: pluginId, }); }, From b71c230beb1f5985eebe8deb78cfd88ce38e6134 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Mon, 3 May 2021 17:37:28 +0200 Subject: [PATCH 10/19] Refactor CacheManager.fromConfig Signed-off-by: Eric Peterson --- .../src/cache/CacheManager.test.ts | 16 ++++++++- .../backend-common/src/cache/CacheManager.ts | 35 +++++++++++-------- 2 files changed, 35 insertions(+), 16 deletions(-) diff --git a/packages/backend-common/src/cache/CacheManager.test.ts b/packages/backend-common/src/cache/CacheManager.test.ts index 625c772dec..08fa06e311 100644 --- a/packages/backend-common/src/cache/CacheManager.test.ts +++ b/packages/backend-common/src/cache/CacheManager.test.ts @@ -41,12 +41,17 @@ describe('CacheManager', () => { describe('CacheManager.fromConfig', () => { it('accesses the backend.cache key', () => { const getOptionalConfig = jest.fn(); + const getOptionalString = jest.fn(); const config = defaultConfig(); config.getOptionalConfig = getOptionalConfig; + config.getOptionalString = getOptionalString; CacheManager.fromConfig(config); - expect(getOptionalConfig.mock.calls[0][0]).toEqual('backend.cache'); + expect(getOptionalString.mock.calls[0][0]).toEqual('backend.cache.store'); + expect(getOptionalConfig.mock.calls[0][0]).toEqual( + 'backend.cache.connection', + ); }); it('does not require the backend.cache key', () => { @@ -55,6 +60,15 @@ describe('CacheManager', () => { CacheManager.fromConfig(config); }).not.toThrowError(); }); + + it('throws on unknown cache store', () => { + const config = new ConfigReader({ + backend: { cache: { store: 'notreal' } }, + }); + expect(() => { + CacheManager.fromConfig(config); + }).toThrowError(); + }); }); describe('CacheManager.forPlugin', () => { diff --git a/packages/backend-common/src/cache/CacheManager.ts b/packages/backend-common/src/cache/CacheManager.ts index ecccbcf712..1c2c003b39 100644 --- a/packages/backend-common/src/cache/CacheManager.ts +++ b/packages/backend-common/src/cache/CacheManager.ts @@ -30,7 +30,7 @@ import { PluginCacheManager } from './types'; */ export class CacheManager { /** - * Keys represented supported `backend.cache.store` values, mapped to + * Keys represents supported `backend.cache.store` values, mapped to * factories that return cacheManager.Cache instances appropriate to the * store. */ @@ -40,6 +40,9 @@ export class CacheManager { none: this.getNoneClient, }; + private readonly store: keyof CacheManager['storeFactories']; + private readonly connection: Config; + /** * Creates a new CacheManager instance by reading from the `backend` config * section, specifically the `.cache` key. @@ -49,12 +52,21 @@ export class CacheManager { static fromConfig(config: Config): CacheManager { // If no `backend.cache` config is provided, instantiate the CacheManager // with empty config; allowing a "none" cache client will be returned. - return new CacheManager( - config.getOptionalConfig('backend.cache') || new ConfigReader(undefined), - ); + const store = config.getOptionalString('backend.cache.store') || 'none'; + const connectionConfig = + config.getOptionalConfig('backend.cache.connection') || + new ConfigReader(undefined); + + return new CacheManager(store, connectionConfig); } - private constructor(private readonly config: Config) {} + private constructor(store: string, connectionConfig: Config) { + if (!this.storeFactories.hasOwnProperty(store)) { + throw new Error(`Unknown cache store: ${store}`); + } + this.store = store as keyof CacheManager['storeFactories']; + this.connection = connectionConfig; + } /** * Generates a CacheManagerInstance for consumption by plugins. @@ -75,19 +87,12 @@ export class CacheManager { } private getClientWithTtl(ttl: number): cacheManager.Cache { - const store = this.config.getOptionalString( - 'store', - ) as keyof CacheManager['storeFactories']; - - if (this.storeFactories.hasOwnProperty(store)) { - return this.storeFactories[store].call(this, ttl); - } - return this.storeFactories.none.call(this, ttl); + return this.storeFactories[this.store].call(this, ttl); } private getMemcacheClient(defaultTtl: number): cacheManager.Cache { - const hosts = this.config.getStringArray('connection.hosts'); - const netTimeout = this.config.getOptionalNumber('connection.netTimeout'); + const hosts = this.connection.getStringArray('hosts'); + const netTimeout = this.connection.getOptionalNumber('netTimeout'); return cacheManager.caching({ store: memcachedStore, driver: Memcache, From 368a5158cc5ebaea6c21b2a8c6428307713e04f3 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Mon, 3 May 2021 18:13:12 +0200 Subject: [PATCH 11/19] Return an explicit undefined on no data. Signed-off-by: Eric Peterson --- .../backend-common/src/cache/CacheClient.test.ts | 4 ++-- packages/backend-common/src/cache/CacheClient.ts | 13 +++++++------ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/packages/backend-common/src/cache/CacheClient.test.ts b/packages/backend-common/src/cache/CacheClient.test.ts index 2e14fdbea1..2ae7cee494 100644 --- a/packages/backend-common/src/cache/CacheClient.test.ts +++ b/packages/backend-common/src/cache/CacheClient.test.ts @@ -64,13 +64,13 @@ describe('CacheClient', () => { expect(actualValue).toMatchObject(expectedValue); }); - it('returns null on any underlying error', async () => { + it('returns undefined on any underlying error', async () => { const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); client.get = jest.fn().mockRejectedValue(undefined); const actualValue = await sut.get('someKey'); - expect(actualValue).toStrictEqual(null); + expect(actualValue).toStrictEqual(undefined); }); }); diff --git a/packages/backend-common/src/cache/CacheClient.ts b/packages/backend-common/src/cache/CacheClient.ts index 52703a28d4..523dae6fed 100644 --- a/packages/backend-common/src/cache/CacheClient.ts +++ b/packages/backend-common/src/cache/CacheClient.ts @@ -34,9 +34,10 @@ type CacheSetOptions = { */ export interface CacheClient { /** - * Reads data from a cache store for the given key. + * Reads data from a cache store for the given key. If no data was found, + * returns undefined. */ - get(key: string): Promise; + get(key: string): Promise; /** * Writes the given data to a cache store, associated with the given key. An @@ -66,13 +67,13 @@ export class DefaultCacheClient implements CacheClient { this.pluginId = pluginId; } - async get(key: string): Promise { + async get(key: string): Promise { const k = this.getNormalizedKey(key); try { const data = (await this.client.get(k)) as string | undefined; return this.deserializeData(data); } catch { - return null; + return undefined; } } @@ -122,7 +123,7 @@ export class DefaultCacheClient implements CacheClient { return JSON.stringify(data); } - private deserializeData(data: string | undefined): JsonValue { - return data ? JSON.parse(data) : null; + private deserializeData(data: string | undefined): JsonValue | undefined { + return data ? JSON.parse(data) : undefined; } } From 7ff6a755752725bd855d5ccdfcff5e8076a2f04a Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Mon, 3 May 2021 18:50:07 +0200 Subject: [PATCH 12/19] Add an onError config to enable plugin devs to catch and handle underlying client errors Signed-off-by: Eric Peterson --- .../src/cache/CacheClient.test.ts | 129 +++++++++++++++--- .../backend-common/src/cache/CacheClient.ts | 22 ++- .../backend-common/src/cache/CacheManager.ts | 3 +- packages/backend-common/src/cache/types.ts | 1 + 4 files changed, 129 insertions(+), 26 deletions(-) diff --git a/packages/backend-common/src/cache/CacheClient.test.ts b/packages/backend-common/src/cache/CacheClient.test.ts index 2ae7cee494..a08bb245f8 100644 --- a/packages/backend-common/src/cache/CacheClient.test.ts +++ b/packages/backend-common/src/cache/CacheClient.test.ts @@ -20,6 +20,7 @@ import cacheManager from 'cache-manager'; describe('CacheClient', () => { const pluginId = 'test'; const defaultTtl = 60; + const onError = 'returnEmpty'; let client: cacheManager.Cache; const b64 = (k: string) => Buffer.from(k).toString('base64'); @@ -34,7 +35,12 @@ describe('CacheClient', () => { describe('CacheClient.get', () => { it('calls client with normalized key', async () => { - const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); + const sut = new DefaultCacheClient({ + client, + defaultTtl, + pluginId, + onError, + }); const keyPartial = 'somekey'; await sut.get(keyPartial); @@ -43,7 +49,12 @@ describe('CacheClient', () => { }); it('calls client with normalized key (very long key)', async () => { - const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); + const sut = new DefaultCacheClient({ + client, + defaultTtl, + pluginId, + onError, + }); const keyPartial = 'x'.repeat(251); await sut.get(keyPartial); @@ -56,7 +67,12 @@ describe('CacheClient', () => { it('performs deserialization on returned data', async () => { const expectedValue = { some: 'value' }; - const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); + const sut = new DefaultCacheClient({ + client, + defaultTtl, + pluginId, + onError, + }); client.get = jest.fn().mockResolvedValue(JSON.stringify(expectedValue)); const actualValue = await sut.get('someKey'); @@ -65,18 +81,41 @@ describe('CacheClient', () => { }); it('returns undefined on any underlying error', async () => { - const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); + const sut = new DefaultCacheClient({ + client, + defaultTtl, + pluginId, + onError, + }); client.get = jest.fn().mockRejectedValue(undefined); const actualValue = await sut.get('someKey'); expect(actualValue).toStrictEqual(undefined); }); + + it('rejects on underlying error if configured', async () => { + const sut = new DefaultCacheClient({ + client, + defaultTtl, + pluginId, + onError: 'reject', + }); + const expectedError = new Error('Some runtime error'); + client.get = jest.fn().mockRejectedValue(expectedError); + + return expect(sut.get('someKey')).rejects.toEqual(expectedError); + }); }); describe('CacheClient.set', () => { it('calls client with normalized key', async () => { - const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); + const sut = new DefaultCacheClient({ + client, + defaultTtl, + pluginId, + onError, + }); const keyPartial = 'somekey'; await sut.set(keyPartial, {}); @@ -87,7 +126,12 @@ describe('CacheClient', () => { }); it('performs serialization on given data', async () => { - const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); + const sut = new DefaultCacheClient({ + client, + defaultTtl, + pluginId, + onError, + }); const expectedData = { some: 'value' }; await sut.set('someKey', expectedData); @@ -98,7 +142,12 @@ describe('CacheClient', () => { }); it('passes ttl to client when given', async () => { - const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); + const sut = new DefaultCacheClient({ + client, + defaultTtl, + pluginId, + onError, + }); const expectedTtl = 3600; await sut.set('someKey', {}, { ttl: expectedTtl }); @@ -109,7 +158,12 @@ describe('CacheClient', () => { }); it('passes defaultTtl to client when not', async () => { - const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); + const sut = new DefaultCacheClient({ + client, + defaultTtl, + pluginId, + onError, + }); await sut.set('someKey', {}); @@ -118,19 +172,40 @@ describe('CacheClient', () => { expect(actualOptions.ttl).toEqual(defaultTtl); }); - it('does not throw errors on any client error', async () => { - const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); + it('does not reject on any client error by default', async () => { + const sut = new DefaultCacheClient({ + client, + defaultTtl, + pluginId, + onError, + }); client.set = jest.fn().mockRejectedValue(undefined); - expect(async () => { - await sut.set('someKey', {}); - }).not.toThrow(); + return expect(sut.set('someKey', {})).resolves.toBeUndefined(); + }); + + it('rejects on underlying error if configured', async () => { + const sut = new DefaultCacheClient({ + client, + defaultTtl, + pluginId, + onError: 'reject', + }); + const expectedError = new Error('Some runtime error'); + client.set = jest.fn().mockRejectedValue(expectedError); + + return expect(sut.set('someKey', {})).rejects.toEqual(expectedError); }); }); describe('CacheClient.delete', () => { it('calls client with normalized key', async () => { - const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); + const sut = new DefaultCacheClient({ + client, + defaultTtl, + pluginId, + onError, + }); const keyPartial = 'somekey'; await sut.delete(keyPartial); @@ -140,13 +215,29 @@ describe('CacheClient', () => { expect(actualKey).toEqual(b64(`${pluginId}:${keyPartial}`)); }); - it('does not throw errors on any client error', async () => { - const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); + it('does not reject on any client error by default', async () => { + const sut = new DefaultCacheClient({ + client, + defaultTtl, + pluginId, + onError, + }); client.del = jest.fn().mockRejectedValue(undefined); - expect(async () => { - await sut.delete('someKey'); - }).not.toThrow(); + return expect(sut.delete('someKey')).resolves.toBeUndefined(); + }); + + it('rejects on underlying error if configured', async () => { + const sut = new DefaultCacheClient({ + client, + defaultTtl, + pluginId, + onError: 'reject', + }); + const expectedError = new Error('Some runtime error'); + client.del = jest.fn().mockRejectedValue(expectedError); + + return expect(sut.delete('someKey')).rejects.toEqual(expectedError); }); }); }); diff --git a/packages/backend-common/src/cache/CacheClient.ts b/packages/backend-common/src/cache/CacheClient.ts index 523dae6fed..e1f849ccb7 100644 --- a/packages/backend-common/src/cache/CacheClient.ts +++ b/packages/backend-common/src/cache/CacheClient.ts @@ -22,6 +22,7 @@ type CacheClientArgs = { client: cacheManager.Cache; pluginId: string; defaultTtl: number; + onError: 'reject' | 'returnEmpty'; }; type CacheSetOptions = { @@ -60,11 +61,13 @@ export class DefaultCacheClient implements CacheClient { private readonly client: cacheManager.Cache; private readonly defaultTtl: number; private readonly pluginId: string; + private readonly onError: 'reject' | 'returnEmpty'; - constructor({ client, defaultTtl, pluginId }: CacheClientArgs) { + constructor({ client, defaultTtl, pluginId, onError }: CacheClientArgs) { this.client = client; this.defaultTtl = defaultTtl; this.pluginId = pluginId; + this.onError = onError; } async get(key: string): Promise { @@ -72,7 +75,10 @@ export class DefaultCacheClient implements CacheClient { try { const data = (await this.client.get(k)) as string | undefined; return this.deserializeData(data); - } catch { + } catch (e) { + if (this.onError === 'reject') { + throw e; + } return undefined; } } @@ -88,8 +94,10 @@ export class DefaultCacheClient implements CacheClient { await this.client.set(k, data, { ttl: opts.ttl || this.defaultTtl, }); - } catch { - return; + } catch (e) { + if (this.onError === 'reject') { + throw e; + } } } @@ -97,8 +105,10 @@ export class DefaultCacheClient implements CacheClient { const k = this.getNormalizedKey(key); try { await this.client.del(k); - } catch { - return; + } catch (e) { + if (this.onError === 'reject') { + throw e; + } } } diff --git a/packages/backend-common/src/cache/CacheManager.ts b/packages/backend-common/src/cache/CacheManager.ts index 1c2c003b39..13fb75e314 100644 --- a/packages/backend-common/src/cache/CacheManager.ts +++ b/packages/backend-common/src/cache/CacheManager.ts @@ -75,12 +75,13 @@ export class CacheManager { */ forPlugin(pluginId: string): PluginCacheManager { return { - getClient: ({ defaultTtl }): CacheClient => { + getClient: ({ defaultTtl, onError }): CacheClient => { const concreteClient = this.getClientWithTtl(defaultTtl); return new DefaultCacheClient({ client: concreteClient, defaultTtl, pluginId: pluginId, + onError: onError || 'returnEmpty', }); }, }; diff --git a/packages/backend-common/src/cache/types.ts b/packages/backend-common/src/cache/types.ts index c93455e6b2..5cc5782b18 100644 --- a/packages/backend-common/src/cache/types.ts +++ b/packages/backend-common/src/cache/types.ts @@ -18,6 +18,7 @@ import { CacheClient } from './CacheClient'; type ClientOptions = { defaultTtl: number; + onError?: 'reject' | 'returnEmpty'; }; /** From 884c2bd54f08b0d9e72eaee4fb81294ab2a92201 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Mon, 3 May 2021 18:54:28 +0200 Subject: [PATCH 13/19] Update docs to reflect changes Signed-off-by: Eric Peterson --- .changeset/cool-cache-bro.md | 17 ++++++++++++++++- packages/backend-common/api-report.md | 6 +++--- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/.changeset/cool-cache-bro.md b/.changeset/cool-cache-bro.md index f86dc39339..f342f39e55 100644 --- a/.changeset/cool-cache-bro.md +++ b/.changeset/cool-cache-bro.md @@ -28,7 +28,7 @@ import { CacheManager } from '@backstage/backend-common'; const cacheManager = CacheManager.fromConfig(config); const somePluginCache = cacheManager.forPlugin('somePlugin'); const defaultTtl = 3600; -const cacheClient = somePluginCache.getClient(defaultTtl); +const cacheClient = somePluginCache.getClient({ defaultTtl }); // Using the cache client: const cachedValue = await cacheClient.get('someKey'); @@ -41,4 +41,19 @@ if (cachedValue) { await cacheClient.delete('someKey'); ``` +For simplicity of use, cache clients will swallow client errors by default. Plugin developers may optionally pass an `onError` argument to the `CacheManager.getClient()` method if they wish to capture and handle those errors in a custom way. + +```typescript +const cacheClient = somePluginCache.getClient({ + defaultTtl: 3600, + onError: 'reject', +}); + +try { + await cacheClient.delete('someKey'); +} catch (e) { + // Attempt again, log, alert, etc. +} +``` + Configuring a cache store is optional. Even when no cache store is configured, the cache manager will dutifully pass plugins a manager that resolves a cache client that does not actually write or read any data. diff --git a/packages/backend-common/api-report.md b/packages/backend-common/api-report.md index 339a291ff3..bd1b017574 100644 --- a/packages/backend-common/api-report.md +++ b/packages/backend-common/api-report.md @@ -66,8 +66,8 @@ export class BitbucketUrlReader implements UrlReader { // @public export interface CacheClient { delete(key: string): Promise; - get(key: string): Promise; - set(key: string, value: JsonValue, ttl?: number): Promise; + get(key: string): Promise; + set(key: string, value: JsonValue, options: CacheSetOptions): Promise; } // @public @@ -254,7 +254,7 @@ export function notFoundHandler(): RequestHandler; // @public export type PluginCacheManager = { - getClient: (ttl: number) => CacheClient; + getClient: (options: ClientOptions) => CacheClient; }; // @public From d6936dcbde0d4f0b7e542ab15a0be90d31a7d443 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Sun, 9 May 2021 17:00:59 +0200 Subject: [PATCH 14/19] Swap out cache-manager for Keyv implementation. Signed-off-by: Eric Peterson --- packages/backend-common/config.d.ts | 18 +- packages/backend-common/package.json | 7 +- .../src/cache/CacheClient.test.ts | 215 +++++------------- .../backend-common/src/cache/CacheClient.ts | 78 ++----- .../src/cache/CacheManager.test.ts | 66 +++--- .../backend-common/src/cache/CacheManager.ts | 79 +++---- packages/backend-common/src/cache/types.ts | 10 +- yarn.lock | 109 ++++----- 8 files changed, 192 insertions(+), 390 deletions(-) diff --git a/packages/backend-common/config.d.ts b/packages/backend-common/config.d.ts index b4cf8580d9..875660a594 100644 --- a/packages/backend-common/config.d.ts +++ b/packages/backend-common/config.d.ts @@ -75,19 +75,11 @@ export interface Config { } | { store: 'memcache'; - connection: { - /** - * An array of memcache hosts, optionally including a port number. - * e.g. 127.0.0.1:11211 - */ - hosts: string[]; - - /** - * Number of milliseconds to wait before assuming there is a - * network timeout. Defaults to 500ms. - */ - netTimeout?: number; - }; + /** + * A memcache connection string in the form `user:pass@host:port`. + * @secret + */ + connection: string; }; cors?: { diff --git a/packages/backend-common/package.json b/packages/backend-common/package.json index 0f331ccc6b..9bd7de1227 100644 --- a/packages/backend-common/package.json +++ b/packages/backend-common/package.json @@ -36,13 +36,10 @@ "@backstage/integration": "^0.5.2", "@google-cloud/storage": "^5.8.0", "@octokit/rest": "^18.5.3", - "@types/cache-manager": "^3.4.0", "@types/cors": "^2.8.6", "@types/dockerode": "^3.2.1", "@types/express": "^4.17.6", "archiver": "^5.0.2", - "cache-manager": "^3.4.3", - "cache-manager-memcached-store": "^4.0.0", "compression": "^1.7.4", "concat-stream": "^2.0.0", "cors": "^2.8.5", @@ -54,10 +51,11 @@ "git-url-parse": "^11.4.4", "helmet": "^4.0.0", "isomorphic-git": "^1.8.0", + "keyv": "^4.0.3", + "keyv-memcache": "^1.2.5", "knex": "^0.95.1", "lodash": "^4.17.15", "logform": "^2.1.1", - "memcache-pp": "^0.3.3", "minimatch": "^3.0.4", "minimist": "^1.2.5", "morgan": "^1.10.0", @@ -80,7 +78,6 @@ "@backstage/cli": "^0.6.10", "@backstage/test-utils": "^0.1.10", "@types/archiver": "^5.1.0", - "@types/cache-manager": "^3.4.0", "@types/compression": "^1.7.0", "@types/concat-stream": "^1.6.0", "@types/fs-extra": "^9.0.3", diff --git a/packages/backend-common/src/cache/CacheClient.test.ts b/packages/backend-common/src/cache/CacheClient.test.ts index a08bb245f8..eb32d4a28f 100644 --- a/packages/backend-common/src/cache/CacheClient.test.ts +++ b/packages/backend-common/src/cache/CacheClient.test.ts @@ -15,229 +15,132 @@ */ import { DefaultCacheClient } from './CacheClient'; -import cacheManager from 'cache-manager'; +import Keyv from 'keyv'; describe('CacheClient', () => { - const pluginId = 'test'; - const defaultTtl = 60; - const onError = 'returnEmpty'; - let client: cacheManager.Cache; + let client: Keyv; const b64 = (k: string) => Buffer.from(k).toString('base64'); beforeEach(() => { - client = cacheManager.caching({ store: 'none', ttl: 0 }); + client = new Keyv(); client.get = jest.fn(); client.set = jest.fn(); - client.del = jest.fn(); + client.delete = jest.fn(); }); afterEach(() => jest.resetAllMocks()); describe('CacheClient.get', () => { it('calls client with normalized key', async () => { - const sut = new DefaultCacheClient({ - client, - defaultTtl, - pluginId, - onError, - }); - const keyPartial = 'somekey'; + const sut = new DefaultCacheClient({ client }); + const key = 'somekey'; - await sut.get(keyPartial); + await sut.get(key); - expect(client.get).toHaveBeenCalledWith(b64(`${pluginId}:${keyPartial}`)); + expect(client.get).toHaveBeenCalledWith(b64(key)); }); it('calls client with normalized key (very long key)', async () => { - const sut = new DefaultCacheClient({ - client, - defaultTtl, - pluginId, - onError, - }); - const keyPartial = 'x'.repeat(251); + const sut = new DefaultCacheClient({ client }); + const key = 'x'.repeat(251); - await sut.get(keyPartial); + await sut.get(key); const spy = client.get as jest.Mock; const actualKey = spy.mock.calls[0][0]; - expect(actualKey).not.toEqual(b64(`${pluginId}:${keyPartial}`)); + expect(actualKey).not.toEqual(b64(key)); expect(actualKey.length).toBeLessThan(250); }); - it('performs deserialization on returned data', async () => { - const expectedValue = { some: 'value' }; - const sut = new DefaultCacheClient({ - client, - defaultTtl, - pluginId, - onError, - }); - client.get = jest.fn().mockResolvedValue(JSON.stringify(expectedValue)); - - const actualValue = await sut.get('someKey'); - - expect(actualValue).toMatchObject(expectedValue); - }); - - it('returns undefined on any underlying error', async () => { - const sut = new DefaultCacheClient({ - client, - defaultTtl, - pluginId, - onError, - }); - client.get = jest.fn().mockRejectedValue(undefined); - - const actualValue = await sut.get('someKey'); - - expect(actualValue).toStrictEqual(undefined); - }); - - it('rejects on underlying error if configured', async () => { - const sut = new DefaultCacheClient({ - client, - defaultTtl, - pluginId, - onError: 'reject', - }); + it('rejects on underlying error', async () => { + const sut = new DefaultCacheClient({ client }); const expectedError = new Error('Some runtime error'); client.get = jest.fn().mockRejectedValue(expectedError); return expect(sut.get('someKey')).rejects.toEqual(expectedError); }); + + it('resolves what underlying client resolves', async () => { + const sut = new DefaultCacheClient({ client }); + const expectedValue = 'some value'; + client.get = jest.fn().mockResolvedValue(expectedValue); + + const actualValue = await sut.get('someKey'); + + return expect(actualValue).toEqual(expectedValue); + }); }); describe('CacheClient.set', () => { it('calls client with normalized key', async () => { - const sut = new DefaultCacheClient({ - client, - defaultTtl, - pluginId, - onError, - }); - const keyPartial = 'somekey'; + const sut = new DefaultCacheClient({ client }); + const key = 'somekey'; - await sut.set(keyPartial, {}); + await sut.set(key, {}); const spy = client.set as jest.Mock; const actualKey = spy.mock.calls[0][0]; - expect(actualKey).toEqual(b64(`${pluginId}:${keyPartial}`)); - }); - - it('performs serialization on given data', async () => { - const sut = new DefaultCacheClient({ - client, - defaultTtl, - pluginId, - onError, - }); - const expectedData = { some: 'value' }; - - await sut.set('someKey', expectedData); - - const spy = client.set as jest.Mock; - const actualData = spy.mock.calls[0][1]; - expect(actualData).toEqual(JSON.stringify(expectedData)); + expect(actualKey).toEqual(b64(key)); }); it('passes ttl to client when given', async () => { - const sut = new DefaultCacheClient({ - client, - defaultTtl, - pluginId, - onError, - }); + const sut = new DefaultCacheClient({ client }); const expectedTtl = 3600; await sut.set('someKey', {}, { ttl: expectedTtl }); const spy = client.set as jest.Mock; - const actualOptions = spy.mock.calls[0][2]; - expect(actualOptions.ttl).toEqual(expectedTtl); - }); - - it('passes defaultTtl to client when not', async () => { - const sut = new DefaultCacheClient({ - client, - defaultTtl, - pluginId, - onError, - }); - - await sut.set('someKey', {}); - - const spy = client.set as jest.Mock; - const actualOptions = spy.mock.calls[0][2]; - expect(actualOptions.ttl).toEqual(defaultTtl); - }); - - it('does not reject on any client error by default', async () => { - const sut = new DefaultCacheClient({ - client, - defaultTtl, - pluginId, - onError, - }); - client.set = jest.fn().mockRejectedValue(undefined); - - return expect(sut.set('someKey', {})).resolves.toBeUndefined(); + const actualTtl = spy.mock.calls[0][2]; + expect(actualTtl).toEqual(expectedTtl); }); it('rejects on underlying error if configured', async () => { - const sut = new DefaultCacheClient({ - client, - defaultTtl, - pluginId, - onError: 'reject', - }); + const sut = new DefaultCacheClient({ client }); const expectedError = new Error('Some runtime error'); client.set = jest.fn().mockRejectedValue(expectedError); return expect(sut.set('someKey', {})).rejects.toEqual(expectedError); }); + + it('resolves what underlying client resolves', async () => { + const sut = new DefaultCacheClient({ client }); + const expectedResponse = true; + client.set = jest.fn().mockReturnValue(expectedResponse); + + const actualResponse = await sut.set('someKey', {}); + + return expect(actualResponse).toEqual(expectedResponse); + }); }); describe('CacheClient.delete', () => { it('calls client with normalized key', async () => { - const sut = new DefaultCacheClient({ - client, - defaultTtl, - pluginId, - onError, - }); - const keyPartial = 'somekey'; + const sut = new DefaultCacheClient({ client }); + const key = 'somekey'; - await sut.delete(keyPartial); + await sut.delete(key); - const spy = client.del as jest.Mock; + const spy = client.delete as jest.Mock; const actualKey = spy.mock.calls[0][0]; - expect(actualKey).toEqual(b64(`${pluginId}:${keyPartial}`)); - }); - - it('does not reject on any client error by default', async () => { - const sut = new DefaultCacheClient({ - client, - defaultTtl, - pluginId, - onError, - }); - client.del = jest.fn().mockRejectedValue(undefined); - - return expect(sut.delete('someKey')).resolves.toBeUndefined(); + expect(actualKey).toEqual(b64(key)); }); it('rejects on underlying error if configured', async () => { - const sut = new DefaultCacheClient({ - client, - defaultTtl, - pluginId, - onError: 'reject', - }); + const sut = new DefaultCacheClient({ client }); const expectedError = new Error('Some runtime error'); - client.del = jest.fn().mockRejectedValue(expectedError); + client.delete = jest.fn().mockRejectedValue(expectedError); return expect(sut.delete('someKey')).rejects.toEqual(expectedError); }); + + it('resolves what underlying client resolves', async () => { + const sut = new DefaultCacheClient({ client }); + const expectedResponse = false; + client.delete = jest.fn().mockResolvedValue(expectedResponse); + + const actualResponse = await sut.delete('someKey'); + + return expect(actualResponse).toEqual(expectedResponse); + }); }); }); diff --git a/packages/backend-common/src/cache/CacheClient.ts b/packages/backend-common/src/cache/CacheClient.ts index e1f849ccb7..943de055ae 100644 --- a/packages/backend-common/src/cache/CacheClient.ts +++ b/packages/backend-common/src/cache/CacheClient.ts @@ -15,17 +15,18 @@ */ import { JsonValue } from '@backstage/config'; -import cacheManager from 'cache-manager'; import { createHash } from 'crypto'; +import Keyv from 'keyv'; type CacheClientArgs = { - client: cacheManager.Cache; - pluginId: string; - defaultTtl: number; - onError: 'reject' | 'returnEmpty'; + client: Keyv; }; type CacheSetOptions = { + /** + * Optional TTL in milliseconds. Defaults to the TTL provided when the client + * was set up (or no TTL if none are provided). + */ ttl?: number; }; @@ -45,80 +46,50 @@ export interface CacheClient { * optional TTL may also be provided, otherwise it defaults to the TTL that * was provided when the client was instantiated. */ - set(key: string, value: JsonValue, options: CacheSetOptions): Promise; + set(key: string, value: JsonValue, options?: CacheSetOptions): Promise; /** - * Removes the given key from the cache store. + * Removes the given key from the cache store. Resolves true if the key + * existed, or false if not. */ - delete(key: string): Promise; + delete(key: string): Promise; } /** - * A simple, concrete implementation of the CacheClient, suitable for almost + * A basic, concrete implementation of the CacheClient, suitable for almost * all uses in Backstage. */ export class DefaultCacheClient implements CacheClient { - private readonly client: cacheManager.Cache; - private readonly defaultTtl: number; - private readonly pluginId: string; - private readonly onError: 'reject' | 'returnEmpty'; + private readonly client: Keyv; - constructor({ client, defaultTtl, pluginId, onError }: CacheClientArgs) { + constructor({ client }: CacheClientArgs) { this.client = client; - this.defaultTtl = defaultTtl; - this.pluginId = pluginId; - this.onError = onError; } async get(key: string): Promise { const k = this.getNormalizedKey(key); - try { - const data = (await this.client.get(k)) as string | undefined; - return this.deserializeData(data); - } catch (e) { - if (this.onError === 'reject') { - throw e; - } - return undefined; - } + return await this.client.get(k); } async set( key: string, value: JsonValue, opts: CacheSetOptions = {}, - ): Promise { + ): Promise { const k = this.getNormalizedKey(key); - try { - const data = this.serializeData(value); - await this.client.set(k, data, { - ttl: opts.ttl || this.defaultTtl, - }); - } catch (e) { - if (this.onError === 'reject') { - throw e; - } - } + return await this.client.set(k, value, opts.ttl) } - async delete(key: string): Promise { + async delete(key: string): Promise { const k = this.getNormalizedKey(key); - try { - await this.client.del(k); - } catch (e) { - if (this.onError === 'reject') { - throw e; - } - } + return await this.client.delete(k); } /** - * Namespaces key by plugin to discourage cross-plugin integration via the - * cache store. + * Ensures keys are well-formed for any/all cache stores. */ - private getNormalizedKey(key: string): string { - // Namespace key by plugin ID and remove potentially invalid characters. - const candidateKey = `${this.pluginId}:${key}`; + private getNormalizedKey(candidateKey: string): string { + // Remove potentially invalid characters. const wellFormedKey = Buffer.from(candidateKey).toString('base64'); // Memcache in particular doesn't do well with keys > 250 bytes. @@ -129,11 +100,4 @@ export class DefaultCacheClient implements CacheClient { return createHash('md5').update(candidateKey).digest('base64'); } - private serializeData(data: JsonValue): string { - return JSON.stringify(data); - } - - private deserializeData(data: string | undefined): JsonValue | undefined { - return data ? JSON.parse(data) : undefined; - } } diff --git a/packages/backend-common/src/cache/CacheManager.test.ts b/packages/backend-common/src/cache/CacheManager.test.ts index 08fa06e311..558964b55c 100644 --- a/packages/backend-common/src/cache/CacheManager.test.ts +++ b/packages/backend-common/src/cache/CacheManager.test.ts @@ -15,11 +15,16 @@ */ import { ConfigReader } from '@backstage/config'; -import cacheManager from 'cache-manager'; +import Keyv from 'keyv'; +/* @ts-expect-error */ +import KeyvMemcache from 'keyv-memcache'; import { DefaultCacheClient } from './CacheClient'; import { CacheManager } from './CacheManager'; -cacheManager.caching = jest.fn() as jest.Mock; +jest.createMockFromModule('keyv'); +jest.mock('keyv'); +jest.createMockFromModule('keyv-memcache'); +jest.mock('keyv-memcache'); jest.mock('./CacheClient', () => { return { DefaultCacheClient: jest.fn(), @@ -40,18 +45,14 @@ describe('CacheManager', () => { describe('CacheManager.fromConfig', () => { it('accesses the backend.cache key', () => { - const getOptionalConfig = jest.fn(); const getOptionalString = jest.fn(); const config = defaultConfig(); - config.getOptionalConfig = getOptionalConfig; config.getOptionalString = getOptionalString; CacheManager.fromConfig(config); expect(getOptionalString.mock.calls[0][0]).toEqual('backend.cache.store'); - expect(getOptionalConfig.mock.calls[0][0]).toEqual( - 'backend.cache.connection', - ); + expect(getOptionalString.mock.calls[1][0]).toEqual('backend.cache.connection'); }); it('does not require the backend.cache key', () => { @@ -76,8 +77,7 @@ describe('CacheManager', () => { it('connects to a cache store scoped to the plugin', async () => { const pluginId = 'test1'; - const expectedTtl = 3600; - manager.forPlugin(pluginId).getClient({ defaultTtl: expectedTtl }); + manager.forPlugin(pluginId).getClient(); const client = DefaultCacheClient as jest.Mock; expect(client).toHaveBeenCalledTimes(1); @@ -90,62 +90,60 @@ describe('CacheManager', () => { manager.forPlugin(plugin1Id).getClient({ defaultTtl: expectedTtl }); manager.forPlugin(plugin2Id).getClient({ defaultTtl: expectedTtl }); - const cache = cacheManager.caching as jest.Mock; const client = DefaultCacheClient as jest.Mock; + const cache = Keyv as unknown as jest.Mock; expect(cache).toHaveBeenCalledTimes(2); expect(client).toHaveBeenCalledTimes(2); - const plugin1CallArgs = client.mock.calls[0]; - const plugin2CallArgs = client.mock.calls[1]; - expect(plugin1CallArgs[0].pluginId).not.toEqual( - plugin2CallArgs[0].pluginId, + const plugin1CallArgs = cache.mock.calls[0]; + const plugin2CallArgs = cache.mock.calls[1]; + expect(plugin1CallArgs[0].namespace).not.toEqual( + plugin2CallArgs[0].namespace, ); }); }); describe('CacheManager.forPlugin stores', () => { - it('returns none client when no cache is configured', () => { + it('returns memory client when no cache is configured', () => { const manager = CacheManager.fromConfig( new ConfigReader({ backend: {} }), ); const expectedTtl = 3600; - manager.forPlugin('test').getClient({ defaultTtl: expectedTtl }); + const expectedNamespace = 'test-plugin'; + manager.forPlugin(expectedNamespace).getClient({ defaultTtl: expectedTtl }); - const cache = cacheManager.caching as jest.Mock; + const cache = Keyv as unknown as jest.Mock; const mockCalls = cache.mock.calls.splice(-1); const callArgs = mockCalls[0]; expect(callArgs[0]).toMatchObject({ - store: 'none', ttl: expectedTtl, + namespace: expectedNamespace, }); }); - it('returns memory client when configured', () => { + it('returns memory client when explicitly configured', () => { const manager = CacheManager.fromConfig(defaultConfig()); const expectedTtl = 3600; - manager.forPlugin('test').getClient({ defaultTtl: expectedTtl }); + const expectedNamespace = 'test-plugin'; + manager.forPlugin(expectedNamespace).getClient({ defaultTtl: expectedTtl }); - const cache = cacheManager.caching as jest.Mock; + const cache = Keyv as unknown as jest.Mock; const mockCalls = cache.mock.calls.splice(-1); const callArgs = mockCalls[0]; expect(callArgs[0]).toMatchObject({ - store: 'memory', ttl: expectedTtl, + namespace: expectedNamespace, }); }); it('returns a memcache client when configured', () => { const expectedHost = '127.0.0.1:11211'; - const expectedTimeout = 1000; const manager = CacheManager.fromConfig( new ConfigReader({ backend: { cache: { store: 'memcache', - connection: { - hosts: [expectedHost], - netTimeout: expectedTimeout, - }, + connection: expectedHost, }, }, }), @@ -153,16 +151,14 @@ describe('CacheManager', () => { const expectedTtl = 3600; manager.forPlugin('test').getClient({ defaultTtl: expectedTtl }); - const cache = cacheManager.caching as jest.Mock; - const mockCalls = cache.mock.calls.splice(-1); - const callArgs = mockCalls[0]; - expect(callArgs[0]).toMatchObject({ - options: { - hosts: [expectedHost], - netTimeout: expectedTimeout, - }, + const cache = Keyv as unknown as jest.Mock; + const mockCacheCalls = cache.mock.calls.splice(-1); + expect(mockCacheCalls[0][0]).toMatchObject({ ttl: expectedTtl, }); + const memcache = KeyvMemcache as jest.Mock; + const mockMemcacheCalls = memcache.mock.calls.splice(-1); + expect(mockMemcacheCalls[0][0]).toEqual(expectedHost); }); }); }); diff --git a/packages/backend-common/src/cache/CacheManager.ts b/packages/backend-common/src/cache/CacheManager.ts index 13fb75e314..00c4cebf18 100644 --- a/packages/backend-common/src/cache/CacheManager.ts +++ b/packages/backend-common/src/cache/CacheManager.ts @@ -14,34 +14,30 @@ * limitations under the License. */ -import { Config, ConfigReader } from '@backstage/config'; -import cacheManager from 'cache-manager'; +import { Config } from '@backstage/config'; +import Keyv from 'keyv'; // @ts-expect-error -import Memcache from 'memcache-pp'; -// @ts-expect-error -import memcachedStore from 'cache-manager-memcached-store'; +import KeyvMemcache from 'keyv-memcache'; import { DefaultCacheClient, CacheClient } from './CacheClient'; import { PluginCacheManager } from './types'; /** * Implements a Cache Manager which will automatically create new cache clients * for plugins when requested. All requested cacheclients are created with the - * credentials provided. + * connection details provided. */ export class CacheManager { /** - * Keys represents supported `backend.cache.store` values, mapped to - * factories that return cacheManager.Cache instances appropriate to the - * store. + * Keys represent supported `backend.cache.store` values, mapped to factories + * that return Keyv instances appropriate to the store. */ private readonly storeFactories = { memcache: this.getMemcacheClient, memory: this.getMemoryClient, - none: this.getNoneClient, }; private readonly store: keyof CacheManager['storeFactories']; - private readonly connection: Config; + private readonly connection: string; /** * Creates a new CacheManager instance by reading from the `backend` config @@ -51,21 +47,18 @@ export class CacheManager { */ static fromConfig(config: Config): CacheManager { // If no `backend.cache` config is provided, instantiate the CacheManager - // with empty config; allowing a "none" cache client will be returned. - const store = config.getOptionalString('backend.cache.store') || 'none'; - const connectionConfig = - config.getOptionalConfig('backend.cache.connection') || - new ConfigReader(undefined); - - return new CacheManager(store, connectionConfig); + // with a "memory" cache client. + const store = config.getOptionalString('backend.cache.store') || 'memory'; + const connectionString = config.getOptionalString('backend.cache.connection') || ''; + return new CacheManager(store, connectionString); } - private constructor(store: string, connectionConfig: Config) { + private constructor(store: string, connectionString: string) { if (!this.storeFactories.hasOwnProperty(store)) { throw new Error(`Unknown cache store: ${store}`); } this.store = store as keyof CacheManager['storeFactories']; - this.connection = connectionConfig; + this.connection = connectionString; } /** @@ -75,47 +68,33 @@ export class CacheManager { */ forPlugin(pluginId: string): PluginCacheManager { return { - getClient: ({ defaultTtl, onError }): CacheClient => { - const concreteClient = this.getClientWithTtl(defaultTtl); + getClient: (opts = {}): CacheClient => { + const concreteClient = this.getClientWithTtl(pluginId, opts.defaultTtl); return new DefaultCacheClient({ client: concreteClient, - defaultTtl, - pluginId: pluginId, - onError: onError || 'returnEmpty', }); }, }; } - private getClientWithTtl(ttl: number): cacheManager.Cache { - return this.storeFactories[this.store].call(this, ttl); + private getClientWithTtl(pluginId: string, ttl: number | undefined): Keyv { + return this.storeFactories[this.store].call(this, pluginId, ttl); } - private getMemcacheClient(defaultTtl: number): cacheManager.Cache { - const hosts = this.connection.getStringArray('hosts'); - const netTimeout = this.connection.getOptionalNumber('netTimeout'); - return cacheManager.caching({ - store: memcachedStore, - driver: Memcache, - options: { - hosts, - ...(netTimeout && { netTimeout }), - }, + private getMemcacheClient(pluginId: string, defaultTtl: number | undefined): Keyv { + const memcache = new KeyvMemcache(this.connection); + return new Keyv({ + namespace: pluginId, + ttl: defaultTtl, + store: memcache, + }); + } + + private getMemoryClient(pluginId: string, defaultTtl: number | undefined): Keyv { + return new Keyv({ + namespace: pluginId, ttl: defaultTtl, }); } - private getMemoryClient(defaultTtl: number): cacheManager.Cache { - return cacheManager.caching({ - store: 'memory', - ttl: defaultTtl, - }); - } - - private getNoneClient(defaultTtl: number): cacheManager.Cache { - return cacheManager.caching({ - store: 'none', - ttl: defaultTtl, - }); - } } diff --git a/packages/backend-common/src/cache/types.ts b/packages/backend-common/src/cache/types.ts index 5cc5782b18..4dd3cf7d1b 100644 --- a/packages/backend-common/src/cache/types.ts +++ b/packages/backend-common/src/cache/types.ts @@ -17,8 +17,12 @@ import { CacheClient } from './CacheClient'; type ClientOptions = { - defaultTtl: number; - onError?: 'reject' | 'returnEmpty'; + /** + * 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; }; /** @@ -32,5 +36,5 @@ export type PluginCacheManager = { * stores so that plugins are discouraged from cache-level integration * and/or cache key collisions. */ - getClient: (options: ClientOptions) => CacheClient; + getClient: (options?: ClientOptions) => CacheClient; }; diff --git a/yarn.lock b/yarn.lock index 4b1ebed024..e792f84bfb 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5737,11 +5737,6 @@ resolved "https://registry.npmjs.org/@types/btoa-lite/-/btoa-lite-1.0.0.tgz#e190a5a548e0b348adb0df9ac7fa5f1151c7cca4" integrity sha512-wJsiX1tosQ+J5+bY5LrSahHxr2wT+uME5UDwdN1kg4frt40euqA+wzECkmq4t5QbveHiJepfdThgQrPw6KiSlg== -"@types/cache-manager@^3.4.0": - version "3.4.0" - resolved "https://registry.npmjs.org/@types/cache-manager/-/cache-manager-3.4.0.tgz#414136ea3807a8cd071b8f20370c5df5dbffd382" - integrity sha512-XVbn2HS+O+Mk2SKRCjr01/8oD5p2Tv1fxxdBqJ0+Cl+UBNiz0WVY5rusHpMGx+qF6Vc2pnRwPVwSKbGaDApCpw== - "@types/cacheable-request@^6.0.1": version "6.0.1" resolved "https://registry.npmjs.org/@types/cacheable-request/-/cacheable-request-6.0.1.tgz#5d22f3dded1fd3a84c0bbeb5039a7419c2c91976" @@ -8157,11 +8152,6 @@ async@0.9.x: resolved "https://registry.npmjs.org/async/-/async-0.9.2.tgz#aea74d5e61c1f899613bf64bda66d4c78f2fd17d" integrity sha1-rqdNXmHB+JlhO/ZL2mbUx48v0X0= -async@3.2.0, async@^3.2.0: - version "3.2.0" - resolved "https://registry.npmjs.org/async/-/async-3.2.0.tgz#b3a2685c5ebb641d3de02d161002c60fc9f85720" - integrity sha512-TR2mEZFVOj2pLStYxLht7TyfuRzaydfpxr3k9RpHIzMgw7A64dzsdqCxH1WJyQdoe8T10nDXd9wnEigmiuHIZw== - async@^2.0.1, async@^2.6.1, async@^2.6.2: version "2.6.3" resolved "https://registry.npmjs.org/async/-/async-2.6.3.tgz#d72625e2344a3656e3a3ad4fa749fa83299d82ff" @@ -8836,7 +8826,7 @@ block-stream@*: dependencies: inherits "~2.0.0" -bluebird@3.7.2, bluebird@^3.3.5, bluebird@^3.4.1, bluebird@^3.5.1, bluebird@^3.5.5, bluebird@^3.7.2: +bluebird@3.7.2, bluebird@^3.3.5, bluebird@^3.5.1, bluebird@^3.5.5, bluebird@^3.7.2: version "3.7.2" resolved "https://registry.npmjs.org/bluebird/-/bluebird-3.7.2.tgz#9f229c15be272454ffa973ace0dbee79a1b0c36f" integrity sha512-XpNj6GDQzdfW+r2Wnn7xiSAd7TM3jzkxGXBGTtWKuSXv1xUV+azxAm8jdWZN06QTQk+2N2XB9jRDkvbmQmcRtg== @@ -9261,20 +9251,6 @@ cache-base@^1.0.1: union-value "^1.0.0" unset-value "^1.0.0" -cache-manager-memcached-store@^4.0.0: - version "4.0.0" - resolved "https://registry.npmjs.org/cache-manager-memcached-store/-/cache-manager-memcached-store-4.0.0.tgz#f68d00cdfaa73696d13137b7b0f39397167d9220" - integrity sha512-Lv1WMaRC1FQHHqxE8Xbi7YWInhgfxjOmLEE6u1K0A3Rwmq+XRLyekKDM7aW9WIzF+PuII/RDKqBK/Ezo5H2QVw== - -cache-manager@^3.4.3: - version "3.4.3" - resolved "https://registry.npmjs.org/cache-manager/-/cache-manager-3.4.3.tgz#c978d58f82b414ade08903d4d72b56a80a4978be" - integrity sha512-6+Hfzy1SNs/thUwo+07pV0ozgxc4sadrAN0eFVGvXl/X9nz3J0BqEnnEoyxEn8jnF+UkEo0MKpyk9BO80hMeiQ== - dependencies: - async "3.2.0" - lodash "^4.17.21" - lru-cache "6.0.0" - cacheable-lookup@^5.0.3: version "5.0.3" resolved "https://registry.npmjs.org/cacheable-lookup/-/cacheable-lookup-5.0.3.tgz#049fdc59dffdd4fc285e8f4f82936591bd59fec3" @@ -9481,11 +9457,6 @@ capture-exit@^2.0.0: dependencies: rsvp "^4.8.4" -carrier@^0.3.0: - version "0.3.0" - resolved "https://registry.npmjs.org/carrier/-/carrier-0.3.0.tgz#bd295d1d3a7524cac63dd779b929ee22a362bad4" - integrity sha1-vSldHTp1JMrGPdd5uSnuIqNiutQ= - case-sensitive-paths-webpack-plugin@^2.2.0: version "2.3.0" resolved "https://registry.npmjs.org/case-sensitive-paths-webpack-plugin/-/case-sensitive-paths-webpack-plugin-2.3.0.tgz#23ac613cc9a856e4f88ff8bb73bbb5e989825cf7" @@ -10302,11 +10273,6 @@ connect-history-api-fallback@^1.6.0: resolved "https://registry.npmjs.org/connect-history-api-fallback/-/connect-history-api-fallback-1.6.0.tgz#8b32089359308d111115d81cad3fceab888f97bc" integrity sha512-e54B99q/OUoH64zYYRf3HBP5z24G38h5D3qXu23JGRoigpX5Ss4r9ZnDk3g0Z8uQC2x2lPaJ+UlWBc1ZWBWdLg== -connection-parse@0.0.x: - version "0.0.7" - resolved "https://registry.npmjs.org/connection-parse/-/connection-parse-0.0.7.tgz#18e7318aab06a699267372b10c5226d25a1c9a69" - integrity sha1-GOcxiqsGppkmc3KxDFIm0locmmk= - console-browserify@^1.1.0: version "1.2.0" resolved "https://registry.npmjs.org/console-browserify/-/console-browserify-1.2.0.tgz#67063cef57ceb6cf4993a2ab3a55840ae8c49336" @@ -14489,9 +14455,9 @@ graphql-extensions@^0.12.8: apollo-server-env "^3.0.0" apollo-server-types "^0.6.3" -graphql-language-service-interface@2.8.2, graphql-language-service-interface@^2.8.2: +graphql-language-service-interface@^2.8.2: version "2.8.2" - resolved "https://registry.npmjs.org/graphql-language-service-interface/-/graphql-language-service-interface-2.8.2.tgz#b3bb2aef7eaf0dff0b4ea419fa412c5f66fa268b" + resolved "https://registry.yarnpkg.com/graphql-language-service-interface/-/graphql-language-service-interface-2.8.2.tgz#b3bb2aef7eaf0dff0b4ea419fa412c5f66fa268b" integrity sha512-otbOQmhgkAJU1QJgQkMztNku6SbJLu/uodoFOYOOtJsizTjrMs93vkYaHCcYnLA3oi1Goj27XcHjMnRCYQOZXQ== dependencies: graphql-language-service-parser "^1.9.0" @@ -14499,9 +14465,9 @@ graphql-language-service-interface@2.8.2, graphql-language-service-interface@^2. graphql-language-service-utils "^2.5.1" vscode-languageserver-types "^3.15.1" -graphql-language-service-parser@1.9.0, graphql-language-service-parser@^1.9.0: +graphql-language-service-parser@^1.9.0: version "1.9.0" - resolved "https://registry.npmjs.org/graphql-language-service-parser/-/graphql-language-service-parser-1.9.0.tgz#79af21294119a0a7e81b6b994a1af36833bab724" + resolved "https://registry.yarnpkg.com/graphql-language-service-parser/-/graphql-language-service-parser-1.9.0.tgz#79af21294119a0a7e81b6b994a1af36833bab724" integrity sha512-B5xPZLbBmIp0kHvpY1Z35I5DtPoDK9wGxQVRDIzcBaiIvAmlTrDvjo3bu7vKREdjFbYKvWNgrEWENuprMbF17Q== dependencies: graphql-language-service-types "^1.8.0" @@ -14780,14 +14746,6 @@ hash.js@^1.0.0, hash.js@^1.0.3: inherits "^2.0.3" minimalistic-assert "^1.0.1" -hashring@^3.2.0: - version "3.2.0" - resolved "https://registry.npmjs.org/hashring/-/hashring-3.2.0.tgz#fda4efde8aa22cdb97fb1d2a65e88401e1c144ce" - integrity sha1-/aTv3oqiLNuX+x0qZeiEAeHBRM4= - dependencies: - connection-parse "0.0.x" - simple-lru-cache "0.0.x" - hast-util-parse-selector@^2.0.0: version "2.2.4" resolved "https://registry.npmjs.org/hast-util-parse-selector/-/hast-util-parse-selector-2.2.4.tgz#60c99d0b519e12ab4ed32e58f150ec3f61ed1974" @@ -16939,7 +16897,7 @@ json-buffer@3.0.0: resolved "https://registry.npmjs.org/json-buffer/-/json-buffer-3.0.0.tgz#5b1f397afc75d677bde8bcfc0e47e1f9a3d9a898" integrity sha1-Wx85evx11ne96Lz8Dkfh+aPZqJg= -json-buffer@3.0.1: +json-buffer@3.0.1, json-buffer@^3.0.1: version "3.0.1" resolved "https://registry.npmjs.org/json-buffer/-/json-buffer-3.0.1.tgz#9338802a30d3b6605fbe0613e094008ca8c05a13" integrity sha512-4bV5BfR2mqfQTJm+V5tPPdf+ZpuhiIvTuAB5g8kcrXOZpTT/QwwVRWBywX1ozr6lEuPdbHxwaJlm9G6mI2sfSQ== @@ -17291,6 +17249,14 @@ keytar@^5.4.0: nan "2.14.1" prebuild-install "5.3.3" +keyv-memcache@^1.2.5: + version "1.2.5" + resolved "https://registry.yarnpkg.com/keyv-memcache/-/keyv-memcache-1.2.5.tgz#9097af5c617dc740e7300ebfd4a2efb8917429e3" + integrity sha512-iG+GHlhXyV83gmlCtMFIqNYVvv0i0towAy42NvziUR7D+k9jp81fAq0lu66H4gooOnW+ojkdigRvRpL40j1f7w== + dependencies: + json-buffer "^3.0.1" + memjs "^1.3.0" + keyv@^3.0.0: version "3.1.0" resolved "https://registry.npmjs.org/keyv/-/keyv-3.1.0.tgz#ecc228486f69991e49e9476485a5be1e8fc5c4d9" @@ -17305,6 +17271,13 @@ keyv@^4.0.0: dependencies: json-buffer "3.0.1" +keyv@^4.0.3: + version "4.0.3" + resolved "https://registry.yarnpkg.com/keyv/-/keyv-4.0.3.tgz#4f3aa98de254803cafcd2896734108daa35e4254" + integrity sha512-zdGa2TOpSZPq5mU6iowDARnMBZgtCqJ11dJROFi6tg6kTn4nuUdU09lFyLFSaHrWqpIJ+EBq4E8/Dc0Vx5vLdA== + dependencies: + json-buffer "3.0.1" + killable@^1.0.1: version "1.0.1" resolved "https://registry.npmjs.org/killable/-/killable-1.0.1.tgz#4c8ce441187a061c7474fb87ca08e2a638194892" @@ -18065,13 +18038,6 @@ lru-cache@2: resolved "https://registry.npmjs.org/lru-cache/-/lru-cache-2.7.3.tgz#6d4524e8b955f95d4f5b58851ce21dd72fb4e952" integrity sha1-bUUk6LlV+V1PW1iFHOId1y+06VI= -lru-cache@6.0.0, lru-cache@^6.0.0: - version "6.0.0" - resolved "https://registry.yarnpkg.com/lru-cache/-/lru-cache-6.0.0.tgz#6d6fe6570ebd96aaf90fcad1dafa3b2566db3a94" - integrity sha512-Jo6dJ04CmSjuznwJSS3pUeWmd/H0ffTlkXXgwZi+eq1UCmqQwCh+eLsYOYCwY991i2Fah4h1BEMCx4qThGbsiA== - dependencies: - yallist "^4.0.0" - lru-cache@^4.0.1: version "4.1.5" resolved "https://registry.npmjs.org/lru-cache/-/lru-cache-4.1.5.tgz#8bbe50ea85bed59bc9e33dcab8235ee9bcf443cd" @@ -18087,6 +18053,13 @@ lru-cache@^5.0.0, lru-cache@^5.1.1: dependencies: yallist "^3.0.2" +lru-cache@^6.0.0: + version "6.0.0" + resolved "https://registry.yarnpkg.com/lru-cache/-/lru-cache-6.0.0.tgz#6d6fe6570ebd96aaf90fcad1dafa3b2566db3a94" + integrity sha512-Jo6dJ04CmSjuznwJSS3pUeWmd/H0ffTlkXXgwZi+eq1UCmqQwCh+eLsYOYCwY991i2Fah4h1BEMCx4qThGbsiA== + dependencies: + yallist "^4.0.0" + lru-queue@^0.1.0: version "0.1.0" resolved "https://registry.npmjs.org/lru-queue/-/lru-queue-0.1.0.tgz#2738bd9f0d3cf4f84490c5736c48699ac632cda3" @@ -18378,16 +18351,10 @@ media-typer@0.3.0: resolved "https://registry.npmjs.org/media-typer/-/media-typer-0.3.0.tgz#8710d7af0aa626f8fffa1ce00168545263255748" integrity sha1-hxDXrwqmJvj/+hzgAWhUUmMlV0g= -memcache-pp@^0.3.3: - version "0.3.3" - resolved "https://registry.npmjs.org/memcache-pp/-/memcache-pp-0.3.3.tgz#3124830e27d55a252499726510841590ac130c99" - integrity sha512-fMitetT5ulUs4DnFaoVqWHrTBiddLG/l8rMHXbV+ibuL7uWjBulaEf8koTBEweyrjmflqhbH4Uy9V709LadXSA== - dependencies: - bluebird "^3.4.1" - carrier "^0.3.0" - debug "^2.6.9" - hashring "^3.2.0" - immutable "^3.8.1" +memjs@^1.3.0: + version "1.3.0" + resolved "https://registry.yarnpkg.com/memjs/-/memjs-1.3.0.tgz#b7959b4ff3770e4c785463fd147f1e4fafd47a24" + integrity sha512-y/V9a0auepA9Lgyr4QieK6K2FczjHucEdTpSS+hHVNmVEkYxruXhkHu8n6DSRQ4HXHEE3cc6Sf9f88WCJXGXsQ== memoize-one@^5.1.1: version "5.1.1" @@ -23833,11 +23800,6 @@ simple-get@^3.0.2, simple-get@^3.0.3: once "^1.3.1" simple-concat "^1.0.0" -simple-lru-cache@0.0.x: - version "0.0.2" - resolved "https://registry.npmjs.org/simple-lru-cache/-/simple-lru-cache-0.0.2.tgz#d59cc3a193c1a5d0320f84ee732f6e4713e511dd" - integrity sha1-1ZzDoZPBpdAyD4Tucy9uRxPlEd0= - simple-swizzle@^0.2.2: version "0.2.2" resolved "https://registry.npmjs.org/simple-swizzle/-/simple-swizzle-0.2.2.tgz#a4da6b635ffcccca33f70d17cb92592de95e557a" @@ -25779,11 +25741,16 @@ typescript-json-schema@^0.49.0: typescript "^4.1.3" yargs "^16.2.0" -typescript@^4.0.3, typescript@^4.1.3, typescript@~4.1.3: +typescript@^4.0.3, typescript@^4.1.3: version "4.2.3" resolved "https://registry.npmjs.org/typescript/-/typescript-4.2.3.tgz#39062d8019912d43726298f09493d598048c1ce3" integrity sha512-qOcYwxaByStAWrBf4x0fibwZvMRG+r4cQoTjbPtUlrWjBHbmCAww1i448U0GJ+3cNNEtebDteo/cHOR3xJ4wEw== +typescript@~4.1.3: + version "4.1.5" + resolved "https://registry.yarnpkg.com/typescript/-/typescript-4.1.5.tgz#123a3b214aaff3be32926f0d8f1f6e704eb89a72" + integrity sha512-6OSu9PTIzmn9TCDiovULTnET6BgXtDYL4Gg4szY+cGsc3JP1dQL8qvE8kShTRx1NIw4Q9IBHlwODjkjWEtMUyA== + ua-parser-js@^0.7.18: version "0.7.28" resolved "https://registry.yarnpkg.com/ua-parser-js/-/ua-parser-js-0.7.28.tgz#8ba04e653f35ce210239c64661685bf9121dec31" From 38e943da0ccec78bb36a506b40ba9d9b0e5e496c Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Sun, 9 May 2021 17:52:06 +0200 Subject: [PATCH 15/19] Implement a NoStore cache backend, and do not provide a cache backend by default. Signed-off-by: Eric Peterson --- app-config.yaml | 2 - .../src/cache/CacheManager.test.ts | 12 +++--- .../backend-common/src/cache/CacheManager.ts | 17 +++++--- packages/backend-common/src/cache/NoStore.ts | 43 +++++++++++++++++++ 4 files changed, 60 insertions(+), 14 deletions(-) create mode 100644 packages/backend-common/src/cache/NoStore.ts diff --git a/app-config.yaml b/app-config.yaml index 8224b2fb6a..3c4f2d7ea5 100644 --- a/app-config.yaml +++ b/app-config.yaml @@ -26,8 +26,6 @@ backend: baseUrl: http://localhost:7000 listen: port: 7000 - cache: - store: memory database: client: sqlite3 connection: ':memory:' diff --git a/packages/backend-common/src/cache/CacheManager.test.ts b/packages/backend-common/src/cache/CacheManager.test.ts index 558964b55c..153ccfd7ac 100644 --- a/packages/backend-common/src/cache/CacheManager.test.ts +++ b/packages/backend-common/src/cache/CacheManager.test.ts @@ -20,6 +20,7 @@ import Keyv from 'keyv'; import KeyvMemcache from 'keyv-memcache'; import { DefaultCacheClient } from './CacheClient'; import { CacheManager } from './CacheManager'; +import { NoStore } from './NoStore'; jest.createMockFromModule('keyv'); jest.mock('keyv'); @@ -104,21 +105,17 @@ describe('CacheManager', () => { }); describe('CacheManager.forPlugin stores', () => { - it('returns memory client when no cache is configured', () => { + it('returns none client when no cache is configured', () => { const manager = CacheManager.fromConfig( new ConfigReader({ backend: {} }), ); - const expectedTtl = 3600; const expectedNamespace = 'test-plugin'; - manager.forPlugin(expectedNamespace).getClient({ defaultTtl: expectedTtl }); + manager.forPlugin(expectedNamespace).getClient(); const cache = Keyv as unknown as jest.Mock; const mockCalls = cache.mock.calls.splice(-1); const callArgs = mockCalls[0]; - expect(callArgs[0]).toMatchObject({ - ttl: expectedTtl, - namespace: expectedNamespace, - }); + expect(callArgs[0].store).toBeInstanceOf(NoStore); }); it('returns memory client when explicitly configured', () => { @@ -156,6 +153,7 @@ describe('CacheManager', () => { expect(mockCacheCalls[0][0]).toMatchObject({ ttl: expectedTtl, }); + expect(mockCacheCalls[0][0].store).toBeInstanceOf(KeyvMemcache); const memcache = KeyvMemcache as jest.Mock; const mockMemcacheCalls = memcache.mock.calls.splice(-1); expect(mockMemcacheCalls[0][0]).toEqual(expectedHost); diff --git a/packages/backend-common/src/cache/CacheManager.ts b/packages/backend-common/src/cache/CacheManager.ts index 00c4cebf18..967ff4224b 100644 --- a/packages/backend-common/src/cache/CacheManager.ts +++ b/packages/backend-common/src/cache/CacheManager.ts @@ -19,11 +19,12 @@ import Keyv from 'keyv'; // @ts-expect-error import KeyvMemcache from 'keyv-memcache'; import { DefaultCacheClient, CacheClient } from './CacheClient'; +import { NoStore } from './NoStore'; import { PluginCacheManager } from './types'; /** * Implements a Cache Manager which will automatically create new cache clients - * for plugins when requested. All requested cacheclients are created with the + * for plugins when requested. All requested cache clients are created with the * connection details provided. */ export class CacheManager { @@ -34,6 +35,7 @@ export class CacheManager { private readonly storeFactories = { memcache: this.getMemcacheClient, memory: this.getMemoryClient, + none: this.getNoneClient, }; private readonly store: keyof CacheManager['storeFactories']; @@ -47,8 +49,8 @@ export class CacheManager { */ static fromConfig(config: Config): CacheManager { // If no `backend.cache` config is provided, instantiate the CacheManager - // with a "memory" cache client. - const store = config.getOptionalString('backend.cache.store') || 'memory'; + // with a "NoStore" cache client. + const store = config.getOptionalString('backend.cache.store') || 'none'; const connectionString = config.getOptionalString('backend.cache.connection') || ''; return new CacheManager(store, connectionString); } @@ -82,11 +84,10 @@ export class CacheManager { } private getMemcacheClient(pluginId: string, defaultTtl: number | undefined): Keyv { - const memcache = new KeyvMemcache(this.connection); return new Keyv({ namespace: pluginId, ttl: defaultTtl, - store: memcache, + store: new KeyvMemcache(this.connection), }); } @@ -97,4 +98,10 @@ export class CacheManager { }); } + private getNoneClient(pluginId: string): Keyv { + return new Keyv({ + namespace: pluginId, + store: new NoStore(), + }) + } } diff --git a/packages/backend-common/src/cache/NoStore.ts b/packages/backend-common/src/cache/NoStore.ts new file mode 100644 index 0000000000..6acb7c0309 --- /dev/null +++ b/packages/backend-common/src/cache/NoStore.ts @@ -0,0 +1,43 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Storage class compatible with Keyv which always results in a no-op. This is + * used when no cache store is configured in a Backstage backend instance. + */ +export class NoStore extends Map { + + clear(): void { + return; + } + + delete(_key: string): boolean { + return false; + } + + get(_key: string) { + return; + } + + has(_key: string): boolean { + return false; + } + + set(_key: string, _value: any): this { + return this; + } + +} From d48b375a81718c9a2b9da7d5c19631a8c754b80d Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Sun, 9 May 2021 17:52:38 +0200 Subject: [PATCH 16/19] Update changeset to reflect most recent changes. Signed-off-by: Eric Peterson --- .changeset/cool-cache-bro.md | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/.changeset/cool-cache-bro.md b/.changeset/cool-cache-bro.md index f342f39e55..9d5f998897 100644 --- a/.changeset/cool-cache-bro.md +++ b/.changeset/cool-cache-bro.md @@ -6,8 +6,8 @@ Introducing: a standard API for App Integrators to configure cache stores and Pl Two cache stores are currently supported. -- `memory`, which is a very simple in-memory LRU cache, intended for local development. -- `memcache`, which can be used to connect to one or more memcache hosts. +- `memory`, which is a very simple in-memory key/value store, intended for local development. +- `memcache`, which can be used to connect to a memcache host. Configuring and working with cache stores is very similar to the process for database connections. @@ -15,10 +15,7 @@ Configuring and working with cache stores is very similar to the process for dat backend: cache: store: memcache - connection: - hosts: - - cache-a.example.com:11211 - - cache-b.example.com:11211 + connection: user:pass@cache.example.com:11211 ``` ```typescript @@ -27,8 +24,7 @@ import { CacheManager } from '@backstage/backend-common'; // Instantiating a cache client for a plugin. const cacheManager = CacheManager.fromConfig(config); const somePluginCache = cacheManager.forPlugin('somePlugin'); -const defaultTtl = 3600; -const cacheClient = somePluginCache.getClient({ defaultTtl }); +const cacheClient = somePluginCache.getClient(); // Using the cache client: const cachedValue = await cacheClient.get('someKey'); @@ -41,19 +37,16 @@ if (cachedValue) { await cacheClient.delete('someKey'); ``` -For simplicity of use, cache clients will swallow client errors by default. Plugin developers may optionally pass an `onError` argument to the `CacheManager.getClient()` method if they wish to capture and handle those errors in a custom way. +Cache clients deal with TTLs in milliseconds. A TTL can be provided as a defaultTtl when getting a client, or may be passed when setting specific objects. If no TTL is provided, data will be persisted indefinitely. ```typescript +// Getting a client with a default TTL const cacheClient = somePluginCache.getClient({ - defaultTtl: 3600, - onError: 'reject', + defaultTtl: 3600000, }); -try { - await cacheClient.delete('someKey'); -} catch (e) { - // Attempt again, log, alert, etc. -} +// Setting a TTL on a per-object basis. +cacheClient.set('someKey', data, { ttl: 3600000 }); ``` Configuring a cache store is optional. Even when no cache store is configured, the cache manager will dutifully pass plugins a manager that resolves a cache client that does not actually write or read any data. From 03ae33c064aa194b45b7ba5c6a756cb7438bd633 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Sun, 9 May 2021 18:08:57 +0200 Subject: [PATCH 17/19] Prettier Signed-off-by: Eric Peterson --- packages/backend-common/src/cache/CacheClient.ts | 9 ++++++--- .../src/cache/CacheManager.test.ts | 16 ++++++++++------ .../backend-common/src/cache/CacheManager.ts | 15 +++++++++++---- packages/backend-common/src/cache/NoStore.ts | 2 -- 4 files changed, 27 insertions(+), 15 deletions(-) diff --git a/packages/backend-common/src/cache/CacheClient.ts b/packages/backend-common/src/cache/CacheClient.ts index 943de055ae..a2c048c265 100644 --- a/packages/backend-common/src/cache/CacheClient.ts +++ b/packages/backend-common/src/cache/CacheClient.ts @@ -46,7 +46,11 @@ export interface CacheClient { * optional TTL may also be provided, otherwise it defaults to the TTL that * was provided when the client was instantiated. */ - set(key: string, value: JsonValue, options?: CacheSetOptions): Promise; + set( + key: string, + value: JsonValue, + options?: CacheSetOptions, + ): Promise; /** * Removes the given key from the cache store. Resolves true if the key @@ -77,7 +81,7 @@ export class DefaultCacheClient implements CacheClient { opts: CacheSetOptions = {}, ): Promise { const k = this.getNormalizedKey(key); - return await this.client.set(k, value, opts.ttl) + return await this.client.set(k, value, opts.ttl); } async delete(key: string): Promise { @@ -99,5 +103,4 @@ export class DefaultCacheClient implements CacheClient { return createHash('md5').update(candidateKey).digest('base64'); } - } diff --git a/packages/backend-common/src/cache/CacheManager.test.ts b/packages/backend-common/src/cache/CacheManager.test.ts index 153ccfd7ac..ece4687ee6 100644 --- a/packages/backend-common/src/cache/CacheManager.test.ts +++ b/packages/backend-common/src/cache/CacheManager.test.ts @@ -53,7 +53,9 @@ describe('CacheManager', () => { CacheManager.fromConfig(config); expect(getOptionalString.mock.calls[0][0]).toEqual('backend.cache.store'); - expect(getOptionalString.mock.calls[1][0]).toEqual('backend.cache.connection'); + expect(getOptionalString.mock.calls[1][0]).toEqual( + 'backend.cache.connection', + ); }); it('does not require the backend.cache key', () => { @@ -92,7 +94,7 @@ describe('CacheManager', () => { manager.forPlugin(plugin2Id).getClient({ defaultTtl: expectedTtl }); const client = DefaultCacheClient as jest.Mock; - const cache = Keyv as unknown as jest.Mock; + const cache = (Keyv as unknown) as jest.Mock; expect(cache).toHaveBeenCalledTimes(2); expect(client).toHaveBeenCalledTimes(2); @@ -112,7 +114,7 @@ describe('CacheManager', () => { const expectedNamespace = 'test-plugin'; manager.forPlugin(expectedNamespace).getClient(); - const cache = Keyv as unknown as jest.Mock; + const cache = (Keyv as unknown) as jest.Mock; const mockCalls = cache.mock.calls.splice(-1); const callArgs = mockCalls[0]; expect(callArgs[0].store).toBeInstanceOf(NoStore); @@ -122,9 +124,11 @@ describe('CacheManager', () => { const manager = CacheManager.fromConfig(defaultConfig()); const expectedTtl = 3600; const expectedNamespace = 'test-plugin'; - manager.forPlugin(expectedNamespace).getClient({ defaultTtl: expectedTtl }); + manager + .forPlugin(expectedNamespace) + .getClient({ defaultTtl: expectedTtl }); - const cache = Keyv as unknown as jest.Mock; + const cache = (Keyv as unknown) as jest.Mock; const mockCalls = cache.mock.calls.splice(-1); const callArgs = mockCalls[0]; expect(callArgs[0]).toMatchObject({ @@ -148,7 +152,7 @@ describe('CacheManager', () => { const expectedTtl = 3600; manager.forPlugin('test').getClient({ defaultTtl: expectedTtl }); - const cache = Keyv as unknown as jest.Mock; + const cache = (Keyv as unknown) as jest.Mock; const mockCacheCalls = cache.mock.calls.splice(-1); expect(mockCacheCalls[0][0]).toMatchObject({ ttl: expectedTtl, diff --git a/packages/backend-common/src/cache/CacheManager.ts b/packages/backend-common/src/cache/CacheManager.ts index 967ff4224b..fbd7e3ba0a 100644 --- a/packages/backend-common/src/cache/CacheManager.ts +++ b/packages/backend-common/src/cache/CacheManager.ts @@ -51,7 +51,8 @@ export class CacheManager { // If no `backend.cache` config is provided, instantiate the CacheManager // with a "NoStore" cache client. const store = config.getOptionalString('backend.cache.store') || 'none'; - const connectionString = config.getOptionalString('backend.cache.connection') || ''; + const connectionString = + config.getOptionalString('backend.cache.connection') || ''; return new CacheManager(store, connectionString); } @@ -83,7 +84,10 @@ export class CacheManager { return this.storeFactories[this.store].call(this, pluginId, ttl); } - private getMemcacheClient(pluginId: string, defaultTtl: number | undefined): Keyv { + private getMemcacheClient( + pluginId: string, + defaultTtl: number | undefined, + ): Keyv { return new Keyv({ namespace: pluginId, ttl: defaultTtl, @@ -91,7 +95,10 @@ export class CacheManager { }); } - private getMemoryClient(pluginId: string, defaultTtl: number | undefined): Keyv { + private getMemoryClient( + pluginId: string, + defaultTtl: number | undefined, + ): Keyv { return new Keyv({ namespace: pluginId, ttl: defaultTtl, @@ -102,6 +109,6 @@ export class CacheManager { return new Keyv({ namespace: pluginId, store: new NoStore(), - }) + }); } } diff --git a/packages/backend-common/src/cache/NoStore.ts b/packages/backend-common/src/cache/NoStore.ts index 6acb7c0309..c89e814c93 100644 --- a/packages/backend-common/src/cache/NoStore.ts +++ b/packages/backend-common/src/cache/NoStore.ts @@ -19,7 +19,6 @@ * used when no cache store is configured in a Backstage backend instance. */ export class NoStore extends Map { - clear(): void { return; } @@ -39,5 +38,4 @@ export class NoStore extends Map { set(_key: string, _value: any): this { return this; } - } From 6e2b9a92d655d0cd7faa34bb9854b9fec7e959fc Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Sun, 9 May 2021 18:16:24 +0200 Subject: [PATCH 18/19] Updating backend-common API Signed-off-by: Eric Peterson --- packages/backend-common/api-report.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/backend-common/api-report.md b/packages/backend-common/api-report.md index bd1b017574..5a9b61dd20 100644 --- a/packages/backend-common/api-report.md +++ b/packages/backend-common/api-report.md @@ -65,9 +65,9 @@ export class BitbucketUrlReader implements UrlReader { // @public export interface CacheClient { - delete(key: string): Promise; + delete(key: string): Promise; get(key: string): Promise; - set(key: string, value: JsonValue, options: CacheSetOptions): Promise; + set(key: string, value: JsonValue, options?: CacheSetOptions): Promise; } // @public @@ -254,7 +254,7 @@ export function notFoundHandler(): RequestHandler; // @public export type PluginCacheManager = { - getClient: (options: ClientOptions) => CacheClient; + getClient: (options?: ClientOptions) => CacheClient; }; // @public From a8c70b3ae45d133d47d42ef82de30590788d81e1 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Mon, 10 May 2021 18:21:27 +0200 Subject: [PATCH 19/19] Maybe not THAT transparent. Signed-off-by: Eric Peterson --- packages/backend-common/api-report.md | 4 ++-- .../src/cache/CacheClient.test.ts | 20 ------------------- .../backend-common/src/cache/CacheClient.ts | 19 +++++++----------- 3 files changed, 9 insertions(+), 34 deletions(-) diff --git a/packages/backend-common/api-report.md b/packages/backend-common/api-report.md index 5a9b61dd20..23e22fe34b 100644 --- a/packages/backend-common/api-report.md +++ b/packages/backend-common/api-report.md @@ -65,9 +65,9 @@ export class BitbucketUrlReader implements UrlReader { // @public export interface CacheClient { - delete(key: string): Promise; + delete(key: string): Promise; get(key: string): Promise; - set(key: string, value: JsonValue, options?: CacheSetOptions): Promise; + set(key: string, value: JsonValue, options?: CacheSetOptions): Promise; } // @public diff --git a/packages/backend-common/src/cache/CacheClient.test.ts b/packages/backend-common/src/cache/CacheClient.test.ts index eb32d4a28f..81e8351b58 100644 --- a/packages/backend-common/src/cache/CacheClient.test.ts +++ b/packages/backend-common/src/cache/CacheClient.test.ts @@ -101,16 +101,6 @@ describe('CacheClient', () => { return expect(sut.set('someKey', {})).rejects.toEqual(expectedError); }); - - it('resolves what underlying client resolves', async () => { - const sut = new DefaultCacheClient({ client }); - const expectedResponse = true; - client.set = jest.fn().mockReturnValue(expectedResponse); - - const actualResponse = await sut.set('someKey', {}); - - return expect(actualResponse).toEqual(expectedResponse); - }); }); describe('CacheClient.delete', () => { @@ -132,15 +122,5 @@ describe('CacheClient', () => { return expect(sut.delete('someKey')).rejects.toEqual(expectedError); }); - - it('resolves what underlying client resolves', async () => { - const sut = new DefaultCacheClient({ client }); - const expectedResponse = false; - client.delete = jest.fn().mockResolvedValue(expectedResponse); - - const actualResponse = await sut.delete('someKey'); - - return expect(actualResponse).toEqual(expectedResponse); - }); }); }); diff --git a/packages/backend-common/src/cache/CacheClient.ts b/packages/backend-common/src/cache/CacheClient.ts index a2c048c265..49b5c367dd 100644 --- a/packages/backend-common/src/cache/CacheClient.ts +++ b/packages/backend-common/src/cache/CacheClient.ts @@ -46,17 +46,12 @@ export interface CacheClient { * optional TTL may also be provided, otherwise it defaults to the TTL that * was provided when the client was instantiated. */ - set( - key: string, - value: JsonValue, - options?: CacheSetOptions, - ): Promise; + set(key: string, value: JsonValue, options?: CacheSetOptions): Promise; /** - * Removes the given key from the cache store. Resolves true if the key - * existed, or false if not. + * Removes the given key from the cache store. */ - delete(key: string): Promise; + delete(key: string): Promise; } /** @@ -79,14 +74,14 @@ export class DefaultCacheClient implements CacheClient { key: string, value: JsonValue, opts: CacheSetOptions = {}, - ): Promise { + ): Promise { const k = this.getNormalizedKey(key); - return await this.client.set(k, value, opts.ttl); + await this.client.set(k, value, opts.ttl); } - async delete(key: string): Promise { + async delete(key: string): Promise { const k = this.getNormalizedKey(key); - return await this.client.delete(k); + await this.client.delete(k); } /**