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/05 22:10:26 UTC
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
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);
+ }
+ }
+}