Merge with api changes

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/main/java/sonia/scm/repository/spi/ModifyCommandRequest.java

@@ -1,8 +1,11 @@
 package sonia.scm.repository.spi;
 
 import org.apache.commons.lang.StringUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import sonia.scm.Validateable;
 import sonia.scm.repository.Person;
+import sonia.scm.util.IOUtil;
 
 import java.io.File;
 import java.io.IOException;
@@ -12,6 +15,8 @@ import java.util.List;
 
 public class ModifyCommandRequest implements Resetable, Validateable {
 
+  private static final Logger LOG = LoggerFactory.getLogger(ModifyCommandRequest.class);
+
   private final List<PartialRequest> requests = new ArrayList<>();
 
   private Person author;
@@ -108,7 +113,11 @@ public class ModifyCommandRequest implements Resetable, Validateable {
     }
 
     void cleanup() {
-      content.delete(); // TODO Handle errors
+      try {
+        IOUtil.delete(content);
+      } catch (IOException e) {
+        LOG.warn("could not delete temporary file {}", content, 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,27 @@ 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);
+        for (ModifyCommandRequest.PartialRequest r : request.getRequests()) {
+          r.execute(worker);
+        }
+        return "target";
+      }
+    );
   }
 
   @Test
@@ -74,8 +80,6 @@ class ModifyCommandBuilderTest {
       .deleteFile("toBeDeleted")
       .execute();
 
-    executeRequest();
-
     verify(worker).delete("toBeDeleted");
   }
 
@@ -85,8 +89,6 @@ class ModifyCommandBuilderTest {
       .moveFile("source", "target")
       .execute();
 
-    executeRequest();
-
     verify(worker).move("source", "target");
   }
 
@@ -100,8 +102,6 @@ class ModifyCommandBuilderTest {
       .createFile("toBeCreated").withData(ByteSource.wrap("content".getBytes()))
       .execute();
 
-    executeRequest();
-
     assertThat(nameCaptor.getValue()).isEqualTo("toBeCreated");
     assertThat(contentCaptor).contains("content");
   }
@@ -116,8 +116,6 @@ class ModifyCommandBuilderTest {
       .createFile("toBeCreated").withData(new ByteArrayInputStream("content".getBytes()))
       .execute();
 
-    executeRequest();
-
     assertThat(nameCaptor.getValue()).isEqualTo("toBeCreated");
     assertThat(contentCaptor).contains("content");
   }
@@ -133,8 +131,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,19 +147,10 @@ class ModifyCommandBuilderTest {
       .modifyFile("toBeModified").withData(ByteSource.wrap("content".getBytes()))
       .execute();
 
-    executeRequest();
-
     assertThat(nameCaptor.getValue()).isEqualTo("toBeModified");
     assertThat(contentCaptor).contains("content");
   }
 
-  private void executeRequest() throws IOException {
-    ModifyCommandRequest request = requestCaptor.getValue();
-    for (ModifyCommandRequest.PartialRequest r : request.getRequests()) {
-      r.execute(worker);
-    }
-  }
-
   private ModifyCommandBuilder initCommand() {
     return commandBuilder
       .setBranch("branch")
@@ -181,8 +168,6 @@ class ModifyCommandBuilderTest {
       .modifyFile("toBeModified").withData(ByteSource.wrap("content".getBytes()))
       .execute();
 
-    executeRequest();
-
     assertThat(Files.list(temp)).isEmpty();
   }
 

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

@@ -66,6 +66,8 @@ public class GitModifyCommandTest extends AbstractGitCommandTestBase {
     request.addRequest(new ModifyCommandRequest.CreateFileRequest("new_file", newFile));
     request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det"));
 
+    command.execute(request);
+
     TreeAssertions assertions = canonicalTreeParser -> assertThat(canonicalTreeParser.findFile("new_file")).isTrue();
 
     assertInTree(assertions);