FELIX-1943 Ensure components which are not satisfied any longer if a service is unregistered are deactivated. Before this would only happen if the component had the service bound, which is not always the case (for example for ComponentFactory components).
FELIX-1944 Ensure Reference.getServiceReferences() returns null if no services are bound to the component (an empty array used to be returned).
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@890260 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java
index 31763c1..b8a71bd 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java
@@ -27,6 +27,7 @@
import java.util.List;
import java.util.Map;
+import org.apache.felix.scr.Component;
import org.apache.felix.scr.Reference;
import org.apache.felix.scr.impl.helper.BindMethod;
import org.apache.felix.scr.impl.helper.UnbindMethod;
@@ -52,9 +53,8 @@
public class DependencyManager implements ServiceListener, Reference
{
// mask of states ok to send events
- private static final int STATE_MASK = AbstractComponentManager.STATE_UNSATISFIED
- | AbstractComponentManager.STATE_ACTIVATING | AbstractComponentManager.STATE_ACTIVE
- | AbstractComponentManager.STATE_REGISTERED | AbstractComponentManager.STATE_FACTORY;
+ private static final int STATE_MASK = Component.STATE_UNSATISFIED | Component.STATE_ACTIVATING
+ | Component.STATE_ACTIVE | Component.STATE_REGISTERED | Component.STATE_FACTORY;
// pseudo service to mark a bound service without actual service instance
private static final Object BOUND_SERVICE_SENTINEL = new Object();
@@ -327,49 +327,36 @@
*
* @param reference The reference to the service unregistering or being
* modified.
- *
- * @return <code>true</code> if the service has been removed without the
- * component being deactivated. <code>true</code> is also returned
- * if the service was not bound at all. <code>false</code> is returned
- * if the component has been deactivated due to this service being
- * removed.
*/
- private boolean serviceRemoved( ServiceReference reference )
+ private void serviceRemoved( ServiceReference reference )
{
+ // if the dependency is not satisfied anymore, we have to
+ // deactivate the component
+ if ( !isSatisfied() )
+ {
+ m_componentManager.log( LogService.LOG_DEBUG,
+ "Dependency Manager: Deactivating component due to mandatory dependency on {0}/{1} not satisfied",
+ new Object[]
+ { m_dependencyMetadata.getName(), m_dependencyMetadata.getInterface() }, null );
+
+ // deactivate the component now
+ m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE );
+ }
+
// check whether we are bound to that service, do nothing if not
if ( getBoundService( reference ) == null )
{
m_componentManager.log( LogService.LOG_DEBUG,
"Dependency Manager: Ignoring removed Service for {0} : Service {1} not bound", new Object[]
{ m_dependencyMetadata.getName(), reference.getProperty( Constants.SERVICE_ID ) }, null );
-
- // service was not bound, we can continue without interruption
- return true;
}
// otherwise check whether the component is in a state to handle the event
else if ( handleServiceEvent() )
{
-
- // if the dependency is not satisfied anymore, we have to
- // deactivate the component
- if ( !isSatisfied() )
- {
- m_componentManager.log( LogService.LOG_DEBUG,
- "Dependency Manager: Deactivating component due to mandatory dependency on {0}/{1} not satisfied",
- new Object[]
- { m_dependencyMetadata.getName(), m_dependencyMetadata.getInterface() }, null );
-
- // deactivate the component now
- m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE );
-
- // component is deactivated, this does all for this service
- return false;
- }
-
// if the dependency is static, we have to reactivate the component
// to "remove" the dependency
- else if ( m_dependencyMetadata.isStatic() )
+ if ( m_dependencyMetadata.isStatic() )
{
try
{
@@ -383,9 +370,6 @@
{
m_componentManager.log( LogService.LOG_ERROR, "Exception while recreating dependency ", ex );
}
-
- // static reference removal causes reactivation, nothing more to do
- return false;
}
// dynamic dependency, multiple or single but this service is the bound one
@@ -407,10 +391,6 @@
new Object[]
{ m_dependencyMetadata.getName(), m_dependencyMetadata.getInterface() }, null );
m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE );
-
- // required service could not be replaced, component
- // is deactivated and we are done
- return false;
}
}
@@ -431,9 +411,6 @@
"Dependency Manager: Ignoring service removal, wrong state {0}", new Object[]
{ m_componentManager.state() }, null );
}
-
- // everything is fine, the component is still active and we continue
- return true;
}
@@ -700,10 +677,16 @@
/**
* Returns an array of <code>ServiceReference</code> instances of all
- * services this instance is bound to.
+ * services this instance is bound to or <code>null</code> if no services
+ * are actually bound.
*/
private ServiceReference[] getBoundServiceReferences()
{
+ if ( m_bound.isEmpty() )
+ {
+ return null;
+ }
+
return ( ServiceReference[] ) m_bound.keySet().toArray( new ServiceReference[m_bound.size()] );
}
@@ -960,12 +943,12 @@
*/
private void unbind( ServiceReference[] boundRefs )
{
- // only invoke the unbind method if there is an instance (might be null
- // in the delayed component situation) and the unbind method is declared.
- boolean doUnbind = m_componentInstance != null && m_dependencyMetadata.getUnbind() != null;
-
if ( boundRefs != null )
{
+ // only invoke the unbind method if there is an instance (might be null
+ // in the delayed component situation) and the unbind method is declared.
+ boolean doUnbind = m_componentInstance != null && m_dependencyMetadata.getUnbind() != null;
+
for ( int i = 0; i < boundRefs.length; i++ )
{
if ( doUnbind )
@@ -982,6 +965,7 @@
}
}
+
/**
* Calls the bind method. In case there is an exception while calling the
* bind method, the service is not considered to be bound to the instance
diff --git a/scr/src/test/java/org/apache/felix/scr/integration/ServiceBindTest.java b/scr/src/test/java/org/apache/felix/scr/integration/ServiceBindTest.java
index 1404012..5c8ae65 100644
--- a/scr/src/test/java/org/apache/felix/scr/integration/ServiceBindTest.java
+++ b/scr/src/test/java/org/apache/felix/scr/integration/ServiceBindTest.java
@@ -19,6 +19,8 @@
package org.apache.felix.scr.integration;
+import java.util.Hashtable;
+
import junit.framework.TestCase;
import org.apache.felix.scr.Component;
@@ -27,11 +29,19 @@
import org.junit.Test;
import org.junit.runner.RunWith;
import org.ops4j.pax.exam.junit.JUnit4TestRunner;
+import org.osgi.framework.InvalidSyntaxException;
+import org.osgi.framework.ServiceReference;
+import org.osgi.service.component.ComponentConstants;
+import org.osgi.service.component.ComponentFactory;
+import org.osgi.service.component.ComponentInstance;
@RunWith(JUnit4TestRunner.class)
public class ServiceBindTest extends ComponentTestBase
{
+
+ private static final String PROP_NAME_FACTORY = ComponentTestBase.PROP_NAME + ".factory";
+
static
{
// uncomment to enable debugging of this test class
@@ -550,6 +560,142 @@
@Test
+ public void test_required_multiple_dynamic_factory() throws InvalidSyntaxException
+ {
+ final String pid = "test_required_multiple_dynamic_factory";
+ final String factoryPid = "factory_" + pid;
+
+ final Component component = findComponentByName( pid );
+ TestCase.assertNotNull( component );
+ TestCase.assertEquals( Component.STATE_DISABLED, component.getState() );
+
+ // async enabling (unsatisfied)
+ component.enable();
+ delay();
+ TestCase.assertEquals( Component.STATE_UNSATISFIED, component.getState() );
+
+ // register service, satisfying
+ final SimpleServiceImpl srv1 = SimpleServiceImpl.create( bundleContext, "srv1" );
+ delay();
+ TestCase.assertEquals( Component.STATE_FACTORY, component.getState() );
+
+ // create a component instance
+ final ServiceReference[] refs = bundleContext.getServiceReferences( ComponentFactory.class.getName(), "("
+ + ComponentConstants.COMPONENT_FACTORY + "=" + factoryPid + ")" );
+ 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_FACTORY, PROP_NAME_FACTORY );
+ final ComponentInstance instance = factory.newInstance( props );
+ TestCase.assertNotNull( instance );
+ TestCase.assertNotNull( instance.getInstance() );
+ TestCase.assertEquals( SimpleComponent.INSTANCE, instance.getInstance() );
+
+ // ensure instance is bound
+ final SimpleComponent sc = SimpleComponent.INSTANCE;
+ TestCase.assertEquals( 1, sc.m_multiRef.size() );
+ TestCase.assertTrue( sc.m_multiRef.contains( srv1 ) );
+
+ // ensure factory is not bound
+ TestCase.assertNull( component.getReferences()[0].getServiceReferences() );
+
+ // assert two components managed
+ final Component[] allFactoryComponents = findComponentsByName( pid );
+ 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 );
+ }
+ }
+
+ // register second service
+ final SimpleServiceImpl srv11 = SimpleServiceImpl.create( bundleContext, "srv11" );
+ delay();
+
+ // ensure instance is bound
+ TestCase.assertEquals( 2, sc.m_multiRef.size() );
+ TestCase.assertTrue( sc.m_multiRef.contains( srv1 ) );
+ TestCase.assertTrue( sc.m_multiRef.contains( srv11 ) );
+
+ // ensure factory is not bound
+ TestCase.assertNull( component.getReferences()[0].getServiceReferences() );
+
+ // drop second service and ensure unbound (and active)
+ srv11.drop();
+ delay();
+ TestCase.assertNotNull( instance.getInstance() );
+ TestCase.assertEquals( SimpleComponent.INSTANCE, instance.getInstance() );
+ TestCase.assertEquals( 1, sc.m_multiRef.size() );
+ TestCase.assertTrue( sc.m_multiRef.contains( srv1 ) );
+ TestCase.assertNull( component.getReferences()[0].getServiceReferences() );
+
+
+ // remove the service, expect factory to deactivate and instance to dispose
+ srv1.drop();
+ delay();
+
+ TestCase.assertEquals( Component.STATE_UNSATISFIED, component.getState() );
+ TestCase.assertNull( instance.getInstance() );
+
+ // assert component factory only managed
+ final Component[] allFactoryComponents2 = findComponentsByName( pid );
+ TestCase.assertNotNull( allFactoryComponents2 );
+ TestCase.assertEquals( 1, allFactoryComponents2.length );
+ for ( int i = 0; i < allFactoryComponents2.length; i++ )
+ {
+ final Component c = allFactoryComponents2[i];
+ if ( c.getId() == component.getId() )
+ {
+ TestCase.assertEquals( Component.STATE_UNSATISFIED, c.getState() );
+ }
+ else
+ {
+ TestCase.fail( "Unexpected Component " + c );
+ }
+ }
+
+ // registeranother service, factory must come back, instance not
+ final SimpleServiceImpl srv2 = SimpleServiceImpl.create( bundleContext, "srv2" );
+ delay();
+
+ TestCase.assertEquals( Component.STATE_FACTORY, component.getState() );
+ TestCase.assertNull( instance.getInstance() );
+
+ // assert component factory only managed
+ final Component[] allFactoryComponents3 = findComponentsByName( pid );
+ TestCase.assertNotNull( allFactoryComponents3 );
+ TestCase.assertEquals( 1, allFactoryComponents3.length );
+ for ( int i = 0; i < allFactoryComponents3.length; i++ )
+ {
+ final Component c = allFactoryComponents3[i];
+ if ( c.getId() == component.getId() )
+ {
+ TestCase.assertEquals( Component.STATE_FACTORY, c.getState() );
+ }
+ else
+ {
+ TestCase.fail( "Unexpected Component " + c );
+ }
+ }
+ }
+
+
+ @Test
public void test_optional_single_static()
{
final Component component = findComponentByName( "test_optional_single_static" );
diff --git a/scr/src/test/resources/integration_test_simple_components_service_binding.xml b/scr/src/test/resources/integration_test_simple_components_service_binding.xml
index 9c8c630..afba9e1 100644
--- a/scr/src/test/resources/integration_test_simple_components_service_binding.xml
+++ b/scr/src/test/resources/integration_test_simple_components_service_binding.xml
@@ -103,6 +103,21 @@
/>
</scr:component>
+ <scr:component name="test_required_multiple_dynamic_factory"
+ enabled="false"
+ configuration-policy="ignore"
+ factory="factory_test_required_multiple_dynamic_factory">
+ <implementation class="org.apache.felix.scr.integration.components.SimpleComponent" />
+ <reference
+ name="ref"
+ interface="org.apache.felix.scr.integration.components.SimpleService"
+ cardinality="1..n"
+ policy="dynamic"
+ bind="bindSimpleService"
+ unbind="unbindSimpleService"
+ />
+ </scr:component>
+
<scr:component name="test_optional_multiple_static"
enabled="false"
configuration-policy="ignore">