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

Immutable object deserialization should be possible without @JsonCreator when the creator is unambiguous. #2852

Closed
lukaszpierog opened this issue Sep 15, 2020 · 12 comments
Labels
duplicate Duplicate of an existing (usually earlier) issue

Comments

@lukaszpierog
Copy link

Hello, where I work we tend to only use immutable objects with all-args constructor. In order to deserialize such object, we must either add annotations like @JsonCreator to the actual class or when it comes from a library, we must create mix-ins.

Consider the following example class:

public final class Example {
  private final String field;
  private final String text;

  @JsonCreator
  public Example(@JsonProperty("field") String field,  @JsonProperty("text")String text) {
    this.field = field;
    this.text = text;
  }

  public String getField() {
    return field;
  }

  public String getText() {
    return text;
  }
}

Such example will deserialize nicely, but it would be great if there was no need to add anything to the class itself, because:

  • when creating domain classes in some low level module we wouldn't want jackson dependencies yet
  • when using external library its not possible to edit class
  • above can be worked around with mix-ins, but this creates a lot of non-testable and probably not necessary code

Actually there already seems to be a deserialization feature that gets me halfway where I want and we can skip the @JsonProperty annotations by configuring ObjectMapper:

new ObjectMapper()
        .registerModule(new ParameterNamesModule(JsonCreator.Mode.PROPERTIES))

However if the @JsonCreator annotation is removed, it will fail, even though there is only a single constructor. I would assume it should be given a try if there is no default nor @JsonCreator annotated method.

In my opinion such scenario should be possible to handle without any additions to the class itself. If not by default, then by configuring a proper deserialization features on ObjectMapper.

@lukaszpierog lukaszpierog added the to-evaluate Issue that has been received but not yet evaluated label Sep 15, 2020
@cowtowncoder
Copy link
Member

I am currently working on #1498, for inclusion in 2.12, and it might do what you want.

Note that one way you can probably implement this at this point for your own use case is by sub-classing JacksonAnnotationIntrospector and override handling of @JsonCreator to indicate that annotation was found based on whatever criteria you want. It is some work but may be worth it for larger number of types.

@lukaszpierog
Copy link
Author

@cowtowncoder
Hi, I can see that the mentioned issue is now closed. Could you please confirm that changes applied there can enable the deserialization I've mentioned in this issue and will be available since version 2.12? Is there a test case for such scenario? Or some code snippet with proposed ObjectMapper configuration etc?

@cowtowncoder
Copy link
Member

Yes, so, #1498 tests are under:

src/test/java/com/fasterxml/jackson/databind/deser/creators/ConstructorDetector1498Test.java

and configuration setting to look for is ConstructorDetector.PROPERTIES. That should actually only matter for 1-argument case (2-argument case should already have worked, iff parameter names are detected) but worth enabling to avoid confusion for that case.

Setting is done using something like:

ObjectMapper mapper = JsonMapper.builder()
    .setConstructorDetector(ConstructorDetector.PROPERTIES)
    .build();

If you can verify this locally with 2.12.0-SNAPSHOT that'd be good; if it does not I would like to see specific test case.

Closing this for now assuming implementation works.

@cowtowncoder cowtowncoder added duplicate Duplicate of an existing (usually earlier) issue and removed to-evaluate Issue that has been received but not yet evaluated labels Oct 9, 2020
@lukaszpierog
Copy link
Author

Hi @cowtowncoder
Do I assume correctly, that I should use:

        ObjectMapper mapper = JsonMapper.builder()
                .constructorDetector(USE_PROPERTIES_BASED)
                .build();

? PROPERTIES seems to be SingleArgConstructor type, not ConstructorDetector

I am using version 2.12.0-rc1 released yesterday in my project, and I have added a following:

public final class Example {
    private final String field;
    private final String text;

    public Example(String field, String text) {
        this.field = field;
        this.text = text;
    }

    public String getField() {
        return field;
    }

    public String getText() {
        return text;
    }

    public static void main(String[] args) throws Exception {
        ObjectMapper mapper = JsonMapper.builder()
                .constructorDetector(USE_PROPERTIES_BASED)
                .build();

        Example obj = mapper.readValue("{\n" +
                "  \"field\": \"email\",\n" +
                "  \"text\": \"some text\"\n" +
                "}", Example.class);

        System.out.println(obj);
    }
}

When I run main() it fails with
Exception in thread "main" com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Cannot construct instance of Example (no Creators, like default constructor, exist): cannot deserialize from Object value (no delegate- or property-based Creator)
Under src/test/java/com/fasterxml/jackson/databind/deser/creators/ConstructorDetector1498Test.java I dont see any tests with immutable object and multi-arg constructor, there is class TwoArgsNotAnnotated but fields are not final, perhaps this is the issue?

@cowtowncoder
Copy link
Member

@lukaszpierog yes, correct wrt usage, I did not doublecheck my example.

On your example: you must register jackson-module-parameter-names, otherwise names of constructor parameters will not be available and there is no way to match JSON properties to arguments. So that would explain the failure.

One challenge so far has been testing, as jackson-databind can not depend on parameter names module (not even tests, due to build order), but it would now be possible to add explicit tests in jackson-integration-tests (which I created recently).

@lukaszpierog
Copy link
Author

@cowtowncoder
Unfortunatelly still the same result after adjusting the example

public final class Example {
    private final String field;
    private final String text;

    public Example(String field, String text) {
        this.field = field;
        this.text = text;
    }

    public String getField() {
        return field;
    }

    public String getText() {
        return text;
    }

    public static void main(String[] args) throws Exception {
        ObjectMapper mapper = JsonMapper.builder()
                .addModule(new ParameterNamesModule())
                .constructorDetector(USE_PROPERTIES_BASED)
                .build();

        Example obj = mapper.readValue("{\n" +
                "  \"field\": \"email\",\n" +
                "  \"text\": \"some text\"\n" +
                "}", Example.class);

        System.out.println(obj);
    }
}

@cowtowncoder
Copy link
Member

@lukaszpierog Hmmh. Ok, thank you, I'll see if I can reproduce.

@cowtowncoder cowtowncoder reopened this Oct 14, 2020
@cowtowncoder
Copy link
Member

@lukaszpierog Unfortunately I can not reproduce your findings: your example works fine for me.
In fact, with 2 parameters, it works without setting mode (mode matters for 1-arg case, as you mentioned).
About the only thing I can think of that could explain this would be if your compiler did not add parameter names in bytecode.

How does the example fail for you?

@lukaszpierog
Copy link
Author

@cowtowncoder
Perhaps I'm missing something.
I've created a simple repo with basic gradle setup -> https://github.com/lukaszpierog/jackson-pojo
You can either run main() in Example class or a junit test in ExampleTest, both fail with

Cannot construct instance of `Example` (no Creators, like default constructor, exist): cannot deserialize from Object value (no delegate- or property-based Creator)
 at [Source: (String)"{
  "field": "email",
  "text": "some text"
}"; line: 2, column: 3]

@cowtowncoder
Copy link
Member

@lukaszpierog I can reproduce the issue with that project. But as per my earlier suspicion, using this:

https://stackoverflow.com/questions/37463902/how-to-pass-parameters-javac-flag-to-java-compiler-via-gradle

that is, adding lines:

compileJava {
    options.compilerArgs << '-parameters'
}

makes test pass.

I don't know why javac by default does not appear to add parameter names in bytecode.

@lukaszpierog
Copy link
Author

@cowtowncoder
Thank you, you are correct. I didn't get your meaning at first, I though you meant some extra compiler flags was causing this, not that its the default behavior.

@cowtowncoder
Copy link
Member

@lukaszpierog Yeah, it is bit odd that default is such that information is left out and not the other way around.
Although there may be legit reasons wrt backwards-compatibility of bytecode -- perhaps some older tools have problem with inclusion.

But at least the mystery is solved.

It would be nice to have some mechanism of indicating likely issue, but this may be tricky. Problem would be reported if constructor was marked with just @JsonCreator.
... I wonder if there was a way to indicate this, however, for special case of not having anything but one constructor; has properties (indicated by getter) and no other way to instantiate a value.
This might be possible to detect reliably enough not to cause false alarms.

Will file an issue for that idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate Duplicate of an existing (usually earlier) issue
Projects
None yet
Development

No branches or pull requests

2 participants