diff --git a/plugins/catalog-backend/src/ingestion/LocationReaders.ts b/plugins/catalog-backend/src/ingestion/LocationReaders.ts index 14b7f727e4..dd3f9eacda 100644 --- a/plugins/catalog-backend/src/ingestion/LocationReaders.ts +++ b/plugins/catalog-backend/src/ingestion/LocationReaders.ts @@ -258,10 +258,9 @@ export class LocationReaders implements LocationReader { } } - private async readLocation( - location: LocationSpec, - ): Promise { - let locationResult: LocationProcessorResult | undefined; + private async readLocation(location: LocationSpec): Promise { + let data: Buffer | undefined = undefined; + let error: Error | undefined = undefined; await this.handleLocation( { @@ -269,13 +268,29 @@ export class LocationReaders implements LocationReader { location, optional: false, }, - r => (locationResult = r), + output => { + if (output.type === 'error' && !error) { + error = output.error; + } else if (output.type === 'data') { + if (data) { + if (!error) { + error = new Error( + 'More than one piece of data loaded unexpectedly', + ); + } + } else { + data = output.data; + } + } + }, ); - if (!locationResult) { - throw new Error('No location loaded'); + if (error) { + throw error; + } else if (!data) { + throw new Error('No data loaded'); } - return locationResult; + return data; } } diff --git a/plugins/catalog-backend/src/ingestion/processors/ApiDefinitionAtLocationProcessor.test.ts b/plugins/catalog-backend/src/ingestion/processors/ApiDefinitionAtLocationProcessor.test.ts index ef6d348ad5..6ae6562339 100644 --- a/plugins/catalog-backend/src/ingestion/processors/ApiDefinitionAtLocationProcessor.test.ts +++ b/plugins/catalog-backend/src/ingestion/processors/ApiDefinitionAtLocationProcessor.test.ts @@ -14,9 +14,7 @@ import { ApiEntity, Entity, LocationSpec } from '@backstage/catalog-model'; * See the License for the specific language governing permissions and * limitations under the License. */ - import { ApiDefinitionAtLocationProcessor } from './ApiDefinitionAtLocationProcessor'; -import { LocationProcessorResult } from './types'; describe('ApiDefinitionAtLocationProcessor', () => { let processor: ApiDefinitionAtLocationProcessor; @@ -45,11 +43,7 @@ describe('ApiDefinitionAtLocationProcessor', () => { }); it('should skip entities without annotation', async () => { - const read = jest.fn( - (): Promise => { - throw new Error(); - }, - ); + const read = jest.fn().mockRejectedValue(new Error('boo')); const generated = (await processor.processEntity( entity, @@ -67,14 +61,7 @@ describe('ApiDefinitionAtLocationProcessor', () => { 'url:http://example.com/openapi.yaml', }; - const read = jest.fn( - (l: LocationSpec): Promise => - Promise.resolve({ - type: 'data', - data: Buffer.from('Hello'), - location: l, - }), - ); + const read = jest.fn().mockResolvedValue(Buffer.from('Hello')); const generated = (await processor.processEntity( entity, @@ -95,38 +82,12 @@ describe('ApiDefinitionAtLocationProcessor', () => { 'backstage.io/definition-at-location': 'missing', }; - const read = jest.fn( - (l: LocationSpec): Promise => - Promise.resolve({ - type: 'error', - error: new Error('Failed to load location'), - location: l, - }), - ); + const read = jest + .fn() + .mockRejectedValue(new Error('Failed to load location')); await expect( processor.processEntity(entity, location, () => {}, read), - ).rejects.toThrow('Failed to read location: Failed to load location'); - }); - - it('should throw errors if location read has wrong type', async () => { - entity.metadata.annotations = { - 'backstage.io/definition-at-location': 'wrong', - }; - - const read = jest.fn( - (l: LocationSpec): Promise => - Promise.resolve({ - type: 'location', - optional: false, - location: l, - }), - ); - - await expect( - processor.processEntity(entity, location, () => {}, read), - ).rejects.toThrow( - `Only supports location processor results of type 'data', but got 'location'`, - ); + ).rejects.toThrow('Failed to load location'); }); }); diff --git a/plugins/catalog-backend/src/ingestion/processors/ApiDefinitionAtLocationProcessor.ts b/plugins/catalog-backend/src/ingestion/processors/ApiDefinitionAtLocationProcessor.ts index c359e78292..e64d112e9c 100644 --- a/plugins/catalog-backend/src/ingestion/processors/ApiDefinitionAtLocationProcessor.ts +++ b/plugins/catalog-backend/src/ingestion/processors/ApiDefinitionAtLocationProcessor.ts @@ -41,19 +41,8 @@ export class ApiDefinitionAtLocationProcessor implements LocationProcessor { const reference = entity.metadata.annotations[DEFINITION_AT_LOCATION_ANNOTATION]; const { type, target } = extractReference(reference); - const result = await read({ type, target }); - - if (result.type === 'error') { - throw new Error(`Failed to read location: ${result.error.message}`); - } - - if (result.type !== 'data') { - throw new Error( - `Only supports location processor results of type 'data', but got '${result.type}'`, - ); - } - - const definition = result.data.toString(); + const data = await read({ type, target }); + const definition = data.toString(); const apiEntity = entity as ApiEntity; apiEntity.spec.definition = definition; diff --git a/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.test.ts b/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.test.ts index 7e6fa528f1..d00e4b7b6f 100644 --- a/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.test.ts +++ b/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.test.ts @@ -22,9 +22,16 @@ import { ResolverParams, yamlPlaceholderResolver, } from './PlaceholderProcessor'; -import { LocationProcessorRead } from './types'; +import { LocationProcessorEmit, LocationProcessorRead } from './types'; describe('PlaceholderProcessor', () => { + const emit: LocationProcessorEmit = jest.fn(); + const read: jest.MockedFunction = jest.fn(); + + beforeEach(() => { + jest.resetAllMocks(); + }); + it('returns placeholder-free data unchanged', async () => { const input: Entity = { apiVersion: 'a', @@ -35,18 +42,11 @@ describe('PlaceholderProcessor', () => { foo: async () => 'replaced', }); await expect( - processor.processEntity( - input, - { type: 't', target: 'l' }, - jest.fn(), - jest.fn(), - ), + processor.processEntity(input, { type: 't', target: 'l' }, emit, read), ).resolves.toBe(input); }); it('replaces placeholders deep in the data', async () => { - const emit = jest.fn(); - const read = jest.fn(); const upperResolver: PlaceholderResolver = jest.fn(async ({ value }) => value!.toString().toUpperCase(), ); @@ -84,8 +84,6 @@ describe('PlaceholderProcessor', () => { }); it('rejects multiple placeholders', async () => { - const emit = jest.fn(); - const read: LocationProcessorRead = jest.fn(); const processor = new PlaceholderProcessor({ foo: jest.fn(), bar: jest.fn(), @@ -111,8 +109,6 @@ describe('PlaceholderProcessor', () => { }); it('rejects unknown placeholders', async () => { - const emit = jest.fn(); - const read: LocationProcessorRead = jest.fn(); const processor = new PlaceholderProcessor({ bar: jest.fn(), }); @@ -135,14 +131,7 @@ describe('PlaceholderProcessor', () => { }); it('has builtin text support', async () => { - const emit = jest.fn(); - const read: LocationProcessorRead = jest - .fn() - .mockImplementation(async location => ({ - type: 'data', - location, - data: Buffer.from('TEXT', 'utf-8'), - })); + read.mockResolvedValue(Buffer.from('TEXT', 'utf-8')); const processor = PlaceholderProcessor.default(); await expect( @@ -175,14 +164,9 @@ describe('PlaceholderProcessor', () => { }); it('has builtin json support', async () => { - const emit = jest.fn(); - const read: LocationProcessorRead = jest - .fn() - .mockImplementation(async location => ({ - type: 'data', - location, - data: Buffer.from(JSON.stringify({ a: ['b', 7] }), 'utf-8'), - })); + read.mockResolvedValue( + Buffer.from(JSON.stringify({ a: ['b', 7] }), 'utf-8'), + ); const processor = PlaceholderProcessor.default(); await expect( @@ -215,14 +199,7 @@ describe('PlaceholderProcessor', () => { }); it('has builtin yaml support', async () => { - const emit = jest.fn(); - const read: LocationProcessorRead = jest - .fn() - .mockImplementation(async location => ({ - type: 'data', - location, - data: Buffer.from('foo:\n - bar: 7', 'utf-8'), - })); + read.mockResolvedValue(Buffer.from('foo:\n - bar: 7', 'utf-8')); const processor = PlaceholderProcessor.default(); await expect( @@ -256,65 +233,46 @@ describe('PlaceholderProcessor', () => { }); describe('yamlPlaceholderResolver', () => { - let read: jest.MockedFunction; - let params: ResolverParams; + const read: jest.MockedFunction = jest.fn(); + const params: ResolverParams = { + key: 'a', + value: './file.yaml', + location: { + type: 'github', + target: 'https://github.com/spotify/backstage/a/b/catalog-info.yaml', + }, + read, + }; beforeEach(() => { - read = jest.fn(); - params = { - key: 'a', - value: './file.yaml', - location: { - type: 'github', - target: 'https://github.com/spotify/backstage/a/b/catalog-info.yaml', - }, - read, - }; + jest.resetAllMocks(); }); it('parses valid yaml', async () => { - read.mockImplementation(async location => ({ - type: 'data', - location, - data: Buffer.from('foo:\n - bar: 7', 'utf-8'), - })); - + read.mockResolvedValue(Buffer.from('foo:\n - bar: 7', 'utf-8')); await expect(yamlPlaceholderResolver(params)).resolves.toEqual({ foo: [{ bar: 7 }], }); }); it('rejects invalid yaml', async () => { - read.mockImplementation(async location => ({ - type: 'data', - location, - data: Buffer.from('a: 1\n----\n', 'utf-8'), - })); - + read.mockResolvedValue(Buffer.from('a: 1\n----\n', 'utf-8')); await expect(yamlPlaceholderResolver(params)).rejects.toThrow( 'Placeholder $a found an error in the data at ./file.yaml, YAMLSemanticError: Implicit map keys need to be followed by map values', ); }); it('rejects multi-document yaml', async () => { - read.mockImplementation(async location => ({ - type: 'data', - location, - data: Buffer.from('foo: 1\n---\nbar: 2\n', 'utf-8'), - })); - + read.mockResolvedValue(Buffer.from('foo: 1\n---\nbar: 2\n', 'utf-8')); await expect(yamlPlaceholderResolver(params)).rejects.toThrow( 'Placeholder $a expected to find exactly one document of data at ./file.yaml, found 2', ); }); it('parses valid json', async () => { - read.mockImplementation(async location => ({ - type: 'data', - location, - data: Buffer.from(JSON.stringify({ a: ['b', 7] }), 'utf-8'), - })); - + read.mockResolvedValue( + Buffer.from(JSON.stringify({ a: ['b', 7] }), 'utf-8'), + ); await expect(yamlPlaceholderResolver(params)).resolves.toEqual({ a: ['b', 7], }); @@ -322,41 +280,32 @@ describe('yamlPlaceholderResolver', () => { }); describe('jsonPlaceholderResolver', () => { - let read: jest.MockedFunction; - let params: ResolverParams; + const read: jest.MockedFunction = jest.fn(); + const params: ResolverParams = { + key: 'a', + value: './file.json', + location: { + type: 'github', + target: 'https://github.com/spotify/backstage/a/b/catalog-info.yaml', + }, + read, + }; beforeEach(() => { - read = jest.fn(); - params = { - key: 'a', - value: './file.json', - location: { - type: 'github', - target: 'https://github.com/spotify/backstage/a/b/catalog-info.yaml', - }, - read, - }; + jest.resetAllMocks(); }); it('parses valid json', async () => { - read.mockImplementation(async location => ({ - type: 'data', - location, - data: Buffer.from(JSON.stringify({ a: ['b', 7] }), 'utf-8'), - })); - + read.mockResolvedValue( + Buffer.from(JSON.stringify({ a: ['b', 7] }), 'utf-8'), + ); await expect(jsonPlaceholderResolver(params)).resolves.toEqual({ a: ['b', 7], }); }); it('rejects invalid json', async () => { - read.mockImplementation(async location => ({ - type: 'data', - location, - data: Buffer.from('}', 'utf-8'), - })); - + read.mockResolvedValue(Buffer.from('}', 'utf-8')); await expect(jsonPlaceholderResolver(params)).rejects.toThrow( 'Placeholder $a failed to parse JSON data at ./file.json, SyntaxError: Unexpected token } in JSON at position 0', ); diff --git a/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.ts b/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.ts index e405e5a5df..65433b1849 100644 --- a/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.ts +++ b/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.ts @@ -174,11 +174,8 @@ async function readTextLocation(params: ResolverParams): Promise { const newLocation = relativeLocation(params); try { - const response = await params.read(newLocation); - if (response.type !== 'data') { - throw new Error(`Expected data, got ${response.type}`); - } - return response.data.toString('utf-8'); + const data = await params.read(newLocation); + return data.toString('utf-8'); } catch (e) { throw new Error( `Placeholder \$${params.key} could not read location ${params.value}, ${e}`, diff --git a/plugins/catalog-backend/src/ingestion/processors/types.ts b/plugins/catalog-backend/src/ingestion/processors/types.ts index 65e2fdbb0a..66359b328b 100644 --- a/plugins/catalog-backend/src/ingestion/processors/types.ts +++ b/plugins/catalog-backend/src/ingestion/processors/types.ts @@ -66,7 +66,7 @@ export type LocationProcessor = { * * @param error The error * @param location The location where the error occurred - * @param emit A sink for items resulting from this handilng + * @param emit A sink for items resulting from this handling * @returns Nothing */ handleError?( @@ -110,6 +110,4 @@ export type LocationProcessorResult = | LocationProcessorEntityResult | LocationProcessorErrorResult; -export type LocationProcessorRead = ( - location: LocationSpec, -) => Promise; +export type LocationProcessorRead = (location: LocationSpec) => Promise;