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 = '';