Add diffusion.querycommits and deprecate diffusion.getcommits

Summary:
Fixes T4344. `diffusion.getcommits` is nasty old bad news. Implement a modern query method.

This method provides limit/paging in a somewhat abstract way so it's sort of ultramodern, but I didn't want the default behavior to return a million rows. I'll probably move more stuff toward this over time, now that cursor paging is pervasive. Here, we needed extra metadata (the identifier map) anyway.

Test Plan: Used console to execute command.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4344

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

src/__phutil_library_map__.php

@@ -168,6 +168,7 @@ phutil_register_library_map(array(
     'ConduitAPI_diffusion_lastmodifiedquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_lastmodifiedquery_Method.php',
     'ConduitAPI_diffusion_looksoon_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_looksoon_Method.php',
     'ConduitAPI_diffusion_mergedcommitsquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_mergedcommitsquery_Method.php',
+    'ConduitAPI_diffusion_querycommits_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_querycommits.php',
     'ConduitAPI_diffusion_rawdiffquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_rawdiffquery_Method.php',
     'ConduitAPI_diffusion_readmequery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_readmequery_Method.php',
     'ConduitAPI_diffusion_refsquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_refsquery_Method.php',
@@ -2637,6 +2638,7 @@ phutil_register_library_map(array(
     'ConduitAPI_diffusion_lastmodifiedquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method',
     'ConduitAPI_diffusion_looksoon_Method' => 'ConduitAPI_diffusion_Method',
     'ConduitAPI_diffusion_mergedcommitsquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method',
+    'ConduitAPI_diffusion_querycommits_Method' => 'ConduitAPI_diffusion_Method',
     'ConduitAPI_diffusion_rawdiffquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method',
     'ConduitAPI_diffusion_readmequery_Method' => 'ConduitAPI_diffusion_abstractquery_Method',
     'ConduitAPI_diffusion_refsquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method',

src/applications/conduit/method/ConduitAPIMethod.php

@@ -1,9 +1,8 @@
 <?php
 
 /**
- *
  * @task  status  Method Status
- * @group conduit
+ * @task  pager   Paging Results
  */
 abstract class ConduitAPIMethod
   extends Phobject
@@ -167,6 +166,63 @@ abstract class ConduitAPIMethod
   }
 
 
+/* -(  Paging Results  )----------------------------------------------------- */
+
+
+  /**
+   * @task pager
+   */
+  protected function getPagerParamTypes() {
+    return array(
+      'before'            => 'optional string',
+      'after'             => 'optional string',
+      'limit'             => 'optional int (default = 100)',
+    );
+  }
+
+
+  /**
+   * @task pager
+   */
+  protected function newPager(ConduitAPIRequest $request) {
+    $limit = $request->getValue('limit', 100);
+    $limit = min(1000, $limit);
+    $limit = max(1, $limit);
+
+    $pager = id(new AphrontCursorPagerView())
+      ->setPageSize($limit);
+
+    $before_id = $request->getValue('before');
+    if ($before_id !== null) {
+      $pager->setBeforeID($before_id);
+    }
+
+    $after_id = $request->getValue('after');
+    if ($after_id !== null) {
+      $pager->setAfterID($after_id);
+    }
+
+    return $pager;
+  }
+
+
+  /**
+   * @task pager
+   */
+  protected function addPagerResults(
+    array $results,
+    AphrontCursorPagerView $pager) {
+
+    $results['cursor'] = array(
+      'limit' => $pager->getPageSize(),
+      'after' => $pager->getNextPageID(),
+      'before' =>$pager->getPrevPageID(),
+    );
+
+    return $results;
+  }
+
+
 /* -(  PhabricatorPolicyInterface  )----------------------------------------- */
 
 

src/applications/diffusion/conduit/ConduitAPI_diffusion_getcommits_Method.php

@@ -7,7 +7,15 @@ final class ConduitAPI_diffusion_getcommits_Method
   extends ConduitAPI_diffusion_Method {
 
   public function getMethodDescription() {
-    return "Retrieve Diffusion commit information.";
+    return pht('Retrieve Diffusion commit information.');
+  }
+
+  public function getMethodStatus() {
+    return self::METHOD_STATUS_DEPRECATED;
+  }
+
+  public function getMethodStatusDescription() {
+    return pht('Obsoleted by diffusion.querycommits.');
   }
 
   public function defineParamTypes() {

src/applications/diffusion/conduit/ConduitAPI_diffusion_querycommits_Method.php

@@ -0,0 +1,83 @@
+<?php
+
+final class ConduitAPI_diffusion_querycommits_Method
+  extends ConduitAPI_diffusion_Method {
+
+  public function getMethodDescription() {
+    return pht('Retrieve information about commits.');
+  }
+
+  public function defineReturnType() {
+    return 'map<string, dict>';
+  }
+
+  public function defineParamTypes() {
+    return array(
+      'ids'               => 'optional list<int>',
+      'phids'             => 'optional list<phid>',
+      'names'             => 'optional list<string>',
+      'repositoryPHID'    => 'optional phid',
+    ) + $this->getPagerParamTypes();
+  }
+
+  public function defineErrorTypes() {
+    return array();
+  }
+
+  protected function execute(ConduitAPIRequest $request) {
+    $query = id(new DiffusionCommitQuery())
+      ->setViewer($request->getUser());
+
+    $repository_phid = $request->getValue('repositoryPHID');
+    if ($repository_phid) {
+      $repository = id(new PhabricatorRepositoryQuery())
+        ->setViewer($request->getUser())
+        ->withPHIDs(array($repository_phid))
+        ->executeOne();
+      if ($repository) {
+        $query->withRepository($repository);
+      }
+    }
+
+    $names = $request->getValue('names');
+    if ($names) {
+      $query->withIdentifiers($names);
+    }
+
+    $ids = $request->getValue('ids');
+    if ($ids) {
+      $query->withIDs($ids);
+    }
+
+    $phids = $request->getValue('phids');
+    if ($phids) {
+      $query->withPHIDs($phids);
+    }
+
+    $pager = $this->newPager($request);
+    $commits = $query->executeWithCursorPager($pager);
+
+    $map = $query->getIdentifierMap();
+    $map = mpull($map, 'getPHID');
+
+    $data = array();
+    foreach ($commits as $commit) {
+      $data[$commit->getPHID()] = array(
+        'id' => $commit->getID(),
+        'phid' => $commit->getPHID(),
+        'repositoryPHID' => $commit->getRepository()->getPHID(),
+        'identifier' => $commit->getCommitIdentifier(),
+        'epoch' => $commit->getEpoch(),
+        'isImporting' => !$commit->isImported(),
+      );
+    }
+
+    $result = array(
+      'data' => $data,
+      'identifierMap' => nonempty($map, (object)array()),
+    );
+
+    return $this->addPagerResults($result, $pager);
+  }
+
+}

src/applications/diffusion/query/DiffusionCommitQuery.php

@@ -259,7 +259,8 @@ final class DiffusionCommitQuery
         // If we discarded all possible identifiers (e.g., they all referenced
         // bogus repositories or were all too short), make sure the query finds
         // nothing.
-        throw new PhabricatorEmptyQueryException('No commit identifiers.');
+        throw new PhabricatorEmptyQueryException(
+          pht('No commit identifiers.'));
       }
 
       $where[] = '('.implode(' OR ', $sql).')';
@@ -286,6 +287,8 @@ final class DiffusionCommitQuery
         $this->repositoryIDs);
     }
 
+    $where[] = $this->buildPagingClause($conn_r);
+
     return $this->formatWhereClause($where);
   }