Skip to content

Commit

Permalink
fix: Remove recursive sync on contact blocking
Browse files Browse the repository at this point in the history
  • Loading branch information
igor-sirotin committed Jul 31, 2023
1 parent bfb5ebc commit 890604d
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 66 deletions.
2 changes: 1 addition & 1 deletion protocol/messenger_backup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,7 @@ func (s *MessengerBackupSuite) TestBackupBlockedContacts() {
actualContacts := bob2.AddedContacts()
s.Require().Equal(actualContacts[0].ID, contactID1)

_, err = bob1.BlockContact(contactID1)
_, err = bob1.BlockContact(contactID1, false)
s.Require().NoError(err)

// Backup
Expand Down
25 changes: 5 additions & 20 deletions protocol/messenger_contact_requests_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1446,7 +1446,7 @@ func (s *MessengerContactRequestSuite) blockContactAndSync(alice1 *Messenger, al
s.Require().NoError(err)

// Alice-1 blocks Bob
_, err = alice1.BlockContact(bobPublicKey)
_, err = alice1.BlockContact(bobPublicKey, false)
s.Require().NoError(err)
s.Require().Len(alice1.BlockedContacts(), 1)
s.Require().Equal(bobPublicKey, alice1.BlockedContacts()[0].ID)
Expand Down Expand Up @@ -1504,26 +1504,11 @@ func (s *MessengerContactRequestSuite) unblockContactAndSync(alice1 *Messenger,
s.Require().NoError(err)
s.Require().Len(alice1.BlockedContacts(), 0)

// Bob wait for a message being unblocked
resp, err := WaitOnMessengerResponse(bob, func(r *MessengerResponse) bool {
// FIXME: When unblocked, we show 'Alice removed you as contact' again
// WARNING: Do we actually expect any messages here?
return len(r.Contacts) == 1 && len(r.Messages()) == 1
}, "bob didn't receive anything after being unblocked")
s.logResponse(resp, "Bob wait for a message being unblocked")
s.Require().NoError(err)
s.Require().NotNil(resp)

s.Require().Len(resp.Contacts, 1)
respContact := resp.Contacts[0]
s.Require().Equal(respContact.ID, alice1.IdentityPublicKeyString())
s.Require().False(respContact.Blocked)
s.Require().False(respContact.Removed)
s.Require().Equal(respContact.ContactRequestLocalState, ContactRequestStateNone)
s.Require().Equal(respContact.ContactRequestRemoteState, ContactRequestStateNone)
// Bob doesn't receive any message on blocking.
// No response wait here.

// Wait for Alice-2 to receive Bob unblocked state
resp, err = WaitOnMessengerResponse(alice2, func(r *MessengerResponse) bool {
resp, err := WaitOnMessengerResponse(alice2, func(r *MessengerResponse) bool {
return len(r.Contacts) == 1
}, "Alice-2 didn't receive Bob unblocked state")
s.logResponse(resp, "Wait for Alice-2 to receive Bob unblocked state")
Expand All @@ -1532,7 +1517,7 @@ func (s *MessengerContactRequestSuite) unblockContactAndSync(alice1 *Messenger,

// Check that Alice-2 has Bob unblocked and removed
s.Require().Len(alice2.Contacts(), 1)
respContact = alice2.Contacts()[0]
respContact := alice2.Contacts()[0]
s.Require().Equal(bobPublicKey, respContact.ID)
s.Require().False(respContact.Blocked)
s.Require().True(respContact.Removed)
Expand Down
89 changes: 49 additions & 40 deletions protocol/messenger_contacts.go
Original file line number Diff line number Diff line change
Expand Up @@ -775,15 +775,17 @@ func (m *Messenger) SetContactLocalNickname(request *requests.SetContactLocalNic
return response, nil
}

func (m *Messenger) blockContact(response *MessengerResponse, contactID string, isDesktopFunc bool) (*Contact, []*Chat, error) {
func (m *Messenger) blockContact(response *MessengerResponse, contactID string, isDesktopFunc bool, syncing bool) error {
contact, err := m.BuildContact(&requests.BuildContact{PublicKey: contactID})
if err != nil {
return nil, nil, err
return err
}

response.AddContact(contact)

_, clock, err := m.getOneToOneAndNextClock(contact)
if err != nil {
return nil, nil, err
return err
}

contact.Block(clock)
Expand All @@ -792,13 +794,10 @@ func (m *Messenger) blockContact(response *MessengerResponse, contactID string,

chats, err := m.persistence.BlockContact(contact, isDesktopFunc)
if err != nil {
return nil, nil, err
return err
}

err = m.sendRetractContactRequest(contact)
if err != nil {
return nil, nil, err
}
response.AddChats(chats)

m.allContacts.Store(contact.ID, contact)
for _, chat := range chats {
Expand All @@ -810,71 +809,81 @@ func (m *Messenger) blockContact(response *MessengerResponse, contactID string,
m.allChats.Delete(buildProfileChatID(contact.ID))
}

err = m.syncContact(context.Background(), contact, m.dispatchMessage)
if err != nil {
return nil, nil, err
}
if !syncing {
err = m.sendRetractContactRequest(contact)
if err != nil {
return err
}

// re-register for push notifications
err = m.reregisterForPushNotifications()
if err != nil {
return nil, nil, err
}
err = m.syncContact(context.Background(), contact, m.dispatchMessage)
if err != nil {
return err
}

// We remove anything that's related to this contact request
notifications, err := m.persistence.DeleteChatContactRequestActivityCenterNotifications(contact.ID, m.getCurrentTimeInMillis())
if err != nil {
return nil, nil, err
// We remove anything that's related to this contact request
notifications, err := m.persistence.DeleteChatContactRequestActivityCenterNotifications(contact.ID, m.getCurrentTimeInMillis())
if err != nil {
return err
}

err = m.syncActivityCenterNotifications(notifications)
if err != nil {
m.logger.Error("BlockContact, error syncing activity center notifications", zap.Error(err))
return err
}
}

err = m.syncActivityCenterNotifications(notifications)
// re-register for push notifications
err = m.reregisterForPushNotifications()
if err != nil {
m.logger.Error("BlockContact, error syncing activity center notifications", zap.Error(err))
return nil, nil, err
return err
}

return contact, chats, nil
return nil
}

func (m *Messenger) BlockContact(contactID string) (*MessengerResponse, error) {
func (m *Messenger) BlockContact(contactID string, syncing bool) (*MessengerResponse, error) {
response := &MessengerResponse{}

contact, chats, err := m.blockContact(response, contactID, false)
err := m.blockContact(response, contactID, false, syncing)
if err != nil {
return nil, err
}
response.AddChats(chats)
response.AddContact(contact)

// WARNING: Move this to private `blockContact`?
response, err = m.DeclineAllPendingGroupInvitesFromUser(response, contactID)
if err != nil {
return nil, err
}

notifications, err := m.persistence.DismissAllActivityCenterNotificationsFromUser(contactID, m.getCurrentTimeInMillis())
if err != nil {
return nil, err
}
// AC notifications are synced separately
// NOTE: Should we still do the local part (persistence.dismiss...) and only skip the syncing?
// This would make the solution
if !syncing {
notifications, err := m.persistence.DismissAllActivityCenterNotificationsFromUser(contactID, m.getCurrentTimeInMillis())
if err != nil {
return nil, err
}

err = m.syncActivityCenterNotifications(notifications)
if err != nil {
m.logger.Error("BlockContact, error syncing activity center notifications", zap.Error(err))
return nil, err
err = m.syncActivityCenterNotifications(notifications)
if err != nil {
m.logger.Error("BlockContact, error syncing activity center notifications", zap.Error(err))
return nil, err
}
}

return response, nil
}

// The same function as the one above.
// Should be removed with https://github.com/status-im/status-desktop/issues/8805
func (m *Messenger) BlockContactDesktop(contactID string) (*MessengerResponse, error) {
response := &MessengerResponse{}

contact, chats, err := m.blockContact(response, contactID, true)
err := m.blockContact(response, contactID, true, false)
if err != nil {
return nil, err
}
response.AddChats(chats)
response.AddContact(contact)

response, err = m.DeclineAllPendingGroupInvitesFromUser(response, contactID)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion protocol/messenger_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ func (m *Messenger) HandleSyncInstallationContact(state *ReceivedMessageState, m
if message.Blocked != contact.Blocked {
if message.Blocked {
state.AllContacts.Store(contact.ID, contact)
response, err := m.BlockContact(contact.ID)
response, err := m.BlockContact(contact.ID, true)
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions protocol/messenger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,7 @@ func (s *MessengerSuite) TestRetrieveBlockedContact() {
}

// Block contact
_, err = s.m.BlockContact(blockedContact.ID)
_, err = s.m.BlockContact(blockedContact.ID, false)
s.Require().NoError(err)

// Blocked contact sends message, we should not receive it
Expand Down Expand Up @@ -1391,7 +1391,7 @@ func (s *MessengerSuite) TestBlockContact() {
err = s.m.SaveMessages(messages)
s.Require().NoError(err)

response, err := s.m.BlockContact(contact.ID)
response, err := s.m.BlockContact(contact.ID, false)
s.Require().NoError(err)

blockedContacts := s.m.BlockedContacts()
Expand Down
2 changes: 1 addition & 1 deletion server/pairing/sync_device_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,7 @@ func (s *SyncDeviceSuite) TestPairingBlockedContact() {
s.checkMutualContact(alice1Messenger, bobPublicKey)

// Block Bob before pairing
_, err = alice1Messenger.BlockContact(bobPublicKey)
_, err = alice1Messenger.BlockContact(bobPublicKey, false)
s.Require().NoError(err)
s.Require().Len(alice1Messenger.BlockedContacts(), 1)
s.Require().Equal(bobPublicKey, alice1Messenger.BlockedContacts()[0].ID)
Expand Down
2 changes: 1 addition & 1 deletion services/ext/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ func (api *PublicAPI) UnmuteChat(parent context.Context, chatID string) error {

func (api *PublicAPI) BlockContact(parent context.Context, contactID string) (*protocol.MessengerResponse, error) {
api.log.Info("blocking contact", "contact", contactID)
return api.service.messenger.BlockContact(contactID)
return api.service.messenger.BlockContact(contactID, false)
}

// This function is the same as the one above, but used only on the desktop side, since at the end it doesn't set
Expand Down

0 comments on commit 890604d

Please sign in to comment.