Skip to content

Provide a public API for resolving the type of a field #549

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
*
* @author Andreas Evers
* @author Andy Wilkinson
* @author Mathias Düsterhöft
*/
public abstract class AbstractFieldsSnippet extends TemplatedSnippet {

Expand Down Expand Up @@ -160,15 +161,15 @@ protected Map<String, Object> createModel(Operation operation) {
content = verifyContent(
this.subsectionExtractor.extractSubsection(content, contentType));
}
ContentHandler contentHandler = getContentHandler(content, contentType);
ContentHandler contentHandler = ContentHandler.forContent(content, contentType);

validateFieldDocumentation(contentHandler);

List<FieldDescriptor> descriptorsToDocument = new ArrayList<>();
for (FieldDescriptor descriptor : this.fieldDescriptors) {
if (!descriptor.isIgnored()) {
try {
Object type = contentHandler.determineFieldType(descriptor);
Object type = contentHandler.resolveFieldType(descriptor);
descriptorsToDocument.add(copyWithType(descriptor, type));
}
catch (FieldDoesNotExistException ex) {
Expand Down Expand Up @@ -200,36 +201,6 @@ private byte[] verifyContent(byte[] content) {
return content;
}

private ContentHandler getContentHandler(byte[] content, MediaType contentType) {
ContentHandler contentHandler = createJsonContentHandler(content);
if (contentHandler == null) {
contentHandler = createXmlContentHandler(content);
if (contentHandler == null) {
throw new PayloadHandlingException("Cannot handle " + contentType
+ " content as it could not be parsed as JSON or XML");
}
}
return contentHandler;
}

private ContentHandler createJsonContentHandler(byte[] content) {
try {
return new JsonContentHandler(content);
}
catch (Exception ex) {
return null;
}
}

private ContentHandler createXmlContentHandler(byte[] content) {
try {
return new XmlContentHandler(content);
}
catch (Exception ex) {
return null;
}
}

private void validateFieldDocumentation(ContentHandler payloadHandler) {
List<FieldDescriptor> missingFields = payloadHandler
.findMissingFields(this.fieldDescriptors);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,15 @@

import java.util.List;

import org.springframework.http.MediaType;

/**
* A handler for the content of a request or response.
*
* @author Andy Wilkinson
* @author Mathias Düsterhöft
*/
interface ContentHandler {
interface ContentHandler extends FieldTypeResolver {

/**
* Finds the fields that are missing from the handler's payload. A field is missing if
Expand All @@ -48,11 +51,26 @@ interface ContentHandler {
String getUndocumentedContent(List<FieldDescriptor> fieldDescriptors);

/**
* Returns the type of the field that is described by the given
* {@code fieldDescriptor} based on the content of the payload.
* @param fieldDescriptor the field descriptor
* @return the type of the field
* Create a {@link ContentHandler} for the given content type and payload.
* @param content the payload
* @param contentType the content type
* @return the ContentHandler
* @throws PayloadHandlingException if no known ContentHandler can handle the content
*/
Object determineFieldType(FieldDescriptor fieldDescriptor);
static ContentHandler forContent(byte[] content, MediaType contentType) {

try {
return new JsonContentHandler(content);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wilkinsona I inlined the helper functions here because I could not keep them private in the interface. That is why I initially went for the factory class in the earlier version. This is now a little ugly. I could make ContentHandler an abstract class to keep the initial structure.

}
catch (Exception je) {
try {
return new XmlContentHandler(content);
}
catch (Exception xe) {
throw new PayloadHandlingException("Cannot handle " + contentType
+ " content as it could not be parsed as JSON or XML");
}
}
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* 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 org.springframework.http.MediaType;

/**
* Public abstraction for external access to field type determination for xml and json
* payloads.
*
* @author Mathias Düsterhöft
* @since 2.0.3
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is new public API so it should be annotated with @since. I think I'd like to sneak this into 2.0.x, so it should be @since 2.0.3.

public interface FieldTypeResolver {

/**
* Create a FieldTypeResolver for the given content and contentType.
* @param content the payload that the {@link FieldTypeResolver} should handle
* @param contentType the content type of the payload
* @return the {@link FieldTypeResolver}
*/
static FieldTypeResolver forContent(byte[] content, MediaType contentType) {
return ContentHandler.forContent(content, contentType);
}

/**
* Returns the type of the field that is described by the given
* {@code fieldDescriptor} based on the content of the payload.
* @param fieldDescriptor the field descriptor
* @return the type of the field
*/
Object resolveFieldType(FieldDescriptor fieldDescriptor);

}
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,9 @@
* A {@link ContentHandler} for JSON content.
*
* @author Andy Wilkinson
* @author Mathias Düsterhöft
*/
class JsonContentHandler implements ContentHandler {
class JsonContentHandler implements ContentHandler, FieldTypeResolver {

private final JsonFieldProcessor fieldProcessor = new JsonFieldProcessor();

Expand Down Expand Up @@ -145,7 +146,7 @@ private boolean isEmpty(Object object) {
}

@Override
public Object determineFieldType(FieldDescriptor fieldDescriptor) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this logic should remain here. It's subtle, but there's a difference between resolving the type of a field (which is what the field type resolver does) and determining the type of a field. It's only the latter that considers the type that's specified on the descriptor and checks whether or not they match. Ideally the former would only use the path (it currently looks at whether the field is optional too). I'd like to keep the current level of separation.

public Object resolveFieldType(FieldDescriptor fieldDescriptor) {
if (fieldDescriptor.getType() == null) {
return this.fieldTypeResolver.resolveFieldType(fieldDescriptor,
readContent());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ private String prettyPrint(Document document) {
}

@Override
public Object determineFieldType(FieldDescriptor fieldDescriptor) {
public Object resolveFieldType(FieldDescriptor fieldDescriptor) {
if (fieldDescriptor.getType() != null) {
return fieldDescriptor.getType();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* 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 org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;

import org.springframework.http.MediaType;

import static org.assertj.core.api.Assertions.assertThat;

/**
* Tests for {@link FieldTypeResolver}.
*
* @author Mathias Düsterhöft
*/
public class FieldTypeResolverTests {

@Rule
public ExpectedException thrownException = ExpectedException.none();

@Test
public void returnJsonFieldTypeResolver() {
assertThat(FieldTypeResolver.forContent("{\"field\": \"value\"}".getBytes(),
MediaType.APPLICATION_JSON)).isInstanceOf(JsonContentHandler.class);
}

@Test
public void returnXmlContentHandler() {
assertThat(FieldTypeResolver.forContent("<a><b>5</b></a>".getBytes(),
MediaType.APPLICATION_XML)).isInstanceOf(XmlContentHandler.class);
}

@Test
public void throwOnInvalidContent() {
this.thrownException.expect(PayloadHandlingException.class);
FieldTypeResolver.forContent("some".getBytes(), MediaType.APPLICATION_XML);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
* Tests for {@link JsonContentHandler}.
*
* @author Andy Wilkinson
* @author Mathias Düsterhöft
*/
public class JsonContentHandlerTests {

Expand All @@ -39,61 +40,61 @@ public class JsonContentHandlerTests {
public void typeForFieldWithNullValueMustMatch() {
this.thrown.expect(FieldTypesDoNotMatchException.class);
new JsonContentHandler("{\"a\": null}".getBytes())
.determineFieldType(new FieldDescriptor("a").type(JsonFieldType.STRING));
.resolveFieldType(new FieldDescriptor("a").type(JsonFieldType.STRING));
}

@Test
public void typeForFieldWithNotNullAndThenNullValueMustMatch() {
this.thrown.expect(FieldTypesDoNotMatchException.class);
new JsonContentHandler("{\"a\":[{\"id\":1},{\"id\":null}]}".getBytes())
.determineFieldType(
.resolveFieldType(
new FieldDescriptor("a[].id").type(JsonFieldType.STRING));
}

@Test
public void typeForFieldWithNullAndThenNotNullValueMustMatch() {
this.thrown.expect(FieldTypesDoNotMatchException.class);
new JsonContentHandler("{\"a\":[{\"id\":null},{\"id\":1}]}".getBytes())
.determineFieldType(
.resolveFieldType(
new FieldDescriptor("a.[].id").type(JsonFieldType.STRING));
}

@Test
public void typeForOptionalFieldWithNumberAndThenNullValueIsNumber() {
Object fieldType = new JsonContentHandler(
"{\"a\":[{\"id\":1},{\"id\":null}]}\"".getBytes())
.determineFieldType(new FieldDescriptor("a[].id").optional());
.resolveFieldType(new FieldDescriptor("a[].id").optional());
assertThat((JsonFieldType) fieldType).isEqualTo(JsonFieldType.NUMBER);
}

@Test
public void typeForOptionalFieldWithNullAndThenNumberIsNumber() {
Object fieldType = new JsonContentHandler(
"{\"a\":[{\"id\":null},{\"id\":1}]}".getBytes())
.determineFieldType(new FieldDescriptor("a[].id").optional());
.resolveFieldType(new FieldDescriptor("a[].id").optional());
assertThat((JsonFieldType) fieldType).isEqualTo(JsonFieldType.NUMBER);
}

@Test
public void typeForFieldWithNumberAndThenNullValueIsVaries() {
Object fieldType = new JsonContentHandler(
"{\"a\":[{\"id\":1},{\"id\":null}]}\"".getBytes())
.determineFieldType(new FieldDescriptor("a[].id"));
.resolveFieldType(new FieldDescriptor("a[].id"));
assertThat((JsonFieldType) fieldType).isEqualTo(JsonFieldType.VARIES);
}

@Test
public void typeForFieldWithNullAndThenNumberIsVaries() {
Object fieldType = new JsonContentHandler(
"{\"a\":[{\"id\":null},{\"id\":1}]}".getBytes())
.determineFieldType(new FieldDescriptor("a[].id"));
.resolveFieldType(new FieldDescriptor("a[].id"));
assertThat((JsonFieldType) fieldType).isEqualTo(JsonFieldType.VARIES);
}

@Test
public void typeForOptionalFieldWithNullValueCanBeProvidedExplicitly() {
Object fieldType = new JsonContentHandler("{\"a\": null}".getBytes())
.determineFieldType(
.resolveFieldType(
new FieldDescriptor("a").type(JsonFieldType.STRING).optional());
assertThat((JsonFieldType) fieldType).isEqualTo(JsonFieldType.STRING);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
* Tests for {@link XmlContentHandler}.
*
* @author Andy Wilkinson
* @author Mathias Düsterhöft
*/
public class XmlContentHandlerTests {

Expand Down