Add and use expect* methods more + some cleanup

This commit uses the expectTrait methods and hasTrait methods more instead of using .get(). This will give much more clear error messages in the event of a logical error in code. This also adds expect* methods that expect a shape to be of a specific type. This cleans up a lot of call sites that were using an unsafe .get() after calling .asBlob, asString, etc.
smithy-lang · Mar 16, 2020 · a44123e · a44123e
1 parent c5aa1d7
commit a44123e
Show file tree

Hide file tree

Showing 74 changed files with 636 additions and 180 deletions.
diff --git a/...src/main/java/software/amazon/smithy/aws/traits/apigateway/AuthorizersTraitValidator.java b/...src/main/java/software/amazon/smithy/aws/traits/apigateway/AuthorizersTraitValidator.java
@@ -58,7 +58,7 @@ private Optional<ValidationEvent> validateService(Model model, ServiceShape serv
             return Optional.empty();
         }
 
-        AuthorizersTrait authorizersTrait = service.getTrait(AuthorizersTrait.class).get();
+        AuthorizersTrait authorizersTrait = service.expectTrait(AuthorizersTrait.class);
         return Optional.of(error(service, authorizersTrait, String.format(
                 "Each `scheme` of the `%s` trait must target one of the auth schemes applied to the service "
                 + "(i.e., [%s]). The following mappings of authorizer names to schemes are invalid: %s",

diff --git a/smithy-aws-traits/src/test/java/software/amazon/smithy/aws/traits/ArnIndexTest.java b/smithy-aws-traits/src/test/java/software/amazon/smithy/aws/traits/ArnIndexTest.java
@@ -43,15 +43,15 @@ public static void beforeClass() {
     public void loadsFromModel() {
         ArnIndex arnIndex = new ArnIndex(model);
         ShapeId id = ShapeId.from("ns.foo#SomeService");
-        Shape someResource = model.getShape(ShapeId.from("ns.foo#SomeResource")).get();
+        Shape someResource = model.expectShape(ShapeId.from("ns.foo#SomeResource"));
         ArnTrait template1 = ArnTrait.builder()
                 .template("someresource/{someId}")
                 .build();
-        Shape childResource = model.getShape(ShapeId.from("ns.foo#ChildResource")).get();
+        Shape childResource = model.expectShape(ShapeId.from("ns.foo#ChildResource"));
         ArnTrait template2 = ArnTrait.builder()
                 .template("someresource/{someId}/{childId}")
                 .build();
-        Shape rootArnResource = model.getShape(ShapeId.from("ns.foo#RootArnResource")).get();
+        Shape rootArnResource = model.expectShape(ShapeId.from("ns.foo#RootArnResource"));
         ArnTrait template3 = ArnTrait.builder()
                 .template("rootArnResource")
                 .noAccount(true)

diff --git a/smithy-aws-traits/src/test/java/software/amazon/smithy/aws/traits/ServiceTraitTest.java b/smithy-aws-traits/src/test/java/software/amazon/smithy/aws/traits/ServiceTraitTest.java
@@ -88,8 +88,8 @@ public void loadsFromModel() {
                 .unwrap();
         ServiceShape service = result
                 .expectShape(ShapeId.from("ns.foo#SomeService"))
-                .asServiceShape().get();
-        ServiceTrait trait = service.getTrait(ServiceTrait.class).get();
+                .expectServiceShape();
+        ServiceTrait trait = service.expectTrait(ServiceTrait.class);
 
         assertThat(trait.getSdkId(), equalTo("Some Value"));
         assertThat(trait.getCloudFormationName(), equalTo("SomeService"));

diff --git a/...e/amazon/smithy/aws/traits/clientendpointdiscovery/ClientDiscoveredEndpointTraitTest.java b/...e/amazon/smithy/aws/traits/clientendpointdiscovery/ClientDiscoveredEndpointTraitTest.java
@@ -33,8 +33,8 @@ public void loadsFromModel() {
                 .unwrap();
         OperationShape operation = result
                 .expectShape(ShapeId.from("ns.foo#GetObject"))
-                .asOperationShape().get();
-        ClientDiscoveredEndpointTrait trait = operation.getTrait(ClientDiscoveredEndpointTrait.class).get();
+                .expectOperationShape();
+        ClientDiscoveredEndpointTrait trait = operation.expectTrait(ClientDiscoveredEndpointTrait.class);
 
         assertTrue(trait.isRequired());
     }

diff --git a/.../amazon/smithy/aws/traits/clientendpointdiscovery/ClientEndpointDiscoveryIdTraitTest.java b/.../amazon/smithy/aws/traits/clientendpointdiscovery/ClientEndpointDiscoveryIdTraitTest.java
@@ -42,10 +42,10 @@ public void loadsFromModel() {
                 .unwrap();
         OperationShape operation = result
                 .expectShape(ShapeId.from("ns.foo#GetObject"))
-                .asOperationShape().get();
+                .expectOperationShape();
         MemberShape member = result
                 .getShape(operation.getInput().get()).get()
-                .asStructureShape().get()
+                .expectStructureShape()
                 .getMember("Id").get();
 
         assertTrue(member.getTrait(ClientEndpointDiscoveryIdTrait.class).isPresent());

diff --git a/...re/amazon/smithy/aws/traits/clientendpointdiscovery/ClientEndpointDiscoveryTraitTest.java b/...re/amazon/smithy/aws/traits/clientendpointdiscovery/ClientEndpointDiscoveryTraitTest.java
@@ -33,8 +33,8 @@ public void loadsFromModel() {
                 .unwrap();
         ServiceShape service = result
                 .expectShape(ShapeId.from("ns.foo#FooService"))
-                .asServiceShape().get();
-        ClientEndpointDiscoveryTrait trait = service.getTrait(ClientEndpointDiscoveryTrait.class).get();
+                .expectServiceShape();
+        ClientEndpointDiscoveryTrait trait = service.expectTrait(ClientEndpointDiscoveryTrait.class);
 
         assertEquals(trait.getOperation(), ShapeId.from("ns.foo#DescribeEndpoints"));
         assertEquals(trait.getError(), ShapeId.from("ns.foo#InvalidEndpointError"));

diff --git a/...-traits/src/test/java/software/amazon/smithy/aws/traits/iam/RequiredActionsTraitTest.java b/...-traits/src/test/java/software/amazon/smithy/aws/traits/iam/RequiredActionsTraitTest.java
@@ -33,7 +33,7 @@ public void loadsFromModel() {
                 .assemble()
                 .unwrap();
 
-        Shape myOperation = result.getShape(ShapeId.from("smithy.example#MyOperation")).get();
+        Shape myOperation = result.expectShape(ShapeId.from("smithy.example#MyOperation"));
 
         assertTrue(myOperation.hasTrait(RequiredActionsTrait.class));
         assertThat(myOperation.getTrait(RequiredActionsTrait.class).get().getValues(), containsInAnyOrder(

diff --git a/smithy-build/src/test/java/software/amazon/smithy/build/SmithyBuildTest.java b/smithy-build/src/test/java/software/amazon/smithy/build/SmithyBuildTest.java
@@ -206,23 +206,23 @@ public void loadsImports() throws Exception {
         Model resultB = results.getProjectionResult("b").get().getModel();
         Model resultC = results.getProjectionResult("c").get().getModel();
 
-        assertTrue(resultA.getShape(ShapeId.from("com.foo#String")).get()
+        assertTrue(resultA.expectShape(ShapeId.from("com.foo#String"))
                            .getTrait(SensitiveTrait.class).isPresent());
-        assertFalse(resultA.getShape(ShapeId.from("com.foo#String")).get()
+        assertFalse(resultA.expectShape(ShapeId.from("com.foo#String"))
                            .getTrait(DocumentationTrait.class).isPresent());
 
-        assertTrue(resultB.getShape(ShapeId.from("com.foo#String")).get()
+        assertTrue(resultB.expectShape(ShapeId.from("com.foo#String"))
                            .getTrait(SensitiveTrait.class).isPresent());
-        assertTrue(resultB.getShape(ShapeId.from("com.foo#String")).get()
+        assertTrue(resultB.expectShape(ShapeId.from("com.foo#String"))
                            .getTrait(DocumentationTrait.class).isPresent());
-        assertThat(resultB.getShape(ShapeId.from("com.foo#String")).get()
+        assertThat(resultB.expectShape(ShapeId.from("com.foo#String"))
                            .getTrait(DocumentationTrait.class).get().getValue(), equalTo("b.json"));
 
-        assertTrue(resultC.getShape(ShapeId.from("com.foo#String")).get()
+        assertTrue(resultC.expectShape(ShapeId.from("com.foo#String"))
                            .getTrait(SensitiveTrait.class).isPresent());
-        assertTrue(resultC.getShape(ShapeId.from("com.foo#String")).get()
+        assertTrue(resultC.expectShape(ShapeId.from("com.foo#String"))
                            .getTrait(DocumentationTrait.class).isPresent());
-        assertThat(resultC.getShape(ShapeId.from("com.foo#String")).get()
+        assertThat(resultC.expectShape(ShapeId.from("com.foo#String"))
                            .getTrait(DocumentationTrait.class).get().getValue(), equalTo("c.json"));
     }
 

diff --git a/smithy-jsonschema/src/main/java/software/amazon/smithy/jsonschema/DefaultRefStrategy.java b/smithy-jsonschema/src/main/java/software/amazon/smithy/jsonschema/DefaultRefStrategy.java
@@ -121,8 +121,8 @@ private String createMemberPointer(MemberShape member) {
     public boolean isInlined(Shape shape) {
         // We could add more logic here in the future if needed to account for
         // member shapes that absolutely must generate a synthesized schema.
-        if (shape.asMemberShape().isPresent()) {
-            MemberShape member = shape.asMemberShape().get();
+        if (shape.isMemberShape()) {
+            MemberShape member = shape.expectMemberShape();
             Shape target = model.expectShape(member.getTarget());
             return isInlined(target);
         }

diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/EventStreamIndex.java b/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/EventStreamIndex.java
@@ -106,7 +106,7 @@ private Optional<EventStreamInfo> createEventStreamInfo(
             StructureShape structure,
             MemberShape member
     ) {
-        EventStreamTrait trait = member.getTrait(EventStreamTrait.class).get();
+        EventStreamTrait trait = member.expectTrait(EventStreamTrait.class);
 
         Shape eventStreamTarget = model.getShape(member.getTarget()).orElse(null);
         if (eventStreamTarget == null) {
@@ -118,14 +118,14 @@ private Optional<EventStreamInfo> createEventStreamInfo(
 
         // Compute the events of the event stream.
         Map<String, StructureShape> events = new HashMap<>();
-        if (eventStreamTarget.asUnionShape().isPresent()) {
-            for (MemberShape unionMember : eventStreamTarget.asUnionShape().get().getAllMembers().values()) {
+        if (eventStreamTarget.isUnionShape()) {
+            for (MemberShape unionMember : eventStreamTarget.expectUnionShape().getAllMembers().values()) {
                 model.getShape(unionMember.getTarget()).flatMap(Shape::asStructureShape).ifPresent(struct -> {
                     events.put(unionMember.getMemberName(), struct);
                 });
             }
-        } else if (eventStreamTarget.asStructureShape().isPresent()) {
-            events.put(member.getMemberName(), eventStreamTarget.asStructureShape().get());
+        } else if (eventStreamTarget.isStructureShape()) {
+            events.put(member.getMemberName(), eventStreamTarget.expectStructureShape());
         } else {
             // If the event target is an invalid type, then we can't create the indexed result.
             LOGGER.severe(() -> String.format(

diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/HttpBindingIndex.java b/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/HttpBindingIndex.java
@@ -134,9 +134,9 @@ public int getResponseCode(ToShapeId shapeOrId) {
         if (shape.isOperationShape()) {
             return getHttpTrait(id).getCode();
         } else if (shape.getTrait(HttpErrorTrait.class).isPresent()) {
-            return shape.getTrait(HttpErrorTrait.class).get().getCode();
+            return shape.expectTrait(HttpErrorTrait.class).getCode();
         } else if (shape.getTrait(ErrorTrait.class).isPresent()) {
-            return shape.getTrait(ErrorTrait.class).get().getDefaultHttpStatusCode();
+            return shape.expectTrait(ErrorTrait.class).getDefaultHttpStatusCode();
         }
 
         throw new IllegalStateException(shape + " must be an operation or error structure");
@@ -328,7 +328,7 @@ private String determineContentType(Collection<HttpBinding> bindings, String doc
 
                 // Use the @mediaType trait if available.
                 if (target.getTrait(MediaTypeTrait.class).isPresent()) {
-                    return target.getTrait(MediaTypeTrait.class).get().getValue();
+                    return target.expectTrait(MediaTypeTrait.class).getValue();
                 } else if (target.isBlobShape()) {
                     return "application/octet-stream";
                 } else if (target.isStringShape()) {
@@ -361,20 +361,20 @@ private List<HttpBinding> createStructureBindings(StructureShape struct, boolean
 
         for (MemberShape member : struct.getAllMembers().values()) {
             if (member.getTrait(HttpHeaderTrait.class).isPresent()) {
-                HttpHeaderTrait trait = member.getTrait(HttpHeaderTrait.class).get();
+                HttpHeaderTrait trait = member.expectTrait(HttpHeaderTrait.class);
                 bindings.add(new HttpBinding(member, HttpBinding.Location.HEADER, trait.getValue(), trait));
             } else if (member.getTrait(HttpPrefixHeadersTrait.class).isPresent()) {
-                HttpPrefixHeadersTrait trait = member.getTrait(HttpPrefixHeadersTrait.class).get();
+                HttpPrefixHeadersTrait trait = member.expectTrait(HttpPrefixHeadersTrait.class);
                 bindings.add(new HttpBinding(member, HttpBinding.Location.PREFIX_HEADERS, trait.getValue(), trait));
             } else if (isRequest && member.getTrait(HttpQueryTrait.class).isPresent()) {
-                HttpQueryTrait trait = member.getTrait(HttpQueryTrait.class).get();
+                HttpQueryTrait trait = member.expectTrait(HttpQueryTrait.class);
                 bindings.add(new HttpBinding(member, HttpBinding.Location.QUERY, trait.getValue(), trait));
             } else if (member.getTrait(HttpPayloadTrait.class).isPresent()) {
                 foundPayload = true;
-                HttpPayloadTrait trait = member.getTrait(HttpPayloadTrait.class).get();
+                HttpPayloadTrait trait = member.expectTrait(HttpPayloadTrait.class);
                 bindings.add(new HttpBinding(member, HttpBinding.Location.PAYLOAD, member.getMemberName(), trait));
             } else if (isRequest && member.getTrait(HttpLabelTrait.class).isPresent()) {
-                HttpLabelTrait trait = member.getTrait(HttpLabelTrait.class).get();
+                HttpLabelTrait trait = member.expectTrait(HttpLabelTrait.class);
                 bindings.add(new HttpBinding(member, HttpBinding.Location.LABEL, member.getMemberName(), trait));
             } else {
                 unbound.add(member);

diff --git a/...hy-model/src/main/java/software/amazon/smithy/model/knowledge/IdentifierBindingIndex.java b/...hy-model/src/main/java/software/amazon/smithy/model/knowledge/IdentifierBindingIndex.java
@@ -100,16 +100,17 @@ private void processResource(ResourceShape resource, OperationIndex operationInd
                     .orElseGet(HashMap::new);
             bindings.get(resource.getId()).put(operationId, computedBindings);
 
-            bindingTypes.get(resource.getId()).put(operationId, isCollection(resource, operationId)
+            BindingType operationBindingType = isCollection(resource, operationId)
                     ? BindingType.COLLECTION
-                    : BindingType.INSTANCE);
+                    : BindingType.INSTANCE;
+            bindingTypes.get(resource.getId()).put(operationId, operationBindingType);
         });
     }
 
     private boolean isCollection(ResourceShape resource, ToShapeId operationId) {
-        return resource.getCollectionOperations().contains(operationId)
-                || (resource.getCreate().isPresent() && resource.getCreate().get().toShapeId().equals(operationId))
-                || (resource.getList().isPresent() && resource.getList().get().toShapeId().equals(operationId));
+        return resource.getCollectionOperations().contains(operationId.toShapeId())
+                || (resource.getCreate().filter(id -> id.equals(operationId)).isPresent())
+                || (resource.getList().filter(id -> id.equals(operationId)).isPresent());
     }
 
     private boolean isImplicitIdentifierBinding(Map.Entry<String, MemberShape> memberEntry, ResourceShape resource) {
@@ -126,6 +127,5 @@ private Map<String, String> computeBindings(ResourceShape resource, StructureSha
                                 ? Stream.of(Pair.of(entry.getKey(), entry.getKey()))
                                 : Stream.empty()))
                 .collect(Collectors.toMap(Pair::getLeft, Pair::getRight));
-
     }
 }
diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/LoaderVisitor.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/LoaderVisitor.java
@@ -537,7 +537,7 @@ private Map<ShapeId, Node> computeTraits(AbstractShapeBuilder shapeBuilder, List
                 traits.put(traitId, value);
             } else if (previous.isArrayNode() && value.isArrayNode()) {
                 // You can merge trait arrays.
-                traits.put(traitId, value.asArrayNode().get().merge(previous.asArrayNode().get()));
+                traits.put(traitId, value.expectArrayNode().merge(previous.expectArrayNode()));
             } else if (previous.equals(value)) {
                 LOGGER.fine(() -> String.format(
                         "Ignoring duplicate %s trait value on %s", traitId, shapeBuilder.getId()));

diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/shapes/BigDecimalShape.java b/smithy-model/src/main/java/software/amazon/smithy/model/shapes/BigDecimalShape.java
@@ -46,6 +46,11 @@ public Optional<BigDecimalShape> asBigDecimalShape() {
         return Optional.of(this);
     }
 
+    @Override
+    public BigDecimalShape expectBigDecimalShape() {
+        return this;
+    }
+
     /**
      * Builder used to create a {@link BigDecimalShape}.
      */

diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/shapes/BigIntegerShape.java b/smithy-model/src/main/java/software/amazon/smithy/model/shapes/BigIntegerShape.java
@@ -46,6 +46,11 @@ public Optional<BigIntegerShape> asBigIntegerShape() {
         return Optional.of(this);
     }
 
+    @Override
+    public BigIntegerShape expectBigIntegerShape() {
+        return this;
+    }
+
     /**
      * Builder used to create a {@link BigIntegerShape}.
      */

diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/shapes/BlobShape.java b/smithy-model/src/main/java/software/amazon/smithy/model/shapes/BlobShape.java
@@ -46,6 +46,11 @@ public Optional<BlobShape> asBlobShape() {
         return Optional.of(this);
     }
 
+    @Override
+    public BlobShape expectBlobShape() {
+        return this;
+    }
+
     /**
      * Builder used to create a {@link BlobShape}.
      */

diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/shapes/BooleanShape.java b/smithy-model/src/main/java/software/amazon/smithy/model/shapes/BooleanShape.java
@@ -46,6 +46,11 @@ public Optional<BooleanShape> asBooleanShape() {
         return Optional.of(this);
     }
 
+    @Override
+    public BooleanShape expectBooleanShape() {
+        return this;
+    }
+
     /**
      * Builder used to create a {@link BooleanShape}.
      */

diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/shapes/ByteShape.java b/smithy-model/src/main/java/software/amazon/smithy/model/shapes/ByteShape.java
@@ -46,6 +46,11 @@ public Optional<ByteShape> asByteShape() {
         return Optional.of(this);
     }
 
+    @Override
+    public ByteShape expectByteShape() {
+        return this;
+    }
+
     /**
      * Builder used to create a {@link ByteShape}.
      */

diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/shapes/DocumentShape.java b/smithy-model/src/main/java/software/amazon/smithy/model/shapes/DocumentShape.java
@@ -46,6 +46,11 @@ public Optional<DocumentShape> asDocumentShape() {
         return Optional.of(this);
     }
 
+    @Override
+    public DocumentShape expectDocumentShape() {
+        return this;
+    }
+
     /**
      * Builder used to create a {@link DocumentShape}.
      */

diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/shapes/DoubleShape.java b/smithy-model/src/main/java/software/amazon/smithy/model/shapes/DoubleShape.java
@@ -46,6 +46,11 @@ public Optional<DoubleShape> asDoubleShape() {
         return Optional.of(this);
     }
 
+    @Override
+    public DoubleShape expectDoubleShape() {
+        return this;
+    }
+
     /**
      * Builder used to create a {@link DoubleShape}.
      */

diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/shapes/FloatShape.java b/smithy-model/src/main/java/software/amazon/smithy/model/shapes/FloatShape.java
@@ -46,6 +46,11 @@ public Optional<FloatShape> asFloatShape() {
         return Optional.of(this);
     }
 
+    @Override
+    public FloatShape expectFloatShape() {
+        return this;
+    }
+
     /**
      * Builder used to create a {@link FloatShape}.
      */

diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/shapes/IntegerShape.java b/smithy-model/src/main/java/software/amazon/smithy/model/shapes/IntegerShape.java
@@ -46,6 +46,11 @@ public Optional<IntegerShape> asIntegerShape() {
         return Optional.of(this);
     }
 
+    @Override
+    public IntegerShape expectIntegerShape() {
+        return this;
+    }
+
     /**
      * Builder used to create a {@link IntegerShape}.
      */

diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/shapes/ListShape.java b/smithy-model/src/main/java/software/amazon/smithy/model/shapes/ListShape.java
@@ -46,6 +46,11 @@ public Optional<ListShape> asListShape() {
         return Optional.of(this);
     }
 
+    @Override
+    public ListShape expectListShape() {
+        return this;
+    }
+
     /**
      * Builder used to create a {@link ListShape}.
      */

diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/shapes/LongShape.java b/smithy-model/src/main/java/software/amazon/smithy/model/shapes/LongShape.java
@@ -46,6 +46,11 @@ public Optional<LongShape> asLongShape() {
         return Optional.of(this);
     }
 
+    @Override
+    public LongShape expectLongShape() {
+        return this;
+    }
+
     /**
      * Builder used to create a {@link LongShape}.
      */

diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/shapes/MapShape.java b/smithy-model/src/main/java/software/amazon/smithy/model/shapes/MapShape.java
@@ -70,6 +70,11 @@ public Optional<MapShape> asMapShape() {
         return Optional.of(this);
     }
 
+    @Override
+    public MapShape expectMapShape() {
+        return this;
+    }
+
     /**
      * Get the value member shape of the map.
      *

diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/shapes/MemberShape.java b/smithy-model/src/main/java/software/amazon/smithy/model/shapes/MemberShape.java
@@ -64,6 +64,11 @@ public Optional<MemberShape> asMemberShape() {
         return Optional.of(this);
     }
 
+    @Override
+    public MemberShape expectMemberShape() {
+        return this;
+    }
+
     /**
      * Get the targeted member shape ID.
      *