From 5cea59bfb100fb88ed40a02a4360995ed82e6536 Mon Sep 17 00:00:00 2001 From: J-N-K Date: Sun, 14 Jan 2024 21:46:53 +0100 Subject: [PATCH] Improve marketplace add-on handling (#4019) It has been reported several times that add-ons were not properly installed / missing after an upgrade or the installation of incompatible add-ons resulted in broken installations. After an upgrade (or clean cache) the `AddonHandler`s try to re-install the add-ons from the download cache (`/marketplace`). This happens without checking compatibility. This was needed before OH4, because the cache was the only source providing information about installed add-ons. This is now different, since we store the add-on information in a JSON database, so the UIDs of the add-ons are known. This PR changes improves the add-on services. It now 1. Reads the information about the installed add-ons from the database and sets the installation status based on information from the handlers. 2. Removes all add-ons that are not installed from the JSON database and remembers their UIDs. 3. Refreshes the remote add-on list (including check for compatibility if not disabled). 4. Tries installation of the add-ons remembered in step 2. Since incompatible add-ons are missing in the add-on list, their installation fails and a warning is logged. This PR is has two corresponding PR in openhab-distro and openhab-linuxpkg to ensure that the upgrade script and `openhab-cli` also clear the marketplace cache. Signed-off-by: Jan N. Klug --- .../AbstractRemoteAddonService.java | 103 +++++++++++------- 1 file changed, 65 insertions(+), 38 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 c177f5e4767..eb7f408b93c 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 @@ -12,6 +12,8 @@ */ package org.openhab.core.addon.marketplace; +import static org.openhab.core.common.ThreadPoolManager.THREAD_POOL_NAME_COMMON; + import java.io.IOException; import java.net.URI; import java.time.Duration; @@ -25,6 +27,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.concurrent.ScheduledExecutorService; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; @@ -36,6 +39,7 @@ import org.openhab.core.addon.AddonService; import org.openhab.core.addon.AddonType; import org.openhab.core.cache.ExpiringCache; +import org.openhab.core.common.ThreadPoolManager; import org.openhab.core.config.core.ConfigParser; import org.openhab.core.events.Event; import org.openhab.core.events.EventPublisher; @@ -82,6 +86,7 @@ public abstract class AbstractRemoteAddonService implements AddonService { protected List installedAddons = List.of(); private final Logger logger = LoggerFactory.getLogger(AbstractRemoteAddonService.class); + private final ScheduledExecutorService scheduler = ThreadPoolManager.getScheduledPool(THREAD_POOL_NAME_COMMON); protected AbstractRemoteAddonService(EventPublisher eventPublisher, ConfigurationAdmin configurationAdmin, StorageService storageService, AddonInfoRegistry addonInfoRegistry, String servicePid) { @@ -113,9 +118,14 @@ public void refreshSource() { getClass()); return; } + List addons = new ArrayList<>(); + + // retrieve add-ons that should be available from storage and check if they are really installed + // this is safe, because the {@link AddonHandler}s only report ready when they installed everything from the + // cache try { - installedAddonStorage.stream().map(this::convertFromStorage).forEach(addons::add); + installedAddonStorage.stream().map(this::convertFromStorage).peek(this::setInstalled).forEach(addons::add); } catch (JsonSyntaxException e) { List.copyOf(installedAddonStorage.getKeys()).forEach(installedAddonStorage::remove); logger.error( @@ -124,18 +134,21 @@ public void refreshSource() { refreshSource(); } + // remove not installed add-ons from the add-ons list, but remember their UIDs to re-install them + List missingAddons = addons.stream().filter(addon -> !addon.isInstalled()).map(Addon::getUid).toList(); + missingAddons.forEach(installedAddonStorage::remove); + addons.removeIf(addon -> missingAddons.contains(addon.getUid())); + // create lookup list to make sure installed addons take precedence List installedAddons = addons.stream().map(Addon::getUid).toList(); + // get the remote addons if (remoteEnabled()) { List remoteAddons = Objects.requireNonNullElse(cachedRemoteAddons.getValue(), List.of()); - remoteAddons.stream().filter(a -> !installedAddons.contains(a.getUid())).forEach(addons::add); + remoteAddons.stream().filter(a -> !installedAddons.contains(a.getUid())).peek(this::setInstalled) + .forEach(addons::add); } - // check real installation status based on handlers - addons.forEach( - addon -> addon.setInstalled(addonHandlers.stream().anyMatch(h -> h.isInstalled(addon.getUid())))); - // remove incompatible add-ons if not enabled boolean showIncompatible = includeIncompatible(); addons.removeIf(addon -> !addon.isInstalled() && !addon.getCompatible() && !showIncompatible); @@ -151,6 +164,15 @@ public void refreshSource() { cachedAddons = addons; this.installedAddons = installedAddons; + + if (!missingAddons.isEmpty()) { + logger.info("Re-installing missing add-ons from remote repository: {}", missingAddons); + scheduler.execute(() -> missingAddons.forEach(this::install)); + } + } + + private void setInstalled(Addon addon) { + addon.setInstalled(addonHandlers.stream().anyMatch(h -> h.isInstalled(addon.getUid()))); } /** @@ -199,52 +221,57 @@ public List getTypes(@Nullable Locale locale) { @Override public void install(String id) { Addon addon = getAddon(id, null); - if (addon != null) { - for (MarketplaceAddonHandler handler : addonHandlers) { - if (handler.supports(addon.getType(), addon.getContentType())) { - if (!handler.isInstalled(addon.getUid())) { - try { - handler.install(addon); - installedAddonStorage.put(id, gson.toJson(addon)); - refreshSource(); - postInstalledEvent(addon.getUid()); - } catch (MarketplaceHandlerException e) { - postFailureEvent(addon.getUid(), e.getMessage()); - } - } else { - postFailureEvent(addon.getUid(), "Add-on is already installed."); + if (addon == null) { + postFailureEvent(id, "Add-on can't be installed because it is not known."); + return; + } + for (MarketplaceAddonHandler handler : addonHandlers) { + if (handler.supports(addon.getType(), addon.getContentType())) { + if (!handler.isInstalled(addon.getUid())) { + try { + handler.install(addon); + addon.setInstalled(true); + installedAddonStorage.put(id, gson.toJson(addon)); + refreshSource(); + postInstalledEvent(addon.getUid()); + } catch (MarketplaceHandlerException e) { + postFailureEvent(addon.getUid(), e.getMessage()); } - return; + } else { + postFailureEvent(addon.getUid(), "Add-on is already installed."); } + return; } } - postFailureEvent(id, "Add-on not known."); + postFailureEvent(id, "Add-on can't be installed because there is no handler for it."); } @Override public void uninstall(String id) { Addon addon = getAddon(id, null); - if (addon != null) { - for (MarketplaceAddonHandler handler : addonHandlers) { - if (handler.supports(addon.getType(), addon.getContentType())) { - if (handler.isInstalled(addon.getUid())) { - try { - handler.uninstall(addon); - installedAddonStorage.remove(id); - refreshSource(); - postUninstalledEvent(addon.getUid()); - } catch (MarketplaceHandlerException e) { - postFailureEvent(addon.getUid(), e.getMessage()); - } - } else { + if (addon == null) { + postFailureEvent(id, "Add-on can't be uninstalled because it is not known."); + return; + } + for (MarketplaceAddonHandler handler : addonHandlers) { + if (handler.supports(addon.getType(), addon.getContentType())) { + if (handler.isInstalled(addon.getUid())) { + try { + handler.uninstall(addon); installedAddonStorage.remove(id); - postFailureEvent(addon.getUid(), "Add-on is not installed."); + refreshSource(); + postUninstalledEvent(addon.getUid()); + } catch (MarketplaceHandlerException e) { + postFailureEvent(addon.getUid(), e.getMessage()); } - return; + } else { + installedAddonStorage.remove(id); + postFailureEvent(addon.getUid(), "Add-on is not installed."); } + return; } } - postFailureEvent(id, "Add-on not known."); + postFailureEvent(id, "Add-on can't be uninstalled because there is no handler for it."); } @Override