-
-
Notifications
You must be signed in to change notification settings - Fork 142
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
See if there is benefit from integrating with jackson-databind better wrt detecting "canonical" Constructor for Scala (case) classes #677
Comments
Note on properties- vs delegation-based Creators -- instead of 2 calls, I was also wondering about single call, that could return one (or zero) of each. This because we can accept one properties-based and delegating creators, used as alternatives (for JSON Object, properties-based, for other JSON values, delegating). |
@cowtowncoder This module already has a very complicated AnnotationIntrospector that works. I would prefer not to rewrite it. If you add extra methods to the super class that I need to override and add to this module's AnnotationIntrospector to avoid compilation issues, I will add something but it will probably be the minimal needed to get things working again. This module traditionally has not had many tests that test what happens when you start adding loads of Jackson annotations to Scala classes. We don't get a large amount of issues complaining about this - so I can only assume that users of this module usually rely on the vanilla behaviour that you get if you don't add Jackson annotations. If we get users complaining, I can handle their complaints on a case by case basis. I fear that making big changes is as likely to break things for some existing users than to fix anything - I can't think of any outstanding issues that might be fixed by the new jackson-databind support. For the record, Java Records are very similar in intent to Scala Case Classes. This module has long been built around supporting Case Classes. |
@pjfanning If it ain't broke don't fix it -- I opened this issue to probe if this could help or not. If not, there is no strict need to do anything with new functionality. Having said that, I was hoping that much improved handling for explicit core annotations could be nice benefit: Scala module would require less code to provide basic abilities for Jackson annotations. |
We can leave this open for a bit, and if nothing comes out of it, close. |
With 2.18 jackson-databind has a new, rewritten logic for POJO Property detection: work was done mostly under FasterXML/jackson-databind#4515.
Logic, at high-level, selects at most one "Properties-based" Creator (constructor / static factory method), in following order:
@JsonCreator
on ctor/factory, or at least one parameter of ctor/factory annotated with@JsonProperty
)This works for many cases. But JVM languages like Scala (and Kotlin) typically have their own definition of canonical constructor, so it'd be good to allow pluggable handling.
(in plain Java we only have Records with the concept of specific canonical (primary) constructor, over alternatives that are also visible)
My initial idea would be to add one (ideally) new method in
AnnotationIntrospector
, which would take 2 sets of candidates -- Constructors and Factory Methods (PotentialCreator
) -- that might qualify; and result would be zero or one of passed-in choices to indicate Canonical, Properties-based creator to use, if any.Method would only be called if no explicit choice were found.
I am not sure if similar functionality should be available to allow implicit Delegating Creator: possibly?
If so, it would be given 2 sets of potential candidates, as well as previously chosen Properties-based creator, if any. But I am not yet sure that is needed.
Would the first part make sense, @pjfanning, from Scala module perspective?
The text was updated successfully, but these errors were encountered: