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/01/16 21:44:41 UTC

svn commit: r1434379 - in /commons/proper/configuration/trunk/src: main/java/org/apache/commons/configuration/builder/BasicBuilderParameters.java test/java/org/apache/commons/configuration/builder/TestBasicBuilderParameters.java

Author: oheger
Date: Wed Jan 16 20:44:41 2013
New Revision: 1434379

URL: http://svn.apache.org/viewvc?rev=1434379&view=rev
Log:
Added cloning support to BasicBuilderParameters.

Modified:
    commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/builder/BasicBuilderParameters.java
    commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/builder/TestBasicBuilderParameters.java

Modified: commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/builder/BasicBuilderParameters.java
URL: http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/builder/BasicBuilderParameters.java?rev=1434379&r1=1434378&r2=1434379&view=diff
==============================================================================
--- commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/builder/BasicBuilderParameters.java (original)
+++ commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/builder/BasicBuilderParameters.java Wed Jan 16 20:44:41 2013
@@ -45,7 +45,7 @@ import org.apache.commons.logging.Log;
  * @version $Id$
  * @since 2.0
  */
-public class BasicBuilderParameters implements BuilderParameters,
+public class BasicBuilderParameters implements Cloneable, BuilderParameters,
         BasicBuilderProperties<BasicBuilderParameters>
 {
     /** The key of the <em>throwExceptionOnMissing</em> property. */
@@ -75,7 +75,7 @@ public class BasicBuilderParameters impl
     private static final String PROP_PARENT_INTERPOLATOR = "parentInterpolator";
 
     /** The map for storing the current property values. */
-    private final Map<String, Object> properties;
+    private Map<String, Object> properties;
 
     /**
      * Creates a new instance of {@code BasicBuilderParameters}.
@@ -88,7 +88,9 @@ public class BasicBuilderParameters impl
 
     /**
      * {@inheritDoc} This implementation returns a copy of the internal
-     * parameters map with the values set so far.
+     * parameters map with the values set so far. Collection structures
+     * (e.g. for lookup objects) are stored as defensive copies, so the
+     * original data cannot be modified.
      */
     public Map<String, Object> getParameters()
     {
@@ -101,6 +103,8 @@ public class BasicBuilderParameters impl
             result.remove(PROP_DEFAULT_LOOKUPS);
             result.remove(PROP_PARENT_INTERPOLATOR);
         }
+
+        createDefensiveCopies(result);
         return result;
     }
 
@@ -267,6 +271,36 @@ public class BasicBuilderParameters impl
     }
 
     /**
+     * Clones this object. This is useful because multiple builder instances may
+     * use a similar set of parameters. However, single instances of parameter
+     * objects must not assigned to multiple builders. Therefore, cloning a
+     * parameters object provides a solution for this use case. This method
+     * creates a new parameters object with the same content as this one. The
+     * internal map storing the parameter values is cloned, too, also collection
+     * structures contained in this map. However, no a full deep clone operation
+     * is performed. Objects like a {@code ConfigurationInterpolator} or
+     * {@code Lookup}s are shared between this and the newly created instance.
+     *
+     * @return a clone of this object
+     */
+    @Override
+    public BasicBuilderParameters clone()
+    {
+        try
+        {
+            BasicBuilderParameters copy =
+                    (BasicBuilderParameters) super.clone();
+            copy.properties = getParameters();
+            return copy;
+        }
+        catch (CloneNotSupportedException cnex)
+        {
+            // should not happen
+            throw new AssertionError(cnex);
+        }
+    }
+
+    /**
      * Sets a property for this parameters object. Properties are stored in an
      * internal map. With this method a new entry can be added to this map. If
      * the value is <b>null</b>, the key is removed from the internal map. This
@@ -320,4 +354,32 @@ public class BasicBuilderParameters impl
         storeProperty(key, value);
         return this;
     }
+
+    /**
+     * Creates defensive copies for collection structures when constructing the
+     * map with parameters. It should not be possible to modify this object's
+     * internal state when having access to the parameters map.
+     *
+     * @param params the map with parameters to be passed to the caller
+     */
+    public static void createDefensiveCopies(HashMap<String, Object> params)
+    {
+        // This is safe to case because we have full control over the map
+        // and thus know the types of the contained values
+        @SuppressWarnings("unchecked")
+        Map<String, ? extends Lookup> prefixLookups =
+                (Map<String, ? extends Lookup>) params.get(PROP_PREFIX_LOOKUPS);
+        if (prefixLookups != null)
+        {
+            params.put(PROP_PREFIX_LOOKUPS, new HashMap<String, Lookup>(
+                    prefixLookups));
+        }
+        @SuppressWarnings("unchecked")
+        Collection<? extends Lookup> defLookups =
+                (Collection<? extends Lookup>) params.get(PROP_DEFAULT_LOOKUPS);
+        if (defLookups != null)
+        {
+            params.put(PROP_DEFAULT_LOOKUPS, new ArrayList<Lookup>(defLookups));
+        }
+    }
 }

Modified: commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/builder/TestBasicBuilderParameters.java
URL: http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/builder/TestBasicBuilderParameters.java?rev=1434379&r1=1434378&r2=1434379&view=diff
==============================================================================
--- commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/builder/TestBasicBuilderParameters.java (original)
+++ commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/builder/TestBasicBuilderParameters.java Wed Jan 16 20:44:41 2013
@@ -21,6 +21,7 @@ import static org.junit.Assert.assertFal
 import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
 
 import java.util.ArrayList;
 import java.util.Collection;
@@ -153,6 +154,8 @@ public class TestBasicBuilderParameters
         assertNotSame("No copy was created", lookups, map);
         assertEquals("Wrong lookup", look, map.get("test"));
         assertEquals("Wrong number of lookups", 1, map.size());
+        Map<?, ?> map2 = (Map<?, ?>) params.getParameters().get("prefixLookups");
+        assertNotSame("No copy in parameters", map, map2);
     }
 
     /**
@@ -181,6 +184,9 @@ public class TestBasicBuilderParameters
         assertNotSame("No copy was created", col, looks);
         assertEquals("Wrong number of lookups", 1, col.size());
         assertSame("Wrong lookup", look, col.iterator().next());
+        Collection<?> col2 =
+                (Collection<?>) params.getParameters().get("defaultLookups");
+        assertNotSame("No copy in parameters", col, col2);
     }
 
     /**
@@ -295,4 +301,65 @@ public class TestBasicBuilderParameters
         assertSame("Wrong interpolator", ci,
                 BasicBuilderParameters.fetchInterpolator(map));
     }
+
+    /**
+     * Tests whether a cloned instance contains the same data as the original
+     * object.
+     */
+    @Test
+    public void testCloneValues()
+    {
+        Log log = EasyMock.createMock(Log.class);
+        ConfigurationInterpolator ci =
+                EasyMock.createMock(ConfigurationInterpolator.class);
+        params.setListDelimiter('#');
+        params.setLogger(log);
+        params.setInterpolator(ci);
+        params.setThrowExceptionOnMissing(true);
+        BasicBuilderParameters clone = params.clone();
+        params.setListDelimiter('.');
+        params.setThrowExceptionOnMissing(false);
+        Map<String, Object> map = clone.getParameters();
+        assertSame("Wrong logger", log, map.get("logger"));
+        assertSame("Wrong interpolator", ci, map.get("interpolator"));
+        assertEquals("Wrong list delimiter", Character.valueOf('#'),
+                map.get("listDelimiter"));
+        assertEquals("Wrong exception flag", Boolean.TRUE,
+                map.get("throwExceptionOnMissing"));
+    }
+
+    /**
+     * Tests whether the map with prefix lookups is cloned, too.
+     */
+    @Test
+    public void testClonePrefixLookups()
+    {
+        Lookup look = EasyMock.createMock(Lookup.class);
+        Map<String, Lookup> lookups = Collections.singletonMap("test", look);
+        params.setPrefixLookups(lookups);
+        BasicBuilderParameters clone = params.clone();
+        Map<?, ?> map = (Map<?, ?>) params.getParameters().get("prefixLookups");
+        map.clear();
+        map = (Map<?, ?>) clone.getParameters().get("prefixLookups");
+        assertEquals("Wrong number of lookups", 1, map.size());
+        assertSame("Wrong lookup", look, map.get("test"));
+    }
+
+    /**
+     * Tests whether the collection with default lookups can be cloned, too.
+     */
+    @Test
+    public void testCloneDefaultLookups()
+    {
+        Lookup look = EasyMock.createMock(Lookup.class);
+        Collection<Lookup> looks = Collections.singleton(look);
+        params.setDefaultLookups(looks);
+        BasicBuilderParameters clone = params.clone();
+        Collection<?> defLooks =
+                (Collection<?>) params.getParameters().get("defaultLookups");
+        defLooks.clear();
+        defLooks = (Collection<?>) clone.getParameters().get("defaultLookups");
+        assertEquals("Wrong number of default lookups", 1, defLooks.size());
+        assertTrue("Wrong default lookup", defLooks.contains(look));
+    }
 }