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 2014/09/09 19:55:59 UTC
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
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);
+ }
}