Only load app routes if the app has already been loaded #18109
Conversation
|
A new inspection was created. |
|
🚀 Test PASSed.🚀 |
| if (!\OC_App::isAppLoaded($app)) { | ||
| // app MUST be loaded before app routes | ||
| // try again next time loadRoutes() is called | ||
| $this->loaded = false; |
There was a problem hiding this comment.
hmmm ... this might have a negative impact on perf? @icewind1991
There was a problem hiding this comment.
This variable shortcuts loading all routes, but even when set to false each set of routes (an individual app) doesn't get reloaded due to the check on line 152: if (!isset($this->loadedApps[$app])) {
There was a problem hiding this comment.
While this helps loading the normal routes, it does not help loading ocs routes of apps.
I will make a second PR fixing this.
|
@bartv2 @icewind1991 Please review, this fixes a regression |
|
👍 looks good |
|
👍 |
Only load app routes if the app has already been loaded
|
This problem has manifested itself again... Displaying the upgrade page results in the Router trying to load all app routes, even though no apps are loaded during the upgrade. This leads to a white screen when an upgrade is required: #15914 (comment) @DeepDiver1975 @icewind1991 @schiesbn We have two options - re-merge this patch, or put a dirty |
|
Or revert this https://github.com/owncloud/core/pull/17961/files#diff-66338aea9c5877c3368c5c7b0ed484ebR69 and not call |
|
@PVince81 That was the original problem, fixed in #18202. This is more fundemental, and has been an issue in ownCloud for a while now. It's amazing it hasn't blown up yet - apparently according to @LukasReschke it has a few times. Basically, somewhere in the upgrade code path, a Router is needed, which tries to load app routes, regardless of whether the app is loaded or not yet. |
Fixes #18095
If an app isn't loaded when the router tries to load routes, it marks itself as 'not loaded', so the next time a route is requested any non-loaded routes attempt to get loaded again.
cc @schiesbn @LukasReschke @PVince81 @DeepDiver1975