svn commit: r1623861 - in /commons/proper/configuration/trunk/src: main/java/org/apache/commons/configuration/AbstractConfiguration.java test/java/org/apache/commons/configuration/TestAbstractConfigurationSynchronization.java

[oheger]

Author: oheger
Date: Tue Sep  9 17:55:59 2014
New Revision: 1623861

URL: http://svn.apache.org/r1623861
Log:
Added synchronization to append() and copy() methods.

The source configuration is locked to avoid potential
concurrent modification exceptions while iterating over the key set.

Modified:
    
commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/AbstractConfiguration.java
    
commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestAbstractConfigurationSynchronization.java

Modified: 
commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/AbstractConfiguration.java
URL: 
http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/AbstractConfiguration.java?rev=1623861&r1=1623860&r2=1623861&view=diff
==============================================================================
--- 
commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/AbstractConfiguration.java
 (original)
+++ 
commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/AbstractConfiguration.java
 Tue Sep  9 17:55:59 2014
@@ -1492,11 +1492,19 @@ public abstract class AbstractConfigurat
     {
         if (c != null)
         {
-            for (Iterator<String> it = c.getKeys(); it.hasNext();)
+            c.lock(LockMode.READ);
+            try
             {
-                String key = it.next();
-                Object value = encodeForCopy(c.getProperty(key));
-                setProperty(key, value);
+                for (Iterator<String> it = c.getKeys(); it.hasNext();)
+                {
+                    String key = it.next();
+                    Object value = encodeForCopy(c.getProperty(key));
+                    setProperty(key, value);
+                }
+            }
+            finally
+            {
+                c.unlock(LockMode.READ);
             }
         }
     }
@@ -1522,11 +1530,19 @@ public abstract class AbstractConfigurat
     {
         if (c != null)
         {
-            for (Iterator<String> it = c.getKeys(); it.hasNext();)
+            c.lock(LockMode.READ);
+            try
+            {
+                for (Iterator<String> it = c.getKeys(); it.hasNext();)
+                {
+                    String key = it.next();
+                    Object value = encodeForCopy(c.getProperty(key));
+                    addProperty(key, value);
+                }
+            }
+            finally
             {
-                String key = it.next();
-                Object value = encodeForCopy(c.getProperty(key));
-                addProperty(key, value);
+                c.unlock(LockMode.READ);
             }
         }
     }

Modified: 
commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestAbstractConfigurationSynchronization.java
URL: 
http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestAbstractConfigurationSynchronization.java?rev=1623861&r1=1623860&r2=1623861&view=diff
==============================================================================
--- 
commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestAbstractConfigurationSynchronization.java
 (original)
+++ 
commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestAbstractConfigurationSynchronization.java
 Tue Sep  9 17:55:59 2014
@@ -21,10 +21,13 @@ import static org.junit.Assert.assertFal
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 
+import java.util.Collections;
+
 import org.apache.commons.configuration.SynchronizerTestImpl.Methods;
 import org.apache.commons.configuration.io.FileHandler;
 import org.apache.commons.configuration.sync.LockMode;
 import org.apache.commons.configuration.sync.NoOpSynchronizer;
+import org.easymock.EasyMock;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -223,4 +226,42 @@ public class TestAbstractConfigurationSy
         assertEquals("Wrong synchronizer for subset",
                 NoOpSynchronizer.INSTANCE, subset.getSynchronizer());
     }
+
+    /**
+     * Prepares a mock configuration for a copy operation.
+     *
+     * @return the mock configuration
+     */
+    private static Configuration prepareConfigurationMockForCopy()
+    {
+        Configuration config2 = EasyMock.createStrictMock(Configuration.class);
+        config2.lock(LockMode.READ);
+        EasyMock.expect(config2.getKeys()).andReturn(
+                Collections.<String> emptySet().iterator());
+        config2.unlock(LockMode.READ);
+        EasyMock.replay(config2);
+        return config2;
+    }
+
+    /**
+     * Tests whether the append() method uses synchronization.
+     */
+    @Test
+    public void testAppendSynchronized()
+    {
+        Configuration config2 = prepareConfigurationMockForCopy();
+        config.append(config2);
+        EasyMock.verify(config2);
+    }
+
+    /**
+     * Tests whether the copy() method uses synchronization.
+     */
+    @Test
+    public void testCopySynchronized()
+    {
+        Configuration config2 = prepareConfigurationMockForCopy();
+        config.copy(config2);
+        EasyMock.verify(config2);
+    }
 }