commons-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ohe...@apache.org
Subject svn commit: r1479366 - in /commons/proper/configuration/trunk/src: main/java/org/apache/commons/configuration/AbstractConfiguration.java test/java/org/apache/commons/configuration/TestAbstractConfigurationSynchronization.java
Date Sun, 05 May 2013 20:10:26 GMT
Author: oheger
Date: Sun May  5 20:10:26 2013
New Revision: 1479366

URL: http://svn.apache.org/r1479366
Log:
Initial integration of Synchronizer into AbstractConfiguration.

This is not yet complete. Currently, methods of AbstractConfiguration calling
the Synchronizer can be overridden by subclasses. This breaks proper
synchronization.

Added:
    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

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=1479366&r1=1479365&r2=1479366&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
Sun May  5 20:10:26 2013
@@ -147,6 +147,9 @@ public abstract class AbstractConfigurat
     /** Stores a reference to the object that handles variable interpolation. */
     private AtomicReference<ConfigurationInterpolator> interpolator;
 
+    /** The object responsible for synchronization. */
+    private volatile Synchronizer synchronizer;
+
     /** Stores the logger.*/
     private Log log;
 
@@ -556,13 +559,52 @@ public abstract class AbstractConfigurat
         });
     }
 
+    /**
+     * Returns the object responsible for synchronizing this configuration. All
+     * access to this configuration - both read and write access - is controlled
+     * by this object. This implementation never returns <b>null</b>. If no
+     * {@code Synchronizer} has been set, a {@link NoOpSynchronizer} is
+     * returned. So, per default, instances of {@code AbstractConfiguration} are
+     * not thread-safe unless a suitable {@code Synchronizer} is set!
+     *
+     * @return the {@code Synchronizer} used by this instance
+     * @since 2.0
+     */
+    public final Synchronizer getSynchronizer()
+    {
+        Synchronizer sync = synchronizer;
+        return (sync != null) ? sync : NoOpSynchronizer.INSTANCE;
+    }
+
+    /**
+     * Sets the object responsible for synchronizing this configuration. This
+     * method has to be called with a suitable {@code Synchronizer} object when
+     * initializing this configuration instance in order to make it thread-safe.
+     *
+     * @param synchronizer the new {@code Synchronizer}; can be <b>null</b>,
+     *        then this instance uses a {@link NoOpSynchronizer}
+     * @since 2.0
+     */
+    public final void setSynchronizer(Synchronizer synchronizer)
+    {
+        this.synchronizer = synchronizer;
+    }
+
     public void addProperty(String key, Object value)
     {
-        fireEvent(EVENT_ADD_PROPERTY, key, value, true);
-        addPropertyValues(key, value,
-                isDelimiterParsingDisabled() ? DISABLED_DELIMITER
-                        : getListDelimiter());
-        fireEvent(EVENT_ADD_PROPERTY, key, value, false);
+        getSynchronizer().beginWrite();
+        try
+        {
+            fireEvent(EVENT_ADD_PROPERTY, key, value, true);
+            addPropertyValues(key, value,
+                    isDelimiterParsingDisabled() ? DISABLED_DELIMITER
+                            : getListDelimiter());
+            fireEvent(EVENT_ADD_PROPERTY, key, value, false);
+        }
+        finally
+        {
+            getSynchronizer().endWrite();
+        }
     }
 
     /**
@@ -633,18 +675,26 @@ public abstract class AbstractConfigurat
 
     public void setProperty(String key, Object value)
     {
-        fireEvent(EVENT_SET_PROPERTY, key, value, true);
-        setDetailEvents(false);
+        getSynchronizer().beginWrite();
         try
         {
-            clearProperty(key);
-            addProperty(key, value);
+            fireEvent(EVENT_SET_PROPERTY, key, value, true);
+            setDetailEvents(false);
+            try
+            {
+                clearProperty(key);
+                addProperty(key, value);
+            }
+            finally
+            {
+                setDetailEvents(true);
+            }
+            fireEvent(EVENT_SET_PROPERTY, key, value, false);
         }
         finally
         {
-            setDetailEvents(true);
+            getSynchronizer().endWrite();
         }
-        fireEvent(EVENT_SET_PROPERTY, key, value, false);
     }
 
     /**
@@ -656,9 +706,17 @@ public abstract class AbstractConfigurat
      */
     public void clearProperty(String key)
     {
-        fireEvent(EVENT_CLEAR_PROPERTY, key, null, true);
-        clearPropertyDirect(key);
-        fireEvent(EVENT_CLEAR_PROPERTY, key, null, false);
+        getSynchronizer().beginWrite();
+        try
+        {
+            fireEvent(EVENT_CLEAR_PROPERTY, key, null, true);
+            clearPropertyDirect(key);
+            fireEvent(EVENT_CLEAR_PROPERTY, key, null, false);
+        }
+        finally
+        {
+            getSynchronizer().endWrite();
+        }
     }
 
     /**
@@ -672,45 +730,54 @@ public abstract class AbstractConfigurat
 
     public void clear()
     {
-        fireEvent(EVENT_CLEAR, null, null, true);
-        setDetailEvents(false);
-        boolean useIterator = true;
+        getSynchronizer().beginWrite();
         try
         {
-            Iterator<String> it = getKeys();
-            while (it.hasNext())
+            fireEvent(EVENT_CLEAR, null, null, true);
+            setDetailEvents(false);
+            boolean useIterator = true;
+            try
             {
-                String key = it.next();
-                if (useIterator)
+                Iterator<String> it = getKeys();
+                while (it.hasNext())
                 {
-                    try
+                    String key = it.next();
+                    if (useIterator)
                     {
-                        it.remove();
+                        try
+                        {
+                            it.remove();
+                        }
+                        catch (UnsupportedOperationException usoex)
+                        {
+                            useIterator = false;
+                        }
                     }
-                    catch (UnsupportedOperationException usoex)
+
+                    if (useIterator && containsKey(key))
                     {
                         useIterator = false;
                     }
-                }
 
-                if (useIterator && containsKey(key))
-                {
-                    useIterator = false;
-                }
-
-                if (!useIterator)
-                {
-                    // workaround for Iterators that do not remove the property
-                    // on calling remove() or do not support remove() at all
-                    clearProperty(key);
+                    if (!useIterator)
+                    {
+                        // workaround for Iterators that do not remove the
+                        // property
+                        // on calling remove() or do not support remove() at all
+                        clearProperty(key);
+                    }
                 }
             }
+            finally
+            {
+                setDetailEvents(true);
+            }
+            fireEvent(EVENT_CLEAR, null, null, false);
         }
         finally
         {
-            setDetailEvents(true);
+            getSynchronizer().endWrite();
         }
-        fireEvent(EVENT_CLEAR, null, null, false);
     }
 
     /**
@@ -1223,7 +1290,7 @@ public abstract class AbstractConfigurat
      */
     public String[] getStringArray(String key)
     {
-        Object value = getProperty(key);
+        Object value = readProperty(key);
 
         String[] array;
 
@@ -1270,7 +1337,7 @@ public abstract class AbstractConfigurat
 
     public List<Object> getList(String key, List<Object> defaultValue)
     {
-        Object value = getProperty(key);
+        Object value = readProperty(key);
         List<Object> list;
 
         if (value instanceof String)
@@ -1319,7 +1386,7 @@ public abstract class AbstractConfigurat
      */
     protected Object resolveContainerStore(String key)
     {
-        Object value = getProperty(key);
+        Object value = readProperty(key);
         if (value != null)
         {
             if (value instanceof Collection)
@@ -1457,4 +1524,23 @@ public abstract class AbstractConfigurat
         c.setDelimiterParsingDisabled(isDelimiterParsingDisabled());
         return c;
     }
+
+    /**
+     * Obtains a value of a property. Ensures proper synchronization.
+     *
+     * @param key the key to be read
+     * @return the value of this property
+     */
+    private Object readProperty(String key)
+    {
+        getSynchronizer().beginRead();
+        try
+        {
+            return getProperty(key);
+        }
+        finally
+        {
+            getSynchronizer().endRead();
+        }
+    }
 }

Added: 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=1479366&view=auto
==============================================================================
--- commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestAbstractConfigurationSynchronization.java
(added)
+++ commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestAbstractConfigurationSynchronization.java
Sun May  5 20:10:26 2013
@@ -0,0 +1,287 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.configuration;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.commons.configuration.io.FileHandler;
+import org.junit.Before;
+import org.junit.Ignore;
+import org.junit.Test;
+
+/**
+ * A test class for the synchronization capabilities of
+ * {@code AbstractConfiguration}. This class mainly checks the collaboration
+ * between a configuration object and its {@code Synchronizer}.
+ *
+ * @version $Id: $
+ */
+public class TestAbstractConfigurationSynchronization
+{
+    /** Constant for the test property accessed by all tests. */
+    private static final String PROP = "configuration.loaded";
+
+    /** The synchronizer used for testing. */
+    private SynchronizerTestImpl sync;
+
+    /** A test configuration. */
+    private AbstractConfiguration config;
+
+    @Before
+    public void setUp() throws Exception
+    {
+        // any concrete class will do
+        PropertiesConfiguration c = new PropertiesConfiguration();
+        new FileHandler(c).load(ConfigurationAssert
+                .getTestFile("test.properties"));
+        sync = new SynchronizerTestImpl();
+        c.setSynchronizer(sync);
+        config = c;
+    }
+
+    /**
+     * Tests the Synchronizer used by default.
+     */
+    @Test
+    public void testDefaultSynchronizer()
+    {
+        assertSame("Wrong default synchronizer", NoOpSynchronizer.INSTANCE,
+                new PropertiesConfiguration().getSynchronizer());
+    }
+
+    /**
+     * Tests the correct synchronization of addProperty().
+     */
+    @Test
+    public void testAddPropertySynchronized()
+    {
+        config.addProperty(PROP, "of course");
+        sync.verify(Methods.BEGIN_WRITE, Methods.END_WRITE);
+    }
+
+    /**
+     * Tests the correct synchronization of setProperty().
+     */
+    @Test
+    public void testSetPropertySynchronized()
+    {
+        config.setProperty(PROP, "yes");
+        sync.verifyStart(Methods.BEGIN_WRITE);
+        sync.verifyEnd(Methods.END_WRITE);
+    }
+
+    /**
+     * Tests the correct synchronization of clearProperty().
+     */
+    @Test
+    public void testClearPropertySynchronized()
+    {
+        config.clearProperty(PROP);
+        sync.verify(Methods.BEGIN_WRITE, Methods.END_WRITE);
+    }
+
+    /**
+     * Tests the correct synchronization of clear().
+     */
+    @Test
+    @Ignore
+    // TODO prevent subclasses from overriding clear()
+    public void testClearSynchronized()
+    {
+        config.clear();
+        sync.verifyStart(Methods.BEGIN_WRITE);
+        sync.verifyEnd(Methods.END_WRITE);
+    }
+
+    /**
+     * Tests whether read access to properties is synchronized.
+     */
+    @Test
+    @Ignore
+    // TODO prevent subclasses from overriding getProperty()
+    public void testGetPropertySynchronized()
+    {
+        assertEquals("Wrong raw value", "true", config.getProperty(PROP));
+        assertTrue("Wrong boolean value", config.getBoolean(PROP));
+        sync.verify(Methods.BEGIN_READ, Methods.END_READ, Methods.BEGIN_READ,
+                Methods.END_READ);
+    }
+
+    /**
+     * Tests whether containsKey() is correctly synchronized.
+     */
+    @Test
+    @Ignore
+    // TODO prevent subclasses from overriding containsKey()
+    public void testContainsKeySychronized()
+    {
+        assertTrue("Wrong result", config.containsKey(PROP));
+        sync.verify(Methods.BEGIN_READ, Methods.END_READ);
+    }
+
+    /**
+     * Tests whether isEmpty() is correctly synchronized.
+     */
+    @Test
+    @Ignore
+    // TODO prevent subclasses from overriding containsKey()
+    public void testIsEmptySychronized()
+    {
+        assertFalse("Configuration is empty", config.isEmpty());
+        sync.verify(Methods.BEGIN_READ, Methods.END_READ);
+    }
+
+    /**
+     * Tests whether getKeys() is correctly synchronized.
+     */
+    @Test
+    @Ignore
+    // TODO prevent subclasses from overriding containsKey()
+    public void testGetKeysSychronized()
+    {
+        assertTrue("No keys", config.getKeys().hasNext());
+        sync.verify(Methods.BEGIN_READ, Methods.END_READ);
+    }
+
+    /**
+     * Tests synchronization of subset().
+     */
+    @Test
+    public void testSubsetSynchronized()
+    {
+        AbstractConfiguration subset =
+                (AbstractConfiguration) config.subset("configuration");
+        sync.verify();
+        assertEquals("Wrong synchronizer for subset",
+                NoOpSynchronizer.INSTANCE, subset.getSynchronizer());
+    }
+
+    /**
+     * An enumeration with the methods of the Synchronizer which can be called.
+     */
+    private static enum Methods
+    {
+        BEGIN_READ, END_READ, BEGIN_WRITE, END_WRITE
+    }
+
+    /**
+     * A test implementation of Synchronizer which allows keeping track about
+     * the methods called by the configuration.
+     */
+    private static class SynchronizerTestImpl implements Synchronizer
+    {
+        /** A buffer for registering the methods invoked by clients. */
+        private final StringBuilder methods = new StringBuilder();
+
+        /**
+         * {@inheritDoc} Registers this invocation.
+         */
+        public void beginRead()
+        {
+            append(Methods.BEGIN_READ);
+        }
+
+        /**
+         * {@inheritDoc} Registers this invocation.
+         */
+        public void endRead()
+        {
+            append(Methods.END_READ);
+        }
+
+        /**
+         * {@inheritDoc} Registers this invocation.
+         */
+        public void beginWrite()
+        {
+            append(Methods.BEGIN_WRITE);
+        }
+
+        /**
+         * {@inheritDoc} Registers this invocation.
+         */
+        public void endWrite()
+        {
+            append(Methods.END_WRITE);
+        }
+
+        /**
+         * Verifies that the passed in methods were called in this order.
+         *
+         * @param expMethods the expected methods
+         */
+        public void verify(Methods... expMethods)
+        {
+            assertEquals("Wrong methods invoked",
+                    constructExpectedMethods(expMethods), methods.toString());
+        }
+
+        /**
+         * Verifies that the specified methods were called at the beginning of
+         * the interaction with the synchronizer.
+         *
+         * @param expMethods the expected methods
+         */
+        public void verifyStart(Methods... expMethods)
+        {
+            assertTrue("Wrong methods at start: " + methods, methods.toString()
+                    .startsWith(constructExpectedMethods(expMethods)));
+        }
+
+        /**
+         * Verifies that the specified methods were called at the end of the
+         * interaction with the synchronizer.
+         *
+         * @param expMethods the expected methods
+         */
+        public void verifyEnd(Methods... expMethods)
+        {
+            assertTrue("Wrong methods at start: " + methods, methods.toString()
+                    .endsWith(constructExpectedMethods(expMethods)));
+        }
+
+        /**
+         * Generates a string with expected methods from the given array.
+         *
+         * @param expMethods the array with expected methods
+         * @return a corresponding string representation
+         */
+        private String constructExpectedMethods(Methods... expMethods)
+        {
+            StringBuilder buf = new StringBuilder();
+            for (Methods m : expMethods)
+            {
+                buf.append(m);
+            }
+            return buf.toString();
+        }
+
+        /**
+         * Adds a method name to the internal buffer. Called by all interface
+         * methods.
+         *
+         * @param m the method that was invoked
+         */
+        private void append(Methods m)
+        {
+            methods.append(m);
+        }
+    }
+}



Mime
View raw message