demus/openhab-core
1
0
Fork 0
mirror of https://github.com/danieldemus/openhab-core.git synced 2026-07-29 12:34:22 +02:00
Code Issues 1 Packages Projects Releases Wiki Activity

Fix the recursive group membership check (#4088)

Browse Source 
Allow a group to be a member of its direct parent and also its parent's ancestors without raising an error.

Looping membership is still detected and prevented as before, thus Stack Overflow is still avoided.

Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
This commit is contained in:
jimtng
2024-02-12 17:21:55 +01:00
committed by GitHub
parent d806771364
commit 0efaf23d4e
4 changed files with 38 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
bundles/org.openhab.core.io.rest.core/src/main/java/org/openhab/core/io/rest/core/item/EnrichedItemDTOMapper.java
+2 -2
View File
@@ -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]);
bundles/org.openhab.core.io.rest.core/src/test/java/org/openhab/core/io/rest/core/item/EnrichedItemDTOMapperTest.java
+17 -2
View File
@@ -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);
}
}
bundles/org.openhab.core.semantics/src/main/java/org/openhab/core/semantics/internal/SemanticsMetadataProvider.java
+2 -2
View File
@@ -143,10 +143,10 @@ public class SemanticsMetadataProvider extends AbstractProvider<Metadata>
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));
}
}
}
bundles/org.openhab.core.semantics/src/test/java/org/openhab/core/semantics/internal/SemanticsMetadataProviderTest.java
+17 -2
View File
@@ -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())) //