Bug 28742 - Import Connection.Interface.StoredMessages from rtcom-tp-glib
Summary: Import Connection.Interface.StoredMessages from rtcom-tp-glib
Status: RESOLVED MOVED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: tp-spec (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Will Thompson
QA Contact: Telepathy bugs list
URL: http://git.collabora.co.uk/?p=user/wj...
Whiteboard:
Keywords: patch
: 24904 (view as bug list)
Depends on: 26838
Blocks:
  Show dependency treegraph
 
Reported: 2010-06-24 08:08 UTC by Will Thompson
Modified: 2019-12-03 20:21 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Will Thompson 2010-06-24 08:08:51 UTC
http://people.freedesktop.org/~wjt/telepathy-spec-import_stored_messages/spec/Connection_Interface_Stored_Messages.html

I didn't make any functional changes relative to the version implemented by telepathy-ring in Maemo 5. I did tweak some names, reword some sections, etc. though.

Also I decided there's really no point doing anything about the assumption that exactly one client is going to be poking this interface beyond documenting it. So just did that.

(I actually think the problem of multiple loggers is going to be a real one relatively soon: if you have both Gnome and KDE installed you will presumably ultimately have two service-activatable observers for text channels and then you get everything logged twice for no good reason.)
Comment 1 Will Thompson 2010-06-24 08:09:52 UTC
CCing Lassi. Is there anything functionally wrong with the Maemo 5 version of this interface that we should change before standardizing it, in your opinion?
Comment 2 Simon McVittie 2010-06-28 03:13:10 UTC
The first two patches "Add a type synonym for message part IDs", "Allow arrays of Protocol_Message_Token" are obviously OK; please merge.

My worry about the new interface is that it's defined in terms of 'message-token', but as I understand it, the unique identifiers in SMS are actually what we'd now call a 'protocol-token' (i.e. they're temporarily unique per contact, but not globally unique).

> Persistently-stored messages MUST also contain a stored-message-received
> header. The first time the message is announced in MessageReceived, this
> SHOULD be identical to the message-token; if the message might have been
> presented earlier, the stored-message-received  header MAY differ from the
> message-token.

This could do with rationale. What's it for? If the intention is that it's the original value of message-token, say so; if the intention is that in re-deliveries it's the new protocol-level ID, again, say so.

See Bug #26838 for a third sort of message identifier; if this is appropriate for what you need, please use that one.

> If the message has been presented earlier in this Connection's lifetime,
> a header rescued with boolean value True  SHOULD be present.

Is this compatible with the usual meaning of 'rescued', which is essentially "a logger that has been paying attention should already have seen this"?
Comment 3 Will Thompson 2010-06-30 07:38:36 UTC
(In reply to comment #2)
> The first two patches "Add a type synonym for message part IDs", "Allow arrays
> of Protocol_Message_Token" are obviously OK; please merge.

I have done so.
Comment 4 Artem Garmash 2010-07-06 03:41:47 UTC
The biggest problem with the original interface support by commhistoryd in Harmattan was the fact that stored messages includes currently pending messages. If such a message is expunged, it's delivered twice. In case of tp-ring, only "rescued" can be used to detect a message delivered via expunging ("message-received" and "stored-message-received" are always identical and they are timestamps, not tokens currently).
Do we really need two message tokens for re-delivered messages? At least for inbound messages, it's not used for anything else but expunge checks and we could use just original token.
Comment 5 Will Thompson 2010-07-22 08:29:35 UTC
(In reply to comment #4)
> The biggest problem with the original interface support by commhistoryd in
> Harmattan was the fact that stored messages includes currently pending
> messages. If such a message is expunged, it's delivered twice. In case of
> tp-ring, only "rescued" can be used to detect a message delivered via expunging
> ("message-received" and "stored-message-received" are always identical and they
> are timestamps, not tokens currently).

I think I've addressed this, by making DeliverStoredMessages() a no-op if messages are already in PendingMessages on some channel. Sound okay?

> Do we really need two message tokens for re-delivered messages? At least for
> inbound messages, it's not used for anything else but expunge checks and we
> could use just original token.

Yeah, I just binned stored-message-token entirely.

I've also reworded the introduction again, added a 'stored' header that indicates that the message also appears on this interface, and some other stuff. I think it's ready to go.
Comment 6 Lassi Syrjala 2010-07-22 08:30:24 UTC
Hello,

I am on vacation until August 8th. In urgent project matters, please contact Naba Kumar.

Regards,
Lassi
Comment 7 Will Thompson 2010-08-13 09:06:15 UTC
StoredMessages is a slightly misleading name for this interface (not that I can think of a better one off-hand). Maybe we could call it MessageSpool? That is, after all, what it is.

And, if we want to support cancelling outgoing messages after we call SendMessage() but before they're actually sent (maybe we're in a tunnel, so no SMSes or XMPP <message>s can be sent), should this interface be the place to do that, too?
Comment 8 Will Thompson 2011-01-31 04:25:47 UTC
(In reply to comment #7)
> StoredMessages is a slightly misleading name for this interface (not that I can
> think of a better one off-hand). Maybe we could call it MessageSpool? That is,
> after all, what it is.

This is purely cosmetic, of course.

> And, if we want to support cancelling outgoing messages after we call
> SendMessage() but before they're actually sent (maybe we're in a tunnel, so no
> SMSes or XMPP <message>s can be sent), should this interface be the place to do
> that, too?

I think that, if we need this, we could add it later.

I've rebased the branch, and updated the HTML copy.
Comment 9 Kai Vehmanen 2011-02-08 07:12:43 UTC
Just a quick note here as well that there are some issues in implementing this interface with the new ofono-based tpring (without adding another layer of disk spooling to the path). See the long recent thread on ofono list:
http://lists.ofono.org/pipermail/ofono/2011-February/008428.html

As a result of the recent discussions, we are now implementing a prototype (involving an ofono plugin and matching tpring patch) to verify we can sanely implement the updated StoredMessage interface. I'll follow up to this bug when we have more information.

One thing we know already is that it will not be possible to implement SetStorageState(), so it would be good to have a possibility to return 'Not Available' (or similar) for the method. Reasoning why this won't be supported is here:
http://lists.ofono.org/pipermail/ofono/2011-February/008455.html

... in theory this might change in some future ofono version, but in any case with some modems this won't be supported, so tp-ring has to prepare for the case (e.g. that it cannot signal to cellular network to stop sending more SMS'es).
Comment 10 Simon McVittie 2013-10-23 13:15:26 UTC
*** Bug 24904 has been marked as a duplicate of this bug. ***
Comment 11 Simon McVittie 2013-10-23 13:15:52 UTC
See Bug #24904 for some other discussion on this.
Comment 12 GitLab Migration User 2019-12-03 20:21:55 UTC
-- 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/76.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.