Fix things in REMOVING state initialize instead of getting removed (o…

…penhab#2828) Signed-off-by: Jan N. Klug <github@klug.nrw> GitOrigin-RevId: 437b31d
ConnectorIO · Jul 12, 2023 · 8f471da · 8f471da
1 parent baf72d0
commit 8f471da
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 4 deletions.
diff --git a/...smarthouse.core.thing/src/main/java/org/openhab/core/thing/internal/ThingManagerImpl.java b/...smarthouse.core.thing/src/main/java/org/openhab/core/thing/internal/ThingManagerImpl.java
@@ -105,8 +105,9 @@
  * {@link ThingManagerImpl} tracks all things in the {@link ThingRegistry} and mediates the communication between the
  * {@link Thing} and the {@link ThingHandler} from the binding. Therefore it tracks {@link ThingHandlerFactory}s and
  * calls {@link ThingHandlerFactory#registerHandler(Thing)} for each thing, that was added to the {@link ThingRegistry}.
- * In addition the {@link ThingManagerImpl} acts as an {@link EventHandler} and subscribes to update and command events.
- * Finally the {@link ThingManagerImpl} implement the {@link ThingTypeMigrationService} to offer a way to change the
+ * In addition the {@link ThingManagerImpl} acts as an {@link org.openhab.core.internal.events.EventHandler}
+ * and subscribes to update and command events.
+ * Finally the {@link ThingManagerImpl} implements the {@link ThingTypeMigrationService} to offer a way to change the
  * thing type of a {@link Thing}.
  *
  * @author Dennis Nobel - Initial contribution
@@ -115,7 +116,7 @@
  *         refactorings due to thing/bridge life cycle
  * @author Simon Kaufmann - Added remove handling, type conversion
  * @author Kai Kreuzer - Removed usage of itemRegistry and thingLinkRegistry, fixed vetoing mechanism
- * @author Andre Fuechsel - Added the {@link ThingTypeMigrationService} 
+ * @author Andre Fuechsel - Added the {@link ThingTypeMigrationService}
  * @author Thomas Höfer - Added localization of thing status info
  * @author Christoph Weitkamp - Moved OSGI ServiceTracker from BaseThingHandler to ThingHandlerCallback
  * @author Henning Sudbrock - Consider thing type properties when migrating to new thing type
@@ -193,6 +194,12 @@ public void statusUpdated(Thing thing, ThingStatusInfo statusInfo) {
 
             if (ThingStatus.REMOVING.equals(oldStatusInfo.getStatus())
                     && !ThingStatus.REMOVED.equals(statusInfo.getStatus())) {
+                // if we go to ONLINE and are still in REMOVING, notify handler about required removal
+                if (ThingStatus.ONLINE.equals(statusInfo.getStatus())) {
+                    logger.debug(
+                            "Handler is initialized now and we try to remove it, because it is in REMOVING state.");
+                    notifyThingHandlerAboutRemoval(thing);
+                }
                 // only allow REMOVING -> REMOVED transition, all others are ignored because they are illegal
                 logger.debug(
                         "Ignoring illegal status transition for thing {} from REMOVING to {}, only REMOVED would have been allowed.",
@@ -795,7 +802,14 @@ private void initializeHandler(Thing thing) {
             }
 
             if (isInitializable(thing, thingType)) {
-                setThingStatus(thing, buildStatusInfo(ThingStatus.INITIALIZING, ThingStatusDetail.NONE));
+                if (ThingStatus.REMOVING.equals(thing.getStatus())) {
+                    // preserve REMOVING state so the callback can later decide to remove the thing after it has been
+                    // initialized
+                    logger.debug("Not setting status to INITIALIZING because thing '{}' is in REMOVING status.",
+                            thing.getUID());
+                } else {
+                    setThingStatus(thing, buildStatusInfo(ThingStatus.INITIALIZING, ThingStatusDetail.NONE));
+                }
                 doInitializeHandler(handler);
             } else {
                 logger.debug(

diff --git a/....core.thing.tests/src/main/java/org/openhab/core/thing/internal/ThingManagerOSGiTest.java b/....core.thing.tests/src/main/java/org/openhab/core/thing/internal/ThingManagerOSGiTest.java
@@ -1020,6 +1020,9 @@ class ThingHandlerState {
         state.callback.statusUpdated(thing, onlineNone);
 
         assertThat(thing.getStatusInfo(), is(removingNone));
+
+        // handleRemoval is called when thing is fully initialized and shall be removed
+        verify(thingHandler).handleRemoval();
     }
 
     private void expectException(Runnable runnable, Class<? extends Exception> exceptionType) {
@@ -1063,6 +1066,30 @@ public void thingManagerHandlesThingStatusUpdatesUninitializedAndInitializingCor
         waitForAssert(() -> assertThat(thing.getStatusInfo(), is(uninitializedHandlerMissing)));
     }
 
+    @Test
+    public void thingManagerHandlesThingStatusUpdatesRemovingAndInitializingCorrectly() {
+        registerThingTypeProvider();
+
+        ThingHandler thingHandler = mock(ThingHandler.class);
+        when(thingHandler.getThing()).thenReturn(thing);
+
+        ThingHandlerFactory thingHandlerFactory = mock(ThingHandlerFactory.class);
+        when(thingHandlerFactory.supportsThingType(any(ThingTypeUID.class))).thenReturn(true);
+        when(thingHandlerFactory.registerHandler(any(Thing.class))).thenReturn(thingHandler);
+
+        registerService(thingHandlerFactory);
+
+        final ThingStatusInfo removingNone = ThingStatusInfoBuilder.create(ThingStatus.REMOVING, ThingStatusDetail.NONE)
+                .build();
+        thing.setStatusInfo(removingNone);
+
+        managedThingProvider.add(thing);
+
+        // verify thing handler initialize is called but thing state stays in REMOVING
+        verify(thingHandler).initialize();
+        assertThat(thing.getStatusInfo(), is(removingNone));
+    }
+
     @Test
     public void thingManagerHandlesThingStatusUpdateUninitializedWithAnExceptionCorrectly() {
         String exceptionMessage = "Some runtime exception occurred!";