You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by oh...@apache.org on 2013/05/22 22:10:47 UTC

svn commit: r1485390 - in /commons/proper/configuration/trunk/src: main/java/org/apache/commons/configuration/BaseHierarchicalConfiguration.java test/java/org/apache/commons/configuration/TestBaseHierarchicalConfigurationSynchronization.java

Author: oheger
Date: Wed May 22 20:10:47 2013
New Revision: 1485390

URL: http://svn.apache.org/r1485390
Log:
Ensure proper synchronization of BaseHierarchicalConfiguration.subset() method.

Modified:
    commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/BaseHierarchicalConfiguration.java
    commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestBaseHierarchicalConfigurationSynchronization.java

Modified: commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/BaseHierarchicalConfiguration.java
URL: http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/BaseHierarchicalConfiguration.java?rev=1485390&r1=1485389&r2=1485390&view=diff
==============================================================================
--- commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/BaseHierarchicalConfiguration.java (original)
+++ commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/BaseHierarchicalConfiguration.java Wed May 22 20:10:47 2013
@@ -541,7 +541,8 @@ public class BaseHierarchicalConfigurati
      * values are merged together). Note that the returned
      * {@code Configuration} object is not connected to its source
      * configuration: updates on the source configuration are not reflected in
-     * the subset and vice versa.
+     * the subset and vice versa. The returned configuration uses the same
+     * {@code Synchronizer} as this configuration.
      *
      * @param prefix the prefix of the keys for the subset
      * @return a new configuration object representing the selected subset
@@ -549,52 +550,70 @@ public class BaseHierarchicalConfigurati
     @Override
     public Configuration subset(String prefix)
     {
-        Collection<ConfigurationNode> nodes = fetchNodeList(prefix);
-        if (nodes.isEmpty())
-        {
-            return new BaseHierarchicalConfiguration();
-        }
-
-        final BaseHierarchicalConfiguration parent = this;
-        BaseHierarchicalConfiguration result = new BaseHierarchicalConfiguration()
+        beginRead();
+        try
         {
-            // Override interpolate to always interpolate on the parent
-            @Override
-            protected Object interpolate(Object value)
+            Collection<ConfigurationNode> nodes = fetchNodeList(prefix);
+            if (nodes.isEmpty())
             {
-                return parent.interpolate(value);
+                return new BaseHierarchicalConfiguration();
             }
-        };
-        CloneVisitor visitor = new CloneVisitor();
 
-        // Initialize the new root node
-        Object value = null;
-        int valueCount = 0;
-        for (ConfigurationNode nd : nodes)
-        {
-            if (nd.getValue() != null)
+            final BaseHierarchicalConfiguration parent = this;
+            BaseHierarchicalConfiguration result =
+                    new BaseHierarchicalConfiguration()
+                    {
+                        // Override interpolate to always interpolate on the parent
+                        @Override
+                        protected Object interpolate(Object value)
+                        {
+                            return parent.interpolate(value);
+                        }
+                    };
+            CloneVisitor visitor = new CloneVisitor();
+
+            // Initialize the new root node
+            Object value = null;
+            int valueCount = 0;
+            for (ConfigurationNode nd : nodes)
             {
-                value = nd.getValue();
-                valueCount++;
+                if (nd.getValue() != null)
+                {
+                    value = nd.getValue();
+                    valueCount++;
+                }
+                nd.visit(visitor);
+
+                for (ConfigurationNode c : visitor.getClone().getChildren())
+                {
+                    result.getRootNode().addChild(c);
+                }
+                for (ConfigurationNode attr : visitor.getClone()
+                        .getAttributes())
+                {
+                    result.getRootNode().addAttribute(attr);
+                }
             }
-            nd.visit(visitor);
 
-            for (ConfigurationNode c : visitor.getClone().getChildren())
+            // Determine the value of the new root
+            if (valueCount == 1)
+            {
+                result.getRootNode().setValue(value);
+            }
+            if (result.isEmpty())
             {
-                result.getRootNode().addChild(c);
+                return new BaseHierarchicalConfiguration();
             }
-            for (ConfigurationNode attr : visitor.getClone().getAttributes())
+            else
             {
-                result.getRootNode().addAttribute(attr);
+                result.setSynchronizer(getSynchronizer());
+                return result;
             }
         }
-
-        // Determine the value of the new root
-        if (valueCount == 1)
+        finally
         {
-            result.getRootNode().setValue(value);
+            endRead();
         }
-        return (result.isEmpty()) ? new BaseHierarchicalConfiguration() : result;
     }
 
     /**

Modified: commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestBaseHierarchicalConfigurationSynchronization.java
URL: http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestBaseHierarchicalConfigurationSynchronization.java?rev=1485390&r1=1485389&r2=1485390&view=diff
==============================================================================
--- commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestBaseHierarchicalConfigurationSynchronization.java (original)
+++ commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestBaseHierarchicalConfigurationSynchronization.java Wed May 22 20:10:47 2013
@@ -21,6 +21,7 @@ import static org.junit.Assert.assertFal
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 
 import java.util.Collections;
@@ -251,4 +252,15 @@ public class TestBaseHierarchicalConfigu
                 Methods.BEGIN_WRITE, Methods.END_WRITE, Methods.BEGIN_READ,
                 Methods.END_READ);
     }
+
+    /**
+     * Tests whether subset() is correctly synchronized.
+     */
+    @Test
+    public void testSubsetSynchronized()
+    {
+        Configuration subset = config.subset("test");
+        sync.verify(Methods.BEGIN_READ, Methods.END_READ);
+        assertSame("Wrong Synchronizer", sync, subset.getSynchronizer());
+    }
 }