FELIX-5155: Adapter/Aspect extra service dependencies injected twice if using callback instance.

The fix simply consists in detecting any callback instances from extra dependencies that are added into adapter
or aspect components. And when a callback instance is detected, it is removed from the dependency;
and is then re-added to actual aspect/adapter component instances.

See FilterComponent.add(Dependency ... dependencies)  and 
FilterComponent.copyDependenciescopyDependencies(List<DependencyContext> dependencies, Component component) methods.


git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1722848 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 d3890bd..23b9e9f 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,8 +538,27 @@
     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 42043e8..da017bf 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,4 +126,17 @@
      * @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/AdapterServiceImpl.java b/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/AdapterServiceImpl.java
index 202f667..92086b4 100644
--- a/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/AdapterServiceImpl.java
+++ b/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/AdapterServiceImpl.java
@@ -122,11 +122,8 @@
                 .add(dependency);
             
             configureAutoConfigState(service, m_component);
-            
-            for (DependencyContext dc : dependencies) {
-                service.add((Dependency) dc.createCopy());
-            }
-            
+            copyDependencies(dependencies, service);
+
             for (ComponentStateListener stateListener : m_stateListeners) {
                 service.add(stateListener);
             }
diff --git a/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/AspectServiceImpl.java b/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/AspectServiceImpl.java
index 8adbe12..9b33df4 100644
--- a/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/AspectServiceImpl.java
+++ b/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/AspectServiceImpl.java
@@ -152,9 +152,7 @@
             
             configureAutoConfigState(service, m_component);
             
-            for (DependencyContext dc : dependencies) {
-                service.add((Dependency) dc.createCopy());
-            }
+            copyDependencies(dependencies, service);
 
             for (int i = 0; i < m_stateListeners.size(); i++) {
                 service.add((ComponentStateListener) m_stateListeners.get(i));
diff --git a/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/BundleAdapterImpl.java b/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/BundleAdapterImpl.java
index 87f3281..166af93 100644
--- a/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/BundleAdapterImpl.java
+++ b/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/BundleAdapterImpl.java
@@ -104,9 +104,7 @@
                     .setCallbacks(m_cbInstance, m_add, m_change, m_remove) // if no callbacks, autoconfig is enabled
                     .setRequired(true));
 
-            for (DependencyContext dc : dependencies) {
-                service.add((Dependency) dc.createCopy());
-            }
+            copyDependencies(dependencies, service);
 
             for (ComponentStateListener stateListener : m_stateListeners) {
                 service.add(stateListener);
diff --git a/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/FactoryConfigurationAdapterImpl.java b/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/FactoryConfigurationAdapterImpl.java
index eccdd2c..6ad328d 100644
--- a/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/FactoryConfigurationAdapterImpl.java
+++ b/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/FactoryConfigurationAdapterImpl.java
@@ -134,9 +134,7 @@
             newService.setCallbacks(m_callbackObject, m_init, m_start, m_stop, m_destroy); // if not set, no effect
             configureAutoConfigState(newService, m_component);
             
-            for (DependencyContext dc : m_component.getDependencies()) {
-                newService.add((Dependency) dc.createCopy());
-            }
+            copyDependencies(m_component.getDependencies(), newService);
             
             for (int i = 0; i < m_stateListeners.size(); i ++) {
                 newService.add(m_stateListeners.get(i));
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 a497d40..3e52f0c 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,6 +19,7 @@
 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;
@@ -61,6 +62,7 @@
     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;
@@ -77,21 +79,33 @@
     }
 
     public Component add(Dependency ... dependencies) {
-        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.
+        // 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;
         for (Dependency dependency : dependencies) {
-            if (((DependencyContext) dependency).isRequired()) {
-                return this;
+            DependencyContext dc = (DependencyContext) dependency;
+            if (dc.isRequired()) {
+                allDependenciesOptional = false;
+            }
+            if (dc.getCallbackInstance() != null) {
+                m_dependencyCallbacks.put(dc, dc.setCallbackInstance(null));
             }
         }
-        // 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);
+        
+        // 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);
+                }
             }
         }
         return this;
@@ -356,4 +370,15 @@
     public Logger getLogger() {
         return m_component.getLogger();
     }
+    
+    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);
+        }
+    }
 }
\ No newline at end of file
diff --git a/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/ResourceAdapterImpl.java b/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/ResourceAdapterImpl.java
index 9bcc696..1115884 100644
--- a/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/ResourceAdapterImpl.java
+++ b/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/ResourceAdapterImpl.java
@@ -131,9 +131,7 @@
             
             configureAutoConfigState(service, m_component);
 
-            for (DependencyContext dc : dependencies) {
-                service.add((Dependency) dc.createCopy());
-            }
+            copyDependencies(dependencies, service);
 
             for (ComponentStateListener stateListener : m_stateListeners) {
                 service.add(stateListener);