Skip to content

Commit 4854bc9

Browse files
wilkinsonawilkinsona
committed
Fix type resolution for fields with null and non-null values
Closes gh-398
1 parent a123e06 commit 4854bc9

File tree

4 files changed

+122
-24
lines changed

4 files changed

+122
-24
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ private boolean isEmpty(Object object) {
107107
@Override
108108
public Object determineFieldType(FieldDescriptor fieldDescriptor) {
109109
if (fieldDescriptor.getType() == null) {
110-
return this.fieldTypeResolver.resolveFieldType(fieldDescriptor.getPath(),
110+
return this.fieldTypeResolver.resolveFieldType(fieldDescriptor,
111111
readContent());
112112
}
113113
if (!(fieldDescriptor.getType() instanceof JsonFieldType)) {
@@ -116,7 +116,7 @@ public Object determineFieldType(FieldDescriptor fieldDescriptor) {
116116
JsonFieldType descriptorFieldType = (JsonFieldType) fieldDescriptor.getType();
117117
try {
118118
JsonFieldType actualFieldType = this.fieldTypeResolver
119-
.resolveFieldType(fieldDescriptor.getPath(), readContent());
119+
.resolveFieldType(fieldDescriptor, readContent());
120120
if (descriptorFieldType == JsonFieldType.VARIES
121121
|| descriptorFieldType == actualFieldType
122122
|| (fieldDescriptor.isOptional()

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

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ class JsonFieldTypeResolver {
2828

2929
private final JsonFieldProcessor fieldProcessor = new JsonFieldProcessor();
3030

31-
JsonFieldType resolveFieldType(String path, Object payload) {
32-
JsonFieldPath fieldPath = JsonFieldPath.compile(path);
31+
JsonFieldType resolveFieldType(FieldDescriptor fieldDescriptor, Object payload) {
32+
JsonFieldPath fieldPath = JsonFieldPath.compile(fieldDescriptor.getPath());
3333
Object field = this.fieldProcessor.extract(fieldPath, payload);
3434
if (field instanceof Collection && !fieldPath.isPrecise()) {
3535
JsonFieldType commonType = null;
@@ -38,8 +38,16 @@ JsonFieldType resolveFieldType(String path, Object payload) {
3838
if (commonType == null) {
3939
commonType = fieldType;
4040
}
41-
else if (fieldType != commonType && fieldType != JsonFieldType.NULL) {
42-
return JsonFieldType.VARIES;
41+
else if (fieldType != commonType) {
42+
if (!fieldDescriptor.isOptional()) {
43+
return JsonFieldType.VARIES;
44+
}
45+
if (commonType == JsonFieldType.NULL) {
46+
commonType = fieldType;
47+
}
48+
else if (fieldType != JsonFieldType.NULL) {
49+
return JsonFieldType.VARIES;
50+
}
4351
}
4452
}
4553
return commonType;

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

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,57 @@ public void typeForFieldWithNullValueMustMatch() throws IOException {
4444
}
4545

4646
@Test
47-
public void typeForOptionalFieldWithNullValueDoesNotHaveToMatch() throws IOException {
47+
public void typeForFieldWithNotNullAndThenNullValueMustMatch() throws IOException {
48+
this.thrown.expect(FieldTypesDoNotMatchException.class);
49+
new JsonContentHandler("{\"a\":[{\"id\":1},{\"id\":null}]}".getBytes())
50+
.determineFieldType(
51+
new FieldDescriptor("a[].id").type(JsonFieldType.STRING));
52+
}
53+
54+
@Test
55+
public void typeForFieldWithNullAndThenNotNullValueMustMatch() throws IOException {
56+
this.thrown.expect(FieldTypesDoNotMatchException.class);
57+
new JsonContentHandler("{\"a\":[{\"id\":null},{\"id\":1}]}".getBytes())
58+
.determineFieldType(
59+
new FieldDescriptor("a.[].id").type(JsonFieldType.STRING));
60+
}
61+
62+
@Test
63+
public void typeForOptionalFieldWithNumberAndThenNullValueIsNumber()
64+
throws IOException {
65+
Object fieldType = new JsonContentHandler(
66+
"{\"a\":[{\"id\":1},{\"id\":null}]}\"".getBytes())
67+
.determineFieldType(new FieldDescriptor("a[].id").optional());
68+
assertThat((JsonFieldType) fieldType, is(equalTo(JsonFieldType.NUMBER)));
69+
}
70+
71+
@Test
72+
public void typeForOptionalFieldWithNullAndThenNumberIsNumber() throws IOException {
73+
Object fieldType = new JsonContentHandler(
74+
"{\"a\":[{\"id\":null},{\"id\":1}]}".getBytes())
75+
.determineFieldType(new FieldDescriptor("a[].id").optional());
76+
assertThat((JsonFieldType) fieldType, is(equalTo(JsonFieldType.NUMBER)));
77+
}
78+
79+
@Test
80+
public void typeForFieldWithNumberAndThenNullValueIsVaries() throws IOException {
81+
Object fieldType = new JsonContentHandler(
82+
"{\"a\":[{\"id\":1},{\"id\":null}]}\"".getBytes())
83+
.determineFieldType(new FieldDescriptor("a[].id"));
84+
assertThat((JsonFieldType) fieldType, is(equalTo(JsonFieldType.VARIES)));
85+
}
86+
87+
@Test
88+
public void typeForFieldWithNullAndThenNumberIsVaries() throws IOException {
89+
Object fieldType = new JsonContentHandler(
90+
"{\"a\":[{\"id\":null},{\"id\":1}]}".getBytes())
91+
.determineFieldType(new FieldDescriptor("a[].id"));
92+
assertThat((JsonFieldType) fieldType, is(equalTo(JsonFieldType.VARIES)));
93+
}
94+
95+
@Test
96+
public void typeForOptionalFieldWithNullValueCanBeProvidedExplicitly()
97+
throws IOException {
4898
Object fieldType = new JsonContentHandler("{\"a\": null}".getBytes())
4999
.determineFieldType(
50100
new FieldDescriptor("a").type(JsonFieldType.STRING).optional());

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

Lines changed: 57 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2014-2015 the original author or authors.
2+
* Copyright 2014-2017 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -32,7 +32,6 @@
3232
* Tests for {@link JsonFieldTypeResolver}.
3333
*
3434
* @author Andy Wilkinson
35-
*
3635
*/
3736
public class JsonFieldTypeResolverTests {
3837

@@ -49,15 +48,15 @@ public void arrayField() throws IOException {
4948
@Test
5049
public void topLevelArray() throws IOException {
5150
assertThat(
52-
this.fieldTypeResolver.resolveFieldType("[]",
51+
this.fieldTypeResolver.resolveFieldType(new FieldDescriptor("[]"),
5352
new ObjectMapper().readValue("[{\"a\":\"alpha\"}]", List.class)),
5453
equalTo(JsonFieldType.ARRAY));
5554
}
5655

5756
@Test
5857
public void nestedArray() throws IOException {
5958
assertThat(
60-
this.fieldTypeResolver.resolveFieldType("a[]",
59+
this.fieldTypeResolver.resolveFieldType(new FieldDescriptor("a[]"),
6160
createPayload("{\"a\": [{\"b\":\"bravo\"}]}")),
6261
equalTo(JsonFieldType.ARRAY));
6362
}
@@ -90,72 +89,112 @@ public void stringField() throws IOException {
9089
@Test
9190
public void nestedField() throws IOException {
9291
assertThat(
93-
this.fieldTypeResolver.resolveFieldType("a.b.c",
92+
this.fieldTypeResolver.resolveFieldType(new FieldDescriptor("a.b.c"),
9493
createPayload("{\"a\":{\"b\":{\"c\":{}}}}")),
9594
equalTo(JsonFieldType.OBJECT));
9695
}
9796

9897
@Test
9998
public void multipleFieldsWithSameType() throws IOException {
10099
assertThat(
101-
this.fieldTypeResolver.resolveFieldType("a[].id",
100+
this.fieldTypeResolver.resolveFieldType(new FieldDescriptor("a[].id"),
102101
createPayload("{\"a\":[{\"id\":1},{\"id\":2}]}")),
103102
equalTo(JsonFieldType.NUMBER));
104103
}
105104

106105
@Test
107106
public void multipleFieldsWithDifferentTypes() throws IOException {
108107
assertThat(
109-
this.fieldTypeResolver.resolveFieldType("a[].id",
108+
this.fieldTypeResolver.resolveFieldType(new FieldDescriptor("a[].id"),
110109
createPayload("{\"a\":[{\"id\":1},{\"id\":true}]}")),
111110
equalTo(JsonFieldType.VARIES));
112111
}
113112

114113
@Test
115114
public void multipleFieldsWithDifferentTypesAndSometimesAbsent() throws IOException {
116115
assertThat(
117-
this.fieldTypeResolver.resolveFieldType("a[].id",
116+
this.fieldTypeResolver.resolveFieldType(new FieldDescriptor("a[].id"),
117+
createPayload("{\"a\":[{\"id\":1},{\"id\":true}, { }]}")),
118+
equalTo(JsonFieldType.VARIES));
119+
}
120+
121+
@Test
122+
public void multipleFieldsWithDifferentTypesAndSometimesAbsentWhenOptionalResolvesToVaries()
123+
throws IOException {
124+
assertThat(
125+
this.fieldTypeResolver.resolveFieldType(
126+
new FieldDescriptor("a[].id").optional(),
118127
createPayload("{\"a\":[{\"id\":1},{\"id\":true}, { }]}")),
119128
equalTo(JsonFieldType.VARIES));
120129
}
121130

122131
@Test
123132
public void multipleFieldsWhenSometimesAbsent() throws IOException {
124133
assertThat(
125-
this.fieldTypeResolver.resolveFieldType("a[].id",
134+
this.fieldTypeResolver.resolveFieldType(new FieldDescriptor("a[].id"),
126135
createPayload("{\"a\":[{\"id\":1},{ }]}")),
127136
equalTo(JsonFieldType.NUMBER));
128137
}
129138

130139
@Test
131140
public void multipleFieldsWithDifferentTypesAndSometimesNull() throws IOException {
132141
assertThat(
133-
this.fieldTypeResolver.resolveFieldType("a[].id",
142+
this.fieldTypeResolver.resolveFieldType(new FieldDescriptor("a[].id"),
134143
createPayload(
135144
"{\"a\":[{\"id\":1},{\"id\":true}, {\"id\":null}]}")),
136145
equalTo(JsonFieldType.VARIES));
137146
}
138147

139148
@Test
140-
public void multipleFieldsWhenSometimesNull() throws IOException {
149+
public void multipleFieldsWhenNotNullThenNullWhenRequiredHasVariesType()
150+
throws IOException {
151+
assertThat(
152+
this.fieldTypeResolver.resolveFieldType(new FieldDescriptor("a[].id"),
153+
createPayload("{\"a\":[{\"id\":1},{\"id\":null}]}")),
154+
equalTo(JsonFieldType.VARIES));
155+
}
156+
157+
@Test
158+
public void multipleFieldsWhenNotNullThenNullWhenOptionalHasSpecificType()
159+
throws IOException {
141160
assertThat(
142-
this.fieldTypeResolver.resolveFieldType("a[].id",
161+
this.fieldTypeResolver.resolveFieldType(
162+
new FieldDescriptor("a[].id").optional(),
143163
createPayload("{\"a\":[{\"id\":1},{\"id\":null}]}")),
144164
equalTo(JsonFieldType.NUMBER));
145165
}
146166

167+
@Test
168+
public void multipleFieldsWhenNullThenNotNullWhenRequiredHasVariesType()
169+
throws IOException {
170+
assertThat(
171+
this.fieldTypeResolver.resolveFieldType(new FieldDescriptor("a[].id"),
172+
createPayload("{\"a\":[{\"id\":null},{\"id\":1}]}")),
173+
equalTo(JsonFieldType.VARIES));
174+
}
175+
176+
@Test
177+
public void multipleFieldsWhenNullThenNotNullWhenOptionalHasSpecificType()
178+
throws IOException {
179+
assertThat(
180+
this.fieldTypeResolver.resolveFieldType(
181+
new FieldDescriptor("a[].id").optional(),
182+
createPayload("{\"a\":[{\"id\":null},{\"id\":1}]}")),
183+
equalTo(JsonFieldType.NUMBER));
184+
}
185+
147186
@Test
148187
public void multipleFieldsWhenEitherNullOrAbsent() throws IOException {
149188
assertThat(
150-
this.fieldTypeResolver.resolveFieldType("a[].id",
189+
this.fieldTypeResolver.resolveFieldType(new FieldDescriptor("a[].id"),
151190
createPayload("{\"a\":[{},{\"id\":null}]}")),
152191
equalTo(JsonFieldType.NULL));
153192
}
154193

155194
@Test
156195
public void multipleFieldsThatAreAllNull() throws IOException {
157196
assertThat(
158-
this.fieldTypeResolver.resolveFieldType("a[].id",
197+
this.fieldTypeResolver.resolveFieldType(new FieldDescriptor("a[].id"),
159198
createPayload("{\"a\":[{\"id\":null},{\"id\":null}]}")),
160199
equalTo(JsonFieldType.NULL));
161200
}
@@ -166,7 +205,8 @@ public void nonExistentSingleFieldProducesFieldDoesNotExistException()
166205
this.thrownException.expect(FieldDoesNotExistException.class);
167206
this.thrownException.expectMessage(
168207
"The payload does not contain a field with the path 'a.b'");
169-
this.fieldTypeResolver.resolveFieldType("a.b", createPayload("{\"a\":{}}"));
208+
this.fieldTypeResolver.resolveFieldType(new FieldDescriptor("a.b"),
209+
createPayload("{\"a\":{}}"));
170210
}
171211

172212
@Test
@@ -175,13 +215,13 @@ public void nonExistentMultipleFieldsProducesFieldDoesNotExistException()
175215
this.thrownException.expect(FieldDoesNotExistException.class);
176216
this.thrownException.expectMessage(
177217
"The payload does not contain a field with the path 'a[].b'");
178-
this.fieldTypeResolver.resolveFieldType("a[].b",
218+
this.fieldTypeResolver.resolveFieldType(new FieldDescriptor("a[].b"),
179219
createPayload("{\"a\":[{\"c\":1},{\"c\":2}]}"));
180220
}
181221

182222
private void assertFieldType(JsonFieldType expectedType, String jsonValue)
183223
throws IOException {
184-
assertThat(this.fieldTypeResolver.resolveFieldType("field",
224+
assertThat(this.fieldTypeResolver.resolveFieldType(new FieldDescriptor("field"),
185225
createSimplePayload(jsonValue)), equalTo(expectedType));
186226
}
187227

0 commit comments

Comments
 (0)