UNOMI-901: change condition evaluator injection to avoid reference issue on stop" (#730)

Author: jsinovassin

persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/ElasticSearchPersistenceServiceImpl.java

@@ -74,8 +74,7 @@
 import org.apache.unomi.persistence.spi.aggregate.DateRangeAggregate;
 import org.apache.unomi.persistence.spi.aggregate.IpRangeAggregate;
 import org.apache.unomi.persistence.spi.conditions.ConditionContextHelper;
-import org.apache.unomi.persistence.spi.conditions.ConditionEvaluator;
-import org.apache.unomi.persistence.spi.conditions.ConditionEvaluatorDispatcher;
+import org.apache.unomi.persistence.spi.conditions.evaluator.ConditionEvaluatorDispatcher;
 import org.elasticsearch.client.*;
 
 import org.osgi.framework.*;
@@ -611,19 +610,6 @@ protected Object execute(Object... args) throws IOException {
         bundleContext.removeBundleListener(this);
     }
 
-    public void bindConditionEvaluator(ServiceReference<ConditionEvaluator> conditionEvaluatorServiceReference) {
-        ConditionEvaluator conditionEvaluator = bundleContext.getService(conditionEvaluatorServiceReference);
-        conditionEvaluatorDispatcher.addEvaluator(conditionEvaluatorServiceReference.getProperty("conditionEvaluatorId").toString(),
-                conditionEvaluator);
-    }
-
-    public void unbindConditionEvaluator(ServiceReference<ConditionEvaluator> conditionEvaluatorServiceReference) {
-        if (conditionEvaluatorServiceReference == null) {
-            return;
-        }
-        conditionEvaluatorDispatcher.removeEvaluator(conditionEvaluatorServiceReference.getProperty("conditionEvaluatorId").toString());
-    }
-
     public void bindConditionESQueryBuilder(ServiceReference<ConditionESQueryBuilder> conditionESQueryBuilderServiceReference) {
         ConditionESQueryBuilder conditionESQueryBuilder = bundleContext.getService(conditionESQueryBuilderServiceReference);
         conditionESQueryBuilderDispatcher.addQueryBuilder(conditionESQueryBuilderServiceReference.getProperty("queryBuilderId").toString(),

persistence-elasticsearch/core/src/main/resources/OSGI-INF/blueprint/blueprint.xml

@@ -75,7 +75,8 @@
 
     <reference id="metricsService" interface="org.apache.unomi.metrics.MetricsService"/>
     <reference id="scriptExecutor" interface="org.apache.unomi.scripting.ScriptExecutor"/>
-    <reference id="conditionEvaluatorDispatcherService" interface="org.apache.unomi.persistence.spi.conditions.ConditionEvaluatorDispatcher"/>
+    <reference id="conditionEvaluatorDispatcherService"
+               interface="org.apache.unomi.persistence.spi.conditions.evaluator.ConditionEvaluatorDispatcher"/>
 
     <service id="elasticSearchPersistenceService" ref="elasticSearchPersistenceServiceImpl">
         <interfaces>
@@ -148,15 +149,6 @@
         <property name="logLevelRestClient" value="${es.logLevelRestClient}"/>
     </bean>
 
-    <!-- We use a listener here because using the list directly for listening to proxies coming from the same bundle didn't seem to work -->
-    <reference-list id="conditionEvaluators"
-                    interface="org.apache.unomi.persistence.spi.conditions.ConditionEvaluator"
-                    availability="optional">
-        <reference-listener
-                bind-method="bindConditionEvaluator" unbind-method="unbindConditionEvaluator"
-                ref="elasticSearchPersistenceServiceImpl"/>
-    </reference-list>
-
     <reference-list id="conditionESQueryBuilders"
                     interface="org.apache.unomi.persistence.elasticsearch.ConditionESQueryBuilder"
                     availability="optional">

persistence-spi/pom.xml

@@ -57,6 +57,11 @@
             <artifactId>unomi-metrics</artifactId>
             <scope>provided</scope>
         </dependency>
+        <dependency>
+            <groupId>org.osgi</groupId>
+            <artifactId>org.osgi.service.component.annotations</artifactId>
+            <scope>provided</scope>
+        </dependency>
         <dependency>
             <groupId>com.fasterxml.jackson.core</groupId>
             <artifactId>jackson-core</artifactId>
@@ -121,9 +126,9 @@
                 <extensions>true</extensions>
                 <configuration>
                     <instructions>
+                        <_dsannotations>*</_dsannotations>
                         <Embed-Dependency>*;scope=compile|runtime</Embed-Dependency>
                         <Import-Package>
-
                             *
                         </Import-Package>
                     </instructions>

…e/spi/conditions/ConditionEvaluator.java …itions/evaluator/ConditionEvaluator.javapersistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/ConditionEvaluator.java renamed to persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/evaluator/ConditionEvaluator.java persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/ConditionEvaluator.java renamed to persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/evaluator/ConditionEvaluator.java

@@ -15,7 +15,7 @@
  * limitations under the License.
  */
 
-package org.apache.unomi.persistence.spi.conditions;
+package org.apache.unomi.persistence.spi.conditions.evaluator;
 
 import org.apache.unomi.api.Item;
 import org.apache.unomi.api.conditions.Condition;

…itions/ConditionEvaluatorDispatcher.java …luator/ConditionEvaluatorDispatcher.javapersistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/ConditionEvaluatorDispatcher.java renamed to persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/evaluator/ConditionEvaluatorDispatcher.java persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/ConditionEvaluatorDispatcher.java renamed to persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/evaluator/ConditionEvaluatorDispatcher.java

@@ -14,7 +14,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.apache.unomi.persistence.spi.conditions;
+package org.apache.unomi.persistence.spi.conditions.evaluator;
 
 import org.apache.unomi.api.Item;
 import org.apache.unomi.api.conditions.Condition;
@@ -23,10 +23,6 @@
 
 public interface ConditionEvaluatorDispatcher {
 
-    void addEvaluator(String name, ConditionEvaluator evaluator);
-
-    void removeEvaluator(String name);
-
     boolean eval(Condition condition, Item item);
 
     boolean eval(Condition condition, Item item, Map<String, Object> context);

…ns/ConditionEvaluatorDispatcherImpl.java …pl/ConditionEvaluatorDispatcherImpl.javapersistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/ConditionEvaluatorDispatcherImpl.java renamed to persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/evaluator/impl/ConditionEvaluatorDispatcherImpl.java persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/ConditionEvaluatorDispatcherImpl.java renamed to persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/evaluator/impl/ConditionEvaluatorDispatcherImpl.java

@@ -15,13 +15,21 @@
  * limitations under the License.
  */
 
-package org.apache.unomi.persistence.spi.conditions;
+package org.apache.unomi.persistence.spi.conditions.evaluator.impl;
 
 import org.apache.unomi.api.Item;
 import org.apache.unomi.api.conditions.Condition;
 import org.apache.unomi.metrics.MetricAdapter;
 import org.apache.unomi.metrics.MetricsService;
+import org.apache.unomi.persistence.spi.conditions.ConditionContextHelper;
+import org.apache.unomi.persistence.spi.conditions.evaluator.ConditionEvaluator;
+import org.apache.unomi.persistence.spi.conditions.evaluator.ConditionEvaluatorDispatcher;
 import org.apache.unomi.scripting.ScriptExecutor;
+import org.osgi.annotation.bundle.Requirement;
+import org.osgi.service.component.annotations.Component;
+import org.osgi.service.component.annotations.Reference;
+import org.osgi.service.component.annotations.ReferenceCardinality;
+import org.osgi.service.component.annotations.ReferencePolicy;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -32,33 +40,37 @@
 /**
  * Entry point for condition evaluation. Will dispatch to all evaluators.
  */
-//TODO change to delarative services remove blueprint
-public class ConditionEvaluatorDispatcherImpl implements ConditionEvaluatorDispatcher {
-    private static final Logger LOGGER = LoggerFactory.getLogger(ConditionEvaluatorDispatcher.class.getName());
+
+@Component(service = ConditionEvaluatorDispatcher.class)
+public class ConditionEvaluatorDispatcherImpl
+        implements ConditionEvaluatorDispatcher {
+    private static final Logger LOGGER = LoggerFactory.getLogger(ConditionEvaluatorDispatcherImpl.class.getName());
 
     private Map<String, ConditionEvaluator> evaluators = new ConcurrentHashMap<>();
 
     private MetricsService metricsService;
     private ScriptExecutor scriptExecutor;
 
-    public ConditionEvaluatorDispatcherImpl() {}
+    public ConditionEvaluatorDispatcherImpl() {
+    }
 
+    @Reference
     public void setMetricsService(MetricsService metricsService) {
         this.metricsService = metricsService;
     }
 
+    @Reference
     public void setScriptExecutor(ScriptExecutor scriptExecutor) {
         this.scriptExecutor = scriptExecutor;
     }
 
-    @Override
-    public void addEvaluator(String name, ConditionEvaluator evaluator) {
-        evaluators.put(name, evaluator);
+    @Reference(service = ConditionEvaluator.class, cardinality = ReferenceCardinality.MULTIPLE, policy = ReferencePolicy.DYNAMIC)
+    public void bindEvaluator(ConditionEvaluator evaluator, Map<String, Object> props) {
+        evaluators.put((String) props.get("conditionEvaluatorId"), evaluator);
     }
 
-    @Override
-    public void removeEvaluator(String name) {
-        evaluators.remove(name);
+    public void unbindEvaluator(ConditionEvaluator evaluator, Map<String, Object> props) {
+        evaluators.remove((String) props.get("conditionEvaluatorId"));
     }
 
     @Override
@@ -83,8 +95,7 @@ public boolean eval(Condition condition, Item item, Map<String, Object> context)
             final ConditionEvaluatorDispatcher dispatcher = this;
             try {
                 return new MetricAdapter<Boolean>(metricsService, this.getClass().getName() + ".conditions." + conditionEvaluatorKey) {
-                    @Override
-                    public Boolean execute(Object... args) throws Exception {
+                    @Override public Boolean execute(Object... args) throws Exception {
                         Condition contextualCondition = ConditionContextHelper.getContextualCondition(condition, context, scriptExecutor);
                         if (contextualCondition != null) {
                             return evaluator.eval(contextualCondition, item, context, dispatcher);

persistence-spi/src/main/resources/OSGI-INF/blueprint/blueprint.xml

@@ -19,8 +19,8 @@
 
 import org.apache.unomi.api.Item;
 import org.apache.unomi.api.conditions.Condition;
-import org.apache.unomi.persistence.spi.conditions.ConditionEvaluator;
-import org.apache.unomi.persistence.spi.conditions.ConditionEvaluatorDispatcher;
+import org.apache.unomi.persistence.spi.conditions.evaluator.ConditionEvaluator;
+import org.apache.unomi.persistence.spi.conditions.evaluator.ConditionEvaluatorDispatcher;
 
 import java.util.List;
 import java.util.Map;

plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/BooleanConditionEvaluator.java

@@ -20,8 +20,8 @@
 import org.apache.commons.beanutils.BeanUtils;
 import org.apache.unomi.api.Item;
 import org.apache.unomi.api.conditions.Condition;
-import org.apache.unomi.persistence.spi.conditions.ConditionEvaluator;
-import org.apache.unomi.persistence.spi.conditions.ConditionEvaluatorDispatcher;
+import org.apache.unomi.persistence.spi.conditions.evaluator.ConditionEvaluator;
+import org.apache.unomi.persistence.spi.conditions.evaluator.ConditionEvaluatorDispatcher;
 import org.apache.unomi.persistence.spi.conditions.geo.DistanceUnit;
 import org.apache.unomi.persistence.spi.conditions.geo.GeoDistance;
 import org.slf4j.Logger;

plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/GeoLocationByPointSessionConditionEvaluator.java

@@ -18,8 +18,8 @@
 
 import org.apache.unomi.api.Item;
 import org.apache.unomi.api.conditions.Condition;
-import org.apache.unomi.persistence.spi.conditions.ConditionEvaluator;
-import org.apache.unomi.persistence.spi.conditions.ConditionEvaluatorDispatcher;
+import org.apache.unomi.persistence.spi.conditions.evaluator.ConditionEvaluator;
+import org.apache.unomi.persistence.spi.conditions.evaluator.ConditionEvaluatorDispatcher;
 
 import java.util.Collection;
 import java.util.Map;