diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b77a6e..b92683f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ # Changelog +## Unreleased + +### Added + +- The backup toast that appears after you leave a world has a Cancel button. + It stops the backup while the world is copied or while the destinations + write. A destination that had already finished is kept and recorded. Once + the backup is being recorded in the catalog it can no longer be cancelled. + ## 0.3.6 (2026-09-08) ### Added diff --git a/src/client/java/dev/ishaanko/worldarchive/runtime/BackgroundBackupWarnings.java b/src/client/java/dev/ishaanko/worldarchive/runtime/BackgroundBackupWarnings.java index 4026956..89dadee 100644 --- a/src/client/java/dev/ishaanko/worldarchive/runtime/BackgroundBackupWarnings.java +++ b/src/client/java/dev/ishaanko/worldarchive/runtime/BackgroundBackupWarnings.java @@ -4,6 +4,7 @@ import dev.ishaanko.worldarchive.model.BackupStatus; import java.util.Objects; import java.util.Optional; +import java.util.concurrent.CancellationException; /** Credential-safe notices for unattended backup outcomes. */ final class BackgroundBackupWarnings { @@ -19,6 +20,11 @@ static Optional worldExit(BackupResult result, Throwable failure) { } static ExitNotice worldExitNotice(BackupResult result, Throwable failure) { + if (isCancellation(failure)) { + return new ExitNotice( + "Backup cancelled; world was saved", + NoticeSeverity.WARNING); + } if (failure != null || result == null) { return new ExitNotice( "World save or backup did not complete", @@ -44,10 +50,19 @@ static String worldExitStartedMessage() { return "Creating backup... Keep Minecraft open until it finishes."; } + /** A cancellation is the user's own choice, so it never becomes a warning. */ + static boolean isCancellation(Throwable failure) { + return failure instanceof CancellationException + || failure != null && failure.getCause() instanceof CancellationException; + } + private static Optional warning( String trigger, BackupResult result, Throwable failure) { + if (isCancellation(failure)) { + return Optional.empty(); + } if (failure != null || result == null) { return Optional.of(trigger + " backup did not complete"); } diff --git a/src/client/java/dev/ishaanko/worldarchive/runtime/BackupProgressToast.java b/src/client/java/dev/ishaanko/worldarchive/runtime/BackupProgressToast.java index 391a777..60b1162 100644 --- a/src/client/java/dev/ishaanko/worldarchive/runtime/BackupProgressToast.java +++ b/src/client/java/dev/ishaanko/worldarchive/runtime/BackupProgressToast.java @@ -5,6 +5,7 @@ import java.util.OptionalDouble; import java.util.concurrent.atomic.AtomicReference; import net.minecraft.ChatFormatting; +import net.minecraft.client.Minecraft; import net.minecraft.client.gui.Font; import net.minecraft.client.gui.GuiGraphicsExtractor; import net.minecraft.client.gui.components.toasts.Toast; @@ -14,13 +15,19 @@ /** * Backup toast that stays visible with a live progress bar while an unattended - * backup runs, then shows the color-coded outcome briefly and hides. Progress - * arrives from worker threads; rendering reads one immutable state snapshot. + * backup runs, then shows the color-coded outcome briefly and hides. While the + * backup can still be cancelled the toast draws a Cancel button. Toasts receive + * no input of their own, so the monitor forwards screen clicks to + * {@link #mouseClicked}. Progress arrives from worker threads; rendering reads + * one immutable state snapshot. */ final class BackupProgressToast implements Toast { private static final Component TITLE = Component.literal("WorldArchive").withStyle(ChatFormatting.BOLD); + private static final Component CANCEL_LABEL = + Component.translatable("screen.worldarchive.backup_toast.cancel"); + private static final int WIDTH = 200; private static final int HEIGHT = 40; @@ -49,10 +56,38 @@ final class BackupProgressToast implements Toast { private static final int BAR_PROGRESS = 0xFF5AAE61; + private static final int BUTTON_TOP = 4; + + private static final int BUTTON_HEIGHT = 12; + + private static final int BUTTON_PADDING = 4; + + private static final int BUTTON_RIGHT_MARGIN = 6; + + private static final int BUTTON_BACKGROUND = 0xFF2A2A2A; + + private static final int BUTTON_BORDER = 0xFF808080; + + private static final int BUTTON_BORDER_HOVERED = 0xFFFFFFFF; + + private final Minecraft minecraft; + private final Font font; + private final Runnable cancel; + private final AtomicReference state; + private final int buttonLeft; + + private final int buttonWidth; + + // Where the toast manager last placed this toast, in GUI coordinates. Written + // and read on the render thread only. + private float left; + + private float top; + private long hideAtVisibleMs = Long.MAX_VALUE; private Toast.Visibility visibility = Toast.Visibility.SHOW; @@ -61,19 +96,37 @@ final class BackupProgressToast implements Toast { private List renderedLines = List.of(); - BackupProgressToast(Font font, String message) { - this.font = Objects.requireNonNull(font, "font"); - this.state = new AtomicReference<>( - new State(message, RUNNING_COLOR, OptionalDouble.empty(), false)); + /** A toast that only shows an outcome; it has no Cancel button. */ + BackupProgressToast(Minecraft minecraft, String message) { + this(minecraft, message, () -> { }, false); + } + + /** A toast for a running backup; {@code cancel} asks that backup to stop. */ + BackupProgressToast(Minecraft minecraft, String message, Runnable cancel) { + this(minecraft, message, cancel, true); + } + + private BackupProgressToast( + Minecraft minecraft, + String message, + Runnable cancel, + boolean cancellable) { + this.minecraft = Objects.requireNonNull(minecraft, "minecraft"); + this.font = minecraft.font; + this.cancel = Objects.requireNonNull(cancel, "cancel"); + this.state = new AtomicReference<>(new State( + message, RUNNING_COLOR, OptionalDouble.empty(), false, cancellable, false)); + this.buttonWidth = font.width(CANCEL_LABEL) + BUTTON_PADDING * 2; + this.buttonLeft = WIDTH - BUTTON_RIGHT_MARGIN - buttonWidth; } - /** Updates the live phase text and completed fraction; ignored once finished. */ + /** Updates the live phase text and completed fraction; ignored once finished or cancelling. */ void progress(String message, OptionalDouble fraction) { Objects.requireNonNull(message, "message"); OptionalDouble clamped = clampFraction(fraction); - state.updateAndGet(current -> current.finished() + state.updateAndGet(current -> current.finished() || current.cancelling() ? current - : new State(message, RUNNING_COLOR, clamped, false)); + : new State(message, RUNNING_COLOR, clamped, false, current.cancellable(), false)); } /** Switches to the outcome message; the toast hides a few seconds later. */ @@ -83,7 +136,41 @@ void finish(String message, BackgroundBackupWarnings.NoticeSeverity severity) { case WARNING -> WARNING_COLOR; case ERROR -> ERROR_COLOR; }; - state.set(new State(message, color, OptionalDouble.of(1), true)); + state.set(new State(message, color, OptionalDouble.of(1), true, false, false)); + } + + /** + * Handles a screen click in GUI coordinates. Returns true when the click landed on + * the Cancel button, in which case the backup was asked to stop and the button is gone. + */ + boolean mouseClicked(double mouseX, double mouseY) { + State current = state.get(); + if (!current.cancellable() || !overButton(mouseX, mouseY)) { + return false; + } + cancel.run(); + state.updateAndGet(latest -> latest.finished() + ? latest + : new State( + "Cancelling backup...", + RUNNING_COLOR, + latest.fraction(), + false, + false, + true)); + return true; + } + + @Override + public float xPos(int screenWidth, float visiblePortion) { + left = Toast.super.xPos(screenWidth, visiblePortion); + return left; + } + + @Override + public float yPos(int firstSlotIndex) { + top = Toast.super.yPos(firstSlotIndex); + return top; } @Override @@ -113,6 +200,9 @@ public void extractRenderState( graphics.fill(0, 0, WIDTH, HEIGHT, BACKGROUND); graphics.outline(0, 0, WIDTH, HEIGHT, BORDER); graphics.text(font, TITLE, TEXT_X, 7, TITLE_COLOR, false); + if (current.cancellable()) { + renderButton(graphics); + } int y = 19; for (FormattedCharSequence line : linesFor(current.message())) { graphics.text(font, line, TEXT_X, y, current.color(), false); @@ -123,6 +213,40 @@ public void extractRenderState( } } + private void renderButton(GuiGraphicsExtractor graphics) { + boolean hovered = !minecraft.mouseHandler.isMouseGrabbed() && overButton( + minecraft.mouseHandler.getScaledXPos(minecraft.getWindow()), + minecraft.mouseHandler.getScaledYPos(minecraft.getWindow())); + graphics.fill( + buttonLeft, + BUTTON_TOP, + buttonLeft + buttonWidth, + BUTTON_TOP + BUTTON_HEIGHT, + BUTTON_BACKGROUND); + graphics.outline( + buttonLeft, + BUTTON_TOP, + buttonWidth, + BUTTON_HEIGHT, + hovered ? BUTTON_BORDER_HOVERED : BUTTON_BORDER); + graphics.text( + font, + CANCEL_LABEL, + buttonLeft + BUTTON_PADDING, + BUTTON_TOP + 2, + TITLE_COLOR, + false); + } + + private boolean overButton(double mouseX, double mouseY) { + double x = mouseX - left; + double y = mouseY - top; + return x >= buttonLeft + && x < buttonLeft + buttonWidth + && y >= BUTTON_TOP + && y < BUTTON_TOP + BUTTON_HEIGHT; + } + private void renderBar( GuiGraphicsExtractor graphics, OptionalDouble fraction, @@ -173,7 +297,17 @@ private static OptionalDouble clampFraction(OptionalDouble fraction) { return OptionalDouble.of(Math.clamp(fraction.orElseThrow(), 0, 1)); } - private record State(String message, int color, OptionalDouble fraction, boolean finished) { + /** + * One render snapshot. {@code cancellable} draws the button; {@code cancelling} holds + * the "Cancelling" text until the outcome arrives. + */ + private record State( + String message, + int color, + OptionalDouble fraction, + boolean finished, + boolean cancellable, + boolean cancelling) { private State { Objects.requireNonNull(message, "message"); Objects.requireNonNull(fraction, "fraction"); diff --git a/src/client/java/dev/ishaanko/worldarchive/runtime/RuntimeBackgroundBackupMonitor.java b/src/client/java/dev/ishaanko/worldarchive/runtime/RuntimeBackgroundBackupMonitor.java index 8991145..b42eadb 100644 --- a/src/client/java/dev/ishaanko/worldarchive/runtime/RuntimeBackgroundBackupMonitor.java +++ b/src/client/java/dev/ishaanko/worldarchive/runtime/RuntimeBackgroundBackupMonitor.java @@ -21,8 +21,12 @@ import java.util.concurrent.atomic.AtomicReference; import java.util.function.BiConsumer; import java.util.function.BooleanSupplier; +import net.fabricmc.fabric.api.client.screen.v1.ScreenEvents; +import net.fabricmc.fabric.api.client.screen.v1.ScreenMouseEvents; import net.minecraft.ChatFormatting; import net.minecraft.client.Minecraft; +import net.minecraft.client.gui.screens.Screen; +import net.minecraft.client.input.MouseButtonEvent; import net.minecraft.network.chat.Component; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -32,6 +36,8 @@ final class RuntimeBackgroundBackupMonitor { private static final Logger LOGGER = LoggerFactory.getLogger(WorldArchiveMetadata.MOD_NAME); + private static final int LEFT_MOUSE_BUTTON = 0; + private final Minecraft minecraft; private final RuntimeNoticeStore noticeStore; @@ -71,6 +77,26 @@ final class RuntimeBackgroundBackupMonitor { } catch (IOException exception) { LOGGER.warn("Stored background backup notice could not be loaded"); } + ScreenEvents.AFTER_INIT.register(this::routeScreenClicks); + } + + // Toasts get no input of their own, so every screen forwards its clicks to the + // active progress toasts. A click on a Cancel button is consumed here. + private void routeScreenClicks(Minecraft ignored, Screen screen, int width, int height) { + ScreenMouseEvents.allowMouseClick(screen) + .register((ignoredScreen, event) -> !clickToast(event)); + } + + private boolean clickToast(MouseButtonEvent event) { + if (event.button() != LEFT_MOUSE_BUTTON || closed.getAsBoolean()) { + return false; + } + for (BackupProgressToast toast : activeToasts.values()) { + if (toast.mouseClicked(event.x(), event.y())) { + return true; + } + } + return false; } Optional warning() { @@ -147,7 +173,9 @@ private void observeExitResult( Object progressKey, BackupResult result, Throwable throwable) { - if (throwable != null) { + if (BackgroundBackupWarnings.isCancellation(throwable)) { + LOGGER.info("World-exit backup was cancelled"); + } else if (throwable != null) { failureLogger.accept( "World-exit backup did not complete", throwable); @@ -193,9 +221,13 @@ void showRetainedWarning() { } } - /** Shows the persistent progress toast for an unattended backup that just started. */ - void beginBackupProgress(String message, Object progressKey) { + /** + * Shows the persistent progress toast for an unattended backup that just started. + * {@code cancel} is run when the user presses the toast's Cancel button. + */ + void beginBackupProgress(String message, Object progressKey, Runnable cancel) { Objects.requireNonNull(progressKey, "progressKey"); + Objects.requireNonNull(cancel, "cancel"); if (closed.getAsBoolean()) { return; } @@ -203,7 +235,7 @@ void beginBackupProgress(String message, Object progressKey) { if (closed.getAsBoolean()) { return; } - BackupProgressToast toast = new BackupProgressToast(minecraft.font, message); + BackupProgressToast toast = new BackupProgressToast(minecraft, message, cancel); activeToasts.put(progressKey, toast); minecraft.gui.toastManager().addToast(toast); }); @@ -249,7 +281,7 @@ private void showBackupNotice( ? null : activeToasts.remove(progressKey); if (toast == null) { - toast = new BackupProgressToast(minecraft.font, notice.message()); + toast = new BackupProgressToast(minecraft, notice.message()); minecraft.gui.toastManager().addToast(toast); } toast.finish(notice.message(), notice.severity()); diff --git a/src/client/java/dev/ishaanko/worldarchive/runtime/RuntimeLifecycle.java b/src/client/java/dev/ishaanko/worldarchive/runtime/RuntimeLifecycle.java index d8edb85..29c657d 100644 --- a/src/client/java/dev/ishaanko/worldarchive/runtime/RuntimeLifecycle.java +++ b/src/client/java/dev/ishaanko/worldarchive/runtime/RuntimeLifecycle.java @@ -19,6 +19,7 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.concurrent.CancellationException; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionStage; import java.util.concurrent.RejectedExecutionException; @@ -419,7 +420,8 @@ private void completeStoppedServer( PendingLiveBackup exit = stopped.value().orElseThrow(); runtime.beginBackupProgress( BackgroundBackupWarnings.worldExitStartedMessage(), - exit.result()); + exit.result(), + () -> cancelExitBackup(exit)); boolean quitting; synchronized (lock) { quitting = clientStopping; @@ -608,10 +610,29 @@ private void resumeLevelSaving(MinecraftServer server) { } private void captureAndDispatch(PendingLiveBackup pending) { - PreparedBackup prepared = prepareCapture(pending); + if (!pending.beginCapture()) { + pending.fail(cancelled()); + return; + } + PreparedBackup prepared; + boolean proceed; + try { + prepared = prepareCapture(pending); + } finally { + proceed = pending.endCapture(); + } if (prepared == null) { return; } + if (!proceed) { + try { + prepared.close(); + } catch (IOException exception) { + runtime.logFailure("A cancelled capture could not be released", exception); + } + pending.fail(cancelled()); + return; + } CompletionStage operation; try { operation = pending.state().coordinator().createPreparedBackup( @@ -631,12 +652,31 @@ private void captureAndDispatch(PendingLiveBackup pending) { } return; } - operation.whenComplete((result, throwable) -> completeBackup( + CompletableFuture future = operation.toCompletableFuture(); + if (!pending.dispatched(future)) { + future.cancel(true); + } + future.whenComplete((result, throwable) -> completeBackup( pending, result, throwable)); } + /** + * Cancel button handler. Safe to call from the render thread: the coordinator + * cancel runs on a worker because it may release captured files. + */ + private void cancelExitBackup(PendingLiveBackup pending) { + CompletableFuture running = pending.requestCancel(); + if (running != null) { + runtime.submit(() -> running.cancel(true)); + } + } + + private static CancellationException cancelled() { + return new CancellationException("Backup was cancelled"); + } + private PreparedBackup prepareCapture(PendingLiveBackup pending) { try { CaptureProgressListener progress = (completed, total) -> pending.state() @@ -652,9 +692,14 @@ private PreparedBackup prepareCapture(PendingLiveBackup pending) { if (exception instanceof InterruptedException) { Thread.currentThread().interrupt(); } - pending.fail(WorldArchiveRuntime.safeFailure( - exception, - "World capture could not be prepared")); + if (pending.isCancelled()) { + // The Cancel button interrupted the capture thread; that is not a failure. + pending.fail(cancelled()); + } else { + pending.fail(WorldArchiveRuntime.safeFailure( + exception, + "World capture could not be prepared")); + } if (exception instanceof Error error) { throw error; } @@ -775,6 +820,14 @@ private static final class PendingLiveBackup { private final CompletableFuture settled = new CompletableFuture<>(); + // Guarded by this. A cancellation interrupts the capture thread while the + // world is copied, and cancels the coordinator operation once dispatched. + private boolean cancelRequested; + + private Thread captureThread; + + private CompletableFuture operation; + private PendingLiveBackup( RuntimeState state, IntegratedServer server, @@ -814,6 +867,57 @@ private CompletableFuture settled() { return settled; } + /** + * Asks the backup to stop. While the world is copied this interrupts the capture + * thread and returns null. Once dispatched it returns the running operation, which + * the caller cancels off-thread; the coordinator refuses once it records the + * result, and the backup then completes normally. + */ + private synchronized CompletableFuture requestCancel() { + if (result.isDone()) { + return null; + } + cancelRequested = true; + if (captureThread != null) { + captureThread.interrupt(); + return null; + } + return operation; + } + + private synchronized boolean isCancelled() { + return cancelRequested; + } + + /** Claims the calling thread for the capture; false when cancelled before it began. */ + private synchronized boolean beginCapture() { + if (cancelRequested) { + return false; + } + captureThread = Thread.currentThread(); + return true; + } + + /** + * Releases the capture thread and clears an interrupt a cancellation may have + * left on it. Returns false when the backup must not continue to its destinations. + */ + private boolean endCapture() { + boolean proceed; + synchronized (this) { + captureThread = null; + proceed = !cancelRequested; + } + Thread.interrupted(); + return proceed; + } + + /** Records the running operation; false when a cancellation arrived first. */ + private synchronized boolean dispatched(CompletableFuture future) { + operation = Objects.requireNonNull(future, "future"); + return !cancelRequested; + } + private void succeed(BackupResult value) { result.complete(value); settled.complete(null); diff --git a/src/client/java/dev/ishaanko/worldarchive/runtime/WorldArchiveRuntime.java b/src/client/java/dev/ishaanko/worldarchive/runtime/WorldArchiveRuntime.java index dd9734d..d1469f2 100644 --- a/src/client/java/dev/ishaanko/worldarchive/runtime/WorldArchiveRuntime.java +++ b/src/client/java/dev/ishaanko/worldarchive/runtime/WorldArchiveRuntime.java @@ -725,8 +725,8 @@ void showRetainedBackgroundWarning() { backgroundBackups.showRetainedWarning(); } - void beginBackupProgress(String message, Object progressKey) { - backgroundBackups.beginBackupProgress(message, progressKey); + void beginBackupProgress(String message, Object progressKey, Runnable cancel) { + backgroundBackups.beginBackupProgress(message, progressKey, cancel); } ProgressListener backupProgressListener(Object progressKey) { diff --git a/src/main/java/dev/ishaanko/worldarchive/core/AsyncTasks.java b/src/main/java/dev/ishaanko/worldarchive/core/AsyncTasks.java index c4cf1d3..43c8ebe 100644 --- a/src/main/java/dev/ishaanko/worldarchive/core/AsyncTasks.java +++ b/src/main/java/dev/ishaanko/worldarchive/core/AsyncTasks.java @@ -1,6 +1,7 @@ package dev.ishaanko.worldarchive.core; import java.util.Objects; +import java.util.concurrent.CancellationException; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionStage; import java.util.concurrent.Executor; @@ -24,6 +25,25 @@ public static CompletionStage supply( } } + /** + * Runs an operation whose worker can be stopped through the returned future. Unlike a + * plain cancel, {@link InterruptibleFuture#stop} keeps the operation's own outcome, so a + * backend that had already published a durable artifact still reports it. + */ + public static InterruptibleFuture supplyInterruptible( + Executor executor, + InterruptibleOperation operation) { + Objects.requireNonNull(executor, "executor"); + Objects.requireNonNull(operation, "operation"); + InterruptibleFuture result = new InterruptibleFuture<>(); + try { + executor.execute(() -> result.run(operation)); + } catch (RejectedExecutionException exception) { + result.completeExceptionally(exception); + } + return result; + } + public static CompletionStage run( Executor executor, Runnable operation) { @@ -35,4 +55,70 @@ public static CompletionStage run( return CompletableFuture.failedFuture(exception); } } + + /** One blocking operation that may be interrupted while it waits on locks or I/O. */ + @FunctionalInterface + public interface InterruptibleOperation { + T run() throws Exception; + } + + /** Future whose worker thread can be interrupted without discarding the outcome. */ + public static final class InterruptibleFuture extends CompletableFuture { + private final Object lock = new Object(); + + // Guarded by lock. + private Thread worker; + + private boolean stopRequested; + + private InterruptibleFuture() { + } + + /** + * Stops the operation. One that has not started never runs, and the future completes + * with a {@link CancellationException}. A running one is interrupted when + * {@code mayInterruptIfRunning} is set, and still completes with its own outcome. + */ + public void stop(boolean mayInterruptIfRunning) { + synchronized (lock) { + stopRequested = true; + if (worker != null && mayInterruptIfRunning) { + worker.interrupt(); + } + } + } + + /** Cancelling stops the worker too; the outcome is then the cancellation itself. */ + @Override + public boolean cancel(boolean mayInterruptIfRunning) { + boolean cancelled = super.cancel(mayInterruptIfRunning); + if (cancelled) { + stop(mayInterruptIfRunning); + } + return cancelled; + } + + private void run(InterruptibleOperation operation) { + synchronized (lock) { + if (stopRequested || isDone()) { + completeExceptionally( + new CancellationException("Operation was stopped before it started")); + return; + } + worker = Thread.currentThread(); + } + try { + complete(operation.run()); + } catch (Throwable throwable) { + completeExceptionally(throwable); + } finally { + synchronized (lock) { + worker = null; + } + // An interrupt that landed during the operation must not follow the thread + // back to its pool. + Thread.interrupted(); + } + } + } } diff --git a/src/main/java/dev/ishaanko/worldarchive/core/BackupBackend.java b/src/main/java/dev/ishaanko/worldarchive/core/BackupBackend.java index ff2acf5..456332a 100644 --- a/src/main/java/dev/ishaanko/worldarchive/core/BackupBackend.java +++ b/src/main/java/dev/ishaanko/worldarchive/core/BackupBackend.java @@ -10,6 +10,11 @@ *

Implementations must be thread-safe, must not perform blocking work on the calling thread, * and must complete with a result for recoverable destination failures. Exceptional completion is * reserved for programming errors or failures that prevent a trustworthy destination result.

+ * + *

Return the stage from {@link AsyncTasks#supplyInterruptible} directly, not a stage + * composed from it. The coordinator stops a cancelled write through that future so the + * destination's own outcome, such as a snapshot published before an interrupted push, is + * still recorded. Any other stage is cancelled outright and its outcome is lost.

*/ public interface BackupBackend { DestinationType destinationType(); diff --git a/src/main/java/dev/ishaanko/worldarchive/core/CancellationState.java b/src/main/java/dev/ishaanko/worldarchive/core/CancellationState.java index 8678c05..2a33fb1 100644 --- a/src/main/java/dev/ishaanko/worldarchive/core/CancellationState.java +++ b/src/main/java/dev/ishaanko/worldarchive/core/CancellationState.java @@ -1,6 +1,10 @@ package dev.ishaanko.worldarchive.core; -/** Point-of-no-return tracking for a create operation's cancellation window. */ +/** + * Point-of-no-return tracking for a create operation's cancellation window. A backup can be + * cancelled while it captures the world and while its destinations write; once the result is + * being recorded in the catalog it is committed. + */ enum CancellationState { CANCELLABLE, CANCELLATION_REQUESTED, diff --git a/src/main/java/dev/ishaanko/worldarchive/core/SerializedBackupCoordinator.java b/src/main/java/dev/ishaanko/worldarchive/core/SerializedBackupCoordinator.java index 6d8742f..72780cf 100644 --- a/src/main/java/dev/ishaanko/worldarchive/core/SerializedBackupCoordinator.java +++ b/src/main/java/dev/ishaanko/worldarchive/core/SerializedBackupCoordinator.java @@ -440,15 +440,9 @@ private void startDestinations( CreateOperation operation, CapturedBackup captured) { synchronized (operation) { - if (operation.cancellationState.compareAndSet( - CancellationState.CANCELLABLE, - CancellationState.COMMITTING)) { - // Destination publication is now the operation's point of no return. - } else if (operation.cancellationState.get() == CancellationState.CANCELLATION_REQUESTED) { + if (operation.cancelled.get()) { finish(operation, null, new CancellationException("Backup was cancelled")); return; - } else { - return; } } if (operation.terminal.get()) { @@ -482,6 +476,10 @@ private void startDestinations( source = CompletableFuture.failedFuture(throwable); } operation.destinationTasks.add(source); + if (operation.cancelled.get()) { + // A cancellation that raced this loop already stopped the earlier tasks. + stopDestination(source, operation.interruptRequested.get()); + } outcomes.add(source.handle((result, throwable) -> destinationOutcome( expectedDestination, result, @@ -491,13 +489,21 @@ private void startDestinations( .whenComplete((ignored, throwable) -> finalizeDestinations(operation, outcomes)); } + /** + * Records what the destinations produced. Recording is the point of no return: a + * cancellation that arrives later is refused. A cancellation that arrived while the + * destinations wrote still records any destination that had already finished, so a + * complete artifact is never left out of the catalog. + */ private void finalizeDestinations( CreateOperation operation, List> outcomes) { - if (operation.cancelled.get()) { - finish(operation, null, new CancellationException("Backup was cancelled")); - return; + synchronized (operation) { + operation.cancellationState.compareAndSet( + CancellationState.CANCELLABLE, + CancellationState.COMMITTING); } + boolean cancelled = operation.cancelled.get(); try { List destinations = outcomes.stream() .map(CompletableFuture::join) @@ -531,6 +537,10 @@ private void finalizeDestinations( "Backup complete; change inventory could not be updated"); } } + if (cancelled) { + finish(operation, null, new CancellationException("Backup was cancelled")); + return; + } finish(operation, result, null); } catch (Throwable throwable) { finish(operation, null, throwable); @@ -565,6 +575,19 @@ private void cancel(CreateOperation operation, boolean mayInterrupt) { } } for (CompletableFuture destination : operation.destinationTasks) { + stopDestination(destination, mayInterrupt); + } + } + + /** + * Stops one destination's work. A destination that can be interrupted keeps its own + * outcome, so a snapshot it already published is still recorded; any other stage is + * cancelled outright. + */ + private static void stopDestination(CompletableFuture destination, boolean mayInterrupt) { + if (destination instanceof AsyncTasks.InterruptibleFuture interruptible) { + interruptible.stop(mayInterrupt); + } else { destination.cancel(mayInterrupt); } } @@ -774,6 +797,11 @@ private static DestinationResult destinationOutcome( DestinationType expectedDestination, DestinationResult result, Throwable throwable) { + if (throwable instanceof CancellationException) { + return DestinationResult.failed( + expectedDestination, + "Cancelled before this destination finished"); + } if (throwable != null || result == null || result.destination() != expectedDestination) { return DestinationResult.failed( expectedDestination, diff --git a/src/main/java/dev/ishaanko/worldarchive/storage/git/GitAsyncExecutor.java b/src/main/java/dev/ishaanko/worldarchive/storage/git/GitAsyncExecutor.java index 6c86198..c75fb6b 100644 --- a/src/main/java/dev/ishaanko/worldarchive/storage/git/GitAsyncExecutor.java +++ b/src/main/java/dev/ishaanko/worldarchive/storage/git/GitAsyncExecutor.java @@ -1,13 +1,16 @@ package dev.ishaanko.worldarchive.storage.git; +import dev.ishaanko.worldarchive.core.AsyncTasks; import java.util.Objects; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutorService; -import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicReference; -/** Cancellation-aware executor ownership for Git storage operations. */ +/** + * Cancellation-aware executor ownership for Git storage operations. A submitted operation + * can be stopped through its future without losing the outcome it produces, which is how a + * snapshot published before an interrupted push still reaches the catalog. + */ final class GitAsyncExecutor implements AutoCloseable { private static final long SHUTDOWN_WAIT_SECONDS = 5; @@ -20,40 +23,8 @@ final class GitAsyncExecutor implements AutoCloseable { this.ownsExecutor = ownsExecutor; } - CompletableFuture submit(ThrowingOperation operation) { - CompletableFuture result = new CompletableFuture<>(); - AtomicReference> taskReference = new AtomicReference<>(); - Future task = executor.submit(() -> run(operation, result)); - taskReference.set(task); - result.whenComplete((ignored, throwable) -> cancelSubmitted(result, taskReference)); - if (result.isCancelled()) { - task.cancel(true); - } - return result; - } - - private static void run( - ThrowingOperation operation, - CompletableFuture result) { - if (result.isCancelled()) { - return; - } - try { - result.complete(operation.run()); - } catch (Throwable throwable) { - result.completeExceptionally(throwable); - } - } - - private static void cancelSubmitted( - CompletableFuture result, - AtomicReference> taskReference) { - if (result.isCancelled()) { - Future submitted = taskReference.get(); - if (submitted != null) { - submitted.cancel(true); - } - } + CompletableFuture submit(AsyncTasks.InterruptibleOperation operation) { + return AsyncTasks.supplyInterruptible(executor, operation); } @Override @@ -68,9 +39,4 @@ public void close() { Thread.currentThread().interrupt(); } } - - @FunctionalInterface - interface ThrowingOperation { - T run() throws Exception; - } } diff --git a/src/main/java/dev/ishaanko/worldarchive/storage/git/GitBackupBackend.java b/src/main/java/dev/ishaanko/worldarchive/storage/git/GitBackupBackend.java index 493a6b9..23885e8 100644 --- a/src/main/java/dev/ishaanko/worldarchive/storage/git/GitBackupBackend.java +++ b/src/main/java/dev/ishaanko/worldarchive/storage/git/GitBackupBackend.java @@ -7,6 +7,7 @@ import dev.ishaanko.worldarchive.model.DestinationResult; import dev.ishaanko.worldarchive.model.DestinationType; import dev.ishaanko.worldarchive.model.WorldId; +import java.io.IOException; import java.nio.file.Path; import java.nio.file.attribute.FileTime; import java.util.List; @@ -123,12 +124,29 @@ public CompletionStage createBackup( ProgressListener progressListener) { Objects.requireNonNull(capture, "capture"); Objects.requireNonNull(progressListener, "progressListener"); - return submit(() -> operations.createBackupBlocking(capture, progressListener)); + return submit(() -> createBackupBlocking(capture, progressListener)); + } + + /** + * Writes the snapshot on the calling thread. An interrupt after the local snapshot is + * published still returns that snapshot as pending sync, so a caller that runs this on + * its own interruptible worker keeps the outcome. + */ + DestinationResult createBackupBlocking( + BackupCapture capture, + ProgressListener progressListener) { + return operations.createBackupBlocking(capture, progressListener); + } + + /** Lists this repository's snapshots on the calling thread. */ + List listSnapshotsBlocking(Optional worldId) + throws IOException, InterruptedException, GitStorageException { + return lock.withLock(() -> operations.listSnapshotsBlocking(worldId)); } public CompletionStage> listSnapshots(Optional worldId) { Objects.requireNonNull(worldId, "worldId"); - return submit(() -> lock.withLock(() -> operations.listSnapshotsBlocking(worldId))); + return submit(() -> listSnapshotsBlocking(worldId)); } public CompletionStage verifySnapshot(WorldId worldId, BackupId backupId) { diff --git a/src/main/java/dev/ishaanko/worldarchive/storage/git/WorldGitSnapshotStore.java b/src/main/java/dev/ishaanko/worldarchive/storage/git/WorldGitSnapshotStore.java index b8fc6a1..2641932 100644 --- a/src/main/java/dev/ishaanko/worldarchive/storage/git/WorldGitSnapshotStore.java +++ b/src/main/java/dev/ishaanko/worldarchive/storage/git/WorldGitSnapshotStore.java @@ -161,13 +161,15 @@ public CompletionStage createBackup( Objects.requireNonNull(progressListener, "progressListener"); WorldId worldId = capture.manifest().worldId(); BackupId backupId = capture.manifest().backupId(); - return locateLocal(worldId, backupId).thenCompose(location -> { - if (location != SnapshotLocation.NONE) { - return CompletableFuture.completedFuture(DestinationResult.failed( + // One interruptible worker runs the whole write, so a cancellation reaches the + // Git thread directly and the coordinator still receives the snapshot it produced. + return AsyncTasks.supplyInterruptible(executor, () -> { + if (locateLocalBlocking(worldId, backupId) != SnapshotLocation.NONE) { + return DestinationResult.failed( DestinationType.GIT, - "The exact Git snapshot already exists in managed storage")); + "The exact Git snapshot already exists in managed storage"); } - return child(worldId).createBackup(capture, progressListener); + return child(worldId).createBackupBlocking(capture, progressListener); }); } @@ -507,19 +509,39 @@ private CompletionStage locateLocal(WorldId worldId, BackupId return child(worldId).listSnapshots(Optional.of(worldId)).thenCombine( legacySnapshots(Optional.of(worldId)), (childSnapshots, legacySnapshots) -> { - boolean childContains = contains(childSnapshots, backupId); - boolean legacyContains = contains(legacySnapshots, backupId); - if (childContains && legacyContains) { - throw new CompletionException(new GitStorageException( - "Git snapshot exists in both isolated and legacy repositories")); + try { + return locate(childSnapshots, legacySnapshots, backupId); + } catch (GitStorageException exception) { + throw new CompletionException(exception); } - if (childContains) { - return SnapshotLocation.CHILD; - } - return legacyContains ? SnapshotLocation.LEGACY : SnapshotLocation.NONE; }); } + private SnapshotLocation locateLocalBlocking(WorldId worldId, BackupId backupId) + throws IOException, InterruptedException, GitStorageException { + List childSnapshots = child(worldId).listSnapshotsBlocking(Optional.of(worldId)); + List legacySnapshots = legacyBackend.isPresent() + ? legacyBackend.orElseThrow().listSnapshotsBlocking(Optional.of(worldId)) + : List.of(); + return locate(childSnapshots, legacySnapshots, backupId); + } + + private static SnapshotLocation locate( + List childSnapshots, + List legacySnapshots, + BackupId backupId) throws GitStorageException { + boolean childContains = contains(childSnapshots, backupId); + boolean legacyContains = contains(legacySnapshots, backupId); + if (childContains && legacyContains) { + throw new GitStorageException( + "Git snapshot exists in both isolated and legacy repositories"); + } + if (childContains) { + return SnapshotLocation.CHILD; + } + return legacyContains ? SnapshotLocation.LEGACY : SnapshotLocation.NONE; + } + /** Shared CHILD/LEGACY dispatch; the NONE case is delegated to the caller's strategy. */ private CompletionStage withLocation( WorldId worldId, diff --git a/src/main/java/dev/ishaanko/worldarchive/storage/zip/ZipBackupBackend.java b/src/main/java/dev/ishaanko/worldarchive/storage/zip/ZipBackupBackend.java index 2f25b7f..de54291 100644 --- a/src/main/java/dev/ishaanko/worldarchive/storage/zip/ZipBackupBackend.java +++ b/src/main/java/dev/ishaanko/worldarchive/storage/zip/ZipBackupBackend.java @@ -59,7 +59,8 @@ public CompletionStage createBackup( Objects.requireNonNull(progressListener, "progressListener"); OperationId operationId = OperationId.create(); long totalBytes = capture.manifest().sourceByteCount(); - return AsyncTasks.supply(executor, () -> { + // Stopping the stage interrupts the write; the store discards the partial archive. + return AsyncTasks.supplyInterruptible(executor, () -> { report(progressListener, progress( operationId, capture, OperationPhase.PREPARING, 0, totalBytes, "Preparing ZIP backup")); diff --git a/src/main/resources/assets/worldarchive/lang/en_us.json b/src/main/resources/assets/worldarchive/lang/en_us.json index c6aa3e2..43a460e 100644 --- a/src/main/resources/assets/worldarchive/lang/en_us.json +++ b/src/main/resources/assets/worldarchive/lang/en_us.json @@ -1,6 +1,7 @@ { "modmenu.descriptionTranslation.worldarchive": "Creates dependable local backups of your single-player worlds.", "screen.worldarchive.backups_button": "World Backups", + "screen.worldarchive.backup_toast.cancel": "Cancel", "screen.worldarchive.settings.archive_folder": "Archive folder", "screen.worldarchive.settings.back": "Back", "screen.worldarchive.settings.browse": "Browse...", diff --git a/src/test/java/dev/ishaanko/worldarchive/core/AsyncTasksTest.java b/src/test/java/dev/ishaanko/worldarchive/core/AsyncTasksTest.java index 47ebfb3..a7dbd52 100644 --- a/src/test/java/dev/ishaanko/worldarchive/core/AsyncTasksTest.java +++ b/src/test/java/dev/ishaanko/worldarchive/core/AsyncTasksTest.java @@ -1,10 +1,21 @@ package dev.ishaanko.worldarchive.core; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CancellationException; import java.util.concurrent.CompletionException; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import org.junit.jupiter.api.Test; final class AsyncTasksTest { @@ -38,4 +49,49 @@ void runExecutorRejectionCompletesTheReturnedStage() { assertInstanceOf(RejectedExecutionException.class, failure.getCause()); } + + @Test + void stopBeforeStartNeverRunsTheOperation() { + AtomicBoolean ran = new AtomicBoolean(); + List queued = new ArrayList<>(); + AsyncTasks.InterruptibleFuture future = AsyncTasks.supplyInterruptible( + queued::add, + () -> { + ran.set(true); + return "ran"; + }); + + future.stop(true); + queued.forEach(Runnable::run); + + assertThrows(CancellationException.class, future::join); + assertFalse(ran.get()); + } + + @Test + void stopInterruptsTheWorkerAndKeepsItsOutcome() throws Exception { + ExecutorService executor = Executors.newSingleThreadExecutor(); + try { + CountDownLatch started = new CountDownLatch(1); + AsyncTasks.InterruptibleFuture future = AsyncTasks.supplyInterruptible( + executor, + () -> { + started.countDown(); + try { + Thread.sleep(Long.MAX_VALUE); + return "finished"; + } catch (InterruptedException exception) { + return "kept after interrupt"; + } + }); + assertTrue(started.await(5, TimeUnit.SECONDS)); + + future.stop(true); + + assertEquals("kept after interrupt", future.get(5, TimeUnit.SECONDS)); + assertFalse(future.isCancelled()); + } finally { + executor.shutdownNow(); + } + } } diff --git a/src/test/java/dev/ishaanko/worldarchive/core/CoordinatorFakes.java b/src/test/java/dev/ishaanko/worldarchive/core/CoordinatorFakes.java new file mode 100644 index 0000000..e9885ca --- /dev/null +++ b/src/test/java/dev/ishaanko/worldarchive/core/CoordinatorFakes.java @@ -0,0 +1,261 @@ +package dev.ishaanko.worldarchive.core; + +import dev.ishaanko.worldarchive.catalog.BackupCatalog; +import dev.ishaanko.worldarchive.model.BackupId; +import dev.ishaanko.worldarchive.model.BackupManifest; +import dev.ishaanko.worldarchive.model.BackupRecord; +import dev.ishaanko.worldarchive.model.BackupResult; +import dev.ishaanko.worldarchive.model.DestinationHealth; +import dev.ishaanko.worldarchive.model.DestinationResult; +import dev.ishaanko.worldarchive.model.DestinationType; +import dev.ishaanko.worldarchive.model.WorldId; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.security.MessageDigest; +import java.time.Instant; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionStage; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.BiFunction; +import java.util.function.Function; +import java.util.function.UnaryOperator; + +/** In-memory fakes shared by the coordinator tests. */ +final class CoordinatorFakes { + private CoordinatorFakes() { + } + + static final class FakeCaptureFactory implements BackupCaptureFactory { + final Path root; + + final AtomicInteger calls = new AtomicInteger(); + + final WorldInventory inventory; + + volatile java.util.function.Consumer observer = ignored -> { + }; + + FakeCaptureFactory(Path root) throws Exception { + this.root = root; + byte[] contents = "contents".getBytes(StandardCharsets.UTF_8); + this.inventory = WorldInventory.create(List.of(new WorldInventory.Entry( + "level.dat", + contents.length, + java.util.HexFormat.of().formatHex( + MessageDigest.getInstance("SHA-256").digest(contents))))); + } + + @Override + public CapturedBackup capture( + CreateBackupRequest request, + BackupId backupId, + Instant createdAt, + Optional previousInventory, + CaptureProgressListener progressListener) throws IOException { + observer.accept(request); + Files.createDirectories(root); + Path staging = Files.createDirectory(root.resolve("capture-" + calls.incrementAndGet())); + long changed = previousInventory.map(inventory::changedFilesSince).orElse(inventory.fileCount()); + BackupManifest manifest = BackupManifest.create( + backupId, + request.worldId(), + request.worldName(), + request.label(), + createdAt, + request.trigger(), + inventory.fileCount(), + inventory.byteCount(), + changed, + inventory.contentSha256(), + inventory.inventorySha256()); + return new CapturedBackup( + new BackupCapture(staging, manifest), + inventory, + () -> Files.deleteIfExists(staging)); + } + } + + static final class FakeBackend implements BackupBackend { + final DestinationType destination; + + final BiFunction> result; + + final AtomicInteger calls = new AtomicInteger(); + + FakeBackend( + DestinationType destination, + Function> result) { + this(destination, (capture, ignored) -> result.apply(capture)); + } + + FakeBackend( + DestinationType destination, + BiFunction> result) { + this.destination = destination; + this.result = result; + } + + static FakeBackend success(DestinationType destination) { + return new FakeBackend(destination, ignored -> CompletableFuture.completedFuture( + DestinationResult.success(destination, destination.name().toLowerCase()))); + } + + @Override + public DestinationType destinationType() { + return destination; + } + + @Override + public CompletionStage createBackup( + BackupCapture capture, + ProgressListener progressListener) { + calls.incrementAndGet(); + return result.apply(capture, progressListener); + } + } + + static final class InMemoryInventoryStore implements WorldInventoryStore { + final Map values = new ConcurrentHashMap<>(); + + IOException loadFailure; + + @Override + public Optional load(WorldId worldId) throws IOException { + if (loadFailure != null) { + throw loadFailure; + } + return Optional.ofNullable(values.get(worldId)); + } + + @Override + public void save(WorldId worldId, WorldInventory inventory) { + values.put(worldId, inventory); + } + } + + static class InMemoryCatalog implements BackupCatalog { + protected final List records = java.util.Collections.synchronizedList(new ArrayList<>()); + + @Override + public void add(BackupRecord record) throws IOException { + records.add(record); + } + + @Override + public Optional find(BackupId backupId) { + return records.stream() + .filter(record -> record.manifest().backupId().equals(backupId)) + .findFirst(); + } + + @Override + public List listAll() { + return List.copyOf(records); + } + + @Override + public List list(WorldId worldId) { + return records.stream() + .filter(record -> record.manifest().worldId().equals(worldId)) + .toList(); + } + + @Override + public Optional update( + BackupId backupId, + UnaryOperator update) { + throw new UnsupportedOperationException(); + } + + @Override + public boolean remove(BackupId backupId) { + return records.removeIf(record -> record.manifest().backupId().equals(backupId)); + } + } + + static final class BlockingCatalog extends InMemoryCatalog { + final CountDownLatch entered = new CountDownLatch(1); + + final CountDownLatch release = new CountDownLatch(1); + + @Override + public void add(BackupRecord record) throws IOException { + entered.countDown(); + try { + if (!release.await(5, TimeUnit.SECONDS)) { + throw new IOException("Timed out waiting to publish test catalog record"); + } + } catch (InterruptedException exception) { + Thread.currentThread().interrupt(); + throw new IOException("Interrupted while publishing test catalog record", exception); + } + super.add(record); + } + } + + static final class UnusedMaintenanceService implements BackupMaintenanceService { + @Override + public CompletionStage> listBackups(Optional worldId) { + return CompletableFuture.failedFuture(new UnsupportedOperationException()); + } + + @Override + public CompletionStage> findBackup(BackupId backupId) { + return CompletableFuture.failedFuture(new UnsupportedOperationException()); + } + + @Override + public CompletionStage restoreBackup( + RestoreBackupRequest request, + ProgressListener progressListener) { + return CompletableFuture.failedFuture(new UnsupportedOperationException()); + } + + @Override + public CompletionStage prepareDelete(BackupId backupId) { + return CompletableFuture.failedFuture(new UnsupportedOperationException()); + } + + @Override + public CompletionStage deleteBackup( + DeleteBackupRequest request, + ProgressListener progressListener) { + return CompletableFuture.failedFuture(new UnsupportedOperationException()); + } + + @Override + public CompletionStage> deleteBackups( + List requests, + ProgressListener progressListener) { + return CompletableFuture.failedFuture(new UnsupportedOperationException()); + } + + @Override + public CompletionStage verifyBackup( + BackupId backupId, + ProgressListener progressListener) { + return CompletableFuture.failedFuture(new UnsupportedOperationException()); + } + + @Override + public CompletionStage syncBackup( + BackupId backupId, + ProgressListener progressListener) { + return CompletableFuture.failedFuture(new UnsupportedOperationException()); + } + + @Override + public CompletionStage> health(Optional worldId) { + return CompletableFuture.failedFuture(new UnsupportedOperationException()); + } + } +} diff --git a/src/test/java/dev/ishaanko/worldarchive/core/SerializedBackupCoordinatorTest.java b/src/test/java/dev/ishaanko/worldarchive/core/SerializedBackupCoordinatorTest.java index d5a5421..3acd65d 100644 --- a/src/test/java/dev/ishaanko/worldarchive/core/SerializedBackupCoordinatorTest.java +++ b/src/test/java/dev/ishaanko/worldarchive/core/SerializedBackupCoordinatorTest.java @@ -7,13 +7,17 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import dev.ishaanko.worldarchive.catalog.BackupCatalog; +import dev.ishaanko.worldarchive.core.CoordinatorFakes.BlockingCatalog; +import dev.ishaanko.worldarchive.core.CoordinatorFakes.FakeBackend; +import dev.ishaanko.worldarchive.core.CoordinatorFakes.FakeCaptureFactory; +import dev.ishaanko.worldarchive.core.CoordinatorFakes.InMemoryCatalog; +import dev.ishaanko.worldarchive.core.CoordinatorFakes.InMemoryInventoryStore; +import dev.ishaanko.worldarchive.core.CoordinatorFakes.UnusedMaintenanceService; import dev.ishaanko.worldarchive.model.BackupId; import dev.ishaanko.worldarchive.model.BackupManifest; -import dev.ishaanko.worldarchive.model.BackupRecord; import dev.ishaanko.worldarchive.model.BackupResult; import dev.ishaanko.worldarchive.model.BackupStatus; import dev.ishaanko.worldarchive.model.BackupTrigger; -import dev.ishaanko.worldarchive.model.DestinationHealth; import dev.ishaanko.worldarchive.model.DestinationResult; import dev.ishaanko.worldarchive.model.DestinationStatus; import dev.ishaanko.worldarchive.model.DestinationType; @@ -22,7 +26,6 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; -import java.security.MessageDigest; import java.time.Clock; import java.time.Instant; import java.time.ZoneOffset; @@ -42,10 +45,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; -import java.util.function.BiFunction; import java.util.function.BooleanSupplier; -import java.util.function.Function; -import java.util.function.UnaryOperator; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -526,6 +526,109 @@ void cancellationIsRejectedAfterDestinationPublicationBegins() throws Exception assertEquals(1, catalog.records.size()); } + @Test + void cancellationDuringDestinationWritesStopsThemAndRecordsNothing() throws Exception { + CompletableFuture writing = new CompletableFuture<>(); + FakeBackend backend = new FakeBackend(DestinationType.ZIP, ignored -> writing); + InMemoryCatalog catalog = new InMemoryCatalog(); + InMemoryInventoryStore inventories = new InMemoryInventoryStore(); + SerializedBackupCoordinator coordinator = coordinator( + catalog, + inventories, + new FakeCaptureFactory(temporaryDirectory.resolve("captures-writing")), + List.of(backend), + BackupCaptureGate.DIRECT, + new LockingWorldOperationGate()); + WorldId worldId = WorldId.create(); + CompletionStage operation = coordinator.createBackup( + request(worldId, "world-writing", BackupTrigger.MANUAL, Optional.empty()), + ProgressListener.NO_OP); + await(() -> backend.calls.get() == 1); + + assertTrue(operation.toCompletableFuture().cancel(true)); + + assertTrue(writing.isCancelled()); + await(() -> !coordinator.isBusy(worldId)); + assertThrows( + java.util.concurrent.CancellationException.class, + () -> operation.toCompletableFuture().join()); + assertEquals(List.of(), catalog.records); + assertEquals(Map.of(), inventories.values); + } + + @Test + void cancellationKeepsAndRecordsDestinationsThatAlreadyFinished() throws Exception { + CompletableFuture gitWriting = new CompletableFuture<>(); + FakeBackend zip = FakeBackend.success(DestinationType.ZIP); + FakeBackend git = new FakeBackend(DestinationType.GIT, ignored -> gitWriting); + InMemoryCatalog catalog = new InMemoryCatalog(); + SerializedBackupCoordinator coordinator = coordinator( + catalog, + new InMemoryInventoryStore(), + new FakeCaptureFactory(temporaryDirectory.resolve("captures-partial")), + List.of(zip, git), + BackupCaptureGate.DIRECT, + new LockingWorldOperationGate()); + WorldId worldId = WorldId.create(); + CompletionStage operation = coordinator.createBackup( + request(worldId, "world-partial", BackupTrigger.MANUAL, Optional.empty()), + ProgressListener.NO_OP); + await(() -> git.calls.get() == 1); + + assertTrue(operation.toCompletableFuture().cancel(true)); + await(() -> !coordinator.isBusy(worldId)); + + assertTrue(operation.toCompletableFuture().isCancelled()); + assertEquals(1, catalog.records.size()); + BackupResult recorded = catalog.records.getFirst().result(); + assertEquals(BackupStatus.PARTIAL_SUCCESS, recorded.status()); + assertEquals( + DestinationStatus.SUCCESS, + destination(recorded, DestinationType.ZIP).status()); + assertEquals( + "Cancelled before this destination finished", + destination(recorded, DestinationType.GIT).message().orElseThrow()); + } + + @Test + void cancellationKeepsAGitSnapshotWhoseSyncWasInterrupted() throws Exception { + CountDownLatch pushing = new CountDownLatch(1); + FakeBackend git = new FakeBackend(DestinationType.GIT, ignored -> AsyncTasks.supplyInterruptible( + coordinatorExecutor, + () -> { + pushing.countDown(); + try { + Thread.sleep(Long.MAX_VALUE); + } catch (InterruptedException exception) { + // The local snapshot exists; only the remote push was cut short. + return DestinationResult.pendingSync( + DestinationType.GIT, "snapshot", "Remote synchronization was cancelled"); + } + return DestinationResult.success(DestinationType.GIT, "snapshot"); + })); + InMemoryCatalog catalog = new InMemoryCatalog(); + SerializedBackupCoordinator coordinator = coordinator( + catalog, + new InMemoryInventoryStore(), + new FakeCaptureFactory(temporaryDirectory.resolve("captures-pending-sync")), + List.of(git), + BackupCaptureGate.DIRECT, + new LockingWorldOperationGate()); + WorldId worldId = WorldId.create(); + CompletionStage operation = coordinator.createBackup( + request(worldId, "world-pending-sync", BackupTrigger.MANUAL, Optional.empty()), + ProgressListener.NO_OP); + assertTrue(pushing.await(5, TimeUnit.SECONDS)); + + assertTrue(operation.toCompletableFuture().cancel(true)); + await(() -> !coordinator.isBusy(worldId)); + + assertEquals(1, catalog.records.size()); + assertEquals( + DestinationStatus.PENDING_SYNC, + destination(catalog.records.getFirst().result(), DestinationType.GIT).status()); + } + @Test void sharedWorldGateBlocksCreateUntilExternalMaintenancePermitCloses() throws Exception { LockingWorldOperationGate operationGate = new LockingWorldOperationGate(); @@ -760,6 +863,13 @@ private static String key(BackupManifest manifest) { return manifest.worldId() + ":" + manifest.label().orElse("none"); } + private static DestinationResult destination(BackupResult result, DestinationType type) { + return result.destinations().stream() + .filter(destination -> destination.destination() == type) + .findFirst() + .orElseThrow(); + } + private static void await(BooleanSupplier condition) throws Exception { long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5); while (!condition.getAsBoolean()) { @@ -769,229 +879,4 @@ private static void await(BooleanSupplier condition) throws Exception { Thread.sleep(10); } } - - private static final class FakeCaptureFactory implements BackupCaptureFactory { - private final Path root; - - private final AtomicInteger calls = new AtomicInteger(); - - private final WorldInventory inventory; - - private volatile java.util.function.Consumer observer = ignored -> { - }; - - private FakeCaptureFactory(Path root) throws Exception { - this.root = root; - byte[] contents = "contents".getBytes(StandardCharsets.UTF_8); - this.inventory = WorldInventory.create(List.of(new WorldInventory.Entry( - "level.dat", - contents.length, - java.util.HexFormat.of().formatHex( - MessageDigest.getInstance("SHA-256").digest(contents))))); - } - - @Override - public CapturedBackup capture( - CreateBackupRequest request, - BackupId backupId, - Instant createdAt, - Optional previousInventory, - CaptureProgressListener progressListener) throws IOException { - observer.accept(request); - Files.createDirectories(root); - Path staging = Files.createDirectory(root.resolve("capture-" + calls.incrementAndGet())); - long changed = previousInventory.map(inventory::changedFilesSince).orElse(inventory.fileCount()); - BackupManifest manifest = BackupManifest.create( - backupId, - request.worldId(), - request.worldName(), - request.label(), - createdAt, - request.trigger(), - inventory.fileCount(), - inventory.byteCount(), - changed, - inventory.contentSha256(), - inventory.inventorySha256()); - return new CapturedBackup( - new BackupCapture(staging, manifest), - inventory, - () -> Files.deleteIfExists(staging)); - } - } - - private static final class FakeBackend implements BackupBackend { - private final DestinationType destination; - - private final BiFunction> result; - - private final AtomicInteger calls = new AtomicInteger(); - - private FakeBackend( - DestinationType destination, - Function> result) { - this(destination, (capture, ignored) -> result.apply(capture)); - } - - private FakeBackend( - DestinationType destination, - BiFunction> result) { - this.destination = destination; - this.result = result; - } - - private static FakeBackend success(DestinationType destination) { - return new FakeBackend(destination, ignored -> CompletableFuture.completedFuture( - DestinationResult.success(destination, destination.name().toLowerCase()))); - } - - @Override - public DestinationType destinationType() { - return destination; - } - - @Override - public CompletionStage createBackup( - BackupCapture capture, - ProgressListener progressListener) { - calls.incrementAndGet(); - return result.apply(capture, progressListener); - } - } - - private static final class InMemoryInventoryStore implements WorldInventoryStore { - private final Map values = new ConcurrentHashMap<>(); - - private IOException loadFailure; - - @Override - public Optional load(WorldId worldId) throws IOException { - if (loadFailure != null) { - throw loadFailure; - } - return Optional.ofNullable(values.get(worldId)); - } - - @Override - public void save(WorldId worldId, WorldInventory inventory) { - values.put(worldId, inventory); - } - } - - private static class InMemoryCatalog implements BackupCatalog { - protected final List records = java.util.Collections.synchronizedList(new ArrayList<>()); - - @Override - public void add(BackupRecord record) throws IOException { - records.add(record); - } - - @Override - public Optional find(BackupId backupId) { - return records.stream() - .filter(record -> record.manifest().backupId().equals(backupId)) - .findFirst(); - } - - @Override - public List listAll() { - return List.copyOf(records); - } - - @Override - public List list(WorldId worldId) { - return records.stream() - .filter(record -> record.manifest().worldId().equals(worldId)) - .toList(); - } - - @Override - public Optional update( - BackupId backupId, - UnaryOperator update) { - throw new UnsupportedOperationException(); - } - - @Override - public boolean remove(BackupId backupId) { - return records.removeIf(record -> record.manifest().backupId().equals(backupId)); - } - } - - private static final class BlockingCatalog extends InMemoryCatalog { - private final CountDownLatch entered = new CountDownLatch(1); - - private final CountDownLatch release = new CountDownLatch(1); - - @Override - public void add(BackupRecord record) throws IOException { - entered.countDown(); - try { - if (!release.await(5, TimeUnit.SECONDS)) { - throw new IOException("Timed out waiting to publish test catalog record"); - } - } catch (InterruptedException exception) { - Thread.currentThread().interrupt(); - throw new IOException("Interrupted while publishing test catalog record", exception); - } - super.add(record); - } - } - - private static final class UnusedMaintenanceService implements BackupMaintenanceService { - @Override - public CompletionStage> listBackups(Optional worldId) { - return CompletableFuture.failedFuture(new UnsupportedOperationException()); - } - - @Override - public CompletionStage> findBackup(BackupId backupId) { - return CompletableFuture.failedFuture(new UnsupportedOperationException()); - } - - @Override - public CompletionStage restoreBackup( - RestoreBackupRequest request, - ProgressListener progressListener) { - return CompletableFuture.failedFuture(new UnsupportedOperationException()); - } - - @Override - public CompletionStage prepareDelete(BackupId backupId) { - return CompletableFuture.failedFuture(new UnsupportedOperationException()); - } - - @Override - public CompletionStage deleteBackup( - DeleteBackupRequest request, - ProgressListener progressListener) { - return CompletableFuture.failedFuture(new UnsupportedOperationException()); - } - - @Override - public CompletionStage> deleteBackups( - List requests, - ProgressListener progressListener) { - return CompletableFuture.failedFuture(new UnsupportedOperationException()); - } - - @Override - public CompletionStage verifyBackup( - BackupId backupId, - ProgressListener progressListener) { - return CompletableFuture.failedFuture(new UnsupportedOperationException()); - } - - @Override - public CompletionStage syncBackup( - BackupId backupId, - ProgressListener progressListener) { - return CompletableFuture.failedFuture(new UnsupportedOperationException()); - } - - @Override - public CompletionStage> health(Optional worldId) { - return CompletableFuture.failedFuture(new UnsupportedOperationException()); - } - } } diff --git a/src/test/java/dev/ishaanko/worldarchive/runtime/BackgroundBackupWarningsTest.java b/src/test/java/dev/ishaanko/worldarchive/runtime/BackgroundBackupWarningsTest.java index 959278f..2c8ed85 100644 --- a/src/test/java/dev/ishaanko/worldarchive/runtime/BackgroundBackupWarningsTest.java +++ b/src/test/java/dev/ishaanko/worldarchive/runtime/BackgroundBackupWarningsTest.java @@ -85,6 +85,24 @@ void worldExitNoticeConfirmsSaveAndReportsBackupOutcome() { new IllegalStateException("save failed"))); } + @Test + void cancelledBackupIsReportedWithoutAWarning() { + Throwable cancelled = new java.util.concurrent.CancellationException("Backup was cancelled"); + + assertTrue(BackgroundBackupWarnings.worldExit(null, cancelled).isEmpty()); + assertTrue(BackgroundBackupWarnings.scheduled(null, cancelled).isEmpty()); + assertNotice( + "Backup cancelled; world was saved", + BackgroundBackupWarnings.NoticeSeverity.WARNING, + BackgroundBackupWarnings.worldExitNotice(null, cancelled)); + assertNotice( + "Backup cancelled; world was saved", + BackgroundBackupWarnings.NoticeSeverity.WARNING, + BackgroundBackupWarnings.worldExitNotice( + null, + new java.util.concurrent.CompletionException(cancelled))); + } + private static void assertNotice( String message, BackgroundBackupWarnings.NoticeSeverity severity, diff --git a/src/test/java/dev/ishaanko/worldarchive/storage/git/WorldGitSnapshotStoreIntegrationTest.java b/src/test/java/dev/ishaanko/worldarchive/storage/git/WorldGitSnapshotStoreIntegrationTest.java index 42a3e71..03f2456 100644 --- a/src/test/java/dev/ishaanko/worldarchive/storage/git/WorldGitSnapshotStoreIntegrationTest.java +++ b/src/test/java/dev/ishaanko/worldarchive/storage/git/WorldGitSnapshotStoreIntegrationTest.java @@ -2,14 +2,17 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import dev.ishaanko.worldarchive.core.AsyncTasks; import dev.ishaanko.worldarchive.core.BackupCapture; import dev.ishaanko.worldarchive.core.ProgressListener; import dev.ishaanko.worldarchive.model.BackupId; import dev.ishaanko.worldarchive.model.BackupManifest; import dev.ishaanko.worldarchive.model.BackupTrigger; +import dev.ishaanko.worldarchive.model.DestinationResult; import dev.ishaanko.worldarchive.model.DestinationStatus; import dev.ishaanko.worldarchive.model.SyncStatus; import dev.ishaanko.worldarchive.model.WorldId; @@ -52,6 +55,23 @@ void requireNativeGitAndLfs() throws Exception { Assumptions.assumeTrue(health.available(), health.summary()); } + @Test + void createBackupReturnsAnInterruptibleWriteSoCancellationKeepsItsOutcome() throws Exception { + WorldId worldId = WorldId.create(); + Path world = world("interruptible-world", "contents"); + try (WorldGitSnapshotStore store = new WorldGitSnapshotStore( + settings(temporaryDirectory.resolve("repositories-interruptible"), Optional.empty()))) { + CompletionStage write = store.createBackup( + capture(world, worldId, BackupId.create(), Instant.now()), + ProgressListener.NO_OP); + + // The coordinator stops a cancelled write through this future type; a composed + // stage would be cancelled outright and lose a snapshot published before the push. + assertInstanceOf(AsyncTasks.InterruptibleFuture.class, write); + assertEquals(DestinationStatus.SUCCESS, await(write).status()); + } + } + @Test void isolatesWorldRepositoriesAndCreatesParentlessSnapshots() throws Exception { Path repositoryRoot = temporaryDirectory.resolve("repositories");