Move a lot of the state for unifiedPatchDetailAction into the base

This way we can reuse the initial patch parsing, validation, and
comment indexing for all types of detailed patch actions.

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

2 files changed, 61 insertions, 58 deletions

diff --git a/webapp/src/com/google/gerrit/server/patch/PatchDetailAction.java b/webapp/src/com/google/gerrit/server/patch/PatchDetailAction.java
index 96b34fdbf6..1c6429da5a 100644
--- a/webapp/src/com/google/gerrit/server/patch/PatchDetailAction.java
+++ b/webapp/src/com/google/gerrit/server/patch/PatchDetailAction.java
@@ -16,17 +16,22 @@ package com.google.gerrit.server.patch;
 
 import com.google.gerrit.client.data.AccountInfoCacheFactory;
 import com.google.gerrit.client.data.PatchLine;
+import com.google.gerrit.client.data.UnifiedPatchDetail;
+import com.google.gerrit.client.reviewdb.Account;
 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.ReviewDb;
 import com.google.gerrit.client.rpc.CorruptEntityException;
+import com.google.gerrit.client.rpc.NoSuchEntityException;
+import com.google.gerrit.client.rpc.RpcUtil;
 import com.google.gerrit.client.rpc.BaseServiceImplementation.Action;
 import com.google.gerrit.client.rpc.BaseServiceImplementation.Failure;
 import com.google.gwtorm.client.OrmException;
 import com.google.gwtorm.client.ResultSet;
 
 import org.spearce.jgit.lib.Constants;
+import org.spearce.jgit.patch.CombinedFileHeader;
 import org.spearce.jgit.patch.FileHeader;
 import org.spearce.jgit.patch.FormatError;
 
@@ -37,11 +42,50 @@ import java.util.List;
 
 abstract class PatchDetailAction<T> implements Action<T> {
   protected final Patch.Id key;
+  protected Patch patch;
+  protected FileHeader file;
+  protected int fileCnt;
+  protected AccountInfoCacheFactory accountInfo;
+  protected Account.Id me;
+
+  protected HashMap<Integer, List<PatchLineComment>> published[];
+  protected HashMap<Integer, List<PatchLineComment>> drafted[];
 
   PatchDetailAction(Patch.Id key) {
     this.key = key;
   }
 
+  protected void init(final ReviewDb db) throws OrmException, Failure {
+    patch = db.patches().get(key);
+    if (patch == null) {
+      throw new Failure(new NoSuchEntityException());
+    }
+
+    FileHeader file = parse(patch, read(db, patch));
+    if (file instanceof CombinedFileHeader) {
+      fileCnt = ((CombinedFileHeader) file).getParentCount() + 1;
+    } else {
+      fileCnt = 2;
+    }
+
+    accountInfo = new AccountInfoCacheFactory(db);
+    me = RpcUtil.getAccountId();
+
+    published = new HashMap[fileCnt];
+    for (int n = 0; n < fileCnt; n++) {
+      published[n] = new HashMap<Integer, List<PatchLineComment>>();
+    }
+    indexComments(published, db.patchComments().published(key));
+
+    if (me != null) {
+      drafted = new HashMap[fileCnt];
+      for (int n = 0; n < fileCnt; n++) {
+        drafted[n] = new HashMap<Integer, List<PatchLineComment>>();
+      }
+      indexComments(drafted, db.patchComments().draft(key, me));
+    }
+  }
+
   protected static void indexComments(
       final HashMap<Integer, List<PatchLineComment>>[] out,
       final ResultSet<PatchLineComment> comments) {
@@ -58,8 +102,7 @@ abstract class PatchDetailAction<T> implements Action<T> {
     }
   }
 
-  protected static void addComments(final AccountInfoCacheFactory accountInfo,
-      final PatchLine pLine,
+  protected void addComments(final PatchLine pLine,
       final HashMap<Integer, List<PatchLineComment>>[] cache, final int side,
       final int line) {
     List<PatchLineComment> l = cache[side].get(line);
diff --git a/webapp/src/com/google/gerrit/server/patch/UnifiedPatchDetailAction.java b/webapp/src/com/google/gerrit/server/patch/UnifiedPatchDetailAction.java
index b380d4e6b4..0fa5ed1473 100644
--- a/webapp/src/com/google/gerrit/server/patch/UnifiedPatchDetailAction.java
+++ b/webapp/src/com/google/gerrit/server/patch/UnifiedPatchDetailAction.java
@@ -17,27 +17,18 @@ package com.google.gerrit.server.patch;
 import static org.spearce.jgit.util.RawParseUtils.decode;
 import static org.spearce.jgit.util.RawParseUtils.nextLF;
 
-import com.google.gerrit.client.data.AccountInfoCacheFactory;
 import com.google.gerrit.client.data.PatchLine;
 import com.google.gerrit.client.data.UnifiedPatchDetail;
 import com.google.gerrit.client.data.PatchLine.Type;
-import com.google.gerrit.client.reviewdb.Account;
 import com.google.gerrit.client.reviewdb.Patch;
-import com.google.gerrit.client.reviewdb.PatchLineComment;
 import com.google.gerrit.client.reviewdb.ReviewDb;
-import com.google.gerrit.client.rpc.NoSuchEntityException;
-import com.google.gerrit.client.rpc.RpcUtil;
 import com.google.gerrit.client.rpc.BaseServiceImplementation.Failure;
 import com.google.gwtorm.client.OrmException;
 
 import org.spearce.jgit.lib.Constants;
-import org.spearce.jgit.patch.CombinedFileHeader;
-import org.spearce.jgit.patch.FileHeader;
 import org.spearce.jgit.patch.HunkHeader;
 
 import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
 
 class UnifiedPatchDetailAction extends PatchDetailAction<UnifiedPatchDetail> {
   UnifiedPatchDetailAction(final Patch.Id key) {
@@ -45,51 +36,20 @@ class UnifiedPatchDetailAction extends PatchDetailAction<UnifiedPatchDetail> {
   }
 
   public UnifiedPatchDetail run(final ReviewDb db) throws OrmException, Failure {
-    final Patch patch = db.patches().get(key);
-    if (patch == null) {
-      throw new Failure(new NoSuchEntityException());
-    }
-
-    final FileHeader fh = parse(patch, read(db, patch));
-    final int fileCnt;
-    if (fh instanceof CombinedFileHeader) {
-      fileCnt = ((CombinedFileHeader) fh).getParentCount() + 1;
-    } else {
-      fileCnt = 2;
-    }
-
-    final AccountInfoCacheFactory acc = new AccountInfoCacheFactory(db);
-    final HashMap<Integer, List<PatchLineComment>> published[];
-    published = new HashMap[fileCnt];
-    for (int n = 0; n < fileCnt; n++) {
-      published[n] = new HashMap<Integer, List<PatchLineComment>>();
-    }
-    indexComments(published, db.patchComments().published(key));
-
-    final Account.Id me = RpcUtil.getAccountId();
-    final HashMap<Integer, List<PatchLineComment>> drafted[];
-    if (me != null) {
-      drafted = new HashMap[fileCnt];
-      for (int n = 0; n < fileCnt; n++) {
-        drafted[n] = new HashMap<Integer, List<PatchLineComment>>();
-      }
-      indexComments(drafted, db.patchComments().draft(key, me));
-    } else {
-      drafted = null;
-    }
+    init(db);
 
-    final byte[] buf = fh.getBuffer();
-    int ptr = fh.getStartOffset();
-    final int end = fh.getEndOffset();
+    final byte[] buf = file.getBuffer();
+    int ptr = file.getStartOffset();
+    final int end = file.getEndOffset();
     final int hdrEnd;
     final ArrayList<PatchLine> lines = new ArrayList<PatchLine>();
 
-    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();
+    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();
     } else {
       hdrEnd = end;
     }
@@ -101,7 +61,7 @@ class UnifiedPatchDetailAction extends PatchDetailAction<UnifiedPatchDetail> {
           buf, ptr, eol)));
     }
 
-    for (final HunkHeader h : fh.getHunks()) {
+    for (final HunkHeader h : file.getHunks()) {
       final int hunkEnd = h.getEndOffset();
       if (ptr < hunkEnd) {
         eol = nextLF(buf, ptr);
@@ -140,18 +100,18 @@ class UnifiedPatchDetailAction extends PatchDetailAction<UnifiedPatchDetail> {
         final PatchLine pLine =
             new PatchLine(oldLine, newLine, type, decode(Constants.CHARSET,
                 buf, ptr, eol));
-        addComments(acc, pLine, published, 0, oldLine);
-        addComments(acc, pLine, published, 1, newLine);
+        addComments(pLine, published, 0, oldLine);
+        addComments(pLine, published, 1, newLine);
         if (drafted != null) {
-          addComments(acc, pLine, drafted, 0, oldLine);
-          addComments(acc, pLine, drafted, 1, newLine);
+          addComments(pLine, drafted, 0, oldLine);
+          addComments(pLine, drafted, 1, newLine);
         }
         lines.add(pLine);
       }
     }
 
     final UnifiedPatchDetail d;
-    d = new UnifiedPatchDetail(patch, acc.create());
+    d = new UnifiedPatchDetail(patch, accountInfo.create());
     d.setLines(lines);
     return d;
   }