From ff6b33f3d8e17776a13e759f9291cf61ac652c6a Mon Sep 17 00:00:00 2001 From: J-N-K Date: Tue, 26 Dec 2023 23:17:05 +0100 Subject: [PATCH] Fix duplicate UIDs in remote add-on services (#3961) Signed-off-by: Jan N. Klug --- .../AbstractRemoteAddonService.java | 18 +++++++++++ .../internal/json/JsonAddonService.java | 22 ++++++++++++-- .../AbstractRemoteAddonServiceTest.java | 30 +++++++++++++++---- 3 files changed, 63 insertions(+), 7 deletions(-) diff --git a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/AbstractRemoteAddonService.java b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/AbstractRemoteAddonService.java index 3ce55c4f6c..8d7aed8d24 100644 --- a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/AbstractRemoteAddonService.java +++ b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/AbstractRemoteAddonService.java @@ -16,7 +16,9 @@ import java.io.IOException; import java.net.URI; import java.time.Duration; import java.util.ArrayList; +import java.util.Comparator; import java.util.Dictionary; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Locale; @@ -58,6 +60,13 @@ import com.google.gson.JsonSyntaxException; public abstract class AbstractRemoteAddonService implements AddonService { static final String CONFIG_REMOTE_ENABLED = "remote"; static final String CONFIG_INCLUDE_INCOMPATIBLE = "includeIncompatible"; + static final Comparator BY_COMPATIBLE_AND_VERSION = (addon1, addon2) -> { + // prefer compatible over incompatible + int compatible = Boolean.compare(addon2.getCompatible(), addon1.getCompatible()); + // prefer newer version over older + return compatible != 0 ? compatible + : new BundleVersion(addon2.getVersion()).compareTo(new BundleVersion(addon1.getVersion())); + }; protected final BundleVersion coreVersion; @@ -131,6 +140,15 @@ public abstract class AbstractRemoteAddonService implements AddonService { boolean showIncompatible = includeIncompatible(); addons.removeIf(addon -> !addon.isInstalled() && !addon.getCompatible() && !showIncompatible); + // check and remove duplicate uids + Map> addonMap = new HashMap<>(); + addons.forEach(a -> addonMap.computeIfAbsent(a.getUid(), k -> new ArrayList<>()).add(a)); + for (List partialAddonList : addonMap.values()) { + if (partialAddonList.size() > 1) { + partialAddonList.stream().sorted(BY_COMPATIBLE_AND_VERSION).skip(1).forEach(addons::remove); + } + } + cachedAddons = addons; this.installedAddons = installedAddons; } diff --git a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/json/JsonAddonService.java b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/json/JsonAddonService.java index 362f5311ea..7ad61709a3 100644 --- a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/json/JsonAddonService.java +++ b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/json/JsonAddonService.java @@ -131,8 +131,26 @@ public class JsonAddonService extends AbstractRemoteAddonService { } catch (IOException e) { return List.of(); } - }).flatMap(List::stream).filter(Objects::nonNull).map(e -> (AddonEntryDTO) e) - .filter(e -> showUnstable || "stable".equals(e.maturity)).map(this::fromAddonEntry).toList(); + }).flatMap(List::stream).filter(Objects::nonNull).map(e -> (AddonEntryDTO) e).filter(this::showAddon) + .map(this::fromAddonEntry).toList(); + } + + /** + * Check if the addon UID is present and the entry is eitehr stable or unstable add-ons are requested + * + * @param addonEntry the add-on entry to check + * @return {@code true} if the add-on entry should be processed, {@code false otherwise} + */ + private boolean showAddon(AddonEntryDTO addonEntry) { + if (addonEntry.uid.isBlank()) { + logger.debug("Skipping {} because the UID is not set", addonEntry); + return false; + } + if (!showUnstable && !"stable".equals(addonEntry.maturity)) { + logger.debug("Skipping {} because the the add-on is not stable and showUnstable is disabled.", addonEntry); + return false; + } + return true; } @Override diff --git a/bundles/org.openhab.core.addon.marketplace/src/test/java/org/openhab/core/addon/marketplace/AbstractRemoteAddonServiceTest.java b/bundles/org.openhab.core.addon.marketplace/src/test/java/org/openhab/core/addon/marketplace/AbstractRemoteAddonServiceTest.java index dac9fa3fc0..1b7393a52a 100644 --- a/bundles/org.openhab.core.addon.marketplace/src/test/java/org/openhab/core/addon/marketplace/AbstractRemoteAddonServiceTest.java +++ b/bundles/org.openhab.core.addon.marketplace/src/test/java/org/openhab/core/addon/marketplace/AbstractRemoteAddonServiceTest.java @@ -13,11 +13,9 @@ package org.openhab.core.addon.marketplace; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.empty; -import static org.hamcrest.Matchers.hasSize; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.notNullValue; -import static org.hamcrest.Matchers.nullValue; +import static org.hamcrest.Matchers.*; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import static org.openhab.core.addon.marketplace.AbstractRemoteAddonService.CONFIG_REMOTE_ENABLED; import static org.openhab.core.addon.marketplace.test.TestAddonService.ALL_ADDON_COUNT; import static org.openhab.core.addon.marketplace.test.TestAddonService.COMPATIBLE_ADDON_COUNT; @@ -31,6 +29,7 @@ import java.io.IOException; import java.util.Dictionary; import java.util.Hashtable; import java.util.List; +import java.util.stream.Stream; import org.eclipse.jdt.annotation.NonNullByDefault; import org.junit.jupiter.api.BeforeEach; @@ -256,6 +255,27 @@ public class AbstractRemoteAddonServiceTest { checkResult(TEST_ADDON, getFullAddonId(TEST_ADDON) + "/failed", false, true); } + // add-comparisons + @Test + public void testAddonOrdering() { + Addon addon1 = getMockedAddon("4.1.0", false); + Addon addon2 = getMockedAddon("4.2.0", true); + Addon addon3 = getMockedAddon("4.1.4", false); + Addon addon4 = getMockedAddon("4.2.1", true); + List actual = Stream.of(addon1, addon2, addon3, addon4) + .sorted(AbstractRemoteAddonService.BY_COMPATIBLE_AND_VERSION).toList(); + List expected = List.of(addon4, addon2, addon3, addon1); + + assertThat(actual, is(equalTo(expected))); + } + + private Addon getMockedAddon(String version, boolean compatible) { + Addon addon = mock(Addon.class); + when(addon.getVersion()).thenReturn(version); + when(addon.getCompatible()).thenReturn(compatible); + return addon; + } + /** * checks that a proper event is posted, the presence in storage and installation status in handler *