FELIX-1826 Throw ComponentException if newInstance fails to create a
component instance (for any one reason, which is logged actually)

git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@830994 13f79535-47bb-0310-9956-ffa450edef68
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 06010aa..682be7f 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
@@ -30,6 +30,7 @@
 import org.osgi.framework.Constants;
 import org.osgi.framework.ServiceRegistration;
 import org.osgi.service.component.ComponentConstants;
+import org.osgi.service.component.ComponentException;
 import org.osgi.service.component.ComponentFactory;
 import org.osgi.service.component.ComponentInstance;
 import org.osgi.service.log.LogService;
@@ -110,8 +111,14 @@
         cm.activateInternal();
 
         final ComponentInstance instance = cm.getComponentInstance();
-        m_componentInstances.put( cm, instance );
+        if ( instance == null )
+        {
+            // activation failed, clean up component manager
+            cm.dispose();
+            throw new ComponentException( "Failed activating component" );
+        }
 
+        m_componentInstances.put( cm, instance );
         return instance;
     }
 
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 3e9465b..450e5a4 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
@@ -32,6 +32,7 @@
 import org.osgi.framework.InvalidSyntaxException;
 import org.osgi.framework.ServiceReference;
 import org.osgi.service.component.ComponentConstants;
+import org.osgi.service.component.ComponentException;
 import org.osgi.service.component.ComponentFactory;
 import org.osgi.service.component.ComponentInstance;
 
@@ -94,4 +95,110 @@
         TestCase.assertEquals( 0, instanceMap.size() );
         TestCase.assertFalse( instanceMap.containsValue( instance ) );
     }
+
+
+    @Test
+    public void test_component_factory_disable_factory() throws InvalidSyntaxException
+    {
+        // tests components remain alive after factory has been disabled
+
+        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 );
+
+        Hashtable<String, String> props = new Hashtable<String, String>();
+        props.put( PROP_NAME, PROP_NAME );
+        final ComponentInstance instance = factory.newInstance( props );
+        TestCase.assertNotNull( instance );
+
+        TestCase.assertNotNull( instance.getInstance() );
+        TestCase.assertEquals( SimpleComponent.INSTANCE, instance.getInstance() );
+        TestCase.assertEquals( PROP_NAME, SimpleComponent.INSTANCE.getProperty( PROP_NAME ) );
+
+        final Map<?, ?> instanceMap = ( Map<?, ?> ) getFieldValue( component, "m_componentInstances" );
+        TestCase.assertNotNull( instanceMap );
+        TestCase.assertEquals( 1, instanceMap.size() );
+        TestCase.assertTrue( instanceMap.containsValue( instance ) );
+
+        // disable the factory
+        component.disable();
+        delay();
+
+        // factory is disabled but the instance is still alive
+        TestCase.assertEquals( Component.STATE_DISABLED, component.getState() );
+        TestCase.assertNotNull( SimpleComponent.INSTANCE );
+        TestCase.assertEquals( 1, instanceMap.size() );
+        TestCase.assertTrue( instanceMap.containsValue( instance ) );
+
+        instance.dispose();
+        TestCase.assertNull( SimpleComponent.INSTANCE );
+
+        TestCase.assertEquals( 0, instanceMap.size() );
+        TestCase.assertFalse( instanceMap.containsValue( instance ) );
+    }
+
+
+    @Test
+    public void test_component_factory_newInstance_failure() throws InvalidSyntaxException
+    {
+        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 );
+
+        Hashtable<String, String> props = new Hashtable<String, String>();
+        props.put( PROP_NAME, PROP_NAME );
+        props.put( SimpleComponent.PROP_ACTIVATE_FAILURE, "Requested Failure" );
+        try {
+            final ComponentInstance instance = factory.newInstance( props );
+            TestCase.assertNotNull( instance );
+            TestCase.fail("Expected newInstance method to fail with ComponentException");
+        } catch (ComponentException ce) {
+            // this is expected !
+        }
+
+        final Map<?, ?> instanceMap = ( Map<?, ?> ) getFieldValue( component, "m_componentInstances" );
+        TestCase.assertNotNull( instanceMap );
+        TestCase.assertTrue( instanceMap.isEmpty() );
+        TestCase.assertNull( SimpleComponent.INSTANCE );
+    }
 }
diff --git a/scr/src/test/java/org/apache/felix/scr/integration/components/SimpleComponent.java b/scr/src/test/java/org/apache/felix/scr/integration/components/SimpleComponent.java
index e1d1765..6d54707 100644
--- a/scr/src/test/java/org/apache/felix/scr/integration/components/SimpleComponent.java
+++ b/scr/src/test/java/org/apache/felix/scr/integration/components/SimpleComponent.java
@@ -33,12 +33,16 @@
 public class SimpleComponent
 {
 
-    public static SimpleComponent INSTANCE;
+    // component configuration property whose existence causes the
+    // activate method to fail
+    public static final String PROP_ACTIVATE_FAILURE = "request.activation.failure";
 
     public static final Map<Long, SimpleComponent> INSTANCES = new HashMap<Long, SimpleComponent>();
 
     public static final Set<SimpleComponent> PREVIOUS_INSTANCES = new HashSet<SimpleComponent>();
 
+    public static SimpleComponent INSTANCE;
+
     private Map<?, ?> m_config;
 
     public long m_id;
@@ -53,6 +57,12 @@
     @SuppressWarnings("unused")
     private void activate( ComponentContext activateContext, Map<?, ?> config )
     {
+        // fail activation if requested so
+        if ( config.containsKey( PROP_ACTIVATE_FAILURE ) )
+        {
+            throw new RuntimeException( String.valueOf( config.get( PROP_ACTIVATE_FAILURE ) ) );
+        }
+
         m_id = ( Long ) config.get( ComponentConstants.COMPONENT_ID );
         m_activateContext = activateContext;