FELIX-5155: Do not invoke callbacks on Factory Pid Adapters components and on AbstractDecorator components (excepts for the first internal dependency declared by aspects and adapters).
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1727864 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/context/AbstractDependency.java b/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/context/AbstractDependency.java
index 23b9e9f..d3890bd 100644
--- a/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/context/AbstractDependency.java
+++ b/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/context/AbstractDependency.java
@@ -538,27 +538,8 @@
public ComponentContext getComponentContext() {
return m_component;
}
-
- /**
- * Returns the dependency callback instance, if there is one.
- * @returns the dependency callback instance if there is one, else null.
- */
- public Object getCallbackInstance() {
- return m_callbackInstance;
- }
/**
- * Sets the dependency callback instance
- * @param callbackInstance the dependency callback instance
- * @return the previous callbackInstance, or <code>null</code> if it did not have one
- */
- public Object setCallbackInstance(Object callbackInstance) {
- Object currentCallbackInstance = m_callbackInstance;
- m_callbackInstance = callbackInstance;
- return currentCallbackInstance;
- }
-
- /**
* Returns the default service, or null.
* @param nullObject if true, a null object may be returned.
* @return the default service
diff --git a/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/context/DependencyContext.java b/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/context/DependencyContext.java
index da017bf..42043e8 100644
--- a/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/context/DependencyContext.java
+++ b/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/context/DependencyContext.java
@@ -126,17 +126,4 @@
* @return a clone of this dependency.
*/
public DependencyContext createCopy();
-
- /**
- * Returns the dependency callback instance, if there is one.
- * @returns the dependency callback instance if there is one, else null.
- */
- public Object getCallbackInstance();
-
- /**
- * Sets the dependency callback instance
- * @param callbackInstance the dependency callback instance
- * @return the previous callbackInstance, or <code>null</code> if it did not have one
- */
- public Object setCallbackInstance(Object callbackInstance);
}
diff --git a/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/ComponentImpl.java b/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/ComponentImpl.java
index 3d7aefe..0a7a49f 100644
--- a/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/ComponentImpl.java
+++ b/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/ComponentImpl.java
@@ -1438,6 +1438,16 @@
return;
}
if (m_invokeCallbackCache.put(event, event) == null) {
+ // FELIX-5155: we must not invoke callbacks on our special internal components (adapters/aspects) if the dependency is not the first one, or
+ // if the internal component is a Factory Pid Adapter.
+ // For aspects/adapters, the first dependency only need to be injected, not the other extra dependencies added by user.
+ // (in fact, we do this because extra dependencies (added by user) may contain a callback instance, and we really don't want to invoke the callbacks twice !
+ Object mainComponentImpl = getInstance();
+ if (mainComponentImpl instanceof AbstractDecorator) {
+ if (mainComponentImpl instanceof FactoryConfigurationAdapterImpl || dc != m_dependencies.get(0)) {
+ return;
+ }
+ }
dc.invokeCallback(type, event);
}
}
diff --git a/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/FilterComponent.java b/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/FilterComponent.java
index 570820e..f7ebb98 100644
--- a/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/FilterComponent.java
+++ b/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/FilterComponent.java
@@ -19,7 +19,6 @@
package org.apache.felix.dm.impl;
import java.util.Dictionary;
-import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -62,7 +61,6 @@
protected volatile Object m_factory;
protected volatile String m_factoryCreateMethod;
protected volatile Dictionary<String, Object> m_serviceProperties;
- private final Map<DependencyContext, Object> m_dependencyCallbacks = new HashMap<>();
public FilterComponent(Component service) {
m_component = (ComponentImpl) service;
@@ -79,37 +77,21 @@
}
public Component add(Dependency ... dependencies) {
- // First, detect if one of the added dependencies is required, and also, remove any callback instance found in
- // dependencies. We'll store such dependency callback instance in our m_dependencyCallbacks map and will
- // re-add them later, in concrete aspect or adapter instances (see the copyDependencies method).
- // We remove dependency callback instance because we don't want to call with internal abstract decorator instances).
-
- boolean allDependenciesOptional = true;
+ m_component.add(dependencies);
+ // Add the dependencies to all already instantiated services.
+ // If one dependency from the list is required, we have nothing to do, since our internal
+ // service will be stopped/restarted.
for (Dependency dependency : dependencies) {
- DependencyContext dc = (DependencyContext) dependency;
- if (dc.isRequired()) {
- allDependenciesOptional = false;
- }
-
- // Temporarily remove dependency callback instance (if set), because we don't want to call it twice (one time from the
- // internal aspect/adapter AbstractDecorator object, and another one time from the actual aspect/adapter component instances).
- // See FELIX-5155.
- if (dc.getCallbackInstance() != null) {
- m_dependencyCallbacks.put(dc, dc.setCallbackInstance(null));
+ if (((DependencyContext) dependency).isRequired()) {
+ return this;
}
}
-
- // Now, add the dependencies in the internal abstract decorator component.
- m_component.add(dependencies);
-
- // If all dependencies are optional, add them to already instantiated aspect or adapter components.
- if (allDependenciesOptional) {
- Object[] instances = m_component.getInstances();
- if (instances.length > 0) {
- AbstractDecorator ad = (AbstractDecorator) instances[0];
- if (ad != null) {
- ad.addDependency(dependencies);
- }
+ // Ok, the list contains no required dependencies: add optionals dependencies in already instantiated services.
+ Object[] instances = m_component.getInstances();
+ if (instances.length > 0) {
+ AbstractDecorator ad = (AbstractDecorator) instances[0];
+ if (ad != null) {
+ ad.addDependency(dependencies);
}
}
return this;
@@ -160,8 +142,6 @@
}
}
}
- // Cleanup possibly cached depenedncy callack instances.
- m_dependencyCallbacks.remove((DependencyContext) dependency);
return this;
}
@@ -380,10 +360,7 @@
protected void copyDependencies(List<DependencyContext> dependencies, Component component) {
for (DependencyContext dc : dependencies) {
DependencyContext copy = dc.createCopy();
- Object callbackInstance = m_dependencyCallbacks.get(dc);
- if (callbackInstance != null) {
- copy.setCallbackInstance(callbackInstance);
- }
+
component.add(copy);
}
}