Skip to content
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

Provide comparator for TreeMap deserialisation #2162

Open
jwgmeligmeyling opened this issue Oct 23, 2018 · 5 comments
Open

Provide comparator for TreeMap deserialisation #2162

jwgmeligmeyling opened this issue Oct 23, 2018 · 5 comments

Comments

@jwgmeligmeyling
Copy link

Issue replaces FasterXML/jackson-core#489, as the issue actually belonged here

Both SortedMap and TreeMap can be deserialised, but by default use no special comparator is provided (i.e. natural order is used). This is because the default constructor is called for instantiating the TreeMap. Now you could write special deserializers or subclass TreeMap and provide the comparator there, but that seems to be a pretty heavy change for something as easy as providing a constructor parameter for the value instantiator. Am I overseeing a certain way of how this can be achieved? And if not, could we consider introducing new annotations that allow users to specify which comparator to use for instantiated SortedSets, SortedMaps, etc?

Based on my stackoverflow question: https://stackoverflow.com/questions/48151413/deserialize-a-sortedset-with-a-provided-comparator

Original response from @cowtowncoder

At the moment I don't think there is a way to achieve that for types that are not Comparable naturally (like Strings).

I am not sure what'd be the best way; while allowing annotations is a possibility this seems like niche case whereas ideally I think annotations would have wider applicability. So perhaps another configuration mechanism should be used. 2.10 introduces bit more flexible construction method for ObjectMappers.
That would give mechanism for global registration.

But regardless, ability to configure this does sound like a good idea.

One minor thing: this actually belongs to jackson-databind (jackson-core only covers streaming parser/generator, not databinding). Could you re-create it there please? (I don't know of github functionality that allows me to do that unfortunately).

@jwgmeligmeyling
Copy link
Author

jwgmeligmeyling commented Oct 23, 2018

I am not entirely sure whether a global configuration will be sufficient. I think the natural order for Comparable objects is usually actually a good standard. One will only ever need this for TreeMaps of objects without a natural order. I do not believe that users that have this exception, will have it for all uses of a sorted map throughout their code base.

I personally think that this can be achieved through two means:

  1. The introduction of a @SortComparator(value = Comparator.class) annotation, which will be used as Comparator parameter when constructing TreeSet or TreeMap instances in the default value instantiator.
  2. The ability to define the way how a value is instantiated on a per field basis (i.e. allow @JsonValueInstantiator annotations to be defined on a field level?)

Are there any other means of implementation that I am missing?

@jwgmeligmeyling
Copy link
Author

@cowtowncoder Could you have a short look into my proposed solutions. If there is a clear direction you'd want to see this go, I'd certainly be willing to contribute it!

plokhotnyuk added a commit to plokhotnyuk/jsoniter-scala that referenced this issue Nov 2, 2018
@cowtowncoder
Copy link
Member

Apologies for missing update here.

I can see why addition of an annotation would be the right way to go here, but I do not like the idea of adding a rather specific, single-use annotation -- ideally annotations would have much more general applicability.
Partly this is because adding support for (new) annotations is non-trivial amount of work, exposing via AnnotationIntrospector (although no work needed for mix-ins which is general mechanism) and so on.

But at the same time, I can't think of superior alternatives.

There is one alternative, however, that might work:

HandlerInstantiator

which is used as an extension point to allow custom construction of various handlers.
So I think I'd probably suggest investigating this as first pass.

Oh. And one other possibility, for annotation-based approach, would actually be to add a new property in JsonDeserialize. Doing this would still require addition of a method in AnnotationIntrospector, but I would prefer it nonetheless for keeping number of annotations more limited -- and this would also sort of fit with existing API.

I hope this helps.

@cowtowncoder cowtowncoder added 2.12 and removed 2.10 labels Mar 26, 2020
@cowtowncoder cowtowncoder removed the 2.12 label May 6, 2020
@mjustin
Copy link

mjustin commented Jul 19, 2023

The same should be done for SortedSet.

@cowtowncoder
Copy link
Member

Note: #4773 improves handling in some ways to work around the issue here; but does not allow what is requested here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants