From 9e1ba8a92291f7f91d858ca7fc76d67e61f57066 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Thu, 22 Nov 2018 11:00:23 +0000 Subject: [PATCH] Improve beneathPath to work with multiple matches with common structure Closes gh-473 --- .../FieldPathPayloadSubsectionExtractor.java | 26 +++-- .../restdocs/payload/JsonFieldPaths.java | 92 +++++++++++++++ ...ldPathPayloadSubsectionExtractorTests.java | 56 +++++---- .../restdocs/payload/JsonFieldPathsTests.java | 107 ++++++++++++++++++ .../payload/ResponseFieldsSnippetTests.java | 13 +++ 5 files changed, 263 insertions(+), 31 deletions(-) create mode 100644 spring-restdocs-core/src/main/java/org/springframework/restdocs/payload/JsonFieldPaths.java create mode 100644 spring-restdocs-core/src/test/java/org/springframework/restdocs/payload/JsonFieldPathsTests.java diff --git a/spring-restdocs-core/src/main/java/org/springframework/restdocs/payload/FieldPathPayloadSubsectionExtractor.java b/spring-restdocs-core/src/main/java/org/springframework/restdocs/payload/FieldPathPayloadSubsectionExtractor.java index 69b07cbb..9b0b0fbb 100644 --- a/spring-restdocs-core/src/main/java/org/springframework/restdocs/payload/FieldPathPayloadSubsectionExtractor.java +++ b/spring-restdocs-core/src/main/java/org/springframework/restdocs/payload/FieldPathPayloadSubsectionExtractor.java @@ -17,12 +17,13 @@ package org.springframework.restdocs.payload; import java.io.IOException; +import java.util.ArrayList; import java.util.List; +import java.util.Set; import com.fasterxml.jackson.databind.ObjectMapper; import org.springframework.http.MediaType; -import org.springframework.restdocs.payload.JsonFieldPath.PathType; import org.springframework.restdocs.payload.JsonFieldProcessor.ExtractedField; /** @@ -44,7 +45,7 @@ public class FieldPathPayloadSubsectionExtractor /** * Creates a new {@code FieldPathPayloadSubsectionExtractor} that will extract the - * subsection of the JSON payload found at the given {@code fieldPath}. The + * subsection of the JSON payload beneath the given {@code fieldPath}. The * {@code fieldPath} prefixed with {@code beneath-} with be used as the subsection ID. * @param fieldPath the path of the field */ @@ -54,8 +55,8 @@ public class FieldPathPayloadSubsectionExtractor /** * Creates a new {@code FieldPathPayloadSubsectionExtractor} that will extract the - * subsection of the JSON payload found at the given {@code fieldPath} and that will - * us the given {@code subsectionId} to identify the subsection. + * subsection of the JSON payload beneath the given {@code fieldPath} and that will + * use the given {@code subsectionId} to identify the subsection. * @param fieldPath the path of the field * @param subsectionId the ID of the subsection */ @@ -70,14 +71,23 @@ public class FieldPathPayloadSubsectionExtractor ExtractedField extractedField = new JsonFieldProcessor().extract( this.fieldPath, objectMapper.readValue(payload, Object.class)); Object value = extractedField.getValue(); - if (value instanceof List && extractedField.getType() == PathType.MULTI) { + if (value instanceof List) { List extractedList = (List) value; - if (extractedList.size() == 1) { + Set uncommonPaths = JsonFieldPaths.from(extractedList) + .getUncommon(); + if (uncommonPaths.isEmpty()) { value = extractedList.get(0); } else { - throw new PayloadHandlingException(this.fieldPath - + " does not uniquely identify a subsection of the payload"); + String message = this.fieldPath + " identifies multiple sections of " + + "the payload and they do not have a common structure. The " + + "following uncommon paths were found: "; + List prefixedPaths = new ArrayList<>(); + for (String uncommonPath : uncommonPaths) { + prefixedPaths.add(this.fieldPath + "." + uncommonPath); + } + message += prefixedPaths; + throw new PayloadHandlingException(message); } } return objectMapper.writeValueAsBytes(value); diff --git a/spring-restdocs-core/src/main/java/org/springframework/restdocs/payload/JsonFieldPaths.java b/spring-restdocs-core/src/main/java/org/springframework/restdocs/payload/JsonFieldPaths.java new file mode 100644 index 00000000..cfd622bf --- /dev/null +++ b/spring-restdocs-core/src/main/java/org/springframework/restdocs/payload/JsonFieldPaths.java @@ -0,0 +1,92 @@ +/* + * Copyright 2014-2018 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.payload; + +import java.util.Collection; +import java.util.HashSet; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; +import java.util.Set; + +/** + * {@code JsonFieldPaths} provides support for extracting fields paths from JSON + * structures and identifying uncommon paths. + * + * @author Andy Wilkinson + */ +final class JsonFieldPaths { + + private final Set uncommonFieldPaths; + + private JsonFieldPaths(Set uncommonFieldPaths) { + this.uncommonFieldPaths = uncommonFieldPaths; + } + + Set getUncommon() { + return this.uncommonFieldPaths; + } + + static JsonFieldPaths from(Collection items) { + Set> itemsFieldPaths = new HashSet<>(); + Set allFieldPaths = new HashSet<>(); + for (Object item : items) { + Set paths = new LinkedHashSet<>(); + from(paths, "", item); + itemsFieldPaths.add(paths); + allFieldPaths.addAll(paths); + } + Set uncommonFieldPaths = new HashSet<>(); + for (Set itemFieldPaths : itemsFieldPaths) { + Set uncommonForItem = new HashSet<>(allFieldPaths); + uncommonForItem.removeAll(itemFieldPaths); + uncommonFieldPaths.addAll(uncommonForItem); + } + return new JsonFieldPaths(uncommonFieldPaths); + } + + private static void from(Set paths, String parent, Object object) { + if (object instanceof List) { + String path = append(parent, "[]"); + paths.add(path); + from(paths, path, (List) object); + } + else if (object instanceof Map) { + from(paths, parent, (Map) object); + } + } + + private static void from(Set paths, String parent, List items) { + for (Object item : items) { + from(paths, parent, item); + } + } + + private static void from(Set paths, String parent, Map map) { + for (Entry entry : map.entrySet()) { + String path = append(parent, entry.getKey()); + paths.add(path); + from(paths, path, entry.getValue()); + } + } + + private static String append(String path, Object suffix) { + return (path.length() == 0) ? ("" + suffix) : (path + "." + suffix); + } + +} diff --git a/spring-restdocs-core/src/test/java/org/springframework/restdocs/payload/FieldPathPayloadSubsectionExtractorTests.java b/spring-restdocs-core/src/test/java/org/springframework/restdocs/payload/FieldPathPayloadSubsectionExtractorTests.java index 42c7ae07..7ef2306f 100644 --- a/spring-restdocs-core/src/test/java/org/springframework/restdocs/payload/FieldPathPayloadSubsectionExtractorTests.java +++ b/spring-restdocs-core/src/test/java/org/springframework/restdocs/payload/FieldPathPayloadSubsectionExtractorTests.java @@ -17,7 +17,6 @@ package org.springframework.restdocs.payload; import java.io.IOException; -import java.util.List; import java.util.Map; import com.fasterxml.jackson.core.JsonParseException; @@ -30,7 +29,6 @@ import org.junit.rules.ExpectedException; import org.springframework.http.MediaType; import static org.assertj.core.api.Assertions.assertThat; -import static org.hamcrest.CoreMatchers.equalTo; /** * Tests for {@link FieldPathPayloadSubsectionExtractor}. @@ -55,20 +53,6 @@ public class FieldPathPayloadSubsectionExtractorTests { assertThat(extracted.get("c")).isEqualTo(5); } - @Test - @SuppressWarnings("unchecked") - public void extractMultiElementArraySubsectionOfJsonMap() - throws JsonParseException, JsonMappingException, IOException { - byte[] extractedPayload = new FieldPathPayloadSubsectionExtractor("a") - .extractSubsection("{\"a\":[{\"b\":5},{\"b\":4}]}".getBytes(), - MediaType.APPLICATION_JSON); - List> extracted = new ObjectMapper() - .readValue(extractedPayload, List.class); - assertThat(extracted.size()).isEqualTo(2); - assertThat(extracted.get(0).get("b")).isEqualTo(5); - assertThat(extracted.get(1).get("b")).isEqualTo(4); - } - @Test @SuppressWarnings("unchecked") public void extractSingleElementArraySubsectionOfJsonMap() @@ -76,10 +60,23 @@ public class FieldPathPayloadSubsectionExtractorTests { byte[] extractedPayload = new FieldPathPayloadSubsectionExtractor("a.[]") .extractSubsection("{\"a\":[{\"b\":5}]}".getBytes(), MediaType.APPLICATION_JSON); - List> extracted = new ObjectMapper() - .readValue(extractedPayload, List.class); + Map extracted = new ObjectMapper().readValue(extractedPayload, + Map.class); assertThat(extracted.size()).isEqualTo(1); - assertThat(extracted.get(0).get("b")).isEqualTo(5); + assertThat(extracted).containsOnlyKeys("b"); + } + + @Test + @SuppressWarnings("unchecked") + public void extractMultiElementArraySubsectionOfJsonMap() + throws JsonParseException, JsonMappingException, IOException { + byte[] extractedPayload = new FieldPathPayloadSubsectionExtractor("a") + .extractSubsection("{\"a\":[{\"b\":5},{\"b\":4}]}".getBytes(), + MediaType.APPLICATION_JSON); + Map extracted = new ObjectMapper().readValue(extractedPayload, + Map.class); + assertThat(extracted.size()).isEqualTo(1); + assertThat(extracted).containsOnlyKeys("b"); } @Test @@ -96,13 +93,26 @@ public class FieldPathPayloadSubsectionExtractorTests { } @Test - public void extractMapSubsectionFromMultiElementArrayInAJsonMap() + @SuppressWarnings("unchecked") + public void extractMapSubsectionWithCommonStructureFromMultiElementArrayInAJsonMap() + throws JsonParseException, JsonMappingException, IOException { + byte[] extractedPayload = new FieldPathPayloadSubsectionExtractor("a.[].b") + .extractSubsection( + "{\"a\":[{\"b\":{\"c\":5}},{\"b\":{\"c\":6}}]}".getBytes(), + MediaType.APPLICATION_JSON); + Map extracted = new ObjectMapper().readValue(extractedPayload, + Map.class); + assertThat(extracted.size()).isEqualTo(1); + assertThat(extracted).containsOnlyKeys("c"); + } + + @Test + public void extractMapSubsectionWithVaryingStructureFromMultiElementArrayInAJsonMap() throws JsonParseException, JsonMappingException, IOException { this.thrown.expect(PayloadHandlingException.class); - this.thrown.expectMessage( - equalTo("a.[].b does not uniquely identify a subsection of the payload")); + this.thrown.expectMessage("The following uncommon paths were found: [a.[].b.d]"); new FieldPathPayloadSubsectionExtractor("a.[].b").extractSubsection( - "{\"a\":[{\"b\":{\"c\":5}},{\"b\":{\"c\":6}}]}".getBytes(), + "{\"a\":[{\"b\":{\"c\":5}},{\"b\":{\"c\":6, \"d\": 7}}]}".getBytes(), MediaType.APPLICATION_JSON); } diff --git a/spring-restdocs-core/src/test/java/org/springframework/restdocs/payload/JsonFieldPathsTests.java b/spring-restdocs-core/src/test/java/org/springframework/restdocs/payload/JsonFieldPathsTests.java new file mode 100644 index 00000000..51dfd9e9 --- /dev/null +++ b/spring-restdocs-core/src/test/java/org/springframework/restdocs/payload/JsonFieldPathsTests.java @@ -0,0 +1,107 @@ +/* + * Copyright 2014-2018 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.payload; + +import java.io.IOException; +import java.util.Arrays; + +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link JsonFieldPaths}. + * + * @author Andy Wilkinson + */ +public class JsonFieldPathsTests { + + @Test + public void noUncommonPathsForSingleItem() { + assertThat(JsonFieldPaths + .from(Arrays + .asList(json("{\"a\": 1, \"b\": [ { \"c\": 2}, {\"c\": 3} ]}"))) + .getUncommon()).isEmpty(); + } + + @Test + public void noUncommonPathsForMultipleIdenticalItems() { + Object item = json("{\"a\": 1, \"b\": [ { \"c\": 2}, {\"c\": 3} ]}"); + assertThat(JsonFieldPaths.from(Arrays.asList(item, item)).getUncommon()) + .isEmpty(); + } + + @Test + public void noUncommonPathsForMultipleMatchingItemsWithDifferentScalarValues() { + assertThat(JsonFieldPaths + .from(Arrays.asList( + json("{\"a\": 1, \"b\": [ { \"c\": 2}, {\"c\": 3} ]}"), + json("{\"a\": 4, \"b\": [ { \"c\": 5}, {\"c\": 6} ]}"))) + .getUncommon()).isEmpty(); + } + + @Test + public void missingEntryInMapIsIdentifiedAsUncommon() { + assertThat(JsonFieldPaths.from(Arrays.asList(json("{\"a\": 1}"), + json("{\"a\": 1}"), json("{\"a\": 1, \"b\": 2}"))).getUncommon()) + .containsExactly("b"); + } + + @Test + public void missingEntryInNestedMapIsIdentifiedAsUncommon() { + assertThat( + JsonFieldPaths + .from(Arrays.asList(json("{\"a\": 1, \"b\": {\"c\": 1}}"), + json("{\"a\": 1, \"b\": {\"c\": 1}}"), + json("{\"a\": 1, \"b\": {\"c\": 1, \"d\": 2}}"))) + .getUncommon()).containsExactly("b.d"); + } + + @Test + public void missingEntriesInNestedMapAreIdentifiedAsUncommon() { + assertThat( + JsonFieldPaths.from(Arrays.asList(json("{\"a\": 1, \"b\": {\"c\": 1}}"), + json("{\"a\": 1, \"b\": {\"c\": 1}}"), + json("{\"a\": 1, \"b\": {\"d\": 2}}"))).getUncommon()) + .containsExactly("b.c", "b.d"); + } + + @Test + public void missingEntryBeneathArrayIsIdentifiedAsUncommon() { + assertThat(JsonFieldPaths.from(Arrays.asList(json("[{\"b\": 1}]"), + json("[{\"b\": 1}]"), json("[{\"b\": 1, \"c\": 2}]"))).getUncommon()) + .containsExactly("[].c"); + } + + @Test + public void missingEntryBeneathNestedArrayIsIdentifiedAsUncommon() { + assertThat(JsonFieldPaths.from(Arrays.asList(json("{\"a\": [{\"b\": 1}]}"), + json("{\"a\": [{\"b\": 1}]}"), json("{\"a\": [{\"b\": 1, \"c\": 2}]}"))) + .getUncommon()).containsExactly("a.[].c"); + } + + private Object json(String json) { + try { + return new ObjectMapper().readValue(json, Object.class); + } + catch (IOException ex) { + throw new RuntimeException(ex); + } + } + +} 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 d31448d8..e2f4c4a8 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 @@ -94,6 +94,19 @@ public class ResponseFieldsSnippetTests extends AbstractSnippetTests { .row("`b`", "`Number`", "one").row("`c`", "`String`", "two")); } + @Test + public void subsectionOfMapResponseBeneathAnArray() throws IOException { + responseFields(beneathPath("a.b.[]"), fieldWithPath("c").description("one"), + fieldWithPath("d.[].e").description("two")) + .document(this.operationBuilder.response().content( + "{\"a\": {\"b\": [{\"c\": 1, \"d\": [{\"e\": 5}]}, {\"c\": 3, \"d\": [{\"e\": 4}]}]}}") + .build()); + assertThat(this.generatedSnippets.snippet("response-fields-beneath-a.b.[]")) + .is(tableWithHeader("Path", "Type", "Description") + .row("`c`", "`Number`", "one") + .row("`d.[].e`", "`Number`", "two")); + } + @Test public void subsectionOfMapResponseWithCommonsPrefix() throws IOException { responseFields(beneathPath("a"))