Bugfix/#87 lost interruption in action impl

module/dal/control/src/main/java/org/openbase/bco/dal/control/action/ActionImpl.java

@@ -267,7 +267,7 @@ public Future<ActionDescription> execute() {
                         boolean retry = false;
 
                         // loop as long as task is not canceled.
-                        while (!(Thread.currentThread().isInterrupted() || actionTask == null || actionTask.isCancelled() || getActionState() == State.CANCELING)) {
+                        while (!(Thread.currentThread().isInterrupted() || actionTask == null || actionTask.isCancelled() || isTerminating())) {
                             try {
                                 // wait in case of a retry
                                 if(retry) {
@@ -282,12 +282,12 @@ public Future<ActionDescription> execute() {
 
                                 if (!isValid()) {
                                     LOGGER.debug(ActionImpl.this + " no longer valid and will be rejected!");
-                                    updateActionStateIfNotCanceled(State.REJECTED);
+                                    updateActionStateIfNotTerminating(State.REJECTED);
                                     break;
                                 }
 
                                 // Submission
-                                updateActionStateIfNotCanceled(State.SUBMISSION);
+                                updateActionStateIfNotTerminating(State.SUBMISSION);
 
                                 // only update requested state if it is an operation state, else throw an exception if not in provider control mode
                                 if (hasOperationService) {
@@ -302,11 +302,11 @@ public Future<ActionDescription> execute() {
                                 // apply new state
                                 unit.performOperationService(serviceState, serviceDescription.getServiceType()).get(EXECUTION_FAILURE_TIMEOUT, TimeUnit.MILLISECONDS);
 
-                                updateActionStateIfNotCanceled(State.EXECUTING);
+                                updateActionStateIfNotTerminating(State.EXECUTING);
 
                                 // action can be finished if not done yet and time has expired or execution time was never required.
                                 if (!isDone() && (isExpired() || getExecutionTimePeriod(TimeUnit.MICROSECONDS) == 0)) {
-                                    updateActionStateIfNotCanceled(State.FINISHED);
+                                    updateActionStateIfNotTerminating(State.FINISHED);
                                 }
                                 break;
 
@@ -319,18 +319,18 @@ public Future<ActionDescription> execute() {
                                 }
 
                                 if (!isDone()) {
-                                    updateActionStateIfNotCanceled(State.SUBMISSION_FAILED);
+                                    updateActionStateIfNotTerminating(State.SUBMISSION_FAILED);
                                 }
 
                                 // handle if action was rejected by the unit itself
                                 if (ex.getCause() instanceof RejectedException) {
-                                    updateActionStateIfNotCanceled(State.REJECTED);
+                                    updateActionStateIfNotTerminating(State.REJECTED);
                                     break;
                                 }
 
                                 // avoid execution in case unit is shutting down
                                 if (unit.isShutdownInProgress() || ExceptionProcessor.isCausedBySystemShutdown(ex)) {
-                                    updateActionStateIfNotCanceled(State.REJECTED);
+                                    updateActionStateIfNotTerminating(State.REJECTED);
                                     break;
                                 }
 
@@ -533,7 +533,9 @@ public Future<ActionDescription> cancel() {
                     try {
                         unit.reschedule();
                     } catch (CouldNotPerformException ex) {
-                        // if the reschedule is not possible because of a system shutdown everything is fine, otherwise it s s a controller error and there is no need to inform the remote about any error if the cancellation was successful.
+                        // if the reschedule is not possible because of a system shutdown everything is fine,
+                        // otherwise it is a controller error and there is no need to inform the remote about
+                        // any error if the cancellation was successful.
                         if (!ExceptionProcessor.isCausedBySystemShutdown(ex)) {
                             ExceptionPrinter.printHistory("Reschedule of " + unit + " failed after action cancellation!", ex, LOGGER);
                         }
@@ -739,9 +741,9 @@ private void cancelActionTask() {
         }
     }
 
-    private void updateActionStateIfNotCanceled(final ActionState.State state) throws InterruptedException {
+    private void updateActionStateIfNotTerminating(final ActionState.State state) throws InterruptedException {
         synchronized (actionTaskLock) {
-            if (actionTask.isCancelled() || getActionState() == State.CANCELING) {
+            if (isTerminating()) {
                 throw new InterruptedException();
             }
         }
@@ -763,7 +765,6 @@ private void updateActionStateWhileHoldingWriteLock(final ActionState.State stat
     private void updateActionState(final ActionState.State state) throws InterruptedException {
         actionDescriptionBuilderLock.lockWriteInterruptibly();
         try {
-
             // duplicated state confirmation should be ok to simplify the code, but than skip the update.
             if (getActionState() == state) {
                 return;
@@ -797,13 +798,6 @@ private void updateActionState(final ActionState.State state) throws Interrupted
 
                 case ABORTING:
                 case CANCELING:
-                    // mark action task already as canceled, to make sure the task is not
-                    // updating any further action states which would otherwise introduce invalid state transitions.
-                    synchronized (actionTaskLock) {
-                        if (!isActionTaskFinished()) {
-                            actionTask.cancel(false);
-                        }
-                    }
                     break;
             }
 

module/dal/lib/src/main/java/org/openbase/bco/dal/lib/action/Action.java

@@ -350,6 +350,19 @@ default boolean isDone() {
         };
     }
 
+    /**
+     * Check if this action is terminating, which means it was canceled
+     * or aborted but has not reached a terminating state yet.
+     *
+     * @return true if canceling or aborting.
+     */
+    default boolean isTerminating() {
+        return switch (getActionState()) {
+            case CANCELING, ABORTING -> true;
+            default -> false;
+        };
+    }
+
     /**
      * Return the current state of this action.
      *

module/registry/unit-registry/lib/src/main/java/org/openbase/bco/registry/unit/lib/filter/UnitConfigFilterImpl.java

@@ -104,7 +104,7 @@ private boolean propertyMatch(final UnitConfig unitConfig) {
         }
 
         // filter by username
-        if (properties.hasUserConfig() && unitConfig.hasUserConfig() && !(properties.getUserConfig().getUserName().equals(unitConfig.getUserConfig().getUserName()))) {
+        if (properties.hasUserConfig() && unitConfig.hasUserConfig() && !(properties.getUserConfig().getUserName().equalsIgnoreCase(unitConfig.getUserConfig().getUserName()))) {
             return false;
         }
 

module/registry/unit-registry/test/src/test/java/org/openbase/bco/registry/unit/test/UnitConfigFilterTest.kt

@@ -9,20 +9,70 @@ import org.openbase.type.domotic.unit.UnitFilterType.UnitFilter
 class UnitConfigFilterTest {
 
     @Test
-    fun `test filter for username`() {
+    fun `test filter for username with exact cases`() {
         val username1 = "Guybrush"
         val username2 = "LeChuck"
 
-        val userGuybrush = UnitConfig.newBuilder()
-        userGuybrush.userConfigBuilder.userName = username1
-        val userLeChuck = UnitConfig.newBuilder()
-        userLeChuck.userConfigBuilder.userName = username2
+        val userGuybrush = UnitConfig
+            .newBuilder()
+            .also { it.userConfigBuilder.userName = username1 }
+            .build()
 
-        val filterConfig = UnitFilter.newBuilder()
-        filterConfig.propertiesBuilder.userConfigBuilder.userName = username1
-        val guybrushFilter = UnitConfigFilterImpl(filterConfig.build())
+        val userLeChuck = UnitConfig
+            .newBuilder()
+            .also { it.userConfigBuilder.userName = username2 }
+            .build()
 
-        guybrushFilter.match(userGuybrush.build()) shouldBe true
-        guybrushFilter.match(userLeChuck.build()) shouldBe false
+        val guybrushFilter = UnitFilter
+            .newBuilder()
+            .also { it.propertiesBuilder.userConfigBuilder.userName = username1 }
+            .build()
+            .let {  UnitConfigFilterImpl(it) }
+
+        val lechuckFilter = UnitFilter
+            .newBuilder()
+            .also {it.propertiesBuilder.userConfigBuilder.userName = username1 }
+            .build()
+            .let { UnitConfigFilterImpl(it) }
+
+        guybrushFilter.match(userGuybrush) shouldBe true
+        guybrushFilter.match(userLeChuck) shouldBe false
+
+        lechuckFilter.match(userGuybrush) shouldBe true
+        lechuckFilter.match(userLeChuck) shouldBe false
+    }
+
+    @Test
+    fun `test filter for username with alternating cases`() {
+        val username1 = "Guybrush"
+        val username2 = "LeChuck"
+
+        val userGuybrush = UnitConfig
+            .newBuilder()
+            .also { it.userConfigBuilder.userName = username1.uppercase() }
+            .build()
+
+        val userLeChuck = UnitConfig
+            .newBuilder()
+            .also { it.userConfigBuilder.userName = username2.lowercase() }
+            .build()
+
+        val guybrushFilter = UnitFilter
+            .newBuilder()
+            .also { it.propertiesBuilder.userConfigBuilder.userName = username1 }
+            .build()
+            .let {  UnitConfigFilterImpl(it) }
+
+        val lechuckFilter = UnitFilter
+            .newBuilder()
+            .also {it.propertiesBuilder.userConfigBuilder.userName = username1 }
+            .build()
+            .let { UnitConfigFilterImpl(it) }
+
+        guybrushFilter.match(userGuybrush) shouldBe true
+        guybrushFilter.match(userLeChuck) shouldBe false
+
+        lechuckFilter.match(userGuybrush) shouldBe true
+        lechuckFilter.match(userLeChuck) shouldBe false
     }
 }