Make streaming blobs required / default

Author: JordonPhillips

docs/source/1.0/guides/migrating-idl-1-to-2.rst

@@ -119,6 +119,60 @@ Needs to be updated to:
     }
 
 
+Add the default trait to streaming blobs
+========================================
+
+Members that target a blob shape with the :ref:`streaming-trait` have always
+had an implicit default empty value. In IDL 2.0, that will become explicit.
+Any such members that are not already marked with the :ref:`required-trait`
+will now need to be marked with the :ref:`default-trait`.
+
+For example, the following model:
+
+.. code-block:: smithy
+
+    $version: "1.0"
+
+    namespace smithy.example
+
+    structure OptionalStream {
+        // This needs to be updated since it doesn't have the required or
+        // default trait already.
+        payload: StreamingBlob
+    }
+
+    structure RequiredStream {
+        // This doesn't need to be updated because it already has the required
+        // trait.
+        @required
+        payload: StreamingBlob
+    }
+
+    @streaming
+    blob StreamingBlob
+
+Needs to be updated to:
+
+.. code-block:: smithy
+
+    $version: "2.0"
+
+    namespace smithy.example
+
+    structure OptionalStream {
+        @default
+        payload: StreamingBlob
+    }
+
+    structure RequiredStream {
+        @required
+        payload: StreamingBlob
+    }
+
+    @streaming
+    blob StreamingBlob
+
+
 Optional migration steps
 ========================
 

docs/source/1.0/spec/core/traits/behavior-traits.rst.template

@@ -538,6 +538,10 @@ Summary
     large and thus should not be stored in memory or that the size is unknown
     at the start of the request.
 
+    .. warning::
+        Members targeting streaming blobs MUST be marked with the
+        :ref:`required-trait` or :ref:`default-trait`.
+
     When applied to a union, it indicates that shape represents an
     :ref:`event stream <event-streams>`.
 Trait selector::

docs/source/1.0/spec/core/traits/http-traits.rst.template

@@ -708,6 +708,7 @@ Trait selector
     The ``httpPayload`` trait can be applied to ``structure`` members that
     target a ``string``, ``blob``, ``structure``, ``union``, ``document``,
     ``set``, ``map``, or ``list``.
+
 Value type
     Annotation trait.
 Conflicts with

smithy-aws-protocol-tests/model/restJson1/services/glacier.smithy

@@ -206,6 +206,7 @@ structure UploadArchiveInput {
     archiveDescription: string,
     @httpHeader("x-amz-sha256-tree-hash")
     checksum: string,
+    @default
     @httpPayload
     body: Stream,
 }
@@ -224,6 +225,7 @@ structure UploadMultipartPartInput {
     checksum: string,
     @httpHeader("Content-Range")
     range: string,
+    @default
     @httpPayload
     body: Stream,
 }

smithy-aws-protocol-tests/model/restJson1/streaming.smithy

@@ -91,6 +91,7 @@ structure StreamingTraitsInputOutput {
     @httpHeader("X-Foo")
     foo: String,
 
+    @default
     @httpPayload
     blob: StreamingBlob,
 }
@@ -151,6 +152,7 @@ structure StreamingTraitsRequireLengthInput {
     @httpHeader("X-Foo")
     foo: String,
 
+    @default
     @httpPayload
     blob: FiniteStreamingBlob,
 }
@@ -212,6 +214,7 @@ structure StreamingTraitsWithMediaTypeInputOutput {
     @httpHeader("X-Foo")
     foo: String,
 
+    @default
     @httpPayload
     blob: StreamingTextPlainBlob,
 }

smithy-cli/src/main/java/software/amazon/smithy/cli/commands/Upgrade1to2Command.java

@@ -28,6 +28,7 @@
 import java.util.EnumSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import java.util.function.BiConsumer;
 import java.util.function.Consumer;
@@ -54,6 +55,7 @@
 import software.amazon.smithy.model.shapes.ShapeVisitor;
 import software.amazon.smithy.model.shapes.SmithyIdlModelSerializer;
 import software.amazon.smithy.model.shapes.StringShape;
+import software.amazon.smithy.model.traits.AnnotationTrait;
 import software.amazon.smithy.model.traits.BoxTrait;
 import software.amazon.smithy.model.traits.DefaultTrait;
 import software.amazon.smithy.model.traits.EnumTrait;
@@ -313,15 +315,13 @@ private void replacePrimitiveTarget(MemberShape member) {
         }
 
         private boolean hasSyntheticDefault(MemberShape shape) {
-            Shape target = completeModel.expectShape(shape.getTarget());
-            if (!(HAD_DEFAULT_VALUE_IN_1_0.contains(target.getType()) && shape.hasTrait(DefaultTrait.class))) {
-                return false;
-            }
+            Optional<SourceLocation> defaultLocation = shape.getTrait(DefaultTrait.class)
+                    .map(AnnotationTrait::getSourceLocation);
             // When Smithy injects the default trait, it sets the source
             // location equal to the shape's source location. This is
             // impossible in any other scenario, so we can use this info
             // to know whether it was injected or not.
-            return shape.getSourceLocation().equals(shape.expectTrait(DefaultTrait.class).getSourceLocation());
+            return defaultLocation.filter(location -> shape.getSourceLocation().equals(location)).isPresent();
         }
 
         @Override

smithy-cli/src/test/java/software/amazon/smithy/cli/commands/Upgrade1to2CommandTest.java

@@ -57,7 +57,7 @@ public void testUpgrade(Path initialPath, String name) {
     public static Stream<Arguments> source() throws Exception {
         Path start = Paths.get(Upgrade1to2CommandTest.class.getResource("upgrade/cases").toURI());
         return Files.walk(start)
-                .filter(path -> path.getFileName().toString().endsWith("enum-with-traits.v1.smithy"))
+                .filter(path -> path.getFileName().toString().endsWith(".v1.smithy"))
                 .map(path -> Arguments.of(path, path.getFileName().toString().replace(".v1.smithy", "")));
     }
 

smithy-cli/src/test/resources/software/amazon/smithy/cli/commands/upgrade/cases/http-payload.v1.smithy

@@ -0,0 +1,20 @@
+$version: "1.0"
+
+namespace smithy.example
+
+structure RequiredPayload {
+    @required
+    payload: StreamingBlob
+}
+
+structure DefaultPayload {
+    @default
+    payload: StreamingBlob
+}
+
+structure NeitherRequiredNorDefaultPayload {
+    payload: StreamingBlob
+}
+
+@streaming
+blob StreamingBlob

smithy-cli/src/test/resources/software/amazon/smithy/cli/commands/upgrade/cases/http-payload.v2.smithy

@@ -0,0 +1,21 @@
+$version: "2.0"
+
+namespace smithy.example
+
+structure RequiredPayload {
+    @required
+    payload: StreamingBlob
+}
+
+structure DefaultPayload {
+    @default
+    payload: StreamingBlob
+}
+
+structure NeitherRequiredNorDefaultPayload {
+    @default
+    payload: StreamingBlob
+}
+
+@streaming
+blob StreamingBlob

smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelUpgrader.java

@@ -29,6 +29,7 @@
 import software.amazon.smithy.model.traits.BoxTrait;
 import software.amazon.smithy.model.traits.DefaultTrait;
 import software.amazon.smithy.model.traits.RequiredTrait;
+import software.amazon.smithy.model.traits.StreamingTrait;
 import software.amazon.smithy.model.transform.ModelTransformer;
 import software.amazon.smithy.model.validation.Severity;
 import software.amazon.smithy.model.validation.ValidatedResult;
@@ -134,8 +135,10 @@ private void upgradeV1Member(ShapeType containerType, MemberShape member) {
 
     private boolean shouldV1MemberHaveDefaultTrait(ShapeType containerType, MemberShape member, Shape target) {
         return containerType == ShapeType.STRUCTURE
-            // Only when the targeted shape had a default value by default in v1.
-            && HAD_DEFAULT_VALUE_IN_1_0.contains(target.getType())
+            // Only when the targeted shape had a default value by default in v1 or if
+            // the member has the http payload trait and targets a streaming blob, which
+            // implies a default in 2.0
+            && (HAD_DEFAULT_VALUE_IN_1_0.contains(target.getType()) || isDefaultPayload(target))
             // Not when the targeted shape was one of the prelude types with the @box trait.
             // This needs special handling here because the @box trait was removed from these
             // prelude shapes in v2.
@@ -150,6 +153,10 @@ private boolean shouldV1MemberHaveDefaultTrait(ShapeType containerType, MemberSh
             && !target.hasTrait(BoxTrait.class);
     }
 
+    private boolean isDefaultPayload(Shape target) {
+        return target.hasTrait(StreamingTrait.class) && target.isBlobShape();
+    }
+
     private MemberShape.Builder createOrReuseBuilder(MemberShape member, MemberShape.Builder builder) {
         return builder == null ? member.toBuilder() : builder;
     }

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

@@ -35,8 +35,10 @@
 import software.amazon.smithy.model.shapes.Shape;
 import software.amazon.smithy.model.shapes.ShapeId;
 import software.amazon.smithy.model.shapes.UnionShape;
+import software.amazon.smithy.model.traits.DefaultTrait;
 import software.amazon.smithy.model.traits.HttpPayloadTrait;
 import software.amazon.smithy.model.traits.ProtocolDefinitionTrait;
+import software.amazon.smithy.model.traits.RequiredTrait;
 import software.amazon.smithy.model.traits.RequiresLengthTrait;
 import software.amazon.smithy.model.traits.StreamingTrait;
 import software.amazon.smithy.model.validation.AbstractValidator;
@@ -63,9 +65,21 @@ public List<ValidationEvent> validate(Model model) {
         validateStreamingTargets(model, events);
         validateAllEventStreamMembersTargetStructures(model, events);
         validateBlobTargetsArePayloads(model, events);
+        validateBlobTargetsAreNonOptional(model, events);
         return events;
     }
 
+    private void validateBlobTargetsAreNonOptional(Model model, List<ValidationEvent> events) {
+        for (MemberShape member : model.toSet(MemberShape.class)) {
+            Shape target = model.expectShape(member.getTarget());
+            if (target.isBlobShape() && target.hasTrait(StreamingTrait.class)
+                    && !(member.hasTrait(RequiredTrait.class) || member.hasTrait(DefaultTrait.class))) {
+                events.add(error(member, "Members that target blobs marked with the `streaming` trait MUST also be "
+                        + "marked with the `required` or `default` trait."));
+            }
+        }
+    }
+
     private void validateBlobTargetsArePayloads(Model model, List<ValidationEvent> events) {
         ServiceIndex serviceIndex = ServiceIndex.of(model);
         Set<ServiceShape> servicesWithPayloadSupportingProtocols = new HashSet<>();

smithy-model/src/test/java/software/amazon/smithy/model/knowledge/HttpBindingIndexTest.java

@@ -23,7 +23,7 @@
 
 import java.util.Map;
 import java.util.Optional;
-import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.SourceLocation;
@@ -51,10 +51,15 @@
 
 public class HttpBindingIndexTest {
 
-    private static Model model = Model.assembler()
-            .addImport(HttpBindingIndex.class.getResource("http-index.json"))
-            .assemble()
-            .unwrap();
+    private static Model model;
+
+    @BeforeAll
+    private static void readModel() {
+        model = Model.assembler()
+                .addImport(HttpBindingIndex.class.getResource("http-index.json"))
+                .assemble()
+                .unwrap();
+    }
 
     @Test
     public void providesResponseCode() {

smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelUpgraderTest.java

@@ -45,6 +45,8 @@ public void upgradesWhenAllModelsUse1_0() {
         assertThat(ShapeId.from("smithy.example#Bytes$nonNull"), not(ShapeMatcher.memberIsNullable(result)));
         assertThat(ShapeId.from("smithy.example#Shorts$nonNull"), not(ShapeMatcher.memberIsNullable(result)));
         assertThat(ShapeId.from("smithy.example#Integers$nonNull"), addedDefaultTrait(result));
+
+        assertThat(ShapeId.from("smithy.example#BlobPayload$payload"), addedDefaultTrait(result));
     }
 
     @Test

smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/requiresLength-validator.json

@@ -17,7 +17,10 @@
             "type": "structure",
             "members": {
                 "Body": {
-                    "target": "ns.foo#StreamingBlob"
+                    "target": "ns.foo#StreamingBlob",
+                    "traits": {
+                        "smithy.api#default": {}
+                    }
                 }
             },
             "traits": {
@@ -28,7 +31,10 @@
             "type": "structure",
             "members": {
                 "Body": {
-                    "target": "ns.foo#StreamingBlob"
+                    "target": "ns.foo#StreamingBlob",
+                    "traits": {
+                        "smithy.api#default": {}
+                    }
                 }
             },
             "traits": {

smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/streaming-blob-without-httppayload.smithy

@@ -29,6 +29,8 @@ structure StreamingOperationInput {}
 structure StreamingOperationOutput {
     @required
     streamId: String,
+
+    @default
     output: StreamingBlob,
 }
 

smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/streaming-default-required.errors

@@ -0,0 +1 @@
+[ERROR] smithy.example#NeitherRequiredNorDefaultStream$payload: Members that target blobs marked with the `streaming` trait MUST also be marked with the `required` or `default` trait. | StreamingTrait

smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/streaming-default-required.smithy

@@ -0,0 +1,35 @@
+$version: "2.0"
+
+namespace smithy.example
+
+@http(uri: "/", method: "POST")
+operation RequiredStreamOperation {
+    input: RequiredStream
+}
+
+structure RequiredStream {
+    @required
+    payload: StreamingBlob
+}
+
+@http(uri: "/", method: "POST")
+operation DefaultStreamOperation {
+    input: DefaultStream
+}
+
+structure DefaultStream {
+    @default
+    payload: StreamingBlob
+}
+
+@http(uri: "/", method: "POST")
+operation NeitherRequiredNorDefaultStreamOperation {
+    input: NeitherRequiredNorDefaultStream
+}
+
+structure NeitherRequiredNorDefaultStream {
+    payload: StreamingBlob
+}
+
+@streaming
+blob StreamingBlob