From 02bd2cb19c2faa755518b8eb42fdcf5d8c1c29e9 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 12 Dec 2024 12:00:33 +0100 Subject: [PATCH 01/13] catalog-backend: avoid JSON parsing when possible Signed-off-by: Patrik Oldsberg --- .changeset/curly-teachers-marry.md | 5 ++ .../config/vocabularies/Backstage/accept.txt | 1 + plugins/catalog-backend/src/catalog/types.ts | 20 ++++- .../service/AuthorizedEntitiesCatalog.test.ts | 2 +- .../src/service/AuthorizedEntitiesCatalog.ts | 11 ++- .../src/service/CatalogBuilder.ts | 12 ++- .../service/DefaultEntitiesCatalog.test.ts | 55 ++++++++---- .../src/service/DefaultEntitiesCatalog.ts | 48 +++++----- .../src/service/createRouter.test.ts | 34 +++---- .../src/service/createRouter.ts | 36 +++++--- .../src/service/response/index.ts | 22 +++++ .../src/service/response/process.ts | 62 +++++++++++++ .../src/service/response/write.ts | 89 +++++++++++++++++++ plugins/catalog-backend/src/service/util.ts | 56 +++++++----- .../src/tests/integration.test.ts | 8 +- 15 files changed, 361 insertions(+), 100 deletions(-) create mode 100644 .changeset/curly-teachers-marry.md create mode 100644 plugins/catalog-backend/src/service/response/index.ts create mode 100644 plugins/catalog-backend/src/service/response/process.ts create mode 100644 plugins/catalog-backend/src/service/response/write.ts diff --git a/.changeset/curly-teachers-marry.md b/.changeset/curly-teachers-marry.md new file mode 100644 index 0000000000..21f41da291 --- /dev/null +++ b/.changeset/curly-teachers-marry.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend': minor +--- + +Added a new `catalog.enableRawJsonResponse` configuration option that avoids JSON deserialization and serialization if possible when reading entities. This can significantly improve the overall performance of the catalog, but it removes the backwards compatibility processing that ensures that both `entity.relation[].target` and `entity.relation[].targetRef` are present in returned entities. diff --git a/.github/vale/config/vocabularies/Backstage/accept.txt b/.github/vale/config/vocabularies/Backstage/accept.txt index e0389f0adb..fc0fd5021e 100644 --- a/.github/vale/config/vocabularies/Backstage/accept.txt +++ b/.github/vale/config/vocabularies/Backstage/accept.txt @@ -97,6 +97,7 @@ deliverables denormalized dependabot deps +deserialization destructured destructuring Deutsche diff --git a/plugins/catalog-backend/src/catalog/types.ts b/plugins/catalog-backend/src/catalog/types.ts index b3e52fb480..d00c38f536 100644 --- a/plugins/catalog-backend/src/catalog/types.ts +++ b/plugins/catalog-backend/src/catalog/types.ts @@ -52,8 +52,22 @@ export type EntitiesRequest = { credentials: BackstageCredentials; }; +/** + * Encapsulates either a deserialized or serialized entities to be sent in a response. + * @internal + */ +export type EntitiesResponseItems = + | { + type: 'objects'; + entities: (Entity | null)[]; + } + | { + type: 'raw'; + entities: (string | null)[]; + }; + export type EntitiesResponse = { - entities: Entity[]; + entities: EntitiesResponseItems; pageInfo: PageInfo; }; @@ -86,7 +100,7 @@ export interface EntitiesBatchResponse { * The list of entities, in the same order as the refs in the request. Entries * that are null signify that no entity existed with that ref. */ - items: Array; + items: EntitiesResponseItems; } export type EntityAncestryResponse = { @@ -224,7 +238,7 @@ export interface QueryEntitiesResponse { /** * The entities for the current pagination request */ - items: Entity[]; + items: EntitiesResponseItems; pageInfo: { /** diff --git a/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.test.ts b/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.test.ts index 3e2dee7ae0..2207802859 100644 --- a/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.test.ts +++ b/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.test.ts @@ -226,7 +226,7 @@ describe('AuthorizedEntitiesCatalog', () => { ]; fakeCatalog.queryEntities.mockResolvedValue({ - items: entities, + items: { type: 'objects', entities }, pageInfo: { nextCursor: { isPrevious: false, diff --git a/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.ts b/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.ts index 5358e90825..225c87cae9 100644 --- a/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.ts @@ -60,7 +60,7 @@ export class AuthorizedEntitiesCatalog implements EntitiesCatalog { if (authorizeDecision.result === AuthorizeResult.DENY) { return { - entities: [], + entities: { type: 'objects', entities: [] }, pageInfo: { hasNextPage: false }, }; } @@ -92,7 +92,10 @@ export class AuthorizedEntitiesCatalog implements EntitiesCatalog { if (authorizeDecision.result === AuthorizeResult.DENY) { return { - items: new Array(request.entityRefs.length).fill(null), + items: { + type: 'objects', + entities: new Array(request.entityRefs.length).fill(null), + }, }; } @@ -123,7 +126,7 @@ export class AuthorizedEntitiesCatalog implements EntitiesCatalog { if (authorizeDecision.result === AuthorizeResult.DENY) { return { - items: [], + items: { type: 'objects', entities: [] }, pageInfo: {}, totalItems: 0, }; @@ -208,7 +211,7 @@ export class AuthorizedEntitiesCatalog implements EntitiesCatalog { allOf: [permissionFilter, basicEntityFilter({ 'metadata.uid': uid })], }, }); - if (entities.length === 0) { + if (entities.entities.length === 0) { throw new NotAllowedError(); } } diff --git a/plugins/catalog-backend/src/service/CatalogBuilder.ts b/plugins/catalog-backend/src/service/CatalogBuilder.ts index 31ec40b887..fd052bf474 100644 --- a/plugins/catalog-backend/src/service/CatalogBuilder.ts +++ b/plugins/catalog-backend/src/service/CatalogBuilder.ts @@ -115,6 +115,7 @@ import { UrlReaderService, SchedulerService, } from '@backstage/backend-plugin-api'; +import { entitiesResponseToObjects } from './response'; /** * This is a duplicate of the alpha `CatalogPermissionRule` type, for use in the stable API. @@ -485,6 +486,10 @@ export class CatalogBuilder { discovery, }); + const enableRawJson = config.getOptionalBoolean( + 'catalog.enableRawJsonResponses', + ); + const policy = this.buildEntityPolicy(); const processors = this.buildProcessors(); const parser = this.parser || defaultEntityDataParser; @@ -521,6 +526,7 @@ export class CatalogBuilder { database: dbClient, logger, stitcher, + enableRawJson, }); let permissionsService: PermissionsService; @@ -566,7 +572,10 @@ export class CatalogBuilder { }, }); - const entitiesByRef = keyBy(entities, stringifyEntityRef); + const entitiesByRef = keyBy( + entitiesResponseToObjects(entities), + stringifyEntityRef, + ); return resourceRefs.map( resourceRef => @@ -629,6 +638,7 @@ export class CatalogBuilder { auth, httpAuth, permissionsService, + enableRawJson, }); await connectEntityProviders(providerDatabase, entityProviders); diff --git a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts index bb2ba3d587..379c861ba1 100644 --- a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts +++ b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts @@ -38,6 +38,7 @@ import { Stitcher } from '../stitching/types'; import { DefaultEntitiesCatalog } from './DefaultEntitiesCatalog'; import { EntitiesRequest } from '../catalog/types'; import { buildEntitySearch } from '../database/operations/stitcher/buildEntitySearch'; +import { entitiesResponseToObjects } from './response'; jest.setTimeout(60_000); @@ -310,10 +311,11 @@ describe('DefaultEntitiesCatalog', () => { const testFilter = { key: 'spec.test', }; - const { entities } = await catalog.entities({ + const res = await catalog.entities({ filter: testFilter, credentials: mockCredentials.none(), }); + const entities = entitiesResponseToObjects(res.entities); expect(entities.length).toBe(1); expect(entities[0]).toEqual(entity2); @@ -351,10 +353,11 @@ describe('DefaultEntitiesCatalog', () => { key: 'spec.test', }, }; - const { entities } = await catalog.entities({ + const res = await catalog.entities({ filter: testFilter, credentials: mockCredentials.none(), }); + const entities = entitiesResponseToObjects(res.entities); expect(entities.length).toBe(1); expect(entities[0]).toEqual(entity1); @@ -416,7 +419,7 @@ describe('DefaultEntitiesCatalog', () => { values: ['red'], }, }; - const { entities } = await catalog.entities({ + const res = await catalog.entities({ filter: { allOf: [ testFilter1, @@ -427,6 +430,7 @@ describe('DefaultEntitiesCatalog', () => { }, credentials: mockCredentials.none(), }); + const entities = entitiesResponseToObjects(res.entities); expect(entities.length).toBe(2); expect(entities).toContainEqual(entity2); @@ -465,7 +469,7 @@ describe('DefaultEntitiesCatalog', () => { const testFilter2 = { key: 'metadata.desc', }; - const { entities } = await catalog.entities({ + const res = await catalog.entities({ filter: { not: { allOf: [testFilter1, testFilter2], @@ -474,6 +478,7 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }); + const entities = entitiesResponseToObjects(res.entities); expect(entities.length).toBe(1); expect(entities).toContainEqual(entity1); @@ -509,10 +514,11 @@ describe('DefaultEntitiesCatalog', () => { key: 'kind', values: [], }; - const { entities } = await catalog.entities({ + const res = await catalog.entities({ filter: testFilter, credentials: mockCredentials.none(), }); + const entities = entitiesResponseToObjects(res.entities); expect(entities.length).toBe(0); }, @@ -553,10 +559,11 @@ describe('DefaultEntitiesCatalog', () => { stitcher, }); - const { entities } = await catalog.entities(); + const res = await catalog.entities(); + const entities = entitiesResponseToObjects(res.entities); expect( - entities.find(e => e.metadata.name === 'one')!.relations, + entities.find(e => e?.metadata.name === 'one')!.relations, ).toEqual([ { type: 'r', @@ -565,7 +572,7 @@ describe('DefaultEntitiesCatalog', () => { }, ]); expect( - entities.find(e => e.metadata.name === 'two')!.relations, + entities.find(e => e?.metadata.name === 'two')!.relations, ).toEqual([ { type: 'r', @@ -615,7 +622,9 @@ describe('DefaultEntitiesCatalog', () => { return catalog .entities({ ...request, credentials: mockCredentials.none() }) .then(response => - response.entities.map(e => e.metadata.name).toSorted(), + entitiesResponseToObjects(response.entities) + .map(e => e!.metadata.name) + .toSorted(), ); } @@ -678,7 +687,11 @@ describe('DefaultEntitiesCatalog', () => { ): Promise { return catalog .entities({ ...request, credentials: mockCredentials.none() }) - .then(response => response.entities.map(e => e.metadata.name)); + .then(response => + entitiesResponseToObjects(response.entities).map( + e => e!.metadata.name, + ), + ); } await expect( @@ -764,7 +777,7 @@ describe('DefaultEntitiesCatalog', () => { stitcher, }); - const { items } = await catalog.entitiesBatch({ + const res = await catalog.entitiesBatch({ entityRefs: [ 'k:default/two', 'k:default/one', @@ -775,6 +788,7 @@ describe('DefaultEntitiesCatalog', () => { ], credentials: mockCredentials.none(), }); + const items = entitiesResponseToObjects(res.items); expect(items.map(e => e && stringifyEntityRef(e))).toEqual([ 'k:default/two', @@ -819,11 +833,12 @@ describe('DefaultEntitiesCatalog', () => { stitcher, }); - const { items } = await catalog.entitiesBatch({ + const res = await catalog.entitiesBatch({ entityRefs: ['k:default/two', 'k:default/one'], filter: { key: 'spec.owner', values: ['me'] }, credentials: mockCredentials.none(), }); + const items = entitiesResponseToObjects(res.items); expect(items.map(e => e && stringifyEntityRef(e))).toEqual([ 'k:default/two', @@ -1830,7 +1845,9 @@ describe('DefaultEntitiesCatalog', () => { orderFields: [{ field: 'metadata.title', order: 'asc' }], credentials: mockCredentials.none(), }) - .then(r => r.items.map(e => e.metadata.name)), + .then(r => + entitiesResponseToObjects(r.items).map(e => e!.metadata.name), + ), ).resolves.toEqual(['CC', 'BB', 'AA']); // 'AA' has no title, ends up last await expect( @@ -1839,7 +1856,9 @@ describe('DefaultEntitiesCatalog', () => { orderFields: [{ field: 'metadata.title', order: 'desc' }], credentials: mockCredentials.none(), }) - .then(r => r.items.map(e => e.metadata.name)), + .then(r => + entitiesResponseToObjects(r.items).map(e => e!.metadata.name), + ), ).resolves.toEqual(['BB', 'CC', 'AA']); // 'AA' has no title, ends up last }, ); @@ -1869,7 +1888,9 @@ describe('DefaultEntitiesCatalog', () => { limit: 10, credentials: mockCredentials.none(), }) - .then(r => r.items.map(e => e.metadata.name)), + .then(r => + entitiesResponseToObjects(r.items).map(e => e!.metadata.name), + ), ).resolves.toEqual(['AA', 'BB']); // simulate a situation where stitching is not yet complete @@ -1884,7 +1905,9 @@ describe('DefaultEntitiesCatalog', () => { limit: 10, credentials: mockCredentials.none(), }) - .then(r => r.items.map(e => e.metadata.name)), + .then(r => + entitiesResponseToObjects(r.items).map(e => e!.metadata.name), + ), ).resolves.toEqual(['BB']); }, ); diff --git a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts index cddc95f496..ee9b46ce55 100644 --- a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts @@ -45,13 +45,14 @@ import { import { Stitcher } from '../stitching/types'; import { - expandLegacyCompoundRelationRefsInResponse, + expandLegacyCompoundRelationsInEntity, isQueryEntitiesCursorRequest, isQueryEntitiesInitialRequest, } from './util'; import { EntityFilter } from '@backstage/plugin-catalog-node'; import { LoggerService } from '@backstage/backend-plugin-api'; import { applyEntityFilterToQuery } from './request/applyEntityFilterToQuery'; +import { processRawEntitiesResult } from './response'; const DEFAULT_LIMIT = 200; @@ -104,15 +105,18 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { private readonly database: Knex; private readonly logger: LoggerService; private readonly stitcher: Stitcher; + private readonly enableRawJson: boolean; constructor(options: { database: Knex; logger: LoggerService; stitcher: Stitcher; + enableRawJson?: boolean; }) { this.database = options.database; this.logger = options.logger; this.stitcher = options.stitcher; + this.enableRawJson = Boolean(options.enableRawJson); } async entities(request?: EntitiesRequest): Promise { @@ -190,16 +194,19 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { }; } - let entities: Entity[] = rows.map(e => JSON.parse(e.final_entity!)); - - if (request?.fields) { - entities = entities.map(e => request.fields!(e)); - } - - expandLegacyCompoundRelationRefsInResponse(entities); - return { - entities, + entities: processRawEntitiesResult( + rows.map(r => r.final_entity!), + this.enableRawJson + ? request?.fields + : e => { + expandLegacyCompoundRelationsInEntity(e); + if (request?.fields) { + return request.fields(e); + } + return e; + }, + ), pageInfo, }; } @@ -207,7 +214,7 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { async entitiesBatch( request: EntitiesBatchRequest, ): Promise { - const lookup = new Map(); + const lookup = new Map(); for (const chunk of lodashChunk(request.entityRefs, 200)) { let query = this.database('final_entities') @@ -227,17 +234,13 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { } for (const row of await query) { - lookup.set(row.entityRef, row.entity ? JSON.parse(row.entity) : null); + lookup.set(row.entityRef, row.entity ? row.entity : null); } } - let items = request.entityRefs.map(ref => lookup.get(ref) ?? null); + const items = request.entityRefs.map(ref => lookup.get(ref) ?? null); - if (request.fields) { - items = items.map(e => e && request.fields!(e)); - } - - return { items }; + return { items: processRawEntitiesResult(items, request.fields) }; } async queryEntities( @@ -505,12 +508,11 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { } : undefined; - const items = rows - .map(e => JSON.parse(e.final_entity!)) - .map(e => (request.fields ? request.fields(e) : e)); - return { - items, + items: processRawEntitiesResult( + rows.map(r => r.final_entity!), + request.fields, + ), pageInfo: { ...(!!prevCursor && { prevCursor }), ...(!!nextCursor && { nextCursor }), diff --git a/plugins/catalog-backend/src/service/createRouter.test.ts b/plugins/catalog-backend/src/service/createRouter.test.ts index e728017f0b..8efb6543a3 100644 --- a/plugins/catalog-backend/src/service/createRouter.test.ts +++ b/plugins/catalog-backend/src/service/createRouter.test.ts @@ -136,7 +136,7 @@ describe('createRouter readonly disabled', () => { ]; entitiesCatalog.queryEntities.mockResolvedValueOnce({ - items: [entities[0]], + items: { type: 'objects', entities: [entities[0]] }, pageInfo: {}, totalItems: 1, }); @@ -149,7 +149,7 @@ describe('createRouter readonly disabled', () => { it('parses single and multiple request parameters and passes them down', async () => { entitiesCatalog.queryEntities.mockResolvedValueOnce({ - items: [], + items: { type: 'objects', entities: [] }, pageInfo: {}, totalItems: 0, }); @@ -185,7 +185,7 @@ describe('createRouter readonly disabled', () => { ]; entitiesCatalog.queryEntities.mockResolvedValueOnce({ - items, + items: { type: 'objects', entities: items }, totalItems: 100, pageInfo: { nextCursor: mockCursor() }, }); @@ -203,7 +203,7 @@ describe('createRouter readonly disabled', () => { it('parses initial request', async () => { entitiesCatalog.queryEntities.mockResolvedValueOnce({ - items: [], + items: { type: 'objects', entities: [] }, pageInfo: {}, totalItems: 0, }); @@ -239,7 +239,7 @@ describe('createRouter readonly disabled', () => { it('parses encoded params request', async () => { entitiesCatalog.queryEntities.mockResolvedValueOnce({ - items: [], + items: { type: 'objects', entities: [] }, pageInfo: {}, totalItems: 0, }); @@ -283,7 +283,7 @@ describe('createRouter readonly disabled', () => { ]; entitiesCatalog.queryEntities.mockResolvedValueOnce({ - items, + items: { type: 'objects', entities: items }, totalItems: 100, pageInfo: { nextCursor: mockCursor() }, }); @@ -312,7 +312,7 @@ describe('createRouter readonly disabled', () => { ]; entitiesCatalog.queryEntities.mockResolvedValueOnce({ - items, + items: { type: 'objects', entities: items }, totalItems: 100, pageInfo: { nextCursor: mockCursor({ fullTextFilter: { term: 'mySearch' } }), @@ -354,7 +354,7 @@ describe('createRouter readonly disabled', () => { ]; entitiesCatalog.queryEntities.mockResolvedValueOnce({ - items, + items: { type: 'objects', entities: items }, totalItems: 100, pageInfo: { nextCursor: mockCursor() }, }); @@ -379,7 +379,7 @@ describe('createRouter readonly disabled', () => { ]; entitiesCatalog.queryEntities.mockResolvedValueOnce({ - items, + items: { type: 'objects', entities: items }, totalItems: 100, pageInfo: { nextCursor: mockCursor() }, }); @@ -402,7 +402,7 @@ describe('createRouter readonly disabled', () => { }, }; entitiesCatalog.entities.mockResolvedValue({ - entities: [entity], + entities: { type: 'objects', entities: [entity] }, pageInfo: { hasNextPage: false }, }); @@ -419,7 +419,7 @@ describe('createRouter readonly disabled', () => { it('responds with a 404 for missing entities', async () => { entitiesCatalog.entities.mockResolvedValue({ - entities: [], + entities: { type: 'objects', entities: [] }, pageInfo: { hasNextPage: false }, }); @@ -446,7 +446,7 @@ describe('createRouter readonly disabled', () => { }, }; entitiesCatalog.entitiesBatch.mockResolvedValue({ - items: [entity], + items: { type: 'objects', entities: [entity] }, }); const response = await request(app).get('/entities/by-name/k/ns/n'); @@ -462,7 +462,7 @@ describe('createRouter readonly disabled', () => { it('responds with a 404 for missing entities', async () => { entitiesCatalog.entitiesBatch.mockResolvedValue({ - items: [null], + items: { type: 'objects', entities: [null] }, }); const response = await request(app).get('/entities/by-name/b/d/c'); @@ -533,7 +533,9 @@ describe('createRouter readonly disabled', () => { }, }; const entityRef = stringifyEntityRef(entity); - entitiesCatalog.entitiesBatch.mockResolvedValue({ items: [entity] }); + entitiesCatalog.entitiesBatch.mockResolvedValue({ + items: { type: 'objects', entities: [entity] }, + }); const response = await request(app) .post('/entities/by-refs?filter=kind=Component') .set('Content-Type', 'application/json') @@ -908,7 +910,7 @@ describe('createRouter readonly enabled', () => { ]; entitiesCatalog.queryEntities.mockResolvedValueOnce({ - items: [entities[0]], + items: { type: 'objects', entities: [entities[0]] }, pageInfo: {}, totalItems: 1, }); @@ -1128,7 +1130,7 @@ describe('NextRouter permissioning', () => { }, }; entitiesCatalog.entities.mockResolvedValueOnce({ - entities: [spideySense], + entities: { type: 'objects', entities: [spideySense] }, pageInfo: { hasNextPage: false }, }); diff --git a/plugins/catalog-backend/src/service/createRouter.ts b/plugins/catalog-backend/src/service/createRouter.ts index d70419c565..b670253089 100644 --- a/plugins/catalog-backend/src/service/createRouter.ts +++ b/plugins/catalog-backend/src/service/createRouter.ts @@ -44,7 +44,7 @@ import { createEntityArrayJsonStream, disallowReadonlyMode, encodeCursor, - expandLegacyCompoundRelationRefsInResponse, + expandLegacyCompoundRelationsInEntity, locationInput, validateRequestBody, } from './util'; @@ -60,6 +60,11 @@ import { import { LocationAnalyzer } from '@backstage/plugin-catalog-node'; import { AuthorizedValidationService } from './AuthorizedValidationService'; import { DeferredPromise, createDeferred } from '@backstage/types'; +import { + processEntitiesResponseItems, + writeEntitiesResponse, + writeSingleEntityResponse, +} from './response'; /** * Options used by {@link createRouter}. @@ -80,6 +85,7 @@ export interface RouterOptions { auth: AuthService; httpAuth: HttpAuthService; permissionsService: PermissionsService; + enableRawJson?: boolean; } /** @@ -107,6 +113,7 @@ export async function createRouter( permissionsService, auth, httpAuth, + enableRawJson = false, } = options; const readonlyEnabled = @@ -164,7 +171,7 @@ export async function createRouter( res.setHeader('link', `<${url.pathname}${url.search}>; rel="next"`); } - res.json(entities); + writeEntitiesResponse(res, entities); return; } @@ -203,12 +210,17 @@ export async function createRouter( : { credentials, fields, limit, cursor }, ); - if (result.items.length) { + if (result.items.entities.length) { await locks?.writeLock; signal.throwIfAborted(); - expandLegacyCompoundRelationRefsInResponse(result.items); + if (!enableRawJson) { + processEntitiesResponseItems( + result.items, + expandLegacyCompoundRelationsInEntity, + ); + } if (!responseStream.send(result.items)) { // The kernel buffer is full. Create the lock but do not await it // yet - we can better spend our time going to the next round of @@ -241,8 +253,8 @@ export async function createRouter( credentials: await httpAuth.credentials(req), }); - res.json({ - items, + writeEntitiesResponse(res, items, entities => ({ + items: entities, totalItems, pageInfo: { ...(pageInfo.nextCursor && { @@ -252,7 +264,7 @@ export async function createRouter( prevCursor: encodeCursor(pageInfo.prevCursor), }), }, - }); + })); }) .get('/entities/by-uid/:uid', async (req, res) => { const { uid } = req.params; @@ -260,10 +272,9 @@ export async function createRouter( filter: basicEntityFilter({ 'metadata.uid': uid }), credentials: await httpAuth.credentials(req), }); - if (!entities.length) { + if (!writeSingleEntityResponse(res, entities)) { throw new NotFoundError(`No entity with uid ${uid}`); } - res.status(200).json(entities[0]); }) .delete('/entities/by-uid/:uid', async (req, res) => { const { uid } = req.params; @@ -278,12 +289,11 @@ export async function createRouter( entityRefs: [stringifyEntityRef({ kind, namespace, name })], credentials: await httpAuth.credentials(req), }); - if (!items[0]) { + if (!writeSingleEntityResponse(res, items)) { throw new NotFoundError( `No entity named '${name}' found, with kind '${kind}' in namespace '${namespace}'`, ); } - res.status(200).json(items[0]); }) .get( '/entities/by-name/:kind/:namespace/:name/ancestry', @@ -298,13 +308,13 @@ export async function createRouter( ) .post('/entities/by-refs', async (req, res) => { const request = entitiesBatchRequest(req); - const response = await entitiesCatalog.entitiesBatch({ + const { items } = await entitiesCatalog.entitiesBatch({ entityRefs: request.entityRefs, filter: parseEntityFilterParams(req.query), fields: parseEntityTransformParams(req.query, request.fields), credentials: await httpAuth.credentials(req), }); - res.status(200).json(response); + writeEntitiesResponse(res, items, entities => ({ items: entities })); }) .get('/entity-facets', async (req, res) => { const response = await entitiesCatalog.facets({ diff --git a/plugins/catalog-backend/src/service/response/index.ts b/plugins/catalog-backend/src/service/response/index.ts new file mode 100644 index 0000000000..73c82c2aee --- /dev/null +++ b/plugins/catalog-backend/src/service/response/index.ts @@ -0,0 +1,22 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export { + processRawEntitiesResult, + processEntitiesResponseItems, + entitiesResponseToObjects, +} from './process'; +export { writeSingleEntityResponse, writeEntitiesResponse } from './write'; diff --git a/plugins/catalog-backend/src/service/response/process.ts b/plugins/catalog-backend/src/service/response/process.ts new file mode 100644 index 0000000000..26ab544a4d --- /dev/null +++ b/plugins/catalog-backend/src/service/response/process.ts @@ -0,0 +1,62 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Entity } from '@backstage/catalog-model'; +import { EntitiesResponseItems } from '../../catalog/types'; + +export function processRawEntitiesResult( + serializedEntities: (string | null)[], + transform?: (entity: Entity) => Entity, +): EntitiesResponseItems { + if (transform) { + return { + type: 'objects', + entities: serializedEntities.map(e => + e !== null ? transform(JSON.parse(e)) : e, + ), + }; + } + + return { + type: 'raw', + entities: serializedEntities, + }; +} + +export function processEntitiesResponseItems( + response: EntitiesResponseItems, + transform?: (entity: Entity) => Entity, +) { + if (!transform) { + return response; + } + if (response.type === 'raw') { + return processRawEntitiesResult(response.entities, transform); + } + return { + type: 'objects', + entities: response.entities.map(e => (e !== null ? transform(e) : e)), + }; +} + +export function entitiesResponseToObjects( + response: EntitiesResponseItems, +): (Entity | null)[] { + if (response.type === 'objects') { + return response.entities; + } + return response.entities.map(e => (e !== null ? JSON.parse(e) : e)); +} diff --git a/plugins/catalog-backend/src/service/response/write.ts b/plugins/catalog-backend/src/service/response/write.ts new file mode 100644 index 0000000000..1f9392e3a9 --- /dev/null +++ b/plugins/catalog-backend/src/service/response/write.ts @@ -0,0 +1,89 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Response } from 'express'; +import { EntitiesResponseItems } from '../../catalog/types'; +import { JsonValue } from '@backstage/types'; + +const JSON_CONTENT_TYPE = 'application/json; charset=utf-8'; + +export function writeSingleEntityResponse( + res: Response, + response: EntitiesResponseItems, +): boolean { + const entity = response.entities[0]; + if (!entity) { + return false; + } + + if (typeof entity === 'string') { + res.setHeader('Content-Type', JSON_CONTENT_TYPE); + res.status(200); + res.write(entity); + } else { + res.json(entity); + } + + return true; +} + +export function writeEntitiesResponse( + res: Response, + response: EntitiesResponseItems, + responseWrapper?: (entities: JsonValue) => JsonValue, +) { + if (response.type === 'objects') { + res.json( + responseWrapper + ? responseWrapper?.(response.entities) + : response.entities, + ); + return; + } + + res.setHeader('Content-Type', JSON_CONTENT_TYPE); + res.status(200); + + // responseWrapper allows the caller to render the entities within an object + let trailing = ''; + if (responseWrapper) { + const marker = `__MARKER_${Math.random().toString(36).slice(2, 10)}__`; + const wrapped = JSON.stringify(responseWrapper(marker)); + const parts = wrapped.split(marker); + if (parts.length !== 2) { + throw new Error( + `Entity items response was incorrectly wrapped into ${parts.length} different parts`, + ); + } + res.write(parts[0], 'utf8'); + trailing = parts[1]; + } + + let first = true; + for (const entity of response.entities) { + if (first) { + res.write('[', 'utf8'); + first = false; + } else { + res.write(',', 'utf8'); + } + res.write(entity, 'utf8'); + } + res.end(']'); + if (trailing) { + res.write(trailing, 'utf8'); + } +} diff --git a/plugins/catalog-backend/src/service/util.ts b/plugins/catalog-backend/src/service/util.ts index c05fb14558..2126cf25b3 100644 --- a/plugins/catalog-backend/src/service/util.ts +++ b/plugins/catalog-backend/src/service/util.ts @@ -20,6 +20,7 @@ import lodash from 'lodash'; import { z } from 'zod'; import { Cursor, + EntitiesResponseItems, QueryEntitiesCursorRequest, QueryEntitiesInitialRequest, QueryEntitiesRequest, @@ -148,35 +149,32 @@ export function decodeCursor(encodedCursor: string) { // sure that all adopters have re-stitched their entities so that the new // targetRef field is present on them, and that they have stopped consuming // the now-removed old field -// TODO(jhaals): Remove this in April 2022 -export function expandLegacyCompoundRelationRefsInResponse( - entities: Entity[], -): void { - for (const entity of entities) { - if (entity.relations) { - for (const relation of entity.relations as any) { - if (!relation.targetRef && relation.target) { - // This is the case where an old-form entity, not yet stitched with - // the updated code, was in the database - relation.targetRef = stringifyEntityRef(relation.target); - } else if (!relation.target && relation.targetRef) { - // This is the case where a new-form entity, stitched with the - // updated code, was in the database but we still want to produce - // the old data shape as well for compatibility reasons - relation.target = parseEntityRef(relation.targetRef); - } +// TODO(patriko): Remove this in catalog 2.0 +export function expandLegacyCompoundRelationsInEntity(entity: Entity): Entity { + if (entity.relations) { + for (const relation of entity.relations as any) { + if (!relation.targetRef && relation.target) { + // This is the case where an old-form entity, not yet stitched with + // the updated code, was in the database + relation.targetRef = stringifyEntityRef(relation.target); + } else if (!relation.target && relation.targetRef) { + // This is the case where a new-form entity, stitched with the + // updated code, was in the database but we still want to produce + // the old data shape as well for compatibility reasons + relation.target = parseEntityRef(relation.targetRef); } } } + return entity; } export interface EntityArrayJsonStream { - send(entities: Entity[]): boolean; + send(entities: EntitiesResponseItems): boolean; complete(): void; close(): void; } -// Helps stream Entity[] as a JSON response stream to avoid performance issues +// Helps stream EntitiesResponseItems[] as a JSON response stream to avoid performance issues export function createEntityArrayJsonStream( res: Response, ): EntityArrayJsonStream { @@ -186,19 +184,33 @@ export function createEntityArrayJsonStream( let completed = false; return { - send(entities) { + send(response) { if (firstSend) { res.setHeader('Content-Type', 'application/json; charset=utf-8'); res.status(200); res.flushHeaders(); } + if (response.type === 'raw') { + let result = true; + for (const item of response.entities) { + if (firstSend) { + result ||= res.write('['); + firstSend = false; + } else { + result ||= res.write(','); + } + result ||= res.write(item); + } + return result; + } + let data: string; if (prettyPrint) { - data = JSON.stringify(entities, null, 2); + data = JSON.stringify(response.entities, null, 2); data = firstSend ? data.slice(0, -2) : `,\n${data.slice(2, -2)}`; } else { - data = JSON.stringify(entities); + data = JSON.stringify(response.entities); data = firstSend ? data.slice(0, -1) : `,${data.slice(1, -1)}`; } diff --git a/plugins/catalog-backend/src/tests/integration.test.ts b/plugins/catalog-backend/src/tests/integration.test.ts index 737bb107ca..fdafbd5edf 100644 --- a/plugins/catalog-backend/src/tests/integration.test.ts +++ b/plugins/catalog-backend/src/tests/integration.test.ts @@ -55,6 +55,7 @@ import { DefaultStitcher } from '../stitching/DefaultStitcher'; import { mockServices } from '@backstage/backend-test-utils'; import { LoggerService } from '@backstage/backend-plugin-api'; import { DatabaseManager } from '@backstage/backend-common'; +import { entitiesResponseToObjects } from '../service/response'; const voidLogger = mockServices.logger.mock(); @@ -365,7 +366,12 @@ class TestHarness { async getOutputEntities(): Promise> { const { entities } = await this.#catalog.entities(); - return Object.fromEntries(entities.map(e => [stringifyEntityRef(e), e])); + return Object.fromEntries( + entitiesResponseToObjects(entities).map(e => [ + stringifyEntityRef(e!), + e!, + ]), + ); } async refresh(options: RefreshOptions) { From 6f09b9af9983a2a1a258ba11760e941e41b8754e Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 12 Dec 2024 13:30:11 +0100 Subject: [PATCH 02/13] catalog-backend: respose item type object + single entity refactor and tests Signed-off-by: Patrik Oldsberg --- plugins/catalog-backend/src/catalog/types.ts | 2 +- .../src/service/AuthorizedEntitiesCatalog.ts | 6 +- .../src/service/createRouter.test.ts | 32 ++--- .../src/service/createRouter.ts | 16 +-- .../src/service/response/process.ts | 6 +- .../src/service/response/write.test.ts | 125 ++++++++++++++++++ .../src/service/response/write.ts | 29 ++-- 7 files changed, 170 insertions(+), 46 deletions(-) create mode 100644 plugins/catalog-backend/src/service/response/write.test.ts diff --git a/plugins/catalog-backend/src/catalog/types.ts b/plugins/catalog-backend/src/catalog/types.ts index d00c38f536..1fbf337986 100644 --- a/plugins/catalog-backend/src/catalog/types.ts +++ b/plugins/catalog-backend/src/catalog/types.ts @@ -58,7 +58,7 @@ export type EntitiesRequest = { */ export type EntitiesResponseItems = | { - type: 'objects'; + type: 'object'; entities: (Entity | null)[]; } | { diff --git a/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.ts b/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.ts index 225c87cae9..073e153dcc 100644 --- a/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.ts @@ -60,7 +60,7 @@ export class AuthorizedEntitiesCatalog implements EntitiesCatalog { if (authorizeDecision.result === AuthorizeResult.DENY) { return { - entities: { type: 'objects', entities: [] }, + entities: { type: 'object', entities: [] }, pageInfo: { hasNextPage: false }, }; } @@ -93,7 +93,7 @@ export class AuthorizedEntitiesCatalog implements EntitiesCatalog { if (authorizeDecision.result === AuthorizeResult.DENY) { return { items: { - type: 'objects', + type: 'object', entities: new Array(request.entityRefs.length).fill(null), }, }; @@ -126,7 +126,7 @@ export class AuthorizedEntitiesCatalog implements EntitiesCatalog { if (authorizeDecision.result === AuthorizeResult.DENY) { return { - items: { type: 'objects', entities: [] }, + items: { type: 'object', entities: [] }, pageInfo: {}, totalItems: 0, }; diff --git a/plugins/catalog-backend/src/service/createRouter.test.ts b/plugins/catalog-backend/src/service/createRouter.test.ts index 8efb6543a3..d4940e4a35 100644 --- a/plugins/catalog-backend/src/service/createRouter.test.ts +++ b/plugins/catalog-backend/src/service/createRouter.test.ts @@ -136,7 +136,7 @@ describe('createRouter readonly disabled', () => { ]; entitiesCatalog.queryEntities.mockResolvedValueOnce({ - items: { type: 'objects', entities: [entities[0]] }, + items: { type: 'object', entities: [entities[0]] }, pageInfo: {}, totalItems: 1, }); @@ -149,7 +149,7 @@ describe('createRouter readonly disabled', () => { it('parses single and multiple request parameters and passes them down', async () => { entitiesCatalog.queryEntities.mockResolvedValueOnce({ - items: { type: 'objects', entities: [] }, + items: { type: 'object', entities: [] }, pageInfo: {}, totalItems: 0, }); @@ -185,7 +185,7 @@ describe('createRouter readonly disabled', () => { ]; entitiesCatalog.queryEntities.mockResolvedValueOnce({ - items: { type: 'objects', entities: items }, + items: { type: 'object', entities: items }, totalItems: 100, pageInfo: { nextCursor: mockCursor() }, }); @@ -203,7 +203,7 @@ describe('createRouter readonly disabled', () => { it('parses initial request', async () => { entitiesCatalog.queryEntities.mockResolvedValueOnce({ - items: { type: 'objects', entities: [] }, + items: { type: 'object', entities: [] }, pageInfo: {}, totalItems: 0, }); @@ -239,7 +239,7 @@ describe('createRouter readonly disabled', () => { it('parses encoded params request', async () => { entitiesCatalog.queryEntities.mockResolvedValueOnce({ - items: { type: 'objects', entities: [] }, + items: { type: 'object', entities: [] }, pageInfo: {}, totalItems: 0, }); @@ -283,7 +283,7 @@ describe('createRouter readonly disabled', () => { ]; entitiesCatalog.queryEntities.mockResolvedValueOnce({ - items: { type: 'objects', entities: items }, + items: { type: 'object', entities: items }, totalItems: 100, pageInfo: { nextCursor: mockCursor() }, }); @@ -312,7 +312,7 @@ describe('createRouter readonly disabled', () => { ]; entitiesCatalog.queryEntities.mockResolvedValueOnce({ - items: { type: 'objects', entities: items }, + items: { type: 'object', entities: items }, totalItems: 100, pageInfo: { nextCursor: mockCursor({ fullTextFilter: { term: 'mySearch' } }), @@ -354,7 +354,7 @@ describe('createRouter readonly disabled', () => { ]; entitiesCatalog.queryEntities.mockResolvedValueOnce({ - items: { type: 'objects', entities: items }, + items: { type: 'object', entities: items }, totalItems: 100, pageInfo: { nextCursor: mockCursor() }, }); @@ -379,7 +379,7 @@ describe('createRouter readonly disabled', () => { ]; entitiesCatalog.queryEntities.mockResolvedValueOnce({ - items: { type: 'objects', entities: items }, + items: { type: 'object', entities: items }, totalItems: 100, pageInfo: { nextCursor: mockCursor() }, }); @@ -402,7 +402,7 @@ describe('createRouter readonly disabled', () => { }, }; entitiesCatalog.entities.mockResolvedValue({ - entities: { type: 'objects', entities: [entity] }, + entities: { type: 'object', entities: [entity] }, pageInfo: { hasNextPage: false }, }); @@ -419,7 +419,7 @@ describe('createRouter readonly disabled', () => { it('responds with a 404 for missing entities', async () => { entitiesCatalog.entities.mockResolvedValue({ - entities: { type: 'objects', entities: [] }, + entities: { type: 'object', entities: [] }, pageInfo: { hasNextPage: false }, }); @@ -446,7 +446,7 @@ describe('createRouter readonly disabled', () => { }, }; entitiesCatalog.entitiesBatch.mockResolvedValue({ - items: { type: 'objects', entities: [entity] }, + items: { type: 'object', entities: [entity] }, }); const response = await request(app).get('/entities/by-name/k/ns/n'); @@ -462,7 +462,7 @@ describe('createRouter readonly disabled', () => { it('responds with a 404 for missing entities', async () => { entitiesCatalog.entitiesBatch.mockResolvedValue({ - items: { type: 'objects', entities: [null] }, + items: { type: 'object', entities: [null] }, }); const response = await request(app).get('/entities/by-name/b/d/c'); @@ -534,7 +534,7 @@ describe('createRouter readonly disabled', () => { }; const entityRef = stringifyEntityRef(entity); entitiesCatalog.entitiesBatch.mockResolvedValue({ - items: { type: 'objects', entities: [entity] }, + items: { type: 'object', entities: [entity] }, }); const response = await request(app) .post('/entities/by-refs?filter=kind=Component') @@ -910,7 +910,7 @@ describe('createRouter readonly enabled', () => { ]; entitiesCatalog.queryEntities.mockResolvedValueOnce({ - items: { type: 'objects', entities: [entities[0]] }, + items: { type: 'object', entities: [entities[0]] }, pageInfo: {}, totalItems: 1, }); @@ -1130,7 +1130,7 @@ describe('NextRouter permissioning', () => { }, }; entitiesCatalog.entities.mockResolvedValueOnce({ - entities: { type: 'objects', entities: [spideySense] }, + entities: { type: 'object', entities: [spideySense] }, pageInfo: { hasNextPage: false }, }); diff --git a/plugins/catalog-backend/src/service/createRouter.ts b/plugins/catalog-backend/src/service/createRouter.ts index b670253089..7208c29a89 100644 --- a/plugins/catalog-backend/src/service/createRouter.ts +++ b/plugins/catalog-backend/src/service/createRouter.ts @@ -23,7 +23,7 @@ import { stringifyEntityRef, } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; -import { InputError, NotFoundError, serializeError } from '@backstage/errors'; +import { InputError, serializeError } from '@backstage/errors'; import express from 'express'; import yn from 'yn'; import { z } from 'zod'; @@ -272,9 +272,7 @@ export async function createRouter( filter: basicEntityFilter({ 'metadata.uid': uid }), credentials: await httpAuth.credentials(req), }); - if (!writeSingleEntityResponse(res, entities)) { - throw new NotFoundError(`No entity with uid ${uid}`); - } + writeSingleEntityResponse(res, entities, `No entity with uid ${uid}`); }) .delete('/entities/by-uid/:uid', async (req, res) => { const { uid } = req.params; @@ -289,11 +287,11 @@ export async function createRouter( entityRefs: [stringifyEntityRef({ kind, namespace, name })], credentials: await httpAuth.credentials(req), }); - if (!writeSingleEntityResponse(res, items)) { - throw new NotFoundError( - `No entity named '${name}' found, with kind '${kind}' in namespace '${namespace}'`, - ); - } + writeSingleEntityResponse( + res, + items, + `No entity named '${name}' found, with kind '${kind}' in namespace '${namespace}'`, + ); }) .get( '/entities/by-name/:kind/:namespace/:name/ancestry', diff --git a/plugins/catalog-backend/src/service/response/process.ts b/plugins/catalog-backend/src/service/response/process.ts index 26ab544a4d..7e7730d697 100644 --- a/plugins/catalog-backend/src/service/response/process.ts +++ b/plugins/catalog-backend/src/service/response/process.ts @@ -23,7 +23,7 @@ export function processRawEntitiesResult( ): EntitiesResponseItems { if (transform) { return { - type: 'objects', + type: 'object', entities: serializedEntities.map(e => e !== null ? transform(JSON.parse(e)) : e, ), @@ -47,7 +47,7 @@ export function processEntitiesResponseItems( return processRawEntitiesResult(response.entities, transform); } return { - type: 'objects', + type: 'object', entities: response.entities.map(e => (e !== null ? transform(e) : e)), }; } @@ -55,7 +55,7 @@ export function processEntitiesResponseItems( export function entitiesResponseToObjects( response: EntitiesResponseItems, ): (Entity | null)[] { - if (response.type === 'objects') { + if (response.type === 'object') { return response.entities; } return response.entities.map(e => (e !== null ? JSON.parse(e) : e)); diff --git a/plugins/catalog-backend/src/service/response/write.test.ts b/plugins/catalog-backend/src/service/response/write.test.ts new file mode 100644 index 0000000000..412aefe404 --- /dev/null +++ b/plugins/catalog-backend/src/service/response/write.test.ts @@ -0,0 +1,125 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import express from 'express'; +import { mockErrorHandler } from '@backstage/backend-test-utils'; +import request from 'supertest'; +import { writeSingleEntityResponse } from './write'; + +describe('writeSingleEntityResponse', () => { + const app = express(); + app.use(express.json()); + app.get('/echo', (req, res) => { + writeSingleEntityResponse(res, req.body, 'not found'); + }); + app.use(mockErrorHandler()); + + describe('in object form', () => { + it('should write a single entity', async () => { + const res = await request(app) + .get('/echo') + .send({ + type: 'object', + entities: [{ kind: 'Component' }, { kind: 'User' }], + }); + + expect(res.status).toBe(200); + expect(res.type).toBe('application/json'); + expect(res.header['content-type']).toBe( + 'application/json; charset=utf-8', + ); + expect(res.body).toEqual({ kind: 'Component' }); + }); + + it('should write a missing entity', async () => { + const res = await request(app) + .get('/echo') + .send({ type: 'object', entities: [null] }); + + expect(res.status).toBe(404); + expect(res.type).toBe('application/json'); + expect(res.header['content-type']).toBe( + 'application/json; charset=utf-8', + ); + expect(res.body).toMatchObject({ + error: { name: 'NotFoundError', message: 'not found' }, + }); + }); + + it('should write no entities', async () => { + const res = await request(app) + .get('/echo') + .send({ type: 'object', entities: [] }); + + expect(res.status).toBe(404); + expect(res.type).toBe('application/json'); + expect(res.header['content-type']).toBe( + 'application/json; charset=utf-8', + ); + expect(res.body).toMatchObject({ + error: { name: 'NotFoundError', message: 'not found' }, + }); + }); + }); + + describe('in raw form', () => { + it('should write a single entity', async () => { + const res = await request(app) + .get('/echo') + .send({ + type: 'raw', + entities: ['{"kind":"Component"}', '{"kind":"User"}'], + }); + + expect(res.status).toBe(200); + expect(res.type).toBe('application/json'); + expect(res.header['content-type']).toBe( + 'application/json; charset=utf-8', + ); + expect(res.body).toEqual({ kind: 'Component' }); + }); + + it('should write a missing entity', async () => { + const res = await request(app) + .get('/echo') + .send({ type: 'raw', entities: [null] }); + + expect(res.status).toBe(404); + expect(res.type).toBe('application/json'); + expect(res.header['content-type']).toBe( + 'application/json; charset=utf-8', + ); + expect(res.body).toMatchObject({ + error: { name: 'NotFoundError', message: 'not found' }, + }); + }); + + it('should write no entities', async () => { + const res = await request(app) + .get('/echo') + .send({ type: 'raw', entities: [] }); + + expect(res.status).toBe(404); + expect(res.type).toBe('application/json'); + expect(res.header['content-type']).toBe( + 'application/json; charset=utf-8', + ); + expect(res.body).toMatchObject({ + error: { name: 'NotFoundError', message: 'not found' }, + }); + }); + }); +}); diff --git a/plugins/catalog-backend/src/service/response/write.ts b/plugins/catalog-backend/src/service/response/write.ts index 1f9392e3a9..1dac8d3ce4 100644 --- a/plugins/catalog-backend/src/service/response/write.ts +++ b/plugins/catalog-backend/src/service/response/write.ts @@ -17,27 +17,29 @@ import { Response } from 'express'; import { EntitiesResponseItems } from '../../catalog/types'; import { JsonValue } from '@backstage/types'; +import { NotFoundError } from '@backstage/errors'; const JSON_CONTENT_TYPE = 'application/json; charset=utf-8'; export function writeSingleEntityResponse( res: Response, response: EntitiesResponseItems, -): boolean { - const entity = response.entities[0]; - if (!entity) { - return false; - } + notFoundMessage: string, +) { + if (response.type === 'object') { + if (!response.entities[0]) { + throw new NotFoundError(notFoundMessage); + } - if (typeof entity === 'string') { - res.setHeader('Content-Type', JSON_CONTENT_TYPE); - res.status(200); - res.write(entity); + res.json(response.entities[0]); } else { - res.json(entity); - } + if (!response.entities[0]) { + throw new NotFoundError(notFoundMessage); + } - return true; + res.setHeader('Content-Type', JSON_CONTENT_TYPE); + res.end(response.entities[0]); + } } export function writeEntitiesResponse( @@ -45,7 +47,7 @@ export function writeEntitiesResponse( response: EntitiesResponseItems, responseWrapper?: (entities: JsonValue) => JsonValue, ) { - if (response.type === 'objects') { + if (response.type === 'object') { res.json( responseWrapper ? responseWrapper?.(response.entities) @@ -55,7 +57,6 @@ export function writeEntitiesResponse( } res.setHeader('Content-Type', JSON_CONTENT_TYPE); - res.status(200); // responseWrapper allows the caller to render the entities within an object let trailing = ''; From daee494dfa26dbdca4cf8f6f3e656f8dda6e29bd Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 12 Dec 2024 13:39:48 +0100 Subject: [PATCH 03/13] catalog-backend: async response writing + tests and fixes Signed-off-by: Patrik Oldsberg --- .../src/service/createRouter.ts | 8 +- .../src/service/response/write.test.ts | 158 +++++++++++++++++- .../src/service/response/write.ts | 28 ++-- 3 files changed, 177 insertions(+), 17 deletions(-) diff --git a/plugins/catalog-backend/src/service/createRouter.ts b/plugins/catalog-backend/src/service/createRouter.ts index 7208c29a89..f89ae745ce 100644 --- a/plugins/catalog-backend/src/service/createRouter.ts +++ b/plugins/catalog-backend/src/service/createRouter.ts @@ -171,7 +171,7 @@ export async function createRouter( res.setHeader('link', `<${url.pathname}${url.search}>; rel="next"`); } - writeEntitiesResponse(res, entities); + await writeEntitiesResponse(res, entities); return; } @@ -253,7 +253,7 @@ export async function createRouter( credentials: await httpAuth.credentials(req), }); - writeEntitiesResponse(res, items, entities => ({ + await writeEntitiesResponse(res, items, entities => ({ items: entities, totalItems, pageInfo: { @@ -312,7 +312,9 @@ export async function createRouter( fields: parseEntityTransformParams(req.query, request.fields), credentials: await httpAuth.credentials(req), }); - writeEntitiesResponse(res, items, entities => ({ items: entities })); + await writeEntitiesResponse(res, items, entities => ({ + items: entities, + })); }) .get('/entity-facets', async (req, res) => { const response = await entitiesCatalog.facets({ diff --git a/plugins/catalog-backend/src/service/response/write.test.ts b/plugins/catalog-backend/src/service/response/write.test.ts index 412aefe404..b1a688b48d 100644 --- a/plugins/catalog-backend/src/service/response/write.test.ts +++ b/plugins/catalog-backend/src/service/response/write.test.ts @@ -17,7 +17,7 @@ import express from 'express'; import { mockErrorHandler } from '@backstage/backend-test-utils'; import request from 'supertest'; -import { writeSingleEntityResponse } from './write'; +import { writeEntitiesResponse, writeSingleEntityResponse } from './write'; describe('writeSingleEntityResponse', () => { const app = express(); @@ -123,3 +123,159 @@ describe('writeSingleEntityResponse', () => { }); }); }); + +describe('writeEntitiesResponse', () => { + const app = express(); + app.use(express.json()); + app.get('/echo', (req, res) => { + writeEntitiesResponse(res, req.body); + }); + app.get('/wrapped', (req, res) => { + writeEntitiesResponse(res, req.body, entities => ({ + page: 1, + items: entities, + totalItems: 1337, + })); + }); + app.use(mockErrorHandler()); + + describe('in object form', () => { + it('should return empty list', async () => { + const res = await request(app).get('/echo').send({ + type: 'object', + entities: [], + }); + + expect(res.status).toBe(200); + expect(res.type).toBe('application/json'); + expect(res.header['content-type']).toBe( + 'application/json; charset=utf-8', + ); + expect(res.body).toEqual([]); + }); + + it('should return mixed objects', async () => { + const res = await request(app) + .get('/echo') + .send({ + type: 'object', + entities: [{ kind: 'Component' }, null, { kind: 'User' }, null], + }); + + expect(res.status).toBe(200); + expect(res.type).toBe('application/json'); + expect(res.header['content-type']).toBe( + 'application/json; charset=utf-8', + ); + expect(res.body).toEqual([ + { kind: 'Component' }, + null, + { kind: 'User' }, + null, + ]); + }); + + it('should wrap response of empty list', async () => { + const res = await request(app) + .get('/wrapped') + .send({ type: 'object', entities: [] }); + + expect(res.status).toBe(200); + expect(res.type).toBe('application/json'); + expect(res.header['content-type']).toBe( + 'application/json; charset=utf-8', + ); + expect(res.body).toEqual({ page: 1, items: [], totalItems: 1337 }); + }); + + it('should wrap response of mixed list', async () => { + const res = await request(app) + .get('/wrapped') + .send({ + type: 'object', + entities: [{ kind: 'Component' }, null, { kind: 'User' }, null], + }); + + expect(res.status).toBe(200); + expect(res.type).toBe('application/json'); + expect(res.header['content-type']).toBe( + 'application/json; charset=utf-8', + ); + expect(res.body).toEqual({ + page: 1, + items: [{ kind: 'Component' }, null, { kind: 'User' }, null], + totalItems: 1337, + }); + }); + }); + + describe('in raw form', () => { + it('should return empty list', async () => { + const res = await request(app).get('/echo').send({ + type: 'raw', + entities: [], + }); + + expect(res.status).toBe(200); + expect(res.type).toBe('application/json'); + expect(res.header['content-type']).toBe( + 'application/json; charset=utf-8', + ); + expect(res.body).toEqual([]); + }); + + it('should return mixed objects', async () => { + const res = await request(app) + .get('/echo') + .send({ + type: 'raw', + entities: ['{"kind":"Component"}', null, '{"kind":"User"}', null], + }); + + expect(res.status).toBe(200); + expect(res.type).toBe('application/json'); + expect(res.header['content-type']).toBe( + 'application/json; charset=utf-8', + ); + expect(res.body).toEqual([ + { kind: 'Component' }, + null, + { kind: 'User' }, + null, + ]); + }); + + it('should wrap response of empty list', async () => { + const res = await request(app) + .get('/wrapped') + .send({ type: 'raw', entities: [] }); + + expect(res.status).toBe(200); + expect(res.type).toBe('application/json'); + expect(res.header['content-type']).toBe( + 'application/json; charset=utf-8', + ); + expect(res.body).toEqual({ page: 1, items: [], totalItems: 1337 }); + }); + + it('should wrap response of mixed list', async () => { + const res = await request(app) + .get('/wrapped') + .send({ + type: 'raw', + entities: ['{"kind":"Component"}', null, '{"kind":"User"}', null], + }); + + expect(res.status).toBe(200); + expect(res.type).toBe('application/json'); + expect(res.header['content-type']).toBe( + 'application/json; charset=utf-8', + ); + expect(res.body).toEqual({ + page: 1, + items: [{ kind: 'Component' }, null, { kind: 'User' }, null], + totalItems: 1337, + }); + }); + }); +}); diff --git a/plugins/catalog-backend/src/service/response/write.ts b/plugins/catalog-backend/src/service/response/write.ts index 1dac8d3ce4..950dd99ba7 100644 --- a/plugins/catalog-backend/src/service/response/write.ts +++ b/plugins/catalog-backend/src/service/response/write.ts @@ -42,7 +42,7 @@ export function writeSingleEntityResponse( } } -export function writeEntitiesResponse( +export async function writeEntitiesResponse( res: Response, response: EntitiesResponseItems, responseWrapper?: (entities: JsonValue) => JsonValue, @@ -63,7 +63,7 @@ export function writeEntitiesResponse( if (responseWrapper) { const marker = `__MARKER_${Math.random().toString(36).slice(2, 10)}__`; const wrapped = JSON.stringify(responseWrapper(marker)); - const parts = wrapped.split(marker); + const parts = wrapped.split(`"${marker}"`); if (parts.length !== 2) { throw new Error( `Entity items response was incorrectly wrapped into ${parts.length} different parts`, @@ -75,16 +75,18 @@ export function writeEntitiesResponse( let first = true; for (const entity of response.entities) { - if (first) { - res.write('[', 'utf8'); - first = false; - } else { - res.write(',', 'utf8'); - } - res.write(entity, 'utf8'); - } - res.end(']'); - if (trailing) { - res.write(trailing, 'utf8'); + const prefix = first ? '[' : ','; + first = false; + + await new Promise((resolve, reject) => { + res.write(prefix + entity, 'utf8', err => { + if (err) { + reject(err); + } else { + resolve(err); + } + }); + }); } + res.end(`${first ? '[' : ''}]${trailing}`); } From b018b8e7e6d9d54ed93f39ca900998bc9c361431 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 12 Dec 2024 13:54:33 +0100 Subject: [PATCH 04/13] catalog-backend: add response processing tests Signed-off-by: Patrik Oldsberg --- .../src/service/response/process.test.ts | 115 ++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 plugins/catalog-backend/src/service/response/process.test.ts diff --git a/plugins/catalog-backend/src/service/response/process.test.ts b/plugins/catalog-backend/src/service/response/process.test.ts new file mode 100644 index 0000000000..60b96608f7 --- /dev/null +++ b/plugins/catalog-backend/src/service/response/process.test.ts @@ -0,0 +1,115 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Entity } from '@backstage/catalog-model'; +import { + entitiesResponseToObjects, + processEntitiesResponseItems, + processRawEntitiesResult, +} from './process'; + +const mockTransform = (entity: Entity): Entity => ({ + ...entity, + kind: `transformed-${entity.kind}`, +}); + +describe('processRawEntitiesResult', () => { + it('should prefer keeping results in raw form', () => { + expect(processRawEntitiesResult(['{"kind":"test"}', null])).toEqual({ + type: 'raw', + entities: ['{"kind":"test"}', null], + }); + + expect( + processRawEntitiesResult(['{"kind":"test"}', null], mockTransform), + ).toEqual({ + type: 'object', + entities: [{ kind: 'transformed-test' }, null], + }); + }); +}); + +describe('processEntitiesResponseItems', () => { + it('should transform entities in object form', () => { + expect( + processEntitiesResponseItems({ + type: 'object', + entities: [{ kind: 'test' } as Entity, null], + }), + ).toEqual({ + type: 'object', + entities: [{ kind: 'test' }, null], + }); + + expect( + processEntitiesResponseItems( + { + type: 'object', + entities: [{ kind: 'test' } as Entity, null], + }, + mockTransform, + ), + ).toEqual({ + type: 'object', + entities: [{ kind: 'transformed-test' }, null], + }); + }); + + it('should transform entities in raw form', () => { + expect( + processEntitiesResponseItems({ + type: 'raw', + entities: ['{"kind":"test"}', null], + }), + ).toEqual({ + type: 'raw', + entities: ['{"kind":"test"}', null], + }); + + expect( + processEntitiesResponseItems( + { + type: 'raw', + entities: ['{"kind":"test"}', null], + }, + mockTransform, + ), + ).toEqual({ + type: 'object', + entities: [{ kind: 'transformed-test' }, null], + }); + }); +}); + +describe('entitiesResponseToObjects', () => { + it('should convert entities in object form', () => { + expect( + entitiesResponseToObjects({ + type: 'object', + entities: [null, { kind: 'test' } as Entity], + }), + ).toEqual([null, { kind: 'test' }]); + }); + + it('should convert entities in raw form', () => { + expect( + entitiesResponseToObjects({ + type: 'raw', + entities: [null, '{"kind":"test"}'], + }), + ).toEqual([null, { kind: 'test' }]); + }); +}); From d706bb5b34b53b2d2379459e19a470368d99a7e9 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 12 Dec 2024 14:23:39 +0100 Subject: [PATCH 05/13] catalog-backend: update entities response writing Signed-off-by: Patrik Oldsberg --- .../src/service/response/write.ts | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/plugins/catalog-backend/src/service/response/write.ts b/plugins/catalog-backend/src/service/response/write.ts index 950dd99ba7..0962ee15d1 100644 --- a/plugins/catalog-backend/src/service/response/write.ts +++ b/plugins/catalog-backend/src/service/response/write.ts @@ -78,15 +78,18 @@ export async function writeEntitiesResponse( const prefix = first ? '[' : ','; first = false; - await new Promise((resolve, reject) => { - res.write(prefix + entity, 'utf8', err => { - if (err) { - reject(err); - } else { - resolve(err); - } + const needsDrain = !res.write(prefix + entity, 'utf8'); + if (needsDrain) { + await new Promise(resolve => { + const cont = () => { + res.off('drain', cont); + res.off('close', cont); + resolve(); + }; + res.on('drain', cont); + res.on('close', cont); }); - }); + } } res.end(`${first ? '[' : ''}]${trailing}`); } From 31e2ca8d1836c21eb4922d171dedcc6c2424ef3a Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 12 Dec 2024 14:34:16 +0100 Subject: [PATCH 06/13] catalog-backend: integration tests for raw response + fix raw json stream Signed-off-by: Patrik Oldsberg --- .../src/service/createRouter.test.ts | 5 +- plugins/catalog-backend/src/service/util.ts | 18 ++--- .../src/tests/integration.test.ts | 81 +++++++++++++++---- 3 files changed, 76 insertions(+), 28 deletions(-) diff --git a/plugins/catalog-backend/src/service/createRouter.test.ts b/plugins/catalog-backend/src/service/createRouter.test.ts index d4940e4a35..312900bcf1 100644 --- a/plugins/catalog-backend/src/service/createRouter.test.ts +++ b/plugins/catalog-backend/src/service/createRouter.test.ts @@ -860,7 +860,7 @@ describe('createRouter readonly disabled', () => { }); }); -describe('createRouter readonly enabled', () => { +describe('createRouter readonly and raw json enabled', () => { let entitiesCatalog: jest.Mocked; let app: express.Express; let locationService: jest.Mocked; @@ -883,6 +883,7 @@ describe('createRouter readonly enabled', () => { getLocationByEntity: jest.fn(), }; const router = await createRouter({ + enableRawJson: true, entitiesCatalog, locationService, logger: mockServices.logger.mock(), @@ -910,7 +911,7 @@ describe('createRouter readonly enabled', () => { ]; entitiesCatalog.queryEntities.mockResolvedValueOnce({ - items: { type: 'object', entities: [entities[0]] }, + items: { type: 'raw', entities: [JSON.stringify(entities[0])] }, pageInfo: {}, totalItems: 1, }); diff --git a/plugins/catalog-backend/src/service/util.ts b/plugins/catalog-backend/src/service/util.ts index 2126cf25b3..7ff7c8fcb3 100644 --- a/plugins/catalog-backend/src/service/util.ts +++ b/plugins/catalog-backend/src/service/util.ts @@ -192,17 +192,13 @@ export function createEntityArrayJsonStream( } if (response.type === 'raw') { - let result = true; + let needsDrain = false; for (const item of response.entities) { - if (firstSend) { - result ||= res.write('['); - firstSend = false; - } else { - result ||= res.write(','); - } - result ||= res.write(item); + const prefix = firstSend ? '[' : ','; + firstSend = false; + needsDrain ||= !res.write(prefix + item, 'utf8'); } - return result; + return !needsDrain; } let data: string; @@ -215,13 +211,13 @@ export function createEntityArrayJsonStream( } firstSend = false; - return res.write(data); + return res.write(data, 'utf8'); }, complete() { if (firstSend) { res.json([]); } else { - res.end(prettyPrint ? '\n]' : ']'); + res.end(prettyPrint ? '\n]' : ']', 'utf8'); } completed = true; }, diff --git a/plugins/catalog-backend/src/tests/integration.test.ts b/plugins/catalog-backend/src/tests/integration.test.ts index fdafbd5edf..32116ace0d 100644 --- a/plugins/catalog-backend/src/tests/integration.test.ts +++ b/plugins/catalog-backend/src/tests/integration.test.ts @@ -30,7 +30,6 @@ import { processingResult, } from '@backstage/plugin-catalog-node'; import { PermissionEvaluator } from '@backstage/plugin-permission-common'; -import { JsonObject } from '@backstage/types'; import { createHash } from 'crypto'; import { Knex } from 'knex'; import { EntitiesCatalog } from '../catalog/types'; @@ -199,7 +198,7 @@ class TestHarness { readonly #proxyProgressTracker: ProxyProgressTracker; static async create(options?: { - config?: JsonObject; + enableRawJson?: boolean; logger?: LoggerService; db?: Knex; permissions?: PermissionEvaluator; @@ -209,21 +208,19 @@ class TestHarness { emit: CatalogProcessorEmit, ): Promise; }) { - const config = new ConfigReader( - options?.config ?? { - backend: { - database: { - client: 'better-sqlite3', - connection: ':memory:', - }, - }, - catalog: { - stitchingStrategy: { - mode: 'immediate', - }, + const config = new ConfigReader({ + backend: { + database: { + client: 'better-sqlite3', + connection: ':memory:', }, }, - ); + catalog: { + stitchingStrategy: { + mode: 'immediate', + }, + }, + }); const logger = options?.logger ?? mockServices.logger.mock(); const db = options?.db ?? @@ -280,6 +277,7 @@ class TestHarness { database: db, logger, stitcher, + enableRawJson: options?.enableRawJson, }); const proxyProgressTracker = new ProxyProgressTracker( new NoopProgressTracker(), @@ -787,4 +785,57 @@ describe('Catalog Backend Integration', () => { ], }); }); + + it('should return valid responses in raw JSON mode', async () => { + const harness = await TestHarness.create({ + enableRawJson: true, + }); + + const entityA = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + name: 'a', + annotations: { + 'backstage.io/managed-by-location': 'url:.', + 'backstage.io/managed-by-origin-location': 'url:.', + }, + }, + }; + const entityB = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + name: 'b', + annotations: { + 'backstage.io/managed-by-location': 'url:.', + 'backstage.io/managed-by-origin-location': 'url:.', + }, + }, + }; + + await harness.setInputEntities([entityA, entityB]); + await expect(harness.process()).resolves.toEqual({}); + + await expect(harness.getOutputEntities()).resolves.toEqual({ + 'component:default/a': { + ...entityA, + metadata: { + ...entityA.metadata, + etag: expect.any(String), + uid: expect.any(String), + }, + relations: [], + }, + 'component:default/b': { + ...entityB, + metadata: { + ...entityB.metadata, + etag: expect.any(String), + uid: expect.any(String), + }, + relations: [], + }, + }); + }); }); From e97b87136717ed3063a0b7bebe5506c3d8d6f633 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 12 Dec 2024 14:35:37 +0100 Subject: [PATCH 07/13] catalog-backend: moved createEntityArrayJsonStream to response utils Signed-off-by: Patrik Oldsberg --- .../src/service/createRouter.ts | 2 +- .../response/createEntityArrayJsonStream.ts | 79 +++++++++++++++++++ .../src/service/response/index.ts | 1 + plugins/catalog-backend/src/service/util.ts | 64 +-------------- 4 files changed, 82 insertions(+), 64 deletions(-) create mode 100644 plugins/catalog-backend/src/service/response/createEntityArrayJsonStream.ts diff --git a/plugins/catalog-backend/src/service/createRouter.ts b/plugins/catalog-backend/src/service/createRouter.ts index f89ae745ce..6ea9ea1c13 100644 --- a/plugins/catalog-backend/src/service/createRouter.ts +++ b/plugins/catalog-backend/src/service/createRouter.ts @@ -41,7 +41,6 @@ import { parseEntityFacetParams } from './request/parseEntityFacetParams'; import { parseEntityOrderParams } from './request/parseEntityOrderParams'; import { LocationService, RefreshService } from './types'; import { - createEntityArrayJsonStream, disallowReadonlyMode, encodeCursor, expandLegacyCompoundRelationsInEntity, @@ -61,6 +60,7 @@ import { LocationAnalyzer } from '@backstage/plugin-catalog-node'; import { AuthorizedValidationService } from './AuthorizedValidationService'; import { DeferredPromise, createDeferred } from '@backstage/types'; import { + createEntityArrayJsonStream, processEntitiesResponseItems, writeEntitiesResponse, writeSingleEntityResponse, diff --git a/plugins/catalog-backend/src/service/response/createEntityArrayJsonStream.ts b/plugins/catalog-backend/src/service/response/createEntityArrayJsonStream.ts new file mode 100644 index 0000000000..4762afd2d9 --- /dev/null +++ b/plugins/catalog-backend/src/service/response/createEntityArrayJsonStream.ts @@ -0,0 +1,79 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { EntitiesResponseItems } from '../../catalog/types'; +import { Response } from 'express'; + +export interface EntityArrayJsonStream { + send(entities: EntitiesResponseItems): boolean; + complete(): void; + close(): void; +} + +// Helps stream EntitiesResponseItems[] as a JSON response stream to avoid performance issues +export function createEntityArrayJsonStream( + res: Response, +): EntityArrayJsonStream { + // Imitate the httpRouter behavior of pretty-printing in development + const prettyPrint = process.env.NODE_ENV === 'development'; + let firstSend = true; + let completed = false; + + return { + send(response) { + if (firstSend) { + res.setHeader('Content-Type', 'application/json; charset=utf-8'); + res.status(200); + res.flushHeaders(); + } + + if (response.type === 'raw') { + let needsDrain = false; + for (const item of response.entities) { + const prefix = firstSend ? '[' : ','; + firstSend = false; + needsDrain ||= !res.write(prefix + item, 'utf8'); + } + return !needsDrain; + } + + let data: string; + if (prettyPrint) { + data = JSON.stringify(response.entities, null, 2); + data = firstSend ? data.slice(0, -2) : `,\n${data.slice(2, -2)}`; + } else { + data = JSON.stringify(response.entities); + data = firstSend ? data.slice(0, -1) : `,${data.slice(1, -1)}`; + } + + firstSend = false; + return res.write(data, 'utf8'); + }, + complete() { + if (firstSend) { + res.json([]); + } else { + res.end(prettyPrint ? '\n]' : ']', 'utf8'); + } + completed = true; + }, + close() { + if (!completed) { + res.end(); + } + }, + }; +} diff --git a/plugins/catalog-backend/src/service/response/index.ts b/plugins/catalog-backend/src/service/response/index.ts index 73c82c2aee..2e01071dbd 100644 --- a/plugins/catalog-backend/src/service/response/index.ts +++ b/plugins/catalog-backend/src/service/response/index.ts @@ -20,3 +20,4 @@ export { entitiesResponseToObjects, } from './process'; export { writeSingleEntityResponse, writeEntitiesResponse } from './write'; +export { createEntityArrayJsonStream } from './createEntityArrayJsonStream'; diff --git a/plugins/catalog-backend/src/service/util.ts b/plugins/catalog-backend/src/service/util.ts index 7ff7c8fcb3..9847cb7225 100644 --- a/plugins/catalog-backend/src/service/util.ts +++ b/plugins/catalog-backend/src/service/util.ts @@ -15,12 +15,11 @@ */ import { InputError, NotAllowedError } from '@backstage/errors'; -import { Request, Response } from 'express'; +import { Request } from 'express'; import lodash from 'lodash'; import { z } from 'zod'; import { Cursor, - EntitiesResponseItems, QueryEntitiesCursorRequest, QueryEntitiesInitialRequest, QueryEntitiesRequest, @@ -167,64 +166,3 @@ export function expandLegacyCompoundRelationsInEntity(entity: Entity): Entity { } return entity; } - -export interface EntityArrayJsonStream { - send(entities: EntitiesResponseItems): boolean; - complete(): void; - close(): void; -} - -// Helps stream EntitiesResponseItems[] as a JSON response stream to avoid performance issues -export function createEntityArrayJsonStream( - res: Response, -): EntityArrayJsonStream { - // Imitate the httpRouter behavior of pretty-printing in development - const prettyPrint = process.env.NODE_ENV === 'development'; - let firstSend = true; - let completed = false; - - return { - send(response) { - if (firstSend) { - res.setHeader('Content-Type', 'application/json; charset=utf-8'); - res.status(200); - res.flushHeaders(); - } - - if (response.type === 'raw') { - let needsDrain = false; - for (const item of response.entities) { - const prefix = firstSend ? '[' : ','; - firstSend = false; - needsDrain ||= !res.write(prefix + item, 'utf8'); - } - return !needsDrain; - } - - let data: string; - if (prettyPrint) { - data = JSON.stringify(response.entities, null, 2); - data = firstSend ? data.slice(0, -2) : `,\n${data.slice(2, -2)}`; - } else { - data = JSON.stringify(response.entities); - data = firstSend ? data.slice(0, -1) : `,${data.slice(1, -1)}`; - } - - firstSend = false; - return res.write(data, 'utf8'); - }, - complete() { - if (firstSend) { - res.json([]); - } else { - res.end(prettyPrint ? '\n]' : ']', 'utf8'); - } - completed = true; - }, - close() { - if (!completed) { - res.end(); - } - }, - }; -} From 9621b02ec619349b3768700e35158278a401538a Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 12 Dec 2024 14:40:32 +0100 Subject: [PATCH 08/13] catalog-backend: update DefaultEntitiesCatalog tests with new response structure Signed-off-by: Patrik Oldsberg --- .../service/DefaultEntitiesCatalog.test.ts | 141 +++++++++++++----- 1 file changed, 100 insertions(+), 41 deletions(-) diff --git a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts index 379c861ba1..070b93ab2c 100644 --- a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts +++ b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts @@ -896,7 +896,10 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response1 = await catalog.queryEntities(request1); - expect(response1.items).toEqual([entityFrom('A'), entityFrom('B')]); + expect(entitiesResponseToObjects(response1.items)).toEqual([ + entityFrom('A'), + entityFrom('B'), + ]); expect(response1.pageInfo.nextCursor).toBeDefined(); expect(response1.pageInfo.prevCursor).toBeUndefined(); expect(response1.totalItems).toBe(names.length); @@ -908,7 +911,10 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response2 = await catalog.queryEntities(request2); - expect(response2.items).toEqual([entityFrom('C'), entityFrom('D')]); + expect(entitiesResponseToObjects(response2.items)).toEqual([ + entityFrom('C'), + entityFrom('D'), + ]); expect(response2.pageInfo.nextCursor).toBeDefined(); expect(response2.pageInfo.prevCursor).toBeDefined(); expect(response2.totalItems).toBe(names.length); @@ -920,7 +926,10 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response3 = await catalog.queryEntities(request3); - expect(response3.items).toEqual([entityFrom('E'), entityFrom('F')]); + expect(entitiesResponseToObjects(response3.items)).toEqual([ + entityFrom('E'), + entityFrom('F'), + ]); expect(response3.pageInfo.nextCursor).toBeDefined(); expect(response3.pageInfo.prevCursor).toBeDefined(); expect(response3.totalItems).toBe(names.length); @@ -932,7 +941,10 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response4 = await catalog.queryEntities(request4); - expect(response4.items).toEqual([entityFrom('C'), entityFrom('D')]); + expect(entitiesResponseToObjects(response4.items)).toEqual([ + entityFrom('C'), + entityFrom('D'), + ]); expect(response4.pageInfo.nextCursor).toBeDefined(); expect(response4.pageInfo.prevCursor).toBeDefined(); expect(response4.totalItems).toBe(names.length); @@ -944,7 +956,10 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response5 = await catalog.queryEntities(request5); - expect(response5.items).toEqual([entityFrom('A'), entityFrom('B')]); + expect(entitiesResponseToObjects(response5.items)).toEqual([ + entityFrom('A'), + entityFrom('B'), + ]); expect(response5.pageInfo.nextCursor).toBeDefined(); expect(response5.pageInfo.prevCursor).toBeUndefined(); expect(response5.totalItems).toBe(names.length); @@ -956,7 +971,10 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response6 = await catalog.queryEntities(request6); - expect(response6.items).toEqual([entityFrom('C'), entityFrom('D')]); + expect(entitiesResponseToObjects(response6.items)).toEqual([ + entityFrom('C'), + entityFrom('D'), + ]); expect(response6.pageInfo.nextCursor).toBeDefined(); expect(response6.pageInfo.prevCursor).toBeDefined(); expect(response6.totalItems).toBe(names.length); @@ -968,7 +986,10 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response7 = await catalog.queryEntities(request7); - expect(response7.items).toEqual([entityFrom('E'), entityFrom('F')]); + expect(entitiesResponseToObjects(response7.items)).toEqual([ + entityFrom('E'), + entityFrom('F'), + ]); expect(response7.pageInfo.nextCursor).toBeDefined(); expect(response7.pageInfo.prevCursor).toBeDefined(); expect(response7.totalItems).toBe(names.length); @@ -980,7 +1001,7 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response7bis = await catalog.queryEntities(request7bis); - expect(response7bis.items).toEqual([ + expect(entitiesResponseToObjects(response7bis.items)).toEqual([ entityFrom('E'), entityFrom('F'), entityFrom('G'), @@ -996,7 +1017,9 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response8 = await catalog.queryEntities(request8); - expect(response8.items).toEqual([entityFrom('G')]); + expect(entitiesResponseToObjects(response8.items)).toEqual([ + entityFrom('G'), + ]); expect(response8.pageInfo.nextCursor).toBeUndefined(); expect(response8.pageInfo.prevCursor).toBeDefined(); expect(response8.totalItems).toBe(names.length); @@ -1050,7 +1073,10 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response1 = await catalog.queryEntities(request1); - expect(response1.items).toEqual([entityFrom('G'), entityFrom('F')]); + expect(entitiesResponseToObjects(response1.items)).toEqual([ + entityFrom('G'), + entityFrom('F'), + ]); expect(response1.pageInfo.nextCursor).toBeDefined(); expect(response1.pageInfo.prevCursor).toBeUndefined(); expect(response1.totalItems).toBe(names.length); @@ -1062,7 +1088,10 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response2 = await catalog.queryEntities(request2); - expect(response2.items).toEqual([entityFrom('E'), entityFrom('D')]); + expect(entitiesResponseToObjects(response2.items)).toEqual([ + entityFrom('E'), + entityFrom('D'), + ]); expect(response2.pageInfo.nextCursor).toBeDefined(); expect(response2.pageInfo.prevCursor).toBeDefined(); expect(response2.totalItems).toBe(names.length); @@ -1074,7 +1103,10 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response3 = await catalog.queryEntities(request3); - expect(response3.items).toEqual([entityFrom('C'), entityFrom('B')]); + expect(entitiesResponseToObjects(response3.items)).toEqual([ + entityFrom('C'), + entityFrom('B'), + ]); expect(response3.pageInfo.nextCursor).toBeDefined(); expect(response3.pageInfo.prevCursor).toBeDefined(); expect(response3.totalItems).toBe(names.length); @@ -1087,7 +1119,10 @@ describe('DefaultEntitiesCatalog', () => { }; const response4 = await catalog.queryEntities(request4); - expect(response4.items).toEqual([entityFrom('E'), entityFrom('D')]); + expect(entitiesResponseToObjects(response4.items)).toEqual([ + entityFrom('E'), + entityFrom('D'), + ]); expect(response4.pageInfo.nextCursor).toBeDefined(); expect(response4.pageInfo.prevCursor).toBeDefined(); expect(response4.totalItems).toBe(names.length); @@ -1099,7 +1134,10 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response5 = await catalog.queryEntities(request5); - expect(response5.items).toEqual([entityFrom('G'), entityFrom('F')]); + expect(entitiesResponseToObjects(response5.items)).toEqual([ + entityFrom('G'), + entityFrom('F'), + ]); expect(response5.pageInfo.nextCursor).toBeDefined(); expect(response5.pageInfo.prevCursor).toBeUndefined(); expect(response5.totalItems).toBe(names.length); @@ -1111,7 +1149,10 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response6 = await catalog.queryEntities(request6); - expect(response6.items).toEqual([entityFrom('E'), entityFrom('D')]); + expect(entitiesResponseToObjects(response6.items)).toEqual([ + entityFrom('E'), + entityFrom('D'), + ]); expect(response6.pageInfo.nextCursor).toBeDefined(); expect(response6.pageInfo.prevCursor).toBeDefined(); expect(response6.totalItems).toBe(names.length); @@ -1123,7 +1164,10 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response7 = await catalog.queryEntities(request7); - expect(response7.items).toEqual([entityFrom('C'), entityFrom('B')]); + expect(entitiesResponseToObjects(response7.items)).toEqual([ + entityFrom('C'), + entityFrom('B'), + ]); expect(response7.pageInfo.nextCursor).toBeDefined(); expect(response7.pageInfo.prevCursor).toBeDefined(); expect(response7.totalItems).toBe(names.length); @@ -1135,7 +1179,7 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response7bis = await catalog.queryEntities(request7bis); - expect(response7bis.items).toEqual([ + expect(entitiesResponseToObjects(response7bis.items)).toEqual([ entityFrom('C'), entityFrom('B'), entityFrom('A'), @@ -1151,7 +1195,9 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response8 = await catalog.queryEntities(request8); - expect(response8.items).toEqual([entityFrom('A')]); + expect(entitiesResponseToObjects(response8.items)).toEqual([ + entityFrom('A'), + ]); expect(response8.pageInfo.nextCursor).toBeUndefined(); expect(response8.pageInfo.prevCursor).toBeDefined(); expect(response8.totalItems).toBe(names.length); @@ -1203,7 +1249,7 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response = await catalog.queryEntities(request); - expect(response.items).toEqual([ + expect(entitiesResponseToObjects(response.items)).toEqual([ entityFrom('atcatss'), entityFrom('cat'), entityFrom('dogcat'), @@ -1261,7 +1307,7 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response = await catalog.queryEntities(request); - expect(response.items).toEqual(entities); + expect(entitiesResponseToObjects(response.items)).toEqual(entities); expect(response.pageInfo.nextCursor).toBeUndefined(); expect(response.pageInfo.prevCursor).toBeUndefined(); expect(response.totalItems).toBe(1); @@ -1316,7 +1362,7 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response = await catalog.queryEntities(request); - expect(response.items).toEqual([ + expect(entitiesResponseToObjects(response.items)).toEqual([ entityFrom('1', { uid: 'id1', title: 'cat' }), entityFrom('2', { uid: 'id2', title: 'atcatss' }), entityFrom('4', { uid: 'id4', title: 'dogcat' }), @@ -1329,7 +1375,7 @@ describe('DefaultEntitiesCatalog', () => { ...request, limit: 2, }); - expect(paginatedResponse.items).toEqual([ + expect(entitiesResponseToObjects(paginatedResponse.items)).toEqual([ entityFrom('1', { uid: 'id1', title: 'cat' }), entityFrom('2', { uid: 'id2', title: 'atcatss' }), ]); @@ -1341,7 +1387,7 @@ describe('DefaultEntitiesCatalog', () => { cursor: paginatedResponse.pageInfo.nextCursor!, credentials: mockCredentials.none(), }); - expect(paginatedResponseNext.items).toEqual([ + expect(entitiesResponseToObjects(paginatedResponseNext.items)).toEqual([ entityFrom('4', { uid: 'id4', title: 'dogcat' }), ]); expect(paginatedResponseNext.pageInfo.nextCursor).toBeUndefined(); @@ -1419,7 +1465,7 @@ describe('DefaultEntitiesCatalog', () => { }; const response = await catalog.queryEntities(request); - expect(response.items).toEqual([ + expect(entitiesResponseToObjects(response.items)).toEqual([ entityFrom('KingOfTheJungle', { uid: 'id0', title: 'lion' }), entityFrom('NotKingOfTheJungle', { uid: 'id1', title: 'cat' }), entityFrom('NotACatKing', { uid: 'id2', title: 'atcatss' }), @@ -1433,7 +1479,7 @@ describe('DefaultEntitiesCatalog', () => { ...request, limit: 2, }); - expect(paginatedResponse.items).toEqual([ + expect(entitiesResponseToObjects(paginatedResponse.items)).toEqual([ entityFrom('KingOfTheJungle', { uid: 'id0', title: 'lion' }), entityFrom('NotKingOfTheJungle', { uid: 'id1', title: 'cat' }), ]); @@ -1445,7 +1491,7 @@ describe('DefaultEntitiesCatalog', () => { cursor: paginatedResponse.pageInfo.nextCursor!, credentials: mockCredentials.none(), }); - expect(paginatedResponseNext.items).toEqual([ + expect(entitiesResponseToObjects(paginatedResponseNext.items)).toEqual([ entityFrom('NotACatKing', { uid: 'id2', title: 'atcatss' }), entityFrom('123', { uid: 'id3', title: 'king' }), ]); @@ -1489,7 +1535,11 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response = await catalog.queryEntities(request); - expect(response).toEqual({ totalItems: 20, items: [], pageInfo: {} }); + expect(response).toEqual({ + totalItems: 20, + items: { type: 'raw', entities: [] }, + pageInfo: {}, + }); }, ); @@ -1524,7 +1574,10 @@ describe('DefaultEntitiesCatalog', () => { let response = await catalog.queryEntities(request); expect(response).toEqual({ totalItems: 0, - items: expect.objectContaining({ length: 10 }), + items: { + type: 'raw', + entities: expect.objectContaining({ length: 10 }), + }, pageInfo: { nextCursor: expect.anything() }, }); response = await catalog.queryEntities({ @@ -1533,7 +1586,10 @@ describe('DefaultEntitiesCatalog', () => { }); expect(response).toEqual({ totalItems: 0, - items: expect.objectContaining({ length: 5 }), + items: { + type: 'raw', + entities: expect.objectContaining({ length: 5 }), + }, pageInfo: { prevCursor: expect.anything() }, }); }, @@ -1568,7 +1624,7 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response1 = await catalog.queryEntities(request1); - expect(response1.items).toMatchObject([ + expect(entitiesResponseToObjects(response1.items)).toMatchObject([ entityFrom('AA'), entityFrom('AA'), ]); @@ -1583,7 +1639,7 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response2 = await catalog.queryEntities(request2); - expect(response2.items).toMatchObject([ + expect(entitiesResponseToObjects(response2.items)).toMatchObject([ entityFrom('AA'), entityFrom('AA'), ]); @@ -1598,7 +1654,10 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response3 = await catalog.queryEntities(request3); - expect(response3.items).toEqual([entityFrom('CC'), entityFrom('DD')]); + expect(entitiesResponseToObjects(response3.items)).toEqual([ + entityFrom('CC'), + entityFrom('DD'), + ]); expect(response3.pageInfo.nextCursor).toBeUndefined(); expect(response3.pageInfo.prevCursor).toBeDefined(); expect(response3.totalItems).toBe(6); @@ -1610,7 +1669,7 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response4 = await catalog.queryEntities(request4); - expect(response4.items).toMatchObject([ + expect(entitiesResponseToObjects(response4.items)).toMatchObject([ entityFrom('AA'), entityFrom('AA'), ]); @@ -1625,7 +1684,7 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response5 = await catalog.queryEntities(request5); - expect(response5.items).toMatchObject([ + expect(entitiesResponseToObjects(response5.items)).toMatchObject([ entityFrom('AA'), entityFrom('AA'), ]); @@ -1693,7 +1752,7 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response1 = await catalog.queryEntities(request1); - expect(response1.items).toMatchObject([ + expect(entitiesResponseToObjects(response1.items)).toMatchObject([ entityFrom('AA', { uid: '1', kind: 'included' }), entityFrom('AA', { uid: '2', kind: 'included' }), ]); @@ -1708,7 +1767,7 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response2 = await catalog.queryEntities(request2); - expect(response2.items).toMatchObject([ + expect(entitiesResponseToObjects(response2.items)).toMatchObject([ entityFrom('AA', { uid: '4', kind: 'included' }), entityFrom('AA', { uid: '5', kind: 'included' }), ]); @@ -1752,7 +1811,7 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response1 = await catalog.queryEntities(request1); - expect(response1.items).toMatchObject([ + expect(entitiesResponseToObjects(response1.items)).toMatchObject([ entityFrom('AA'), entityFrom('CC'), ]); @@ -1767,7 +1826,7 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response2 = await catalog.queryEntities(request2); - expect(response2.items).toMatchObject([ + expect(entitiesResponseToObjects(response2.items)).toMatchObject([ entityFrom('DD'), entityFrom('AA', { namespace: 'namespace2' }), ]); @@ -1782,7 +1841,7 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response3 = await catalog.queryEntities(request3); - expect(response3.items).toMatchObject([ + expect(entitiesResponseToObjects(response3.items)).toMatchObject([ entityFrom('AA', { namespace: 'namespace3' }), entityFrom('AA', { namespace: 'namespace4' }), ]); @@ -1797,7 +1856,7 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response4 = await catalog.queryEntities(request4); - expect(response4.items).toMatchObject([ + expect(entitiesResponseToObjects(response4.items)).toMatchObject([ entityFrom('DD'), entityFrom('AA', { namespace: 'namespace2' }), ]); @@ -1812,7 +1871,7 @@ describe('DefaultEntitiesCatalog', () => { credentials: mockCredentials.none(), }; const response5 = await catalog.queryEntities(request5); - expect(response5.items).toMatchObject([ + expect(entitiesResponseToObjects(response5.items)).toMatchObject([ entityFrom('AA'), entityFrom('CC'), ]); From 8955923488c768a549b4cd508292c868b452fb41 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 12 Dec 2024 14:43:18 +0100 Subject: [PATCH 09/13] catalog-backend: update AuthorizedEntitiesCatalog tests with new response structure Signed-off-by: Patrik Oldsberg --- .../service/AuthorizedEntitiesCatalog.test.ts | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.test.ts b/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.test.ts index 2207802859..894949a691 100644 --- a/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.test.ts +++ b/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.test.ts @@ -64,7 +64,7 @@ describe('AuthorizedEntitiesCatalog', () => { credentials: mockCredentials.none(), }), ).toEqual({ - entities: [], + entities: { type: 'object', entities: [] }, pageInfo: { hasNextPage: false }, }); }); @@ -113,7 +113,7 @@ describe('AuthorizedEntitiesCatalog', () => { credentials: mockCredentials.none(), }), ).resolves.toEqual({ - items: [null], + items: { type: 'object', entities: [null] }, }); expect(fakeCatalog.entitiesBatch).not.toHaveBeenCalled(); @@ -174,7 +174,7 @@ describe('AuthorizedEntitiesCatalog', () => { filter: { key: 'kind', values: ['b'] }, }), ).resolves.toEqual({ - items: [], + items: { type: 'object', entities: [] }, pageInfo: {}, totalItems: 0, }); @@ -226,7 +226,7 @@ describe('AuthorizedEntitiesCatalog', () => { ]; fakeCatalog.queryEntities.mockResolvedValue({ - items: { type: 'objects', entities }, + items: { type: 'object', entities }, pageInfo: { nextCursor: { isPrevious: false, @@ -254,7 +254,7 @@ describe('AuthorizedEntitiesCatalog', () => { }); expect(response).toEqual({ - items: entities, + items: { type: 'object', entities: entities }, totalItems: 4, pageInfo: { nextCursor: { @@ -290,7 +290,7 @@ describe('AuthorizedEntitiesCatalog', () => { }); expect(response).toEqual({ - items: entities, + items: { type: 'object', entities: entities }, totalItems: 4, pageInfo: { nextCursor: { @@ -338,7 +338,9 @@ describe('AuthorizedEntitiesCatalog', () => { conditions: { rule: 'IS_ENTITY_KIND', params: { kinds: ['b'] } }, }, ]); - fakeCatalog.entities.mockResolvedValue({ entities: [] }); + fakeCatalog.entities.mockResolvedValue({ + entities: { type: 'object', entities: [] }, + }); const catalog = new AuthorizedEntitiesCatalog( fakeCatalog, fakePermissionApi, @@ -360,7 +362,10 @@ describe('AuthorizedEntitiesCatalog', () => { }, ]); fakeCatalog.entities.mockResolvedValue({ - entities: [{ kind: 'b', namespace: 'default', name: 'my-component' }], + entities: { + type: 'object', + entities: [{ kind: 'b', namespace: 'default', name: 'my-component' }], + }, }); const catalog = new AuthorizedEntitiesCatalog( fakeCatalog, From 14ce4026ed3d0753cccd7c06f4ed3090ea6f3605 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 13 Dec 2024 00:53:50 +0100 Subject: [PATCH 10/13] catalog-backend: rename enableRawJsonRespone -> disableRelationsCompatibility Signed-off-by: Patrik Oldsberg --- .changeset/curly-teachers-marry.md | 2 +- plugins/catalog-backend/config.d.ts | 10 ++++++++++ plugins/catalog-backend/src/service/CatalogBuilder.ts | 8 ++++---- .../src/service/DefaultEntitiesCatalog.ts | 10 ++++++---- .../catalog-backend/src/service/createRouter.test.ts | 2 +- plugins/catalog-backend/src/service/createRouter.ts | 6 +++--- plugins/catalog-backend/src/tests/integration.test.ts | 6 +++--- 7 files changed, 28 insertions(+), 16 deletions(-) diff --git a/.changeset/curly-teachers-marry.md b/.changeset/curly-teachers-marry.md index 21f41da291..ad22613612 100644 --- a/.changeset/curly-teachers-marry.md +++ b/.changeset/curly-teachers-marry.md @@ -2,4 +2,4 @@ '@backstage/plugin-catalog-backend': minor --- -Added a new `catalog.enableRawJsonResponse` configuration option that avoids JSON deserialization and serialization if possible when reading entities. This can significantly improve the overall performance of the catalog, but it removes the backwards compatibility processing that ensures that both `entity.relation[].target` and `entity.relation[].targetRef` are present in returned entities. +Added a new `catalog.disableRelationsCompatibility` configuration option that avoids JSON deserialization and serialization if possible when reading entities. This can significantly improve the overall performance of the catalog, but it removes the backwards compatibility processing that ensures that both `entity.relation[].target` and `entity.relation[].targetRef` are present in returned entities. diff --git a/plugins/catalog-backend/config.d.ts b/plugins/catalog-backend/config.d.ts index 8dd5e50480..be6104ac84 100644 --- a/plugins/catalog-backend/config.d.ts +++ b/plugins/catalog-backend/config.d.ts @@ -138,6 +138,16 @@ export interface Config { }>; }>; + /** + * Disables the compatibility layer for relations in returned entities that + * ensures that all relations objects have both `target` and `targetRef`. + * + * Enabling this option can very significantly improve the performance of + * the catalog, but may break consumers that rely on the existence of + * `target` in the relations objects. + */ + disableRelationsCompatibility?: boolean; + /** * The strategy to use for entities that are orphaned, i.e. no longer have * any other entities or providers referencing them. The default value is diff --git a/plugins/catalog-backend/src/service/CatalogBuilder.ts b/plugins/catalog-backend/src/service/CatalogBuilder.ts index fd052bf474..7ce308e796 100644 --- a/plugins/catalog-backend/src/service/CatalogBuilder.ts +++ b/plugins/catalog-backend/src/service/CatalogBuilder.ts @@ -486,8 +486,8 @@ export class CatalogBuilder { discovery, }); - const enableRawJson = config.getOptionalBoolean( - 'catalog.enableRawJsonResponses', + const disableRelationsCompatibility = config.getOptionalBoolean( + 'catalog.disableRelationsCompatibility', ); const policy = this.buildEntityPolicy(); @@ -526,7 +526,7 @@ export class CatalogBuilder { database: dbClient, logger, stitcher, - enableRawJson, + disableRelationsCompatibility, }); let permissionsService: PermissionsService; @@ -638,7 +638,7 @@ export class CatalogBuilder { auth, httpAuth, permissionsService, - enableRawJson, + disableRelationsCompatibility, }); await connectEntityProviders(providerDatabase, entityProviders); diff --git a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts index ee9b46ce55..f0e0667bef 100644 --- a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts @@ -105,18 +105,20 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { private readonly database: Knex; private readonly logger: LoggerService; private readonly stitcher: Stitcher; - private readonly enableRawJson: boolean; + private readonly disableRelationsCompatibility: boolean; constructor(options: { database: Knex; logger: LoggerService; stitcher: Stitcher; - enableRawJson?: boolean; + disableRelationsCompatibility?: boolean; }) { this.database = options.database; this.logger = options.logger; this.stitcher = options.stitcher; - this.enableRawJson = Boolean(options.enableRawJson); + this.disableRelationsCompatibility = Boolean( + options.disableRelationsCompatibility, + ); } async entities(request?: EntitiesRequest): Promise { @@ -197,7 +199,7 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { return { entities: processRawEntitiesResult( rows.map(r => r.final_entity!), - this.enableRawJson + this.disableRelationsCompatibility ? request?.fields : e => { expandLegacyCompoundRelationsInEntity(e); diff --git a/plugins/catalog-backend/src/service/createRouter.test.ts b/plugins/catalog-backend/src/service/createRouter.test.ts index 312900bcf1..53f831d320 100644 --- a/plugins/catalog-backend/src/service/createRouter.test.ts +++ b/plugins/catalog-backend/src/service/createRouter.test.ts @@ -883,7 +883,7 @@ describe('createRouter readonly and raw json enabled', () => { getLocationByEntity: jest.fn(), }; const router = await createRouter({ - enableRawJson: true, + disableRelationsCompatibility: true, entitiesCatalog, locationService, logger: mockServices.logger.mock(), diff --git a/plugins/catalog-backend/src/service/createRouter.ts b/plugins/catalog-backend/src/service/createRouter.ts index 6ea9ea1c13..84826267f2 100644 --- a/plugins/catalog-backend/src/service/createRouter.ts +++ b/plugins/catalog-backend/src/service/createRouter.ts @@ -85,7 +85,7 @@ export interface RouterOptions { auth: AuthService; httpAuth: HttpAuthService; permissionsService: PermissionsService; - enableRawJson?: boolean; + disableRelationsCompatibility?: boolean; } /** @@ -113,7 +113,7 @@ export async function createRouter( permissionsService, auth, httpAuth, - enableRawJson = false, + disableRelationsCompatibility = false, } = options; const readonlyEnabled = @@ -215,7 +215,7 @@ export async function createRouter( signal.throwIfAborted(); - if (!enableRawJson) { + if (!disableRelationsCompatibility) { processEntitiesResponseItems( result.items, expandLegacyCompoundRelationsInEntity, diff --git a/plugins/catalog-backend/src/tests/integration.test.ts b/plugins/catalog-backend/src/tests/integration.test.ts index 32116ace0d..e85ebe48ce 100644 --- a/plugins/catalog-backend/src/tests/integration.test.ts +++ b/plugins/catalog-backend/src/tests/integration.test.ts @@ -198,7 +198,7 @@ class TestHarness { readonly #proxyProgressTracker: ProxyProgressTracker; static async create(options?: { - enableRawJson?: boolean; + disableRelationsCompatibility?: boolean; logger?: LoggerService; db?: Knex; permissions?: PermissionEvaluator; @@ -277,7 +277,7 @@ class TestHarness { database: db, logger, stitcher, - enableRawJson: options?.enableRawJson, + disableRelationsCompatibility: options?.disableRelationsCompatibility, }); const proxyProgressTracker = new ProxyProgressTracker( new NoopProgressTracker(), @@ -788,7 +788,7 @@ describe('Catalog Backend Integration', () => { it('should return valid responses in raw JSON mode', async () => { const harness = await TestHarness.create({ - enableRawJson: true, + disableRelationsCompatibility: true, }); const entityA = { From 38436041d3158a6b5249e848d2a68af68589b315 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 13 Dec 2024 10:50:09 +0100 Subject: [PATCH 11/13] catalog-backend: add test for response writing and properly stop on close Signed-off-by: Patrik Oldsberg --- .../src/service/response/write.test.ts | 36 +++++++++++++++++++ .../src/service/response/write.ts | 24 ++++++++----- 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/plugins/catalog-backend/src/service/response/write.test.ts b/plugins/catalog-backend/src/service/response/write.test.ts index b1a688b48d..fdca874904 100644 --- a/plugins/catalog-backend/src/service/response/write.test.ts +++ b/plugins/catalog-backend/src/service/response/write.test.ts @@ -277,5 +277,41 @@ describe('writeEntitiesResponse', () => { totalItems: 1337, }); }); + + it('should write a large wrapped response', async () => { + const entityMock = JSON.stringify({ + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + name: 'my-component', + namespace: 'default', + annotations: { + 'backstage.io/managed-by-location': 'url:https://example.com', + }, + }, + spec: { + type: 'service', + owner: 'me', + lifecycle: 'production', + }, + }); + const res = await request(app) + .get('/wrapped') + .send({ + type: 'raw', + entities: Array(300).fill(entityMock), + }); + + expect(res.status).toBe(200); + expect(res.type).toBe('application/json'); + expect(res.header['content-type']).toBe( + 'application/json; charset=utf-8', + ); + expect(res.body).toEqual({ + page: 1, + items: expect.objectContaining({ length: 300 }), + totalItems: 1337, + }); + }); }); }); diff --git a/plugins/catalog-backend/src/service/response/write.ts b/plugins/catalog-backend/src/service/response/write.ts index 0962ee15d1..c2bca0953a 100644 --- a/plugins/catalog-backend/src/service/response/write.ts +++ b/plugins/catalog-backend/src/service/response/write.ts @@ -80,15 +80,23 @@ export async function writeEntitiesResponse( const needsDrain = !res.write(prefix + entity, 'utf8'); if (needsDrain) { - await new Promise(resolve => { - const cont = () => { - res.off('drain', cont); - res.off('close', cont); - resolve(); - }; - res.on('drain', cont); - res.on('close', cont); + const closed = await new Promise(resolve => { + function onContinue() { + res.off('drain', onContinue); + res.off('close', onClose); + resolve(false); + } + function onClose() { + res.off('drain', onContinue); + res.off('close', onClose); + resolve(true); + } + res.on('drain', onContinue); + res.on('close', onClose); }); + if (closed) { + return; + } } } res.end(`${first ? '[' : ''}]${trailing}`); From a86d259430a7c8d83a87454fe19df621ad17a6b4 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 13 Dec 2024 11:58:09 +0100 Subject: [PATCH 12/13] catalog-backend: fix relations compat flag not being applied Signed-off-by: Patrik Oldsberg --- plugins/catalog-backend/src/service/createRouter.ts | 2 +- plugins/catalog-backend/src/service/response/process.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/catalog-backend/src/service/createRouter.ts b/plugins/catalog-backend/src/service/createRouter.ts index 84826267f2..601b0b1098 100644 --- a/plugins/catalog-backend/src/service/createRouter.ts +++ b/plugins/catalog-backend/src/service/createRouter.ts @@ -216,7 +216,7 @@ export async function createRouter( signal.throwIfAborted(); if (!disableRelationsCompatibility) { - processEntitiesResponseItems( + result.items = processEntitiesResponseItems( result.items, expandLegacyCompoundRelationsInEntity, ); diff --git a/plugins/catalog-backend/src/service/response/process.ts b/plugins/catalog-backend/src/service/response/process.ts index 7e7730d697..fa07063f5b 100644 --- a/plugins/catalog-backend/src/service/response/process.ts +++ b/plugins/catalog-backend/src/service/response/process.ts @@ -39,7 +39,7 @@ export function processRawEntitiesResult( export function processEntitiesResponseItems( response: EntitiesResponseItems, transform?: (entity: Entity) => Entity, -) { +): EntitiesResponseItems { if (!transform) { return response; } From 5dcb0f3b7d975046c5ab40711fc6eb5c74ab99cd Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 13 Dec 2024 13:30:07 +0100 Subject: [PATCH 13/13] catalog-backend: update description of disableRelationsCompatibility post testing Signed-off-by: Patrik Oldsberg --- .changeset/curly-teachers-marry.md | 2 +- plugins/catalog-backend/config.d.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.changeset/curly-teachers-marry.md b/.changeset/curly-teachers-marry.md index ad22613612..9bf771f4d1 100644 --- a/.changeset/curly-teachers-marry.md +++ b/.changeset/curly-teachers-marry.md @@ -2,4 +2,4 @@ '@backstage/plugin-catalog-backend': minor --- -Added a new `catalog.disableRelationsCompatibility` configuration option that avoids JSON deserialization and serialization if possible when reading entities. This can significantly improve the overall performance of the catalog, but it removes the backwards compatibility processing that ensures that both `entity.relation[].target` and `entity.relation[].targetRef` are present in returned entities. +Added a new `catalog.disableRelationsCompatibility` configuration option that avoids JSON deserialization and serialization if possible when reading entities. This significantly reduces the memory usage of the catalog, and slightly increases performance, but it removes the backwards compatibility processing that ensures that both `entity.relation[].target` and `entity.relation[].targetRef` are present in returned entities. diff --git a/plugins/catalog-backend/config.d.ts b/plugins/catalog-backend/config.d.ts index be6104ac84..384765f92e 100644 --- a/plugins/catalog-backend/config.d.ts +++ b/plugins/catalog-backend/config.d.ts @@ -142,9 +142,9 @@ export interface Config { * Disables the compatibility layer for relations in returned entities that * ensures that all relations objects have both `target` and `targetRef`. * - * Enabling this option can very significantly improve the performance of - * the catalog, but may break consumers that rely on the existence of - * `target` in the relations objects. + * Enabling this option significantly reduces the memory usage of the + * catalog, and slightly increases performance, but may break consumers that + * rely on the existence of `target` in the relations objects. */ disableRelationsCompatibility?: boolean;