From 8ae718c2aab57dcc7f4087036a5dbb0b3e0ef1d3 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 4 Mar 2013 12:33:05 -0800 Subject: [PATCH] Require a viewer for Remarkup rendering Summary: Provide a viewer to all remarkup engines. This fixes commit summaries in Diffusion, which were failing to link because they didn't have a user and thus couldn't see/load `D123`, e.g. Test Plan: Grepped for engine creation. Reviewers: vrana Reviewed By: vrana CC: aran, edward Maniphest Tasks: T603 Differential Revision: https://secure.phabricator.com/D5152 --- .../controller/PhabricatorConfigEditController.php | 1 + .../__tests__/DifferentialParseRenderTestCase.php | 1 + .../DifferentialBlameRevisionFieldSpecification.php | 1 + .../specification/DifferentialUnitFieldSpecification.php | 1 + .../differential/remarkup/DifferentialRemarkupRule.php | 5 ----- .../diffusion/controller/DiffusionBrowseController.php | 1 + .../diffusion/controller/DiffusionCommitController.php | 1 + .../diffusion/query/browse/DiffusionBrowseQuery.php | 1 + .../diffusion/remarkup/DiffusionRemarkupRule.php | 4 ---- .../diviner/renderer/DivinerDefaultRenderer.php | 1 + .../maniphest/remarkup/ManiphestRemarkupRule.php | 4 ---- .../paste/remarkup/PhabricatorPasteRemarkupRule.php | 4 ---- .../controller/PhabricatorPeopleProfileController.php | 7 ++++--- src/applications/pholio/remarkup/PholioRemarkupRule.php | 5 ----- src/applications/ponder/remarkup/PonderRemarkupRule.php | 5 ----- .../controller/PhabricatorSlowvotePollController.php | 1 + ...abricatorApplicationTransactionTextDiffDetailView.php | 5 ++++- src/infrastructure/markup/PhabricatorMarkupEngine.php | 2 ++ .../markup/rule/PhabricatorRemarkupRuleObject.php | 9 +-------- 19 files changed, 20 insertions(+), 39 deletions(-) diff --git a/src/applications/config/controller/PhabricatorConfigEditController.php b/src/applications/config/controller/PhabricatorConfigEditController.php index 2ac257efec..b5ff45195d 100644 --- a/src/applications/config/controller/PhabricatorConfigEditController.php +++ b/src/applications/config/controller/PhabricatorConfigEditController.php @@ -130,6 +130,7 @@ final class PhabricatorConfigEditController } $engine = new PhabricatorMarkupEngine(); + $engine->setViewer($user); $engine->addObject($option, 'description'); $engine->process(); $description = phutil_tag( diff --git a/src/applications/differential/__tests__/DifferentialParseRenderTestCase.php b/src/applications/differential/__tests__/DifferentialParseRenderTestCase.php index c6e1e06d40..41165bf23e 100644 --- a/src/applications/differential/__tests__/DifferentialParseRenderTestCase.php +++ b/src/applications/differential/__tests__/DifferentialParseRenderTestCase.php @@ -43,6 +43,7 @@ final class DifferentialParseRenderTestCase extends PhabricatorTestCase { $changeset = head($diff->getChangesets()); $engine = new PhabricatorMarkupEngine(); + $engine->setViewer(new PhabricatorUser()); $cparser = new DifferentialChangesetParser(); $cparser->setDisableCache(true); diff --git a/src/applications/differential/field/specification/DifferentialBlameRevisionFieldSpecification.php b/src/applications/differential/field/specification/DifferentialBlameRevisionFieldSpecification.php index a246aa1a31..d03ffa0cb6 100644 --- a/src/applications/differential/field/specification/DifferentialBlameRevisionFieldSpecification.php +++ b/src/applications/differential/field/specification/DifferentialBlameRevisionFieldSpecification.php @@ -49,6 +49,7 @@ final class DifferentialBlameRevisionFieldSpecification return null; } $engine = PhabricatorMarkupEngine::newDifferentialMarkupEngine(); + $engine->setConfig('viewer', $this->getUser()); return $engine->markupText($this->value); } diff --git a/src/applications/differential/field/specification/DifferentialUnitFieldSpecification.php b/src/applications/differential/field/specification/DifferentialUnitFieldSpecification.php index 9cfd7b69ee..546dcd2177 100644 --- a/src/applications/differential/field/specification/DifferentialUnitFieldSpecification.php +++ b/src/applications/differential/field/specification/DifferentialUnitFieldSpecification.php @@ -116,6 +116,7 @@ final class DifferentialUnitFieldSpecification $userdata = str_replace("\000", '', $userdata); } $engine = PhabricatorMarkupEngine::newDifferentialMarkupEngine(); + $engine->setConfig('viewer', $this->getUser()); $userdata = $engine->markupText($userdata); $rows[] = array( 'style' => 'details', diff --git a/src/applications/differential/remarkup/DifferentialRemarkupRule.php b/src/applications/differential/remarkup/DifferentialRemarkupRule.php index 4ef3d7b7d0..1535463a29 100644 --- a/src/applications/differential/remarkup/DifferentialRemarkupRule.php +++ b/src/applications/differential/remarkup/DifferentialRemarkupRule.php @@ -12,11 +12,6 @@ final class DifferentialRemarkupRule protected function loadObjects(array $ids) { $viewer = $this->getEngine()->getConfig('viewer'); - - if (!$viewer) { - return array(); - } - return id(new DifferentialRevisionQuery()) ->setViewer($viewer) ->withIDs($ids) diff --git a/src/applications/diffusion/controller/DiffusionBrowseController.php b/src/applications/diffusion/controller/DiffusionBrowseController.php index fc57792dde..d720ad8872 100644 --- a/src/applications/diffusion/controller/DiffusionBrowseController.php +++ b/src/applications/diffusion/controller/DiffusionBrowseController.php @@ -107,6 +107,7 @@ final class DiffusionBrowseController extends DiffusionController { private function markupText($text) { $engine = PhabricatorMarkupEngine::newDiffusionMarkupEngine(); + $engine->setConfig('viewer', $this->getRequest()->getUser()); $text = $engine->markupText($text); $text = phutil_tag( diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php index 2b3328686d..ef1ada2f3e 100644 --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -67,6 +67,7 @@ final class DiffusionCommitController extends DiffusionController { $content[] = $top_anchor; } else { $engine = PhabricatorMarkupEngine::newDifferentialMarkupEngine(); + $engine->setConfig('viewer', $user); require_celerity_resource('diffusion-commit-view-css'); require_celerity_resource('phabricator-remarkup-css'); diff --git a/src/applications/diffusion/query/browse/DiffusionBrowseQuery.php b/src/applications/diffusion/query/browse/DiffusionBrowseQuery.php index 1f97a339df..d2151dd183 100644 --- a/src/applications/diffusion/query/browse/DiffusionBrowseQuery.php +++ b/src/applications/diffusion/query/browse/DiffusionBrowseQuery.php @@ -137,6 +137,7 @@ abstract class DiffusionBrowseQuery { } else { // Markup extensionless files as remarkup so we get links and such. $engine = PhabricatorMarkupEngine::newDiffusionMarkupEngine(); + $engine->setConfig('viewer', $this->getViewer()); $readme_content = $engine->markupText($readme_content); $class = 'phabricator-remarkup'; diff --git a/src/applications/diffusion/remarkup/DiffusionRemarkupRule.php b/src/applications/diffusion/remarkup/DiffusionRemarkupRule.php index 65880bc193..fc5be4b9f7 100644 --- a/src/applications/diffusion/remarkup/DiffusionRemarkupRule.php +++ b/src/applications/diffusion/remarkup/DiffusionRemarkupRule.php @@ -23,10 +23,6 @@ final class DiffusionRemarkupRule $viewer = $this->getEngine()->getConfig('viewer'); $min_qualified = PhabricatorRepository::MINIMUM_QUALIFIED_HASH; - if (!$viewer) { - return array(); - } - $commits = id(new DiffusionCommitQuery()) ->setViewer($viewer) ->withIdentifiers($ids) diff --git a/src/applications/diviner/renderer/DivinerDefaultRenderer.php b/src/applications/diviner/renderer/DivinerDefaultRenderer.php index f8380156c6..f573cdbd97 100644 --- a/src/applications/diviner/renderer/DivinerDefaultRenderer.php +++ b/src/applications/diviner/renderer/DivinerDefaultRenderer.php @@ -186,6 +186,7 @@ final class DivinerDefaultRenderer extends DivinerRenderer { array( 'preserve-linebreaks' => false, )); + $engine->setConfig('viewer', new PhabricatorUser()); $engine->setConfig('diviner.renderer', $this); return $engine; } diff --git a/src/applications/maniphest/remarkup/ManiphestRemarkupRule.php b/src/applications/maniphest/remarkup/ManiphestRemarkupRule.php index 0f09eab057..6508252f19 100644 --- a/src/applications/maniphest/remarkup/ManiphestRemarkupRule.php +++ b/src/applications/maniphest/remarkup/ManiphestRemarkupRule.php @@ -13,10 +13,6 @@ final class ManiphestRemarkupRule protected function loadObjects(array $ids) { $viewer = $this->getEngine()->getConfig('viewer'); - if (!$viewer) { - return array(); - } - return id(new ManiphestTaskQuery()) ->setViewer($viewer) ->withTaskIDs($ids) diff --git a/src/applications/paste/remarkup/PhabricatorPasteRemarkupRule.php b/src/applications/paste/remarkup/PhabricatorPasteRemarkupRule.php index ef6bfb96cd..47f37ddcb5 100644 --- a/src/applications/paste/remarkup/PhabricatorPasteRemarkupRule.php +++ b/src/applications/paste/remarkup/PhabricatorPasteRemarkupRule.php @@ -13,10 +13,6 @@ final class PhabricatorPasteRemarkupRule protected function loadObjects(array $ids) { $viewer = $this->getEngine()->getConfig('viewer'); - if (!$viewer) { - return array(); - } - return id(new PhabricatorPasteQuery()) ->setViewer($viewer) ->withIDs($ids) diff --git a/src/applications/people/controller/PhabricatorPeopleProfileController.php b/src/applications/people/controller/PhabricatorPeopleProfileController.php index ab81cd1061..5fe8ceb07b 100644 --- a/src/applications/people/controller/PhabricatorPeopleProfileController.php +++ b/src/applications/people/controller/PhabricatorPeopleProfileController.php @@ -171,11 +171,12 @@ final class PhabricatorPeopleProfileController $profile->getBlurb(), '//'.pht('Nothing is known about this rare specimen.').'//'); - $engine = PhabricatorMarkupEngine::newProfileMarkupEngine(); - $blurb = $engine->markupText($blurb); - $viewer = $this->getRequest()->getUser(); + $engine = PhabricatorMarkupEngine::newProfileMarkupEngine(); + $engine->setConfig('viewer', $viewer); + $blurb = $engine->markupText($blurb); + $content = hsprintf( '

Basic Information

diff --git a/src/applications/pholio/remarkup/PholioRemarkupRule.php b/src/applications/pholio/remarkup/PholioRemarkupRule.php index 7506be3298..85204eff16 100644 --- a/src/applications/pholio/remarkup/PholioRemarkupRule.php +++ b/src/applications/pholio/remarkup/PholioRemarkupRule.php @@ -9,11 +9,6 @@ final class PholioRemarkupRule protected function loadObjects(array $ids) { $viewer = $this->getEngine()->getConfig('viewer'); - - if (!$viewer) { - return array(); - } - return id(new PholioMockQuery()) ->setViewer($viewer) ->withIDs($ids) diff --git a/src/applications/ponder/remarkup/PonderRemarkupRule.php b/src/applications/ponder/remarkup/PonderRemarkupRule.php index 83944e5c80..9102600cfc 100644 --- a/src/applications/ponder/remarkup/PonderRemarkupRule.php +++ b/src/applications/ponder/remarkup/PonderRemarkupRule.php @@ -9,11 +9,6 @@ final class PonderRemarkupRule protected function loadObjects(array $ids) { $viewer = $this->getEngine()->getConfig('viewer'); - - if (!$viewer) { - return array(); - } - return id(new PonderQuestionQuery()) ->setViewer($viewer) ->withIDs($ids) diff --git a/src/applications/slowvote/controller/PhabricatorSlowvotePollController.php b/src/applications/slowvote/controller/PhabricatorSlowvotePollController.php index 5d9aa40bcc..6db949faa6 100644 --- a/src/applications/slowvote/controller/PhabricatorSlowvotePollController.php +++ b/src/applications/slowvote/controller/PhabricatorSlowvotePollController.php @@ -200,6 +200,7 @@ final class PhabricatorSlowvotePollController $viewer = $this->getRequest()->getUser(); $engine = PhabricatorMarkupEngine::newSlowvoteMarkupEngine(); + $engine->setConfig('viewer', $viewer); $comment_markup = array(); foreach ($comments as $comment) { diff --git a/src/applications/transactions/view/PhabricatorApplicationTransactionTextDiffDetailView.php b/src/applications/transactions/view/PhabricatorApplicationTransactionTextDiffDetailView.php index aaa8d98228..ac062125df 100644 --- a/src/applications/transactions/view/PhabricatorApplicationTransactionTextDiffDetailView.php +++ b/src/applications/transactions/view/PhabricatorApplicationTransactionTextDiffDetailView.php @@ -33,9 +33,12 @@ final class PhabricatorApplicationTransactionTextDiffDetailView $whitespace_mode = DifferentialChangesetParser::WHITESPACE_SHOW_ALL; + $markup_engine = new PhabricatorMarkupEngine(); + $markup_engine->setViewer($this->getUser()); + $parser = new DifferentialChangesetParser(); $parser->setChangeset($changeset); - $parser->setMarkupEngine(new PhabricatorMarkupEngine()); + $parser->setMarkupEngine($markup_engine); $parser->setWhitespaceMode($whitespace_mode); return $parser->render(0, PHP_INT_MAX, array()); diff --git a/src/infrastructure/markup/PhabricatorMarkupEngine.php b/src/infrastructure/markup/PhabricatorMarkupEngine.php index 31007e4359..dc9fd2f417 100644 --- a/src/infrastructure/markup/PhabricatorMarkupEngine.php +++ b/src/infrastructure/markup/PhabricatorMarkupEngine.php @@ -461,6 +461,7 @@ final class PhabricatorMarkupEngine { $mentions = array(); $engine = self::newDifferentialMarkupEngine(); + $engine->setConfig('viewer', PhabricatorUser::getOmnipotentUser()); foreach ($content_blocks as $content_block) { $engine->markupText($content_block); @@ -478,6 +479,7 @@ final class PhabricatorMarkupEngine { $files = array(); $engine = self::newDifferentialMarkupEngine(); + $engine->setConfig('viewer', PhabricatorUser::getOmnipotentUser()); foreach ($content_blocks as $content_block) { $engine->markupText($content_block); diff --git a/src/infrastructure/markup/rule/PhabricatorRemarkupRuleObject.php b/src/infrastructure/markup/rule/PhabricatorRemarkupRuleObject.php index 779b81dd66..f8559235f5 100644 --- a/src/infrastructure/markup/rule/PhabricatorRemarkupRuleObject.php +++ b/src/infrastructure/markup/rule/PhabricatorRemarkupRuleObject.php @@ -24,14 +24,7 @@ abstract class PhabricatorRemarkupRuleObject $query = new PhabricatorObjectHandleData($phids); $viewer = $this->getEngine()->getConfig('viewer'); - if ($viewer) { - $query->setViewer($viewer); - } else { - // TODO: This needs to be fixed; all markup engines need to set viewers -- - // but there are a lot of them (T603). - $query->setViewer(PhabricatorUser::getOmnipotentUser()); - phlog("Warning: Loading handles without a viewing user."); - } + $query->setViewer($viewer); $handles = $query->loadHandles(); $result = array();