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;