feat(BRIDGE-391): Simplified internal label conflict resolver

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]
+		_, ok := apiLabels[mbox.RemoteID]
-		if !ok {
+		if ok {
-			// Label does not come from API, we should delete it.
+			// This is a critical issue, we shouldn't have conflicting mailboxes coming from API (system + user labels/folders)
-			// Due diligence, check if there are any messages associated with the mailbox.
+			logger.Error("API defined mailbox name conflicts with internal")
-			msgCount, _ := r.mailboxMessageCountFetch(ctx, mbox.InternalID)
+			if rerr := r.reporter.ReportMessageWithContext("Internal mailbox name conflict. Conflicting with API label.",
-			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")
+				logger.WithError(rerr).Error("Failed to send report to Sentry")
 			}
 
-				if !r.allowNonEmptyMailboxDeletion {
+			return combineIMAPUpdateFns(updateFns), fmt.Errorf("internal mailbox conflicting with API label")
-					return combineIMAPUpdateFns(updateFns), fmt.Errorf("internal mailbox conflicting non-api label has associated messages")
-				}
 		}
 
 		fn := func() []imap.Update {
 			return []imap.Update{imap.NewMailboxDeletedSilent(imap.MailboxID(mbox.RemoteID))}
 		}
 		updateFns = append(updateFns, fn)
-			continue
 	}
 
-		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, "/"),
-			)
-		}
-
-		// 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)
-	}
 	return combineIMAPUpdateFns(updateFns), nil
 }

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.Error(t, err, "internal mailbox conflicting with API label")
-	assert.Contains(t, err.Error(), "API label")
-	assert.Contains(t, err.Error(), "conflicts with internal label")
 }
 
 func TestInternalLabelConflictResolver_MailboxFetchError(t *testing.T) {