Diffusion - break out readme query all on its own

Summary: nice title. Ref T2784. Fixes T3171.

Test Plan: For all 3 VCS types, viewed phabricator repository and saw readme. browsed said repository and saw readme.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2784, T3171

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

src/__phutil_library_map__.php

@@ -160,6 +160,7 @@ phutil_register_library_map(array(
     'ConduitAPI_diffusion_lastmodifiedquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_lastmodifiedquery_Method.php',
     'ConduitAPI_diffusion_mergedcommitsquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_mergedcommitsquery_Method.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',
     'ConduitAPI_diffusion_searchquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_searchquery_Method.php',
     'ConduitAPI_diffusion_stablecommitnamequery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_stablecommitnamequery_Method.php',
@@ -1967,6 +1968,7 @@ phutil_register_library_map(array(
     'ConduitAPI_diffusion_lastmodifiedquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method',
     'ConduitAPI_diffusion_mergedcommitsquery_Method' => 'ConduitAPI_diffusion_abstractquery_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',
     'ConduitAPI_diffusion_searchquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method',
     'ConduitAPI_diffusion_stablecommitnamequery_Method' => 'ConduitAPI_diffusion_abstractquery_Method',

src/applications/diffusion/conduit/ConduitAPI_diffusion_browsequery_Method.php

@@ -21,97 +21,14 @@ final class ConduitAPI_diffusion_browsequery_Method
       'path' => 'optional string',
       'commit' => 'optional string',
       'needValidityOnly' => 'optional bool',
-      'renderReadme' => 'optional bool',
     );
   }
 
   protected function getResult(ConduitAPIRequest $request) {
     $result = parent::getResult($request);
-    if ($request->getValue('renderReadme', false)) {
-      $readme = $this->renderReadme($request, $result);
-    }
     return $result->toDictionary();
   }
 
-  final private function renderReadme(
-    ConduitAPIRequest $request,
-    DiffusionBrowseResultSet $result) {
-    $drequest = $this->getDiffusionRequest();
-
-    $readme = null;
-    foreach ($result->getPaths() as $result_path) {
-      $file_type = $result_path->getFileType();
-      if (($file_type != ArcanistDiffChangeType::FILE_NORMAL) &&
-          ($file_type != ArcanistDiffChangeType::FILE_TEXT)) {
-        // Skip directories, etc.
-        continue;
-      }
-
-      $path = $result_path->getPath();
-
-      if (preg_match('/^readme(|\.txt|\.remarkup|\.rainbow|\.md)$/i', $path)) {
-        $readme = $result_path;
-        break;
-      }
-    }
-
-    if (!$readme) {
-      return null;
-    }
-
-    $readme_request = DiffusionRequest::newFromDictionary(
-      array(
-        'user' => $request->getUser(),
-        'repository' => $drequest->getRepository(),
-        'commit' => $drequest->getStableCommitName(),
-        'path' => $readme->getFullPath(),
-      ));
-
-    $file_content = DiffusionFileContent::newFromConduit(
-      DiffusionQuery::callConduitWithDiffusionRequest(
-        $request->getUser(),
-        $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);
-
-      $class = null;
-    } else if (preg_match('/\\.rainbow$/', $readme->getPath())) {
-      $highlighter = new PhutilRainbowSyntaxHighlighter();
-      $readme_content = $highlighter
-        ->getHighlightFuture($readme_content)
-        ->resolve();
-      $readme_content = phutil_escape_html_newlines($readme_content);
-
-      require_celerity_resource('syntax-highlighting-css');
-      $class = 'remarkup-code';
-    } else {
-      // Markup extensionless files as remarkup so we get links and such.
-      $engine = PhabricatorMarkupEngine::newDiffusionMarkupEngine();
-      $engine->setConfig('viewer', $request->getUser());
-      $readme_content = $engine->markupText($readme_content);
-
-      $class = 'phabricator-remarkup';
-    }
-
-    $readme_content = phutil_tag(
-      'div',
-      array(
-        'class' => $class,
-      ),
-      $readme_content);
-
-    $result->setReadmeContent($readme_content);
-    return $result;
-  }
-
   protected function getGitResult(ConduitAPIRequest $request) {
     $drequest = $this->getDiffusionRequest();
     $repository = $drequest->getRepository();

src/applications/diffusion/conduit/ConduitAPI_diffusion_readmequery_Method.php

@@ -0,0 +1,106 @@
+<?php
+
+/**
+ * @group conduit
+ */
+final class ConduitAPI_diffusion_readmequery_Method
+  extends ConduitAPI_diffusion_abstractquery_Method {
+
+  public function getMethodDescription() {
+    return
+      'Retrieve any "readme" that can be found for a set of paths in '.
+      'repository.';
+  }
+
+  public function defineReturnType() {
+    return 'string';
+  }
+
+  protected function defineCustomParamTypes() {
+    return array(
+      'paths' => 'required array <string>',
+    );
+  }
+
+  protected function getResult(ConduitAPIRequest $request) {
+    $drequest = $this->getDiffusionRequest();
+    $path_dicts = $request->getValue('paths', array());
+    $paths = array();
+    foreach ($path_dicts as $dict) {
+      $paths[] = DiffusionRepositoryPath::newFromDictionary($dict);
+    }
+
+    $readme = '';
+    foreach ($paths as $result_path) {
+      $file_type = $result_path->getFileType();
+      if (($file_type != ArcanistDiffChangeType::FILE_NORMAL) &&
+          ($file_type != ArcanistDiffChangeType::FILE_TEXT)) {
+        // Skip directories, etc.
+        continue;
+      }
+
+      $path = $result_path->getPath();
+
+      if (preg_match('/^readme(|\.txt|\.remarkup|\.rainbow|\.md)$/i', $path)) {
+        $readme = $result_path;
+        break;
+      }
+    }
+
+    if (!$readme) {
+      return '';
+    }
+
+    $readme_request = DiffusionRequest::newFromDictionary(
+      array(
+        'user' => $request->getUser(),
+        'repository' => $drequest->getRepository(),
+        'commit' => $drequest->getStableCommitName(),
+        'path' => $readme->getFullPath(),
+      ));
+
+    $file_content = DiffusionFileContent::newFromConduit(
+      DiffusionQuery::callConduitWithDiffusionRequest(
+        $request->getUser(),
+        $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);
+
+      $class = null;
+    } else if (preg_match('/\\.rainbow$/', $readme->getPath())) {
+      $highlighter = new PhutilRainbowSyntaxHighlighter();
+      $readme_content = $highlighter
+        ->getHighlightFuture($readme_content)
+        ->resolve();
+      $readme_content = phutil_escape_html_newlines($readme_content);
+
+      require_celerity_resource('syntax-highlighting-css');
+      $class = 'remarkup-code';
+    } else {
+      // Markup extensionless files as remarkup so we get links and such.
+      $engine = PhabricatorMarkupEngine::newDiffusionMarkupEngine();
+      $engine->setConfig('viewer', $request->getUser());
+      $readme_content = $engine->markupText($readme_content);
+
+      $class = 'phabricator-remarkup';
+    }
+
+    $readme_content = phutil_tag(
+      'div',
+      array(
+        'class' => $class,
+      ),
+      $readme_content);
+
+    return $readme_content;
+  }
+
+}

src/applications/diffusion/controller/DiffusionBrowseController.php

@@ -15,7 +15,6 @@ final class DiffusionBrowseController extends DiffusionController {
           array(
             'path' => $drequest->getPath(),
             'commit' => $drequest->getCommit(),
-            'renderReadme' => true,
           )));
       $reason = $results->getReasonForEmptyResultSet();
       $is_file = ($reason == DiffusionBrowseResultSet::REASON_IS_FILE);
@@ -84,7 +83,11 @@ final class DiffusionBrowseController extends DiffusionController {
 
       $content[] = $this->buildOpenRevisions();
 
-      $readme = $results->getReadmeContent();
+      $readme = $this->callConduitWithDiffusionRequest(
+        'diffusion.readmequery',
+        array(
+          'paths' => $results->getPathDicts()
+        ));
       if ($readme) {
         $box = new PHUIBoxView();
         $box->setShadow(true);

src/applications/diffusion/controller/DiffusionRepositoryController.php

@@ -28,12 +28,10 @@ final class DiffusionRepositoryController extends DiffusionController {
         array(
           'path' => $drequest->getPath(),
           'commit' => $drequest->getCommit(),
-          'renderReadme' => true,
         )));
     $browse_paths = $browse_results->getPaths();
 
     $phids = array();
-
     foreach ($history as $item) {
       $data = $item->getCommitData();
       if ($data) {
@@ -57,10 +55,15 @@ final class DiffusionRepositoryController extends DiffusionController {
         }
       }
     }
-
     $phids = array_keys($phids);
     $handles = $this->loadViewerHandles($phids);
 
+    $readme = $this->callConduitWithDiffusionRequest(
+      'diffusion.readmequery',
+      array(
+       'paths' => $browse_results->getPathDicts()
+        ));
+
     $history_table = new DiffusionHistoryTableView();
     $history_table->setUser($this->getRequest()->getUser());
     $history_table->setDiffusionRequest($drequest);
@@ -109,7 +112,6 @@ final class DiffusionRepositoryController extends DiffusionController {
 
     $content[] = $this->buildBranchListTable($drequest);
 
-    $readme = $browse_results->getReadmeContent();
     if ($readme) {
       $box = new PHUIBoxView();
       $box->setShadow(true);

src/applications/diffusion/data/DiffusionBrowseResultSet.php

@@ -14,7 +14,6 @@ final class DiffusionBrowseResultSet {
   private $reasonForEmptyResultSet;
   private $existedAtCommit;
   private $deletedAtCommit;
-  private $readmeContent;
 
   public function setPaths(array $paths) {
     assert_instances_of($paths, 'DiffusionRepositoryPath');
@@ -57,27 +56,23 @@ final class DiffusionBrowseResultSet {
     return $this->deletedAtCommit;
   }
 
-  public function setReadmeContent($readme_content) {
-    $this->readmeContent = $readme_content;
-    return $this;
-  }
-  public function getReadmeContent() {
-    return $this->readmeContent;
-  }
-
   public function toDictionary() {
-    $paths = $this->getPaths();
+    $paths = $this->getPathDicts();
-    if ($paths) {
-      $paths = mpull($paths, 'toDictionary');
-    }
 
     return array(
       'paths' => $paths,
       'isValidResults' => $this->isValidResults(),
       'reasonForEmptyResultSet' => $this->getReasonForEmptyResultSet(),
       'existedAtCommit' => $this->getExistedAtCommit(),
-      'deletedAtCommit' => $this->getDeletedAtCommit(),
+      'deletedAtCommit' => $this->getDeletedAtCommit());
-      'readmeContent' => $this->getReadmeContent());
+  }
+
+  public function getPathDicts() {
+    $paths = $this->getPaths();
+    if ($paths) {
+      return mpull($paths, 'toDictionary');
+    }
+    return array();
   }
 
   public static function newFromConduit(array $data) {
@@ -91,7 +86,6 @@ final class DiffusionBrowseResultSet {
       ->setIsValidResults($data['isValidResults'])
       ->setReasonForEmptyResultSet($data['reasonForEmptyResultSet'])
       ->setExistedAtCommit($data['existedAtCommit'])
-      ->setDeletedAtCommit($data['deletedAtCommit'])
+      ->setDeletedAtCommit($data['deletedAtCommit']);
-      ->setReadmeContent($data['readmeContent']);
   }
 }