From 0b1e1b66ab585cc02a985a74ac12a7884ff0ee0c Mon Sep 17 00:00:00 2001 From: J-N-K Date: Wed, 17 Jan 2024 18:31:29 +0100 Subject: [PATCH] Improve RemoteAddonService and fix test (#4049) Signed-off-by: Jan N. Klug --- .../AbstractRemoteAddonService.java | 18 ++++++++++++------ .../AbstractRemoteAddonServiceTest.java | 13 +++++-------- .../marketplace/test/TestAddonService.java | 15 +++++++++------ 3 files changed, 26 insertions(+), 20 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 6414e5ad7e..5fe1369426 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 @@ -70,12 +70,18 @@ public abstract class AbstractRemoteAddonService implements AddonService { 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 version2.compareTo(version1); + try { + // 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 version2.compareTo(version1); + } catch (IllegalArgumentException e) { + // assume they are equal (for ordering) if we can't compare the versions + return 0; + } }; protected final BundleVersion coreVersion; 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 29cfd7cfc9..577aeafbf9 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 @@ -14,8 +14,7 @@ package org.openhab.core.addon.marketplace; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.*; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*; 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; @@ -246,13 +245,11 @@ public class AbstractRemoteAddonServiceTest { } @Test - public void testAddonUninstallRemovesStorageEntryOnUninstalledAddon() { + public void testUninstalledAddonIsReinstalled() { addonService.addToStorage(TEST_ADDON); addonService.getAddons(null); - addonService.uninstall(TEST_ADDON); - - checkResult(TEST_ADDON, getFullAddonId(TEST_ADDON) + "/failed", false, true); + checkResult(TEST_ADDON, getFullAddonId(TEST_ADDON) + "/installed", true, true); } // add-comparisons @@ -299,11 +296,11 @@ public class AbstractRemoteAddonServiceTest { private void checkResult(String id, String expectedEventTopic, boolean installStatus, boolean present) { // assert expected event is posted ArgumentCaptor eventCaptor = ArgumentCaptor.forClass(Event.class); - Mockito.verify(eventPublisher).post(eventCaptor.capture()); + Mockito.verify(eventPublisher, timeout(5000)).post(eventCaptor.capture()); Event event = eventCaptor.getValue(); String topic = "openhab/addons/" + expectedEventTopic; - assertThat(event.getTopic(), is(topic)); + assertThat(event.toString(), event.getTopic(), is(topic)); // assert addon handler was called (by checking it's installed status) assertThat(addonHandler.isInstalled(getFullAddonId(id)), is(installStatus)); diff --git a/bundles/org.openhab.core.addon.marketplace/src/test/java/org/openhab/core/addon/marketplace/test/TestAddonService.java b/bundles/org.openhab.core.addon.marketplace/src/test/java/org/openhab/core/addon/marketplace/test/TestAddonService.java index 9194e38e54..793180f49c 100644 --- a/bundles/org.openhab.core.addon.marketplace/src/test/java/org/openhab/core/addon/marketplace/test/TestAddonService.java +++ b/bundles/org.openhab.core.addon.marketplace/src/test/java/org/openhab/core/addon/marketplace/test/TestAddonService.java @@ -73,9 +73,12 @@ public class TestAddonService extends AbstractRemoteAddonService { @Override protected List getRemoteAddons() { remoteCalls++; - return REMOTE_ADDONS.stream().map(id -> Addon.create(SERVICE_PID + ":" + id).withType("binding") - .withId(id.substring("binding-".length())).withContentType(TestAddonHandler.TEST_ADDON_CONTENT_TYPE) - .withCompatible(!id.equals(INCOMPATIBLE_VERSION)).build()).toList(); + return REMOTE_ADDONS.stream() + .map(id -> Addon.create(SERVICE_PID + ":" + id).withType("binding").withVersion("4.1.0") + .withId(id.substring("binding-".length())) + .withContentType(TestAddonHandler.TEST_ADDON_CONTENT_TYPE) + .withCompatible(!id.equals(INCOMPATIBLE_VERSION)).build()) + .toList(); } @Override @@ -90,7 +93,7 @@ public class TestAddonService extends AbstractRemoteAddonService { @Override public @Nullable Addon getAddon(String id, @Nullable Locale locale) { - String remoteId = SERVICE_PID + ":" + id; + String remoteId = id.startsWith(SERVICE_PID) ? id : SERVICE_PID + ":" + id; return cachedAddons.stream().filter(a -> remoteId.equals(a.getUid())).findAny().orElse(null); } @@ -115,7 +118,7 @@ public class TestAddonService extends AbstractRemoteAddonService { */ public void setInstalled(String id) { Addon addon = Addon.create(SERVICE_PID + ":" + id).withType("binding").withId(id.substring("binding-".length())) - .withContentType(TestAddonHandler.TEST_ADDON_CONTENT_TYPE).build(); + .withVersion("4.1.0").withContentType(TestAddonHandler.TEST_ADDON_CONTENT_TYPE).build(); addonHandlers.forEach(addonHandler -> { try { @@ -133,7 +136,7 @@ public class TestAddonService extends AbstractRemoteAddonService { */ public void addToStorage(String id) { Addon addon = Addon.create(SERVICE_PID + ":" + id).withType("binding").withId(id.substring("binding-".length())) - .withContentType(TestAddonHandler.TEST_ADDON_CONTENT_TYPE).build(); + .withVersion("4.1.0").withContentType(TestAddonHandler.TEST_ADDON_CONTENT_TYPE).build(); addon.setInstalled(true); installedAddonStorage.put(id, gson.toJson(addon));