Extract Soy-independent commit prep into a CommitData class

Change-Id: Id05860abb3662e3ba7a2f6471e9777ee2b64fb7f
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/CommitData.java b/gitiles-servlet/src/main/java/com/google/gitiles/CommitData.java
new file mode 100644
index 0000000..1c118dd
--- /dev/null
+++ b/gitiles-servlet/src/main/java/com/google/gitiles/CommitData.java
@@ -0,0 +1,255 @@
+// Copyright (C) 2014 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.gitiles;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
+
+import com.google.common.base.Function;
+import com.google.common.base.Objects;
+import com.google.common.base.Predicate;
+import com.google.common.collect.FluentIterable;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Ordering;
+
+import org.eclipse.jgit.diff.DiffEntry;
+import org.eclipse.jgit.diff.DiffFormatter;
+import org.eclipse.jgit.http.server.ServletUtils;
+import org.eclipse.jgit.lib.AbbreviatedObjectId;
+import org.eclipse.jgit.lib.AnyObjectId;
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectReader;
+import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.treewalk.AbstractTreeIterator;
+import org.eclipse.jgit.treewalk.CanonicalTreeParser;
+import org.eclipse.jgit.treewalk.EmptyTreeIterator;
+import org.eclipse.jgit.util.io.NullOutputStream;
+
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import javax.annotation.Nullable;
+import javax.servlet.http.HttpServletRequest;
+
+/** Format-independent data about a single commit. */
+class CommitData {
+  static enum Field {
+    ABBREV_SHA,
+    ARCHIVE_TYPE,
+    ARCHIVE_URL,
+    AUTHOR,
+    BRANCHES,
+    COMMITTER,
+    DIFF_TREE,
+    LOG_URL,
+    MESSAGE,
+    PARENTS,
+    SHA,
+    SHORT_MESSAGE,
+    TAGS,
+    TREE,
+    TREE_URL,
+    URL;
+  }
+
+  static class DiffList {
+    Revision oldRevision;
+    List<DiffEntry> entries;
+  }
+
+  static class Builder {
+    private RevWalk walk;
+    private ArchiveFormat archiveFormat;
+    private Map<AnyObjectId, Set<Ref>> refsById;
+
+    Builder setRevWalk(@Nullable RevWalk walk) {
+      this.walk = walk;
+      return this;
+    }
+
+    Builder setArchiveFormat(@Nullable ArchiveFormat archiveFormat) {
+      this.archiveFormat = archiveFormat;
+      return this;
+    }
+
+    CommitData build(HttpServletRequest req, RevCommit c, Set<Field> fs)
+        throws IOException {
+      checkFields(fs);
+      checkNotNull(req, "request");
+      Repository repo = ServletUtils.getRepository(req);
+      GitilesView view = ViewFilter.getView(req);
+
+      CommitData result = new CommitData();
+
+      if (fs.contains(Field.AUTHOR)) {
+        result.author = c.getAuthorIdent();
+      }
+      if (fs.contains(Field.COMMITTER)) {
+        result.committer = c.getCommitterIdent();
+      }
+      if (fs.contains(Field.SHA)) {
+        result.sha = c.copy();
+      }
+      if (fs.contains(Field.ABBREV_SHA)) {
+        ObjectReader reader = repo.getObjectDatabase().newReader();
+        try {
+          result.abbrev = reader.abbreviate(c);
+        } finally {
+          reader.release();
+        }
+      }
+      if (fs.contains(Field.URL)) {
+        result.url = GitilesView.revision()
+            .copyFrom(view)
+            .setRevision(c)
+            .toUrl();
+      }
+      if (fs.contains(Field.LOG_URL)) {
+        result.logUrl = urlFromView(view, c, GitilesView.log());
+      }
+      if (fs.contains(Field.ARCHIVE_URL)) {
+        result.archiveUrl = urlFromView(view, c,
+            GitilesView.archive().setExtension(archiveFormat.getDefaultSuffix()));
+      }
+      if (fs.contains(Field.ARCHIVE_TYPE)) {
+        result.archiveType = archiveFormat;
+      }
+      if (fs.contains(Field.TREE)) {
+        result.tree = c.getTree().copy();
+      }
+      if (fs.contains(Field.TREE_URL)) {
+        result.treeUrl = GitilesView.path().copyFrom(view).setPathPart("/").toUrl();
+      }
+      if (fs.contains(Field.PARENTS)) {
+        result.parents = Arrays.asList(c.getParents());
+      }
+      if (fs.contains(Field.SHORT_MESSAGE)) {
+        result.shortMessage = c.getShortMessage();
+      }
+      if (fs.contains(Field.BRANCHES)) {
+        result.branches = getRefsById(repo, c, Constants.R_HEADS);
+      }
+      if (fs.contains(Field.TAGS)) {
+        result.tags = getRefsById(repo, c, Constants.R_TAGS);
+      }
+      if (fs.contains(Field.MESSAGE)) {
+        result.message = c.getFullMessage();
+      }
+      if (fs.contains(Field.DIFF_TREE)) {
+        result.diffEntries = computeDiffEntries(repo, view, c);
+      }
+
+      return result;
+    }
+
+    private void checkFields(Set<Field> fs) {
+      checkState(!fs.contains(Field.DIFF_TREE) || walk != null, "RevWalk required for diffTree");
+      if (fs.contains(Field.ARCHIVE_URL) || fs.contains(Field.ARCHIVE_TYPE)) {
+        checkState(archiveFormat != null, "archive format required");
+      }
+    }
+
+    private static String urlFromView(GitilesView view, RevCommit commit,
+        GitilesView.Builder builder) {
+      Revision rev = view.getRevision();
+      return builder.copyFrom(view)
+          .setRevision(rev.getId().equals(commit) ? rev.getName() : commit.name(), commit)
+          .setPathPart(null)
+          .toUrl();
+    }
+
+    private List<Ref> getRefsById(Repository repo, ObjectId id, final String prefix) {
+      if (refsById == null) {
+        refsById = repo.getAllRefsByPeeledObjectId();
+      }
+      return FluentIterable.from(Objects.firstNonNull(refsById.get(id), ImmutableSet.<Ref> of()))
+        .filter(new Predicate<Ref>() {
+          @Override
+          public boolean apply(Ref ref) {
+            return ref.getName().startsWith(prefix);
+          }
+        }).toSortedList(Ordering.natural().onResultOf(new Function<Ref, String>() {
+          @Override
+          public String apply(Ref ref) {
+            return ref.getName();
+          }
+        }));
+    }
+
+    private AbstractTreeIterator getTreeIterator(RevCommit commit) throws IOException {
+      CanonicalTreeParser p = new CanonicalTreeParser();
+      p.reset(walk.getObjectReader(), walk.parseTree(walk.parseCommit(commit).getTree()));
+      return p;
+    }
+
+    private DiffList computeDiffEntries(Repository repo, GitilesView view, RevCommit commit)
+        throws IOException {
+      DiffList result = new DiffList();
+      AbstractTreeIterator oldTree;
+      switch (commit.getParentCount()) {
+        case 0:
+          result.oldRevision = Revision.NULL;
+          oldTree = new EmptyTreeIterator();
+          break;
+        case 1:
+          result.oldRevision =
+              Revision.peeled(view.getRevision().getName() + "^", commit.getParent(0));
+          oldTree = getTreeIterator(commit.getParent(0));
+          break;
+        default:
+          // TODO(dborowitz): handle merges
+          return result;
+      }
+      AbstractTreeIterator newTree = getTreeIterator(commit);
+
+      DiffFormatter diff = new DiffFormatter(NullOutputStream.INSTANCE);
+      try {
+        diff.setRepository(repo);
+        diff.setDetectRenames(true);
+        result.entries = diff.scan(oldTree, newTree);
+        return result;
+      } finally {
+        diff.release();
+      }
+    }
+  }
+
+  ObjectId sha;
+  PersonIdent author;
+  PersonIdent committer;
+  AbbreviatedObjectId abbrev;
+  ObjectId tree;
+  List<RevCommit> parents;
+  String shortMessage;
+  String message;
+
+  List<Ref> branches;
+  List<Ref> tags;
+  DiffList diffEntries;
+
+  String url;
+  String logUrl;
+  String treeUrl;
+  String archiveUrl;
+  ArchiveFormat archiveType;
+}
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/CommitSoyData.java b/gitiles-servlet/src/main/java/com/google/gitiles/CommitSoyData.java
index 5afb1e7..973eede 100644
--- a/gitiles-servlet/src/main/java/com/google/gitiles/CommitSoyData.java
+++ b/gitiles-servlet/src/main/java/com/google/gitiles/CommitSoyData.java
@@ -14,202 +14,131 @@
 
 package com.google.gitiles;
 
-import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
 import static org.eclipse.jgit.diff.DiffEntry.ChangeType.COPY;
 import static org.eclipse.jgit.diff.DiffEntry.ChangeType.DELETE;
 import static org.eclipse.jgit.diff.DiffEntry.ChangeType.RENAME;
 
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
+import com.google.gitiles.CommitData.DiffList;
+import com.google.gitiles.CommitData.Field;
 import com.google.template.soy.data.restricted.NullData;
 
 import org.eclipse.jgit.diff.DiffEntry;
 import org.eclipse.jgit.diff.DiffEntry.ChangeType;
-import org.eclipse.jgit.diff.DiffFormatter;
-import org.eclipse.jgit.http.server.ServletUtils;
-import org.eclipse.jgit.lib.AnyObjectId;
 import org.eclipse.jgit.lib.Constants;
-import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.ObjectReader;
 import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.lib.Ref;
-import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevWalk;
-import org.eclipse.jgit.treewalk.AbstractTreeIterator;
-import org.eclipse.jgit.treewalk.CanonicalTreeParser;
-import org.eclipse.jgit.treewalk.EmptyTreeIterator;
 import org.eclipse.jgit.util.GitDateFormatter;
 import org.eclipse.jgit.util.RelativeDateFormatter;
-import org.eclipse.jgit.util.io.NullOutputStream;
 
 import java.io.IOException;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.Comparator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+import javax.annotation.Nullable;
 import javax.servlet.http.HttpServletRequest;
 
 /** Soy data converter for git commits. */
-public class CommitSoyData {
-  static enum Field {
-    ABBREV_SHA,
-    ARCHIVE_TYPE,
-    ARCHIVE_URL,
-    AUTHOR,
-    BRANCHES,
-    COMMITTER,
-    DIFF_TREE,
-    LOG_URL,
-    MESSAGE,
-    PARENTS,
-    SHA,
-    SHORT_MESSAGE,
-    TAGS,
-    TREE,
-    TREE_URL,
-    URL;
-  }
-
-  /** Valid sets of fields to include in data for commits. */
-  static enum FieldSet {
-    DETAIL(Field.AUTHOR, Field.COMMITTER, Field.SHA, Field.TREE, Field.TREE_URL, Field.PARENTS,
-        Field.MESSAGE, Field.LOG_URL, Field.ARCHIVE_URL, Field.ARCHIVE_TYPE),
-    DETAIL_DIFF_TREE(DETAIL, Field.DIFF_TREE),
-    SHORTLOG(Field.ABBREV_SHA, Field.URL, Field.SHORT_MESSAGE, Field.AUTHOR, Field.BRANCHES,
-        Field.TAGS),
-    DEFAULT(DETAIL);
-
-    private final ImmutableSet<Field> fields;
-
-    private FieldSet(Field... fields) {
-      this.fields = Sets.immutableEnumSet(Arrays.asList(fields));
-    }
-
-    private FieldSet(FieldSet other, Field... fields) {
-      this.fields = Sets.immutableEnumSet(Iterables.concat(other.fields, Arrays.asList(fields)));
-    }
-
-    private boolean contains(Field field) {
-      return fields.contains(field);
-    }
-  }
+class CommitSoyData {
+  static final ImmutableSet<Field> DEFAULT_FIELDS = Sets.immutableEnumSet(Field.AUTHOR,
+      Field.COMMITTER, Field.SHA, Field.TREE, Field.TREE_URL, Field.PARENTS, Field.MESSAGE,
+      Field.LOG_URL, Field.ARCHIVE_URL, Field.ARCHIVE_TYPE);
 
   private Linkifier linkifier;
-  private Repository repo;
   private RevWalk walk;
-  private GitilesView view;
-  private Map<AnyObjectId, Set<Ref>> refsById;
   private ArchiveFormat archiveFormat;
 
-  public CommitSoyData setLinkifier(Linkifier linkifier) {
-    this.linkifier = checkNotNull(linkifier, "linkifier");
+  CommitSoyData setLinkifier(@Nullable Linkifier linkifier) {
+    this.linkifier = linkifier;
     return this;
   }
 
-  public CommitSoyData setRevWalk(RevWalk walk) {
-    this.walk = checkNotNull(walk, "walk");
+  CommitSoyData setRevWalk(@Nullable RevWalk walk) {
+    this.walk = walk;
     return this;
   }
 
-  public CommitSoyData setArchiveFormat(ArchiveFormat archiveFormat) {
-    this.archiveFormat = checkNotNull(archiveFormat, "archiveFormat");
+  CommitSoyData setArchiveFormat(@Nullable ArchiveFormat archiveFormat) {
+    this.archiveFormat = archiveFormat;
     return this;
   }
 
-  public Map<String, Object> toSoyData(HttpServletRequest req, RevCommit commit, FieldSet fs,
+  Map<String, Object> toSoyData(HttpServletRequest req, RevCommit c, Set<Field> fs,
       GitDateFormatter df) throws IOException {
-    checkFields(fs);
-    checkNotNull(req, "request");
-    repo = ServletUtils.getRepository(req);
-    view = ViewFilter.getView(req);
+    GitilesView view = ViewFilter.getView(req);
+    CommitData cd = new CommitData.Builder()
+        .setRevWalk(walk)
+        .setArchiveFormat(archiveFormat)
+        .build(req, c, fs);
 
-    Map<String, Object> data = Maps.newHashMapWithExpectedSize(FieldSet.DEFAULT.fields.size());
-    if (fs.contains(Field.AUTHOR)) {
-      data.put("author", toSoyData(commit.getAuthorIdent(), df));
+    Map<String, Object> data = Maps.newHashMapWithExpectedSize(fs.size());
+    if (cd.author != null) {
+      data.put("author", toSoyData(cd.author, df));
     }
-    if (fs.contains(Field.COMMITTER)) {
-      data.put("committer", toSoyData(commit.getCommitterIdent(), df));
+    if (cd.committer != null) {
+      data.put("committer", toSoyData(cd.committer, df));
     }
-    if (fs.contains(Field.SHA)) {
-      data.put("sha", ObjectId.toString(commit));
+    if (cd.sha != null) {
+      data.put("sha", cd.sha.name());
     }
-    if (fs.contains(Field.ABBREV_SHA)) {
-      ObjectReader reader = repo.getObjectDatabase().newReader();
-      try {
-        data.put("abbrevSha", reader.abbreviate(commit).name());
-      } finally {
-        reader.release();
-      }
+    if (cd.abbrev != null) {
+      data.put("abbrevSha", cd.abbrev.name());
     }
-    if (fs.contains(Field.URL)) {
-      data.put("url", GitilesView.revision()
-          .copyFrom(view)
-          .setRevision(commit)
-          .toUrl());
+    if (cd.url != null) {
+      data.put("url", cd.url);
     }
-    if (fs.contains(Field.LOG_URL)) {
-      data.put("logUrl", urlFromView(view, commit, GitilesView.log()));
+    if (cd.logUrl != null) {
+      data.put("logUrl", cd.logUrl);
     }
-    if (fs.contains(Field.ARCHIVE_URL)) {
-      data.put("archiveUrl", urlFromView(view, commit,
-          GitilesView.archive().setExtension(archiveFormat.getDefaultSuffix())));
+    if (cd.archiveUrl != null) {
+      data.put("archiveUrl", cd.archiveUrl);
     }
-    if (fs.contains(Field.ARCHIVE_TYPE)) {
-      data.put("archiveType", archiveFormat.getShortName());
+    if (cd.archiveType != null) {
+      data.put("archiveType", cd.archiveType.getShortName());
     }
-    if (fs.contains(Field.TREE)) {
-      data.put("tree", ObjectId.toString(commit.getTree()));
+    if (cd.tree != null) {
+      data.put("tree", cd.tree.name());
     }
-    if (fs.contains(Field.TREE_URL)) {
-      data.put("treeUrl", GitilesView.path().copyFrom(view).setPathPart("/").toUrl());
+    if (cd.treeUrl != null) {
+      data.put("treeUrl", cd.treeUrl);
     }
-    if (fs.contains(Field.PARENTS)) {
-      data.put("parents", toSoyData(view, commit.getParents()));
+    if (cd.parents != null) {
+      data.put("parents", toSoyData(view, cd.parents));
     }
-    if (fs.contains(Field.SHORT_MESSAGE)) {
-      data.put("shortMessage", commit.getShortMessage());
+    if (cd.shortMessage != null) {
+      data.put("shortMessage", cd.shortMessage);
     }
-    if (fs.contains(Field.BRANCHES)) {
-      data.put("branches", getRefsById(commit, Constants.R_HEADS));
+    if (cd.branches != null) {
+      data.put("branches", toSoyData(view, cd.branches, Constants.R_HEADS));
     }
-    if (fs.contains(Field.TAGS)) {
-      data.put("tags", getRefsById(commit, Constants.R_TAGS));
+    if (cd.tags != null) {
+      data.put("tags", toSoyData(view, cd.tags, Constants.R_TAGS));
     }
-    if (fs.contains(Field.MESSAGE)) {
+    if (cd.message != null) {
       if (linkifier != null) {
-        data.put("message", linkifier.linkify(req, commit.getFullMessage()));
+        data.put("message", linkifier.linkify(req, cd.message));
       } else {
-        data.put("message", commit.getFullMessage());
+        data.put("message", cd.message);
       }
     }
-    if (fs.contains(Field.DIFF_TREE)) {
-      data.put("diffTree", computeDiffTree(commit));
+    if (cd.diffEntries != null) {
+      data.put("diffTree", toSoyData(view, cd.diffEntries));
     }
-    checkState(fs.fields.size() == data.size(), "bad commit data fields: %s != %s", fs.fields,
-        data.keySet());
+    checkState(fs.size() == data.size(), "bad commit data fields: %s != %s", fs, data.keySet());
     return ImmutableMap.copyOf(data);
   }
 
-  public Map<String, Object> toSoyData(HttpServletRequest req, RevCommit commit,
+  Map<String, Object> toSoyData(HttpServletRequest req, RevCommit commit,
       GitDateFormatter df) throws IOException {
-    return toSoyData(req, commit, FieldSet.DEFAULT, df);
-  }
-
-  private void checkFields(FieldSet fs) {
-    checkState(!fs.contains(Field.DIFF_TREE) || walk != null, "RevWalk required for diffTree");
-    if (fs.contains(Field.ARCHIVE_URL) || fs.contains(Field.ARCHIVE_TYPE)) {
-      checkState(archiveFormat != null, "archive format required");
-    }
+    return toSoyData(req, commit, DEFAULT_FIELDS, df);
   }
 
   // TODO(dborowitz): Extract this.
@@ -222,8 +151,8 @@
         "relativeTime", RelativeDateFormatter.format(ident.getWhen()));
   }
 
-  private List<Map<String, String>> toSoyData(GitilesView view, RevCommit[] parents) {
-    List<Map<String, String>> result = Lists.newArrayListWithCapacity(parents.length);
+  private List<Map<String, String>> toSoyData(GitilesView view, List<RevCommit> parents) {
+    List<Map<String, String>> result = Lists.newArrayListWithCapacity(parents.size());
     int i = 1;
     // TODO(dborowitz): Render something slightly different when we're actively
     // viewing a diff against one of the parents.
@@ -231,7 +160,7 @@
       String name = parent.name();
       GitilesView.Builder diff = GitilesView.diff().copyFrom(view).setPathPart("");
       String parentName;
-      if (parents.length == 1) {
+      if (parents.size() == 1) {
         parentName = view.getRevision().getName() + "^";
       } else {
         parentName = view.getRevision().getName() + "^" + (i++);
@@ -247,85 +176,43 @@
     return result;
   }
 
-  private AbstractTreeIterator getTreeIterator(RevWalk walk, RevCommit commit) throws IOException {
-    CanonicalTreeParser p = new CanonicalTreeParser();
-    p.reset(walk.getObjectReader(), walk.parseTree(walk.parseCommit(commit).getTree()));
-    return p;
-  }
-
-  private Object computeDiffTree(RevCommit commit) throws IOException {
-    AbstractTreeIterator oldTree;
+  private static Object toSoyData(GitilesView view, DiffList dl) {
+    if (dl.oldRevision == null) {
+      return NullData.INSTANCE;
+    }
     GitilesView.Builder diffUrl = GitilesView.diff().copyFrom(view)
         .setPathPart("");
-    Revision oldRevision;
-    switch (commit.getParentCount()) {
-      case 0:
-        oldTree = new EmptyTreeIterator();
-        oldRevision = Revision.NULL;
-        break;
-      case 1:
-        oldTree = getTreeIterator(walk, commit.getParent(0));
-        oldRevision = Revision.peeled(view.getRevision().getName() + "^", commit.getParent(0));
-        break;
-      default:
-        // TODO(dborowitz): handle merges
-        return NullData.INSTANCE;
-    }
-    diffUrl.setOldRevision(oldRevision);
-    AbstractTreeIterator newTree = getTreeIterator(walk, commit);
 
-    DiffFormatter diff = new DiffFormatter(NullOutputStream.INSTANCE);
-    try {
-      diff.setRepository(repo);
-      diff.setDetectRenames(true);
-
-      List<Object> result = Lists.newArrayList();
-      for (DiffEntry e : diff.scan(oldTree, newTree)) {
-        Map<String, Object> entry = Maps.newHashMapWithExpectedSize(5);
-        ChangeType type = e.getChangeType();
-        if (type != DELETE) {
-          entry.put("path", e.getNewPath());
-          entry.put("url", GitilesView.path()
-              .copyFrom(view)
-              .setPathPart(e.getNewPath())
-              .toUrl());
-        } else {
-          entry.put("path", e.getOldPath());
-          entry.put("url", GitilesView.path()
-              .copyFrom(view)
-              .setRevision(oldRevision)
-              .setPathPart(e.getOldPath())
-              .toUrl());
-        }
-        entry.put("diffUrl", diffUrl.setAnchor("F" + result.size()).toUrl());
-        entry.put("changeType", e.getChangeType().toString());
-        if (type == COPY || type == RENAME) {
-          entry.put("oldPath", e.getOldPath());
-        }
-        result.add(entry);
+    List<Object> result = Lists.newArrayListWithCapacity(dl.entries.size());
+    for (DiffEntry e : dl.entries) {
+      Map<String, Object> entry = Maps.newHashMapWithExpectedSize(5);
+      ChangeType type = e.getChangeType();
+      if (type != DELETE) {
+        entry.put("path", e.getNewPath());
+        entry.put("url", GitilesView.path()
+            .copyFrom(view)
+            .setPathPart(e.getNewPath())
+            .toUrl());
+      } else {
+        entry.put("path", e.getOldPath());
+        entry.put("url", GitilesView.path()
+            .copyFrom(view)
+            .setRevision(dl.oldRevision)
+            .setPathPart(e.getOldPath())
+            .toUrl());
       }
-      return result;
-    } finally {
-      diff.release();
+      entry.put("diffUrl", diffUrl.setAnchor("F" + result.size()).toUrl());
+      entry.put("changeType", e.getChangeType().toString());
+      if (type == COPY || type == RENAME) {
+        entry.put("oldPath", e.getOldPath());
+      }
+      result.add(entry);
     }
+    return result;
   }
 
-  private static final Comparator<Map<String, String>> NAME_COMPARATOR =
-      new Comparator<Map<String, String>>() {
-        @Override
-        public int compare(Map<String, String> o1, Map<String, String> o2) {
-          return o1.get("name").compareTo(o2.get("name"));
-        }
-      };
-
-  private List<Map<String, String>> getRefsById(ObjectId id, String prefix) {
-    if (refsById == null) {
-      refsById = repo.getAllRefsByPeeledObjectId();
-    }
-    Set<Ref> refs = refsById.get(id);
-    if (refs == null) {
-      return ImmutableList.of();
-    }
+  private static List<Map<String, String>> toSoyData(GitilesView view, List<Ref> refs,
+      String prefix) {
     List<Map<String, String>> result = Lists.newArrayListWithCapacity(refs.size());
     for (Ref ref : refs) {
       if (ref.getName().startsWith(prefix)) {
@@ -337,15 +224,6 @@
               .toUrl()));
       }
     }
-    Collections.sort(result, NAME_COMPARATOR);
     return result;
   }
-
-  private String urlFromView(GitilesView view, RevCommit commit, GitilesView.Builder builder) {
-    Revision rev = view.getRevision();
-    return builder.copyFrom(view)
-        .setRevision(rev.getId().equals(commit) ? rev.getName() : commit.name(), commit)
-        .setPathPart(null)
-        .toUrl();
-  }
 }
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/LogSoyData.java b/gitiles-servlet/src/main/java/com/google/gitiles/LogSoyData.java
index 688a0f4..d266a1c 100644
--- a/gitiles-servlet/src/main/java/com/google/gitiles/LogSoyData.java
+++ b/gitiles-servlet/src/main/java/com/google/gitiles/LogSoyData.java
@@ -14,9 +14,11 @@
 
 package com.google.gitiles;
 
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
-import com.google.gitiles.CommitSoyData.FieldSet;
+import com.google.common.collect.Sets;
+import com.google.gitiles.CommitData.Field;
 
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.revwalk.RevCommit;
@@ -31,6 +33,9 @@
 import javax.servlet.http.HttpServletRequest;
 
 public class LogSoyData {
+  private static final ImmutableSet<Field> FIELDS = Sets.immutableEnumSet(Field.ABBREV_SHA,
+      Field.URL, Field.SHORT_MESSAGE, Field.AUTHOR, Field.BRANCHES, Field.TAGS);
+
   private final HttpServletRequest req;
   private final GitilesView view;
 
@@ -50,7 +55,7 @@
 
     List<Map<String, Object>> entries = Lists.newArrayListWithCapacity(paginator.getLimit());
     for (RevCommit c : paginator) {
-      entries.add(new CommitSoyData().toSoyData(req, c, FieldSet.SHORTLOG, df));
+      entries.add(new CommitSoyData().toSoyData(req, c, FIELDS, df));
     }
 
     data.put("entries", entries);
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/RevisionServlet.java b/gitiles-servlet/src/main/java/com/google/gitiles/RevisionServlet.java
index b6e2f64..ccdaa53 100644
--- a/gitiles-servlet/src/main/java/com/google/gitiles/RevisionServlet.java
+++ b/gitiles-servlet/src/main/java/com/google/gitiles/RevisionServlet.java
@@ -21,9 +21,13 @@
 import static org.eclipse.jgit.lib.Constants.OBJ_TAG;
 import static org.eclipse.jgit.lib.Constants.OBJ_TREE;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
-import com.google.gitiles.CommitSoyData.FieldSet;
+import com.google.common.collect.Sets;
+import com.google.gitiles.CommitData.Field;
 
 import org.eclipse.jgit.errors.IncorrectObjectTypeException;
 import org.eclipse.jgit.errors.MissingObjectException;
@@ -49,6 +53,9 @@
 
 /** Serves an HTML page with detailed information about a ref. */
 public class RevisionServlet extends BaseServlet {
+  private static final ImmutableSet<Field> COMMIT_FIELDS = Sets.immutableEnumSet(
+      Iterables.concat(CommitSoyData.DEFAULT_FIELDS, ImmutableList.of(Field.DIFF_TREE)));
+
   private static final long serialVersionUID = 1L;
   private static final Logger log = LoggerFactory.getLogger(RevisionServlet.class);
 
@@ -85,7 +92,7 @@
                       .setLinkifier(linkifier)
                       .setRevWalk(walk)
                       .setArchiveFormat(getArchiveFormat(accessFactory.forRequest(req)))
-                      .toSoyData(req, (RevCommit) obj, FieldSet.DETAIL_DIFF_TREE, df)));
+                      .toSoyData(req, (RevCommit) obj, COMMIT_FIELDS, df)));
               break;
             case OBJ_TREE:
               soyObjects.add(ImmutableMap.of(