You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@unomi.apache.org by sh...@apache.org on 2021/09/17 10:50:45 UTC

[unomi] branch unomi-1.6.x updated: UNOMI-509 : improve groovy actions services and fix integrations tests (#341)

This is an automated email from the ASF dual-hosted git repository.

shuber pushed a commit to branch unomi-1.6.x
in repository https://gitbox.apache.org/repos/asf/unomi.git


The following commit(s) were added to refs/heads/unomi-1.6.x by this push:
     new 1b62a12  UNOMI-509 : improve groovy actions services and fix integrations tests (#341)
1b62a12 is described below

commit 1b62a12c9eb874009aac25a4355f40d4c4a804f8
Author: jsinovassin <58...@users.noreply.github.com>
AuthorDate: Fri Sep 17 11:32:08 2021 +0200

    UNOMI-509 : improve groovy actions services and fix integrations tests (#341)
    
    (cherry picked from commit e900f47d3b123ef74c31d464bb44a20ac1f08eb9)
---
 .../groovy/actions/GroovyActionDispatcher.java     | 15 ++------
 .../actions/listener/GroovyActionListener.java     | 12 ++----
 .../actions/services/GroovyActionsService.java     | 22 ++++++++---
 .../services/impl/GroovyActionsServiceImpl.java    | 44 ++++++++++++++--------
 .../unomi/itests/GroovyActionsServiceIT.java       | 16 ++++----
 itests/src/test/resources/groovy/MyAction.groovy   |  2 +-
 6 files changed, 60 insertions(+), 51 deletions(-)

diff --git a/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/GroovyActionDispatcher.java b/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/GroovyActionDispatcher.java
index 0a2b02c..fadce74 100644
--- a/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/GroovyActionDispatcher.java
+++ b/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/GroovyActionDispatcher.java
@@ -16,9 +16,7 @@
  */
 package org.apache.unomi.groovy.actions;
 
-import groovy.lang.GroovyCodeSource;
 import groovy.lang.GroovyObject;
-import groovy.util.GroovyScriptEngine;
 import org.apache.unomi.api.Event;
 import org.apache.unomi.api.actions.Action;
 import org.apache.unomi.api.actions.ActionDispatcher;
@@ -26,7 +24,6 @@ import org.apache.unomi.groovy.actions.services.GroovyActionsService;
 import org.apache.unomi.metrics.MetricAdapter;
 import org.apache.unomi.metrics.MetricsService;
 import org.osgi.framework.BundleContext;
-import org.osgi.framework.wiring.BundleWiring;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -63,21 +60,15 @@ public class GroovyActionDispatcher implements ActionDispatcher {
     }
 
     public Integer execute(Action action, Event event, String actionName) {
-        GroovyAction groovyAction = groovyActionsService.getGroovyAction(actionName);
-        if (groovyAction == null) {
+        GroovyObject groovyObject = groovyActionsService.getGroovyObject(actionName);
+        if (groovyObject == null) {
             logger.warn("Couldn't find a Groovy action with name {}, action will not execute !", actionName);
         } else {
             try {
                 return new MetricAdapter<Integer>(metricsService, this.getClass().getName() + ".action.groovy." + actionName) {
                     @Override
                     public Integer execute(Object... args) throws Exception {
-                        GroovyBundleResourceConnector bundleResourceConnector = new GroovyBundleResourceConnector(bundleContext);
-                        GroovyScriptEngine engine = new GroovyScriptEngine(bundleResourceConnector,
-                                bundleContext.getBundle().adapt(BundleWiring.class).getClassLoader());
-
-                        Class clazzScript = engine.getGroovyClassLoader().parseClass(new GroovyCodeSource(groovyAction.getScript(), actionName, "/groovy/script"));
-                        GroovyObject groovyObj = (GroovyObject) clazzScript.newInstance();
-                        return Integer.valueOf((String) groovyObj.invokeMethod("execute", new Object[] { action, event }));
+                        return Integer.valueOf((String) groovyObject.invokeMethod("execute", new Object[] { action, event }));
                     }
                 }.runWithTimer();
             } catch (Exception e) {
diff --git a/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/listener/GroovyActionListener.java b/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/listener/GroovyActionListener.java
index c870640..0101c1c 100644
--- a/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/listener/GroovyActionListener.java
+++ b/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/listener/GroovyActionListener.java
@@ -20,14 +20,12 @@ import groovy.util.GroovyScriptEngine;
 import org.apache.commons.io.FilenameUtils;
 import org.apache.commons.io.IOUtils;
 import org.apache.unomi.groovy.actions.GroovyAction;
-import org.apache.unomi.groovy.actions.GroovyBundleResourceConnector;
 import org.apache.unomi.groovy.actions.services.GroovyActionsService;
 import org.apache.unomi.persistence.spi.PersistenceService;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.BundleEvent;
 import org.osgi.framework.SynchronousBundleListener;
-import org.osgi.framework.wiring.BundleWiring;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -119,16 +117,14 @@ public class GroovyActionListener implements SynchronousBundleListener {
 
     private void addGroovyAction(URL groovyActionURL) {
         try {
-            groovyActionsService.save(FilenameUtils.getName(groovyActionURL.getPath()),IOUtils.toString(groovyActionURL.openStream()));
+            groovyActionsService.save(FilenameUtils.getName(groovyActionURL.getPath()), IOUtils.toString(groovyActionURL.openStream()));
         } catch (IOException e) {
             logger.error("Failed to load the groovy action {}", groovyActionURL.getPath(), e);
         }
     }
 
-    private void removeGroovyActions(BundleContext bundleContext, URL groovyActionURL) {
-        GroovyBundleResourceConnector bundleResourceConnector = new GroovyBundleResourceConnector(bundleContext);
-        GroovyScriptEngine engine = new GroovyScriptEngine(bundleResourceConnector,
-                bundleContext.getBundle().adapt(BundleWiring.class).getClassLoader());
+    private void removeGroovyAction(URL groovyActionURL) {
+        GroovyScriptEngine engine = groovyActionsService.getGroovyScriptEngine();
         try {
             Class classScript = engine.getGroovyClassLoader().parseClass(IOUtils.toString(groovyActionURL.openStream()));
             groovyActionsService.remove(classScript.getName());
@@ -159,7 +155,7 @@ public class GroovyActionListener implements SynchronousBundleListener {
         while (bundleGroovyActions.hasMoreElements()) {
             URL groovyActionURL = bundleGroovyActions.nextElement();
             logger.debug("Found Groovy action at {}, loading... ", groovyActionURL.getPath());
-            removeGroovyActions(bundleContext, groovyActionURL);
+            removeGroovyAction(groovyActionURL);
         }
     }
 }
diff --git a/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/services/GroovyActionsService.java b/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/services/GroovyActionsService.java
index 99c6102..3a5fcd5 100644
--- a/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/services/GroovyActionsService.java
+++ b/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/services/GroovyActionsService.java
@@ -16,10 +16,10 @@
  */
 package org.apache.unomi.groovy.actions.services;
 
+import groovy.lang.GroovyObject;
+import groovy.util.GroovyScriptEngine;
 import org.apache.unomi.groovy.actions.GroovyAction;
 
-import java.io.File;
-
 /**
  * A service to load groovy files and manage {@link GroovyAction}
  */
@@ -27,21 +27,31 @@ public interface GroovyActionsService {
 
     /**
      * Save a groovy action from a groovy file
-     * @param actionName actionName
+     *
+     * @param actionName   actionName
      * @param groovyScript script to save
      */
     void save(String actionName, String groovyScript);
 
     /**
      * Remove a groovy action
+     *
      * @param id of the action to remove
      */
     void remove(String id);
 
     /**
-     * Get a groovy action by an id
+     * Get a groovy object by an id
+     *
      * @param id of the action to get
-     * @return Groovy action
+     * @return Groovy object
+     */
+    GroovyObject getGroovyObject(String id);
+
+    /**
+     * Get the groovy script engine to allow to execute groovy script
+     *
+     * @return GroovyScriptEngine
      */
-    GroovyAction getGroovyAction(String id);
+    GroovyScriptEngine getGroovyScriptEngine();
 }
diff --git a/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/services/impl/GroovyActionsServiceImpl.java b/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/services/impl/GroovyActionsServiceImpl.java
index 7f63e57..546fbff 100644
--- a/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/services/impl/GroovyActionsServiceImpl.java
+++ b/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/services/impl/GroovyActionsServiceImpl.java
@@ -17,6 +17,7 @@
 package org.apache.unomi.groovy.actions.services.impl;
 
 import groovy.lang.GroovyCodeSource;
+import groovy.lang.GroovyObject;
 import groovy.util.GroovyScriptEngine;
 import org.apache.unomi.api.Metadata;
 import org.apache.unomi.api.actions.ActionType;
@@ -34,8 +35,9 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.util.Arrays;
+import java.util.HashMap;
 import java.util.HashSet;
-import java.util.List;
+import java.util.Map;
 import java.util.TimerTask;
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
@@ -48,6 +50,8 @@ public class GroovyActionsServiceImpl implements GroovyActionsService {
 
     private BundleContext bundleContext;
 
+    private GroovyScriptEngine groovyScriptEngine;
+
     private static final Logger logger = LoggerFactory.getLogger(GroovyActionsServiceImpl.class.getName());
 
     public void setBundleContext(BundleContext bundleContext) {
@@ -63,7 +67,7 @@ public class GroovyActionsServiceImpl implements GroovyActionsService {
     @Reference
     private SchedulerService schedulerService;
 
-    private List<GroovyAction> groovyActions;
+    private Map<String, GroovyObject> groovyObjects;
 
     private Integer groovyActionsRefreshInterval = 1000;
 
@@ -83,8 +87,17 @@ public class GroovyActionsServiceImpl implements GroovyActionsService {
         this.schedulerService = schedulerService;
     }
 
+    public GroovyScriptEngine getGroovyScriptEngine() {
+        return groovyScriptEngine;
+    }
+
     public void postConstruct() {
-        logger.debug("postConstruct {" + bundleContext.getBundle() + "}");
+        groovyObjects = new HashMap<>();
+        logger.debug("postConstruct {}", bundleContext.getBundle());
+        GroovyBundleResourceConnector bundleResourceConnector = new GroovyBundleResourceConnector(bundleContext);
+
+        groovyScriptEngine = new GroovyScriptEngine(bundleResourceConnector,
+                bundleContext.getBundle().adapt(BundleWiring.class).getClassLoader());
 
         initializeTimers();
         logger.info("Groovy action service initialized.");
@@ -124,24 +137,18 @@ public class GroovyActionsServiceImpl implements GroovyActionsService {
     }
 
     @Override
-    public GroovyAction getGroovyAction(String id) {
-        return groovyActions.stream().filter(groovyAction -> groovyAction.getItemId().equals(id)).findFirst().orElse(null);
+    public GroovyObject getGroovyObject(String id) {
+        return groovyObjects.get(id);
     }
 
     private void removeActionType(String actionId) {
-
-        GroovyAction groovyAction = getGroovyAction(actionId);
-        Class classScript = buildClassScript(groovyAction.getScript(), groovyAction.getItemId());
-        definitionsService.removeActionType(((Action) classScript.getAnnotation(Action.class)).id());
+        GroovyObject groovyObject = getGroovyObject(actionId);
+        definitionsService.removeActionType(groovyObject.getClass().getAnnotation(Action.class).id());
     }
 
     private Class buildClassScript(String groovyScript, String actionName) {
-        GroovyBundleResourceConnector bundleResourceConnector = new GroovyBundleResourceConnector(bundleContext);
-
         GroovyCodeSource groovyCodeSource = new GroovyCodeSource(groovyScript, actionName, "/groovy/script");
-        GroovyScriptEngine engine = new GroovyScriptEngine(bundleResourceConnector,
-                bundleContext.getBundle().adapt(BundleWiring.class).getClassLoader());
-        return engine.getGroovyClassLoader().parseClass(groovyCodeSource);
+        return groovyScriptEngine.getGroovyClassLoader().parseClass(groovyCodeSource);
     }
 
     private void saveScript(String name, String script) {
@@ -150,7 +157,14 @@ public class GroovyActionsServiceImpl implements GroovyActionsService {
     }
 
     private void refreshGroovyActions() {
-        groovyActions = persistenceService.getAllItems(GroovyAction.class);
+        persistenceService.getAllItems(GroovyAction.class).forEach(groovyAction -> {
+            try {
+                GroovyObject groovyObject = (GroovyObject) buildClassScript(groovyAction.getScript(), groovyAction.getName()).newInstance();
+                groovyObjects.put(groovyAction.getName(), groovyObject);
+            } catch (InstantiationException | IllegalAccessException e) {
+                logger.error("Failed to instantiate groovy action {}", groovyAction.getName(), e);
+            }
+        });
     }
 
     private void initializeTimers() {
diff --git a/itests/src/test/java/org/apache/unomi/itests/GroovyActionsServiceIT.java b/itests/src/test/java/org/apache/unomi/itests/GroovyActionsServiceIT.java
index c81039d..3a798b3 100644
--- a/itests/src/test/java/org/apache/unomi/itests/GroovyActionsServiceIT.java
+++ b/itests/src/test/java/org/apache/unomi/itests/GroovyActionsServiceIT.java
@@ -17,10 +17,10 @@
 
 package org.apache.unomi.itests;
 
+import groovy.lang.GroovyObject;
 import org.apache.commons.io.IOUtils;
 import org.apache.unomi.api.actions.ActionType;
 import org.apache.unomi.api.services.DefinitionsService;
-import org.apache.unomi.groovy.actions.GroovyAction;
 import org.apache.unomi.groovy.actions.services.GroovyActionsService;
 import org.junit.After;
 import org.junit.Assert;
@@ -69,13 +69,11 @@ public class GroovyActionsServiceIT extends BaseIT {
 
         Thread.sleep(2000);
 
-        GroovyAction groovyAction = groovyActionsService.getGroovyAction("MyAction");
+        GroovyObject groovyObject = groovyActionsService.getGroovyObject("MyAction");
 
         ActionType actionType = definitionsService.getActionType("scriptGroovyAction");
 
-        Assert.assertEquals("MyAction", groovyAction.getItemId());
-        Assert.assertEquals("MyAction", groovyAction.getName());
-        Assert.assertTrue(groovyAction.getScript().contains("A test Groovy"));
+        Assert.assertEquals("MyAction", groovyObject.getClass().getName());
 
         Assert.assertTrue(actionType.getMetadata().getId().contains("scriptGroovyAction"));
         Assert.assertEquals(2, actionType.getMetadata().getSystemTags().size());
@@ -93,17 +91,17 @@ public class GroovyActionsServiceIT extends BaseIT {
 
         Thread.sleep(2000);
 
-        GroovyAction groovyAction = groovyActionsService.getGroovyAction("MyAction");
+        GroovyObject groovyObject = groovyActionsService.getGroovyObject("MyAction");
 
-        Assert.assertNotNull(groovyAction);
+        Assert.assertNotNull(groovyObject);
 
         groovyActionsService.remove("MyAction");
 
         Thread.sleep(2000);
 
-        groovyAction = groovyActionsService.getGroovyAction("MyAction");
+        groovyObject = groovyActionsService.getGroovyObject("MyAction");
 
-        Assert.assertNull(groovyAction);
+        Assert.assertNull(groovyObject);
 
         ActionType actionType = definitionsService.getActionType("scriptGroovyAction");
 
diff --git a/itests/src/test/resources/groovy/MyAction.groovy b/itests/src/test/resources/groovy/MyAction.groovy
index 8cfd1ee..15d8f45 100644
--- a/itests/src/test/resources/groovy/MyAction.groovy
+++ b/itests/src/test/resources/groovy/MyAction.groovy
@@ -30,7 +30,7 @@ import java.util.logging.Logger
         parameters = [@Parameter(id = "param1", type = "string", multivalued = false), @Parameter(id = "param2", type = "string", multivalued =
                 false)])
 class MyAction {
-    Logger logger = Logger.getdLogger("")
+    Logger logger = Logger.getLogger("")
 
     String execute(action, event) {
         logger.info("Groovy action for event type: " + event.getEventType())