From cfa265f02091a486bf6aea1fa09afaea08c5f9a6 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Sat, 10 May 2014 18:06:41 -0700
Subject: [PATCH] Make sure READMEs can hit the markup cache in Diffusion

Summary: Ref T2683. Normally not a big deal, but if a readme has some codeblocks missing the cache can slow things down.

Test Plan:
  - Verified we hit the cache.
  - Verified TOC still works.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5028, T2683

Differential Revision: https://secure.phabricator.com/D9049
---
 ...onduitAPI_diffusion_readmequery_Method.php | 31 +++++++++++++------
 .../PhabricatorRepositoryGraphCache.php       |  9 +++++-
 .../markup/PhabricatorMarkupEngine.php        |  5 +++
 .../markup/PhabricatorMarkupOneOff.php        | 14 ++++++++-
 4 files changed, 47 insertions(+), 12 deletions(-)

diff --git a/src/applications/diffusion/conduit/ConduitAPI_diffusion_readmequery_Method.php b/src/applications/diffusion/conduit/ConduitAPI_diffusion_readmequery_Method.php
index 23f18b544d..b6d781e3ef 100644
--- a/src/applications/diffusion/conduit/ConduitAPI_diffusion_readmequery_Method.php
+++ b/src/applications/diffusion/conduit/ConduitAPI_diffusion_readmequery_Method.php
@@ -85,19 +85,30 @@ final class ConduitAPI_diffusion_readmequery_Method
       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);
+      // 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 = PhutilRemarkupEngineRemarkupHeaderBlockRule::renderTableOfContents(
         $engine);
       if ($toc) {
-        $toc = phutil_tag_div('phabricator-remarkup-toc', array(
-          phutil_tag_div(
-            'phabricator-remarkup-toc-header',
-            pht('Table of Contents')),
-          $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);
       }
 
diff --git a/src/applications/repository/graphcache/PhabricatorRepositoryGraphCache.php b/src/applications/repository/graphcache/PhabricatorRepositoryGraphCache.php
index ae2f652bef..432bde057c 100644
--- a/src/applications/repository/graphcache/PhabricatorRepositoryGraphCache.php
+++ b/src/applications/repository/graphcache/PhabricatorRepositoryGraphCache.php
@@ -355,10 +355,17 @@ final class PhabricatorRepositoryGraphCache {
     // Build the actual data for the cache.
     foreach ($commit_ids as $commit_id) {
       $parent_ids = array();
-      if (isset($parents[$commit_id])) {
+      if (!empty($parents[$commit_id])) {
         foreach ($parents[$commit_id] as $row) {
           $parent_ids[] = (int)$row['parentCommitID'];
         }
+      } else {
+        // We expect all rows to have parents (commits with no parents get
+        // an explicit "0" placeholder). If we're in an older repository, the
+        // parent information might not have been populated yet. Decline to fill
+        // the cache if we don't have the parent information, since the fill
+        // will be incorrect.
+        continue;
       }
 
       if (isset($path_changes[$commit_id])) {
diff --git a/src/infrastructure/markup/PhabricatorMarkupEngine.php b/src/infrastructure/markup/PhabricatorMarkupEngine.php
index 64f3a03924..bedbc089da 100644
--- a/src/infrastructure/markup/PhabricatorMarkupEngine.php
+++ b/src/infrastructure/markup/PhabricatorMarkupEngine.php
@@ -374,6 +374,11 @@ final class PhabricatorMarkupEngine {
         $engine = self::newMarkupEngine(array());
         $engine->setConfig('preserve-linebreaks', false);
         break;
+      case 'diffusion-readme':
+        $engine = self::newMarkupEngine(array());
+        $engine->setConfig('preserve-linebreaks', false);
+        $engine->setConfig('header.generate-toc', true);
+        break;
       case 'diviner':
         $engine = self::newMarkupEngine(array());
         $engine->setConfig('preserve-linebreaks', false);
diff --git a/src/infrastructure/markup/PhabricatorMarkupOneOff.php b/src/infrastructure/markup/PhabricatorMarkupOneOff.php
index ec112e5c00..d3350bfd27 100644
--- a/src/infrastructure/markup/PhabricatorMarkupOneOff.php
+++ b/src/infrastructure/markup/PhabricatorMarkupOneOff.php
@@ -16,6 +16,16 @@ final class PhabricatorMarkupOneOff implements PhabricatorMarkupInterface {
 
   private $content;
   private $preserveLinebreaks;
+  private $engineRuleset;
+
+  public function setEngineRuleset($engine_ruleset) {
+    $this->engineRuleset = $engine_ruleset;
+    return $this;
+  }
+
+  public function getEngineRuleset() {
+    return $this->engineRuleset;
+  }
 
   public function setPreserveLinebreaks($preserve_linebreaks) {
     $this->preserveLinebreaks = $preserve_linebreaks;
@@ -36,7 +46,9 @@ final class PhabricatorMarkupOneOff implements PhabricatorMarkupInterface {
   }
 
   public function newMarkupEngine($field) {
-    if ($this->preserveLinebreaks) {
+    if ($this->engineRuleset) {
+      return PhabricatorMarkupEngine::getEngine($this->engineRuleset);
+    } else if ($this->preserveLinebreaks) {
       return PhabricatorMarkupEngine::getEngine();
     } else {
       return PhabricatorMarkupEngine::getEngine('nolinebreaks');