commons-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ohe...@apache.org
Subject svn commit: r711782 - in /commons/proper/configuration/trunk: src/java/org/apache/commons/configuration/AbstractFileConfiguration.java src/test/org/apache/commons/configuration/TestFileConfiguration.java xdocs/changes.xml
Date Thu, 06 Nov 2008 07:23:41 GMT
Author: oheger
Date: Wed Nov  5 23:23:41 2008
New Revision: 711782

URL: http://svn.apache.org/viewvc?rev=711782&view=rev
Log:
CONFIGURATION-347: The Iterator returned by AbstractConfiguration.getKeys() now points to
a snapshot of the keys. This prevents ConcurrentModificationExceptions during iteration when
a reload happens.

Modified:
    commons/proper/configuration/trunk/src/java/org/apache/commons/configuration/AbstractFileConfiguration.java
    commons/proper/configuration/trunk/src/test/org/apache/commons/configuration/TestFileConfiguration.java
    commons/proper/configuration/trunk/xdocs/changes.xml

Modified: commons/proper/configuration/trunk/src/java/org/apache/commons/configuration/AbstractFileConfiguration.java
URL: http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/java/org/apache/commons/configuration/AbstractFileConfiguration.java?rev=711782&r1=711781&r2=711782&view=diff
==============================================================================
--- commons/proper/configuration/trunk/src/java/org/apache/commons/configuration/AbstractFileConfiguration.java
(original)
+++ commons/proper/configuration/trunk/src/java/org/apache/commons/configuration/AbstractFileConfiguration.java
Wed Nov  5 23:23:41 2008
@@ -32,6 +32,8 @@
 import java.net.URL;
 import java.net.URLConnection;
 import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.List;
 
 import org.apache.commons.configuration.reloading.InvariantReloadingStrategy;
 import org.apache.commons.configuration.reloading.ReloadingStrategy;
@@ -40,7 +42,7 @@
 
 /**
  * <p>Partial implementation of the <code>FileConfiguration</code> interface.
- * Developpers of file based configuration may want to extend this class,
+ * Developers of file based configuration may want to extend this class,
  * the two methods left to implement are <code>{@link FileConfiguration#load(Reader)}</code>
  * and <code>{@link FileConfiguration#save(Writer)}</code>.</p>
  * <p>This base class already implements a couple of ways to specify the location
@@ -934,10 +936,39 @@
         return super.containsKey(key);
     }
 
+    /**
+     * Returns an <code>Iterator</code> with the keys contained in this
+     * configuration. This implementation performs a reload if necessary before
+     * obtaining the keys. The <code>Iterator</code> returned by this method
+     * points to a snapshot taken when this method was called. Later changes at
+     * the set of keys (including those caused by a reload) won't be visible.
+     * This is because a reload can happen at any time during iteration, and it
+     * is impossible to determine how this reload affects the current iteration.
+     * When using the iterator a client has to be aware that changes of the
+     * configuration are possible at any time. For instance, if after a reload
+     * operation some keys are no longer present, the iterator will still return
+     * those keys because they were found when it was created.
+     *
+     * @return an <code>Iterator</code> with the keys of this configuration
+     */
     public Iterator getKeys()
     {
         reload();
-        return super.getKeys();
+        List keyList = new LinkedList();
+        enterNoReload();
+        try
+        {
+            for (Iterator it = super.getKeys(); it.hasNext();)
+            {
+                keyList.add(it.next());
+            }
+
+            return keyList.iterator();
+        }
+        finally
+        {
+            exitNoReload();
+        }
     }
 
     /**

Modified: commons/proper/configuration/trunk/src/test/org/apache/commons/configuration/TestFileConfiguration.java
URL: http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/test/org/apache/commons/configuration/TestFileConfiguration.java?rev=711782&r1=711781&r2=711782&view=diff
==============================================================================
--- commons/proper/configuration/trunk/src/test/org/apache/commons/configuration/TestFileConfiguration.java
(original)
+++ commons/proper/configuration/trunk/src/test/org/apache/commons/configuration/TestFileConfiguration.java
Wed Nov  5 23:23:41 2008
@@ -17,20 +17,21 @@
 
 package org.apache.commons.configuration;
 
-import java.net.URL;
-import java.util.Properties;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.FileWriter;
 import java.io.IOException;
 import java.io.PrintWriter;
+import java.net.URL;
+import java.util.Iterator;
+import java.util.Properties;
+
+import junit.framework.TestCase;
 
 import org.apache.commons.configuration.reloading.FileAlwaysReloadingStrategy;
 import org.apache.commons.configuration.reloading.FileChangedReloadingStrategy;
 
-import junit.framework.TestCase;
-
 /**
  * @author Emmanuel Bourg
  * @version $Revision$, $Date$
@@ -426,7 +427,7 @@
     }
 
     /**
-     * Tests to invoke save() without explicitely setting a file name. This
+     * Tests to invoke save() without explicitly setting a file name. This
      * will cause an exception.
      */
     public void testSaveWithoutFileName() throws Exception
@@ -590,6 +591,44 @@
     }
 
     /**
+     * Tests iterating over the keys of a non hierarchical file-based
+     * configuration while a reload happens. This test is related to
+     * CONFIGURATION-347.
+     */
+    public void testIterationWithReloadFlat() throws ConfigurationException
+    {
+        PropertiesConfiguration config = new PropertiesConfiguration(TEST_FILE);
+        checkIterationWithReload(config);
+    }
+
+    /**
+     * Tests iterating over the keys of a hierarchical file-based configuration
+     * while a reload happens. This test is related to CONFIGURATION-347.
+     */
+    public void testIterationWithReloadHierarchical()
+            throws ConfigurationException
+    {
+        XMLConfiguration config = new XMLConfiguration("test.xml");
+        checkIterationWithReload(config);
+    }
+
+    /**
+     * Helper method for testing an iteration over the keys of a file-based
+     * configuration while a reload happens.
+     *
+     * @param config the configuration to test
+     */
+    private void checkIterationWithReload(FileConfiguration config)
+    {
+        config.setReloadingStrategy(new FileAlwaysReloadingStrategy());
+        for (Iterator it = config.getKeys(); it.hasNext();)
+        {
+            String key = (String) it.next();
+            assertNotNull("No value for key " + key, config.getProperty(key));
+        }
+    }
+
+    /**
      * Helper method for comparing the content of two configuration objects.
      *
      * @param config1 the first configuration

Modified: commons/proper/configuration/trunk/xdocs/changes.xml
URL: http://svn.apache.org/viewvc/commons/proper/configuration/trunk/xdocs/changes.xml?rev=711782&r1=711781&r2=711782&view=diff
==============================================================================
--- commons/proper/configuration/trunk/xdocs/changes.xml (original)
+++ commons/proper/configuration/trunk/xdocs/changes.xml Wed Nov  5 23:23:41 2008
@@ -23,6 +23,12 @@
 
   <body>
     <release version="1.6" date="in SVN" description="">
+      <action dev="oheger" type="fix" issue="CONFIGURATION-347">
+        AbstractFileConfiguration.getKeys() now returns an iterator that points
+        to a snapshot of the keys of the configuration. This prevents
+        ConcurrentModificationExceptions during iteration when a reload is
+        performed.
+      </action>
       <action dev="oheger" type="fix" issue="CONFIGURATION-346">
         ConfigurationUtils.convertToHierarchical() now creates multiple
         configuration nodes for properties with multiple values. This



Mime
View raw message