From 2bceba69549c6017485e5089c64d6131889aa76a Mon Sep 17 00:00:00 2001 From: J-N-K Date: Sun, 6 Nov 2022 20:50:05 +0100 Subject: [PATCH] Fix missing stateDescriptionProvider in Group items (#3144) * Fix missing stateDescriptionProvider in Group items The stateDescriptionProvider was not properly passed to the base item. Because of that units set in the state description were not properly used when setting the group state after calculating aggregation functions. Signed-off-by: Jan N. Klug --- .../core/internal/items/ItemRegistryImpl.java | 8 ++--- .../org/openhab/core/items/GroupItem.java | 34 ++++++++++++++----- .../core/items/ItemRegistryImplTest.java | 29 ++++++++++++++++ 3 files changed, 59 insertions(+), 12 deletions(-) diff --git a/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/items/ItemRegistryImpl.java b/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/items/ItemRegistryImpl.java index 6159b544280..d2c1c97bcb6 100644 --- a/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/items/ItemRegistryImpl.java +++ b/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/items/ItemRegistryImpl.java @@ -293,14 +293,14 @@ protected void unsetReadyService(ReadyService readyService) { } @Reference(cardinality = ReferenceCardinality.OPTIONAL, policy = ReferencePolicy.DYNAMIC) - protected void setUnitProvider(UnitProvider unitProvider) { + public void setUnitProvider(UnitProvider unitProvider) { this.unitProvider = unitProvider; for (Item item : getItems()) { ((GenericItem) item).setUnitProvider(unitProvider); } } - protected void unsetUnitProvider(UnitProvider unitProvider) { + public void unsetUnitProvider(UnitProvider unitProvider) { this.unitProvider = null; for (Item item : getItems()) { ((GenericItem) item).setUnitProvider(null); @@ -455,7 +455,7 @@ protected void deactivate() { } @Reference(cardinality = ReferenceCardinality.OPTIONAL, policy = ReferencePolicy.DYNAMIC) - protected void setStateDescriptionService(StateDescriptionService stateDescriptionService) { + public void setStateDescriptionService(StateDescriptionService stateDescriptionService) { this.stateDescriptionService = stateDescriptionService; for (Item item : getItems()) { @@ -463,7 +463,7 @@ protected void setStateDescriptionService(StateDescriptionService stateDescripti } } - protected void unsetStateDescriptionService(StateDescriptionService stateDescriptionService) { + public void unsetStateDescriptionService(StateDescriptionService stateDescriptionService) { this.stateDescriptionService = null; for (Item item : getItems()) { diff --git a/bundles/org.openhab.core/src/main/java/org/openhab/core/items/GroupItem.java b/bundles/org.openhab.core/src/main/java/org/openhab/core/items/GroupItem.java index 1a8429c294e..a97ad952de5 100644 --- a/bundles/org.openhab.core/src/main/java/org/openhab/core/items/GroupItem.java +++ b/bundles/org.openhab.core/src/main/java/org/openhab/core/items/GroupItem.java @@ -27,6 +27,8 @@ import org.eclipse.jdt.annotation.Nullable; import org.openhab.core.i18n.UnitProvider; import org.openhab.core.items.events.ItemEventFactory; +import org.openhab.core.service.CommandDescriptionService; +import org.openhab.core.service.StateDescriptionService; import org.openhab.core.types.Command; import org.openhab.core.types.State; import org.slf4j.Logger; @@ -225,14 +227,6 @@ public void removeAllMembers() { members.clear(); } - @Override - public void setUnitProvider(@Nullable UnitProvider unitProvider) { - super.setUnitProvider(unitProvider); - if (baseItem != null && baseItem instanceof GenericItem) { - ((GenericItem) baseItem).setUnitProvider(unitProvider); - } - } - /** * The accepted data types of a group item is the same as of the underlying base item. * If none is defined, the intersection of all sets of accepted data types of all group @@ -395,6 +389,30 @@ public void setState(State state) { notifyListeners(oldState, state); } + @Override + public void setStateDescriptionService(@Nullable StateDescriptionService stateDescriptionService) { + super.setStateDescriptionService(stateDescriptionService); + if (baseItem instanceof GenericItem) { + ((GenericItem) baseItem).setStateDescriptionService(stateDescriptionService); + } + } + + @Override + public void setCommandDescriptionService(@Nullable CommandDescriptionService commandDescriptionService) { + super.setCommandDescriptionService(commandDescriptionService); + if (baseItem instanceof GenericItem) { + ((GenericItem) baseItem).setCommandDescriptionService(commandDescriptionService); + } + } + + @Override + public void setUnitProvider(@Nullable UnitProvider unitProvider) { + super.setUnitProvider(unitProvider); + if (baseItem instanceof GenericItem) { + ((GenericItem) baseItem).setUnitProvider(unitProvider); + } + } + private void sendGroupStateChangedEvent(String memberName, State newState, State oldState) { if (eventPublisher != null) { eventPublisher diff --git a/itests/org.openhab.core.tests/src/main/java/org/openhab/core/items/ItemRegistryImplTest.java b/itests/org.openhab.core.tests/src/main/java/org/openhab/core/items/ItemRegistryImplTest.java index a2ef3beb06e..942e1452589 100644 --- a/itests/org.openhab.core.tests/src/main/java/org/openhab/core/items/ItemRegistryImplTest.java +++ b/itests/org.openhab.core.tests/src/main/java/org/openhab/core/items/ItemRegistryImplTest.java @@ -379,15 +379,44 @@ public void assertOldItemIsBeingDisposedOnUpdate() { assertEquals(0, item.listeners.size()); } + @Test + public void assertStateDescriptionServiceGetsInjected() { + GenericItem item = spy(new SwitchItem("Item1")); + NumberItem baseItem = spy(new NumberItem("baseItem")); + GenericItem group = new GroupItem("Group", baseItem); + itemProvider.add(item); + itemProvider.add(group); + + verify(item).setStateDescriptionService(any(StateDescriptionService.class)); + verify(baseItem).setStateDescriptionService(any(StateDescriptionService.class)); + } + + @Test + public void assertUnitProviderGetsInjected() { + GenericItem item = spy(new SwitchItem("Item1")); + NumberItem baseItem = spy(new NumberItem("baseItem")); + GenericItem group = new GroupItem("Group", baseItem); + itemProvider.add(item); + itemProvider.add(group); + + verify(item).setUnitProvider(any(UnitProvider.class)); + verify(baseItem).setUnitProvider(any(UnitProvider.class)); + } + @Test public void assertCommandDescriptionServiceGetsInjected() { GenericItem item = spy(new SwitchItem("Item1")); + NumberItem baseItem = spy(new NumberItem("baseItem")); + GenericItem group = new GroupItem("Group", baseItem); itemProvider.add(item); + itemProvider.add(group); verify(item).setCommandDescriptionService(null); ((ItemRegistryImpl) itemRegistry).setCommandDescriptionService(mock(CommandDescriptionService.class)); verify(item).setCommandDescriptionService(any(CommandDescriptionService.class)); + + verify(baseItem).setCommandDescriptionService(any(CommandDescriptionService.class)); } @Test