From 444d73ee0a17bf441408e5378c7cf13b4e0911e3 Mon Sep 17 00:00:00 2001 From: Nadahar Date: Wed, 8 Jan 2025 20:06:53 +0100 Subject: [PATCH 1/5] Create Version and VersionRange and replace BundleVersion Signed-off-by: Ravi Nadahar --- .../eclipse/internal/EclipseAddonService.java | 3 +- .../AbstractRemoteAddonService.java | 35 +- .../core/addon/marketplace/BundleVersion.java | 160 ------ .../CommunityBundleAddonHandler.java | 5 +- .../CommunityMarketplaceAddonService.java | 73 +-- .../CommunityUIWidgetAddonHandler.java | 2 +- .../internal/json/JsonAddonService.java | 12 +- .../AbstractRemoteAddonServiceTest.java | 3 +- .../addon/marketplace/BundleVersionTest.java | 107 ---- .../marketplace/test/TestAddonService.java | 19 +- .../java/org/openhab/core/addon/Addon.java | 140 ++--- .../org/openhab/core/addon/dto/AddonDTO.java | 170 ++++++ .../addon/internal/JarFileAddonService.java | 3 +- .../org/openhab/core/addon/AddonTest.java | 198 +++++++ .../AddonConsoleCommandExtension.java | 2 +- .../core/internal/addons/AddonResource.java | 15 +- .../karaf/internal/KarafAddonService.java | 4 +- .../java/org/openhab/core/common/Version.java | 490 ++++++++++++++++++ .../org/openhab/core/common/VersionRange.java | 249 +++++++++ .../openhab/core/common/VersionRangeTest.java | 110 ++++ .../org/openhab/core/common/VersionTest.java | 142 +++++ 21 files changed, 1533 insertions(+), 409 deletions(-) delete mode 100644 bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/BundleVersion.java delete mode 100644 bundles/org.openhab.core.addon.marketplace/src/test/java/org/openhab/core/addon/marketplace/BundleVersionTest.java create mode 100644 bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/dto/AddonDTO.java create mode 100644 bundles/org.openhab.core.addon/src/test/java/org/openhab/core/addon/AddonTest.java create mode 100644 bundles/org.openhab.core/src/main/java/org/openhab/core/common/Version.java create mode 100644 bundles/org.openhab.core/src/main/java/org/openhab/core/common/VersionRange.java create mode 100644 bundles/org.openhab.core/src/test/java/org/openhab/core/common/VersionRangeTest.java create mode 100644 bundles/org.openhab.core/src/test/java/org/openhab/core/common/VersionTest.java diff --git a/bundles/org.openhab.core.addon.eclipse/src/main/java/org/openhab/core/addon/eclipse/internal/EclipseAddonService.java b/bundles/org.openhab.core.addon.eclipse/src/main/java/org/openhab/core/addon/eclipse/internal/EclipseAddonService.java index daf13dd8c1a..9b8d99a02d2 100644 --- a/bundles/org.openhab.core.addon.eclipse/src/main/java/org/openhab/core/addon/eclipse/internal/EclipseAddonService.java +++ b/bundles/org.openhab.core.addon.eclipse/src/main/java/org/openhab/core/addon/eclipse/internal/EclipseAddonService.java @@ -32,6 +32,7 @@ import org.openhab.core.addon.AddonInfoRegistry; import org.openhab.core.addon.AddonService; import org.openhab.core.addon.AddonType; +import org.openhab.core.common.Version; import org.openhab.core.config.core.ConfigurableService; import org.osgi.framework.Bundle; import org.osgi.framework.BundleContext; @@ -136,7 +137,7 @@ private Addon getAddon(Bundle bundle, @Nullable Locale locale) { String uid = type + Addon.ADDON_SEPARATOR + name; Addon.Builder addon = Addon.create(ADDON_ID_PREFIX + uid).withType(type).withId(name) - .withContentType(ADDONS_CONTENT_TYPE).withVersion(bundle.getVersion().toString()) + .withContentType(ADDONS_CONTENT_TYPE).withVersion(Version.valueOf(bundle.getVersion())) .withAuthor(ADDONS_AUTHOR, true).withInstalled(true); AddonInfo addonInfo = addonInfoRegistry.getAddonInfo(uid, locale); 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 b016f3a87b0..c4130096d4b 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 @@ -36,7 +36,9 @@ import org.openhab.core.addon.AddonInfoRegistry; import org.openhab.core.addon.AddonService; import org.openhab.core.addon.AddonType; +import org.openhab.core.addon.dto.AddonDTO; import org.openhab.core.common.ThreadPoolManager; +import org.openhab.core.common.Version; import org.openhab.core.config.core.ConfigParser; import org.openhab.core.events.Event; import org.openhab.core.events.EventPublisher; @@ -67,21 +69,11 @@ public abstract class AbstractRemoteAddonService implements AddonService { if (compatible != 0) { return compatible; } - 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; - } + Version v2 = addon2.getVersion(); + return v2 == null ? 1 : v2.compareTo(addon1.getVersion()); }; - protected final BundleVersion coreVersion; + protected final Version coreVersion; protected final Gson gson = new GsonBuilder().setDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'").create(); protected final Set addonHandlers = new CopyOnWriteArraySet<>(); @@ -105,12 +97,12 @@ protected AbstractRemoteAddonService(EventPublisher eventPublisher, Configuratio this.coreVersion = getCoreVersion(); } - protected BundleVersion getCoreVersion() { - return new BundleVersion(FrameworkUtil.getBundle(OpenHAB.class).getVersion().toString()); + protected Version getCoreVersion() { + return Version.valueOf(FrameworkUtil.getBundle(OpenHAB.class).getVersion()); } private Addon convertFromStorage(Map.Entry entry) { - Addon storedAddon = Objects.requireNonNull(gson.fromJson(entry.getValue(), Addon.class)); + Addon storedAddon = Objects.requireNonNull(gson.fromJson(entry.getValue(), AddonDTO.class)).toAddon(); AddonInfo addonInfo = addonInfoRegistry.getAddonInfo(storedAddon.getType() + "-" + storedAddon.getId()); if (addonInfo != null && storedAddon.getConfigDescriptionURI().isBlank()) { return Addon.create(storedAddon).withConfigDescriptionURI(addonInfo.getConfigDescriptionURI()).build(); @@ -125,7 +117,7 @@ public void refreshSource() { } private synchronized void refreshSource(boolean fetchRemoteAddons) { - if (!addonHandlers.stream().allMatch(MarketplaceAddonHandler::isReady)) { + if (addonHandlers.isEmpty() || !addonHandlers.stream().allMatch(MarketplaceAddonHandler::isReady)) { logger.debug("Add-on service '{}' tried to refresh source before add-on handlers ready. Exiting.", getClass()); return; @@ -177,7 +169,8 @@ private synchronized void refreshSource(boolean fetchRemoteAddons) { // check and remove duplicate uids Map> addonMap = new HashMap<>(); - addons.forEach(a -> addonMap.computeIfAbsent(a.getUid(), k -> new ArrayList<>()).add(a)); + addons.forEach( + a -> Objects.requireNonNull(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); @@ -250,11 +243,13 @@ public void install(String id) { try { handler.install(addon); addon.setInstalled(true); - installedAddonStorage.put(id, gson.toJson(addon)); + installedAddonStorage.put(id, gson.toJson(new AddonDTO(addon))); refreshSource(false); postInstalledEvent(addon.getUid()); } catch (MarketplaceHandlerException e) { postFailureEvent(addon.getUid(), e.getMessage()); + logger.warn("Failed to install add-on \"{}\": {}", addon.getUid(), e.getMessage()); + logger.trace("", e); } } else { postFailureEvent(addon.getUid(), "Add-on is already installed."); @@ -282,6 +277,8 @@ public void uninstall(String id) { postUninstalledEvent(addon.getUid()); } catch (MarketplaceHandlerException e) { postFailureEvent(addon.getUid(), e.getMessage()); + logger.warn("Failed to uninstall add-on \"{}\": {}", addon.getUid(), e.getMessage()); + logger.trace("", e); } } else { installedAddonStorage.remove(id); diff --git a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/BundleVersion.java b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/BundleVersion.java deleted file mode 100644 index 671b1ef1007..00000000000 --- a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/BundleVersion.java +++ /dev/null @@ -1,160 +0,0 @@ -/* - * Copyright (c) 2010-2026 Contributors to the openHAB project - * - * See the NOTICE file(s) distributed with this work for additional - * information. - * - * This program and the accompanying materials are made available under the - * terms of the Eclipse Public License 2.0 which is available at - * http://www.eclipse.org/legal/epl-2.0 - * - * SPDX-License-Identifier: EPL-2.0 - */ -package org.openhab.core.addon.marketplace; - -import java.util.Objects; -import java.util.regex.Matcher; -import java.util.regex.Pattern; - -import org.eclipse.jdt.annotation.NonNullByDefault; -import org.eclipse.jdt.annotation.Nullable; - -/** - * The {@link BundleVersion} wraps a bundle version and provides a method to compare them - * - * @author Jan N. Klug - Initial contribution - */ -@NonNullByDefault -public class BundleVersion { - private static final Pattern VERSION_PATTERN = Pattern.compile( - "(?\\d+)\\.(?\\d+)\\.(?\\d+)(\\.((?RC)|(?M))?(?\\d+))?"); - public static final Pattern RANGE_PATTERN = Pattern.compile( - "\\[(?\\d+\\.\\d+(?\\.\\d+(\\.\\w+)?)?);(?\\d+\\.\\d+(?\\.\\d+(\\.\\w+)?)?)(?[)\\]])"); - - private final String version; - private final int major; - private final int minor; - private final int micro; - private final @Nullable Long qualifier; - - public BundleVersion(String version) { - Matcher matcher = VERSION_PATTERN.matcher(version); - if (matcher.matches()) { - this.version = version; - this.major = Integer.parseInt(matcher.group("major")); - this.minor = Integer.parseInt(matcher.group("minor")); - this.micro = Integer.parseInt(matcher.group("micro")); - String qualifier = matcher.group("qualifier"); - if (qualifier != null) { - long intQualifier = Long.parseLong(qualifier); - if (matcher.group("rc") != null) { - // we can safely assume that there are less than Integer.MAX_VALUE milestones - // so RCs are always newer than milestones - // since snapshot qualifiers are larger than 10*Integer.MAX_VALUE they are - // still considered newer - this.qualifier = intQualifier + Integer.MAX_VALUE; - } else { - this.qualifier = intQualifier; - } - } else { - this.qualifier = null; - } - } else { - throw new IllegalArgumentException("Input does not match pattern"); - } - } - - /** - * Test if this version is within the provided range - * - * @param range a Maven like version range - * @return {@code true} if this version is inside range, {@code false} otherwise - * @throws IllegalArgumentException if {@code range} does not represent a valid range - */ - public boolean inRange(@Nullable String range) throws IllegalArgumentException { - if (range == null || range.isBlank()) { - // if no range is given, we assume the range covers everything - return true; - } - Matcher matcher = RANGE_PATTERN.matcher(range); - if (!matcher.matches()) { - throw new IllegalArgumentException(range + "is not a valid version range"); - } - String startString = matcher.group("startmicro") != null ? matcher.group("start") - : matcher.group("start") + ".0"; - BundleVersion startVersion = new BundleVersion(startString); - if (this.compareTo(startVersion) < 0) { - return false; - } - - String endString = matcher.group("endmicro") != null ? matcher.group("end") : matcher.group("stop") + ".0"; - boolean inclusive = "]".equals(matcher.group("endtype")); - BundleVersion endVersion = new BundleVersion(endString); - int comparison = this.compareTo(endVersion); - return (inclusive && comparison == 0) || comparison < 0; - } - - @Override - public boolean equals(@Nullable Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - BundleVersion version = (BundleVersion) o; - return major == version.major && minor == version.minor && micro == version.micro - && Objects.equals(qualifier, version.qualifier); - } - - @Override - public int hashCode() { - return Objects.hash(major, minor, micro, qualifier); - } - - /** - * Compares two bundle versions - * - * @param other the other bundle version - * @return a positive integer if this version is newer than the other version, a negative number if this version is - * older than the other version and 0 if the versions are equal - */ - public int compareTo(BundleVersion other) { - int result = major - other.major; - if (result != 0) { - return result; - } - - result = minor - other.minor; - if (result != 0) { - return result; - } - - result = micro - other.micro; - if (result != 0) { - return result; - } - - if (Objects.equals(qualifier, other.qualifier)) { - return 0; - } - - // the release is always newer than a milestone or snapshot - Long thisQualifier = qualifier; - if (thisQualifier == null) { // we are the release - return 1; - } - Long otherQualifier = other.qualifier; - if (otherQualifier == null) { // the other is the release - return -1; - } - - // both versions are milestones, we can compare them - return Long.compare(thisQualifier, otherQualifier); - } - - @Override - public String toString() { - return version; - } -} diff --git a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityBundleAddonHandler.java b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityBundleAddonHandler.java index 0764f0f02ca..8d9009e91a1 100644 --- a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityBundleAddonHandler.java +++ b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityBundleAddonHandler.java @@ -89,8 +89,9 @@ public void install(Addon addon) throws MarketplaceHandlerException { } catch (IllegalArgumentException | MalformedURLException | URISyntaxException e) { throw new MarketplaceHandlerException("Malformed source URL: " + e.getMessage(), e); } - addBundleToCache(addon.getUid(), sourceUrl); - installFromCache(bundleContext, addon.getUid()); + String addonId = addon.getUid(); + addBundleToCache(addonId, sourceUrl); + installFromCache(bundleContext, addonId); } @Override diff --git a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityMarketplaceAddonService.java b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityMarketplaceAddonService.java index 950581f212a..4b745dbb0d7 100644 --- a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityMarketplaceAddonService.java +++ b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityMarketplaceAddonService.java @@ -39,7 +39,6 @@ import org.openhab.core.addon.AddonService; import org.openhab.core.addon.AddonType; import org.openhab.core.addon.marketplace.AbstractRemoteAddonService; -import org.openhab.core.addon.marketplace.BundleVersion; import org.openhab.core.addon.marketplace.MarketplaceAddonHandler; import org.openhab.core.addon.marketplace.internal.community.model.DiscourseCategoryResponseDTO; import org.openhab.core.addon.marketplace.internal.community.model.DiscourseCategoryResponseDTO.DiscoursePosterInfo; @@ -47,6 +46,7 @@ import org.openhab.core.addon.marketplace.internal.community.model.DiscourseCategoryResponseDTO.DiscourseUser; import org.openhab.core.addon.marketplace.internal.community.model.DiscourseTopicResponseDTO; import org.openhab.core.addon.marketplace.internal.community.model.DiscourseTopicResponseDTO.DiscoursePostLink; +import org.openhab.core.common.VersionRange; import org.openhab.core.config.core.ConfigParser; import org.openhab.core.config.core.ConfigurableService; import org.openhab.core.events.EventPublisher; @@ -94,7 +94,7 @@ public class CommunityMarketplaceAddonService extends AbstractRemoteAddonService private static final String ADDON_ID_PREFIX = SERVICE_ID + ":"; private static final Pattern CODE_MARKUP_PATTERN = Pattern.compile( - "[a-z]+)\">(?.*?)", + "[-a-zA-Z]+)\">(?.*?)\\n?", Pattern.DOTALL); private static final Integer BUNDLES_CATEGORY = 73; @@ -225,6 +225,8 @@ protected List getRemoteAddons() { return convertTopicToAddon(parsed); } } catch (Exception e) { + logger.debug("An error occurred while creating add-on for '{}': {}", uid, e.getMessage()); + logger.trace("", e); return null; } } @@ -303,23 +305,19 @@ private String getContentType(@Nullable Integer category, List tags) { String title = topic.title; boolean compatible = true; - int compatibilityStart = topic.title.lastIndexOf("["); // version range always starts with [ - if (topic.title.lastIndexOf(" ") < compatibilityStart) { // check includes [ not present - String potentialRange = topic.title.substring(compatibilityStart); - Matcher matcher = BundleVersion.RANGE_PATTERN.matcher(potentialRange); - if (matcher.matches()) { - try { - compatible = coreVersion.inRange(potentialRange); - title = topic.title.substring(0, compatibilityStart).trim(); - logger.debug("{} is {}compatible with core version {}", topic.title, compatible ? "" : "NOT ", - coreVersion); - } catch (IllegalArgumentException e) { - logger.debug("Failed to determine compatibility for addon {}: {}", topic.title, e.getMessage()); - compatible = true; - } - } else { - logger.debug("Range pattern does not match '{}'", potentialRange); + Matcher matcher = VersionRange.RANGE_PATTERN.matcher(title); + if (matcher.find()) { + try { + compatible = VersionRange.valueOf(matcher.group().trim()).includes(coreVersion); + title = title.substring(0, matcher.start()); + logger.debug("{} is {}compatible with core version {}", topic.title, compatible ? "" : "NOT ", + coreVersion); + } catch (IllegalArgumentException e) { + logger.debug("Failed to determine compatibility for add-on {}: {}", topic.title, e.getMessage()); + compatible = true; } + } else { + logger.trace("No version range pattern found for add-on {}", topic.title); } String link = COMMUNITY_TOPIC_URL + topic.id.toString(); @@ -362,8 +360,8 @@ private String getContentType(@Nullable Integer category, List tags) { * @return the unescaped content */ private String unescapeEntities(String content) { - return content.replace(""", "\"").replace("&", "&").replace("'", "'").replace("<", "<") - .replace(">", ">"); + return content.replace(""", "\"").replace("'", "'").replace("<", "<").replace(">", ">") + .replace("&", "&"); } /** @@ -390,9 +388,15 @@ private Addon convertTopicToAddon(DiscourseTopicResponseDTO topic) { String maturity = tags.stream().filter(CODE_MATURITY_LEVELS::contains).findAny().orElse(null); Map properties = new HashMap<>(10); - properties.put("created_at", createdDate); - properties.put("updated_at", updatedDate); - properties.put("last_posted", lastPostedDate); + if (createdDate != null) { + properties.put("created_at", createdDate); + } + if (updatedDate != null) { + properties.put("updated_at", updatedDate); + } + if (lastPostedDate != null) { + properties.put("last_posted", lastPostedDate); + } properties.put("like_count", likeCount); properties.put("views", views); properties.put("posts_count", postsCount); @@ -404,18 +408,18 @@ private Addon convertTopicToAddon(DiscourseTopicResponseDTO topic) { // try to extract contents or links if (topic.postStream.posts[0].linkCounts != null) { for (DiscoursePostLink postLink : topic.postStream.posts[0].linkCounts) { - if (postLink.url.endsWith(".jar")) { + if (postLink.url.toLowerCase(Locale.ROOT).endsWith(".jar")) { properties.put(JAR_DOWNLOAD_URL_PROPERTY, postLink.url); id = determineIdFromUrl(postLink.url); } - if (postLink.url.endsWith(".kar")) { + if (postLink.url.toLowerCase(Locale.ROOT).endsWith(".kar")) { properties.put(KAR_DOWNLOAD_URL_PROPERTY, postLink.url); id = determineIdFromUrl(postLink.url); } - if (postLink.url.endsWith(".json")) { + if (postLink.url.toLowerCase(Locale.ROOT).endsWith(".json")) { properties.put(JSON_DOWNLOAD_URL_PROPERTY, postLink.url); } - if (postLink.url.endsWith(".yaml")) { + if (postLink.url.toLowerCase(Locale.ROOT).endsWith(".yaml")) { properties.put(YAML_DOWNLOAD_URL_PROPERTY, postLink.url); } } @@ -436,17 +440,16 @@ private Addon convertTopicToAddon(DiscourseTopicResponseDTO topic) { .anyMatch(handler -> handler.supports(type, contentType) && handler.isInstalled(uid)); String title = topic.title; - int compatibilityStart = topic.title.lastIndexOf("["); // version range always starts with [ - if (topic.title.lastIndexOf(" ") < compatibilityStart) { // check includes [ not present - String potentialRange = topic.title.substring(compatibilityStart); - Matcher matcher = BundleVersion.RANGE_PATTERN.matcher(potentialRange); - if (matcher.matches()) { - title = topic.title.substring(0, compatibilityStart).trim(); - } + boolean compatible = true; + Matcher matcher = VersionRange.RANGE_PATTERN.matcher(title); + if (matcher.find()) { + compatible = VersionRange.valueOf(matcher.group().trim()).includes(coreVersion); + title = matcher.replaceFirst("").trim(); } Addon.Builder builder = Addon.create(uid).withType(type).withId(id).withContentType(contentType) - .withLabel(title).withImageLink(topic.imageUrl).withLink(COMMUNITY_TOPIC_URL + topic.id.toString()) + .withCompatible(compatible).withLabel(title).withImageLink(topic.imageUrl) + .withLink(COMMUNITY_TOPIC_URL + topic.id.toString()) .withAuthor(topic.postStream.posts[0].displayUsername).withMaturity(maturity) .withDetailedDescription(detailedDescription).withInstalled(installed).withProperties(properties); diff --git a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityUIWidgetAddonHandler.java b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityUIWidgetAddonHandler.java index 3b781dc57b6..135b5085146 100644 --- a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityUIWidgetAddonHandler.java +++ b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityUIWidgetAddonHandler.java @@ -118,7 +118,7 @@ public void install(Addon addon) throws MarketplaceHandlerException { throw new MarketplaceHandlerException("Widget cannot be downloaded.", e); } catch (Exception e) { logger.error("Widget from marketplace is invalid: {}", e.getMessage()); - throw new MarketplaceHandlerException("Widget is not valid.", e); + throw new MarketplaceHandlerException("Failed to install widget: " + e.getMessage(), e); } } 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 d4163301a1c..29655345d80 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 @@ -38,6 +38,8 @@ import org.openhab.core.addon.marketplace.AbstractRemoteAddonService; import org.openhab.core.addon.marketplace.MarketplaceAddonHandler; import org.openhab.core.addon.marketplace.internal.json.model.AddonEntryDTO; +import org.openhab.core.common.Version; +import org.openhab.core.common.VersionRange; import org.openhab.core.config.core.ConfigParser; import org.openhab.core.config.core.ConfigurableService; import org.openhab.core.events.EventPublisher; @@ -199,16 +201,18 @@ private Addon fromAddonEntry(AddonEntryDTO addonEntry) { boolean compatible = true; try { - compatible = coreVersion.inRange(addonEntry.compatibleVersions); + compatible = VersionRange.valueOf(addonEntry.compatibleVersions).includes(coreVersion); } catch (IllegalArgumentException e) { logger.debug("Failed to determine compatibility for addon {}: {}", addonEntry.id, e.getMessage()); } + Version v = addonEntry.version == null || addonEntry.version.isBlank() ? null + : Version.valueOf(addonEntry.version); return Addon.create(uid).withType(addonEntry.type).withId(addonEntry.id).withInstalled(installed) .withDetailedDescription(addonEntry.description).withContentType(addonEntry.contentType) - .withAuthor(addonEntry.author).withVersion(addonEntry.version).withLabel(addonEntry.title) - .withCompatible(compatible).withMaturity(addonEntry.maturity).withProperties(properties) - .withLink(addonEntry.link).withImageLink(addonEntry.imageUrl).withKeywords(addonEntry.keywords) + .withAuthor(addonEntry.author).withVersion(v).withLabel(addonEntry.title).withCompatible(compatible) + .withMaturity(addonEntry.maturity).withProperties(properties).withLink(addonEntry.link) + .withImageLink(addonEntry.imageUrl).withKeywords(addonEntry.keywords) .withConfigDescriptionURI(addonEntry.configDescriptionURI).withLoggerPackages(addonEntry.loggerPackages) .withConnection(addonEntry.connection).withCountries(addonEntry.countries).build(); } 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 6f8991ae0dc..9e05f388b08 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 @@ -44,6 +44,7 @@ import org.openhab.core.addon.AddonInfoRegistry; import org.openhab.core.addon.marketplace.test.TestAddonHandler; import org.openhab.core.addon.marketplace.test.TestAddonService; +import org.openhab.core.common.Version; import org.openhab.core.events.Event; import org.openhab.core.events.EventPublisher; import org.openhab.core.storage.Storage; @@ -283,7 +284,7 @@ public void testSnapshotVersionsAreParsedProperly() { private Addon getMockedAddon(String version, boolean compatible) { Addon addon = mock(Addon.class); - when(addon.getVersion()).thenReturn(version); + when(addon.getVersion()).thenReturn(Version.valueOf(version)); when(addon.getCompatible()).thenReturn(compatible); return addon; } diff --git a/bundles/org.openhab.core.addon.marketplace/src/test/java/org/openhab/core/addon/marketplace/BundleVersionTest.java b/bundles/org.openhab.core.addon.marketplace/src/test/java/org/openhab/core/addon/marketplace/BundleVersionTest.java deleted file mode 100644 index ad9465f9ac2..00000000000 --- a/bundles/org.openhab.core.addon.marketplace/src/test/java/org/openhab/core/addon/marketplace/BundleVersionTest.java +++ /dev/null @@ -1,107 +0,0 @@ -/* - * Copyright (c) 2010-2026 Contributors to the openHAB project - * - * See the NOTICE file(s) distributed with this work for additional - * information. - * - * This program and the accompanying materials are made available under the - * terms of the Eclipse Public License 2.0 which is available at - * http://www.eclipse.org/legal/epl-2.0 - * - * SPDX-License-Identifier: EPL-2.0 - */ -package org.openhab.core.addon.marketplace; - -import static org.hamcrest.CoreMatchers.is; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.greaterThan; -import static org.hamcrest.Matchers.lessThan; -import static org.junit.jupiter.api.Assertions.assertThrows; - -import java.util.stream.Stream; - -import org.eclipse.jdt.annotation.NonNullByDefault; -import org.eclipse.jdt.annotation.Nullable; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.MethodSource; -import org.mockito.junit.jupiter.MockitoExtension; -import org.mockito.junit.jupiter.MockitoSettings; -import org.mockito.quality.Strictness; - -/** - * The {@link BundleVersionTest} contains tests for the {@link BundleVersion} class - * - * @author Jan N. Klug - Initial contribution - */ -@NonNullByDefault -@ExtendWith(MockitoExtension.class) -@MockitoSettings(strictness = Strictness.LENIENT) -public class BundleVersionTest { - - private static Stream provideCompareVersionsArguments() { - return Stream.of( // - Arguments.of("3.1.0", "3.1.0", Result.EQUAL), // same versions are equal - Arguments.of("3.1.0", "3.0.2", Result.NEWER), // minor version is more important than micro - Arguments.of("3.7.0", "4.0.1.202105311711", Result.OLDER), // major version is more important than minor - Arguments.of("3.9.1.M1", "3.9.0.M5", Result.NEWER), // micro version is more important than qualifier - Arguments.of("3.0.0.202105311032", "3.0.0.202106011144", Result.OLDER), // snapshots - Arguments.of("3.1.0.M3", "3.1.0.M1", Result.NEWER), // milestones are compared numerically - Arguments.of("3.1.0.M1", "3.1.0.197705310021", Result.OLDER), // snapshot is newer than milestone - Arguments.of("3.3.0", "3.3.0.202206302115", Result.NEWER), // release is newer than snapshot - Arguments.of("3.3.0", "3.3.0.RC1", Result.NEWER), // releases are newer than release candidates - Arguments.of("3.3.0.M5", "3.3.0.RC1", Result.OLDER), // milestones are older than release candidates - Arguments.of("3.3.0.RC2", "3.3.0.202305201715", Result.OLDER) // snapshots are newer than release - // candidates - ); - } - - @Test - public void testIllegalRangeThrowsException() { - BundleVersion bundleVersion = new BundleVersion("3.1.0"); - assertThrows(IllegalArgumentException.class, () -> bundleVersion.inRange("illegal")); - } - - @ParameterizedTest - @MethodSource("provideCompareVersionsArguments") - public void testCompareVersions(String v1, String v2, Result result) { - BundleVersion version1 = new BundleVersion(v1); - BundleVersion version2 = new BundleVersion(v2); - switch (result) { - case OLDER: - assertThat(version1.compareTo(version2), lessThan(0)); - break; - case NEWER: - assertThat(version1.compareTo(version2), greaterThan(0)); - break; - case EQUAL: - assertThat(version1.compareTo(version2), is(0)); - break; - } - } - - private static Stream provideInRangeArguments() { - return Stream.of(Arguments.of("[3.1.0;3.2.1)", true), // in range - Arguments.of("[3.1.0;3.2.0)", false), // at end of range, non-inclusive - Arguments.of("[3.1.0;3.2.0]", true), // at end of range, inclusive - Arguments.of("[3.1.0;3.1.5)", false), // above range - Arguments.of("[3.3.0;3.4.0)", false), // below range - Arguments.of("", true), // empty range assumes in range - Arguments.of(null, true)); - } - - @ParameterizedTest - @MethodSource("provideInRangeArguments") - public void inRangeTest(@Nullable String range, boolean result) { - BundleVersion frameworkVersion = new BundleVersion("3.2.0"); - assertThat(frameworkVersion.inRange(range), is(result)); - } - - private enum Result { - OLDER, - NEWER, - EQUAL - } -} 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 a5a2fc6c250..18262208236 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 @@ -21,10 +21,11 @@ import org.eclipse.jdt.annotation.Nullable; import org.openhab.core.addon.Addon; import org.openhab.core.addon.AddonInfoRegistry; +import org.openhab.core.addon.dto.AddonDTO; import org.openhab.core.addon.marketplace.AbstractRemoteAddonService; -import org.openhab.core.addon.marketplace.BundleVersion; import org.openhab.core.addon.marketplace.MarketplaceAddonHandler; import org.openhab.core.addon.marketplace.MarketplaceHandlerException; +import org.openhab.core.common.Version; import org.openhab.core.events.EventPublisher; import org.openhab.core.storage.StorageService; import org.osgi.service.cm.ConfigurationAdmin; @@ -56,8 +57,8 @@ public TestAddonService(EventPublisher eventPublisher, ConfigurationAdmin config } @Override - protected BundleVersion getCoreVersion() { - return new BundleVersion("3.2.0"); + protected Version getCoreVersion() { + return Version.valueOf("3.2.0"); } @Override @@ -74,8 +75,8 @@ public void removeAddonHandler(MarketplaceAddonHandler handler) { protected List getRemoteAddons() { remoteCalls++; return REMOTE_ADDONS.stream() - .map(id -> Addon.create(SERVICE_PID + ":" + id).withType("binding").withVersion("4.1.0") - .withId(id.substring("binding-".length())) + .map(id -> Addon.create(SERVICE_PID + ":" + id).withType("binding") + .withVersion(Version.valueOf("4.1.0")).withId(id.substring("binding-".length())) .withContentType(TestAddonHandler.TEST_ADDON_CONTENT_TYPE) .withCompatible(!id.equals(INCOMPATIBLE_VERSION)).build()) .toList(); @@ -118,7 +119,8 @@ public int getRemoteCalls() { */ public void setInstalled(String id) { Addon addon = Addon.create(SERVICE_PID + ":" + id).withType("binding").withId(id.substring("binding-".length())) - .withVersion("4.1.0").withContentType(TestAddonHandler.TEST_ADDON_CONTENT_TYPE).build(); + .withVersion(Version.valueOf("4.1.0")).withContentType(TestAddonHandler.TEST_ADDON_CONTENT_TYPE) + .build(); addonHandlers.forEach(addonHandler -> { try { @@ -136,9 +138,10 @@ public void setInstalled(String id) { */ public void addToStorage(String id) { Addon addon = Addon.create(SERVICE_PID + ":" + id).withType("binding").withId(id.substring("binding-".length())) - .withVersion("4.1.0").withContentType(TestAddonHandler.TEST_ADDON_CONTENT_TYPE).build(); + .withVersion(Version.valueOf("4.1.0")).withContentType(TestAddonHandler.TEST_ADDON_CONTENT_TYPE) + .build(); addon.setInstalled(true); - installedAddonStorage.put(id, gson.toJson(addon)); + installedAddonStorage.put(id, gson.toJson(new AddonDTO(addon))); } } diff --git a/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/Addon.java b/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/Addon.java index 6196eac8090..b4984695d77 100644 --- a/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/Addon.java +++ b/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/Addon.java @@ -18,7 +18,9 @@ import java.util.Objects; import java.util.Set; +import org.eclipse.jdt.annotation.NonNull; import org.eclipse.jdt.annotation.Nullable; +import org.openhab.core.common.Version; /** * This class defines an add-on. @@ -34,14 +36,14 @@ public class Addon { private final String id; private final String label; - private final String version; + private final @Nullable Version version; private final @Nullable String maturity; private final boolean compatible; private final String contentType; private final @Nullable String link; private final String author; private final boolean verifiedAuthor; - private boolean installed; + private volatile boolean installed; private final String type; private final @Nullable String description; private final @Nullable String detailedDescription; @@ -84,19 +86,19 @@ public class Addon { * @param loggerPackages a {@link List} containing the package names belonging to this add-on * @throws IllegalArgumentException when a mandatory parameter is invalid */ - private Addon(String uid, String type, String id, String label, String version, @Nullable String maturity, + Addon(String uid, String type, String id, String label, @Nullable Version version, @Nullable String maturity, boolean compatible, String contentType, @Nullable String link, String author, boolean verifiedAuthor, boolean installed, @Nullable String description, @Nullable String detailedDescription, String configDescriptionURI, String keywords, List countries, @Nullable String license, String connection, @Nullable String backgroundColor, @Nullable String imageLink, @Nullable Map properties, List loggerPackages) { - if (uid.isBlank()) { + if (uid == null || uid.isBlank()) { throw new IllegalArgumentException("uid must not be empty"); } - if (type.isBlank()) { + if (type == null || type.isBlank()) { throw new IllegalArgumentException("type must not be empty"); } - if (id.isBlank()) { + if (id == null || id.isBlank()) { throw new IllegalArgumentException("id must not be empty"); } @@ -111,19 +113,20 @@ private Addon(String uid, String type, String id, String label, String version, this.contentType = contentType; this.description = description; this.detailedDescription = detailedDescription; - this.configDescriptionURI = configDescriptionURI; - this.keywords = keywords; - this.countries = countries; + this.configDescriptionURI = configDescriptionURI == null || configDescriptionURI.isBlank() ? "" + : configDescriptionURI; + this.keywords = keywords == null || keywords.isBlank() ? "" : keywords; + this.countries = countries == null ? List.of() : List.copyOf(countries); this.license = license; - this.connection = connection; + this.connection = connection == null || connection.isBlank() ? "" : connection; this.backgroundColor = backgroundColor; this.link = link; this.imageLink = imageLink; - this.author = author; + this.author = author == null || author.isBlank() ? "" : author; this.verifiedAuthor = verifiedAuthor; this.installed = installed; - this.properties = properties == null ? Map.of() : properties; - this.loggerPackages = loggerPackages; + this.properties = properties == null ? Map.of() : Map.copyOf(properties); + this.loggerPackages = loggerPackages == null ? List.of() : List.copyOf(loggerPackages); } /** @@ -178,7 +181,7 @@ public boolean isVerifiedAuthor() { /** * The version of the add-on */ - public String getVersion() { + public @Nullable Version getVersion() { return version; } @@ -305,61 +308,64 @@ public static Builder create(String uid) { } public static Builder create(Addon addon) { - Addon.Builder builder = new Builder(addon.uid); - builder.id = addon.id; - builder.label = addon.label; - builder.version = addon.version; - builder.maturity = addon.maturity; - builder.compatible = addon.compatible; - builder.contentType = addon.contentType; - builder.link = addon.link; - builder.author = addon.author; - builder.verifiedAuthor = addon.verifiedAuthor; - builder.installed = addon.installed; - builder.type = addon.type; - builder.description = addon.description; - builder.detailedDescription = addon.detailedDescription; - builder.configDescriptionURI = addon.configDescriptionURI; - builder.keywords = addon.keywords; - builder.countries = addon.countries; - builder.license = addon.license; - builder.connection = addon.connection; - builder.backgroundColor = addon.backgroundColor; - builder.imageLink = addon.imageLink; - builder.properties = addon.properties; - builder.loggerPackages = addon.loggerPackages; - return builder; + return new Builder(addon); } public static class Builder { - private final String uid; - private String id; - private String label; - private String version = ""; + private final @NonNull String uid; + private @Nullable String id; + private @Nullable String label; + private @Nullable Version version; private @Nullable String maturity; private boolean compatible = true; - private String contentType; + private @Nullable String contentType; private @Nullable String link; - private String author = ""; + private @Nullable String author; private boolean verifiedAuthor = false; private boolean installed = false; - private String type; + private @Nullable String type; private @Nullable String description; private @Nullable String detailedDescription; - private String configDescriptionURI = ""; - private String keywords = ""; - private List countries = List.of(); + private @Nullable String configDescriptionURI; + private @Nullable String keywords; + private @Nullable List<@NonNull String> countries = List.of(); private @Nullable String license; - private String connection = ""; + private @Nullable String connection; private @Nullable String backgroundColor; private @Nullable String imageLink; - private Map properties = new HashMap<>(); - private List loggerPackages = List.of(); + private @Nullable Map<@NonNull String, @NonNull Object> properties; + private @Nullable List<@NonNull String> loggerPackages = List.of(); - private Builder(String uid) { + private Builder(@NonNull String uid) { this.uid = uid; } + private Builder(Addon addon) { + this.uid = addon.uid; + this.id = addon.id; + this.label = addon.label; + this.version = addon.version; + this.maturity = addon.maturity; + this.compatible = addon.compatible; + this.contentType = addon.contentType; + this.link = addon.link; + this.author = addon.author; + this.verifiedAuthor = addon.verifiedAuthor; + this.installed = addon.installed; + this.type = addon.type; + this.description = addon.description; + this.detailedDescription = addon.detailedDescription; + this.configDescriptionURI = addon.configDescriptionURI; + this.keywords = addon.keywords; + this.countries = addon.countries; + this.license = addon.license; + this.connection = addon.connection; + this.backgroundColor = addon.backgroundColor; + this.imageLink = addon.imageLink; + this.properties = addon.properties; + this.loggerPackages = addon.loggerPackages; + } + public Builder withType(String type) { this.type = type; return this; @@ -375,7 +381,7 @@ public Builder withLabel(String label) { return this; } - public Builder withVersion(String version) { + public Builder withVersion(@Nullable Version version) { this.version = version; return this; } @@ -436,7 +442,11 @@ public Builder withKeywords(String keywords) { return this; } - public Builder withCountries(List countries) { + public @Nullable List<@NonNull String> getCountries() { + return countries; + } + + public Builder withCountries(List<@NonNull String> countries) { this.countries = countries; return this; } @@ -461,17 +471,26 @@ public Builder withImageLink(@Nullable String imageLink) { return this; } - public Builder withProperty(String key, Object value) { - this.properties.put(key, value); + public Builder withProperty(@NonNull String key, @NonNull Object value) { + Map<@NonNull String, @NonNull Object> props = this.properties; + if (props == null) { + props = new HashMap<>(); + } + props.put(key, value); + this.properties = props; return this; } - public Builder withProperties(Map properties) { - this.properties.putAll(properties); + public Builder withProperties(@Nullable Map<@NonNull String, @NonNull Object> properties) { + this.properties = properties; return this; } - public Builder withLoggerPackages(List loggerPackages) { + public @Nullable List<@NonNull String> getLoggerPackages() { + return loggerPackages; + } + + public Builder withLoggerPackages(@Nullable List<@NonNull String> loggerPackages) { this.loggerPackages = loggerPackages; return this; } @@ -479,8 +498,7 @@ public Builder withLoggerPackages(List loggerPackages) { public Addon build() { return new Addon(uid, type, id, label, version, maturity, compatible, contentType, link, author, verifiedAuthor, installed, description, detailedDescription, configDescriptionURI, keywords, - countries, license, connection, backgroundColor, imageLink, - properties.isEmpty() ? null : properties, loggerPackages); + countries, license, connection, backgroundColor, imageLink, properties, loggerPackages); } } } diff --git a/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/dto/AddonDTO.java b/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/dto/AddonDTO.java new file mode 100644 index 00000000000..d09a5a11de4 --- /dev/null +++ b/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/dto/AddonDTO.java @@ -0,0 +1,170 @@ +/* + * Copyright (c) 2010-2026 Contributors to the openHAB project + * + * See the NOTICE file(s) distributed with this work for additional + * information. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + */ +package org.openhab.core.addon.dto; + +import java.util.List; +import java.util.Map; + +import org.eclipse.jdt.annotation.NonNull; +import org.openhab.core.addon.Addon; +import org.openhab.core.common.Version; + +/** + * This is a data transfer object that is used to serialize add-ons. + * + * @author Ravi Nadahar - Initial contribution + */ +public class AddonDTO { + + public String uid; + public String id; + public String label; + public String version; + public String maturity; + public boolean compatible; + public String contentType; + public String link; + public String author; + public boolean verifiedAuthor; + public boolean installed; + public String type; + public String description; + public String detailedDescription; + public String configDescriptionURI; + public String keywords; + public List<@NonNull String> countries; + public String license; + public String connection; + public String backgroundColor; + public String imageLink; + public Map<@NonNull String, @NonNull Object> properties; + public List<@NonNull String> loggerPackages; + + /** + * Create a new, empty {@link AddonDTO} instance. + */ + public AddonDTO() { + } + + /** + * Create a new {@link AddonDTO} instance from the specified {@link Addon}. + * + * @param addon the {@link Addon}. + */ + public AddonDTO(@NonNull Addon addon) { + this.uid = addon.getUid(); + this.id = addon.getId(); + this.label = addon.getLabel(); + Version v = addon.getVersion(); + if (v != null) { + this.version = v.toString(); + } + this.maturity = addon.getMaturity(); + this.compatible = addon.getCompatible(); + this.contentType = addon.getContentType(); + this.link = addon.getLink(); + this.author = addon.getAuthor(); + this.verifiedAuthor = addon.isVerifiedAuthor(); + this.installed = addon.isInstalled(); + this.type = addon.getType(); + this.description = addon.getDescription(); + this.detailedDescription = addon.getDetailedDescription(); + this.configDescriptionURI = addon.getConfigDescriptionURI(); + this.keywords = addon.getKeywords(); + List<@NonNull String> stringList = addon.getCountries(); + if (!stringList.isEmpty()) { + this.countries = stringList; + } + this.license = addon.getLicense(); + this.connection = addon.getConnection(); + this.backgroundColor = addon.getBackgroundColor(); + this.imageLink = addon.getImageLink(); + Map<@NonNull String, @NonNull Object> map = addon.getProperties(); + if (!map.isEmpty()) { + this.properties = map; + } + stringList = addon.getLoggerPackages(); + if (!stringList.isEmpty()) { + this.loggerPackages = stringList; + } + } + + /** + * Create a new {@link Addon} instance from this {@link AddonDTO}. + * + * @return The new {@link Addon} instance. + */ + public @NonNull Addon toAddon() { + Addon.Builder b = Addon.create(this.uid); + if (this.id != null) { + b.withId(this.id); + } + if (this.label != null) { + b.withLabel(this.label); + } + if (this.version != null) { + b.withVersion(Version.valueOf(this.version)); + } + if (this.maturity != null) { + b.withMaturity(this.maturity); + } + b.withCompatible(this.compatible); + if (this.contentType != null) { + b.withContentType(this.contentType); + } + if (this.link != null) { + b.withLink(this.link); + } + if (this.author != null) { + b.withAuthor(this.author, this.verifiedAuthor); + } + b.withInstalled(this.installed); + if (this.type != null) { + b.withType(this.type); + } + if (this.description != null) { + b.withDescription(this.description); + } + if (this.detailedDescription != null) { + b.withDetailedDescription(this.detailedDescription); + } + if (this.configDescriptionURI != null) { + b.withConfigDescriptionURI(this.configDescriptionURI); + } + if (this.keywords != null) { + b.withKeywords(this.keywords); + } + if (this.countries != null) { + b.withCountries(this.countries); + } + if (this.license != null) { + b.withLicense(this.license); + } + if (this.connection != null) { + b.withConnection(this.connection); + } + if (this.backgroundColor != null) { + b.withBackgroundColor(this.backgroundColor); + } + if (this.imageLink != null) { + b.withImageLink(this.imageLink); + } + if (this.properties != null) { + b.withProperties(this.properties); + } + if (this.loggerPackages != null) { + b.withLoggerPackages(this.loggerPackages); + } + return b.build(); + } +} diff --git a/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/JarFileAddonService.java b/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/JarFileAddonService.java index b2759297cea..99e73bd3272 100644 --- a/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/JarFileAddonService.java +++ b/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/JarFileAddonService.java @@ -31,6 +31,7 @@ import org.openhab.core.addon.AddonService; import org.openhab.core.addon.AddonType; import org.openhab.core.common.ThreadPoolManager; +import org.openhab.core.common.Version; import org.osgi.framework.Bundle; import org.osgi.framework.BundleContext; import org.osgi.framework.BundleEvent; @@ -153,7 +154,7 @@ public synchronized void refreshSource() { private Addon toAddon(Bundle bundle, AddonInfo addonInfo) { String uid = ADDON_ID_PREFIX + addonInfo.getUID(); return Addon.create(uid).withId(addonInfo.getId()).withType(addonInfo.getType()).withInstalled(true) - .withVersion(bundle.getVersion().toString()).withLabel(addonInfo.getName()) + .withVersion(Version.valueOf(bundle.getVersion())).withLabel(addonInfo.getName()) .withConnection(addonInfo.getConnection()).withCountries(addonInfo.getCountries()) .withConfigDescriptionURI(addonInfo.getConfigDescriptionURI()) .withDescription(Objects.requireNonNullElse(addonInfo.getDescription(), bundle.getSymbolicName())) diff --git a/bundles/org.openhab.core.addon/src/test/java/org/openhab/core/addon/AddonTest.java b/bundles/org.openhab.core.addon/src/test/java/org/openhab/core/addon/AddonTest.java new file mode 100644 index 00000000000..84c8dc9c24e --- /dev/null +++ b/bundles/org.openhab.core.addon/src/test/java/org/openhab/core/addon/AddonTest.java @@ -0,0 +1,198 @@ +/* + * Copyright (c) 2010-2026 Contributors to the openHAB project + * + * See the NOTICE file(s) distributed with this work for additional + * information. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + */ +package org.openhab.core.addon; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.hasEntry; +import static org.junit.jupiter.api.Assertions.*; + +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; +import java.util.SortedMap; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.junit.jupiter.MockitoSettings; +import org.mockito.quality.Strictness; +import org.openhab.core.common.Version; + +/** + * The {@link AddonTest} contains tests for the {@link Addon} class. + * + * @author - Initial contribution + */ +@NonNullByDefault +@ExtendWith(MockitoExtension.class) +@MockitoSettings(strictness = Strictness.LENIENT) +public class AddonTest { + + @Test + public void testBasics() { + assertThrows(IllegalArgumentException.class, () -> new Addon(null, null, null, null, null, null, false, null, + null, null, false, false, null, null, null, null, null, null, null, null, null, null, null)); + assertThrows(IllegalArgumentException.class, () -> new Addon(" ", null, null, null, null, null, false, null, + null, null, false, false, null, null, null, null, null, null, null, null, null, null, null)); + assertThrows(IllegalArgumentException.class, () -> new Addon("test", null, null, null, null, null, false, null, + null, null, false, false, null, null, null, null, null, null, null, null, null, null, null)); + assertThrows(IllegalArgumentException.class, () -> new Addon("test", "\t", null, null, null, null, false, null, + null, null, false, false, null, null, null, null, null, null, null, null, null, null, null)); + assertThrows(IllegalArgumentException.class, () -> new Addon("test", "binding", null, null, null, null, false, + null, null, null, false, false, null, null, null, null, null, null, null, null, null, null, null)); + assertThrows(IllegalArgumentException.class, () -> new Addon("test", "binding", "", null, null, null, false, + null, null, null, false, false, null, null, null, null, null, null, null, null, null, null, null)); + + Addon addon = new Addon("testuid", "binding", "testid", null, null, null, false, null, null, null, false, false, + null, null, null, null, null, null, null, null, null, null, null); + assertTrue(addon.getProperties().isEmpty()); + + addon = new Addon("testuid", "binding", "testid", null, null, null, false, null, null, null, false, false, null, + null, null, null, List.of("DE", "PL", "UA"), null, null, null, null, null, null); + assertEquals(3, addon.getCountries().size()); + assertTrue(addon.getProperties().isEmpty()); + + addon = new Addon("testuid", "binding", "testid", null, null, null, false, null, null, null, false, false, null, + null, null, null, List.of("DE", "PL", "UA"), null, null, null, null, + Map.of("key1", "value1", "key2", "value2"), null); + assertEquals(3, addon.getCountries().size()); + assertEquals(2, addon.getProperties().size()); + + addon = new Addon("testuid", "binding", "testid", null, null, null, false, null, null, null, false, false, null, + null, null, null, List.of("DE", "PL", "UA"), null, null, null, null, + Map.of("key1", "value1", "key2", "value2"), List.of("com.example.addon")); + assertEquals(3, addon.getCountries().size()); + assertEquals(2, addon.getProperties().size()); + assertEquals(1, addon.getLoggerPackages().size()); + + addon = new Addon("testuid", "automation", "testid", "Test", Version.valueOf("0.9"), "stable", false, + "application/x-test", "http://example.com", "Santa", true, false, "None", "Still none", null, + "nothing, none", List.of("US"), "GPL", "none", "red", "http://image.exammple.com", null, + List.of("com.example")); + assertEquals(1, addon.getCountries().size()); + assertEquals(0, addon.getProperties().size()); + assertEquals(1, addon.getLoggerPackages().size()); + assertEquals("testuid", addon.getUid()); + assertEquals("automation", addon.getType()); + assertEquals("testid", addon.getId()); + assertEquals("Test", addon.getLabel()); + assertEquals(Version.valueOf("0.9"), addon.getVersion()); + assertEquals("stable", addon.getMaturity()); + assertFalse(addon.getCompatible()); + assertEquals("application/x-test", addon.getContentType()); + assertEquals("http://example.com", addon.getLink()); + assertEquals("Santa", addon.getAuthor()); + assertTrue(addon.isVerifiedAuthor()); + assertFalse(addon.isInstalled()); + assertEquals("None", addon.getDescription()); + assertEquals("Still none", addon.getDetailedDescription()); + assertMapsEquals(Map.of(), addon.getProperties()); + assertEquals("nothing, none", addon.getKeywords()); + assertIterableEquals(List.of("US"), addon.getCountries()); + assertEquals("GPL", addon.getLicense()); + assertEquals("none", addon.getConnection()); + assertEquals("red", addon.getBackgroundColor()); + assertEquals("http://image.exammple.com", addon.getImageLink()); + assertEquals("", addon.getConfigDescriptionURI()); + assertIterableEquals(List.of("com.example"), addon.getLoggerPackages()); + addon.setInstalled(true); + assertTrue(addon.isInstalled()); + } + + @Test + public void testBuilder() { + Addon.Builder b = Addon.create("uid"); + assertThrows(IllegalArgumentException.class, () -> b.build()); + assertThrows(IllegalArgumentException.class, () -> b.withType("ui").build()); + assertEquals("ui", b.withId("id").build().getType()); + assertEquals("TLabel", b.withLabel("TLabel").build().getLabel()); + assertEquals(Version.EMPTY_VERSION, b.withVersion(new Version(0, 0, 0)).build().getVersion()); + assertEquals("beta", b.withMaturity("beta").build().getMaturity()); + assertTrue(b.withCompatible(true).build().getCompatible()); + assertEquals("img/gif", b.withContentType("img/gif").build().getContentType()); + assertEquals("http://link.example.com", b.withLink("http://link.example.com").build().getLink()); + assertEquals("Nadar", b.withAuthor("Nadar").build().getAuthor()); + assertTrue(b.withInstalled(true).build().isInstalled()); + assertEquals("Nadar", b.withAuthor("Nadar", true).build().getAuthor()); + assertTrue(b.build().isVerifiedAuthor()); + assertEquals("Description", b.withDescription("Description").build().getDescription()); + assertEquals("Detailed description", + b.withDetailedDescription("Detailed description").build().getDetailedDescription()); + assertEquals("", b.withConfigDescriptionURI(null).build().getConfigDescriptionURI()); + assertEquals("smart, light", b.withKeywords("smart, light").build().getKeywords()); + assertTrue(b.withCountries(null).build().getCountries().isEmpty()); + assertNull(b.getCountries()); + assertEquals("EPL", b.withLicense("EPL").build().getLicense()); + assertEquals("local", b.withConnection("local").build().getConnection()); + assertEquals("green", b.withBackgroundColor("green").build().getBackgroundColor()); + assertEquals("http://image.example.com", b.withImageLink("http://image.example.com").build().getImageLink()); + assertMapsEquals(Map.of("priority", Double.valueOf(2d)), + b.withProperty("priority", Double.valueOf(2d)).build().getProperties()); + b.withProperties(Map.of("link", "http://example.com", "fresh", Boolean.FALSE)); + assertThat(b.build().getProperties(), hasEntry("link", "http://example.com")); + assertThat(b.build().getProperties(), hasEntry("fresh", Boolean.FALSE)); + assertIterableEquals(List.of("com.example.basic", "com.example.advanced"), + b.withLoggerPackages(List.of("com.example.basic", "com.example.advanced")).build().getLoggerPackages()); + assertIterableEquals(List.of("com.example.basic", "com.example.advanced"), b.getLoggerPackages()); + + Addon addon = b.build(); + Addon addon2 = Addon.create(addon).withCompatible(false).build(); + assertTrue(addon.getCompatible()); + assertFalse(addon2.getCompatible()); + + assertEquals(addon.getType(), addon2.getType()); + assertEquals(addon.getLabel(), addon2.getLabel()); + assertEquals(addon.getVersion(), addon2.getVersion()); + assertEquals(addon.getMaturity(), addon2.getMaturity()); + assertEquals(addon.getContentType(), addon2.getContentType()); + assertEquals(addon.getLink(), addon2.getLink()); + assertEquals(addon.getAuthor(), addon2.getAuthor()); + assertEquals(addon.isVerifiedAuthor(), addon2.isVerifiedAuthor()); + assertEquals(addon.isInstalled(), addon2.isInstalled()); + assertEquals(addon.getDescription(), addon2.getDescription()); + assertEquals(addon.getDetailedDescription(), addon2.getDetailedDescription()); + assertEquals(addon.getConfigDescriptionURI(), addon2.getConfigDescriptionURI()); + assertEquals(addon.getKeywords(), addon2.getKeywords()); + assertIterableEquals(addon.getCountries(), addon2.getCountries()); + assertEquals(addon.getLicense(), addon2.getLicense()); + assertEquals(addon.getConnection(), addon2.getConnection()); + assertEquals(addon.getBackgroundColor(), addon2.getBackgroundColor()); + assertEquals(addon.getImageLink(), addon2.getImageLink()); + assertMapsEquals(addon.getProperties(), addon2.getProperties()); + assertIterableEquals(addon.getLoggerPackages(), addon2.getLoggerPackages()); + } + + private void assertMapsEquals(@Nullable Map a, @Nullable Map b) { + if (a == null || b == null) { + assertTrue(a == null && b == null); + return; + } + assertEquals(a.size(), b.size()); + if (a instanceof SortedMap && b instanceof SortedMap) { + Iterator iterator = b.entrySet().iterator(); + Object o; + for (Entry entry : a.entrySet()) { + o = iterator.next(); + assertEquals(entry.getKey(), ((Entry) o).getKey()); + assertEquals(entry.getValue(), ((Entry) o).getValue()); + } + } else { + for (Entry entry : a.entrySet()) { + assertEquals(entry.getValue(), b.get(entry.getKey())); + } + } + } +} diff --git a/bundles/org.openhab.core.io.console/src/main/java/org/openhab/core/io/console/internal/extension/AddonConsoleCommandExtension.java b/bundles/org.openhab.core.io.console/src/main/java/org/openhab/core/io/console/internal/extension/AddonConsoleCommandExtension.java index 03f94de861f..47d3433cb0c 100644 --- a/bundles/org.openhab.core.io.console/src/main/java/org/openhab/core/io/console/internal/extension/AddonConsoleCommandExtension.java +++ b/bundles/org.openhab.core.io.console/src/main/java/org/openhab/core/io/console/internal/extension/AddonConsoleCommandExtension.java @@ -141,7 +141,7 @@ private void listAddons(Console console, String serviceId) { addons = service.getAddons(null); } addons.forEach(addon -> console.println(String.format("%s %-45s %-20s %s", addon.isInstalled() ? "i" : " ", - addon.getUid(), addon.getVersion().isBlank() ? "not set" : addon.getVersion(), addon.getLabel()))); + addon.getUid(), addon.getVersion() == null ? "not set" : addon.getVersion(), addon.getLabel()))); } private void installAddon(Console console, String addonUid) { diff --git a/bundles/org.openhab.core.io.rest.core/src/main/java/org/openhab/core/io/rest/core/internal/addons/AddonResource.java b/bundles/org.openhab.core.io.rest.core/src/main/java/org/openhab/core/io/rest/core/internal/addons/AddonResource.java index 56ba18d6d3d..84a7df0dff1 100644 --- a/bundles/org.openhab.core.io.rest.core/src/main/java/org/openhab/core/io/rest/core/internal/addons/AddonResource.java +++ b/bundles/org.openhab.core.io.rest.core/src/main/java/org/openhab/core/io/rest/core/internal/addons/AddonResource.java @@ -50,6 +50,7 @@ import org.openhab.core.addon.AddonInfoRegistry; import org.openhab.core.addon.AddonService; import org.openhab.core.addon.AddonType; +import org.openhab.core.addon.dto.AddonDTO; import org.openhab.core.auth.Role; import org.openhab.core.common.ThreadPoolManager; import org.openhab.core.config.core.ConfigDescription; @@ -151,7 +152,7 @@ protected void removeAddonService(AddonService featureService) { @GET @Produces(MediaType.APPLICATION_JSON) @Operation(operationId = "getAddons", summary = "Get all add-ons.", responses = { - @ApiResponse(responseCode = "200", description = "OK", content = @Content(array = @ArraySchema(schema = @Schema(implementation = Addon.class)))), + @ApiResponse(responseCode = "200", description = "OK", content = @Content(array = @ArraySchema(schema = @Schema(implementation = AddonDTO.class)))), @ApiResponse(responseCode = "404", description = "Service not found") }) public Response getAddon( @HeaderParam("Accept-Language") @Parameter(description = "language") @Nullable String language, @@ -184,7 +185,7 @@ public Response getAddon( addons = addons.filter(Addon::isInstalled); } - return Response.ok(new Stream2JSONInputStream(addons)).build(); + return Response.ok(new Stream2JSONInputStream(addons.map(a -> new AddonDTO(a)))).build(); } @GET @@ -204,7 +205,7 @@ public Response getServices( @Path("/suggestions") @Produces(MediaType.APPLICATION_JSON) @Operation(operationId = "getSuggestedAddons", summary = "Get suggested add-ons to be installed.", responses = { - @ApiResponse(responseCode = "200", description = "OK", content = @Content(array = @ArraySchema(schema = @Schema(implementation = Addon.class)))), }) + @ApiResponse(responseCode = "200", description = "OK", content = @Content(array = @ArraySchema(schema = @Schema(implementation = AddonInfo.class)))), }) public Response getSuggestions( @HeaderParam("Accept-Language") @Parameter(description = "language") @Nullable String language) { logger.debug("Received HTTP GET request at '{}'", uriInfo.getPath()); @@ -243,7 +244,7 @@ public Response getTypes( @Path("/{addonId: [a-zA-Z_0-9-:]+}") @Produces(MediaType.APPLICATION_JSON) @Operation(operationId = "getAddonById", summary = "Get add-on with given ID.", responses = { - @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = Addon.class))), + @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = AddonDTO.class))), @ApiResponse(responseCode = "404", description = "Not found") }) public Response getById( @HeaderParam("Accept-Language") @Parameter(description = "language") @Nullable String language, @@ -257,7 +258,7 @@ public Response getById( } Addon responseObject = addonService.getAddon(addonId, locale); if (responseObject != null) { - return Response.ok(responseObject).build(); + return Response.ok(new AddonDTO(responseObject)).build(); } return Response.status(HttpStatus.NOT_FOUND_404).build(); @@ -279,7 +280,7 @@ public Response installAddon(final @PathParam("addonId") @Parameter(description try { addonService.install(addonId); } catch (Exception e) { - logger.error("Exception while installing add-on: {}", e.getMessage()); + logger.error("An error occurred while installing add-on '{}': {}", addonId, e.getMessage()); postFailureEvent(addonId, e.getMessage()); } }); @@ -429,7 +430,7 @@ private void postFailureEvent(String addonId, @Nullable String msg) { .findFirst().orElse(addonServices.stream().findFirst().orElse(null)); } - private Stream getAllAddons(Locale locale) { + private Stream getAllAddons(@Nullable Locale locale) { return addonServices.stream().map(s -> s.getAddons(locale)).flatMap(Collection::stream); } diff --git a/bundles/org.openhab.core.karaf/src/main/java/org/openhab/core/karaf/internal/KarafAddonService.java b/bundles/org.openhab.core.karaf/src/main/java/org/openhab/core/karaf/internal/KarafAddonService.java index a00eff2f826..7ad284e91fb 100644 --- a/bundles/org.openhab.core.karaf/src/main/java/org/openhab/core/karaf/internal/KarafAddonService.java +++ b/bundles/org.openhab.core.karaf/src/main/java/org/openhab/core/karaf/internal/KarafAddonService.java @@ -32,6 +32,7 @@ import org.openhab.core.addon.AddonInfoRegistry; import org.openhab.core.addon.AddonService; import org.openhab.core.addon.AddonType; +import org.openhab.core.common.Version; import org.osgi.service.component.annotations.Activate; import org.osgi.service.component.annotations.Component; import org.osgi.service.component.annotations.Reference; @@ -130,7 +131,8 @@ private Addon getAddon(Feature feature, @Nullable Locale locale) { boolean isInstalled = featuresService.isInstalled(feature); Addon.Builder addon = Addon.create(uid).withType(type).withId(name).withContentType(ADDONS_CONTENT_TYPE) - .withVersion(feature.getVersion()).withAuthor(ADDONS_AUTHOR, true).withInstalled(isInstalled); + .withVersion(Version.valueOf(feature.getVersion())).withAuthor(ADDONS_AUTHOR, true) + .withInstalled(isInstalled); AddonInfo addonInfo = addonInfoRegistry.getAddonInfo(uid, locale); diff --git a/bundles/org.openhab.core/src/main/java/org/openhab/core/common/Version.java b/bundles/org.openhab.core/src/main/java/org/openhab/core/common/Version.java new file mode 100644 index 00000000000..493eab6ca8c --- /dev/null +++ b/bundles/org.openhab.core/src/main/java/org/openhab/core/common/Version.java @@ -0,0 +1,490 @@ +/* + * Copyright (c) 2010-2026 Contributors to the openHAB project + * + * See the NOTICE file(s) distributed with this work for additional + * information. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + */ +package org.openhab.core.common; + +import java.util.Objects; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; + +/** + * This class holds version information in a standardized form, which can easily be compared, sorted or checked for + * equality. It allows for consistent, system-wide treatment of versions. It also bridges with + * {@link org.osgi.framework.Version} by having to and from conversion methods. + *

+ * This class is immutable. + * + * @author Ravi Nadahar - Initial contribution + */ +@NonNullByDefault +public class Version implements Comparable { + + /** The {@link Pattern} used to parse version strings */ + public static final Pattern VERSION_PATTERN = Pattern.compile( + "^\\s*(?\\d+)(?:\\s*\\.\\s*(?\\d+)(?:\\s*\\.\\s*(?\\d+)(?:\\s*(?\\.|-|_)\\s*(?[a-zA-Z0-9_-]+))?)?)?\\s*$"); + protected static final Pattern QUALIFIER_PATTERN = Pattern.compile("^[a-zA-Z0-9_-]+$"); + protected static final Pattern RC_PATTERN = Pattern.compile("(?i)rc(\\d+)"); + protected static final Pattern MILESTONE_PATTERN = Pattern.compile("(?i)m(\\d+)"); + protected static final Pattern SNAPSHOT_PATTERN = Pattern.compile("(?i)snapshot"); + protected static final char SEPARATOR = '.'; + + protected final int major; + protected final int minor; + protected final int micro; + protected final String qualifier; + protected final char lastSeparator; + private transient volatile @Nullable String versionString; + private transient volatile int hash; + + /** + * The "empty" version, {@code 0.0.0}. + */ + public static final Version EMPTY_VERSION = new Version(0, 0, 0); + + /** + * Create a new instance by parsing the specified version string. + * + * @param version the version string to parse. + * @throws IllegalArgumentException If a version syntax can't be parsed from {@code version}. + */ + public Version(String version) { + String s; + Matcher m = VERSION_PATTERN.matcher(version); + if (!m.find()) { + throw new IllegalArgumentException("Invalid version format \"" + version + '"'); + } + + major = Integer.parseInt(m.group("major")); + minor = (s = m.group("minor")) == null ? 0 : Integer.parseInt(s); + micro = (s = m.group("micro")) == null ? 0 : Integer.parseInt(s); + lastSeparator = (s = m.group("lastSeparator")) == null ? '.' : s.charAt(0); + qualifier = (s = m.group("qualifier")) == null ? "" : s; + } + + /** + * Create a new instance with the specified major, minor and micro values. + * + * @param major the major version. + * @param minor the minor version. + * @param micro the micro version. + * @throws IllegalArgumentException If {@code major}, {@code minor} or {@code micro} is negative. + */ + public Version(int major, int minor, int micro) { + this(major, minor, micro, null); + } + + /** + * Create a new instance with specified major, minor and micro values, and an optional qualifier value. + * + * @param major the major version. + * @param minor the minor version. + * @param micro the micro version. + * @param qualifier the qualifier. + * @throws IllegalArgumentException If {@code major}, {@code minor} or {@code micro} is negative, or if + * {@code qualifier} is invalid (not in {@code [a-zA-Z0-9_-]}). + */ + public Version(int major, int minor, int micro, @Nullable String qualifier) { + this(major, minor, micro, '.', qualifier); + } + + /** + * Create a new instance with specified major, minor and micro values, and an optional qualifier value. + * + * @param major the major version. + * @param minor the minor version. + * @param micro the micro version. + * @param lastSeparator the last separator, which can differ from the others and be one of "{@code .}", + * "{@code -}" or "{@code _}". + * @param qualifier the qualifier. + * @throws IllegalArgumentException If {@code major}, {@code minor} or {@code micro} is negative, if + * {@code lastSeparator} is invalid (not in {@code [._-]}), or if {@code qualifier} is invalid (not in + * {@code [a-zA-Z0-9_-]}). + */ + public Version(int major, int minor, int micro, char lastSeparator, @Nullable String qualifier) { + if (major < 0) { + throw new IllegalArgumentException("Major version cannot be negative: " + major); + } + if (minor < 0) { + throw new IllegalArgumentException("Minor version cannot be negative:" + minor); + } + if (micro < 0) { + throw new IllegalArgumentException("Micro version cannot be negative:" + micro); + } + if (lastSeparator != '.' && lastSeparator != '-' && lastSeparator != '_') { + throw new IllegalArgumentException("Invalid last separator: \"" + lastSeparator + '"'); + } + this.major = major; + this.minor = minor; + this.micro = micro; + this.lastSeparator = lastSeparator; + if (qualifier == null || qualifier.isEmpty()) { + this.qualifier = ""; + } else { + if (!QUALIFIER_PATTERN.matcher(qualifier).find()) { + throw new IllegalArgumentException("Invalid qualifier: \"" + qualifier + '"'); + } + this.qualifier = qualifier; + } + } + + /** + * @return The major version. + */ + public int getMajor() { + return major; + } + + /** + * @return The minor version. + */ + public int getMinor() { + return minor; + } + + /** + * @return The micro version. + */ + public int getMicro() { + return micro; + } + + /** + * @return The last separator, which can differ from the others and be one of "{@code .}", "{@code -}" or + * "{@code _}". + */ + public char getLastSeparator() { + return lastSeparator; + } + + /** + * @return The qualifier. + */ + public String getQualifier() { + return qualifier; + } + + /** + * Convert this {@link Version} to an {@link org.osgi.framework.Version} instance. + * + * @return The corresponding {@link org.osgi.framework.Version} instance. + */ + public org.osgi.framework.Version toOSGiVersion() { + return new org.osgi.framework.Version(major, minor, micro, qualifier); + } + + @Override + public int hashCode() { + int h = hash; + if (h != 0) { + return h; + } + return hash = Objects.hash(major, micro, minor, lastSeparator, qualifier); + } + + @Override + public boolean equals(@Nullable Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof Version)) { + return false; + } + Version other = (Version) obj; + return lastSeparator == other.lastSeparator && major == other.major && micro == other.micro + && minor == other.minor && Objects.equals(qualifier, other.qualifier); + } + + @Override + public String toString() { + String s = versionString; + if (s != null) { + return s; + } + int qLen = qualifier.length(); + StringBuilder sb = new StringBuilder(20 + qLen); + sb.append(major).append(SEPARATOR).append(minor).append(SEPARATOR).append(micro); + if (qLen > 0) { + sb.append(lastSeparator).append(qualifier); + } + return versionString = sb.toString(); + } + + /** + * {@inheritDoc} + * + * @implNote This implementation is inconsistent with equals for {@code lastSeparator}, which isn't taken into + * account when comparing versions. + */ + @Override + @SuppressWarnings("PMD.CompareObjectsWithEquals") + public int compareTo(@Nullable Version other) { + if (other == this) { + return 0; + } + if (other == null) { + return 1; + } + + int result = major - other.major; + if (result != 0) { + return result; + } + + result = minor - other.minor; + if (result != 0) { + return result; + } + + result = micro - other.micro; + if (result != 0) { + return result; + } + + if (qualifier.equals(other.qualifier)) { + // Includes if both are empty + return 0; + } + if (qualifier.isEmpty()) { + // Release is newer + return 1; + } + if (other.qualifier.isEmpty()) { + // Release is newer + return -1; + } + + Matcher rcMatcher = RC_PATTERN.matcher(qualifier); + Matcher orcMatcher = RC_PATTERN.matcher(other.qualifier); + Matcher msMatcher = MILESTONE_PATTERN.matcher(qualifier); + Matcher omsMatcher = MILESTONE_PATTERN.matcher(other.qualifier); + boolean rc = rcMatcher.matches(); + boolean orc = orcMatcher.matches(); + boolean ms = msMatcher.matches(); + boolean oms = omsMatcher.matches(); + if (rc && orc) { + // Both are release candidates + int n = Integer.valueOf(rcMatcher.group(1)); + int on = Integer.valueOf(orcMatcher.group(1)); + return Integer.compare(n, on); + } + if (ms && oms) { + // Both are milestones + int n = Integer.valueOf(msMatcher.group(1)); + int on = Integer.valueOf(omsMatcher.group(1)); + return Integer.compare(n, on); + } + + if (rc && oms) { + // Release candidate is newer than milestone + return 1; + } + if (ms && orc) { + // Milestone is older than release candidate + return -1; + } + + long ql, oql; + try { + ql = Long.parseLong(qualifier); + } catch (NumberFormatException e) { + ql = Long.MIN_VALUE; + } + try { + oql = Long.parseLong(other.qualifier); + } catch (NumberFormatException e) { + oql = Long.MIN_VALUE; + } + + if (ql >= 0 && oql >= 0) { + // Both are positive integers, compare numerically + return Long.compare(ql, oql); + } + + boolean ss = SNAPSHOT_PATTERN.matcher(qualifier).matches(); + boolean oss = SNAPSHOT_PATTERN.matcher(other.qualifier).matches(); + if (ql < 0 && oql < 0) { + // Both aren't positive integers, snapshots are newer, otherwise do a simple string comparison + if (ss) { + // Snapshots are newer + return 1; + } + if (oss) { + // Non-snapshots are older + return -1; + } + + // If both are snapshots but have different case, compare them to remain consistent with equals() + return qualifier.compareTo(other.qualifier); + } + + if (ss) { + // Snapshots are newer than numbers + return 1; + } + + if (oss) { + // Numbers are older than snapshots + return -1; + } + + // Numbers are newer than non-numbers + return ql >= 0 ? 1 : -1; + } + + /** + * Compare this with a {@link org.osgi.framework.Version}. + * + * @param other the {@link org.osgi.framework.Version} to compare with. + * @return A negative integer, zero, or a positive integer as this instance is less than, equal to, or greater than + * the specified {@link org.osgi.framework.Version}. + */ + public int compareTo(org.osgi.framework.Version other) { + int result = major - other.getMajor(); + if (result != 0) { + return result; + } + + result = minor - other.getMinor(); + if (result != 0) { + return result; + } + + result = micro - other.getMicro(); + if (result != 0) { + return result; + } + + String oq = other.getQualifier(); + + if (qualifier.equals(oq)) { + // Includes if both are empty + return 0; + } + if (qualifier.isEmpty()) { + // Release is newer + return 1; + } + if (oq.isEmpty()) { + // Release is newer + return -1; + } + + Matcher rcMatcher = RC_PATTERN.matcher(qualifier); + Matcher orcMatcher = RC_PATTERN.matcher(oq); + Matcher msMatcher = MILESTONE_PATTERN.matcher(qualifier); + Matcher omsMatcher = MILESTONE_PATTERN.matcher(oq); + boolean rc = rcMatcher.matches(); + boolean orc = orcMatcher.matches(); + boolean ms = msMatcher.matches(); + boolean oms = omsMatcher.matches(); + if (rc && orc) { + // Both are release candidates + int n = Integer.valueOf(rcMatcher.group(1)); + int on = Integer.valueOf(orcMatcher.group(1)); + return Integer.compare(n, on); + } + if (ms && oms) { + // Both are milestones + int n = Integer.valueOf(msMatcher.group(1)); + int on = Integer.valueOf(omsMatcher.group(1)); + return Integer.compare(n, on); + } + + if (rc && oms) { + // Release candidate is newer than milestone + return 1; + } + if (ms && orc) { + // Milestone is older than release candidate + return -1; + } + + long ql, oql; + try { + ql = Long.parseLong(qualifier); + } catch (NumberFormatException e) { + ql = Long.MIN_VALUE; + } + try { + oql = Long.parseLong(oq); + } catch (NumberFormatException e) { + oql = Long.MIN_VALUE; + } + + if (ql >= 0 && oql >= 0) { + // Both are positive integers, compare numerically + return Long.compare(ql, oql); + } + + boolean ss = SNAPSHOT_PATTERN.matcher(qualifier).matches(); + boolean oss = SNAPSHOT_PATTERN.matcher(oq).matches(); + if (ql < 0 && oql < 0) { + // Both aren't positive integers, snapshots are newer, otherwise do a simple string comparison + if (ss) { + // Snapshots are newer + return 1; + } + if (oss) { + // Non-snapshots are older + return -1; + } + + // If both are snapshots but have different case, compare them to remain consistent with equals() + return qualifier.compareTo(oq); + } + + if (ss) { + // Snapshots are newer than numbers + return 1; + } + + if (oss) { + // Numbers are older than snapshots + return -1; + } + + // Numbers are newer than non-numbers + return ql >= 0 ? 1 : -1; + } + + /** + * Create a new {@link Version} instance by parsing the specified version string. + * + * @param version the version string to parse. + * @return The new {@link Version} instance. + * @throws IllegalArgumentException If a version syntax can't be parsed from {@code version}. + */ + public static Version valueOf(@Nullable String version) { + if (version == null) { + return EMPTY_VERSION; + } + String v = version.trim(); + if (v.length() == 0) { + return EMPTY_VERSION; + } + + return new Version(v); + } + + /** + * Create a new {@link Version} instance from the specified {@link org.osgi.framework.Version}. + * + * @param version the {@link org.osgi.framework.Version} to get the version info from. + * @return The new {@link Version} instance. + */ + public static Version valueOf(org.osgi.framework.Version version) { + return new Version(version.getMajor(), version.getMinor(), version.getMicro(), version.getQualifier()); + } +} diff --git a/bundles/org.openhab.core/src/main/java/org/openhab/core/common/VersionRange.java b/bundles/org.openhab.core/src/main/java/org/openhab/core/common/VersionRange.java new file mode 100644 index 00000000000..bc8ffb3dbc7 --- /dev/null +++ b/bundles/org.openhab.core/src/main/java/org/openhab/core/common/VersionRange.java @@ -0,0 +1,249 @@ +/* + * Copyright (c) 2010-2026 Contributors to the openHAB project + * + * See the NOTICE file(s) distributed with this work for additional + * information. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + */ +package org.openhab.core.common; + +import java.util.Objects; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; + +/** + * This class represents a version range, where it can be evaluated whether any {@link Version} is included in the + * range. It allows for consistent, system-wide parsing and evaluation of version ranges. The supported string syntax + * expresses a version range using mathematical interval notation. + *

+ * A valid range string requires an opening bracket or parenthesis, a mandatory lower bound, a range separator + * ("{@code ;}", "{@code ,}", or "{@code ..}"), an optional upper bound, and a closing bracket or parenthesis. Brackets, + * "{@code [}" and "{@code ]}", designate inclusive endpoints, while parentheses, "{@code (}" and "{@code )}",designate + * exclusive endpoints. + *

+ *

Formal Syntax Grammar (ABNF)

+ * + * 
{@code
+ * version-range   = interval
+ *
+ * interval        = ( "[" / "(" ) version separator [ version ] ( "]" / ")" )
+ *
+ * separator       = ";" / "," / ".."
+ *
+ * version         = major [ "." minor [ "." micro [ "." qualifier ] ] ]
+ * major           = 1*DIGIT
+ * minor           = 1*DIGIT
+ * micro           = 1*DIGIT
+ * qualifier       = 1*alphanumeric
+ *
+ * alphanumeric    = ALPHA / DIGIT / "-" / "_"
+ * }
+ * + *

Examples

+ *
    + *
  • {@code [5.1.0;5.2.0]} - Inclusive range from 5.1.0 to 5.2.0
    • + *
  • {@code [5.0.0..)} - Open upper bound (version 5.0.0 or any later version)
    • + *
  • {@code [5.2.0;5.2.0]} - Exact match for version 5.2.0
    • + *
  • {@code [4.2.3,6)} - Range including 4.2.3 and anything before 6.0.0
    • + *
+ *

+ * This class is immutable. + * + * @author Ravi Nadahar - Initial contribution + */ +@NonNullByDefault +public class VersionRange { + + public static final VersionRange ANY = new VersionRange(true, Version.valueOf("0.0.0"), null, true); + + /** The left endpoint is exclusive ({@code '('}) */ + public static final char LEFT_EXCLUSIVE = '('; + + /** The left endpoint is inclusive ({@code '['}) */ + public static final char LEFT_INCLUSIVE = '['; + + /** The right endpoint is exclusive ({@code ')'}) */ + public static final char RIGHT_EXCLUSIVE = ')'; + + /** The right endpoint is inclusive ({@code ']'}) */ + public static final char RIGHT_INCLUSIVE = ']'; + + /** The {@link Pattern} used to parse version range strings */ + public static final Pattern RANGE_PATTERN = Pattern.compile( + "\\s*(?[\\[\\(])(?\\d+(\\.\\d+(\\.\\d+(\\.[^\\)\\]]+)?)?)?)(?:,|;|\\.\\.)(?\\d+(\\.\\d+(\\.\\d+(\\.[^\\)\\]]+)?)?)?)?(?[\\]\\)])\\s*"); + + protected static final Pattern WHITESPACE = Pattern.compile("\\s+"); + protected static final Pattern SEPARATORS = Pattern.compile(":|;|\\.\\."); + + protected final boolean leftInclusive; + protected final Version left; + protected final @Nullable Version right; + protected final boolean rightInclusive; + private transient volatile @Nullable String versionRangeString; + private transient volatile int hash; + + /** + * Create a new instance by parsing the specified version range string. See {@link VersionRange} for syntax + * description. + * + * @param range the version range to parse. + * @throws IllegalArgumentException If a valid version range can't be parsed from {@code range}. + */ + public VersionRange(String range) { + Objects.requireNonNull(range, "range cannot be null"); + if (range.isBlank()) { + throw new IllegalArgumentException("range cannot be blank"); + } + String r = range; + Matcher matcher = WHITESPACE.matcher(r); + if (matcher.find()) { + r = matcher.replaceAll(""); + } + matcher = SEPARATORS.matcher(r); + if (matcher.find()) { + r = matcher.replaceAll(","); + } + matcher = RANGE_PATTERN.matcher(r); + if (!matcher.find() || matcher.group("left").isBlank()) { + throw new IllegalArgumentException("Invalid range \"" + range + '"'); + } + String right = matcher.group("right"); + this.leftInclusive = "[".equals(matcher.group("leftType")); + this.left = Version.valueOf(matcher.group("left")); + this.right = right == null ? null : Version.valueOf(right); + this.rightInclusive = "]".equals(matcher.group("rightType")); + } + + /** + * Create a new instance using the specified parameters. + * + * @param leftType the left/opening bracket or parenthesis. + * @param leftEndpoint the "from" version. + * @param rightEndpoint the "to" version. + * @param rightType the right/closing bracket or parenthesis. + * @throws IllegalArgumentException If {@code leftType} or {@code rightType} isn't one of the valid characters. + */ + public VersionRange(char leftType, Version leftEndpoint, @Nullable Version rightEndpoint, char rightType) { + if ((leftType != LEFT_INCLUSIVE) && (leftType != LEFT_EXCLUSIVE)) { + throw new IllegalArgumentException("Invalid leftType \"" + leftType + "\""); + } + if ((rightType != RIGHT_EXCLUSIVE) && (rightType != RIGHT_INCLUSIVE)) { + throw new IllegalArgumentException("Invalid rightType \"" + rightType + "\""); + } + this.leftInclusive = leftType == LEFT_INCLUSIVE; + this.left = leftEndpoint; + this.right = rightEndpoint; + this.rightInclusive = rightType == RIGHT_INCLUSIVE; + } + + /** + * Create a new instance using the specified parameters. + * + * @param leftInclusive whether the left/opening version is inclusive. + * @param leftEndpoint the "from" version. + * @param rightEndpoint the "to" version. + * @param rightInclusive whether the right/closing version is inclusive. + */ + public VersionRange(boolean leftInclusive, Version leftEndpoint, @Nullable Version rightEndpoint, + boolean rightInclusive) { + this.leftInclusive = leftInclusive; + this.left = leftEndpoint; + this.right = rightEndpoint; + this.rightInclusive = rightInclusive; + } + + /** + * Check if the specified version is in this version range. + * + * @param version the version to evaluate. + * @return {@code true} if it {@code version} is in the version range, {@code false} otherwise. + */ + public boolean includes(org.osgi.framework.Version version) { + return includes(Version.valueOf(version)); + } + + /** + * Check if the specified version is in this version range. + * + * @param version the version to evaluate. + * @return {@code true} if it {@code version} is in the version range, {@code false} otherwise. + */ + public boolean includes(Version version) { + Version v = version; + Version right = this.right; + if (left.compareTo(v) >= (leftInclusive ? 1 : 0)) { + return false; + } + if (!v.getQualifier().isEmpty() && !this.rightInclusive && right != null && right.getQualifier().isEmpty()) { + // This is a special case where, although technically correct, we don't want e.g 5.0.0.RC1 to be included in + // [x.x.x,5.0.0) + v = new Version(version.getMajor(), version.getMinor(), version.getMicro()); + } + if (right == null) { + return true; + } + return right.compareTo(v) >= (rightInclusive ? 0 : 1); + } + + @Override + public int hashCode() { + int h = hash; + if (h != 0) { + return h; + } + return h = Objects.hash(leftInclusive, left, right, rightInclusive); + } + + @Override + public boolean equals(@Nullable Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof VersionRange)) { + return false; + } + VersionRange other = (VersionRange) obj; + return Objects.equals(left, other.left) && leftInclusive == other.leftInclusive + && Objects.equals(right, other.right) && rightInclusive == other.rightInclusive; + } + + @Override + public String toString() { + String s = versionRangeString; + if (s != null) { + return s; + } + StringBuilder sb = new StringBuilder(); + sb.append(leftInclusive ? LEFT_INCLUSIVE : LEFT_EXCLUSIVE).append(left.toString()).append(','); + Version r; + if ((r = right) == null) { + sb.append(RIGHT_INCLUSIVE); + } else { + sb.append(r.toString()).append(rightInclusive ? RIGHT_INCLUSIVE : RIGHT_EXCLUSIVE); + } + return versionRangeString = sb.toString(); + } + + /** + * Create a new {@link VersionRange} instance by parsing the specified version range string. See + * {@link VersionRange} for syntax definition. + * + * @param range the version range string to parse. + * @return The new {@link VersionRange} instance. + * @throws IllegalArgumentException If a valid version range can't be parsed from {@code range}. + */ + public static VersionRange valueOf(@Nullable String range) { + if (range == null || range.isBlank()) { + return ANY; + } + return new VersionRange(range); + } +} diff --git a/bundles/org.openhab.core/src/test/java/org/openhab/core/common/VersionRangeTest.java b/bundles/org.openhab.core/src/test/java/org/openhab/core/common/VersionRangeTest.java new file mode 100644 index 00000000000..0cb5b57ba7b --- /dev/null +++ b/bundles/org.openhab.core/src/test/java/org/openhab/core/common/VersionRangeTest.java @@ -0,0 +1,110 @@ +/* + * Copyright (c) 2010-2026 Contributors to the openHAB project + * + * See the NOTICE file(s) distributed with this work for additional + * information. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + */ +package org.openhab.core.common; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.*; + +import java.util.stream.Stream; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.junit.jupiter.MockitoSettings; +import org.mockito.quality.Strictness; + +/** + * The {@link VersionRangeTest} contains tests for the {@link VersionRange} class + * + * @author - Initial contribution + */ +@NonNullByDefault +@ExtendWith(MockitoExtension.class) +@MockitoSettings(strictness = Strictness.LENIENT) +public class VersionRangeTest { + + @Test + public void testIllegalRangeThrowsException() { + assertThrows(IllegalArgumentException.class, () -> VersionRange.valueOf("illegal")); + assertThrows(IllegalArgumentException.class, () -> VersionRange.valueOf("[4.3,0:)")); + assertThrows(IllegalArgumentException.class, () -> new VersionRange(" ")); + assertThrows(IllegalArgumentException.class, () -> new VersionRange("[,]")); + assertThrows(IllegalArgumentException.class, () -> new VersionRange("[ , ]")); + assertThrows(IllegalArgumentException.class, () -> new VersionRange('b', Version.valueOf("1.3.0"), null, ']')); + assertThrows(IllegalArgumentException.class, () -> new VersionRange('(', Version.valueOf("1.3.0"), null, 'e')); + } + + @Test + public void testConstructor() { + VersionRange range = new VersionRange('(', Version.valueOf("4.2.2"), null, ')'); + assertThat(range.includes(Version.valueOf((String) null)), is(false)); + assertThat(range.includes(Version.valueOf("4.2.2")), is(false)); + range = new VersionRange('[', Version.valueOf("4.2.2"), null, ')'); + assertThat(range.includes(Version.valueOf("4.2.2")), is(true)); + } + + @Test + public void testIncludes() { + assertThat(VersionRange.valueOf("[3.3.0;3.4.0)").includes(Version.valueOf("3.2.0")), is(false)); + assertThat(VersionRange.valueOf("[3.3.0;3.4.0]").includes(Version.valueOf("3.4.0.M1")), is(true)); + assertThat(VersionRange.valueOf("[3.3.0;3.4.0)").includes(Version.valueOf("3.4.0.M1")), is(false)); + assertThat(VersionRange.valueOf("[3.3.0;)").includes(Version.valueOf("3.4.0.M1")), is(true)); + } + + @Test + public void testToString() { + assertEquals("[0.0.0,]", VersionRange.valueOf(null).toString()); + assertEquals("[0.0.0,]", VersionRange.valueOf("").toString()); + assertEquals("[2.4.0,]", VersionRange.valueOf("[2.4:]").toString()); + assertEquals("(4.2.3,]", VersionRange.valueOf("(4.2.3;]").toString()); + assertEquals("[3.1.0,3.2.0)", VersionRange.valueOf("[3.1.0;3.2.0)").toString()); + assertEquals("[3.1.0,3.2.0]", VersionRange.valueOf("[3.1.0;3.2.0]").toString()); + assertEquals("(3.1.0,3.2.9.alpha)", VersionRange.valueOf("(3.1.0;3.2.9.alpha)").toString()); + assertEquals("(3.1.0,3.2.0.SNAPSHOT]", VersionRange.valueOf("(3.1.0 ; 3.2. 0.SNAPSHOT]").toString()); + } + + private static Stream provideInRangeArguments() { + return Stream.of(Arguments.of("5.0.0.RC1", "[3.3.0;5.0.0.0]", true), + Arguments.of("3.2.0", "[3.1.0;3.2.1)", true), // in range + Arguments.of("3.2.0", "[3.1.0;3.2.0)", false), // at end of range, non-inclusive + Arguments.of("3.2.0", "[3.1.0;3.2.0]", true), // at end of range, inclusive + Arguments.of("3.2.0", "[3.1.0;3.1.5)", false), // above range + Arguments.of("3.2.0", "[3.3.0;3.4.0)", false), // below range + Arguments.of("3.2.0", "", true), // empty range assumes in range + Arguments.of("3.2.0", null, true), // null range assumes in range + Arguments.of("5.0.0.RC1", "[3.3.0;5.0.0)", false), Arguments.of("5.0.0.RC1", "[3.3.0;5.0.0.0]", true), + Arguments.of("5.0.0.M4", "[3.3.0;5.0)", false), Arguments.of("5.0.0.202510140119", "[3.3.0:5)", false), + Arguments.of("5.0.0.202510140119", "[3.3.0,)", true), + Arguments.of("3.3.0.202310140119", "[3.3.0,)", false), + Arguments.of("3.3.0.202310140119", "[3.3.0.0,)", true), Arguments.of("3.3.0", "[3.3.0,)", true), + Arguments.of("3.3.0", "(3.3.0,)", false), Arguments.of("3.3.1", "(3.3.0..)", true), + Arguments.of("2.0.0.M2", "[2.0.0.M1;2.0.0.RC1)", true), Arguments.of("5.2.0-RC1", "[5.2.0.M3;6)", true), + Arguments.of("5.2.0-SNAPSHOT", "[5.2.0.M3;6)", true), Arguments.of("5.2.0.M1", "[5.2.0.M3;6)", false), + Arguments.of("6.0.0.M1", "[5.2.0.M3;6)", false), + Arguments.of("3.3.0", " [ 3 . 3 .0. alpha;)", true)); + } + + @ParameterizedTest + @MethodSource("provideInRangeArguments") + public void inRangeTest(String versionStr, @Nullable String rangeStr, boolean result) { + Version version = Version.valueOf(versionStr); + VersionRange range = VersionRange.valueOf(rangeStr); + assertThat(range.includes(version), is(result)); + } +} diff --git a/bundles/org.openhab.core/src/test/java/org/openhab/core/common/VersionTest.java b/bundles/org.openhab.core/src/test/java/org/openhab/core/common/VersionTest.java new file mode 100644 index 00000000000..81959314219 --- /dev/null +++ b/bundles/org.openhab.core/src/test/java/org/openhab/core/common/VersionTest.java @@ -0,0 +1,142 @@ +/* + * Copyright (c) 2010-2026 Contributors to the openHAB project + * + * See the NOTICE file(s) distributed with this work for additional + * information. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + */ +package org.openhab.core.common; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.lessThan; +import static org.junit.jupiter.api.Assertions.*; + +import java.util.stream.Stream; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.junit.jupiter.MockitoSettings; +import org.mockito.quality.Strictness; + +/** + * The {@link VersionTest} contains tests for the {@link Version} class + * + * @author - Initial contribution + */ +@NonNullByDefault +@ExtendWith(MockitoExtension.class) +@MockitoSettings(strictness = Strictness.LENIENT) +public class VersionTest { + + private static Stream provideCompareVersionsArguments() { + return Stream.of( // + Arguments.of(null, null, Result.EQUAL), // same versions are equal + Arguments.of(null, "4.3.0", Result.OLDER), // null is older than everything + Arguments.of("3.1.0", "3.1.0", Result.EQUAL), // same versions are equal + Arguments.of("3.1.0", "3.0.2", Result.NEWER), // minor version is more important than micro + Arguments.of("3.7.0", "4.0.1.202105311711", Result.OLDER), // major version is more important than minor + Arguments.of("3.9.1.M1", "3.9.0.M5", Result.NEWER), // micro version is more important than qualifier + Arguments.of("3.0.0.202105311032", "3.0.0.202106011144", Result.OLDER), // snapshots + Arguments.of("3.1.0.M3", "3.1.0.M1", Result.NEWER), // milestones are compared numerically + Arguments.of("3.1.0.M1", "3.1.0.197705310021", Result.OLDER), // snapshot is newer than milestone + Arguments.of("3.3.0", "3.3.0.202206302115", Result.NEWER), // release is newer than snapshot + Arguments.of("3.3.0", "3.3.0.RC1", Result.NEWER), // releases are newer than release candidates + Arguments.of("3.3.0.M5", "3.3.0.RC1", Result.OLDER), // milestones are older than release candidates + Arguments.of("3.3.0.RC2", "3.3.0.202305201715", Result.OLDER), // snapshots are newer than release + // candidates + Arguments.of("3.3.0-SNAPSHOT", "3.3.0.202305201715", Result.NEWER), + Arguments.of("3.3.0-RC2", "3.3.0.RC11", Result.OLDER), + Arguments.of("3.3.0-RC4", "3.3.0.M11", Result.NEWER), + Arguments.of("5.0.0-SNAPSHOT", "5.0.0", Result.OLDER), + Arguments.of("5.0.0-SNAPSHOT", "5.0.0-alpha", Result.NEWER), + Arguments.of("5.0.0-alpha", "5.0.0-beta", Result.OLDER), + Arguments.of("5.0.0_snapshot", "5.0.0-SNAPSHOT", Result.NEWER), + Arguments.of("5.0.0_SNAPSHOT", "5.0.0-SNAPSHOT", Result.EQUAL), + Arguments.of("5.0.0-SNAPSHOT", "5.0.0-SNAPSHOT", Result.EQUAL), + Arguments.of("5.0.0.M2", "5.0.0-SNAPSHOT", Result.OLDER), Arguments.of("5", "5.0.0", Result.EQUAL), + Arguments.of("5.0.0", "5.0", Result.EQUAL), + Arguments.of("3.3.0.202305201715", "3.3.0.alpha", Result.NEWER), + Arguments.of("5.0.0_202501132145", "5.0.0.snapshot", Result.OLDER)); + } + + @ParameterizedTest + @MethodSource("provideCompareVersionsArguments") + public void testCompareVersions(String v1, String v2, Result result) { + Version version1 = Version.valueOf(v1); + Version version2 = Version.valueOf(v2); + switch (result) { + case OLDER: + assertThat(version1.compareTo(version2), lessThan(0)); + break; + case NEWER: + assertThat(version1.compareTo(version2), greaterThan(0)); + break; + case EQUAL: + assertThat(version1.compareTo(version2), is(0)); + break; + } + } + + @Test + public void testConstructors() { + assertThrows(IllegalArgumentException.class, () -> new Version("illegal")); + assertThrows(IllegalArgumentException.class, () -> new Version("5.0.2:alpha")); + assertThrows(IllegalArgumentException.class, () -> new Version("5.-2.2.alpha")); + assertThrows(IllegalArgumentException.class, () -> new Version(0, -1, 3)); + assertThrows(IllegalArgumentException.class, () -> new Version(-2, 1, 3)); + assertThrows(IllegalArgumentException.class, () -> new Version(2, 1, -3)); + assertThrows(IllegalArgumentException.class, () -> new Version(1, 2, 3, "snap$hot")); + assertThrows(IllegalArgumentException.class, () -> new Version(1, 2, 3, ':', "snapshot")); + assertEquals("1.2.3", new Version(1, 2, 3).toString()); + assertEquals("1.2.3", new Version(1, 2, 3, "").toString()); + assertEquals("1.2.3.beta", new Version(1, 2, 3, "beta").toString()); + assertEquals("1.2.3.93", new Version(1, 2, 3, "93").toString()); + assertEquals("1.2.3-SNAPSHOT", new Version(1, 2, 3, '-', "SNAPSHOT").toString()); + assertEquals("1.2.3_test", new Version(1, 2, 3, '_', "test").toString()); + } + + @SuppressWarnings("unlikely-arg-type") + @Test + public void testMisc() { + Version v = Version.valueOf((String) null); + org.osgi.framework.Version ov = v.toOSGiVersion(); + assertEquals('.', v.getLastSeparator()); + assertEquals(Version.EMPTY_VERSION, v); + assertEquals("0.0.0", v.toString()); + assertEquals(v.toString(), v.toString()); + assertEquals(v.hashCode(), v.hashCode()); + assertTrue(v.equals(v)); + assertFalse(v.equals(ov)); + assertEquals(0, v.compareTo(ov)); + Version v2 = Version.valueOf(" "); + assertEquals(v, v2); + v = new Version(9, 8, 0, null); + ov = v.toOSGiVersion(); + assertEquals(0, v.compareTo(ov)); + v2 = Version.valueOf(ov); + assertEquals(v, v2); + v = new Version(9, 8, 0, "alpha"); + ov = v.toOSGiVersion(); + assertEquals(0, v.compareTo(ov)); + v2 = Version.valueOf(ov); + assertEquals(v, v2); + } + + private enum Result { + OLDER, + NEWER, + EQUAL + } +} From a1c09f294bea5b17eef359bb8e11dbf993e2c471 Mon Sep 17 00:00:00 2001 From: Ravi Nadahar Date: Mon, 13 Jul 2026 19:10:52 +0200 Subject: [PATCH 2/5] Fix NPE in MarketplaceAddonHandler.supports() and annotate accordingly Signed-off-by: Ravi Nadahar --- .../karaf/internal/community/CommunityKarafAddonHandler.java | 3 ++- .../core/addon/marketplace/MarketplaceAddonHandler.java | 3 ++- .../internal/community/CommunityBlockLibaryAddonHandler.java | 3 ++- .../internal/community/CommunityBundleAddonHandler.java | 5 +++-- .../community/CommunityRuleTemplateAddonHandler.java | 3 ++- .../community/CommunityTransformationAddonHandler.java | 3 ++- .../internal/community/CommunityUIWidgetAddonHandler.java | 3 ++- .../core/addon/marketplace/test/TestAddonHandler.java | 3 ++- 8 files changed, 17 insertions(+), 9 deletions(-) diff --git a/bundles/org.openhab.core.addon.marketplace.karaf/src/main/java/org/openhab/core/addon/marketplace/karaf/internal/community/CommunityKarafAddonHandler.java b/bundles/org.openhab.core.addon.marketplace.karaf/src/main/java/org/openhab/core/addon/marketplace/karaf/internal/community/CommunityKarafAddonHandler.java index 8c77849fbc4..c28e7095867 100644 --- a/bundles/org.openhab.core.addon.marketplace.karaf/src/main/java/org/openhab/core/addon/marketplace/karaf/internal/community/CommunityKarafAddonHandler.java +++ b/bundles/org.openhab.core.addon.marketplace.karaf/src/main/java/org/openhab/core/addon/marketplace/karaf/internal/community/CommunityKarafAddonHandler.java @@ -30,6 +30,7 @@ import org.apache.karaf.kar.KarService; import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; import org.openhab.core.OpenHAB; import org.openhab.core.addon.Addon; import org.openhab.core.addon.marketplace.MarketplaceAddonHandler; @@ -72,7 +73,7 @@ public CommunityKarafAddonHandler(@Reference KarService karService) { } @Override - public boolean supports(String type, String contentType) { + public boolean supports(String type, @Nullable String contentType) { return SUPPORTED_EXT_TYPES.contains(type) && KAR_CONTENT_TYPE.equals(contentType); } diff --git a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/MarketplaceAddonHandler.java b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/MarketplaceAddonHandler.java index 1616a4dcd78..626443a5456 100644 --- a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/MarketplaceAddonHandler.java +++ b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/MarketplaceAddonHandler.java @@ -13,6 +13,7 @@ package org.openhab.core.addon.marketplace; import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; import org.openhab.core.addon.Addon; /** @@ -38,7 +39,7 @@ public interface MarketplaceAddonHandler { * @param contentType the content type of the add-on on question * @return true, if the addon type and contentType are supported, false otherwise */ - boolean supports(String type, String contentType); + boolean supports(String type, @Nullable String contentType); /** * Tells whether a given add-on is currently installed. diff --git a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityBlockLibaryAddonHandler.java b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityBlockLibaryAddonHandler.java index 950193f9960..d760bfaa6c4 100644 --- a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityBlockLibaryAddonHandler.java +++ b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityBlockLibaryAddonHandler.java @@ -30,6 +30,7 @@ import java.util.concurrent.ConcurrentHashMap; import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; import org.openhab.core.addon.Addon; import org.openhab.core.addon.marketplace.MarketplaceAddonHandler; import org.openhab.core.addon.marketplace.MarketplaceHandlerException; @@ -84,7 +85,7 @@ protected void removeParser(RootUIComponentParser parser) { } @Override - public boolean supports(String type, String contentType) { + public boolean supports(String type, @Nullable String contentType) { return "automation".equals(type) && BLOCKLIBRARIES_CONTENT_TYPE.equals(contentType); } diff --git a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityBundleAddonHandler.java b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityBundleAddonHandler.java index 8d9009e91a1..833411c45df 100644 --- a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityBundleAddonHandler.java +++ b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityBundleAddonHandler.java @@ -22,6 +22,7 @@ import java.util.concurrent.ScheduledExecutorService; import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; import org.openhab.core.addon.Addon; import org.openhab.core.addon.marketplace.MarketplaceAddonHandler; import org.openhab.core.addon.marketplace.MarketplaceBundleInstaller; @@ -65,9 +66,9 @@ public CommunityBundleAddonHandler(BundleContext bundleContext) { } @Override - public boolean supports(String type, String contentType) { + public boolean supports(String type, @Nullable String contentType) { // we support only certain extension types, and only as pure OSGi bundles - return SUPPORTED_EXT_TYPES.contains(type) && contentType.equals(JAR_CONTENT_TYPE); + return SUPPORTED_EXT_TYPES.contains(type) && JAR_CONTENT_TYPE.equals(contentType); } @Override diff --git a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityRuleTemplateAddonHandler.java b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityRuleTemplateAddonHandler.java index 6df36f58c44..e980f6ac48a 100644 --- a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityRuleTemplateAddonHandler.java +++ b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityRuleTemplateAddonHandler.java @@ -23,6 +23,7 @@ import java.nio.charset.StandardCharsets; import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; import org.openhab.core.addon.Addon; import org.openhab.core.addon.marketplace.MarketplaceAddonHandler; import org.openhab.core.addon.marketplace.MarketplaceHandlerException; @@ -57,7 +58,7 @@ public CommunityRuleTemplateAddonHandler( } @Override - public boolean supports(String type, String contentType) { + public boolean supports(String type, @Nullable String contentType) { return "automation".equals(type) && RULETEMPLATES_CONTENT_TYPE.equals(contentType); } diff --git a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityTransformationAddonHandler.java b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityTransformationAddonHandler.java index 7889bad2139..72d4efa572a 100644 --- a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityTransformationAddonHandler.java +++ b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityTransformationAddonHandler.java @@ -29,6 +29,7 @@ import java.util.concurrent.CopyOnWriteArrayList; import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; import org.openhab.core.addon.Addon; import org.openhab.core.addon.marketplace.MarketplaceAddonHandler; import org.openhab.core.addon.marketplace.MarketplaceHandlerException; @@ -79,7 +80,7 @@ public CommunityTransformationAddonHandler(final @Reference StorageService stora } @Override - public boolean supports(String type, String contentType) { + public boolean supports(String type, @Nullable String contentType) { return "transformation".equals(type) && TRANSFORMATIONS_CONTENT_TYPE.equals(contentType); } diff --git a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityUIWidgetAddonHandler.java b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityUIWidgetAddonHandler.java index 135b5085146..0f12348ac08 100644 --- a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityUIWidgetAddonHandler.java +++ b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityUIWidgetAddonHandler.java @@ -30,6 +30,7 @@ import java.util.concurrent.ConcurrentHashMap; import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; import org.openhab.core.addon.Addon; import org.openhab.core.addon.marketplace.MarketplaceAddonHandler; import org.openhab.core.addon.marketplace.MarketplaceHandlerException; @@ -87,7 +88,7 @@ protected void removeParser(RootUIComponentParser parser) { } @Override - public boolean supports(String type, String contentType) { + public boolean supports(String type, @Nullable String contentType) { return "ui".equals(type) && UIWIDGETS_CONTENT_TYPE.equals(contentType); } diff --git a/bundles/org.openhab.core.addon.marketplace/src/test/java/org/openhab/core/addon/marketplace/test/TestAddonHandler.java b/bundles/org.openhab.core.addon.marketplace/src/test/java/org/openhab/core/addon/marketplace/test/TestAddonHandler.java index a6092c358af..f452e4cac92 100644 --- a/bundles/org.openhab.core.addon.marketplace/src/test/java/org/openhab/core/addon/marketplace/test/TestAddonHandler.java +++ b/bundles/org.openhab.core.addon.marketplace/src/test/java/org/openhab/core/addon/marketplace/test/TestAddonHandler.java @@ -19,6 +19,7 @@ import java.util.Set; import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; import org.openhab.core.addon.Addon; import org.openhab.core.addon.marketplace.MarketplaceAddonHandler; import org.openhab.core.addon.marketplace.MarketplaceHandlerException; @@ -47,7 +48,7 @@ public boolean isReady() { } @Override - public boolean supports(String type, String contentType) { + public boolean supports(String type, @Nullable String contentType) { return SUPPORTED_ADDON_TYPES.contains(type) && TEST_ADDON_CONTENT_TYPE.equals(contentType); } From edab11f2b1baf61f90cba511ffc0cb5c856f4246 Mon Sep 17 00:00:00 2001 From: Ravi Nadahar Date: Sat, 2 Aug 2025 21:52:47 +0200 Subject: [PATCH 3/5] Process GitHub URLs to get "raw" version Signed-off-by: Ravi Nadahar --- .../CommunityMarketplaceAddonService.java | 70 ++++++++++++++++--- 1 file changed, 62 insertions(+), 8 deletions(-) diff --git a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityMarketplaceAddonService.java b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityMarketplaceAddonService.java index 4b745dbb0d7..17e32b119a9 100644 --- a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityMarketplaceAddonService.java +++ b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityMarketplaceAddonService.java @@ -18,6 +18,7 @@ import java.io.InputStreamReader; import java.io.Reader; import java.net.URI; +import java.net.URISyntaxException; import java.net.URL; import java.net.URLConnection; import java.util.ArrayList; @@ -407,20 +408,32 @@ private Addon convertTopicToAddon(DiscourseTopicResponseDTO topic) { // try to extract contents or links if (topic.postStream.posts[0].linkCounts != null) { + URI uri; + String path; for (DiscoursePostLink postLink : topic.postStream.posts[0].linkCounts) { - if (postLink.url.toLowerCase(Locale.ROOT).endsWith(".jar")) { - properties.put(JAR_DOWNLOAD_URL_PROPERTY, postLink.url); + try { + uri = processResourceURL(postLink.url); + path = uri.getPath(); + if (path == null) { + continue; + } + } catch (IllegalArgumentException e) { + continue; + } + path = path.toLowerCase(Locale.ROOT); + if (path.endsWith(".jar")) { + properties.put(JAR_DOWNLOAD_URL_PROPERTY, uri.toString()); id = determineIdFromUrl(postLink.url); } - if (postLink.url.toLowerCase(Locale.ROOT).endsWith(".kar")) { - properties.put(KAR_DOWNLOAD_URL_PROPERTY, postLink.url); + if (path.endsWith(".kar")) { + properties.put(KAR_DOWNLOAD_URL_PROPERTY, uri.toString()); id = determineIdFromUrl(postLink.url); } - if (postLink.url.toLowerCase(Locale.ROOT).endsWith(".json")) { - properties.put(JSON_DOWNLOAD_URL_PROPERTY, postLink.url); + if (path.endsWith(".json")) { + properties.put(JSON_DOWNLOAD_URL_PROPERTY, uri.toString()); } - if (postLink.url.toLowerCase(Locale.ROOT).endsWith(".yaml")) { - properties.put(YAML_DOWNLOAD_URL_PROPERTY, postLink.url); + if (path.endsWith(".yaml")) { + properties.put(YAML_DOWNLOAD_URL_PROPERTY, uri.toString()); } } } @@ -456,6 +469,47 @@ private Addon convertTopicToAddon(DiscourseTopicResponseDTO topic) { return builder.build(); } + private URI processResourceURL(String url) throws IllegalArgumentException { + URI uri; + try { + uri = new URI(url); + if (uri.getFragment() != null) { + throw new IllegalArgumentException("Fragment not allowed in resource URLs: " + uri.getFragment()); + } + String host = uri.getHost(); + if (host == null) { + throw new IllegalArgumentException("Missing host in resource URL: " + url); + } + String query, path; + switch (host) { + case "github.com": + case "www.github.com": + // Modify GitHub URLs to use their "raw" version if necessary + path = uri.getPath(); + if (path != null && path.contains("/blob/")) { + query = uri.getQuery(); + if (query == null) { + return new URI(uri.getScheme(), uri.getAuthority(), uri.getPath(), "raw=true", null); + } + if (!query.contains("raw=true")) { + String[] parts = query.split("&"); + String[] newParts = new String[parts.length + 1]; + System.arraycopy(parts, 0, newParts, 0, parts.length); + newParts[parts.length] = "raw=true"; + return new URI(uri.getScheme(), uri.getAuthority(), uri.getPath(), + String.join("&", newParts), null); + } + } + break; + default: + break; + } + return uri; + } catch (URISyntaxException e) { + throw new IllegalArgumentException("Invalid URL: " + url, e); + } + } + private @Nullable String determineIdFromUrl(String url) { Matcher matcher = BUNDLE_NAME_PATTERN.matcher(url); if (matcher.matches()) { From 364bd58b6c13e07ce08bfdfffa677b3598a9c58c Mon Sep 17 00:00:00 2001 From: Ravi Nadahar Date: Sat, 18 Jul 2026 18:14:17 +0200 Subject: [PATCH 4/5] Extract add-on resource URL from the post text instead of from 'linkCounts', to make sure that we always grab the last one Signed-off-by: Ravi Nadahar --- .../CommunityMarketplaceAddonService.java | 67 ++++++++++--------- 1 file changed, 34 insertions(+), 33 deletions(-) diff --git a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityMarketplaceAddonService.java b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityMarketplaceAddonService.java index 17e32b119a9..7ede38282eb 100644 --- a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityMarketplaceAddonService.java +++ b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityMarketplaceAddonService.java @@ -46,7 +46,6 @@ import org.openhab.core.addon.marketplace.internal.community.model.DiscourseCategoryResponseDTO.DiscourseTopicItem; import org.openhab.core.addon.marketplace.internal.community.model.DiscourseCategoryResponseDTO.DiscourseUser; import org.openhab.core.addon.marketplace.internal.community.model.DiscourseTopicResponseDTO; -import org.openhab.core.addon.marketplace.internal.community.model.DiscourseTopicResponseDTO.DiscoursePostLink; import org.openhab.core.common.VersionRange; import org.openhab.core.config.core.ConfigParser; import org.openhab.core.config.core.ConfigurableService; @@ -97,6 +96,8 @@ public class CommunityMarketplaceAddonService extends AbstractRemoteAddonService private static final Pattern CODE_MARKUP_PATTERN = Pattern.compile( "[-a-zA-Z]+)\">(?.*?)\\n?", Pattern.DOTALL); + private static final Pattern LAST_RESOURCE_LINK_PATTERN = Pattern.compile( + ".*href=\"(?[^\"]+\\.(?jar|kar|json|yaml))\"", Pattern.DOTALL | Pattern.CASE_INSENSITIVE); private static final Integer BUNDLES_CATEGORY = 73; private static final Integer RULETEMPLATES_CATEGORY = 74; @@ -406,35 +407,36 @@ private Addon convertTopicToAddon(DiscourseTopicResponseDTO topic) { String detailedDescription = topic.postStream.posts[0].cooked; String id = null; - // try to extract contents or links - if (topic.postStream.posts[0].linkCounts != null) { - URI uri; - String path; - for (DiscoursePostLink postLink : topic.postStream.posts[0].linkCounts) { - try { - uri = processResourceURL(postLink.url); - path = uri.getPath(); - if (path == null) { - continue; + Matcher matcher = LAST_RESOURCE_LINK_PATTERN.matcher(detailedDescription); + if (matcher.find()) { + try { + URI uri = processResourceURL(matcher.group("url")); + String path = uri.getPath(); + if (path != null) { + switch (matcher.group("extension").toLowerCase(Locale.ROOT)) { + case "jar": + properties.put(JAR_DOWNLOAD_URL_PROPERTY, uri.toString()); + id = determineIdFromUrl(path); + break; + case "kar": + properties.put(KAR_DOWNLOAD_URL_PROPERTY, uri.toString()); + id = determineIdFromUrl(path); + break; + case "json": + properties.put(JSON_DOWNLOAD_URL_PROPERTY, uri.toString()); + break; + case "yaml": + properties.put(YAML_DOWNLOAD_URL_PROPERTY, uri.toString()); + break; } - } catch (IllegalArgumentException e) { - continue; - } - path = path.toLowerCase(Locale.ROOT); - if (path.endsWith(".jar")) { - properties.put(JAR_DOWNLOAD_URL_PROPERTY, uri.toString()); - id = determineIdFromUrl(postLink.url); - } - if (path.endsWith(".kar")) { - properties.put(KAR_DOWNLOAD_URL_PROPERTY, uri.toString()); - id = determineIdFromUrl(postLink.url); - } - if (path.endsWith(".json")) { - properties.put(JSON_DOWNLOAD_URL_PROPERTY, uri.toString()); - } - if (path.endsWith(".yaml")) { - properties.put(YAML_DOWNLOAD_URL_PROPERTY, uri.toString()); + } else { + logger.debug( + "Failed to extract path from resource URL for marketplace add-on '{}'. This should be impossible", + topic.title); } + } catch (IllegalArgumentException e) { + logger.debug("Add-on '{}' ({}) has an invalid resource URL '{}': {}", topic.title, topic.id, + matcher.group("url"), e.getMessage()); } } @@ -442,10 +444,9 @@ private Addon convertTopicToAddon(DiscourseTopicResponseDTO topic) { id = topic.id.toString(); // this is a fallback if we couldn't find a better id } - Matcher codeMarkup = CODE_MARKUP_PATTERN.matcher(detailedDescription); - if (codeMarkup.find()) { - properties.put(codeMarkup.group("lang") + CODE_CONTENT_SUFFIX, - unescapeEntities(codeMarkup.group("content"))); + matcher = CODE_MARKUP_PATTERN.matcher(detailedDescription); + if (matcher.find()) { + properties.put(matcher.group("lang") + CODE_CONTENT_SUFFIX, unescapeEntities(matcher.group("content"))); } // try to use a handler to determine if the add-on is installed @@ -454,7 +455,7 @@ private Addon convertTopicToAddon(DiscourseTopicResponseDTO topic) { String title = topic.title; boolean compatible = true; - Matcher matcher = VersionRange.RANGE_PATTERN.matcher(title); + matcher = VersionRange.RANGE_PATTERN.matcher(title); if (matcher.find()) { compatible = VersionRange.valueOf(matcher.group().trim()).includes(coreVersion); title = matcher.replaceFirst("").trim(); From ccf071822526ddcfbaf53edf9bb320349e7b074f Mon Sep 17 00:00:00 2001 From: Ravi Nadahar Date: Sat, 25 Jul 2026 18:33:26 +0200 Subject: [PATCH 5/5] Address review feedback Signed-off-by: Ravi Nadahar --- .../addon/marketplace/AbstractRemoteAddonService.java | 3 ++- .../src/main/java/org/openhab/core/addon/Addon.java | 8 ++++---- .../main/java/org/openhab/core/common/VersionRange.java | 2 +- 3 files changed, 7 insertions(+), 6 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 c4130096d4b..424afd517da 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 @@ -69,8 +69,9 @@ public abstract class AbstractRemoteAddonService implements AddonService { if (compatible != 0) { return compatible; } + Version v1 = addon1.getVersion(); Version v2 = addon2.getVersion(); - return v2 == null ? 1 : v2.compareTo(addon1.getVersion()); + return v1 == null && v2 == null ? 0 : v2 == null ? 1 : v2.compareTo(v1); }; protected final Version coreVersion; diff --git a/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/Addon.java b/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/Addon.java index b4984695d77..57c0cfc9ad8 100644 --- a/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/Addon.java +++ b/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/Addon.java @@ -12,7 +12,7 @@ */ package org.openhab.core.addon; -import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Objects; @@ -362,7 +362,7 @@ private Builder(Addon addon) { this.connection = addon.connection; this.backgroundColor = addon.backgroundColor; this.imageLink = addon.imageLink; - this.properties = addon.properties; + this.properties = new LinkedHashMap<>(addon.properties); this.loggerPackages = addon.loggerPackages; } @@ -446,7 +446,7 @@ public Builder withKeywords(String keywords) { return countries; } - public Builder withCountries(List<@NonNull String> countries) { + public Builder withCountries(@Nullable List<@NonNull String> countries) { this.countries = countries; return this; } @@ -474,7 +474,7 @@ public Builder withImageLink(@Nullable String imageLink) { public Builder withProperty(@NonNull String key, @NonNull Object value) { Map<@NonNull String, @NonNull Object> props = this.properties; if (props == null) { - props = new HashMap<>(); + props = new LinkedHashMap<>(); } props.put(key, value); this.properties = props; diff --git a/bundles/org.openhab.core/src/main/java/org/openhab/core/common/VersionRange.java b/bundles/org.openhab.core/src/main/java/org/openhab/core/common/VersionRange.java index bc8ffb3dbc7..e2ce28c3459 100644 --- a/bundles/org.openhab.core/src/main/java/org/openhab/core/common/VersionRange.java +++ b/bundles/org.openhab.core/src/main/java/org/openhab/core/common/VersionRange.java @@ -199,7 +199,7 @@ public int hashCode() { if (h != 0) { return h; } - return h = Objects.hash(leftInclusive, left, right, rightInclusive); + return hash = Objects.hash(leftInclusive, left, right, rightInclusive); } @Override