From 7ee38d363695b74c31fe4abb7a0726b7fac16aa8 Mon Sep 17 00:00:00 2001 From: Dani Pinyol Date: Wed, 16 Jun 2021 11:48:02 +0200 Subject: [PATCH] refactor(contentful): FallbackCache memory limit in Contentful options --- .../src/contentful/cms-contentful.ts | 9 ++++++-- .../src/contentful/delivery/fallback-cache.ts | 22 +++++++++++++++++-- .../botonic-plugin-contentful/src/plugin.ts | 5 +++++ .../src/util/memoizer.ts | 19 +++------------- .../delivery/fallback-cache.test.ts | 6 ++--- 5 files changed, 38 insertions(+), 23 deletions(-) diff --git a/packages/botonic-plugin-contentful/src/contentful/cms-contentful.ts b/packages/botonic-plugin-contentful/src/contentful/cms-contentful.ts index d72832948f..5f37a13be9 100644 --- a/packages/botonic-plugin-contentful/src/contentful/cms-contentful.ts +++ b/packages/botonic-plugin-contentful/src/contentful/cms-contentful.ts @@ -14,7 +14,7 @@ import { TopContent, TopContentType, } from '../cms' -import { ContentfulOptions } from '../plugin' +import { ContentfulOptions, DEFAULT_FALLBACK_CACHE_LIMIT_KB } from '../plugin' import { SearchCandidate } from '../search' import { AssetDelivery } from './contents/asset' import { ButtonDelivery } from './contents/button' @@ -76,7 +76,12 @@ export class Contentful implements cms.CMS { } let client: ReducedClientApi = createContentfulClientApi(options) if (!options.disableFallbackCache) { - client = new FallbackCachedClientApi(client, reporter, logger) + client = new FallbackCachedClientApi( + client, + options.fallbackCacheLimitKB ?? DEFAULT_FALLBACK_CACHE_LIMIT_KB, + reporter, + logger + ) } if (!options.disableCache) { client = new CachedClientApi(client, options.cacheTtlMs, reporter) diff --git a/packages/botonic-plugin-contentful/src/contentful/delivery/fallback-cache.ts b/packages/botonic-plugin-contentful/src/contentful/delivery/fallback-cache.ts index 25e740c6d0..6f4669d51f 100644 --- a/packages/botonic-plugin-contentful/src/contentful/delivery/fallback-cache.ts +++ b/packages/botonic-plugin-contentful/src/contentful/delivery/fallback-cache.ts @@ -1,6 +1,7 @@ import * as contentful from 'contentful' import { ContentType } from 'contentful' +import { InMemoryCache, LimitedCacheDecorator } from '../../util/cache' import { fallbackStrategy, Memoizer } from '../../util/memoizer' import { ClientApiErrorReporter, @@ -21,18 +22,29 @@ export class FallbackCachedClientApi implements ReducedClientApi { readonly getEntries: GetEntriesType readonly getEntry: GetEntryType readonly getContentType: (id: string) => Promise + private static readonly NUM_APIS = 5 + private numMemoizations = 0 constructor( client: ReducedClientApi, + cacheLimitKB: number, reporter: ClientApiErrorReporter, - private readonly logger: (msg: string) => void = console.error + logger: (msg: string) => void = console.error ) { + // TODO share the same cache for all APIs to avoid reaching a Memoizer limit + // while others have empty space + const memoizerCache = () => + new LimitedCacheDecorator( + new InMemoryCache(), + cacheLimitKB / FallbackCachedClientApi.NUM_APIS, + logger + ) // We could maybe use a more optimal normalizer than jsonNormalizer // (like they do in fast-json-stringify to avoid JSON.stringify for functions with a single nulls, numbers and booleans). // But it's not worth since stringify will have a cost much lower than constructing/rendering a botonic component // (and we're already optimizing the costly call to CMS) this.memoizer = new Memoizer({ - logger: this.logger, + cacheFactory: memoizerCache, strategy: fallbackStrategy((f, args, e) => reporter( `Successfully used cached fallback after Contentful API error`, @@ -49,6 +61,11 @@ export class FallbackCachedClientApi implements ReducedClientApi { ) as GetEntriesType this.getEntry = this.memoize(client.getEntry.bind(client)) as GetEntryType this.getContentType = this.memoize(client.getContentType.bind(client)) + if (this.numMemoizations != FallbackCachedClientApi.NUM_APIS) { + throw new Error( + 'FallbackCachedClientApi.NUM_APIS must equal the number of memoized APIs' + ) + } } memoize< @@ -56,6 +73,7 @@ export class FallbackCachedClientApi implements ReducedClientApi { Return, F extends (...args: Args) => Promise >(func: F): F { + this.numMemoizations++ return this.memoizer.memoize(func) } } diff --git a/packages/botonic-plugin-contentful/src/plugin.ts b/packages/botonic-plugin-contentful/src/plugin.ts index 4f2272a935..a925ac8d97 100644 --- a/packages/botonic-plugin-contentful/src/plugin.ts +++ b/packages/botonic-plugin-contentful/src/plugin.ts @@ -24,6 +24,7 @@ export interface CmsOptions extends OptionsBase { export const DEFAULT_TIMEOUT_MS = 30000 export const DEFAULT_CACHE_TTL_MS = 10000 +export const DEFAULT_FALLBACK_CACHE_LIMIT_KB = 100 * 1024 export interface ContentfulCredentials { spaceId: string @@ -50,6 +51,10 @@ export interface ContentfulOptions extends OptionsBase, ContentfulCredentials { * fail. */ disableFallbackCache?: boolean + /** + * {@link DEFAULT_FALLBACK_CACHE_LIMIT_KB} by default + */ + fallbackCacheLimitKB?: number contentfulFactory?: (opts: ContentfulOptions) => cms.CMS diff --git a/packages/botonic-plugin-contentful/src/util/memoizer.ts b/packages/botonic-plugin-contentful/src/util/memoizer.ts index 5a39c13bcf..b49f7699ce 100644 --- a/packages/botonic-plugin-contentful/src/util/memoizer.ts +++ b/packages/botonic-plugin-contentful/src/util/memoizer.ts @@ -1,9 +1,5 @@ -import { - Cache, - InMemoryCache, - LimitedCacheDecorator, - NOT_FOUND_IN_CACHE, -} from './cache' +import { Cache, InMemoryCache, NOT_FOUND_IN_CACHE } from './cache' + export type MemoizerNormalizer = (...args: any) => string export const jsonNormalizer: MemoizerNormalizer = (...args: any) => { @@ -23,7 +19,6 @@ export type MemoizerStrategy = ( export interface MemoizerOptions { strategy: MemoizerStrategy - logger?: (msg: string) => void cacheFactory?: () => Cache normalizer?: MemoizerNormalizer } @@ -33,16 +28,8 @@ export class Memoizer { constructor(opts: MemoizerOptions) { this.opts = { strategy: opts.strategy, - logger: opts.logger || console.error, normalizer: opts.normalizer || jsonNormalizer, - cacheFactory: - opts.cacheFactory || - (() => - new LimitedCacheDecorator( - new InMemoryCache(), - 100 * 1024, - this.opts.logger - )), + cacheFactory: opts.cacheFactory || (() => new InMemoryCache()), } } diff --git a/packages/botonic-plugin-contentful/tests/contentful/delivery/fallback-cache.test.ts b/packages/botonic-plugin-contentful/tests/contentful/delivery/fallback-cache.test.ts index 65e6fed328..e9041ca1b1 100644 --- a/packages/botonic-plugin-contentful/tests/contentful/delivery/fallback-cache.test.ts +++ b/packages/botonic-plugin-contentful/tests/contentful/delivery/fallback-cache.test.ts @@ -54,7 +54,7 @@ async function testCallAfterHit( expectedReturn: (api: MockClientApi) => R ) { const mockApi = new MockClientApi() - const sut = new FallbackCachedClientApi(mockApi, usingFallback) + const sut = new FallbackCachedClientApi(mockApi, 1, usingFallback) const expected = expectedReturn(mockApi) await expect(call(sut)).resolves.toBe(expected) @@ -69,7 +69,7 @@ async function testFallbackIfFailure( expectedReturn: (api: MockClientApi) => R ) { const mockApi = new MockClientApi() - const sut = new FallbackCachedClientApi(mockApi, usingFallback) + const sut = new FallbackCachedClientApi(mockApi, 1, usingFallback) const expected = expectedReturn(mockApi) await expect(call(sut)).resolves.toBe(expected) @@ -93,7 +93,7 @@ async function testSuccessAfterFailure( expectedReturn: (api: MockClientApi) => R ) { const mockApi = new MockClientApi() - const sut = new FallbackCachedClientApi(mockApi, usingFallback) + const sut = new FallbackCachedClientApi(mockApi, 1, usingFallback) const expected = expectedReturn(mockApi) mockApi.error = new Error('forced failure')