chore(catalog): tweak getEntityByName a bit, handle 404s properly

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<Location | undefined> {
-    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<any> {
+    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<any | undefined> {
+    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<Location | undefined> {
+    return await this.getOptional(`/locations/${id}`);
+  }
+
   async getEntities(
     filter?: Record<string, string>,
   ): Promise<DescriptorEnvelope[]> {
@@ -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<DescriptorEnvelope> {
-    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<Entity | undefined> {
+    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<Location | undefined> {
-    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}`);
   }
 }

plugins/catalog/src/api/types.ts

@@ -22,13 +22,17 @@ export const catalogApiRef = createApiRef<CatalogApi>({
     '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<Entity>;
   getLocationById(id: String): Promise<Location | undefined>;
+  getEntityByName(
+    compoundName: EntityCompoundName,
+  ): Promise<Entity | undefined>;
   getEntities(filter?: Record<string, string>): Promise<Entity[]>;
   addLocation(type: string, target: string): Promise<AddLocationResponse>;
   getLocationByEntity(entity: Entity): Promise<Location | undefined>;

plugins/catalog/src/components/ComponentPage/ComponentPage.tsx

@@ -59,7 +59,10 @@ export const ComponentPage: FC<ComponentPageProps> = ({ match, history }) => {
 
   const catalogApi = useApi(catalogApiRef);
   const { value: component, error, loading } = useAsync<Component>(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 };
   });

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<typeof catalogApiRef.T> = {
   /* 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 = () => ({
     </MemoryRouter>,
   ),
 });
+
 describe('RegisterComponentPage', () => {
   afterEach(() => cleanup());