From 230ac848ebd72ff476d925a7b03fbf0d4b447580 Mon Sep 17 00:00:00 2001 From: Eduard Heimbuch Date: Tue, 20 Aug 2019 16:39:50 +0200 Subject: [PATCH 01/35] ignore old plugins folder --- .../src/main/java/sonia/scm/plugin/PluginProcessor.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/scm-webapp/src/main/java/sonia/scm/plugin/PluginProcessor.java b/scm-webapp/src/main/java/sonia/scm/plugin/PluginProcessor.java index b91ee9b1ee..0613f4ef07 100644 --- a/scm-webapp/src/main/java/sonia/scm/plugin/PluginProcessor.java +++ b/scm-webapp/src/main/java/sonia/scm/plugin/PluginProcessor.java @@ -60,6 +60,8 @@ import java.util.Date; import java.util.List; import java.util.Set; +import static java.util.stream.Collectors.toList; + //~--- JDK imports ------------------------------------------------------------ /** @@ -171,7 +173,11 @@ public final class PluginProcessor extract(archives); - List dirs = collectPluginDirectories(pluginDirectory); + List dirs = + collectPluginDirectories(pluginDirectory) + .stream() + .filter(dir -> !dir.endsWith("sonia.scm.plugins")) + .collect(toList()); logger.debug("process {} directories: {}", dirs.size(), dirs); From 72fe69b2d55ef8b177344404026719cabb6482d7 Mon Sep 17 00:00:00 2001 From: Eduard Heimbuch Date: Wed, 21 Aug 2019 10:34:40 +0200 Subject: [PATCH 02/35] add new IllegalIdentifierChangeException --- .../scm/IllegalIdentifierChangeException.java | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 scm-core/src/main/java/sonia/scm/IllegalIdentifierChangeException.java diff --git a/scm-core/src/main/java/sonia/scm/IllegalIdentifierChangeException.java b/scm-core/src/main/java/sonia/scm/IllegalIdentifierChangeException.java new file mode 100644 index 0000000000..63b9d0d4fc --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/IllegalIdentifierChangeException.java @@ -0,0 +1,21 @@ +package sonia.scm; + +import java.util.Collections; + +public class IllegalIdentifierChangeException extends BadRequestException { + + private static final String CODE = "thbsUFokjk"; + + public IllegalIdentifierChangeException(ContextEntry.ContextBuilder context, String message) { + super(context.build(), message); + } + + public IllegalIdentifierChangeException(String message) { + super(Collections.emptyList(), message); + } + + @Override + public String getCode() { + return CODE; + } +} From 7a29bba3398c52cd0ed0947edb1ef185e78a6d54 Mon Sep 17 00:00:00 2001 From: Eduard Heimbuch Date: Wed, 21 Aug 2019 10:36:48 +0200 Subject: [PATCH 03/35] use new IllegalIdentifierChangeException in SingleResourceManagerAdapter --- .../scm/api/v2/resources/SingleResourceManagerAdapter.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/SingleResourceManagerAdapter.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/SingleResourceManagerAdapter.java index a7b9146d00..76371a0b54 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/SingleResourceManagerAdapter.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/SingleResourceManagerAdapter.java @@ -2,6 +2,7 @@ package sonia.scm.api.v2.resources; import de.otto.edison.hal.HalRepresentation; import sonia.scm.ConcurrentModificationException; +import sonia.scm.IllegalIdentifierChangeException; import sonia.scm.Manager; import sonia.scm.ModelObject; import sonia.scm.NotFoundException; @@ -11,8 +12,6 @@ import java.util.function.Function; import java.util.function.Predicate; import java.util.function.Supplier; -import static javax.ws.rs.core.Response.Status.BAD_REQUEST; - /** * Adapter from resource http endpoints to managers, for Single resources (e.g. {@code /user/name}). * @@ -55,7 +54,7 @@ class SingleResourceManagerAdapter Date: Tue, 27 Aug 2019 08:52:38 +0200 Subject: [PATCH 04/35] pluginprocessor only consider directories that contains plugin.xml --- .../src/main/java/sonia/scm/plugin/PluginProcessor.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/scm-webapp/src/main/java/sonia/scm/plugin/PluginProcessor.java b/scm-webapp/src/main/java/sonia/scm/plugin/PluginProcessor.java index 0613f4ef07..6f1b18034e 100644 --- a/scm-webapp/src/main/java/sonia/scm/plugin/PluginProcessor.java +++ b/scm-webapp/src/main/java/sonia/scm/plugin/PluginProcessor.java @@ -59,6 +59,7 @@ import java.util.ArrayList; import java.util.Date; import java.util.List; import java.util.Set; +import java.util.function.Predicate; import static java.util.stream.Collectors.toList; @@ -176,7 +177,7 @@ public final class PluginProcessor List dirs = collectPluginDirectories(pluginDirectory) .stream() - .filter(dir -> !dir.endsWith("sonia.scm.plugins")) + .filter(isPluginDirectory()) .collect(toList()); logger.debug("process {} directories: {}", dirs.size(), dirs); @@ -200,6 +201,10 @@ public final class PluginProcessor return ImmutableSet.copyOf(wrappers); } + private Predicate isPluginDirectory() { + return dir -> new File(dir.resolve("META-INF/scm/plugin.xml").toString()).exists(); + } + /** * Method description * From 840b44af378c194e613e0bafcfb0404fd4b837ef Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Tue, 27 Aug 2019 11:41:36 +0000 Subject: [PATCH 05/35] Close branch bugfix/redirect_collection_links From b36c2dd698f14d5e951e41870ca2dada2c6d8300 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Tue, 27 Aug 2019 13:59:14 +0200 Subject: [PATCH 06/35] use nio api and added test for plugin directory check --- .../main/java/sonia/scm/plugin/PluginProcessor.java | 3 ++- .../java/sonia/scm/plugin/PluginProcessorTest.java | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/scm-webapp/src/main/java/sonia/scm/plugin/PluginProcessor.java b/scm-webapp/src/main/java/sonia/scm/plugin/PluginProcessor.java index 6f1b18034e..5588349cc8 100644 --- a/scm-webapp/src/main/java/sonia/scm/plugin/PluginProcessor.java +++ b/scm-webapp/src/main/java/sonia/scm/plugin/PluginProcessor.java @@ -71,6 +71,7 @@ import static java.util.stream.Collectors.toList; * * TODO don't mix nio and io */ +@SuppressWarnings("squid:S3725") // performance is not critical, for this type of checks public final class PluginProcessor { @@ -202,7 +203,7 @@ public final class PluginProcessor } private Predicate isPluginDirectory() { - return dir -> new File(dir.resolve("META-INF/scm/plugin.xml").toString()).exists(); + return dir -> Files.exists(dir.resolve(DIRECTORY_METAINF).resolve("scm").resolve("plugin.xml")); } /** diff --git a/scm-webapp/src/test/java/sonia/scm/plugin/PluginProcessorTest.java b/scm-webapp/src/test/java/sonia/scm/plugin/PluginProcessorTest.java index 87e9cbf7b7..e9eaea3afa 100644 --- a/scm-webapp/src/test/java/sonia/scm/plugin/PluginProcessorTest.java +++ b/scm-webapp/src/test/java/sonia/scm/plugin/PluginProcessorTest.java @@ -134,6 +134,16 @@ public class PluginProcessorTest assertThat(plugin.getId(), is(PLUGIN_A.id)); } + @Test + public void shouldCollectPluginsAndDoNotFailOnNonPluginDirectories() throws IOException { + new File(pluginDirectory, "some-directory").mkdirs(); + + copySmp(PLUGIN_A); + PluginWrapper plugin = collectAndGetFirst(); + + assertThat(plugin.getId(), is(PLUGIN_A.id)); + } + /** * Method description * From 3cd1cdfa4f6bf3efd002e40a31d8b7da4d9987eb Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Tue, 27 Aug 2019 12:30:25 +0000 Subject: [PATCH 07/35] Close branch bugfix/ignore_old_plugin_dir From c42b3cca0c281673e1804342110c83eb00ccc396 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Tue, 27 Aug 2019 13:16:39 +0000 Subject: [PATCH 08/35] Close branch bugfix/ci_status_validation From 79037afdf8a6ba91b070801b25eec0b5379fbdb8 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Tue, 27 Aug 2019 15:20:38 +0200 Subject: [PATCH 09/35] update maven-deploy-plugin from 2.7 to 2.8.2 --- pom.xml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 3166c6ec75..3ab3cb82a3 100644 --- a/pom.xml +++ b/pom.xml @@ -439,6 +439,13 @@ smp-maven-plugin 1.0.0-alpha-6 + + + org.apache.maven.plugins + maven-deploy-plugin + 2.8.2 + + @@ -633,7 +640,6 @@ org.apache.maven.plugins maven-deploy-plugin - 2.7 From 769207c2c15af17b6a894333adf439700bdbd99b Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Tue, 27 Aug 2019 15:33:30 +0200 Subject: [PATCH 10/35] fixed compilation error --- .../src/test/java/sonia/scm/plugin/PluginProcessorTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scm-webapp/src/test/java/sonia/scm/plugin/PluginProcessorTest.java b/scm-webapp/src/test/java/sonia/scm/plugin/PluginProcessorTest.java index dc5a9add68..bb518ec731 100644 --- a/scm-webapp/src/test/java/sonia/scm/plugin/PluginProcessorTest.java +++ b/scm-webapp/src/test/java/sonia/scm/plugin/PluginProcessorTest.java @@ -139,7 +139,7 @@ public class PluginProcessorTest new File(pluginDirectory, "some-directory").mkdirs(); copySmp(PLUGIN_A); - PluginWrapper plugin = collectAndGetFirst(); + InstalledPlugin plugin = collectAndGetFirst(); assertThat(plugin.getId(), is(PLUGIN_A.id)); } From 4f21756d007c40f23cbed0f1c1f020154385adf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 27 Aug 2019 16:56:44 +0200 Subject: [PATCH 11/35] Extract class to provide temporary work directories --- .../repository/util/SimpleWorkdirFactory.java | 20 +++---------- .../scm/repository/util/WorkdirProvider.java | 29 +++++++++++++++++++ .../util/SimpleWorkdirFactoryTest.java | 3 +- .../spi/SimpleGitWorkdirFactory.java | 8 ++--- .../repository/spi/GitBranchCommandTest.java | 5 ++-- .../repository/spi/GitMergeCommandTest.java | 3 +- .../spi/SimpleGitWorkdirFactoryTest.java | 11 ++++--- .../spi/SimpleHgWorkdirFactory.java | 4 ++- .../repository/spi/HgBranchCommandTest.java | 3 +- 9 files changed, 55 insertions(+), 31 deletions(-) create mode 100644 scm-core/src/main/java/sonia/scm/repository/util/WorkdirProvider.java diff --git a/scm-core/src/main/java/sonia/scm/repository/util/SimpleWorkdirFactory.java b/scm-core/src/main/java/sonia/scm/repository/util/SimpleWorkdirFactory.java index 0872242612..7236e0c3fe 100644 --- a/scm-core/src/main/java/sonia/scm/repository/util/SimpleWorkdirFactory.java +++ b/scm-core/src/main/java/sonia/scm/repository/util/SimpleWorkdirFactory.java @@ -7,29 +7,21 @@ import sonia.scm.repository.Repository; import java.io.File; import java.io.IOException; -import java.nio.file.Files; public abstract class SimpleWorkdirFactory implements WorkdirFactory { private static final Logger logger = LoggerFactory.getLogger(SimpleWorkdirFactory.class); - private final File poolDirectory; + private final WorkdirProvider workdirProvider; - public SimpleWorkdirFactory() { - this(new File(System.getProperty("scm.workdir" , System.getProperty("java.io.tmpdir")), "scm-work")); - } - - public SimpleWorkdirFactory(File poolDirectory) { - this.poolDirectory = poolDirectory; - if (!poolDirectory.exists() && !poolDirectory.mkdirs()) { - throw new IllegalStateException("could not create pool directory " + poolDirectory); - } + public SimpleWorkdirFactory(WorkdirProvider workdirProvider) { + this.workdirProvider = workdirProvider; } @Override public WorkingCopy createWorkingCopy(C context) { try { - File directory = createNewWorkdir(); + File directory = workdirProvider.createNewWorkdir(); ParentAndClone parentAndClone = cloneRepository(context, directory); return new WorkingCopy<>(parentAndClone.getClone(), parentAndClone.getParent(), this::close, directory); } catch (IOException e) { @@ -45,10 +37,6 @@ public abstract class SimpleWorkdirFactory implements WorkdirFactory protected abstract ParentAndClone cloneRepository(C context, File target) throws IOException; - private File createNewWorkdir() throws IOException { - return Files.createTempDirectory(poolDirectory.toPath(),"workdir").toFile(); - } - private void close(R repository) { try { closeRepository(repository); diff --git a/scm-core/src/main/java/sonia/scm/repository/util/WorkdirProvider.java b/scm-core/src/main/java/sonia/scm/repository/util/WorkdirProvider.java new file mode 100644 index 0000000000..35dae56faa --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/repository/util/WorkdirProvider.java @@ -0,0 +1,29 @@ +package sonia.scm.repository.util; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; + +public class WorkdirProvider { + + private final File poolDirectory; + + public WorkdirProvider() { + this(new File(System.getProperty("scm.workdir" , System.getProperty("java.io.tmpdir")), "scm-work")); + } + + public WorkdirProvider(File poolDirectory) { + this.poolDirectory = poolDirectory; + if (!poolDirectory.exists() && !poolDirectory.mkdirs()) { + throw new IllegalStateException("could not create pool directory " + poolDirectory); + } + } + + public File createNewWorkdir() { + try { + return Files.createTempDirectory(poolDirectory.toPath(),"workdir").toFile(); + } catch (IOException e) { + throw new RuntimeException("could not create temporary workdir", e); + } + } +} diff --git a/scm-core/src/test/java/sonia/scm/repository/util/SimpleWorkdirFactoryTest.java b/scm-core/src/test/java/sonia/scm/repository/util/SimpleWorkdirFactoryTest.java index 2d3c2ed59e..31aa11c604 100644 --- a/scm-core/src/test/java/sonia/scm/repository/util/SimpleWorkdirFactoryTest.java +++ b/scm-core/src/test/java/sonia/scm/repository/util/SimpleWorkdirFactoryTest.java @@ -28,7 +28,8 @@ public class SimpleWorkdirFactoryTest { @Before public void initFactory() throws IOException { - simpleWorkdirFactory = new SimpleWorkdirFactory(temporaryFolder.newFolder()) { + WorkdirProvider workdirProvider = new WorkdirProvider(temporaryFolder.newFolder()); + simpleWorkdirFactory = new SimpleWorkdirFactory(workdirProvider) { @Override protected Repository getScmRepository(Context context) { return REPOSITORY; diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/SimpleGitWorkdirFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/SimpleGitWorkdirFactory.java index c30ff59afb..28042f0834 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/SimpleGitWorkdirFactory.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/SimpleGitWorkdirFactory.java @@ -7,16 +7,14 @@ import org.eclipse.jgit.transport.ScmTransportProtocol; import sonia.scm.repository.GitWorkdirFactory; import sonia.scm.repository.InternalRepositoryException; import sonia.scm.repository.util.SimpleWorkdirFactory; +import sonia.scm.repository.util.WorkdirProvider; import java.io.File; public class SimpleGitWorkdirFactory extends SimpleWorkdirFactory implements GitWorkdirFactory { - public SimpleGitWorkdirFactory() { - } - - SimpleGitWorkdirFactory(File poolDirectory) { - super(poolDirectory); + public SimpleGitWorkdirFactory(WorkdirProvider workdirProvider) { + super(workdirProvider); } @Override diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitBranchCommandTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitBranchCommandTest.java index e429aa5a42..aa5e641b31 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitBranchCommandTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitBranchCommandTest.java @@ -5,6 +5,7 @@ import org.junit.Rule; import org.junit.Test; import sonia.scm.repository.Branch; import sonia.scm.repository.api.BranchRequest; +import sonia.scm.repository.util.WorkdirProvider; import java.io.IOException; import java.util.List; @@ -25,7 +26,7 @@ public class GitBranchCommandTest extends AbstractGitCommandTestBase { branchRequest.setParentBranch(source.getName()); branchRequest.setNewBranch("new_branch"); - new GitBranchCommand(context, repository, new SimpleGitWorkdirFactory()).branch(branchRequest); + new GitBranchCommand(context, repository, new SimpleGitWorkdirFactory(new WorkdirProvider())).branch(branchRequest); Branch newBranch = findBranch(context, "new_branch"); Assertions.assertThat(newBranch.getRevision()).isEqualTo(source.getRevision()); @@ -45,7 +46,7 @@ public class GitBranchCommandTest extends AbstractGitCommandTestBase { BranchRequest branchRequest = new BranchRequest(); branchRequest.setNewBranch("new_branch"); - new GitBranchCommand(context, repository, new SimpleGitWorkdirFactory()).branch(branchRequest); + new GitBranchCommand(context, repository, new SimpleGitWorkdirFactory(new WorkdirProvider())).branch(branchRequest); Assertions.assertThat(readBranches(context)).filteredOn(b -> b.getName().equals("new_branch")).isNotEmpty(); } diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMergeCommandTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMergeCommandTest.java index e8a62f5b86..8b3b4548d9 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMergeCommandTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMergeCommandTest.java @@ -24,6 +24,7 @@ import sonia.scm.repository.RepositoryManager; import sonia.scm.repository.api.HookContextFactory; import sonia.scm.repository.api.MergeCommandResult; import sonia.scm.repository.api.MergeDryRunCommandResult; +import sonia.scm.repository.util.WorkdirProvider; import sonia.scm.user.User; import java.io.IOException; @@ -244,6 +245,6 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { } private GitMergeCommand createCommand() { - return new GitMergeCommand(createContext(), repository, new SimpleGitWorkdirFactory()); + return new GitMergeCommand(createContext(), repository, new SimpleGitWorkdirFactory(new WorkdirProvider())); } } diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/SimpleGitWorkdirFactoryTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/SimpleGitWorkdirFactoryTest.java index 616df5e68e..1b3c730ef1 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/SimpleGitWorkdirFactoryTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/SimpleGitWorkdirFactoryTest.java @@ -11,6 +11,7 @@ import sonia.scm.repository.GitRepositoryHandler; import sonia.scm.repository.PreProcessorUtil; import sonia.scm.repository.RepositoryManager; import sonia.scm.repository.api.HookContextFactory; +import sonia.scm.repository.util.WorkdirProvider; import sonia.scm.repository.util.WorkingCopy; import java.io.File; @@ -29,19 +30,21 @@ public class SimpleGitWorkdirFactoryTest extends AbstractGitCommandTestBase { // keep this so that it will not be garbage collected (Transport keeps this in a week reference) private ScmTransportProtocol proto; + private WorkdirProvider workdirProvider; @Before - public void bindScmProtocol() { + public void bindScmProtocol() throws IOException { HookContextFactory hookContextFactory = new HookContextFactory(mock(PreProcessorUtil.class)); HookEventFacade hookEventFacade = new HookEventFacade(of(mock(RepositoryManager.class)), hookContextFactory); GitRepositoryHandler gitRepositoryHandler = mock(GitRepositoryHandler.class); proto = new ScmTransportProtocol(of(hookEventFacade), of(gitRepositoryHandler)); Transport.register(proto); + workdirProvider = new WorkdirProvider(temporaryFolder.newFolder()); } @Test public void emptyPoolShouldCreateNewWorkdir() throws IOException { - SimpleGitWorkdirFactory factory = new SimpleGitWorkdirFactory(temporaryFolder.newFolder()); + SimpleGitWorkdirFactory factory = new SimpleGitWorkdirFactory(workdirProvider); File masterRepo = createRepositoryDirectory(); try (WorkingCopy workingCopy = factory.createWorkingCopy(createContext())) { @@ -59,7 +62,7 @@ public class SimpleGitWorkdirFactoryTest extends AbstractGitCommandTestBase { @Test public void cloneFromPoolShouldNotBeReused() throws IOException { - SimpleGitWorkdirFactory factory = new SimpleGitWorkdirFactory(temporaryFolder.newFolder()); + SimpleGitWorkdirFactory factory = new SimpleGitWorkdirFactory(workdirProvider); File firstDirectory; try (WorkingCopy workingCopy = factory.createWorkingCopy(createContext())) { @@ -73,7 +76,7 @@ public class SimpleGitWorkdirFactoryTest extends AbstractGitCommandTestBase { @Test public void cloneFromPoolShouldBeDeletedOnClose() throws IOException { - SimpleGitWorkdirFactory factory = new SimpleGitWorkdirFactory(temporaryFolder.newFolder()); + SimpleGitWorkdirFactory factory = new SimpleGitWorkdirFactory(workdirProvider); File directory; try (WorkingCopy workingCopy = factory.createWorkingCopy(createContext())) { diff --git a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/SimpleHgWorkdirFactory.java b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/SimpleHgWorkdirFactory.java index 9565f672d5..b9194145e6 100644 --- a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/SimpleHgWorkdirFactory.java +++ b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/SimpleHgWorkdirFactory.java @@ -4,6 +4,7 @@ import com.aragost.javahg.Repository; import com.aragost.javahg.commands.CloneCommand; import com.aragost.javahg.commands.PullCommand; import sonia.scm.repository.util.SimpleWorkdirFactory; +import sonia.scm.repository.util.WorkdirProvider; import sonia.scm.web.HgRepositoryEnvironmentBuilder; import javax.inject.Inject; @@ -18,7 +19,8 @@ public class SimpleHgWorkdirFactory extends SimpleWorkdirFactory hgRepositoryEnvironmentBuilder; @Inject - public SimpleHgWorkdirFactory(Provider hgRepositoryEnvironmentBuilder) { + public SimpleHgWorkdirFactory(Provider hgRepositoryEnvironmentBuilder, WorkdirProvider workdirProvider) { + super(workdirProvider); this.hgRepositoryEnvironmentBuilder = hgRepositoryEnvironmentBuilder; } @Override diff --git a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgBranchCommandTest.java b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgBranchCommandTest.java index c015aa0f62..7976af8b3b 100644 --- a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgBranchCommandTest.java +++ b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgBranchCommandTest.java @@ -7,6 +7,7 @@ import org.junit.Test; import sonia.scm.repository.Branch; import sonia.scm.repository.HgTestUtil; import sonia.scm.repository.api.BranchRequest; +import sonia.scm.repository.util.WorkdirProvider; import sonia.scm.web.HgRepositoryEnvironmentBuilder; import java.io.IOException; @@ -20,7 +21,7 @@ public class HgBranchCommandTest extends AbstractHgCommandTestBase { HgRepositoryEnvironmentBuilder hgRepositoryEnvironmentBuilder = new HgRepositoryEnvironmentBuilder(handler, HgTestUtil.createHookManager()); - SimpleHgWorkdirFactory workdirFactory = new SimpleHgWorkdirFactory(Providers.of(hgRepositoryEnvironmentBuilder)) { + SimpleHgWorkdirFactory workdirFactory = new SimpleHgWorkdirFactory(Providers.of(hgRepositoryEnvironmentBuilder), new WorkdirProvider()) { @Override public void configure(PullCommand pullCommand) { // we do not want to configure http hooks in this unit test From 44d99fcc9d8ff093dccfd55453baddeb35ff9ca2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 27 Aug 2019 16:57:10 +0200 Subject: [PATCH 12/35] Create new modification command --- .../api/ModificationCommandBuilder.java | 194 ++++++++++++++++++ .../repository/spi/ModificationCommand.java | 15 ++ .../api/ModificationCommandBuilderTest.java | 160 +++++++++++++++ 3 files changed, 369 insertions(+) create mode 100644 scm-core/src/main/java/sonia/scm/repository/api/ModificationCommandBuilder.java create mode 100644 scm-core/src/main/java/sonia/scm/repository/spi/ModificationCommand.java create mode 100644 scm-core/src/test/java/sonia/scm/repository/api/ModificationCommandBuilderTest.java diff --git a/scm-core/src/main/java/sonia/scm/repository/api/ModificationCommandBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/ModificationCommandBuilder.java new file mode 100644 index 0000000000..863fb1731b --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/repository/api/ModificationCommandBuilder.java @@ -0,0 +1,194 @@ +package sonia.scm.repository.api; + +import com.google.common.io.ByteSource; +import com.google.common.io.ByteStreams; +import com.google.common.io.Files; +import sonia.scm.repository.spi.ModificationCommand; +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 ModificationCommandBuilder { + + private final ModificationCommand command; + private final File workdir; + + private final List modifications = new ArrayList<>(); + + ModificationCommandBuilder(ModificationCommand command, WorkdirProvider workdirProvider) { + this.command = command; + this.workdir = workdirProvider.createNewWorkdir(); + } + + ModificationCommandBuilder setBranchToModify(String branchToModify) { + return this; + } + + ContentLoader createFile(String path) { + return new ContentLoader( + content -> modifications.add(new CreateFile(path, content)) + ); + } + + ContentLoader modifyFile(String path) { + return new ContentLoader( + content -> modifications.add(new ModifyFile(path, content)) + ); + } + + ModificationCommandBuilder deleteFile(String path) { + modifications.add(new DeleteFile(path)); + return this; + } + + ModificationCommandBuilder moveFile(String sourcePath, String targetPath) { + modifications.add(new MoveFile(sourcePath, targetPath)); + return this; + } + + String execute() { + modifications.forEach(FileModification::execute); + modifications.forEach(FileModification::cleanup); + return command.commit(); + } + + private Content loadData(ByteSource data) throws IOException { + File file = createTemporaryFile(); + data.copyTo(Files.asByteSink(file)); + return new Content(file); + } + + private Content loadData(InputStream data) throws IOException { + File file = createTemporaryFile(); + OutputStream out = Files.asByteSink(file).openBufferedStream(); + ByteStreams.copy(data, out); + out.close(); + return new Content(file); + } + + private File createTemporaryFile() throws IOException { + return File.createTempFile("upload-", "", workdir); + } + + private interface FileModification { + void execute(); + + default void cleanup() { + } + } + + private class DeleteFile implements FileModification { + private final String path; + + public DeleteFile(String path) { + this.path = path; + } + + @Override + public void execute() { + command.delete(path); + } + } + + 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 class ContentLoader { + + private final Consumer contentConsumer; + + private ContentLoader(Consumer contentConsumer) { + this.contentConsumer = contentConsumer; + } + + public ModificationCommandBuilder withData(ByteSource data) throws IOException { + Content content = loadData(data); + contentConsumer.accept(content); + return ModificationCommandBuilder.this; + } + public ModificationCommandBuilder withData(InputStream data) throws IOException { + Content content = loadData(data); + contentConsumer.accept(content); + return ModificationCommandBuilder.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/ModificationCommand.java b/scm-core/src/main/java/sonia/scm/repository/spi/ModificationCommand.java new file mode 100644 index 0000000000..c221acc1d6 --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/repository/spi/ModificationCommand.java @@ -0,0 +1,15 @@ +package sonia.scm.repository.spi; + +import java.io.File; + +public interface ModificationCommand { + String commit(); + + void delete(String toBeDeleted); + + void create(String toBeCreated, File file); + + void modify(String path, File file); + + void move(String sourcePath, String targetPath); +} diff --git a/scm-core/src/test/java/sonia/scm/repository/api/ModificationCommandBuilderTest.java b/scm-core/src/test/java/sonia/scm/repository/api/ModificationCommandBuilderTest.java new file mode 100644 index 0000000000..f27b89964c --- /dev/null +++ b/scm-core/src/test/java/sonia/scm/repository/api/ModificationCommandBuilderTest.java @@ -0,0 +1,160 @@ +package sonia.scm.repository.api; + +import com.google.common.io.ByteSource; +import org.junit.jupiter.api.BeforeEach; +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.Mock; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.stubbing.Answer; +import sonia.scm.repository.spi.ModificationCommand; +import sonia.scm.repository.util.WorkdirProvider; + +import java.io.ByteArrayInputStream; +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +@ExtendWith(TempDirectory.class) +class ModificationCommandBuilderTest { + + @Mock + ModificationCommand command; + @Mock + WorkdirProvider workdirProvider; + + ModificationCommandBuilder commandBuilder; + + @BeforeEach + void initWorkdir(@TempDirectory.TempDir Path temp) throws IOException { + lenient().when(workdirProvider.createNewWorkdir()).thenReturn(temp.toFile()); + commandBuilder = new ModificationCommandBuilder(command, workdirProvider); + } + + @Test + void shouldReturnTargetRevisionFromCommit() { + when(command.commit()).thenReturn("target"); + + String targetRevision = commandBuilder.execute(); + + assertThat(targetRevision).isEqualTo("target"); + } + + @Test + void shouldExecuteDelete() { + commandBuilder + .deleteFile("toBeDeleted") + .execute(); + + verify(command).delete("toBeDeleted"); + } + + @Test + void shouldExecuteMove() { + commandBuilder + .moveFile("source", "target") + .execute(); + + verify(command).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()); + + commandBuilder + .createFile("toBeCreated").withData(ByteSource.wrap("content".getBytes())) + .execute(); + + assertThat(nameCaptor.getValue()).isEqualTo("toBeCreated"); + assertThat(contentCaptor).contains("content"); + } + + @Test + void shouldExecuteCreateWithInputStreamContent() throws IOException { + ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); + List contentCaptor = new ArrayList<>(); + doAnswer(new ExtractContent(contentCaptor)).when(command).create(nameCaptor.capture(), any()); + + commandBuilder + .createFile("toBeCreated").withData(new ByteArrayInputStream("content".getBytes())) + .execute(); + + assertThat(nameCaptor.getValue()).isEqualTo("toBeCreated"); + assertThat(contentCaptor).contains("content"); + } + + @Test + void shouldExecuteCreateMultipleTimes() throws IOException { + ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); + List contentCaptor = new ArrayList<>(); + doAnswer(new ExtractContent(contentCaptor)).when(command).create(nameCaptor.capture(), any()); + + commandBuilder + .createFile("toBeCreated_1").withData(new ByteArrayInputStream("content_1".getBytes())) + .createFile("toBeCreated_2").withData(new ByteArrayInputStream("content_2".getBytes())) + .execute(); + + List createdNames = nameCaptor.getAllValues(); + assertThat(createdNames.get(0)).isEqualTo("toBeCreated_1"); + assertThat(createdNames.get(1)).isEqualTo("toBeCreated_2"); + assertThat(contentCaptor).contains("content_1", "content_2"); + } + + @Test + void shouldExecuteModify() throws IOException { + ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); + List contentCaptor = new ArrayList<>(); + doAnswer(new ExtractContent(contentCaptor)).when(command).modify(nameCaptor.capture(), any()); + + commandBuilder + .modifyFile("toBeModified").withData(ByteSource.wrap("content".getBytes())) + .execute(); + + assertThat(nameCaptor.getValue()).isEqualTo("toBeModified"); + assertThat(contentCaptor).contains("content"); + } + + @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()); + + commandBuilder + .modifyFile("toBeModified").withData(ByteSource.wrap("content".getBytes())) + .execute(); + + assertThat(Files.list(temp)).isEmpty(); + } + + private static class ExtractContent implements Answer { + private final List contentCaptor; + + public ExtractContent(List contentCaptor) { + this.contentCaptor = contentCaptor; + } + + @Override + public Object answer(InvocationOnMock invocation) throws Throwable { + return contentCaptor.add(Files.readAllLines(((File) invocation.getArgument(1)).toPath()).get(0)); + } + } +} From 956d9a42c8a3eb6ad17d518ce78422128b6d037b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 28 Aug 2019 06:32:15 +0200 Subject: [PATCH 13/35] Fix injection --- .../java/sonia/scm/repository/spi/SimpleGitWorkdirFactory.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/SimpleGitWorkdirFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/SimpleGitWorkdirFactory.java index 28042f0834..9edcf0c0ea 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/SimpleGitWorkdirFactory.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/SimpleGitWorkdirFactory.java @@ -9,10 +9,12 @@ import sonia.scm.repository.InternalRepositoryException; import sonia.scm.repository.util.SimpleWorkdirFactory; import sonia.scm.repository.util.WorkdirProvider; +import javax.inject.Inject; import java.io.File; public class SimpleGitWorkdirFactory extends SimpleWorkdirFactory implements GitWorkdirFactory { + @Inject public SimpleGitWorkdirFactory(WorkdirProvider workdirProvider) { super(workdirProvider); } From 57e748b4e4ef2bd45dc5239d9947422a0964ba50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 28 Aug 2019 06:43:18 +0200 Subject: [PATCH 14/35] Rename class to distinguish between 'modifications' and 'modify' --- ...Builder.java => ModifyCommandBuilder.java} | 22 +++++++++---------- ...icationCommand.java => ModifyCommand.java} | 2 +- ...est.java => ModifyCommandBuilderTest.java} | 10 ++++----- 3 files changed, 17 insertions(+), 17 deletions(-) rename scm-core/src/main/java/sonia/scm/repository/api/{ModificationCommandBuilder.java => ModifyCommandBuilder.java} (85%) rename scm-core/src/main/java/sonia/scm/repository/spi/{ModificationCommand.java => ModifyCommand.java} (86%) rename scm-core/src/test/java/sonia/scm/repository/api/{ModificationCommandBuilderTest.java => ModifyCommandBuilderTest.java} (95%) diff --git a/scm-core/src/main/java/sonia/scm/repository/api/ModificationCommandBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/ModifyCommandBuilder.java similarity index 85% rename from scm-core/src/main/java/sonia/scm/repository/api/ModificationCommandBuilder.java rename to scm-core/src/main/java/sonia/scm/repository/api/ModifyCommandBuilder.java index 863fb1731b..9653aed934 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/ModificationCommandBuilder.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/ModifyCommandBuilder.java @@ -3,7 +3,7 @@ package sonia.scm.repository.api; import com.google.common.io.ByteSource; import com.google.common.io.ByteStreams; import com.google.common.io.Files; -import sonia.scm.repository.spi.ModificationCommand; +import sonia.scm.repository.spi.ModifyCommand; import sonia.scm.repository.util.WorkdirProvider; import java.io.File; @@ -14,19 +14,19 @@ import java.util.ArrayList; import java.util.List; import java.util.function.Consumer; -public class ModificationCommandBuilder { +public class ModifyCommandBuilder { - private final ModificationCommand command; + private final ModifyCommand command; private final File workdir; private final List modifications = new ArrayList<>(); - ModificationCommandBuilder(ModificationCommand command, WorkdirProvider workdirProvider) { + ModifyCommandBuilder(ModifyCommand command, WorkdirProvider workdirProvider) { this.command = command; this.workdir = workdirProvider.createNewWorkdir(); } - ModificationCommandBuilder setBranchToModify(String branchToModify) { + ModifyCommandBuilder setBranchToModify(String branchToModify) { return this; } @@ -42,12 +42,12 @@ public class ModificationCommandBuilder { ); } - ModificationCommandBuilder deleteFile(String path) { + ModifyCommandBuilder deleteFile(String path) { modifications.add(new DeleteFile(path)); return this; } - ModificationCommandBuilder moveFile(String sourcePath, String targetPath) { + ModifyCommandBuilder moveFile(String sourcePath, String targetPath) { modifications.add(new MoveFile(sourcePath, targetPath)); return this; } @@ -164,15 +164,15 @@ public class ModificationCommandBuilder { this.contentConsumer = contentConsumer; } - public ModificationCommandBuilder withData(ByteSource data) throws IOException { + public ModifyCommandBuilder withData(ByteSource data) throws IOException { Content content = loadData(data); contentConsumer.accept(content); - return ModificationCommandBuilder.this; + return ModifyCommandBuilder.this; } - public ModificationCommandBuilder withData(InputStream data) throws IOException { + public ModifyCommandBuilder withData(InputStream data) throws IOException { Content content = loadData(data); contentConsumer.accept(content); - return ModificationCommandBuilder.this; + return ModifyCommandBuilder.this; } } diff --git a/scm-core/src/main/java/sonia/scm/repository/spi/ModificationCommand.java b/scm-core/src/main/java/sonia/scm/repository/spi/ModifyCommand.java similarity index 86% rename from scm-core/src/main/java/sonia/scm/repository/spi/ModificationCommand.java rename to scm-core/src/main/java/sonia/scm/repository/spi/ModifyCommand.java index c221acc1d6..8843965295 100644 --- a/scm-core/src/main/java/sonia/scm/repository/spi/ModificationCommand.java +++ b/scm-core/src/main/java/sonia/scm/repository/spi/ModifyCommand.java @@ -2,7 +2,7 @@ package sonia.scm.repository.spi; import java.io.File; -public interface ModificationCommand { +public interface ModifyCommand { String commit(); void delete(String toBeDeleted); diff --git a/scm-core/src/test/java/sonia/scm/repository/api/ModificationCommandBuilderTest.java b/scm-core/src/test/java/sonia/scm/repository/api/ModifyCommandBuilderTest.java similarity index 95% rename from scm-core/src/test/java/sonia/scm/repository/api/ModificationCommandBuilderTest.java rename to scm-core/src/test/java/sonia/scm/repository/api/ModifyCommandBuilderTest.java index f27b89964c..86e8dc4fc1 100644 --- a/scm-core/src/test/java/sonia/scm/repository/api/ModificationCommandBuilderTest.java +++ b/scm-core/src/test/java/sonia/scm/repository/api/ModifyCommandBuilderTest.java @@ -10,7 +10,7 @@ import org.mockito.Mock; import org.mockito.invocation.InvocationOnMock; import org.mockito.junit.jupiter.MockitoExtension; import org.mockito.stubbing.Answer; -import sonia.scm.repository.spi.ModificationCommand; +import sonia.scm.repository.spi.ModifyCommand; import sonia.scm.repository.util.WorkdirProvider; import java.io.ByteArrayInputStream; @@ -31,19 +31,19 @@ import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) @ExtendWith(TempDirectory.class) -class ModificationCommandBuilderTest { +class ModifyCommandBuilderTest { @Mock - ModificationCommand command; + ModifyCommand command; @Mock WorkdirProvider workdirProvider; - ModificationCommandBuilder commandBuilder; + ModifyCommandBuilder commandBuilder; @BeforeEach void initWorkdir(@TempDirectory.TempDir Path temp) throws IOException { lenient().when(workdirProvider.createNewWorkdir()).thenReturn(temp.toFile()); - commandBuilder = new ModificationCommandBuilder(command, workdirProvider); + commandBuilder = new ModifyCommandBuilder(command, workdirProvider); } @Test 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 15/35] 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(); } From a8ce0f028d6a2fc55b05616f98c2f4c0ec1461ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 28 Aug 2019 13:03:51 +0200 Subject: [PATCH 16/35] Add missing getter in reuest --- .../scm/repository/spi/ModifyCommandRequest.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) 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 index 057ec2d875..f3dcca18ff 100644 --- a/scm-core/src/main/java/sonia/scm/repository/spi/ModifyCommandRequest.java +++ b/scm-core/src/main/java/sonia/scm/repository/spi/ModifyCommandRequest.java @@ -45,6 +45,18 @@ public class ModifyCommandRequest implements Resetable, Validateable { return Collections.unmodifiableList(requests); } + public Person getAuthor() { + return author; + } + + public String getCommitMessage() { + return commitMessage; + } + + public String getBranch() { + return branch; + } + @Override public boolean isValid() { return StringUtils.isNotEmpty(commitMessage) && StringUtils.isNotEmpty(branch) && author != null && !requests.isEmpty(); From e0ecb3440bc9db4c2f3bf176efb5c2410af1a80d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 28 Aug 2019 13:19:59 +0200 Subject: [PATCH 17/35] Add modify command --- scm-core/src/main/java/sonia/scm/repository/api/Command.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scm-core/src/main/java/sonia/scm/repository/api/Command.java b/scm-core/src/main/java/sonia/scm/repository/api/Command.java index 3249e54ec3..8e95e314bb 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/Command.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/Command.java @@ -66,5 +66,5 @@ public enum Command /** * @since 2.0 */ - MODIFICATIONS, MERGE, DIFF_RESULT, BRANCH; + MODIFICATIONS, MERGE, DIFF_RESULT, BRANCH, MODIFY; } From baa4f6a91731ffb77d8b71522c4be4ee0d084dc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 28 Aug 2019 13:53:24 +0200 Subject: [PATCH 18/35] Make modify command available --- .../repository/api/ModifyCommandBuilder.java | 160 +++++++++++++----- .../scm/repository/api/RepositoryService.java | 31 +++- .../api/RepositoryServiceFactory.java | 5 +- .../spi/RepositoryServiceProvider.java | 8 + .../repository/api/RepositoryServiceTest.java | 6 +- 5 files changed, 162 insertions(+), 48 deletions(-) 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 dc3ed4a331..501a2ac3b0 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 @@ -15,6 +15,29 @@ import java.io.InputStream; import java.io.OutputStream; import java.util.function.Consumer; +/** + * Use this {@link ModifyCommandBuilder} to make file changes to the head of a branch. You can + *
    + *
  • create new files ({@link #createFile(String)}
    • + *
  • modify existing files ({@link #modifyFile(String)}
    • + *
  • delete existing files ({@link #deleteFile(String)}
    • + *
  • move/rename existing files ({@link #moveFile(String, String)}
    • + *
+ * + * You can collect multiple changes before they are executed with a call to {@link #execute()}. + * + *

Example: + * 

+ * commandBuilder
+ *     .setBranch("feature/branch")
+ *     .setCommitMessage("make some changes")
+ *     .setAuthor(new Person())
+ *     .createFile("file/to/create").withData(inputStream)
+ *     .deleteFile("old/file/to/delete")
+ *     .execute();
+ *
+ *

+ */ public class ModifyCommandBuilder { private final ModifyCommand command; @@ -27,37 +50,127 @@ public class ModifyCommandBuilder { this.workdir = workdirProvider.createNewWorkdir(); } - ModifyCommandBuilder setBranchToModify(String branchToModify) { + /** + * Set the branch that should be modified. The new commit will be made for this branch. + * @param branchToModify The branch to modify. + * @return This builder instance. + */ + public ModifyCommandBuilder setBranchToModify(String branchToModify) { return this; } - ContentLoader createFile(String path) { + /** + * Create a new file. The content of the file will be specified in a subsequent call to + * {@link ContentLoader#withData(ByteSource)} or {@link ContentLoader#withData(InputStream)}. + * @param path The path and the name of the file that should be created. + * @return The loader to specify the content of the new file. + */ + public ContentLoader createFile(String path) { return new ContentLoader( content -> request.addRequest(new ModifyCommandRequest.CreateFileRequest(path, content)) ); } - ContentLoader modifyFile(String path) { + /** + * Modify an existing file. The new content of the file will be specified in a subsequent call to + * {@link ContentLoader#withData(ByteSource)} or {@link ContentLoader#withData(InputStream)}. + * @param path The path and the name of the file that should be modified. + * @return The loader to specify the new content of the file. + */ + public ContentLoader modifyFile(String path) { return new ContentLoader( content -> request.addRequest(new ModifyCommandRequest.ModifyFileRequest(path, content)) ); } - ModifyCommandBuilder deleteFile(String path) { + /** + * Delete an existing file. + * @param path The path and the name of the file that should be deleted. + * @return This builder instance. + */ + public ModifyCommandBuilder deleteFile(String path) { request.addRequest(new ModifyCommandRequest.DeleteFileRequest(path)); return this; } - ModifyCommandBuilder moveFile(String sourcePath, String targetPath) { + /** + * Move an existing file. + * @param sourcePath The path and the name of the file that should be moved. + * @param targetPath The new path and name the file should be moved to. + * @return This builder instance. + */ + public ModifyCommandBuilder moveFile(String sourcePath, String targetPath) { request.addRequest(new ModifyCommandRequest.MoveFileRequest(sourcePath, targetPath)); return this; } - String execute() { + /** + * Apply the changes and create a new commit with the given message and author. + * @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); } + /** + * Set the commit message for the new commit. + * @return This builder instance. + */ + public ModifyCommandBuilder setCommitMessage(String message) { + request.setCommitMessage(message); + return this; + } + + /** + * Set the author for the new commit. + * @return This builder instance. + */ + public ModifyCommandBuilder setAuthor(Person author) { + request.setAuthor(author); + return this; + } + + /** + * Set the branch the changes should be made upon. + * @return This builder instance. + */ + public ModifyCommandBuilder setBranch(String branch) { + request.setBranch(branch); + return this; + } + + public class ContentLoader { + + private final Consumer contentConsumer; + + private ContentLoader(Consumer contentConsumer) { + this.contentConsumer = contentConsumer; + } + + /** + * Specify the data of the file using a {@link ByteSource}. + * @return The builder instance. + * @throws IOException If the data could not be read. + */ + public ModifyCommandBuilder withData(ByteSource data) throws IOException { + File content = loadData(data); + contentConsumer.accept(content); + return ModifyCommandBuilder.this; + } + + /** + * Specify the data of the file using an {@link InputStream}. + * @return The builder instance. + * @throws IOException If the data could not be read. + */ + public ModifyCommandBuilder withData(InputStream data) throws IOException { + File content = loadData(data); + contentConsumer.accept(content); + return ModifyCommandBuilder.this; + } + } + private File loadData(ByteSource data) throws IOException { File file = createTemporaryFile(); data.copyTo(Files.asByteSink(file)); @@ -75,39 +188,4 @@ public class ModifyCommandBuilder { private File createTemporaryFile() throws IOException { return File.createTempFile("upload-", "", workdir); } - - public ModifyCommandBuilder setCommitMessage(String message) { - request.setCommitMessage(message); - return this; - } - - public ModifyCommandBuilder setAuthor(Person author) { - request.setAuthor(author); - return this; - } - - public ModifyCommandBuilder setBranch(String branch) { - request.setBranch(branch); - return this; - } - - public class ContentLoader { - - private final Consumer contentConsumer; - - private ContentLoader(Consumer contentConsumer) { - this.contentConsumer = contentConsumer; - } - - public ModifyCommandBuilder withData(ByteSource data) throws IOException { - File content = loadData(data); - contentConsumer.accept(content); - return ModifyCommandBuilder.this; - } - public ModifyCommandBuilder withData(InputStream data) throws IOException { - File content = loadData(data); - contentConsumer.accept(content); - return ModifyCommandBuilder.this; - } - } } diff --git a/scm-core/src/main/java/sonia/scm/repository/api/RepositoryService.java b/scm-core/src/main/java/sonia/scm/repository/api/RepositoryService.java index 5807ffa998..080d66ebf2 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/RepositoryService.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/RepositoryService.java @@ -40,6 +40,7 @@ import sonia.scm.repository.PreProcessorUtil; import sonia.scm.repository.Repository; import sonia.scm.repository.RepositoryPermissions; import sonia.scm.repository.spi.RepositoryServiceProvider; +import sonia.scm.repository.util.WorkdirProvider; import java.io.Closeable; import java.io.IOException; @@ -91,22 +92,25 @@ public final class RepositoryService implements Closeable { private final RepositoryServiceProvider provider; private final Repository repository; private final Set protocolProviders; + private final WorkdirProvider workdirProvider; /** * Constructs a new {@link RepositoryService}. This constructor should only * be called from the {@link RepositoryServiceFactory}. - * @param cacheManager cache manager + * @param cacheManager cache manager * @param provider implementation for {@link RepositoryServiceProvider} * @param repository the repository + * @param workdirProvider */ RepositoryService(CacheManager cacheManager, - RepositoryServiceProvider provider, Repository repository, - PreProcessorUtil preProcessorUtil, Set protocolProviders) { + RepositoryServiceProvider provider, Repository repository, + PreProcessorUtil preProcessorUtil, Set protocolProviders, WorkdirProvider workdirProvider) { this.cacheManager = cacheManager; this.provider = provider; this.repository = repository; this.preProcessorUtil = preProcessorUtil; this.protocolProviders = protocolProviders; + this.workdirProvider = workdirProvider; } /** @@ -399,6 +403,27 @@ public final class RepositoryService implements Closeable { return new MergeCommandBuilder(provider.getMergeCommand()); } + /** + * The modify command makes changes to the head of a branch. It is possible to + *
    + *
  • create new files
    • + *
  • delete existing files
    • + *
  • modify/replace files
    • + *
  • move files
    • + *
+ * + * @return instance of {@link ModifyCommandBuilder} + * @throws CommandNotSupportedException if the command is not supported + * by the implementation of the repository service provider. + * @since 2.0.0 + */ + public ModifyCommandBuilder getModifyCommand() { + LOG.debug("create modify command for repository {}", + repository.getNamespaceAndName()); + + return new ModifyCommandBuilder(provider.getModifyCommand(), workdirProvider); + } + /** * Returns true if the command is supported by the repository service. * diff --git a/scm-core/src/main/java/sonia/scm/repository/api/RepositoryServiceFactory.java b/scm-core/src/main/java/sonia/scm/repository/api/RepositoryServiceFactory.java index 8db3ab7546..fe2af9f9e1 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/RepositoryServiceFactory.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/RepositoryServiceFactory.java @@ -61,6 +61,7 @@ import sonia.scm.repository.RepositoryManager; import sonia.scm.repository.RepositoryPermissions; import sonia.scm.repository.spi.RepositoryServiceProvider; import sonia.scm.repository.spi.RepositoryServiceResolver; +import sonia.scm.repository.util.WorkdirProvider; import sonia.scm.security.ScmSecurityException; import java.util.Set; @@ -256,7 +257,7 @@ public final class RepositoryServiceFactory } service = new RepositoryService(cacheManager, provider, repository, - preProcessorUtil, protocolProviders); + preProcessorUtil, protocolProviders, workdirProvider); break; } @@ -373,4 +374,6 @@ public final class RepositoryServiceFactory private final Set resolvers; private Set protocolProviders; + + private WorkdirProvider workdirProvider; } diff --git a/scm-core/src/main/java/sonia/scm/repository/spi/RepositoryServiceProvider.java b/scm-core/src/main/java/sonia/scm/repository/spi/RepositoryServiceProvider.java index bf9cdf6a25..cdd0417cf7 100644 --- a/scm-core/src/main/java/sonia/scm/repository/spi/RepositoryServiceProvider.java +++ b/scm-core/src/main/java/sonia/scm/repository/spi/RepositoryServiceProvider.java @@ -275,4 +275,12 @@ public abstract class RepositoryServiceProvider implements Closeable { throw new CommandNotSupportedException(Command.MERGE); } + + /** + * @since 2.0 + */ + public ModifyCommand getModifyCommand() + { + throw new CommandNotSupportedException(Command.MODIFY); + } } diff --git a/scm-core/src/test/java/sonia/scm/repository/api/RepositoryServiceTest.java b/scm-core/src/test/java/sonia/scm/repository/api/RepositoryServiceTest.java index 2ceafc19bb..330a4c956a 100644 --- a/scm-core/src/test/java/sonia/scm/repository/api/RepositoryServiceTest.java +++ b/scm-core/src/test/java/sonia/scm/repository/api/RepositoryServiceTest.java @@ -24,7 +24,7 @@ public class RepositoryServiceTest { @Test public void shouldReturnMatchingProtocolsFromProvider() { - RepositoryService repositoryService = new RepositoryService(null, provider, repository, null, Collections.singleton(new DummyScmProtocolProvider())); + RepositoryService repositoryService = new RepositoryService(null, provider, repository, null, Collections.singleton(new DummyScmProtocolProvider()), null); Stream supportedProtocols = repositoryService.getSupportedProtocols(); assertThat(sizeOf(supportedProtocols.collect(Collectors.toList()))).isEqualTo(1); @@ -32,7 +32,7 @@ public class RepositoryServiceTest { @Test public void shouldFindKnownProtocol() { - RepositoryService repositoryService = new RepositoryService(null, provider, repository, null, Collections.singleton(new DummyScmProtocolProvider())); + RepositoryService repositoryService = new RepositoryService(null, provider, repository, null, Collections.singleton(new DummyScmProtocolProvider()), null); HttpScmProtocol protocol = repositoryService.getProtocol(HttpScmProtocol.class); @@ -41,7 +41,7 @@ public class RepositoryServiceTest { @Test public void shouldFailForUnknownProtocol() { - RepositoryService repositoryService = new RepositoryService(null, provider, repository, null, Collections.singleton(new DummyScmProtocolProvider())); + RepositoryService repositoryService = new RepositoryService(null, provider, repository, null, Collections.singleton(new DummyScmProtocolProvider()), null); assertThrows(IllegalArgumentException.class, () -> { repositoryService.getProtocol(UnknownScmProtocol.class); From 55ac40363d35b608b374e2db39a0d18554576aef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 28 Aug 2019 13:57:39 +0200 Subject: [PATCH 19/35] Add missing injection --- .../scm/repository/api/RepositoryServiceFactory.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/repository/api/RepositoryServiceFactory.java b/scm-core/src/main/java/sonia/scm/repository/api/RepositoryServiceFactory.java index fe2af9f9e1..07c0b8fd47 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/RepositoryServiceFactory.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/RepositoryServiceFactory.java @@ -136,13 +136,14 @@ public final class RepositoryServiceFactory * @param resolvers a set of {@link RepositoryServiceResolver} * @param preProcessorUtil helper object for pre processor handling * + * @param workdirProvider * @since 1.21 */ @Inject public RepositoryServiceFactory(ScmConfiguration configuration, - CacheManager cacheManager, RepositoryManager repositoryManager, - Set resolvers, PreProcessorUtil preProcessorUtil, - Set protocolProviders) + CacheManager cacheManager, RepositoryManager repositoryManager, + Set resolvers, PreProcessorUtil preProcessorUtil, + Set protocolProviders, WorkdirProvider workdirProvider) { this.configuration = configuration; this.cacheManager = cacheManager; @@ -150,6 +151,7 @@ public final class RepositoryServiceFactory this.resolvers = resolvers; this.preProcessorUtil = preProcessorUtil; this.protocolProviders = protocolProviders; + this.workdirProvider = workdirProvider; ScmEventBus.getInstance().register(new CacheClearHook(cacheManager)); } @@ -375,5 +377,5 @@ public final class RepositoryServiceFactory private Set protocolProviders; - private WorkdirProvider workdirProvider; + private final WorkdirProvider workdirProvider; } From 406620bd9d3c1805f59bf822f9cfb904e49f7894 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 28 Aug 2019 15:07:14 +0200 Subject: [PATCH 20/35] First steps to implement create in modify command --- .../scm/repository/spi/ModifyCommand.java | 3 +- .../repository/spi/ModifyCommandRequest.java | 5 +- .../api/ModifyCommandBuilderTest.java | 10 +- .../scm/repository/spi/GitModifyCommand.java | 173 ++++++++++++++++++ .../spi/GitRepositoryServiceProvider.java | 5 + .../repository/spi/GitMergeCommandTest.java | 13 -- .../repository/spi/GitModifyCommandTest.java | 31 ++++ 7 files changed, 220 insertions(+), 20 deletions(-) create mode 100644 scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java create mode 100644 scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java 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 07b29e8d21..a8b3efd6d9 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 @@ -1,6 +1,7 @@ package sonia.scm.repository.spi; import java.io.File; +import java.io.IOException; public interface ModifyCommand { @@ -9,7 +10,7 @@ public interface ModifyCommand { interface Worker { void delete(String toBeDeleted); - void create(String toBeCreated, File file); + void create(String toBeCreated, File file) throws IOException; void modify(String path, File file); 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 index f3dcca18ff..1ef92dab0a 100644 --- a/scm-core/src/main/java/sonia/scm/repository/spi/ModifyCommandRequest.java +++ b/scm-core/src/main/java/sonia/scm/repository/spi/ModifyCommandRequest.java @@ -5,6 +5,7 @@ import sonia.scm.Validateable; import sonia.scm.repository.Person; import java.io.File; +import java.io.IOException; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -63,7 +64,7 @@ public class ModifyCommandRequest implements Resetable, Validateable { } public interface PartialRequest { - void execute(ModifyCommand.Worker worker); + void execute(ModifyCommand.Worker worker) throws IOException; } public static class DeleteFileRequest implements PartialRequest { @@ -121,7 +122,7 @@ public class ModifyCommandRequest implements Resetable, Validateable { } @Override - public void execute(ModifyCommand.Worker worker) { + public void execute(ModifyCommand.Worker worker) throws IOException { worker.create(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 adbefe7d2a..e2dfb955a5 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 @@ -69,7 +69,7 @@ class ModifyCommandBuilderTest { } @Test - void shouldExecuteDelete() { + void shouldExecuteDelete() throws IOException { initCommand() .deleteFile("toBeDeleted") .execute(); @@ -80,7 +80,7 @@ class ModifyCommandBuilderTest { } @Test - void shouldExecuteMove() { + void shouldExecuteMove() throws IOException { initCommand() .moveFile("source", "target") .execute(); @@ -157,9 +157,11 @@ class ModifyCommandBuilderTest { assertThat(contentCaptor).contains("content"); } - private void executeRequest() { + private void executeRequest() throws IOException { ModifyCommandRequest request = requestCaptor.getValue(); - request.getRequests().forEach(r -> r.execute(worker)); + for (ModifyCommandRequest.PartialRequest r : request.getRequests()) { + r.execute(worker); + } } private ModifyCommandBuilder initCommand() { diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java new file mode 100644 index 0000000000..8256b7e766 --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java @@ -0,0 +1,173 @@ +package sonia.scm.repository.spi; + +import org.apache.shiro.SecurityUtils; +import org.apache.shiro.subject.Subject; +import org.eclipse.jgit.api.Git; +import org.eclipse.jgit.api.errors.GitAPIException; +import org.eclipse.jgit.api.errors.RefNotFoundException; +import org.eclipse.jgit.lib.ObjectId; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import sonia.scm.repository.GitWorkdirFactory; +import sonia.scm.repository.InternalRepositoryException; +import sonia.scm.repository.Person; +import sonia.scm.repository.Repository; +import sonia.scm.repository.util.WorkingCopy; +import sonia.scm.user.User; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +import static sonia.scm.ContextEntry.ContextBuilder.entity; +import static sonia.scm.NotFoundException.notFound; + +public class GitModifyCommand extends AbstractGitCommand implements ModifyCommand { + + private static final Logger LOG = LoggerFactory.getLogger(GitModifyCommand.class); + + private final GitWorkdirFactory workdirFactory; + + public GitModifyCommand(GitContext context, Repository repository, GitWorkdirFactory workdirFactory) { + super(context, repository); + this.workdirFactory = workdirFactory; + } + + @Override + public String execute(ModifyCommandRequest request) { + try (WorkingCopy workingCopy = workdirFactory.createWorkingCopy(context)) { + org.eclipse.jgit.lib.Repository repository = workingCopy.getWorkingRepository(); + LOG.debug("cloned repository to folder {}", repository.getWorkTree()); + try { + return new ModifyWorker(repository, request).execute(); + } catch (IOException e) { + return throwInternalRepositoryException("could not apply modifications to cloned repository", e); + } + } + } + + private class ModifyWorker implements Worker { + + private final Git clone; + private final File workDir; + private final ModifyCommandRequest request; + + ModifyWorker(org.eclipse.jgit.lib.Repository repository, ModifyCommandRequest request) { + this.clone = new Git(repository); + this.workDir = repository.getWorkTree(); + this.request = request; + } + + String execute() throws IOException { + checkOutBranch(); + for (ModifyCommandRequest.PartialRequest r: request.getRequests()) { + r.execute(this); + } + return null; + } + + @Override + public void create(String toBeCreated, File file) throws IOException { + Path targetFile = new File(workDir, toBeCreated).toPath(); + Files.createDirectories(targetFile.getParent()); + Files.copy(file.toPath(), targetFile); + try { + clone.add().addFilepattern(toBeCreated).call(); + } catch (GitAPIException e) { + throwInternalRepositoryException("could not add new file to index", e); + } + } + + @Override + public void delete(String toBeDeleted) { + + } + + @Override + public void modify(String path, File file) { + + } + + @Override + public void move(String sourcePath, String targetPath) { + + } + + private void checkOutBranch() throws IOException { + String branch = request.getBranch(); + try { + clone.checkout().setName(branch).call(); + } catch (RefNotFoundException e) { + LOG.trace("could not checkout branch {} for modifications directly; trying to create local branch", branch, e); + checkOutTargetAsNewLocalBranch(); + } catch (GitAPIException e) { + throwInternalRepositoryException("could not checkout target branch for merge: " + branch, e); + } + } + + private void checkOutTargetAsNewLocalBranch() throws IOException { + String branch = request.getBranch(); + try { + ObjectId targetRevision = resolveRevision(branch); + clone.checkout().setStartPoint(targetRevision.getName()).setName(branch).setCreateBranch(true).call(); + } catch (RefNotFoundException e) { + LOG.debug("could not checkout branch {} for modifications as local branch", branch, e); + throw notFound(entity("Branch", branch).in(context.getRepository())); + } catch (GitAPIException e) { + throw new InternalRepositoryException(context.getRepository(), "could not checkout branch for modifications as local branch: " + branch, e); + } + } + + private ObjectId resolveRevision(String revision) throws IOException { + ObjectId resolved = clone.getRepository().resolve(revision); + if (resolved == null) { + return resolveRevisionOrThrowNotFound(clone.getRepository(), "origin/" + revision); + } else { + return resolved; + } + } + + private ObjectId resolveRevisionOrThrowNotFound(org.eclipse.jgit.lib.Repository repository, String revision) throws IOException { + ObjectId resolved = repository.resolve(revision); + if (resolved == null) { + throw notFound(entity("Revision", revision).in(context.getRepository())); + } else { + return resolved; + } + } + + private void doCommit() { + String branch = request.getBranch(); + LOG.debug("modified branch {}", branch); + Person authorToUse = determineAuthor(); + try { + if (!clone.status().call().isClean()) { + clone.commit() + .setAuthor(authorToUse.getName(), authorToUse.getMail()) + .setMessage(request.getCommitMessage()) + .call(); + } + } catch (GitAPIException e) { + throw new InternalRepositoryException(context.getRepository(), "could not commit modifications on branch " + request.getBranch(), e); + } + } + + private Person determineAuthor() { + if (request.getAuthor() == null) { + Subject subject = SecurityUtils.getSubject(); + User user = subject.getPrincipals().oneByType(User.class); + String name = user.getDisplayName(); + String email = user.getMail(); + LOG.debug("no author set; using logged in user: {} <{}>", name, email); + return new Person(name, email); + } else { + return request.getAuthor(); + } + } + } + + private String throwInternalRepositoryException(String message, Exception e) { + throw new InternalRepositoryException(context.getRepository(), message, e); + } +} diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitRepositoryServiceProvider.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitRepositoryServiceProvider.java index 6870c85dea..22a9fdf5f3 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitRepositoryServiceProvider.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitRepositoryServiceProvider.java @@ -268,6 +268,11 @@ public class GitRepositoryServiceProvider extends RepositoryServiceProvider return new GitMergeCommand(context, repository, handler.getWorkdirFactory()); } + @Override + public ModifyCommand getModifyCommand() { + return new GitModifyCommand(context, repository, handler.getWorkdirFactory()); + } + @Override public Set getSupportedFeatures() { return FEATURES; diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMergeCommandTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMergeCommandTest.java index 8b3b4548d9..674da396b5 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMergeCommandTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMergeCommandTest.java @@ -10,30 +10,17 @@ import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; -import org.eclipse.jgit.transport.ScmTransportProtocol; -import org.eclipse.jgit.transport.Transport; -import org.junit.After; -import org.junit.Before; import org.junit.Rule; import org.junit.Test; import sonia.scm.NotFoundException; -import sonia.scm.repository.GitRepositoryHandler; import sonia.scm.repository.Person; -import sonia.scm.repository.PreProcessorUtil; -import sonia.scm.repository.RepositoryManager; -import sonia.scm.repository.api.HookContextFactory; import sonia.scm.repository.api.MergeCommandResult; -import sonia.scm.repository.api.MergeDryRunCommandResult; import sonia.scm.repository.util.WorkdirProvider; import sonia.scm.user.User; import java.io.IOException; -import static com.google.inject.util.Providers.of; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; @SubjectAware(configuration = "classpath:sonia/scm/configuration/shiro.ini", username = "admin", password = "secret") public class GitMergeCommandTest extends AbstractGitCommandTestBase { diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java new file mode 100644 index 0000000000..3c55849219 --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java @@ -0,0 +1,31 @@ +package sonia.scm.repository.spi; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import sonia.scm.repository.util.WorkdirProvider; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; + +public class GitModifyCommandTest extends AbstractGitCommandTestBase { + + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); + @Rule + public BindTransportProtocolRule transportProtocolRule = new BindTransportProtocolRule(); + + @Test + public void shouldCreateNewFile() throws IOException { + File newFile = Files.write(temporaryFolder.newFile().toPath(), "new content".getBytes()).toFile(); + + GitModifyCommand command = new GitModifyCommand(createContext(), repository, new SimpleGitWorkdirFactory(new WorkdirProvider())); + + ModifyCommandRequest request = new ModifyCommandRequest(); + request.setBranch("master"); + request.setCommitMessage("test commit"); + request.addRequest(new ModifyCommandRequest.CreateFileRequest("new/file", newFile)); + command.execute(request); + } +} From b0267d69097f3964cfa466b45bc1568a091fb0c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 28 Aug 2019 15:10:38 +0200 Subject: [PATCH 21/35] Handle error on delete --- .../scm/repository/spi/ModifyCommandRequest.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) 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 index f3dcca18ff..e8f2223bdd 100644 --- a/scm-core/src/main/java/sonia/scm/repository/spi/ModifyCommandRequest.java +++ b/scm-core/src/main/java/sonia/scm/repository/spi/ModifyCommandRequest.java @@ -1,16 +1,22 @@ 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; import java.util.ArrayList; import java.util.Collections; import java.util.List; public class ModifyCommandRequest implements Resetable, Validateable { + private static final Logger LOG = LoggerFactory.getLogger(ModifyCommandRequest.class); + private final List requests = new ArrayList<>(); private Person author; @@ -107,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); + } } } From aae9ba6a59922bfa21859bc260d518d800fad4f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 28 Aug 2019 16:23:33 +0200 Subject: [PATCH 22/35] Commit and push changes --- .../scm/repository/spi/GitModifyCommand.java | 14 +++- .../repository/spi/GitModifyCommandTest.java | 76 ++++++++++++++++++- 2 files changed, 85 insertions(+), 5 deletions(-) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java index 8256b7e766..91c7700737 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java @@ -64,7 +64,9 @@ public class GitModifyCommand extends AbstractGitCommand implements ModifyComman for (ModifyCommandRequest.PartialRequest r: request.getRequests()) { r.execute(this); } - return null; + doCommit(); + push(); + return clone.getRepository().getRefDatabase().findRef("HEAD").getObjectId().name(); } @Override @@ -165,6 +167,16 @@ public class GitModifyCommand extends AbstractGitCommand implements ModifyComman return request.getAuthor(); } } + + private void push() { + try { + clone.push().call(); + } catch (GitAPIException e) { + throw new IntegrateChangesFromWorkdirException(repository, + "could not push modified branch " + request.getBranch() + " into central repository", e); + } + LOG.debug("pushed modified branch {}", request.getBranch()); + } } private String throwInternalRepositoryException(String message, Exception e) { diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java index 3c55849219..5001c87e9b 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java @@ -1,31 +1,99 @@ package sonia.scm.repository.spi; +import com.github.sdorra.shiro.ShiroRule; +import com.github.sdorra.shiro.SubjectAware; +import org.eclipse.jgit.api.Git; +import org.eclipse.jgit.api.errors.GitAPIException; +import org.eclipse.jgit.errors.CorruptObjectException; +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.ObjectReader; +import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; +import org.eclipse.jgit.treewalk.CanonicalTreeParser; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; +import sonia.scm.repository.Person; import sonia.scm.repository.util.WorkdirProvider; import java.io.File; import java.io.IOException; import java.nio.file.Files; +import static org.assertj.core.api.Assertions.assertThat; + +@SubjectAware(configuration = "classpath:sonia/scm/configuration/shiro.ini", username = "admin", password = "secret") public class GitModifyCommandTest extends AbstractGitCommandTestBase { @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); @Rule public BindTransportProtocolRule transportProtocolRule = new BindTransportProtocolRule(); + @Rule + public ShiroRule shiro = new ShiroRule(); @Test - public void shouldCreateNewFile() throws IOException { + public void shouldCreateCommit() throws IOException, GitAPIException { File newFile = Files.write(temporaryFolder.newFile().toPath(), "new content".getBytes()).toFile(); - GitModifyCommand command = new GitModifyCommand(createContext(), repository, new SimpleGitWorkdirFactory(new WorkdirProvider())); + GitModifyCommand command = createCommand(); ModifyCommandRequest request = new ModifyCommandRequest(); request.setBranch("master"); request.setCommitMessage("test commit"); - request.addRequest(new ModifyCommandRequest.CreateFileRequest("new/file", newFile)); - command.execute(request); + request.addRequest(new ModifyCommandRequest.CreateFileRequest("new_file", newFile)); + request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); + + String newRef = command.execute(request); + + try (Git git = new Git(createContext().open())) { + RevCommit lastCommit = getLastCommit(git); + assertThat(lastCommit.getFullMessage()).isEqualTo("test commit"); + assertThat(lastCommit.getAuthorIdent().getName()).isEqualTo("Dirk Gently"); + assertThat(newRef).isEqualTo(lastCommit.toObjectId().name()); + } + } + + @Test + public void shouldCreateNewFile() throws IOException, GitAPIException { + File newFile = Files.write(temporaryFolder.newFile().toPath(), "new content".getBytes()).toFile(); + + GitModifyCommand command = createCommand(); + + ModifyCommandRequest request = new ModifyCommandRequest(); + request.setBranch("master"); + request.setCommitMessage("test commit"); + request.addRequest(new ModifyCommandRequest.CreateFileRequest("new_file", newFile)); + request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); + + TreeAssertions assertions = canonicalTreeParser -> assertThat(canonicalTreeParser.findFile("new_file")).isTrue(); + + assertInTree(assertions); + } + + private void assertInTree(TreeAssertions assertions) throws IOException, GitAPIException { + 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(); + try (ObjectReader reader = git.getRepository().newObjectReader()) { + assertions.checkAssertions(new CanonicalTreeParser(null, reader, treeId)); + } + } + } + } + + private RevCommit getLastCommit(Git git) throws GitAPIException { + return git.log().setMaxCount(1).call().iterator().next(); + } + + private GitModifyCommand createCommand() { + return new GitModifyCommand(createContext(), repository, new SimpleGitWorkdirFactory(new WorkdirProvider())); + } + + @FunctionalInterface + private interface TreeAssertions { + void checkAssertions(CanonicalTreeParser treeParser) throws CorruptObjectException; } } From bb0682fab59dd1163fcf7a43f774b68833137ef5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 28 Aug 2019 17:04:22 +0200 Subject: [PATCH 23/35] Delete temporary workdir after command was executed --- .../repository/api/ModifyCommandBuilder.java | 17 +++++++-- .../api/ModifyCommandBuilderTest.java | 35 ++++++------------- 2 files changed, 25 insertions(+), 27 deletions(-) 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 501a2ac3b0..9cfcaebd6a 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 @@ -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); + } + } } /** 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 adbefe7d2a..7d4d96642a 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,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 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 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(); } From de7647ba550799eca9ba8546400317ab4b92cd63 Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Thu, 29 Aug 2019 10:31:25 +0200 Subject: [PATCH 24/35] Extract common code --- .../repository/spi/AbstractGitCommand.java | 128 +++++++++++++++++- .../scm/repository/spi/GitMergeCommand.java | 103 ++------------ .../scm/repository/spi/GitModifyCommand.java | 128 ++---------------- 3 files changed, 143 insertions(+), 216 deletions(-) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java index f94ccd59f6..d8c0ff1c60 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java @@ -35,15 +35,29 @@ package sonia.scm.repository.spi; //~--- non-JDK imports -------------------------------------------------------- import com.google.common.base.Strings; +import org.apache.shiro.SecurityUtils; +import org.apache.shiro.subject.Subject; +import org.eclipse.jgit.api.Git; +import org.eclipse.jgit.api.errors.GitAPIException; +import org.eclipse.jgit.api.errors.RefNotFoundException; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.repository.GitUtil; +import sonia.scm.repository.GitWorkdirFactory; +import sonia.scm.repository.InternalRepositoryException; +import sonia.scm.repository.Person; +import sonia.scm.repository.util.WorkingCopy; +import sonia.scm.user.User; import java.io.IOException; import java.util.Optional; +import java.util.function.Function; + +import static sonia.scm.ContextEntry.ContextBuilder.entity; +import static sonia.scm.NotFoundException.notFound; //~--- JDK imports ------------------------------------------------------------ @@ -51,7 +65,7 @@ import java.util.Optional; * * @author Sebastian Sdorra */ -public class AbstractGitCommand +class AbstractGitCommand { /** @@ -66,7 +80,7 @@ public class AbstractGitCommand * @param context * @param repository */ - protected AbstractGitCommand(GitContext context, + AbstractGitCommand(GitContext context, sonia.scm.repository.Repository repository) { this.repository = repository; @@ -83,12 +97,12 @@ public class AbstractGitCommand * * @throws IOException */ - protected Repository open() throws IOException + Repository open() throws IOException { return context.open(); } - protected ObjectId getCommitOrDefault(Repository gitRepository, String requestedCommit) throws IOException { + ObjectId getCommitOrDefault(Repository gitRepository, String requestedCommit) throws IOException { ObjectId commit; if ( Strings.isNullOrEmpty(requestedCommit) ) { commit = getDefaultBranch(gitRepository); @@ -98,7 +112,7 @@ public class AbstractGitCommand return commit; } - protected ObjectId getDefaultBranch(Repository gitRepository) throws IOException { + ObjectId getDefaultBranch(Repository gitRepository) throws IOException { Ref ref = getBranchOrDefault(gitRepository, null); if (ref == null) { return null; @@ -107,7 +121,7 @@ public class AbstractGitCommand } } - protected Ref getBranchOrDefault(Repository gitRepository, String requestedBranch) throws IOException { + Ref getBranchOrDefault(Repository gitRepository, String requestedBranch) throws IOException { if ( Strings.isNullOrEmpty(requestedBranch) ) { String defaultBranchName = context.getConfig().getDefaultBranch(); if (!Strings.isNullOrEmpty(defaultBranchName)) { @@ -122,6 +136,108 @@ public class AbstractGitCommand } } + > R inClone(Function workerSupplier, GitWorkdirFactory workdirFactory) { + try (WorkingCopy workingCopy = workdirFactory.createWorkingCopy(context)) { + Repository repository = workingCopy.getWorkingRepository(); + logger.debug("cloned repository to folder {}", repository.getWorkTree()); + return workerSupplier.apply(new Git(repository)).run(); + } catch (IOException e) { + throw new InternalRepositoryException(context.getRepository(), "could not clone repository", e); + } + } + + ObjectId resolveRevisionOrThrowNotFound(Repository repository, String revision) throws IOException { + ObjectId resolved = repository.resolve(revision); + if (resolved == null) { + throw notFound(entity("Revision", revision).in(context.getRepository())); + } else { + return resolved; + } + } + + abstract class GitCloneWorker { + private final Git clone; + + GitCloneWorker(Git clone) { + this.clone = clone; + } + + abstract R run() throws IOException; + + Git getClone() { + return clone; + } + + void checkOutBranch(String branchName) throws IOException { + try { + clone.checkout().setName(branchName).call(); + } catch (RefNotFoundException e) { + logger.trace("could not checkout branch {} directly; trying to create local branch", branchName, e); + checkOutTargetAsNewLocalBranch(branchName); + } catch (GitAPIException e) { + throw new InternalRepositoryException(context.getRepository(), "could not checkout branch: " + branchName, e); + } + } + + private void checkOutTargetAsNewLocalBranch(String branchName) throws IOException { + try { + ObjectId targetRevision = resolveRevision(branchName); + clone.checkout().setStartPoint(targetRevision.getName()).setName(branchName).setCreateBranch(true).call(); + } catch (RefNotFoundException e) { + logger.debug("could not checkout branch {} as local branch", branchName, e); + throw notFound(entity("Revision", branchName).in(context.getRepository())); + } catch (GitAPIException e) { + throw new InternalRepositoryException(context.getRepository(), "could not checkout branch as local branch: " + branchName, e); + } + } + + ObjectId resolveRevision(String revision) throws IOException { + ObjectId resolved = clone.getRepository().resolve(revision); + if (resolved == null) { + return resolveRevisionOrThrowNotFound(clone.getRepository(), "origin/" + revision); + } else { + return resolved; + } + } + + void doCommit(String message, Person author) { + Person authorToUse = determineAuthor(author); + try { + if (!clone.status().call().isClean()) { + clone.commit() + .setAuthor(authorToUse.getName(), authorToUse.getMail()) + .setMessage(message) + .call(); + } + } catch (GitAPIException e) { + throw new InternalRepositoryException(context.getRepository(), "could not commit changes", e); + } + } + + void push() { + try { + clone.push().call(); + } catch (GitAPIException e) { + throw new IntegrateChangesFromWorkdirException(repository, + "could not push changes into central repository", e); + } + logger.debug("pushed changes"); + } + + private Person determineAuthor(Person author) { + if (author == null) { + Subject subject = SecurityUtils.getSubject(); + User user = subject.getPrincipals().oneByType(User.class); + String name = user.getDisplayName(); + String email = user.getMail(); + logger.debug("no author set; using logged in user: {} <{}>", name, email); + return new Person(name, email); + } else { + return author; + } + } + } + //~--- fields --------------------------------------------------------------- /** Field description */ diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeCommand.java index 58c71b8dac..b4a7954b02 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeCommand.java @@ -1,13 +1,10 @@ package sonia.scm.repository.spi; import com.google.common.base.Strings; -import org.apache.shiro.SecurityUtils; -import org.apache.shiro.subject.Subject; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.MergeCommand.FastForwardMode; import org.eclipse.jgit.api.MergeResult; import org.eclipse.jgit.api.errors.GitAPIException; -import org.eclipse.jgit.api.errors.RefNotFoundException; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.merge.MergeStrategy; @@ -17,18 +14,12 @@ import org.slf4j.LoggerFactory; import sonia.scm.repository.GitWorkdirFactory; import sonia.scm.repository.InternalRepositoryException; import sonia.scm.repository.Person; -import sonia.scm.repository.RepositoryPermissions; import sonia.scm.repository.api.MergeCommandResult; import sonia.scm.repository.api.MergeDryRunCommandResult; -import sonia.scm.repository.util.WorkingCopy; -import sonia.scm.user.User; import java.io.IOException; import java.text.MessageFormat; -import static sonia.scm.ContextEntry.ContextBuilder.entity; -import static sonia.scm.NotFoundException.notFound; - public class GitMergeCommand extends AbstractGitCommand implements MergeCommand { private static final Logger logger = LoggerFactory.getLogger(GitMergeCommand.class); @@ -47,13 +38,7 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand @Override public MergeCommandResult merge(MergeCommandRequest request) { - try (WorkingCopy workingCopy = workdirFactory.createWorkingCopy(context)) { - Repository repository = workingCopy.getWorkingRepository(); - logger.debug("cloned repository to folder {}", repository.getWorkTree()); - return new MergeWorker(repository, request).merge(); - } catch (IOException e) { - throw new InternalRepositoryException(context.getRepository(), "could not clone repository for merge", e); - } + return inClone(clone -> new MergeWorker(clone, request), workdirFactory); } @Override @@ -70,32 +55,23 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand } } - private ObjectId resolveRevisionOrThrowNotFound(Repository repository, String revision) throws IOException { - ObjectId resolved = repository.resolve(revision); - if (resolved == null) { - throw notFound(entity("Revision", revision).in(context.getRepository())); - } else { - return resolved; - } - } - - private class MergeWorker { + private class MergeWorker extends GitCloneWorker { private final String target; private final String toMerge; private final Person author; - private final Git clone; private final String messageTemplate; - private MergeWorker(Repository clone, MergeCommandRequest request) { + private MergeWorker(Git clone, MergeCommandRequest request) { + super(clone); this.target = request.getTargetBranch(); this.toMerge = request.getBranchToMerge(); this.author = request.getAuthor(); this.messageTemplate = request.getMessageTemplate(); - this.clone = new Git(clone); } - private MergeCommandResult merge() throws IOException { + @Override + MergeCommandResult run() throws IOException { checkOutTargetBranch(); MergeResult result = doMergeInClone(); if (result.getMergeStatus().isSuccessful()) { @@ -108,33 +84,14 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand } private void checkOutTargetBranch() throws IOException { - try { - clone.checkout().setName(target).call(); - } catch (RefNotFoundException e) { - logger.trace("could not checkout target branch {} for merge directly; trying to create local branch", target, e); - checkOutTargetAsNewLocalBranch(); - } catch (GitAPIException e) { - throw new InternalRepositoryException(context.getRepository(), "could not checkout target branch for merge: " + target, e); - } - } - - private void checkOutTargetAsNewLocalBranch() throws IOException { - try { - ObjectId targetRevision = resolveRevision(target); - clone.checkout().setStartPoint(targetRevision.getName()).setName(target).setCreateBranch(true).call(); - } catch (RefNotFoundException e) { - logger.debug("could not checkout target branch {} for merge as local branch", target, e); - throw notFound(entity("Revision", target).in(context.getRepository())); - } catch (GitAPIException e) { - throw new InternalRepositoryException(context.getRepository(), "could not checkout target branch for merge as local branch: " + target, e); - } + checkOutBranch(target); } private MergeResult doMergeInClone() throws IOException { MergeResult result; try { ObjectId sourceRevision = resolveRevision(toMerge); - result = clone.merge() + result = getClone().merge() .setFastForward(FastForwardMode.NO_FF) .setCommit(false) // we want to set the author manually .include(toMerge, sourceRevision) @@ -147,17 +104,7 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand private void doCommit() { logger.debug("merged branch {} into {}", toMerge, target); - Person authorToUse = determineAuthor(); - try { - if (!clone.status().call().isClean()) { - clone.commit() - .setAuthor(authorToUse.getName(), authorToUse.getMail()) - .setMessage(MessageFormat.format(determineMessageTemplate(), toMerge, target)) - .call(); - } - } catch (GitAPIException e) { - throw new InternalRepositoryException(context.getRepository(), "could not commit merge between branch " + toMerge + " and " + target, e); - } + doCommit(MessageFormat.format(determineMessageTemplate(), toMerge, target), author); } private String determineMessageTemplate() { @@ -168,41 +115,9 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand } } - private Person determineAuthor() { - if (author == null) { - Subject subject = SecurityUtils.getSubject(); - User user = subject.getPrincipals().oneByType(User.class); - String name = user.getDisplayName(); - String email = user.getMail(); - logger.debug("no author set; using logged in user: {} <{}>", name, email); - return new Person(name, email); - } else { - return author; - } - } - - private void push() { - try { - clone.push().call(); - } catch (GitAPIException e) { - throw new IntegrateChangesFromWorkdirException(repository, - "could not push merged branch " + target + " into central repository", e); - } - logger.debug("pushed merged branch {}", target); - } - private MergeCommandResult analyseFailure(MergeResult result) { logger.info("could not merged branch {} into {} due to conflict in paths {}", toMerge, target, result.getConflicts().keySet()); return MergeCommandResult.failure(result.getConflicts().keySet()); } - - private ObjectId resolveRevision(String revision) throws IOException { - ObjectId resolved = clone.getRepository().resolve(revision); - if (resolved == null) { - return resolveRevisionOrThrowNotFound(clone.getRepository(), "origin/" + revision); - } else { - return resolved; - } - } } } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java index 91c7700737..8825645a3c 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java @@ -1,72 +1,50 @@ package sonia.scm.repository.spi; -import org.apache.shiro.SecurityUtils; -import org.apache.shiro.subject.Subject; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.errors.GitAPIException; -import org.eclipse.jgit.api.errors.RefNotFoundException; -import org.eclipse.jgit.lib.ObjectId; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import sonia.scm.repository.GitWorkdirFactory; import sonia.scm.repository.InternalRepositoryException; -import sonia.scm.repository.Person; import sonia.scm.repository.Repository; -import sonia.scm.repository.util.WorkingCopy; -import sonia.scm.user.User; import java.io.File; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import static sonia.scm.ContextEntry.ContextBuilder.entity; -import static sonia.scm.NotFoundException.notFound; - public class GitModifyCommand extends AbstractGitCommand implements ModifyCommand { - private static final Logger LOG = LoggerFactory.getLogger(GitModifyCommand.class); - private final GitWorkdirFactory workdirFactory; - public GitModifyCommand(GitContext context, Repository repository, GitWorkdirFactory workdirFactory) { + GitModifyCommand(GitContext context, Repository repository, GitWorkdirFactory workdirFactory) { super(context, repository); this.workdirFactory = workdirFactory; } @Override public String execute(ModifyCommandRequest request) { - try (WorkingCopy workingCopy = workdirFactory.createWorkingCopy(context)) { - org.eclipse.jgit.lib.Repository repository = workingCopy.getWorkingRepository(); - LOG.debug("cloned repository to folder {}", repository.getWorkTree()); - try { - return new ModifyWorker(repository, request).execute(); - } catch (IOException e) { - return throwInternalRepositoryException("could not apply modifications to cloned repository", e); - } - } + return inClone(clone -> new ModifyWorker(clone, request), workdirFactory); } - private class ModifyWorker implements Worker { + private class ModifyWorker extends GitCloneWorker implements Worker { - private final Git clone; private final File workDir; private final ModifyCommandRequest request; - ModifyWorker(org.eclipse.jgit.lib.Repository repository, ModifyCommandRequest request) { - this.clone = new Git(repository); - this.workDir = repository.getWorkTree(); + ModifyWorker(Git clone, ModifyCommandRequest request) { + super(clone); + this.workDir = clone.getRepository().getWorkTree(); this.request = request; } - String execute() throws IOException { - checkOutBranch(); + @Override + String run() throws IOException { + checkOutBranch(request.getBranch()); for (ModifyCommandRequest.PartialRequest r: request.getRequests()) { r.execute(this); } - doCommit(); + doCommit(request.getCommitMessage(), request.getAuthor()); push(); - return clone.getRepository().getRefDatabase().findRef("HEAD").getObjectId().name(); + return getClone().getRepository().getRefDatabase().findRef("HEAD").getObjectId().name(); } @Override @@ -75,7 +53,7 @@ public class GitModifyCommand extends AbstractGitCommand implements ModifyComman Files.createDirectories(targetFile.getParent()); Files.copy(file.toPath(), targetFile); try { - clone.add().addFilepattern(toBeCreated).call(); + getClone().add().addFilepattern(toBeCreated).call(); } catch (GitAPIException e) { throwInternalRepositoryException("could not add new file to index", e); } @@ -95,88 +73,6 @@ public class GitModifyCommand extends AbstractGitCommand implements ModifyComman public void move(String sourcePath, String targetPath) { } - - private void checkOutBranch() throws IOException { - String branch = request.getBranch(); - try { - clone.checkout().setName(branch).call(); - } catch (RefNotFoundException e) { - LOG.trace("could not checkout branch {} for modifications directly; trying to create local branch", branch, e); - checkOutTargetAsNewLocalBranch(); - } catch (GitAPIException e) { - throwInternalRepositoryException("could not checkout target branch for merge: " + branch, e); - } - } - - private void checkOutTargetAsNewLocalBranch() throws IOException { - String branch = request.getBranch(); - try { - ObjectId targetRevision = resolveRevision(branch); - clone.checkout().setStartPoint(targetRevision.getName()).setName(branch).setCreateBranch(true).call(); - } catch (RefNotFoundException e) { - LOG.debug("could not checkout branch {} for modifications as local branch", branch, e); - throw notFound(entity("Branch", branch).in(context.getRepository())); - } catch (GitAPIException e) { - throw new InternalRepositoryException(context.getRepository(), "could not checkout branch for modifications as local branch: " + branch, e); - } - } - - private ObjectId resolveRevision(String revision) throws IOException { - ObjectId resolved = clone.getRepository().resolve(revision); - if (resolved == null) { - return resolveRevisionOrThrowNotFound(clone.getRepository(), "origin/" + revision); - } else { - return resolved; - } - } - - private ObjectId resolveRevisionOrThrowNotFound(org.eclipse.jgit.lib.Repository repository, String revision) throws IOException { - ObjectId resolved = repository.resolve(revision); - if (resolved == null) { - throw notFound(entity("Revision", revision).in(context.getRepository())); - } else { - return resolved; - } - } - - private void doCommit() { - String branch = request.getBranch(); - LOG.debug("modified branch {}", branch); - Person authorToUse = determineAuthor(); - try { - if (!clone.status().call().isClean()) { - clone.commit() - .setAuthor(authorToUse.getName(), authorToUse.getMail()) - .setMessage(request.getCommitMessage()) - .call(); - } - } catch (GitAPIException e) { - throw new InternalRepositoryException(context.getRepository(), "could not commit modifications on branch " + request.getBranch(), e); - } - } - - private Person determineAuthor() { - if (request.getAuthor() == null) { - Subject subject = SecurityUtils.getSubject(); - User user = subject.getPrincipals().oneByType(User.class); - String name = user.getDisplayName(); - String email = user.getMail(); - LOG.debug("no author set; using logged in user: {} <{}>", name, email); - return new Person(name, email); - } else { - return request.getAuthor(); - } - } - - private void push() { - try { - clone.push().call(); - } catch (GitAPIException e) { - throw new IntegrateChangesFromWorkdirException(repository, - "could not push modified branch " + request.getBranch() + " into central repository", e); - } - LOG.debug("pushed modified branch {}", request.getBranch()); - } } private String throwInternalRepositoryException(String message, Exception e) { From 924912b437ebd0b4a067897c1fdbb74e3f8a0dfe Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Thu, 29 Aug 2019 13:19:14 +0200 Subject: [PATCH 25/35] Fix preconditions --- .../java/sonia/scm/repository/api/ModifyCommandBuilder.java | 2 +- .../java/sonia/scm/repository/spi/ModifyCommandRequest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 9cfcaebd6a..0dbd45d0a0 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 @@ -115,7 +115,7 @@ public class ModifyCommandBuilder { */ public String execute() { try { - Preconditions.checkArgument(request.isValid(), "commit message, author and branch are required"); + Preconditions.checkArgument(request.isValid(), "commit message, branch and at least one request are required"); return command.execute(request); } finally { try { 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 index 958cd4a485..94bf84ae70 100644 --- a/scm-core/src/main/java/sonia/scm/repository/spi/ModifyCommandRequest.java +++ b/scm-core/src/main/java/sonia/scm/repository/spi/ModifyCommandRequest.java @@ -65,7 +65,7 @@ public class ModifyCommandRequest implements Resetable, Validateable { @Override public boolean isValid() { - return StringUtils.isNotEmpty(commitMessage) && StringUtils.isNotEmpty(branch) && author != null && !requests.isEmpty(); + return StringUtils.isNotEmpty(commitMessage) && StringUtils.isNotEmpty(branch) && !requests.isEmpty(); } public interface PartialRequest { From 6bd9e0e7dd44d69d186d3bd36e289526b7657245 Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Thu, 29 Aug 2019 13:47:28 +0200 Subject: [PATCH 26/35] Add messages for IllegalIdentifierChangeException --- scm-webapp/src/main/resources/locales/de/plugins.json | 4 ++++ scm-webapp/src/main/resources/locales/en/plugins.json | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/scm-webapp/src/main/resources/locales/de/plugins.json b/scm-webapp/src/main/resources/locales/de/plugins.json index 92b81c5772..bf534dc138 100644 --- a/scm-webapp/src/main/resources/locales/de/plugins.json +++ b/scm-webapp/src/main/resources/locales/de/plugins.json @@ -167,6 +167,10 @@ "CHRM7IQzo1": { "displayName": "Änderung fehlgeschlagen", "description": "Die Änderung ist fehlgeschlagen. Bitte wenden Sie sich an ihren Administrator für weitere Hinweise." + }, + "thbsUFokjk": { + "displayName": "Unerlaubte Änderung eines Schlüsselwerts", + "description": "Ein Schlüsselwert wurde unerlaubterweise geändert." } }, "namespaceStrategies": { diff --git a/scm-webapp/src/main/resources/locales/en/plugins.json b/scm-webapp/src/main/resources/locales/en/plugins.json index 3f70000624..471da10159 100644 --- a/scm-webapp/src/main/resources/locales/en/plugins.json +++ b/scm-webapp/src/main/resources/locales/en/plugins.json @@ -167,6 +167,10 @@ "CHRM7IQzo1": { "displayName": "Change failed", "description": "The change failed. Please contact your administrator for further assistance." + }, + "thbsUFokjk": { + "displayName": "Illegal change of an identifier", + "description": "A identifier value has been changed in the entity. This is not allowed." } }, "namespaceStrategies": { From 2674dbaf52580076ee9ec7e418d7961778d675bb Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 29 Aug 2019 15:32:36 +0200 Subject: [PATCH 27/35] suppress UnstableApiUsage and use try with resources --- .../sonia/scm/repository/api/ModifyCommandBuilder.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) 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 9cfcaebd6a..28adca18ae 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 @@ -184,17 +184,19 @@ public class ModifyCommandBuilder { } } + @SuppressWarnings("UnstableApiUsage") // Files only used internal private File loadData(ByteSource data) throws IOException { File file = createTemporaryFile(); data.copyTo(Files.asByteSink(file)); return file; } + @SuppressWarnings("UnstableApiUsage") // Files and ByteStreams only used internal private File loadData(InputStream data) throws IOException { File file = createTemporaryFile(); - OutputStream out = Files.asByteSink(file).openBufferedStream(); - ByteStreams.copy(data, out); - out.close(); + try (OutputStream out = Files.asByteSink(file).openBufferedStream()) { + ByteStreams.copy(data, out); + } return file; } From f85460ae62cd200cc6dcf3cbbb7768cd1a744c19 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 29 Aug 2019 13:34:48 +0000 Subject: [PATCH 28/35] Close branch feature/modification_api From 41b27d6e09756097c85329ee11a76890574305ef Mon Sep 17 00:00:00 2001 From: Eduard Heimbuch Date: Thu, 29 Aug 2019 16:27:40 +0200 Subject: [PATCH 29/35] rename extensionPoints --- scm-ui/src/repos/sources/components/FileTree.js | 2 +- scm-ui/src/repos/sources/components/FileTreeLeaf.js | 4 ++-- scm-ui/src/repos/sources/containers/Content.js | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/scm-ui/src/repos/sources/components/FileTree.js b/scm-ui/src/repos/sources/components/FileTree.js index 2f76b8bce7..d62a2b44db 100644 --- a/scm-ui/src/repos/sources/components/FileTree.js +++ b/scm-ui/src/repos/sources/components/FileTree.js @@ -125,7 +125,7 @@ class FileTree extends React.Component { {t("sources.file-tree.description")} - {binder.hasExtension("sourceView.right") && ( + {binder.hasExtension("repos.sources.tree.row.right") && ( )} diff --git a/scm-ui/src/repos/sources/components/FileTreeLeaf.js b/scm-ui/src/repos/sources/components/FileTreeLeaf.js index 4e9d5008a1..d3c81b960c 100644 --- a/scm-ui/src/repos/sources/components/FileTreeLeaf.js +++ b/scm-ui/src/repos/sources/components/FileTreeLeaf.js @@ -92,11 +92,11 @@ class FileTreeLeaf extends React.Component { > {file.description} - {binder.hasExtension("sourceView.right") && ( + {binder.hasExtension("repos.sources.tree.row.right") && ( {!file.directory && ( diff --git a/scm-ui/src/repos/sources/containers/Content.js b/scm-ui/src/repos/sources/containers/Content.js index d2290913f0..64a0fb472f 100644 --- a/scm-ui/src/repos/sources/containers/Content.js +++ b/scm-ui/src/repos/sources/containers/Content.js @@ -100,7 +100,7 @@ class Content extends React.Component {
{selector}
From 0e02390019795409eae3d8015ccbfbcbd20dd537 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 29 Aug 2019 14:44:01 +0000 Subject: [PATCH 30/35] Close branch feature/editor_plugin_download From f02dec5cc6e0160fe465e7de4ec70f592ed15c15 Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Thu, 29 Aug 2019 17:22:50 +0200 Subject: [PATCH 31/35] Add flag to optionally overwrite files with create --- .../repository/api/ModifyCommandBuilder.java | 83 ++++++++++++++----- .../scm/repository/spi/ModifyCommand.java | 2 +- .../repository/spi/ModifyCommandRequest.java | 6 +- .../api/ModifyCommandBuilderTest.java | 40 ++++++++- .../scm/repository/spi/GitModifyCommand.java | 17 +++- .../repository/spi/GitModifyCommandTest.java | 39 ++++++++- 6 files changed, 156 insertions(+), 31 deletions(-) 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 bddf9b378a..f5561c839a 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 @@ -16,12 +16,14 @@ import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.util.function.BiConsumer; import java.util.function.Consumer; /** * Use this {@link ModifyCommandBuilder} to make file changes to the head of a branch. You can *
    - *
  • create new files ({@link #createFile(String)}
    • + *
  • create new files ({@link #createFile(String)} (with the option to overwrite a file, if it already exists; by + * default a {@link sonia.scm.AlreadyExistsException} will be thrown)
    • *
  • modify existing files ({@link #modifyFile(String)}
    • *
  • delete existing files ({@link #deleteFile(String)}
    • *
  • move/rename existing files ({@link #moveFile(String, String)}
    • @@ -66,24 +68,24 @@ public class ModifyCommandBuilder { /** * Create a new file. The content of the file will be specified in a subsequent call to - * {@link ContentLoader#withData(ByteSource)} or {@link ContentLoader#withData(InputStream)}. + * {@link SimpleContentLoader#withData(ByteSource)} or {@link SimpleContentLoader#withData(InputStream)}. * @param path The path and the name of the file that should be created. * @return The loader to specify the content of the new file. */ - public ContentLoader createFile(String path) { - return new ContentLoader( - content -> request.addRequest(new ModifyCommandRequest.CreateFileRequest(path, content)) + public WithOverwriteFlagContentLoader createFile(String path) { + return new WithOverwriteFlagContentLoader( + (content, overwrite) -> request.addRequest(new ModifyCommandRequest.CreateFileRequest(path, content, overwrite)) ); } /** * Modify an existing file. The new content of the file will be specified in a subsequent call to - * {@link ContentLoader#withData(ByteSource)} or {@link ContentLoader#withData(InputStream)}. + * {@link SimpleContentLoader#withData(ByteSource)} or {@link SimpleContentLoader#withData(InputStream)}. * @param path The path and the name of the file that should be modified. * @return The loader to specify the new content of the file. */ - public ContentLoader modifyFile(String path) { - return new ContentLoader( + public SimpleContentLoader modifyFile(String path) { + return new SimpleContentLoader( content -> request.addRequest(new ModifyCommandRequest.ModifyFileRequest(path, content)) ); } @@ -153,30 +155,39 @@ public class ModifyCommandBuilder { return this; } - public class ContentLoader { - - private final Consumer contentConsumer; - - private ContentLoader(Consumer contentConsumer) { - this.contentConsumer = contentConsumer; - } - + public interface ContentLoader { /** * Specify the data of the file using a {@link ByteSource}. + * * @return The builder instance. * @throws IOException If the data could not be read. */ - public ModifyCommandBuilder withData(ByteSource data) throws IOException { - File content = loadData(data); - contentConsumer.accept(content); - return ModifyCommandBuilder.this; - } + ModifyCommandBuilder withData(ByteSource data) throws IOException; /** * Specify the data of the file using an {@link InputStream}. * @return The builder instance. * @throws IOException If the data could not be read. */ + ModifyCommandBuilder withData(InputStream data) throws IOException; + } + + public class SimpleContentLoader implements ContentLoader { + + private final Consumer contentConsumer; + + private SimpleContentLoader(Consumer contentConsumer) { + this.contentConsumer = contentConsumer; + } + + @Override + public ModifyCommandBuilder withData(ByteSource data) throws IOException { + File content = loadData(data); + contentConsumer.accept(content); + return ModifyCommandBuilder.this; + } + + @Override public ModifyCommandBuilder withData(InputStream data) throws IOException { File content = loadData(data); contentConsumer.accept(content); @@ -184,6 +195,36 @@ public class ModifyCommandBuilder { } } + public class WithOverwriteFlagContentLoader implements ContentLoader { + + private final ContentLoader contentLoader; + private boolean overwrite = false; + + private WithOverwriteFlagContentLoader(BiConsumer contentConsumer) { + this.contentLoader = new SimpleContentLoader(file -> contentConsumer.accept(file, overwrite)); + } + + /** + * Set this to true to overwrite the file if it already exists. Otherwise an + * {@link sonia.scm.AlreadyExistsException} will be thrown. + * @return This loader instance. + */ + public WithOverwriteFlagContentLoader setOverwrite(boolean overwrite) { + this.overwrite = overwrite; + return this; + } + + @Override + public ModifyCommandBuilder withData(ByteSource data) throws IOException { + return contentLoader.withData(data); + } + + @Override + public ModifyCommandBuilder withData(InputStream data) throws IOException { + return contentLoader.withData(data); + } + } + @SuppressWarnings("UnstableApiUsage") // Files only used internal private File loadData(ByteSource data) throws IOException { File file = createTemporaryFile(); 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 a8b3efd6d9..578944c47f 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 @@ -10,7 +10,7 @@ public interface ModifyCommand { interface Worker { void delete(String toBeDeleted); - void create(String toBeCreated, File file) throws IOException; + void create(String toBeCreated, File file, boolean overwrite) throws IOException; void modify(String path, File file); 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 index 94bf84ae70..723b287b6e 100644 --- a/scm-core/src/main/java/sonia/scm/repository/spi/ModifyCommandRequest.java +++ b/scm-core/src/main/java/sonia/scm/repository/spi/ModifyCommandRequest.java @@ -124,15 +124,17 @@ public class ModifyCommandRequest implements Resetable, Validateable { public static class CreateFileRequest extends ContentModificationRequest { private final String path; + private final boolean overwrite; - public CreateFileRequest(String path, File content) { + public CreateFileRequest(String path, File content, boolean overwrite) { super(content); this.path = path; + this.overwrite = overwrite; } @Override public void execute(ModifyCommand.Worker worker) throws IOException { - worker.create(path, getContent()); + worker.create(path, getContent(), overwrite); 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 d88d720cdb..db9a247d65 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 @@ -1,6 +1,7 @@ package sonia.scm.repository.api; import com.google.common.io.ByteSource; +import com.sun.org.apache.xpath.internal.operations.Bool; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -25,6 +26,7 @@ import java.util.List; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.lenient; @@ -96,7 +98,7 @@ class ModifyCommandBuilderTest { void shouldExecuteCreateWithByteSourceContent() throws IOException { ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); List contentCaptor = new ArrayList<>(); - doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any()); + doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any(), anyBoolean()); initCommand() .createFile("toBeCreated").withData(ByteSource.wrap("content".getBytes())) @@ -110,7 +112,7 @@ class ModifyCommandBuilderTest { void shouldExecuteCreateWithInputStreamContent() throws IOException { ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); List contentCaptor = new ArrayList<>(); - doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any()); + doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any(), anyBoolean()); initCommand() .createFile("toBeCreated").withData(new ByteArrayInputStream("content".getBytes())) @@ -120,11 +122,43 @@ class ModifyCommandBuilderTest { assertThat(contentCaptor).contains("content"); } + @Test + void shouldExecuteCreateWithOverwriteFalseAsDefault() throws IOException { + ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); + ArgumentCaptor overwriteCaptor = ArgumentCaptor.forClass(Boolean.class); + List contentCaptor = new ArrayList<>(); + doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any(), overwriteCaptor.capture()); + + initCommand() + .createFile("toBeCreated").withData(new ByteArrayInputStream("content".getBytes())) + .execute(); + + assertThat(nameCaptor.getValue()).isEqualTo("toBeCreated"); + assertThat(overwriteCaptor.getValue()).isFalse(); + assertThat(contentCaptor).contains("content"); + } + + @Test + void shouldExecuteCreateWithOverwriteIfSet() throws IOException { + ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); + ArgumentCaptor overwriteCaptor = ArgumentCaptor.forClass(Boolean.class); + List contentCaptor = new ArrayList<>(); + doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any(), overwriteCaptor.capture()); + + initCommand() + .createFile("toBeCreated").setOverwrite(true).withData(new ByteArrayInputStream("content".getBytes())) + .execute(); + + assertThat(nameCaptor.getValue()).isEqualTo("toBeCreated"); + assertThat(overwriteCaptor.getValue()).isTrue(); + assertThat(contentCaptor).contains("content"); + } + @Test void shouldExecuteCreateMultipleTimes() throws IOException { ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); List contentCaptor = new ArrayList<>(); - doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any()); + doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any(), anyBoolean()); initCommand() .createFile("toBeCreated_1").withData(new ByteArrayInputStream("content_1".getBytes())) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java index 8825645a3c..2c473398c5 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java @@ -8,9 +8,14 @@ import sonia.scm.repository.Repository; import java.io.File; import java.io.IOException; +import java.nio.file.FileAlreadyExistsException; import java.nio.file.Files; import java.nio.file.Path; +import static java.nio.file.StandardCopyOption.REPLACE_EXISTING; +import static sonia.scm.AlreadyExistsException.alreadyExists; +import static sonia.scm.ContextEntry.ContextBuilder.entity; + public class GitModifyCommand extends AbstractGitCommand implements ModifyCommand { private final GitWorkdirFactory workdirFactory; @@ -48,10 +53,18 @@ public class GitModifyCommand extends AbstractGitCommand implements ModifyComman } @Override - public void create(String toBeCreated, File file) throws IOException { + public void create(String toBeCreated, File file, boolean overwrite) throws IOException { Path targetFile = new File(workDir, toBeCreated).toPath(); Files.createDirectories(targetFile.getParent()); - Files.copy(file.toPath(), targetFile); + if (overwrite) { + Files.copy(file.toPath(), targetFile, REPLACE_EXISTING); + } else { + try { + Files.copy(file.toPath(), targetFile); + } catch (FileAlreadyExistsException e) { + throw alreadyExists(entity("file", toBeCreated).in("branch", request.getBranch()).in(context.getRepository())); + } + } try { getClone().add().addFilepattern(toBeCreated).call(); } catch (GitAPIException e) { diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java index e8574b7baf..2d9f5d0235 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java @@ -13,6 +13,7 @@ import org.eclipse.jgit.treewalk.CanonicalTreeParser; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; +import sonia.scm.AlreadyExistsException; import sonia.scm.repository.Person; import sonia.scm.repository.util.WorkdirProvider; @@ -41,7 +42,7 @@ public class GitModifyCommandTest extends AbstractGitCommandTestBase { ModifyCommandRequest request = new ModifyCommandRequest(); request.setBranch("master"); request.setCommitMessage("test commit"); - request.addRequest(new ModifyCommandRequest.CreateFileRequest("new_file", newFile)); + request.addRequest(new ModifyCommandRequest.CreateFileRequest("new_file", newFile, false)); request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); String newRef = command.execute(request); @@ -63,7 +64,7 @@ public class GitModifyCommandTest extends AbstractGitCommandTestBase { ModifyCommandRequest request = new ModifyCommandRequest(); request.setBranch("master"); request.setCommitMessage("test commit"); - request.addRequest(new ModifyCommandRequest.CreateFileRequest("new_file", newFile)); + request.addRequest(new ModifyCommandRequest.CreateFileRequest("new_file", newFile, false)); request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); command.execute(request); @@ -73,6 +74,40 @@ public class GitModifyCommandTest extends AbstractGitCommandTestBase { assertInTree(assertions); } + @Test(expected = AlreadyExistsException.class) + public void shouldFailIfOverwritingExistingFileWithoutOverwriteFlag() throws IOException, GitAPIException { + File newFile = Files.write(temporaryFolder.newFile().toPath(), "new content".getBytes()).toFile(); + + GitModifyCommand command = createCommand(); + + ModifyCommandRequest request = new ModifyCommandRequest(); + request.setBranch("master"); + request.setCommitMessage("test commit"); + request.addRequest(new ModifyCommandRequest.CreateFileRequest("a.txt", newFile, false)); + request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); + + command.execute(request); + } + + @Test + public void shouldOverwriteExistingFileIfOverwriteFlagSet() throws IOException, GitAPIException { + File newFile = Files.write(temporaryFolder.newFile().toPath(), "new content".getBytes()).toFile(); + + GitModifyCommand command = createCommand(); + + ModifyCommandRequest request = new ModifyCommandRequest(); + request.setBranch("master"); + request.setCommitMessage("test commit"); + request.addRequest(new ModifyCommandRequest.CreateFileRequest("a.txt", newFile, true)); + request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); + + command.execute(request); + + TreeAssertions assertions = canonicalTreeParser -> assertThat(canonicalTreeParser.findFile("a.txt")).isTrue(); + + assertInTree(assertions); + } + private void assertInTree(TreeAssertions assertions) throws IOException, GitAPIException { try (Git git = new Git(createContext().open())) { RevCommit lastCommit = getLastCommit(git); From bdc2622df81a25dbb5b0e9c4a796d88474573480 Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Thu, 29 Aug 2019 17:30:33 +0200 Subject: [PATCH 32/35] Fix javadoc --- .../sonia/scm/repository/api/ModifyCommandBuilder.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) 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 f5561c839a..5baa7dcd26 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 @@ -68,7 +68,10 @@ public class ModifyCommandBuilder { /** * Create a new file. The content of the file will be specified in a subsequent call to - * {@link SimpleContentLoader#withData(ByteSource)} or {@link SimpleContentLoader#withData(InputStream)}. + * {@link ContentLoader#withData(ByteSource)} or {@link ContentLoader#withData(InputStream)}. + * By default, an {@link sonia.scm.AlreadyExistsException} will be thrown, when there already + * exists a file with the given path. You can disable this setting + * {@link WithOverwriteFlagContentLoader#setOverwrite(boolean)} to true. * @param path The path and the name of the file that should be created. * @return The loader to specify the content of the new file. */ @@ -80,7 +83,7 @@ public class ModifyCommandBuilder { /** * Modify an existing file. The new content of the file will be specified in a subsequent call to - * {@link SimpleContentLoader#withData(ByteSource)} or {@link SimpleContentLoader#withData(InputStream)}. + * {@link ContentLoader#withData(ByteSource)} or {@link ContentLoader#withData(InputStream)}. * @param path The path and the name of the file that should be modified. * @return The loader to specify the new content of the file. */ From 8a7b50a079fa019e9e9b556439451bfff0b711f2 Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Thu, 29 Aug 2019 17:57:29 +0200 Subject: [PATCH 33/35] Throw exception when no changes were made --- .../repository/spi/AbstractGitCommand.java | 22 ++++++++++++++++--- .../scm/repository/spi/GitModifyCommand.java | 16 +++++++++++++- .../repository/spi/GitModifyCommandTest.java | 16 ++++++++++++++ .../main/resources/locales/de/plugins.json | 4 ++++ .../main/resources/locales/en/plugins.json | 4 ++++ 5 files changed, 58 insertions(+), 4 deletions(-) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java index d8c0ff1c60..6066b66371 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java @@ -43,6 +43,7 @@ import org.eclipse.jgit.api.errors.RefNotFoundException; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.revwalk.RevCommit; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.repository.GitUtil; @@ -55,7 +56,10 @@ import sonia.scm.user.User; import java.io.IOException; import java.util.Optional; import java.util.function.Function; +import java.util.function.Supplier; +import static java.util.Optional.empty; +import static java.util.Optional.of; import static sonia.scm.ContextEntry.ContextBuilder.entity; import static sonia.scm.NotFoundException.notFound; @@ -200,14 +204,26 @@ class AbstractGitCommand } } - void doCommit(String message, Person author) { + void failIfNotChanged(Supplier doThrow) { + try { + if (clone.status().call().isClean()) { + throw doThrow.get(); + } + } catch (GitAPIException e) { + throw new InternalRepositoryException(context.getRepository(), "could not read status of repository", e); + } + } + + Optional doCommit(String message, Person author) { Person authorToUse = determineAuthor(author); try { if (!clone.status().call().isClean()) { - clone.commit() + return of(clone.commit() .setAuthor(authorToUse.getName(), authorToUse.getMail()) .setMessage(message) - .call(); + .call()); + } else { + return empty(); } } catch (GitAPIException e) { throw new InternalRepositoryException(context.getRepository(), "could not commit changes", e); diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java index 2c473398c5..c86c94aea1 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java @@ -2,6 +2,8 @@ package sonia.scm.repository.spi; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.errors.GitAPIException; +import sonia.scm.BadRequestException; +import sonia.scm.ContextEntry; import sonia.scm.repository.GitWorkdirFactory; import sonia.scm.repository.InternalRepositoryException; import sonia.scm.repository.Repository; @@ -44,9 +46,10 @@ public class GitModifyCommand extends AbstractGitCommand implements ModifyComman @Override String run() throws IOException { checkOutBranch(request.getBranch()); - for (ModifyCommandRequest.PartialRequest r: request.getRequests()) { + for (ModifyCommandRequest.PartialRequest r : request.getRequests()) { r.execute(this); } + failIfNotChanged(NoChangesMadeException::new); doCommit(request.getCommitMessage(), request.getAuthor()); push(); return getClone().getRepository().getRefDatabase().findRef("HEAD").getObjectId().name(); @@ -86,6 +89,17 @@ public class GitModifyCommand extends AbstractGitCommand implements ModifyComman public void move(String sourcePath, String targetPath) { } + + private class NoChangesMadeException extends BadRequestException { + public NoChangesMadeException() { + super(ContextEntry.ContextBuilder.entity(context.getRepository()).build(), "no changes detected to branch " + ModifyWorker.this.request.getBranch()); + } + + @Override + public String getCode() { + return "40RaYIeeR1"; + } + } } private String throwInternalRepositoryException(String message, Exception e) { diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java index 2d9f5d0235..c1e1c05631 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java @@ -14,6 +14,7 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; import sonia.scm.AlreadyExistsException; +import sonia.scm.BadRequestException; import sonia.scm.repository.Person; import sonia.scm.repository.util.WorkdirProvider; @@ -108,6 +109,21 @@ public class GitModifyCommandTest extends AbstractGitCommandTestBase { assertInTree(assertions); } + @Test(expected = BadRequestException.class) + public void shouldFailIfNoChangesMade() throws IOException, GitAPIException { + File newFile = Files.write(temporaryFolder.newFile().toPath(), "b\n".getBytes()).toFile(); + + GitModifyCommand command = createCommand(); + + ModifyCommandRequest request = new ModifyCommandRequest(); + request.setBranch("master"); + request.setCommitMessage("test commit"); + request.addRequest(new ModifyCommandRequest.CreateFileRequest("b.txt", newFile, true)); + request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); + + command.execute(request); + } + private void assertInTree(TreeAssertions assertions) throws IOException, GitAPIException { try (Git git = new Git(createContext().open())) { RevCommit lastCommit = getLastCommit(git); diff --git a/scm-webapp/src/main/resources/locales/de/plugins.json b/scm-webapp/src/main/resources/locales/de/plugins.json index bf534dc138..9c5bb3f5f5 100644 --- a/scm-webapp/src/main/resources/locales/de/plugins.json +++ b/scm-webapp/src/main/resources/locales/de/plugins.json @@ -171,6 +171,10 @@ "thbsUFokjk": { "displayName": "Unerlaubte Änderung eines Schlüsselwerts", "description": "Ein Schlüsselwert wurde unerlaubterweise geändert." + }, + "40RaYIeeR1": { + "displayName": "Es wurden keine Änderungen durchgeführt", + "description": "Das Repository wurde nicht verändert. Daher konnte kein neuer Commit erzeugt werden." } }, "namespaceStrategies": { diff --git a/scm-webapp/src/main/resources/locales/en/plugins.json b/scm-webapp/src/main/resources/locales/en/plugins.json index 471da10159..1ffffd73b7 100644 --- a/scm-webapp/src/main/resources/locales/en/plugins.json +++ b/scm-webapp/src/main/resources/locales/en/plugins.json @@ -171,6 +171,10 @@ "thbsUFokjk": { "displayName": "Illegal change of an identifier", "description": "A identifier value has been changed in the entity. This is not allowed." + }, + "40RaYIeeR1": { + "displayName": "No changes were made", + "description": "No changes were made to the files of the repository. Therefor no new commit could be created." } }, "namespaceStrategies": { From 8ccd99975f7fa627ed6745e708ad2b6c50e54ec4 Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Thu, 29 Aug 2019 17:59:34 +0200 Subject: [PATCH 34/35] Use revision from commit as result --- .../java/sonia/scm/repository/spi/GitModifyCommand.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java index c86c94aea1..680ef153bd 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java @@ -2,6 +2,7 @@ package sonia.scm.repository.spi; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.errors.GitAPIException; +import org.eclipse.jgit.revwalk.RevCommit; import sonia.scm.BadRequestException; import sonia.scm.ContextEntry; import sonia.scm.repository.GitWorkdirFactory; @@ -13,6 +14,7 @@ import java.io.IOException; import java.nio.file.FileAlreadyExistsException; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Optional; import static java.nio.file.StandardCopyOption.REPLACE_EXISTING; import static sonia.scm.AlreadyExistsException.alreadyExists; @@ -50,9 +52,9 @@ public class GitModifyCommand extends AbstractGitCommand implements ModifyComman r.execute(this); } failIfNotChanged(NoChangesMadeException::new); - doCommit(request.getCommitMessage(), request.getAuthor()); + Optional revCommit = doCommit(request.getCommitMessage(), request.getAuthor()); push(); - return getClone().getRepository().getRefDatabase().findRef("HEAD").getObjectId().name(); + return revCommit.orElseThrow(NoChangesMadeException::new).name(); } @Override From 86db0b4565990b7df9dafa4e6f4f0bbda7183828 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Fri, 30 Aug 2019 05:51:20 +0000 Subject: [PATCH 35/35] Close branch feature/git_modification_api