Update field snippets to no longer document whole subsection by default
Previously, when a field was documented it would implicitly document the whole subsection of the payload identified by that field. This could lead to users inadvertently failing to document part of the payload. Arguably, this was a bug as it violated REST Docs' principle of producing accurate, detail documentation. However, fixing it requires a breaking change as people may also be relying on this behaviour. A balance needed to be struck so the fix is being made in a minor release. This commit introduces a new subsectionWithPath method which returns a SubsectionDescriptor; a specialisation of FieldDescriptor. Users that were intentionally relying on the old behaviour will have to replace some usage of fieldWithPath with subsectionWithPath instead. Users who were unintentionally relying on the old behaviour will have to add some additional descriptors produced using fieldWithPath and will receive more accurate documentation in return. Closes gh-274
This commit is contained in:
@@ -201,6 +201,26 @@ public class JsonFieldProcessorTests {
|
||||
assertThat(payload.size(), equalTo(0));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void mapWithEntriesIsNotRemovedWhenNotAlsoRemovingDescendants() {
|
||||
Map<String, Object> payload = new HashMap<>();
|
||||
Map<String, Object> alpha = new HashMap<>();
|
||||
payload.put("a", alpha);
|
||||
alpha.put("b", "bravo");
|
||||
this.fieldProcessor.remove(JsonFieldPath.compile("a"), payload);
|
||||
assertThat(payload.size(), equalTo(1));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void removeSubsectionRemovesMapWithEntries() {
|
||||
Map<String, Object> payload = new HashMap<>();
|
||||
Map<String, Object> alpha = new HashMap<>();
|
||||
payload.put("a", alpha);
|
||||
alpha.put("b", "bravo");
|
||||
this.fieldProcessor.removeSubsection(JsonFieldPath.compile("a"), payload);
|
||||
assertThat(payload.size(), equalTo(0));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void removeNestedMapEntry() {
|
||||
Map<String, Object> payload = new HashMap<>();
|
||||
@@ -229,6 +249,51 @@ public class JsonFieldProcessorTests {
|
||||
assertThat(payload.size(), equalTo(0));
|
||||
}
|
||||
|
||||
@SuppressWarnings("unchecked")
|
||||
@Test
|
||||
public void removeDoesNotRemoveArrayWithMapEntries() throws IOException {
|
||||
Map<String, Object> payload = new ObjectMapper()
|
||||
.readValue("{\"a\": [{\"b\":\"bravo\"},{\"b\":\"bravo\"}]}", Map.class);
|
||||
this.fieldProcessor.remove(JsonFieldPath.compile("a[]"), payload);
|
||||
assertThat(payload.size(), equalTo(1));
|
||||
}
|
||||
|
||||
@SuppressWarnings("unchecked")
|
||||
@Test
|
||||
public void removeDoesNotRemoveArrayWithListEntries() throws IOException {
|
||||
Map<String, Object> payload = new ObjectMapper().readValue("{\"a\": [[2],[3]]}",
|
||||
Map.class);
|
||||
this.fieldProcessor.remove(JsonFieldPath.compile("a[]"), payload);
|
||||
assertThat(payload.size(), equalTo(1));
|
||||
}
|
||||
|
||||
@SuppressWarnings("unchecked")
|
||||
@Test
|
||||
public void removeRemovesArrayWithOnlyScalarEntries() throws IOException {
|
||||
Map<String, Object> payload = new ObjectMapper()
|
||||
.readValue("{\"a\": [\"bravo\", \"charlie\"]}", Map.class);
|
||||
this.fieldProcessor.remove(JsonFieldPath.compile("a"), payload);
|
||||
assertThat(payload.size(), equalTo(0));
|
||||
}
|
||||
|
||||
@SuppressWarnings("unchecked")
|
||||
@Test
|
||||
public void removeSubsectionRemovesArrayWithMapEntries() throws IOException {
|
||||
Map<String, Object> payload = new ObjectMapper()
|
||||
.readValue("{\"a\": [{\"b\":\"bravo\"},{\"b\":\"bravo\"}]}", Map.class);
|
||||
this.fieldProcessor.removeSubsection(JsonFieldPath.compile("a[]"), payload);
|
||||
assertThat(payload.size(), equalTo(0));
|
||||
}
|
||||
|
||||
@SuppressWarnings("unchecked")
|
||||
@Test
|
||||
public void removeSubsectionRemovesArrayWithListEntries() throws IOException {
|
||||
Map<String, Object> payload = new ObjectMapper().readValue("{\"a\": [[2],[3]]}",
|
||||
Map.class);
|
||||
this.fieldProcessor.removeSubsection(JsonFieldPath.compile("a[]"), payload);
|
||||
assertThat(payload.size(), equalTo(0));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void extractNestedEntryWithDotInKeys() throws IOException {
|
||||
Map<String, Object> payload = new HashMap<>();
|
||||
|
||||
@@ -131,8 +131,8 @@ public class RequestFieldsSnippetFailureTests {
|
||||
@Test
|
||||
public void undocumentedXmlRequestField() throws IOException {
|
||||
this.thrown.expect(SnippetException.class);
|
||||
this.thrown.expectMessage(startsWith(
|
||||
"The following parts of the payload were not" + " documented:"));
|
||||
this.thrown.expectMessage(
|
||||
startsWith("The following parts of the payload were not documented:"));
|
||||
new RequestFieldsSnippet(Collections.<FieldDescriptor>emptyList())
|
||||
.document(this.operationBuilder.request("http://localhost")
|
||||
.content("<a><b>5</b></a>").header(HttpHeaders.CONTENT_TYPE,
|
||||
@@ -140,6 +140,20 @@ public class RequestFieldsSnippetFailureTests {
|
||||
.build());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void xmlDescendentsAreNotDocumentedByFieldDescriptor() throws IOException {
|
||||
this.thrown.expect(SnippetException.class);
|
||||
this.thrown.expectMessage(
|
||||
startsWith("The following parts of the payload were not documented:"));
|
||||
new RequestFieldsSnippet(
|
||||
Arrays.asList(fieldWithPath("a").type("a").description("one")))
|
||||
.document(this.operationBuilder.request("http://localhost")
|
||||
.content("<a><b>5</b></a>")
|
||||
.header(HttpHeaders.CONTENT_TYPE,
|
||||
MediaType.APPLICATION_XML_VALUE)
|
||||
.build());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void xmlRequestFieldWithNoType() throws IOException {
|
||||
this.thrown.expect(FieldTypeRequiredException.class);
|
||||
|
||||
@@ -36,6 +36,7 @@ import static org.mockito.Mockito.mock;
|
||||
import static org.springframework.restdocs.payload.PayloadDocumentation.beneathPath;
|
||||
import static org.springframework.restdocs.payload.PayloadDocumentation.fieldWithPath;
|
||||
import static org.springframework.restdocs.payload.PayloadDocumentation.requestFields;
|
||||
import static org.springframework.restdocs.payload.PayloadDocumentation.subsectionWithPath;
|
||||
import static org.springframework.restdocs.snippet.Attributes.attributes;
|
||||
import static org.springframework.restdocs.snippet.Attributes.key;
|
||||
|
||||
@@ -65,6 +66,19 @@ public class RequestFieldsSnippetTests extends AbstractSnippetTests {
|
||||
.build());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void entireSubsectionsCanBeDocumented() throws IOException {
|
||||
this.snippets.expectRequestFields()
|
||||
.withContents(tableWithHeader("Path", "Type", "Description").row("`a`",
|
||||
"`Object`", "one"));
|
||||
|
||||
new RequestFieldsSnippet(
|
||||
Arrays.asList(subsectionWithPath("a").description("one")))
|
||||
.document(this.operationBuilder.request("http://localhost")
|
||||
.content("{\"a\": {\"b\": 5, \"c\": \"charlie\"}}")
|
||||
.build());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void subsectionOfMapRequest() throws IOException {
|
||||
this.snippets.expect("request-fields-beneath-a")
|
||||
@@ -121,6 +135,18 @@ public class RequestFieldsSnippetTests extends AbstractSnippetTests {
|
||||
.content("{\"a\": 5, \"b\": 4}").build());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void entireSubsectionCanBeIgnored() throws IOException {
|
||||
this.snippets.expectRequestFields()
|
||||
.withContents(tableWithHeader("Path", "Type", "Description").row("`c`",
|
||||
"`Number`", "Field c"));
|
||||
|
||||
new RequestFieldsSnippet(Arrays.asList(subsectionWithPath("a").ignored(),
|
||||
fieldWithPath("c").description("Field c")))
|
||||
.document(this.operationBuilder.request("http://localhost")
|
||||
.content("{\"a\": {\"b\": 5}, \"c\": 4}").build());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void allUndocumentedRequestFieldsCanBeIgnored() throws IOException {
|
||||
this.snippets.expectRequestFields()
|
||||
@@ -256,6 +282,20 @@ public class RequestFieldsSnippetTests extends AbstractSnippetTests {
|
||||
.build());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void entireSubsectionOfXmlPayloadCanBeDocumented() throws IOException {
|
||||
this.snippets.expectRequestFields().withContents(
|
||||
tableWithHeader("Path", "Type", "Description").row("`a`", "`a`", "one"));
|
||||
|
||||
new RequestFieldsSnippet(
|
||||
Arrays.asList(subsectionWithPath("a").description("one").type("a")))
|
||||
.document(this.operationBuilder.request("http://localhost")
|
||||
.content("<a><b>5</b><c>charlie</c></a>")
|
||||
.header(HttpHeaders.CONTENT_TYPE,
|
||||
MediaType.APPLICATION_XML_VALUE)
|
||||
.build());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void additionalDescriptors() throws IOException {
|
||||
this.snippets.expectRequestFields()
|
||||
|
||||
@@ -0,0 +1,91 @@
|
||||
/*
|
||||
* 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.payload;
|
||||
|
||||
import java.util.Arrays;
|
||||
|
||||
import org.junit.Test;
|
||||
|
||||
import static org.hamcrest.CoreMatchers.equalTo;
|
||||
import static org.hamcrest.CoreMatchers.is;
|
||||
import static org.hamcrest.CoreMatchers.nullValue;
|
||||
import static org.junit.Assert.assertThat;
|
||||
import static org.springframework.restdocs.payload.PayloadDocumentation.fieldWithPath;
|
||||
import static org.springframework.restdocs.payload.PayloadDocumentation.subsectionWithPath;
|
||||
|
||||
/**
|
||||
* Tests for {@link XmlContentHandler}.
|
||||
*
|
||||
* @author Andy Wilkinson
|
||||
*/
|
||||
public class XmlContentHandlerTests {
|
||||
|
||||
@Test
|
||||
public void topLevelElementCanBeDocumented() {
|
||||
String undocumentedContent = createHandler("<a>5</a>").getUndocumentedContent(
|
||||
Arrays.asList(fieldWithPath("a").type("a").description("description")));
|
||||
assertThat(undocumentedContent, is(nullValue()));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void nestedElementCanBeDocumentedLeavingAncestors() {
|
||||
String undocumentedContent = createHandler("<a><b>5</b></a>")
|
||||
.getUndocumentedContent(Arrays.asList(
|
||||
fieldWithPath("a/b").type("b").description("description")));
|
||||
assertThat(undocumentedContent, is(equalTo(String.format("<a/>%n"))));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void fieldDescriptorDoesNotDocumentEntireSubsection() {
|
||||
String undocumentedContent = createHandler("<a><b>5</b></a>")
|
||||
.getUndocumentedContent(Arrays
|
||||
.asList(fieldWithPath("a").type("a").description("description")));
|
||||
assertThat(undocumentedContent,
|
||||
is(equalTo(String.format("<a>%n <b>5</b>%n</a>%n"))));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void subsectionDescriptorDocumentsEntireSubsection() {
|
||||
String undocumentedContent = createHandler("<a><b>5</b></a>")
|
||||
.getUndocumentedContent(Arrays.asList(
|
||||
subsectionWithPath("a").type("a").description("description")));
|
||||
assertThat(undocumentedContent, is(nullValue()));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void multipleElementsCanBeInDescendingOrderDocumented() {
|
||||
String undocumentedContent = createHandler("<a><b>5</b></a>")
|
||||
.getUndocumentedContent(Arrays.asList(
|
||||
fieldWithPath("a").type("a").description("description"),
|
||||
fieldWithPath("a/b").type("b").description("description")));
|
||||
assertThat(undocumentedContent, is(nullValue()));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void multipleElementsCanBeInAscendingOrderDocumented() {
|
||||
String undocumentedContent = createHandler("<a><b>5</b></a>")
|
||||
.getUndocumentedContent(Arrays.asList(
|
||||
fieldWithPath("a/b").type("b").description("description"),
|
||||
fieldWithPath("a").type("a").description("description")));
|
||||
assertThat(undocumentedContent, is(nullValue()));
|
||||
}
|
||||
|
||||
private XmlContentHandler createHandler(String xml) {
|
||||
return new XmlContentHandler(xml.getBytes());
|
||||
}
|
||||
|
||||
}
|
||||
Reference in New Issue
Block a user