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 <[email protected]> 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 <[email protected]> --- .../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. */
