Skip to content

Commit

Permalink
Push construction of routing maps into Sites
Browse files Browse the repository at this point in the history
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
  • Loading branch information
epriestley committed Aug 31, 2015
1 parent 2665970 commit bcc5e55
Show file tree
Hide file tree
Showing 18 changed files with 478 additions and 284 deletions.
28 changes: 0 additions & 28 deletions scripts/aphront/aphrontpath.php

This file was deleted.

8 changes: 4 additions & 4 deletions src/__phutil_library_map__.php
Expand Up @@ -158,6 +158,8 @@
'AphrontRequest' => 'aphront/AphrontRequest.php',
'AphrontRequestTestCase' => 'aphront/__tests__/AphrontRequestTestCase.php',
'AphrontResponse' => 'aphront/response/AphrontResponse.php',
'AphrontRoutingMap' => 'aphront/site/AphrontRoutingMap.php',
'AphrontRoutingResult' => 'aphront/site/AphrontRoutingResult.php',
'AphrontSideNavFilterView' => 'view/layout/AphrontSideNavFilterView.php',
'AphrontSite' => 'aphront/site/AphrontSite.php',
'AphrontStackTraceView' => 'view/widget/AphrontStackTraceView.php',
Expand All @@ -166,7 +168,6 @@
'AphrontTagView' => 'view/AphrontTagView.php',
'AphrontTokenizerTemplateView' => 'view/control/AphrontTokenizerTemplateView.php',
'AphrontTypeaheadTemplateView' => 'view/control/AphrontTypeaheadTemplateView.php',
'AphrontURIMapper' => 'aphront/AphrontURIMapper.php',
'AphrontUnhandledExceptionResponse' => 'aphront/response/AphrontUnhandledExceptionResponse.php',
'AphrontUsageException' => 'aphront/exception/AphrontUsageException.php',
'AphrontView' => 'view/AphrontView.php',
Expand Down Expand Up @@ -3126,7 +3127,6 @@
'PhameBlogListController' => 'applications/phame/controller/blog/PhameBlogListController.php',
'PhameBlogLiveController' => 'applications/phame/controller/blog/PhameBlogLiveController.php',
'PhameBlogQuery' => 'applications/phame/query/PhameBlogQuery.php',
'PhameBlogResourceSite' => 'applications/phame/site/PhameBlogResourceSite.php',
'PhameBlogSearchEngine' => 'applications/phame/query/PhameBlogSearchEngine.php',
'PhameBlogSite' => 'applications/phame/site/PhameBlogSite.php',
'PhameBlogSkin' => 'applications/phame/skins/PhameBlogSkin.php',
Expand Down Expand Up @@ -3777,6 +3777,8 @@
'AphrontRequest' => 'Phobject',
'AphrontRequestTestCase' => 'PhabricatorTestCase',
'AphrontResponse' => 'Phobject',
'AphrontRoutingMap' => 'Phobject',
'AphrontRoutingResult' => 'Phobject',
'AphrontSideNavFilterView' => 'AphrontView',
'AphrontSite' => 'Phobject',
'AphrontStackTraceView' => 'AphrontView',
Expand All @@ -3785,7 +3787,6 @@
'AphrontTagView' => 'AphrontView',
'AphrontTokenizerTemplateView' => 'AphrontView',
'AphrontTypeaheadTemplateView' => 'AphrontView',
'AphrontURIMapper' => 'Phobject',
'AphrontUnhandledExceptionResponse' => 'AphrontStandaloneHTMLResponse',
'AphrontUsageException' => 'AphrontException',
'AphrontView' => array(
Expand Down Expand Up @@ -7238,7 +7239,6 @@
'PhameBlogListController' => 'PhameController',
'PhameBlogLiveController' => 'PhameController',
'PhameBlogQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'PhameBlogResourceSite' => 'PhameSite',
'PhameBlogSearchEngine' => 'PhabricatorApplicationSearchEngine',
'PhameBlogSite' => 'PhameSite',
'PhameBlogSkin' => 'PhabricatorController',
Expand Down
20 changes: 20 additions & 0 deletions src/aphront/AphrontRequest.php
Expand Up @@ -26,6 +26,8 @@ final class AphrontRequest extends Phobject {
private $requestData;
private $user;
private $applicationConfiguration;
private $site;
private $controller;
private $uriData;
private $cookiePrefix;

Expand Down Expand Up @@ -77,6 +79,24 @@ public function getHost() {
return $uri->getDomain();
}

public function setSite(AphrontSite $site) {
$this->site = $site;
return $this;
}

public function getSite() {
return $this->site;
}

public function setController(AphrontController $controller) {
$this->controller = $controller;
return $this;
}

public function getController() {
return $this->controller;
}


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

Expand Down
50 changes: 0 additions & 50 deletions src/aphront/AphrontURIMapper.php

This file was deleted.

110 changes: 31 additions & 79 deletions src/aphront/configuration/AphrontApplicationConfiguration.php
Expand Up @@ -210,7 +210,9 @@ public function processRequest(
));
$multimeter->setEventContext('web.'.$controller_class);

$request->setController($controller);
$request->setURIMap($uri_data);

$controller->setRequest($request);

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


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

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

// TODO: Really, the Site should get more control here and be able to
// do its own routing logic if it wants, but we don't need that for now.
$path = $site->getPathForRouting($request);

list($controller, $uri_data) = $this->buildControllerForPath($path);
if (!$controller) {
if (!preg_match('@/$@', $path)) {
// If we failed to match anything but don't have a trailing slash, try
// to add a trailing slash and issue a redirect if that resolves.
list($controller, $uri_data) = $this->buildControllerForPath($path.'/');

// NOTE: For POST, just 404 instead of redirecting, since the redirect
// will be a GET without parameters.
$maps = $site->getRoutingMaps();
$path = $request->getPath();

if ($controller && !$request->isHTTPPost()) {
$slash_uri = $request->getRequestURI()->setPath($path.'/');
$result = $this->routePath($maps, $path);
if ($result) {
return $result;
}

$external = strlen($request->getRequestURI()->getDomain());
return $this->buildRedirectController($slash_uri, $external);
}
// If we failed to match anything but don't have a trailing slash, try
// to add a trailing slash and issue a redirect if that resolves.

// NOTE: We only do this for GET, since redirects switch to GET and drop
// data like POST parameters.
if (!preg_match('@/$@', $path) && $request->isHTTPGet()) {
$result = $this->routePath($maps, $path.'/');
if ($result) {
$slash_uri = $request->getRequestURI()->setPath($path.'/');
$external = strlen($request->getRequestURI()->getDomain());
return $this->buildRedirectController($slash_uri, $external);
}
return $this->build404Controller();
}

return array($controller, $uri_data);
return $this->build404Controller();
}


/**
* Map a specific path to the corresponding controller. For a description
* of routing, see @{method:buildController}.
*
* @param list<AphrontRoutingMap> List of routing maps.
* @param string Path to route.
* @return pair<AphrontController,dict> Controller and dictionary of request
* parameters.
* @task routing
*/
final public function buildControllerForPath($path) {
$maps = array();

$applications = PhabricatorApplication::getAllInstalledApplications();
foreach ($applications as $application) {
$maps[] = array($application, $application->getRoutes());
}

$current_application = null;
$controller_class = null;
foreach ($maps as $map_info) {
list($application, $map) = $map_info;

$mapper = new AphrontURIMapper($map);
list($controller_class, $uri_data) = $mapper->mapPath($path);

if ($controller_class) {
if ($application) {
$current_application = $application;
}
break;
private function routePath(array $maps, $path) {
foreach ($maps as $map) {
$result = $map->routePath($path);
if ($result) {
return array($result->getController(), $result->getURIData());
}
}

if (!$controller_class) {
return array(null, null);
}

$request = $this->getRequest();

$controller = newv($controller_class, array());
if ($current_application) {
$controller->setCurrentApplication($current_application);
}

return array($controller, $uri_data);
}

private function buildSiteForRequest(AphrontRequest $request) {
Expand All @@ -469,6 +419,8 @@ private function buildSiteForRequest(AphrontRequest $request) {
$host));
}

$request->setSite($site);

return $site;
}

Expand Down

0 comments on commit bcc5e55

Please sign in to comment.