Repository: smithy-lang/smithy
Pull request #2629 (2025-05-12 17:23:25)

Fix declarative endpoint traits toNode
#### Background
Calling `toNode` on standardRegionalEndpoints or standardPartitionalEndpoints can raise NPE when special cases are missing non-required members.

This PR fixes that and adds equals/toHash methods as well.

#### Testing
* Modified existing tests to test that traits converted to node and back are equal.

---
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/smithy-aws-endpoints/src/main/java/software/amazon/smithy/rulesengine/aws/traits/PartitionEndpointSpecialCase.java b/smithy-aws-endpoints/src/main/java/software/amazon/smithy/rulesengine/aws/traits/PartitionEndpointSpecialCase.java
index 0bc71101d..3f8f3868d 100644
--- a/smithy-aws-endpoints/src/main/java/software/amazon/smithy/rulesengine/aws/traits/PartitionEndpointSpecialCase.java
+++ b/smithy-aws-endpoints/src/main/java/software/amazon/smithy/rulesengine/aws/traits/PartitionEndpointSpecialCase.java
@@ -5,6 +5,7 @@
 package software.amazon.smithy.rulesengine.aws.traits;
 
 import java.util.Objects;
+import java.util.Optional;
 import software.amazon.smithy.model.FromSourceLocation;
 import software.amazon.smithy.model.SourceLocation;
 import software.amazon.smithy.model.node.Node;
@@ -77,10 +78,10 @@ public final class PartitionEndpointSpecialCase
     @Override
     public Node toNode() {
         return Node.objectNodeBuilder()
-                .withMember(ENDPOINT, endpoint)
-                .withMember(REGION, region)
-                .withMember(DUAL_STACK, dualStack.toString())
-                .withMember(FIPS, fips.toString())
+                .withOptionalMember(ENDPOINT, Optional.ofNullable(endpoint).map(Node::from))
+                .withOptionalMember(REGION, Optional.ofNullable(region).map(Node::from))
+                .withOptionalMember(DUAL_STACK, Optional.ofNullable(dualStack).map(Node::from))
+                .withOptionalMember(FIPS, Optional.ofNullable(fips).map(Node::from))
                 .build();
     }
 
@@ -99,6 +100,26 @@ public final class PartitionEndpointSpecialCase
         return FromSourceLocation.super.getSourceLocation();
     }
 
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) {
+            return true;
+        }
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
+        PartitionEndpointSpecialCase that = (PartitionEndpointSpecialCase) o;
+        return Objects.equals(endpoint, that.endpoint)
+                && Objects.equals(region, that.region)
+                && Objects.equals(dualStack, that.dualStack)
+                && Objects.equals(fips, that.fips);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(endpoint, region, dualStack, fips);
+    }
+
     /**
      * Creates a {@link PartitionEndpointSpecialCase} instance from the given Node information.
      *
diff --git a/smithy-aws-endpoints/src/main/java/software/amazon/smithy/rulesengine/aws/traits/PartitionSpecialCase.java b/smithy-aws-endpoints/src/main/java/software/amazon/smithy/rulesengine/aws/traits/PartitionSpecialCase.java
index d0fe877d0..d510ca9dd 100644
--- a/smithy-aws-endpoints/src/main/java/software/amazon/smithy/rulesengine/aws/traits/PartitionSpecialCase.java
+++ b/smithy-aws-endpoints/src/main/java/software/amazon/smithy/rulesengine/aws/traits/PartitionSpecialCase.java
@@ -84,6 +84,25 @@ public final class PartitionSpecialCase implements FromSourceLocation, ToNode, T
         return FromSourceLocation.super.getSourceLocation();
     }
 
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) {
+            return true;
+        }
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
+        PartitionSpecialCase that = (PartitionSpecialCase) o;
+        return Objects.equals(endpoint, that.endpoint)
+                && Objects.equals(dualStack, that.dualStack)
+                && Objects.equals(fips, that.fips);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(endpoint, dualStack, fips);
+    }
+
     /**
      * Creates a {@link PartitionSpecialCase} instance from the given Node information.
      *
diff --git a/smithy-aws-endpoints/src/main/java/software/amazon/smithy/rulesengine/aws/traits/RegionSpecialCase.java b/smithy-aws-endpoints/src/main/java/software/amazon/smithy/rulesengine/aws/traits/RegionSpecialCase.java
index 03fe7ffad..63f0f0433 100644
--- a/smithy-aws-endpoints/src/main/java/software/amazon/smithy/rulesengine/aws/traits/RegionSpecialCase.java
+++ b/smithy-aws-endpoints/src/main/java/software/amazon/smithy/rulesengine/aws/traits/RegionSpecialCase.java
@@ -5,6 +5,7 @@
 package software.amazon.smithy.rulesengine.aws.traits;
 
 import java.util.Objects;
+import java.util.Optional;
 import software.amazon.smithy.model.FromSourceLocation;
 import software.amazon.smithy.model.SourceLocation;
 import software.amazon.smithy.model.node.Node;
@@ -75,9 +76,10 @@ public final class RegionSpecialCase implements FromSourceLocation, ToNode, ToSm
     public Node toNode() {
         return Node.objectNodeBuilder()
                 .withMember(ENDPOINT, endpoint)
-                .withMember(DUAL_STACK, dualStack.toString())
-                .withMember(FIPS, fips.toString())
-                .withMember(SIGNING_REGION, signingRegion)
+                .withOptionalMember(DUAL_STACK, Optional.ofNullable(dualStack).map(Node::from))
+                .withOptionalMember(FIPS, Optional.ofNullable(fips).map(Node::from))
+                .withOptionalMember(DUAL_STACK, Optional.ofNullable(dualStack).map(Node::from))
+                .withOptionalMember(SIGNING_REGION, Optional.ofNullable(signingRegion).map(Node::from))
                 .build();
     }
 
@@ -96,6 +98,26 @@ public final class RegionSpecialCase implements FromSourceLocation, ToNode, ToSm
         return FromSourceLocation.super.getSourceLocation();
     }
 
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) {
+            return true;
+        }
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
+        RegionSpecialCase that = (RegionSpecialCase) o;
+        return Objects.equals(endpoint, that.endpoint)
+                && Objects.equals(dualStack, that.dualStack)
+                && Objects.equals(fips, that.fips)
+                && Objects.equals(signingRegion, that.signingRegion);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(endpoint, dualStack, fips, signingRegion);
+    }
+
     /**
      * Creates a {@link RegionSpecialCase} instance from the given Node information.
      *
diff --git a/smithy-aws-endpoints/src/main/java/software/amazon/smithy/rulesengine/aws/traits/StandardPartitionalEndpointsTrait.java b/smithy-aws-endpoints/src/main/java/software/amazon/smithy/rulesengine/aws/traits/StandardPartitionalEndpointsTrait.java
index 1c75f188a..d061de51d 100644
--- a/smithy-aws-endpoints/src/main/java/software/amazon/smithy/rulesengine/aws/traits/StandardPartitionalEndpointsTrait.java
+++ b/smithy-aws-endpoints/src/main/java/software/amazon/smithy/rulesengine/aws/traits/StandardPartitionalEndpointsTrait.java
@@ -77,10 +77,29 @@ public final class StandardPartitionalEndpointsTrait extends AbstractTrait
     @Override
     public Builder toBuilder() {
         return new Builder()
+                .sourceLocation(getSourceLocation())
                 .partitionEndpointSpecialCases(partitionEndpointSpecialCases)
                 .endpointPatternType(endpointPatternType);
     }
 
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) {
+            return true;
+        }
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
+        StandardPartitionalEndpointsTrait that = (StandardPartitionalEndpointsTrait) o;
+        return partitionEndpointSpecialCases.equals(that.partitionEndpointSpecialCases)
+                && endpointPatternType.equals(that.endpointPatternType);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(partitionEndpointSpecialCases, endpointPatternType);
+    }
+
     public static Builder builder() {
         return new Builder();
     }
diff --git a/smithy-aws-endpoints/src/main/java/software/amazon/smithy/rulesengine/aws/traits/StandardRegionalEndpointsTrait.java b/smithy-aws-endpoints/src/main/java/software/amazon/smithy/rulesengine/aws/traits/StandardRegionalEndpointsTrait.java
index bf70d6c13..2ff8adfaa 100644
--- a/smithy-aws-endpoints/src/main/java/software/amazon/smithy/rulesengine/aws/traits/StandardRegionalEndpointsTrait.java
+++ b/smithy-aws-endpoints/src/main/java/software/amazon/smithy/rulesengine/aws/traits/StandardRegionalEndpointsTrait.java
@@ -8,6 +8,7 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import software.amazon.smithy.model.node.Node;
 import software.amazon.smithy.model.node.ObjectNode;
 import software.amazon.smithy.model.shapes.ShapeId;
@@ -84,10 +85,29 @@ public final class StandardRegionalEndpointsTrait extends AbstractTrait
     @Override
     public Builder toBuilder() {
         return new Builder()
+                .sourceLocation(getSourceLocation())
                 .partitionSpecialCases(partitionSpecialCases)
                 .regionSpecialCases(regionSpecialCases);
     }
 
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) {
+            return true;
+        }
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
+        StandardRegionalEndpointsTrait that = (StandardRegionalEndpointsTrait) o;
+        return partitionSpecialCases.equals(that.partitionSpecialCases)
+                && regionSpecialCases.equals(that.regionSpecialCases);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(partitionSpecialCases, regionSpecialCases);
+    }
+
     public static Builder builder() {
         return new Builder();
     }


Test patch:
diff --git a/smithy-aws-endpoints/src/test/java/software/amazon/smithy/rulesengine/aws/traits/StandardPartitionalEndpointsTraitTest.java b/smithy-aws-endpoints/src/test/java/software/amazon/smithy/rulesengine/aws/traits/StandardPartitionalEndpointsTraitTest.java
index 25c19dc51..e41de3964 100644
--- a/smithy-aws-endpoints/src/test/java/software/amazon/smithy/rulesengine/aws/traits/StandardPartitionalEndpointsTraitTest.java
+++ b/smithy-aws-endpoints/src/test/java/software/amazon/smithy/rulesengine/aws/traits/StandardPartitionalEndpointsTraitTest.java
@@ -27,6 +27,10 @@ class StandardPartitionalEndpointsTraitTest {
 
         assertEquals(trait.getEndpointPatternType(), EndpointPatternType.SERVICE_DNSSUFFIX);
         assertEquals(trait.getPartitionEndpointSpecialCases().size(), 0);
+        assertEquals(trait,
+                new StandardPartitionalEndpointsTrait.Provider()
+                        .createTrait(StandardPartitionalEndpointsTrait.ID,
+                                trait.toBuilder().build().toNode()));
 
         trait = getTraitFromService(model, "ns.foo#Service2");
 
@@ -47,6 +51,11 @@ class StandardPartitionalEndpointsTraitTest {
         assertEquals(case2.getRegion(), "us-west-2");
         assertNull(case2.getFips());
 
+        assertEquals(trait,
+                new StandardPartitionalEndpointsTrait.Provider()
+                        .createTrait(StandardPartitionalEndpointsTrait.ID,
+                                trait.toBuilder().build().toNode()));
+
         trait = getTraitFromService(model, "ns.foo#Service3");
 
         assertEquals(trait.getEndpointPatternType(), EndpointPatternType.AWS_RECOMMENDED);
@@ -59,6 +68,11 @@ class StandardPartitionalEndpointsTraitTest {
         assertEquals(case1.getRegion(), "us-west-2");
         assertNull(case1.getDualStack());
         assertNull(case1.getFips());
+
+        assertEquals(trait,
+                new StandardPartitionalEndpointsTrait.Provider()
+                        .createTrait(StandardPartitionalEndpointsTrait.ID,
+                                trait.toBuilder().build().toNode()));
     }
 
     private StandardPartitionalEndpointsTrait getTraitFromService(Model model, String service) {
diff --git a/smithy-aws-endpoints/src/test/java/software/amazon/smithy/rulesengine/aws/traits/StandardRegionalEndpointsTraitTest.java b/smithy-aws-endpoints/src/test/java/software/amazon/smithy/rulesengine/aws/traits/StandardRegionalEndpointsTraitTest.java
index cf6136cfd..2f5ce15d9 100644
--- a/smithy-aws-endpoints/src/test/java/software/amazon/smithy/rulesengine/aws/traits/StandardRegionalEndpointsTraitTest.java
+++ b/smithy-aws-endpoints/src/test/java/software/amazon/smithy/rulesengine/aws/traits/StandardRegionalEndpointsTraitTest.java
@@ -27,11 +27,17 @@ class StandardRegionalEndpointsTraitTest {
 
         assertEquals(trait.getPartitionSpecialCases().size(), 0);
         assertEquals(trait.getRegionSpecialCases().size(), 0);
+        assertEquals(trait,
+                new StandardRegionalEndpointsTrait.Provider().createTrait(StandardRegionalEndpointsTrait.ID,
+                        trait.toBuilder().build().toNode()));
 
         trait = getTraitFromService(model, "ns.foo#Service2");
 
         assertEquals(trait.getPartitionSpecialCases().size(), 0);
         assertEquals(trait.getRegionSpecialCases().size(), 0);
+        assertEquals(trait,
+                new StandardRegionalEndpointsTrait.Provider().createTrait(StandardRegionalEndpointsTrait.ID,
+                        trait.toBuilder().build().toNode()));
 
         trait = getTraitFromService(model, "ns.foo#Service3");
 
@@ -50,7 +56,13 @@ class StandardRegionalEndpointsTraitTest {
         assertNull(partitionSpecialCase2.getFips());
 
         List<RegionSpecialCase> regionSpecialCases = trait.getRegionSpecialCases().get("us-east-1");
-        assertEquals(regionSpecialCases.size(), 0);
+        assertEquals(regionSpecialCases.size(), 1);
+        RegionSpecialCase regionSpecialCase = regionSpecialCases.get(0);
+        assertEquals(regionSpecialCase.getEndpoint(), "https://myservice.amazonaws.com");
+        assertEquals(regionSpecialCase.getDualStack(), true);
+        assertEquals(trait,
+                new StandardRegionalEndpointsTrait.Provider().createTrait(StandardRegionalEndpointsTrait.ID,
+                        trait.toBuilder().build().toNode()));
 
         Node.assertEquals(trait.toNode(), trait.toBuilder().build().toNode());
     }
diff --git a/smithy-aws-endpoints/src/test/resources/software/amazon/smithy/rulesengine/aws/traits/standardRegionalEndpoints.smithy b/smithy-aws-endpoints/src/test/resources/software/amazon/smithy/rulesengine/aws/traits/standardRegionalEndpoints.smithy
index 37bf9547a..1f3d74ecc 100644
--- a/smithy-aws-endpoints/src/test/resources/software/amazon/smithy/rulesengine/aws/traits/standardRegionalEndpoints.smithy
+++ b/smithy-aws-endpoints/src/test/resources/software/amazon/smithy/rulesengine/aws/traits/standardRegionalEndpoints.smithy
@@ -29,6 +29,10 @@ service Service2 {
     },
     regionSpecialCases: {
         "us-east-1": [
+            {
+                endpoint: "https://myservice.amazonaws.com",
+                dualStack: true
+            }
         ]
     }
 )