-
-
Notifications
You must be signed in to change notification settings - Fork 422
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.
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
Remove JAX-RS Connector (com.eclipsesource.jaxrs) dependency #726
Comments
Because of openhab/openhab-core#726 services are started multiple times. services have been annotated to prevent that from happening. The UPnP Server now also works for IPv6 and respects multiple configured discovery addresses. Writing configAdmin configuration doesn't tear down the entire hue emulation service anymore. Only make the rest endpoints available after everything has been setup and upnp is running. Signed-off-by: David Graeff <david.graeff@web.de>
Because of openhab/openhab-core#726 services are started multiple times. services have been annotated to prevent that from happening. The UPnP Server now also works for IPv6 and respects multiple configured discovery addresses. Writing configAdmin configuration doesn't tear down the entire hue emulation service anymore. Only make the rest endpoints available after everything has been setup and upnp is running. Signed-off-by: David Graeff <david.graeff@web.de>
Because of openhab/openhab-core#726 services are started multiple times. services have been annotated to prevent that from happening. The UPnP Server now also works for IPv6 and respects multiple configured discovery addresses. Writing configAdmin configuration doesn't tear down the entire hue emulation service anymore. Only make the rest endpoints available after everything has been setup and upnp is running. Signed-off-by: David Graeff <david.graeff@web.de>
* Split RESTapi class into logical units (ConfigurationAccess, UserManagement, LightsAndGroups) * rules support * scenes support * schedule support * New self-test page under /api/status * Refactor tests: Use jersey JAX-RS server to test without requiring the framework (non OSGi tests) * Worked around doube-activate problem. Because of openhab/openhab-core#726 services are started multiple times. services have been annotated to prevent that from happening. The UPnP Server now also works for IPv6 and respects multiple configured discovery addresses. Writing configAdmin configuration doesn't tear down the entire hue emulation service anymore. Only make the rest endpoints available after everything has been setup and upnp is running. Signed-off-by: David Graeff <david.graeff@web.de>
IMHO we should migrate to the usage of the R7 JAX-RS whiteboard pattern. @davidgraeff You already summarized what the publisher is doing. For reference I would like to add also the following link: https://github.com/hstaudacher/osgi-jax-rs-connector#publisher I did not test it myself, but what do you think about that idea:
That way we could use both mechanism for the transition phase and migrate one by one. WDYT? |
The JAX-RS Connector ignores routes with a special annotation (forgot the name). My migration PR to the whiteboard pattern did more than just using a different API though. |
If my reading is correct if a At the time I looked at your PR I realized that there is much more cleanup done than just the migration. IMHO it is easier to work with upstream if small and separate (by topic) improvements are provided instead of a big PR that clean ups multiple stuff. That's the reason for my migration path. |
Afair, the only "problematic" REST endpoints were the ones of the sitemaps (or even more specifically, the subscriptions for sitemap changes). |
If even only one implementation is kept that requires the old implementation we need a way to use both implementations in parallel. So, I don't see any reason to require to first migrate all except one. It does not make any difference if we need both in parallel if the migration is step by step. Another option is to wait for a volunteer that invest all the work to migrate all REST implementation except the sitemap one and do a long review session. |
I made it! 😉 |
That sounds like a very nice accomplishment! 😄 🥇 |
Sounds awesome, @maggu2810! |
I realized that the Web UI "cometvisu" also uses the old JAX-RS implementations:
What's your plan? -- WRT the 2.5.x branches of openhab-webui and openhab2-addons the following bundles needs be migrated:
|
Will you be creating PR(s) for solving this issue @maggu2810? That would be highly appreciated! There's quite a significant as reward for solving it as well. 🙂 |
Sorry @wborn, but currently I don't plan to put any effort into openHAB development. |
That's unfortunate to hear @maggu2810, but at least we know someone else will have to put effort into this. Do you perhaps have some branches with your (WIP) changes on GitHub that we can use as source of inspiration? |
What's take on this one? Is it on the roadmap for 3.0? |
Yes it certainly is. That's why I ask around if there are still contributors looking into fixing it. If there are none I might just have to look into it myself one day. 😉 Any previous efforts regarding this would certainly help. There's also David's branch as source of inspiration. |
I also would love to see this solved for 3.0 and I would hope that there isn't even too much work left as @maggu2810 apparently already made it work. I very much hope that he can provide us a link to his work, so that somebody can take it over from there instead of starting from scratch, which would be a real pity. |
You can have a look at into this branch: https://github.com/maggu2810/openhab-core/tree/for-openhab-upstream-to-look-into This commits should be of your interest: |
Many thanks for the branch and pointers to commits @maggu2810! I'll certainly have a look at it. 👍 |
I started porting those commits to the current code base @maggu2810! Let's hope it results in something useful. 😉 |
We made it! :-) The code by @maggu2810 was also top-notch and reusing it saved a lot of time. 👍 |
Yes, awesome work by @maggu2810, a big thanks to him as well! |
What does this dependency do?
It scans all OSGi services (by STARTING THEM!!) for JAX-RS
@Path
or@Provided
annotations and adds them to a servlet.The problems with this dependency
Unfortunately the package is not maintained (no major update since 5 years, no commit in general since 3 years). It is not using newer OSGi features (the entire codebase would shrink by half or more), it uses tycho as its main buildsystem with respective outdated examples, and it does really bad things like actually starting up services to find annotations.
I debugged my Hue Emulation Service for probably 3 hours to find out that double service starts are caused by that bundle. Especially if you do a lot of heavy initialization in activate() (even if async) to throw that away moments later, that adds a lot to OH start up time.
Solution
We have the huge advantage in openHAB, that all our REST endpoints not only are annotated with
@Path
or@Provided
but also implementRestResouce
and register themselves to the rest core bean. We can therefore spin up our own servlet in about 20 lines of code without this really badly designed dependency.Example code (without debouncing/restarting to accommodate for dynamically appearing endpoints):
Notes
This dependency is not related to
com.eclipsesource.jaxrs.provider.swagger
which is also used by openHAB.provider.swagger
configures swagger via a properties file and is apart from that unrelated to jax-rs.The text was updated successfully, but these errors were encountered: