Don't use bytes anymore for LastMessage

LastMessage in chat was encoded in bytes so that we don't have to
encoded/decode everytime we save to db or pass the client.

An issue with emoji surfaced a problem with this approach.
Chat.LastClockValue represent the last clock value of any type of
message exchanged in a chat (emoji,group membership updates, contact
updates).
So when receving a new message, we should update LastMessage if the
clock of the LastMessage is lower than the received message, and we
should not only check LastClockValue, otherwise the message might be
discarded although it is the most recent.

This commit fixes the issue by keeping LastMessage as an object and
comparing LastMessage.Clock instead of LastClockValue
This commit is contained in:
Andrea Maria Piana 2020-07-30 22:54:33 +02:00
parent 14e426f39f
commit ab01a05cd6
7 changed files with 80 additions and 79 deletions

View File

@ -1 +1 @@
0.56.4 0.56.5

View File

@ -3,7 +3,6 @@ package protocol
import ( import (
"crypto/ecdsa" "crypto/ecdsa"
"encoding/hex" "encoding/hex"
"encoding/json"
"errors" "errors"
"math/rand" "math/rand"
@ -51,8 +50,8 @@ type Chat struct {
DeletedAtClockValue uint64 `json:"deletedAtClockValue"` DeletedAtClockValue uint64 `json:"deletedAtClockValue"`
// Denormalized fields // Denormalized fields
UnviewedMessagesCount uint `json:"unviewedMessagesCount"` UnviewedMessagesCount uint `json:"unviewedMessagesCount"`
LastMessage []byte `json:"lastMessage"` LastMessage *Message `json:"lastMessage"`
// Group chat fields // Group chat fields
// Members are the members who have been invited to the group chat // Members are the members who have been invited to the group chat
@ -104,53 +103,6 @@ func (c *Chat) Validate() error {
return nil return nil
} }
func (c *Chat) MarshalJSON() ([]byte, error) {
type ChatAlias Chat
item := struct {
*ChatAlias
LastMessage json.RawMessage `json:"lastMessage"`
}{
ChatAlias: (*ChatAlias)(c),
LastMessage: c.LastMessage,
}
return json.Marshal(item)
}
func (c *Chat) UnmarshalJSON(data []byte) error {
type ChatAlias Chat
aux := struct {
*ChatAlias
LastMessage *Message `json:"lastMessage"`
}{
ChatAlias: (*ChatAlias)(c),
}
if err := json.Unmarshal(data, &aux); err != nil {
return err
}
c.ID = aux.ID
c.Name = aux.Name
c.Color = aux.Color
c.Active = aux.Active
c.ChatType = aux.ChatType
c.Timestamp = aux.Timestamp
c.LastClockValue = aux.LastClockValue
c.DeletedAtClockValue = aux.DeletedAtClockValue
c.UnviewedMessagesCount = aux.UnviewedMessagesCount
c.Members = aux.Members
c.MembershipUpdates = aux.MembershipUpdates
if aux.LastMessage != nil {
data, err := json.Marshal(aux.LastMessage)
if err != nil {
return err
}
c.LastMessage = data
}
return nil
}
func (c *Chat) MembersAsPublicKeys() ([]*ecdsa.PublicKey, error) { func (c *Chat) MembersAsPublicKeys() ([]*ecdsa.PublicKey, error) {
publicKeys := make([]string, len(c.Members)) publicKeys := make([]string, len(c.Members))
for idx, item := range c.Members { for idx, item := range c.Members {
@ -219,17 +171,13 @@ func (c *Chat) NextClockAndTimestamp(timesource TimeSource) (uint64, uint64) {
func (c *Chat) UpdateFromMessage(message *Message, timesource TimeSource) error { func (c *Chat) UpdateFromMessage(message *Message, timesource TimeSource) error {
c.Timestamp = int64(timesource.GetCurrentTime()) c.Timestamp = int64(timesource.GetCurrentTime())
higherClock := c.LastClockValue <= message.Clock
// If the clock is higher, or last message is nil, we set the message // If the clock of the last message is lower, we set the message
if higherClock || c.LastMessage == nil { if c.LastMessage == nil || c.LastMessage.Clock <= message.Clock {
jsonMessage, err := json.Marshal(message) c.LastMessage = message
if err != nil {
return err
}
c.LastMessage = jsonMessage
} }
// If the clock is higher we set the clock // If the clock is higher we set the clock
if higherClock { if c.LastClockValue < message.Clock {
c.LastClockValue = message.Clock c.LastClockValue = message.Clock
} }
return nil return nil

View File

@ -1,6 +1,7 @@
package protocol package protocol
import ( import (
"encoding/json"
"testing" "testing"
"github.com/stretchr/testify/suite" "github.com/stretchr/testify/suite"
@ -84,7 +85,7 @@ func (s *ChatTestSuite) TestUpdateFromMessage() {
// Clock is lower and lastMessage is not nil // Clock is lower and lastMessage is not nil
message = &Message{} message = &Message{}
lastMessage := []byte("test") lastMessage := message
chat = &Chat{LastClockValue: 2, LastMessage: lastMessage} chat = &Chat{LastClockValue: 2, LastMessage: lastMessage}
message.Clock = 1 message.Clock = 1
@ -100,4 +101,40 @@ func (s *ChatTestSuite) TestUpdateFromMessage() {
s.Require().NoError(chat.UpdateFromMessage(message, &testTimeSource{})) s.Require().NoError(chat.UpdateFromMessage(message, &testTimeSource{}))
s.Require().NotNil(chat.LastMessage) s.Require().NotNil(chat.LastMessage)
s.Require().Equal(uint64(2), chat.LastClockValue) s.Require().Equal(uint64(2), chat.LastClockValue)
// Clock is higher but lastMessage has lower clock message then the receiving one
message = &Message{}
chat = &Chat{LastClockValue: 2}
message.Clock = 1
s.Require().NoError(chat.UpdateFromMessage(message, &testTimeSource{}))
s.Require().NotNil(chat.LastMessage)
s.Require().Equal(uint64(2), chat.LastClockValue)
chat.LastClockValue = 4
message = &Message{}
message.Clock = 3
s.Require().NoError(chat.UpdateFromMessage(message, &testTimeSource{}))
s.Require().Equal(chat.LastMessage, message)
s.Require().Equal(uint64(4), chat.LastClockValue)
}
func (s *ChatTestSuite) TestSerializeJSON() {
message := &Message{}
chat := &Chat{}
message.Clock = 1
message.Text = "`some markdown text`"
s.Require().NoError(message.PrepareContent())
chat.LastMessage = message
encodedJSON, err := json.Marshal(chat)
s.Require().NoError(err)
decodedChat := &Chat{}
s.Require().NoError(json.Unmarshal(encodedJSON, decodedChat))
s.Require().Equal(chat, decodedChat)
} }

View File

@ -100,7 +100,7 @@ type Message struct {
// RTL is whether this is a right-to-left message (arabic/hebrew script etc) // RTL is whether this is a right-to-left message (arabic/hebrew script etc)
RTL bool `json:"rtl"` RTL bool `json:"rtl"`
// ParsedText is the parsed markdown for displaying // ParsedText is the parsed markdown for displaying
ParsedText []byte `json:"parsedText"` ParsedText []byte `json:"parsedText,omitempty"`
// LineCount is the count of newlines in the message // LineCount is the count of newlines in the message
LineCount int `json:"lineCount"` LineCount int `json:"lineCount"`
// Base64Image is the converted base64 image // Base64Image is the converted base64 image
@ -133,7 +133,7 @@ func (m *Message) MarshalJSON() ([]byte, error) {
OutgoingStatus string `json:"outgoingStatus,omitempty"` OutgoingStatus string `json:"outgoingStatus,omitempty"`
QuotedMessage *QuotedMessage `json:"quotedMessage"` QuotedMessage *QuotedMessage `json:"quotedMessage"`
RTL bool `json:"rtl"` RTL bool `json:"rtl"`
ParsedText json.RawMessage `json:"parsedText"` ParsedText json.RawMessage `json:"parsedText,omitempty"`
LineCount int `json:"lineCount"` LineCount int `json:"lineCount"`
Text string `json:"text"` Text string `json:"text"`
ChatID string `json:"chatId"` ChatID string `json:"chatId"`
@ -199,6 +199,7 @@ func (m *Message) UnmarshalJSON(data []byte) error {
ChatID string `json:"chatId"` ChatID string `json:"chatId"`
Sticker *protobuf.StickerMessage `json:"sticker"` Sticker *protobuf.StickerMessage `json:"sticker"`
AudioDurationMs uint64 `json:"audioDurationMs"` AudioDurationMs uint64 `json:"audioDurationMs"`
ParsedText json.RawMessage `json:"parsedText"`
ContentType protobuf.ChatMessage_ContentType `json:"contentType"` ContentType protobuf.ChatMessage_ContentType `json:"contentType"`
}{ }{
Alias: (*Alias)(m), Alias: (*Alias)(m),
@ -218,6 +219,7 @@ func (m *Message) UnmarshalJSON(data []byte) error {
m.EnsName = aux.EnsName m.EnsName = aux.EnsName
m.ChatId = aux.ChatID m.ChatId = aux.ChatID
m.ContentType = aux.ContentType m.ContentType = aux.ContentType
m.ParsedText = aux.ParsedText
return nil return nil
} }

View File

@ -761,7 +761,7 @@ func (db sqlitePersistence) BlockContact(contact *Contact) ([]*Chat, error) {
if err != nil { if err != nil {
return nil, err return nil, err
} }
c.LastMessage = encodedMessage c.LastMessage = message
} }

View File

@ -941,7 +941,7 @@ func (s *MessengerSuite) TestChatPersistencePublic() {
LastClockValue: 20, LastClockValue: 20,
DeletedAtClockValue: 30, DeletedAtClockValue: 30,
UnviewedMessagesCount: 40, UnviewedMessagesCount: 40,
LastMessage: []byte("test"), LastMessage: &Message{},
} }
s.Require().NoError(s.m.SaveChat(&chat)) s.Require().NoError(s.m.SaveChat(&chat))
@ -966,7 +966,7 @@ func (s *MessengerSuite) TestDeleteChat() {
LastClockValue: 20, LastClockValue: 20,
DeletedAtClockValue: 30, DeletedAtClockValue: 30,
UnviewedMessagesCount: 40, UnviewedMessagesCount: 40,
LastMessage: []byte("test"), LastMessage: &Message{},
} }
s.Require().NoError(s.m.SaveChat(&chat)) s.Require().NoError(s.m.SaveChat(&chat))
@ -989,7 +989,7 @@ func (s *MessengerSuite) TestChatPersistenceUpdate() {
LastClockValue: 20, LastClockValue: 20,
DeletedAtClockValue: 30, DeletedAtClockValue: 30,
UnviewedMessagesCount: 40, UnviewedMessagesCount: 40,
LastMessage: []byte("test"), LastMessage: &Message{},
} }
s.Require().NoError(s.m.SaveChat(&chat)) s.Require().NoError(s.m.SaveChat(&chat))
@ -1023,7 +1023,7 @@ func (s *MessengerSuite) TestChatPersistenceOneToOne() {
LastClockValue: 20, LastClockValue: 20,
DeletedAtClockValue: 30, DeletedAtClockValue: 30,
UnviewedMessagesCount: 40, UnviewedMessagesCount: 40,
LastMessage: []byte("test"), LastMessage: &Message{},
} }
contact := Contact{ contact := Contact{
ID: testPK, ID: testPK,
@ -1112,7 +1112,7 @@ func (s *MessengerSuite) TestChatPersistencePrivateGroupChat() {
LastClockValue: 20, LastClockValue: 20,
DeletedAtClockValue: 30, DeletedAtClockValue: 30,
UnviewedMessagesCount: 40, UnviewedMessagesCount: 40,
LastMessage: []byte("test"), LastMessage: &Message{},
} }
s.Require().NoError(s.m.SaveChat(&chat)) s.Require().NoError(s.m.SaveChat(&chat))
savedChats := s.m.Chats() savedChats := s.m.Chats()
@ -1279,17 +1279,12 @@ func (s *MessengerSuite) TestBlockContact() {
s.Require().Equal(uint(2), response[1].UnviewedMessagesCount) s.Require().Equal(uint(2), response[1].UnviewedMessagesCount)
// The new message content is updated // The new message content is updated
decodedMessage := &Message{}
s.Require().NotNil(response[0].LastMessage) s.Require().NotNil(response[0].LastMessage)
s.Require().NoError(json.Unmarshal(response[0].LastMessage, decodedMessage)) s.Require().Equal("test-7", response[0].LastMessage.ID)
s.Require().Equal("test-7", decodedMessage.ID)
decodedMessage = &Message{}
s.Require().NotNil(response[1].LastMessage) s.Require().NotNil(response[1].LastMessage)
s.Require().Equal("test-5", response[1].LastMessage.ID)
s.Require().NoError(json.Unmarshal(response[1].LastMessage, decodedMessage))
s.Require().Equal("test-5", decodedMessage.ID)
// The contact is updated // The contact is updated
savedContacts := s.m.Contacts() savedContacts := s.m.Contacts()
@ -2087,7 +2082,7 @@ func (s *MessengerSuite) TestMessageJSON() {
From: "from-field", From: "from-field",
} }
expectedJSON := `{"id":"test-1","whisperTimestamp":0,"from":"from-field","alias":"alias","identicon":"","seen":false,"quotedMessage":null,"rtl":false,"parsedText":null,"lineCount":0,"text":"test-1","chatId":"remote-chat-id","localChatId":"local-chat-id","clock":1,"replace":"","responseTo":"","ensName":"","sticker":null,"commandParameters":null,"timestamp":0,"contentType":0,"messageType":0}` expectedJSON := `{"id":"test-1","whisperTimestamp":0,"from":"from-field","alias":"alias","identicon":"","seen":false,"quotedMessage":null,"rtl":false,"lineCount":0,"text":"test-1","chatId":"remote-chat-id","localChatId":"local-chat-id","clock":1,"replace":"","responseTo":"","ensName":"","sticker":null,"commandParameters":null,"timestamp":0,"contentType":0,"messageType":0}`
messageJSON, err := json.Marshal(message) messageJSON, err := json.Marshal(message)
s.Require().NoError(err) s.Require().NoError(err)

View File

@ -5,6 +5,7 @@ import (
"context" "context"
"database/sql" "database/sql"
"encoding/gob" "encoding/gob"
"encoding/json"
"github.com/pkg/errors" "github.com/pkg/errors"
@ -111,6 +112,15 @@ func (db sqlitePersistence) saveChat(tx *sql.Tx, chat Chat) error {
return err return err
} }
// encode last message
var encodedLastMessage []byte
if chat.LastMessage != nil {
encodedLastMessage, err = json.Marshal(chat.LastMessage)
if err != nil {
return err
}
}
// Insert record // Insert record
stmt, err := tx.Prepare(`INSERT INTO chats(id, name, color, active, type, timestamp, deleted_at_clock_value, unviewed_message_count, last_clock_value, last_message, members, membership_updates, muted) stmt, err := tx.Prepare(`INSERT INTO chats(id, name, color, active, type, timestamp, deleted_at_clock_value, unviewed_message_count, last_clock_value, last_message, members, membership_updates, muted)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?,?)`) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?,?)`)
@ -129,7 +139,7 @@ func (db sqlitePersistence) saveChat(tx *sql.Tx, chat Chat) error {
chat.DeletedAtClockValue, chat.DeletedAtClockValue,
chat.UnviewedMessagesCount, chat.UnviewedMessagesCount,
chat.LastClockValue, chat.LastClockValue,
chat.LastMessage, encodedLastMessage,
encodedMembers.Bytes(), encodedMembers.Bytes(),
encodedMembershipUpdates.Bytes(), encodedMembershipUpdates.Bytes(),
chat.Muted, chat.Muted,
@ -208,6 +218,7 @@ func (db sqlitePersistence) chats(tx *sql.Tx) (chats []*Chat, err error) {
chat Chat chat Chat
encodedMembers []byte encodedMembers []byte
encodedMembershipUpdates []byte encodedMembershipUpdates []byte
lastMessageBytes []byte
) )
err = rows.Scan( err = rows.Scan(
&chat.ID, &chat.ID,
@ -219,7 +230,7 @@ func (db sqlitePersistence) chats(tx *sql.Tx) (chats []*Chat, err error) {
&chat.DeletedAtClockValue, &chat.DeletedAtClockValue,
&chat.UnviewedMessagesCount, &chat.UnviewedMessagesCount,
&chat.LastClockValue, &chat.LastClockValue,
&chat.LastMessage, &lastMessageBytes,
&encodedMembers, &encodedMembers,
&encodedMembershipUpdates, &encodedMembershipUpdates,
&chat.Muted, &chat.Muted,
@ -244,6 +255,14 @@ func (db sqlitePersistence) chats(tx *sql.Tx) (chats []*Chat, err error) {
return return
} }
// Restore last message
if lastMessageBytes != nil {
message := &Message{}
if err = json.Unmarshal(lastMessageBytes, message); err != nil {
return
}
chat.LastMessage = message
}
chat.Alias = alias.String chat.Alias = alias.String
chat.Identicon = identicon.String chat.Identicon = identicon.String