-
-
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
@JsonProperty
not serializing field names properly on @JsonCreator
in Record
#4452
Comments
Tested using public static record TestObject(String testFieldName, Integer testOtherField) {
@JsonCreator
public TestObject(@JsonProperty("strField") String testFieldName, @JsonProperty("intField") Integer testOtherField) {
this.testFieldName = testFieldName;
this.testOtherField = testOtherField;
}
public String testFieldName() {
return this.testFieldName;
}
public Integer testOtherField() {
return this.testOtherField;
}
} ...the Only by annotating in the Record's header i.e.: public static record TestObject(@JsonProperty("strField") String testFieldName, @JsonProperty("intField") Integer testOtherField) {
@JsonCreator
public TestObject(String testFieldName, Integer testOtherField) {
this.testFieldName = testFieldName;
this.testOtherField = testOtherField;
}
public String testFieldName() {
return this.testFieldName;
}
public Integer testOtherField() {
return this.testOtherField;
}
} ...will a usable compiled code be produced: public static record TestObject(String testFieldName, Integer testOtherField) {
@JsonCreator
public TestObject(String testFieldName, Integer testOtherField) {
this.testFieldName = testFieldName;
this.testOtherField = testOtherField;
}
@JsonProperty("strField")
public String testFieldName() {
return this.testFieldName;
}
@JsonProperty("intField")
public Integer testOtherField() {
return this.testOtherField;
}
} |
Yeah, I think we've figured out this test specifically doesn't work on any version. We're not sure why it works for us with our application's old dependency combination. I don't know if it's something about the parameter names module or properties on the spring Is this something that is intended to be supported or is it a requirement that |
If the
...then it'll work but I think that's more of a happy coincidence that worked because the constructor argument names happen to match the property (field/accessor) names. And that's now broken in I'm not entirely sure, but I vaguely remember seeing an issue talking about names... But I'm not gonna dig further since I'm just here to make sure it wasn't my contribution from 1 year ago that caused this - imma leave this to the core maintainers. 🏃♀️💨 |
Much appreciated, this helps with analysis for sure. I think that narrows this down to something changed between import static org.junit.Assert.assertTrue;
import org.junit.Test;
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.module.paramnames.ParameterNamesModule;
public class RecordTest {
public static final ObjectMapper OBJECT_MAPPER = new ObjectMapper()
.registerModule(new ParameterNamesModule()).findAndRegisterModules();
public record TestObject(String testFieldName, Integer testOtherField) {
@JsonCreator
public TestObject(@JsonProperty ("strField") String testFieldName,
@JsonProperty ("intField") Integer testOtherField) {
this.testFieldName = testFieldName;
this.testOtherField = testOtherField;
}
}
@Test
public void test() throws JsonProcessingException {
final String result = OBJECT_MAPPER.writeValueAsString(new TestObject("test", 1));
assertTrue(result.contains("strField"));
}
} For now, our solution is to eliminate our I don't know if there is supposed to be support for the above test case from the repo maintainers, but I will also stop and let them comment. |
So as per release note there seem to be these three PRs --#3906, #3992, #4175 -- that are titled with the word |
I don't think this thing is Record-specific. |
Because of project's current on-going milestone aka |
I think that's fine. The below test does work on 2.16.x and that suits our use cases for now. import static org.junit.Assert.assertTrue;
import org.junit.Test;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.module.paramnames.ParameterNamesModule;
public class RecordTest {
public static final ObjectMapper OBJECT_MAPPER = new ObjectMapper()
.registerModule(new ParameterNamesModule()).findAndRegisterModules();
public record TestObject(@JsonProperty ("strField") String testFieldName,
@JsonProperty ("intField") Integer testOtherField) {
}
@Test
public void test() throws JsonProcessingException {
final String result = OBJECT_MAPPER.writeValueAsString(new TestObject("test", 1));
assertTrue(result.contains("strField"));
}
} It's possible that this is the correct and only supported future usage, but there probably needs to be some consideration (an error, documentation, warning, etc) about the presence of a import static org.junit.Assert.assertTrue;
import org.junit.Test;
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.module.paramnames.ParameterNamesModule;
public class RecordTest {
public static final ObjectMapper OBJECT_MAPPER = new ObjectMapper()
.registerModule(new ParameterNamesModule()).findAndRegisterModules();
public record TestObject(String testFieldName, Integer testOtherField) {
@JsonCreator
public TestObject(@JsonProperty ("strField") String testFieldName,
@JsonProperty ("someOtherIntField") Integer testOtherIntField,
@JsonProperty ("intField") Integer testOtherField) {
this(testFieldName, testOtherField + testOtherIntField);
}
}
@Test
public void test() throws JsonProcessingException {
final String result = OBJECT_MAPPER.writeValueAsString(new TestObject("test", 2, 1));
assertTrue(result.contains("intField"));
assertTrue(result.contains("strField"));
}
} I'm not even sure how the above could work, so it's most likely not valid or going to be supported, but I don't know that as a consumer. |
FWTW, if not Record-specific, reproduction without Record would be great (since it's likely in shared code b/w POJOs, Records). |
Looking at the original case, it does seem like a flaw: And while fix may or may not be easy, a PR for reproduction, would be great.
Looking at the original reproduction, it would seem it is? |
Ok, at any rate: a PR for failing test case (under |
Most test cases here make use of |
Ugh. I missed Correct, tests here cannot use it. There are a few tests tho that implement replica, |
Filed PR #4477. It does fail from 2.16 version and on. |
@JsonProperty
not serializing field names properly on @JsonCreator
in Record
Search before asking
Describe the bug
Serialization of java records do not appear to be honoring
@JsonProperty
names in@JsonCreator
constructors.Version Information
2.15.x+
Reproduction
A test below fails for me and I expect it to pass.
Expected behavior
The field is renamed to what is specified in the
@JsonProperty
annotation in the record's@JsonCreator
Additional context
We encountered this upgrading from 2.14.x up to 2.16.x, so this functionality was working at that earlier version (perhaps before records were really addressed).
Perhaps we have versions incorrect between jackson modules and libraries, but we haven't found the exact mismatch yet.
The text was updated successfully, but these errors were encountered: