projectlombok / lombok

Very spicy additions to the Java programming language.
https://projectlombok.org/
Other
12.83k stars 2.37k forks source link

1.16.20 @Data object no longer constructable for jackson? #1563

Closed josephlbarnett closed 5 years ago

josephlbarnett commented 6 years ago

Seeing the following exception from jackson 2.9.2 when trying to parse a json string into a @Data annotated class with lombok 1.16.20. Everything works fine with lombok 1.16.18, however.

com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Cannot construct instance of `com.xyz.EventEnvelope` (no Creators, like default construct, exist): cannot deserialize from Object value (no delegate- or property-based Creator)
 at [Source: (String)"{
  "eventType" : "A",
  "eventTypeVersion" : 1,
  "producerName" : "producerName",
  "eventTime" : 1516213186401,
  "serializationType" : "JSON",
  "eventBytes" : "ewogICJldmVudEFmaWVsZDEiIDogImYxIiwKICAiZXZlbnRBZmllbGQyIiA6IDQ1NiwKICAiZXZlbnRBZmllbGQzIiA6IFsgMjAxOCwgMSwgMTcsIDE4LCAxOSwgNDYsIDQwMSBdCn0="
}"; line: 2, column: 3]
    at com.fasterxml.jackson.databind.exc.InvalidDefinitionException.from(InvalidDefinitionException.java:67)
    at com.fasterxml.jackson.databind.DeserializationContext.reportBadDefinition(DeserializationContext.java:1451)
    at com.fasterxml.jackson.databind.DeserializationContext.handleMissingInstantiator(DeserializationContext.java:1027)
    at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromObjectUsingNonDefault(BeanDeserializerBase.java:1275)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:325)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:159)
    at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4001)
    at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:2992)

the class looks like:

@Data
@AllArgsConstructor
public class EventEnvelope {

    private String eventType;
    private int eventTypeVersion;
    private String producerName;
    private Instant eventTime;
    private SerializationType serializationType;   // simple ENUM type with JSON as an enumerated value
    private byte[] eventBytes;
   @Override
   public void toString() {
       // override string impl
   }
}
iherasymenko commented 6 years ago

lombok.anyConstructor.addConstructorProperties=true in your lombok.config should resolve the issue.

josephlbarnett commented 6 years ago

ah, missed the changelog, but see what you mean now. It looks like JDK9 "broke" those annotations, so just to understand the future a little better, is there an expected/supported way to get data annotated classes and jackson object mapper to interact properly?

iherasymenko commented 6 years ago

Try https://github.com/FasterXML/jackson-modules-java8/tree/master/parameter-names or add javax.desktop to your module dependencies (kinda ugly, I know)

Falochu commented 6 years ago

Tried these workarounds (adding lombok.anyConstructor.addConstructorProperties=true in config and adding jackson-modules-java8 dependencies) with no success.

ldeck commented 6 years ago

Just to confirm that this effects @Value classes as well.

The following works fine with spring + jackson deserialisation in 1.16.18 but not 1.16.20 where it throws an invalid request exception.

@Value
@Builder
public class Foo {
  private String bar;
}
yurii-polishchuk commented 6 years ago

Yep, @Value has the same problem with Jackson. Correct me if I'm wrong, but using a builder for deserialization will work. e.g.

@Value
@Builder
@JsonDeserialize(builder = Foo.FooBuilder.class)
public class Foo {
  private String bar;

  @JsonPOJOBuilder(withPrefix = "")
  public static final class FooBuilder {
  }

}

But I don’t think you can rename the fields with @JsonProperty though.

any other ideas how to make all work with Jackson a little bit painless? :)

fpoyer commented 6 years ago

Adding

lombok.anyConstructor.addConstructorProperties=true
config.stopBubbling = true

in a file named lombok.config in the root directory of my project fixed it for me (using lombok 1.16.20, Oracle JDK 1.8 and Gradle 4.x). Still, I'm slightly sad that the default behavior changed (I'm using lombok in roughly 100 projects, so a "minor" change like that is a big deal for me...) Any chance the default behavior be changed back to what it was? I'm not sure I understand why the behavior change was required...

ivansenic commented 6 years ago

Just that I understand everything, as we are hitting the same bug. The change basically means that lombok is not adding anymore the @ConstructorProperties annotation to the generated constructors. And this one was used by the jackson when there was only an all-args constructor available. And since @Data and @Builder both add only the all-args constructor, the jackson has no way to instantiate the object.

Solution one would be setting the property as described above (seems easy, at least for Java 1.8).

Second would be to explicitly add @NoArgsConstructor (and unfortunately also @AllArgsConstructor if you are using @Builder) to all classes annotated with @Data or/and @Builder (I confirmed this works as jackson goes for no-arg constructor most likely). Is there any kind of property we can use to generate both types of constructors with @Data or/and @Budiler? Then I would not care about @ConstructorProperties at all. Or I am wrong here somehow?

And what does it mean that

Oracle more or less broke this annotation with the release of JDK9, necessitating this breaking change.

Does it mean that if I set lombok.anyConstructor.addConstructorProperties=true I am gonna hit the same problem as soon as I migrate to Java 9 or not?

rspilker commented 6 years ago

If you migrate to Java 9, you need to add a dependency: requires java.desktop.

rspilker commented 6 years ago

We made this change because, when using the java9 compiler, you would always need to add java.desktop as a dependency, otherwise your code wouldn't compile. Even if you don't use any java9 feature, or need any other desktop feature.

So, basically we were forced to make a choice:

We chose the latter, but we underestimated the effect, otherwise we would have announced it louder.

rspilker commented 6 years ago

I am open to the idea of always generating a NoArgs constructor, if that's configured in lombok.config.

But before we can do that, we would need to be able to generate no-arg constructors for uninitialized final fields. Not necessary too hard, but we cannot do that yet.

fpoyer commented 6 years ago

We made this change because, when using the java9 compiler, you would always need to add java.desktop as a dependency, otherwise your code wouldn't compile. Even if you don't use any java9 feature, or need any other desktop feature.

I understand the change, but how would that (having the java.desktop dependency) be different from the situation in Java 8. Are there any downsides of having that dependency that would be introduced? Otherwise, the potential benefit (jar size reduction? one can use proguard if that is really needed) doesn't seem that important for such a behavior regression/change... Just my 2 cents though, other people might disagree (or I may be missing and important point altogether, if so feel free to tell me)

Here's another suggestion: how about generating a no args c'tor if the @Data/@Builder annotated class has no un-initialized final fields (most Data Transfert Objects, like almost all objects used with jackson and the like for example)? That is basically a "all required args c'tor", except no args are actually required so it is a no args c'tor. Wouldn't that kind of fix the problem for a very common use case (reflection based instantiation of DTOs)?

ivansenic commented 6 years ago

I am open to the idea of always generating a NoArgs constructor, if that's configured in lombok.config.

But before we can do that, we would need to be able to generate no-arg constructors for uninitialized final fields. Not necessary too hard, but we cannot do that yet.

Couldn't this be solved easily, if there are any uninitialized final fields, don't generate the no-args constructor? Ahh I see @fpoyer has the same suggestion :+1:

rspilker commented 6 years ago

Yeah, that might work. Would it work with Jackson if there is a private no-args constructor? That would definitely reduce the impact and increase the likelyhood of introducing this without requiring opt-in via lombok.config

ivansenic commented 6 years ago

Yep private no-args constructor would work imo.

randakar commented 6 years ago

That solves it for @Data, but not for @Value objects. Especially not when inheritance gets involved. And I really prefer my DTO's to be immutable, for various reasons.

I think the best option is for the lombok package to depend on java.desktop itself. Given that we don't want existing installations to break, it is just the price you have to pay to maintain backwards compatibility and the one option that is most likely to work everywhere without fancy tricks.

Speaking of immutable DTO's, there is still a lot of boilerplate I have to generate for some types of jackson classes if I want to use both inheritance and @Value.

Here's how a class like that looks, currently:


@Value @EqualsAndHashCode(callSuper = true) @JsonDeserialize(builder = SomeDTO.SomeDTOBuilder.class) public class SomeDTO extends SomeDTOSuperClass {

@NonNull
private final Integer field;

@Builder(toBuilder = true)
public SomeDTO(long id, int field, ....) {
    super(id);
    this.field = field;
}

@JsonPOJOBuilder(withPrefix = "")
public static final class SomeDTOBuilder {
}

}

That's still quite a lot of boilerplate for a simple single-field DTO that inherits from some other class. On the upside, it avoids the problem we're discussing here, but at a cost. I've seen bugs filed to deal with the inheriting constructor problem (which is a hard one to solve, and not relevant here) but for the jackson stuff it would be nice if those jackson annotations landed on the builder automatically somehow.

Now, all projects that build with Jackson do have one thing in common: They depend on Jackson. Meaning that if there is a jackson dependency, these annotations are going to be available, and if there is not, they aren't. Maybe we can go this route, solving the problem for Jackson at least.

On Wed, Feb 7, 2018 at 11:03 AM, Roel Spilker notifications@github.com wrote:

Yeah, that might work. Would it work with Jackson is there is a private no-args constructor? That would definitely reduce the impact and increase the likelyhood of introducing this without requiring opt-in via lombok.config

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/rzwitserloot/lombok/issues/1563#issuecomment-363718276, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKCRep36pxB_t2nO-aaRw83nYM0oXpaks5tSXTngaJpZM4RhyCj .

-- "Don't only practice your art, but force your way into it's secrets, for it and knowledge can raise men to the divine." -- Ludwig von Beethoven

rspilker commented 6 years ago

I looked at generating a private no-args constructor and indeed did encounter several problems:

randakar commented 6 years ago

Maybe we can annotate the constructor of the superclass? If there were an annotation that told Lombok to use that we might at least have a partial solution for the whole constructor-inheritance thing.

On Tue, Feb 13, 2018 at 5:04 PM, Roel Spilker notifications@github.com wrote:

I looked at generating a private no-args constructor and indeed did encounter several problems:

  • If there is a superclass, there's no way we can safely generate any constructor. We don't have any information on the available constructors of the superclass, and even if we had, we wouldn't know what constructor to invoke.
  • We should only generate the constructor if there is a constructor, but it has at least one parameter. If there are no constructors, generating a private no-args constructor would break code.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rzwitserloot/lombok/issues/1563#issuecomment-365313068, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKCRaZijeNyHyxuTuJtN1Y9TJfNgpXhks5tUbKPgaJpZM4RhyCj .

-- "Don't only practice your art, but force your way into it's secrets, for it and knowledge can raise men to the divine." -- Ludwig von Beethoven

fpoyer commented 6 years ago

As soon as inheritance is on the table, I feel like there is only 2 cases really:

Having a no-args constructor in most simple cases would allow the avoidance of much boilerplate while retaining a certain level of backward compatibility with external tools like jackson.

Obviously, more advanced cases like @randakar's "immutable DTOs" would still need some additional work...

Maybe we can annotate the constructor of the superclass?

That wouldn't work in the all-too-common case where the super class is from third-party code and you cannot change it in any way.

randakar commented 6 years ago

The whole definition of @Value is that all fields are final and immutable. So no setters, and a no-args constructor makes no sense unless the class has no fields at all - in which case the @RequiredArgsConstructor implied by @Value will generate that already.

As for the "superclass is in third party code" case - yeah, that's why it is only a partial solution. It would help with the case where all classes are under your own control though. Also, in that case a workaround would be to add an intermediate level class that fronts for the third-party object just to make it possible to annotate the constructor. That would still reduce boilerplate since you'd only write one constructor, not one for every subclass.

On Wed, Feb 14, 2018 at 2:13 PM, François Poyer notifications@github.com wrote:

As soon as inheritance is on the table, I feel like there is only 2 cases really:

  • either the superclass has no "default" (no args) constructor. In my opinion, in that case, lombok should not generate any constructor for @Data nor @Value, as the implementer of the subclass must write all constructors of the subclass (including a potential no-args constructor) to decide what constructor of the superclass is called (and with what parameters) anyway.
  • or the superclass has a (visible) default constructor. In my opinion, in that case lombok could/should call that no-args constructor in all generated constructors. And I feel like lombok should always generate a public no-args constructor for @Data/@Value classes when possible (no un-initialized final field and a visible no-args super constructor). If one want to restrict/avoid that, it could either be opted-out (via a property in lombok.config) or simply overriden with a hand-written private (or protected or package) no-args constructor.

Having a no-args constructor in most simple cases would allow the avoidance of much boilerplate while retaining a certain level of backward compatibility with external tools like jackson.

Obviously, more advanced cases like @randakar https://github.com/randakar's "immutable DTOs" would still need some additional work...

Maybe we can annotate the constructor of the superclass?

That wouldn't work in the all-too-common case where the super class is from third-party code and you cannot change it in any way.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/rzwitserloot/lombok/issues/1563#issuecomment-365603160, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKCRexY8MixrYfrxZvALMOWLxXckIWrks5tUtv-gaJpZM4RhyCj .

-- "Don't only practice your art, but force your way into it's secrets, for it and knowledge can raise men to the divine." -- Ludwig von Beethoven

DuncanConroy commented 6 years ago

Are there any final plans on how to tackle this issue? For now, I will just skip the lombok update, but I also want to update to JDK 9 in the not so far future :) Thanks in advance!

bobtiernay-okta commented 6 years ago

What about just compiling with javac -parameters. Wouldn't this be enough for Jackson as more recent versions support this?

https://github.com/FasterXML/jackson-modules-java8/tree/master/parameter-names

and from https://github.com/FasterXML/jackson-modules-java8

Jackson 3.x changes things as it requires Java 8 to work and can thereby directly supported features. Because of this parameter-names and datatypes modules are merged into jackson-databind and need not be registered; datetime module (JavaTimeModule) remains separate module due to its size and configurability options.

rzwitserloot commented 6 years ago

@DuncanConroy Yes, you can add the lombok.config key as described, or use javac -parameters, or wait for @rspilker to find a way to get generation of a no-args private constructor on rails, but you'd have to add a key to make lombok generate this (probably to lombok.config). Oracle messed this up, there isn't anything we can do that results in you having to do absolutely nothing. Their fault.

udalrich commented 6 years ago

One solution that avoids the need for user change is to change the generation based on the target version. If you are building for 8-, generate @ConstructorProperties like you used to. If you are building for 9+, do not do that.

Java 8 builds then work like they used to. Java 9 did not work before 1.16.20, so there is not a large codebase that expects anything.

Obviously, also include a big warning in the documentation, that things work differently in Java 8 and Java 9.

rzwitserloot commented 5 years ago

Nope, we aren't going to fork what lombok generates based on what java version you have; that would mean what your code does silently changes depending on which java version you compiled it with. This is a real, but rather exotic thing that can happen to you, but we don't want to add to that mess.

Lombok has various config keys to fix this problem as best as possible. I understand this is still suboptimal, but blame oracle for the mess, we've spent more time on it than we ever wanted to.

jowparks commented 4 years ago

I had a similar issue to this and it wasn't fixed by adding the lombok.config file/entries. Setup is: JDK 13.0.1 org.projectlombok:lombok:1.18.8

I was trying to use lombok with graphql and had one of my classes like so (StructureType is enum):

@Data
public class AnnotationInput {
    private final StructureType structureType;
}

Which was causing an exception when accessing the endpoint. Changed to this:

@Data
@NoArgsConstructor
public class AnnotationInput {
    private StructureType structureType;
}

and it started working. Weird thing is that I have other classes mapped with lombok similarly to the class AnnotationInput example above, and they weren't causing exceptions. Something funny is going on with lombok for this, not sure what though.

randakar commented 4 years ago

If you run that class through delombok, does the problem persist? What is the delomboked code missing?

On Wed, May 6, 2020, 08:22 jowparks notifications@github.com wrote:

I had a similar issue to this and it wasn't fixed by adding the lombok.config file/entries. Setup is: JDK 13.0.1 org.projectlombok:lombok:1.18.8

I was trying to use lombok with graphql and had one of my classes like so (StructureType is enum):

@Data public class AnnotationInput { private final StructureType structureType; }

Which was causing an exception when accessing the endpoint. Changed to this:

@Data @NoArgsConstructor public class AnnotationInput { private StructureType structureType; }

and it started working. Weird thing is that I have other classes mapped with lombok similarly to the class AnnotationInput example above, and they weren't causing exceptions. Something funny is going on with lombok for this, not sure what though.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/rzwitserloot/lombok/issues/1563#issuecomment-624463599, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABIERJRSHFCI3ZNLUG4HYTRQD6ZTANCNFSM4EMHECRQ .

jowparks commented 4 years ago

Class looks fine when delomboked, not sure what the issue is:

// Generated by delombok at Wed May 06 21:23:49 PDT 2020
public class AnnotationInput {
    private final StructureType structureType;

    @java.lang.SuppressWarnings("all")
    public AnnotationInput(final StructureType structureType) {
        this.structureType = structureType;
    }

    @java.lang.SuppressWarnings("all")
    public StructureType getStructureType() {
        return this.structureType;
    }

    @java.lang.Override
    @java.lang.SuppressWarnings("all")
    public boolean equals(final java.lang.Object o) {
        if (o == this) return true;
        if (!(o instanceof AnnotationInput)) return false;
        final AnnotationInput other = (AnnotationInput) o;
        if (!other.canEqual((java.lang.Object) this)) return false;
        final java.lang.Object this$structureType = this.getStructureType();
        final java.lang.Object other$structureType = other.getStructureType();
        if (this$structureType == null ? other$structureType != null : !this$structureType.equals(other$structureType)) return false;
        return true;
    }

    @java.lang.SuppressWarnings("all")
    protected boolean canEqual(final java.lang.Object other) {
        return other instanceof AnnotationInput;
    }

    @java.lang.Override
    @java.lang.SuppressWarnings("all")
    public int hashCode() {
        final int PRIME = 59;
        int result = 1;
        final java.lang.Object $structureType = this.getStructureType();
        result = result * PRIME + ($structureType == null ? 43 : $structureType.hashCode());
        return result;
    }

    @java.lang.Override
    @java.lang.SuppressWarnings("all")
    public java.lang.String toString() {
        return "AnnotationInput(structureType=" + this.getStructureType() + ")";
    }
}
randakar commented 4 years ago

Your problem is the constructor. It's lacking the @ConstructorProperties annotation so Jackson doesn't know how to deserialize the data into an object. There are multiple ways around it but the lombok config key "addConstructorProperties" listed here: https://projectlombok.org/features/constructor should do the trick.

On Thu, May 7, 2020 at 6:27 AM jowparks notifications@github.com wrote:

Class looks fine when delomboked, not sure what the issue is:

// Generated by delombok at Wed May 06 21:23:49 PDT 2020 public class AnnotationInput { private final StructureType structureType;

@java.lang.SuppressWarnings("all")
public AnnotationInput(final StructureType structureType) {
    this.structureType = structureType;
}

@java.lang.SuppressWarnings("all")
public StructureType getStructureType() {
    return this.structureType;
}

@java.lang.Override
@java.lang.SuppressWarnings("all")
public boolean equals(final java.lang.Object o) {
    if (o == this) return true;
    if (!(o instanceof AnnotationInput)) return false;
    final AnnotationInput other = (AnnotationInput) o;
    if (!other.canEqual((java.lang.Object) this)) return false;
    final java.lang.Object this$structureType = this.getStructureType();
    final java.lang.Object other$structureType = other.getStructureType();
    if (this$structureType == null ? other$structureType != null : !this$structureType.equals(other$structureType)) return false;
    return true;
}

@java.lang.SuppressWarnings("all")
protected boolean canEqual(final java.lang.Object other) {
    return other instanceof AnnotationInput;
}

@java.lang.Override
@java.lang.SuppressWarnings("all")
public int hashCode() {
    final int PRIME = 59;
    int result = 1;
    final java.lang.Object $structureType = this.getStructureType();
    result = result * PRIME + ($structureType == null ? 43 : $structureType.hashCode());
    return result;
}

@java.lang.Override
@java.lang.SuppressWarnings("all")
public java.lang.String toString() {
    return "AnnotationInput(structureType=" + this.getStructureType() + ")";
}

}

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/rzwitserloot/lombok/issues/1563#issuecomment-625021907, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABIERNZF4ZXJQJE3IAV26LRQI2CDANCNFSM4EMHECRQ .

-- "Don't only practice your art, but force your way into it's secrets, for it and knowledge can raise men to the divine." -- Ludwig von Beethoven

jowparks commented 4 years ago

@randakar thanks for the info. I had seen the comments above and tried adding a lombok.config to main part of my package with this config item (also tried resources folder) and I was still getting the same error. It was almost like the config wasn’t being recognized.

randakar commented 4 years ago

To verify if this is the problem you can just add a constructor with that annotation and test if the problem good away.

Note though that there is a '@Jacksonize' annotation in the works that should make all this a lot easier.

As for the config key: try putting lombok.config in the project root? That's where I keep mine and it works without issue

On Fri, May 8, 2020, 17:52 jowparks notifications@github.com wrote:

@randakar https://github.com/randakar thanks for the info. I had seen the comments above and tried adding a lombok.config to main part of my package with this config item (also tried resources folder) and I was still getting the same error. It was almost like the config wasn’t being recognized.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/rzwitserloot/lombok/issues/1563#issuecomment-625882172, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABIERJ3HPTNM5OUCOOFDOTRQQTE7ANCNFSM4EMHECRQ .

seadub commented 2 years ago

openjdk version "11.0.4" 2019-07-16

Added both NoArgs and AllArgs to satisfy the Data and Builder configs I had on my class, and it worked as expected.

istibekesi commented 1 year ago

Setting suggested configuration above breaks @JsonUnwrapped, causing the following jackson exception: Cannot define Creator property "point" as @JsonUnwrapped: combination not yet supported

Any idea how to overcome this and apply the configuration at the same time?