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

Support @JsonCreator annotation on record classes #3180

Closed
markelliot opened this issue Jun 21, 2021 · 7 comments · Fixed by #3724
Closed

Support @JsonCreator annotation on record classes #3180

markelliot opened this issue Jun 21, 2021 · 7 comments · Fixed by #3724
Labels
Record Issue related to JDK17 java.lang.Record support
Milestone

Comments

@markelliot
Copy link

Using Jackson 2.12.

One common pattern in code on my teams is to use 'alias' types to decorate otherwise primitive value types in APIs and/or for dimensional units to gain compiler support for detecting otherwise subtle programming errors (such as using the wrong unit or passing parameters in the wrong order).

Today, to do this with Java records, we need to write classes like

public record Watts(@JsonValue double value) {
    @JsonCreator
    public static Watts of(double value) {
        return new Watts(value);
    }
}

Without the @JsonCreator annotated static method (or similarly trivial but verbose constructor), Jackson produces the error

com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot construct instance of `...Watts` 
  (although at least one Creator exists): no double/Double-argument constructor/factory method to 
  deserialize from Number value (<value>)

If we could place @JsonCreator on record classes and infer use of the single argument default constructor I think it'd cut the boilerplate even more. Alternatively, or in addition, it might be interesting to infer that a record class with a single labeled @JsonValue parameter should use the default constructor, allowing us to write either:

@JsonCreator
public record Watts(@JsonValue int value) {}

or

public record Watts(@JsonValue int value) {}
@cowtowncoder cowtowncoder added the Record Issue related to JDK17 java.lang.Record support label Jun 26, 2021
@yihtserns
Copy link
Contributor

yihtserns commented Nov 7, 2022

@markelliot You can do this:

public record Watts(@JsonValue double value) {
    @JsonCreator(mode = JsonCreator.Mode.DELEGATING)
    public Watts {}
}

BTW, how can this even work in 2.12.0:

public record Watts(@JsonValue double value) {
    @JsonCreator
    public static Watts of(double value) {
        return new Watts(value);
    }
}

Using that, I'm getting:

com.fasterxml.jackson.databind.JsonMappingException: Problem with definition of [AnnotedClass .Watts]: Multiple 'as-value' properties defined ([field .Watts#value] vs [method .Watts#value()])

For it to work, I need to move the @JsonValue annotation to the overridden accessor like this:

public record Watts(double value) {
    @JsonCreator
    public static Watts of(double value) {
        return new Watts(value);
    }

    @JsonValue // HERE
    @Override
    public double value() {
        return value;
    }
}

UPDATE: Apparently this one is a known issue: #3063d.

@markelliot
Copy link
Author

I think the specific minor version for my example was 2.12.2, though I'm unsure if the minor version made a difference in behavior.

My example was extracted from:
https://github.com/markelliot/allezgo/blob/0.1.0/allezgo-service/src/main/java/io/allezgo/units/Watts.java

Where the version of Jackson is specified in:
https://github.com/markelliot/allezgo/blob/0.1.0/versions.props#L3

@yihtserns
Copy link
Contributor

yihtserns commented Nov 7, 2022

@markelliot OK thanks I think I've satisfied my curiousity: a quick scan through the codebase seems to hint that Watts is mostly used for JSON deserialization, never for serialization so the @JsonValue wasn't used at all which is why no error was experienced.

BTW does the solution I suggested in the previous comment satisfies this request, i.e.:

public record Watts(@JsonValue double value) {
    @JsonCreator(mode = JsonCreator.Mode.DELEGATING)
    public Watts {}
}

But after looking at Watts.java, maybe you'd want this instead to replace the two factory methods:

public record Watts(@JsonValue double value) {
    @JsonCreator(mode = JsonCreator.Mode.DELEGATING)
    public Watts(Number value) {
        this(value.doubleValue());
    }
}

@markelliot
Copy link
Author

Thanks -- I'll take your advice on reducing the total boilerplate.

BTW does the solution I suggested in the previous comment satisfies this request

I think it'd be more ergonomic for Jackson to have better default handling of record classes, such as functioning with sane defaults using minimal (or no) annotations:

public record Watts(@JsonValue int value) {}

@cowtowncoder
Copy link
Member

@markelliot Took me a while to understand intent here but I guess I can see it. So the assumption is that use of @JsonValue would imply expectation to use DELEGATING mode for 1-argument constructed (1-property Record).

This may be tricky to pull off, but would also, I think, apply to equivalent non-record POJO case.

I will file a separate issue since it's both more specific and more general than this issue.

@yihtserns
Copy link
Contributor

yihtserns commented Jan 16, 2023

public record Watts(@JsonValue int value) {} will work starting from 2.15.x (auto detects 1-arg constructor + @JsonValue as mode=DELEGATING creator, thanks to #3724 + #3654), but it won't fully satisfy your need because from the given example, I see that you need to support both int & double value.

So starting from 2.15.x, if you don't want to use @JsonCreator you have these options to choose from:

Option 1

public record Watts(@JsonValue double value) {
    public Watts(int value) {
        this((double) value);
    }
}

Option 2

public record Watts(@JsonValue double value) {

    // Method name must be "valueOf"
    public static Watts valueOf(int value) { 
        return new Watts(value);
    }
}

Observation: Parameter of type Number is accepted if a constructor/factory method is annotated with @JsonCreator, but not for implicit detector. 🤔

yihtserns added a commit to yihtserns/jackson-databind that referenced this issue Jan 17, 2023
yihtserns added a commit to yihtserns/jackson-databind that referenced this issue Jan 17, 2023
@cowtowncoder
Copy link
Member

@yihtserns Right, Number will not work just because there is a small set of "well-known" types that are supported (int, long, boolean, String, double). While it would be possible to expand that there's diminishing return for effort.
But explicitly annotated creator can use any type -- however, there can only be one such fully-delegating creator method, whereas "well-known" types can exist separate from others.

Come to think of that, the problem with Number one would be that it would overlap with all of int, long and double so logic needed to validate Creators would get a bit complicated (I'm sure it's doable with enough effort, Number being less-specific than, say, double, so would have lower precedence.
But not sure how valuable it'd be overall.

cowtowncoder pushed a commit that referenced this issue Jan 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Record Issue related to JDK17 java.lang.Record support
Projects
None yet
3 participants