fix(NavigationManager): only resolve navigations of booted apps

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

Author: susnux

lib/private/NavigationManager.php

@@ -39,6 +39,7 @@ class NavigationManager implements INavigationManager {
 	private ?array $customAppOrder = null;
 	/** List of loaded app info */
 	private array $loadedAppInfo = [];
+	private bool $additionalEntriesLoaded = false;
 
 	public function __construct(
 		protected IAppManager $appManager,
@@ -206,35 +207,50 @@ private function init(): void {
 	 * Resolve the app navigation entries from closures and info.xml files.
 	 */
 	private function resolveAppNavigationEntries(): void {
-		// Resolve app navigation closures
-		while ($c = array_pop($this->closureEntries)) {
-			$this->add($c());
-		}
+		$this->resolveAppInfoEntries();
+
+		// we do not really know the current bootstrapping state
+		// but we know that the files app is always enabled and loaded when "filesystem" is loaded thus the server is ready or close-to-ready.
+		if ($this->appManager->isAppLoaded('files')) {
+			// Resolve app navigation closures
+			while ($c = array_pop($this->closureEntries)) {
+				$this->add($c());
+			}
 
-		// Resolve dynamically added navigation entries via event listeners
-		if ($this->loadedAppInfo === []) {
-			$this->eventDispatcher->dispatchTyped(new LoadAdditionalEntriesEvent());
+			// Resolve dynamically added navigation entries via event listeners
+			if (!$this->additionalEntriesLoaded) {
+				$this->additionalEntriesLoaded = true;
+				$this->eventDispatcher->dispatchTyped(new LoadAdditionalEntriesEvent());
+			}
 		}
+	}
 
-		// Resolve classic info.xml based navigation entries
+	/**
+	 * Resolve classic info.xml based navigation entires
+	 */
+	private function resolveAppInfoEntries(): void {
 		if ($this->userSession->isLoggedIn()) {
 			$user = $this->userSession->getUser();
 			$apps = $this->appManager->getEnabledAppsForUser($user);
 		} else {
 			$apps = $this->appManager->getEnabledApps();
 		}
 
-		foreach ($apps as $app) {
-			// skip already loaded apps
-			if (in_array($app, $this->loadedAppInfo)) {
-				continue;
-			}
+		$appsToLoad = array_diff($apps, $this->loadedAppInfo);
+		$appsToLoad = array_filter($appsToLoad, $this->appManager->isAppLoaded(...));
+		if ($appsToLoad === []) {
+			return;
+		}
 
+		foreach ($appsToLoad as $app) {
 			// load plugins and collections from info.xml
 			$info = $this->appManager->getAppInfo($app);
 			if (!isset($info['navigations']['navigation'])) {
+				// this app does not have any navigation entries, skip it
+				$this->loadedAppInfo[] = $app;
 				continue;
 			}
+
 			foreach ($info['navigations']['navigation'] as $key => $nav) {
 				$nav['type'] = $nav['type'] ?? 'link';
 				if (!isset($nav['name'])) {
@@ -250,8 +266,11 @@ private function resolveAppNavigationEntries(): void {
 				}
 				$id = $nav['id'] ?? $app . ($key === 0 ? '' : $key);
 				$order = $nav['order'] ?? 100;
-				$type = $nav['type'];
-				$route = !empty($nav['route']) ? $this->urlGenerator->linkToRoute($nav['route']) : '';
+				$type = $nav['type'] ?? 'link';
+				$route = $nav['route'] ?? '';
+				if ($route !== '') {
+					$route = $this->urlGenerator->linkToRoute($route);
+				}
 				$icon = $nav['icon'] ?? null;
 				if ($icon !== null) {
 					try {

tests/lib/NavigationManagerTest.php

@@ -233,6 +233,10 @@ public function testWithAppManager($expected, $navigation, $isAdmin = false): vo
 			->method('getAppInfo')
 			->with('test')
 			->willReturn($navigation);
+		$this->appManager->expects($this->any())
+			->method('isAppLoaded')
+			->with('test')
+			->willReturn(true);
 		$this->urlGenerator->expects($this->any())
 			->method('imagePath')
 			->willReturnCallback(function ($appName, $file) {
@@ -259,7 +263,7 @@ public function testWithAppManager($expected, $navigation, $isAdmin = false): vo
 		$this->groupManager->expects($this->any())->method('isAdmin')->willReturn($isAdmin);
 
 		$this->navigationManager->clear();
-		$this->dispatcher->expects($this->once())
+		$this->dispatcher->expects($this->atLeastOnce())
 			->method('dispatchTyped')
 			->willReturnCallback(function ($event): void {
 				$this->assertInstanceOf(LoadAdditionalEntriesEvent::class, $event);
@@ -428,6 +432,7 @@ function (string $userId, string $appName, string $key, mixed $default = '') use
 			->with('theming')
 			->willReturn(true);
 		$this->appManager->expects($this->once())->method('getAppInfo')->with('test')->willReturn($navigation);
+		$this->appManager->expects($this->any())->method('isAppLoaded')->with('test')->willReturn(true);
 		$this->appManager->expects($this->once())->method('getAppIcon')->with('test')->willReturn('/apps/test/img/app.svg');
 		$this->l10nFac->expects($this->any())->method('get')->willReturn($l);
 		$this->urlGenerator->expects($this->any())->method('imagePath')->willReturnCallback(function ($appName, $file) {