Address Feedback

smithy-lang · Nov 11, 2020 · b209eb3 · b209eb3
1 parent 13c7ac2
commit b209eb3
Show file tree

Hide file tree

Showing 5 changed files with 81 additions and 24 deletions.
diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/PaginatedIndex.java b/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/PaginatedIndex.java
@@ -32,6 +32,7 @@
 import software.amazon.smithy.model.traits.PaginatedTrait;
 import software.amazon.smithy.model.traits.Trait;
 import software.amazon.smithy.model.validation.validators.PaginatedTraitValidator;
+import software.amazon.smithy.utils.ListUtils;
 import software.amazon.smithy.utils.OptionalUtils;
 
 /**
@@ -83,17 +84,17 @@ private Optional<PaginationInfo> create(
 
         MemberShape inputToken = trait.getInputToken().flatMap(input::getMember).orElse(null);
         List<MemberShape> outputTokenPath = trait.getOutputToken()
-                .flatMap(path -> PaginatedTrait.resolvePath(path, model, output))
-                .orElse(null);
+                .map(path -> PaginatedTrait.resolvePathToMember(path, model, output))
+                .orElse(ListUtils.of());
 
-        if (inputToken == null || outputTokenPath == null) {
+        if (inputToken == null || outputTokenPath.size() == 0) {
             return Optional.empty();
         }
 
         MemberShape pageSizeMember = trait.getPageSize().flatMap(input::getMember).orElse(null);
         List<MemberShape> itemsMemberPath = trait.getItems()
-                .flatMap(path -> PaginatedTrait.resolvePath(path, model, output))
-                .orElse(null);
+                .map(path -> PaginatedTrait.resolvePathToMember(path, model, output))
+                .orElse(ListUtils.of());
 
         return Optional.of(new PaginationInfo(
                 service, operation, input, output, trait,

diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/PaginationInfo.java b/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/PaginationInfo.java
@@ -91,14 +91,11 @@ public MemberShape getInputTokenMember() {
 
     /**
      * @return the last {@link MemberShape} of the output path.
-     * @deprecated See getOutputTokenPath.
+     *
+     * @deprecated See {@link PaginationInfo#getOutputTokenPath} to retrieve the full path.
      */
     public MemberShape getOutputTokenMember() {
-        int size = outputToken.size();
-        if (size == 0) {
-            return null;
-        }
-        return outputToken.get(size - 1);
+        return outputToken.get(outputToken.size() - 1);
     }
 
     public List<MemberShape> getOutputTokenPath() {
@@ -107,7 +104,8 @@ public List<MemberShape> getOutputTokenPath() {
 
     /**
      * @return the last {@link MemberShape} of the items path.
-     * @deprecated See getItemsMemberPath
+     *
+     * @deprecated See {@link PaginationInfo#getItemsMemberPath} to retrieve the full path.
      */
     public Optional<MemberShape> getItemsMember() {
         int size = items.size();
@@ -117,8 +115,8 @@ public Optional<MemberShape> getItemsMember() {
         return Optional.ofNullable(items.get(size - 1));
     }
 
-    public Optional<List<MemberShape>> getItemsMemberPath() {
-        return Optional.ofNullable(items);
+    public List<MemberShape> getItemsMemberPath() {
+        return items;
     }
 
     public Optional<MemberShape> getPageSizeMember() {

diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/traits/PaginatedTrait.java b/smithy-model/src/main/java/software/amazon/smithy/model/traits/PaginatedTrait.java
@@ -27,6 +27,7 @@
 import software.amazon.smithy.model.shapes.ShapeId;
 import software.amazon.smithy.model.shapes.StructureShape;
 import software.amazon.smithy.model.validation.validators.PaginatedTraitValidator;
+import software.amazon.smithy.utils.ListUtils;
 import software.amazon.smithy.utils.MapUtils;
 import software.amazon.smithy.utils.ToSmithyBuilder;
 
@@ -86,13 +87,40 @@ public Optional<String> getPageSize() {
      * <p>A path is a series of identifiers separated by dots (`.`) where each identifier
      * represents a member name in a structure.
      *
-     * @param path The path to resolve.
+     * @param path  The path to resolve.
      * @param model The model to be searched when resolving the path.
      * @param shape The shape where path resolution should start, e.g. the output shape
      *              of an operation.
      * @return The optional member shape that the path resolves to.
+     * @deprecated This method only returns the last {@link MemberShape} of an output path. To resolve each path
+     * identifier to it's respective {@link MemberShape} see {@link PaginatedTrait#resolvePathToMember}
      */
-    public static Optional<List<MemberShape>> resolvePath(
+    public static Optional<MemberShape> resolvePath(
+            String path,
+            Model model,
+            StructureShape shape
+    ) {
+        List<MemberShape> memberShapes = resolvePathToMember(path, model, shape);
+        if (memberShapes.size() == 0) {
+            return Optional.empty();
+        }
+        return Optional.of(memberShapes.get(memberShapes.size() - 1));
+    }
+
+    /**
+     * Resolves an output path to a list of {@link MemberShape}.
+     *
+     * <p>A path is a series of identifiers separated by dots (`.`) where each identifier
+     * represents a member name in a structure.
+     *
+     * @param path  The path to resolve.
+     * @param model The model to be searched when resolving the path.
+     * @param shape The shape where path resolution should start, e.g. the output shape
+     *              of an operation.
+     * @return The list of member shapes corresponding to each path identifier. An unresolvable path will be returned
+     * as an empty list.
+     */
+    public static List<MemberShape> resolvePathToMember(
             String path,
             Model model,
             StructureShape shape
@@ -102,16 +130,16 @@ public static Optional<List<MemberShape>> resolvePath(
         // For each member name in the path, try to find that member in the previous structure
         Optional<MemberShape> memberShape;
         Optional<StructureShape> container = Optional.of(shape);
-        for (String memberName: PATH_PATTERN.split(path)) {
+        for (String memberName : PATH_PATTERN.split(path)) {
             memberShape = container.flatMap(structure -> structure.getMember(memberName));
             if (!memberShape.isPresent()) {
-                return Optional.empty();
+                return ListUtils.of();
             }
             memberShapes.add(memberShape.get());
             container = model.getShape(memberShape.get().getTarget())
                     .flatMap(Shape::asStructureShape);
         }
-        return Optional.of(memberShapes);
+        return memberShapes;
     }
 
     /**

diff --git a/...main/java/software/amazon/smithy/model/validation/validators/PaginatedTraitValidator.java b/...main/java/software/amazon/smithy/model/validation/validators/PaginatedTraitValidator.java
@@ -228,8 +228,13 @@ Optional<MemberShape> getMember(
         ) {
             Optional<StructureShape> outputShape = opIndex.getOutput(operation);
             return outputShape.flatMap(structureShape -> getMemberPath(opIndex, operation, trait)
-                    .flatMap(path -> PaginatedTrait.resolvePath(path, model, structureShape)))
-                    .map(memberShapes -> memberShapes.get(memberShapes.size() - 1));
+                    .map(path -> PaginatedTrait.resolvePathToMember(path, model, structureShape)))
+                    .flatMap(memberShapes -> {
+                        if (memberShapes.size() == 0) {
+                            return Optional.empty();
+                        }
+                        return Optional.of(memberShapes.get(memberShapes.size() - 1));
+                    });
         }
     }
 

diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/knowledge/PaginatedIndexTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/knowledge/PaginatedIndexTest.java
@@ -19,11 +19,17 @@
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
 
+import java.util.Collections;
+import java.util.List;
 import java.util.Optional;
+import java.util.stream.Collectors;
 import org.junit.jupiter.api.Test;
+import org.junit.platform.commons.util.CollectionUtils;
 import software.amazon.smithy.model.Model;
+import software.amazon.smithy.model.shapes.MemberShape;
 import software.amazon.smithy.model.shapes.ShapeId;
 import software.amazon.smithy.model.validation.ValidatedResult;
+import software.amazon.smithy.utils.ListUtils;
 
 public class PaginatedIndexTest {
     @Test
@@ -43,9 +49,17 @@ public void findDirectChildren() {
         assertThat(info.getOutput().getId(), is(ShapeId.from("ns.foo#ValidOutput")));
         assertThat(info.getInputTokenMember().getMemberName(), equalTo("nextToken"));
         assertThat(info.getOutputTokenMember().getMemberName(), equalTo("nextToken"));
+        assertThat(info.getOutputTokenPath().isEmpty(), is(false));
+        assertThat(info.getOutputTokenPath().stream()
+                .map(MemberShape::getMemberName)
+                .collect(Collectors.toList()), equalTo(ListUtils.of("nextToken")));
         assertThat(info.getPageSizeMember().isPresent(), is(true));
         assertThat(info.getPageSizeMember().get().getMemberName(), equalTo("pageSize"));
         assertThat(info.getItemsMember().get().getMemberName(), equalTo("items"));
+        assertThat(info.getItemsMemberPath().isEmpty(), is(false));
+        assertThat(info.getItemsMemberPath().stream()
+                .map(MemberShape::getMemberName)
+                .collect(Collectors.toList()), equalTo(ListUtils.of("items")));
     }
 
     @Test
@@ -59,9 +73,20 @@ public void findIndirectChildren() {
 
         ShapeId service = ShapeId.from("ns.foo#Service");
         ShapeId operation = ShapeId.from("ns.foo#ValidNestedOutputOperation");
-        Optional<PaginationInfo> info = index.getPaginationInfo(service, operation);
+        Optional<PaginationInfo> optionalInfo = index.getPaginationInfo(service, operation);
+        assertThat(optionalInfo.isPresent(), is(true));
 
-        assertThat(info.isPresent(), is(true));
-        assertThat(info.get().getItemsMember().isPresent(), is(true));
+        PaginationInfo info = optionalInfo.get();
+        assertThat(info.getOutputTokenMember().getMemberName(), equalTo("nextToken"));
+        assertThat(info.getOutputTokenPath().isEmpty(), is(false));
+        assertThat(info.getOutputTokenPath().stream()
+                .map(MemberShape::getMemberName)
+                .collect(Collectors.toList()), equalTo(ListUtils.of("result", "nextToken")));
+        assertThat(info.getItemsMember().isPresent(), is(true));
+        assertThat(info.getItemsMember().get().getMemberName(), equalTo("items"));
+        assertThat(info.getItemsMemberPath().isEmpty(), is(false));
+        assertThat(info.getItemsMemberPath().stream()
+                .map(MemberShape::getMemberName)
+                .collect(Collectors.toList()), equalTo(ListUtils.of("result", "items")));
     }
 }