Merge pull request #8652 from nextcloud/navigation-controller-etag

Add ETag to NavigationController

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 @@ class NavigationController extends OCSController {
 		if ($absolute) {
 			$navigation = $this->rewriteToAbsoluteUrls($navigation);
 		}
-		return new DataResponse($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);
+		return $response;
 	}
 
 	/**
@@ -69,7 +77,28 @@ class NavigationController extends OCSController {
 		if ($absolute) {
 			$navigation = $this->rewriteToAbsoluteUrls($navigation);
 		}
-		return new DataResponse($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);
+		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));
 	}
 
 	/**

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 @@ class NavigationControllerTest extends TestCase {
 		}
 	}
 
+	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' => 'logout', 'href' => '/index.php/apps/files', 'icon' => 'icon' ] ];
+		$this->request->expects($this->once())
+			->method('getHeader')
+			->with('If-None-Match')
+			->willReturn(md5(json_encode([ ['id' => 'logout', 'href' => 'logout', 'icon' => 'icon' ] ])));
+		$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());
+	}
+
 }