You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by gg...@apache.org on 2026/10/17 20:23:39 UTC

(commons-configuration) branch master updated: Bump Apache Commons Parent from 105 to 106 (#663)

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

garydgregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-configuration.git


The following commit(s) were added to refs/heads/master by this push:
     new 735011f5e Bump Apache Commons Parent from 105 to 106 (#663)
735011f5e is described below

commit 735011f5e8743f8014b728eb12198176188ae4a0
Author: Gary Gregory <ga...@users.noreply.github.com>
AuthorDate: Sat Oct 17 16:23:34 2026 -0400

    Bump Apache Commons Parent from 105 to 106 (#663)
    
    * Bump Apache Commons Parent from 105 to 106
    
    * Fix SpotBugs clone warnings and exclude intentional lock acquisition
    
    Avoid overridable setter and getter calls during cloning while
    preserving
    layout listener registration and independent builder parameters. Add
    subclass regression tests for both getters and setters.
    
    Exclude lock warnings for beginRead() and beginWrite(), which
    intentionally
    hold locks until the matching end methods.
---
 pom.xml                                            |  2 +-
 src/conf/findbugs-exclude-filter.xml               |  9 ++++
 .../configuration2/PropertiesConfiguration.java    |  2 +-
 .../combined/CombinedBuilderParametersImpl.java    |  2 +-
 .../combined/MultiFileBuilderParametersImpl.java   |  2 +-
 .../TestPropertiesConfiguration.java               | 23 +++++++++++
 .../TestCombinedBuilderParametersImpl.java         | 48 ++++++++++++++++++++++
 .../TestMultiFileBuilderParametersImpl.java        | 48 ++++++++++++++++++++++
 8 files changed, 132 insertions(+), 4 deletions(-)

diff --git a/pom.xml b/pom.xml
index 9a9964a7d..e7cd2e54d 100644
--- a/pom.xml
+++ b/pom.xml
@@ -20,7 +20,7 @@
   <parent>
     <groupId>org.apache.commons</groupId>
     <artifactId>commons-parent</artifactId>
-    <version>105</version>
+    <version>106</version>
   </parent>
   <modelVersion>4.0.0</modelVersion>
   <artifactId>commons-configuration2</artifactId>
diff --git a/src/conf/findbugs-exclude-filter.xml b/src/conf/findbugs-exclude-filter.xml
index 2165e2435..fba976cfb 100644
--- a/src/conf/findbugs-exclude-filter.xml
+++ b/src/conf/findbugs-exclude-filter.xml
@@ -112,4 +112,13 @@
     <Class name="~.*" />
     <Bug pattern="CT_CONSTRUCTOR_THROW" />
   </Match>  
+  <!-- Reason: Locks intentionally remain held until the matching endRead() or endWrite() call. -->
+  <Match>
+    <Class name="org.apache.commons.configuration2.sync.ReadWriteSynchronizer" />
+    <Or>
+      <Method name="beginRead" />
+      <Method name="beginWrite" />
+    </Or>
+    <Bug pattern="UL_UNRELEASED_LOCK" />
+  </Match>
 </FindBugsFilter>
diff --git a/src/main/java/org/apache/commons/configuration2/PropertiesConfiguration.java b/src/main/java/org/apache/commons/configuration2/PropertiesConfiguration.java
index f7050fd14..f546bbcff 100644
--- a/src/main/java/org/apache/commons/configuration2/PropertiesConfiguration.java
+++ b/src/main/java/org/apache/commons/configuration2/PropertiesConfiguration.java
@@ -1296,7 +1296,7 @@ public class PropertiesConfiguration extends BaseConfiguration implements FileBa
     public Object clone() {
         final PropertiesConfiguration copy = (PropertiesConfiguration) super.clone();
         if (layout != null) {
-            copy.setLayout(new PropertiesConfigurationLayout(layout));
+            copy.installLayout(new PropertiesConfigurationLayout(layout));
         }
         return copy;
     }
diff --git a/src/main/java/org/apache/commons/configuration2/builder/combined/CombinedBuilderParametersImpl.java b/src/main/java/org/apache/commons/configuration2/builder/combined/CombinedBuilderParametersImpl.java
index e0b62f3a8..886c05f9e 100644
--- a/src/main/java/org/apache/commons/configuration2/builder/combined/CombinedBuilderParametersImpl.java
+++ b/src/main/java/org/apache/commons/configuration2/builder/combined/CombinedBuilderParametersImpl.java
@@ -119,7 +119,7 @@ public class CombinedBuilderParametersImpl extends BasicBuilderParameters implem
     @Override
     public CombinedBuilderParametersImpl clone() {
         final CombinedBuilderParametersImpl copy = (CombinedBuilderParametersImpl) super.clone();
-        copy.setDefinitionBuilderParameters((BuilderParameters) ConfigurationUtils.cloneIfPossible(getDefinitionBuilderParameters()));
+        copy.definitionBuilderParameters = (BuilderParameters) ConfigurationUtils.cloneIfPossible(definitionBuilderParameters);
         return copy;
     }
 
diff --git a/src/main/java/org/apache/commons/configuration2/builder/combined/MultiFileBuilderParametersImpl.java b/src/main/java/org/apache/commons/configuration2/builder/combined/MultiFileBuilderParametersImpl.java
index 15052f21d..3ebf943cf 100644
--- a/src/main/java/org/apache/commons/configuration2/builder/combined/MultiFileBuilderParametersImpl.java
+++ b/src/main/java/org/apache/commons/configuration2/builder/combined/MultiFileBuilderParametersImpl.java
@@ -101,7 +101,7 @@ public class MultiFileBuilderParametersImpl extends BasicBuilderParameters imple
     @Override
     public MultiFileBuilderParametersImpl clone() {
         final MultiFileBuilderParametersImpl copy = (MultiFileBuilderParametersImpl) super.clone();
-        copy.setManagedBuilderParameters((BuilderParameters) ConfigurationUtils.cloneIfPossible(getManagedBuilderParameters()));
+        copy.managedBuilderParameters = (BuilderParameters) ConfigurationUtils.cloneIfPossible(managedBuilderParameters);
         return copy;
     }
 
diff --git a/src/test/java/org/apache/commons/configuration2/TestPropertiesConfiguration.java b/src/test/java/org/apache/commons/configuration2/TestPropertiesConfiguration.java
index 6ea513bc3..e49b3b50a 100644
--- a/src/test/java/org/apache/commons/configuration2/TestPropertiesConfiguration.java
+++ b/src/test/java/org/apache/commons/configuration2/TestPropertiesConfiguration.java
@@ -461,6 +461,29 @@ public class TestPropertiesConfiguration {
         assertEquals(outConf.toString(), outCopy.toString());
     }
 
+    /**
+     * Tests whether cloning a subclass installs an independent layout without invoking its setter.
+     */
+    @Test
+    void testCloneDoesNotCallOverriddenSetter() {
+        final PropertiesConfiguration original = new PropertiesConfiguration() {
+            @Override
+            public void setLayout(final PropertiesConfigurationLayout layout) {
+                throw new AssertionError("Cloning must not invoke the overridden setter");
+            }
+        };
+        original.addProperty("key", "original");
+        final PropertiesConfiguration copy = (PropertiesConfiguration) original.clone();
+        assertSame(original.getClass(), copy.getClass());
+        assertNotSame(original.getLayout(), copy.getLayout());
+        assertEquals(1, copy.getEventListeners(ConfigurationEvent.ANY).size());
+        assertSame(copy.getLayout(), copy.getEventListeners(ConfigurationEvent.ANY).iterator().next());
+        copy.addProperty("copyOnly", "value");
+        assertTrue(copy.getLayout().getKeys().contains("copyOnly"));
+        assertFalse(original.getLayout().getKeys().contains("copyOnly"));
+        assertFalse(original.containsKey("copyOnly"));
+    }
+
     /**
      * Tests the clone() method when no layout object exists yet.
      */
diff --git a/src/test/java/org/apache/commons/configuration2/builder/combined/TestCombinedBuilderParametersImpl.java b/src/test/java/org/apache/commons/configuration2/builder/combined/TestCombinedBuilderParametersImpl.java
index 7609ca4a4..849c1cc7a 100644
--- a/src/test/java/org/apache/commons/configuration2/builder/combined/TestCombinedBuilderParametersImpl.java
+++ b/src/test/java/org/apache/commons/configuration2/builder/combined/TestCombinedBuilderParametersImpl.java
@@ -76,6 +76,54 @@ public class TestCombinedBuilderParametersImpl {
                 clone.getDefinitionBuilderParameters().getParameters().get("systemID"));
     }
 
+    /**
+     * Tests whether cloning a subclass copies stored parameters without invoking its getter.
+     */
+    @Test
+    void testCloneDoesNotCallOverriddenGetter() {
+        final int[] getterCalls = {0};
+        final XMLBuilderParametersImpl nested = new XMLBuilderParametersImpl();
+        nested.setSystemID("testSystemID");
+        final CombinedBuilderParametersImpl original = new CombinedBuilderParametersImpl() {
+            @Override
+            public BuilderParameters getDefinitionBuilderParameters() {
+                getterCalls[0]++;
+                return super.getDefinitionBuilderParameters();
+            }
+        };
+        original.setDefinitionBuilderParameters(nested);
+        final CombinedBuilderParametersImpl copy = original.clone();
+        assertEquals(0, getterCalls[0]);
+        assertSame(original.getClass(), copy.getClass());
+        assertSame(nested, original.getDefinitionBuilderParameters());
+        assertNotSame(nested, copy.getDefinitionBuilderParameters());
+        assertEquals("testSystemID", copy.getDefinitionBuilderParameters().getParameters().get("systemID"));
+    }
+
+    /**
+     * Tests whether cloning a subclass copies nested parameters without invoking its setter.
+     */
+    @Test
+    void testCloneDoesNotCallOverriddenSetter() {
+        final XMLBuilderParametersImpl nested = new XMLBuilderParametersImpl();
+        nested.setSystemID("testSystemID");
+        final CombinedBuilderParametersImpl original = new CombinedBuilderParametersImpl() {
+            {
+                super.setDefinitionBuilderParameters(nested);
+            }
+
+            @Override
+            public CombinedBuilderParametersImpl setDefinitionBuilderParameters(final BuilderParameters parameters) {
+                throw new AssertionError("Cloning must not invoke the overridden setter");
+            }
+        };
+        final CombinedBuilderParametersImpl copy = original.clone();
+        assertSame(original.getClass(), copy.getClass());
+        assertSame(nested, original.getDefinitionBuilderParameters());
+        assertNotSame(nested, copy.getDefinitionBuilderParameters());
+        assertEquals("testSystemID", copy.getDefinitionBuilderParameters().getParameters().get("systemID"));
+    }
+
     /**
      * Tests whether a new instance can be created if none is found in the parameters map.
      */
diff --git a/src/test/java/org/apache/commons/configuration2/builder/combined/TestMultiFileBuilderParametersImpl.java b/src/test/java/org/apache/commons/configuration2/builder/combined/TestMultiFileBuilderParametersImpl.java
index 68a4be865..65c138e0a 100644
--- a/src/test/java/org/apache/commons/configuration2/builder/combined/TestMultiFileBuilderParametersImpl.java
+++ b/src/test/java/org/apache/commons/configuration2/builder/combined/TestMultiFileBuilderParametersImpl.java
@@ -78,6 +78,54 @@ public class TestMultiFileBuilderParametersImpl {
                 ((FileBasedBuilderParametersImpl) clone.getManagedBuilderParameters()).getFileHandler().getFileName());
     }
 
+    /**
+     * Tests whether cloning a subclass copies stored parameters without invoking its getter.
+     */
+    @Test
+    void testCloneDoesNotCallOverriddenGetter() {
+        final int[] getterCalls = {0};
+        final FileBasedBuilderParametersImpl nested = new FileBasedBuilderParametersImpl();
+        nested.setFileName("test.xml");
+        final MultiFileBuilderParametersImpl original = new MultiFileBuilderParametersImpl() {
+            @Override
+            public BuilderParameters getManagedBuilderParameters() {
+                getterCalls[0]++;
+                return super.getManagedBuilderParameters();
+            }
+        };
+        original.setManagedBuilderParameters(nested);
+        final MultiFileBuilderParametersImpl copy = original.clone();
+        assertEquals(0, getterCalls[0]);
+        assertSame(original.getClass(), copy.getClass());
+        assertSame(nested, original.getManagedBuilderParameters());
+        assertNotSame(nested, copy.getManagedBuilderParameters());
+        assertEquals("test.xml", ((FileBasedBuilderParametersImpl) copy.getManagedBuilderParameters()).getFileHandler().getFileName());
+    }
+
+    /**
+     * Tests whether cloning a subclass copies nested parameters without invoking its setter.
+     */
+    @Test
+    void testCloneDoesNotCallOverriddenSetter() {
+        final FileBasedBuilderParametersImpl nested = new FileBasedBuilderParametersImpl();
+        nested.setFileName("test.xml");
+        final MultiFileBuilderParametersImpl original = new MultiFileBuilderParametersImpl() {
+            {
+                super.setManagedBuilderParameters(nested);
+            }
+
+            @Override
+            public MultiFileBuilderParametersImpl setManagedBuilderParameters(final BuilderParameters parameters) {
+                throw new AssertionError("Cloning must not invoke the overridden setter");
+            }
+        };
+        final MultiFileBuilderParametersImpl copy = original.clone();
+        assertSame(original.getClass(), copy.getClass());
+        assertSame(nested, original.getManagedBuilderParameters());
+        assertNotSame(nested, copy.getManagedBuilderParameters());
+        assertEquals("test.xml", ((FileBasedBuilderParametersImpl) copy.getManagedBuilderParameters()).getFileHandler().getFileName());
+    }
+
     /**
      * Tests whether an instance can be obtained from a parameters map.
      */