From c57ef73475bfe7005c85004b3d196f30d22d5844 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 28 Aug 2019 12:58:56 +0200 Subject: [PATCH] Introduce command request for modify command --- .../repository/api/ModifyCommandBuilder.java | 135 ++++-------------- .../scm/repository/spi/ModifyCommand.java | 13 +- .../repository/spi/ModifyCommandRequest.java | 133 +++++++++++++++++ .../api/ModifyCommandBuilderTest.java | 73 +++++++--- 4 files changed, 224 insertions(+), 130 deletions(-) create mode 100644 scm-core/src/main/java/sonia/scm/repository/spi/ModifyCommandRequest.java diff --git a/scm-core/src/main/java/sonia/scm/repository/api/ModifyCommandBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/ModifyCommandBuilder.java index 9653aed934..dc3ed4a331 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/ModifyCommandBuilder.java +++ b/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 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 contentConsumer; + private final Consumer contentConsumer; - private ContentLoader(Consumer contentConsumer) { + private ContentLoader(Consumer 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(); - } - } } diff --git a/scm-core/src/main/java/sonia/scm/repository/spi/ModifyCommand.java b/scm-core/src/main/java/sonia/scm/repository/spi/ModifyCommand.java index 8843965295..07b29e8d21 100644 --- a/scm-core/src/main/java/sonia/scm/repository/spi/ModifyCommand.java +++ b/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); + } } diff --git a/scm-core/src/main/java/sonia/scm/repository/spi/ModifyCommandRequest.java b/scm-core/src/main/java/sonia/scm/repository/spi/ModifyCommandRequest.java new file mode 100644 index 0000000000..057ec2d875 --- /dev/null +++ b/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 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 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(); + } + } +} diff --git a/scm-core/src/test/java/sonia/scm/repository/api/ModifyCommandBuilderTest.java b/scm-core/src/test/java/sonia/scm/repository/api/ModifyCommandBuilderTest.java index 86e8dc4fc1..adbefe7d2a 100644 --- a/scm-core/src/test/java/sonia/scm/repository/api/ModifyCommandBuilderTest.java +++ b/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 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 nameCaptor = ArgumentCaptor.forClass(String.class); List 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 nameCaptor = ArgumentCaptor.forClass(String.class); List 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 nameCaptor = ArgumentCaptor.forClass(String.class); List 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 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 nameCaptor = ArgumentCaptor.forClass(String.class); List 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 nameCaptor = ArgumentCaptor.forClass(String.class); ArgumentCaptor 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(); }