Skip to content

Make sure urlparams are correctly injected in global routes #14626

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
MorrisJobke merged 2 commits into from
Mar 12, 2019
Merged

Make sure urlparams are correctly injected in global routes #14626

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4e8b033
Make sure urlParams are correctly injected in global routes
nickvergessen Mar 11, 2019
a3c9e5b
Add a message about disabled global routes
nickvergessen Mar 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Make sure urlParams are correctly injected in global routes 
Signed-off-by: Joas Schilling <[email protected]>
  • Loading branch information
@nickvergessen
nickvergessen committed Mar 11, 2019
commit 4e8b033281a1ecb4bd1fd8651cd91630022e9d38
101 changes: 11 additions & 90 deletions core/routes.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,17 @@
['name' => 'contactsMenu#findOne', 'url' => '/contactsmenu/findOne', 'verb' => 'POST'],
['name' => 'WalledGarden#get', 'url' => '/204', 'verb' => 'GET'],
['name' => 'Search#search', 'url' => '/core/search', 'verb' => 'GET'],

// Legacy routes that need to be globally available while they are handled by an app
['name' => 'viewcontroller#showFile', 'url' => '/f/{fileid}', 'verb' => 'GET', 'app' => 'files'],
Copy link
Member Author

@nickvergessen nickvergessen Mar 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should also be possible, to move those to the respective apps, when at the same time we allow defining a root' => '/cloud' similar to OCS urls.
Just not sure how this breaks existing route generations (because all lowercase instead of CamelCase controller names, which is why the hacks in lib/private/AppFramework/Routing/RouteConfig.php were necessary)

['name' => 'sharecontroller#showShare', 'url' => '/s/{token}', 'verb' => 'GET', 'app' => 'files_sharing'],
['name' => 'sharecontroller#showAuthenticate', 'url' => '/s/{token}/authenticate/{redirect}', 'verb' => 'GET', 'app' => 'files_sharing'],
['name' => 'sharecontroller#authenticate', 'url' => '/s/{token}/authenticate/{redirect}', 'verb' => 'POST', 'app' => 'files_sharing'],
['name' => 'sharecontroller#downloadShare', 'url' => '/s/{token}/download', 'verb' => 'GET', 'app' => 'files_sharing'],
['name' => 'publicpreview#directLink', 'url' => '/s/{token}/preview', 'verb' => 'GET', 'app' => 'files_sharing'],
['name' => 'requesthandlercontroller#addShare', 'url' => '/ocm/shares', 'verb' => 'POST', 'app' => 'cloud_federation_api'],
['name' => 'requesthandlercontroller#receiveNotification', 'url' => '/ocm/notifications', 'verb' => 'POST', 'app' => 'cloud_federation_api'],
['name' => 'pagecontroller#showCall', 'url' => '/call/{token}', 'verb' => 'GET', 'app' => 'spreed'],
],
'ocs' => [
['root' => '/cloud', 'name' => 'OCS#getCapabilities', 'url' => '/capabilities', 'verb' => 'GET'],
Expand Down Expand Up @@ -109,93 +120,3 @@
// Routing
$this->create('core_ajax_update', '/core/ajax/update.php')
->actionInclude('core/ajax/update.php');

// File routes
$this->create('files.viewcontroller.showFile', '/f/{fileid}')->action(function($urlParams) {
$app = new \OCA\Files\AppInfo\Application($urlParams);
$app->dispatch('ViewController', 'index');
});

// Call routes
/**
* @suppress PhanUndeclaredClassConstant
* @suppress PhanUndeclaredClassMethod
*/
$this->create('spreed.pagecontroller.showCall', '/call/{token}')->action(function($urlParams) {
if (class_exists(\OCA\Spreed\AppInfo\Application::class, false)) {
$app = new \OCA\Spreed\AppInfo\Application($urlParams);
$app->dispatch('PageController', 'index');
} else {
throw new \OC\HintException('App spreed is not enabled');
}
});

// OCM routes
/**
* @suppress PhanUndeclaredClassConstant
* @suppress PhanUndeclaredClassMethod
*/
$this->create('cloud_federation_api.requesthandlercontroller.addShare', '/ocm/shares')->post()->action(function($urlParams) {
if (class_exists(\OCA\CloudFederationAPI\AppInfo\Application::class, false)) {
$app = new \OCA\CloudFederationAPI\AppInfo\Application($urlParams);
$app->dispatch('RequestHandlerController', 'addShare');
} else {
throw new \OC\HintException('Cloud Federation API not enabled');
}
});

/**
* @suppress PhanUndeclaredClassConstant
* @suppress PhanUndeclaredClassMethod
*/
$this->create('cloud_federation_api.requesthandlercontroller.receiveNotification', '/ocm/notifications')->post()->action(function($urlParams) {
if (class_exists(\OCA\CloudFederationAPI\AppInfo\Application::class, false)) {
$app = new \OCA\CloudFederationAPI\AppInfo\Application($urlParams);
$app->dispatch('RequestHandlerController', 'receiveNotification');
} else {
throw new \OC\HintException('Cloud Federation API not enabled');
}
});


// Sharing routes
$this->create('files_sharing.sharecontroller.showShare', '/s/{token}')->action(function($urlParams) {
if (class_exists(\OCA\Files_Sharing\AppInfo\Application::class, false)) {
$app = new \OCA\Files_Sharing\AppInfo\Application($urlParams);
$app->dispatch('ShareController', 'showShare');
} else {
throw new \OC\HintException('App file sharing is not enabled');
}
});
$this->create('files_sharing.sharecontroller.authenticate', '/s/{token}/authenticate/{redirect}')->post()->action(function($urlParams) {
if (class_exists(\OCA\Files_Sharing\AppInfo\Application::class, false)) {
$app = new \OCA\Files_Sharing\AppInfo\Application($urlParams);
$app->dispatch('ShareController', 'authenticate');
} else {
throw new \OC\HintException('App file sharing is not enabled');
}
});
$this->create('files_sharing.sharecontroller.showAuthenticate', '/s/{token}/authenticate/{redirect}')->get()->action(function($urlParams) {
if (class_exists(\OCA\Files_Sharing\AppInfo\Application::class, false)) {
$app = new \OCA\Files_Sharing\AppInfo\Application($urlParams);
$app->dispatch('ShareController', 'showAuthenticate');
} else {
throw new \OC\HintException('App file sharing is not enabled');
}
});
$this->create('files_sharing.sharecontroller.downloadShare', '/s/{token}/download')->get()->action(function($urlParams) {
if (class_exists(\OCA\Files_Sharing\AppInfo\Application::class, false)) {
$app = new \OCA\Files_Sharing\AppInfo\Application($urlParams);
$app->dispatch('ShareController', 'downloadShare');
} else {
throw new \OC\HintException('App file sharing is not enabled');
}
});
$this->create('files_sharing.publicpreview.directLink', '/s/{token}/preview')->get()->action(function($urlParams) {
if (class_exists(\OCA\Files_Sharing\AppInfo\Application::class, false)) {
$app = new \OCA\Files_Sharing\AppInfo\Application($urlParams);
$app->dispatch('PublicPreviewController', 'directLink');
} else {
throw new \OC\HintException('App file sharing is not enabled');
}
});
16 changes: 15 additions & 1 deletion lib/private/AppFramework/Routing/RouteConfig.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
namespace OC\AppFramework\Routing;

use OC\AppFramework\DependencyInjection\DIContainer;
use OCP\AppFramework\App;
use OCP\Route\IRouter;

/**
Expand Down Expand Up @@ -155,8 +156,21 @@ private function processSimpleRoutes(array $routes): void {

$controllerName = $this->buildControllerName($controller);
$actionName = $this->buildActionName($action);
$appName = $simpleRoute['app'] ?? $this->appName;

if (isset($simpleRoute['app'])) {
// Legacy routes that need to be globally available while they are handled by an app
// E.g. '/f/{id}', '/s/{token}', '/call/{token}', …
$controllerName = str_replace('controllerController', 'Controller', $controllerName);
if ($controllerName === 'PublicpreviewController') {
$controllerName = 'PublicPreviewController';
} else if ($controllerName === 'RequesthandlerController') {
$controllerName = 'RequestHandlerController';
}
$controllerName = App::buildAppNamespace($appName) . '\\Controller\\' . $controllerName;
}

$routeName = $this->appName . '.' . $controller . '.' . $action . $postfix;
$routeName = $appName . '.' . $controller . '.' . $action . $postfix;

// register the route
$handler = new RouteActionHandler($this->container, $controllerName, $actionName);
Expand Down