Skip to content

Commit d8a9aea

Browse files
dnaultdnault
committed
JCBC-1574 JsonObject/Array.put(String,Object) fails on Map or List
Backported from SDK 3 commit fb2d3845d53ce202b91ad1f886fb7c701ea1ff4a Motivation ---------- There already exist specialized methods for putting Map and List values, but they are not invoked if the exact type of the value is not known at compile time. Support passing a Map or List to JsonObject/Array.put(String,Object). This is important since these methods are used by Spring Data Couchbase. Modifications ------------- Add a JsonValue.coerce(Object) method that converts the given object to a type that can be put in a JsonArray or JsonObject. Use this method everywhere a value of unknown type is added. For consistency, convert any ClassCastExceptions to InvalidArgumentExceptions. Change-Id: Iea39648a94743c9a05020ed7e5a78ee37ae2b589 Reviewed-on: http://review.couchbase.org/120924 Tested-by: David Nault <david.nault@couchbase.com> Reviewed-by: Michael Nitschinger <michael.nitschinger@couchbase.com>
1 parent 5fabec4 commit d8a9aea

File tree

5 files changed

+108
-187
lines changed

5 files changed

+108
-187
lines changed

‎src/main/java/com/couchbase/client/java/document/json/JsonArray.java‎

Lines changed: 4 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import java.util.ArrayList;
2525
import java.util.Iterator;
2626
import java.util.List;
27-
import java.util.ListIterator;
2827
import java.util.Map;
2928

3029
/**
@@ -36,8 +35,6 @@
3635
* The {@link JsonArray} is backed by a {@link List} and is intended to work similar to it API wise, but to only
3736
* allow to store such objects which can be represented by JSON.
3837
*
39-
* @author Michael Nitschinger
40-
* @author Simon Baslé
4138
* @since 2.0
4239
*/
4340
public class JsonArray extends JsonValue implements Iterable<Object>, Serializable {
@@ -91,11 +88,7 @@ public static JsonArray create() {
9188
public static JsonArray from(Object... items) {
9289
JsonArray array = new JsonArray(items.length);
9390
for (Object item : items) {
94-
if (checkType(item)) {
95-
array.add(item);
96-
} else {
97-
throw new IllegalArgumentException("Unsupported type for JsonArray: " + item.getClass());
98-
}
91+
array.add(coerce(item));
9992
}
10093
return array;
10194
}
@@ -120,47 +113,11 @@ public static JsonArray from(Object... items) {
120113
public static JsonArray from(List<?> items) {
121114
if (items == null) {
122115
throw new NullPointerException("Null list unsupported");
123-
} else if (items.isEmpty()) {
124-
return JsonArray.empty();
125116
}
126117

127118
JsonArray array = new JsonArray(items.size());
128-
ListIterator<?> iter = items.listIterator();
129-
while (iter.hasNext()) {
130-
int i = iter.nextIndex();
131-
Object item = iter.next();
132-
133-
if (item == JsonValue.NULL) {
134-
item = null;
135-
}
136-
137-
if (item instanceof Map) {
138-
try {
139-
JsonObject sub = JsonObject.from((Map<String, ?>) item);
140-
array.add(sub);
141-
} catch (ClassCastException e) {
142-
throw e;
143-
} catch (Exception e) {
144-
ClassCastException c = new ClassCastException("Couldn't convert sub-Map at " + i + " to JsonObject");
145-
c.initCause(e);
146-
throw c;
147-
}
148-
} else if (item instanceof List) {
149-
try {
150-
JsonArray sub = JsonArray.from((List<?>) item);
151-
array.add(sub);
152-
} catch (Exception e) {
153-
//no risk of a direct ClassCastException here
154-
ClassCastException c = new ClassCastException(
155-
"Couldn't convert sub-List at " + i + " to JsonArray");
156-
c.initCause(e);
157-
throw c;
158-
}
159-
} else if (checkType(item)) {
160-
array.add(item);
161-
} else {
162-
throw new IllegalArgumentException("Unsupported type for JsonArray: " + item.getClass());
163-
}
119+
for (Object item : items) {
120+
array.add(coerce(item));
164121
}
165122
return array;
166123
}
@@ -206,13 +163,8 @@ public Object get(int index) {
206163
public JsonArray add(Object value) {
207164
if (value == this) {
208165
throw new IllegalArgumentException("Cannot add self");
209-
} else if (value == JsonValue.NULL) {
210-
addNull();
211-
} else if (checkType(value)) {
212-
content.add(value);
213-
} else {
214-
throw new IllegalArgumentException("Unsupported type for JsonArray: " + value.getClass());
215166
}
167+
content.add(coerce(value));
216168
return this;
217169
}
218170

‎src/main/java/com/couchbase/client/java/document/json/JsonObject.java‎

Lines changed: 14 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@
3333
import java.util.Map;
3434
import java.util.Set;
3535

36+
import static java.util.Objects.requireNonNull;
37+
3638
/**
3739
* Represents a JSON object that can be stored and loaded from Couchbase Server.
3840
*
@@ -42,8 +44,6 @@
4244
* The {@link JsonObject} is backed by a {@link Map} and is intended to work similar to it API wise, but to only
4345
* allow to store such objects which can be represented by JSON.
4446
*
45-
* @author Michael Nitschinger
46-
* @author Simon Baslé
4747
* @since 2.0
4848
*/
4949
public class JsonObject extends JsonValue implements Serializable {
@@ -127,50 +127,19 @@ public static JsonObject create() {
127127
* @throws ClassCastException if map contains a sub-Map or sub-List not supported (see above)
128128
*/
129129
public static JsonObject from(Map<String, ?> mapData) {
130-
if (mapData == null) {
131-
throw new NullPointerException("Null input Map unsupported");
132-
} else if (mapData.isEmpty()) {
133-
return JsonObject.empty();
134-
}
130+
requireNonNull(mapData, "Null input Map unsupported");
135131

136132
JsonObject result = new JsonObject(mapData.size());
137-
for (Map.Entry<String, ?> entry : mapData.entrySet()) {
138-
String key = entry.getKey();
139-
Object value = entry.getValue();
140-
141-
if (value == JsonValue.NULL) {
142-
value = null;
143-
}
144-
145-
if (key == null) {
146-
throw new NullPointerException("The key is not allowed to be null");
147-
} else if (value instanceof Map) {
148-
try {
149-
JsonObject sub = JsonObject.from((Map<String, ?>) value);
150-
result.put(key, sub);
151-
} catch (ClassCastException e) {
152-
throw e;
153-
} catch (Exception e) {
154-
ClassCastException c = new ClassCastException("Couldn't convert sub-Map " + key + " to JsonObject");
155-
c.initCause(e);
156-
throw c;
157-
}
158-
} else if (value instanceof List) {
159-
try {
160-
JsonArray sub = JsonArray.from((List<?>) value);
161-
result.put(key, sub);
162-
} catch (Exception e) {
163-
//no risk of a direct ClassCastException here
164-
ClassCastException c = new ClassCastException("Couldn't convert sub-List " + key + " to JsonArray");
165-
c.initCause(e);
166-
throw c;
167-
}
168-
} else if (!checkType(value)) {
169-
throw new IllegalArgumentException("Unsupported type for JsonObject: " + value.getClass());
170-
} else {
171-
result.put(key, value);
133+
try {
134+
for (Map.Entry<String, ?> entry : mapData.entrySet()) {
135+
String key = requireNonNull(entry.getKey(), "The key is not allowed to be null");
136+
Object value = entry.getValue();
137+
result.put(key, coerce(value));
172138
}
139+
} catch (ClassCastException e) {
140+
throw new IllegalArgumentException("Map key must be String", e);
173141
}
142+
174143
return result;
175144
}
176145

@@ -203,13 +172,8 @@ public static JsonObject fromJson(String s) {
203172
public JsonObject put(final String name, final Object value) {
204173
if (this == value) {
205174
throw new IllegalArgumentException("Cannot put self");
206-
} else if (value == JsonValue.NULL) {
207-
putNull(name);
208-
} else if (checkType(value)) {
209-
content.put(name, value);
210-
} else {
211-
throw new IllegalArgumentException("Unsupported type for JsonObject: " + value.getClass());
212175
}
176+
content.put(name, coerce(value));
213177
return this;
214178
}
215179

@@ -230,15 +194,7 @@ public JsonObject put(final String name, final Object value) {
230194
*/
231195
public JsonObject putAndEncrypt(final String name, final Object value, String providerName) {
232196
addValueEncryptionInfo(name, providerName, true);
233-
if (this == value) {
234-
throw new IllegalArgumentException("Cannot put self");
235-
} else if (value == JsonValue.NULL) {
236-
putNull(name);
237-
} else if (checkType(value)) {
238-
content.put(name, value);
239-
} else {
240-
throw new IllegalArgumentException("Unsupported type for JsonObject: " + value.getClass());
241-
}
197+
put(name, value);
242198
return this;
243199
}
244200

@@ -1243,4 +1199,4 @@ public boolean equals(Object o) {
12431199
public int hashCode() {
12441200
return content.hashCode();
12451201
}
1246-
}
1202+
}

‎src/main/java/com/couchbase/client/java/document/json/JsonValue.java‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,19 +17,20 @@
1717

1818
import java.math.BigDecimal;
1919
import java.math.BigInteger;
20+
import java.util.List;
21+
import java.util.Map;
2022

2123
/**
2224
* Represents a JSON value (either a {@link JsonObject} or a {@link JsonArray}.
2325
*
24-
* @author Michael Nitschinger
2526
* @since 2.0
2627
*/
2728
public abstract class JsonValue {
2829

2930
/**
3031
* Represents a Json "null".
3132
*/
32-
public static JsonNull NULL = JsonNull.INSTANCE;
33+
public static final JsonNull NULL = JsonNull.INSTANCE;
3334

3435
/**
3536
* Static factory method to create an empty {@link JsonObject}.
@@ -68,4 +69,24 @@ public static boolean checkType(Object item) {
6869
|| item instanceof JsonArray;
6970
}
7071

72+
/**
73+
* Returns the given value converted to a type that passes the {@link #checkType} test.
74+
* @throws IllegalArgumentException if conversion is not possible.
75+
*/
76+
@SuppressWarnings("unchecked")
77+
static Object coerce(Object value) {
78+
if (checkType(value)) {
79+
return value;
80+
}
81+
if (value instanceof Map) {
82+
return JsonObject.from((Map) value);
83+
}
84+
if (value instanceof List) {
85+
return JsonArray.from((List) value);
86+
}
87+
if (value instanceof JsonNull) {
88+
return null;
89+
}
90+
throw new IllegalArgumentException("Unsupported type for JSON value: " + value.getClass());
91+
}
7192
}

‎src/test/java/com/couchbase/client/java/document/json/JsonArrayTest.java‎

Lines changed: 37 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
import java.util.List;
2929
import java.util.Map;
3030

31+
import static java.util.Collections.singletonList;
32+
import static java.util.Collections.singletonMap;
3133
import static org.junit.Assert.assertEquals;
3234
import static org.junit.Assert.assertNotEquals;
3335
import static org.junit.Assert.assertNotNull;
@@ -36,13 +38,6 @@
3638
import static org.junit.Assert.fail;
3739
import static org.junit.Assume.assumeFalse;
3840

39-
/**
40-
* Verifies the functionality provided by a {@link JsonArray}.
41-
*
42-
* @author Michael Nitschinger
43-
* @author Simon Baslé
44-
* @since 2.0
45-
*/
4641
public class JsonArrayTest {
4742

4843
@Test
@@ -139,7 +134,7 @@ public void shouldDetectIncorrectItemInList() {
139134

140135
@Test
141136
public void shouldRecursiveParseList() {
142-
List<?> subList = Collections.singletonList("test");
137+
List<?> subList = singletonList("test");
143138
List<Object> source = new ArrayList<Object>(2);
144139
source.add("item1");
145140
source.add(subList);
@@ -154,7 +149,7 @@ public void shouldRecursiveParseList() {
154149

155150
@Test
156151
public void shouldRecursiveParseMap() {
157-
Map<String, ?> subMap = Collections.singletonMap("test", 2.5d);
152+
Map<String, ?> subMap = singletonMap("test", 2.5d);
158153
List<Object> source = new ArrayList<Object>(2);
159154
source.add("item1");
160155
source.add(subMap);
@@ -168,48 +163,36 @@ public void shouldRecursiveParseMap() {
168163
}
169164

170165
@Test
171-
public void shouldClassCastOnBadSubMap() {
172-
Map<Integer, String> badMap1 = Collections.singletonMap(1, "test");
173-
Map<String, Object> badMap2 = Collections.singletonMap("key1", (Object) new CloneNotSupportedException());
166+
public void shouldThrowOnBadSubMap() {
167+
Map<Integer, String> badMap1 = singletonMap(1, "test");
168+
Map<String, Object> badMap2 = singletonMap("key1", (Object) new CloneNotSupportedException());
174169

175-
List<?> source = Collections.singletonList(badMap1);
170+
List<?> source = singletonList(badMap1);
176171
try {
177172
JsonArray.from(source);
178-
fail("ClassCastException expected");
179-
} catch (ClassCastException e) {
180-
if (e.getCause() != null) {
181-
fail("No cause expected for sub map that are not Map<String, ?>");
182-
}
183-
} catch (Exception e) {
184-
fail("ClassCastException expected, not " + e);
173+
fail("IllegalArgumentException expected");
174+
} catch (IllegalArgumentException e) {
175+
// expected
185176
}
186177

187-
source = Collections.singletonList(badMap2);
178+
source = singletonList(badMap2);
188179
try {
189180
JsonArray.from(source);
190-
fail("ClassCastException expected");
191-
} catch (ClassCastException e) {
192-
if (!(e.getCause() instanceof IllegalArgumentException)) {
193-
fail("ClassCastException with an IllegalArgumentException cause expected");
194-
}
195-
} catch (Exception e) {
196-
fail("ClassCastException expected");
181+
fail("IllegalArgumentException expected");
182+
} catch (IllegalArgumentException e) {
183+
// expected
197184
}
198185
}
199186

200187
@Test
201-
public void shouldClassCastWithCauseOnBadSubList() {
202-
List<?> badSubList = Collections.singletonList(new CloneNotSupportedException());
203-
List<?> source = Collections.singletonList(badSubList);
188+
public void shouldThrowOnBadSubList() {
189+
List<?> badSubList = singletonList(new CloneNotSupportedException());
190+
List<?> source = singletonList(badSubList);
204191
try {
205192
JsonArray.from(source);
206-
fail("ClassCastException expected");
207-
} catch (ClassCastException e) {
208-
if (!(e.getCause() instanceof IllegalArgumentException)) {
209-
fail("ClassCastException with an IllegalArgumentException cause expected");
210-
}
211-
} catch (Exception e) {
212-
fail("ClassCastException expected");
193+
fail("IllegalArgumentException expected");
194+
} catch (IllegalArgumentException e) {
195+
// expected
213196
}
214197
}
215198

@@ -219,9 +202,9 @@ public void shouldTreatJsonValueNullConstantAsNull() {
219202
JsonArray arr = JsonArray.create();
220203
arr.add(JsonValue.NULL);
221204
arr.add(JsonObject.from(
222-
Collections.singletonMap("subNull", JsonValue.NULL)));
205+
singletonMap("subNull", JsonValue.NULL)));
223206
arr.add(JsonArray.from(
224-
Collections.singletonList(JsonValue.NULL)));
207+
singletonList(JsonValue.NULL)));
225208

226209
assertEquals(3, arr.size());
227210
assertNull(arr.get(0));
@@ -399,4 +382,18 @@ public void shouldSupportBigDecimalConverted() throws Exception {
399382
assertEquals(1234.567890123457, decoded.getDouble(0), 0);
400383
assertTrue(decoded.getNumber(0) instanceof Double);
401384
}
385+
386+
@Test
387+
public void canPutWhenTypeIsUnknown() {
388+
Object map = singletonMap("one", 1);
389+
Object list = singletonList("red");
390+
JsonArray json = JsonArray.create()
391+
.add(map)
392+
.add(list);
393+
394+
assertEquals(JsonArray.from(map, list), json);
395+
assertEquals(JsonObject.from(singletonMap("one", 1)), json.getObject(0));
396+
assertEquals(JsonArray.from(singletonList("red")), json.getArray(1));
397+
}
398+
402399
}

0 commit comments

Comments
 (0)