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

Deserialize Enums using ordinal does not work for map keys #1877

Closed
mawifu opened this issue Jan 9, 2018 · 4 comments
Closed

Deserialize Enums using ordinal does not work for map keys #1877

mawifu opened this issue Jan 9, 2018 · 4 comments

Comments

@mawifu
Copy link

mawifu commented Jan 9, 2018

We are using java enums to communicate between java and c. So we decided to serialize enums using the java ordinal. The SerializationFeature.WRITE_ENUMS_USING_INDEX works fine for that. Thank you for that feature!
By the way, we are using java 1.8.0_151 and jackson-databind 2.9.2

Concerning deserialization we hit an unbalance concerning enums as map key:
Within the serialization step the enum is written as ordinal.
Within the deserialization the interpretation of the ordinal as enum failes only for map keys.
For common properties the deserialization from ordinal number works fine.

The following sample code describes this more precise:

package jackson_enum_as_key;

import com.fasterxml.jackson.databind.*;

import java.io.IOException;
import java.util.*;

public class Main {

    public enum Type {
        ANY,
        OTHER
    }

    public static class Container {
        private Type simpleType;
        private Map<Type, String> map;

        public Type getSimpleType() {
            return simpleType;
        }

        public void setSimpleType(Type simpleType) {
            this.simpleType= simpleType;
        }

        public Map<Type, String> getMap() {
            return map;
        }

        public void setMap(Map<Type, String> map) {
            this.map = map;
        }
    }

    public static void main(String[] args) throws IOException {
        ObjectMapper objectMapper = new ObjectMapper();
        objectMapper.configure(SerializationFeature.WRITE_ENUMS_USING_INDEX, true);

        Map<Type, String> map = new HashMap<>();
        map.put(Type.OTHER, "hello world");
        Container container = new Container();
        container.setSimpleType(Type.ANY);
        container.setMap(map);

        String json = objectMapper.writeValueAsString(container);

        System.out.println(json);

        objectMapper.readValue(json, Container.class);
    }
}

This code produces the following output. Serialization works fine, deserialization fails on the map key:

{"simpleType":0,"map":{"1":"hello world"}}
Exception in thread "main" com.fasterxml.jackson.databind.exc.InvalidFormatException: Cannot deserialize Map key of type `jackson_enum_as_key.Main$Type` from String "1": not a valid representation, problem: (com.fasterxml.jackson.databind.exc.InvalidFormatException) Cannot deserialize Map key of type `jackson_enum_as_key.Main$Type` from String "1": not one of values excepted for Enum class: [OTHER, ANY]
 at [Source: (String)"{"simpleType":0,"map":{"1":"hello world"}}"; line: 1, column: 24]
 at [Source: (String)"{"simpleType":0,"map":{"1":"hello world"}}"; line: 1, column: 24] (through reference chain: jackson_enum_as_key.Main$Container["map"])
	at com.fasterxml.jackson.databind.exc.InvalidFormatException.from(InvalidFormatException.java:67)
	at com.fasterxml.jackson.databind.DeserializationContext.weirdKeyException(DeserializationContext.java:1527)
	at com.fasterxml.jackson.databind.DeserializationContext.handleWeirdKey(DeserializationContext.java:866)
	at com.fasterxml.jackson.databind.deser.std.StdKeyDeserializer.deserializeKey(StdKeyDeserializer.java:133)
	at com.fasterxml.jackson.databind.deser.std.MapDeserializer._readAndBind(MapDeserializer.java:449)
	at com.fasterxml.jackson.databind.deser.std.MapDeserializer.deserialize(MapDeserializer.java:367)
	at com.fasterxml.jackson.databind.deser.std.MapDeserializer.deserialize(MapDeserializer.java:29)
	at com.fasterxml.jackson.databind.deser.impl.MethodProperty.deserializeAndSet(MethodProperty.java:127)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.vanillaDeserialize(BeanDeserializer.java:287)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:151)
	at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4001)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:2992)
	at jackson_enum_as_key.Main.main(Main.java:50)

Process finished with exit code 1

Running the code above without SerializationFeature.WRITE_ENUMS_USING_INDEX set to true, everything works fine with following output:
{"simpleType":"ANY","map":{"OTHER":"hello world"}}

So deserialization fails only for enums repesented as ordinal used as map keys!

Regarding the code in jackson-databind we found an unbalance in the Deserialization-Classes:
The usual com.fasterxml.jackson.databind.deser.std.EnumDeserializer used for simple properties checks for JsonToken#VALUE_NUMBER_INT and has code path for enums as ordinal. Source code is commented by But let's consider int acceptable as well (if within ordinal range)

Unlike the com.fasterxml.jackson.databind.deser.std.StdKeyDeserializer.EnumKD used for map keys uses only a _byNameResolver and has no code path for resolving by ordinal. This explains why resolving by ordinal does not work for map keys.

This unbalanced situation seems like a missing feature or bug.

Our workaround: We use a custom deserializer for our enum.

So in com.fasterxml.jackson.databind.deser.BasicDeserializerFactory#_createEnumKeyDeserializer for the map key a different code path is used, triggering a com.fasterxml.jackson.databind.deser.std.StdKeyDeserializer.DelegatingKD which delegates to our custom deserializer which is able to resolves enum by ordinal.

Not a very elegant workaround, because we have to register a concrete deserializer only for those enums which are used as map keys. Enums only used as normal properties are fine with the standard EnumDeserializer.

public class EnumKeyDeserializer<E extends Enum<E>> extends StdDeserializer<E> {

    private final Class<E> enumType;

    public EnumKeyDeserializer(Class<E> enumType) {
        super(enumType);
        this.enumType = enumType;
    }

    @Override
    public E deserialize(JsonParser p, DeserializationContext ctxt) throws IOException {

        String str = p.getText().trim();

        try {
            int index = Integer.parseInt(str);

            final E[] enumValues = enumType.getEnumConstants();
            if (index >= 0 && index < enumValues.length) {
                return enumValues[index];
            } else {
                throw ctxt.weirdNumberException(index, enumType,
                        "index value outside legal index range [0.." + (enumValues.length - 1) + "]"
                );
            }

        } catch (NumberFormatException nfe) {
            return Enum.valueOf(enumType, str);
        }
    }
}

The most relevant issue we found here is #1674 - this issue mentions the unbalance between deserializer for normal properties versus deserializier for map keys. But this issue does not mention the enum ordinal in any way. May be relating to #1859 but this issue regards unknown enum values, not ordinals.

Regarding issue #1882 I tried a EnumMap instead of a HashMap. But this didn't change anything. Same Exception.

@renzihui
Copy link

After upgrade from 2.7 to 2.9, our app is broken due to this issue too. The change seems introduced in #1570
When SerializationFeature.WRITE_ENUMS_USING_INDEX is enabled
ObjectMapper serializes Map<Enum, ?> to a format itself is unable to de-serialize.

BTW, since json's key is always string type, IMO it is uncommon to write enum to a string of number, it would be better using a different flag, e.g. SerializationFeature.WRITE_MAP_KEY_ENUMS_USING_INDEX, so user could opt-in(or out)

@cowtowncoder
Copy link
Member

@renzihui Just to make sure: if the behavior that broke is different from this issue, please file a new one specifically for that. Similarly, idea of separate serialization feature (which I think makes sense) is better filed as a new issue. That way I can address specific original problem here, and then consider addition separately -- this is necessary because new feature enums can only be introduced in new minor version, whereas I can fix bugs in a patch release.

Also, note that there is DeserializationFeature.FAIL_ON_NUMBERS_FOR_ENUMS which is somewhat relevant here.

@renzihui
Copy link

renzihui commented Sep 6, 2018

@cowtowncoder
Thanks for your response, I agree. Though I was talking the same error in this issue, I was proposing a different solution(from our perspective), new issue filed
#2129

@cowtowncoder
Copy link
Member

Closing this in favor of #2129.

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