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

Allow custom JsonNode implementations #3699

Closed
marschall opened this issue Dec 14, 2022 · 3 comments
Closed

Allow custom JsonNode implementations #3699

marschall opened this issue Dec 14, 2022 · 3 comments
Milestone

Comments

@marschall
Copy link
Contributor

marschall commented Dec 14, 2022

Is your feature request related to a problem? Please describe.
com.fasterxml.jackson.databind.ObjectReader#readValue(JsonNode) currently only works with JsonNode implementations from the jackson-databind module. It does not work with custom JsonNode implementations. We have a use case where we would like to use custom JsonNode implementations.

Describe the solution you'd like
com.fasterxml.jackson.databind.ObjectReader#readValue(JsonNode) should work with any JsonNode implementation. The reason this currently does not work is because ObjectCursor currently casts to ObjectNode

There is no need for this as #fields() is defined on JsonNode. ArrayCursor for example does not cast to ArrayNode and just calls JsonNode#elements().

Usage example

JsonNode jsonNode = new CustomObjectNode();

this.objectMapper.readerFor(Custom.class).readValue(jsonNode);

Additional context
On our project we settled on Jackson and jackson-databind for our JSON parsing and object mapping needs. So far this has worked well for us. We also store JSON in the database as LOBs. Our database vendor has introduced a native JSON datatype. Part of this is a custom binary format to send JSON preparsed over the wire to the driver. The driver can use this format directly without the need to serialize to text first. The driver exposes this as javax.json.JsonObject objects to our code.

We are experimenting with adapting javax.json.JsonObject to com.fasterxml.jackson.databind.JsonNode. This would give us the efficiency of being able to use the driver to parse the database internal format while still being able to use jackson-databind for the mapping.

Simply removing the cast seems to do the trick. An additional check could be introduced, on the other hand ArrayCursor has no such check.

marschall@1209c84

@marschall marschall added the to-evaluate Issue that has been received but not yet evaluated label Dec 14, 2022
@cowtowncoder cowtowncoder removed the to-evaluate Issue that has been received but not yet evaluated label Dec 15, 2022
@cowtowncoder
Copy link
Member

@marschall Sounds like a good improvement if as you say fields() exists in JsonNode.
If you have time to provide a PR against 2.14 branch (seems like safe enough to add in a patch) I'd be happy to get it merged. A simple test to verify ability to use custom Object implementation would be awesome.

marschall added a commit to marschall/jackson-databind that referenced this issue Dec 15, 2022
marschall added a commit to marschall/jackson-databind that referenced this issue Dec 15, 2022
@marschall
Copy link
Contributor Author

There you to #3701. I have previously contributed to jackson-core and signed the CLA. See FasterXML/jackson-core#798

@cowtowncoder cowtowncoder changed the title Allow custom JsonNode implementations Allow custom JsonNode implementations Dec 20, 2022
@cowtowncoder cowtowncoder added this to the 2.14.2 milestone Dec 20, 2022
@cowtowncoder
Copy link
Member

Thank you @marschall! Will be in 2.14.2 patch when released.

cowtowncoder added a commit that referenced this issue Dec 20, 2022
dongjoon-hyun pushed a commit to apache/spark that referenced this issue Feb 7, 2023
### What changes were proposed in this pull request?
This pr aims upgrade `Jackson` related dependencies to 2.14.2

### Why are the changes needed?
This version include some bug fix and improvement of databind, such as:

- FasterXML/jackson-databind#3063
- FasterXML/jackson-databind#3699

The full release notes as follows:

- https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.14.2

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
Pass Github Actions

Closes #39898 from LuciferYang/SPARK-42354.

Authored-by: yangjie01 <yangjie01@baidu.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
dongjoon-hyun pushed a commit to apache/spark that referenced this issue Feb 7, 2023
### What changes were proposed in this pull request?
This pr aims upgrade `Jackson` related dependencies to 2.14.2

### Why are the changes needed?
This version include some bug fix and improvement of databind, such as:

- FasterXML/jackson-databind#3063
- FasterXML/jackson-databind#3699

The full release notes as follows:

- https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.14.2

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
Pass Github Actions

Closes #39898 from LuciferYang/SPARK-42354.

Authored-by: yangjie01 <yangjie01@baidu.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
(cherry picked from commit eb8b97f)
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
snmvaughan pushed a commit to snmvaughan/spark that referenced this issue Jun 20, 2023
### What changes were proposed in this pull request?
This pr aims upgrade `Jackson` related dependencies to 2.14.2

### Why are the changes needed?
This version include some bug fix and improvement of databind, such as:

- FasterXML/jackson-databind#3063
- FasterXML/jackson-databind#3699

The full release notes as follows:

- https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.14.2

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
Pass Github Actions

Closes apache#39898 from LuciferYang/SPARK-42354.

Authored-by: yangjie01 <yangjie01@baidu.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
(cherry picked from commit eb8b97f)
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
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

2 participants