Skip to content

Commit

Permalink
fix(sync)_: sync fallback notification (#5888)
Browse files Browse the repository at this point in the history 
* fix(sync)_: Improve EnableInstallationAndSync and add EnableInstallationV2

- Refactor EnableInstallationAndSync for better error handling and response merging
- Add new EnableInstallationV2 method returning the installation
- Update tests to check for installation in response
- Deprecate old EnableInstallation method

* chore_: remove EnableInstallationV2

* fix(sync)_: create/delete AC notification only when targetInstallationID match

- Add targetInstallationID parameter to SendPairInstallation function
- Update protobuf SyncPairInstallation struct with TargetInstallationId field
- Modify method calls across multiple test files to include new parameter
- Update pairing.proto and pairing.pb.go with new field for local pairing

* chore_: rename stubEnableInstallationAndPair

chore_: move InstallationIDProvider

chore_: revert endpoints.go

test_: check AC with resp

chore_: rename ModifiedInstallationsTargetedToThisDevice

test_: add InstallationIDProvider

chore_: revert endpoints.go

chore_: remove comment

test_: add TestNewInstallationCreatedIsNotDeleted
  • Loading branch information
@qfrank
qfrank authored Oct 7, 2024
1 parent 94ff99d commit a08319f
Show file tree
Hide file tree
Showing 24 changed files with 251 additions and 108 deletions.
51 changes: 28 additions & 23 deletions protocol/messenger.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3147,9 +3147,10 @@ type ReceivedMessageState struct {
// List of contacts modified
ModifiedContacts *stringBoolMap
// All installations in memory
AllInstallations *installationMap
// List of communities modified
AllInstallations *installationMap
ModifiedInstallations *stringBoolMap
// List of installations targeted to this device modified
TargetedInstallations *stringBoolMap
// Map of existing messages
ExistingMessagesMap map[string]bool
// EmojiReactions is a list of emoji reactions for the current batch
Expand Down Expand Up @@ -3322,6 +3323,7 @@ func (m *Messenger) buildMessageState() *ReceivedMessageState {
ModifiedContacts: new(stringBoolMap),
AllInstallations: m.allInstallations,
ModifiedInstallations: m.modifiedInstallations,
TargetedInstallations: new(stringBoolMap),
ExistingMessagesMap: make(map[string]bool),
EmojiReactions: make(map[string]*EmojiReaction),
GroupChatInvitations: make(map[string]*GroupChatInvitation),
Expand Down Expand Up @@ -3735,28 +3737,31 @@ func (m *Messenger) saveDataAndPrepareResponse(messageState *ReceivedMessageStat
}
}

if installation.Enabled {
// Delete AC notif since the installation is now enabled
err = m.deleteNotification(messageState.Response, id)
if err != nil {
m.logger.Error("error deleting notification", zap.Error(err))
return false
}
} else if id != m.installationID {
// Add activity center notification when we receive a new installation
notification := &ActivityCenterNotification{
ID: types.FromHex(id),
Type: ActivityCenterNotificationTypeNewInstallationReceived,
InstallationID: id,
Timestamp: m.getTimesource().GetCurrentTime(),
Read: false,
Deleted: false,
UpdatedAt: m.GetCurrentTimeInMillis(),
}
targeted, _ := messageState.TargetedInstallations.Load(id)
if targeted {
if installation.Enabled {
// Delete AC notif since the installation is now enabled
err = m.deleteNotification(messageState.Response, id)
if err != nil {
m.logger.Error("error deleting notification", zap.Error(err))
return false
}
} else if id != m.installationID {
// Add activity center notification when we receive a new installation
notification := &ActivityCenterNotification{
ID: types.FromHex(id),
Type: ActivityCenterNotificationTypeNewInstallationReceived,
InstallationID: id,
Timestamp: m.getTimesource().GetCurrentTime(),
Read: false,
Deleted: false,
UpdatedAt: m.GetCurrentTimeInMillis(),
}

err = m.addActivityCenterNotification(messageState.Response, notification, nil)
if err != nil {
return false
err = m.addActivityCenterNotification(messageState.Response, notification, nil)
if err != nil {
return false
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions protocol/messenger_delete_message_for_me_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func (s *MessengerDeleteMessageForMeSuite) Pair() {
DeviceType: "alice2",
})
s.Require().NoError(err)
response, err := s.alice2.SendPairInstallation(context.Background(), nil)
response, err := s.alice2.SendPairInstallation(context.Background(), "", nil)
s.Require().NoError(err)
s.Require().NotNil(response)
s.Require().Len(response.Chats(), 1)
Expand All @@ -101,7 +101,7 @@ func (s *MessengerDeleteMessageForMeSuite) Pair() {
s.Require().Equal("alice2", actualInstallation.InstallationMetadata.Name)
s.Require().Equal("alice2", actualInstallation.InstallationMetadata.DeviceType)

err = s.alice1.EnableInstallation(s.alice2.installationID)
_, err = s.alice1.EnableInstallation(s.alice2.installationID)
s.Require().NoError(err)
}

Expand Down
2 changes: 2 additions & 0 deletions protocol/messenger_handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1321,6 +1321,8 @@ func (m *Messenger) HandleSyncPairInstallation(state *ReceivedMessageState, mess
// TODO(samyoul) remove storing of an updated reference pointer?
state.AllInstallations.Store(message.InstallationId, installation)
state.ModifiedInstallations.Store(message.InstallationId, true)
targeted := message.TargetInstallationId == m.installationID
state.TargetedInstallations.Store(message.InstallationId, targeted)

return nil
}
Expand Down
4 changes: 2 additions & 2 deletions protocol/messenger_identity_display_name_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func (s *MessengerProfileDisplayNameHandlerSuite) TestDisplayNameSync() {
}
err = alicesOtherDevice.SetInstallationMetadata(alicesOtherDevice.installationID, im1)
s.Require().NoError(err)
response, err := alicesOtherDevice.SendPairInstallation(context.Background(), nil)
response, err := alicesOtherDevice.SendPairInstallation(context.Background(), "", nil)
s.Require().NoError(err)
s.Require().NotNil(response)
s.Require().Len(response.Chats(), 1)
Expand All @@ -133,7 +133,7 @@ func (s *MessengerProfileDisplayNameHandlerSuite) TestDisplayNameSync() {
s.Require().Equal("alice's-other-device", actualInstallation.InstallationMetadata.Name)
s.Require().Equal("alice's-other-device-type", actualInstallation.InstallationMetadata.DeviceType)

err = s.m.EnableInstallation(alicesOtherDevice.installationID)
_, err = s.m.EnableInstallation(alicesOtherDevice.installationID)
s.Require().NoError(err)

// Set new display name on alice's device
Expand Down
12 changes: 6 additions & 6 deletions protocol/messenger_installations_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func (s *MessengerInstallationSuite) TestReceiveInstallation() {
DeviceType: "their-device-type",
})
s.Require().NoError(err)
response, err := theirMessenger.SendPairInstallation(context.Background(), nil)
response, err := theirMessenger.SendPairInstallation(context.Background(), "", nil)
s.Require().NoError(err)
s.Require().NotNil(response)
s.Require().Len(response.Chats(), 1)
Expand All @@ -63,7 +63,7 @@ func (s *MessengerInstallationSuite) TestReceiveInstallation() {
s.Require().Equal("their-name", actualInstallation.InstallationMetadata.Name)
s.Require().Equal("their-device-type", actualInstallation.InstallationMetadata.DeviceType)

err = s.m.EnableInstallation(theirMessenger.installationID)
_, err = s.m.EnableInstallation(theirMessenger.installationID)
s.Require().NoError(err)

contactKey, err := crypto.GenerateKey()
Expand Down Expand Up @@ -242,7 +242,7 @@ func (s *MessengerInstallationSuite) TestSyncInstallation() {
DeviceType: "their-device-type",
})
s.Require().NoError(err)
response, err = theirMessenger.SendPairInstallation(context.Background(), nil)
response, err = theirMessenger.SendPairInstallation(context.Background(), "", nil)
s.Require().NoError(err)
s.Require().NotNil(response)
s.Require().Len(response.Chats(), 1)
Expand All @@ -262,7 +262,7 @@ func (s *MessengerInstallationSuite) TestSyncInstallation() {
s.Require().Equal("their-name", actualInstallation.InstallationMetadata.Name)
s.Require().Equal("their-device-type", actualInstallation.InstallationMetadata.DeviceType)

err = s.m.EnableInstallation(theirMessenger.installationID)
_, err = s.m.EnableInstallation(theirMessenger.installationID)
s.Require().NoError(err)

// sync
Expand Down Expand Up @@ -366,7 +366,7 @@ func (s *MessengerInstallationSuite) TestSyncInstallationNewMessages() {
DeviceType: "their-device-type",
})
s.Require().NoError(err)
response, err := bob2.SendPairInstallation(context.Background(), nil)
response, err := bob2.SendPairInstallation(context.Background(), "", nil)
s.Require().NoError(err)
s.Require().NotNil(response)
s.Require().Len(response.Chats(), 1)
Expand All @@ -382,7 +382,7 @@ func (s *MessengerInstallationSuite) TestSyncInstallationNewMessages() {
s.Require().NoError(err)
actualInstallation := response.Installations()[0]
s.Require().Equal(bob2.installationID, actualInstallation.ID)
err = bob1.EnableInstallation(bob2.installationID)
_, err = bob1.EnableInstallation(bob2.installationID)
s.Require().NoError(err)

// send a message from bob1 to alice, it should be received on both bob1 and bob2
Expand Down
64 changes: 40 additions & 24 deletions protocol/messenger_pairing_and_syncing.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,42 +17,55 @@ import (
"github.com/status-im/status-go/protocol/requests"
)

type InstallationIDProvider interface {
GetInstallationID() string
Validate() error
}

func (m *Messenger) EnableInstallationAndSync(request *requests.EnableInstallationAndSync) (*MessengerResponse, error) {
if err := request.Validate(); err != nil {
return nil, err
}
err := m.EnableInstallation(request.InstallationID)

installation, err := m.EnableInstallation(request.InstallationID)
if err != nil {
return nil, err
}
response, err := m.SendPairInstallation(context.Background(), nil)

response := &MessengerResponse{}
response.AddInstallation(installation)

pairResponse, err := m.SendPairInstallation(context.Background(), request.InstallationID, nil)
if err != nil {
return nil, err
}
err = m.SyncDevices(context.Background(), "", "", nil)
if err != nil {

if err = m.SyncDevices(context.Background(), "", "", nil); err != nil {
return nil, err
}

// Delete AC notif
err = m.deleteNotification(response, request.InstallationID)
if err != nil {
if err = m.deleteNotification(pairResponse, request.InstallationID); err != nil {
return nil, err
}

if err = pairResponse.Merge(response); err != nil {
return nil, err
}

return response, nil
return pairResponse, nil
}

func (m *Messenger) EnableInstallationAndPair(request *requests.EnableInstallationAndPair) (*MessengerResponse, error) {
func (m *Messenger) EnableInstallationAndPair(request InstallationIDProvider) (*MessengerResponse, error) {
if err := request.Validate(); err != nil {
return nil, err
}

myIdentity := crypto.CompressPubkey(&m.identity.PublicKey)
timestamp := time.Now().UnixNano()
installationID := request.GetInstallationID()

installation := &multidevice.Installation{
ID: request.InstallationID,
ID: installationID,
Enabled: true,
Version: 2,
Timestamp: timestamp,
Expand All @@ -62,20 +75,20 @@ func (m *Messenger) EnableInstallationAndPair(request *requests.EnableInstallati
if err != nil {
return nil, err
}
i, ok := m.allInstallations.Load(request.InstallationID)
i, ok := m.allInstallations.Load(installationID)
if !ok {
i = installation
} else {
i.Enabled = true
}
m.allInstallations.Store(request.InstallationID, i)
response, err := m.SendPairInstallation(context.Background(), nil)
m.allInstallations.Store(installationID, i)
response, err := m.SendPairInstallation(context.Background(), request.GetInstallationID(), nil)
if err != nil {
return nil, err
}

notification := &ActivityCenterNotification{
ID: types.FromHex(request.InstallationID),
ID: types.FromHex(installationID),
Type: ActivityCenterNotificationTypeNewInstallationCreated,
InstallationID: m.installationID, // Put our own installation ID, as we're the initiator of the pairing
Timestamp: m.getTimesource().GetCurrentTime(),
Expand All @@ -92,7 +105,7 @@ func (m *Messenger) EnableInstallationAndPair(request *requests.EnableInstallati
}

// SendPairInstallation sends a pair installation message
func (m *Messenger) SendPairInstallation(ctx context.Context, rawMessageHandler RawMessageHandler) (*MessengerResponse, error) {
func (m *Messenger) SendPairInstallation(ctx context.Context, targetInstallationID string, rawMessageHandler RawMessageHandler) (*MessengerResponse, error) {
var err error
var response MessengerResponse

Expand All @@ -108,11 +121,13 @@ func (m *Messenger) SendPairInstallation(ctx context.Context, rawMessageHandler
clock, chat := m.getLastClockWithRelatedChat()

pairMessage := &protobuf.SyncPairInstallation{
Clock: clock,
Name: installation.InstallationMetadata.Name,
InstallationId: installation.ID,
DeviceType: installation.InstallationMetadata.DeviceType,
Version: installation.Version}
Clock: clock,
Name: installation.InstallationMetadata.Name,
InstallationId: installation.ID,
DeviceType: installation.InstallationMetadata.DeviceType,
Version: installation.Version,
TargetInstallationId: targetInstallationID,
}
encodedMessage, err := proto.Marshal(pairMessage)
if err != nil {
return nil, err
Expand Down Expand Up @@ -496,20 +511,21 @@ func (m *Messenger) SetInstallationName(id string, name string) error {
return m.encryptor.SetInstallationName(m.IdentityPublicKey(), id, name)
}

func (m *Messenger) EnableInstallation(id string) error {
// EnableInstallation enables an installation and returns the installation
func (m *Messenger) EnableInstallation(id string) (*multidevice.Installation, error) {
installation, ok := m.allInstallations.Load(id)
if !ok {
return errors.New("no installation found")
return nil, errors.New("no installation found")
}

err := m.encryptor.EnableInstallation(&m.identity.PublicKey, id)
if err != nil {
return err
return nil, err
}
installation.Enabled = true
// TODO(samyoul) remove storing of an updated reference pointer?
m.allInstallations.Store(id, installation)
return nil
return installation, nil
}

func (m *Messenger) DisableInstallation(id string) error {
Expand Down
Loading

0 comments on commit a08319f

Please sign in to comment.