From 300cd4ef8e7884d2b81191278b9ad0d2e64e477a Mon Sep 17 00:00:00 2001 From: Mark Herwege Date: Thu, 14 Dec 2023 09:27:22 +0100 Subject: [PATCH 1/6] change discovery method schema Signed-off-by: Mark Herwege --- .../schema/addon-1.0.0.xsd | 15 ++++- .../core/addon/AddonDiscoveryMethod.java | 16 ++--- .../openhab/core/addon/AddonParameter.java | 62 +++++++++++++++++++ .../xml/AddonDiscoveryMethodConverter.java | 11 ++-- .../internal/xml/AddonParameterConverter.java | 53 ++++++++++++++++ .../core/addon/AddonInfoListReaderTest.java | 24 ++++--- .../addon/AddonInfoRegistryMergeTest.java | 15 ++--- .../discovery/addon/mdns/MDNSAddonFinder.java | 29 +++++---- .../mdns/tests/MDNSAddonFinderTests.java | 5 +- .../tests/AddonSuggestionServiceTests.java | 5 +- 10 files changed, 189 insertions(+), 46 deletions(-) create mode 100644 bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/AddonParameter.java create mode 100644 bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/xml/AddonParameterConverter.java diff --git a/bundles/org.openhab.core.addon/schema/addon-1.0.0.xsd b/bundles/org.openhab.core.addon/schema/addon-1.0.0.xsd index c32e01a70e4..38587f19782 100644 --- a/bundles/org.openhab.core.addon/schema/addon-1.0.0.xsd +++ b/bundles/org.openhab.core.addon/schema/addon-1.0.0.xsd @@ -90,11 +90,24 @@ - + + + + + + + + + + + + + + diff --git a/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/AddonDiscoveryMethod.java b/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/AddonDiscoveryMethod.java index 88eb8669819..e9f0d9d8f0c 100644 --- a/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/AddonDiscoveryMethod.java +++ b/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/AddonDiscoveryMethod.java @@ -26,16 +26,16 @@ @NonNullByDefault public class AddonDiscoveryMethod { private @NonNullByDefault({}) String serviceType; - private @Nullable String mdnsServiceType; + private @Nullable List parameters; private @Nullable List matchProperties; public String getServiceType() { return serviceType.toLowerCase(); } - public String getMdnsServiceType() { - String mdnsServiceType = this.mdnsServiceType; - return mdnsServiceType != null ? mdnsServiceType : ""; + public List getParameters() { + List parameters = this.parameters; + return parameters != null ? parameters : List.of(); } public List getMatchProperties() { @@ -48,8 +48,8 @@ public AddonDiscoveryMethod setServiceType(String serviceType) { return this; } - public AddonDiscoveryMethod setMdnsServiceType(@Nullable String mdnsServiceType) { - this.mdnsServiceType = mdnsServiceType; + public AddonDiscoveryMethod setParameters(@Nullable List parameters) { + this.parameters = parameters; return this; } @@ -60,7 +60,7 @@ public AddonDiscoveryMethod setMatchProperties(@Nullable List parameters = !(paramObject instanceof List list) ? null + : list.stream().filter(e -> (e instanceof AddonParameter)).map(e -> ((AddonParameter) e)).toList(); - Object object = nodeIterator.nextList("match-properties", false); - List matchProperties = !(object instanceof List list) ? null + Object matchPropObject = nodeIterator.nextList("match-properties", false); + List matchProperties = !(matchPropObject instanceof List list) ? null : list.stream().filter(e -> (e instanceof AddonMatchProperty)).map(e -> ((AddonMatchProperty) e)) .toList(); nodeIterator.assertEndOfType(); - return new AddonDiscoveryMethod().setServiceType(serviceType).setMdnsServiceType(mdnsServiceType) + return new AddonDiscoveryMethod().setServiceType(serviceType).setParameters(parameters) .setMatchProperties(matchProperties); } } diff --git a/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/xml/AddonParameterConverter.java b/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/xml/AddonParameterConverter.java new file mode 100644 index 00000000000..914912efb04 --- /dev/null +++ b/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/xml/AddonParameterConverter.java @@ -0,0 +1,53 @@ +/** + * Copyright (c) 2010-2023 Contributors to the openHAB project + * + * See the NOTICE file(s) distributed with this work for additional + * information. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + */ +package org.openhab.core.addon.internal.xml; + +import java.util.List; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; +import org.openhab.core.addon.AddonMatchProperty; +import org.openhab.core.addon.AddonParameter; +import org.openhab.core.config.core.xml.util.GenericUnmarshaller; +import org.openhab.core.config.core.xml.util.NodeIterator; + +import com.thoughtworks.xstream.converters.UnmarshallingContext; +import com.thoughtworks.xstream.io.HierarchicalStreamReader; + +/** + * The {@link AddonParameterConverter} is a concrete implementation of the {@code XStream} {@link Converter} + * interface used to convert add-on discovery method parameter information within an XML document into a + * {@link AddonMatchProperty} object. + * + * @author Mark Herwege - Initial contribution + */ +@NonNullByDefault +public class AddonParameterConverter extends GenericUnmarshaller { + + public AddonParameterConverter() { + super(AddonParameter.class); + } + + @Override + public @Nullable Object unmarshal(HierarchicalStreamReader reader, UnmarshallingContext context) { + List nodes = (List) context.convertAnother(context, List.class); + NodeIterator nodeIterator = new NodeIterator(nodes); + + String name = requireNonEmpty((String) nodeIterator.nextValue("name", true), "Name is null or empty"); + String value = requireNonEmpty((String) nodeIterator.nextValue("value", true), "Value is null or empty"); + + nodeIterator.assertEndOfType(); + + return new AddonMatchProperty(name, value); + } +} diff --git a/bundles/org.openhab.core.addon/src/test/java/org/openhab/core/addon/AddonInfoListReaderTest.java b/bundles/org.openhab.core.addon/src/test/java/org/openhab/core/addon/AddonInfoListReaderTest.java index 66c8485230f..b08c85a8174 100644 --- a/bundles/org.openhab.core.addon/src/test/java/org/openhab/core/addon/AddonInfoListReaderTest.java +++ b/bundles/org.openhab.core.addon/src/test/java/org/openhab/core/addon/AddonInfoListReaderTest.java @@ -12,10 +12,7 @@ */ package org.openhab.core.addon; -import static org.junit.jupiter.api.Assertions.assertEquals; -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.junit.jupiter.api.Assertions.*; import java.util.List; @@ -44,7 +41,12 @@ class AddonInfoListReaderTest { + " " + " " + " mdns" - + " _printer._tcp.local." + + " " + + " " + + " mdnsServiceType" + + " _printer._tcp.local." + + " " + + " " + " " + " " + " rp" @@ -91,7 +93,13 @@ void testAddonInfoListReader() { AddonDiscoveryMethod method = discoveryMethods.get(0); assertNotNull(method); assertEquals("mdns", method.getServiceType()); - assertEquals("_printer._tcp.local.", method.getMdnsServiceType()); + List parameters = method.getParameters(); + assertNotNull(parameters); + assertEquals(1, parameters.size()); + AddonParameter parameter = parameters.get(0); + assertNotNull(parameter); + assertEquals("mdnsServiceType", parameter.getName()); + assertEquals("_printer._tcp.local.", parameter.getValue()); List matchProperties = method.getMatchProperties(); assertNotNull(matchProperties); assertEquals(2, matchProperties.size()); @@ -104,7 +112,9 @@ void testAddonInfoListReader() { method = discoveryMethods.get(1); assertNotNull(method); assertEquals("upnp", method.getServiceType()); - assertEquals("", method.getMdnsServiceType()); + parameters = method.getParameters(); + assertNotNull(parameters); + assertEquals(0, parameters.size()); matchProperties = method.getMatchProperties(); assertNotNull(matchProperties); assertEquals(1, matchProperties.size()); diff --git a/bundles/org.openhab.core.addon/src/test/java/org/openhab/core/addon/AddonInfoRegistryMergeTest.java b/bundles/org.openhab.core.addon/src/test/java/org/openhab/core/addon/AddonInfoRegistryMergeTest.java index 335dc1012ec..ce7c5628d3c 100644 --- a/bundles/org.openhab.core.addon/src/test/java/org/openhab/core/addon/AddonInfoRegistryMergeTest.java +++ b/bundles/org.openhab.core.addon/src/test/java/org/openhab/core/addon/AddonInfoRegistryMergeTest.java @@ -12,16 +12,9 @@ */ package org.openhab.core.addon; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertTrue; -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; +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; import java.util.List; import java.util.Locale; @@ -67,7 +60,7 @@ private AddonInfoProvider createAddonInfoProvider0() { private AddonInfoProvider createAddonInfoProvider1() { AddonDiscoveryMethod discoveryMethod = new AddonDiscoveryMethod().setServiceType("mdns") - .setMdnsServiceType("_hue._tcp.local."); + .setParameters(List.of(new AddonParameter("mdnsServiceType", "_hue._tcp.local."))); AddonInfo addonInfo = AddonInfo.builder("hue", "binding").withName("name-one") .withDescription("description-one").withCountries("GB,NL").withConnection("local") .withDiscoveryMethods(List.of(discoveryMethod)).build(); diff --git a/bundles/org.openhab.core.config.discovery.addon.mdns/src/main/java/org/openhab/core/config/discovery/addon/mdns/MDNSAddonFinder.java b/bundles/org.openhab.core.config.discovery.addon.mdns/src/main/java/org/openhab/core/config/discovery/addon/mdns/MDNSAddonFinder.java index 6af2d646663..8f4d4c47138 100644 --- a/bundles/org.openhab.core.config.discovery.addon.mdns/src/main/java/org/openhab/core/config/discovery/addon/mdns/MDNSAddonFinder.java +++ b/bundles/org.openhab.core.config.discovery.addon.mdns/src/main/java/org/openhab/core/config/discovery/addon/mdns/MDNSAddonFinder.java @@ -12,8 +12,7 @@ */ package org.openhab.core.config.discovery.addon.mdns; -import static org.openhab.core.config.discovery.addon.AddonFinderConstants.SERVICE_NAME_MDNS; -import static org.openhab.core.config.discovery.addon.AddonFinderConstants.SERVICE_TYPE_MDNS; +import static org.openhab.core.config.discovery.addon.AddonFinderConstants.*; import java.util.HashSet; import java.util.List; @@ -44,10 +43,12 @@ import org.slf4j.LoggerFactory; /** - * This is a {@link MDNSAddonFinder} for finding suggested add-ons via mDNS. + * This is a {@link MDNSAddonFinder} for finding suggested add-ons via mDNS. This finder requires a + * {@code mdnsServiceType} parameter to be present in the add-on info discovery method. * * @author Andrew Fiddian-Green - Initial contribution * @author Mark Herwege - refactor to allow uninstall + * @author Mark Herwege - change to discovery method schema */ @NonNullByDefault @Component(service = AddonFinder.class, name = MDNSAddonFinder.SERVICE_NAME) @@ -71,7 +72,7 @@ public MDNSAddonFinder(@Reference MDNSClient mdnsClient) { /** * Adds the given mDNS service to the set of discovered services. - * + * * @param device the mDNS service to be added. */ public void addService(ServiceInfo service, boolean isResolved) { @@ -94,15 +95,15 @@ public void setAddonCandidates(List candidates) { // Remove listeners for all service types that are no longer in candidates addonCandidates.stream().filter(c -> !candidates.contains(c)) .forEach(c -> c.getDiscoveryMethods().stream().filter(m -> SERVICE_TYPE.equals(m.getServiceType())) - .filter(m -> !m.getMdnsServiceType().isEmpty()) - .forEach(m -> mdnsClient.removeServiceListener(m.getMdnsServiceType(), this))); + .filter(m -> !getMdnsServiceType(m).isEmpty()) + .forEach(m -> mdnsClient.removeServiceListener(getMdnsServiceType(m), this))); // Add listeners for all service types in candidates super.setAddonCandidates(candidates); addonCandidates .forEach(c -> c.getDiscoveryMethods().stream().filter(m -> SERVICE_TYPE.equals(m.getServiceType())) - .filter(m -> !m.getMdnsServiceType().isEmpty()).forEach(m -> { - String serviceType = m.getMdnsServiceType(); + .filter(m -> !getMdnsServiceType(m).isEmpty()).forEach(m -> { + String serviceType = getMdnsServiceType(m); mdnsClient.addServiceListener(serviceType, this); scheduler.submit(() -> mdnsClient.list(serviceType)); })); @@ -111,8 +112,8 @@ public void setAddonCandidates(List candidates) { @Override public void unsetAddonCandidates() { addonCandidates.forEach(c -> c.getDiscoveryMethods().stream() - .filter(m -> SERVICE_TYPE.equals(m.getServiceType())).filter(m -> !m.getMdnsServiceType().isEmpty()) - .forEach(m -> mdnsClient.removeServiceListener(m.getMdnsServiceType(), this))); + .filter(m -> SERVICE_TYPE.equals(m.getServiceType())).filter(m -> !getMdnsServiceType(m).isEmpty()) + .forEach(m -> mdnsClient.removeServiceListener(getMdnsServiceType(m), this))); super.unsetAddonCandidates(); } @@ -133,7 +134,7 @@ public Set getSuggestedAddons() { for (ServiceInfo service : services.values()) { logger.trace("Checking service: {}/{}", service.getQualifiedName(), service.getNiceTextString()); - if (method.getMdnsServiceType().equals(service.getType()) + if (getMdnsServiceType(method).equals(service.getType()) && propertyMatches(matchProperties, NAME, service.getName()) && propertyMatches(matchProperties, APPLICATION, service.getApplication()) && matchPropertyKeys.stream().allMatch( @@ -148,6 +149,12 @@ && propertyMatches(matchProperties, APPLICATION, service.getApplication()) return result; } + private String getMdnsServiceType(AddonDiscoveryMethod method) { + String param = method.getParameters().stream().filter(p -> "mdnsServiceType".equals(p.getName())) + .map(p -> p.getValue()).findFirst().orElse(""); + return param == null ? "" : param; + } + @Override public String getServiceName() { return SERVICE_NAME; diff --git a/bundles/org.openhab.core.config.discovery.addon.mdns/src/test/java/org/openhab/core/config/discovery/addon/mdns/tests/MDNSAddonFinderTests.java b/bundles/org.openhab.core.config.discovery.addon.mdns/src/test/java/org/openhab/core/config/discovery/addon/mdns/tests/MDNSAddonFinderTests.java index d202dea4d68..8088ffeae0c 100644 --- a/bundles/org.openhab.core.config.discovery.addon.mdns/src/test/java/org/openhab/core/config/discovery/addon/mdns/tests/MDNSAddonFinderTests.java +++ b/bundles/org.openhab.core.config.discovery.addon.mdns/src/test/java/org/openhab/core/config/discovery/addon/mdns/tests/MDNSAddonFinderTests.java @@ -33,6 +33,7 @@ import org.openhab.core.addon.AddonDiscoveryMethod; import org.openhab.core.addon.AddonInfo; import org.openhab.core.addon.AddonMatchProperty; +import org.openhab.core.addon.AddonParameter; import org.openhab.core.config.discovery.addon.AddonFinder; import org.openhab.core.config.discovery.addon.AddonFinderConstants; import org.openhab.core.config.discovery.addon.AddonSuggestionService; @@ -103,12 +104,12 @@ private void setupAddonInfos() { AddonDiscoveryMethod hp = new AddonDiscoveryMethod().setServiceType(AddonFinderConstants.SERVICE_TYPE_MDNS) .setMatchProperties( List.of(new AddonMatchProperty("rp", ".*"), new AddonMatchProperty("ty", "hp (.*)"))) - .setMdnsServiceType("_printer._tcp.local."); + .setParameters(List.of(new AddonParameter("mdnsServiceType", "_printer._tcp.local."))); addonInfos.add(AddonInfo.builder("hpprinter", "binding").withName("HP").withDescription("HP Printer") .withDiscoveryMethods(List.of(hp)).build()); AddonDiscoveryMethod hue = new AddonDiscoveryMethod().setServiceType(AddonFinderConstants.SERVICE_TYPE_MDNS) - .setMdnsServiceType("_hue._tcp.local."); + .setParameters(List.of(new AddonParameter("mdnsServiceType", "_hue._tcp.local."))); addonInfos.add(AddonInfo.builder("hue", "binding").withName("Hue").withDescription("Hue Bridge") .withDiscoveryMethods(List.of(hue)).build()); } diff --git a/bundles/org.openhab.core.config.discovery.addon/src/test/java/org/openhab/core/config/discovery/addon/tests/AddonSuggestionServiceTests.java b/bundles/org.openhab.core.config.discovery.addon/src/test/java/org/openhab/core/config/discovery/addon/tests/AddonSuggestionServiceTests.java index 0d72ac4d597..5b0280df6bd 100644 --- a/bundles/org.openhab.core.config.discovery.addon/src/test/java/org/openhab/core/config/discovery/addon/tests/AddonSuggestionServiceTests.java +++ b/bundles/org.openhab.core.config.discovery.addon/src/test/java/org/openhab/core/config/discovery/addon/tests/AddonSuggestionServiceTests.java @@ -35,6 +35,7 @@ import org.openhab.core.addon.AddonInfo; import org.openhab.core.addon.AddonInfoProvider; import org.openhab.core.addon.AddonMatchProperty; +import org.openhab.core.addon.AddonParameter; import org.openhab.core.config.discovery.addon.AddonFinder; import org.openhab.core.config.discovery.addon.AddonFinderConstants; import org.openhab.core.config.discovery.addon.AddonSuggestionService; @@ -125,13 +126,13 @@ private void setupMockAddonInfoProvider() { AddonDiscoveryMethod hp = new AddonDiscoveryMethod().setServiceType(AddonFinderConstants.SERVICE_TYPE_MDNS) .setMatchProperties( List.of(new AddonMatchProperty("rp", ".*"), new AddonMatchProperty("ty", "hp (.*)"))) - .setMdnsServiceType("_printer._tcp.local."); + .setParameters(List.of(new AddonParameter("mdnsServiceType", "_printer._tcp.local."))); AddonDiscoveryMethod hue1 = new AddonDiscoveryMethod().setServiceType(AddonFinderConstants.SERVICE_TYPE_UPNP) .setMatchProperties(List.of(new AddonMatchProperty("modelName", "Philips hue bridge"))); AddonDiscoveryMethod hue2 = new AddonDiscoveryMethod().setServiceType(AddonFinderConstants.SERVICE_TYPE_MDNS) - .setMdnsServiceType("_hue._tcp.local."); + .setParameters(List.of(new AddonParameter("mdnsServiceType", "_hue._tcp.local."))); // create the mock addonInfoProvider = mock(AddonInfoProvider.class); From f078adac6851e8f5a0cb7b26dd83f9e1cba062cf Mon Sep 17 00:00:00 2001 From: Mark Herwege Date: Thu, 14 Dec 2023 09:58:16 +0100 Subject: [PATCH 2/6] fix tests Signed-off-by: Mark Herwege --- .../core/addon/xml/test/AddonInfoTest.java | 20 ++++++++++++------- .../OH-INF/addon/addon.xml | 7 ++++++- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/itests/org.openhab.core.addon.tests/src/main/java/org/openhab/core/addon/xml/test/AddonInfoTest.java b/itests/org.openhab.core.addon.tests/src/main/java/org/openhab/core/addon/xml/test/AddonInfoTest.java index 3e109b7a5a9..7f4048d661f 100644 --- a/itests/org.openhab.core.addon.tests/src/main/java/org/openhab/core/addon/xml/test/AddonInfoTest.java +++ b/itests/org.openhab.core.addon.tests/src/main/java/org/openhab/core/addon/xml/test/AddonInfoTest.java @@ -12,12 +12,9 @@ */ package org.openhab.core.addon.xml.test; -import static org.hamcrest.CoreMatchers.is; -import static org.hamcrest.CoreMatchers.notNullValue; +import static org.hamcrest.CoreMatchers.*; import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.*; import java.net.URI; import java.util.List; @@ -32,6 +29,7 @@ import org.openhab.core.addon.AddonInfo; import org.openhab.core.addon.AddonInfoRegistry; import org.openhab.core.addon.AddonMatchProperty; +import org.openhab.core.addon.AddonParameter; import org.openhab.core.config.core.ConfigDescription; import org.openhab.core.config.core.ConfigDescriptionParameter; import org.openhab.core.config.core.ConfigDescriptionRegistry; @@ -80,7 +78,13 @@ public void assertThatAddonInfoIsReadProperly() throws Exception { AddonDiscoveryMethod discoveryMethod = discoveryMethods.get(0); assertNotNull(discoveryMethod); assertEquals("mdns", discoveryMethod.getServiceType()); - assertEquals("_hue._tcp.local.", discoveryMethod.getMdnsServiceType()); + List parameters = discoveryMethod.getParameters(); + assertNotNull(parameters); + assertEquals(1, parameters.size()); + AddonParameter parameter = parameters.get(0); + assertNotNull(parameter); + assertEquals("mdnsServiceType", parameter.getName()); + assertEquals("_hue._tcp.local.", parameter.getValue()); List properties = discoveryMethod.getMatchProperties(); assertNotNull(properties); assertEquals(0, properties.size()); @@ -88,7 +92,9 @@ public void assertThatAddonInfoIsReadProperly() throws Exception { discoveryMethod = discoveryMethods.get(1); assertNotNull(discoveryMethod); assertEquals("upnp", discoveryMethod.getServiceType()); - assertEquals("", discoveryMethod.getMdnsServiceType()); + parameters = discoveryMethod.getParameters(); + assertNotNull(parameters); + assertEquals(0, parameters.size()); properties = discoveryMethod.getMatchProperties(); assertNotNull(properties); assertEquals(1, properties.size()); diff --git a/itests/org.openhab.core.addon.tests/src/main/resources/test-bundle-pool/BundleInfoTest.bundle/OH-INF/addon/addon.xml b/itests/org.openhab.core.addon.tests/src/main/resources/test-bundle-pool/BundleInfoTest.bundle/OH-INF/addon/addon.xml index 9e5db0944bd..81987312cb6 100644 --- a/itests/org.openhab.core.addon.tests/src/main/resources/test-bundle-pool/BundleInfoTest.bundle/OH-INF/addon/addon.xml +++ b/itests/org.openhab.core.addon.tests/src/main/resources/test-bundle-pool/BundleInfoTest.bundle/OH-INF/addon/addon.xml @@ -34,7 +34,12 @@ mdns - _hue._tcp.local. + + + mdnsServiceType + _hue._tcp.local. + + upnp From 011644bd96dcf8972166c862b84cfe5b39778fb2 Mon Sep 17 00:00:00 2001 From: Mark Herwege Date: Thu, 14 Dec 2023 10:30:51 +0100 Subject: [PATCH 3/6] rename parameter to avoid name conflict Signed-off-by: Mark Herwege --- bundles/org.openhab.core.addon/schema/addon-1.0.0.xsd | 4 ++-- .../core/addon/internal/xml/AddonInfoListReader.java | 4 +++- .../org/openhab/core/addon/AddonInfoListReaderTest.java | 8 ++++---- .../BundleInfoTest.bundle/OH-INF/addon/addon.xml | 8 ++++---- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/bundles/org.openhab.core.addon/schema/addon-1.0.0.xsd b/bundles/org.openhab.core.addon/schema/addon-1.0.0.xsd index 38587f19782..64e6b042618 100644 --- a/bundles/org.openhab.core.addon/schema/addon-1.0.0.xsd +++ b/bundles/org.openhab.core.addon/schema/addon-1.0.0.xsd @@ -90,14 +90,14 @@ - + - + diff --git a/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/xml/AddonInfoListReader.java b/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/xml/AddonInfoListReader.java index a733e169492..efee500abf3 100644 --- a/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/xml/AddonInfoListReader.java +++ b/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/xml/AddonInfoListReader.java @@ -18,6 +18,7 @@ import org.openhab.core.addon.AddonDiscoveryMethod; import org.openhab.core.addon.AddonInfoList; import org.openhab.core.addon.AddonMatchProperty; +import org.openhab.core.addon.AddonParameter; import org.openhab.core.config.core.ConfigDescription; import org.openhab.core.config.core.ConfigDescriptionParameter; import org.openhab.core.config.core.ConfigDescriptionParameterGroup; @@ -94,7 +95,8 @@ protected void registerAliases(XStream xstream) { xstream.alias("discovery-methods", NodeList.class); xstream.alias("discovery-method", AddonDiscoveryMethod.class); xstream.alias("service-type", NodeValue.class); - xstream.alias("mdns-service-type", NodeValue.class); + xstream.alias("discovery-parameters", NodeList.class); + xstream.alias("discovery-parameter", AddonParameter.class); xstream.alias("match-properties", NodeList.class); xstream.alias("match-property", AddonMatchProperty.class); xstream.alias("regex", NodeValue.class); diff --git a/bundles/org.openhab.core.addon/src/test/java/org/openhab/core/addon/AddonInfoListReaderTest.java b/bundles/org.openhab.core.addon/src/test/java/org/openhab/core/addon/AddonInfoListReaderTest.java index b08c85a8174..83e70e86336 100644 --- a/bundles/org.openhab.core.addon/src/test/java/org/openhab/core/addon/AddonInfoListReaderTest.java +++ b/bundles/org.openhab.core.addon/src/test/java/org/openhab/core/addon/AddonInfoListReaderTest.java @@ -41,12 +41,12 @@ class AddonInfoListReaderTest { + " " + " " + " mdns" - + " " - + " " + + " " + + " " + " mdnsServiceType" + " _printer._tcp.local." - + " " - + " " + + " " + + " " + " " + " " + " rp" diff --git a/itests/org.openhab.core.addon.tests/src/main/resources/test-bundle-pool/BundleInfoTest.bundle/OH-INF/addon/addon.xml b/itests/org.openhab.core.addon.tests/src/main/resources/test-bundle-pool/BundleInfoTest.bundle/OH-INF/addon/addon.xml index 81987312cb6..e05524d7d7a 100644 --- a/itests/org.openhab.core.addon.tests/src/main/resources/test-bundle-pool/BundleInfoTest.bundle/OH-INF/addon/addon.xml +++ b/itests/org.openhab.core.addon.tests/src/main/resources/test-bundle-pool/BundleInfoTest.bundle/OH-INF/addon/addon.xml @@ -34,12 +34,12 @@ mdns - - + + mdnsServiceType _hue._tcp.local. - - + + upnp From 8577681843f2e3cccb5a54e4d8c1802f6da2440b Mon Sep 17 00:00:00 2001 From: Mark Herwege Date: Thu, 14 Dec 2023 12:07:42 +0100 Subject: [PATCH 4/6] fix xml parsing Signed-off-by: Mark Herwege --- .../core/addon/internal/xml/AddonDiscoveryMethodConverter.java | 2 +- .../openhab/core/addon/internal/xml/AddonInfoListReader.java | 2 ++ .../core/addon/internal/xml/AddonParameterConverter.java | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/xml/AddonDiscoveryMethodConverter.java b/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/xml/AddonDiscoveryMethodConverter.java index 7a107f194b8..26cb3357a77 100644 --- a/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/xml/AddonDiscoveryMethodConverter.java +++ b/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/xml/AddonDiscoveryMethodConverter.java @@ -47,7 +47,7 @@ public AddonDiscoveryMethodConverter() { String serviceType = requireNonEmpty((String) nodeIterator.nextValue("service-type", true), "Service type is null or empty"); - Object paramObject = nodeIterator.nextList("parameters", false); + Object paramObject = nodeIterator.nextList("discovery-parameters", false); List parameters = !(paramObject instanceof List list) ? null : list.stream().filter(e -> (e instanceof AddonParameter)).map(e -> ((AddonParameter) e)).toList(); diff --git a/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/xml/AddonInfoListReader.java b/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/xml/AddonInfoListReader.java index efee500abf3..4b6c8c60453 100644 --- a/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/xml/AddonInfoListReader.java +++ b/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/xml/AddonInfoListReader.java @@ -70,6 +70,7 @@ protected void registerConverters(XStream xstream) { xstream.registerConverter(new ConfigDescriptionParameterGroupConverter()); xstream.registerConverter(new FilterCriteriaConverter()); xstream.registerConverter(new AddonDiscoveryMethodConverter()); + xstream.registerConverter(new AddonParameterConverter()); xstream.registerConverter(new AddonMatchPropertyConverter()); } @@ -99,6 +100,7 @@ protected void registerAliases(XStream xstream) { xstream.alias("discovery-parameter", AddonParameter.class); xstream.alias("match-properties", NodeList.class); xstream.alias("match-property", AddonMatchProperty.class); + xstream.alias("value", NodeValue.class); xstream.alias("regex", NodeValue.class); } } diff --git a/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/xml/AddonParameterConverter.java b/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/xml/AddonParameterConverter.java index 914912efb04..ea9c3a45233 100644 --- a/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/xml/AddonParameterConverter.java +++ b/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/xml/AddonParameterConverter.java @@ -48,6 +48,6 @@ public AddonParameterConverter() { nodeIterator.assertEndOfType(); - return new AddonMatchProperty(name, value); + return new AddonParameter(name, value); } } From 9a08abaf7b224179f57324b50a10382f3a80a37a Mon Sep 17 00:00:00 2001 From: Mark Herwege Date: Thu, 14 Dec 2023 13:49:06 +0100 Subject: [PATCH 5/6] reviewer feedback Signed-off-by: Mark Herwege --- .../core/config/discovery/addon/mdns/MDNSAddonFinder.java | 4 +++- .../discovery/addon/mdns/tests/MDNSAddonFinderTests.java | 5 +++-- .../discovery/addon/tests/AddonSuggestionServiceTests.java | 6 ++++-- .../java/org/openhab/core/addon/xml/test/AddonInfoTest.java | 3 ++- .../BundleInfoTest.bundle/OH-INF/addon/addon.xml | 2 +- 5 files changed, 13 insertions(+), 7 deletions(-) diff --git a/bundles/org.openhab.core.config.discovery.addon.mdns/src/main/java/org/openhab/core/config/discovery/addon/mdns/MDNSAddonFinder.java b/bundles/org.openhab.core.config.discovery.addon.mdns/src/main/java/org/openhab/core/config/discovery/addon/mdns/MDNSAddonFinder.java index 8f4d4c47138..c6ae17dbcd7 100644 --- a/bundles/org.openhab.core.config.discovery.addon.mdns/src/main/java/org/openhab/core/config/discovery/addon/mdns/MDNSAddonFinder.java +++ b/bundles/org.openhab.core.config.discovery.addon.mdns/src/main/java/org/openhab/core/config/discovery/addon/mdns/MDNSAddonFinder.java @@ -57,6 +57,8 @@ public class MDNSAddonFinder extends BaseAddonFinder implements ServiceListener public static final String SERVICE_TYPE = SERVICE_TYPE_MDNS; public static final String SERVICE_NAME = SERVICE_NAME_MDNS; + public static final String MDNS_SERVICE_TYPE = "mdnsServiceType"; + private static final String NAME = "name"; private static final String APPLICATION = "application"; @@ -150,7 +152,7 @@ && propertyMatches(matchProperties, APPLICATION, service.getApplication()) } private String getMdnsServiceType(AddonDiscoveryMethod method) { - String param = method.getParameters().stream().filter(p -> "mdnsServiceType".equals(p.getName())) + String param = method.getParameters().stream().filter(p -> MDNS_SERVICE_TYPE.equals(p.getName())) .map(p -> p.getValue()).findFirst().orElse(""); return param == null ? "" : param; } diff --git a/bundles/org.openhab.core.config.discovery.addon.mdns/src/test/java/org/openhab/core/config/discovery/addon/mdns/tests/MDNSAddonFinderTests.java b/bundles/org.openhab.core.config.discovery.addon.mdns/src/test/java/org/openhab/core/config/discovery/addon/mdns/tests/MDNSAddonFinderTests.java index 8088ffeae0c..50ca7bf4efd 100644 --- a/bundles/org.openhab.core.config.discovery.addon.mdns/src/test/java/org/openhab/core/config/discovery/addon/mdns/tests/MDNSAddonFinderTests.java +++ b/bundles/org.openhab.core.config.discovery.addon.mdns/src/test/java/org/openhab/core/config/discovery/addon/mdns/tests/MDNSAddonFinderTests.java @@ -15,6 +15,7 @@ import static org.junit.jupiter.api.Assertions.*; import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.*; +import static org.openhab.core.config.discovery.addon.mdns.MDNSAddonFinder.MDNS_SERVICE_TYPE; import java.util.ArrayList; import java.util.Collections; @@ -104,12 +105,12 @@ private void setupAddonInfos() { AddonDiscoveryMethod hp = new AddonDiscoveryMethod().setServiceType(AddonFinderConstants.SERVICE_TYPE_MDNS) .setMatchProperties( List.of(new AddonMatchProperty("rp", ".*"), new AddonMatchProperty("ty", "hp (.*)"))) - .setParameters(List.of(new AddonParameter("mdnsServiceType", "_printer._tcp.local."))); + .setParameters(List.of(new AddonParameter(MDNS_SERVICE_TYPE, "_printer._tcp.local."))); addonInfos.add(AddonInfo.builder("hpprinter", "binding").withName("HP").withDescription("HP Printer") .withDiscoveryMethods(List.of(hp)).build()); AddonDiscoveryMethod hue = new AddonDiscoveryMethod().setServiceType(AddonFinderConstants.SERVICE_TYPE_MDNS) - .setParameters(List.of(new AddonParameter("mdnsServiceType", "_hue._tcp.local."))); + .setParameters(List.of(new AddonParameter(MDNS_SERVICE_TYPE, "_hue._tcp.local."))); addonInfos.add(AddonInfo.builder("hue", "binding").withName("Hue").withDescription("Hue Bridge") .withDiscoveryMethods(List.of(hue)).build()); } diff --git a/bundles/org.openhab.core.config.discovery.addon/src/test/java/org/openhab/core/config/discovery/addon/tests/AddonSuggestionServiceTests.java b/bundles/org.openhab.core.config.discovery.addon/src/test/java/org/openhab/core/config/discovery/addon/tests/AddonSuggestionServiceTests.java index 5b0280df6bd..04f85574c12 100644 --- a/bundles/org.openhab.core.config.discovery.addon/src/test/java/org/openhab/core/config/discovery/addon/tests/AddonSuggestionServiceTests.java +++ b/bundles/org.openhab.core.config.discovery.addon/src/test/java/org/openhab/core/config/discovery/addon/tests/AddonSuggestionServiceTests.java @@ -53,6 +53,8 @@ @TestInstance(Lifecycle.PER_CLASS) public class AddonSuggestionServiceTests { + public static final String MDNS_SERVICE_TYPE = "mdnsServiceType"; + private @NonNullByDefault({}) ConfigurationAdmin configurationAdmin; private @NonNullByDefault({}) LocaleProvider localeProvider; private @NonNullByDefault({}) AddonInfoProvider addonInfoProvider; @@ -126,13 +128,13 @@ private void setupMockAddonInfoProvider() { AddonDiscoveryMethod hp = new AddonDiscoveryMethod().setServiceType(AddonFinderConstants.SERVICE_TYPE_MDNS) .setMatchProperties( List.of(new AddonMatchProperty("rp", ".*"), new AddonMatchProperty("ty", "hp (.*)"))) - .setParameters(List.of(new AddonParameter("mdnsServiceType", "_printer._tcp.local."))); + .setParameters(List.of(new AddonParameter(MDNS_SERVICE_TYPE, "_printer._tcp.local."))); AddonDiscoveryMethod hue1 = new AddonDiscoveryMethod().setServiceType(AddonFinderConstants.SERVICE_TYPE_UPNP) .setMatchProperties(List.of(new AddonMatchProperty("modelName", "Philips hue bridge"))); AddonDiscoveryMethod hue2 = new AddonDiscoveryMethod().setServiceType(AddonFinderConstants.SERVICE_TYPE_MDNS) - .setParameters(List.of(new AddonParameter("mdnsServiceType", "_hue._tcp.local."))); + .setParameters(List.of(new AddonParameter(MDNS_SERVICE_TYPE, "_hue._tcp.local."))); // create the mock addonInfoProvider = mock(AddonInfoProvider.class); diff --git a/itests/org.openhab.core.addon.tests/src/main/java/org/openhab/core/addon/xml/test/AddonInfoTest.java b/itests/org.openhab.core.addon.tests/src/main/java/org/openhab/core/addon/xml/test/AddonInfoTest.java index 7f4048d661f..c5388d72db4 100644 --- a/itests/org.openhab.core.addon.tests/src/main/java/org/openhab/core/addon/xml/test/AddonInfoTest.java +++ b/itests/org.openhab.core.addon.tests/src/main/java/org/openhab/core/addon/xml/test/AddonInfoTest.java @@ -15,6 +15,7 @@ import static org.hamcrest.CoreMatchers.*; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.*; +import static org.openhab.core.config.discovery.addon.mdns.MDNSAddonFinder.MDNS_SERVICE_TYPE; import java.net.URI; import java.util.List; @@ -83,7 +84,7 @@ public void assertThatAddonInfoIsReadProperly() throws Exception { assertEquals(1, parameters.size()); AddonParameter parameter = parameters.get(0); assertNotNull(parameter); - assertEquals("mdnsServiceType", parameter.getName()); + assertEquals(MDNS_SERVICE_TYPE, parameter.getName()); assertEquals("_hue._tcp.local.", parameter.getValue()); List properties = discoveryMethod.getMatchProperties(); assertNotNull(properties); diff --git a/itests/org.openhab.core.addon.tests/src/main/resources/test-bundle-pool/BundleInfoTest.bundle/OH-INF/addon/addon.xml b/itests/org.openhab.core.addon.tests/src/main/resources/test-bundle-pool/BundleInfoTest.bundle/OH-INF/addon/addon.xml index e05524d7d7a..5234bf472c8 100644 --- a/itests/org.openhab.core.addon.tests/src/main/resources/test-bundle-pool/BundleInfoTest.bundle/OH-INF/addon/addon.xml +++ b/itests/org.openhab.core.addon.tests/src/main/resources/test-bundle-pool/BundleInfoTest.bundle/OH-INF/addon/addon.xml @@ -35,7 +35,7 @@ mdns - + mdnsServiceType _hue._tcp.local. From 2fab6606ec66c73ba5d7b5e862eca355f8daaf6e Mon Sep 17 00:00:00 2001 From: Mark Herwege Date: Thu, 14 Dec 2023 14:44:46 +0100 Subject: [PATCH 6/6] AddonInfoReader changes Signed-off-by: Mark Herwege --- .../openhab/core/addon/internal/xml/AddonInfoReader.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/xml/AddonInfoReader.java b/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/xml/AddonInfoReader.java index 47cad85e943..892b84cfaa1 100644 --- a/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/xml/AddonInfoReader.java +++ b/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/xml/AddonInfoReader.java @@ -17,6 +17,7 @@ import org.eclipse.jdt.annotation.NonNullByDefault; import org.openhab.core.addon.AddonDiscoveryMethod; import org.openhab.core.addon.AddonMatchProperty; +import org.openhab.core.addon.AddonParameter; import org.openhab.core.config.core.ConfigDescription; import org.openhab.core.config.core.ConfigDescriptionParameter; import org.openhab.core.config.core.ConfigDescriptionParameterGroup; @@ -70,6 +71,7 @@ protected void registerConverters(XStream xstream) { xstream.registerConverter(new ConfigDescriptionParameterGroupConverter()); xstream.registerConverter(new FilterCriteriaConverter()); xstream.registerConverter(new AddonDiscoveryMethodConverter()); + xstream.registerConverter(new AddonParameterConverter()); xstream.registerConverter(new AddonMatchPropertyConverter()); } @@ -93,9 +95,11 @@ protected void registerAliases(XStream xstream) { xstream.alias("discovery-methods", NodeList.class); xstream.alias("discovery-method", AddonDiscoveryMethod.class); xstream.alias("service-type", NodeValue.class); - xstream.alias("mdns-service-type", NodeValue.class); + xstream.alias("discovery-parameters", NodeList.class); + xstream.alias("discovery-parameter", AddonParameter.class); xstream.alias("match-properties", NodeList.class); xstream.alias("match-property", AddonMatchProperty.class); + xstream.alias("value", NodeValue.class); xstream.alias("regex", NodeValue.class); } }