FELIX-3506 Apply patch by David Jencks (thanks alot)
- allow bind, updated, and unbind methods to update service registration properties
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1337437 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/helper/ActivateMethod.java b/scr/src/main/java/org/apache/felix/scr/impl/helper/ActivateMethod.java
index 6fc381b..3589468 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/helper/ActivateMethod.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/helper/ActivateMethod.java
@@ -142,12 +142,6 @@
return "activate";
}
- protected boolean returnValue()
- {
- // allow returning Map if declared as DS 1.2-Felix or newer
- return isDS12Felix();
- }
-
public MethodResult invoke(Object componentInstance, Object rawParameter, final MethodResult methodCallFailureResult)
{
if (methodExists())
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/helper/BaseMethod.java b/scr/src/main/java/org/apache/felix/scr/impl/helper/BaseMethod.java
index 3abe587..cc2c912 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/helper/BaseMethod.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/helper/BaseMethod.java
@@ -259,14 +259,10 @@
return MethodResult.VOID; // TODO: or null ??
}
- protected void processResult( Object configResults, Object result, Method method )
- {
- //no op
- }
-
protected boolean returnValue()
{
- return false;
+ // allow returning Map if declared as DS 1.2-Felix or newer
+ return isDS12Felix();
}
/**
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 08f7f93..e014c21 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
@@ -30,6 +30,7 @@
import org.apache.felix.scr.Component;
import org.apache.felix.scr.Reference;
import org.apache.felix.scr.impl.BundleComponentActivator;
+import org.apache.felix.scr.impl.helper.MethodResult;
import org.apache.felix.scr.impl.metadata.ComponentMetadata;
import org.apache.felix.scr.impl.metadata.ReferenceMetadata;
import org.apache.felix.scr.impl.metadata.ServiceMetadata;
@@ -797,7 +798,7 @@
public abstract Dictionary getProperties();
- public abstract void setServiceProperties(Dictionary serviceProperties, boolean updateServiceRegistration);
+ public abstract void setServiceProperties( Dictionary serviceProperties );
/**
* Returns the subset of component properties to be used as service
@@ -900,6 +901,15 @@
m_state = newState;
}
+ public void setServiceProperties( MethodResult methodResult )
+ {
+ if ( methodResult.hasResult() )
+ {
+ Dictionary serviceProps = ( methodResult.getResult() == null) ? null : new Hashtable( methodResult.getResult() );
+ setServiceProperties(serviceProps );
+ }
+ }
+
//--------- State classes
/**
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.java
index 0b0aa76..6fd2b45 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.java
@@ -136,7 +136,7 @@
public void setServiceProperties(Dictionary properties)
{
- getComponentManager().setServiceProperties(properties, true);
+ getComponentManager().setServiceProperties(properties );
}
}
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 7039d17..d7a4f1e 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
@@ -209,7 +209,7 @@
return props;
}
- public void setServiceProperties(Dictionary serviceProperties, boolean updateServiceRegistration)
+ public void setServiceProperties( Dictionary serviceProperties )
{
throw new IllegalStateException( "ComponentFactory service properties are immutable" );
}
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 fe25047..27373ac 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
@@ -1047,7 +1047,7 @@
{
if ( m_bind != null )
{
- return m_bind.invoke( m_componentInstance, new BindMethod.Service()
+ MethodResult result = m_bind.invoke( m_componentInstance, new BindMethod.Service()
{
public ServiceReference getReference()
{
@@ -1060,7 +1060,13 @@
{
return getService( ref );
}
- }, MethodResult.VOID ) != null;
+ }, MethodResult.VOID );
+ if ( result == null )
+ {
+ return false;
+ }
+ m_componentManager.setServiceProperties( result );
+ return true;
}
// Concurrency Issue: The component instance still exists but
@@ -1113,10 +1119,9 @@
{
// The updated method is only invoked if the implementation object is not
// null. This is valid for both immediate and delayed components
- //TODO should updated methods be able to change config properties?
if ( m_componentInstance != null )
{
- m_updated.invoke( m_componentInstance, new BindMethod.Service()
+ MethodResult methodResult = m_updated.invoke( m_componentInstance, new BindMethod.Service()
{
public ServiceReference getReference()
{
@@ -1129,6 +1134,10 @@
return getService( ref );
}
}, MethodResult.VOID );
+ if ( methodResult != null)
+ {
+ m_componentManager.setServiceProperties( methodResult );
+ }
}
else
{
@@ -1157,7 +1166,7 @@
// null. This is valid for both immediate and delayed components
if ( m_componentInstance != null )
{
- m_unbind.invoke( m_componentInstance, new BindMethod.Service()
+ MethodResult methodResult = m_unbind.invoke( m_componentInstance, new BindMethod.Service()
{
public ServiceReference getReference()
{
@@ -1170,6 +1179,10 @@
return getService( ref );
}
}, MethodResult.VOID );
+ if ( methodResult != null )
+ {
+ m_componentManager.setServiceProperties( methodResult );
+ }
}
else
{
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 437bdb6..8eff631 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
@@ -23,7 +23,6 @@
import java.util.Hashtable;
import java.util.Iterator;
import java.util.List;
-import java.util.Map;
import org.apache.felix.scr.impl.BundleComponentActivator;
import org.apache.felix.scr.impl.config.ComponentHolder;
@@ -233,7 +232,7 @@
// 4. Call the activate method, if present
final MethodResult result = m_activateMethod.invoke(implementationObject, new ActivatorParameter(
componentContext, 1), null);
- if (result == null)
+ if ( result == null )
{
// 112.5.8 If the activate method throws an exception, SCR must log an error message
// containing the exception with the Log Service and activation fails
@@ -246,9 +245,9 @@
return null;
}
- else if (result.hasResult())
+ else
{
- setServiceProperties(result.getResult(), true);
+ setServiceProperties( result );
}
return implementationObject;
@@ -272,9 +271,9 @@
// exception with the Log Service and continue) has already been logged
final MethodResult result = m_deactivateMethod.invoke( implementationObject, new ActivatorParameter( componentContext,
reason ), null );
- if (result != null && result.hasResult())
+ if ( result != null )
{
- setServiceProperties(result.getResult(), true);
+ setServiceProperties( result );
}
// 2. Unbind any bound services
@@ -376,14 +375,7 @@
return m_properties;
}
-
- public void setServiceProperties(Map serviceProperties, boolean updateServiceRegistration)
- {
- Dictionary serviceProps = (serviceProperties == null) ? null : new Hashtable(serviceProperties);
- setServiceProperties(serviceProps, updateServiceRegistration);
- }
-
- public void setServiceProperties(Dictionary serviceProperties, boolean updateServiceRegistration)
+ public void setServiceProperties( Dictionary serviceProperties )
{
if ( serviceProperties == null || serviceProperties.isEmpty() )
{
@@ -397,10 +389,7 @@
m_serviceProperties.put( ComponentConstants.COMPONENT_ID, new Long( getId() ) );
}
- if (updateServiceRegistration)
- {
- updateServiceRegistration();
- }
+ updateServiceRegistration();
}
public Dictionary getServiceProperties() {
@@ -550,7 +539,7 @@
// 4. call method (nothing to do when failed, since it has already been logged)
final MethodResult result = m_modifyMethod.invoke(getInstance(),
new ActivatorParameter(m_componentContext, -1), null);
- if (result == null)
+ if ( result == null )
{
// log an error if the declared method cannot be found
log( LogService.LOG_ERROR, "Declared modify method ''{0}'' cannot be found, configuring by reactivation",
@@ -558,10 +547,6 @@
{ getComponentMetadata().getModified() }, null );
return false;
}
- else if (result.hasResult())
- {
- setServiceProperties(result.getResult(), false);
- }
// 5. update the target filter on the services now, this may still
// result in unsatisfied dependencies, in which case we abort
@@ -575,10 +560,17 @@
return false;
}
- // 6. update service registration properties
- updateServiceRegistration();
+ // 6. update service registration properties if we didn't just do it
+ if ( result.hasResult() )
+ {
+ setServiceProperties( result );
+ }
+ else
+ {
+ updateServiceRegistration();
+ }
- // 7. everything set and done, the component has been udpated
+ // 7. everything set and done, the component has been updated
return true;
}
diff --git a/scr/src/test/java/org/apache/felix/scr/integration/MutablePropertiesTest.java b/scr/src/test/java/org/apache/felix/scr/integration/MutablePropertiesTest.java
index bc4bb61..b225ed8 100644
--- a/scr/src/test/java/org/apache/felix/scr/integration/MutablePropertiesTest.java
+++ b/scr/src/test/java/org/apache/felix/scr/integration/MutablePropertiesTest.java
@@ -27,6 +27,7 @@
import junit.framework.TestCase;
import org.apache.felix.scr.Component;
import org.apache.felix.scr.integration.components.MutatingService;
+import org.apache.felix.scr.integration.components.SimpleServiceImpl;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.ops4j.pax.exam.junit.JUnit4TestRunner;
@@ -41,7 +42,7 @@
static
{
// uncomment to enable debugging of this test class
- // paxRunnerVmOption = DEBUG_VM_OPTION;
+ //paxRunnerVmOption = DEBUG_VM_OPTION;
descriptorFile = "/integration_test_mutable_properties.xml";
}
@@ -83,11 +84,11 @@
@Test
public void test_mutable_properties_returned() throws InvalidSyntaxException
{
- final Component component = findComponentByName( "components.mutable.properties2" );
+ final Component component = findComponentByName( "components.mutable.properties.return" );
TestCase.assertNotNull( component );
TestCase.assertEquals( Component.STATE_REGISTERED, component.getState() );
- ServiceReference[] serviceReferences = bundleContext.getServiceReferences( MutatingService.class.getName(), "(service.pid=components.mutable.properties2)" );
+ ServiceReference[] serviceReferences = bundleContext.getServiceReferences( MutatingService.class.getName(), "(service.pid=components.mutable.properties.return)" );
TestCase.assertEquals( 1, serviceReferences.length );
ServiceReference serviceReference = serviceReferences[0];
checkProperties( serviceReference, 8, "otherValue", "p1", "p2" );
@@ -102,7 +103,7 @@
checkProperties(serviceReference, 5, "anotherValue", "p1", "p2");
//configure with configAdmin
- configure( "components.mutable.properties2" );
+ configure( "components.mutable.properties.return" );
delay();
delay();
//no change
@@ -114,10 +115,40 @@
bundleContext.ungetService(serviceReference);
}
+ @Test
+ public void test_mutable_properties_bind_returned() throws InvalidSyntaxException
+ {
+ final Component component = findComponentByName( "components.mutable.properties.bind" );
+ TestCase.assertNotNull( component );
+ TestCase.assertEquals( Component.STATE_REGISTERED, component.getState() );
+
+ ServiceReference[] serviceReferences = bundleContext.getServiceReferences( MutatingService.class.getName(), "(service.pid=components.mutable.properties.bind)" );
+ TestCase.assertEquals( 1, serviceReferences.length );
+ ServiceReference serviceReference = serviceReferences[0];
+ checkProperties( serviceReference, 8, "otherValue", "p1", "p2" );
+ MutatingService s = ( MutatingService ) bundleContext.getService( serviceReference );
+
+ SimpleServiceImpl srv1 = SimpleServiceImpl.create( bundleContext, "srv1" );
+ checkProperties( serviceReference, 5, null, "p1", "p2" );
+ Assert.assertEquals("bound", serviceReference.getProperty("SimpleService"));
+
+ srv1.update( "foo" );
+ checkProperties( serviceReference, 5, null, "p1", "p2" );
+ Assert.assertEquals("updated", serviceReference.getProperty("SimpleService"));
+
+ srv1.drop();
+ checkProperties( serviceReference, 5, null, "p1", "p2" );
+ Assert.assertEquals("unbound", serviceReference.getProperty("SimpleService"));
+
+ bundleContext.ungetService(serviceReference);
+ }
private void checkProperties(ServiceReference serviceReference, int count, String otherValue, String p1, String p2) {
Assert.assertEquals("wrong property count", count, serviceReference.getPropertyKeys().length);
- Assert.assertEquals(otherValue, serviceReference.getProperty(PROP_NAME));
+ if ( otherValue != null )
+ {
+ Assert.assertEquals(otherValue, serviceReference.getProperty(PROP_NAME));
+ }
if ( count > 5 ) {
Assert.assertEquals(p1, serviceReference.getProperty("p1"));
Assert.assertEquals(p2, serviceReference.getProperty("p2"));
diff --git a/scr/src/test/java/org/apache/felix/scr/integration/components/MutatingServiceImpl.java b/scr/src/test/java/org/apache/felix/scr/integration/components/MutatingServiceImpl.java
index faf9554..07554c1 100644
--- a/scr/src/test/java/org/apache/felix/scr/integration/components/MutatingServiceImpl.java
+++ b/scr/src/test/java/org/apache/felix/scr/integration/components/MutatingServiceImpl.java
@@ -19,6 +19,7 @@
package org.apache.felix.scr.integration.components;
+import java.util.Collections;
import java.util.Dictionary;
import java.util.Hashtable;
import java.util.Map;
@@ -45,26 +46,43 @@
{
this.activateContext = activateContext;
Map result = new Hashtable( (Map )activateContext.getProperties() );
- result.put( "theValue", "anotherValue1");
+ result.put( "theValue", "anotherValue1" );
return result;
}
private Map modifiedMutate( ComponentContext activateContext )
{
Map result = new Hashtable( (Map )activateContext.getProperties() );
- result.put( "theValue", "anotherValue2");
+ result.put( "theValue", "anotherValue2" );
return result;
}
private Map deactivateMutate( ComponentContext activateContext )
{
Map result = new Hashtable( (Map )activateContext.getProperties() );
- result.put( "theValue", "anotherValue3");
+ result.put( "theValue", "anotherValue3" );
return result;
}
- public void updateProperties(Dictionary changes) {
- ((ExtComponentContext)activateContext).setServiceProperties(changes);
+ public void updateProperties( Dictionary changes )
+ {
+ ( ( ExtComponentContext ) activateContext ).setServiceProperties( changes );
}
+ private Map bindSimpleService( SimpleService ss )
+ {
+ return Collections.singletonMap( "SimpleService", "bound" );
+ }
+
+ private Map unbindSimpleService( SimpleService ss )
+ {
+ return Collections.singletonMap( "SimpleService", "unbound" );
+ }
+
+ private Map updateSimpleService( SimpleService ss )
+ {
+ return Collections.singletonMap( "SimpleService", "updated" );
+ }
+
+
}
diff --git a/scr/src/test/resources/integration_test_mutable_properties.xml b/scr/src/test/resources/integration_test_mutable_properties.xml
index d9052cf..08214ef 100644
--- a/scr/src/test/resources/integration_test_mutable_properties.xml
+++ b/scr/src/test/resources/integration_test_mutable_properties.xml
@@ -33,7 +33,8 @@
<property name="p1" value="p1" />
<property name="p2" value="p2" />
</scr:component>
- <scr:component name="components.mutable.properties2"
+
+ <scr:component name="components.mutable.properties.return"
xmlns:scr="http://felix.apache.org/xmlns/scr/v1.2.0-felix"
enabled="true"
configuration-policy="optional"
@@ -44,9 +45,35 @@
<service>
<provide interface="org.apache.felix.scr.integration.components.MutatingService" />
</service>
- <property name="service.pid" value="components.mutable.properties2" />
+ <property name="service.pid" value="components.mutable.properties.return" />
<property name="theValue" value="otherValue" />
<property name="p1" value="p1" />
<property name="p2" value="p2" />
</scr:component>
+
+ <scr:component name="components.mutable.properties.bind"
+ xmlns:scr="http://felix.apache.org/xmlns/scr/v1.2.0-felix"
+ enabled="true"
+ configuration-policy="optional"
+ activate="activateMutate"
+ modified="modifiedMutate"
+ deactivate="deactivateMutate">
+ <implementation class="org.apache.felix.scr.integration.components.MutatingServiceImpl"/>
+ <service>
+ <provide interface="org.apache.felix.scr.integration.components.MutatingService"/>
+ </service>
+ <reference
+ name="simpleService"
+ interface="org.apache.felix.scr.integration.components.SimpleService"
+ cardinality="0..1"
+ policy="dynamic"
+ bind="bindSimpleService"
+ unbind="unbindSimpleService"
+ updated="updateSimpleService"
+ />
+ <property name="service.pid" value="components.mutable.properties.bind"/>
+ <property name="theValue" value="otherValue"/>
+ <property name="p1" value="p1"/>
+ <property name="p2" value="p2"/>
+ </scr:component>
</components>