Diffusion - move history query to conduit

Summary: Ref T2784

Test Plan: for each flavor of VCS, I loaded up the repository home page. verified I saw some parent action where appropos. next, clicked through to 'view history' and verified it loaded up A-OK.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2784

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

src/__phutil_library_map__.php

@@ -156,6 +156,7 @@ phutil_register_library_map(array(
     'ConduitAPI_diffusion_getcommits_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_getcommits_Method.php',
     'ConduitAPI_diffusion_getlintmessages_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_getlintmessages_Method.php',
     'ConduitAPI_diffusion_getrecentcommitsbypath_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_getrecentcommitsbypath_Method.php',
+    'ConduitAPI_diffusion_historyquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_historyquery_Method.php',
     'ConduitAPI_diffusion_lastmodifiedquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_lastmodifiedquery_Method.php',
     'ConduitAPI_diffusion_rawdiffquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_rawdiffquery_Method.php',
     'ConduitAPI_diffusion_refsquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_refsquery_Method.php',
@@ -443,13 +444,11 @@ phutil_register_library_map(array(
     'DiffusionGitCommitParentsQuery' => 'applications/diffusion/query/parents/DiffusionGitCommitParentsQuery.php',
     'DiffusionGitExpandShortNameQuery' => 'applications/diffusion/query/expandshortname/DiffusionGitExpandShortNameQuery.php',
     'DiffusionGitFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php',
-    'DiffusionGitHistoryQuery' => 'applications/diffusion/query/history/DiffusionGitHistoryQuery.php',
     'DiffusionGitMergedCommitsQuery' => 'applications/diffusion/query/mergedcommits/DiffusionGitMergedCommitsQuery.php',
     'DiffusionGitRawDiffQuery' => 'applications/diffusion/query/rawdiff/DiffusionGitRawDiffQuery.php',
     'DiffusionGitRequest' => 'applications/diffusion/request/DiffusionGitRequest.php',
     'DiffusionGitStableCommitNameQuery' => 'applications/diffusion/query/stablecommitname/DiffusionGitStableCommitNameQuery.php',
     'DiffusionHistoryController' => 'applications/diffusion/controller/DiffusionHistoryController.php',
-    'DiffusionHistoryQuery' => 'applications/diffusion/query/history/DiffusionHistoryQuery.php',
     'DiffusionHistoryTableView' => 'applications/diffusion/view/DiffusionHistoryTableView.php',
     'DiffusionHomeController' => 'applications/diffusion/controller/DiffusionHomeController.php',
     'DiffusionHovercardEventListener' => 'applications/diffusion/events/DiffusionHovercardEventListener.php',
@@ -462,7 +461,6 @@ phutil_register_library_map(array(
     'DiffusionMercurialCommitParentsQuery' => 'applications/diffusion/query/parents/DiffusionMercurialCommitParentsQuery.php',
     'DiffusionMercurialExpandShortNameQuery' => 'applications/diffusion/query/expandshortname/DiffusionMercurialExpandShortNameQuery.php',
     'DiffusionMercurialFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionMercurialFileContentQuery.php',
-    'DiffusionMercurialHistoryQuery' => 'applications/diffusion/query/history/DiffusionMercurialHistoryQuery.php',
     'DiffusionMercurialMergedCommitsQuery' => 'applications/diffusion/query/mergedcommits/DiffusionMercurialMergedCommitsQuery.php',
     'DiffusionMercurialRawDiffQuery' => 'applications/diffusion/query/rawdiff/DiffusionMercurialRawDiffQuery.php',
     'DiffusionMercurialRequest' => 'applications/diffusion/request/DiffusionMercurialRequest.php',
@@ -488,7 +486,6 @@ phutil_register_library_map(array(
     'DiffusionStableCommitNameQuery' => 'applications/diffusion/query/stablecommitname/DiffusionStableCommitNameQuery.php',
     'DiffusionSvnCommitParentsQuery' => 'applications/diffusion/query/parents/DiffusionSvnCommitParentsQuery.php',
     'DiffusionSvnFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionSvnFileContentQuery.php',
-    'DiffusionSvnHistoryQuery' => 'applications/diffusion/query/history/DiffusionSvnHistoryQuery.php',
     'DiffusionSvnMergedCommitsQuery' => 'applications/diffusion/query/mergedcommits/DiffusionSvnMergedCommitsQuery.php',
     'DiffusionSvnRawDiffQuery' => 'applications/diffusion/query/rawdiff/DiffusionSvnRawDiffQuery.php',
     'DiffusionSvnRequest' => 'applications/diffusion/request/DiffusionSvnRequest.php',
@@ -1969,6 +1966,7 @@ phutil_register_library_map(array(
     'ConduitAPI_diffusion_getcommits_Method' => 'ConduitAPI_diffusion_Method',
     'ConduitAPI_diffusion_getlintmessages_Method' => 'ConduitAPI_diffusion_Method',
     'ConduitAPI_diffusion_getrecentcommitsbypath_Method' => 'ConduitAPI_diffusion_Method',
+    'ConduitAPI_diffusion_historyquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method',
     'ConduitAPI_diffusion_lastmodifiedquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method',
     'ConduitAPI_diffusion_rawdiffquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method',
     'ConduitAPI_diffusion_refsquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method',
@@ -2244,13 +2242,11 @@ phutil_register_library_map(array(
     'DiffusionGitCommitParentsQuery' => 'DiffusionCommitParentsQuery',
     'DiffusionGitExpandShortNameQuery' => 'DiffusionExpandShortNameQuery',
     'DiffusionGitFileContentQuery' => 'DiffusionFileContentQuery',
-    'DiffusionGitHistoryQuery' => 'DiffusionHistoryQuery',
     'DiffusionGitMergedCommitsQuery' => 'DiffusionMergedCommitsQuery',
     'DiffusionGitRawDiffQuery' => 'DiffusionRawDiffQuery',
     'DiffusionGitRequest' => 'DiffusionRequest',
     'DiffusionGitStableCommitNameQuery' => 'DiffusionStableCommitNameQuery',
     'DiffusionHistoryController' => 'DiffusionController',
-    'DiffusionHistoryQuery' => 'DiffusionQuery',
     'DiffusionHistoryTableView' => 'DiffusionView',
     'DiffusionHomeController' => 'DiffusionController',
     'DiffusionHovercardEventListener' => 'PhutilEventListener',
@@ -2262,7 +2258,6 @@ phutil_register_library_map(array(
     'DiffusionMercurialCommitParentsQuery' => 'DiffusionCommitParentsQuery',
     'DiffusionMercurialExpandShortNameQuery' => 'DiffusionExpandShortNameQuery',
     'DiffusionMercurialFileContentQuery' => 'DiffusionFileContentQuery',
-    'DiffusionMercurialHistoryQuery' => 'DiffusionHistoryQuery',
     'DiffusionMercurialMergedCommitsQuery' => 'DiffusionMergedCommitsQuery',
     'DiffusionMercurialRawDiffQuery' => 'DiffusionRawDiffQuery',
     'DiffusionMercurialRequest' => 'DiffusionRequest',
@@ -2280,7 +2275,6 @@ phutil_register_library_map(array(
     'DiffusionStableCommitNameQuery' => 'DiffusionQuery',
     'DiffusionSvnCommitParentsQuery' => 'DiffusionCommitParentsQuery',
     'DiffusionSvnFileContentQuery' => 'DiffusionFileContentQuery',
-    'DiffusionSvnHistoryQuery' => 'DiffusionHistoryQuery',
     'DiffusionSvnMergedCommitsQuery' => 'DiffusionMergedCommitsQuery',
     'DiffusionSvnRawDiffQuery' => 'DiffusionRawDiffQuery',
     'DiffusionSvnRequest' => 'DiffusionRequest',

src/applications/diffusion/conduit/ConduitAPI_diffusion_getrecentcommitsbypath_Method.php

@@ -43,11 +43,19 @@ final class ConduitAPI_diffusion_getrecentcommitsbypath_Method
       $request->getValue('limit'),
       self::DEFAULT_LIMIT);
 
-    $history = DiffusionHistoryQuery::newFromDiffusionRequest($drequest)
+    $history_result = DiffusionQuery::callConduitWithDiffusionRequest(
-    ->setLimit($limit)
+      $request->getUser(),
-    ->needDirectChanges(true)
+      $drequest,
-    ->needChildChanges(true)
+      'diffusion.historyquery',
-    ->loadHistory();
+      array(
+        'commit' => $drequest->getCommit(),
+        'path' => $drequest->getPath(),
+        'offset' => 0,
+        'limit' => $limit,
+        'needDirectChanges' => true,
+        'needChildChanges' => true));
+    $history = DiffusionPathChange::newFromConduit(
+      $history_result['pathChanges']);
 
     $raw_commit_identifiers = mpull($history, 'getCommitIdentifier');
     $result = array();

src/applications/diffusion/conduit/ConduitAPI_diffusion_historyquery_Method.php

@@ -0,0 +1,266 @@
+<?php
+
+/**
+ * @group conduit
+ */
+final class ConduitAPI_diffusion_historyquery_Method
+extends ConduitAPI_diffusion_abstractquery_Method {
+
+  private $parents = array();
+
+  public function getMethodDescription() {
+    return 'Returns history information for a repository at a specific '.
+      'commit and path.';
+  }
+
+  public function defineReturnType() {
+    return 'array';
+  }
+
+  protected function defineCustomParamTypes() {
+    return array(
+      'commit' => 'required string',
+      'path' => 'required string',
+      'offset' => 'required int',
+      'limit' => 'required int',
+      'needDirectChanges' => 'optional bool',
+      'needChildChanges' => 'optional bool',
+    );
+  }
+
+  protected function getResult(ConduitAPIRequest $request) {
+    $path_changes = parent::getResult($request);
+
+    return array(
+      'pathChanges' => mpull($path_changes, 'toDictionary'),
+      'parents' => $this->parents);
+  }
+
+  protected function getGitResult(ConduitAPIRequest $request) {
+    $drequest = $this->getDiffusionRequest();
+    $repository = $drequest->getRepository();
+    $commit_hash = $request->getValue('commit');
+    $path = $request->getValue('path');
+    $offset = $request->getValue('offset');
+    $limit = $request->getValue('limit');
+
+    list($stdout) = $repository->execxLocalCommand(
+      'log '.
+        '--skip=%d '.
+        '-n %d '.
+        '--pretty=format:%s '.
+        '%s -- %C',
+      $offset,
+      $limit,
+      '%H:%P',
+      $commit_hash,
+      // Git omits merge commits if the path is provided, even if it is empty.
+      (strlen($path) ? csprintf('%s', $path) : ''));
+
+    $lines = explode("\n", trim($stdout));
+    $lines = array_filter($lines);
+    if (!$lines) {
+      return array();
+    }
+
+    $hash_list = array();
+    $parent_map = array();
+    foreach ($lines as $line) {
+      list($hash, $parents) = explode(":", $line);
+      $hash_list[] = $hash;
+      $parent_map[$hash] = preg_split('/\s+/', $parents);
+    }
+
+    $this->parents = $parent_map;
+
+    return DiffusionQuery::loadHistoryForCommitIdentifiers(
+      $hash_list,
+      $drequest);
+  }
+
+  protected function getMercurialResult(ConduitAPIRequest $request) {
+    $drequest = $this->getDiffusionRequest();
+    $repository = $drequest->getRepository();
+    $commit_hash = $request->getValue('commit');
+    $path = $request->getValue('path');
+    $offset = $request->getValue('offset');
+    $limit = $request->getValue('limit');
+
+    $path = DiffusionPathIDQuery::normalizePath($path);
+    $path = ltrim($path, '/');
+
+    // NOTE: Older versions of Mercurial give different results for these
+    // commands (see T1268):
+    //
+    //   $ hg log -- ''
+    //   $ hg log
+    //
+    // All versions of Mercurial give different results for these commands
+    // (merge commits are excluded with the "." version):
+    //
+    //   $ hg log -- .
+    //   $ hg log
+    //
+    // If we don't have a path component in the query, omit it from the command
+    // entirely to avoid these inconsistencies.
+
+    // NOTE: When viewing the history of a file, we don't use "-b", because
+    // Mercurial stops history at the branchpoint but we're interested in all
+    // ancestors. When viewing history of a branch, we do use "-b", and thus
+    // stop history (this is more consistent with the Mercurial worldview of
+    // branches).
+
+    if (strlen($path)) {
+      $path_arg = csprintf('-- %s', $path);
+      $branch_arg = '';
+    } else {
+      $path_arg = '';
+      // NOTE: --branch used to be called --only-branch; use -b for
+      // compatibility.
+      $branch_arg = csprintf('-b %s', $drequest->getBranch());
+    }
+
+    list($stdout) = $repository->execxLocalCommand(
+      'log --debug --template %s --limit %d %C --rev %s %C',
+      '{node};{parents}\\n',
+      ($offset + $limit), // No '--skip' in Mercurial.
+      $branch_arg,
+      hgsprintf('reverse(%s::%s)', '0', $commit_hash),
+      $path_arg);
+
+    $lines = explode("\n", trim($stdout));
+    $lines = array_slice($lines, $offset);
+
+    $hash_list = array();
+    $parent_map = array();
+
+    $last = null;
+    foreach (array_reverse($lines) as $line) {
+      list($hash, $parents) = explode(';', $line);
+      $parents = trim($parents);
+      if (!$parents) {
+        if ($last === null) {
+          $parent_map[$hash] = array('...');
+        } else {
+          $parent_map[$hash] = array($last);
+        }
+      } else {
+        $parents = preg_split('/\s+/', $parents);
+        foreach ($parents as $parent) {
+          list($plocal, $phash) = explode(':', $parent);
+          if (!preg_match('/^0+$/', $phash)) {
+            $parent_map[$hash][] = $phash;
+          }
+        }
+        // This may happen for the zeroth commit in repository, both hashes
+        // are "000000000...".
+        if (empty($parent_map[$hash])) {
+          $parent_map[$hash] = array('...');
+        }
+      }
+
+      // The rendering code expects the first commit to be "mainline", like
+      // Git. Flip the order so it does the right thing.
+      $parent_map[$hash] = array_reverse($parent_map[$hash]);
+
+      $hash_list[] = $hash;
+      $last = $hash;
+    }
+
+    $hash_list = array_reverse($hash_list);
+    $this->parents = $parent_map;
+
+    return DiffusionQuery::loadHistoryForCommitIdentifiers(
+      $hash_list,
+      $drequest);
+  }
+
+  protected function getSVNResult(ConduitAPIRequest $request) {
+    $drequest = $this->getDiffusionRequest();
+    $repository = $drequest->getRepository();
+    $commit = $request->getValue('commit');
+    $path = $request->getValue('path');
+    $offset = $request->getValue('offset');
+    $limit = $request->getValue('limit');
+    $need_direct_changes = $request->getValue('needDirectChanges');
+    $need_child_changes = $request->getValue('needChildChanges');
+
+    $conn_r = $repository->establishConnection('r');
+
+    $paths = queryfx_all(
+      $conn_r,
+      'SELECT id, path FROM %T WHERE pathHash IN (%Ls)',
+      PhabricatorRepository::TABLE_PATH,
+      array(md5('/'.trim($path, '/'))));
+    $paths = ipull($paths, 'id', 'path');
+    $path_id = idx($paths, '/'.trim($path, '/'));
+
+    if (!$path_id) {
+      return array();
+    }
+
+    $filter_query = '';
+    if ($need_direct_changes) {
+      if ($need_child_changes) {
+        $type = DifferentialChangeType::TYPE_CHILD;
+        $filter_query = 'AND (isDirect = 1 OR changeType = '.$type.')';
+      } else {
+        $filter_query = 'AND (isDirect = 1)';
+      }
+    }
+
+    $history_data = queryfx_all(
+      $conn_r,
+      'SELECT * FROM %T WHERE repositoryID = %d AND pathID = %d
+        AND commitSequence <= %d
+        %Q
+        ORDER BY commitSequence DESC
+        LIMIT %d, %d',
+      PhabricatorRepository::TABLE_PATHCHANGE,
+      $repository->getID(),
+      $path_id,
+      $commit ? $commit : 0x7FFFFFFF,
+      $filter_query,
+      $offset,
+      $limit);
+
+    $commits = array();
+    $commit_data = array();
+
+    $commit_ids = ipull($history_data, 'commitID');
+    if ($commit_ids) {
+      $commits = id(new PhabricatorRepositoryCommit())->loadAllWhere(
+        'id IN (%Ld)',
+        $commit_ids);
+      if ($commits) {
+        $commit_data = id(new PhabricatorRepositoryCommitData())->loadAllWhere(
+          'commitID in (%Ld)',
+          $commit_ids);
+        $commit_data = mpull($commit_data, null, 'getCommitID');
+      }
+    }
+
+    $history = array();
+    foreach ($history_data as $row) {
+      $item = new DiffusionPathChange();
+
+      $commit = idx($commits, $row['commitID']);
+      if ($commit) {
+        $item->setCommit($commit);
+        $item->setCommitIdentifier($commit->getCommitIdentifier());
+        $data = idx($commit_data, $commit->getID());
+        if ($data) {
+          $item->setCommitData($data);
+        }
+      }
+
+      $item->setChangeType($row['changeType']);
+      $item->setFileType($row['fileType']);
+
+      $history[] = $item;
+    }
+
+    return $history;
+  }
+
+}

src/applications/diffusion/conduit/ConduitAPI_diffusion_lastmodifiedquery_Method.php

@@ -55,12 +55,19 @@ final class ConduitAPI_diffusion_lastmodifiedquery_Method
 
     $path = $drequest->getPath();
 
-    $history_query = DiffusionHistoryQuery::newFromDiffusionRequest(
+    $history_result = DiffusionQuery::callConduitWithDiffusionRequest(
-      $drequest);
+      $request->getUser(),
-    $history_query->setLimit(1);
+      $drequest,
-    $history_query->needChildChanges(true);
+      'diffusion.historyquery',
-    $history_query->needDirectChanges(true);
+      array(
-    $history_array = $history_query->loadHistory();
+        'commit' => $drequest->getCommit(),
+        'path' => $path,
+        'limit' => 1,
+        'offset' => 0,
+        'needDirectChanges' => true,
+        'needChildChanges' => true));
+    $history_array = DiffusionPathChange::newFromConduit(
+      $history_result['pathChanges']);
 
     if (!$history_array) {
       return array(array(), array());

src/applications/diffusion/controller/DiffusionHistoryController.php

@@ -9,22 +9,21 @@ final class DiffusionHistoryController extends DiffusionController {
     $page_size = $request->getInt('pagesize', 100);
     $offset = $request->getInt('page', 0);
 
-    $history_query = DiffusionHistoryQuery::newFromDiffusionRequest(
+    $params = array(
-      $drequest);
+      'commit' => $drequest->getCommit(),
-    $history_query->setOffset($offset);
+      'path' => $drequest->getPath(),
-    $history_query->setLimit($page_size + 1);
+      'offset' => $offset,
-
+      'limit' => $page_size + 1);
     if (!$request->getBool('copies')) {
-      $history_query->needDirectChanges(true);
+      $params['needDirectChanges'] = true;
-      $history_query->needChildChanges(true);
+      $params['needChildChanges'] = true;
     }
 
-    $show_graph = !strlen($drequest->getPath());
+    $history_results = $this->callConduitWithDiffusionRequest(
-    if ($show_graph) {
+      'diffusion.historyquery',
-      $history_query->needParents(true);
+      $params);
-    }
+    $history = DiffusionPathChange::newFromConduit(
-
+      $history_results['pathChanges']);
-    $history = $history_query->loadHistory();
 
     $pager = new AphrontPagerView();
     $pager->setPageSize($page_size);
@@ -37,6 +36,7 @@ final class DiffusionHistoryController extends DiffusionController {
     }
     $pager->setURI($request->getRequestURI(), 'page');
 
+    $show_graph = !strlen($drequest->getPath());
     $content = array();
 
     if ($request->getBool('copies')) {
@@ -66,7 +66,7 @@ final class DiffusionHistoryController extends DiffusionController {
     $history_table->setHandles($handles);
 
     if ($show_graph) {
-      $history_table->setParents($history_query->getParents());
+      $history_table->setParents($history_results['parents']);
       $history_table->setIsHead($offset == 0);
     }
 

src/applications/diffusion/controller/DiffusionRepositoryController.php

@@ -12,11 +12,15 @@ final class DiffusionRepositoryController extends DiffusionController {
 
     $content[] = $this->buildPropertiesTable($drequest->getRepository());
 
-    $history_query = DiffusionHistoryQuery::newFromDiffusionRequest(
+    $history_results = $this->callConduitWithDiffusionRequest(
-      $drequest);
+      'diffusion.historyquery',
-    $history_query->setLimit(15);
+      array(
-    $history_query->needParents(true);
+        'commit' => $drequest->getCommit(),
-    $history = $history_query->loadHistory();
+        'path' => $drequest->getPath(),
+        'offset' => 0,
+        'limit' => 15));
+    $history = DiffusionPathChange::newFromConduit(
+      $history_results['pathChanges']);
 
     $browse_results = DiffusionBrowseResultSet::newFromConduit(
       $this->callConduitWithDiffusionRequest(
@@ -63,7 +67,7 @@ final class DiffusionRepositoryController extends DiffusionController {
     $history_table->setHandles($handles);
     $history_table->setHistory($history);
     $history_table->loadRevisions();
-    $history_table->setParents($history_query->getParents());
+    $history_table->setParents($history_results['parents']);
     $history_table->setIsHead(true);
 
     $callsign = $drequest->getRepository()->getCallsign();

src/applications/diffusion/data/DiffusionPathChange.php

@@ -149,4 +149,36 @@ final class DiffusionPathChange {
     return $diff->getChangesets();
   }
 
+  public function toDictionary() {
+    return array(
+      'path' => $this->getPath(),
+      'commitIdentifier' => $this->getCommitIdentifier(),
+      'commit' => $this->getCommit()->toDictionary(),
+      'commitData' => $this->getCommitData()->toDictionary(),
+      'fileType' => $this->getFileType(),
+      'targetPath' =>  $this->getTargetPath(),
+      'targetCommitIdentifier' => $this->getTargetCommitIdentifier(),
+      'awayPaths' => $this->getAwayPaths());
+  }
+
+  public static function newFromConduit(array $dicts) {
+    $results = array();
+    foreach ($dicts as $dict) {
+      $commit = PhabricatorRepositoryCommit::newFromDictionary($dict['commit']);
+      $commit_data =
+        PhabricatorRepositoryCommitData::newFromDictionary(
+          $dict['commitData']);
+      $results[] = id(new DiffusionPathChange())
+        ->setPath($dict['path'])
+        ->setCommitIdentifier($dict['commitIdentifier'])
+        ->setCommit($commit)
+        ->setCommitData($commit_data)
+        ->setFileType($dict['fileType'])
+        ->setTargetPath($dict['targetPath'])
+        ->setTargetCommitIdentifier($dict['targetCommitIdentifier'])
+        ->setAwayPaths($dict['awayPaths']);
+    }
+    return $results;
+  }
+
 }

src/applications/diffusion/query/DiffusionQuery.php

@@ -75,7 +75,9 @@ abstract class DiffusionQuery extends PhabricatorQuery {
 /* -(  Query Utilities  )---------------------------------------------------- */
 
 
-  final protected function loadCommitsByIdentifiers(array $identifiers) {
+  final public static function loadCommitsByIdentifiers(
+    array $identifiers,
+    DiffusionRequest $drequest) {
     if (!$identifiers) {
       return array();
     }
@@ -83,7 +85,6 @@ abstract class DiffusionQuery extends PhabricatorQuery {
     $commits = array();
     $commit_data = array();
 
-    $drequest = $this->getRequest();
     $repository = $drequest->getRepository();
 
     $commits = id(new PhabricatorRepositoryCommit())->loadAllWhere(
@@ -131,14 +132,16 @@ abstract class DiffusionQuery extends PhabricatorQuery {
     return $commits;
   }
 
-  final protected function loadHistoryForCommitIdentifiers(array $identifiers) {
+  final public static function loadHistoryForCommitIdentifiers(
+    array $identifiers,
+    DiffusionRequest $drequest) {
+
     if (!$identifiers) {
       return array();
     }
 
-    $drequest = $this->getRequest();
     $repository = $drequest->getRepository();
-    $commits = self::loadCommitsByIdentifiers($identifiers);
+    $commits = self::loadCommitsByIdentifiers($identifiers, $drequest);
 
     if (!$commits) {
       return array();

src/applications/diffusion/query/history/DiffusionGitHistoryQuery.php

@@ -1,44 +0,0 @@
-<?php
-
-final class DiffusionGitHistoryQuery extends DiffusionHistoryQuery {
-
-  protected function executeQuery() {
-    $drequest = $this->getRequest();
-
-    $repository = $drequest->getRepository();
-    $path = $drequest->getPath();
-    $commit_hash = $drequest->getCommit();
-
-    list($stdout) = $repository->execxLocalCommand(
-      'log '.
-        '--skip=%d '.
-        '-n %d '.
-        '--pretty=format:%s '.
-        '%s -- %C',
-      $this->getOffset(),
-      $this->getLimit(),
-      '%H:%P',
-      $commit_hash,
-      // Git omits merge commits if the path is provided, even if it is empty.
-      (strlen($path) ? csprintf('%s', $path) : ''));
-
-    $lines = explode("\n", trim($stdout));
-    $lines = array_filter($lines);
-    if (!$lines) {
-      return array();
-    }
-
-    $hash_list = array();
-    $parent_map = array();
-    foreach ($lines as $line) {
-      list($hash, $parents) = explode(":", $line);
-      $hash_list[] = $hash;
-      $parent_map[$hash] = preg_split('/\s+/', $parents);
-    }
-
-    $this->parents = $parent_map;
-
-    return $this->loadHistoryForCommitIdentifiers($hash_list);
-  }
-
-}

src/applications/diffusion/query/history/DiffusionHistoryQuery.php

@@ -1,64 +0,0 @@
-<?php
-
-abstract class DiffusionHistoryQuery extends DiffusionQuery {
-
-  private $limit = 100;
-  private $offset = 0;
-
-  protected $needDirectChanges;
-  protected $needChildChanges;
-  protected $needParents;
-
-  protected $parents = array();
-
-  final public static function newFromDiffusionRequest(
-    DiffusionRequest $request) {
-
-    return parent::newQueryObject(__CLASS__, $request);
-  }
-
-  final public function needDirectChanges($direct) {
-    $this->needDirectChanges = $direct;
-    return $this;
-  }
-
-  final public function needChildChanges($child) {
-    $this->needChildChanges = $child;
-    return $this;
-  }
-
-  final public function needParents($parents) {
-    $this->needParents = $parents;
-    return $this;
-  }
-
-  final public function getParents() {
-    if (!$this->needParents) {
-      throw new Exception('Specify needParents() before calling getParents()!');
-    }
-    return $this->parents;
-  }
-
-  final public function loadHistory() {
-    return $this->executeQuery();
-  }
-
-  final public function setLimit($limit) {
-    $this->limit = $limit;
-    return $this;
-  }
-
-  final public function getLimit() {
-    return $this->limit;
-  }
-
-  final public function setOffset($offset) {
-    $this->offset = $offset;
-    return $this;
-  }
-
-  final public function getOffset() {
-    return $this->offset;
-  }
-
-}

src/applications/diffusion/query/history/DiffusionMercurialHistoryQuery.php

@@ -1,99 +0,0 @@
-<?php
-
-final class DiffusionMercurialHistoryQuery extends DiffusionHistoryQuery {
-
-  protected function executeQuery() {
-    $drequest = $this->getRequest();
-
-    $repository = $drequest->getRepository();
-    $path = $drequest->getPath();
-    $commit_hash = $drequest->getStableCommitName();
-
-    $path = DiffusionPathIDQuery::normalizePath($path);
-    $path = ltrim($path, '/');
-
-    // NOTE: Older versions of Mercurial give different results for these
-    // commands (see T1268):
-    //
-    //   $ hg log -- ''
-    //   $ hg log
-    //
-    // All versions of Mercurial give different results for these commands
-    // (merge commits are excluded with the "." version):
-    //
-    //   $ hg log -- .
-    //   $ hg log
-    //
-    // If we don't have a path component in the query, omit it from the command
-    // entirely to avoid these inconsistencies.
-
-    // NOTE: When viewing the history of a file, we don't use "-b", because
-    // Mercurial stops history at the branchpoint but we're interested in all
-    // ancestors. When viewing history of a branch, we do use "-b", and thus
-    // stop history (this is more consistent with the Mercurial worldview of
-    // branches).
-
-    if (strlen($path)) {
-      $path_arg = csprintf('-- %s', $path);
-      $branch_arg = '';
-    } else {
-      $path_arg = '';
-      // NOTE: --branch used to be called --only-branch; use -b for
-      // compatibility.
-      $branch_arg = csprintf('-b %s', $drequest->getBranch());
-    }
-
-    list($stdout) = $repository->execxLocalCommand(
-      'log --debug --template %s --limit %d %C --rev %s %C',
-      '{node};{parents}\\n',
-      ($this->getOffset() + $this->getLimit()), // No '--skip' in Mercurial.
-      $branch_arg,
-      hgsprintf('reverse(%s::%s)', '0', $commit_hash),
-      $path_arg);
-
-    $lines = explode("\n", trim($stdout));
-    $lines = array_slice($lines, $this->getOffset());
-
-    $hash_list = array();
-    $parent_map = array();
-
-    $last = null;
-    foreach (array_reverse($lines) as $line) {
-      list($hash, $parents) = explode(';', $line);
-      $parents = trim($parents);
-      if (!$parents) {
-        if ($last === null) {
-          $parent_map[$hash] = array('...');
-        } else {
-          $parent_map[$hash] = array($last);
-        }
-      } else {
-        $parents = preg_split('/\s+/', $parents);
-        foreach ($parents as $parent) {
-          list($plocal, $phash) = explode(':', $parent);
-          if (!preg_match('/^0+$/', $phash)) {
-            $parent_map[$hash][] = $phash;
-          }
-        }
-        // This may happen for the zeroth commit in repository, both hashes
-        // are "000000000...".
-        if (empty($parent_map[$hash])) {
-          $parent_map[$hash] = array('...');
-        }
-      }
-
-      // The rendering code expects the first commit to be "mainline", like
-      // Git. Flip the order so it does the right thing.
-      $parent_map[$hash] = array_reverse($parent_map[$hash]);
-
-      $hash_list[] = $hash;
-      $last = $hash;
-    }
-
-    $hash_list = array_reverse($hash_list);
-    $this->parents = $parent_map;
-
-    return $this->loadHistoryForCommitIdentifiers($hash_list);
-  }
-
-}

src/applications/diffusion/query/history/DiffusionSvnHistoryQuery.php

@@ -1,91 +0,0 @@
-<?php
-
-final class DiffusionSvnHistoryQuery extends DiffusionHistoryQuery {
-
-  protected function executeQuery() {
-    $drequest = $this->getRequest();
-
-    $repository = $drequest->getRepository();
-    $path = $drequest->getPath();
-    $commit = $drequest->getCommit();
-
-    $conn_r = $repository->establishConnection('r');
-
-    $paths = queryfx_all(
-      $conn_r,
-      'SELECT id, path FROM %T WHERE pathHash IN (%Ls)',
-      PhabricatorRepository::TABLE_PATH,
-      array(md5('/'.trim($path, '/'))));
-    $paths = ipull($paths, 'id', 'path');
-    $path_id = idx($paths, '/'.trim($path, '/'));
-
-    if (!$path_id) {
-      return array();
-    }
-
-    $filter_query = '';
-    if ($this->needDirectChanges) {
-      if ($this->needChildChanges) {
-        $type = DifferentialChangeType::TYPE_CHILD;
-        $filter_query = 'AND (isDirect = 1 OR changeType = '.$type.')';
-      } else {
-        $filter_query = 'AND (isDirect = 1)';
-      }
-    }
-
-    $history_data = queryfx_all(
-      $conn_r,
-      'SELECT * FROM %T WHERE repositoryID = %d AND pathID = %d
-        AND commitSequence <= %d
-        %Q
-        ORDER BY commitSequence DESC
-        LIMIT %d, %d',
-      PhabricatorRepository::TABLE_PATHCHANGE,
-      $repository->getID(),
-      $path_id,
-      $commit ? $commit : 0x7FFFFFFF,
-      $filter_query,
-      $this->getOffset(),
-      $this->getLimit());
-
-    $commits = array();
-    $commit_data = array();
-
-    $commit_ids = ipull($history_data, 'commitID');
-    if ($commit_ids) {
-      $commits = id(new PhabricatorRepositoryCommit())->loadAllWhere(
-        'id IN (%Ld)',
-        $commit_ids);
-      if ($commits) {
-        $commit_data = id(new PhabricatorRepositoryCommitData())->loadAllWhere(
-          'commitID in (%Ld)',
-          $commit_ids);
-        $commit_data = mpull($commit_data, null, 'getCommitID');
-      }
-    }
-
-    $history = array();
-    foreach ($history_data as $row) {
-      $item = new DiffusionPathChange();
-
-      $commit = idx($commits, $row['commitID']);
-      if ($commit) {
-        $item->setCommit($commit);
-        $item->setCommitIdentifier($commit->getCommitIdentifier());
-        $data = idx($commit_data, $commit->getID());
-        if ($data) {
-          $item->setCommitData($data);
-        }
-      }
-
-      $item->setChangeType($row['changeType']);
-      $item->setFileType($row['fileType']);
-
-
-      $history[] = $item;
-    }
-
-    return $history;
-  }
-
-}

src/applications/diffusion/query/mergedcommits/DiffusionGitMergedCommitsQuery.php

@@ -33,7 +33,9 @@ final class DiffusionGitMergedCommitsQuery extends DiffusionMergedCommitsQuery {
     // Remove the merge commit.
     $hashes = array_diff($hashes, array($request->getCommit()));
 
-    return $this->loadHistoryForCommitIdentifiers($hashes);
+    return DiffusionQuery::loadHistoryForCommitIdentifiers(
+      $hashes,
+      $request);
   }
 
 }

src/applications/diffusion/query/mergedcommits/DiffusionMercurialMergedCommitsQuery.php

@@ -35,7 +35,9 @@ final class DiffusionMercurialMergedCommitsQuery
     // Remove the merge commit.
     $hashes = array_diff($hashes, array($request->getCommit()));
 
-    return $this->loadHistoryForCommitIdentifiers($hashes);
+    return DiffusionQuery::loadHistoryForCommitIdentifiers(
+      $hashes,
+      $request);
   }
 
 }

src/applications/diffusion/query/parents/DiffusionGitCommitParentsQuery.php

@@ -14,6 +14,6 @@ final class DiffusionGitCommitParentsQuery
 
     $hashes = preg_split('/\s+/', trim($stdout));
 
-    return self::loadCommitsByIdentifiers($hashes);
+    return self::loadCommitsByIdentifiers($hashes, $drequest);
   }
 }

src/applications/diffusion/query/parents/DiffusionMercurialCommitParentsQuery.php

@@ -25,6 +25,6 @@ final class DiffusionMercurialCommitParentsQuery
       }
     }
 
-    return self::loadCommitsByIdentifiers($hashes);
+    return self::loadCommitsByIdentifiers($hashes, $drequest);
   }
 }

src/applications/diffusion/query/parents/DiffusionSvnCommitParentsQuery.php

@@ -17,6 +17,6 @@ final class DiffusionSvnCommitParentsQuery
       $ids = array();
     }
 
-    return self::loadCommitsByIdentifiers($ids);
+    return self::loadCommitsByIdentifiers($ids, $drequest);
   }
 }