-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Failure to find creator property with Lombok and an unwrapping mixin involved #1239
Comments
@olivergierke There is an open but unresolved problem regarding resolution of constructor properties, which could play part here (I forget the issue number). Unfortunately right now a full rewrite of introspection code would be needed; and whereas detection of fields/setters/getters as accessors is relatively simple, finding actual creator properties is anything but. This both because they are contained within constructor/factory method, which in itself needs to be considered visible, as well as only one to use; and because parameter-name detection further complicates things. The challenge boils down to the fact that essentially only parameters of a single creator methods should be merged with "regular" accessors (this does not include possible alternative "delegating" cases where name is not used, and parameter is not merged with properties). I don't know if above explanation helps at all, except by acknowledging that there are issues within handling of creator parameters. Also: it seems that Lombok, auto-values and other frameworks also add their challenges both for polymorphic handling (one codes against interfaces/abstract classes, and there's no linkage from those to concrete types; at least by Jackson -- may need datatype libs to detect annotations frameworks are likely to add), and for parameter name detection. |
I see. Actually, I think the sample at hand here is quite straight forward as there's only one constructor available. No static factory methods. I guess what puzzles me most is that the getter seems to play into that, something I would've imagined only being looked at for serialization, not deserialization. Generally speaking, I think that types with a canonical constructor and some form of parameter name (either through an explicit annotation, debug info or JDK 8 -parameter) should be the most straight forward to use. Or to phrase it the other way round: while I can totally see that there are cases of complex ambiguities, I had hoped that by exposing only one way to instantiate the type I'd make it easy to Jackson to use exactly that way. Especially, as this worked on 2.6 :). That said, don't get too hung up on the issue, I can easily work around it tweaking Lombok's configuration for now. I was more puzzled by that special interplay of things :). |
@olivergierke yeah my explanation was a more general lamentation on complexities involved for the whole system. But back to this example: what does Now: creator information is indeed used on serialization side, as all property information must be considered as a whole. This because some users only annotate their fields; other getters; other setters; yet some creator parameters, or perhaps some combination. So looking at just a subset will not cover all the cases users expect to. And you don't know what you are missing without looking to see what all is there. This does cause a few issues for bogus failures (there are some still open) too. |
Switched to @SpringBootTest and removed references to @SpringApplicationConfiguration and SpringJunit4ClassRunner (in favor of SpringRunner). Tweaked Lombok to avoid generating @ConstructorProperty on arguments of generated constructors as this is causing issues with Jackson [0]. Removed the explicit @EntityScan as the upgrade to Hibernate 5.1 brings support for JDK8 date/time types out of the box. Removed the property override for the Spring Framework version. Upgraded to Lombok 1.16.8 along the way. [0] FasterXML/jackson-databind#1239
It's pretty simple: @AllArgsConstructor
class Foo {
String foo, bar;
Nested nested;
} translates to: class Foo {
String foo, bar;
Nested nested;
@java.beans.ConstructorProperties({"foo", "bar", "nested"})
public Foo(String foo, String bar, Nested nested) { … }
} (also documented here). So it boils down to a single constructor with explicit parameter names, which as unambiguous as it can get actually. Really just code I would've written by hand but I am happy having not to due to its boilerplate nature. |
@olivergierke And equivalent set up with no remaining |
As indicated above, teaching Lombok to not generate the annotation solves the issue. So I am kind of puzzled that basically duplicating information that has been there in the first place causes the issue. Especially, as it works just fine with the (superfluous) annotation with 2.6. |
Sigh. Double, triple sigh. Yes and no. Are As to 2.6, it did not contain jdk7 annotation handling and just ignored that annotation, unless jdk7 module was included. |
Quick note: I wonder if #1383 (fixed in 2.7.8, will be in 2.8.4) is related. Possibly not, as that's for static factory, but it is in same area. |
Our team ran in to this issue today in a JDK8 project when upgrading from Spring Boot 1.5.1 to 1.5.2 (tons of dependency updates). After the upgrade, our controllers could not deserialize nested Lombok-annotated objects until a child object's We ultimately created a
It's been hard for us to work through the above conversation and links to understand the context and the underlying issue. Is this an appropriate fix for the problem? Would there be any value in us creating an example project to share demonstrating the issue? |
@lnhrdt I think at this point it would be useful to have a non-Lombok-based unit test for reproducing the issue: using My best guess at this point is that although problem was surfaced by inclusion of JDK7 module in Jackson 2.8, it did exist before as well, and that it is a real problem. But unfortunately not one that can be easily resolved because of the challenges in unifying information from "simple" property accessors (fields, setter/getter methods) and creators (constructors, static factory methods) -- former are easy to collect, unify, since there is no bundling; whereas latter are much more difficult since they contain bundles of potential properties, of which only one (or zero) bundle is to be used. So: repro for ensuring my understanding is correct would be helpful, but I am not very optimistic on finding a fix soon. |
A similar issue came up today with this ticket in Spring Data REST. The symptom is that with the constructor properties annotation our deserializer modifications are ignored and thus deserialization fails. Again, explicitly configuring Lombok to not generate the constructor properties makes things work again. |
I think I may be running into a similar issue. Here's a nearly minimal example (without lombok): import com.fasterxml.jackson.annotation.JsonFormat;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.base.Objects;
import java.beans.ConstructorProperties;
import java.util.Date;
public class TestJsonDateFormatError {
public static void main(String[] args) throws Exception {
TestJsonFormatDate original = new TestJsonFormatDate();
original.setDate(new Date(0));
ObjectMapper objectMapper = new ObjectMapper();
final String json = objectMapper.writeValueAsString(original);
TestJsonFormatDate after = objectMapper.readValue(json, TestJsonFormatDate.class);
if(after.equals(original)) {
System.out.println("Success");
}
}
public static class TestJsonFormatDate {
public TestJsonFormatDate() {
}
// (1) Comment out constructor properties and it deserializes correctly
@ConstructorProperties({"date", "foo"})
public TestJsonFormatDate(Date date, String foo) {
this.date = date;
this.foo = foo;
}
@JsonFormat(pattern = "yyyy-MM-dd HH:mm:ss.SSS")
private Date date;
// (2) Comment out @JsonIgnore on other properties and it deserializes correctly
@JsonIgnore
private String foo;
public Date getDate() {
return this.date;
}
public void setDate(Date date) {
this.date = date;
}
public String getFoo() {
return this.foo;
}
public void setFoo(String foo) {
this.foo = foo;
}
public boolean equals(Object o) {
if (!(o instanceof TestJsonFormatDate)) {
return false;
}
TestJsonFormatDate other = (TestJsonFormatDate) o;
return Objects.equal(this.date, other.date)
&& Objects.equal(this.foo, other.foo);
}
}
} The serialization works fine and formats the date as expected according to
Comment out either of the two lines called out in the example above and all is fine. Let me know if this is the same issue or if I should create a separate issue. |
@jayanderson I can't say for sure, but I think it'd make sense to file a separate issue. And ideally verify it occurs with 2.9.0 (or if already done, add a note this is the case). |
@cowtowncoder Done: #1722. Thanks. |
Would be good if someone could verify this still occurs (or not) with |
@cowtowncoder I got the issue on 2.9.6. @Data
@Builder
@AllArgsConstructor
public class InstanceTO {
private String id;
private String name;
@JsonProperty("addresses")
private AddressesTO addrs;
@JsonIgnore
private List<AddressTO> addresses;
} The deserialization error message is 👍
Remove the @builder and @AllArgsConstructor works fine. Dependencies version:
|
@coderliux Thank you for checking this. If anyone knows how to see actual Java class Lombok pre-processor creates, that would be helpful. But I think that the problem is with |
The example presented in my original post works on current Lombok generations as 1.16.20 changed its behavior to now not add the In fact, this ins't even a Lombok issue at all. I've just removed Lombok and added public class JacksonTest {
@Test
public void testname() throws Exception {
ObjectMapper mapper = new ObjectMapper();
mapper.registerModule(new ParameterNamesModule());
mapper.addMixIn(Foo.class, FooMixin.class);
mapper.readValue("{ \"foo\" : \"something\", \"bar\" : \"something\" }", Foo.class);
}
static class Foo {
String foo, bar;
Nested nested;
@ConstructorProperties({ "foo", "bar", "nested" })
public Foo(String foo, String bar, Nested nested) {}
public Nested getNested() {
return nested;
}
}
@AllArgsConstructor
static class Nested {}
interface FooMixin {
@JsonUnwrapped
Nested getNested();
}
} Removing
|
Ok, yes: combination of |
@cowtowncoder Can you link that issue so that we all can subscribe to that Thanks! |
Sure, I think #1467 is the wish for improving handling in future. |
Switched to @SpringBootTest and removed references to @SpringApplicationConfiguration and SpringJunit4ClassRunner (in favor of SpringRunner). Tweaked Lombok to avoid generating @ConstructorProperty on arguments of generated constructors as this is causing issues with Jackson [0]. Removed the explicit @EntityScan as the upgrade to Hibernate 5.1 brings support for JDK8 date/time types out of the box. Removed the property override for the Spring Framework version. Upgraded to Lombok 1.16.8 along the way. [0] FasterXML/jackson-databind#1239
The following test case:
results in the following exception:
The following things make it work and I kind of have a hard time understanding why :):
@JsonUnwrapped
from the mixingetNested()
from the mixin entirelysuppressConstructorProperties = true
to@AllArgsConstructor
onFoo
.So it looks like that the "override" of the accessor in the mixin has a higher priority than the constructor that's generated by default Lombok. However, tricking Lombok into not adding the explicit constructor argument annotations seems to resolve the issue, too.
The error appears on 2.7.4, 2.6.5 works fine. Originally I thought I run into something related to #1122, but that one was resolved in 2.7.4.
The text was updated successfully, but these errors were encountered: