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

readerForUpdating(objectToUpdate).readValue(json) behaves unexpectedly on Optional<List> #214

Closed
jc84-dev opened this issue Apr 19, 2021 · 5 comments
Milestone

Comments

@jc84-dev
Copy link

assume we have


class A {
//@JsonMerge  --> if this is added here, it will throw exception in the runtime
public Optional<List<String>> list = Optional.empty();
}

A obj = objectMapper.readValue("{list:['a']", A.class)   --> obj.list = ["a"] as expected
readerForUpdating(obj).readValue({list:['b'])    --> obj.list = ["b"]  which is not as expected.  I expected obj.list = ["a", "b"] here 

@cowtowncoder
Copy link
Member

Hmmh. I can see why this is sub-optimal. I think the root cause may be due to Optional itself being immutable (you cannot change value one has, only create new instance), which may be why merge is not attempted.

Quick question: just for reproduction, would behavior be identical if you used AtomicReference? I ask because they both use same base deserializer, ReferenceTypeDeserializer, and should for the most part exhibit same behavior.
If so, I could add reproduction using AtomicReference in jackson-databind and same fix would likely apply.
If that did not fail the problem is more likely in this package.

@jc84-dev
Copy link
Author

AtomicReference works well. looks like it is specific to Optional

@jc84-dev jc84-dev reopened this Apr 21, 2021
cowtowncoder added a commit that referenced this issue Apr 24, 2021
@cowtowncoder cowtowncoder added this to the 2.12.4 milestone Apr 24, 2021
@cowtowncoder
Copy link
Member

Was about to claim I cannot reproduce this, until I realized I had not initialized Optional field to Optional.empty(). Once I add that, I see the exception.

Note that @JsonMerge is absolutely needed there (or global configuration to default to merge): otherwise that property won't be merged no matter what.

I think I can fix this for 2.12.4.

@cowtowncoder
Copy link
Member

As per notes, now fixed in 2.12 branch, to be included in 2.12.4.

@jc84-dev
Copy link
Author

jc84-dev commented May 7, 2021

thanks

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