Make the new jackson version ignore default values

[wix-andriusb]

Hi guys, I tried to dig through the code and the documentation, but it was too difficult for me.

Is there a way to disable default values in the new jackson version?

Is there some kind of configuration for that?

[pjfanning]

This behaviour is not currently configurable.

One option to work around this would be to create your own version of ScalaAnnotationIntrospectorModule that omits the defaultValue code.

[nick-benoit14]

@pjfanning Would you accept a PR to make this behavior configurable? In general very excited about getting the respect for default params, but am concerned for certain parts of our team's app.

The problem case for us is mostly centered around a generic framework we use to power public apis. We do not explicitly serialize nulls and it operates on many business objects scattered throughout the app. I'm a little worried about not being able to effectively audit the thousands of case classes in our app for places where a previously benign default param might change some data in an unexpected way.

The problem case I'm imagining would probably go something like this:

Case class RandomBusinessObject(k1: String, appLevelFeatureFlag: Option[String] = Some("New instances of this class should have this turned on")) 

We have some case class that is compositionally included in many other objects. A feature change is introduced (perhaps a migration of some kind), along with a default parameter that previously would only be non-null upon the apply method being called. In this example the new object can immediately uptake the new behavior.

For a legacy version of this object, we expect this param to be None, as it has never been set before. We expect new instances of this class to have the flag / use the new behavior, but for older objects we need to run a migration script or in some other way prepare for flipping this flag.

When Jackson Module Scala changes it's behavior, all of a sudden our expected, but non serialized nulls will be inadvertently overriden by the default param potentially landing us in hot water.

Like I said, in general we are very excited about respecting default params on case classes as that is definitely what a programmer would expect, but being able to slowly opt parts of our app into this behavior would make these changes far more digestible.

I think what would be ideal for our use case would be to be able to turn this setting off for the specific mapper that powers the aforementioned framework, then basically have it turned on anywhere else. I imagine there could be others in similar situations as well.

Thanks,
Nick

[pjfanning]

@nick-benoit14 PRs are always welcome - best targeted to the 2.13 branch.

Would you be able to provide some ideas about the config mechanism you intend to use? jackson-module-scala is lacking configurability and it would be good to discuss solutions.

[pjfanning]

@cowtowncoder would it be possible to add a JsonReadFeature to control if default values are applied when deserialising json?

[cowtowncoder]

A new feature is a possibility, given that this could be something usable for other modules (Kotlin, and maybe even "plain" Java one eventually).
It probably should not be one of features from jackson-core as those are related to low-level token access (StreamReadFeature for format-agnostic, JsonReadFeature for JSON-specific), and I think this is for databind.

But it could be either additional DeserializationFeature (if changeable on per-call basis via ObjectReader) or, possibly more likely?, MapperFeature for things that must be configured once before use and not changed afterwards.
(division is bit problematic wrt naming as this feature would only be for deserialization)

Downside of defining a general-purpose feature is just that it may be unclear where exactly it is applied (assuming other modules won't immediately start using it).

Kotlin module is adding support for configuration via Builder for 2.13, for what that is worth; it might be worth considering for Scala module too. But I am open to all options.

[pjfanning]

@cowtowncoder I think that the feature is useful for other modules (even if they don't implement support yet). My gut feeling is a DeserializationFeature would be the best way to go. One thing though, it might be best to make the feature something like DeserializationFeature.IgnoreDefaultValues so that by default - default values are used. Today, default values are supported by default and it would affect users if we forced them to opt in to keeping the feature enabled.

[nick-benoit14]

@pjfanning I think to the point in the code where I understand how to make my desired change, but yeah not really sure how the configuration piece should work. I think that a DeserializationFeature would make sense as well. Can you shed any light on what the process of getting a new DeserializationFeature into core jackson looks like?

[cowtowncoder]

DeserializationFeature is part of jackson-databind, and configured outside of modules. Code typically accesses it via DeserializationContext (and fundamentally DeserializationConfig).

So for testing you'd probably want to build jackson-databind 2.13.0-SNAPSHOT with local changes to add the new feature -- that part is easy (just add a new enum entry in DeserializationFeature)

[pjfanning]

@cowtowncoder I added FasterXML/jackson-databind#3181

[nick-benoit14]

Hi @pjfanning i'm having a hard time finding docs / figuring out how to build the jackson deps locally to test with my branch of jackson-scala-module. Are there any existing docs on that that can point me towards?

[cowtowncoder]

Dependency you need wrt jackson-databind would be built and installed locally with

mvn clean install

and this would produce 2.13.0-SNAPSHOT (from 2.13 branch -- master is for 3.0.0-SNAPSHOT).
Although I am not sure if sbt uses locally built snapshot and so @pjfanning may know better.

[pjfanning]

@nick-benoit14 #521 is one possible solution

[nick-benoit14]

I opened: #527

Is that kind of what you had in mind @pjfanning?

[pjfanning]

@nick-benoit14 we'll have to see if FasterXML/jackson-databind#3181 gets merged

[cowtowncoder]

Added a note there. Will merge if:

1. Setting is something that may be changed for ObjectReader between calls (i.e. user can toggle enable/disable without constructing new mapper). This is the key distinction between Deserialization- and MapperFeatures.
2. Implementation on Scala side is likely to get in 2.13 -- I am ok at this point for plan to get it done; if it is not ready in 2.13 timeline I'll revert the change (historically there has been one or two cases of settings added before being implemented and I'd like to avoid it)

[cowtowncoder]

Ah... Apologies for asking for (2), I had missed the part of this really just being changing behavior of existing functionality.
Obviously functionality is already there.

So on (1): it looks like this may indeed be changed between calls, like so:

ObjectMapper mapper =  .... ; // create Scala mapper

Value withDefaults = mapper.readerFor(Value.class)
    .with(DeserializationFeature.APPLY_DEFAULT_VALUES)
    .readValue(source);

Value noDefaults = mapper.readerFor(Value.class)
    .without(DeserializationFeature.APPLY_DEFAULT_VALUES)
    .readValue(source);

and if so DeserilalizationFeature works well.

It'd be good to have a unit test that verifies this, just in case.

[nick-benoit14]

I was working on adding a test to make sure the above example worked. It does not work right now on my branch. It seems like maybe when you do mapper.readerFor(Value.class) somewhere a ScalaValueInstantiator is cached or something and not being constructed each time readerFor is called?. Do you have any thoughts on that @pjfanning?

[pjfanning]

@nick-benoit14 did you read my comment on your PR? could be related to your test issue too

[cowtowncoder]

As long as check for setting occurs via DeserializationContext (or DeserializationConfig if passed on specific call), it should reflect the current state.

But if DeserializationConfig (or settings it has) are fetched and stored during construction/initialization of ValueInstantiator (etc) then that would not change.

[nick-benoit14]

Made changes to support (1), and I added a test for that behavior as well. The previous issue was that I was not checking the config in the null value provider, but rather in the class that created the null value provider.

@pjfanning I saw your comment / made a change to address. Thanks!

[pjfanning]

In 2.13.0-RC1, MapperFeature.APPLY_DEFAULT_VALUES will be supported - defaults to true.

    val builder = JsonMapper.builder().disable(MapperFeature.APPLY_DEFAULT_VALUES).addModule(DefaultScalaModule)
    val mapper = builder.build()

[nick-benoit14]

Thanks @pjfanning. Are we good to go ahead and close out this issue then?

[pjfanning]

thanks @nick-benoit14 - I'll close this now - the fix will be in the 2.13.0-RC1 release which should be released soon