[Bug 28366] Add DBusTube interface support

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=28366

--- Comment #13 from Olli Salli <ollisal at gmail.com> 2011-06-14 08:48:47 PDT ---
(In reply to comment #12)
> NOTE: The branch has been rebased on top of master.
> 
> Everything has been fixed...

+ * DBusTubeChannel features can be enabled by constructing a ChannelFactory
and enabling the desired features,
+ * and passing it to ChannelRequest or ClientRegistrar when creating them as
appropriate. However,

No you don't ever create ChannelRequest objects yourself. The channel factory
has been passed to the AccountManager (or in rare cases a single Account
created directly) in this use case. It suffices to say AccountManager like the
rest of the proxies do.

    // It makes sense only if this is a room, if that's not the case just spit
a warning
    if (parent->targetHandleType() == static_cast<uint>(Tp::HandleTypeRoom)) {
        parent->connect(dbusTubeInterface,
SIGNAL(DBusNamesChanged(Tp::DBusTubeParticipants,Tp::UIntList)),
                        parent,
SLOT(onDBusNamesChanged(Tp::DBusTubeParticipants,Tp::UIntList)));
    } else {
        debug() << "FeatureBusNameMonitoring does not make sense in a P2P
context";
    }

    // Request the current DBusNames property
    connect(dbusTubeInterface->requestPropertyDBusNames(),
SIGNAL(finished(Tp::PendingOperation*)),
            parent,
SLOT(onRequestPropertyDBusNamesFinished(Tp::PendingOperation*)));

You don't need to make the D-Bus call to fetch the DBusNames property either if
the handle type isn't Room. Just finish the feature directly in the else
branch.

Another, more serious flaw is that you currently finish the feature before the
contacts/names actually appear in busNames(), as they go around the queued
contact builder thingy.

Channel only finishes its introspection when all the initial member contacts
have been built and appeared in the member sets, for example. You should do the
same; I think that would be most easily accomplished by ignoring bus name
change signals until you get the initial bus names property value (which you
should do anyway), and then finishing the feature in the contact build slot
once pendingNewBusNamesToAdd becomes empty.

Watch out for the case where there are no initial bus names though, and be sure
to test both cases, and bus name changes during introspection, in your unit
tests.

+    if (parent->immutableProperties().contains(QLatin1String("Service")) &&
+       
parent->immutableProperties().contains(QLatin1String("SupportedAccessControls")))
{

This won't ever be true, because immutableProperties() are fully qualified
property names (the property name is prefixed with the interface name and a
'.'). As a fix, check for fully qualified names here, make extractProperties
take a map with fully qualified property names, and prefix the properties you
get in onRequestAllPropertiesFinished with the interface name to make them the
same format as the immutable props.

> , except:
> (In reply to comment #10)
> > +            } else {
> > +                warning() << "Trying to remove a bus name which has not been
> > retrieved previously!";
> > +            }
> > 
> > You could print the bus name in question.
> 
> Actually no, as I just get a list of contacts. If the contact is not associated
> to any bus name, I cannot retrieve it - I can print the contact id though.

Ah, true, please print the contact id indeed.

> 
> For the rest, I'll provide unit tests with the next update

Ok, looking forward to that.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.