From 28620e8fcd6978c2a75543fb3fd567946ef8061f Mon Sep 17 00:00:00 2001 From: Hayden Baker Date: Mon, 24 Oct 2022 11:50:36 -0700 Subject: [PATCH 1/2] Fix cfn-mutability for inherited identifiers For resources with parent resources, identifiers that are inherited should be marked with create-and-read mutability instead of the defaults for that resource. This fixes that case, and adjusts the test cases to verify the correct behavior --- .../traits/CfnResourceIndex.java | 28 ++++++++++++++++--- .../traits/CfnResourceProperty.java | 5 +++- .../traits/CfnResourceIndexTest.java | 6 ++-- .../smithy-testservice-basil.cfn.json | 2 +- 4 files changed, 32 insertions(+), 9 deletions(-) diff --git a/smithy-aws-cloudformation-traits/src/main/java/software/amazon/smithy/aws/cloudformation/traits/CfnResourceIndex.java b/smithy-aws-cloudformation-traits/src/main/java/software/amazon/smithy/aws/cloudformation/traits/CfnResourceIndex.java index 2c4222fd618..09fe6ceef19 100644 --- a/smithy-aws-cloudformation-traits/src/main/java/software/amazon/smithy/aws/cloudformation/traits/CfnResourceIndex.java +++ b/smithy-aws-cloudformation-traits/src/main/java/software/amazon/smithy/aws/cloudformation/traits/CfnResourceIndex.java @@ -23,7 +23,9 @@ import java.util.Optional; import java.util.Set; import java.util.function.Function; +import java.util.stream.Collectors; import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.knowledge.BottomUpIndex; import software.amazon.smithy.model.knowledge.IdentifierBindingIndex; import software.amazon.smithy.model.knowledge.KnowledgeIndex; import software.amazon.smithy.model.knowledge.OperationIndex; @@ -48,6 +50,8 @@ public final class CfnResourceIndex implements KnowledgeIndex { static final Set FULLY_MUTABLE = SetUtils.of( Mutability.CREATE, Mutability.READ, Mutability.WRITE); + static final Set INHERITED_MUTABILITY = SetUtils.of( + Mutability.CREATE, Mutability.READ); private final Map resourceDefinitions = new HashMap<>(); @@ -85,15 +89,22 @@ public enum Mutability { public CfnResourceIndex(Model model) { OperationIndex operationIndex = OperationIndex.of(model); + BottomUpIndex bottomUpIndex = BottomUpIndex.of(model); model.shapes(ResourceShape.class) .filter(shape -> shape.hasTrait(CfnResourceTrait.ID)) .forEach(resource -> { CfnResource.Builder builder = CfnResource.builder(); ShapeId resourceId = resource.getId(); + Set parentResources = model.getServiceShapes() + .stream() + .map(service -> bottomUpIndex.getResourceBinding(service, resourceId)) + .flatMap(Optional::stream) + .collect(Collectors.toSet()); + // Start with the explicit resource identifiers. builder.primaryIdentifiers(resource.getIdentifiers().keySet()); - setIdentifierMutabilities(builder, resource); + setIdentifierMutabilities(builder, resource, parentResources); // Use the read lifecycle's input to collect the additional identifiers // and its output to collect readable properties. @@ -164,13 +175,22 @@ public Optional getResource(ToShapeId resource) { return Optional.ofNullable(resourceDefinitions.get(resource.toShapeId())); } - private void setIdentifierMutabilities(CfnResource.Builder builder, ResourceShape resource) { - Set mutability = getDefaultIdentifierMutabilities(resource); + private boolean identifierIsInherited(String identifier, Set parentResources) { + return parentResources.stream() + .anyMatch(parentResource -> parentResource.getIdentifiers().containsKey(identifier)); + } + + private void setIdentifierMutabilities( + CfnResource.Builder builder, + ResourceShape resource, + Set parentResources) { + Set defaultIdentifierMutability = getDefaultIdentifierMutabilities(resource); resource.getIdentifiers().forEach((name, shape) -> { builder.putPropertyDefinition(name, CfnResourceProperty.builder() .hasExplicitMutability(true) - .mutabilities(mutability) + .mutabilities(identifierIsInherited(name, parentResources) + ? INHERITED_MUTABILITY : defaultIdentifierMutability) .addShapeId(shape) .build()); }); diff --git a/smithy-aws-cloudformation-traits/src/main/java/software/amazon/smithy/aws/cloudformation/traits/CfnResourceProperty.java b/smithy-aws-cloudformation-traits/src/main/java/software/amazon/smithy/aws/cloudformation/traits/CfnResourceProperty.java index 385e0669101..f0a333b3569 100644 --- a/smithy-aws-cloudformation-traits/src/main/java/software/amazon/smithy/aws/cloudformation/traits/CfnResourceProperty.java +++ b/smithy-aws-cloudformation-traits/src/main/java/software/amazon/smithy/aws/cloudformation/traits/CfnResourceProperty.java @@ -66,7 +66,10 @@ public ShapeId getShapeId() { * by the use of a trait instead of derived through its lifecycle * bindings within a resource. * - * @return Returns true if the mutability is explicitly defined by a trait. + *

Also returns true for identifiers, since their mutability is inherent + * + * @return Returns true if the mutability is explicitly defined by a trait or + * if property is an identifier * * @see CfnMutabilityTrait */ diff --git a/smithy-aws-cloudformation-traits/src/test/java/software/amazon/smithy/aws/cloudformation/traits/CfnResourceIndexTest.java b/smithy-aws-cloudformation-traits/src/test/java/software/amazon/smithy/aws/cloudformation/traits/CfnResourceIndexTest.java index 2aaba836ca9..65ccbe73db9 100644 --- a/smithy-aws-cloudformation-traits/src/test/java/software/amazon/smithy/aws/cloudformation/traits/CfnResourceIndexTest.java +++ b/smithy-aws-cloudformation-traits/src/test/java/software/amazon/smithy/aws/cloudformation/traits/CfnResourceIndexTest.java @@ -99,15 +99,15 @@ public static Collection data() { bazResource.identifiers = SetUtils.of("barId", "bazId"); bazResource.additionalIdentifiers = ListUtils.of(); bazResource.mutabilities = MapUtils.of( - "barId", SetUtils.of(Mutability.READ), + "barId", SetUtils.of(Mutability.CREATE, Mutability.READ), "bazId", SetUtils.of(Mutability.READ), "bazExplicitMutableProperty", CfnResourceIndex.FULLY_MUTABLE, "bazImplicitFullyMutableProperty", CfnResourceIndex.FULLY_MUTABLE, "bazImplicitCreateProperty", SetUtils.of(Mutability.CREATE, Mutability.READ), "bazImplicitReadProperty", SetUtils.of(Mutability.READ), "bazImplicitWriteProperty", SetUtils.of(Mutability.CREATE, Mutability.WRITE)); - bazResource.createOnlyProperties = SetUtils.of("bazImplicitCreateProperty"); - bazResource.readOnlyProperties = SetUtils.of("barId", "bazId", "bazImplicitReadProperty"); + bazResource.createOnlyProperties = SetUtils.of("barId", "bazImplicitCreateProperty"); + bazResource.readOnlyProperties = SetUtils.of("bazId", "bazImplicitReadProperty"); bazResource.writeOnlyProperties = SetUtils.of("bazImplicitWriteProperty"); return ListUtils.of(fooResource, barResource, bazResource); diff --git a/smithy-aws-cloudformation/src/test/resources/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/smithy-testservice-basil.cfn.json b/smithy-aws-cloudformation/src/test/resources/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/smithy-testservice-basil.cfn.json index 66ba45b2073..05ca14e6a15 100644 --- a/smithy-aws-cloudformation/src/test/resources/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/smithy-testservice-basil.cfn.json +++ b/smithy-aws-cloudformation/src/test/resources/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/smithy-testservice-basil.cfn.json @@ -25,7 +25,6 @@ } }, "readOnlyProperties": [ - "/properties/BarId", "/properties/BazId", "/properties/BazImplicitReadProperty" ], @@ -33,6 +32,7 @@ "/properties/BazImplicitWriteProperty" ], "createOnlyProperties": [ + "/properties/BarId", "/properties/BazImplicitCreateProperty" ], "primaryIdentifier": [ From f9193687ca6355a0315f38c315d59953b27d643a Mon Sep 17 00:00:00 2001 From: Hayden Baker Date: Mon, 24 Oct 2022 12:49:26 -0700 Subject: [PATCH 2/2] Use OptionalUtils::stream instead of Java9 --- .../smithy/aws/cloudformation/traits/CfnResourceIndex.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/smithy-aws-cloudformation-traits/src/main/java/software/amazon/smithy/aws/cloudformation/traits/CfnResourceIndex.java b/smithy-aws-cloudformation-traits/src/main/java/software/amazon/smithy/aws/cloudformation/traits/CfnResourceIndex.java index 09fe6ceef19..0733408101c 100644 --- a/smithy-aws-cloudformation-traits/src/main/java/software/amazon/smithy/aws/cloudformation/traits/CfnResourceIndex.java +++ b/smithy-aws-cloudformation-traits/src/main/java/software/amazon/smithy/aws/cloudformation/traits/CfnResourceIndex.java @@ -38,6 +38,7 @@ import software.amazon.smithy.model.shapes.StructureShape; import software.amazon.smithy.model.shapes.ToShapeId; import software.amazon.smithy.utils.MapUtils; +import software.amazon.smithy.utils.OptionalUtils; import software.amazon.smithy.utils.SetUtils; /** @@ -99,7 +100,7 @@ public CfnResourceIndex(Model model) { Set parentResources = model.getServiceShapes() .stream() .map(service -> bottomUpIndex.getResourceBinding(service, resourceId)) - .flatMap(Optional::stream) + .flatMap(OptionalUtils::stream) .collect(Collectors.toSet()); // Start with the explicit resource identifiers.