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

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);

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) {
+  public function getModifiedTimeForName($name) {
-    $info = $this->lookupSymbolInformation($symbol);
+    $package_hash = null;
-    if ($info) {
+    foreach ($this->packageMap['packages'] as $hash => $package) {
-      $root = dirname(phutil_get_library_root('phabricator')).'/webroot';
+      if ($package['name'] == $name) {
-      return (int)filemtime($root.$info['disk']);
+        $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
+   * Return the absolute URI for the resource associated with a symbol. This
-   * a symbol. This method is fairly low-level and ignores packaging.
+   * 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
+   * Return the absolute URI for the resource associated with a resource name.
-   * a resource name. This method is fairly low-level and ignores packaging.
+   * 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;
   }
 

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;
       }

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->getPackagedNamesForSymbols(array($symbol));
-    $packaged = $map->packageResources($resolved);
     return $this->renderPackagedResources($packaged);
   }
 
@@ -80,9 +81,10 @@ final class CelerityStaticResourceResponse {
     $this->resolveResources();
 
     $resources = array();
-    foreach ($this->packaged as $resource) {
+    foreach ($this->packaged as $name) {
-      if ($resource['type'] == $type) {
+      $resource_type = CelerityResourceTransformer::getResourceType($name);
-        $resources[] = $resource;
+      if ($resource_type == $type) {
+        $resources[] = $name;
       }
     }
 
@@ -91,21 +93,22 @@ final class CelerityStaticResourceResponse {
 
   private function renderPackagedResources(array $resources) {
     $output = array();
-    foreach ($resources as $resource) {
+    foreach ($resources as $name) {
-      if (isset($this->hasRendered[$resource['uri']])) {
+      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) {
+  private function renderResource($name) {
-    $uri = $this->getURI($resource);
+    $uri = $this->getURI($name);
-    switch ($resource['type']) {
+    $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) {
+  private function getURI($name) {
-    $uri = $resource['uri'];
+    $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';
+      $mtime = $map->getModifiedTimeForName($name);
-      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));
-        }
-      }
-
       $uri = preg_replace('@^/res/@', '/res/'.$mtime.'T/', $uri);
     }
 

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;
   }