Something a bit wrong with `@DBusBoundProperty`

[brett-smith]

There appears to be something fundamentally wrong with @DBusBoundProperty. I can't quite put my finger on it yet, but am getting closer.

All seems well if I am importing an object using these annotations into dbus-java from a service that is also using dbus-java to export it (both ends are using the same interface classes but this doesn't matter).

However, if I try to import interfaces into dbus-java from a service exported by something other than dbus-java, it fails.

The following example fails.

public class AccountsTest {
	
	public static void main(String[] args) throws Exception  {
		try(var conx = DBusConnectionBuilder.forSystemBus().build()) {
			var accounts = conx.getRemoteObject("org.freedesktop.Accounts", "/org/freedesktop/Accounts", Accounts.class);
			System.out.println(accounts.getDaemonVersion());
		}
	}
	
	@DBusInterfaceName("org.freedesktop.Accounts")
	public interface Accounts extends DBusInterface, Properties {
	
		@DBusBoundProperty
		public String getDaemonVersion();
	}
	
}

Results in the exception ...

Exception in thread "main" org.freedesktop.dbus.errors.UnknownMethod: No such method “Get”
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480)
	at org.freedesktop.dbus.messages.Error.getException(Error.java:111)
	at org.freedesktop.dbus.messages.Error.throwException(Error.java:138)
	at org.freedesktop.dbus.RemoteInvocationHandler.executeRemoteMethod(RemoteInvocationHandler.java:237)
	at org.freedesktop.dbus.RemoteInvocationHandler.executeRemoteMethod(RemoteInvocationHandler.java:250)
	at org.freedesktop.dbus.propertyref.PropRefRemoteHandler.handleDBusBoundProperty(PropRefRemoteHandler.java:69)
	at org.freedesktop.dbus.RemoteInvocationHandler.invoke(RemoteInvocationHandler.java:94)
	at jdk.proxy2/jdk.proxy2.$Proxy4.getDaemonVersion(Unknown Source)
	at test.AccountsTest.main(AccountsTest.java:14)

Now I'm sure this used to work during the initial development, but I cannot find any changes since that may have broken it, so maybe it never actually worked and I only ever tested with dbus-java on both ends.

I wondered if part of the problem was what was addressed by this PR. https://github.com/hypfvieh/dbus-java/pull/252. However, fixing that made no difference.

Digging deeper, I found the following two changes seem to fix it. I've not made a PR for this yet, as this was just me stabbing randomly at the keyboard until it worked, and it probably breaks other stuff.

This is in PropRefRemoteHandler. It basically changes the interface that is held by RemoteObject to Properties. I am fairly happy this is a correct change, and safe, as it only applies when the annotation is in use.

The second change was in Marshalling. The was necessary because the return value was always coming back as a Variant in this case. This one was a bit brute force, and looks likely to break something else. But it should hopefully be a clue as to what is actually going on.

The fact that it works with dbus-java exporting the service is a bit odd. What is it doing differently I wonder? It seems like it might not be following the specification of Properties properly, and the importing side was incorrectly designed to match that.

[hypfvieh]

I've looked at this case and I agree that something is missing here.

I first investigated what you mentioned about PropRefRemoteHandler. When the method handleDBusBoundProperty is used it is assumed that Properties is supported by the given remote object.

That is wrong. The remote object the method receives is an arbitrary object which is not necessarily related to Properties. In most cases the object behind the remote path is supporting Properties but not necessarily the object passed to this method.

So your fix to create another remote object using Properties will work because the calls are delegated to the object behind the path, not to the given object itself.

For me this seems to be a valid fix. Some tests would be great, but how to test a thing which did work before?

The second thing you mentioned is Marshalling class. I have to admit I don't see why there is a need to change anything. Also I'm not sure what your change should do. When debugging this the parameter is not Variant it is String so the code will never be executed.

The right fix for this is again in PropRefRemoteHandler. When handleDBusBoundProperty has executed the remote method call, it must take care of the result.

What is happening here? The remote call will (hopefully) resolve the requested property and as the javadoc on that method already says, all properties on DBus are always Variant. Variant may not be the type of object we want to return here. This depends on how the @DBusBoundProperty was used (e.g. the return of the method is Variant).

So we may have to unwrap the Variant here to get the correct object type:

Object result = null;

if (access == Access.READ) {
    result = RemoteInvocationHandler.executeRemoteMethod(propertiesRemoteObj, PROP_GET_METHOD,
            new Type[] {_method.getGenericReturnType()}, _conn, RemoteInvocationHandler.CALL_TYPE_SYNC, null, DBusNamingUtil.getInterfaceName(_method.getDeclaringClass()), name);
} else {
    result = RemoteInvocationHandler.executeRemoteMethod(propertiesRemoteObj, PROP_SET_METHOD, variantType,
            new Type[] {_method.getGenericReturnType()}, _conn, RemoteInvocationHandler.CALL_TYPE_SYNC, null, DBusNamingUtil.getInterfaceName(_method.getDeclaringClass()), name, _args[0]);
}

// requested return type is not Variant but the result is -> unwrap Variant
if (_method.getReturnType() != Variant.class && typeClass != Variant.class && result instanceof Variant<?> v) {
    return v.getValue();
}

return result;

I've already updated master branch to include this patch and your patch as well.

[hypfvieh]

I did some more work on this stuff. I found that there are some combinations of implementing Properties and using @DBusBoundProperty annotation which will not work or will not produce the expected results. Especially some instanceof checks with @DBusProperties annotations were definitely wrong as no object exported on/to the bus can/will ever be a subclass of an annotation.

All changes are already in master branch so you can try it when the new snapshot is deployed to snapshot repository.