Skip to content

feat: add OCC command to list all routes (incl. fixing the Router) - #52919

Closed
susnux wants to merge 5 commits into
masterfrom
feat/router-list-routs-cmd
Closed

feat: add OCC command to list all routes (incl. fixing the Router)#52919
susnux wants to merge 5 commits into
masterfrom
feat/router-list-routs-cmd

Conversation

@susnux

Copy link
Copy Markdown
Contributor

Summary

Main idea was to add a command to list all registered routes, to make the annotation more usable.
But when doing so I noticed the Router is broken as retrieving individual collections of routes was broken,
so this also contains a fix for it:

  • Add routes to individual collections for each app
    and then merge them for the root collection holding all routes for the
    URL generator.
  • Short cut route loading with already loaded apps
  • Simplify active collection handling
  • Remove deprecated OC_App usage
  • Fully type Router methods

Checklist

@susnuxsusnux added this to the Nextcloud 32 milestone May 16, 2025
@susnux
susnuxforce-pushed the feat/router-list-routs-cmd branch from 1886595 to 6669894CompareMay 17, 2025 10:16
protected function configure(): void {
parent::configure();
$this
->setName('router:list')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

#[AsCommand(
name: 'router:list',
description: 'List registered routes',
)]

nit

$rows = $this->formatRoutes($allRoutes);
$this->writeTableInOutputFormat($input, $output, $rows);
}
return 0;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
return0;
returnself::SUCCESS;

Comment threadlib/private/Route/Router.php Outdated
Comment on lines +83 to +85
$apps = $user === null
? $this->appManager->getEnabledApps()
: $this->appManager->getEnabledAppsForUser($user);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
$apps = $user === null
? $this->appManager->getEnabledApps()
: $this->appManager->getEnabledAppsForUser($user);
if ($user === null) {
$apps = $this->appManager->getEnabledApps();
} else {
$apps = $this->appManager->getEnabledAppsForUser($user);
}

I would like to suggest using a normal if-else statement here instead of the ternary operator for better readability.

@come-nccome-nc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When testing this branch (which will need a rebase after #52793 is merged), I do not see the ajax routes of user_ldap in the output of occ router:list.
So either the command is missing them or you broke their loading in the router.
And I do not see anything special in the command code, apart from the wrong assumption that all routes have a caller default.

@susnux

Copy link
Copy Markdown
ContributorAuthor

And I do not see anything special in the command code, apart from the wrong assumption that all routes have a caller default.

It is set by the parser?

$defaults['caller'] = [$appName, $controllerName, $actionName];

susnux added 4 commits June 5, 2025 21:45
- Add routes to individual collections for each app
and then merge them for the root collection holding all routes for the
URL generator.
- Short cut route loading with already loaded apps
- Simplify active collection handling
- Remove deprecated `OC_App` usage
- Fully type Router methods
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux
susnuxforce-pushed the feat/router-list-routs-cmd branch from 4f41e49 to 129983fCompareJune 5, 2025 19:51
@susnux

Copy link
Copy Markdown
ContributorAuthor

So either the command is missing them or you broke their loading in the router.

Yes TBH thought we removed all places not returning an array from routes.php in #42678 but seems like I missed that we still have the IRouter (deprecated since 9!) that exposes the create method.

Probably revert this here :)

@come-nc

Copy link
Copy Markdown
Contributor

Yeah, I had the issue with the cached routes PR, there are still routes returned by routes.php and relying on a file include.
Also there is the possibility to use a callable, but I did not find any use of that.

Yes IRouter::create is deprecated, but as it’s not obvious that it’s what is used in routes.php tooling will not warn application developpers about it.

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc

Copy link
Copy Markdown
Contributor

With the tiny fix I pushed, it now works for old school routes:

$ occ router:list user_ldap
+-----------+-------------------------+------------------------------+--------+-----------------------------------------------------+----------------------------------------------------------+--------------+
| app | controller | function | method | path | defaults | requirements |
+-----------+-------------------------+------------------------------+--------+-----------------------------------------------------+----------------------------------------------------------+--------------+
| | | | | /apps/user_ldap/ajax/clearMappings.php | {"file":"user_ldap\/ajax\/clearMappings.php"} | |
| | | | | /apps/user_ldap/ajax/deleteConfiguration.php | {"file":"user_ldap\/ajax\/deleteConfiguration.php"} | |
| | | | | /apps/user_ldap/ajax/getConfiguration.php | {"file":"user_ldap\/ajax\/getConfiguration.php"} | |
| | | | | /apps/user_ldap/ajax/getNewServerConfigPrefix.php | {"file":"user_ldap\/ajax\/getNewServerConfigPrefix.php"} | |
| | | | | /apps/user_ldap/ajax/setConfiguration.php | {"file":"user_ldap\/ajax\/setConfiguration.php"} | |
| | | | | /apps/user_ldap/ajax/testConfiguration.php | {"file":"user_ldap\/ajax\/testConfiguration.php"} | |
| | | | | /apps/user_ldap/ajax/wizard.php | {"file":"user_ldap\/ajax\/wizard.php"} | |
| user_ldap | RenewPasswordController | tryRenewPassword | POST | /apps/user_ldap/renewpassword | | |
| user_ldap | RenewPasswordController | showRenewPasswordForm | GET | /apps/user_ldap/renewpassword/{user} | | |
| user_ldap | RenewPasswordController | cancel | GET | /apps/user_ldap/renewpassword/cancel | | |
| user_ldap | RenewPasswordController | showLoginFormInvalidPassword | GET | /apps/user_ldap/renewpassword/invalidlogin/{user} | | |
| user_ldap | ConfigAPIController | getConfig | GET | /ocs/v2.php/ocs/v2.php/config | | |
| user_ldap | ConfigAPIController | getCapabilities | GET | /ocs/v2.php/ocs/v2.php/cloud/capabilities | | |
| user_ldap | ConfigAPIController | personCheck | POST | /ocs/v2.php/ocs/v2.php/person/check | | |
| user_ldap | ConfigAPIController | getIdentityProof | GET | /ocs/v2.php/ocs/v2.php/identityproof/key/{cloudId} | | |
| user_ldap | ConfigAPIController | create | POST | /ocs/v2.php/apps/user_ldap/api/v1/config | | |
| user_ldap | ConfigAPIController | show | GET | /ocs/v2.php/apps/user_ldap/api/v1/config/{configID} | | |
| user_ldap | ConfigAPIController | modify | PUT | /ocs/v2.php/apps/user_ldap/api/v1/config/{configID} | | |
| user_ldap | ConfigAPIController | delete | DELETE | /ocs/v2.php/apps/user_ldap/api/v1/config/{configID} | | |
+-----------+-------------------------+------------------------------+--------+-----------------------------------------------------+----------------------------------------------------------+--------------+

The main weird thing is the empty application. It should just use the application id it nows about.

@kesselb

Copy link
Copy Markdown
Contributor

A command, to list routes, was added by: #53669

What about the fix for the router?

@susnux

Copy link
Copy Markdown
ContributorAuthor

#53669

@susnuxsusnux closed this Jan 14, 2026
@kesselb
kesselb deleted the feat/router-list-routs-cmd branch January 14, 2026 14:08
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2. developingWork in progressenhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@susnux@come-nc@kesselb@skjnldsv