Repository: smithy-lang/smithy
Pull request #1882 (2023-07-25 16:15:35)

Update docId resolution method
*Description of changes:* This commit changes the resolution of the default docId property of the service trait by removing the reference to the target from the trait and passing the targeted shape itself as a parameter.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.



Patch:
diff --git a/docs/source-1.0/spec/core/http-traits.rst b/docs/source-1.0/spec/core/http-traits.rst
index b6ae367a1..61d2368e9 100644
--- a/docs/source-1.0/spec/core/http-traits.rst
+++ b/docs/source-1.0/spec/core/http-traits.rst
@@ -707,13 +707,9 @@ the body of the response.
 Summary
     Binds a single structure member to the body of an HTTP message.
 Trait selector
-    .. code-block:: none
-
-        structure > :test(member > :test(string, blob, structure, union, document, list, set, map))
+    ``structure > member``
 
-    The ``httpPayload`` trait can be applied to ``structure`` members that
-    target a ``string``, ``blob``, ``structure``, ``union``, ``document``,
-    ``set``, ``map``, or ``list``.
+    *Any structure member*
 Value type
     Annotation trait.
 Conflicts with
@@ -777,10 +773,9 @@ or :ref:`httpPrefixHeaders-trait`.
 
 #. When a string or blob member is referenced, the raw value is serialized
    as the body of the message.
-#. When a :ref:`structure <structure>`, :ref:`union <union>`, :ref:`list <list>`,
-   :ref:`set <set>`, :ref:`map <map>`, or document type is targeted,
-   the shape value is serialized as a :ref:`protocol-specific <protocolDefinition-trait>`
-   document that is sent as the body of the message.
+#. When any other type of member is referenced, the shape value is serialized
+   as a :ref:`protocol-specific <protocolDefinition-trait>` value that is sent
+   as the body of the message.
 
 
 .. smithy-trait:: smithy.api#httpPrefixHeaders
diff --git a/docs/source-2.0/spec/http-bindings.rst b/docs/source-2.0/spec/http-bindings.rst
index aa8ce2f2d..2748eb2cd 100644
--- a/docs/source-2.0/spec/http-bindings.rst
+++ b/docs/source-2.0/spec/http-bindings.rst
@@ -709,14 +709,9 @@ the body of the response.
 Summary
     Binds a single structure member to the body of an HTTP message.
 Trait selector
-    .. code-block:: none
-
-        structure > :test(member > :test(string, blob, structure, union, document, list, map))
-
-    The ``httpPayload`` trait can be applied to ``structure`` members that
-    target a ``string``, ``blob``, ``structure``, ``union``, ``document``,
-    ``map``, or ``list``.
+    ``structure > member``
 
+    *Any structure member*
 Value type
     Annotation trait.
 Conflicts with
@@ -781,9 +776,8 @@ or :ref:`httpPrefixHeaders-trait`.
 
 #. When a string or blob member is referenced, the raw value is serialized
    as the body of the message.
-#. When a :ref:`structure <structure>`, :ref:`union <union>`, :ref:`list <list>`,
-   :ref:`map <map>`, or document type is targeted, the shape value is serialized
-   as a :ref:`protocol-specific <protocolDefinition-trait>` document that is sent
+#. When any other type of member is referenced, the shape value is serialized
+   as a :ref:`protocol-specific <protocolDefinition-trait>` value that is sent
    as the body of the message.
 
 
diff --git a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/ServiceTrait.java b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/ServiceTrait.java
index bc8cdcd3b..7d4c43afe 100644
--- a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/ServiceTrait.java
+++ b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/ServiceTrait.java
@@ -19,8 +19,8 @@ import java.util.Locale;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.logging.Logger;
-import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.SourceException;
+import software.amazon.smithy.model.node.ExpectationNotMetException;
 import software.amazon.smithy.model.node.Node;
 import software.amazon.smithy.model.node.ObjectNode;
 import software.amazon.smithy.model.node.StringNode;
@@ -40,7 +40,6 @@ public final class ServiceTrait extends AbstractTrait implements ToSmithyBuilder
     public static final ShapeId ID = ShapeId.from("aws.api#service");
     private static final Logger LOGGER = Logger.getLogger(ServiceTrait.class.getName());
 
-    private final ShapeId target;
     private final String cloudFormationName;
     private final String arnNamespace;
     private final String sdkId;
@@ -50,7 +49,6 @@ public final class ServiceTrait extends AbstractTrait implements ToSmithyBuilder
 
     private ServiceTrait(Builder builder) {
         super(ID, builder.getSourceLocation());
-        this.target = SmithyBuilder.requiredState("target", builder.target);
         this.sdkId = SmithyBuilder.requiredState("sdkId", builder.sdkId);
         this.arnNamespace = SmithyBuilder.requiredState("arnNamespace", builder.arnNamespace);
         this.cloudFormationName = SmithyBuilder.requiredState("cloudFormationName", builder.cloudFormationName);
@@ -149,25 +147,31 @@ public final class ServiceTrait extends AbstractTrait implements ToSmithyBuilder
     }
 
     /**
-     * Returns the documentation identifier value for the service.
+     * Resolves the doc id value for the service.
      *
      * <p> When value on trait is not set, this method defaults to the lower
      * cased value of the sdkId followed by the service version, separated by
      * dashes.
      *
+     * @param serviceShape the shape which this trait targets
      * @return Returns the documentation identifier value for the service name.
+     * @throws ExpectationNotMetException if the shape is not the target of this trait.
      */
-    public String getDocId(Model model) {
-        return getDocId().orElseGet(() -> buildDefaultDocId(model));
+    public String resolveDocId(ServiceShape serviceShape) {
+        return getDocId().orElseGet(() -> buildDefaultDocId(serviceShape));
     }
 
     protected Optional<String> getDocId() {
         return Optional.ofNullable(docId);
     }
 
-    private String buildDefaultDocId(Model model) {
-        String version = model.expectShape(target, ServiceShape.class).getVersion();
-        return sdkId.replace(" ", "-").toLowerCase(Locale.US) + "-" + version;
+    private String buildDefaultDocId(ServiceShape serviceShape) {
+        if (!serviceShape.expectTrait(ServiceTrait.class).equals(this)) {
+            throw new ExpectationNotMetException(String.format(
+                    "Provided service shape `%s` is not the target of this trait.", serviceShape.getId()), this);
+        }
+
+        return sdkId.replace(" ", "-").toLowerCase(Locale.US) + "-" + serviceShape.getVersion();
     }
 
     /**
@@ -192,7 +196,6 @@ public final class ServiceTrait extends AbstractTrait implements ToSmithyBuilder
     @Override
     public Builder toBuilder() {
         return new Builder()
-                .target(target)
                 .sdkId(sdkId)
                 .sourceLocation(getSourceLocation())
                 .cloudFormationName(cloudFormationName)
@@ -206,7 +209,6 @@ public final class ServiceTrait extends AbstractTrait implements ToSmithyBuilder
     protected Node createNode() {
         return Node.objectNodeBuilder()
                 .sourceLocation(getSourceLocation())
-                .withMember("target", Node.from(target.toString()))
                 .withMember("sdkId", Node.from(sdkId))
                 .withMember("arnNamespace", Node.from(getArnNamespace()))
                 .withMember("cloudFormationName", Node.from(getCloudFormationName()))
@@ -227,7 +229,6 @@ public final class ServiceTrait extends AbstractTrait implements ToSmithyBuilder
         } else {
             ServiceTrait os = (ServiceTrait) other;
             return sdkId.equals(os.sdkId)
-                    && target.equals(os.target)
                     && arnNamespace.equals(os.arnNamespace)
                     && cloudFormationName.equals(os.cloudFormationName)
                     && cloudTrailEventSource.equals(os.cloudTrailEventSource)
@@ -240,13 +241,12 @@ public final class ServiceTrait extends AbstractTrait implements ToSmithyBuilder
 
     @Override
     public int hashCode() {
-        return Objects.hash(toShapeId(), target, sdkId, arnNamespace, cloudFormationName,
+        return Objects.hash(toShapeId(), sdkId, arnNamespace, cloudFormationName,
                             cloudTrailEventSource, docId, endpointPrefix);
     }
 
     /** Builder for {@link ServiceTrait}. */
     public static final class Builder extends AbstractTraitBuilder<ServiceTrait, Builder> {
-        private ShapeId target;
         private String sdkId;
         private String cloudFormationName;
         private String arnNamespace;
@@ -266,8 +266,6 @@ public final class ServiceTrait extends AbstractTrait implements ToSmithyBuilder
         }
 
         public ServiceTrait build(ShapeId target) {
-            this.target = target;
-
             // Fill in default values if they weren't set.
             if (arnNamespace == null) {
                 arnNamespace(target.getName().toLowerCase(Locale.US));
@@ -288,17 +286,6 @@ public final class ServiceTrait extends AbstractTrait implements ToSmithyBuilder
             return new ServiceTrait(this);
         }
 
-        /**
-         * Sets the target shape to which the trait is applied.
-         *
-         * @param target the ShapeId targeted by the trait.
-         * @return Returns the builder.
-         */
-        private Builder target(ShapeId target) {
-            this.target = target;
-            return this;
-        }
-
         /**
          * Sets the AWS CloudFormation resource type service name.
          *
diff --git a/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude.smithy b/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude.smithy
index 17dfc9e46..2fc4b2c42 100644
--- a/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude.smithy
+++ b/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude.smithy
@@ -832,7 +832,7 @@ string httpPrefixHeaders
 
 /// Binds a single structure member to the body of an HTTP request.
 @trait(
-    selector: "structure > :test(member > :test(string, blob, structure, union, document, list, map))",
+    selector: "structure > member",
     conflicts: [httpLabel, httpQuery, httpHeader, httpPrefixHeaders, httpResponseCode, httpQueryParams],
     structurallyExclusive: "member",
     breakingChanges: [{change: "presence"}]


Test patch:
diff --git a/smithy-aws-traits/src/test/java/software/amazon/smithy/aws/traits/EventSourceValidatorTest.java b/smithy-aws-traits/src/test/java/software/amazon/smithy/aws/traits/EventSourceValidatorTest.java
index 2515de84e..91906dce3 100644
--- a/smithy-aws-traits/src/test/java/software/amazon/smithy/aws/traits/EventSourceValidatorTest.java
+++ b/smithy-aws-traits/src/test/java/software/amazon/smithy/aws/traits/EventSourceValidatorTest.java
@@ -10,22 +10,20 @@ import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.shapes.ServiceShape;
-import software.amazon.smithy.model.shapes.ShapeId;
 import software.amazon.smithy.model.validation.Severity;
 import software.amazon.smithy.model.validation.ValidationEvent;
 
 public class EventSourceValidatorTest {
     @Test
     public void detectsWhenEventSourceIsUnexpected() {
-        ShapeId id = ShapeId.from("smithy.example#Foo");
         ServiceTrait trait = ServiceTrait.builder()
                 .sdkId("Foo")
                 .arnNamespace("foo")
                 .cloudTrailEventSource("REPLACE_ME_LATER")
                 .cloudFormationName("AWS::Foo")
-                .build(id);
+                .build();
         ServiceShape service = ServiceShape.builder()
-                .id(id)
+                .id("smithy.example#Foo")
                 .version("123")
                 .addTrait(trait)
                 .build();
@@ -41,15 +39,14 @@ public class EventSourceValidatorTest {
 
     @Test
     public void detectsWhenEventSourceIsPlaceholder() {
-        ShapeId id = ShapeId.from("smithy.example#Foo");
         ServiceTrait trait = ServiceTrait.builder()
                 .sdkId("Foo")
                 .arnNamespace("foo")
                 .cloudTrailEventSource("notfoo.amazonaws.com")
                 .cloudFormationName("AWS::Foo")
-                .build(id);
+                .build();
         ServiceShape service = ServiceShape.builder()
-                .id(id)
+                .id("smithy.example#Foo")
                 .version("123")
                 .addTrait(trait)
                 .build();
@@ -66,15 +63,14 @@ public class EventSourceValidatorTest {
 
     @Test
     public void ignoresKnownExceptions() {
-        ShapeId id = ShapeId.from("smithy.example#Foo");
         ServiceTrait trait = ServiceTrait.builder()
                 .sdkId("Foo")
                 .arnNamespace("cloudwatch")
                 .cloudTrailEventSource("monitoring.amazonaws.com")
                 .cloudFormationName("AWS::Foo")
-                .build(id);
+                .build();
         ServiceShape service = ServiceShape.builder()
-                .id(id)
+                .id("smithy.example#Foo")
                 .version("123")
                 .addTrait(trait)
                 .build();
diff --git a/smithy-aws-traits/src/test/java/software/amazon/smithy/aws/traits/ServiceTraitTest.java b/smithy-aws-traits/src/test/java/software/amazon/smithy/aws/traits/ServiceTraitTest.java
index 21bff845d..1d2e870c5 100644
--- a/smithy-aws-traits/src/test/java/software/amazon/smithy/aws/traits/ServiceTraitTest.java
+++ b/smithy-aws-traits/src/test/java/software/amazon/smithy/aws/traits/ServiceTraitTest.java
@@ -16,7 +16,6 @@
 package software.amazon.smithy.aws.traits;
 
 import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -26,6 +25,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
 import java.util.Optional;
 import org.junit.jupiter.api.Test;
 import software.amazon.smithy.model.Model;
+import software.amazon.smithy.model.node.ExpectationNotMetException;
 import software.amazon.smithy.model.node.Node;
 import software.amazon.smithy.model.shapes.ServiceShape;
 import software.amazon.smithy.model.shapes.ShapeId;
@@ -50,8 +50,6 @@ public class ServiceTraitTest {
         assertThat(serviceTrait.getEndpointPrefix(), equalTo("foo"));
         assertThat(serviceTrait.toBuilder().build(), equalTo(serviceTrait));
         assertFalse(serviceTrait.getDocId().isPresent());
-        assertThat(Node.prettyPrintJson(serviceTrait.createNode()),
-                containsString("\"target\": \"ns.foo#Foo\","));
     }
 
     @Test
@@ -89,6 +87,27 @@ public class ServiceTraitTest {
         assertThrows(IllegalStateException.class, () -> ServiceTrait.builder().build());
     }
 
+    @Test
+    public void requiresProperServiceShapeToResolveDocId() {
+        ServiceTrait trait = ServiceTrait.builder()
+                .sdkId("Foo SDK")
+                .arnNamespace("foo")
+                .cloudTrailEventSource("cloudTrailEventSource")
+                .cloudFormationName("AWS::Foo")
+                .build();
+        ServiceShape service = ServiceShape.builder()
+                .id("smithy.example#Foo")
+                .version("123")
+                .addTrait(trait)
+                .build();
+        ServiceShape anotherService = ServiceShape.builder()
+                .id("smithy.example#Bar")
+                .build();
+
+        assertThat(trait.resolveDocId(service), equalTo("foo-sdk-123"));
+        assertThrows(ExpectationNotMetException.class, () -> trait.resolveDocId(anotherService));
+    }
+
     @Test
     public void loadsFromModel() {
         Model result = Model.assembler()
@@ -105,6 +124,6 @@ public class ServiceTraitTest {
         assertThat(trait.getArnNamespace(), equalTo("service"));
         assertThat(trait.getEndpointPrefix(), equalTo("some-service"));
         assertFalse(trait.getDocId().isPresent());
-        assertThat(trait.getDocId(result), equalTo("some-value-2018-03-17"));
+        assertThat(trait.resolveDocId(service), equalTo("some-value-2018-03-17"));
     }
 }
diff --git a/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/protocols/aws-protocols-do-not-support-list-set-map-payloads.errors b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/protocols/aws-protocols-do-not-support-list-set-map-payloads.errors
index 7342a67c1..f581a8f91 100644
--- a/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/protocols/aws-protocols-do-not-support-list-set-map-payloads.errors
+++ b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/protocols/aws-protocols-do-not-support-list-set-map-payloads.errors
@@ -1,3 +1,6 @@
 [ERROR] smithy.example#InvalidBindingOperationInput$listBinding: AWS Protocols only support binding the following shape types to the payload: string, blob, structure, union, and document | ProtocolHttpPayload
 [ERROR] smithy.example#InvalidBindingOperationOutput$mapBinding: AWS Protocols only support binding the following shape types to the payload: string, blob, structure, union, and document | ProtocolHttpPayload
 [ERROR] smithy.example#InvalidBindingError$setBinding: AWS Protocols only support binding the following shape types to the payload: string, blob, structure, union, and document | ProtocolHttpPayload
+[ERROR] smithy.example#InvalidSimpleBindingOperationInput$booleanBinding: AWS Protocols only support binding the following shape types to the payload: string, blob, structure, union, and document | ProtocolHttpPayload
+[ERROR] smithy.example#InvalidSimpleBindingOperationOutput$integerBinding: AWS Protocols only support binding the following shape types to the payload: string, blob, structure, union, and document | ProtocolHttpPayload
+[ERROR] smithy.example#InvalidSimpleBindingError$doubleBinding: AWS Protocols only support binding the following shape types to the payload: string, blob, structure, union, and document | ProtocolHttpPayload
diff --git a/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/protocols/aws-protocols-do-not-support-list-set-map-payloads.smithy b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/protocols/aws-protocols-do-not-support-list-set-map-payloads.smithy
index bc6dc913b..0659896e2 100644
--- a/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/protocols/aws-protocols-do-not-support-list-set-map-payloads.smithy
+++ b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/protocols/aws-protocols-do-not-support-list-set-map-payloads.smithy
@@ -13,7 +13,10 @@ use smithy.api#httpPayload
 @restJson1
 service InvalidExample {
     version: "2020-12-29",
-    operations: [InvalidBindingOperation],
+    operations: [
+        InvalidBindingOperation,
+        InvalidSimpleBindingOperation
+    ],
 }
 
 @http(method: "POST", uri: "/invalid-payload")
@@ -41,6 +44,31 @@ structure InvalidBindingError {
     setBinding: StringSet
 }
 
+@http(method: "POST", uri: "/invalid-simple-payload")
+operation InvalidSimpleBindingOperation {
+    input: InvalidSimpleBindingOperationInput,
+    output: InvalidSimpleBindingOperationOutput,
+    errors: [InvalidSimpleBindingError],
+}
+
+@input
+structure InvalidSimpleBindingOperationInput {
+    @httpPayload
+    booleanBinding: Boolean,
+}
+
+@output
+structure InvalidSimpleBindingOperationOutput {
+    @httpPayload
+    integerBinding: Integer,
+}
+
+@error("client")
+structure InvalidSimpleBindingError {
+    @httpPayload
+    doubleBinding: Double
+}
+
 list StringList {
     member: String
 }