Allow more control over default creator property values #1224
Conversation
There are no javadocs or tests yet; this is to set up a pull request for discussion. This should be a backwards-compatible change. A new method is being added to ValueInstantiator, but by default it will call an old method and have exactly the same old behavior. The behavior only changes if and when the application decides to override this new method in their own ValueInstantiator. (I suppose that if someone out there already made their own ValueInstantiator subclass with this exact signature, this change would not be backwards-compatible for them. But that seems like a very unlikely scenario.) The hasParameter and getParameter methods of PropertyValueBuffer are not used by jackson-databind itself. Those are the methods that we expect applications to use if they want finer-grained control over default values. Instead of eagerly fetching every default value (and possibly throwing an exception for missing-but-required parameters), applications can use hasParameter to check if each property is present, and then use getParameter to fetch the known-present values one by one. One weird thing about this change is that it exposes a class in the deser.impl package (PropertyValueBuffer) in the API of the root deser package (ValueInstantiator). That seems "wrong" given the names of the packages, but maybe no one cares? We could make PropertyValueBuffer implement an interface that exposes hasParameter, getParameter, and getParameters, then use that interface in ValueInstantiator instead... I suspect that no one would ever implement that interface except PropertyValueBuffer, making the interface useless. The end goal of this is to let Kotlin applications use that language's support for default parameter values. The same mechanism might work for other JVM-based languages with default values, but I have only proved it out with a Kotlin application. Related discussion: FasterXML/jackson-module-kotlin#26
|
@michaelhixson Ok. I think that giving more responsibility for |
|
@cowtowncoder Thanks for the review. I'll get to work on the javadocs and tests then. Will ping you when that's in place. |
michaelhixson
changed the title
|
@cowtowncoder I added some javadoc and tests. Mind taking a look? If this gets merged then I will start working on the PR for the Kotlin module. |
|
Why is the test casting to |
|
They cast to |
|
Yes, I saw then when updating Jackson-Kotlin module, so have it check that it is actually that type before casting, then raising exception if something unexpected happens in future implementation with what type is received. |
|
@michaelhixson can this backport to 2.7.x? |
|
@apatrida I think so. |
|
I don't think that can be done as this opens up new API to call. Such changes should be reserved to new minor versions. |
Don't merge this (yet)!This is following up on a previous discussion in jackson-module-kotlin: FasterXML/jackson-module-kotlin#26
I haven't bothered with javadoc and tests yet because I'm wondering if this is a good approach in general. It's the best solution I could come up with, and it does solve the original problem I was having, but who knows what problems a Jackson expert might spot. Should I keep moving forward with this approach?
Update: This PR is now complete from my perspective. Please review and/or merge when you like.