From 2c48dd4a60c6550021f269bb7f5d6df1d29a6bfa Mon Sep 17 00:00:00 2001 From: Lagoni <68890168+ALagoni97@users.noreply.github.com> Date: Wed, 15 Jul 2026 19:19:17 +0200 Subject: [PATCH] fix(openapi): preserve component schema names through dereferencing Dereferencing inlines $ref targets and discards component names, so Modelina named nested models after property paths - every 'items' array item became 'ItemsItem', and models from different operations collided on the same file and silently overwrote each other (a transactions response ended up typed with the users item model). Tag each components.schemas / definitions entry with Modelina's x-modelgen-inferred-name hint right after dereferencing. Inlined occurrences share object identity with the component entry, so the name propagates to every usage site. Namespaced names shorten to their last dot-segment when unambiguous (GetTransactions.Model.TransactionModel -> TransactionModel), otherwise the full name is used. Standalone component models reuse the same name so both generation paths resolve to a single identical file. Also copy response schemas before decorating with x-modelina-status-codes: the schema object is shared post-dereference, so mutating it leaked status codes across operations. Co-Authored-By: Claude Opus 4.8 --- .../inputs/openapi/generators/payloads.ts | 60 +++++----- src/codegen/inputs/openapi/parser.ts | 3 + src/codegen/inputs/openapi/utils.ts | 83 ++++++++++++++ .../generators/typescript/payload.spec.ts | 29 +++++ test/codegen/inputs/openapi/utils.spec.ts | 74 +++++++++++- test/configs/openapi-shared-items.json | 105 ++++++++++++++++++ 6 files changed, 328 insertions(+), 26 deletions(-) create mode 100644 test/configs/openapi-shared-items.json 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" + } + } + } + } + } +}