[suggestion-finder] various tweaks

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
openhab · Sep 21, 2023 · 63e022c · 63e022c
1 parent ff351a6
commit 63e022c
Show file tree

Hide file tree

Showing 8 changed files with 108 additions and 59 deletions.
diff --git a/.../finder/AddonSuggestionFinderService.java → ...y/addon/AddonSuggestionFinderService.java b/.../finder/AddonSuggestionFinderService.java → ...y/addon/AddonSuggestionFinderService.java
@@ -10,7 +10,7 @@
  *
  * SPDX-License-Identifier: EPL-2.0
  */
-package org.openhab.core.config.discovery.addon.finder;
+package org.openhab.core.config.discovery.addon;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -49,8 +49,6 @@
 import org.osgi.service.component.annotations.Reference;
 import org.osgi.service.component.annotations.ReferenceCardinality;
 import org.osgi.service.component.annotations.ReferencePolicy;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import com.thoughtworks.xstream.XStreamException;
 
@@ -89,7 +87,6 @@ public void serviceResolved(@Nullable ServiceEvent event) {
 
     private final AddonCandidatesSerializer addonCandidatesSerializer = new AddonCandidatesSerializer();
     private final Set<AddonService> addonServices = new CopyOnWriteArraySet<>();
-    private final Logger logger = LoggerFactory.getLogger(AddonSuggestionFinderService.class);
     private final List<MdnsCandidate> mdnsCandidates = new ArrayList<>();
     private final NoOp noop = new NoOp();
     private final ScheduledExecutorService scheduler = ThreadPoolManager.getScheduledPool(SERVICE_NAME);
@@ -235,13 +232,9 @@ private void startScanMdns() {
         if (task != null) {
             task.cancel(false);
         }
-        mdnsScanTask = scheduler.submit(() -> {
-            mdnsCandidates.forEach(c -> {
-                Arrays.stream(mdnsClient.list(c.getMdnsServiceType())).filter(s -> c.equals(s)).forEach(s -> {
-                    suggestionFound(ServiceType.MDNS, c.getAddonId());
-                });
-            });
-        });
+        mdnsScanTask = scheduler
+                .submit(() -> mdnsCandidates.forEach(c -> Arrays.stream(mdnsClient.list(c.getMdnsServiceType()))
+                        .filter(s -> c.equals(s)).forEach(s -> suggestionFound(ServiceType.MDNS, c.getAddonId()))));
     }
 
     /**
@@ -254,13 +247,8 @@ private void startScanUpnp() {
         if (task != null) {
             task.cancel(false);
         }
-        upnpScanTask = scheduler.submit(() -> {
-            upnpService.getRegistry().getRemoteDevices().forEach(d -> {
-                upnpCandidates.stream().filter(c -> c.equals(d)).forEach(c -> {
-                    suggestionFound(ServiceType.UPNP, c.getAddonId());
-                });
-            });
-        });
+        upnpScanTask = scheduler.submit(() -> upnpService.getRegistry().getRemoteDevices().forEach(d -> upnpCandidates
+                .stream().filter(c -> c.equals(d)).forEach(c -> suggestionFound(ServiceType.UPNP, c.getAddonId()))));
     }
 
     /**
@@ -270,7 +258,6 @@ private void startScanUpnp() {
      * @param addonUid the Uid of the found addon.
      */
     private synchronized void suggestionFound(ServiceType origin, String addonUid) {
-        logger.debug("Service {} found suggested addon id:{}", origin, addonUid);
         if (!suggestedAddonUids.contains(addonUid)) {
             suggestedAddonUids.add(addonUid);
         }

diff --git a/...org.openhab.core.config.discovery.addon/src/main/resources/suggested-addon-candidates.xml b/...org.openhab.core.config.discovery.addon/src/main/resources/suggested-addon-candidates.xml
@@ -9,11 +9,11 @@
 			<service-type>mdns</service-type>
 			<match-property>
 				<name>rp</name>
-				<regex>/.*\S.*/</regex>
+				<regex>.*</regex>
 			</match-property>
 			<match-property>
 				<name>ty</name>
-				<regex>^hp</regex>
+				<regex>hp (.*)</regex>
 			</match-property>
 			<mdns-service-type>_printer._tcp.local.</mdns-service-type>
 		</discovery-method>

diff --git a/....rest.core/src/main/java/org/openhab/core/io/rest/core/internal/addons/AddonResource.java b/....rest.core/src/main/java/org/openhab/core/io/rest/core/internal/addons/AddonResource.java
@@ -56,7 +56,7 @@
 import org.openhab.core.config.core.ConfigDescriptionRegistry;
 import org.openhab.core.config.core.ConfigUtil;
 import org.openhab.core.config.core.Configuration;
-import org.openhab.core.config.discovery.addon.finder.AddonSuggestionFinderService;
+import org.openhab.core.config.discovery.addon.AddonSuggestionFinderService;
 import org.openhab.core.events.Event;
 import org.openhab.core.events.EventPublisher;
 import org.openhab.core.io.rest.JSONResponse;

diff --git a/itests/org.openhab.core.config.discovery.addon.tests/.classpath b/itests/org.openhab.core.config.discovery.addon.tests/.classpath
@@ -0,0 +1,29 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<classpath>
+	<classpathentry kind="src" output="target/classes" path="src/main/java">
+		<attributes>
+			<attribute name="optional" value="true"/>
+			<attribute name="maven.pomderived" value="true"/>
+		</attributes>
+	</classpathentry>
+	<classpathentry kind="src" output="target/test-classes" path="src/test/java">
+		<attributes>
+			<attribute name="optional" value="true"/>
+			<attribute name="maven.pomderived" value="true"/>
+			<attribute name="test" value="true"/>
+		</attributes>
+	</classpathentry>
+	<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-17">
+		<attributes>
+			<attribute name="maven.pomderived" value="true"/>
+			<attribute name="annotationpath" value="target/dependency"/>
+		</attributes>
+	</classpathentry>
+	<classpathentry kind="con" path="org.eclipse.m2e.MAVEN2_CLASSPATH_CONTAINER">
+		<attributes>
+			<attribute name="maven.pomderived" value="true"/>
+			<attribute name="annotationpath" value="target/dependency"/>
+		</attributes>
+	</classpathentry>
+	<classpathentry kind="output" path="target/classes"/>
+</classpath>
diff --git a/itests/org.openhab.core.config.discovery.addon.tests/.project b/itests/org.openhab.core.config.discovery.addon.tests/.project
@@ -0,0 +1,23 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<projectDescription>
+	<name>org.openhab.core.config.discovery.addon.tests</name>
+	<comment></comment>
+	<projects>
+	</projects>
+	<buildSpec>
+		<buildCommand>
+			<name>org.eclipse.jdt.core.javabuilder</name>
+			<arguments>
+			</arguments>
+		</buildCommand>
+		<buildCommand>
+			<name>org.eclipse.m2e.core.maven2Builder</name>
+			<arguments>
+			</arguments>
+		</buildCommand>
+	</buildSpec>
+	<natures>
+		<nature>org.eclipse.jdt.core.javanature</nature>
+		<nature>org.eclipse.m2e.core.maven2Nature</nature>
+	</natures>
+</projectDescription>
diff --git a/itests/org.openhab.core.config.discovery.addon.tests/itest.bndrun b/itests/org.openhab.core.config.discovery.addon.tests/itest.bndrun
@@ -1,9 +1,9 @@
 -include: ../itest-common.bndrun
 
 Bundle-SymbolicName: ${project.artifactId}
-Fragment-Host: org.openhab.core.config.discovery.mdns
+Fragment-Host: org.openhab.core.config.discovery.addon
 
--runrequires: bnd.identity;id='org.openhab.core.config.discovery.mdns.tests'
+-runrequires: bnd.identity;id='org.openhab.core.config.discovery.addon.tests'
 
 #
 # done
@@ -55,8 +55,8 @@ Fragment-Host: org.openhab.core.config.discovery.mdns
 	org.openhab.core;version='[4.1.0,4.1.1)',\
 	org.openhab.core.config.core;version='[4.1.0,4.1.1)',\
 	org.openhab.core.config.discovery;version='[4.1.0,4.1.1)',\
-	org.openhab.core.config.discovery.mdns;version='[4.1.0,4.1.1)',\
-	org.openhab.core.config.discovery.mdns.tests;version='[4.1.0,4.1.1)',\
+	org.openhab.core.config.discovery.addon;version='[4.1.0,4.1.1)',\
+	org.openhab.core.config.discovery.addon.tests;version='[4.1.0,4.1.1)',\
 	org.openhab.core.io.console;version='[4.1.0,4.1.1)',\
 	org.openhab.core.io.transport.mdns;version='[4.1.0,4.1.1)',\
 	org.openhab.core.test;version='[4.1.0,4.1.1)',\

diff --git a/itests/org.openhab.core.config.discovery.addon.tests/pom.xml b/itests/org.openhab.core.config.discovery.addon.tests/pom.xml
@@ -9,7 +9,7 @@
     <version>4.1.0-SNAPSHOT</version>
   </parent>
 
-  <artifactId>org.openhab.core.config.discovery.addons.tests</artifactId>
+  <artifactId>org.openhab.core.config.discovery.addon.tests</artifactId>
 
   <name>openHAB Core :: Integration Tests :: Suggested Addon Finder Tests</name>
 

diff --git a/.../SuggestedAddonFinderServiceOSGiTest.java → .../SuggestedAddonFinderServiceOSGiTest.java b/.../SuggestedAddonFinderServiceOSGiTest.java → .../SuggestedAddonFinderServiceOSGiTest.java
@@ -10,12 +10,16 @@
  *
  * SPDX-License-Identifier: EPL-2.0
  */
-package org.openhab.core.config.discovery.addon.tests.internal;
+package org.openhab.core.config.discovery.addon.tests;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -51,7 +55,7 @@
 import org.mockito.Mockito;
 import org.openhab.core.addon.Addon;
 import org.openhab.core.addon.AddonService;
-import org.openhab.core.config.discovery.addon.finder.AddonSuggestionFinderService;
+import org.openhab.core.config.discovery.addon.AddonSuggestionFinderService;
 import org.openhab.core.io.transport.mdns.MDNSClient;
 import org.openhab.core.test.java.JavaOSGiTest;
 
@@ -73,35 +77,52 @@ public class SuggestedAddonFinderServiceOSGiTest extends JavaOSGiTest {
 
     @BeforeAll
     public void setup() {
-        setupAddonServiceMock();
-        setupMdnsClientMock();
-        setupUpnpServiceMock();
-        setupService();
+        setupMockAddonService();
+        setupMockMdnsClient();
+        setupMockUpnpService();
+        createAddonSuggestionFinderService();
     }
 
-    private void setupAddonServiceMock() {
+    private void createAddonSuggestionFinderService() {
+        // create the service
+        try {
+            addonSuggestionFinderService = new AddonSuggestionFinderService(mdnsClient, upnpService);
+        } catch (IOException e) {
+            fail("Error loading XML");
+        } catch (XStreamException e) {
+            fail("Error parsing XML");
+        }
+        addonSuggestionFinderService.addAddonService(addonService);
+
+        // check that it exists
+        assertNotNull(addonSuggestionFinderService);
+    }
+
+    private void setupMockAddonService() {
         // create the mock
         addonService = mock(AddonService.class);
         List<Addon> addons = new ArrayList<>();
         addons.add(Addon.create("binding.hue").withType("binding").withId("hue").build());
         addons.add(Addon.create("binding.hpprinter").withType("binding").withId("hpprinter").build());
-        when(addonService.getAddons(Locale.US)).thenReturn(addons);
+        when(addonService.getAddons(any(Locale.class))).thenReturn(addons);
 
         // check that it works
         assertNotNull(addonService);
         assertEquals(2, addonService.getAddons(Locale.US).size());
-        assertEquals("binding.hue", addonService.getAddons(Locale.US).get(0).getUid());
-        assertEquals("binding.hpprinter", addonService.getAddons(Locale.US).get(1).getUid());
+        assertTrue(addonService.getAddons(Locale.US).stream().anyMatch(a -> "binding.hue".equals(a.getUid())));
+        assertTrue(addonService.getAddons(Locale.US).stream().anyMatch(a -> "binding.hpprinter".equals(a.getUid())));
+        assertFalse(addonService.getAddons(Locale.US).stream().anyMatch(a -> "aardvark".equals(a.getUid())));
     }
 
-    private void setupMdnsClientMock() {
+    private void setupMockMdnsClient() {
         // create the mock
         mdnsClient = mock(MDNSClient.class);
-        ServiceInfo hueService = ServiceInfo.create("test", "hue", 0, 0, 0, false, "hue service");
-        when(mdnsClient.list("_hue._tcp.local.")).thenReturn(new ServiceInfo[] { hueService });
-        ServiceInfo hpService = ServiceInfo.create("test", "hpprinter", 0, 0, 0, false, "hp printer service");
+        when(mdnsClient.list(anyString())).thenReturn(new ServiceInfo[] {});
+        ServiceInfo hueService = ServiceInfo.create("mdnsTest", "hue", 0, 0, 0, false, "hue service");
+        when(mdnsClient.list(eq("_hue._tcp.local."))).thenReturn(new ServiceInfo[] { hueService });
+        ServiceInfo hpService = ServiceInfo.create("mdnsTest", "hpprinter", 0, 0, 0, false, "hp printer service");
         hpService.setText(Map.of("ty", "hp printer", "rp", "anything"));
-        when(mdnsClient.list("_printer._tcp.local.")).thenReturn(new ServiceInfo[] { hpService });
+        when(mdnsClient.list(eq("_printer._tcp.local."))).thenReturn(new ServiceInfo[] { hpService });
 
         // check that it works
         assertNotNull(mdnsClient);
@@ -114,24 +135,11 @@ private void setupMdnsClientMock() {
         result = mdnsClient.list("_hue._tcp.local.");
         assertEquals(1, result.length);
         assertEquals("hue", result[0].getName());
+        result = mdnsClient.list("aardvark");
+        assertEquals(0, result.length);
     }
 
-    private void setupService() {
-        // create the service
-        try {
-            addonSuggestionFinderService = new AddonSuggestionFinderService(mdnsClient, upnpService);
-        } catch (IOException e) {
-            fail("Error loading XML");
-        } catch (XStreamException e) {
-            fail("Error parsing XML");
-        }
-        addonSuggestionFinderService.addAddonService(addonService);
-
-        // check that it exists
-        assertNotNull(addonSuggestionFinderService);
-    }
-
-    private void setupUpnpServiceMock() {
+    private void setupMockUpnpService() {
         // create the mock
         upnpService = mock(UpnpService.class, Mockito.RETURNS_DEEP_STUBS);
         URL url = null;
@@ -150,7 +158,7 @@ private void setupUpnpServiceMock() {
         RemoteDeviceIdentity identity = new RemoteDeviceIdentity(udn, 0, url, new byte[] {}, address);
         DeviceType type = new DeviceType("nameSpace", "type");
         ManufacturerDetails manDetails = new ManufacturerDetails("manufacturer", "manufacturerURI");
-        ModelDetails modDetails = new ModelDetails("modelName", "modelDescription", "modelNumber", "modelURI");
+        ModelDetails modDetails = new ModelDetails("Philips hue bridge", "modelDescription", "modelNumber", "modelURI");
         DeviceDetails devDetails = new DeviceDetails("friendlyName", manDetails, modDetails, "serialNumber",
                 "000123456789");
         List<@Nullable RemoteDevice> remoteDevice = new ArrayList<>();
@@ -180,5 +188,7 @@ public void testGetAddons() {
         assertTrue(addonSuggestionFinderService.scanDone());
         List<Addon> addons = addonSuggestionFinderService.getAddons(Locale.US);
         assertEquals(2, addons.size());
+        assertTrue(addons.stream().anyMatch(a -> "binding.hue".equals(a.getUid())));
+        assertTrue(addons.stream().anyMatch(a -> "binding.hpprinter".equals(a.getUid())));
     }
 }