From b86d995b4030dc5782d6d8fc7fa9d41d62f22a63 Mon Sep 17 00:00:00 2001
From: Bob Trahan <bob.trahan@gmail.com>
Date: Thu, 9 Aug 2012 09:27:45 -0700
Subject: [PATCH] Detect if a commit *really* doesn't exist and 4oh4 from
 Diffusion commit view

Summary: rather than showing an erroneous "we still parsing" message.

Test Plan: for each version control system, typed in a garbage URL and got a 4oh4. (note this actually fails for SVN -- see comment about how my code fails atm -- and DiffusionGitRequest seems to pick off this error in advance and returns an AphrontUsageException.)

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1624

Differential Revision: https://secure.phabricator.com/D3201
---
 src/__phutil_library_map__.php                |  8 ++++
 .../controller/DiffusionCommitController.php  | 11 +++--
 .../query/exists/DiffusionExistsQuery.php     | 29 ++++++++++++++
 .../query/exists/DiffusionGitExistsQuery.php  | 33 +++++++++++++++
 .../exists/DiffusionMercurialExistsQuery.php  | 33 +++++++++++++++
 .../query/exists/DiffusionSvnExistsQuery.php  | 40 +++++++++++++++++++
 6 files changed, 150 insertions(+), 4 deletions(-)
 create mode 100644 src/applications/diffusion/query/exists/DiffusionExistsQuery.php
 create mode 100644 src/applications/diffusion/query/exists/DiffusionGitExistsQuery.php
 create mode 100644 src/applications/diffusion/query/exists/DiffusionMercurialExistsQuery.php
 create mode 100644 src/applications/diffusion/query/exists/DiffusionSvnExistsQuery.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 726ed273f4..cb0e310c72 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -327,6 +327,7 @@ phutil_register_library_map(array(
     'DiffusionDiffController' => 'applications/diffusion/controller/DiffusionDiffController.php',
     'DiffusionDiffQuery' => 'applications/diffusion/query/diff/DiffusionDiffQuery.php',
     'DiffusionEmptyResultView' => 'applications/diffusion/view/DiffusionEmptyResultView.php',
+    'DiffusionExistsQuery' => 'applications/diffusion/query/exists/DiffusionExistsQuery.php',
     'DiffusionExternalController' => 'applications/diffusion/controller/DiffusionExternalController.php',
     'DiffusionFileContent' => 'applications/diffusion/data/DiffusionFileContent.php',
     'DiffusionFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionFileContentQuery.php',
@@ -337,6 +338,7 @@ phutil_register_library_map(array(
     'DiffusionGitCommitTagsQuery' => 'applications/diffusion/query/committags/DiffusionGitCommitTagsQuery.php',
     'DiffusionGitContainsQuery' => 'applications/diffusion/query/contains/DiffusionGitContainsQuery.php',
     'DiffusionGitDiffQuery' => 'applications/diffusion/query/diff/DiffusionGitDiffQuery.php',
+    'DiffusionGitExistsQuery' => 'applications/diffusion/query/exists/DiffusionGitExistsQuery.php',
     'DiffusionGitFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php',
     'DiffusionGitHistoryQuery' => 'applications/diffusion/query/history/DiffusionGitHistoryQuery.php',
     'DiffusionGitLastModifiedQuery' => 'applications/diffusion/query/lastmodified/DiffusionGitLastModifiedQuery.php',
@@ -359,6 +361,7 @@ phutil_register_library_map(array(
     'DiffusionMercurialCommitTagsQuery' => 'applications/diffusion/query/committags/DiffusionMercurialCommitTagsQuery.php',
     'DiffusionMercurialContainsQuery' => 'applications/diffusion/query/contains/DiffusionMercurialContainsQuery.php',
     'DiffusionMercurialDiffQuery' => 'applications/diffusion/query/diff/DiffusionMercurialDiffQuery.php',
+    'DiffusionMercurialExistsQuery' => 'applications/diffusion/query/exists/DiffusionMercurialExistsQuery.php',
     'DiffusionMercurialFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionMercurialFileContentQuery.php',
     'DiffusionMercurialHistoryQuery' => 'applications/diffusion/query/history/DiffusionMercurialHistoryQuery.php',
     'DiffusionMercurialLastModifiedQuery' => 'applications/diffusion/query/lastmodified/DiffusionMercurialLastModifiedQuery.php',
@@ -388,6 +391,7 @@ phutil_register_library_map(array(
     'DiffusionSvnCommitTagsQuery' => 'applications/diffusion/query/committags/DiffusionSvnCommitTagsQuery.php',
     'DiffusionSvnContainsQuery' => 'applications/diffusion/query/contains/DiffusionSvnContainsQuery.php',
     'DiffusionSvnDiffQuery' => 'applications/diffusion/query/diff/DiffusionSvnDiffQuery.php',
+    'DiffusionSvnExistsQuery' => 'applications/diffusion/query/exists/DiffusionSvnExistsQuery.php',
     'DiffusionSvnFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionSvnFileContentQuery.php',
     'DiffusionSvnHistoryQuery' => 'applications/diffusion/query/history/DiffusionSvnHistoryQuery.php',
     'DiffusionSvnLastModifiedQuery' => 'applications/diffusion/query/lastmodified/DiffusionSvnLastModifiedQuery.php',
@@ -1440,6 +1444,7 @@ phutil_register_library_map(array(
     'DiffusionDiffController' => 'DiffusionController',
     'DiffusionDiffQuery' => 'DiffusionQuery',
     'DiffusionEmptyResultView' => 'DiffusionView',
+    'DiffusionExistsQuery' => 'DiffusionQuery',
     'DiffusionExternalController' => 'DiffusionController',
     'DiffusionFileContentQuery' => 'DiffusionQuery',
     'DiffusionGitBranchQuery' => 'DiffusionBranchQuery',
@@ -1449,6 +1454,7 @@ phutil_register_library_map(array(
     'DiffusionGitCommitTagsQuery' => 'DiffusionCommitTagsQuery',
     'DiffusionGitContainsQuery' => 'DiffusionContainsQuery',
     'DiffusionGitDiffQuery' => 'DiffusionDiffQuery',
+    'DiffusionGitExistsQuery' => 'DiffusionExistsQuery',
     'DiffusionGitFileContentQuery' => 'DiffusionFileContentQuery',
     'DiffusionGitHistoryQuery' => 'DiffusionHistoryQuery',
     'DiffusionGitLastModifiedQuery' => 'DiffusionLastModifiedQuery',
@@ -1471,6 +1477,7 @@ phutil_register_library_map(array(
     'DiffusionMercurialCommitTagsQuery' => 'DiffusionCommitTagsQuery',
     'DiffusionMercurialContainsQuery' => 'DiffusionContainsQuery',
     'DiffusionMercurialDiffQuery' => 'DiffusionDiffQuery',
+    'DiffusionMercurialExistsQuery' => 'DiffusionExistsQuery',
     'DiffusionMercurialFileContentQuery' => 'DiffusionFileContentQuery',
     'DiffusionMercurialHistoryQuery' => 'DiffusionHistoryQuery',
     'DiffusionMercurialLastModifiedQuery' => 'DiffusionLastModifiedQuery',
@@ -1492,6 +1499,7 @@ phutil_register_library_map(array(
     'DiffusionSvnCommitTagsQuery' => 'DiffusionCommitTagsQuery',
     'DiffusionSvnContainsQuery' => 'DiffusionContainsQuery',
     'DiffusionSvnDiffQuery' => 'DiffusionDiffQuery',
+    'DiffusionSvnExistsQuery' => 'DiffusionExistsQuery',
     'DiffusionSvnFileContentQuery' => 'DiffusionFileContentQuery',
     'DiffusionSvnHistoryQuery' => 'DiffusionHistoryQuery',
     'DiffusionSvnLastModifiedQuery' => 'DiffusionLastModifiedQuery',
diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php
index 3086fb362b..f2ab9060ed 100644
--- a/src/applications/diffusion/controller/DiffusionCommitController.php
+++ b/src/applications/diffusion/controller/DiffusionCommitController.php
@@ -50,13 +50,16 @@ final class DiffusionCommitController extends DiffusionController {
     $commit = $drequest->loadCommit();
 
     if (!$commit) {
-      // TODO -- T1624 -- detect if this has actually not been parsed yet
-      // and show this UI if so, else 404
+      $query = DiffusionExistsQuery::newFromDiffusionRequest($drequest);
+      $exists = $query->loadExistentialData();
+      if (!$exists) {
+        return new Aphront404Response();
+      }
       return $this->buildStandardPageResponse(
         id(new AphrontErrorView())
         ->setTitle('Error displaying commit.')
-        ->appendChild('Failed to load the commit. The commit has not been '.
-                      'parsed yet.'),
+        ->appendChild('Failed to load the commit because the commit has not '.
+                      'been parsed yet.'),
           array('title' => 'Commit Still Parsing')
         );
     }
diff --git a/src/applications/diffusion/query/exists/DiffusionExistsQuery.php b/src/applications/diffusion/query/exists/DiffusionExistsQuery.php
new file mode 100644
index 0000000000..1e67a72856
--- /dev/null
+++ b/src/applications/diffusion/query/exists/DiffusionExistsQuery.php
@@ -0,0 +1,29 @@
+<?php
+
+/*
+ * Copyright 2012 Facebook, Inc.
+ *
+ * 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.
+ */
+
+abstract class DiffusionExistsQuery extends DiffusionQuery {
+
+  final public static function newFromDiffusionRequest(
+    DiffusionRequest $request) {
+    return self::newQueryObject(__CLASS__, $request);
+  }
+
+  final public function loadExistentialData() {
+    return $this->executeQuery();
+  }
+}
diff --git a/src/applications/diffusion/query/exists/DiffusionGitExistsQuery.php b/src/applications/diffusion/query/exists/DiffusionGitExistsQuery.php
new file mode 100644
index 0000000000..40cbf49e4b
--- /dev/null
+++ b/src/applications/diffusion/query/exists/DiffusionGitExistsQuery.php
@@ -0,0 +1,33 @@
+<?php
+
+/*
+ * Copyright 2012 Facebook, Inc.
+ *
+ * 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.
+ */
+
+final class DiffusionGitExistsQuery extends DiffusionExistsQuery {
+
+  final protected function executeQuery() {
+    $request    = $this->getRequest();
+    $repository = $request->getRepository();
+
+    list($err, $merge_base) = $repository->execLocalCommand(
+      'cat-file -t %s',
+      $request->getCommit()
+    );
+
+    return !$err;
+  }
+
+}
diff --git a/src/applications/diffusion/query/exists/DiffusionMercurialExistsQuery.php b/src/applications/diffusion/query/exists/DiffusionMercurialExistsQuery.php
new file mode 100644
index 0000000000..ea28beb20a
--- /dev/null
+++ b/src/applications/diffusion/query/exists/DiffusionMercurialExistsQuery.php
@@ -0,0 +1,33 @@
+<?php
+
+/*
+ * Copyright 2012 Facebook, Inc.
+ *
+ * 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.
+ */
+
+final class DiffusionMercurialExistsQuery extends DiffusionExistsQuery {
+
+  protected function executeQuery() {
+    $request    = $this->getRequest();
+    $repository = $request->getRepository();
+
+    list($err, $stdout) = $repository->execLocalCommand(
+      'id --rev %s',
+      $request->getCommit()
+    );
+
+    return !$err;
+
+  }
+}
diff --git a/src/applications/diffusion/query/exists/DiffusionSvnExistsQuery.php b/src/applications/diffusion/query/exists/DiffusionSvnExistsQuery.php
new file mode 100644
index 0000000000..33da27facd
--- /dev/null
+++ b/src/applications/diffusion/query/exists/DiffusionSvnExistsQuery.php
@@ -0,0 +1,40 @@
+<?php
+
+/*
+ * Copyright 2012 Facebook, Inc.
+ *
+ * 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.
+ */
+
+final class DiffusionSvnExistsQuery extends DiffusionExistsQuery {
+
+  protected function executeQuery() {
+    $request    = $this->getRequest();
+    $repository = $request->getRepository();
+
+    list($info) = $repository->execxRemoteCommand(
+      'info %s',
+      $repository->getRemoteURI()
+    );
+
+    $matches = null;
+    $exists  = false;
+    if (preg_match('/^Revision: (\d+)$/m', $info, $matches)) {
+      $base_revision = $matches[1];
+      $exists = $base_revision >= $request->getCommit();
+    }
+
+    return $exists;
+  }
+
+}