Attached is a patch with an extended unit test (which the patched code passes, but the current 1.10 fails).
Just using the 2.0 code does not work, because - the 1.10 API uses Map<String, Object> and not Map<String, String> - the 1.10 MapConfiguration(Properties props) constructor mandates that the implementation has to ignore property keys which are not strings. So instead the patch simply creates a new HashMap from the Properties entries with String keys. However, this makes TestSystemConfigurationRegression fail now. I understand why, but I don't understand why this is a valid test case. TestSystemConfigurationRegression assumes that a SystemConfiguration instances changes when a System property is added. The old code did that, and the patched code does not. However, MapConfiguration(Properties props) javadoc for 1.10 clearly states: "The resulting configuration is not connected to the Properties object". So why is this a valid test case? </nk> Confidentiality Notice:This email and any files transmitted with it are confidential and intended solely for the use of the individual or entity to whom they are addressed. This message contains confidential information and is intended only for the individual named. If you are not the named addressee you should not disseminate, distribute or copy this e-mail. Please notify the sender immediately by e-mail if you have received this e-mail by mistake and delete this e-mail from your system. If you are not the intended recipient you are notified that disclosing, copying, distributing or taking any action in reliance on the contents of this information is strictly prohibited
From b4c34b691998dd8bd3eb992677851c3b66f08979 Mon Sep 17 00:00:00 2001 From: Norbert Kiesel <[email protected]> Date: Mon, 11 Jan 2016 11:57:06 -0800 Subject: [PATCH] Patch with unit test for 1.10 regression Provide MapConfiguration.convertPropertiesToMap implementation that results in a mutable underlying map, and adjust unit tests accordingly. --- .../commons/configuration/MapConfiguration.java | 35 ++++------------ .../TestMapConfigurationRegression.java | 48 +++++++++++++++++++--- 2 files changed, 49 insertions(+), 34 deletions(-) diff --git src/main/java/org/apache/commons/configuration/MapConfiguration.java src/main/java/org/apache/commons/configuration/MapConfiguration.java index f6d8b4d..34dbe4c 100644 --- src/main/java/org/apache/commons/configuration/MapConfiguration.java +++ src/main/java/org/apache/commons/configuration/MapConfiguration.java @@ -20,6 +20,7 @@ package org.apache.commons.configuration; import java.util.AbstractMap; import java.util.ArrayList; import java.util.HashSet; +import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -259,37 +260,15 @@ public class MapConfiguration extends AbstractConfiguration implements Cloneable */ private static Map<String, Object> convertPropertiesToMap(final Properties props) { - return new AbstractMap<String, Object>() { - - @Override - public Set<Map.Entry<String, Object>> entrySet() + HashMap propsMap = new HashMap<String, Object>(); + for (final Map.Entry<Object, Object> propertyEntry : props.entrySet()) { - Set<Map.Entry<String, Object>> entries = new HashSet<Map.Entry<String, Object>>(); - for (final Map.Entry<Object, Object> propertyEntry : props.entrySet()) - { - if (propertyEntry.getKey() instanceof String) + final Object key = propertyEntry.getKey(); + if (key instanceof String) { - entries.add(new Map.Entry<String, Object>() { - - public String getKey() - { - return propertyEntry.getKey().toString(); - } - - public Object getValue() - { - return propertyEntry.getValue(); - } - - public Object setValue(Object value) - { - throw new UnsupportedOperationException(); - } - }); + propsMap.put(key, propertyEntry.getValue()); } - } - return entries; } - }; + return propsMap; } } diff --git src/test/java/org/apache/commons/configuration/TestMapConfigurationRegression.java src/test/java/org/apache/commons/configuration/TestMapConfigurationRegression.java index 124df61..9876628 100644 --- src/test/java/org/apache/commons/configuration/TestMapConfigurationRegression.java +++ src/test/java/org/apache/commons/configuration/TestMapConfigurationRegression.java @@ -17,8 +17,9 @@ package org.apache.commons.configuration; -import java.util.Collections; import java.util.Map; +import java.util.HashMap; +import java.util.Properties; import java.util.UUID; import org.junit.Assert; @@ -27,13 +28,48 @@ import org.junit.Test; public class TestMapConfigurationRegression { @Test - public void testMapConfigurationRegression() + public void testMapConfigurationRegressionMap() { - final String key = UUID.randomUUID().toString(); - final String value = UUID.randomUUID().toString(); - final Map<String, String> map = Collections.singletonMap(key, value); + final String key1 = UUID.randomUUID().toString(); + final String value1 = UUID.randomUUID().toString(); + final Map<String, String> map = new HashMap<String, String>(); + map.put(key1, value1); final MapConfiguration mc = new MapConfiguration(map); Assert.assertEquals(1, mc.getMap().size()); - Assert.assertEquals(value, mc.getString(key)); + Assert.assertEquals(value1, mc.getString(key1)); + + // Make sure the resulting configuration is modifiable + final String key2 = UUID.randomUUID().toString(); + final String value2 = UUID.randomUUID().toString(); + mc.getMap().put(key2, value2); + Assert.assertEquals(2, mc.getMap().size()); + Assert.assertEquals(value1, mc.getString(key1)); + Assert.assertEquals(value2, mc.getString(key2)); + + mc.clearPropertyDirect(key1); + Assert.assertEquals(1, mc.getMap().size()); + } + + @Test + public void testMapConfigurationRegressionProperties() + { + final String key1 = UUID.randomUUID().toString(); + final String value1 = UUID.randomUUID().toString(); + final Properties props = new Properties(); + props.setProperty(key1, value1); + final MapConfiguration mc = new MapConfiguration(props); + Assert.assertEquals(1, mc.getMap().size()); + Assert.assertEquals(value1, mc.getString(key1)); + + // Make sure the resulting configuration is modifiable + final String key2 = UUID.randomUUID().toString(); + final String value2 = UUID.randomUUID().toString(); + mc.getMap().put(key2, value2); + Assert.assertEquals(2, mc.getMap().size()); + Assert.assertEquals(value1, mc.getString(key1)); + Assert.assertEquals(value2, mc.getString(key2)); + + mc.clearPropertyDirect(key1); + Assert.assertEquals(1, mc.getMap().size()); } } -- 2.7.0.rc3
--------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
