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

Only avoid Records fields detection for deserialization #3894

Merged
merged 1 commit into from
May 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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)
yihtserns marked this conversation as resolved.
Show resolved Hide resolved
if (!isRecordType()) {
// 17-Apr-2023: Need Records' fields for serialization for cases like [databind#3895] & [databind#3628]
if (!isRecordType() || _forSerialization) {
Copy link
Member

@cowtowncoder cowtowncoder Apr 24, 2023

Choose a reason for hiding this comment

The 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.

Copy link
Member

Choose a reason for hiding this comment

The 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 Fields but since it's only for serialization perhaps it's not a problem).

Copy link
Member

@JooHyukKim JooHyukKim Apr 27, 2023

Choose a reason for hiding this comment

The 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 🥲

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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. 😮‍💨

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've did test this against that issue,

@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);
Expand Down
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);
}
}