FELIX-3729 Deal with service changes when target filter changes for dynamic multiple

git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1424305 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java
index 02775c4..c9c1c26 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java
@@ -25,7 +25,9 @@
 import java.util.Collections;
 import java.util.Dictionary;
 import java.util.List;
+import java.util.Map;
 import java.util.SortedMap;
+import java.util.TreeMap;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 
@@ -121,14 +123,20 @@
         void setTracker( ServiceTracker<T, RefPair<T>> tracker );
 
         void setTrackerOpened();
+
+        void setPreviousRefMap( Map<ServiceReference<T>, RefPair<T>> previousRefMap );
     }
 
     private abstract class AbstractCustomizer implements Customizer<T>
     {
+        private final Map<ServiceReference<T>, RefPair<T>> EMPTY_REF_MAP = Collections.emptyMap();
+
         private ServiceTracker<T, RefPair<T>> tracker;
 
         private volatile boolean trackerOpened;
 
+        private volatile Map<ServiceReference<T>, RefPair<T>> previousRefMap = EMPTY_REF_MAP;
+
         public void setTracker( ServiceTracker<T, RefPair<T>> tracker )
         {
             this.tracker = tracker;
@@ -163,6 +171,23 @@
             trackerOpened = true;
         }
 
+        protected Map<ServiceReference<T>, RefPair<T>> getPreviousRefMap()
+        {
+            return previousRefMap;
+        }
+
+        public void setPreviousRefMap( Map<ServiceReference<T>, RefPair<T>> previousRefMap )
+        {
+            if ( previousRefMap != null )
+            {
+                this.previousRefMap = previousRefMap;
+            }
+            else
+            {
+                this.previousRefMap = EMPTY_REF_MAP;
+            }
+
+        }
     }
 
 
@@ -223,7 +248,11 @@
 
         public RefPair<T> addingService( ServiceReference<T> serviceReference )
         {
-            RefPair<T> refPair = new RefPair<T>( serviceReference  );
+            RefPair<T> refPair = getPreviousRefMap().get( serviceReference );
+            if ( refPair == null )
+            {
+                refPair = new RefPair<T>( serviceReference  );
+            }
             if (isActive())
             {
                  if (!m_bindMethods.getBind().getServiceObject( refPair, m_componentManager.getActivator().getBundleContext()))
@@ -236,16 +265,19 @@
 
         public void addedService( ServiceReference<T> serviceReference, RefPair<T> refPair )
         {
-            if (isActive())
+            if ( getPreviousRefMap().remove( serviceReference ) == null )
             {
-                if ( !refPair.isFailed() )
+                if (isActive())
                 {
-                    m_componentManager.invokeBindMethod( DependencyManager.this, refPair );
+                    if ( !refPair.isFailed() )
+                    {
+                        m_componentManager.invokeBindMethod( DependencyManager.this, refPair );
+                    }
                 }
-            }
-            else if ( isTrackerOpened() && !isOptional() )
-            {
-                m_componentManager.activateInternal();
+                else if ( isTrackerOpened() && !isOptional() )
+                {
+                    m_componentManager.activateInternal();
+                }
             }
         }
 
@@ -2106,9 +2138,14 @@
             filterString = "(&" + filterString + m_target + ")";
         }
 
+        SortedMap<ServiceReference<T>, RefPair<T>> refMap;
         if ( registered )
         {
-            unregisterServiceListener();
+            refMap = unregisterServiceListener();
+        }
+        else
+        {
+            refMap = new TreeMap<ServiceReference<T>, RefPair<T>>(Collections.reverseOrder());
         }
         m_componentManager.log( LogService.LOG_DEBUG, "Setting target property for dependency {0} to {1}", new Object[]
                 {m_dependencyMetadata.getName(), target}, null );
@@ -2124,7 +2161,7 @@
             m_targetFilter = null;
         }
 
-        registerServiceListener();
+        registerServiceListener( refMap );
 
         //TODO deal with changes in what matches filter.
 
@@ -2225,15 +2262,21 @@
 
     }
 
-    private void registerServiceListener() throws InvalidSyntaxException
+    private void registerServiceListener( SortedMap<ServiceReference<T>, RefPair<T>> refMap ) throws InvalidSyntaxException
     {
+        final ServiceTracker<T, RefPair<T>> oldTracker = trackerRef.get();
         Customizer<T> customizer = newCustomizer();
+        customizer.setPreviousRefMap( refMap );
         ServiceTracker<T, RefPair<T>> tracker = new ServiceTracker<T, RefPair<T>>( m_componentManager.getActivator().getBundleContext(), m_targetFilter, customizer );
         customizer.setTracker( tracker );
         trackerRef.set( tracker );
         registered = true;
         tracker.open();
         customizer.setTrackerOpened();
+        if ( oldTracker != null )
+        {
+            oldTracker.completeClose( refMap );
+        }
         m_componentManager.log( LogService.LOG_DEBUG, "registering service listener for dependency {0}", new Object[]
                 {m_dependencyMetadata.getName()}, null );
     }
@@ -2278,17 +2321,23 @@
         return customizer;
     }
 
-    void unregisterServiceListener()
+    SortedMap<ServiceReference<T>, RefPair<T>> unregisterServiceListener()
     {
+        SortedMap<ServiceReference<T>, RefPair<T>> refMap;
         ServiceTracker<T, RefPair<T>> tracker = trackerRef.get();
-        trackerRef.set( null ); //???
+//        trackerRef.set( null ); //???
         if ( tracker != null )
         {
-            tracker.close();
+            refMap = tracker.close();
+        }
+        else
+        {
+            refMap = new TreeMap<ServiceReference<T>, RefPair<T>>(Collections.reverseOrder());
         }
         registered = false;
         m_componentManager.log( LogService.LOG_DEBUG, "unregistering service listener for dependency {0}", new Object[]
                 {m_dependencyMetadata.getName()}, null );
+        return refMap;
     }
 
 
@@ -2305,20 +2354,6 @@
     }
 
 
-    /**
-     * Checks whether the service references matches the target filter of this
-     * dependency.
-     *
-     * @param ref The service reference to check
-     * @return <code>true</code> if this dependency has no target filter or if
-     *      the target filter matches the service reference.
-     */
-    private boolean targetFilterMatch( ServiceReference ref )
-    {
-        return m_targetFilter == null || m_targetFilter.match( ref );
-    }
-
-
     public String toString()
     {
         return "DependencyManager: Component [" + m_componentManager + "] reference [" + m_dependencyMetadata.getName() + "]";
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/ServiceTracker.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/ServiceTracker.java
index 136092f..41ce377 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/ServiceTracker.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/ServiceTracker.java
@@ -368,20 +368,22 @@
 	 * This implementation calls {@link #getServiceReferences()} to get the list
 	 * of tracked services to remove.
 	 */
-	public void close() {
+	public SortedMap<ServiceReference<S>, T> close() {
 		final Tracked outgoing;
-		final ServiceReference<S>[] references;
+//		final ServiceReference<S>[] references;
+        SortedMap<ServiceReference<S>, T> map = new TreeMap<ServiceReference<S>, T>(Collections.reverseOrder());
 		synchronized (this) {
 			outgoing = tracked;
 			if (outgoing == null) {
-				return;
+				return map;
 			}
 			if (DEBUG) {
 				System.out.println("ServiceTracker.close: " + filter);
 			}
 			outgoing.close();
-			references = getServiceReferences();
-			tracked = null;
+            outgoing.copyEntries( map );
+//			references = getServiceReferences();
+//			tracked = null;
 			try {
 				context.removeServiceListener(outgoing);
 			} catch (IllegalStateException e) {
@@ -392,18 +394,36 @@
 		synchronized (outgoing) {
 			outgoing.notifyAll(); /* wake up any waiters */
 		}
-		if (references != null) {
-			for (int i = 0; i < references.length; i++) {
-				outgoing.untrack(references[i], null);
-			}
-		}
+//		if (references != null) {
+//			for (int i = 0; i < references.length; i++) {
+//				outgoing.untrack(references[i], null);
+//			}
+//		}
 		if (DEBUG) {
 			if ((cachedReference == null) && (cachedService == null)) {
 				System.out.println("ServiceTracker.close[cached cleared]: " + filter);
 			}
 		}
+        return map;
 	}
 
+    public void completeClose(Map<ServiceReference<S>, T> toUntrack) {
+        final Tracked outgoing;
+        synchronized (this) {
+            outgoing = tracked;
+            if (outgoing == null) {
+                return;
+            }
+            tracked = null;
+            if (DEBUG) {
+                System.out.println("ServiceTracker.close: " + filter);
+            }
+        }
+        for (ServiceReference<S> ref: toUntrack.keySet()) {
+            outgoing.untrack( ref, null );
+        }
+    }
+
 	/**
 	 * Default implementation of the
 	 * {@code ServiceTrackerCustomizer.addingService} method.
@@ -820,7 +840,7 @@
             return;
         }
         synchronized (t) {
-                active = false;
+            active = false;
         }
     }