Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix things in REMOVING state initialize instead of getting removed #2828

Merged
merged 2 commits into from
Mar 14, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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.",
Expand Down Expand Up @@ -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 device to remove the thing after it has been
J-N-K marked this conversation as resolved.
Show resolved Hide resolved
// 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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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!";
Expand Down