-
Notifications
You must be signed in to change notification settings - Fork 36
Fix record serialization when using JacksonAnnotationExtension #207
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
base: 2.x
Are you sure you want to change the base?
Fix record serialization when using JacksonAnnotationExtension #207
Conversation
if (orig != null) { | ||
newProp = APropBuilder.merge(orig, newProp); | ||
} | ||
((IndexedMap<String, APropBuilder>) _props) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rationale: we don't want to change position, because of the renaming
Hi @cowtowncoder, I'd very appreciate if you could take a look at this PR (of course nothing urgent). Thanks in advance! |
@TomaszGaweda sorry for delay, hoping to review this week (having to focus on 3.0.0-rc10 first). Just thought I'll add a note to let you know this is on my radar, todo list. |
Thanks a lot @cowtowncoder for the update and no worries :) |
jr-annotation-support/src/main/java/com/fasterxml/jackson/jr/annotationsupport/IndexedMap.java
Show resolved
Hide resolved
…introspection' into fix-records-with-annotationbasedintrospection
jr-annotation-support/src/main/java/com/fasterxml/jackson/jr/annotationsupport/IndexedMap.java
Outdated
Show resolved
Hide resolved
Object value = prop.getReader().readNext(r, p); | ||
values[prop.getIndex()] = value; | ||
} | ||
for (int i = 0; i < values.length; i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would require a comment -- explaining that for primitive types null
s not allowed.
} | ||
for (int i = 0; i < values.length; i++) { | ||
if (values[i] == null) { | ||
for (BeanPropertyReader prop : _propsByName.values()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is rather inefficient with n*n complexity (although I guess mostly n if no null
s from missing entries?). Would be good to have either direct lookup (ordered array/List of props by index), or...
actually better idea, I think -- for records, pre-created Object[]
with defaults for primitive values.
Then something like:
final Object[] values = Arrays.copyOf(preset, preset.length);
and avoid post-scanning?
jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/impl/BeanReader.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty good; we are almost there! Couple of small changes needed and we are good to go!
This PR fixes record serialization and deserialization when using Java Records with JacksonAnnotationExtension
Fixes #176