From 723b8764d111fd414cf2e37ae06e0b9a29c9dff7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Mon, 5 Mar 2018 12:19:20 +0100 Subject: [PATCH 1/3] Add ETag to NavigationController MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- core/Controller/NavigationController.php | 18 +++++++++-- .../Controller/NavigationControllerTest.php | 31 +++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/core/Controller/NavigationController.php b/core/Controller/NavigationController.php index 3521fac3b46b5..b178cb97cd50a 100644 --- a/core/Controller/NavigationController.php +++ b/core/Controller/NavigationController.php @@ -22,6 +22,7 @@ */ namespace OC\Core\Controller; +use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\OCSController; use OCP\INavigationManager; @@ -54,7 +55,14 @@ public function getAppsNavigation(bool $absolute = false): DataResponse { if ($absolute) { $navigation = $this->rewriteToAbsoluteUrls($navigation); } - return new DataResponse($navigation); + + $etag = md5(json_encode($navigation)); + if ($this->request->getHeader('If-None-Match') === $etag) { + return new DataResponse([], Http::STATUS_NOT_MODIFIED); + } + $response = new DataResponse($navigation); + $response->setEtag($etag); + return $response; } /** @@ -69,7 +77,13 @@ public function getSettingsNavigation(bool $absolute = false): DataResponse { if ($absolute) { $navigation = $this->rewriteToAbsoluteUrls($navigation); } - return new DataResponse($navigation); + $etag = md5(json_encode($navigation)); + if ($this->request->getHeader('If-None-Match') === $etag) { + return new DataResponse([], Http::STATUS_NOT_MODIFIED); + } + $response = new DataResponse($navigation); + $response->setEtag($etag); + return $response; } /** diff --git a/tests/Core/Controller/NavigationControllerTest.php b/tests/Core/Controller/NavigationControllerTest.php index 1143ed003f01f..ef4720604fb07 100644 --- a/tests/Core/Controller/NavigationControllerTest.php +++ b/tests/Core/Controller/NavigationControllerTest.php @@ -23,6 +23,7 @@ namespace Tests\Core\Controller; use OC\Core\Controller\NavigationController; +use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; use OCP\INavigationManager; use OCP\IRequest; @@ -126,4 +127,34 @@ public function testGetSettingsNavigation($absolute) { } } + public function testGetAppNavigationEtagMatch() { + $navigation = [ ['id' => 'files', 'href' => '/index.php/apps/files', 'icon' => 'icon' ] ]; + $this->request->expects($this->once()) + ->method('getHeader') + ->with('If-None-Match') + ->willReturn(md5(json_encode($navigation))); + $this->navigationManager->expects($this->once()) + ->method('getAll') + ->with('link') + ->willReturn($navigation); + $actual = $this->controller->getAppsNavigation(); + $this->assertInstanceOf(DataResponse::class, $actual); + $this->assertEquals(Http::STATUS_NOT_MODIFIED, $actual->getStatus()); + } + + public function testGetSettingsNavigationEtagMatch() { + $navigation = [ ['id' => 'files', 'href' => '/index.php/apps/files', 'icon' => 'icon' ] ]; + $this->request->expects($this->once()) + ->method('getHeader') + ->with('If-None-Match') + ->willReturn(md5(json_encode($navigation))); + $this->navigationManager->expects($this->once()) + ->method('getAll') + ->with('settings') + ->willReturn($navigation); + $actual = $this->controller->getSettingsNavigation(); + $this->assertInstanceOf(DataResponse::class, $actual); + $this->assertEquals(Http::STATUS_NOT_MODIFIED, $actual->getStatus()); + } + } From 11b6cc3f68f55e1c5e725b737b39c0a7f79e0ec1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Mon, 5 Mar 2018 16:22:18 +0100 Subject: [PATCH 2/3] Replace logout href to avoid new etag on every request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- core/Controller/NavigationController.php | 23 +++++++++++++++---- .../Controller/NavigationControllerTest.php | 6 ++--- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/core/Controller/NavigationController.php b/core/Controller/NavigationController.php index b178cb97cd50a..2397fb3c7b4d9 100644 --- a/core/Controller/NavigationController.php +++ b/core/Controller/NavigationController.php @@ -56,12 +56,12 @@ public function getAppsNavigation(bool $absolute = false): DataResponse { $navigation = $this->rewriteToAbsoluteUrls($navigation); } - $etag = md5(json_encode($navigation)); + $etag = $this->generateETag($navigation); if ($this->request->getHeader('If-None-Match') === $etag) { return new DataResponse([], Http::STATUS_NOT_MODIFIED); } $response = new DataResponse($navigation); - $response->setEtag($etag); + $response->setETag($etag); return $response; } @@ -77,15 +77,30 @@ public function getSettingsNavigation(bool $absolute = false): DataResponse { if ($absolute) { $navigation = $this->rewriteToAbsoluteUrls($navigation); } - $etag = md5(json_encode($navigation)); + $etag = $this->generateETag($navigation); if ($this->request->getHeader('If-None-Match') === $etag) { return new DataResponse([], Http::STATUS_NOT_MODIFIED); } $response = new DataResponse($navigation); - $response->setEtag($etag); + $response->setETag($etag); return $response; } + /** + * Generate an ETag for a list of navigation entries + * + * @param array $navigation + * @return string + */ + 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 * diff --git a/tests/Core/Controller/NavigationControllerTest.php b/tests/Core/Controller/NavigationControllerTest.php index ef4720604fb07..a33ddf7dd23da 100644 --- a/tests/Core/Controller/NavigationControllerTest.php +++ b/tests/Core/Controller/NavigationControllerTest.php @@ -132,7 +132,7 @@ public function testGetAppNavigationEtagMatch() { $this->request->expects($this->once()) ->method('getHeader') ->with('If-None-Match') - ->willReturn(md5(json_encode($navigation))); + ->willReturn(md5(json_encode(['files']))); $this->navigationManager->expects($this->once()) ->method('getAll') ->with('link') @@ -143,11 +143,11 @@ public function testGetAppNavigationEtagMatch() { } public function testGetSettingsNavigationEtagMatch() { - $navigation = [ ['id' => 'files', 'href' => '/index.php/apps/files', 'icon' => 'icon' ] ]; + $navigation = [ ['id' => 'logout', 'href' => '/index.php/apps/files', 'icon' => 'icon' ] ]; $this->request->expects($this->once()) ->method('getHeader') ->with('If-None-Match') - ->willReturn(md5(json_encode($navigation))); + ->willReturn(md5(json_encode([ ['id' => 'logout', 'href' => 'logout', 'icon' => 'icon' ] ]))); $this->navigationManager->expects($this->once()) ->method('getAll') ->with('settings') From 16ac8eaac9779f9ab297ab99fa5211e37563002c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Wed, 7 Mar 2018 09:17:18 +0100 Subject: [PATCH 3/3] Fix tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- tests/Core/Controller/NavigationControllerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Core/Controller/NavigationControllerTest.php b/tests/Core/Controller/NavigationControllerTest.php index a33ddf7dd23da..86173405c1c05 100644 --- a/tests/Core/Controller/NavigationControllerTest.php +++ b/tests/Core/Controller/NavigationControllerTest.php @@ -132,7 +132,7 @@ public function testGetAppNavigationEtagMatch() { $this->request->expects($this->once()) ->method('getHeader') ->with('If-None-Match') - ->willReturn(md5(json_encode(['files']))); + ->willReturn(md5(json_encode($navigation))); $this->navigationManager->expects($this->once()) ->method('getAll') ->with('link')