Skip to content

Commit 31b6f69

Browse files
author
epriestley
committed
Allow CelerityResourceResponse to hold resources from multiple maps
Summary: Ref T4222. Currently, CelerityResourceResponse holds response resources in flat maps. Instead, specify which map resources appear in. Also, provide `requireResource()` and `initBehavior()` APIs on the Controller and View base classes. These provide a cleaner abstraction over `require_celerity_resource()` and `Javelin::initBehavior()`, but are otherwise the same. Move a few callsites over. Test Plan: - Reloaded pages. - Browsed around Differential. Reviewers: btrahan, hach-que Reviewed By: btrahan CC: aran Maniphest Tasks: T4222 Differential Revision: https://secure.phabricator.com/D7876
1 parent 09341be commit 31b6f69

18 files changed

+146
-53
lines changed

src/aphront/AphrontController.php

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ abstract class AphrontController extends Phobject {
99
private $currentApplication;
1010
private $delegatingController;
1111

12-
1312
public function setDelegatingController(
1413
AphrontController $delegating_controller) {
1514
$this->delegatingController = $delegating_controller;
@@ -64,4 +63,24 @@ final public function getCurrentApplication() {
6463
return $this->currentApplication;
6564
}
6665

66+
public function getDefaultResourceSource() {
67+
throw new Exception(
68+
pht(
69+
'A Controller must implement getDefaultResourceSource() before you '.
70+
'can invoke requireResource() or initBehavior().'));
71+
}
72+
73+
public function requireResource($symbol) {
74+
$response = CelerityAPI::getStaticResourceResponse();
75+
$response->requireResource($symbol, $this->getDefaultResourceSource());
76+
return $this;
77+
}
78+
79+
public function initBehavior($name, $config = array()) {
80+
Javelin::initBehavior(
81+
$name,
82+
$config,
83+
$this->getDefaultResourceSource());
84+
}
85+
6786
}

src/applications/base/controller/PhabricatorController.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,4 +408,9 @@ protected function explainApplicationCapability(
408408
return array($can_act, $message);
409409
}
410410

411+
public function getDefaultResourceSource() {
412+
return 'phabricator';
413+
}
414+
415+
411416
}

src/applications/differential/controller/DifferentialRevisionViewController.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -497,8 +497,8 @@ private function getRevisionActions(DifferentialRevision $revision) {
497497
);
498498
}
499499

500-
require_celerity_resource('phabricator-object-selector-css');
501-
require_celerity_resource('javelin-behavior-phabricator-object-selector');
500+
$this->requireResource('phabricator-object-selector-css');
501+
$this->requireResource('javelin-behavior-phabricator-object-selector');
502502

503503
$links[] = array(
504504
'icon' => 'link',

src/applications/differential/view/DifferentialAddCommentView.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public function setCCs(array $names) {
4848

4949
public function render() {
5050

51-
require_celerity_resource('differential-revision-add-comment-css');
51+
$this->requireResource('differential-revision-add-comment-css');
5252

5353
$is_serious = PhabricatorEnv::getEnvConfig('phabricator.serious-business');
5454

src/applications/differential/view/DifferentialChangesetDetailView.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,8 @@ public function getFileIcon($filename) {
9494
}
9595

9696
public function render() {
97-
require_celerity_resource('differential-changeset-view-css');
98-
require_celerity_resource('syntax-highlighting-css');
97+
$this->requireResource('differential-changeset-view-css');
98+
$this->requireResource('syntax-highlighting-css');
9999

100100
Javelin::initBehavior('phabricator-oncopy', array());
101101

src/applications/differential/view/DifferentialChangesetListView.php

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ public function setRawFileURIs($l, $r) {
103103
}
104104

105105
public function render() {
106-
require_celerity_resource('differential-changeset-view-css');
106+
$this->requireResource('differential-changeset-view-css');
107107

108108
$changesets = $this->changesets;
109109

@@ -183,20 +183,20 @@ public function render() {
183183
$output[] = $detail->render();
184184
}
185185

186-
require_celerity_resource('aphront-tooltip-css');
186+
$this->requireResource('aphront-tooltip-css');
187187

188-
Javelin::initBehavior('differential-populate', array(
188+
$this->initBehavior('differential-populate', array(
189189
'registry' => $mapping,
190190
'whitespace' => $this->whitespace,
191191
'uri' => $this->renderURI,
192192
));
193193

194-
Javelin::initBehavior('differential-show-more', array(
194+
$this->initBehavior('differential-show-more', array(
195195
'uri' => $this->renderURI,
196196
'whitespace' => $this->whitespace,
197197
));
198198

199-
Javelin::initBehavior('differential-comment-jump', array());
199+
$this->initBehavior('differential-comment-jump', array());
200200

201201
if ($this->inlineURI) {
202202
$undo_templates = $this->renderUndoTemplates();

src/applications/differential/view/DifferentialDiffTableOfContentsView.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@ public function setWhitespace($whitespace) {
5454

5555
public function render() {
5656

57-
require_celerity_resource('differential-core-view-css');
58-
require_celerity_resource('differential-table-of-contents-css');
57+
$this->requireResource('differential-core-view-css');
58+
$this->requireResource('differential-table-of-contents-css');
5959

6060
$rows = array();
6161

src/applications/differential/view/DifferentialLocalCommitsView.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public function render() {
2020
return null;
2121
}
2222

23-
require_celerity_resource('differential-local-commits-view-css');
23+
$this->requireResource('differential-local-commits-view-css');
2424

2525
$has_tree = false;
2626
$has_local = false;

src/applications/differential/view/DifferentialResultsTableView.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,10 +97,10 @@ public function render() {
9797
),
9898
phutil_tag('th', array('colspan' => 2), $hide_more));
9999

100-
Javelin::initBehavior('differential-show-field-details');
100+
$this->initBehavior('differential-show-field-details');
101101
}
102102

103-
require_celerity_resource('differential-results-table-css');
103+
$this->requireResource('differential-results-table-css');
104104

105105
return javelin_tag(
106106
'table',

src/applications/differential/view/DifferentialRevisionCommentListView.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public function getID() {
5353

5454
public function render() {
5555

56-
require_celerity_resource('differential-revision-comment-list-css');
56+
$this->requireResource('differential-revision-comment-list-css');
5757

5858
$engine = new PhabricatorMarkupEngine();
5959
$engine->setViewer($this->user);
@@ -154,7 +154,7 @@ public function render() {
154154
$visible = array_reverse($visible);
155155

156156
if ($hidden) {
157-
Javelin::initBehavior(
157+
$this->initBehavior(
158158
'differential-show-all-comments',
159159
array(
160160
'markup' => implode("\n", $hidden),

src/applications/differential/view/DifferentialRevisionCommentView.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,8 @@ public function render() {
6767
throw new Exception("Call setUser() before rendering!");
6868
}
6969

70-
require_celerity_resource('phabricator-remarkup-css');
71-
require_celerity_resource('differential-revision-comment-css');
70+
$this->requireResource('phabricator-remarkup-css');
71+
$this->requireResource('differential-revision-comment-css');
7272

7373
$comment = $this->comment;
7474

src/applications/differential/view/DifferentialRevisionDetailView.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public function setAuxiliaryFields(array $fields) {
4545

4646
public function render() {
4747

48-
require_celerity_resource('differential-core-view-css');
48+
$this->requireResource('differential-core-view-css');
4949

5050
$revision = $this->revision;
5151
$user = $this->getUser();

src/applications/differential/view/DifferentialRevisionListView.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,8 @@ public function render() {
102102
-$stale);
103103
}
104104

105-
Javelin::initBehavior('phabricator-tooltips', array());
106-
require_celerity_resource('aphront-tooltip-css');
105+
$this->initBehavior('phabricator-tooltips', array());
106+
$this->requireResource('aphront-tooltip-css');
107107

108108
$flagged = mpull($this->flags, null, 'getObjectPHID');
109109

src/applications/differential/view/DifferentialRevisionUpdateHistoryView.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ public function setSelectedWhitespace($whitespace) {
3030

3131
public function render() {
3232

33-
require_celerity_resource('differential-core-view-css');
34-
require_celerity_resource('differential-revision-history-css');
33+
$this->requireResource('differential-core-view-css');
34+
$this->requireResource('differential-revision-history-css');
3535

3636
$data = array(
3737
array(

src/infrastructure/celerity/CelerityResourceMap.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,4 +236,8 @@ public function isPackageResource($name) {
236236
return isset($this->packageMap[$name]);
237237
}
238238

239+
public function getResourceTypeForName($name) {
240+
return $this->resources->getResourceType($name);
241+
}
242+
239243
}

src/infrastructure/celerity/CelerityStaticResourceResponse.php

Lines changed: 74 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,12 @@ public function getMetadataBlock() {
3939
* a behavior will execute only once even if it is initialized multiple times.
4040
* If $config is nonempty, the behavior will be invoked once for each config.
4141
*/
42-
public function initBehavior($behavior, array $config = array()) {
43-
$this->requireResource('javelin-behavior-'.$behavior);
42+
public function initBehavior(
43+
$behavior,
44+
array $config = array(),
45+
$source_name = 'phabricator') {
46+
47+
$this->requireResource('javelin-behavior-'.$behavior, $source_name);
4448

4549
if (empty($this->behaviors[$behavior])) {
4650
$this->behaviors[$behavior] = array();
@@ -53,19 +57,39 @@ public function initBehavior($behavior, array $config = array()) {
5357
return $this;
5458
}
5559

56-
public function requireResource($symbol) {
57-
$this->symbols[$symbol] = true;
60+
public function requireResource($symbol, $source_name) {
61+
if (isset($this->symbols[$source_name][$symbol])) {
62+
return $this;
63+
}
64+
65+
// Verify that the resource exists.
66+
$map = CelerityResourceMap::getNamedInstance($source_name);
67+
$name = $map->getResourceNameForSymbol($symbol);
68+
if ($name === null) {
69+
throw new Exception(
70+
pht(
71+
'No resource with symbol "%s" exists in source "%s"!',
72+
$symbol,
73+
$source_name));
74+
}
75+
76+
$this->symbols[$source_name][$symbol] = true;
5877
$this->needsResolve = true;
78+
5979
return $this;
6080
}
6181

6282
private function resolveResources() {
6383
if ($this->needsResolve) {
64-
$map = CelerityResourceMap::getNamedInstance('phabricator');
84+
$this->packaged = array();
85+
foreach ($this->symbols as $source_name => $symbols_map) {
86+
$symbols = array_keys($symbols_map);
6587

66-
$symbols = array_keys($this->symbols);
67-
$this->packaged = $map->getPackagedNamesForSymbols($symbols);
88+
$map = CelerityResourceMap::getNamedInstance($source_name);
89+
$packaged = $map->getPackagedNamesForSymbols($symbols);
6890

91+
$this->packaged[$source_name] = $packaged;
92+
}
6993
$this->needsResolve = false;
7094
}
7195
return $this;
@@ -74,40 +98,54 @@ private function resolveResources() {
7498
public function renderSingleResource($symbol, $source_name) {
7599
$map = CelerityResourceMap::getNamedInstance($source_name);
76100
$packaged = $map->getPackagedNamesForSymbols(array($symbol));
77-
return $this->renderPackagedResources($packaged);
101+
return $this->renderPackagedResources($map, $packaged);
78102
}
79103

80104
public function renderResourcesOfType($type) {
81105
$this->resolveResources();
82106

83-
$resources = array();
84-
foreach ($this->packaged as $name) {
85-
$resource_type = CelerityResourceTransformer::getResourceType($name);
86-
if ($resource_type == $type) {
87-
$resources[] = $name;
107+
$result = array();
108+
foreach ($this->packaged as $source_name => $resource_names) {
109+
$map = CelerityResourceMap::getNamedInstance($source_name);
110+
111+
$resources_of_type = array();
112+
foreach ($resource_names as $resource_name) {
113+
$resource_type = $map->getResourceTypeForName($resource_name);
114+
if ($resource_type == $type) {
115+
$resources_of_type[] = $resource_name;
116+
}
88117
}
118+
119+
$result[] = $this->renderPackagedResources($map, $resources_of_type);
89120
}
90121

91-
return $this->renderPackagedResources($resources);
122+
return phutil_implode_html('', $result);
92123
}
93124

94-
private function renderPackagedResources(array $resources) {
125+
private function renderPackagedResources(
126+
CelerityResourceMap $map,
127+
array $resources) {
128+
95129
$output = array();
96130
foreach ($resources as $name) {
97131
if (isset($this->hasRendered[$name])) {
98132
continue;
99133
}
100134
$this->hasRendered[$name] = true;
101135

102-
$output[] = $this->renderResource($name);
103-
$output[] = "\n";
136+
$output[] = $this->renderResource($map, $name);
104137
}
105-
return phutil_implode_html('', $output);
138+
139+
return $output;
106140
}
107141

108-
private function renderResource($name) {
109-
$uri = $this->getURI($name);
110-
$type = CelerityResourceTransformer::getResourceType($name);
142+
private function renderResource(
143+
CelerityResourceMap $map,
144+
$name) {
145+
146+
$uri = $this->getURI($map, $name);
147+
$type = $map->getResourceTypeForName($name);
148+
111149
switch ($type) {
112150
case 'css':
113151
return phutil_tag(
@@ -126,7 +164,12 @@ private function renderResource($name) {
126164
),
127165
'');
128166
}
129-
throw new Exception("Unable to render resource.");
167+
168+
throw new Exception(
169+
pht(
170+
'Unable to render resource "%s", which has unknown type "%s".',
171+
$name,
172+
$type));
130173
}
131174

132175
public function renderHTMLFooter() {
@@ -225,8 +268,11 @@ public function buildAjaxResponse($payload, $error = null) {
225268

226269
$this->resolveResources();
227270
$resources = array();
228-
foreach ($this->packaged as $resource) {
229-
$resources[] = $this->getURI($resource);
271+
foreach ($this->packaged as $source_name => $resource_names) {
272+
$map = CelerityResourceMap::getNamedInstance($source_name);
273+
foreach ($resource_names as $resource_name) {
274+
$resources[] = $this->getURI($map, $resource_name);
275+
}
230276
}
231277
if ($resources) {
232278
$response['javelin_resources'] = $resources;
@@ -235,8 +281,10 @@ public function buildAjaxResponse($payload, $error = null) {
235281
return $response;
236282
}
237283

238-
private function getURI($name) {
239-
$map = CelerityResourceMap::getNamedInstance('phabricator');
284+
private function getURI(
285+
CelerityResourceMap $map,
286+
$name) {
287+
240288
$uri = $map->getURIForName($name);
241289

242290
// In developer mode, we dump file modification times into the URI. When a

src/infrastructure/celerity/api.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@
1515
*
1616
* @group celerity
1717
*/
18-
function require_celerity_resource($symbol) {
18+
function require_celerity_resource($symbol, $source_name = 'phabricator') {
1919
$response = CelerityAPI::getStaticResourceResponse();
20-
$response->requireResource($symbol);
20+
$response->requireResource($symbol, $source_name);
2121
}
2222

2323

0 commit comments

Comments
 (0)