From 1854cfcfba350ef1bf11813f85cca89c96ea4b5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 6 Nov 2018 14:10:51 +0100 Subject: [PATCH 01/27] Add command api for merge --- .../sonia/scm/repository/api/Command.java | 6 +- .../repository/api/MergeCommandBuilder.java | 35 ++++++++++ .../scm/repository/spi/MergeCommand.java | 5 ++ .../repository/spi/MergeCommandRequest.java | 70 +++++++++++++++++++ .../spi/RepositoryServiceProvider.java | 8 +++ 5 files changed, 123 insertions(+), 1 deletion(-) create mode 100644 scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java create mode 100644 scm-core/src/main/java/sonia/scm/repository/spi/MergeCommand.java create mode 100644 scm-core/src/main/java/sonia/scm/repository/spi/MergeCommandRequest.java 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 2f844cfbfb..fa09cea2cb 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,6 +66,10 @@ public enum Command /** * @since 2.0 */ - MODIFICATIONS + MODIFICATIONS, + /** + * @since 2.0 + */ + MERGE } diff --git a/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java new file mode 100644 index 0000000000..6d9b332f0a --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java @@ -0,0 +1,35 @@ +package sonia.scm.repository.api; + +import com.google.common.base.Preconditions; +import sonia.scm.repository.spi.MergeCommand; +import sonia.scm.repository.spi.MergeCommandRequest; + +public class MergeCommandBuilder { + + private final MergeCommand mergeCommand; + private final MergeCommandRequest request = new MergeCommandRequest(); + + public MergeCommandBuilder(MergeCommand mergeCommand) { + this.mergeCommand = mergeCommand; + } + + public MergeCommandBuilder setBranchToMerge(String branchToMerge) { + request.setBranchToMerge(branchToMerge); + return this; + } + + public MergeCommandBuilder setTargetBranch(String targetBranch) { + request.setTargetBranch(targetBranch); + return this; + } + + public MergeCommandBuilder reset() { + request.reset(); + return this; + } + + public boolean execute() { + Preconditions.checkArgument(request.isValid(), "revision to merge and target revision is required"); + return mergeCommand.merge(request); + } +} diff --git a/scm-core/src/main/java/sonia/scm/repository/spi/MergeCommand.java b/scm-core/src/main/java/sonia/scm/repository/spi/MergeCommand.java new file mode 100644 index 0000000000..326bba4dce --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/repository/spi/MergeCommand.java @@ -0,0 +1,5 @@ +package sonia.scm.repository.spi; + +public interface MergeCommand { + boolean merge(MergeCommandRequest request); +} diff --git a/scm-core/src/main/java/sonia/scm/repository/spi/MergeCommandRequest.java b/scm-core/src/main/java/sonia/scm/repository/spi/MergeCommandRequest.java new file mode 100644 index 0000000000..e01e580afa --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/repository/spi/MergeCommandRequest.java @@ -0,0 +1,70 @@ +package sonia.scm.repository.spi; + +import com.google.common.base.MoreObjects; +import com.google.common.base.Objects; +import com.google.common.base.Strings; +import sonia.scm.Validateable; + +import java.io.Serializable; + +public class MergeCommandRequest implements Validateable, Resetable, Serializable, Cloneable { + + private static final long serialVersionUID = -2650236557922431528L; + + private String branchToMerge; + private String targetBranch; + + public String getBranchToMerge() { + return branchToMerge; + } + + public void setBranchToMerge(String branchToMerge) { + this.branchToMerge = branchToMerge; + } + + public String getTargetBranch() { + return targetBranch; + } + + public void setTargetBranch(String targetBranch) { + this.targetBranch = targetBranch; + } + + public boolean isValid() { + return !Strings.isNullOrEmpty(getBranchToMerge()) && !Strings.isNullOrEmpty(getTargetBranch()); + } + + public void reset() { + this.setBranchToMerge(null); + this.setTargetBranch(null); + } + + @Override + public boolean equals(Object obj) { + if (obj == null) { + return false; + } + + if (getClass() != obj.getClass()) { + return false; + } + + final MergeCommandRequest other = (MergeCommandRequest) obj; + + return Objects.equal(branchToMerge, other.branchToMerge) + && Objects.equal(targetBranch, other.targetBranch); + } + + @Override + public int hashCode() { + return Objects.hashCode(branchToMerge, targetBranch); + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("branchToMerge", branchToMerge) + .add("targetBranch", targetBranch) + .toString(); + } +} 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 c66c56c0f1..77201d1a72 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 @@ -251,4 +251,12 @@ public abstract class RepositoryServiceProvider implements Closeable { throw new CommandNotSupportedException(Command.UNBUNDLE); } + + /** + * @since 2.0 + */ + public MergeCommand getMergeCommand() + { + throw new CommandNotSupportedException(Command.MERGE); + } } From 8df0b53f364175775893f528f612eb86b585c9b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 6 Nov 2018 14:27:08 +0100 Subject: [PATCH 02/27] Add command api for dry run merge --- .../sonia/scm/repository/api/Command.java | 7 +- .../api/MergeDryRunCommandBuilder.java | 35 ++++++++++ .../repository/spi/MergeDryRunCommand.java | 5 ++ .../spi/MergeDryRunCommandRequest.java | 70 +++++++++++++++++++ .../spi/RepositoryServiceProvider.java | 8 +++ 5 files changed, 124 insertions(+), 1 deletion(-) create mode 100644 scm-core/src/main/java/sonia/scm/repository/api/MergeDryRunCommandBuilder.java create mode 100644 scm-core/src/main/java/sonia/scm/repository/spi/MergeDryRunCommand.java create mode 100644 scm-core/src/main/java/sonia/scm/repository/spi/MergeDryRunCommandRequest.java 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 fa09cea2cb..2987232ff2 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 @@ -71,5 +71,10 @@ public enum Command /** * @since 2.0 */ - MERGE + MERGE, + + /** + * @since 2.0 + */ + MERGE_DRY_RUN } diff --git a/scm-core/src/main/java/sonia/scm/repository/api/MergeDryRunCommandBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/MergeDryRunCommandBuilder.java new file mode 100644 index 0000000000..852ffdcfbd --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/repository/api/MergeDryRunCommandBuilder.java @@ -0,0 +1,35 @@ +package sonia.scm.repository.api; + +import com.google.common.base.Preconditions; +import sonia.scm.repository.spi.MergeCommand; +import sonia.scm.repository.spi.MergeCommandRequest; + +public class MergeDryRunCommandBuilder { + + private final MergeCommand mergeCommand; + private final MergeCommandRequest request = new MergeCommandRequest(); + + public MergeDryRunCommandBuilder(MergeCommand mergeCommand) { + this.mergeCommand = mergeCommand; + } + + public MergeDryRunCommandBuilder setBranchToMerge(String branchToMerge) { + request.setBranchToMerge(branchToMerge); + return this; + } + + public MergeDryRunCommandBuilder setTargetBranch(String targetBranch) { + request.setTargetBranch(targetBranch); + return this; + } + + public MergeDryRunCommandBuilder reset() { + request.reset(); + return this; + } + + public boolean execute() { + Preconditions.checkArgument(request.isValid(), "revision to merge and target revision is required"); + return mergeCommand.merge(request); + } +} diff --git a/scm-core/src/main/java/sonia/scm/repository/spi/MergeDryRunCommand.java b/scm-core/src/main/java/sonia/scm/repository/spi/MergeDryRunCommand.java new file mode 100644 index 0000000000..af4fbe06bd --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/repository/spi/MergeDryRunCommand.java @@ -0,0 +1,5 @@ +package sonia.scm.repository.spi; + +public interface MergeDryRunCommand { + boolean isMergeable(MergeDryRunCommandRequest request); +} diff --git a/scm-core/src/main/java/sonia/scm/repository/spi/MergeDryRunCommandRequest.java b/scm-core/src/main/java/sonia/scm/repository/spi/MergeDryRunCommandRequest.java new file mode 100644 index 0000000000..b0cd737bd1 --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/repository/spi/MergeDryRunCommandRequest.java @@ -0,0 +1,70 @@ +package sonia.scm.repository.spi; + +import com.google.common.base.MoreObjects; +import com.google.common.base.Objects; +import com.google.common.base.Strings; +import sonia.scm.Validateable; + +import java.io.Serializable; + +public class MergeDryRunCommandRequest implements Validateable, Resetable, Serializable, Cloneable { + + private static final long serialVersionUID = -2650236557922431528L; + + private String branchToMerge; + private String targetBranch; + + public String getBranchToMerge() { + return branchToMerge; + } + + public void setBranchToMerge(String branchToMerge) { + this.branchToMerge = branchToMerge; + } + + public String getTargetBranch() { + return targetBranch; + } + + public void setTargetBranch(String targetBranch) { + this.targetBranch = targetBranch; + } + + public boolean isValid() { + return !Strings.isNullOrEmpty(getBranchToMerge()) && !Strings.isNullOrEmpty(getTargetBranch()); + } + + public void reset() { + this.setBranchToMerge(null); + this.setTargetBranch(null); + } + + @Override + public boolean equals(Object obj) { + if (obj == null) { + return false; + } + + if (getClass() != obj.getClass()) { + return false; + } + + final MergeDryRunCommandRequest other = (MergeDryRunCommandRequest) obj; + + return Objects.equal(branchToMerge, other.branchToMerge) + && Objects.equal(targetBranch, other.targetBranch); + } + + @Override + public int hashCode() { + return Objects.hashCode(branchToMerge, targetBranch); + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("branchToMerge", branchToMerge) + .add("targetBranch", targetBranch) + .toString(); + } +} 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 77201d1a72..33dd137302 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 @@ -259,4 +259,12 @@ public abstract class RepositoryServiceProvider implements Closeable { throw new CommandNotSupportedException(Command.MERGE); } + + /** + * @since 2.0 + */ + public MergeDryRunCommand getMergeDryRunCommand() + { + throw new CommandNotSupportedException(Command.MERGE_DRY_RUN); + } } From e0cc99a3d83ef6905f0efad946252d46d42a8a03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 6 Nov 2018 14:40:55 +0100 Subject: [PATCH 03/27] Implement simple merge dry run --- .../repository/spi/GitMergeDryRunCommand.java | 25 +++++++++++++++++++ .../spi/GitRepositoryServiceProvider.java | 9 +++++-- .../spi/GitMergeDryRunCommandTest.java | 23 +++++++++++++++++ 3 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeDryRunCommand.java create mode 100644 scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMergeDryRunCommandTest.java diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeDryRunCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeDryRunCommand.java new file mode 100644 index 0000000000..e49c520457 --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeDryRunCommand.java @@ -0,0 +1,25 @@ +package sonia.scm.repository.spi; + +import org.eclipse.jgit.merge.MergeStrategy; +import org.eclipse.jgit.merge.ResolveMerger; +import sonia.scm.repository.InternalRepositoryException; +import sonia.scm.repository.Repository; + +import java.io.IOException; + +public class GitMergeDryRunCommand extends AbstractGitCommand implements MergeDryRunCommand { + GitMergeDryRunCommand(GitContext context, Repository repository) { + super(context, repository); + } + + @Override + public boolean isMergeable(MergeDryRunCommandRequest request) { + try { + org.eclipse.jgit.lib.Repository repository = context.open(); + ResolveMerger merger = (ResolveMerger) MergeStrategy.RECURSIVE.newMerger(repository, true); + return merger.merge(repository.resolve(request.getBranchToMerge()), repository.resolve(request.getTargetBranch())); + } catch (IOException e) { + throw new InternalRepositoryException(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 d60abd424d..dd6c1b5492 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 @@ -63,7 +63,8 @@ public class GitRepositoryServiceProvider extends RepositoryServiceProvider Command.INCOMING, Command.OUTGOING, Command.PUSH, - Command.PULL + Command.PULL, + Command.MERGE_DRY_RUN ); //J+ @@ -240,7 +241,11 @@ public class GitRepositoryServiceProvider extends RepositoryServiceProvider return new GitTagsCommand(context, repository); } - //~--- fields --------------------------------------------------------------- + @Override + public MergeDryRunCommand getMergeDryRunCommand() { + return new GitMergeDryRunCommand(context, repository); + } +//~--- fields --------------------------------------------------------------- /** Field description */ private GitContext context; diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMergeDryRunCommandTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMergeDryRunCommandTest.java new file mode 100644 index 0000000000..891b8537dd --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMergeDryRunCommandTest.java @@ -0,0 +1,23 @@ +package sonia.scm.repository.spi; + +import org.junit.Assert; +import org.junit.Test; + +public class GitMergeDryRunCommandTest extends AbstractGitCommandTestBase { + @Test + public void shouldDetectNotMergeableBranches() { + GitMergeDryRunCommand command = createCommand(); + MergeDryRunCommandRequest request = new MergeDryRunCommandRequest(); + request.setBranchToMerge("test-branch"); + request.setTargetBranch("master"); + + boolean mergeable = command.isMergeable(request); + + Assert.assertFalse(mergeable); + } + + private GitMergeDryRunCommand createCommand() + { + return new GitMergeDryRunCommand(createContext(), repository); + } +} From 6863337faf1e7fe12a69caa181c83064bc40430c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 6 Nov 2018 17:14:27 +0100 Subject: [PATCH 04/27] Implement first steps for merge --- .../scm/repository/CloseableWrapper.java | 25 ++++++++ .../scm/repository/GitRepositoryHandler.java | 9 ++- .../sonia/scm/repository/GitWorkdirPool.java | 46 +++++++++++++++ .../scm/repository/spi/GitMergeCommand.java | 35 +++++++++++ .../spi/GitRepositoryServiceProvider.java | 6 ++ .../scm/repository/CloseableWrapperTest.java | 29 +++++++++ .../scm/repository/GitWorkdirPoolTest.java | 59 +++++++++++++++++++ .../spi/AbstractGitCommandTestBase.java | 4 +- 8 files changed, 211 insertions(+), 2 deletions(-) create mode 100644 scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/CloseableWrapper.java create mode 100644 scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitWorkdirPool.java create mode 100644 scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeCommand.java create mode 100644 scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/CloseableWrapperTest.java create mode 100644 scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitWorkdirPoolTest.java diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/CloseableWrapper.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/CloseableWrapper.java new file mode 100644 index 0000000000..553f0f5a00 --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/CloseableWrapper.java @@ -0,0 +1,25 @@ +package sonia.scm.repository; + +import java.util.function.Consumer; + +public class CloseableWrapper implements AutoCloseable { + + private final C wrapped; + private final Consumer cleanup; + + public CloseableWrapper(C wrapped, Consumer cleanup) { + this.wrapped = wrapped; + this.cleanup = cleanup; + } + + public C get() { return wrapped; } + + @Override + public void close() { + try { + cleanup.accept(wrapped); + } catch (Throwable t) { + throw new RuntimeException(t); + } + } +} diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitRepositoryHandler.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitRepositoryHandler.java index c3436c2395..00add44466 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitRepositoryHandler.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitRepositoryHandler.java @@ -90,6 +90,8 @@ public class GitRepositoryHandler private static final Object LOCK = new Object(); private final Scheduler scheduler; + + private final GitWorkdirPool workdirPool; private Task task; @@ -104,10 +106,11 @@ public class GitRepositoryHandler * @param scheduler */ @Inject - public GitRepositoryHandler(ConfigurationStoreFactory storeFactory, FileSystem fileSystem, Scheduler scheduler) + public GitRepositoryHandler(ConfigurationStoreFactory storeFactory, FileSystem fileSystem, Scheduler scheduler, GitWorkdirPool workdirPool) { super(storeFactory, fileSystem); this.scheduler = scheduler; + this.workdirPool = workdirPool; } //~--- get methods ---------------------------------------------------------- @@ -235,4 +238,8 @@ public class GitRepositoryHandler { return new File(directory, DIRECTORY_REFS).exists(); } + + public GitWorkdirPool getWorkdirPool() { + return workdirPool; + } } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitWorkdirPool.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitWorkdirPool.java new file mode 100644 index 0000000000..2c94a75be5 --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitWorkdirPool.java @@ -0,0 +1,46 @@ +package sonia.scm.repository; + +import org.eclipse.jgit.api.Git; +import org.eclipse.jgit.api.errors.GitAPIException; +import org.eclipse.jgit.lib.Repository; + +import java.io.File; +import java.util.Random; + + +/** + * Config: + * + * 1. Overall and absolute maximum of temp work directories + * 2. Maximum number of temp work directories pooled overall + * 3. Maximum number of temp work directories pooled for one master repository + */ +public class GitWorkdirPool { + + private final Random random = new Random(); + private final File poolDirectory; + + public GitWorkdirPool() { + this(new File(System.getProperty("java.io.tmpdir"))); + } + + public GitWorkdirPool(File poolDirectory) { + this.poolDirectory = poolDirectory; + } + + public CloseableWrapper getWorkingCopy(File bareRepository) { + try { + Git clone = cloneRepository(bareRepository, new File(poolDirectory, Long.toString(random.nextLong()))); + return new CloseableWrapper<>(clone.getRepository(), r -> clone.close()); + } catch (GitAPIException e) { + throw new InternalRepositoryException("could not clone working copy of repository", e); + } + } + + protected Git cloneRepository(File bareRepository, File target) throws GitAPIException { + return Git.cloneRepository() + .setURI(bareRepository.getAbsolutePath()) + .setDirectory(target) + .call(); + } +} 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 new file mode 100644 index 0000000000..b648681f61 --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeCommand.java @@ -0,0 +1,35 @@ +package sonia.scm.repository.spi; + +import org.eclipse.jgit.merge.MergeStrategy; +import org.eclipse.jgit.merge.ResolveMerger; +import sonia.scm.repository.CloseableWrapper; +import sonia.scm.repository.GitWorkdirPool; +import sonia.scm.repository.InternalRepositoryException; +import sonia.scm.repository.Repository; + +import java.io.IOException; + +public class GitMergeCommand extends AbstractGitCommand implements MergeCommand { + + private final GitWorkdirPool workdirPool; + + GitMergeCommand(GitContext context, Repository repository, GitWorkdirPool workdirPool) { + super(context, repository); + this.workdirPool = workdirPool; + } + + @Override + public boolean merge(MergeCommandRequest request) { + try (CloseableWrapper workingCopy = workdirPool.getWorkingCopy(context.open().getDirectory())) { + org.eclipse.jgit.lib.Repository repository = workingCopy.get(); + ResolveMerger merger = (ResolveMerger) MergeStrategy.RECURSIVE.newMerger(repository); + boolean mergeResult = merger.merge(repository.resolve(request.getBranchToMerge()), repository.resolve(request.getTargetBranch())); + if (mergeResult) { + // TODO push and verify push was successful + } + return mergeResult; + } catch (IOException e) { + throw new InternalRepositoryException(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 dd6c1b5492..ac5c161dc4 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 @@ -245,6 +245,12 @@ public class GitRepositoryServiceProvider extends RepositoryServiceProvider public MergeDryRunCommand getMergeDryRunCommand() { return new GitMergeDryRunCommand(context, repository); } + + @Override + public MergeCommand getMergeCommand() { + return new GitMergeCommand(context, repository, handler.getWorkdirPool()); + } + //~--- fields --------------------------------------------------------------- /** Field description */ diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/CloseableWrapperTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/CloseableWrapperTest.java new file mode 100644 index 0000000000..34c5e8e7e2 --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/CloseableWrapperTest.java @@ -0,0 +1,29 @@ +package sonia.scm.repository; + +import org.junit.Test; + +import java.util.function.Consumer; + +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; + +public class CloseableWrapperTest { + + @Test + public void x() { + Consumer wrapped = new Consumer() { + // no this cannot be replaced with a lambda because otherwise we could not use Mockito#spy + @Override + public void accept(String s) { + } + }; + + Consumer closer = spy(wrapped); + + try (CloseableWrapper wrapper = new CloseableWrapper<>("test", closer)) { + // nothing to do here + } + + verify(closer).accept("test"); + } +} diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitWorkdirPoolTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitWorkdirPoolTest.java new file mode 100644 index 0000000000..133df75ea7 --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitWorkdirPoolTest.java @@ -0,0 +1,59 @@ +package sonia.scm.repository; + +import org.eclipse.jgit.api.Git; +import org.eclipse.jgit.api.errors.GitAPIException; +import org.eclipse.jgit.lib.Repository; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import sonia.scm.repository.spi.AbstractGitCommandTestBase; + +import java.io.File; +import java.io.IOException; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; + +public class GitWorkdirPoolTest extends AbstractGitCommandTestBase { + + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); + + @Test + public void emptyPoolShouldCreateNewWorkdir() throws IOException { + GitWorkdirPool pool = new GitWorkdirPool(temporaryFolder.newFolder()); + File masterRepo = createRepositoryDirectory(); + + CloseableWrapper workingCopy = pool.getWorkingCopy(masterRepo); + + assertThat(workingCopy) + .isNotNull() + .extracting(w -> w.get().getDirectory()) + .isNotEqualTo(masterRepo); + } + + @Test + public void cloneFromPoolShouldBeClosed() throws IOException { + PoolWithSpy pool = new PoolWithSpy(temporaryFolder.newFolder()); + File masterRepo = createRepositoryDirectory(); + + try (CloseableWrapper workingCopy = pool.getWorkingCopy(masterRepo)) { + assertThat(workingCopy).isNotNull(); + } + verify(pool.createdClone).close(); + } + + private static class PoolWithSpy extends GitWorkdirPool { + PoolWithSpy(File poolDirectory) { + super(poolDirectory); + } + + Git createdClone; + @Override + protected Git cloneRepository(File bareRepository, File destination) throws GitAPIException { + createdClone = spy(super.cloneRepository(bareRepository, destination)); + return createdClone; + } + } +} diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/AbstractGitCommandTestBase.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/AbstractGitCommandTestBase.java index 496b71e656..7d56db00d7 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/AbstractGitCommandTestBase.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/AbstractGitCommandTestBase.java @@ -50,7 +50,9 @@ public class AbstractGitCommandTestBase extends ZippedRepositoryTestBase @After public void close() { - context.close(); + if (context != null) { + context.close(); + } } /** From 23783d43f689b214832b7ff2b81fc7baaaf7ec3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 7 Nov 2018 08:19:38 +0100 Subject: [PATCH 05/27] Fix build breaker --- .../sonia/scm/repository/GitRepositoryHandlerTest.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitRepositoryHandlerTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitRepositoryHandlerTest.java index a9969f27f8..26c449835b 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitRepositoryHandlerTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitRepositoryHandlerTest.java @@ -61,6 +61,9 @@ public class GitRepositoryHandlerTest extends SimpleRepositoryHandlerTestBase { @Mock private ConfigurationStoreFactory factory; + @Mock + private GitWorkdirPool gitWorkdirPool; + @Override protected void checkDirectory(File directory) { File head = new File(directory, "HEAD"); @@ -84,7 +87,7 @@ public class GitRepositoryHandlerTest extends SimpleRepositoryHandlerTestBase { protected RepositoryHandler createRepositoryHandler(ConfigurationStoreFactory factory, File directory) { GitRepositoryHandler repositoryHandler = new GitRepositoryHandler(factory, - new DefaultFileSystem(), scheduler); + new DefaultFileSystem(), scheduler, gitWorkdirPool); repositoryHandler.init(contextProvider); @@ -100,7 +103,7 @@ public class GitRepositoryHandlerTest extends SimpleRepositoryHandlerTestBase { @Test public void getDirectory() { GitRepositoryHandler repositoryHandler = new GitRepositoryHandler(factory, - new DefaultFileSystem(), scheduler); + new DefaultFileSystem(), scheduler, gitWorkdirPool); GitConfig gitConfig = new GitConfig(); gitConfig.setRepositoryDirectory(new File("/path")); From fc3e08d61272b634fdda19fe32a9af1dfff3b741 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 7 Nov 2018 08:32:27 +0100 Subject: [PATCH 06/27] Include dry run in merge command --- .../repository/api/MergeCommandBuilder.java | 7 +- .../repository/api/MergeCommandResult.java | 13 ++++ .../api/MergeDryRunCommandBuilder.java | 35 ---------- .../api/MergeDryRunCommandResult.java | 14 ++++ .../scm/repository/spi/MergeCommand.java | 7 +- .../repository/spi/MergeDryRunCommand.java | 5 -- .../spi/MergeDryRunCommandRequest.java | 70 ------------------- .../scm/repository/spi/GitMergeCommand.java | 17 ++++- .../repository/spi/GitMergeDryRunCommand.java | 25 ------- .../spi/GitRepositoryServiceProvider.java | 5 -- .../repository/spi/GitMergeCommandTest.java | 22 ++++++ .../spi/GitMergeDryRunCommandTest.java | 23 ------ 12 files changed, 76 insertions(+), 167 deletions(-) create mode 100644 scm-core/src/main/java/sonia/scm/repository/api/MergeCommandResult.java delete mode 100644 scm-core/src/main/java/sonia/scm/repository/api/MergeDryRunCommandBuilder.java create mode 100644 scm-core/src/main/java/sonia/scm/repository/api/MergeDryRunCommandResult.java delete mode 100644 scm-core/src/main/java/sonia/scm/repository/spi/MergeDryRunCommand.java delete mode 100644 scm-core/src/main/java/sonia/scm/repository/spi/MergeDryRunCommandRequest.java delete mode 100644 scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeDryRunCommand.java create mode 100644 scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMergeCommandTest.java delete mode 100644 scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMergeDryRunCommandTest.java diff --git a/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java index 6d9b332f0a..65eed2f2a1 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java @@ -28,8 +28,13 @@ public class MergeCommandBuilder { return this; } - public boolean execute() { + public MergeCommandResult executeMerge() { Preconditions.checkArgument(request.isValid(), "revision to merge and target revision is required"); return mergeCommand.merge(request); } + + public MergeDryRunCommandResult dryRun() { + Preconditions.checkArgument(request.isValid(), "revision to merge and target revision is required"); + return mergeCommand.dryRun(request); + } } diff --git a/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandResult.java b/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandResult.java new file mode 100644 index 0000000000..024e7bbc89 --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandResult.java @@ -0,0 +1,13 @@ +package sonia.scm.repository.api; + +public class MergeCommandResult { + private final boolean success; + + public MergeCommandResult(boolean success) { + this.success = success; + } + + public boolean isSuccess() { + return success; + } +} diff --git a/scm-core/src/main/java/sonia/scm/repository/api/MergeDryRunCommandBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/MergeDryRunCommandBuilder.java deleted file mode 100644 index 852ffdcfbd..0000000000 --- a/scm-core/src/main/java/sonia/scm/repository/api/MergeDryRunCommandBuilder.java +++ /dev/null @@ -1,35 +0,0 @@ -package sonia.scm.repository.api; - -import com.google.common.base.Preconditions; -import sonia.scm.repository.spi.MergeCommand; -import sonia.scm.repository.spi.MergeCommandRequest; - -public class MergeDryRunCommandBuilder { - - private final MergeCommand mergeCommand; - private final MergeCommandRequest request = new MergeCommandRequest(); - - public MergeDryRunCommandBuilder(MergeCommand mergeCommand) { - this.mergeCommand = mergeCommand; - } - - public MergeDryRunCommandBuilder setBranchToMerge(String branchToMerge) { - request.setBranchToMerge(branchToMerge); - return this; - } - - public MergeDryRunCommandBuilder setTargetBranch(String targetBranch) { - request.setTargetBranch(targetBranch); - return this; - } - - public MergeDryRunCommandBuilder reset() { - request.reset(); - return this; - } - - public boolean execute() { - Preconditions.checkArgument(request.isValid(), "revision to merge and target revision is required"); - return mergeCommand.merge(request); - } -} diff --git a/scm-core/src/main/java/sonia/scm/repository/api/MergeDryRunCommandResult.java b/scm-core/src/main/java/sonia/scm/repository/api/MergeDryRunCommandResult.java new file mode 100644 index 0000000000..aeb18a753d --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/repository/api/MergeDryRunCommandResult.java @@ -0,0 +1,14 @@ +package sonia.scm.repository.api; + +public class MergeDryRunCommandResult { + + private final boolean mergeable; + + public MergeDryRunCommandResult(boolean mergeable) { + this.mergeable = mergeable; + } + + public boolean isMergeable() { + return mergeable; + } +} diff --git a/scm-core/src/main/java/sonia/scm/repository/spi/MergeCommand.java b/scm-core/src/main/java/sonia/scm/repository/spi/MergeCommand.java index 326bba4dce..0a3680f6b3 100644 --- a/scm-core/src/main/java/sonia/scm/repository/spi/MergeCommand.java +++ b/scm-core/src/main/java/sonia/scm/repository/spi/MergeCommand.java @@ -1,5 +1,10 @@ package sonia.scm.repository.spi; +import sonia.scm.repository.api.MergeCommandResult; +import sonia.scm.repository.api.MergeDryRunCommandResult; + public interface MergeCommand { - boolean merge(MergeCommandRequest request); + MergeCommandResult merge(MergeCommandRequest request); + + MergeDryRunCommandResult dryRun(MergeCommandRequest request); } diff --git a/scm-core/src/main/java/sonia/scm/repository/spi/MergeDryRunCommand.java b/scm-core/src/main/java/sonia/scm/repository/spi/MergeDryRunCommand.java deleted file mode 100644 index af4fbe06bd..0000000000 --- a/scm-core/src/main/java/sonia/scm/repository/spi/MergeDryRunCommand.java +++ /dev/null @@ -1,5 +0,0 @@ -package sonia.scm.repository.spi; - -public interface MergeDryRunCommand { - boolean isMergeable(MergeDryRunCommandRequest request); -} diff --git a/scm-core/src/main/java/sonia/scm/repository/spi/MergeDryRunCommandRequest.java b/scm-core/src/main/java/sonia/scm/repository/spi/MergeDryRunCommandRequest.java deleted file mode 100644 index b0cd737bd1..0000000000 --- a/scm-core/src/main/java/sonia/scm/repository/spi/MergeDryRunCommandRequest.java +++ /dev/null @@ -1,70 +0,0 @@ -package sonia.scm.repository.spi; - -import com.google.common.base.MoreObjects; -import com.google.common.base.Objects; -import com.google.common.base.Strings; -import sonia.scm.Validateable; - -import java.io.Serializable; - -public class MergeDryRunCommandRequest implements Validateable, Resetable, Serializable, Cloneable { - - private static final long serialVersionUID = -2650236557922431528L; - - private String branchToMerge; - private String targetBranch; - - public String getBranchToMerge() { - return branchToMerge; - } - - public void setBranchToMerge(String branchToMerge) { - this.branchToMerge = branchToMerge; - } - - public String getTargetBranch() { - return targetBranch; - } - - public void setTargetBranch(String targetBranch) { - this.targetBranch = targetBranch; - } - - public boolean isValid() { - return !Strings.isNullOrEmpty(getBranchToMerge()) && !Strings.isNullOrEmpty(getTargetBranch()); - } - - public void reset() { - this.setBranchToMerge(null); - this.setTargetBranch(null); - } - - @Override - public boolean equals(Object obj) { - if (obj == null) { - return false; - } - - if (getClass() != obj.getClass()) { - return false; - } - - final MergeDryRunCommandRequest other = (MergeDryRunCommandRequest) obj; - - return Objects.equal(branchToMerge, other.branchToMerge) - && Objects.equal(targetBranch, other.targetBranch); - } - - @Override - public int hashCode() { - return Objects.hashCode(branchToMerge, targetBranch); - } - - @Override - public String toString() { - return MoreObjects.toStringHelper(this) - .add("branchToMerge", branchToMerge) - .add("targetBranch", targetBranch) - .toString(); - } -} 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 b648681f61..ba1cc0e496 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 @@ -6,6 +6,8 @@ import sonia.scm.repository.CloseableWrapper; import sonia.scm.repository.GitWorkdirPool; import sonia.scm.repository.InternalRepositoryException; import sonia.scm.repository.Repository; +import sonia.scm.repository.api.MergeCommandResult; +import sonia.scm.repository.api.MergeDryRunCommandResult; import java.io.IOException; @@ -19,7 +21,7 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand } @Override - public boolean merge(MergeCommandRequest request) { + public MergeCommandResult merge(MergeCommandRequest request) { try (CloseableWrapper workingCopy = workdirPool.getWorkingCopy(context.open().getDirectory())) { org.eclipse.jgit.lib.Repository repository = workingCopy.get(); ResolveMerger merger = (ResolveMerger) MergeStrategy.RECURSIVE.newMerger(repository); @@ -27,7 +29,18 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand if (mergeResult) { // TODO push and verify push was successful } - return mergeResult; + return new MergeCommandResult(mergeResult); + } catch (IOException e) { + throw new InternalRepositoryException(e); + } + } + + @Override + public MergeDryRunCommandResult dryRun(MergeCommandRequest request) { + try { + org.eclipse.jgit.lib.Repository repository = context.open(); + ResolveMerger merger = (ResolveMerger) MergeStrategy.RECURSIVE.newMerger(repository, true); + return new MergeDryRunCommandResult(merger.merge(repository.resolve(request.getBranchToMerge()), repository.resolve(request.getTargetBranch()))); } catch (IOException e) { throw new InternalRepositoryException(e); } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeDryRunCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeDryRunCommand.java deleted file mode 100644 index e49c520457..0000000000 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeDryRunCommand.java +++ /dev/null @@ -1,25 +0,0 @@ -package sonia.scm.repository.spi; - -import org.eclipse.jgit.merge.MergeStrategy; -import org.eclipse.jgit.merge.ResolveMerger; -import sonia.scm.repository.InternalRepositoryException; -import sonia.scm.repository.Repository; - -import java.io.IOException; - -public class GitMergeDryRunCommand extends AbstractGitCommand implements MergeDryRunCommand { - GitMergeDryRunCommand(GitContext context, Repository repository) { - super(context, repository); - } - - @Override - public boolean isMergeable(MergeDryRunCommandRequest request) { - try { - org.eclipse.jgit.lib.Repository repository = context.open(); - ResolveMerger merger = (ResolveMerger) MergeStrategy.RECURSIVE.newMerger(repository, true); - return merger.merge(repository.resolve(request.getBranchToMerge()), repository.resolve(request.getTargetBranch())); - } catch (IOException e) { - throw new InternalRepositoryException(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 ac5c161dc4..c29da1029f 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 @@ -241,11 +241,6 @@ public class GitRepositoryServiceProvider extends RepositoryServiceProvider return new GitTagsCommand(context, repository); } - @Override - public MergeDryRunCommand getMergeDryRunCommand() { - return new GitMergeDryRunCommand(context, repository); - } - @Override public MergeCommand getMergeCommand() { return new GitMergeCommand(context, repository, handler.getWorkdirPool()); 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 new file mode 100644 index 0000000000..3c58916807 --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMergeCommandTest.java @@ -0,0 +1,22 @@ +package sonia.scm.repository.spi; + +import org.junit.Assert; +import org.junit.Test; + +public class GitMergeCommandTest extends AbstractGitCommandTestBase { + @Test + public void shouldDetectNotMergeableBranches() { + GitMergeCommand command = createCommand(); + MergeCommandRequest request = new MergeCommandRequest(); + request.setBranchToMerge("test-branch"); + request.setTargetBranch("master"); + + boolean mergeable = command.dryRun(request).isMergeable(); + + Assert.assertFalse(mergeable); + } + + private GitMergeCommand createCommand() { + return new GitMergeCommand(createContext(), repository, null); + } +} diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMergeDryRunCommandTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMergeDryRunCommandTest.java deleted file mode 100644 index 891b8537dd..0000000000 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMergeDryRunCommandTest.java +++ /dev/null @@ -1,23 +0,0 @@ -package sonia.scm.repository.spi; - -import org.junit.Assert; -import org.junit.Test; - -public class GitMergeDryRunCommandTest extends AbstractGitCommandTestBase { - @Test - public void shouldDetectNotMergeableBranches() { - GitMergeDryRunCommand command = createCommand(); - MergeDryRunCommandRequest request = new MergeDryRunCommandRequest(); - request.setBranchToMerge("test-branch"); - request.setTargetBranch("master"); - - boolean mergeable = command.isMergeable(request); - - Assert.assertFalse(mergeable); - } - - private GitMergeDryRunCommand createCommand() - { - return new GitMergeDryRunCommand(createContext(), repository); - } -} From 4cdf3e468cf040727bc7a6df795f8f682287df2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 7 Nov 2018 08:38:26 +0100 Subject: [PATCH 07/27] Add merge command to repository service api --- .../scm/repository/api/RepositoryService.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) 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 bdd6e4b320..fe0529e6b5 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 @@ -79,6 +79,7 @@ import java.util.stream.Stream; * @apiviz.uses sonia.scm.repository.api.PushCommandBuilder * @apiviz.uses sonia.scm.repository.api.BundleCommandBuilder * @apiviz.uses sonia.scm.repository.api.UnbundleCommandBuilder + * @apiviz.uses sonia.scm.repository.api.MergeCommandBuilder * @since 1.17 */ @Slf4j @@ -353,6 +354,22 @@ public final class RepositoryService implements Closeable { repository); } + /** + * The merge command executes a merge of two branches. It is possible to do a dry run to check, whether the given + * branches can be merged without conflicts. + * + * @return instance of {@link MergeCommandBuilder} + * @throws CommandNotSupportedException if the command is not supported + * by the implementation of the repository service provider. + * @since 2.0.0 + */ + public MergeCommandBuilder gerMergeCommand() { + logger.debug("create unbundle command for repository {}", + repository.getNamespaceAndName()); + + return new MergeCommandBuilder(provider.getMergeCommand()); + } + /** * Returns true if the command is supported by the repository service. * From 33642352ead7c1ac5d840700244bbfd782d42180 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 7 Nov 2018 10:28:27 +0100 Subject: [PATCH 08/27] Start with a simple factory instead of a complex pool --- .../scm/repository/GitRepositoryHandler.java | 10 +-- .../scm/repository/GitWorkdirFactory.java | 8 ++ .../sonia/scm/repository/GitWorkdirPool.java | 46 ---------- .../sonia/scm/repository/spi/GitContext.java | 4 + .../scm/repository/spi/GitMergeCommand.java | 15 ++-- .../spi/GitRepositoryServiceProvider.java | 2 +- .../spi/SimpleGitWorkdirFactory.java | 56 ++++++++++++ .../sonia/scm/repository/spi/WorkingCopy.java | 12 +++ .../repository/GitRepositoryHandlerTest.java | 6 +- .../scm/repository/GitWorkdirPoolTest.java | 59 ------------- .../spi/SimpleGitWorkdirFactoryTest.java | 87 +++++++++++++++++++ 11 files changed, 183 insertions(+), 122 deletions(-) create mode 100644 scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitWorkdirFactory.java delete mode 100644 scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitWorkdirPool.java create mode 100644 scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/SimpleGitWorkdirFactory.java create mode 100644 scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/WorkingCopy.java delete mode 100644 scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitWorkdirPoolTest.java create mode 100644 scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/SimpleGitWorkdirFactoryTest.java diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitRepositoryHandler.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitRepositoryHandler.java index 00add44466..faf2b23a27 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitRepositoryHandler.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitRepositoryHandler.java @@ -91,7 +91,7 @@ public class GitRepositoryHandler private final Scheduler scheduler; - private final GitWorkdirPool workdirPool; + private final GitWorkdirFactory workdirFactory; private Task task; @@ -106,11 +106,11 @@ public class GitRepositoryHandler * @param scheduler */ @Inject - public GitRepositoryHandler(ConfigurationStoreFactory storeFactory, FileSystem fileSystem, Scheduler scheduler, GitWorkdirPool workdirPool) + public GitRepositoryHandler(ConfigurationStoreFactory storeFactory, FileSystem fileSystem, Scheduler scheduler, GitWorkdirFactory workdirFactory) { super(storeFactory, fileSystem); this.scheduler = scheduler; - this.workdirPool = workdirPool; + this.workdirFactory = workdirFactory; } //~--- get methods ---------------------------------------------------------- @@ -239,7 +239,7 @@ public class GitRepositoryHandler return new File(directory, DIRECTORY_REFS).exists(); } - public GitWorkdirPool getWorkdirPool() { - return workdirPool; + public GitWorkdirFactory getWorkdirFactory() { + return workdirFactory; } } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitWorkdirFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitWorkdirFactory.java new file mode 100644 index 0000000000..f93713a221 --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitWorkdirFactory.java @@ -0,0 +1,8 @@ +package sonia.scm.repository; + +import sonia.scm.repository.spi.GitContext; +import sonia.scm.repository.spi.WorkingCopy; + +public interface GitWorkdirFactory { + WorkingCopy createWorkingCopy(GitContext gitContext); +} diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitWorkdirPool.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitWorkdirPool.java deleted file mode 100644 index 2c94a75be5..0000000000 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitWorkdirPool.java +++ /dev/null @@ -1,46 +0,0 @@ -package sonia.scm.repository; - -import org.eclipse.jgit.api.Git; -import org.eclipse.jgit.api.errors.GitAPIException; -import org.eclipse.jgit.lib.Repository; - -import java.io.File; -import java.util.Random; - - -/** - * Config: - * - * 1. Overall and absolute maximum of temp work directories - * 2. Maximum number of temp work directories pooled overall - * 3. Maximum number of temp work directories pooled for one master repository - */ -public class GitWorkdirPool { - - private final Random random = new Random(); - private final File poolDirectory; - - public GitWorkdirPool() { - this(new File(System.getProperty("java.io.tmpdir"))); - } - - public GitWorkdirPool(File poolDirectory) { - this.poolDirectory = poolDirectory; - } - - public CloseableWrapper getWorkingCopy(File bareRepository) { - try { - Git clone = cloneRepository(bareRepository, new File(poolDirectory, Long.toString(random.nextLong()))); - return new CloseableWrapper<>(clone.getRepository(), r -> clone.close()); - } catch (GitAPIException e) { - throw new InternalRepositoryException("could not clone working copy of repository", e); - } - } - - protected Git cloneRepository(File bareRepository, File target) throws GitAPIException { - return Git.cloneRepository() - .setURI(bareRepository.getAbsolutePath()) - .setDirectory(target) - .call(); - } -} diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitContext.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitContext.java index 2175846d5a..a10e16e200 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitContext.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitContext.java @@ -106,6 +106,10 @@ public class GitContext implements Closeable return repository; } + File getDirectory() { + return directory; + } + //~--- 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 ba1cc0e496..6e69a2c5ba 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 @@ -2,10 +2,9 @@ package sonia.scm.repository.spi; import org.eclipse.jgit.merge.MergeStrategy; import org.eclipse.jgit.merge.ResolveMerger; -import sonia.scm.repository.CloseableWrapper; -import sonia.scm.repository.GitWorkdirPool; +import org.eclipse.jgit.lib.Repository; +import sonia.scm.repository.GitWorkdirFactory; import sonia.scm.repository.InternalRepositoryException; -import sonia.scm.repository.Repository; import sonia.scm.repository.api.MergeCommandResult; import sonia.scm.repository.api.MergeDryRunCommandResult; @@ -13,17 +12,17 @@ import java.io.IOException; public class GitMergeCommand extends AbstractGitCommand implements MergeCommand { - private final GitWorkdirPool workdirPool; + private final GitWorkdirFactory workdirPool; - GitMergeCommand(GitContext context, Repository repository, GitWorkdirPool workdirPool) { + GitMergeCommand(GitContext context, sonia.scm.repository.Repository repository, GitWorkdirFactory workdirPool) { super(context, repository); this.workdirPool = workdirPool; } @Override public MergeCommandResult merge(MergeCommandRequest request) { - try (CloseableWrapper workingCopy = workdirPool.getWorkingCopy(context.open().getDirectory())) { - org.eclipse.jgit.lib.Repository repository = workingCopy.get(); + try (WorkingCopy workingCopy = workdirPool.createWorkingCopy(context)) { + Repository repository = workingCopy.get(); ResolveMerger merger = (ResolveMerger) MergeStrategy.RECURSIVE.newMerger(repository); boolean mergeResult = merger.merge(repository.resolve(request.getBranchToMerge()), repository.resolve(request.getTargetBranch())); if (mergeResult) { @@ -38,7 +37,7 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand @Override public MergeDryRunCommandResult dryRun(MergeCommandRequest request) { try { - org.eclipse.jgit.lib.Repository repository = context.open(); + Repository repository = context.open(); ResolveMerger merger = (ResolveMerger) MergeStrategy.RECURSIVE.newMerger(repository, true); return new MergeDryRunCommandResult(merger.merge(repository.resolve(request.getBranchToMerge()), repository.resolve(request.getTargetBranch()))); } catch (IOException 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 c29da1029f..846c127c4c 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 @@ -243,7 +243,7 @@ public class GitRepositoryServiceProvider extends RepositoryServiceProvider @Override public MergeCommand getMergeCommand() { - return new GitMergeCommand(context, repository, handler.getWorkdirPool()); + return new GitMergeCommand(context, repository, handler.getWorkdirFactory()); } //~--- fields --------------------------------------------------------------- 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 new file mode 100644 index 0000000000..c69acf5c6e --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/SimpleGitWorkdirFactory.java @@ -0,0 +1,56 @@ +package sonia.scm.repository.spi; + +import org.eclipse.jgit.api.Git; +import org.eclipse.jgit.api.errors.GitAPIException; +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.util.FileUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import sonia.scm.repository.GitWorkdirFactory; +import sonia.scm.repository.InternalRepositoryException; + +import java.io.File; +import java.io.IOException; +import java.util.Random; + +public class SimpleGitWorkdirFactory implements GitWorkdirFactory { + + private static final Logger logger = LoggerFactory.getLogger(SimpleGitWorkdirFactory.class); + + private final Random random = new Random(); + private final File poolDirectory; + + public SimpleGitWorkdirFactory() { + this(new File(System.getProperty("java.io.tmpdir"), "scmm-git-pool")); + } + + public SimpleGitWorkdirFactory(File poolDirectory) { + this.poolDirectory = poolDirectory; + } + + public WorkingCopy createWorkingCopy(GitContext gitContext) { + try { + Repository clone = cloneRepository(gitContext.getDirectory(), new File(poolDirectory, Long.toString(random.nextLong()))); + return new WorkingCopy(clone, this::close); + } catch (GitAPIException e) { + throw new InternalRepositoryException("could not clone working copy of repository", e); + } + } + + protected Repository cloneRepository(File bareRepository, File target) throws GitAPIException { + return Git.cloneRepository() + .setURI(bareRepository.getAbsolutePath()) + .setDirectory(target) + .call() + .getRepository(); + } + + private void close(Repository repository) { + repository.close(); + try { + FileUtils.delete(repository.getDirectory(), FileUtils.RECURSIVE); + } catch (IOException e) { + logger.warn("could not delete temporary git workdir '{}'", repository.getDirectory(), e); + } + } +} diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/WorkingCopy.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/WorkingCopy.java new file mode 100644 index 0000000000..fd0cba510b --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/WorkingCopy.java @@ -0,0 +1,12 @@ +package sonia.scm.repository.spi; + +import org.eclipse.jgit.lib.Repository; +import sonia.scm.repository.CloseableWrapper; + +import java.util.function.Consumer; + +public class WorkingCopy extends CloseableWrapper { + WorkingCopy(Repository wrapped, Consumer cleanup) { + super(wrapped, cleanup); + } +} diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitRepositoryHandlerTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitRepositoryHandlerTest.java index 26c449835b..ad731280c5 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitRepositoryHandlerTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitRepositoryHandlerTest.java @@ -62,7 +62,7 @@ public class GitRepositoryHandlerTest extends SimpleRepositoryHandlerTestBase { private ConfigurationStoreFactory factory; @Mock - private GitWorkdirPool gitWorkdirPool; + private GitWorkdirFactory gitWorkdirFactory; @Override protected void checkDirectory(File directory) { @@ -87,7 +87,7 @@ public class GitRepositoryHandlerTest extends SimpleRepositoryHandlerTestBase { protected RepositoryHandler createRepositoryHandler(ConfigurationStoreFactory factory, File directory) { GitRepositoryHandler repositoryHandler = new GitRepositoryHandler(factory, - new DefaultFileSystem(), scheduler, gitWorkdirPool); + new DefaultFileSystem(), scheduler, gitWorkdirFactory); repositoryHandler.init(contextProvider); @@ -103,7 +103,7 @@ public class GitRepositoryHandlerTest extends SimpleRepositoryHandlerTestBase { @Test public void getDirectory() { GitRepositoryHandler repositoryHandler = new GitRepositoryHandler(factory, - new DefaultFileSystem(), scheduler, gitWorkdirPool); + new DefaultFileSystem(), scheduler, gitWorkdirFactory); GitConfig gitConfig = new GitConfig(); gitConfig.setRepositoryDirectory(new File("/path")); diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitWorkdirPoolTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitWorkdirPoolTest.java deleted file mode 100644 index 133df75ea7..0000000000 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitWorkdirPoolTest.java +++ /dev/null @@ -1,59 +0,0 @@ -package sonia.scm.repository; - -import org.eclipse.jgit.api.Git; -import org.eclipse.jgit.api.errors.GitAPIException; -import org.eclipse.jgit.lib.Repository; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.TemporaryFolder; -import sonia.scm.repository.spi.AbstractGitCommandTestBase; - -import java.io.File; -import java.io.IOException; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.verify; - -public class GitWorkdirPoolTest extends AbstractGitCommandTestBase { - - @Rule - public TemporaryFolder temporaryFolder = new TemporaryFolder(); - - @Test - public void emptyPoolShouldCreateNewWorkdir() throws IOException { - GitWorkdirPool pool = new GitWorkdirPool(temporaryFolder.newFolder()); - File masterRepo = createRepositoryDirectory(); - - CloseableWrapper workingCopy = pool.getWorkingCopy(masterRepo); - - assertThat(workingCopy) - .isNotNull() - .extracting(w -> w.get().getDirectory()) - .isNotEqualTo(masterRepo); - } - - @Test - public void cloneFromPoolShouldBeClosed() throws IOException { - PoolWithSpy pool = new PoolWithSpy(temporaryFolder.newFolder()); - File masterRepo = createRepositoryDirectory(); - - try (CloseableWrapper workingCopy = pool.getWorkingCopy(masterRepo)) { - assertThat(workingCopy).isNotNull(); - } - verify(pool.createdClone).close(); - } - - private static class PoolWithSpy extends GitWorkdirPool { - PoolWithSpy(File poolDirectory) { - super(poolDirectory); - } - - Git createdClone; - @Override - protected Git cloneRepository(File bareRepository, File destination) throws GitAPIException { - createdClone = spy(super.cloneRepository(bareRepository, destination)); - return createdClone; - } - } -} 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 new file mode 100644 index 0000000000..d9c41f85b5 --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/SimpleGitWorkdirFactoryTest.java @@ -0,0 +1,87 @@ +package sonia.scm.repository.spi; + +import org.eclipse.jgit.api.errors.GitAPIException; +import org.eclipse.jgit.lib.Repository; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +import java.io.File; +import java.io.IOException; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; + +public class SimpleGitWorkdirFactoryTest extends AbstractGitCommandTestBase { + + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); + + @Test + public void emptyPoolShouldCreateNewWorkdir() throws IOException { + SimpleGitWorkdirFactory factory = new SimpleGitWorkdirFactory(temporaryFolder.newFolder()); + File masterRepo = createRepositoryDirectory(); + + try (WorkingCopy workingCopy = factory.createWorkingCopy(createContext())) { + + assertThat(workingCopy.get().getDirectory()) + .exists() + .isNotEqualTo(masterRepo) + .isDirectory(); + assertThat(new File(workingCopy.get().getWorkTree(), "a.txt")) + .exists() + .isFile() + .hasContent("a\nline for blame"); + } + } + + @Test + public void cloneFromPoolShouldBeClosed() throws IOException { + PoolWithSpy factory = new PoolWithSpy(temporaryFolder.newFolder()); + + try (WorkingCopy workingCopy = factory.createWorkingCopy(createContext())) { + assertThat(workingCopy).isNotNull(); + } + verify(factory.createdClone).close(); + } + + @Test + public void cloneFromPoolShouldNotBeReused() throws IOException { + SimpleGitWorkdirFactory factory = new SimpleGitWorkdirFactory(temporaryFolder.newFolder()); + + File firstDirectory; + try (WorkingCopy workingCopy = factory.createWorkingCopy(createContext())) { + firstDirectory = workingCopy.get().getDirectory(); + } + try (WorkingCopy workingCopy = factory.createWorkingCopy(createContext())) { + File secondDirectory = workingCopy.get().getDirectory(); + assertThat(secondDirectory).isNotEqualTo(firstDirectory); + } + } + + @Test + public void cloneFromPoolShouldBeDeletedOnClose() throws IOException { + SimpleGitWorkdirFactory factory = new SimpleGitWorkdirFactory(temporaryFolder.newFolder()); + + File directory; + try (WorkingCopy workingCopy = factory.createWorkingCopy(createContext())) { + directory = workingCopy.get().getDirectory(); + } + assertThat(directory).doesNotExist(); + } + + private static class PoolWithSpy extends SimpleGitWorkdirFactory { + PoolWithSpy(File poolDirectory) { + super(poolDirectory); + } + + Repository createdClone; + + @Override + protected Repository cloneRepository(File bareRepository, File destination) throws GitAPIException { + createdClone = spy(super.cloneRepository(bareRepository, destination)); + return createdClone; + } + } +} From 62c20b071b06254117ad9774129e3639f26a2bb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 7 Nov 2018 10:33:47 +0100 Subject: [PATCH 09/27] Fix reminders of dry run --- .../src/main/java/sonia/scm/repository/api/Command.java | 7 +------ .../scm/repository/spi/RepositoryServiceProvider.java | 8 -------- .../scm/repository/spi/GitRepositoryServiceProvider.java | 2 +- 3 files changed, 2 insertions(+), 15 deletions(-) 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 2987232ff2..fa09cea2cb 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 @@ -71,10 +71,5 @@ public enum Command /** * @since 2.0 */ - MERGE, - - /** - * @since 2.0 - */ - MERGE_DRY_RUN + MERGE } 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 33dd137302..77201d1a72 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 @@ -259,12 +259,4 @@ public abstract class RepositoryServiceProvider implements Closeable { throw new CommandNotSupportedException(Command.MERGE); } - - /** - * @since 2.0 - */ - public MergeDryRunCommand getMergeDryRunCommand() - { - throw new CommandNotSupportedException(Command.MERGE_DRY_RUN); - } } 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 846c127c4c..a0a3074014 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 @@ -64,7 +64,7 @@ public class GitRepositoryServiceProvider extends RepositoryServiceProvider Command.OUTGOING, Command.PUSH, Command.PULL, - Command.MERGE_DRY_RUN + Command.MERGE ); //J+ From e2766d494a0eea7b32a613613e66aa168430e3bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 7 Nov 2018 10:38:01 +0100 Subject: [PATCH 10/27] Register SimpleGitWorkdirFactory in guice context --- .../src/main/java/sonia/scm/web/GitServletModule.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitServletModule.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitServletModule.java index e731e01a62..a3dac0e7d1 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitServletModule.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitServletModule.java @@ -41,6 +41,8 @@ import org.mapstruct.factory.Mappers; import sonia.scm.api.v2.resources.GitConfigDtoToGitConfigMapper; import sonia.scm.api.v2.resources.GitConfigToGitConfigDtoMapper; import sonia.scm.plugin.Extension; +import sonia.scm.repository.GitWorkdirFactory; +import sonia.scm.repository.spi.SimpleGitWorkdirFactory; import sonia.scm.web.lfs.LfsBlobStoreFactory; /** @@ -63,5 +65,7 @@ public class GitServletModule extends ServletModule bind(GitConfigDtoToGitConfigMapper.class).to(Mappers.getMapper(GitConfigDtoToGitConfigMapper.class).getClass()); bind(GitConfigToGitConfigDtoMapper.class).to(Mappers.getMapper(GitConfigToGitConfigDtoMapper.class).getClass()); + + bind(GitWorkdirFactory.class).to(SimpleGitWorkdirFactory.class); } } From 04c5d6f84ac5aa69592d971b063c6f3fc25e52eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 7 Nov 2018 11:13:21 +0100 Subject: [PATCH 11/27] Fix deletion of temporary clone directory --- .../repository/spi/SimpleGitWorkdirFactory.java | 17 ++++++++++++----- .../spi/SimpleGitWorkdirFactoryTest.java | 2 +- 2 files changed, 13 insertions(+), 6 deletions(-) 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 c69acf5c6e..d62cd90bbc 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 @@ -11,13 +11,11 @@ import sonia.scm.repository.InternalRepositoryException; import java.io.File; import java.io.IOException; -import java.util.Random; public class SimpleGitWorkdirFactory implements GitWorkdirFactory { private static final Logger logger = LoggerFactory.getLogger(SimpleGitWorkdirFactory.class); - private final Random random = new Random(); private final File poolDirectory; public SimpleGitWorkdirFactory() { @@ -30,13 +28,22 @@ public class SimpleGitWorkdirFactory implements GitWorkdirFactory { public WorkingCopy createWorkingCopy(GitContext gitContext) { try { - Repository clone = cloneRepository(gitContext.getDirectory(), new File(poolDirectory, Long.toString(random.nextLong()))); + Repository clone = cloneRepository(gitContext.getDirectory(), createNewWorkdir()); return new WorkingCopy(clone, this::close); } catch (GitAPIException e) { throw new InternalRepositoryException("could not clone working copy of repository", e); + } catch (IOException e) { + throw new InternalRepositoryException("could not create temporary directory for copy of repository", e); } } + private File createNewWorkdir() throws IOException { + File workdir = File.createTempFile("workdir", "", poolDirectory); + workdir.delete(); + workdir.mkdir(); + return workdir; + } + protected Repository cloneRepository(File bareRepository, File target) throws GitAPIException { return Git.cloneRepository() .setURI(bareRepository.getAbsolutePath()) @@ -48,9 +55,9 @@ public class SimpleGitWorkdirFactory implements GitWorkdirFactory { private void close(Repository repository) { repository.close(); try { - FileUtils.delete(repository.getDirectory(), FileUtils.RECURSIVE); + FileUtils.delete(repository.getWorkTree(), FileUtils.RECURSIVE); } catch (IOException e) { - logger.warn("could not delete temporary git workdir '{}'", repository.getDirectory(), e); + logger.warn("could not delete temporary git workdir '{}'", repository.getWorkTree(), e); } } } 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 d9c41f85b5..0c39a1deb0 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 @@ -66,7 +66,7 @@ public class SimpleGitWorkdirFactoryTest extends AbstractGitCommandTestBase { File directory; try (WorkingCopy workingCopy = factory.createWorkingCopy(createContext())) { - directory = workingCopy.get().getDirectory(); + directory = workingCopy.get().getWorkTree(); } assertThat(directory).doesNotExist(); } From e377ce598826c66f2421589af3120cbc730c79f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 7 Nov 2018 11:52:49 +0100 Subject: [PATCH 12/27] Implement first steps for actual merge --- .../scm/repository/spi/GitMergeCommand.java | 50 ++++++++++++++---- .../repository/spi/GitMergeCommandTest.java | 43 ++++++++++++++- .../scm/repository/spi/scm-git-spi-test.zip | Bin 20891 -> 23641 bytes 3 files changed, 81 insertions(+), 12 deletions(-) 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 6e69a2c5ba..a4b416420d 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,8 +1,11 @@ package sonia.scm.repository.spi; +import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.merge.MergeStrategy; import org.eclipse.jgit.merge.ResolveMerger; import org.eclipse.jgit.lib.Repository; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import sonia.scm.repository.GitWorkdirFactory; import sonia.scm.repository.InternalRepositoryException; import sonia.scm.repository.api.MergeCommandResult; @@ -12,23 +15,20 @@ import java.io.IOException; public class GitMergeCommand extends AbstractGitCommand implements MergeCommand { - private final GitWorkdirFactory workdirPool; + private static final Logger logger = LoggerFactory.getLogger(GitMergeCommand.class); - GitMergeCommand(GitContext context, sonia.scm.repository.Repository repository, GitWorkdirFactory workdirPool) { + private final GitWorkdirFactory workdirFactory; + + GitMergeCommand(GitContext context, sonia.scm.repository.Repository repository, GitWorkdirFactory workdirFactory) { super(context, repository); - this.workdirPool = workdirPool; + this.workdirFactory = workdirFactory; } @Override public MergeCommandResult merge(MergeCommandRequest request) { - try (WorkingCopy workingCopy = workdirPool.createWorkingCopy(context)) { + try (WorkingCopy workingCopy = workdirFactory.createWorkingCopy(context)) { Repository repository = workingCopy.get(); - ResolveMerger merger = (ResolveMerger) MergeStrategy.RECURSIVE.newMerger(repository); - boolean mergeResult = merger.merge(repository.resolve(request.getBranchToMerge()), repository.resolve(request.getTargetBranch())); - if (mergeResult) { - // TODO push and verify push was successful - } - return new MergeCommandResult(mergeResult); + return new MergeWorker(repository).merge(request); } catch (IOException e) { throw new InternalRepositoryException(e); } @@ -44,4 +44,34 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand throw new InternalRepositoryException(e); } } + + private static class MergeWorker { + + private final Repository clone; + private MergeWorker(Repository clone) { + this.clone = clone; + } + + private MergeCommandResult merge(MergeCommandRequest request) throws IOException { + ResolveMerger merger = (ResolveMerger) MergeStrategy.RECURSIVE.newMerger(clone); + boolean mergeResult = merger.merge( + resolveRevision(clone, request.getTargetBranch()), + resolveRevision(clone, request.getBranchToMerge()) + ); + if (mergeResult) { + logger.info("Merged branch {} into {}", request.getBranchToMerge(), request.getTargetBranch()); + // TODO commit, push and verify push was successful + } + return new MergeCommandResult(mergeResult); + } + + private ObjectId resolveRevision(Repository repository, String branchToMerge) throws IOException { + ObjectId resolved = repository.resolve(branchToMerge); + if (resolved == null) { + return repository.resolve("origin/" + branchToMerge); + } else { + return resolved; + } + } + } } 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 3c58916807..bd140c68cb 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 @@ -3,7 +3,22 @@ package sonia.scm.repository.spi; import org.junit.Assert; import org.junit.Test; +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; + public class GitMergeCommandTest extends AbstractGitCommandTestBase { + + @Test + public void shouldDetectMergeableBranches() { + GitMergeCommand command = createCommand(); + MergeCommandRequest request = new MergeCommandRequest(); + request.setBranchToMerge("mergeable"); + request.setTargetBranch("master"); + + boolean mergeable = command.dryRun(request).isMergeable(); + + assertThat(mergeable).isTrue(); + } + @Test public void shouldDetectNotMergeableBranches() { GitMergeCommand command = createCommand(); @@ -13,10 +28,34 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { boolean mergeable = command.dryRun(request).isMergeable(); - Assert.assertFalse(mergeable); + assertThat(mergeable).isFalse(); + } + + @Test + public void shouldMergeMergeableBranches() { + GitMergeCommand command = createCommand(); + MergeCommandRequest request = new MergeCommandRequest(); + request.setTargetBranch("master"); + request.setBranchToMerge("mergeable"); + + boolean mergeable = command.merge(request).isSuccess(); + + assertThat(mergeable).isTrue(); + } + + @Test + public void shouldNotMergeConflictingBranches() { + GitMergeCommand command = createCommand(); + MergeCommandRequest request = new MergeCommandRequest(); + request.setBranchToMerge("test-branch"); + request.setTargetBranch("master"); + + boolean mergeable = command.merge(request).isSuccess(); + + assertThat(mergeable).isFalse(); } private GitMergeCommand createCommand() { - return new GitMergeCommand(createContext(), repository, null); + return new GitMergeCommand(createContext(), repository, new SimpleGitWorkdirFactory()); } } diff --git a/scm-plugins/scm-git-plugin/src/test/resources/sonia/scm/repository/spi/scm-git-spi-test.zip b/scm-plugins/scm-git-plugin/src/test/resources/sonia/scm/repository/spi/scm-git-spi-test.zip index 3fbab0be38e115c7c9fe342e52ea2a4bdc4e6893..8f689e9664fc97179530c73aca4c969b1cfe962a 100644 GIT binary patch delta 7381 zcma)A2Uru?7M@H11w$1fbOb^jNFk(9rFTI@kWd6dC@MukAG)#u6_r&idslg)qH952 zJNAZcK}AH7wJj>jDvPXwE4r3Nd@~6!ff@W>_y)qAd(S!doc}-foV#fU<>4txTx2L! zNe#k2m77;HHpnVGN0LbJDXiWGl;C??#+IJ7VR!cdN7s;=nqJ zzf>2sVJx&8wW_8`xu^#H&jf}bHPot%tb$oi;uLYFL?*YA+@j;_u9s8UG^MZO>z11T zu>4wHpyiNm{?w+F2hWE8a}W`o+$0 z)|&suCGYl4y;IZ>|K>DLXYT9qQ!egXdzX_JQXP4$l+ksh;Iqp|`qH@L>R)R8X5Ex@ zjV)_4xz>7VqfP%d7k4u}7{UZNBYa)1!G!V)?eW+%%-!p@m;fcd9(Y9=KfZ$)3$R|r zN1HHyv{OZ$0;E(H$*YUhF)(;PhU^t@>=P=<3I^@v7|^eWki2r!U#tYwblpi_#p>>c zfjP`2c_o1jDd4IyoA?T-s%(%76Eed8_y{D{6bQP4KFTN#vU1WSg>q<-c?p8@|EA4N zPfkvTQ}1;K{`=is9jVFkX4m4%Rk4 zEQ;CkrI}Ax=LMEAv>zMy^fLdADy$U2rKNX1{ztd5WeVUsmoMGKM^HwKq(T5@+rMf1(;4SjpJy?Ourr_bj!o_+NRNT{m6 z*(#(@Z+Y6UraE_?a#TI7T5VZE-3LvF`^>RhAE(&XZ~K_n&)Im0(gO@u>C#T!*`52~ z>yzsCoc3SKd3NtFE3fqIPwl^T;mzv}r;V0G1JMqBW~WZJqsv$-Xr>t1$sL-W%N*h= z0YMNXBeu6E9r&2&YZJH(W}39>kG3i)Tn|h#*Pl4bdwC8LqihF-=8OpjD5?oFL5;Z= z5gte3iRu(7mc2AqEL?UN%@f`zJ`7NN%+AfoXU>(R=QA@UDQWr4Y{}dVNlNN$dD_4d z89@aEtzYet1GBXPW(n%vX=m2|^T1Ts-&@O$(XP^O;GlcfD9%WE^{>KH)1&gB!pE`h zMVf!Ji|c-qn*SXA*A|xspVjB;XV>_p-cPX1HPJSncP{bS?RUFfUcdQ$-ftgz{!REY zGby%ciL_KuBUq+y^{an(7Ba)=!MyOJF_}e%PkY5@XL$A@Kl%TnRv%-3Mk{RXe~}3* z*$XPTQ&M^V?DbnSySq$cxUA(|Ny;y~=a-zTy4Cwu6N`y1U`^5o6D_QX?g7&+>U8C~ z6ywmM)32^Nqj{7y4&<5YO0m>J;)J^Oi8*&!TBTc0?|Wl8_W0Wi9(#QJ;j%!#Sk{TF z*?!+g{unh~=dWb3;QaZ#c<(v({?D%mZ*JuOY23Q`eqxvdsA+tp78Y!v$+pcBA9K}G zTG6m%rS7`K$ctGyhcooE=z(b&EiVh)T%~5!LUHTUtnK&1Y?VdyF+ZMv{$p>(oX#`p z>9;o3+HF0_Oga)(rsKP;D7EB8^{%Ygn<)qd zJ@`(l`LX!)(<22pLiWFmGyUDqa^=~XYY(irR7w4NLS4j_c`4n?nBmgvZv_SW9lRWN z_FhbvR(1eZs@b8gBh%*V`5*aphx)atNj$eJ2E@E9m#&Kl@^uMcy|=*poX$>R1^u`M zSFNUF+xk1rl!=iEE3?Z#i47N?Ny%=h@S|CdbDV!{U%;NKkLy00wLM%Oc3-F;dMlvo zWnb^(*VolcJ@&@U&-~+c%_F~wCRy9ty7bs>7ryH~VR~qx^1;j0x}G=v6IyrmtBkqk zD#|w9b>QF`zl{m?p_gw*6-{lmQlh!%T%x?or1bavCWzZuX|soB0a?M_6;8+V@00{G zwu|NmmoJ5HoY)mJF+S(}iDqWc+NKM(TxdGmwS80C#f7Imsa_QP{KDu6Y%ERm$XSME z9I%RK_>F8q2YC!N@Ru!3+j-;(2gcfow0|F2Nq~I28c<Fja#o(M6%c9rPzBHK;wL>6SzAO@}E# z4h^@a1=)u*hBiHgp$##rA7m$FQn(zwrd+Z}yK%{oRn3hB40~G=%q7{2NL+i}UI;uK z$>fV1$&gg%7z+Yyj7=nbxkrIFA=Fl#Ir!yH-_BzKY$rvpQ30p1zV>b4T8WL>9 zp|B&;YiKyIXIMzs%v7N;O(J9o(#2^kE}zfg3k508Y_T&-kiz4pNK)7-E-oBrSx6LB zNr#Muph8Oa>ckK#Teq~R$-eu$IoH01Z3`NgSbSz>Qw5^Fu5ZOjsPpX$lz%=>cXs@qNK9WQv@xPyI^-rG+$ zeN^#H30p5ijcK49HH#3MHUaBkBO>AuNQ=2(kmcClvD2($a*foYWNbD~}`Qu=p%iI)@N3)uD)i z7Hel-wb}*us@T3yS$Enzx2onpe4w1&ux2A2D)Hg?etxI^Z(D2hqwvkAnyq&wol0)L zI-?Qdm)V>IM)uYqnL{JtM-hjNADtYbiF5k5ur9SBJ4L!|UZAIft+v`oXah3`viUza z+yiQSE%Y>_fi|9oJ|Tq;Wb1=L^5_bjX!_en${v{W6`iw*qXvqcI3$d}=w!kdq7mBu zydvBVjSlvxqG>K7DmZXPkiWN|%<~vVhz<2kz6)xl#dt!^4nbg!fK8J4j|$vKM!yOW zn&O242PSr zkCkMg6~7!Im}Mic6F|3zDJXW;fX~5Tt*ic+0qa168v^&i;G-KdG^B|RO;!RPZni^1 z4(k#TXjAFc`%Lh#72(&5`xMq!AQ)BCxk9 z$nZ27GPc&w6wo|1%(!Ndyx*J_jV^ujXk_rQd7mORr1^vMWH2Y6JUon8PmFD0@S_C{ zZc7V#?R|2XJb9lhlZtiF!|*4Q3T}JZP$}jRXf`*HqK2|G?+r)kpl&%Nu>)avSFH%i zsEGr6CZ{Yap1>4E@xXbKx#~2F0U6*>BqMXwxZ-Uqmh*5Uc9vvW4hR?>--AkVK7-0h zh8ArMXsAS#xr|_~j2N*I8a5Nrl5((`{1!lJ{SY|Q8ocr|9s-D_KLQ`H1|I%CLqk>G z2wZCe8olvZJ>CeFY72oLE3Pi;x0jDc;>XDsrIam5@Wt0i(V+%gaKIOr+#iB4HlxXO z&{lQ|dH|yG=xrnA*`Nxa*+JmEB^UhQjgv1Y7=fGY!RBCG&joa7yaRY0jO)=0L>Lf) z>){LJdO``fUmk)`{SgRcu(+TrWVoJ72nd^u>&cyrz$%Vl%Vb*FtewFT5lR7hJ$P1rph%~5ukbBAE9pIK%mH;3tR*65I+fY zq&Ndw5bl0v5H^w+gvV%g7y|2X!MQMeQX4uH%LVG;qr^zoh{$84>4MuQ#M%`JLbHdSMu=GjIu9=$h(%^=Bm&>_0IE}rhPL1^1%VAcLEIF}ZvxAZvD$M$ zwn7;}pcAl-M5ryE5cnCr&YBgeo1)?nAGIt`1A>JvGM82qlS%6?}xYwoBCAhJ!`Hd z;_%uy2=VN(barm8F4ujUZjO6F{xqF?p1#n1VnOy)ods=RCpI9M*EC+$cUt&esKc!p}s|I_l% z$~MgXQk*w4%zys0T-{tFca50;cqS1=t^Eu0va+&>i5Xv}6x)=jxhtP=mskBgVB^^{ zH-27JQqn3*8_rv5)^@_i->Ld@x@Gwv1OGkw_1^pmZ(}o+2PIF-($^{@+uVlj>^ty@ z;6lZQb8F#{%-%3DD>-_7QQLvODyx8|I18KO!H=3Iyxq~FuI5=-?r41_UH^}^y%nV; zVVMzTlkL(@tGRYo{+Hs1{{(J^0@V%%(#*jsbs&uLq`Wjm)YHQ{76_{m>Ih1(V8tCvH3+u=C7WUg;0{4KDJQF&~fH72g-tG~HQu>P6Yk zp*C^h3k@{^!7pM|E#sYbJevMfQ_Z=Cnj@_ret+q)a`Se?k(;S*2V9;V^1S8pT4jCh zwyL4Uw^Az37ZfhPf8yz$q0f$-auDVeb55!1LZx*XB{h*RMy+vpKT}Ysm=SGoi0Ef{ z_ISy>ml5}0J3PHVY5m2z#WP=Mo%J$$qMbWQe}pOn=)^|s2@o*Nr~ zx1=%%t#Jcjm7C+hUNUUDEK~GXBcaw!?2!BLPAWqZzPLrObYZGH?)Hjdb}(a*g!vf$ zHmDYA$pP>|%y*=2SOzRYz*k6IeX|wDjukv+GyT|0%cqL1$Ndb&GG|EgN`y*pFSz6> zWhr%zRLd6=U+Wb}Js`n{O@5(|mQURoJ?*^ok;9?{JMd9kz+P#8N0u0jB-bSf;E6Pp zzv%7P7xUXdKbY_B3+udWz&lvlD?+M9042ep{EG}9t_L^5yEt2zuCjtsi6u*{dnH=_ zSVpV^JP$*DkPwPh{aJK2sI)9O^-tq$pRQ8 z4`s=jUfuvQiMJiImGIDVfv`|#_D605jcZfaV#L%(@Czw|ec`qa)P=!#4mZO?_>7ed z3c~#1QiLDNlm3x%_-B}cCEJ4VAuRH_;iMyn5OG=dPe_ZDj%8Uh(-LU~<~}LF(-$E; zbkX*bZ2W0jF3k{^S<%ZPNofrh>D(T%5Vbw_D0^z8Q|8vR3Vgdmr(g@Yx}u!I{OL3E z_0vYDvnj)wD3Z9vftOK^eLA*eh>j%*I|8Q1l3hnrqe)^07dAypyN)Wb)G;%-8SB+` z)E`TgnL|i4t#}0=HT8k_-4s1zNTS{nM#s=t4`WC!vPV$rI0$@Vt-1gg#gG;)0}%RF zub)z+fJd>ykzK32DQMFs!`vB7L}2DH$5k$Nu=!C!vQA`g!B_9>vCpxaLedE z2u~!5u^v#6DD0}EQhc<<15PGJcO4ywCy7;q;Xynt^(CI<-WrU+U8<&>ZT>nW7@tBC z>0(%!LhDrGqYYwsm_j3;(vV`Ygm%*}$W#Ss47`g8BzLj|L6cMs-idVB)}u&qsAo5X zJQG6JqpwJYVH%Ps@q}3#+I(}I2(HCj%WI`HfSMYJ|1bA&NFuQ5>=_OkTw>vc;EF^I z8{=uW=8qr=7jM`%f)2k9A7y#NA0ue{IDF!PH$2zSQZ{2qVxSDd#?VsNlSm?iglp-v z)T?xo*hGTsXj%$m$sOf%907s7P}$i5kmVdIf8k9YWO&kcK_ZOQf%kb4UAe) zF)6PZta@jXT#XNcYoTgbl}RI*Cz0Y`yCE3&42;06K}{0L6(|rWWokH{L?cjPn%)|n`gtyPZ;p)de`87}H-Yb@a3Q{Y$tZEJdV z&J>e1#uNoS1qc)fBJF@KFv``4K!m#eW5-xA%xg?NmgFu9M9>m$4|B%S!8YRa_xQXa u!l}o_Hbjf(X%OUVUAl~|z(>_Va7^pr)@{a_nNt^W4E`C1+tVwU`s;r;+;4^e From d5da106038ca8e269e4c9b27aa54e08a54b764fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 7 Nov 2018 12:52:40 +0100 Subject: [PATCH 13/27] Implement merge using api of class Git --- .../scm/repository/spi/GitMergeCommand.java | 46 +++++++++++++------ 1 file changed, 31 insertions(+), 15 deletions(-) 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 a4b416420d..c9a6e007ab 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,9 +1,12 @@ package sonia.scm.repository.spi; +import org.eclipse.jgit.api.Git; +import org.eclipse.jgit.api.MergeResult; +import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.merge.MergeStrategy; import org.eclipse.jgit.merge.ResolveMerger; -import org.eclipse.jgit.lib.Repository; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.repository.GitWorkdirFactory; @@ -47,28 +50,41 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand private static class MergeWorker { - private final Repository clone; + private final Git clone; private MergeWorker(Repository clone) { - this.clone = clone; + this.clone = new Git(clone); } private MergeCommandResult merge(MergeCommandRequest request) throws IOException { - ResolveMerger merger = (ResolveMerger) MergeStrategy.RECURSIVE.newMerger(clone); - boolean mergeResult = merger.merge( - resolveRevision(clone, request.getTargetBranch()), - resolveRevision(clone, request.getBranchToMerge()) - ); - if (mergeResult) { - logger.info("Merged branch {} into {}", request.getBranchToMerge(), request.getTargetBranch()); - // TODO commit, push and verify push was successful + try { + clone.checkout().setName(request.getTargetBranch()).call(); + } catch (GitAPIException e) { + throw new InternalRepositoryException("could not checkout target branch " + request.getTargetBranch(), e); + } + MergeResult result; + try { + result = clone.merge().include(request.getBranchToMerge(), resolveRevision(request.getBranchToMerge())).setCommit(true).call(); + } catch (GitAPIException e) { + throw new InternalRepositoryException("could not merge branch " + request.getBranchToMerge() + " into " + request.getTargetBranch(), e); + } + if (result.getMergeStatus().isSuccessful()) { + logger.info("Merged branch {} into {}", request.getBranchToMerge(), request.getTargetBranch()); + try { + clone.push().call(); + } catch (GitAPIException e) { + throw new InternalRepositoryException("could not push merged branch " + request.getBranchToMerge() + " to origin", e); + } + return new MergeCommandResult(true); + } else { + logger.info("Could not merged branch {} into {}", request.getBranchToMerge(), request.getTargetBranch()); + return new MergeCommandResult(false); } - return new MergeCommandResult(mergeResult); } - private ObjectId resolveRevision(Repository repository, String branchToMerge) throws IOException { - ObjectId resolved = repository.resolve(branchToMerge); + private ObjectId resolveRevision(String branchToMerge) throws IOException { + ObjectId resolved = clone.getRepository().resolve(branchToMerge); if (resolved == null) { - return repository.resolve("origin/" + branchToMerge); + return clone.getRepository().resolve("origin/" + branchToMerge); } else { return resolved; } From 26d5e6e062232ccb5f5b761400850403deb8af37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 7 Nov 2018 13:17:21 +0100 Subject: [PATCH 14/27] Set merge message --- .../scm/repository/spi/GitMergeCommand.java | 24 +++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) 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 c9a6e007ab..c2b903e57f 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 @@ -19,6 +19,10 @@ import java.io.IOException; public class GitMergeCommand extends AbstractGitCommand implements MergeCommand { private static final Logger logger = LoggerFactory.getLogger(GitMergeCommand.class); + public static final String MERGE_COMMIT_MESSAGE_TEMPLATE = + "Merge of branch %s into %s\n" + + "\n" + + "Automatic merge by SCM-Manager."; private final GitWorkdirFactory workdirFactory; @@ -56,27 +60,33 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand } private MergeCommandResult merge(MergeCommandRequest request) throws IOException { + String target = request.getTargetBranch(); + String toMerge = request.getBranchToMerge(); try { - clone.checkout().setName(request.getTargetBranch()).call(); + clone.checkout().setName(target).call(); } catch (GitAPIException e) { - throw new InternalRepositoryException("could not checkout target branch " + request.getTargetBranch(), e); + throw new InternalRepositoryException("could not checkout target branch " + target, e); } MergeResult result; try { - result = clone.merge().include(request.getBranchToMerge(), resolveRevision(request.getBranchToMerge())).setCommit(true).call(); + result = clone.merge() + .setCommit(true) + .setMessage(String.format(MERGE_COMMIT_MESSAGE_TEMPLATE, toMerge, target)) + .include(toMerge, resolveRevision(toMerge)) + .call(); } catch (GitAPIException e) { - throw new InternalRepositoryException("could not merge branch " + request.getBranchToMerge() + " into " + request.getTargetBranch(), e); + throw new InternalRepositoryException("could not merge branch " + toMerge + " into " + target, e); } if (result.getMergeStatus().isSuccessful()) { - logger.info("Merged branch {} into {}", request.getBranchToMerge(), request.getTargetBranch()); + logger.info("merged branch {} into {}", toMerge, target); try { clone.push().call(); } catch (GitAPIException e) { - throw new InternalRepositoryException("could not push merged branch " + request.getBranchToMerge() + " to origin", e); + throw new InternalRepositoryException("could not push merged branch " + toMerge + " to origin", e); } return new MergeCommandResult(true); } else { - logger.info("Could not merged branch {} into {}", request.getBranchToMerge(), request.getTargetBranch()); + logger.info("could not merged branch {} into {} due to {}", toMerge, target, result.getConflicts().keySet()); return new MergeCommandResult(false); } } From 2d04e6c2f0f1314ca21546d8ee382a9a9b20de5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 7 Nov 2018 13:34:35 +0100 Subject: [PATCH 15/27] Specify not mergeable files in merge result --- .../repository/api/MergeCommandResult.java | 28 +++++++++++++++---- .../scm/repository/spi/GitMergeCommand.java | 16 ++++++----- .../repository/spi/GitMergeCommandTest.java | 15 ++++++---- 3 files changed, 41 insertions(+), 18 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandResult.java b/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandResult.java index 024e7bbc89..c5a4a57d42 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandResult.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandResult.java @@ -1,13 +1,31 @@ package sonia.scm.repository.api; -public class MergeCommandResult { - private final boolean success; +import java.util.Collection; +import java.util.HashSet; - public MergeCommandResult(boolean success) { - this.success = success; +import static java.util.Collections.emptyList; +import static java.util.Collections.unmodifiableCollection; + +public class MergeCommandResult { + private final Collection filesWithConflict; + + private MergeCommandResult(Collection filesWithConflict) { + this.filesWithConflict = filesWithConflict; + } + + public static MergeCommandResult success() { + return new MergeCommandResult(emptyList()); + } + + public static MergeCommandResult failure(Collection filesWithConflict) { + return new MergeCommandResult(new HashSet<>(filesWithConflict)); } public boolean isSuccess() { - return success; + return filesWithConflict.isEmpty(); + } + + public Collection getFilesWithConflict() { + return unmodifiableCollection(filesWithConflict); } } 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 c2b903e57f..800317ef67 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 @@ -35,9 +35,10 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand public MergeCommandResult merge(MergeCommandRequest request) { try (WorkingCopy workingCopy = workdirFactory.createWorkingCopy(context)) { Repository repository = workingCopy.get(); + logger.debug("cloned repository to folder {}", repository.getWorkTree()); return new MergeWorker(repository).merge(request); } catch (IOException e) { - throw new InternalRepositoryException(e); + throw new InternalRepositoryException("could not clone repository for merge", e); } } @@ -48,7 +49,7 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand ResolveMerger merger = (ResolveMerger) MergeStrategy.RECURSIVE.newMerger(repository, true); return new MergeDryRunCommandResult(merger.merge(repository.resolve(request.getBranchToMerge()), repository.resolve(request.getTargetBranch()))); } catch (IOException e) { - throw new InternalRepositoryException(e); + throw new InternalRepositoryException("could not clone repository for merge", e); } } @@ -65,7 +66,7 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand try { clone.checkout().setName(target).call(); } catch (GitAPIException e) { - throw new InternalRepositoryException("could not checkout target branch " + target, e); + throw new InternalRepositoryException("could not checkout target branch for merge: " + target, e); } MergeResult result; try { @@ -78,16 +79,17 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand throw new InternalRepositoryException("could not merge branch " + toMerge + " into " + target, e); } if (result.getMergeStatus().isSuccessful()) { - logger.info("merged branch {} into {}", toMerge, target); + logger.debug("merged branch {} into {}", toMerge, target); try { clone.push().call(); } catch (GitAPIException e) { throw new InternalRepositoryException("could not push merged branch " + toMerge + " to origin", e); } - return new MergeCommandResult(true); + logger.debug("pushed merged branch {}", target); + return MergeCommandResult.success(); } else { - logger.info("could not merged branch {} into {} due to {}", toMerge, target, result.getConflicts().keySet()); - return new MergeCommandResult(false); + logger.info("could not merged branch {} into {} due to conflict in paths {}", toMerge, target, result.getConflicts().keySet()); + return MergeCommandResult.failure(result.getConflicts().keySet()); } } 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 bd140c68cb..c6949a3a11 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 @@ -1,9 +1,11 @@ package sonia.scm.repository.spi; -import org.junit.Assert; +import org.assertj.core.api.Assertions; import org.junit.Test; +import sonia.scm.repository.api.MergeCommandResult; + +import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.AssertionsForClassTypes.assertThat; public class GitMergeCommandTest extends AbstractGitCommandTestBase { @@ -38,9 +40,9 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { request.setTargetBranch("master"); request.setBranchToMerge("mergeable"); - boolean mergeable = command.merge(request).isSuccess(); + MergeCommandResult mergeCommandResult = command.merge(request); - assertThat(mergeable).isTrue(); + assertThat(mergeCommandResult.isSuccess()).isTrue(); } @Test @@ -50,9 +52,10 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { request.setBranchToMerge("test-branch"); request.setTargetBranch("master"); - boolean mergeable = command.merge(request).isSuccess(); + MergeCommandResult mergeCommandResult = command.merge(request); - assertThat(mergeable).isFalse(); + assertThat(mergeCommandResult.isSuccess()).isFalse(); + assertThat(mergeCommandResult.getFilesWithConflict()).containsExactly("a.txt"); } private GitMergeCommand createCommand() { From 8771f68081b7bfdbee0b3c98f26c8f4ccbd0b399 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 7 Nov 2018 14:02:02 +0100 Subject: [PATCH 16/27] Set author for merge --- .../repository/api/MergeCommandBuilder.java | 6 ++++++ .../repository/spi/MergeCommandRequest.java | 21 ++++++++++++++++--- .../scm/repository/spi/GitMergeCommand.java | 11 ++++++++-- .../repository/spi/GitMergeCommandTest.java | 17 ++++++++++++++- 4 files changed, 49 insertions(+), 6 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java index 65eed2f2a1..00acb66874 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java @@ -1,6 +1,7 @@ package sonia.scm.repository.api; import com.google.common.base.Preconditions; +import sonia.scm.repository.Person; import sonia.scm.repository.spi.MergeCommand; import sonia.scm.repository.spi.MergeCommandRequest; @@ -23,6 +24,11 @@ public class MergeCommandBuilder { return this; } + public MergeCommandBuilder setAuthor(Person author) { + request.setAuthor(author); + return this; + } + public MergeCommandBuilder reset() { request.reset(); return this; diff --git a/scm-core/src/main/java/sonia/scm/repository/spi/MergeCommandRequest.java b/scm-core/src/main/java/sonia/scm/repository/spi/MergeCommandRequest.java index e01e580afa..a4e22b5ad1 100644 --- a/scm-core/src/main/java/sonia/scm/repository/spi/MergeCommandRequest.java +++ b/scm-core/src/main/java/sonia/scm/repository/spi/MergeCommandRequest.java @@ -4,6 +4,8 @@ import com.google.common.base.MoreObjects; import com.google.common.base.Objects; import com.google.common.base.Strings; import sonia.scm.Validateable; +import sonia.scm.repository.Person; +import sonia.scm.util.Util; import java.io.Serializable; @@ -13,6 +15,7 @@ public class MergeCommandRequest implements Validateable, Resetable, Serializabl private String branchToMerge; private String targetBranch; + private Person author; public String getBranchToMerge() { return branchToMerge; @@ -30,8 +33,18 @@ public class MergeCommandRequest implements Validateable, Resetable, Serializabl this.targetBranch = targetBranch; } + public Person getAuthor() { + return author; + } + + public void setAuthor(Person author) { + this.author = author; + } + public boolean isValid() { - return !Strings.isNullOrEmpty(getBranchToMerge()) && !Strings.isNullOrEmpty(getTargetBranch()); + return !Strings.isNullOrEmpty(getBranchToMerge()) + && !Strings.isNullOrEmpty(getTargetBranch()) + && getAuthor() != null; } public void reset() { @@ -52,12 +65,13 @@ public class MergeCommandRequest implements Validateable, Resetable, Serializabl final MergeCommandRequest other = (MergeCommandRequest) obj; return Objects.equal(branchToMerge, other.branchToMerge) - && Objects.equal(targetBranch, other.targetBranch); + && Objects.equal(targetBranch, other.targetBranch) + && Objects.equal(author, other.author); } @Override public int hashCode() { - return Objects.hashCode(branchToMerge, targetBranch); + return Objects.hashCode(branchToMerge, targetBranch, author); } @Override @@ -65,6 +79,7 @@ public class MergeCommandRequest implements Validateable, Resetable, Serializabl return MoreObjects.toStringHelper(this) .add("branchToMerge", branchToMerge) .add("targetBranch", targetBranch) + .add("author", author) .toString(); } } 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 800317ef67..8ed3e4961c 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 @@ -71,8 +71,7 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand MergeResult result; try { result = clone.merge() - .setCommit(true) - .setMessage(String.format(MERGE_COMMIT_MESSAGE_TEMPLATE, toMerge, target)) + .setCommit(false) // we want to set the author manually .include(toMerge, resolveRevision(toMerge)) .call(); } catch (GitAPIException e) { @@ -80,6 +79,14 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand } if (result.getMergeStatus().isSuccessful()) { logger.debug("merged branch {} into {}", toMerge, target); + try { + clone.commit() + .setAuthor(request.getAuthor().getName(), request.getAuthor().getMail()) + .setMessage(String.format(MERGE_COMMIT_MESSAGE_TEMPLATE, toMerge, target)) + .call(); + } catch (GitAPIException e) { + throw new InternalRepositoryException("could not commit merge between branch " + toMerge + " and " + target, e); + } try { clone.push().call(); } catch (GitAPIException e) { 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 c6949a3a11..23b41bf54a 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 @@ -1,9 +1,17 @@ package sonia.scm.repository.spi; import org.assertj.core.api.Assertions; +import org.eclipse.jgit.api.Git; +import org.eclipse.jgit.api.errors.GitAPIException; +import org.eclipse.jgit.lib.PersonIdent; +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.revwalk.RevCommit; import org.junit.Test; +import sonia.scm.repository.Person; import sonia.scm.repository.api.MergeCommandResult; +import java.io.IOException; + import static org.assertj.core.api.Assertions.assertThat; @@ -34,15 +42,22 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { } @Test - public void shouldMergeMergeableBranches() { + public void shouldMergeMergeableBranches() throws IOException, GitAPIException { GitMergeCommand command = createCommand(); MergeCommandRequest request = new MergeCommandRequest(); request.setTargetBranch("master"); request.setBranchToMerge("mergeable"); + request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); MergeCommandResult mergeCommandResult = command.merge(request); assertThat(mergeCommandResult.isSuccess()).isTrue(); + + Repository repository = createContext().open(); + Iterable mergeCommit = new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call(); + PersonIdent mergeAuthor = mergeCommit.iterator().next().getAuthorIdent(); + assertThat(mergeAuthor.getName()).isEqualTo("Dirk Gently"); + assertThat(mergeAuthor.getEmailAddress()).isEqualTo("dirk@holistic.det"); } @Test From 2f91531d057493c91e9580dfab0fe3297aac30bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 7 Nov 2018 14:04:00 +0100 Subject: [PATCH 17/27] Assert existence of pool directory --- .../java/sonia/scm/repository/spi/SimpleGitWorkdirFactory.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 d62cd90bbc..f7e409e3bd 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 @@ -24,6 +24,7 @@ public class SimpleGitWorkdirFactory implements GitWorkdirFactory { public SimpleGitWorkdirFactory(File poolDirectory) { this.poolDirectory = poolDirectory; + poolDirectory.mkdirs(); } public WorkingCopy createWorkingCopy(GitContext gitContext) { @@ -33,7 +34,7 @@ public class SimpleGitWorkdirFactory implements GitWorkdirFactory { } catch (GitAPIException e) { throw new InternalRepositoryException("could not clone working copy of repository", e); } catch (IOException e) { - throw new InternalRepositoryException("could not create temporary directory for copy of repository", e); + throw new InternalRepositoryException("could not create temporary directory for clone of repository", e); } } From 0714d5f96036133133ed81833c2498dc3e204273 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Thu, 8 Nov 2018 09:32:43 +0100 Subject: [PATCH 18/27] Extract methods to emphasize flow --- .../scm/repository/spi/GitMergeCommand.java | 30 +++++++++++++++---- 1 file changed, 24 insertions(+), 6 deletions(-) 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 8ed3e4961c..e97b9a85e3 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 @@ -11,6 +11,7 @@ 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.api.MergeCommandResult; import sonia.scm.repository.api.MergeDryRunCommandResult; @@ -36,7 +37,7 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand try (WorkingCopy workingCopy = workdirFactory.createWorkingCopy(context)) { Repository repository = workingCopy.get(); logger.debug("cloned repository to folder {}", repository.getWorkTree()); - return new MergeWorker(repository).merge(request); + return new MergeWorker(repository, request).merge(); } catch (IOException e) { throw new InternalRepositoryException("could not clone repository for merge", e); } @@ -55,19 +56,32 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand private static class MergeWorker { + private final String target; + private final String toMerge; + private final Person author; private final Git clone; - private MergeWorker(Repository clone) { + private MergeWorker(Repository clone, MergeCommandRequest request) { + this.target = request.getTargetBranch(); + this.toMerge = request.getBranchToMerge(); + this.author = request.getAuthor(); this.clone = new Git(clone); } - private MergeCommandResult merge(MergeCommandRequest request) throws IOException { - String target = request.getTargetBranch(); - String toMerge = request.getBranchToMerge(); + private MergeCommandResult merge() throws IOException { + createClone(); + MergeResult result = doMergeInClone(); + return analyzeResult(result); + } + + private void createClone() { try { clone.checkout().setName(target).call(); } catch (GitAPIException e) { throw new InternalRepositoryException("could not checkout target branch for merge: " + target, e); } + } + + private MergeResult doMergeInClone() throws IOException { MergeResult result; try { result = clone.merge() @@ -77,11 +91,15 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand } catch (GitAPIException e) { throw new InternalRepositoryException("could not merge branch " + toMerge + " into " + target, e); } + return result; + } + + private MergeCommandResult analyzeResult(MergeResult result) { if (result.getMergeStatus().isSuccessful()) { logger.debug("merged branch {} into {}", toMerge, target); try { clone.commit() - .setAuthor(request.getAuthor().getName(), request.getAuthor().getMail()) + .setAuthor(author.getName(), author.getMail()) .setMessage(String.format(MERGE_COMMIT_MESSAGE_TEMPLATE, toMerge, target)) .call(); } catch (GitAPIException e) { From 5ca8d4d0491b3c5d8cad4a0068f14e4d93975fd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Thu, 8 Nov 2018 09:53:35 +0100 Subject: [PATCH 19/27] Extract methods to emphasize flow --- .../scm/repository/spi/GitMergeCommand.java | 53 +++++++++++-------- 1 file changed, 31 insertions(+), 22 deletions(-) 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 e97b9a85e3..50b9636dc2 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 @@ -60,6 +60,7 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand private final String toMerge; private final Person author; private final Git clone; + private MergeWorker(Repository clone, MergeCommandRequest request) { this.target = request.getTargetBranch(); this.toMerge = request.getBranchToMerge(); @@ -70,7 +71,13 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand private MergeCommandResult merge() throws IOException { createClone(); MergeResult result = doMergeInClone(); - return analyzeResult(result); + if (result.getMergeStatus().isSuccessful()) { + doCommit(); + push(); + return MergeCommandResult.success(); + } else { + return analyseFailure(result); + } } private void createClone() { @@ -94,30 +101,32 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand return result; } - private MergeCommandResult analyzeResult(MergeResult result) { - if (result.getMergeStatus().isSuccessful()) { - logger.debug("merged branch {} into {}", toMerge, target); - try { - clone.commit() - .setAuthor(author.getName(), author.getMail()) - .setMessage(String.format(MERGE_COMMIT_MESSAGE_TEMPLATE, toMerge, target)) - .call(); - } catch (GitAPIException e) { - throw new InternalRepositoryException("could not commit merge between branch " + toMerge + " and " + target, e); - } - try { - clone.push().call(); - } catch (GitAPIException e) { - throw new InternalRepositoryException("could not push merged branch " + toMerge + " to origin", e); - } - logger.debug("pushed merged branch {}", target); - return MergeCommandResult.success(); - } else { - logger.info("could not merged branch {} into {} due to conflict in paths {}", toMerge, target, result.getConflicts().keySet()); - return MergeCommandResult.failure(result.getConflicts().keySet()); + private void doCommit() { + logger.debug("merged branch {} into {}", toMerge, target); + try { + clone.commit() + .setAuthor(author.getName(), author.getMail()) + .setMessage(String.format(MERGE_COMMIT_MESSAGE_TEMPLATE, toMerge, target)) + .call(); + } catch (GitAPIException e) { + throw new InternalRepositoryException("could not commit merge between branch " + toMerge + " and " + target, e); } } + private void push() { + try { + clone.push().call(); + } catch (GitAPIException e) { + throw new InternalRepositoryException("could not push merged branch " + toMerge + " to origin", 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 branchToMerge) throws IOException { ObjectId resolved = clone.getRepository().resolve(branchToMerge); if (resolved == null) { From 6f7aa24700f55936d51a7bc0775cb16b597aec23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Thu, 8 Nov 2018 10:00:21 +0100 Subject: [PATCH 20/27] Use jav nio to create temp directory --- .../sonia/scm/repository/spi/SimpleGitWorkdirFactory.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) 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 f7e409e3bd..a8a54d3c0c 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 @@ -11,6 +11,7 @@ import sonia.scm.repository.InternalRepositoryException; import java.io.File; import java.io.IOException; +import java.nio.file.Files; public class SimpleGitWorkdirFactory implements GitWorkdirFactory { @@ -39,10 +40,7 @@ public class SimpleGitWorkdirFactory implements GitWorkdirFactory { } private File createNewWorkdir() throws IOException { - File workdir = File.createTempFile("workdir", "", poolDirectory); - workdir.delete(); - workdir.mkdir(); - return workdir; + return Files.createTempDirectory(poolDirectory.toPath(),"workdir").toFile(); } protected Repository cloneRepository(File bareRepository, File target) throws GitAPIException { From 720862296ef66768015f4b2459abb75e6fb7643f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Thu, 8 Nov 2018 13:30:05 +0100 Subject: [PATCH 21/27] Use logged in user for author as default --- .../repository/spi/MergeCommandRequest.java | 3 +- .../scm/repository/spi/GitMergeCommand.java | 19 +++++++++- .../repository/spi/GitMergeCommandTest.java | 37 ++++++++++++++++++- 3 files changed, 54 insertions(+), 5 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/repository/spi/MergeCommandRequest.java b/scm-core/src/main/java/sonia/scm/repository/spi/MergeCommandRequest.java index a4e22b5ad1..05f160ec0a 100644 --- a/scm-core/src/main/java/sonia/scm/repository/spi/MergeCommandRequest.java +++ b/scm-core/src/main/java/sonia/scm/repository/spi/MergeCommandRequest.java @@ -43,8 +43,7 @@ public class MergeCommandRequest implements Validateable, Resetable, Serializabl public boolean isValid() { return !Strings.isNullOrEmpty(getBranchToMerge()) - && !Strings.isNullOrEmpty(getTargetBranch()) - && getAuthor() != null; + && !Strings.isNullOrEmpty(getTargetBranch()); } public void reset() { 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 50b9636dc2..a060c0fecc 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,5 +1,7 @@ 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.MergeResult; import org.eclipse.jgit.api.errors.GitAPIException; @@ -14,6 +16,7 @@ import sonia.scm.repository.InternalRepositoryException; import sonia.scm.repository.Person; import sonia.scm.repository.api.MergeCommandResult; import sonia.scm.repository.api.MergeDryRunCommandResult; +import sonia.scm.user.User; import java.io.IOException; @@ -103,9 +106,10 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand private void doCommit() { logger.debug("merged branch {} into {}", toMerge, target); + Person authorToUse = determineAuthor(); try { clone.commit() - .setAuthor(author.getName(), author.getMail()) + .setAuthor(authorToUse.getName(), authorToUse.getMail()) .setMessage(String.format(MERGE_COMMIT_MESSAGE_TEMPLATE, toMerge, target)) .call(); } catch (GitAPIException e) { @@ -113,6 +117,19 @@ 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(); 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 23b41bf54a..307416011e 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 @@ -1,22 +1,32 @@ package sonia.scm.repository.spi; -import org.assertj.core.api.Assertions; +import com.github.sdorra.shiro.ShiroRule; +import com.github.sdorra.shiro.SubjectAware; +import org.apache.shiro.subject.SimplePrincipalCollection; +import org.apache.shiro.subject.Subject; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; +import org.junit.Rule; import org.junit.Test; import sonia.scm.repository.Person; import sonia.scm.repository.api.MergeCommandResult; +import sonia.scm.user.User; import java.io.IOException; import static org.assertj.core.api.Assertions.assertThat; - +@SubjectAware(configuration = "classpath:sonia/scm/configuration/shiro.ini") public class GitMergeCommandTest extends AbstractGitCommandTestBase { + private static final String REALM = "AdminRealm"; + + @Rule + public ShiroRule shiro = new ShiroRule(); + @Test public void shouldDetectMergeableBranches() { GitMergeCommand command = createCommand(); @@ -73,6 +83,29 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { assertThat(mergeCommandResult.getFilesWithConflict()).containsExactly("a.txt"); } + @Test + @SubjectAware(username = "admin", password = "secret") + public void shouldTakeAuthorFromSubjectIfNotSet() throws IOException, GitAPIException { + shiro.setSubject( + new Subject.Builder() + .principals(new SimplePrincipalCollection(new User("dirk", "Dirk Gently", "dirk@holistic.det"), REALM)) + .buildSubject()); + GitMergeCommand command = createCommand(); + MergeCommandRequest request = new MergeCommandRequest(); + request.setTargetBranch("master"); + request.setBranchToMerge("mergeable"); + + MergeCommandResult mergeCommandResult = command.merge(request); + + assertThat(mergeCommandResult.isSuccess()).isTrue(); + + Repository repository = createContext().open(); + Iterable mergeCommit = new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call(); + PersonIdent mergeAuthor = mergeCommit.iterator().next().getAuthorIdent(); + assertThat(mergeAuthor.getName()).isEqualTo("Dirk Gently"); + assertThat(mergeAuthor.getEmailAddress()).isEqualTo("dirk@holistic.det"); + } + private GitMergeCommand createCommand() { return new GitMergeCommand(createContext(), repository, new SimpleGitWorkdirFactory()); } From 42f0632a23305b8e707056af652ae177cad9f118 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Thu, 8 Nov 2018 14:03:09 +0100 Subject: [PATCH 22/27] Add check for merged file to unit test --- .../sonia/scm/repository/spi/GitMergeCommandTest.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) 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 307416011e..5bb9af7d8f 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 @@ -64,10 +64,15 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { assertThat(mergeCommandResult.isSuccess()).isTrue(); Repository repository = createContext().open(); - Iterable mergeCommit = new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call(); - PersonIdent mergeAuthor = mergeCommit.iterator().next().getAuthorIdent(); + Iterable commits = new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call(); + RevCommit mergeCommit = commits.iterator().next(); + PersonIdent mergeAuthor = mergeCommit.getAuthorIdent(); assertThat(mergeAuthor.getName()).isEqualTo("Dirk Gently"); assertThat(mergeAuthor.getEmailAddress()).isEqualTo("dirk@holistic.det"); + // We expect the merge result of file b.txt here by looking up the sha hash of its content. + // If the file is missing (aka not merged correctly) this will throw a MissingObjectException: + byte[] contentOfFileB = repository.open(repository.resolve("9513e9c76e73f3e562fd8e4c909d0607113c77c6")).getBytes(); + assertThat(new String(contentOfFileB)).isEqualTo("b\ncontent from branch\n"); } @Test From 3c70ca7aa5d001e78c4a3ad737b4a66d97b96818 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Thu, 8 Nov 2018 14:35:35 +0100 Subject: [PATCH 23/27] Add JavaDoc for API --- .../repository/api/MergeCommandBuilder.java | 80 ++++++++++++++++++- .../repository/api/MergeCommandResult.java | 13 +++ .../api/MergeDryRunCommandResult.java | 8 ++ 3 files changed, 100 insertions(+), 1 deletion(-) diff --git a/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java index 00acb66874..3823b2b3a7 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java @@ -5,40 +5,118 @@ import sonia.scm.repository.Person; import sonia.scm.repository.spi.MergeCommand; import sonia.scm.repository.spi.MergeCommandRequest; +/** + * Use this {@link MergeCommandBuilder} to merge two branches of a repository ({@link #executeMerge()}) or to check if + * the branches could be merged without conflicts ({@link #dryRun()}). To do so, you have to specify the name of + * the target branch ({@link #setTargetBranch(String)}) and the name of the branch that should be merged + * ({@link #setBranchToMerge(String)}). Additionally you can specify an author that should be used for the commit + * ({@link #setAuthor(Person)}) if you are not doing a dry run only. If no author is specified, the logged in user + * will be used instead. + * + * To actually merge feature_branch into integration_branch do this: + * 

+ *     repositoryService.gerMergeCommand()
+ *       .setBranchToMerge("feature_branch")
+ *       .setTargetBranch("integration_branch")
+ *       .executeMerge();
+ *
+ * + * If the merge is successful, the result will look like this: + * 

+ *                            O    <- Merge result (new head of integration_branch)
+ *                            |\
+ *                            | \
+ *  old integration_branch -> O  O <- feature_branch
+ *                            |  |
+ *                            O  O
+ *
+ * + * To check whether they can be merged without conflicts beforehand do this: + * 

+ *     repositoryService.gerMergeCommand()
+ *       .setBranchToMerge("feature_branch")
+ *       .setTargetBranch("integration_branch")
+ *       .dryRun()
+ *       .isMergeable();
+ *
+ * + * Keep in mind that you should always check the result of a merge even though you may have done a dry run + * beforehand, because the branches can change between the dry run and the actual merge. + * + * @since 2.0.0 + */ public class MergeCommandBuilder { private final MergeCommand mergeCommand; private final MergeCommandRequest request = new MergeCommandRequest(); - public MergeCommandBuilder(MergeCommand mergeCommand) { + MergeCommandBuilder(MergeCommand mergeCommand) { this.mergeCommand = mergeCommand; } + /** + * Use this to set the branch that should be merged into the target branch. + * + * This is mandatory. + * + * @return This builder instance. + */ public MergeCommandBuilder setBranchToMerge(String branchToMerge) { request.setBranchToMerge(branchToMerge); return this; } + /** + * Use this to set the target branch the other branch should be merged into. + * + * This is mandatory. + * + * @return This builder instance. + */ public MergeCommandBuilder setTargetBranch(String targetBranch) { request.setTargetBranch(targetBranch); return this; } + /** + * Use this to set the author of the merge commit manually. If this is omitted, the currently logged in user will be + * used instead. + * + * This is optional and for {@link #executeMerge()} only. + * + * @return This builder instance. + */ public MergeCommandBuilder setAuthor(Person author) { request.setAuthor(author); return this; } + /** + * Use this to reset the command. + * @return This builder instance. + */ public MergeCommandBuilder reset() { request.reset(); return this; } + /** + * Use this to actually do the merge. If an automatic merge is not possible, {@link MergeCommandResult#isSuccess()} + * will return false. + * + * @return The result of the merge. + */ public MergeCommandResult executeMerge() { Preconditions.checkArgument(request.isValid(), "revision to merge and target revision is required"); return mergeCommand.merge(request); } + /** + * Use this to check whether the given branches can be merged autmatically. If this is possible, + * {@link MergeDryRunCommandResult#isMergeable()} will return true. + * + * @return The result whether the given branches can be merged automatically. + */ public MergeDryRunCommandResult dryRun() { Preconditions.checkArgument(request.isValid(), "revision to merge and target revision is required"); return mergeCommand.dryRun(request); diff --git a/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandResult.java b/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandResult.java index c5a4a57d42..53f712cddc 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandResult.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandResult.java @@ -6,6 +6,11 @@ import java.util.HashSet; import static java.util.Collections.emptyList; import static java.util.Collections.unmodifiableCollection; +/** + * This class keeps the result of a merge of branches. Use {@link #isSuccess()} to check whether the merge was + * sucessfully executed. If the result is false the merge could not be done without conflicts. In this + * case you can use {@link #getFilesWithConflict()} to get a list of files with merge conflicts. + */ public class MergeCommandResult { private final Collection filesWithConflict; @@ -21,10 +26,18 @@ public class MergeCommandResult { return new MergeCommandResult(new HashSet<>(filesWithConflict)); } + /** + * If this returns true, the merge was successfull. If this returns false there were + * merge conflicts. In this case you can use {@link #getFilesWithConflict()} to check what files could not be merged. + */ public boolean isSuccess() { return filesWithConflict.isEmpty(); } + /** + * If the merge was not successful ({@link #isSuccess()} returns false) this will give you a list of + * file paths that could not be merged automatically. + */ public Collection getFilesWithConflict() { return unmodifiableCollection(filesWithConflict); } diff --git a/scm-core/src/main/java/sonia/scm/repository/api/MergeDryRunCommandResult.java b/scm-core/src/main/java/sonia/scm/repository/api/MergeDryRunCommandResult.java index aeb18a753d..6ebb330aae 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/MergeDryRunCommandResult.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/MergeDryRunCommandResult.java @@ -1,5 +1,9 @@ package sonia.scm.repository.api; +/** + * This class keeps the result of a merge dry run. Use {@link #isMergeable()} to check whether an automatic merge is + * possible or not. + */ public class MergeDryRunCommandResult { private final boolean mergeable; @@ -8,6 +12,10 @@ public class MergeDryRunCommandResult { this.mergeable = mergeable; } + /** + * This will return true, when an automatic merge is possible at the moment; false + * otherwise. + */ public boolean isMergeable() { return mergeable; } From 8d057f774506fc98b225580f7cfd74e9cef008cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Thu, 8 Nov 2018 14:37:18 +0100 Subject: [PATCH 24/27] Hide details --- .../main/java/sonia/scm/repository/spi/GitMergeCommand.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 a060c0fecc..6376262228 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 @@ -23,7 +23,8 @@ import java.io.IOException; public class GitMergeCommand extends AbstractGitCommand implements MergeCommand { private static final Logger logger = LoggerFactory.getLogger(GitMergeCommand.class); - public static final String MERGE_COMMIT_MESSAGE_TEMPLATE = + + private static final String MERGE_COMMIT_MESSAGE_TEMPLATE = "Merge of branch %s into %s\n" + "\n" + "Automatic merge by SCM-Manager."; From 97d158ab3515efe77b74efac9405f98537b075c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Thu, 8 Nov 2018 14:39:30 +0100 Subject: [PATCH 25/27] Naming --- .../main/java/sonia/scm/repository/spi/GitMergeCommand.java | 4 ++-- .../test/java/sonia/scm/repository/CloseableWrapperTest.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) 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 6376262228..f61038b069 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 @@ -73,7 +73,7 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand } private MergeCommandResult merge() throws IOException { - createClone(); + checkOutTargetBranch(); MergeResult result = doMergeInClone(); if (result.getMergeStatus().isSuccessful()) { doCommit(); @@ -84,7 +84,7 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand } } - private void createClone() { + private void checkOutTargetBranch() { try { clone.checkout().setName(target).call(); } catch (GitAPIException e) { diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/CloseableWrapperTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/CloseableWrapperTest.java index 34c5e8e7e2..e92ee7abb5 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/CloseableWrapperTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/CloseableWrapperTest.java @@ -10,7 +10,7 @@ import static org.mockito.Mockito.verify; public class CloseableWrapperTest { @Test - public void x() { + public void shouldExecuteGivenMethodAtClose() { Consumer wrapped = new Consumer() { // no this cannot be replaced with a lambda because otherwise we could not use Mockito#spy @Override From 6bddf9951f5add4e7df03519b4843371d79e6fac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Thu, 8 Nov 2018 14:59:00 +0100 Subject: [PATCH 26/27] Make commit message configurable --- .../repository/api/MergeCommandBuilder.java | 23 +++++++++++++++++-- .../repository/spi/MergeCommandRequest.java | 9 ++++++++ .../scm/repository/spi/GitMergeCommand.java | 22 ++++++++++++++---- .../repository/spi/GitMergeCommandTest.java | 22 ++++++++++++++++++ 4 files changed, 69 insertions(+), 7 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java index 3823b2b3a7..881a374864 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java @@ -10,8 +10,8 @@ import sonia.scm.repository.spi.MergeCommandRequest; * the branches could be merged without conflicts ({@link #dryRun()}). To do so, you have to specify the name of * the target branch ({@link #setTargetBranch(String)}) and the name of the branch that should be merged * ({@link #setBranchToMerge(String)}). Additionally you can specify an author that should be used for the commit - * ({@link #setAuthor(Person)}) if you are not doing a dry run only. If no author is specified, the logged in user - * will be used instead. + * ({@link #setAuthor(Person)}) and a message template ({@link #setMessageTemplate(String)}) if you are not doing a dry + * run only. If no author is specified, the logged in user and a default message will be used instead. * * To actually merge feature_branch into integration_branch do this: * 

@@ -91,6 +91,25 @@ public class MergeCommandBuilder {
     return this;
   }
 
+  /**
+   * Use this to set a template for the commit message. If no message is set, a default message will be used.
+   *
+   * You can use the placeholder {0} for the branch to be merged and {1} for the target
+   * branch, eg.:
+   *
+   * 

+   * ...setMessageTemplate("Merge of {0} into {1}")...
+   * 

+   *
+   * This is optional and for {@link #executeMerge()} only.
+   *
+   * @return This builder instance.
+   */
+  public MergeCommandBuilder setMessageTemplate(String messageTemplate) {
+    request.setMessageTemplate(messageTemplate);
+    return this;
+  }
+
   /**
    * Use this to reset the command.
    * @return This builder instance.
diff --git a/scm-core/src/main/java/sonia/scm/repository/spi/MergeCommandRequest.java b/scm-core/src/main/java/sonia/scm/repository/spi/MergeCommandRequest.java
index 05f160ec0a..baf03a0aef 100644
--- a/scm-core/src/main/java/sonia/scm/repository/spi/MergeCommandRequest.java
+++ b/scm-core/src/main/java/sonia/scm/repository/spi/MergeCommandRequest.java
@@ -16,6 +16,7 @@ public class MergeCommandRequest implements Validateable, Resetable, Serializabl
   private String branchToMerge;
   private String targetBranch;
   private Person author;
+  private String messageTemplate;
 
   public String getBranchToMerge() {
     return branchToMerge;
@@ -41,6 +42,14 @@ public class MergeCommandRequest implements Validateable, Resetable, Serializabl
     this.author = author;
   }
 
+  public String getMessageTemplate() {
+    return messageTemplate;
+  }
+
+  public void setMessageTemplate(String messageTemplate) {
+    this.messageTemplate = messageTemplate;
+  }
+
   public boolean isValid() {
     return !Strings.isNullOrEmpty(getBranchToMerge())
       && !Strings.isNullOrEmpty(getTargetBranch());
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 f61038b069..05541e8295 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,5 +1,6 @@
 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;
@@ -19,15 +20,16 @@ import sonia.scm.repository.api.MergeDryRunCommandResult;
 import sonia.scm.user.User;
 
 import java.io.IOException;
+import java.text.MessageFormat;
 
 public class GitMergeCommand extends AbstractGitCommand implements MergeCommand {
 
   private static final Logger logger = LoggerFactory.getLogger(GitMergeCommand.class);
 
-  private static final String MERGE_COMMIT_MESSAGE_TEMPLATE =
-    "Merge of branch %s into %s\n" +
-    "\n" +
-    "Automatic merge by SCM-Manager.";
+  private static final String MERGE_COMMIT_MESSAGE_TEMPLATE = String.join("\n",
+    "Merge of branch {0} into {1}",
+    "",
+    "Automatic merge by SCM-Manager.");
 
   private final GitWorkdirFactory workdirFactory;
 
@@ -64,11 +66,13 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand
     private final String toMerge;
     private final Person author;
     private final Git clone;
+    private final String messageTemplate;
 
     private MergeWorker(Repository clone, MergeCommandRequest request) {
       this.target = request.getTargetBranch();
       this.toMerge = request.getBranchToMerge();
       this.author = request.getAuthor();
+      this.messageTemplate = request.getMessageTemplate();
       this.clone = new Git(clone);
     }
 
@@ -111,13 +115,21 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand
       try {
         clone.commit()
           .setAuthor(authorToUse.getName(), authorToUse.getMail())
-          .setMessage(String.format(MERGE_COMMIT_MESSAGE_TEMPLATE, toMerge, target))
+          .setMessage(MessageFormat.format(determineMessageTemplate(), toMerge, target))
           .call();
       } catch (GitAPIException e) {
         throw new InternalRepositoryException("could not commit merge between branch " + toMerge + " and " + target, e);
       }
     }
 
+    private String determineMessageTemplate() {
+      if (Strings.isNullOrEmpty(messageTemplate)) {
+        return MERGE_COMMIT_MESSAGE_TEMPLATE;
+      } else {
+        return messageTemplate;
+      }
+    }
+
     private Person determineAuthor() {
       if (author == null) {
         Subject subject = SecurityUtils.getSubject();
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 5bb9af7d8f..1fca7814ed 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
@@ -67,14 +67,36 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase {
     Iterable commits = new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call();
     RevCommit mergeCommit = commits.iterator().next();
     PersonIdent mergeAuthor = mergeCommit.getAuthorIdent();
+    String message = mergeCommit.getFullMessage();
     assertThat(mergeAuthor.getName()).isEqualTo("Dirk Gently");
     assertThat(mergeAuthor.getEmailAddress()).isEqualTo("dirk@holistic.det");
+    assertThat(message).contains("master", "mergeable");
     // We expect the merge result of file b.txt here by looking up the sha hash of its content.
     // If the file is missing (aka not merged correctly) this will throw a MissingObjectException:
     byte[] contentOfFileB = repository.open(repository.resolve("9513e9c76e73f3e562fd8e4c909d0607113c77c6")).getBytes();
     assertThat(new String(contentOfFileB)).isEqualTo("b\ncontent from branch\n");
   }
 
+  @Test
+  public void shouldUseConfiguredCommitMessageTemplate() throws IOException, GitAPIException {
+    GitMergeCommand command = createCommand();
+    MergeCommandRequest request = new MergeCommandRequest();
+    request.setTargetBranch("master");
+    request.setBranchToMerge("mergeable");
+    request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det"));
+    request.setMessageTemplate("simple");
+
+    MergeCommandResult mergeCommandResult = command.merge(request);
+
+    assertThat(mergeCommandResult.isSuccess()).isTrue();
+
+    Repository repository = createContext().open();
+    Iterable commits = new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call();
+    RevCommit mergeCommit = commits.iterator().next();
+    String message = mergeCommit.getFullMessage();
+    assertThat(message).isEqualTo("simple");
+  }
+
   @Test
   public void shouldNotMergeConflictingBranches() {
     GitMergeCommand command = createCommand();

From c04f6ff61665601170520d72dc0c11e6d3b88a03 Mon Sep 17 00:00:00 2001
From: Sebastian Sdorra 
Date: Thu, 8 Nov 2018 15:08:07 +0000
Subject: [PATCH 27/27] Close branch feature/merge_api