open-dsi
/
nextcloud-server
mirror of https://github.com/nextcloud/server
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

fix(NavigationManager): resolve closure and event entries when all apps loaded

Browse Source 
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
pull/61120/head
Ferdinand Thiessen 1 month ago
parent dab318ff69
commit bd7fc29d57
No known key found for this signature in database
GPG Key ID: 7E849AE05218500F
3 changed files with 161 additions and 16 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 3
      lib/OC.php
  2. 43
      lib/private/NavigationManager.php
  3. 131
      tests/lib/NavigationManagerTest.php

3
lib/OC.php
Unescape View File

@ -7,6 +7,7 @@ declare(strict_types=1);
* SPDX-License-Identifier: AGPL-3.0-only
*/
use OC\NavigationManager;
use OC\Profiler\BuiltInProfiler;
use OC\Security\CSP\ContentSecurityPolicyNonceManager;
use OC\Share20\GroupDeletedListener;
@ -1203,6 +1204,8 @@ class OC {
}
// All apps are now loaded to handle the request
Server::get(NavigationManager::class)->setup();
// if we are not on CLI, try to match the request to a route and handle it
if (!self::$CLI) {
try {

43
lib/private/NavigationManager.php
Unescape View File

@ -166,10 +166,14 @@ class NavigationManager implements INavigationManager {
/**
* removes all the entries
*/
public function clear(bool $loadDefaultLinks = true): void {
public function clear(bool $resetInit = true): void {
$this->entries = [];
$this->closureEntries = [];
$this->init = !$loadDefaultLinks;
if ($resetInit) {
$this->loadedAppInfo = [];
$this->init = false;
}
}
#[Override]
@ -203,20 +207,25 @@ class NavigationManager implements INavigationManager {
}
/**
* Resolve the app navigation entries from closures and info.xml files.
* Setup the navigation manager.
* @internal - This is only used by Nextcloud core to setup the navigation manager. It is not intended for use by apps.
*/
private function resolveAppNavigationEntries(): void {
public function setup(): void {
// 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());
}
$this->eventDispatcher->dispatchTyped(new LoadAdditionalEntriesEvent());
}
// Resolve classic info.xml based navigation entries
/**
* Resolve classic info.xml based navigation entires.
* Some code relies on this to be available earlier then the app loading finished.
* So we need to resolve the navigation entries here, even if not all apps are loaded yet.
*/
private function resolveAppNavigationEntries(): void {
if ($this->userSession->isLoggedIn()) {
$user = $this->userSession->getUser();
$apps = $this->appManager->getEnabledAppsForUser($user);
@ -225,16 +234,23 @@ class NavigationManager implements INavigationManager {
}
foreach ($apps as $app) {
// skip already loaded apps
if (in_array($app, $this->loadedAppInfo)) {
if (in_array($app, $this->loadedAppInfo, true)) {
// already loaded
continue;
}
if (!$this->appManager->isAppLoaded($app)) {
// app is not loaded yet, skip it
continue;
}
// 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 @@ class NavigationManager implements INavigationManager {
}
$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 {

131
tests/lib/NavigationManagerTest.php
Unescape View File

@ -157,6 +157,7 @@ class NavigationManagerTest extends TestCase {
$this->assertEquals(0, $testAddClosureNumberOfCalls, 'Expected that the closure is not called by add()');
$this->navigationManager->setup();
$navigationEntries = $this->navigationManager->getAll('all');
$this->assertEquals(1, $testAddClosureNumberOfCalls, 'Expected that the closure is called by getAll()');
$this->assertCount(1, $navigationEntries, 'Expected that 1 navigation entry exists');
@ -233,6 +234,12 @@ class NavigationManagerTest extends TestCase {
->method('getAppInfo')
->with('test')
->willReturn($navigation);
$this->appManager->expects($this->any())
->method('isAppLoaded')
->willReturnMap([
['test', true],
['files', true],
]);
$this->urlGenerator->expects($this->any())
->method('imagePath')
->willReturnCallback(function ($appName, $file) {
@ -259,11 +266,12 @@ class NavigationManagerTest extends TestCase {
$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);
});
$this->navigationManager->setup();
$entries = $this->navigationManager->getAll('all');
$this->assertEquals($expected, $entries);
}
@ -427,8 +435,20 @@ class NavigationManagerTest extends TestCase {
->method('isEnabledForUser')
->with('theming')
->willReturn(true);
$this->appManager->expects($this->once())->method('getAppInfo')->with('test')->willReturn($navigation);
$this->appManager->expects($this->once())->method('getAppIcon')->with('test')->willReturn('/apps/test/img/app.svg');
$this->appManager->expects($this->once())
->method('getAppIcon')
->with('test')
->willReturn('/apps/test/img/app.svg');
$this->appManager->expects($this->once())
->method('getAppInfo')
->with('test')
->willReturn($navigation);
$this->appManager->expects($this->atLeastOnce())
->method('isAppLoaded')
->willReturnMap([
['test', true],
['files', true],
]);
$this->l10nFac->expects($this->any())->method('get')->willReturn($l);
$this->urlGenerator->expects($this->any())->method('imagePath')->willReturnCallback(function ($appName, $file) {
return "/apps/$appName/img/$file";
@ -455,10 +475,106 @@ class NavigationManagerTest extends TestCase {
->willReturnCallback(function ($event): void {
$this->assertInstanceOf(LoadAdditionalEntriesEvent::class, $event);
});
$this->navigationManager->setup();
$entries = $this->navigationManager->getAll();
$this->assertEquals($expected, $entries);
}
/**
* Navigation entries of enabled apps that are not booted yet must not be resolved.
*/
public function testResolveOnlyLoadedApps(): void {
/* Return default value */
$this->config->method('getUserValue')->willReturnArgument(3);
$user = $this->createMock(IUser::class);
$user->method('getUID')->willReturn('user001');
$this->userSession->method('getUser')->willReturn($user);
$this->userSession->method('isLoggedIn')->willReturn(true);
$this->appManager->method('getEnabledAppsForUser')->with($user)->willReturn(['test']);
// The app is enabled but not booted yet ...
$this->appManager->expects($this->atLeastOnce())
->method('isAppLoaded')
->with('test')
->willReturn(false);
// ... so its info.xml navigation entries must never be read
$this->appManager->expects($this->never())->method('getAppInfo');
$this->navigationManager->clear();
$this->assertEquals([], $this->navigationManager->getAll('all'));
}
/**
* The LoadAdditionalEntriesEvent is only dispatched by setup(), not by getAll().
*/
public function testGetAllDoesNotDispatchAdditionalEntries(): void {
$this->userSession->method('isLoggedIn')->willReturn(false);
$this->appManager->method('getEnabledApps')->willReturn([]);
$this->dispatcher->expects($this->never())->method('dispatchTyped');
$this->navigationManager->clear();
$this->assertEquals([], $this->navigationManager->getAll('all'));
}
/**
* An app's info.xml must only be resolved once, even across multiple getAll() calls
* and even when the app does not provide any navigation entries.
*/
public function testAppInfoResolvedOnlyOnce(): void {
$this->config->method('getUserValue')->willReturnArgument(3);
$user = $this->createMock(IUser::class);
$user->method('getUID')->willReturn('user001');
$this->userSession->method('getUser')->willReturn($user);
$this->userSession->method('isLoggedIn')->willReturn(true);
$this->appManager->method('getEnabledAppsForUser')->with($user)->willReturn(['test']);
$this->appManager->method('isAppLoaded')->with('test')->willReturn(true);
// App has no navigation entries; info.xml must only be read once
$this->appManager->expects($this->once())
->method('getAppInfo')
->with('test')
->willReturn(['navigations' => []]);
$this->navigationManager->clear();
$this->assertEquals([], $this->navigationManager->getAll('all'));
$this->assertEquals([], $this->navigationManager->getAll('all'));
}
/**
* clear(false) keeps the resolved state, so already loaded apps are not resolved again;
* clear(true) resets it, forcing a fresh resolve.
*/
public function testClearResetsResolvedStateOnlyWhenRequested(): void {
$this->config->method('getUserValue')->willReturnArgument(3);
$user = $this->createMock(IUser::class);
$user->method('getUID')->willReturn('user001');
$this->userSession->method('getUser')->willReturn($user);
$this->userSession->method('isLoggedIn')->willReturn(true);
$this->appManager->method('getEnabledAppsForUser')->with($user)->willReturn(['test']);
$this->appManager->method('isAppLoaded')->with('test')->willReturn(true);
// Resolved once for the initial getAll(), then again after clear(true) resets the state
$this->appManager->expects($this->exactly(2))
->method('getAppInfo')
->with('test')
->willReturn(['navigations' => []]);
$this->navigationManager->clear();
$this->assertEquals([], $this->navigationManager->getAll('all'));
// Soft clear keeps the resolved state, so getAppInfo is not called again
$this->navigationManager->clear(false);
$this->assertEquals([], $this->navigationManager->getAll('all'));
// Full clear resets the resolved state, so the app is resolved again
$this->navigationManager->clear(true);
$this->assertEquals([], $this->navigationManager->getAll('all'));
}
public static function provideDefaultEntries(): array {
return [
// none specified, default to files
@ -630,7 +746,13 @@ class NavigationManagerTest extends TestCase {
];
});
$this->appManager->method('getEnabledApps')->willReturn([]);
$this->appManager->method('getEnabledApps')->willReturn(['files']);
$this->appManager->expects($this->atLeastOnce())
->method('isAppLoaded')
->willReturnMap([
['test', true],
['files', true],
]);
$user = $this->createMock(IUser::class);
$user->method('getUID')->willReturn('user1');
@ -651,6 +773,7 @@ class NavigationManagerTest extends TestCase {
['user1', 'core', 'apporder', '[]', $userApporder],
]);
$this->navigationManager->setup();
$this->assertEquals($expectedApp, $this->navigationManager->getDefaultEntryIdForUser(null, $withFallbacks));
}

Write Preview
Loading…
Cancel
Save