Skip to content

Commit 2aeb207

Browse files
plaflammeplaflamme
committed
fix: sttp4 oneOf with parent properties - resolve aliased children, propagate parent props
When a schema has both its own properties and a oneOf composition, the sttp4 generator previously had three issues: 1. Empty oneOf children (no type, no properties) were silently dropped because DefaultCodegen's unaliasSchema treated them as type aliases, replacing the model name with the language type (e.g. io.circe.Json). 2. Parent-owned properties (e.g. cancellation_datetime) were lost entirely - they didn't appear on the sealed trait or any child. 3. Sibling properties from other oneOf children could contaminate each child's vars list via DefaultCodegen's property merging. This fix: - Resolves aliased oneOf children by looking at the original OpenAPI schema's $refs to restore correct model names - Identifies parent-owned properties and propagates them to each child model's allVars so they appear in generated case classes - Renders parent properties as abstract def members on the sealed trait for compile-time enforcement - Adds a test covering the CancellationInfo pattern (parent with properties + oneOf with empty and non-empty children)
1 parent d7355ec commit 2aeb207

4 files changed

Lines changed: 241 additions & 0 deletions

File tree

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

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,38 @@ private void setParameterDefaults(CodegenParameter param) {
251251
public Map<String, ModelsMap> postProcessAllModels(Map<String, ModelsMap> objs) {
252252
final Map<String, ModelsMap> processed = super.postProcessAllModels(objs);
253253

254+
// Pre-pass: fix aliased oneOf children.
255+
// When a oneOf child schema has no type and no properties (e.g. just title + description),
256+
// DefaultCodegen's unaliasSchema treats it as a type alias and replaces the model name
257+
// in cModel.oneOf with the language type (e.g. "io.circe.Json" instead of "CancellationInfoHotel").
258+
// We fix this by looking at the original OpenAPI schema's oneOf $refs to get the real model names.
259+
Map<String, Schema> allSchemas = this.openAPI.getComponents() != null
260+
? this.openAPI.getComponents().getSchemas()
261+
: Collections.emptyMap();
262+
263+
for (ModelsMap mm : processed.values()) {
264+
for (ModelMap model : mm.getModels()) {
265+
CodegenModel cModel = model.getModel();
266+
if (!cModel.oneOf.isEmpty() && allSchemas != null) {
267+
Schema parentSchema = allSchemas.get(cModel.name);
268+
if (parentSchema != null && parentSchema.getOneOf() != null) {
269+
Set<String> resolvedOneOf = new LinkedHashSet<>();
270+
for (Object o : parentSchema.getOneOf()) {
271+
Schema childSchema = (Schema) o;
272+
if (childSchema.get$ref() != null) {
273+
String refName = toModelName(org.openapitools.codegen.utils.ModelUtils.getSimpleRef(childSchema.get$ref()));
274+
resolvedOneOf.add(refName);
275+
}
276+
}
277+
if (!resolvedOneOf.isEmpty()) {
278+
cModel.oneOf.clear();
279+
cModel.oneOf.addAll(resolvedOneOf);
280+
}
281+
}
282+
}
283+
}
284+
}
285+
254286
// First pass: count how many oneOf parents each model has
255287
Map<String, Integer> oneOfMemberCount = new HashMap<>();
256288
for (ModelsMap mm : processed.values()) {
@@ -272,6 +304,14 @@ public Map<String, ModelsMap> postProcessAllModels(Map<String, ModelsMap> objs)
272304
if (!cModel.oneOf.isEmpty()) {
273305
cModel.getVendorExtensions().put("x-isSealedTrait", true);
274306

307+
// Identify the parent schema's own properties (not inherited from oneOf children).
308+
// These need to be propagated to all children and rendered as abstract members on the sealed trait.
309+
List<CodegenProperty> parentOwnProps = getParentOwnProperties(cModel, allSchemas);
310+
if (!parentOwnProps.isEmpty()) {
311+
cModel.getVendorExtensions().put("x-parentProps", parentOwnProps);
312+
cModel.getVendorExtensions().put("x-hasParentProps", true);
313+
}
314+
275315
// Collect child models for inline generation
276316
// Only inline if they are used exclusively by this oneOf parent
277317
List<CodegenModel> childModels = new ArrayList<>();
@@ -283,6 +323,10 @@ public Map<String, ModelsMap> postProcessAllModels(Map<String, ModelsMap> objs)
283323
childModel.getVendorExtensions().put("x-isOneOfMember", true);
284324
childModel.getVendorExtensions().put("x-oneOfParent", cModel.classname);
285325

326+
// Propagate parent's own properties to child's allVars
327+
// so they appear in the generated case class fields
328+
propagateParentProperties(childModel, parentOwnProps);
329+
286330
// Add discriminator mapping value if present
287331
if (cModel.discriminator != null) {
288332
String discriminatorName = cModel.discriminator.getPropertyName();
@@ -352,6 +396,64 @@ public Map<String, ModelsMap> postProcessAllModels(Map<String, ModelsMap> objs)
352396
return processed;
353397
}
354398

399+
/**
400+
* Identifies properties that are defined directly on a oneOf parent schema
401+
* (not inherited from oneOf children). These are the parent's "own" properties
402+
* that need to be propagated to all children.
403+
*/
404+
private List<CodegenProperty> getParentOwnProperties(CodegenModel parentModel, Map<String, Schema> allSchemas) {
405+
List<CodegenProperty> parentOwnProps = new ArrayList<>();
406+
if (allSchemas == null) return parentOwnProps;
407+
408+
Schema parentSchema = allSchemas.get(parentModel.name);
409+
if (parentSchema == null || parentSchema.getProperties() == null) return parentOwnProps;
410+
411+
Set<String> parentPropNames = parentSchema.getProperties().keySet();
412+
Set<String> parentRequired = parentSchema.getRequired() != null
413+
? new HashSet<>(parentSchema.getRequired())
414+
: Collections.emptySet();
415+
416+
for (String propName : parentPropNames) {
417+
// Find matching CodegenProperty in the parent model's vars
418+
for (CodegenProperty cp : parentModel.vars) {
419+
if (cp.baseName.equals(propName)) {
420+
CodegenProperty cloned = cp.clone();
421+
// Ensure required flag reflects the parent schema's required list
422+
cloned.required = parentRequired.contains(propName);
423+
parentOwnProps.add(cloned);
424+
break;
425+
}
426+
}
427+
}
428+
return parentOwnProps;
429+
}
430+
431+
/**
432+
* Propagates parent-owned properties to a child model's allVars.
433+
* Parent properties are prepended so they appear first in the generated case class.
434+
* Properties already present on the child (by baseName) are not duplicated.
435+
*/
436+
private void propagateParentProperties(CodegenModel childModel, List<CodegenProperty> parentOwnProps) {
437+
if (parentOwnProps.isEmpty()) return;
438+
439+
Set<String> existingPropNames = new HashSet<>();
440+
for (CodegenProperty cp : childModel.allVars) {
441+
existingPropNames.add(cp.baseName);
442+
}
443+
444+
List<CodegenProperty> toAdd = new ArrayList<>();
445+
for (CodegenProperty parentProp : parentOwnProps) {
446+
if (!existingPropNames.contains(parentProp.baseName)) {
447+
toAdd.add(parentProp.clone());
448+
}
449+
}
450+
451+
if (!toAdd.isEmpty()) {
452+
toAdd.addAll(childModel.allVars);
453+
childModel.allVars = toAdd;
454+
}
455+
}
456+
355457
/**
356458
* Update/clean up model imports
357459
* <p>

‎modules/openapi-generator/src/main/resources/scala-sttp4/model.mustache‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,16 @@ import {{import}}
2121
{{/javadocRenderer}}
2222
{{/description}}
2323
{{#vendorExtensions.x-isSealedTrait}}
24+
{{#vendorExtensions.x-hasParentProps}}
25+
sealed trait {{classname}} {
26+
{{#vendorExtensions.x-parentProps}}
27+
def {{{name}}}: {{^required}}Option[{{/required}}{{dataType}}{{^required}}]{{/required}}
28+
{{/vendorExtensions.x-parentProps}}
29+
}
30+
{{/vendorExtensions.x-hasParentProps}}
31+
{{^vendorExtensions.x-hasParentProps}}
2432
sealed trait {{classname}}
33+
{{/vendorExtensions.x-hasParentProps}}
2534

2635
{{! Generate inline case classes for oneOf members }}
2736
{{#vendorExtensions.x-oneOfMembers}}

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

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.nio.file.Paths;
2020

2121
import static org.openapitools.codegen.TestUtils.assertFileContains;
22+
import static org.openapitools.codegen.TestUtils.assertFileNotContains;
2223

2324
public class Sttp4CodegenTest {
2425

@@ -209,6 +210,76 @@ public void verifyOneOfWithEmptyMembers() throws IOException {
209210
assertFileContains(eventPath, "case class ViewEvent(\n) extends Event");
210211
}
211212

213+
@Test
214+
public void verifyOneOfWithParentProperties() throws IOException {
215+
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();
216+
output.deleteOnExit();
217+
String outputPath = output.getAbsolutePath().replace('\\', '/');
218+
219+
OpenAPI openAPI = new OpenAPIParser()
220+
.readLocation("src/test/resources/3_0/scala/sttp4-oneOf-with-parent-props.yaml", null,
221+
new ParseOptions())
222+
.getOpenAPI();
223+
224+
ScalaSttp4ClientCodegen codegen = new ScalaSttp4ClientCodegen();
225+
codegen.setOutputDir(output.getAbsolutePath());
226+
codegen.additionalProperties().put("jsonLibrary", "circe");
227+
228+
ClientOptInput input = new ClientOptInput();
229+
input.openAPI(openAPI);
230+
input.config(codegen);
231+
232+
DefaultGenerator generator = new DefaultGenerator();
233+
234+
generator.setGeneratorPropertyDefault(CodegenConstants.MODELS, "true");
235+
generator.setGeneratorPropertyDefault(CodegenConstants.MODEL_TESTS, "false");
236+
generator.setGeneratorPropertyDefault(CodegenConstants.MODEL_DOCS, "false");
237+
generator.setGeneratorPropertyDefault(CodegenConstants.APIS, "false");
238+
generator.setGeneratorPropertyDefault(CodegenConstants.SUPPORTING_FILES, "false");
239+
generator.opts(input).generate();
240+
241+
Path parentPath = Paths.get(
242+
outputPath + "/src/main/scala/org/openapitools/client/model/Parent.scala");
243+
String content = Files.readString(parentPath);
244+
245+
// Sealed trait exists with abstract member for parent property
246+
assertFileContains(parentPath, "sealed trait Parent {");
247+
assertFileContains(parentPath, "def createdAt: OffsetDateTime");
248+
249+
// ChildOne exists (empty child is NOT dropped) and extends Parent
250+
assertFileContains(parentPath, "case class ChildOne(");
251+
assertFileContains(parentPath, ") extends Parent");
252+
253+
// ChildOne has parent property propagated
254+
int childOneStart = content.indexOf("case class ChildOne(");
255+
int childOneEnd = content.indexOf(") extends Parent", childOneStart);
256+
Assert.assertTrue(childOneStart >= 0 && childOneEnd >= 0,
257+
"ChildOne case class should exist");
258+
String childOneBlock = content.substring(childOneStart, childOneEnd);
259+
Assert.assertTrue(childOneBlock.contains("createdAt: OffsetDateTime"),
260+
"ChildOne should have parent property 'createdAt'");
261+
262+
// ChildOne does NOT have sibling properties
263+
Assert.assertFalse(childOneBlock.contains("status"),
264+
"ChildOne should NOT contain sibling property 'status'");
265+
Assert.assertFalse(childOneBlock.contains("detail"),
266+
"ChildOne should NOT contain sibling property 'detail'");
267+
268+
// ChildTwo has parent property + its own properties
269+
assertFileContains(parentPath, "case class ChildTwo(");
270+
int childTwoStart = content.indexOf("case class ChildTwo(");
271+
int childTwoEnd = content.indexOf(") extends Parent", childTwoStart);
272+
Assert.assertTrue(childTwoStart >= 0 && childTwoEnd >= 0,
273+
"ChildTwo case class should exist");
274+
String childTwoBlock = content.substring(childTwoStart, childTwoEnd);
275+
Assert.assertTrue(childTwoBlock.contains("createdAt: OffsetDateTime"),
276+
"ChildTwo should have parent property 'createdAt'");
277+
Assert.assertTrue(childTwoBlock.contains("status: String"),
278+
"ChildTwo should have its own property 'status'");
279+
Assert.assertTrue(childTwoBlock.contains("detail: Option[Nested]"),
280+
"ChildTwo should have its own property 'detail'");
281+
}
282+
212283
@Test
213284
public void verifyDateTimeLocalGeneratesLocalDateTime() throws IOException {
214285
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();

‎modules/openapi-generator/src/test/resources/3_0/scala/sttp4-oneOf-with-parent-props.yaml‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
openapi: 3.0.0
2+
info:
3+
title: OneOf with Parent Properties Test
4+
version: 1.0.0
5+
paths:
6+
/parents:
7+
get:
8+
operationId: getParent
9+
responses:
10+
'200':
11+
description: A parent object
12+
content:
13+
application/json:
14+
schema:
15+
$ref: '#/components/schemas/Parent'
16+
components:
17+
schemas:
18+
Nested:
19+
type: object
20+
properties:
21+
amount:
22+
type: number
23+
currency:
24+
type: string
25+
26+
Parent:
27+
title: Parent
28+
description: "A parent type with shared properties and oneOf children"
29+
required:
30+
- created_at
31+
properties:
32+
created_at:
33+
description: "The creation timestamp."
34+
type: string
35+
format: date-time
36+
oneOf:
37+
- $ref: '#/components/schemas/ChildOne'
38+
- $ref: '#/components/schemas/ChildTwo'
39+
40+
ChildOne:
41+
title: ChildOne
42+
description: "First child - intentionally has no properties"
43+
44+
ChildTwo:
45+
title: ChildTwo
46+
description: "Second child with its own properties"
47+
required:
48+
- status
49+
properties:
50+
status:
51+
type: string
52+
description: "The status of this child."
53+
enum:
54+
- active
55+
- inactive
56+
detail:
57+
allOf:
58+
- $ref: '#/components/schemas/Nested'
59+
description: "Nested detail object"

0 commit comments

Comments
 (0)