UNOMI-847: fix groovy action deletion

apache · Oct 9, 2024 · 8b2bb28 · 8b2bb28
1 parent 9323346
commit 8b2bb28
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 38 deletions.
diff --git a/...ctions/rest/src/main/java/org/apache/unomi/groovy/actions/rest/GroovyActionsEndPoint.java b/...ctions/rest/src/main/java/org/apache/unomi/groovy/actions/rest/GroovyActionsEndPoint.java
@@ -85,10 +85,11 @@ public Response save(@Multipart(value = "file") Attachment file) {
      * Deletes the rule identified by the specified identifier.
      *
      * @param actionId the identifier of the groovy action that we want to delete
+     * @return true if the action was successfully deleted, false otherwise
      */
     @DELETE
     @Path("/{actionId}")
-    public void remove(@PathParam("actionId") String actionId) {
-        groovyActionsService.remove(actionId);
+    public boolean remove(@PathParam("actionId") String actionId) {
+        return groovyActionsService.remove(actionId);
     }
 }
diff --git a/...s/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/GroovyAction.java b/...s/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/GroovyAction.java
@@ -34,10 +34,11 @@ public class GroovyAction extends MetadataItem {
     public GroovyAction() {
     }
 
-    public GroovyAction(String name, String script) {
-        super(new Metadata(name));
+    public GroovyAction(String id, String name, String script) {
+        super(new Metadata(id));
         this.name = name;
         this.script = script;
+
     }
 
     public String getName() {

diff --git a/...services/src/main/java/org/apache/unomi/groovy/actions/services/GroovyActionsService.java b/...services/src/main/java/org/apache/unomi/groovy/actions/services/GroovyActionsService.java
@@ -29,17 +29,18 @@ public interface GroovyActionsService {
     /**
      * Save a groovy action from a groovy file
      *
-     * @param actionName   actionName
+     * @param fileName     fileName
      * @param groovyScript script to save
      */
-    void save(String actionName, String groovyScript);
+    void save(String fileName, String groovyScript);
 
     /**
      * Remove a groovy action
      *
      * @param id of the action to remove
+     * @return true if the action was successfully deleted, false otherwise
      */
-    void remove(String id);
+    boolean remove(String id);
 
     /**
      * Get a groovy code source object by an id

diff --git a/...src/main/java/org/apache/unomi/groovy/actions/services/impl/GroovyActionsServiceImpl.java b/...src/main/java/org/apache/unomi/groovy/actions/services/impl/GroovyActionsServiceImpl.java
@@ -144,7 +144,7 @@ public void onDestroy() {
      * script will be parsed with the GroovyShell (groovyShell.parse(...)), the action will extends the base script, so the functions
      * could be called
      *
-     * @throws IOException
+     * @throws IOException if the base script is not found
      */
     private void loadBaseScript() throws IOException {
         URL groovyBaseScriptURL = bundleContext.getBundle().getEntry("META-INF/base/BaseScript.groovy");
@@ -179,14 +179,15 @@ private ImportCustomizer createImportCustomizer() {
     }
 
     @Override
-    public void save(String actionName, String groovyScript) {
-        GroovyCodeSource groovyCodeSource = buildClassScript(groovyScript, actionName);
+    public void save(String fileName, String groovyScript) {
+        GroovyCodeSource groovyCodeSource = buildClassScript(groovyScript, fileName);
         try {
-            saveActionType(groovyShell.parse(groovyCodeSource).getClass().getMethod("execute").getAnnotation(Action.class));
-            saveScript(actionName, groovyScript);
-            LOGGER.info("The script {} has been loaded.", actionName);
+            Action action = groovyShell.parse(groovyCodeSource).getClass().getMethod("execute").getAnnotation(Action.class);
+            saveActionType(action);
+            persistenceService.save(new GroovyAction(action.id(), action.name().isEmpty() ? action.id(): action.name(), groovyScript));
+            LOGGER.info("The script {} has been loaded and the action {} has been persisted.", fileName, action.id());
         } catch (NoSuchMethodException e) {
-            LOGGER.error("Failed to save the script {}", actionName, e);
+            LOGGER.error("Failed to save the script {}", fileName, e);
         }
     }
 
@@ -196,7 +197,7 @@ public void save(String actionName, String groovyScript) {
      * @param action Annotation containing the values to save
      */
     private void saveActionType(Action action) {
-        Metadata metadata = new Metadata(null, action.id(), action.name().equals("") ? action.id() : action.name(), action.description());
+        Metadata metadata = new Metadata(null, action.id(), action.name().isEmpty() ? action.id() : action.name(), action.description());
         metadata.setHidden(action.hidden());
         metadata.setReadOnly(true);
         metadata.setSystemTags(new HashSet<>(asList(action.systemTags())));
@@ -210,16 +211,17 @@ private void saveActionType(Action action) {
     }
 
     @Override
-    public void remove(String id) {
+    public boolean remove(String id) {
         if (groovyCodeSourceMap.containsKey(id)) {
             try {
                 definitionsService.removeActionType(
                         groovyShell.parse(groovyCodeSourceMap.get(id)).getClass().getMethod("execute").getAnnotation(Action.class).id());
             } catch (NoSuchMethodException e) {
                 LOGGER.error("Failed to delete the action type for the id {}", id, e);
             }
-            persistenceService.remove(id, GroovyAction.class);
+            return persistenceService.remove(id, GroovyAction.class);
         }
+        return false;
     }
 
     @Override
@@ -238,27 +240,15 @@ private GroovyCodeSource buildClassScript(String groovyScript, String actionName
         return new GroovyCodeSource(groovyScript, actionName, "/groovy/script");
     }
 
-    private void saveScript(String actionName, String script) {
-        GroovyAction groovyScript = new GroovyAction(actionName, script);
-        persistenceService.save(groovyScript);
-        LOGGER.info("The script {} has been persisted.", actionName);
-    }
-
     private void refreshGroovyActions() {
         Map<String, GroovyCodeSource> refreshedGroovyCodeSourceMap = new HashMap<>();
         persistenceService.getAllItems(GroovyAction.class).forEach(groovyAction -> refreshedGroovyCodeSourceMap
-                .put(groovyAction.getName(), buildClassScript(groovyAction.getScript(), groovyAction.getName())));
+                .put(groovyAction.getMetadata().getId(), buildClassScript(groovyAction.getScript(), groovyAction.getMetadata().getId())));
         groovyCodeSourceMap = refreshedGroovyCodeSourceMap;
     }
 
     private void initializeTimers() {
-        TimerTask task = new TimerTask() {
-            @Override
-            public void run() {
-                refreshGroovyActions();
-            }
-        };
-        scheduledFuture = schedulerService.getScheduleExecutorService().scheduleWithFixedDelay(task, 0, config.services_groovy_actions_refresh_interval(),
+        scheduledFuture = schedulerService.getScheduleExecutorService().scheduleWithFixedDelay(this::refreshGroovyActions, 0, config.services_groovy_actions_refresh_interval(),
                 TimeUnit.MILLISECONDS);
     }
 }
diff --git a/itests/src/test/java/org/apache/unomi/itests/GroovyActionsServiceIT.java b/itests/src/test/java/org/apache/unomi/itests/GroovyActionsServiceIT.java
@@ -95,7 +95,7 @@ public void testGroovyActionsService_triggerGroovyAction() throws IOException, I
         groovyActionsService.save(UPDATE_ADDRESS_ACTION, loadGroovyAction(UPDATE_ADDRESS_ACTION_GROOVY_FILE));
 
         keepTrying("Failed waiting for the creation of the GroovyAction for the trigger action test",
-                () -> groovyActionsService.getGroovyCodeSource(UPDATE_ADDRESS_ACTION), Objects::nonNull, DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES);
+                () -> groovyActionsService.getGroovyCodeSource(UPDATE_ADDRESS_GROOVY_ACTION), Objects::nonNull, DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES);
 
         ActionType actionType = keepTrying("Failed waiting for the creation of the GroovyAction for trigger action test",
                 () -> definitionsService.getActionType(UPDATE_ADDRESS_GROOVY_ACTION), Objects::nonNull, DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES);
@@ -110,14 +110,13 @@ public void testGroovyActionsService_triggerGroovyAction() throws IOException, I
     @Test
     public void testGroovyActionsService_saveActionAndTestSavedValues() throws IOException, InterruptedException, ClassNotFoundException {
         groovyActionsService.save(UPDATE_ADDRESS_ACTION, loadGroovyAction(UPDATE_ADDRESS_ACTION_GROOVY_FILE));
-
         ActionType actionType = keepTrying("Failed waiting for the creation of the GroovyAction for the save test",
                 () -> definitionsService.getActionType(UPDATE_ADDRESS_GROOVY_ACTION), Objects::nonNull, DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES);
 
         GroovyCodeSource groovyCodeSource = keepTrying("Failed waiting for the creation of the GroovyAction for the save test",
-                () -> groovyActionsService.getGroovyCodeSource(UPDATE_ADDRESS_ACTION), Objects::nonNull, DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES);
+                () -> groovyActionsService.getGroovyCodeSource(UPDATE_ADDRESS_GROOVY_ACTION), Objects::nonNull, DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES);
 
-        Assert.assertEquals(UPDATE_ADDRESS_ACTION, groovyActionsService.getGroovyCodeSource(UPDATE_ADDRESS_ACTION).getName());
+        Assert.assertEquals(UPDATE_ADDRESS_GROOVY_ACTION, groovyCodeSource.getName());
 
         Assert.assertTrue(actionType.getMetadata().getId().contains(UPDATE_ADDRESS_GROOVY_ACTION));
         Assert.assertEquals(2, actionType.getMetadata().getSystemTags().size());
@@ -134,13 +133,13 @@ public void testGroovyActionsService_removeGroovyAction() throws IOException, In
         groovyActionsService.save(UPDATE_ADDRESS_ACTION, loadGroovyAction(UPDATE_ADDRESS_ACTION_GROOVY_FILE));
 
         GroovyCodeSource groovyCodeSource = keepTrying("Failed waiting for the creation of the GroovyAction for the remove test",
-                () -> groovyActionsService.getGroovyCodeSource(UPDATE_ADDRESS_ACTION), Objects::nonNull, DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES);
+                () -> groovyActionsService.getGroovyCodeSource(UPDATE_ADDRESS_GROOVY_ACTION), Objects::nonNull, DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES);
 
         Assert.assertNotNull(groovyCodeSource);
 
-        groovyActionsService.remove(UPDATE_ADDRESS_ACTION);
+        groovyActionsService.remove(UPDATE_ADDRESS_GROOVY_ACTION);
 
-        waitForNullValue("Groovy action is still present", () -> groovyActionsService.getGroovyCodeSource(UPDATE_ADDRESS_ACTION),
+        waitForNullValue("Groovy action is still present", () -> groovyActionsService.getGroovyCodeSource(UPDATE_ADDRESS_GROOVY_ACTION),
                 DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES);
 
         waitForNullValue("Action type is still present", () -> definitionsService.getActionType(UPDATE_ADDRESS_GROOVY_ACTION),