From 88e26ba88b88e8e064772ec14a6b7f8778185d81 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Mon, 13 Jul 2015 11:44:06 +0100 Subject: [PATCH] Improve diagnostics when no type is provided for absent optional field Previously, if an optional field was being documented and that field was not present in the payload a failure would occur with the message "The payload does not contain a field with the path 'the.field.path'". This isn't very helpful as it doesn't explain why the field was being looked for (to resolve its type). This commit improves the diagnostics to improve the message to explain that a field's type could not be determined as it didn't exist in the payload and to suggest the use of FieldDescriptor.type(FieldType) to provide a type. Closes gh-83 --- .../payload/FieldDoesNotExistException.java | 37 +++++++++++++++++++ .../restdocs/payload/FieldProcessor.java | 3 +- .../payload/FieldSnippetResultHandler.java | 23 ++++++++++-- .../payload/FieldTypeRequiredException.java | 37 +++++++++++++++++++ .../restdocs/payload/FieldProcessorTests.java | 14 +++---- .../payload/FieldTypeResolverTests.java | 2 +- .../payload/PayloadDocumentationTests.java | 8 ++++ 7 files changed, 111 insertions(+), 13 deletions(-) create mode 100644 spring-restdocs/src/main/java/org/springframework/restdocs/payload/FieldDoesNotExistException.java create mode 100644 spring-restdocs/src/main/java/org/springframework/restdocs/payload/FieldTypeRequiredException.java diff --git a/spring-restdocs/src/main/java/org/springframework/restdocs/payload/FieldDoesNotExistException.java b/spring-restdocs/src/main/java/org/springframework/restdocs/payload/FieldDoesNotExistException.java new file mode 100644 index 00000000..99f565e2 --- /dev/null +++ b/spring-restdocs/src/main/java/org/springframework/restdocs/payload/FieldDoesNotExistException.java @@ -0,0 +1,37 @@ +/* + * Copyright 2014-2015 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; + +/** + * A {@code FieldDoesNotExistException} is thrown when a requested field does not exist in + * a payload. + * + * @author Andy Wilkinson + */ +@SuppressWarnings("serial") +public class FieldDoesNotExistException extends RuntimeException { + + /** + * Creates a new {@code FieldDoesNotExistException} that indicates that the field with + * the given {@code fieldPath} does not exist. + * + * @param fieldPath the path of the field that does not exist + */ + public FieldDoesNotExistException(FieldPath fieldPath) { + super("The payload does not contain a field with the path '" + fieldPath + "'"); + } +} 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 bfa7f20a..f0e3d25d 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 @@ -55,8 +55,7 @@ final class FieldProcessor { }); if (matches.isEmpty()) { - throw new IllegalArgumentException( - "The payload does not contain a field with the path '" + path + "'"); + throw new FieldDoesNotExistException(path); } if (path.isPrecise()) { return matches.get(0); 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 5ad94157..84627c42 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 @@ -78,15 +78,32 @@ public abstract class FieldSnippetResultHandler extends SnippetWritingResultHand for (Entry entry : FieldSnippetResultHandler.this.descriptorsByPath .entrySet()) { FieldDescriptor descriptor = entry.getValue(); - FieldType type = descriptor.getType() != null ? descriptor.getType() - : FieldSnippetResultHandler.this.fieldTypeResolver - .resolveFieldType(descriptor.getPath(), payload); + FieldType type = getFieldType(descriptor, payload); tableWriter.row(entry.getKey().toString(), type.toString(), entry .getValue().getDescription()); } } + private FieldType getFieldType(FieldDescriptor descriptor, Object payload) { + if (descriptor.getType() != null) { + return descriptor.getType(); + } + else { + try { + return FieldSnippetResultHandler.this.fieldTypeResolver + .resolveFieldType(descriptor.getPath(), payload); + } + catch (FieldDoesNotExistException ex) { + String message = "Cannot determine the type of the field '" + + descriptor.getPath() + "' as it is not present in the" + + " payload. Please provide a type using" + + " FieldDescriptor.type(FieldType)."; + throw new FieldTypeRequiredException(message); + } + } + } + }); } diff --git a/spring-restdocs/src/main/java/org/springframework/restdocs/payload/FieldTypeRequiredException.java b/spring-restdocs/src/main/java/org/springframework/restdocs/payload/FieldTypeRequiredException.java new file mode 100644 index 00000000..c9fa4121 --- /dev/null +++ b/spring-restdocs/src/main/java/org/springframework/restdocs/payload/FieldTypeRequiredException.java @@ -0,0 +1,37 @@ +/* + * Copyright 2014-2015 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; + +/** + * A {@code FieldTypeRequiredException} is thrown when a field's type cannot be determined + * automatically and, therefore, must be explicitly provided. + * + * @author Andy Wilkinson + */ +@SuppressWarnings("serial") +public class FieldTypeRequiredException extends RuntimeException { + + /** + * Creates a new {@code FieldTypeRequiredException} indicating that a type is required + * for the reason described in the given {@code message}. + * + * @param message the message + */ + public FieldTypeRequiredException(String message) { + super(message); + } +} diff --git a/spring-restdocs/src/test/java/org/springframework/restdocs/payload/FieldProcessorTests.java b/spring-restdocs/src/test/java/org/springframework/restdocs/payload/FieldProcessorTests.java index 949ae10f..da71af5e 100644 --- a/spring-restdocs/src/test/java/org/springframework/restdocs/payload/FieldProcessorTests.java +++ b/spring-restdocs/src/test/java/org/springframework/restdocs/payload/FieldProcessorTests.java @@ -129,27 +129,27 @@ public class FieldProcessorTests { Arrays.asList(4)))); } - @Test(expected = IllegalArgumentException.class) + @Test(expected = FieldDoesNotExistException.class) public void nonExistentTopLevelField() { this.fieldProcessor .extract(FieldPath.compile("a"), new HashMap()); } - @Test(expected = IllegalArgumentException.class) + @Test(expected = FieldDoesNotExistException.class) public void nonExistentNestedField() { HashMap payload = new HashMap(); payload.put("a", new HashMap()); this.fieldProcessor.extract(FieldPath.compile("a.b"), payload); } - @Test(expected = IllegalArgumentException.class) + @Test(expected = FieldDoesNotExistException.class) public void nonExistentNestedFieldWhenParentIsNotAMap() { HashMap payload = new HashMap(); payload.put("a", 5); this.fieldProcessor.extract(FieldPath.compile("a.b"), payload); } - @Test(expected = IllegalArgumentException.class) + @Test(expected = FieldDoesNotExistException.class) public void nonExistentFieldWhenParentIsAnArray() { HashMap payload = new HashMap(); HashMap alpha = new HashMap(); @@ -158,20 +158,20 @@ public class FieldProcessorTests { this.fieldProcessor.extract(FieldPath.compile("a.b.c"), payload); } - @Test(expected = IllegalArgumentException.class) + @Test(expected = FieldDoesNotExistException.class) public void nonExistentArrayField() { HashMap payload = new HashMap(); this.fieldProcessor.extract(FieldPath.compile("a[]"), payload); } - @Test(expected = IllegalArgumentException.class) + @Test(expected = FieldDoesNotExistException.class) public void nonExistentArrayFieldAsTypeDoesNotMatch() { HashMap payload = new HashMap(); payload.put("a", 5); this.fieldProcessor.extract(FieldPath.compile("a[]"), payload); } - @Test(expected = IllegalArgumentException.class) + @Test(expected = FieldDoesNotExistException.class) public void nonExistentFieldBeneathAnArray() { HashMap payload = new HashMap(); HashMap alpha = new HashMap(); diff --git a/spring-restdocs/src/test/java/org/springframework/restdocs/payload/FieldTypeResolverTests.java b/spring-restdocs/src/test/java/org/springframework/restdocs/payload/FieldTypeResolverTests.java index 42d5fc9f..4376bea5 100644 --- a/spring-restdocs/src/test/java/org/springframework/restdocs/payload/FieldTypeResolverTests.java +++ b/spring-restdocs/src/test/java/org/springframework/restdocs/payload/FieldTypeResolverTests.java @@ -93,7 +93,7 @@ public class FieldTypeResolverTests { @Test public void nonExistentFieldProducesIllegalArgumentException() throws IOException { - this.thrownException.expect(IllegalArgumentException.class); + this.thrownException.expect(FieldDoesNotExistException.class); this.thrownException .expectMessage("The payload does not contain a field with the path 'a.b'"); this.fieldTypeResolver.resolveFieldType("a.b", createPayload("{\"a\":{}}")); 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 e8750ccf..7e99a145 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 @@ -141,6 +141,14 @@ public class PayloadDocumentationTests { result(get("/foo").content("{}"))); } + @Test + public void missingOptionalRequestFieldWithNoTypeProvided() throws IOException { + this.thrown.expect(FieldTypeRequiredException.class); + documentRequestFields("missing-optional-request-field-with-no-type", + fieldWithPath("a.b").description("one").optional()).handle( + result(get("/foo").content("{ }"))); + } + @Test public void undocumentedRequestFieldAndMissingRequestField() throws IOException { this.thrown.expect(SnippetGenerationException.class);