DiffusionFileContentQuery => Conduit

Summary: Ref T2784. This is probably pretty good except the fancy lint error saver now issue serial queries via Conduit.

Test Plan: reparsed commits on 3 repos - yay. viewed readme from diffusion UI on 3 repos - yay. viewed file content from diffusion UI on 3 repos - yay.

Reviewers: epriestley

Reviewed By: epriestley

CC: chad, aran, Korvin

Maniphest Tasks: T2784

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

src/__phutil_library_map__.php

@@ -147,6 +147,7 @@ phutil_register_library_map(array(
     'ConduitAPI_diffusion_abstractquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_abstractquery_Method.php',
     'ConduitAPI_diffusion_branchquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_branchquery_Method.php',
     'ConduitAPI_diffusion_existsquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_existsquery_Method.php',
+    'ConduitAPI_diffusion_filecontentquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_filecontentquery_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',
@@ -1921,6 +1922,7 @@ phutil_register_library_map(array(
     'ConduitAPI_diffusion_abstractquery_Method' => 'ConduitAPI_diffusion_Method',
     'ConduitAPI_diffusion_branchquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method',
     'ConduitAPI_diffusion_existsquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method',
+    'ConduitAPI_diffusion_filecontentquery_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',
@@ -3475,6 +3477,7 @@ phutil_register_library_map(array(
     'ReleephDiffSizeFieldSpecification' => 'ReleephFieldSpecification',
     'ReleephEvent' => 'ReleephDAO',
     'ReleephFieldParseException' => 'Exception',
+    'ReleephFieldSpecification' => 'PhabricatorMarkupInterface',
     'ReleephFieldSpecificationIncompleteException' => 'Exception',
     'ReleephInactiveProjectListView' => 'AphrontView',
     'ReleephIntentFieldSpecification' => 'ReleephFieldSpecification',

src/applications/diffusion/conduit/ConduitAPI_diffusion_abstractquery_Method.php

@@ -6,6 +6,16 @@
 abstract class ConduitAPI_diffusion_abstractquery_Method
   extends ConduitAPI_diffusion_Method {
 
+  public function getMethodStatus() {
+    return self::METHOD_STATUS_UNSTABLE;
+  }
+  public function getMethodStatusDescription() {
+    return pht(
+      'See T2784 - migrating diffusion working copy calls to conduit methods. '.
+      'Until that task is completed (and possibly after) these methods are '.
+      'unstable.');
+  }
+
   private $diffusionRequest;
   protected function setDiffusionRequest(DiffusionRequest $request) {
     $this->diffusionRequest = $request;
@@ -71,7 +81,9 @@ abstract class ConduitAPI_diffusion_abstractquery_Method
   final protected function execute(ConduitAPIRequest $request) {
     $drequest = DiffusionRequest::newFromDictionary(
       array(
-        'callsign'  => $request->getValue('callsign'),
+        'callsign' => $request->getValue('callsign'),
+        'path' => $request->getValue('path'),
+        'commit' => $request->getValue('commit'),
       ));
     $this->setDiffusionRequest($drequest);
 

src/applications/diffusion/conduit/ConduitAPI_diffusion_filecontentquery_Method.php

@@ -0,0 +1,45 @@
+<?php
+
+/**
+ * @group conduit
+ */
+final class ConduitAPI_diffusion_filecontentquery_Method
+  extends ConduitAPI_diffusion_abstractquery_Method {
+
+  public function getMethodDescription() {
+    return 'Retrieve file content from a repository.';
+  }
+
+  public function defineReturnType() {
+    return 'array';
+  }
+
+  protected function defineCustomParamTypes() {
+    return array(
+      'path' => 'required string',
+      'commit' => 'required string',
+      'needsBlame' => 'optional bool',
+    );
+  }
+
+  protected function getResult(ConduitAPIRequest $request) {
+    $drequest = $this->getDiffusionRequest();
+    $needs_blame = $request->getValue('needsBlame');
+    $file_query = DiffusionFileContentQuery::newFromDiffusionRequest(
+      $drequest);
+    $file_query
+      ->setViewer($request->getUser())
+      ->setNeedsBlame($needs_blame);
+    $file_content = $file_query->loadFileContent();
+    if ($needs_blame) {
+      list($text_list, $rev_list, $blame_dict) = $file_query->getBlameData();
+    } else {
+      $text_list = $rev_list = $blame_dict = array();
+    }
+    $file_content
+      ->setBlameDict($blame_dict)
+      ->setRevList($rev_list)
+      ->setTextList($text_list);
+    return $file_content->toDictionary();
+  }
+}

src/applications/diffusion/controller/DiffusionBrowseFileController.php

@@ -40,12 +40,15 @@ final class DiffusionBrowseFileController extends DiffusionController {
       $needs_blame = true;
     }
 
-    $file_query = DiffusionFileContentQuery::newFromDiffusionRequest(
-      $this->diffusionRequest);
-    $file_query->setViewer($request->getUser());
-    $file_query->setNeedsBlame($needs_blame);
-    $file_query->loadFileContent();
-    $data = $file_query->getRawData();
+    $file_content = DiffusionFileContent::newFromConduit(
+      $this->callConduitWithDiffusionRequest(
+        'diffusion.filecontentquery',
+        array(
+          'commit' => $drequest->getCommit(),
+          'path' => $drequest->getPath(),
+          'needsBlame' => $needs_blame,
+        )));
+    $data = $file_content->getCorpus();
 
     if ($selected === 'raw') {
       return $this->buildRawResponse($path, $data);
@@ -56,7 +59,7 @@ final class DiffusionBrowseFileController extends DiffusionController {
     // Build the content of the file.
     $corpus = $this->buildCorpus(
       $selected,
-      $file_query,
+      $file_content,
       $needs_blame,
       $drequest,
       $path,
@@ -160,12 +163,13 @@ final class DiffusionBrowseFileController extends DiffusionController {
       '/'.$drequest->getPath());
   }
 
-  private function buildCorpus($selected,
-                               DiffusionFileContentQuery $file_query,
-                               $needs_blame,
-                               DiffusionRequest $drequest,
-                               $path,
-                               $data) {
+  private function buildCorpus(
+    $selected,
+    DiffusionFileContent $file_content,
+    $needs_blame,
+    DiffusionRequest $drequest,
+    $path,
+    $data) {
 
     if (ArcanistDiffUtils::isHeuristicBinaryFile($data)) {
       $file = $this->loadFileForData($path, $data);
@@ -189,15 +193,16 @@ final class DiffusionBrowseFileController extends DiffusionController {
           array(
             'style' => $style,
           ),
-          $file_query->getRawData());
+          $file_content->getCorpus());
 
           break;
 
       case 'plainblame':
         $style =
           "margin: 1em 2em; width: 90%; height: 80em; font-family: monospace";
-        list($text_list, $rev_list, $blame_dict) =
-          $file_query->getBlameData();
+        $text_list = $file_content->getTextList();
+        $rev_list = $file_content->getRevList();
+        $blame_dict = $file_content->getBlameDict();
 
         $rows = array();
         foreach ($text_list as $k => $line) {
@@ -213,15 +218,15 @@ final class DiffusionBrowseFileController extends DiffusionController {
             'style' => $style,
           ),
           implode("\n", $rows));
-
         break;
 
       case 'highlighted':
       case 'blame':
       default:
         require_celerity_resource('syntax-highlighting-css');
-
-        list($text_list, $rev_list, $blame_dict) = $file_query->getBlameData();
+        $text_list = $file_content->getTextList();
+        $rev_list = $file_content->getRevList();
+        $blame_dict = $file_content->getBlameDict();
 
         $text_list = implode("\n", $text_list);
         $text_list = PhabricatorSyntaxHighlighter::highlightWithFilename(
@@ -230,7 +235,7 @@ final class DiffusionBrowseFileController extends DiffusionController {
         $text_list = explode("\n", $text_list);
 
         $rows = $this->buildDisplayRows($text_list, $rev_list, $blame_dict,
-          $needs_blame, $drequest, $file_query, $selected);
+          $needs_blame, $drequest, $selected);
 
         $corpus_table = javelin_tag(
           'table',
@@ -423,7 +428,6 @@ final class DiffusionBrowseFileController extends DiffusionController {
     array $blame_dict,
     $needs_blame,
     DiffusionRequest $drequest,
-    DiffusionFileContentQuery $file_query,
     $selected) {
 
     $handles = array();

src/applications/diffusion/data/DiffusionBranchInformation.php

@@ -30,7 +30,7 @@ final class DiffusionBranchInformation {
     foreach ($dicts as $dict) {
       $branches[] = id(new DiffusionBranchInformation())
         ->setName($dict['name'])
-        ->setHeadCommitIdentifier($dict['head_commit_identifier']);
+        ->setHeadCommitIdentifier($dict['headCommitIdentifier']);
     }
     return $branches;
   }
@@ -38,7 +38,7 @@ final class DiffusionBranchInformation {
   public function toDictionary() {
     return array(
       'name' => $this->getName(),
-      'head_commit_identifier' => $this->getHeadCommitIdentifier()
+      'headCommitIdentifier' => $this->getHeadCommitIdentifier()
     );
   }
 

src/applications/diffusion/data/DiffusionFileContent.php

@@ -3,14 +3,58 @@
 final class DiffusionFileContent {
 
   private $corpus;
+  private $blameDict;
+  private $revList;
+  private $textList;
 
-  final public function setCorpus($corpus) {
+  public function setTextList(array $text_list) {
+    $this->textList = $text_list;
+    return $this;
+  }
+  public function getTextList() {
+    return $this->textList;
+  }
+
+  public function setRevList(array $rev_list) {
+    $this->revList = $rev_list;
+    return $this;
+  }
+  public function getRevList() {
+    return $this->revList;
+  }
+
+  public function setBlameDict(array $blame_dict) {
+    $this->blameDict = $blame_dict;
+    return $this;
+  }
+  public function getBlameDict() {
+    return $this->blameDict;
+  }
+
+  public function setCorpus($corpus) {
     $this->corpus = $corpus;
     return $this;
   }
 
-  final public function getCorpus() {
+  public function getCorpus() {
     return $this->corpus;
   }
 
+  public function toDictionary() {
+    return array(
+      'corpus' => $this->getCorpus(),
+      'blameDict' => $this->getBlameDict(),
+      'revList' => $this->getRevList(),
+      'textList' => $this->getTextList()
+    );
+  }
+
+  public static function newFromConduit(array $dict) {
+    return id(new DiffusionFileContent())
+      ->setCorpus($dict['corpus'])
+      ->setBlameDict($dict['blameDict'])
+      ->setRevList($dict['revList'])
+      ->setTextList($dict['textList']);
+  }
+
 }

src/applications/diffusion/query/browse/DiffusionBrowseQuery.php

@@ -116,11 +116,17 @@ abstract class DiffusionBrowseQuery {
         'path'        => $readme->getFullPath(),
       ));
 
-    $content_query = DiffusionFileContentQuery::newFromDiffusionRequest(
-      $readme_request);
-    $content_query->setViewer($this->getViewer());
-    $content_query->loadFileContent();
-    $readme_content = $content_query->getRawData();
+    $file_content = DiffusionFileContent::newFromConduit(
+      DiffusionQuery::callConduitWithDiffusionRequest(
+        $this->getViewer(),
+        $readme_request,
+        'diffusion.filecontentquery',
+        array(
+          'commit' => $drequest->getStableCommitName(),
+          'path' => $readme->getFullPath(),
+          'needsBlame' => false,
+        )));
+    $readme_content = $file_content->getCorpus();
 
     if (preg_match('/\\.txt$/', $readme->getPath())) {
       $readme_content = phutil_escape_html_newlines($readme_content);

src/applications/diffusion/query/filecontent/DiffusionFileContentQuery.php

@@ -1,5 +1,11 @@
 <?php
 
+/**
+ * NOTE: this class should only be used where local access to the repository
+ * is guaranteed and NOT from within the Diffusion application. Diffusion
+ * should use Conduit method 'diffusion.filecontentquery' to get this sort
+ * of data.
+ */
 abstract class DiffusionFileContentQuery extends DiffusionQuery {
 
   private $needsBlame;
@@ -40,11 +46,30 @@ abstract class DiffusionFileContentQuery extends DiffusionQuery {
     return $this->fileContent->getCorpus();
   }
 
+  /**
+   * Pretty hairy function. If getNeedsBlame is false, this returns
+   *
+   *   ($text_list, array(), array())
+   *
+   * Where $text_list is the raw file content with trailing new lines stripped.
+   *
+   * If getNeedsBlame is true, this returns
+   *
+   *   ($text_list, $line_rev_dict, $blame_dict)
+   *
+   * Where $text_list is just the lines of code -- the raw file content will
+   * contain lots of blame data, $line_rev_dict is a dictionary of line number
+   * => revision id, and $blame_dict is another complicated data structure.
+   * In detail, $blame_dict contains [revision id][author] keys, as well
+   * as [commit id][authorPhid] and [commit id][epoch] keys.
+   *
+   * @return ($text_list, $line_rev_dict, $blame_dict)
+   */
   final public function getBlameData() {
     $raw_data = preg_replace('/\n$/', '', $this->getRawData());
 
     $text_list = array();
-    $rev_list = array();
+    $line_rev_dict = array();
     $blame_dict = array();
 
     if (!$this->getNeedsBlame()) {
@@ -56,14 +81,14 @@ abstract class DiffusionFileContentQuery extends DiffusionQuery {
 
         list($rev_id, $author, $text) = $lines[$k];
         $text_list[$k] = $text;
-        $rev_list[$k] = $rev_id;
+        $line_rev_dict[$k] = $rev_id;
       }
 
-      $rev_list = $this->processRevList($rev_list);
+      $line_rev_dict = $this->processRevList($line_rev_dict);
 
       foreach ($lines as $k => $line) {
         list($rev_id, $author, $text) = $line;
-        $rev_id = $rev_list[$k];
+        $rev_id = $line_rev_dict[$k];
 
         if (!isset($blame_dict[$rev_id])) {
           $blame_dict[$rev_id]['author'] = $author;
@@ -75,7 +100,7 @@ abstract class DiffusionFileContentQuery extends DiffusionQuery {
       $commits = id(new PhabricatorAuditCommitQuery())
         ->withIdentifiers(
           $repository->getID(),
-          array_unique($rev_list))
+          array_unique($line_rev_dict))
         ->execute();
 
       foreach ($commits as $commit) {
@@ -101,7 +126,7 @@ abstract class DiffusionFileContentQuery extends DiffusionQuery {
 
    }
 
-    return array($text_list, $rev_list, $blame_dict);
+    return array($text_list, $line_rev_dict, $blame_dict);
   }
 
   abstract protected function tokenizeLine($line);