Webhook service

[digitaldan]

This adds a new webhook service for bindings who provide public webhooks, like the cloud binding
see openhab/openhab-addons#20486 for more info . Right now bindings that want to use the cloud binding's webhook functionality have to go through some ugly hacks to avoid java dependencies between them.

[digitaldan]

Leaving as a draft for review.

[mherwege]

I am not convinced yet. If I understand this well, you would need the openhab cloud addon installed for a binding to be able to use this. This then shields you from having to define this dependency. And it becomes up to the user (and documentation) to make sure the dependency is met. It also creates potential issues with startup sequence.
Why is relying on explicitly defining the dependency in the feature of the addon that requires this bad? It may be less convenient, but we went the same way when splitting up e.g. the mqtt binding.

[digitaldan]

but we went the same way when splitting up e.g. the mqtt binding.

Did this not create a hard dependency where installing one of the split out sub-bindings still installs the base mqtt binding? In this case, would installing a binding that could optionally use the cloud webhook service then force the cloud binding to get installed even if the user did not intend to use it? I'm not super familiar with how that was broken out.

[mherwege]

Did this not create a hard dependency where installing one of the split out sub-bindings still installs the base mqtt binding?

In that case, yes. But I believe it is possible to define a karaf feature as optional. It should then be possible to check in the code if the webhook is there, and if not, log a message to install the openhab cloud addon, or not have the specific function available when the cloud addon is not installed.

[digitaldan]

I’m not sure i understand how that would work to avoid the crazy reflection i have to use right now to invoke functions on the cloud binding to avoid importing cloud binding classes, which would throw a runtime exception if the cloud binding was not installed. Right now the Twilio binding is doing this, and it does work, but it’s ugly and i know there are other bindings who will want to optionally use this.

[digitaldan]

the Twilio solution for this just fyi

https://github.com/openhab/openhab-addons/blob/c8dae90cbe50793a794a902bb4cdc9f625c6ed88/bundles/org.openhab.binding.twilio/src/main/java/org/openhab/binding/twilio/internal/service/TwilioCloudWebhookService.java#L131-L182

[mherwege]

The reflection is not nice. I was under the impression that @lo92fr was able to make it work without reflection (openhab/openhab-addons#20486 (comment)), but I don't find it in his code. I did see he was using adding the bnd dependency in his pom, but I don't see anything in the feature.xml.
I have not tried or tested any of this. I am just a bit worried to introduce an interface in core that can only do something if an addon is installed. I don't think there is any other mechanism in OH that uses an interface in core to create dependencies between addons. I don't find it very comforting to do that.

[lo92fr]

Hello,

Yes as said yesterday I've finally was able to prevent reflexion using an optional dependency.
You can look in my github repos, branch smartthingsNG.

There is mainly 2 important point:

• Definition of depedency in pom.xml : https://github.com/lo92fr/openhab-addons/blob/smartthingsNG/bundles/org.openhab.binding.smartthings/pom.xml
• Using a ServiceTracker to get back the dependency (see SmartThingsBridgeHandler.java)
  https://github.com/lo92fr/openhab-addons/blob/smartthingsNG/bundles/org.openhab.binding.smartthings/src/main/java/org/openhab/binding/smartthings/internal/handler/SmartThingsBridgeHandler.java

Not need to add anythings else in feature.xml

Say that, I think it's not ideal, very complex to implement.
I'm think that adding an interface directly in core would be much more help in this case.
And I think it make really sense for such a central addon as openhab cloud service.

@mherwege: I understand you concerns about adding specific binding mechanism in core.
But the question there is if the cloud binding is not somewhere such a central mechanism of openHab so that we could accept to have some base interface in core for it.

Laurent

[mherwege]

But the question there is if the cloud binding is not somewhere such a central mechanism of openHab so that we could accept to have some base interface in core for it.

Shouldn't we then move it to core and avoid the problem?

It is probably my knowlege that is lacking here. But does only having an interface in core solve the problem of the openhab cloud addon that maybe is not installed? If calling the methods in the interface, and the addon is not installed, what will happen? Don't we need a mechanism for the binding to query if there is an implementation for the webhook service available before even trying to call it? I would expect you need to build logic in core now to handle that.
I am thinking about an analogy with a persistence service (admitted, not to be called directly from a binding), where we have a registry and mechanisms to check if a persistence service is installed before doing anything.

[lo92fr]

In my sense, I would up-vote for moving full openhab cloud to core.
For me it would be great if it's enable for everyone at the first startup !
But I'm not sure that everyone is ready for this :)

In regards to persistance, you comparison are good. But we have many implementation for persistance, only one for cloud !

[digitaldan]

Yes as said yesterday I've finally was able to prevent reflexion using an optional dependency.
You can look in my github repos, branch smartthingsNG

I’m confused, you are importing import org.openhab.io.openhabcloud.WebhookService in your class , which is why you don’t need reflection, but if the cloud binding is not installed, regardless of the build requirement, does that not throw a class not found error at runtime?