Adding a Case Insensitive DeserializationFeature

[mressler]

Added case insensitivity as a DeserializationFeature. Will close #566 in jackson-databind.

[cowtowncoder]

Ok, one question here: would this just basically lower-case expected names, but not change actual incoming names -- that is, work for case where incoming JSON names are all-lowercase (possibly by expecting or configuring JsonParser to lower-case names to expose)?

Also: one practical note on features, something I forgot to mention earlier: all DeserializationFeatures must be dynamically changeable, so this feature actually would need to go in MapperFeature (which can not be changed after initial configuration), along with a few existing features. This despite the fact that it only applies to deserialization side.

[mressler]

It leaves all names unchanged. The only change is in the bucketing key in BeanPropertyMap - so all values external to BeanPropertyMap remain the same. It really only affects the find method.

How would you like me to update this pull request with the change to make it a MapperFeature?

[cowtowncoder]

Oh yes. I did actually miss the specific line where incoming key is lower-cased. My bad.

[cowtowncoder]

I think you can simply modify your forked branch, commit, push, and it should update.
So just move the feature in MapperFeature; access should be similar to that of DeserializationFeature, via provider?

[mressler]

Seems to be the case, I'll do that shortly. Thanks for the pointers!

[cowtowncoder]

Thank you in advance for this contribution!

The next practical thing (the first and last part of The Process) is that we need a Contributor License Agreement (CLA):

https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf

filled, signed, scanned and sent to info at fasterxml dot com. We need it filed once for any and all contributions, and it's needed to confirm that we can distribute code jars and such. Apologies for inconvenience.

Besides this; one thing I was wondering was whether it might make sense to allow per-type (Class) annotation to note that handling of a specific type should use case-sensitive/case-insensitive handling.
Or, if there is a simple interface/class that could specify transformation to use (something from JDK ideally), could even allow specifying name converter to use. This would be the ultimate method of dynamically unifying names -- may sound like an overkill, but if it's really just a String-to-String converter, supporting that would not be much more work than on/off feature. There are a few annotation-added handlers, and I can help with plugging such thing in.

This would be in addition to global setting which we could implement using same mechanism, and only overridden by annotation on per-class basis.

[mressler]

I definitely wanted to get to an annotation based approach for this. The primary problem with the annotation approach as I saw it was that the BeanPropertyMap was being built in the build() method which didn't have easy access to any annotation or configuration context. The comment on line 339 effectively says the same thing.

[mressler]

Although the resulting JsonDeserializer will eventually have its deserialize method called with the DeserializationContext - which would allow us to override the case insensitivity settings as needed.

[cowtowncoder]

Ok, right. It is true that necessary contextual information will not yet be available at time of construction.

So this would have to come via createContextual method of BeanDeserializer, and call a method in BeanPropertyMap to just take properties, rename, and construct a new instance.
Calling this from actual deserialize method is possible but little bit cumbersome, compared to doing it from contextualization.

On plus side, this would allow per-property case-insensitivity, not just per-class. Not sure this matters a lot, but it may help in cases where external types are referenced.

[cowtowncoder]

Looks like we could use Converter<String,String> as the thing to define for defining conversions.

As to annotation to use; while @JsonDeserialize could do, it is bit heavy on properties already.
And it is not easy to handle inheritance (i.e. adding one to class will override anything supertypes might use).
So another perhaps better fit could be @JsonNaming; we could add a new property ("dynamic"?), with default of 'none'. Or, if we loosened type limitation on value, could just dynamically detect what is given: if class extends PropertyNamingStrategy, do old static change; if Converter, use dynamic conversion.
Or if it all sounds bit too complex (too much auto-magic), could also just simply add a new annotation type.

Besides annotation itself, there's then need to add a method in AnnotationIntrospector to locate Converter to use (if any), for given annotated thing (property, Bean class). And finally, make BeanDeserializer.createContextual() call it, and if a Converter received, call a method ("withConverter()"?) on BeanPropertyMap, which will then construct a new instance with appropriately modified names & reference to converter to use.

[mressler]

My thought around Converter is that I don't really want the bean property names or incoming property names to be converted - I want them as is. I just want the comparison logic to accept the differences in case. Which is why I've stayed away from all manner of converters.

It feels like either a whole new annotation or add a property to @JsonNaming(caseSensitive=false) or something to that effect.

Do you agree that the names should be left as is? That approach is really what I'd like to see (and would expect) from an implementation such as this. Modifying values just seems so side-effecty. I wouldn't expect to see my beautiful property names all mangled in debug (or on serialization). Thoughts?

[mressler]

After thinking about it more, my objection to conversion stems from the fact that I really just want to change the comparison/matching behavior. The naming of the properties themselves is orthogonal to the comparison behavior and I don't want to conflate the two.

[cowtowncoder]

By converter I simply meant a mechanism for doing what pull request is already doing -- that is, converting things on-the-fly, but solely for purpose of matching. That is, it would NOT change names accessed from JsonParser, nor name that would be reported with exceptions.
However, it would change the name (similar to how pull request does, unless I am mistaken) that is contained in BeanPropertyMap. So it would only be there to generalize the work being done from "leave as-is or lower-case" into "canonicalize for purpose of matching strings that are logically but not physically equivalent".
Or did I misunderstand how pull request works?

The reason why I would prefer Converter over simple flag is that there are many other kinds of name mapping cases where existing static naming strategy is limiting: having a general on-the-fly conversion would allow things like stripping underscores or hyphens and such.

I can see why name 'converter', and use of @JsonNaming could give wrong impression here.

[cowtowncoder]

On comparison: if there was an efficient way to plug different comparison, it could be an alternative. But this is where I actually do not see a good alternative; linear search over N/2 names, then doing comparison is rather inefficient. Because of this, canonicalizing bean property names once, and canonicalizing name used for lookup seems like a reasonable way to go.
I think this is how case-insensitive matching is usually implemented; although more invasive changes where hashing is combined with canonicalizing are theoretically possible too.

[cowtowncoder]

@mressler Ok sorry for the slow follow-up here.

One thing I realized is that use of DeserializationFeature is bit problematic, conceptually: although it is something for deserialization, it can not be changed on per-call basis. Or, rather, it may well be changed (via ObjectReader), but will not take effect once deserializer has been constructed.

One way to change this would be to change it to be a MapperFeature, which contains things that can be only changed once after constructing ObjectMapper. That is probably the simplest way to go.

And I think I got the CLA. So I can go ahead, merge this, and modify as mentioned above.

[cowtowncoder]

Ah! Should have re-read my comments. You had already addressed feature part! Awesome.

Thank you again -- this is something that should be quite valuable for many users.

[mressler]

Quite welcome, thanks for the great library!
On Dec 11, 2014 10:23 PM, "Tatu Saloranta" [email protected] wrote:

Ah! Should have re-read my comments. You had already addressed feature
part! Awesome.

Thank you again -- this is something that should be quite valuable for
many users.

—
Reply to this email directly or view it on GitHub
#568 (comment)
.