Skip to content

Commit f8ddd3b

Browse files
thorstenhirschthorstenhirsch
committed
[Java][native] Fix oneOf wrapper inheritance and inherited readOnly @JsonCreator
1 parent 9432aaf commit f8ddd3b

6 files changed

Lines changed: 248 additions & 2 deletions

File tree

‎modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2794,7 +2794,13 @@ protected void updateModelForComposedSchema(CodegenModel m, Schema schema, Map<S
27942794
addImport(composed, refSchema, m, modelName);
27952795

27962796
if (allDefinitions != null && refSchema != null) {
2797-
if (allParents.contains(ref) && supportsMultipleInheritance) {
2797+
// Inline properties from oneOf wrapper schemas (cannot be used as parent classes)
2798+
if (ModelUtils.isOneOfWrapperSchema(refSchema)) {
2799+
Map<String, Schema> newProperties = new LinkedHashMap<>();
2800+
addProperties(newProperties, required, refSchema, new HashSet<>());
2801+
mergeProperties(properties, newProperties);
2802+
addProperties(allProperties, allRequired, refSchema, new HashSet<>());
2803+
} else if (allParents.contains(ref) && supportsMultipleInheritance) {
27982804
// multiple inheritance
27992805
addProperties(allProperties, allRequired, refSchema, new HashSet<>());
28002806
} else if (parentName != null && parentName.equals(ref) && supportsInheritance) {
@@ -3156,6 +3162,15 @@ public CodegenModel fromModel(String name, Schema schema) {
31563162
// remove duplicated properties
31573163
m.removeAllDuplicatedProperty();
31583164

3165+
// Mark inherited readonly properties for template to use setter instead of direct field access
3166+
if (m.parent != null && m.readOnlyVars != null) {
3167+
for (CodegenProperty roVar : m.readOnlyVars) {
3168+
if (!Boolean.TRUE.equals(roVar.isOverridden)) {
3169+
roVar.vendorExtensions.put("x-is-inherited-readonly", Boolean.TRUE);
3170+
}
3171+
}
3172+
}
3173+
31593174
// set isDiscriminator on the discriminator property
31603175
if (m.discriminator != null) {
31613176
String discPropName = m.discriminator.getPropertyBaseName();

‎modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1584,6 +1584,10 @@ public static String getParentName(Schema composedSchema, Map<String, Schema> al
15841584
if (s == null) {
15851585
LOGGER.error("Failed to obtain schema from {}", parentName);
15861586
parentNameCandidates.add("UNKNOWN_PARENT_NAME");
1587+
} else if (isOneOfWrapperSchema(s)) {
1588+
// Skip oneOf wrapper schemas (generate AbstractOpenApiSchema subclasses)
1589+
hasAmbiguousParents = true;
1590+
refedWithoutDiscriminator.add(parentName);
15871591
} else if (hasOrInheritsDiscriminator(s, allSchemas, new ArrayList<Schema>())) {
15881592
// discriminator.propertyName is used or x-parent is used
15891593
parentNameCandidates.add(parentName);
@@ -1650,6 +1654,8 @@ private static List<String> getAllParentsName(
16501654
if (s == null) {
16511655
LOGGER.error("Failed to obtain schema from {}", parentName);
16521656
names.add("UNKNOWN_PARENT_NAME");
1657+
} else if (isOneOfWrapperSchema(s)) {
1658+
// Skip oneOf wrapper schemas - properties will be inlined
16531659
} else if (hasOrInheritsDiscriminator(s, allSchemas, new ArrayList<Schema>())) {
16541660
// discriminator.propertyName is used or x-parent is used
16551661
names.add(parentName);
@@ -2122,6 +2128,24 @@ public static boolean hasOneOf(Schema schema) {
21222128
return false;
21232129
}
21242130

2131+
/**
2132+
* Returns true if the schema is a oneOf wrapper (has oneOf + properties/discriminator).
2133+
* Such schemas generate AbstractOpenApiSchema subclasses and cannot be used as parents.
2134+
*
2135+
* @param schema the schema
2136+
* @return true if the schema is a oneOf wrapper
2137+
*/
2138+
public static boolean isOneOfWrapperSchema(Schema schema) {
2139+
if (schema == null) {
2140+
return false;
2141+
}
2142+
boolean hasOneOf = schema.getOneOf() != null && !schema.getOneOf().isEmpty();
2143+
boolean hasDiscriminator = schema.getDiscriminator() != null &&
2144+
schema.getDiscriminator().getPropertyName() != null;
2145+
boolean hasProperties = schema.getProperties() != null && !schema.getProperties().isEmpty();
2146+
return hasOneOf && (hasDiscriminator || hasProperties);
2147+
}
2148+
21252149
/**
21262150
* Returns true if the schema contains anyOf but
21272151
* no properties/allOf/anyOf defined.

‎modules/openapi-generator/src/main/resources/Java/libraries/native/pojo.mustache‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ public class {{classname}} {{#parent}}extends {{{.}}} {{/parent}}{{#vendorExtens
9595
) {
9696
this();
9797
{{#readOnlyVars}}
98-
this.{{name}} = {{#vendorExtensions.x-is-jackson-optional-nullable}}{{name}} == null ? JsonNullable.<{{{datatypeWithEnum}}}>undefined() : JsonNullable.of({{name}}){{/vendorExtensions.x-is-jackson-optional-nullable}}{{^vendorExtensions.x-is-jackson-optional-nullable}}{{name}}{{/vendorExtensions.x-is-jackson-optional-nullable}};
98+
{{#vendorExtensions.x-is-inherited-readonly}}this.set{{nameInPascalCase}}({{#vendorExtensions.x-is-jackson-optional-nullable}}{{name}} == null ? JsonNullable.<{{{datatypeWithEnum}}}>undefined() : JsonNullable.of({{name}}){{/vendorExtensions.x-is-jackson-optional-nullable}}{{^vendorExtensions.x-is-jackson-optional-nullable}}{{name}}{{/vendorExtensions.x-is-jackson-optional-nullable}});{{/vendorExtensions.x-is-inherited-readonly}}{{^vendorExtensions.x-is-inherited-readonly}}this.{{name}} = {{#vendorExtensions.x-is-jackson-optional-nullable}}{{name}} == null ? JsonNullable.<{{{datatypeWithEnum}}}>undefined() : JsonNullable.of({{name}}){{/vendorExtensions.x-is-jackson-optional-nullable}}{{^vendorExtensions.x-is-jackson-optional-nullable}}{{name}}{{/vendorExtensions.x-is-jackson-optional-nullable}};{{/vendorExtensions.x-is-inherited-readonly}}
9999
{{/readOnlyVars}}
100100
}{{/withXml}}{{/vendorExtensions.x-has-readonly-properties}}
101101
{{#vars}}
@@ -254,6 +254,19 @@ public class {{classname}} {{#parent}}extends {{{.}}} {{/parent}}{{#vendorExtens
254254
{{/vendorExtensions.x-is-jackson-optional-nullable}}
255255
}
256256
{{/isReadOnly}}
257+
{{#isReadOnly}}
258+
/**
259+
* Protected setter for {{name}} (readOnly property, used by subclasses' @JsonCreator).
260+
*/
261+
protected void {{setter}}({{>nullable_var_annotations}} {{{datatypeWithEnum}}} {{name}}) {
262+
{{#vendorExtensions.x-is-jackson-optional-nullable}}
263+
this.{{name}} = {{name}} == null ? JsonNullable.<{{{datatypeWithEnum}}}>undefined() : JsonNullable.of({{name}});
264+
{{/vendorExtensions.x-is-jackson-optional-nullable}}
265+
{{^vendorExtensions.x-is-jackson-optional-nullable}}
266+
this.{{name}} = {{name}};
267+
{{/vendorExtensions.x-is-jackson-optional-nullable}}
268+
}
269+
{{/isReadOnly}}
257270

258271
{{/vars}}
259272
{{>libraries/native/additional_properties}}

‎modules/openapi-generator/src/test/java/org/openapitools/codegen/DefaultCodegenTest.java‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5074,4 +5074,80 @@ private List<String> getNames(List<CodegenProperty> props) {
50745074
if (props == null) return null;
50755075
return props.stream().map(v -> v.name).collect(Collectors.toList());
50765076
}
5077+
5078+
/**
5079+
* Test that oneOf wrapper schemas are not used as parent classes.
5080+
* Schemas with oneOf + properties generate AbstractOpenApiSchema subclasses
5081+
* which cannot be extended. Properties should be inlined instead.
5082+
*/
5083+
@Test
5084+
public void testOneOfWrapperSchemaShouldNotBeUsedAsParent() {
5085+
final DefaultCodegen codegen = new DefaultCodegen();
5086+
final OpenAPI openAPI = TestUtils.parseFlattenSpec("src/test/resources/3_0/issue-oneOf-wrapper-inheritance.yaml");
5087+
codegen.setOpenAPI(openAPI);
5088+
5089+
Schema clickEventSchema = openAPI.getComponents().getSchemas().get("ClickEvent");
5090+
CodegenModel clickEventModel = codegen.fromModel("ClickEvent", clickEventSchema);
5091+
5092+
assertNull(clickEventModel.parent, "ClickEvent should not have EventWrapper as parent");
5093+
5094+
Set<String> clickEventVarNames = clickEventModel.vars.stream()
5095+
.map(v -> v.name)
5096+
.collect(Collectors.toSet());
5097+
assertTrue(clickEventVarNames.contains("timestamp"), "ClickEvent should have 'timestamp' inlined");
5098+
assertTrue(clickEventVarNames.contains("userId"), "ClickEvent should have 'userId' inlined");
5099+
assertTrue(clickEventVarNames.contains("sessionId"), "ClickEvent should have 'sessionId' inlined");
5100+
assertTrue(clickEventVarNames.contains("elementId"), "ClickEvent should have 'elementId'");
5101+
assertTrue(clickEventVarNames.contains("clickX"), "ClickEvent should have 'clickX'");
5102+
assertTrue(clickEventVarNames.contains("clickY"), "ClickEvent should have 'clickY'");
5103+
}
5104+
5105+
@Test
5106+
public void testIsOneOfWrapperSchema() {
5107+
final OpenAPI openAPI = TestUtils.parseFlattenSpec("src/test/resources/3_0/issue-oneOf-wrapper-inheritance.yaml");
5108+
5109+
Schema eventWrapperSchema = openAPI.getComponents().getSchemas().get("EventWrapper");
5110+
assertTrue(ModelUtils.isOneOfWrapperSchema(eventWrapperSchema),
5111+
"EventWrapper should be detected as oneOf wrapper");
5112+
5113+
Schema clickEventSchema = openAPI.getComponents().getSchemas().get("ClickEvent");
5114+
assertFalse(ModelUtils.isOneOfWrapperSchema(clickEventSchema),
5115+
"ClickEvent should not be detected as oneOf wrapper");
5116+
5117+
Schema scrollEventSchema = openAPI.getComponents().getSchemas().get("ScrollEvent");
5118+
assertFalse(ModelUtils.isOneOfWrapperSchema(scrollEventSchema),
5119+
"ScrollEvent should not be detected as oneOf wrapper");
5120+
}
5121+
5122+
/**
5123+
* Test that inherited readOnly properties use protected setters in @JsonCreator.
5124+
* Child classes must use setters for inherited readOnly properties since they're private in parent.
5125+
*/
5126+
@Test
5127+
public void testInheritedReadOnlyPropertiesHaveProtectedSetters() {
5128+
final DefaultCodegen codegen = new DefaultCodegen();
5129+
final OpenAPI openAPI = TestUtils.parseFlattenSpec("src/test/resources/3_0/issue-inherited-readonly-jsoncreator.yaml");
5130+
codegen.setOpenAPI(openAPI);
5131+
5132+
Schema baseModelSchema = openAPI.getComponents().getSchemas().get("BaseModel");
5133+
CodegenModel baseModel = codegen.fromModel("BaseModel", baseModelSchema);
5134+
5135+
assertTrue(baseModel.readOnlyVars != null && !baseModel.readOnlyVars.isEmpty(),
5136+
"BaseModel should have readOnly properties");
5137+
5138+
Schema extendedModelSchema = openAPI.getComponents().getSchemas().get("ExtendedModel");
5139+
CodegenModel extendedModel = codegen.fromModel("ExtendedModel", extendedModelSchema);
5140+
5141+
assertEquals("BaseModel", extendedModel.parent, "ExtendedModel should extend BaseModel");
5142+
5143+
boolean foundInheritedReadOnly = false;
5144+
for (CodegenProperty roVar : extendedModel.readOnlyVars) {
5145+
if (!Boolean.TRUE.equals(roVar.isOverridden)) {
5146+
foundInheritedReadOnly = true;
5147+
assertTrue(Boolean.TRUE.equals(roVar.vendorExtensions.get("x-is-inherited-readonly")),
5148+
"Inherited readOnly '" + roVar.name + "' should have x-is-inherited-readonly");
5149+
}
5150+
}
5151+
assertTrue(foundInheritedReadOnly, "ExtendedModel should have inherited readOnly properties");
5152+
}
50775153
}

‎modules/openapi-generator/src/test/resources/3_0/issue-inherited-readonly-jsoncreator.yaml‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
openapi: 3.0.3
2+
info:
3+
title: Test API for inherited readonly properties @JsonCreator fix
4+
version: 1.0.0
5+
description: |
6+
This spec demonstrates the readOnly fix works when there's proper inheritance.
7+
8+
IMPORTANT NOTE about discriminator:
9+
- The discriminator IS required by OpenAPI Generator to establish the parent-child relationship
10+
- Without a discriminator (or x-parent extension), allOf refs are not recognized as inheritance
11+
- The readOnly fix itself works on any inherited readOnly properties, but inheritance
12+
must first be established via discriminator or x-parent
13+
14+
The readOnly fix ensures that when a child class inherits readOnly properties from a parent,
15+
the child's @JsonCreator constructor uses protected setters (not direct field access)
16+
for the inherited properties, since they are private in the parent.
17+
paths: {}
18+
components:
19+
schemas:
20+
# Parent with readOnly properties - discriminator REQUIRED for inheritance recognition
21+
# The discriminator is NOT specifically for the readOnly fix, but for inheritance itself
22+
BaseModel:
23+
type: object
24+
discriminator:
25+
propertyName: modelType
26+
properties:
27+
id:
28+
type: string
29+
format: uuid
30+
readOnly: true
31+
description: Read-only identifier inherited from parent
32+
createdAt:
33+
type: string
34+
format: date-time
35+
readOnly: true
36+
description: Read-only creation timestamp inherited from parent
37+
modelType:
38+
type: string
39+
description: Discriminator property for inheritance
40+
name:
41+
type: string
42+
description: Regular read-write property
43+
required:
44+
- id
45+
- modelType
46+
- name
47+
48+
# Child extends parent - @JsonCreator should use protected setters for inherited readOnly properties
49+
ExtendedModel:
50+
allOf:
51+
- $ref: '#/components/schemas/BaseModel'
52+
- type: object
53+
properties:
54+
description:
55+
type: string
56+
status:
57+
type: string
58+
enum: [active, inactive]
59+
required:
60+
- status

‎modules/openapi-generator/src/test/resources/3_0/issue-oneOf-wrapper-inheritance.yaml‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
openapi: 3.0.3
2+
info:
3+
title: Test API for oneOf wrapper schema inheritance fix (WITHOUT discriminator)
4+
version: 1.0.0
5+
description: |
6+
This spec demonstrates the oneOf wrapper fix works without requiring a discriminator.
7+
A oneOf wrapper schema (oneOf + properties, without discriminator) cannot be used as
8+
a parent class. Instead, its properties should be inlined into child schemas.
9+
paths: {}
10+
components:
11+
schemas:
12+
# This schema has oneOf AND properties but NO discriminator - it's still a "oneOf wrapper"
13+
# It should NOT be used as a parent class for inheritance
14+
EventWrapper:
15+
oneOf:
16+
- $ref: '#/components/schemas/ClickEvent'
17+
- $ref: '#/components/schemas/ScrollEvent'
18+
properties:
19+
timestamp:
20+
type: string
21+
format: date-time
22+
readOnly: true
23+
userId:
24+
type: string
25+
sessionId:
26+
type: string
27+
required:
28+
- timestamp
29+
- userId
30+
31+
# This schema uses allOf to reference the oneOf wrapper
32+
# Properties from EventWrapper should be inlined, not inherited
33+
ClickEvent:
34+
allOf:
35+
- $ref: '#/components/schemas/EventWrapper'
36+
- type: object
37+
properties:
38+
elementId:
39+
type: string
40+
clickX:
41+
type: integer
42+
clickY:
43+
type: integer
44+
required:
45+
- elementId
46+
47+
ScrollEvent:
48+
allOf:
49+
- $ref: '#/components/schemas/EventWrapper'
50+
- type: object
51+
properties:
52+
scrollDepth:
53+
type: integer
54+
direction:
55+
type: string
56+
enum: [up, down, left, right]
57+
required:
58+
- scrollDepth

0 commit comments

Comments
 (0)