Diffusion - move DiffusionExistsQuery to work over conduit

Summary: le title. However, this also "is the first" conversion so sets the precedence for how this will all work. See comments in the code. I think this helps us keep the new code we're writing to a minimum. Wondering if the conduit end point could be more generic, and rather than have a switch statement on VCS type, one can just implement the "handleSubversion" version and have that called?  Ref T2784

Test Plan: slapped an "or true" in the conditional protecting this code path. verified it worked on all 3 vcs systems, including typing in garbage and getting a 404

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2784

Differential Revision: https://secure.phabricator.com/D5803

src/__phutil_library_map__.php

@@ -144,6 +144,8 @@ phutil_register_library_map(array(
     'ConduitAPI_differential_updaterevision_Method' => 'applications/differential/conduit/ConduitAPI_differential_updaterevision_Method.php',
     'ConduitAPI_differential_updateunitresults_Method' => 'applications/differential/conduit/ConduitAPI_differential_updateunitresults_Method.php',
     'ConduitAPI_diffusion_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_Method.php',
+    'ConduitAPI_diffusion_abstractquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_abstractquery_Method.php',
+    'ConduitAPI_diffusion_existsquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_existsquery_Method.php',
     'ConduitAPI_diffusion_findsymbols_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_findsymbols_Method.php',
     'ConduitAPI_diffusion_getcommits_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_getcommits_Method.php',
     'ConduitAPI_diffusion_getlintmessages_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_getlintmessages_Method.php',
@@ -418,7 +420,6 @@ 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',
@@ -429,7 +430,6 @@ 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',
@@ -455,7 +455,6 @@ 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',
@@ -486,7 +485,6 @@ 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',
@@ -1913,6 +1911,8 @@ phutil_register_library_map(array(
     'ConduitAPI_differential_updaterevision_Method' => 'ConduitAPIMethod',
     'ConduitAPI_differential_updateunitresults_Method' => 'ConduitAPIMethod',
     'ConduitAPI_diffusion_Method' => 'ConduitAPIMethod',
+    'ConduitAPI_diffusion_abstractquery_Method' => 'ConduitAPI_diffusion_Method',
+    'ConduitAPI_diffusion_existsquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method',
     'ConduitAPI_diffusion_findsymbols_Method' => 'ConduitAPI_diffusion_Method',
     'ConduitAPI_diffusion_getcommits_Method' => 'ConduitAPI_diffusion_Method',
     'ConduitAPI_diffusion_getlintmessages_Method' => 'ConduitAPI_diffusion_Method',
@@ -2176,7 +2176,6 @@ phutil_register_library_map(array(
     'DiffusionDiffController' => 'DiffusionController',
     'DiffusionDiffQuery' => 'DiffusionQuery',
     'DiffusionEmptyResultView' => 'DiffusionView',
-    'DiffusionExistsQuery' => 'DiffusionQuery',
     'DiffusionExternalController' => 'DiffusionController',
     'DiffusionFileContentQuery' => 'DiffusionQuery',
     'DiffusionGitBranchQuery' => 'DiffusionBranchQuery',
@@ -2186,7 +2185,6 @@ phutil_register_library_map(array(
     'DiffusionGitCommitTagsQuery' => 'DiffusionCommitTagsQuery',
     'DiffusionGitContainsQuery' => 'DiffusionContainsQuery',
     'DiffusionGitDiffQuery' => 'DiffusionDiffQuery',
-    'DiffusionGitExistsQuery' => 'DiffusionExistsQuery',
     'DiffusionGitFileContentQuery' => 'DiffusionFileContentQuery',
     'DiffusionGitHistoryQuery' => 'DiffusionHistoryQuery',
     'DiffusionGitLastModifiedQuery' => 'DiffusionLastModifiedQuery',
@@ -2211,7 +2209,6 @@ phutil_register_library_map(array(
     'DiffusionMercurialCommitTagsQuery' => 'DiffusionCommitTagsQuery',
     'DiffusionMercurialContainsQuery' => 'DiffusionContainsQuery',
     'DiffusionMercurialDiffQuery' => 'DiffusionDiffQuery',
-    'DiffusionMercurialExistsQuery' => 'DiffusionExistsQuery',
     'DiffusionMercurialFileContentQuery' => 'DiffusionFileContentQuery',
     'DiffusionMercurialHistoryQuery' => 'DiffusionHistoryQuery',
     'DiffusionMercurialLastModifiedQuery' => 'DiffusionLastModifiedQuery',
@@ -2234,7 +2231,6 @@ phutil_register_library_map(array(
     'DiffusionSvnCommitTagsQuery' => 'DiffusionCommitTagsQuery',
     'DiffusionSvnContainsQuery' => 'DiffusionContainsQuery',
     'DiffusionSvnDiffQuery' => 'DiffusionDiffQuery',
-    'DiffusionSvnExistsQuery' => 'DiffusionExistsQuery',
     'DiffusionSvnFileContentQuery' => 'DiffusionFileContentQuery',
     'DiffusionSvnHistoryQuery' => 'DiffusionHistoryQuery',
     'DiffusionSvnLastModifiedQuery' => 'DiffusionLastModifiedQuery',

src/applications/diffusion/conduit/ConduitAPI_diffusion_abstractquery_Method.php

@@ -0,0 +1,101 @@
+<?php
+
+/**
+ * @group conduit
+ */
+abstract class ConduitAPI_diffusion_abstractquery_Method
+  extends ConduitAPI_diffusion_Method {
+
+  private $diffusionRequest;
+  protected function setDiffusionRequest(DiffusionRequest $request) {
+    $this->diffusionRequest = $request;
+    return $this;
+  }
+  protected function getDiffusionRequest() {
+    return $this->diffusionRequest;
+  }
+
+  final public function defineErrorTypes() {
+    return $this->defineCustomErrorTypes() +
+      array(
+        'ERR-UNKNOWN-REPOSITORY-VCS' =>
+          pht('Unknown repository VCS type.'),
+        'ERR-UNSUPPORTED-VCS' =>
+          pht('VCS is not supported for this method.'));
+  }
+  /**
+   * Subclasses should override this to specify custom error types.
+   */
+  protected function defineCustomErrorTypes() {
+    return array();
+  }
+
+  final public function defineParamTypes() {
+    return $this->defineCustomParamTypes() +
+      array(
+      'callsign' => 'required string');
+  }
+  /**
+   * Subclasses should override this to specify custom param types.
+   */
+  protected function defineCustomParamTypes() {
+    return array();
+  }
+
+  /**
+   * Subclasses should override these methods with the proper result for the
+   * pertinent version control system, e.g. getGitResult for Git.
+   *
+   * If the result is not supported for that VCS, do not implement it. e.g.
+   * Subversion (SVN) does not support branches.
+   */
+  protected function getGitResult(ConduitAPIRequest $request) {
+    throw new ConduitException('ERR-UNSUPPORTED-VCS');
+  }
+  protected function getSVNResult(ConduitAPIRequest $request) {
+    throw new ConduitException('ERR-UNSUPPORTED-VCS');
+  }
+  protected function getMercurialResult(ConduitAPIRequest $request) {
+    throw new ConduitException('ERR-UNSUPPORTED-VCS');
+  }
+
+  /**
+   * This method is final because each query will need to construct a
+   * @{class:DiffusionRequest} and use it. Consolidating this codepath and
+   * enforcing @{method:getDiffusionRequest} works when we need it is good.
+   *
+   * @{method:getResult} should be overridden by subclasses as necessary, e.g.
+   * there is a common operation across all version control systems that
+   * should occur after @{method:getResult}, like formatting a timestamp.
+   */
+  final protected function execute(ConduitAPIRequest $request) {
+    $drequest = DiffusionRequest::newFromDictionary(
+      array(
+        'callsign'  => $request->getValue('callsign'),
+      ));
+    $this->setDiffusionRequest($drequest);
+
+    return $this->getResult($request);
+  }
+
+  protected function getResult(ConduitAPIRequest $request) {
+    $drequest = $this->getDiffusionRequest();
+    $repository = $drequest->getRepository();
+    $result = null;
+    switch ($repository->getVersionControlSystem()) {
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
+        $result = $this->getGitResult($request);
+        break;
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
+        $result = $this->getMercurialResult($request);
+        break;
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
+        $result = $this->getSVNResult($request);
+        break;
+      default:
+        throw new ConduitException('ERR-UNKNOWN-REPOSITORY-VCS');
+        break;
+    }
+    return $result;
+  }
+}

src/applications/diffusion/conduit/ConduitAPI_diffusion_existsquery_Method.php

@@ -0,0 +1,55 @@
+<?php
+
+/**
+ * @group conduit
+ */
+final class ConduitAPI_diffusion_existsquery_Method
+  extends ConduitAPI_diffusion_abstractquery_Method {
+
+  public function getMethodDescription() {
+    return "Determine if code exists in a version control system.";
+  }
+
+  public function defineReturnType() {
+    return 'bool';
+  }
+
+  protected function defineCustomParamTypes() {
+    return array(
+      'commit' => 'required string'
+    );
+  }
+
+  protected function getGitResult(ConduitAPIRequest $request) {
+    $repository = $this->getDiffusionRequest()->getRepository();
+    $commit = $request->getValue('commit');
+    list($err, $merge_base) = $repository->execLocalCommand(
+      'cat-file -t %s',
+      $commit);
+    return !$err;
+  }
+
+  protected function getSVNResult(ConduitAPIRequest $request) {
+    $repository = $this->getDiffusionRequest()->getRepository();
+    $commit = $request->getValue('commit');
+    list($info) = $repository->execxRemoteCommand(
+      'info %s',
+      $repository->getRemoteURI());
+    $exists = false;
+    $matches = null;
+    if (preg_match('/^Revision: (\d+)$/m', $info, $matches)) {
+      $base_revision = $matches[1];
+      $exists = $base_revision >= $commit;
+    }
+    return $exists;
+  }
+
+  protected function getMercurialResult(ConduitAPIRequest $request) {
+    $repository = $this->getDiffusionRequest()->getRepository();
+    $commit = $request->getValue('commit');
+    list($err, $stdout) = $repository->execLocalCommand(
+      'id --rev %s',
+      $commit);
+    return  !$err;
+  }
+}

src/applications/diffusion/controller/DiffusionCommitController.php

@@ -30,8 +30,9 @@ final class DiffusionCommitController extends DiffusionController {
     $commit = $drequest->loadCommit();
 
     if (!$commit) {
-      $query = DiffusionExistsQuery::newFromDiffusionRequest($drequest);
+      $exists = $this->callConduitWithDiffusionRequest(
-      $exists = $query->loadExistentialData();
+        'diffusion.existsquery',
+        array('commit' => $drequest->getCommit()));
       if (!$exists) {
         return new Aphront404Response();
       }

src/applications/diffusion/controller/DiffusionController.php

@@ -321,4 +321,17 @@ abstract class DiffusionController extends PhabricatorController {
     return $crumb_list;
   }
 
+  protected function callConduitWithDiffusionRequest(
+    $method,
+    array $params = array()) {
+
+    $user = $this->getRequest()->getUser();
+    $drequest = $this->getDiffusionRequest();
+
+    return DiffusionQuery::callConduitWithDiffusionRequest(
+      $user,
+      $drequest,
+      $method,
+      $params);
+  }
 }

src/applications/diffusion/query/DiffusionQuery.php

@@ -36,6 +36,27 @@ abstract class DiffusionQuery extends PhabricatorQuery {
     return $this->request;
   }
 
+  final public static function callConduitWithDiffusionRequest(
+    PhabricatorUser $user,
+    DiffusionRequest $drequest,
+    $method,
+    array $params = array()) {
+
+    $repository = $drequest->getRepository();
+
+    $core_params = array(
+      'callsign' => $repository->getCallsign()
+    );
+    $params = $params + $core_params;
+
+    return id(new ConduitCall(
+      $method,
+      $params
+    ))
+    ->setUser($user)
+    ->execute();
+  }
+
   public function execute() {
     return $this->executeQuery();
   }

src/applications/diffusion/query/exists/DiffusionExistsQuery.php

@@ -1,13 +0,0 @@
-<?php
-
-abstract class DiffusionExistsQuery extends DiffusionQuery {
-
-  final public static function newFromDiffusionRequest(
-    DiffusionRequest $request) {
-    return self::newQueryObject(__CLASS__, $request);
-  }
-
-  final public function loadExistentialData() {
-    return $this->executeQuery();
-  }
-}

src/applications/diffusion/query/exists/DiffusionGitExistsQuery.php

@@ -1,16 +0,0 @@
-<?php
-
-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;
-  }
-
-}

src/applications/diffusion/query/exists/DiffusionMercurialExistsQuery.php

@@ -1,16 +0,0 @@
-<?php
-
-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;
-
-  }
-}

src/applications/diffusion/query/exists/DiffusionSvnExistsQuery.php

@@ -1,23 +0,0 @@
-<?php
-
-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;
-  }
-
-}