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/15 22:06:45 UTC

svn commit: r1483061 - in /commons/proper/configuration/trunk/src: main/java/org/apache/commons/configuration/CompositeConfiguration.java test/java/org/apache/commons/configuration/TestCompositeConfiguration.java

Author: oheger
Date: Wed May 15 20:06:45 2013
New Revision: 1483061

URL: http://svn.apache.org/r1483061
Log:
Reworked CompositeConfiguration regarding thread-safety.

The methods for accessing the list of child configurations and the in-memory
configuration now call the Synchronizer correctly. Updated Javadoc regarding
thread-safety.

Modified:
    commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/CompositeConfiguration.java
    commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestCompositeConfiguration.java

Modified: commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/CompositeConfiguration.java
URL: http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/CompositeConfiguration.java?rev=1483061&r1=1483060&r2=1483061&view=diff
==============================================================================
--- commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/CompositeConfiguration.java (original)
+++ commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/CompositeConfiguration.java Wed May 15 20:06:45 2013
@@ -47,6 +47,18 @@ import java.util.Set;
  * configurations at the position it was added, i.e. its priority for property
  * queries can be defined by adding the child configurations in the correct
  * order.</p>
+ * <p>
+ * This configuration class uses a {@code Synchronizer} to control concurrent
+ * access. While all methods for reading and writing configuration properties
+ * make use of this {@code Synchronizer} per default, the methods for managing
+ * the list of child configurations and the in-memory configuration
+ * ({@code addConfiguration(), getNumberOfConfigurations(), removeConfiguration(),
+ * getConfiguration(), getInMemoryConfiguration()}) are guarded, too. Because
+ * most methods for accessing configuration data delegate to the list of child
+ * configurations, the thread-safety of a {@code CompositeConfiguration}
+ * object also depends on the {@code Synchronizer} objects used by these
+ * children.
+ * </p>
  *
  * @author <a href="mailto:epugh@upstate.com">Eric Pugh</a>
  * @author <a href="mailto:hps@intermeta.de">Henning P. Schmiedehausen</a>
@@ -158,37 +170,44 @@ implements Cloneable
      */
     public void addConfiguration(Configuration config, boolean asInMemory)
     {
-        if (!configList.contains(config))
+        beginWrite();
+        try
         {
-            if (asInMemory)
+            if (!configList.contains(config))
             {
-                replaceInMemoryConfiguration(config);
-                inMemoryConfigIsChild = true;
-            }
+                if (asInMemory)
+                {
+                    replaceInMemoryConfiguration(config);
+                    inMemoryConfigIsChild = true;
+                }
 
-            if (!inMemoryConfigIsChild)
-            {
-                // As the inMemoryConfiguration contains all manually added
-                // keys, we must make sure that it is always last. "Normal", non
-                // composed configurations add their keys at the end of the
-                // configuration and we want to mimic this behavior.
-                configList.add(configList.indexOf(inMemoryConfiguration),
-                        config);
-            }
-            else
-            {
-                // However, if the in-memory configuration is a regular child,
-                // only the order in which child configurations are added is
-                // relevant
-                configList.add(config);
-            }
+                if (!inMemoryConfigIsChild)
+                {
+                    // As the inMemoryConfiguration contains all manually added
+                    // keys, we must make sure that it is always last. "Normal", non
+                    // composed configurations add their keys at the end of the
+                    // configuration and we want to mimic this behavior.
+                    configList.add(configList.indexOf(inMemoryConfiguration),
+                            config);
+                }
+                else
+                {
+                    // However, if the in-memory configuration is a regular child,
+                    // only the order in which child configurations are added is relevant
+                    configList.add(config);
+                }
 
-            if (config instanceof AbstractConfiguration)
-            {
-                ((AbstractConfiguration) config)
-                        .setThrowExceptionOnMissing(isThrowExceptionOnMissing());
+                if (config instanceof AbstractConfiguration)
+                {
+                    ((AbstractConfiguration) config)
+                            .setThrowExceptionOnMissing(isThrowExceptionOnMissing());
+                }
             }
         }
+        finally
+        {
+            endWrite();
+        }
     }
 
     /**
@@ -198,11 +217,19 @@ implements Cloneable
      */
     public void removeConfiguration(Configuration config)
     {
-        // Make sure that you can't remove the inMemoryConfiguration from
-        // the CompositeConfiguration object
-        if (!config.equals(inMemoryConfiguration))
+        beginWrite();
+        try
         {
-            configList.remove(config);
+            // Make sure that you can't remove the inMemoryConfiguration from
+            // the CompositeConfiguration object
+            if (!config.equals(inMemoryConfiguration))
+            {
+                configList.remove(config);
+            }
+        }
+        finally
+        {
+            endWrite();
         }
     }
 
@@ -213,7 +240,15 @@ implements Cloneable
      */
     public int getNumberOfConfigurations()
     {
-        return configList.size();
+        beginRead();
+        try
+        {
+            return configList.size();
+        }
+        finally
+        {
+            endRead();
+        }
     }
 
     /**
@@ -399,7 +434,15 @@ implements Cloneable
      */
     public Configuration getConfiguration(int index)
     {
-        return configList.get(index);
+        beginRead();
+        try
+        {
+            return configList.get(index);
+        }
+        finally
+        {
+            endRead();
+        }
     }
 
     /**
@@ -410,7 +453,15 @@ implements Cloneable
      */
     public Configuration getInMemoryConfiguration()
     {
-        return inMemoryConfiguration;
+        beginRead();
+        try
+        {
+            return inMemoryConfiguration;
+        }
+        finally
+        {
+            endRead();
+        }
     }
 
     /**

Modified: commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestCompositeConfiguration.java
URL: http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestCompositeConfiguration.java?rev=1483061&r1=1483060&r2=1483061&view=diff
==============================================================================
--- commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestCompositeConfiguration.java (original)
+++ commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestCompositeConfiguration.java Wed May 15 20:06:45 2013
@@ -33,6 +33,7 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.NoSuchElementException;
 
+import org.apache.commons.configuration.SynchronizerTestImpl.Methods;
 import org.apache.commons.configuration.event.ConfigurationEvent;
 import org.apache.commons.configuration.event.ConfigurationListener;
 import org.apache.commons.configuration.io.FileHandler;
@@ -41,7 +42,7 @@ import org.junit.Before;
 import org.junit.Test;
 
 /**
- * Test loading multiple configurations.
+ * Test class for {@code CompositeConfiguration}.
  *
  * @version $Id$
  */
@@ -873,6 +874,77 @@ public class TestCompositeConfiguration
     }
 
     /**
+     * Creates a test synchronizer and installs it at the test configuration.
+     *
+     * @return the test synchronizer
+     */
+    private SynchronizerTestImpl installSynchronizer()
+    {
+        cc.addConfiguration(conf1);
+        cc.addConfiguration(conf2);
+        SynchronizerTestImpl sync = new SynchronizerTestImpl();
+        cc.setSynchronizer(sync);
+        return sync;
+    }
+
+    /**
+     * Tests whether adding a child configuration is synchronized.
+     */
+    @Test
+    public void testAddConfigurationSynchronized()
+    {
+        SynchronizerTestImpl sync = installSynchronizer();
+        cc.addConfiguration(xmlConf);
+        sync.verify(Methods.BEGIN_WRITE, Methods.END_WRITE);
+    }
+
+    /**
+     * Tests whether removing a child configuration is synchronized.
+     */
+    @Test
+    public void testRemoveConfigurationSynchronized()
+    {
+        SynchronizerTestImpl sync = installSynchronizer();
+        cc.removeConfiguration(conf1);
+        sync.verify(Methods.BEGIN_WRITE, Methods.END_WRITE);
+    }
+
+    /**
+     * Tests whether access to a configuration by index is synchronized.
+     */
+    @Test
+    public void testGetConfigurationSynchronized()
+    {
+        SynchronizerTestImpl sync = installSynchronizer();
+        assertEquals("Wrong result", conf1, cc.getConfiguration(0));
+        sync.verify(Methods.BEGIN_READ, Methods.END_READ);
+    }
+
+    /**
+     * Tests whether access to the in-memory configuration is synchronized.
+     */
+    @Test
+    public void testGetInMemoryConfigurationSynchronized()
+    {
+        SynchronizerTestImpl sync = installSynchronizer();
+        cc.getInMemoryConfiguration();
+        sync.verify(Methods.BEGIN_READ, Methods.END_READ);
+    }
+
+    /**
+     * Tests whether querying the number of child configurations is
+     * synchronized.
+     */
+    @Test
+    public void testGetNumberOfConfigurationsSynchronized()
+    {
+        SynchronizerTestImpl sync = installSynchronizer();
+        assertEquals("Wrong number of configurations", 3,
+                cc.getNumberOfConfigurations());
+        sync.verify(Methods.BEGIN_READ, Methods.END_READ);
+    }
+
+    /**
      * A test configuration event listener that counts the number of received
      * events. Used for testing the event facilities.
      */