Repository: smithy-lang/smithy
Pull request #1887 (2023-07-27 23:17:59)

Wrap refs with defaults in an allOf
Fixes https://github.com/smithy-lang/smithy/issues/1844.

According to both the [3.1.0 spec](https://spec.openapis.org/oas/latest.html#referenceObject), and the [3.0.2 spec](https://spec.openapis.org/oas/v3.0.2#referenceObject), additional fields on `referenceObjects` are ignored.

This PR adds a JsonSchema configuration option, `enableDefaultValues` which is enabled by default. JsonSchemaShapeVisitor is updated to wrap any members with a `@default` trait applied which target a `$ref` with an `allOf` so that the default can be handled correctly by OpenAPI when this configuration option is enabled.

While JsonSchema's configuration will use this setting by default, it is disabled by default for API Gateway. API Gateway does not support default values anyways, so disabling this option will make `$ref` and top-level definitions consistent by not using defaults in either form.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.


mtdowling: Can you test this to make sure it works with APIGW?
srchase: > Can you test this to make sure it works with APIGW?

Updated the issue description with API Gateway testing.

Patch:
diff --git a/docs/source-2.0/guides/converting-to-openapi.rst b/docs/source-2.0/guides/converting-to-openapi.rst
index 721fe396e..39bad112e 100644
--- a/docs/source-2.0/guides/converting-to-openapi.rst
+++ b/docs/source-2.0/guides/converting-to-openapi.rst
@@ -790,6 +790,89 @@ supportNonNumericFloats (``boolean``)
         }
 
 
+.. _generate-openapi-setting-disableDefaultValues:
+
+disableDefaultValues (``boolean``)
+    Set to true to disable adding default values.
+
+    .. code-block:: json
+
+        {
+            "version": "2.0",
+            "plugins": {
+                "openapi": {
+                    "service": "example.weather#Weather",
+                    "disableDefaultValues": true
+                }
+            }
+        }
+
+    With this disabled, default values will not appear in the output:
+
+    .. code-block:: json
+
+        {
+            "Foo": {
+                "type": "object",
+                "properties": {
+                    "bam": {
+                        "type": "array",
+                        "items": {
+                            "type": "string"
+                        }
+                    },
+                    "bar": {
+                        "type": "number"
+                    },
+                    "bat": {
+                        "$ref": "#/definitions/MyEnum"
+                    },
+                    "baz": {
+                        "type": "string"
+                    }
+                }
+            }
+        }
+
+    With this enabled (the default), default values will be added, with ``$ref``
+    pointers wrapped in an ``allOf``:
+
+    .. code-block:: json
+
+        {
+            "Foo": {
+                "type": "object",
+                "properties": {
+                    "bam": {
+                        "type": "array",
+                        "items": {
+                            "type": "string"
+                        },
+                        "default": []
+                    },
+                    "bar": {
+                        "type": "number",
+                        "default": 0
+                    },
+                    "bat": {
+                        "allOf": [
+                            {
+                                "$ref": "#/definitions/MyEnum"
+                            },
+                            {
+                                "default": "FOO"
+                            }
+                        ]
+                    },
+                    "baz": {
+                        "type": "string",
+                        "default": ""
+                    }
+                }
+            }
+        }
+
+
 ----------------
 Security schemes
 ----------------
@@ -1438,7 +1521,9 @@ Amazon API Gateway limitations
 
 The ``default`` property in OpenAPI is not currently supported by Amazon
 API Gateway. The ``default`` property is automatically removed from OpenAPI
-models when they are generated for Amazon API Gateway.
+models when they are generated for Amazon API Gateway. Additionally, ``default``
+values will not be set on ``$ref`` pointers or wrapped in an ``allOf`` as
+described in :ref:`disableDefaultValues <generate-openapi-setting-disableDefaultValues>`.
 
 
 -------------------------------
diff --git a/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddDefaultConfigSettings.java b/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddDefaultConfigSettings.java
index 4e507f846..4c95190b5 100644
--- a/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddDefaultConfigSettings.java
+++ b/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddDefaultConfigSettings.java
@@ -35,5 +35,6 @@ final class AddDefaultConfigSettings implements ApiGatewayMapper {
     public void updateDefaultSettings(Model model, OpenApiConfig config) {
         config.setAlphanumericOnlyRefs(true);
         config.getDisableFeatures().add("default");
+        config.setDisableDefaultValues(true);
     }
 }
diff --git a/smithy-jsonschema/src/main/java/software/amazon/smithy/jsonschema/JsonSchemaConfig.java b/smithy-jsonschema/src/main/java/software/amazon/smithy/jsonschema/JsonSchemaConfig.java
index 79a0726c2..9c332b809 100644
--- a/smithy-jsonschema/src/main/java/software/amazon/smithy/jsonschema/JsonSchemaConfig.java
+++ b/smithy-jsonschema/src/main/java/software/amazon/smithy/jsonschema/JsonSchemaConfig.java
@@ -112,6 +112,7 @@ public class JsonSchemaConfig {
     private boolean supportNonNumericFloats = false;
     private boolean enableOutOfServiceReferences = false;
     private boolean useIntegerType;
+    private boolean disableDefaultValues = false;
 
     public JsonSchemaConfig() {
         nodeMapper.setWhenMissingSetter(NodeMapper.WhenMissing.IGNORE);
@@ -406,6 +407,20 @@ public class JsonSchemaConfig {
     }
 
 
+    public boolean getDisableDefaultValues() {
+        return disableDefaultValues;
+    }
+
+    /**
+     * Set to true to disable default values on schemas, including wrapping $ref pointers in an `allOf`.
+     *
+     * @param disableDefaultValues True to disable setting default values.
+     */
+    public void setDisableDefaultValues(boolean disableDefaultValues) {
+        this.disableDefaultValues = disableDefaultValues;
+    }
+
+
     /**
      * JSON schema version to use when converting Smithy shapes into Json Schema.
      *
diff --git a/smithy-jsonschema/src/main/java/software/amazon/smithy/jsonschema/JsonSchemaShapeVisitor.java b/smithy-jsonschema/src/main/java/software/amazon/smithy/jsonschema/JsonSchemaShapeVisitor.java
index ac11846b9..05a32b8b2 100644
--- a/smithy-jsonschema/src/main/java/software/amazon/smithy/jsonschema/JsonSchemaShapeVisitor.java
+++ b/smithy-jsonschema/src/main/java/software/amazon/smithy/jsonschema/JsonSchemaShapeVisitor.java
@@ -100,9 +100,13 @@ final class JsonSchemaShapeVisitor extends ShapeVisitor.Default<Schema> {
         if (converter.isInlined(member)) {
             return member.accept(this);
         } else {
-            Schema.Builder builder = Schema.builder().ref(converter.toPointer(member.getTarget()));
-            member.getTrait(DefaultTrait.class).ifPresent(trait -> builder.defaultValue(trait.toNode()));
-            return builder.build();
+            // Wrap the ref and default in an allOf if disableDefaultValues has been not been disabled on config.
+            if (member.hasTrait(DefaultTrait.class) && !converter.getConfig().getDisableDefaultValues()) {
+                Schema ref = Schema.builder().ref(converter.toPointer(member.getTarget())).build();
+                Schema def = Schema.builder().defaultValue(member.expectTrait(DefaultTrait.class).toNode()).build();
+                return Schema.builder().allOf(ListUtils.of(ref, def)).build();
+            }
+            return Schema.builder().ref(converter.toPointer(member.getTarget())).build();
         }
     }
 
@@ -317,8 +321,9 @@ final class JsonSchemaShapeVisitor extends ShapeVisitor.Default<Schema> {
                 .map(EnumTrait::getEnumDefinitionValues)
                 .ifPresent(builder::enumValues);
 
-        shape.getTrait(DefaultTrait.class)
-                .ifPresent(trait -> builder.defaultValue(trait.toNode()));
+        if (shape.hasTrait(DefaultTrait.class) && !converter.getConfig().getDisableDefaultValues()) {
+            builder.defaultValue(shape.expectTrait(DefaultTrait.class).toNode());
+        }
 
         return builder;
     }


Test patch:
diff --git a/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/omits-default-trait.openapi.json b/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/omits-default-trait.openapi.json
index 198eb7c9a..8cdb9e15c 100644
--- a/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/omits-default-trait.openapi.json
+++ b/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/omits-default-trait.openapi.json
@@ -34,6 +34,13 @@
     },
     "components": {
         "schemas": {
+            "DefaultEnum": {
+                "type": "string",
+                "enum": [
+                    "FOO",
+                    "BAR"
+                ]
+            },
             "HasDefaultRequestContent": {
                 "type": "object",
                 "properties": {
@@ -59,6 +66,9 @@
                         "items": {
                             "type": "string"
                         }
+                    },
+                    "baz": {
+                        "$ref": "#/components/schemas/DefaultEnum"
                     }
                 }
             }
diff --git a/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/omits-default-trait.smithy b/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/omits-default-trait.smithy
index 92eb6e59f..cefc45c92 100644
--- a/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/omits-default-trait.smithy
+++ b/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/omits-default-trait.smithy
@@ -2,7 +2,6 @@ $version: "2.0"
 
 namespace example.smithy
 
-use aws.protocols#httpChecksum
 use aws.protocols#restJson1
 
 @restJson1
@@ -20,9 +19,15 @@ operation HasDefault {
     output := {
         foo: String = ""
         bar: StringList = []
+        baz: DefaultEnum = "FOO"
     }
 }
 
 list StringList {
     member: String
 }
+
+enum DefaultEnum {
+    FOO
+    BAR
+}
diff --git a/smithy-jsonschema/src/test/java/software/amazon/smithy/jsonschema/JsonSchemaConverterTest.java b/smithy-jsonschema/src/test/java/software/amazon/smithy/jsonschema/JsonSchemaConverterTest.java
index 5a5266c98..23cf2627f 100644
--- a/smithy-jsonschema/src/test/java/software/amazon/smithy/jsonschema/JsonSchemaConverterTest.java
+++ b/smithy-jsonschema/src/test/java/software/amazon/smithy/jsonschema/JsonSchemaConverterTest.java
@@ -689,7 +689,7 @@ public class JsonSchemaConverterTest {
     }
 
     @Test
-    public void appliesDefaults() {
+    public void appliesDefaultsByDefault() {
         Model model = Model.assembler()
                 .addImport(getClass().getResource("default-values.smithy"))
                 .assemble()
@@ -703,4 +703,23 @@ public class JsonSchemaConverterTest {
                 IoUtils.toUtf8String(getClass().getResourceAsStream("default-values.jsonschema.v07.json")));
         Node.assertEquals(document.toNode(), expected);
     }
+
+    @Test
+    public void defaultsCanBeDisabled() {
+        Model model = Model.assembler()
+                .addImport(getClass().getResource("default-values.smithy"))
+                .assemble()
+                .unwrap();
+        JsonSchemaConfig config = new JsonSchemaConfig();
+        config.setDisableDefaultValues(true);
+        SchemaDocument document = JsonSchemaConverter.builder()
+                .config(config)
+                .model(model)
+                .build()
+                .convert();
+
+        Node expected = Node.parse(
+                IoUtils.toUtf8String(getClass().getResourceAsStream("default-values-disabled.jsonschema.v07.json")));
+        Node.assertEquals(document.toNode(), expected);
+    }
 }
diff --git a/smithy-jsonschema/src/test/resources/software/amazon/smithy/jsonschema/default-values-disabled.jsonschema.v07.json b/smithy-jsonschema/src/test/resources/software/amazon/smithy/jsonschema/default-values-disabled.jsonschema.v07.json
new file mode 100644
index 000000000..434826c38
--- /dev/null
+++ b/smithy-jsonschema/src/test/resources/software/amazon/smithy/jsonschema/default-values-disabled.jsonschema.v07.json
@@ -0,0 +1,31 @@
+{
+    "definitions": {
+        "Foo": {
+            "type": "object",
+            "properties": {
+                "bam": {
+                    "type": "array",
+                    "items": {
+                        "type": "string"
+                    }
+                },
+                "bar": {
+                    "type": "number"
+                },
+                "bat": {
+                    "$ref": "#/definitions/TestEnum"
+                },
+                "baz": {
+                    "type": "string"
+                }
+            }
+        },
+        "TestEnum": {
+            "type": "string",
+            "enum": [
+                "FOO",
+                "BAR"
+            ]
+        }
+    }
+}
diff --git a/smithy-jsonschema/src/test/resources/software/amazon/smithy/jsonschema/default-values.jsonschema.v07.json b/smithy-jsonschema/src/test/resources/software/amazon/smithy/jsonschema/default-values.jsonschema.v07.json
index 98d2d6e8f..af1e66b3c 100644
--- a/smithy-jsonschema/src/test/resources/software/amazon/smithy/jsonschema/default-values.jsonschema.v07.json
+++ b/smithy-jsonschema/src/test/resources/software/amazon/smithy/jsonschema/default-values.jsonschema.v07.json
@@ -14,11 +14,28 @@
                     "type": "number",
                     "default": 0
                 },
+                "bat": {
+                    "allOf": [
+                        {
+                            "$ref": "#/definitions/TestEnum"
+                        },
+                        {
+                            "default": "FOO"
+                        }
+                    ]
+                },
                 "baz": {
                     "type": "string",
                     "default": ""
                 }
             }
+        },
+        "TestEnum": {
+            "type": "string",
+            "enum": [
+                "FOO",
+                "BAR"
+            ]
         }
     }
 }
diff --git a/smithy-jsonschema/src/test/resources/software/amazon/smithy/jsonschema/default-values.smithy b/smithy-jsonschema/src/test/resources/software/amazon/smithy/jsonschema/default-values.smithy
index cc44a5e32..31607ba6f 100644
--- a/smithy-jsonschema/src/test/resources/software/amazon/smithy/jsonschema/default-values.smithy
+++ b/smithy-jsonschema/src/test/resources/software/amazon/smithy/jsonschema/default-values.smithy
@@ -5,9 +5,15 @@ namespace smithy.example
 structure Foo {
     bar: Integer = 0
     baz: String = ""
-    bam: StringList = []
+    bam: StringList = [],
+    bat: TestEnum = "FOO"
 }
 
 list StringList {
     member: String
 }
+
+enum TestEnum {
+    FOO
+    BAR
+}