fix(unviewed): fix edge case where unviewed count would be -1 (#3491) (#3496)

VERSION

@@ -1 +1 @@
-0.151.6
+0.151.7

protocol/message_persistence.go

@@ -1726,14 +1726,8 @@ func (db sqlitePersistence) MarkAllRead(chatID string, clock uint64) (int64, int
 
 	_, err = tx.Exec(
 		`UPDATE chats
-		   SET unviewed_message_count =
+		   SET unviewed_message_count = 0,
-		   (SELECT COUNT(1)
+		   unviewed_mentions_count = 0,
-		   FROM user_messages
-		   WHERE local_chat_id = ? AND seen = 0),
-		   unviewed_mentions_count =
-		   (SELECT COUNT(1)
-		   FROM user_messages
-		   WHERE local_chat_id = ? AND seen = 0 AND (mentioned or replied)),
                    highlight = 0
 		WHERE id = ?`, chatID, chatID, chatID)
 

protocol/messenger.go

@@ -4857,7 +4857,7 @@ func (m *Messenger) markAllRead(chatID string, clock uint64, shouldBeSynced bool
 		return errors.New("chat not found")
 	}
 
-	seen, mentioned, err := m.persistence.MarkAllRead(chatID, clock)
+	_, _, err := m.persistence.MarkAllRead(chatID, clock)
 	if err != nil {
 		return err
 	}
@@ -4872,17 +4872,8 @@ func (m *Messenger) markAllRead(chatID string, clock uint64, shouldBeSynced bool
 	chat.ReadMessagesAtClockValue = clock
 	chat.Highlight = false
 
-	if chat.UnviewedMessagesCount >= uint(seen) {
+	chat.UnviewedMessagesCount = 0
-		chat.UnviewedMessagesCount -= uint(seen)
+	chat.UnviewedMentionsCount = 0
-	} else {
-		chat.UnviewedMessagesCount = 0
-	}
-
-	if chat.UnviewedMentionsCount >= uint(mentioned) {
-		chat.UnviewedMentionsCount -= uint(mentioned)
-	} else {
-		chat.UnviewedMentionsCount = 0
-	}
 
 	// TODO(samyoul) remove storing of an updated reference pointer?
 	m.allChats.Store(chat.ID, chat)

protocol/messenger_chats.go

@@ -477,6 +477,7 @@ func (m *Messenger) saveChat(chat *Chat) error {
 		chat.Alias = name
 		chat.Identicon = identicon
 	}
+
 	// Sync chat if it's a new active public chat, but not a timeline chat
 	if !ok && chat.Active && chat.Public() && !chat.ProfileUpdates() && !chat.Timeline() {
 

protocol/messenger_delete_message_test.go

@@ -453,6 +453,68 @@ func (s *MessengerDeleteMessageSuite) TestDeleteMessageWithAMention() {
 	s.Require().Len(response.Chats(), 1)
 	s.Require().Len(response.Messages(), 1)
 	// Receiver (us) is  no longer mentioned
-	s.Require().Equal(int(response.Chats()[0].UnviewedMessagesCount), 0)
+	s.Require().Equal(int(state.Response.Chats()[0].UnviewedMessagesCount), 0)
-	s.Require().Equal(int(response.Chats()[0].UnviewedMentionsCount), 0)
+	s.Require().Equal(int(state.Response.Chats()[0].UnviewedMentionsCount), 0)
+}
+
+// This test makes sure the UnviewMessageCount doesn't go below 0 in a very rare case where the Chat could be marked
+// as read but the message still unseen (Seen == false)
+func (s *MessengerDeleteMessageSuite) TestDeleteMessageAndChatIsAlreadyRead() {
+	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)
+
+	inputMessage := buildTestMessage(*theirChat)
+	sendResponse, err := theirMessenger.SendChatMessage(context.Background(), inputMessage)
+	s.NoError(err)
+	s.Require().Len(sendResponse.Messages(), 1)
+
+	response, err := WaitOnMessengerResponse(
+		s.m,
+		func(r *MessengerResponse) bool { return len(r.messages) == 1 },
+		"no messages",
+	)
+	s.Require().NoError(err)
+	s.Require().Len(response.Chats(), 1)
+	s.Require().Equal(response.Chats()[0].UnviewedMessagesCount, uint(1))
+	s.Require().Len(response.Messages(), 1)
+
+	// Force UnviewedMessagesCount to 0 to test if the uint validation is done correctly
+	ourChat.UnviewedMessagesCount = 0
+	err = s.m.saveChat(ourChat)
+
+	s.Require().NoError(err)
+
+	ogMessage := sendResponse.Messages()[0]
+
+	deleteMessage := DeleteMessage{
+		DeleteMessage: protobuf.DeleteMessage{
+			Clock:       2,
+			MessageType: protobuf.MessageType_ONE_TO_ONE,
+			MessageId:   ogMessage.ID,
+			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)
+	s.Require().Len(response.Chats(), 1)
+	s.Require().Len(response.Messages(), 1)
+	// Receiver (us) no longer has unread messages and it's not negative
+	s.Require().Equal(0, int(state.Response.Chats()[0].UnviewedMessagesCount))
 }

protocol/messenger_handler.go

@@ -1632,8 +1632,10 @@ func (m *Messenger) HandleDeleteMessage(state *ReceivedMessageState, deleteMessa
 		// Reduce chat mention count and unread count if unread
 		if !messageToDelete.Seen && !unreadCountDecreased {
 			unreadCountDecreased = true
-			chat.UnviewedMessagesCount--
+			if chat.UnviewedMessagesCount > 0 {
-			if messageToDelete.Mentioned || messageToDelete.Replied {
+				chat.UnviewedMessagesCount--
+			}
+			if chat.UnviewedMentionsCount > 0 && (messageToDelete.Mentioned || messageToDelete.Replied) {
 				chat.UnviewedMentionsCount--
 			}
 			err := m.saveChat(chat)

protocol/persistence.go

@@ -418,6 +418,9 @@ func (db sqlitePersistence) Chat(chatID string) (*Chat, error) {
 		imagePayload             []byte
 	)
 
+	var unviewedMessagesCount int
+	var unviewedMentionsCount int
+
 	err := db.db.QueryRow(`
 		SELECT
 			id,
@@ -459,8 +462,8 @@ func (db sqlitePersistence) Chat(chatID string) (*Chat, error) {
 		&chat.Timestamp,
 		&chat.ReadMessagesAtClockValue,
 		&chat.DeletedAtClockValue,
-		&chat.UnviewedMessagesCount,
+		&unviewedMessagesCount,
-		&chat.UnviewedMentionsCount,
+		&unviewedMentionsCount,
 		&chat.LastClockValue,
 		&lastMessageBytes,
 		&encodedMembers,
@@ -498,6 +501,18 @@ func (db sqlitePersistence) Chat(chatID string) (*Chat, error) {
 		if profile.Valid {
 			chat.Profile = profile.String
 		}
+
+		// Set UnviewedCounts and make sure they are above 0
+		// Since Chat's UnviewedMessagesCount is uint and the SQL column is INT, it can create a discrepancy
+		if unviewedMessagesCount < 0 {
+			unviewedMessagesCount = 0
+		}
+		if unviewedMentionsCount < 0 {
+			unviewedMentionsCount = 0
+		}
+		chat.UnviewedMessagesCount = uint(unviewedMessagesCount)
+		chat.UnviewedMentionsCount = uint(unviewedMentionsCount)
+
 		// Restore members
 		membersDecoder := gob.NewDecoder(bytes.NewBuffer(encodedMembers))
 		err = membersDecoder.Decode(&chat.Members)