Fix diffusion.readmequery to work in a cluster enviroment

Summary:
Ref T2783. This method is kind of goofballs:

  - We send a big list of paths to it.
  - It sends back a giant blob of HTML.

Instead, just figure out the path we want locally, then fetch the content with `diffusion.filecontentquery`.

Test Plan:
  - Viewed main view and directory view, saw a README.
  - See screenshots.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T2783

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

src/__phutil_library_map__.php

@@ -542,7 +542,7 @@ phutil_register_library_map(array(
     'DiffusionQueryPathsConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionQueryPathsConduitAPIMethod.php',
     'DiffusionRawDiffQuery' => 'applications/diffusion/query/rawdiff/DiffusionRawDiffQuery.php',
     'DiffusionRawDiffQueryConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionRawDiffQueryConduitAPIMethod.php',
-    'DiffusionReadmeQueryConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionReadmeQueryConduitAPIMethod.php',
+    'DiffusionReadmeView' => 'applications/diffusion/view/DiffusionReadmeView.php',
     'DiffusionRefNotFoundException' => 'applications/diffusion/exception/DiffusionRefNotFoundException.php',
     'DiffusionRefsQueryConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionRefsQueryConduitAPIMethod.php',
     'DiffusionRenameHistoryQuery' => 'applications/diffusion/query/DiffusionRenameHistoryQuery.php',
@@ -3573,7 +3573,7 @@ phutil_register_library_map(array(
     'DiffusionQueryPathsConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod',
     'DiffusionRawDiffQuery' => 'DiffusionQuery',
     'DiffusionRawDiffQueryConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod',
-    'DiffusionReadmeQueryConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod',
+    'DiffusionReadmeView' => 'DiffusionView',
     'DiffusionRefNotFoundException' => 'Exception',
     'DiffusionRefsQueryConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod',
     'DiffusionRepositoryController' => 'DiffusionController',

src/applications/diffusion/conduit/DiffusionReadmeQueryConduitAPIMethod.php

@@ -1,167 +0,0 @@
-<?php
-
-final class DiffusionReadmeQueryConduitAPIMethod
-  extends DiffusionQueryConduitAPIMethod {
-
-  public function getAPIMethodName() {
-    return 'diffusion.readmequery';
-  }
-
-  public function getMethodDescription() {
-    return
-      pht('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>',
-      'commit' => 'optional 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);
-    }
-
-    $best = -1;
-    $readme = '';
-    $best_render_type = 'plain';
-    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 = strtolower($result_path->getPath());
-
-      if ($path === 'readme') {
-        $path .= '.remarkup';
-      }
-
-      if (strncmp($path, 'readme.', 7) !== 0) {
-        continue;
-      }
-
-      $priority = 0;
-      switch (substr($path, 7)) {
-        case 'remarkup':
-          $priority = 100;
-          $render_type = 'remarkup';
-          break;
-        case 'rainbow':
-          $priority = 90;
-          $render_type = 'rainbow';
-          break;
-        case 'md':
-          $priority = 50;
-          $render_type = 'remarkup';
-          break;
-        case 'txt':
-          $priority = 10;
-          $render_type = 'plain';
-          break;
-        default:
-          $priority = 0;
-          $render_type = 'plain';
-          break;
-      }
-
-      if ($priority > $best) {
-        $best = $priority;
-        $readme = $result_path;
-        $best_render_type = $render_type;
-      }
-    }
-
-    if (!$readme) {
-      return '';
-    }
-
-    $readme_request = DiffusionRequest::newFromDictionary(
-      array(
-        'user' => $request->getUser(),
-        'repository' => $drequest->getRepository(),
-        'commit' => $drequest->getStableCommit(),
-        'path' => $readme->getFullPath(),
-      ));
-
-    $file_content = DiffusionFileContent::newFromConduit(
-      DiffusionQuery::callConduitWithDiffusionRequest(
-        $request->getUser(),
-        $readme_request,
-        'diffusion.filecontentquery',
-        array(
-          'commit' => $drequest->getStableCommit(),
-          'path' => $readme->getFullPath(),
-          'needsBlame' => false,
-        )));
-    $readme_content = $file_content->getCorpus();
-
-    switch ($best_render_type) {
-      case 'plain':
-        $readme_content = phutil_escape_html_newlines($readme_content);
-        $class = null;
-        break;
-      case 'rainbow':
-        $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';
-        break;
-      case 'remarkup':
-        // TODO: This is sketchy, but make sure we hit the markup cache.
-        $markup_object = id(new PhabricatorMarkupOneOff())
-          ->setEngineRuleset('diffusion-readme')
-          ->setContent($readme_content);
-        $markup_field = 'default';
-
-        $readme_content = id(new PhabricatorMarkupEngine())
-          ->setViewer($request->getUser())
-          ->addObject($markup_object, $markup_field)
-          ->process()
-          ->getOutput($markup_object, $markup_field);
-
-        $engine = $markup_object->newMarkupEngine($markup_field);
-        $toc = PhutilRemarkupHeaderBlockRule::renderTableOfContents($engine);
-        if ($toc) {
-          $toc = phutil_tag_div(
-            'phabricator-remarkup-toc',
-            array(
-              phutil_tag_div(
-                'phabricator-remarkup-toc-header',
-                pht('Table of Contents')),
-              $toc,
-            ));
-          $readme_content = array($toc, $readme_content);
-        }
-
-        $class = 'phabricator-remarkup';
-        break;
-    }
-
-    $readme_content = phutil_tag(
-      'div',
-      array(
-        'class' => $class,
-      ),
-      $readme_content);
-
-    return $readme_content;
-  }
-
-}

src/applications/diffusion/controller/DiffusionBrowseDirectoryController.php

@@ -66,22 +66,21 @@ final class DiffusionBrowseDirectoryController
 
     $content[] = $this->buildOpenRevisions();
 
-    $readme = $this->callConduitWithDiffusionRequest(
-      'diffusion.readmequery',
-      array(
-        'paths' => $results->getPathDicts(),
-        'commit' => $drequest->getStableCommit(),
-      ));
-    if ($readme) {
-      $box = new PHUIBoxView();
-      $box->appendChild($readme);
-      $box->addPadding(PHUI::PADDING_LARGE);
 
-      $object_box = id(new PHUIObjectBoxView())
+    $readme_path = $results->getReadmePath();
-        ->setHeaderText(pht('README'))
+    if ($readme_path) {
-        ->appendChild($box);
+      $readme_content = $this->callConduitWithDiffusionRequest(
-
+        'diffusion.filecontentquery',
-      $content[] = $object_box;
+        array(
+          'path' => $readme_path,
+          'commit' => $drequest->getStableCommit(),
+        ));
+      if ($readme_content) {
+        $content[] = id(new DiffusionReadmeView())
+          ->setUser($this->getViewer())
+          ->setPath($readme_path)
+          ->setContent($readme_content['corpus']);
+      }
     }
 
     $crumbs = $this->buildCrumbs(

src/applications/diffusion/controller/DiffusionRepositoryController.php

@@ -151,15 +151,23 @@ final class DiffusionRepositoryController extends DiffusionController {
     $phids = array_keys($phids);
     $handles = $this->loadViewerHandles($phids);
 
+    $readme = null;
     if ($browse_results) {
-      $readme = $this->callConduitWithDiffusionRequest(
+      $readme_path = $browse_results->getReadmePath();
-        'diffusion.readmequery',
+      if ($readme_path) {
-        array(
+        $readme_content = $this->callConduitWithDiffusionRequest(
-         'paths' => $browse_results->getPathDicts(),
+          'diffusion.filecontentquery',
-         'commit' => $drequest->getStableCommit(),
+          array(
-        ));
+            'path' => $readme_path,
-    } else {
+            'commit' => $drequest->getStableCommit(),
-      $readme = null;
+          ));
+        if ($readme_content) {
+          $readme = id(new DiffusionReadmeView())
+            ->setUser($this->getViewer())
+            ->setPath($readme_path)
+            ->setContent($readme_content['corpus']);
+        }
+      }
     }
 
     $content[] = $this->buildBrowseTable(
@@ -195,14 +203,7 @@ final class DiffusionRepositoryController extends DiffusionController {
     }
 
     if ($readme) {
-      $box = new PHUIBoxView();
+      $content[] = $readme;
-      $box->appendChild($readme);
-      $box->addPadding(PHUI::PADDING_LARGE);
-
-      $panel = new PHUIObjectBoxView();
-      $panel->setHeaderText(pht('README'));
-      $panel->appendChild($box);
-      $content[] = $panel;
     }
 
     return $content;

src/applications/diffusion/data/DiffusionBrowseResultSet.php

@@ -76,6 +76,76 @@ final class DiffusionBrowseResultSet {
     return array();
   }
 
+  /**
+   * Get the best README file in this result set, if one exists.
+   *
+   * Callers should normally use `diffusion.filecontentquery` to pull README
+   * content.
+   *
+   * @return string|null Full path to best README, or null if one does not
+   *   exist.
+   */
+  public function getReadmePath() {
+    $allowed_types = array(
+      ArcanistDiffChangeType::FILE_NORMAL => true,
+      ArcanistDiffChangeType::FILE_TEXT => true,
+    );
+
+    $candidates = array();
+    foreach ($this->getPaths() as $path_object) {
+      if (empty($allowed_types[$path_object->getFileType()])) {
+        // Skip directories, images, etc.
+        continue;
+      }
+
+      $local_path = $path_object->getPath();
+      if (!preg_match('/^readme(\.|$)/i', $local_path)) {
+        // Skip files not named "README".
+        continue;
+      }
+
+      $full_path = $path_object->getFullPath();
+      $candidates[$full_path] = self::getReadmePriority($local_path);
+    }
+
+    if (!$candidates) {
+      return null;
+    }
+
+    arsort($candidates);
+    return head_key($candidates);
+  }
+
+  /**
+   * Get the priority of a README file.
+   *
+   * When a directory contains several README files, this function scores them
+   * so the caller can select a preferred file. See @{method:getReadmePath}.
+   *
+   * @param string Local README path, like "README.txt".
+   * @return int Priority score, with higher being more preferred.
+   */
+  public static function getReadmePriority($path) {
+    $path = phutil_utf8_strtolower($path);
+    if ($path == 'readme') {
+      return 90;
+    }
+
+    $ext = last(explode('.', $path));
+    switch ($ext) {
+      case 'remarkup':
+        return 100;
+      case 'rainbow':
+        return 80;
+      case 'md':
+        return 70;
+      case 'txt':
+        return 60;
+      default:
+        return 50;
+    }
+  }
+
   public static function newFromConduit(array $data) {
     $paths = array();
     $path_dicts = $data['paths'];

src/applications/diffusion/view/DiffusionReadmeView.php

@@ -0,0 +1,124 @@
+<?php
+
+final class DiffusionReadmeView extends DiffusionView {
+
+  private $path;
+  private $content;
+
+  public function setPath($path) {
+    $this->path = $path;
+    return $this;
+  }
+
+  public function getPath() {
+    return $this->path;
+  }
+
+  public function setContent($content) {
+    $this->content = $content;
+    return $this;
+  }
+
+  public function getContent() {
+    return $this->content;
+  }
+
+  /**
+   * Get the markup language a README should be interpreted as.
+   *
+   * @param string Local README path, like "README.txt".
+   * @return string Best markup interpreter (like "remarkup") for this file.
+   */
+   private function getReadmeLanguage($path) {
+    $path = phutil_utf8_strtolower($path);
+    if ($path == 'readme') {
+      return 'remarkup';
+    }
+
+    $ext = last(explode('.', $path));
+    switch ($ext) {
+      case 'remarkup':
+      case 'md':
+        return 'remarkup';
+      case 'rainbow':
+        return 'rainbow';
+      case 'txt':
+      default:
+        return 'text';
+    }
+  }
+
+
+  public function render() {
+    $readme_path = $this->getPath();
+    $readme_name = basename($readme_path);
+    $interpreter = $this->getReadmeLanguage($readme_name);
+
+    $content = $this->getContent();
+
+    $class = null;
+    switch ($interpreter) {
+      case 'remarkup':
+        // TODO: This is sketchy, but make sure we hit the markup cache.
+        $markup_object = id(new PhabricatorMarkupOneOff())
+          ->setEngineRuleset('diffusion-readme')
+          ->setContent($content);
+        $markup_field = 'default';
+
+        $content = id(new PhabricatorMarkupEngine())
+          ->setViewer($this->getUser())
+          ->addObject($markup_object, $markup_field)
+          ->process()
+          ->getOutput($markup_object, $markup_field);
+
+        $engine = $markup_object->newMarkupEngine($markup_field);
+        $toc = PhutilRemarkupHeaderBlockRule::renderTableOfContents($engine);
+        if ($toc) {
+          $toc = phutil_tag_div(
+            'phabricator-remarkup-toc',
+            array(
+              phutil_tag_div(
+                'phabricator-remarkup-toc-header',
+                pht('Table of Contents')),
+              $toc,
+            ));
+          $content = array($toc, $content);
+        }
+
+        $readme_content = $content;
+        $class = 'phabricator-remarkup';
+        break;
+      case 'rainbow':
+        $content = id(new PhutilRainbowSyntaxHighlighter())
+          ->getHighlightFuture($content)
+          ->resolve();
+        $readme_content = phutil_escape_html_newlines($content);
+
+        require_celerity_resource('syntax-highlighting-css');
+        $class = 'remarkup-code';
+        break;
+      default:
+      case 'text':
+        $readme_content = phutil_escape_html_newlines($content);
+        break;
+    }
+
+    $readme_content = phutil_tag(
+      'div',
+      array(
+        'class' => $class,
+      ),
+      $readme_content);
+
+    $box = new PHUIBoxView();
+    $box->appendChild($readme_content);
+    $box->addPadding(PHUI::PADDING_LARGE);
+
+    $object_box = id(new PHUIObjectBoxView())
+      ->setHeaderText($readme_name)
+      ->appendChild($box);
+
+    return $object_box;
+  }
+
+}