Repository: smithy-lang/smithy
Pull request #2405 (2024-09-25 15:20:33)

fix: Don't assume that all service bindings existed before
#### Background
* What do these changes do? 

I changed `model.expectShape` to `model.getShape` to so that the code doesn't fail if a given service is not present in the old model which can happen in certain cases. 

* Why are they important?

This is a fix for `software.amazon.smithy.model.node.ExpectationNotMetException: Shape not found in model: a.b#c` that occurs when a new service is being added that references an already existing operation which was also changed in some way. 

Full stacktrace:
```
 at software.amazon.smithy.model.Model.lambda$expectShape$1(Model.java:713)
        at java.base/java.util.Optional.orElseThrow(Optional.java:403)
        at software.amazon.smithy.model.Model.expectShape(Model.java:713)
        at software.amazon.smithy.model.Model.expectShape(Model.java:729)
        at software.amazon.smithy.aws.traits.auth.diff.SigV4Migration.evaluate(SigV4Migration.java:86)
        at software.amazon.smithy.diff.ModelDiff$Builder.lambda$compare$2(ModelDiff.java:293)
        at java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:273)
        at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1625)
        at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
        at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
        at java.base/java.util.stream.ReduceOps$ReduceTask.doLeaf(ReduceOps.java:960)
        at java.base/java.util.stream.ReduceOps$ReduceTask.doLeaf(ReduceOps.java:934)
        at java.base/java.util.stream.AbstractTask.compute(AbstractTask.java:327)
        at java.base/java.util.concurrent.CountedCompleter.exec(CountedCompleter.java:754)
        at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:373)
        at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1182)
        at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1655)
        at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1622)
        at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:165)
``` 

#### Testing
* How did you test these changes?

I minimized the issue and then reproduced it in the unit test. 

#### Links
* Links to additional context, if necessary
* Issue #, if applicable (see [here](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword) for a list of keywords to use for linking issues)

`SigV4Migration` was introduced in https://github.com/smithy-lang/smithy/pull/2245

---
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-2.0/spec/json-ast.rst b/docs/source-2.0/spec/json-ast.rst
index 0f975d0e7..d981a9637 100644
--- a/docs/source-2.0/spec/json-ast.rst
+++ b/docs/source-2.0/spec/json-ast.rst
@@ -414,8 +414,8 @@ shapes defined in JSON support the same properties as the Smithy IDL.
       - ``service``
     * - version
       - ``string``
-      - **Required**. Defines the version of the service. The version can be
-        provided in any format (e.g., ``2017-02-11``, ``2.0``, etc).
+      - Defines the version of the service. The version can be provided in any
+        format (e.g., ``2017-02-11``, ``2.0``, etc).
     * - :ref:`operations <service-operations>`
       - [:ref:`AST shape reference <ast-shape-reference>`]
       - Binds a list of operations to the service. Each reference MUST target
diff --git a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/auth/diff/SigV4Migration.java b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/auth/diff/SigV4Migration.java
index 890c9fda8..87e91abf1 100644
--- a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/auth/diff/SigV4Migration.java
+++ b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/auth/diff/SigV4Migration.java
@@ -83,23 +83,27 @@ public final class SigV4Migration extends AbstractDiffEvaluator {
             }
             // Validate Operation effective auth schemes
             for (ServiceShape newServiceShape : newOperationServiceBindings) {
-                ServiceShape oldServiceShape = oldModel.expectShape(newServiceShape.getId(), ServiceShape.class);
-                OperationShape oldOperationShape = change.getOldShape();
-                List<ShapeId> oldAuthSchemes = oldServiceIndex
-                    .getEffectiveAuthSchemes(oldServiceShape, oldOperationShape)
-                    .keySet()
-                    .stream()
-                    .collect(Collectors.toList());
-                List<ShapeId> newAuthSchemes = newServiceIndex
-                    .getEffectiveAuthSchemes(newServiceShape, newOperationShape)
-                    .keySet()
-                    .stream()
-                    .collect(Collectors.toList());
-                validateMigration(
-                    newOperationShape,
-                    oldAuthSchemes,
-                    newAuthSchemes,
-                    events);
+                oldModel.getShape(newServiceShape.getId())
+                    .filter(Shape::isServiceShape)
+                    .map(s -> (ServiceShape) s)
+                    .ifPresent(oldServiceShape -> {
+                        OperationShape oldOperationShape = change.getOldShape();
+                        List<ShapeId> oldAuthSchemes = oldServiceIndex
+                            .getEffectiveAuthSchemes(oldServiceShape, oldOperationShape)
+                            .keySet()
+                            .stream()
+                            .collect(Collectors.toList());
+                        List<ShapeId> newAuthSchemes = newServiceIndex
+                            .getEffectiveAuthSchemes(newServiceShape, newOperationShape)
+                            .keySet()
+                            .stream()
+                            .collect(Collectors.toList());
+                        validateMigration(
+                            newOperationShape,
+                            oldAuthSchemes,
+                            newAuthSchemes,
+                            events);
+                    });
             }
         }
 


Test patch:
diff --git a/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/diffs/no-old-sigv4-new-service.a.smithy b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/diffs/no-old-sigv4-new-service.a.smithy
new file mode 100644
index 000000000..e75d0177d
--- /dev/null
+++ b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/diffs/no-old-sigv4-new-service.a.smithy
@@ -0,0 +1,21 @@
+$version: "2.0"
+
+metadata suppressions = [
+    {
+        id: "ModifiedTrait.Update.smithy.api#auth"
+        namespace: "ns.foo"
+    }
+]
+
+namespace ns.foo
+
+@auth([])
+@httpBearerAuth
+service Service {
+    operations: [
+        Operation
+    ]
+}
+
+@auth([])
+operation Operation {}
diff --git a/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/diffs/no-old-sigv4-new-service.b.smithy b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/diffs/no-old-sigv4-new-service.b.smithy
new file mode 100644
index 000000000..44870b7e7
--- /dev/null
+++ b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/diffs/no-old-sigv4-new-service.b.smithy
@@ -0,0 +1,30 @@
+$version: "2.0"
+
+metadata suppressions = [
+    {
+        id: "ModifiedTrait.Update.smithy.api#auth"
+        namespace: "ns.foo"
+    }
+]
+
+namespace ns.foo
+
+@auth([httpBearerAuth])
+@httpBearerAuth
+service Service {
+    operations: [
+        Operation
+    ]
+}
+
+@auth([httpBearerAuth])
+@httpBearerAuth
+service NewService {
+    operations: [
+        Operation
+    ]
+}
+
+
+@auth([httpBearerAuth])
+operation Operation {}
diff --git a/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/diffs/no-old-sigv4-new-service.events b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/diffs/no-old-sigv4-new-service.events
new file mode 100644
index 000000000..7ebdd3d2d
--- /dev/null
+++ b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/diffs/no-old-sigv4-new-service.events
@@ -0,0 +1,1 @@
+[NOTE] ns.foo#NewService: Added service `ns.foo#NewService` | AddedShape
diff --git a/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/http-checksum-trait.errors b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/http-checksum-trait.errors
index 750e98437..361bfb9f0 100644
--- a/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/http-checksum-trait.errors
+++ b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/http-checksum-trait.errors
@@ -3,5 +3,6 @@
 [SUPPRESSED] smithy.example#NoOutputForResponse: This shape applies a trait that is unstable: aws.protocols#httpChecksum | UnstableTrait
 [SUPPRESSED] smithy.example#NoResponseAlgorithms: This shape applies a trait that is unstable: aws.protocols#httpChecksum | UnstableTrait
 [ERROR] smithy.example#NoBehavior: The `httpChecksum` trait must define at least one of the `request` or `response` checksum behaviors. | HttpChecksumTrait
+[ERROR] smithy.example#NoBehaviorRequestChecksumRequiredFalse: The `httpChecksum` trait must define at least one of the `request` or `response` checksum behaviors. | HttpChecksumTrait
 [ERROR] smithy.example#NoModeForResponse: The `httpChecksum` trait must model the `requestValidationModeMember` property to support response checksum behavior. | HttpChecksumTrait
 [ERROR] smithy.example#NoResponseAlgorithms: The `httpChecksum` trait must model the `responseAlgorithms` property to support response checksum behavior. | HttpChecksumTrait
diff --git a/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/http-checksum-trait.smithy b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/http-checksum-trait.smithy
index 24ce4cf32..4710c018c 100644
--- a/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/http-checksum-trait.smithy
+++ b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/http-checksum-trait.smithy
@@ -14,6 +14,18 @@ operation NoBehavior {
 @input
 structure NoBehaviorInput {}
 
+@httpChecksum(
+    requestChecksumRequired: false,
+)
+@suppress(["UnstableTrait"])
+operation NoBehaviorRequestChecksumRequiredFalse {
+    input: NoBehaviorRequestChecksumRequiredFalseInput,
+    output: Unit
+}
+
+@input
+structure NoBehaviorRequestChecksumRequiredFalseInput {}
+
 @httpChecksum(
     requestChecksumRequired: true,
 )