Improved error handling

fizzed · Oct 28, 2023 · 5b11c26 · 5b11c26
1 parent 6a745aa
commit 5b11c26
Show file tree

Hide file tree

Showing 7 changed files with 265 additions and 91 deletions.
diff --git a/blaze-core/src/main/java/com/fizzed/blaze/core/DirectoryNotEmptyException.java b/blaze-core/src/main/java/com/fizzed/blaze/core/DirectoryNotEmptyException.java
@@ -0,0 +1,28 @@
+/*
+ * Copyright 2015 Fizzed, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.fizzed.blaze.core;
+
+public class DirectoryNotEmptyException extends BlazeException {
+
+    public DirectoryNotEmptyException(String msg) {
+        super(msg);
+    }
+
+    public DirectoryNotEmptyException(String msg, Throwable cause) {
+        super(msg, cause);
+    }
+
+}
diff --git a/blaze-core/src/main/java/com/fizzed/blaze/internal/FileHelper.java b/blaze-core/src/main/java/com/fizzed/blaze/internal/FileHelper.java
@@ -31,6 +31,7 @@
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.util.Base64;
+import java.util.stream.Stream;
 
 /**
  *
@@ -86,7 +87,17 @@ static public String fileExtension(Path path) {
         }
         return name.substring(lastIndexOf);
     }
-
+
+    static public boolean isEmptyDir(Path dir) throws IOException {
+        return !isNotEmptyDir(dir);
+    }
+
+    static public boolean isNotEmptyDir(Path dir) throws IOException {
+        try (Stream<Path> files = Files.list(dir)) {
+            return files.findFirst().isPresent();
+        }
+    }
+
     static public Path concatToFileName(Path path, String moreFileName) {
         return path.resolveSibling(path.getFileName().toString() + moreFileName);
     }

diff --git a/blaze-core/src/main/java/com/fizzed/blaze/system/Copy.java b/blaze-core/src/main/java/com/fizzed/blaze/system/Copy.java
@@ -18,6 +18,8 @@
 import com.fizzed.blaze.Context;
 import com.fizzed.blaze.core.Action;
 import com.fizzed.blaze.core.BlazeException;
+import com.fizzed.blaze.core.DirectoryNotEmptyException;
+import com.fizzed.blaze.core.FileNotFoundException;
 import com.fizzed.blaze.core.Verbosity;
 import com.fizzed.blaze.core.VerbosityMixin;
 import com.fizzed.blaze.util.*;
@@ -29,6 +31,8 @@
 import java.util.*;
 import java.util.stream.Stream;
 
+import static com.fizzed.blaze.internal.FileHelper.isNotEmptyDir;
+
 public class Copy extends Action<Copy.Result,Void> implements VerbosityMixin<Copy> {
 
     static public class Result extends com.fizzed.blaze.core.Result<Copy,Void,Result> {
@@ -179,7 +183,7 @@ protected Copy.Result doRun() throws BlazeException {
         // the sources must all exist (we should check this first before we do anything)
         for (Path source : this.sources) {
             if (!Files.exists(source)) {
-                throw new BlazeException("Copy source " + source + " does not exist");
+                throw new FileNotFoundException("Copy source file " + source + " not found");
             }
         }
 
@@ -192,8 +196,8 @@ protected Copy.Result doRun() throws BlazeException {
 
                 if (Files.isDirectory(source)) {
                     // if the source is a directory, does it have stuff in it?
-                    if (hasFiles(source) && !this.recursive) {
-                        throw new BlazeException("Copy source directory " + source + " is not empty (and recursive is disabled)");
+                    if (isNotEmptyDir(source) && !this.recursive) {
+                        throw new DirectoryNotEmptyException("Copy source directory " + source + " is not empty (and recursive is disabled)");
                     }
 
                     // source is a directory
@@ -270,12 +274,6 @@ protected Copy.Result doRun() throws BlazeException {
         return new Copy.Result(this, null);
     }
 
-    static private boolean hasFiles(Path dir) throws IOException {
-        try (Stream<Path> files = Files.list(dir)) {
-            return files.findFirst().isPresent();
-        }
-    }
-
     private void copyDirectory(Path sourceDir, Path targetDir, Result result) throws IOException {
         Files.walkFileTree(sourceDir, new SimpleFileVisitor<Path>() {
             @Override

diff --git a/blaze-core/src/main/java/com/fizzed/blaze/system/Remove.java b/blaze-core/src/main/java/com/fizzed/blaze/system/Remove.java
@@ -16,8 +16,8 @@
 package com.fizzed.blaze.system;
 
 import com.fizzed.blaze.Context;
-import com.fizzed.blaze.core.Action;
-import com.fizzed.blaze.core.BlazeException;
+import com.fizzed.blaze.core.*;
+
 import java.io.IOException;
 import java.nio.file.FileVisitResult;
 import java.nio.file.Files;
@@ -26,10 +26,11 @@
 import java.nio.file.attribute.BasicFileAttributes;
 import java.util.ArrayList;
 import java.util.List;
-import com.fizzed.blaze.core.PathsMixin;
-import com.fizzed.blaze.core.VerbosityMixin;
+
 import com.fizzed.blaze.util.VerboseLogger;
 
+import static com.fizzed.blaze.internal.FileHelper.isNotEmptyDir;
+
 /**
  * rm - remove files or directories
  * 
@@ -83,25 +84,23 @@ public Remove recursive(boolean recursive) {
     @Override
     protected Result doRun() throws BlazeException {
         try {
-            if (!recursive) {
-                for (Path path : paths) {
-                    log.verbose("Deleting {}", path);
+            for (Path path : paths) {
+                log.verbose("Deleting {}", path);
+
+                if (!Files.exists(path)) {
                     if (!force) {
-                        Files.delete(path);
-                    } else {
-                        Files.deleteIfExists(path);
+                        throw new FileNotFoundException("File " + path + " not found (and force is disabled)");
                     }
+                    continue;
                 }
-            } else {
-                // http://docs.oracle.com/javase/7/docs/api/java/nio/file/FileVisitor.html
-                for (Path path : paths) {
-                    log.verbose("Deleting {}", path);
 
-                    // if path doesn't exist we should throw an error unless we are forced
-                    if (force && !Files.exists(path)) {
-                        continue;
+                if (!recursive) {
+                    if (Files.isDirectory(path) && isNotEmptyDir(path)) {
+                        throw new DirectoryNotEmptyException("Directory " + path + " is not empty (and recursive is disabled)");
                     }
-
+
+                    Files.delete(path);
+                } else {
                     Files.walkFileTree(path, new SimpleFileVisitor<Path>() {
                         @Override
                         public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {

diff --git a/blaze-core/src/test/java/com/fizzed/blaze/system/CopyTest.java b/blaze-core/src/test/java/com/fizzed/blaze/system/CopyTest.java
@@ -1,65 +1,31 @@
 package com.fizzed.blaze.system;
 
-import com.fizzed.blaze.Config;
 import com.fizzed.blaze.core.BlazeException;
-import com.fizzed.blaze.internal.ConfigHelper;
-import com.fizzed.blaze.internal.ContextImpl;
-import com.fizzed.blaze.internal.FileHelper;
+import com.fizzed.blaze.core.DirectoryNotEmptyException;
+import com.fizzed.blaze.core.FileNotFoundException;
 import org.apache.commons.io.FileUtils;
 import org.junit.Before;
 import org.junit.Test;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
-import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.Paths;
-import java.nio.file.StandardOpenOption;
-import java.util.List;
 
 import static com.fizzed.blaze.util.Globber.globber;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
-import static org.mockito.Mockito.spy;
 
-public class CopyTest {
-    private final static Logger log = LoggerFactory.getLogger(CopyTest.class);
+public class CopyTest extends TestAbstractBase {
 
-    Config config;
-    ContextImpl context;
     Path testCopyDir;
 
-    private Path createEmptyDir(Path path, boolean parents) throws IOException {
-        FileUtils.deleteDirectory(path.toFile());
-        if (parents) {
-            Files.createDirectories(path);
-        } else {
-            Files.createDirectory(path);
-        }
-        return path;
-    }
-
-    private Path createFile(Path path) throws IOException {
-        return createFile(path, "test");
-    }
-
-    private Path createFile(Path path, String text) throws IOException {
-        Files.write(path, text.getBytes(StandardCharsets.UTF_8), StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING);
-        return path;
-    }
-
     @Before
     public void setup() throws Exception {
-        this.config = ConfigHelper.create(null);
-        this.context = spy(new ContextImpl(null, null, Paths.get("blaze.java"), config));
-        // this will help find the compile target directory, where is should be in target/test-classes
-        this.testCopyDir = FileHelper.resourceAsPath("/fixtures/resource-locator.txt").resolve("../../../copy-test").normalize();
+        this.testCopyDir = targetDir.resolve("copy-test");
         Files.createDirectories(this.testCopyDir);
     }
 
-    @Test(expected= BlazeException.class)
+    @Test(expected=FileNotFoundException.class)
     public void fileToFileFailsIfSourceDoesNotExist() {
         new Copy(this.context)
             .source(this.testCopyDir.resolve("notexist"))
@@ -101,7 +67,7 @@ public void fileToDirCopyFailsToSameFile() throws Exception {
     @Test
     public void fileToFileCopy() throws Exception {
         final Path sourceFile = createFile(this.testCopyDir.resolve("fileToFileCopy.txt"));
-        final Path targetDir = createEmptyDir(this.testCopyDir.resolve("fileToFileCopyDir"), false);
+        final Path targetDir = createDir(this.testCopyDir.resolve("fileToFileCopyDir"));
         final Path targetFile = targetDir.resolve(sourceFile.getFileName());
 
         assertThat(Files.exists(targetFile), is(false));
@@ -117,7 +83,7 @@ public void fileToFileCopy() throws Exception {
     @Test(expected= BlazeException.class)
     public void fileToFileCopyFailsIfAlreadyExists() throws Exception {
         final Path sourceFile = createFile(this.testCopyDir.resolve("fileToFileCopyFailsIfAlreadyExists.txt"));
-        final Path targetDir = createEmptyDir(this.testCopyDir.resolve("fileToFileCopyFailsIfAlreadyExists"), false);
+        final Path targetDir = createDir(this.testCopyDir.resolve("fileToFileCopyFailsIfAlreadyExists"));
         final Path targetFile = createFile(targetDir.resolve("exists.txt"));
 
         new Copy(this.context)
@@ -129,7 +95,7 @@ public void fileToFileCopyFailsIfAlreadyExists() throws Exception {
     @Test
     public void fileToFileCopyForced() throws Exception {
         final Path sourceFile = createFile(this.testCopyDir.resolve("fileToFileCopyFailsIfAlreadyExists.txt"), "hello world");
-        final Path targetDir = createEmptyDir(this.testCopyDir.resolve("fileToFileCopyFailsIfAlreadyExists"), false);
+        final Path targetDir = createDir(this.testCopyDir.resolve("fileToFileCopyFailsIfAlreadyExists"));
         final Path targetFile = createFile(targetDir.resolve("exists.txt"));
 
         assertThat(FileUtils.readFileToString(targetFile.toFile(), StandardCharsets.UTF_8), is("test"));
@@ -146,7 +112,7 @@ public void fileToFileCopyForced() throws Exception {
     @Test
     public void fileToDirCopy() throws Exception {
         final Path sourceFile = createFile(this.testCopyDir.resolve("fileToDirCopy.txt"));
-        final Path targetDir = createEmptyDir(this.testCopyDir.resolve("fileToDirCopy"), false);
+        final Path targetDir = createDir(this.testCopyDir.resolve("fileToDirCopy"));
         final Path targetFile = targetDir.resolve(sourceFile.getFileName());
 
         assertThat(Files.exists(targetFile), is(false));
@@ -163,7 +129,7 @@ public void fileToDirCopy() throws Exception {
     @Test(expected=BlazeException.class)
     public void dirToDirCopyFailsIfTargetExistsAsFile() throws Exception {
         // create a source directory with a file, a subdir, and the subdir with a file
-        final Path sourceDir = createEmptyDir(this.testCopyDir.resolve("dirToDirCopyFailsIfTargetExistsAsFile"), false);
+        final Path sourceDir = createDir(this.testCopyDir.resolve("dirToDirCopyFailsIfTargetExistsAsFile"));
         final Path sourceDirFile = createFile(sourceDir.resolve("test1.txt"));
 
         final Path targetFile = createFile(this.testCopyDir.resolve("dirToDirCopyFailsIfTargetExistsAsFileToCopyTo"));
@@ -177,7 +143,7 @@ public void dirToDirCopyFailsIfTargetExistsAsFile() throws Exception {
     @Test(expected=BlazeException.class)
     public void dirToDirCopyFailsIfTargetIsSameDirectory() throws Exception {
         // create a source directory with a file, a subdir, and the subdir with a file
-        final Path sourceDir = createEmptyDir(this.testCopyDir.resolve("dirToDirCopyFailsIfTargetIsSameDirectory"), false);
+        final Path sourceDir = createDir(this.testCopyDir.resolve("dirToDirCopyFailsIfTargetIsSameDirectory"));
         final Path sourceDirFile = createFile(sourceDir.resolve("test1.txt"));
 
         new Copy(this.context)
@@ -186,12 +152,12 @@ public void dirToDirCopyFailsIfTargetIsSameDirectory() throws Exception {
             .run();
     }
 
-    @Test(expected=BlazeException.class)
+    @Test(expected=DirectoryNotEmptyException.class)
     public void dirToDirCopyFailsIfNotRecursive() throws Exception {
         // create a source directory with a file, a subdir, and the subdir with a file
-        final Path sourceDir = createEmptyDir(this.testCopyDir.resolve("dirToDirCopy"), false);
+        final Path sourceDir = createDir(this.testCopyDir.resolve("dirToDirCopy"));
         final Path sourceDirFile = createFile(sourceDir.resolve("test1.txt"));
-        final Path sourceDirSubDir = createEmptyDir(sourceDir.resolve("subdir"), false);
+        final Path sourceDirSubDir = createDir(sourceDir.resolve("subdir"));
         final Path sourceDirSubDirFile = createFile(sourceDirSubDir.resolve("test2.txt"));
 
         // create a non-existent directory we want to copy to
@@ -209,9 +175,9 @@ public void dirToDirCopyFailsIfNotRecursive() throws Exception {
     @Test
     public void dirToDirCopy() throws Exception {
         // create a source directory with a file, a subdir, and the subdir with a file
-        final Path sourceDir = createEmptyDir(this.testCopyDir.resolve("dirToDirCopy"), false);
+        final Path sourceDir = createDir(this.testCopyDir.resolve("dirToDirCopy"));
         final Path sourceDirFile = createFile(sourceDir.resolve("test1.txt"));
-        final Path sourceDirSubDir = createEmptyDir(sourceDir.resolve("subdir"), false);
+        final Path sourceDirSubDir = createDir(sourceDir.resolve("subdir"));
         final Path sourceDirSubDirFile = createFile(sourceDirSubDir.resolve("test2.txt"));
 
         // create a non-existent directory we want to copy to
@@ -235,16 +201,16 @@ public void dirToDirCopy() throws Exception {
     @Test(expected=BlazeException.class)
     public void dirToDirCopyFailsIfFileAlreadyExists() throws Exception {
         // create a source directory with a file, a subdir, and the subdir with a file
-        final Path sourceDir = createEmptyDir(this.testCopyDir.resolve("dirToDirCopyFailsIfFileAlreadyExists"), false);
+        final Path sourceDir = createDir(this.testCopyDir.resolve("dirToDirCopyFailsIfFileAlreadyExists"));
         final Path sourceDirFile = createFile(sourceDir.resolve("test1.txt"));
-        final Path sourceDirSubDir = createEmptyDir(sourceDir.resolve("subdir"), false);
+        final Path sourceDirSubDir = createDir(sourceDir.resolve("subdir"), false);
         final Path sourceDirSubDirFile = createFile(sourceDirSubDir.resolve("test2.txt"));
 
         // create a non-existent directory we want to copy to
-        final Path targetDir = createEmptyDir(this.testCopyDir.resolve("dirToDirCopyFailsIfFileAlreadyExistsCopyTo"), false);
+        final Path targetDir = createDir(this.testCopyDir.resolve("dirToDirCopyFailsIfFileAlreadyExistsCopyTo"));
         final Path targetDirFile = createFile(targetDir.resolve("test1.txt"));
         // create a directory here that already exists
-        createEmptyDir(targetDir.resolve("dirToDirCopyFailsIfFileAlreadyExists"), false);
+        createDir(targetDir.resolve("dirToDirCopyFailsIfFileAlreadyExists"));
 
         assertThat(Files.exists(targetDirFile), is(true));
 
@@ -257,8 +223,8 @@ public void dirToDirCopyFailsIfFileAlreadyExists() throws Exception {
 
     @Test(expected=BlazeException.class)
     public void globberCopyFailsIfNoneFound() throws Exception {
-        final Path sourceDir = createEmptyDir(this.testCopyDir.resolve("globberCopy"), false);
-        final Path targetDir = createEmptyDir(this.testCopyDir.resolve("globberCopyto"), false);
+        final Path sourceDir = createDir(this.testCopyDir.resolve("globberCopy"));
+        final Path targetDir = createDir(this.testCopyDir.resolve("globberCopyto"));
 
         new Copy(this.context)
             .sources(globber(sourceDir, "*.{java,js}"))
@@ -268,8 +234,8 @@ public void globberCopyFailsIfNoneFound() throws Exception {
 
     @Test
     public void globberCopyNoneFoundForced() throws Exception {
-        final Path sourceDir = createEmptyDir(this.testCopyDir.resolve("globberCopy"), false);
-        final Path targetDir = createEmptyDir(this.testCopyDir.resolve("globberCopyto"), false);
+        final Path sourceDir = createDir(this.testCopyDir.resolve("globberCopy"));
+        final Path targetDir = createDir(this.testCopyDir.resolve("globberCopyto"));
 
         new Copy(this.context)
             .sources(globber(sourceDir, "*.{java,js}"))
@@ -280,11 +246,11 @@ public void globberCopyNoneFoundForced() throws Exception {
 
     @Test
     public void globberCopy() throws Exception {
-        final Path sourceDir = createEmptyDir(this.testCopyDir.resolve("globberCopy"), false);
+        final Path sourceDir = createDir(this.testCopyDir.resolve("globberCopy"));
         final Path sourceDirJavaFile = createFile(sourceDir.resolve("test.java"));
         final Path sourceDirJsFile = createFile(sourceDir.resolve("test.js"));
         final Path sourceDirKtFile = createFile(sourceDir.resolve("test.kt"));
-        final Path targetDir = createEmptyDir(this.testCopyDir.resolve("globberCopyTo"), false);
+        final Path targetDir = createDir(this.testCopyDir.resolve("globberCopyTo"));
 
         new Copy(this.context)
             .sources(globber(sourceDir, "*.{java,js}"))
@@ -298,11 +264,11 @@ public void globberCopy() throws Exception {
 
     @Test
     public void globberFileAndDirCopy() throws Exception {
-        final Path sourceDir = createEmptyDir(this.testCopyDir.resolve("globberFileAndDirCopy"), false);
+        final Path sourceDir = createDir(this.testCopyDir.resolve("globberFileAndDirCopy"));
         final Path sourceDirJavaFile = createFile(sourceDir.resolve("test.java"));
-        final Path sourceDirSubDir = createEmptyDir(sourceDir.resolve("subdir"), false);
+        final Path sourceDirSubDir = createDir(sourceDir.resolve("subdir"));
         final Path sourceDirSubDirKtFile = createFile(sourceDirSubDir.resolve("test.kt"));
-        final Path targetDir = createEmptyDir(this.testCopyDir.resolve("globberFileAndDirCopyTo"), false);
+        final Path targetDir = createDir(this.testCopyDir.resolve("globberFileAndDirCopyTo"));
 
         new Copy(this.context)
             .sources(globber(sourceDir, "*"))