From 36d4dfcfa7fb0c4869ad35cdf4c9c72b2dcae00e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 10 Jun 2020 09:00:01 +0200 Subject: [PATCH] chore(catalog): tweak getEntityByName a bit, handle 404s properly --- plugins/catalog/src/api/CatalogClient.ts | 99 ++++++++++--------- plugins/catalog/src/api/types.ts | 14 ++- .../ComponentPage/ComponentPage.tsx | 5 +- .../RegisterComponentPage.test.tsx | 4 +- 4 files changed, 66 insertions(+), 56 deletions(-) diff --git a/plugins/catalog/src/api/CatalogClient.ts b/plugins/catalog/src/api/CatalogClient.ts index 1d88a57ecc..5717735d9f 100644 --- a/plugins/catalog/src/api/CatalogClient.ts +++ b/plugins/catalog/src/api/CatalogClient.ts @@ -14,14 +14,14 @@ * limitations under the License. */ -import { CatalogApi } from './types'; -import { DescriptorEnvelope } from '../types'; import { Entity, Location, LOCATION_ANNOTATION, } from '@backstage/catalog-model'; import Cache from 'node-cache'; +import { DescriptorEnvelope } from '../types'; +import { CatalogApi, EntityCompoundName } from './types'; export class CatalogClient implements CatalogApi { // TODO(blam): This cache is just temporary until we have GraphQL. @@ -30,6 +30,7 @@ export class CatalogClient implements CatalogApi { private cache: Cache; private apiOrigin: string; private basePath: string; + constructor({ apiOrigin, basePath, @@ -41,16 +42,41 @@ export class CatalogClient implements CatalogApi { this.basePath = basePath; this.cache = new Cache({ stdTTL: 10 }); } - async getLocationById(id: String): Promise { - const response = await fetch( - `${this.apiOrigin}${this.basePath}/locations/${id}`, - ); - if (response.ok) { - const location = await response.json(); - if (location) return location.data; + + private async getRequired(path: string): Promise { + const url = `${this.apiOrigin}${this.basePath}${path}`; + const response = await fetch(url); + + if (!response.ok) { + const payload = await response.text(); + const message = `Request failed with ${response.status} ${response.statusText}, ${payload}`; + throw new Error(message); } - return undefined; + + return await response.json(); } + + private async getOptional(path: string): Promise { + const url = `${this.apiOrigin}${this.basePath}${path}`; + const response = await fetch(url); + + if (!response.ok) { + if (response.status === 404) { + return undefined; + } + + const payload = await response.text(); + const message = `Request failed with ${response.status} ${response.statusText}, ${payload}`; + throw new Error(message); + } + + return await response.json(); + } + + async getLocationById(id: String): Promise { + return await this.getOptional(`/locations/${id}`); + } + async getEntities( filter?: Record, ): Promise { @@ -59,49 +85,25 @@ export class CatalogClient implements CatalogApi { ); if (cachedValue) return cachedValue; - let url = `${this.apiOrigin}${this.basePath}/entities`; + let path = `/entities`; if (filter) { - url += '?'; - url += Object.entries(filter) + path += '?'; + path += Object.entries(filter) .map( ([key, value]) => `${encodeURIComponent(key)}=${encodeURIComponent(value)}`, ) .join('&'); } - const response = await fetch(url); - if (!response.ok) { - const payload = await response.text(); - throw new Error( - `Request failed with ${response.status} ${response.statusText}, ${payload}`, - ); - } - const value = await response.json(); - if (value?.length) { - this.cache.set(`get:${JSON.stringify(filter)}`, value); - } - - return value; + return await this.getRequired(path); } - async getEntity({ - name, - namespace, - kind, - }: { - name: string; - namespace?: string; - kind: string; - }): Promise { - const response = await fetch( - `${this.apiOrigin}${this.basePath}/entities/by-name/${kind}/${ - namespace ?? 'default' - }/${name}`, - ); - const entity = await response.json(); - if (entity) return entity; - throw new Error(`'Entity not found: ${name}`); + async getEntityByName( + compoundName: EntityCompoundName, + ): Promise { + const { kind, namespace = 'default', name } = compoundName; + return this.getOptional(`/entities/by-name/${kind}/${namespace}/${name}`); } async addLocation(type: string, target: string) { @@ -132,11 +134,10 @@ export class CatalogClient implements CatalogApi { } async getLocationByEntity(entity: Entity): Promise { - const locationId = entity.metadata.annotations?.[LOCATION_ANNOTATION]; - if (!locationId) return undefined; - - const location = this.getLocationById(locationId); - - return location; + const locationCompound = entity.metadata.annotations?.[LOCATION_ANNOTATION]; + const all: { data: Location }[] = await this.getRequired('/locations'); + return all + .map(r => r.data) + .find(l => locationCompound === `${l.type}:${l.target}`); } } diff --git a/plugins/catalog/src/api/types.ts b/plugins/catalog/src/api/types.ts index bcaa340036..9edf0358d1 100644 --- a/plugins/catalog/src/api/types.ts +++ b/plugins/catalog/src/api/types.ts @@ -22,13 +22,17 @@ export const catalogApiRef = createApiRef({ 'Used by the Catalog plugin to make requests to accompanying backend', }); +export type EntityCompoundName = { + kind: string; + namespace?: string; + name: string; +}; + export interface CatalogApi { - getEntity(params: { - name: string; - namespace?: string; - kind: string; - }): Promise; getLocationById(id: String): Promise; + getEntityByName( + compoundName: EntityCompoundName, + ): Promise; getEntities(filter?: Record): Promise; addLocation(type: string, target: string): Promise; getLocationByEntity(entity: Entity): Promise; diff --git a/plugins/catalog/src/components/ComponentPage/ComponentPage.tsx b/plugins/catalog/src/components/ComponentPage/ComponentPage.tsx index f487926183..3f3ecfa006 100644 --- a/plugins/catalog/src/components/ComponentPage/ComponentPage.tsx +++ b/plugins/catalog/src/components/ComponentPage/ComponentPage.tsx @@ -59,7 +59,10 @@ export const ComponentPage: FC = ({ match, history }) => { const catalogApi = useApi(catalogApiRef); const { value: component, error, loading } = useAsync(async () => { - const entity = await catalogApi.getEntity({ name, namespace, kind }); + const entity = await catalogApi.getEntityByName({ name, namespace, kind }); + if (!entity) { + throw new Error(`No entity found with that name`); + } const location = await catalogApi.getLocationByEntity(entity); return { ...entityToComponent(entity), location }; }); diff --git a/plugins/register-component/src/components/RegisterComponentPage/RegisterComponentPage.test.tsx b/plugins/register-component/src/components/RegisterComponentPage/RegisterComponentPage.test.tsx index 4da68fe343..0c394d3b60 100644 --- a/plugins/register-component/src/components/RegisterComponentPage/RegisterComponentPage.test.tsx +++ b/plugins/register-component/src/components/RegisterComponentPage/RegisterComponentPage.test.tsx @@ -24,11 +24,12 @@ import { catalogApiRef } from '@backstage/plugin-catalog'; import { MemoryRouter } from 'react-router-dom'; const errorApi = { post: () => {} }; + const catalogApi: jest.Mocked = { /* eslint-disable-next-line @typescript-eslint/no-unused-vars */ addLocation: jest.fn((_a, _b) => new Promise(() => {})), getEntities: jest.fn(), - getEntity: jest.fn(), + getEntityByName: jest.fn(), getLocationByEntity: jest.fn(), getLocationById: jest.fn(), }; @@ -49,6 +50,7 @@ const setup = () => ({ , ), }); + describe('RegisterComponentPage', () => { afterEach(() => cleanup());