You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@felix.apache.org by fm...@apache.org on 2012/02/16 15:00:52 UTC

svn commit: r1244978 - in /felix/trunk/configadmin/src: main/java/org/apache/felix/cm/impl/ConfigurationImpl.java main/java/org/apache/felix/cm/impl/ConfigurationManager.java test/java/org/apache/felix/cm/integration/ConfigurationBaseTest.java

Author: fmeschbe
Date: Thu Feb 16 14:00:52 2012
New Revision: 1244978

URL: http://svn.apache.org/viewvc?rev=1244978&view=rev
Log:
FELIX-2888 Return newly created factory configurations from getConfiguration even though such configuration is not persisted yet.

Modified:
    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
    felix/trunk/configadmin/src/test/java/org/apache/felix/cm/integration/ConfigurationBaseTest.java

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=1244978&r1=1244977&r2=1244978&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 Thu Feb 16 14:00:52 2012
@@ -178,10 +178,7 @@ class ConfigurationImpl extends Configur
         // case the configuration is only stored when first updated
         if ( factoryPid == null )
         {
-            Dictionary props = new Hashtable();
-            setAutoProperties( props, true );
-            props.put( CONFIGURATION_NEW, Boolean.TRUE );
-            persistenceManager.store( pid, props );
+            storeNewConfiguration();
         }
     }
 
@@ -381,13 +378,6 @@ class ConfigurationImpl extends Configur
             String factoryPid = getFactoryPid();
             if ( factoryPid != null )
             {
-                // If this is a new factory configuration, we also have to add
-                // it to the configuration manager cache
-                if ( isNew() )
-                {
-                    getConfigurationManager().cacheConfiguration( this );
-                }
-
                 Factory factory = getConfigurationManager().getFactory( factoryPid );
                 if ( factory.addPID( getPid() ) )
                 {
@@ -447,6 +437,36 @@ class ConfigurationImpl extends Configur
 
     // ---------- private helper -----------------------------------------------
 
+    /**
+     * Stores the configuration if it is a newly factory configuration
+     * which has not been persisted yet.
+     * <p>
+     * This is used to ensure a configuration c as in
+     * <pre>
+     * Configuration cf = cm.createFactoryConfiguration(factoryPid);
+     * Configuration c = cm.getConfiguration(cf.getPid());
+     * </pre>
+     * is persisted after <code>getConfiguration</code> while
+     * <code>createConfiguration</code> alone does not persist yet.
+     */
+    void ensureFactoryConfigPersisted() throws IOException
+    {
+        if ( this.factoryPID != null && isNew() && !getPersistenceManager().exists( getPid() ) )
+        {
+            storeNewConfiguration();
+        }
+    }
+
+
+    private void storeNewConfiguration() throws IOException
+    {
+        Dictionary props = new Hashtable();
+        setAutoProperties( props, true );
+        props.put( CONFIGURATION_NEW, Boolean.TRUE );
+        getPersistenceManager().store( getPid(), props );
+    }
+
+
     void store() throws IOException
     {
         // we don't need a deep copy, since we are not modifying

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=1244978&r1=1244977&r2=1244978&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 Thu Feb 16 14:00:52 2012
@@ -438,7 +438,7 @@ public class ConfigurationManager implem
 
     ConfigurationImpl createFactoryConfiguration( String factoryPid, String location ) throws IOException
     {
-        return createConfiguration( createPid( factoryPid ), factoryPid, location );
+        return cacheConfiguration( createConfiguration( createPid( factoryPid ), factoryPid, location ) );
     }
 
 
@@ -459,6 +459,9 @@ public class ConfigurationManager implem
         {
             log( LogService.LOG_DEBUG, "Found cached configuration {0} bound to {1}", new Object[]
                 { pid, config.getBundleLocation() } );
+
+            config.ensureFactoryConfigPersisted();
+
             return config;
         }
 

Modified: felix/trunk/configadmin/src/test/java/org/apache/felix/cm/integration/ConfigurationBaseTest.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/test/java/org/apache/felix/cm/integration/ConfigurationBaseTest.java?rev=1244978&r1=1244977&r2=1244978&view=diff
==============================================================================
--- felix/trunk/configadmin/src/test/java/org/apache/felix/cm/integration/ConfigurationBaseTest.java (original)
+++ felix/trunk/configadmin/src/test/java/org/apache/felix/cm/integration/ConfigurationBaseTest.java Thu Feb 16 14:00:52 2012
@@ -33,6 +33,7 @@ import org.ops4j.pax.exam.junit.JUnit4Te
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleException;
 import org.osgi.framework.Constants;
+import org.osgi.framework.InvalidSyntaxException;
 import org.osgi.service.cm.Configuration;
 import org.osgi.service.cm.ConfigurationAdmin;
 
@@ -833,4 +834,67 @@ public class ConfigurationBaseTest exten
         TestCase.assertEquals( 1, tester.numManagedServiceFactoryDeleteCalls );
     }
 
+    @Test
+    public void test_factory_configuration_collision() throws IOException, InvalidSyntaxException, BundleException {
+        final String factoryPid = "test_factory_configuration_collision";
+
+        final Configuration cf = getConfigurationAdmin().createFactoryConfiguration( factoryPid, null );
+        TestCase.assertNotNull( cf );
+        final String pid = cf.getPid();
+
+        try
+        {
+            bundle = installBundle( factoryPid, ManagedServiceFactoryTestActivator.class );
+            bundle.start();
+            delay();
+
+            final ManagedServiceFactoryTestActivator tester = ManagedServiceFactoryTestActivator.INSTANCE;
+            TestCase.assertEquals( "MSF must not be updated with new configuration", 0, tester.numManagedServiceFactoryUpdatedCalls );
+
+            TestCase.assertNotNull( "Configuration must have PID", pid );
+            TestCase.assertEquals( "Factory configuration must have requested factory PID", factoryPid, cf.getFactoryPid() );
+
+            // assert getConfiguration returns the same configurtion
+            final Configuration c1 = getConfigurationAdmin().getConfiguration( pid, null );
+            TestCase.assertEquals( "getConfiguration must retrieve required PID", pid, c1.getPid() );
+            TestCase.assertEquals( "getConfiguration must retrieve new factory configuration", factoryPid, c1.getFactoryPid() );
+            TestCase.assertNull( "Configuration must not have properties", c1.getProperties() );
+
+            TestCase.assertEquals( "MSF must not be updated with new configuration", 0, tester.numManagedServiceFactoryUpdatedCalls );
+
+            // restart config admin and verify getConfiguration persisted
+            // the new factory configuration as such
+            final Bundle cmBundle = getCmBundle();
+            TestCase.assertNotNull( "Config Admin Bundle missing", cmBundle );
+            cmBundle.stop();
+            delay();
+            cmBundle.start();
+            delay();
+
+            TestCase.assertEquals( "MSF must not be updated with new configuration even after CM restart", 0, tester.numManagedServiceFactoryUpdatedCalls );
+
+            final Configuration c2 = getConfigurationAdmin().getConfiguration( pid, null );
+            TestCase.assertEquals( "getConfiguration must retrieve required PID", pid, c2.getPid() );
+            TestCase.assertEquals( "getConfiguration must retrieve new factory configuration from persistence", factoryPid, c2.getFactoryPid() );
+            TestCase.assertNull( "Configuration must not have properties", c2.getProperties() );
+
+            c2.update( theConfig );
+            delay();
+
+            TestCase.assertEquals( 1, tester.numManagedServiceFactoryUpdatedCalls );
+            TestCase.assertEquals( theConfig.get( PROP_NAME ), tester.configs.get( cf.getPid() ).get( PROP_NAME ) );
+
+            final Configuration[] cfs = getConfigurationAdmin().listConfigurations( "(" + ConfigurationAdmin.SERVICE_FACTORYPID + "="
+                + factoryPid + ")" );
+            TestCase.assertNotNull( "Expect at least one configuration", cfs );
+            TestCase.assertEquals( "Expect exactly one configuration", 1, cfs.length );
+            TestCase.assertEquals( cf.getPid(), cfs[0].getPid() );
+            TestCase.assertEquals( cf.getFactoryPid(), cfs[0].getFactoryPid() );
+        }
+        finally
+        {
+            // make sure no configuration survives ...
+            getConfigurationAdmin().getConfiguration( pid, null ).delete();
+        }
+    }
 }