From 798877788c90f86ad1d7f80c8f99a8cd33d75e0f Mon Sep 17 00:00:00 2001 From: kaichao Date: Thu, 11 Jul 2024 19:41:49 +0800 Subject: [PATCH] fix_: delivered message should not send envelope sent signal (#5502) * fix_: delivered message should not send envelope sent signal * chore_: refactor --- protocol/messenger.go | 66 +++++++++++++++++++++----------------- protocol/messenger_test.go | 7 ++-- 2 files changed, 39 insertions(+), 34 deletions(-) diff --git a/protocol/messenger.go b/protocol/messenger.go index 43fa9428e..80a467252 100644 --- a/protocol/messenger.go +++ b/protocol/messenger.go @@ -259,18 +259,26 @@ func (m *Messenger) ResolvePrimaryName(mentionID string) (string, error) { // EnvelopeSent triggered when envelope delivered at least to 1 peer. func (interceptor EnvelopeEventsInterceptor) EnvelopeSent(identifiers [][]byte) { if interceptor.Messenger != nil { - var ids []string + signalIDs := make([][]byte, 0, len(identifiers)) for _, identifierBytes := range identifiers { - ids = append(ids, types.EncodeHex(identifierBytes)) - } + messageID := types.EncodeHex(identifierBytes) + err := interceptor.Messenger.processSentMessage(messageID) + if err != nil { + interceptor.Messenger.logger.Info("messenger failed to process sent messages", zap.Error(err)) + } - err := interceptor.Messenger.processSentMessages(ids) - if err != nil { - interceptor.Messenger.logger.Info("messenger failed to process sent messages", zap.Error(err)) + message, err := interceptor.Messenger.MessageByID(messageID) + if err != nil { + interceptor.Messenger.logger.Error("failed to query message outgoing status", zap.Error(err)) + continue + } + if message.OutgoingStatus == common.OutgoingStatusDelivered { + // We don't want to send the signal if the message was already marked as delivered + continue + } + signalIDs = append(signalIDs, identifierBytes) } - - // We notify the client, regardless whether we were able to mark them as sent - interceptor.EnvelopeEventsHandler.EnvelopeSent(identifiers) + interceptor.EnvelopeEventsHandler.EnvelopeSent(signalIDs) } else { // NOTE(rasom): In case if interceptor.Messenger is not nil and // some error occurred on processing sent message we don't want @@ -682,35 +690,33 @@ func (m *Messenger) EnableBackedupMessagesProcessing() { m.processBackedupMessages = true } -func (m *Messenger) processSentMessages(ids []string) error { +func (m *Messenger) processSentMessage(id string) error { if m.connectionState.Offline { return errors.New("Can't mark message as sent while offline") } - for _, id := range ids { - rawMessage, err := m.persistence.RawMessageByID(id) - // If we have no raw message, we create a temporary one, so that - // the sent status is preserved - if err == sql.ErrNoRows || rawMessage == nil { - rawMessage = &common.RawMessage{ - ID: id, - MessageType: protobuf.ApplicationMetadataMessage_CHAT_MESSAGE, - } - } else if err != nil { - return errors.Wrapf(err, "Can't get raw message with id %v", id) + rawMessage, err := m.persistence.RawMessageByID(id) + // If we have no raw message, we create a temporary one, so that + // the sent status is preserved + if err == sql.ErrNoRows || rawMessage == nil { + rawMessage = &common.RawMessage{ + ID: id, + MessageType: protobuf.ApplicationMetadataMessage_CHAT_MESSAGE, } + } else if err != nil { + return errors.Wrapf(err, "Can't get raw message with id %v", id) + } - rawMessage.Sent = true + rawMessage.Sent = true - err = m.persistence.SaveRawMessage(rawMessage) - if err != nil { - return errors.Wrapf(err, "Can't save raw message marked as sent") - } + err = m.persistence.SaveRawMessage(rawMessage) + if err != nil { + return errors.Wrapf(err, "Can't save raw message marked as sent") + } - err = m.UpdateMessageOutgoingStatus(id, common.OutgoingStatusSent) - if err != nil { - return err - } + err = m.UpdateMessageOutgoingStatus(id, common.OutgoingStatusSent) + if err != nil { + return err } return nil diff --git a/protocol/messenger_test.go b/protocol/messenger_test.go index 80f0505dd..ac085fe53 100644 --- a/protocol/messenger_test.go +++ b/protocol/messenger_test.go @@ -2167,7 +2167,7 @@ func (s *MessengerSuite) TestSentEventTracking() { s.False(rawMessage.Sent) //when message sent, its sent field should be true after we got confirmation - err = s.m.processSentMessages([]string{inputMessage.ID}) + err = s.m.processSentMessage(inputMessage.ID) s.NoError(err) rawMessage, err = s.m.persistence.RawMessageByID(inputMessage.ID) @@ -2392,7 +2392,7 @@ func (s *MessengerSuite) TestMessageSent() { s.False(rawMessage.Sent) //imitate chat message sent - err = s.m.processSentMessages([]string{inputMessage.ID}) + err = s.m.processSentMessage(inputMessage.ID) s.NoError(err) rawMessage, err = s.m.persistence.RawMessageByID(inputMessage.ID) @@ -2402,8 +2402,7 @@ func (s *MessengerSuite) TestMessageSent() { } func (s *MessengerSuite) TestProcessSentMessages() { - ids := []string{"a"} - err := s.m.processSentMessages(ids) + err := s.m.processSentMessage("a") s.Require().NoError(err) }