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 factory method is ignored #660

Closed
iovesnov opened this issue Dec 20, 2014 · 3 comments
Closed

JsonCreator factory method is ignored #660

iovesnov opened this issue Dec 20, 2014 · 3 comments
Milestone

Comments

@iovesnov
Copy link

I am using Jackson version 2.4.4

Below two classes Test1 and Test2. Both classes has private constructor and JsonCreator method which accepts the single parameter. In case of Test1 class that parameter has 'double' type and Object in Test2 class.
I've added the code which converts JSON string into the Test1 and Test2 classes and as the result different methods are invoked. For the Test1 class invoked only constructor but for the Test2 factory method is invoked.

Code:

public static void main(String []args) throws IOException {
    ObjectMapper objectMapper = new ObjectMapper();
    String json = objectMapper.writeValueAsString(-0.5);
    objectMapper.readValue(json, Test1.class);
    System.out.println("-----------------");
    objectMapper.readValue(json, Test2.class);
}

class Test1 {
    private final double n;

    private Test1(double n) {
        this.n = n;
    }

    @JsonCreator
    public static Test1 factory(double n) {
        return new Test1(n);
    }
}

class Test2 {
    private final Data n;

    private Test2(Data n) {
        this.n = n;
    }

    @JsonCreator
    public static Test2 checked(Data n) {
        return new Test2(n);
    }
}

Output:

Test 1. Constructor.
-----------------
Test 2. JsonCreator.
Test 2. Constructor.

I debugged the Jackson a little bit and found that different types are treated differently: BasicDeserializerFactory#_handleSingleArgumentConstructor:

    if (type == String.class) {
        if (isCreator || isVisible) {
            creators.addStringCreator(ctor);
        }
        return true;
    }
    if (type == int.class || type == Integer.class) {
        if (isCreator || isVisible) {
            creators.addIntCreator(ctor);
        }
        return true;
    }
    if (type == long.class || type == Long.class) {
        if (isCreator || isVisible) {
            creators.addLongCreator(ctor);
        }
        return true;
    }
    if (type == double.class || type == Double.class) {
        if (isCreator || isVisible) {
            creators.addDoubleCreator(ctor);
        }
        return true;
    }
    if (type == boolean.class || type == Boolean.class) {
        if (isCreator || isVisible) {
            creators.addBooleanCreator(ctor);
        }
        return true;
    }
    // Delegating Creator ok iff it has @JsonCreator (etc)
    if (isCreator) {
        creators.addDelegatingCreator(ctor, null);
        return true;
    }
    return false;

As you can see all Java types has a check isCreator || isVisible but not for the Object or custom class. This is really makes the difference because of the constructor with some Java types (like double or String) will override the factory method...

I personally think that Jackson should work the same independently to the object type.
It is you call but I would also say that factory method annotated with JsonCreator annotation should has higher priority then constructor.

@cowtowncoder
Copy link
Member

This behavior is intentional, in that unless type is one of small-set of well-known types -- int, long, String, double, boolean -- annotation has to be used. So it is not a flaw or something to be changed, but rather established convention at this point (although could use more documentation).
Big part of the reason is that applicability between scalar types and objects is quite different: there is not much confusion for int values, since they only match JSON numbers. But Objects would match JSON Objects, and would then prevent use of regular property-based deserialization. As such scalar types are auto-detected, Object- and Array-types not.

As to precedence between factory method, constructor, that is a judgment call. But at this point it really can not be changed without some explicit configuration setting -- maybe adding a MapperFeature would make sense? -- since that would change behavior in backwards-incompatible way, and possibly break existing usage.

I'll have to check what precedence is; and based on that I could add a config feature.

@iovesnov
Copy link
Author

I was looking for the serialization features that would define the priority for the factory method or constructor. I think this is a good way to solve this problem without affecting already implemented projects.
You mentioned that for the well-known types annotation should be used. What annotation do you mean? In the example that I provided JsonCreator annotation will be ignored for all well-known types and constructor will be used.

@cowtowncoder
Copy link
Member

@iovesnov Oh. Actually, looking at the example again, you are right in that explicit @JsonCreator for static factory method SHOULD have precedence over non-annotated constructor. Somehow I missed that important part. This needs to be fixed.

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

2 participants