-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Different behaviour of string value deserialization in jackson 2.17 and 2.18 #5008
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
Comments
Can you try 2.18.3? |
If this ever worked then I think that was an accident. This is not JSON. |
Same problem in 2.18.3 I'm afraid |
From reading the README in https://github.com/askoog/jackson-deserialization, the actual JSON case now passes - so that seems like an improvement. |
The example is to make it easier to grasp. If you have a more complex json, it would look like this in 2.17
And in jackson 2.18 it must now be strucutred as
Both examples are valid json as far as I can see |
The correct way to support flattening classes that have single string values to be that string value is to use Jackson annotations. I don't personally like adding annotations and prefer to use customised DTO classes that match the JSON format that I want - so I am not an expert on the annotations that Jackson supports. Good overview: If this was my problem, I would possibly start with adding Since you do not have a setter for the I suggest trying a few annotation set ups to find one that works. |
Yes, I'm not asking for how to fix the code. You could also add a |
My view is that 2.18 fixed a bug in 2.17. If you don't like 2.18, you can
If enough people complain, we could add an optional feature to default the behaviour to 2.17 style. This is a lot of work so will probably only be done if there is a lot of complaints. The priority work in Jackson now is on 3.0.0. |
Ok, thanks for your suggestions. So the support for In 2.17 the I don't agree that the way 2.17 works is a bug, instead it has default support for having a constructor taking a single String value without any need for extra annotations. This support has been removed in 2.18 |
https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.18 are the release notes for the record. |
There have been a lot of code changes in the last 6 months or more that may have affected this. One that relates to property creators is #4689. Based on the release notes, it seems reasonably likely that this change was not deliberate because none of the issues listed and their descriptions seems to spell out a case like this. I may be overlooking something. For me, I actually like the new behaviour so I would like a solution that supports makes both of the test cases in https://github.com/askoog/jackson-deserialization pass. |
Could you please include POJO in question in-line, instead of (or in addition to) describing @askoog ? This is about binding JSON String into POJO, via -- I assume -- constructor of factory method, so the most important part is class definition. EDIT: I added class def. |
I created #5010 - I can reproduce results in https://github.com/askoog/jackson-deserialization but my PR doesn't seem to reproduce it (yet). |
@askoog This is unfortunately very much ambiguous case and unfortunately 2.17 and previous selected "Delegating" case for Constructor. Auto-detection in 2.18 selects "Properties" approach because of existing of logical There are multiple ways to annotate things differently, or possibly rename (f.ex, changing constructor parameter name). Annotation to use would be
|
By any chance, is it possible to register the ParameterNamesModule with delegating mode?
|
No, |
There is configuration setting There's also: https://cowtowncoder.medium.com/jackson-2-12-most-wanted-3-5-246624e2d3d0 which gives better example. It might solve @askoog 's problem if he always wants case like this to resolve to Delegating. |
To give some context, we have roughly 500-ish systems with different kinds of json parsing (mostly rest-api:s) and we're in the process of upgrading these to a newer spring boot version (3.3 -> 3.4) which also updates jackson from 2.17 to 2.18. Any solutions that retain the old functionality would be greatly appreciated. As of now, we might end up with really detailed acceptance testing of each system to assure this doesn't break any existing functionality ... I'll check if |
@askoog Yes, this is unfortunate. The big challenge with auto-detection has been difference between intended/document and actual implementation, for the specific case of 1-argument Creator (constructor / static factory methods) which is fundamentally ambiguous. 2.18 attempted to resolve all known issues with property introspection. This lead to improved handling/combining of information from Constructors and other accessors (Fields, getter/setter methods), which in turn subtly changed actual heuristics (for this particular case, detection of implied "value" property -- and allowing Constructor to be auto-detected if (but only if!) there is just 1 declared visible (public) constructor). So change b/w 2.17 and 2.18 was due to major rewrite of property introspection: and this was done without breaking any of existing unit tests. But the test suite did not cover all cases users had, obviously -- and in some cases behavior change, while fix, lead to incompatibilities. Intent now, however, is to stabilize things to 2.18 behavior. I hope this helps give some more context on why this unfortunate incompatibility occurred, and why it will not be reverted: this particular case should have worked the way it now does in 2.18, even in 2.17. Having said that, wherever we can help with some options wrt backwards-compatibility, we should try to do so. |
I will close this as "wont-fix" since, for better or worse, 2.18.x behavior is the way things are expected to work now. Use of |
Uh oh!
There was an error while loading. Please reload this page.
Search before asking
Describe the bug
Jackson 2.17 and 2.18 behaves differently when deserializing simple objects that wraps a String, given that
-parameters
to ensure parameter name retention in compiled codeParameterNamesModule
is registered as a module in the object mapperIn 2.17 a json document with a raw string value (
"abc123"
) is parsed correctly, but in 2.18 this is throws an exception and the json must be formatted as a json object ({ "value": "abc123" }
) instead (wherevalue
is the name of the field in the class). In 2.17 the json object form cannot be parsed.This makes the migration from 2.17 to 2.18 harder since it might cause existing json documents to became unparseable.
Version Information
2.17.x and 2.18.
Reproduction
Here is an example repository that reproduces the problem: https://github.com/askoog/jackson-deserialization.
Run
and
For different results, where one test case succeeds and one fails in each run.
Expected behavior
No regression should occur when upgrading to 2.18.x, the old json format should still be parseable.
Additional context
Value class used:
The text was updated successfully, but these errors were encountered: