hypfvieh / dbus-java

Improved version of java DBus library provided by freedesktop.org (https://dbus.freedesktop.org/doc/dbus-java/)
https://hypfvieh.github.io/dbus-java/
MIT License
185 stars 73 forks source link

Exporting concrete class no longer works #157

Closed brett-smith closed 2 years ago

brett-smith commented 2 years ago

A regression seems to have crept into 4.0.0. In the 3.x branch, the following worked.

conn.exportObject("/", new MyService());

and

public class MyService extends DBusInterface {
    public String getMessage() {
       return "Hello!";
    }
}

This no longer actually exports the interface to the bus. It doesn't error, it simply isn't there. I tried adding @DBusInterfaceName, but this made no difference.

The only way I could make this work was by hacking ExportedObject around line 275. I commented out the clazz.isInterface().

 List<Class<?>> interfaces = Arrays.asList(clazz.getInterfaces());
            if (/* clazz.isInterface() && */ interfaces.contains(DBusInterface.class)) {
                // clazz is interface and directly extends the DBusInterface
                result.add(clazz);
            }

Am I missing something here? exportObject() takes an instance of an object, not an interface. If I were to create an interface, how would that be exported and bound to the instance?

Edit: Looks like its something to do with this. https://github.com/hypfvieh/dbus-java/pull/143

hypfvieh commented 2 years ago

Your sample code is invalid and will not even compile because you "extend" DBusInterface (which is an interface) using a class.

All exported objects on the bus should implement an interface which inherits from DBusInterface and contains all methods which should be exposed on the bus.

If it did work without interface before it was simply "wrong" and was an unintended behaviour.

Exposing object to the bus was explained here: https://hypfvieh.github.io/dbus-java/exporting-objects.html There is and was no guarantee that dbus-java will work as expected when not using the intended way described in the link.

brett-smith commented 2 years ago

Sorry, late night coding mistake :\

That should have said implements not extends.

public class MyService implements DBusInterface {
    public String getMessage() {
       return "Hello!";
    }
}

So to confirm, to export an object you must always split it into an interface and a concrete class, even if you are not going to use the interface in any client code?

This most definitely did work in version 3. I had an application in production that only exports a simple service for other clients (clients are not using Java, so I don't need the interface in any of my own Java code). I recently updated this from dbus version 3 to 4 with no other changes, which broke because of this.

If this really is the new behaviour, an exception might be nice on exportObject() if nothing is actually exported?

hypfvieh commented 2 years ago

The main reason for providing interfaces is to support importing something from the bus. In this case it a class would not make sense because the "real implementation" is called remotely. Dbus-java will create a proxy object which will delegate all calls to DBus which will than call the methods on the providing end of the communication returned whatever it produces.

In this case it has something to do with the introspection data which is used to tell other applications which interfaces, methods and properties are supported by our application (so we are the "remote" end). I think it would not hurt anyone to support classes directly using DBusInterface when creating introspection data.

I updated the code to allow classes as well as interfaces when gathering introspection data. I also added a unit test to be sure that the created introspection data will only contain each interface once.

The patch provided in PR #143 fixes some serious issues when creating introspection data if a class implement multiple interfaces which may extend the same interface (e.g. Foo extends Bar,Baz; Bar extends Properties; Baz extends Properties). This setup caused multiple (identical) interface export data to be present in introspection data (see PR for details).

Nevertheless I would expect dbus-java 3.3.1 to behave exactly like 4.0.0 because the PR was pulled-in in both releases.

brett-smith commented 2 years ago

Awesome, understood.

I checked the history of the versions I used in this project. I started of with 3.0.2, then used several versions up to 3.3.0. Then finally jumped to 4.0.0 which is when this happened. So that ties in, I never actually used 3.3.1.

I didn't notice this when working with the 4.0.0-SNAPSHOT, but I was entirely focussed on a different app, that did define a separate interface.

hypfvieh commented 2 years ago

I guess that would explain why you didn't see this in 3.x series. Anyways I would strongly suggest creating and using proper sub-interfaces of DBusInterface instead of implementing it directly.

As 4.0.0 was just released and I don't see that this issue is very critical, I close this issue as it will be fixed with 4.0.1 whenever this will be released.

For all others reading this: The workaround is to use a proper interface sub-classing DBusInterface and implementing that to your class. This is the way it was intended to be used and will also provide all interfaces required to access/use your exported object from DBus.