diff --git a/gradle/changelog/committer_in_squash_merge.yaml b/gradle/changelog/committer_in_squash_merge.yaml new file mode 100644 index 0000000000..bed4dbd6f1 --- /dev/null +++ b/gradle/changelog/committer_in_squash_merge.yaml @@ -0,0 +1,2 @@ +- type: fixed + description: The committer in squash merge, rebase, and regular merge is now set to the current user. diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java index 2f6d68a794..88c2021594 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java @@ -18,16 +18,11 @@ package sonia.scm.repository.spi; import com.google.common.base.Strings; -import org.apache.shiro.SecurityUtils; -import org.apache.shiro.subject.Subject; import org.eclipse.jgit.api.Git; -import org.eclipse.jgit.api.Status; import org.eclipse.jgit.api.errors.GitAPIException; -import org.eclipse.jgit.api.errors.RefNotFoundException; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; -import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.transport.PushResult; import org.eclipse.jgit.transport.RefSpec; import org.eclipse.jgit.transport.RemoteRefUpdate; @@ -37,21 +32,15 @@ import sonia.scm.ConcurrentModificationException; import sonia.scm.ContextEntry; import sonia.scm.repository.GitWorkingCopyFactory; import sonia.scm.repository.InternalRepositoryException; -import sonia.scm.repository.Person; import sonia.scm.repository.work.WorkingCopy; -import sonia.scm.user.User; import java.io.IOException; import java.util.Collection; import java.util.Iterator; -import java.util.Optional; import java.util.function.Function; -import java.util.function.Supplier; import static java.util.Arrays.asList; import static java.util.Arrays.stream; -import static java.util.Optional.empty; -import static java.util.Optional.of; import static java.util.stream.Collectors.toList; import static org.eclipse.jgit.transport.RemoteRefUpdate.Status.NON_EXISTING; import static org.eclipse.jgit.transport.RemoteRefUpdate.Status.OK; @@ -65,7 +54,7 @@ import static sonia.scm.repository.spi.IntegrateChangesFromWorkdirException.forM class AbstractGitCommand { - + private static final Logger logger = LoggerFactory.getLogger(AbstractGitCommand.class); private static final Collection ACCEPTED_UPDATE_STATUS = asList(OK, UP_TO_DATE, NON_EXISTING); @@ -78,6 +67,15 @@ class AbstractGitCommand { this.context = context; } + static ObjectId resolveRevisionOrThrowNotFound(Repository repository, String revision, sonia.scm.repository.Repository scmRepository) throws IOException { + ObjectId resolved = repository.resolve(revision); + if (resolved == null) { + throw notFound(entity("Revision", revision).in(scmRepository)); + } else { + return resolved; + } + } + Repository open() { return context.open(); } @@ -125,15 +123,6 @@ class AbstractGitCommand { return resolveRevisionOrThrowNotFound(repository, revision, scmRepository); } - static ObjectId resolveRevisionOrThrowNotFound(Repository repository, String revision, sonia.scm.repository.Repository scmRepository) throws IOException { - ObjectId resolved = repository.resolve(revision); - if (resolved == null) { - throw notFound(entity("Revision", revision).in(scmRepository)); - } else { - return resolved; - } - } - abstract static class GitCloneWorker { private final Git clone; private final GitContext context; @@ -151,37 +140,10 @@ class AbstractGitCommand { return clone; } - GitContext getContext() { - return context; - } - sonia.scm.repository.Repository getRepository() { return repository; } - void checkOutBranch(String branchName) throws IOException { - try { - clone.checkout().setName(branchName).call(); - } catch (RefNotFoundException e) { - logger.trace("could not checkout branch {} directly; trying to create local branch", branchName, e); - checkOutTargetAsNewLocalBranch(branchName); - } catch (GitAPIException e) { - throw new InternalRepositoryException(repository, "could not checkout branch: " + branchName, e); - } - } - - private void checkOutTargetAsNewLocalBranch(String branchName) throws IOException { - try { - ObjectId targetRevision = resolveRevision(branchName); - clone.checkout().setStartPoint(targetRevision.getName()).setName(branchName).setCreateBranch(true).call(); - } catch (RefNotFoundException e) { - logger.debug("could not checkout branch {} as local branch", branchName, e); - throw notFound(entity("Revision", branchName).in(repository)); - } catch (GitAPIException e) { - throw new InternalRepositoryException(repository, "could not checkout branch as local branch: " + branchName, e); - } - } - ObjectId resolveRevision(String revision) throws IOException { ObjectId resolved = clone.getRepository().resolve(revision); if (resolved == null) { @@ -191,40 +153,6 @@ class AbstractGitCommand { } } - void failIfNotChanged(Supplier doThrow) { - try { - if (clone.status().call().isClean()) { - throw doThrow.get(); - } - } catch (GitAPIException e) { - throw new InternalRepositoryException(repository, "could not read status of repository", e); - } - } - - Optional doCommit(String message, Person author, boolean sign) { - Person authorToUse = determineAuthor(author); - try { - Status status = clone.status().call(); - if (!status.isClean() || isInMerge()) { - return of(clone.commit() - .setAuthor(authorToUse.getName(), authorToUse.getMail()) - .setCommitter(authorToUse.getName(), authorToUse.getMail()) - .setMessage(message) - .setSign(sign) - .setSigningKey(sign ? "SCM-MANAGER-DEFAULT-KEY" : null) - .call()); - } else { - return empty(); - } - } catch (GitAPIException | IOException e) { - throw new InternalRepositoryException(repository, "could not commit changes", e); - } - } - - private boolean isInMerge() throws IOException { - return clone.getRepository().readMergeHeads() != null && !clone.getRepository().readMergeHeads().isEmpty(); - } - void push(String... refSpecs) { push(false, refSpecs); } @@ -269,22 +197,5 @@ class AbstractGitCommand { } logger.debug("pushed changes"); } - - ObjectId getCurrentObjectId() throws IOException { - return getClone().getRepository().getRefDatabase().findRef("HEAD").getObjectId(); - } - - private Person determineAuthor(Person author) { - if (author == null) { - Subject subject = SecurityUtils.getSubject(); - User user = subject.getPrincipals().oneByType(User.class); - String name = user.getDisplayName(); - String email = user.getMail(); - logger.debug("no author set; using logged in user: {} <{}>", name, email); - return new Person(name, email); - } else { - return author; - } - } } } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeWithSquash.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeWithSquash.java index 82b16083a9..49502a2029 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeWithSquash.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeWithSquash.java @@ -16,9 +16,13 @@ package sonia.scm.repository.spi; +import org.apache.shiro.SecurityUtils; +import org.apache.shiro.subject.Subject; import org.eclipse.jgit.lib.ObjectId; +import sonia.scm.repository.Person; import sonia.scm.repository.RepositoryManager; import sonia.scm.repository.api.MergeCommandResult; +import sonia.scm.user.User; class GitMergeWithSquash { @@ -31,6 +35,14 @@ class GitMergeWithSquash { } MergeCommandResult run() { - return mergeHelper.doRecursiveMerge(request, (sourceRevision, targetRevision) -> new ObjectId[]{targetRevision}); + return mergeHelper.doRecursiveMerge(request, determineCommitter(), (sourceRevision, targetRevision) -> new ObjectId[]{targetRevision}); + } + + private Person determineCommitter() { + Subject subject = SecurityUtils.getSubject(); + User user = subject.getPrincipals().oneByType(User.class); + String name = user.getDisplayName(); + String email = user.getMail(); + return new Person(name, email); } } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/MergeHelper.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/MergeHelper.java index ac574374f0..678a01a99a 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/MergeHelper.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/MergeHelper.java @@ -22,12 +22,12 @@ import org.eclipse.jgit.api.errors.CanceledException; import org.eclipse.jgit.api.errors.UnsupportedSigningFormatException; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Repository; -import org.eclipse.jgit.merge.RecursiveMerger; import org.eclipse.jgit.merge.ResolveMerger; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevWalk; import sonia.scm.NoChangesMadeException; import sonia.scm.repository.InternalRepositoryException; +import sonia.scm.repository.Person; import sonia.scm.repository.RepositoryManager; import sonia.scm.repository.api.MergeCommandResult; @@ -128,6 +128,10 @@ class MergeHelper { } MergeCommandResult doRecursiveMerge(MergeCommandRequest request, BiFunction parents) { + return doRecursiveMerge(request, request.getAuthor(), parents); + } + + MergeCommandResult doRecursiveMerge(MergeCommandRequest request, Person committer, BiFunction parents) { log.trace("merge branch {} into {}", branchToMerge, targetBranch); try { org.eclipse.jgit.lib.Repository repository = context.open(); @@ -149,7 +153,7 @@ class MergeHelper { ObjectId commitId = commitHelper.createCommit( newTreeId, request.getAuthor(), - request.getAuthor(), + committer, determineMessage(), request.isSign(), parents.apply(sourceRevision, targetRevision) 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 2b8d9439fd..f716bad6c8 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 @@ -71,8 +71,8 @@ import static org.mockito.Mockito.when; @SubjectAware(configuration = "classpath:sonia/scm/configuration/shiro.ini", username = "admin", password = "secret") public class GitMergeCommandTest extends AbstractGitCommandTestBase { + static final User COMMITTER_PRINCIPAL = new User("committer", "Comrade Mitter", "ilike@mittens.net"); private static final String REALM = "AdminRealm"; - @Rule public ShiroRule shiro = new ShiroRule(); @Mock @@ -167,18 +167,22 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { assertThat(mergeCommandResult.getRevisionToMerge()).isEqualTo("91b99de908fcd04772798a31c308a64aea1a5523"); assertThat(mergeCommandResult.getTargetRevision()).isEqualTo("fcd0ef1831e4002ac43ea539f4094334c79ea9ec"); - Repository repository = createContext().open(); - 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"); + try (GitContext context = createContext(); Repository repository = context.open()) { + Iterable commits = new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call(); + RevCommit mergeCommit = commits.iterator().next(); + PersonIdent mergeAuthor = mergeCommit.getAuthorIdent(); + PersonIdent mergeCommitter = mergeCommit.getCommitterIdent(); + String message = mergeCommit.getFullMessage(); + assertThat(mergeAuthor.getName()).isEqualTo("Dirk Gently"); + assertThat(mergeAuthor.getEmailAddress()).isEqualTo("dirk@holistic.det"); + assertThat(mergeCommitter.getName()).isEqualTo("Dirk Gently"); + assertThat(mergeCommitter.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 @@ -194,12 +198,13 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { 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(); - assertThat(mergeCommit.getParentCount()).isEqualTo(2); - assertThat(mergeCommit.getParent(0).name()).isEqualTo("fcd0ef1831e4002ac43ea539f4094334c79ea9ec"); - assertThat(mergeCommit.getParent(1).name()).isEqualTo("d81ad6c63d7e2162308d69637b339dedd1d9201c"); + try (GitContext context = createContext(); Repository repository = context.open()) { + Iterable commits = new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call(); + RevCommit mergeCommit = commits.iterator().next(); + assertThat(mergeCommit.getParentCount()).isEqualTo(2); + assertThat(mergeCommit.getParent(0).name()).isEqualTo("fcd0ef1831e4002ac43ea539f4094334c79ea9ec"); + assertThat(mergeCommit.getParent(1).name()).isEqualTo("d81ad6c63d7e2162308d69637b339dedd1d9201c"); + } } @Test(expected = NoChangesMadeException.class) @@ -215,8 +220,9 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { assertThat(mergeCommandResult.isSuccess()).isTrue(); - Repository repository = createContext().open(); - new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call().iterator().next().getId(); + try (GitContext context = createContext(); Repository repository = context.open()) { + new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call().iterator().next().getId(); + } command.merge(request); } @@ -235,11 +241,12 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { 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"); + try (GitContext context = createContext(); Repository repository = context.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 @@ -267,12 +274,12 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { // create concurrent modification after the pre commit hook was fired doAnswer(invocation -> { - RefUpdate refUpdate = createCommand() - .open() - .updateRef("refs/heads/master"); - refUpdate.setNewObjectId(ObjectId.fromString("2f95f02d9c568594d31e78464bd11a96c62e3f91")); - refUpdate.update(); - return null; + try (Repository repository = createCommand().open()) { + RefUpdate refUpdate = repository.updateRef("refs/heads/master"); + refUpdate.setNewObjectId(ObjectId.fromString("2f95f02d9c568594d31e78464bd11a96c62e3f91")); + refUpdate.update(); + return null; + } }).when(repositoryManager).fireHookEvent(any()); command.merge(request); @@ -280,14 +287,8 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { @Test public void shouldTakeAuthorFromSubjectIfNotSet() throws IOException, GitAPIException { - SimplePrincipalCollection principals = new SimplePrincipalCollection(); - principals.add("admin", REALM); - principals.add(new User("dirk", "Dirk Gently", "dirk@holistic.det"), REALM); - shiro.setSubject( - new Subject.Builder() - .principals(principals) - .authenticated(true) - .buildSubject()); + prepareCommitterSubject(); + GitMergeCommand command = createCommand(); MergeCommandRequest request = new MergeCommandRequest(); request.setTargetBranch("master"); @@ -298,11 +299,16 @@ 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(); - assertThat(mergeAuthor.getName()).isEqualTo("Dirk Gently"); - assertThat(mergeAuthor.getEmailAddress()).isEqualTo("dirk@holistic.det"); + try (GitContext context = createContext(); Repository repository = context.open()) { + Iterable mergeCommitIterator = new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call(); + RevCommit mergeCommit = mergeCommitIterator.iterator().next(); + PersonIdent mergeAuthor = mergeCommit.getAuthorIdent(); + PersonIdent mergeCommitter = mergeCommit.getCommitterIdent(); + + assertThat(mergeAuthor.getName()).isEqualTo(COMMITTER_PRINCIPAL.getDisplayName()); + assertThat(mergeAuthor.getEmailAddress()).isEqualTo(COMMITTER_PRINCIPAL.getMail()); + assertCommitter(mergeCommitter); + } } @Test @@ -316,26 +322,34 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { MergeCommandResult mergeCommandResult = command.merge(request); - Repository repository = createContext().open(); - assertThat(mergeCommandResult.isSuccess()).isTrue(); + try (GitContext context = createContext(); Repository repository = context.open()) { + assertThat(mergeCommandResult.isSuccess()).isTrue(); - Iterable commits = new Git(repository).log().add(repository.resolve("mergeable")).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"); + Iterable commits = new Git(repository).log().add(repository.resolve("mergeable")).setMaxCount(1).call(); + RevCommit mergeCommit = commits.iterator().next(); + PersonIdent mergeAuthor = mergeCommit.getAuthorIdent(); + PersonIdent mergeCommitter = mergeCommit.getCommitterIdent(); + String message = mergeCommit.getFullMessage(); + assertThat(mergeAuthor.getName()).isEqualTo("Dirk Gently"); + assertThat(mergeAuthor.getEmailAddress()).isEqualTo("dirk@holistic.det"); + assertThat(mergeCommitter.getName()).isEqualTo("Dirk Gently"); + assertThat(mergeCommitter.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 shouldSquashCommitsIfSquashIsEnabled() throws IOException, GitAPIException { + GitMergeCommand command = createCommand(); MergeCommandRequest request = new MergeCommandRequest(); + + prepareCommitterSubject(); + request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); request.setBranchToMerge("squash"); request.setTargetBranch("master"); @@ -344,75 +358,91 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { MergeCommandResult mergeCommandResult = command.merge(request); - Repository repository = createContext().open(); - assertThat(mergeCommandResult.isSuccess()).isTrue(); - assertThat(mergeCommandResult.getRevisionToMerge()).isEqualTo("35597e9e98fe53167266583848bfef985c2adb27"); - assertThat(mergeCommandResult.getTargetRevision()).isEqualTo("fcd0ef1831e4002ac43ea539f4094334c79ea9ec"); + try (GitContext context = createContext(); Repository repository = context.open()) { + assertThat(mergeCommandResult.isSuccess()).isTrue(); + assertThat(mergeCommandResult.getRevisionToMerge()).isEqualTo("35597e9e98fe53167266583848bfef985c2adb27"); + assertThat(mergeCommandResult.getTargetRevision()).isEqualTo("fcd0ef1831e4002ac43ea539f4094334c79ea9ec"); - Iterable commits = new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call(); - RevCommit mergeCommit = commits.iterator().next(); - assertThat(mergeCommit.getParentCount()).isEqualTo(1); - PersonIdent mergeAuthor = mergeCommit.getAuthorIdent(); - String message = mergeCommit.getFullMessage(); - assertThat(mergeAuthor.getName()).isEqualTo("Dirk Gently"); - assertThat(message).isEqualTo("this is a squash"); + Iterable commits = new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call(); + RevCommit mergeCommit = commits.iterator().next(); + assertThat(mergeCommit.getParentCount()).isEqualTo(1); + PersonIdent mergeAuthor = mergeCommit.getAuthorIdent(); + PersonIdent mergeCommitter = mergeCommit.getCommitterIdent(); + String message = mergeCommit.getFullMessage(); + assertThat(mergeAuthor.getName()).isEqualTo("Dirk Gently"); + assertThat(mergeAuthor.getEmailAddress()).isEqualTo("dirk@holistic.det"); + assertCommitter(mergeCommitter); + assertThat(message).isEqualTo("this is a squash"); + } } @Test public void shouldSquashThreeCommitsIntoOne() throws IOException, GitAPIException { GitMergeCommand command = createCommand(); MergeCommandRequest request = new MergeCommandRequest(); + + prepareCommitterSubject(); + request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); request.setBranchToMerge("squash"); request.setTargetBranch("master"); request.setMessageTemplate("squash three commits"); request.setMergeStrategy(MergeStrategy.SQUASH); - Repository gitRepository = createContext().open(); - MergeCommandResult mergeCommandResult = command.merge(request); - assertThat(mergeCommandResult.isSuccess()).isTrue(); + try (GitContext context = createContext(); Repository gitRepository = context.open()) { + MergeCommandResult mergeCommandResult = command.merge(request); - Iterable commits = new Git(gitRepository).log().add(gitRepository.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(message).isEqualTo("squash three commits"); - assertThat(mergeCommit.getParentCount()).isEqualTo(1); - assertThat(mergeCommit.getParent(0).name()).isEqualTo("fcd0ef1831e4002ac43ea539f4094334c79ea9ec"); + assertThat(mergeCommandResult.isSuccess()).isTrue(); - GitModificationsCommand modificationsCommand = new GitModificationsCommand(createContext()); - List changes = modificationsCommand.getModifications("master").getAdded(); - assertThat(changes).hasSize(3); + Iterable commits = new Git(gitRepository).log().add(gitRepository.resolve("master")).setMaxCount(1).call(); + RevCommit mergeCommit = commits.iterator().next(); + PersonIdent mergeAuthor = mergeCommit.getAuthorIdent(); + PersonIdent mergeCommitter = mergeCommit.getCommitterIdent(); + String message = mergeCommit.getFullMessage(); + + assertThat(mergeAuthor.getName()).isEqualTo("Dirk Gently"); + assertCommitter(mergeCommitter); + assertThat(message).isEqualTo("squash three commits"); + assertThat(mergeCommit.getParentCount()).isEqualTo(1); + assertThat(mergeCommit.getParent(0).name()).isEqualTo("fcd0ef1831e4002ac43ea539f4094334c79ea9ec"); + + GitModificationsCommand modificationsCommand = new GitModificationsCommand(createContext()); + List changes = modificationsCommand.getModifications("master").getAdded(); + assertThat(changes).hasSize(3); + } } @Test public void shouldMergeWithFastForward() throws IOException, GitAPIException { - Repository repository = createContext().open(); + try (GitContext context = createContext(); Repository repository = context.open()) { - ObjectId featureBranchHead = new Git(repository).log().add(repository.resolve("squash")).setMaxCount(1).call().iterator().next().getId(); + ObjectId featureBranchHead = new Git(repository).log().add(repository.resolve("squash")).setMaxCount(1).call().iterator().next().getId(); - GitMergeCommand command = createCommand(); - MergeCommandRequest request = new MergeCommandRequest(); - request.setBranchToMerge("squash"); - request.setTargetBranch("master"); - request.setMergeStrategy(MergeStrategy.FAST_FORWARD_IF_POSSIBLE); - request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); + GitMergeCommand command = createCommand(); + MergeCommandRequest request = new MergeCommandRequest(); + request.setBranchToMerge("squash"); + request.setTargetBranch("master"); + request.setMergeStrategy(MergeStrategy.FAST_FORWARD_IF_POSSIBLE); + request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); - MergeCommandResult mergeCommandResult = command.merge(request); - assertThat(mergeCommandResult.getNewHeadRevision()).isEqualTo("35597e9e98fe53167266583848bfef985c2adb27"); - assertThat(mergeCommandResult.getRevisionToMerge()).isEqualTo("35597e9e98fe53167266583848bfef985c2adb27"); - assertThat(mergeCommandResult.getTargetRevision()).isEqualTo("fcd0ef1831e4002ac43ea539f4094334c79ea9ec"); + MergeCommandResult mergeCommandResult = command.merge(request); + assertThat(mergeCommandResult.getNewHeadRevision()).isEqualTo("35597e9e98fe53167266583848bfef985c2adb27"); + assertThat(mergeCommandResult.getRevisionToMerge()).isEqualTo("35597e9e98fe53167266583848bfef985c2adb27"); + assertThat(mergeCommandResult.getTargetRevision()).isEqualTo("fcd0ef1831e4002ac43ea539f4094334c79ea9ec"); - assertThat(mergeCommandResult.isSuccess()).isTrue(); + assertThat(mergeCommandResult.isSuccess()).isTrue(); - Iterable commits = new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call(); - RevCommit mergeCommit = commits.iterator().next(); - assertThat(mergeCommit.getParentCount()).isEqualTo(1); - PersonIdent mergeAuthor = mergeCommit.getAuthorIdent(); - assertThat(mergeAuthor.getName()).isEqualTo("Philip J Fry"); - assertThat(mergeCommit.getId()).isEqualTo(featureBranchHead); + Iterable commits = new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call(); + RevCommit mergeCommit = commits.iterator().next(); + assertThat(mergeCommit.getParentCount()).isEqualTo(1); + PersonIdent mergeAuthor = mergeCommit.getAuthorIdent(); + PersonIdent mergeCommitter = mergeCommit.getCommitterIdent(); + + assertThat(mergeAuthor.getName()).isEqualTo("Philip J Fry"); + assertThat(mergeCommitter.getName()).isEqualTo("Eduard Heimbuch"); + assertThat(mergeCommit.getId()).isEqualTo(featureBranchHead); + } } @Test @@ -428,15 +458,16 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { 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(); - PersonIdent mergeAuthor = mergeCommit.getAuthorIdent(); - assertThat(mergeCommit.getParentCount()).isEqualTo(2); - String message = mergeCommit.getFullMessage(); - assertThat(mergeAuthor.getName()).isEqualTo("Dirk Gently"); - assertThat(mergeAuthor.getEmailAddress()).isEqualTo("dirk@holistic.det"); - assertThat(message).contains("master", "mergeable"); + try (GitContext context = createContext(); Repository repository = context.open()) { + Iterable commits = new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call(); + RevCommit mergeCommit = commits.iterator().next(); + PersonIdent mergeAuthor = mergeCommit.getAuthorIdent(); + assertThat(mergeCommit.getParentCount()).isEqualTo(2); + String message = mergeCommit.getFullMessage(); + assertThat(mergeAuthor.getName()).isEqualTo("Dirk Gently"); + assertThat(mergeAuthor.getEmailAddress()).isEqualTo("dirk@holistic.det"); + assertThat(message).contains("master", "mergeable"); + } } @Test(expected = NotFoundException.class) @@ -494,12 +525,12 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { 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(); - assertThat(mergeCommit.getRawGpgSignature()).isNotEmpty(); - assertThat(mergeCommit.getRawGpgSignature()).isEqualTo(GitTestHelper.SimpleGpgSigner.getSignature()); - + try (GitContext context = createContext(); Repository repository = context.open()) { + Iterable commits = new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call(); + RevCommit mergeCommit = commits.iterator().next(); + assertThat(mergeCommit.getRawGpgSignature()).isNotEmpty(); + assertThat(mergeCommit.getRawGpgSignature()).isEqualTo(GitTestHelper.SimpleGpgSigner.getSignature()); + } } @Test @@ -516,40 +547,53 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { 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(); - assertThat(mergeCommit.getRawGpgSignature()).isNullOrEmpty(); - + try (GitContext context = createContext(); Repository repository = context.open()) { + Iterable commits = new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call(); + RevCommit mergeCommit = commits.iterator().next(); + assertThat(mergeCommit.getRawGpgSignature()).isNullOrEmpty(); + } } @Test public void shouldAllowMergeWithRebase() throws IOException, GitAPIException { GitMergeCommand command = createCommand(); MergeCommandRequest request = new MergeCommandRequest(); + + prepareCommitterSubject(); + request.setTargetBranch("master"); request.setBranchToMerge("mergeable"); request.setMergeStrategy(MergeStrategy.REBASE); - request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); 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(); - assertThat(mergeCommit.getParentCount()).isEqualTo(1); - assertThat(mergeCommit.getParent(0).name()).isEqualTo("fcd0ef1831e4002ac43ea539f4094334c79ea9ec"); - assertThat(mergeCommit.getName()).isEqualTo(mergeCommandResult.getNewHeadRevision()); - assertThat(mergeCommit.getName()).doesNotStartWith("91b99de908fcd04772798a31c308a64aea1a5523"); - assertThat(mergeCommit.getAuthorIdent().getWhenAsInstant()).isEqualTo("2018-11-07T10:20:52Z"); // the timestamp of the original commit + try (GitContext context = createContext(); Repository repository = context.open()) { + Iterable commits = new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call(); + RevCommit mergeCommit = commits.iterator().next(); + assertThat(mergeCommit.getParentCount()).isEqualTo(1); + assertThat(mergeCommit.getParent(0).name()).isEqualTo("fcd0ef1831e4002ac43ea539f4094334c79ea9ec"); + assertThat(mergeCommit.getName()).isEqualTo(mergeCommandResult.getNewHeadRevision()); + assertThat(mergeCommit.getName()).doesNotStartWith("91b99de908fcd04772798a31c308a64aea1a5523"); + assertThat(mergeCommit.getAuthorIdent().getWhenAsInstant()).isEqualTo("2018-11-07T10:20:52Z"); // the timestamp of the original commit + + PersonIdent mergeAuthor = mergeCommit.getAuthorIdent(); + PersonIdent mergeCommitter = mergeCommit.getCommitterIdent(); + + assertThat(mergeAuthor.getName()).isEqualTo("Douglas Adams"); + assertThat(mergeAuthor.getEmailAddress()).isEqualTo("douglas.adams@hitchhiker.com"); + assertCommitter(mergeCommitter); + } } @Test public void shouldRebaseMultipleCommits() throws IOException, GitAPIException { GitMergeCommand command = createCommand(); MergeCommandRequest request = new MergeCommandRequest(); + + prepareCommitterSubject(); + request.setTargetBranch("master"); request.setBranchToMerge("squash"); request.setMergeStrategy(MergeStrategy.REBASE); @@ -557,19 +601,19 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { command.merge(request); - Repository repository = createContext().open(); - Iterable commits = new Git(repository).log().add(repository.resolve("master")).setMaxCount(6).call(); - - assertThat(commits) - .extracting("shortMessage") - .containsExactly( - "third", - "second commit", - "first commit", - "added new line for blame", - "added file f", - "added file d and e in folder c" - ); + try (GitContext context = createContext(); Repository repository = context.open()) { + Iterable commits = new Git(repository).log().add(repository.resolve("master")).setMaxCount(6).call(); + assertThat(commits) + .extracting("shortMessage") + .containsExactly( + "third", + "second commit", + "first commit", + "added new line for blame", + "added file f", + "added file d and e in folder c" + ); + } } @Test @@ -581,12 +625,16 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { request.setMergeStrategy(MergeStrategy.REBASE); request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); + prepareCommitterSubject(); + MergeCommandResult mergeCommandResult = command.merge(request); assertThat(mergeCommandResult.isSuccess()).isFalse(); assertThat(mergeCommandResult.getFilesWithConflict()).containsExactly("a.txt"); - Repository repository = createContext().open(); - Iterable commits = new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call(); + Iterable commits; + try (GitContext context = createContext(); Repository repository = context.open()) { + commits = new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call(); + } RevCommit headCommit = commits.iterator().next(); assertThat(headCommit.getName()).isEqualTo("fcd0ef1831e4002ac43ea539f4094334c79ea9ec"); } @@ -628,4 +676,20 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { } }; } + + private void prepareCommitterSubject() { + SimplePrincipalCollection principals = new SimplePrincipalCollection(); + principals.add("admin", REALM); + principals.add(COMMITTER_PRINCIPAL, REALM); + shiro.setSubject( + new Subject.Builder() + .principals(principals) + .authenticated(true) + .buildSubject()); + } + + private void assertCommitter(PersonIdent mergeCommitter) { + assertThat(mergeCommitter.getName()).isEqualTo(COMMITTER_PRINCIPAL.getDisplayName()); + assertThat(mergeCommitter.getEmailAddress()).isEqualTo(COMMITTER_PRINCIPAL.getMail()); + } }