Add recursive deletion in modify command (#1821)

Adds a method in the ModifyCommand to delete not only files, but also directories recursively.

gradle/changelog/recursive_delete.yaml

@@ -0,0 +1,2 @@
+- type: Added
+  description: Method to delete files recursively in modify command ([#1821](https://github.com/scm-manager/scm-manager/pull/1821))

scm-core/src/main/java/sonia/scm/repository/api/ModifyCommandBuilder.java

@@ -120,7 +120,11 @@ public class ModifyCommandBuilder {
    * @return This builder instance.
    */
   public ModifyCommandBuilder deleteFile(String path) {
-    request.addRequest(new ModifyCommandRequest.DeleteFileRequest(path));
+    return this.deleteFile(path, false);
+  }
+
+  public ModifyCommandBuilder deleteFile(String path, boolean recursive) {
+    request.addRequest(new ModifyCommandRequest.DeleteFileRequest(path, recursive));
     return this;
   }
 

scm-core/src/main/java/sonia/scm/repository/spi/ModifyCommand.java

@@ -21,7 +21,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-    
+
 package sonia.scm.repository.spi;
 
 import java.io.File;
@@ -32,7 +32,7 @@ public interface ModifyCommand {
   String execute(ModifyCommandRequest request);
 
   interface Worker {
-    void delete(String toBeDeleted) throws IOException;
+    void delete(String toBeDeleted, boolean recursive) throws IOException;
 
     void create(String toBeCreated, File file, boolean overwrite) throws IOException;
 

scm-core/src/main/java/sonia/scm/repository/spi/ModifyCommandRequest.java

@@ -128,14 +128,24 @@ public class ModifyCommandRequest implements Resetable, Validateable, CommandWit
 
   public static class DeleteFileRequest implements PartialRequest {
     private final String path;
+    private final boolean recursive;
 
+    /**
+     * @deprecated This is kept for compatibility, only. Use {@link #DeleteFileRequest(String, boolean)} instead.
+     */
+    @Deprecated
     public DeleteFileRequest(String path) {
+      this(path, false);
+    }
+
+    public DeleteFileRequest(String path, boolean recursive) {
       this.path = path;
+      this.recursive = recursive;
     }
 
     @Override
     public void execute(ModifyCommand.Worker worker) throws IOException {
-      worker.delete(path);
+      worker.delete(path, recursive);
     }
   }
 

scm-core/src/main/java/sonia/scm/repository/spi/ModifyWorkerHelper.java

@@ -29,6 +29,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import sonia.scm.ContextEntry;
 import sonia.scm.repository.Repository;
+import sonia.scm.util.IOUtil;
 
 import java.io.File;
 import java.io.IOException;
@@ -52,12 +53,16 @@ public interface ModifyWorkerHelper extends ModifyCommand.Worker {
   Logger LOG = LoggerFactory.getLogger(ModifyWorkerHelper.class);
 
   @Override
-  default void delete(String toBeDeleted) throws IOException {
+  default void delete(String toBeDeleted, boolean recursive) throws IOException {
     Path fileToBeDeleted = getTargetFile(toBeDeleted);
-    try {
-      Files.delete(fileToBeDeleted);
-    } catch (NoSuchFileException e) {
-      throw notFound(createFileContext(toBeDeleted));
+    if (recursive) {
+      IOUtil.delete(fileToBeDeleted.toFile());
+    } else {
+      try {
+        Files.delete(fileToBeDeleted);
+      } catch (NoSuchFileException e) {
+        throw notFound(createFileContext(toBeDeleted));
+      }
     }
     doScmDelete(toBeDeleted);
   }

scm-core/src/test/java/sonia/scm/repository/api/ModifyCommandBuilderTest.java

@@ -157,7 +157,16 @@ class ModifyCommandBuilderTest {
         .deleteFile("toBeDeleted")
         .execute();
 
-      verify(worker).delete("toBeDeleted");
+      verify(worker).delete("toBeDeleted", false);
+    }
+
+    @Test
+    void shouldExecuteRecursiveDelete() throws IOException {
+      initCommand()
+        .deleteFile("toBeDeleted", true)
+        .execute();
+
+      verify(worker).delete("toBeDeleted", true);
     }
 
     @Test

scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java

@@ -29,6 +29,7 @@ import org.eclipse.jgit.api.errors.GitAPIException;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.treewalk.TreeWalk;
 import org.junit.Test;
 import sonia.scm.AlreadyExistsException;
 import sonia.scm.BadRequestException;
@@ -46,6 +47,7 @@ import java.nio.file.Files;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.fail;
 import static org.awaitility.Awaitility.await;
 import static org.mockito.ArgumentMatchers.argThat;
 import static org.mockito.Mockito.description;
@@ -244,7 +246,7 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase {
 
     ModifyCommandRequest request = new ModifyCommandRequest();
     request.setCommitMessage("test commit");
-    request.addRequest(new ModifyCommandRequest.DeleteFileRequest("a.txt"));
+    request.addRequest(new ModifyCommandRequest.DeleteFileRequest("a.txt", false));
     request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det"));
 
     command.execute(request);
@@ -254,13 +256,41 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase {
     assertInTree(assertions);
   }
 
+  @Test
+  public void shouldDeleteExistingDirectory() throws IOException, GitAPIException {
+    GitModifyCommand command = createCommand();
+
+    ModifyCommandRequest request = new ModifyCommandRequest();
+    request.setCommitMessage("test commit");
+    request.addRequest(new ModifyCommandRequest.DeleteFileRequest("c", true));
+    request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det"));
+
+    command.execute(request);
+
+    try (Git git = new Git(createContext().open())) {
+      RevCommit lastCommit = getLastCommit(git);
+      try (RevWalk walk = new RevWalk(git.getRepository())) {
+        RevCommit commit = walk.parseCommit(lastCommit);
+        ObjectId treeId = commit.getTree().getId();
+        TreeWalk treeWalk = new TreeWalk(git.getRepository());
+        treeWalk.setRecursive(true);
+        treeWalk.addTree(treeId);
+        while (treeWalk.next()) {
+          if (treeWalk.getPathString().startsWith("c/")) {
+            fail("directory should be deleted");
+          }
+        }
+      }
+    }
+  }
+
   @Test(expected = NotFoundException.class)
   public void shouldThrowNotFoundExceptionWhenFileToDeleteDoesNotExist() {
     GitModifyCommand command = createCommand();
 
     ModifyCommandRequest request = new ModifyCommandRequest();
     request.setCommitMessage("test commit");
-    request.addRequest(new ModifyCommandRequest.DeleteFileRequest("no/such/file"));
+    request.addRequest(new ModifyCommandRequest.DeleteFileRequest("no/such/file", false));
     request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det"));
 
     command.execute(request);
@@ -273,7 +303,7 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase {
     ModifyCommandRequest request = new ModifyCommandRequest();
     request.setBranch("does-not-exist");
     request.setCommitMessage("test commit");
-    request.addRequest(new ModifyCommandRequest.DeleteFileRequest("a.txt"));
+    request.addRequest(new ModifyCommandRequest.DeleteFileRequest("a.txt", false));
     request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det"));
 
     command.execute(request);

scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgModifyCommandTest.java

@@ -65,13 +65,27 @@ public class HgModifyCommandTest extends AbstractHgCommandTestBase {
   @Test
   public void shouldRemoveFiles() {
     ModifyCommandRequest request = new ModifyCommandRequest();
-    request.addRequest(new ModifyCommandRequest.DeleteFileRequest("a.txt"));
+    request.addRequest(new ModifyCommandRequest.DeleteFileRequest("a.txt", false));
     request.setCommitMessage("this is great");
     request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com"));
 
     String result = hgModifyCommand.execute(request);
 
     assertThat(cmdContext.open().tip().getNode()).isEqualTo(result);
+    assertThat(cmdContext.open().tip().getDeletedFiles().size()).isEqualTo(1);
+  }
+
+  @Test
+  public void shouldRemoveDirectoriesRecursively() {
+    ModifyCommandRequest request = new ModifyCommandRequest();
+    request.addRequest(new ModifyCommandRequest.DeleteFileRequest("c", true));
+    request.setCommitMessage("this is great");
+    request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com"));
+
+    String result = hgModifyCommand.execute(request);
+
+    assertThat(cmdContext.open().tip().getNode()).isEqualTo(result);
+    assertThat(cmdContext.open().tip().getDeletedFiles().size()).isEqualTo(2);
   }
 
   @Test

scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/SvnModifyCommandTest.java

@@ -77,7 +77,7 @@ public class SvnModifyCommandTest extends AbstractSvnCommandTestBase {
   @Test
   public void shouldRemoveFiles() {
     ModifyCommandRequest request = new ModifyCommandRequest();
-    request.addRequest(new ModifyCommandRequest.DeleteFileRequest("a.txt"));
+    request.addRequest(new ModifyCommandRequest.DeleteFileRequest("a.txt", false));
     request.setCommitMessage("this is great");
     request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com"));
 
@@ -87,6 +87,19 @@ public class SvnModifyCommandTest extends AbstractSvnCommandTestBase {
     assertThat(new File(workingCopy.getWorkingRepository().getAbsolutePath() + "/c")).exists();
   }
 
+  @Test
+  public void shouldRemoveDirectory() {
+    ModifyCommandRequest request = new ModifyCommandRequest();
+    request.addRequest(new ModifyCommandRequest.DeleteFileRequest("c", true));
+    request.setCommitMessage("this is great");
+    request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com"));
+
+    svnModifyCommand.execute(request);
+    WorkingCopy<File, File> workingCopy = workingCopyFactory.createWorkingCopy(context, null);
+    assertThat(new File(workingCopy.getWorkingRepository().getAbsolutePath() + "/a.txt")).exists();
+    assertThat(new File(workingCopy.getWorkingRepository().getAbsolutePath() + "/c")).doesNotExist();
+  }
+
   @Test
   public void shouldAddNewFile() throws IOException {
     File testfile = temporaryFolder.newFile("Test123");