From 8109b384c583dcb67e773733cd3854e831ae68b3 Mon Sep 17 00:00:00 2001 From: Atanas Janeshliev Date: Tue, 27 May 2025 12:12:30 +0200 Subject: [PATCH] fix(BRIDGE-362): added label conflict reconciliation logic --- COPYING_NOTES.md | 1 + go.mod | 3 +- go.sum | 15 +- internal/services/imapservice/conflicts.go | 211 ++++++ .../services/imapservice/conflicts_test.go | 682 ++++++++++++++++++ internal/services/imapservice/connector.go | 10 +- internal/services/imapservice/imap_updates.go | 19 + .../services/imapservice/server_manager.go | 7 + internal/services/imapservice/service.go | 11 +- .../imapservice/service_address_events.go | 2 +- .../imapservice/service_label_events.go | 32 +- .../imapservice/sync_update_applier.go | 27 +- internal/services/imapsmtpserver/service.go | 4 + internal/unleash/service.go | 9 +- internal/user/user.go | 1 + 15 files changed, 1008 insertions(+), 26 deletions(-) create mode 100644 internal/services/imapservice/conflicts.go create mode 100644 internal/services/imapservice/conflicts_test.go diff --git a/COPYING_NOTES.md b/COPYING_NOTES.md index 1788fbbf..456cd8d2 100644 --- a/COPYING_NOTES.md +++ b/COPYING_NOTES.md @@ -127,6 +127,7 @@ Proton Mail Bridge includes the following 3rd party software: * [blackfriday](https://github.com/russross/blackfriday/v2) available under [license](https://github.com/russross/blackfriday/v2/blob/master/LICENSE) * [pflag](https://github.com/spf13/pflag) available under [license](https://github.com/spf13/pflag/blob/master/LICENSE) * [bom](https://github.com/ssor/bom) available under [license](https://github.com/ssor/bom/blob/master/LICENSE) +* [objx](https://github.com/stretchr/objx) available under [license](https://github.com/stretchr/objx/blob/master/LICENSE) * [golang-asm](https://github.com/twitchyliquid64/golang-asm) available under [license](https://github.com/twitchyliquid64/golang-asm/blob/master/LICENSE) * [codec](https://github.com/ugorji/go/codec) available under [license](https://github.com/ugorji/go/codec/blob/master/LICENSE) * [tagparser](https://github.com/vmihailenco/tagparser/v2) available under [license](https://github.com/vmihailenco/tagparser/v2/blob/master/LICENSE) diff --git a/go.mod b/go.mod index 11720e8c..4a52eaa3 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,7 @@ toolchain go1.24.2 require ( github.com/0xAX/notificator v0.0.0-20220220101646-ee9b8921e557 github.com/Masterminds/semver/v3 v3.2.0 - github.com/ProtonMail/gluon v0.17.1-0.20250528075147-f86b8e45326a + github.com/ProtonMail/gluon v0.17.1-0.20250528125353-9b611f58b753 github.com/ProtonMail/go-autostart v0.0.0-20210130080809-00ed301c8e9a github.com/ProtonMail/go-proton-api v0.4.1-0.20250417134000-e624a080f7ba github.com/ProtonMail/gopenpgp/v2 v2.8.2-proton @@ -114,6 +114,7 @@ require ( github.com/russross/blackfriday/v2 v2.1.0 // indirect github.com/spf13/pflag v1.0.5 // indirect github.com/ssor/bom v0.0.0-20170718123548-6386211fdfcf // indirect + github.com/stretchr/objx v0.5.0 // indirect github.com/twitchyliquid64/golang-asm v0.15.1 // indirect github.com/ugorji/go/codec v1.2.11 // indirect github.com/vmihailenco/tagparser/v2 v2.0.0 // indirect diff --git a/go.sum b/go.sum index fb570303..11d73012 100644 --- a/go.sum +++ b/go.sum @@ -36,10 +36,17 @@ github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAE github.com/ProtonMail/bcrypt v0.0.0-20210511135022-227b4adcab57/go.mod h1:HecWFHognK8GfRDGnFQbW/LiV7A3MX3gZVs45vk5h8I= github.com/ProtonMail/bcrypt v0.0.0-20211005172633-e235017c1baf h1:yc9daCCYUefEs69zUkSzubzjBbL+cmOXgnmt9Fyd9ug= github.com/ProtonMail/bcrypt v0.0.0-20211005172633-e235017c1baf/go.mod h1:o0ESU9p83twszAU8LBeJKFAAMX14tISa0yk4Oo5TOqo= -github.com/ProtonMail/gluon v0.17.1-0.20250516142615-1211bf144af5 h1:7vLMnvLacvKRNDwzdmgKmoWHZqrgauV8LSj9eWvn1jY= -github.com/ProtonMail/gluon v0.17.1-0.20250516142615-1211bf144af5/go.mod h1:0/c03TzZPNiSgY5UDJK1iRDkjlDPwWugxTT6et2qDu8= -github.com/ProtonMail/gluon v0.17.1-0.20250528075147-f86b8e45326a h1:kx9rudQK7JCtgtTLNaMvarYISzgtzAKfLC1g5vEKoQ4= -github.com/ProtonMail/gluon v0.17.1-0.20250528075147-f86b8e45326a/go.mod h1:0/c03TzZPNiSgY5UDJK1iRDkjlDPwWugxTT6et2qDu8= +github.com/ProtonMail/gluon v0.17.1-0.20250513141309-843796a505bc h1:2oppv7H5ZeFnRDohTbLZW5A8I1ylhoX2QEi3RtKxrLE= +github.com/ProtonMail/gluon v0.17.1-0.20250513141309-843796a505bc/go.mod h1:0/c03TzZPNiSgY5UDJK1iRDkjlDPwWugxTT6et2qDu8= +github.com/ProtonMail/gluon v0.17.1-0.20250514104052-2f93fdfc4850 h1:OFMVeakcDS9nHW5kQ/CuBXri84iPBqPgZFHz5Xs/8jo= +github.com/ProtonMail/gluon v0.17.1-0.20250514104052-2f93fdfc4850/go.mod h1:0/c03TzZPNiSgY5UDJK1iRDkjlDPwWugxTT6et2qDu8= +github.com/ProtonMail/gluon v0.17.1-0.20250515084749-4afe6a076ac4 h1:L1JeVS2op3VIcPKctS493+qOBFGhr488mMkYVSLr9eY= +github.com/ProtonMail/gluon v0.17.1-0.20250515084749-4afe6a076ac4/go.mod h1:0/c03TzZPNiSgY5UDJK1iRDkjlDPwWugxTT6et2qDu8= +github.com/ProtonMail/gluon v0.17.1-0.20250516132429-a4b2de331311 h1:8oEkpmF8PD7GyCQjmTto+4yhz4vE1tTT2djL2BgJcBI= +github.com/ProtonMail/gluon v0.17.1-0.20250516132429-a4b2de331311/go.mod h1:0/c03TzZPNiSgY5UDJK1iRDkjlDPwWugxTT6et2qDu8= +github.com/ProtonMail/gluon v0.17.1-0.20250527153202-a7383713882a/go.mod h1:0/c03TzZPNiSgY5UDJK1iRDkjlDPwWugxTT6et2qDu8= +github.com/ProtonMail/gluon v0.17.1-0.20250528125353-9b611f58b753 h1:Zym7WHKLOu1RAUc9b8vkhwEaEU2Gi6MkaurCm7zpK6E= +github.com/ProtonMail/gluon v0.17.1-0.20250528125353-9b611f58b753/go.mod h1:0/c03TzZPNiSgY5UDJK1iRDkjlDPwWugxTT6et2qDu8= github.com/ProtonMail/go-crypto v0.0.0-20230321155629-9a39f2531310/go.mod h1:8TI4H3IbrackdNgv+92dI+rhpCaLqM0IfpgCgenFvRE= github.com/ProtonMail/go-crypto v1.1.4-proton h1:KIo9uNlk3vzlwI7o5VjhiEjI4Ld1TDixOMnoNZyfpFE= github.com/ProtonMail/go-crypto v1.1.4-proton/go.mod h1:zNoyBJW3p/yVWiHNZgfTF9VsjwqYof5YY0M9kt2QaX0= diff --git a/internal/services/imapservice/conflicts.go b/internal/services/imapservice/conflicts.go new file mode 100644 index 00000000..d38290f4 --- /dev/null +++ b/internal/services/imapservice/conflicts.go @@ -0,0 +1,211 @@ +// Copyright (c) 2025 Proton AG +// +// This file is part of Proton Mail Bridge. +// +// Proton Mail Bridge is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Proton Mail Bridge is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Proton Mail Bridge. If not, see . + +package imapservice + +import ( + "context" + "errors" + "fmt" + "strings" + + "github.com/ProtonMail/gluon/db" + "github.com/ProtonMail/gluon/imap" + "github.com/ProtonMail/gluon/reporter" + "github.com/ProtonMail/go-proton-api" + "github.com/ProtonMail/proton-bridge/v3/internal/unleash" + "github.com/sirupsen/logrus" +) + +type GluonLabelNameProvider interface { + GetUserMailboxByName(ctx context.Context, addrID string, labelName []string) (imap.MailboxData, error) +} + +type gluonIDProvider interface { + GetGluonID(addrID string) (string, bool) +} + +type sentryReporter interface { + ReportMessageWithContext(string, reporter.Context) error +} + +type apiClient interface { + GetLabel(ctx context.Context, labelID string, labelTypes ...proton.LabelType) (proton.Label, error) +} + +type mailboxFetcherFn func(ctx context.Context, label proton.Label) (imap.MailboxData, error) + +type LabelConflictManager struct { + gluonLabelNameProvider GluonLabelNameProvider + gluonIDProvider gluonIDProvider + client apiClient + reporter sentryReporter + getFeatureFlagValueFn unleash.GetFlagValueFn +} + +func NewLabelConflictManager( + gluonLabelNameProvider GluonLabelNameProvider, + gluonIDProvider gluonIDProvider, + client apiClient, + reporter sentryReporter, + getFeatureFlagValueFn unleash.GetFlagValueFn) *LabelConflictManager { + return &LabelConflictManager{ + gluonLabelNameProvider: gluonLabelNameProvider, + gluonIDProvider: gluonIDProvider, + client: client, + reporter: reporter, + getFeatureFlagValueFn: getFeatureFlagValueFn, + } +} + +func (m *LabelConflictManager) generateMailboxFetcher(connectors []*Connector) mailboxFetcherFn { + return func(ctx context.Context, label proton.Label) (imap.MailboxData, error) { + for _, updateCh := range connectors { + addrID, ok := m.gluonIDProvider.GetGluonID(updateCh.addrID) + if !ok { + continue + } + return m.gluonLabelNameProvider.GetUserMailboxByName(ctx, addrID, GetMailboxName(label)) + } + return imap.MailboxData{}, errors.New("no gluon connectors found") + } +} + +type LabelConflictResolver interface { + ResolveConflict(ctx context.Context, label proton.Label, visited map[string]bool) (func() []imap.Update, error) +} +type labelConflictResolverImpl struct { + mailboxFetch mailboxFetcherFn + client apiClient + reporter sentryReporter +} + +type nullLabelConflictResolverImpl struct { +} + +func (r *nullLabelConflictResolverImpl) ResolveConflict(_ context.Context, _ proton.Label, _ map[string]bool) (func() []imap.Update, error) { + return func() []imap.Update { + return []imap.Update{} + }, nil +} + +func (m *LabelConflictManager) NewConflictResolver(connectors []*Connector) LabelConflictResolver { + if m.getFeatureFlagValueFn(unleash.LabelConflictResolverDisabled) { + return &nullLabelConflictResolverImpl{} + } + + return &labelConflictResolverImpl{ + mailboxFetch: m.generateMailboxFetcher(connectors), + client: m.client, + reporter: m.reporter, + } +} + +func (r *labelConflictResolverImpl) ResolveConflict(ctx context.Context, label proton.Label, visited map[string]bool) (func() []imap.Update, error) { + var updateFns []func() []imap.Update + + // There's a cycle, such as in a label swap operation, we'll need to temporarily rename the label. + // The change will be overwritten by one of the previous recursive calls. + if visited[label.ID] { + fn := func() []imap.Update { + return []imap.Update{newMailboxUpdatedOrCreated(imap.MailboxID(label.ID), getMailboxNameWithTempPrefix(label))} + } + updateFns = append(updateFns, fn) + return combineIMAPUpdateFns(updateFns), nil + } + visited[label.ID] = true + + // Fetch the gluon mailbox data and verify whether there are conflicts with the name. + mailboxData, err := r.mailboxFetch(ctx, label) + if err != nil { + // Name is free, create the mailbox. + if db.IsErrNotFound(err) { + fn := func() []imap.Update { + return []imap.Update{newMailboxUpdatedOrCreated(imap.MailboxID(label.ID), GetMailboxName(label))} + } + updateFns = append(updateFns, fn) + return combineIMAPUpdateFns(updateFns), nil + } + return combineIMAPUpdateFns(updateFns), err + } + + // Verify whether the label name corresponds to the same label ID. If true terminate, we don't need to update. + if mailboxData.RemoteID == label.ID { + return combineIMAPUpdateFns(updateFns), nil + } + + // If the label name belongs to some other label ID. Fetch it's state from the remote. + conflictingLabel, err := r.client.GetLabel(ctx, mailboxData.RemoteID, proton.LabelTypeFolder, proton.LabelTypeLabel) + if err != nil { + // If it's not present on the remote we should delete it. And create the new label. + if errors.Is(err, proton.ErrNoSuchLabel) { + fn := func() []imap.Update { + return []imap.Update{ + imap.NewMailboxDeleted(imap.MailboxID(mailboxData.RemoteID)), + newMailboxUpdatedOrCreated(imap.MailboxID(label.ID), GetMailboxName(label)), + } + } + updateFns = append(updateFns, fn) + return combineIMAPUpdateFns(updateFns), nil + } + return combineIMAPUpdateFns(updateFns), err + } + + // Check if the conflicting label name has changed. If not, then this is a BE inconsistency. + if compareLabelNames(GetMailboxName(conflictingLabel), mailboxData.BridgeName) { + if err := r.reporter.ReportMessageWithContext("Unexpected label conflict", reporter.Context{ + "labelID": label.ID, + "conflictingLabelID": conflictingLabel.ID, + }); err != nil { + logrus.WithError(err).Error("Failed to report update error") + } + + err := fmt.Errorf("unexpected label conflict: the name of label ID %s is already used by label ID %s", label.ID, conflictingLabel.ID) + return combineIMAPUpdateFns(updateFns), err + } + + // The name of the conflicting label has changed on the remote. We need to verify that the new name does not conflict with anything else. + // Thus, a recursive check can be performed. + childUpdateFns, err := r.ResolveConflict(ctx, conflictingLabel, visited) + if err != nil { + return combineIMAPUpdateFns(updateFns), err + } + updateFns = append(updateFns, childUpdateFns) + + fn := func() []imap.Update { + return []imap.Update{newMailboxUpdatedOrCreated(imap.MailboxID(label.ID), GetMailboxName(label))} + } + updateFns = append(updateFns, fn) + + return combineIMAPUpdateFns(updateFns), nil +} + +func combineIMAPUpdateFns(updateFunctions []func() []imap.Update) func() []imap.Update { + return func() []imap.Update { + var updates []imap.Update + for _, fn := range updateFunctions { + updates = append(updates, fn()...) + } + return updates + } +} + +func compareLabelNames(labelName1, labelName2 []string) bool { + name1 := strings.Join(labelName1, "") + name2 := strings.Join(labelName2, "") + return name1 == name2 +} diff --git a/internal/services/imapservice/conflicts_test.go b/internal/services/imapservice/conflicts_test.go new file mode 100644 index 00000000..9cf654a3 --- /dev/null +++ b/internal/services/imapservice/conflicts_test.go @@ -0,0 +1,682 @@ +// Copyright (c) 2025 Proton AG +// +// This file is part of Proton Mail Bridge. +// +// Proton Mail Bridge is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Proton Mail Bridge is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Proton Mail Bridge. If not, see . + +package imapservice_test + +import ( + "context" + "errors" + "fmt" + "testing" + + "github.com/ProtonMail/gluon/db" + "github.com/ProtonMail/gluon/imap" + "github.com/ProtonMail/gluon/reporter" + "github.com/ProtonMail/go-proton-api" + "github.com/ProtonMail/proton-bridge/v3/internal/services/imapservice" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func getFeatureFlagValueMock(_ string) bool { + return false +} + +type mockLabelNameProvider struct { + mock.Mock +} + +func (m *mockLabelNameProvider) GetUserMailboxByName(ctx context.Context, addrID string, labelName []string) (imap.MailboxData, error) { + args := m.Called(ctx, addrID, labelName) + v, ok := args.Get(0).(imap.MailboxData) + if !ok { + return imap.MailboxData{}, fmt.Errorf("failed to assert type") + } + return v, args.Error(1) +} + +type mockIDProvider struct { + mock.Mock +} + +func (m *mockIDProvider) GetGluonID(addrID string) (string, bool) { + args := m.Called(addrID) + return args.String(0), args.Bool(1) +} + +type mockAPIClient struct { + mock.Mock +} + +func (m *mockAPIClient) GetLabel(ctx context.Context, id string, types ...proton.LabelType) (proton.Label, error) { + args := m.Called(ctx, id, types) + v, ok := args.Get(0).(proton.Label) + if !ok { + return proton.Label{}, fmt.Errorf("failed to assert type") + } + return v, args.Error(1) +} + +type mockReporter struct { + mock.Mock +} + +func (m *mockReporter) ReportMessageWithContext(msg string, ctx reporter.Context) error { + args := m.Called(msg, ctx) + return args.Error(0) +} + +func TestResolveConflict_UnexpectedLabelConflict(t *testing.T) { + ctx := context.Background() + label := proton.Label{ + ID: "label-1", + Path: []string{"Work"}, + Type: proton.LabelTypeLabel, + } + conflictingLabel := proton.Label{ + ID: "label-2", + Path: []string{"Work"}, + Type: proton.LabelTypeLabel, + } + conflictMbox := imap.MailboxData{ + RemoteID: "label-2", + BridgeName: []string{"Labels", "Work"}, + } + + mockLabelProvider := new(mockLabelNameProvider) + mockIDProvider := new(mockIDProvider) + mockClient := new(mockAPIClient) + mockReporter := new(mockReporter) + + mockLabelProvider.On("GetUserMailboxByName", mock.Anything, "gluon-id", imapservice.GetMailboxName(label)). + Return(conflictMbox, nil) + mockIDProvider.On("GetGluonID", "addr-1").Return("gluon-id", true) + mockClient.On("GetLabel", mock.Anything, "label-2", mock.Anything). + Return(conflictingLabel, nil) + mockReporter.On("ReportMessageWithContext", "Unexpected label conflict", mock.Anything). + Return(nil) + + connector := &imapservice.Connector{} + connector.SetAddrIDTest("addr-1") + resolver := imapservice.NewLabelConflictManager(mockLabelProvider, mockIDProvider, mockClient, mockReporter, getFeatureFlagValueMock). + NewConflictResolver([]*imapservice.Connector{connector}) + + visited := make(map[string]bool) + _, err := resolver.ResolveConflict(ctx, label, visited) + assert.Error(t, err) + assert.Contains(t, err.Error(), "unexpected label conflict") +} + +func TestResolveDiscrepancy_LabelDoesNotExist(t *testing.T) { + ctx := context.Background() + label := proton.Label{ + ID: "label-id-1", + Name: "Inbox", + Type: proton.LabelTypeLabel, + } + + mockLabelProvider := new(mockLabelNameProvider) + mockIDProvider := new(mockIDProvider) + mockClient := new(mockAPIClient) + mockReporter := new(mockReporter) + + mockLabelProvider.On("GetUserMailboxByName", mock.Anything, "gluon-id-1", imapservice.GetMailboxName(label)). + Return(imap.MailboxData{}, db.ErrNotFound) + + mockIDProvider.On("GetGluonID", "addr-1").Return("gluon-id-1", true) + + connector := &imapservice.Connector{} + connector.SetAddrIDTest("addr-1") + connectors := []*imapservice.Connector{connector} + + manager := imapservice.NewLabelConflictManager(mockLabelProvider, mockIDProvider, mockClient, mockReporter, getFeatureFlagValueMock) + resolver := manager.NewConflictResolver(connectors) + + visited := make(map[string]bool) + fn, err := resolver.ResolveConflict(ctx, label, visited) + + assert.NoError(t, err) + updates := fn() + assert.Len(t, updates, 1) + muc, ok := updates[0].(*imap.MailboxUpdatedOrCreated) + assert.True(t, ok) + assert.Equal(t, imap.MailboxID(label.ID), muc.Mailbox.ID) +} + +func TestResolveConflict_MailboxFetchError(t *testing.T) { + ctx := context.Background() + label := proton.Label{ + ID: "111", + Path: []string{"Work"}, + Type: proton.LabelTypeLabel, + } + + mockLabelProvider := new(mockLabelNameProvider) + mockIDProvider := new(mockIDProvider) + mockClient := new(mockAPIClient) + mockReporter := new(mockReporter) + + mockLabelProvider.On("GetUserMailboxByName", mock.Anything, "gluon-id", imapservice.GetMailboxName(label)). + Return(imap.MailboxData{}, errors.New("database connection error")) + mockIDProvider.On("GetGluonID", "addr-1").Return("gluon-id", true) + + connector := &imapservice.Connector{} + connector.SetAddrIDTest("addr-1") + resolver := imapservice.NewLabelConflictManager(mockLabelProvider, mockIDProvider, mockClient, mockReporter, getFeatureFlagValueMock). + NewConflictResolver([]*imapservice.Connector{connector}) + + visited := make(map[string]bool) + _, err := resolver.ResolveConflict(ctx, label, visited) + assert.Error(t, err) + assert.Contains(t, err.Error(), "database connection error") +} + +func TestResolveDiscrepancy_ConflictingLabelDeletedRemotely(t *testing.T) { + ctx := context.Background() + label := proton.Label{ + ID: "label-new", + Path: []string{"Work"}, + Type: proton.LabelTypeLabel, + } + conflictMbox := imap.MailboxData{ + RemoteID: "label-old", + BridgeName: []string{"Labels", "Work"}, + } + + mockLabelProvider := new(mockLabelNameProvider) + mockIDProvider := new(mockIDProvider) + mockClient := new(mockAPIClient) + mockReporter := new(mockReporter) + + mockLabelProvider.On("GetUserMailboxByName", mock.Anything, "gluon-id-1", imapservice.GetMailboxName(label)). + Return(conflictMbox, nil) + + mockIDProvider.On("GetGluonID", "addr-1").Return("gluon-id-1", true) + + mockClient.On("GetLabel", mock.Anything, "label-old", mock.Anything). + Return(proton.Label{}, proton.ErrNoSuchLabel) + + connector := &imapservice.Connector{} + connector.SetAddrIDTest("addr-1") + connectors := []*imapservice.Connector{connector} + + manager := imapservice.NewLabelConflictManager(mockLabelProvider, mockIDProvider, mockClient, mockReporter, getFeatureFlagValueMock) + resolver := manager.NewConflictResolver(connectors) + + visited := make(map[string]bool) + fn, err := resolver.ResolveConflict(ctx, label, visited) + + assert.NoError(t, err) + updates := fn() + assert.Len(t, updates, 2) + deleted, ok := updates[0].(*imap.MailboxDeleted) + assert.True(t, ok) + assert.Equal(t, imap.MailboxID("label-old"), deleted.MailboxID) + + updated, ok := updates[1].(*imap.MailboxUpdatedOrCreated) + assert.True(t, ok) + assert.Equal(t, "Work", updated.Mailbox.Name[len(updated.Mailbox.Name)-1]) +} + +func TestResolveDiscrepancy_LabelAlreadyCorrect(t *testing.T) { + ctx := context.Background() + label := proton.Label{ + ID: "label-id-1", + Name: "Personal", + Type: proton.LabelTypeLabel, + } + mbox := imap.MailboxData{ + RemoteID: "label-id-1", + BridgeName: []string{"Labels", "Personal"}, + } + + mockLabelProvider := new(mockLabelNameProvider) + mockIDProvider := new(mockIDProvider) + mockClient := new(mockAPIClient) + mockReporter := new(mockReporter) + + mockLabelProvider.On("GetUserMailboxByName", mock.Anything, "gluon-id-1", imapservice.GetMailboxName(label)). + Return(mbox, nil) + mockIDProvider.On("GetGluonID", "addr-1").Return("gluon-id-1", true) + + connector := &imapservice.Connector{} + connector.SetAddrIDTest("addr-1") + connectors := []*imapservice.Connector{connector} + + manager := imapservice.NewLabelConflictManager(mockLabelProvider, mockIDProvider, mockClient, mockReporter, getFeatureFlagValueMock) + resolver := manager.NewConflictResolver(connectors) + + visited := make(map[string]bool) + fn, err := resolver.ResolveConflict(ctx, label, visited) + + assert.NoError(t, err) + assert.Len(t, fn(), 0) +} + +func TestResolveConflict_DeepNestedPath(t *testing.T) { + ctx := context.Background() + label := proton.Label{ + ID: "111", + Path: []string{"Level1", "Level2", "Level3", "DeepFolder"}, + Type: proton.LabelTypeFolder, + } + + mockLabelProvider := new(mockLabelNameProvider) + mockIDProvider := new(mockIDProvider) + mockClient := new(mockAPIClient) + mockReporter := new(mockReporter) + + mockLabelProvider.On("GetUserMailboxByName", mock.Anything, "gluon-id", imapservice.GetMailboxName(label)). + Return(imap.MailboxData{}, db.ErrNotFound) + mockIDProvider.On("GetGluonID", "addr-1").Return("gluon-id", true) + + connector := &imapservice.Connector{} + connector.SetAddrIDTest("addr-1") + resolver := imapservice.NewLabelConflictManager(mockLabelProvider, mockIDProvider, mockClient, mockReporter, getFeatureFlagValueMock). + NewConflictResolver([]*imapservice.Connector{connector}) + + visited := make(map[string]bool) + fn, err := resolver.ResolveConflict(ctx, label, visited) + + assert.NoError(t, err) + updates := fn() + assert.Len(t, updates, 1) + + updated, ok := updates[0].(*imap.MailboxUpdatedOrCreated) + assert.True(t, ok) + assert.Equal(t, imap.MailboxID("111"), updated.Mailbox.ID) + expectedName := imapservice.GetMailboxName(label) + assert.Equal(t, expectedName, updated.Mailbox.Name) +} + +func TestResolveLabelDiscrepancy_LabelSwap(t *testing.T) { + apiLabels := []proton.Label{ + { + ID: "111", + Path: []string{"X"}, + Type: proton.LabelTypeLabel, + }, + { + ID: "222", + Path: []string{"Y"}, + Type: proton.LabelTypeLabel, + }, + } + + gluonLabels := []imap.MailboxData{ + { + RemoteID: "111", + BridgeName: []string{"Labels", "Y"}, + }, + { + RemoteID: "222", + BridgeName: []string{"Labels", "X"}, + }, + } + + mockLabelProvider := new(mockLabelNameProvider) + mockClient := new(mockAPIClient) + mockIDProvider := new(mockIDProvider) + mockReporter := new(mockReporter) + + mockIDProvider.On("GetGluonID", "addr-1").Return("gluon-id-1", true) + + for _, mbox := range gluonLabels { + mockLabelProvider. + On("GetUserMailboxByName", mock.Anything, "gluon-id-1", mbox.BridgeName). + Return(mbox, nil) + } + + for _, label := range apiLabels { + mockClient. + On("GetLabel", mock.Anything, label.ID, []proton.LabelType{proton.LabelTypeFolder, proton.LabelTypeLabel}). + Return(label, nil) + } + + connector := &imapservice.Connector{} + connector.SetAddrIDTest("addr-1") + connectors := []*imapservice.Connector{connector} + + manager := imapservice.NewLabelConflictManager(mockLabelProvider, mockIDProvider, mockClient, mockReporter, getFeatureFlagValueMock) + resolver := manager.NewConflictResolver(connectors) + + visited := make(map[string]bool) + fn, err := resolver.ResolveConflict(context.Background(), apiLabels[0], visited) + require.NoError(t, err) + + updates := fn() + assert.NotEmpty(t, updates) + assert.Equal(t, 3, len(updates)) // We expect three calls to be made for a swap operation. + + updateOne, ok := updates[0].(*imap.MailboxUpdatedOrCreated) + assert.True(t, ok) + assert.Equal(t, imap.MailboxID(apiLabels[0].ID), updateOne.Mailbox.ID) + assert.Equal(t, "tmp_X", updateOne.Mailbox.Name[len(updateOne.Mailbox.Name)-1]) + + updateTwo, ok := updates[1].(*imap.MailboxUpdatedOrCreated) + assert.True(t, ok) + assert.Equal(t, imap.MailboxID(apiLabels[1].ID), updateTwo.Mailbox.ID) + assert.Equal(t, "Y", updateTwo.Mailbox.Name[len(updateTwo.Mailbox.Name)-1]) + + updateThree, ok := updates[2].(*imap.MailboxUpdatedOrCreated) + assert.True(t, ok) + assert.Equal(t, imap.MailboxID(apiLabels[0].ID), updateThree.Mailbox.ID) + assert.Equal(t, "X", updateThree.Mailbox.Name[len(updateThree.Mailbox.Name)-1]) +} + +func TestResolveLabelDiscrepancy_LabelSwapExtended(t *testing.T) { + apiLabels := []proton.Label{ + { + ID: "111", + Path: []string{"X"}, + Type: proton.LabelTypeLabel, + }, + { + ID: "222", + Path: []string{"Y"}, + Type: proton.LabelTypeLabel, + }, + { + ID: "333", + Path: []string{"Z"}, + Type: proton.LabelTypeLabel, + }, + { + ID: "444", + Path: []string{"D"}, + Type: proton.LabelTypeLabel, + }, + } + + gluonLabels := []imap.MailboxData{ + { + RemoteID: "111", + BridgeName: []string{"Labels", "D"}, + }, + { + RemoteID: "222", + BridgeName: []string{"Labels", "Z"}, + }, + { + RemoteID: "333", + BridgeName: []string{"Labels", "Y"}, + }, + { + RemoteID: "444", + BridgeName: []string{"Labels", "X"}, + }, + } + + mockLabelProvider := new(mockLabelNameProvider) + mockClient := new(mockAPIClient) + mockIDProvider := new(mockIDProvider) + mockReporter := new(mockReporter) + + mockIDProvider.On("GetGluonID", "addr-1").Return("gluon-id-1", true) + + for _, mbox := range gluonLabels { + mockLabelProvider. + On("GetUserMailboxByName", mock.Anything, "gluon-id-1", mbox.BridgeName). + Return(mbox, nil) + } + + for _, label := range apiLabels { + mockClient. + On("GetLabel", mock.Anything, label.ID, []proton.LabelType{proton.LabelTypeFolder, proton.LabelTypeLabel}). + Return(label, nil) + } + + connector := &imapservice.Connector{} + connector.SetAddrIDTest("addr-1") + connectors := []*imapservice.Connector{connector} + + manager := imapservice.NewLabelConflictManager(mockLabelProvider, mockIDProvider, mockClient, mockReporter, getFeatureFlagValueMock) + resolver := manager.NewConflictResolver(connectors) + + fn, err := resolver.ResolveConflict(context.Background(), apiLabels[0], make(map[string]bool)) + require.NoError(t, err) + + updates := fn() + assert.NotEmpty(t, updates) + // Three calls yet again for a swap operation. + assert.Equal(t, 3, len(updates)) + updateOne, ok := updates[0].(*imap.MailboxUpdatedOrCreated) + assert.True(t, ok) + assert.Equal(t, imap.MailboxID(apiLabels[0].ID), updateOne.Mailbox.ID) + assert.Equal(t, "tmp_X", updateOne.Mailbox.Name[len(updateOne.Mailbox.Name)-1]) + + updateTwo, ok := updates[1].(*imap.MailboxUpdatedOrCreated) + assert.True(t, ok) + assert.Equal(t, imap.MailboxID(apiLabels[3].ID), updateTwo.Mailbox.ID) + assert.Equal(t, "D", updateTwo.Mailbox.Name[len(updateTwo.Mailbox.Name)-1]) + + updateThree, ok := updates[2].(*imap.MailboxUpdatedOrCreated) + assert.True(t, ok) + assert.Equal(t, imap.MailboxID(apiLabels[0].ID), updateThree.Mailbox.ID) + assert.Equal(t, "X", updateThree.Mailbox.Name[len(updateThree.Mailbox.Name)-1]) + + // Fix the secondary swap. + fn, err = resolver.ResolveConflict(context.Background(), apiLabels[1], make(map[string]bool)) + require.NoError(t, err) + + updates = fn() + assert.Equal(t, 3, len(updates)) + updateOne, ok = updates[0].(*imap.MailboxUpdatedOrCreated) + assert.True(t, ok) + assert.Equal(t, imap.MailboxID(apiLabels[1].ID), updateOne.Mailbox.ID) + assert.Equal(t, "tmp_Y", updateOne.Mailbox.Name[len(updateOne.Mailbox.Name)-1]) + + updateTwo, ok = updates[1].(*imap.MailboxUpdatedOrCreated) + assert.True(t, ok) + assert.Equal(t, imap.MailboxID(apiLabels[2].ID), updateTwo.Mailbox.ID) + assert.Equal(t, "Z", updateTwo.Mailbox.Name[len(updateTwo.Mailbox.Name)-1]) + + updateThree, ok = updates[2].(*imap.MailboxUpdatedOrCreated) + assert.True(t, ok) + assert.Equal(t, imap.MailboxID(apiLabels[1].ID), updateThree.Mailbox.ID) + assert.Equal(t, "Y", updateThree.Mailbox.Name[len(updateThree.Mailbox.Name)-1]) +} + +func TestResolveLabelDiscrepancy_LabelSwapCyclic(t *testing.T) { + apiLabels := []proton.Label{ + {ID: "111", Path: []string{"A"}, Type: proton.LabelTypeLabel}, + {ID: "222", Path: []string{"B"}, Type: proton.LabelTypeLabel}, + {ID: "333", Path: []string{"C"}, Type: proton.LabelTypeLabel}, + {ID: "444", Path: []string{"D"}, Type: proton.LabelTypeLabel}, + } + + gluonLabels := []imap.MailboxData{ + {RemoteID: "111", BridgeName: []string{"Labels", "D"}}, // A <- D + {RemoteID: "222", BridgeName: []string{"Labels", "A"}}, // B <- A + {RemoteID: "333", BridgeName: []string{"Labels", "B"}}, // C <- B + {RemoteID: "444", BridgeName: []string{"Labels", "C"}}, // D <- C + } + + mockLabelProvider := new(mockLabelNameProvider) + mockClient := new(mockAPIClient) + mockIDProvider := new(mockIDProvider) + mockReporter := new(mockReporter) + + mockIDProvider.On("GetGluonID", "addr-1").Return("gluon-id-1", true) + + for _, mbox := range gluonLabels { + mockLabelProvider. + On("GetUserMailboxByName", mock.Anything, "gluon-id-1", mbox.BridgeName). + Return(mbox, nil) + } + + for _, label := range apiLabels { + mockClient. + On("GetLabel", mock.Anything, label.ID, []proton.LabelType{proton.LabelTypeFolder, proton.LabelTypeLabel}). + Return(label, nil) + } + + connector := &imapservice.Connector{} + connector.SetAddrIDTest("addr-1") + connectors := []*imapservice.Connector{connector} + + manager := imapservice.NewLabelConflictManager(mockLabelProvider, mockIDProvider, mockClient, mockReporter, getFeatureFlagValueMock) + resolver := manager.NewConflictResolver(connectors) + + fn, err := resolver.ResolveConflict(context.Background(), apiLabels[0], make(map[string]bool)) + require.NoError(t, err) + + updates := fn() + assert.NotEmpty(t, updates) + assert.Equal(t, 5, len(updates)) + + updateOne, ok := updates[0].(*imap.MailboxUpdatedOrCreated) + assert.True(t, ok) + assert.Equal(t, imap.MailboxID(apiLabels[0].ID), updateOne.Mailbox.ID) + assert.Equal(t, "tmp_A", updateOne.Mailbox.Name[len(updateOne.Mailbox.Name)-1]) + + updateTwo, ok := updates[1].(*imap.MailboxUpdatedOrCreated) + assert.True(t, ok) + assert.Equal(t, imap.MailboxID(apiLabels[3].ID), updateTwo.Mailbox.ID) + assert.Equal(t, "D", updateTwo.Mailbox.Name[len(updateTwo.Mailbox.Name)-1]) + + updateThree, ok := updates[2].(*imap.MailboxUpdatedOrCreated) + assert.True(t, ok) + assert.Equal(t, imap.MailboxID(apiLabels[2].ID), updateThree.Mailbox.ID) + assert.Equal(t, "C", updateThree.Mailbox.Name[len(updateThree.Mailbox.Name)-1]) + + updateFour, ok := updates[3].(*imap.MailboxUpdatedOrCreated) + assert.True(t, ok) + assert.Equal(t, imap.MailboxID(apiLabels[1].ID), updateFour.Mailbox.ID) + assert.Equal(t, "B", updateFour.Mailbox.Name[len(updateFour.Mailbox.Name)-1]) + + updateFive, ok := updates[4].(*imap.MailboxUpdatedOrCreated) + assert.True(t, ok) + assert.Equal(t, imap.MailboxID(apiLabels[0].ID), updateFive.Mailbox.ID) + assert.Equal(t, "A", updateFive.Mailbox.Name[len(updateFive.Mailbox.Name)-1]) +} + +func TestResolveLabelDiscrepancy_LabelSwapCyclicWithDeletedLabel(t *testing.T) { + apiLabels := []proton.Label{ + {ID: "111", Path: []string{"A"}, Type: proton.LabelTypeLabel}, + {ID: "333", Path: []string{"C"}, Type: proton.LabelTypeLabel}, + {ID: "444", Path: []string{"D"}, Type: proton.LabelTypeLabel}, + } + + gluonLabels := []imap.MailboxData{ + {RemoteID: "111", BridgeName: []string{"Labels", "D"}}, + {RemoteID: "222", BridgeName: []string{"Labels", "A"}}, + {RemoteID: "333", BridgeName: []string{"Labels", "B"}}, + {RemoteID: "444", BridgeName: []string{"Labels", "C"}}, + } + + mockLabelProvider := new(mockLabelNameProvider) + mockClient := new(mockAPIClient) + mockIDProvider := new(mockIDProvider) + mockReporter := new(mockReporter) + + mockIDProvider.On("GetGluonID", "addr-1").Return("gluon-id-1", true) + + for _, mbox := range gluonLabels { + mockLabelProvider. + On("GetUserMailboxByName", mock.Anything, "gluon-id-1", mbox.BridgeName). + Return(mbox, nil) + } + + for _, label := range apiLabels { + mockClient. + On("GetLabel", mock.Anything, label.ID, []proton.LabelType{proton.LabelTypeFolder, proton.LabelTypeLabel}). + Return(label, nil) + } + mockClient.On("GetLabel", mock.Anything, "222", []proton.LabelType{proton.LabelTypeFolder, proton.LabelTypeLabel}).Return(proton.Label{}, proton.ErrNoSuchLabel) + + connector := &imapservice.Connector{} + connector.SetAddrIDTest("addr-1") + connectors := []*imapservice.Connector{connector} + + manager := imapservice.NewLabelConflictManager(mockLabelProvider, mockIDProvider, mockClient, mockReporter, getFeatureFlagValueMock) + resolver := manager.NewConflictResolver(connectors) + + fn, err := resolver.ResolveConflict(context.Background(), apiLabels[2], make(map[string]bool)) + require.NoError(t, err) + + updates := fn() + assert.NotEmpty(t, updates) + assert.Equal(t, 3, len(updates)) + + updateOne, ok := updates[0].(*imap.MailboxDeleted) + assert.True(t, ok) + assert.Equal(t, imap.MailboxID("222"), updateOne.MailboxID) + + updateTwo, ok := updates[1].(*imap.MailboxUpdatedOrCreated) + assert.True(t, ok) + assert.Equal(t, imap.MailboxID(apiLabels[0].ID), updateTwo.Mailbox.ID) + assert.Equal(t, "A", updateTwo.Mailbox.Name[len(updateTwo.Mailbox.Name)-1]) + + updateThree, ok := updates[2].(*imap.MailboxUpdatedOrCreated) + assert.True(t, ok) + assert.Equal(t, imap.MailboxID(apiLabels[2].ID), updateThree.Mailbox.ID) + assert.Equal(t, "D", updateThree.Mailbox.Name[len(updateThree.Mailbox.Name)-1]) +} + +func TestResolveLabelDiscrepancy_LabelSwapCyclicWithDeletedLabel_KillSwitchEnabled(t *testing.T) { + apiLabels := []proton.Label{ + {ID: "111", Path: []string{"A"}, Type: proton.LabelTypeLabel}, + {ID: "333", Path: []string{"C"}, Type: proton.LabelTypeLabel}, + {ID: "444", Path: []string{"D"}, Type: proton.LabelTypeLabel}, + } + + gluonLabels := []imap.MailboxData{ + {RemoteID: "111", BridgeName: []string{"Labels", "D"}}, + {RemoteID: "222", BridgeName: []string{"Labels", "A"}}, + {RemoteID: "333", BridgeName: []string{"Labels", "B"}}, + {RemoteID: "444", BridgeName: []string{"Labels", "C"}}, + } + + mockLabelProvider := new(mockLabelNameProvider) + mockClient := new(mockAPIClient) + mockIDProvider := new(mockIDProvider) + mockReporter := new(mockReporter) + + mockIDProvider.On("GetGluonID", "addr-1").Return("gluon-id-1", true) + + for _, mbox := range gluonLabels { + mockLabelProvider. + On("GetUserMailboxByName", mock.Anything, "gluon-id-1", mbox.BridgeName). + Return(mbox, nil) + } + + for _, label := range apiLabels { + mockClient. + On("GetLabel", mock.Anything, label.ID, []proton.LabelType{proton.LabelTypeFolder, proton.LabelTypeLabel}). + Return(label, nil) + } + mockClient.On("GetLabel", mock.Anything, "222", []proton.LabelType{proton.LabelTypeFolder, proton.LabelTypeLabel}).Return(proton.Label{}, proton.ErrNoSuchLabel) + + connector := &imapservice.Connector{} + connector.SetAddrIDTest("addr-1") + connectors := []*imapservice.Connector{connector} + + getFeatureFlagFn := func(_ string) bool { + return true + } + + manager := imapservice.NewLabelConflictManager(mockLabelProvider, mockIDProvider, mockClient, mockReporter, getFeatureFlagFn) + resolver := manager.NewConflictResolver(connectors) + + fn, err := resolver.ResolveConflict(context.Background(), apiLabels[2], make(map[string]bool)) + require.NoError(t, err) + + updates := fn() + assert.Empty(t, updates) +} diff --git a/internal/services/imapservice/connector.go b/internal/services/imapservice/connector.go index e5b29670..826569c1 100644 --- a/internal/services/imapservice/connector.go +++ b/internal/services/imapservice/connector.go @@ -800,8 +800,10 @@ func (s *Connector) createDraftWithParser(ctx context.Context, parser *parser.Pa return draft, nil } -func (s *Connector) publishUpdate(_ context.Context, update imap.Update) { - s.updateCh.Enqueue(update) +func (s *Connector) publishUpdate(_ context.Context, updates ...imap.Update) { + for _, update := range updates { + s.updateCh.Enqueue(update) + } } func fixGODT3003Labels( @@ -903,3 +905,7 @@ func (s *Connector) getSenderProtonAddress(p *parser.Parser) (proton.Address, er return addressList[index], nil } + +func (s *Connector) SetAddrIDTest(addrID string) { + s.addrID = addrID +} diff --git a/internal/services/imapservice/imap_updates.go b/internal/services/imapservice/imap_updates.go index cc7c01cd..c6379e2f 100644 --- a/internal/services/imapservice/imap_updates.go +++ b/internal/services/imapservice/imap_updates.go @@ -102,6 +102,16 @@ func newMailboxCreatedUpdate(labelID imap.MailboxID, labelName []string) *imap.M }) } +func newMailboxUpdatedOrCreated(labelID imap.MailboxID, labelName []string) *imap.MailboxUpdatedOrCreated { + return imap.NewMailboxUpdatedOrCreated(imap.Mailbox{ + ID: labelID, + Name: labelName, + Flags: defaultMailboxFlags(), + PermanentFlags: defaultMailboxPermanentFlags(), + Attributes: imap.NewFlagSet(), + }) +} + func GetMailboxName(label proton.Label) []string { var name []string @@ -122,3 +132,12 @@ func GetMailboxName(label proton.Label) []string { return name } + +func nameWithTempPrefix(path []string) []string { + path[len(path)-1] = "tmp_" + path[len(path)-1] + return path +} + +func getMailboxNameWithTempPrefix(label proton.Label) []string { + return nameWithTempPrefix(GetMailboxName(label)) +} diff --git a/internal/services/imapservice/server_manager.go b/internal/services/imapservice/server_manager.go index ad2dbd33..fbeea542 100644 --- a/internal/services/imapservice/server_manager.go +++ b/internal/services/imapservice/server_manager.go @@ -21,6 +21,7 @@ import ( "context" "github.com/ProtonMail/gluon/connector" + "github.com/ProtonMail/gluon/imap" "github.com/ProtonMail/proton-bridge/v3/internal/services/syncservice" ) @@ -37,6 +38,8 @@ type IMAPServerManager interface { LogRemoteLabelIDs(ctx context.Context, provider GluonIDProvider, addrID ...string) error + GetUserMailboxByName(ctx context.Context, addrID string, mailboxName []string) (imap.MailboxData, error) + GetOpenIMAPSessionCount() int GetRollingIMAPConnectionCount() int } @@ -70,6 +73,10 @@ func (n NullIMAPServerManager) LogRemoteLabelIDs( return nil } +func (n NullIMAPServerManager) GetUserMailboxByName(_ context.Context, _ string, _ []string) (imap.MailboxData, error) { + return imap.MailboxData{}, nil +} + func (n NullIMAPServerManager) GetOpenIMAPSessionCount() int { return 0 } diff --git a/internal/services/imapservice/service.go b/internal/services/imapservice/service.go index bc0ea368..475a4f3f 100644 --- a/internal/services/imapservice/service.go +++ b/internal/services/imapservice/service.go @@ -36,6 +36,7 @@ import ( "github.com/ProtonMail/proton-bridge/v3/internal/services/syncservice" "github.com/ProtonMail/proton-bridge/v3/internal/services/userevents" "github.com/ProtonMail/proton-bridge/v3/internal/services/useridentity" + "github.com/ProtonMail/proton-bridge/v3/internal/unleash" "github.com/ProtonMail/proton-bridge/v3/internal/usertypes" "github.com/ProtonMail/proton-bridge/v3/pkg/cpc" "github.com/sirupsen/logrus" @@ -91,7 +92,8 @@ type Service struct { lastHandledEventID string isSyncing atomic.Bool - observabilitySender observability.Sender + observabilitySender observability.Sender + labelConflictManager *LabelConflictManager } func NewService( @@ -112,6 +114,7 @@ func NewService( maxSyncMemory uint64, showAllMail bool, observabilitySender observability.Sender, + getFeatureFlagValueFn unleash.GetFlagValueFn, ) *Service { subscriberName := fmt.Sprintf("imap-%v", identityState.User.ID) @@ -121,7 +124,8 @@ func NewService( }) rwIdentity := newRWIdentity(identityState, bridgePassProvider, keyPassProvider) - syncUpdateApplier := NewSyncUpdateApplier() + labelConflictManager := NewLabelConflictManager(serverManager, gluonIDProvider, client, reporter, getFeatureFlagValueFn) + syncUpdateApplier := NewSyncUpdateApplier(labelConflictManager) syncMessageBuilder := NewSyncMessageBuilder(rwIdentity) syncReporter := newSyncReporter(identityState.User.ID, eventPublisher, time.Second) @@ -156,7 +160,8 @@ func NewService( syncReporter: syncReporter, syncConfigPath: GetSyncConfigPath(syncConfigDir, identityState.User.ID), - observabilitySender: observabilitySender, + observabilitySender: observabilitySender, + labelConflictManager: labelConflictManager, } } diff --git a/internal/services/imapservice/service_address_events.go b/internal/services/imapservice/service_address_events.go index 75117b8e..e1eb4e36 100644 --- a/internal/services/imapservice/service_address_events.go +++ b/internal/services/imapservice/service_address_events.go @@ -165,7 +165,7 @@ func addNewAddressSplitMode(ctx context.Context, s *Service, addrID string) erro s.connectors[connector.addrID] = connector - updates, err := syncLabels(ctx, s.labels.GetLabelMap(), []*Connector{connector}) + updates, err := syncLabels(ctx, s.labels.GetLabelMap(), []*Connector{connector}, s.labelConflictManager) if err != nil { return fmt.Errorf("failed to create labels updates for new address: %w", err) } diff --git a/internal/services/imapservice/service_label_events.go b/internal/services/imapservice/service_label_events.go index 899c5f62..b6450dc0 100644 --- a/internal/services/imapservice/service_label_events.go +++ b/internal/services/imapservice/service_label_events.go @@ -42,7 +42,10 @@ func (s *Service) HandleLabelEvents(ctx context.Context, events []proton.LabelEv continue } - updates := onLabelCreated(ctx, s, event) + updates, err := onLabelCreated(ctx, s, event) + if err != nil { + return fmt.Errorf("failed to handle create label event: %w", err) + } if err := waitOnIMAPUpdates(ctx, updates); err != nil { return err @@ -74,8 +77,8 @@ func (s *Service) HandleLabelEvents(ctx context.Context, events []proton.LabelEv return nil } -func onLabelCreated(ctx context.Context, s *Service, event proton.LabelEvent) []imap.Update { - updates := make([]imap.Update, 0, len(s.connectors)) +func onLabelCreated(ctx context.Context, s *Service, event proton.LabelEvent) ([]imap.Update, error) { + updates := []imap.Update{} s.log.WithFields(logrus.Fields{ "labelID": event.ID, @@ -87,7 +90,17 @@ func onLabelCreated(ctx context.Context, s *Service, event proton.LabelEvent) [] wr.SetLabel(event.Label.ID, event.Label, "onLabelCreated") + labelConflictResolver := s.labelConflictManager.NewConflictResolver(maps.Values(s.connectors)) + conflictUpdatesGenerator, err := labelConflictResolver.ResolveConflict(ctx, event.Label, make(map[string]bool)) + if err != nil { + return updates, err + } + for _, updateCh := range maps.Values(s.connectors) { + conflictUpdates := conflictUpdatesGenerator() + updateCh.publishUpdate(ctx, conflictUpdates...) + updates = append(updates, conflictUpdates...) + update := newMailboxCreatedUpdate(imap.MailboxID(event.ID), GetMailboxName(event.Label)) updateCh.publishUpdate(ctx, update) updates = append(updates, update) @@ -99,7 +112,7 @@ func onLabelCreated(ctx context.Context, s *Service, event proton.LabelEvent) [] Name: event.Label.Name, }) - return updates + return updates, nil } func onLabelUpdated(ctx context.Context, s *Service, event proton.LabelEvent) ([]imap.Update, error) { @@ -136,8 +149,19 @@ func onLabelUpdated(ctx context.Context, s *Service, event proton.LabelEvent) ([ // Update the label in the map. wr.SetLabel(apiLabel.ID, apiLabel, "onLabelUpdatedApiID") + // Resolve potential conflicts + labelConflictResolver := s.labelConflictManager.NewConflictResolver(maps.Values(s.connectors)) + conflictUpdatesGenerator, err := labelConflictResolver.ResolveConflict(ctx, event.Label, make(map[string]bool)) + if err != nil { + return updates, err + } + // Notify the IMAP clients. for _, updateCh := range maps.Values(s.connectors) { + conflictUpdates := conflictUpdatesGenerator() + updateCh.publishUpdate(ctx, conflictUpdates...) + updates = append(updates, conflictUpdates...) + update := imap.NewMailboxUpdated( imap.MailboxID(apiLabel.ID), GetMailboxName(apiLabel), diff --git a/internal/services/imapservice/sync_update_applier.go b/internal/services/imapservice/sync_update_applier.go index eda913a0..2d1611bf 100644 --- a/internal/services/imapservice/sync_update_applier.go +++ b/internal/services/imapservice/sync_update_applier.go @@ -31,8 +31,9 @@ import ( ) type SyncUpdateApplier struct { - requestCh chan updateRequest - replyCh chan updateReply + requestCh chan updateRequest + replyCh chan updateReply + labelConflictManager *LabelConflictManager } type updateReply struct { @@ -42,10 +43,11 @@ type updateReply struct { type updateRequest = func(ctx context.Context, mode usertypes.AddressMode, connectors map[string]*Connector) ([]imap.Update, error) -func NewSyncUpdateApplier() *SyncUpdateApplier { +func NewSyncUpdateApplier(labelConflictManager *LabelConflictManager) *SyncUpdateApplier { return &SyncUpdateApplier{ - requestCh: make(chan updateRequest), - replyCh: make(chan updateReply), + requestCh: make(chan updateRequest), + replyCh: make(chan updateReply), + labelConflictManager: labelConflictManager, } } @@ -113,7 +115,7 @@ func (s *SyncUpdateApplier) ApplySyncUpdates(ctx context.Context, updates []sync func (s *SyncUpdateApplier) SyncLabels(ctx context.Context, labels map[string]proton.Label) error { request := func(ctx context.Context, _ usertypes.AddressMode, connectors map[string]*Connector) ([]imap.Update, error) { - return syncLabels(ctx, labels, maps.Values(connectors)) + return syncLabels(ctx, labels, maps.Values(connectors), s.labelConflictManager) } updates, err := s.sendRequest(ctx, request) @@ -128,9 +130,11 @@ func (s *SyncUpdateApplier) SyncLabels(ctx context.Context, labels map[string]pr } // nolint:exhaustive -func syncLabels(ctx context.Context, labels map[string]proton.Label, connectors []*Connector) ([]imap.Update, error) { +func syncLabels(ctx context.Context, labels map[string]proton.Label, connectors []*Connector, labelConflictManager *LabelConflictManager) ([]imap.Update, error) { var updates []imap.Update + labelConflictResolver := labelConflictManager.NewConflictResolver(connectors) + // Create placeholder Folders/Labels mailboxes with the \Noselect attribute. for _, prefix := range []string{folderPrefix, labelPrefix} { for _, updateCh := range connectors { @@ -155,7 +159,16 @@ func syncLabels(ctx context.Context, labels map[string]proton.Label, connectors } case proton.LabelTypeFolder, proton.LabelTypeLabel: + conflictUpdatesGenerator, err := labelConflictResolver.ResolveConflict(ctx, label, make(map[string]bool)) + if err != nil { + return updates, err + } + for _, updateCh := range connectors { + conflictUpdates := conflictUpdatesGenerator() + updateCh.publishUpdate(ctx, conflictUpdates...) + updates = append(updates, conflictUpdates...) + update := newMailboxCreatedUpdate(imap.MailboxID(labelID), GetMailboxName(label)) updateCh.publishUpdate(ctx, update) updates = append(updates, update) diff --git a/internal/services/imapsmtpserver/service.go b/internal/services/imapsmtpserver/service.go index 4a8c753f..c745cc81 100644 --- a/internal/services/imapsmtpserver/service.go +++ b/internal/services/imapsmtpserver/service.go @@ -204,6 +204,10 @@ func (sm *Service) RemoveSMTPAccount(ctx context.Context, service *bridgesmtp.Se return err } +func (sm *Service) GetUserMailboxByName(ctx context.Context, addrID string, mailboxName []string) (imap.MailboxData, error) { + return sm.imapServer.GetUserMailboxByName(ctx, addrID, mailboxName) +} + func (sm *Service) GetOpenIMAPSessionCount() int { return sm.imapServer.GetOpenSessionCount() } diff --git a/internal/unleash/service.go b/internal/unleash/service.go index 7c74ceda..fadc4979 100644 --- a/internal/unleash/service.go +++ b/internal/unleash/service.go @@ -37,10 +37,11 @@ var pollJitter = 2 * time.Minute //nolint:gochecknoglobals const filename = "unleash_flags" const ( - EventLoopNotificationDisabled = "InboxBridgeEventLoopNotificationDisabled" - IMAPAuthenticateCommandDisabled = "InboxBridgeImapAuthenticateCommandDisabled" - UserRemovalGluonDataCleanupDisabled = "InboxBridgeUserRemovalGluonDataCleanupDisabled" - UpdateUseNewVersionFileStructureDisabled = "InboxBridgeUpdateWithOsFilterDisabled" + EventLoopNotificationDisabled = "InboxBridgeEventLoopNotificationDisabled" + IMAPAuthenticateCommandDisabled = "InboxBridgeImapAuthenticateCommandDisabled" + UserRemovalGluonDataCleanupDisabled = "InboxBridgeUserRemovalGluonDataCleanupDisabled" + UpdateUseNewVersionFileStructureDisabled = "InboxBridgeUpdateWithOsFilterDisabled" + LabelConflictResolverDisabled = "InboxBridgeLabelConflictResolverDisabled" SMTPSubmissionRequestSentryReportDisabled = "InboxBridgeSmtpSubmissionRequestSentryReportDisabled" ) diff --git a/internal/user/user.go b/internal/user/user.go index 702abf1a..926b729c 100644 --- a/internal/user/user.go +++ b/internal/user/user.go @@ -284,6 +284,7 @@ func newImpl( user.maxSyncMemory, showAllMail, observabilityService, + getFlagValueFn, ) user.notificationService = notifications.NewService(user.id, user.eventService, user, notificationStore, featureFlagValueProvider, observabilityService)