Rework JsonFieldTypeResolver to only discover the field types

See spring-projectsgh-549

Author: wilkinsona

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

@@ -37,7 +37,7 @@ class JsonContentHandler implements ContentHandler {
 
 	private final JsonFieldProcessor fieldProcessor = new JsonFieldProcessor();
 
-	private final JsonFieldTypeResolver fieldTypeResolver = new JsonFieldTypeResolver();
+	private final JsonFieldTypesDiscoverer fieldTypesDiscoverer = new JsonFieldTypesDiscoverer();
 
 	private final ObjectMapper objectMapper = new ObjectMapper()
 			.enable(SerializationFeature.INDENT_OUTPUT);
@@ -147,16 +147,18 @@ private boolean isEmpty(Object object) {
 	@Override
 	public Object determineFieldType(FieldDescriptor fieldDescriptor) {
 		if (fieldDescriptor.getType() == null) {
-			return this.fieldTypeResolver.resolveFieldType(fieldDescriptor,
-					readContent());
+			return this.fieldTypesDiscoverer
+					.discoverFieldTypes(fieldDescriptor.getPath(), readContent())
+					.coalesce(fieldDescriptor.isOptional());
 		}
 		if (!(fieldDescriptor.getType() instanceof JsonFieldType)) {
 			return fieldDescriptor.getType();
 		}
 		JsonFieldType descriptorFieldType = (JsonFieldType) fieldDescriptor.getType();
 		try {
-			JsonFieldType actualFieldType = this.fieldTypeResolver
-					.resolveFieldType(fieldDescriptor, readContent());
+			JsonFieldType actualFieldType = this.fieldTypesDiscoverer
+					.discoverFieldTypes(fieldDescriptor.getPath(), readContent())
+					.coalesce(fieldDescriptor.isOptional());
 			if (descriptorFieldType == JsonFieldType.VARIES
 					|| descriptorFieldType == actualFieldType
 					|| (fieldDescriptor.isOptional()

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

@@ -41,20 +41,25 @@ boolean hasField(String path, Object payload) {
 
 	ExtractedField extract(String path, Object payload) {
 		JsonFieldPath compiledPath = JsonFieldPath.compile(path);
-		final List<Object> matches = new ArrayList<>();
+		final List<Object> values = new ArrayList<>();
 		traverse(new ProcessingContext(payload, compiledPath), new MatchCallback() {
 
 			@Override
 			public void foundMatch(Match match) {
-				matches.add(match.getValue());
+				values.add(match.getValue());
+			}
+
+			@Override
+			public void absent() {
+				values.add(ExtractedField.ABSENT);
 			}
 
 		});
-		if (matches.isEmpty()) {
-			throw new FieldDoesNotExistException(path);
+		if (values.isEmpty()) {
+			values.add(ExtractedField.ABSENT);
 		}
 		return new ExtractedField(
-				(compiledPath.getType() != PathType.SINGLE) ? matches : matches.get(0),
+				(compiledPath.getType() != PathType.SINGLE) ? values : values.get(0),
 				compiledPath.getType());
 	}
 
@@ -427,6 +432,8 @@ private ProcessingContext descend(Object payload, Match match) {
 	 */
 	static class ExtractedField {
 
+		static final Object ABSENT = new Object();
+
 		private final Object value;
 
 		private final PathType type;

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

@@ -0,0 +1,57 @@
+/*
+ * 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.Collections;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Set;
+
+/**
+ * {@link JsonFieldType Types} for a field discovered in a JSON payload.
+ *
+ * @author Andy Wilkinson
+ */
+class JsonFieldTypes implements Iterable<JsonFieldType> {
+
+	private final Set<JsonFieldType> fieldTypes;
+
+	JsonFieldTypes(JsonFieldType fieldType) {
+		this(Collections.singleton(fieldType));
+	}
+
+	JsonFieldTypes(Set<JsonFieldType> fieldTypes) {
+		this.fieldTypes = fieldTypes;
+	}
+
+	JsonFieldType coalesce(boolean optional) {
+		Set<JsonFieldType> types = new HashSet<>(this.fieldTypes);
+		if (optional && types.size() > 1) {
+			types.remove(JsonFieldType.NULL);
+		}
+		if (types.size() == 1) {
+			return types.iterator().next();
+		}
+		return JsonFieldType.VARIES;
+	}
+
+	@Override
+	public Iterator<JsonFieldType> iterator() {
+		return this.fieldTypes.iterator();
+	}
+
+}

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2014-2017 the original author or authors.
+ * 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.
@@ -17,50 +17,45 @@
 package org.springframework.restdocs.payload;
 
 import java.util.Collection;
+import java.util.HashSet;
 import java.util.Map;
+import java.util.Set;
 
 import org.springframework.restdocs.payload.JsonFieldPath.PathType;
 import org.springframework.restdocs.payload.JsonFieldProcessor.ExtractedField;
 
 /**
- * Resolves the type of a field in a JSON request or response payload.
+ * Discovers the types of the fields found at a path in a JSON request or response
+ * payload.
  *
  * @author Andy Wilkinson
  */
-class JsonFieldTypeResolver {
+class JsonFieldTypesDiscoverer {
 
 	private final JsonFieldProcessor fieldProcessor = new JsonFieldProcessor();
 
-	JsonFieldType resolveFieldType(FieldDescriptor fieldDescriptor, Object payload) {
-		ExtractedField extractedField = this.fieldProcessor
-				.extract(fieldDescriptor.getPath(), payload);
+	JsonFieldTypes discoverFieldTypes(String path, Object payload) {
+		ExtractedField extractedField = this.fieldProcessor.extract(path, payload);
 		Object value = extractedField.getValue();
 		if (value instanceof Collection && extractedField.getType() == PathType.MULTI) {
-			JsonFieldType commonType = null;
-			for (Object item : (Collection<?>) value) {
-				JsonFieldType fieldType = determineFieldType(item);
-				if (commonType == null) {
-					commonType = fieldType;
-				}
-				else if (fieldType != commonType) {
-					if (!fieldDescriptor.isOptional()) {
-						return JsonFieldType.VARIES;
-					}
-					if (commonType == JsonFieldType.NULL) {
-						commonType = fieldType;
-					}
-					else if (fieldType != JsonFieldType.NULL) {
-						return JsonFieldType.VARIES;
-					}
-				}
+			Collection<?> values = (Collection<?>) value;
+			if (allAbsent(values)) {
+				throw new FieldDoesNotExistException(path);
 			}
-			return commonType;
+			Set<JsonFieldType> fieldTypes = new HashSet<>();
+			for (Object item : values) {
+				fieldTypes.add(determineFieldType(item));
+			}
+			return new JsonFieldTypes(fieldTypes);
+		}
+		if (value == ExtractedField.ABSENT) {
+			throw new FieldDoesNotExistException(path);
 		}
-		return determineFieldType(value);
+		return new JsonFieldTypes(determineFieldType(value));
 	}
 
 	private JsonFieldType determineFieldType(Object fieldValue) {
-		if (fieldValue == null) {
+		if (fieldValue == null || fieldValue == ExtractedField.ABSENT) {
 			return JsonFieldType.NULL;
 		}
 		if (fieldValue instanceof String) {
@@ -78,4 +73,13 @@ private JsonFieldType determineFieldType(Object fieldValue) {
 		return JsonFieldType.NUMBER;
 	}
 
+	private boolean allAbsent(Collection<?> values) {
+		for (Object value : values) {
+			if (value != ExtractedField.ABSENT) {
+				return false;
+			}
+		}
+		return true;
+	}
+
 }

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

@@ -28,6 +28,8 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import org.junit.Test;
 
+import org.springframework.restdocs.payload.JsonFieldProcessor.ExtractedField;
+
 import static org.assertj.core.api.Assertions.assertThat;
 
 /**
@@ -109,7 +111,7 @@ public void extractOccasionallyAbsentFieldFromItemsInArray() {
 				new HashMap<String, Object>());
 		payload.put("a", alpha);
 		assertThat(this.fieldProcessor.extract("a[].b", payload).getValue())
-				.isEqualTo(Arrays.asList("bravo"));
+				.isEqualTo(Arrays.asList("bravo", ExtractedField.ABSENT));
 	}
 
 	@Test
@@ -165,54 +167,61 @@ public void extractArraysFromItemsInNestedArray() {
 						Arrays.asList(4)));
 	}
 
-	@Test(expected = FieldDoesNotExistException.class)
+	@Test
 	public void nonExistentTopLevelField() {
-		this.fieldProcessor.extract("a", Collections.emptyMap());
+		assertThat(this.fieldProcessor.extract("a", Collections.emptyMap()).getValue())
+				.isEqualTo(ExtractedField.ABSENT);
 	}
 
-	@Test(expected = FieldDoesNotExistException.class)
+	@Test
 	public void nonExistentNestedField() {
 		HashMap<String, Object> payload = new HashMap<>();
 		payload.put("a", new HashMap<String, Object>());
-		this.fieldProcessor.extract("a.b", payload);
+		assertThat(this.fieldProcessor.extract("a.b", payload).getValue())
+				.isEqualTo(ExtractedField.ABSENT);
 	}
 
-	@Test(expected = FieldDoesNotExistException.class)
+	@Test
 	public void nonExistentNestedFieldWhenParentIsNotAMap() {
 		HashMap<String, Object> payload = new HashMap<>();
 		payload.put("a", 5);
-		this.fieldProcessor.extract("a.b", payload);
+		assertThat(this.fieldProcessor.extract("a.b", payload).getValue())
+				.isEqualTo(ExtractedField.ABSENT);
 	}
 
-	@Test(expected = FieldDoesNotExistException.class)
+	@Test
 	public void nonExistentFieldWhenParentIsAnArray() {
 		HashMap<String, Object> payload = new HashMap<>();
 		HashMap<String, Object> alpha = new HashMap<>();
 		alpha.put("b", Arrays.asList(new HashMap<String, Object>()));
 		payload.put("a", alpha);
-		this.fieldProcessor.extract("a.b.c", payload);
+		assertThat(this.fieldProcessor.extract("a.b.c", payload).getValue())
+				.isEqualTo(ExtractedField.ABSENT);
 	}
 
-	@Test(expected = FieldDoesNotExistException.class)
+	@Test
 	public void nonExistentArrayField() {
 		HashMap<String, Object> payload = new HashMap<>();
-		this.fieldProcessor.extract("a[]", payload);
+		assertThat(this.fieldProcessor.extract("a[]", payload).getValue())
+				.isEqualTo(ExtractedField.ABSENT);
 	}
 
-	@Test(expected = FieldDoesNotExistException.class)
+	@Test
 	public void nonExistentArrayFieldAsTypeDoesNotMatch() {
 		HashMap<String, Object> payload = new HashMap<>();
 		payload.put("a", 5);
-		this.fieldProcessor.extract("a[]", payload);
+		assertThat(this.fieldProcessor.extract("a[]", payload).getValue())
+				.isEqualTo(ExtractedField.ABSENT);
 	}
 
-	@Test(expected = FieldDoesNotExistException.class)
+	@Test
 	public void nonExistentFieldBeneathAnArray() {
 		HashMap<String, Object> payload = new HashMap<>();
 		HashMap<String, Object> alpha = new HashMap<>();
 		alpha.put("b", Arrays.asList(new HashMap<String, Object>()));
 		payload.put("a", alpha);
-		this.fieldProcessor.extract("a.b[].id", payload);
+		assertThat(this.fieldProcessor.extract("a.b[].id", payload).getValue())
+				.isEqualTo(Arrays.asList(ExtractedField.ABSENT));
 	}
 
 	@Test