diff --git a/bundles/org.openhab.core.io.rest.core/src/main/java/org/openhab/core/io/rest/core/item/EnrichedItemDTOMapper.java b/bundles/org.openhab.core.io.rest.core/src/main/java/org/openhab/core/io/rest/core/item/EnrichedItemDTOMapper.java index d4db73d4b..c8c1272cb 100644 --- a/bundles/org.openhab.core.io.rest.core/src/main/java/org/openhab/core/io/rest/core/item/EnrichedItemDTOMapper.java +++ b/bundles/org.openhab.core.io.rest.core/src/main/java/org/openhab/core/io/rest/core/item/EnrichedItemDTOMapper.java @@ -105,10 +105,10 @@ public class EnrichedItemDTOMapper { for (Item member : groupItem.getMembers()) { if (parents.contains(member)) { LOGGER.error( - "Recursive group membership found: {} is both, a direct or indirect parent and a child of {}.", + "Recursive group membership found: {} is a member of {}, but it is also one of its ancestors.", member.getName(), groupItem.getName()); } else if (itemFilter == null || itemFilter.test(member)) { - members.add(mapRecursive(member, itemFilter, uriBuilder, locale, parents)); + members.add(mapRecursive(member, itemFilter, uriBuilder, locale, new ArrayList<>(parents))); } } memberDTOs = members.toArray(new EnrichedItemDTO[0]); diff --git a/bundles/org.openhab.core.io.rest.core/src/test/java/org/openhab/core/io/rest/core/item/EnrichedItemDTOMapperTest.java b/bundles/org.openhab.core.io.rest.core/src/test/java/org/openhab/core/io/rest/core/item/EnrichedItemDTOMapperTest.java index a2ff09211..85af584aa 100644 --- a/bundles/org.openhab.core.io.rest.core/src/test/java/org/openhab/core/io/rest/core/item/EnrichedItemDTOMapperTest.java +++ b/bundles/org.openhab.core.io.rest.core/src/test/java/org/openhab/core/io/rest/core/item/EnrichedItemDTOMapperTest.java @@ -95,7 +95,7 @@ public class EnrichedItemDTOMapperTest extends JavaTest { assertDoesNotThrow(() -> EnrichedItemDTOMapper.map(groupItem1, true, null, null, null)); assertLogMessage(EnrichedItemDTOMapper.class, LogLevel.ERROR, - "Recursive group membership found: group1 is both, a direct or indirect parent and a child of group2."); + "Recursive group membership found: group1 is a member of group2, but it is also one of its ancestors."); } @Test @@ -111,7 +111,7 @@ public class EnrichedItemDTOMapperTest extends JavaTest { assertDoesNotThrow(() -> EnrichedItemDTOMapper.map(groupItem1, true, null, null, null)); assertLogMessage(EnrichedItemDTOMapper.class, LogLevel.ERROR, - "Recursive group membership found: group1 is both, a direct or indirect parent and a child of group3."); + "Recursive group membership found: group1 is a member of group3, but it is also one of its ancestors."); } @Test @@ -128,4 +128,19 @@ public class EnrichedItemDTOMapperTest extends JavaTest { assertNoLogMessage(EnrichedItemDTOMapper.class); } + + @Test + public void testDuplicateMembershipOfGroupItemsDoesNotTriggerWarning() { + GroupItem groupItem1 = new GroupItem("group1"); + GroupItem groupItem2 = new GroupItem("group2"); + GroupItem groupItem3 = new GroupItem("group3"); + + groupItem1.addMember(groupItem2); + groupItem1.addMember(groupItem3); + groupItem2.addMember(groupItem3); + + EnrichedItemDTOMapper.map(groupItem1, true, null, null, null); + + assertNoLogMessage(EnrichedItemDTOMapper.class); + } } diff --git a/bundles/org.openhab.core.semantics/src/main/java/org/openhab/core/semantics/internal/SemanticsMetadataProvider.java b/bundles/org.openhab.core.semantics/src/main/java/org/openhab/core/semantics/internal/SemanticsMetadataProvider.java index 7d59a64fa..662333ff1 100644 --- a/bundles/org.openhab.core.semantics/src/main/java/org/openhab/core/semantics/internal/SemanticsMetadataProvider.java +++ b/bundles/org.openhab.core.semantics/src/main/java/org/openhab/core/semantics/internal/SemanticsMetadataProvider.java @@ -143,10 +143,10 @@ public class SemanticsMetadataProvider extends AbstractProvider for (Item memberItem : groupItem.getMembers()) { if (parentItems.contains(memberItem.getName())) { logger.error( - "Recursive group membership found: {} is both, a direct or indirect parent and a child of {}.", + "Recursive group membership found: {} is a member of {}, but it is also one of its ancestors.", memberItem.getName(), groupItem.getName()); } else { - processItem(memberItem, parentItems); + processItem(memberItem, new ArrayList<>(parentItems)); } } } diff --git a/bundles/org.openhab.core.semantics/src/test/java/org/openhab/core/semantics/internal/SemanticsMetadataProviderTest.java b/bundles/org.openhab.core.semantics/src/test/java/org/openhab/core/semantics/internal/SemanticsMetadataProviderTest.java index e0c520dd9..684a791d3 100644 --- a/bundles/org.openhab.core.semantics/src/test/java/org/openhab/core/semantics/internal/SemanticsMetadataProviderTest.java +++ b/bundles/org.openhab.core.semantics/src/test/java/org/openhab/core/semantics/internal/SemanticsMetadataProviderTest.java @@ -250,7 +250,7 @@ public class SemanticsMetadataProviderTest extends JavaTest { assertDoesNotThrow(() -> semanticsMetadataProvider.added(groupItem1)); assertLogMessage(SemanticsMetadataProvider.class, LogLevel.ERROR, - "Recursive group membership found: group1 is both, a direct or indirect parent and a child of group2."); + "Recursive group membership found: group1 is a member of group2, but it is also one of its ancestors."); } @Test @@ -266,7 +266,7 @@ public class SemanticsMetadataProviderTest extends JavaTest { assertDoesNotThrow(() -> semanticsMetadataProvider.added(groupItem1)); assertLogMessage(SemanticsMetadataProvider.class, LogLevel.ERROR, - "Recursive group membership found: group1 is both, a direct or indirect parent and a child of group3."); + "Recursive group membership found: group1 is a member of group3, but it is also one of its ancestors."); } @Test @@ -284,6 +284,21 @@ public class SemanticsMetadataProviderTest extends JavaTest { assertNoLogMessage(SemanticsMetadataProvider.class); } + @Test + public void testDuplicateMembershipOfGroupItemsDoesNotTriggerWarning() { + GroupItem groupItem1 = new GroupItem("group1"); + GroupItem groupItem2 = new GroupItem("group2"); + GroupItem groupItem3 = new GroupItem("group3"); + + groupItem1.addMember(groupItem2); + groupItem1.addMember(groupItem3); + groupItem2.addMember(groupItem3); + + semanticsMetadataProvider.added(groupItem1); + + assertNoLogMessage(SemanticsMetadataProvider.class); + } + private @Nullable Metadata getMetadata(Item item) { return semanticsMetadataProvider.getAll().stream() // .filter(metadata -> metadata.getUID().getItemName().equals(item.getName())) //