After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 610859 - dbus utilities for creating proxy wrapper classes
dbus utilities for creating proxy wrapper classes
Status: RESOLVED FIXED
Product: gjs
Classification: Bindings
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gjs-maint
gjs-maint
Depends on:
Blocks:
 
 
Reported: 2010-02-23 17:23 UTC by Dan Winship
Modified: 2010-10-29 19:07 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
add imports.misc.dbusUtils, for creating D-Bus proxy wrapper classes (3.05 KB, patch)
2010-02-23 17:23 UTC, Dan Winship
reviewed Details | Review
[dbus] add makeProxyClass(), nameToPath(), and pathToName() (1.58 KB, patch)
2010-02-25 16:49 UTC, Dan Winship
none Details | Review
dbus: add makeProxyClass() (1.33 KB, patch)
2010-10-29 17:33 UTC, Dan Winship
committed Details | Review

Description Dan Winship 2010-02-23 17:23:17 UTC
example:

const CHANNEL_DISPATCH_OPERATION_NAME = TELEPATHY + '.ChannelDispatchOperation';
const ChannelDispatchOperationIface = {
    name: CHANNEL_DISPATCH_OPERATION_NAME,
    methods: [
        { name: 'HandleWith',
          inSignature: 's',
          outSignature: '' },
        { name: 'Claim',
          inSignature: '',
          outSignature: '' }
    ]
};
let ChannelDispatchOperation = DBusUtils.makeProxyClass(ChannelDispatchOperationIface);


later:

        let op = new Telepathy.ChannelDispatchOperation(sender, dispatchOperationPath);
        op.ClaimRemote();
Comment 1 Dan Winship 2010-02-23 17:23:19 UTC
Created attachment 154522 [details] [review]
add imports.misc.dbusUtils, for creating D-Bus proxy wrapper classes
Comment 2 Colin Walters 2010-02-24 17:11:39 UTC
Review of attachment 154522 [details] [review]:

There's two things conceptually here as I see it:

* A function for defining a class, shorthand for creating a prototype which takes the interface and path
* A shorthand for the case where path is a transformation of interface

The first seems like it could definitely go in gjs/modules/dbus.js.  I'm less sure of the second; I see how it's convenient but it still feels a little too "automagic" to me.  If it's really useful though maybe that part could live in telepathyUtils.js or something.
Comment 3 Dan Winship 2010-02-25 15:06:33 UTC
(In reply to comment #2)
> * A function for defining a class, shorthand for creating a prototype which
> takes the interface and path

is that the canonical ordering btw? interface first, path second?

> * A shorthand for the case where path is a transformation of interface
> 
> The first seems like it could definitely go in gjs/modules/dbus.js.  I'm less
> sure of the second; I see how it's convenient but it still feels a little too
> "automagic" to me.

My understanding is that it's pretty common for the path and interface to match. (I think more than half of the uses in telepathyClient.js are like that.)

At least the nameify and pathify functions should probably stick around, even if the magic parts don't?

Also, I realized belated that makeProxyClass assumes you're using the session bus, not the system bus. So what makes more sense:

    let Channel = DBus.session.makeProxyClass(ChannelIface);
    let chan = new Channel(name, path);

    let Channel = DBus.makeProxyClass(DBus.session, ChannelIface);
    let chan = new Channel(name, path);

    let Channel = DBus.makeProxyClass(ChannelIface);
    let chan = new Channel(DBus.session, name, path);

probably the last one, right?
Comment 4 Colin Walters 2010-02-25 15:34:50 UTC
(In reply to comment #3)
> 
> 
> My understanding is that it's pretty common for the path and interface to
> match. (I think more than half of the uses in telepathyClient.js are like
> that.)

Oh it's definitely not unusual.  But think of the path like a variable name, and an interface like a type.  Just because the type is ShellAppSystem, would you want the language to have a default applicationSystem name for the variable?  This is obviously a problem for things that aren't singletons, which is most types.

I guess it's true that in DBus the majority of use is around singletons, but if we have default substitutions like this I think it makes it harder to learn some of the DBus concepts.  And none of the bindings I know of do it currently.

> At least the nameify and pathify functions should probably stick around, even
> if the magic parts don't?

I don't much like them in the default dbus.js but in telepathy.js or whatever seems fine...

> Also, I realized belated that makeProxyClass assumes you're using the session
> bus, not the system bus. So what makes more sense:
> 
>     let Channel = DBus.session.makeProxyClass(ChannelIface);
>     let chan = new Channel(name, path);
> 
>     let Channel = DBus.makeProxyClass(DBus.session, ChannelIface);
>     let chan = new Channel(name, path);
> 
>     let Channel = DBus.makeProxyClass(ChannelIface);
>     let chan = new Channel(DBus.session, name, path);
> 
> probably the last one, right?

I like the second one more; seems like it's more obvious how it works on top of DBus.proxifyPrototype.
Comment 5 Dan Winship 2010-02-25 16:49:14 UTC
Created attachment 154704 [details] [review]
[dbus] add makeProxyClass(), nameToPath(), and pathToName()

I'd already written this based on how I *assumed* Colin was going to answer
my last comment and then he went and answered differently. Anyway:

1) we can remove nameToPath() and pathToName() if you want, but it seems
like this is a pretty common D-Bus operation

2) I ended up implementing makeProxyClass() so that you have to pass either
DBus.session or DBus.system to the constructor (option 3, while Colin
suggested option 2). I can change it to use option 2, but then we can't
define IntrospectableProxy in terms of it, since that needs to be
applicable to both busses.
Comment 6 Dan Winship 2010-10-29 17:33:10 UTC
Created attachment 173513 [details] [review]
dbus: add makeProxyClass()

rebased to master, and with just makeProxyClass (which we now need in
another place in gnome-shell), no nameToPath or pathToName
Comment 7 Colin Walters 2010-10-29 18:29:36 UTC
Review of attachment 173513 [details] [review]:

Looks fine to me.
Comment 8 Dan Winship 2010-10-29 19:07:46 UTC
Attachment 173513 [details] pushed as ac74752 - dbus: add makeProxyClass()