benjdlambert-1 review changes

Signed-off-by: gabemontero <gmontero@redhat.com>
This commit is contained in:
gabemontero
2025-12-08 15:14:32 -05:00
parent dce18246e4
commit 643f60e667
4 changed files with 59 additions and 101 deletions
@@ -16,6 +16,7 @@
import { createRegisterCatalogEntitiesAction } from './createRegisterCatalogEntitiesAction';
import { catalogServiceMock } from '@backstage/plugin-catalog-node/testUtils';
import { actionsRegistryServiceMock } from '@backstage/backend-test-utils/alpha';
import { ForwardedError } from '@backstage/errors';
describe('createRegisterCatalogEntitiesAction', () => {
it('should successfully register a catalog location with a valid URL', async () => {
@@ -48,7 +49,6 @@ describe('createRegisterCatalogEntitiesAction', () => {
expect(result.output).toEqual({
locationID: mockLocationId,
error: undefined,
});
expect(mockCatalog.addLocation).toHaveBeenCalledWith(
{
@@ -61,7 +61,7 @@ describe('createRegisterCatalogEntitiesAction', () => {
);
});
it('should return an error if locationURL is not provided', async () => {
it('should throw an error if locationURL is not provided', async () => {
const mockActionsRegistry = actionsRegistryServiceMock();
const mockCatalog = catalogServiceMock();
@@ -70,17 +70,15 @@ describe('createRegisterCatalogEntitiesAction', () => {
actionsRegistry: mockActionsRegistry,
});
const result = await mockActionsRegistry.invoke({
id: 'test:register-catalog-entities',
input: { locationURL: '' },
});
expect(result.output).toEqual({
error: 'a location URL must be specified',
});
await expect(
mockActionsRegistry.invoke({
id: 'test:register-catalog-entities',
input: { locationURL: '' },
}),
).rejects.toThrow('a location URL must be specified');
});
it('should return an error if locationURL is not a valid URL', async () => {
it('should throw an error if locationURL is not a valid URL', async () => {
const mockActionsRegistry = actionsRegistryServiceMock();
const mockCatalog = catalogServiceMock();
@@ -89,17 +87,15 @@ describe('createRegisterCatalogEntitiesAction', () => {
actionsRegistry: mockActionsRegistry,
});
const result = await mockActionsRegistry.invoke({
id: 'test:register-catalog-entities',
input: { locationURL: 'not-a-valid-url' },
});
expect(result.output).toEqual({
error: 'location URL must be a valid URL string',
});
await expect(
mockActionsRegistry.invoke({
id: 'test:register-catalog-entities',
input: { locationURL: 'not-a-valid-url' },
}),
).rejects.toThrow('locationURL "not-a-valid-url" an invalid URL');
});
it('should return an error if catalog.addLocation throws an error', async () => {
it('should throw a ForwardedError if catalog.addLocation throws an error', async () => {
const mockActionsRegistry = actionsRegistryServiceMock();
const mockCatalog = catalogServiceMock();
@@ -113,16 +109,14 @@ describe('createRegisterCatalogEntitiesAction', () => {
actionsRegistry: mockActionsRegistry,
});
const result = await mockActionsRegistry.invoke({
id: 'test:register-catalog-entities',
input: {
locationURL:
'https://github.com/example/repo/blob/main/catalog-info.yaml',
},
});
expect(result.output).toEqual({
error: errorMessage,
});
await expect(
mockActionsRegistry.invoke({
id: 'test:register-catalog-entities',
input: {
locationURL:
'https://github.com/example/repo/blob/main/catalog-info.yaml',
},
}),
).rejects.toThrow(ForwardedError);
});
});
@@ -15,6 +15,7 @@
*/
import { ActionsRegistryService } from '@backstage/backend-plugin-api/alpha';
import { CatalogService } from '@backstage/plugin-catalog-node';
import { InputError, ForwardedError } from '@backstage/errors';
export const createRegisterCatalogEntitiesAction = ({
catalog,
@@ -45,10 +46,7 @@ The dynamically generated, random UUID created to go along with the newly create
to the caller. That UUID can be later used if you want to unregister or delete the Location entity and all
of its children entities.
If an error occurred during processing, the 'locationID' field will be blank, and the 'error' field will contain
a message describing the error.
Example invocation and the output from the invocation:
/eExample invocation and the output from the invocation:
# Register a Location from a GitHub URL
register-catalog-entities locationURL: https://github.com/redhat-ai-dev/model-catalog-example/blob/main/developer-model-service/catalog-info.yaml
Output: {
@@ -70,19 +68,11 @@ Example invocation and the output from the invocation:
.string()
.optional()
.describe('The location ID generated by the registration'),
error: z
.string()
.optional()
.describe('Error message if creation fails'),
}),
},
action: async ({ input, credentials }) => {
if (!input.locationURL || input.locationURL === '') {
return {
output: {
error: 'a location URL must be specified',
},
};
throw new InputError('a location URL must be specified');
}
// Validate that the locationURL is a valid URL
@@ -90,11 +80,9 @@ Example invocation and the output from the invocation:
// eslint-disable-next-line no-new
new URL(input.locationURL);
} catch {
return {
output: {
error: 'location URL must be a valid URL string',
},
};
throw new InputError(
`locationURL "${input.locationURL}" an invalid URL`,
);
}
try {
const locReq = {
@@ -108,15 +96,13 @@ Example invocation and the output from the invocation:
return {
output: {
locationID: result.location.id,
error: undefined,
},
};
} catch (error) {
return {
output: {
error: error.message,
},
};
throw new ForwardedError(
`catalog import of "${input.locationURL} failed"`,
error,
);
}
},
});
@@ -16,6 +16,7 @@
import { createUnregisterCatalogEntitiesAction } from './createUnregisterCatalogEntitiesAction';
import { catalogServiceMock } from '@backstage/plugin-catalog-node/testUtils';
import { actionsRegistryServiceMock } from '@backstage/backend-test-utils/alpha';
import { ForwardedError } from '@backstage/errors';
describe('createUnregisterCatalogEntitiesAction', () => {
it('should successfully unregister a catalog location with a valid locationId', async () => {
@@ -34,9 +35,7 @@ describe('createUnregisterCatalogEntitiesAction', () => {
input: { locationId: 'test-location-id-1234' },
});
expect(result.output).toEqual({
error: undefined,
});
expect(result.output).toEqual({});
expect(mockCatalog.removeLocationById).toHaveBeenCalledWith(
'test-location-id-1234',
expect.objectContaining({
@@ -45,7 +44,7 @@ describe('createUnregisterCatalogEntitiesAction', () => {
);
});
it('should return an error if locationId is not provided', async () => {
it('should throw an error if locationId is not provided', async () => {
const mockActionsRegistry = actionsRegistryServiceMock();
const mockCatalog = catalogServiceMock();
@@ -54,17 +53,15 @@ describe('createUnregisterCatalogEntitiesAction', () => {
actionsRegistry: mockActionsRegistry,
});
const result = await mockActionsRegistry.invoke({
id: 'test:unregister-catalog-entities',
input: { locationId: '' },
});
expect(result.output).toEqual({
error: 'a location ID must be specified',
});
await expect(
mockActionsRegistry.invoke({
id: 'test:unregister-catalog-entities',
input: { locationId: '' },
}),
).rejects.toThrow('a locationID must be specified');
});
it('should return an error if catalog.removeLocationById throws an error', async () => {
it('should throw a ForwardedError if catalog.removeLocationById throws an error', async () => {
const mockActionsRegistry = actionsRegistryServiceMock();
const mockCatalog = catalogServiceMock();
@@ -78,13 +75,11 @@ describe('createUnregisterCatalogEntitiesAction', () => {
actionsRegistry: mockActionsRegistry,
});
const result = await mockActionsRegistry.invoke({
id: 'test:unregister-catalog-entities',
input: { locationId: 'test-location-id-1234' },
});
expect(result.output).toEqual({
error: errorMessage,
});
await expect(
mockActionsRegistry.invoke({
id: 'test:unregister-catalog-entities',
input: { locationId: 'test-location-id-1234' },
}),
).rejects.toThrow(ForwardedError);
});
});
@@ -15,6 +15,7 @@
*/
import { ActionsRegistryService } from '@backstage/backend-plugin-api/alpha';
import { CatalogService } from '@backstage/plugin-catalog-node';
import { ForwardedError, InputError } from '@backstage/errors';
export const createUnregisterCatalogEntitiesAction = ({
catalog,
@@ -38,8 +39,6 @@ where you supply the UUID for a Location entity link that allows for the removal
the various Entities (Components, Systems, Resources, APIs, Users, and Groups) that were also created
when the Location was imported.
If an error occurred during processing, the 'error' field will contain a message describing the error.
Example invocation and the output from the invocation:
# Register a Location from a GitHub URL
unregister-catalog-entities locationId: aaa-bbb-ccc-ddd
@@ -54,40 +53,24 @@ Example invocation and the output from the invocation:
`Location ID returned from the 'register-catalog-entities' call.`,
),
}),
output: z =>
z.object({
error: z
.string()
.optional()
.describe('Error message if creation fails'),
}),
output: z => z.object({}),
},
action: async ({ input, credentials }) => {
if (!input.locationId || input.locationId === '') {
return {
output: {
error: 'a location ID must be specified',
},
};
throw new InputError('a locationID must be specified');
}
try {
await catalog.removeLocationById(input.locationId, {
credentials,
});
return {
output: {
error: undefined,
},
};
} catch (error) {
return {
output: {
error: error.message,
},
};
throw new ForwardedError(
`catalog removal of "${input.locationId} failed"`,
error,
);
}
return { output: {} };
},
});
};