feat: add a validate button to the register-component page

This allows the user to validate his location before adding it.
This commit is contained in:
Oliver Sand
2020-10-21 09:37:25 +02:00
parent 948052cbb4
commit 2ebcfac8d4
14 changed files with 327 additions and 151 deletions
+1 -1
View File
@@ -2,7 +2,7 @@
'@backstage/plugin-catalog-backend': minor
---
Add ability to dry run adding a new location ot the catalog API.
Add ability to dry run adding a new location to the catalog API.
The location is now added in a transaction and afterwards rolled back.
This allows users to dry run this operation to see if there entity has issues.
+8
View File
@@ -0,0 +1,8 @@
---
'@backstage/plugin-catalog': minor
'@backstage/plugin-register-component': minor
---
Add a validate button to the register-component page
This allows the user to validate his location before adding it.
@@ -95,25 +95,33 @@ describe('DatabaseEntitiesCatalog', () => {
namespace: 'd',
},
};
const existingTransaction: jest.Mocked<Transaction> = {
rollback: jest.fn(),
};
db.entities.mockResolvedValue([]);
db.addEntities.mockResolvedValue([{ entity }]);
db.addEntities.mockResolvedValue([
{ entity: { ...entity, metadata: { ...entity.metadata, uid: 'u' } } },
]);
const catalog = new DatabaseEntitiesCatalog(db, getVoidLogger());
const result = await catalog.addOrUpdateEntity(entity, {
tx: existingTransaction,
});
expect(db.addEntities).toHaveBeenCalledTimes(1);
expect(db.addEntities).toHaveBeenCalledWith(
existingTransaction,
expect.anything(),
const result = await catalog.batchAddOrUpdateEntities(
[{ entity, relations: [] }],
{ tx: existingTransaction },
);
expect(result).toBe(entity);
expect(db.entities).toHaveBeenCalledTimes(1);
expect(db.entities).toHaveBeenCalledWith(expect.anything(), [
{
kind: 'b',
'metadata.namespace': 'd',
'metadata.name': ['c'],
},
]);
expect(db.setRelations).toHaveBeenCalledTimes(1);
expect(db.setRelations).toHaveBeenCalledWith(expect.anything(), 'u', []);
expect(db.addEntities).toHaveBeenCalledTimes(1);
expect(result).toEqual([{ entityId: 'u' }]);
});
it('updates when given uid', async () => {
@@ -70,46 +70,44 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog {
tx?: Transaction;
}): Promise<Entity[]> {
const filters = options?.filters;
// TODO: Support with and without transaction!
const items = await this.database.transaction(tx =>
this.database.entities(tx, filters),
);
const items = options?.tx
? await this.database.entities(options?.tx, filters)
: await this.database.transaction(tx =>
this.database.entities(tx, filters),
);
return items.map(i => i.entity);
}
private async addOrUpdateEntity(
entity: Entity,
tx: Transaction,
locationId?: string,
): Promise<Entity> {
return await this.database.transaction(async tx => {
// Find a matching (by uid, or by compound name, depending on the given
// entity) existing entity, to know whether to update or add
const existing = entity.metadata.uid
? await this.database.entityByUid(tx, entity.metadata.uid)
: await this.database.entityByName(tx, getEntityName(entity));
// Find a matching (by uid, or by compound name, depending on the given
// entity) existing entity, to know whether to update or add
const existing = entity.metadata.uid
? await this.database.entityByUid(tx, entity.metadata.uid)
: await this.database.entityByName(tx, getEntityName(entity));
// If it's an update, run the algorithm for annotation merging, updating
// etag/generation, etc.
let response: DbEntityResponse;
if (existing) {
const updated = generateUpdatedEntity(existing.entity, entity);
response = await this.database.updateEntity(
tx,
{ locationId, entity: updated },
existing.entity.metadata.etag,
existing.entity.metadata.generation,
);
} else {
const added = await this.database.addEntities(tx, [
{ locationId, entity },
]);
response = added[0];
}
// If it's an update, run the algorithm for annotation merging, updating
// etag/generation, etc.
let response: DbEntityResponse;
if (existing) {
const updated = generateUpdatedEntity(existing.entity, entity);
response = await this.database.updateEntity(
tx,
{ locationId, entity: updated },
existing.entity.metadata.etag,
existing.entity.metadata.generation,
);
} else {
const added = await this.database.addEntities(tx, [
{ locationId, entity },
]);
response = added[0];
}
return response.entity;
});
return response.entity;
}
async removeEntityByUid(uid: string): Promise<void> {
@@ -143,14 +141,41 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog {
* Writes a number of entities efficiently to storage.
*
* @param entities Some entities
* @param locationId The location that they all belong to
* @param options.locationId The location that they all belong to
* @param options.tx A database transaction to execute the queries in
*/
async batchAddOrUpdateEntities(
requests: EntityUpsertRequest[],
options?: { locationId?: string; tx?: Transaction },
options?: {
locationId?: string;
tx?: Transaction;
},
): Promise<EntityUpsertResponse[]> {
// TODO: How to work with the transaction here?
const locationId = options?.locationId;
if (options?.tx) {
return this.batchAddOrUpdateEntitiesInTransaction(
requests,
options.tx,
locationId,
);
}
return await this.database.transaction(
async tx =>
await this.batchAddOrUpdateEntitiesInTransaction(
requests,
tx,
locationId,
),
);
}
private async batchAddOrUpdateEntitiesInTransaction(
requests: EntityUpsertRequest[],
tx: Transaction,
locationId?: string,
): Promise<EntityUpsertResponse[]> {
// Group the entities by unique kind+namespace combinations
const entitiesByKindAndNamespace = groupBy(requests, ({ entity }) => {
const name = getEntityName(entity);
@@ -180,29 +205,30 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog {
const context = {
kind,
namespace,
locationId: options?.locationId,
locationId,
};
for (let attempt = 1; attempt <= BATCH_ATTEMPTS; ++attempt) {
try {
const { toAdd, toUpdate, toIgnore } = await this.analyzeBatch(
batch,
context,
tx,
);
if (toAdd.length) {
modifiedEntityIds.push(
...(await this.batchAdd(toAdd, context)),
...(await this.batchAdd(toAdd, context, tx)),
);
}
if (toUpdate.length) {
modifiedEntityIds.push(
...(await this.batchUpdate(toUpdate, context)),
...(await this.batchUpdate(toUpdate, context, tx)),
);
}
// TODO(Rugvip): We currently always update relations, but we
// likely want to figure out a way to avoid that
for (const { entity, relations } of toIgnore) {
const entityId = entity.metadata.uid!;
await this.setRelations(entityId, relations);
await this.setRelations(entityId, relations, tx);
modifiedEntityIds.push({ entityId });
}
@@ -231,10 +257,9 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog {
private async setRelations(
originatingEntityId: string,
relations: EntityRelationSpec[],
tx: Transaction,
): Promise<void> {
return await this.database.transaction(tx =>
this.database.setRelations(tx, originatingEntityId, relations),
);
await this.database.setRelations(tx, originatingEntityId, relations);
}
// Given a batch of entities that were just read from a location, take them
@@ -244,6 +269,7 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog {
private async analyzeBatch(
requests: EntityUpsertRequest[],
{ kind, namespace }: BatchContext,
tx: Transaction,
): Promise<{
toAdd: EntityUpsertRequest[];
toUpdate: EntityUpsertRequest[];
@@ -260,6 +286,7 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog {
'metadata.name': names,
},
],
tx,
});
const oldEntitiesByName = new Map(
@@ -299,15 +326,13 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog {
private async batchAdd(
requests: EntityUpsertRequest[],
{ locationId }: BatchContext,
tx: Transaction,
): Promise<EntityUpsertResponse[]> {
const markTimestamp = process.hrtime();
const res = await this.database.transaction(
async tx =>
await this.database.addEntities(
tx,
requests.map(({ entity }) => ({ locationId, entity })),
),
const res = await this.database.addEntities(
tx,
requests.map(({ entity }) => ({ locationId, entity })),
);
const entityIds = res.map(({ entity }) => ({
@@ -315,7 +340,7 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog {
}));
for (const [index, { entityId }] of entityIds.entries()) {
await this.setRelations(entityId, requests[index].relations);
await this.setRelations(entityId, requests[index].relations, tx);
}
this.logger.debug(
@@ -330,15 +355,16 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog {
private async batchUpdate(
requests: EntityUpsertRequest[],
{ locationId }: BatchContext,
tx: Transaction,
): Promise<EntityUpsertResponse[]> {
const markTimestamp = process.hrtime();
const responseIds: EntityUpsertResponse[] = [];
// TODO(freben): Still not batched
for (const entity of requests) {
const res = await this.addOrUpdateEntity(entity.entity, locationId);
const res = await this.addOrUpdateEntity(entity.entity, tx, locationId);
const entityId = res.metadata.uid!;
responseIds.push({ entityId });
await this.setRelations(entityId, entity.relations);
await this.setRelations(entityId, entity.relations, tx);
}
this.logger.debug(
+4 -1
View File
@@ -45,7 +45,10 @@ export type EntitiesCatalog = {
*/
batchAddOrUpdateEntities(
entities: EntityUpsertRequest[],
options?: { locationId?: string; tx?: Transaction },
options?: {
tx?: Transaction;
locationId?: string;
},
): Promise<EntityUpsertResponse[]>;
};
@@ -63,6 +63,7 @@ describe('HigherOrderOperations', () => {
entityByName: jest.fn(),
entityByUid: jest.fn(),
removeEntityByUid: jest.fn(),
setRelations: jest.fn(),
addLocation: jest.fn(),
removeLocation: jest.fn(),
location: jest.fn(),
@@ -81,6 +82,7 @@ describe('HigherOrderOperations', () => {
beforeEach(() => {
jest.resetAllMocks();
database.transaction.mockImplementation(async f => f(transaction));
});
describe('addLocation', () => {
@@ -89,7 +91,6 @@ describe('HigherOrderOperations', () => {
type: 'a',
target: 'b',
};
database.transaction.mockImplementation(x => x(transaction));
locationsCatalog.addLocation.mockImplementation(x => Promise.resolve(x));
locationsCatalog.locations.mockResolvedValue([]);
locationReader.read.mockResolvedValue({
@@ -121,6 +122,62 @@ describe('HigherOrderOperations', () => {
expect(transaction.rollback).not.toBeCalled();
});
it('insert the location and its entities', async () => {
const spec = {
type: 'a',
target: 'b',
};
const location: LocationSpec = { type: '', target: '' };
const entity: Entity = {
apiVersion: 'a',
kind: 'b',
metadata: { name: 'n' },
};
locationsCatalog.addLocation.mockImplementation(x => Promise.resolve(x));
locationsCatalog.locations.mockResolvedValue([]);
locationsCatalog.locations.mockResolvedValue([]);
entitiesCatalog.batchAddOrUpdateEntities.mockResolvedValue([
{
entityId: 'id',
},
]);
entitiesCatalog.entities.mockResolvedValue([entity]);
locationReader.read.mockResolvedValue({
entities: [
{
location,
entity,
relations: [],
},
],
errors: [],
});
const result = await higherOrderOperation.addLocation(spec);
expect(result.location).toEqual(
expect.objectContaining({
id: expect.anything(),
...spec,
}),
);
expect(result.entities).toEqual([entity]);
expect(locationsCatalog.locations).toBeCalledTimes(1);
expect(locationsCatalog.addLocation).toBeCalledTimes(1);
expect(locationsCatalog.addLocation).toBeCalledWith(
expect.objectContaining({
id: expect.anything(),
...spec,
}),
{ tx: transaction },
);
expect(locationReader.read).toBeCalledTimes(1);
expect(locationReader.read).toBeCalledWith({ type: 'a', target: 'b' });
expect(entitiesCatalog.entities).toBeCalledTimes(1);
expect(entitiesCatalog.batchAddOrUpdateEntities).toBeCalledTimes(1);
expect(transaction.rollback).not.toBeCalled();
});
it('reuses the location if a match already existed', async () => {
const spec = {
type: 'a',
@@ -131,7 +188,6 @@ describe('HigherOrderOperations', () => {
...spec,
};
database.transaction.mockImplementation(x => x(transaction));
locationsCatalog.locations.mockResolvedValue([
{
currentStatus: { timestamp: '', status: '', message: '' },
@@ -167,7 +223,6 @@ describe('HigherOrderOperations', () => {
metadata: { name: 'n' },
};
database.transaction.mockImplementation(x => x(transaction));
locationsCatalog.locations.mockResolvedValue([]);
locationReader.read.mockResolvedValue({
entities: [{ entity, location, relations: [] }],
@@ -188,10 +243,31 @@ describe('HigherOrderOperations', () => {
type: 'a',
target: 'b',
};
database.transaction.mockImplementation(x => x(transaction));
const location: LocationSpec = { type: '', target: '' };
const entity: Entity = {
apiVersion: 'a',
kind: 'b',
metadata: { name: 'n' },
};
locationsCatalog.addLocation.mockImplementation(x => Promise.resolve(x));
locationsCatalog.locations.mockResolvedValue([]);
locationReader.read.mockResolvedValue({ entities: [], errors: [] });
locationsCatalog.locations.mockResolvedValue([]);
entitiesCatalog.batchAddOrUpdateEntities.mockResolvedValue([
{
entityId: 'id',
},
]);
entitiesCatalog.entities.mockResolvedValue([entity]);
locationReader.read.mockResolvedValue({
entities: [
{
location,
entity,
relations: [],
},
],
errors: [],
});
const result = await higherOrderOperation.addLocation(spec, {
dryRun: true,
@@ -203,11 +279,8 @@ describe('HigherOrderOperations', () => {
...spec,
}),
);
expect(result.entities).toEqual([]);
expect(result.entities).toEqual([entity]);
expect(locationsCatalog.locations).toBeCalledTimes(1);
expect(locationReader.read).toBeCalledTimes(1);
expect(locationReader.read).toBeCalledWith({ type: 'a', target: 'b' });
expect(entitiesCatalog.addOrUpdateEntity).not.toBeCalled();
expect(locationsCatalog.addLocation).toBeCalledTimes(1);
expect(locationsCatalog.addLocation).toBeCalledWith(
expect.objectContaining({
@@ -216,6 +289,10 @@ describe('HigherOrderOperations', () => {
}),
{ tx: transaction },
);
expect(locationReader.read).toBeCalledTimes(1);
expect(locationReader.read).toBeCalledWith({ type: 'a', target: 'b' });
expect(entitiesCatalog.entities).toBeCalledTimes(1);
expect(entitiesCatalog.batchAddOrUpdateEntities).toBeCalledTimes(1);
expect(transaction.rollback).toBeCalled();
});
});
@@ -281,7 +358,9 @@ describe('HigherOrderOperations', () => {
relations: [],
}),
],
'123',
{
locationId: '123',
},
);
});
@@ -90,21 +90,18 @@ export class HigherOrderOperations implements HigherOrderOperation {
await this.locationsCatalog.addLocation(location, { tx });
}
if (readerOutput.entities.length === 0) {
return { location, entities: [] };
return [];
}
const writtenEntities = await this.entitiesCatalog.batchAddOrUpdateEntities(
readerOutput.entities,
{
locationId: location.id,
tx,
},
{ locationId: location.id, tx },
);
const outputEntities = await this.entitiesCatalog.entities(
[{ 'metadata.uid': writtenEntities.map(e => e.entityId) }],
{ tx },
);
const outputEntities = await this.entitiesCatalog.entities({
filters: [{ 'metadata.uid': writtenEntities.map(e => e.entityId) }],
tx,
});
if (dryRun) {
// If this is only a dry run, cancel the database transaction even if it was successful.
@@ -23,8 +23,6 @@ import { LocationResponse } from '../catalog/types';
import { HigherOrderOperation } from '../ingestion/types';
import { createRouter } from './router';
// TODO: ...
describe('createRouter', () => {
let entitiesCatalog: jest.Mocked<EntitiesCatalog>;
let locationsCatalog: jest.Mocked<LocationsCatalog>;
+4 -1
View File
@@ -95,9 +95,12 @@ export class CatalogClient implements CatalogApi {
async addLocation({
type = 'url',
target,
dryRun,
}: AddLocationRequest): Promise<AddLocationResponse> {
const response = await fetch(
`${await this.discoveryApi.getBaseUrl('catalog')}/locations`,
`${await this.discoveryApi.getBaseUrl('catalog')}/locations${
dryRun ? '?dryRun=true' : ''
}`,
{
headers: {
'Content-Type': 'application/json',
+1
View File
@@ -43,6 +43,7 @@ export interface CatalogApi {
export type AddLocationRequest = {
type?: string;
target: string;
dryRun?: boolean;
};
export type AddLocationResponse = {
@@ -20,17 +20,18 @@ import React from 'react';
import { RegisterComponentForm } from './RegisterComponentForm';
describe('RegisterComponentForm', () => {
it('should initially render a disabled button', async () => {
it('should initially render disabled buttons', async () => {
render(<RegisterComponentForm onSubmit={jest.fn()} />);
expect(
await screen.findByText(/Enter the full path to the catalog-info.yaml/),
).toBeInTheDocument();
expect(screen.getByText('Submit').closest('button')).toBeDisabled();
expect(screen.getByText('Validate').closest('button')).toBeDisabled();
expect(screen.getByText('Register').closest('button')).toBeDisabled();
});
it('should enable a submit button when the target url is set', async () => {
it('should enable the submit buttons when the target url is set', async () => {
render(<RegisterComponentForm onSubmit={jest.fn()} />);
await act(async () => {
@@ -40,7 +41,8 @@ describe('RegisterComponentForm', () => {
);
});
expect(screen.getByText('Submit').closest('button')).not.toBeDisabled();
expect(screen.getByText('Validate').closest('button')).not.toBeDisabled();
expect(screen.getByText('Register').closest('button')).not.toBeDisabled();
});
it('should show spinner while submitting', async () => {
@@ -33,8 +33,11 @@ const useStyles = makeStyles<BackstageTheme>(theme => ({
display: 'flex',
flexFlow: 'column nowrap',
},
submit: {
marginTop: theme.spacing(1),
buttonSpacing: {
marginLeft: theme.spacing(1),
},
buttons: {
marginTop: theme.spacing(2),
},
select: {
minWidth: 120,
@@ -54,12 +57,21 @@ export const RegisterComponentForm = ({ onSubmit, submitting }: Props) => {
const hasErrors = !!errors.entityLocation;
const dirty = formState?.isDirty;
const onSubmitValidate = handleSubmit(data => {
data.mode = 'validate';
onSubmit(data);
});
const onSubmitRegister = handleSubmit(data => {
data.mode = 'register';
onSubmit(data);
});
return submitting ? (
<LinearProgress data-testid="loading-progress" />
) : (
<form
autoComplete="off"
onSubmit={handleSubmit(onSubmit)}
className={classes.form}
data-testid="register-form"
>
@@ -87,15 +99,27 @@ export const RegisterComponentForm = ({ onSubmit, submitting }: Props) => {
)}
</FormControl>
<Button
variant="contained"
color="primary"
type="submit"
disabled={!dirty || hasErrors}
className={classes.submit}
>
Submit
</Button>
<div className={classes.buttons}>
<Button
variant="outlined"
color="primary"
type="submit"
disabled={!dirty || hasErrors}
onClick={onSubmitValidate}
>
Validate
</Button>
<Button
variant="contained"
color="primary"
type="submit"
className={classes.buttonSpacing}
disabled={!dirty || hasErrors}
onClick={onSubmitRegister}
>
Register
</Button>
</div>
</form>
);
};
@@ -75,27 +75,30 @@ export const RegisterComponentPage = ({
location: Location;
} | null;
error: null | Error;
dryRun: boolean;
}>({
data: null,
error: null,
dryRun: false,
});
const handleSubmit = async (formData: Record<string, string>) => {
setFormState(FormStates.Submitting);
const { entityLocation: target } = formData;
const { entityLocation: target, mode } = formData;
const dryRun = mode === 'validate';
try {
const data = await catalogApi.addLocation({ target });
const data = await catalogApi.addLocation({ target, dryRun });
if (!isMounted()) return;
setResult({ error: null, data });
setResult({ error: null, data, dryRun });
setFormState(FormStates.Success);
} catch (e) {
errorApi.post(e);
if (!isMounted()) return;
setResult({ error: e, data: null });
setResult({ error: e, data: null, dryRun });
setFormState(FormStates.Idle);
}
};
@@ -124,6 +127,7 @@ export const RegisterComponentPage = ({
{formState === FormStates.Success && (
<RegisterComponentResultDialog
entities={result.data!.entities}
dryRun={result.dryRun}
onClose={() => setFormState(FormStates.Idle)}
classes={{ paper: classes.dialogPaper }}
catalogRouteRef={catalogRouteRef}
@@ -29,7 +29,7 @@ import {
List,
ListItem,
} from '@material-ui/core';
import React from 'react';
import React, { useState } from 'react';
import { generatePath, resolvePath } from 'react-router';
import { Link as RouterLink } from 'react-router-dom';
@@ -37,6 +37,7 @@ type Props = {
onClose: () => void;
classes?: Record<string, string>;
entities: Entity[];
dryRun?: boolean;
catalogRouteRef: RouteRef;
};
@@ -64,49 +65,71 @@ export const RegisterComponentResultDialog = ({
onClose,
classes,
entities,
dryRun,
catalogRouteRef,
}: Props) => (
<Dialog open onClose={onClose} classes={classes}>
<DialogTitle>Registration Result</DialogTitle>
<DialogContent>
<DialogContentText>
The following entities have been successfully created:
</DialogContentText>
<List>
{entities.map((entity: any, index: number) => {
const entityPath = getEntityCatalogPath({ entity, catalogRouteRef });
return (
<React.Fragment
key={`${entity.metadata.namespace}-${entity.metadata.name}`}
>
<ListItem>
<StructuredMetadataTable
dense
metadata={{
name: entity.metadata.name,
type: entity.spec.type,
link: (
<Link component={RouterLink} to={entityPath}>
{entityPath}
</Link>
),
}}
/>
</ListItem>
{index < entities.length - 1 && <Divider component="li" />}
</React.Fragment>
);
})}
</List>
</DialogContent>
<DialogActions>
<Button
component={RouterLink}
to={`/${catalogRouteRef.path}`}
color="default"
>
To Catalog
</Button>
</DialogActions>
</Dialog>
);
}: Props) => {
const [open, setOpen] = useState(true);
const handleClose = () => {
setOpen(false);
};
return (
<Dialog open={open} onClose={onClose} classes={classes}>
<DialogTitle>
{dryRun ? 'Validation Result' : 'Registration Result'}
</DialogTitle>
<DialogContent>
<DialogContentText>
{dryRun
? 'The following entities would be created:'
: 'The following entities have been successfully created:'}
</DialogContentText>
<List>
{entities.map((entity: any, index: number) => {
const entityPath = getEntityCatalogPath({
entity,
catalogRouteRef,
});
return (
<React.Fragment
key={`${entity.metadata.namespace}-${entity.metadata.name}`}
>
<ListItem>
<StructuredMetadataTable
dense
metadata={{
name: entity.metadata.name,
type: entity.spec.type,
link: (
<Link component={RouterLink} to={entityPath}>
{entityPath}
</Link>
),
}}
/>
</ListItem>
{index < entities.length - 1 && <Divider component="li" />}
</React.Fragment>
);
})}
</List>
</DialogContent>
<DialogActions>
{dryRun && (
<Button onClick={handleClose} color="default">
Close
</Button>
)}
{!dryRun && (
<Button
component={RouterLink}
to={`/${catalogRouteRef.path}`}
color="default"
>
To Catalog
</Button>
)}
</DialogActions>
</Dialog>
);
};