FELIX-3639 fix up Factory instance behavior when component instance is registered as a service

git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1377921 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 acacd39..fc74cd9 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
@@ -654,6 +654,8 @@
 
     abstract State getSatisfiedState();
 
+    abstract State getActiveState();
+
     /**
      * Registers the service on behalf of the component.
      *
@@ -1344,10 +1346,7 @@
                 acm.escalateLock( "AbstractComponentManager.Unsatisifed.activate.1" );
                 try
                 {
-                    if ( acm.isImmediate() )
-                    {
-                        acm.changeState( Active.getInstance() );
-                    }
+                    acm.changeState( acm.getActiveState() );
                     if ( !acm.createComponent() )
                     {
                         // component creation failed, not active now
@@ -1549,17 +1548,16 @@
      * deactivated due to not being satisified any longer. See section 112.5.5,
      * Factory Component, for full details.
      */
-    protected static final class FactoryInstance extends Satisfied//Registered
+    protected static final class FactoryInstance extends Satisfied
     {
         private static final FactoryInstance m_inst = new FactoryInstance();
 
 
         private FactoryInstance()
         {
-            super( "Active", STATE_ACTIVE );
+            super("FactoryInstance", STATE_ACTIVE);
         }
 
-
         static State getInstance()
         {
             return m_inst;
@@ -1567,35 +1565,18 @@
 
         Object getService( ImmediateComponentManager dcm )
         {
-            if ( dcm.createComponent() )
-            {
-                dcm.changeState( Active.getInstance() );
-                return dcm.getInstance();
-            }
+            return dcm.getInstance();
+        }
 
-            // log that the delayed component cannot be created (we don't
-            // know why at this moment; this should already have been logged)
-            dcm.log( LogService.LOG_ERROR, "Failed creating the component instance; see log for reason", null );
 
-            // component could not really be created. This may be temporary
-            // so we stay in the registered state but ensure the component
-            // instance is deleted
-            try
-            {
-                dcm.deleteComponent( ComponentConstants.DEACTIVATION_REASON_UNSPECIFIED );
-            }
-            catch ( Throwable t )
-            {
-                dcm.log( LogService.LOG_DEBUG, "Cannot delete incomplete component instance. Ignoring.", t );
-            }
-
-            // no service can be returned (be prepared for more logging !!)
-            return null;
+        void ungetService( ImmediateComponentManager dcm )
+        {
+            dcm.deleteComponent( ComponentConstants.DEACTIVATION_REASON_UNSPECIFIED );
+            dcm.changeState( Registered.getInstance() );
         }
 
         void deactivate( AbstractComponentManager acm, int reason )
         {
-            acm.changeState( Active.getInstance() );
             acm.disposeInternal( reason );
         }
     }
@@ -1636,7 +1617,7 @@
 
         void dispose( AbstractComponentManager acm )
         {
-            throw new IllegalStateException( "dispose: " + this );
+            //factory instance can have dispose called with no effect. 112.5.5
         }
 
         void enable( AbstractComponentManager acm )
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 f146dc7..1b199ac 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
@@ -250,6 +250,11 @@
         return Factory.getInstance();
     }
 
+    State getActiveState()
+    {
+        return Factory.getInstance();
+    }
+
     //---------- Component interface
 
 
@@ -396,7 +401,7 @@
             super( activator, componentHolder, metadata );
         }
 
-        State getSatisfiedState()
+        State getActiveState()
         {
             return FactoryInstance.getInstance();
         }
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java
index 00c019a..21de92d 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java
@@ -300,6 +300,11 @@
         return Registered.getInstance();
     }
 
+    State getActiveState()
+    {
+        return Active.getInstance();
+    }
+
     protected void setFactoryProperties( Dictionary dictionary )
     {
         m_factoryProperties = copyTo( null, dictionary );
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 e6b1db3..b7c2dae 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
@@ -512,10 +512,10 @@
         TestCase.assertNull( instanceNonMatch.getInstance() );
         TestCase.assertNull( SimpleComponent.INSTANCE );
 
-        //FactoryInstance.deactivate disposes the instance.  Don't do it again
-//        instanceNonMatch.dispose();
-//        TestCase.assertNull( SimpleComponent.INSTANCE );
-//        TestCase.assertNull( instanceNonMatch.getInstance() ); // SCR 112.12.6.2
+        //Check that calling dispose on a deactivated instance has no effect
+        instanceNonMatch.dispose();
+        TestCase.assertNull( SimpleComponent.INSTANCE );
+        TestCase.assertNull( instanceNonMatch.getInstance() ); // SCR 112.12.6.2
     }
 
     @Test