Fix config parsing and version handling in RemoteAddonServices (openh…

…ab#4043) Signed-off-by: Jan N. Klug <[email protected]>
wborn · Jan 15, 2024 · d3d0fe3 · d3d0fe3
1 parent c82a9e6
commit d3d0fe3
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 5 deletions.
diff --git a/...ketplace/src/main/java/org/openhab/core/addon/marketplace/AbstractRemoteAddonService.java b/...ketplace/src/main/java/org/openhab/core/addon/marketplace/AbstractRemoteAddonService.java
@@ -65,11 +65,17 @@ public abstract class AbstractRemoteAddonService implements AddonService {
     static final String CONFIG_REMOTE_ENABLED = "remote";
     static final String CONFIG_INCLUDE_INCOMPATIBLE = "includeIncompatible";
     static final Comparator<Addon> BY_COMPATIBLE_AND_VERSION = (addon1, addon2) -> {
-        // prefer compatible over incompatible
+        // prefer compatible to incompatible
         int compatible = Boolean.compare(addon2.getCompatible(), addon1.getCompatible());
+        if (compatible != 0) {
+            return compatible;
+        }
+        // Add-on versions often contain a dash instead of a dot as separator for the qualifier (e.g. -SNAPSHOT)
+        // This is not a valid format and everything after the dash needs to be removed.
+        BundleVersion version1 = new BundleVersion(addon1.getVersion().replaceAll("-.*", ".0"));
+        BundleVersion version2 = new BundleVersion(addon2.getVersion().replaceAll("-.*", ".0"));
         // prefer newer version over older
-        return compatible != 0 ? compatible
-                : new BundleVersion(addon2.getVersion()).compareTo(new BundleVersion(addon1.getVersion()));
+        return version2.compareTo(version1);
     };
 
     protected final BundleVersion coreVersion;

diff --git a/...lace/src/main/java/org/openhab/core/addon/marketplace/internal/json/JsonAddonService.java b/...lace/src/main/java/org/openhab/core/addon/marketplace/internal/json/JsonAddonService.java
@@ -34,6 +34,7 @@
 import org.openhab.core.addon.marketplace.AbstractRemoteAddonService;
 import org.openhab.core.addon.marketplace.MarketplaceAddonHandler;
 import org.openhab.core.addon.marketplace.internal.json.model.AddonEntryDTO;
+import org.openhab.core.config.core.ConfigParser;
 import org.openhab.core.config.core.ConfigurableService;
 import org.openhab.core.events.EventPublisher;
 import org.openhab.core.storage.StorageService;
@@ -87,9 +88,9 @@ public JsonAddonService(@Reference EventPublisher eventPublisher, @Reference Sto
     @Modified
     public void modified(@Nullable Map<String, Object> config) {
         if (config != null) {
-            String urls = Objects.requireNonNullElse((String) config.get(CONFIG_URLS), "");
+            String urls = ConfigParser.valueAsOrElse(config.get(CONFIG_URLS), String.class, "");
             addonServiceUrls = Arrays.asList(urls.split("\\|"));
-            showUnstable = (Boolean) config.getOrDefault(CONFIG_SHOW_UNSTABLE, false);
+            showUnstable = ConfigParser.valueAsOrElse(config.get(CONFIG_SHOW_UNSTABLE), Boolean.class, false);
             cachedRemoteAddons.invalidateValue();
             refreshSource();
         }

diff --git a/...lace/src/test/java/org/openhab/core/addon/marketplace/AbstractRemoteAddonServiceTest.java b/...lace/src/test/java/org/openhab/core/addon/marketplace/AbstractRemoteAddonServiceTest.java
@@ -269,6 +269,18 @@ public void testAddonOrdering() {
         assertThat(actual, is(equalTo(expected)));
     }
 
+    @Test
+    public void testSnapshotVersionsAreParsedProperly() {
+        Addon addon1 = getMockedAddon("4.1.0", true);
+        Addon addon2 = getMockedAddon("4.2.0-SNAPSHOT", true);
+
+        List<Addon> actual = Stream.of(addon1, addon2).sorted(AbstractRemoteAddonService.BY_COMPATIBLE_AND_VERSION)
+                .toList();
+        List<Addon> expected = List.of(addon2, addon1);
+
+        assertThat(actual, is(equalTo(expected)));
+    }
+
     private Addon getMockedAddon(String version, boolean compatible) {
         Addon addon = mock(Addon.class);
         when(addon.getVersion()).thenReturn(version);