Fix bug with cloudformation unions

This fixes a bug where having unions in your model would cause the cloudformation schema generation to fail.
smithy-lang · May 11, 2021 · d19b8e0 · d19b8e0
1 parent c6e54b9
commit d19b8e0
Showing 4 changed files with 104 additions and 6 deletions.
diff --git a/...mazon/smithy/aws/cloudformation/schema/fromsmithy/mappers/AdditionalPropertiesMapper.java b/...mazon/smithy/aws/cloudformation/schema/fromsmithy/mappers/AdditionalPropertiesMapper.java
@@ -15,12 +15,15 @@
 
 package software.amazon.smithy.aws.cloudformation.schema.fromsmithy.mappers;
 
+import java.util.ArrayList;
 import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import software.amazon.smithy.aws.cloudformation.schema.fromsmithy.CfnMapper;
 import software.amazon.smithy.aws.cloudformation.schema.fromsmithy.Context;
 import software.amazon.smithy.aws.cloudformation.schema.model.ResourceSchema;
+import software.amazon.smithy.model.node.ArrayNode;
 import software.amazon.smithy.model.node.Node;
 import software.amazon.smithy.model.node.ObjectNode;
 import software.amazon.smithy.model.node.StringNode;
@@ -54,15 +57,31 @@ public ObjectNode updateNode(Context context, ResourceSchema resourceSchema, Obj
         if (definitionsOptional.isPresent()) {
             for (Map.Entry<StringNode, Node> entry : definitionsOptional.get().getMembers().entrySet()) {
                 ObjectNode valueNode = entry.getValue().expectObjectNode();
-
-                if (valueNode.expectStringMember("type").getValue().equals("object")) {
-                    valueNode = valueNode.withMember("additionalProperties", false);
-                }
-
-                updatedNodes.put(entry.getKey(), valueNode);
+                updatedNodes.put(entry.getKey(), addAdditionalProperties(valueNode));
             }
             node = node.withMember("definitions", Node.objectNode(updatedNodes));
         }
         return node;
     }
+
+    private Node addAdditionalProperties(Node node) {
+        if (!node.isObjectNode()) {
+            return node;
+        }
+        ObjectNode valueNode = node.expectObjectNode();
+
+        // Unions may be expressed as a "oneOf" which won't have a type.
+        Optional<String> type = valueNode.getStringMember("type").map(StringNode::getValue);
+        if (!type.isPresent() && valueNode.getArrayMember("oneOf").isPresent()) {
+            List<Node> elements = valueNode.expectArrayMember("oneOf").getElements();
+            List<Node> updatedElements = new ArrayList<>(elements.size());
+            for (Node element: elements) {
+                updatedElements.add(addAdditionalProperties(element));
+            }
+            valueNode = valueNode.withMember("oneOf", ArrayNode.fromNodes(updatedElements));
+        } else if (type.isPresent() && type.get().equals("object")) {
+            valueNode = valueNode.withMember("additionalProperties", false);
+        }
+        return valueNode;
+    }
 }
diff --git a/...ware/amazon/smithy/aws/cloudformation/schema/fromsmithy/disable-caps-fooresource.cfn.json b/...ware/amazon/smithy/aws/cloudformation/schema/fromsmithy/disable-caps-fooresource.cfn.json
@@ -14,6 +14,36 @@
       },
       "additionalProperties": false
     },
+    "ConditionalProperty": {
+      "oneOf": [
+        {
+          "type": "object",
+          "title": "optionOne",
+          "properties": {
+            "optionOne": {
+              "type": "string"
+            }
+          },
+          "required": [
+            "optionOne"
+          ],
+          "additionalProperties": false
+        },
+        {
+          "type": "object",
+          "title": "optionTwo",
+          "properties": {
+            "optionTwo": {
+              "type": "string"
+            }
+          },
+          "required": [
+            "optionTwo"
+          ],
+          "additionalProperties": false
+        }
+      ]
+    },
     "FooMap": {
       "type": "object",
       "patternProperties": {
@@ -25,6 +55,9 @@
     }
   },
   "properties": {
+    "conditionalProperty": {
+      "$ref": "#/definitions/ConditionalProperty"
+    },
     "fooId": {
       "type": "string"
     },

diff --git a/...mazon/smithy/aws/cloudformation/schema/fromsmithy/smithy-testservice-fooresource.cfn.json b/...mazon/smithy/aws/cloudformation/schema/fromsmithy/smithy-testservice-fooresource.cfn.json
@@ -14,6 +14,36 @@
       },
       "additionalProperties": false
     },
+    "ConditionalProperty": {
+      "oneOf": [
+        {
+          "type": "object",
+          "title": "OptionOne",
+          "properties": {
+            "OptionOne": {
+              "type": "string"
+            }
+          },
+          "required": [
+            "OptionOne"
+          ],
+          "additionalProperties": false
+        },
+        {
+          "type": "object",
+          "title": "OptionTwo",
+          "properties": {
+            "OptionTwo": {
+              "type": "string"
+            }
+          },
+          "required": [
+            "OptionTwo"
+          ],
+          "additionalProperties": false
+        }
+      ]
+    },
     "FooMap": {
       "type": "object",
       "patternProperties": {
@@ -25,6 +55,9 @@
     }
   },
   "properties": {
+    "ConditionalProperty": {
+      "$ref": "#/definitions/ConditionalProperty"
+    },
     "FooId": {
       "type": "string"
     },

diff --git a/...resources/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/test-service.smithy b/...resources/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/test-service.smithy
@@ -37,6 +37,8 @@ structure CreateFooRequest {
     fooValidCreateReadProperty: String,
 
     fooValidFullyMutableProperty: ComplexProperty,
+
+    conditionalProperty: ConditionalProperty,
 }
 
 structure CreateFooResponse {
@@ -62,6 +64,8 @@ structure GetFooResponse {
     fooValidCreateReadProperty: String,
 
     fooValidFullyMutableProperty: ComplexProperty,
+
+    conditionalProperty: ConditionalProperty,
 }
 
 operation UpdateFooOperation {
@@ -77,6 +81,8 @@ structure UpdateFooRequest {
     fooValidWriteProperty: String,
 
     fooValidFullyMutableProperty: ComplexProperty,
+
+    conditionalProperty: ConditionalProperty,
 }
 
 structure UpdateFooResponse {
@@ -85,6 +91,8 @@ structure UpdateFooResponse {
     fooValidReadProperty: String,
 
     fooValidFullyMutableProperty: ComplexProperty,
+
+    conditionalProperty: ConditionalProperty,
 }
 
 /// A Bar resource, not that kind of bar though.
@@ -239,6 +247,11 @@ structure ComplexProperty {
     another: String,
 }
 
+union ConditionalProperty {
+    optionOne: String,
+    optionTwo: String,
+}
+
 map FooMap {
     key: String,
     value: String