Fix duplicate UIDs in remote add-on services (#3961)

Signed-off-by: Jan N. Klug <github@klug.nrw>
pull/3968/head
J-N-K 2023-12-26 23:17:05 +01:00 committed by GitHub
parent ad1c37d382
commit ff6b33f3d8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 63 additions and 7 deletions

View File

@ -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<Addon> 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<String, List<Addon>> addonMap = new HashMap<>();
addons.forEach(a -> addonMap.computeIfAbsent(a.getUid(), k -> new ArrayList<>()).add(a));
for (List<Addon> partialAddonList : addonMap.values()) {
if (partialAddonList.size() > 1) {
partialAddonList.stream().sorted(BY_COMPATIBLE_AND_VERSION).skip(1).forEach(addons::remove);
}
}
cachedAddons = addons;
this.installedAddons = installedAddons;
}

View File

@ -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

View File

@ -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<Addon> actual = Stream.of(addon1, addon2, addon3, addon4)
.sorted(AbstractRemoteAddonService.BY_COMPATIBLE_AND_VERSION).toList();
List<Addon> 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
*