FELIX-1952 Ensure components created for factory configurations are not disposed when configuration is updated (or some binding changes). Also include a testcase validating this.

git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@892241 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java
index a1a27f5..60bffec 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java
@@ -381,8 +381,14 @@
         {
             if ( getInstance() != null )
             {
+                if ( this instanceof ComponentFactoryImpl.ComponentFactoryConfiguredInstance )
+                {
+                    return Active.getInstance();
+                }
+
                 return FactoryInstance.getInstance();
             }
+
             return Factory.getInstance();
         }
         else if ( m_componentMetadata.isImmediate() )
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java
index a6b80b3..1116ae8 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java
@@ -102,7 +102,7 @@
      */
     public ComponentInstance newInstance( Dictionary dictionary )
     {
-        final ImmediateComponentManager cm = createComponentManager();
+        final ImmediateComponentManager cm = createComponentManager( true );
 
         cm.setFactoryProperties( dictionary );
         cm.reconfigure( m_configuration );
@@ -272,7 +272,7 @@
             if ( cm == null )
             {
                 // create a new instance with the current configuration
-                cm = createComponentManager();
+                cm = createComponentManager( false );
 
                 // this should not call component reactivation because it is
                 // not active yet
@@ -368,9 +368,14 @@
      * instance. The component manager is kept in the internal set of created
      * components. The component is neither configured nor enabled.
      */
-    private ImmediateComponentManager createComponentManager()
+    private ImmediateComponentManager createComponentManager( final boolean newInstance )
     {
-        return new ImmediateComponentManager( getActivator(), this, getComponentMetadata() );
+        if ( newInstance )
+        {
+            return new ComponentFactoryNewInstance( getActivator(), this, getComponentMetadata() );
+        }
+
+        return new ComponentFactoryConfiguredInstance( getActivator(), this, getComponentMetadata() );
     }
 
 
@@ -388,4 +393,24 @@
 
         return new ImmediateComponentManager[0];
     }
+
+    static class ComponentFactoryNewInstance extends ImmediateComponentManager {
+
+        public ComponentFactoryNewInstance( BundleComponentActivator activator, ComponentHolder componentHolder,
+            ComponentMetadata metadata )
+        {
+            super( activator, componentHolder, metadata );
+        }
+
+    }
+
+    static class ComponentFactoryConfiguredInstance extends ImmediateComponentManager {
+
+        public ComponentFactoryConfiguredInstance( BundleComponentActivator activator, ComponentHolder componentHolder,
+            ComponentMetadata metadata )
+        {
+            super( activator, componentHolder, metadata );
+        }
+
+    }
 }
diff --git a/scr/src/test/java/org/apache/felix/scr/integration/ComponentFactoryTest.java b/scr/src/test/java/org/apache/felix/scr/integration/ComponentFactoryTest.java
index e5ca1c0..dc93819 100644
--- a/scr/src/test/java/org/apache/felix/scr/integration/ComponentFactoryTest.java
+++ b/scr/src/test/java/org/apache/felix/scr/integration/ComponentFactoryTest.java
@@ -19,6 +19,8 @@
 package org.apache.felix.scr.integration;
 
 
+import java.io.IOException;
+import java.util.Dictionary;
 import java.util.Hashtable;
 import java.util.Map;
 
@@ -31,6 +33,7 @@
 import org.ops4j.pax.exam.junit.JUnit4TestRunner;
 import org.osgi.framework.InvalidSyntaxException;
 import org.osgi.framework.ServiceReference;
+import org.osgi.service.cm.Configuration;
 import org.osgi.service.component.ComponentConstants;
 import org.osgi.service.component.ComponentException;
 import org.osgi.service.component.ComponentFactory;
@@ -324,4 +327,165 @@
         TestCase.assertFalse( instanceMap.containsValue( instanceManager ) );
     }
 
+    @Test
+    public void test_component_factory_with_factory_configuration() throws InvalidSyntaxException, IOException
+    {
+        // this test is about non-standard behaviour of ComponentFactory services
+
+        final String componentname = "factory.component";
+        final String componentfactory = "factory.component.factory";
+
+        final Component component = findComponentByName( componentname );
+
+        TestCase.assertNotNull( component );
+        TestCase.assertFalse( component.isDefaultEnabled() );
+
+        TestCase.assertEquals( Component.STATE_DISABLED, component.getState() );
+        TestCase.assertNull( SimpleComponent.INSTANCE );
+
+        component.enable();
+        delay();
+
+        TestCase.assertEquals( Component.STATE_FACTORY, component.getState() );
+        TestCase.assertNull( SimpleComponent.INSTANCE );
+
+        final ServiceReference[] refs = bundleContext.getServiceReferences( ComponentFactory.class.getName(), "("
+            + ComponentConstants.COMPONENT_FACTORY + "=" + componentfactory + ")" );
+        TestCase.assertNotNull( refs );
+        TestCase.assertEquals( 1, refs.length );
+        final ComponentFactory factory = ( ComponentFactory ) bundleContext.getService( refs[0] );
+        TestCase.assertNotNull( factory );
+
+        final String factoryConfigPid = createFactoryConfiguration( componentname );
+        delay();
+
+        TestCase.assertNotNull( SimpleComponent.INSTANCE );
+        TestCase.assertEquals( PROP_NAME, SimpleComponent.INSTANCE.getProperty( PROP_NAME ) );
+
+        final Map<?, ?> instanceMap = ( Map<?, ?> ) getFieldValue( component, "m_configuredServices" );
+        TestCase.assertNotNull( instanceMap );
+        TestCase.assertEquals( 1, instanceMap.size() );
+
+        final Object instanceManager = getFieldValue( SimpleComponent.INSTANCE.m_activateContext.getComponentInstance(), "m_componentManager" );
+        TestCase.assertTrue( instanceMap.containsValue( instanceManager ) );
+
+
+        // check registered components
+        Component[] allFactoryComponents = findComponentsByName( componentname );
+        TestCase.assertNotNull( allFactoryComponents );
+        TestCase.assertEquals( 2, allFactoryComponents.length );
+        for ( int i = 0; i < allFactoryComponents.length; i++ )
+        {
+            final Component c = allFactoryComponents[i];
+            if ( c.getId() == component.getId() )
+            {
+                TestCase.assertEquals( Component.STATE_FACTORY, c.getState() );
+            }
+            else if ( c.getId() == SimpleComponent.INSTANCE.m_id )
+            {
+                TestCase.assertEquals( Component.STATE_ACTIVE, c.getState() );
+            }
+            else
+            {
+                TestCase.fail( "Unexpected Component " + c );
+            }
+        }
+
+        // modify the configuration
+        Configuration config = getConfigurationAdmin().getConfiguration( factoryConfigPid );
+        Dictionary props = config.getProperties();
+        props.put( PROP_NAME, PROP_NAME_FACTORY );
+        config.update( props );
+        delay();
+
+        // ensure instance with new configuration
+        TestCase.assertNotNull( SimpleComponent.INSTANCE );
+        TestCase.assertEquals( PROP_NAME_FACTORY, SimpleComponent.INSTANCE.getProperty( PROP_NAME ) );
+
+        // check registered components
+        allFactoryComponents = findComponentsByName( componentname );
+        TestCase.assertNotNull( allFactoryComponents );
+        TestCase.assertEquals( 2, allFactoryComponents.length );
+        for ( int i = 0; i < allFactoryComponents.length; i++ )
+        {
+            final Component c = allFactoryComponents[i];
+            if ( c.getId() == component.getId() )
+            {
+                TestCase.assertEquals( Component.STATE_FACTORY, c.getState() );
+            }
+            else if ( c.getId() == SimpleComponent.INSTANCE.m_id )
+            {
+                TestCase.assertEquals( Component.STATE_ACTIVE, c.getState() );
+            }
+            else
+            {
+                TestCase.fail( "Unexpected Component " + c );
+            }
+        }
+
+        // disable the factory
+        component.disable();
+        delay();
+
+        // factory is disabled and so is the instance
+        TestCase.assertEquals( Component.STATE_DISABLED, component.getState() );
+        TestCase.assertNull( SimpleComponent.INSTANCE );
+        TestCase.assertEquals( 1, instanceMap.size() );
+
+        // enabled the factory
+        component.enable();
+        delay();
+
+        // factory is enabled and so is the instance
+        TestCase.assertEquals( Component.STATE_FACTORY, component.getState() );
+        TestCase.assertNotNull( SimpleComponent.INSTANCE );
+        TestCase.assertEquals( 1, instanceMap.size() );
+
+        // check registered components
+        allFactoryComponents = findComponentsByName( componentname );
+        TestCase.assertNotNull( allFactoryComponents );
+        TestCase.assertEquals( 2, allFactoryComponents.length );
+        for ( int i = 0; i < allFactoryComponents.length; i++ )
+        {
+            final Component c = allFactoryComponents[i];
+            if ( c.getId() == component.getId() )
+            {
+                TestCase.assertEquals( Component.STATE_FACTORY, c.getState() );
+            }
+            else if ( c.getId() == SimpleComponent.INSTANCE.m_id )
+            {
+                TestCase.assertEquals( Component.STATE_ACTIVE, c.getState() );
+            }
+            else
+            {
+                TestCase.fail( "Unexpected Component " + c );
+            }
+        }
+
+        // delete the configuration
+        getConfigurationAdmin().getConfiguration( factoryConfigPid ).delete();
+        delay();
+
+        // factory is enabled but instance has been removed
+        TestCase.assertEquals( Component.STATE_FACTORY, component.getState() );
+        TestCase.assertNull( SimpleComponent.INSTANCE );
+        TestCase.assertEquals( 0, instanceMap.size() );
+
+        // check registered components
+        allFactoryComponents = findComponentsByName( componentname );
+        TestCase.assertNotNull( allFactoryComponents );
+        TestCase.assertEquals( 1, allFactoryComponents.length );
+        for ( int i = 0; i < allFactoryComponents.length; i++ )
+        {
+            final Component c = allFactoryComponents[i];
+            if ( c.getId() == component.getId() )
+            {
+                TestCase.assertEquals( Component.STATE_FACTORY, c.getState() );
+            }
+            else
+            {
+                TestCase.fail( "Unexpected Component " + c );
+            }
+        }
+    }
 }
diff --git a/scr/src/test/java/org/apache/felix/scr/integration/ComponentTestBase.java b/scr/src/test/java/org/apache/felix/scr/integration/ComponentTestBase.java
index 963530d..5ef14af 100644
--- a/scr/src/test/java/org/apache/felix/scr/integration/ComponentTestBase.java
+++ b/scr/src/test/java/org/apache/felix/scr/integration/ComponentTestBase.java
@@ -22,6 +22,7 @@
 import static org.ops4j.pax.exam.CoreOptions.mavenBundle;
 import static org.ops4j.pax.exam.CoreOptions.options;
 import static org.ops4j.pax.exam.CoreOptions.provision;
+import static org.ops4j.pax.exam.CoreOptions.systemProperty;
 import static org.ops4j.pax.swissbox.tinybundles.core.TinyBundles.withBnd;
 
 import java.io.File;
@@ -110,7 +111,8 @@
                 CoreOptions.bundle( bundleFile.toURI().toString() ),
                 mavenBundle( "org.ops4j.pax.swissbox", "pax-swissbox-tinybundles", "1.0.0" ),
                 mavenBundle( "org.apache.felix", "org.apache.felix.configadmin", "1.0.10" )
-             )
+             ),
+             systemProperty( "ds.factory.enabled" ).value( "true" )
         );
         final Option vmOption = ( paxRunnerVmOption != null ) ? PaxRunnerOptions.vmOption( paxRunnerVmOption ) : null;
         return OptionUtils.combine( base, vmOption );