Skip to content

Commit bcc5e55

Browse files
author
epriestley
committed
Push construction of routing maps into Sites
Summary: This enables CORGI. Currently, `AphrontSite` subclasses can't really have their own routes. They can do this sort of hacky rewriting of paths, but that's a mess and not desirable in the long run. Instead, let subclasses build their own routing maps. This will let CORP and ORG have their own routing maps. I was able to get rid of the `PhameBlogResourcesSite` since it can really just share the standard resources site. Test Plan: - With no base URI set, and a base URI set, loaded main page and resources (from main site). - With file domain set, loaded resources from main site and file site. - Loaded a skinned blog from a domain. - Loaded a skinned blog from the main site. - Viewed "Request" tab of DarkConsole to see site/controller info. Reviewers: chad Reviewed By: chad Differential Revision: https://secure.phabricator.com/D14008
1 parent 2665970 commit bcc5e55

18 files changed

+478
-284
lines changed

scripts/aphront/aphrontpath.php

Lines changed: 0 additions & 28 deletions
This file was deleted.

src/__phutil_library_map__.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,8 @@
158158
'AphrontRequest' => 'aphront/AphrontRequest.php',
159159
'AphrontRequestTestCase' => 'aphront/__tests__/AphrontRequestTestCase.php',
160160
'AphrontResponse' => 'aphront/response/AphrontResponse.php',
161+
'AphrontRoutingMap' => 'aphront/site/AphrontRoutingMap.php',
162+
'AphrontRoutingResult' => 'aphront/site/AphrontRoutingResult.php',
161163
'AphrontSideNavFilterView' => 'view/layout/AphrontSideNavFilterView.php',
162164
'AphrontSite' => 'aphront/site/AphrontSite.php',
163165
'AphrontStackTraceView' => 'view/widget/AphrontStackTraceView.php',
@@ -166,7 +168,6 @@
166168
'AphrontTagView' => 'view/AphrontTagView.php',
167169
'AphrontTokenizerTemplateView' => 'view/control/AphrontTokenizerTemplateView.php',
168170
'AphrontTypeaheadTemplateView' => 'view/control/AphrontTypeaheadTemplateView.php',
169-
'AphrontURIMapper' => 'aphront/AphrontURIMapper.php',
170171
'AphrontUnhandledExceptionResponse' => 'aphront/response/AphrontUnhandledExceptionResponse.php',
171172
'AphrontUsageException' => 'aphront/exception/AphrontUsageException.php',
172173
'AphrontView' => 'view/AphrontView.php',
@@ -3126,7 +3127,6 @@
31263127
'PhameBlogListController' => 'applications/phame/controller/blog/PhameBlogListController.php',
31273128
'PhameBlogLiveController' => 'applications/phame/controller/blog/PhameBlogLiveController.php',
31283129
'PhameBlogQuery' => 'applications/phame/query/PhameBlogQuery.php',
3129-
'PhameBlogResourceSite' => 'applications/phame/site/PhameBlogResourceSite.php',
31303130
'PhameBlogSearchEngine' => 'applications/phame/query/PhameBlogSearchEngine.php',
31313131
'PhameBlogSite' => 'applications/phame/site/PhameBlogSite.php',
31323132
'PhameBlogSkin' => 'applications/phame/skins/PhameBlogSkin.php',
@@ -3777,6 +3777,8 @@
37773777
'AphrontRequest' => 'Phobject',
37783778
'AphrontRequestTestCase' => 'PhabricatorTestCase',
37793779
'AphrontResponse' => 'Phobject',
3780+
'AphrontRoutingMap' => 'Phobject',
3781+
'AphrontRoutingResult' => 'Phobject',
37803782
'AphrontSideNavFilterView' => 'AphrontView',
37813783
'AphrontSite' => 'Phobject',
37823784
'AphrontStackTraceView' => 'AphrontView',
@@ -3785,7 +3787,6 @@
37853787
'AphrontTagView' => 'AphrontView',
37863788
'AphrontTokenizerTemplateView' => 'AphrontView',
37873789
'AphrontTypeaheadTemplateView' => 'AphrontView',
3788-
'AphrontURIMapper' => 'Phobject',
37893790
'AphrontUnhandledExceptionResponse' => 'AphrontStandaloneHTMLResponse',
37903791
'AphrontUsageException' => 'AphrontException',
37913792
'AphrontView' => array(
@@ -7238,7 +7239,6 @@
72387239
'PhameBlogListController' => 'PhameController',
72397240
'PhameBlogLiveController' => 'PhameController',
72407241
'PhameBlogQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
7241-
'PhameBlogResourceSite' => 'PhameSite',
72427242
'PhameBlogSearchEngine' => 'PhabricatorApplicationSearchEngine',
72437243
'PhameBlogSite' => 'PhameSite',
72447244
'PhameBlogSkin' => 'PhabricatorController',

src/aphront/AphrontRequest.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ final class AphrontRequest extends Phobject {
2626
private $requestData;
2727
private $user;
2828
private $applicationConfiguration;
29+
private $site;
30+
private $controller;
2931
private $uriData;
3032
private $cookiePrefix;
3133

@@ -77,6 +79,24 @@ public function getHost() {
7779
return $uri->getDomain();
7880
}
7981

82+
public function setSite(AphrontSite $site) {
83+
$this->site = $site;
84+
return $this;
85+
}
86+
87+
public function getSite() {
88+
return $this->site;
89+
}
90+
91+
public function setController(AphrontController $controller) {
92+
$this->controller = $controller;
93+
return $this;
94+
}
95+
96+
public function getController() {
97+
return $this->controller;
98+
}
99+
80100

81101
/* -( Accessing Request Data )--------------------------------------------- */
82102

src/aphront/AphrontURIMapper.php

Lines changed: 0 additions & 50 deletions
This file was deleted.

src/aphront/configuration/AphrontApplicationConfiguration.php

Lines changed: 31 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,9 @@ public function processRequest(
210210
));
211211
$multimeter->setEventContext('web.'.$controller_class);
212212

213+
$request->setController($controller);
213214
$request->setURIMap($uri_data);
215+
214216
$controller->setRequest($request);
215217

216218
// If execution throws an exception and then trying to render that
@@ -283,35 +285,13 @@ public function processRequest(
283285

284286

285287
/**
286-
* Using builtin and application routes, build the appropriate
287-
* @{class:AphrontController} class for the request. To route a request, we
288-
* first test if the HTTP_HOST is configured as a valid Phabricator URI. If
289-
* it isn't, we do a special check to see if it's a custom domain for a blog
290-
* in the Phame application and if that fails we error. Otherwise, we test
291-
* against all application routes from installed
292-
* @{class:PhabricatorApplication}s.
293-
*
294-
* If we match a route, we construct the controller it points at, build it,
295-
* and return it.
296-
*
297-
* If we fail to match a route, but the current path is missing a trailing
298-
* "/", we try routing the same path with a trailing "/" and do a redirect
299-
* if that has a valid route. The idea is to canoncalize URIs for consistency,
300-
* but avoid breaking noncanonical URIs that we can easily salvage.
301-
*
302-
* NOTE: We only redirect on GET. On POST, we'd drop parameters and most
303-
* likely mutate the request implicitly, and a bad POST usually indicates a
304-
* programming error rather than a sloppy typist.
305-
*
306-
* If the failing path already has a trailing "/", or we can't route the
307-
* version with a "/", we call @{method:build404Controller}, which build a
308-
* fallback @{class:AphrontController}.
288+
* Build a controller to respond to the request.
309289
*
310290
* @return pair<AphrontController,dict> Controller and dictionary of request
311291
* parameters.
312292
* @task routing
313293
*/
314-
final public function buildController() {
294+
final private function buildController() {
315295
$request = $this->getRequest();
316296

317297
// If we're configured to operate in cluster mode, reject requests which
@@ -373,78 +353,48 @@ final public function buildController() {
373353
}
374354
}
375355

376-
// TODO: Really, the Site should get more control here and be able to
377-
// do its own routing logic if it wants, but we don't need that for now.
378-
$path = $site->getPathForRouting($request);
379-
380-
list($controller, $uri_data) = $this->buildControllerForPath($path);
381-
if (!$controller) {
382-
if (!preg_match('@/$@', $path)) {
383-
// If we failed to match anything but don't have a trailing slash, try
384-
// to add a trailing slash and issue a redirect if that resolves.
385-
list($controller, $uri_data) = $this->buildControllerForPath($path.'/');
386-
387-
// NOTE: For POST, just 404 instead of redirecting, since the redirect
388-
// will be a GET without parameters.
356+
$maps = $site->getRoutingMaps();
357+
$path = $request->getPath();
389358

390-
if ($controller && !$request->isHTTPPost()) {
391-
$slash_uri = $request->getRequestURI()->setPath($path.'/');
359+
$result = $this->routePath($maps, $path);
360+
if ($result) {
361+
return $result;
362+
}
392363

393-
$external = strlen($request->getRequestURI()->getDomain());
394-
return $this->buildRedirectController($slash_uri, $external);
395-
}
364+
// If we failed to match anything but don't have a trailing slash, try
365+
// to add a trailing slash and issue a redirect if that resolves.
366+
367+
// NOTE: We only do this for GET, since redirects switch to GET and drop
368+
// data like POST parameters.
369+
if (!preg_match('@/$@', $path) && $request->isHTTPGet()) {
370+
$result = $this->routePath($maps, $path.'/');
371+
if ($result) {
372+
$slash_uri = $request->getRequestURI()->setPath($path.'/');
373+
$external = strlen($request->getRequestURI()->getDomain());
374+
return $this->buildRedirectController($slash_uri, $external);
396375
}
397-
return $this->build404Controller();
398376
}
399377

400-
return array($controller, $uri_data);
378+
return $this->build404Controller();
401379
}
402380

403-
404381
/**
405382
* Map a specific path to the corresponding controller. For a description
406383
* of routing, see @{method:buildController}.
407384
*
385+
* @param list<AphrontRoutingMap> List of routing maps.
386+
* @param string Path to route.
408387
* @return pair<AphrontController,dict> Controller and dictionary of request
409388
* parameters.
410389
* @task routing
411390
*/
412-
final public function buildControllerForPath($path) {
413-
$maps = array();
414-
415-
$applications = PhabricatorApplication::getAllInstalledApplications();
416-
foreach ($applications as $application) {
417-
$maps[] = array($application, $application->getRoutes());
418-
}
419-
420-
$current_application = null;
421-
$controller_class = null;
422-
foreach ($maps as $map_info) {
423-
list($application, $map) = $map_info;
424-
425-
$mapper = new AphrontURIMapper($map);
426-
list($controller_class, $uri_data) = $mapper->mapPath($path);
427-
428-
if ($controller_class) {
429-
if ($application) {
430-
$current_application = $application;
431-
}
432-
break;
391+
private function routePath(array $maps, $path) {
392+
foreach ($maps as $map) {
393+
$result = $map->routePath($path);
394+
if ($result) {
395+
return array($result->getController(), $result->getURIData());
433396
}
434397
}
435-
436-
if (!$controller_class) {
437-
return array(null, null);
438-
}
439-
440-
$request = $this->getRequest();
441-
442-
$controller = newv($controller_class, array());
443-
if ($current_application) {
444-
$controller->setCurrentApplication($current_application);
445-
}
446-
447-
return array($controller, $uri_data);
448398
}
449399

450400
private function buildSiteForRequest(AphrontRequest $request) {
@@ -469,6 +419,8 @@ private function buildSiteForRequest(AphrontRequest $request) {
469419
$host));
470420
}
471421

422+
$request->setSite($site);
423+
472424
return $site;
473425
}
474426

0 commit comments

Comments
 (0)