fix(image-album): make sure to delete all images part of an album (#3380)

* fix(image-album): make sure to delete all images part of an album

* test(delete): add test that deletes a message part of an album

* test(delete): add test where the signal to delete images is after

also adds the handling of deleteForMe

* fix(delete): add album deletion handling for deleteForMe
This commit is contained in:
Jonathan Rainville 2023-04-14 13:17:56 -04:00 committed by GitHub
parent 302437e32e
commit 62da2d404c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 356 additions and 104 deletions

View File

@ -1 +1 @@
0.145.0
0.145.1

View File

@ -6176,3 +6176,19 @@ func (m *Messenger) myHexIdentity() string {
func (m *Messenger) GetMentionsManager() *MentionManager {
return m.mentionsManager
}
func (m *Messenger) getMessagesToDelete(message *common.Message, chatID string) ([]*common.Message, error) {
var messagesToDelete []*common.Message
// In case of Image messages, we need to delete all the images in the album
if message.ContentType == protobuf.ChatMessage_IMAGE {
image := message.GetImage()
messagesInTheAlbum, err := m.persistence.albumMessages(chatID, image.GetAlbumId())
if err != nil {
return nil, err
}
messagesToDelete = append(messagesToDelete, messagesInTheAlbum...)
} else {
messagesToDelete = append(messagesToDelete, message)
}
return messagesToDelete, nil
}

View File

@ -238,3 +238,156 @@ func (s *MessengerDeleteMessageSuite) TestDeleteMessageFirstThenMessage() {
s.Require().Len(state.Response.RemovedMessages(), 0)
s.Require().Nil(state.Response.Chats()[0].LastMessage)
}
func (s *MessengerDeleteMessageSuite) TestDeleteImageMessage() {
theirMessenger := s.newMessenger()
_, err := theirMessenger.Start()
s.Require().NoError(err)
theirChat := CreateOneToOneChat("Their 1TO1", &s.privateKey.PublicKey, s.m.transport)
err = theirMessenger.SaveChat(theirChat)
s.Require().NoError(err)
ourChat := CreateOneToOneChat("Our 1TO1", &theirMessenger.identity.PublicKey, s.m.transport)
err = s.m.SaveChat(ourChat)
s.Require().NoError(err)
messageCount := 3
var album []*common.Message
for i := 0; i < messageCount; i++ {
image, err := buildImageWithoutAlbumIDMessage(*ourChat)
s.NoError(err)
album = append(album, image)
}
response, err := s.m.SendChatMessages(context.Background(), album)
s.NoError(err)
// Check that album count was the number of the images sent
imagesCount := uint32(0)
for _, message := range response.Messages() {
if message.ContentType == protobuf.ChatMessage_IMAGE {
imagesCount++
}
}
for _, message := range response.Messages() {
s.Require().NotNil(message.GetImage())
s.Require().Equal(message.GetImage().AlbumImagesCount, imagesCount)
}
s.Require().Equal(messageCount, len(response.Messages()), "it returns the messages")
s.Require().NoError(err)
s.Require().Len(response.Messages(), messageCount)
response, err = WaitOnMessengerResponse(
theirMessenger,
func(r *MessengerResponse) bool { return len(r.messages) == messageCount },
"no messages",
)
s.Require().NoError(err)
s.Require().Len(response.Chats(), 1)
s.Require().Len(response.Messages(), messageCount)
for _, message := range response.Messages() {
image := message.GetImage()
s.Require().NotNil(image, "Message.ID=%s", message.ID)
s.Require().Equal(image.AlbumImagesCount, imagesCount)
s.Require().NotEmpty(image.AlbumId, "Message.ID=%s", message.ID)
}
firstMessageID := response.Messages()[0].ID
sendResponse, err := s.m.DeleteMessageAndSend(context.Background(), firstMessageID)
s.Require().NoError(err)
s.Require().Len(sendResponse.Messages(), 0)
s.Require().Len(sendResponse.RemovedMessages(), 3)
s.Require().Equal(sendResponse.RemovedMessages()[0].DeletedBy, "")
s.Require().Len(sendResponse.Chats(), 1)
// LastMessage is removed
s.Require().Nil(sendResponse.Chats()[0].LastMessage)
// Main instance user attempts to delete the message it received from theirMessenger
_, err = theirMessenger.DeleteMessageAndSend(context.Background(), firstMessageID)
s.Require().ErrorContains(err, "Chat not found")
}
func (s *MessengerDeleteMessageSuite) TestDeleteImageMessageFirstThenMessage() {
theirMessenger := s.newMessenger()
_, err := theirMessenger.Start()
s.Require().NoError(err)
theirChat := CreateOneToOneChat("Their 1TO1", &s.privateKey.PublicKey, s.m.transport)
err = theirMessenger.SaveChat(theirChat)
s.Require().NoError(err)
contact, err := BuildContactFromPublicKey(&theirMessenger.identity.PublicKey)
s.Require().NoError(err)
ourChat := CreateOneToOneChat("Our 1TO1", &theirMessenger.identity.PublicKey, s.m.transport)
err = s.m.SaveChat(ourChat)
s.Require().NoError(err)
messageID1 := "message-id1"
messageID2 := "message-id2"
messageCount := 2
var album []*common.Message
for i := 0; i < messageCount; i++ {
image, err := buildImageWithoutAlbumIDMessage(*ourChat)
image.Clock = 1
s.NoError(err)
album = append(album, image)
}
deleteMessage := DeleteMessage{
DeleteMessage: protobuf.DeleteMessage{
Clock: 2,
MessageType: protobuf.MessageType_ONE_TO_ONE,
MessageId: messageID1,
ChatId: theirChat.ID,
},
From: common.PubkeyToHex(&theirMessenger.identity.PublicKey),
}
state := &ReceivedMessageState{
Response: &MessengerResponse{},
}
// Handle Delete first
err = s.m.HandleDeleteMessage(state, deleteMessage)
s.Require().NoError(err)
// Handle first image message
state = &ReceivedMessageState{
Response: &MessengerResponse{},
CurrentMessageState: &CurrentMessageState{
Message: album[0].ChatMessage,
MessageID: messageID1,
WhisperTimestamp: s.m.getTimesource().GetCurrentTime(),
Contact: contact,
PublicKey: &theirMessenger.identity.PublicKey,
},
}
err = s.m.HandleChatMessage(state)
s.Require().NoError(err)
s.Require().Len(state.Response.Messages(), 0) // Message should not be added to response
s.Require().Len(state.Response.RemovedMessages(), 0)
s.Require().Nil(state.Response.Chats()[0].LastMessage)
// Handle second image message
state = &ReceivedMessageState{
Response: &MessengerResponse{},
CurrentMessageState: &CurrentMessageState{
Message: album[1].ChatMessage,
MessageID: messageID2,
WhisperTimestamp: s.m.getTimesource().GetCurrentTime(),
Contact: contact,
PublicKey: &theirMessenger.identity.PublicKey,
},
}
err = s.m.HandleChatMessage(state)
s.Require().NoError(err)
s.Require().Len(state.Response.Messages(), 0) // Message should not be added to response even if we didn't delete that ID
s.Require().Len(state.Response.RemovedMessages(), 0)
s.Require().Nil(state.Response.Chats()[0].LastMessage)
}

View File

@ -1583,44 +1583,51 @@ func (m *Messenger) HandleDeleteMessage(state *ReceivedMessageState, deleteMessa
}
}
// Update message and return it
originalMessage.Deleted = true
originalMessage.DeletedBy = deleteMessage.DeleteMessage.DeletedBy
err := m.persistence.SaveMessages([]*common.Message{originalMessage})
messagesToDelete, err := m.getMessagesToDelete(originalMessage, deleteMessage.ChatId)
if err != nil {
return err
}
m.logger.Debug("deleting activity center notification for message", zap.String("chatID", chat.ID), zap.String("messageID", deleteMessage.MessageId))
err = m.persistence.DeleteActivityCenterNotificationForMessage(chat.ID, deleteMessage.MessageId)
if err != nil {
m.logger.Warn("failed to delete notifications for deleted message", zap.Error(err))
return err
}
if chat.LastMessage != nil && chat.LastMessage.ID == originalMessage.ID {
if err := m.updateLastMessage(chat); err != nil {
for _, messageToDelete := range messagesToDelete {
messageToDelete.Deleted = true
messageToDelete.DeletedBy = deleteMessage.DeleteMessage.DeletedBy
err := m.persistence.SaveMessages([]*common.Message{messageToDelete})
if err != nil {
return err
}
if chat.LastMessage != nil && !chat.LastMessage.Seen && chat.OneToOne() && !chat.Active {
m.createMessageNotification(chat, state)
m.logger.Debug("deleting activity center notification for message", zap.String("chatID", chat.ID), zap.String("messageID", messageToDelete.ID))
err = m.persistence.DeleteActivityCenterNotificationForMessage(chat.ID, messageToDelete.ID)
if err != nil {
m.logger.Warn("failed to delete notifications for deleted message", zap.Error(err))
return err
}
state.Response.AddRemovedMessage(&RemovedMessage{MessageID: messageToDelete.ID, ChatID: chat.ID, DeletedBy: deleteMessage.DeleteMessage.DeletedBy})
state.Response.AddNotification(DeletedMessageNotification(messageToDelete.ID, chat))
state.Response.AddActivityCenterNotification(&ActivityCenterNotification{
ID: types.FromHex(messageToDelete.ID),
Deleted: true,
})
if chat.LastMessage != nil && chat.LastMessage.ID == originalMessage.ID {
if err := m.updateLastMessage(chat); err != nil {
return err
}
if chat.LastMessage != nil && !chat.LastMessage.Seen && chat.OneToOne() && !chat.Active {
m.createMessageNotification(chat, state)
}
}
}
state.Response.AddRemovedMessage(&RemovedMessage{MessageID: messageID, ChatID: chat.ID, DeletedBy: deleteMessage.DeleteMessage.DeletedBy})
state.Response.AddChat(chat)
state.Response.AddNotification(DeletedMessageNotification(messageID, chat))
state.Response.AddActivityCenterNotification(&ActivityCenterNotification{
ID: types.FromHex(messageID),
Deleted: true,
})
return nil
}
// TODO do this delete too
func (m *Messenger) HandleDeleteForMeMessage(state *ReceivedMessageState, deleteForMeMessage DeleteForMeMessage) error {
if err := ValidateDeleteForMeMessage(deleteForMeMessage.DeleteForMeMessage); err != nil {
return err
@ -1648,29 +1655,35 @@ func (m *Messenger) HandleDeleteForMeMessage(state *ReceivedMessageState, delete
return errors.New("chat not found")
}
// Update message and return it
originalMessage.DeletedForMe = true
err := m.persistence.SaveMessages([]*common.Message{originalMessage})
messagesToDelete, err := m.getMessagesToDelete(originalMessage, deleteForMeMessage.LocalChatID)
if err != nil {
return err
}
m.logger.Debug("deleting activity center notification for message", zap.String("chatID", chat.ID), zap.String("messageID", deleteForMeMessage.MessageId))
for _, messageToDelete := range messagesToDelete {
messageToDelete.DeletedForMe = true
err = m.persistence.DeleteActivityCenterNotificationForMessage(chat.ID, deleteForMeMessage.MessageId)
if err != nil {
m.logger.Warn("failed to delete notifications for deleted message", zap.Error(err))
return err
}
if chat.LastMessage != nil && chat.LastMessage.ID == originalMessage.ID {
if err := m.updateLastMessage(chat); err != nil {
err := m.persistence.SaveMessages([]*common.Message{messageToDelete})
if err != nil {
return err
}
}
state.Response.AddMessage(originalMessage)
m.logger.Debug("deleting activity center notification for message", zap.String("chatID", chat.ID), zap.String("messageID", messageToDelete.ID))
err = m.persistence.DeleteActivityCenterNotificationForMessage(chat.ID, messageToDelete.ID)
if err != nil {
m.logger.Warn("failed to delete notifications for deleted message", zap.Error(err))
return err
}
if chat.LastMessage != nil && chat.LastMessage.ID == messageToDelete.ID {
if err := m.updateLastMessage(chat); err != nil {
return err
}
}
state.Response.AddMessage(messageToDelete)
}
state.Response.AddChat(chat)
return nil
@ -2628,34 +2641,82 @@ func (m *Messenger) checkForEdits(message *common.Message) error {
return nil
}
func (m *Messenger) getMessagesToCheckForDelete(message *common.Message) ([]*common.Message, error) {
var messagesToCheck []*common.Message
if message.ContentType == protobuf.ChatMessage_IMAGE {
image := message.GetImage()
messagesInTheAlbum, err := m.persistence.albumMessages(message.ChatId, image.GetAlbumId())
if err != nil {
return nil, err
}
messagesToCheck = append(messagesToCheck, messagesInTheAlbum...)
}
messagesToCheck = append(messagesToCheck, message)
return messagesToCheck, nil
}
func (m *Messenger) checkForDeletes(message *common.Message) error {
// Check for any pending deletes
// If any pending deletes are available and valid, apply them
messageDeletes, err := m.persistence.GetDeletes(message.ID, message.From)
messagesToCheck, err := m.getMessagesToCheckForDelete(message)
if err != nil {
return err
}
if len(messageDeletes) == 0 {
return nil
}
var messageDeletes []*DeleteMessage
applyDelete := false
for _, messageToCheck := range messagesToCheck {
if !applyDelete {
// Check for any pending deletes
// If any pending deletes are available and valid, apply them
messageDeletes, err = m.persistence.GetDeletes(messageToCheck.ID, messageToCheck.From)
if err != nil {
return err
}
return m.applyDeleteMessage(messageDeletes, message)
if len(messageDeletes) == 0 {
continue
}
}
// Once one messageDelete has been found, we apply it to all the images in the album
applyDelete = true
err := m.applyDeleteMessage(messageDeletes, messageToCheck)
if err != nil {
return err
}
}
return nil
}
func (m *Messenger) checkForDeleteForMes(message *common.Message) error {
// Check for any pending delete for mes
// If any pending deletes are available and valid, apply them
messageDeleteForMes, err := m.persistence.GetDeleteForMes(message.ID, message.From)
messagesToCheck, err := m.getMessagesToCheckForDelete(message)
if err != nil {
return err
}
if len(messageDeleteForMes) == 0 {
return nil
}
var messageDeleteForMes []*DeleteForMeMessage
applyDelete := false
for _, messageToCheck := range messagesToCheck {
if !applyDelete {
// Check for any pending delete for mes
// If any pending deletes are available and valid, apply them
messageDeleteForMes, err = m.persistence.GetDeleteForMes(messageToCheck.ID, messageToCheck.From)
if err != nil {
return err
}
return m.applyDeleteForMeMessage(messageDeleteForMes, message)
if len(messageDeleteForMes) == 0 {
continue
}
}
// Once one messageDeleteForMes has been found, we apply it to all the images in the album
applyDelete = true
err := m.applyDeleteForMeMessage(messageDeleteForMes, messageToCheck)
if err != nil {
return err
}
}
return nil
}
func (m *Messenger) isMessageAllowedFrom(publicKey string, chat *Chat) (bool, error) {

View File

@ -157,6 +157,11 @@ func (m *Messenger) DeleteMessageAndSend(ctx context.Context, messageID string)
return nil, ErrInvalidDeleteTypeAuthor
}
messagesToDelete, err := m.getMessagesToDelete(message, message.ChatId)
if err != nil {
return nil, err
}
clock, _ := chat.NextClockAndTimestamp(m.getTimesource())
deleteMessage := &DeleteMessage{}
@ -184,22 +189,24 @@ func (m *Messenger) DeleteMessageAndSend(ctx context.Context, messageID string)
return nil, err
}
message.Deleted = true
message.DeletedBy = deletedBy
err = m.persistence.SaveMessages([]*common.Message{message})
if err != nil {
return nil, err
}
if chat.LastMessage != nil && chat.LastMessage.ID == message.ID {
if err := m.updateLastMessage(chat); err != nil {
response := &MessengerResponse{}
for _, messageToDelete := range messagesToDelete {
messageToDelete.Deleted = true
messageToDelete.DeletedBy = deletedBy
err = m.persistence.SaveMessages([]*common.Message{messageToDelete})
if err != nil {
return nil, err
}
response.AddMessage(messageToDelete)
response.AddRemovedMessage(&RemovedMessage{MessageID: messageToDelete.ID, ChatID: chat.ID, DeletedBy: deletedBy})
if chat.LastMessage != nil && chat.LastMessage.ID == messageToDelete.ID {
if err := m.updateLastMessage(chat); err != nil {
return nil, err
}
}
}
response := &MessengerResponse{}
response.AddMessage(message)
response.AddRemovedMessage(&RemovedMessage{MessageID: messageID, ChatID: chat.ID, DeletedBy: deletedBy})
response.AddChat(chat)
return response, nil
@ -226,20 +233,27 @@ func (m *Messenger) DeleteMessageForMeAndSync(ctx context.Context, chatID string
return nil, ErrInvalidDeleteTypeAuthor
}
message.DeletedForMe = true
err = m.persistence.SaveMessages([]*common.Message{message})
messagesToDelete, err := m.getMessagesToDelete(message, message.ChatId)
if err != nil {
return nil, err
}
if chat.LastMessage != nil && chat.LastMessage.ID == message.ID {
if err := m.updateLastMessage(chat); err != nil {
response := &MessengerResponse{}
for _, messageToDelete := range messagesToDelete {
messageToDelete.DeletedForMe = true
err = m.persistence.SaveMessages([]*common.Message{messageToDelete})
if err != nil {
return nil, err
}
}
response := &MessengerResponse{}
response.AddMessage(message)
if chat.LastMessage != nil && chat.LastMessage.ID == messageToDelete.ID {
if err := m.updateLastMessage(chat); err != nil {
return nil, err
}
}
response.AddMessage(messageToDelete)
}
response.AddChat(chat)
if m.hasPairedDevices() {

View File

@ -3,8 +3,6 @@ package protocol
import (
"context"
"crypto/ecdsa"
"io/ioutil"
"os"
"testing"
"github.com/stretchr/testify/suite"
@ -64,34 +62,6 @@ func (s *MessengerSendImagesAlbumSuite) newMessenger() *Messenger {
return messenger
}
func buildImageWithoutAlbumIDMessage(s *MessengerSendImagesAlbumSuite, chat Chat) *common.Message {
file, err := os.Open("../_assets/tests/test.jpg")
s.Require().NoError(err)
defer file.Close()
payload, err := ioutil.ReadAll(file)
s.Require().NoError(err)
clock, timestamp := chat.NextClockAndTimestamp(&testTimeSource{})
message := &common.Message{}
message.ChatId = chat.ID
message.Clock = clock
message.Timestamp = timestamp
message.WhisperTimestamp = clock
message.LocalChatID = chat.ID
message.MessageType = protobuf.MessageType_ONE_TO_ONE
message.ContentType = protobuf.ChatMessage_IMAGE
image := protobuf.ImageMessage{
Payload: payload,
Type: protobuf.ImageType_JPEG,
Width: 1200,
Height: 1000,
}
message.Payload = &protobuf.ChatMessage_Image{Image: &image}
return message
}
func (s *MessengerSendImagesAlbumSuite) TestAlbumImageMessagesSend() {
theirMessenger := s.newMessenger()
_, err := theirMessenger.Start()
@ -109,7 +79,9 @@ func (s *MessengerSendImagesAlbumSuite) TestAlbumImageMessagesSend() {
var album []*common.Message
for i := 0; i < messageCount; i++ {
album = append(album, buildImageWithoutAlbumIDMessage(s, *ourChat))
image, err := buildImageWithoutAlbumIDMessage(*ourChat)
s.NoError(err)
album = append(album, image)
}
err = s.m.SaveChat(ourChat)
@ -167,7 +139,8 @@ func (s *MessengerSendImagesAlbumSuite) TestAlbumImageMessagesWithMentionSend()
var album []*common.Message
for i := 0; i < messageCount; i++ {
outgoingMessage := buildImageWithoutAlbumIDMessage(s, *ourChat)
outgoingMessage, err := buildImageWithoutAlbumIDMessage(*ourChat)
s.NoError(err)
outgoingMessage.Mentioned = true
outgoingMessage.Text = "hey @" + common.PubkeyToHex(&theirMessenger.identity.PublicKey)
album = append(album, outgoingMessage)

View File

@ -5,7 +5,9 @@ import (
"encoding/hex"
"encoding/json"
"errors"
"io/ioutil"
"math/big"
"os"
"strings"
"testing"
"time"
@ -2319,6 +2321,39 @@ func (s *MessengerSuite) TestResendExpiredEmojis() {
s.True(rawMessage.SendCount >= 2)
}
func buildImageWithoutAlbumIDMessage(chat Chat) (*common.Message, error) {
file, err := os.Open("../_assets/tests/test.jpg")
if err != err {
return nil, err
}
defer file.Close()
payload, err := ioutil.ReadAll(file)
if err != err {
return nil, err
}
clock, timestamp := chat.NextClockAndTimestamp(&testTimeSource{})
message := &common.Message{}
message.ChatId = chat.ID
message.Clock = clock
message.Timestamp = timestamp
message.WhisperTimestamp = clock
message.LocalChatID = chat.ID
message.MessageType = protobuf.MessageType_ONE_TO_ONE
message.ContentType = protobuf.ChatMessage_IMAGE
image := protobuf.ImageMessage{
Payload: payload,
Type: protobuf.ImageType_JPEG,
Width: 1200,
Height: 1000,
}
message.Payload = &protobuf.ChatMessage_Image{Image: &image}
return message, nil
}
type testTimeSource struct{}
func (t *testTimeSource) GetCurrentTime() uint64 {