Fix FELIX-4449 and FELIX-4448

git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1575208 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/dependency/impl/ServiceReferenceManager.java b/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/dependency/impl/ServiceReferenceManager.java
index 04d6f47..7bba1cc 100644
--- a/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/dependency/impl/ServiceReferenceManager.java
+++ b/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/dependency/impl/ServiceReferenceManager.java
@@ -125,8 +125,9 @@
     }
 
     public void open() {
-        m_trackingInterceptorTracker = new Tracker(m_dependency.getBundleContext(),
-                ServiceTrackingInterceptor.class.getName(),
+        // The opening order matters, first binding, then ranking and finally tracking.
+        m_bindingInterceptorTracker = new Tracker(m_dependency.getBundleContext(),
+                ServiceBindingInterceptor.class.getName(),
                 new TrackerCustomizer() {
 
                     public boolean addingService(ServiceReference reference) {
@@ -134,11 +135,10 @@
                     }
 
                     public void addedService(ServiceReference reference) {
-                        ServiceTrackingInterceptor interceptor = (ServiceTrackingInterceptor) m_trackingInterceptorTracker
+                        ServiceBindingInterceptor interceptor = (ServiceBindingInterceptor) m_bindingInterceptorTracker
                                 .getService(reference);
-
                         if (interceptor != null) {
-                            addTrackingInterceptor(interceptor);
+                            addBindingInterceptor(interceptor);
                         } else {
                             m_dependency.getComponentInstance().getFactory().getLogger().log(Log.ERROR,
                                     "Cannot retrieve the interceptor object from service reference " + reference
@@ -148,20 +148,19 @@
                     }
 
                     public void modifiedService(ServiceReference reference, Object service) {
-                        // Not supported yet.
-                        // TODO it would be nice to support the modification of the interceptor TARGET property.
+                        // Not supported.
                     }
 
                     public void removedService(ServiceReference reference, Object service) {
-                        if (service != null && service instanceof ServiceTrackingInterceptor &&
-                                m_trackingInterceptors.contains(service)
-                        ) {
-                            removeTrackingInterceptor((ServiceTrackingInterceptor) service);
+                        if (service != null && service instanceof ServiceBindingInterceptor &&
+                                m_bindingInterceptors.contains(service)
+                                ) {
+                            removeBindingInterceptor((ServiceBindingInterceptor) service);
                         }
                     }
-                });
-
-        m_trackingInterceptorTracker.open();
+                }
+        );
+        m_bindingInterceptorTracker.open();
 
         // Initialize the service interceptor tracker.
         m_rankingInterceptorTracker = new Tracker(m_dependency.getBundleContext(), ServiceRankingInterceptor.class.getName(),
@@ -210,8 +209,8 @@
                 });
         m_rankingInterceptorTracker.open();
 
-        m_bindingInterceptorTracker = new Tracker(m_dependency.getBundleContext(),
-                ServiceBindingInterceptor.class.getName(),
+        m_trackingInterceptorTracker = new Tracker(m_dependency.getBundleContext(),
+                ServiceTrackingInterceptor.class.getName(),
                 new TrackerCustomizer() {
 
                     public boolean addingService(ServiceReference reference) {
@@ -219,10 +218,11 @@
                     }
 
                     public void addedService(ServiceReference reference) {
-                        ServiceBindingInterceptor interceptor = (ServiceBindingInterceptor) m_bindingInterceptorTracker
+                        ServiceTrackingInterceptor interceptor = (ServiceTrackingInterceptor) m_trackingInterceptorTracker
                                 .getService(reference);
+
                         if (interceptor != null) {
-                            addBindingInterceptor(interceptor);
+                            addTrackingInterceptor(interceptor);
                         } else {
                             m_dependency.getComponentInstance().getFactory().getLogger().log(Log.ERROR,
                                     "Cannot retrieve the interceptor object from service reference " + reference
@@ -232,19 +232,20 @@
                     }
 
                     public void modifiedService(ServiceReference reference, Object service) {
-                        // Not supported.
+                        // Not supported yet.
+                        // TODO it would be nice to support the modification of the interceptor TARGET property.
                     }
 
                     public void removedService(ServiceReference reference, Object service) {
-                        if (service != null && service instanceof ServiceBindingInterceptor &&
-                                m_bindingInterceptors.contains(service)
+                        if (service != null && service instanceof ServiceTrackingInterceptor &&
+                                m_trackingInterceptors.contains(service)
                                 ) {
-                            removeBindingInterceptor((ServiceBindingInterceptor) service);
+                            removeTrackingInterceptor((ServiceTrackingInterceptor) service);
                         }
                     }
-                }
-        );
-        m_bindingInterceptorTracker.open();
+                });
+
+        m_trackingInterceptorTracker.open();
     }
 
     private void addTrackingInterceptor(ServiceTrackingInterceptor interceptor) {
@@ -324,6 +325,8 @@
     private ChangeSet computeChangesInMatchingServices() {
         if (m_dependency.getTracker() == null || m_dependency.getTracker().getServiceReferences() == null) {
             // Tracker closed, no problem
+            m_dependency.getComponentInstance().getFactory().getLogger().log(Logger.DEBUG,
+                    "Tracker closed when recomputing dependency " + m_dependency.getId());
             return new ChangeSet(Collections.<ServiceReference>emptyList(),
                     Collections.<ServiceReference>emptyList(),
                     Collections.<ServiceReference>emptyList(),
@@ -349,6 +352,9 @@
                     }
                 }
             }
+            m_dependency.getComponentInstance().getFactory().getLogger().log(Logger.DEBUG,
+                    "Matching services have been recomputed: " + ServiceReferenceUtils.toString(m_matchingReferences.values()));
+
 
             // We have the new matching set.
             List<ServiceReference> beforeRanking = getSelectedServices();
@@ -358,6 +364,8 @@
             if (allServices.isEmpty()) {
                 references = Collections.emptyList();
             } else {
+                m_dependency.getComponentInstance().getFactory().getLogger().log(Logger.DEBUG,
+                        "iPOJO >> Calling getServiceReferences on the interceptor " + m_rankingInterceptor);
                 references = m_rankingInterceptor.getServiceReferences(m_dependency, allServices);
             }
 
diff --git a/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/dependency/impl/ServiceReferenceUtils.java b/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/dependency/impl/ServiceReferenceUtils.java
index d54e6e4..b8614e2 100644
--- a/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/dependency/impl/ServiceReferenceUtils.java
+++ b/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/dependency/impl/ServiceReferenceUtils.java
@@ -24,6 +24,7 @@
 import org.osgi.framework.Filter;
 import org.osgi.framework.ServiceReference;
 
+import java.util.Collection;
 import java.util.List;
 
 /**
@@ -124,4 +125,21 @@
         return !(ref1 == null || ref2 == null)
                 && ref1.getProperty(Constants.SERVICE_ID).equals(ref2.getProperty(Constants.SERVICE_ID));
     }
+
+    public static String toString(Collection<? extends ServiceReference> references) {
+        if (references == null  || references.isEmpty()) {
+            return "[]";
+        } else {
+            StringBuilder buffer = new StringBuilder("[");
+            for (ServiceReference reference : references) {
+                if (buffer.length() == 1) {
+                    buffer.append(reference.getProperty(Constants.SERVICE_ID));
+                } else {
+                    buffer.append(", ").append(reference.getProperty(Constants.SERVICE_ID));
+                }
+            }
+            buffer.append("]");
+            return buffer.toString();
+        }
+    }
 }
diff --git a/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/handlers/configuration/ConfigurationHandler.java b/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/handlers/configuration/ConfigurationHandler.java
index 5942b3e..e5da154 100644
--- a/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/handlers/configuration/ConfigurationHandler.java
+++ b/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/handlers/configuration/ConfigurationHandler.java
@@ -29,6 +29,7 @@
 import org.apache.felix.ipojo.parser.MethodMetadata;
 import org.apache.felix.ipojo.parser.PojoMetadata;
 import org.apache.felix.ipojo.util.Callback;
+import org.apache.felix.ipojo.util.Log;
 import org.apache.felix.ipojo.util.Property;
 import org.apache.felix.ipojo.util.SecurityHelper;
 import org.osgi.framework.Constants;
@@ -96,7 +97,11 @@
     /**
      * The configuration listeners.
      */
-    private List<ConfigurationListener> m_listeners = new ArrayList<ConfigurationListener>();
+    private final Set<ConfigurationListener> m_listeners = new LinkedHashSet<ConfigurationListener>();
+    /**
+     * The last configuration sent to listeners.
+     */
+    private Map<String, Object> m_lastConfiguration;
 
     /**
      * Initialize the component type.
@@ -198,7 +203,7 @@
             String man = configurables[i].getAttribute("mandatory");
             mandatory = man != null && man.equalsIgnoreCase("true");
 
-            PropertyDescription pd = null;
+            PropertyDescription pd;
             if (value == null) {
                 pd = new PropertyDescription(name, type, null, false); // Cannot be immutable if we have no value.
             } else {
@@ -333,6 +338,7 @@
             m_sr.unregister();
             m_sr = null;
         }
+        m_lastConfiguration = Collections.emptyMap();
     }
 
     /**
@@ -800,30 +806,27 @@
 
     /**
      * Remove the given listener from the configuration handler's list of listeners.
+     * If the listeners is not registered, this method does nothing.
      *
      * @param listener the {@code ConfigurationListener} object to be removed
      * @throws NullPointerException   if {@code listener} is {@code null}
-     * @throws NoSuchElementException if {@code listener} wasn't present the in configuration handler's list of listeners
      */
     public void removeListener(ConfigurationListener listener) {
         if (listener == null) {
-            throw new NullPointerException("null listener");
+            throw new NullPointerException("The list of listener is null");
         }
         synchronized (m_listeners) {
             // We definitely cannot rely on listener's equals method...
             // ...so we need to manually search for the listener, using ==.
-            int i = -1;
-            for (int j = m_listeners.size() - 1; j >= 0; j--) {
-                if (m_listeners.get(j) == listener) {
-                    // Found!
-                    i = j;
+            ConfigurationListener found = null;
+            for (ConfigurationListener l : m_listeners) {
+                if (l == listener) {
+                    found = l;
                     break;
                 }
             }
-            if (i != -1) {
-                m_listeners.remove(i);
-            } else {
-                throw new NoSuchElementException("no such listener");
+            if (found != null) {
+                m_listeners.remove(found);
             }
         }
     }
@@ -834,17 +837,54 @@
      * @param map the new configuration of the component instance.
      */
     private void notifyListeners(Map<String, Object> map) {
+
         // Get a snapshot of the listeners
+        // and check if we had a change in the map.
         List<ConfigurationListener> tmp;
         synchronized (m_listeners) {
             tmp = new ArrayList<ConfigurationListener>(m_listeners);
+
+            if (map == null) {
+                if (m_lastConfiguration == null) {
+                    // No change.
+                    return;
+                }
+                // Else trigger the change.
+            } else {
+                if (m_lastConfiguration != null && m_lastConfiguration.size() == map.size()) {
+                    // Must compare key by key
+                    boolean diff = false;
+                    for (String k : map.keySet()) {
+                        if (! map.get(k).equals(m_lastConfiguration.get(k))) {
+                            // Difference found, break;
+                            diff = true;
+                            break;
+                        }
+                    }
+                    if (! diff) {
+                        // no difference found, skip notification
+                        return;
+                    }
+                }
+                // Else difference found, triggers the change
+            }
+
+            if (map == null) {
+                m_lastConfiguration = Collections.emptyMap();
+            } else {
+                m_lastConfiguration = Collections.unmodifiableMap(map);
+            }
+
+        }
+        if (! tmp.isEmpty()) {
+            getLogger().log(Log.DEBUG, String.format(
+                    "[%s] Notifying configuration listener: %s", getInstanceManager().getInstanceName(), tmp));
         }
         // Protect the map.
-        map = Collections.unmodifiableMap(map);
         // Do notify, outside any lock
         for (ConfigurationListener l : tmp) {
             try {
-                l.configurationChanged(getInstanceManager(), map);
+                l.configurationChanged(getInstanceManager(), m_lastConfiguration);
             } catch (Throwable e) {
                 // Failure inside a listener: put a warning on the logger, and continue
                 warn(String.format(
diff --git a/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/util/DependencyModel.java b/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/util/DependencyModel.java
index c27e439..1040787 100644
--- a/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/util/DependencyModel.java
+++ b/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/util/DependencyModel.java
@@ -21,7 +21,6 @@
 import org.apache.felix.ipojo.ComponentInstance;

 import org.apache.felix.ipojo.IPOJOServiceFactory;

 import org.apache.felix.ipojo.dependency.impl.ServiceReferenceManager;

-import org.apache.felix.ipojo.handlers.dependency.DependencyCallback;

 import org.osgi.framework.BundleContext;

 import org.osgi.framework.Filter;

 import org.osgi.framework.InvalidSyntaxException;