fix(@desktop/chat): chat - messages in 1-1 chat arrive out of order
An explanation why we keep track of `timestamp` and `localTimestamp` here,
introduction of those two actually fixes the following issues:
- https://github.com/status-im/status-desktop/issues/6004
- https://github.com/status-im/status-desktop/issues/7058
We should always refer to `whisperTimestamp` as it is set for a message by the network
in order they are sent (that solves the issue #6004), but another issue #7058 is happening
cause `whisperTimestamp` has one second accuracy (which is a very big timeframe for messages).
That further means that all messsages sent by user A within 1000ms will be received with the
same `whisperTimestamp` value on the side of user B, in that case to differ the order of
those message we're using localy set `timestamp` on the sender side which is received unchanged
on the receiver side.
Now a question why don't we use only locally set `timestamp` may araise... the answer is...
because of issue #6004, cause it can be that users A and B send a message in almost the same
time in that case message sent by user A will be immediatelly added to the message list, while
message sent by user B will arrive like a less then a second later and in that case user A may
see user B message before or after his message and the same for user B, depends on local time
of those 2 users which is set for `timestamp` time in the moment they sent a message.
If we anyhow find a way to have here accutacy higher than 1 second, then we can go on only
with `whisperTimestamp`
3f987cc565/protocol/messenger.go (L3726)
Fixes: #7058
This commit is contained in:
parent
07a0cdc680
commit
89f4fa7440
|
@ -85,7 +85,8 @@ proc createMessageItemFromDto(self: Module, message: MessageDto, chatDetails: Ch
|
||||||
message.image,
|
message.image,
|
||||||
message.containsContactMentions(),
|
message.containsContactMentions(),
|
||||||
message.seen,
|
message.seen,
|
||||||
message.timestamp,
|
timestamp = message.whisperTimestamp,
|
||||||
|
localTimestamp = message.timestamp,
|
||||||
ContentType(message.contentType),
|
ContentType(message.contentType),
|
||||||
message.messageType,
|
message.messageType,
|
||||||
message.sticker.url,
|
message.sticker.url,
|
||||||
|
|
|
@ -87,6 +87,7 @@ proc createFetchMoreMessagesItem(self: Module): Item =
|
||||||
messageContainsMentions = false,
|
messageContainsMentions = false,
|
||||||
seen = true,
|
seen = true,
|
||||||
timestamp = 0,
|
timestamp = 0,
|
||||||
|
localTimestamp = 0,
|
||||||
ContentType.FetchMoreMessagesButton,
|
ContentType.FetchMoreMessagesButton,
|
||||||
messageType = -1,
|
messageType = -1,
|
||||||
sticker = "",
|
sticker = "",
|
||||||
|
@ -125,6 +126,7 @@ proc createChatIdentifierItem(self: Module): Item =
|
||||||
messageContainsMentions = false,
|
messageContainsMentions = false,
|
||||||
seen = true,
|
seen = true,
|
||||||
timestamp = 0,
|
timestamp = 0,
|
||||||
|
localTimestamp = 0,
|
||||||
ContentType.ChatIdentifier,
|
ContentType.ChatIdentifier,
|
||||||
messageType = -1,
|
messageType = -1,
|
||||||
sticker = "",
|
sticker = "",
|
||||||
|
@ -189,7 +191,8 @@ method newMessagesLoaded*(self: Module, messages: seq[MessageDto], reactions: se
|
||||||
m.image,
|
m.image,
|
||||||
m.containsContactMentions(),
|
m.containsContactMentions(),
|
||||||
m.seen,
|
m.seen,
|
||||||
m.whisperTimestamp,
|
timestamp = m.whisperTimestamp,
|
||||||
|
localTimestamp = m.timestamp,
|
||||||
m.contentType.ContentType,
|
m.contentType.ContentType,
|
||||||
m.messageType,
|
m.messageType,
|
||||||
sticker = m.sticker.url,
|
sticker = m.sticker.url,
|
||||||
|
@ -278,7 +281,8 @@ method messageAdded*(self: Module, message: MessageDto) =
|
||||||
message.image,
|
message.image,
|
||||||
message.containsContactMentions(),
|
message.containsContactMentions(),
|
||||||
message.seen,
|
message.seen,
|
||||||
message.whisperTimestamp,
|
timestamp = message.whisperTimestamp,
|
||||||
|
localTimestamp = message.timestamp,
|
||||||
message.contentType.ContentType,
|
message.contentType.ContentType,
|
||||||
message.messageType,
|
message.messageType,
|
||||||
sticker = message.sticker.url,
|
sticker = message.sticker.url,
|
||||||
|
|
|
@ -177,7 +177,8 @@ proc buildPinnedMessageItem(self: Module, messageId: string, actionInitiatedBy:
|
||||||
m.image,
|
m.image,
|
||||||
m.containsContactMentions(),
|
m.containsContactMentions(),
|
||||||
m.seen,
|
m.seen,
|
||||||
m.timestamp,
|
timestamp = m.whisperTimestamp,
|
||||||
|
localTimestamp = m.timestamp,
|
||||||
m.contentType.ContentType,
|
m.contentType.ContentType,
|
||||||
m.messageType,
|
m.messageType,
|
||||||
m.sticker.url,
|
m.sticker.url,
|
||||||
|
|
|
@ -5,6 +5,31 @@ import ../../../app_service/service/contacts/dto/contacts
|
||||||
export types.ContentType
|
export types.ContentType
|
||||||
import message_reaction_model, message_reaction_item, message_transaction_parameters_item
|
import message_reaction_model, message_reaction_item, message_transaction_parameters_item
|
||||||
|
|
||||||
|
## An explanation why we keep track of `timestamp` and `localTimestamp` here
|
||||||
|
##
|
||||||
|
## introduction of those two actually fixes the following issues:
|
||||||
|
## https://github.com/status-im/status-desktop/issues/6004
|
||||||
|
## https://github.com/status-im/status-desktop/issues/7058
|
||||||
|
##
|
||||||
|
## We should always refer to `whisperTimestamp` as it is set for a message by the network
|
||||||
|
## in order they are sent (that solves the issue #6004), but another issue #7058 is happening
|
||||||
|
## cause `whisperTimestamp` has one second accuracy (which is a very big timeframe for messages).
|
||||||
|
## That further means that all messsages sent by user A within 1000ms will be received with the
|
||||||
|
## same `whisperTimestamp` value on the side of user B, in that case to differ the order of
|
||||||
|
## those message we're using localy set `timestamp` on the sender side which is received unchanged
|
||||||
|
## on the receiver side.
|
||||||
|
## Now a question why don't we use only locally set `timestamp` may araise... the answer is...
|
||||||
|
## because of issue #6004, cause it can be that users A and B send a message in almost the same
|
||||||
|
## time in that case message sent by user A will be immediatelly added to the message list, while
|
||||||
|
## message sent by user B will arrive like a less then a second later and in that case user A may
|
||||||
|
## see user B message before or after his message and the same for user B, depends on local time
|
||||||
|
## of those 2 users which is set for `timestamp` time in the moment they sent a message.
|
||||||
|
##
|
||||||
|
## If we anyhow find a way to have here accutacy higher than 1 second, then we can go on only
|
||||||
|
## with `whisperTimestamp`
|
||||||
|
## https://github.com/status-im/status-go/blob/3f987cc565091327f017bfe674c08ed01e301d00/protocol/messenger.go#L3726
|
||||||
|
|
||||||
|
|
||||||
type
|
type
|
||||||
Item* = ref object
|
Item* = ref object
|
||||||
id: string
|
id: string
|
||||||
|
@ -26,6 +51,7 @@ type
|
||||||
gapFrom: int64
|
gapFrom: int64
|
||||||
gapTo: int64
|
gapTo: int64
|
||||||
timestamp: int64
|
timestamp: int64
|
||||||
|
localTimestamp: int64
|
||||||
contentType: ContentType
|
contentType: ContentType
|
||||||
messageType: int
|
messageType: int
|
||||||
reactionsModel: MessageReactionModel
|
reactionsModel: MessageReactionModel
|
||||||
|
@ -54,7 +80,8 @@ proc initItem*(
|
||||||
image: string,
|
image: string,
|
||||||
messageContainsMentions,
|
messageContainsMentions,
|
||||||
seen: bool,
|
seen: bool,
|
||||||
timestamp: int64,
|
timestamp: int64, # whisper timestamp, with 1s accuracy (even accuracy looks like 1ms, last 3 digits are always 0)
|
||||||
|
localTimestamp: int64, # local timestamp obtained when a message is being sent, with 1ms accuracy
|
||||||
contentType: ContentType,
|
contentType: ContentType,
|
||||||
messageType: int,
|
messageType: int,
|
||||||
sticker: string,
|
sticker: string,
|
||||||
|
@ -81,6 +108,7 @@ proc initItem*(
|
||||||
result.messageImage = image
|
result.messageImage = image
|
||||||
result.messageContainsMentions = messageContainsMentions
|
result.messageContainsMentions = messageContainsMentions
|
||||||
result.timestamp = timestamp
|
result.timestamp = timestamp
|
||||||
|
result.localTimestamp = localTimestamp
|
||||||
result.contentType = contentType
|
result.contentType = contentType
|
||||||
result.messageType = messageType
|
result.messageType = messageType
|
||||||
result.pinned = false
|
result.pinned = false
|
||||||
|
@ -112,6 +140,7 @@ proc `$`*(self: Item): string =
|
||||||
messageText:{self.messageText},
|
messageText:{self.messageText},
|
||||||
messageContainsMentions:{self.messageContainsMentions},
|
messageContainsMentions:{self.messageContainsMentions},
|
||||||
timestamp:{$self.timestamp},
|
timestamp:{$self.timestamp},
|
||||||
|
localTimestamp:{$self.localTimestamp}
|
||||||
contentType:{$self.contentType.int},
|
contentType:{$self.contentType.int},
|
||||||
messageType:{$self.messageType},
|
messageType:{$self.messageType},
|
||||||
pinned:{$self.pinned},
|
pinned:{$self.pinned},
|
||||||
|
@ -207,6 +236,9 @@ proc seen*(self: Item): bool {.inline.} =
|
||||||
proc timestamp*(self: Item): int64 {.inline.} =
|
proc timestamp*(self: Item): int64 {.inline.} =
|
||||||
self.timestamp
|
self.timestamp
|
||||||
|
|
||||||
|
proc localTimestamp*(self: Item): int64 {.inline.} =
|
||||||
|
self.localTimestamp
|
||||||
|
|
||||||
proc contentType*(self: Item): ContentType {.inline.} =
|
proc contentType*(self: Item): ContentType {.inline.} =
|
||||||
self.contentType
|
self.contentType
|
||||||
|
|
||||||
|
@ -277,6 +309,7 @@ proc toJsonNode*(self: Item): JsonNode =
|
||||||
"gapFrom": self.gapFrom,
|
"gapFrom": self.gapFrom,
|
||||||
"gapTo": self.gapTo,
|
"gapTo": self.gapTo,
|
||||||
"timestamp": self.timestamp,
|
"timestamp": self.timestamp,
|
||||||
|
"localTimestamp": self.localTimestamp,
|
||||||
"contentType": self.contentType.int,
|
"contentType": self.contentType.int,
|
||||||
"messageType": self.messageType,
|
"messageType": self.messageType,
|
||||||
"pinned": self.pinned,
|
"pinned": self.pinned,
|
||||||
|
|
|
@ -228,10 +228,13 @@ QtObject:
|
||||||
if(self.items[i].responseToMessageWithId == messageId):
|
if(self.items[i].responseToMessageWithId == messageId):
|
||||||
result.add(self.items[i].id)
|
result.add(self.items[i].id)
|
||||||
|
|
||||||
proc findIndexBasedOnTimestampToInsertTo(self: Model, timestamp: int64): int =
|
proc findIndexBasedOnTimestampToInsertTo(self: Model, timestamp: int64, localTimestamp: int64): int =
|
||||||
for i in 0 ..< self.items.len:
|
for i in 0 ..< self.items.len:
|
||||||
if(timestamp > self.items[i].timestamp):
|
if timestamp > self.items[i].timestamp:
|
||||||
return i
|
return i
|
||||||
|
elif timestamp == self.items[i].timestamp:
|
||||||
|
if localTimestamp > self.items[i].localTimestamp:
|
||||||
|
return i
|
||||||
return 0
|
return 0
|
||||||
|
|
||||||
proc filterExistingItems(self: Model, items: seq[Item]): seq[Item] =
|
proc filterExistingItems(self: Model, items: seq[Item]): seq[Item] =
|
||||||
|
@ -311,7 +314,7 @@ QtObject:
|
||||||
let parentModelIndex = newQModelIndex()
|
let parentModelIndex = newQModelIndex()
|
||||||
defer: parentModelIndex.delete
|
defer: parentModelIndex.delete
|
||||||
|
|
||||||
let position = self.findIndexBasedOnTimestampToInsertTo(item.timestamp)
|
let position = self.findIndexBasedOnTimestampToInsertTo(item.timestamp, item.localTimestamp)
|
||||||
|
|
||||||
self.beginInsertRows(parentModelIndex, position, position)
|
self.beginInsertRows(parentModelIndex, position, position)
|
||||||
self.items.insert(item, position)
|
self.items.insert(item, position)
|
||||||
|
|
Loading…
Reference in New Issue