FELIX-3577 Canalize all ManagedService[Factory] updates through the BaseTracker.*Configuration methods
  - move error handling to BaseTracker
  - improve service selection in BaseTracker.getServices for targeted PIDs
  - expose raw PID from TargetedPID

git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1356645 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java b/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java
index 398524f..b36b014 100644
--- a/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java
+++ b/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java
@@ -51,12 +51,9 @@
 import org.osgi.framework.ServiceRegistration;
 import org.osgi.service.cm.ConfigurationAdmin;
 import org.osgi.service.cm.ConfigurationEvent;
-import org.osgi.service.cm.ConfigurationException;
 import org.osgi.service.cm.ConfigurationListener;
 import org.osgi.service.cm.ConfigurationPermission;
 import org.osgi.service.cm.ConfigurationPlugin;
-import org.osgi.service.cm.ManagedService;
-import org.osgi.service.cm.ManagedServiceFactory;
 import org.osgi.service.cm.SynchronousConfigurationListener;
 import org.osgi.service.log.LogService;
 import org.osgi.util.tracker.ServiceTracker;
@@ -798,17 +795,28 @@
      * service property as a String[], which may be <code>null</code> if
      * the ManagedServiceFactory does not have such a property.
      */
-    //TODO: replace above configure methods
-    public void configure( String pid, ServiceReference sr, Object service, final boolean factory )
+    /**
+     * 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 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 )
     {
         Runnable r;
         if ( factory )
         {
-            r = new ManagedServiceFactoryUpdate( pid, sr, ( ManagedServiceFactory ) service );
+            r = new ManagedServiceFactoryUpdate( pid, sr );
         }
         else
         {
-            r = new ManagedServiceUpdate( pid, sr, ( ManagedService ) service );
+            r = new ManagedServiceUpdate( pid, sr );
         }
         updateThread.schedule( r );
         log( LogService.LOG_DEBUG, "[{0}] scheduled", new Object[]
@@ -1074,17 +1082,15 @@
     }
 
 
-
-
     public static String toString( ServiceReference ref )
     {
         String[] ocs = ( String[] ) ref.getProperty( "objectClass" );
-        StringBuffer buf = new StringBuffer("[");
+        StringBuffer buf = new StringBuffer( "[" );
         for ( int i = 0; i < ocs.length; i++ )
         {
-            buf.append(ocs[i]);
+            buf.append( ocs[i] );
             if ( i < ocs.length - 1 )
-                buf.append(", ");
+                buf.append( ", " );
         }
 
         buf.append( ", id=" ).append( ref.getProperty( Constants.SERVICE_ID ) );
@@ -1105,34 +1111,6 @@
     }
 
 
-    public void handleCallBackError( final Throwable error, final ServiceReference target, final ConfigurationImpl config )
-    {
-        if ( error instanceof ConfigurationException )
-        {
-            final ConfigurationException ce = ( ConfigurationException ) error;
-            if ( ce.getProperty() != null )
-            {
-                log( LogService.LOG_ERROR, "{0}: Updating property {1} of configuration {2} caused a problem: {3}",
-                    new Object[]
-                        { toString( target ), ce.getProperty(), config.getPid(), ce.getReason(), ce } );
-            }
-            else
-            {
-                log( LogService.LOG_ERROR, "{0}: Updating configuration {1} caused a problem: {2}", new Object[]
-                    { toString( target ), config.getPid(), ce.getReason(), ce } );
-            }
-        }
-        else
-        {
-            {
-                log( LogService.LOG_ERROR, "{0}: Unexpected problem updating configuration {1}", new Object[]
-                    { toString( target ), config, error } );
-            }
-
-        }
-    }
-
-
     /**
      * Checks whether the bundle is allowed to receive the configuration
      * with the given location binding.
@@ -1200,19 +1178,16 @@
 
         private final ServiceReference sr;
 
-        private final ManagedService service;
-
         private final ConfigurationImpl config;
 
         private final Dictionary rawProperties;
 
         private final long revision;
 
-        ManagedServiceUpdate( String pid, ServiceReference sr, ManagedService service )
+        ManagedServiceUpdate( String pid, ServiceReference sr )
         {
             this.pid = pid;
             this.sr = sr;
-            this.service = service;
 
             // get or load configuration for the pid
             ConfigurationImpl config = null;
@@ -1266,9 +1241,6 @@
                 {
                     // 104.4.2 Dynamic Binding
                     config.tryBindLocation( serviceBundle.getLocation() );
-
-                    // prepare the configuration for the service (call plugins)
-                    callPlugins( properties, pid, sr, config );
                 }
                 else
                 {
@@ -1293,15 +1265,7 @@
                 properties = null;
             }
 
-            // update the service with the configuration
-            try
-            {
-                service.updated( properties );
-            }
-            catch ( Throwable t )
-            {
-                handleCallBackError( t, sr, config );
-            }
+            managedServiceTracker.provideConfiguration( sr, config, properties );
         }
 
         public String toString()
@@ -1323,17 +1287,14 @@
 
         private final ServiceReference sr;
 
-        private final ManagedServiceFactory service;
-
         private final Map configs;
 
         private final Map revisions;
 
-        ManagedServiceFactoryUpdate( String factoryPid, ServiceReference sr, ManagedServiceFactory service )
+        ManagedServiceFactoryUpdate( String factoryPid, ServiceReference sr )
         {
             this.factoryPid = factoryPid;
             this.sr = sr;
-            this.service = service;
 
             Factory factory = null;
             Map configs = null;
@@ -1456,25 +1417,12 @@
                     // 104.4.2 Dynamic Binding
                     cfg.tryBindLocation( serviceBundle.getLocation() );
 
-                    // prepare the configuration for the service (call plugins)
-                    // call the plugins with cm.target set to the service's factory PID
-                    // (clarification in Section 104.9.1 of Compendium 4.2)
-                    callPlugins( properties, factoryPid, sr, cfg );
-
                     // 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() } );
-
-                        try
-                        {
-                            service.updated( cfg.getPid(), properties );
-                        }
-                        catch ( Throwable t )
-                        {
-                            handleCallBackError( t, sr, cfg );
-                        }
+                        managedServiceFactoryTracker.provideConfiguration( sr, cfg, properties );
                     }
                 }
             }
diff --git a/configadmin/src/main/java/org/apache/felix/cm/impl/helper/BaseTracker.java b/configadmin/src/main/java/org/apache/felix/cm/impl/helper/BaseTracker.java
index ab41181..5cc641e 100644
--- a/configadmin/src/main/java/org/apache/felix/cm/impl/helper/BaseTracker.java
+++ b/configadmin/src/main/java/org/apache/felix/cm/impl/helper/BaseTracker.java
@@ -18,6 +18,7 @@
  */
 package org.apache.felix.cm.impl.helper;
 
+
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -30,9 +31,13 @@
 import org.apache.felix.cm.impl.RankingComparator;
 import org.osgi.framework.Constants;
 import org.osgi.framework.ServiceReference;
+import org.osgi.service.cm.ConfigurationException;
+import org.osgi.service.cm.ManagedService;
+import org.osgi.service.cm.ManagedServiceFactory;
 import org.osgi.service.log.LogService;
 import org.osgi.util.tracker.ServiceTracker;
 
+
 /**
  * The <code>BaseTracker</code> is the base class for tracking
  * <code>ManagedService</code> and <code>ManagedServiceFactory</code>
@@ -44,11 +49,15 @@
 {
     protected final ConfigurationManager cm;
 
+    private final boolean managedServiceFactory;
 
-    protected BaseTracker( ConfigurationManager cm, Class<S> type )
+
+    protected BaseTracker( final ConfigurationManager cm, final boolean managedServiceFactory )
     {
-        super( cm.getBundleContext(), type.getName(), null );
+        super( cm.getBundleContext(), ( managedServiceFactory ? ManagedServiceFactory.class.getName()
+            : ManagedService.class.getName() ), null );
         this.cm = cm;
+        this.managedServiceFactory = managedServiceFactory;
         open();
     }
 
@@ -73,18 +82,20 @@
     }
 
 
-//    public void removedService( ServiceReference<ManagedServiceFactory> reference,
-//        ServiceHolder<ManagedServiceFactory> holder )
-//    {
-//        // nothing really to do
-//    }
-
-
-    //                 cm.configure( pids, reference, msf);
-    protected void configure( ServiceReference<S> reference, String[] pids )
+    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 )
             {
@@ -98,7 +109,7 @@
 
                     for ( int i = 0; i < pids.length; i++ )
                     {
-                        this.cm.configure( pids[i], reference, service, isFactory() );
+                        this.cm.configure( pids[i], reference, managedServiceFactory );
                     }
                 }
                 finally
@@ -118,13 +129,12 @@
             ArrayList<ServiceReference<S>> result = new ArrayList<ServiceReference<S>>( refs.length );
             for ( ServiceReference<S> ref : refs )
             {
-                if ( pid.matchesTarget( ref ) )
+                ConfigurationMap map = this.getService( ref );
+                if ( map != null
+                    && ( map.accepts( pid.getRawPid() ) || ( map.accepts( pid.getServicePid() ) && pid
+                        .matchesTarget( ref ) ) ) )
                 {
-                    ConfigurationMap map = this.getService( ref );
-                    if ( map != null && map.accepts( pid.getServicePid() ) )
-                    {
-                        result.add( ref );
-                    }
+                    result.add( ref );
                 }
             }
 
@@ -140,10 +150,9 @@
     }
 
 
-    protected abstract boolean isFactory();
-
     // Updates
-    public abstract void provideConfiguration( ServiceReference<S> service, ConfigurationImpl config, Dictionary<String, ?> properties );
+    public abstract void provideConfiguration( ServiceReference<S> service, ConfigurationImpl config,
+        Dictionary<String, ?> properties );
 
 
     public abstract void removeConfiguration( ServiceReference<S> service, ConfigurationImpl config );
@@ -160,13 +169,47 @@
         this.context.ungetService( reference );
     }
 
-    protected final Dictionary getProperties( Dictionary<String, ?> rawProperties, String targetPid, ServiceReference service )
+
+    protected final Dictionary getProperties( Dictionary<String, ?> rawProperties, String targetPid,
+        ServiceReference service )
     {
         Dictionary props = new CaseInsensitiveDictionary( rawProperties );
-        this.cm.callPlugins( props, targetPid, service, null /* config */ );
+        this.cm.callPlugins( props, targetPid, service, null /* config */);
         return props;
     }
 
+
+    protected final void handleCallBackError( final Throwable error, final ServiceReference target,
+        final ConfigurationImpl config )
+    {
+        if ( error instanceof ConfigurationException )
+        {
+            final ConfigurationException ce = ( ConfigurationException ) error;
+            if ( ce.getProperty() != null )
+            {
+                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 } );
+            }
+            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 } );
+            }
+        }
+        else
+        {
+            {
+                this.cm.log( LogService.LOG_ERROR, "{0}: Unexpected problem updating configuration {1}", new Object[]
+                    { ConfigurationManager.toString( target ), config, error } );
+            }
+
+        }
+    }
+
+
     /**
      * Returns the <code>service.pid</code> property of the service reference as
      * an array of strings or <code>null</code> if the service reference does
diff --git a/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceFactoryTracker.java b/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceFactoryTracker.java
index 4f6fc81..58eed84 100644
--- a/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceFactoryTracker.java
+++ b/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceFactoryTracker.java
@@ -30,14 +30,7 @@
 
     public ManagedServiceFactoryTracker( ConfigurationManager cm )
     {
-        super( cm, ManagedServiceFactory.class );
-    }
-
-
-    @Override
-    protected boolean isFactory()
-    {
-        return true;
+        super( cm, true );
     }
 
 
@@ -54,7 +47,7 @@
             }
             catch ( Throwable t )
             {
-                this.cm.handleCallBackError( t, reference, config );
+                this.handleCallBackError( t, reference, config );
             }
             finally
             {
@@ -76,7 +69,7 @@
             }
             catch ( Throwable t )
             {
-                this.cm.handleCallBackError( t, reference, config );
+                this.handleCallBackError( t, reference, config );
             }
             finally
             {
diff --git a/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceTracker.java b/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceTracker.java
index be04d70..56f81a2 100644
--- a/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceTracker.java
+++ b/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceTracker.java
@@ -18,6 +18,7 @@
  */
 package org.apache.felix.cm.impl.helper;
 
+
 import java.util.Dictionary;
 
 import org.apache.felix.cm.impl.ConfigurationImpl;
@@ -25,37 +26,49 @@
 import org.osgi.framework.ServiceReference;
 import org.osgi.service.cm.ManagedService;
 
+
 public class ManagedServiceTracker extends BaseTracker<ManagedService>
 {
 
-
     public ManagedServiceTracker( ConfigurationManager cm )
     {
-        super( cm, ManagedService.class );
+        super( cm, false );
     }
 
 
     @Override
-    protected boolean isFactory()
+    public void provideConfiguration( ServiceReference<ManagedService> service, final ConfigurationImpl config,
+        Dictionary<String, ?> properties )
     {
-        return false;
+        updateService( service, config, properties );
     }
 
 
     @Override
-    public void provideConfiguration( ServiceReference<ManagedService> service, final ConfigurationImpl config, Dictionary<String, ?> properties )
+    public void removeConfiguration( ServiceReference<ManagedService> service, final ConfigurationImpl config )
+    {
+        updateService( service, config, null );
+    }
+
+
+    private void updateService( ServiceReference<ManagedService> service, final ConfigurationImpl config,
+        Dictionary<String, ?> properties )
     {
         ManagedService srv = this.getRealService( service );
         if ( srv != null )
         {
             try
             {
-                Dictionary props = getProperties( properties, config.getPid(), service );
-                srv.updated( props );
+                if ( properties != null )
+                {
+                    properties = getProperties( properties, config.getPid(), service );
+                }
+
+                srv.updated( properties );
             }
             catch ( Throwable t )
             {
-                this.cm.handleCallBackError( t, service, config );
+                this.handleCallBackError( t, service, config );
             }
             finally
             {
@@ -63,22 +76,4 @@
             }
         }
     }
-
-    @Override
-    public void removeConfiguration( ServiceReference<ManagedService> service, final ConfigurationImpl config )
-    {
-        ManagedService srv = this.getRealService( service );
-        try
-        {
-            srv.updated( null );
-        }
-        catch ( Throwable t )
-        {
-            this.cm.handleCallBackError( t, service, config );
-        }
-        finally
-        {
-            this.ungetRealService( service );
-        }
-    }
 }
\ No newline at end of file
diff --git a/configadmin/src/main/java/org/apache/felix/cm/impl/helper/TargetedPID.java b/configadmin/src/main/java/org/apache/felix/cm/impl/helper/TargetedPID.java
index b50fc6d..784ef8f 100644
--- a/configadmin/src/main/java/org/apache/felix/cm/impl/helper/TargetedPID.java
+++ b/configadmin/src/main/java/org/apache/felix/cm/impl/helper/TargetedPID.java
@@ -150,9 +150,21 @@
 
 
     /**
+     * Gets the raw PID with which this instance has been created.
+     * <p>
+     * If an actual service PID contains pipe symbols that PID might be
+     * considered being targeted PID without it actually being one. This
+     * method provides access to the raw PID to allow for such services to
+     * be configured.
+     */
+    public String getRawPid()
+    {
+        return rawPid;
+    }
+
+    /**
      * Returns the service PID of this targeted PID which basically is
      * the targeted PID without the targeting information.
-     * @return
      */
     public String getServicePid()
     {
@@ -191,6 +203,8 @@
      */
     public int matchLevel( final ServiceReference ref )
     {
+
+        // 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