Skip to content

fix(Router): Load attribute routes of all apps when not app is specified - #47508

Merged
provokateurin merged 1 commit into
masterfrom
fix/router/attribute-routes-all-apps
Aug 27, 2024
Merged

fix(Router): Load attribute routes of all apps when not app is specified#47508
provokateurin merged 1 commit into
masterfrom
fix/router/attribute-routes-all-apps

Conversation

@provokateurin

@provokateurinprovokateurin commented Aug 27, 2024

Copy link
Copy Markdown
Member

Summary

Found while debugging #47479 (also necessary for the fix to it).

The problem was that when loadRoutes(null) was called the attribute routes should have been loaded for all apps since no app was specified. It only loaded the traditional routes.php files and then continued to set loaded = true. Any future call would return early, even if an app was specified because everything must be loaded already.
This logic now matches the loading process of routes.php which it should have done from the start.
Unfortunately back then I didn't find this problem and it seems to only trigger in specific places where the app is not known (e.g. when loading the /index.php path where the server has to load the routes first and then has to figure out which app should be used).

Checklist

@provokateurinprovokateurin added this to the Nextcloud 31 milestone Aug 27, 2024
@provokateurin
provokateurin requested review from a team, Altahrim, nfebe, nickvergessen, susnux and yemkareems and removed request for a teamAugust 27, 2024 07:07
@provokateurin

Copy link
Copy Markdown
MemberAuthor

/backport to stable30

@provokateurin

Copy link
Copy Markdown
MemberAuthor

/backport to stable29

Comment threadlib/private/Route/Router.php Outdated
@provokateurin
provokateurinforce-pushed the fix/router/attribute-routes-all-apps branch from 42e604e to 554b7a9CompareAugust 27, 2024 07:13
Comment threadlib/private/Route/Router.php Outdated
Signed-off-by: provokateurin <kate@provokateurin.de>
@provokateurin
provokateurinforce-pushed the fix/router/attribute-routes-all-apps branch from 554b7a9 to 12b4484CompareAugust 27, 2024 08:43
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@provokateurin@nickvergessen@artonge@come-nc