From 245132a0b22e9f572acda9b7b90b96475845d4b9 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 5 Apr 2018 08:21:12 -0700 Subject: [PATCH] Pull file Document Engine rendering out of "Files" application controllers Summary: Ref T13105. This separates document rendering from the Controllers which trigger it so it can be reused elsewhere (notably, in Diffusion). This shouldn't cause any application behavior to change, it just pulls the rendering logic out so it can be reused elsewhere. Test Plan: Viewed various types of files in Files; toggled rendering, highlighting, and encoding. Reviewers: mydeveloperday Reviewed By: mydeveloperday Maniphest Tasks: T13105 Differential Revision: https://secure.phabricator.com/D19301 --- src/__phutil_library_map__.php | 4 + .../PhabricatorFilesApplication.php | 2 +- .../PhabricatorFileDocumentController.php | 100 +----- .../PhabricatorFileViewController.php | 113 +------ .../document/PhabricatorDocumentEngine.php | 12 - .../PhabricatorDocumentRenderingEngine.php | 285 ++++++++++++++++++ ...PhabricatorFileDocumentRenderingEngine.php | 47 +++ 7 files changed, 344 insertions(+), 219 deletions(-) create mode 100644 src/applications/files/document/render/PhabricatorDocumentRenderingEngine.php create mode 100644 src/applications/files/document/render/PhabricatorFileDocumentRenderingEngine.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 391b8fb1f2..95f8ed45b2 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2819,6 +2819,7 @@ phutil_register_library_map(array( 'PhabricatorDivinerApplication' => 'applications/diviner/application/PhabricatorDivinerApplication.php', 'PhabricatorDocumentEngine' => 'applications/files/document/PhabricatorDocumentEngine.php', 'PhabricatorDocumentRef' => 'applications/files/document/PhabricatorDocumentRef.php', + 'PhabricatorDocumentRenderingEngine' => 'applications/files/document/render/PhabricatorDocumentRenderingEngine.php', 'PhabricatorDoorkeeperApplication' => 'applications/doorkeeper/application/PhabricatorDoorkeeperApplication.php', 'PhabricatorDraft' => 'applications/draft/storage/PhabricatorDraft.php', 'PhabricatorDraftDAO' => 'applications/draft/storage/PhabricatorDraftDAO.php', @@ -3010,6 +3011,7 @@ phutil_register_library_map(array( 'PhabricatorFileDeleteController' => 'applications/files/controller/PhabricatorFileDeleteController.php', 'PhabricatorFileDeleteTransaction' => 'applications/files/xaction/PhabricatorFileDeleteTransaction.php', 'PhabricatorFileDocumentController' => 'applications/files/controller/PhabricatorFileDocumentController.php', + 'PhabricatorFileDocumentRenderingEngine' => 'applications/files/document/render/PhabricatorFileDocumentRenderingEngine.php', 'PhabricatorFileDropUploadController' => 'applications/files/controller/PhabricatorFileDropUploadController.php', 'PhabricatorFileEditController' => 'applications/files/controller/PhabricatorFileEditController.php', 'PhabricatorFileEditEngine' => 'applications/files/editor/PhabricatorFileEditEngine.php', @@ -8401,6 +8403,7 @@ phutil_register_library_map(array( 'PhabricatorDivinerApplication' => 'PhabricatorApplication', 'PhabricatorDocumentEngine' => 'Phobject', 'PhabricatorDocumentRef' => 'Phobject', + 'PhabricatorDocumentRenderingEngine' => 'Phobject', 'PhabricatorDoorkeeperApplication' => 'PhabricatorApplication', 'PhabricatorDraft' => 'PhabricatorDraftDAO', 'PhabricatorDraftDAO' => 'PhabricatorLiskDAO', @@ -8624,6 +8627,7 @@ phutil_register_library_map(array( 'PhabricatorFileDeleteController' => 'PhabricatorFileController', 'PhabricatorFileDeleteTransaction' => 'PhabricatorFileTransactionType', 'PhabricatorFileDocumentController' => 'PhabricatorFileController', + 'PhabricatorFileDocumentRenderingEngine' => 'PhabricatorDocumentRenderingEngine', 'PhabricatorFileDropUploadController' => 'PhabricatorFileController', 'PhabricatorFileEditController' => 'PhabricatorFileController', 'PhabricatorFileEditEngine' => 'PhabricatorEditEngine', diff --git a/src/applications/files/application/PhabricatorFilesApplication.php b/src/applications/files/application/PhabricatorFilesApplication.php index 5bf1ebccc4..14bbd83e78 100644 --- a/src/applications/files/application/PhabricatorFilesApplication.php +++ b/src/applications/files/application/PhabricatorFilesApplication.php @@ -73,7 +73,7 @@ final class PhabricatorFilesApplication extends PhabricatorApplication { => 'PhabricatorFileViewController', '/file/' => array( '(query/(?P[^/]+)/)?' => 'PhabricatorFileListController', - 'view/(?P[^/]+)/'. + 'view/(?P[1-9]\d*)/'. '(?:(?P[^/]+)/)?'. '(?:\$(?P\d+(?:-\d+)?))?' => 'PhabricatorFileViewController', diff --git a/src/applications/files/controller/PhabricatorFileDocumentController.php b/src/applications/files/controller/PhabricatorFileDocumentController.php index 15fadaa289..e74b0bc6eb 100644 --- a/src/applications/files/controller/PhabricatorFileDocumentController.php +++ b/src/applications/files/controller/PhabricatorFileDocumentController.php @@ -3,10 +3,6 @@ final class PhabricatorFileDocumentController extends PhabricatorFileController { - private $file; - private $engine; - private $ref; - public function shouldAllowPublic() { return true; } @@ -26,102 +22,14 @@ final class PhabricatorFileDocumentController 'This file ("%s") does not exist or could not be loaded.', $file_phid)); } - $this->file = $file; $ref = id(new PhabricatorDocumentRef()) ->setFile($file); - $this->ref = $ref; - $engines = PhabricatorDocumentEngine::getEnginesForRef($viewer, $ref); - $engine_key = $request->getURIData('engineKey'); - if (!isset($engines[$engine_key])) { - return $this->newErrorResponse( - pht( - 'The engine ("%s") is unknown, or unable to render this document.', - $engine_key)); - } - $engine = $engines[$engine_key]; - $this->engine = $engine; - - $encode_setting = $request->getStr('encode'); - if (strlen($encode_setting)) { - $engine->setEncodingConfiguration($encode_setting); - } - - $highlight_setting = $request->getStr('highlight'); - if (strlen($highlight_setting)) { - $engine->setHighlightingConfiguration($highlight_setting); - } - - try { - $content = $engine->newDocument($ref); - } catch (Exception $ex) { - return $this->newErrorResponse($ex->getMessage()); - } - - return $this->newContentResponse($content); - } - - private function newErrorResponse($message) { - $container = phutil_tag( - 'div', - array( - 'class' => 'document-engine-error', - ), - array( - id(new PHUIIconView()) - ->setIcon('fa-exclamation-triangle red'), - ' ', - $message, - )); - - return $this->newContentResponse($container); - } - - - private function newContentResponse($content) { - $viewer = $this->getViewer(); - $request = $this->getRequest(); - - $file = $this->file; - $engine = $this->engine; - $ref = $this->ref; - - if ($request->isAjax()) { - return id(new AphrontAjaxResponse()) - ->setContent( - array( - 'markup' => hsprintf('%s', $content), - )); - } - - $crumbs = $this->buildApplicationCrumbs(); - if ($file) { - $crumbs->addTextCrumb($file->getMonogram(), $file->getInfoURI()); - } - - $label = $engine->getViewAsLabel($ref); - if ($label) { - $crumbs->addTextCrumb($label); - } - - $crumbs->setBorder(true); - - $content_frame = id(new PHUIObjectBoxView()) - ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) - ->appendChild($content); - - $page_frame = id(new PHUITwoColumnView()) - ->setFooter($content_frame); - - return $this->newPage() - ->setCrumbs($crumbs) - ->setTitle( - array( - $ref->getName(), - pht('Standalone'), - )) - ->appendChild($page_frame); + return id(new PhabricatorFileDocumentRenderingEngine()) + ->setRequest($request) + ->setController($this) + ->newRenderResponse($ref); } } diff --git a/src/applications/files/controller/PhabricatorFileViewController.php b/src/applications/files/controller/PhabricatorFileViewController.php index 9d32ff6fb1..42a632424d 100644 --- a/src/applications/files/controller/PhabricatorFileViewController.php +++ b/src/applications/files/controller/PhabricatorFileViewController.php @@ -403,122 +403,15 @@ final class PhabricatorFileViewController extends PhabricatorFileController { } private function newFileContent(PhabricatorFile $file) { - $viewer = $this->getViewer(); $request = $this->getRequest(); $ref = id(new PhabricatorDocumentRef()) ->setFile($file); - $engines = PhabricatorDocumentEngine::getEnginesForRef($viewer, $ref); + $engine = id(new PhabricatorFileDocumentRenderingEngine()) + ->setRequest($request); - $engine_key = $request->getURIData('engineKey'); - if (!isset($engines[$engine_key])) { - $engine_key = head_key($engines); - } - $engine = $engines[$engine_key]; - - $lines = $request->getURILineRange('lines', 1000); - if ($lines) { - $engine->setHighlightedLines(range($lines[0], $lines[1])); - } - - $encode_setting = $request->getStr('encode'); - if (strlen($encode_setting)) { - $engine->setEncodingConfiguration($encode_setting); - } - - $highlight_setting = $request->getStr('highlight'); - if (strlen($highlight_setting)) { - $engine->setHighlightingConfiguration($highlight_setting); - } - - $views = array(); - foreach ($engines as $candidate_key => $candidate_engine) { - $label = $candidate_engine->getViewAsLabel($ref); - if ($label === null) { - continue; - } - - $view_uri = '/file/view/'.$file->getID().'/'.$candidate_key.'/'; - - $view_icon = $candidate_engine->getViewAsIconIcon($ref); - $view_color = $candidate_engine->getViewAsIconColor($ref); - $loading = $candidate_engine->newLoadingContent($ref); - - $views[] = array( - 'viewKey' => $candidate_engine->getDocumentEngineKey(), - 'icon' => $view_icon, - 'color' => $view_color, - 'name' => $label, - 'engineURI' => $candidate_engine->getRenderURI($ref), - 'viewURI' => $view_uri, - 'loadingMarkup' => hsprintf('%s', $loading), - 'canEncode' => $candidate_engine->canConfigureEncoding($ref), - 'canHighlight' => $candidate_engine->CanConfigureHighlighting($ref), - ); - } - - $viewport_id = celerity_generate_unique_node_id(); - $control_id = celerity_generate_unique_node_id(); - $icon = $engine->newDocumentIcon($ref); - - if ($engine->shouldRenderAsync($ref)) { - $content = $engine->newLoadingContent($ref); - $config = array( - 'renderControlID' => $control_id, - ); - } else { - $content = $engine->newDocument($ref); - $config = array(); - } - - Javelin::initBehavior('document-engine', $config); - - $viewport = phutil_tag( - 'div', - array( - 'id' => $viewport_id, - ), - $content); - - $meta = array( - 'viewportID' => $viewport_id, - 'viewKey' => $engine->getDocumentEngineKey(), - 'views' => $views, - 'standaloneURI' => $engine->getRenderURI($ref), - 'encode' => array( - 'icon' => 'fa-font', - 'name' => pht('Change Text Encoding...'), - 'uri' => '/services/encoding/', - 'value' => $encode_setting, - ), - 'highlight' => array( - 'icon' => 'fa-lightbulb-o', - 'name' => pht('Highlight As...'), - 'uri' => '/services/highlight/', - 'value' => $highlight_setting, - ), - ); - - $view_button = id(new PHUIButtonView()) - ->setTag('a') - ->setText(pht('View Options')) - ->setIcon('fa-file-image-o') - ->setColor(PHUIButtonView::GREY) - ->setID($control_id) - ->setMetadata($meta) - ->setDropdown(true) - ->addSigil('document-engine-view-dropdown'); - - $header = id(new PHUIHeaderView()) - ->setHeaderIcon($icon) - ->setHeader($ref->getName()) - ->addActionLink($view_button); - - return id(new PHUIObjectBoxView()) - ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) - ->setHeader($header) - ->appendChild($viewport); + return $engine->newDocumentView($ref); } } diff --git a/src/applications/files/document/PhabricatorDocumentEngine.php b/src/applications/files/document/PhabricatorDocumentEngine.php index c3a1a7317a..17feed1c3e 100644 --- a/src/applications/files/document/PhabricatorDocumentEngine.php +++ b/src/applications/files/document/PhabricatorDocumentEngine.php @@ -152,18 +152,6 @@ abstract class PhabricatorDocumentEngine return null; } - public function getRenderURI(PhabricatorDocumentRef $ref) { - $file = $ref->getFile(); - if (!$file) { - throw new PhutilMethodNotImplementedException(); - } - - $engine_key = $this->getDocumentEngineKey(); - $file_phid = $file->getPHID(); - - return "/file/document/{$engine_key}/{$file_phid}/"; - } - final public static function getEnginesForRef( PhabricatorUser $viewer, PhabricatorDocumentRef $ref) { diff --git a/src/applications/files/document/render/PhabricatorDocumentRenderingEngine.php b/src/applications/files/document/render/PhabricatorDocumentRenderingEngine.php new file mode 100644 index 0000000000..37b0917a65 --- /dev/null +++ b/src/applications/files/document/render/PhabricatorDocumentRenderingEngine.php @@ -0,0 +1,285 @@ +request = $request; + return $this; + } + + final public function getRequest() { + if (!$this->request) { + throw new PhutilInvalidStateException('setRequest'); + } + + return $this->request; + } + + final public function setController(PhabricatorController $controller) { + $this->controller = $controller; + return $this; + } + + final public function getController() { + if (!$this->controller) { + throw new PhutilInvalidStateException('setController'); + } + + return $this->controller; + } + + final protected function getActiveEngine() { + if (!$this->activeEngine) { + throw new PhutilInvalidStateException('setActiveEngine'); + } + + return $this->activeEngine; + } + + final public function newDocumentView(PhabricatorDocumentRef $ref) { + $request = $this->getRequest(); + $viewer = $request->getViewer(); + + $engines = PhabricatorDocumentEngine::getEnginesForRef($viewer, $ref); + + $engine_key = $this->getSelectedDocumentEngineKey(); + if (!isset($engines[$engine_key])) { + $engine_key = head_key($engines); + } + $engine = $engines[$engine_key]; + + $lines = $request->getURILineRange('lines', 1000); + if ($lines) { + $engine->setHighlightedLines(range($lines[0], $lines[1])); + } + + $encode_setting = $request->getStr('encode'); + if (strlen($encode_setting)) { + $engine->setEncodingConfiguration($encode_setting); + } + + $highlight_setting = $request->getStr('highlight'); + if (strlen($highlight_setting)) { + $engine->setHighlightingConfiguration($highlight_setting); + } + + $views = array(); + foreach ($engines as $candidate_key => $candidate_engine) { + $label = $candidate_engine->getViewAsLabel($ref); + if ($label === null) { + continue; + } + + $view_uri = $this->newRefViewURI($ref, $candidate_engine); + + $view_icon = $candidate_engine->getViewAsIconIcon($ref); + $view_color = $candidate_engine->getViewAsIconColor($ref); + $loading = $candidate_engine->newLoadingContent($ref); + + $views[] = array( + 'viewKey' => $candidate_engine->getDocumentEngineKey(), + 'icon' => $view_icon, + 'color' => $view_color, + 'name' => $label, + 'engineURI' => $this->newRefRenderURI($ref, $candidate_engine), + 'viewURI' => $view_uri, + 'loadingMarkup' => hsprintf('%s', $loading), + 'canEncode' => $candidate_engine->canConfigureEncoding($ref), + 'canHighlight' => $candidate_engine->CanConfigureHighlighting($ref), + ); + } + + $viewport_id = celerity_generate_unique_node_id(); + $control_id = celerity_generate_unique_node_id(); + $icon = $engine->newDocumentIcon($ref); + + if ($engine->shouldRenderAsync($ref)) { + $content = $engine->newLoadingContent($ref); + $config = array( + 'renderControlID' => $control_id, + ); + } else { + $content = $engine->newDocument($ref); + $config = array(); + } + + Javelin::initBehavior('document-engine', $config); + + $viewport = phutil_tag( + 'div', + array( + 'id' => $viewport_id, + ), + $content); + + $meta = array( + 'viewportID' => $viewport_id, + 'viewKey' => $engine->getDocumentEngineKey(), + 'views' => $views, + 'encode' => array( + 'icon' => 'fa-font', + 'name' => pht('Change Text Encoding...'), + 'uri' => '/services/encoding/', + 'value' => $encode_setting, + ), + 'highlight' => array( + 'icon' => 'fa-lightbulb-o', + 'name' => pht('Highlight As...'), + 'uri' => '/services/highlight/', + 'value' => $highlight_setting, + ), + ); + + $view_button = id(new PHUIButtonView()) + ->setTag('a') + ->setText(pht('View Options')) + ->setIcon('fa-file-image-o') + ->setColor(PHUIButtonView::GREY) + ->setID($control_id) + ->setMetadata($meta) + ->setDropdown(true) + ->addSigil('document-engine-view-dropdown'); + + $header = id(new PHUIHeaderView()) + ->setHeaderIcon($icon) + ->setHeader($ref->getName()) + ->addActionLink($view_button); + + return id(new PHUIObjectBoxView()) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) + ->setHeader($header) + ->appendChild($viewport); + } + + abstract protected function newRefViewURI( + PhabricatorDocumentRef $ref, + PhabricatorDocumentEngine $engine); + + abstract protected function newRefRenderURI( + PhabricatorDocumentRef $ref, + PhabricatorDocumentEngine $engine); + + protected function getSelectedDocumentEngineKey() { + return $this->getRequest()->getURIData('engineKey'); + } + + final public function newRenderResponse(PhabricatorDocumentRef $ref) { + $request = $this->getRequest(); + $viewer = $request->getViewer(); + + $engines = PhabricatorDocumentEngine::getEnginesForRef($viewer, $ref); + $engine_key = $request->getURIData('engineKey'); + if (!isset($engines[$engine_key])) { + return $this->newErrorResponse( + $ref, + pht( + 'The engine ("%s") is unknown, or unable to render this document.', + $engine_key)); + } + $engine = $engines[$engine_key]; + + $this->activeEngine = $engine; + + $encode_setting = $request->getStr('encode'); + if (strlen($encode_setting)) { + $engine->setEncodingConfiguration($encode_setting); + } + + $highlight_setting = $request->getStr('highlight'); + if (strlen($highlight_setting)) { + $engine->setHighlightingConfiguration($highlight_setting); + } + + try { + $content = $engine->newDocument($ref); + } catch (Exception $ex) { + return $this->newErrorResponse($ref, $ex->getMessage()); + } + + return $this->newContentResponse($ref, $content); + } + + private function newErrorResponse( + PhabricatorDocumentRef $ref, + $message) { + + $container = phutil_tag( + 'div', + array( + 'class' => 'document-engine-error', + ), + array( + id(new PHUIIconView()) + ->setIcon('fa-exclamation-triangle red'), + ' ', + $message, + )); + + return $this->newContentResponse($ref, $container); + } + + private function newContentResponse( + PhabricatorDocumentRef $ref, + $content) { + + $request = $this->getRequest(); + $viewer = $request->getViewer(); + $controller = $this->getController(); + + if ($request->isAjax()) { + return id(new AphrontAjaxResponse()) + ->setContent( + array( + 'markup' => hsprintf('%s', $content), + )); + } + + $crumbs = $this->newCrumbs($ref); + if ($crumbs) { + $crumbs->setBorder(true); + } + + $content_frame = id(new PHUIObjectBoxView()) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) + ->appendChild($content); + + $page_frame = id(new PHUITwoColumnView()) + ->setFooter($content_frame); + + return $controller->newPage() + ->setCrumbs($crumbs) + ->setTitle( + array( + $ref->getName(), + pht('Standalone'), + )) + ->appendChild($page_frame); + } + + protected function newCrumbs(PhabricatorDocumentRef $ref) { + $controller = $this->getController(); + $crumbs = $controller->buildApplicationCrumbsForEditEngine(); + + $this->addApplicationCrumbs($ref, $crumbs); + + $engine = $this->getActiveEngine(); + $label = $engine->getViewAsLabel($ref); + if ($label) { + $crumbs->addTextCrumb($label); + } + + return $crumbs; + } + + protected function addApplicationCrumbs( + PhabricatorDocumentRef $ref, + PHUICrumbsView $crumbs) { + return; + } + +} diff --git a/src/applications/files/document/render/PhabricatorFileDocumentRenderingEngine.php b/src/applications/files/document/render/PhabricatorFileDocumentRenderingEngine.php new file mode 100644 index 0000000000..f48336bea3 --- /dev/null +++ b/src/applications/files/document/render/PhabricatorFileDocumentRenderingEngine.php @@ -0,0 +1,47 @@ +getFile(); + $engine_key = $engine->getDocumentEngineKey(); + + return urisprintf( + '/file/view/%d/%s/', + $file->getID(), + $engine_key); + } + + protected function newRefRenderURI( + PhabricatorDocumentRef $ref, + PhabricatorDocumentEngine $engine) { + $file = $ref->getFile(); + if (!$file) { + throw new PhutilMethodNotImplementedException(); + } + + $engine_key = $engine->getDocumentEngineKey(); + $file_phid = $file->getPHID(); + + return urisprintf( + '/file/document/%s/%s/', + $engine_key, + $file_phid); + } + + protected function addApplicationCrumbs( + PhabricatorDocumentRef $ref, + PHUICrumbsView $crumbs) { + + $file = $ref->getFile(); + if ($file) { + $crumbs->addTextCrumb($file->getMonogram(), $file->getInfoURI()); + } + + } + +}