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

JsonArray.contains(JsonObject / JsonArray) returns false, in case JsonObject / JsonArray is represented as Map / List internally #5397

Closed
kristian opened this issue Nov 19, 2024 · 3 comments · Fixed by #5398
Labels
Milestone

Comments

@kristian
Copy link
Contributor

If you want to check whether a JsonArray contains a certain JsonObject or other JsonArray you may wrongly assume you can use the contains method. However as sometimes a JsonObject / JsonArray is represented as a Map or List in the internal JsonArray list, contains might return false even though semantically it should return true. Minimal reproducible example:

JsonArray jsonArray = new JsonArray("[{\"foo\":\"bar\"}]");
System.out.println(jsonArray.contains(new JsonObject().put("foo", "bar"))); // false

System.out.println(jsonArray.contains(new JsonObject().put("foo", "bar").getMap())); // true
System.out.println(jsonArray.stream().anyMatch(obj -> obj.equals(new JsonObject().put("foo", "bar")))); // true

My assumption would be that the contains function behaves similarly to the contains function of the List interface, where it is documented that it returns true in case:

Returns true if this list contains the specified element. More formally, returns true if and only if this list contains at least one element e such that (o==null ? e==null : o.equals(e)).

Which is not the case for the contains function of the JsonArray. So I would suggest this should be either treated as a bug, or the behaviour should be documented in the JavaDoc of the contains function.

@kristian kristian added the bug label Nov 19, 2024
@tsegismont tsegismont added this to the 4.5.12 milestone Nov 19, 2024
@tsegismont
Copy link
Contributor

Thanks for detailed report @kristian . Would you like to propose a PR ?

@kristian
Copy link
Contributor Author

Sure @tsegismont, I can create a PR. Thanks for the quick reply!

@kristian
Copy link
Contributor Author

@tsegismont PR #5398 👍

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

Successfully merging a pull request may close this issue.

3 participants