Fix #198 failed to delete theme folder

build.gradle

@@ -1,8 +1,8 @@
 plugins {
     id 'org.springframework.boot' version '2.1.3.RELEASE'
     id "io.freefair.lombok" version "3.6.6"
+//    id 'war'
     id 'java'
-    id 'war'
 }
 
 apply plugin: 'io.spring.dependency-management'

src/main/java/run/halo/app/service/base/BasePostService.java

@@ -182,7 +182,6 @@ public interface BasePostService<POST extends BasePost> extends CrudService<POST
     @Transactional
     void increaseLike(@NonNull Integer postId);
 
-
     /**
      * Creates or updates by post.
      *

src/main/java/run/halo/app/service/impl/ThemeServiceImpl.java

@@ -5,7 +5,6 @@ import org.apache.commons.lang3.StringUtils;
 import org.eclipse.jgit.api.Git;
 import org.eclipse.jgit.api.PullResult;
 import org.eclipse.jgit.api.errors.GitAPIException;
-import org.eclipse.jgit.errors.RepositoryNotFoundException;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.transport.RemoteConfig;
 import org.eclipse.jgit.transport.URIish;
@@ -34,6 +33,7 @@ import run.halo.app.service.OptionService;
 import run.halo.app.service.ThemeService;
 import run.halo.app.utils.FileUtils;
 import run.halo.app.utils.FilenameUtils;
+import run.halo.app.utils.GitUtils;
 import run.halo.app.utils.HaloUtils;
 
 import java.io.ByteArrayInputStream;
@@ -46,6 +46,7 @@ import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.*;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import java.util.zip.ZipInputStream;
 
 import static run.halo.app.model.support.HaloConst.DEFAULT_THEME_ID;
@@ -127,15 +128,17 @@ public class ThemeServiceImpl implements ThemeService {
 
         Optional<ThemeProperty[]> themePropertiesOptional = cacheStore.getAny(THEMES_CACHE_KEY, ThemeProperty[].class);
 
-        try {
+        if (themePropertiesOptional.isPresent()) {
-            if (themePropertiesOptional.isPresent()) {
+            // Convert to theme properties
-                // Convert to theme properties
+            ThemeProperty[] themeProperties = themePropertiesOptional.get();
-                ThemeProperty[] themeProperties = themePropertiesOptional.get();
+            return new HashSet<>(Arrays.asList(themeProperties));
-                return new HashSet<>(Arrays.asList(themeProperties));
+        }
-            }
+
+        try (Stream<Path> pathStream = Files.list(getBasePath())) {
 
             // List and filter sub folders
-            List<Path> themePaths = Files.list(getBasePath()).filter(path -> Files.isDirectory(path)).collect(Collectors.toList());
+            List<Path> themePaths = pathStream.filter(path -> Files.isDirectory(path))
+                    .collect(Collectors.toList());
 
             if (CollectionUtils.isEmpty(themePaths)) {
                 return Collections.emptySet();
@@ -172,9 +175,8 @@ public class ThemeServiceImpl implements ThemeService {
         // Get the theme path
         Path themePath = Paths.get(getThemeOfNonNullBy(themeId).getThemePath());
 
-        try {
+        try (Stream<Path> pathStream = Files.list(themePath)) {
-            return Files.list(themePath)
+            return pathStream.filter(path -> StringUtils.startsWithIgnoreCase(path.getFileName().toString(), CUSTOM_SHEET_PREFIX))
-                    .filter(path -> StringUtils.startsWithIgnoreCase(path.getFileName().toString(), CUSTOM_SHEET_PREFIX))
                     .map(path -> {
                         // Remove prefix
                         String customTemplate = StringUtils.removeStartIgnoreCase(path.getFileName().toString(), CUSTOM_SHEET_PREFIX);
@@ -253,8 +255,7 @@ public class ThemeServiceImpl implements ThemeService {
 
         try {
             // Delete the folder
-            FileUtils.del(Paths.get(themeProperty.getThemePath()));
+            FileUtils.deleteFolder(Paths.get(themeProperty.getThemePath()));
-
             // Delete theme cache
             eventPublisher.publishEvent(new ThemeUpdatedEvent(this));
         } catch (Exception e) {
@@ -408,7 +409,10 @@ public class ThemeServiceImpl implements ThemeService {
         Assert.isTrue(Files.isDirectory(themeTmpPath), "Theme temporary path must be a directory");
 
         log.debug("Children path of [{}]:", themeTmpPath);
-        Files.list(themeTmpPath).forEach(path -> log.debug(path.toString()));
+
+        try (Stream<Path> pathStream = Files.list(themeTmpPath)) {
+            pathStream.forEach(path -> log.debug(path.toString()));
+        }
 
         // Check property config
         ThemeProperty tmpThemeProperty = getProperty(themeTmpPath);
@@ -451,7 +455,8 @@ public class ThemeServiceImpl implements ThemeService {
                 downloadZipAndUnzip(uri, themeTmpPath);
             } else {
                 uri = StringUtils.appendIfMissingIgnoreCase(uri, ".git", ".git");
-                cloneFromGit(uri, themeTmpPath);
+                // Clone from git
+                GitUtils.cloneFromGit(uri, themeTmpPath);
             }
 
             return add(themeTmpPath);
@@ -491,83 +496,56 @@ public class ThemeServiceImpl implements ThemeService {
         String branch = StringUtils.isBlank(themeProperty.getBranch()) ?
                 DEFAULT_REMOTE_BRANCH : themeProperty.getBranch();
 
-        File themeFolder = Paths.get(themeProperty.getThemePath()).toFile();
+        Git git = null;
-        // Open the theme path
-        Git git;
 
         try {
-            git = Git.open(themeFolder);
+            git = GitUtils.openOrInit(Paths.get(themeProperty.getThemePath()));
-        } catch (RepositoryNotFoundException e) {
+            // Force to set remote name
-            // Repository is not initialized
+            git.remoteRemove().setRemoteName(THEME_PROVIDER_REMOTE_NAME).call();
-            git = Git.init().setDirectory(themeFolder).call();
+            RemoteConfig remoteConfig = git.remoteAdd()
-        }
+                    .setName(THEME_PROVIDER_REMOTE_NAME)
-
+                    .setUri(new URIish(themeProperty.getRepo()))
-        // Force to set remote name
-        git.remoteRemove().setRemoteName(THEME_PROVIDER_REMOTE_NAME).call();
-        RemoteConfig remoteConfig = git.remoteAdd()
-                .setName(THEME_PROVIDER_REMOTE_NAME)
-                .setUri(new URIish(themeProperty.getRepo()))
-                .call();
-
-        // Add all changes
-        git.add()
-                .addFilepattern(".")
-                .call();
-        // Commit the changes
-        git.commit().setMessage("Commit by halo automatically").call();
-
-        // Check out to specified branch
-        if (!StringUtils.equalsIgnoreCase(branch, git.getRepository().getBranch())) {
-            boolean present = git.branchList()
-                    .call()
-                    .stream()
-                    .map(Ref::getName)
-                    .anyMatch(name -> StringUtils.equalsIgnoreCase(name, branch));
-
-            git.checkout()
-                    .setCreateBranch(true)
-                    .setForced(!present)
-                    .setName(branch)
                     .call();
+
+            // Add all changes
+            git.add()
+                    .addFilepattern(".")
+                    .call();
+            // Commit the changes
+            git.commit().setMessage("Commit by halo automatically").call();
+
+            // Check out to specified branch
+            if (!StringUtils.equalsIgnoreCase(branch, git.getRepository().getBranch())) {
+                boolean present = git.branchList()
+                        .call()
+                        .stream()
+                        .map(Ref::getName)
+                        .anyMatch(name -> StringUtils.equalsIgnoreCase(name, branch));
+
+                git.checkout()
+                        .setCreateBranch(true)
+                        .setForced(!present)
+                        .setName(branch)
+                        .call();
+            }
+
+            // Pull with rebasing
+            PullResult pullResult = git.pull()
+                    .setRemote(remoteConfig.getName())
+                    .setRemoteBranchName(branch)
+                    .setRebase(true)
+                    .call();
+
+            if (!pullResult.isSuccessful()) {
+                log.debug("Rebase result: [{}]", pullResult.getRebaseResult());
+                log.debug("Merge result: [{}]", pullResult.getMergeResult());
+
+                throw new ThemeUpdateException("拉取失败！您与主题作者可能同时更改了同一个文件");
+            }
+        } finally {
+            GitUtils.closeQuietly(git);
         }
 
-        // Pull with rebasing
-        PullResult pullResult = git.pull()
-                .setRemote(remoteConfig.getName())
-                .setRemoteBranchName(branch)
-                .setRebase(true)
-                .call();
-
-        if (!pullResult.isSuccessful()) {
-            log.debug("Rebase result: [{}]", pullResult.getRebaseResult());
-            log.debug("Merge result: [{}]", pullResult.getMergeResult());
-
-            throw new ThemeUpdateException("拉取失败！您与主题作者可能同时更改了同一个文件");
-        }
-        // Close git
-        git.close();
-    }
-
-    /**
-     * Clones theme from git.
-     *
-     * @param gitUrl     git url must not be blank
-     * @param targetPath target path must not be null
-     * @throws GitAPIException throws when clone error
-     */
-    private void cloneFromGit(@NonNull String gitUrl, @NonNull Path targetPath) throws GitAPIException {
-        Assert.hasText(gitUrl, "Git url must not be blank");
-        Assert.notNull(targetPath, "Target path must not be null");
-
-        log.debug("Cloning git repo [{}] to [{}]", gitUrl, targetPath);
-
-        // Clone it
-        Git.cloneRepository()
-                .setURI(gitUrl)
-                .setDirectory(targetPath.toFile())
-                .call();
-
-        log.debug("Cloned git repo [{}]", gitUrl);
     }
 
     /**
@@ -625,10 +603,10 @@ public class ThemeServiceImpl implements ThemeService {
             return null;
         }
 
-        try {
+        try (Stream<Path> pathStream = Files.list(topPath)) {
             List<ThemeFile> themeFiles = new LinkedList<>();
 
-            Files.list(topPath).forEach(path -> {
+            pathStream.forEach(path -> {
                 // Build theme file
                 ThemeFile themeFile = new ThemeFile();
                 themeFile.setName(path.getFileName().toString());
@@ -787,12 +765,13 @@ public class ThemeServiceImpl implements ThemeService {
     private Optional<String> getScreenshotsFileName(@NonNull Path themePath) throws IOException {
         Assert.notNull(themePath, "Theme path must not be null");
 
-        return Files.list(themePath)
+        try (Stream<Path> pathStream = Files.list(themePath)) {
-                .filter(path -> Files.isRegularFile(path)
+            return pathStream.filter(path -> Files.isRegularFile(path)
-                        && Files.isReadable(path)
+                    && Files.isReadable(path)
-                        && FilenameUtils.getBasename(path.toString()).equalsIgnoreCase(THEME_SCREENSHOTS_NAME))
+                    && FilenameUtils.getBasename(path.toString()).equalsIgnoreCase(THEME_SCREENSHOTS_NAME))
-                .findFirst()
+                    .findFirst()
-                .map(path -> path.getFileName().toString());
+                    .map(path -> path.getFileName().toString());
+        }
     }
 
     /**

src/main/java/run/halo/app/utils/FileUtils.java

@@ -1,20 +1,18 @@
 package run.halo.app.utils;
 
-import cn.hutool.core.io.IORuntimeException;
 import lombok.extern.slf4j.Slf4j;
 import org.springframework.lang.NonNull;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 import run.halo.app.exception.ForbiddenException;
 
-import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.file.*;
 import java.nio.file.attribute.BasicFileAttributes;
-import java.util.Comparator;
 import java.util.List;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipInputStream;
 
@@ -67,14 +65,26 @@ public class FileUtils {
     public static void deleteFolder(@NonNull Path deletingPath) throws IOException {
         Assert.notNull(deletingPath, "Deleting path must not be null");
 
-        log.debug("Deleting [{}]", deletingPath);
+        log.info("Deleting [{}]", deletingPath);
 
-        Files.walk(deletingPath)
+        // Delete folder recursively
-                .sorted(Comparator.reverseOrder())
+        org.eclipse.jgit.util.FileUtils.delete(deletingPath.toFile(),
-                .map(Path::toFile)
+                org.eclipse.jgit.util.FileUtils.RECURSIVE | org.eclipse.jgit.util.FileUtils.RETRY);
-                .forEach(File::delete);
 
-        log.debug("Deleted [{}] successfully", deletingPath);
+//        try (Stream<Path> pathStream = Files.walk(deletingPath)) {
+//            pathStream.sorted(Comparator.reverseOrder())
+//                    .peek(path -> log.debug("Try to delete [{}]", path.toString()))
+//                    .forEach(path -> {
+//                        try {
+//                            Files.delete(path);
+//                            log.debug("Deleted [{}] successfully", path.toString());
+//                        } catch (IOException e) {
+//                            throw new ServiceException("Failed to delete " + path.toString(), e).setErrorData(deletingPath.toString());
+//                        }
+//                    });
+//        }
+
+        log.info("Deleted [{}] successfully", deletingPath);
     }
 
     /**
@@ -125,13 +135,15 @@ public class FileUtils {
     public static Path skipZipParentFolder(@NonNull Path unzippedPath) throws IOException {
         Assert.notNull(unzippedPath, "Unzipped folder must not be  null");
 
-        List<Path> childrenPath = Files.list(unzippedPath).collect(Collectors.toList());
+        // TODO May cause a latent problem.
+        try (Stream<Path> pathStream = Files.list(unzippedPath)) {
+            List<Path> childrenPath = pathStream.collect(Collectors.toList());
 
-        if (childrenPath.size() == 1 && Files.isDirectory(childrenPath.get(0))) {
+            if (childrenPath.size() == 1 && Files.isDirectory(childrenPath.get(0))) {
-            return childrenPath.get(0);
+                return childrenPath.get(0);
+            }
+            return unzippedPath;
         }
-
-        return unzippedPath;
     }
 
     /**
@@ -161,7 +173,9 @@ public class FileUtils {
     public static boolean isEmpty(@NonNull Path path) throws IOException {
         Assert.notNull(path, "Path must not be null");
 
-        return Files.list(path).count() == 0;
+        try (Stream<Path> pathStream = Files.list(path)) {
+            return pathStream.count() == 0;
+        }
     }
 
     /**
@@ -261,43 +275,4 @@ public class FileUtils {
         }
     }
 
-    /**
-     * 删除文件或者文件夹
-     *
-     * @param path path
-     * @return boolean
-     * @throws IORuntimeException IORuntimeException
-     */
-    public static boolean del(Path path) throws IORuntimeException {
-        if (Files.notExists(path)) {
-            return true;
-        }
-        try {
-            if (Files.isDirectory(path)) {
-                Files.walkFileTree(path, new SimpleFileVisitor<Path>() {
-
-                    @Override
-                    public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
-                        Files.delete(file);
-                        return FileVisitResult.CONTINUE;
-                    }
-
-                    @Override
-                    public FileVisitResult postVisitDirectory(Path dir, IOException e) throws IOException {
-                        if (e == null) {
-                            Files.delete(dir);
-                            return FileVisitResult.CONTINUE;
-                        } else {
-                            throw e;
-                        }
-                    }
-                });
-            } else {
-                Files.delete(path);
-            }
-        } catch (IOException e) {
-            throw new IORuntimeException(e);
-        }
-        return true;
-    }
 }

src/main/java/run/halo/app/utils/GitUtils.java

@@ -0,0 +1,69 @@
+package run.halo.app.utils;
+
+import lombok.extern.slf4j.Slf4j;
+import org.eclipse.jgit.api.Git;
+import org.eclipse.jgit.api.errors.GitAPIException;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.storage.file.WindowCacheConfig;
+import org.springframework.lang.NonNull;
+import org.springframework.util.Assert;
+
+import java.io.IOException;
+import java.nio.file.Path;
+
+/**
+ * Git utilities.
+ *
+ * @author johnniang
+ * @date 19-6-12
+ */
+@Slf4j
+public class GitUtils {
+
+    private GitUtils() {
+        // Config packed git MMAP
+        WindowCacheConfig config = new WindowCacheConfig();
+        config.setPackedGitMMAP(false);
+        config.install();
+    }
+
+    public static void cloneFromGit(@NonNull String repoUrl, @NonNull Path targetPath) throws GitAPIException {
+        Assert.hasText(repoUrl, "Repository remote url must not be blank");
+        Assert.notNull(targetPath, "Target path must not be null");
+
+        log.debug("Trying to clone git repo [{}] to [{}]", repoUrl, targetPath);
+
+        // Use try-with-resource-statement
+        Git git = null;
+        try {
+            git = Git.cloneRepository()
+                    .setURI(repoUrl)
+                    .setDirectory(targetPath.toFile())
+                    .call();
+            log.debug("Cloned git repo [{}] successfully", repoUrl);
+        } finally {
+            closeQuietly(git);
+        }
+    }
+
+    public static Git openOrInit(Path repoPath) throws IOException, GitAPIException {
+        Git git;
+
+        try {
+            git = Git.open(repoPath.toFile());
+        } catch (RepositoryNotFoundException e) {
+            log.warn("Git repository may not exist, we will try to initialize an empty repository", e);
+            git = Git.init().setDirectory(repoPath.toFile()).call();
+        }
+
+        return git;
+    }
+
+    public static void closeQuietly(Git git) {
+        if (git != null) {
+            git.getRepository().close();
+            git.close();
+        }
+    }
+
+}

src/test/java/run/halo/app/utils/FileUtilsTest.java

@@ -8,6 +8,7 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.List;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import static org.hamcrest.Matchers.equalTo;
 
@@ -27,26 +28,29 @@ public class FileUtilsTest {
         // Create test folders
         Files.createDirectories(testPath);
 
-        System.out.println("Walk path list");
+        try (Stream<Path> pathStream = Files.walk(tempDirectory)) {
-        List<Path> walkList = Files.walk(tempDirectory).collect(Collectors.toList());
+            List<Path> walkList = pathStream.collect(Collectors.toList());
-        walkList.forEach(System.out::println);
+            walkList.forEach(System.out::println);
-        Assert.assertThat(walkList.size(), equalTo(4));
+            Assert.assertThat(walkList.size(), equalTo(4));
+        }
 
+        try (Stream<Path> pathStream = Files.walk(tempDirectory, 1)) {
+            List<Path> walkList = pathStream.collect(Collectors.toList());
+            walkList.forEach(System.out::println);
+            Assert.assertThat(walkList.size(), equalTo(2));
+        }
 
-        System.out.println("Walk 1 deep path list");
+        try (Stream<Path> pathStream = Files.list(tempDirectory)) {
-        List<Path> walk1DeepList = Files.walk(tempDirectory, 1).collect(Collectors.toList());
+            List<Path> walkList = pathStream.collect(Collectors.toList());
-        walk1DeepList.forEach(System.out::println);
+            walkList.forEach(System.out::println);
-        Assert.assertThat(walk1DeepList.size(), equalTo(2));
+            Assert.assertThat(walkList.size(), equalTo(1));
+        }
 
-        System.out.println("List path list");
+        try (Stream<Path> pathStream = Files.list(testPath)) {
-        List<Path> listList = Files.list(tempDirectory).collect(Collectors.toList());
+            List<Path> walkList = pathStream.collect(Collectors.toList());
-        listList.forEach(System.out::println);
+            walkList.forEach(System.out::println);
-        Assert.assertThat(listList.size(), equalTo(1));
+            Assert.assertThat(walkList.size(), equalTo(0));
-
+        }
-        System.out.println("List test path list");
-        List<Path> testPathList = Files.list(testPath).collect(Collectors.toList());
-        testPathList.forEach(System.out::println);
-        Assert.assertThat(testPathList.size(), equalTo(0));
 
         // Delete it
         FileUtils.deleteFolder(tempDirectory);

src/test/java/run/halo/app/utils/PathTest.java

@@ -1,13 +1,12 @@
 package run.halo.app.utils;
 
-import com.sun.nio.zipfs.JarFileSystemProvider;
 import org.junit.Test;
 
-import java.io.IOException;
 import java.net.URI;
 import java.net.URISyntaxException;
-import java.nio.file.*;
+import java.nio.file.FileSystemNotFoundException;
-import java.util.Collections;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 
 /**
  * Path test.
@@ -25,18 +24,4 @@ public class PathTest {
 
         System.out.println("Path: " + path.toString());
     }
-
-//    @Test
-//    public void getPathOfJarFileSuccessfully() throws URISyntaxException, IOException {
-//        String file = "jar:file:/path/to/jar/xxx.jar!/BOOT-INF/classes!/templates/themes";
-//        URI uri = new URI(file);
-//        FileSystem fileSystem = FileSystems.newFileSystem(uri, Collections.emptyMap());
-//        Path path = fileSystem.getPath("/BOOT-INF/classes/templates/themes");
-//
-//        System.out.println("Path: " + path.toString());
-//
-//        Files.walk(path, 1).forEach(p -> {
-//            System.out.println(p.toString());
-//        });
-//    }
 }

src/test/java/run/halo/app/utils/PathsTest.java

@@ -1,6 +1,5 @@
 package run.halo.app.utils;
 
-import io.micrometer.core.annotation.TimedSet;
 import org.junit.Test;
 
 import java.nio.file.Path;