Refactor patch parsing support to be usable outside of the action code

We need to access the same file content and line map data in order
to quote source lines when displaying comments.  Pulling this logic
out into the new PatchFile class makes that easier to resue later.

Signed-off-by: Shawn O. Pearce <sop@google.com>

5 files changed, 450 insertions, 243 deletions

diff --git a/src/main/java/com/google/gerrit/server/patch/PatchDetailAction.java b/src/main/java/com/google/gerrit/server/patch/PatchDetailAction.java
index 073d5be294..643bafdaaa 100644
--- a/src/main/java/com/google/gerrit/server/patch/PatchDetailAction.java
+++ b/src/main/java/com/google/gerrit/server/patch/PatchDetailAction.java
@@ -19,7 +19,6 @@ import com.google.gerrit.client.data.LineWithComments;
 import com.google.gerrit.client.reviewdb.Account;
 import com.google.gerrit.client.reviewdb.Change;
 import com.google.gerrit.client.reviewdb.Patch;
-import com.google.gerrit.client.reviewdb.PatchContent;
 import com.google.gerrit.client.reviewdb.PatchLineComment;
 import com.google.gerrit.client.reviewdb.PatchSet;
 import com.google.gerrit.client.reviewdb.ReviewDb;
@@ -35,35 +34,20 @@ import com.google.gerrit.git.RepositoryCache;
 import com.google.gwtorm.client.OrmException;
 import com.google.gwtorm.client.ResultSet;
 
-import org.spearce.jgit.errors.IncorrectObjectTypeException;
-import org.spearce.jgit.lib.AnyObjectId;
-import org.spearce.jgit.lib.Constants;
-import org.spearce.jgit.lib.ObjectId;
-import org.spearce.jgit.lib.ObjectLoader;
-import org.spearce.jgit.lib.Repository;
-import org.spearce.jgit.patch.CombinedFileHeader;
-import org.spearce.jgit.patch.FileHeader;
-import org.spearce.jgit.patch.FormatError;
-
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
 
 
 abstract class PatchDetailAction<T> implements Action<T> {
-  protected static final byte[] EMPTY_FILE = {};
-
   protected final Patch.Key patchKey;
   protected final List<PatchSet.Id> requestedVersions;
-  protected final boolean direct;
+  private final boolean direct;
   private final RepositoryCache repoCache;
-  private Repository repo;
   protected Change change;
   protected Patch patch;
-  protected FileHeader file;
-  protected int fileCnt;
+  protected PatchFile file;
   protected AccountInfoCacheFactory accountInfo;
   protected Account.Id me;
 
@@ -99,16 +83,31 @@ abstract class PatchDetailAction<T> implements Action<T> {
     change = db.changes().get(patch.getKey().getParentKey().getParentKey());
     BaseServiceImplementation.assertCanRead(change);
 
-    file = readFileHeader(db);
-    if (file instanceof CombinedFileHeader) {
-      fileCnt = ((CombinedFileHeader) file).getParentCount() + 1;
-    } else {
-      fileCnt = 2;
+    try {
+      file = new PatchFile(repoCache, change, db, patch);
+      if (!direct) {
+        file.setRequestedVersions(requestedVersions);
+      }
+    } catch (InvalidRepositoryException e) {
+      throw new Failure(e);
     }
 
     accountInfo = new AccountInfoCacheFactory(db);
     me = Common.getAccountId();
 
+    final int fileCnt;
+    try {
+      fileCnt = file.getFileCount();
+    } catch (CorruptEntityException e) {
+      throw new Failure(e);
+    } catch (NoSuchEntityException e) {
+      throw new Failure(e);
+    } catch (IOException e) {
+      throw new Failure(e);
+    } catch (NoDifferencesException e) {
+      throw new Failure(e);
+    }
+
     published = new HashMap[fileCnt];
     for (int n = 0; n < fileCnt; n++) {
       published[n] = new HashMap<Integer, List<PatchLineComment>>();
@@ -126,89 +125,6 @@ abstract class PatchDetailAction<T> implements Action<T> {
     }
   }
 
-  protected FileHeader readFileHeader(final ReviewDb db) throws Failure,
-      OrmException {
-    if (direct) {
-      return readCachedFileHeader(db);
-    }
-
-    // TODO Fix this gross hack so we aren't running git diff-tree for
-    // an interdiff file side by side view.
-    //
-    final Map<PatchSet.Id, PatchSet> psMap =
-        db.patchSets().toMap(db.patchSets().byChange(change.getId()));
-    final Repository repo = openRepository();
-    final List<String> args = new ArrayList<String>();
-    args.add("git");
-    args.add("--git-dir=.");
-    args.add("diff-tree");
-    args.add("--full-index");
-    if (requestedVersions.size() > 2) {
-      args.add("--cc");
-    } else {
-      args.add("--unified=5");
-    }
-    for (int i = 0; i < requestedVersions.size(); i++) {
-      final PatchSet.Id psi = requestedVersions.get(i);
-      if (psi == null) {
-        throw new Failure(new NoSuchEntityException());
-
-      } else if (psi.equals(PatchSet.BASE)) {
-        final PatchSet p = psMap.get(patchKey.getParentKey());
-        if (p == null || p.getRevision() == null
-            || p.getRevision().get() == null
-            || !ObjectId.isId(p.getRevision().get())) {
-          throw new Failure(new NoSuchEntityException());
-        }
-        args.add(p.getRevision().get() + "^" + (i + 1));
-
-      } else {
-        final PatchSet p = psMap.get(psi);
-        if (p == null || p.getRevision() == null
-            || p.getRevision().get() == null
-            || !ObjectId.isId(p.getRevision().get())) {
-          throw new Failure(new NoSuchEntityException());
-        }
-        args.add(p.getRevision().get());
-      }
-    }
-    args.add("--");
-    args.add(patch.getFileName());
-
-    try {
-      final Process proc =
-          Runtime.getRuntime().exec(args.toArray(new String[args.size()]),
-              null, repo.getDirectory());
-      try {
-        final org.spearce.jgit.patch.Patch p =
-            new org.spearce.jgit.patch.Patch();
-        proc.getOutputStream().close();
-        proc.getErrorStream().close();
-        p.parse(proc.getInputStream());
-        proc.getInputStream().close();
-        if (p.getFiles().isEmpty())
-          throw new Failure(new NoDifferencesException());
-        if (p.getFiles().size() != 1)
-          throw new IOException("unexpected file count back");
-        return p.getFiles().get(0);
-      } finally {
-        try {
-          if (proc.waitFor() != 0) {
-            throw new IOException("git diff-tree exited abnormally");
-          }
-        } catch (InterruptedException ie) {
-        }
-      }
-    } catch (IOException e) {
-      throw new Failure(e);
-    }
-  }
-
-  protected FileHeader readCachedFileHeader(final ReviewDb db) throws Failure,
-      OrmException {
-    return parse(patch, read(db, patch));
-  }
-
   protected List<Patch> history(final ReviewDb db) throws OrmException {
     return db.patches().history(change.getId(), patch.getFileName()).toList();
   }
@@ -257,70 +173,4 @@ abstract class PatchDetailAction<T> implements Action<T> {
       }
     }
   }
-
-  protected Repository openRepository() throws Failure {
-    if (repo == null) {
-      if (change.getDest() == null) {
-        throw new Failure(new CorruptEntityException(change.getId()));
-      }
-      try {
-        repo = repoCache.get(change.getDest().getParentKey().get());
-      } catch (InvalidRepositoryException err) {
-        throw new Failure(err);
-      }
-    }
-    return repo;
-  }
-
-  protected byte[] read(final Repository repo, final AnyObjectId id)
-      throws Failure {
-    if (id == null || ObjectId.zeroId().equals(id)) {
-      return EMPTY_FILE;
-    }
-    try {
-      final ObjectLoader ldr = repo.openObject(id);
-      if (ldr == null) {
-        throw new Failure(new CorruptEntityException(patch.getKey()));
-      }
-      final byte[] content = ldr.getCachedBytes();
-      if (ldr.getType() != Constants.OBJ_BLOB) {
-        throw new Failure(new IncorrectObjectTypeException(id.toObjectId(),
-            Constants.TYPE_BLOB));
-      }
-      return content;
-    } catch (IOException err) {
-      throw new Failure(err);
-    }
-  }
-
-  protected static String read(final ReviewDb db, final Patch patch)
-      throws Failure, OrmException {
-    final PatchContent.Key key = patch.getContent();
-    if (key == null) {
-      throw new Failure(new CorruptEntityException(patch.getKey()));
-    }
-
-    final PatchContent pc = db.patchContents().get(key);
-    if (pc == null || pc.getContent() == null) {
-      throw new Failure(new CorruptEntityException(patch.getKey()));
-    }
-
-    return pc.getContent();
-  }
-
-  protected static FileHeader parse(final Patch patch, final String content)
-      throws Failure {
-    final byte[] buf = Constants.encode(content);
-    final org.spearce.jgit.patch.Patch p = new org.spearce.jgit.patch.Patch();
-    p.parse(buf, 0, buf.length);
-    for (final FormatError err : p.getErrors()) {
-      if (err.getSeverity() == FormatError.Severity.ERROR) {
-        throw new Failure(new CorruptEntityException(patch.getKey()));
-      }
-    }
-    if (p.getFiles().size() != 1) {
-      throw new Failure(new CorruptEntityException(patch.getKey()));
-    }
-    return p.getFiles().get(0);
-  }
 }
diff --git a/src/main/java/com/google/gerrit/server/patch/PatchDetailServiceImpl.java b/src/main/java/com/google/gerrit/server/patch/PatchDetailServiceImpl.java
index 9d2f384b87..7f5263b71d 100644
--- a/src/main/java/com/google/gerrit/server/patch/PatchDetailServiceImpl.java
+++ b/src/main/java/com/google/gerrit/server/patch/PatchDetailServiceImpl.java
@@ -80,7 +80,12 @@ public class PatchDetailServiceImpl extends BaseServiceImplementation implements
 
   public void unifiedPatchDetail(final Patch.Key key,
       final AsyncCallback<UnifiedPatchDetail> callback) {
-    run(callback, new UnifiedPatchDetailAction(key));
+    final RepositoryCache rc = server.getRepositoryCache();
+    if (rc == null) {
+      callback.onFailure(new Exception("No Repository Cache configured"));
+      return;
+    }
+    run(callback, new UnifiedPatchDetailAction(rc, key));
   }
 
   public void myDrafts(final Patch.Key key,
diff --git a/src/main/java/com/google/gerrit/server/patch/PatchFile.java b/src/main/java/com/google/gerrit/server/patch/PatchFile.java
new file mode 100644
index 0000000000..5b9fe4aa71
--- /dev/null
+++ b/src/main/java/com/google/gerrit/server/patch/PatchFile.java
@@ -0,0 +1,364 @@
+// Copyright (C) 2009 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.patch;
+
+import com.google.gerrit.client.reviewdb.Change;
+import com.google.gerrit.client.reviewdb.Patch;
+import com.google.gerrit.client.reviewdb.PatchContent;
+import com.google.gerrit.client.reviewdb.PatchSet;
+import com.google.gerrit.client.reviewdb.ReviewDb;
+import com.google.gerrit.client.rpc.CorruptEntityException;
+import com.google.gerrit.client.rpc.NoDifferencesException;
+import com.google.gerrit.client.rpc.NoSuchEntityException;
+import com.google.gerrit.git.InvalidRepositoryException;
+import com.google.gerrit.git.RepositoryCache;
+import com.google.gwtorm.client.OrmException;
+
+import org.spearce.jgit.errors.IncorrectObjectTypeException;
+import org.spearce.jgit.lib.AbbreviatedObjectId;
+import org.spearce.jgit.lib.AnyObjectId;
+import org.spearce.jgit.lib.Constants;
+import org.spearce.jgit.lib.ObjectId;
+import org.spearce.jgit.lib.ObjectLoader;
+import org.spearce.jgit.lib.Repository;
+import org.spearce.jgit.patch.CombinedFileHeader;
+import org.spearce.jgit.patch.FileHeader;
+import org.spearce.jgit.patch.FormatError;
+import org.spearce.jgit.util.IntList;
+import org.spearce.jgit.util.RawParseUtils;
+
+import java.io.IOException;
+import java.nio.charset.CharacterCodingException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+
+/** State supporting processing of a single {@link Patch} instance. */
+public class PatchFile {
+  private static final byte[] EMPTY_FILE = {};
+
+  private final Repository repo;
+  private final ReviewDb db;
+  private final Patch patch;
+  private List<PatchSet.Id> requestedVersions;
+
+  private String patchContent;
+  private FileHeader fileHeader;
+  private byte[][] fileContents;
+  private IntList[] fileLines;
+
+  public PatchFile(final RepositoryCache rc, final Change chg,
+      final ReviewDb db, final Patch patch) throws InvalidRepositoryException {
+    this(openRepository(rc, chg), db, patch);
+  }
+
+  private static Repository openRepository(final RepositoryCache rc,
+      final Change chg) throws InvalidRepositoryException {
+    return rc.get(chg.getDest().getParentKey().get());
+  }
+
+  public PatchFile(final Repository repo, final ReviewDb db, final Patch patch) {
+    this.repo = repo;
+    this.db = db;
+    this.patch = patch;
+  }
+
+  void setRequestedVersions(final List<PatchSet.Id> rv) {
+    if (fileHeader != null) {
+      throw new IllegalStateException("setRequestedVersions before fileHeader");
+    }
+    requestedVersions = rv;
+  }
+
+  /** @return the source repository where the full data is stored. */
+  public Repository getRepository() {
+    return repo;
+  }
+
+  /** @return the raw patch represented by the delta. */
+  public String getPatchContent() throws CorruptEntityException, OrmException {
+    if (patchContent == null) {
+      final PatchContent.Key key = patch.getContent();
+      if (key == null) {
+        throw new CorruptEntityException(patch.getKey());
+      }
+
+      final PatchContent pc = db.patchContents().get(key);
+      if (pc == null || pc.getContent() == null) {
+        throw new CorruptEntityException(patch.getKey());
+      }
+
+      patchContent = pc.getContent();
+    }
+    return patchContent;
+  }
+
+  /**
+   * @return the parsed patch header, with its hunk information.
+   * @throws NoDifferencesException
+   * @throws IOException
+   * @throws NoSuchEntityException
+   */
+  public FileHeader getFileHeader() throws CorruptEntityException,
+      OrmException, NoSuchEntityException, IOException, NoDifferencesException {
+    if (fileHeader == null) {
+      if (requestedVersions == null) {
+        fileHeader = parseCached();
+      } else {
+        fileHeader = parseExecute();
+      }
+    }
+    return fileHeader;
+  }
+
+
+  /**
+   * @return the total number of files/sides in this patch.
+   * @throws NoDifferencesException
+   * @throws IOException
+   * @throws NoSuchEntityException
+   */
+  public int getFileCount() throws CorruptEntityException, OrmException,
+      NoSuchEntityException, IOException, NoDifferencesException {
+    final FileHeader fh = getFileHeader();
+    if (fh instanceof CombinedFileHeader) {
+      return ((CombinedFileHeader) fh).getParentCount() + 1;
+    }
+    return 2;
+  }
+
+  /**
+   * Get the raw file content of a single side of the patch.
+   * 
+   * @param file file number; 0..{@link #getFileCount()}-1.
+   * @return the raw binary content of the new file.
+   * @throws CorruptEntityException the patch cannot be read.
+   * @throws OrmException the patch cannot be read.
+   * @throws IOException the patch or complete file content cannot be read.
+   * @throws NoDifferencesException
+   * @throws NoSuchEntityException
+   */
+  public byte[] getFileContent(final int file) throws CorruptEntityException,
+      OrmException, IOException, NoSuchEntityException, NoDifferencesException {
+    if (fileContents == null) {
+      fileContents = new byte[getFileCount()][];
+    }
+    if (fileContents[file] == null) {
+      final FileHeader fh = getFileHeader();
+      if (file == fileContents.length - 1) {
+        // Request for the last file is always the new image.
+        //
+        if (fh.getNewId() == null || !fh.getNewId().isComplete()) {
+          throw new CorruptEntityException(patch.getKey());
+        }
+        fileContents[file] = read(fh.getNewId().toObjectId());
+
+      } else {
+        // All other file ids are some sort of old image.
+        //
+        if (fh instanceof CombinedFileHeader) {
+          final CombinedFileHeader ch = (CombinedFileHeader) fh;
+          final AbbreviatedObjectId old = ch.getOldId(file);
+          if (old == null || !old.isComplete()) {
+            throw new CorruptEntityException(patch.getKey());
+          }
+          fileContents[file] = read(old.toObjectId());
+
+        } else {
+          if (fh.getOldId() == null || !fh.getOldId().isComplete()) {
+            throw new CorruptEntityException(patch.getKey());
+          }
+          fileContents[file] = read(fh.getOldId().toObjectId());
+        }
+      }
+    }
+    return fileContents[file];
+  }
+
+  /**
+   * Get the table of line numbers to byte positions in the file content.
+   * 
+   * @param file file number; 0..{@link #getFileCount()}-1.
+   * @return the raw binary content of the new file.
+   * @throws CorruptEntityException the patch cannot be read.
+   * @throws OrmException the patch cannot be read.
+   * @throws IOException the patch or complete file content cannot be read.
+   * @throws NoDifferencesException
+   * @throws NoSuchEntityException
+   */
+  public IntList getLineMap(final int file) throws CorruptEntityException,
+      OrmException, IOException, NoSuchEntityException, NoDifferencesException {
+    if (fileLines == null) {
+      fileLines = new IntList[getFileCount()];
+    }
+    if (fileLines[file] == null) {
+      final byte[] c = getFileContent(file);
+      fileLines[file] = RawParseUtils.lineMap(c, 0, c.length);
+    }
+    return fileLines[file];
+  }
+
+  /**
+   * Get the number of lines in the file.
+   * 
+   * @param file the file to examine
+   * @return total number of lines in the file
+   * @throws CorruptEntityException the patch cannot be read.
+   * @throws OrmException the patch cannot be read.
+   * @throws IOException the patch or complete file content cannot be read.
+   * @throws NoDifferencesException
+   * @throws NoSuchEntityException
+   */
+  public int getLineCount(final int file) throws CorruptEntityException,
+      OrmException, IOException, NoSuchEntityException, NoDifferencesException {
+    final byte[] c = getFileContent(file);
+    final IntList m = getLineMap(file);
+    final int n = m.size();
+    if (n > 0 && m.get(n - 1) == c.length) {
+      return n - 1;
+    }
+    return n;
+  }
+
+  /**
+   * Extract a line from the file, as a string.
+   * 
+   * @param file the file index to extract.
+   * @param line the line number to extract (1 based; 1 is the first line).
+   * @return the string version of the file line.
+   * @throws CorruptEntityException the patch cannot be read.
+   * @throws OrmException the patch cannot be read.
+   * @throws IOException the patch or complete file content cannot be read.
+   * @throws NoDifferencesException
+   * @throws NoSuchEntityException
+   * @throws CharacterCodingException the file is not a known character set.
+   */
+  public String getLine(final int file, final int line)
+      throws CorruptEntityException, OrmException, IOException,
+      NoSuchEntityException, NoDifferencesException {
+    final byte[] c = getFileContent(file);
+    final IntList m = getLineMap(file);
+    final int b = m.get(line);
+    int e = m.get(line + 1);
+    if (b <= e - 1 && e - 1 < c.length && c[e - 1] == '\n') {
+      e--;
+    }
+    return RawParseUtils.decodeNoFallback(Constants.CHARSET, c, b, e);
+  }
+
+  private byte[] read(final AnyObjectId id) throws CorruptEntityException,
+      IOException {
+    if (id == null || ObjectId.zeroId().equals(id)) {
+      return EMPTY_FILE;
+    }
+
+    final ObjectLoader ldr = repo.openObject(id);
+    if (ldr == null) {
+      throw new CorruptEntityException(patch.getKey());
+    }
+
+    final byte[] content = ldr.getCachedBytes();
+    if (ldr.getType() != Constants.OBJ_BLOB) {
+      throw new IncorrectObjectTypeException(id.toObjectId(),
+          Constants.TYPE_BLOB);
+    }
+    return content;
+  }
+
+  private FileHeader parseCached() throws CorruptEntityException, OrmException {
+    final byte[] buf = Constants.encode(getPatchContent());
+    final org.spearce.jgit.patch.Patch p = new org.spearce.jgit.patch.Patch();
+    p.parse(buf, 0, buf.length);
+    for (final FormatError err : p.getErrors()) {
+      if (err.getSeverity() == FormatError.Severity.ERROR) {
+        throw new CorruptEntityException(patch.getKey());
+      }
+    }
+    if (p.getFiles().size() != 1) {
+      throw new CorruptEntityException(patch.getKey());
+    }
+    return p.getFiles().get(0);
+  }
+
+  private FileHeader parseExecute() throws NoSuchEntityException, OrmException,
+      IOException, NoDifferencesException {
+    // TODO Fix this gross hack so we aren't running git diff-tree for
+    // an interdiff file side by side view.
+    //
+    final PatchSet.Id psk = patch.getKey().getParentKey();
+    final Map<PatchSet.Id, PatchSet> psMap =
+        db.patchSets().toMap(db.patchSets().byChange(psk.getParentKey()));
+    final List<String> args = new ArrayList<String>();
+    args.add("git");
+    args.add("--git-dir=.");
+    args.add("diff-tree");
+    args.add("--full-index");
+    if (requestedVersions.size() > 2) {
+      args.add("--cc");
+    } else {
+      args.add("--unified=5");
+    }
+    for (int i = 0; i < requestedVersions.size(); i++) {
+      final PatchSet.Id psi = requestedVersions.get(i);
+      if (psi == null) {
+        throw new NoSuchEntityException();
+
+      } else if (psi.equals(PatchSet.BASE)) {
+        final PatchSet p = psMap.get(psk);
+        if (p == null || p.getRevision() == null
+            || p.getRevision().get() == null
+            || !ObjectId.isId(p.getRevision().get())) {
+          throw new NoSuchEntityException();
+        }
+        args.add(p.getRevision().get() + "^" + (i + 1));
+
+      } else {
+        final PatchSet p = psMap.get(psi);
+        if (p == null || p.getRevision() == null
+            || p.getRevision().get() == null
+            || !ObjectId.isId(p.getRevision().get())) {
+          throw new NoSuchEntityException();
+        }
+        args.add(p.getRevision().get());
+      }
+    }
+    args.add("--");
+    args.add(patch.getFileName());
+
+    final Process proc =
+        Runtime.getRuntime().exec(args.toArray(new String[args.size()]), null,
+            repo.getDirectory());
+    try {
+      final org.spearce.jgit.patch.Patch p = new org.spearce.jgit.patch.Patch();
+      proc.getOutputStream().close();
+      proc.getErrorStream().close();
+      p.parse(proc.getInputStream());
+      proc.getInputStream().close();
+      if (p.getFiles().isEmpty()) {
+        throw new NoDifferencesException();
+      }
+      if (p.getFiles().size() != 1)
+        throw new IOException("unexpected file count back");
+      return p.getFiles().get(0);
+    } finally {
+      try {
+        if (proc.waitFor() != 0) {
+          throw new IOException("git diff-tree exited abnormally");
+        }
+      } catch (InterruptedException ie) {
+      }
+    }
+  }
+}
diff --git a/src/main/java/com/google/gerrit/server/patch/SideBySidePatchDetailAction.java b/src/main/java/com/google/gerrit/server/patch/SideBySidePatchDetailAction.java
index 2c921fd6ab..0795609962 100644
--- a/src/main/java/com/google/gerrit/server/patch/SideBySidePatchDetailAction.java
+++ b/src/main/java/com/google/gerrit/server/patch/SideBySidePatchDetailAction.java
@@ -23,29 +23,25 @@ import com.google.gerrit.client.reviewdb.Patch;
 import com.google.gerrit.client.reviewdb.PatchSet;
 import com.google.gerrit.client.reviewdb.ReviewDb;
 import com.google.gerrit.client.rpc.CorruptEntityException;
+import com.google.gerrit.client.rpc.NoDifferencesException;
+import com.google.gerrit.client.rpc.NoSuchEntityException;
 import com.google.gerrit.client.rpc.BaseServiceImplementation.Failure;
 import com.google.gerrit.git.RepositoryCache;
 import com.google.gwtorm.client.OrmException;
 
-import org.spearce.jgit.lib.AbbreviatedObjectId;
 import org.spearce.jgit.lib.Constants;
-import org.spearce.jgit.lib.Repository;
 import org.spearce.jgit.patch.CombinedFileHeader;
 import org.spearce.jgit.patch.CombinedHunkHeader;
+import org.spearce.jgit.patch.FileHeader;
 import org.spearce.jgit.patch.HunkHeader;
-import org.spearce.jgit.util.IntList;
-import org.spearce.jgit.util.RawParseUtils;
 
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 
 class SideBySidePatchDetailAction extends
     PatchDetailAction<SideBySidePatchDetail> {
-  private int fileCount;
-  private byte[][] fileContents;
-  private IntList[] lineIndex;
-
   SideBySidePatchDetailAction(final RepositoryCache rc, final Patch.Key key,
       final List<PatchSet.Id> fileVersions) {
     super(rc, key, fileVersions);
@@ -55,20 +51,36 @@ class SideBySidePatchDetailAction extends
       Failure {
     init(db);
 
-    if (file.getHunks().isEmpty()) {
-      throw new Failure(new CorruptEntityException(patchKey));
+    final FileHeader fh;
+    final int fileCount;
+    int maxLine = 0;
+    try {
+      fh = file.getFileHeader();
+      if (fh.getHunks().isEmpty()) {
+        throw new Failure(new CorruptEntityException(patchKey));
+      }
+      fileCount = file.getFileCount();
+      for (int i = 0; i < fileCount; i++) {
+        maxLine = Math.max(maxLine, file.getLineCount(i));
+      }
+    } catch (CorruptEntityException e) {
+      throw new Failure(e);
+    } catch (NoSuchEntityException e) {
+      throw new Failure(e);
+    } catch (IOException e) {
+      throw new Failure(e);
+    } catch (NoDifferencesException e) {
+      throw new Failure(e);
     }
 
-    openContents();
-
     final ArrayList<List<SideBySideLine>> lines =
         new ArrayList<List<SideBySideLine>>();
-    if (file instanceof CombinedFileHeader) {
-      for (final CombinedHunkHeader h : ((CombinedFileHeader) file).getHunks()) {
+    if (fh instanceof CombinedFileHeader) {
+      for (final CombinedHunkHeader h : ((CombinedFileHeader) fh).getHunks()) {
       }
 
     } else {
-      for (final HunkHeader h : file.getHunks()) {
+      for (final HunkHeader h : fh.getHunks()) {
         int oldLine = h.getOldImage().getStartLine();
         int newLine = h.getNewStartLine();
 
@@ -141,53 +153,10 @@ class SideBySidePatchDetailAction extends
       }
     }
 
-    int maxLine = 0;
-    for (int i = 0; i < fileCount; i++) {
-      maxLine = Math.max(maxLine, lineIndex[i].size());
-    }
-
     final SideBySidePatchDetail d;
     d = new SideBySidePatchDetail(patch, accountInfo.create());
     d.setLines(fileCount, maxLine, lines);
     d.setHistory(history(db));
     return d;
   }
-
-  private void openContents() throws Failure {
-    final Repository repo = openRepository();
-
-    if (file instanceof CombinedFileHeader) {
-      final CombinedFileHeader ch = (CombinedFileHeader) file;
-
-      fileCount = ch.getParentCount() + 1;
-      fileContents = new byte[fileCount][];
-      for (int i = 0; i < ch.getParentCount(); i++) {
-        final AbbreviatedObjectId old = ch.getOldId(i);
-        if (old == null || !old.isComplete()) {
-          throw new Failure(new CorruptEntityException(patchKey));
-        }
-        fileContents[i] = read(repo, old.toObjectId());
-      }
-
-    } else {
-      if (file.getOldId() == null || !file.getOldId().isComplete()) {
-        throw new Failure(new CorruptEntityException(patchKey));
-      }
-
-      fileCount = 2;
-      fileContents = new byte[fileCount][];
-      fileContents[0] = read(repo, file.getOldId().toObjectId());
-    }
-
-    if (file.getNewId() == null || !file.getNewId().isComplete()) {
-      throw new Failure(new CorruptEntityException(patchKey));
-    }
-    fileContents[fileCount - 1] = read(repo, file.getNewId().toObjectId());
-
-    lineIndex = new IntList[fileCount];
-    for (int i = 0; i < fileCount; i++) {
-      final byte[] c = fileContents[i];
-      lineIndex[i] = RawParseUtils.lineMap(c, 0, c.length);
-    }
-  }
 }
diff --git a/src/main/java/com/google/gerrit/server/patch/UnifiedPatchDetailAction.java b/src/main/java/com/google/gerrit/server/patch/UnifiedPatchDetailAction.java
index 3abc2f0b7a..f6daa66ee4 100644
--- a/src/main/java/com/google/gerrit/server/patch/UnifiedPatchDetailAction.java
+++ b/src/main/java/com/google/gerrit/server/patch/UnifiedPatchDetailAction.java
@@ -22,34 +22,53 @@ import com.google.gerrit.client.data.UnifiedPatchDetail;
 import com.google.gerrit.client.data.PatchLine.Type;
 import com.google.gerrit.client.reviewdb.Patch;
 import com.google.gerrit.client.reviewdb.ReviewDb;
+import com.google.gerrit.client.rpc.CorruptEntityException;
+import com.google.gerrit.client.rpc.NoDifferencesException;
+import com.google.gerrit.client.rpc.NoSuchEntityException;
 import com.google.gerrit.client.rpc.BaseServiceImplementation.Failure;
+import com.google.gerrit.git.RepositoryCache;
 import com.google.gwtorm.client.OrmException;
 
 import org.spearce.jgit.lib.Constants;
+import org.spearce.jgit.patch.FileHeader;
 import org.spearce.jgit.patch.HunkHeader;
 
+import java.io.IOException;
 import java.util.ArrayList;
 
 class UnifiedPatchDetailAction extends PatchDetailAction<UnifiedPatchDetail> {
-  UnifiedPatchDetailAction(final Patch.Key key) {
-    super(null, key, null);
+  UnifiedPatchDetailAction(final RepositoryCache rc, final Patch.Key key) {
+    super(rc, key, null);
   }
 
   public UnifiedPatchDetail run(final ReviewDb db) throws OrmException, Failure {
     init(db);
 
-    final byte[] buf = file.getBuffer();
-    int ptr = file.getStartOffset();
-    final int end = file.getEndOffset();
+    final FileHeader fh;
+    try {
+      fh = file.getFileHeader();
+    } catch (CorruptEntityException e) {
+      throw new Failure(e);
+    } catch (NoSuchEntityException e) {
+      throw new Failure(e);
+    } catch (IOException e) {
+      throw new Failure(e);
+    } catch (NoDifferencesException e) {
+      throw new Failure(e);
+    }
+
+    final byte[] buf = fh.getBuffer();
+    int ptr = fh.getStartOffset();
+    final int end = fh.getEndOffset();
     final int hdrEnd;
     final ArrayList<PatchLine> lines = new ArrayList<PatchLine>();
 
-    if (file.getHunks().size() > 0) {
-      hdrEnd = file.getHunks().get(0).getStartOffset();
-    } else if (file.getForwardBinaryHunk() != null) {
-      hdrEnd = file.getForwardBinaryHunk().getStartOffset();
-    } else if (file.getReverseBinaryHunk() != null) {
-      hdrEnd = file.getReverseBinaryHunk().getStartOffset();
+    if (fh.getHunks().size() > 0) {
+      hdrEnd = fh.getHunks().get(0).getStartOffset();
+    } else if (fh.getForwardBinaryHunk() != null) {
+      hdrEnd = fh.getForwardBinaryHunk().getStartOffset();
+    } else if (fh.getReverseBinaryHunk() != null) {
+      hdrEnd = fh.getReverseBinaryHunk().getStartOffset();
     } else {
       hdrEnd = end;
     }
@@ -61,7 +80,7 @@ class UnifiedPatchDetailAction extends PatchDetailAction<UnifiedPatchDetail> {
           buf, ptr, eol)));
     }
 
-    for (final HunkHeader h : file.getHunks()) {
+    for (final HunkHeader h : fh.getHunks()) {
       final int hunkEnd = h.getEndOffset();
       if (ptr < hunkEnd) {
         eol = nextLF(buf, ptr);