diff --git a/plugins/techdocs-backend/src/service/CachedEntityLoader.test.ts b/plugins/techdocs-backend/src/service/CachedEntityLoader.test.ts index 7f90df3714..104efcefcf 100644 --- a/plugins/techdocs-backend/src/service/CachedEntityLoader.test.ts +++ b/plugins/techdocs-backend/src/service/CachedEntityLoader.test.ts @@ -16,13 +16,11 @@ import { CachedEntityLoader } from './CachedEntityLoader'; import { CompoundEntityRef } from '@backstage/catalog-model'; -import { mockServices } from '@backstage/backend-test-utils'; +import { mockCredentials, mockServices } from '@backstage/backend-test-utils'; import { catalogServiceMock } from '@backstage/plugin-catalog-node/testUtils'; -import { BackstageCredentials } from '@backstage/backend-plugin-api'; describe('CachedEntityLoader', () => { const cache = mockServices.cache.mock(); - const auth = mockServices.auth.mock(); const entityName: CompoundEntityRef = { kind: 'component', @@ -39,21 +37,8 @@ describe('CachedEntityLoader', () => { }, }; - const userCredentials: BackstageCredentials = { - $$type: '@backstage/BackstageCredentials', - principal: { - type: 'user', - userEntityRef: 'user:default/test-user', - }, - }; - - const pluginCredentials: BackstageCredentials = { - $$type: '@backstage/BackstageCredentials', - principal: { - type: 'plugin', - subject: 'plugin:test-plugin', - }, - }; + const userCredentials = mockCredentials.user('user:default/test-user'); + const serviceCredentials = mockCredentials.service('plugin:test-plugin'); afterEach(() => { jest.resetAllMocks(); @@ -63,9 +48,8 @@ describe('CachedEntityLoader', () => { cache.get.mockResolvedValue(undefined); const catalog = catalogServiceMock({ entities: [entity] }); jest.spyOn(catalog, 'getEntityByRef'); - auth.isPrincipal.mockReturnValue(true); - const loader = new CachedEntityLoader({ auth, catalog, cache }); + const loader = new CachedEntityLoader({ catalog, cache }); const result = await loader.load(userCredentials, entityName); expect(result).toEqual(entity); @@ -73,7 +57,7 @@ describe('CachedEntityLoader', () => { credentials: userCredentials, }); expect(cache.set).toHaveBeenCalledWith( - 'catalog:component:default/test:user:default/test-user', + `catalog:component:default/test:${userCredentials}`, entity, { ttl: 5000 }, ); @@ -83,9 +67,8 @@ describe('CachedEntityLoader', () => { const catalog = catalogServiceMock(); jest.spyOn(catalog, 'getEntityByRef'); cache.get.mockResolvedValue(entity); - auth.isPrincipal.mockReturnValue(true); - const loader = new CachedEntityLoader({ auth, catalog, cache }); + const loader = new CachedEntityLoader({ catalog, cache }); const result = await loader.load(userCredentials, entityName); expect(result).toEqual(entity); @@ -95,33 +78,14 @@ describe('CachedEntityLoader', () => { it('does not cache missing entities', async () => { const catalog = catalogServiceMock({ entities: [] }); cache.get.mockResolvedValue(undefined); - auth.isPrincipal.mockReturnValue(true); - const loader = new CachedEntityLoader({ auth, catalog, cache }); + const loader = new CachedEntityLoader({ catalog, cache }); const result = await loader.load(userCredentials, entityName); expect(result).toBeUndefined(); expect(cache.set).not.toHaveBeenCalled(); }); - it('uses entity ref as cache key for service credentials', async () => { - const catalog = catalogServiceMock({ entities: [entity] }); - cache.get.mockResolvedValue(undefined); - auth.isPrincipal.mockReturnValueOnce(false).mockReturnValueOnce(true); - - const loader = new CachedEntityLoader({ auth, catalog, cache }); - const result = await loader.load(pluginCredentials, entityName); - - expect(result).toEqual(entity); - expect(cache.set).toHaveBeenCalledWith( - 'catalog:component:default/test:plugin:test-plugin', - entity, - { - ttl: 5000, - }, - ); - }); - it('calls the catalog if the cache read takes too long', async () => { cache.get.mockImplementation( () => @@ -130,80 +94,48 @@ describe('CachedEntityLoader', () => { }), ); const catalog = catalogServiceMock({ entities: [entity] }); - auth.isPrincipal.mockReturnValue(true); - const loader = new CachedEntityLoader({ auth, catalog, cache }); + const loader = new CachedEntityLoader({ catalog, cache }); const result = await loader.load(userCredentials, entityName); expect(result).toEqual(entity); }); - it('creates different cache keys for different users', async () => { + it('creates different cache keys for different credentials', async () => { const catalog = catalogServiceMock({ entities: [entity] }); cache.get.mockResolvedValue(undefined); - auth.isPrincipal.mockReturnValue(true); - const loader = new CachedEntityLoader({ auth, catalog, cache }); + const loader = new CachedEntityLoader({ catalog, cache }); - const anotherUserCredentials: BackstageCredentials = { - $$type: '@backstage/BackstageCredentials', - principal: { - type: 'user', - userEntityRef: 'user:default/another-user', - }, - }; + const anotherUserCredentials = mockCredentials.user( + 'user:default/another-user', + ); await loader.load(userCredentials, entityName); await loader.load(anotherUserCredentials, entityName); expect(cache.set).toHaveBeenCalledWith( - 'catalog:component:default/test:user:default/test-user', + `catalog:component:default/test:${userCredentials}`, entity, { ttl: 5000 }, ); expect(cache.set).toHaveBeenCalledWith( - 'catalog:component:default/test:user:default/another-user', + `catalog:component:default/test:${anotherUserCredentials}`, entity, { ttl: 5000 }, ); }); - it('creates cache key with service subject for service credentials', async () => { + it('uses service credentials as cache key for service credentials', async () => { const catalog = catalogServiceMock({ entities: [entity] }); cache.get.mockResolvedValue(undefined); - auth.isPrincipal.mockReturnValueOnce(false).mockReturnValueOnce(true); - const loader = new CachedEntityLoader({ auth, catalog, cache }); - const result = await loader.load(pluginCredentials, entityName); + const loader = new CachedEntityLoader({ catalog, cache }); + const result = await loader.load(serviceCredentials, entityName); expect(result).toEqual(entity); expect(cache.set).toHaveBeenCalledWith( - 'catalog:component:default/test:plugin:test-plugin', - entity, - { ttl: 5000 }, - ); - expect(auth.isPrincipal).toHaveBeenCalledWith(pluginCredentials, 'user'); - expect(auth.isPrincipal).toHaveBeenCalledWith(pluginCredentials, 'service'); - }); - - it('handles credentials that are neither user nor service', async () => { - const catalog = catalogServiceMock({ entities: [entity] }); - cache.get.mockResolvedValue(undefined); - auth.isPrincipal.mockReturnValue(false); - - const unknownCredentials: BackstageCredentials = { - $$type: '@backstage/BackstageCredentials', - principal: { - type: 'unknown' as any, - }, - }; - - const loader = new CachedEntityLoader({ auth, catalog, cache }); - const result = await loader.load(unknownCredentials, entityName); - - expect(result).toEqual(entity); - expect(cache.set).toHaveBeenCalledWith( - 'catalog:component:default/test', + `catalog:component:default/test:${serviceCredentials}`, entity, { ttl: 5000 }, ); diff --git a/plugins/techdocs-backend/src/service/CachedEntityLoader.ts b/plugins/techdocs-backend/src/service/CachedEntityLoader.ts index 8e19bb2411..da3ad513e8 100644 --- a/plugins/techdocs-backend/src/service/CachedEntityLoader.ts +++ b/plugins/techdocs-backend/src/service/CachedEntityLoader.ts @@ -15,7 +15,6 @@ */ import { - AuthService, BackstageCredentials, CacheService, } from '@backstage/backend-plugin-api'; @@ -27,19 +26,16 @@ import { import { CatalogService } from '@backstage/plugin-catalog-node'; export type CachedEntityLoaderOptions = { - auth: AuthService; catalog: CatalogService; cache: CacheService; }; export class CachedEntityLoader { - private readonly auth: AuthService; private readonly catalog: CatalogService; private readonly cache: CacheService; private readonly readTimeout = 1000; - constructor({ auth, catalog, cache }: CachedEntityLoaderOptions) { - this.auth = auth; + constructor({ catalog, cache }: CachedEntityLoaderOptions) { this.catalog = catalog; this.cache = cache; } @@ -77,14 +73,10 @@ export class CachedEntityLoader { entityName: CompoundEntityRef, credentials: BackstageCredentials, ): string { - const key = ['catalog', stringifyEntityRef(entityName)]; - - if (this.auth.isPrincipal(credentials, 'user')) { - key.push(credentials.principal.userEntityRef); - } else if (this.auth.isPrincipal(credentials, 'service')) { - key.push(credentials.principal.subject); - } - - return key.join(':'); + return [ + 'catalog', + stringifyEntityRef(entityName), + String(credentials), // these have a well defined toString method + ].join(':'); } } diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index 53e9d2003a..30a279ea27 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -124,7 +124,6 @@ export async function createRouter( // 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({ - auth, catalog, cache: options.cache, });