Skip to content

Commit f00d64f

Browse files
abrahamabraham
authored
Merge pull request #428 from abraham/copilot/fix-2bbfd475-3fd0-4df7-a548-a98ad4d0a92c
Fix entity attribute type parsing for multiple entity references and nullable handling
2 parents 5114ea2 + 441530f commit f00d64f

File tree

5 files changed

+248
-18
lines changed

5 files changed

+248
-18
lines changed

‎dist/schema.json‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37217,6 +37217,9 @@
3721737217
{
3721837218
"$ref": "#/components/schemas/Quote"
3721937219
},
37220+
{
37221+
"$ref": "#/components/schemas/ShallowQuote"
37222+
},
3722037223
{
3722137224
"type": "null"
3722237225
}

‎src/__tests__/integration/status-quote-oneOf.test.ts‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
import { OpenAPIGenerator } from '../../generators/OpenAPIGenerator';
2+
import { EntityParser } from '../../parsers/EntityParser';
3+
import { ApiMethodsFile } from '../../interfaces/ApiMethodsFile';
4+
5+
describe('Status Quote Attribute Integration Test', () => {
6+
it('should generate OpenAPI schema with oneOf for Status.quote attribute', () => {
7+
const generator = new OpenAPIGenerator();
8+
const entityParser = new EntityParser();
9+
10+
// Parse entities and generate the schema
11+
const entities = entityParser.parseAllEntities();
12+
const methodFiles: ApiMethodsFile[] = []; // Empty method files for this test
13+
const spec = generator.generateSchema(entities, methodFiles);
14+
15+
// Find the Status schema
16+
const statusSchema = spec.components?.schemas?.Status;
17+
expect(statusSchema).toBeDefined();
18+
19+
if (statusSchema && statusSchema.properties) {
20+
const quoteProperty = statusSchema.properties.quote;
21+
expect(quoteProperty).toBeDefined();
22+
23+
console.log(
24+
'Generated Status.quote property:',
25+
JSON.stringify(quoteProperty, null, 2)
26+
);
27+
28+
// Check that the quote property has a oneOf structure with both Quote and ShallowQuote plus null
29+
if (quoteProperty.oneOf) {
30+
// Check if it's directly oneOf with Quote and ShallowQuote plus null
31+
const refs = quoteProperty.oneOf
32+
.filter((item: any) => item.$ref)
33+
.map((item: any) => item.$ref);
34+
35+
const hasQuote = refs.includes('#/components/schemas/Quote');
36+
const hasShallowQuote = refs.includes(
37+
'#/components/schemas/ShallowQuote'
38+
);
39+
const hasNull = quoteProperty.oneOf.some(
40+
(item: any) => item.type === 'null'
41+
);
42+
43+
expect(hasQuote).toBe(true);
44+
expect(hasShallowQuote).toBe(true);
45+
expect(hasNull).toBe(true);
46+
47+
console.log(
48+
'✓ Found both Quote and ShallowQuote references plus null in oneOf'
49+
);
50+
} else {
51+
// If there's a nested structure due to nullable handling, check it
52+
console.log('Quote property structure:', quoteProperty);
53+
54+
// This might happen if nullable handling creates a different structure
55+
// Let's still verify both entities are referenced somewhere in the structure
56+
const jsonStr = JSON.stringify(quoteProperty);
57+
expect(jsonStr).toContain('#/components/schemas/Quote');
58+
expect(jsonStr).toContain('#/components/schemas/ShallowQuote');
59+
expect(jsonStr).toContain('"type":"null"');
60+
61+
console.log(
62+
'✓ Found both Quote and ShallowQuote references and null in the structure'
63+
);
64+
}
65+
}
66+
});
67+
68+
it('should verify that Quote and ShallowQuote schemas exist', () => {
69+
const generator = new OpenAPIGenerator();
70+
const entityParser = new EntityParser();
71+
72+
const entities = entityParser.parseAllEntities();
73+
const methodFiles: ApiMethodsFile[] = [];
74+
const spec = generator.generateSchema(entities, methodFiles);
75+
76+
// Verify that both Quote and ShallowQuote entities exist in the schema
77+
expect(spec.components?.schemas?.Quote).toBeDefined();
78+
expect(spec.components?.schemas?.ShallowQuote).toBeDefined();
79+
80+
console.log(
81+
'Quote schema keys:',
82+
spec.components?.schemas?.Quote
83+
? Object.keys(spec.components.schemas.Quote)
84+
: 'Not found'
85+
);
86+
console.log(
87+
'ShallowQuote schema keys:',
88+
spec.components?.schemas?.ShallowQuote
89+
? Object.keys(spec.components.schemas.ShallowQuote)
90+
: 'Not found'
91+
);
92+
});
93+
});

‎src/__tests__/parsers/AttributeParser.quote-types.test.ts‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
import { AttributeParser } from '../../parsers/AttributeParser';
2+
import { TypeParser } from '../../generators/TypeParser';
3+
import { UtilityHelpers } from '../../generators/UtilityHelpers';
4+
5+
describe('AttributeParser - Quote Types Issue', () => {
6+
let typeParser: TypeParser;
7+
8+
beforeEach(() => {
9+
const utilityHelpers = new UtilityHelpers();
10+
typeParser = new TypeParser(utilityHelpers);
11+
});
12+
13+
it('should parse quote attribute with multiple types (Quote, ShallowQuote, null)', () => {
14+
// Mock content representing the Status entity quote attribute
15+
// This reproduces the exact content from Status.md
16+
const content = `
17+
### \`quote\` {#quote}
18+
19+
**Description:** Information about the status being quoted, if any\\
20+
**Type:** {{<nullable>}} [Quote]({{< relref "entities/quote" >}}), [ShallowQuote]({{< relref "entities/ShallowQuote" >}}) or null\\
21+
**Version history:**\\
22+
4.4.0 - added
23+
`;
24+
25+
const attributes = AttributeParser.parseAttributesFromSection(
26+
content,
27+
'Status'
28+
);
29+
30+
expect(attributes).toHaveLength(1);
31+
const quoteAttribute = attributes[0];
32+
33+
expect(quoteAttribute.name).toBe('quote');
34+
expect(quoteAttribute.nullable).toBe(true);
35+
36+
// The key issue: the type should capture both Quote and ShallowQuote
37+
// Currently it likely only captures Quote
38+
console.log('Parsed quote attribute type:', quoteAttribute.type);
39+
console.log(
40+
'Full quote attribute:',
41+
JSON.stringify(quoteAttribute, null, 2)
42+
);
43+
44+
// This test will initially fail, showing us the current behavior
45+
// The type should ideally be something that includes both Quote and ShallowQuote
46+
expect(quoteAttribute.type).toContain('Quote');
47+
expect(quoteAttribute.type).toContain('ShallowQuote');
48+
});
49+
50+
it('should parse quote attribute from actual Status entity file', () => {
51+
// Parse actual Status entity to check the real behavior
52+
const { EntityParser } = require('../../parsers/EntityParser');
53+
const parser = new EntityParser();
54+
const entities = parser.parseAllEntities();
55+
const statusEntity = entities.find((e: any) => e.name === 'Status');
56+
57+
expect(statusEntity).toBeDefined();
58+
59+
if (statusEntity) {
60+
const quoteAttribute = statusEntity.attributes.find(
61+
(a: any) => a.name === 'quote'
62+
);
63+
64+
expect(quoteAttribute).toBeDefined();
65+
66+
if (quoteAttribute) {
67+
console.log('Real Status quote attribute type:', quoteAttribute.type);
68+
console.log(
69+
'Real Status quote attribute:',
70+
JSON.stringify(quoteAttribute, null, 2)
71+
);
72+
73+
// This should include both Quote and ShallowQuote types
74+
expect(quoteAttribute.type).toContain('Quote');
75+
// This will likely fail, showing the current issue
76+
expect(quoteAttribute.type).toContain('ShallowQuote');
77+
}
78+
}
79+
});
80+
81+
it('should parse TypeParser.parseType with multiple entity references', () => {
82+
// Test the TypeParser directly to verify our fix
83+
const typeString = '[Quote](), [ShallowQuote]() or null';
84+
const parsedType = typeParser.parseType(typeString);
85+
86+
console.log(
87+
'TypeParser result for multiple entities:',
88+
JSON.stringify(parsedType, null, 2)
89+
);
90+
91+
// After our fix, this should return a oneOf structure with both Quote and ShallowQuote
92+
expect(parsedType.oneOf).toBeDefined();
93+
expect(parsedType.oneOf).toHaveLength(2);
94+
95+
// Check that both entities are referenced
96+
const refs = parsedType.oneOf?.map((item: any) => item.$ref) || [];
97+
expect(refs).toContain('#/components/schemas/Quote');
98+
expect(refs).toContain('#/components/schemas/ShallowQuote');
99+
});
100+
101+
it('should handle single entity reference correctly', () => {
102+
// Test that single entity references still work as before
103+
const typeString = '[Quote]() or null';
104+
const parsedType = typeParser.parseType(typeString);
105+
106+
console.log(
107+
'TypeParser result for single entity:',
108+
JSON.stringify(parsedType, null, 2)
109+
);
110+
111+
// Should return a direct reference for single entity
112+
expect(parsedType.$ref).toBe('#/components/schemas/Quote');
113+
expect(parsedType.oneOf).toBeUndefined();
114+
});
115+
});

‎src/generators/EntityConverter.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,9 @@ class EntityConverter {
459459
oneOf: [{ $ref: property.$ref }, { type: 'null' }],
460460
...(property.deprecated && { deprecated: true }),
461461
};
462+
} else if (property.oneOf) {
463+
// For properties that already have oneOf (e.g., multiple entity references), add null to the oneOf array
464+
property.oneOf.push({ type: 'null' });
462465
} else if (property.type && typeof property.type === 'string') {
463466
// For regular type properties, convert type to an array that includes null
464467
property.type = [property.type, 'null'];

‎src/generators/TypeParser.ts‎

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -35,27 +35,43 @@ class TypeParser {
3535

3636
// Handle references to other entities (only for actual entity names, not documentation links)
3737
if (typeString.includes('[') && typeString.includes(']')) {
38-
const refMatch = typeString.match(/\[([^\]]+)\]/);
39-
if (refMatch) {
40-
const refName = refMatch[1];
41-
42-
// Only treat as entity reference if it's an actual entity name
43-
// Skip documentation references like "Datetime", "Date", etc.
44-
const isDocumentationLink =
45-
refName.toLowerCase().includes('/') ||
46-
refName.toLowerCase() === 'datetime' ||
47-
refName.toLowerCase() === 'date' ||
48-
refName.toLowerCase().includes('iso8601');
49-
50-
if (!isDocumentationLink) {
51-
// Clean up reference name and sanitize for OpenAPI compliance
52-
const cleanRefName = refName.replace(/[^\w:]/g, '');
53-
const sanitizedRefName =
54-
this.utilityHelpers.sanitizeSchemaName(cleanRefName);
38+
// Find all entity references using global flag to capture multiple entities
39+
const entityMatches = typeString.match(/\[([^\]]+)\]/g);
40+
if (entityMatches) {
41+
const validEntityRefs: OpenAPIProperty[] = [];
42+
43+
for (const match of entityMatches) {
44+
const refName = match.slice(1, -1); // Remove [ and ]
45+
46+
// Only treat as entity reference if it's an actual entity name
47+
// Skip documentation references like "Datetime", "Date", etc.
48+
const isDocumentationLink =
49+
refName.toLowerCase().includes('/') ||
50+
refName.toLowerCase() === 'datetime' ||
51+
refName.toLowerCase() === 'date' ||
52+
refName.toLowerCase().includes('iso8601');
53+
54+
if (!isDocumentationLink) {
55+
// Clean up reference name and sanitize for OpenAPI compliance
56+
const cleanRefName = refName.replace(/[^\w:]/g, '');
57+
const sanitizedRefName =
58+
this.utilityHelpers.sanitizeSchemaName(cleanRefName);
59+
validEntityRefs.push({
60+
$ref: `#/components/schemas/${sanitizedRefName}`,
61+
});
62+
}
63+
}
64+
65+
// If we found multiple valid entities, return oneOf
66+
if (validEntityRefs.length > 1) {
5567
return {
56-
$ref: `#/components/schemas/${sanitizedRefName}`,
68+
oneOf: validEntityRefs,
5769
};
5870
}
71+
// If we found exactly one valid entity, return it directly
72+
else if (validEntityRefs.length === 1) {
73+
return validEntityRefs[0];
74+
}
5975
}
6076
}
6177

0 commit comments

Comments
 (0)