This is an automated email from the ASF dual-hosted git repository. chibenwa pushed a commit to branch 3.9.x in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 4a424f5f6d6f5e5326af0565a3b6a4d444c02ef0 Author: Benoit TELLIER <[email protected]> AuthorDate: Wed Jun 17 14:42:10 2026 +0200 [ENHANCEMENT] Add a autoMerge mode (default to false) to SolveMailboxInconsistencies --- .../mail/task/MailboxMergingTaskRunner.java | 31 ++++- ...lveMailboxInconsistenciesRunningOptionsDTO.java | 59 +++++++++ .../task/SolveMailboxInconsistenciesService.java | 134 +++++++++++++++++---- .../SolveMailboxInconsistenciesServiceTest.java | 57 ++++++++- ...ailboxInconsistenciesTaskSerializationTest.java | 6 +- .../SolveMailboxInconsistenciesRequestToTask.java | 12 +- 6 files changed, 260 insertions(+), 39 deletions(-) diff --git a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/task/MailboxMergingTaskRunner.java b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/task/MailboxMergingTaskRunner.java index 478fcebdbf..06bde8bf61 100644 --- a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/task/MailboxMergingTaskRunner.java +++ b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/task/MailboxMergingTaskRunner.java @@ -19,6 +19,8 @@ package org.apache.james.mailbox.cassandra.mail.task; +import java.util.function.Function; + import jakarta.inject.Inject; import org.apache.james.core.Username; @@ -66,19 +68,36 @@ public class MailboxMergingTaskRunner { } public Task.Result run(CassandraId oldMailboxId, CassandraId newMailboxId, MailboxMergingTask.Context context) { + return runReactive(oldMailboxId, newMailboxId, context) + .block(); + } + + public Mono<Task.Result> runReactive(CassandraId oldMailboxId, CassandraId newMailboxId, MailboxMergingTask.Context context) { return moveMessages(oldMailboxId, newMailboxId, mailboxSession, context) - .onComplete( - () -> mergeRights(oldMailboxId, newMailboxId).block(), - () -> mailboxDAO.delete(oldMailboxId).block()); + .flatMap(onMoveCompleteOperations(oldMailboxId, newMailboxId)); } - private Task.Result moveMessages(CassandraId oldMailboxId, CassandraId newMailboxId, MailboxSession session, MailboxMergingTask.Context context) { + private Function<Task.Result, Mono<Task.Result>> onMoveCompleteOperations(CassandraId oldMailboxId, CassandraId newMailboxId) { + return result -> { + if (result == Task.Result.COMPLETED) { + return mergeRights(oldMailboxId, newMailboxId) + .then(mailboxDAO.delete(oldMailboxId)) + .thenReturn(result) + .onErrorResume(e -> { + LOGGER.error("Error while executing move completion operation", e); + return Mono.just(Task.Result.PARTIAL); + }); + } + return Mono.just(result); + }; + } + + private Mono<Task.Result> moveMessages(CassandraId oldMailboxId, CassandraId newMailboxId, MailboxSession session, MailboxMergingTask.Context context) { return cassandraMessageIdDAO.retrieveMessages(oldMailboxId, MessageRange.all(), Limit.unlimited()) .map(CassandraMessageMetadata::getComposedMessageId) .map(ComposedMessageIdWithMetaData::getComposedMessageId) .concatMap(messageId -> Mono.fromCallable(() -> moveMessage(newMailboxId, messageId, session, context)).subscribeOn(ReactorUtils.BLOCKING_CALL_WRAPPER)) - .reduce(Task.Result.COMPLETED, Task::combine) - .block(); + .reduce(Task.Result.COMPLETED, Task::combine); } private Task.Result moveMessage(CassandraId newMailboxId, ComposedMessageId composedMessageId, MailboxSession session, MailboxMergingTask.Context context) { diff --git a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesRunningOptionsDTO.java b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesRunningOptionsDTO.java new file mode 100644 index 0000000000..b53ab20fdc --- /dev/null +++ b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesRunningOptionsDTO.java @@ -0,0 +1,59 @@ +/**************************************************************** + * Licensed to the Apache Software Foundation (ASF) under one * + * or more contributor license agreements. See the NOTICE file * + * distributed with this work for additional information * + * regarding copyright ownership. The ASF licenses this file * + * to you under the Apache License, Version 2.0 (the * + * "License"); you may not use this file except in compliance * + * with the License. You may obtain a copy of the License at * + * * + * http://www.apache.org/licenses/LICENSE-2.0 * + * * + * Unless required by applicable law or agreed to in writing, * + * software distributed under the License is distributed on an * + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY * + * KIND, either express or implied. See the License for the * + * specific language governing permissions and limitations * + * under the License. * + ****************************************************************/ + +package org.apache.james.mailbox.cassandra.mail.task; + +import java.util.Optional; + +import org.apache.james.mailbox.cassandra.mail.task.SolveMailboxInconsistenciesService.RunningOptions; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; + +public class SolveMailboxInconsistenciesRunningOptionsDTO { + public static SolveMailboxInconsistenciesRunningOptionsDTO asDTO(RunningOptions domainObject) { + return new SolveMailboxInconsistenciesRunningOptionsDTO( + Optional.of(domainObject.getMaxIterations()), + Optional.of(domainObject.isAutoMerge())); + } + + private final Optional<Integer> maxIterations; + private final Optional<Boolean> autoMerge; + + @JsonCreator + public SolveMailboxInconsistenciesRunningOptionsDTO(@JsonProperty("maxIterations") Optional<Integer> maxIterations, + @JsonProperty("autoMerge") Optional<Boolean> autoMerge) { + this.maxIterations = maxIterations; + this.autoMerge = autoMerge; + } + + public Optional<Integer> getMaxIterations() { + return maxIterations; + } + + public Optional<Boolean> getAutoMerge() { + return autoMerge; + } + + public RunningOptions asDomainObject() { + return new RunningOptions( + maxIterations.orElse(RunningOptions.DEFAULT_MAX_ITERATIONS), + autoMerge.orElse(RunningOptions.DEFAULT_AUTO_MERGE)); + } +} diff --git a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesService.java b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesService.java index d7f71b2042..e345faa0d7 100644 --- a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesService.java +++ b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesService.java @@ -56,26 +56,39 @@ public class SolveMailboxInconsistenciesService { @FunctionalInterface public interface Inconsistency { static Mono<Inconsistency> detectMailboxDaoInconsistency(Mailbox mailboxEntry, Mono<Mailbox> pathEntry) { + // Read-repair entry point: never auto-merges (no merging runner available, and read paths + // must not trigger destructive ghost merges). + return detectMailboxDaoInconsistency(mailboxEntry, pathEntry, null, false); + } + + static Mono<Inconsistency> detectMailboxDaoInconsistency(Mailbox mailboxEntry, Mono<Mailbox> pathEntry, + MailboxMergingTaskRunner mergingRunner, boolean autoMerge) { return pathEntry .map(mailboxByPath -> { if (mailboxByPath.getMailboxId().equals(mailboxEntry.getMailboxId())) { return NO_INCONSISTENCY; } // Path entry references another mailbox. - return new ConflictingEntryInconsistency(mailboxEntry, mailboxByPath); + return new ConflictingEntryInconsistency(mailboxEntry, mailboxByPath, mergingRunner, autoMerge); }) .defaultIfEmpty(new OrphanMailboxDAOEntry(mailboxEntry)); } static Mono<Inconsistency> detectMailboxPathDaoInconsistency(Mailbox mailboxByPathEntry, Mono<Mailbox> mailboxEntry) { + // Read-repair entry point: never auto-merges (this site only ever yields same-id conflicts anyway). + return detectMailboxPathDaoInconsistency(mailboxByPathEntry, mailboxEntry, null, false); + } + + static Mono<Inconsistency> detectMailboxPathDaoInconsistency(Mailbox mailboxByPathEntry, Mono<Mailbox> mailboxEntry, + MailboxMergingTaskRunner mergingRunner, boolean autoMerge) { return mailboxEntry .map(mailboxById -> { if (mailboxByPathEntry.generateAssociatedPath().equals(mailboxById.generateAssociatedPath())) { return NO_INCONSISTENCY; } // Mailbox references another path - return new ConflictingEntryInconsistency(mailboxById, mailboxByPathEntry); + return new ConflictingEntryInconsistency(mailboxById, mailboxByPathEntry, mergingRunner, autoMerge); }) .defaultIfEmpty(new OrphanMailboxPathDAOEntry(mailboxByPathEntry)); } @@ -180,10 +193,15 @@ public class SolveMailboxInconsistenciesService { private static class ConflictingEntryInconsistency implements Inconsistency { private final Mailbox mailboxDaoEntry; private final Mailbox mailboxPathEntry; + private final MailboxMergingTaskRunner mergingRunner; + private final boolean autoMerge; - private ConflictingEntryInconsistency(Mailbox mailboxDaoEntry, Mailbox mailboxPathEntry) { + private ConflictingEntryInconsistency(Mailbox mailboxDaoEntry, Mailbox mailboxPathEntry, + MailboxMergingTaskRunner mergingRunner, boolean autoMerge) { this.mailboxDaoEntry = mailboxDaoEntry; this.mailboxPathEntry = mailboxPathEntry; + this.mergingRunner = mergingRunner; + this.autoMerge = autoMerge; } @Override @@ -197,9 +215,59 @@ public class SolveMailboxInconsistenciesService { return fixSameMailboxConflict(context, mailboxDAO, pathV3DAO); } + if (autoMerge) { + return autoMergeConflict(context, mailboxDAO, pathV3DAO); + } return reportConflict(context); } + // Two *different* mailboxes resolve to the same path (the historical "ghost mailbox"): the + // path table registers the winner, while the loser is a projection entry squatting that path. + // The path table being the source of truth, the registered mailbox wins and the squatting + // projection is merged into it (its messages and rights are moved over, then its projection + // row is dropped) using the same machinery as MailboxMergingTask. + // + // We only merge once re-reading (STRONG) confirms the clean-ghost picture: the winner still + // owns the path, the loser still resolves to it, and -- crucially -- the path table vouches + // for the loser *nowhere*. If the loser owns another path, it is a genuine mailbox with its + // own (same-id) conflict to resolve first, so we report rather than destroy it. + private Mono<Result> autoMergeConflict(Context context, CassandraMailboxDAO mailboxDAO, CassandraMailboxPathV3DAO pathV3DAO) { + CassandraId winnerId = (CassandraId) mailboxPathEntry.getMailboxId(); + CassandraId loserId = (CassandraId) mailboxDaoEntry.getMailboxId(); + MailboxPath conflictingPath = mailboxPathEntry.generateAssociatedPath(); + + Mono<Boolean> pathStillOwnedByWinner = pathV3DAO.retrieve(conflictingPath, STRONG) + .map(entry -> entry.getMailboxId().equals(winnerId)) + .defaultIfEmpty(false); + Mono<Boolean> loserStillResolvesToPath = mailboxDAO.retrieveMailbox(loserId) + .map(projection -> projection.generateAssociatedPath().equals(conflictingPath)) + .defaultIfEmpty(false); + Mono<Boolean> loserIsUnregistered = pathV3DAO.listUserMailboxes(mailboxDaoEntry.getNamespace(), mailboxDaoEntry.getUser(), STRONG) + .filter(entry -> entry.getMailboxId().equals(loserId)) + .hasElements() + .map(referenced -> !referenced); + + return Mono.zip(pathStillOwnedByWinner, loserStillResolvesToPath, loserIsUnregistered) + .flatMap(state -> { + boolean cleanGhost = state.getT1() && state.getT2() && state.getT3(); + if (!cleanGhost) { + // State no longer matches the clean-ghost picture (already reconciled, or the + // loser owns another path): do not merge. + if (state.getT1() && state.getT2()) { + // Still conflicting but the loser is registered elsewhere: leave it to an admin. + return reportConflict(context); + } + return Mono.just(Result.COMPLETED); + } + return mergingRunner.runReactive(loserId, winnerId, new MailboxMergingTask.Context(0)) + .doOnNext(result -> { + LOGGER.info("Auto-merged ghost mailbox {} into {} at path {}", + loserId.serialize(), winnerId.serialize(), conflictingPath.asString()); + context.addFixedInconsistency(winnerId); + }); + }); + } + // Auto-resolution is restricted to the case where BOTH the conflicting path entry and the // projection's path are registered to the same mailbox id: only then are they two genuine // aliases of a single mailbox, hence safe to deduplicate without data loss. If the @@ -265,31 +333,39 @@ public class SolveMailboxInconsistenciesService { public static class RunningOptions { public static final int DEFAULT_MAX_ITERATIONS = 1; - public static final RunningOptions DEFAULT = new RunningOptions(DEFAULT_MAX_ITERATIONS); + public static final boolean DEFAULT_AUTO_MERGE = false; + public static final RunningOptions DEFAULT = new RunningOptions(DEFAULT_MAX_ITERATIONS, DEFAULT_AUTO_MERGE); private final int maxIterations; + private final boolean autoMerge; - public RunningOptions(int maxIterations) { + public RunningOptions(int maxIterations, boolean autoMerge) { Preconditions.checkArgument(maxIterations >= 1, "'maxIterations' must be strictly positive"); this.maxIterations = maxIterations; + this.autoMerge = autoMerge; } public int getMaxIterations() { return maxIterations; } + public boolean isAutoMerge() { + return autoMerge; + } + @Override public final boolean equals(Object o) { if (o instanceof RunningOptions) { RunningOptions that = (RunningOptions) o; - return this.maxIterations == that.maxIterations; + return this.maxIterations == that.maxIterations + && this.autoMerge == that.autoMerge; } return false; } @Override public final int hashCode() { - return Objects.hash(maxIterations); + return Objects.hash(maxIterations, autoMerge); } } @@ -477,13 +553,15 @@ public class SolveMailboxInconsistenciesService { private final CassandraMailboxDAO mailboxDAO; private final CassandraMailboxPathV3DAO mailboxPathV3DAO; private final CassandraSchemaVersionManager versionManager; + private final MailboxMergingTaskRunner mergingRunner; @Inject SolveMailboxInconsistenciesService(CassandraMailboxDAO mailboxDAO, CassandraMailboxPathV3DAO mailboxPathV3DAO, - CassandraSchemaVersionManager versionManager) { + CassandraSchemaVersionManager versionManager, MailboxMergingTaskRunner mergingRunner) { this.mailboxDAO = mailboxDAO; this.mailboxPathV3DAO = mailboxPathV3DAO; this.versionManager = versionManager; + this.mergingRunner = mergingRunner; } public Mono<Result> fixMailboxInconsistencies(Context context) { @@ -492,7 +570,7 @@ public class SolveMailboxInconsistenciesService { public Mono<Result> fixMailboxInconsistencies(Context context, RunningOptions runningOptions) { assertValidVersion(); - return fixUntilStable(context, runningOptions.getMaxIterations()); + return fixUntilStable(context, runningOptions.getMaxIterations(), runningOptions.isAutoMerge()); } // Reconciliation is run to a fixpoint: fixing an inconsistency in one pass (dropping a stale @@ -500,23 +578,23 @@ public class SolveMailboxInconsistenciesService { // detects. We re-run as long as a pass keeps applying fixes, bounded by maxIterations to guard // against oscillation. As fixes only ever grow the fixedInconsistencies list, a pass that adds // none means we reached a stable state. - private Mono<Result> fixUntilStable(Context context, int remainingIterations) { + private Mono<Result> fixUntilStable(Context context, int remainingIterations, boolean autoMerge) { int fixedBefore = context.snapshot().getFixedInconsistencies().size(); - return runOnePass(context) + return runOnePass(context, autoMerge) .flatMap(result -> { boolean appliedFixes = context.snapshot().getFixedInconsistencies().size() > fixedBefore; if (appliedFixes && remainingIterations > 1) { - return fixUntilStable(context, remainingIterations - 1) + return fixUntilStable(context, remainingIterations - 1, autoMerge) .map(nextResult -> Task.combine(result, nextResult)); } return Mono.just(result); }); } - private Mono<Result> runOnePass(Context context) { + private Mono<Result> runOnePass(Context context, boolean autoMerge) { return Flux.concat( - processMailboxDaoInconsistencies(context), - processMailboxPathDaoInconsistencies(context)) + processMailboxDaoInconsistencies(context, autoMerge), + processMailboxPathDaoInconsistencies(context, autoMerge)) .reduce(Result.COMPLETED, Task::combine); } @@ -531,9 +609,9 @@ public class SolveMailboxInconsistenciesService { version.getValue()); } - private Flux<Result> processMailboxPathDaoInconsistencies(Context context) { + private Flux<Result> processMailboxPathDaoInconsistencies(Context context, boolean autoMerge) { return mailboxPathV3DAO.listAll() - .flatMap(this::detectMailboxPathDaoInconsistency, DEFAULT_CONCURRENCY) + .flatMap(entry -> detectMailboxPathDaoInconsistency(entry, autoMerge), DEFAULT_CONCURRENCY) .doOnNext(any -> context.incrementProcessedMailboxPathEntries()) // Detect every inconsistency first, then fix them one at a time. Resolving a same-mailbox // conflict may realign the projection, so a fully materialized detection set fixed @@ -546,22 +624,28 @@ public class SolveMailboxInconsistenciesService { .concatMap(inconsistency -> inconsistency.fix(context, mailboxDAO, mailboxPathV3DAO))); } - private Flux<Result> processMailboxDaoInconsistencies(Context context) { + private Flux<Result> processMailboxDaoInconsistencies(Context context, boolean autoMerge) { return mailboxDAO.retrieveAllMailboxes() - .flatMap(this::detectMailboxDaoInconsistency, DEFAULT_CONCURRENCY) - .flatMap(inconsistency -> inconsistency.fix(context, mailboxDAO, mailboxPathV3DAO), DEFAULT_CONCURRENCY) - .doOnNext(any -> context.incrementProcessedMailboxEntries()); + .flatMap(entry -> detectMailboxDaoInconsistency(entry, autoMerge), DEFAULT_CONCURRENCY) + .doOnNext(any -> context.incrementProcessedMailboxEntries()) + // Detect every inconsistency first, then fix them one at a time. Auto-merging a ghost + // mailbox mutates message and projection state, so a fully materialized detection set + // fixed sequentially prevents a fix from racing with the detection of a sibling entry. + .filter(inconsistency -> inconsistency != NO_INCONSISTENCY) + .collectList() + .flatMapMany(inconsistencies -> Flux.fromIterable(inconsistencies) + .concatMap(inconsistency -> inconsistency.fix(context, mailboxDAO, mailboxPathV3DAO))); } - private Mono<Inconsistency> detectMailboxDaoInconsistency(Mailbox mailboxEntry) { + private Mono<Inconsistency> detectMailboxDaoInconsistency(Mailbox mailboxEntry, boolean autoMerge) { Mono<Mailbox> pathEntry = mailboxPathV3DAO.retrieve(mailboxEntry.generateAssociatedPath(), STRONG); - return Inconsistency.detectMailboxDaoInconsistency(mailboxEntry, pathEntry); + return Inconsistency.detectMailboxDaoInconsistency(mailboxEntry, pathEntry, mergingRunner, autoMerge); } - private Mono<Inconsistency> detectMailboxPathDaoInconsistency(Mailbox mailboxByPathEntry) { + private Mono<Inconsistency> detectMailboxPathDaoInconsistency(Mailbox mailboxByPathEntry, boolean autoMerge) { CassandraId cassandraId = (CassandraId) mailboxByPathEntry.getMailboxId(); Mono<Mailbox> mailboxEntry = mailboxDAO.retrieveMailbox(cassandraId); - return Inconsistency.detectMailboxPathDaoInconsistency(mailboxByPathEntry, mailboxEntry); + return Inconsistency.detectMailboxPathDaoInconsistency(mailboxByPathEntry, mailboxEntry, mergingRunner, autoMerge); } } diff --git a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesServiceTest.java b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesServiceTest.java index 8179fc883e..ea92010ccc 100644 --- a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesServiceTest.java +++ b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesServiceTest.java @@ -23,6 +23,12 @@ import static org.apache.james.JsonSerializationVerifier.recursiveComparisonConf import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import org.apache.james.backends.cassandra.CassandraCluster; import org.apache.james.backends.cassandra.CassandraClusterExtension; @@ -70,6 +76,7 @@ class SolveMailboxInconsistenciesServiceTest { CassandraMailboxDAO mailboxDAO; CassandraMailboxPathV3DAO mailboxPathV3DAO; CassandraSchemaVersionDAO versionDAO; + MailboxMergingTaskRunner mergingRunner; SolveMailboxInconsistenciesService testee; @BeforeEach @@ -80,7 +87,8 @@ class SolveMailboxInconsistenciesServiceTest { mailboxPathV3DAO = new CassandraMailboxPathV3DAO( cassandra.getConf()); versionDAO = new CassandraSchemaVersionDAO(cassandra.getConf()); - testee = new SolveMailboxInconsistenciesService(mailboxDAO, mailboxPathV3DAO, new CassandraSchemaVersionManager(versionDAO)); + mergingRunner = mock(MailboxMergingTaskRunner.class); + testee = new SolveMailboxInconsistenciesService(mailboxDAO, mailboxPathV3DAO, new CassandraSchemaVersionManager(versionDAO), mergingRunner); versionDAO.updateVersion(new SchemaVersion(8)).block(); } @@ -420,4 +428,51 @@ class SolveMailboxInconsistenciesServiceTest { .containsExactlyInAnyOrder(MAILBOX_2); }); } + + @Test + void fixMailboxInconsistenciesShouldAutoMergeGhostMailboxWhenAutoMergeEnabled() { + // Two different mailboxes resolve to the same path "abc": CASSANDRA_ID_2 owns the path (path + // table = source of truth), CASSANDRA_ID_1 is a ghost projection squatting it with no path + // registration of its own. With autoMerge on, the ghost is merged into the path owner. + Mailbox pathOwnerProjection = new Mailbox(MAILBOX_PATH, UID_VALIDITY_2, CASSANDRA_ID_2); + mailboxDAO.save(MAILBOX).block(); + mailboxDAO.save(pathOwnerProjection).block(); + mailboxPathV3DAO.save(MAILBOX_2).block(); + + // The merging runner moves messages then drops the loser's projection row: simulate that side effect. + when(mergingRunner.runReactive(eq(CASSANDRA_ID_1), eq(CASSANDRA_ID_2), any())) + .thenReturn(mailboxDAO.delete(CASSANDRA_ID_1).thenReturn(Result.COMPLETED)); + + Context context = new Context(); + testee.fixMailboxInconsistencies(context, new SolveMailboxInconsistenciesService.RunningOptions(1, true)).block(); + + SoftAssertions.assertSoftly(softly -> { + verify(mergingRunner).runReactive(eq(CASSANDRA_ID_1), eq(CASSANDRA_ID_2), any()); + softly.assertThat(mailboxDAO.retrieveAllMailboxes().collectList().block()) + .containsExactlyInAnyOrder(pathOwnerProjection); + softly.assertThat(mailboxPathV3DAO.listAll().collectList().block()) + .containsExactlyInAnyOrder(MAILBOX_2); + softly.assertThat(context.snapshot().getFixedInconsistencies()) + .containsExactly(CASSANDRA_ID_2); + }); + } + + @Test + void fixMailboxInconsistenciesShouldNotAutoMergeWhenLoserIsRegisteredElsewhere() { + // CASSANDRA_ID_1 squats path "abc" (owned by CASSANDRA_ID_2) but also legitimately owns path + // "xyz": it is not a clean ghost, so the path table vouches for it and it must not be merged away. + Mailbox pathOwnerProjection = new Mailbox(MAILBOX_PATH, UID_VALIDITY_2, CASSANDRA_ID_2); + Mailbox loserOtherPath = new Mailbox(NEW_MAILBOX_PATH, UID_VALIDITY_1, CASSANDRA_ID_1); + mailboxDAO.save(MAILBOX).block(); + mailboxDAO.save(pathOwnerProjection).block(); + mailboxPathV3DAO.save(MAILBOX_2).block(); + mailboxPathV3DAO.save(loserOtherPath).block(); + + testee.fixMailboxInconsistencies(new Context(), new SolveMailboxInconsistenciesService.RunningOptions(1, true)).block(); + + verify(mergingRunner, never()).runReactive(any(), any(), any()); + // The ghost projection is preserved (not destroyed by an auto-merge). + assertThat(mailboxDAO.retrieveAllMailboxes().collectList().block()) + .contains(MAILBOX); + } } \ No newline at end of file diff --git a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesTaskSerializationTest.java b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesTaskSerializationTest.java index 85d25369eb..97fec53ee5 100644 --- a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesTaskSerializationTest.java +++ b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesTaskSerializationTest.java @@ -41,11 +41,11 @@ class SolveMailboxInconsistenciesTaskSerializationTest { public static final String MAILBOX_ID_AS_STRING = "464765a0-e4e7-11e4-aba4-710c1de3782b"; private static final CassandraId MAILBOX_ID = CassandraId.of(UUID.fromString(MAILBOX_ID_AS_STRING)); - private static final SolveMailboxInconsistenciesService.RunningOptions RUNNING_OPTIONS = new SolveMailboxInconsistenciesService.RunningOptions(3); + private static final SolveMailboxInconsistenciesService.RunningOptions RUNNING_OPTIONS = new SolveMailboxInconsistenciesService.RunningOptions(3, true); private static final SolveMailboxInconsistenciesService SERVICE = mock(SolveMailboxInconsistenciesService.class); private static final SolveMailboxInconsistenciesTask TASK = new SolveMailboxInconsistenciesTask(SERVICE, RUNNING_OPTIONS); - private static final String SERIALIZED_TASK = "{\"type\": \"solve-mailbox-inconsistencies\", \"runningOptions\":{\"maxIterations\":3}}"; + private static final String SERIALIZED_TASK = "{\"type\": \"solve-mailbox-inconsistencies\", \"runningOptions\":{\"maxIterations\":3,\"autoMerge\":true}}"; private static final ConflictingEntry CONFLICTING_ENTRY = ConflictingEntry.builder() .mailboxDaoEntry(MAILBOX_PATH, MAILBOX_ID) .mailboxPathDaoEntry(MAILBOX_PATH_2, MAILBOX_ID); @@ -68,7 +68,7 @@ class SolveMailboxInconsistenciesTaskSerializationTest { " }]," + " \"errors\":3," + " \"timestamp\":\"2018-11-13T12:00:55Z\"," + - " \"runningOptions\":{\"maxIterations\":3}" + + " \"runningOptions\":{\"maxIterations\":3,\"autoMerge\":true}" + "}"; @Test diff --git a/server/protocols/webadmin/webadmin-cassandra/src/main/java/org/apache/james/webadmin/routes/SolveMailboxInconsistenciesRequestToTask.java b/server/protocols/webadmin/webadmin-cassandra/src/main/java/org/apache/james/webadmin/routes/SolveMailboxInconsistenciesRequestToTask.java index 6ec5928b9c..988e342e25 100644 --- a/server/protocols/webadmin/webadmin-cassandra/src/main/java/org/apache/james/webadmin/routes/SolveMailboxInconsistenciesRequestToTask.java +++ b/server/protocols/webadmin/webadmin-cassandra/src/main/java/org/apache/james/webadmin/routes/SolveMailboxInconsistenciesRequestToTask.java @@ -19,6 +19,8 @@ package org.apache.james.webadmin.routes; +import java.util.Optional; + import jakarta.inject.Inject; import org.apache.james.mailbox.cassandra.mail.task.SolveMailboxInconsistenciesService; @@ -43,11 +45,13 @@ public class SolveMailboxInconsistenciesRequestToTask extends TaskFromRequestReg "`ALL-SERVICES-ARE-OFFLINE` in order to prevent accidental calls. " + "Check the documentation for details."); - RunningOptions runningOptions = RunningOptionsParser.intQueryParameter(request, "maxIterations") - .map(RunningOptions::new) - .orElse(RunningOptions.DEFAULT); + int maxIterations = RunningOptionsParser.intQueryParameter(request, "maxIterations") + .orElse(RunningOptions.DEFAULT_MAX_ITERATIONS); + boolean autoMerge = Optional.ofNullable(request.queryParams("autoMerge")) + .map(Boolean::parseBoolean) + .orElse(RunningOptions.DEFAULT_AUTO_MERGE); - return new SolveMailboxInconsistenciesTask(service, runningOptions); + return new SolveMailboxInconsistenciesTask(service, new RunningOptions(maxIterations, autoMerge)); }); } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
