You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by gn...@apache.org on 2026/08/28 13:23:14 UTC

(maven) branch maven-4.0.x updated: Fix field cache in EnhancedCompositeBeanHelper to let child fields win over parent (#12626)

This is an automated email from the ASF dual-hosted git repository.

gnodet pushed a commit to branch maven-4.0.x
in repository https://gitbox.apache.org/repos/asf/maven.git


The following commit(s) were added to refs/heads/maven-4.0.x by this push:
     new 0da6147962 Fix field cache in EnhancedCompositeBeanHelper to let child fields win over parent (#12626)
0da6147962 is described below

commit 0da614796226334bf482e841396ad18085eded05
Author: Guillaume Nodet <gn...@gmail.com>
AuthorDate: Fri Aug 28 15:23:08 2026 +0200

    Fix field cache in EnhancedCompositeBeanHelper to let child fields win over parent (#12626)
    
    buildFieldCache() used put() which let parent fields overwrite child fields
    with the same name, because DeclaredMembers iterates child-first then parent.
    This broke Mojo parameter injection for shadowed fields (e.g. the 'skip'
    field in TestResourcesMojo vs ResourcesMojo).
    
    Switch to putIfAbsent() — matching the method cache — so the child's field
    takes precedence.
    
    Reported-in: apache/maven-resources-plugin#497
    
    Co-authored-by: Claude Opus 4.6 <no...@anthropic.com>
---
 .../internal/EnhancedCompositeBeanHelper.java      |  2 +-
 .../internal/EnhancedCompositeBeanHelperTest.java  | 70 ++++++++++++++++++++++
 2 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/impl/maven-core/src/main/java/org/apache/maven/configuration/internal/EnhancedCompositeBeanHelper.java b/impl/maven-core/src/main/java/org/apache/maven/configuration/internal/EnhancedCompositeBeanHelper.java
index 59ae2fa69b..0fbc818b81 100644
--- a/impl/maven-core/src/main/java/org/apache/maven/configuration/internal/EnhancedCompositeBeanHelper.java
+++ b/impl/maven-core/src/main/java/org/apache/maven/configuration/internal/EnhancedCompositeBeanHelper.java
@@ -229,7 +229,7 @@ private Map<String, Field> buildFieldCache(Class<?> beanType) {
         for (Object member : new DeclaredMembers(beanType, View.FIELDS)) {
             Field field = (Field) member;
             if (!Modifier.isStatic(field.getModifiers())) {
-                fieldMap.put(field.getName(), field);
+                fieldMap.putIfAbsent(field.getName(), field);
             }
         }
 
diff --git a/impl/maven-core/src/test/java/org/apache/maven/configuration/internal/EnhancedCompositeBeanHelperTest.java b/impl/maven-core/src/test/java/org/apache/maven/configuration/internal/EnhancedCompositeBeanHelperTest.java
index ecca3af926..4b33f7d119 100644
--- a/impl/maven-core/src/test/java/org/apache/maven/configuration/internal/EnhancedCompositeBeanHelperTest.java
+++ b/impl/maven-core/src/test/java/org/apache/maven/configuration/internal/EnhancedCompositeBeanHelperTest.java
@@ -208,6 +208,50 @@ void testMultipleFieldAccessesDoNotLeakAccessibility() throws Exception {
         assertEquals("value2", bean2.getDirectField());
     }
 
+    /**
+     * Verifies that when a child class shadows a parent field with the same name,
+     * setting the property injects into the child's field (not the parent's).
+     * This is the scenario reported in apache/maven-resources-plugin#497:
+     * both ResourcesMojo and TestResourcesMojo declare {@code private boolean skip},
+     * and DeclaredMembers iterates child-first then parent. Using {@code putIfAbsent}
+     * in buildFieldCache ensures the child's field wins.
+     */
+    @Test
+    void testSetPropertyOnShadowedFieldInjectsIntoChildField() throws Exception {
+        ChildBean bean = new ChildBean();
+        PlexusConfiguration config = new XmlPlexusConfiguration("skip");
+        config.setValue("true");
+
+        when(evaluator.evaluate("true")).thenReturn("true");
+
+        helper.setProperty(bean, "skip", null, config);
+
+        // The child's field must receive the value
+        assertTrue(bean.isChildSkip(), "Child's shadowed 'skip' field should be true");
+    }
+
+    /**
+     * Verifies that the field cache resolves the child's field even after
+     * the cache has been populated by a prior lookup on the same class.
+     */
+    @Test
+    void testShadowedFieldCacheConsistency() throws Exception {
+        ChildBean bean1 = new ChildBean();
+        ChildBean bean2 = new ChildBean();
+        PlexusConfiguration config = new XmlPlexusConfiguration("skip");
+        config.setValue("true");
+
+        when(evaluator.evaluate("true")).thenReturn("true");
+
+        // First call populates the cache
+        helper.setProperty(bean1, "skip", null, config);
+        // Second call uses the cache
+        helper.setProperty(bean2, "skip", null, config);
+
+        assertTrue(bean1.isChildSkip(), "First bean: child's 'skip' field should be true");
+        assertTrue(bean2.isChildSkip(), "Second bean: child's 'skip' field should be true");
+    }
+
     /**
      * Test bean class for testing property setting.
      */
@@ -236,4 +280,30 @@ public void addItem(String item) {
             this.items.add(item);
         }
     }
+
+    /**
+     * Parent bean that declares a {@code skip} field — mirrors ResourcesMojo.
+     */
+    public static class ParentBean {
+        @SuppressWarnings("unused")
+        private boolean skip;
+
+        public boolean isParentSkip() {
+            return skip;
+        }
+    }
+
+    /**
+     * Child bean that shadows the parent's {@code skip} field — mirrors TestResourcesMojo.
+     * Both parent and child declare {@code private boolean skip}; only the child's
+     * field should receive the injected value.
+     */
+    public static class ChildBean extends ParentBean {
+        @SuppressWarnings("unused")
+        private boolean skip;
+
+        public boolean isChildSkip() {
+            return skip;
+        }
+    }
 }