From f98bec317ec5efbfc56630ee02ac50b7df167d7f Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Fri, 15 May 2015 21:05:13 +0100 Subject: [PATCH] Add support for documenting fields in array payloads 764daf7 added support for documenting fields in payloads that contain arrays, but the array had to be nested within a map. This commit builds on that support to allow fields in an array payload to be documented. The fields in the payload: [ { "a": { "b": 5 } }, { "a": { "c": "charlie" } } ] Can be documented using: []a.b []a.c []a A dot separator can, optionally, be used between the [] and the map key: [].a.b [].a.c [].a Closes gh-69 --- .../restdocs/payload/FieldPath.java | 30 +++++--- .../restdocs/payload/FieldProcessor.java | 69 ++++++------------- .../payload/FieldSnippetResultHandler.java | 13 +--- .../restdocs/payload/FieldTypeResolver.java | 2 +- .../restdocs/payload/FieldValidator.java | 22 +++--- .../restdocs/payload/FieldPathTests.java | 52 ++++++++++++++ .../restdocs/payload/FieldValidatorTests.java | 23 +++++++ .../payload/PayloadDocumentationTests.java | 45 ++++++++++-- 8 files changed, 170 insertions(+), 86 deletions(-) diff --git a/spring-restdocs/src/main/java/org/springframework/restdocs/payload/FieldPath.java b/spring-restdocs/src/main/java/org/springframework/restdocs/payload/FieldPath.java index e72c6283..cbb87a12 100644 --- a/spring-restdocs/src/main/java/org/springframework/restdocs/payload/FieldPath.java +++ b/spring-restdocs/src/main/java/org/springframework/restdocs/payload/FieldPath.java @@ -75,24 +75,32 @@ class FieldPath { return true; } - static List extractSegments(String path) { + private static List extractSegments(String path) { Matcher matcher = ARRAY_INDEX_PATTERN.matcher(path); - String processedPath; - StringBuffer buffer = new StringBuffer(); + StringBuilder buffer = new StringBuilder(); + int previous = 0; while (matcher.find()) { - matcher.appendReplacement(buffer, ".[$1]"); + appendWithSeparatorIfNecessary(buffer, + path.substring(previous, matcher.start(0))); + appendWithSeparatorIfNecessary(buffer, matcher.group()); + previous = matcher.end(0); + } + if (previous < path.length()) { + appendWithSeparatorIfNecessary(buffer, path.substring(previous)); } - matcher.appendTail(buffer); - if (buffer.length() > 0) { - processedPath = buffer.toString(); - } - else { - processedPath = path; - } + String processedPath = buffer.toString(); return Arrays.asList(processedPath.indexOf('.') > -1 ? processedPath.split("\\.") : new String[] { processedPath }); } + private static void appendWithSeparatorIfNecessary(StringBuilder buffer, + String toAppend) { + if (buffer.length() > 0 && (buffer.lastIndexOf(".") != buffer.length() - 1) + && !toAppend.startsWith(".")) { + buffer.append("."); + } + buffer.append(toAppend); + } } diff --git a/spring-restdocs/src/main/java/org/springframework/restdocs/payload/FieldProcessor.java b/spring-restdocs/src/main/java/org/springframework/restdocs/payload/FieldProcessor.java index eb9e0ca8..0407e567 100644 --- a/spring-restdocs/src/main/java/org/springframework/restdocs/payload/FieldProcessor.java +++ b/spring-restdocs/src/main/java/org/springframework/restdocs/payload/FieldProcessor.java @@ -31,38 +31,26 @@ import java.util.concurrent.atomic.AtomicReference; */ class FieldProcessor { - boolean hasField(FieldPath fieldPath, Map payload) { + boolean hasField(FieldPath fieldPath, Object payload) { final AtomicReference hasField = new AtomicReference(false); traverse(new ProcessingContext(payload, fieldPath), new MatchCallback() { @Override - public boolean foundMatch(Match match) { + public void foundMatch(Match match) { hasField.set(true); - return false; - } - - @Override - public boolean matchNotFound() { - return false; } }); return hasField.get(); } - Object extract(final FieldPath path, Map payload) { + Object extract(FieldPath path, Object payload) { final List matches = new ArrayList(); traverse(new ProcessingContext(payload, path), new MatchCallback() { @Override - public boolean foundMatch(Match match) { + public void foundMatch(Match match) { matches.add(match.getValue()); - return true; - } - - @Override - public boolean matchNotFound() { - return false; } }); @@ -78,75 +66,59 @@ class FieldProcessor { } } - void remove(final FieldPath path, final Map payload) { + void remove(final FieldPath path, Object payload) { traverse(new ProcessingContext(payload, path), new MatchCallback() { @Override - public boolean foundMatch(Match match) { + public void foundMatch(Match match) { match.remove(); - return true; - } - - @Override - public boolean matchNotFound() { - return true; } }); } - private boolean traverse(ProcessingContext context, MatchCallback matchCallback) { + private void traverse(ProcessingContext context, MatchCallback matchCallback) { final String segment = context.getSegment(); if (FieldPath.isArraySegment(segment)) { if (context.getPayload() instanceof List) { - return handleListPayload(context, matchCallback); + handleListPayload(context, matchCallback); } } else if (context.getPayload() instanceof Map && ((Map) context.getPayload()).containsKey(segment)) { - return handleMapPayload(context, matchCallback); + handleMapPayload(context, matchCallback); } - - return matchCallback.matchNotFound(); } - private boolean handleListPayload(ProcessingContext context, - MatchCallback matchCallback) { + private void handleListPayload(ProcessingContext context, MatchCallback matchCallback) { List list = context.getPayload(); final Iterator items = list.iterator(); if (context.isLeaf()) { while (items.hasNext()) { Object item = items.next(); - if (!matchCallback.foundMatch(new ListMatch(items, list, item, context - .getParentMatch()))) { - return false; - } + matchCallback.foundMatch(new ListMatch(items, list, item, context + .getParentMatch())); } - return true; } else { - boolean result = true; - while (items.hasNext() && result) { + while (items.hasNext()) { Object item = items.next(); - result = result - && traverse(context.descend(item, new ListMatch(items, list, - item, context.parent)), matchCallback); + traverse(context.descend(item, new ListMatch(items, list, item, + context.parent)), matchCallback); } - return result; } } - private boolean handleMapPayload(ProcessingContext context, - MatchCallback matchCallback) { + private void handleMapPayload(ProcessingContext context, MatchCallback matchCallback) { Map map = context.getPayload(); - final Object item = map.get(context.getSegment()); + Object item = map.get(context.getSegment()); MapMatch mapMatch = new MapMatch(item, map, context.getSegment(), context.getParentMatch()); if (context.isLeaf()) { - return matchCallback.foundMatch(mapMatch); + matchCallback.foundMatch(mapMatch); } else { - return traverse(context.descend(item, mapMatch), matchCallback); + traverse(context.descend(item, mapMatch), matchCallback); } } @@ -216,9 +188,8 @@ class FieldProcessor { private interface MatchCallback { - boolean foundMatch(Match match); + void foundMatch(Match match); - boolean matchNotFound(); } private interface Match { diff --git a/spring-restdocs/src/main/java/org/springframework/restdocs/payload/FieldSnippetResultHandler.java b/spring-restdocs/src/main/java/org/springframework/restdocs/payload/FieldSnippetResultHandler.java index 7d9bd533..5ad94157 100644 --- a/spring-restdocs/src/main/java/org/springframework/restdocs/payload/FieldSnippetResultHandler.java +++ b/spring-restdocs/src/main/java/org/springframework/restdocs/payload/FieldSnippetResultHandler.java @@ -68,7 +68,7 @@ public abstract class FieldSnippetResultHandler extends SnippetWritingResultHand this.fieldValidator.validate(getPayloadReader(result), this.fieldDescriptors); - final Map payload = extractPayload(result); + final Object payload = extractPayload(result); writer.table(new TableAction() { @@ -91,15 +91,8 @@ public abstract class FieldSnippetResultHandler extends SnippetWritingResultHand } - @SuppressWarnings("unchecked") - private Map extractPayload(MvcResult result) throws IOException { - Reader payloadReader = getPayloadReader(result); - try { - return this.objectMapper.readValue(payloadReader, Map.class); - } - finally { - payloadReader.close(); - } + private Object extractPayload(MvcResult result) throws IOException { + return this.objectMapper.readValue(getPayloadReader(result), Object.class); } protected abstract Reader getPayloadReader(MvcResult result) throws IOException; diff --git a/spring-restdocs/src/main/java/org/springframework/restdocs/payload/FieldTypeResolver.java b/spring-restdocs/src/main/java/org/springframework/restdocs/payload/FieldTypeResolver.java index 89d2541b..dff50cda 100644 --- a/spring-restdocs/src/main/java/org/springframework/restdocs/payload/FieldTypeResolver.java +++ b/spring-restdocs/src/main/java/org/springframework/restdocs/payload/FieldTypeResolver.java @@ -28,7 +28,7 @@ class FieldTypeResolver { private final FieldProcessor fieldProcessor = new FieldProcessor(); - FieldType resolveFieldType(String path, Map payload) { + FieldType resolveFieldType(String path, Object payload) { FieldPath fieldPath = FieldPath.compile(path); Object field = this.fieldProcessor.extract(fieldPath, payload); if (field instanceof Collection && !fieldPath.isPrecise()) { diff --git a/spring-restdocs/src/main/java/org/springframework/restdocs/payload/FieldValidator.java b/spring-restdocs/src/main/java/org/springframework/restdocs/payload/FieldValidator.java index 4404ff68..9055cc87 100644 --- a/spring-restdocs/src/main/java/org/springframework/restdocs/payload/FieldValidator.java +++ b/spring-restdocs/src/main/java/org/springframework/restdocs/payload/FieldValidator.java @@ -40,18 +40,15 @@ class FieldValidator { private final ObjectMapper objectMapper = new ObjectMapper() .enable(SerializationFeature.INDENT_OUTPUT); - @SuppressWarnings("unchecked") void validate(Reader payloadReader, List fieldDescriptors) throws IOException { - Map payload = this.objectMapper.readValue(payloadReader, - Map.class); + Object payload = this.objectMapper.readValue(payloadReader, Object.class); List missingFields = findMissingFields(payload, fieldDescriptors); - Map undocumentedPayload = findUndocumentedFields(payload, - fieldDescriptors); + Object undocumentedPayload = findUndocumentedFields(payload, fieldDescriptors); - if (!missingFields.isEmpty() || !undocumentedPayload.isEmpty()) { + if (!missingFields.isEmpty() || !isEmpty(undocumentedPayload)) { String message = ""; - if (!undocumentedPayload.isEmpty()) { + if (!isEmpty(undocumentedPayload)) { message += String.format( "The following parts of the payload were not documented:%n%s", this.objectMapper.writeValueAsString(undocumentedPayload)); @@ -67,7 +64,14 @@ class FieldValidator { } } - private List findMissingFields(Map payload, + private boolean isEmpty(Object object) { + if (object instanceof Map) { + return ((Map) object).isEmpty(); + } + return ((List) object).isEmpty(); + } + + private List findMissingFields(Object payload, List fieldDescriptors) { List missingFields = new ArrayList(); @@ -82,7 +86,7 @@ class FieldValidator { return missingFields; } - private Map findUndocumentedFields(Map payload, + private Object findUndocumentedFields(Object payload, List fieldDescriptors) { for (FieldDescriptor fieldDescriptor : fieldDescriptors) { FieldPath path = FieldPath.compile(fieldDescriptor.getPath()); diff --git a/spring-restdocs/src/test/java/org/springframework/restdocs/payload/FieldPathTests.java b/spring-restdocs/src/test/java/org/springframework/restdocs/payload/FieldPathTests.java index e082aaf1..0e185720 100644 --- a/spring-restdocs/src/test/java/org/springframework/restdocs/payload/FieldPathTests.java +++ b/spring-restdocs/src/test/java/org/springframework/restdocs/payload/FieldPathTests.java @@ -16,7 +16,9 @@ package org.springframework.restdocs.payload; +import static org.hamcrest.Matchers.contains; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import org.junit.Test; @@ -38,6 +40,16 @@ public class FieldPathTests { assertTrue(FieldPath.compile("a.b").isPrecise()); } + @Test + public void topLevelArrayIsNotPrecise() { + assertFalse(FieldPath.compile("[]").isPrecise()); + } + + @Test + public void fieldBeneathTopLevelArrayIsNotPrecise() { + assertFalse(FieldPath.compile("[]a").isPrecise()); + } + @Test public void arrayIsNotPrecise() { assertFalse(FieldPath.compile("a[]").isPrecise()); @@ -58,4 +70,44 @@ public class FieldPathTests { assertFalse(FieldPath.compile("a[].b").isPrecise()); } + @Test + public void compilationOfSingleElementPath() { + assertThat(FieldPath.compile("a").getSegments(), contains("a")); + } + + @Test + public void compilationOfMultipleElementPath() { + assertThat(FieldPath.compile("a.b.c").getSegments(), contains("a", "b", "c")); + } + + @Test + public void compilationOfPathWithArraysWithNoDotSeparators() { + assertThat(FieldPath.compile("a[]b[]c").getSegments(), + contains("a", "[]", "b", "[]", "c")); + } + + @Test + public void compilationOfPathWithArraysWithPreAndPostDotSeparators() { + assertThat(FieldPath.compile("a.[].b.[].c").getSegments(), + contains("a", "[]", "b", "[]", "c")); + } + + @Test + public void compilationOfPathWithArraysWithPreDotSeparators() { + assertThat(FieldPath.compile("a.[]b.[]c").getSegments(), + contains("a", "[]", "b", "[]", "c")); + } + + @Test + public void compilationOfPathWithArraysWithPostDotSeparators() { + assertThat(FieldPath.compile("a[].b[].c").getSegments(), + contains("a", "[]", "b", "[]", "c")); + } + + @Test + public void compilationOfPathStartingWithAnArray() { + assertThat(FieldPath.compile("[]a.b.c").getSegments(), + contains("[]", "a", "b", "c")); + } + } diff --git a/spring-restdocs/src/test/java/org/springframework/restdocs/payload/FieldValidatorTests.java b/spring-restdocs/src/test/java/org/springframework/restdocs/payload/FieldValidatorTests.java index 081e3be5..e8ec0f29 100644 --- a/spring-restdocs/src/test/java/org/springframework/restdocs/payload/FieldValidatorTests.java +++ b/spring-restdocs/src/test/java/org/springframework/restdocs/payload/FieldValidatorTests.java @@ -38,6 +38,9 @@ public class FieldValidatorTests { @Rule public ExpectedException thrownException = ExpectedException.none(); + private StringReader listPayload = new StringReader( + "[{\"a\":1},{\"a\":2},{\"b\":{\"c\":3}}]"); + private StringReader payload = new StringReader( "{\"a\":{\"b\":{},\"c\":true,\"d\":[{\"e\":1},{\"e\":2}]}}"); @@ -88,4 +91,24 @@ public class FieldValidatorTests { this.fieldValidator.validate(this.payload, Arrays.asList(new FieldDescriptor("a.b"), new FieldDescriptor("a.d"))); } + + @Test + public void listPayloadNoMissingFieldsAllFieldsDocumented() throws IOException { + this.fieldValidator.validate(this.listPayload, Arrays.asList(new FieldDescriptor( + "[]b.c"), new FieldDescriptor("[]b"), new FieldDescriptor("[]a"), + new FieldDescriptor("[]"))); + } + + @Test + public void listPayloadParentIsDocumentedWhenAllChildrenAreDocumented() + throws IOException { + this.fieldValidator.validate(this.listPayload, + Arrays.asList(new FieldDescriptor("[]b.c"), new FieldDescriptor("[]a"))); + } + + @Test + public void listPayloadChildIsDocumentedWhenParentIsDocumented() throws IOException { + this.fieldValidator.validate(this.listPayload, + Arrays.asList(new FieldDescriptor("[]"))); + } } diff --git a/spring-restdocs/src/test/java/org/springframework/restdocs/payload/PayloadDocumentationTests.java b/spring-restdocs/src/test/java/org/springframework/restdocs/payload/PayloadDocumentationTests.java index 38a8d38c..e8750ccf 100644 --- a/spring-restdocs/src/test/java/org/springframework/restdocs/payload/PayloadDocumentationTests.java +++ b/spring-restdocs/src/test/java/org/springframework/restdocs/payload/PayloadDocumentationTests.java @@ -48,14 +48,14 @@ public class PayloadDocumentationTests { public final ExpectedSnippet snippet = new ExpectedSnippet(); @Test - public void requestWithFields() throws IOException { - this.snippet.expectRequestFields("request-with-fields").withContents( // + public void mapRequestWithFields() throws IOException { + this.snippet.expectRequestFields("map-request-with-fields").withContents( // tableWithHeader("Path", "Type", "Description") // .row("a.b", "Number", "one") // .row("a.c", "String", "two") // .row("a", "Object", "three")); - documentRequestFields("request-with-fields", + documentRequestFields("map-request-with-fields", fieldWithPath("a.b").description("one"), fieldWithPath("a.c").description("two"), fieldWithPath("a").description("three")).handle( @@ -63,8 +63,24 @@ public class PayloadDocumentationTests { } @Test - public void responseWithFields() throws IOException { - this.snippet.expectResponseFields("response-with-fields").withContents(// + public void arrayRequestWithFields() throws IOException { + this.snippet.expectRequestFields("array-request-with-fields").withContents( // + tableWithHeader("Path", "Type", "Description") // + .row("[]a.b", "Number", "one") // + .row("[]a.c", "String", "two") // + .row("[]a", "Object", "three")); + + documentRequestFields("array-request-with-fields", + fieldWithPath("[]a.b").description("one"), + fieldWithPath("[]a.c").description("two"), + fieldWithPath("[]a").description("three")).handle( + result(get("/foo").content( + "[{\"a\": {\"b\": 5}},{\"a\": {\"c\": \"charlie\"}}]"))); + } + + @Test + public void mapResponseWithFields() throws IOException { + this.snippet.expectResponseFields("map-response-with-fields").withContents(// tableWithHeader("Path", "Type", "Description") // .row("id", "Number", "one") // .row("date", "String", "two") // @@ -77,7 +93,7 @@ public class PayloadDocumentationTests { response.getWriter().append( "{\"id\": 67,\"date\": \"2015-01-20\",\"assets\":" + " [{\"id\":356,\"name\": \"sample\"}]}"); - documentResponseFields("response-with-fields", + documentResponseFields("map-response-with-fields", fieldWithPath("id").description("one"), fieldWithPath("date").description("two"), fieldWithPath("assets").description("three"), @@ -87,6 +103,23 @@ public class PayloadDocumentationTests { result(response)); } + @Test + public void arrayResponseWithFields() throws IOException { + this.snippet.expectResponseFields("array-response-with-fields").withContents( // + tableWithHeader("Path", "Type", "Description") // + .row("[]a.b", "Number", "one") // + .row("[]a.c", "String", "two") // + .row("[]a", "Object", "three")); + + MockHttpServletResponse response = new MockHttpServletResponse(); + response.getWriter() + .append("[{\"a\": {\"b\": 5}},{\"a\": {\"c\": \"charlie\"}}]"); + documentResponseFields("array-response-with-fields", + fieldWithPath("[]a.b").description("one"), + fieldWithPath("[]a.c").description("two"), + fieldWithPath("[]a").description("three")).handle(result(response)); + } + @Test public void undocumentedRequestField() throws IOException { this.thrown.expect(SnippetGenerationException.class);