Bug 23148 (tp-spec-1.0)

Additional small things:

Account.Valid, AccountManager.ValidAccounts etc. should be renamed to "Usable" - "valid" already means something in a lot of APIs.

*** Bug 24624 has been marked as a duplicate of this bug. ***

Other changes that are easy enough that we can just do them immediately:

* the former Bug #24624: relax .client naming as outlined on that bug (requested by Sjoerd)

* merge Requests and Contacts into the core Connection interface

* merge Messages and Text into the Text channel type, possibly renaming it to Chan.T.Messages in the process

* rename the "WithHints" variants of the CD methods to remove the suffix, deleting the old simple versions

* rename CR.SucceededWithChannel to Succeeded and delete the old simple version

* delete all deprecated things, including StreamedMedia, complex Presence and simple Capabilities

* promote Account.Parameters to its own interface (rationale: users of accounts that aren't account-editing UIs don't care; requested by Sjoerd, IIRC)

* use o.fd.DBus.Properties.PropertiesChanged for at least Account

* rename Handle_Type to Target_Type and TargetHandleType to TargetType

* consider reorganizing Observer, Approver and Handler methods to promote things from the a{sv} of misc to the top level

Related to Bug #32468: change the expected RCC for ContactSearch to fix the TargetHandleType to NONE, or remove the SHOULD in Channel_Class

(In reply to comment #1)
> * merge Messages and Text into the Text channel type, possibly renaming it to
> Chan.T.Messages in the process
> * rename the "WithHints" variants of the CD methods to remove the suffix,
> deleting the old simple versions
> * rename CR.SucceededWithChannel to Succeeded and delete the old simple version
> 
> * delete all deprecated things, including StreamedMedia, complex Presence and
> simple Capabilities

Jonny did this, it looks good

> Account.Valid, AccountManager.ValidAccounts etc. should be renamed to "Usable"
> - "valid" already means something in a lot of APIs.
> * the former Bug #24624: relax .client naming as outlined on that bug
> (requested by Sjoerd)
> * Related to Bug #32468: change the expected RCC for ContactSearch to fix the
> TargetHandleType to NONE, or remove the SHOULD in Channel_Class

Done on my 'next' branch, Jonny is reviewing these

(In reply to comment #3)
> * merge Requests and Contacts into the core Connection interface
> * promote Account.Parameters to its own interface (rationale: users of accounts
> that aren't account-editing UIs don't care; requested by Sjoerd, IIRC)
> * use o.fd.DBus.Properties.PropertiesChanged for at least Account
> * rename Handle_Type to Target_Type and TargetHandleType to TargetType
> * consider reorganizing Observer, Approver and Handler methods to promote
> things from the a{sv} of misc to the top level

Not done yet

Still left TODO I think:

> * promote Account.Parameters to its own interface (rationale: users of
> accounts that aren't account-editing UIs don't care; requested by Sjoerd,
> IIRC)

What's the benefit of this? It's not like Parameters were changing a lot.

> * use o.fd.DBus.Properties.PropertiesChanged for at least Account

Good to have I think.

> * rename Handle_Type to Target_Type and TargetHandleType to TargetType

Do we still want this?

> * consider reorganizing Observer, Approver and Handler methods to promote
> things from the a{sv} of misc to the top level

So we should have:

  - ObserveChannel (o: Account, o: Connection, o: Channel, a{sv}: Channel_Properties, o: Dispatch_Operation, a{oa{sv}}: Requests_Satisfied, b: Recovering, a{sv}: Observer_Info) 

  - HandleChannel (o: Account, o: Connection, o: Channel, a{sv}: Channel_Properties, a{oa{sv}}: Requests_Satisfied, x: User_Action_Time, a{sv}: Handler_Info)

(In reply to comment #6)
> So we should have:
> 
>   - ObserveChannel (o: Account, o: Connection, o: Channel, a{sv}:
> Channel_Properties, o: Dispatch_Operation, a{oa{sv}}: Requests_Satisfied, b:
> Recovering, a{sv}: Observer_Info) 
> 
>   - HandleChannel (o: Account, o: Connection, o: Channel, a{sv}:
> Channel_Properties, a{oa{sv}}: Requests_Satisfied, x: User_Action_Time,
> a{sv}: Handler_Info)

Something like that, yeah.

I'm not sure that moving Recovering out of Observer_Info is worth it, though - it's quite a niche thing anyway?

(In reply to comment #6)
> > * promote Account.Parameters to its own interface (rationale: users of
> > accounts that aren't account-editing UIs don't care; requested by Sjoerd,
> > IIRC)
> 
> What's the benefit of this? It's not like Parameters were changing a lot.

I forget. It might have been about change-notification noise, or it might have been "if the Parameters have public keys, etc., in them, then they could be pretty large to be in every Tp process's memory"?

> > * use o.fd.DBus.Properties.PropertiesChanged for at least Account
> 
> Good to have I think.

Also not rocket science.

> 
> > * rename Handle_Type to Target_Type and TargetHandleType to TargetType
> 
> Do we still want this?

Given that we've mostly eradicated handles from our high-level API, it seems somewhat appealing to remove that bit of jargon too; OTOH, perhaps this is one sed too many.

(In reply to comment #7)
> (In reply to comment #6)
> > So we should have:
> > 
> >   - ObserveChannel (o: Account, o: Connection, o: Channel, a{sv}:
> > Channel_Properties, o: Dispatch_Operation, a{oa{sv}}: Requests_Satisfied, b:
> > Recovering, a{sv}: Observer_Info) 
> > 
> >   - HandleChannel (o: Account, o: Connection, o: Channel, a{sv}:
> > Channel_Properties, a{oa{sv}}: Requests_Satisfied, x: User_Action_Time,
> > a{sv}: Handler_Info)
> 
> Something like that, yeah.

OK, I'll do that.

> I'm not sure that moving Recovering out of Observer_Info is worth it, though
> - it's quite a niche thing anyway?

Agreed, I wasn't so sure about this one either.

(In reply to comment #8)
> (In reply to comment #6)
> > > * promote Account.Parameters to its own interface (rationale: users of
> > > accounts that aren't account-editing UIs don't care; requested by Sjoerd,
> > > IIRC)
> > 
> > What's the benefit of this? It's not like Parameters were changing a lot.
> 
> I forget. It might have been about change-notification noise, or it might
> have been "if the Parameters have public keys, etc., in them, then they
> could be pretty large to be in every Tp process's memory"?

< sjoerd> smcv: I think i remember what the main reasn was for 
          the splitting of the parameters, it meant that your user 
          password constantly flew over the bus which annoyed me

... which is no longer true under a "normal" configuration even in Telepathy 0.x, because we recommend SASL channels over secret parameters now. So this is no longer an issue, and we don't really need to change this.

Here we go:
http://cgit.collabora.com/git/user/cassidy/telepathy-glib/log/?h=next-client
http://cgit.collabora.com/git/user/cassidy/telepathy-spec/log/?h=next-client

I still have to do MC.

(In reply to comment #11)
> Here we go:
> http://cgit.collabora.com/git/user/cassidy/telepathy-glib/log/?h=next-client
> http://cgit.collabora.com/git/user/cassidy/telepathy-spec/log/?h=next-client
> 
> I still have to do MC.

Looks good so far. Remember to squash the "fixup!" commit in -glib.

(In reply to comment #11)
> I still have to do MC.

http://cgit.collabora.com/git/user/cassidy/telepathy-mission-control/log/?h=next-client

(In reply to comment #13)
> http://cgit.collabora.com/git/user/cassidy/telepathy-mission-control/log/?h=next-client

Looks good, please squash the fixup commit in tp-glib and merge all three. I assume Empathy uses high-level API for everything anyway.

(In reply to comment #14)
> (In reply to comment #13)
> > http://cgit.collabora.com/git/user/cassidy/telepathy-mission-control/log/?h=next-client
> 
> Looks good, please squash the fixup commit in tp-glib and merge all three. I
> assume Empathy uses high-level API for everything anyway.

I merged the 3 branches.

(In reply to comment #8)
> > > * use o.fd.DBus.Properties.PropertiesChanged for at least Account
> > 
> > Good to have I think.
> 
> Also not rocket science.

http://cgit.collabora.com/git/user/cassidy/telepathy-spec/log/?h=next-account
http://cgit.collabora.com/git/user/cassidy/telepathy-glib/log/?h=next-account
http://cgit.collabora.com/git/user/cassidy/telepathy-mission-control/log/?h=next-account

(In reply to comment #16)
> http://cgit.collabora.com/git/user/cassidy/telepathy-spec/log/?h=next-account
> http://cgit.collabora.com/git/user/cassidy/telepathy-glib/log/?h=next-account
> http://cgit.collabora.com/git/user/cassidy/telepathy-mission-control/log/?h=next-account

Looks good!

> + <annotation name="org.freedesktop.DBus.Property.EmitsChangedSignal"

I was about to say "s/Property/Properties/" but then I looked at the D-Bus Specification, and you're right, it does have this weird name. :-(

(In reply to comment #17)
> (In reply to comment #16)
> > http://cgit.collabora.com/git/user/cassidy/telepathy-spec/log/?h=next-account
> > http://cgit.collabora.com/git/user/cassidy/telepathy-glib/log/?h=next-account
> > http://cgit.collabora.com/git/user/cassidy/telepathy-mission-control/log/?h=next-account
> 
> Looks good!

Merged.

(In reply to comment #8)
> > > * rename Handle_Type to Target_Type and TargetHandleType to TargetType
> > 
> > Do we still want this?
> 
> Given that we've mostly eradicated handles from our high-level API, it seems
> somewhat appealing to remove that bit of jargon too; OTOH, perhaps this is
> one sed too many.

Shouldn't we remove Channel.TargetHandle as well then?

(In reply to comment #19)
> (In reply to comment #8)
> > > > * rename Handle_Type to Target_Type and TargetHandleType to TargetType
> > > 
> > > Do we still want this?
> > 
> > Given that we've mostly eradicated handles from our high-level API, it seems
> > somewhat appealing to remove that bit of jargon too; OTOH, perhaps this is
> > one sed too many.
> 
> Shouldn't we remove Channel.TargetHandle as well then?

If we don't we'll have to keep describing the exact semantic of TargetHandle using TargetType; is that what we want?

Also, the current description of Target_Type_None would be something like:
""""A 'null' target type used to indicate the absence of a handle. When a target type and a handle appear as a pair, if the target type is zero, the handle must also be zero."""

That sounds a bit weird to me but maybe that's ok?

(In reply to comment #19)
> (In reply to comment #8)
> > > > * rename Handle_Type to Target_Type and TargetHandleType to TargetType
> > > 
> > > Do we still want this?
> > 
> > Given that we've mostly eradicated handles from our high-level API, it seems
> > somewhat appealing to remove that bit of jargon too; OTOH, perhaps this is
> > one sed too many.
> 
> Shouldn't we remove Channel.TargetHandle as well then?

We still need handles *and* IDs for as long as our general-purpose APIs (mostly meaning the ones called by TpContact) primarily take handles rather than IDs as their parameters. For instance, RequestContactInfo() takes a handle.

All I'm trying to do right now is to change the conceptual model from:

    A channel has a *target handle*, which (like all handles) has a
    *handle type*. It also has a *target identifier* which is the string
    form of the handle

to:

    A channel has a *target*, represented as a pair
    (target type, identifier) or equivalently (target type, handle)

which makes it a D-Bus-level implementation detail whether the handle or the identifier is more useful.

I don't think it's worthwhile to change all the D-Bus APIs from handle-based to ID-based for Telepathy 1.0, but what I'm aiming for here is that if we want to, we can make that change for (say) Telepathy 1.2 by breaking D-Bus API, without having to also break the GLib- and Qt-level API.

(In reply to comment #20)
> If we don't we'll have to keep describing the exact semantic of TargetHandle
> using TargetType; is that what we want?

We only have None, Contact and Room at the moment, right? I'll try to sketch out some wording.

Target_Type
    Enumeration describing the type of entity with which a channel
    communicates.

    For instance, a text chat or VoIP call with a person or automated system has
    target type "contact", a text chatroom or a VoIP call to a conferencing
    system ideally has target type "room", and a channel listing chatrooms
    available on a server has target type "none".

    None (0)
        The absence of a target, contact, room, etc. For instance,
        _ContactSearch_, _RoomList_, _ServerAuthentication_ and
        _ServerTLSConnection_ channels do not communicate with a specific
        entity, so they have this channel type.

        This channel type is also used for chatrooms that cannot be
        addressed by name (so users cannot join by typing a name or number,
        only by being invited), such as multi-user "switchboards" in MSNP.

        Where this appears alongside a _Handle_ and/or _Identifier_,
        the handle MUST be 0 and the identifier MUST be the empty string.

    Contact (1)
        A contact, i.e. a person or automated system.

        The corresponding identifier is a normalized form of the
        contact's unique machine-readable identifier in this particular
        protocol (such as an XMPP JID, SIP URI, AIM screen-name, ICQ number
        or IRC nickname). For instance, any parts of the identifier
        that are defined to be case-insensitive by the protocol SHOULD
        be normalized to lower-case.

        In protocols where conferencing systems cannot reliably be
        distinguished from individuals, conferencing systems MAY
        also appear with Target_Type_Contact.

        | [rationale]
        | For instance, calls to/from SIP teleconferencing "rooms"
        | typically appear as a simple SIP URI

    Room (2)
        A chatroom or teleconferencing system that can be addressed
        by name.

        The corresponding identifier is a normalized form of the
        room's unique machine-readable identifier in this particular
        protocol, such as the JID "jdev@conference.jabber.org"
        on XMPP, or the channel name "#telepathy" on IRC.

Handle
    An unsigned 32-bit integer representing a string. A handle is
    meaningless on its own, and must be paired with a _Target_Type_.
    Among nonzero handles with the same Target_Type, there is a
    bidirectional mapping between handles and strings.

    The handle 0 is special, and represents the absence of a
    contact, room etc. It corresponds to Target_Type_None.

Contact_Handle
    A nonzero _Handle_ of type _Target_Type_Contact_, or 0 to represent
    the absence of a contact (_Target_Type_None_).

Room_Handle
    A nonzero _Handle_ of type _Target_Type_Room_, or 0 to represent
    the absence of a room (_Target_Type_None_).

We could also consider stealing terminology from telepathy-logger and calling it "entity type"... but Tpl's TplEntityType has a separate entry for "self", which is represented as a Contact in Tp. I suppose there's nothing to stop us from introducing Entity_Type_Self and documenting that the Self and Contact "namespaces" overlap?

Another thing we could consider would be moving the Target_Type_None channels to Target_Type_Server (Bug #70489).

(In reply to comment #23)
> We could also consider stealing terminology from telepathy-logger and
> calling it "entity type"... but Tpl's TplEntityType has a separate entry for
> "self", which is represented as a Contact in Tp. I suppose there's nothing
> to stop us from introducing Entity_Type_Self and documenting that the Self
> and Contact "namespaces" overlap?

"Entity type" may be more generic if we ever use it outside of the context of a channel.

> Another thing we could consider would be moving the Target_Type_None
> channels to Target_Type_Server (Bug #70489).

That makes sense. 'Server' is much clearer than 'None'.

It may just be a bit weird for unamed chatrooms as the channel is actually communicating with a bunch of people and not to the server directly.

(In reply to comment #24)
> > Another thing we could consider would be moving the Target_Type_None
> > channels to Target_Type_Server (Bug #70489).
> 
> That makes sense. 'Server' is much clearer than 'None'.
> 
> It may just be a bit weird for unamed chatrooms as the channel is actually
> communicating with a bunch of people and not to the server directly.

Right, we can't just rename it (because there are type-None channels that genuinely do need that type); we'd have to keep None and *add* Server.

(In reply to comment #25)
> Right, we can't just rename it (because there are type-None channels that
> genuinely do need that type); we'd have to keep None and *add* Server.

My intention on Bug #70489 was that if it worked like this, it'd be with (type, ID) pairs of the form (Target_Type_Server, "collabora.co.uk").

That could replace Channel.Type.Foo.Server in several places, but I suspect we'd still want Target_Type_None (or some other way to say "unspecified server") *anyway*, for non-federated protocols where you're communicating with some vague cloud of servers whose names are not discoverable or particularly relevant (MSNP, Skype, AIM, ICQ etc.).

Ok, so let's add Type_Server in Bug #70489 but for now we should first do this rename and so decide between Target_Type_* or Entity_Type_*.
I don't have a strong opinion for either so just pick one. :)

<smcv> cassidy: I think I slightly lean towards Entity_Type_* if you make them numerically equal to the ones in Logger
<smcv> cassidy: which would require changing Logger's ABI to insert 0 = None, and either having an Entity_Type_Self in the tp-spec or permanently reserving a number for it
<smcv> cassidy: Entity_Type_Self would say "not used within Telepathy itself, but reserved for use by telepathy-logger" or some such?
<cassidy> yeah we could do that
<smcv> cassidy: indeed TplEntityType could become a typedef for TpEntityType, or something
<cassidy> ok so: a) Renaming TargetHandleType to EntityType
<cassidy> b) add Entity_Type_Self
<cassidy> c) use those in logger
<cassidy> d) add Entity_Type_Server

(In reply to comment #28)
> <cassidy> ok so: a) Renaming TargetHandleType to EntityType

http://cgit.collabora.com/git/user/cassidy/telepathy-spec/log/?h=next-handle-type
http://cgit.collabora.com/git/user/cassidy/telepathy-glib/log/?h=next-handle-type

I'll start porting the world now.

(In reply to comment #29)
> (In reply to comment #28)
> > <cassidy> ok so: a) Renaming TargetHandleType to EntityType
> 
> http://cgit.collabora.com/git/user/cassidy/telepathy-spec/log/?h=next-handle-
> type
> http://cgit.collabora.com/git/user/cassidy/telepathy-glib/log/?h=next-handle-
> type
> 
> I'll start porting the world now.

http://cgit.collabora.com/git/user/cassidy/telepathy-gabble/log/?h=next-handle-type
http://cgit.collabora.com/git/user/cassidy/telepathy-salut/log/?h=next-handle-type
http://cgit.collabora.com/git/user/cassidy/telepathy-idle/log/?h=next-handle-type
http://cgit.collabora.com/git/user/cassidy/telepathy-rakia/log/?h=next-handle-type
http://cgit.collabora.com/git/user/cassidy/telepathy-haze/log/?h=next-handle-type

http://cgit.collabora.com/git/user/cassidy/telepathy-mission-control/log/?h=next-handle-type

http://cgit.collabora.com/git/user/cassidy/folks/log/?h=next-handle-type
http://cgit.collabora.com/git/user/cassidy/empathy/log/?h=next-handle-type

I don't want to merge this without adding Entity_Type_Self and making TplEntityType numerically equal to TpEntityType, even if TplEntityType still exists as a distinct C type for a while. It'd just be too confusing otherwise.

Happily, adding Entity_Type_Self and reshuffling the members of TpEntityType and TplEntityType should be a source-compatible (API-compatible) change, even though it'll break binary compatibility (ABI); so hopefully you can just append that to the tp-glib and tp-spec branches, and append a constants.py change to the Twisted tests, without invalidating any of the other porting you did.

-spec, -glib look OK so far.

I would like to replace handle_type, handle-type, HandleType, HANDLE_TYPE in C APIs with the entity equivalents, too; but that can be a separate sed + merge.

All seems reasonable.

Done, those branches have been updated:

http://cgit.collabora.com/git/user/cassidy/telepathy-spec/log/?h=next-handle-type
http://cgit.collabora.com/git/user/cassidy/telepathy-glib/log/?h=next-handle-type
http://cgit.collabora.com/git/user/cassidy/empathy/log/?h=next-handle-type

  * Returns: a TplEntity instance with identifier, alias and
  * avatar's token copied. Type parameter is useful to differentiate between
- * normal contact and self contact, thus only %TPL_ENTITY_CONTACT and
- * %TPL_ENTITY_SELF are accepted. If contact is %NULL, an entity of type
- * %TPL_ENTITY_UNKNOWN with id set to "unknown" is returned.
+ * normal contact and self contact, thus only %TP_ENTITY_TYPE_CONTACT and
+ * %TP_ENTITY_TYPE_SELF are accepted. If contact is %NULL, an entity of type
+ * %TP_ENTITY_TYPE_NONE with id set to "unknown" is returned.
  */
 TplEntity *
 tpl_entity_new_from_tp_contact (TpContact *contact,
- TplEntityType type)
+ TpEntityType type)

Should tpl_entity_new_from_tp_contact (NULL, TP_ENTITY_TYPE_NONE) also be accepted? At the moment it'll trip a g_return_val_if_fail. Please either allow that combination (but not (non-NULL, NONE)), or add something like this to the doc-comment:

  If @contact is %NULL, an entity of type %TP_ENTITY_TYPE_NONE is returned,
  with @id set to "unknown". (Note that it is not valid to call this function
  with @type set to %TP_ENTITY_TYPE_NONE.)

Other than that, all looks good.

(In reply to comment #35)
> Should tpl_entity_new_from_tp_contact (NULL, TP_ENTITY_TYPE_NONE) also be
> accepted? At the moment it'll trip a g_return_val_if_fail. Please either
> allow that combination (but not (non-NULL, NONE)), or add something like
> this to the doc-comment:

Done: http://cgit.collabora.com/git/user/cassidy/telepathy-glib/log/?h=next-handle-type

(In reply to comment #36)
> (In reply to comment #35)
> > Should tpl_entity_new_from_tp_contact (NULL, TP_ENTITY_TYPE_NONE) also be
> > accepted?
>
> Done:
> http://cgit.collabora.com/git/user/cassidy/telepathy-glib/log/?h=next-handle-type

Looks good

All merged!

(In reply to comment #3)
> Other changes that are easy enough that we can just do them immediately:
> 
> * the former Bug #24624: relax .client naming as outlined on that bug
> (requested by Sjoerd)
> 
> * merge Requests and Contacts into the core Connection interface
> 
> * merge Messages and Text into the Text channel type, possibly renaming it
> to Chan.T.Messages in the process
> 
> * rename the "WithHints" variants of the CD methods to remove the suffix,
> deleting the old simple versions
> 
> * rename CR.SucceededWithChannel to Succeeded and delete the old simple
> version
> 
> * delete all deprecated things, including StreamedMedia, complex Presence
> and simple Capabilities
> 
> * promote Account.Parameters to its own interface (rationale: users of
> accounts that aren't account-editing UIs don't care; requested by Sjoerd,
> IIRC)
> 
> * use o.fd.DBus.Properties.PropertiesChanged for at least Account
> 
> * rename Handle_Type to Target_Type and TargetHandleType to TargetType
> 
> * consider reorganizing Observer, Approver and Handler methods to promote
> things from the a{sv} of misc to the top level

I think we now did all those, except moving Account.Parameters which we agreed to not do.

This is a meta-bug and all depends-on are closed. Should we consider tp-spec1.0 to be frozen now?

(In reply to comment #40)
> This is a meta-bug and all depends-on are closed. Should we consider
> tp-spec1.0 to be frozen now?

That depends whether we think we can still get tp-glib-1.0 stable for GNOME 3.14.

If we can't, then production-quality versions of the major CMs won't implement spec 1.0, so it won't be usable anyway; in which case we might as well leave the spec unfrozen, and try to sneak in Bug #25033 and Bug #14540.

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/telepathy/telepathy-spec/issues/32.