From d8c11a2106058401e96589b160b0392e2b457a45 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 31 Dec 2013 19:21:56 -0800 Subject: [PATCH] Move build-time resources to "CelerityPhysicalResources" to fix Phame Summary: Ref T4222. This fixes some issues with Phame's resource construction. Phame requires a fully virtual resource source, and since I want to run wordpress templates unmodified some day I don't want to build resource maps for skins. Move all the stuff that depends on resource lists being discoverable at build time to `CelerityPhysicalResources`, and only generate maps for subclasses. The root `CelerityResources` can now construct virtual resources; construct a virtual resource for Phame and use it. Test Plan: Off-domain blogs work correctly now. On-domain blogs with custom skins work correctly now. Reviewers: btrahan, hach-que Reviewed By: hach-que CC: aran Maniphest Tasks: T4222 Differential Revision: https://secure.phabricator.com/D7873 --- resources/celerity/map.php | 10 ++-- src/__phutil_library_map__.php | 6 ++- .../phame/celerity/PhameCelerityResources.php | 28 ++++++++++ .../controller/PhameResourceController.php | 18 +++++-- .../celerity/CelerityResourceController.php | 8 +-- .../CelerityManagementMapWorkflow.php | 20 +++---- .../resources/CelerityPhysicalResources.php | 52 +++++++++++++++++++ .../celerity/resources/CelerityResources.php | 43 ++------------- .../resources/CelerityResourcesOnDisk.php | 2 +- 9 files changed, 126 insertions(+), 61 deletions(-) create mode 100644 src/applications/phame/celerity/PhameCelerityResources.php create mode 100644 src/infrastructure/celerity/resources/CelerityPhysicalResources.php diff --git a/resources/celerity/map.php b/resources/celerity/map.php index b8c4f9482f..3ac2b20197 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -7,7 +7,7 @@ return array( 'names' => array( - 'core.pkg.css' => '6237350d', + 'core.pkg.css' => 'c70f102f', 'core.pkg.js' => 'e5bed99d', 'css/aphront/aphront-bars.css' => 'acda2fd6', 'css/aphront/aphront-notes.css' => '93ef49a2', @@ -87,7 +87,7 @@ return array( 'css/application/ponder/feed.css' => 'ed499cdd', 'css/application/ponder/post.css' => '604dd0db', 'css/application/ponder/vote.css' => 'f7ae64cf', - 'css/application/profile/profile-view.css' => '3a1ce3a0', + 'css/application/profile/profile-view.css' => '2c0b2070', 'css/application/projects/phabricator-object-list-view.css' => '1f269d69', 'css/application/projects/project-tag.css' => '866ceb4c', 'css/application/releeph/releeph-branch.css' => '2e8bac9d', @@ -129,7 +129,7 @@ return array( 'css/phui/phui-info-panel.css' => '04242a5c', 'css/phui/phui-list.css' => 'ac42d16a', 'css/phui/phui-object-box.css' => '99263256', - 'css/phui/phui-object-item-list-view.css' => '236014b6', + 'css/phui/phui-object-item-list-view.css' => '6247b27c', 'css/phui/phui-pinboard-view.css' => '007436fb', 'css/phui/phui-property-list-view.css' => '405c1b0e', 'css/phui/phui-remarkup-preview.css' => '9a9bf0a0', @@ -695,7 +695,7 @@ return array( 'phabricator-object-selector-css' => '244c904f', 'phabricator-phtize' => '2abf8c21', 'phabricator-prefab' => '8925fbf3', - 'phabricator-profile-css' => '3a1ce3a0', + 'phabricator-profile-css' => '2c0b2070', 'phabricator-project-tag-css' => '866ceb4c', 'phabricator-remarkup-css' => '5e94c989', 'phabricator-search-results-css' => '194d433f', @@ -741,7 +741,7 @@ return array( 'phui-info-panel-css' => '04242a5c', 'phui-list-view-css' => 'ac42d16a', 'phui-object-box-css' => '99263256', - 'phui-object-item-list-view-css' => '236014b6', + 'phui-object-item-list-view-css' => '6247b27c', 'phui-pinboard-view-css' => '007436fb', 'phui-property-list-view-css' => '405c1b0e', 'phui-remarkup-preview-css' => '9a9bf0a0', diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index ec46010ff1..e07ec0e9e9 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -98,6 +98,7 @@ phutil_register_library_map(array( 'CelerityManagementWorkflow' => 'infrastructure/celerity/management/CelerityManagementWorkflow.php', 'CelerityPhabricatorResourceController' => 'infrastructure/celerity/CelerityPhabricatorResourceController.php', 'CelerityPhabricatorResources' => 'infrastructure/celerity/resources/CelerityPhabricatorResources.php', + 'CelerityPhysicalResources' => 'infrastructure/celerity/resources/CelerityPhysicalResources.php', 'CelerityResourceController' => 'infrastructure/celerity/CelerityResourceController.php', 'CelerityResourceGraph' => 'infrastructure/celerity/CelerityResourceGraph.php', 'CelerityResourceMap' => 'infrastructure/celerity/CelerityResourceMap.php', @@ -2096,6 +2097,7 @@ phutil_register_library_map(array( 'PhameBlogQuery' => 'applications/phame/query/PhameBlogQuery.php', 'PhameBlogSkin' => 'applications/phame/skins/PhameBlogSkin.php', 'PhameBlogViewController' => 'applications/phame/controller/blog/PhameBlogViewController.php', + 'PhameCelerityResources' => 'applications/phame/celerity/PhameCelerityResources.php', 'PhameController' => 'applications/phame/controller/PhameController.php', 'PhameDAO' => 'applications/phame/storage/PhameDAO.php', 'PhamePost' => 'applications/phame/storage/PhamePost.php', @@ -2514,10 +2516,11 @@ phutil_register_library_map(array( 'CelerityManagementWorkflow' => 'PhabricatorManagementWorkflow', 'CelerityPhabricatorResourceController' => 'CelerityResourceController', 'CelerityPhabricatorResources' => 'CelerityResourcesOnDisk', + 'CelerityPhysicalResources' => 'CelerityResources', 'CelerityResourceController' => 'PhabricatorController', 'CelerityResourceGraph' => 'AbstractDirectedGraph', 'CelerityResourceTransformerTestCase' => 'PhabricatorTestCase', - 'CelerityResourcesOnDisk' => 'CelerityResources', + 'CelerityResourcesOnDisk' => 'CelerityPhysicalResources', 'CommandBuildStepImplementation' => 'VariableBuildStepImplementation', 'ConduitAPIMethod' => array( @@ -4726,6 +4729,7 @@ phutil_register_library_map(array( 'PhameBlogQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhameBlogSkin' => 'PhabricatorController', 'PhameBlogViewController' => 'PhameController', + 'PhameCelerityResources' => 'CelerityResources', 'PhameController' => 'PhabricatorController', 'PhameDAO' => 'PhabricatorLiskDAO', 'PhamePost' => diff --git a/src/applications/phame/celerity/PhameCelerityResources.php b/src/applications/phame/celerity/PhameCelerityResources.php new file mode 100644 index 0000000000..d7951dc731 --- /dev/null +++ b/src/applications/phame/celerity/PhameCelerityResources.php @@ -0,0 +1,28 @@ +skin = $skin; + return $this; + } + + public function getSkin() { + return $this->skin; + } + + public function getName() { + return 'phame:'.$this->getSkin()->getName(); + } + + public function getResourceData($name) { + $resource_path = $this->skin->getRootDirectory().DIRECTORY_SEPARATOR.$name; + return Filesystem::readFile($resource_path); + } + +} diff --git a/src/applications/phame/controller/PhameResourceController.php b/src/applications/phame/controller/PhameResourceController.php index a6f52a3ceb..f63c3d765c 100644 --- a/src/applications/phame/controller/PhameResourceController.php +++ b/src/applications/phame/controller/PhameResourceController.php @@ -9,9 +9,10 @@ final class PhameResourceController extends CelerityResourceController { private $hash; private $name; private $root; + private $celerityResourceMap; - protected function getRootDirectory() { - return $this->root; + public function getCelerityResourceMap() { + return $this->celerityResourceMap; } public function willProcessRequest(array $data) { @@ -26,9 +27,13 @@ final class PhameResourceController extends CelerityResourceController { // We require a visible blog associated with a given skin to serve // resources, so you can't go fishing around where you shouldn't be. + // However, since these resources may be served off a CDN domain, we're + // bypassing the actual policy check. The blog needs to exist, but you + // don't necessarily need to be able to see it in order to see static + // resources on it. $blog = id(new PhameBlogQuery()) - ->setViewer($user) + ->setViewer(PhabricatorUser::getOmnipotentUser()) ->withIDs(array($this->id)) ->executeOne(); if (!$blog) { @@ -38,7 +43,12 @@ final class PhameResourceController extends CelerityResourceController { $skin = $blog->getSkinRenderer($request); $spec = $skin->getSpecification(); - $this->root = $spec->getRootDirectory().DIRECTORY_SEPARATOR; + $resources = new PhameCelerityResources(); + $resources->setSkin($spec); + + $this->root = $spec->getRootDirectory(); + $this->celerityResourceMap = new CelerityResourceMap($resources); + return $this->serveResource($this->name); } diff --git a/src/infrastructure/celerity/CelerityResourceController.php b/src/infrastructure/celerity/CelerityResourceController.php index fac4e86551..f192c049b5 100644 --- a/src/infrastructure/celerity/CelerityResourceController.php +++ b/src/infrastructure/celerity/CelerityResourceController.php @@ -2,8 +2,6 @@ abstract class CelerityResourceController extends PhabricatorController { - abstract protected function getRootDirectory(); - protected function buildResourceTransformer() { return null; } @@ -16,6 +14,10 @@ abstract class CelerityResourceController extends PhabricatorController { return false; } + public function getCelerityResourceMap() { + return CelerityResourceMap::getInstance(); + } + protected function serveResource($path, $package_hash = null) { // Sanity checking to keep this from exposing anything sensitive, since it // ultimately boils down to disk reads. @@ -37,7 +39,7 @@ abstract class CelerityResourceController extends PhabricatorController { return $this->makeResponseCacheable(new Aphront304Response()); } - $map = CelerityResourceMap::getInstance(); + $map = $this->getCelerityResourceMap(); if ($map->isPackageResource($path)) { $resource_names = $map->getResourceNamesForPackageName($path); diff --git a/src/infrastructure/celerity/management/CelerityManagementMapWorkflow.php b/src/infrastructure/celerity/management/CelerityManagementMapWorkflow.php index 1f84cf28c7..0795ac5467 100644 --- a/src/infrastructure/celerity/management/CelerityManagementMapWorkflow.php +++ b/src/infrastructure/celerity/management/CelerityManagementMapWorkflow.php @@ -13,7 +13,7 @@ final class CelerityManagementMapWorkflow } public function execute(PhutilArgumentParser $args) { - $resources_map = CelerityResources::getAll(); + $resources_map = CelerityPhysicalResources::getAll(); $this->log( pht( @@ -32,10 +32,10 @@ final class CelerityManagementMapWorkflow /** * Rebuild the resource map for a resource source. * - * @param CelerityResources Resource source to rebuild. + * @param CelerityPhysicalResources Resource source to rebuild. * @return void */ - private function rebuildResources(CelerityResources $resources) { + private function rebuildResources(CelerityPhysicalResources $resources) { $this->log( pht( 'Rebuilding resource source "%s" (%s)...', @@ -127,10 +127,12 @@ final class CelerityManagementMapWorkflow * Find binary resources (like PNG and SWF) and return information about * them. * - * @param CelerityResources Resource map to find binary resources for. + * @param CelerityPhysicalResources Resource map to find binary resources for. * @return map> Resource information map. */ - private function rebuildBinaryResources(CelerityResources $resources) { + private function rebuildBinaryResources( + CelerityPhysicalResources $resources) { + $binary_map = $resources->findBinaryResources(); $result_map = array(); @@ -150,12 +152,12 @@ final class CelerityManagementMapWorkflow /** * Find text resources (like JS and CSS) and return information about them. * - * @param CelerityResources Resource map to find text resources for. + * @param CelerityPhysicalResources Resource map to find text resources for. * @param CelerityResourceTransformer Configured resource transformer. * @return map> Resource information map. */ private function rebuildTextResources( - CelerityResources $resources, + CelerityPhysicalResources $resources, CelerityResourceTransformer $xformer) { $text_map = $resources->findTextResources(); @@ -262,13 +264,13 @@ final class CelerityManagementMapWorkflow /** * Build package specifications for a given resource source. * - * @param CelerityResources Resource source to rebuild. + * @param CelerityPhysicalResources Resource source to rebuild. * @param list Map of `@provides` to hashes. * @param list Map of hashes to resource names. * @return map> Package information maps. */ private function rebuildPackages( - CelerityResources $resources, + CelerityPhysicalResources $resources, array $symbol_map, array $reverse_map) { diff --git a/src/infrastructure/celerity/resources/CelerityPhysicalResources.php b/src/infrastructure/celerity/resources/CelerityPhysicalResources.php new file mode 100644 index 0000000000..e6d37dc276 --- /dev/null +++ b/src/infrastructure/celerity/resources/CelerityPhysicalResources.php @@ -0,0 +1,52 @@ +map === null) { + $this->map = include $this->getPathToMap(); + } + return $this->map; + } + + public static function getAll() { + static $resources_map; + if ($resources_map === null) { + $resources_map = array(); + + $resources_list = id(new PhutilSymbolLoader()) + ->setAncestorClass('CelerityPhysicalResources') + ->loadObjects(); + + foreach ($resources_list as $resources) { + $name = $resources->getName(); + if (empty($resources_map[$name])) { + $resources_map[$name] = $resources; + } else { + $old = get_class($resources_map[$name]); + $new = get_class($resources); + throw new Exception( + pht( + 'Celerity resource maps must have unique names, but maps %s and '. + '%s share the same name, "%s".', + $old, + $new, + $name)); + } + } + } + + return $resources_map; + } + +} diff --git a/src/infrastructure/celerity/resources/CelerityResources.php b/src/infrastructure/celerity/resources/CelerityResources.php index 67090f827c..f3d8a93b76 100644 --- a/src/infrastructure/celerity/resources/CelerityResources.php +++ b/src/infrastructure/celerity/resources/CelerityResources.php @@ -8,11 +8,11 @@ abstract class CelerityResources { private $map; abstract public function getName(); - abstract public function getPathToMap(); abstract public function getResourceData($name); - abstract public function findBinaryResources(); - abstract public function findTextResources(); - abstract public function getResourceModifiedTime($name); + + public function getResourceModifiedTime($name) { + return 0; + } public function getCelerityHash($data) { $tail = PhabricatorEnv::getEnvConfig('celerity.resource-hash'); @@ -33,40 +33,7 @@ abstract class CelerityResources { } public function loadMap() { - if ($this->map === null) { - $this->map = include $this->getPathToMap(); - } - return $this->map; - } - - public static function getAll() { - static $resources_map; - if ($resources_map === null) { - $resources_map = array(); - - $resources_list = id(new PhutilSymbolLoader()) - ->setAncestorClass('CelerityResources') - ->loadObjects(); - - foreach ($resources_list as $resources) { - $name = $resources->getName(); - if (empty($resources_map[$name])) { - $resources_map[$name] = $resources; - } else { - $old = get_class($resources_map[$name]); - $new = get_class($resources); - throw new Exception( - pht( - 'Celerity resource maps must have unique names, but maps %s and '. - '%s share the same name, "%s".', - $old, - $new, - $name)); - } - } - } - - return $resources_map; + return array(); } } diff --git a/src/infrastructure/celerity/resources/CelerityResourcesOnDisk.php b/src/infrastructure/celerity/resources/CelerityResourcesOnDisk.php index b944c22187..0aa46bb3d9 100644 --- a/src/infrastructure/celerity/resources/CelerityResourcesOnDisk.php +++ b/src/infrastructure/celerity/resources/CelerityResourcesOnDisk.php @@ -3,7 +3,7 @@ /** * Defines the location of static resources on disk. */ -abstract class CelerityResourcesOnDisk extends CelerityResources { +abstract class CelerityResourcesOnDisk extends CelerityPhysicalResources { abstract public function getPathToResources();