Skip to content

Commit

Permalink
Merge pull request #552 from johnjaylward/JSONArrayCopyConstructor
Browse files Browse the repository at this point in the history
Updates for JSONArray.putAll methods
  • Loading branch information
stleary authored Aug 1, 2020
2 parents 6351fa6 + d30ecad commit 5541a6d
Show file tree
Hide file tree
Showing 4 changed files with 241 additions and 19 deletions.
39 changes: 39 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,45 @@ Some notable exceptions that the JSON Parser in this library accepts are:
* Unescaped literals like "tab" in string values `{ "key": "value with an unescaped tab" }`
* Numbers out of range for `Double` or `Long` are parsed as strings

Recent pull requests added a new method `putAll` on the JSONArray. The `putAll` method
works similarly as other `put` mehtods in that it does not call `JSONObject.wrap` for items
added. This can lead to inconsistent object representation in JSONArray structures.

For example, code like this will create a mixed JSONArray, some items wrapped, others
not:

```java
SomeBean[] myArr = new SomeBean[]{ new SomeBean(1), new SomeBean(2) };
// these will be wrapped
JSONArray jArr = new JSONArray(myArr);
// these will not be wrapped
jArr.putAll(new SomeBean[]{ new SomeBean(3), new SomeBean(4) });
```

For structure consistency, it would be recommended that the above code is changed
to look like 1 of 2 ways.

Option 1:
```Java
SomeBean[] myArr = new SomeBean[]{ new SomeBean(1), new SomeBean(2) };
JSONArray jArr = new JSONArray();
// these will not be wrapped
jArr.putAll(myArr);
// these will not be wrapped
jArr.putAll(new SomeBean[]{ new SomeBean(3), new SomeBean(4) });
// our jArr is now consistent.
```

Option 2:
```Java
SomeBean[] myArr = new SomeBean[]{ new SomeBean(1), new SomeBean(2) };
// these will be wrapped
JSONArray jArr = new JSONArray(myArr);
// these will be wrapped
jArr.putAll(new JSONArray(new SomeBean[]{ new SomeBean(3), new SomeBean(4) }));
// our jArr is now consistent.
```

**Unit Test Conventions**

Test filenames should consist of the name of the module being tested, with the suffix "Test".
Expand Down
143 changes: 126 additions & 17 deletions src/main/java/org/json/JSONArray.java
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,37 @@ public JSONArray(Collection<?> collection) {
this.myArrayList = new ArrayList<Object>();
} else {
this.myArrayList = new ArrayList<Object>(collection.size());
this.addAll(collection);
this.addAll(collection, true);
}
}

/**
* Construct a JSONArray from an Iterable. This is a shallow copy.
*
* @param iter
* A Iterable collection.
*/
public JSONArray(Iterable<?> iter) {
this();
if (iter == null) {
return;
}
this.addAll(iter, true);
}

/**
* Construct a JSONArray from another JSONArray. This is a shallow copy.
*
* @param array
* A array.
*/
public JSONArray(JSONArray array) {
if (array == null) {
this.myArrayList = new ArrayList<Object>();
} else {
// shallow copy directly the internal array lists as any wrapping
// should have been done already in the original JSONArray
this.myArrayList = new ArrayList<Object>(array.myArrayList);
}
}

Expand All @@ -191,7 +221,11 @@ public JSONArray(Collection<?> collection) {
*/
public JSONArray(Object array) throws JSONException {
this();
this.addAll(array);
if (!array.getClass().isArray()) {
throw new JSONException(
"JSONArray initial value should be a string or collection or array.");
}
this.addAll(array, true);
}

/**
Expand Down Expand Up @@ -1165,32 +1199,58 @@ public JSONArray put(int index, Object value) throws JSONException {
}

/**
* Put or replace a collection's elements in the JSONArray.
* Put a collection's elements in to the JSONArray.
*
* @param collection
* A Collection.
* @return this.
*/
public JSONArray putAll(Collection<?> collection) {
this.addAll(collection);
this.addAll(collection, false);
return this;
}

/**
* Put an Iterable's elements in to the JSONArray.
*
* @param iter
* An Iterable.
* @return this.
*/
public JSONArray putAll(Iterable<?> iter) {
this.addAll(iter, false);
return this;
}

/**
* Put or replace an array's elements in the JSONArray.
* Put a JSONArray's elements in to the JSONArray.
*
* @param array
* Array. If the parameter passed is null, or not an array, an
* A JSONArray.
* @return this.
*/
public JSONArray putAll(JSONArray array) {
// directly copy the elements from the source array to this one
// as all wrapping should have been done already in the source.
this.myArrayList.addAll(array.myArrayList);
return this;
}

/**
* Put an array's elements in to the JSONArray.
*
* @param array
* Array. If the parameter passed is null, or not an array or Iterable, an
* exception will be thrown.
* @return this.
*
* @throws JSONException
* If not an array or if an array value is non-finite number.
* If not an array, JSONArray, Iterable or if an value is non-finite number.
* @throws NullPointerException
* Thrown if the array parameter is null.
*/
public JSONArray putAll(Object array) throws JSONException {
this.addAll(array);
this.addAll(array, false);
return this;
}

Expand Down Expand Up @@ -1520,39 +1580,88 @@ public boolean isEmpty() {
return this.myArrayList.isEmpty();
}


/**
* Add a collection's elements to the JSONArray.
*
* @param collection
* A Collection.
* @param wrap
* {@code true} to call {@link JSONObject#wrap(Object)} for each item,
* {@code false} to add the items directly
*
*/
private void addAll(Collection<?> collection) {
private void addAll(Collection<?> collection, boolean wrap) {
this.myArrayList.ensureCapacity(this.myArrayList.size() + collection.size());
for (Object o: collection){
this.myArrayList.add(JSONObject.wrap(o));
if (wrap) {
for (Object o: collection){
this.put(JSONObject.wrap(o));
}
} else {
for (Object o: collection){
this.put(o);
}
}
}

/**
* Add an Iterable's elements to the JSONArray.
*
* @param iter
* An Iterable.
* @param wrap
* {@code true} to call {@link JSONObject#wrap(Object)} for each item,
* {@code false} to add the items directly
*/
private void addAll(Iterable<?> iter, boolean wrap) {
if (wrap) {
for (Object o: iter){
this.put(JSONObject.wrap(o));
}
} else {
for (Object o: iter){
this.put(o);
}
}
}

/**
* Add an array's elements to the JSONArray.
*
* @param array
* Array. If the parameter passed is null, or not an array, an
* exception will be thrown.
* Array. If the parameter passed is null, or not an array,
* JSONArray, Collection, or Iterable, an exception will be
* thrown.
* @param wrap
* {@code true} to call {@link JSONObject#wrap(Object)} for each item,
* {@code false} to add the items directly
*
* @throws JSONException
* If not an array or if an array value is non-finite number.
* @throws NullPointerException
* Thrown if the array parameter is null.
*/
private void addAll(Object array) throws JSONException {
private void addAll(Object array, boolean wrap) throws JSONException {
if (array.getClass().isArray()) {
int length = Array.getLength(array);
this.myArrayList.ensureCapacity(this.myArrayList.size() + length);
for (int i = 0; i < length; i += 1) {
this.put(JSONObject.wrap(Array.get(array, i)));
if (wrap) {
for (int i = 0; i < length; i += 1) {
this.put(JSONObject.wrap(Array.get(array, i)));
}
} else {
for (int i = 0; i < length; i += 1) {
this.put(Array.get(array, i));
}
}
} else if (array instanceof JSONArray) {
// use the built in array list `addAll` as all object
// wrapping should have been completed in the original
// JSONArray
this.myArrayList.addAll(((JSONArray)array).myArrayList);
} else if (array instanceof Collection) {
this.addAll((Collection<?>)array, wrap);
} else if (array instanceof Iterable) {
this.addAll((Iterable<?>)array, wrap);
} else {
throw new JSONException(
"JSONArray initial value should be a string or collection or array.");
Expand Down
71 changes: 69 additions & 2 deletions src/test/java/org/json/junit/JSONArrayTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -979,9 +979,9 @@ public void write() throws IOException {
JSONArray jsonArray = new JSONArray(str);
String expectedStr = str;
StringWriter stringWriter = new StringWriter();
jsonArray.write(stringWriter);
String actualStr = stringWriter.toString();
try {
jsonArray.write(stringWriter);
String actualStr = stringWriter.toString();
JSONArray finalArray = new JSONArray(actualStr);
Util.compareActualVsExpectedJsonArrays(jsonArray, finalArray);
assertTrue("write() expected " + expectedStr +
Expand Down Expand Up @@ -1187,4 +1187,71 @@ public void testJSONArrayInt() {
e.getMessage());
}
}

/**
* Verifies that the object constructor can properly handle any supported collection object.
*/
@Test
@SuppressWarnings({ "unchecked", "boxing" })
public void testObjectConstructor() {
// should copy the array
Object o = new Object[] {2, "test2", true};
JSONArray a = new JSONArray(o);
assertNotNull("Should not error", a);
assertEquals("length", 3, a.length());

// should NOT copy the collection
// this is required for backwards compatibility
o = new ArrayList<Object>();
((Collection<Object>)o).add(1);
((Collection<Object>)o).add("test");
((Collection<Object>)o).add(false);
try {
a = new JSONArray(o);
assertNull("Should error", a);
} catch (JSONException ex) {
}

// should NOT copy the JSONArray
// this is required for backwards compatibility
o = a;
try {
a = new JSONArray(o);
assertNull("Should error", a);
} catch (JSONException ex) {
}
}

/**
* Verifies that the JSONArray constructor properly copies the original.
*/
@Test
public void testJSONArrayConstructor() {
// should copy the array
JSONArray a1 = new JSONArray("[2, \"test2\", true]");
JSONArray a2 = new JSONArray(a1);
assertNotNull("Should not error", a2);
assertEquals("length", a1.length(), a2.length());

for(int i = 0; i < a1.length(); i++) {
assertEquals("index " + i + " are equal", a1.get(i), a2.get(i));
}
}

/**
* Verifies that the object constructor can properly handle any supported collection object.
*/
@Test
public void testJSONArrayPutAll() {
// should copy the array
JSONArray a1 = new JSONArray("[2, \"test2\", true]");
JSONArray a2 = new JSONArray();
a2.putAll(a1);
assertNotNull("Should not error", a2);
assertEquals("length", a1.length(), a2.length());

for(int i = 0; i < a1.length(); i++) {
assertEquals("index " + i + " are equal", a1.get(i), a2.get(i));
}
}
}
7 changes: 7 additions & 0 deletions src/test/java/org/json/junit/JSONObjectTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -3201,4 +3201,11 @@ public void testPutNullObject() {
fail("Expected an exception");
}

@Test
public void testIssue548ObjectWithEmptyJsonArray() {
JSONObject jsonObject = new JSONObject("{\"empty_json_array\": []}");
assertTrue("missing expected key 'empty_json_array'", jsonObject.has("empty_json_array"));
assertNotNull("'empty_json_array' should be an array", jsonObject.getJSONArray("empty_json_array"));
assertEquals("'empty_json_array' should have a length of 0", 0, jsonObject.getJSONArray("empty_json_array").length());
}
}

0 comments on commit 5541a6d

Please sign in to comment.