-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Only avoid Records fields detection for deserialization #3894
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -440,7 +440,8 @@ protected void collectAll() | |
|
||
// 15-Jan-2023, tatu: [databind#3736] Let's avoid detecting fields of Records | ||
// altogether (unless we find a good reason to detect them) | ||
if (!isRecordType()) { | ||
// 17-Apr-2023: Need Records' fields for serialization for cases like [databind#3895] & [databind#3628] | ||
if (!isRecordType() || _forSerialization) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmmm. I don't think this should be done, even if it handles this particular problem, it seems like a hack... But let me think about this bit more. I guess the idea is that doing this would "pull in" getters. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, thank you @yihtserns -- it's not so much for pulling in getters, but for alternate visibility. I think this is necessary, then. I am bit worried about possible side-effects of doing this (wrt forcing access to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the problem in #3897 taken into account, or affected by this PR? Hope not 🥲 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've did test this against that issue, the other day - apparently they're unrelated. 😮💨 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@yihtserns Great! ✌🏼✌🏼 If you have the test code used, maybe we can merge it to 2.15, under failing? If you don't have time for it, you can just share it here and I can include it in #3899 |
||
_addFields(props); // note: populates _fieldRenameMappings | ||
} | ||
_addMethods(props); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
package com.fasterxml.jackson.databind.records; | ||
|
||
import com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility; | ||
import com.fasterxml.jackson.annotation.JsonPropertyOrder; | ||
import com.fasterxml.jackson.annotation.PropertyAccessor; | ||
import com.fasterxml.jackson.databind.BaseMapTest; | ||
import com.fasterxml.jackson.databind.ObjectMapper; | ||
|
||
public class RecordIgnoreNonAccessorGetterTest extends BaseMapTest { | ||
|
||
// [databind#3628] | ||
interface InterfaceWithGetter { | ||
|
||
String getId(); | ||
|
||
String getName(); | ||
} | ||
|
||
@JsonPropertyOrder({"id", "name", "count"}) // easier to assert when JSON field ordering is always the same | ||
record RecordWithInterfaceWithGetter(String name) implements InterfaceWithGetter { | ||
|
||
@Override | ||
public String getId() { | ||
return "ID:" + name; | ||
} | ||
|
||
@Override | ||
public String getName() { | ||
return name; | ||
} | ||
|
||
// [databind#3895] | ||
public int getCount() { | ||
return 999; | ||
} | ||
} | ||
|
||
private final ObjectMapper MAPPER = newJsonMapper(); | ||
|
||
public void testSerializeIgnoreInterfaceGetter_WithoutUsingVisibilityConfig() throws Exception { | ||
String json = MAPPER.writeValueAsString(new RecordWithInterfaceWithGetter("Bob")); | ||
|
||
assertEquals("{\"id\":\"ID:Bob\",\"name\":\"Bob\",\"count\":999}", json); | ||
} | ||
|
||
public void testSerializeIgnoreInterfaceGetter_UsingVisibilityConfig() throws Exception { | ||
MAPPER.setVisibility(PropertyAccessor.GETTER, Visibility.NONE); | ||
MAPPER.setVisibility(PropertyAccessor.FIELD, Visibility.ANY); | ||
|
||
String json = MAPPER.writeValueAsString(new RecordWithInterfaceWithGetter("Bob")); | ||
|
||
assertEquals("{\"name\":\"Bob\"}", json); | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.