From 0f1f84e6a604532a37e1f760c24629009f8ea424 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Wed, 24 Aug 2016 12:29:14 +0100 Subject: [PATCH] Fix JSON field type resolution for top-level and nested array fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, the logic that determined the field’s type would incorrectly look at the contents of the array. For example, if the array contained one or more objects, the field’s type would be resolved as Object rather than Array. This commit updates JsonFieldPath to record when a path explicitly identifies an array so that the array itself is used to determine the field’s type rather than its contents. Closes gh-292 --- .../restdocs/payload/JsonFieldPath.java | 19 ++++++-- .../restdocs/payload/JsonFieldProcessor.java | 2 +- .../payload/JsonFieldTypeResolver.java | 2 +- .../restdocs/payload/JsonFieldPathTests.java | 48 ++++++++++++------- .../payload/JsonFieldProcessorTests.java | 12 +++++ .../payload/JsonFieldTypeResolverTests.java | 17 +++++++ .../payload/RequestFieldsSnippetTests.java | 19 ++++---- .../payload/ResponseFieldsSnippetTests.java | 6 +-- 8 files changed, 91 insertions(+), 34 deletions(-) diff --git a/spring-restdocs-core/src/main/java/org/springframework/restdocs/payload/JsonFieldPath.java b/spring-restdocs-core/src/main/java/org/springframework/restdocs/payload/JsonFieldPath.java index 10478a83..03ddfefb 100644 --- a/spring-restdocs-core/src/main/java/org/springframework/restdocs/payload/JsonFieldPath.java +++ b/spring-restdocs-core/src/main/java/org/springframework/restdocs/payload/JsonFieldPath.java @@ -17,6 +17,7 @@ package org.springframework.restdocs.payload; import java.util.ArrayList; +import java.util.Iterator; import java.util.List; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -42,16 +43,24 @@ final class JsonFieldPath { private final boolean precise; - private JsonFieldPath(String rawPath, List segments, boolean precise) { + private final boolean array; + + private JsonFieldPath(String rawPath, List segments, boolean precise, + boolean array) { this.rawPath = rawPath; this.segments = segments; this.precise = precise; + this.array = array; } boolean isPrecise() { return this.precise; } + boolean isArray() { + return this.array; + } + List getSegments() { return this.segments; } @@ -63,7 +72,8 @@ final class JsonFieldPath { static JsonFieldPath compile(String path) { List segments = extractSegments(path); - return new JsonFieldPath(path, segments, matchesSingleValue(segments)); + return new JsonFieldPath(path, segments, matchesSingleValue(segments), + isArraySegment(segments.get(segments.size() - 1))); } static boolean isArraySegment(String segment) { @@ -71,8 +81,9 @@ final class JsonFieldPath { } static boolean matchesSingleValue(List segments) { - for (String segment : segments) { - if (isArraySegment(segment)) { + Iterator iterator = segments.iterator(); + while (iterator.hasNext()) { + if (isArraySegment(iterator.next()) && iterator.hasNext()) { return false; } } diff --git a/spring-restdocs-core/src/main/java/org/springframework/restdocs/payload/JsonFieldProcessor.java b/spring-restdocs-core/src/main/java/org/springframework/restdocs/payload/JsonFieldProcessor.java index fb63a214..dbaf6d88 100644 --- a/spring-restdocs-core/src/main/java/org/springframework/restdocs/payload/JsonFieldProcessor.java +++ b/spring-restdocs-core/src/main/java/org/springframework/restdocs/payload/JsonFieldProcessor.java @@ -57,7 +57,7 @@ final class JsonFieldProcessor { if (matches.isEmpty()) { throw new FieldDoesNotExistException(path); } - if (path.isPrecise()) { + if ((!path.isArray()) && path.isPrecise()) { return matches.get(0); } else { diff --git a/spring-restdocs-core/src/main/java/org/springframework/restdocs/payload/JsonFieldTypeResolver.java b/spring-restdocs-core/src/main/java/org/springframework/restdocs/payload/JsonFieldTypeResolver.java index f5bc6ec3..25c373cd 100644 --- a/spring-restdocs-core/src/main/java/org/springframework/restdocs/payload/JsonFieldTypeResolver.java +++ b/spring-restdocs-core/src/main/java/org/springframework/restdocs/payload/JsonFieldTypeResolver.java @@ -44,7 +44,7 @@ class JsonFieldTypeResolver { } return commonType; } - return determineFieldType(this.fieldProcessor.extract(fieldPath, payload)); + return determineFieldType(field); } private JsonFieldType determineFieldType(Object fieldValue) { diff --git a/spring-restdocs-core/src/test/java/org/springframework/restdocs/payload/JsonFieldPathTests.java b/spring-restdocs-core/src/test/java/org/springframework/restdocs/payload/JsonFieldPathTests.java index 04397796..dc71f15e 100644 --- a/spring-restdocs-core/src/test/java/org/springframework/restdocs/payload/JsonFieldPathTests.java +++ b/spring-restdocs-core/src/test/java/org/springframework/restdocs/payload/JsonFieldPathTests.java @@ -32,43 +32,59 @@ import static org.junit.Assert.assertTrue; public class JsonFieldPathTests { @Test - public void singleFieldIsPrecise() { - assertTrue(JsonFieldPath.compile("a").isPrecise()); + public void singleFieldIsPreciseAndNotAnArray() { + JsonFieldPath path = JsonFieldPath.compile("a"); + assertTrue(path.isPrecise()); + assertFalse(path.isArray()); } @Test - public void singleNestedFieldIsPrecise() { - assertTrue(JsonFieldPath.compile("a.b").isPrecise()); + public void singleNestedFieldIsPreciseAndNotAnArray() { + JsonFieldPath path = JsonFieldPath.compile("a.b"); + assertTrue(path.isPrecise()); + assertFalse(path.isArray()); } @Test - public void topLevelArrayIsNotPrecise() { - assertFalse(JsonFieldPath.compile("[]").isPrecise()); + public void topLevelArrayIsPreciseAndAnArray() { + JsonFieldPath path = JsonFieldPath.compile("[]"); + assertTrue(path.isPrecise()); + assertTrue(path.isArray()); } @Test - public void fieldBeneathTopLevelArrayIsNotPrecise() { - assertFalse(JsonFieldPath.compile("[]a").isPrecise()); + public void fieldBeneathTopLevelArrayIsNotPreciseAndNotAnArray() { + JsonFieldPath path = JsonFieldPath.compile("[]a"); + assertFalse(path.isPrecise()); + assertFalse(path.isArray()); } @Test - public void arrayIsNotPrecise() { - assertFalse(JsonFieldPath.compile("a[]").isPrecise()); + public void arrayIsPreciseAndAnArray() { + JsonFieldPath path = JsonFieldPath.compile("a[]"); + assertTrue(path.isPrecise()); + assertTrue(path.isArray()); } @Test - public void nestedArrayIsNotPrecise() { - assertFalse(JsonFieldPath.compile("a.b[]").isPrecise()); + public void nestedArrayIsPreciseAndAnArray() { + JsonFieldPath path = JsonFieldPath.compile("a.b[]"); + assertTrue(path.isPrecise()); + assertTrue(path.isArray()); } @Test - public void arrayOfArraysIsNotPrecise() { - assertFalse(JsonFieldPath.compile("a[][]").isPrecise()); + public void arrayOfArraysIsNotPreciseAndIsAnArray() { + JsonFieldPath path = JsonFieldPath.compile("a[][]"); + assertFalse(path.isPrecise()); + assertTrue(path.isArray()); } @Test - public void fieldBeneathAnArrayIsNotPrecise() { - assertFalse(JsonFieldPath.compile("a[].b").isPrecise()); + public void fieldBeneathAnArrayIsNotPreciseAndIsNotAnArray() { + JsonFieldPath path = JsonFieldPath.compile("a[].b"); + assertFalse(path.isPrecise()); + assertFalse(path.isArray()); } @Test diff --git a/spring-restdocs-core/src/test/java/org/springframework/restdocs/payload/JsonFieldProcessorTests.java b/spring-restdocs-core/src/test/java/org/springframework/restdocs/payload/JsonFieldProcessorTests.java index 01af2097..395ca4df 100644 --- a/spring-restdocs-core/src/test/java/org/springframework/restdocs/payload/JsonFieldProcessorTests.java +++ b/spring-restdocs-core/src/test/java/org/springframework/restdocs/payload/JsonFieldProcessorTests.java @@ -17,6 +17,7 @@ package org.springframework.restdocs.payload; import java.io.IOException; +import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; import java.util.List; @@ -55,6 +56,17 @@ public class JsonFieldProcessorTests { equalTo((Object) "bravo")); } + @Test + public void extractTopLevelArray() { + List> payload = new ArrayList<>(); + Map bravo = new HashMap<>(); + bravo.put("b", "bravo"); + payload.add(bravo); + payload.add(bravo); + assertThat(this.fieldProcessor.extract(JsonFieldPath.compile("[]"), payload), + equalTo((Object) payload)); + } + @Test public void extractArray() { Map payload = new HashMap<>(); diff --git a/spring-restdocs-core/src/test/java/org/springframework/restdocs/payload/JsonFieldTypeResolverTests.java b/spring-restdocs-core/src/test/java/org/springframework/restdocs/payload/JsonFieldTypeResolverTests.java index 502bfd29..e1ba3ec5 100644 --- a/spring-restdocs-core/src/test/java/org/springframework/restdocs/payload/JsonFieldTypeResolverTests.java +++ b/spring-restdocs-core/src/test/java/org/springframework/restdocs/payload/JsonFieldTypeResolverTests.java @@ -17,6 +17,7 @@ package org.springframework.restdocs.payload; import java.io.IOException; +import java.util.List; import java.util.Map; import com.fasterxml.jackson.databind.ObjectMapper; @@ -45,6 +46,22 @@ public class JsonFieldTypeResolverTests { assertFieldType(JsonFieldType.ARRAY, "[]"); } + @Test + public void topLevelArray() throws IOException { + assertThat( + this.fieldTypeResolver.resolveFieldType("[]", + new ObjectMapper().readValue("[{\"a\":\"alpha\"}]", List.class)), + equalTo(JsonFieldType.ARRAY)); + } + + @Test + public void nestedArray() throws IOException { + assertThat( + this.fieldTypeResolver.resolveFieldType("a[]", + createPayload("{\"a\": [{\"b\":\"bravo\"}]}")), + equalTo(JsonFieldType.ARRAY)); + } + @Test public void booleanField() throws IOException { assertFieldType(JsonFieldType.BOOLEAN, "true"); diff --git a/spring-restdocs-core/src/test/java/org/springframework/restdocs/payload/RequestFieldsSnippetTests.java b/spring-restdocs-core/src/test/java/org/springframework/restdocs/payload/RequestFieldsSnippetTests.java index b88f6a51..8970a43d 100644 --- a/spring-restdocs-core/src/test/java/org/springframework/restdocs/payload/RequestFieldsSnippetTests.java +++ b/spring-restdocs-core/src/test/java/org/springframework/restdocs/payload/RequestFieldsSnippetTests.java @@ -67,13 +67,14 @@ public class RequestFieldsSnippetTests extends AbstractSnippetTests { public void arrayRequestWithFields() throws IOException { this.snippet.expectRequestFields() .withContents(tableWithHeader("Path", "Type", "Description") - .row("`[]a.b`", "`Number`", "one") - .row("`[]a.c`", "`String`", "two") - .row("`[]a`", "`Object`", "three")); + .row("`[]`", "`Array`", "one").row("`[]a.b`", "`Number`", "two") + .row("`[]a.c`", "`String`", "three") + .row("`[]a`", "`Object`", "four")); - new RequestFieldsSnippet(Arrays.asList(fieldWithPath("[]a.b").description("one"), - fieldWithPath("[]a.c").description("two"), - fieldWithPath("[]a").description("three"))) + new RequestFieldsSnippet(Arrays.asList(fieldWithPath("[]").description("one"), + fieldWithPath("[]a.b").description("two"), + fieldWithPath("[]a.c").description("three"), + fieldWithPath("[]a").description("four"))) .document(this.operationBuilder.request("http://localhost") .content( "[{\"a\": {\"b\": 5}},{\"a\": {\"c\": \"charlie\"}}]") @@ -238,7 +239,7 @@ public class RequestFieldsSnippetTests extends AbstractSnippetTests { .requestFields(fieldWithPath("a.b").description("one"), fieldWithPath("a.c").description("two")) .and(fieldWithPath("a").description("three")) - .document(operationBuilder.request("http://localhost") + .document(this.operationBuilder.request("http://localhost") .content("{\"a\": {\"b\": 5, \"c\": \"charlie\"}}").build()); } @@ -252,7 +253,7 @@ public class RequestFieldsSnippetTests extends AbstractSnippetTests { PayloadDocumentation.requestFields(fieldWithPath("a").description("one")) .andWithPrefix("a.", fieldWithPath("b").description("two"), fieldWithPath("c").description("three")) - .document(operationBuilder.request("http://localhost") + .document(this.operationBuilder.request("http://localhost") .content("{\"a\": {\"b\": 5, \"c\": \"charlie\"}}").build()); } @@ -265,7 +266,7 @@ public class RequestFieldsSnippetTests extends AbstractSnippetTests { new RequestFieldsSnippet(Arrays.asList( fieldWithPath("Foo|Bar").type("one|two").description("three|four"))) - .document(operationBuilder.request("http://localhost") + .document(this.operationBuilder.request("http://localhost") .content("{\"Foo|Bar\": 5}").build()); } diff --git a/spring-restdocs-core/src/test/java/org/springframework/restdocs/payload/ResponseFieldsSnippetTests.java b/spring-restdocs-core/src/test/java/org/springframework/restdocs/payload/ResponseFieldsSnippetTests.java index daaaa233..d22fdb33 100644 --- a/spring-restdocs-core/src/test/java/org/springframework/restdocs/payload/ResponseFieldsSnippetTests.java +++ b/spring-restdocs-core/src/test/java/org/springframework/restdocs/payload/ResponseFieldsSnippetTests.java @@ -54,7 +54,7 @@ public class ResponseFieldsSnippetTests extends AbstractSnippetTests { .withContents(tableWithHeader("Path", "Type", "Description") .row("`id`", "`Number`", "one").row("`date`", "`String`", "two") .row("`assets`", "`Array`", "three") - .row("`assets[]`", "`Object`", "four") + .row("`assets[]`", "`Array`", "four") .row("`assets[].id`", "`Number`", "five") .row("`assets[].name`", "`String`", "six")); new ResponseFieldsSnippet(Arrays.asList(fieldWithPath("id").description("one"), @@ -90,7 +90,7 @@ public class ResponseFieldsSnippetTests extends AbstractSnippetTests { public void arrayResponse() throws IOException { this.snippet.expectResponseFields() .withContents(tableWithHeader("Path", "Type", "Description").row("`[]`", - "`String`", "one")); + "`Array`", "one")); new ResponseFieldsSnippet(Arrays.asList(fieldWithPath("[]").description("one"))) .document(this.operationBuilder.response() .content("[\"a\", \"b\", \"c\"]").build()); @@ -291,7 +291,7 @@ public class ResponseFieldsSnippetTests extends AbstractSnippetTests { .withContents(tableWithHeader("Path", "Type", "Description") .row("`id`", "`Number`", "one").row("`date`", "`String`", "two") .row("`assets`", "`Array`", "three") - .row("`assets[]`", "`Object`", "four") + .row("`assets[]`", "`Array`", "four") .row("`assets[].id`", "`Number`", "five") .row("`assets[].name`", "`String`", "six")); PayloadDocumentation