From 821b497a256e0c2825a78c0b070ee3398f79a509 Mon Sep 17 00:00:00 2001 From: Keuin Date: Thu, 21 Jan 2021 12:13:48 +0800 Subject: Fix minor bug in ZipUtil. Do some clean if backup is failed. Small code refactor. --- .../kbackupfabric/backup/BackupFilesystemUtil.java | 2 +- .../identifier/StorageObjectLoader.java | 4 +- .../kbackupfabric/operation/BackupOperation.java | 8 ++-- .../backup/feedback/FailedBackupFeedback.java | 20 ++++++++ .../backup/feedback/SuccessBackupFeedback.java | 35 ++++++++++++++ .../method/ConfiguredIncrementalBackupMethod.java | 56 +++++++++++++++------- .../method/ConfiguredPrimitiveBackupMethod.java | 25 ++++++++-- .../java/com/keuin/kbackupfabric/util/ZipUtil.java | 36 ++++++++------ 8 files changed, 143 insertions(+), 43 deletions(-) create mode 100644 src/main/java/com/keuin/kbackupfabric/operation/backup/feedback/FailedBackupFeedback.java create mode 100644 src/main/java/com/keuin/kbackupfabric/operation/backup/feedback/SuccessBackupFeedback.java diff --git a/src/main/java/com/keuin/kbackupfabric/backup/BackupFilesystemUtil.java b/src/main/java/com/keuin/kbackupfabric/backup/BackupFilesystemUtil.java index 49fe6a0..eac7ce9 100644 --- a/src/main/java/com/keuin/kbackupfabric/backup/BackupFilesystemUtil.java +++ b/src/main/java/com/keuin/kbackupfabric/backup/BackupFilesystemUtil.java @@ -53,7 +53,7 @@ public final class BackupFilesystemUtil { public static String getLevelPath(MinecraftServer server) throws IOException { if (!(server instanceof MinecraftDedicatedServer)) throw new IllegalStateException("This plugin is server-side only."); - String path = (new File(server.getRunDirectory().getCanonicalPath(), ((MinecraftDedicatedServer) server).getLevelName())).getAbsolutePath(); + String path = (new File(server.getRunDirectory().getCanonicalPath(), server.getLevelName())).getAbsolutePath(); Logger.getLogger("getLevelPath").info(String.format("Level path: %s", path)); assert (new File(path)).exists(); return path; diff --git a/src/main/java/com/keuin/kbackupfabric/backup/incremental/identifier/StorageObjectLoader.java b/src/main/java/com/keuin/kbackupfabric/backup/incremental/identifier/StorageObjectLoader.java index 55dd6bd..bf5baf8 100644 --- a/src/main/java/com/keuin/kbackupfabric/backup/incremental/identifier/StorageObjectLoader.java +++ b/src/main/java/com/keuin/kbackupfabric/backup/incremental/identifier/StorageObjectLoader.java @@ -16,10 +16,8 @@ public class StorageObjectLoader { ObjectIdentifier identifier; identifier = Sha256Identifier.fromFileName(fileName); - if (identifier != null) - return identifier; + return identifier; // Add more identifiers. - return null; } } diff --git a/src/main/java/com/keuin/kbackupfabric/operation/BackupOperation.java b/src/main/java/com/keuin/kbackupfabric/operation/BackupOperation.java index b38921d..0d9cb65 100644 --- a/src/main/java/com/keuin/kbackupfabric/operation/BackupOperation.java +++ b/src/main/java/com/keuin/kbackupfabric/operation/BackupOperation.java @@ -33,6 +33,7 @@ public class BackupOperation extends InvokableAsyncBlockingOperation { protected void async() { String backupSaveDirectory = ""; MinecraftServer server = context.getSource().getMinecraftServer(); + boolean success = false; // only success when everything is done try { //// Do our main backup logic @@ -44,11 +45,12 @@ public class BackupOperation extends InvokableAsyncBlockingOperation { // Backup BackupFeedback result = configuredBackupMethod.backup(); - if (result.isSuccess()) { - // Restore old auto-save switch stat + success = result.isSuccess(); + if (success) { + // Restore previous auto-save switch stat server.getWorlds().forEach(world -> world.savingDisabled = oldWorldsSavingDisabled.getOrDefault(world, true)); - // Print finish message: time elapsed and file size + // Finish. Print time elapsed and file size long timeElapsedMillis = System.currentTimeMillis() - startTime; String msgText = String.format("Backup finished. Time elapsed: %.2fs. ", timeElapsedMillis / 1000.0) + result.getFeedback(); PrintUtil.msgInfo(context, msgText, true); diff --git a/src/main/java/com/keuin/kbackupfabric/operation/backup/feedback/FailedBackupFeedback.java b/src/main/java/com/keuin/kbackupfabric/operation/backup/feedback/FailedBackupFeedback.java new file mode 100644 index 0000000..e7cbbd8 --- /dev/null +++ b/src/main/java/com/keuin/kbackupfabric/operation/backup/feedback/FailedBackupFeedback.java @@ -0,0 +1,20 @@ +package com.keuin.kbackupfabric.operation.backup.feedback; + +public abstract class FailedBackupFeedback implements BackupFeedback { + + private final String message; + + public FailedBackupFeedback(String message) { + this.message = message; + } + + @Override + public boolean isSuccess() { + return false; + } + + @Override + public String getFeedback() { + return message; + } +} diff --git a/src/main/java/com/keuin/kbackupfabric/operation/backup/feedback/SuccessBackupFeedback.java b/src/main/java/com/keuin/kbackupfabric/operation/backup/feedback/SuccessBackupFeedback.java new file mode 100644 index 0000000..dec4e63 --- /dev/null +++ b/src/main/java/com/keuin/kbackupfabric/operation/backup/feedback/SuccessBackupFeedback.java @@ -0,0 +1,35 @@ +package com.keuin.kbackupfabric.operation.backup.feedback; + +import java.util.Objects; + +public abstract class SuccessBackupFeedback implements BackupFeedback { + + private final String message; + + public SuccessBackupFeedback(String successMessage) { + this.message = Objects.requireNonNull(successMessage); + } + + @Override + public boolean isSuccess() { + return true; + } + + @Override + public String getFeedback() { + return message; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + SuccessBackupFeedback that = (SuccessBackupFeedback) o; + return message.equals(that.message); + } + + @Override + public int hashCode() { + return Objects.hash(message); + } +} diff --git a/src/main/java/com/keuin/kbackupfabric/operation/backup/method/ConfiguredIncrementalBackupMethod.java b/src/main/java/com/keuin/kbackupfabric/operation/backup/method/ConfiguredIncrementalBackupMethod.java index 60b825e..baa6580 100644 --- a/src/main/java/com/keuin/kbackupfabric/operation/backup/method/ConfiguredIncrementalBackupMethod.java +++ b/src/main/java/com/keuin/kbackupfabric/operation/backup/method/ConfiguredIncrementalBackupMethod.java @@ -14,6 +14,7 @@ import java.io.IOException; import java.nio.file.Paths; import java.util.Arrays; import java.util.HashSet; +import java.util.logging.Logger; public class ConfiguredIncrementalBackupMethod implements ConfiguredBackupMethod { @@ -22,6 +23,8 @@ public class ConfiguredIncrementalBackupMethod implements ConfiguredBackupMethod private final String backupIndexFileSaveDirectory; private final String backupBaseDirectory; + private static final Logger LOGGER = Logger.getLogger(ConfiguredIncrementalBackupMethod.class.getName()); + public ConfiguredIncrementalBackupMethod(String backupIndexFileName, String levelPath, String backupIndexFileSaveDirectory, String backupBaseDirectory) { this.backupIndexFileName = backupIndexFileName; this.levelPath = levelPath; @@ -30,26 +33,45 @@ public class ConfiguredIncrementalBackupMethod implements ConfiguredBackupMethod } @Override - public IncrementalBackupFeedback backup() throws IOException { - File levelPathFile = new File(levelPath); + public IncrementalBackupFeedback backup() { + IncrementalBackupFeedback feedback; + try { + File levelPathFile = new File(levelPath); + + // construct incremental backup index + PrintUtil.info("Hashing files..."); + ObjectCollection collection = new ObjectCollectionFactory<>(Sha256Identifier.getFactory()) + .fromDirectory(levelPathFile, new HashSet<>(Arrays.asList("session.lock", "kbackup_metadata"))); + + // update storage + PrintUtil.info("Copying files..."); + IncrementalBackupStorageManager storageManager = new IncrementalBackupStorageManager(Paths.get(backupBaseDirectory)); + int filesAdded = storageManager.addObjectCollection(collection, levelPathFile); + + // save index file + PrintUtil.info("Saving index file..."); + ObjectCollectionSerializer.toFile(collection, new File(backupIndexFileSaveDirectory, backupIndexFileName)); + + // return result + PrintUtil.info("Incremental backup finished."); + feedback = new IncrementalBackupFeedback(filesAdded >= 0, filesAdded); + } catch (IOException e) { + feedback = new IncrementalBackupFeedback(false, 0); + } - // construct incremental backup index - PrintUtil.info("Hashing files..."); - ObjectCollection collection = new ObjectCollectionFactory<>(Sha256Identifier.getFactory()) - .fromDirectory(levelPathFile, new HashSet<>(Arrays.asList("session.lock", "kbackup_metadata"))); + if (!feedback.isSuccess()) { + // do clean-up if failed + File backupIndexFile = new File(backupIndexFileSaveDirectory, backupIndexFileName); + if (backupIndexFile.exists()) { + if (!backupIndexFile.delete()) { + LOGGER.warning("Failed to clean up: cannot delete file " + backupIndexFile.getName()); + } + } - // update storage - PrintUtil.info("Copying files..."); - IncrementalBackupStorageManager storageManager = new IncrementalBackupStorageManager(Paths.get(backupBaseDirectory)); - int filesAdded = storageManager.addObjectCollection(collection, levelPathFile); - - // save index file - PrintUtil.info("Saving index file..."); - ObjectCollectionSerializer.toFile(collection, new File(backupIndexFileSaveDirectory, backupIndexFileName)); + //TODO: do more deep clean for object files + } - // return result - PrintUtil.info("Incremental backup finished."); - return new IncrementalBackupFeedback(filesAdded >= 0, filesAdded); + return feedback; } @Override diff --git a/src/main/java/com/keuin/kbackupfabric/operation/backup/method/ConfiguredPrimitiveBackupMethod.java b/src/main/java/com/keuin/kbackupfabric/operation/backup/method/ConfiguredPrimitiveBackupMethod.java index 87a3043..86a60df 100644 --- a/src/main/java/com/keuin/kbackupfabric/operation/backup/method/ConfiguredPrimitiveBackupMethod.java +++ b/src/main/java/com/keuin/kbackupfabric/operation/backup/method/ConfiguredPrimitiveBackupMethod.java @@ -14,6 +14,7 @@ import java.io.File; import java.io.IOException; import java.nio.file.Paths; import java.time.LocalDateTime; +import java.util.logging.Logger; public class ConfiguredPrimitiveBackupMethod implements ConfiguredBackupMethod { @@ -21,6 +22,8 @@ public class ConfiguredPrimitiveBackupMethod implements ConfiguredBackupMethod { private final String levelPath; private final String backupSavePath; + private final Logger LOGGER = Logger.getLogger(ConfiguredPrimitiveBackupMethod.class.getName()); + public ConfiguredPrimitiveBackupMethod(String backupFileName, String levelPath, String backupSavePath) { this.backupFileName = backupFileName; this.levelPath = levelPath; @@ -34,20 +37,34 @@ public class ConfiguredPrimitiveBackupMethod implements ConfiguredBackupMethod { } @Override - public PrimitiveBackupFeedback backup() throws IOException { + public PrimitiveBackupFeedback backup() { + + PrimitiveBackupFeedback feedback; + try { String customBackupName = new PrimitiveBackupFileNameEncoder().decode(backupFileName).customName; BackupMetadata backupMetadata = new BackupMetadata(System.currentTimeMillis(), customBackupName); PrintUtil.info(String.format("zip(srcPath=%s, destPath=%s)", levelPath, backupSavePath)); PrintUtil.info("Compressing level ..."); ZipUtil.makeBackupZip(levelPath, backupSavePath, backupFileName, backupMetadata); + feedback = new PrimitiveBackupFeedback(true, FilesystemUtil.getFileSizeBytes(backupSavePath, backupFileName)); } catch (ZipUtilException exception) { PrintUtil.info("Infinite recursive of directory tree detected, backup was aborted."); - return new PrimitiveBackupFeedback(false, 0); + feedback = new PrimitiveBackupFeedback(false, 0); + } catch (IOException e) { + feedback = new PrimitiveBackupFeedback(false, 0); } - // Get backup file size and return - return new PrimitiveBackupFeedback(true, FilesystemUtil.getFileSizeBytes(backupSavePath, backupFileName)); + if (!feedback.isSuccess()) { + // do clean-up if failed + File backupFile = new File(backupSavePath, backupFileName); + if (backupFile.exists()) { + if (!backupFile.delete()) { + LOGGER.warning("Failed to clean up: cannot delete file " + backupFile.getName()); + } + } + } + return feedback; } @Override diff --git a/src/main/java/com/keuin/kbackupfabric/util/ZipUtil.java b/src/main/java/com/keuin/kbackupfabric/util/ZipUtil.java index 62482c5..07486c5 100644 --- a/src/main/java/com/keuin/kbackupfabric/util/ZipUtil.java +++ b/src/main/java/com/keuin/kbackupfabric/util/ZipUtil.java @@ -4,17 +4,14 @@ import com.keuin.kbackupfabric.exception.ZipUtilException; import com.keuin.kbackupfabric.metadata.BackupMetadata; import java.io.*; -import java.util.Collections; -import java.util.Enumeration; -import java.util.Optional; -import java.util.Set; +import java.util.*; import java.util.zip.*; import static java.util.zip.Deflater.DEFAULT_COMPRESSION; public final class ZipUtil { - private static final int bufferSize = 1024 * 1024 * 8; // 1MB + private static final int bufferSize = 1024 * 1024 * 8; // 8MB /** * 递归压缩文件夹 @@ -30,12 +27,13 @@ public final class ZipUtil { return; } - if (file.getName().equals(BackupMetadata.metadataFileName)) + boolean skipping = Optional.ofNullable(filesSkipping).orElse(Collections.emptySet()).contains(file.getName()) + || file.getName().equals(BackupMetadata.metadataFileName); + if (skipping) return; // Reject // 如果是文件,则直接压缩该文件 - boolean skipping = Optional.ofNullable(filesSkipping).orElse(Collections.emptySet()).contains(file.getName()); - if (file.isFile() && !skipping) { + if (file.isFile()) { int count; // 获取文件相对于压缩文件夹根目录的子路径 @@ -66,11 +64,6 @@ public final class ZipUtil { } } -// public static void makeZipBackup(String srcPath, String zipPath, String zipFileName) throws IOException, ZipUtilException { -// zip(srcPath, zipPath, zipFileName, null); -// } - - /** * 对文件或文件目录进行压缩 * @@ -81,9 +74,21 @@ public final class ZipUtil { * @throws ZipUtilException General exception, such as loop recursion. */ public static void makeBackupZip(String srcPath, String zipPath, String zipFileName, BackupMetadata backupMetadata, int zipLevel) throws IOException, ZipUtilException { - if (srcPath == null || zipPath == null || zipFileName == null || backupMetadata == null || srcPath.isEmpty() || zipPath.isEmpty() || zipFileName.isEmpty()) { - throw new IllegalArgumentException("Parameter for zip() contains null."); + Objects.requireNonNull(srcPath); + Objects.requireNonNull(zipPath); + Objects.requireNonNull(zipFileName); + Objects.requireNonNull(backupMetadata); + Objects.requireNonNull(srcPath); + if (srcPath.isEmpty()) { + throw new IllegalArgumentException("srcPath cannot be empty"); + } + if (zipPath.isEmpty()) { + throw new IllegalArgumentException("zipPath cannot be empty"); } + if (zipFileName.isEmpty()) { + throw new IllegalArgumentException("zipFileName cannot be empty"); + } + CheckedOutputStream checkedOutputStream; ZipOutputStream zipOutputStream = null; try { @@ -137,6 +142,7 @@ public final class ZipUtil { srcRootDir = srcPath.substring(0, index); } } + //调用递归压缩方法进行目录或文件压缩 zip(srcRootDir, srcFile, zipOutputStream, Collections.singleton("session.lock"), new byte[bufferSize]); zipOutputStream.flush(); -- cgit v1.2.3