Diffusion - move commit parents query to conduit

Summary:
Ref T2784. Relatively complicated one as this bad boy is used in a repository daemon.

While testing, I noticed bugs in the expandshortname query stuff. Those variables are private to the parent class so they need some setX love.

Also, was unable to find links to the "before" stuff, but made them by hand by looking at some of these T2784 diffs, browsing a file at a specific revision, then hacking the "before" variable to be some known commit that also touched the file. This produced sensical results. On the process of doing that I upgraded a query to use the proper policy query.

Test Plan: In git, mercurial, svn, verified on a commit page the "parents" showed up correctly. played around with ?before parameter on specific file browse page, with commits known to have interesting history and stuff looked good

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2784

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

src/__phutil_library_map__.php

@@ -148,6 +148,7 @@ phutil_register_library_map(array(
     'ConduitAPI_diffusion_branchquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_branchquery_Method.php',
     'ConduitAPI_diffusion_browsequery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_browsequery_Method.php',
     'ConduitAPI_diffusion_commitbranchesquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_commitbranchesquery_Method.php',
+    'ConduitAPI_diffusion_commitparentsquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_commitparentsquery_Method.php',
     'ConduitAPI_diffusion_diffquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_diffquery_Method.php',
     'ConduitAPI_diffusion_existsquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_existsquery_Method.php',
     'ConduitAPI_diffusion_expandshortcommitquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_expandshortcommitquery_Method.php',
@@ -1957,6 +1958,7 @@ phutil_register_library_map(array(
     'ConduitAPI_diffusion_branchquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method',
     'ConduitAPI_diffusion_browsequery_Method' => 'ConduitAPI_diffusion_abstractquery_Method',
     'ConduitAPI_diffusion_commitbranchesquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method',
+    'ConduitAPI_diffusion_commitparentsquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method',
     'ConduitAPI_diffusion_diffquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method',
     'ConduitAPI_diffusion_existsquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method',
     'ConduitAPI_diffusion_expandshortcommitquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method',

src/applications/diffusion/conduit/ConduitAPI_diffusion_commitparentsquery_Method.php

@@ -0,0 +1,31 @@
+<?php
+
+/**
+ * @group conduit
+ */
+final class ConduitAPI_diffusion_commitparentsquery_Method
+  extends ConduitAPI_diffusion_abstractquery_Method {
+
+  public function getMethodDescription() {
+    return
+      'Commit parent(s) information for a commit in a repository.';
+  }
+
+  public function defineReturnType() {
+    return 'array';
+  }
+
+  protected function defineCustomParamTypes() {
+    return array(
+      'commit' => 'required string',
+    );
+  }
+
+  protected function getResult(ConduitAPIRequest $request) {
+    $drequest = $this->getDiffusionRequest();
+
+    $query = DiffusionCommitParentsQuery::newFromDiffusionRequest($drequest);
+    $parents = $query->loadParents();
+    return $parents;
+  }
+}

src/applications/diffusion/controller/DiffusionBrowseFileController.php

@@ -866,6 +866,7 @@ final class DiffusionBrowseFileController extends DiffusionController {
         $rename_query = new DiffusionRenameHistoryQuery();
         $rename_query->setRequest($drequest);
         $rename_query->setOldCommit($grandparent->getCommitIdentifier());
+        $rename_query->setViewer($request->getUser());
         $old_filename = $rename_query->loadOldFilename();
         $was_created = $rename_query->getWasCreated();
       }
@@ -954,16 +955,21 @@ final class DiffusionBrowseFileController extends DiffusionController {
 
   private function loadParentRevisionOf($commit) {
     $drequest = $this->getDiffusionRequest();
+    $user = $this->getRequest()->getUser();
 
     $before_req = DiffusionRequest::newFromDictionary(
       array(
-        'user' => $this->getRequest()->getUser(),
+        'user' => $user,
         'repository' => $drequest->getRepository(),
         'commit'     => $commit,
       ));
 
-    $query = DiffusionCommitParentsQuery::newFromDiffusionRequest($before_req);
-    $parents = $query->loadParents();
+    $parents = DiffusionQuery::callConduitWithDiffusionRequest(
+      $user,
+      $before_req,
+      'diffusion.commitparentsquery',
+      array(
+        'commit' => $commit));
 
     return head($parents);
   }

src/applications/diffusion/controller/DiffusionCommitController.php

@@ -74,8 +74,9 @@ final class DiffusionCommitController extends DiffusionController {
       require_celerity_resource('diffusion-commit-view-css');
       require_celerity_resource('phabricator-remarkup-css');
 
-      $parent_query = DiffusionCommitParentsQuery::newFromDiffusionRequest(
-        $drequest);
+      $parents = $this->callConduitWithDiffusionRequest(
+        'diffusion.commitparentsquery',
+        array('commit' => $drequest->getCommit()));
 
       $headsup_view = id(new PhabricatorHeaderView())
         ->setHeader(nonempty($commit->getSummary(), pht('Commit Detail')));
@@ -85,7 +86,7 @@ final class DiffusionCommitController extends DiffusionController {
       $commit_properties = $this->loadCommitProperties(
         $commit,
         $commit_data,
-        $parent_query->loadParents());
+        $parents);
       $property_list = id(new PhabricatorPropertyListView())
         ->setHasKeyboardShortcuts(true)
         ->setUser($user)

src/applications/diffusion/query/DiffusionRenameHistoryQuery.php

@@ -5,6 +5,12 @@ final class DiffusionRenameHistoryQuery {
   private $oldCommit;
   private $wasCreated;
   private $request;
+  private $viewer;
+
+  public function setViewer(PhabricatorUser $viewer) {
+    $this->viewer = $viewer;
+    return $this;
+  }
 
   public function getWasCreated() {
     return $this->wasCreated;
@@ -72,10 +78,11 @@ final class DiffusionRenameHistoryQuery {
   }
 
   private function loadCommitId($commit_identifier) {
-    $commit = id(new PhabricatorRepositoryCommit())->loadOneWhere(
-      'repositoryID = %d AND commitIdentifier = %s',
-      $this->request->getRepository()->getID(),
-      $commit_identifier);
+    $commit = id(new DiffusionCommitQuery())
+      ->setViewer($this->viewer)
+      ->withIdentifiers(array($commit_identifier))
+      ->withDefaultRepository($this->request->getRepository())
+      ->executeOne();
     return $commit->getID();
   }
 

src/applications/diffusion/query/expandshortname/DiffusionExpandShortNameQuery.php

@@ -22,6 +22,15 @@ abstract class DiffusionExpandShortNameQuery extends DiffusionQuery {
     return $this->repository;
   }
 
+  protected function setCommitType($type) {
+    $this->commitType = $type;
+    return $this;
+  }
+  protected function setTagContent($content) {
+    $this->tagContent = $content;
+    return $this;
+  }
+
   final public static function newFromRepository(
     PhabricatorRepository $repository) {
 

src/applications/diffusion/query/expandshortname/DiffusionGitExpandShortNameQuery.php

@@ -16,12 +16,12 @@ extends DiffusionExpandShortNameQuery {
     if ($type == 'missing') {
       throw new DiffusionExpandCommitQueryException(
         DiffusionExpandCommitQueryException::CODE_MISSING,
-        "Bad commit '{$this->commit}'.");
+        "Bad commit '{$commit}'.");
     }
 
     switch ($type) {
       case 'tag':
-        $this->commitType = 'tag';
+        $this->setCommitType('tag');
 
         $matches = null;
         $ok = preg_match(
@@ -35,18 +35,18 @@ extends DiffusionExpandShortNameQuery {
         }
 
         $hash = $matches[1];
-        $this->tagContent = trim($matches[2]);
+        $this->setTagContent(trim($matches[2]));
         break;
       case 'commit':
         break;
       default:
         throw new DiffusionExpandCommitQueryException(
           DiffusionExpandCommitQueryException::CODE_INVALID,
-          "The reference '{$this->commit}' does not name a valid ".
+          "The reference '{$commit}' does not name a valid ".
           'commit or a tag in this repository.');
         break;
     }
 
-    $this->commit = $hash;
+    $this->setCommit($hash);
   }
 }

src/applications/diffusion/query/expandshortname/DiffusionMercurialExpandShortNameQuery.php

@@ -17,7 +17,7 @@ extends DiffusionExpandShortNameQuery {
     // TODO: Show "multiple matching commits" if count is larger than 1. For
     // now, pick the first one.
 
-    $this->commit = head($full_hash);
+    $this->setCommit(head($full_hash));
   }
 
 }