From 34ec98d17b298b3f4f0b882c4fd1e6e180b2e0d6 Mon Sep 17 00:00:00 2001 From: Michael Nahkies Date: Wed, 28 Aug 2019 13:31:38 +0100 Subject: [PATCH] [core] [regression] set parentName when a single possible parent exists (#3771) Whilst the spec states that the 'allOf' relationship does not imply a hierarchy: > While composition offers model extensibility, it does not imply a hierarchy between the models. > To support polymorphism, the OpenAPI Specification adds the discriminator field. Unfortunately this does not make sense for many existing use cases, that were supported by older versions of the generator. Therefore, I've restored the older behavior, specifically in the case that only a single possible parent schema is present. I think a more complete solution would generate interfaces for the composed schemas, and mark the generated class as implementing these. fixes issue 2845, and fixes issue #3523 --- .../org/openapitools/codegen/utils/ModelUtils.java | 10 ++++++++++ .../openapitools/codegen/java/JavaInheritanceTest.java | 9 +++++---- 2 files changed, 15 insertions(+), 4 deletions(-) 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 ad991f67954..50c9250224b 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 @@ -896,6 +896,8 @@ public class ModelUtils { public static String getParentName(ComposedSchema composedSchema, Map allSchemas) { List interfaces = getInterfaces(composedSchema); + List refedParentNames = new ArrayList<>(); + if (interfaces != null && !interfaces.isEmpty()) { for (Schema schema : interfaces) { // get the actual schema @@ -911,6 +913,7 @@ public class ModelUtils { } else { LOGGER.debug("Not a parent since discriminator.propertyName is not set {}", s.get$ref()); // not a parent since discriminator.propertyName is not set + refedParentNames.add(parentName); } } else { // not a ref, doing nothing @@ -918,6 +921,13 @@ public class ModelUtils { } } + // parent name only makes sense when there is a single obvious parent + if (refedParentNames.size() == 1) { + LOGGER.warn("[deprecated] inheritance without use of 'discriminator.propertyName' is deprecated " + + "and will be removed in a future release. Generating model for {}", composedSchema.getName()); + return refedParentNames.get(0); + } + return null; } 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 80bf59d12c2..3b8d691fac9 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 @@ -32,9 +32,10 @@ import org.testng.annotations.Test; public class JavaInheritanceTest { - @Test(description = "convert a composed model without discriminator") + + @Test(description = "convert a composed model with parent") public void javaInheritanceTest() { - final Schema allOfModel = new Schema().name("Base"); + final Schema parentModel = new Schema().name("Base"); final Schema schema = new ComposedSchema() .addAllOfItem(new Schema().$ref("Base")) @@ -42,7 +43,7 @@ public class JavaInheritanceTest { OpenAPI openAPI = TestUtils.createOpenAPI(); openAPI.setComponents(new Components() - .addSchemas(allOfModel.getName(), allOfModel) + .addSchemas(parentModel.getName(),parentModel) .addSchemas(schema.getName(), schema) ); @@ -52,7 +53,7 @@ public class JavaInheritanceTest { Assert.assertEquals(cm.name, "sample"); Assert.assertEquals(cm.classname, "Sample"); - Assert.assertEquals(cm.parent, null); + Assert.assertEquals(cm.parent, "Base"); Assert.assertEquals(cm.imports, Sets.newHashSet("Base")); }