Add primaryIdentifier to cfnResource trait

docs/source-2.0/aws/aws-cloudformation.rst

@@ -55,6 +55,11 @@ supports the following members:
         Members of these structures with the same names MUST resolve to the
         same target. See :ref:`aws-cloudformation-property-deriviation` for
         more information.
+    * - primaryIdentifier
+      - ``string``
+      - **Deprecated** An alternative resource property to use as the primary
+        identifier for the CloudFormation resource. The value MUST be the name
+        of a property on the resource shape that targets a string shape.
 
 The following example defines a simple resource that is also a CloudFormation
 resource:

smithy-aws-cloudformation-traits/src/main/java/software/amazon/smithy/aws/cloudformation/traits/CfnResourceIndex.java

@@ -174,6 +174,14 @@ public CfnResourceIndex(Model model) {
                                 });
                     }
 
+                    // If the primary identifier is rerouted, the derived identifiers are
+                    // additional identifiers for the resource.
+                    if (trait.getPrimaryIdentifier().isPresent()) {
+                        CfnResource tempResource = builder.build();
+                        builder.addAdditionalIdentifier(SetUtils.copyOf(tempResource.getPrimaryIdentifiers()));
+                        builder.primaryIdentifiers(SetUtils.of(trait.getPrimaryIdentifier().get()));
+                    }
+
                     resourceDefinitions.put(resourceId, builder.build());
                 });
     }

smithy-aws-cloudformation-traits/src/main/java/software/amazon/smithy/aws/cloudformation/traits/CfnResourcePropertyValidator.java

@@ -4,6 +4,8 @@
  */
 package software.amazon.smithy.aws.cloudformation.traits;
 
+import static java.lang.String.format;
+
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
@@ -12,6 +14,7 @@
 import java.util.TreeSet;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.shapes.ResourceShape;
+import software.amazon.smithy.model.shapes.Shape;
 import software.amazon.smithy.model.shapes.ShapeId;
 import software.amazon.smithy.model.validation.AbstractValidator;
 import software.amazon.smithy.model.validation.ValidationEvent;
@@ -27,10 +30,9 @@ public List<ValidationEvent> validate(Model model) {
         List<ValidationEvent> events = new ArrayList<>();
 
         CfnResourceIndex cfnResourceIndex = CfnResourceIndex.of(model);
-        model.shapes(ResourceShape.class)
-                .filter(shape -> shape.hasTrait(CfnResourceTrait.ID))
-                .map(shape -> validateResource(model, cfnResourceIndex, shape))
-                .forEach(events::addAll);
+        for (ResourceShape resource : model.getResourceShapesWithTrait(CfnResourceTrait.class)) {
+            events.addAll(validateResource(model, cfnResourceIndex, resource));
+        }
 
         return events;
     }
@@ -44,17 +46,48 @@ private List<ValidationEvent> validateResource(
         List<ValidationEvent> events = new ArrayList<>();
         String resourceName = trait.getName().orElse(resource.getId().getName());
 
-        cfnResourceIndex.getResource(resource)
-                .map(CfnResource::getProperties)
-                .ifPresent(properties -> {
-                    for (Map.Entry<String, CfnResourceProperty> property : properties.entrySet()) {
-                        validateResourceProperty(model, resource, resourceName, property).ifPresent(events::add);
-                    }
-                });
+        if (trait.getPrimaryIdentifier().isPresent()) {
+            validateResourcePrimaryIdentifier(model, resource, trait.getPrimaryIdentifier().get())
+                    .ifPresent(events::add);
+        }
+
+        Optional<CfnResource> cfnResourceOptional = cfnResourceIndex.getResource(resource);
+        if (cfnResourceOptional.isPresent()) {
+            for (Map.Entry<String, CfnResourceProperty> property : cfnResourceOptional.get()
+                    .getProperties()
+                    .entrySet()) {
+                validateResourceProperty(model, resource, resourceName, property).ifPresent(events::add);
+            }
+        }
 
         return events;
     }
 
+    private Optional<ValidationEvent> validateResourcePrimaryIdentifier(
+            Model model,
+            ResourceShape resource,
+            String primaryIdentifier
+    ) {
+        Map<String, ShapeId> properties = resource.getProperties();
+        if (!properties.containsKey(primaryIdentifier)) {
+            return Optional.of(error(resource,
+                    resource.expectTrait(CfnResourceTrait.class),
+                    format("The alternative resource primary identifier, `%s`, must be a property of the resource.",
+                            primaryIdentifier)));
+        }
+
+        Shape propertyTarget = model.expectShape(properties.get(primaryIdentifier));
+        if (!propertyTarget.isStringShape() && !propertyTarget.isEnumShape()) {
+            return Optional.of(error(resource,
+                    resource.expectTrait(CfnResourceTrait.class),
+                    format("The alternative resource primary identifier, `%s`, targets a `%s` shape, it must target a `string`.",
+                            primaryIdentifier,
+                            propertyTarget.getType())));
+        }
+
+        return Optional.empty();
+    }
+
     private Optional<ValidationEvent> validateResourceProperty(
             Model model,
             ResourceShape resource,
@@ -72,7 +105,7 @@ private Optional<ValidationEvent> validateResourceProperty(
 
         if (propertyTargets.size() > 1) {
             return Optional.of(error(resource,
-                    String.format("The `%s` property of the generated `%s` "
+                    format("The `%s` property of the generated `%s` "
                             + "CloudFormation resource targets multiple shapes: %s. Reusing member names that "
                             + "target different shapes can cause confusion for users of the API. This target "
                             + "discrepancy must either be resolved in the model or one of the members must be "

smithy-aws-cloudformation-traits/src/main/java/software/amazon/smithy/aws/cloudformation/traits/CfnResourceTrait.java

@@ -25,11 +25,13 @@ public final class CfnResourceTrait extends AbstractTrait
 
     private final String name;
     private final List<ShapeId> additionalSchemas;
+    private final String primaryIdentifier;
 
     private CfnResourceTrait(Builder builder) {
         super(ID, builder.getSourceLocation());
         name = builder.name;
         additionalSchemas = ListUtils.copyOf(builder.additionalSchemas);
+        primaryIdentifier = builder.primaryIdentifier;
     }
 
     /**
@@ -50,6 +52,15 @@ public List<ShapeId> getAdditionalSchemas() {
         return additionalSchemas;
     }
 
+    /**
+     * Gets the alternative resource property to use as the primary identifier for the CloudFormation resource.
+     *
+     * @return Returns the optional alternative primary identifier.
+     */
+    public Optional<String> getPrimaryIdentifier() {
+        return Optional.ofNullable(primaryIdentifier);
+    }
+
     public static Builder builder() {
         return new Builder();
     }
@@ -83,6 +94,7 @@ public Trait createTrait(ShapeId target, Node value) {
     public static final class Builder extends AbstractTraitBuilder<CfnResourceTrait, Builder> {
         private String name;
         private final List<ShapeId> additionalSchemas = new ArrayList<>();
+        private String primaryIdentifier;
 
         private Builder() {}
 
@@ -106,5 +118,10 @@ public Builder additionalSchemas(List<ShapeId> additionalSchemas) {
             this.additionalSchemas.addAll(additionalSchemas);
             return this;
         }
+
+        public Builder primaryIdentifier(String primaryIdentifier) {
+            this.primaryIdentifier = primaryIdentifier;
+            return this;
+        }
     }
 }

smithy-aws-cloudformation-traits/src/main/resources/META-INF/smithy/aws.cloudformation.smithy

@@ -6,8 +6,8 @@ namespace aws.cloudformation
 /// additional identifier for the resource.
 @unstable
 @trait(
-    selector: "structure > :test(member > string)",
-    conflicts: [cfnExcludeProperty],
+    selector: "structure > :test(member > string)"
+    conflicts: [cfnExcludeProperty]
     breakingChanges: [{change: "remove"}]
 )
 structure cfnAdditionalIdentifier {}
@@ -16,7 +16,7 @@ structure cfnAdditionalIdentifier {}
 /// to differ from a structure member name used in the model.
 @unstable
 @trait(
-    selector: "structure > member",
+    selector: "structure > member"
     breakingChanges: [{change: "any"}]
 )
 string cfnName
@@ -25,12 +25,12 @@ string cfnName
 /// CloudFormation resource definitions.
 @unstable
 @trait(
-    selector: "structure > member",
+    selector: "structure > member"
     conflicts: [
-        cfnAdditionalIdentifier,
-        cfnMutability,
+        cfnAdditionalIdentifier
+        cfnMutability
         cfnDefaultValue
-    ],
+    ]
     breakingChanges: [{change: "add"}]
 )
 structure cfnExcludeProperty {}
@@ -39,7 +39,7 @@ structure cfnExcludeProperty {}
 /// for the property of the CloudFormation resource.
 @unstable
 @trait(
-    selector: "resource > operation -[input, output]-> structure > member",
+    selector: "resource > operation -[input, output]-> structure > member"
     conflicts: [cfnExcludeProperty]
 )
 structure cfnDefaultValue {}
@@ -48,7 +48,7 @@ structure cfnDefaultValue {}
 /// when part of a CloudFormation resource.
 @unstable
 @trait(
-    selector: "structure > member",
+    selector: "structure > member"
     conflicts: [cfnExcludeProperty]
 )
 enum cfnMutability {
@@ -86,16 +86,22 @@ enum cfnMutability {
 /// Indicates that a Smithy resource is a CloudFormation resource.
 @unstable
 @trait(
-    selector: "resource",
+    selector: "resource"
     breakingChanges: [{change: "presence"}]
 )
 structure cfnResource {
     /// Provides a custom CloudFormation resource name.
-    name: String,
+    name: String
 
     /// A list of additional shape IDs of structures that will have their
     /// properties added to the CloudFormation resource.
-    additionalSchemas: StructureIdList,
+    additionalSchemas: StructureIdList
+
+    /// An alternative resource property to use as the primary identifier
+    /// for the CloudFormation resource. The value MUST be the name of a
+    /// property on the resource shape that targets a string shape.
+    @deprecated(message: "Prefer the resource's identifiers when generating resource schemas.")
+    primaryIdentifier: String
 }
 
 @private

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

@@ -28,6 +28,7 @@ public class CfnResourceIndexTest {
     private static final ShapeId FOO = ShapeId.from("smithy.example#FooResource");
     private static final ShapeId BAR = ShapeId.from("smithy.example#BarResource");
     private static final ShapeId BAZ = ShapeId.from("smithy.example#BazResource");
+    private static final ShapeId TAD = ShapeId.from("smithy.example#TadResource");
 
     private static Model model;
     private static CfnResourceIndex cfnResourceIndex;
@@ -118,7 +119,20 @@ public static Collection<ResourceData> data() {
         bazResource.readOnlyProperties = SetUtils.of("bazId", "bazImplicitReadProperty");
         bazResource.writeOnlyProperties = SetUtils.of("bazImplicitWriteProperty");
 
-        return ListUtils.of(fooResource, barResource, bazResource);
+        ResourceData tadResource = new ResourceData();
+        tadResource.resourceId = TAD;
+        tadResource.identifiers = SetUtils.of("tadArn");
+        tadResource.additionalIdentifiers = ListUtils.of(SetUtils.of("tadId"));
+        tadResource.mutabilities = MapUtils.of(
+                "tadId",
+                SetUtils.of(Mutability.READ),
+                "tadArn",
+                SetUtils.of(Mutability.READ));
+        tadResource.createOnlyProperties = SetUtils.of();
+        tadResource.readOnlyProperties = SetUtils.of("tadId", "tadArn");
+        tadResource.writeOnlyProperties = SetUtils.of();
+
+        return ListUtils.of(fooResource, tadResource, bazResource, tadResource);
     }
 
     @ParameterizedTest

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

@@ -7,6 +7,7 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.equalTo;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
@@ -36,6 +37,14 @@ public void loadsFromModel() {
         assertThat(barTrait.getName().get(), equalTo("CustomResource"));
         assertFalse(barTrait.getAdditionalSchemas().isEmpty());
         assertThat(barTrait.getAdditionalSchemas(), contains(ShapeId.from("smithy.example#ExtraBarRequest")));
+
+        Shape tadResource = result.expectShape(ShapeId.from("smithy.example#TadResource"));
+        assertTrue(tadResource.hasTrait(CfnResourceTrait.class));
+        CfnResourceTrait tadTrait = tadResource.expectTrait(CfnResourceTrait.class);
+        assertFalse(tadTrait.getName().isPresent());
+        assertTrue(tadTrait.getAdditionalSchemas().isEmpty());
+        assertTrue(tadTrait.getPrimaryIdentifier().isPresent());
+        assertEquals("tadArn", tadTrait.getPrimaryIdentifier().get());
     }
 
     @Test

smithy-aws-cloudformation-traits/src/test/resources/software/amazon/smithy/aws/cloudformation/traits/cfn-resources.smithy

@@ -12,23 +12,57 @@ resource FooResource {
 }
 
 @cfnResource(
-    name: "CustomResource",
+    name: "CustomResource"
     additionalSchemas: [ExtraBarRequest]
 )
 resource BarResource {
     identifiers: {
         barId: BarId
-    },
-    operations: [ExtraBarOperation],
+    }
+    operations: [ExtraBarOperation]
+}
+
+@cfnResource(primaryIdentifier: "tadArn")
+resource TadResource {
+    identifiers: {
+        tadId: String
+    }
+    properties: {
+        tadArn: String
+    }
+    create: CreateTad
+    read: GetTad
+}
+
+operation CreateTad {
+    input := {}
+    output := {
+        @required
+        tadId: String
+        tadArn: String
+    }
+}
+
+@readonly
+operation GetTad {
+    input := {
+        @required
+        tadId: String
+    }
+    output := {
+        @required
+        tadId: String
+        tadArn: String
+    }
 }
 
 operation ExtraBarOperation {
-    input: ExtraBarRequest,
+    input: ExtraBarRequest
 }
 
 structure ExtraBarRequest {
     @required
-    barId: BarId,
+    barId: BarId
 }
 
 string FooId

smithy-aws-cloudformation-traits/src/test/resources/software/amazon/smithy/aws/cloudformation/traits/errorfiles/primary-identifier-swap.errors

@@ -0,0 +1,5 @@
+[ERROR] smithy.example#BarResource: The alternative resource primary identifier, `barArn`, must be a property of the resource. | CfnResourceProperty
+[ERROR] smithy.example#BazResource: The alternative resource primary identifier, `bazArn`, targets a `integer` shape, it must target a `string`. | CfnResourceProperty
+[WARNING] smithy.example#FooResource: This shape applies a trait that is unstable: aws.cloudformation#cfnResource | UnstableTrait.aws.cloudformation#cfnResource
+[WARNING] smithy.example#BarResource: This shape applies a trait that is unstable: aws.cloudformation#cfnResource | UnstableTrait.aws.cloudformation#cfnResource
+[WARNING] smithy.example#BazResource: This shape applies a trait that is unstable: aws.cloudformation#cfnResource | UnstableTrait.aws.cloudformation#cfnResource