Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Casting list of JSONObject to JSONObject throws ClassCastException #491

Closed
ggavriilidis opened this issue Oct 22, 2019 · 4 comments
Closed

Comments

@ggavriilidis
Copy link
Contributor

ggavriilidis commented Oct 22, 2019

When calling the toList() method of a JSONArray which contains JSONObject the returned list contains HashMap elements.
The documentation mentions:

Returns a java.util.List containing all of the elements in this array.
If an element in the array is a JSONArray or JSONObject it will also be converted

However, the elements are not converted to List.
Casting to JSONObject throws an exception if the toList() method is used.
Below is an example of the problem occurring:

Steps to reproduce

Run the below unit test:

@Test
   public void shouldCastArrayOfJSONObjectsToList() {
       String a = "{\"a\": 1}";
       String b = "{\"b\":2}";
       String jsonString = "{\"list\":[" + a + "," + b + "]}";
       JSONObject jsonObject = new JSONObject(jsonString);

       JSONArray jsonArray = jsonObject.getJSONArray("list");

       List<JSONObject> jsonObjects = jsonArray.toList().stream().map(JSONObject.class::cast).collect(Collectors.toList());

       assertEquals(jsonObjects.size(), 2);
   }

Expected Behaviour

No exception should be thrown and the test should pass

Actual Behaviour

ClassCastException is thrown.

java.lang.ClassCastException: Cannot cast java.util.HashMap to org.json.JSONObject

	at java.lang.Class.cast(Class.java:3369)
	at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193)
	at java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1382)
	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481)
	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
	at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708)
	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:499)
	at JsonTest.shouldCastArrayOfJSONObjectsToList(JsonTest.java:21)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
	at com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:47)
	at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242)
	at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)

Using iterator works

The following test however passes:

@Test
    public void shouldCastArrayOfJSONObjectsToList() {
        String a = "{\"a\": 1}";
        String b = "{\"b\":2}";
        String jsonString = "{\"list\":[" + a + "," + b + "]}";
        JSONObject jsonObject = new JSONObject(jsonString);

        JSONArray jsonArray = jsonObject.getJSONArray("list");

        List<JSONObject> jsonObjects = Lists.newArrayList(jsonArray.iterator()).stream().map(JSONObject.class::cast).collect(Collectors.toList());

        assertEquals(jsonObjects.size(), 2);
    }

Environment Information:
Operating System: macOS Sierra v 10.12.6
JDK Version: 1.8.0_152
org.json version: 20190722

@johnjaylward
Copy link
Contributor

johnjaylward commented Oct 22, 2019

The toList method does not convert to a list of JSONObject. It converts both JSONArray and JSONObject to standard java types List and Map. For a general use case, you would use something like this:

public void shouldCastArrayOfJSONObjectsToList() {
       String a = "{\"a\": 1}";
       String b = "{\"b\":2}";
       String jsonString = "{\"list\":[" + a + "," + b + ",8" + "]}";
       JSONObject jsonObject = new JSONObject(jsonString);

       JSONArray jsonArray = jsonObject.getJSONArray("list");

       List<Object> objects = jsonArray.toList();

       assertEquals(objects.size(), 3); // list of Map, Map, Integer
    }

For your specific example, your List would not be of JSONObject, but instead Map objects:

public void shouldCastArrayOfJSONObjectsToList() {
       String a = "{\"a\": 1}";
       String b = "{\"b\":2}";
       String jsonString = "{\"list\":[" + a + "," + b + "]}";
       JSONObject jsonObject = new JSONObject(jsonString);

       JSONArray jsonArray = jsonObject.getJSONArray("list");

       List<Map<String,Object>> objects = jsonArray.toList().stream().map(Map.class::cast).collect(Collectors.toList());

       assertEquals(objects.size(), 2); // list of Map, Map
    }

However, If you really want JSONObjects, I would use this method:

public void shouldCastArrayOfJSONObjectsToList() {
       String a = "{\"a\": 1}";
       String b = "{\"b\":2}";
       String jsonString = "{\"list\":[" + a + "," + b + "]}";
       JSONObject jsonObject = new JSONObject(jsonString);

       JSONArray jsonArray = jsonObject.getJSONArray("list");

       List<JSONObject> objects = StreamSupport.stream(jsonArray.spliterator(), false).map(o -> {
            if(o instanceof JSONObject) {
                return (JSONObject)o;
            }
            return null;
        }).filter(Objects::nonNull)
        .collect(Collectors.toList());;

       assertEquals(objects.size(), 2); // list of Map, Map
    }

The last method prevents a lot of work that the toList method is performing and will also filter out non-Json objects from the list.

@ggavriilidis
Copy link
Contributor Author

ggavriilidis commented Oct 29, 2019

Thanks for your response @johnjaylward .

Is it expected that the JSONArray iterator and spliterator methods behave differently than the toList method?

If this is expected maybe the documentation could add some more clarification.
Currently it mentions:

Returns a java.util.List containing all of the elements in this array.
If an element in the array is a JSONArray or JSONObject it will also be converted.

Could it be saying instead:

Returns a java.util.List containing all of the elements in this array.
If an element in the array is a JSONArray or JSONObject it will also be converted to a List and a Map respectively.

This clarifies what the output of the method is.

I have created #492 to address this.

Thanks

@stleary
Copy link
Owner

stleary commented Oct 30, 2019

Fixed in #492

@stleary stleary closed this as completed Oct 30, 2019
@johnjaylward
Copy link
Contributor

johnjaylward commented Oct 30, 2019

@ggavriilidis sorry for the late reply. You are correct that the toList method and the iterator/spliterator methods are meant to be used differently.

The JSONArray.toList and JSONObject.toMap methods are both supposed to remove any dependency on the org.json library by converting the JSONArray and JSONObject objects in a deep traversal to java.util.List and java.util.Map objects.

The iterator/spliterator methods are meant for direct access to the stored data. Using them in a loop would be akin to doing something like this:

JSONArray arr = new JSONArray();
// populate the array with some data ... somehow

// standard iteration of a populated JSONArray
for (int i = 0; i<arr.length(); i++) {
    Object o = arr.opt(i);
    // do something with my object
}

// the above loop is the same as this:
Iterator i = arr.iterator();
while (i.hasNext()) {
    Object o = i.next();
    // do something with my object
}

// or the same as this for Java 8+
StreamSupport.stream(arr.spliterator(), false).forEach(o -> {
    // do something with my object
});

In each case, above, the different iteration methods are all getting the same values for Object o, no conversion is done to the data.

This differs wildly from the toList method which is pre-traversing the entire collection for you and performing conversions where necessary. If you iterate over the return from toList, you will have effectively iterated over your JSONArray twice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants