Let PatchListLoader always load parent commits

For comparing a commit against one of its parent commits,
PatchListLoader accepts PatchListKeys with a parentNum. If parentNum
in a PatchListKey is set, PatchListLoader takes care to load the
parent commit.

However some code paths did not make use of this functionality in
PatchListLoader. Instead they loaded the parent commit upfront and
then constructed a PatchListKey with 2 commits (the commit and its
parent commit) to load the PatchList.

Loading the parent commit upfront is unneeded since PatchListLoader
knows how to do this. If the parent commit is always loaded by
PatchListLoader, the code for loading the parent commit upfront can be
deleted. Also for listing files the repository is opened once less
now.

Change-Id: Ifca30529d25419aaf5af2252f1bcd45ab5dbb853
Signed-off-by: Edwin Kempin <ekempin@google.com>

gerrit-server/src/main/java/com/google/gerrit/server/change/FileInfoJson.java

@@ -14,8 +14,6 @@
 
 package com.google.gerrit.server.change;
 
-import static com.google.gerrit.server.util.GitUtil.getParent;
-
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
 import com.google.gerrit.extensions.common.FileInfo;
@@ -23,7 +21,6 @@ import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Patch;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.RevId;
-import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.patch.PatchList;
 import com.google.gerrit.server.patch.PatchListCache;
 import com.google.gerrit.server.patch.PatchListEntry;
@@ -32,24 +29,18 @@ import com.google.gerrit.server.patch.PatchListNotAvailableException;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
 
-import org.eclipse.jgit.errors.RepositoryNotFoundException;
 import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.Repository;
 
-import java.io.IOException;
 import java.util.Map;
 import java.util.TreeMap;
 
 @Singleton
 public class FileInfoJson {
   private final PatchListCache patchListCache;
-  private final GitRepositoryManager repoManager;
 
   @Inject
   FileInfoJson(
-      PatchListCache patchListCache,
-      GitRepositoryManager repoManager) {
-    this.repoManager = repoManager;
+      PatchListCache patchListCache) {
     this.patchListCache = patchListCache;
   }
 
@@ -64,24 +55,19 @@ public class FileInfoJson {
         ? null
         : ObjectId.fromString(base.getRevision().get());
     ObjectId b = ObjectId.fromString(revision.get());
-    return toFileInfoMap(change, a, b);
+    return toFileInfoMap(change, new PatchListKey(a, b, Whitespace.IGNORE_NONE));
   }
 
   Map<String, FileInfo> toFileInfoMap(Change change, RevId revision, int parent)
-      throws RepositoryNotFoundException, IOException,
-          PatchListNotAvailableException {
+      throws PatchListNotAvailableException {
     ObjectId b = ObjectId.fromString(revision.get());
-    ObjectId a;
-    try (Repository git = repoManager.openRepository(change.getProject())) {
-      a = getParent(git, b, parent);
-    }
-    return toFileInfoMap(change, a, b);
+    return toFileInfoMap(change,
+        PatchListKey.againstParentNum(parent + 1, b, Whitespace.IGNORE_NONE));
   }
 
   private Map<String, FileInfo> toFileInfoMap(Change change,
-      ObjectId a, ObjectId b) throws PatchListNotAvailableException {
-    PatchList list = patchListCache.get(
-        new PatchListKey(a, b, Whitespace.IGNORE_NONE), change.getProject());
+      PatchListKey key) throws PatchListNotAvailableException {
+    PatchList list = patchListCache.get(key, change.getProject());
 
     Map<String, FileInfo> files = new TreeMap<>();
     for (PatchListEntry e : list.getPatches()) {

gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptFactory.java

@@ -15,7 +15,6 @@
 package com.google.gerrit.server.patch;
 
 import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.gerrit.server.util.GitUtil.getParent;
 
 import com.google.common.base.Optional;
 import com.google.gerrit.common.Nullable;
@@ -214,8 +213,6 @@ public class PatchScriptFactory implements Callable<PatchScript> {
       bId = toObjectId(psEntityB);
       if (parentNum < 0) {
         aId = psEntityA != null ? toObjectId(psEntityA) : null;
-      } else {
-        aId = getParent(git, bId, parentNum);
       }
 
       try {
@@ -247,8 +244,11 @@ public class PatchScriptFactory implements Callable<PatchScript> {
   }
 
   private PatchListKey keyFor(final Whitespace whitespace) {
+    if (parentNum < 0) {
       return new PatchListKey(aId, bId, whitespace);
     }
+    return PatchListKey.againstParentNum(parentNum + 1, bId, whitespace);
+  }
 
   private PatchList listFor(final PatchListKey key)
       throws PatchListNotAvailableException {

gerrit-server/src/main/java/com/google/gerrit/server/util/GitUtil.java

@@ -1,51 +0,0 @@
-// Copyright (C) 2016 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.server.util;
-
-import org.eclipse.jgit.errors.IncorrectObjectTypeException;
-import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.revwalk.RevCommit;
-import org.eclipse.jgit.revwalk.RevWalk;
-
-import java.io.IOException;
-
-public class GitUtil {
-
-  /**
-   * @param git
-   * @param commitId
-   * @param parentNum
-   * @return the {@code paretNo} parent of given commit or {@code null}
-   *             when {@code parentNo} exceed number of {@code commitId} parents.
-   * @throws IncorrectObjectTypeException
-   *             the supplied id is not a commit or an annotated tag.
-   * @throws IOException
-   *             a pack file or loose object could not be read.
-   */
-  public static RevCommit getParent(Repository git,
-      ObjectId commitId, int parentNum) throws IOException {
-    try (RevWalk walk = new RevWalk(git)) {
-      RevCommit commit = walk.parseCommit(commitId);
-      if (commit.getParentCount() > parentNum) {
-        return commit.getParent(parentNum);
-      }
-    }
-    return null;
-  }
-
-  private GitUtil() {
-  }
-}