Skip to content
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

Properties in auto generated introspection data #130

Closed
mk868 opened this issue Feb 20, 2021 · 4 comments
Closed

Properties in auto generated introspection data #130

mk868 opened this issue Feb 20, 2021 · 4 comments

Comments

@mk868
Copy link
Contributor

mk868 commented Feb 20, 2021

Hello,
I like to use this library at work, so I'm writing with a proposal to extend the Introspection functionality.
Currently, automatically generated introspection data doesn't provide information about properties in the interfaces.
This is obvious because property names/types/access for interfaces are not declared anywhere.
The property information can be helpful during development, especially when using tools like d-feet.

Current state

Let's suppose that we have two interfaces:

@DBusInterfaceName("com.example.Foo")
public interface Foo extends DBusInterface {
  String method1();
}
@DBusInterfaceName("com.example.Bar")
public interface Bar extends DBusInterface {
  String method2(int arg);
}

And we have one object implementing interfaces:

public class ExampleObject implements Foo, Bar, Properties {
  // ...
}

The introspection data generated for exported ExampleObject object:

<!DOCTYPE node PUBLIC "-//freedesktop//DTD D-BUS Object Introspection 1.0//EN" "http://www.freedesktop.org/standards/dbus/1.0/introspect.dtd">
<node name="/aa/bb/ccc1">
 <interface name="com.example.Foo">
  <method name="method1" >
   <arg type="s" direction="out"/>
  </method>
 </interface>
 <interface name="com.example.Bar">
  <method name="method2" >
   <arg type="i" direction="in"/>
   <arg type="s" direction="out"/>
  </method>
 </interface>
 <interface name="org.freedesktop.DBus.Properties">
  <method name="Set" >
   <arg type="s" direction="in"/>
   <arg type="s" direction="in"/>
   <arg type="v" direction="in"/>
  </method>
  <method name="Get" >
   <arg type="s" direction="in"/>
   <arg type="s" direction="in"/>
   <arg type="v" direction="out"/>
  </method>
  <method name="GetAll" >
   <arg type="s" direction="in"/>
   <arg type="a{sv}" direction="out"/>
  </method>
  <signal name="PropertiesChanged">
   <arg type="s" direction="out" />
   <arg type="a{sv}" direction="out" />
   <arg type="as" direction="out" />
  </signal>
 </interface>
 <interface name="org.freedesktop.DBus.Introspectable">
  <method name="Introspect">
   <arg type="s" direction="out"/>
  </method>
 </interface>
 <interface name="org.freedesktop.DBus.Peer">
  <method name="Ping">
  </method>
 </interface>
</node>

DBusProperty annotation proposal

My idea is to create annotation which allows to describe properties in the interface:

@Target(ElementType.TYPE)
@Retention(RetentionPolicy.RUNTIME)
@Repeatable(DBusProperties.class)
public @interface DBusProperty {
  // Property name
  String name() default "";
  // Java type, use in case of base types
  Class<?> typeClass() default Void.class;
  // DBus type as a string, use in case of complex DBus types
  String type() default "";
  // property access type
  Access access() default Access.READ_WRITE;

  enum Access {
    READ,
    READ_WRITE,
    WRITE
  }
}

With this annotation we could append information about the interface properties:

@DBusInterfaceName("com.example.Foo")
@DBusProperty(name="Id", typeClass=String.class, access=Access.READ)
@DBusProperty(name="Label", typeClass=String.class, access=Access.READ_WRITE)
public interface Foo extends DBusInterface {
  String method1();
}
@DBusInterfaceName("com.example.Bar")
@DBusProperty(name="SomeMap", type="a{sv}", access=Access.READ)
public interface Bar extends DBusInterface {
  String method2(int arg);
}

At this point, the introspection generator can discover the properties used in the interface and add them to the response body.
The data from introspection will look like this:

<!DOCTYPE node PUBLIC "-//freedesktop//DTD D-BUS Object Introspection 1.0//EN" "http://www.freedesktop.org/standards/dbus/1.0/introspect.dtd">
<node name="/aa/bb/ccc1">
 <interface name="com.example.Foo">
  <property name="Id" type="s" access="read"/>
  <property name="Label" type="s" access="readwrite"/>
  <method name="method1" >
   <arg type="s" direction="out"/>
  </method>
 </interface>
 <interface name="com.example.Bar">
  <property name="SomeMap" type="a{sv}" access="read"/>
  <method name="method2" >
   <arg type="i" direction="in"/>
   <arg type="s" direction="out"/>
  </method>
 </interface>
 <interface name="org.freedesktop.DBus.Properties">
  <method name="Set" >
   <arg type="s" direction="in"/>
   <arg type="s" direction="in"/>
   <arg type="v" direction="in"/>
  </method>
  <method name="Get" >
   <arg type="s" direction="in"/>
   <arg type="s" direction="in"/>
   <arg type="v" direction="out"/>
  </method>
  <method name="GetAll" >
   <arg type="s" direction="in"/>
   <arg type="a{sv}" direction="out"/>
  </method>
  <signal name="PropertiesChanged">
   <arg type="s" direction="out" />
   <arg type="a{sv}" direction="out" />
   <arg type="as" direction="out" />
  </signal>
 </interface>
 <interface name="org.freedesktop.DBus.Introspectable">
  <method name="Introspect">
   <arg type="s" direction="out"/>
  </method>
 </interface>
 <interface name="org.freedesktop.DBus.Peer">
  <method name="Ping">
  </method>
 </interface>
</node>

With this change, programs like d-feet will be able to fully detect and correctly display properties in interfaces.
I hope for opinions on this solution, if it's acceptable I could prepare a PR with changes

@hypfvieh
Copy link
Owner

Sounds like a good idea. Only thing I did not like about it, is the way "type" is specified.

You expect that the developer adds the DBus type string in "type" field.
Two things:

  1. As it is any arbitrary string, you have to check if this is a valid DBus type when creating introspection data.
  2. Many developers didn't know or care about the DBus representation of a type (including me). Looking up those type stuff is awkward and error prone and will also be difficult if you have really complex types (eg. List of Maps).

I would be more happy with a way to specify the type as classes (e.g. String.class), which will probably not work with complex types, especially generics (how to represent Map<K, V> ?).

I already thought about an extra interface providing a method where all properties should be returned.
But I guess that this will also create difficulties for generics, and one have to use things like the TypeReference class Jackson uses to provide information about the used generic types.
Other ideas still welcome.

Also please note: I will change the libraries license to MIT at the end of the month.
If you create a PR you will agree in the license change, otherwise I have to refuse your PR.

@mk868
Copy link
Contributor Author

mk868 commented Feb 21, 2021

Yes, you're right, type as a String can cause mess and potential typos.
It's better to provide the type as a class.

By default, we can treat maps and lists as follows:

@DBusProperty(type=... Interpreted as DBus type
Map.class Map<Variant, Variant> a{vv}
List.class List<Variant> av

It would be enought for most of users.

Regarding to the TypeReference you mentioned, we can provide full information about generic type by explicitly declaring the "TypeRef" interface:

public interface TypeRef<T> {
}

And use this interface to define the complex type:

@DBusInterfaceName("com.example.Bar")
@DBusProperty(name = "SomeMap", type = ComplexTypeWithMapAndList.class, access = Access.READ)
public interface Bar extends DBusInterface {
  
  // user wants very, very much to provide detailed type information, so he declared such an interface
  interface ComplexTypeWithMapAndList extends TypeRef<Map<String, List<String>>> {
  }
}

It will work similar to the Jackson TypeReference

@hypfvieh
Copy link
Owner

👍
Great, exactly what I was thinking of!

mk868 added a commit to mk868/dbus-java that referenced this issue Feb 22, 2021
hypfvieh added a commit that referenced this issue Feb 23, 2021
@mk868
Copy link
Contributor Author

mk868 commented Feb 23, 2021

Thank you for accepting the PR, This issue can be closed now.

Soon I will prepare a PR with the InterfaceCodeGenerator update.
This way, <property tags will be recognized and added as interface annotations

@mk868 mk868 closed this as completed Feb 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants