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.

Sign up for GitHub

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

Jump to bottom

Fix issues 4697 and 2461 (@JsonUnwrapped caching) #4722

Merged
merged 8 commits into from
Oct 3, 2024
Merged

Fix issues 4697 and 2461 (@JsonUnwrapped caching) #4722

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a996e3d
Fixed issues 4697 and 2461
SandeepGaur2016 Sep 29, 2024
0a304a1
removed unwanted changes pushed in last commit
SandeepGaur2016 Sep 29, 2024
1d24c26
removed unwanted changes pushed in last commit
SandeepGaur2016 Sep 29, 2024
6ed3cda
Minor chnages as per review comments
SandeepGaur2016 Sep 29, 2024
db03c14
Minor chnages as per review comments
SandeepGaur2016 Sep 30, 2024
53fd410
Update release notes
cowtowncoder Oct 1, 2024
09d7973
Rm accidentally added test file
cowtowncoder Oct 1, 2024
a066fa6
Merge branch '2.19' into fix-4697-2461
cowtowncoder Oct 2, 2024
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
24 changes: 17 additions & 7 deletions src/main/java/com/fasterxml/jackson/databind/ser/BeanPropertyWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
import com.fasterxml.jackson.databind.util.ClassUtil;
import com.fasterxml.jackson.databind.util.NameTransformer;

import static com.fasterxml.jackson.databind.ser.impl.PropertySerializerMap.emptyForProperties;
cowtowncoder marked this conversation as resolved.
Show resolved Hide resolved

/**
* Base bean property handler class, which implements common parts of
* reflection-based functionality for accessing a property value and serializing
Expand Down Expand Up @@ -222,8 +224,7 @@ public BeanPropertyWriter(BeanPropertyDefinition propDef,

_declaredType = declaredType;
_serializer = (JsonSerializer<Object>) ser;
_dynamicSerializers = (ser == null) ? PropertySerializerMap
.emptyForProperties() : null;
_dynamicSerializers = (ser == null) ? emptyForProperties() : null;
_typeSerializer = typeSer;
_cfgSerializationType = serType;

Expand Down Expand Up @@ -325,7 +326,12 @@ protected BeanPropertyWriter(BeanPropertyWriter base, PropertyName name) {
base._internalSettings);
}
_cfgSerializationType = base._cfgSerializationType;
_dynamicSerializers = base._dynamicSerializers;
if(base instanceof UnwrappingBeanPropertyWriter) {
Copy link
Member

Choose a reason for hiding this comment

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

Dependency from super class to subclasses doesn't seem to be the right direction.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, did the same thing in a better way now

_dynamicSerializers =
emptyForProperties();
} else {
_dynamicSerializers = base._dynamicSerializers;
}
Copy link
Member

Choose a reason for hiding this comment

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

Keep the format consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, I will set the formatter.

_suppressNulls = base._suppressNulls;
_suppressableValue = base._suppressableValue;
_includeInViews = base._includeInViews;
Expand All @@ -350,8 +356,13 @@ protected BeanPropertyWriter(BeanPropertyWriter base, SerializedString name) {
base._internalSettings);
}
_cfgSerializationType = base._cfgSerializationType;
_dynamicSerializers = base._dynamicSerializers;
_suppressNulls = base._suppressNulls;
if(base instanceof UnwrappingBeanPropertyWriter) {
_dynamicSerializers =
emptyForProperties();
} else {
_dynamicSerializers = base._dynamicSerializers;
}
_suppressNulls = base._suppressNulls;
Copy link
Member

Choose a reason for hiding this comment

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

Formatting needs to be done.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

_suppressableValue = base._suppressableValue;
_includeInViews = base._includeInViews;
_typeSerializer = base._typeSerializer;
Expand Down Expand Up @@ -459,7 +470,7 @@ Object readResolve() {
_field = null;
}
if (_serializer == null) {
_dynamicSerializers = PropertySerializerMap.emptyForProperties();
_dynamicSerializers = emptyForProperties();
Copy link
Member

Choose a reason for hiding this comment

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

Here also, unncessary change (at least for this PR).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done, though it was a clean way to import statically

Copy link
Member

Choose a reason for hiding this comment

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

I prefer avoiding static imports for method calls fwtw. Maybe that should be added in style guide (or maybe it is mentioned and CONTRIBUTING.md should refer). I know IDEs can show where method comes from but for me (and I realize this is matter of preference) little bit of redundancy can be helpful.

}
return this;
}
Expand Down Expand Up @@ -988,4 +999,3 @@ public String toString() {
sb.append(')');
return sb.toString();
}
}
22 changes: 20 additions & 2 deletions .../com/fasterxml/jackson/databind/tofix/JsonUnwrappedInconsistentSerialization4697Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,16 @@ public Third() {
}
}

public static class AnotherFirst {
public Third thrid = new Third();
}

public static class Common {
public String a;
public String b;
public Common() {}
}

@JacksonTestFailureExpected
@Test
public void testInconsistentSer() throws Exception {
First first = new First();
Expand All @@ -51,8 +54,23 @@ public void testInconsistentSer() throws Exception {
ObjectMapper secondMapper = newJsonMapper();

firstMapper.writeValueAsString(first);
firstMapper.writeValueAsString(second);
assertEquals(
firstMapper.writeValueAsString(second),
secondMapper.writeValueAsString(second));
}
@Test
public void testInconsistentSer1() throws Exception {
AnotherFirst first = new AnotherFirst();
Second second = new Second();

ObjectMapper firstMapper = newJsonMapper();
ObjectMapper secondMapper = newJsonMapper();

firstMapper.writeValueAsString(first);
firstMapper.writeValueAsString(second);
assertEquals(
firstMapper.writeValueAsString(second),
secondMapper.writeValueAsString(second));
}
}
}
1 change: 0 additions & 1 deletion src/test/java/com/fasterxml/jackson/databind/tofix/UnwrappedCaching2461Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ static class OuterContainer {
}

// [databind#2461]
@JacksonTestFailureExpected
@Test
void unwrappedCaching() throws Exception {
final InnerContainer inner = new InnerContainer(new Base("12345"));
Expand Down