diff --git a/resources/sql/patches/harbormasterobject.sql b/resources/sql/patches/harbormasterobject.sql index c1eb93cb2c..f252ae8e5e 100644 --- a/resources/sql/patches/harbormasterobject.sql +++ b/resources/sql/patches/harbormasterobject.sql @@ -4,7 +4,7 @@ CREATE TABLE {$NAMESPACE}_harbormaster.harbormaster_object ( name VARCHAR(255) COLLATE utf8_general_ci, dateCreated INT UNSIGNED NOT NULL, dateModified INT UNSIGNED NOT NULL -); +) ENGINE=InnoDB, COLLATE utf8_general_ci; CREATE TABLE {$NAMESPACE}_harbormaster.edge ( src VARCHAR(64) NOT NULL COLLATE utf8_bin, diff --git a/resources/sql/patches/markupcache.sql b/resources/sql/patches/markupcache.sql new file mode 100644 index 0000000000..e13a236f25 --- /dev/null +++ b/resources/sql/patches/markupcache.sql @@ -0,0 +1,10 @@ +CREATE TABLE {$NAMESPACE}_cache.cache_markupcache ( + id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, + cacheKey VARCHAR(128) NOT NULL collate utf8_bin, + cacheData LONGTEXT NOT NULL COLLATE utf8_bin, + metadata LONGTEXT NOT NULL COLLATE utf8_bin, + dateCreated INT UNSIGNED NOT NULL, + dateModified INT UNSIGNED NOT NULL, + UNIQUE KEY (cacheKey), + KEY (dateCreated) +) ENGINE=InnoDB, COLLATE utf8_general_ci; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 60ca2aa07f..a7510a5d9d 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -563,6 +563,7 @@ phutil_register_library_map(array( 'PhabricatorAuthController' => 'applications/auth/controller/PhabricatorAuthController.php', 'PhabricatorBaseEnglishTranslation' => 'infrastructure/internationalization/PhabricatorBaseEnglishTranslation.php', 'PhabricatorBuiltinPatchList' => 'infrastructure/storage/patch/PhabricatorBuiltinPatchList.php', + 'PhabricatorCacheDAO' => 'applications/cache/storage/PhabricatorCacheDAO.php', 'PhabricatorCalendarBrowseController' => 'applications/calendar/controller/PhabricatorCalendarBrowseController.php', 'PhabricatorCalendarController' => 'applications/calendar/controller/PhabricatorCalendarController.php', 'PhabricatorCalendarDAO' => 'applications/calendar/storage/PhabricatorCalendarDAO.php', @@ -738,7 +739,9 @@ phutil_register_library_map(array( 'PhabricatorMailImplementationSendGridAdapter' => 'applications/metamta/adapter/PhabricatorMailImplementationSendGridAdapter.php', 'PhabricatorMailImplementationTestAdapter' => 'applications/metamta/adapter/PhabricatorMailImplementationTestAdapter.php', 'PhabricatorMailReplyHandler' => 'applications/metamta/replyhandler/PhabricatorMailReplyHandler.php', + 'PhabricatorMarkupCache' => 'applications/cache/storage/PhabricatorMarkupCache.php', 'PhabricatorMarkupEngine' => 'infrastructure/markup/PhabricatorMarkupEngine.php', + 'PhabricatorMarkupInterface' => 'infrastructure/markup/PhabricatorMarkupInterface.php', 'PhabricatorMercurialGraphStream' => 'applications/repository/daemon/PhabricatorMercurialGraphStream.php', 'PhabricatorMetaMTAAttachment' => 'applications/metamta/storage/PhabricatorMetaMTAAttachment.php', 'PhabricatorMetaMTAController' => 'applications/metamta/controller/PhabricatorMetaMTAController.php', @@ -1590,6 +1593,7 @@ phutil_register_library_map(array( 'PhabricatorAuthController' => 'PhabricatorController', 'PhabricatorBaseEnglishTranslation' => 'PhabricatorTranslation', 'PhabricatorBuiltinPatchList' => 'PhabricatorSQLPatchList', + 'PhabricatorCacheDAO' => 'PhabricatorLiskDAO', 'PhabricatorCalendarBrowseController' => 'PhabricatorCalendarController', 'PhabricatorCalendarController' => 'PhabricatorController', 'PhabricatorCalendarDAO' => 'PhabricatorLiskDAO', @@ -1742,6 +1746,7 @@ phutil_register_library_map(array( 'PhabricatorMailImplementationPHPMailerLiteAdapter' => 'PhabricatorMailImplementationAdapter', 'PhabricatorMailImplementationSendGridAdapter' => 'PhabricatorMailImplementationAdapter', 'PhabricatorMailImplementationTestAdapter' => 'PhabricatorMailImplementationAdapter', + 'PhabricatorMarkupCache' => 'PhabricatorCacheDAO', 'PhabricatorMetaMTAController' => 'PhabricatorController', 'PhabricatorMetaMTADAO' => 'PhabricatorLiskDAO', 'PhabricatorMetaMTAEmailBodyParserTestCase' => 'PhabricatorTestCase', @@ -2037,7 +2042,11 @@ phutil_register_library_map(array( 'PhrictionDAO' => 'PhabricatorLiskDAO', 'PhrictionDeleteController' => 'PhrictionController', 'PhrictionDiffController' => 'PhrictionController', - 'PhrictionDocument' => 'PhrictionDAO', + 'PhrictionDocument' => + array( + 0 => 'PhrictionDAO', + 1 => 'PhabricatorMarkupInterface', + ), 'PhrictionDocumentController' => 'PhrictionController', 'PhrictionDocumentPreviewController' => 'PhrictionController', 'PhrictionDocumentStatus' => 'PhrictionConstants', diff --git a/src/applications/cache/storage/PhabricatorCacheDAO.php b/src/applications/cache/storage/PhabricatorCacheDAO.php new file mode 100644 index 0000000000..4d4dc4a72f --- /dev/null +++ b/src/applications/cache/storage/PhabricatorCacheDAO.php @@ -0,0 +1,25 @@ + array( + 'cacheData' => self::SERIALIZATION_PHP, + 'metadata' => self::SERIALIZATION_JSON, + ), + ) + parent::getConfiguration(); + } + +} diff --git a/src/applications/phriction/controller/PhrictionHistoryController.php b/src/applications/phriction/controller/PhrictionHistoryController.php index c6af894098..e266a6d7a0 100644 --- a/src/applications/phriction/controller/PhrictionHistoryController.php +++ b/src/applications/phriction/controller/PhrictionHistoryController.php @@ -61,7 +61,7 @@ final class PhrictionHistoryController $rows = array(); foreach ($history as $content) { - $uri = PhrictionDocument::getSlugURI($document->getSlug()); + $slug_uri = PhrictionDocument::getSlugURI($document->getSlug()); $version = $content->getVersion(); $diff_uri = new PhutilURI('/phriction/diff/'.$document->getID().'/'); @@ -102,7 +102,7 @@ final class PhrictionHistoryController phutil_render_tag( 'a', array( - 'href' => $uri.'?v='.$version, + 'href' => $slug_uri.'?v='.$version, ), 'Version '.$version), $handles[$content->getAuthorPHID()]->renderLink(), diff --git a/src/applications/phriction/storage/PhrictionContent.php b/src/applications/phriction/storage/PhrictionContent.php index b03e731906..da84fb1ce0 100644 --- a/src/applications/phriction/storage/PhrictionContent.php +++ b/src/applications/phriction/storage/PhrictionContent.php @@ -17,9 +17,14 @@ */ /** + * @task markup Markup Interface + * * @group phriction */ -final class PhrictionContent extends PhrictionDAO { +final class PhrictionContent extends PhrictionDAO + implements PhabricatorMarkupInterface { + + const MARKUP_FIELD_BODY = 'markup:body'; protected $id; protected $documentID; @@ -35,11 +40,55 @@ final class PhrictionContent extends PhrictionDAO { protected $changeRef; public function renderContent() { - $engine = PhabricatorMarkupEngine::newPhrictionMarkupEngine(); - $markup = $engine->markupText($this->getContent()); + return PhabricatorMarkupEngine::renderOneObject( + $this, + self::MARKUP_FIELD_BODY); + } + + +/* -( Markup Interface )--------------------------------------------------- */ + + + /** + * @task markup + */ + public function getMarkupFieldKey($field) { + if ($this->shouldUseMarkupCache($field)) { + $id = $this->getID(); + } else { + $id = PhabricatorHash::digest($this->getMarkupText($field)); + } + return "phriction:{$field}:{$id}"; + } + + + /** + * @task markup + */ + public function getMarkupText($field) { + return $this->getContent(); + } + + + /** + * @task markup + */ + public function newMarkupEngine($field) { + return PhabricatorMarkupEngine::newPhrictionMarkupEngine(); + } + + + /** + * @task markup + */ + public function didMarkupText( + $field, + $output, + PhutilMarkupEngine $engine) { $toc = PhutilRemarkupEngineRemarkupHeaderBlockRule::renderTableOfContents( $engine); + if ($toc) { $toc = '
'. @@ -53,8 +102,17 @@ final class PhrictionContent extends PhrictionDAO { return '
'. $toc. - $markup. + $output. '
'; } + + /** + * @task markup + */ + public function shouldUseMarkupCache($field) { + return (bool)$this->getID(); + } + + } diff --git a/src/infrastructure/markup/PhabricatorMarkupEngine.php b/src/infrastructure/markup/PhabricatorMarkupEngine.php index c64527167e..fd03ef3178 100644 --- a/src/infrastructure/markup/PhabricatorMarkupEngine.php +++ b/src/infrastructure/markup/PhabricatorMarkupEngine.php @@ -16,41 +16,268 @@ * limitations under the License. */ -class PhabricatorMarkupEngine { +/** + * Manages markup engine selection, configuration, application, caching and + * pipelining. + * + * @{class:PhabricatorMarkupEngine} can be used to render objects which + * implement @{interface:PhabricatorMarkupInterface} in a batched, cache-aware + * way. For example, if you have a list of comments written in remarkup (and + * the objects implement the correct interface) you can render them by first + * building an engine and adding the fields with @{method:addObject}. + * + * $field = 'field:body'; // Field you want to render. Each object exposes + * // one or more fields of markup. + * + * $engine = new PhabricatorMarkupEngine(); + * foreach ($comments as $comment) { + * $engine->addObject($comment, $field); + * } + * + * Now, call @{method:process} to perform the actual cache/rendering + * step. This is a heavyweight call which does batched data access and + * transforms the markup into output. + * + * $engine->process(); + * + * Finally, do something with the results: + * + * $results = array(); + * foreach ($comments as $comment) { + * $results[] = $engine->getOutput($comment, $field); + * } + * + * If you have a single object to render, you can use the convenience method + * @{method:renderOneObject}. + * + * @task markup Markup Pipeline + * @task engine Engine Construction + */ +final class PhabricatorMarkupEngine { - public static function extractPHIDsFromMentions(array $content_blocks) { - $mentions = array(); + private $objects = array(); - $engine = self::newDifferentialMarkupEngine(); - foreach ($content_blocks as $content_block) { - $engine->markupText($content_block); - $phids = $engine->getTextMetadata( - PhabricatorRemarkupRuleMention::KEY_MENTIONED, - array()); - $mentions += $phids; - } +/* -( Markup Pipeline )---------------------------------------------------- */ - return $mentions; + + /** + * Convenience method for pushing a single object through the markup + * pipeline. + * + * @param PhabricatorMarkupInterface The object to render. + * @param string The field to render. + * @return string Marked up output. + * @task markup + */ + public static function renderOneObject( + PhabricatorMarkupInterface $object, + $field) { + return id(new PhabricatorMarkupEngine()) + ->addObject($object, $field) + ->process() + ->getOutput($object, $field); } + + /** + * Queue an object for markup generation when @{method:process} is + * called. You can retrieve the output later with @{method:getOutput}. + * + * @param PhabricatorMarkupInterface The object to render. + * @param string The field to render. + * @return this + * @task markup + */ + public function addObject(PhabricatorMarkupInterface $object, $field) { + $key = $this->getMarkupFieldKey($object, $field); + $this->objects[$key] = array( + 'object' => $object, + 'field' => $field, + ); + + return $this; + } + + + /** + * Process objects queued with @{method:addObject}. You can then retrieve + * the output with @{method:getOutput}. + * + * @return this + * @task markup + */ + public function process() { + $keys = array(); + foreach ($this->objects as $key => $info) { + if (!isset($info['markup'])) { + $keys[] = $key; + } + } + + if (!$keys) { + return; + } + + $objects = array_select_keys($this->objects, $keys); + + // Build all the markup engines. We need an engine for each field whether + // we have a cache or not, since we still need to postprocess the cache. + $engines = array(); + foreach ($objects as $key => $info) { + $engines[$key] = $info['object']->newMarkupEngine($info['field']); + } + + // Load or build the preprocessor caches. + $blocks = $this->loadPreprocessorCaches($engines, $objects); + + // Finalize the output. + foreach ($objects as $key => $info) { + $data = $blocks[$key]->getCacheData(); + $engine = $engines[$key]; + $field = $info['field']; + $object = $info['object']; + + $output = $engine->postprocessText($data); + $output = $object->didMarkupText($field, $output, $engine); + $this->objects[$key]['output'] = $output; + } + + return $this; + } + + + /** + * Get the output of markup processing for a field queued with + * @{method:addObject}. Before you can call this method, you must call + * @{method:process}. + * + * @param PhabricatorMarkupInterface The object to retrieve. + * @param string The field to retrieve. + * @return string Processed output. + * @task markup + */ + public function getOutput(PhabricatorMarkupInterface $object, $field) { + $key = $this->getMarkupFieldKey($object, $field); + + if (empty($this->objects[$key])) { + throw new Exception( + "Call addObject() before getOutput() (key = '{$key}')."); + } + + if (!isset($this->objects[$key]['output'])) { + throw new Exception( + "Call process() before getOutput()."); + } + + return $this->objects[$key]['output']; + } + + + /** + * @task markup + */ + private function getMarkupFieldKey( + PhabricatorMarkupInterface $object, + $field) { + return $object->getMarkupFieldKey($field); + } + + + /** + * @task markup + */ + private function loadPreprocessorCaches(array $engines, array $objects) { + $blocks = array(); + + $use_cache = array(); + foreach ($objects as $key => $info) { + if ($info['object']->shouldUseMarkupCache($info['field'])) { + $use_cache[$key] = true; + } + } + + if ($use_cache) { + $blocks = id(new PhabricatorMarkupCache())->loadAllWhere( + 'cacheKey IN (%Ls)', + array_keys($use_cache)); + $blocks = mpull($blocks, null, 'getCacheKey'); + } + + foreach ($objects as $key => $info) { + if (isset($blocks[$key])) { + // If we already have a preprocessing cache, we don't need to rebuild + // it. + continue; + } + + $text = $info['object']->getMarkupText($info['field']); + $data = $engines[$key]->preprocessText($text); + + // NOTE: This is just debugging information to help sort out cache issues. + // If one machine is misconfigured and poisoning caches you can use this + // field to hunt it down. + + $metadata = array( + 'host' => php_uname('n'), + ); + + $blocks[$key] = id(new PhabricatorMarkupCache()) + ->setCacheKey($key) + ->setCacheData($data) + ->setMetadata($metadata); + + if (isset($use_cache[$key])) { + // This is just filling a cache and always safe, even on a read pathway. + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); + try { + $blocks[$key]->save(); + } catch (AphrontQueryDuplicateKeyException $ex) { + // Ignore this, we just raced to write the cache. + } + unset($unguarded); + } + } + + return $blocks; + } + + +/* -( Engine Construction )------------------------------------------------ */ + + + /** + * @task engine + */ public static function newManiphestMarkupEngine() { return self::newMarkupEngine(array( )); } + + /** + * @task engine + */ public static function newPhrictionMarkupEngine() { return self::newMarkupEngine(array( 'header.generate-toc' => true, )); } + + /** + * @task engine + */ public static function newPhameMarkupEngine() { return self::newMarkupEngine(array( 'macros' => false, )); } + + /** + * @task engine + */ public static function newFeedMarkupEngine() { return self::newMarkupEngine( array( @@ -61,6 +288,10 @@ class PhabricatorMarkupEngine { )); } + + /** + * @task engine + */ public static function newDifferentialMarkupEngine(array $options = array()) { return self::newMarkupEngine(array( 'custom-inline' => PhabricatorEnv::getEnvConfig( @@ -71,21 +302,37 @@ class PhabricatorMarkupEngine { )); } + + /** + * @task engine + */ public static function newDiffusionMarkupEngine(array $options = array()) { return self::newMarkupEngine(array( )); } + + /** + * @task engine + */ public static function newProfileMarkupEngine() { return self::newMarkupEngine(array( )); } + + /** + * @task engine + */ public static function newSlowvoteMarkupEngine() { return self::newMarkupEngine(array( )); } + + /** + * @task engine + */ private static function getMarkupEngineDefaultConfiguration() { return array( 'pygments' => PhabricatorEnv::getEnvConfig('pygments.enabled'), @@ -104,6 +351,10 @@ class PhabricatorMarkupEngine { ); } + + /** + * @task engine + */ private static function newMarkupEngine(array $options) { $options += self::getMarkupEngineDefaultConfiguration(); @@ -198,4 +449,20 @@ class PhabricatorMarkupEngine { return $engine; } + public static function extractPHIDsFromMentions(array $content_blocks) { + $mentions = array(); + + $engine = self::newDifferentialMarkupEngine(); + + foreach ($content_blocks as $content_block) { + $engine->markupText($content_block); + $phids = $engine->getTextMetadata( + PhabricatorRemarkupRuleMention::KEY_MENTIONED, + array()); + $mentions += $phids; + } + + return $mentions; + } + } diff --git a/src/infrastructure/markup/PhabricatorMarkupInterface.php b/src/infrastructure/markup/PhabricatorMarkupInterface.php new file mode 100644 index 0000000000..a3c96449c2 --- /dev/null +++ b/src/infrastructure/markup/PhabricatorMarkupInterface.php @@ -0,0 +1,103 @@ + 'db', 'name' => 'xhpastview', ), + 'db.cache' => array( + 'type' => 'db', + 'name' => 'cache', + ), '0000.legacy.sql' => array( 'type' => 'sql', 'name' => $this->getPatchPath('0000.legacy.sql'), @@ -903,6 +907,10 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList { 'type' => 'sql', 'name' => $this->getPatchPath('harbormasterobject.sql'), ), + 'markupcache.sql' => array( + 'type' => 'sql', + 'name' => $this->getPatchPath('markupcache.sql'), + ), ); }