FELIX-3481 - Fix targeted PID case where the service PID contains a pipe (|) symbol and add a testcase for the fix
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1358809 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java b/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java
index 33d6eb4..9eccab3 100644
--- a/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java
+++ b/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java
@@ -1606,7 +1606,8 @@
{
try
{
- final ConfigurationImpl rc = getTargetedConfiguration( this.config.getPid().getServicePid(), sr );
+ final String configPidString = this.helper.getServicePid( sr, this.config.getPid() );
+ final ConfigurationImpl rc = getTargetedConfiguration( configPidString, sr );
if ( rc != null )
{
final TargetedPID configPid;
diff --git a/configadmin/src/main/java/org/apache/felix/cm/impl/helper/BaseTracker.java b/configadmin/src/main/java/org/apache/felix/cm/impl/helper/BaseTracker.java
index fba40b9..471ee45 100644
--- a/configadmin/src/main/java/org/apache/felix/cm/impl/helper/BaseTracker.java
+++ b/configadmin/src/main/java/org/apache/felix/cm/impl/helper/BaseTracker.java
@@ -136,6 +136,24 @@
protected abstract ConfigurationMap<?> createConfigurationMap( String[] pids );
+ /**
+ * Returns the String to be used as the PID of the service PID for the
+ * {@link TargetedPID pid} retrieved from the configuration.
+ * <p>
+ * This method will return {@link TargetedPID#getServicePid()} most of
+ * the time except if the service PID used for the consumer's service
+ * registration contains one or more pipe symbols (|). In this case
+ * {@link TargetedPID#getRawPid()} might be returned.
+ *
+ * @param service The reference ot the service for which the service
+ * PID is to be returned.
+ * @param pid The {@link TargetedPID} for which to return the service
+ * PID.
+ * @return The service PID or <code>null</code> if the service does not
+ * respond to the targeted PID at all.
+ */
+ public abstract String getServicePid( ServiceReference<S> service, TargetedPID pid );
+
/**
* Updates the given service with the provided configuration.
diff --git a/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ConfigurationMap.java b/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ConfigurationMap.java
index f80641f..e782773 100644
--- a/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ConfigurationMap.java
+++ b/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ConfigurationMap.java
@@ -52,20 +52,46 @@
protected T get( final TargetedPID key )
{
- return this.configurations.get( key.getServicePid() );
+ final String servicePid = getKeyPid( key );
+ if ( servicePid != null )
+ {
+ return this.configurations.get( servicePid );
+ }
+
+ // the targeted PID does not match here
+ return null;
}
protected void put( final TargetedPID key, final T value )
{
- final String servicePid = key.getServicePid();
- if ( this.accepts( servicePid ) )
+ final String servicePid = getKeyPid( key );
+ if ( servicePid != null )
{
this.configurations.put( servicePid, value );
}
}
+ protected String getKeyPid( final TargetedPID targetedPid )
+ {
+ // regular use case: service PID is the key
+ if ( this.accepts( targetedPid.getServicePid() ) )
+ {
+ return targetedPid.getServicePid();
+ }
+
+ // the raw PID is the key (if the service PID contains pipes)
+ if ( this.accepts( targetedPid.getRawPid() ) )
+ {
+ return targetedPid.getRawPid();
+ }
+
+ // this is not really expected here
+ return null;
+ }
+
+
/**
* Returns <code>true</code> if this map is foreseen to take a
* configuration with the given service PID.
diff --git a/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceConfigurationMap.java b/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceConfigurationMap.java
index d70c757..c5b6f12 100644
--- a/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceConfigurationMap.java
+++ b/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceConfigurationMap.java
@@ -75,7 +75,6 @@
// update if the used targeted PID matches
if ( configPid.equals( entry.targetedPid ) )
{
- this.put( configPid, null );
return true;
}
diff --git a/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceFactoryTracker.java b/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceFactoryTracker.java
index 0a5e79d..f57f3c4 100644
--- a/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceFactoryTracker.java
+++ b/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceFactoryTracker.java
@@ -40,6 +40,18 @@
}
+ /**
+ * Always returns the raw PID because for a ManagedServiceFactory
+ * the configuration's PID is automatically generated and is not a
+ * real targeted PID.
+ */
+ @Override
+ public String getServicePid( ServiceReference<ManagedServiceFactory> service, TargetedPID pid )
+ {
+ return pid.getRawPid();
+ }
+
+
@Override
public void provideConfiguration( ServiceReference<ManagedServiceFactory> reference, TargetedPID configPid,
TargetedPID factoryPid, Dictionary<String, ?> properties, long revision )
diff --git a/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceTracker.java b/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceTracker.java
index e0f659e..b0d8c68 100644
--- a/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceTracker.java
+++ b/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceTracker.java
@@ -46,6 +46,20 @@
}
+ @Override
+ public String getServicePid( ServiceReference<ManagedService> service, TargetedPID pid )
+ {
+ final ConfigurationMap configs = this.getService( service );
+ if ( configs != null )
+ {
+ return configs.getKeyPid( pid );
+ }
+
+ // this service is not handled...
+ return null;
+ }
+
+
/**
* Provides the given configuration to the managed service.
* <p>
diff --git a/configadmin/src/test/java/org/apache/felix/cm/integration/TargetedPidTest.java b/configadmin/src/test/java/org/apache/felix/cm/integration/TargetedPidTest.java
index 7c56040..b35d08e 100644
--- a/configadmin/src/test/java/org/apache/felix/cm/integration/TargetedPidTest.java
+++ b/configadmin/src/test/java/org/apache/felix/cm/integration/TargetedPidTest.java
@@ -222,4 +222,52 @@
}
}
+ @Test
+ public void test_pid_with_pipe() throws BundleException
+ {
+ final String pid0 = "test_targeted";
+ final String pid1 = String.format( "%s|%s", pid0, ManagedServiceTestActivator.class.getName() );
+ try
+ {
+
+ // start the bundle and assert this
+ bundle = installBundle( pid1 );
+
+ configure( pid0 );
+ configure( pid1 );
+
+ bundle.start();
+ final ManagedServiceTestActivator tester = ManagedServiceTestActivator.INSTANCE;
+ TestCase.assertNotNull( "Activator not started !!", tester );
+
+ // give cm time for distribution
+ delay();
+
+ // assert activater has configuration
+ int callCount = 0;
+ TestCase.assertNotNull( "Expect Properties after update", tester.props );
+ TestCase.assertEquals( "Expect PID", pid1, tester.props.get( Constants.SERVICE_PID ) );
+ TestCase.assertEquals( "Expect calls", ++callCount, tester.numManagedServiceUpdatedCalls );
+
+ // delete pid1 - don't expect pid0 is assigned
+ deleteConfig( pid1 );
+ delay();
+
+ // final delete
+ TestCase.assertNull( "Expect no Properties after delete", tester.props );
+ TestCase.assertEquals( "Expect calls", ++callCount, tester.numManagedServiceUpdatedCalls );
+
+ // cleanup
+ bundle.uninstall();
+ bundle = null;
+
+ }
+ finally
+ {
+ // remove the configuration for good
+ deleteConfig( pid0 );
+ deleteConfig( pid1 );
+ }
+ }
+
}