-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add any-setter handling in PropertyValueBuffer
post #562
#4720
Conversation
PropertyValueBuffer
post #562PropertyValueBuffer
post #562
if (_anyParamSetter != null) { | ||
if (prop.getCreatorIndex() == _anyParamSetter.getParameterIndex()) { | ||
return true; | ||
} | ||
} |
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.
I am not sure if this is the most optimal way of checking, any idea?
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.
Let me rewrite it slightly, considering that expectation is that match (true) is more likely than non-match.
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.
Rewrite looks cleaner, thanks!
release-notes/VERSION-2.x
Outdated
@@ -81,6 +81,9 @@ Project: jackson-databind | |||
(contributed by @pjfanning) | |||
#4709: Add `JacksonCollectors` with `toArrayNode()` implementation | |||
(contributed by @rikkarth) | |||
#4508: Deserialized JsonAnySetter field in Kotlin data class is null #4508 | |||
(reported by @MaximValeev) |
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.
2.18.0 is released already - shouldn't this be in the 2.18.1 section?
/** | ||
* Helper method called to create and set any values buffered for "any setter" | ||
* | ||
* @since 2.18 |
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.
Do we really need since annotations on private methods? In which case, this is wrong, it should be 2.18.1 - be I prefer not to annotate private methods.
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.
Makes sense.
any way to test this in a unit test? |
Will try to come up one soon! |
Added reproduction (with fix) in PR descrption. cc @pjfanning @cowtowncoder |
Right, one in the middle should be pointing to #562. Thank you @reneleonhardt ! |
fixes #4508
This sort of adds missing handling in addition to #562 fix. Where we should also consider situations like #4508, where extensions (like Kotlin module) iterates through parameters one by one, via
hasParmeter()
andgetParameter()
, unlike StdValueInstantiator going through all parameters at once viagetParameters(SettableBeanProperty[])
.Reproduction with fix