Skip to content

Commit

Permalink
JCBC-1574 JsonObject/Array.put(String,Object) fails on Map or List
Browse files Browse the repository at this point in the history 
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 <[email protected]>
Reviewed-by: Michael Nitschinger <[email protected]>
  • Loading branch information
@dnault
dnault committed Jan 22, 2020
1 parent 5fabec4 commit d8a9aea
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 187 deletions.
56 changes: 4 additions & 52 deletions src/main/java/com/couchbase/client/java/document/json/JsonArray.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.ListIterator;
import java.util.Map;

/**
Expand All @@ -36,8 +35,6 @@
* The {@link JsonArray} is backed by a {@link List} and is intended to work similar to it API wise, but to only
* allow to store such objects which can be represented by JSON.
*
* @author Michael Nitschinger
* @author Simon Baslé
* @since 2.0
*/
public class JsonArray extends JsonValue implements Iterable<Object>, Serializable {
Expand Down Expand Up @@ -91,11 +88,7 @@ public static JsonArray create() {
public static JsonArray from(Object... items) {
JsonArray array = new JsonArray(items.length);
for (Object item : items) {
if (checkType(item)) {
array.add(item);
} else {
throw new IllegalArgumentException("Unsupported type for JsonArray: " + item.getClass());
}
array.add(coerce(item));
}
return array;
}
Expand All @@ -120,47 +113,11 @@ public static JsonArray from(Object... items) {
public static JsonArray from(List<?> items) {
if (items == null) {
throw new NullPointerException("Null list unsupported");
} else if (items.isEmpty()) {
return JsonArray.empty();
}

JsonArray array = new JsonArray(items.size());
ListIterator<?> iter = items.listIterator();
while (iter.hasNext()) {
int i = iter.nextIndex();
Object item = iter.next();

if (item == JsonValue.NULL) {
item = null;
}

if (item instanceof Map) {
try {
JsonObject sub = JsonObject.from((Map<String, ?>) item);
array.add(sub);
} catch (ClassCastException e) {
throw e;
} catch (Exception e) {
ClassCastException c = new ClassCastException("Couldn't convert sub-Map at " + i + " to JsonObject");
c.initCause(e);
throw c;
}
} else if (item instanceof List) {
try {
JsonArray sub = JsonArray.from((List<?>) item);
array.add(sub);
} catch (Exception e) {
//no risk of a direct ClassCastException here
ClassCastException c = new ClassCastException(
"Couldn't convert sub-List at " + i + " to JsonArray");
c.initCause(e);
throw c;
}
} else if (checkType(item)) {
array.add(item);
} else {
throw new IllegalArgumentException("Unsupported type for JsonArray: " + item.getClass());
}
for (Object item : items) {
array.add(coerce(item));
}
return array;
}
Expand Down Expand Up @@ -206,13 +163,8 @@ public Object get(int index) {
public JsonArray add(Object value) {
if (value == this) {
throw new IllegalArgumentException("Cannot add self");
} else if (value == JsonValue.NULL) {
addNull();
} else if (checkType(value)) {
content.add(value);
} else {
throw new IllegalArgumentException("Unsupported type for JsonArray: " + value.getClass());
}
content.add(coerce(value));
return this;
}

Expand Down
72 changes: 14 additions & 58 deletions src/main/java/com/couchbase/client/java/document/json/JsonObject.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
import java.util.Map;
import java.util.Set;

import static java.util.Objects.requireNonNull;

/**
* Represents a JSON object that can be stored and loaded from Couchbase Server.
*
Expand All @@ -42,8 +44,6 @@
* The {@link JsonObject} is backed by a {@link Map} and is intended to work similar to it API wise, but to only
* allow to store such objects which can be represented by JSON.
*
* @author Michael Nitschinger
* @author Simon Baslé
* @since 2.0
*/
public class JsonObject extends JsonValue implements Serializable {
Expand Down Expand Up @@ -127,50 +127,19 @@ public static JsonObject create() {
* @throws ClassCastException if map contains a sub-Map or sub-List not supported (see above)
*/
public static JsonObject from(Map<String, ?> mapData) {
if (mapData == null) {
throw new NullPointerException("Null input Map unsupported");
} else if (mapData.isEmpty()) {
return JsonObject.empty();
}
requireNonNull(mapData, "Null input Map unsupported");

JsonObject result = new JsonObject(mapData.size());
for (Map.Entry<String, ?> entry : mapData.entrySet()) {
String key = entry.getKey();
Object value = entry.getValue();

if (value == JsonValue.NULL) {
value = null;
}

if (key == null) {
throw new NullPointerException("The key is not allowed to be null");
} else if (value instanceof Map) {
try {
JsonObject sub = JsonObject.from((Map<String, ?>) value);
result.put(key, sub);
} catch (ClassCastException e) {
throw e;
} catch (Exception e) {
ClassCastException c = new ClassCastException("Couldn't convert sub-Map " + key + " to JsonObject");
c.initCause(e);
throw c;
}
} else if (value instanceof List) {
try {
JsonArray sub = JsonArray.from((List<?>) value);
result.put(key, sub);
} catch (Exception e) {
//no risk of a direct ClassCastException here
ClassCastException c = new ClassCastException("Couldn't convert sub-List " + key + " to JsonArray");
c.initCause(e);
throw c;
}
} else if (!checkType(value)) {
throw new IllegalArgumentException("Unsupported type for JsonObject: " + value.getClass());
} else {
result.put(key, value);
try {
for (Map.Entry<String, ?> entry : mapData.entrySet()) {
String key = requireNonNull(entry.getKey(), "The key is not allowed to be null");
Object value = entry.getValue();
result.put(key, coerce(value));
}
} catch (ClassCastException e) {
throw new IllegalArgumentException("Map key must be String", e);
}

return result;
}

Expand Down Expand Up @@ -203,13 +172,8 @@ public static JsonObject fromJson(String s) {
public JsonObject put(final String name, final Object value) {
if (this == value) {
throw new IllegalArgumentException("Cannot put self");
} else if (value == JsonValue.NULL) {
putNull(name);
} else if (checkType(value)) {
content.put(name, value);
} else {
throw new IllegalArgumentException("Unsupported type for JsonObject: " + value.getClass());
}
content.put(name, coerce(value));
return this;
}

Expand All @@ -230,15 +194,7 @@ public JsonObject put(final String name, final Object value) {
*/
public JsonObject putAndEncrypt(final String name, final Object value, String providerName) {
addValueEncryptionInfo(name, providerName, true);
if (this == value) {
throw new IllegalArgumentException("Cannot put self");
} else if (value == JsonValue.NULL) {
putNull(name);
} else if (checkType(value)) {
content.put(name, value);
} else {
throw new IllegalArgumentException("Unsupported type for JsonObject: " + value.getClass());
}
put(name, value);
return this;
}

Expand Down Expand Up @@ -1243,4 +1199,4 @@ public boolean equals(Object o) {
public int hashCode() {
return content.hashCode();
}
}
}
25 changes: 23 additions & 2 deletions src/main/java/com/couchbase/client/java/document/json/JsonValue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,20 @@

import java.math.BigDecimal;
import java.math.BigInteger;
import java.util.List;
import java.util.Map;

/**
* Represents a JSON value (either a {@link JsonObject} or a {@link JsonArray}.
*
* @author Michael Nitschinger
* @since 2.0
*/
public abstract class JsonValue {

/**
* Represents a Json "null".
*/
public static JsonNull NULL = JsonNull.INSTANCE;
public static final JsonNull NULL = JsonNull.INSTANCE;

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

/**
* Returns the given value converted to a type that passes the {@link #checkType} test.
* @throws IllegalArgumentException if conversion is not possible.
*/
@SuppressWarnings("unchecked")
static Object coerce(Object value) {
if (checkType(value)) {
return value;
}
if (value instanceof Map) {
return JsonObject.from((Map) value);
}
if (value instanceof List) {
return JsonArray.from((List) value);
}
if (value instanceof JsonNull) {
return null;
}
throw new IllegalArgumentException("Unsupported type for JSON value: " + value.getClass());
}
}
77 changes: 37 additions & 40 deletions src/test/java/com/couchbase/client/java/document/json/JsonArrayTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
import java.util.List;
import java.util.Map;

import static java.util.Collections.singletonList;
import static java.util.Collections.singletonMap;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
Expand All @@ -36,13 +38,6 @@
import static org.junit.Assert.fail;
import static org.junit.Assume.assumeFalse;

/**
* Verifies the functionality provided by a {@link JsonArray}.
*
* @author Michael Nitschinger
* @author Simon Baslé
* @since 2.0
*/
public class JsonArrayTest {

@Test
Expand Down Expand Up @@ -139,7 +134,7 @@ public void shouldDetectIncorrectItemInList() {

@Test
public void shouldRecursiveParseList() {
List<?> subList = Collections.singletonList("test");
List<?> subList = singletonList("test");
List<Object> source = new ArrayList<Object>(2);
source.add("item1");
source.add(subList);
Expand All @@ -154,7 +149,7 @@ public void shouldRecursiveParseList() {

@Test
public void shouldRecursiveParseMap() {
Map<String, ?> subMap = Collections.singletonMap("test", 2.5d);
Map<String, ?> subMap = singletonMap("test", 2.5d);
List<Object> source = new ArrayList<Object>(2);
source.add("item1");
source.add(subMap);
Expand All @@ -168,48 +163,36 @@ public void shouldRecursiveParseMap() {
}

@Test
public void shouldClassCastOnBadSubMap() {
Map<Integer, String> badMap1 = Collections.singletonMap(1, "test");
Map<String, Object> badMap2 = Collections.singletonMap("key1", (Object) new CloneNotSupportedException());
public void shouldThrowOnBadSubMap() {
Map<Integer, String> badMap1 = singletonMap(1, "test");
Map<String, Object> badMap2 = singletonMap("key1", (Object) new CloneNotSupportedException());

List<?> source = Collections.singletonList(badMap1);
List<?> source = singletonList(badMap1);
try {
JsonArray.from(source);
fail("ClassCastException expected");
} catch (ClassCastException e) {
if (e.getCause() != null) {
fail("No cause expected for sub map that are not Map<String, ?>");
}
} catch (Exception e) {
fail("ClassCastException expected, not " + e);
fail("IllegalArgumentException expected");
} catch (IllegalArgumentException e) {
// expected
}

source = Collections.singletonList(badMap2);
source = singletonList(badMap2);
try {
JsonArray.from(source);
fail("ClassCastException expected");
} catch (ClassCastException e) {
if (!(e.getCause() instanceof IllegalArgumentException)) {
fail("ClassCastException with an IllegalArgumentException cause expected");
}
} catch (Exception e) {
fail("ClassCastException expected");
fail("IllegalArgumentException expected");
} catch (IllegalArgumentException e) {
// expected
}
}

@Test
public void shouldClassCastWithCauseOnBadSubList() {
List<?> badSubList = Collections.singletonList(new CloneNotSupportedException());
List<?> source = Collections.singletonList(badSubList);
public void shouldThrowOnBadSubList() {
List<?> badSubList = singletonList(new CloneNotSupportedException());
List<?> source = singletonList(badSubList);
try {
JsonArray.from(source);
fail("ClassCastException expected");
} catch (ClassCastException e) {
if (!(e.getCause() instanceof IllegalArgumentException)) {
fail("ClassCastException with an IllegalArgumentException cause expected");
}
} catch (Exception e) {
fail("ClassCastException expected");
fail("IllegalArgumentException expected");
} catch (IllegalArgumentException e) {
// expected
}
}

Expand All @@ -219,9 +202,9 @@ public void shouldTreatJsonValueNullConstantAsNull() {
JsonArray arr = JsonArray.create();
arr.add(JsonValue.NULL);
arr.add(JsonObject.from(
Collections.singletonMap("subNull", JsonValue.NULL)));
singletonMap("subNull", JsonValue.NULL)));
arr.add(JsonArray.from(
Collections.singletonList(JsonValue.NULL)));
singletonList(JsonValue.NULL)));

assertEquals(3, arr.size());
assertNull(arr.get(0));
Expand Down Expand Up @@ -399,4 +382,18 @@ public void shouldSupportBigDecimalConverted() throws Exception {
assertEquals(1234.567890123457, decoded.getDouble(0), 0);
assertTrue(decoded.getNumber(0) instanceof Double);
}

@Test
public void canPutWhenTypeIsUnknown() {
Object map = singletonMap("one", 1);
Object list = singletonList("red");
JsonArray json = JsonArray.create()
.add(map)
.add(list);

assertEquals(JsonArray.from(map, list), json);
assertEquals(JsonObject.from(singletonMap("one", 1)), json.getObject(0));
assertEquals(JsonArray.from(singletonList("red")), json.getArray(1));
}

}
Loading

0 comments on commit d8a9aea

Please sign in to comment.