Fix minor bug in ZipUtil. Do some clean if backup is failed. Small code refactor.

8 files changed, 143 insertions, 43 deletions

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();