From 6cc07bc0a026276f0d20582d08a8ad05710ce714 Mon Sep 17 00:00:00 2001 From: Zhongpin Wang Date: Thu, 30 Jan 2025 13:15:47 +0100 Subject: [PATCH] feat: Support resolve external option (#5360) --- .changeset/stale-plums-sneeze.md | 5 ++ packages/openapi-generator/src/generator.ts | 3 +- .../src/options/generator-options.spec.ts | 3 +- .../openapi-generator/src/options/options.ts | 11 +++ .../openapi-generator/src/parser/api.spec.ts | 2 +- .../src/parser/document.spec.ts | 6 +- .../openapi-generator/src/parser/document.ts | 6 +- .../openapi-generator/src/parser/index.ts | 1 - .../src/parser/media-type.spec.ts | 6 +- .../src/parser/operation.spec.ts | 12 ++- .../openapi-generator/src/parser/options.ts | 4 + .../openapi-generator/src/parser/refs.spec.ts | 78 +++++++++++++++++-- packages/openapi-generator/src/parser/refs.ts | 6 +- .../src/parser/request-body.spec.ts | 6 +- .../src/parser/responses.spec.ts | 6 +- .../src/parser/schema-naming.spec.ts | 9 ++- .../src/parser/schema.spec.ts | 9 ++- .../src/parser/swagger-parser-workaround.ts | 33 -------- .../src/parser/unique-naming.spec.ts | 12 ++- packages/openapi-generator/test/test-util.ts | 2 +- .../generate-openapi-services.ts | 44 ++++++----- 21 files changed, 177 insertions(+), 87 deletions(-) create mode 100644 .changeset/stale-plums-sneeze.md delete mode 100644 packages/openapi-generator/src/parser/swagger-parser-workaround.ts diff --git a/.changeset/stale-plums-sneeze.md b/.changeset/stale-plums-sneeze.md new file mode 100644 index 0000000000..5235f96e63 --- /dev/null +++ b/.changeset/stale-plums-sneeze.md @@ -0,0 +1,5 @@ +--- +'@sap-cloud-sdk/openapi-generator': minor +--- + +[New Functionality] Add `resolveExternal` option to determine whether external $ref pointers will be resolved. diff --git a/packages/openapi-generator/src/generator.ts b/packages/openapi-generator/src/generator.ts index 0d810696f1..39203d0242 100644 --- a/packages/openapi-generator/src/generator.ts +++ b/packages/openapi-generator/src/generator.ts @@ -272,7 +272,8 @@ async function generateService( serviceOptions, { strictNaming: !options.skipValidation, - schemaPrefix: options.schemaPrefix + schemaPrefix: options.schemaPrefix, + resolveExternal: options.resolveExternal } ); diff --git a/packages/openapi-generator/src/options/generator-options.spec.ts b/packages/openapi-generator/src/options/generator-options.spec.ts index 3e7db8cb5f..ad4431ab82 100644 --- a/packages/openapi-generator/src/options/generator-options.spec.ts +++ b/packages/openapi-generator/src/options/generator-options.spec.ts @@ -38,7 +38,8 @@ describe('parseGeneratorOptions', () => { overwrite: false, config: undefined, generateESM: false, - schemaPrefix: '' + schemaPrefix: '', + resolveExternal: true }; it('gets default options', () => { diff --git a/packages/openapi-generator/src/options/options.ts b/packages/openapi-generator/src/options/options.ts index 4597e400f8..d868750069 100644 --- a/packages/openapi-generator/src/options/options.ts +++ b/packages/openapi-generator/src/options/options.ts @@ -23,6 +23,10 @@ export interface OpenAPIGeneratorOptions { * @experimental */ schemaPrefix?: string; + /** + * Resolve external references. + */ + resolveExternal?: boolean; } /** @@ -48,5 +52,12 @@ export const cliOptions = { type: 'string', default: '', hidden: true + }, + resolveExternal: { + describe: + 'By default, external $ref pointers will be resolved. If set to false, external $ref pointers will be ignored.', + type: 'boolean', + default: true, + hidden: true } } as const satisfies Options; diff --git a/packages/openapi-generator/src/parser/api.spec.ts b/packages/openapi-generator/src/parser/api.spec.ts index e53cd7b340..7f16a7857f 100644 --- a/packages/openapi-generator/src/parser/api.spec.ts +++ b/packages/openapi-generator/src/parser/api.spec.ts @@ -4,7 +4,7 @@ import { parseApis } from './api'; import { createRefs } from './refs'; import type { OpenAPIV3 } from 'openapi-types'; -const options = { strictNaming: true, schemaPrefix: '' }; +const options = { strictNaming: true, schemaPrefix: '', resolveExternal: true }; describe('parseApis', () => { it('throws an error if there are APIs without paths', async () => { const refs = await createTestRefs(); diff --git a/packages/openapi-generator/src/parser/document.spec.ts b/packages/openapi-generator/src/parser/document.spec.ts index 91fc2e7296..69ef2a6cad 100644 --- a/packages/openapi-generator/src/parser/document.spec.ts +++ b/packages/openapi-generator/src/parser/document.spec.ts @@ -4,7 +4,7 @@ import * as api from './api'; import type { ServiceOptions } from '@sap-cloud-sdk/generator-common/dist/options-per-service'; import type { OpenAPIV3 } from 'openapi-types'; -const options = { strictNaming: true, schemaPrefix: '' }; +const options = { strictNaming: true, schemaPrefix: '', resolveExternal: true }; describe('parseOpenApiDocument()', () => { it('does not modify input service specification', () => { const input: OpenAPIV3.Document = { @@ -54,7 +54,7 @@ describe('parseOpenApiDocument()', () => { packageName: '@sap/cloud-sdk-openapi-test-service', directoryName: 'test-service' }, - { strictNaming: false, schemaPrefix: '' } + { strictNaming: false, schemaPrefix: '', resolveExternal: true } ); expect(parsedDocument.schemas).toEqual([ @@ -276,7 +276,7 @@ describe('parseOpenApiDocument()', () => { packageName: '@sap/cloud-sdk-openapi-test-service', directoryName: 'test-service' }, - { strictNaming: false, schemaPrefix: '' } + { strictNaming: false, schemaPrefix: '', resolveExternal: true } ); expect(parsedDocument.schemas).toEqual([ diff --git a/packages/openapi-generator/src/parser/document.ts b/packages/openapi-generator/src/parser/document.ts index 6225ca2ff5..3e88d41de4 100644 --- a/packages/openapi-generator/src/parser/document.ts +++ b/packages/openapi-generator/src/parser/document.ts @@ -1,3 +1,4 @@ +import SwaggerParser from '@apidevtools/swagger-parser'; import { isAllOfSchema, isOneOfSchema, @@ -10,7 +11,6 @@ import { } from './schema'; import { parseApis } from './api'; import { createRefs } from './refs'; -import { parseBound } from './swagger-parser-workaround'; import type { OpenAPIV3 } from 'openapi-types'; import type { ServiceOptions } from '@sap-cloud-sdk/generator-common/internal'; import type { @@ -35,7 +35,9 @@ export async function parseOpenApiDocument( options: ParserOptions ): Promise { const clonedContent = JSON.parse(JSON.stringify(fileContent)); - const document = (await parseBound(clonedContent)) as OpenAPIV3.Document; + const document = (await SwaggerParser.parse( + clonedContent + )) as OpenAPIV3.Document; const refs = await createRefs(document, options); const schemas = parseSchemas(document, refs, options); sanitizeDiscriminatedSchemas(schemas, refs, options); diff --git a/packages/openapi-generator/src/parser/index.ts b/packages/openapi-generator/src/parser/index.ts index 51a78ad54f..ffd3c675e0 100644 --- a/packages/openapi-generator/src/parser/index.ts +++ b/packages/openapi-generator/src/parser/index.ts @@ -10,6 +10,5 @@ export * from './request-body'; export * from './responses'; export * from './schema-naming'; export * from './schema'; -export * from './swagger-parser-workaround'; export * from './type-mapping'; export * from './unique-naming'; diff --git a/packages/openapi-generator/src/parser/media-type.spec.ts b/packages/openapi-generator/src/parser/media-type.spec.ts index 2f0a3f1308..933d90b873 100644 --- a/packages/openapi-generator/src/parser/media-type.spec.ts +++ b/packages/openapi-generator/src/parser/media-type.spec.ts @@ -1,7 +1,11 @@ import { createTestRefs, emptyObjectSchema } from '../../test/test-util'; import { parseTopLevelMediaType, parseMediaType } from './media-type'; -const defaultOptions = { strictNaming: true, schemaPrefix: '' }; +const defaultOptions = { + strictNaming: true, + schemaPrefix: '', + resolveExternal: true +}; describe('parseTopLevelMediaType', () => { it('returns undefined if the media type is not supported', async () => { expect( diff --git a/packages/openapi-generator/src/parser/operation.spec.ts b/packages/openapi-generator/src/parser/operation.spec.ts index 2a49ba2966..9eae465aaa 100644 --- a/packages/openapi-generator/src/parser/operation.spec.ts +++ b/packages/openapi-generator/src/parser/operation.spec.ts @@ -8,7 +8,11 @@ import { import type { OpenAPIV3 } from 'openapi-types'; import type { OpenApiParameter } from '../openapi-types'; -const defaultOptions = { strictNaming: true, schemaPrefix: '' }; +const defaultOptions = { + strictNaming: true, + schemaPrefix: '', + resolveExternal: true +}; describe('getRelevantParameters', () => { it('ignores cookie parameters', async () => { expect( @@ -111,7 +115,8 @@ describe('parsePathParameters', () => { expect( parsePathParameters([], await createTestRefs(), { strictNaming: false, - schemaPrefix: '' + schemaPrefix: '', + resolveExternal: true }) ).toEqual([]); }); @@ -131,7 +136,8 @@ describe('parsePathParameters', () => { expect( parsePathParameters([pathParam1, pathParam2], refs, { strictNaming: false, - schemaPrefix: '' + schemaPrefix: '', + resolveExternal: true }) ).toEqual([ { ...pathParam1, originalName: 'param1', schemaProperties: {} }, diff --git a/packages/openapi-generator/src/parser/options.ts b/packages/openapi-generator/src/parser/options.ts index 51483b198a..73a0c9576d 100644 --- a/packages/openapi-generator/src/parser/options.ts +++ b/packages/openapi-generator/src/parser/options.ts @@ -11,4 +11,8 @@ export interface ParserOptions { * Add prefix to schema names. */ schemaPrefix: string; + /** + * Resolve external references. + */ + resolveExternal: boolean; } diff --git a/packages/openapi-generator/src/parser/refs.spec.ts b/packages/openapi-generator/src/parser/refs.spec.ts index e5c13355c4..4b6b8e8561 100644 --- a/packages/openapi-generator/src/parser/refs.spec.ts +++ b/packages/openapi-generator/src/parser/refs.spec.ts @@ -5,16 +5,78 @@ import type { OpenApiDocumentRefs } from './refs'; describe('OpenApiDocumentRefs', () => { let refs: OpenApiDocumentRefs; const typeName: OpenAPIV3.SchemaObject = { type: 'string' }; - beforeAll(async () => { + beforeEach(async () => { refs = await createRefs( { ...emptyDocument, components: { schemas: { typeName } } }, - { strictNaming: true, schemaPrefix: '' } + { strictNaming: true, schemaPrefix: '', resolveExternal: true } ); }); + describe('createRefs', () => { + it('throws if external refs does not exist', async () => { + await expect(() => + createRefs( + { + ...emptyDocument, + paths: { + '/test': { + get: { + responses: { + '200': { + description: 'A test response', + content: { + 'application/json': { + schema: { + $ref: '/path/to/external.json' + } + } + } + } + } + } + } + } + }, + { strictNaming: true, schemaPrefix: 'xyz', resolveExternal: true } + ) + ).rejects.toThrowErrorMatchingInlineSnapshot( + '"Error opening file "/path/to/external.json""' + ); + }); + + it('should ignore external refs if resolveExternal set to false', async () => { + await expect( + createRefs( + { + ...emptyDocument, + paths: { + '/test': { + get: { + responses: { + '200': { + description: 'A test response', + content: { + 'application/json': { + schema: { + $ref: '/path/to/external.json' + } + } + } + } + } + } + } + } + }, + { strictNaming: true, schemaPrefix: 'xyz', resolveExternal: false } + ) + ).resolves.toBeDefined(); + }); + }); + describe('resolveObject', () => { it('resolves reference', async () => { expect( @@ -67,7 +129,7 @@ describe('OpenApiDocumentRefs', () => { ...emptyDocument, components: { schemas: { typeName } } }, - { strictNaming: true, schemaPrefix: 'xyz' } + { strictNaming: true, schemaPrefix: 'xyz', resolveExternal: true } ); expect( @@ -86,7 +148,7 @@ describe('OpenApiDocumentRefs', () => { ...emptyDocument, components: { schemas: { '123456': {}, 'something.Else%': {} } } }, - { strictNaming: false, schemaPrefix: '' } + { strictNaming: false, schemaPrefix: '', resolveExternal: true } ); expect(refs.getSchemaNaming('#/components/schemas/123456')).toEqual({ fileName: 'schema-123456', @@ -109,7 +171,7 @@ describe('OpenApiDocumentRefs', () => { schemas: { '123456': {}, schema123456: {}, schema12345_6: {} } } }, - { strictNaming: false, schemaPrefix: '' } + { strictNaming: false, schemaPrefix: '', resolveExternal: true } ); expect(refs.getSchemaNaming('#/components/schemas/123456')).toEqual({ fileName: 'schema-123456', @@ -139,7 +201,7 @@ describe('OpenApiDocumentRefs', () => { ...emptyDocument, components: { schemas: { '123456': {} } } }, - { strictNaming: true, schemaPrefix: '' } + { strictNaming: true, schemaPrefix: '', resolveExternal: true } ) ).rejects.toThrowError( 'The service specification contains invalid schema names.' @@ -152,7 +214,7 @@ describe('OpenApiDocumentRefs', () => { ...emptyDocument, components: { schemas: { name: {}, Name: {} } } }, - { strictNaming: false, schemaPrefix: '' } + { strictNaming: false, schemaPrefix: '', resolveExternal: true } ); expect(refs.getSchemaNaming('#/components/schemas/name')).toEqual({ fileName: 'name-1', @@ -171,7 +233,7 @@ describe('OpenApiDocumentRefs', () => { ...emptyDocument, components: { schemas: { name400: {}, Name400: {} } } }, - { strictNaming: false, schemaPrefix: '' } + { strictNaming: false, schemaPrefix: '', resolveExternal: true } ); expect(refs.getSchemaNaming('#/components/schemas/name400')).toEqual({ fileName: 'name-400-1', diff --git a/packages/openapi-generator/src/parser/refs.ts b/packages/openapi-generator/src/parser/refs.ts index 5601ca0a59..492fff428f 100644 --- a/packages/openapi-generator/src/parser/refs.ts +++ b/packages/openapi-generator/src/parser/refs.ts @@ -1,8 +1,8 @@ import { pascalCase, kebabCase } from '@sap-cloud-sdk/util'; +import SwaggerParser from '@apidevtools/swagger-parser'; import { isReferenceObject } from '../schema-util'; import { ensureUniqueNames } from './unique-naming'; import { ensureValidSchemaNames } from './schema-naming'; -import { resolveBound } from './swagger-parser-workaround'; import type { OpenAPIV3 } from 'openapi-types'; import type { $Refs } from '@apidevtools/swagger-parser'; import type { SchemaNaming } from '../openapi-types'; @@ -40,7 +40,9 @@ export class OpenApiDocumentRefs { options: ParserOptions ): Promise { return new OpenApiDocumentRefs( - await resolveBound(document), + await SwaggerParser.resolve(document, { + resolve: { external: options.resolveExternal } + }), OpenApiDocumentRefs.parseSchemaRefMapping(document, options) ); } diff --git a/packages/openapi-generator/src/parser/request-body.spec.ts b/packages/openapi-generator/src/parser/request-body.spec.ts index 122b847010..b39821d8d2 100644 --- a/packages/openapi-generator/src/parser/request-body.spec.ts +++ b/packages/openapi-generator/src/parser/request-body.spec.ts @@ -2,7 +2,11 @@ import { createLogger } from '@sap-cloud-sdk/util'; import { createTestRefs } from '../../test/test-util'; import { parseRequestBody } from './request-body'; -const defaultOptions = { strictNaming: true, schemaPrefix: '' }; +const defaultOptions = { + strictNaming: true, + schemaPrefix: '', + resolveExternal: true +}; describe('getRequestBody', () => { it('returns undefined for undefined', async () => { const logger = createLogger('openapi-generator'); diff --git a/packages/openapi-generator/src/parser/responses.spec.ts b/packages/openapi-generator/src/parser/responses.spec.ts index 63cebf6ec7..32d8138f8f 100644 --- a/packages/openapi-generator/src/parser/responses.spec.ts +++ b/packages/openapi-generator/src/parser/responses.spec.ts @@ -1,7 +1,11 @@ import { createTestRefs } from '../../test/test-util'; import { parseResponses } from './responses'; -const defaultOptions = { strictNaming: true, schemaPrefix: '' }; +const defaultOptions = { + strictNaming: true, + schemaPrefix: '', + resolveExternal: true +}; describe('parseResponses', () => { it('parses response schema without content', async () => { expect( diff --git a/packages/openapi-generator/src/parser/schema-naming.spec.ts b/packages/openapi-generator/src/parser/schema-naming.spec.ts index 1a80cbfa60..08c08494ae 100644 --- a/packages/openapi-generator/src/parser/schema-naming.spec.ts +++ b/packages/openapi-generator/src/parser/schema-naming.spec.ts @@ -5,7 +5,8 @@ describe('schema-naming', () => { expect( ensureValidSchemaNames(['1234ABC'], { strictNaming: false, - schemaPrefix: '' + schemaPrefix: '', + resolveExternal: true }) ).toEqual(['schema1234ABC']); }); @@ -14,7 +15,8 @@ describe('schema-naming', () => { expect( ensureValidSchemaNames(['#som%eth.ing'], { strictNaming: false, - schemaPrefix: '' + schemaPrefix: '', + resolveExternal: true }) ).toEqual(['something']); }); @@ -23,7 +25,8 @@ describe('schema-naming', () => { expect(() => ensureValidSchemaNames(['1234ABC', '#som%eth.ing'], { strictNaming: true, - schemaPrefix: '' + schemaPrefix: '', + resolveExternal: true }) ).toThrowErrorMatchingInlineSnapshot(` "The service specification contains invalid schema names. Adjust the definition file or enable automatic name adjustment with \`skipValidation\`. diff --git a/packages/openapi-generator/src/parser/schema.spec.ts b/packages/openapi-generator/src/parser/schema.spec.ts index 8d2b7cba87..f93286c4c4 100644 --- a/packages/openapi-generator/src/parser/schema.spec.ts +++ b/packages/openapi-generator/src/parser/schema.spec.ts @@ -6,7 +6,11 @@ import type { OpenAPIV3 } from 'openapi-types'; describe('schema parser', () => { describe('parseSchema()', () => { - const defaultOptions = { strictNaming: true, schemaPrefix: '' }; + const defaultOptions = { + strictNaming: true, + schemaPrefix: '', + resolveExternal: true + }; it('parses reference schema', async () => { const schema = { $ref: '#/components/schemas/test' }; @@ -350,7 +354,8 @@ describe('schema parser', () => { }; parseSchema(schema, await createTestRefs(), { strictNaming: false, - schemaPrefix: '' + schemaPrefix: '', + resolveExternal: true }); expect(logger.warn).toHaveBeenCalledWith( 'null was used as a parameter in an enum, although the schema was not declared as nullable' diff --git a/packages/openapi-generator/src/parser/swagger-parser-workaround.ts b/packages/openapi-generator/src/parser/swagger-parser-workaround.ts deleted file mode 100644 index 8fb87fee73..0000000000 --- a/packages/openapi-generator/src/parser/swagger-parser-workaround.ts +++ /dev/null @@ -1,33 +0,0 @@ -import SwaggerParser, { parse, resolve } from '@apidevtools/swagger-parser'; -import type { OpenAPIV3 } from 'openapi-types'; -import type { $Refs } from '@apidevtools/swagger-parser'; - -/** - * These to methods are a workaround until the swagger-parser is updated: https://github.com/APIDevTools/swagger-parser/issues/186 - * In the parser the `this` context is used for exported methods of a module. - * TypeScript has tightened the rules for this in version 4.4.: https://github.com/Microsoft/TypeScript-wiki/blob/main/Breaking-Changes.md#typescript-44. - * In order to update the TypeScript version we introduce these methods to set the `this` pointer explicitly. - * Once the swagger parser updates we can remove this. - */ - -/** - * Resolves a document. - * @param document - document to be resolved - * @returns resolve document - * @internal - */ -export async function resolveBound( - document: OpenAPIV3.Document -): Promise<$Refs> { - return resolve.bind(SwaggerParser)(document); -} - -/** - * Parses a document - * @param content - content to be parsed - * @returns resolve document - * @internal - */ -export async function parseBound(content: any): Promise { - return parse.bind(SwaggerParser)(content); -} diff --git a/packages/openapi-generator/src/parser/unique-naming.spec.ts b/packages/openapi-generator/src/parser/unique-naming.spec.ts index 4af7727a48..be4b7f1ccd 100644 --- a/packages/openapi-generator/src/parser/unique-naming.spec.ts +++ b/packages/openapi-generator/src/parser/unique-naming.spec.ts @@ -3,7 +3,11 @@ import { ensureUniqueNames } from './unique-naming'; describe('ensureUniqueNames', () => { describe('with strictNaming enabled', () => { - const options = { strictNaming: true, schemaPrefix: '' }; + const options = { + strictNaming: true, + schemaPrefix: '', + resolveExternal: true + }; it('does not throw for empty list of names', () => { expect(() => ensureUniqueNames([], options)).not.toThrow(); }); @@ -67,7 +71,11 @@ describe('ensureUniqueNames', () => { }); describe('with strictNaming disabled', () => { - const options = { strictNaming: false, schemaPrefix: '' }; + const options = { + strictNaming: false, + schemaPrefix: '', + resolveExternal: true + }; it('replaces duplicate names using defaults', () => { expect(ensureUniqueNames(['duplicate', 'duplicate'], options)).toEqual([ diff --git a/packages/openapi-generator/test/test-util.ts b/packages/openapi-generator/test/test-util.ts index 4920fa7c79..7c3dca0761 100644 --- a/packages/openapi-generator/test/test-util.ts +++ b/packages/openapi-generator/test/test-util.ts @@ -14,7 +14,7 @@ export function createTestRefs( ): Promise { return createRefs( { ...emptyDocument, components }, - { strictNaming: true, schemaPrefix: '' } + { strictNaming: true, schemaPrefix: '', resolveExternal: true } ); } diff --git a/test-packages/test-services-openapi/generate-openapi-services.ts b/test-packages/test-services-openapi/generate-openapi-services.ts index 24049fdfc7..b718c96fc5 100644 --- a/test-packages/test-services-openapi/generate-openapi-services.ts +++ b/test-packages/test-services-openapi/generate-openapi-services.ts @@ -14,28 +14,30 @@ const generatorConfigOpenApi: Partial = { }; async function generateOpenApi() { - await generate({ - ...generatorConfigOpenApi, - input: resolve( - '..', - '..', - 'test-resources', - 'openapi-service-specs', - 'specifications' - ), - outputDir: resolve('.'), - transpile: true, - optionsPerService: resolve( - '..', - '..', - 'test-resources', - 'openapi-service-specs', - 'config' - ) - }).catch(reason => { - logger.error(`Unhandled rejection at: ${reason}`); + try { + await generate({ + ...generatorConfigOpenApi, + input: resolve( + '..', + '..', + 'test-resources', + 'openapi-service-specs', + 'specifications' + ), + outputDir: resolve('.'), + transpile: true, + optionsPerService: resolve( + '..', + '..', + 'test-resources', + 'openapi-service-specs', + 'config' + ) + }); + } catch (error) { + logger.error(`Unhandled rejection at: ${error}`); process.exit(1); - }); + } } generateOpenApi();