Bug 37413

Summary: Clarify message edits (edit-timestamp and supersedes)
Product: Telepathy Reporter: David Laban <david.laban>
Component: tp-specAssignee: Telepathy bugs list <telepathy-bugs>
Status: RESOLVED FIXED QA Contact: Telepathy bugs list <telepathy-bugs>
Severity: normal    
Priority: medium CC: danielle, david.laban, nicolas
Version: git masterKeywords: patch
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:

Description David Laban 2011-05-20 09:58:16 UTC
We need an additional "edit-timestamp" message header to make it clear that the timestamp on a message with supersedes set is that of the original message (this is required for efficiently locating the original message in e.g. the logger). Messing about with the semantics of received-timestamp is also not acceptable.

We should also clarify the chain of superseding messages, and what is considered an error or not.

The following case should be valid:
message a;
message b with supersedes = a;
message c with supersedes = a;

The following should also be valid, and handled gracefully by clients in case the CM can't retrieve it (it's not in the interests of the sender to re-send the message x telling you how he fucked up if you lost it: he mostly wants you to know what he *meant to say*):
message x gets lost
message y with supersedes = x;
message z with supersedes = x;

I would quite like to ban the following case (we handle every case in the logger, but most people will only think to handle one case or another, and it would make the code a lot simpler if we just specified that one is valid and the other isn't).
message a;
message b with supersedes = a;
message c with supersedes = b;
Comment 1 Nicolas Dufresne 2011-05-21 07:52:00 UTC
On little note, the header shall be "message-edited" to follow other timestamps naming style (message-sent, message-received).
Comment 2 Simon McVittie 2011-05-23 02:19:47 UTC
(In reply to comment #0)
> We need an additional "edit-timestamp" message header to make it clear that the
> timestamp on a message with supersedes set is that of the original message
> (this is required for efficiently locating the original message in e.g. the
> logger). Messing about with the semantics of received-timestamp is also not
> acceptable.

Is edit-timestamp the timestamp when the edit was sent, or when it was received?

I'd have personally assumed that the edit would have its own (later) sent and received timestamps (this would fit the conceptual model of a new message superseding an old message, IMO), but if that causes practical problems, we could have extra timestamps, as long as the spec is clear.

However, at the moment, received-timestamp is mandatory. If you retroactively change its semantics to "the time at which we received the original message of a supersedes chain", then that will have to be relaxed, because at the point when you receive the superseding message, the CM is likely to have no idea when you received the superseded message (it might even have been in a previous session!).

The more I think about this, the more I think sent/received should refer to the individual messages...

Of course, if the *logger* wants to present a timestamp with "original message if possible" semantics, that's fine.

> The following case should be valid:
> message a;
> message b with supersedes = a;
> message c with supersedes = a;

Agreed. I think this is what XMPP does, in practice.

> The following should also be valid [...]
> message x gets lost
> message y with supersedes = x;
> message z with supersedes = x;

Yes.

> I would quite like to ban the following case [...]
> message a;
> message b with supersedes = a;
> message c with supersedes = b;

Only if we're absolutely sure that no protocol will ever implement superseding/edits with this sort of structure. At the time that the CM receives c, it's not at all guaranteed that it will remember a - so if the underlying protocol represents edits like a linked list in this way, the CM has no choice but to do the same.

If the *logger* wants to canonicalize the superseding structure into the first form you mentioned, that might not be a bad idea, though - unlike the CM, it's likely to have enough historical context to do that properly.
Comment 3 David Laban 2011-05-23 16:42:12 UTC
(In reply to comment #2)
> (In reply to comment #0)
> > We need an additional "edit-timestamp" message header to make it clear that the
> > timestamp on a message with supersedes set is that of the original message
> > (this is required for efficiently locating the original message in e.g. the
> > logger). Messing about with the semantics of received-timestamp is also not
> > acceptable.
> 
> Is edit-timestamp the timestamp when the edit was sent, or when it was
> received?
> 
Sent

> I'd have personally assumed that the edit would have its own (later) sent and
> received timestamps (this would fit the conceptual model of a new message
> superseding an old message, IMO), but if that causes practical problems, we
> could have extra timestamps, as long as the spec is clear.


When I think "message B supersedes message A", I expect message B to be the same as message A, but with the bits that the remote member doesn't like changed. I also don't think of an edit as a *separate* message as such (indeed, the logger squashes superseded events together (returning them at the place in the list where the original message was logged, with a method on the returned object to get the chain of superseded events).

> 
> However, at the moment, received-timestamp is mandatory. If you retroactively
> change its semantics to "the time at which we received the original message of
> a supersedes chain", then that will have to be relaxed, because at the point
> when you receive the superseding message, the CM is likely to have no idea when
> you received the superseded message (it might even have been in a previous
> session!).
> 
> The more I think about this, the more I think sent/received should refer to the
> individual messages...

I probably wouldn't object to the edit having a "message-received" timestamp that corresponds to when the *edit* was received. 

When I think of "message-sent", the main use that I can think for it is sorting to view the conversation in the order that the remote user sees it (in case the protocol doesn't guarantee message ordering).

This is one of the reasons why I suggested setting "message-sent" to that of the original message (the other is that I find it more useful to visualise a message and its edits as one message). "message-sent" is also optional for incoming messages, so could be omitted if the CM doesn't know the date of the original message.

The alternative to my original solution would be to have "original-message-sent" and/or "original-message-received" optional headers. Does this sound like a better idea? Also, are there any better names that we could use?

> > The following case should be valid:
> > message a;
> > message b with supersedes = a;
> > message c with supersedes = a;
> 
> Agreed. I think this is what XMPP does, in practice.
> 
> > The following should also be valid [...]
> > message x gets lost
> > message y with supersedes = x;
> > message z with supersedes = x;
> 
> Yes.
> 
> > I would quite like to ban the following case [...]
> > message a;
> > message b with supersedes = a;
> > message c with supersedes = b;
> 
> Only if we're absolutely sure that no protocol will ever implement
> superseding/edits with this sort of structure. At the time that the CM receives
> c, it's not at all guaranteed that it will remember a - so if the underlying
> protocol represents edits like a linked list in this way, the CM has no choice
> but to do the same.
> 
It is only the telepathy spec's choice of the word "supersedes" that suggests that the banned form might be valid. If you think of the use-case: "I want to edit message a... Oh no: still wrong. Let's try again." then the XMPP form is very obviously the most natural one.

I'd give odds of at least 10:1 that it will *always* be possible for the CM to represent message edits in the XMPP form. Marking the alternative as a banned form for now makes writing/reviewing code a lot easier. If we have to retrospectively allow the banned form then we probably have to go back and test/audit all of our code anyway (implementing code which is resilient to both is more complicated, and there are no CM implementations for people to test against) so I don't think we actually gain anything by allowing it.
Comment 5 Danielle Madeley 2011-05-24 23:11:27 UTC
(In reply to comment #4)
> I have written some spec at:
> 
> http://cgit.collabora.com/git/user/alsuren/telepathy-spec.git/log/?h=message-edits-37413

+                originally said." It is often in the interests of the
+                remote side for message x to be lost.</tp:rationale></dd>

"It is often in the interests of the remote side for message x to be lost." ?

+              <code>
+                message {token = a};
+                message {token = b, supersedes = a};
+                message {token = c, supersedes = a};
+              </code>

Would <pre> be better so that it obeys your wrapping?

I'm not sure what this means.

+              present. It MAY be used as a hint to help the UI to locate the
+              original message in its logs. If present, comparing the tuple

"hint to help clients" ?

For original-message-{sent,received} perhaps the second time say "it MAY be used as a hint for clients in the same was as ..." rather than repeating the text.
Comment 6 Danielle Madeley 2011-05-24 23:18:48 UTC
(In reply to comment #4)

> I have also updated the logger to reflect the clarified timestamp semantics.
> http://cgit.freedesktop.org/~alsuren/telepathy-logger/log/?h=original-timestamp

++
Comment 7 David Laban 2011-05-25 10:52:50 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > I have written some spec at:
> > 
> > http://cgit.collabora.com/git/user/alsuren/telepathy-spec.git/log/?h=message-edits-37413
> 
> +                originally said." It is often in the interests of the
> +                remote side for message x to be lost.</tp:rationale></dd>
> 
see http://cgit.collabora.com/git/user/alsuren/telepathy-spec.git/commit/?h=message-edits-37413&id=35c4b23e407e4d00c24d331998260cf61cd7af4a for more details

> "It is often in the interests of the remote side for message x to be lost." ?
> 
> +              <code>
> +                message {token = a};
> +                message {token = b, supersedes = a};
> +                message {token = c, supersedes = a};
> +              </code>
> 
> Would <pre> be better so that it obeys your wrapping?
> 
fixed in http://cgit.collabora.com/git/user/alsuren/telepathy-spec.git/commit/?h=message-edits-37413&id=592bbf46d6e97e07f0f19aeafb16f19887378f49

> I'm not sure what this means.
> 
> +              present. It MAY be used as a hint to help the UI to locate the
> +              original message in its logs. If present, comparing the tuple
> 
> "hint to help clients" ?
> 
> For original-message-{sent,received} perhaps the second time say "it MAY be
> used as a hint for clients in the same was as ..." rather than repeating the
> text.
fixed in http://cgit.collabora.com/git/user/alsuren/telepathy-spec.git/commit/?h=message-edits-37413&id=256fbd26957cc5a441479a65dc81d7c639dbfda9

final result is at http://people.freedesktop.org/~alsuren/telepathy-spec-message_edits_37413/spec/Channel_Interface_Messages.html#Simple-Type:Message_Header_Key

and what I plan to push is at http://cgit.collabora.com/git/user/alsuren/telepathy-spec.git/log/?h=message-edits-37413-rebased
Comment 8 David Laban 2011-05-25 11:43:26 UTC
smcv had some suggestions on IRC that I fixed.

Rebased and pushed to master.
Comment 9 Danielle Madeley 2011-05-25 16:31:30 UTC
(In reply to comment #7)

> see
> http://cgit.collabora.com/git/user/alsuren/telepathy-spec.git/commit/?h=message-edits-37413&id=35c4b23e407e4d00c24d331998260cf61cd7af4a
> for more details

I would instead say: this handles the case where the message is deliberately lost or irretrievable by the receiver (e.g. to hide an embarrassing mistake or confidential information).
Comment 10 Danielle Madeley 2011-05-25 19:48:48 UTC
Just noticed that the logger seems to now be providing messages sorted by timestamp, instead of original-timestamp (where appropriate).

So messages which are the digits 1-9, where I edited message "5", in Empathy I get "6", "7", "8", "9", "5E" as the last 5 messages.
Comment 11 Danielle Madeley 2011-05-25 19:55:18 UTC
(In reply to comment #10)
> Just noticed that the logger seems to now be providing messages sorted by
> timestamp, instead of original-timestamp (where appropriate).
> 
> So messages which are the digits 1-9, where I edited message "5", in Empathy I
> get "6", "7", "8", "9", "5E" as the last 5 messages.

Filed as #37611

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.