Skip to content

Commit 5541a6d

Browse files
stlearystleary
authored
Merge pull request stleary#552 from johnjaylward/JSONArrayCopyConstructor
Updates for JSONArray.putAll methods
2 parents 6351fa6 + d30ecad commit 5541a6d

4 files changed

Lines changed: 241 additions & 19 deletions

File tree

‎README.md‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,45 @@ Some notable exceptions that the JSON Parser in this library accepts are:
118118
* Unescaped literals like "tab" in string values `{ "key": "value with an unescaped tab" }`
119119
* Numbers out of range for `Double` or `Long` are parsed as strings
120120

121+
Recent pull requests added a new method `putAll` on the JSONArray. The `putAll` method
122+
works similarly as other `put` mehtods in that it does not call `JSONObject.wrap` for items
123+
added. This can lead to inconsistent object representation in JSONArray structures.
124+
125+
For example, code like this will create a mixed JSONArray, some items wrapped, others
126+
not:
127+
128+
```java
129+
SomeBean[] myArr = new SomeBean[]{ new SomeBean(1), new SomeBean(2) };
130+
// these will be wrapped
131+
JSONArray jArr = new JSONArray(myArr);
132+
// these will not be wrapped
133+
jArr.putAll(new SomeBean[]{ new SomeBean(3), new SomeBean(4) });
134+
```
135+
136+
For structure consistency, it would be recommended that the above code is changed
137+
to look like 1 of 2 ways.
138+
139+
Option 1:
140+
```Java
141+
SomeBean[] myArr = new SomeBean[]{ new SomeBean(1), new SomeBean(2) };
142+
JSONArray jArr = new JSONArray();
143+
// these will not be wrapped
144+
jArr.putAll(myArr);
145+
// these will not be wrapped
146+
jArr.putAll(new SomeBean[]{ new SomeBean(3), new SomeBean(4) });
147+
// our jArr is now consistent.
148+
```
149+
150+
Option 2:
151+
```Java
152+
SomeBean[] myArr = new SomeBean[]{ new SomeBean(1), new SomeBean(2) };
153+
// these will be wrapped
154+
JSONArray jArr = new JSONArray(myArr);
155+
// these will be wrapped
156+
jArr.putAll(new JSONArray(new SomeBean[]{ new SomeBean(3), new SomeBean(4) }));
157+
// our jArr is now consistent.
158+
```
159+
121160
**Unit Test Conventions**
122161

123162
Test filenames should consist of the name of the module being tested, with the suffix "Test".

‎src/main/java/org/json/JSONArray.java‎

Lines changed: 126 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,37 @@ public JSONArray(Collection<?> collection) {
173173
this.myArrayList = new ArrayList<Object>();
174174
} else {
175175
this.myArrayList = new ArrayList<Object>(collection.size());
176-
this.addAll(collection);
176+
this.addAll(collection, true);
177+
}
178+
}
179+
180+
/**
181+
* Construct a JSONArray from an Iterable. This is a shallow copy.
182+
*
183+
* @param iter
184+
* A Iterable collection.
185+
*/
186+
public JSONArray(Iterable<?> iter) {
187+
this();
188+
if (iter == null) {
189+
return;
190+
}
191+
this.addAll(iter, true);
192+
}
193+
194+
/**
195+
* Construct a JSONArray from another JSONArray. This is a shallow copy.
196+
*
197+
* @param array
198+
* A array.
199+
*/
200+
public JSONArray(JSONArray array) {
201+
if (array == null) {
202+
this.myArrayList = new ArrayList<Object>();
203+
} else {
204+
// shallow copy directly the internal array lists as any wrapping
205+
// should have been done already in the original JSONArray
206+
this.myArrayList = new ArrayList<Object>(array.myArrayList);
177207
}
178208
}
179209

@@ -191,7 +221,11 @@ public JSONArray(Collection<?> collection) {
191221
*/
192222
public JSONArray(Object array) throws JSONException {
193223
this();
194-
this.addAll(array);
224+
if (!array.getClass().isArray()) {
225+
throw new JSONException(
226+
"JSONArray initial value should be a string or collection or array.");
227+
}
228+
this.addAll(array, true);
195229
}
196230

197231
/**
@@ -1165,32 +1199,58 @@ public JSONArray put(int index, Object value) throws JSONException {
11651199
}
11661200

11671201
/**
1168-
* Put or replace a collection's elements in the JSONArray.
1202+
* Put a collection's elements in to the JSONArray.
11691203
*
11701204
* @param collection
11711205
* A Collection.
11721206
* @return this.
11731207
*/
11741208
public JSONArray putAll(Collection<?> collection) {
1175-
this.addAll(collection);
1209+
this.addAll(collection, false);
1210+
return this;
1211+
}
1212+
1213+
/**
1214+
* Put an Iterable's elements in to the JSONArray.
1215+
*
1216+
* @param iter
1217+
* An Iterable.
1218+
* @return this.
1219+
*/
1220+
public JSONArray putAll(Iterable<?> iter) {
1221+
this.addAll(iter, false);
11761222
return this;
11771223
}
11781224

11791225
/**
1180-
* Put or replace an array's elements in the JSONArray.
1226+
* Put a JSONArray's elements in to the JSONArray.
11811227
*
11821228
* @param array
1183-
* Array. If the parameter passed is null, or not an array, an
1229+
* A JSONArray.
1230+
* @return this.
1231+
*/
1232+
public JSONArray putAll(JSONArray array) {
1233+
// directly copy the elements from the source array to this one
1234+
// as all wrapping should have been done already in the source.
1235+
this.myArrayList.addAll(array.myArrayList);
1236+
return this;
1237+
}
1238+
1239+
/**
1240+
* Put an array's elements in to the JSONArray.
1241+
*
1242+
* @param array
1243+
* Array. If the parameter passed is null, or not an array or Iterable, an
11841244
* exception will be thrown.
11851245
* @return this.
11861246
*
11871247
* @throws JSONException
1188-
* If not an array or if an array value is non-finite number.
1248+
* If not an array, JSONArray, Iterable or if an value is non-finite number.
11891249
* @throws NullPointerException
11901250
* Thrown if the array parameter is null.
11911251
*/
11921252
public JSONArray putAll(Object array) throws JSONException {
1193-
this.addAll(array);
1253+
this.addAll(array, false);
11941254
return this;
11951255
}
11961256

@@ -1520,39 +1580,88 @@ public boolean isEmpty() {
15201580
return this.myArrayList.isEmpty();
15211581
}
15221582

1523-
15241583
/**
15251584
* Add a collection's elements to the JSONArray.
15261585
*
15271586
* @param collection
15281587
* A Collection.
1588+
* @param wrap
1589+
* {@code true} to call {@link JSONObject#wrap(Object)} for each item,
1590+
* {@code false} to add the items directly
1591+
*
15291592
*/
1530-
private void addAll(Collection<?> collection) {
1593+
private void addAll(Collection<?> collection, boolean wrap) {
15311594
this.myArrayList.ensureCapacity(this.myArrayList.size() + collection.size());
1532-
for (Object o: collection){
1533-
this.myArrayList.add(JSONObject.wrap(o));
1595+
if (wrap) {
1596+
for (Object o: collection){
1597+
this.put(JSONObject.wrap(o));
1598+
}
1599+
} else {
1600+
for (Object o: collection){
1601+
this.put(o);
1602+
}
15341603
}
15351604
}
15361605

1606+
/**
1607+
* Add an Iterable's elements to the JSONArray.
1608+
*
1609+
* @param iter
1610+
* An Iterable.
1611+
* @param wrap
1612+
* {@code true} to call {@link JSONObject#wrap(Object)} for each item,
1613+
* {@code false} to add the items directly
1614+
*/
1615+
private void addAll(Iterable<?> iter, boolean wrap) {
1616+
if (wrap) {
1617+
for (Object o: iter){
1618+
this.put(JSONObject.wrap(o));
1619+
}
1620+
} else {
1621+
for (Object o: iter){
1622+
this.put(o);
1623+
}
1624+
}
1625+
}
1626+
15371627
/**
15381628
* Add an array's elements to the JSONArray.
15391629
*
15401630
* @param array
1541-
* Array. If the parameter passed is null, or not an array, an
1542-
* exception will be thrown.
1631+
* Array. If the parameter passed is null, or not an array,
1632+
* JSONArray, Collection, or Iterable, an exception will be
1633+
* thrown.
1634+
* @param wrap
1635+
* {@code true} to call {@link JSONObject#wrap(Object)} for each item,
1636+
* {@code false} to add the items directly
15431637
*
15441638
* @throws JSONException
15451639
* If not an array or if an array value is non-finite number.
15461640
* @throws NullPointerException
15471641
* Thrown if the array parameter is null.
15481642
*/
1549-
private void addAll(Object array) throws JSONException {
1643+
private void addAll(Object array, boolean wrap) throws JSONException {
15501644
if (array.getClass().isArray()) {
15511645
int length = Array.getLength(array);
15521646
this.myArrayList.ensureCapacity(this.myArrayList.size() + length);
1553-
for (int i = 0; i < length; i += 1) {
1554-
this.put(JSONObject.wrap(Array.get(array, i)));
1647+
if (wrap) {
1648+
for (int i = 0; i < length; i += 1) {
1649+
this.put(JSONObject.wrap(Array.get(array, i)));
1650+
}
1651+
} else {
1652+
for (int i = 0; i < length; i += 1) {
1653+
this.put(Array.get(array, i));
1654+
}
15551655
}
1656+
} else if (array instanceof JSONArray) {
1657+
// use the built in array list `addAll` as all object
1658+
// wrapping should have been completed in the original
1659+
// JSONArray
1660+
this.myArrayList.addAll(((JSONArray)array).myArrayList);
1661+
} else if (array instanceof Collection) {
1662+
this.addAll((Collection<?>)array, wrap);
1663+
} else if (array instanceof Iterable) {
1664+
this.addAll((Iterable<?>)array, wrap);
15561665
} else {
15571666
throw new JSONException(
15581667
"JSONArray initial value should be a string or collection or array.");

‎src/test/java/org/json/junit/JSONArrayTest.java‎

Lines changed: 69 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -979,9 +979,9 @@ public void write() throws IOException {
979979
JSONArray jsonArray = new JSONArray(str);
980980
String expectedStr = str;
981981
StringWriter stringWriter = new StringWriter();
982-
jsonArray.write(stringWriter);
983-
String actualStr = stringWriter.toString();
984982
try {
983+
jsonArray.write(stringWriter);
984+
String actualStr = stringWriter.toString();
985985
JSONArray finalArray = new JSONArray(actualStr);
986986
Util.compareActualVsExpectedJsonArrays(jsonArray, finalArray);
987987
assertTrue("write() expected " + expectedStr +
@@ -1187,4 +1187,71 @@ public void testJSONArrayInt() {
11871187
e.getMessage());
11881188
}
11891189
}
1190+
1191+
/**
1192+
* Verifies that the object constructor can properly handle any supported collection object.
1193+
*/
1194+
@Test
1195+
@SuppressWarnings({ "unchecked", "boxing" })
1196+
public void testObjectConstructor() {
1197+
// should copy the array
1198+
Object o = new Object[] {2, "test2", true};
1199+
JSONArray a = new JSONArray(o);
1200+
assertNotNull("Should not error", a);
1201+
assertEquals("length", 3, a.length());
1202+
1203+
// should NOT copy the collection
1204+
// this is required for backwards compatibility
1205+
o = new ArrayList<Object>();
1206+
((Collection<Object>)o).add(1);
1207+
((Collection<Object>)o).add("test");
1208+
((Collection<Object>)o).add(false);
1209+
try {
1210+
a = new JSONArray(o);
1211+
assertNull("Should error", a);
1212+
} catch (JSONException ex) {
1213+
}
1214+
1215+
// should NOT copy the JSONArray
1216+
// this is required for backwards compatibility
1217+
o = a;
1218+
try {
1219+
a = new JSONArray(o);
1220+
assertNull("Should error", a);
1221+
} catch (JSONException ex) {
1222+
}
1223+
}
1224+
1225+
/**
1226+
* Verifies that the JSONArray constructor properly copies the original.
1227+
*/
1228+
@Test
1229+
public void testJSONArrayConstructor() {
1230+
// should copy the array
1231+
JSONArray a1 = new JSONArray("[2, \"test2\", true]");
1232+
JSONArray a2 = new JSONArray(a1);
1233+
assertNotNull("Should not error", a2);
1234+
assertEquals("length", a1.length(), a2.length());
1235+
1236+
for(int i = 0; i < a1.length(); i++) {
1237+
assertEquals("index " + i + " are equal", a1.get(i), a2.get(i));
1238+
}
1239+
}
1240+
1241+
/**
1242+
* Verifies that the object constructor can properly handle any supported collection object.
1243+
*/
1244+
@Test
1245+
public void testJSONArrayPutAll() {
1246+
// should copy the array
1247+
JSONArray a1 = new JSONArray("[2, \"test2\", true]");
1248+
JSONArray a2 = new JSONArray();
1249+
a2.putAll(a1);
1250+
assertNotNull("Should not error", a2);
1251+
assertEquals("length", a1.length(), a2.length());
1252+
1253+
for(int i = 0; i < a1.length(); i++) {
1254+
assertEquals("index " + i + " are equal", a1.get(i), a2.get(i));
1255+
}
1256+
}
11901257
}

‎src/test/java/org/json/junit/JSONObjectTest.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3201,4 +3201,11 @@ public void testPutNullObject() {
32013201
fail("Expected an exception");
32023202
}
32033203

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

0 commit comments

Comments
 (0)