Fix resource identifier binding validation

This commit fixes a bug where only the names of identifiers were
validated, not the ShapeId that represent them, when testing the
binding of operations to resources.

Author: kstich

smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/ResourceIdentifierBindingValidator.java

@@ -23,6 +23,7 @@
 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.shapes.StructureShape;
 import software.amazon.smithy.model.traits.ResourceIdentifierTrait;
 import software.amazon.smithy.model.validation.AbstractValidator;
 import software.amazon.smithy.model.validation.ValidationEvent;
@@ -189,7 +190,7 @@ private Stream<ValidationEvent> validateCollectionBindings(
                 // Get their operation shapes.
                 .flatMap(id -> OptionalUtils.stream(model.getShape(id).flatMap(Shape::asOperationShape)))
                 // Find collection operations which improperly bind all the resource identifiers.
-                .filter(operation -> hasAllIdentifiersBound(resource, operation, identifierIndex))
+                .filter(operation -> hasAllIdentifiersBound(model, resource, operation, identifierIndex))
                 .map(operation -> error(operation,
                         format(
                                 "This operation is bound as a collection operation on the `%s` resource, but it improperly "
@@ -210,7 +211,7 @@ private Stream<ValidationEvent> validateInstanceBindings(
                 // Get their operation shapes.
                 .flatMap(id -> OptionalUtils.stream(model.getShape(id).flatMap(Shape::asOperationShape)))
                 // Find instance operations which do not bind all of the resource identifiers.
-                .filter(operation -> !hasAllIdentifiersBound(resource, operation, bindingIndex))
+                .filter(operation -> !hasAllIdentifiersBound(model, resource, operation, bindingIndex))
                 .map(operation -> {
                     String expectedIdentifiers = createBindingMessage(resource.getIdentifiers());
                     String boundIds = createBindingMessage(bindingIndex.getOperationInputBindings(resource, operation));
@@ -227,13 +228,24 @@ private Stream<ValidationEvent> validateInstanceBindings(
     }
 
     private boolean hasAllIdentifiersBound(
+            Model model,
             ResourceShape resource,
             OperationShape operation,
             IdentifierBindingIndex bindingIndex
     ) {
-        return bindingIndex.getOperationInputBindings(resource, operation)
-                .keySet()
-                .containsAll(resource.getIdentifiers().keySet());
+        StructureShape inputShape = model.expectShape(operation.getInputShape(), StructureShape.class);
+        Map<String, String> bindings = bindingIndex.getOperationInputBindings(resource, operation);
+        for (Map.Entry<String, ShapeId> identifier : resource.getIdentifiers().entrySet()) {
+            if (!bindings.containsKey(identifier.getKey())) {
+                return false;
+            }
+
+            MemberShape identifierMember = inputShape.getMember(bindings.get(identifier.getKey())).get();
+            if (!identifierMember.getTarget().equals(identifier.getValue())) {
+                return false;
+            }
+        }
+        return true;
     }
 
     private String createBindingMessage(Map<String, ?> bindings) {

smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/resource-operation-binding-validator.errors

@@ -1,2 +1,4 @@
 [ERROR] ns.foo#InvalidResourceOperation1: This operation does not form a valid instance operation when bound to resource `ns.foo#InvalidResource`. All of the identifiers of the resource were not implicitly or explicitly bound to the input of the operation. Expected the following identifier bindings: [required member named `bar` that targets `smithy.api#String`]. Found the following identifier bindings: [] | ResourceIdentifierBinding
+[ERROR] ns.foo#InvalidResourceOperation2: This operation does not form a valid instance operation when bound to resource `ns.foo#InvalidResource`. All of the identifiers of the resource were not implicitly or explicitly bound to the input of the operation. Expected the following identifier bindings: [required member named `bar` that targets `smithy.api#String`]. Found the following identifier bindings: [] | ResourceIdentifierBinding
 [ERROR] ns.foo#ValidResourceOperation2: This operation does not form a valid instance operation when bound to resource `ns.foo#ValidResource`. All of the identifiers of the resource were not implicitly or explicitly bound to the input of the operation. Expected the following identifier bindings: [required member named `foo` that targets `smithy.api#String`]. Found the following identifier bindings: [] | ResourceIdentifierBinding
+[ERROR] ns.foo#ValidResourceOperation4: This operation is bound as a collection operation on the `ns.foo#ValidResource` resource, but it improperly binds all of the identifiers of the resource to members of the operation input. | ResourceIdentifierBinding

smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/resource-operation-binding-validator.json

@@ -27,6 +27,14 @@
                 {
                     "target": "ns.foo#ValidResourceOperation2"
                 }
+            ],
+            "collectionOperations": [
+                {
+                    "target": "ns.foo#ValidResourceOperation3"
+                },
+                {
+                    "target": "ns.foo#ValidResourceOperation4"
+                }
             ]
         },
         "ns.foo#ValidResourceOperation1": {
@@ -80,6 +88,47 @@
                 "smithy.api#output": {}
             }
         },
+        "ns.foo#ValidResourceOperation3": {
+            "type": "operation",
+            "input": {
+                "target": "ns.foo#ValidResourceOperation3Input"
+            }
+        },
+        "ns.foo#ValidResourceOperation3Input": {
+            "type": "structure",
+            "members": {
+                "foo": {
+                    "target": "ns.foo#OtherString",
+                    "traits": {
+                        "smithy.api#required": {}
+                    }
+                }
+            },
+            "traits": {
+                "smithy.api#input": {}
+            }
+        },
+        "ns.foo#ValidResourceOperation4": {
+            "type": "operation",
+            "input": {
+                "target": "ns.foo#ValidResourceOperation4Input"
+            }
+        },
+        "ns.foo#ValidResourceOperation4Input": {
+            "type": "structure",
+            "members": {
+                "foo": {
+                    "target": "smithy.api#String",
+                    "traits": {
+                        "smithy.api#required": {},
+                        "smithy.api#suppress": ["MemberShouldReferenceResource"]
+                    }
+                }
+            },
+            "traits": {
+                "smithy.api#input": {}
+            }
+        },
         "ns.foo#InvalidResource": {
             "type": "resource",
             "identifiers": {
@@ -130,7 +179,7 @@
             "type": "structure",
             "members": {
                 "bar": {
-                    "target": "smithy.api#String",
+                    "target": "ns.foo#OtherString",
                     "traits": {
                         "smithy.api#required": {}
                     }
@@ -145,6 +194,9 @@
             "traits": {
                 "smithy.api#output": {}
             }
+        },
+        "ns.foo#OtherString": {
+            "type": "string"
         }
     }
 }