From 11c48737bc13df9c6c18e24a1b0a9019f234226e Mon Sep 17 00:00:00 2001 From: Julian Poyourow Date: Tue, 27 Feb 2024 12:17:56 -0800 Subject: [PATCH] feat(type-cacheable): update to load firestore adapter from lambda --- .../src/lib/stripe-mapper.service.spec.ts | 2 +- .../src/lib/contentful.client.spec.ts | 27 +++++--- .../contentful/src/lib/contentful.client.ts | 37 ++++++----- .../src/lib/contentful.manager.spec.ts | 9 ++- libs/shared/db/type-cacheable/src/index.ts | 1 - .../src/lib/firestore-cacheable.ts | 65 ------------------- package.json | 2 + packages/fxa-auth-server/bin/key_server.js | 22 ++++--- packages/fxa-auth-server/package.json | 2 - packages/fxa-shared/package.json | 2 - yarn.lock | 31 ++++----- 11 files changed, 71 insertions(+), 129 deletions(-) delete mode 100644 libs/shared/db/type-cacheable/src/lib/firestore-cacheable.ts diff --git a/libs/payments/legacy/src/lib/stripe-mapper.service.spec.ts b/libs/payments/legacy/src/lib/stripe-mapper.service.spec.ts index b8bfd11beeb..f6fe9b6872d 100644 --- a/libs/payments/legacy/src/lib/stripe-mapper.service.spec.ts +++ b/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) diff --git a/libs/shared/contentful/src/lib/contentful.client.spec.ts b/libs/shared/contentful/src/lib/contentful.client.spec.ts index 24ee46c832d..9da4e90341a 100644 --- a/libs/shared/contentful/src/lib/contentful.client.spec.ts +++ b/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); }); diff --git a/libs/shared/contentful/src/lib/contentful.client.ts b/libs/shared/contentful/src/lib/contentful.client.ts index 3ebd26aa635..308dff53bfe 100644 --- a/libs/shared/contentful/src/lib/contentful.client.ts +++ b/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 = {}; - 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( query: TypedDocumentNode, variables: Variables diff --git a/libs/shared/contentful/src/lib/contentful.manager.spec.ts b/libs/shared/contentful/src/lib/contentful.manager.spec.ts index 23d9d177418..08d8630ce9f 100644 --- a/libs/shared/contentful/src/lib/contentful.manager.spec.ts +++ b/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; diff --git a/libs/shared/db/type-cacheable/src/index.ts b/libs/shared/db/type-cacheable/src/index.ts index 648a61f0058..bc195f8bf18 100644 --- a/libs/shared/db/type-cacheable/src/index.ts +++ b/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'; diff --git a/libs/shared/db/type-cacheable/src/lib/firestore-cacheable.ts b/libs/shared/db/type-cacheable/src/lib/firestore-cacheable.ts deleted file mode 100644 index 8bc65940a6a..00000000000 --- a/libs/shared/db/type-cacheable/src/lib/firestore-cacheable.ts +++ /dev/null @@ -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> | 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(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; - }; -} diff --git a/package.json b/package.json index 37028518278..242a43e5fb1 100644 --- a/package.json +++ b/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", diff --git a/packages/fxa-auth-server/bin/key_server.js b/packages/fxa-auth-server/bin/key_server.js index 13098fa3ea6..1e236e5f819 100755 --- a/packages/fxa-auth-server/bin/key_server.js +++ b/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); diff --git a/packages/fxa-auth-server/package.json b/packages/fxa-auth-server/package.json index 0a39c178d79..c462f5d1f28 100644 --- a/packages/fxa-auth-server/package.json +++ b/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", diff --git a/packages/fxa-shared/package.json b/packages/fxa-shared/package.json index afce41f03c4..6e5cbb10c91 100644 --- a/packages/fxa-shared/package.json +++ b/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", diff --git a/yarn.lock b/yarn.lock index 8fb4a096786..d41b59d583e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -20643,14 +20643,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 @@ -29940,9 +29940,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 @@ -36987,8 +36987,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 @@ -37884,8 +37882,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 @@ -38030,6 +38026,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 @@ -56404,14 +56402,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