From 4e4d01a459d3c2972fb1f4a7220978b19ffec165 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Wed, 24 Aug 2016 12:49:03 +0100 Subject: [PATCH 1/2] Consolidate logic in test rules for getting output dir and operation name --- .../headers/RequestHeadersSnippetTests.java | 2 + .../headers/ResponseHeadersSnippetTests.java | 2 + .../restdocs/test/ExpectedSnippet.java | 11 ++-- .../restdocs/test/OperationBuilder.java | 23 ++------ .../restdocs/test/OperationTestRule.java | 54 +++++++++++++++++++ 5 files changed, 67 insertions(+), 25 deletions(-) create mode 100644 spring-restdocs-core/src/test/java/org/springframework/restdocs/test/OperationTestRule.java diff --git a/spring-restdocs-core/src/test/java/org/springframework/restdocs/headers/RequestHeadersSnippetTests.java b/spring-restdocs-core/src/test/java/org/springframework/restdocs/headers/RequestHeadersSnippetTests.java index 805c1f7c..373e83b7 100644 --- a/spring-restdocs-core/src/test/java/org/springframework/restdocs/headers/RequestHeadersSnippetTests.java +++ b/spring-restdocs-core/src/test/java/org/springframework/restdocs/headers/RequestHeadersSnippetTests.java @@ -93,6 +93,8 @@ public class RequestHeadersSnippetTests extends AbstractSnippetTests { @Test public void undocumentedRequestHeader() throws IOException { + this.snippet.expectRequestHeaders().withContents( + tableWithHeader("Name", "Description").row("`X-Test`", "one")); new RequestHeadersSnippet( Arrays.asList(headerWithName("X-Test").description("one"))) .document(this.operationBuilder.request("http://localhost") diff --git a/spring-restdocs-core/src/test/java/org/springframework/restdocs/headers/ResponseHeadersSnippetTests.java b/spring-restdocs-core/src/test/java/org/springframework/restdocs/headers/ResponseHeadersSnippetTests.java index 86ac3eef..ba572598 100644 --- a/spring-restdocs-core/src/test/java/org/springframework/restdocs/headers/ResponseHeadersSnippetTests.java +++ b/spring-restdocs-core/src/test/java/org/springframework/restdocs/headers/ResponseHeadersSnippetTests.java @@ -81,6 +81,8 @@ public class ResponseHeadersSnippetTests extends AbstractSnippetTests { @Test public void undocumentedResponseHeader() throws IOException { + this.snippet.expectResponseHeaders().withContents( + tableWithHeader("Name", "Description").row("`X-Test`", "one")); new ResponseHeadersSnippet( Arrays.asList(headerWithName("X-Test").description("one"))).document( this.operationBuilder.response().header("X-Test", "test") diff --git a/spring-restdocs-core/src/test/java/org/springframework/restdocs/test/ExpectedSnippet.java b/spring-restdocs-core/src/test/java/org/springframework/restdocs/test/ExpectedSnippet.java index e5975f86..8036e98e 100644 --- a/spring-restdocs-core/src/test/java/org/springframework/restdocs/test/ExpectedSnippet.java +++ b/spring-restdocs-core/src/test/java/org/springframework/restdocs/test/ExpectedSnippet.java @@ -20,8 +20,6 @@ import java.io.File; import java.io.IOException; import org.hamcrest.Matcher; -import org.junit.rules.TestRule; -import org.junit.runner.Description; import org.junit.runners.model.Statement; import org.springframework.restdocs.snippet.TemplatedSnippet; @@ -38,7 +36,7 @@ import static org.junit.Assert.assertThat; * @author Andy Wilkinson * @author Andreas Evers */ -public class ExpectedSnippet implements TestRule { +public class ExpectedSnippet extends OperationTestRule { private final TemplateFormat templateFormat; @@ -56,9 +54,10 @@ public class ExpectedSnippet implements TestRule { } @Override - public Statement apply(final Statement base, Description description) { - this.outputDirectory = new File( - "build/" + description.getTestClass().getSimpleName()); + public Statement apply(final Statement base, File outputDirectory, + String operationName) { + this.outputDirectory = outputDirectory; + this.expectedName = operationName; return new ExpectedSnippetStatement(base); } diff --git a/spring-restdocs-core/src/test/java/org/springframework/restdocs/test/OperationBuilder.java b/spring-restdocs-core/src/test/java/org/springframework/restdocs/test/OperationBuilder.java index 8acc87b0..698a80da 100644 --- a/spring-restdocs-core/src/test/java/org/springframework/restdocs/test/OperationBuilder.java +++ b/spring-restdocs-core/src/test/java/org/springframework/restdocs/test/OperationBuilder.java @@ -24,8 +24,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import org.junit.rules.TestRule; -import org.junit.runner.Description; import org.junit.runners.model.Statement; import org.springframework.http.HttpHeaders; @@ -57,7 +55,7 @@ import org.springframework.restdocs.templates.mustache.MustacheTemplateEngine; * * @author Andy Wilkinson */ -public class OperationBuilder implements TestRule { +public class OperationBuilder extends OperationTestRule { private final Map attributes = new HashMap<>(); @@ -133,22 +131,9 @@ public class OperationBuilder implements TestRule { } @Override - public Statement apply(final Statement base, final Description description) { - return new Statement() { - - @Override - public void evaluate() throws Throwable { - String operationName = description.getMethodName(); - int index = operationName.indexOf('['); - if (index > 0) { - operationName = operationName.substring(0, index); - } - OperationBuilder.this.prepare(operationName, - new File("build/" + description.getTestClass().getSimpleName())); - base.evaluate(); - } - - }; + public Statement apply(Statement base, File outputDirectory, String operationName) { + prepare(operationName, outputDirectory); + return base; } /** diff --git a/spring-restdocs-core/src/test/java/org/springframework/restdocs/test/OperationTestRule.java b/spring-restdocs-core/src/test/java/org/springframework/restdocs/test/OperationTestRule.java new file mode 100644 index 00000000..28ea176d --- /dev/null +++ b/spring-restdocs-core/src/test/java/org/springframework/restdocs/test/OperationTestRule.java @@ -0,0 +1,54 @@ +/* + * Copyright 2014-2016 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.restdocs.test; + +import java.io.File; + +import org.junit.rules.TestRule; +import org.junit.runner.Description; +import org.junit.runners.model.Statement; + +/** + * Abstract base class for Operation-related {@link TestRule TestRules}. + * + * @author Andy Wilkinson + */ +abstract class OperationTestRule implements TestRule { + + @Override + public final Statement apply(Statement base, Description description) { + return apply(base, determineOutputDirectory(description), + determineOperationName(description)); + } + + private File determineOutputDirectory(Description description) { + return new File("build/" + description.getTestClass().getSimpleName()); + } + + private String determineOperationName(Description description) { + String operationName = description.getMethodName(); + int index = operationName.indexOf('['); + if (index > 0) { + operationName = operationName.substring(0, index); + } + return operationName; + } + + protected abstract Statement apply(Statement base, File outputDirectory, + String operationName); + +} From 0f1f84e6a604532a37e1f760c24629009f8ea424 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Wed, 24 Aug 2016 12:29:14 +0100 Subject: [PATCH 2/2] 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