Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -2924,7 +2924,7 @@ protected void updateModelForComposedSchema(CodegenModel m, Schema schema, Map<S
addAdditionPropertiesToCodeGenModel(m, schema);
}

if (Boolean.TRUE.equals(schema.getNullable())) {
if (ModelUtils.isNullable(schema)) {

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: Removing the getTypes().contains("null") check from ModelUtils.isNullable silently changes behavior for OAS 3.1 type: [X, "null"] unions that have not gone through normalization: they are no longer detected as nullable (only the oneOf composed path still is). This also makes the implementation inconsistent with the method's own Javadoc, which still describes the null type as the 3.1 way to mark a schema nullable. The added ModelUtils.isNullable(...) usages in DefaultCodegen now depend on this behavior, so callers operating on raw/un-normalized 3.1 schemas may no longer mark such properties/parameters as nullable. Consider updating the Javadoc and confirming that every relevant path is normalized before relying on the reduced detection.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java, line 2927:

<comment>Removing the `getTypes().contains("null")` check from `ModelUtils.isNullable` silently changes behavior for OAS 3.1 `type: [X, "null"]` unions that have not gone through normalization: they are no longer detected as nullable (only the oneOf composed path still is). This also makes the implementation inconsistent with the method's own Javadoc, which still describes the `null` type as the 3.1 way to mark a schema nullable. The added `ModelUtils.isNullable(...)` usages in DefaultCodegen now depend on this behavior, so callers operating on raw/un-normalized 3.1 schemas may no longer mark such properties/parameters as nullable. Consider updating the Javadoc and confirming that every relevant path is normalized before relying on the reduced detection.</comment>

<file context>
@@ -2924,7 +2924,7 @@ protected void updateModelForComposedSchema(CodegenModel m, Schema schema, Map<S
         }
 
-        if (Boolean.TRUE.equals(schema.getNullable())) {
+        if (ModelUtils.isNullable(schema)) {
             m.isNullable = Boolean.TRUE;
         }
</file context>

m.isNullable = Boolean.TRUE;
}

Expand Down Expand Up @@ -3926,8 +3926,6 @@ public CodegenProperty fromProperty(String name, Schema p, boolean required, boo
}
if (ModelUtils.isNullable(p)) {
property.isNullable = true;
} else if (p.getNullable() != null) {
property.isNullable = p.getNullable();
}

if (p.getExtensions() != null && !p.getExtensions().isEmpty()) {
Expand Down Expand Up @@ -3977,14 +3975,8 @@ public CodegenProperty fromProperty(String name, Schema p, boolean required, boo
}
}

// set isNullable using nullable or x-nullable in the schema
if (ModelUtils.isNullable(referencedSchema)) {
property.isNullable = true;
} else if (referencedSchema.getNullable() != null) {
property.isNullable = referencedSchema.getNullable();
} else if (referencedSchema.getExtensions() != null &&
referencedSchema.getExtensions().containsKey(X_NULLABLE)) {
property.isNullable = (Boolean) referencedSchema.getExtensions().get(X_NULLABLE);
}

final XML referencedSchemaXml = referencedSchema.getXml();
Expand Down Expand Up @@ -4085,10 +4077,6 @@ public CodegenProperty fromProperty(String name, Schema p, boolean required, boo
// evaluate common attributes if defined in the top level
if (ModelUtils.isNullable(p)) {
property.isNullable = true;
} else if (p.getNullable() != null) {
property.isNullable = p.getNullable();
} else if (p.getExtensions() != null && p.getExtensions().containsKey(X_NULLABLE)) {
property.isNullable = (Boolean) p.getExtensions().get(X_NULLABLE);
}

if (p.getReadOnly() != null) {
Expand Down Expand Up @@ -5292,7 +5280,7 @@ public CodegenParameter fromParameter(Parameter parameter, Set<String> imports)
codegenParameter.setTypeProperties(parameterSchema, openAPI);
codegenParameter.setComposedSchemas(getComposedSchemas(parameterSchema));

if (Boolean.TRUE.equals(parameterSchema.getNullable())) { // use nullable defined in the spec
if (ModelUtils.isNullable(parameterSchema)) { // use nullable defined in the spec
codegenParameter.isNullable = true;
}

Expand Down Expand Up @@ -8066,7 +8054,9 @@ public CodegenParameter fromRequestBody(RequestBody body, Set<String> imports, S
if (original.getNullable() != null) {
codegenParameter.isNullable = original.getNullable();
} else if (original.getExtensions() != null && original.getExtensions().containsKey(X_NULLABLE)) {
codegenParameter.isNullable = (Boolean) original.getExtensions().get(X_NULLABLE);
codegenParameter.isNullable = Boolean.parseBoolean(String.valueOf(original.getExtensions().get(X_NULLABLE)));
} else if (ModelUtils.isNullable(original)) {
codegenParameter.isNullable = true;
}

if (original.getExtensions() != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -998,8 +998,8 @@ public Schema normalizeSchema(Schema schema, Set<Schema> visitedSchemas) {
}
normalizeProperties(schema, visitedSchemas);
} else if (schema.getAdditionalProperties() instanceof Schema) { // map
normalizeMapSchema(schema);
Schema additionalProperties = (Schema) schema.getAdditionalProperties();
Schema result = normalizeMapSchema(schema);

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: A type-less OAS 3.1 map schema (declared only via additionalProperties: with no type: object) can lose its value schema during normalization. The map branch now routes through normalizeMapSchema -> processNormalize31Spec, which replaces a type-less JsonSchema with an empty Schema, dropping additionalProperties and producing a null value schema at result.getAdditionalProperties(). The very same hazard is explicitly guarded against a few lines up for the object-with-properties case (the comment notes processNormalize31Spec 'can replace a JsonSchema with properties (but no explicit type) with an empty schema, discarding all properties'), so the map case should receive the same protection.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/java/org/openapitools/codegen/OpenAPINormalizer.java, line 1001:

<comment>A type-less OAS 3.1 map schema (declared only via `additionalProperties:` with no `type: object`) can lose its value schema during normalization. The map branch now routes through `normalizeMapSchema` -> `processNormalize31Spec`, which replaces a type-less `JsonSchema` with an empty `Schema`, dropping `additionalProperties` and producing a null value schema at `result.getAdditionalProperties()`. The very same hazard is explicitly guarded against a few lines up for the object-with-properties case (the comment notes processNormalize31Spec 'can replace a JsonSchema with properties (but no explicit type) with an empty schema, discarding all properties'), so the map case should receive the same protection.</comment>

<file context>
@@ -998,8 +998,8 @@ public Schema normalizeSchema(Schema schema, Set<Schema> visitedSchemas) {
         } else if (schema.getAdditionalProperties() instanceof Schema) { // map
-            normalizeMapSchema(schema);
-            Schema additionalProperties = (Schema) schema.getAdditionalProperties();
+            Schema result = normalizeMapSchema(schema);
+            Schema additionalProperties = (Schema) result.getAdditionalProperties();
             if (getRule(NORMALIZE_31SPEC) && ModelUtils.isNullTypeSchema(openAPI, additionalProperties)) {
</file context>

Schema additionalProperties = (Schema) result.getAdditionalProperties();
if (getRule(NORMALIZE_31SPEC) && ModelUtils.isNullTypeSchema(openAPI, additionalProperties)) {
// OAS 3.1 allows a map value schema of `type: "null"` (e.g.
// `additionalProperties: { type: "null" }`). There's no OAS 3.0 equivalent type,
Expand All @@ -1008,15 +1008,17 @@ public Schema normalizeSchema(Schema schema, Set<Schema> visitedSchemas) {
// generated as a normal (nullable) object instead.
Schema anyTypeNullable = new Schema();
anyTypeNullable.setNullable(true);
schema.setAdditionalProperties(anyTypeNullable);
result.setAdditionalProperties(anyTypeNullable);
} else {
Schema normalized = normalizeSchema(additionalProperties, visitedSchemas);
if (getRule(NORMALIZE_31SPEC)) {
// capture the normalized value schema (e.g. an OAS 3.1 `type: [array, "null"]`
// value is rewritten to a proper array schema), which would otherwise be lost.
schema.setAdditionalProperties(normalized);
result.setAdditionalProperties(normalized);
}
}

return result;
} else if (schema instanceof BooleanSchema) {
normalizeBooleanSchema(schema, visitedSchemas);
} else if (schema instanceof IntegerSchema) {
Expand Down Expand Up @@ -1105,7 +1107,8 @@ protected Schema normalizeArraySchema(Schema schema) {
}

protected Schema normalizeMapSchema(Schema schema) {
return processSetMapToNullable(schema);
Schema result = processNormalize31Spec(schema, new HashSet<>());

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.

P3: normalizeMapSchema now returns a possibly-replaced schema from processNormalize31Spec, but the map branch at normalizeSchema() discards that return value (normalizeMapSchema(schema);), unlike the array branch which consumes it. If processNormalize31Spec returns a replacement schema (e.g. the empty new Schema() it produces for a type-less JsonSchema), that normalized replacement is silently lost and all later map handling keeps using the original object. Consider consuming the returned schema at the call site or documenting that the map case must intentionally keep the original; this makes the behavior consistent with normalizeArraySchema and avoids a latent bug.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/java/org/openapitools/codegen/OpenAPINormalizer.java, line 1108:

<comment>normalizeMapSchema now returns a possibly-replaced schema from processNormalize31Spec, but the map branch at normalizeSchema() discards that return value (`normalizeMapSchema(schema);`), unlike the array branch which consumes it. If processNormalize31Spec returns a replacement schema (e.g. the empty `new Schema()` it produces for a type-less JsonSchema), that normalized replacement is silently lost and all later map handling keeps using the original object. Consider consuming the returned schema at the call site or documenting that the map case must intentionally keep the original; this makes the behavior consistent with normalizeArraySchema and avoids a latent bug.</comment>

<file context>
@@ -1105,7 +1105,8 @@ protected Schema normalizeArraySchema(Schema schema) {
 
     protected Schema normalizeMapSchema(Schema schema) {
-        return processSetMapToNullable(schema);
+        Schema result = processNormalize31Spec(schema, new HashSet<>());
+        return processSetMapToNullable(result);
     }
</file context>

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: normalizeMapSchema now wraps processNormalize31Spec, which can return a brand-new Schema instance (the empty-JsonSchema branch returns new Schema(), dropping sibling data). But the map branch of normalizeSchema calls normalizeMapSchema(schema) and ignores its return value, so any replacement-schema normalization from the new line is silently lost. For consistency with the array branch (which captures normalizeArraySchema's result and uses it), consider capturing the returned schema in the map branch so the 3.1 normalization actually takes effect.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/java/org/openapitools/codegen/OpenAPINormalizer.java, line 1108:

<comment>normalizeMapSchema now wraps processNormalize31Spec, which can return a brand-new Schema instance (the empty-JsonSchema branch returns `new Schema()`, dropping sibling data). But the map branch of normalizeSchema calls `normalizeMapSchema(schema)` and ignores its return value, so any replacement-schema normalization from the new line is silently lost. For consistency with the array branch (which captures `normalizeArraySchema`'s result and uses it), consider capturing the returned schema in the map branch so the 3.1 normalization actually takes effect.</comment>

<file context>
@@ -1105,7 +1105,8 @@ protected Schema normalizeArraySchema(Schema schema) {
 
     protected Schema normalizeMapSchema(Schema schema) {
-        return processSetMapToNullable(schema);
+        Schema result = processNormalize31Spec(schema, new HashSet<>());
+        return processSetMapToNullable(result);
     }
</file context>

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.

P1: Maps whose parent omits type lose their declared value schema when NORMALIZE_31SPEC is enabled. processNormalize31Spec replaces a typeless JsonSchema with a new empty schema, so preserve the original map when normalizing a parent with no explicit type.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/java/org/openapitools/codegen/OpenAPINormalizer.java, line 1110:

<comment>Maps whose parent omits `type` lose their declared value schema when `NORMALIZE_31SPEC` is enabled. `processNormalize31Spec` replaces a typeless `JsonSchema` with a new empty schema, so preserve the original map when normalizing a parent with no explicit type.</comment>

<file context>
@@ -1105,7 +1107,8 @@ protected Schema normalizeArraySchema(Schema schema) {
 
     protected Schema normalizeMapSchema(Schema schema) {
-        return processSetMapToNullable(schema);
+        Schema result = processNormalize31Spec(schema, new HashSet<>());
+        return processSetMapToNullable(result);
     }
</file context>
Suggested change
Schema result = processNormalize31Spec(schema, new HashSet<>());
Schema result = schema;
if (schema.getType() != null || schema.getTypes() != null) {
result = processNormalize31Spec(schema, new HashSet<>());
}

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.

P1: Schemas that omit an explicit type but define additionalProperties lose the map and its value type during 3.1 normalization. processNormalize31Spec replaces that JsonSchema with an empty Schema, so the map's additionalProperties is discarded; retaining the original map for type-less schemas (or copying all map metadata when replacing it) preserves the generated map type.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/java/org/openapitools/codegen/OpenAPINormalizer.java, line 1110:

<comment>Schemas that omit an explicit `type` but define `additionalProperties` lose the map and its value type during 3.1 normalization. `processNormalize31Spec` replaces that `JsonSchema` with an empty `Schema`, so the map's `additionalProperties` is discarded; retaining the original map for type-less schemas (or copying all map metadata when replacing it) preserves the generated map type.</comment>

<file context>
@@ -1105,7 +1107,8 @@ protected Schema normalizeArraySchema(Schema schema) {
 
     protected Schema normalizeMapSchema(Schema schema) {
-        return processSetMapToNullable(schema);
+        Schema result = processNormalize31Spec(schema, new HashSet<>());
+        return processSetMapToNullable(result);
     }
</file context>
Suggested change
Schema result = processNormalize31Spec(schema, new HashSet<>());
Schema result = schema.getType() == null && schema.getTypes() == null
? schema
: processNormalize31Spec(schema, new HashSet<>());

return processSetMapToNullable(result);
}

protected Schema normalizeSimpleSchema(Schema schema, Set<Schema> visitedSchemas) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2001,9 +2001,7 @@ public static boolean isNullable(Schema schema) {
if (schema.getExtensions() != null && schema.getExtensions().get(X_NULLABLE) != null) {
return Boolean.parseBoolean(schema.getExtensions().get(X_NULLABLE).toString());
}
if (schema.getTypes() != null && schema.getTypes().contains("null")) {
return true;
}

// In OAS 3.1, the recommended way to define a nullable property or object is to use oneOf.
if (isComposedSchema(schema)) {
return isNullableComposedSchema(schema);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public void testNullableAttributeInOas31_triggerWarning() {
/**
* The nullable-deprecated warning must NOT fire for an OAS 3.1 spec using the correct 3.1 null type syntax.
*/
@Test(enabled = false, description = "correct OAS 3.1 null type syntax (type: [string, null]) must NOT trigger the nullable-deprecated warning")
@Test(description = "correct OAS 3.1 null type syntax (type: [string, null]) must NOT trigger the nullable-deprecated warning")
public void testNullTypeInOas31_noWarning() {
OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_1/null-types-simple.yaml");
Schema<?> stringDataOrNull = (Schema<?>) openAPI.getComponents().getSchemas().get("WithNullableType").getProperties().get("stringDataOrNull");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,7 @@ paths:
additionalProperties:
format: int32
type: integer
type: object
description: successful operation
security:
- api_key: []
Expand Down
Loading