From 60cb65bfbb2def8139c411495dfd0bb02593bc9d Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 31 Dec 2013 18:03:24 -0800 Subject: [PATCH] Make resolveResources() and packageResources() private on CelerityResourceMap Summary: Ref T4222. These are the last two "return a big ball of mud" methods. Make the API stronger so I can swap out the implementations. Test Plan: Reloaded pages. Reviewers: btrahan, hach-que Reviewed By: hach-que CC: aran Maniphest Tasks: T4222 Differential Revision: https://secure.phabricator.com/D7871 --- .../skins/PhameBasicTemplateBlogSkin.php | 4 +- .../celerity/CelerityResourceMap.php | 75 +++++++++++++++---- .../celerity/CelerityResourceTransformer.php | 2 +- .../CelerityStaticResourceResponse.php | 48 ++++++------ src/infrastructure/celerity/api.php | 2 +- 5 files changed, 85 insertions(+), 46 deletions(-) diff --git a/src/applications/phame/skins/PhameBasicTemplateBlogSkin.php b/src/applications/phame/skins/PhameBasicTemplateBlogSkin.php index 25d0b66449..8bee67fe8f 100644 --- a/src/applications/phame/skins/PhameBasicTemplateBlogSkin.php +++ b/src/applications/phame/skins/PhameBasicTemplateBlogSkin.php @@ -31,14 +31,14 @@ final class PhameBasicTemplateBlogSkin extends PhameBasicBlogSkin { $map = CelerityResourceMap::getInstance(); $resource_symbol = 'syntax-highlighting-css'; - $resource_uri = $map->getFullyQualifiedURIForSymbol($resource_symbol); + $resource_uri = $map->getURIForSymbol($resource_symbol); $this->cssResources[] = phutil_tag( 'link', array( 'rel' => 'stylesheet', 'type' => 'text/css', - 'href' => $resource_uri, + 'href' => PhabricatorEnv::getCDNURI($resource_uri), )); $this->cssResources = phutil_implode_html("\n", $this->cssResources); diff --git a/src/infrastructure/celerity/CelerityResourceMap.php b/src/infrastructure/celerity/CelerityResourceMap.php index 3e52e58de6..623fca2bdd 100644 --- a/src/infrastructure/celerity/CelerityResourceMap.php +++ b/src/infrastructure/celerity/CelerityResourceMap.php @@ -35,7 +35,12 @@ final class CelerityResourceMap { return $this; } - public function resolveResources(array $symbols) { + public function getPackagedNamesForSymbols(array $symbols) { + $resolved = $this->resolveResources($symbols); + return $this->packageResources($resolved); + } + + private function resolveResources(array $symbols) { $map = array(); foreach ($symbols as $symbol) { if (!empty($map[$symbol])) { @@ -69,7 +74,7 @@ final class CelerityResourceMap { return $this; } - public function packageResources(array $resolved_map) { + private function packageResources(array $resolved_map) { $packaged = array(); $handled = array(); foreach ($resolved_map as $symbol => $info) { @@ -87,7 +92,17 @@ final class CelerityResourceMap { } } } - return $packaged; + + $names = array(); + foreach ($packaged as $key => $resource) { + if (isset($resource['disk'])) { + $names[] = $resource['disk']; + } else { + $names[] = $key; + } + } + + return $names; } public function getResourceDataForName($resource_name) { @@ -132,25 +147,48 @@ final class CelerityResourceMap { * @param string Resource symbol to lookup. * @return int Epoch timestamp of last resource modification. */ - public function getModifiedTimeForSymbol($symbol) { - $info = $this->lookupSymbolInformation($symbol); - if ($info) { - $root = dirname(phutil_get_library_root('phabricator')).'/webroot'; - return (int)filemtime($root.$info['disk']); + public function getModifiedTimeForName($name) { + $package_hash = null; + foreach ($this->packageMap['packages'] as $hash => $package) { + if ($package['name'] == $name) { + $package_hash = $hash; + break; + } } - return 0; + + $root = dirname(phutil_get_library_root('phabricator')).'/webroot'; + + $mtime = 0; + + if ($package_hash) { + $names = $this->getResourceNamesForPackageHash($package_hash); + foreach ($names as $component_name) { + $info = $this->lookupFileInformation($component_name); + if ($info) { + $mtime = max($mtime, (int)filemtime($root.$info['disk'])); + } + } + } else { + $info = $this->lookupFileInformation($name); + if ($info) { + $root = dirname(phutil_get_library_root('phabricator')).'/webroot'; + $mtime = (int)filemtime($root.$info['disk']); + } + } + + return $mtime; } /** - * Return the fully-qualified, absolute URI for the resource associated with - * a symbol. This method is fairly low-level and ignores packaging. + * Return the absolute URI for the resource associated with a symbol. This + * method is fairly low-level and ignores packaging. * * @param string Resource symbol to lookup. * @return string|null Fully-qualified resource URI, or null if the symbol * is unknown. */ - public function getFullyQualifiedURIForSymbol($symbol) { + public function getURIForSymbol($symbol) { $info = $this->lookupSymbolInformation($symbol); if ($info) { return idx($info, 'uri'); @@ -160,18 +198,25 @@ final class CelerityResourceMap { /** - * Return the fully-qualified, absolute URI for the resource associated with - * a resource name. This method is fairly low-level and ignores packaging. + * Return the absolute URI for the resource associated with a resource name. + * This method is fairly low-level and ignores packaging. * * @param string Resource name to lookup. * @return string|null Fully-qualified resource URI, or null if the name * is unknown. */ - public function getFullyQualifiedURIForName($name) { + public function getURIForName($name) { $info = $this->lookupFileInformation($name); if ($info) { return idx($info, 'uri'); } + + foreach ($this->packageMap['packages'] as $hash => $package) { + if ($package['name'] == $name) { + return $package['uri']; + } + } + return null; } diff --git a/src/infrastructure/celerity/CelerityResourceTransformer.php b/src/infrastructure/celerity/CelerityResourceTransformer.php index d6b74926c7..846d8b5e89 100644 --- a/src/infrastructure/celerity/CelerityResourceTransformer.php +++ b/src/infrastructure/celerity/CelerityResourceTransformer.php @@ -127,7 +127,7 @@ final class CelerityResourceTransformer { $uri = $this->rawResourceMap[$uri]['uri']; } } else if ($this->celerityMap) { - $resource_uri = $this->celerityMap->getFullyQualifiedURIForName($uri); + $resource_uri = $this->celerityMap->getURIForName($uri); if ($resource_uri) { $uri = $resource_uri; } diff --git a/src/infrastructure/celerity/CelerityStaticResourceResponse.php b/src/infrastructure/celerity/CelerityStaticResourceResponse.php index aee24e4790..ee55074ac4 100644 --- a/src/infrastructure/celerity/CelerityStaticResourceResponse.php +++ b/src/infrastructure/celerity/CelerityStaticResourceResponse.php @@ -62,8 +62,10 @@ final class CelerityStaticResourceResponse { private function resolveResources() { if ($this->needsResolve) { $map = CelerityResourceMap::getInstance(); - $this->resolved = $map->resolveResources(array_keys($this->symbols)); - $this->packaged = $map->packageResources($this->resolved); + + $symbols = array_keys($this->symbols); + $this->packaged = $map->getPackagedNamesForSymbols($symbols); + $this->needsResolve = false; } return $this; @@ -71,8 +73,7 @@ final class CelerityStaticResourceResponse { public function renderSingleResource($symbol) { $map = CelerityResourceMap::getInstance(); - $resolved = $map->resolveResources(array($symbol)); - $packaged = $map->packageResources($resolved); + $packaged = $map->getPackagedNamesForSymbols(array($symbol)); return $this->renderPackagedResources($packaged); } @@ -80,9 +81,10 @@ final class CelerityStaticResourceResponse { $this->resolveResources(); $resources = array(); - foreach ($this->packaged as $resource) { - if ($resource['type'] == $type) { - $resources[] = $resource; + foreach ($this->packaged as $name) { + $resource_type = CelerityResourceTransformer::getResourceType($name); + if ($resource_type == $type) { + $resources[] = $name; } } @@ -91,21 +93,22 @@ final class CelerityStaticResourceResponse { private function renderPackagedResources(array $resources) { $output = array(); - foreach ($resources as $resource) { - if (isset($this->hasRendered[$resource['uri']])) { + foreach ($resources as $name) { + if (isset($this->hasRendered[$name])) { continue; } - $this->hasRendered[$resource['uri']] = true; + $this->hasRendered[$name] = true; - $output[] = $this->renderResource($resource); + $output[] = $this->renderResource($name); $output[] = "\n"; } return phutil_implode_html('', $output); } - private function renderResource(array $resource) { - $uri = $this->getURI($resource); - switch ($resource['type']) { + private function renderResource($name) { + $uri = $this->getURI($name); + $type = CelerityResourceTransformer::getResourceType($name); + switch ($type) { case 'css': return phutil_tag( 'link', @@ -232,8 +235,9 @@ final class CelerityStaticResourceResponse { return $response; } - private function getURI($resource) { - $uri = $resource['uri']; + private function getURI($name) { + $map = CelerityResourceMap::getInstance(); + $uri = $map->getURIForName($name); // In developer mode, we dump file modification times into the URI. When a // page is reloaded in the browser, any resources brought in by Ajax calls @@ -242,17 +246,7 @@ final class CelerityStaticResourceResponse { // the map script). In production, we can assume the map script gets run // after changes, and safely skip this. if (PhabricatorEnv::getEnvConfig('phabricator.developer-mode')) { - $root = dirname(phutil_get_library_root('phabricator')).'/webroot'; - if (isset($resource['disk'])) { - $mtime = (int)filemtime($root.$resource['disk']); - } else { - $mtime = 0; - foreach ($resource['symbols'] as $symbol) { - $map = CelerityResourceMap::getInstance(); - $mtime = max($mtime, $map->getModifiedTimeForSymbol($symbol)); - } - } - + $mtime = $map->getModifiedTimeForName($name); $uri = preg_replace('@^/res/@', '/res/'.$mtime.'T/', $uri); } diff --git a/src/infrastructure/celerity/api.php b/src/infrastructure/celerity/api.php index d12f3c3f16..1f5346df90 100644 --- a/src/infrastructure/celerity/api.php +++ b/src/infrastructure/celerity/api.php @@ -52,7 +52,7 @@ function celerity_generate_unique_node_id() { function celerity_get_resource_uri($resource) { $map = CelerityResourceMap::getInstance(); - $uri = $map->getFullyQualifiedURIForName($resource); + $uri = $map->getURIForName($resource); if ($uri) { return $uri; }