diff --git a/src/codegen/inputs/openapi/generators/payloads.ts b/src/codegen/inputs/openapi/generators/payloads.ts index 15809b24..0cd10fc7 100644 --- a/src/codegen/inputs/openapi/generators/payloads.ts +++ b/src/codegen/inputs/openapi/generators/payloads.ts @@ -214,18 +214,23 @@ function extractPayloadsFromOperations( } if (responseSchema) { + // Copy before decorating: after dereferencing the schema object is + // shared with the component entry and every other usage site, so + // mutating it would leak status codes across operations. + const decoratedSchema: any = { + ...responseSchema, + $id: `${operationId}_Response_${statusCode}` + }; + // Add status code information for proper discrimination if (statusCode !== 'default' && !isNaN(Number(statusCode))) { hasStatusCodes = true; - responseSchema['x-modelina-status-codes'] = { + decoratedSchema['x-modelina-status-codes'] = { code: Number(statusCode) }; } - responseSchemas.push({ - ...responseSchema, - $id: `${operationId}_Response_${statusCode}` - }); + responseSchemas.push(decoratedSchema); } } @@ -260,21 +265,35 @@ function extractComponentSchemas( ): {schema: any; schemaId: string}[] { const componentSchemas: {schema: any; schemaId: string}[] = []; + const pushComponentSchema = (schemaName: string, schema: unknown) => { + if (!schema || typeof schema !== 'object') { + return; + } + // Prefer the name reflected onto the schema at parse time so the + // standalone component model and the nested models split out of + // operation payloads (which share this exact schema object) resolve to + // the same model name and file. + const inferredName = (schema as Record)[ + 'x-modelgen-inferred-name' + ]; + const modelName = + typeof inferredName === 'string' ? inferredName : schemaName; + componentSchemas.push({ + schema: { + ...schema, + $id: modelName, + $schema: JSON_SCHEMA_DRAFT_07 + }, + schemaId: modelName + }); + }; + // OpenAPI 3.x components if ('components' in openapiDocument && openapiDocument.components?.schemas) { for (const [schemaName, schema] of Object.entries( openapiDocument.components.schemas )) { - if (schema && typeof schema === 'object') { - componentSchemas.push({ - schema: { - ...schema, - $id: schemaName, - $schema: JSON_SCHEMA_DRAFT_07 - }, - schemaId: schemaName - }); - } + pushComponentSchema(schemaName, schema); } } @@ -283,16 +302,7 @@ function extractComponentSchemas( for (const [schemaName, schema] of Object.entries( openapiDocument.definitions )) { - if (schema && typeof schema === 'object') { - componentSchemas.push({ - schema: { - ...schema, - $id: schemaName, - $schema: JSON_SCHEMA_DRAFT_07 - }, - schemaId: schemaName - }); - } + pushComponentSchema(schemaName, schema); } } diff --git a/src/codegen/inputs/openapi/parser.ts b/src/codegen/inputs/openapi/parser.ts index 25785c32..1adf0631 100644 --- a/src/codegen/inputs/openapi/parser.ts +++ b/src/codegen/inputs/openapi/parser.ts @@ -9,6 +9,7 @@ import {createInputDocumentError} from '../../errors'; import {isRemoteUrl} from '../../../utils/inputSource'; import {fetchRemoteDocument} from '../../../utils/remoteFetch'; import {createOpenapiRefParserResolver} from '../../../utils/refResolvers'; +import {reflectComponentSchemaNames} from './utils'; export async function loadOpenapi( context: RunGeneratorContext @@ -77,6 +78,8 @@ export async function loadOpenapiDocument( } } + reflectComponentSchemaNames(dereferenced); + Logger.debug(`OpenAPI document loaded and dereferenced`); return dereferenced; } catch (error) { diff --git a/src/codegen/inputs/openapi/utils.ts b/src/codegen/inputs/openapi/utils.ts index a369db11..e906983d 100644 --- a/src/codegen/inputs/openapi/utils.ts +++ b/src/codegen/inputs/openapi/utils.ts @@ -2,6 +2,89 @@ * Shared helpers for deriving stable identifiers from OpenAPI operations. */ import {FormatHelpers} from '@asyncapi/modelina'; +import {OpenAPIV2, OpenAPIV3, OpenAPIV3_1} from 'openapi-types'; + +/** + * Modelina's name-hint extension. It has the lowest naming precedence + * (`title || $id || x-modelgen-inferred-name`), so it never overrides names + * the spec author already provided, and unlike `$id` it carries no base-URI + * semantics that could confuse downstream JSON Schema tooling. + */ +const MODELINA_INFERRED_NAME = 'x-modelgen-inferred-name'; + +/** + * Tag every component/definition schema with its component name so the name + * survives dereferencing. + * + * Dereferencing inlines `$ref` targets by object reference, which means the + * component names are otherwise lost and Modelina falls back to naming nested + * models after the property path (e.g. every `items` array item becomes + * `ItemsItem`). Different components then collide on the same model name and + * silently overwrite each other's generated files. Because the inlined + * occurrences share object identity with the component entry, tagging the + * component here propagates the name to every usage site. + * + * Must be called on the *dereferenced* document. + */ +export function reflectComponentSchemaNames( + document: OpenAPIV3.Document | OpenAPIV2.Document | OpenAPIV3_1.Document +): void { + const schemaMaps: Record[] = []; + if ('components' in document && document.components?.schemas) { + schemaMaps.push(document.components.schemas); + } + if ('definitions' in document && document.definitions) { + schemaMaps.push(document.definitions); + } + + for (const schemas of schemaMaps) { + const chosenNames = chooseComponentModelNames(Object.keys(schemas)); + for (const [componentName, schema] of Object.entries(schemas)) { + if ( + schema && + typeof schema === 'object' && + !(MODELINA_INFERRED_NAME in schema) + ) { + // eslint-disable-next-line security/detect-object-injection + (schema as Record)[MODELINA_INFERRED_NAME] = + chosenNames.get(componentName); + } + } + } +} + +/** + * Pick a model name per component: the last dot-segment when it is + * unambiguous across all components (`GetTransactions.Model.TransactionModel` + * -> `TransactionModel`), otherwise the full component name. Namespaced + * component names are common in generated specs (NSwag, Swashbuckle) and the + * full name makes for unwieldy class names. + */ +export function chooseComponentModelNames( + componentNames: string[] +): Map { + const fullNames = new Set(componentNames); + const shortNameCounts = new Map(); + const shortNameOf = (name: string): string => { + const segments = name.split('.').filter((segment) => segment.length > 0); + return segments.at(-1) ?? name; + }; + + for (const name of componentNames) { + const short = shortNameOf(name); + shortNameCounts.set(short, (shortNameCounts.get(short) ?? 0) + 1); + } + + const chosen = new Map(); + for (const name of componentNames) { + const short = shortNameOf(name); + const isUnambiguous = + short === name || + (shortNameCounts.get(short) === 1 && !fullNames.has(short)); + chosen.set(name, isUnambiguous ? short : name); + } + return chosen; +} /** * Derive the operation identifier used to correlate payloads, parameters, diff --git a/test/codegen/generators/typescript/payload.spec.ts b/test/codegen/generators/typescript/payload.spec.ts index 078c9df4..12fa5e6b 100644 --- a/test/codegen/generators/typescript/payload.spec.ts +++ b/test/codegen/generators/typescript/payload.spec.ts @@ -155,6 +155,35 @@ describe('payloads', () => { }, ]); }); + it('should preserve component names for nested array models across operations', async () => { + // Regression: dereferencing used to drop component names, so every + // `items` array item was named `ItemsItem` and models from different + // operations silently overwrote each other's files. + const parsedOpenAPIDocument = await loadOpenapiDocument(path.resolve(__dirname, '../../../configs/openapi-shared-items.json')); + + const renderedContent = await generateTypescriptPayload({ + generator: { + ...defaultTypeScriptPayloadGenerator, + outputPath: path.resolve(__dirname, './output') + }, + inputType: 'openapi', + openapiDocument: parsedOpenAPIDocument, + dependencyOutputs: { } + }); + + const fileNames = renderedContent.files.map((file) => path.basename(file.path)); + expect(fileNames).not.toContain('ItemsItem.ts'); + expect(fileNames).toContain('TransactionModel.ts'); + expect(fileNames).toContain('UserModel.ts'); + + const transactionFile = renderedContent.files.find((file) => file.path.endsWith('TransactionModel.ts')); + expect(transactionFile?.content).toContain('transactionId'); + + const transactionsResponse = renderedContent.files.find((file) => file.path.endsWith('GetV2TransactionsResponse_200.ts')); + expect(transactionsResponse?.content).toContain('TransactionModel[]'); + const usersResponse = renderedContent.files.find((file) => file.path.endsWith('GetV2UsersResponse_200.ts')); + expect(usersResponse?.content).toContain('UserModel[]'); + }); it('should work with basic OpenAPI 3.0 inputs', async () => { const parsedOpenAPIDocument = await loadOpenapiDocument(path.resolve(__dirname, '../../../configs/openapi-3.json')); diff --git a/test/codegen/inputs/openapi/utils.spec.ts b/test/codegen/inputs/openapi/utils.spec.ts index b4adb767..aa2c9910 100644 --- a/test/codegen/inputs/openapi/utils.spec.ts +++ b/test/codegen/inputs/openapi/utils.spec.ts @@ -1,4 +1,9 @@ -import {deriveOperationId} from '../../../../src/codegen/inputs/openapi/utils'; +import { + chooseComponentModelNames, + deriveOperationId, + reflectComponentSchemaNames +} from '../../../../src/codegen/inputs/openapi/utils'; +import {OpenAPIV3} from 'openapi-types'; describe('OpenAPI operation id derivation', () => { describe('deriveOperationId', () => { @@ -37,3 +42,70 @@ describe('OpenAPI operation id derivation', () => { }); }); }); + +describe('OpenAPI component schema naming', () => { + describe('chooseComponentModelNames', () => { + it('shortens namespaced names to their last segment when unambiguous', () => { + const chosen = chooseComponentModelNames([ + 'GetTransactions.Model.TransactionModel', + 'GetUsers.Model.UserModel' + ]); + expect(chosen.get('GetTransactions.Model.TransactionModel')).toEqual( + 'TransactionModel' + ); + expect(chosen.get('GetUsers.Model.UserModel')).toEqual('UserModel'); + }); + + it('keeps the full name when the last segment is shared', () => { + const chosen = chooseComponentModelNames([ + 'GetTransactions.Model', + 'GetUsers.Model' + ]); + expect(chosen.get('GetTransactions.Model')).toEqual( + 'GetTransactions.Model' + ); + expect(chosen.get('GetUsers.Model')).toEqual('GetUsers.Model'); + }); + + it('keeps the full name when the short name equals another full name', () => { + const chosen = chooseComponentModelNames([ + 'Transaction', + 'Legacy.Transaction' + ]); + expect(chosen.get('Transaction')).toEqual('Transaction'); + expect(chosen.get('Legacy.Transaction')).toEqual('Legacy.Transaction'); + }); + }); + + describe('reflectComponentSchemaNames', () => { + it('tags component schemas without overriding existing name hints', () => { + const preTagged = { + type: 'object', + 'x-modelgen-inferred-name': 'AlreadyNamed' + }; + const document = { + openapi: '3.0.0', + info: {title: 'Test', version: '1.0.0'}, + paths: {}, + components: { + schemas: { + 'GetTransactions.Model.TransactionModel': {type: 'object'}, + Existing: preTagged + } + } + } as unknown as OpenAPIV3.Document; + + reflectComponentSchemaNames(document); + + const schemas = document.components?.schemas as Record; + expect( + schemas['GetTransactions.Model.TransactionModel'][ + 'x-modelgen-inferred-name' + ] + ).toEqual('TransactionModel'); + expect(schemas['Existing']['x-modelgen-inferred-name']).toEqual( + 'AlreadyNamed' + ); + }); + }); +}); diff --git a/test/configs/openapi-shared-items.json b/test/configs/openapi-shared-items.json new file mode 100644 index 00000000..3f6ae2ca --- /dev/null +++ b/test/configs/openapi-shared-items.json @@ -0,0 +1,105 @@ +{ + "openapi": "3.0.1", + "info": { + "title": "Shared items collision API", + "version": "1.0.0" + }, + "paths": { + "/v2/users": { + "get": { + "operationId": "getV2Users", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/GetUsers.Model" + } + } + } + } + } + } + }, + "/v2/transactions": { + "get": { + "operationId": "getV2Transactions", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/GetTransactions.Model" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "GetUsers.Model": { + "type": "object", + "properties": { + "items": { + "type": "array", + "items": { + "$ref": "#/components/schemas/GetUsers.Model.UserModel" + } + }, + "count": { + "type": "integer" + } + } + }, + "GetUsers.Model.UserModel": { + "required": ["safepayAccountId", "name"], + "type": "object", + "properties": { + "safepayAccountId": { + "type": "string" + }, + "name": { + "type": "string" + } + } + }, + "GetTransactions.Model": { + "type": "object", + "properties": { + "items": { + "type": "array", + "items": { + "$ref": "#/components/schemas/GetTransactions.Model.TransactionModel" + } + }, + "count": { + "type": "integer" + } + } + }, + "GetTransactions.Model.TransactionModel": { + "required": ["transactionId", "amount", "currency", "status"], + "type": "object", + "properties": { + "transactionId": { + "type": "string" + }, + "amount": { + "type": "number" + }, + "currency": { + "type": "string" + }, + "status": { + "type": "string" + } + } + } + } + } +}