Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix marketplace add-ons missing config description URI #3688

Merged
merged 1 commit into from
Jul 6, 2023
Merged

Fix marketplace add-ons missing config description URI #3688

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions ...ketplace/src/main/java/org/openhab/core/addon/marketplace/AbstractRemoteAddonService.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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.openhab.core.addon/src/main/java/org/openhab/core/addon/Addon.java
View file
Open in desktop
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