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 53f712cddc..1e6d5c6447 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 @@ -12,18 +12,25 @@ import static java.util.Collections.unmodifiableCollection; * case you can use {@link #getFilesWithConflict()} to get a list of files with merge conflicts. */ public class MergeCommandResult { + private final Collection filesWithConflict; + private final String newHeadRevision; + private final String targetRevision; + private final String revisionToMerge; - private MergeCommandResult(Collection filesWithConflict) { + private MergeCommandResult(Collection filesWithConflict, String targetRevision, String revisionToMerge, String newHeadRevision) { this.filesWithConflict = filesWithConflict; + this.targetRevision = targetRevision; + this.revisionToMerge = revisionToMerge; + this.newHeadRevision = newHeadRevision; } - public static MergeCommandResult success() { - return new MergeCommandResult(emptyList()); + public static MergeCommandResult success(String targetRevision, String revisionToMerge, String newHeadRevision) { + return new MergeCommandResult(emptyList(), targetRevision, revisionToMerge, newHeadRevision); } - public static MergeCommandResult failure(Collection filesWithConflict) { - return new MergeCommandResult(new HashSet<>(filesWithConflict)); + public static MergeCommandResult failure(String targetRevision, String revisionToMerge, Collection filesWithConflict) { + return new MergeCommandResult(new HashSet<>(filesWithConflict), targetRevision, revisionToMerge, null); } /** @@ -31,7 +38,7 @@ public class MergeCommandResult { * merge conflicts. In this case you can use {@link #getFilesWithConflict()} to check what files could not be merged. */ public boolean isSuccess() { - return filesWithConflict.isEmpty(); + return filesWithConflict.isEmpty() && newHeadRevision != null; } /** @@ -41,4 +48,26 @@ public class MergeCommandResult { public Collection getFilesWithConflict() { return unmodifiableCollection(filesWithConflict); } + + /** + * Returns the revision of the new head of the target branch, if the merge was successful ({@link #isSuccess()}) + */ + public String getNewHeadRevision() { + return newHeadRevision; + } + + /** + * Returns the revision of the target branch prior to the merge. + */ + public String getTargetRevision() { + return targetRevision; + } + + /** + * Returns the revision of the branch that was merged into the target (or in case of a conflict of the revision that + * should have been merged). + */ + public String getRevisionToMerge() { + return revisionToMerge; + } } 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 a62e373dca..79d6b12c25 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 @@ -7,6 +7,12 @@ import sonia.scm.repository.api.MergeStrategy; import java.util.Set; public interface MergeCommand { + /** + * Executes the merge. + * @param request The parameters specifying the merge. + * @return Result holding either the new revision or a list of conflicting files. + * @throws sonia.scm.NoChangesMadeException If the merge neither had a conflict nor made any change. + */ MergeCommandResult merge(MergeCommandRequest request); MergeDryRunCommandResult dryRun(MergeCommandRequest request); 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 adf7878221..1c807c6fc7 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 @@ -186,6 +186,10 @@ class AbstractGitCommand return context; } + sonia.scm.repository.Repository getRepository() { + return repository; + } + void checkOutBranch(String branchName) throws IOException { try { clone.checkout().setName(branchName).call(); diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/Differ.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/Differ.java index 0204ca4e3c..fdecac6314 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/Differ.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/Differ.java @@ -2,6 +2,7 @@ package sonia.scm.repository.spi; import com.google.common.base.Strings; import org.eclipse.jgit.diff.DiffEntry; +import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; @@ -10,6 +11,8 @@ import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.treewalk.EmptyTreeIterator; import org.eclipse.jgit.treewalk.TreeWalk; import org.eclipse.jgit.treewalk.filter.PathFilter; +import sonia.scm.ContextEntry; +import sonia.scm.NotFoundException; import sonia.scm.repository.GitUtil; import sonia.scm.util.Util; @@ -35,49 +38,48 @@ final class Differ implements AutoCloseable { } private static Differ create(Repository repository, DiffCommandRequest request) throws IOException { - RevWalk walk = new RevWalk(repository); + RevWalk walk = new RevWalk(repository); - ObjectId revision = repository.resolve(request.getRevision()); - RevCommit commit = walk.parseCommit(revision); + ObjectId revision = repository.resolve(request.getRevision()); + if (revision == null) { + throw NotFoundException.notFound(ContextEntry.ContextBuilder.entity("revision not found", request.getRevision())); + } + RevCommit commit; + try { + commit = walk.parseCommit(revision); + } catch (MissingObjectException ex) { + throw NotFoundException.notFound(ContextEntry.ContextBuilder.entity("revision not found", request.getRevision())); + } - walk.markStart(commit); - commit = walk.next(); - TreeWalk treeWalk = new TreeWalk(repository); - treeWalk.reset(); - treeWalk.setRecursive(true); + walk.markStart(commit); + commit = walk.next(); + TreeWalk treeWalk = new TreeWalk(repository); + treeWalk.reset(); + treeWalk.setRecursive(true); - if (Util.isNotEmpty(request.getPath())) - { - treeWalk.setFilter(PathFilter.create(request.getPath())); - } + if (Util.isNotEmpty(request.getPath())) { + treeWalk.setFilter(PathFilter.create(request.getPath())); + } - if (!Strings.isNullOrEmpty(request.getAncestorChangeset())) - { - ObjectId otherRevision = repository.resolve(request.getAncestorChangeset()); - ObjectId ancestorId = GitUtil.computeCommonAncestor(repository, revision, otherRevision); - RevTree tree = walk.parseCommit(ancestorId).getTree(); + if (!Strings.isNullOrEmpty(request.getAncestorChangeset())) { + ObjectId otherRevision = repository.resolve(request.getAncestorChangeset()); + ObjectId ancestorId = GitUtil.computeCommonAncestor(repository, revision, otherRevision); + RevTree tree = walk.parseCommit(ancestorId).getTree(); + treeWalk.addTree(tree); + } else if (commit.getParentCount() > 0) { + RevTree tree = commit.getParent(0).getTree(); + + if (tree != null) { treeWalk.addTree(tree); - } - else if (commit.getParentCount() > 0) - { - RevTree tree = commit.getParent(0).getTree(); - - if (tree != null) - { - treeWalk.addTree(tree); - } - else - { - treeWalk.addTree(new EmptyTreeIterator()); - } - } - else - { + } else { treeWalk.addTree(new EmptyTreeIterator()); } + } else { + treeWalk.addTree(new EmptyTreeIterator()); + } - treeWalk.addTree(commit.getTree()); + treeWalk.addTree(commit.getTree()); return new Differ(commit, walk, treeWalk); } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitFastForwardIfPossible.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitFastForwardIfPossible.java index 64a20a33cb..84ea1a0bbc 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitFastForwardIfPossible.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitFastForwardIfPossible.java @@ -7,6 +7,7 @@ import sonia.scm.repository.Repository; import sonia.scm.repository.api.MergeCommandResult; import java.io.IOException; +import java.util.Collections; class GitFastForwardIfPossible extends GitMergeStrategy { @@ -22,7 +23,7 @@ class GitFastForwardIfPossible extends GitMergeStrategy { MergeResult fastForwardResult = mergeWithFastForwardOnlyMode(); if (fastForwardResult.getMergeStatus().isSuccessful()) { push(); - return MergeCommandResult.success(); + return createSuccessResult(fastForwardResult.getNewHead().name()); } else { return fallbackMerge.run(); } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeCommit.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeCommit.java index 6aa68a0ea8..9a7d290d3a 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeCommit.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeCommit.java @@ -3,10 +3,16 @@ package sonia.scm.repository.spi; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.MergeCommand; import org.eclipse.jgit.api.MergeResult; +import org.eclipse.jgit.revwalk.RevCommit; +import sonia.scm.NoChangesMadeException; import sonia.scm.repository.Repository; import sonia.scm.repository.api.MergeCommandResult; import java.io.IOException; +import java.util.Collections; +import java.util.Optional; + +import static sonia.scm.repository.spi.GitRevisionExtractor.extractRevisionFromRevCommit; class GitMergeCommit extends GitMergeStrategy { @@ -21,11 +27,12 @@ class GitMergeCommit extends GitMergeStrategy { MergeResult result = doMergeInClone(mergeCommand); if (result.getMergeStatus().isSuccessful()) { - doCommit(); + RevCommit revCommit = doCommit().orElseThrow(() -> new NoChangesMadeException(getRepository())); push(); - return MergeCommandResult.success(); + return createSuccessResult(extractRevisionFromRevCommit(revCommit)); } else { return analyseFailure(result); } } + } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeStrategy.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeStrategy.java index 1d53b99c99..8e9f79d1b9 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeStrategy.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeStrategy.java @@ -6,6 +6,7 @@ import org.eclipse.jgit.api.MergeCommand; import org.eclipse.jgit.api.MergeResult; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.revwalk.RevCommit; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.repository.InternalRepositoryException; @@ -14,6 +15,7 @@ import sonia.scm.repository.api.MergeCommandResult; import java.io.IOException; import java.text.MessageFormat; +import java.util.Optional; abstract class GitMergeStrategy extends AbstractGitCommand.GitCloneWorker { @@ -24,37 +26,57 @@ abstract class GitMergeStrategy extends AbstractGitCommand.GitCloneWorker doCommit() { + logger.debug("merged branch {} into {}", branchToMerge, targetBranch); + return doCommit(MessageFormat.format(determineMessageTemplate(), branchToMerge, targetBranch), author); + } + + MergeCommandResult createSuccessResult(String newRevision) { + return MergeCommandResult.success(targetRevision.name(), revisionToMerge.name(), newRevision); + } + + ObjectId getTargetRevision() { + return targetRevision; + } + + ObjectId getRevisionToMerge() { + return revisionToMerge; } private String determineMessageTemplate() { @@ -66,7 +88,7 @@ abstract class GitMergeStrategy extends AbstractGitCommand.GitCloneWorker new NoChangesMadeException(getRepository())); push(); - return MergeCommandResult.success(); + return MergeCommandResult.success(getTargetRevision().name(), revCommit.name(), extractRevisionFromRevCommit(revCommit)); } else { return analyseFailure(result); } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitRevisionExtractor.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitRevisionExtractor.java new file mode 100644 index 0000000000..5055024151 --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitRevisionExtractor.java @@ -0,0 +1,12 @@ +package sonia.scm.repository.spi; + +import org.eclipse.jgit.revwalk.RevCommit; + +import java.util.Optional; + +public class GitRevisionExtractor { + + static String extractRevisionFromRevCommit(RevCommit revCommit) { + return revCommit.toString().split(" ")[1]; + } +} 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 fcd721c3a2..2616d5b6e5 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 @@ -12,6 +12,7 @@ import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; import org.junit.Rule; import org.junit.Test; +import sonia.scm.NoChangesMadeException; import sonia.scm.NotFoundException; import sonia.scm.repository.Person; import sonia.scm.repository.api.MergeCommandResult; @@ -70,6 +71,8 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { MergeCommandResult mergeCommandResult = command.merge(request); assertThat(mergeCommandResult.isSuccess()).isTrue(); + 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(); @@ -106,7 +109,7 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { assertThat(mergeCommit.getParent(1).name()).isEqualTo("d81ad6c63d7e2162308d69637b339dedd1d9201c"); } - @Test + @Test(expected = NoChangesMadeException.class) public void shouldNotMergeTwice() throws IOException, GitAPIException { GitMergeCommand command = createCommand(); MergeCommandRequest request = new MergeCommandRequest(); @@ -120,15 +123,9 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { assertThat(mergeCommandResult.isSuccess()).isTrue(); Repository repository = createContext().open(); - ObjectId firstMergeCommit = new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call().iterator().next().getId(); + new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call().iterator().next().getId(); - MergeCommandResult secondMergeCommandResult = command.merge(request); - - assertThat(secondMergeCommandResult.isSuccess()).isTrue(); - - ObjectId secondMergeCommit = new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call().iterator().next().getId(); - - assertThat(secondMergeCommit).isEqualTo(firstMergeCommit); + command.merge(request); } @Test @@ -234,6 +231,8 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { Repository repository = createContext().open(); assertThat(mergeCommandResult.isSuccess()).isTrue(); + assertThat(mergeCommandResult.getRevisionToMerge()).isEqualTo(mergeCommandResult.getNewHeadRevision()); + assertThat(mergeCommandResult.getTargetRevision()).isEqualTo("fcd0ef1831e4002ac43ea539f4094334c79ea9ec"); Iterable commits = new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call(); RevCommit mergeCommit = commits.iterator().next(); @@ -284,6 +283,9 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { 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"); assertThat(mergeCommandResult.isSuccess()).isTrue(); diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitRevisionExtractorTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitRevisionExtractorTest.java new file mode 100644 index 0000000000..52a609b6cd --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitRevisionExtractorTest.java @@ -0,0 +1,21 @@ +package sonia.scm.repository.spi; + +import org.eclipse.jgit.revwalk.RevCommit; +import org.junit.jupiter.api.Test; + +import java.util.Optional; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class GitRevisionExtractorTest { + + @Test + void shouldReturnRevisionFromRevCommit() { + RevCommit revCommit = mock(RevCommit.class); + when(revCommit.toString()).thenReturn("commit 123456abcdef -t 4561"); + String revision = GitRevisionExtractor.extractRevisionFromRevCommit(revCommit); + assertThat(revision).isEqualTo("123456abcdef"); + } +} diff --git a/scm-ui/ui-components/src/repos/LoadingDiff.tsx b/scm-ui/ui-components/src/repos/LoadingDiff.tsx index 45b62f9b35..ec782bc968 100644 --- a/scm-ui/ui-components/src/repos/LoadingDiff.tsx +++ b/scm-ui/ui-components/src/repos/LoadingDiff.tsx @@ -7,8 +7,11 @@ import parser from "gitdiff-parser"; import Loading from "../Loading"; import Diff from "./Diff"; import { DiffObjectProps, File } from "./DiffTypes"; +import { NotFoundError } from "../errors"; +import { Notification } from "../index"; +import {withTranslation, WithTranslation} from "react-i18next"; -type Props = DiffObjectProps & { +type Props = WithTranslation & DiffObjectProps & { url: string; defaultCollapse?: boolean; }; @@ -43,7 +46,7 @@ class LoadingDiff extends React.Component { fetchDiff = () => { const { url } = this.props; - this.setState({loading: true}); + this.setState({ loading: true }); apiClient .get(url) .then(response => response.text()) @@ -66,6 +69,9 @@ class LoadingDiff extends React.Component { render() { const { diff, loading, error } = this.state; if (error) { + if (error instanceof NotFoundError) { + return {this.props.t("changesets.noChangesets")}; + } return ; } else if (loading) { return ; @@ -77,4 +83,4 @@ class LoadingDiff extends React.Component { } } -export default LoadingDiff; +export default withTranslation("repos")(LoadingDiff); diff --git a/scm-ui/ui-components/src/repos/changesets/ChangesetDiff.tsx b/scm-ui/ui-components/src/repos/changesets/ChangesetDiff.tsx index 78f4e90cb3..1789cb5517 100644 --- a/scm-ui/ui-components/src/repos/changesets/ChangesetDiff.tsx +++ b/scm-ui/ui-components/src/repos/changesets/ChangesetDiff.tsx @@ -28,7 +28,7 @@ class ChangesetDiff extends React.Component { return {t("changeset.diffNotSupported")}; } else { const url = this.createUrl(changeset); - return ; + return ; } } } diff --git a/scm-ui/ui-webapp/public/locales/de/repos.json b/scm-ui/ui-webapp/public/locales/de/repos.json index 5a357efd95..7fc9fc3403 100644 --- a/scm-ui/ui-webapp/public/locales/de/repos.json +++ b/scm-ui/ui-webapp/public/locales/de/repos.json @@ -72,7 +72,7 @@ "changesets": { "errorTitle": "Fehler", "errorSubtitle": "Changesets konnten nicht abgerufen werden", - "noChangesets": "Keine Changesets in diesem Branch gefunden.", + "noChangesets": "Keine Changesets in diesem Branch gefunden. Die Commits könnten gelöscht worden sein.", "branchSelectorLabel": "Branches", "collapseDiffs": "Auf-/Zuklappen" }, diff --git a/scm-ui/ui-webapp/public/locales/en/repos.json b/scm-ui/ui-webapp/public/locales/en/repos.json index c1e7a835cd..0f5a052256 100644 --- a/scm-ui/ui-webapp/public/locales/en/repos.json +++ b/scm-ui/ui-webapp/public/locales/en/repos.json @@ -72,7 +72,7 @@ "changesets": { "errorTitle": "Error", "errorSubtitle": "Could not fetch changesets", - "noChangesets": "No changesets found for this branch.", + "noChangesets": "No changesets found for this branch. The commits could have been removed.", "branchSelectorLabel": "Branches", "collapseDiffs": "Collapse" }, diff --git a/scm-ui/ui-webapp/public/locales/es/repos.json b/scm-ui/ui-webapp/public/locales/es/repos.json index 3b2451cceb..4e18669287 100644 --- a/scm-ui/ui-webapp/public/locales/es/repos.json +++ b/scm-ui/ui-webapp/public/locales/es/repos.json @@ -72,7 +72,7 @@ "changesets": { "errorTitle": "Error", "errorSubtitle": "No se han podido recuperar los changesets", - "noChangesets": "No se han encontrado changesets para esta rama branch.", + "noChangesets": "No se han encontrado changesets para esta rama branch. Los commits podrían haber sido eliminados.", "branchSelectorLabel": "Ramas", "collapseDiffs": "Colapso" },