felix-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From fmesc...@apache.org
Subject svn commit: r735651 - in /felix/trunk/configadmin: configadmin/ src/main/java/org/apache/felix/cm/impl/ src/test/java/org/apache/felix/cm/ src/test/java/org/apache/felix/cm/impl/
Date Mon, 19 Jan 2009 10:40:29 GMT
Author: fmeschbe
Date: Mon Jan 19 02:40:28 2009
New Revision: 735651

URL: http://svn.apache.org/viewvc?rev=735651&view=rev
Log:
FELIX-890 Configuration.getProperties() must return a full copy of the
dictionary, that is also arrays and collections must be copied

Added:
    felix/trunk/configadmin/configadmin/
    felix/trunk/configadmin/src/test/java/org/apache/felix/cm/MockPersistenceManager.java
  (with props)
    felix/trunk/configadmin/src/test/java/org/apache/felix/cm/impl/ConfigurationAdapterTest.java
  (with props)
    felix/trunk/configadmin/src/test/java/org/apache/felix/cm/impl/MockConfigurationManager.java
  (with props)
Modified:
    felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/CaseInsensitiveDictionary.java
    felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdapter.java
    felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationImpl.java
    felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java

Modified: felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/CaseInsensitiveDictionary.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/CaseInsensitiveDictionary.java?rev=735651&r1=735650&r2=735651&view=diff
==============================================================================
--- felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/CaseInsensitiveDictionary.java
(original)
+++ felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/CaseInsensitiveDictionary.java
Mon Jan 19 02:40:28 2009
@@ -19,6 +19,7 @@
 package org.apache.felix.cm.impl;
 
 
+import java.lang.reflect.Array;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -26,6 +27,8 @@
 import java.util.Enumeration;
 import java.util.Hashtable;
 import java.util.Iterator;
+import java.util.Map;
+import java.util.Vector;
 
 
 /**
@@ -88,9 +91,43 @@
     }
 
 
-    CaseInsensitiveDictionary( CaseInsensitiveDictionary props )
+    CaseInsensitiveDictionary( CaseInsensitiveDictionary props, boolean deepCopy )
     {
-        internalMap = new Hashtable( props.internalMap );
+        Hashtable tmp = new Hashtable( Math.max( 2 * props.internalMap.size(), 11 ), 0.75f
);
+        if ( deepCopy )
+        {
+            Iterator entries = props.internalMap.entrySet().iterator();
+            while ( entries.hasNext() )
+            {
+                Map.Entry entry = ( Map.Entry ) entries.next();
+                Object value = entry.getValue();
+                if ( value.getClass().isArray() )
+                {
+                    // copy array
+                    int length = Array.getLength( value );
+                    Object newValue = Array.newInstance( value.getClass().getComponentType(),
length );
+                    System.arraycopy( value, 0, newValue, 0, length );
+                    value = newValue;
+                }
+                else if ( value instanceof Collection )
+                {
+                    // copy collection, create Vector
+                    // a Vector is created because the R4 and R4.1 specs
+                    // state that the values must be simple, array or
+                    // Vector. And even though we accept Collection nowadays
+                    // there might be clients out there still written against
+                    // R4 and R4.1 spec expecting Vector
+                    value = new Vector( ( Collection ) value );
+                }
+                tmp.put( entry.getKey(), value );
+            }
+        }
+        else
+        {
+            tmp.putAll( props.internalMap );
+        }
+        
+        internalMap = tmp;
         originalKeys = new Hashtable( props.originalKeys );
     }
 

Modified: felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdapter.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdapter.java?rev=735651&r1=735650&r2=735651&view=diff
==============================================================================
--- felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdapter.java
(original)
+++ felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdapter.java
Mon Jan 19 02:40:28 2009
@@ -29,7 +29,7 @@
  * The <code>ConfigurationAdapter</code> TODO
  *
  * @author fmeschbe
- * @version $Rev: 527592 $, $Date$
+ * @version $Rev:616219 $, $Date$
  */
 public class ConfigurationAdapter implements Configuration
 {
@@ -117,7 +117,10 @@
     public Dictionary getProperties()
     {
         checkDeleted();
-        return delegatee.getProperties();
+        
+        // return a deep copy since the spec says, that modification of
+        // any value should not modify the internal, stored value
+        return delegatee.getProperties( true );
     }
 
 

Modified: felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationImpl.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationImpl.java?rev=735651&r1=735650&r2=735651&view=diff
==============================================================================
--- felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationImpl.java
(original)
+++ felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationImpl.java
Mon Jan 19 02:40:28 2009
@@ -223,10 +223,20 @@
     }
 
 
-    /* (non-Javadoc)
-     * @see org.osgi.service.cm.Configuration#getProperties()
+    /**
+     * Returns an optionally deep copy of the properties of this configuration
+     * instance.
+     * <p>
+     * This method returns a copy of the internal dictionary. If the
+     * <code>deepCopy</code> parameter is true array and collection values are
+     * copied into new arrays or collections. Otherwise just a new dictionary
+     * referring to the same objects is returned.
+     * 
+     * @param deepCopy
+     *            <code>true</code> if a deep copy is to be returned.
+     * @return
      */
-    public Dictionary getProperties()
+    public Dictionary getProperties( boolean deepCopy )
     {
         // no properties yet
         if ( properties == null )
@@ -234,7 +244,7 @@
             return null;
         }
 
-        CaseInsensitiveDictionary props = new CaseInsensitiveDictionary( properties );
+        CaseInsensitiveDictionary props = new CaseInsensitiveDictionary( properties, deepCopy
);
 
         // fix special properties (pid, factory PID, bundle location)
         setAutoProperties( props, false );
@@ -375,7 +385,10 @@
 
     void store() throws IOException
     {
-        Dictionary props = getProperties();
+        // we don't need a deep copy, since we are not modifying
+        // any value in the dictionary itself. we are just adding
+        // properties to it, which are required for storing
+        Dictionary props = getProperties( false );
 
         // if this is a new configuration, we just use an empty Dictionary
         if ( props == null )

Modified: felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java?rev=735651&r1=735650&r2=735651&view=diff
==============================================================================
--- felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java
(original)
+++ felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java
Mon Jan 19 02:40:28 2009
@@ -680,7 +680,9 @@
      */
     private Dictionary callPlugins( ServiceReference sr, ConfigurationImpl cfg )
     {
-        Dictionary props = cfg.getProperties();
+        // return a deep copy, since the plugins may tamper with the array
+        // and collection elements, which should not modify the internal data
+        Dictionary props = cfg.getProperties( true );
 
         // guard against NPE for new configuration never updated
         if (props == null) {

Added: felix/trunk/configadmin/src/test/java/org/apache/felix/cm/MockPersistenceManager.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/test/java/org/apache/felix/cm/MockPersistenceManager.java?rev=735651&view=auto
==============================================================================
--- felix/trunk/configadmin/src/test/java/org/apache/felix/cm/MockPersistenceManager.java
(added)
+++ felix/trunk/configadmin/src/test/java/org/apache/felix/cm/MockPersistenceManager.java
Mon Jan 19 02:40:28 2009
@@ -0,0 +1,71 @@
+/*
+ * 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.felix.cm;
+
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.Dictionary;
+import java.util.Enumeration;
+import java.util.HashMap;
+import java.util.Map;
+
+
+public class MockPersistenceManager implements PersistenceManager
+{
+
+    private final Map configs = new HashMap();
+
+
+    public void delete( String pid )
+    {
+        configs.remove( pid );
+    }
+
+
+    public boolean exists( String pid )
+    {
+        return configs.containsKey( pid );
+    }
+
+
+    public Enumeration getDictionaries()
+    {
+        return Collections.enumeration( configs.values() );
+    }
+
+
+    public Dictionary load( String pid ) throws IOException
+    {
+        Dictionary config = ( Dictionary ) configs.get( pid );
+        if ( config != null )
+        {
+            return config;
+        }
+
+        throw new IOException( "No such configuration: " + pid );
+    }
+
+
+    public void store( String pid, Dictionary properties )
+    {
+        configs.put( pid, properties );
+    }
+
+}

Propchange: felix/trunk/configadmin/src/test/java/org/apache/felix/cm/MockPersistenceManager.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: felix/trunk/configadmin/src/test/java/org/apache/felix/cm/MockPersistenceManager.java
------------------------------------------------------------------------------
    svn:keywords = Author Date Id Revision Rev Url

Added: felix/trunk/configadmin/src/test/java/org/apache/felix/cm/impl/ConfigurationAdapterTest.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/test/java/org/apache/felix/cm/impl/ConfigurationAdapterTest.java?rev=735651&view=auto
==============================================================================
--- felix/trunk/configadmin/src/test/java/org/apache/felix/cm/impl/ConfigurationAdapterTest.java
(added)
+++ felix/trunk/configadmin/src/test/java/org/apache/felix/cm/impl/ConfigurationAdapterTest.java
Mon Jan 19 02:40:28 2009
@@ -0,0 +1,160 @@
+/*
+ * 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.felix.cm.impl;
+
+
+import java.io.IOException;
+import java.lang.reflect.Array;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Dictionary;
+import java.util.Hashtable;
+
+import junit.framework.TestCase;
+
+import org.apache.felix.cm.MockPersistenceManager;
+import org.apache.felix.cm.PersistenceManager;
+import org.osgi.framework.Constants;
+import org.osgi.service.cm.Configuration;
+
+
+public class ConfigurationAdapterTest extends TestCase
+{
+
+    private static final String SCALAR = "scalar";
+    private static final String STRING_VALUE = "String Value";
+    private static final String STRING_VALUE2 = "Another String Value";
+
+    private static final String ARRAY = "array";
+    private final String[] ARRAY_VALUE;
+
+    private static final String COLLECTION = "collection";
+    private final Collection COLLECTION_VALUE;
+
+    private static final String TEST_PID = "test.pid";
+    private static final String TEST_LOCATION = "test:location";
+
+    private final PersistenceManager pm = new MockPersistenceManager();
+    private final MockConfigurationManager configMgr = new MockConfigurationManager();
+
+    {
+        ARRAY_VALUE = new String[]
+            { STRING_VALUE };
+        COLLECTION_VALUE = new ArrayList();
+        COLLECTION_VALUE.add( STRING_VALUE );
+    }
+
+
+    private Configuration getConfiguration() throws IOException
+    {
+        ConfigurationImpl cimpl = new ConfigurationImpl( configMgr, pm, TEST_PID, null, TEST_LOCATION
);
+        return new ConfigurationAdapter( null, cimpl );
+    }
+
+
+    public void testScalar() throws IOException
+    {
+        Configuration cimpl = getConfiguration();
+        Dictionary props = cimpl.getProperties();
+        assertNull( "Configuration is fresh", props );
+
+        props = new Hashtable();
+        props.put( SCALAR, STRING_VALUE );
+        cimpl.update( props );
+
+        Dictionary newProps = cimpl.getProperties();
+        assertNotNull( "Configuration is not fresh", newProps );
+        assertEquals( "Expect 2 elements", 2, newProps.size() );
+        assertEquals( "Service.pid must match", TEST_PID, newProps.get( Constants.SERVICE_PID
) );
+        assertEquals( "Scalar value must match", STRING_VALUE, newProps.get( SCALAR ) );
+    }
+
+
+    public void testArray() throws IOException
+    {
+        Configuration cimpl = getConfiguration();
+
+        Dictionary props = cimpl.getProperties();
+        assertNull( "Configuration is fresh", props );
+
+        props = new Hashtable();
+        props.put( ARRAY, ARRAY_VALUE );
+        cimpl.update( props );
+
+        Dictionary newProps = cimpl.getProperties();
+        assertNotNull( "Configuration is not fresh", newProps );
+        assertEquals( "Expect 2 elements", 2, newProps.size() );
+        assertEquals( "Service.pid must match", TEST_PID, newProps.get( Constants.SERVICE_PID
) );
+
+        Object testProp = newProps.get( ARRAY );
+        assertNotNull( testProp );
+        assertTrue( testProp.getClass().isArray() );
+        assertEquals( 1, Array.getLength( testProp ) );
+        assertEquals( STRING_VALUE, Array.get( testProp, 0 ) );
+
+        // modify the array property
+        Array.set( testProp, 0, STRING_VALUE2 );
+
+        // the array element change must not be reflected in the configuration
+        Dictionary newProps2 = cimpl.getProperties();
+        Object testProp2 = newProps2.get( ARRAY );
+        assertNotNull( testProp2 );
+        assertTrue( testProp2.getClass().isArray() );
+        assertEquals( 1, Array.getLength( testProp2 ) );
+        assertEquals( STRING_VALUE, Array.get( testProp2, 0 ) );
+    }
+
+
+    public void testCollection() throws IOException
+    {
+        Configuration cimpl = getConfiguration();
+
+        Dictionary props = cimpl.getProperties();
+        assertNull( "Configuration is fresh", props );
+
+        props = new Hashtable();
+        props.put( COLLECTION, COLLECTION_VALUE );
+        cimpl.update( props );
+
+        Dictionary newProps = cimpl.getProperties();
+        assertNotNull( "Configuration is not fresh", newProps );
+        assertEquals( "Expect 2 elements", 2, newProps.size() );
+        assertEquals( "Service.pid must match", TEST_PID, newProps.get( Constants.SERVICE_PID
) );
+
+        Object testProp = newProps.get( COLLECTION );
+        assertNotNull( testProp );
+        assertTrue( testProp instanceof Collection );
+        Collection coll = ( Collection ) testProp;
+        assertEquals( 1, coll.size() );
+        assertEquals( STRING_VALUE, coll.iterator().next() );
+
+        // modify the array property
+        coll.clear();
+        coll.add( STRING_VALUE2 );
+
+        // the array element change must not be reflected in the configuration
+        Dictionary newProps2 = cimpl.getProperties();
+        Object testProp2 = newProps2.get( COLLECTION );
+        assertNotNull( testProp2 );
+        assertTrue( testProp2 instanceof Collection );
+        Collection coll2 = ( Collection ) testProp2;
+        assertEquals( 1, coll2.size() );
+        assertEquals( STRING_VALUE, coll2.iterator().next() );
+    }
+}

Propchange: felix/trunk/configadmin/src/test/java/org/apache/felix/cm/impl/ConfigurationAdapterTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: felix/trunk/configadmin/src/test/java/org/apache/felix/cm/impl/ConfigurationAdapterTest.java
------------------------------------------------------------------------------
    svn:keywords = Author Date Id Revision Rev Url

Added: felix/trunk/configadmin/src/test/java/org/apache/felix/cm/impl/MockConfigurationManager.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/test/java/org/apache/felix/cm/impl/MockConfigurationManager.java?rev=735651&view=auto
==============================================================================
--- felix/trunk/configadmin/src/test/java/org/apache/felix/cm/impl/MockConfigurationManager.java
(added)
+++ felix/trunk/configadmin/src/test/java/org/apache/felix/cm/impl/MockConfigurationManager.java
Mon Jan 19 02:40:28 2009
@@ -0,0 +1,41 @@
+/*
+ * 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.felix.cm.impl;
+
+
+public class MockConfigurationManager extends ConfigurationManager
+{
+
+    void updated( ConfigurationImpl config )
+    {
+        // do nothing, might register the update call
+    }
+
+
+    void deleted( ConfigurationImpl config )
+    {
+        // do nothing, might register the update call
+    }
+
+
+    void log( int level, String message, Throwable t )
+    {
+        // no logging for now
+    }
+}

Propchange: felix/trunk/configadmin/src/test/java/org/apache/felix/cm/impl/MockConfigurationManager.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: felix/trunk/configadmin/src/test/java/org/apache/felix/cm/impl/MockConfigurationManager.java
------------------------------------------------------------------------------
    svn:keywords = Author Date Id Revision Rev Url



Mime
View raw message