Deserializing Map with enum keys results in runtime string keys

[ivangreene]

trafficstars

Attempting to deserialize json into a Map with enum keys will result in the instance having String keys at runtime. Using an enum as the value side of a Map works as expected. We should support Map keys with natural string representations in JSON (immediately, enums and UUIDs come to mind, although I'm sure some other types would benefit from this). This is somewhat related to #110, #177 and #253, although those issues have slightly ambiguous behavior since there is no natural representation, other than the JSON-in-JSON string as the key

A simple example:

public class EnumKeyTest {

    public static class Container {
        public Map<Letter, Integer> letterToOrdinal;
        public Map<String, Letter> nameToLetter;
    }

    @Test
    public void shouldHaveEnumKeys() {
        Jsonb jsonb = JsonbBuilder.create();
        Container container = jsonb.fromJson("{\"letterToOrdinal\":{\"B\": 1}, \"nameToLetter\":{\"a\":\"A\"}}", Container.class);
        assert container.nameToLetter.values().iterator().next() instanceof Letter : "value is Letter";
        assert container.nameToLetter.values().iterator().next() == Letter.A : "value == A";
        assert container.letterToOrdinal.keySet().iterator().next() instanceof Letter : "key is letter";
        assert container.letterToOrdinal.keySet().iterator().next() == Letter.B : "key == B";
    }

    enum Letter {
        A, B, C
    }
}

This test will fail on key is letter since the key value is actually a String.

[derekm]

The JAX-RS spec calls for supporting these stringly-typed types:

https://github.com/eclipse-ee4j/jersey/blob/eafb9bdcb82dfa3fd76dd957d307b99d4a22c87f/docs/src/main/docbook/jaxrs-resources.xml#L363

[aguibert]

related to #294

[aguibert]

thanks for raising this issue @ivangreene and for including a test case to reproduce the issue!

I've created a PR that pulls in this test case and gets the Enum key scenario working

[rmartinc]

Hi!

We have issues reported about this. And #347 doesn't fix the problem completely. I'm going to attach a simple maven project reproducer at the end. To reproduce with current 2.0.2 version just execute mvn test with it:

unzip issue-map-enum-key.zip
cd issue-map-enum-key
mvn clean  test

The MapToObjectSerializer is used if all the keys are strings, enums or numbers here. This class transforms enums (using name) or numbers (using String.valueOf) into strings for the json key here. But the deserializer class MapDeserializer does anything to convert back those strings into enums or numbers (see here). So the deserialized map always contains String as keys.

For a enum KeyEnum {ONE, TWO} the process is the following (the final map contains string "ONE" as the key):

Map<KeyEnum,String>                Json                             Map<KeyEnum,String>
{ONE="value"}       --serialize--> {"ONE":"value"} --deserialize--> {"ONE"="value"}

The MapDeserializer says that json keys are strings by spec, so this seems to be done on purpose.

We have thought about trying to convert back the string to enums and numbers using the key type in the map. Another option is avoiding the use of the MapToObjectSerializer in favor of the MapToEntriesArraySerializer in more cases by default... But we are not sure even reading the JSR-367 spec...

WDYT? Is this a bug or an adapter is just needed for this?

Reproducer

issue-map-enum-key.zip

[don-spyker]

Hi,

I have a similar issue like @rmartinc faces, just with Integer as map key: Map<Integer, Object>.

The json structure is being deserialized, but when trying to iterate about the keyset() of the map a ClassCastException is being thrown.

I would prefer a solution that converts the key back into the original type, which is actually defined. Using the same DTO on server and client side should be the goal.

[rmartinc]

@Verdent Could you please take a look to this issue when you have time?

I have been doing some work here based on the idea of converting back to the original objects. This is my WIP branch and the interesting part is in the DeserializerMap. The idea is just trying to unmarshall the key into the specified type in the map. (There is more work done for configuring the string value used for a null key, but that's less important.) WDYT? Would it be a worthy PR? Or do you prefer another approach?

[Verdent]

@rmartinc @don-spyker Hi guys, I will take a look at it next week. When I read more about the issue, I will provide some feedback to it :-)

[Verdent]

@rmartinc I think this is something we should support. Since we do support transforming other Map key types to String, it should be possible to do it also the other way around. I might ask you to create PR, when this feature is ready and we can go through it there :-) Or if you have any questions about the issue, feel free to contact me or write here.

[rmartinc]

Thanks @Verdent! I have submitted PR #509 with an initial and tentative fix. There are several options to fix this but we should start with something. Just comment me there if you prefer something different.

[Sboddd]

I ran into this same issue, but with a map that had Integer keys - the deserialized Map had Strings for its keys.

[reinhapa]

We got the same problems here and had to implement a workaround. @rmartinc anything we can help to get this forward?

[rmartinc]

@reinhapa The status right now is that there is a PR and it's being reviewed. The first review detected three changes, two are clear to me but in the third point we are still deciding what to do.

@Verdent What do you think about my last proposal for the null key? The more I think about it the more sense it makes to me. I think that a map with a null key should not be serialized with a MapToObjectSerializer (JSON does not accept a null key in a object) but with the MapToEntriesArraySerializer (it would be something like [{"key":null,"value":...},...]). It avoids the property and I think it has no risks, because null keys are not common in java and now a null key is also broken (in MapToObjectSerializer the null key is de-serialized to a string "null" key). I can re-submit the PR branch with the three changes if you agree with this point.

Thanks!

[Verdent]

@rmartinc I will add comment to the PR to not split the conversation :-)

[reinhapa]

... I think that a map with a null key should not be serialized with a MapToObjectSerializer (JSON does not accept a null key in a object) but with the MapToEntriesArraySerializer

@rmartinc The concept of null keys in a Map is a bad practice anyway, as the consumer side can not tell the difference if there is an entry value where the key and value was null or if there was no entry with a null key.

[rmartinc]

@reinhapa Yes, that's why I commented that it's not common (and more important broken right now). But we have reached an agreement, we do what I proposed but adding a yasson property to use the MapToObjectSerializer back in case of null keys (just in case someone is using null keys knowing that null is converted to the "null" string).

I expect to submit a new PR on Monday.

[rmartinc]

@Verdent PR also sent to the 1.x branch but without the property, just deserializing Numbers and Enums, it's #522.