Fix cfn-mutability for inherited identifiers (#1465)

* 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

Author: haydenbaker

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;
@@ -36,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;
 
 /**
@@ -48,6 +51,8 @@ public final class CfnResourceIndex implements KnowledgeIndex {
 
     static final Set<Mutability> FULLY_MUTABLE = SetUtils.of(
             Mutability.CREATE, Mutability.READ, Mutability.WRITE);
+    static final Set<Mutability> INHERITED_MUTABILITY = SetUtils.of(
+            Mutability.CREATE, Mutability.READ);
 
     private final Map<ShapeId, CfnResource> resourceDefinitions = new HashMap<>();
 
@@ -85,15 +90,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<ResourceShape> parentResources = model.getServiceShapes()
+                            .stream()
+                            .map(service -> bottomUpIndex.getResourceBinding(service, resourceId))
+                            .flatMap(OptionalUtils::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 +176,22 @@ public Optional<CfnResource> getResource(ToShapeId resource) {
         return Optional.ofNullable(resourceDefinitions.get(resource.toShapeId()));
     }
 
-    private void setIdentifierMutabilities(CfnResource.Builder builder, ResourceShape resource) {
-        Set<Mutability> mutability = getDefaultIdentifierMutabilities(resource);
+    private boolean identifierIsInherited(String identifier, Set<ResourceShape> parentResources) {
+        return parentResources.stream()
+                .anyMatch(parentResource -> parentResource.getIdentifiers().containsKey(identifier));
+    }
+
+    private void setIdentifierMutabilities(
+            CfnResource.Builder builder,
+            ResourceShape resource,
+            Set<ResourceShape> parentResources) {
+        Set<Mutability> 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());
         });

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.
+     * <p> 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
      */

smithy-aws-cloudformation-traits/src/test/java/software/amazon/smithy/aws/cloudformation/traits/CfnResourceIndexTest.java

@@ -99,15 +99,15 @@ public static Collection<ResourceData> 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);

smithy-aws-cloudformation/src/test/resources/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/smithy-testservice-basil.cfn.json

@@ -25,14 +25,14 @@
     }
   },
   "readOnlyProperties": [
-    "/properties/BarId",
     "/properties/BazId",
     "/properties/BazImplicitReadProperty"
   ],
   "writeOnlyProperties": [
     "/properties/BazImplicitWriteProperty"
   ],
   "createOnlyProperties": [
+    "/properties/BarId",
     "/properties/BazImplicitCreateProperty"
   ],
   "primaryIdentifier": [