Delete temporary workdir after command was executed

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

@@ -4,10 +4,13 @@ import com.google.common.base.Preconditions;
 import com.google.common.io.ByteSource;
 import com.google.common.io.ByteStreams;
 import com.google.common.io.Files;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import sonia.scm.repository.Person;
 import sonia.scm.repository.spi.ModifyCommand;
 import sonia.scm.repository.spi.ModifyCommandRequest;
 import sonia.scm.repository.util.WorkdirProvider;
+import sonia.scm.util.IOUtil;
 
 import java.io.File;
 import java.io.IOException;
@@ -40,6 +43,8 @@ import java.util.function.Consumer;
  */
 public class ModifyCommandBuilder {
 
+  private static final Logger LOG = LoggerFactory.getLogger(ModifyCommandBuilder.class);
+
   private final ModifyCommand command;
   private final File workdir;
 
@@ -109,8 +114,16 @@ public class ModifyCommandBuilder {
    * @return The revision of the new commit.
    */
   public String execute() {
-    Preconditions.checkArgument(request.isValid(), "commit message, author and branch are required");
-    return command.execute(request);
+    try {
+      Preconditions.checkArgument(request.isValid(), "commit message, author and branch are required");
+      return command.execute(request);
+    } finally {
+      try {
+        IOUtil.delete(workdir);
+      } catch (IOException e) {
+        LOG.warn("could not delete temporary workdir '{}'", workdir, e);
+      }
+    }
   }
 
   /**

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

@@ -6,7 +6,6 @@ import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.junitpioneer.jupiter.TempDirectory;
 import org.mockito.ArgumentCaptor;
-import org.mockito.Captor;
 import org.mockito.Mock;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.junit.jupiter.MockitoExtension;
@@ -43,20 +42,25 @@ class ModifyCommandBuilderTest {
   @Mock
   ModifyCommand.Worker worker;
 
-  @Captor
-  ArgumentCaptor<ModifyCommandRequest> requestCaptor;
-
   ModifyCommandBuilder commandBuilder;
+  Path workdir;
 
   @BeforeEach
   void initWorkdir(@TempDirectory.TempDir Path temp) throws IOException {
-    lenient().when(workdirProvider.createNewWorkdir()).thenReturn(temp.toFile());
+    workdir = Files.createDirectory(temp.resolve("workdir"));
+    lenient().when(workdirProvider.createNewWorkdir()).thenReturn(workdir.toFile());
     commandBuilder = new ModifyCommandBuilder(command, workdirProvider);
   }
 
   @BeforeEach
   void initRequestCaptor() {
-    when(command.execute(requestCaptor.capture())).thenReturn("target");
+    when(command.execute(any())).thenAnswer(
+      invocation -> {
+        ModifyCommandRequest request = invocation.getArgument(0);
+        request.getRequests().forEach(r -> r.execute(worker));
+        return "target";
+      }
+    );
   }
 
   @Test
@@ -74,8 +78,6 @@ class ModifyCommandBuilderTest {
       .deleteFile("toBeDeleted")
       .execute();
 
-    executeRequest();
-
     verify(worker).delete("toBeDeleted");
   }
 
@@ -85,8 +87,6 @@ class ModifyCommandBuilderTest {
       .moveFile("source", "target")
       .execute();
 
-    executeRequest();
-
     verify(worker).move("source", "target");
   }
 
@@ -100,8 +100,6 @@ class ModifyCommandBuilderTest {
       .createFile("toBeCreated").withData(ByteSource.wrap("content".getBytes()))
       .execute();
 
-    executeRequest();
-
     assertThat(nameCaptor.getValue()).isEqualTo("toBeCreated");
     assertThat(contentCaptor).contains("content");
   }
@@ -116,8 +114,6 @@ class ModifyCommandBuilderTest {
       .createFile("toBeCreated").withData(new ByteArrayInputStream("content".getBytes()))
       .execute();
 
-    executeRequest();
-
     assertThat(nameCaptor.getValue()).isEqualTo("toBeCreated");
     assertThat(contentCaptor).contains("content");
   }
@@ -133,8 +129,6 @@ class ModifyCommandBuilderTest {
       .createFile("toBeCreated_2").withData(new ByteArrayInputStream("content_2".getBytes()))
       .execute();
 
-    executeRequest();
-
     List<String> createdNames = nameCaptor.getAllValues();
     assertThat(createdNames.get(0)).isEqualTo("toBeCreated_1");
     assertThat(createdNames.get(1)).isEqualTo("toBeCreated_2");
@@ -151,17 +145,10 @@ class ModifyCommandBuilderTest {
       .modifyFile("toBeModified").withData(ByteSource.wrap("content".getBytes()))
       .execute();
 
-    executeRequest();
-
     assertThat(nameCaptor.getValue()).isEqualTo("toBeModified");
     assertThat(contentCaptor).contains("content");
   }
 
-  private void executeRequest() {
-    ModifyCommandRequest request = requestCaptor.getValue();
-    request.getRequests().forEach(r -> r.execute(worker));
-  }
-
   private ModifyCommandBuilder initCommand() {
     return commandBuilder
       .setBranch("branch")
@@ -179,8 +166,6 @@ class ModifyCommandBuilderTest {
       .modifyFile("toBeModified").withData(ByteSource.wrap("content".getBytes()))
       .execute();
 
-    executeRequest();
-
     assertThat(Files.list(temp)).isEmpty();
   }