diff --git a/CHANGELOG.md b/CHANGELOG.md index ccb7219..3a099a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,19 @@ # Changelog +## 0.3.7.1 (2026-09-10) + +### Fixed + +- Storage cleanup now deletes the backups it lists. Before, it only removed + the ZIP copy of a backup and left the Git snapshot and the catalog entry in + place, so the backup was still there after the cleanup and after a restart. + A backup the keep settings do not protect is now deleted the same way the + Delete button deletes it: Git snapshot, remote copy, ZIP, and catalog entry. +- Cleanup frees the Git LFS space of deleted snapshots. Only the objects a + remaining snapshot still points at are kept. +- A protected backup that loses its local Git copy keeps its synchronized + remote copy in the catalog, so it stays visible and can be deleted later. + ## 0.3.7 (2026-09-10) ### Added diff --git a/README.md b/README.md index a953c5c..4760526 100644 --- a/README.md +++ b/README.md @@ -89,6 +89,12 @@ Guided cleanup keeps: Within a period, manual backups and backups with more changed files win. Cleanup always shows a preview first. Nothing is deleted until you confirm. +Cleanup deletes the backups it lists the same way the Delete button does: the +Git snapshot, its copy on the configured remote, the ZIP, and the catalog +entry. A protected backup is never deleted. When space is still short after +that, cleanup can drop the local Git copies of protected backups, but only when +each of them keeps a ZIP or a verified remote copy. + ## Minecraft versions Every new backup records the Minecraft version it was made with. The restore diff --git a/gradle.properties b/gradle.properties index 6751749..8ac9949 100644 --- a/gradle.properties +++ b/gradle.properties @@ -9,6 +9,6 @@ loom_version=1.17.19 fabric_api_version=0.157.0+26.2 modmenu_version=20.0.1 -mod_version=0.3.7 +mod_version=0.3.7.1 maven_group=dev.ishaanko automatic_release_channel=alpha diff --git a/src/client/java/dev/ishaanko/worldarchive/ui/CleanupConfirmationScreen.java b/src/client/java/dev/ishaanko/worldarchive/ui/CleanupConfirmationScreen.java index 2a6dba7..1fe3ab3 100644 --- a/src/client/java/dev/ishaanko/worldarchive/ui/CleanupConfirmationScreen.java +++ b/src/client/java/dev/ishaanko/worldarchive/ui/CleanupConfirmationScreen.java @@ -52,7 +52,7 @@ final class CleanupConfirmationScreen extends Screen { BackupClientFacade facade, CleanupPlan plan, Set selected) { - super(Component.literal("Confirm Local Cleanup")); + super(Component.literal("Confirm Cleanup")); this.preview = Objects.requireNonNull(preview, "preview"); this.returnTo = Objects.requireNonNull(returnTo, "returnTo"); this.world = Objects.requireNonNull(world, "world"); @@ -76,7 +76,7 @@ protected void init() { contentWidth, 18, Component.literal( - "These backups will be deleted from this computer. This cannot be undone.") + "These backups will be deleted. This cannot be undone.") .withStyle(ChatFormatting.RED), font)); int pageSize = Math.max(1, Math.min(6, (height - 142) / 24)); @@ -91,8 +91,9 @@ protected void init() { + " · " + item.label().orElse("unlabeled") + " · " - + (item.removeLocalGit() ? "Git " : "") - + (item.removeZip() ? "ZIP" : ""); + + (plan.protectedBackups().contains(item.backupId()) + ? "local Git copy only" + : (item.removeGit() ? "Git " : "") + (item.removeZip() ? "ZIP" : "")); StringWidget row = new StringWidget( x, y, diff --git a/src/client/java/dev/ishaanko/worldarchive/ui/CleanupPreviewScreen.java b/src/client/java/dev/ishaanko/worldarchive/ui/CleanupPreviewScreen.java index afa4bd1..75d2e07 100644 --- a/src/client/java/dev/ishaanko/worldarchive/ui/CleanupPreviewScreen.java +++ b/src/client/java/dev/ishaanko/worldarchive/ui/CleanupPreviewScreen.java @@ -62,7 +62,7 @@ protected void init() { addRenderableOnly(Widgets.title(font, x, 9, contentWidth, 20, title)); String summary = plan.items().isEmpty() ? "Nothing to clean up right now. Your keep settings protect every backup." - : "Choose which backups to delete from this computer. Copies on GitHub or in linked folders are not touched."; + : "Backups your keep settings do not protect are deleted everywhere, including GitHub. Protected backups only lose their local Git copy."; addRenderableOnly(new MultiLineTextWidget( x, 31, @@ -89,7 +89,7 @@ private void addItems(int x, int contentWidth, int pageSize) { + " · " + identity(item) + " · " - + actions(item) + + actions(plan, item) + " · " + item.changedFileCount() + " changed"; @@ -105,9 +105,10 @@ private void addItems(int x, int contentWidth, int pageSize) { } } + /** Protected backups give up their local Git copies together or not at all. */ private void toggle(CleanupItem item) { boolean removing = selected.contains(item.backupId()); - if (!item.removeLocalGit()) { + if (!item.removeGit() || !plan.protectedBackups().contains(item.backupId())) { if (removing) { selected.remove(item.backupId()); } else { @@ -116,8 +117,9 @@ private void toggle(CleanupItem item) { return; } plan.items().stream() - .filter(CleanupItem::removeLocalGit) + .filter(CleanupItem::removeGit) .map(CleanupItem::backupId) + .filter(plan.protectedBackups()::contains) .forEach(backupId -> { if (removing) { selected.remove(backupId); @@ -192,11 +194,14 @@ private static String identity(CleanupItem item) { return item.label().orElse(item.backupId().toString().substring(0, 8)); } - private static String actions(CleanupItem item) { - if (item.removeLocalGit() && item.removeZip()) { - return "Git + ZIP"; + private static String actions(CleanupPlan plan, CleanupItem item) { + if (plan.protectedBackups().contains(item.backupId())) { + return "local Git copy only"; } - return item.removeLocalGit() ? "local Git" : "ZIP"; + if (item.removeGit() && item.removeZip()) { + return "delete Git + ZIP"; + } + return item.removeGit() ? "delete Git" : "delete ZIP"; } static String details(CleanupItem item) { 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 23885e8..b32bc4a 100644 --- a/src/main/java/dev/ishaanko/worldarchive/storage/git/GitBackupBackend.java +++ b/src/main/java/dev/ishaanko/worldarchive/storage/git/GitBackupBackend.java @@ -246,17 +246,18 @@ public CompletionStage deleteLocalSnapshot(WorldId worldId, BackupId ba Objects.requireNonNull(worldId, "worldId"); Objects.requireNonNull(backupId, "backupId"); return submit(() -> lock.withLock(() -> - new GitStorageCompactor(settings, repository, refs, commands) + new GitStorageCompactor(settings, repository, refs, commands, verifier) .deleteLocalSnapshot(worldId, backupId))); } + /** Frees Git and LFS storage that no remaining snapshot in this repository uses. */ public CompletionStage compactStorage(WorldId worldId) { Objects.requireNonNull(worldId, "worldId"); return submit(() -> lock.withLock(() -> { repository.requireWorld(worldId); repository.requireBare(); - boolean noSnapshots = operations.listSnapshotsBlocking(Optional.of(worldId)).isEmpty(); - new GitStorageCompactor(settings, repository, refs, commands).compact(worldId, noSnapshots); + new GitStorageCompactor(settings, repository, refs, commands, verifier) + .compact(worldId, operations.listSnapshotsBlocking(Optional.empty())); return null; })); } diff --git a/src/main/java/dev/ishaanko/worldarchive/storage/git/GitSnapshotVerifier.java b/src/main/java/dev/ishaanko/worldarchive/storage/git/GitSnapshotVerifier.java index 5dabef0..cf8eb56 100644 --- a/src/main/java/dev/ishaanko/worldarchive/storage/git/GitSnapshotVerifier.java +++ b/src/main/java/dev/ishaanko/worldarchive/storage/git/GitSnapshotVerifier.java @@ -207,6 +207,22 @@ private void requireCommitMessage( } } + /** + * Lists the LFS objects a snapshot tree points at without checking that the + * objects exist. Compaction uses this to decide which objects must stay. + */ + List readLfsPointers(String commit) + throws IOException, InterruptedException, GitStorageException { + List pointers = new ArrayList<>(); + for (GitTreeEntry entry : readTreeEntries(commit)) { + if (entry.path().equals(GitBackupBackend.MANIFEST_PATH)) { + continue; + } + readBlob(entry).pointer().ifPresent(pointers::add); + } + return List.copyOf(pointers); + } + private List findAndVerifySnapshotFiles( List treeEntries, BackupManifest manifest) @@ -217,26 +233,9 @@ private List findAndVerifySnapshotFiles( if (entry.path().equals(GitBackupBackend.MANIFEST_PATH)) { continue; } - GitCommandResult contents = commands.run( - List.of( - "cat-file", - "blob", - entry.objectId()), - settings.repository(), - Map.of(), - new byte[0], - LFS_POINTER_OUTPUT_BYTES); - if (!contents.successful()) { - throw new GitStorageException(GitCommands.failureMessage(contents)); - } - if (contents.standardErrorTruncated()) { - throw new GitStorageException( - "Git LFS pointer inspection exceeded its safety limit"); - } - Optional pointer = GitLfsPointer.parse( - entry, - contents.standardOutput(), - contents.standardOutputTruncated()); + SnapshotBlob blob = readBlob(entry); + GitCommandResult contents = blob.contents(); + Optional pointer = blob.pointer(); if (pointer.isPresent()) { verifyLfsObject(pointer.get()); pointers.add(pointer.get()); @@ -255,6 +254,31 @@ private List findAndVerifySnapshotFiles( return List.copyOf(pointers); } + /** Reads one tree blob up to the pointer size limit and parses it as an LFS pointer. */ + private SnapshotBlob readBlob(GitTreeEntry entry) + throws IOException, InterruptedException, GitStorageException { + GitCommandResult contents = commands.run( + List.of( + "cat-file", + "blob", + entry.objectId()), + settings.repository(), + Map.of(), + new byte[0], + LFS_POINTER_OUTPUT_BYTES); + if (!contents.successful()) { + throw new GitStorageException(GitCommands.failureMessage(contents)); + } + if (contents.standardErrorTruncated()) { + throw new GitStorageException( + "Git LFS pointer inspection exceeded its safety limit"); + } + return new SnapshotBlob(contents, GitLfsPointer.parse( + entry, + contents.standardOutput(), + contents.standardOutputTruncated())); + } + private void verifyLfsObject(GitLfsPointer pointer) throws IOException, GitStorageException { Path object = pointer.objectPath(settings.repository()); Path parent = object.getParent(); @@ -279,6 +303,9 @@ private static String sha256(Path path) throws IOException { return Digests.sha256(path); } + private record SnapshotBlob(GitCommandResult contents, Optional pointer) { + } + record VerifiedSnapshot( GitSnapshotManifest manifest, List lfsPointers) { diff --git a/src/main/java/dev/ishaanko/worldarchive/storage/git/GitStorageCompactor.java b/src/main/java/dev/ishaanko/worldarchive/storage/git/GitStorageCompactor.java index 7d3ccc7..d27bfd4 100644 --- a/src/main/java/dev/ishaanko/worldarchive/storage/git/GitStorageCompactor.java +++ b/src/main/java/dev/ishaanko/worldarchive/storage/git/GitStorageCompactor.java @@ -3,15 +3,23 @@ import dev.ishaanko.worldarchive.model.WorldId; import dev.ishaanko.worldarchive.model.BackupId; import java.io.IOException; +import java.nio.file.FileVisitResult; import java.nio.file.Files; import java.nio.file.LinkOption; import java.nio.file.Path; +import java.nio.file.SimpleFileVisitor; +import java.nio.file.attribute.BasicFileAttributes; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Set; +import java.util.regex.Pattern; /** Exact local-only compaction after confirmed snapshot-ref cleanup. */ final class GitStorageCompactor { + private static final Pattern LFS_OBJECT_NAME = Pattern.compile("[0-9a-f]{64}"); + private final GitBackendSettings settings; private final GitRepositoryManager repository; @@ -20,22 +28,41 @@ final class GitStorageCompactor { private final GitCommands commands; + private final GitSnapshotVerifier verifier; + GitStorageCompactor( GitBackendSettings settings, GitRepositoryManager repository, GitRefStore refs, - GitCommands commands) { + GitCommands commands, + GitSnapshotVerifier verifier) { this.settings = Objects.requireNonNull(settings, "settings"); this.repository = Objects.requireNonNull(repository, "repository"); this.refs = Objects.requireNonNull(refs, "refs"); this.commands = Objects.requireNonNull(commands, "commands"); + this.verifier = Objects.requireNonNull(verifier, "verifier"); } - void compact(WorldId worldId, boolean noSnapshots) + /** + * Reclaims space after snapshot refs were deliberately deleted. Snapshot commits + * chain onto each other, so Git's own reachability keeps every older commit alive + * and {@code git lfs prune} cannot tell a deleted snapshot from a kept one. The LFS + * objects that stay are therefore computed exactly: every object that a remaining + * snapshot's tree points at. Anything else under {@code lfs/objects} is removed. + * + *

The world's history ref exists so an import can rebuild snapshot refs that + * went missing by accident. After a deletion it would rebuild the deleted snapshot + * with its content gone, so it is removed here as well. + */ + void compact(WorldId worldId, List remainingSnapshots) throws IOException, InterruptedException, GitStorageException { - if (noSnapshots) { - refs.deleteIfPresent(repository.historyRef(worldId)); + Set retained = new HashSet<>(); + for (GitSnapshot snapshot : remainingSnapshots) { + for (GitLfsPointer pointer : verifier.readLfsPointers(snapshot.commitId())) { + retained.add(pointer.sha256()); + } } + refs.deleteIfPresent(repository.historyRef(worldId)); commands.checked( List.of( "reflog", @@ -52,18 +79,7 @@ void compact(WorldId worldId, boolean noSnapshots) settings.repository(), Map.of(), new byte[0]); - if (noSnapshots) { - deleteAllLfsObjects(); - } else { - commands.checked( - List.of( - "lfs", - "prune", - "--force"), - settings.repository(), - Map.of(), - new byte[0]); - } + deleteUnreferencedLfsObjects(retained); } boolean deleteLocalSnapshot(WorldId worldId, BackupId backupId) @@ -79,11 +95,34 @@ boolean deleteLocalSnapshot(WorldId worldId, BackupId backupId) return true; } - private void deleteAllLfsObjects() throws GitStorageException { + private void deleteUnreferencedLfsObjects(Set retained) + throws IOException, GitStorageException { Path lfsObjects = settings.repository().resolve("lfs").resolve("objects"); - GitTemporaryFiles.deleteTree(lfsObjects); - if (Files.exists(lfsObjects, LinkOption.NOFOLLOW_LINKS)) { - throw new GitStorageException("Unreferenced Git LFS objects could not be removed"); + if (!Files.isDirectory(lfsObjects, LinkOption.NOFOLLOW_LINKS)) { + return; + } + for (String sha256 : retained) { + Path object = lfsObjects.resolve(sha256.substring(0, 2)) + .resolve(sha256.substring(2, 4)) + .resolve(sha256); + if (!Files.isRegularFile(object, LinkOption.NOFOLLOW_LINKS)) { + throw new GitStorageException( + "A Git LFS object needed by a remaining snapshot is missing"); + } } + Files.walkFileTree(lfsObjects, new SimpleFileVisitor<>() { + @Override + public FileVisitResult visitFile(Path file, BasicFileAttributes attributes) + throws IOException { + String name = file.getFileName().toString(); + if (attributes.isRegularFile() + && !attributes.isSymbolicLink() + && LFS_OBJECT_NAME.matcher(name).matches() + && !retained.contains(name)) { + Files.delete(file); + } + return FileVisitResult.CONTINUE; + } + }); } } diff --git a/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupExecutor.java b/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupExecutor.java index 5f9c939..73e0911 100644 --- a/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupExecutor.java +++ b/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupExecutor.java @@ -25,7 +25,11 @@ import java.util.Optional; import java.util.Set; -/** Applies a confirmed {@link CleanupPlan}, including its remote-safety guardrails. */ +/** + * Applies a confirmed {@link CleanupPlan}. A backup the keep settings do not protect is + * deleted the same way the Delete button deletes it: Git snapshot (local and remote), + * ZIP, and catalog record. A protected backup only loses its local Git copy. + */ final class CleanupExecutor { private final BackupCatalog catalog; @@ -60,12 +64,10 @@ CleanupResult apply(CleanupPlan plan, CleanupRequest request) throws Exception { throw new IOException("Storage changed after the preview; review cleanup again"); } requireVerifiedSafetyFloor(current, plan.verifiedSafetyFloor()); - Set remoteCopies = requireCurrentRemoteCopies( - plan, request, current); + requireCurrentRemoteCopies(plan, request, current); long before = current.totalBytes(); Map failures = new LinkedHashMap<>(); - boolean removedGit = applyItems( - plan, request, current, remoteCopies, failures); + boolean removedGit = applyItems(plan, request, current, failures); if (removedGit) { try { ManagedStorageSupport.await(git.compactCurrentStorage(plan.worldId())); @@ -75,7 +77,7 @@ CleanupResult apply(CleanupPlan plan, CleanupRequest request) throws Exception { } catch (Exception exception) { failures.putIfAbsent( plan.items().stream() - .filter(CleanupItem::removeLocalGit) + .filter(CleanupItem::removeGit) .map(CleanupItem::backupId) .findFirst() .orElseThrow(), @@ -106,9 +108,12 @@ private static void validateSelection( || removesSafetyFloor) { throw new IOException("Cleanup selection does not match its preview"); } + // Protected backups lose their local Git copies as one group: the space only + // comes back once none of them keeps the shared history alive. Set gitGroup = plan.items().stream() - .filter(CleanupItem::removeLocalGit) + .filter(CleanupItem::removeGit) .map(CleanupItem::backupId) + .filter(plan.protectedBackups()::contains) .collect(java.util.stream.Collectors.toSet()); boolean someGitSelected = request.selectedBackups().stream() .anyMatch(gitGroup::contains); @@ -118,11 +123,14 @@ private static void validateSelection( } } + /** + * Git goes first: a remote that refuses the deletion fails the item before its ZIP + * is touched, so the backup stays whole and restorable. + */ private boolean applyItems( CleanupPlan plan, CleanupRequest request, Snapshot current, - Set remoteCopies, Map failures) throws InterruptedException { boolean removedGit = false; for (CleanupItem item : plan.items()) { @@ -130,18 +138,28 @@ private boolean applyItems( continue; } try { + if (item.removeGit()) { + if (plan.protectedBackups().contains(item.backupId())) { + ManagedStorageSupport.await(git.deleteCurrentLocalSnapshot( + plan.worldId(), + item.backupId())); + // The catalog keeps pointing at a synchronized remote copy, so + // the backup stays visible, verifiable, and deletable later. + removeDestination( + item.backupId(), + DestinationType.GIT, + synchronizedRemoteCopy(current, item.backupId())); + } else { + ManagedStorageSupport.await(git.deleteSnapshot( + plan.worldId(), + item.backupId())); + removeDestination(item.backupId(), DestinationType.GIT, false); + } + removedGit = true; + } if (item.removeZip()) { removeZip(current, item.backupId()); } - if (item.removeLocalGit()) { - ManagedStorageSupport.await(git.deleteCurrentLocalSnapshot( - plan.worldId(), - item.backupId())); - removeGitCatalogCopy( - item.backupId(), - remoteCopies.contains(item.backupId())); - removedGit = true; - } } catch (InterruptedException exception) { Thread.currentThread().interrupt(); throw exception; @@ -248,14 +266,22 @@ private boolean currentRemoteContainsSnapshot(Snapshot snapshot, BackupId backup } } - private Set requireCurrentRemoteCopies( + private static boolean synchronizedRemoteCopy(Snapshot snapshot, BackupId backupId) { + return ManagedStorageSupport.destination( + ManagedStorageSupport.record(snapshot, backupId), DestinationType.GIT) + .filter(result -> result.ownership() == ArtifactOwnership.MANAGED + && result.syncStatus() == SyncStatus.SYNCED) + .isPresent(); + } + + /** A protected backup may lose its last local copy only if the remote provably has it. */ + private void requireCurrentRemoteCopies( CleanupPlan plan, CleanupRequest request, Snapshot snapshot) throws Exception { - Set remoteCopies = new HashSet<>(); for (CleanupItem item : plan.items()) { if (!request.selectedBackups().contains(item.backupId()) - || !item.removeLocalGit() + || !item.removeGit() || !plan.protectedBackups().contains(item.backupId())) { continue; } @@ -280,9 +306,7 @@ private Set requireCurrentRemoteCopies( throw new IOException( "The configured remote changed after the cleanup preview"); } - remoteCopies.add(item.backupId()); } - return Set.copyOf(remoteCopies); } private void removeZip(Snapshot snapshot, BackupId backupId) throws Exception { @@ -319,13 +343,6 @@ private void restoreRecord(BackupRecord previous, Exception deletionFailure) { } } - private void removeGitCatalogCopy( - BackupId backupId, - boolean verifiedRemoteRemains) throws IOException { - removeDestination( - backupId, DestinationType.GIT, verifiedRemoteRemains); - } - private void removeDestination( BackupId backupId, DestinationType type, diff --git a/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupItem.java b/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupItem.java index 54e1c7a..324b265 100644 --- a/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupItem.java +++ b/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupItem.java @@ -5,13 +5,18 @@ import java.util.Objects; import java.util.Optional; -/** One exact local cleanup action shown before confirmation. */ +/** + * One cleanup action shown before confirmation. For a backup the keep settings do not + * protect, {@code removeGit} deletes the Git snapshot everywhere, including the + * configured remote, and the backup leaves the catalog. For a protected backup it drops + * only the local Git copy; the ZIP or the verified remote copy stays. + */ public record CleanupItem( BackupId backupId, Instant createdAt, Optional label, long changedFileCount, - boolean removeLocalGit, + boolean removeGit, boolean removeZip, Optional gitRef, Optional zipArtifactId, @@ -27,10 +32,10 @@ public record CleanupItem( if (changedFileCount < 0 || estimatedGitBytes < 0 || exactZipBytes < 0) { throw new IllegalArgumentException("Cleanup counts must not be negative"); } - if (!removeLocalGit && !removeZip) { + if (!removeGit && !removeZip) { throw new IllegalArgumentException("Cleanup item must remove at least one local artifact"); } - if (removeLocalGit != gitRef.isPresent() || removeZip != zipArtifactId.isPresent()) { + if (removeGit != gitRef.isPresent() || removeZip != zipArtifactId.isPresent()) { throw new IllegalArgumentException("Cleanup artifact identities do not match their actions"); } } diff --git a/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupPlanner.java b/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupPlanner.java index 7bbfb0f..e3bb7c2 100644 --- a/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupPlanner.java +++ b/src/main/java/dev/ishaanko/worldarchive/storage/management/CleanupPlanner.java @@ -44,6 +44,12 @@ final class CleanupPlanner { this.zoneId = Objects.requireNonNull(zoneId, "zoneId"); } + /** + * Picks the lightest backups the keep settings do not protect and deletes each one + * completely. Only when that still leaves the world over its target does the plan + * drop the local Git copies of protected backups, and only when every one of them + * keeps a ZIP or a verified remote copy. + */ CleanupPlan prepare(WorldId worldId, Snapshot snapshot) throws Exception { BackupId safetyFloor = verifiedSafetyFloor(snapshot) .orElseThrow(() -> new IOException( @@ -59,55 +65,19 @@ CleanupPlan prepare(WorldId worldId, Snapshot snapshot) throws Exception { protectedIds); long target = Math.multiplyExact(policy.budgetBytes(), CLEANUP_TARGET_BUDGET_NUMERATOR) / CLEANUP_TARGET_BUDGET_DENOMINATOR; - long projected = snapshot.totalBytes(); + long gitShare = snapshot.localGitSnapshots().isEmpty() + ? 0 + : snapshot.gitBytes() / snapshot.localGitSnapshots().size(); Map selected = new LinkedHashMap<>(); - for (BackupRecord record : cleanupOrder) { - if (projected <= target) { - break; - } - ZipBackupArtifact zip = snapshot.zipArtifacts().get( - record.manifest().backupId()); - if (zip == null - || !ManagedStorageSupport.managedDestination(record, DestinationType.ZIP)) { - continue; + long projected = deleteUnprotected( + snapshot, cleanupOrder, target, gitShare, selected); + if (projected > target) { + Optional> remoteCopies = executor.protectedRemoteCopiesForGitRemoval( + snapshot, protectedIds, safetyFloor); + if (remoteCopies.isPresent()) { + projected = evictRemainingLocalGit( + snapshot, remoteCopies.orElseThrow(), gitShare, projected, selected); } - long bytes = ManagedStorageSupport.artifactBytes(zip); - CleanupItem item = item(record, new CleanupItemFlags(false, true, false, 0, bytes)); - selected.put(record.manifest().backupId(), item); - projected = Math.max(0, projected - bytes); - } - - Optional> protectedRemoteCopies = projected > target - ? executor.protectedRemoteCopiesForGitRemoval( - snapshot, protectedIds, safetyFloor) - : Optional.empty(); - boolean removeCompleteGit = protectedRemoteCopies.isPresent(); - if (removeCompleteGit) { - Set remoteCopies = protectedRemoteCopies.orElseThrow(); - List localGitIds = snapshot.localGitSnapshots().keySet().stream() - .sorted() - .toList(); - long quotient = localGitIds.isEmpty() - ? 0 - : snapshot.gitBytes() / localGitIds.size(); - long remainder = localGitIds.isEmpty() - ? 0 - : snapshot.gitBytes() % localGitIds.size(); - for (int index = 0; index < localGitIds.size(); index++) { - BackupId backupId = localGitIds.get(index); - BackupRecord record = ManagedStorageSupport.record(snapshot, backupId); - CleanupItem previous = selected.get(backupId); - long estimate = quotient + (index == 0 ? remainder : 0); - selected.put(backupId, item( - record, - new CleanupItemFlags( - true, - previous != null && previous.removeZip(), - remoteCopies.contains(backupId), - estimate, - previous == null ? 0 : previous.exactZipBytes()))); - } - projected = Math.max(0, projected - snapshot.gitBytes()); } OperationId token = OperationId.create(); @@ -126,6 +96,68 @@ CleanupPlan prepare(WorldId worldId, Snapshot snapshot) throws Exception { snapshot.fingerprint()); } + /** Adds whole-backup deletions, lightest first, until the target is met. */ + private static long deleteUnprotected( + Snapshot snapshot, + List cleanupOrder, + long target, + long gitShare, + Map selected) throws IOException { + long projected = snapshot.totalBytes(); + for (BackupRecord record : cleanupOrder) { + if (projected <= target) { + break; + } + BackupId backupId = record.manifest().backupId(); + ZipBackupArtifact zip = snapshot.zipArtifacts().get(backupId); + boolean removeZip = zip != null + && ManagedStorageSupport.managedDestination(record, DestinationType.ZIP); + boolean removeGit = ManagedStorageSupport.managedDestination( + record, DestinationType.GIT); + if (!removeZip && !removeGit) { + continue; + } + long zipBytes = removeZip ? ManagedStorageSupport.artifactBytes(zip) : 0; + long gitBytes = removeGit && snapshot.localGitSnapshots().containsKey(backupId) + ? gitShare + : 0; + selected.put(backupId, item( + record, + new CleanupItemFlags(removeGit, removeZip, false, gitBytes, zipBytes))); + projected = Math.max(0, projected - zipBytes - gitBytes); + } + return projected; + } + + /** Drops the local Git copies that remain after whole-backup deletions, as one group. */ + private static long evictRemainingLocalGit( + Snapshot snapshot, + Set remoteCopies, + long gitShare, + long projected, + Map selected) { + List remainingGitIds = snapshot.localGitSnapshots().keySet().stream() + .filter(backupId -> !selected.containsKey(backupId) + || !selected.get(backupId).removeGit()) + .sorted() + .toList(); + long remaining = projected; + for (BackupId backupId : remainingGitIds) { + BackupRecord record = ManagedStorageSupport.record(snapshot, backupId); + CleanupItem previous = selected.get(backupId); + selected.put(backupId, item( + record, + new CleanupItemFlags( + true, + previous != null && previous.removeZip(), + remoteCopies.contains(backupId), + gitShare, + previous == null ? 0 : previous.exactZipBytes()))); + remaining = Math.max(0, remaining - gitShare); + } + return remaining; + } + private static Optional verifiedSafetyFloor(Snapshot snapshot) { return snapshot.records().stream() .filter(record -> hasVerifiedLocalArtifact(snapshot, record)) diff --git a/src/main/java/dev/ishaanko/worldarchive/storage/management/ManagedStorageService.java b/src/main/java/dev/ishaanko/worldarchive/storage/management/ManagedStorageService.java index c177608..ac94ca7 100644 --- a/src/main/java/dev/ishaanko/worldarchive/storage/management/ManagedStorageService.java +++ b/src/main/java/dev/ishaanko/worldarchive/storage/management/ManagedStorageService.java @@ -19,7 +19,7 @@ import java.util.concurrent.Executor; import java.util.function.Supplier; -/** Measures, forecasts, previews, and explicitly applies managed-local cleanup. */ +/** Measures, forecasts, previews, and explicitly applies storage cleanup. */ public final class ManagedStorageService { private final FileStorageReviewStore reviews; diff --git a/src/test/java/dev/ishaanko/worldarchive/storage/management/CleanupIntegrationTest.java b/src/test/java/dev/ishaanko/worldarchive/storage/management/CleanupIntegrationTest.java new file mode 100644 index 0000000..782758c --- /dev/null +++ b/src/test/java/dev/ishaanko/worldarchive/storage/management/CleanupIntegrationTest.java @@ -0,0 +1,319 @@ +package dev.ishaanko.worldarchive.storage.management; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import dev.ishaanko.worldarchive.catalog.FileBackupCatalog; +import dev.ishaanko.worldarchive.catalog.FileBackupDeletionRegistry; +import dev.ishaanko.worldarchive.config.GitDestinationConfig; +import dev.ishaanko.worldarchive.config.StoragePolicy; +import dev.ishaanko.worldarchive.config.TriggerConfig; +import dev.ishaanko.worldarchive.config.WorldArchiveConfig; +import dev.ishaanko.worldarchive.config.WorldConfig; +import dev.ishaanko.worldarchive.config.ZipDestinationConfig; +import dev.ishaanko.worldarchive.core.BackupCapture; +import dev.ishaanko.worldarchive.core.ProgressListener; +import dev.ishaanko.worldarchive.core.WorldInventory; +import dev.ishaanko.worldarchive.importing.FileBackupImportService; +import dev.ishaanko.worldarchive.importing.FileImportSourceRegistry; +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.BackupTrigger; +import dev.ishaanko.worldarchive.model.DestinationResult; +import dev.ishaanko.worldarchive.model.DestinationType; +import dev.ishaanko.worldarchive.model.SyncStatus; +import dev.ishaanko.worldarchive.model.VerificationStatus; +import dev.ishaanko.worldarchive.model.WorldId; +import dev.ishaanko.worldarchive.storage.git.GitBackendSettings; +import dev.ishaanko.worldarchive.storage.git.SystemGitCommandRunner; +import dev.ishaanko.worldarchive.storage.git.WorldGitSnapshotStore; +import dev.ishaanko.worldarchive.storage.zip.ZipBackupArtifact; +import dev.ishaanko.worldarchive.storage.zip.ZipBackupStore; +import dev.ishaanko.worldarchive.storage.zip.ZipBackupStoreResolver; +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.Duration; +import java.time.Instant; +import java.time.ZoneOffset; +import java.util.ArrayList; +import java.util.HexFormat; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.concurrent.CompletionStage; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** Cleanup against real Git and Git LFS: what is deleted, what stays, what survives a restart. */ +final class CleanupIntegrationTest { + private static final Instant NOW = Instant.parse("2026-09-10T12:00:00Z"); + + /** One byte of budget forces cleanup to go as far as the keep settings allow. */ + private static final StoragePolicy KEEP_ONE_DAILY = new StoragePolicy(1, 1, 0, 0); + + private static final StoragePolicy KEEP_ONLY_SAFETY_FLOOR = new StoragePolicy(1, 0, 0, 0); + + @TempDir + Path temporaryDirectory; + + private final ExecutorService executor = Executors.newCachedThreadPool(); + + private WorldId worldId; + + private Path remote; + + private WorldGitSnapshotStore git; + + private ZipBackupStore zipStore; + + private FileBackupCatalog catalog; + + private FileBackupDeletionRegistry deletions; + + @BeforeEach + void setUp() throws Exception { + worldId = WorldId.create(); + remote = temporaryDirectory.resolve("remote.git"); + nativeGit("init", "--bare", remote.toString()); + git = new WorldGitSnapshotStore( + new GitBackendSettings( + true, + temporaryDirectory.resolve("git"), + "git", + "origin", + Optional.empty(), + GitDestinationConfig.DEFAULT_LFS_PATTERNS, + GitBackendSettings.DEFAULT_COMMAND_TIMEOUT, + GitBackendSettings.DEFAULT_MAXIMUM_OUTPUT_BYTES), + Optional.empty(), + Map.of(worldId, remote.toUri().toString()), + new SystemGitCommandRunner(), + executor); + Assumptions.assumeTrue(await(git.probeTools()).available(), "git and git-lfs required"); + zipStore = new ZipBackupStore(temporaryDirectory.resolve("archives")); + catalog = new FileBackupCatalog(temporaryDirectory.resolve("catalog.json")); + deletions = new FileBackupDeletionRegistry(temporaryDirectory.resolve("deleted.txt")); + } + + @AfterEach + void tearDown() { + git.close(); + executor.shutdownNow(); + } + + @Test + void unprotectedBackupsAreDeletedEverywhereAndTheirLfsObjectsFreed() throws Exception { + BackupId oldest = backup(3, true); + BackupId middle = backup(2, true); + BackupId newest = backup(1, true); + assertEquals(6, lfsObjectCount()); + + ManagedStorageService service = service(KEEP_ONE_DAILY); + CleanupPlan plan = await(service.prepareCleanup(worldId)); + assertEquals(Set.of(newest), plan.protectedBackups()); + Map items = plan.items().stream() + .collect(Collectors.toMap(CleanupItem::backupId, item -> item)); + for (BackupId deleted : List.of(oldest, middle)) { + assertTrue(items.get(deleted).removeGit() && items.get(deleted).removeZip()); + assertTrue(items.get(deleted).removesRestorePoint()); + } + assertFalse(items.get(newest).removesRestorePoint(), + "the protected backup is only offered as a local Git eviction"); + + CleanupResult result = await(service.applyCleanup( + new CleanupRequest(plan.confirmationToken(), Set.of(oldest, middle)))); + assertEquals(Map.of(), result.failures()); + + assertEquals(List.of(newest), catalog.list(worldId).stream() + .map(record -> record.manifest().backupId()).toList()); + assertEquals(Set.of(newest), await(git.listCurrentSnapshots(worldId)).stream() + .map(snapshot -> snapshot.backupId()).collect(Collectors.toSet())); + assertEquals(Set.of(newest), remoteBackupIds()); + assertEquals(1, zipStore.listArchives().size()); + assertTrue(deletions.contains(oldest) && deletions.contains(middle)); + assertEquals(2, lfsObjectCount(), "only the newest snapshot's objects remain"); + assertTrue(await(git.verifyCurrentSnapshot(worldId, newest)).valid()); + assertTrue(result.reclaimedBytes() > 0); + + imports().rebuildLocal().toCompletableFuture().get(30, TimeUnit.SECONDS); + assertEquals(List.of(newest), catalog.list(worldId).stream() + .map(record -> record.manifest().backupId()).toList()); + } + + @Test + void protectedBackupKeepsItsRemoteCopyWhenLocalGitIsEvicted() throws Exception { + BackupId older = backup(2, true); + BackupId newest = backup(1, false); + + ManagedStorageService service = service(KEEP_ONLY_SAFETY_FLOOR); + CleanupPlan plan = await(service.prepareCleanup(worldId)); + assertEquals(Set.of(older), plan.protectedBackups(), + "the verified safety floor is the only backup with a ZIP"); + Map items = plan.items().stream() + .collect(Collectors.toMap(CleanupItem::backupId, item -> item)); + assertTrue(items.get(newest).removeGit() && !items.get(newest).removeZip()); + assertTrue(items.get(newest).removesRestorePoint()); + assertTrue(items.get(older).removeGit() && !items.get(older).removeZip()); + assertFalse(items.get(older).removesRestorePoint()); + + CleanupResult result = await(service.applyCleanup( + new CleanupRequest(plan.confirmationToken(), items.keySet()))); + assertEquals(Map.of(), result.failures()); + + assertEquals(Set.of(older), remoteBackupIds(), "the unprotected backup left the remote"); + assertTrue(await(git.listCurrentSnapshots(worldId)).isEmpty()); + assertEquals(0, lfsObjectCount()); + BackupRecord kept = catalog.find(older).orElseThrow(); + assertEquals( + List.of(DestinationType.GIT, DestinationType.ZIP), + kept.result().destinations().stream() + .map(DestinationResult::destination).toList()); + assertFalse(catalog.find(newest).isPresent()); + } + + private BackupId backup(int daysAgo, boolean withZip) throws Exception { + BackupId backupId = BackupId.create(); + Path source = Files.createDirectories(temporaryDirectory.resolve("world-" + backupId)); + byte[] level = ("level " + backupId).getBytes(StandardCharsets.UTF_8); + byte[] region = ("region " + backupId).getBytes(StandardCharsets.UTF_8); + Files.write(source.resolve("level.dat"), level); + Files.write(source.resolve("r.0.0.mca"), region); + WorldInventory inventory = WorldInventory.create(List.of( + new WorldInventory.Entry("level.dat", level.length, sha256(level)), + new WorldInventory.Entry("r.0.0.mca", region.length, sha256(region)))); + BackupManifest manifest = BackupManifest.create( + backupId, + worldId, + "Cleanup World", + Optional.empty(), + NOW.minus(Duration.ofDays(daysAgo)), + BackupTrigger.SCHEDULED, + inventory.fileCount(), + inventory.byteCount(), + inventory.fileCount(), + inventory.contentSha256(), + inventory.inventorySha256()); + BackupCapture capture = new BackupCapture(source, manifest); + DestinationResult gitResult = await(git.createBackup(capture, ProgressListener.NO_OP)); + assertEquals(SyncStatus.SYNCED, gitResult.syncStatus()); + List destinations = new ArrayList<>(); + destinations.add(gitResult); + if (withZip) { + ZipBackupArtifact artifact = zipStore.create(capture); + destinations.add(DestinationResult.success(DestinationType.ZIP, artifact.artifactId()) + .withVerification(VerificationStatus.VERIFIED)); + } + catalog.add(new BackupRecord( + manifest, + BackupResult.aggregate(backupId, worldId, destinations, NOW))); + return backupId; + } + + private ManagedStorageService service(StoragePolicy policy) { + WorldConfig world = new WorldConfig( + worldId, + true, + temporaryDirectory.resolve("live-world"), + Optional.of(remote.toUri().toString()), + Optional.empty(), + policy); + WorldArchiveConfig config = new WorldArchiveConfig( + WorldArchiveConfig.CURRENT_SCHEMA_VERSION, + TriggerConfig.defaults(), + GitDestinationConfig.defaults(), + ZipDestinationConfig.defaults(), + List.of(world)); + return new ManagedStorageService( + () -> config, + catalog, + deletions, + git, + zipStore, + new FileStorageHistoryStore(temporaryDirectory.resolve("history")), + new FileStorageReviewStore(temporaryDirectory.resolve("reviews")), + ignored -> () -> { + }, + executor, + Clock.fixed(NOW, ZoneOffset.UTC), + ZoneOffset.UTC); + } + + /** The catalog rebuild that runs at every game start. */ + private FileBackupImportService imports() { + ZipBackupStoreResolver stores = new ZipBackupStoreResolver() { + @Override + public ZipBackupStore store(WorldId ignored) { + return zipStore; + } + + @Override + public ZipBackupStore defaultStore() { + return zipStore; + } + }; + return new FileBackupImportService( + catalog, + new FileImportSourceRegistry(temporaryDirectory.resolve("sources.json")), + deletions, + git, + stores, + () -> Set.of(worldId), + executor); + } + + private Set remoteBackupIds() throws Exception { + return nativeGit("--git-dir=" + remote, "for-each-ref", "--format=%(refname)", + "refs/heads/backups/") + .lines() + .filter(line -> !line.isBlank()) + .map(line -> BackupId.parse(line.substring(line.length() - 36))) + .collect(Collectors.toSet()); + } + + private long lfsObjectCount() throws Exception { + Path objects = git.repositoryFor(worldId).resolve("lfs").resolve("objects"); + if (!Files.isDirectory(objects)) { + return 0; + } + try (Stream paths = Files.walk(objects)) { + return paths.filter(Files::isRegularFile).count(); + } + } + + private String nativeGit(String... arguments) throws Exception { + List command = new ArrayList<>(); + command.add("git"); + command.addAll(List.of(arguments)); + Process process = new ProcessBuilder(command) + .directory(temporaryDirectory.toFile()) + .redirectErrorStream(true) + .start(); + String output = new String(process.getInputStream().readAllBytes(), StandardCharsets.UTF_8); + assertTrue(process.waitFor(30, TimeUnit.SECONDS)); + assertEquals(0, process.exitValue(), output); + return output; + } + + private static T await(CompletionStage stage) throws Exception { + return stage.toCompletableFuture().get(60, TimeUnit.SECONDS); + } + + private static String sha256(byte[] bytes) throws Exception { + return HexFormat.of().formatHex(MessageDigest.getInstance("SHA-256").digest(bytes)); + } +} diff --git a/src/test/java/dev/ishaanko/worldarchive/storage/management/ManagedStorageServiceTest.java b/src/test/java/dev/ishaanko/worldarchive/storage/management/ManagedStorageServiceTest.java index 2374c78..2802b44 100644 --- a/src/test/java/dev/ishaanko/worldarchive/storage/management/ManagedStorageServiceTest.java +++ b/src/test/java/dev/ishaanko/worldarchive/storage/management/ManagedStorageServiceTest.java @@ -350,7 +350,7 @@ void staleSynchronizedStatusCannotAuthorizeLastLocalGitCopyRemoval() CleanupPlan plan = await(fixture.service().prepareCleanup(worldId)); assertTrue(plan.items().stream() - .noneMatch(CleanupItem::removeLocalGit)); + .noneMatch(CleanupItem::removeGit)); assertTrue(Files.exists(repository.resolve("objects.bin"))); } }