fix: missing messages after changing primary address

Changelog.md

@@ -42,6 +42,7 @@ Changelog [format](http://keepachangelog.com/en/1.0.0/)
 * Use correct binary name when finding location of addcert.scpt
 * GODT-267 Correctly detect if a message is a draft even if does not have DraftLabel
 * GODT-308 reduce minimum read speed threshold to avoid issues with flaky internet
+* GODT-321 Changing address ordering would cause all messages to disappear in combined mode
 
 ## [v1.2.6] Donghai - beta (2020-03-31)
 

internal/bridge/mocks/mocks.go

@@ -5,10 +5,11 @@
 package mocks
 
 import (
+	reflect "reflect"
+
 	credentials "github.com/ProtonMail/proton-bridge/internal/bridge/credentials"
 	pmapi "github.com/ProtonMail/proton-bridge/pkg/pmapi"
 	gomock "github.com/golang/mock/gomock"
-	reflect "reflect"
 )
 
 // MockConfiger is a mock of Configer interface

internal/store/mocks/mocks.go

@@ -5,9 +5,10 @@
 package mocks
 
 import (
+	reflect "reflect"
+
 	pmapi "github.com/ProtonMail/proton-bridge/pkg/pmapi"
 	gomock "github.com/golang/mock/gomock"
-	reflect "reflect"
 )
 
 // MockPanicHandler is a mock of PanicHandler interface

internal/store/mocks/utils_mocks.go

@@ -5,9 +5,10 @@
 package mocks
 
 import (
-	gomock "github.com/golang/mock/gomock"
 	reflect "reflect"
 	time "time"
+
+	gomock "github.com/golang/mock/gomock"
 )
 
 // MockListener is a mock of Listener interface

internal/store/user_address.go

@@ -46,6 +46,8 @@ func (store *Store) RebuildMailboxes() (err error) {
 
 	log.WithField("user", store.UserID()).Trace("Truncating mailboxes")
 
+	store.addresses = nil
+
 	if err = store.truncateMailboxesBucket(); err != nil {
 		log.WithError(err).Error("Could not truncate mailboxes bucket")
 		return
@@ -127,7 +129,12 @@ func (store *Store) createOrDeleteAddressesEvent() (err error) {
 		delete(store.addresses, addr.addressID)
 	}
 
-	return err
+	if err = store.truncateMailboxesBucket(); err != nil {
+		log.WithError(err).Error("Could not truncate mailboxes bucket")
+		return
+	}
+
+	return store.initMailboxesBucket()
 }
 
 // truncateAddressInfoBucket removes the address info bucket.
@@ -153,8 +160,6 @@ func (store *Store) truncateAddressInfoBucket() (err error) {
 func (store *Store) truncateMailboxesBucket() (err error) {
 	log.Trace("Truncating mailboxes bucket")
 
-	store.addresses = nil
-
 	tx := func(tx *bolt.Tx) (err error) {
 		mbs := tx.Bucket(mailboxesBucket)
 

pkg/pmapi/addresses.go

@@ -175,8 +175,6 @@ func (c *client) GetAddresses() (addresses AddressList, err error) {
 }
 
 func (c *client) ReorderAddresses(addressIDs []string) (err error) {
-	defer c.UpdateUser()
-
 	var reqBody struct {
 		AddressIDs []string
 	}
@@ -193,6 +191,8 @@ func (c *client) ReorderAddresses(addressIDs []string) (err error) {
 		return
 	}
 
+	_, err = c.UpdateUser()
+
 	return
 }
 

pkg/pmapi/mocks/mocks.go

@@ -5,11 +5,12 @@
 package mocks
 
 import (
+	io "io"
+	reflect "reflect"
+
 	crypto "github.com/ProtonMail/gopenpgp/crypto"
 	pmapi "github.com/ProtonMail/proton-bridge/pkg/pmapi"
 	gomock "github.com/golang/mock/gomock"
-	io "io"
-	reflect "reflect"
 )
 
 // MockClient is a mock of Client interface
@@ -530,6 +531,20 @@ func (mr *MockClientMockRecorder) MarkMessagesUnread(arg0 interface{}) *gomock.C
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MarkMessagesUnread", reflect.TypeOf((*MockClient)(nil).MarkMessagesUnread), arg0)
 }
 
+// ReorderAddresses mocks base method
+func (m *MockClient) ReorderAddresses(arg0 []string) error {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "ReorderAddresses", arg0)
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+// ReorderAddresses indicates an expected call of ReorderAddresses
+func (mr *MockClientMockRecorder) ReorderAddresses(arg0 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReorderAddresses", reflect.TypeOf((*MockClient)(nil).ReorderAddresses), arg0)
+}
+
 // ReportBugWithEmailClient mocks base method
 func (m *MockClient) ReportBugWithEmailClient(arg0, arg1, arg2, arg3, arg4, arg5, arg6 string) error {
 	m.ctrl.T.Helper()

test/accounts/accounts.go

@@ -55,7 +55,7 @@ func (a *TestAccounts) GetTestAccount(username string) *TestAccount {
 	return a.GetTestAccountWithAddress(username, "")
 }
 
-// GetTestAccount returns the test account with the given username configured to use the given bddAddressID.
+// GetTestAccountWithAddress returns the test account with the given username configured to use the given bddAddressID.
 func (a *TestAccounts) GetTestAccountWithAddress(username, bddAddressID string) *TestAccount {
 	// Do lookup by full address and convert to name in tests.
 	// Used by getting real data to ensure correct address or address ID.

test/bridge_actions_test.go

@@ -142,9 +142,9 @@ func swapsAddressWithAddress(bddUserID, bddAddressID1, bddAddressID2 string) err
 
 	address1ID := account.GetAddressID(bddAddressID1)
 	address2ID := account.GetAddressID(bddAddressID2)
+	addressIDs := make([]string, len(*account.Addresses()))
 
 	var address1Index, address2Index int
-	var addressIDs []string
 	for i, v := range *account.Addresses() {
 		if v.ID == address1ID {
 			address1Index = i
@@ -152,13 +152,12 @@ func swapsAddressWithAddress(bddUserID, bddAddressID1, bddAddressID2 string) err
 		if v.ID == address2ID {
 			address2Index = i
 		}
-		addressIDs = append(addressIDs, v.ID)
+		addressIDs[i] = v.ID
 	}
 
 	addressIDs[address1Index], addressIDs[address2Index] = addressIDs[address2Index], addressIDs[address1Index]
 
 	ctx.ReorderAddresses(account.Username(), bddAddressID1, bddAddressID2)
-	ctx.GetPMAPIController().ReorderAddresses(account.User(), addressIDs)
 
-	return nil
+	return ctx.GetPMAPIController().ReorderAddresses(account.User(), addressIDs)
 }

test/fakeapi/events.go

@@ -113,7 +113,7 @@ func (api *FakePMAPI) addEventAddress(action pmapi.EventAction, address *pmapi.A
 		Addresses: []*pmapi.EventAddress{{
 			EventItem: pmapi.EventItem{
 				ID:     address.ID,
-				Action: pmapi.EventUpdate,
+				Action: action,
 			},
 			Address: address,
 		}},

test/features/bridge/addressmode.feature

@@ -47,9 +47,30 @@ Feature: Address mode
 
   Scenario: Make secondary address primary in combined mode
     Given there is "userMoreAddresses" in "combined" address mode
-    When "userMoreAddresses" swaps address "primary" with address "secondary"
-    And "userMoreAddresses" receives an address event
     Then mailbox "Folders/mbox" for address "primary" of "userMoreAddresses" has messages
       | from              | to          | subject |
       | john.doe@mail.com | [primary]   | foo     |
       | jane.doe@mail.com | [secondary] | bar     |
+    When "userMoreAddresses" swaps address "primary" with address "secondary"
+    And "userMoreAddresses" receives an address event
+    Then mailbox "Folders/mbox" for address "primary" of "userMoreAddresses" has messages
+      | from              | to          | subject |
+      | john.doe@mail.com | [secondary] | foo     |
+      | jane.doe@mail.com | [primary]   | bar     |
+
+  Scenario: Make secondary address primary in split mode
+    Given there is "userMoreAddresses" in "split" address mode
+    Then mailbox "Folders/mbox" for address "primary" of "userMoreAddresses" has messages
+      | from              | to        | subject |
+      | john.doe@mail.com | [primary] | foo     |
+    And mailbox "Folders/mbox" for address "secondary" of "userMoreAddresses" has messages
+      | from              | to          | subject |
+      | jane.doe@mail.com | [secondary] | bar     |
+    When "userMoreAddresses" swaps address "primary" with address "secondary"
+    And "userMoreAddresses" receives an address event
+    Then mailbox "Folders/mbox" for address "primary" of "userMoreAddresses" has messages
+      | from              | to        | subject |
+      | jane.doe@mail.com | [primary] | bar     |
+    And mailbox "Folders/mbox" for address "secondary" of "userMoreAddresses" has messages
+      | from              | to          | subject |
+      | john.doe@mail.com | [secondary] | foo     |