Inspection of "Merge pull request #52546 from nextcloud/fix/52278..." - nextcloud/server - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( a67720...be1982 )

by Daniel

created 2025-04-30 20:28 UTC

Status

Indentation +109 added lines, -109 removed lines patch added patch discarded remove patch

@@ -14,124 +14,124 @@
 block discarded – undo
 use Test\TestCase;
 
 class NavigationControllerTest extends TestCase {
-	/** @var IRequest|\PHPUnit\Framework\MockObject\MockObject */
-	private $request;
+    /** @var IRequest|\PHPUnit\Framework\MockObject\MockObject */
+    private $request;
 
-	/** @var INavigationManager|\PHPUnit\Framework\MockObject\MockObject */
-	private $navigationManager;
+    /** @var INavigationManager|\PHPUnit\Framework\MockObject\MockObject */
+    private $navigationManager;
 
-	/** @var IURLGenerator|\PHPUnit\Framework\MockObject\MockObject */
-	private $urlGenerator;
+    /** @var IURLGenerator|\PHPUnit\Framework\MockObject\MockObject */
+    private $urlGenerator;
 
-	/** @var NavigationController */
-	private $controller;
+    /** @var NavigationController */
+    private $controller;
 
-	protected function setUp(): void {
-		parent::setUp();
+    protected function setUp(): void {
+        parent::setUp();
 
-		$this->request = $this->createMock(IRequest::class);
-		$this->navigationManager = $this->createMock(INavigationManager::class);
-		$this->urlGenerator = $this->createMock(IURLGenerator::class);
+        $this->request = $this->createMock(IRequest::class);
+        $this->navigationManager = $this->createMock(INavigationManager::class);
+        $this->urlGenerator = $this->createMock(IURLGenerator::class);
 
-		$this->controller = new NavigationController(
-			'core',
-			$this->request,
-			$this->navigationManager,
-			$this->urlGenerator
-		);
-	}
+        $this->controller = new NavigationController(
+            'core',
+            $this->request,
+            $this->navigationManager,
+            $this->urlGenerator
+        );
+    }
 
-	public static function dataGetNavigation(): array {
-		return [
-			[false],
-			[true],
-		];
-	}
-	/** @dataProvider dataGetNavigation */
-	public function testGetAppNavigation(bool $absolute): void {
-		$this->navigationManager->expects($this->once())
-			->method('getAll')
-			->with('link')
-			->willReturn(['files' => ['id' => 'files', 'href' => '/index.php/apps/files', 'icon' => 'icon' ] ]);
-		if ($absolute) {
-			$this->urlGenerator->expects($this->any())
-				->method('getBaseURL')
-				->willReturn('http://localhost/');
-			$this->urlGenerator->expects($this->exactly(2))
-				->method('getAbsoluteURL')
-				->willReturnMap([
-					['/index.php/apps/files', 'http://localhost/index.php/apps/files'],
-					['icon', 'http://localhost/icon'],
-				]);
-			$actual = $this->controller->getAppsNavigation($absolute);
-			$this->assertInstanceOf(DataResponse::class, $actual);
-			$this->assertEquals('http://localhost/index.php/apps/files', $actual->getData()[0]['href']);
-			$this->assertEquals('http://localhost/icon', $actual->getData()[0]['icon']);
-		} else {
-			$actual = $this->controller->getAppsNavigation($absolute);
-			$this->assertInstanceOf(DataResponse::class, $actual);
-			$this->assertEquals('/index.php/apps/files', $actual->getData()[0]['href']);
-			$this->assertEquals('icon', $actual->getData()[0]['icon']);
-		}
-	}
+    public static function dataGetNavigation(): array {
+        return [
+            [false],
+            [true],
+        ];
+    }
+    /** @dataProvider dataGetNavigation */
+    public function testGetAppNavigation(bool $absolute): void {
+        $this->navigationManager->expects($this->once())
+            ->method('getAll')
+            ->with('link')
+            ->willReturn(['files' => ['id' => 'files', 'href' => '/index.php/apps/files', 'icon' => 'icon' ] ]);
+        if ($absolute) {
+            $this->urlGenerator->expects($this->any())
+                ->method('getBaseURL')
+                ->willReturn('http://localhost/');
+            $this->urlGenerator->expects($this->exactly(2))
+                ->method('getAbsoluteURL')
+                ->willReturnMap([
+                    ['/index.php/apps/files', 'http://localhost/index.php/apps/files'],
+                    ['icon', 'http://localhost/icon'],
+                ]);
+            $actual = $this->controller->getAppsNavigation($absolute);
+            $this->assertInstanceOf(DataResponse::class, $actual);
+            $this->assertEquals('http://localhost/index.php/apps/files', $actual->getData()[0]['href']);
+            $this->assertEquals('http://localhost/icon', $actual->getData()[0]['icon']);
+        } else {
+            $actual = $this->controller->getAppsNavigation($absolute);
+            $this->assertInstanceOf(DataResponse::class, $actual);
+            $this->assertEquals('/index.php/apps/files', $actual->getData()[0]['href']);
+            $this->assertEquals('icon', $actual->getData()[0]['icon']);
+        }
+    }
 
-	/** @dataProvider dataGetNavigation */
-	public function testGetSettingsNavigation(bool $absolute): void {
-		$this->navigationManager->expects($this->once())
-			->method('getAll')
-			->with('settings')
-			->willReturn(['settings' => ['id' => 'settings', 'href' => '/index.php/settings/user', 'icon' => '/core/img/settings.svg'] ]);
-		if ($absolute) {
-			$this->urlGenerator->expects($this->any())
-				->method('getBaseURL')
-				->willReturn('http://localhost/');
-			$this->urlGenerator->expects($this->exactly(2))
-				->method('getAbsoluteURL')
-				->willReturnMap([
-					['/index.php/settings/user', 'http://localhost/index.php/settings/user'],
-					['/core/img/settings.svg', 'http://localhost/core/img/settings.svg']
-				]);
-			$actual = $this->controller->getSettingsNavigation($absolute);
-			$this->assertInstanceOf(DataResponse::class, $actual);
-			$this->assertEquals('http://localhost/index.php/settings/user', $actual->getData()[0]['href']);
-			$this->assertEquals('http://localhost/core/img/settings.svg', $actual->getData()[0]['icon']);
-		} else {
-			$actual = $this->controller->getSettingsNavigation($absolute);
-			$this->assertInstanceOf(DataResponse::class, $actual);
-			$this->assertEquals('/index.php/settings/user', $actual->getData()[0]['href']);
-			$this->assertEquals('/core/img/settings.svg', $actual->getData()[0]['icon']);
-		}
-	}
+    /** @dataProvider dataGetNavigation */
+    public function testGetSettingsNavigation(bool $absolute): void {
+        $this->navigationManager->expects($this->once())
+            ->method('getAll')
+            ->with('settings')
+            ->willReturn(['settings' => ['id' => 'settings', 'href' => '/index.php/settings/user', 'icon' => '/core/img/settings.svg'] ]);
+        if ($absolute) {
+            $this->urlGenerator->expects($this->any())
+                ->method('getBaseURL')
+                ->willReturn('http://localhost/');
+            $this->urlGenerator->expects($this->exactly(2))
+                ->method('getAbsoluteURL')
+                ->willReturnMap([
+                    ['/index.php/settings/user', 'http://localhost/index.php/settings/user'],
+                    ['/core/img/settings.svg', 'http://localhost/core/img/settings.svg']
+                ]);
+            $actual = $this->controller->getSettingsNavigation($absolute);
+            $this->assertInstanceOf(DataResponse::class, $actual);
+            $this->assertEquals('http://localhost/index.php/settings/user', $actual->getData()[0]['href']);
+            $this->assertEquals('http://localhost/core/img/settings.svg', $actual->getData()[0]['icon']);
+        } else {
+            $actual = $this->controller->getSettingsNavigation($absolute);
+            $this->assertInstanceOf(DataResponse::class, $actual);
+            $this->assertEquals('/index.php/settings/user', $actual->getData()[0]['href']);
+            $this->assertEquals('/core/img/settings.svg', $actual->getData()[0]['icon']);
+        }
+    }
 
-	public function testEtagIgnoresLogout(): void {
-		$navigation1 = [
-			['id' => 'files', 'href' => '/index.php/apps/files', 'icon' => 'icon' ],
-			['id' => 'logout', 'href' => '/index.php/logout?requesttoken=abcd', 'icon' => 'icon' ],
-		];
-		$navigation2 = [
-			['id' => 'files', 'href' => '/index.php/apps/files', 'icon' => 'icon' ],
-			['id' => 'logout', 'href' => '/index.php/logout?requesttoken=1234', 'icon' => 'icon' ],
-		];
-		$navigation3 = [
-			['id' => 'files', 'href' => '/index.php/apps/files/test', 'icon' => 'icon' ],
-			['id' => 'logout', 'href' => '/index.php/logout?requesttoken=1234', 'icon' => 'icon' ],
-		];
-		$this->navigationManager->expects($this->exactly(3))
-			->method('getAll')
-			->with('link')
-			->willReturnOnConsecutiveCalls(
-				$navigation1,
-				$navigation2,
-				$navigation3,
-			);
+    public function testEtagIgnoresLogout(): void {
+        $navigation1 = [
+            ['id' => 'files', 'href' => '/index.php/apps/files', 'icon' => 'icon' ],
+            ['id' => 'logout', 'href' => '/index.php/logout?requesttoken=abcd', 'icon' => 'icon' ],
+        ];
+        $navigation2 = [
+            ['id' => 'files', 'href' => '/index.php/apps/files', 'icon' => 'icon' ],
+            ['id' => 'logout', 'href' => '/index.php/logout?requesttoken=1234', 'icon' => 'icon' ],
+        ];
+        $navigation3 = [
+            ['id' => 'files', 'href' => '/index.php/apps/files/test', 'icon' => 'icon' ],
+            ['id' => 'logout', 'href' => '/index.php/logout?requesttoken=1234', 'icon' => 'icon' ],
+        ];
+        $this->navigationManager->expects($this->exactly(3))
+            ->method('getAll')
+            ->with('link')
+            ->willReturnOnConsecutiveCalls(
+                $navigation1,
+                $navigation2,
+                $navigation3,
+            );
 
-		// Changes in the logout url should not change the ETag
-		$request1 = $this->controller->getAppsNavigation();
-		$request2 = $this->controller->getAppsNavigation();
-		$this->assertEquals($request1->getETag(), $request2->getETag());
+        // Changes in the logout url should not change the ETag
+        $request1 = $this->controller->getAppsNavigation();
+        $request2 = $this->controller->getAppsNavigation();
+        $this->assertEquals($request1->getETag(), $request2->getETag());
 
-		// Changes in non-logout urls should result in a different ETag
-		$request3 = $this->controller->getAppsNavigation();
-		$this->assertNotEquals($request2->getETag(), $request3->getETag());
-	}
+        // Changes in non-logout urls should result in a different ETag
+        $request3 = $this->controller->getAppsNavigation();
+        $this->assertNotEquals($request2->getETag(), $request3->getETag());
+    }
 }

Please login to merge, or discard this patch.

core/Controller/NavigationController.php 1 patch

Indentation +78 added lines, -78 removed lines patch added patch discarded remove patch

@@ -20,86 +20,86 @@
 block discarded – undo
  * @psalm-import-type CoreNavigationEntry from ResponseDefinitions
  */
 class NavigationController extends OCSController {
-	public function __construct(
-		string $appName,
-		IRequest $request,
-		private INavigationManager $navigationManager,
-		private IURLGenerator $urlGenerator,
-	) {
-		parent::__construct($appName, $request);
-	}
+    public function __construct(
+        string $appName,
+        IRequest $request,
+        private INavigationManager $navigationManager,
+        private IURLGenerator $urlGenerator,
+    ) {
+        parent::__construct($appName, $request);
+    }
 
-	/**
-	 * Get the apps navigation
-	 *
-	 * @param bool $absolute Rewrite URLs to absolute ones
-	 * @return DataResponse<Http::STATUS_OK, list<CoreNavigationEntry>, array{}>|DataResponse<Http::STATUS_NOT_MODIFIED, list<empty>, array{}>
-	 *
-	 * 200: Apps navigation returned
-	 * 304: No apps navigation changed
-	 */
-	#[NoAdminRequired]
-	#[NoCSRFRequired]
-	#[ApiRoute(verb: 'GET', url: '/navigation/apps', root: '/core')]
-	public function getAppsNavigation(bool $absolute = false): DataResponse {
-		$navigation = $this->navigationManager->getAll();
-		if ($absolute) {
-			$navigation = $this->rewriteToAbsoluteUrls($navigation);
-		}
-		$navigation = array_values($navigation);
-		$response = new DataResponse($navigation);
-		$response->setETag($this->generateETag($navigation));
-		return $response;
-	}
+    /**
+     * Get the apps navigation
+     *
+     * @param bool $absolute Rewrite URLs to absolute ones
+     * @return DataResponse<Http::STATUS_OK, list<CoreNavigationEntry>, array{}>|DataResponse<Http::STATUS_NOT_MODIFIED, list<empty>, array{}>
+     *
+     * 200: Apps navigation returned
+     * 304: No apps navigation changed
+     */
+    #[NoAdminRequired]
+    #[NoCSRFRequired]
+    #[ApiRoute(verb: 'GET', url: '/navigation/apps', root: '/core')]
+    public function getAppsNavigation(bool $absolute = false): DataResponse {
+        $navigation = $this->navigationManager->getAll();
+        if ($absolute) {
+            $navigation = $this->rewriteToAbsoluteUrls($navigation);
+        }
+        $navigation = array_values($navigation);
+        $response = new DataResponse($navigation);
+        $response->setETag($this->generateETag($navigation));
+        return $response;
+    }
 
-	/**
-	 * Get the settings navigation
-	 *
-	 * @param bool $absolute Rewrite URLs to absolute ones
-	 * @return DataResponse<Http::STATUS_OK, list<CoreNavigationEntry>, array{}>|DataResponse<Http::STATUS_NOT_MODIFIED, list<empty>, array{}>
-	 *
-	 * 200: Apps navigation returned
-	 * 304: No apps navigation changed
-	 */
-	#[NoAdminRequired]
-	#[NoCSRFRequired]
-	#[ApiRoute(verb: 'GET', url: '/navigation/settings', root: '/core')]
-	public function getSettingsNavigation(bool $absolute = false): DataResponse {
-		$navigation = $this->navigationManager->getAll('settings');
-		if ($absolute) {
-			$navigation = $this->rewriteToAbsoluteUrls($navigation);
-		}
-		$navigation = array_values($navigation);
-		$response = new DataResponse($navigation);
-		$response->setETag($this->generateETag($navigation));
-		return $response;
-	}
+    /**
+     * Get the settings navigation
+     *
+     * @param bool $absolute Rewrite URLs to absolute ones
+     * @return DataResponse<Http::STATUS_OK, list<CoreNavigationEntry>, array{}>|DataResponse<Http::STATUS_NOT_MODIFIED, list<empty>, array{}>
+     *
+     * 200: Apps navigation returned
+     * 304: No apps navigation changed
+     */
+    #[NoAdminRequired]
+    #[NoCSRFRequired]
+    #[ApiRoute(verb: 'GET', url: '/navigation/settings', root: '/core')]
+    public function getSettingsNavigation(bool $absolute = false): DataResponse {
+        $navigation = $this->navigationManager->getAll('settings');
+        if ($absolute) {
+            $navigation = $this->rewriteToAbsoluteUrls($navigation);
+        }
+        $navigation = array_values($navigation);
+        $response = new DataResponse($navigation);
+        $response->setETag($this->generateETag($navigation));
+        return $response;
+    }
 
-	/**
-	 * Generate an ETag for a list of navigation entries
-	 */
-	private function generateETag(array $navigation): string {
-		foreach ($navigation as &$nav) {
-			if ($nav['id'] === 'logout') {
-				$nav['href'] = 'logout';
-			}
-		}
-		return md5(json_encode($navigation));
-	}
+    /**
+     * Generate an ETag for a list of navigation entries
+     */
+    private function generateETag(array $navigation): string {
+        foreach ($navigation as &$nav) {
+            if ($nav['id'] === 'logout') {
+                $nav['href'] = 'logout';
+            }
+        }
+        return md5(json_encode($navigation));
+    }
 
-	/**
-	 * Rewrite href attribute of navigation entries to an absolute URL
-	 */
-	private function rewriteToAbsoluteUrls(array $navigation): array {
-		foreach ($navigation as &$entry) {
-			/* If parse_url finds no host it means the URL is not absolute */
-			if (!isset(\parse_url($entry['href'])['host'])) {
-				$entry['href'] = $this->urlGenerator->getAbsoluteURL($entry['href']);
-			}
-			if (!str_starts_with($entry['icon'], $this->urlGenerator->getBaseUrl())) {
-				$entry['icon'] = $this->urlGenerator->getAbsoluteURL($entry['icon']);
-			}
-		}
-		return $navigation;
-	}
+    /**
+     * Rewrite href attribute of navigation entries to an absolute URL
+     */
+    private function rewriteToAbsoluteUrls(array $navigation): array {
+        foreach ($navigation as &$entry) {
+            /* If parse_url finds no host it means the URL is not absolute */
+            if (!isset(\parse_url($entry['href'])['host'])) {
+                $entry['href'] = $this->urlGenerator->getAbsoluteURL($entry['href']);
+            }
+            if (!str_starts_with($entry['icon'], $this->urlGenerator->getBaseUrl())) {
+                $entry['icon'] = $this->urlGenerator->getAbsoluteURL($entry['icon']);
+            }
+        }
+        return $navigation;
+    }
 }

Please login to merge, or discard this patch.

		@@ -14,124 +14,124 @@
		block discarded – undo
14	14	use Test\TestCase;
15	15
16	16	class NavigationControllerTest extends TestCase {
17		- /** @var IRequest\|\PHPUnit\Framework\MockObject\MockObject */
18		- private $request;
	17	+ /** @var IRequest\|\PHPUnit\Framework\MockObject\MockObject */
	18	+ private $request;
19	19
20		- /** @var INavigationManager\|\PHPUnit\Framework\MockObject\MockObject */
21		- private $navigationManager;
	20	+ /** @var INavigationManager\|\PHPUnit\Framework\MockObject\MockObject */
	21	+ private $navigationManager;
22	22
23		- /** @var IURLGenerator\|\PHPUnit\Framework\MockObject\MockObject */
24		- private $urlGenerator;
	23	+ /** @var IURLGenerator\|\PHPUnit\Framework\MockObject\MockObject */
	24	+ private $urlGenerator;
25	25
26		- /** @var NavigationController */
27		- private $controller;
	26	+ /** @var NavigationController */
	27	+ private $controller;
28	28
29		- protected function setUp(): void {
30		- parent::setUp();
	29	+ protected function setUp(): void {
	30	+ parent::setUp();
31	31
32		- $this->request = $this->createMock(IRequest::class);
33		- $this->navigationManager = $this->createMock(INavigationManager::class);
34		- $this->urlGenerator = $this->createMock(IURLGenerator::class);
	32	+ $this->request = $this->createMock(IRequest::class);
	33	+ $this->navigationManager = $this->createMock(INavigationManager::class);
	34	+ $this->urlGenerator = $this->createMock(IURLGenerator::class);
35	35
36		- $this->controller = new NavigationController(
37		- 'core',
38		- $this->request,
39		- $this->navigationManager,
40		- $this->urlGenerator
41		- );
42		- }
	36	+ $this->controller = new NavigationController(
	37	+ 'core',
	38	+ $this->request,
	39	+ $this->navigationManager,
	40	+ $this->urlGenerator
	41	+ );
	42	+ }
43	43
44		- public static function dataGetNavigation(): array {
45		- return [
46		- [false],
47		- [true],
48		- ];
49		- }
50		- /** @dataProvider dataGetNavigation */
51		- public function testGetAppNavigation(bool $absolute): void {
52		- $this->navigationManager->expects($this->once())
53		- ->method('getAll')
54		- ->with('link')
55		- ->willReturn(['files' => ['id' => 'files', 'href' => '/index.php/apps/files', 'icon' => 'icon' ] ]);
56		- if ($absolute) {
57		- $this->urlGenerator->expects($this->any())
58		- ->method('getBaseURL')
59		- ->willReturn('http://localhost/');
60		- $this->urlGenerator->expects($this->exactly(2))
61		- ->method('getAbsoluteURL')
62		- ->willReturnMap([
63		- ['/index.php/apps/files', 'http://localhost/index.php/apps/files'],
64		- ['icon', 'http://localhost/icon'],
65		- ]);
66		- $actual = $this->controller->getAppsNavigation($absolute);
67		- $this->assertInstanceOf(DataResponse::class, $actual);
68		- $this->assertEquals('http://localhost/index.php/apps/files', $actual->getData()[0]['href']);
69		- $this->assertEquals('http://localhost/icon', $actual->getData()[0]['icon']);
70		- } else {
71		- $actual = $this->controller->getAppsNavigation($absolute);
72		- $this->assertInstanceOf(DataResponse::class, $actual);
73		- $this->assertEquals('/index.php/apps/files', $actual->getData()[0]['href']);
74		- $this->assertEquals('icon', $actual->getData()[0]['icon']);
75		- }
76		- }
	44	+ public static function dataGetNavigation(): array {
	45	+ return [
	46	+ [false],
	47	+ [true],
	48	+ ];
	49	+ }
	50	+ /** @dataProvider dataGetNavigation */
	51	+ public function testGetAppNavigation(bool $absolute): void {
	52	+ $this->navigationManager->expects($this->once())
	53	+ ->method('getAll')
	54	+ ->with('link')
	55	+ ->willReturn(['files' => ['id' => 'files', 'href' => '/index.php/apps/files', 'icon' => 'icon' ] ]);
	56	+ if ($absolute) {
	57	+ $this->urlGenerator->expects($this->any())
	58	+ ->method('getBaseURL')
	59	+ ->willReturn('http://localhost/');
	60	+ $this->urlGenerator->expects($this->exactly(2))
	61	+ ->method('getAbsoluteURL')
	62	+ ->willReturnMap([
	63	+ ['/index.php/apps/files', 'http://localhost/index.php/apps/files'],
	64	+ ['icon', 'http://localhost/icon'],
	65	+ ]);
	66	+ $actual = $this->controller->getAppsNavigation($absolute);
	67	+ $this->assertInstanceOf(DataResponse::class, $actual);
	68	+ $this->assertEquals('http://localhost/index.php/apps/files', $actual->getData()[0]['href']);
	69	+ $this->assertEquals('http://localhost/icon', $actual->getData()[0]['icon']);
	70	+ } else {
	71	+ $actual = $this->controller->getAppsNavigation($absolute);
	72	+ $this->assertInstanceOf(DataResponse::class, $actual);
	73	+ $this->assertEquals('/index.php/apps/files', $actual->getData()[0]['href']);
	74	+ $this->assertEquals('icon', $actual->getData()[0]['icon']);
	75	+ }
	76	+ }
77	77
78		- /** @dataProvider dataGetNavigation */
79		- public function testGetSettingsNavigation(bool $absolute): void {
80		- $this->navigationManager->expects($this->once())
81		- ->method('getAll')
82		- ->with('settings')
83		- ->willReturn(['settings' => ['id' => 'settings', 'href' => '/index.php/settings/user', 'icon' => '/core/img/settings.svg'] ]);
84		- if ($absolute) {
85		- $this->urlGenerator->expects($this->any())
86		- ->method('getBaseURL')
87		- ->willReturn('http://localhost/');
88		- $this->urlGenerator->expects($this->exactly(2))
89		- ->method('getAbsoluteURL')
90		- ->willReturnMap([
91		- ['/index.php/settings/user', 'http://localhost/index.php/settings/user'],
92		- ['/core/img/settings.svg', 'http://localhost/core/img/settings.svg']
93		- ]);
94		- $actual = $this->controller->getSettingsNavigation($absolute);
95		- $this->assertInstanceOf(DataResponse::class, $actual);
96		- $this->assertEquals('http://localhost/index.php/settings/user', $actual->getData()[0]['href']);
97		- $this->assertEquals('http://localhost/core/img/settings.svg', $actual->getData()[0]['icon']);
98		- } else {
99		- $actual = $this->controller->getSettingsNavigation($absolute);
100		- $this->assertInstanceOf(DataResponse::class, $actual);
101		- $this->assertEquals('/index.php/settings/user', $actual->getData()[0]['href']);
102		- $this->assertEquals('/core/img/settings.svg', $actual->getData()[0]['icon']);
103		- }
104		- }
	78	+ /** @dataProvider dataGetNavigation */
	79	+ public function testGetSettingsNavigation(bool $absolute): void {
	80	+ $this->navigationManager->expects($this->once())
	81	+ ->method('getAll')
	82	+ ->with('settings')
	83	+ ->willReturn(['settings' => ['id' => 'settings', 'href' => '/index.php/settings/user', 'icon' => '/core/img/settings.svg'] ]);
	84	+ if ($absolute) {
	85	+ $this->urlGenerator->expects($this->any())
	86	+ ->method('getBaseURL')
	87	+ ->willReturn('http://localhost/');
	88	+ $this->urlGenerator->expects($this->exactly(2))
	89	+ ->method('getAbsoluteURL')
	90	+ ->willReturnMap([
	91	+ ['/index.php/settings/user', 'http://localhost/index.php/settings/user'],
	92	+ ['/core/img/settings.svg', 'http://localhost/core/img/settings.svg']
	93	+ ]);
	94	+ $actual = $this->controller->getSettingsNavigation($absolute);
	95	+ $this->assertInstanceOf(DataResponse::class, $actual);
	96	+ $this->assertEquals('http://localhost/index.php/settings/user', $actual->getData()[0]['href']);
	97	+ $this->assertEquals('http://localhost/core/img/settings.svg', $actual->getData()[0]['icon']);
	98	+ } else {
	99	+ $actual = $this->controller->getSettingsNavigation($absolute);
	100	+ $this->assertInstanceOf(DataResponse::class, $actual);
	101	+ $this->assertEquals('/index.php/settings/user', $actual->getData()[0]['href']);
	102	+ $this->assertEquals('/core/img/settings.svg', $actual->getData()[0]['icon']);
	103	+ }
	104	+ }
105	105
106		- public function testEtagIgnoresLogout(): void {
107		- $navigation1 = [
108		- ['id' => 'files', 'href' => '/index.php/apps/files', 'icon' => 'icon' ],
109		- ['id' => 'logout', 'href' => '/index.php/logout?requesttoken=abcd', 'icon' => 'icon' ],
110		- ];
111		- $navigation2 = [
112		- ['id' => 'files', 'href' => '/index.php/apps/files', 'icon' => 'icon' ],
113		- ['id' => 'logout', 'href' => '/index.php/logout?requesttoken=1234', 'icon' => 'icon' ],
114		- ];
115		- $navigation3 = [
116		- ['id' => 'files', 'href' => '/index.php/apps/files/test', 'icon' => 'icon' ],
117		- ['id' => 'logout', 'href' => '/index.php/logout?requesttoken=1234', 'icon' => 'icon' ],
118		- ];
119		- $this->navigationManager->expects($this->exactly(3))
120		- ->method('getAll')
121		- ->with('link')
122		- ->willReturnOnConsecutiveCalls(
123		- $navigation1,
124		- $navigation2,
125		- $navigation3,
126		- );
	106	+ public function testEtagIgnoresLogout(): void {
	107	+ $navigation1 = [
	108	+ ['id' => 'files', 'href' => '/index.php/apps/files', 'icon' => 'icon' ],
	109	+ ['id' => 'logout', 'href' => '/index.php/logout?requesttoken=abcd', 'icon' => 'icon' ],
	110	+ ];
	111	+ $navigation2 = [
	112	+ ['id' => 'files', 'href' => '/index.php/apps/files', 'icon' => 'icon' ],
	113	+ ['id' => 'logout', 'href' => '/index.php/logout?requesttoken=1234', 'icon' => 'icon' ],
	114	+ ];
	115	+ $navigation3 = [
	116	+ ['id' => 'files', 'href' => '/index.php/apps/files/test', 'icon' => 'icon' ],
	117	+ ['id' => 'logout', 'href' => '/index.php/logout?requesttoken=1234', 'icon' => 'icon' ],
	118	+ ];
	119	+ $this->navigationManager->expects($this->exactly(3))
	120	+ ->method('getAll')
	121	+ ->with('link')
	122	+ ->willReturnOnConsecutiveCalls(
	123	+ $navigation1,
	124	+ $navigation2,
	125	+ $navigation3,
	126	+ );
127	127
128		- // Changes in the logout url should not change the ETag
129		- $request1 = $this->controller->getAppsNavigation();
130		- $request2 = $this->controller->getAppsNavigation();
131		- $this->assertEquals($request1->getETag(), $request2->getETag());
	128	+ // Changes in the logout url should not change the ETag
	129	+ $request1 = $this->controller->getAppsNavigation();
	130	+ $request2 = $this->controller->getAppsNavigation();
	131	+ $this->assertEquals($request1->getETag(), $request2->getETag());
132	132
133		- // Changes in non-logout urls should result in a different ETag
134		- $request3 = $this->controller->getAppsNavigation();
135		- $this->assertNotEquals($request2->getETag(), $request3->getETag());
136		- }
	133	+ // Changes in non-logout urls should result in a different ETag
	134	+ $request3 = $this->controller->getAppsNavigation();
	135	+ $this->assertNotEquals($request2->getETag(), $request3->getETag());
	136	+ }
137	137	}

		@@ -20,86 +20,86 @@
		block discarded – undo
20	20	* @psalm-import-type CoreNavigationEntry from ResponseDefinitions
21	21	*/
22	22	class NavigationController extends OCSController {
23		- public function __construct(
24		- string $appName,
25		- IRequest $request,
26		- private INavigationManager $navigationManager,
27		- private IURLGenerator $urlGenerator,
28		- ) {
29		- parent::__construct($appName, $request);
30		- }
	23	+ public function __construct(
	24	+ string $appName,
	25	+ IRequest $request,
	26	+ private INavigationManager $navigationManager,
	27	+ private IURLGenerator $urlGenerator,
	28	+ ) {
	29	+ parent::__construct($appName, $request);
	30	+ }
31	31
32		- /**
33		- * Get the apps navigation
34		- *
35		- * @param bool $absolute Rewrite URLs to absolute ones
36		- * @return DataResponse<Http::STATUS_OK, list<CoreNavigationEntry>, array{}>\|DataResponse<Http::STATUS_NOT_MODIFIED, list<empty>, array{}>
37		- *
38		- * 200: Apps navigation returned
39		- * 304: No apps navigation changed
40		- */
41		- #[NoAdminRequired]
42		- #[NoCSRFRequired]
43		- #[ApiRoute(verb: 'GET', url: '/navigation/apps', root: '/core')]
44		- public function getAppsNavigation(bool $absolute = false): DataResponse {
45		- $navigation = $this->navigationManager->getAll();
46		- if ($absolute) {
47		- $navigation = $this->rewriteToAbsoluteUrls($navigation);
48		- }
49		- $navigation = array_values($navigation);
50		- $response = new DataResponse($navigation);
51		- $response->setETag($this->generateETag($navigation));
52		- return $response;
53		- }
	32	+ /**
	33	+ * Get the apps navigation
	34	+ *
	35	+ * @param bool $absolute Rewrite URLs to absolute ones
	36	+ * @return DataResponse<Http::STATUS_OK, list<CoreNavigationEntry>, array{}>\|DataResponse<Http::STATUS_NOT_MODIFIED, list<empty>, array{}>
	37	+ *
	38	+ * 200: Apps navigation returned
	39	+ * 304: No apps navigation changed
	40	+ */
	41	+ #[NoAdminRequired]
	42	+ #[NoCSRFRequired]
	43	+ #[ApiRoute(verb: 'GET', url: '/navigation/apps', root: '/core')]
	44	+ public function getAppsNavigation(bool $absolute = false): DataResponse {
	45	+ $navigation = $this->navigationManager->getAll();
	46	+ if ($absolute) {
	47	+ $navigation = $this->rewriteToAbsoluteUrls($navigation);
	48	+ }
	49	+ $navigation = array_values($navigation);
	50	+ $response = new DataResponse($navigation);
	51	+ $response->setETag($this->generateETag($navigation));
	52	+ return $response;
	53	+ }
54	54
55		- /**
56		- * Get the settings navigation
57		- *
58		- * @param bool $absolute Rewrite URLs to absolute ones
59		- * @return DataResponse<Http::STATUS_OK, list<CoreNavigationEntry>, array{}>\|DataResponse<Http::STATUS_NOT_MODIFIED, list<empty>, array{}>
60		- *
61		- * 200: Apps navigation returned
62		- * 304: No apps navigation changed
63		- */
64		- #[NoAdminRequired]
65		- #[NoCSRFRequired]
66		- #[ApiRoute(verb: 'GET', url: '/navigation/settings', root: '/core')]
67		- public function getSettingsNavigation(bool $absolute = false): DataResponse {
68		- $navigation = $this->navigationManager->getAll('settings');
69		- if ($absolute) {
70		- $navigation = $this->rewriteToAbsoluteUrls($navigation);
71		- }
72		- $navigation = array_values($navigation);
73		- $response = new DataResponse($navigation);
74		- $response->setETag($this->generateETag($navigation));
75		- return $response;
76		- }
	55	+ /**
	56	+ * Get the settings navigation
	57	+ *
	58	+ * @param bool $absolute Rewrite URLs to absolute ones
	59	+ * @return DataResponse<Http::STATUS_OK, list<CoreNavigationEntry>, array{}>\|DataResponse<Http::STATUS_NOT_MODIFIED, list<empty>, array{}>
	60	+ *
	61	+ * 200: Apps navigation returned
	62	+ * 304: No apps navigation changed
	63	+ */
	64	+ #[NoAdminRequired]
	65	+ #[NoCSRFRequired]
	66	+ #[ApiRoute(verb: 'GET', url: '/navigation/settings', root: '/core')]
	67	+ public function getSettingsNavigation(bool $absolute = false): DataResponse {
	68	+ $navigation = $this->navigationManager->getAll('settings');
	69	+ if ($absolute) {
	70	+ $navigation = $this->rewriteToAbsoluteUrls($navigation);
	71	+ }
	72	+ $navigation = array_values($navigation);
	73	+ $response = new DataResponse($navigation);
	74	+ $response->setETag($this->generateETag($navigation));
	75	+ return $response;
	76	+ }
77	77
78		- /**
79		- * Generate an ETag for a list of navigation entries
80		- */
81		- private function generateETag(array $navigation): string {
82		- foreach ($navigation as &$nav) {
83		- if ($nav['id'] === 'logout') {
84		- $nav['href'] = 'logout';
85		- }
86		- }
87		- return md5(json_encode($navigation));
88		- }
	78	+ /**
	79	+ * Generate an ETag for a list of navigation entries
	80	+ */
	81	+ private function generateETag(array $navigation): string {
	82	+ foreach ($navigation as &$nav) {
	83	+ if ($nav['id'] === 'logout') {
	84	+ $nav['href'] = 'logout';
	85	+ }
	86	+ }
	87	+ return md5(json_encode($navigation));
	88	+ }
89	89
90		- /**
91		- * Rewrite href attribute of navigation entries to an absolute URL
92		- */
93		- private function rewriteToAbsoluteUrls(array $navigation): array {
94		- foreach ($navigation as &$entry) {
95		- /* If parse_url finds no host it means the URL is not absolute */
96		- if (!isset(\parse_url($entry['href'])['host'])) {
97		- $entry['href'] = $this->urlGenerator->getAbsoluteURL($entry['href']);
98		- }
99		- if (!str_starts_with($entry['icon'], $this->urlGenerator->getBaseUrl())) {
100		- $entry['icon'] = $this->urlGenerator->getAbsoluteURL($entry['icon']);
101		- }
102		- }
103		- return $navigation;
104		- }
	90	+ /**
	91	+ * Rewrite href attribute of navigation entries to an absolute URL
	92	+ */
	93	+ private function rewriteToAbsoluteUrls(array $navigation): array {
	94	+ foreach ($navigation as &$entry) {
	95	+ /* If parse_url finds no host it means the URL is not absolute */
	96	+ if (!isset(\parse_url($entry['href'])['host'])) {
	97	+ $entry['href'] = $this->urlGenerator->getAbsoluteURL($entry['href']);
	98	+ }
	99	+ if (!str_starts_with($entry['icon'], $this->urlGenerator->getBaseUrl())) {
	100	+ $entry['icon'] = $this->urlGenerator->getAbsoluteURL($entry['icon']);
	101	+ }
	102	+ }
	103	+ return $navigation;
	104	+ }
105	105	}

nextcloud / server

Push — master ( a67720...be1982 )

Status

Category

Indentation +109 added lines, -109 removed lines patch added patch discarded remove patch

Indentation +78 added lines, -78 removed lines patch added patch discarded remove patch