Add onConflict query parameter to POST /locations endpoint
Adds an optional `onConflict` query parameter to the location creation
endpoint. When set to 'refresh', a conflict due to an already-registered
location triggers a refresh of the existing location entity instead of
returning a 409 error. The default behavior ('reject') is unchanged.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Fredrik Adelöw <freben@spotify.com>
This commit is contained in:
@@ -15,6 +15,7 @@ export type AddLocationRequest = {
|
||||
type?: string;
|
||||
target: string;
|
||||
dryRun?: boolean;
|
||||
onConflict?: 'refresh' | 'reject';
|
||||
};
|
||||
|
||||
// @public
|
||||
|
||||
@@ -567,12 +567,15 @@ export class CatalogClient implements CatalogApi {
|
||||
request: AddLocationRequest,
|
||||
options?: CatalogRequestOptions,
|
||||
): Promise<AddLocationResponse> {
|
||||
const { type = 'url', target, dryRun } = request;
|
||||
const { type = 'url', target, dryRun, onConflict } = request;
|
||||
|
||||
const response = await this.apiClient.createLocation(
|
||||
{
|
||||
body: { type, target },
|
||||
query: { dryRun: dryRun ? 'true' : undefined },
|
||||
query: {
|
||||
dryRun: dryRun ? 'true' : undefined,
|
||||
onConflict,
|
||||
},
|
||||
},
|
||||
options,
|
||||
);
|
||||
|
||||
@@ -178,6 +178,7 @@ export type CreateLocation = {
|
||||
body: CreateLocationRequest;
|
||||
query: {
|
||||
dryRun?: string;
|
||||
onConflict?: 'refresh' | 'reject';
|
||||
};
|
||||
};
|
||||
/**
|
||||
@@ -592,6 +593,7 @@ export class DefaultApiClient {
|
||||
* Create a location for a given target.
|
||||
* @param createLocationRequest -
|
||||
* @param dryRun -
|
||||
* @param onConflict - Behavior when the location already exists. \'reject\' (default) returns a 409 error, \'refresh\' triggers a refresh of the existing location entity and returns 201.
|
||||
*/
|
||||
public async createLocation(
|
||||
// @ts-ignore
|
||||
@@ -600,7 +602,7 @@ export class DefaultApiClient {
|
||||
): Promise<TypedResponse<CreateLocation201Response>> {
|
||||
const baseUrl = await this.discoveryApi.getBaseUrl(pluginId);
|
||||
|
||||
const uriTemplate = `/locations{?dryRun}`;
|
||||
const uriTemplate = `/locations{?dryRun,onConflict}`;
|
||||
|
||||
const uri = parser.parse(uriTemplate).expand({
|
||||
...request.query,
|
||||
|
||||
@@ -396,6 +396,12 @@ export type AddLocationRequest = {
|
||||
* contain the entities that match the given location.
|
||||
*/
|
||||
dryRun?: boolean;
|
||||
/**
|
||||
* Behavior when the location already exists. If set to `'reject'` (the
|
||||
* default), a conflict error is returned. If set to `'refresh'`, the
|
||||
* existing location entity is marked for refresh and a 201 is returned.
|
||||
*/
|
||||
onConflict?: 'refresh' | 'reject';
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
@@ -1267,6 +1267,18 @@ paths:
|
||||
allowReserved: true
|
||||
schema:
|
||||
type: string
|
||||
- in: query
|
||||
name: onConflict
|
||||
required: false
|
||||
schema:
|
||||
type: string
|
||||
enum:
|
||||
- refresh
|
||||
- reject
|
||||
description: >-
|
||||
Behavior when the location already exists. 'reject' (default)
|
||||
returns a 409 error, 'refresh' triggers a refresh of the
|
||||
existing location entity and returns 201.
|
||||
requestBody:
|
||||
required: true
|
||||
content:
|
||||
|
||||
@@ -172,6 +172,7 @@ export type CreateLocation = {
|
||||
body: CreateLocationRequest;
|
||||
query: {
|
||||
dryRun?: string;
|
||||
onConflict?: 'refresh' | 'reject';
|
||||
};
|
||||
response: CreateLocation201Response | Error | Error;
|
||||
};
|
||||
|
||||
@@ -1490,6 +1490,17 @@ export const spec = {
|
||||
type: 'string',
|
||||
},
|
||||
},
|
||||
{
|
||||
in: 'query',
|
||||
name: 'onConflict',
|
||||
required: false,
|
||||
schema: {
|
||||
type: 'string',
|
||||
enum: ['refresh', 'reject'],
|
||||
},
|
||||
description:
|
||||
"Behavior when the location already exists. 'reject' (default) returns a 409 error, 'refresh' triggers a refresh of the existing location entity and returns 201.",
|
||||
},
|
||||
],
|
||||
requestBody: {
|
||||
required: true,
|
||||
|
||||
@@ -46,6 +46,7 @@ export class AuthorizedLocationService implements LocationService {
|
||||
spec: LocationInput,
|
||||
dryRun: boolean,
|
||||
options: {
|
||||
onConflict?: 'refresh' | 'reject';
|
||||
credentials: BackstageCredentials;
|
||||
},
|
||||
): Promise<{
|
||||
|
||||
@@ -591,14 +591,18 @@ export class CatalogBuilder {
|
||||
new RepoLocationAnalyzer(logger, integrations, this.locationAnalyzers),
|
||||
permissionsService,
|
||||
);
|
||||
const innerRefreshService = new DefaultRefreshService({
|
||||
database: catalogDatabase,
|
||||
});
|
||||
const locationService = new AuthorizedLocationService(
|
||||
new DefaultLocationService(locationStore, orchestrator, {
|
||||
allowedLocationTypes: this.allowedLocationType,
|
||||
refreshService: innerRefreshService,
|
||||
}),
|
||||
permissionsService,
|
||||
);
|
||||
const refreshService = new AuthorizedRefreshService(
|
||||
new DefaultRefreshService({ database: catalogDatabase }),
|
||||
innerRefreshService,
|
||||
permissionsService,
|
||||
);
|
||||
|
||||
|
||||
@@ -16,8 +16,8 @@
|
||||
|
||||
import { DefaultLocationService } from './DefaultLocationService';
|
||||
import { CatalogProcessingOrchestrator } from '../processing/types';
|
||||
import { LocationStore } from './types';
|
||||
import { InputError } from '@backstage/errors';
|
||||
import { LocationStore, RefreshService } from './types';
|
||||
import { ConflictError, InputError } from '@backstage/errors';
|
||||
|
||||
describe('DefaultLocationServiceTest', () => {
|
||||
const orchestrator: jest.Mocked<CatalogProcessingOrchestrator> = {
|
||||
@@ -309,6 +309,86 @@ describe('DefaultLocationServiceTest', () => {
|
||||
).rejects.toThrow(InputError);
|
||||
});
|
||||
|
||||
it('should refresh existing location on conflict when onConflict is refresh', async () => {
|
||||
const locationSpec = {
|
||||
type: 'url',
|
||||
target: 'https://backstage.io/catalog-info.yaml',
|
||||
};
|
||||
|
||||
const refreshService: jest.Mocked<RefreshService> = {
|
||||
refresh: jest.fn(),
|
||||
};
|
||||
|
||||
const locationServiceWithRefresh = new DefaultLocationService(
|
||||
store,
|
||||
orchestrator,
|
||||
{ allowedLocationTypes: ['url'], refreshService },
|
||||
);
|
||||
|
||||
store.createLocation.mockRejectedValueOnce(
|
||||
new ConflictError(
|
||||
`Location ${locationSpec.type}:${locationSpec.target} already exists`,
|
||||
),
|
||||
);
|
||||
store.getLocationByEntity.mockResolvedValueOnce({
|
||||
id: 'existing-id',
|
||||
...locationSpec,
|
||||
});
|
||||
|
||||
const credentials = {} as any;
|
||||
const result = await locationServiceWithRefresh.createLocation(
|
||||
locationSpec,
|
||||
false,
|
||||
{ onConflict: 'refresh', credentials },
|
||||
);
|
||||
|
||||
expect(result).toEqual({
|
||||
location: { id: 'existing-id', ...locationSpec },
|
||||
entities: [],
|
||||
});
|
||||
expect(refreshService.refresh).toHaveBeenCalledWith({
|
||||
entityRef: expect.stringMatching(/^location:default\/generated-/),
|
||||
credentials,
|
||||
});
|
||||
});
|
||||
|
||||
it('should still throw conflict error when onConflict is reject', async () => {
|
||||
const locationSpec = {
|
||||
type: 'url',
|
||||
target: 'https://backstage.io/catalog-info.yaml',
|
||||
};
|
||||
|
||||
store.createLocation.mockRejectedValueOnce(
|
||||
new ConflictError(
|
||||
`Location ${locationSpec.type}:${locationSpec.target} already exists`,
|
||||
),
|
||||
);
|
||||
|
||||
await expect(
|
||||
locationService.createLocation(locationSpec, false, {
|
||||
onConflict: 'reject',
|
||||
credentials: {} as any,
|
||||
}),
|
||||
).rejects.toThrow(ConflictError);
|
||||
});
|
||||
|
||||
it('should throw conflict error by default when onConflict is not set', async () => {
|
||||
const locationSpec = {
|
||||
type: 'url',
|
||||
target: 'https://backstage.io/catalog-info.yaml',
|
||||
};
|
||||
|
||||
store.createLocation.mockRejectedValueOnce(
|
||||
new ConflictError(
|
||||
`Location ${locationSpec.type}:${locationSpec.target} already exists`,
|
||||
),
|
||||
);
|
||||
|
||||
await expect(
|
||||
locationService.createLocation(locationSpec, false),
|
||||
).rejects.toThrow(ConflictError);
|
||||
});
|
||||
|
||||
it('should return default InputError for failed processed entities in dryRun mode', async () => {
|
||||
store.listLocations.mockResolvedValueOnce([]);
|
||||
|
||||
|
||||
@@ -24,15 +24,21 @@ import {
|
||||
} from '@backstage/catalog-model';
|
||||
import { Location } from '@backstage/catalog-client';
|
||||
import { CatalogProcessingOrchestrator } from '../processing/types';
|
||||
import { LocationInput, LocationService, LocationStore } from './types';
|
||||
import {
|
||||
LocationInput,
|
||||
LocationService,
|
||||
LocationStore,
|
||||
RefreshService,
|
||||
} from './types';
|
||||
import { locationSpecToMetadataName } from '../util/conversion';
|
||||
import { InputError } from '@backstage/errors';
|
||||
import { ConflictError, InputError } from '@backstage/errors';
|
||||
import { DeferredEntity } from '@backstage/plugin-catalog-node';
|
||||
import { FilterPredicate } from '@backstage/filter-predicates';
|
||||
import { BackstageCredentials } from '@backstage/backend-plugin-api';
|
||||
|
||||
export type DefaultLocationServiceOptions = {
|
||||
allowedLocationTypes: string[];
|
||||
refreshService?: RefreshService;
|
||||
};
|
||||
|
||||
export class DefaultLocationService implements LocationService {
|
||||
@@ -55,6 +61,10 @@ export class DefaultLocationService implements LocationService {
|
||||
async createLocation(
|
||||
input: LocationInput,
|
||||
dryRun: boolean,
|
||||
options?: {
|
||||
onConflict?: 'refresh' | 'reject';
|
||||
credentials?: BackstageCredentials;
|
||||
},
|
||||
): Promise<{ location: Location; entities: Entity[]; exists?: boolean }> {
|
||||
if (!this.options.allowedLocationTypes.includes(input.type)) {
|
||||
throw new InputError(
|
||||
@@ -66,7 +76,48 @@ export class DefaultLocationService implements LocationService {
|
||||
if (dryRun) {
|
||||
return this.dryRunCreateLocation(input);
|
||||
}
|
||||
const location = await this.store.createLocation(input);
|
||||
try {
|
||||
const location = await this.store.createLocation(input);
|
||||
return { location, entities: [] };
|
||||
} catch (error) {
|
||||
if (options?.onConflict === 'refresh' && error instanceof ConflictError) {
|
||||
return this.refreshExistingLocation(input, options);
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
private async refreshExistingLocation(
|
||||
input: LocationInput,
|
||||
options: { credentials?: BackstageCredentials },
|
||||
): Promise<{ location: Location; entities: Entity[] }> {
|
||||
const { refreshService } = this.options;
|
||||
if (!refreshService) {
|
||||
throw new InputError(
|
||||
'onConflict refresh is not supported: no refresh service available',
|
||||
);
|
||||
}
|
||||
if (!options.credentials) {
|
||||
throw new InputError(
|
||||
'onConflict refresh requires credentials to be provided',
|
||||
);
|
||||
}
|
||||
|
||||
const location = await this.store.getLocationByEntity(
|
||||
`location:default/${locationSpecToMetadataName(input)}`,
|
||||
);
|
||||
|
||||
const entityRef = stringifyEntityRef({
|
||||
kind: 'Location',
|
||||
namespace: 'default',
|
||||
name: locationSpecToMetadataName(input),
|
||||
});
|
||||
|
||||
await refreshService.refresh({
|
||||
entityRef,
|
||||
credentials: options.credentials,
|
||||
});
|
||||
|
||||
return { location, entities: [] };
|
||||
}
|
||||
|
||||
|
||||
@@ -606,6 +606,10 @@ export async function createRouter(
|
||||
.post('/locations', async (req, res) => {
|
||||
const location = await validateRequestBody(req, locationInput);
|
||||
const dryRun = yn(req.query.dryRun, { default: false });
|
||||
const onConflict = req.query.onConflict as
|
||||
| 'refresh'
|
||||
| 'reject'
|
||||
| undefined;
|
||||
|
||||
const auditorEvent = await auditor.createEvent({
|
||||
eventId: 'location-mutate',
|
||||
@@ -629,6 +633,7 @@ export async function createRouter(
|
||||
location,
|
||||
dryRun,
|
||||
{
|
||||
onConflict,
|
||||
credentials: await httpAuth.credentials(req),
|
||||
},
|
||||
);
|
||||
|
||||
@@ -35,6 +35,7 @@ export interface LocationService {
|
||||
location: LocationInput,
|
||||
dryRun: boolean,
|
||||
options: {
|
||||
onConflict?: 'refresh' | 'reject';
|
||||
credentials: BackstageCredentials;
|
||||
},
|
||||
): Promise<{ location: Location; entities: Entity[]; exists?: boolean }>;
|
||||
|
||||
Reference in New Issue
Block a user