FELIX-3584 start work on location changed. Breaks factoryPIDs
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1480104 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/config/ComponentHolder.java b/scr/src/main/java/org/apache/felix/scr/impl/config/ComponentHolder.java
index 83515c5..0c0a240 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/config/ComponentHolder.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/config/ComponentHolder.java
@@ -23,6 +23,7 @@
import org.apache.felix.scr.Component;
import org.apache.felix.scr.impl.BundleComponentActivator;
+import org.apache.felix.scr.impl.TargetedPID;
import org.apache.felix.scr.impl.manager.ImmediateComponentManager;
import org.apache.felix.scr.impl.metadata.ComponentMetadata;
@@ -69,14 +70,21 @@
* @param pid The PID of the configuration used to configure the component.
* @param props the property dictionary from the configuration.
* @param changeCount change count of the configuration, or R4 imitation.
+ * @param targetedPid TODO
*/
- void configurationUpdated( String pid, Dictionary<String, Object> props, long changeCount );
+ void configurationUpdated( String pid, Dictionary<String, Object> props, long changeCount, TargetedPID targetedPid );
/**
* Change count (or fake R4 imitation)
* @return the last change count from a configurationUpdated call for the given pid.
*/
long getChangeCount( String pid );
+
+ /**
+ * Returns the targeted PID used to configure this component
+ * @return
+ */
+ TargetedPID getConfigurationTargetedPID();
/**
* Returns all <code>Component</code> instances held by this holder.
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurationSupport.java b/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurationSupport.java
index 3fe70ec..1ec228c 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurationSupport.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurationSupport.java
@@ -127,9 +127,12 @@
{
for (Configuration config: factory)
{
- config = getConfiguration( ca, config.getPid(), bundleContext.getBundle() );
- long changeCount = changeCounter.getChangeCount( config, false, -1 );
- holder.configurationUpdated(config.getPid(), config.getProperties(), changeCount);
+ config = getConfiguration( ca, config.getPid() );
+ if ( checkBundleLocation( config, bundleContext.getBundle() ))
+ {
+ long changeCount = changeCounter.getChangeCount( config, false, -1 );
+ holder.configurationUpdated(config.getPid(), config.getProperties(), changeCount, new TargetedPID(config.getFactoryPid()));
+ }
}
}
else
@@ -138,9 +141,12 @@
Configuration singleton = findSingletonConfiguration(ca, confPid, bundleContext.getBundle());
if (singleton != null)
{
- singleton = getConfiguration( ca, singleton.getPid(), bundleContext.getBundle() );
+ singleton = getConfiguration( ca, singleton.getPid() );
+ if ( singleton != null && checkBundleLocation( singleton, bundleContext.getBundle() ))
+ {
long changeCount = changeCounter.getChangeCount( singleton, false, -1 );
- holder.configurationUpdated(confPid, singleton.getProperties(), changeCount);
+ holder.configurationUpdated(confPid, singleton.getProperties(), changeCount, new TargetedPID(singleton.getPid()));
+ }
}
}
}
@@ -237,10 +243,14 @@
{
switch (event.getType()) {
case ConfigurationEvent.CM_DELETED:
+ //TODO fall back to less-strong pid match
componentHolder.configurationDeleted(pid.getServicePid());
break;
case ConfigurationEvent.CM_UPDATED:
+ {
+ //TODO what if this is a new config with a stronger (or weaker) binding than
+ // an existing matching config?
final BundleComponentActivator activator = componentHolder.getActivator();
if (activator == null)
{
@@ -253,57 +263,71 @@
break;
}
- final ServiceReference caRef = bundleContext
- .getServiceReference(ComponentRegistry.CONFIGURATION_ADMIN);
- if (caRef != null)
+ final Configuration config = getConfiguration( pid, componentHolder, bundleContext );
+ if ( checkBundleLocation( config, bundleContext.getBundle() ) )
{
- try
+ long changeCount = changeCounter.getChangeCount( config, true, componentHolder.getChangeCount( pid.getServicePid() ) );
+ componentHolder.configurationUpdated( pid.getServicePid(), config.getProperties(), changeCount, pid );
+ }
+
+ break;
+ }
+ case ConfigurationEvent.CM_LOCATION_CHANGED:
+ {
+ //TODO is this logic correct for factory pids????
+ final BundleComponentActivator activator = componentHolder.getActivator();
+ if (activator == null)
+ {
+ break;
+ }
+
+ final BundleContext bundleContext = activator.getBundleContext();
+ if (bundleContext == null)
+ {
+ break;
+ }
+
+ TargetedPID oldTargetedPID = componentHolder.getConfigurationTargetedPID();
+ if ( pid.equals(oldTargetedPID))
+ {
+ //this sets the location to this component's bundle if not already set. OK here
+ //since it used to be set to this bundle, ok to reset it
+ final Configuration config = getConfiguration( pid, componentHolder, bundleContext );
+ //this config was used on this component. Does it still match?
+ if (!checkBundleLocation( config, bundleContext.getBundle() ))
{
- final Object cao = bundleContext.getService(caRef);
- if (cao != null)
- {
- try
- {
- if ( cao instanceof ConfigurationAdmin )
- {
- final ConfigurationAdmin ca = ( ConfigurationAdmin ) cao;
- final Configuration config = getConfiguration( ca, pid.getRawPid(), bundleContext.getBundle() );
- if ( config != null )
- {
- long changeCount = changeCounter.getChangeCount( config, true, componentHolder.getChangeCount( pid.getServicePid() ) );
- componentHolder.configurationUpdated( pid.getServicePid(), config.getProperties(), changeCount );
- }
- }
- else
- {
- Activator.log( LogService.LOG_WARNING, null, "Cannot reconfigure component "
- + componentHolder.getComponentMetadata().getName(), null );
- Activator.log( LogService.LOG_WARNING, null,
- "Component Bundle's Configuration Admin is not compatible with " +
- "ours. This happens if multiple Configuration Admin API versions " +
- "are deployed and different bundles wire to different versions",
- null );
- }
- }
- finally
- {
- bundleContext.ungetService( caRef );
- }
- }
+ //no, delete it
+ componentHolder.configurationDeleted( pid.getServicePid() );
+ //maybe there's another match
+ configureComponentHolder(componentHolder);
+
}
- catch (IllegalStateException ise)
+ //else still matches
+ break;
+ }
+ boolean better = pid.bindsStronger( oldTargetedPID );
+ if ( better )
+ {
+ //this sets the location to this component's bundle if not already set. OK here
+ //because if it is set to this bundle we will use it.
+ final Configuration config = getConfiguration( pid, componentHolder, bundleContext );
+ //this component was not configured with this config. Should it be now?
+ if ( checkBundleLocation( config, bundleContext.getBundle() ) )
{
- // If the bundle has been stopped concurrently
- Activator.log(LogService.LOG_WARNING, null, "Unknown ConfigurationEvent type " + event.getType(),
- ise);
+ if ( oldTargetedPID != null )
+ {
+ //this is a better match, delete old before setting new
+ componentHolder.configurationDeleted( pid.getServicePid() );
+ }
+ long changeCount = changeCounter.getChangeCount( config, true,
+ componentHolder.getChangeCount( pid.getServicePid() ) );
+ componentHolder.configurationUpdated( pid.getServicePid(), config.getProperties(),
+ changeCount, pid );
}
}
+ //else worse match, do nothing
break;
-
- case ConfigurationEvent.CM_LOCATION_CHANGED:
- // FELIX-3584: Implement event support
- break;
-
+ }
default:
Activator.log(LogService.LOG_WARNING, null, "Unknown ConfigurationEvent type " + event.getType(),
null);
@@ -312,19 +336,58 @@
}
}
- private Configuration getConfiguration(final ConfigurationAdmin ca, final String pid, final Bundle bundle)
+ private Configuration getConfiguration(final TargetedPID pid, ComponentHolder componentHolder,
+ final BundleContext bundleContext)
+ {
+ final ServiceReference caRef = bundleContext
+ .getServiceReference(ComponentRegistry.CONFIGURATION_ADMIN);
+ if (caRef != null)
+ {
+ try
+ {
+ final Object cao = bundleContext.getService(caRef);
+ if (cao != null)
+ {
+ try
+ {
+ if ( cao instanceof ConfigurationAdmin )
+ {
+ final ConfigurationAdmin ca = ( ConfigurationAdmin ) cao;
+ final Configuration config = getConfiguration( ca, pid.getRawPid() );
+ return config;
+ }
+ else
+ {
+ Activator.log( LogService.LOG_WARNING, null, "Cannot reconfigure component "
+ + componentHolder.getComponentMetadata().getName(), null );
+ Activator.log( LogService.LOG_WARNING, null,
+ "Component Bundle's Configuration Admin is not compatible with " +
+ "ours. This happens if multiple Configuration Admin API versions " +
+ "are deployed and different bundles wire to different versions",
+ null );
+ }
+ }
+ finally
+ {
+ bundleContext.ungetService( caRef );
+ }
+ }
+ }
+ catch (IllegalStateException ise)
+ {
+ // If the bundle has been stopped concurrently
+ Activator.log(LogService.LOG_WARNING, null, "Bundle in unexpected state",
+ ise);
+ }
+ }
+ return null;
+ }
+
+ private Configuration getConfiguration(final ConfigurationAdmin ca, final String pid)
{
try
{
- final Configuration cfg = ca.getConfiguration(pid);
- if (checkBundleLocation( cfg, bundle ))
- {
- return cfg;
- }
-
- // configuration belongs to another bundle, cannot be used here
- Activator.log(LogService.LOG_INFO, null, "Cannot use configuration pid=" + pid + " for bundle "
- + bundle.getLocation() + " because it belongs to bundle " + cfg.getBundleLocation(), null);
+ return ca.getConfiguration(pid);
}
catch (IOException ioe)
{
@@ -412,6 +475,10 @@
private boolean checkBundleLocation(Configuration config, Bundle bundle)
{
+ if (config == null)
+ {
+ return false;
+ }
String configBundleLocation = config.getBundleLocation();
if ( configBundleLocation == null)
{
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/config/ImmediateComponentHolder.java b/scr/src/main/java/org/apache/felix/scr/impl/config/ImmediateComponentHolder.java
index 7ae7569..7a243fb 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/config/ImmediateComponentHolder.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/config/ImmediateComponentHolder.java
@@ -26,6 +26,7 @@
import org.apache.felix.scr.Component;
import org.apache.felix.scr.impl.BundleComponentActivator;
+import org.apache.felix.scr.impl.TargetedPID;
import org.apache.felix.scr.impl.helper.ComponentMethods;
import org.apache.felix.scr.impl.helper.SimpleLogger;
import org.apache.felix.scr.impl.manager.DelayedComponentManager;
@@ -100,12 +101,14 @@
* Whether components have already been enabled by calling the
* {@link #enableComponents(boolean)} method. If this field is <code>true</code>
* component instances created per configuration by the
- * {@link #configurationUpdated(String, Dictionary, long)} method are also
+ * {@link #configurationUpdated(String, Dictionary, long, TargetedPID)} method are also
* enabled. Otherwise they are not enabled immediately.
*/
private volatile boolean m_enabled;
private final ComponentMethods m_componentMethods;
+ private TargetedPID m_targetedPID;
+
public ImmediateComponentHolder( final BundleComponentActivator activator, final ComponentMetadata metadata )
{
@@ -188,6 +191,7 @@
log( LogService.LOG_DEBUG, "ImmediateComponentHolder configuration deleted for pid {0}",
new Object[] {pid}, null);
+ m_targetedPID = null;
// component to deconfigure or dispose of
final ImmediateComponentManager icm;
boolean deconfigure = false;
@@ -268,11 +272,18 @@
* this case a new component is created, configured and stored in the map</li>
* </ul>
*/
- public void configurationUpdated( final String pid, final Dictionary<String, Object> props, long changeCount )
+ public void configurationUpdated( final String pid, final Dictionary<String, Object> props, long changeCount, TargetedPID targetedPid )
{
log( LogService.LOG_DEBUG, "ImmediateComponentHolder configuration updated for pid {0} with properties {1}",
new Object[] {pid, props}, null);
+ if ( m_targetedPID != null && !m_targetedPID.equals( targetedPid ))
+ {
+ log( LogService.LOG_ERROR, "ImmediateComponentHolder unexpected change in targetedPID from {0} to {1}",
+ new Object[] {m_targetedPID, targetedPid}, null);
+ throw new IllegalStateException("Unexpected targetedPID change");
+ }
+ m_targetedPID = targetedPid;
// component to update or create
final ImmediateComponentManager icm;
final String message;
@@ -560,4 +571,9 @@
}
}
+ public TargetedPID getConfigurationTargetedPID()
+ {
+ return m_targetedPID;
+ }
+
}
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 064e4ee..3066c93 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
@@ -29,6 +29,7 @@
import org.apache.felix.scr.Component;
import org.apache.felix.scr.impl.BundleComponentActivator;
+import org.apache.felix.scr.impl.TargetedPID;
import org.apache.felix.scr.impl.config.ComponentHolder;
import org.apache.felix.scr.impl.helper.ComponentMethods;
import org.apache.felix.scr.impl.metadata.ComponentMetadata;
@@ -89,6 +90,8 @@
* Configuration change count (R5) or imitation (R4)
*/
protected volatile long m_changeCount = -1;
+
+ private TargetedPID m_targetedPID;
public ComponentFactoryImpl( BundleComponentActivator activator, ComponentMetadata metadata )
{
@@ -305,6 +308,7 @@
public void configurationDeleted( String pid )
{
+ m_targetedPID = null;
if ( pid.equals( getComponentMetadata().getConfigurationPid() ) )
{
log( LogService.LOG_DEBUG, "Handling configuration removal", null );
@@ -338,8 +342,15 @@
}
- public void configurationUpdated( String pid, Dictionary<String, Object> configuration, long changeCount )
+ public void configurationUpdated( String pid, Dictionary<String, Object> configuration, long changeCount, TargetedPID targetedPid )
{
+ if ( m_targetedPID != null && !m_targetedPID.equals( targetedPid ))
+ {
+ log( LogService.LOG_ERROR, "ImmediateComponentHolder unexpected change in targetedPID from {0} to {1}",
+ new Object[] {m_targetedPID, targetedPid}, null);
+ throw new IllegalStateException("Unexpected targetedPID change");
+ }
+ m_targetedPID = targetedPid;
if ( configuration != null )
{
if ( changeCount <= m_changeCount )
@@ -526,4 +537,10 @@
}
+ public TargetedPID getConfigurationTargetedPID()
+ {
+ return m_targetedPID;
+ }
+
+
}
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/ConfigurationComponentFactoryImpl.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/ConfigurationComponentFactoryImpl.java
index 05af4d3..6177799 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/ConfigurationComponentFactoryImpl.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/ConfigurationComponentFactoryImpl.java
@@ -27,6 +27,7 @@
import org.apache.felix.scr.Component;
import org.apache.felix.scr.impl.BundleComponentActivator;
+import org.apache.felix.scr.impl.TargetedPID;
import org.apache.felix.scr.impl.config.ComponentHolder;
import org.apache.felix.scr.impl.helper.ComponentMethods;
import org.apache.felix.scr.impl.metadata.ComponentMetadata;
@@ -132,11 +133,11 @@
}
- public void configurationUpdated( String pid, Dictionary<String, Object> configuration, long changeCount )
+ public void configurationUpdated( String pid, Dictionary<String, Object> configuration, long changeCount, TargetedPID targetedPid )
{
if ( pid.equals( getComponentMetadata().getConfigurationPid() ) )
{
- super.configurationUpdated( pid, configuration, changeCount );
+ super.configurationUpdated( pid, configuration, changeCount, targetedPid );
}
else //non-spec backwards compatible
{
diff --git a/scr/src/test/java/org/apache/felix/scr/impl/config/ConfiguredComponentHolderTest.java b/scr/src/test/java/org/apache/felix/scr/impl/config/ConfiguredComponentHolderTest.java
index 028920b..8992782 100644
--- a/scr/src/test/java/org/apache/felix/scr/impl/config/ConfiguredComponentHolderTest.java
+++ b/scr/src/test/java/org/apache/felix/scr/impl/config/ConfiguredComponentHolderTest.java
@@ -27,6 +27,7 @@
import junit.framework.TestCase;
import org.apache.felix.scr.impl.BundleComponentActivator;
+import org.apache.felix.scr.impl.TargetedPID;
import org.apache.felix.scr.impl.helper.ComponentMethods;
import org.apache.felix.scr.impl.manager.ImmediateComponentManager;
import org.apache.felix.scr.impl.metadata.ComponentMetadata;
@@ -71,7 +72,7 @@
// configure with the singleton configuration
final Dictionary config = new Hashtable();
config.put( "value", name );
- holder.configurationUpdated( name, config, 0 );
+ holder.configurationUpdated( name, config, 0, new TargetedPID(name) );
// assert single component and no map
final ImmediateComponentManager cmgrAfterConfig = getSingleManager( holder );
@@ -115,7 +116,7 @@
final String pid1 = "test.factory.0001";
final Dictionary config1 = new Hashtable();
config1.put( "value", pid1 );
- holder.configurationUpdated( pid1, config1, 0 );
+ holder.configurationUpdated( pid1, config1, 0, new TargetedPID(name) );
// assert single component and single-entry map
final ImmediateComponentManager cmgrAfterConfig = getSingleManager( holder );
@@ -128,7 +129,7 @@
final String pid2 = "test.factory.0002";
final Dictionary config2 = new Hashtable();
config1.put( "value", pid2 );
- holder.configurationUpdated( pid2, config2, 1 );
+ holder.configurationUpdated( pid2, config2, 1, new TargetedPID(name) );
// assert single component and single-entry map
final ImmediateComponentManager cmgrAfterConfig2 = getSingleManager( holder );
@@ -148,7 +149,7 @@
assertEquals( "Expect one component manager in list", 1, cmgrsAfterUnConfig2.length );
// add second config again and remove first config -> replace singleton component
- holder.configurationUpdated( pid2, config2, 2 );
+ holder.configurationUpdated( pid2, config2, 2, new TargetedPID(name) );
holder.configurationDeleted( pid1 );
// assert single component and single-entry map