Update docId resolution method (smithy-lang#1882)

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.

Author: Jose Vitor Schneid

smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/ServiceTrait.java

@@ -19,8 +19,8 @@
 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 String getCloudTrailEventSource() {
     }
 
     /**
-     * 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 Optional<String> getAbbreviation() {
     @Override
     public Builder toBuilder() {
         return new Builder()
-                .target(target)
                 .sdkId(sdkId)
                 .sourceLocation(getSourceLocation())
                 .cloudFormationName(cloudFormationName)
@@ -206,7 +209,6 @@ public Builder toBuilder() {
     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 boolean equals(Object other) {
         } 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 boolean equals(Object other) {
 
     @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 ServiceTrait build() {
         }
 
         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 ServiceTrait build(ShapeId target) {
             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.
          *

smithy-aws-traits/src/test/java/software/amazon/smithy/aws/traits/EventSourceValidatorTest.java

@@ -10,22 +10,20 @@
 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 void detectsWhenEventSourceIsUnexpected() {
 
     @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 void detectsWhenEventSourceIsPlaceholder() {
 
     @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();

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 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 void loadsTraitWithString() {
         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 void requiresSdkServiceId() {
         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 void loadsFromModel() {
         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"));
     }
 }