From 16df575b38f847f0a8fc459e35921d26e8a57058 Mon Sep 17 00:00:00 2001 From: J-N-K Date: Fri, 12 May 2023 13:59:01 +0200 Subject: [PATCH] [systeminfo] Use AbstractStorageBasedTypeProvider (#14501) * [systeminfo] Use AbstractDynamicTypeProvider --------- Signed-off-by: Jan N. Klug --- .../internal/SysteminfoThingTypeProvider.java | 48 +++++++------------ .../internal/handler/SysteminfoHandler.java | 6 +++ .../systeminfo/test/SysteminfoOSGiTest.java | 44 ++++++----------- 3 files changed, 37 insertions(+), 61 deletions(-) diff --git a/bundles/org.openhab.binding.systeminfo/src/main/java/org/openhab/binding/systeminfo/internal/SysteminfoThingTypeProvider.java b/bundles/org.openhab.binding.systeminfo/src/main/java/org/openhab/binding/systeminfo/internal/SysteminfoThingTypeProvider.java index 103eed41f..03cd6d20a 100644 --- a/bundles/org.openhab.binding.systeminfo/src/main/java/org/openhab/binding/systeminfo/internal/SysteminfoThingTypeProvider.java +++ b/bundles/org.openhab.binding.systeminfo/src/main/java/org/openhab/binding/systeminfo/internal/SysteminfoThingTypeProvider.java @@ -15,23 +15,22 @@ package org.openhab.binding.systeminfo.internal; import static org.openhab.binding.systeminfo.internal.SysteminfoBindingConstants.*; import java.net.URI; -import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.List; -import java.util.Locale; import java.util.Map; import java.util.stream.Collectors; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; import org.openhab.core.config.core.Configuration; +import org.openhab.core.storage.StorageService; import org.openhab.core.thing.Channel; import org.openhab.core.thing.ChannelUID; import org.openhab.core.thing.Thing; import org.openhab.core.thing.ThingTypeUID; import org.openhab.core.thing.ThingUID; -import org.openhab.core.thing.binding.BaseThingHandler; +import org.openhab.core.thing.binding.AbstractStorageBasedTypeProvider; import org.openhab.core.thing.binding.ThingTypeProvider; import org.openhab.core.thing.binding.builder.ChannelBuilder; import org.openhab.core.thing.type.ChannelGroupDefinition; @@ -51,7 +50,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * Extended channels can be auto discovered and added to newly created groups in the {@link SystemInfoHandler}. The + * Extended channels can be auto discovered and added to newly created groups in the {@link SysteminfoHandler}. The * thing needs to be updated to add the groups. The `SysteminfoThingTypeProvider` OSGi service gives access to the * `ThingTypeRegistry` and serves the updated `ThingType`. * @@ -60,41 +59,25 @@ import org.slf4j.LoggerFactory; */ @NonNullByDefault @Component(service = { SysteminfoThingTypeProvider.class, ThingTypeProvider.class }) -public class SysteminfoThingTypeProvider implements ThingTypeProvider { +public class SysteminfoThingTypeProvider extends AbstractStorageBasedTypeProvider { private final Logger logger = LoggerFactory.getLogger(SysteminfoThingTypeProvider.class); private final ThingTypeRegistry thingTypeRegistry; private final ChannelGroupTypeRegistry channelGroupTypeRegistry; private final ChannelTypeRegistry channelTypeRegistry; - private final Map thingTypes = new HashMap<>(); - private final Map> thingChannelsConfig = new HashMap<>(); @Activate public SysteminfoThingTypeProvider(@Reference ThingTypeRegistry thingTypeRegistry, @Reference ChannelGroupTypeRegistry channelGroupTypeRegistry, - @Reference ChannelTypeRegistry channelTypeRegistry) { - super(); + @Reference ChannelTypeRegistry channelTypeRegistry, @Reference StorageService storageService) { + super(storageService); this.thingTypeRegistry = thingTypeRegistry; this.channelGroupTypeRegistry = channelGroupTypeRegistry; this.channelTypeRegistry = channelTypeRegistry; } - @Override - public Collection getThingTypes(@Nullable Locale locale) { - return thingTypes.values(); - } - - @Override - public @Nullable ThingType getThingType(ThingTypeUID thingTypeUID, @Nullable Locale locale) { - return thingTypes.get(thingTypeUID); - } - - private void setThingType(ThingTypeUID uid, ThingType type) { - thingTypes.put(uid, type); - } - /** * Create thing type with the provided typeUID and add it to the thing type registry. * @@ -114,17 +97,20 @@ public class SysteminfoThingTypeProvider implements ThingTypeProvider { * @return false if `typeUID` or its base type UID `systeminfo:computer` cannot be found in the thingTypeRegistry */ public boolean updateThingType(ThingTypeUID typeUID, List groupDefs) { - ThingTypeUID baseTypeUID = THING_TYPE_COMPUTER; - if (thingTypes.containsKey(typeUID)) { - baseTypeUID = typeUID; + ThingType baseType = thingTypeRegistry.getThingType(typeUID); + if (baseType == null) { + baseType = thingTypeRegistry.getThingType(THING_TYPE_COMPUTER); + if (baseType == null) { + logger.warn("Could not find base thing type in registry."); + return false; + } } - ThingType baseType = thingTypeRegistry.getThingType(baseTypeUID); - ThingTypeBuilder builder = createThingTypeBuilder(typeUID, baseTypeUID); - if (baseType != null && builder != null) { + ThingTypeBuilder builder = createThingTypeBuilder(typeUID, baseType.getUID()); + if (builder != null) { logger.trace("Adding channel group definitions to thing type"); ThingType type = builder.withChannelGroupDefinitions(groupDefs).build(); - setThingType(typeUID, type); + putThingType(type); return true; } else { logger.debug("Error adding channel groups"); @@ -253,7 +239,7 @@ public class SysteminfoThingTypeProvider implements ThingTypeProvider { /** * Store the channel configurations for a thing, to be able to restore them later when the thing handler for the * same thing gets recreated with a new thing type. This is necessary because the - * {@link BaseThingHandler#changeThingType()} method reverts channel configurations to their defaults. + * {@link BaseThingHandler##changeThingType()} method reverts channel configurations to their defaults. * * @param thing */ diff --git a/bundles/org.openhab.binding.systeminfo/src/main/java/org/openhab/binding/systeminfo/internal/handler/SysteminfoHandler.java b/bundles/org.openhab.binding.systeminfo/src/main/java/org/openhab/binding/systeminfo/internal/handler/SysteminfoHandler.java index 5dced54d9..36ba69e20 100644 --- a/bundles/org.openhab.binding.systeminfo/src/main/java/org/openhab/binding/systeminfo/internal/handler/SysteminfoHandler.java +++ b/bundles/org.openhab.binding.systeminfo/src/main/java/org/openhab/binding/systeminfo/internal/handler/SysteminfoHandler.java @@ -159,6 +159,12 @@ public class SysteminfoHandler extends BaseThingHandler { } } + @Override + public void handleRemoval() { + thingTypeProvider.removeThingType(thing.getThingTypeUID()); + super.handleRemoval(); + } + private boolean instantiateSysteminfoLibrary() { try { systeminfo.initializeSysteminfo(); diff --git a/itests/org.openhab.binding.systeminfo.tests/src/main/java/org/openhab/binding/systeminfo/test/SysteminfoOSGiTest.java b/itests/org.openhab.binding.systeminfo.tests/src/main/java/org/openhab/binding/systeminfo/test/SysteminfoOSGiTest.java index 0d4720245..2a8f35967 100644 --- a/itests/org.openhab.binding.systeminfo.tests/src/main/java/org/openhab/binding/systeminfo/test/SysteminfoOSGiTest.java +++ b/itests/org.openhab.binding.systeminfo.tests/src/main/java/org/openhab/binding/systeminfo/test/SysteminfoOSGiTest.java @@ -23,7 +23,6 @@ import java.math.BigDecimal; import java.net.UnknownHostException; import java.util.Hashtable; import java.util.List; -import java.util.Locale; import javax.measure.quantity.ElectricPotential; import javax.measure.quantity.Temperature; @@ -35,7 +34,6 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.ArgumentMatchers; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.mockito.junit.jupiter.MockitoSettings; @@ -85,10 +83,7 @@ import org.openhab.core.thing.binding.builder.ThingBuilder; import org.openhab.core.thing.link.ItemChannelLink; import org.openhab.core.thing.link.ManagedItemChannelLinkProvider; import org.openhab.core.thing.type.ChannelKind; -import org.openhab.core.thing.type.ChannelType; -import org.openhab.core.thing.type.ChannelTypeProvider; import org.openhab.core.thing.type.ChannelTypeUID; -import org.openhab.core.thing.type.ThingType; import org.openhab.core.types.State; import org.openhab.core.types.UnDefType; @@ -129,6 +124,7 @@ public class SysteminfoOSGiTest extends JavaOSGiTest { private @NonNullByDefault({}) SysteminfoHandlerFactory systeminfoHandlerFactory; private @NonNullByDefault({}) ThingRegistry thingRegistry; private @NonNullByDefault({}) ItemRegistry itemRegistry; + private @NonNullByDefault({}) SysteminfoThingTypeProvider systemThingTypeProvider; @BeforeEach public void setUp() { @@ -152,10 +148,16 @@ public class SysteminfoOSGiTest extends JavaOSGiTest { registerService(mockedSystemInfo); + waitForAssert(() -> { + systemThingTypeProvider = getService(ThingTypeProvider.class, SysteminfoThingTypeProvider.class); + assertThat(systemThingTypeProvider, is(notNullValue())); + }); + waitForAssert(() -> { systeminfoHandlerFactory = getService(ThingHandlerFactory.class, SysteminfoHandlerFactory.class); assertThat(systeminfoHandlerFactory, is(notNullValue())); }); + if (systeminfoHandlerFactory != null) { // Unbind oshiSystemInfo service and bind the mock service to make the systeminfo binding tests independent // of the external OSHI library @@ -167,13 +169,8 @@ public class SysteminfoOSGiTest extends JavaOSGiTest { } waitForAssert(() -> { - ThingTypeProvider thingTypeProvider = getService(ThingTypeProvider.class, - SysteminfoThingTypeProvider.class); - assertThat(thingTypeProvider, is(notNullValue())); - }); - waitForAssert(() -> { - SysteminfoThingTypeProvider systeminfoThingTypeProvider = getService(SysteminfoThingTypeProvider.class); - assertThat(systeminfoThingTypeProvider, is(notNullValue())); + systeminfoHandlerFactory = getService(ThingHandlerFactory.class, SysteminfoHandlerFactory.class); + assertThat(systeminfoHandlerFactory, is(notNullValue())); }); waitForAssert(() -> { @@ -256,8 +253,11 @@ public class SysteminfoOSGiTest extends JavaOSGiTest { ThingUID thingUID = new ThingUID(thingTypeUID, DEFAULT_TEST_THING_NAME); ChannelUID channelUID = new ChannelUID(thingUID, channelID); - ChannelTypeUID channelTypeUID = new ChannelTypeUID(SysteminfoBindingConstants.BINDING_ID, - channelUID.getIdWithoutGroup()); + String channelTypeId = channelUID.getIdWithoutGroup(); + if ("load1".equals(channelTypeId) || "load5".equals(channelTypeId) || "load15".equals(channelTypeId)) { + channelTypeId = "loadAverage"; + } + ChannelTypeUID channelTypeUID = new ChannelTypeUID(SysteminfoBindingConstants.BINDING_ID, channelTypeId); Configuration channelConfig = new Configuration(); channelConfig.put("priority", priority); channelConfig.put("pid", new BigDecimal(pid)); @@ -268,22 +268,6 @@ public class SysteminfoOSGiTest extends JavaOSGiTest { .withChannel(channel).build(); systemInfoThing = thing; - // TODO: This is a technically not correct work-around as the thing types are currently not made available by - // the binding. It should be properly fixes in the binding that thing-types are added to the registry. The - // "correct" solution here would be to wait until the thing manager initializes the thing with a missing thing - // type, but that would make each test take 120+ s - ThingTypeProvider thingTypeProviderMock = mock(ThingTypeProvider.class); - when(thingTypeProviderMock.getThingType(ArgumentMatchers.any(ThingTypeUID.class), nullable(Locale.class))) - .thenReturn(mock(ThingType.class)); - registerService(thingTypeProviderMock); - - ChannelType channelTypeMock = mock(ChannelType.class); - when(channelTypeMock.getKind()).thenReturn(ChannelKind.STATE); - ChannelTypeProvider channelTypeProviderMock = mock(ChannelTypeProvider.class); - when(channelTypeProviderMock.getChannelType(ArgumentMatchers.any(ChannelTypeUID.class), nullable(Locale.class))) - .thenReturn(channelTypeMock); - registerService(channelTypeProviderMock); - ManagedThingProvider managedThingProvider = getService(ThingProvider.class, ManagedThingProvider.class); assertThat(managedThingProvider, is(notNullValue()));