Merge pull request #16487 from mozilla/polish-update-type-cacheable

feat(type-cacheable): update to load firestore adapter from lambda

libs/payments/legacy/src/lib/stripe-mapper.service.spec.ts

@@ -26,7 +26,7 @@ describe('StripeMapperService', () => {
         .mockResolvedValue(
           mockContentfulConfigUtil as unknown as PurchaseWithDetailsOfferingContentUtil
         );
-      const contentfulClient = new ContentfulClient({} as any);
+      const contentfulClient = new ContentfulClient({} as any, {} as any);
       const mockStatsd = {} as any;
       stripeMapper = new StripeMapperService(
         new ContentfulManager(contentfulClient, mockStatsd)

libs/shared/contentful/src/lib/contentful.client.spec.ts

@@ -13,6 +13,7 @@ import {
   ContentfulError,
 } from './contentful.error';
 import { ContentfulCDNErrorFactory } from './factories';
+import { Firestore } from '@google-cloud/firestore';
 
 jest.mock('graphql-request', () => ({
   GraphQLClient: function () {
@@ -22,12 +23,15 @@ jest.mock('graphql-request', () => ({
   },
 }));
 
-jest.mock('@fxa/shared/db/type-cacheable', () => ({
-  FirestoreCacheable: () => {
+jest.mock('@type-cacheable/core', () => ({
+  Cacheable: () => {
     return (target: any, propertyKey: any, descriptor: any) => {
       return descriptor;
     };
   },
+}));
+
+jest.mock('@fxa/shared/db/type-cacheable', () => ({
   NetworkFirstStrategy: function () {},
 }));
 
@@ -36,14 +40,17 @@ describe('ContentfulClient', () => {
   const onCallback = jest.fn();
 
   beforeEach(() => {
-    contentfulClient = new ContentfulClient({
-      cdnApiUri: faker.string.uuid(),
-      graphqlApiKey: faker.string.uuid(),
-      graphqlApiUri: faker.string.uuid(),
-      graphqlSpaceId: faker.string.uuid(),
-      graphqlEnvironment: faker.string.uuid(),
-      firestoreCacheCollectionName: faker.string.uuid(),
-    });
+    contentfulClient = new ContentfulClient(
+      {
+        cdnApiUri: faker.string.uuid(),
+        graphqlApiKey: faker.string.uuid(),
+        graphqlApiUri: faker.string.uuid(),
+        graphqlSpaceId: faker.string.uuid(),
+        graphqlEnvironment: faker.string.uuid(),
+        firestoreCacheCollectionName: faker.string.uuid(),
+      },
+      {} as unknown as Firestore
+    );
     contentfulClient.on('response', onCallback);
   });
 

libs/shared/contentful/src/lib/contentful.client.ts

@@ -5,7 +5,7 @@
 import type { OperationVariables } from '@apollo/client';
 import { GraphQLClient } from 'graphql-request';
 import type { TypedDocumentNode } from '@graphql-typed-document-node/core';
-import { Injectable } from '@nestjs/common';
+import { Inject, Injectable } from '@nestjs/common';
 import { determineLocale } from '@fxa/shared/l10n';
 import { DEFAULT_LOCALE } from './constants';
 import { ContentfulClientConfig } from './contentful.client.config';
@@ -17,10 +17,13 @@ import {
 import { ContentfulErrorResponse } from './types';
 import EventEmitter from 'events';
 import {
-  FirestoreCacheable,
+  FirestoreAdapter,
   NetworkFirstStrategy,
 } from '@fxa/shared/db/type-cacheable';
 import { CONTENTFUL_QUERY_CACHE_KEY, cacheKeyForQuery } from './util';
+import { Cacheable } from '@type-cacheable/core';
+import { FirestoreService } from '@fxa/shared/db/firestore';
+import { Firestore } from '@google-cloud/firestore';
 
 const DEFAULT_FIRESTORE_CACHE_TTL = 604800; // Seconds. 604800 is 7 days.
 const DEFAULT_MEM_CACHE_TTL = 300; // Seconds
@@ -49,7 +52,10 @@ export class ContentfulClient {
   ) => EventEmitter;
   private graphqlMemCache: Record<string, unknown> = {};
 
-  constructor(private contentfulClientConfig: ContentfulClientConfig) {
+  constructor(
+    private contentfulClientConfig: ContentfulClientConfig,
+    @Inject(FirestoreService) private firestore: Firestore
+  ) {
     this.setupCacheBust();
     this.emitter = new EventEmitter();
     this.on = this.emitter.on.bind(this.emitter);
@@ -64,20 +70,19 @@ export class ContentfulClient {
     return result;
   }
 
-  @FirestoreCacheable(
-    {
-      cacheKey: (args: any) => cacheKeyForQuery(args[0], args[1]),
-      strategy: new NetworkFirstStrategy(),
-      ttlSeconds: (_, context) =>
-        context.contentfulClientConfig.firestoreCacheTTL ||
-        DEFAULT_FIRESTORE_CACHE_TTL,
-    },
-    {
-      collectionName: (_, context) =>
+  @Cacheable({
+    cacheKey: (args: any) => cacheKeyForQuery(args[0], args[1]),
+    strategy: new NetworkFirstStrategy(),
+    ttlSeconds: (_, context: ContentfulClient) =>
+      context.contentfulClientConfig.firestoreCacheTTL ||
+      DEFAULT_FIRESTORE_CACHE_TTL,
+    client: (_, context: ContentfulClient) =>
+      new FirestoreAdapter(
+        context.firestore,
         context.contentfulClientConfig.firestoreCacheCollectionName ||
-        CONTENTFUL_QUERY_CACHE_KEY,
-    }
-  )
+          CONTENTFUL_QUERY_CACHE_KEY
+      ),
+  })
   async query<Result, Variables extends OperationVariables>(
     query: TypedDocumentNode<Result, Variables>,
     variables: Variables

libs/shared/contentful/src/lib/contentful.manager.spec.ts

@@ -21,12 +21,15 @@ import { PurchaseWithDetailsOfferingContentUtil } from './queries/purchase-with-
 import { PurchaseWithDetailsOfferingContentByPlanIdsResultFactory } from './queries/purchase-with-details-offering-content/factories';
 import { StatsD } from 'hot-shots';
 
-jest.mock('@fxa/shared/db/type-cacheable', () => ({
-  FirestoreCacheable: () => {
+jest.mock('@type-cacheable/core', () => ({
+  Cacheable: () => {
     return (target: any, propertyKey: any, descriptor: any) => {
       return descriptor;
     };
   },
+}));
+
+jest.mock('@fxa/shared/db/type-cacheable', () => ({
   NetworkFirstStrategy: function () {},
 }));
 
@@ -36,7 +39,7 @@ describe('ContentfulManager', () => {
   let mockStatsd: StatsD;
 
   beforeEach(async () => {
-    mockClient = new ContentfulClient({} as any);
+    mockClient = new ContentfulClient({} as any, {} as any);
     mockStatsd = {
       timing: jest.fn().mockReturnValue({}),
     } as unknown as StatsD;

libs/shared/db/type-cacheable/src/index.ts

@@ -1,6 +1,5 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-export * from './lib/firestore-cacheable';
 export * from './lib/type-cachable-network-first';
 export * from './lib/type-cachable-firestore-adapter';

libs/shared/db/type-cacheable/src/lib/firestore-cacheable.ts

@@ -1,65 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-import { Inject } from '@nestjs/common';
-import { Cacheable, CacheOptions } from '@type-cacheable/core';
-import { AuthFirestore, FirestoreService } from '@fxa/shared/db/firestore';
-import { FirestoreAdapter } from './type-cachable-firestore-adapter';
-import { Firestore } from '@google-cloud/firestore';
-import Container from 'typedi';
-
-export interface FirestoreCacheableOptions {
-  collectionName: string | ((args: any[], context: any) => string);
-}
-
-export function FirestoreCacheable(
-  cacheableOptions: CacheOptions,
-  firestoreOptions: FirestoreCacheableOptions
-) {
-  // We try to fetch Firestore here with Nest DI, but need typedi compatibility where there's no Nest for now.
-  let injectFirestore: ReturnType<typeof Inject<Firestore>> | undefined;
-  try {
-    injectFirestore = Inject(FirestoreService);
-  } catch (e) {}
-
-  return (
-    target: any,
-    propertyKey: string,
-    propertyDescriptor: PropertyDescriptor
-  ) => {
-    injectFirestore?.(target, 'firestore');
-
-    const originalMethod = propertyDescriptor.value;
-
-    propertyDescriptor.value = async function (...args: any[]) {
-      // We try to fetch typedi Firestore if available in case this was loaded in a non-Nest environment
-      let typediFirestore: Firestore | undefined;
-      try {
-        typediFirestore = Container.get<Firestore>(AuthFirestore);
-      } catch (e) {}
-
-      const firestore = typediFirestore || (this as any).firestore;
-      if (!firestore) {
-        throw new Error('Could not load Firestore from Nest or typedi');
-      }
-
-      const collectionName =
-        typeof firestoreOptions.collectionName === 'string'
-          ? firestoreOptions.collectionName
-          : firestoreOptions.collectionName(args, this);
-
-      const newDescriptor = Cacheable({
-        ...cacheableOptions,
-        client: new FirestoreAdapter(firestore, collectionName),
-      })(target, propertyKey, {
-        ...propertyDescriptor,
-        value: originalMethod,
-      });
-
-      return await newDescriptor.value.apply(this, args);
-    };
-
-    return propertyDescriptor;
-  };
-}

package.json

@@ -76,6 +76,8 @@
     "@sentry/node": "^7.100.1",
     "@sentry/opentelemetry-node": "^7.100.1",
     "@swc/helpers": "~0.5.0",
+    "@type-cacheable/core": "^14.0.1",
+    "@type-cacheable/ioredis-adapter": "^10.0.4",
     "class-transformer": "^0.5.1",
     "class-validator": "^0.14.1",
     "diffparser": "^2.0.1",

packages/fxa-auth-server/bin/key_server.js

@@ -123,15 +123,19 @@ async function run(config) {
       config.contentful.environment &&
       config.contentful.firestoreCacheCollectionName
     ) {
-      const contentfulClient = new ContentfulClient({
-        cdnApiUri: config.contentful.cdnUrl,
-        graphqlApiUri: config.contentful.graphqlUrl,
-        graphqlApiKey: config.contentful.apiKey,
-        graphqlSpaceId: config.contentful.spaceId,
-        graphqlEnvironment: config.contentful.environment,
-        firestoreCacheCollectionName:
-          config.contentful.firestoreCacheCollectionName,
-      });
+      const firestore = Container.get(AuthFirestore);
+      const contentfulClient = new ContentfulClient(
+        {
+          cdnApiUri: config.contentful.cdnUrl,
+          graphqlApiUri: config.contentful.graphqlUrl,
+          graphqlApiKey: config.contentful.apiKey,
+          graphqlSpaceId: config.contentful.spaceId,
+          graphqlEnvironment: config.contentful.environment,
+          firestoreCacheCollectionName:
+            config.contentful.firestoreCacheCollectionName,
+        },
+        firestore
+      );
       const contentfulManager = new ContentfulManager(contentfulClient, statsd);
       Container.set(ContentfulManager, contentfulManager);
       const capabilityManager = new CapabilityManager(contentfulManager);

packages/fxa-auth-server/package.json

@@ -72,8 +72,6 @@
     "@hapi/hawk": "^8.0.0",
     "@hapi/hoek": "^11.0.2",
     "@mozilla/glean": "^4.0.0",
-    "@type-cacheable/core": "^13.0.0",
-    "@type-cacheable/ioredis-adapter": "^10.0.4",
     "@types/convict": "5.2.2",
     "@types/ejs": "^3.0.6",
     "@types/mjml": "^4.7.0",

packages/fxa-shared/package.json

@@ -250,8 +250,6 @@
   },
   "homepage": "https://github.com/mozilla/fxa/tree/main/packages/fxa-shared#readme",
   "devDependencies": {
-    "@type-cacheable/core": "^13.0.0",
-    "@type-cacheable/ioredis-adapter": "^10.0.4",
     "@types/accept-language-parser": "^1.5.3",
     "@types/chai": "^4.2.18",
     "@types/chance": "^1.1.2",

yarn.lock

@@ -20735,14 +20735,14 @@ __metadata:
   languageName: node
   linkType: hard
 
-"@type-cacheable/core@npm:^13.0.0":
-  version: 13.0.0
-  resolution: "@type-cacheable/core@npm:13.0.0"
+"@type-cacheable/core@npm:^14.0.1":
+  version: 14.0.1
+  resolution: "@type-cacheable/core@npm:14.0.1"
   dependencies:
     blueimp-md5: ^2.19.0
-    reflect-metadata: ^0.1.13
+    reflect-metadata: ^0.2.0
     serialize-javascript: ^6.0.0
-  checksum: 866e74608e94e0752ff9cad3bb758f17fea06168cdababa9741d67717b55912bc8ecd90f92f6b9023403384e6cd84a97ef4e73d608dd4db513be870e20206f9b
+  checksum: b325458266dc232b6b051f7cc78f902e6f1cc6ad8fab42c7ac79257279e5ef5d5955dfeb6ddf26447051120b12474f77275ef47fd167afa41ebab1b65b0a29c9
   languageName: node
   linkType: hard
 
@@ -30032,9 +30032,9 @@ __metadata:
   linkType: hard
 
 "compare-versions@npm:^4.0.0":
-  version: 4.1.3
-  resolution: "compare-versions@npm:4.1.3"
-  checksum: 54460756ab2d62f8a9d672db249b248fec7ca41c3e8ed242925e2f2257793ad3e83cecb2cdfd60b46a3aabc962a3a4cbf37a4b928c8f30517822d2bde937a3d1
+  version: 4.1.4
+  resolution: "compare-versions@npm:4.1.4"
+  checksum: c1617544b79c2f36a1d543c50efd0da1a994040294c8923218080bc0df46da83ca414e3378282e93cab073744995124946417d130d8987e8efb5d1a73c0c4ba6
   languageName: node
   linkType: hard
 
@@ -37223,8 +37223,6 @@ fsevents@~2.1.1:
     "@storybook/addon-toolbars": ^7.0.23
     "@storybook/html": ^7.5.2
     "@storybook/html-webpack5": ^7.6.13
-    "@type-cacheable/core": ^13.0.0
-    "@type-cacheable/ioredis-adapter": ^10.0.4
     "@types/async-retry": ^1
     "@types/babel__core": 7.1.14
     "@types/babel__preset-env": ^7
@@ -38120,8 +38118,6 @@ fsevents@~2.1.1:
   dependencies:
     "@fluent/langneg": ^0.7.0
     "@mozilla/glean": ^4.0.0
-    "@type-cacheable/core": ^13.0.0
-    "@type-cacheable/ioredis-adapter": ^10.0.4
     "@types/accept-language-parser": ^1.5.3
     "@types/chai": ^4.2.18
     "@types/chance": ^1.1.2
@@ -38266,6 +38262,8 @@ fsevents@~2.1.1:
     "@swc/core": ~1.3.51
     "@swc/helpers": ~0.5.0
     "@testing-library/react": ^14.2.1
+    "@type-cacheable/core": ^14.0.1
+    "@type-cacheable/ioredis-adapter": ^10.0.4
     "@types/babel__core": ^7
     "@types/jest": ^29.5.1
     "@types/mysql": ^2
@@ -56640,14 +56638,7 @@ fsevents@~2.1.1:
   languageName: node
   linkType: hard
 
-"reflect-metadata@npm:^0.1.13":
-  version: 0.1.13
-  resolution: "reflect-metadata@npm:0.1.13"
-  checksum: 798d379a7b6f6455501145419505c97dd11cbc23857a386add2b9ef15963ccf15a48d9d15507afe01d4cd74116df8a213247200bac00320bd7c11ddeaa5e8fb4
-  languageName: node
-  linkType: hard
-
-"reflect-metadata@npm:^0.2.1":
+"reflect-metadata@npm:^0.2.0, reflect-metadata@npm:^0.2.1":
   version: 0.2.1
   resolution: "reflect-metadata@npm:0.2.1"
   checksum: 772f552a544e04b999c1bf2c868225fef10032274e9d9e315bc3e7a687a504b8b115fa71966665b9619acfd323123a941f892b593250140da809330d41564181