From 45f37c98354ff6a4749f2857a0260863d44882fa Mon Sep 17 00:00:00 2001 From: Joe Porpeglia Date: Wed, 5 Jan 2022 18:16:25 -0500 Subject: [PATCH] Use CachedEntityLoader in techdocs router Signed-off-by: Joe Porpeglia --- .../src/service/router.test.ts | 43 ++++++++++--------- .../techdocs-backend/src/service/router.ts | 31 ++++++++----- .../src/service/standaloneServer.ts | 4 ++ 3 files changed, 48 insertions(+), 30 deletions(-) diff --git a/plugins/techdocs-backend/src/service/router.test.ts b/plugins/techdocs-backend/src/service/router.test.ts index 84ab0c6d88..fc5033ede8 100644 --- a/plugins/techdocs-backend/src/service/router.test.ts +++ b/plugins/techdocs-backend/src/service/router.test.ts @@ -17,9 +17,9 @@ import { errorHandler, getVoidLogger, + PluginCacheManager, PluginEndpointDiscovery, } from '@backstage/backend-common'; -import { CatalogClient } from '@backstage/catalog-client'; import { ConfigReader } from '@backstage/config'; import { NotModifiedError } from '@backstage/errors'; import { @@ -30,21 +30,23 @@ import { import express, { Response } from 'express'; import request from 'supertest'; import { DocsSynchronizer, DocsSynchronizerSyncOpts } from './DocsSynchronizer'; +import { CachedEntityLoader } from './CachedEntityLoader'; import { createEventStream, createHttpResponse, createRouter } from './router'; jest.mock('@backstage/catalog-client'); jest.mock('@backstage/config'); +jest.mock('./CachedEntityLoader'); jest.mock('./DocsSynchronizer'); const MockedConfigReader = ConfigReader as jest.MockedClass< typeof ConfigReader >; -const MockCatalogClient = CatalogClient as jest.MockedClass< - typeof CatalogClient ->; const MockDocsSynchronizer = DocsSynchronizer as jest.MockedClass< typeof DocsSynchronizer >; +const MockCachedEntityLoader = CachedEntityLoader as jest.MockedClass< + typeof CachedEntityLoader +>; describe('createRouter', () => { const entity = { @@ -82,6 +84,9 @@ describe('createRouter', () => { getBaseUrl: jest.fn(), getExternalBaseUrl: jest.fn(), }; + const cache: jest.Mocked = { + getClient: jest.fn(), + }; let app: express.Express; @@ -102,12 +107,14 @@ describe('createRouter', () => { config: new ConfigReader({}), logger: getVoidLogger(), discovery, + cache, }); const recommendedRouter = await createRouter({ publisher, config: new ConfigReader({}), logger: getVoidLogger(), discovery, + cache, }); app = express(); @@ -119,9 +126,7 @@ describe('createRouter', () => { describe('GET /sync/:namespace/:kind/:name', () => { describe('accept application/json', () => { it('should return not found if entity is not found', async () => { - MockCatalogClient.prototype.getEntityByName.mockResolvedValue( - undefined, - ); + MockCachedEntityLoader.prototype.load.mockResolvedValue(undefined); const response = await request(app) .get('/sync/default/Component/test') @@ -131,7 +136,7 @@ describe('createRouter', () => { }); it('should return not found if entity has no uid', async () => { - MockCatalogClient.prototype.getEntityByName.mockResolvedValue( + MockCachedEntityLoader.prototype.load.mockResolvedValue( entityWithoutMetadata, ); @@ -144,7 +149,7 @@ describe('createRouter', () => { it('should not check for an update without local builder', async () => { MockedConfigReader.prototype.getString.mockReturnValue('external'); - MockCatalogClient.prototype.getEntityByName.mockResolvedValue(entity); + MockCachedEntityLoader.prototype.load.mockResolvedValue(entity); const response = await request(app) .get('/sync/default/Component/test') @@ -155,7 +160,7 @@ describe('createRouter', () => { it('should error if missing builder', async () => { MockedConfigReader.prototype.getString.mockReturnValue('local'); - MockCatalogClient.prototype.getEntityByName.mockResolvedValue(entity); + MockCachedEntityLoader.prototype.load.mockResolvedValue(entity); const response = await request(app) .get('/recommended/sync/default/Component/test') @@ -171,7 +176,7 @@ describe('createRouter', () => { it('should execute synchronization', async () => { MockedConfigReader.prototype.getString.mockReturnValue('local'); - MockCatalogClient.prototype.getEntityByName.mockResolvedValue(entity); + MockCachedEntityLoader.prototype.load.mockResolvedValue(entity); MockDocsSynchronizer.prototype.doSync.mockImplementation( async ({ responseHandler }) => responseHandler.finish({ updated: true }), @@ -194,7 +199,7 @@ describe('createRouter', () => { it('should return on updated', async () => { MockedConfigReader.prototype.getString.mockReturnValue('local'); - MockCatalogClient.prototype.getEntityByName.mockResolvedValue(entity); + MockCachedEntityLoader.prototype.load.mockResolvedValue(entity); MockDocsSynchronizer.prototype.doSync.mockImplementation( async ({ responseHandler }) => { const { log, finish } = responseHandler; @@ -219,9 +224,7 @@ describe('createRouter', () => { describe('accept text/event-stream', () => { it('should return not found if entity is not found', async () => { - MockCatalogClient.prototype.getEntityByName.mockResolvedValue( - undefined, - ); + MockCachedEntityLoader.prototype.load.mockResolvedValue(undefined); const response = await request(app) .get('/sync/default/Component/test') @@ -232,7 +235,7 @@ describe('createRouter', () => { }); it('should return not found if entity has no uid', async () => { - MockCatalogClient.prototype.getEntityByName.mockResolvedValue( + MockCachedEntityLoader.prototype.load.mockResolvedValue( entityWithoutMetadata, ); @@ -246,7 +249,7 @@ describe('createRouter', () => { it('should not check for an update without local builder', async () => { MockedConfigReader.prototype.getString.mockReturnValue('external'); - MockCatalogClient.prototype.getEntityByName.mockResolvedValue(entity); + MockCachedEntityLoader.prototype.load.mockResolvedValue(entity); const response = await request(app) .get('/sync/default/Component/test') @@ -265,7 +268,7 @@ data: {"updated":false} it('should error if missing builder', async () => { MockedConfigReader.prototype.getString.mockReturnValue('local'); - MockCatalogClient.prototype.getEntityByName.mockResolvedValue(entity); + MockCachedEntityLoader.prototype.load.mockResolvedValue(entity); const response = await request(app) .get('/recommended/sync/default/Component/test') @@ -286,7 +289,7 @@ data: "Invalid configuration. 'techdocs.builder' was set to 'local' but no 'prep it('should execute synchronization', async () => { MockedConfigReader.prototype.getString.mockReturnValue('local'); - MockCatalogClient.prototype.getEntityByName.mockResolvedValue(entity); + MockCachedEntityLoader.prototype.load.mockResolvedValue(entity); MockDocsSynchronizer.prototype.doSync.mockImplementation( async ({ responseHandler }) => responseHandler.finish({ updated: true }), @@ -312,7 +315,7 @@ data: "Invalid configuration. 'techdocs.builder' was set to 'local' but no 'prep it('should return an event-stream', async () => { MockedConfigReader.prototype.getString.mockReturnValue('local'); - MockCatalogClient.prototype.getEntityByName.mockResolvedValue(entity); + MockCachedEntityLoader.prototype.load.mockResolvedValue(entity); MockDocsSynchronizer.prototype.doSync.mockImplementation( async ({ responseHandler }) => { const { log, finish } = responseHandler; diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index dc719f2860..789a6a771b 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -34,6 +34,8 @@ import { Logger } from 'winston'; import { ScmIntegrations } from '@backstage/integration'; import { DocsSynchronizer, DocsSynchronizerSyncOpts } from './DocsSynchronizer'; import { createCacheMiddleware, TechDocsCache } from '../cache'; +import { CachedEntityLoader } from './CachedEntityLoader'; +import { IdentityClient } from '@backstage/plugin-auth-backend'; /** * All of the required dependencies for running TechDocs in the "out-of-the-box" @@ -47,7 +49,7 @@ type OutOfTheBoxDeploymentOptions = { discovery: PluginEndpointDiscovery; database?: Knex; // TODO: Make database required when we're implementing database stuff. config: Config; - cache?: PluginCacheManager; + cache: PluginCacheManager; }; /** @@ -59,7 +61,7 @@ type RecommendedDeploymentOptions = { logger: Logger; discovery: PluginEndpointDiscovery; config: Config; - cache?: PluginCacheManager; + cache: PluginCacheManager; }; /** @@ -85,11 +87,23 @@ export async function createRouter( const router = Router(); const { publisher, config, logger, discovery } = options; const catalogClient = new CatalogClient({ discoveryApi: discovery }); + const identity = new IdentityClient({ + discovery, + issuer: await discovery.getBaseUrl('auth'), + }); + + // Entities are cached to optimize the /static/docs request path, which can be called many times + // when loading a single techdocs page. + const entityLoader = new CachedEntityLoader({ + catalog: catalogClient, + cache: options.cache.getClient(), + identity, + }); // Set up a cache client if configured. let cache: TechDocsCache | undefined; const defaultTtl = config.getOptionalNumber('techdocs.cache.ttl'); - if (options.cache && defaultTtl) { + if (defaultTtl) { const cacheClient = options.cache.getClient({ defaultTtl }); cache = TechDocsCache.fromConfig(config, { cache: cacheClient, logger }); } @@ -109,7 +123,7 @@ export async function createRouter( const token = getBearerToken(req.headers.authorization); // Verify that the related entity exists and the current user has permission to view it. - const entity = await catalogClient.getEntityByName(entityName, { token }); + const entity = await entityLoader.load(entityName, token); if (!entity) { throw new NotFoundError( @@ -141,7 +155,7 @@ export async function createRouter( const entityName = { kind, namespace, name }; const token = getBearerToken(req.headers.authorization); - const entity = await catalogClient.getEntityByName(entityName, { token }); + const entity = await entityLoader.load(entityName, token); if (!entity) { throw new NotFoundError( @@ -173,10 +187,7 @@ export async function createRouter( const { kind, namespace, name } = req.params; const token = getBearerToken(req.headers.authorization); - const entity = await catalogClient.getEntityByName( - { kind, namespace, name }, - { token }, - ); + const entity = await entityLoader.load({ kind, namespace, name }, token); if (!entity?.metadata?.uid) { throw new NotFoundError('Entity metadata UID missing'); @@ -237,7 +248,7 @@ export async function createRouter( const entityName = { kind, namespace, name }; const token = getBearerToken(req.headers.authorization); - const entity = await catalogClient.getEntityByName(entityName, { token }); + const entity = await entityLoader.load(entityName, token); if (!entity) { throw new NotFoundError( diff --git a/plugins/techdocs-backend/src/service/standaloneServer.ts b/plugins/techdocs-backend/src/service/standaloneServer.ts index e8981963f1..eeb85bfad0 100644 --- a/plugins/techdocs-backend/src/service/standaloneServer.ts +++ b/plugins/techdocs-backend/src/service/standaloneServer.ts @@ -15,6 +15,7 @@ */ import { + CacheManager, createServiceBuilder, DockerContainerRunner, SingleHostDiscovery, @@ -78,6 +79,8 @@ export async function startStandaloneServer( const publisher = await Publisher.fromConfig(config, { logger, discovery }); + const cache = CacheManager.fromConfig(config).forPlugin('techdocs'); + logger.debug('Starting application server...'); const router = await createRouter({ preparers, @@ -86,6 +89,7 @@ export async function startStandaloneServer( publisher, config, discovery, + cache, }); let service = createServiceBuilder(module) .setPort(options.port)