From 59ad78d4abc60a54da22edf2e6e62801bc0117ac Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 31 Dec 2013 18:03:17 -0800
Subject: [PATCH] Improve package resolution APIs on CelerityResourceMap

Summary:
Ref T4222. A few diffs from now, `CelerityResourceMap` will have a `CelerityResources` inside of it:

  - Rename `resolvePackage()` to `getResourceNamesForPackageHash()`. This isn't a functional change, it's just making it clear what it does.
  - Add `getResourceDataForName()`, to push details about storage into `CelerityResources`.

Test Plan: Reloaded a bunch of pages, rebuilt map.

Reviewers: btrahan, hach-que

Reviewed By: hach-que

CC: aran

Maniphest Tasks: T4222

Differential Revision: https://secure.phabricator.com/D7869
---
 .../celerity/CelerityResourceController.php   | 19 +++++++------------
 .../celerity/CelerityResourceMap.php          |  8 +++++++-
 2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/src/infrastructure/celerity/CelerityResourceController.php b/src/infrastructure/celerity/CelerityResourceController.php
index 8c72134875..0f60885c55 100644
--- a/src/infrastructure/celerity/CelerityResourceController.php
+++ b/src/infrastructure/celerity/CelerityResourceController.php
@@ -16,10 +16,6 @@ abstract class CelerityResourceController extends PhabricatorController {
     return false;
   }
 
-  private function getDiskPath($to_resource = null) {
-    return $this->getRootDirectory().$to_resource;
-  }
-
   protected function serveResource($path, $package_hash = null) {
     // Sanity checking to keep this from exposing anything sensitive, since it
     // ultimately boils down to disk reads.
@@ -41,18 +37,18 @@ abstract class CelerityResourceController extends PhabricatorController {
       return $this->makeResponseCacheable(new Aphront304Response());
     }
 
+    $map = CelerityResourceMap::getInstance();
+
     if ($package_hash) {
-      $map = CelerityResourceMap::getInstance();
-      $paths = $map->resolvePackage($package_hash);
-      if (!$paths) {
+      $resource_names = $map->getResourceNamesForPackageHash($package_hash);
+      if (!$resource_names) {
         return new Aphront404Response();
       }
 
       try {
         $data = array();
-        foreach ($paths as $package_path) {
-          $disk_path = $this->getDiskPath($package_path);
-          $data[] = Filesystem::readFile($disk_path);
+        foreach ($resource_names as $resource_name) {
+          $data[] = $map->getResourceDataForName($resource_name);
         }
         $data = implode("\n\n", $data);
       } catch (Exception $ex) {
@@ -60,8 +56,7 @@ abstract class CelerityResourceController extends PhabricatorController {
       }
     } else {
       try {
-        $disk_path = $this->getDiskPath($path);
-        $data = Filesystem::readFile($disk_path);
+        $data = $map->getResourceDataForName($path);
       } catch (Exception $ex) {
         return new Aphront404Response();
       }
diff --git a/src/infrastructure/celerity/CelerityResourceMap.php b/src/infrastructure/celerity/CelerityResourceMap.php
index ddcf5e038b..3e52e58de6 100644
--- a/src/infrastructure/celerity/CelerityResourceMap.php
+++ b/src/infrastructure/celerity/CelerityResourceMap.php
@@ -90,7 +90,13 @@ final class CelerityResourceMap {
     return $packaged;
   }
 
-  public function resolvePackage($package_hash) {
+  public function getResourceDataForName($resource_name) {
+    $root = phutil_get_library_root('phabricator');
+    $root = dirname($root).'/webroot/';
+    return Filesystem::readFile($root.$resource_name);
+  }
+
+  public function getResourceNamesForPackageHash($package_hash) {
     $package = idx($this->packageMap['packages'], $package_hash);
     if (!$package) {
       return null;