In ViewFilter, if a revision cannot be found then throw an exception
instead of returning null.

Change-Id: I9c00409c67688d319f6c440afa4ad73fb5ecb7a9

java/com/google/gitiles/ViewFilter.java

diff --git a/java/com/google/gitiles/ViewFilter.java b/java/com/google/gitiles/ViewFilter.java
index 6f124e5..e3bb71f 100644
--- a/java/com/google/gitiles/ViewFilter.java
+++ b/java/com/google/gitiles/ViewFilter.java
@@ -194,7 +194,7 @@
       return null;
     }
     RevisionParser.Result result = parseRevision(req, path);
-    if (result == null || result.getOldRevision() != null) {
+    if (result.getOldRevision() != null) {
       return null;
     }
     return GitilesView.archive()
@@ -212,9 +212,6 @@
       return null;
     }
     RevisionParser.Result result = parseRevision(req, path);
-    if (result == null) {
-      return null;
-    }
     if (result.getOldRevision() != null) {
       return parseDiffCommand(repoName, result);
     }
@@ -234,7 +231,7 @@
       return null;
     }
     RevisionParser.Result result = parseRevision(req, path);
-    if (result == null || result.getOldRevision() != null || result.getPath().isEmpty()) {
+    if (result.getOldRevision() != null || result.getPath().isEmpty()) {
       return null;
     }
     return GitilesView.blame()
@@ -257,9 +254,6 @@
 
   private @Nullable GitilesView.Builder parseDiffCommand(
       String repoName, RevisionParser.Result result) {
-    if (result == null) {
-      return null;
-    }
     return GitilesView.diff()
         .setRepositoryName(repoName)
         .setRevision(result.getRevision())
@@ -273,9 +267,6 @@
       return GitilesView.log().setRepositoryName(repoName);
     }
     RevisionParser.Result result = parseRevision(req, path);
-    if (result == null) {
-      return null;
-    }
     return GitilesView.log()
         .setRepositoryName(repoName)
         .setRevision(result.getRevision())
@@ -294,7 +285,7 @@
 
   private @Nullable GitilesView.Builder parseShowCommand(
       String repoName, RevisionParser.Result result) {
-    if (result == null || result.getOldRevision() != null) {
+    if (result.getOldRevision() != null) {
       return null;
     }
     if (result.getPath().isEmpty()) {
@@ -313,7 +304,7 @@
 
   private @Nullable GitilesView.Builder parseDocCommand(
       String repoName, RevisionParser.Result result) {
-    if (result == null || result.getOldRevision() != null) {
+    if (result.getOldRevision() != null) {
       return null;
     }
     GitilesView.Builder b =
@@ -332,7 +323,11 @@
             accessFactory.forRequest(req),
             visibilityCache,
             getBranchRedirect(req));
-    return revParser.parse(checkLeadingSlash(path));
+    RevisionParser.Result rev = revParser.parse(checkLeadingSlash(path));
+    if (rev == null) {
+      throw new GitilesRequestFailureException(FailureReason.OBJECT_NOT_FOUND);
+    }
+    return rev;
   }
 
   private BranchRedirect getBranchRedirect(HttpServletRequest req) {

javatests/com/google/gitiles/ViewFilterTest.java

diff --git a/javatests/com/google/gitiles/ViewFilterTest.java b/javatests/com/google/gitiles/ViewFilterTest.java
index 79b0b09..dff1de2 100644
--- a/javatests/com/google/gitiles/ViewFilterTest.java
+++ b/javatests/com/google/gitiles/ViewFilterTest.java
@@ -17,6 +17,7 @@
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth.assertWithMessage;
 import static com.google.gitiles.MoreAssert.assertThrows;
+import static java.lang.String.format;
 
 import com.google.common.net.HttpHeaders;
 import java.io.IOException;
@@ -239,6 +240,11 @@
     assertThat(view.getRevision().getId()).isEqualTo(master);
     assertThat(view.getPathPart()).isEqualTo("");
 
+    view = getView(format("/repo/+show/%s/", master.getId().name()));
+    assertThat(view.getType()).isEqualTo(GitilesView.Type.PATH);
+    assertThat(view.getRevision().getId()).isEqualTo(master);
+    assertThat(view.getPathPart()).isEqualTo("");
+
     view = getView("/repo/+show/master/foo");
     assertThat(view.getType()).isEqualTo(GitilesView.Type.PATH);
     assertThat(view.getRevision().getId()).isEqualTo(master);
@@ -259,6 +265,18 @@
   }
 
   @Test
+  public void revisionNotFound() throws Exception {
+    var exception = assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+show/0123456789abcdef"));
+    assertThat(exception.getReason()).isEqualTo(GitilesRequestFailureException.FailureReason.OBJECT_NOT_FOUND);
+  }
+
+  @Test
+  public void cannotParse() throws Exception {
+    var exception = assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+showmaster"));
+    assertThat(exception.getReason()).isEqualTo(GitilesRequestFailureException.FailureReason.CANNOT_PARSE_GITILES_VIEW);
+  }
+
+  @Test
   public void doc() throws Exception {
     RevCommit master = repo.branch(MASTER).commit().create();
     repo.branch("refs/heads/stable").commit().create();