Correct the handling of additionalProperties in OpenAPI (2.x)

openapi/src/main/java/io/helidon/openapi/ExpandedTypeDescription.java

@@ -34,13 +34,15 @@
 import org.yaml.snakeyaml.introspector.PropertyUtils;
 import org.yaml.snakeyaml.nodes.Node;
 import org.yaml.snakeyaml.nodes.ScalarNode;
+import org.yaml.snakeyaml.nodes.Tag;
 
 /**
  * Extension of {@link TypeDescription} that handles:
  * <ul>
  *     <li>nested enums,</li>
- *     <li>extensible types, and</li>
- *     <li>references.</li>
+ *     <li>extensible types,</li>
+ *     <li>references, and</li>
+ *     <li>additional properties (which can be either Boolean or Schema).</li>
  * </ul>
  * <p>
  *     The OpenAPI document format uses lower-case enum names and values, while the SmallRye
@@ -60,14 +62,22 @@
  *     description simplifies defining the {@code $ref} property to those types that support it.
  * </p>
  * <p>
+ *     In schemas, the {@code additionalProperties} value can be either a boolean or a schema. The MicroProfile
+ *     {@link org.eclipse.microprofile.openapi.models.media.Schema} type exposes {@code getAdditionalPropertiesBoolean},
+ *     {@code setAdditionalPropertiesBoolean}, {@code getAdditionalPropertiesSchema}, and {@code setAdditionalPropertiesSchema}
+ *     methods. We do not know until runtime and the value is available for each {@code additionalProperties} instance which
+ *     type (Boolean or Schema) to use, so we cannot just prepare a smart SnakeYAML {@code Property} implementation. Instead
+ *     we augment the schema-specific {@code TypeDescription} so it knows how to decide, at runtime, what to do.
+ * </p>
+ * <p>
  *     We use this expanded version of {@code TypeDescription} with the generated SnakeYAMLParserHelper class.
  * </p>
  */
 class ExpandedTypeDescription extends TypeDescription {
 
     private static final String EXTENSION_PROPERTY_PREFIX = "x-";
 
-    private static final PropertyUtils PROPERTY_UTILS = new PropertyUtils();
+    static final PropertyUtils PROPERTY_UTILS = new PropertyUtils();
 
     private Class<?> impl;
 
@@ -221,53 +231,42 @@ private static boolean isRef(String name) {
      * Specific type description for {@code Schema}.
      * <p>
      *     The {@code Schema} node allows the {@code additionalProperties} subnode to be either
-     *     {@code Boolean} or another {@code Schema}. This type description provides a customized
-     *     property description for {@code additionalProperties} that infers which variant a
-     *     specific node in the document actually uses and then processes it accordingly.
+     *     {@code Boolean} or another {@code Schema}, and the {@code Schema} class exposes getters and setters for all of
+     *     {@code additionalProperties}, {@code additionalPropertiesBoolean}, and {@code additionalPropertiesSchema}.
+     *     This type description customizes the handling of {@code additionalProperties} to account for all that.
      * </p>
+     * @see io.helidon.openapi.Serializer (specifically doRepresentJavaBeanProperty) for output handling for additionalProperties
      */
     static final class SchemaTypeDescription extends ExpandedTypeDescription {
 
-        private static final PropertyDescriptor ADDL_PROPS_PROP_DESCRIPTOR = preparePropertyDescriptor();
-
-        private static final Property ADDL_PROPS_PROPERTY =
-                new MethodProperty(ADDL_PROPS_PROP_DESCRIPTOR) {
-
-                    @Override
-                    public void set(Object object, Object value) throws Exception {
-                        Schema s = Schema.class.cast(object);
-                        if (value instanceof Schema) {
-                            s.setAdditionalPropertiesSchema((Schema) value);
-                        } else {
-                            s.setAdditionalPropertiesBoolean((Boolean) value);
-                        }
-                    }
-
-                    @Override
-                    public Object get(Object object) {
-                        Schema s = Schema.class.cast(object);
-                        Boolean b = s.getAdditionalPropertiesBoolean();
-                        return b != null ? b : s.getAdditionalPropertiesSchema();
-                    }
-                };
-
-        private static PropertyDescriptor preparePropertyDescriptor() {
-            try {
-                return new PropertyDescriptor("additionalProperties",
-                        Schema.class.getMethod("getAdditionalPropertiesSchema"),
-                        Schema.class.getMethod("setAdditionalPropertiesSchema", Schema.class));
-            } catch (IntrospectionException | NoSuchMethodException e) {
-                throw new RuntimeException(e);
+        @Override
+        public boolean setupPropertyType(String key, Node valueNode) {
+            if (key.equals("additionalProperties")) {
+                valueNode.setType(valueNode.getTag().equals(Tag.BOOL) ? Boolean.class : Schema.class);
+                return true;
             }
+            return super.setupPropertyType(key, valueNode);
         }
 
-        private SchemaTypeDescription(Class<? extends Object> clazz, Class<?> impl) {
-            super(clazz, impl);
+        @Override
+        public boolean setProperty(Object targetBean, String propertyName, Object value) throws Exception {
+            if (!(targetBean instanceof Schema) || !propertyName.equals("additionalProperties")) {
+                return super.setProperty(targetBean, propertyName, value);
+            }
+            Schema schema = (Schema) targetBean;
+            if (value instanceof Boolean) {
+                schema.setAdditionalPropertiesBoolean((Boolean) value);
+            } else if (value instanceof Schema) {
+                schema.setAdditionalPropertiesSchema((Schema) value);
+            } else {
+                throw new IllegalArgumentException("Expected additionalProperties as Boolean or Schema but was "
+                                                           + value.getClass().getName());
+            }
+            return true;
         }
 
-        @Override
-        public Property getProperty(String name) {
-            return name.equals("additionalProperties") ? ADDL_PROPS_PROPERTY : super.getProperty(name);
+        private SchemaTypeDescription(Class<? extends Object> clazz, Class<?> impl) {
+            super(clazz, impl);
         }
     }
 

openapi/src/main/java/io/helidon/openapi/Serializer.java

@@ -32,6 +32,7 @@
 import org.eclipse.microprofile.openapi.models.Extensible;
 import org.eclipse.microprofile.openapi.models.OpenAPI;
 import org.eclipse.microprofile.openapi.models.Reference;
+import org.eclipse.microprofile.openapi.models.media.Schema;
 import org.eclipse.microprofile.openapi.models.parameters.Parameter;
 import org.yaml.snakeyaml.DumperOptions;
 import org.yaml.snakeyaml.Yaml;
@@ -156,9 +157,23 @@ protected NodeTuple representJavaBeanProperty(Object javaBean, Property property
 
         private NodeTuple doRepresentJavaBeanProperty(Object javaBean, Property property, Object propertyValue, Tag customTag) {
             NodeTuple defaultTuple = super.representJavaBeanProperty(javaBean, property, propertyValue, customTag);
-            return (javaBean instanceof Reference) && property.getName().equals("ref")
-                    ? new NodeTuple(representData("$ref"), defaultTuple.getValueNode())
-                    : defaultTuple;
+            if ((javaBean instanceof Reference) && property.getName().equals("ref")) {
+                return new NodeTuple(representData("$ref"), defaultTuple.getValueNode());
+            }
+            if (javaBean instanceof Schema) {
+                /*
+                 * At most one of additionalPropertiesBoolean and additionalPropertiesSchema will return a non-null value.
+                 * Whichever one does (if either), replace the name with "additionalProperties" for output. Skip whatever is
+                 * returned from the deprecated additionalProperties method itself.
+                 */
+                String propertyName = property.getName();
+                if (propertyName.equals("additionalProperties")) {
+                    return null;
+                } else if (propertyName.startsWith("additionalProperties")) {
+                    return new NodeTuple(representData("additionalProperties"), defaultTuple.getValueNode());
+                }
+            }
+            return defaultTuple;
         }
 
         private Object adjustPropertyValue(Object propertyValue) {

openapi/src/test/java/io/helidon/openapi/TestAdditionalProperties.java

@@ -0,0 +1,123 @@
+/*
+ * Copyright (c) 2021 Oracle and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.helidon.openapi;
+
+
+import java.io.IOException;
+import java.io.StringWriter;
+import java.util.Map;
+
+import io.smallrye.openapi.runtime.io.Format;
+import org.eclipse.microprofile.openapi.models.OpenAPI;
+import org.eclipse.microprofile.openapi.models.media.Schema;
+import org.junit.jupiter.api.Test;
+import org.yaml.snakeyaml.Yaml;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.hasKey;
+import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.notNullValue;
+import static org.hamcrest.Matchers.nullValue;
+
+class TestAdditionalProperties {
+
+    private static SnakeYAMLParserHelper<ExpandedTypeDescription> helper = OpenAPISupport.helper();
+
+
+    @Test
+    void checkParsingBooleanAdditionalProperties() throws IOException {
+        OpenAPI openAPI = ParserTest.parse(helper, "/withBooleanAddlProps.yml", OpenAPISupport.OpenAPIMediaType.YAML);
+        Schema itemSchema = openAPI.getComponents().getSchemas().get("item");
+
+        Schema additionalPropertiesSchema = itemSchema.getAdditionalPropertiesSchema();
+        Boolean additionalPropertiesBoolean = itemSchema.getAdditionalPropertiesBoolean();
+
+        assertThat("Additional properties as schema", additionalPropertiesSchema, is(nullValue()));
+        assertThat("Additional properties as boolean", additionalPropertiesBoolean, is(notNullValue()));
+        assertThat("Additional properties value", additionalPropertiesBoolean.booleanValue(), is(false));
+    }
+
+    @Test
+    void checkParsingSchemaAdditionalProperties() throws IOException {
+        OpenAPI openAPI = ParserTest.parse(helper, "/withSchemaAddlProps.yml", OpenAPISupport.OpenAPIMediaType.YAML);
+        Schema itemSchema = openAPI.getComponents().getSchemas().get("item");
+
+        Schema additionalPropertiesSchema = itemSchema.getAdditionalPropertiesSchema();
+        Boolean additionalPropertiesBoolean = itemSchema.getAdditionalPropertiesBoolean();
+
+        assertThat("Additional properties as boolean", additionalPropertiesBoolean, is(nullValue()));
+        assertThat("Additional properties as schema", additionalPropertiesSchema, is(notNullValue()));
+
+        Map<String, Schema> additionalProperties = additionalPropertiesSchema.getProperties();
+        assertThat("Additional property 'code'", additionalProperties, hasKey("code"));
+        assertThat("Additional property 'text'", additionalProperties, hasKey("text"));
+    }
+
+    @Test
+    void checkWritingSchemaAdditionalProperties() throws IOException {
+        OpenAPI openAPI = ParserTest.parse(helper, "/withSchemaAddlProps.yml", OpenAPISupport.OpenAPIMediaType.YAML);
+        String document = formatModel(openAPI);
+
+        /*
+         * Expected output (although the
+               additionalProperties:
+        type: object
+        properties:
+          code:
+            type: integer
+          text:
+            type: string
+         */
+        Yaml yaml = new Yaml();
+        Map<String, Object> model = yaml.load(document);
+        Map<String, ?> item = asMap(model, "components", "schemas", "item");
+
+        Object additionalProperties = item.get("additionalProperties");
+
+        assertThat("Additional properties node type", additionalProperties, is(instanceOf(Map.class)));
+
+    }
+
+    private static Map<String, ?> asMap(Map<String, ?> map, String... keys) {
+        Map<String, ?> m = map;
+        for (String key : keys) {
+            m = (Map<String, ?>) m.get(key);
+        }
+        return m;
+    }
+
+    @Test
+    void checkWritingBooleanAdditionalProperties() throws IOException {
+        OpenAPI openAPI = ParserTest.parse(helper, "/withBooleanAddlProps.yml", OpenAPISupport.OpenAPIMediaType.YAML);
+        String document = formatModel(openAPI);
+
+        /*
+         * Expected output: additionalProperties: false
+         */
+
+        assertThat("Formatted OpenAPI document matches expected pattern",
+                   document, containsString("additionalProperties: false"));
+    }
+
+    private String formatModel(OpenAPI model) {
+        StringWriter sw = new StringWriter();
+        Map<Class<?>, ExpandedTypeDescription> implsToTypes = OpenAPISupport.buildImplsToTypes(helper);
+        Serializer.serialize(helper.types(), implsToTypes, model, Format.YAML, sw);
+        return sw.toString();
+    }
+}

openapi/src/test/resources/withBooleanAddlProps.yml

@@ -0,0 +1,45 @@
+#
+# Copyright (c) 2021 Oracle and/or its affiliates.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+openapi: 3.1.0
+
+info:
+  title: Some service
+  version: 0.1.0
+
+components:
+  schemas:
+    item:
+      type: object
+      additionalProperties: false
+      properties:
+        id:
+          type: string
+        title:
+          type: string
+
+paths:
+  /items:
+    get:
+      responses:
+        '200':
+          description: Get items
+          content:
+            application/json:
+              schema:
+                type: array
+                items:
+                  $ref: '#/components/schemas/item'

openapi/src/test/resources/withSchemaAddlProps.yml

@@ -0,0 +1,51 @@
+#
+# Copyright (c) 2021 Oracle and/or its affiliates.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+openapi: 3.1.0
+
+info:
+  title: Some service
+  version: 0.1.0
+
+components:
+  schemas:
+    item:
+      type: object
+      additionalProperties:
+        type: object
+        properties:
+          code:
+            type: integer
+          text:
+            type: string
+      properties:
+        id:
+          type: string
+        title:
+          type: string
+
+paths:
+  /items:
+    get:
+      responses:
+        '200':
+          description: Get items
+          content:
+            application/json:
+              schema:
+                type: array
+                items:
+                  $ref: '#/components/schemas/item'