address comments

smithy-lang · Nov 13, 2024 · 56bc3c6 · 56bc3c6
1 parent 5d5f317
commit 56bc3c6
Show file tree

Hide file tree

Showing 5 changed files with 83 additions and 37 deletions.
diff --git a/...mation/src/main/java/software/amazon/smithy/aws/cloudformation/schema/model/Property.java b/...mation/src/main/java/software/amazon/smithy/aws/cloudformation/schema/model/Property.java
@@ -15,8 +15,8 @@
 
 package software.amazon.smithy.aws.cloudformation.schema.model;
 
-import java.util.ArrayList;
-import java.util.List;
+import java.util.*;
+
 import software.amazon.smithy.jsonschema.Schema;
 import software.amazon.smithy.model.node.Node;
 import software.amazon.smithy.model.node.ObjectNode;
@@ -32,8 +32,6 @@
  * @see <a href="https://github.com/aws-cloudformation/cloudformation-cli/blob/master/src/rpdk/core/data/schema/provider.definition.schema.v1.jsonL74">Resource Type Properties JSON Schema</a>
  */
 public final class Property implements ToNode, ToSmithyBuilder<Property> {
-    private final boolean insertionOrder;
-    private final List<String> dependencies;
     private final Schema schema;
     // Other reserved property names in definition but not in the validation
     // JSON Schema, so not defined in code:
@@ -49,14 +47,12 @@ private Property(Builder builder) {
             schemaBuilder = builder.schema.toBuilder();
         }
 
-        this.insertionOrder = builder.insertionOrder;
-        if (this.insertionOrder) {
+        if (builder.insertionOrder) {
             schemaBuilder.putExtension("insertionOrder", Node.from(true));
         }
 
-        this.dependencies = builder.dependencies;
-        if (!this.dependencies.isEmpty()) {
-            schemaBuilder.putExtension("dependencies", Node.fromStrings(this.dependencies));
+        if (!builder.dependencies.isEmpty()) {
+            schemaBuilder.putExtension("dependencies", Node.fromStrings(builder.dependencies));
         }
 
         this.schema = schemaBuilder.build();
@@ -69,19 +65,12 @@ public Node toNode() {
 
     @Override
     public Builder toBuilder() {
-        return builder()
-                .insertionOrder(insertionOrder)
-                .dependencies(dependencies)
-                .schema(schema);
+        return builder().schema(schema);
     }
 
     public static Property fromNode(Node node) {
         ObjectNode objectNode = node.expectObjectNode();
         Builder builder = builder();
-
-        objectNode.getBooleanMember("insertionOrder", builder::insertionOrder);
-        objectNode.getArrayMember("dependencies", StringNode::getValue, builder::dependencies);
-
         builder.schema(Schema.fromNode(objectNode));
 
         return builder.build();
@@ -96,11 +85,17 @@ public static Builder builder() {
     }
 
     public boolean isInsertionOrder() {
-        return insertionOrder;
+        Optional<Boolean> insertionOrder = schema.getExtension("insertionOrder")
+                .map(n -> n.toNode().expectBooleanNode().getValue());
+
+        return insertionOrder.orElse(false);
     }
 
     public List<String> getDependencies() {
-        return dependencies;
+        Optional<List<String>> dependencies = schema.getExtension("dependencies")
+                .map(n -> n.toNode().expectArrayNode().getElementsAs(StringNode::getValue));
+
+        return dependencies.orElse(Collections.emptyList());
     }
 
     public Schema getSchema() {

diff --git a/.../src/main/java/software/amazon/smithy/aws/cloudformation/schema/model/ResourceSchema.java b/.../src/main/java/software/amazon/smithy/aws/cloudformation/schema/model/ResourceSchema.java
@@ -166,6 +166,11 @@ public Builder toBuilder() {
                 .tagging(tagging);
     }
 
+    public static ResourceSchema fromNode(Node node) {
+        NodeMapper mapper = new NodeMapper();
+        return mapper.deserializeInto(node, ResourceSchema.builder()).build();
+    }
+
     public static Builder builder() {
         return new Builder();
     }

diff --git a/.../java/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/ResourceSchemaTest.java b/.../java/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/ResourceSchemaTest.java
@@ -19,7 +19,6 @@
 import org.junit.jupiter.params.provider.MethodSource;
 import software.amazon.smithy.aws.cloudformation.schema.model.ResourceSchema;
 import software.amazon.smithy.model.node.Node;
-import software.amazon.smithy.model.node.NodeMapper;
 import software.amazon.smithy.utils.IoUtils;
 
 import java.io.IOException;
@@ -35,19 +34,18 @@ public class ResourceSchemaTest {
     @ParameterizedTest
     @MethodSource("resourceSchemaFiles")
     public void validateResourceSchemaFromNodeToNode(String resourceSchemaFile) {
-        NodeMapper mapper = new NodeMapper();
         String json = IoUtils.readUtf8File(resourceSchemaFile);
 
         Node node = Node.parse(json);
-        ResourceSchema schemaFromNode = mapper.deserialize(node, ResourceSchema.class);
+        ResourceSchema schemaFromNode = ResourceSchema.fromNode(node);
         Node nodeFromSchema = schemaFromNode.toNode();
 
-        Node.assertEquals(nodeFromSchema.withDeepSortedKeys(), node.withDeepSortedKeys());
+        Node.assertEquals(nodeFromSchema, node);
     }
 
     public static List<String> resourceSchemaFiles() {
         try {
-            Path definitionPath = Paths.get(ResourceSchemaTest.class.getResource("aws-sagemaker-domain.json").toURI());
+            Path definitionPath = Paths.get(ResourceSchemaTest.class.getResource("aws-sagemaker-domain.cfn.json").toURI());
 
             return Files.walk(Paths.get(definitionPath.getParent().toUri()))
                     .filter(Files::isRegularFile)

diff --git a/...hema/fromsmithy/aws-sagemaker-domain.json → .../fromsmithy/aws-sagemaker-domain.cfn.json b/...hema/fromsmithy/aws-sagemaker-domain.json → .../fromsmithy/aws-sagemaker-domain.cfn.json
@@ -499,8 +499,8 @@
     "/properties/DomainSettings/RStudioServerProDomainSettings/DefaultResourceSpec",
     "/properties/KmsKeyId",
     "/properties/SubnetIds",
-    "/properties/VpcId",
-    "/properties/Tags"
+    "/properties/Tags",
+    "/properties/VpcId"
   ],
   "writeOnlyProperties": [
     "/properties/Tags"
@@ -510,27 +510,27 @@
   ],
   "readOnlyProperties": [
     "/properties/DomainArn",
-    "/properties/Url",
     "/properties/DomainId",
     "/properties/HomeEfsFileSystemId",
     "/properties/SecurityGroupIdForDomainBoundary",
-    "/properties/SingleSignOnManagedApplicationInstanceId"
+    "/properties/SingleSignOnManagedApplicationInstanceId",
+    "/properties/Url"
   ],
   "handlers": {
     "create": {
       "permissions": [
-        "sagemaker:CreateApp",
-        "sagemaker:CreateDomain",
-        "sagemaker:DescribeDomain",
-        "sagemaker:DescribeImage",
-        "sagemaker:DescribeImageVersion",
+        "efs:CreateFileSystem",
         "iam:CreateServiceLinkedRole",
         "iam:PassRole",
-        "efs:CreateFileSystem",
         "kms:CreateGrant",
         "kms:Decrypt",
         "kms:DescribeKey",
-        "kms:GenerateDataKeyWithoutPlainText"
+        "kms:GenerateDataKeyWithoutPlainText",
+        "sagemaker:CreateApp",
+        "sagemaker:CreateDomain",
+        "sagemaker:DescribeDomain",
+        "sagemaker:DescribeImage",
+        "sagemaker:DescribeImageVersion"
       ]
     },
     "read": {
@@ -540,12 +540,12 @@
     },
     "update": {
       "permissions": [
+        "iam:PassRole",
         "sagemaker:CreateApp",
-        "sagemaker:UpdateDomain",
         "sagemaker:DescribeDomain",
         "sagemaker:DescribeImage",
         "sagemaker:DescribeImageVersion",
-        "iam:PassRole"
+        "sagemaker:UpdateDomain"
       ]
     },
     "delete": {

diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/node/NodeSorter.java b/smithy-model/src/main/java/software/amazon/smithy/model/node/NodeSorter.java
@@ -0,0 +1,48 @@
+/*
+ * Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License").
+ * You may not use this file except in compliance with the License.
+ * A copy of the License is located at
+ *
+ *  http://aws.amazon.com/apache2.0
+ *
+ * or in the "license" file accompanying this file. This file 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 software.amazon.smithy.model.node;
+
+import java.util.Comparator;
+import java.util.Map;
+import java.util.TreeMap;
+
+class NodeSorter extends NodeVisitor.Default<Node> {
+    private final Comparator<Node> comparator;
+
+    NodeSorter(Comparator<Node> comparator) {
+        this.comparator = comparator;
+    }
+
+    @Override
+    protected Node getDefault(Node node) {
+        return node;
+    }
+
+    @Override
+    public Node objectNode(ObjectNode node) {
+        Map<StringNode, Node> members = new TreeMap<>(comparator);
+        node.getMembers().forEach((k, v) -> members.put(k, v.accept(this)));
+        return new ObjectNode(members, node.getSourceLocation());
+    }
+
+    @Override
+    public Node arrayNode(ArrayNode node) {
+        return node.getElements().stream()
+                .map(v -> v.accept(this))
+                .sorted(comparator)
+                .collect(ArrayNode.collect(node.getSourceLocation()));
+    }
+}