Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

14044 handle edit messages in bridge #4983

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
0.177.0
0.177.1
55 changes: 51 additions & 4 deletions protocol/message_persistence.go
Original file line number Diff line number Diff line change
Expand Up @@ -1551,6 +1551,18 @@ func (db sqlitePersistence) SaveMessages(messages []*common.Message) (err error)
}

if msg.ContentType == protobuf.ChatMessage_BRIDGE_MESSAGE {
// check updates first
var hasMessage bool
hasMessage, err = db.bridgeMessageExists(tx, msg.GetBridgeMessage().MessageID)
if err != nil {
return
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case of err, maybe we should just ignore and save the message as before?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, if it returned the error, it would mean that there is something wrong with db connection and another db operation would fail also. Am I right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't mean return the error. I mean just not return. Because since you return, you will miss that message

}
if hasMessage {
// bridge message exists, this is edit
err = db.updateBridgeMessageContent(tx, msg.GetBridgeMessage().MessageID, msg.GetBridgeMessage().Content)
return
}

err = db.saveBridgeMessage(tx, msg.GetBridgeMessage(), msg.ID)
if err != nil {
return
Expand Down Expand Up @@ -2967,9 +2979,26 @@ func (db sqlitePersistence) findStatusMessageIdsReplies(tx *sql.Tx, bridgeMessag
return statusMessageIDs, nil
}

// Finds status messages id which are replies for bridgeMessageID
func (db sqlitePersistence) findStatusMessageIdsRepliedTo(tx *sql.Tx, parentMessageID string) (string, error) {
rows, err := tx.Query(`SELECT user_messages_id FROM bridge_messages WHERE message_id = ?`, parentMessageID)
func (db sqlitePersistence) FindStatusMessageIdForBridgeMessageId(messageID string) (string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean it could get slower over time? Excuse my ignorance, are the messages split per chats or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, we query from bridge messages.
Bridge messages have a separated table. They are joined with messages via a messageId.
I did not investigate how much time does it take.

rows, err := db.db.Query(`SELECT user_messages_id FROM bridge_messages WHERE message_id = ?`, messageID)
if err != nil {
return "", err
}
defer rows.Close()

if rows.Next() {
var statusMessageID string
err = rows.Scan(&statusMessageID)
if err != nil {
return "", err
}
return statusMessageID, nil
}
return "", nil
}

func (db sqlitePersistence) findStatusMessageIdForBridgeMessageId(tx *sql.Tx, messageID string) (string, error) {
rows, err := tx.Query(`SELECT user_messages_id FROM bridge_messages WHERE message_id = ?`, messageID)
if err != nil {
return "", err
}
Expand Down Expand Up @@ -3003,6 +3032,23 @@ func (db sqlitePersistence) updateStatusMessagesWithResponse(tx *sql.Tx, statusM
return err
}

func (db sqlitePersistence) bridgeMessageExists(tx *sql.Tx, bridgeMessageID string) (exists bool, err error) {
err = tx.QueryRow(`SELECT EXISTS(SELECT 1 FROM bridge_messages WHERE message_id = ?)`, bridgeMessageID).Scan(&exists)
return exists, err
}

func (db sqlitePersistence) updateBridgeMessageContent(tx *sql.Tx, bridgeMessageID string, content string) error {
sql := "UPDATE bridge_messages SET content = ? WHERE message_id = ?"
stmt, err := tx.Prepare(sql)
if err != nil {
return err
}
defer stmt.Close()

_, err = stmt.Exec(content, bridgeMessageID)
return err
}

// Finds if there are any messages that are replies to that message (in case replies were received earlier)
func (db sqlitePersistence) findAndUpdateReplies(tx *sql.Tx, bridgeMessageID string, statusMessageID string) error {
replyMessageIds, err := db.findStatusMessageIdsReplies(tx, bridgeMessageID)
Expand All @@ -3016,7 +3062,8 @@ func (db sqlitePersistence) findAndUpdateReplies(tx *sql.Tx, bridgeMessageID str
}

func (db sqlitePersistence) findAndUpdateRepliedTo(tx *sql.Tx, discordParentMessageID string, statusMessageID string) error {
repliedMessageID, err := db.findStatusMessageIdsRepliedTo(tx, discordParentMessageID)
// Finds status messages id which are replies for bridgeMessageID
repliedMessageID, err := db.findStatusMessageIdForBridgeMessageId(tx, discordParentMessageID)
if err != nil {
return err
}
Expand Down
4 changes: 4 additions & 0 deletions protocol/messenger.go
Original file line number Diff line number Diff line change
Expand Up @@ -6042,3 +6042,7 @@ func (m *Messenger) startMessageSegmentsCleanupLoop() {
}
}()
}

func (m *Messenger) FindStatusMessageIdForBridgeMessageId(bridgeMessageID string) (string, error) {
return m.persistence.FindStatusMessageIdForBridgeMessageId(bridgeMessageID)
}
74 changes: 74 additions & 0 deletions protocol/messenger_edit_message_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,80 @@ func (s *MessengerEditMessageSuite) TestEditMessage() {
s.Require().Equal(ErrInvalidEditOrDeleteAuthor, err)
}

func (s *MessengerEditMessageSuite) TestEditBridgeMessage() {
theirMessenger := s.newMessenger()
defer TearDownMessenger(&s.Suite, theirMessenger)

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)

bridgeMessage := buildTestMessage(*theirChat)
bridgeMessage.ContentType = protobuf.ChatMessage_BRIDGE_MESSAGE
bridgeMessage.Payload = &protobuf.ChatMessage_BridgeMessage{
BridgeMessage: &protobuf.BridgeMessage{
BridgeName: "discord",
UserName: "user1",
UserAvatar: "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAADIAAAAyCAIAAACRXR/mAAAAjklEQVR4nOzXwQmFMBAAUZXUYh32ZB32ZB02sxYQQSZGsod55/91WFgSS0RM+SyjA56ZRZhFmEWYRRT6h+M6G16zrxv6fdJpmUWYRbxsYr13dKfanpN0WmYRZhGzXz6AWYRZRIfbaX26fT9Jk07LLMIsosPt9I/dTDotswizCG+nhFmEWYRZhFnEHQAA///z1CFkYamgfQAAAABJRU5ErkJggg==",
UserID: "123",
Content: "text1",
MessageID: "456",
ParentMessageID: "789",
},
}

sendResponse, err := theirMessenger.SendChatMessage(context.Background(), bridgeMessage)
s.NoError(err)
s.Require().Len(sendResponse.Messages(), 1)

response, err := WaitOnMessengerResponse(
s.m,
func(r *MessengerResponse) bool { return len(r.messages) > 0 },
"no messages",
)
s.Require().NoError(err)
s.Require().Len(response.Chats(), 1)
s.Require().Len(response.Messages(), 1)

messageToEdit := sendResponse.Messages()[0]

messageID, err := types.DecodeHex(messageToEdit.ID)
s.Require().NoError(err)

editedText := "edited text"
editedMessage := &requests.EditMessage{
ID: messageID,
Text: editedText,
ContentType: protobuf.ChatMessage_BRIDGE_MESSAGE,
}

sendResponse, err = theirMessenger.EditMessage(context.Background(), editedMessage)
s.Require().NoError(err)
s.Require().Len(sendResponse.Messages(), 1)
s.Require().NotEmpty(sendResponse.Messages()[0].EditedAt)
s.Require().Equal(sendResponse.Messages()[0].Text, "text-input-message")
s.Require().Equal(sendResponse.Messages()[0].GetBridgeMessage().Content, editedText)
s.Require().Len(sendResponse.Chats(), 1)
s.Require().NotNil(sendResponse.Chats()[0].LastMessage)
s.Require().NotEmpty(sendResponse.Chats()[0].LastMessage.EditedAt)

response, err = WaitOnMessengerResponse(
s.m,
func(r *MessengerResponse) bool { return len(r.messages) > 0 },
"no messages",
)
s.Require().NoError(err)
s.Require().Len(response.Chats(), 1)
s.Require().Len(response.Messages(), 1)

s.Require().NotEmpty(response.Chats()[0].LastMessage.EditedAt)
s.Require().Equal(response.Messages()[0].GetBridgeMessage().Content, "edited text")
}

func (s *MessengerEditMessageSuite) TestEditMessageEdgeCases() {
theirMessenger := s.newMessenger()
defer TearDownMessenger(&s.Suite, theirMessenger)
Expand Down
10 changes: 8 additions & 2 deletions protocol/messenger_messages.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func (m *Messenger) EditMessage(ctx context.Context, request *requests.EditMessa
return nil, ErrInvalidEditOrDeleteAuthor
}

if message.ContentType != protobuf.ChatMessage_TEXT_PLAIN && message.ContentType != protobuf.ChatMessage_EMOJI && message.ContentType != protobuf.ChatMessage_IMAGE {
if message.ContentType != protobuf.ChatMessage_TEXT_PLAIN && message.ContentType != protobuf.ChatMessage_EMOJI && message.ContentType != protobuf.ChatMessage_IMAGE && message.ContentType != protobuf.ChatMessage_BRIDGE_MESSAGE {
return nil, ErrInvalidEditContentType
}

Expand Down Expand Up @@ -373,7 +373,13 @@ func (m *Messenger) applyEditMessage(editMessage *protobuf.EditMessage, message
if err := ValidateText(editMessage.Text); err != nil {
return err
}
message.Text = editMessage.Text

if editMessage.ContentType != protobuf.ChatMessage_BRIDGE_MESSAGE {
message.Text = editMessage.Text
} else {
message.GetBridgeMessage().Content = editMessage.Text
}

message.EditedAt = editMessage.Clock
message.UnfurledLinks = editMessage.UnfurledLinks
message.UnfurledStatusLinks = editMessage.UnfurledStatusLinks
Expand Down