Jersey2 supports additional properties with composed schema (#6523)

* Mustache template should use invokerPackage tag to generate import * fix typo, fix script issue, add log statement for troubleshooting * Add java jersey2 samples with OpenAPI doc that has HTTP signature security scheme * Add sample for Java jersey2 and HTTP signature scheme * Add unit test for oneOf schema deserialization * Add unit test for oneOf schema deserialization * Add log statements * Add profile for jersey2 * Temporarily disable unit test * Temporarily disable unit test * support for discriminator in jersey2 * fix typo in pom.xml * disable unit test because jersey2 deserialization is broken * disable unit test because jersey2 deserialization is broken * fix duplicate jersey2 samples * fix duplicate jersey2 samples * Add code comments * fix duplicate artifact id * fix duplicate jersey2 samples * run samples scripts * resolve merge conflicts * Add unit tests * fix unit tests * continue implementation of discriminator lookup * throw deserialization exception when value is null and schema does not allow null value * continue implementation of compose schema * continue implementation of compose schema * continue implementation of compose schema * Add more unit tests * Add unit tests for anyOf * Add unit tests * Set supportsAdditionalPropertiesWithComposedSchema to true for Java jersey2 * Support additional properties as nested field * Support additional properties as nested field * add code comments * add customer deserializer * Fix 'method too big' error with generated code * resolve merge conflicts Co-authored-by: Vikrant Balyan (vvb) <[email protected]>
OpenAPITools · Jun 5, 2020 · 1bcc917 · 1bcc917
1 parent bbe7976
commit 1bcc917
Show file tree

Hide file tree

Showing 138 changed files with 869 additions and 286 deletions.
diff --git a/CI/samples.ci/client/petstore/java/test-manual/jersey2-java8/JSONComposedSchemaTest.java b/CI/samples.ci/client/petstore/java/test-manual/jersey2-java8/JSONComposedSchemaTest.java
@@ -94,23 +94,14 @@ public void testOneOfSchemaWithDiscriminator() throws Exception {
             assertNotNull(o);
             assertTrue(o.getActualInstance() instanceof Zebra);
             Zebra z = (Zebra)o.getActualInstance();
-            // TODO: this is incorrect: assert the value is Zebra.TypeEnum.PLAINS
-            assertNull(z.getType());
-            //assertEquals(Zebra.TypeEnum.PLAINS, z.getType());
+            assertEquals(Zebra.TypeEnum.PLAINS, z.getType());
         }
         {
             // The discriminator value is valid but the 'type' value is invalid.
-            // TODO: the current deserialization code is incorrectly accepting the input data.
-            // The unit test code below should be rewritten to assert an exception.
             String str = "{ \"className\": \"zebra\", \"type\": \"garbage_value\" }";
-            AbstractOpenApiSchema o = json.getContext(null).readValue(str, Mammal.class);
-            assertNotNull(o);
-            assertTrue(o.getActualInstance() instanceof Zebra);
-            Zebra z = (Zebra)o.getActualInstance();
-            assertNull(z.getType());
-            //Exception exception = assertThrows(JsonMappingException.class, () -> {
-            //json.getContext(null).readValue(str, Mammal.class);
-            //});
+            Exception exception = assertThrows(JsonMappingException.class, () -> {
+                json.getContext(null).readValue(str, Mammal.class);
+            });
         }
         {
             // The discriminator value is zebra but the properties belong to Whale.

diff --git a/...enapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java b/...enapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java
@@ -1647,10 +1647,24 @@ public void setAdditionalModelTypeAnnotations(final List<String> additionalModel
 
     @Override
     protected void addAdditionPropertiesToCodeGenModel(CodegenModel codegenModel, Schema schema) {
-        super.addAdditionPropertiesToCodeGenModel(codegenModel, schema);
+        if (!supportsAdditionalPropertiesWithComposedSchema) {
+            // The additional (undeclared) propertiees are modeled in Java as a HashMap.
+            // 
+            // 1. supportsAdditionalPropertiesWithComposedSchema is set to false:
+            //    The generated model class extends from the HashMap. That does not work
+            //    with composed schemas that also use a discriminator because the model class
+            //    is supposed to extend from the generated parent model class.
+            // 2. supportsAdditionalPropertiesWithComposedSchema is set to true:
+            //    The HashMap is a field.
+            super.addAdditionPropertiesToCodeGenModel(codegenModel, schema);
+        }
 
         // See https://github.com/OpenAPITools/openapi-generator/pull/1729#issuecomment-449937728
-        codegenModel.additionalPropertiesType = getSchemaType(getAdditionalProperties(schema));
-        addImport(codegenModel, codegenModel.additionalPropertiesType);
+        Schema s = getAdditionalProperties(schema);
+        // 's' may be null if 'additionalProperties: false' in the OpenAPI schema.
+        if (s != null) {
+            codegenModel.additionalPropertiesType = getSchemaType(s);
+            addImport(codegenModel, codegenModel.additionalPropertiesType);
+        }
     }
 }
diff --git a/...openapi-generator/src/main/java/org/openapitools/codegen/languages/JavaClientCodegen.java b/...openapi-generator/src/main/java/org/openapitools/codegen/languages/JavaClientCodegen.java
@@ -176,6 +176,7 @@ public JavaClientCodegen() {
         // inherit from self, any oneOf schemas, any anyOf schemas, any x-discriminator-values,
         // and the discriminator mapping schemas in the OAS document.
         this.setLegacyDiscriminatorBehavior(false);
+
     }
 
     @Override
@@ -371,6 +372,14 @@ public void processOpts() {
             }
             supportingFiles.add(new SupportingFile("AbstractOpenApiSchema.mustache", (sourceFolder + File.separator + modelPackage().replace('.', File.separatorChar)).replace('/', File.separatorChar), "AbstractOpenApiSchema.java"));
             forceSerializationLibrary(SERIALIZATION_LIBRARY_JACKSON);
+
+            // Composed schemas can have the 'additionalProperties' keyword, as specified in JSON schema.
+            // In principle, this should be enabled by default for all code generators. However due to limitations
+            // in other code generators, support needs to be enabled on a case-by-case basis.
+            // The flag below should be set for all Java libraries, but the templates need to be ported
+            // one by one for each library.
+            supportsAdditionalPropertiesWithComposedSchema = true;
+
         } else if (NATIVE.equals(getLibrary())) {
             setJava8Mode(true);
             additionalProperties.put("java8", "true");

diff --git a/...penapi-generator/src/main/resources/Java/libraries/jersey2/AbstractOpenApiSchema.mustache b/...penapi-generator/src/main/resources/Java/libraries/jersey2/AbstractOpenApiSchema.mustache
@@ -113,4 +113,7 @@ public abstract class AbstractOpenApiSchema {
             return Boolean.FALSE;
         }
     }
+
+{{>libraries/jersey2/additional_properties}}
+
 }
diff --git a/modules/openapi-generator/src/main/resources/Java/libraries/jersey2/JSON.mustache b/modules/openapi-generator/src/main/resources/Java/libraries/jersey2/JSON.mustache
@@ -104,15 +104,13 @@ public class JSON implements ContextResolver<ObjectMapper> {
     Map<String, Class> discriminatorMappings;
 
     // Constructs a new class discriminator.
-    ClassDiscriminatorMapping(Class cls, String name) {
+    ClassDiscriminatorMapping(Class cls, String propertyName, Map<String, Class> mappings) {
       modelClass = cls;
-      discriminatorName = name;
+      discriminatorName = propertyName;
       discriminatorMappings = new HashMap<String, Class>();
-    }
-
-    // Register a discriminator mapping for the specified model class.
-    void registerMapping(String mapping, Class cls) {
-      discriminatorMappings.put(mapping, cls);
+      if (mappings != null) {
+        discriminatorMappings.putAll(mappings);
+      }
     }
 
     // Return the name of the discriminator property for this model class.
@@ -215,41 +213,35 @@ public class JSON implements ContextResolver<ObjectMapper> {
     return false;
   }
 
+  /**
+   * A map of discriminators for all model classes.
+   */
   private static Map<Class, ClassDiscriminatorMapping> modelDiscriminators = new HashMap<Class, ClassDiscriminatorMapping>();
 
   /**
-   * Register the discriminators for all composed models.
+   * A map of oneOf/anyOf descendants for each model class.
    */
-  private static void registerDiscriminators() {
-    {{#models}}
-    {{#model}}
-    {{#discriminator}}
-    {
-      // Initialize the discriminator mappings for '{{classname}}'.
-      ClassDiscriminatorMapping m = new ClassDiscriminatorMapping({{classname}}.class, "{{propertyBaseName}}");
-      {{#mappedModels}}
-      m.registerMapping("{{mappingName}}", {{modelName}}.class);
-      {{/mappedModels}}
-      m.registerMapping("{{name}}", {{classname}}.class);
-      modelDiscriminators.put({{classname}}.class, m);
-    }
-    {{/discriminator}}
-    {{/model}}
-    {{/models}}
-  }
-
   private static Map<Class, Map<String, GenericType>> modelDescendants = new HashMap<Class, Map<String, GenericType>>();
 
   /**
-   * Register the oneOf/anyOf descendants.
-   * TODO: this should not be a public method.
-   */
-  public static void registerDescendants(Class modelClass, Map<String, GenericType> descendants) {
-    modelDescendants.put(modelClass, descendants);
+    * Register a model class discriminator.
+    *
+    * @param modelClass the model class
+    * @param discriminatorPropertyName the name of the discriminator property
+    * @param mappings a map with the discriminator mappings.
+    */
+  public static void registerDiscriminator(Class modelClass, String discriminatorPropertyName, Map<String, Class> mappings) {
+    ClassDiscriminatorMapping m = new ClassDiscriminatorMapping(modelClass, discriminatorPropertyName, mappings);
+    modelDiscriminators.put(modelClass, m);
   }
 
-  static {
-    registerDiscriminators();
+  /**
+    * Register the oneOf/anyOf descendants of the modelClass.
+    *
+    * @param modelClass the model class
+    * @param descendants a map of oneOf/anyOf descendants.
+    */
+  public static void registerDescendants(Class modelClass, Map<String, GenericType> descendants) {
+    modelDescendants.put(modelClass, descendants);
   }
-
 }
diff --git a/...penapi-generator/src/main/resources/Java/libraries/jersey2/additional_properties.mustache b/...penapi-generator/src/main/resources/Java/libraries/jersey2/additional_properties.mustache
@@ -0,0 +1,31 @@
+{{#additionalPropertiesType}}
+  /**
+   * A container for additional, undeclared properties.
+   * This is a holder for any undeclared properties as specified with
+   * the 'additionalProperties' keyword in the OAS document.
+   */
+  @JsonUnwrapped
+  private Map<String, {{{.}}}> additionalProperties;
+
+  /**
+   * Set the additional (undeclared) property with the specified name and value.
+   * If the property does not already exist, create it otherwise replace it.
+   */
+  public {{classname}} putAdditionalProperty(String key, {{{.}}} value) {
+    if (this.additionalProperties == null) {
+        this.additionalProperties = new HashMap<String, {{{.}}}>();
+    }
+    this.additionalProperties.put(key, value);
+    return this;
+  }
+
+  /**
+   * Return the additional (undeclared) property with the specified name.
+   */
+  public {{{.}}} getAdditionalProperty(String key) {
+    if (this.additionalProperties == null) {
+        return null;
+    }
+    return this.additionalProperties.get(key);
+  }
+{{/additionalPropertiesType}}
diff --git a/modules/openapi-generator/src/main/resources/Java/libraries/jersey2/anyof_model.mustache b/modules/openapi-generator/src/main/resources/Java/libraries/jersey2/anyof_model.mustache
@@ -5,10 +5,7 @@ import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.HashSet;
-import java.util.Map;
-import {{invokerPackage}}.JSON;
 
 import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.core.JsonProcessingException;
@@ -65,6 +62,28 @@ public class {{classname}} extends AbstractOpenApiSchema{{#vendorExtensions.x-im
             throw new IOException(String.format("Failed deserialization for {{classname}}: no match found"));
         }
 
+        {{#additionalPropertiesType}}
+        /**
+         * Method called to deal with a property that did not map to a known Bean property.
+         * Method can deal with the problem as it sees fit (ignore, throw exception); but if it does return,
+         * it has to skip the matching Json content parser has.
+         *
+         * @param p - Parser that points to value of the unknown property
+         * @param ctxt - Context for deserialization; allows access to the parser, error reporting functionality
+         * @param instanceOrClass - Instance that is being populated by this deserializer, or if not known, Class that would be instantiated. If null, will assume type is what getValueClass() returns.
+         * @param propName - Name of the property that cannot be mapped
+         */
+        @Override
+        protected void handleUnknownProperty(JsonParser p,
+                                     DeserializationContext ctxt,
+                                     Object instanceOrClass,
+                                     String propName) throws IOException {
+            System.out.println("Deserializing unknown property " + propName);
+            {{{.}}} deserialized = p.readValueAs({{{.}}}.class);
+            additionalProperties.put(propName, deserialized);
+        }
+        {{/additionalPropertiesType}}
+
         /**
          * Handle deserialization of the 'null' value.
          */
@@ -85,7 +104,18 @@ public class {{classname}} extends AbstractOpenApiSchema{{#vendorExtensions.x-im
     public {{classname}}() {
         super("anyOf", {{#isNullable}}Boolean.TRUE{{/isNullable}}{{^isNullable}}Boolean.FALSE{{/isNullable}});
     }
+{{> libraries/jersey2/additional_properties }}
+    {{#additionalPropertiesType}}
+    @Override
+    public boolean equals(Object o) {
+        return super.equals(o) && Objects.equals(this.additionalProperties, o.additionalProperties)
+    }
 
+    @Override
+    public int hashCode() {
+        return Objects.hash(instance, isNullable, schemaType, additionalProperties);
+    }
+    {{/additionalPropertiesType}}
     {{#anyOf}}
     public {{classname}}({{{.}}} o) {
         super("anyOf", {{#isNullable}}Boolean.TRUE{{/isNullable}}{{^isNullable}}Boolean.FALSE{{/isNullable}});
@@ -99,6 +129,15 @@ public class {{classname}} extends AbstractOpenApiSchema{{#vendorExtensions.x-im
         });
         {{/anyOf}}
         JSON.registerDescendants({{classname}}.class, Collections.unmodifiableMap(schemas));
+        {{#discriminator}}
+        // Initialize and register the discriminator mappings.
+        Map<String, Class> mappings = new HashMap<String, Class>();
+        {{#mappedModels}}
+        mappings.put("{{mappingName}}", {{modelName}}.class);
+        {{/mappedModels}}
+        mappings.put("{{name}}", {{classname}}.class);
+        JSON.registerDiscriminator({{classname}}.class, "{{propertyBaseName}}", mappings);
+        {{/discriminator}}
     }
 
     @Override

diff --git a/modules/openapi-generator/src/main/resources/Java/libraries/jersey2/model.mustache b/modules/openapi-generator/src/main/resources/Java/libraries/jersey2/model.mustache
@@ -6,9 +6,20 @@ package {{package}};
 import org.apache.commons.lang3.builder.EqualsBuilder;
 import org.apache.commons.lang3.builder.HashCodeBuilder;
 {{/useReflectionEqualsHashCode}}
+{{#models}}
+{{#model}}
+{{#additionalPropertiesType}}
+import java.util.Map;
+import java.util.HashMap;
+import com.fasterxml.jackson.annotation.JsonUnwrapped;
+{{/additionalPropertiesType}}
+{{/model}}
+{{/models}}
 {{^supportJava6}}
 import java.util.Objects;
 import java.util.Arrays;
+import java.util.Map;
+import java.util.HashMap;
 {{/supportJava6}}
 {{#supportJava6}}
 import org.apache.commons.lang3.ObjectUtils;
@@ -39,6 +50,7 @@ import javax.validation.Valid;
 {{#performBeanValidation}}
 import org.hibernate.validator.constraints.*;
 {{/performBeanValidation}}
+import {{invokerPackage}}.JSON;
 
 {{#models}}
 {{#model}}

diff --git a/modules/openapi-generator/src/main/resources/Java/libraries/jersey2/oneof_model.mustache b/modules/openapi-generator/src/main/resources/Java/libraries/jersey2/oneof_model.mustache
@@ -5,10 +5,7 @@ import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.HashSet;
-import java.util.Map;
-import {{invokerPackage}}.JSON;
 
 import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.core.JsonProcessingException;
@@ -73,6 +70,28 @@ public class {{classname}} extends AbstractOpenApiSchema{{#vendorExtensions.x-im
             throw new IOException(String.format("Failed deserialization for {{classname}}: %d classes match result, expected 1", match));
         }
 
+        {{#additionalPropertiesType}}
+        /**
+         * Method called to deal with a property that did not map to a known Bean property.
+         * Method can deal with the problem as it sees fit (ignore, throw exception); but if it does return,
+         * it has to skip the matching Json content parser has.
+         *
+         * @param p - Parser that points to value of the unknown property
+         * @param ctxt - Context for deserialization; allows access to the parser, error reporting functionality
+         * @param instanceOrClass - Instance that is being populated by this deserializer, or if not known, Class that would be instantiated. If null, will assume type is what getValueClass() returns.
+         * @param propName - Name of the property that cannot be mapped
+         */
+        @Override
+        protected void handleUnknownProperty(JsonParser p,
+                                     DeserializationContext ctxt,
+                                     Object instanceOrClass,
+                                     String propName) throws IOException {
+            System.out.println("Deserializing unknown property " + propName);
+            {{{.}}} deserialized = p.readValueAs({{{.}}}.class);
+            additionalProperties.put(propName, deserialized);
+        }
+        {{/additionalPropertiesType}}
+
         /**
          * Handle deserialization of the 'null' value.
          */
@@ -93,7 +112,18 @@ public class {{classname}} extends AbstractOpenApiSchema{{#vendorExtensions.x-im
     public {{classname}}() {
         super("oneOf", {{#isNullable}}Boolean.TRUE{{/isNullable}}{{^isNullable}}Boolean.FALSE{{/isNullable}});
     }
+{{> libraries/jersey2/additional_properties }}
+    {{#additionalPropertiesType}}
+    @Override
+    public boolean equals(Object o) {
+        return super.equals(o) && Objects.equals(this.additionalProperties, o.additionalProperties)
+    }
 
+    @Override
+    public int hashCode() {
+        return Objects.hash(instance, isNullable, schemaType, additionalProperties);
+    }
+    {{/additionalPropertiesType}}
     {{#oneOf}}
     public {{classname}}({{{.}}} o) {
         super("oneOf", {{#isNullable}}Boolean.TRUE{{/isNullable}}{{^isNullable}}Boolean.FALSE{{/isNullable}});
@@ -107,6 +137,15 @@ public class {{classname}} extends AbstractOpenApiSchema{{#vendorExtensions.x-im
         });
         {{/oneOf}}
         JSON.registerDescendants({{classname}}.class, Collections.unmodifiableMap(schemas));
+        {{#discriminator}}
+        // Initialize and register the discriminator mappings.
+        Map<String, Class> mappings = new HashMap<String, Class>();
+        {{#mappedModels}}
+        mappings.put("{{mappingName}}", {{modelName}}.class);
+        {{/mappedModels}}
+        mappings.put("{{name}}", {{classname}}.class);
+        JSON.registerDiscriminator({{classname}}.class, "{{propertyBaseName}}", mappings);
+        {{/discriminator}}
     }
 
     @Override
-Original file line number
+Diff line change
@@ Expand Up / @@ -113,4 +113,7 @@ public abstract class AbstractOpenApiSchema { @@
                 return Boolean.FALSE;
             }
         }
+    {{>libraries/jersey2/additional_properties}}
     }