[systeminfo] Use AbstractStorageBasedTypeProvider (#14501)

* [systeminfo] Use AbstractDynamicTypeProvider

---------

Signed-off-by: Jan N. Klug <github@klug.nrw>

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<ThingTypeUID, ThingType> thingTypes = new HashMap<>();
-
     private final Map<ThingUID, Map<String, Configuration>> thingChannelsConfig = new HashMap<>();
 
     @Activate
     public SysteminfoThingTypeProvider(@Reference ThingTypeRegistry thingTypeRegistry,
             @Reference ChannelGroupTypeRegistry channelGroupTypeRegistry,
-            @Reference ChannelTypeRegistry channelTypeRegistry) {
+            @Reference ChannelTypeRegistry channelTypeRegistry, @Reference StorageService storageService) {
-        super();
+        super(storageService);
         this.thingTypeRegistry = thingTypeRegistry;
         this.channelGroupTypeRegistry = channelGroupTypeRegistry;
         this.channelTypeRegistry = channelTypeRegistry;
     }
 
-    @Override
-    public Collection<ThingType> 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<ChannelGroupDefinition> groupDefs) {
-        ThingTypeUID baseTypeUID = THING_TYPE_COMPUTER;
+        ThingType baseType = thingTypeRegistry.getThingType(typeUID);
-        if (thingTypes.containsKey(typeUID)) {
+        if (baseType == null) {
-            baseTypeUID = typeUID;
+            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, baseType.getUID());
-        ThingTypeBuilder builder = createThingTypeBuilder(typeUID, baseTypeUID);
+        if (builder != null) {
-        if (baseType != null && 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
      */

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();

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,
+            systeminfoHandlerFactory = getService(ThingHandlerFactory.class, SysteminfoHandlerFactory.class);
-                    SysteminfoThingTypeProvider.class);
+            assertThat(systeminfoHandlerFactory, is(notNullValue()));
-            assertThat(thingTypeProvider, is(notNullValue()));
-        });
-        waitForAssert(() -> {
-            SysteminfoThingTypeProvider systeminfoThingTypeProvider = getService(SysteminfoThingTypeProvider.class);
-            assertThat(systeminfoThingTypeProvider, 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,
+        String channelTypeId = channelUID.getIdWithoutGroup();
-                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()));