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

svn commit: r1357578 - in /felix/trunk/configadmin/src: main/java/org/apache/felix/cm/impl/ main/java/org/apache/felix/cm/impl/helper/ test/java/org/apache/felix/cm/impl/helper/

Author: fmeschbe
Date: Thu Jul  5 12:28:06 2012
New Revision: 1357578

URL: http://svn.apache.org/viewvc?rev=1357578&view=rev
Log:
FELIX-3481 - Implement targeted PID support for MSF
FELIX-3577 - Move the configuration selection for the
	MSFUpdate tasks from the constructor to the run
	method. This now serializes configuration selections
	for known services and service selections for known
	configurations in the Configuration Update thread
FELIX-3554 - Check configuration revision counter before
	providing configuration. Also do not remove the same
    configuration multiple times

Added:
    felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceConfigurationMap.java
    felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceFactoryConfigurationMap.java
Modified:
    felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationImpl.java
    felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java
    felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/BaseTracker.java
    felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ConfigurationMap.java
    felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceFactoryTracker.java
    felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceTracker.java
    felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/TargetedPID.java
    felix/trunk/configadmin/src/test/java/org/apache/felix/cm/impl/helper/ConfigurationMapTest.java

Modified: felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationImpl.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationImpl.java?rev=1357578&r1=1357577&r2=1357578&view=diff
==============================================================================
--- felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationImpl.java (original)
+++ felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationImpl.java Thu Jul  5 12:28:06 2012
@@ -386,12 +386,15 @@ public class ConfigurationImpl extends C
             // persist new configuration
             localPersistenceManager.store( getPidString(), newProperties );
 
-            // if this is a factory configuration, update the factory with
-            updateFactory();
-
             // finally assign the configuration for use
             configure( newProperties );
 
+            // if this is a factory configuration, update the factory with
+            // do this only after configuring with current properties such
+            // that a concurrently registered ManagedServiceFactory service
+            // does not receive a new/unusable configuration
+            updateFactory();
+
             // update the service and fire an CM_UPDATED event
             getConfigurationManager().updated( this, true );
         }
@@ -530,7 +533,7 @@ public class ConfigurationImpl extends C
      * counter, the two calls should be synchronized on this instance to
      * ensure configuration values and revision counter match.
      */
-    long getRevision()
+    public long getRevision()
     {
         return revision;
     }
@@ -621,6 +624,14 @@ public class ConfigurationImpl extends C
     }
 
 
+    static void setAutoProperties( Dictionary properties, String pid, String factoryPid )
+    {
+        replaceProperty( properties, Constants.SERVICE_PID, pid );
+        replaceProperty( properties, ConfigurationAdmin.SERVICE_FACTORYPID, factoryPid );
+        properties.remove( ConfigurationAdmin.SERVICE_BUNDLELOCATION );
+    }
+
+
     static void clearAutoProperties( Dictionary properties )
     {
         properties.remove( Constants.SERVICE_PID );

Modified: felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java?rev=1357578&r1=1357577&r2=1357578&view=diff
==============================================================================
--- felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java (original)
+++ felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java Thu Jul  5 12:28:06 2012
@@ -31,7 +31,6 @@ import java.util.Hashtable;
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
-import java.util.Map;
 import java.util.Random;
 
 import org.apache.felix.cm.PersistenceManager;
@@ -477,6 +476,10 @@ public class ConfigurationManager implem
     /**
      * Returns a targeted configuration for the given service PID and
      * the reference target service.
+     * <p>
+     * A configuration returned has already been checked for visibility
+     * by the bundle registering the referenced service. Additionally,
+     * the configuration is also dynamically bound if needed.
      *
      * @param rawPid The raw service PID to get targeted configuration for.
      * @param target The target <code>ServiceReference</code> to get
@@ -491,7 +494,8 @@ public class ConfigurationManager implem
         final Bundle serviceBundle = target.getBundle();
         if ( serviceBundle != null )
         {
-            // for pre-1.5 API compatibility
+            // list of targeted PIDs to check
+            // (StringBuffer for pre-1.5 API compatibility)
             final StringBuffer targetedPid = new StringBuffer( rawPid );
             int i = 3;
             String[] names = new String[4];
@@ -508,10 +512,30 @@ public class ConfigurationManager implem
                 ConfigurationImpl config = getConfiguration( candidate );
                 if ( config != null )
                 {
-                    return config;
+                    // check visibility to use and dynamically bind
+                    if ( canReceive( serviceBundle, config.getBundleLocation() ) )
+                    {
+                        config.tryBindLocation( serviceBundle.getLocation() );
+                        return config;
+                    }
+
+                    // CM 1.4 / 104.13.2.2 / 104.5.3
+                    // act as if there is no configuration
+                    log(
+                        LogService.LOG_DEBUG,
+                        "Cannot use configuration {0} for {1}: No visibility to configuration bound to {2}; calling with null",
+                        new Object[]
+                            { config.getPid(), toString( target ), config.getBundleLocation() } );
                 }
             }
         }
+        else
+        {
+            log( LogService.LOG_INFO,
+                "Service for PID {0} seems to already have been unregistered, not updating with configuration",
+                new Object[]
+                    { rawPid } );
+        }
 
         // service already unregistered, nothing to do really
         return null;
@@ -844,16 +868,22 @@ public class ConfigurationManager implem
      * Schedules the configuration of the referenced service with
      * configuration for the given PID.
      *
-     * @param pid The service PID of the configuration to be provided
-     *      to the referenced service.
+     * @param pid The list of service PID of the configurations to be
+     *      provided to the referenced service.
      * @param sr The <code>ServiceReference</code> to the service
      *      to be configured.
      * @param factory <code>true</code> If the service is considered to
      *      be a <code>ManagedServiceFactory</code>. Otherwise the service
      *      is considered to be a <code>ManagedService</code>.
      */
-    public void configure( String pid, ServiceReference sr, final boolean factory )
+    public void configure( String[] pid, ServiceReference sr, final boolean factory )
     {
+        if ( this.isLogEnabled( LogService.LOG_DEBUG ) )
+        {
+            this.log( LogService.LOG_DEBUG, "configure(ManagedService {0})", new Object[]
+                { toString( sr ) } );
+        }
+
         Runnable r;
         if ( factory )
         {
@@ -1022,31 +1052,32 @@ public class ConfigurationManager implem
 
     /**
      * Calls the registered configuration plugins on the given configuration
-     * properties from the given configuration object unless the configuration
-     * has just been created and not been updated yet.
+     * properties from the given configuration object.
+     * <p>
+     * The plugins to be called are selected as <code>ConfigurationPlugin</code>
+     * services registered with a <code>cm.target</code> property set to
+     * <code>*</code> or the factory PID of the configuration (for factory
+     * configurations) or the PID of the configuration (for non-factory
+     * configurations).
      *
      * @param props The configuraiton properties run through the registered
-     *          ConfigurationPlugin services. This may be <code>null</code>
-     *          in which case this method just immediately returns.
-     * @param targetPid The identification of the configuration update used to
-     *          select the plugins according to their cm.target service
-     *          property
+     *          ConfigurationPlugin services. This must not be
+     *          <code>null</code>.
      * @param sr The service reference of the managed service (factory) which
      *          is to be updated with configuration
-     * @param cfg The configuration object whose properties have to be passed
-     *          through the plugins
+     * @param configPid The PID of the configuration object whose properties
+     *          are to be augmented
+     * @param factoryPid the factory PID of the configuration object whose
+     *          properties are to be augmented. This is non-<code>null</code>
+     *          only for a factory configuration.
      */
-    public void callPlugins( final Dictionary props, final String targetPid, final ServiceReference sr,
-        final ConfigurationImpl cfg )
+    public void callPlugins( final Dictionary props, final ServiceReference sr, final String configPid,
+        final String factoryPid )
     {
-        // guard against NPE for new configuration never updated
-        if (props == null) {
-            return;
-        }
-
         ServiceReference[] plugins = null;
         try
         {
+            final String targetPid = (factoryPid == null) ? configPid : factoryPid;
             String filter = "(|(!(cm.target=*))(cm.target=" + targetPid + "))";
             plugins = bundleContext.getServiceReferences( ConfigurationPlugin.class.getName(), filter );
         }
@@ -1088,7 +1119,7 @@ public class ConfigurationManager implem
                     // ensure ungetting the plugin
                     bundleContext.ungetService( pluginRef );
                 }
-                cfg.setAutoProperties( props, false );
+                ConfigurationImpl.setAutoProperties( props, configPid, factoryPid );
             }
         }
     }
@@ -1308,103 +1339,74 @@ public class ConfigurationManager implem
      */
     private class ManagedServiceUpdate implements Runnable
     {
-        private final String pid;
+        private final String[] pids;
 
         private final ServiceReference sr;
 
-        private final ConfigurationImpl config;
-
-        private final Dictionary rawProperties;
-
-        private final long revision;
-
-        ManagedServiceUpdate( String pid, ServiceReference sr )
+        ManagedServiceUpdate( String[] pids, ServiceReference sr )
         {
-            this.pid = pid;
+            this.pids = pids;
             this.sr = sr;
-
-            // get or load configuration for the pid
-            ConfigurationImpl config = null;
-            Dictionary rawProperties = null;
-            long revision = -1;
-            try
-            {
-                config = getTargetedConfiguration( pid, sr );
-                if ( config != null )
-                {
-                    synchronized ( config )
-                    {
-                        rawProperties = config.getProperties( true );
-                        revision = config.getRevision();
-                    }
-                }
-            }
-            catch ( IOException ioe )
-            {
-                log( LogService.LOG_ERROR, "Error loading configuration for {0}", new Object[]
-                    { pid, ioe } );
-            }
-
-            this.config = config;
-            this.rawProperties = rawProperties;
-            this.revision = revision;
         }
 
 
         public void run()
         {
-            Dictionary properties = rawProperties;
-
-            // check configuration and call plugins if existing
-            if ( config != null )
+            for ( String pid : this.pids )
             {
-                log( LogService.LOG_DEBUG, "Updating service {0} to with configuration {1}@{2}", new Object[]
-                    { pid, this.config.getPid(), new Long( revision ) } );
-
-                Bundle serviceBundle = sr.getBundle();
-                if ( serviceBundle == null )
+                try
                 {
-                    log( LogService.LOG_INFO,
-                        "Service for PID {0} seems to already have been unregistered, not updating with configuration",
-                        new Object[]
-                            { pid } );
-                    return;
+                    final ConfigurationImpl config = getTargetedConfiguration( pid, this.sr );
+                    provide( pid, config );
                 }
-
-                if ( canReceive( serviceBundle, config.getBundleLocation() ) )
+                catch ( IOException ioe )
                 {
-                    // 104.4.2 Dynamic Binding
-                    config.tryBindLocation( serviceBundle.getLocation() );
+                    log( LogService.LOG_ERROR, "Error loading configuration for {0}", new Object[]
+                        { pid, ioe } );
                 }
-                else
+                catch ( Exception e )
                 {
-                    // CM 1.4 / 104.13.2.2 / 104.5.3
-                    // act as if there is no configuration
-                    log(
-                        LogService.LOG_DEBUG,
-                        "Cannot use configuration {0} for {1}: No visibility to configuration bound to {2}; calling with null",
+                    log( LogService.LOG_ERROR, "Unexpected problem providing configuration {0} to service {1}",
                         new Object[]
-                            { pid, ConfigurationManager.toString( sr ), config.getBundleLocation() } );
-
-                    // CM 1.4 / 104.5.3 ManagedService.updated must be
-                    // called with null if configuration is no visible
-                    properties = null;
+                            { pid, ConfigurationManager.toString( this.sr ), e } );
                 }
+            }
+        }
 
+
+        private void provide(final String servicePid, final ConfigurationImpl config)
+        {
+            // check configuration
+            final TargetedPID configPid;
+            final Dictionary properties;
+            final long revision;
+            if ( config != null )
+            {
+                synchronized ( config )
+                {
+                    configPid = config.getPid();
+                    properties = config.getProperties( true );
+                    revision = config.getRevision();
+                }
             }
             else
             {
                 // 104.5.3 ManagedService.updated must be called with null
                 // if no configuration is available
+                configPid = new TargetedPID( servicePid );
                 properties = null;
+                revision = -1;
             }
 
-            managedServiceTracker.provideConfiguration( sr, config, properties );
+            log( LogService.LOG_DEBUG, "Updating service {0} with configuration {1}@{2}", new Object[]
+                { servicePid, configPid, new Long( revision ) } );
+
+            managedServiceTracker.provideConfiguration( sr, configPid, null, properties, revision);
         }
 
         public String toString()
         {
-            return "ManagedService Update: pid=" + pid;
+            return "ManagedService Update: pid=" + Arrays.asList( pids );
         }
     }
 
@@ -1417,106 +1419,113 @@ public class ConfigurationManager implem
      */
     private class ManagedServiceFactoryUpdate implements Runnable
     {
-        private final String factoryPid;
+        private final String[] factoryPids;
 
         private final ServiceReference sr;
 
-        private final Map configs;
-
-        private final Map revisions;
 
-        ManagedServiceFactoryUpdate( String factoryPid, ServiceReference sr )
+        ManagedServiceFactoryUpdate( String[] factoryPids, ServiceReference sr )
         {
-            this.factoryPid = factoryPid;
+            this.factoryPids = factoryPids;
             this.sr = sr;
+        }
 
-            List<Factory> factories = null;
-            Map configs = null;
-            Map revisions = null;
-            try
+
+        public void run()
+        {
+            for ( String factoryPid : this.factoryPids )
             {
-                factories = getTargetedFactories( factoryPid, sr );
-                for (Factory factory : factories) {
-                    configs = new HashMap();
-                    revisions = new HashMap();
-                    for ( Iterator pi = factory.getPIDs().iterator(); pi.hasNext(); )
-                    {
-                        final String pid = ( String ) pi.next();
-                        ConfigurationImpl cfg;
-                        try
-                        {
-                            cfg = getConfiguration( pid );
-                        }
-                        catch ( IOException ioe )
-                        {
-                            log( LogService.LOG_ERROR, "Error loading configuration for {0}", new Object[]
-                                { pid, ioe } );
-                            continue;
-                        }
 
-                        // sanity check on the configuration
-                        if ( cfg == null )
-                        {
-                            log( LogService.LOG_ERROR, "Configuration {0} referred to by factory {1} does not exist",
-                                new Object[]
-                                    { pid, factoryPid } );
-                            factory.removePID( pid );
-                            factory.storeSilently();
-                            continue;
-                        }
-                        else if ( cfg.isNew() )
-                        {
-                            // Configuration has just been created but not yet updated
-                            // we currently just ignore it and have the update mechanism
-                            // provide the configuration to the ManagedServiceFactory
-                            // As of FELIX-612 (not storing new factory configurations)
-                            // this should not happen. We keep this for added stability
-                            // but raise the logging level to error.
-                            log( LogService.LOG_ERROR, "Ignoring new configuration pid={0}", new Object[]
-                                { pid } );
-                            continue;
-                        }
-                        /*
-                         * this code would catch targeted factory PIDs;
-                         * since this is not expected any way, we can
-                         * leave this out
-                         */
-                        /*
-                        else if ( !factoryPid.equals( cfg.getFactoryPid() ) )
+                List<Factory> factories = null;
+                try
+                {
+                    factories = getTargetedFactories( factoryPid, sr );
+                    for ( Factory factory : factories )
+                    {
+                        for ( Iterator pi = factory.getPIDs().iterator(); pi.hasNext(); )
                         {
-                            log( LogService.LOG_ERROR,
-                                "Configuration {0} referred to by factory {1} seems to belong to factory {2}",
-                                new Object[]
-                                    { pid, factoryPid, cfg.getFactoryPid() } );
-                            factory.removePID( pid );
-                            factory.storeSilently();
-                            continue;
-                        }
-                        */
+                            final String pid = ( String ) pi.next();
+                            ConfigurationImpl cfg;
+                            try
+                            {
+                                cfg = getConfiguration( pid );
+                            }
+                            catch ( IOException ioe )
+                            {
+                                log( LogService.LOG_ERROR, "Error loading configuration for {0}", new Object[]
+                                    { pid, ioe } );
+                                continue;
+                            }
+
+                            // sanity check on the configuration
+                            if ( cfg == null )
+                            {
+                                log( LogService.LOG_ERROR,
+                                    "Configuration {0} referred to by factory {1} does not exist", new Object[]
+                                        { pid, factoryPid } );
+                                factory.removePID( pid );
+                                factory.storeSilently();
+                                continue;
+                            }
+                            else if ( cfg.isNew() )
+                            {
+                                // Configuration has just been created but not yet updated
+                                // we currently just ignore it and have the update mechanism
+                                // provide the configuration to the ManagedServiceFactory
+                                // As of FELIX-612 (not storing new factory configurations)
+                                // this should not happen. We keep this for added stability
+                                // but raise the logging level to error.
+                                log( LogService.LOG_ERROR, "Ignoring new configuration pid={0}", new Object[]
+                                    { pid } );
+                                continue;
+                            }
+
+                            /*
+                             * this code would catch targeted factory PIDs;
+                             * since this is not expected any way, we can
+                             * leave this out
+                             */
+                            /*
+                            else if ( !factoryPid.equals( cfg.getFactoryPid() ) )
+                            {
+                                log( LogService.LOG_ERROR,
+                                    "Configuration {0} referred to by factory {1} seems to belong to factory {2}",
+                                    new Object[]
+                                        { pid, factoryPid, cfg.getFactoryPid() } );
+                                factory.removePID( pid );
+                                factory.storeSilently();
+                                continue;
+                            }
+                            */
 
-                        // get the configuration properties for later
-                        synchronized ( cfg )
-                        {
-                            configs.put( cfg, cfg.getProperties( true ) );
-                            revisions.put( cfg, new Long( cfg.getRevision() ) );
+                            provide( factoryPid, cfg );
                         }
                     }
                 }
+                catch ( IOException ioe )
+                {
+                    log( LogService.LOG_ERROR, "Cannot get factory mapping for factory PID {0}", new Object[]
+                        { factoryPid, ioe } );
+                }
             }
-            catch ( IOException ioe )
+        }
+
+
+        private void provide(final String factoryPid, final ConfigurationImpl config) {
+
+            final Dictionary rawProperties;
+            final long revision;
+            synchronized ( config )
             {
-                log( LogService.LOG_ERROR, "Cannot get factory mapping for factory PID {0}", new Object[]
-                    { factoryPid, ioe } );
+                rawProperties = config.getProperties( true );
+                revision = config.getRevision();
             }
 
-            this.configs = configs;
-            this.revisions = revisions;
-        }
-
+            log( LogService.LOG_DEBUG, "Updating service {0} with configuration {1}/{2}@{3}", new Object[]
+                { factoryPid, config.getFactoryPid(), config.getPid(), new Long( revision ) } );
 
-        public void run()
-        {
-            Bundle serviceBundle = sr.getBundle();
+            // CM 1.4 / 104.13.2.1
+            final Bundle serviceBundle = this.sr.getBundle();
             if ( serviceBundle == null )
             {
                 log(
@@ -1527,52 +1536,34 @@ public class ConfigurationManager implem
                 return;
             }
 
-            if ( configs == null || configs.isEmpty() )
+            if ( !canReceive( serviceBundle, config.getBundleLocation() ) )
             {
-                log( LogService.LOG_DEBUG, "No configuration with factory PID {0}; not updating ManagedServiceFactory",
+                log( LogService.LOG_ERROR,
+                    "Cannot use configuration {0} for {1}: No visibility to configuration bound to {2}",
                     new Object[]
-                        { factoryPid } );
-            }
-            else
-            {
-                for ( Iterator ci = configs.entrySet().iterator(); ci.hasNext(); )
-                {
-                    final Map.Entry entry = ( Map.Entry ) ci.next();
-                    final ConfigurationImpl cfg = ( ConfigurationImpl ) entry.getKey();
-                    final Dictionary properties = ( Dictionary ) entry.getValue();
-                    final long revision = ( ( Long ) revisions.get( cfg ) ).longValue();
+                        { config.getPid(), ConfigurationManager.toString( sr ), config.getBundleLocation() } );
 
-                    log( LogService.LOG_DEBUG, "Updating service {0} with configuration {1}/{2}@{3}", new Object[]
-                        { this.factoryPid, cfg.getFactoryPid(), cfg.getPid(), new Long( revision ) } );
-
-                    // CM 1.4 / 104.13.2.1
-                    if ( !canReceive( serviceBundle, cfg.getBundleLocation() ) )
-                    {
-                        log( LogService.LOG_ERROR,
-                            "Cannot use configuration {0} for {1}: No visibility to configuration bound to {2}",
-                            new Object[]
-                                { cfg.getPid(), ConfigurationManager.toString( sr ), cfg.getBundleLocation() } );
-                        continue;
-                    }
+                // no service, really, bail out
+                return;
+            }
 
-                    // 104.4.2 Dynamic Binding
-                    cfg.tryBindLocation( serviceBundle.getLocation() );
+            // 104.4.2 Dynamic Binding
+            config.tryBindLocation( serviceBundle.getLocation() );
 
-                    // update the service with the configuration (if non-null)
-                    if ( properties != null )
-                    {
-                        log( LogService.LOG_DEBUG, "{0}: Updating configuration pid={1}", new Object[]
-                            { ConfigurationManager.toString( sr ), cfg.getPid() } );
-                        managedServiceFactoryTracker.provideConfiguration( sr, cfg, properties );
-                    }
-                }
+            // update the service with the configuration (if non-null)
+            if ( rawProperties != null )
+            {
+                log( LogService.LOG_DEBUG, "{0}: Updating configuration pid={1}", new Object[]
+                    { ConfigurationManager.toString( sr ), config.getPid() } );
+                managedServiceFactoryTracker.provideConfiguration( sr, config.getPid(), config.getFactoryPid(),
+                    rawProperties, revision );
             }
         }
 
 
         public String toString()
         {
-            return "ManagedServiceFactory Update: factoryPid=" + factoryPid;
+            return "ManagedServiceFactory Update: factoryPid=" + Arrays.asList( this.factoryPids );
         }
     }
 
@@ -1607,6 +1598,48 @@ public class ConfigurationManager implem
             }
             return this.config.getPid();
         }
+
+
+        protected boolean provideReplacement( ServiceReference<T> sr )
+        {
+            if ( this.config.getFactoryPid() == null )
+            {
+                try
+                {
+                    final ConfigurationImpl rc = getTargetedConfiguration( this.config.getPid().getServicePid(), sr );
+                    if ( rc != null )
+                    {
+                        final TargetedPID configPid;
+                        final Dictionary properties;
+                        final long revision;
+                        synchronized ( config )
+                        {
+                            configPid = config.getPid();
+                            properties = config.getProperties( true );
+                            revision = config.getRevision();
+                        }
+
+                        helper.provideConfiguration( sr, configPid, null, properties, revision );
+
+                        return true;
+                    }
+                }
+                catch ( IOException ioe )
+                {
+                    log( LogService.LOG_ERROR, "Error loading configuration for {0}", new Object[]
+                        { this.config.getPid(), ioe } );
+                }
+                catch ( Exception e )
+                {
+                    log( LogService.LOG_ERROR, "Unexpected problem providing configuration {0} to service {1}",
+                        new Object[]
+                            { this.config.getPid(), ConfigurationManager.toString( sr ), e } );
+                }
+            }
+
+            // factory or no replacement available
+            return false;
+        }
     }
 
     /**
@@ -1651,7 +1684,8 @@ public class ConfigurationManager implem
                     }
                     else if ( canReceive( refBundle, configBundleLocation ) )
                     {
-                        helper.provideConfiguration( ref, this.config, this.properties );
+                        helper.provideConfiguration( ref, this.config.getPid(), this.config.getFactoryPid(),
+                            this.properties, this.revision );
                     }
                     else
                     {
@@ -1720,7 +1754,12 @@ public class ConfigurationManager implem
                     }
                     else if ( canReceive( srBundle, configLocation ) )
                     {
-                        this.helper.removeConfiguration( sr, this.config );
+                        // revoke configuration unless a replacement
+                        // configuration can be provided
+                        if ( !this.provideReplacement( sr ) )
+                        {
+                            this.helper.removeConfiguration( sr, this.config.getPid(), this.config.getFactoryPid() );
+                        }
                     }
                     else
                     {
@@ -1798,16 +1837,21 @@ public class ConfigurationManager implem
 
                     if ( wasVisible && !isVisible )
                     {
-                        // call deleted method
-                        helper.removeConfiguration( sr, this.config );
-                        log( LogService.LOG_DEBUG, "Configuration {0} revoked from {1} (no more visibility)",
-                            new Object[]
-                                { config.getPid(), ConfigurationManager.toString( sr ) } );
+                        // revoke configuration unless a replacement
+                        // configuration can be provided
+                        if ( !this.provideReplacement( sr ) )
+                        {
+                            helper.removeConfiguration( sr, this.config.getPid(), this.config.getFactoryPid() );
+                            log( LogService.LOG_DEBUG, "Configuration {0} revoked from {1} (no more visibility)",
+                                new Object[]
+                                    { config.getPid(), ConfigurationManager.toString( sr ) } );
+                        }
                     }
                     else if ( !wasVisible && isVisible )
                     {
                         // call updated method
-                        helper.provideConfiguration( sr, this.config, this.properties );
+                        helper.provideConfiguration( sr, this.config.getPid(), this.config.getFactoryPid(),
+                            this.properties, this.revision );
                         log( LogService.LOG_DEBUG, "Configuration {0} provided to {1} (new visibility)", new Object[]
                             { config.getPid(), ConfigurationManager.toString( sr ) } );
                     }
@@ -1958,3 +2002,4 @@ public class ConfigurationManager implem
         }
     }
 }
+

Modified: felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/BaseTracker.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/BaseTracker.java?rev=1357578&r1=1357577&r2=1357578&view=diff
==============================================================================
--- felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/BaseTracker.java (original)
+++ felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/BaseTracker.java Thu Jul  5 12:28:06 2012
@@ -26,7 +26,6 @@ import java.util.Dictionary;
 import java.util.List;
 
 import org.apache.felix.cm.impl.CaseInsensitiveDictionary;
-import org.apache.felix.cm.impl.ConfigurationImpl;
 import org.apache.felix.cm.impl.ConfigurationManager;
 import org.apache.felix.cm.impl.RankingComparator;
 import org.osgi.framework.Constants;
@@ -45,7 +44,7 @@ import org.osgi.util.tracker.ServiceTrac
  * {@link ConfigurationMap} mapping their service PIDs to provided
  * configuration.
  */
-public abstract class BaseTracker<S> extends ServiceTracker<S, ConfigurationMap>
+public abstract class BaseTracker<S> extends ServiceTracker<S, ConfigurationMap<?>>
 {
     protected final ConfigurationManager cm;
 
@@ -62,17 +61,23 @@ public abstract class BaseTracker<S> ext
     }
 
 
-    public ConfigurationMap addingService( ServiceReference<S> reference )
+    public ConfigurationMap<?> addingService( ServiceReference<S> reference )
     {
+        this.cm.log( LogService.LOG_DEBUG, "Registering service {0}", new String[]
+            { ConfigurationManager.toString( reference ) } );
+
         final String[] pids = getServicePid( reference );
         configure( reference, pids );
-        return new ConfigurationMap( pids );
+        return createConfigurationMap( pids );
     }
 
 
     @Override
-    public void modifiedService( ServiceReference<S> reference, ConfigurationMap service )
+    public void modifiedService( ServiceReference<S> reference, ConfigurationMap<?> service )
     {
+        this.cm.log( LogService.LOG_DEBUG, "Modified service {0}", new String[]
+            { ConfigurationManager.toString( reference ) } );
+
         String[] pids = getServicePid( reference );
         if ( service.isDifferentPids( pids ) )
         {
@@ -82,41 +87,20 @@ public abstract class BaseTracker<S> ext
     }
 
 
+    @Override
+    public void removedService( ServiceReference<S> reference, ConfigurationMap<?> service )
+    {
+        // just log
+        this.cm.log( LogService.LOG_DEBUG, "Unregistering service {0}", new String[]
+            { ConfigurationManager.toString( reference ) } );
+    }
+
+
     private void configure( ServiceReference<S> reference, String[] pids )
     {
         if ( pids != null )
         {
-
-            /*
-             * We get the service here for performance reasons only.
-             * Assuming a service is registered as a ServiceFactory with
-             * multiple PIDs, it may be instantiated and destroyed multiple
-             * times during its inital setup phase. By getting it first
-             * and ungetting it at the end we prevent this cycling ensuring
-             * all updates go the same service instance.
-             */
-
-            S service = getRealService( reference );
-            if ( service != null )
-            {
-                try
-                {
-                    if ( this.cm.isLogEnabled( LogService.LOG_DEBUG ) )
-                    {
-                        this.cm.log( LogService.LOG_DEBUG, "configure(ManagedService {0})", new Object[]
-                            { ConfigurationManager.toString( reference ) } );
-                    }
-
-                    for ( int i = 0; i < pids.length; i++ )
-                    {
-                        this.cm.configure( pids[i], reference, managedServiceFactory );
-                    }
-                }
-                finally
-                {
-                    ungetRealService( reference );
-                }
-            }
+            this.cm.configure( pids, reference, managedServiceFactory );
         }
     }
 
@@ -150,12 +134,15 @@ public abstract class BaseTracker<S> ext
     }
 
 
+    protected abstract ConfigurationMap<?> createConfigurationMap( String[] pids );
+
+
     // Updates
-    public abstract void provideConfiguration( ServiceReference<S> service, ConfigurationImpl config,
-        Dictionary<String, ?> properties );
+    public abstract void provideConfiguration( ServiceReference<S> service, TargetedPID configPid,
+        TargetedPID factoryPid, Dictionary<String, ?> properties, long revision );
 
 
-    public abstract void removeConfiguration( ServiceReference<S> service, ConfigurationImpl config );
+    public abstract void removeConfiguration( ServiceReference<S> service, TargetedPID configPid, TargetedPID factoryPid );
 
 
     protected final S getRealService( ServiceReference<S> reference )
@@ -170,17 +157,16 @@ public abstract class BaseTracker<S> ext
     }
 
 
-    protected final Dictionary getProperties( Dictionary<String, ?> rawProperties, String targetPid,
-        ServiceReference service )
+    protected final Dictionary getProperties( Dictionary<String, ?> rawProperties, ServiceReference service,
+        String configPid, String factoryPid )
     {
         Dictionary props = new CaseInsensitiveDictionary( rawProperties );
-        this.cm.callPlugins( props, targetPid, service, null /* config */);
+        this.cm.callPlugins( props, service, configPid, factoryPid );
         return props;
     }
 
 
-    protected final void handleCallBackError( final Throwable error, final ServiceReference target,
-        final ConfigurationImpl config )
+    protected final void handleCallBackError( final Throwable error, final ServiceReference target, final TargetedPID pid )
     {
         if ( error instanceof ConfigurationException )
         {
@@ -189,21 +175,20 @@ public abstract class BaseTracker<S> ext
             {
                 this.cm.log( LogService.LOG_ERROR,
                     "{0}: Updating property {1} of configuration {2} caused a problem: {3}", new Object[]
-                        { ConfigurationManager.toString( target ), ce.getProperty(), config.getPid(), ce.getReason(),
-                            ce } );
+                        { ConfigurationManager.toString( target ), ce.getProperty(), pid, ce.getReason(), ce } );
             }
             else
             {
                 this.cm.log( LogService.LOG_ERROR, "{0}: Updating configuration {1} caused a problem: {2}",
                     new Object[]
-                        { ConfigurationManager.toString( target ), config.getPid(), ce.getReason(), ce } );
+                        { ConfigurationManager.toString( target ), pid, ce.getReason(), ce } );
             }
         }
         else
         {
             {
                 this.cm.log( LogService.LOG_ERROR, "{0}: Unexpected problem updating configuration {1}", new Object[]
-                    { ConfigurationManager.toString( target ), config, error } );
+                    { ConfigurationManager.toString( target ), pid, error } );
             }
 
         }

Modified: felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ConfigurationMap.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ConfigurationMap.java?rev=1357578&r1=1357577&r2=1357578&view=diff
==============================================================================
--- felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ConfigurationMap.java (original)
+++ felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ConfigurationMap.java Thu Jul  5 12:28:06 2012
@@ -18,43 +18,102 @@
  */
 package org.apache.felix.cm.impl.helper;
 
+
 import java.util.Arrays;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 
-public class ConfigurationMap
+
+abstract class ConfigurationMap<T>
 {
-    private Map<String, Object> configurations;
+    private Map<String, T> configurations;
 
 
-    public ConfigurationMap( final String[] configuredPids )
+    protected ConfigurationMap( final String[] configuredPids )
     {
         this.configurations = Collections.emptyMap();
         setConfiguredPids( configuredPids );
     }
 
-    public boolean accepts(final String servicePid) {
+
+    protected abstract Map<String, T> createMap( int size );
+
+
+    protected abstract boolean shallTake( TargetedPID configPid, TargetedPID factoryPid, long revision );
+
+
+    protected abstract void record( TargetedPID configPid, TargetedPID factoryPid, long revision );
+
+
+    protected abstract boolean removeConfiguration( TargetedPID configPid, TargetedPID factoryPid );
+
+
+    protected T get( final TargetedPID key )
+    {
+        return this.configurations.get( key.getServicePid() );
+    }
+
+
+    protected void put( final TargetedPID key, final T value )
+    {
+        final String servicePid = key.getServicePid();
+        if ( this.accepts( servicePid ) )
+        {
+            this.configurations.put( servicePid, value );
+        }
+    }
+
+
+    /**
+     * Returns <code>true</code> if this map is foreseen to take a
+     * configuration with the given service PID.
+     *
+     * @param servicePid The service PID of the configuration which is
+     *      the part of the targeted PID without the bundle's symbolic
+     *      name, version, and location; i.e. {@link TargetedPID#getServicePid()}
+     *
+     * @return <code>true</code> if this map is configured to take
+     *      configurations for the service PID.
+     */
+    public boolean accepts( final String servicePid )
+    {
         return configurations.containsKey( servicePid );
     }
 
+
     public void setConfiguredPids( String[] configuredPids )
     {
-        HashMap<String, Object> newConfigs = new HashMap<String, Object>();
+        final Map<String, T> newConfigs;
         if ( configuredPids != null )
         {
+            newConfigs = this.createMap( configuredPids.length );
             for ( String pid : configuredPids )
             {
                 newConfigs.put( pid, this.configurations.get( pid ) );
             }
         }
+        else
+        {
+            newConfigs = Collections.emptyMap();
+        }
         this.configurations = newConfigs;
     }
 
 
-    public boolean isDifferentPids( final String[] pids )
+    /**
+     * Returns <code>true</code> if the set of service PIDs given is
+     * different from the current set of service PIDs.
+     * <p>
+     * For comparison a <code>null</code> argument is considered to
+     * be an empty set of service PIDs.
+     *
+     * @param pids The new set of service PIDs to be compared to the
+     *      current set of service PIDs.
+     * @return <code>true</code> if the set is different
+     */
+    boolean isDifferentPids( final String[] pids )
     {
         if ( this.configurations.isEmpty() && pids == null )
         {

Added: felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceConfigurationMap.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceConfigurationMap.java?rev=1357578&view=auto
==============================================================================
--- felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceConfigurationMap.java (added)
+++ felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceConfigurationMap.java Thu Jul  5 12:28:06 2012
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.felix.cm.impl.helper;
+
+
+import java.util.HashMap;
+import java.util.Map;
+
+
+class ManagedServiceConfigurationMap extends ConfigurationMap<ManagedServiceConfigurationMap.Entry>
+{
+
+    protected ManagedServiceConfigurationMap( String[] configuredPids )
+    {
+        super( configuredPids );
+    }
+
+
+    @Override
+    protected Map<String, Entry> createMap( int size )
+    {
+        return new HashMap<String, Entry>( size );
+    }
+
+
+    @Override
+    protected boolean shallTake( TargetedPID configPid, TargetedPID factoryPid, long revision )
+    {
+        Entry entry = this.get( configPid );
+
+        // no configuration assigned yet, take it
+        if ( entry == null )
+        {
+            return true;
+        }
+
+        // compare revision numbers if raw PID is the same
+        if ( configPid.equals( entry.targetedPid ) )
+        {
+            return revision > entry.revision;
+        }
+
+        // otherwise only take if targeted PID is more binding
+        return configPid.bindsStronger( entry.targetedPid );
+    }
+
+
+    @Override
+    protected boolean removeConfiguration( TargetedPID configPid, TargetedPID factoryPid )
+    {
+        Entry entry = this.get( configPid );
+
+        // nothing to remove because the service does not know it anyway
+        if ( entry == null )
+        {
+            return false;
+        }
+
+        // update if the used targeted PID matches
+        if ( configPid.equals( entry.targetedPid ) )
+        {
+            this.put( configPid, null );
+            return true;
+        }
+
+        // the config is not assigned and so there must not be a removal
+        return false;
+    }
+
+
+    @Override
+    protected void record( TargetedPID configPid, TargetedPID factoryPid, long revision )
+    {
+        final Entry entry = ( revision < 0 ) ? null : new Entry( configPid, revision );
+        this.put( configPid, entry );
+    }
+
+    static class Entry
+    {
+        final TargetedPID targetedPid;
+        final long revision;
+
+
+        Entry( final TargetedPID targetedPid, final long revision )
+        {
+            this.targetedPid = targetedPid;
+            this.revision = revision;
+        }
+
+
+        @Override
+        public String toString()
+        {
+            return "Entry(pid=" + targetedPid + ",rev=" + revision + ")";
+        }
+    }
+}

Added: felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceFactoryConfigurationMap.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceFactoryConfigurationMap.java?rev=1357578&view=auto
==============================================================================
--- felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceFactoryConfigurationMap.java (added)
+++ felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceFactoryConfigurationMap.java Thu Jul  5 12:28:06 2012
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.felix.cm.impl.helper;
+
+
+import java.util.HashMap;
+import java.util.Map;
+
+
+public class ManagedServiceFactoryConfigurationMap extends ConfigurationMap<Map<TargetedPID, Long>>
+{
+
+    protected ManagedServiceFactoryConfigurationMap( String[] configuredPids )
+    {
+        super( configuredPids );
+    }
+
+
+    @Override
+    protected Map<String, Map<TargetedPID, Long>> createMap( int size )
+    {
+        return new HashMap<String, Map<TargetedPID, Long>>( size );
+    }
+
+
+    @Override
+    protected boolean shallTake( TargetedPID configPid, TargetedPID factoryPid, long revision )
+    {
+        Map<TargetedPID, Long> configs = this.get( factoryPid );
+
+        // no configuration yet, yes we can
+        if (configs == null) {
+            return true;
+        }
+
+        Long rev = configs.get( configPid );
+
+        // this config is missing, yes we can
+        if (rev == null) {
+            return true;
+        }
+
+        // finally take if newer
+        return rev < revision;
+    }
+
+
+    @Override
+    protected boolean removeConfiguration( TargetedPID configPid, TargetedPID factoryPid )
+    {
+        Map<TargetedPID, Long> configs = this.get( factoryPid );
+        return configs != null && configs.containsKey( configPid );
+    }
+
+
+    @Override
+    protected void record( TargetedPID configPid, TargetedPID factoryPid, long revision )
+    {
+        Map<TargetedPID, Long> configs = this.get( factoryPid );
+
+        if (configs == null) {
+            configs = new HashMap<TargetedPID, Long>( 4 );
+        }
+
+        if (revision < 0) {
+            configs.remove( configPid );
+        } else {
+            configs.put(configPid, revision);
+        }
+
+        if (configs.size() == 0) {
+            configs = null;
+        }
+
+        this.put( factoryPid, configs );
+    }
+}

Modified: felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceFactoryTracker.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceFactoryTracker.java?rev=1357578&r1=1357577&r2=1357578&view=diff
==============================================================================
--- felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceFactoryTracker.java (original)
+++ felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceFactoryTracker.java Thu Jul  5 12:28:06 2012
@@ -20,7 +20,6 @@ package org.apache.felix.cm.impl.helper;
 
 import java.util.Dictionary;
 
-import org.apache.felix.cm.impl.ConfigurationImpl;
 import org.apache.felix.cm.impl.ConfigurationManager;
 import org.osgi.framework.ServiceReference;
 import org.osgi.service.cm.ManagedServiceFactory;
@@ -35,45 +34,65 @@ public class ManagedServiceFactoryTracke
 
 
     @Override
-    public void provideConfiguration( ServiceReference<ManagedServiceFactory> reference, final ConfigurationImpl config, final Dictionary<String, ?> rawProps )
+    protected ConfigurationMap<?> createConfigurationMap( String[] pids )
+    {
+        return new ManagedServiceFactoryConfigurationMap( pids );
+    }
+
+
+    @Override
+    public void provideConfiguration( ServiceReference<ManagedServiceFactory> reference, TargetedPID configPid,
+        TargetedPID factoryPid, Dictionary<String, ?> properties, long revision )
     {
         ManagedServiceFactory service = getRealService( reference );
-        if ( service != null )
+        final ConfigurationMap configs = this.getService( reference );
+        if ( service != null && configs != null )
         {
-            try
-            {
-                Dictionary props = getProperties( rawProps, config.getFactoryPidString(), reference );
-                service.updated( config.getPidString(), props );
-            }
-            catch ( Throwable t )
+            if ( configs.shallTake( configPid, factoryPid, revision ) )
             {
-                this.handleCallBackError( t, reference, config );
-            }
-            finally
-            {
-                this.ungetRealService( reference );
+                try
+                {
+                    Dictionary props = getProperties( properties, reference, configPid.toString(),
+                        factoryPid.toString() );
+                    service.updated( configPid.toString(), props );
+                    configs.record( configPid, factoryPid, revision );
+                }
+                catch ( Throwable t )
+                {
+                    this.handleCallBackError( t, reference, configPid );
+                }
+                finally
+                {
+                    this.ungetRealService( reference );
+                }
             }
         }
     }
 
 
     @Override
-    public void removeConfiguration( ServiceReference<ManagedServiceFactory> reference, final ConfigurationImpl config )
+    public void removeConfiguration( ServiceReference<ManagedServiceFactory> reference, TargetedPID configPid,
+        TargetedPID factoryPid )
     {
-        ManagedServiceFactory service = this.getRealService( reference );
-        if ( service != null )
+        final ManagedServiceFactory service = this.getRealService( reference );
+        final ConfigurationMap configs = this.getService( reference );
+        if ( service != null && configs != null)
         {
-            try
-            {
-                service.deleted( config.getPidString() );
-            }
-            catch ( Throwable t )
-            {
-                this.handleCallBackError( t, reference, config );
-            }
-            finally
+            if ( configs.removeConfiguration( configPid, factoryPid ) )
             {
-                this.ungetRealService( reference );
+                try
+                {
+                    service.deleted( configPid.toString() );
+                    configs.record( configPid, factoryPid, -1 );
+                }
+                catch ( Throwable t )
+                {
+                    this.handleCallBackError( t, reference, configPid );
+                }
+                finally
+                {
+                    this.ungetRealService( reference );
+                }
             }
         }
     }

Modified: felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceTracker.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceTracker.java?rev=1357578&r1=1357577&r2=1357578&view=diff
==============================================================================
--- felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceTracker.java (original)
+++ felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceTracker.java Thu Jul  5 12:28:06 2012
@@ -20,8 +20,8 @@ package org.apache.felix.cm.impl.helper;
 
 
 import java.util.Dictionary;
+import java.util.Hashtable;
 
-import org.apache.felix.cm.impl.ConfigurationImpl;
 import org.apache.felix.cm.impl.ConfigurationManager;
 import org.osgi.framework.ServiceReference;
 import org.osgi.service.cm.ManagedService;
@@ -30,6 +30,9 @@ import org.osgi.service.cm.ManagedServic
 public class ManagedServiceTracker extends BaseTracker<ManagedService>
 {
 
+    private static final Dictionary<String, ?> INITIAL_MARKER = new Hashtable<String, Object>( 0 );
+
+
     public ManagedServiceTracker( ConfigurationManager cm )
     {
         super( cm, false );
@@ -37,43 +40,76 @@ public class ManagedServiceTracker exten
 
 
     @Override
-    public void provideConfiguration( ServiceReference<ManagedService> service, final ConfigurationImpl config,
-        Dictionary<String, ?> properties )
+    protected ConfigurationMap<?> createConfigurationMap( String[] pids )
     {
-        updateService( service, config, properties );
+        return new ManagedServiceConfigurationMap( pids );
     }
 
 
     @Override
-    public void removeConfiguration( ServiceReference<ManagedService> service, final ConfigurationImpl config )
+    public void provideConfiguration( ServiceReference<ManagedService> service, TargetedPID configPid,
+        TargetedPID factoryPid, Dictionary<String, ?> properties, long revision )
     {
-        updateService( service, config, null );
+        Dictionary<String, ?> supplied = ( properties == null ) ? INITIAL_MARKER : properties;
+        updateService( service, configPid, supplied, revision );
     }
 
 
-    private void updateService( ServiceReference<ManagedService> service, final ConfigurationImpl config,
-        Dictionary<String, ?> properties )
+    @Override
+    public void removeConfiguration( ServiceReference<ManagedService> service, TargetedPID configPid,
+        TargetedPID factoryPid )
     {
-        ManagedService srv = this.getRealService( service );
-        if ( srv != null )
+        updateService( service, configPid, null, -1 );
+    }
+
+
+    private void updateService( ServiceReference<ManagedService> service, final TargetedPID configPid,
+        Dictionary<String, ?> properties, long revision )
+    {
+        final ManagedService srv = this.getRealService( service );
+        final ConfigurationMap configs = this.getService( service );
+        if ( srv != null && configs != null )
         {
-            try
+            boolean doUpdate = false;
+            if ( properties == null )
             {
-                if ( properties != null )
-                {
-                    properties = getProperties( properties, config.getPidString(), service );
-                }
-
-                srv.updated( properties );
+                doUpdate = configs.removeConfiguration( configPid, null );
+            }
+            else if ( properties == INITIAL_MARKER )
+            {
+                // initial call to ManagedService may supply null properties
+                properties = null;
+                revision = -1;
+                doUpdate = true;
             }
-            catch ( Throwable t )
+            else if ( configs.shallTake( configPid, null, revision ) )
             {
-                this.handleCallBackError( t, service, config );
+                // run the plugins and cause the update
+                properties = getProperties( properties, service, configPid.toString(), null );
+                doUpdate = true;
             }
-            finally
+            else
             {
-                this.ungetRealService( service );
+                // new configuration is not a better match, don't update
+                doUpdate = false;
+            }
+
+            if ( doUpdate )
+            {
+                try
+                {
+                    srv.updated( properties );
+                    configs.record( configPid, null, revision );
+                }
+                catch ( Throwable t )
+                {
+                    this.handleCallBackError( t, service, configPid );
+                }
+                finally
+                {
+                    this.ungetRealService( service );
+                }
             }
         }
-    }
+   }
 }
\ No newline at end of file

Modified: felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/TargetedPID.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/TargetedPID.java?rev=1357578&r1=1357577&r2=1357578&view=diff
==============================================================================
--- felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/TargetedPID.java (original)
+++ felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/helper/TargetedPID.java Thu Jul  5 12:28:06 2012
@@ -44,6 +44,17 @@ public class TargetedPID
     private final String version;
     private final String location;
 
+    /**
+     * The level of binding of this targeted PID:
+     * <ul>
+     * <li><code>0</code> -- this PID is not targeted at all</li>
+     * <li><code>1</code> -- this PID is targeted by the symbolic name</li>
+     * <li><code>2</code> -- this PID is targeted by the symbolic name and version</li>
+     * <li><code>3</code> -- this PID is targeted by the symoblic name, version, and location</li>
+     * </ul>
+     */
+    private final short bindingLevel;
+
 
     /**
      * Returns the bundle's version as required for targeted PIDs: If the
@@ -75,6 +86,7 @@ public class TargetedPID
             this.symbolicName = null;
             this.version = null;
             this.location = null;
+            this.bindingLevel = 0;
         }
         else
         {
@@ -93,11 +105,13 @@ public class TargetedPID
                 {
                     this.version = rawPid.substring( start, end );
                     this.location = rawPid.substring( end + 1 );
+                    this.bindingLevel = 3;
                 }
                 else
                 {
                     this.version = rawPid.substring( start );
                     this.location = null;
+                    this.bindingLevel = 2;
                 }
             }
             else
@@ -105,6 +119,7 @@ public class TargetedPID
                 this.symbolicName = rawPid.substring( start );
                 this.version = null;
                 this.location = null;
+                this.bindingLevel = 1;
             }
         }
     }
@@ -181,6 +196,7 @@ public class TargetedPID
         return rawPid;
     }
 
+
     /**
      * Returns the service PID of this targeted PID which basically is
      * the targeted PID without the targeting information.
@@ -190,86 +206,22 @@ public class TargetedPID
         return servicePid;
     }
 
+
     /**
-     * Returns how string this <code>TargetedPID</code> matches the
-     * given <code>ServiceReference</code>. The return value is one of
-     * those listed in the table:
-     *
-     * <table>
-     * <tr><th>level</th><th>Description</th></tr>
-     * <tr><td>-1</td><td>The targeted PID does not match at all. This means
-     *      that either the service PID, the bundle's symbolic name, the
-     *      bundle's version or bundle's location does not match the
-     *      respective non-<code>null</code> parts of the targeted PID.
-     *      This value is also returned if the raw PID fully matches the
-     *      service PID.</td></tr>
-     * <tr><td>0</td><td>The targeted PID only has the service PID which
-     *      also matches. The bundle's symbolic name, version, and
-     *      location are not considered.</td></tr>
-     * <tr><td>1</td><td>The targeted PID only has the service PID and
-     *      bundle symbolic name which both match. The bundle's version and
-     *      location are not considered.</td></tr>
-     * <tr><td>2</td><td>The targeted PID only has the service PID, bundle
-     *      symbolic name and version which all match. The bundle's
-     *      location is not considered.</td></tr>
-     * <tr><td>3</td><td>The targeted PID has the service PID as well as
-     *      the bundle symbolic name, version, and location which all
-     *      match.</td></tr>
-     * </table>
+     * Returns <code>true</code> if the <code>other</code> {@link TargetedPID}
+     * binds stronger than this.
+     * <p>
+     * This method assumes both targeted PIDs have already been checked for
+     * suitability for the bundle encoded in the targetting.
      *
-     * @param ref
-     * @return
+     * @param other The targeted PID to check whether it is binding stronger
+     *      or not.
+     * @return <code>true</code> if the <code>other</code> targeted PID
+     *      is binding strong.
      */
-    public int matchLevel( final ServiceReference ref )
+    boolean bindsStronger( final TargetedPID other )
     {
-
-        // TODO: this fails on multi-value PID properties !
-        final Object servicePid = ref.getProperty( Constants.SERVICE_PID );
-
-        // in case the service PID contains | characters, this allows
-        // it to match the raw version of the targeted PID
-        if ( this.rawPid.equals( servicePid ) )
-        {
-            return 1;
-        }
-
-        if ( !this.servicePid.equals( servicePid ) )
-        {
-            return -1;
-        }
-
-        if ( this.symbolicName == null )
-        {
-            return 0;
-        }
-
-        final Bundle refBundle = ref.getBundle();
-        if ( !this.symbolicName.equals( refBundle.getSymbolicName() ) )
-        {
-            return -1;
-        }
-
-        if ( this.version == null )
-        {
-            return 1;
-        }
-
-        if ( !this.version.equals( refBundle.getHeaders().get( Constants.BUNDLE_VERSION ) ) )
-        {
-            return -1;
-        }
-
-        if ( this.location == null )
-        {
-            return 2;
-        }
-
-        if ( !this.location.equals( refBundle.getLocation() ) )
-        {
-            return -1;
-        }
-
-        return 3;
+        return other.bindingLevel > this.bindingLevel;
     }
 
 
@@ -285,7 +237,7 @@ public class TargetedPID
     {
         if ( obj == null )
         {
-            return true;
+            return false;
         }
         else if ( obj == this )
         {
@@ -302,6 +254,7 @@ public class TargetedPID
         return false;
     }
 
+
     @Override
     public String toString()
     {

Modified: felix/trunk/configadmin/src/test/java/org/apache/felix/cm/impl/helper/ConfigurationMapTest.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/test/java/org/apache/felix/cm/impl/helper/ConfigurationMapTest.java?rev=1357578&r1=1357577&r2=1357578&view=diff
==============================================================================
--- felix/trunk/configadmin/src/test/java/org/apache/felix/cm/impl/helper/ConfigurationMapTest.java (original)
+++ felix/trunk/configadmin/src/test/java/org/apache/felix/cm/impl/helper/ConfigurationMapTest.java Thu Jul  5 12:28:06 2012
@@ -19,6 +19,9 @@
 package org.apache.felix.cm.impl.helper;
 
 
+import java.util.HashMap;
+import java.util.Map;
+
 import junit.framework.TestCase;
 
 import org.junit.Test;
@@ -28,9 +31,22 @@ public class ConfigurationMapTest
 {
 
     @Test
+    public void test_accepts()
+    {
+        ConfigurationMap holder = new TestConfigurationMap( new String[]
+            { "a", "b", "c" } );
+
+        TestCase.assertTrue( holder.accepts( "a" ) );
+        TestCase.assertTrue( holder.accepts( "b" ) );
+        TestCase.assertTrue( holder.accepts( "c" ) );
+
+        TestCase.assertFalse( holder.accepts( "x" ) );
+    }
+
+    @Test
     public void test_isDifferentPids_null_null()
     {
-        ConfigurationMap holder = new ConfigurationMap( null );
+        ConfigurationMap holder = new TestConfigurationMap( null );
         TestCase.assertFalse( "Expect both pids null to be the same", holder.isDifferentPids( null ) );
     }
 
@@ -38,7 +54,7 @@ public class ConfigurationMapTest
     @Test
     public void test_isDifferentPids_null_notNull()
     {
-        ConfigurationMap holder = new ConfigurationMap( null );
+        ConfigurationMap holder = new TestConfigurationMap( null );
         TestCase.assertTrue( "Expect not same for one pid not null", holder.isDifferentPids( new String[]
             { "entry" } ) );
     }
@@ -47,7 +63,7 @@ public class ConfigurationMapTest
     @Test
     public void test_isDifferentPids_notNull_null()
     {
-        ConfigurationMap holder = new ConfigurationMap( new String[]
+        ConfigurationMap holder = new TestConfigurationMap( new String[]
             { "entry" } );
         TestCase.assertTrue( "Expect not same for one pid not null", holder.isDifferentPids( null ) );
     }
@@ -65,16 +81,60 @@ public class ConfigurationMapTest
         final String[] pids30 =
             { "a", "b", "c" };
 
-        final ConfigurationMap holder10 = new ConfigurationMap( pids10 );
+        final ConfigurationMap holder10 = new TestConfigurationMap( pids10 );
         TestCase.assertFalse( holder10.isDifferentPids( pids10 ) );
         TestCase.assertFalse( holder10.isDifferentPids( pids11 ) );
         TestCase.assertTrue( holder10.isDifferentPids( pids20 ) );
         TestCase.assertTrue( holder10.isDifferentPids( pids30 ) );
 
-        final ConfigurationMap holder20 = new ConfigurationMap( pids20 );
+        final ConfigurationMap holder20 = new TestConfigurationMap( pids20 );
         TestCase.assertTrue( holder20.isDifferentPids( pids10 ) );
         TestCase.assertTrue( holder20.isDifferentPids( pids11 ) );
         TestCase.assertFalse( holder20.isDifferentPids( pids20 ) );
         TestCase.assertTrue( holder20.isDifferentPids( pids30 ) );
     }
+
+    /*
+     * Simple ConfigurationMap implementation sufficing for these tests
+     * which only test the methods in the abstract base class.
+     */
+    static class TestConfigurationMap extends ConfigurationMap<String>
+    {
+
+        protected TestConfigurationMap( String[] configuredPids )
+        {
+            super( configuredPids );
+        }
+
+
+        @Override
+        protected Map<String, String> createMap( int size )
+        {
+            return new HashMap<String, String>( size );
+        }
+
+
+        @Override
+        protected void record( TargetedPID configPid, TargetedPID factoryPid, long revision )
+        {
+            TestCase.fail( "<record> is not implemented" );
+        }
+
+
+        @Override
+        protected boolean shallTake( TargetedPID configPid, TargetedPID factoryPid, long revision )
+        {
+            TestCase.fail( "<shallTake> is not implemented" );
+            return false;
+        }
+
+
+        @Override
+        protected boolean removeConfiguration( TargetedPID configPid, TargetedPID factoryPid )
+        {
+            TestCase.fail( "<removeConfiguration> is not implemented" );
+            return false;
+        }
+
+    }
 }