Fix type resolution for fields with null and non-null values

Closes gh-398

spring-restdocs-core/src/main/java/org/springframework/restdocs/payload/JsonContentHandler.java

@@ -107,7 +107,7 @@ class JsonContentHandler implements ContentHandler {
 	@Override
 	public Object determineFieldType(FieldDescriptor fieldDescriptor) {
 		if (fieldDescriptor.getType() == null) {
-			return this.fieldTypeResolver.resolveFieldType(fieldDescriptor.getPath(),
+			return this.fieldTypeResolver.resolveFieldType(fieldDescriptor,
 					readContent());
 		}
 		if (!(fieldDescriptor.getType() instanceof JsonFieldType)) {
@@ -116,7 +116,7 @@ class JsonContentHandler implements ContentHandler {
 		JsonFieldType descriptorFieldType = (JsonFieldType) fieldDescriptor.getType();
 		try {
 			JsonFieldType actualFieldType = this.fieldTypeResolver
-					.resolveFieldType(fieldDescriptor.getPath(), readContent());
+					.resolveFieldType(fieldDescriptor, readContent());
 			if (descriptorFieldType == JsonFieldType.VARIES
 					|| descriptorFieldType == actualFieldType
 					|| (fieldDescriptor.isOptional()

spring-restdocs-core/src/main/java/org/springframework/restdocs/payload/JsonFieldTypeResolver.java

@@ -28,8 +28,8 @@ class JsonFieldTypeResolver {
 
 	private final JsonFieldProcessor fieldProcessor = new JsonFieldProcessor();
 
-	JsonFieldType resolveFieldType(String path, Object payload) {
-		JsonFieldPath fieldPath = JsonFieldPath.compile(path);
+	JsonFieldType resolveFieldType(FieldDescriptor fieldDescriptor, Object payload) {
+		JsonFieldPath fieldPath = JsonFieldPath.compile(fieldDescriptor.getPath());
 		Object field = this.fieldProcessor.extract(fieldPath, payload);
 		if (field instanceof Collection && !fieldPath.isPrecise()) {
 			JsonFieldType commonType = null;
@@ -38,8 +38,16 @@ class JsonFieldTypeResolver {
 				if (commonType == null) {
 					commonType = fieldType;
 				}
-				else if (fieldType != commonType && fieldType != JsonFieldType.NULL) {
-					return JsonFieldType.VARIES;
+				else if (fieldType != commonType) {
+					if (!fieldDescriptor.isOptional()) {
+						return JsonFieldType.VARIES;
+					}
+					if (commonType == JsonFieldType.NULL) {
+						commonType = fieldType;
+					}
+					else if (fieldType != JsonFieldType.NULL) {
+						return JsonFieldType.VARIES;
+					}
 				}
 			}
 			return commonType;

spring-restdocs-core/src/test/java/org/springframework/restdocs/payload/JsonContentHandlerTests.java

@@ -44,7 +44,57 @@ public class JsonContentHandlerTests {
 	}
 
 	@Test
-	public void typeForOptionalFieldWithNullValueDoesNotHaveToMatch() throws IOException {
+	public void typeForFieldWithNotNullAndThenNullValueMustMatch() throws IOException {
+		this.thrown.expect(FieldTypesDoNotMatchException.class);
+		new JsonContentHandler("{\"a\":[{\"id\":1},{\"id\":null}]}".getBytes())
+				.determineFieldType(
+						new FieldDescriptor("a[].id").type(JsonFieldType.STRING));
+	}
+
+	@Test
+	public void typeForFieldWithNullAndThenNotNullValueMustMatch() throws IOException {
+		this.thrown.expect(FieldTypesDoNotMatchException.class);
+		new JsonContentHandler("{\"a\":[{\"id\":null},{\"id\":1}]}".getBytes())
+				.determineFieldType(
+						new FieldDescriptor("a.[].id").type(JsonFieldType.STRING));
+	}
+
+	@Test
+	public void typeForOptionalFieldWithNumberAndThenNullValueIsNumber()
+			throws IOException {
+		Object fieldType = new JsonContentHandler(
+				"{\"a\":[{\"id\":1},{\"id\":null}]}\"".getBytes())
+						.determineFieldType(new FieldDescriptor("a[].id").optional());
+		assertThat((JsonFieldType) fieldType, is(equalTo(JsonFieldType.NUMBER)));
+	}
+
+	@Test
+	public void typeForOptionalFieldWithNullAndThenNumberIsNumber() throws IOException {
+		Object fieldType = new JsonContentHandler(
+				"{\"a\":[{\"id\":null},{\"id\":1}]}".getBytes())
+						.determineFieldType(new FieldDescriptor("a[].id").optional());
+		assertThat((JsonFieldType) fieldType, is(equalTo(JsonFieldType.NUMBER)));
+	}
+
+	@Test
+	public void typeForFieldWithNumberAndThenNullValueIsVaries() throws IOException {
+		Object fieldType = new JsonContentHandler(
+				"{\"a\":[{\"id\":1},{\"id\":null}]}\"".getBytes())
+						.determineFieldType(new FieldDescriptor("a[].id"));
+		assertThat((JsonFieldType) fieldType, is(equalTo(JsonFieldType.VARIES)));
+	}
+
+	@Test
+	public void typeForFieldWithNullAndThenNumberIsVaries() throws IOException {
+		Object fieldType = new JsonContentHandler(
+				"{\"a\":[{\"id\":null},{\"id\":1}]}".getBytes())
+						.determineFieldType(new FieldDescriptor("a[].id"));
+		assertThat((JsonFieldType) fieldType, is(equalTo(JsonFieldType.VARIES)));
+	}
+
+	@Test
+	public void typeForOptionalFieldWithNullValueCanBeProvidedExplicitly()
+			throws IOException {
 		Object fieldType = new JsonContentHandler("{\"a\": null}".getBytes())
 				.determineFieldType(
 						new FieldDescriptor("a").type(JsonFieldType.STRING).optional());

spring-restdocs-core/src/test/java/org/springframework/restdocs/payload/JsonFieldTypeResolverTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2014-2015 the original author or authors.
+ * Copyright 2014-2017 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.
@@ -32,7 +32,6 @@ import static org.junit.Assert.assertThat;
  * Tests for {@link JsonFieldTypeResolver}.
  *
  * @author Andy Wilkinson
- *
  */
 public class JsonFieldTypeResolverTests {
 
@@ -49,7 +48,7 @@ public class JsonFieldTypeResolverTests {
 	@Test
 	public void topLevelArray() throws IOException {
 		assertThat(
-				this.fieldTypeResolver.resolveFieldType("[]",
+				this.fieldTypeResolver.resolveFieldType(new FieldDescriptor("[]"),
 						new ObjectMapper().readValue("[{\"a\":\"alpha\"}]", List.class)),
 				equalTo(JsonFieldType.ARRAY));
 	}
@@ -57,7 +56,7 @@ public class JsonFieldTypeResolverTests {
 	@Test
 	public void nestedArray() throws IOException {
 		assertThat(
-				this.fieldTypeResolver.resolveFieldType("a[]",
+				this.fieldTypeResolver.resolveFieldType(new FieldDescriptor("a[]"),
 						createPayload("{\"a\": [{\"b\":\"bravo\"}]}")),
 				equalTo(JsonFieldType.ARRAY));
 	}
@@ -90,7 +89,7 @@ public class JsonFieldTypeResolverTests {
 	@Test
 	public void nestedField() throws IOException {
 		assertThat(
-				this.fieldTypeResolver.resolveFieldType("a.b.c",
+				this.fieldTypeResolver.resolveFieldType(new FieldDescriptor("a.b.c"),
 						createPayload("{\"a\":{\"b\":{\"c\":{}}}}")),
 				equalTo(JsonFieldType.OBJECT));
 	}
@@ -98,7 +97,7 @@ public class JsonFieldTypeResolverTests {
 	@Test
 	public void multipleFieldsWithSameType() throws IOException {
 		assertThat(
-				this.fieldTypeResolver.resolveFieldType("a[].id",
+				this.fieldTypeResolver.resolveFieldType(new FieldDescriptor("a[].id"),
 						createPayload("{\"a\":[{\"id\":1},{\"id\":2}]}")),
 				equalTo(JsonFieldType.NUMBER));
 	}
@@ -106,7 +105,7 @@ public class JsonFieldTypeResolverTests {
 	@Test
 	public void multipleFieldsWithDifferentTypes() throws IOException {
 		assertThat(
-				this.fieldTypeResolver.resolveFieldType("a[].id",
+				this.fieldTypeResolver.resolveFieldType(new FieldDescriptor("a[].id"),
 						createPayload("{\"a\":[{\"id\":1},{\"id\":true}]}")),
 				equalTo(JsonFieldType.VARIES));
 	}
@@ -114,7 +113,17 @@ public class JsonFieldTypeResolverTests {
 	@Test
 	public void multipleFieldsWithDifferentTypesAndSometimesAbsent() throws IOException {
 		assertThat(
-				this.fieldTypeResolver.resolveFieldType("a[].id",
+				this.fieldTypeResolver.resolveFieldType(new FieldDescriptor("a[].id"),
+						createPayload("{\"a\":[{\"id\":1},{\"id\":true}, { }]}")),
+				equalTo(JsonFieldType.VARIES));
+	}
+
+	@Test
+	public void multipleFieldsWithDifferentTypesAndSometimesAbsentWhenOptionalResolvesToVaries()
+			throws IOException {
+		assertThat(
+				this.fieldTypeResolver.resolveFieldType(
+						new FieldDescriptor("a[].id").optional(),
 						createPayload("{\"a\":[{\"id\":1},{\"id\":true}, { }]}")),
 				equalTo(JsonFieldType.VARIES));
 	}
@@ -122,7 +131,7 @@ public class JsonFieldTypeResolverTests {
 	@Test
 	public void multipleFieldsWhenSometimesAbsent() throws IOException {
 		assertThat(
-				this.fieldTypeResolver.resolveFieldType("a[].id",
+				this.fieldTypeResolver.resolveFieldType(new FieldDescriptor("a[].id"),
 						createPayload("{\"a\":[{\"id\":1},{ }]}")),
 				equalTo(JsonFieldType.NUMBER));
 	}
@@ -130,24 +139,54 @@ public class JsonFieldTypeResolverTests {
 	@Test
 	public void multipleFieldsWithDifferentTypesAndSometimesNull() throws IOException {
 		assertThat(
-				this.fieldTypeResolver.resolveFieldType("a[].id",
+				this.fieldTypeResolver.resolveFieldType(new FieldDescriptor("a[].id"),
 						createPayload(
 								"{\"a\":[{\"id\":1},{\"id\":true}, {\"id\":null}]}")),
 				equalTo(JsonFieldType.VARIES));
 	}
 
 	@Test
-	public void multipleFieldsWhenSometimesNull() throws IOException {
+	public void multipleFieldsWhenNotNullThenNullWhenRequiredHasVariesType()
+			throws IOException {
 		assertThat(
-				this.fieldTypeResolver.resolveFieldType("a[].id",
+				this.fieldTypeResolver.resolveFieldType(new FieldDescriptor("a[].id"),
 						createPayload("{\"a\":[{\"id\":1},{\"id\":null}]}")),
+				equalTo(JsonFieldType.VARIES));
+	}
+
+	@Test
+	public void multipleFieldsWhenNotNullThenNullWhenOptionalHasSpecificType()
+			throws IOException {
+		assertThat(
+				this.fieldTypeResolver.resolveFieldType(
+						new FieldDescriptor("a[].id").optional(),
+						createPayload("{\"a\":[{\"id\":1},{\"id\":null}]}")),
+				equalTo(JsonFieldType.NUMBER));
+	}
+
+	@Test
+	public void multipleFieldsWhenNullThenNotNullWhenRequiredHasVariesType()
+			throws IOException {
+		assertThat(
+				this.fieldTypeResolver.resolveFieldType(new FieldDescriptor("a[].id"),
+						createPayload("{\"a\":[{\"id\":null},{\"id\":1}]}")),
+				equalTo(JsonFieldType.VARIES));
+	}
+
+	@Test
+	public void multipleFieldsWhenNullThenNotNullWhenOptionalHasSpecificType()
+			throws IOException {
+		assertThat(
+				this.fieldTypeResolver.resolveFieldType(
+						new FieldDescriptor("a[].id").optional(),
+						createPayload("{\"a\":[{\"id\":null},{\"id\":1}]}")),
 				equalTo(JsonFieldType.NUMBER));
 	}
 
 	@Test
 	public void multipleFieldsWhenEitherNullOrAbsent() throws IOException {
 		assertThat(
-				this.fieldTypeResolver.resolveFieldType("a[].id",
+				this.fieldTypeResolver.resolveFieldType(new FieldDescriptor("a[].id"),
 						createPayload("{\"a\":[{},{\"id\":null}]}")),
 				equalTo(JsonFieldType.NULL));
 	}
@@ -155,7 +194,7 @@ public class JsonFieldTypeResolverTests {
 	@Test
 	public void multipleFieldsThatAreAllNull() throws IOException {
 		assertThat(
-				this.fieldTypeResolver.resolveFieldType("a[].id",
+				this.fieldTypeResolver.resolveFieldType(new FieldDescriptor("a[].id"),
 						createPayload("{\"a\":[{\"id\":null},{\"id\":null}]}")),
 				equalTo(JsonFieldType.NULL));
 	}
@@ -166,7 +205,8 @@ public class JsonFieldTypeResolverTests {
 		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\":{}}"));
+		this.fieldTypeResolver.resolveFieldType(new FieldDescriptor("a.b"),
+				createPayload("{\"a\":{}}"));
 	}
 
 	@Test
@@ -175,13 +215,13 @@ public class JsonFieldTypeResolverTests {
 		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",
+		this.fieldTypeResolver.resolveFieldType(new FieldDescriptor("a[].b"),
 				createPayload("{\"a\":[{\"c\":1},{\"c\":2}]}"));
 	}
 
 	private void assertFieldType(JsonFieldType expectedType, String jsonValue)
 			throws IOException {
-		assertThat(this.fieldTypeResolver.resolveFieldType("field",
+		assertThat(this.fieldTypeResolver.resolveFieldType(new FieldDescriptor("field"),
 				createSimplePayload(jsonValue)), equalTo(expectedType));
 	}