From 8f471dac6b14fd61dcdf974b597c79d4140df1d3 Mon Sep 17 00:00:00 2001 From: J-N-K Date: Mon, 14 Mar 2022 15:49:26 +0100 Subject: [PATCH] Fix things in REMOVING state initialize instead of getting removed (#2828) Signed-off-by: Jan N. Klug GitOrigin-RevId: 437b31dbe93d39c8b129b1e10aaafb072d075904 --- .../core/thing/internal/ThingManagerImpl.java | 22 ++++++++++++--- .../thing/internal/ThingManagerOSGiTest.java | 27 +++++++++++++++++++ 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/bundles/org.opensmarthouse.core.thing/src/main/java/org/openhab/core/thing/internal/ThingManagerImpl.java b/bundles/org.opensmarthouse.core.thing/src/main/java/org/openhab/core/thing/internal/ThingManagerImpl.java index 8beb845b056..bfe6bd06a07 100644 --- a/bundles/org.opensmarthouse.core.thing/src/main/java/org/openhab/core/thing/internal/ThingManagerImpl.java +++ b/bundles/org.opensmarthouse.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/itests/org.opensmarthouse.core.thing.tests/src/main/java/org/openhab/core/thing/internal/ThingManagerOSGiTest.java b/itests/org.opensmarthouse.core.thing.tests/src/main/java/org/openhab/core/thing/internal/ThingManagerOSGiTest.java index ba430e6e0b2..c721207c895 100644 --- a/itests/org.opensmarthouse.core.thing.tests/src/main/java/org/openhab/core/thing/internal/ThingManagerOSGiTest.java +++ b/itests/org.opensmarthouse.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 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!";