diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java index 945d828d9295..bffbed2d6f6a 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java @@ -2941,7 +2941,7 @@ private void mergeProperties(Map existingProperties, Map - existingProperties.put(key, ModelUtils.cloneSchema(value, specVersionGreaterThanOrEqualTo310(openAPI))) + putProperty(existingProperties, key, ModelUtils.cloneSchema(value, specVersionGreaterThanOrEqualTo310(openAPI))) ); if (null != existingType && null != newType && null != newType.getEnum() && !newType.getEnum().isEmpty()) { for (Object e : newType.getEnum()) { @@ -3608,7 +3608,7 @@ protected void addProperties(Map properties, List requir if (ModelUtils.isComposedSchema(schema)) { // fix issue #16797 and #15796, constructor fail by missing parent required params if (ModelUtils.hasProperties(schema)) { - properties.putAll(schema.getProperties()); + putProperties(properties, schema.getProperties()); } if (schema.getAllOf() != null) { @@ -3642,13 +3642,39 @@ protected void addProperties(Map properties, List requir return; } if (schema.getProperties() != null) { - properties.putAll(schema.getProperties()); + putProperties(properties, schema.getProperties()); } if (schema.getRequired() != null) { required.addAll(schema.getRequired()); } } + /** + * Adds each property to the target map. When a property of the same name is already present + * with type information and the incoming schema carries no type of its own (for example an + * allOf part that only sets 'nullable: true' on an inherited property), the incoming + * constraints (nullable, description, validation keywords, format, default, extensions) + * are applied on top of the existing schema instead of replacing it, so the type is not + * lost. See issue #4128. + */ + private void putProperties(Map targetProperties, Map newProperties) { + newProperties.forEach((name, incoming) -> putProperty(targetProperties, name, incoming)); + } + + private void putProperty(Map targetProperties, String name, Schema incoming) { + Schema existing = targetProperties.get(name); + if (existing != null && incoming != null + && !ModelUtils.isAnyType(existing) + && ModelUtils.isMetadataOnlySchema(incoming) + && incoming.getEnum() == null && incoming.getConst() == null && incoming.getNot() == null) { + Schema merged = ModelUtils.cloneSchema(existing, specVersionGreaterThanOrEqualTo310(openAPI)); + ModelUtils.copyConstraints(incoming, merged); + targetProperties.put(name, merged); + } else { + targetProperties.put(name, incoming); + } + } + /** * Camelize the method name of the getter and setter * diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java index 17ce0a391b46..704b3f8f6dab 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java @@ -2807,6 +2807,55 @@ public static void copyMetadata(Schema from, Schema to) { } } + /** + * Copies metadata plus the validation and format keywords that copyMetadata does not cover. + * Used when an allOf part only constrains an inherited property (see issue #4128), so no + * declared keyword is lost while the type is kept. + * + * @param from schema to copy from + * @param to schema to copy to + */ + public static void copyConstraints(Schema from, Schema to) { + Map targetExtensions = to.getExtensions() == null ? null : new HashMap<>(to.getExtensions()); + copyMetadata(from, to); + // merge extensions per key instead of replacing, the source wins on conflicts + if (targetExtensions != null && from.getExtensions() != null) { + Map mergedExtensions = new HashMap<>(targetExtensions); + mergedExtensions.putAll(from.getExtensions()); + to.setExtensions(mergedExtensions); + } + if (from.getFormat() != null) { + to.setFormat(from.getFormat()); + } + if (from.getPattern() != null) { + to.setPattern(from.getPattern()); + } + if (from.getExclusiveMaximum() != null) { + to.setExclusiveMaximum(from.getExclusiveMaximum()); + } + if (from.getExclusiveMinimum() != null) { + to.setExclusiveMinimum(from.getExclusiveMinimum()); + } + if (from.getExclusiveMaximumValue() != null) { + to.setExclusiveMaximumValue(from.getExclusiveMaximumValue()); + } + if (from.getExclusiveMinimumValue() != null) { + to.setExclusiveMinimumValue(from.getExclusiveMinimumValue()); + } + if (from.getMultipleOf() != null) { + to.setMultipleOf(from.getMultipleOf()); + } + if (from.getUniqueItems() != null) { + to.setUniqueItems(from.getUniqueItems()); + } + if (from.getMaxProperties() != null) { + to.setMaxProperties(from.getMaxProperties()); + } + if (from.getMinProperties() != null) { + to.setMinProperties(from.getMinProperties()); + } + } + /** * Returns true if a schema is only metadata and not an actual type. * For example, a schema that only has a `description` without any `properties` or `$ref` defined. diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/DefaultCodegenTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/DefaultCodegenTest.java index dbc777e2ca15..28573bc2d2cf 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/DefaultCodegenTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/DefaultCodegenTest.java @@ -1314,6 +1314,27 @@ public void testAllOfRequired() { assertEquals(getRequiredVars(childModel), Collections.singletonList("name")); } + @Test + public void testAllOfNullableWithoutTypeKeepsType() { + // issue #4128: an allOf part that only sets 'nullable: true' on a property + // defined in another part must not erase the property type + final OpenAPI openAPI = TestUtils.parseFlattenSpec("src/test/resources/3_0/allOf-nullable-typeless-override.yaml"); + DefaultCodegen codegen = new DefaultCodegen(); + codegen.setOpenAPI(openAPI); + + Schema schema = openAPI.getComponents().getSchemas().get("UpdateFirm"); + CodegenModel model = codegen.fromModel("UpdateFirm", schema); + + CodegenProperty addressId = model.vars.stream() + .filter(v -> "addressId".equals(v.baseName)).findFirst().orElseThrow(); + assertEquals("String", addressId.dataType); + assertTrue(addressId.isNullable); + assertEquals(Integer.valueOf(36), addressId.maxLength); + // extensions from both parts survive the merge + assertEquals("keep", addressId.vendorExtensions.get("x-base-marker")); + assertEquals("added", addressId.vendorExtensions.get("x-overlay-marker")); + } + @Test public void testAllOfSingleAndDoubleRefWithOwnPropsNoDiscriminator() { final OpenAPI openAPI = TestUtils.parseFlattenSpec("src/test/resources/3_0/allOf_composition.yaml"); diff --git a/modules/openapi-generator/src/test/resources/3_0/allOf-nullable-typeless-override.yaml b/modules/openapi-generator/src/test/resources/3_0/allOf-nullable-typeless-override.yaml new file mode 100644 index 000000000000..32f2c9840914 --- /dev/null +++ b/modules/openapi-generator/src/test/resources/3_0/allOf-nullable-typeless-override.yaml @@ -0,0 +1,41 @@ +openapi: 3.0.3 +info: + title: allOf nullable without type + version: 1.0.0 +paths: + /firm/{firmId}: + patch: + operationId: updateFirm + parameters: + - in: path + name: firmId + required: true + schema: + type: integer + format: int64 + requestBody: + content: + application/json: + schema: + $ref: "#/components/schemas/UpdateFirm" + responses: + "204": + description: Updated +components: + schemas: + FirmProperties: + properties: + addressId: + type: string + x-base-marker: keep + UpdateFirm: + allOf: + - $ref: "#/components/schemas/FirmProperties" + - properties: + firmName: + type: string + nullable: true + addressId: + nullable: true + maxLength: 36 + x-overlay-marker: added