From 31a885ced8b6589ee42e62dfb4d11096083e4496 Mon Sep 17 00:00:00 2001 From: Andrea Maria Piana Date: Tue, 15 Dec 2020 16:28:05 +0100 Subject: [PATCH] remove photo path in favor of images in contact --- multiaccounts/database.go | 17 +-- protocol/contact.go | 6 +- protocol/message_handler.go | 71 ++-------- protocol/messenger.go | 1 - protocol/messenger_contact_update_test.go | 2 - protocol/messenger_test.go | 4 - protocol/migrations/migrations.go | 8 +- ..._chat_identity_last_published_table.up.sql | 8 ++ protocol/persistence.go | 132 +++++++++++++++--- protocol/persistence_test.go | 48 +++++++ 10 files changed, 188 insertions(+), 109 deletions(-) diff --git a/multiaccounts/database.go b/multiaccounts/database.go index 784bed910..271f2791e 100644 --- a/multiaccounts/database.go +++ b/multiaccounts/database.go @@ -157,20 +157,13 @@ func (db *Database) GetIdentityImages(keyUID string) ([]*images.IdentityImage, e } func (db *Database) GetIdentityImage(keyUID, it string) (*images.IdentityImage, error) { - rows, err := db.db.Query("SELECT key_uid, name, image_payload, width, height, file_size, resize_target FROM identity_images WHERE key_uid = ? AND name = ?", keyUID, it) - if err != nil { + var ii images.IdentityImage + err := db.db.QueryRow("SELECT key_uid, name, image_payload, width, height, file_size, resize_target FROM identity_images WHERE key_uid = ? AND name = ?", keyUID, it).Scan(&ii.KeyUID, &ii.Name, &ii.Payload, &ii.Width, &ii.Height, &ii.FileSize, &ii.ResizeTarget) + if err == sql.ErrNoRows { + return nil, nil + } else if err != nil { return nil, err } - defer rows.Close() - - var ii images.IdentityImage - for rows.Next() { - err = rows.Scan(&ii.KeyUID, &ii.Name, &ii.Payload, &ii.Width, &ii.Height, &ii.FileSize, &ii.ResizeTarget) - if err != nil { - return nil, err - } - } - return &ii, nil } diff --git a/protocol/contact.go b/protocol/contact.go index 251393dd0..c90b63f47 100644 --- a/protocol/contact.go +++ b/protocol/contact.go @@ -6,6 +6,7 @@ import ( "github.com/status-im/status-go/eth-node/crypto" "github.com/status-im/status-go/eth-node/types" + "github.com/status-im/status-go/images" "github.com/status-im/status-go/protocol/identity/alias" "github.com/status-im/status-go/protocol/identity/identicon" ) @@ -28,7 +29,6 @@ type ContactDeviceInfo struct { // Contact has information about a "Contact". A contact is not necessarily one // that we added or added us, that's based on SystemTags. -// TODO remove use of photoPath in Contact{} type Contact struct { // ID of the contact. It's a hex-encoded public key (prefixed with 0x). ID string `json:"id"` @@ -49,8 +49,6 @@ type Contact struct { Alias string `json:"alias,omitempty"` // Identicon generated from public key Identicon string `json:"identicon"` - // Photo is the base64 encoded photo - Photo string `json:"photoPath,omitempty"` // LastUpdated is the last time we received an update from the contact // updates should be discarded if last updated is less than the one stored LastUpdated uint64 `json:"lastUpdated"` @@ -61,6 +59,8 @@ type Contact struct { DeviceInfo []ContactDeviceInfo `json:"deviceInfo"` TributeToTalk string `json:"tributeToTalk,omitempty"` LocalNickname string `json:"localNickname,omitempty"` + + Images map[string]images.IdentityImage `json:"images"` } func (c Contact) PublicKey() (*ecdsa.PublicKey, error) { diff --git a/protocol/message_handler.go b/protocol/message_handler.go index 6bd96959b..6a0f70779 100644 --- a/protocol/message_handler.go +++ b/protocol/message_handler.go @@ -237,7 +237,6 @@ func (m *MessageHandler) HandleSyncInstallationContact(state *ReceivedMessageSta contact.Name = message.EnsName contact.ENSVerified = false } - contact.Photo = message.ProfileImage contact.LastUpdated = message.Clock contact.LocalNickname = message.LocalNickname @@ -286,7 +285,6 @@ func (m *MessageHandler) HandleContactUpdate(state *ReceivedMessageState, messag contact.Name = message.EnsName contact.ENSVerified = false } - contact.Photo = message.ProfileImage contact.LastUpdated = message.Clock state.ModifiedContacts[contact.ID] = true state.AllContacts[contact.ID] = contact @@ -807,72 +805,23 @@ func (m *MessageHandler) HandleGroupChatInvitation(state *ReceivedMessageState, func (m *MessageHandler) HandleChatIdentity(state *ReceivedMessageState, ci protobuf.ChatIdentity) error { logger := m.logger.With(zap.String("site", "HandleChatIdentity")) contact := state.CurrentMessageState.Contact - chat, ok := state.AllChats[contact.ID] - if !ok { - chat = OneToOneFromPublicKey(state.CurrentMessageState.PublicKey, state.Timesource) - // We don't want to show the chat to the user - chat.Active = false - } logger.Info("Handling contact update") + newImages, err := m.persistence.SaveContactChatIdentity(contact.ID, &ci) + if err != nil { + return err + } + if newImages { + for imageType, image := range ci.Images { + if contact.Images == nil { + contact.Images = make(map[string]images.IdentityImage) + } + contact.Images[imageType] = images.IdentityImage{Name: imageType, Payload: image.Payload} - // TODO investigate potential race condition where user updates this contact's details and the contact update's - // clock value is just less than the ChatIdentity clock and the ChatIdentity is processed first. - // In this case the contact update would not be processed - // - // TODO Potential fix: create an ChatIdentity last updated field on the contact table. - logger.Info(fmt.Sprintf("Update times, contact.LastUpdated '%d', ChatIdentity.Clock '%d'", contact.LastUpdated, ci.Clock)) - if contact.LastUpdated < ci.Clock { - logger.Info("Updating contact") - if !contact.HasBeenAdded() && contact.ID != contactIDFromPublicKey(&m.identity.PublicKey) { - contact.SystemTags = append(contact.SystemTags, contactRequestReceived) } - - // TODO handle ENS things - /* if contact.Name != message.EnsName { - contact.Name = message.EnsName - contact.ENSVerified = false - } */ - - logger.Info(fmt.Sprintf("ChatIdentity has %d images attached", len(ci.Images))) - if len(ci.Images) > 0 { - // Get the largest - var name string - var iiSize int - for n, ii := range ci.Images { - if iiSize < len(ii.Payload) { - iiSize = len(ii.Payload) - name = n - } - } - - logger.Info(fmt.Sprintf("largest image : name '%s', size '%d'", name, iiSize)) - - if ci.Images[name] == nil { - logger.Info("image empty") - return errors.New("image empty") - } - - dataURI, err := images.GetPayloadDataURI(ci.Images[name].Payload) - if err != nil { - return err - } - contact.Photo = dataURI - - logger.Info(fmt.Sprintf("image payload '%s'", dataURI)) - } - - contact.LastUpdated = ci.Clock state.ModifiedContacts[contact.ID] = true state.AllContacts[contact.ID] = contact } - if chat.LastClockValue < ci.Clock { - chat.LastClockValue = ci.Clock - } - - state.ModifiedChats[chat.ID] = true - state.AllChats[chat.ID] = chat - return nil } diff --git a/protocol/messenger.go b/protocol/messenger.go index 5caf515c8..d875cdc20 100644 --- a/protocol/messenger.go +++ b/protocol/messenger.go @@ -2413,7 +2413,6 @@ func (m *Messenger) syncContact(ctx context.Context, contact *Contact) error { Clock: clock, Id: contact.ID, EnsName: contact.Name, - ProfileImage: contact.Photo, LocalNickname: contact.LocalNickname, } encodedMessage, err := proto.Marshal(syncMessage) diff --git a/protocol/messenger_contact_update_test.go b/protocol/messenger_contact_update_test.go index 8e847e7c9..ccb65663f 100644 --- a/protocol/messenger_contact_update_test.go +++ b/protocol/messenger_contact_update_test.go @@ -112,7 +112,6 @@ func (s *MessengerContactUpdateSuite) TestReceiveContactUpdate() { receivedContact := response.Contacts[0] s.Require().Equal(theirName, receivedContact.Name) - s.Require().Equal(theirPicture, receivedContact.Photo) s.Require().False(receivedContact.ENSVerified) s.Require().True(receivedContact.HasBeenAdded()) s.Require().NotEmpty(receivedContact.LastUpdated) @@ -135,7 +134,6 @@ func (s *MessengerContactUpdateSuite) TestReceiveContactUpdate() { receivedContact = response.Contacts[0] s.Require().Equal(theirContactID, receivedContact.ID) s.Require().Equal(newName, receivedContact.Name) - s.Require().Equal(newPicture, receivedContact.Photo) s.Require().False(receivedContact.ENSVerified) s.Require().True(receivedContact.HasBeenAdded()) s.Require().NotEmpty(receivedContact.LastUpdated) diff --git a/protocol/messenger_test.go b/protocol/messenger_test.go index 29ccc2e78..98924ff6d 100644 --- a/protocol/messenger_test.go +++ b/protocol/messenger_test.go @@ -631,7 +631,6 @@ func (s *MessengerSuite) TestRetrieveBlockedContact() { blockedContact := Contact{ ID: publicKeyHex, Name: "contact-name", - Photo: "contact-photo", LastUpdated: 20, SystemTags: []string{contactBlocked}, TributeToTalk: "talk", @@ -1192,7 +1191,6 @@ func (s *MessengerSuite) TestBlockContact() { contact := Contact{ ID: testPK, Name: "contact-name", - Photo: "contact-photo", LastUpdated: 20, SystemTags: []string{contactAdded, contactRequestReceived}, DeviceInfo: []ContactDeviceInfo{ @@ -1375,7 +1373,6 @@ func (s *MessengerSuite) TestContactPersistence() { ID: testPK, Name: "contact-name", - Photo: "contact-photo", LastUpdated: 20, SystemTags: []string{contactAdded, contactRequestReceived}, DeviceInfo: []ContactDeviceInfo{ @@ -1410,7 +1407,6 @@ func (s *MessengerSuite) TestContactPersistenceUpdate() { contact := Contact{ ID: contactID, Name: "contact-name", - Photo: "contact-photo", LastUpdated: 20, SystemTags: []string{contactAdded, contactRequestReceived}, DeviceInfo: []ContactDeviceInfo{ diff --git a/protocol/migrations/migrations.go b/protocol/migrations/migrations.go index 6715e36b4..da0d55b36 100644 --- a/protocol/migrations/migrations.go +++ b/protocol/migrations/migrations.go @@ -32,7 +32,7 @@ // 1603816533_add_links.down.sql (0) // 1603816533_add_links.up.sql (48B) // 1603888149_create_chat_identity_last_published_table.down.sql (40B) -// 1603888149_create_chat_identity_last_published_table.up.sql (175B) +// 1603888149_create_chat_identity_last_published_table.up.sql (407B) // doc.go (850B) package migrations @@ -742,7 +742,7 @@ func _1603888149_create_chat_identity_last_published_tableDownSql() (*asset, err return a, nil } -var __1603888149_create_chat_identity_last_published_tableUpSql = []byte("\x1f\x8b\x08\x00\x00\x00\x00\x00\x00\xff\x3c\xcb\x41\xaa\xc2\x30\x14\x85\xe1\x79\xa1\x7b\x38\xc3\xf7\xc0\x1d\x38\x4a\xc3\x2d\x06\x63\x52\xd2\xab\xd8\x51\x88\x6d\x21\xc5\xa0\x42\x53\xc1\xdd\x0b\x05\x3b\x3d\xe7\xff\xa4\x23\xc1\x04\x16\x95\x26\xa8\x1a\xc6\x32\xe8\xaa\x5a\x6e\xd1\xc7\x90\xfd\x34\x8c\x8f\x3c\xe5\x8f\x4f\x61\xce\xfe\xb5\xdc\xd2\x34\xc7\x71\xc0\x5f\x59\xe0\x57\xe0\x22\x9c\x3c\x08\xb7\x62\x73\xd6\x1a\x8d\x53\x27\xe1\x3a\x1c\xa9\x83\x35\x90\xd6\xd4\x5a\x49\x86\xa3\x46\x0b\x49\xbb\x15\xa7\x67\x7f\xf7\xef\x90\x96\x11\xca\xf0\x86\xd7\x33\x86\x39\xa2\xd2\xb6\xda\xe6\xb2\xf8\xdf\x97\xc5\x37\x00\x00\xff\xff\x0a\xfa\x7a\x2a\xaf\x00\x00\x00") +var __1603888149_create_chat_identity_last_published_tableUpSql = []byte("\x1f\x8b\x08\x00\x00\x00\x00\x00\x00\xff\x8c\x90\xc1\x4a\xc4\x30\x10\x86\xef\x85\xbe\xc3\x1c\x77\x61\xdf\xc0\x53\x1a\x66\x31\x18\xd3\x35\x9b\x8a\x7b\x0a\x63\x1b\x4c\x30\x6e\x0b\xc9\x0a\x7d\x7b\x21\xab\x55\xa1\x88\xc7\xe4\x9f\x9f\xef\x9b\xe1\x1a\x99\x41\x30\xac\x91\x08\x62\x0f\xaa\x35\x80\x4f\xe2\x68\x8e\xd0\x7b\xca\x36\x0c\xee\x9c\x43\x9e\x6d\xa4\x94\xed\x74\x79\x8e\x21\x79\x37\xc0\xa6\xae\xe0\x6b\x02\x1e\x99\xe6\xb7\x4c\x97\xb2\xea\xa4\x84\x83\x16\xf7\x4c\x9f\xe0\x0e\x4f\xd0\x2a\xe0\xad\xda\x4b\xc1\x0d\x68\x3c\x48\xc6\x71\x57\xca\x71\xec\x5f\xed\x3b\xc5\x8b\x03\xa1\xcc\x52\x2e\xa1\xa7\xe4\xa1\x91\x6d\xb3\x7c\xd7\xd5\xf6\xa6\xae\xea\xea\xdf\xc2\xfd\x78\xce\xd4\xe7\xf4\xa9\x7a\x7d\xad\xd9\x16\x60\x78\xa3\x17\x67\xf3\x3c\xb9\xf5\xfc\x4f\xdb\x89\xe6\x38\xd2\xf0\x5b\xb8\x24\x9d\x12\x0f\x1d\x6e\xbe\xe9\xbb\x1f\xa4\xed\xda\x6d\xae\x7b\x7e\x04\x00\x00\xff\xff\xff\xdb\x89\x48\x97\x01\x00\x00") func _1603888149_create_chat_identity_last_published_tableUpSqlBytes() ([]byte, error) { return bindataRead( @@ -757,8 +757,8 @@ func _1603888149_create_chat_identity_last_published_tableUpSql() (*asset, error return nil, err } - info := bindataFileInfo{name: "1603888149_create_chat_identity_last_published_table.up.sql", size: 175, mode: os.FileMode(0644), modTime: time.Unix(1608048601, 0)} - a := &asset{bytes: bytes, info: info, digest: [32]uint8{0xb0, 0x9b, 0x9a, 0xb6, 0x33, 0x2e, 0xb1, 0x8b, 0xe4, 0x18, 0x90, 0xe, 0x40, 0xa4, 0xdd, 0x13, 0x3f, 0x50, 0x55, 0x7, 0x31, 0x50, 0xe0, 0xb8, 0x1e, 0x51, 0xcc, 0xfd, 0xa5, 0x7e, 0x3, 0x7b}} + info := bindataFileInfo{name: "1603888149_create_chat_identity_last_published_table.up.sql", size: 407, mode: os.FileMode(0644), modTime: time.Unix(1608048655, 0)} + a := &asset{bytes: bytes, info: info, digest: [32]uint8{0x7f, 0x9, 0xf, 0xfb, 0xdb, 0x3c, 0x86, 0x70, 0x82, 0xda, 0x10, 0x25, 0xe2, 0x4e, 0x40, 0x45, 0xab, 0x8b, 0x1c, 0x91, 0x7c, 0xf1, 0x70, 0x2e, 0x81, 0xf3, 0x71, 0x45, 0xda, 0xe2, 0xa4, 0x57}} return a, nil } diff --git a/protocol/migrations/sqlite/1603888149_create_chat_identity_last_published_table.up.sql b/protocol/migrations/sqlite/1603888149_create_chat_identity_last_published_table.up.sql index 942aee4e2..1853d808a 100644 --- a/protocol/migrations/sqlite/1603888149_create_chat_identity_last_published_table.up.sql +++ b/protocol/migrations/sqlite/1603888149_create_chat_identity_last_published_table.up.sql @@ -3,3 +3,11 @@ CREATE TABLE IF NOT EXISTS chat_identity_last_published ( clock_value INT NOT NULL, hash BLOB NOT NULL ); + +CREATE TABLE IF NOT EXISTS chat_identity_contacts ( + contact_id VARCHAR NOT NULL, + image_type VARCHAR NOT NULL, + clock_value INT NOT NULL, + payload BLOB NOT NULL, + UNIQUE(contact_id, image_type) ON CONFLICT REPLACE +); diff --git a/protocol/persistence.go b/protocol/persistence.go index 6d6aa50b6..6c346f209 100644 --- a/protocol/persistence.go +++ b/protocol/persistence.go @@ -11,6 +11,7 @@ import ( "github.com/pkg/errors" "github.com/status-im/status-go/eth-node/crypto" + "github.com/status-im/status-go/images" "github.com/status-im/status-go/protocol/common" "github.com/status-im/status-go/protocol/protobuf" ) @@ -378,44 +379,50 @@ func (db sqlitePersistence) Chat(chatID string) (*Chat, error) { } func (db sqlitePersistence) Contacts() ([]*Contact, error) { + allContacts := make(map[string]*Contact) + rows, err := db.db.Query(` SELECT - id, - address, - name, - alias, - identicon, - photo, - last_updated, - system_tags, - device_info, - ens_verified, - ens_verified_at, - tribute_to_talk, - local_nickname - FROM contacts + c.id, + c.address, + c.name, + c.alias, + c.identicon, + c.last_updated, + c.system_tags, + c.device_info, + c.ens_verified, + c.ens_verified_at, + c.tribute_to_talk, + c.local_nickname, + i.image_type, + i.payload + FROM contacts c LEFT JOIN chat_identity_contacts i ON c.id = i.contact_id `) if err != nil { return nil, err } defer rows.Close() - var response []*Contact - for rows.Next() { + var ( contact Contact encodedDeviceInfo []byte encodedSystemTags []byte nickname sql.NullString + imageType sql.NullString + imagePayload []byte ) + + contact.Images = make(map[string]images.IdentityImage) + err := rows.Scan( &contact.ID, &contact.Address, &contact.Name, &contact.Alias, &contact.Identicon, - &contact.Photo, &contact.LastUpdated, &encodedSystemTags, &encodedDeviceInfo, @@ -423,6 +430,8 @@ func (db sqlitePersistence) Contacts() ([]*Contact, error) { &contact.ENSVerifiedAt, &contact.TributeToTalk, &nickname, + &imageType, + &imagePayload, ) if err != nil { return nil, err @@ -448,12 +457,89 @@ func (db sqlitePersistence) Contacts() ([]*Contact, error) { } } - response = append(response, &contact) + previousContact, ok := allContacts[contact.ID] + if !ok { + + if imageType.Valid { + contact.Images[imageType.String] = images.IdentityImage{Name: imageType.String, Payload: imagePayload} + } + + allContacts[contact.ID] = &contact + + } else if imageType.Valid { + previousContact.Images[imageType.String] = images.IdentityImage{Name: imageType.String, Payload: imagePayload} + allContacts[contact.ID] = previousContact + + } } + var response []*Contact + for key := range allContacts { + response = append(response, allContacts[key]) + + } return response, nil } +func (db sqlitePersistence) SaveContactChatIdentity(contactID string, chatIdentity *protobuf.ChatIdentity) (updated bool, err error) { + if chatIdentity.Clock == 0 { + return false, errors.New("clock value unset") + } + + tx, err := db.db.BeginTx(context.Background(), &sql.TxOptions{}) + if err != nil { + return false, err + } + defer func() { + if err == nil { + err = tx.Commit() + return + } + // don't shadow original error + _ = tx.Rollback() + }() + + for imageType, image := range chatIdentity.Images { + var exists bool + err := tx.QueryRow(`SELECT EXISTS(SELECT 1 FROM chat_identity_contacts WHERE contact_id = ? AND image_type = ? AND clock_value >= ?)`, contactID, imageType, chatIdentity.Clock).Scan(&exists) + if err != nil { + return false, err + } + + if exists { + continue + } + + stmt, err := tx.Prepare(`INSERT INTO chat_identity_contacts (contact_id, image_type, clock_value, payload) VALUES (?, ?, ?, ?)`) + if err != nil { + return false, err + } + defer stmt.Close() + if image.Payload == nil { + continue + } + + // Validate image URI to make sure it's serializable + _, err = images.GetPayloadDataURI(image.Payload) + if err != nil { + return false, err + } + + _, err = stmt.Exec( + contactID, + imageType, + chatIdentity.Clock, + image.Payload, + ) + if err != nil { + return false, err + } + updated = true + } + + return +} + func (db sqlitePersistence) SaveRawMessage(message *common.RawMessage) error { var pubKeys [][]byte for _, pk := range message.Recipients { @@ -649,15 +735,15 @@ func (db sqlitePersistence) SaveContact(contact *Contact, tx *sql.Tx) (err error name, alias, identicon, - photo, last_updated, system_tags, device_info, ens_verified, ens_verified_at, tribute_to_talk, - local_nickname - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?,?) + local_nickname, + photo + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?,?, ?) `) if err != nil { return @@ -670,7 +756,6 @@ func (db sqlitePersistence) SaveContact(contact *Contact, tx *sql.Tx) (err error contact.Name, contact.Alias, contact.Identicon, - contact.Photo, contact.LastUpdated, encodedSystemTags.Bytes(), encodedDeviceInfo.Bytes(), @@ -678,6 +763,9 @@ func (db sqlitePersistence) SaveContact(contact *Contact, tx *sql.Tx) (err error contact.ENSVerifiedAt, contact.TributeToTalk, contact.LocalNickname, + // Photo is not used anymore but constrained to be NOT NULL + // we set it to blank for now to avoid a migration of the table + "", ) return } diff --git a/protocol/persistence_test.go b/protocol/persistence_test.go index 6d10c1793..cd3bfbcca 100644 --- a/protocol/persistence_test.go +++ b/protocol/persistence_test.go @@ -671,6 +671,54 @@ func TestSqlitePersistence_GetWhenChatIdentityLastPublished(t *testing.T) { require.Nil(t, actualHash2) } +func TestSaveContactIdentityImage(t *testing.T) { + db, err := openTestDB() + require.NoError(t, err) + p := sqlitePersistence{db: db} + + key, err := crypto.GenerateKey() + require.NoError(t, err) + + contactID := types.EncodeHex(crypto.FromECDSAPub(&key.PublicKey)) + + err = p.SaveContact(&Contact{ID: contactID}, nil) + require.NoError(t, err) + + jpegType := []byte{0xff, 0xd8, 0xff, 0x1} + identityImages := make(map[string]*protobuf.IdentityImage) + identityImages["large"] = &protobuf.IdentityImage{ + Payload: jpegType, + SourceType: protobuf.IdentityImage_RAW_PAYLOAD, + ImageType: protobuf.ImageType_PNG, + } + + identityImages["small"] = &protobuf.IdentityImage{ + Payload: jpegType, + SourceType: protobuf.IdentityImage_RAW_PAYLOAD, + ImageType: protobuf.ImageType_PNG, + } + + images := &protobuf.ChatIdentity{ + Clock: 1, + Images: identityImages, + } + + result, err := p.SaveContactChatIdentity(contactID, images) + require.NoError(t, err) + require.True(t, result) + + // Save again same clock, it should return false + result, err = p.SaveContactChatIdentity(contactID, images) + require.NoError(t, err) + require.False(t, result) + + contacts, err := p.Contacts() + require.NoError(t, err) + require.Len(t, contacts, 1) + + require.Len(t, contacts[0].Images, 2) +} + func TestSaveLinks(t *testing.T) { chatID := testPublicChatID db, err := openTestDB()