Extend marketplace to accept kar and some improvements #2490

J-N-K · 2021-09-18T10:37:22Z

Following up a discussion with @ghys this adds another MarketplaceAddonHandler for installing .kar files.

Signed-off-by: Jan N. Klug <[email protected]>

wborn · 2021-09-18T11:10:34Z

Yes it would be nice to support KAR files as well. One of the reasons the ESH Marketplace probably didn't support it, could be that this is Karaf only. That's why many Karaf specific functionality is moved into separate bundles to keep it optional when using OHC. That way you can still easily use Felix instead of Karaf as OSGi runtime. Though I don't know how common it is to use OHC with another runtime these days. 🙂 I think this might not make the Marketplace usable/debuggable when used with Eclipse for instance.

J-N-K · 2021-09-18T11:15:16Z

Well, it would be easy to move the class to another bundle if you prefer that. Which one would fit best?

J-N-K · 2021-09-18T11:25:49Z

@wborn Do you know what happens if two feature repositories provide features with the same name (e.g. a .kar has an updated version of a binding that is available in distribution)?

kaikreuzer · 2021-09-18T12:06:47Z

I think it is ok to leave the class (and thus the Karaf dependency) in here. If someone builds their own distro without Karaf, he is likely to also not include the marketplace, which is hosted on our Discourse.

Do you know what happens if two feature repositories provide features with the same name

My guess is that both are available and upon an install (without specifying a specific version) the one with the highest version is picked.

J-N-K · 2021-09-18T12:12:51Z

Ok, we can always make an administrative rule that the marketplace-version should include a qualifier to avoid duplicate version-numbers.

This work is purely "theoretical" as I can't post to the marketplace category on the forum and therefore not test it. When will that be opened?

wborn · 2021-09-18T12:17:59Z

I think it is ok to leave the class (and thus the Karaf dependency) in here. If someone builds their own distro without Karaf, he is likely to also not include the marketplace, which is hosted on our Discourse.

That distro is what you have when debugging openHAB in Eclipse. 😉 With this dependency you cannot run or debug the Marketplace with Eclipse because there are no Karaf bundles in that runtime. You'd probably still be able to run and debug it when the Karaf code is moved into a separate bundle like org.openhab.core.addon.marketplace.karaf.

kaikreuzer · 2021-09-18T12:34:31Z

@wborn Ok, I didn't think about that - this is then indeed be a strong reason to move the class to a dedicated bundle.

Signed-off-by: Jan N. Klug <[email protected]>

wborn · 2021-09-18T12:53:18Z

Good good 👍 I think you are now working on to also add the new bundle to the BOM and Karaf feature file? 🙂

J-N-K · 2021-09-18T12:54:21Z

Sure 😀

I added it to the marketplace feature. I guess that's ok, since the feature requires karaf anyway, so there should be no issue.

Signed-off-by: Jan N. Klug <[email protected]>

wborn

Thanks for working on this! I have a few comments/improvements below:

.../org/openhab/core/addon/marketplace/karaf/internal/community/CommunityKarafAddonHandler.java

.../org/openhab/core/addon/marketplace/internal/community/CommunityMarketplaceAddonService.java

.../org/openhab/core/addon/marketplace/karaf/internal/community/CommunityKarafAddonHandler.java

.../org/openhab/core/addon/marketplace/internal/community/CommunityMarketplaceAddonService.java

.../org/openhab/core/addon/marketplace/karaf/internal/community/CommunityKarafAddonHandler.java

.../org/openhab/core/addon/marketplace/internal/community/CommunityMarketplaceAddonService.java

Signed-off-by: Jan N. Klug <[email protected]>

J-N-K · 2021-09-18T16:01:49Z

Would you like to the the other bundle-addon-handler also refactored to the new Path/Files-API?

wborn · 2021-09-18T16:33:19Z

Yes that would be nice too. 👍

Signed-off-by: Jan N. Klug <[email protected]>

...marketplace/src/main/java/org/openhab/core/addon/marketplace/MarketplaceBundleInstaller.java

Signed-off-by: Jan N. Klug <[email protected]>

J-N-K · 2021-09-18T18:29:55Z

I have implemented what I think should work. The last commit only contains those changes. I'm not sure what to do with the maturity tag, it's included in the tags-property but there is no field in Addon that could be used.

.../org/openhab/core/addon/marketplace/internal/community/CommunityMarketplaceAddonService.java

Signed-off-by: Jan N. Klug <[email protected]>

wborn · 2021-09-18T19:32:32Z

there is no field in Addon that could be used

We can add another field, like all the other fields that were added in #2405, so the info can be shown in the UI and used for filtering.

Signed-off-by: Jan N. Klug <[email protected]>

J-N-K · 2021-09-19T08:51:00Z

I added the field for maturity and license (also that is currently always null). Looking at Addon, I wonder if it really makes sense to do add more fields each time (which will always be breaking) or if we should change to a builder.

Signed-off-by: Jan N. Klug <[email protected]>

J-N-K · 2021-09-19T18:36:04Z

re-triggering build

wborn · 2021-09-19T20:04:40Z

I just tested this PR and got my Marketplace entry succesfully recognized as a KAR:

However it did not install properly. The only logging that showed was:

[Fatal Error] :1:1: Content is not allowed in prolog.

So I added a try/catch to get more detailed logging and this is what showed up:

[ERROR] [.community.CommunityKarafAddonHandler] - Error installing KAR add-on
org.openhab.core.addon.marketplace.MarketplaceHandlerException: Cannot install bundle from marketplace cache: Unable to load file:///home/wouter/software/openhab/instance/userdata/marketplace/kar/126674/org.openhab.binding.systeminfo-3.2.0-SNAPSHOT.kar : file:///home/wouter/software/openhab/instance/userdata/marketplace/kar/126674/org.openhab.binding.systeminfo-3.2.0-SNAPSHOT.kar
	at org.openhab.core.addon.marketplace.karaf.internal.community.CommunityKarafAddonHandler.installFromCache(CommunityKarafAddonHandler.java:166) ~[?:?]
	at org.openhab.core.addon.marketplace.karaf.internal.community.CommunityKarafAddonHandler.install(CommunityKarafAddonHandler.java:103) ~[?:?]
	at org.openhab.core.addon.marketplace.internal.community.CommunityMarketplaceAddonService.install(CommunityMarketplaceAddonService.java:233) ~[?:?]
	at org.openhab.core.io.rest.core.internal.addons.AddonResource.lambda$1(AddonResource.java:223) ~[?:?]
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515) [?:?]
	at java.util.concurrent.FutureTask.run(FutureTask.java:264) [?:?]
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) [?:?]
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) [?:?]
	at java.lang.Thread.run(Thread.java:829) [?:?]
Caused by: java.lang.RuntimeException: Unable to load file:///home/wouter/software/openhab/instance/userdata/marketplace/kar/126674/org.openhab.binding.systeminfo-3.2.0-SNAPSHOT.kar : file:///home/wouter/software/openhab/instance/userdata/marketplace/kar/126674/org.openhab.binding.systeminfo-3.2.0-SNAPSHOT.kar
	at org.apache.karaf.features.internal.service.RepositoryImpl.load(RepositoryImpl.java:121) ~[?:?]
	at org.apache.karaf.features.internal.service.RepositoryImpl.<init>(RepositoryImpl.java:51) ~[?:?]
	at org.apache.karaf.features.internal.service.RepositoryCacheImpl.create(RepositoryCacheImpl.java:51) ~[?:?]
	at org.apache.karaf.features.internal.service.FeaturesServiceImpl.addRepository(FeaturesServiceImpl.java:386) ~[?:?]
	at org.openhab.core.addon.marketplace.karaf.internal.community.CommunityKarafAddonHandler.installFromCache(CommunityKarafAddonHandler.java:164) ~[?:?]
	... 8 more
Caused by: java.lang.RuntimeException: Unable to load file:///home/wouter/software/openhab/instance/userdata/marketplace/kar/126674/org.openhab.binding.systeminfo-3.2.0-SNAPSHOT.kar
	at org.apache.karaf.features.internal.model.JaxbUtil.unmarshalValidate(JaxbUtil.java:139) ~[?:?]
	at org.apache.karaf.features.internal.model.JaxbUtil.unmarshal(JaxbUtil.java:100) ~[?:?]
	at org.apache.karaf.features.internal.service.RepositoryImpl.load(RepositoryImpl.java:118) ~[?:?]
	at org.apache.karaf.features.internal.service.RepositoryImpl.<init>(RepositoryImpl.java:51) ~[?:?]
	at org.apache.karaf.features.internal.service.RepositoryCacheImpl.create(RepositoryCacheImpl.java:51) ~[?:?]
	at org.apache.karaf.features.internal.service.FeaturesServiceImpl.addRepository(FeaturesServiceImpl.java:386) ~[?:?]
	at org.openhab.core.addon.marketplace.karaf.internal.community.CommunityKarafAddonHandler.installFromCache(CommunityKarafAddonHandler.java:164) ~[?:?]
	... 8 more
Caused by: org.xml.sax.SAXParseException: Content is not allowed in prolog.
	at com.sun.org.apache.xerces.internal.parsers.DOMParser.parse(DOMParser.java:261) ~[?:?]
	at com.sun.org.apache.xerces.internal.jaxp.DocumentBuilderImpl.parse(DocumentBuilderImpl.java:339) ~[?:?]
	at javax.xml.parsers.DocumentBuilder.parse(DocumentBuilder.java:122) ~[?:?]
	at org.apache.karaf.util.XmlUtils.parse(XmlUtils.java:65) ~[?:?]
	at org.apache.karaf.features.internal.model.JaxbUtil.unmarshalValidate(JaxbUtil.java:112) ~[?:?]
	at org.apache.karaf.features.internal.model.JaxbUtil.unmarshal(JaxbUtil.java:100) ~[?:?]
	at org.apache.karaf.features.internal.service.RepositoryImpl.load(RepositoryImpl.java:118) ~[?:?]
	at org.apache.karaf.features.internal.service.RepositoryImpl.<init>(RepositoryImpl.java:51) ~[?:?]
	at org.apache.karaf.features.internal.service.RepositoryCacheImpl.create(RepositoryCacheImpl.java:51) ~[?:?]
	at org.apache.karaf.features.internal.service.FeaturesServiceImpl.addRepository(FeaturesServiceImpl.java:386) ~[?:?]
	at org.openhab.core.addon.marketplace.karaf.internal.community.CommunityKarafAddonHandler.installFromCache(CommunityKarafAddonHandler.java:164) ~[?:?]
	... 8 more

Most likely KAR files need to be installed using the KarService and FeaturesServiceImpl.addRepository expects URLs to unpacked repositories?

I tested using this org.openhab.binding.systeminfo-3.2.0-SNAPSHOT.kar. I also checked the file in /userdata/marketplace and the content was as expected. This KAR file works if I copy it into my /addons dir.

Some other things I noticed:

There seems to be a limit of 3 tags on posts:

Also, after installing a KAR file, most likely the UI will then list the add-ons of the KAR as any other "openHAB Distribution" add-on? That can be confusing.

Signed-off-by: Jan N. Klug <[email protected]>

J-N-K · 2021-09-20T15:29:20Z

@wborn Correct, I didn't think about that (I used the FeaturesService to install a feature.xml from a maven repository). KarService looks indeed like the correct one. I changed that, but as I said before: I can't test anything because I don't have access to the marketplace.

WDYT about the builder for Addon? Perhaps it would be better to do that in another PR, also it would simplify the code here and in the existing AddonService

Signed-off-by: Jan N. Klug <[email protected]>

* Automatically start bundles * Fix isInstalled not detecting that KAR is installed * Fix exceptions during uninstallation Signed-off-by: Wouter Born <[email protected]>

.../org/openhab/core/addon/marketplace/karaf/internal/community/CommunityKarafAddonHandler.java

wborn · 2021-09-20T20:34:37Z

Using the KarService it works a lot better. 👍

Though there were a few more issues that I've fixed in my commit. I think it is better to automatically start the bundles or they won't work. That way you can also use feature names that do not start with openhab-binding etc. so they will not show up like "openHAB Distribution" add-ons, which prevents:

Also, after installing a KAR file, most likely the UI will then list the add-ons of the KAR as any other "openHAB Distribution" add-on? That can be confusing.

I also had to manually remove lastnpe and slf4j from my KAR file and features definition. Otherwise it would not properly install, so we probably need to exclude those from KAR generation.

Now it seems to work well and I think it is mergable. 😄

WDYT about the builder for Addon? Perhaps it would be better to do that in another PR, also it would simplify the code here and in the existing AddonService

Yes it is not convenient and ugly to have so many constructor arguments. I'm also fine with doing that in a follow up PR.

features/karaf/openhab-core/src/main/feature/feature.xml

Signed-off-by: Wouter Born <[email protected]>

wborn

I think the code is ready to be merged now. 👍 Can you also review this @openhab/core-maintainers? It also contains a few of my commits.

fwolter

The PR contains .classpath and .project files. Otherwise LGTM.

wborn · 2021-09-21T10:08:11Z

The PR contains .classpath and .project files.

Yes the openhab-core repo still contains these because IIRC there are still some issues to import projects (without compilation errors) in Eclipse using only POMs.

J-N-K · 2021-09-21T12:18:09Z

It would be great if we can enable the category in Discourse then.

ghys · 2021-09-21T17:43:28Z

There seems to be a limit of 3 tags on posts

the brown color & reset button means it was changed from the default (5). I don't know why that is, so I reverted to 5.

Also, after installing a KAR file, most likely the UI will then list the add-ons of the KAR as any other "openHAB Distribution" add-on? That can be confusing.

What do you mean, I didn't understand?

ghys · 2021-09-21T17:53:52Z

I can't test anything because I don't have access to the marketplace.

I created a special group of "marketplace curators" and put you in it, so you should have access. Currently only maintainers and forum moderators can create posts in the marketplace category until we unveil it to the public.

kaikreuzer

lgtm, many thanks! Great to have KAR files supported now.

Also-by: Wouter Born <[email protected]> Signed-off-by: Jan N. Klug <[email protected]> GitOrigin-RevId: 4569eea

J-N-K added 3 commits September 17, 2021 19:10

add marketplace handler for .kar

fba3be5

Signed-off-by: Jan N. Klug <[email protected]>

improvements

3bfff72

Signed-off-by: Jan N. Klug <[email protected]>

improvement

3bfcfcd

Signed-off-by: Jan N. Klug <[email protected]>

J-N-K requested a review from a team as a code owner September 18, 2021 10:37

move to separate bundle

361c5d3

Signed-off-by: Jan N. Klug <[email protected]>

add missing changes to bom and feature

fb8e21e

Signed-off-by: Jan N. Klug <[email protected]>

wborn requested changes Sep 18, 2021

View reviewed changes

address review comments

cd1068b

Signed-off-by: Jan N. Klug <[email protected]>

also refactor MarketPlaceBundleInstaller

6fa6a25

Signed-off-by: Jan N. Klug <[email protected]>

J-N-K commented Sep 18, 2021

View reviewed changes

...marketplace/src/main/java/org/openhab/core/addon/marketplace/MarketplaceBundleInstaller.java Outdated Show resolved Hide resolved

allow different types via tags

ce2b44c

Signed-off-by: Jan N. Klug <[email protected]>

wborn reviewed Sep 18, 2021

View reviewed changes

.../org/openhab/core/addon/marketplace/internal/community/CommunityMarketplaceAddonService.java Outdated Show resolved Hide resolved

address review comment

dba0b83

Signed-off-by: Jan N. Klug <[email protected]>

add fields for code maturity and license

c6f4f17

Signed-off-by: Jan N. Klug <[email protected]>

fix sample addon service

db71472

Signed-off-by: Jan N. Klug <[email protected]>

J-N-K closed this Sep 19, 2021

J-N-K reopened this Sep 19, 2021

use karservice

7ca6f97

Signed-off-by: Jan N. Klug <[email protected]>

J-N-K and others added 2 commits September 20, 2021 20:18

add missing bundle to feature

0232f95

Signed-off-by: Jan N. Klug <[email protected]>

Fix remaining issues

503b450

* Automatically start bundles * Fix isInstalled not detecting that KAR is installed * Fix exceptions during uninstallation Signed-off-by: Wouter Born <[email protected]>

J-N-K commented Sep 20, 2021

View reviewed changes

.../org/openhab/core/addon/marketplace/karaf/internal/community/CommunityKarafAddonHandler.java Outdated Show resolved Hide resolved

wborn reviewed Sep 20, 2021

View reviewed changes

features/karaf/openhab-core/src/main/feature/feature.xml Outdated Show resolved Hide resolved

wborn added 3 commits September 21, 2021 09:06

Use "kar" feature dependency instead of bundle dependency

2eccbad

Signed-off-by: Wouter Born <[email protected]>

Revert Addon.getCountries JavaDoc change

35d3e5d

Signed-off-by: Wouter Born <[email protected]>

More polishing

9ed5080

Signed-off-by: Wouter Born <[email protected]>

wborn approved these changes Sep 21, 2021

View reviewed changes

wborn requested a review from a team September 21, 2021 09:26

fwolter approved these changes Sep 21, 2021

View reviewed changes

kaikreuzer approved these changes Sep 23, 2021

View reviewed changes

kaikreuzer merged commit 4569eea into openhab:main Sep 23, 2021

wborn added the enhancement An enhancement or new feature of the Core label Sep 23, 2021

wborn added this to the 3.2 milestone Sep 23, 2021

J-N-K deleted the add-kar-addon-service branch September 23, 2021 17:04

J-N-K mentioned this pull request Sep 27, 2021

replace multi-parameter constructor with builder on org.openhab.core.Addon #2499

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extend marketplace to accept kar and some improvements #2490

Extend marketplace to accept kar and some improvements #2490

J-N-K commented Sep 18, 2021

wborn commented Sep 18, 2021 •

edited

Loading

J-N-K commented Sep 18, 2021

J-N-K commented Sep 18, 2021

kaikreuzer commented Sep 18, 2021

J-N-K commented Sep 18, 2021

wborn commented Sep 18, 2021

kaikreuzer commented Sep 18, 2021

wborn commented Sep 18, 2021

J-N-K commented Sep 18, 2021 •

edited

Loading

wborn left a comment •

edited

Loading

J-N-K commented Sep 18, 2021

wborn commented Sep 18, 2021

J-N-K commented Sep 18, 2021

wborn commented Sep 18, 2021

J-N-K commented Sep 19, 2021

J-N-K commented Sep 19, 2021

wborn commented Sep 19, 2021 •

edited

Loading

J-N-K commented Sep 20, 2021 •

edited

Loading

wborn commented Sep 20, 2021 •

edited

Loading

wborn left a comment •

edited

Loading

fwolter left a comment

wborn commented Sep 21, 2021

J-N-K commented Sep 21, 2021

ghys commented Sep 21, 2021 •

edited

Loading

ghys commented Sep 21, 2021

kaikreuzer left a comment

Extend marketplace to accept kar and some improvements #2490

Extend marketplace to accept kar and some improvements #2490

Conversation

J-N-K commented Sep 18, 2021

wborn commented Sep 18, 2021 • edited Loading

J-N-K commented Sep 18, 2021

J-N-K commented Sep 18, 2021

kaikreuzer commented Sep 18, 2021

J-N-K commented Sep 18, 2021

wborn commented Sep 18, 2021

kaikreuzer commented Sep 18, 2021

wborn commented Sep 18, 2021

J-N-K commented Sep 18, 2021 • edited Loading

wborn left a comment • edited Loading

Choose a reason for hiding this comment

J-N-K commented Sep 18, 2021

wborn commented Sep 18, 2021

J-N-K commented Sep 18, 2021

wborn commented Sep 18, 2021

J-N-K commented Sep 19, 2021

J-N-K commented Sep 19, 2021

wborn commented Sep 19, 2021 • edited Loading

J-N-K commented Sep 20, 2021 • edited Loading

wborn commented Sep 20, 2021 • edited Loading

wborn left a comment • edited Loading

Choose a reason for hiding this comment

fwolter left a comment

Choose a reason for hiding this comment

wborn commented Sep 21, 2021

J-N-K commented Sep 21, 2021

ghys commented Sep 21, 2021 • edited Loading

ghys commented Sep 21, 2021

kaikreuzer left a comment

Choose a reason for hiding this comment

wborn commented Sep 18, 2021 •

edited

Loading

J-N-K commented Sep 18, 2021 •

edited

Loading

wborn left a comment •

edited

Loading

wborn commented Sep 19, 2021 •

edited

Loading

J-N-K commented Sep 20, 2021 •

edited

Loading

wborn commented Sep 20, 2021 •

edited

Loading

wborn left a comment •

edited

Loading

ghys commented Sep 21, 2021 •

edited

Loading