Make setting a service version optional

Not all use cases for Smithy modelt s can provide a service version. For
example, when using Smithy to generate libraries, not client/server
interactions, there's not really a version negotiation, so no version is
necessary in the model definition.

docs/source/1.0/spec/core/model.rst

@@ -1002,8 +1002,8 @@ The service shape supports the following properties:
       - Description
     * - 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 optional version of the service. The version can be provided in
+        any format (e.g., ``2017-02-11``, ``2.0``, etc).
     * - :ref:`operations <service-operations>`
       - [``string``]
       - Binds a set of ``operation`` shapes to the service. Each

smithy-model/src/main/java/software/amazon/smithy/model/shapes/ModelSerializer.java

@@ -34,6 +34,7 @@
 import software.amazon.smithy.utils.FunctionalUtils;
 import software.amazon.smithy.utils.Pair;
 import software.amazon.smithy.utils.SmithyBuilder;
+import software.amazon.smithy.utils.StringUtils;
 
 /**
  * Serializes a {@link Model} to an {@link ObjectNode}.
@@ -143,54 +144,58 @@ public ModelSerializer build() {
 
     private final class ShapeSerializer extends ShapeVisitor.Default<Node> {
 
-        private ObjectNode createTypedNode(Shape shape) {
-            return Node.objectNode().withMember("type", Node.from(shape.getType().toString()));
+        private ObjectNode.Builder createTypedNode(Shape shape) {
+            return Node.objectNodeBuilder().withMember("type", Node.from(shape.getType().toString()));
         }
 
-        private ObjectNode withTraits(Shape shape, ObjectNode node) {
+        private ObjectNode.Builder withTraits(Shape shape, ObjectNode.Builder shapeBuilder) {
             if (shape.getAllTraits().isEmpty()) {
-                return node;
+                return shapeBuilder;
             }
 
-            ObjectNode.Builder builder = Node.objectNodeBuilder();
+            ObjectNode.Builder traitBuilder = Node.objectNodeBuilder();
             shape.getAllTraits().values().stream()
                     .filter(traitFilter)
                     .sorted(Comparator.comparing(Trait::toShapeId))
-                    .forEach(trait -> builder.withMember(trait.toShapeId().toString(), trait.toNode()));
+                    .forEach(trait -> traitBuilder.withMember(trait.toShapeId().toString(), trait.toNode()));
 
-            return node.withMember("traits", builder.build());
+            return shapeBuilder.withMember("traits", traitBuilder.build());
         }
 
         @Override
         protected ObjectNode getDefault(Shape shape) {
-            return withTraits(shape, createTypedNode(shape));
+            return withTraits(shape, createTypedNode(shape)).build();
         }
 
         @Override
         public Node listShape(ListShape shape) {
             return withTraits(shape, createTypedNode(shape)
-                    .withMember("member", shape.getMember().accept(this)));
+                    .withMember("member", shape.getMember().accept(this)))
+                    .build();
         }
 
         @Override
         public Node setShape(SetShape shape) {
             return withTraits(shape, createTypedNode(shape)
-                    .withMember("member", shape.getMember().accept(this)));
+                    .withMember("member", shape.getMember().accept(this)))
+                    .build();
         }
 
         @Override
         public Node mapShape(MapShape shape) {
             return withTraits(shape, createTypedNode(shape)
                     .withMember("key", shape.getKey().accept(this))
-                    .withMember("value", shape.getValue().accept(this)));
+                    .withMember("value", shape.getValue().accept(this)))
+                    .build();
         }
 
         @Override
         public Node operationShape(OperationShape shape) {
             return withTraits(shape, createTypedNode(shape)
                     .withOptionalMember("input", shape.getInput().map(this::serializeReference))
                     .withOptionalMember("output", shape.getOutput().map(this::serializeReference))
-                    .withOptionalMember("errors", createOptionalIdList(shape.getErrors())));
+                    .withOptionalMember("errors", createOptionalIdList(shape.getErrors())))
+                    .build();
         }
 
         @Override
@@ -213,25 +218,30 @@ public Node resourceShape(ResourceShape shape) {
                     .withOptionalMember("list", shape.getList().map(this::serializeReference))
                     .withOptionalMember("operations", createOptionalIdList(shape.getOperations()))
                     .withOptionalMember("collectionOperations", createOptionalIdList(shape.getCollectionOperations()))
-                    .withOptionalMember("resources", createOptionalIdList(shape.getResources())));
+                    .withOptionalMember("resources", createOptionalIdList(shape.getResources())))
+                    .build();
         }
 
         @Override
         public Node serviceShape(ServiceShape shape) {
-            ObjectNode result = withTraits(shape, createTypedNode(shape)
-                    .withMember("version", Node.from(shape.getVersion()))
-                    .withOptionalMember("operations", createOptionalIdList(shape.getOperations()))
-                    .withOptionalMember("resources", createOptionalIdList(shape.getResources())));
+            ObjectNode.Builder serviceBuilder = withTraits(shape, createTypedNode(shape));
+
+            if (!StringUtils.isBlank(shape.getVersion())) {
+                serviceBuilder.withMember("version", Node.from(shape.getVersion()));
+            }
+
+            serviceBuilder.withOptionalMember("operations", createOptionalIdList(shape.getOperations()));
+            serviceBuilder.withOptionalMember("resources", createOptionalIdList(shape.getResources()));
 
             if (!shape.getRename().isEmpty()) {
-                ObjectNode.Builder builder = Node.objectNodeBuilder();
+                ObjectNode.Builder renameBuilder = Node.objectNodeBuilder();
                 for (Map.Entry<ShapeId, String> entry : shape.getRename().entrySet()) {
-                    builder.withMember(entry.getKey().toString(), entry.getValue());
+                    renameBuilder.withMember(entry.getKey().toString(), entry.getValue());
                 }
-                result = result.withMember("rename", builder.build());
+                serviceBuilder.withMember("rename", renameBuilder.build());
             }
 
-            return result;
+            return serviceBuilder.build();
         }
 
         private Optional<Node> createOptionalIdList(Collection<ShapeId> list) {
@@ -257,16 +267,25 @@ public Node unionShape(UnionShape shape) {
         }
 
         private ObjectNode createStructureAndUnion(Shape shape, Map<String, MemberShape> members) {
-            ObjectNode result = createTypedNode(shape);
-            result = result.withMember("members", members.entrySet().stream()
-                    .map(entry -> Pair.of(entry.getKey(), entry.getValue().accept(this)))
-                    .collect(ObjectNode.collectStringKeys(Pair::getLeft, Pair::getRight)));
-            return withTraits(shape, result);
+            ObjectNode.Builder builder = createTypedNode(shape);
+
+            ObjectNode.Builder memberBuilder = ObjectNode.objectNodeBuilder();
+            for (MemberShape member : members.values()) {
+                Node memberValue = member.accept(this);
+                memberBuilder.withMember(member.getMemberName(), memberValue);
+            }
+
+            builder.withMember("members", memberBuilder.build());
+            withTraits(shape, builder);
+
+            return builder.build();
         }
 
         @Override
         public Node memberShape(MemberShape shape) {
-            return withTraits(shape, serializeReference(shape.getTarget()));
+            ObjectNode.Builder builder = serializeReference(shape.getTarget()).toBuilder();
+            withTraits(shape, builder);
+            return builder.build();
         }
 
         private ObjectNode serializeReference(ShapeId id) {

smithy-model/src/main/java/software/amazon/smithy/model/shapes/ServiceShape.java

@@ -20,7 +20,6 @@
 import java.util.Optional;
 import java.util.TreeMap;
 import software.amazon.smithy.utils.MapUtils;
-import software.amazon.smithy.utils.SmithyBuilder;
 import software.amazon.smithy.utils.ToSmithyBuilder;
 
 /**
@@ -33,7 +32,7 @@ public final class ServiceShape extends EntityShape implements ToSmithyBuilder<S
 
     private ServiceShape(Builder builder) {
         super(builder);
-        version = SmithyBuilder.requiredState("version", builder.version);
+        version = builder.version;
         rename = MapUtils.orderedCopyOf(builder.rename);
     }
 
@@ -71,6 +70,9 @@ public boolean equals(Object other) {
     }
 
     /**
+     * Get the version of the service. An empty string is returned
+     * if the version is undefined.
+     *
      * @return The version of the service.
      */
     public String getVersion() {
@@ -107,7 +109,7 @@ public String getContextualName(ToShapeId shape) {
      * Builder used to create a {@link ServiceShape}.
      */
     public static final class Builder extends EntityShape.Builder<Builder, ServiceShape> {
-        private String version;
+        private String version = "";
         private final Map<ShapeId, String> rename = new TreeMap<>();
 
         @Override
@@ -121,7 +123,7 @@ public ShapeType getShapeType() {
         }
 
         public Builder version(String version) {
-            this.version = version;
+            this.version = version == null ? "" : version;
             return this;
         }
 

smithy-model/src/main/java/software/amazon/smithy/model/shapes/SmithyIdlModelSerializer.java

@@ -424,8 +424,12 @@ public Void unionShape(UnionShape shape) {
         @Override
         public Void serviceShape(ServiceShape shape) {
             serializeTraits(shape);
-            codeWriter.openBlock("service $L {", shape.getId().getName())
-                    .write("version: $S,", shape.getVersion());
+            codeWriter.openBlock("service $L {", shape.getId().getName());
+
+            if (!StringUtils.isBlank(shape.getVersion())) {
+                codeWriter.write("version: $S,", shape.getVersion());
+            }
+
             codeWriter.writeOptionalIdList("operations", shape.getOperations());
             codeWriter.writeOptionalIdList("resources", shape.getResources());
             if (!shape.getRename().isEmpty()) {

smithy-model/src/test/java/software/amazon/smithy/model/shapes/ModelSerializerTest.java

@@ -18,6 +18,7 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasKey;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.not;
 import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -27,6 +28,7 @@
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.SourceLocation;
 import software.amazon.smithy.model.node.Node;
+import software.amazon.smithy.model.node.NodePointer;
 import software.amazon.smithy.model.node.ObjectNode;
 import software.amazon.smithy.model.traits.DocumentationTrait;
 import software.amazon.smithy.model.traits.SensitiveTrait;
@@ -141,4 +143,20 @@ public void doesNotSerializePreludeTraitsOrShapes() {
 
         assertFalse(serialized.getMember("smithy.api").isPresent());
     }
+
+    @Test
+    public void doesNotSerializeEmptyServiceVersions() {
+        ServiceShape service = ServiceShape.builder()
+                .id("com.foo#Example")
+                .build();
+        Model model = Model.builder().addShape(service).build();
+        ModelSerializer serializer = ModelSerializer.builder().build();
+        ObjectNode result = serializer.serialize(model);
+
+        assertThat(NodePointer.parse("/shapes/com.foo#Example")
+                           .getValue(result)
+                           .expectObjectNode()
+                           .getStringMap(),
+                   not(hasKey("version")));
+    }
 }

smithy-model/src/test/java/software/amazon/smithy/model/shapes/ServiceShapeTest.java

@@ -58,4 +58,13 @@ public void providesContextualShapeName() {
 
         assertThat(serviceShape.getContextualName(id), equalTo("FooName"));
     }
+
+    @Test
+    public void versionDefaultsToEmptyString() {
+        ServiceShape shape = ServiceShape.builder()
+                .id("com.foo#Example")
+                .build();
+
+        assertThat(shape.getVersion(), equalTo(""));
+    }
 }

smithy-model/src/test/java/software/amazon/smithy/model/shapes/SmithyIdlModelSerializerTest.java

@@ -3,6 +3,7 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.aMapWithSize;
 import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasKey;
@@ -19,6 +20,7 @@
 import org.junit.jupiter.api.DynamicTest;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.TestFactory;
+import org.junit.jupiter.params.provider.EnumSource;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.node.StringNode;
 import software.amazon.smithy.model.traits.DocumentationTrait;
@@ -136,4 +138,16 @@ public void basePathAppliesToMetadataOnlyModel() {
         Map<Path, String> serialized = serializer.serialize(model);
         assertThat(serialized.keySet(), contains(basePath.resolve("metadata.smithy")));
     }
+
+    @Test
+    public void emptyServiceVersionNotSerialized() {
+        ServiceShape service = ServiceShape.builder()
+                .id("com.foo#Example")
+                .build();
+        Model model = Model.builder().addShape(service).build();
+        SmithyIdlModelSerializer serializer = SmithyIdlModelSerializer.builder().build();
+        Map<Path, String> serialized = serializer.serialize(model);
+
+        assertThat(serialized.get(Paths.get("com.foo.smithy")), not(containsString("version: \"\"")));
+    }
 }