From ee0a83fdc4a19f70fa8d08347ef67c0ca3d67f1e Mon Sep 17 00:00:00 2001 From: Andrea Maria Piana Date: Wed, 20 May 2020 14:16:12 +0200 Subject: [PATCH] Load identicon & alias in chat. Why make the changes? Mainly performance, those fields are almost always present in the database but they are re-calculated on load by the client as it does not have necessarily access to it. What has changed? - Remove `_legacy` persistence namespaces as it's a vestige of the initial move frmo status-react to status-go - Pulling chats is now a join with contacts to add contact & alias --- VERSION | 2 +- protocol/chat.go | 5 ++ ...tence_legacy.go => message_persistence.go} | 36 ++++---- protocol/messenger.go | 83 ++++++++++++------- protocol/messenger_test.go | 7 ++ protocol/persistence.go | 35 +++++--- ...nce_legacy_test.go => persistence_test.go} | 12 +-- 7 files changed, 114 insertions(+), 66 deletions(-) rename protocol/{persistence_legacy.go => message_persistence.go} (90%) rename protocol/{persistence_legacy_test.go => persistence_test.go} (96%) diff --git a/VERSION b/VERSION index 3f99a6d57..75fcee630 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.53.1 +0.53.2 diff --git a/protocol/chat.go b/protocol/chat.go index e212f294c..846324ffb 100644 --- a/protocol/chat.go +++ b/protocol/chat.go @@ -59,6 +59,11 @@ type Chat struct { Members []ChatMember `json:"members"` // MembershipUpdates is all the membership events in the chat MembershipUpdates []v1protocol.MembershipUpdateEvent `json:"membershipUpdateEvents"` + + // Generated username name of the chat for one-to-ones + Alias string `json:"alias,omitempty"` + // Identicon generated from public key + Identicon string `json:"identicon"` } func (c *Chat) PublicKey() (*ecdsa.PublicKey, error) { diff --git a/protocol/persistence_legacy.go b/protocol/message_persistence.go similarity index 90% rename from protocol/persistence_legacy.go rename to protocol/message_persistence.go index 5cbb3e9ed..1e5532c77 100644 --- a/protocol/persistence_legacy.go +++ b/protocol/message_persistence.go @@ -16,7 +16,7 @@ var ( errRecordNotFound = errors.New("record not found") ) -func (db sqlitePersistence) tableUserMessagesLegacyAllFields() string { +func (db sqlitePersistence) tableUserMessagesAllFields() string { return `id, whisper_timestamp, source, @@ -47,7 +47,7 @@ func (db sqlitePersistence) tableUserMessagesLegacyAllFields() string { response_to` } -func (db sqlitePersistence) tableUserMessagesLegacyAllFieldsJoin() string { +func (db sqlitePersistence) tableUserMessagesAllFieldsJoin() string { return `m1.id, m1.whisper_timestamp, m1.source, @@ -82,15 +82,15 @@ func (db sqlitePersistence) tableUserMessagesLegacyAllFieldsJoin() string { c.identicon` } -func (db sqlitePersistence) tableUserMessagesLegacyAllFieldsCount() int { - return strings.Count(db.tableUserMessagesLegacyAllFields(), ",") + 1 +func (db sqlitePersistence) tableUserMessagesAllFieldsCount() int { + return strings.Count(db.tableUserMessagesAllFields(), ",") + 1 } type scanner interface { Scan(dest ...interface{}) error } -func (db sqlitePersistence) tableUserMessagesLegacyScanAllFields(row scanner, message *Message, others ...interface{}) error { +func (db sqlitePersistence) tableUserMessagesScanAllFields(row scanner, message *Message, others ...interface{}) error { var quotedText sql.NullString var quotedFrom sql.NullString var alias sql.NullString @@ -157,7 +157,7 @@ func (db sqlitePersistence) tableUserMessagesLegacyScanAllFields(row scanner, me return nil } -func (db sqlitePersistence) tableUserMessagesLegacyAllValues(message *Message) ([]interface{}, error) { +func (db sqlitePersistence) tableUserMessagesAllValues(message *Message) ([]interface{}, error) { sticker := message.GetSticker() if sticker == nil { sticker = &protobuf.StickerMessage{} @@ -217,7 +217,7 @@ func (db sqlitePersistence) messageByID(tx *sql.Tx, id string) (*Message, error) var message Message - allFields := db.tableUserMessagesLegacyAllFieldsJoin() + allFields := db.tableUserMessagesAllFieldsJoin() row := tx.QueryRow( fmt.Sprintf(` SELECT @@ -238,7 +238,7 @@ func (db sqlitePersistence) messageByID(tx *sql.Tx, id string) (*Message, error) `, allFields), id, ) - err = db.tableUserMessagesLegacyScanAllFields(row, &message) + err = db.tableUserMessagesScanAllFields(row, &message) switch err { case sql.ErrNoRows: return nil, errRecordNotFound @@ -253,7 +253,7 @@ func (db sqlitePersistence) MessageByCommandID(chatID, id string) (*Message, err var message Message - allFields := db.tableUserMessagesLegacyAllFieldsJoin() + allFields := db.tableUserMessagesAllFieldsJoin() row := db.db.QueryRow( fmt.Sprintf(` SELECT @@ -279,7 +279,7 @@ func (db sqlitePersistence) MessageByCommandID(chatID, id string) (*Message, err id, chatID, ) - err := db.tableUserMessagesLegacyScanAllFields(row, &message) + err := db.tableUserMessagesScanAllFields(row, &message) switch err { case sql.ErrNoRows: return nil, errRecordNotFound @@ -335,7 +335,7 @@ func (db sqlitePersistence) MessagesByIDs(ids []string) ([]*Message, error) { idsArgs = append(idsArgs, id) } - allFields := db.tableUserMessagesLegacyAllFieldsJoin() + allFields := db.tableUserMessagesAllFieldsJoin() inVector := strings.Repeat("?, ", len(ids)-1) + "?" // nolint: gosec @@ -362,7 +362,7 @@ func (db sqlitePersistence) MessagesByIDs(ids []string) ([]*Message, error) { var result []*Message for rows.Next() { var message Message - if err := db.tableUserMessagesLegacyScanAllFields(rows, &message); err != nil { + if err := db.tableUserMessagesScanAllFields(rows, &message); err != nil { return nil, err } result = append(result, &message) @@ -379,7 +379,7 @@ func (db sqlitePersistence) MessageByChatID(chatID string, currCursor string, li if currCursor != "" { cursorWhere = "AND cursor <= ?" } - allFields := db.tableUserMessagesLegacyAllFieldsJoin() + allFields := db.tableUserMessagesAllFieldsJoin() args := []interface{}{chatID} if currCursor != "" { args = append(args, currCursor) @@ -425,7 +425,7 @@ func (db sqlitePersistence) MessageByChatID(chatID string, currCursor string, li message Message cursor string ) - if err := db.tableUserMessagesLegacyScanAllFields(rows, &message, &cursor); err != nil { + if err := db.tableUserMessagesScanAllFields(rows, &message, &cursor); err != nil { return nil, "", err } result = append(result, &message) @@ -440,7 +440,7 @@ func (db sqlitePersistence) MessageByChatID(chatID string, currCursor string, li return result, newCursor, nil } -func (db sqlitePersistence) SaveMessagesLegacy(messages []*Message) (err error) { +func (db sqlitePersistence) SaveMessages(messages []*Message) (err error) { tx, err := db.db.BeginTx(context.Background(), &sql.TxOptions{}) if err != nil { return @@ -454,8 +454,8 @@ func (db sqlitePersistence) SaveMessagesLegacy(messages []*Message) (err error) _ = tx.Rollback() }() - allFields := db.tableUserMessagesLegacyAllFields() - valuesVector := strings.Repeat("?, ", db.tableUserMessagesLegacyAllFieldsCount()-1) + "?" + allFields := db.tableUserMessagesAllFields() + valuesVector := strings.Repeat("?, ", db.tableUserMessagesAllFieldsCount()-1) + "?" query := "INSERT INTO user_messages(" + allFields + ") VALUES (" + valuesVector + ")" // nolint: gosec stmt, err := tx.Prepare(query) if err != nil { @@ -464,7 +464,7 @@ func (db sqlitePersistence) SaveMessagesLegacy(messages []*Message) (err error) for _, msg := range messages { var allValues []interface{} - allValues, err = db.tableUserMessagesLegacyAllValues(msg) + allValues, err = db.tableUserMessagesAllValues(msg) if err != nil { return } diff --git a/protocol/messenger.go b/protocol/messenger.go index 84f5599d5..61ace70cf 100644 --- a/protocol/messenger.go +++ b/protocol/messenger.go @@ -687,7 +687,7 @@ func (m *Messenger) CreateGroupChatWithMembers(ctx context.Context, name string, response.Chats = []*Chat{&chat} response.Messages = buildSystemMessages(chat.MembershipUpdates, m.systemMessagesTranslations) - err = m.persistence.SaveMessagesLegacy(response.Messages) + err = m.persistence.SaveMessages(response.Messages) if err != nil { return nil, err } @@ -752,7 +752,7 @@ func (m *Messenger) RemoveMemberFromGroupChat(ctx context.Context, chatID string response.Chats = []*Chat{chat} response.Messages = buildSystemMessages(chat.MembershipUpdates, m.systemMessagesTranslations) - err = m.persistence.SaveMessagesLegacy(response.Messages) + err = m.persistence.SaveMessages(response.Messages) if err != nil { return nil, err } @@ -816,7 +816,7 @@ func (m *Messenger) AddMembersToGroupChat(ctx context.Context, chatID string, me response.Chats = []*Chat{chat} response.Messages = buildSystemMessages([]v1protocol.MembershipUpdateEvent{event}, m.systemMessagesTranslations) - err = m.persistence.SaveMessagesLegacy(response.Messages) + err = m.persistence.SaveMessages(response.Messages) if err != nil { return nil, err } @@ -882,7 +882,7 @@ func (m *Messenger) ChangeGroupChatName(ctx context.Context, chatID string, name var response MessengerResponse response.Chats = []*Chat{chat} response.Messages = buildSystemMessages([]v1protocol.MembershipUpdateEvent{event}, m.systemMessagesTranslations) - err = m.persistence.SaveMessagesLegacy(response.Messages) + err = m.persistence.SaveMessages(response.Messages) if err != nil { return nil, err } @@ -947,7 +947,7 @@ func (m *Messenger) AddAdminsToGroupChat(ctx context.Context, chatID string, mem response.Chats = []*Chat{chat} response.Messages = buildSystemMessages([]v1protocol.MembershipUpdateEvent{event}, m.systemMessagesTranslations) - err = m.persistence.SaveMessagesLegacy(response.Messages) + err = m.persistence.SaveMessages(response.Messages) if err != nil { return nil, err } @@ -1014,7 +1014,7 @@ func (m *Messenger) ConfirmJoiningGroup(ctx context.Context, chatID string) (*Me response.Chats = []*Chat{chat} response.Messages = buildSystemMessages([]v1protocol.MembershipUpdateEvent{event}, m.systemMessagesTranslations) - err = m.persistence.SaveMessagesLegacy(response.Messages) + err = m.persistence.SaveMessages(response.Messages) if err != nil { return nil, err } @@ -1086,7 +1086,7 @@ func (m *Messenger) LeaveGroupChat(ctx context.Context, chatID string, remove bo response.Chats = []*Chat{chat} response.Messages = buildSystemMessages([]v1protocol.MembershipUpdateEvent{event}, m.systemMessagesTranslations) - err = m.persistence.SaveMessagesLegacy(response.Messages) + err = m.persistence.SaveMessages(response.Messages) if err != nil { return nil, err } @@ -1096,6 +1096,15 @@ func (m *Messenger) LeaveGroupChat(ctx context.Context, chatID string, remove bo func (m *Messenger) saveChat(chat *Chat) error { _, ok := m.allChats[chat.ID] + if chat.OneToOne() { + name, identicon, err := generateAliasAndIdenticon(chat.ID) + if err != nil { + return err + } + + chat.Alias = name + chat.Identicon = identicon + } // Sync chat if it's a new active public chat if !ok && chat.Active && chat.Public() { if err := m.syncPublicChat(context.Background(), chat); err != nil { @@ -1163,18 +1172,12 @@ func (m *Messenger) isNewContact(contact *Contact) bool { } func (m *Messenger) saveContact(contact *Contact) error { - identicon, err := identicon.GenerateBase64(contact.ID) + name, identicon, err := generateAliasAndIdenticon(contact.ID) if err != nil { return err } contact.Identicon = identicon - - name, err := alias.GenerateFromPublicKeyString(contact.ID) - if err != nil { - return err - } - contact.Alias = name if m.isNewContact(contact) { @@ -1190,9 +1193,10 @@ func (m *Messenger) saveContact(contact *Contact) error { } m.allContacts[contact.ID] = contact - return nil + return nil } + func (m *Messenger) SaveContact(contact *Contact) error { m.mutex.Lock() defer m.mutex.Unlock() @@ -1453,7 +1457,7 @@ func (m *Messenger) SendChatMessage(ctx context.Context, message *Message) (*Mes return nil, err } - err = m.persistence.SaveMessagesLegacy([]*Message{message}) + err = m.persistence.SaveMessages([]*Message{message}) if err != nil { return nil, err } @@ -1981,10 +1985,6 @@ func (m *Messenger) handleRetrievedMessages(chatWithMessages map[transport.Filte } } - for id := range messageState.ModifiedChats { - messageState.Response.Chats = append(messageState.Response.Chats, messageState.AllChats[id]) - } - var contactsToSave []*Contact for id := range messageState.ModifiedContacts { contact := messageState.AllContacts[id] @@ -1999,6 +1999,19 @@ func (m *Messenger) handleRetrievedMessages(chatWithMessages map[transport.Filte } } + for id := range messageState.ModifiedChats { + chat := messageState.AllChats[id] + if chat.OneToOne() { + contact, ok := m.allContacts[chat.ID] + if ok { + chat.Alias = contact.Alias + chat.Identicon = contact.Identicon + } + } + + messageState.Response.Chats = append(messageState.Response.Chats, chat) + } + for id := range messageState.ModifiedInstallations { installation := messageState.AllInstallations[id] messageState.Response.Installations = append(messageState.Response.Installations, installation) @@ -2082,7 +2095,7 @@ func (m *Messenger) MessageByChatID(chatID, cursor string, limit int) ([]*Messag } func (m *Messenger) SaveMessages(messages []*Message) error { - return m.persistence.SaveMessagesLegacy(messages) + return m.persistence.SaveMessages(messages) } func (m *Messenger) DeleteMessage(id string) error { @@ -2273,7 +2286,7 @@ func (m *Messenger) RequestTransaction(ctx context.Context, chatID, value, contr return nil, err } - err = m.persistence.SaveMessagesLegacy([]*Message{message}) + err = m.persistence.SaveMessages([]*Message{message}) if err != nil { return nil, err } @@ -2349,7 +2362,7 @@ func (m *Messenger) RequestAddressForTransaction(ctx context.Context, chatID, fr return nil, err } - err = m.persistence.SaveMessagesLegacy([]*Message{message}) + err = m.persistence.SaveMessages([]*Message{message}) if err != nil { return nil, err } @@ -2444,7 +2457,7 @@ func (m *Messenger) AcceptRequestAddressForTransaction(ctx context.Context, mess return nil, err } - err = m.persistence.SaveMessagesLegacy([]*Message{message}) + err = m.persistence.SaveMessages([]*Message{message}) if err != nil { return nil, err } @@ -2526,7 +2539,7 @@ func (m *Messenger) DeclineRequestTransaction(ctx context.Context, messageID str return nil, err } - err = m.persistence.SaveMessagesLegacy([]*Message{message}) + err = m.persistence.SaveMessages([]*Message{message}) if err != nil { return nil, err } @@ -2608,7 +2621,7 @@ func (m *Messenger) DeclineRequestAddressForTransaction(ctx context.Context, mes return nil, err } - err = m.persistence.SaveMessagesLegacy([]*Message{message}) + err = m.persistence.SaveMessages([]*Message{message}) if err != nil { return nil, err } @@ -2707,7 +2720,7 @@ func (m *Messenger) AcceptRequestTransaction(ctx context.Context, transactionHas return nil, err } - err = m.persistence.SaveMessagesLegacy([]*Message{message}) + err = m.persistence.SaveMessages([]*Message{message}) if err != nil { return nil, err } @@ -2788,7 +2801,7 @@ func (m *Messenger) SendTransaction(ctx context.Context, chatID, value, contract return nil, err } - err = m.persistence.SaveMessagesLegacy([]*Message{message}) + err = m.persistence.SaveMessages([]*Message{message}) if err != nil { return nil, err } @@ -2931,3 +2944,17 @@ func (m *Messenger) getTimesource() TimeSource { func (m *Messenger) Timesource() TimeSource { return m.getTimesource() } + +func generateAliasAndIdenticon(pk string) (string, string, error) { + identicon, err := identicon.GenerateBase64(pk) + if err != nil { + return "", "", err + } + + name, err := alias.GenerateFromPublicKeyString(pk) + if err != nil { + return "", "", err + } + return name, identicon, nil + +} diff --git a/protocol/messenger_test.go b/protocol/messenger_test.go index b58ec523e..441bbecd8 100644 --- a/protocol/messenger_test.go +++ b/protocol/messenger_test.go @@ -1025,6 +1025,10 @@ func (s *MessengerSuite) TestChatPersistenceOneToOne() { UnviewedMessagesCount: 40, LastMessage: []byte("test"), } + contact := Contact{ + ID: testPK, + } + publicKeyBytes, err := hex.DecodeString(testPK[2:]) s.Require().NoError(err) @@ -1032,6 +1036,7 @@ func (s *MessengerSuite) TestChatPersistenceOneToOne() { s.Require().NoError(err) s.Require().NoError(s.m.SaveChat(&chat)) + s.Require().NoError(s.m.SaveContact(&contact)) savedChats := s.m.Chats() s.Require().Equal(1, len(savedChats)) @@ -1044,6 +1049,8 @@ func (s *MessengerSuite) TestChatPersistenceOneToOne() { s.Require().Equal(pk, actualPk) s.Require().Equal(expectedChat, actualChat) + s.Require().NotEmpty(actualChat.Identicon) + s.Require().NotEmpty(actualChat.Alias) } func (s *MessengerSuite) TestChatPersistencePrivateGroupChat() { diff --git a/protocol/persistence.go b/protocol/persistence.go index 411836bd3..9ef0720ba 100644 --- a/protocol/persistence.go +++ b/protocol/persistence.go @@ -158,19 +158,21 @@ func (db sqlitePersistence) chats(tx *sql.Tx) (chats []*Chat, err error) { rows, err := tx.Query(` SELECT - id, - name, - color, - active, - type, - timestamp, - deleted_at_clock_value, - unviewed_message_count, - last_clock_value, - last_message, - members, - membership_updates - FROM chats + chats.id, + chats.name, + chats.color, + chats.active, + chats.type, + chats.timestamp, + chats.deleted_at_clock_value, + chats.unviewed_message_count, + chats.last_clock_value, + chats.last_message, + chats.members, + chats.membership_updates, + contacts.identicon, + contacts.alias + FROM chats LEFT JOIN contacts ON chats.id = contacts.id ORDER BY chats.timestamp DESC `) if err != nil { @@ -180,6 +182,8 @@ func (db sqlitePersistence) chats(tx *sql.Tx) (chats []*Chat, err error) { for rows.Next() { var ( + alias sql.NullString + identicon sql.NullString chat Chat encodedMembers []byte encodedMembershipUpdates []byte @@ -197,6 +201,8 @@ func (db sqlitePersistence) chats(tx *sql.Tx) (chats []*Chat, err error) { &chat.LastMessage, &encodedMembers, &encodedMembershipUpdates, + &identicon, + &alias, ) if err != nil { return @@ -216,6 +222,9 @@ func (db sqlitePersistence) chats(tx *sql.Tx) (chats []*Chat, err error) { return } + chat.Alias = alias.String + chat.Identicon = identicon.String + chats = append(chats, &chat) } diff --git a/protocol/persistence_legacy_test.go b/protocol/persistence_test.go similarity index 96% rename from protocol/persistence_legacy_test.go rename to protocol/persistence_test.go index acb6dbe2a..d11683cd0 100644 --- a/protocol/persistence_legacy_test.go +++ b/protocol/persistence_test.go @@ -17,8 +17,8 @@ import ( func TestTableUserMessagesAllFieldsCount(t *testing.T) { db := sqlitePersistence{} - expected := len(strings.Split(db.tableUserMessagesLegacyAllFields(), ",")) - require.Equal(t, expected, db.tableUserMessagesLegacyAllFieldsCount()) + expected := len(strings.Split(db.tableUserMessagesAllFields(), ",")) + require.Equal(t, expected, db.tableUserMessagesAllFieldsCount()) } func TestSaveMessages(t *testing.T) { @@ -141,7 +141,7 @@ func TestMessageByChatID(t *testing.T) { }) } - err = p.SaveMessagesLegacy(messages) + err = p.SaveMessages(messages) require.NoError(t, err) var ( @@ -215,7 +215,7 @@ func TestMessageReplies(t *testing.T) { messages := []*Message{message1, message2, message3} - err = p.SaveMessagesLegacy(messages) + err = p.SaveMessages(messages) require.NoError(t, err) retrievedMessages, _, err := p.MessageByChatID(chatID, "", 10) @@ -253,7 +253,7 @@ func TestMessageByChatIDWithTheSameClocks(t *testing.T) { }) } - err = p.SaveMessagesLegacy(messages) + err = p.SaveMessages(messages) require.NoError(t, err) var ( @@ -384,7 +384,7 @@ func openTestDB() (*sql.DB, error) { } func insertMinimalMessage(p sqlitePersistence, id string) error { - return p.SaveMessagesLegacy([]*Message{{ + return p.SaveMessages([]*Message{{ ID: id, LocalChatID: "chat-id", ChatMessage: protobuf.ChatMessage{Text: "some-text"},