Merge pull request #7186 from backstage/mob/catalog-import-refresh

Add refresh support for catalog-import

.changeset/lovely-cycles-shout.md

@@ -0,0 +1,5 @@
+---
+'@backstage/catalog-client': patch
+---
+
+Update `AddLocationResponse` to optionally return `exists` to signal that the location already exists, this is only returned when calling `addLocation` in dryRun.

.changeset/lovely-keys-occur.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-catalog-backend': patch
+---
+
+Update `createLocation` to optionally return `exists` to signal that the location already exists, this is only returned for dry runs.

.changeset/unlucky-laws-divide.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-catalog-import': patch
+---
+
+The import form is now aware of locations that already exist. It lists them separately and shows a button for triggering a refresh.

packages/catalog-client/api-report.md

@@ -19,6 +19,7 @@ export type AddLocationRequest = {
 export type AddLocationResponse = {
   location: Location_2;
   entities: Entity[];
+  exists?: boolean;
 };
 
 // @public (undocumented)

packages/catalog-client/src/CatalogClient.ts

@@ -176,7 +176,7 @@ export class CatalogClient implements CatalogApi {
       throw new Error(await response.text());
     }
 
-    const { location, entities } = await response.json();
+    const { location, entities, exists } = await response.json();
 
     if (!location) {
       throw new Error(`Location wasn't added: ${target}`);
@@ -185,6 +185,7 @@ export class CatalogClient implements CatalogApi {
     return {
       location,
       entities,
+      exists,
     };
   }
 

packages/catalog-client/src/types/api.ts

@@ -93,4 +93,6 @@ export type AddLocationRequest = {
 export type AddLocationResponse = {
   location: Location;
   entities: Entity[];
+  // Exists is only set in DryRun mode.
+  exists?: boolean;
 };

plugins/catalog-backend/api-report.md

@@ -1148,6 +1148,7 @@ export interface LocationService {
   ): Promise<{
     location: Location_2;
     entities: Entity[];
+    exists?: boolean;
   }>;
   // (undocumented)
   deleteLocation(id: string): Promise<void>;

plugins/catalog-backend/src/next/DefaultLocationService.test.ts

@@ -36,6 +36,7 @@ describe('DefaultLocationServiceTest', () => {
 
   describe('createLocation', () => {
     it('should support dry run', async () => {
+      store.listLocations.mockResolvedValueOnce([]);
       orchestrator.process.mockResolvedValueOnce({
         ok: true,
         state: new Map(),
@@ -116,6 +117,63 @@ describe('DefaultLocationServiceTest', () => {
       expect(store.createLocation).not.toBeCalled();
     });
 
+    it('should check for location existence when running in dry run', async () => {
+      const locationSpec = {
+        type: 'url',
+        target: 'https://backstage.io/catalog-info.yaml',
+      };
+      orchestrator.process.mockResolvedValueOnce({
+        ok: true,
+        state: new Map(),
+        completedEntity: {
+          apiVersion: 'backstage.io/v1alpha1',
+          kind: 'Component',
+          metadata: {
+            name: 'bar',
+          },
+        },
+        deferredEntities: [],
+        relations: [],
+        errors: [],
+      });
+
+      store.listLocations.mockResolvedValueOnce([
+        { id: '137', ...locationSpec },
+      ]);
+      const result = await locationService.createLocation(
+        { type: 'url', target: 'https://backstage.io/catalog-info.yaml' },
+        true,
+      );
+
+      expect(result.exists).toBe(true);
+    });
+
+    it('should return exists false when the location does not exist beforehand', async () => {
+      orchestrator.process.mockResolvedValueOnce({
+        ok: true,
+        state: new Map(),
+        completedEntity: {
+          apiVersion: 'backstage.io/v1alpha1',
+          kind: 'Component',
+          metadata: {
+            name: 'bar',
+          },
+        },
+        deferredEntities: [],
+        relations: [],
+        errors: [],
+      });
+
+      store.listLocations.mockResolvedValueOnce([
+        { id: '987', type: 'url', target: 'https://example.com' },
+      ]);
+      const result = await locationService.createLocation(
+        { type: 'url', target: 'https://backstage.io/catalog-info.yaml' },
+        true,
+      );
+      expect(result.exists).toBe(false);
+    });
+
     it('should create location', async () => {
       const locationSpec = {
         type: 'url',

plugins/catalog-backend/src/next/DefaultLocationService.ts

@@ -36,7 +36,7 @@ export class DefaultLocationService implements LocationService {
   async createLocation(
     spec: LocationSpec,
     dryRun: boolean,
-  ): Promise<{ location: Location; entities: Entity[] }> {
+  ): Promise<{ location: Location; entities: Entity[]; exists?: boolean }> {
     if (dryRun) {
       return this.dryRunCreateLocation(spec);
     }
@@ -56,7 +56,14 @@ export class DefaultLocationService implements LocationService {
 
   private async dryRunCreateLocation(
     spec: LocationSpec,
-  ): Promise<{ location: Location; entities: Entity[] }> {
+  ): Promise<{ location: Location; entities: Entity[]; exists?: boolean }> {
+    // Run the existence check in parallel with the processing
+    const existsPromise = this.store
+      .listLocations()
+      .then(locations =>
+        locations.some(l => l.type === spec.type && l.target === spec.target),
+      );
+
     const entity = {
       apiVersion: 'backstage.io/v1alpha1',
       kind: 'Location',
@@ -100,6 +107,7 @@ export class DefaultLocationService implements LocationService {
     }
 
     return {
+      exists: await existsPromise,
       location: { ...spec, id: `${spec.type}:${spec.target}` },
       entities,
     };

plugins/catalog-backend/src/next/types.ts

@@ -21,7 +21,7 @@ export interface LocationService {
   createLocation(
     spec: LocationSpec,
     dryRun: boolean,
-  ): Promise<{ location: Location; entities: Entity[] }>;
+  ): Promise<{ location: Location; entities: Entity[]; exists?: boolean }>;
   listLocations(): Promise<Location[]>;
   getLocation(id: string): Promise<Location>;
   deleteLocation(id: string): Promise<void>;

plugins/catalog-import/api-report.md

@@ -34,6 +34,7 @@ export type AnalyzeResult =
       type: 'locations';
       locations: Array<{
         target: string;
+        exists?: boolean;
         entities: EntityName[];
       }>;
     }
@@ -248,5 +249,5 @@ export const StepPrepareCreatePullRequest: ({
 
 // Warnings were encountered during analysis:
 //
-// src/api/CatalogImportApi.d.ts:14:5 - (ae-forgotten-export) The symbol "PartialEntity" needs to be exported by the entry point index.d.ts
+// src/api/CatalogImportApi.d.ts:15:5 - (ae-forgotten-export) The symbol "PartialEntity" needs to be exported by the entry point index.d.ts
 ```

plugins/catalog-import/package.json

@@ -52,7 +52,8 @@
     "react-router": "6.0.0-beta.0",
     "react-router-dom": "6.0.0-beta.0",
     "react-use": "^17.2.4",
-    "yaml": "^1.10.0"
+    "yaml": "^1.10.0",
+    "lodash": "^4.17.21"
   },
   "devDependencies": {
     "@backstage/cli": "^0.7.12",

plugins/catalog-import/src/api/CatalogImportApi.ts

@@ -29,6 +29,7 @@ export type AnalyzeResult =
       type: 'locations';
       locations: Array<{
         target: string;
+        exists?: boolean;
         entities: EntityName[];
       }>;
     }

plugins/catalog-import/src/api/CatalogImportClient.ts

@@ -31,6 +31,7 @@ import { Base64 } from 'js-base64';
 import { PartialEntity } from '../types';
 import { AnalyzeResult, CatalogImportApi } from './CatalogImportApi';
 import { getGithubIntegrationConfig } from './GitHub';
+import { trimEnd } from 'lodash';
 
 export class CatalogImportClient implements CatalogImportApi {
   private readonly discoveryApi: DiscoveryApi;
@@ -71,6 +72,7 @@ export class CatalogImportClient implements CatalogImportApi {
         type: 'locations',
         locations: [
           {
+            exists: location.exists,
             target: location.location.target,
             entities: location.entities.map(e => ({
               kind: e.kind,
@@ -238,29 +240,23 @@ the component will become available.\n\nFor more information, read an \
 
       return await Promise.all(
         searchResult.data.items
-          .map(
-            i => `${url.replace(/[\/]*$/, '')}/blob/${defaultBranch}/${i.path}`,
-          )
-          .map(
-            async i =>
-              ({
-                target: i,
-                entities: (
-                  await this.catalogApi.addLocation({
-                    type: 'url',
-                    target: i,
-                    dryRun: true,
-                  })
-                ).entities.map(e => ({
-                  kind: e.kind,
-                  namespace: e.metadata.namespace ?? 'default',
-                  name: e.metadata.name,
-                })),
-              } as {
-                target: string;
-                entities: EntityName[];
-              }),
-          ),
+          .map(i => `${trimEnd(url, '/')}/blob/${defaultBranch}/${i.path}`)
+          .map(async target => {
+            const result = await this.catalogApi.addLocation({
+              type: 'url',
+              target,
+              dryRun: true,
+            });
+            return {
+              target,
+              exists: result.exists,
+              entities: result.entities.map(e => ({
+                kind: e.kind,
+                namespace: e.metadata.namespace ?? 'default',
+                name: e.metadata.name,
+              })),
+            };
+          }),
       );
     }
 

plugins/catalog-import/src/components/StepFinishImportLocation/StepFinishImportLocation.tsx

@@ -21,15 +21,22 @@ import { BackButton } from '../Buttons';
 import { EntityListComponent } from '../EntityListComponent';
 import { PrepareResult } from '../useImportState';
 import { Link } from '@backstage/core-components';
+import partition from 'lodash/partition';
 
 type Props = {
   prepareResult: PrepareResult;
   onReset: () => void;
 };
 
-export const StepFinishImportLocation = ({ prepareResult, onReset }: Props) => (
-  <>
-    {prepareResult.type === 'repository' && (
+export const StepFinishImportLocation = ({ prepareResult, onReset }: Props) => {
+  const continueButton = (
+    <Grid container spacing={0}>
+      <BackButton onClick={onReset}>Register another</BackButton>
+    </Grid>
+  );
+
+  if (prepareResult.type === 'repository') {
+    return (
       <>
         <Typography paragraph>
           The following Pull Request has been opened:{' '}
@@ -45,21 +52,46 @@ export const StepFinishImportLocation = ({ prepareResult, onReset }: Props) => (
         <Typography paragraph>
           Your entities will be imported as soon as the Pull Request is merged.
         </Typography>
+
+        {continueButton}
       </>
-    )}
+    );
+  }
 
-    <Typography>
-      The following entities have been added to the catalog:
-    </Typography>
+  const [existingLocations, newLocations] = partition(
+    prepareResult.locations,
+    l => l.exists,
+  );
 
-    <EntityListComponent
-      locations={prepareResult.locations}
-      locationListItemIcon={() => <LocationOnIcon />}
-      withLinks
-    />
+  return (
+    <>
+      {newLocations.length > 0 && (
+        <>
+          <Typography>
+            The following entities have been added to the catalog:
+          </Typography>
 
-    <Grid container spacing={0}>
-      <BackButton onClick={onReset}>Register another</BackButton>
-    </Grid>
-  </>
-);
+          <EntityListComponent
+            locations={newLocations}
+            locationListItemIcon={() => <LocationOnIcon />}
+            withLinks
+          />
+        </>
+      )}
+      {existingLocations.length > 0 && (
+        <>
+          <Typography>
+            A refresh was triggered for the following locations:
+          </Typography>
+
+          <EntityListComponent
+            locations={existingLocations}
+            locationListItemIcon={() => <LocationOnIcon />}
+            withLinks
+          />
+        </>
+      )}
+      {continueButton}
+    </>
+  );
+};

plugins/catalog-import/src/components/StepPrepareSelectLocations/StepPrepareSelectLocations.test.tsx

@@ -26,7 +26,7 @@ describe('<StepPrepareSelectLocations />', () => {
     type: 'locations',
     locations: [
       {
-        target: 'url',
+        target: 'url-1',
         entities: [
           {
             kind: 'component',
@@ -57,8 +57,8 @@ describe('<StepPrepareSelectLocations />', () => {
     jest.resetAllMocks();
   });
 
-  it('renders without exploding', async () => {
-    const { getByRole } = await renderInTestApp(
+  it('renders display locations to be added', async () => {
+    const rendered = await renderInTestApp(
       <StepPrepareSelectLocations
         analyzeResult={analyzeResult}
         onPrepare={() => undefined}
@@ -66,7 +66,48 @@ describe('<StepPrepareSelectLocations />', () => {
       />,
     );
 
-    expect(getByRole('button', { name: /Review/i })).toBeDisabled();
+    expect(rendered.getByText('url-1')).toBeInTheDocument();
+    expect(rendered.getByText('url-2')).toBeInTheDocument();
+    expect(
+      rendered.queryByText(/Select one or more locations/),
+    ).toBeInTheDocument();
+    expect(
+      rendered.queryByText(/locations already exist/),
+    ).not.toBeInTheDocument();
+    expect(rendered.getByRole('button', { name: /Review/i })).toBeDisabled();
+  });
+
+  it('should display existing locations only', async () => {
+    const analyzeResultWithExistingLocation = {
+      type: 'locations',
+      locations: [
+        {
+          target: 'my-target',
+          exists: true,
+          entities: [
+            {
+              kind: 'component',
+              namespace: 'default',
+              name: 'name',
+            },
+          ],
+        },
+      ],
+    } as Extract<AnalyzeResult, { type: 'locations' }>;
+
+    const rendered = await renderInTestApp(
+      <StepPrepareSelectLocations
+        analyzeResult={analyzeResultWithExistingLocation}
+        onPrepare={() => undefined}
+        onGoBack={() => undefined}
+      />,
+    );
+
+    expect(rendered.getByText(/my-target/)).toBeInTheDocument();
+    expect(rendered.queryByText(/locations already exist/)).toBeInTheDocument();
+    expect(
+      rendered.queryByText(/Select one or more locations/),
+    ).not.toBeInTheDocument();
   });
 
   it('should select and deselect all', async () => {
@@ -188,7 +229,7 @@ describe('<StepPrepareSelectLocations />', () => {
       type: 'locations',
       locations: [
         {
-          target: 'url',
+          target: 'url-1',
           entities: [
             {
               kind: 'component',

plugins/catalog-import/src/components/StepPrepareSelectLocations/StepPrepareSelectLocations.tsx

@@ -22,11 +22,13 @@ import {
   ListItemText,
   Typography,
 } from '@material-ui/core';
+import LocationOnIcon from '@material-ui/icons/LocationOn';
 import React, { useCallback, useState } from 'react';
 import { AnalyzeResult } from '../../api';
 import { BackButton, NextButton } from '../Buttons';
 import { EntityListComponent } from '../EntityListComponent';
 import { PrepareResult } from '../useImportState';
+import partition from 'lodash/partition';
 
 type Props = {
   analyzeResult: Extract<AnalyzeResult, { type: 'locations' }>;
@@ -53,14 +55,17 @@ export const StepPrepareSelectLocations = ({
     prepareResult?.locations.map(l => l.target) || [],
   );
 
+  const [existingLocations, locations] = partition(
+    analyzeResult?.locations,
+    l => l.exists,
+  );
+
   const handleResult = useCallback(async () => {
     onPrepare({
       type: 'locations',
-      locations: analyzeResult.locations.filter((l: any) =>
-        selectedUrls.includes(l.target),
-      ),
+      locations: locations.filter((l: any) => selectedUrls.includes(l.target)),
     });
-  }, [analyzeResult.locations, onPrepare, selectedUrls]);
+  }, [locations, onPrepare, selectedUrls]);
 
   const onItemClick = (url: string) => {
     setSelectedUrls(urls =>
@@ -70,48 +75,62 @@ export const StepPrepareSelectLocations = ({
 
   const onSelectAll = () => {
     setSelectedUrls(urls =>
-      urls.length < analyzeResult.locations.length
-        ? analyzeResult.locations.map(l => l.target)
-        : [],
+      urls.length < locations.length ? locations.map(l => l.target) : [],
     );
   };
 
   return (
     <>
-      <Typography>
-        Select one or more locations that are present in your git repository:
-      </Typography>
-
-      <EntityListComponent
-        firstListItem={
-          <ListItem dense button onClick={onSelectAll}>
-            <ListItemIcon>
+      {locations.length > 0 && (
+        <>
+          <Typography>
+            Select one or more locations that are present in your git
+            repository:
+          </Typography>
+          <EntityListComponent
+            firstListItem={
+              <ListItem dense button onClick={onSelectAll}>
+                <ListItemIcon>
+                  <Checkbox
+                    edge="start"
+                    checked={selectedUrls.length === locations.length}
+                    indeterminate={
+                      selectedUrls.length > 0 &&
+                      selectedUrls.length < locations.length
+                    }
+                    tabIndex={-1}
+                    disableRipple
+                  />
+                </ListItemIcon>
+                <ListItemText primary="Select All" />
+              </ListItem>
+            }
+            onItemClick={onItemClick}
+            locations={locations}
+            locationListItemIcon={target => (
               <Checkbox
                 edge="start"
-                checked={selectedUrls.length === analyzeResult.locations.length}
-                indeterminate={
-                  selectedUrls.length > 0 &&
-                  selectedUrls.length < analyzeResult.locations.length
-                }
+                checked={selectedUrls.includes(target)}
                 tabIndex={-1}
                 disableRipple
               />
-            </ListItemIcon>
-            <ListItemText primary="Select All" />
-          </ListItem>
-        }
-        onItemClick={onItemClick}
-        locations={analyzeResult.locations}
-        locationListItemIcon={target => (
-          <Checkbox
-            edge="start"
-            checked={selectedUrls.includes(target)}
-            tabIndex={-1}
-            disableRipple
+            )}
+            collapsed
           />
-        )}
-        collapsed
-      />
+        </>
+      )}
+
+      {existingLocations.length > 0 && (
+        <>
+          <Typography>These locations already exist in the catalog:</Typography>
+          <EntityListComponent
+            locations={existingLocations}
+            locationListItemIcon={() => <LocationOnIcon />}
+            withLinks
+            collapsed
+          />
+        </>
+      )}
 
       <Grid container spacing={0}>
         {onGoBack && <BackButton onClick={onGoBack} />}

plugins/catalog-import/src/components/StepReviewLocation/StepReviewLocation.tsx

@@ -24,6 +24,7 @@ import { PrepareResult, ReviewResult } from '../useImportState';
 
 import { configApiRef, useApi } from '@backstage/core-plugin-api';
 import { Link } from '@backstage/core-components';
+import { stringifyEntityRef } from '@backstage/catalog-model';
 
 type Props = {
   prepareResult: PrepareResult;
@@ -43,27 +44,48 @@ export const StepReviewLocation = ({
 
   const [submitted, setSubmitted] = useState(false);
   const [error, setError] = useState<string>();
-
-  const handleImport = useCallback(async () => {
+  const exists =
+    prepareResult.type === 'locations' &&
+    prepareResult.locations.some(l => l.exists)
+      ? true
+      : false;
+  const handleClick = useCallback(async () => {
     setSubmitted(true);
     try {
-      const result = await Promise.all(
-        prepareResult.locations.map(l =>
-          catalogApi.addLocation({
-            type: 'url',
-            target: l.target,
-            presence:
-              prepareResult.type === 'repository' ? 'optional' : 'required',
+      let refreshed = new Array<{ target: string }>();
+      if (prepareResult.type === 'locations') {
+        refreshed = await Promise.all(
+          prepareResult.locations
+            .filter(l => l.exists)
+            .map(async l => {
+              const ref = stringifyEntityRef(l.entities[0] ?? l);
+              await catalogApi.refreshEntity(ref);
+              return { target: l.target };
+            }),
+        );
+      }
+
+      const locations = await Promise.all(
+        prepareResult.locations
+          .filter((l: unknown) => !(l as { exists?: boolean }).exists)
+          .map(async l => {
+            const result = await catalogApi.addLocation({
+              type: 'url',
+              target: l.target,
+              presence:
+                prepareResult.type === 'repository' ? 'optional' : 'required',
+            });
+            return {
+              target: result.location.target,
+              entities: result.entities,
+            };
           }),
-        ),
       );
 
       onReview({
         ...prepareResult,
-        locations: result.map(r => ({
-          target: r.location.target,
-          entities: r.entities,
-        })),
+        ...{ refreshed },
+        locations,
       });
     } catch (e) {
       // TODO: this error should be handled differently. We add it as 'optional' and
@@ -111,7 +133,9 @@ export const StepReviewLocation = ({
       )}
 
       <Typography>
-        The following entities will be added to the catalog:
+        {exists
+          ? 'The following locations already exist in the catalog:'
+          : 'The following entities will be added to the catalog:'}
       </Typography>
 
       <EntityListComponent
@@ -126,9 +150,9 @@ export const StepReviewLocation = ({
         <NextButton
           disabled={submitted}
           loading={submitted}
-          onClick={() => handleImport()}
+          onClick={() => handleClick()}
         >
-          Import
+          {exists ? 'Refresh' : 'Import'}
         </NextButton>
       </Grid>
     </>

plugins/catalog-import/src/components/useImportState.test.tsx

@@ -50,6 +50,7 @@ describe('useImportState', () => {
         entities: [] as Entity[],
       },
     ],
+    refreshed: [],
   };
 
   it('should use initial url', async () => {
@@ -131,14 +132,16 @@ describe('useImportState', () => {
       });
 
       act(() => result.current.onReset());
-
       expect(result.current).toMatchObject({
         activeFlow: 'unknown',
         activeStepNumber: 0,
         analysisUrl: undefined,
         activeState: 'analyze',
         analyzeResult: undefined,
-        prepareResult: locationR,
+        prepareResult: {
+          type: 'locations',
+          locations: [{ target: 'https://0', entities: [] }],
+        },
         reviewResult: undefined,
       });
     });

plugins/catalog-import/src/components/useImportState.ts

@@ -33,6 +33,7 @@ export type PrepareResult =
   | {
       type: 'locations';
       locations: Array<{
+        exists?: boolean;
         target: string;
         entities: EntityName[];
       }>;
@@ -58,6 +59,7 @@ export type ReviewResult =
         target: string;
         entities: Entity[];
       }>;
+      refreshed: Array<{ target: string }>;
     }
   | {
       type: 'repository';