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 <github@klug.nrw>
openhab · Nov 6, 2022 · 2bceba6 · 2bceba6
1 parent 9cdaa48
commit 2bceba6
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 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
@@ -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,15 +455,15 @@ 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()) {
             ((GenericItem) item).setStateDescriptionService(stateDescriptionService);
         }
     }
 
-    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
@@ -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
@@ -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