diff --git a/internal/services/imapservice/conflicts.go b/internal/services/imapservice/conflicts.go index 89827cfd..763e84f6 100644 --- a/internal/services/imapservice/conflicts.go +++ b/internal/services/imapservice/conflicts.go @@ -330,59 +330,23 @@ func (r *internalLabelConflictResolverImpl) ResolveConflict(ctx context.Context, logger.Info("Encountered conflict, resolving.") // There is a discrepancy, let's see if it comes from API. - apiLabel, ok := apiLabels[mbox.RemoteID] - if !ok { - // Label does not come from API, we should delete it. - // Due diligence, check if there are any messages associated with the mailbox. - msgCount, _ := r.mailboxMessageCountFetch(ctx, mbox.InternalID) - if msgCount != 0 { - logger.WithField("conflictingLabelMessageCount", msgCount).Info("Non-API conflicting label has associated messages") - - reporterContext["conflictingLabelMessageCount"] = msgCount - if rerr := r.reporter.ReportWarningWithContext("Internal mailbox name conflict. Conflicting non-API label has messages.", - reporterContext); rerr != nil { - logger.WithError(rerr).Error("Failed to send report to sentry") - } - - if !r.allowNonEmptyMailboxDeletion { - return combineIMAPUpdateFns(updateFns), fmt.Errorf("internal mailbox conflicting non-api label has associated messages") - } + _, ok := apiLabels[mbox.RemoteID] + if ok { + // This is a critical issue, we shouldn't have conflicting mailboxes coming from API (system + user labels/folders) + logger.Error("API defined mailbox name conflicts with internal") + if rerr := r.reporter.ReportMessageWithContext("Internal mailbox name conflict. Conflicting with API label.", + reporterContext); rerr != nil { + logger.WithError(rerr).Error("Failed to send report to Sentry") } - fn := func() []imap.Update { - return []imap.Update{imap.NewMailboxDeletedSilent(imap.MailboxID(mbox.RemoteID))} - } - updateFns = append(updateFns, fn) - continue + return combineIMAPUpdateFns(updateFns), fmt.Errorf("internal mailbox conflicting with API label") } - reporterContext["conflictingLabelType"] = apiLabel.Type - - // Label is indeed from API let's see if it's name has changed. - if compareLabelNames(GetMailboxName(apiLabel), internalLabel.Path) { - logger.Error("Conflict, same-name mailbox is returned by API") - - if err := r.reporter.ReportMessageWithContext("Internal mailbox name conflict. Same-name mailbox is returned by API", reporterContext); err != nil { - logger.WithError(err).Error("Could not send report to sentry") - } - - return combineIMAPUpdateFns(updateFns), fmt.Errorf("API label %s conflicts with internal label %s", - GetMailboxName(apiLabel), - strings.Join(mbox.BridgeName, "/"), - ) + fn := func() []imap.Update { + return []imap.Update{imap.NewMailboxDeletedSilent(imap.MailboxID(mbox.RemoteID))} } - - // If it's name has changed then we ought to rename it while still taking care of potential conflicts. - labelRenameUpdates, err := r.userLabelConflictResolver.ResolveConflict(ctx, apiLabel, make(map[string]bool)) - if err != nil { - reporterContext["err"] = err.Error() - if rerr := r.reporter.ReportMessageWithContext("Failed to resolve internal mailbox conflict", reporterContext); rerr != nil { - logger.WithError(rerr).Error("Could not send report to sentry") - } - return combineIMAPUpdateFns(updateFns), - fmt.Errorf("failed to resolve user label conflict for '%s': %w", apiLabel.Name, err) - } - updateFns = append(updateFns, labelRenameUpdates) + updateFns = append(updateFns, fn) } + return combineIMAPUpdateFns(updateFns), nil } diff --git a/internal/services/imapservice/conflicts_test.go b/internal/services/imapservice/conflicts_test.go index 451306e4..3f613000 100644 --- a/internal/services/imapservice/conflicts_test.go +++ b/internal/services/imapservice/conflicts_test.go @@ -807,54 +807,6 @@ func TestInternalLabelConflictResolver_ConflictingNonAPILabel_ZeroCount(t *testi assert.Equal(t, imap.MailboxID("wrong-id"), deleted.MailboxID) } -func TestInternalLabelConflictResolver_ConflictingNonAPILabel_PositiveCount(t *testing.T) { - ctx := context.Background() - - mockLabelProvider := new(mockLabelNameProvider) - mockClient := new(mockAPIClient) - mockIDProvider := new(mockIDProvider) - mockReporter := new(mockReporter) - mockCountProvider := new(mockMailboxCountProvider) - - mockIDProvider.On("GetGluonID", "addr-1").Return("gluon-id-1", true) - - mockReporter.On("ReportWarningWithContext", mock.Anything, mock.Anything). - Return(nil) - - // Mock mailbox fetch to return conflicting mailbox - mockLabelProvider.On("GetUserMailboxByName", mock.Anything, "gluon-id-1", []string{"Folders"}). - Return(imap.MailboxData{RemoteID: "wrong-id", BridgeName: []string{"Folders"}, InternalID: imap.InternalMailboxID(123)}, nil) - mockLabelProvider.On("GetUserMailboxByName", mock.Anything, "gluon-id-1", []string{"Labels"}). - Return(imap.MailboxData{}, db.ErrNotFound) - - // Mock message count fetch to return 0 messages. - mockLabelProvider.On("GetMailboxMessageCount", mock.Anything, "gluon-id-1", imap.InternalMailboxID(123)). - Return(0, nil) - - connector := &imapservice.Connector{} - connector.SetAddrIDTest("addr-1") - mockCountProvider.On("GetUserMailboxCountByInternalID", - mock.Anything, - "gluon-id-1", - imap.InternalMailboxID(123)). - Return(10, nil) - - connector.SetGluonIDProviderTest(mockIDProvider) - connector.SetMailboxCountProviderTest(mockCountProvider) - connectors := []*imapservice.Connector{connector} - - manager := imapservice.NewLabelConflictManager(mockLabelProvider, mockIDProvider, mockClient, mockReporter, ffProviderFalse{}) - resolver := manager.NewInternalLabelConflictResolver(connectors) - - // API labels don't contain the conflicting label ID - apiLabels := make(map[string]proton.Label) - fn, err := resolver.ResolveConflict(ctx, apiLabels) - assert.EqualError(t, err, "internal mailbox conflicting non-api label has associated messages") - - updates := fn() - assert.Empty(t, updates, 0) -} - func TestInternalLabelConflictResolver_ConflictingAPILabelSameName(t *testing.T) { ctx := context.Background() @@ -871,7 +823,7 @@ func TestInternalLabelConflictResolver_ConflictingAPILabelSameName(t *testing.T) mockLabelProvider.On("GetUserMailboxByName", mock.Anything, "gluon-id-1", []string{"Labels"}). Return(imap.MailboxData{}, db.ErrNotFound) - mockReporter.On("ReportMessageWithContext", "Internal mailbox name conflict. Same-name mailbox is returned by API", mock.Anything). + mockReporter.On("ReportMessageWithContext", "Internal mailbox name conflict. Conflicting with API label.", mock.Anything). Return(nil) connector := &imapservice.Connector{} @@ -894,9 +846,7 @@ func TestInternalLabelConflictResolver_ConflictingAPILabelSameName(t *testing.T) } _, err := resolver.ResolveConflict(ctx, apiLabels) - assert.Error(t, err) - assert.Contains(t, err.Error(), "API label") - assert.Contains(t, err.Error(), "conflicts with internal label") + assert.Error(t, err, "internal mailbox conflicting with API label") } func TestInternalLabelConflictResolver_MailboxFetchError(t *testing.T) {