Skip to content

Commit

Permalink
Improve RemoteAddonService and fix test (#4049)
Browse files Browse the repository at this point in the history
Signed-off-by: Jan N. Klug <github@klug.nrw>
Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
  • Loading branch information
J-N-K authored and Ciprian Pascu committed Jan 17, 2024
1 parent 89817bd commit 71fd2b4
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@

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;
Expand Down Expand Up @@ -246,13 +245,11 @@ public void testAddonUninstallFailsOnUnknownAddon() {
}

@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
Expand Down Expand Up @@ -299,11 +296,11 @@ private Addon getMockedAddon(String version, boolean compatible) {
private void checkResult(String id, String expectedEventTopic, boolean installStatus, boolean present) {
// assert expected event is posted
ArgumentCaptor<Event> 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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,12 @@ public void removeAddonHandler(MarketplaceAddonHandler handler) {
@Override
protected List<Addon> 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
Expand All @@ -90,7 +93,7 @@ public String getName() {

@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);
}

Expand All @@ -115,7 +118,7 @@ public int getRemoteCalls() {
*/
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 {
Expand All @@ -133,7 +136,7 @@ public void setInstalled(String id) {
*/
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));
Expand Down

0 comments on commit 71fd2b4

Please sign in to comment.