Introduce command request for modify command

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

@@ -1,17 +1,18 @@
 package sonia.scm.repository.api;
 
+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 sonia.scm.repository.Person;
 import sonia.scm.repository.spi.ModifyCommand;
+import sonia.scm.repository.spi.ModifyCommandRequest;
 import sonia.scm.repository.util.WorkdirProvider;
 
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
-import java.util.ArrayList;
-import java.util.List;
 import java.util.function.Consumer;
 
 public class ModifyCommandBuilder {
@@ -19,7 +20,7 @@ public class ModifyCommandBuilder {
   private final ModifyCommand command;
   private final File workdir;
 
-  private final List<FileModification> modifications = new ArrayList<>();
+  private final ModifyCommandRequest request = new ModifyCommandRequest();
 
   ModifyCommandBuilder(ModifyCommand command, WorkdirProvider workdirProvider) {
     this.command = command;
@@ -32,163 +33,81 @@ public class ModifyCommandBuilder {
 
   ContentLoader createFile(String path) {
     return new ContentLoader(
-      content -> modifications.add(new CreateFile(path, content))
+      content -> request.addRequest(new ModifyCommandRequest.CreateFileRequest(path, content))
     );
   }
 
   ContentLoader modifyFile(String path) {
     return new ContentLoader(
-      content -> modifications.add(new ModifyFile(path, content))
+      content -> request.addRequest(new ModifyCommandRequest.ModifyFileRequest(path, content))
     );
   }
 
   ModifyCommandBuilder deleteFile(String path) {
-    modifications.add(new DeleteFile(path));
+    request.addRequest(new ModifyCommandRequest.DeleteFileRequest(path));
     return this;
   }
 
   ModifyCommandBuilder moveFile(String sourcePath, String targetPath) {
-    modifications.add(new MoveFile(sourcePath, targetPath));
+    request.addRequest(new ModifyCommandRequest.MoveFileRequest(sourcePath, targetPath));
     return this;
   }
 
   String execute() {
-    modifications.forEach(FileModification::execute);
-    modifications.forEach(FileModification::cleanup);
-    return command.commit();
+    Preconditions.checkArgument(request.isValid(), "commit message, author and branch are required");
+    return command.execute(request);
   }
 
-  private Content loadData(ByteSource data) throws IOException {
+  private File loadData(ByteSource data) throws IOException {
     File file = createTemporaryFile();
     data.copyTo(Files.asByteSink(file));
-    return new Content(file);
+    return file;
   }
 
-  private Content loadData(InputStream data) throws IOException {
+  private File loadData(InputStream data) throws IOException {
     File file = createTemporaryFile();
     OutputStream out = Files.asByteSink(file).openBufferedStream();
     ByteStreams.copy(data, out);
     out.close();
-    return new Content(file);
+    return file;
   }
 
   private File createTemporaryFile() throws IOException {
     return File.createTempFile("upload-", "", workdir);
   }
 
-  private interface FileModification {
-    void execute();
-
-    default void cleanup() {
-    }
+  public ModifyCommandBuilder setCommitMessage(String message) {
+    request.setCommitMessage(message);
+    return this;
   }
 
-  private class DeleteFile implements FileModification {
-    private final String path;
-
-    public DeleteFile(String path) {
-      this.path = path;
-    }
-
-    @Override
-    public void execute() {
-      command.delete(path);
-    }
+  public ModifyCommandBuilder setAuthor(Person author) {
+    request.setAuthor(author);
+    return this;
   }
 
-  private class MoveFile implements FileModification {
-    private final String sourcePath;
-    private final String targetPath;
-
-    private MoveFile(String sourcePath, String targetPath) {
-      this.sourcePath = sourcePath;
-      this.targetPath = targetPath;
-    }
-
-    @Override
-    public void execute() {
-      command.move(sourcePath, targetPath);
-    }
-  }
-
-  private abstract class DataBasedModification implements FileModification {
-
-    private final Content content;
-
-    DataBasedModification(Content content) {
-      this.content = content;
-    }
-
-    public Content getContent() {
-      return content;
-    }
-    @Override
-    public void cleanup() {
-      content.deleteFile();
-    }
-  }
-
-  private class CreateFile extends DataBasedModification {
-
-    private final String path;
-
-    CreateFile(String path, Content content) {
-      super(content);
-      this.path = path;
-    }
-    @Override
-    public void execute() {
-      command.create(path, getContent().getFile());
-    }
-  }
-
-  private class ModifyFile extends DataBasedModification {
-
-    private final String path;
-
-    ModifyFile(String path, Content content) {
-      super(content);
-      this.path = path;
-    }
-    @Override
-    public void execute() {
-      command.modify(path, getContent().getFile());
-    }
+  public ModifyCommandBuilder setBranch(String branch) {
+    request.setBranch(branch);
+    return this;
   }
 
   public class ContentLoader {
 
-    private final Consumer<Content> contentConsumer;
+    private final Consumer<File> contentConsumer;
 
-    private ContentLoader(Consumer<Content> contentConsumer) {
+    private ContentLoader(Consumer<File> contentConsumer) {
       this.contentConsumer = contentConsumer;
     }
 
     public ModifyCommandBuilder withData(ByteSource data) throws IOException {
-      Content content = loadData(data);
+      File content = loadData(data);
       contentConsumer.accept(content);
       return ModifyCommandBuilder.this;
     }
     public ModifyCommandBuilder withData(InputStream data) throws IOException {
-      Content content = loadData(data);
+      File content = loadData(data);
       contentConsumer.accept(content);
       return ModifyCommandBuilder.this;
     }
   }
-
-  private class Content {
-
-    private final File file;
-
-    Content(File file) {
-      this.file = file;
-    }
-
-    private File getFile() {
-      return file;
-    }
-    public void deleteFile() {
-      file.delete();
-    }
-  }
 }

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

@@ -3,13 +3,16 @@ package sonia.scm.repository.spi;
 import java.io.File;
 
 public interface ModifyCommand {
-  String commit();
 
-  void delete(String toBeDeleted);
+  String execute(ModifyCommandRequest request);
 
-  void create(String toBeCreated, File file);
+  interface Worker {
+    void delete(String toBeDeleted);
 
-  void modify(String path, File file);
+    void create(String toBeCreated, File file);
 
-  void move(String sourcePath, String targetPath);
+    void modify(String path, File file);
+
+    void move(String sourcePath, String targetPath);
+  }
 }

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

@@ -0,0 +1,133 @@
+package sonia.scm.repository.spi;
+
+import org.apache.commons.lang.StringUtils;
+import sonia.scm.Validateable;
+import sonia.scm.repository.Person;
+
+import java.io.File;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+public class ModifyCommandRequest implements Resetable, Validateable {
+
+  private final List<PartialRequest> requests = new ArrayList<>();
+
+  private Person author;
+  private String commitMessage;
+  private String branch;
+
+  @Override
+  public void reset() {
+    requests.clear();
+    author = null;
+    commitMessage = null;
+    branch = null;
+  }
+
+  public void addRequest(PartialRequest request) {
+    this.requests.add(request);
+  }
+
+  public void setAuthor(Person author) {
+    this.author = author;
+  }
+
+  public void setCommitMessage(String commitMessage) {
+    this.commitMessage = commitMessage;
+  }
+
+  public void setBranch(String branch) {
+    this.branch = branch;
+  }
+
+  public List<PartialRequest> getRequests() {
+    return Collections.unmodifiableList(requests);
+  }
+
+  @Override
+  public boolean isValid() {
+    return StringUtils.isNotEmpty(commitMessage) && StringUtils.isNotEmpty(branch) && author != null && !requests.isEmpty();
+  }
+
+  public interface PartialRequest {
+    void execute(ModifyCommand.Worker worker);
+  }
+
+  public static class DeleteFileRequest implements PartialRequest {
+    private final String path;
+
+    public DeleteFileRequest(String path) {
+      this.path = path;
+    }
+
+    @Override
+    public void execute(ModifyCommand.Worker worker) {
+      worker.delete(path);
+    }
+  }
+
+  public static class MoveFileRequest implements PartialRequest {
+    private final String sourcePath;
+    private final String targetPath;
+
+    public MoveFileRequest(String sourcePath, String targetPath) {
+      this.sourcePath = sourcePath;
+      this.targetPath = targetPath;
+    }
+
+    @Override
+    public void execute(ModifyCommand.Worker worker) {
+      worker.move(sourcePath, targetPath);
+    }
+  }
+
+  private abstract static class ContentModificationRequest implements PartialRequest {
+
+    private final File content;
+
+    ContentModificationRequest(File content) {
+      this.content = content;
+    }
+
+    File getContent() {
+      return content;
+    }
+
+    void cleanup() {
+      content.delete(); // TODO Handle errors
+    }
+  }
+
+  public static class CreateFileRequest extends ContentModificationRequest {
+
+    private final String path;
+
+    public CreateFileRequest(String path, File content) {
+      super(content);
+      this.path = path;
+    }
+
+    @Override
+    public void execute(ModifyCommand.Worker worker) {
+      worker.create(path, getContent());
+      cleanup();
+    }
+  }
+
+  public static class ModifyFileRequest extends ContentModificationRequest {
+
+    private final String path;
+
+    public ModifyFileRequest(String path, File content) {
+      super(content);
+      this.path = path;
+    }
+
+    @Override
+    public void execute(ModifyCommand.Worker worker) {
+      worker.modify(path, getContent());
+      cleanup();
+    }
+  }
+}

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

@@ -6,11 +6,14 @@ 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;
 import org.mockito.stubbing.Answer;
+import sonia.scm.repository.Person;
 import sonia.scm.repository.spi.ModifyCommand;
+import sonia.scm.repository.spi.ModifyCommandRequest;
 import sonia.scm.repository.util.WorkdirProvider;
 
 import java.io.ByteArrayInputStream;
@@ -37,6 +40,11 @@ class ModifyCommandBuilderTest {
   ModifyCommand command;
   @Mock
   WorkdirProvider workdirProvider;
+  @Mock
+  ModifyCommand.Worker worker;
+
+  @Captor
+  ArgumentCaptor<ModifyCommandRequest> requestCaptor;
 
   ModifyCommandBuilder commandBuilder;
 
@@ -46,43 +54,54 @@ class ModifyCommandBuilderTest {
     commandBuilder = new ModifyCommandBuilder(command, workdirProvider);
   }
 
+  @BeforeEach
+  void initRequestCaptor() {
+    when(command.execute(requestCaptor.capture())).thenReturn("target");
+  }
+
   @Test
   void shouldReturnTargetRevisionFromCommit() {
-    when(command.commit()).thenReturn("target");
-
-    String targetRevision = commandBuilder.execute();
+    String targetRevision = initCommand()
+      .deleteFile("toBeDeleted")
+      .execute();
 
     assertThat(targetRevision).isEqualTo("target");
   }
 
   @Test
   void shouldExecuteDelete() {
-    commandBuilder
+    initCommand()
       .deleteFile("toBeDeleted")
       .execute();
 
-    verify(command).delete("toBeDeleted");
+    executeRequest();
+
+    verify(worker).delete("toBeDeleted");
   }
 
   @Test
   void shouldExecuteMove() {
-    commandBuilder
+    initCommand()
       .moveFile("source", "target")
       .execute();
 
-    verify(command).move("source", "target");
+    executeRequest();
+
+    verify(worker).move("source", "target");
   }
 
   @Test
   void shouldExecuteCreateWithByteSourceContent() throws IOException {
     ArgumentCaptor<String> nameCaptor = ArgumentCaptor.forClass(String.class);
     List<String> contentCaptor = new ArrayList<>();
-    doAnswer(new ExtractContent(contentCaptor)).when(command).create(nameCaptor.capture(), any());
+    doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any());
 
-    commandBuilder
+    initCommand()
       .createFile("toBeCreated").withData(ByteSource.wrap("content".getBytes()))
       .execute();
 
+    executeRequest();
+
     assertThat(nameCaptor.getValue()).isEqualTo("toBeCreated");
     assertThat(contentCaptor).contains("content");
   }
@@ -91,12 +110,14 @@ class ModifyCommandBuilderTest {
   void shouldExecuteCreateWithInputStreamContent() throws IOException {
     ArgumentCaptor<String> nameCaptor = ArgumentCaptor.forClass(String.class);
     List<String> contentCaptor = new ArrayList<>();
-    doAnswer(new ExtractContent(contentCaptor)).when(command).create(nameCaptor.capture(), any());
+    doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any());
 
-    commandBuilder
+    initCommand()
       .createFile("toBeCreated").withData(new ByteArrayInputStream("content".getBytes()))
       .execute();
 
+    executeRequest();
+
     assertThat(nameCaptor.getValue()).isEqualTo("toBeCreated");
     assertThat(contentCaptor).contains("content");
   }
@@ -105,13 +126,15 @@ class ModifyCommandBuilderTest {
   void shouldExecuteCreateMultipleTimes() throws IOException {
     ArgumentCaptor<String> nameCaptor = ArgumentCaptor.forClass(String.class);
     List<String> contentCaptor = new ArrayList<>();
-    doAnswer(new ExtractContent(contentCaptor)).when(command).create(nameCaptor.capture(), any());
+    doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any());
 
-    commandBuilder
+    initCommand()
       .createFile("toBeCreated_1").withData(new ByteArrayInputStream("content_1".getBytes()))
       .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");
@@ -122,26 +145,42 @@ class ModifyCommandBuilderTest {
   void shouldExecuteModify() throws IOException {
     ArgumentCaptor<String> nameCaptor = ArgumentCaptor.forClass(String.class);
     List<String> contentCaptor = new ArrayList<>();
-    doAnswer(new ExtractContent(contentCaptor)).when(command).modify(nameCaptor.capture(), any());
+    doAnswer(new ExtractContent(contentCaptor)).when(worker).modify(nameCaptor.capture(), any());
 
-    commandBuilder
+    initCommand()
       .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")
+      .setCommitMessage("message")
+      .setAuthor(new Person());
+  }
+
   @Test
   void shouldDeleteTemporaryFiles(@TempDirectory.TempDir Path temp) throws IOException {
     ArgumentCaptor<String> nameCaptor = ArgumentCaptor.forClass(String.class);
     ArgumentCaptor<File> fileCaptor = ArgumentCaptor.forClass(File.class);
-    doNothing().when(command).modify(nameCaptor.capture(), fileCaptor.capture());
+    doNothing().when(worker).modify(nameCaptor.capture(), fileCaptor.capture());
 
-    commandBuilder
+    initCommand()
       .modifyFile("toBeModified").withData(ByteSource.wrap("content".getBytes()))
       .execute();
 
+    executeRequest();
+
     assertThat(Files.list(temp)).isEmpty();
   }