From 56bc3c6ea281da12d61d0dd95bd8fe9836ee6e06 Mon Sep 17 00:00:00 2001 From: Hayden Baker Date: Tue, 12 Nov 2024 13:05:10 -0800 Subject: [PATCH] address comments --- .../cloudformation/schema/model/Property.java | 33 ++++++------- .../schema/model/ResourceSchema.java | 5 ++ .../schema/fromsmithy/ResourceSchemaTest.java | 8 ++-- ...ain.json => aws-sagemaker-domain.cfn.json} | 26 +++++----- .../amazon/smithy/model/node/NodeSorter.java | 48 +++++++++++++++++++ 5 files changed, 83 insertions(+), 37 deletions(-) rename smithy-aws-cloudformation/src/test/resources/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/{aws-sagemaker-domain.json => aws-sagemaker-domain.cfn.json} (98%) create mode 100644 smithy-model/src/main/java/software/amazon/smithy/model/node/NodeSorter.java diff --git a/smithy-aws-cloudformation/src/main/java/software/amazon/smithy/aws/cloudformation/schema/model/Property.java b/smithy-aws-cloudformation/src/main/java/software/amazon/smithy/aws/cloudformation/schema/model/Property.java index 947ce3ef0ef..e95d190054e 100644 --- a/smithy-aws-cloudformation/src/main/java/software/amazon/smithy/aws/cloudformation/schema/model/Property.java +++ b/smithy-aws-cloudformation/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 Resource Type Properties JSON Schema */ public final class Property implements ToNode, ToSmithyBuilder { - private final boolean insertionOrder; - private final List 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 insertionOrder = schema.getExtension("insertionOrder") + .map(n -> n.toNode().expectBooleanNode().getValue()); + + return insertionOrder.orElse(false); } public List getDependencies() { - return dependencies; + Optional> dependencies = schema.getExtension("dependencies") + .map(n -> n.toNode().expectArrayNode().getElementsAs(StringNode::getValue)); + + return dependencies.orElse(Collections.emptyList()); } public Schema getSchema() { diff --git a/smithy-aws-cloudformation/src/main/java/software/amazon/smithy/aws/cloudformation/schema/model/ResourceSchema.java b/smithy-aws-cloudformation/src/main/java/software/amazon/smithy/aws/cloudformation/schema/model/ResourceSchema.java index 7ae658ca61c..5268dfb98f9 100644 --- a/smithy-aws-cloudformation/src/main/java/software/amazon/smithy/aws/cloudformation/schema/model/ResourceSchema.java +++ b/smithy-aws-cloudformation/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/smithy-aws-cloudformation/src/test/java/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/ResourceSchemaTest.java b/smithy-aws-cloudformation/src/test/java/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/ResourceSchemaTest.java index 8c87f7b3f46..ce5259be83c 100644 --- a/smithy-aws-cloudformation/src/test/java/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/ResourceSchemaTest.java +++ b/smithy-aws-cloudformation/src/test/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 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/smithy-aws-cloudformation/src/test/resources/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/aws-sagemaker-domain.json b/smithy-aws-cloudformation/src/test/resources/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/aws-sagemaker-domain.cfn.json similarity index 98% rename from smithy-aws-cloudformation/src/test/resources/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/aws-sagemaker-domain.json rename to smithy-aws-cloudformation/src/test/resources/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/aws-sagemaker-domain.cfn.json index 9581e2d3d3a..cdb1e347fc9 100644 --- a/smithy-aws-cloudformation/src/test/resources/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/aws-sagemaker-domain.json +++ b/smithy-aws-cloudformation/src/test/resources/software/amazon/smithy/aws/cloudformation/schema/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 new file mode 100644 index 00000000000..b6282d618f2 --- /dev/null +++ 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 { + private final Comparator comparator; + + NodeSorter(Comparator comparator) { + this.comparator = comparator; + } + + @Override + protected Node getDefault(Node node) { + return node; + } + + @Override + public Node objectNode(ObjectNode node) { + Map 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())); + } +}