You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@struts.apache.org by lu...@apache.org on 2026/10/23 08:04:12 UTC
(struts) 03/04: WW-5790 fix(rest): compare nested beans leaf by leaf against a same-class baseline
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/struts.git
commit eb890e27b0050dd89060899253e3e153b01030be
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Fri Oct 23 07:42:14 2026 +0200
WW-5790 fix(rest): compare nested beans leaf by leaf against a same-class baseline
Co-Authored-By: Claude Mythos 5.1 <no...@anthropic.com>
---
.../struts2/rest/ContentTypeInterceptor.java | 22 +++++--
.../ContentTypeInterceptorIntegrationTest.java | 70 ++++++++++++++++++++++
2 files changed, 86 insertions(+), 6 deletions(-)
diff --git a/plugins/rest/src/main/java/org/apache/struts2/rest/ContentTypeInterceptor.java b/plugins/rest/src/main/java/org/apache/struts2/rest/ContentTypeInterceptor.java
index 4e51a6e2a..a0afa0b61 100644
--- a/plugins/rest/src/main/java/org/apache/struts2/rest/ContentTypeInterceptor.java
+++ b/plugins/rest/src/main/java/org/apache/struts2/rest/ContentTypeInterceptor.java
@@ -242,16 +242,21 @@ public class ContentTypeInterceptor extends AbstractInterceptor {
Object sourceValue = readMethod.invoke(source);
Object baselineValue = baseline == null ? null : readMethod.invoke(baseline);
- // A null, or a value equal to the baseline's constructor default, reads as "not in the body":
- // copying it would overwrite what params or prepare() put on the target. The cost is that an
- // explicit null, or a body value equal to the constructor default, is not applied either.
- if (sourceValue == null || (baseline != null && Objects.deepEquals(sourceValue, baselineValue))) {
+ // A null, or a leaf or collection equal to the baseline's constructor default, reads as "not in
+ // the body": copying it would overwrite what params or prepare() put on the target. The cost is
+ // that an explicit null, a primitive at its zero value, an empty collection, or any body value
+ // equal to the constructor default is not applied either. A nested bean is compared leaf by leaf
+ // below, never through its own equals.
+ if (sourceValue == null) {
+ continue;
+ }
+ boolean nestedBean = isNestedBeanType(sourceValue.getClass());
+ if (!nestedBean && baseline != null && Objects.deepEquals(sourceValue, baselineValue)) {
continue;
}
- if (isNestedBeanType(sourceValue.getClass())) {
+ if (nestedBean) {
// Complex bean: recurse to authorize nested fields, passing authTarget unchanged
- Object nestedBaseline = baselineValue != null ? baselineValue : createFreshInstance(sourceValue.getClass());
Object targetValue = readMethod.invoke(target);
if (targetValue == null) {
Object newTarget = createFreshInstance(sourceValue.getClass());
@@ -268,6 +273,11 @@ public class ContentTypeInterceptor extends AbstractInterceptor {
continue;
}
}
+ // The baseline must be the handler's runtime type: a constructor default of the declared
+ // supertype cannot answer for a subtype's own properties.
+ Object nestedBaseline = baselineValue != null && baselineValue.getClass() == sourceValue.getClass()
+ ? baselineValue
+ : createFreshInstance(sourceValue.getClass());
copyAuthorizedProperties(sourceValue, nestedBaseline, targetValue, action, authTarget, fullPath);
} else if (sourceValue instanceof Collection) {
writeMethod.invoke(target,
diff --git a/plugins/rest/src/test/java/org/apache/struts2/rest/ContentTypeInterceptorIntegrationTest.java b/plugins/rest/src/test/java/org/apache/struts2/rest/ContentTypeInterceptorIntegrationTest.java
index 67d381283..0ac5ce458 100644
--- a/plugins/rest/src/test/java/org/apache/struts2/rest/ContentTypeInterceptorIntegrationTest.java
+++ b/plugins/rest/src/test/java/org/apache/struts2/rest/ContentTypeInterceptorIntegrationTest.java
@@ -254,6 +254,34 @@ public class ContentTypeInterceptorIntegrationTest extends TestCase {
assertEquals("documented limit of the baseline comparison", "approved", model.getStatus());
}
+ public void testTwoPhaseCopyLimitAPrimitiveAtItsZeroValueIsNotApplied() throws Exception {
+ DefaultedModel model = runScriptedBody(m -> m.setEnabled(true), m -> m.setEnabled(false));
+
+ assertTrue("documented limit: a primitive zero value equals the constructor default", model.isEnabled());
+ }
+
+ public void testTwoPhaseCopyLimitAnEmptyCollectionDoesNotClearTheTarget() throws Exception {
+ DefaultedModel model = runScriptedBody(m -> m.setTags(new ArrayList<>(List.of("kept"))), m -> m.setTags(new ArrayList<>()));
+
+ assertEquals("documented limit: an empty collection equals the constructor default", List.of("kept"), model.getTags());
+ }
+
+ public void testTwoPhaseCopyAppliesALeafOfANestedSubtypeTheBodySets() throws Exception {
+ DefaultedModel model = runScriptedBody(m -> m.setAddress(new HomeAddress()), m -> {
+ HomeAddress home = new HomeAddress();
+ home.setFloor("3");
+ m.setAddress(home);
+ });
+
+ assertEquals("3", ((HomeAddress) model.getAddress()).getFloor());
+ }
+
+ public void testTwoPhaseCopyRecursesIntoANestedBeanWhoseEqualsIgnoresTheChangedLeaf() throws Exception {
+ DefaultedModel model = runScriptedBody(m -> { }, m -> m.getBilling().setCity("Warsaw"));
+
+ assertEquals("Warsaw", model.getBilling().getCity());
+ }
+
private DefaultedModel runScriptedBody(Consumer<DefaultedModel> alreadyOnTheModel, Consumer<DefaultedModel> body) throws Exception {
ModelDrivenRestAction modelDriven = new ModelDrivenRestAction();
DefaultedModel model = modelDriven.getModel();
@@ -488,9 +516,19 @@ public class ContentTypeInterceptorIntegrationTest extends TestCase {
private List<String> tags = new ArrayList<>();
private DefaultedAddress address = new DefaultedAddress();
private PinnedAddress pinned;
+ private IdEqualAddress billing = new IdEqualAddress();
+ private boolean enabled;
public String getStatus() { return status; }
+ public boolean isEnabled() { return enabled; }
+
+ public void setEnabled(boolean enabled) { this.enabled = enabled; }
+
+ public IdEqualAddress getBilling() { return billing; }
+
+ public void setBilling(IdEqualAddress billing) { this.billing = billing; }
+
public void setStatus(String status) { this.status = status; }
public String getName() { return name; }
@@ -523,6 +561,38 @@ public class ContentTypeInterceptorIntegrationTest extends TestCase {
public void setCity(String city) { this.city = city; }
}
+ public static class HomeAddress extends DefaultedAddress {
+ private String floor;
+
+ public String getFloor() { return floor; }
+
+ public void setFloor(String floor) { this.floor = floor; }
+ }
+
+ /**
+ * WW-5790: equals by id only, the shape of a JPA entity; a leaf-blind equals must not hide a changed leaf.
+ */
+ public static class IdEqualAddress {
+ private Long id;
+ private String city;
+
+ public Long getId() { return id; }
+
+ public void setId(Long id) { this.id = id; }
+
+ public String getCity() { return city; }
+
+ public void setCity(String city) { this.city = city; }
+
+ @Override
+ public boolean equals(Object other) {
+ return other instanceof IdEqualAddress that && java.util.Objects.equals(id, that.id);
+ }
+
+ @Override
+ public int hashCode() { return java.util.Objects.hashCode(id); }
+ }
+
/**
* WW-5790: no no-arg constructor, so the copy cannot build a baseline for this subtree.
*/