Skip to content

Commit

Permalink
Fix marketplace add-ons missing config description URI (openhab#3688)
Browse files Browse the repository at this point in the history 
Signed-off-by: Jan N. Klug <github@klug.nrw>
GitOrigin-RevId: abcfe54
  • Loading branch information
@J-N-K @splatch
J-N-K authored and splatch committed Jul 12, 2023
1 parent ebe74a3 commit 65ea21c
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 17 deletions.
20 changes: 17 additions & 3 deletions ...ketplace/src/main/java/org/openhab/core/addon/marketplace/AbstractRemoteAddonService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;
Expand All @@ -29,6 +30,8 @@
import org.openhab.core.OpenHAB;
import org.openhab.core.addon.Addon;
import org.openhab.core.addon.AddonEventFactory;
import org.openhab.core.addon.AddonInfo;
import org.openhab.core.addon.AddonInfoRegistry;
import org.openhab.core.addon.AddonService;
import org.openhab.core.addon.AddonType;
import org.openhab.core.cache.ExpiringCache;
Expand Down Expand Up @@ -66,13 +69,15 @@ public abstract class AbstractRemoteAddonService implements AddonService {
protected final ConfigurationAdmin configurationAdmin;
protected final ExpiringCache<List<Addon>> cachedRemoteAddons = new ExpiringCache<>(Duration.ofMinutes(15),
this::getRemoteAddons);
protected final AddonInfoRegistry addonInfoRegistry;
protected List<Addon> cachedAddons = List.of();
protected List<String> installedAddons = List.of();

private final Logger logger = LoggerFactory.getLogger(AbstractRemoteAddonService.class);

public AbstractRemoteAddonService(EventPublisher eventPublisher, ConfigurationAdmin configurationAdmin,
StorageService storageService, String servicePid) {
StorageService storageService, AddonInfoRegistry addonInfoRegistry, String servicePid) {
this.addonInfoRegistry = addonInfoRegistry;
this.eventPublisher = eventPublisher;
this.configurationAdmin = configurationAdmin;
this.installedAddonStorage = storageService.getStorage(servicePid);
Expand All @@ -83,6 +88,16 @@ protected BundleVersion getCoreVersion() {
return new BundleVersion(FrameworkUtil.getBundle(OpenHAB.class).getVersion().toString());
}

private Addon convertFromStorage(Map.Entry<String, @Nullable String> entry) {
Addon storedAddon = Objects.requireNonNull(gson.fromJson(entry.getValue(), Addon.class));
AddonInfo addonInfo = addonInfoRegistry.getAddonInfo(storedAddon.getType() + "-" + storedAddon.getId());
if (addonInfo != null && storedAddon.getConfigDescriptionURI().isBlank()) {
return Addon.create(storedAddon).withConfigDescriptionURI(addonInfo.getConfigDescriptionURI()).build();
} else {
return storedAddon;
}
}

@Override
public void refreshSource() {
if (!addonHandlers.stream().allMatch(MarketplaceAddonHandler::isReady)) {
Expand All @@ -92,8 +107,7 @@ public void refreshSource() {
}
List<Addon> addons = new ArrayList<>();
try {
installedAddonStorage.stream().map(e -> Objects.requireNonNull(gson.fromJson(e.getValue(), Addon.class)))
.forEach(addons::add);
installedAddonStorage.stream().map(this::convertFromStorage).forEach(addons::add);
} catch (JsonSyntaxException e) {
List.copyOf(installedAddonStorage.getKeys()).forEach(installedAddonStorage::remove);
logger.error(
Expand Down
22 changes: 14 additions & 8 deletions ...g/openhab/core/addon/marketplace/internal/community/CommunityMarketplaceAddonService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.core.addon.Addon;
import org.openhab.core.addon.AddonInfoRegistry;
import org.openhab.core.addon.AddonService;
import org.openhab.core.addon.AddonType;
import org.openhab.core.addon.marketplace.AbstractRemoteAddonService;
Expand Down Expand Up @@ -115,8 +116,8 @@ public class CommunityMarketplaceAddonService extends AbstractRemoteAddonService
@Activate
public CommunityMarketplaceAddonService(final @Reference EventPublisher eventPublisher,
@Reference ConfigurationAdmin configurationAdmin, @Reference StorageService storageService,
Map<String, Object> config) {
super(eventPublisher, configurationAdmin, storageService, SERVICE_PID);
@Reference AddonInfoRegistry addonInfoRegistry, Map<String, Object> config) {
super(eventPublisher, configurationAdmin, storageService, addonInfoRegistry, SERVICE_PID);
modified(config);
}

Expand Down Expand Up @@ -200,10 +201,13 @@ protected List<Addon> getRemoteAddons() {

@Override
public @Nullable Addon getAddon(String uid, @Nullable Locale locale) {
String queryId = uid.startsWith(ADDON_ID_PREFIX) ? uid : ADDON_ID_PREFIX + uid;

// check if it is an installed add-on (cachedAddons also contains possibly incomplete results from the remote
// side, we need to retrieve them from Discourse)
if (installedAddons.contains(uid)) {
return cachedAddons.stream().filter(e -> uid.equals(e.getUid())).findAny().orElse(null);

if (installedAddons.contains(queryId)) {
return cachedAddons.stream().filter(e -> queryId.equals(e.getUid())).findAny().orElse(null);
}

if (!remoteEnabled()) {
Expand Down Expand Up @@ -437,11 +441,13 @@ private Addon convertTopicToAddon(DiscourseTopicResponseDTO topic) {
boolean installed = addonHandlers.stream()
.anyMatch(handler -> handler.supports(type, contentType) && handler.isInstalled(uid));

return Addon.create(uid).withType(type).withId(id).withContentType(contentType).withLabel(topic.title)
.withImageLink(topic.imageUrl).withLink(COMMUNITY_TOPIC_URL + topic.id.toString())
Addon.Builder builder = Addon.create(uid).withType(type).withId(id).withContentType(contentType)
.withLabel(topic.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)
.build();
.withDetailedDescription(detailedDescription).withInstalled(installed).withProperties(properties);

return builder.build();
}

private @Nullable String determineIdFromUrl(String url) {
Expand Down
6 changes: 4 additions & 2 deletions ...lace/src/main/java/org/openhab/core/addon/marketplace/internal/json/JsonAddonService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.core.addon.Addon;
import org.openhab.core.addon.AddonInfoRegistry;
import org.openhab.core.addon.AddonService;
import org.openhab.core.addon.marketplace.AbstractRemoteAddonService;
import org.openhab.core.addon.marketplace.MarketplaceAddonHandler;
Expand Down Expand Up @@ -78,8 +79,9 @@ public class JsonAddonService extends AbstractRemoteAddonService {

@Activate
public JsonAddonService(@Reference EventPublisher eventPublisher, @Reference StorageService storageService,
@Reference ConfigurationAdmin configurationAdmin, Map<String, Object> config) {
super(eventPublisher, configurationAdmin, storageService, SERVICE_PID);
@Reference ConfigurationAdmin configurationAdmin, @Reference AddonInfoRegistry addonInfoRegistry,
Map<String, Object> config) {
super(eventPublisher, configurationAdmin, storageService, addonInfoRegistry, SERVICE_PID);
modified(config);
}

Expand Down
6 changes: 4 additions & 2 deletions ...lace/src/test/java/org/openhab/core/addon/marketplace/AbstractRemoteAddonServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import org.mockito.junit.jupiter.MockitoSettings;
import org.mockito.quality.Strictness;
import org.openhab.core.addon.Addon;
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.events.Event;
Expand All @@ -64,6 +65,7 @@
@NonNullByDefault
public class AbstractRemoteAddonServiceTest {
private @Mock @NonNullByDefault({}) StorageService storageService;
private @Mock @NonNullByDefault({}) AddonInfoRegistry addonInfoRegistry;
private @Mock @NonNullByDefault({}) ConfigurationAdmin configurationAdmin;
private @Mock @NonNullByDefault({}) EventPublisher eventPublisher;
private @Mock @NonNullByDefault({}) Configuration configuration;
Expand All @@ -82,7 +84,7 @@ public void initialize() throws IOException {

addonHandler = new TestAddonHandler();

addonService = new TestAddonService(eventPublisher, configurationAdmin, storageService);
addonService = new TestAddonService(eventPublisher, configurationAdmin, storageService, addonInfoRegistry);
addonService.addAddonHandler(addonHandler);
}

Expand All @@ -93,7 +95,7 @@ public void testSourceNotRefreshedIfAddonHandlerNotReady() {
addonHandler = new TestAddonHandler();
addonHandler.setReady(false);

addonService = new TestAddonService(eventPublisher, configurationAdmin, storageService);
addonService = new TestAddonService(eventPublisher, configurationAdmin, storageService, addonInfoRegistry);
addonService.addAddonHandler(addonHandler);

List<Addon> addons = addonService.getAddons(null);
Expand Down
5 changes: 3 additions & 2 deletions ...n.marketplace/src/test/java/org/openhab/core/addon/marketplace/test/TestAddonService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.core.addon.Addon;
import org.openhab.core.addon.AddonInfoRegistry;
import org.openhab.core.addon.marketplace.AbstractRemoteAddonService;
import org.openhab.core.addon.marketplace.BundleVersion;
import org.openhab.core.addon.marketplace.MarketplaceAddonHandler;
Expand Down Expand Up @@ -51,8 +52,8 @@ public class TestAddonService extends AbstractRemoteAddonService {
private int remoteCalls = 0;

public TestAddonService(EventPublisher eventPublisher, ConfigurationAdmin configurationAdmin,
StorageService storageService) {
super(eventPublisher, configurationAdmin, storageService, SERVICE_PID);
StorageService storageService, AddonInfoRegistry addonInfoRegistry) {
super(eventPublisher, configurationAdmin, storageService, addonInfoRegistry, SERVICE_PID);
}

@Override
Expand Down
27 changes: 27 additions & 0 deletions bundles/org.opensmarthouse.core.addon/src/main/java/org/openhab/core/addon/Addon.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,33 @@ public static Builder create(String uid) {
return new Builder(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;
}

public static class Builder {
private final String uid;
private String id;
Expand Down

0 comments on commit 65ea21c

Please sign in to comment.