Skip to content

fix(core): mark form parameters required from the schema and its allOf chain #25627

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
khayashi4337 wants to merge 2 commits into
base: master
Choose a base branch
from
+534 −6
Open

fix(core): mark form parameters required from the schema and its allOf chain #25627

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fa31da4
fix(core): mark form parameters required from the schema and its allO…
khayashi4337 Nov 11, 2026
e82af8a
fix(core): honor required listed next to $ref in form schemas
khayashi4337 Nov 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 84 additions & 6 deletions modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4005,6 +4005,78 @@ protected void updatePropertyForObject(CodegenProperty property, Schema p) {
addVarsRequiredVarsAdditionalProps(p, property);
}

/**
* Collects {@code required} entries only along the schema's own list and
* its allOf members (a single-allOf wrapper included), resolving $ref.
* Unlike {@link #addProperties}, oneOf/anyOf branches are not traversed
* unconditionally: a branch's required entries only hold if that branch
* is chosen, so only the entries shared by *every* branch of a oneOf/anyOf
* found along the way are real requirements of the enclosing shape.
*/
private void collectRequiredFromAllOfChain(Schema schema, List<String> required, Set<Schema> visitedSchemas) {
required.addAll(guaranteedRequired(schema, visitedSchemas));
}

/**
* Returns the {@code required} entries guaranteed by the schema itself:
* its own list, its allOf members' guarantees and, for oneOf/anyOf, the
* entries every branch requires (a field required by all alternatives is
* required no matter which one the caller picks). {@code visitedSchemas}
* keeps cyclic $ref chains from recursing forever; a schema already
* consumed contributes nothing the second time.
*/
private Set<String> guaranteedRequired(Schema schema, Set<Schema> visitedSchemas) {
Set<String> required = new LinkedHashSet<>();
if (schema == null || !visitedSchemas.add(schema)) {
return required;
}
// collect the schema's own `required` before following $ref: in OAS

@cubic-dev-ai cubic-dev-ai Bot Nov 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The $ref early return still runs before the allOf/oneOf/anyOf handling. A $ref with sibling allOf/oneOf/anyOf (valid in OAS 3.1) therefore drops those members, even though sibling required is now honored. Consider walking the composition keywords before returning from the $ref branch, or confirm the parser never produces such siblings.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java, line 4033:

<comment>The `$ref` early return still runs before the allOf/oneOf/anyOf handling. A `$ref` with sibling `allOf`/`oneOf`/`anyOf` (valid in OAS 3.1) therefore drops those members, even though sibling `required` is now honored. Consider walking the composition keywords before returning from the `$ref` branch, or confirm the parser never produces such siblings.</comment>

<file context>
@@ -4030,13 +4030,16 @@ private Set<String> guaranteedRequired(Schema schema, Set<Schema> visitedSchemas
         if (schema == null || !visitedSchemas.add(schema)) {
             return required;
         }
+        // collect the schema's own `required` before following $ref: in OAS
+        // 3.1 a `required` list may sit next to $ref and would otherwise be
+        // dropped when only the referenced schema is visited
</file context>

// 3.1 a `required` list may sit next to $ref and would otherwise be
// dropped when only the referenced schema is visited
if (schema.getRequired() != null) {
required.addAll(schema.getRequired());
}
if (StringUtils.isNotBlank(schema.get$ref())) {
required.addAll(guaranteedRequired(ModelUtils.getReferencedSchema(this.openAPI, schema), visitedSchemas));
return required;
}
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
Resolved
if (schema.getAllOf() != null) {
for (Object component : schema.getAllOf()) {
required.addAll(guaranteedRequired((Schema) component, visitedSchemas));
}
}
required.addAll(commonBranchRequired(schema.getOneOf(), visitedSchemas));
required.addAll(commonBranchRequired(schema.getAnyOf(), visitedSchemas));
return required;
}

/**
* Returns the {@code required} entries shared by every branch of a
* oneOf/anyOf list: each branch (a $ref resolved, its own allOf chain
* included) contributes its guaranteed entries and only the intersection
* holds for the enclosing schema.
*/
private Set<String> commonBranchRequired(List<Schema> branches, Set<Schema> visitedSchemas) {
Set<String> common = null;
if (branches != null) {
for (Schema branch : branches) {
// each branch gets its own copy of the visited set: a schema
// consumed while walking one branch must still contribute its
// required entries when reached again through a sibling branch
Set<String> branchRequired = guaranteedRequired(branch, new HashSet<>(visitedSchemas));
if (common == null) {
common = new LinkedHashSet<>(branchRequired);
} else {
common.retainAll(branchRequired);
}
if (common.isEmpty()) {
break;
}
}
}
return common == null ? Collections.emptySet() : common;
}

protected void updatePropertyForAnyType(CodegenProperty property, Schema p) {
property.isNullable = property.isNullable ||
!(ModelUtils.isComposedSchema(p)) ||
Expand Down Expand Up @@ -7846,7 +7918,14 @@ public List<CodegenParameter> fromRequestBodyToFormParameters(RequestBody body,
// TODO in the future have this return one codegenParameter of type object or composed which includes all definition
// that will be needed for complex composition use cases
// https://github.com/OpenAPITools/openapi-generator/issues/10415
addProperties(properties, allRequired, schema, new HashSet<>());
addProperties(properties, new ArrayList<>(), schema, new HashSet<>());
// addProperties unions the required list of every composed member,
// including oneOf/anyOf alternatives whose required entries only hold
// when that branch is chosen. For form parameters only the schema's own
// required list and the ones along the allOf chain are real
// requirements, so collect them separately. Starting from the original
// (pre-unwrap) schema also picks up a single-allOf wrapper's required.
collectRequiredFromAllOfChain(original != null ? original : schema, allRequired, new HashSet<>());

boolean isOneOfOrAnyOf = ModelUtils.isOneOf(schema) || ModelUtils.isAnyOf(schema);

Expand All @@ -7863,12 +7942,11 @@ public List<CodegenParameter> fromRequestBodyToFormParameters(RequestBody body,
// for oneOf/anyOf, mark all the properties collected from the sub-schemas as optional
// so that users can choose which property to include in the form parameters
codegenParameter.required = false;
} else if (!codegenParameter.required && schema.getRequired() != null) {
// Set 'required' flag defined in the schema element
codegenParameter.required = schema.getRequired().contains(entry.getKey());
} else if (!codegenParameter.required) {
// Set 'required' flag for properties declared inside the allOf
codegenParameter.required = allRequired.stream().anyMatch(r -> r.equals(codegenParameter.paramName));
// 'required' applies to the schema property name (baseName); comparing
// against the normalized paramName would silently drop the flag for
// e.g. snake_case names or collision-renamed parameters.
codegenParameter.required = allRequired.contains(entry.getKey());
}

parameters.add(codegenParameter);
Expand Down
181 changes: 181 additions & 0 deletions modules/openapi-generator/src/test/java/org/openapitools/codegen/DefaultCodegenTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,187 @@ public void testOAS31ContentMediaTypeBinaryFormParameter() {
assertFalse(image.isFile);
}

@Test
public void testAllOfFormRequiredMatchesByBaseName() {
// required matching must use the schema property name (baseName), not the
// normalized paramName: `user_id` -> `userId` used to silently drop the flag
final OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/form-allof-required.yaml");

final DefaultCodegen codegen = new DefaultCodegen();
codegen.setOpenAPI(openAPI);

RequestBody requestBody = openAPI.getPaths().get("/register").getPost().getRequestBody();
List<CodegenParameter> formParams = codegen.fromRequestBodyToFormParameters(requestBody, new HashSet<>());
Map<String, CodegenParameter> paramsByBaseName = formParams.stream()
.collect(Collectors.toMap(param -> param.baseName, param -> param));

assertTrue(paramsByBaseName.get("query").required,
"allOf member required must apply");
assertTrue(paramsByBaseName.get("user_id").required,
"required must match the schema name even when paramName is normalized to userId");
assertTrue(paramsByBaseName.get("prop-with-dash").required,
"required must match the schema name even when paramName is normalized to propWithDash");
assertFalse(paramsByBaseName.get("nickname").required,
"non-required allOf member stays optional");
}

@Test
public void testTopLevelRequiredDoesNotMaskAllOfMemberRequired() {
// sibling bug: a top-level `required` used to bypass the allOf-member
// required lists entirely
final OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/form-allof-required.yaml");

final DefaultCodegen codegen = new DefaultCodegen();
codegen.setOpenAPI(openAPI);

RequestBody requestBody = openAPI.getPaths().get("/register-top").getPost().getRequestBody();
List<CodegenParameter> formParams = codegen.fromRequestBodyToFormParameters(requestBody, new HashSet<>());
Map<String, CodegenParameter> paramsByBaseName = formParams.stream()
.collect(Collectors.toMap(param -> param.baseName, param -> param));

assertTrue(paramsByBaseName.get("top").required,
"top-level required must apply");
assertTrue(paramsByBaseName.get("member_req").required,
"allOf member required must apply alongside top-level required");
assertFalse(paramsByBaseName.get("member_opt").required,
"non-required member stays optional");
}

@Test
public void testOneOfBranchesDoNotForceFormRequired() {
// a schema carrying both `properties`/`required` and oneOf branches is not
// caught by ModelUtils.isOneOf (which requires empty properties); without
// care the allOf-style required union would also force branch-only fields
final OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/form-allof-required.yaml");

final DefaultCodegen codegen = new DefaultCodegen();
codegen.setOpenAPI(openAPI);

RequestBody requestBody = openAPI.getPaths().get("/register-mixed").getPost().getRequestBody();
List<CodegenParameter> formParams = codegen.fromRequestBodyToFormParameters(requestBody, new HashSet<>());
Map<String, CodegenParameter> paramsByBaseName = formParams.stream()
.collect(Collectors.toMap(param -> param.baseName, param -> param));

assertTrue(paramsByBaseName.get("common").required,
"top-level required must apply");
assertFalse(paramsByBaseName.get("a").required,
"oneOf branch required must not force the form parameter");
assertFalse(paramsByBaseName.get("b").required,
"oneOf branch required must not force the form parameter");
}

@Test
public void testSingleAllOfWrapperOwnRequired() {
// the single-allOf unwrapping keeps the wrapper schema in `original`; its
// own `required` list must still apply to member properties
final OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/form-allof-required.yaml");

final DefaultCodegen codegen = new DefaultCodegen();
codegen.setOpenAPI(openAPI);

RequestBody requestBody = openAPI.getPaths().get("/register-wrapper").getPost().getRequestBody();
List<CodegenParameter> formParams = codegen.fromRequestBodyToFormParameters(requestBody, new HashSet<>());
Map<String, CodegenParameter> paramsByBaseName = formParams.stream()
.collect(Collectors.toMap(param -> param.baseName, param -> param));

assertTrue(paramsByBaseName.get("wrapper_req").required,
"allOf wrapper's own required must apply");
assertFalse(paramsByBaseName.get("wrapper_opt").required,
"non-required member stays optional");
}

@Test
public void testOneOfMemberInsideAllOfDoesNotForceBranchRequired() {
// a oneOf inside the allOf chain must not union branch requireds into
// the form: `a`/`b` are only mandatory when that branch is chosen.
// master's addProperties union forced both, so this fails on master
final OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/form-allof-required.yaml");

final DefaultCodegen codegen = new DefaultCodegen();
codegen.setOpenAPI(openAPI);

RequestBody requestBody = openAPI.getPaths().get("/register-intersect").getPost().getRequestBody();
List<CodegenParameter> formParams = codegen.fromRequestBodyToFormParameters(requestBody, new HashSet<>());
Map<String, CodegenParameter> paramsByBaseName = formParams.stream()
.collect(Collectors.toMap(param -> param.baseName, param -> param));

assertTrue(paramsByBaseName.get("base_req").required,
"allOf member required must apply");
assertFalse(paramsByBaseName.get("a").required,
"oneOf branch-only required must not force the form parameter");
assertFalse(paramsByBaseName.get("b").required,
"oneOf branch-only required must not force the form parameter");
}

@Test
public void testRequiredSharedByAllOneOfBranchesInsideAllOfApplies() {
// `x` is required by every branch of the oneOf inside the allOf chain,
// so it is required no matter which branch the caller picks — dropping
// it would be an under-required regression versus master's union
final OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/form-allof-required.yaml");

final DefaultCodegen codegen = new DefaultCodegen();
codegen.setOpenAPI(openAPI);

RequestBody requestBody = openAPI.getPaths().get("/register-allbranches").getPost().getRequestBody();
List<CodegenParameter> formParams = codegen.fromRequestBodyToFormParameters(requestBody, new HashSet<>());
Map<String, CodegenParameter> paramsByBaseName = formParams.stream()
.collect(Collectors.toMap(param -> param.baseName, param -> param));

assertTrue(paramsByBaseName.get("x").required,
"required shared by every oneOf branch must apply (incl. via $ref)");
assertFalse(paramsByBaseName.get("only_a").required,
"branch-only required stays optional");
assertFalse(paramsByBaseName.get("only_b").required,
"branch-only required stays optional");
}

@Test
public void testRequiredSharedByAllAnyOfBranchesInsideAllOfApplies() {
// same intersection rule for anyOf members of the allOf chain
final OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/form-allof-required.yaml");

final DefaultCodegen codegen = new DefaultCodegen();
codegen.setOpenAPI(openAPI);

RequestBody requestBody = openAPI.getPaths().get("/register-anyof-common").getPost().getRequestBody();
List<CodegenParameter> formParams = codegen.fromRequestBodyToFormParameters(requestBody, new HashSet<>());
Map<String, CodegenParameter> paramsByBaseName = formParams.stream()
.collect(Collectors.toMap(param -> param.baseName, param -> param));

assertTrue(paramsByBaseName.get("x2").required,
"required shared by every anyOf branch must apply");
assertFalse(paramsByBaseName.get("aa").required,
"branch-only required stays optional");
assertFalse(paramsByBaseName.get("bb").required,
"branch-only required stays optional");
}

@Test
public void testRequiredSiblingOfRefApplies() {
// OAS 3.1 allows a `required` list next to $ref (JSON Schema 2020-12).
// guaranteedRequired used to return the referenced schema's entries
// only, silently dropping the sibling list
final OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_1/form-ref-sibling-required.yaml");

final DefaultCodegen codegen = new DefaultCodegen();
codegen.setOpenAPI(openAPI);

for (String path : new String[]{"/register-ref-sibling", "/register-inline-sibling"}) {
RequestBody requestBody = openAPI.getPaths().get(path).getPost().getRequestBody();
List<CodegenParameter> formParams = codegen.fromRequestBodyToFormParameters(requestBody, new HashSet<>());
Map<String, CodegenParameter> paramsByBaseName = formParams.stream()
.collect(Collectors.toMap(param -> param.baseName, param -> param));

assertTrue(paramsByBaseName.get("base_req").required,
path + ": required of the referenced schema must apply");
assertTrue(paramsByBaseName.get("extra").required,
path + ": required listed next to $ref must apply");
assertFalse(paramsByBaseName.get("optional_field").required,
path + ": non-required property stays optional");
}
}

@Test
public void testOriginalOpenApiDocumentVersion() {
// Test with OAS 2.0 document.
Expand Down
Loading
Loading