You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@felix.apache.org by dj...@apache.org on 2013/05/07 23:50:14 UTC

svn commit: r1480104 - in /felix/trunk/scr/src: main/java/org/apache/felix/scr/impl/config/ main/java/org/apache/felix/scr/impl/manager/ test/java/org/apache/felix/scr/impl/config/

Author: djencks
Date: Tue May  7 21:50:14 2013
New Revision: 1480104

URL: http://svn.apache.org/r1480104
Log:
FELIX-3584 start work on location changed.  Breaks factoryPIDs

Modified:
    felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ComponentHolder.java
    felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurationSupport.java
    felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ImmediateComponentHolder.java
    felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java
    felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ConfigurationComponentFactoryImpl.java
    felix/trunk/scr/src/test/java/org/apache/felix/scr/impl/config/ConfiguredComponentHolderTest.java

Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ComponentHolder.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ComponentHolder.java?rev=1480104&r1=1480103&r2=1480104&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ComponentHolder.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ComponentHolder.java Tue May  7 21:50:14 2013
@@ -23,6 +23,7 @@ import java.util.Dictionary;
 
 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 @@ public interface ComponentHolder
      * @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.

Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurationSupport.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurationSupport.java?rev=1480104&r1=1480103&r2=1480104&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurationSupport.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurationSupport.java Tue May  7 21:50:14 2013
@@ -127,9 +127,12 @@ public class ConfigurationSupport implem
                             {
                                 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 @@ public class ConfigurationSupport implem
                                 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 @@ public class ConfigurationSupport implem
             {
                 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 @@ public class ConfigurationSupport implem
                         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 @@ public class ConfigurationSupport implem
         }
     }
 
-    private Configuration getConfiguration(final ConfigurationAdmin ca, final String pid, final Bundle bundle)
+    private Configuration getConfiguration(final TargetedPID pid, ComponentHolder componentHolder,
+            final BundleContext bundleContext)
     {
-        try
+        final ServiceReference caRef = bundleContext
+            .getServiceReference(ComponentRegistry.CONFIGURATION_ADMIN);
+        if (caRef != null)
         {
-            final Configuration cfg = ca.getConfiguration(pid);
-            if (checkBundleLocation( cfg, bundle ))
+            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)
             {
-                return cfg;
+                // If the bundle has been stopped concurrently
+                Activator.log(LogService.LOG_WARNING, null, "Bundle in unexpected state",
+                    ise);
             }
+        }
+        return null;
+    }
 
-            // 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);
+    private Configuration getConfiguration(final ConfigurationAdmin ca, final String pid)
+    {
+        try
+        {
+            return ca.getConfiguration(pid);
         }
         catch (IOException ioe)
         {
@@ -412,6 +475,10 @@ public class ConfigurationSupport implem
     
     private boolean checkBundleLocation(Configuration config, Bundle bundle)
     {
+        if (config == null)
+        {
+            return false;
+        }
         String configBundleLocation = config.getBundleLocation();
         if ( configBundleLocation == null)
         {

Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ImmediateComponentHolder.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ImmediateComponentHolder.java?rev=1480104&r1=1480103&r2=1480104&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ImmediateComponentHolder.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ImmediateComponentHolder.java Tue May  7 21:50:14 2013
@@ -26,6 +26,7 @@ import java.util.Map;
 
 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 @@ public class ImmediateComponentHolder im
      * 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 @@ public class ImmediateComponentHolder im
         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 @@ public class ImmediateComponentHolder im
      * 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 class ImmediateComponentHolder im
         }
     }
 
+    public TargetedPID getConfigurationTargetedPID()
+    {
+        return m_targetedPID;
+    }
+
 }

Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java?rev=1480104&r1=1480103&r2=1480104&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java Tue May  7 21:50:14 2013
@@ -29,6 +29,7 @@ import java.util.Map;
 
 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 @@ public class ComponentFactoryImpl<S> ext
      * 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 class ComponentFactoryImpl<S> ext
 
     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 class ComponentFactoryImpl<S> ext
     }
 
 
-    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 class ComponentFactoryImpl<S> ext
 
     }
 
+    public TargetedPID getConfigurationTargetedPID()
+    {
+        return m_targetedPID;
+    }
+
+
 }

Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ConfigurationComponentFactoryImpl.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ConfigurationComponentFactoryImpl.java?rev=1480104&r1=1480103&r2=1480104&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ConfigurationComponentFactoryImpl.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ConfigurationComponentFactoryImpl.java Tue May  7 21:50:14 2013
@@ -27,6 +27,7 @@ import java.util.Map;
 
 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 class ConfigurationComponentFacto
     }
 
 
-    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
         {

Modified: felix/trunk/scr/src/test/java/org/apache/felix/scr/impl/config/ConfiguredComponentHolderTest.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/test/java/org/apache/felix/scr/impl/config/ConfiguredComponentHolderTest.java?rev=1480104&r1=1480103&r2=1480104&view=diff
==============================================================================
--- felix/trunk/scr/src/test/java/org/apache/felix/scr/impl/config/ConfiguredComponentHolderTest.java (original)
+++ felix/trunk/scr/src/test/java/org/apache/felix/scr/impl/config/ConfiguredComponentHolderTest.java Tue May  7 21:50:14 2013
@@ -27,6 +27,7 @@ import java.util.Hashtable;
 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 @@ public class ConfiguredComponentHolderTe
         // 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 @@ public class ConfiguredComponentHolderTe
         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 @@ public class ConfiguredComponentHolderTe
         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 @@ public class ConfiguredComponentHolderTe
         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