Allow /+archive of any tree-ish

Change-Id: I06b0fe636fd475d9d99392ad7404893e286c1cd9

gitiles-servlet/src/main/java/com/google/gitiles/ArchiveServlet.java

diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/ArchiveServlet.java b/gitiles-servlet/src/main/java/com/google/gitiles/ArchiveServlet.java
index 2534d82..b0ab8fb 100644
--- a/gitiles-servlet/src/main/java/com/google/gitiles/ArchiveServlet.java
+++ b/gitiles-servlet/src/main/java/com/google/gitiles/ArchiveServlet.java
@@ -22,8 +22,12 @@
 import org.eclipse.jgit.errors.IncorrectObjectTypeException;
 import org.eclipse.jgit.http.server.ServletUtils;
 import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.FileMode;
+import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevTree;
 import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.treewalk.TreeWalk;
 
 import java.io.IOException;
 import java.util.Map;
@@ -49,17 +53,10 @@
     Revision rev = view.getRevision();
     Repository repo = ServletUtils.getRepository(req);
 
-    // Check object type before starting the archive. If we just caught the
-    // exception from cmd.call() below, we wouldn't know whether it was because
-    // the input object is not a tree or something broke later.
-    RevWalk walk = new RevWalk(repo);
-    try {
-      walk.parseTree(rev.getId());
-    } catch (IncorrectObjectTypeException e) {
+    ObjectId treeId = getTree(view, repo, rev);
+    if (treeId.equals(ObjectId.zeroId())) {
       res.sendError(SC_NOT_FOUND);
       return;
-    } finally {
-      walk.release();
     }
 
     ArchiveFormat format = byExt.get(view.getExtension());
@@ -70,7 +67,7 @@
     try {
       new ArchiveCommand(repo)
           .setFormat(format.name())
-          .setTree(rev.getId())
+          .setTree(treeId)
           .setOutputStream(res.getOutputStream())
           .call();
     } catch (GitAPIException e) {
@@ -78,12 +75,35 @@
     }
   }
 
+  private ObjectId getTree(GitilesView view, Repository repo, Revision rev) throws IOException {
+    RevWalk rw = new RevWalk(repo);
+    try {
+      RevTree tree = rw.parseTree(rev.getId());
+      if (view.getPathPart() == null) {
+        return tree;
+      }
+      TreeWalk tw = TreeWalk.forPath(rw.getObjectReader(), view.getPathPart(), tree);
+      if (tw.getFileMode(0) != FileMode.TREE) {
+        return ObjectId.zeroId();
+      }
+      return tw.getObjectId(0);
+    } catch (IncorrectObjectTypeException e) {
+      return ObjectId.zeroId();
+    } finally {
+      rw.release();
+    }
+  }
+
   private String getFilename(GitilesView view, Revision rev, String ext) {
-    return new StringBuilder()
+    StringBuilder sb = new StringBuilder()
         .append(Paths.basename(view.getRepositoryName()))
         .append('-')
-        .append(rev.getName())
-        .append(ext)
+        .append(rev.getName());
+    if (view.getPathPart() != null) {
+      sb.append('-')
+          .append(view.getPathPart().replace('/', '-'));
+    }
+    return sb.append(ext)
         .toString();
   }
 }

gitiles-servlet/src/main/java/com/google/gitiles/GitilesView.java

diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/GitilesView.java b/gitiles-servlet/src/main/java/com/google/gitiles/GitilesView.java
index 3066462..50923bb 100644
--- a/gitiles-servlet/src/main/java/com/google/gitiles/GitilesView.java
+++ b/gitiles-servlet/src/main/java/com/google/gitiles/GitilesView.java
@@ -101,10 +101,10 @@
           oldRevision = other.oldRevision;
           // Fallthrough.
         case PATH:
+        case ARCHIVE:
           path = other.path;
           // Fallthrough.
         case REVISION:
-        case ARCHIVE:
           revision = other.revision;
           // Fallthrough.
         case DESCRIBE:
@@ -219,6 +219,7 @@
         case DIFF:
           checkState(path != null, "cannot set null path on %s view", type);
           break;
+        case ARCHIVE:
         case DESCRIBE:
         case REFS:
         case LOG:
@@ -547,8 +548,11 @@
         url.append(repositoryName).append("/+/").append(revision.getName());
         break;
       case ARCHIVE:
-        url.append(repositoryName).append("/+archive/").append(revision.getName())
-            .append(Objects.firstNonNull(extension, DEFAULT_ARCHIVE_EXTENSION));
+        url.append(repositoryName).append("/+archive/").append(revision.getName());
+        if (path != null) {
+          url.append('/').append(path);
+        }
+        url.append(Objects.firstNonNull(extension, DEFAULT_ARCHIVE_EXTENSION));
         break;
       case PATH:
         url.append(repositoryName).append("/+/").append(revision.getName()).append('/')

gitiles-servlet/src/main/java/com/google/gitiles/ViewFilter.java

diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/ViewFilter.java b/gitiles-servlet/src/main/java/com/google/gitiles/ViewFilter.java
index 582daca..50a4584 100644
--- a/gitiles-servlet/src/main/java/com/google/gitiles/ViewFilter.java
+++ b/gitiles-servlet/src/main/java/com/google/gitiles/ViewFilter.java
@@ -16,10 +16,18 @@
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
-
 import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
 import static javax.servlet.http.HttpServletResponse.SC_SERVICE_UNAVAILABLE;
 
+import com.google.common.base.Strings;
+import com.google.common.collect.Sets;
+
+import org.eclipse.jgit.http.server.ServletUtils;
+import org.eclipse.jgit.http.server.glue.WrappedRequest;
+import org.eclipse.jgit.lib.Config;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import java.io.IOException;
 import java.util.Map;
 import java.util.Set;
@@ -29,14 +37,6 @@
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
-import org.eclipse.jgit.http.server.ServletUtils;
-import org.eclipse.jgit.http.server.glue.WrappedRequest;
-import org.eclipse.jgit.lib.Config;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import com.google.common.collect.Sets;
-
 /** Filter to parse URLs and convert them to {@link GitilesView}s. */
 public class ViewFilter extends AbstractHttpFilter {
   private static final Logger log = LoggerFactory.getLogger(ViewFilter.class);
@@ -166,7 +166,7 @@
         break;
       }
     }
-    if (ext == null) {
+    if (ext == null || path.endsWith("/")) {
       return null;
     }
     RevisionParser.Result result = parseRevision(req, path);
@@ -176,6 +176,7 @@
     return GitilesView.archive()
         .setRepositoryName(repoName)
         .setRevision(result.getRevision())
+        .setPathPart(Strings.emptyToNull(result.getPath()))
         .setExtension(ext);
   }
 

gitiles-servlet/src/test/java/com/google/gitiles/GitilesViewTest.java

diff --git a/gitiles-servlet/src/test/java/com/google/gitiles/GitilesViewTest.java b/gitiles-servlet/src/test/java/com/google/gitiles/GitilesViewTest.java
index 3cc846e..44e69c3 100644
--- a/gitiles-servlet/src/test/java/com/google/gitiles/GitilesViewTest.java
+++ b/gitiles-servlet/src/test/java/com/google/gitiles/GitilesViewTest.java
@@ -516,6 +516,49 @@
         view.getBreadcrumbs());
   }
 
+  public void testArchiveWithNoPath() throws Exception {
+    ObjectId id = ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
+    GitilesView view = GitilesView.archive()
+        .copyFrom(HOST)
+        .setRepositoryName("foo/bar")
+        .setRevision(Revision.unpeeled("master", id))
+        .setExtension(".tar.bz2")
+        .build();
+
+    assertEquals("/b", view.getServletPath());
+    assertEquals(Type.ARCHIVE, view.getType());
+    assertEquals("host", view.getHostName());
+    assertEquals("foo/bar", view.getRepositoryName());
+    assertEquals(id, view.getRevision().getId());
+    assertEquals("master", view.getRevision().getName());
+    assertEquals(Revision.NULL, view.getOldRevision());
+    assertNull(view.getPathPart());
+    assertTrue(HOST.getParameters().isEmpty());
+    assertEquals("/b/foo/bar/+archive/master.tar.bz2", view.toUrl());
+  }
+
+  public void testArchiveWithPath() throws Exception {
+    ObjectId id = ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
+    GitilesView view = GitilesView.archive()
+        .copyFrom(HOST)
+        .setRepositoryName("foo/bar")
+        .setRevision(Revision.unpeeled("master", id))
+        .setPathPart("/path/to/a/dir")
+        .setExtension(".tar.bz2")
+        .build();
+
+    assertEquals("/b", view.getServletPath());
+    assertEquals(Type.ARCHIVE, view.getType());
+    assertEquals("host", view.getHostName());
+    assertEquals("foo/bar", view.getRepositoryName());
+    assertEquals(id, view.getRevision().getId());
+    assertEquals("master", view.getRevision().getName());
+    assertEquals(Revision.NULL, view.getOldRevision());
+    assertEquals("path/to/a/dir", view.getPathPart());
+    assertTrue(HOST.getParameters().isEmpty());
+    assertEquals("/b/foo/bar/+archive/master/path/to/a/dir.tar.bz2", view.toUrl());
+  }
+
   public void testEscaping() throws Exception {
     ObjectId id = ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
     ObjectId parent = ObjectId.fromString("efab5678efab5678efab5678efab5678efab5678");

gitiles-servlet/src/test/java/com/google/gitiles/ViewFilterTest.java

diff --git a/gitiles-servlet/src/test/java/com/google/gitiles/ViewFilterTest.java b/gitiles-servlet/src/test/java/com/google/gitiles/ViewFilterTest.java
index 201ac9d..4fddf0f 100644
--- a/gitiles-servlet/src/test/java/com/google/gitiles/ViewFilterTest.java
+++ b/gitiles-servlet/src/test/java/com/google/gitiles/ViewFilterTest.java
@@ -19,14 +19,9 @@
 import static com.google.gitiles.GitilesFilter.REPO_REGEX;
 import static com.google.gitiles.GitilesFilter.ROOT_REGEX;
 
-import java.io.IOException;
-import java.util.concurrent.atomic.AtomicReference;
-import java.util.regex.Pattern;
-
-import javax.servlet.ServletException;
-import javax.servlet.http.HttpServlet;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
+import com.google.common.collect.ImmutableList;
+import com.google.common.util.concurrent.Atomics;
+import com.google.gitiles.GitilesView.Type;
 
 import junit.framework.TestCase;
 
@@ -39,9 +34,14 @@
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.revwalk.RevCommit;
 
-import com.google.common.collect.ImmutableList;
-import com.google.common.util.concurrent.Atomics;
-import com.google.gitiles.GitilesView.Type;
+import java.io.IOException;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.regex.Pattern;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 
 /** Tests for the view filter. */
 public class ViewFilterTest extends TestCase {
@@ -391,6 +391,8 @@
     assertNull(getView("/repo/+archive/master..branch"));
     assertNull(getView("/repo/+archive/master.foo"));
     assertNull(getView("/repo/+archive/master.zip"));
+    assertNull(getView("/repo/+archive/master/.tar.gz"));
+    assertNull(getView("/repo/+archive/master/foo/.tar.gz"));
 
     view = getView("/repo/+archive/master.tar.gz");
     assertEquals(Type.ARCHIVE, view.getType());
@@ -409,6 +411,15 @@
     assertEquals(Revision.NULL, view.getOldRevision());
     assertEquals(".tar.bz2", view.getExtension());
     assertNull(view.getPathPart());
+
+    view = getView("/repo/+archive/master/foo/bar.tar.gz");
+    assertEquals(Type.ARCHIVE, view.getType());
+    assertEquals("repo", view.getRepositoryName());
+    assertEquals("master", view.getRevision().getName());
+    assertEquals(master, view.getRevision().getId());
+    assertEquals(Revision.NULL, view.getOldRevision());
+    assertEquals(".tar.gz", view.getExtension());
+    assertEquals("foo/bar", view.getPathPart());
   }
 
   private GitilesView getView(String pathAndQuery) throws ServletException, IOException {