refactor(contentful): FallbackCache memory limit in Contentful options

hubtype · Jun 16, 2021 · 7ee38d3 · 7ee38d3
1 parent 3aeaa10
commit 7ee38d3
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 23 deletions.
diff --git 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
@@ -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<ContentType>
+  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<any>(),
+        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,13 +61,19 @@ 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<
     Args extends any[],
     Return,
     F extends (...args: Args) => Promise<Return>
   >(func: F): F {
+    this.numMemoizations++
     return this.memoizer.memoize<Args, Return, F>(func)
   }
 }
diff --git 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
@@ -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 = <Args extends any[], Return>(
 
 export interface MemoizerOptions {
   strategy: MemoizerStrategy
-  logger?: (msg: string) => void
   cacheFactory?: () => Cache<any>
   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<any>(),
-            100 * 1024,
-            this.opts.logger
-          )),
+      cacheFactory: opts.cacheFactory || (() => new InMemoryCache<any>()),
     }
   }
 

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
@@ -54,7 +54,7 @@ async function testCallAfterHit<R>(
   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<R>(
   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<R>(
   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')