From 70130edaab0ed26c6def607d41eebc84c6f9bc9e Mon Sep 17 00:00:00 2001 From: Nikita Shmakov Date: Wed, 15 May 2024 14:44:39 +0300 Subject: [PATCH] Properties with custom types inheritance fix (#18052) * custom types support in inheritance fix * files changed after scripts run * remove unused method * move cloneSchema to ModelUtils * imports * changes after scripts run * test cloning array of enums schema --- modules/openapi-generator/pom.xml | 5 ++ .../openapitools/codegen/DefaultCodegen.java | 9 +-- .../codegen/utils/ModelUtils.java | 33 +++++------ .../codegen/java/JavaInheritanceTest.java | 29 +++++++++- .../codegen/utils/ModelUtilsTest.java | 55 +++++++++++++++++++ samples/client/echo_api/r/R/data_query.R | 4 +- samples/client/echo_api/r/docs/DataQuery.md | 2 +- 7 files changed, 107 insertions(+), 30 deletions(-) diff --git a/modules/openapi-generator/pom.xml b/modules/openapi-generator/pom.xml index 0166805440c6..15e0d19819bf 100644 --- a/modules/openapi-generator/pom.xml +++ b/modules/openapi-generator/pom.xml @@ -397,6 +397,11 @@ jackson-datatype-joda ${jackson.version} + + com.fasterxml.jackson.core + jackson-annotations + ${jackson.version} + com.github.joschi.jackson jackson-datatype-threetenbp 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 35f7ae259a5a..7882cc3ab04b 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 @@ -17,7 +17,6 @@ package org.openapitools.codegen; -import com.fasterxml.jackson.annotation.JsonCreator; import com.github.benmanes.caffeine.cache.Cache; import com.github.benmanes.caffeine.cache.Caffeine; import com.github.benmanes.caffeine.cache.Ticker; @@ -26,7 +25,6 @@ import com.samskivert.mustache.Mustache; import com.samskivert.mustache.Mustache.Compiler; import com.samskivert.mustache.Mustache.Lambda; -import io.swagger.v3.core.util.AnnotationsUtils; import io.swagger.v3.core.util.Json; import io.swagger.v3.oas.models.OpenAPI; import io.swagger.v3.oas.models.Operation; @@ -65,7 +63,6 @@ import org.openapitools.codegen.serializer.SerializerUtils; import org.openapitools.codegen.templating.MustacheEngineAdapter; import org.openapitools.codegen.templating.mustache.*; -import org.openapitools.codegen.utils.CamelizeOption; import org.openapitools.codegen.utils.ModelUtils; import org.openapitools.codegen.utils.OneOfImplementorAdditionalData; import org.slf4j.Logger; @@ -2945,11 +2942,7 @@ private void mergeProperties(Map existingProperties, Map - existingProperties.put( - key, - ModelUtils.cloneSchema(value, specVersionGreaterThanOrEqualTo310(openAPI)) - )); + newProperties.forEach((key, value) -> existingProperties.put(key, ModelUtils.cloneSchema(value))); if (null != existingType && null != newType && null != newType.getEnum() && !newType.getEnum().isEmpty()) { for (Object e : newType.getEnum()) { // ensure all interface enum types are added to schema 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 0aec52f4c1c6..c5f6422fc7a7 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 @@ -17,9 +17,10 @@ package org.openapitools.codegen.utils; +import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; -import io.swagger.v3.core.util.AnnotationsUtils; +import com.fasterxml.jackson.databind.jsontype.BasicPolymorphicTypeValidator; import io.swagger.v3.oas.models.OpenAPI; import io.swagger.v3.oas.models.Operation; import io.swagger.v3.oas.models.PathItem; @@ -75,10 +76,16 @@ public class ModelUtils { private static final ObjectMapper JSON_MAPPER; private static final ObjectMapper YAML_MAPPER; + private static final ObjectMapper TYPED_JSON_MAPPER = new ObjectMapper(); static { JSON_MAPPER = ObjectMapperFactory.createJson(); YAML_MAPPER = ObjectMapperFactory.createYaml(); + + BasicPolymorphicTypeValidator ptv = BasicPolymorphicTypeValidator.builder() + .allowIfSubType(Object.class) + .build(); + TYPED_JSON_MAPPER.activateDefaultTyping(ptv, ObjectMapper.DefaultTyping.EVERYTHING); } public static boolean isDisallowAdditionalPropertiesIfNotPresent() { @@ -2179,24 +2186,14 @@ public static boolean isParent(Schema schema) { return false; } - /** - * Returns a clone of the schema. - * - * @param schema the schema. - * @param specVersionGreaterThanOrEqualTo310 true if spec version is 3.1.0 or later. - * @return a clone of the schema. - */ - public static Schema cloneSchema(Schema schema, boolean specVersionGreaterThanOrEqualTo310) { - Schema clone = AnnotationsUtils.clone(schema, specVersionGreaterThanOrEqualTo310); - - // check to see if type is set and clone it if needed - // in openapi-generator, we also store type in `type` for 3.1 schema - // to make it backward compatible with the rest of the code base. - if (schema.getType() != null) { - clone.setType(schema.getType()); + public static Schema cloneSchema(Schema schema) { + try { + String json = TYPED_JSON_MAPPER.writeValueAsString(schema); + return TYPED_JSON_MAPPER.readValue(json, schema.getClass()); + } catch (JsonProcessingException ex) { + LOGGER.error("Can't clone schema {}", schema, ex); + return schema; } - - return clone; } @FunctionalInterface diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaInheritanceTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaInheritanceTest.java index 9d55893acd6f..2fbe08939c90 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaInheritanceTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaInheritanceTest.java @@ -37,7 +37,6 @@ public class JavaInheritanceTest { - @Test(description = "convert a composed model with parent") public void javaInheritanceTest() { final Schema parentModel = new Schema().name("Base"); @@ -181,4 +180,32 @@ public void javaInheritanceWithRequiredAttributesOnComposedObject() { Assert.assertFalse(propertyCD.required); Assert.assertEquals(cm.requiredVars.size() + cm.optionalVars.size(), cm.allVars.size()); } + + @Test(description = "convert a composed model with parent with custom schema param") + public void javaInheritanceWithCustomSchemaTest() { + Schema custom = new Schema() + .name("Custom") + .addProperty("value", new StringSchema()); + Schema parentModel = new Schema() + .name("Base") + .addProperty("customProperty", new Schema().type("custom")); + Schema schema = new ComposedSchema() + .name("Composed") + .addAllOfItem(new Schema().$ref("Base")); + + OpenAPI openAPI = TestUtils.createOpenAPI(); + openAPI.setComponents(new Components() + .addSchemas(custom.getName(), custom) + .addSchemas(parentModel.getName(), parentModel) + .addSchemas(schema.getName(), schema) + ); + + JavaClientCodegen codegen = new JavaClientCodegen(); + codegen.setOpenAPI(openAPI); + codegen.schemaMapping() + .put("custom", custom.getName()); + CodegenModel model = codegen.fromModel("sample", schema); + + Assert.assertTrue(model.imports.contains(custom.getName())); + } } diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/utils/ModelUtilsTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/utils/ModelUtilsTest.java index d99c2fa678c6..187e22f55cc8 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/utils/ModelUtilsTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/utils/ModelUtilsTest.java @@ -26,6 +26,7 @@ import org.testng.Assert; import org.testng.annotations.Test; +import java.math.BigDecimal; import java.util.*; public class ModelUtilsTest { @@ -391,4 +392,58 @@ public void test31Schemas() { Schema complexComposedSchema = ModelUtils.getSchema(openAPI, "ComplexComposedSchema"); Assert.assertTrue(ModelUtils.isComplexComposedSchema(complexComposedSchema)); } + + @Test + public void testCloneNumberSchema() { + Schema schema = new NumberSchema() + .name("test-schema") + .minimum(new BigDecimal(100)); + + Schema deepCopy = ModelUtils.cloneSchema(schema); + + Assert.assertEquals(schema, deepCopy); + } + + @Test + public void testCloneCustomSchema() { + Schema schema = new Schema().type("money"); + + Schema deepCopy = ModelUtils.cloneSchema(schema); + + Assert.assertEquals(schema, deepCopy); + } + + @Test + public void testCloneComposedSchema() { + Schema base1 = new Schema() + .name("Base1") + .addProperty("foo", new StringSchema()); + Schema base2 = new Schema() + .name("Base2") + .addProperty("bar", new StringSchema()); + Schema composedSchema = new ComposedSchema() + .name("Composed") + .allOf(List.of(base1, base2)) + .addProperty("baz", new StringSchema()); + + var deepCopy = ModelUtils.cloneSchema(composedSchema); + + Assert.assertEquals(composedSchema, deepCopy); + } + + @Test + public void testCloneArrayOfEnumsSchema() { + Schema arraySchema = new Schema() + .name("ArrayType") + .type("array") + .items(new Schema() + .type("string") + ._enum(List.of("SUCCESS", "FAILURE", "SKIPPED")) + ) + ._default(List.of("SUCCESS", "FAILURE")); + + var deepCopy = ModelUtils.cloneSchema(arraySchema); + + Assert.assertEquals(arraySchema, deepCopy); + } } diff --git a/samples/client/echo_api/r/R/data_query.R b/samples/client/echo_api/r/R/data_query.R index 0a749f98d6ed..228cd65b8467 100644 --- a/samples/client/echo_api/r/R/data_query.R +++ b/samples/client/echo_api/r/R/data_query.R @@ -30,13 +30,13 @@ DataQuery <- R6::R6Class( #' Initialize a new DataQuery class. #' #' @param id Query - #' @param outcomes outcomes. Default to [SUCCESS, FAILURE]. + #' @param outcomes outcomes. Default to ["SUCCESS","FAILURE"]. #' @param suffix test suffix #' @param text Some text containing white spaces #' @param date A date #' @param ... Other optional arguments. #' @export - initialize = function(`id` = NULL, `outcomes` = [SUCCESS, FAILURE], `suffix` = NULL, `text` = NULL, `date` = NULL, ...) { + initialize = function(`id` = NULL, `outcomes` = ["SUCCESS","FAILURE"], `suffix` = NULL, `text` = NULL, `date` = NULL, ...) { if (!is.null(`id`)) { if (!(is.numeric(`id`) && length(`id`) == 1)) { stop(paste("Error! Invalid data for `id`. Must be an integer:", `id`)) diff --git a/samples/client/echo_api/r/docs/DataQuery.md b/samples/client/echo_api/r/docs/DataQuery.md index 097f1885fe5f..50015244dc5b 100644 --- a/samples/client/echo_api/r/docs/DataQuery.md +++ b/samples/client/echo_api/r/docs/DataQuery.md @@ -5,7 +5,7 @@ Name | Type | Description | Notes ------------ | ------------- | ------------- | ------------- **id** | **integer** | Query | [optional] -**outcomes** | **array[character]** | | [optional] [default to [SUCCESS, FAILURE]] [Enum: ] +**outcomes** | **array[character]** | | [optional] [default to ["SUCCESS","FAILURE"]] [Enum: ] **suffix** | **character** | test suffix | [optional] **text** | **character** | Some text containing white spaces | [optional] **date** | **character** | A date | [optional]