Skip to content

Commit

Permalink
Fix addon name localisation before installation (#3908)
Browse files Browse the repository at this point in the history 
* fix addon name and description

Signed-off-by: Mark Herwege <[email protected]>
  • Loading branch information
@mherwege
mherwege authored Dec 9, 2023
1 parent bb30eab commit 708a954
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 18 deletions.
9 changes: 6 additions & 3 deletions ...on.eclipse/src/main/java/org/openhab/core/addon/eclipse/internal/EclipseAddonService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,12 @@ private Addon getAddon(Bundle bundle, @Nullable Locale locale) {
AddonInfo addonInfo = addonInfoRegistry.getAddonInfo(uid, locale);

if (addonInfo != null) {
// only enrich if this add-on is installed, otherwise wrong data might be added
addon = addon.withLabel(addonInfo.getName()).withDescription(addonInfo.getDescription())
.withConnection(addonInfo.getConnection()).withCountries(addonInfo.getCountries())
if (addonInfo.isMasterAddonInfo()) {
addon = addon.withLabel(addonInfo.getName()).withDescription(addonInfo.getDescription());
} else {
addon = addon.withLabel(name);
}
addon = addon.withConnection(addonInfo.getConnection()).withCountries(addonInfo.getCountries())
.withLink(getDefaultDocumentationLink(type, name))
.withConfigDescriptionURI(addonInfo.getConfigDescriptionURI());
} else {
Expand Down
23 changes: 19 additions & 4 deletions bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/AddonInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@
@NonNullByDefault
public class AddonInfo implements Identifiable<String> {

public static final String NA = "n/a";

private static final Set<String> SUPPORTED_ADDON_TYPES = Set.of("automation", "binding", "misc", "persistence",
"transformation", "ui", "voice");

Expand All @@ -50,10 +48,13 @@ public class AddonInfo implements Identifiable<String> {
private @Nullable String sourceBundle;
private @Nullable List<AddonDiscoveryMethod> discoveryMethods;

private boolean masterAddonInfo = true;

private AddonInfo(String id, String type, @Nullable String uid, String name, String description,
@Nullable String connection, List<String> countries, @Nullable String configDescriptionURI,
@Nullable String serviceId, @Nullable String sourceBundle,
@Nullable List<AddonDiscoveryMethod> discoveryMethods) throws IllegalArgumentException {
@Nullable List<AddonDiscoveryMethod> discoveryMethods, boolean isMasterAddonInfo)
throws IllegalArgumentException {
// mandatory fields
if (id.isBlank()) {
throw new IllegalArgumentException("The ID must neither be null nor empty!");
Expand Down Expand Up @@ -81,6 +82,8 @@ private AddonInfo(String id, String type, @Nullable String uid, String name, Str
this.serviceId = Objects.requireNonNullElse(serviceId, type + "." + id);
this.sourceBundle = sourceBundle;
this.discoveryMethods = discoveryMethods;

this.masterAddonInfo = isMasterAddonInfo;
}

/**
Expand Down Expand Up @@ -155,6 +158,10 @@ public List<AddonDiscoveryMethod> getDiscoveryMethods() {
return discoveryMethods != null ? discoveryMethods : List.of();
}

public boolean isMasterAddonInfo() {
return masterAddonInfo;
}

public static Builder builder(String id, String type) {
return new Builder(id, type);
}
Expand All @@ -177,6 +184,8 @@ public static class Builder {
private @Nullable String sourceBundle;
private @Nullable List<AddonDiscoveryMethod> discoveryMethods;

private boolean masterAddonInfo = true;

private Builder(String id, String type) {
this.id = id;
this.type = type;
Expand All @@ -194,6 +203,7 @@ private Builder(AddonInfo addonInfo) {
this.serviceId = addonInfo.serviceId;
this.sourceBundle = addonInfo.sourceBundle;
this.discoveryMethods = addonInfo.discoveryMethods;
this.masterAddonInfo = addonInfo.masterAddonInfo;
}

public Builder withUID(@Nullable String uid) {
Expand Down Expand Up @@ -246,6 +256,11 @@ public Builder withDiscoveryMethods(@Nullable List<AddonDiscoveryMethod> discove
return this;
}

public Builder isMasterAddonInfo(boolean masterAddonInfo) {
this.masterAddonInfo = masterAddonInfo;
return this;
}

/**
* Build an {@link AddonInfo} from this builder
*
Expand All @@ -254,7 +269,7 @@ public Builder withDiscoveryMethods(@Nullable List<AddonDiscoveryMethod> discove
*/
public AddonInfo build() throws IllegalArgumentException {
return new AddonInfo(id, type, uid, name, description, connection, countries, configDescriptionURI,
serviceId, sourceBundle, discoveryMethods);
serviceId, sourceBundle, discoveryMethods, masterAddonInfo);
}
}
}
20 changes: 12 additions & 8 deletions bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/AddonInfoRegistry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
import org.osgi.service.component.annotations.Reference;
import org.osgi.service.component.annotations.ReferenceCardinality;
import org.osgi.service.component.annotations.ReferencePolicy;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* The {@link AddonInfoRegistry} provides access to {@link AddonInfo} objects.
Expand All @@ -40,6 +42,8 @@
@NonNullByDefault
public class AddonInfoRegistry {

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

private final Collection<AddonInfoProvider> addonInfoProviders = new CopyOnWriteArrayList<>();

@Reference(cardinality = ReferenceCardinality.MULTIPLE, policy = ReferencePolicy.DYNAMIC)
Expand Down Expand Up @@ -85,7 +89,7 @@ public void removeAddonInfoProvider(AddonInfoProvider addonInfoProvider) {
* <p>
* If the first object has a non-null field value the result object takes the first value, or if the second object
* has a non-null field value the result object takes the second value. Otherwise the field remains null.
*
*
* @param a the first {@link AddonInfo} (could be null)
* @param b the second {@link AddonInfo} (could be null)
* @return a new {@link AddonInfo} containing the combined field values (could be null)
Expand All @@ -97,15 +101,15 @@ public void removeAddonInfoProvider(AddonInfoProvider addonInfoProvider) {
return a;
}
AddonInfo.Builder builder = AddonInfo.builder(a);
if (AddonInfo.NA.equals(a.getName())) {
builder.withName(b.getName());
} else if (AddonInfo.NA.equals(b.getName())) {
if (a.isMasterAddonInfo()) {
builder.withName(a.getName());
}
if (AddonInfo.NA.equals(a.getDescription())) {
builder.withDescription(b.getDescription());
} else if (AddonInfo.NA.equals(b.getDescription())) {
builder.withDescription(a.getDescription());
} else {
builder.withName(b.getName());
builder.withDescription(b.getDescription());
}
if (!(a.isMasterAddonInfo() || b.isMasterAddonInfo())) {
builder.isMasterAddonInfo(false);
}
if (a.getConnection() == null && b.getConnection() != null) {
builder.withConnection(b.getConnection());
Expand Down
2 changes: 1 addition & 1 deletion ...e.addon/src/main/java/org/openhab/core/addon/internal/xml/AddonInfoAddonsXmlProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ private void initialize() {
String xml = Files.readString(f.toPath());
if (xml != null && !xml.isBlank()) {
addonInfos.addAll(reader.readFromXML(xml).getAddons().stream()
.map(a -> AddonInfo.builder(a).withName(AddonInfo.NA).withDescription(AddonInfo.NA).build())
.map(a -> AddonInfo.builder(a).isMasterAddonInfo(false).build())
.collect(Collectors.toSet()));
} else {
logger.warn("File '{}' contents are null or empty", f.getName());
Expand Down
8 changes: 6 additions & 2 deletions ...g.openhab.core.karaf/src/main/java/org/openhab/core/karaf/internal/KarafAddonService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,12 @@ private Addon getAddon(Feature feature, @Nullable Locale locale) {
AddonInfo addonInfo = addonInfoRegistry.getAddonInfo(uid, locale);

if (addonInfo != null) {
addon = addon.withLabel(addonInfo.getName()).withDescription(addonInfo.getDescription())
.withConnection(addonInfo.getConnection()).withCountries(addonInfo.getCountries())
if (addonInfo.isMasterAddonInfo()) {
addon = addon.withLabel(addonInfo.getName()).withDescription(addonInfo.getDescription());
} else {
addon = addon.withLabel(feature.getDescription());
}
addon = addon.withConnection(addonInfo.getConnection()).withCountries(addonInfo.getCountries())
.withLink(getDefaultDocumentationLink(type, name))
.withConfigDescriptionURI(addonInfo.getConfigDescriptionURI());
} else {
Expand Down

0 comments on commit 708a954

Please sign in to comment.