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

JsonCreator on static method in Enum and Enum used as key in map fails randomly #2725

Closed
BigMichi1 opened this issue May 18, 2020 · 6 comments
Milestone

Comments

@BigMichi1
Copy link

given this piece of code and jackson-databind 2.10.3

import java.util.HashMap;
import java.util.Map;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonValue;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;

public class Test {
    public static void main(String[] args) throws JsonProcessingException {
        final ObjectMapper mapper = new ObjectMapper();

        final Map<TestEnum, String> input = new HashMap<>();
        input.put(TestEnum.FOO, "Hello");
        final String inputJson = mapper.writeValueAsString(input);
        System.out.println("inputJson = " + inputJson);

        final Map<TestEnum, String> output = mapper.readValue(inputJson, new TypeReference<Map<TestEnum, String>>() {
        });
        System.out.println("output = " + output);
    }

    private enum TestEnum {
        FOO(1);

        private final int i;

        TestEnum(final int i) {
            this.i = i;
        }

        @JsonValue
        public int getI() {
            return i;
        }

        @JsonCreator
        public static TestEnum getByIntegerId(final Integer id) {
            return id == FOO.i ? FOO : null;
        }

        @JsonCreator
        public static TestEnum getByStringId(final String id) {
            return Integer.parseInt(id) == FOO.i ? FOO : null;
        }
    }
}

and running it on multiple computers the output is different.
sometimes it works and the map is deserialized correctly and sometimes it does not work and fails with Exception in thread "main" java.lang.IllegalArgumentException: Parameter #0 type for factory method ([method foo.Test$TestEnum#getByIntegerId(1 params)]) not suitable, must be java.lang.String

now when I change the order of the methods annotated with JsonCreator it works, but then it started to fail on other computers with the same exception as above.

I tried to debug this and found out that sometimes the order of the annotated methods is different that is iterated over in BasicDeserializerFactory#_createEnumKeyDeserializer when taking a look at the beanDesc.getFactoryMethods() methods. These list of methods as far as I could see is created in AnnotatedCreatorCollector#_findPotentialFactories via ClassUtil.getClassMethods which then calls ClassUtil.getDeclaredMethods(cls) and this is using Class#getDeclaredMethods which as written in the JavaDoc that the methods elements in the returned array are not sorted and are not in any particular order. So this is now probably the root of the issues which I can see. And as the loop stops with the exception after the first candidate has been checked in BasicDeserializerFactory the remaining methods are not taken into consideration.

From my perspective, all annotated methods need to be checked before the exception can be thrown or at least the annotation parser should fail when multiple creators are detected.

@tomas-skalicky
Copy link

I would prefer the first proposal: going through all found static @JsonCreator methods. There are use cases when you need more creators.

@cowtowncoder
Copy link
Member

cowtowncoder commented May 19, 2020

First things first: I agree that whether this works or not should be deterministic.
I have some ideas on other aspects, but this is the first thing to resolve.

One suggestion: since 2.11.0 was just released, may make sense to see if that has improved handling. I think I very recently noticed a missing validation in CreatorCollector.

@BigMichi1
Copy link
Author

I upgraded to 2.11.0 but the behaviour is still the same, sometimes it works, sometimes it fails

@cowtowncoder
Copy link
Member

Thank you for checking. I was about to ask if this only concerns Enum-as-Map-key case, but reading through desc, I think this is true, and "enum as value" case does not have same issue.

@cowtowncoder
Copy link
Member

@tomas-skalicky I think one challenge here is that whereas for more general Enum (de)serialization, where multiple shapes can be usable (integer and String values at least), case of Enum as Map key is bit more strict as JSON can only provide String.
But code in this case tries to share some of former for latter; and annotation to use @JsonCreator makes no distinction. So some specific logic is needed to kind of cover the case of ignoring int/Integer creator. I haven't looked at code but I suspect this is doable.

cowtowncoder added a commit that referenced this issue May 19, 2020
@cowtowncoder
Copy link
Member

I can reproduce this locally, added a failing test. Seems easy enough to change not to fail on first ineligible @JsonCreator in this particular case -- and reason is specifically that the other creator is valid when deserializing Enum values, just not as Map key.

This is bit messy area, as division between "values" and "Map keys" is not well modeled by Jackson; but it stems from JSON only having Strings as key values, and corresponding need to limit value set, as well as requirements for actual (de)serializers (as Map keys are exposed as FIELD_NAMEs, not VALUE_STRINGs).

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