svn commit: r1775739 - in /commons/proper/configuration/trunk/src: main/java/org/apache/commons/configuration2/PropertiesConfigurationLayout.java test/java/org/apache/commons/configuration2/builder/TestFileBasedConfigurationBuilder.java

[oheger]

Author: oheger
Date: Thu Dec 22 21:03:57 2016
New Revision: 1775739

URL: http://svn.apache.org/viewvc?rev=1775739&view=rev
Log:
[CONFIGURATION-646] Changed handling of loads in layout.

PropertiesConfigurationLayout used to remove its event listener
registration when a load operation starts and add it again
afterwards. This caused problems with the auto-save mechanism
because update events were processed by the layout after the listener
for auto-save was triggered, resulting in incomplete configuration
files.

Now the listener is not removed, but temporarily disabled.

Modified:
    
commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration2/PropertiesConfigurationLayout.java
    
commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration2/builder/TestFileBasedConfigurationBuilder.java

Modified: 
commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration2/PropertiesConfigurationLayout.java
URL: 
http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration2/PropertiesConfigurationLayout.java?rev=1775739&r1=1775738&r2=1775739&view=diff
==============================================================================
--- 
commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration2/PropertiesConfigurationLayout.java
 (original)
+++ 
commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration2/PropertiesConfigurationLayout.java
 Thu Dec 22 21:03:57 2016
@@ -23,6 +23,7 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
 
 import org.apache.commons.configuration2.event.ConfigurationEvent;
 import org.apache.commons.configuration2.event.EventListener;
@@ -133,7 +134,7 @@ public class PropertiesConfigurationLayo
     private String lineSeparator;
 
     /** A counter for determining nested load calls. */
-    private int loadCounter;
+    private final AtomicInteger loadCounter;
 
     /** Stores the force single line flag. */
     private boolean forceSingleLine;
@@ -154,6 +155,7 @@ public class PropertiesConfigurationLayo
      */
     public PropertiesConfigurationLayout(PropertiesConfigurationLayout c)
     {
+        loadCounter = new AtomicInteger();
         layoutData = new LinkedHashMap<String, PropertyLayoutData>();
 
         if (c != null)
@@ -476,10 +478,7 @@ public class PropertiesConfigurationLayo
     public void load(PropertiesConfiguration config, Reader in)
             throws ConfigurationException
     {
-        if (++loadCounter == 1)
-        {
-            config.removeEventListener(ConfigurationEvent.ANY, this);
-        }
+        loadCounter.incrementAndGet();
         PropertiesConfiguration.PropertiesReader reader =
                 config.getIOFactory().createPropertiesReader(in);
 
@@ -527,10 +526,7 @@ public class PropertiesConfigurationLayo
         }
         finally
         {
-            if (--loadCounter == 0)
-            {
-                config.addEventListener(ConfigurationEvent.ANY, this);
-            }
+            loadCounter.decrementAndGet();
         }
     }
 
@@ -601,7 +597,7 @@ public class PropertiesConfigurationLayo
     @Override
     public void onEvent(ConfigurationEvent event)
     {
-        if (!event.isBeforeUpdate())
+        if (!event.isBeforeUpdate() && loadCounter.get() == 0)
         {
             if (ConfigurationEvent.ADD_PROPERTY.equals(event.getEventType()))
             {
@@ -796,7 +792,7 @@ public class PropertiesConfigurationLayo
      */
     private int checkHeaderComment(List<String> commentLines)
     {
-        if (loadCounter == 1 && layoutData.isEmpty())
+        if (loadCounter.get() == 1 && layoutData.isEmpty())
         {
             // This is the first comment. Search for blanc lines.
             int index = commentLines.size() - 1;

Modified: 
commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration2/builder/TestFileBasedConfigurationBuilder.java
URL: 
http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration2/builder/TestFileBasedConfigurationBuilder.java?rev=1775739&r1=1775738&r2=1775739&view=diff
==============================================================================
--- 
commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration2/builder/TestFileBasedConfigurationBuilder.java
 (original)
+++ 
commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration2/builder/TestFileBasedConfigurationBuilder.java
 Thu Dec 22 21:03:57 2016
@@ -409,6 +409,26 @@ public class TestFileBasedConfigurationB
     }
 
     /**
+     * Tests whether auto save mode works with a properties configuration.
+     * This is related to CONFIGURATION-646.
+     */
+    @Test
+    public void testAutoSaveWithPropertiesConfiguration() throws 
ConfigurationException,
+            IOException
+    {
+        File file = folder.newFile();
+        FileBasedConfigurationBuilder<PropertiesConfiguration> builder =
+                new FileBasedConfigurationBuilder<PropertiesConfiguration>(
+                        PropertiesConfiguration.class)
+                        .configure(new FileBasedBuilderParametersImpl()
+                                .setFile(file));
+        builder.setAutoSave(true);
+        PropertiesConfiguration config = builder.getConfiguration();
+        config.setProperty(PROP, 1);
+        checkSavedConfig(file, 1);
+    }
+
+    /**
      * Tries to set a default encoding for a null class.
      */
     @Test(expected = IllegalArgumentException.class)