Merged in feature/pr_history (pull request #378)

Feature/pr history

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<String> filesWithConflict;
+  private final String newHeadRevision;
+  private final String targetRevision;
+  private final String revisionToMerge;
 
-  private MergeCommandResult(Collection<String> filesWithConflict) {
+  private MergeCommandResult(Collection<String> 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<String> filesWithConflict) {
-    return new MergeCommandResult(new HashSet<>(filesWithConflict));
+  public static MergeCommandResult failure(String targetRevision, String revisionToMerge, Collection<String> 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<String> 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;
+  }
 }

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);

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();

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;
 
@@ -38,7 +41,15 @@ final class Differ implements AutoCloseable {
     RevWalk walk = new RevWalk(repository);
 
     ObjectId revision = repository.resolve(request.getRevision());
-      RevCommit commit = walk.parseCommit(revision);
+    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();
@@ -46,34 +57,25 @@ final class Differ implements AutoCloseable {
     treeWalk.reset();
     treeWalk.setRecursive(true);
 
-      if (Util.isNotEmpty(request.getPath()))
-      {
+    if (Util.isNotEmpty(request.getPath())) {
       treeWalk.setFilter(PathFilter.create(request.getPath()));
     }
 
 
-      if (!Strings.isNullOrEmpty(request.getAncestorChangeset()))
-      {
+    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)
-      {
+    } else if (commit.getParentCount() > 0) {
       RevTree tree = commit.getParent(0).getTree();
 
-        if (tree != null)
-        {
+      if (tree != null) {
         treeWalk.addTree(tree);
-        }
-        else
-        {
+      } else {
         treeWalk.addTree(new EmptyTreeIterator());
       }
-      }
-      else
-      {
+    } else {
       treeWalk.addTree(new EmptyTreeIterator());
     }
 

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();
     }

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);
     }
   }
+
 }

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<MergeCommandResult> {
 
@@ -24,37 +26,57 @@ abstract class GitMergeStrategy extends AbstractGitCommand.GitCloneWorker<MergeC
     "",
     "Automatic merge by SCM-Manager.");
 
-  private final String target;
-  private final String toMerge;
+  private final String targetBranch;
+  private final ObjectId targetRevision;
+  private final String branchToMerge;
+  private final ObjectId revisionToMerge;
   private final Person author;
   private final String messageTemplate;
 
   GitMergeStrategy(Git clone, MergeCommandRequest request, GitContext context, sonia.scm.repository.Repository repository) {
     super(clone, context, repository);
-    this.target = request.getTargetBranch();
-    this.toMerge = request.getBranchToMerge();
+    this.targetBranch = request.getTargetBranch();
+    this.branchToMerge = request.getBranchToMerge();
     this.author = request.getAuthor();
     this.messageTemplate = request.getMessageTemplate();
+    try {
+      this.targetRevision = resolveRevision(request.getTargetBranch());
+      this.revisionToMerge = resolveRevision(request.getBranchToMerge());
+    } catch (IOException e) {
+      throw new InternalRepositoryException(repository, "Could not resolve revisions of target branch or branch to merge", e);
+    }
   }
 
   MergeResult doMergeInClone(MergeCommand mergeCommand) throws IOException {
     MergeResult result;
     try {
-      ObjectId sourceRevision = resolveRevision(toMerge);
+      ObjectId sourceRevision = resolveRevision(branchToMerge);
       mergeCommand
         .setCommit(false) // we want to set the author manually
-        .include(toMerge, sourceRevision);
+        .include(branchToMerge, sourceRevision);
 
       result = mergeCommand.call();
     } catch (GitAPIException e) {
-      throw new InternalRepositoryException(getContext().getRepository(), "could not merge branch " + toMerge + " into " + target, e);
+      throw new InternalRepositoryException(getContext().getRepository(), "could not merge branch " + branchToMerge + " into " + targetBranch, e);
     }
     return result;
   }
 
-  void doCommit() {
-    logger.debug("merged branch {} into {}", toMerge, target);
-    doCommit(MessageFormat.format(determineMessageTemplate(), toMerge, target), author);
+  Optional<RevCommit> 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<MergeC
   }
 
   MergeCommandResult analyseFailure(MergeResult result) {
-    logger.info("could not merge branch {} into {} due to conflict in paths {}", toMerge, target, result.getConflicts().keySet());
-    return MergeCommandResult.failure(result.getConflicts().keySet());
+    logger.info("could not merge branch {} into {} due to conflict in paths {}", branchToMerge, targetBranch, result.getConflicts().keySet());
+    return MergeCommandResult.failure(targetRevision.name(), revisionToMerge.name(), result.getConflicts().keySet());
   }
 }

scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeWithSquash.java

@@ -1,13 +1,17 @@
 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 org.eclipse.jgit.api.MergeCommand;
 
 import java.io.IOException;
 
+import static sonia.scm.repository.spi.GitRevisionExtractor.extractRevisionFromRevCommit;
+
 class GitMergeWithSquash extends GitMergeStrategy {
 
   GitMergeWithSquash(Git clone, MergeCommandRequest request, GitContext context, Repository repository) {
@@ -21,9 +25,9 @@ class GitMergeWithSquash extends GitMergeStrategy {
     MergeResult result = doMergeInClone(mergeCommand);
 
     if (result.getMergeStatus().isSuccessful()) {
-      doCommit();
+      RevCommit revCommit = doCommit().orElseThrow(() -> new NoChangesMadeException(getRepository()));
       push();
-      return MergeCommandResult.success();
+      return MergeCommandResult.success(getTargetRevision().name(), revCommit.name(), extractRevisionFromRevCommit(revCommit));
     } else {
       return analyseFailure(result);
     }

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];
+  }
+}

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<RevCommit> 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<RevCommit> 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();
 

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");
+  }
+}

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<Props, State> {
 
   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<Props, State> {
   render() {
     const { diff, loading, error } = this.state;
     if (error) {
+      if (error instanceof NotFoundError) {
+        return <Notification type="info">{this.props.t("changesets.noChangesets")}</Notification>;
+      }
       return <ErrorNotification error={error} />;
     } else if (loading) {
       return <Loading />;
@@ -77,4 +83,4 @@ class LoadingDiff extends React.Component<Props, State> {
   }
 }
 
-export default LoadingDiff;
+export default withTranslation("repos")(LoadingDiff);

scm-ui/ui-components/src/repos/changesets/ChangesetDiff.tsx

@@ -28,7 +28,7 @@ class ChangesetDiff extends React.Component<Props> {
       return <Notification type="danger">{t("changeset.diffNotSupported")}</Notification>;
     } else {
       const url = this.createUrl(changeset);
-      return <LoadingDiff url={url} defaultCollapse={defaultCollapse} />;
+      return <LoadingDiff url={url} defaultCollapse={defaultCollapse} sideBySide={false}/>;
     }
   }
 }

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"
   },

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"
   },

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"
   },