move getMergeBase to Commit, implement a method for checking whether a git...

move getMergeBase to Commit, implement a method for checking whether a git command failed (propably)

src/de/uni_passau/fim/seibt/gitwrapper/repo/Commit.java

@@ -56,6 +56,43 @@ public class Commit {
         return revList.map(toParentsList).orElse(Collections.emptyList());
     }
 
+    /**
+     * Optionally returns the merge base for <code>this</code> and <code>other</code>. The <code>other</code> commit
+     * must be part of the same {@link Repository} this {@link Commit} is.
+     *
+     * @param other
+     *         the other {@link Commit}
+     * @return the merge base or an empty {@link Optional} if there is no merge base or an exception occurred
+     */
+    public Optional<Commit> getMergeBase(Commit other) {
+
+        if (!repo.equals(other.repo)) {
+            LOG.warning(() -> {
+                String msg = "Failed to obtain a merge base for %s and %s as they are not from the same repository.";
+                return String.format(msg, this, other);
+            });
+
+            return Optional.empty();
+        }
+
+        Optional<ExecRes> mergeBase = git.exec(repo.getDir(), "merge-base", getId(), other.getId());
+        Function<ExecRes, Commit> toCommit = res -> {
+
+            if (git.failed(res)) {
+                LOG.warning(() -> String.format("Failed to obtain a merge base for %s and %s.", this, other));
+                return null;
+            }
+
+            Commit base = repo.getCommit(res.output.trim());
+
+            LOG.fine(() -> String.format("Commits %s and %s have the merge base %s.", this, other, base));
+
+            return base;
+        };
+
+        return mergeBase.map(toCommit);
+    }
+
     /**
      * Returns the ID of this commit.
      *
@@ -86,6 +123,6 @@ public class Commit {
 
     @Override
     public String toString() {
-        return String.format("Commit{id='%s'}", id);
+        return String.valueOf(id);
     }
 }

src/de/uni_passau/fim/seibt/gitwrapper/repo/GitWrapper.java

@@ -23,6 +23,9 @@ public class GitWrapper extends ToolWrapper {
 
     private static final Logger LOG = Logger.getLogger(GitWrapper.class.getCanonicalName());
 
+    private static final String FATAL_PREFIX = "fatal";
+    private static final String ERROR_PREFIX = "error";
+
     private static final Pattern CLONING_INTO = Pattern.compile("Cloning into '(.*)'\\.\\.\\.");
     private static final Pattern ALREADY_EXISTS = Pattern.compile("fatal: destination path '(.*)' already exists and is not an empty directory\\.");
 
@@ -119,7 +122,18 @@ public class GitWrapper extends ToolWrapper {
         }
 
         Optional<ExecRes> status = exec(directory, "rev-parse", "--is-inside-git-dir");
-        return status.map(res -> res.exitCode == EXIT_SUCCESS).orElse(false);
+        return status.map(res -> !failed(res)).orElse(false);
+    }
+
+    /**
+     * Returns whether the given git command failed. This relies on exit code first and then on the assumption that
+     * the output (in case of failure) starts with either "{@value FATAL_PREFIX}" or "{@value ERROR_PREFIX}".
+     *
+     * @param res the {@link ExecRes} to check
+     * @return whether the git command failed
+     */
+    public boolean failed(ExecRes res) {
+        return res.exitCode != EXIT_SUCCESS || res.output.startsWith(FATAL_PREFIX) || res.output.startsWith(ERROR_PREFIX);
     }
 
     /**

src/de/uni_passau/fim/seibt/gitwrapper/repo/Repository.java

@@ -16,15 +16,12 @@ import java.util.stream.Collectors;
 
 import de.uni_passau.fim.seibt.gitwrapper.process.ProcessExecutor.ExecRes;
 
-import static de.uni_passau.fim.seibt.gitwrapper.repo.GitWrapper.EXIT_SUCCESS;
-
 /**
  * A git {@link Repository}.
  */
 public class Repository {
 
     private static final Logger LOG = Logger.getLogger(Repository.class.getCanonicalName());
-    private static final String FAILURE_PREFIX = "fatal";
 
     private GitWrapper git;
 
@@ -65,6 +62,12 @@ public class Repository {
     public List<Commit> getMergeCommits() {
         Optional<ExecRes> revList = git.exec(dir, "rev-list", "--all", "--merges");
         Function<ExecRes, List<Commit>> toCommitList = res -> {
+
+            if (git.failed(res)) {
+                LOG.warning(() -> String.format("Failed to obtain the merge commits from %s.", this));
+                return null;
+            }
+
             String[] lines = res.output.split("[\\r?\\n]+");
 
             LOG.fine(() -> String.format("Found %d merge commits in %s.", lines.length, this));
@@ -76,40 +79,6 @@ public class Repository {
         return revList.map(toCommitList).orElse(Collections.emptyList());
     }
 
-    /**
-     * Returns the merge base of the two given {@link Commit} objects. Both must be from the same (this)
-     * {@link Repository}.
-     *
-     * @param a
-     *         the first {@link Commit}
-     * @param b
-     *         the second {@link Commit}
-     * @return the merge base or
-     */
-    public Optional<Commit> getMergeBase(Commit a, Commit b) {
-
-        if (!(commits.values().contains(a) && commits.values().contains(b))) {
-            LOG.warning(() -> "Both commits must be part of the repository to get a merge base for them.");
-            return Optional.empty();
-        }
-
-        Optional<ExecRes> mergeBase = git.exec(dir, "merge-base", a.getId(), b.getId());
-        Function<ExecRes, Commit> toCommit = res -> {
-
-            if (res.exitCode != EXIT_SUCCESS || res.output.startsWith(FAILURE_PREFIX)) {
-                return null;
-            }
-
-            Commit base = getCommit(res.output.trim());
-
-            LOG.fine(() -> String.format("Commits %s and %s have the merge base %s.", a.getId(), b.getId(), base.getId()));
-
-            return base;
-        };
-
-        return mergeBase.map(toCommit);
-    }
-
     /**
      * Returns a {@link Commit} for the given id. The <code>id</code> is not checked for validity, this method
      * only ensures that only one {@link Commit} object is created for every <code>id</code>.