Skip to content

Commit b82d54b

Browse files
mrlubosmrlubos
authored
Merge pull request #3417 from hey-api/copilot/fix-hoisting-sibling-schemas
Use unprefixed schema names from external files unless conflicts exist
2 parents 918ed5d + dd48a08 commit b82d54b

325 files changed

Lines changed: 1322 additions & 908 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.changeset/clean-llamas-learn.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@hey-api/json-schema-ref-parser": patch
3+
"@hey-api/openapi-ts": patch
4+
---
5+
6+
**parser**: prefer unprefixed schema names from external files

‎packages/json-schema-ref-parser/.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
.gen

‎packages/json-schema-ref-parser/src/__tests__/__snapshots__/circular-ref-with-description.json‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"schemas": {
3+
"Foo": {
4+
"$ref": "#/schemas/Bar"
5+
},
6+
"Bar": {
7+
"description": "ok",
8+
"$ref": "#/schemas/Foo"
9+
}
10+
}
11+
}

‎packages/json-schema-ref-parser/src/__tests__/__snapshots__/main-with-external-siblings.json‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
{
2+
"openapi": "3.0.0",
3+
"info": {
4+
"title": "Test API",
5+
"version": "1.0.0"
6+
},
7+
"paths": {
8+
"/resolution": {
9+
"get": {
10+
"summary": "Get resolution step",
11+
"responses": {
12+
"200": {
13+
"description": "Success",
14+
"content": {
15+
"application/json": {
16+
"schema": {
17+
"$ref": "#/components/schemas/ResolutionStep"
18+
}
19+
}
20+
}
21+
}
22+
}
23+
}
24+
},
25+
"/action": {
26+
"get": {
27+
"summary": "Get action info",
28+
"responses": {
29+
"200": {
30+
"description": "Success",
31+
"content": {
32+
"application/json": {
33+
"schema": {
34+
"$ref": "#/components/schemas/ActionInfo"
35+
}
36+
}
37+
}
38+
}
39+
}
40+
}
41+
}
42+
},
43+
"components": {
44+
"schemas": {
45+
"ActionInfo": {
46+
"type": "object",
47+
"properties": {
48+
"ActionId": {
49+
"type": "string"
50+
}
51+
}
52+
},
53+
"ResolutionStep": {
54+
"type": "object",
55+
"properties": {
56+
"ResolutionType": {
57+
"oneOf": [
58+
{
59+
"$ref": "#/components/schemas/ResolutionType"
60+
}
61+
]
62+
},
63+
"ActionName": {
64+
"type": "string"
65+
}
66+
}
67+
},
68+
"ResolutionType": {
69+
"type": "string",
70+
"enum": [
71+
"ContactVendor",
72+
"ResetToDefaults",
73+
"RetryOperation"
74+
]
75+
}
76+
}
77+
}
78+
}

‎packages/json-schema-ref-parser/src/__tests__/__snapshots__/multiple-refs.json‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
{
2+
"paths": {
3+
"/test1/{pathId}": {
4+
"get": {
5+
"summary": "First endpoint using the same pathId schema",
6+
"parameters": [
7+
{
8+
"$ref": "#/components/parameters/pathId"
9+
}
10+
],
11+
"responses": {
12+
"200": {
13+
"description": "Test 1 response"
14+
}
15+
}
16+
}
17+
},
18+
"/test2/{pathId}": {
19+
"get": {
20+
"summary": "Second endpoint using the same pathId schema",
21+
"parameters": [
22+
{
23+
"$ref": "#/components/parameters/pathId"
24+
}
25+
],
26+
"responses": {
27+
"200": {
28+
"description": "Test 2 response"
29+
}
30+
}
31+
}
32+
}
33+
},
34+
"components": {
35+
"parameters": {
36+
"pathId": {
37+
"name": "pathId",
38+
"in": "path",
39+
"required": true,
40+
"schema": {
41+
"type": "string",
42+
"format": "uuid",
43+
"description": "Unique identifier for the path"
44+
}
45+
}
46+
}
47+
}
48+
}

‎packages/json-schema-ref-parser/src/__tests__/__snapshots__/redfish-like.json‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
{
2+
"openapi": "3.0.0",
3+
"info": {
4+
"title": "Redfish-like API",
5+
"version": "1.0.0",
6+
"description": "Test API simulating Redfish structure with versioned schemas"
7+
},
8+
"paths": {
9+
"/redfish/v1/Systems": {
10+
"get": {
11+
"summary": "Get Systems",
12+
"responses": {
13+
"200": {
14+
"description": "Success",
15+
"content": {
16+
"application/json": {
17+
"schema": {
18+
"$ref": "#/components/schemas/ResolutionStep_v1_0_1_ResolutionStep"
19+
}
20+
}
21+
}
22+
},
23+
"default": {
24+
"description": "Error"
25+
}
26+
}
27+
}
28+
},
29+
"/redfish/v1/Actions": {
30+
"post": {
31+
"summary": "Submit Action",
32+
"responses": {
33+
"200": {
34+
"description": "Success",
35+
"content": {
36+
"application/json": {
37+
"schema": {
38+
"$ref": "#/components/schemas/ResolutionStep_v1_0_1_ActionParameters"
39+
}
40+
}
41+
}
42+
}
43+
}
44+
}
45+
}
46+
},
47+
"components": {
48+
"schemas": {
49+
"ResolutionStep_v1_0_1_ActionParameters": {
50+
"type": "object",
51+
"properties": {
52+
"ActionId": {
53+
"type": "string"
54+
},
55+
"ActionType": {
56+
"$ref": "#/components/schemas/ResolutionStep_v1_0_1_ResolutionType"
57+
}
58+
}
59+
},
60+
"ResolutionStep_v1_0_1_ResolutionStep": {
61+
"type": "object",
62+
"properties": {
63+
"ResolutionType": {
64+
"oneOf": [
65+
{
66+
"$ref": "#/components/schemas/ResolutionStep_v1_0_1_ResolutionType"
67+
}
68+
]
69+
},
70+
"ActionName": {
71+
"type": "string",
72+
"description": "Name of the action"
73+
}
74+
}
75+
},
76+
"ResolutionStep_v1_0_1_ResolutionType": {
77+
"type": "string",
78+
"enum": [
79+
"ContactVendor",
80+
"ResetToDefaults",
81+
"RetryOperation"
82+
],
83+
"description": "Types of resolution actions"
84+
}
85+
}
86+
}
87+
}

‎packages/json-schema-ref-parser/src/__tests__/bundle.test.ts‎

Lines changed: 59 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,38 @@
1+
import fs from 'node:fs';
12
import path from 'node:path';
3+
import { fileURLToPath } from 'node:url';
24

35
import { $RefParser } from '..';
46
import { getSpecsPath } from './utils';
57

8+
const __filename = fileURLToPath(import.meta.url);
9+
const __dirname = path.dirname(__filename);
10+
11+
const getSnapshotsPath = () => path.join(__dirname, '__snapshots__');
12+
const getTempSnapshotsPath = () => path.join(__dirname, '.gen', 'snapshots');
13+
14+
/**
15+
* Helper function to compare a bundled schema with a snapshot file.
16+
* Handles writing the schema to a temp file and comparing with the snapshot.
17+
*
18+
* @param schema - The bundled schema to compare
19+
* @param snapshotName - The name of the snapshot file (e.g., 'circular-ref-with-description.json')
20+
*/
21+
const expectBundledSchemaToMatchSnapshot = async (schema: unknown, snapshotName: string) => {
22+
const outputPath = path.join(getTempSnapshotsPath(), snapshotName);
23+
const snapshotPath = path.join(getSnapshotsPath(), snapshotName);
24+
25+
// Ensure directory exists
26+
fs.mkdirSync(path.dirname(outputPath), { recursive: true });
27+
28+
// Write the bundled result
29+
const content = JSON.stringify(schema, null, 2);
30+
fs.writeFileSync(outputPath, content);
31+
32+
// Compare with snapshot
33+
await expect(content).toMatchFileSnapshot(snapshotPath);
34+
};
35+
636
describe('bundle', () => {
737
it('handles circular reference with description', async () => {
838
const refParser = new $RefParser();
@@ -12,17 +42,8 @@ describe('bundle', () => {
1242
'circular-ref-with-description.json',
1343
);
1444
const schema = await refParser.bundle({ pathOrUrlOrSchema });
15-
expect(schema).toEqual({
16-
schemas: {
17-
Bar: {
18-
$ref: '#/schemas/Foo',
19-
description: 'ok',
20-
},
21-
Foo: {
22-
$ref: '#/schemas/Bar',
23-
},
24-
},
25-
});
45+
46+
await expectBundledSchemaToMatchSnapshot(schema, 'circular-ref-with-description.json');
2647
});
2748

2849
it('bundles multiple references to the same file correctly', async () => {
@@ -32,28 +53,32 @@ describe('bundle', () => {
3253
'json-schema-ref-parser',
3354
'multiple-refs.json',
3455
);
35-
const schema = (await refParser.bundle({ pathOrUrlOrSchema })) as any;
36-
37-
// Both parameters should now be $ref to the same internal definition
38-
const firstParam = schema.paths['/test1/{pathId}'].get.parameters[0];
39-
const secondParam = schema.paths['/test2/{pathId}'].get.parameters[0];
40-
41-
// The $ref should match the output structure in file_context_0
42-
expect(firstParam.$ref).toBe('#/components/parameters/path-parameter_pathId');
43-
expect(secondParam.$ref).toBe('#/components/parameters/path-parameter_pathId');
44-
45-
// The referenced parameter should exist and match the expected structure
46-
expect(schema.components).toBeDefined();
47-
expect(schema.components.parameters).toBeDefined();
48-
expect(schema.components.parameters['path-parameter_pathId']).toEqual({
49-
in: 'path',
50-
name: 'pathId',
51-
required: true,
52-
schema: {
53-
description: 'Unique identifier for the path',
54-
format: 'uuid',
55-
type: 'string',
56-
},
57-
});
56+
const schema = await refParser.bundle({ pathOrUrlOrSchema });
57+
58+
await expectBundledSchemaToMatchSnapshot(schema, 'multiple-refs.json');
59+
});
60+
61+
it('hoists sibling schemas from external files', async () => {
62+
const refParser = new $RefParser();
63+
const pathOrUrlOrSchema = path.join(
64+
getSpecsPath(),
65+
'json-schema-ref-parser',
66+
'main-with-external-siblings.json',
67+
);
68+
const schema = await refParser.bundle({ pathOrUrlOrSchema });
69+
70+
await expectBundledSchemaToMatchSnapshot(schema, 'main-with-external-siblings.json');
71+
});
72+
73+
it('hoists sibling schemas from YAML files with versioned names (Redfish-like)', async () => {
74+
const refParser = new $RefParser();
75+
const pathOrUrlOrSchema = path.join(
76+
getSpecsPath(),
77+
'json-schema-ref-parser',
78+
'redfish-like.yaml',
79+
);
80+
const schema = await refParser.bundle({ pathOrUrlOrSchema });
81+
82+
await expectBundledSchemaToMatchSnapshot(schema, 'redfish-like.json');
5883
});
5984
});

‎packages/json-schema-ref-parser/src/bundle.ts‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -586,7 +586,22 @@ function remap(parser: $RefParser, inventory: Array<InventoryEntry>) {
586586
} catch {
587587
// Ignore errors
588588
}
589-
const proposed = `${proposedBase}_${lastToken(entry.hash)}`;
589+
590+
// Try without prefix first (cleaner names)
591+
const schemaName = lastToken(entry.hash);
592+
let proposed = schemaName;
593+
594+
// Check if this name would conflict with existing schemas from other files
595+
if (!usedNamesByObj.has(container)) {
596+
usedNamesByObj.set(container, new Set<string>(Object.keys(container || {})));
597+
}
598+
const used = usedNamesByObj.get(container)!;
599+
600+
// If the name is already used, add the file prefix
601+
if (used.has(proposed)) {
602+
proposed = `${proposedBase}_${schemaName}`;
603+
}
604+
590605
defName = uniqueName(container, proposed);
591606
namesForPrefix.set(targetKey, defName);
592607
// Store the resolved value under the container

‎packages/openapi-ts-tests/main/test/__snapshots__/2.0.x/external/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
// This file is auto-generated by @hey-api/openapi-ts
22

3-
export type { ClientOptions, ExternalAllOfSchema, ExternalAnyOfSchema, ExternalArraySchema, ExternalDoubleNestedNumeric, ExternalDoubleNestedProp, ExternalMixedProperties, ExternalNestedNumericObjectA, ExternalNestedNumericObjectB, ExternalNestedObjectA, ExternalNestedObjectB, ExternalSchemaA, ExternalSchemaB, ExternalSchemaC, ExternalSchemaExternalProp, ExternalSchemaExternalPropAlias, ExternalSchemaPathA, ExternalSchemaPathB, ExternalSchemaPropertyA, ExternalSchemaPropertyB, ExternalSchemaPropertyC, ExternalSchemaPropertyD, ExternalShared1, ExternalSharedDeep, ExternalSharedExternalNested, ExternalSharedExternalNestedNumeric, ExternalSharedExternalSharedModel, ExternalSharedExternalSharedModelWithUuid, ExternalSharedId, ExternalSharedName, ExternalUnionSchema, GetExternalArrayData, GetExternalArrayResponse, GetExternalArrayResponses, GetExternalMixedData, GetExternalMixedResponse, GetExternalMixedResponses, GetExternalModelData, GetExternalModelError, GetExternalModelErrors, GetExternalModelResponse, GetExternalModelResponses, GetExternalNestedData, GetExternalNestedResponse, GetExternalNestedResponses, GetExternalPropertiesByIdData, GetExternalPropertiesByIdResponse, GetExternalPropertiesByIdResponses, GetExternalUnionData, GetExternalUnionResponses, GetExternalUuidData, GetExternalUuidResponse, GetExternalUuidResponses, PostExternalArrayData, PostExternalArrayResponse, PostExternalArrayResponses, PostExternalMixedData, PostExternalMixedResponse, PostExternalMixedResponses, PostExternalModelData, PostExternalModelErrors, PostExternalModelResponse, PostExternalModelResponses, PostExternalNestedData, PostExternalNestedResponse, PostExternalNestedResponses, PostExternalUnionData, PostExternalUnionResponses, PutExternalUuidData, PutExternalUuidResponse, PutExternalUuidResponses } from './types.gen';
3+
export type { _1, ClientOptions, Deep, ExternalAllOfSchema, ExternalAnyOfSchema, ExternalArraySchema, ExternalDoubleNestedNumeric, ExternalDoubleNestedProp, ExternalMixedProperties, ExternalNested, ExternalNestedNumeric, ExternalNestedNumericObjectA, ExternalNestedNumericObjectB, ExternalNestedObjectA, ExternalNestedObjectB, ExternalSchemaA, ExternalSchemaB, ExternalSchemaC, ExternalSchemaExternalProp, ExternalSchemaExternalPropAlias, ExternalSchemaPathA, ExternalSchemaPathB, ExternalSchemaPropertyA, ExternalSchemaPropertyB, ExternalSchemaPropertyC, ExternalSchemaPropertyD, ExternalSharedModel, ExternalSharedModelWithUuid, ExternalUnionSchema, GetExternalArrayData, GetExternalArrayResponse, GetExternalArrayResponses, GetExternalMixedData, GetExternalMixedResponse, GetExternalMixedResponses, GetExternalModelData, GetExternalModelError, GetExternalModelErrors, GetExternalModelResponse, GetExternalModelResponses, GetExternalNestedData, GetExternalNestedResponse, GetExternalNestedResponses, GetExternalPropertiesByIdData, GetExternalPropertiesByIdResponse, GetExternalPropertiesByIdResponses, GetExternalUnionData, GetExternalUnionResponses, GetExternalUuidData, GetExternalUuidResponse, GetExternalUuidResponses, Id, Name, PostExternalArrayData, PostExternalArrayResponse, PostExternalArrayResponses, PostExternalMixedData, PostExternalMixedResponse, PostExternalMixedResponses, PostExternalModelData, PostExternalModelErrors, PostExternalModelResponse, PostExternalModelResponses, PostExternalNestedData, PostExternalNestedResponse, PostExternalNestedResponses, PostExternalUnionData, PostExternalUnionResponses, PutExternalUuidData, PutExternalUuidResponse, PutExternalUuidResponses } from './types.gen';

0 commit comments

Comments
 (0)