Use private links in activities

owncloud · Feb 11, 2019 · 9b1b050 · 9b1b050
1 parent 723e5ba
commit 9b1b050
Show file tree

Hide file tree

Showing 6 changed files with 60 additions and 35 deletions.
diff --git a/lib/Controller/OCSEndPoint.php b/lib/Controller/OCSEndPoint.php
@@ -1,6 +1,7 @@
 <?php
 /**
  * @author Joas Schilling <[email protected]>
+ * @author Vincent Petry <[email protected]>
  *
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  * @license AGPL-3.0
@@ -292,7 +293,7 @@ protected function getPreview($owner, $fileId, $filePath) {
 		}
 
 		$preview = [
-			'link'			=> $this->getPreviewLink($info['path'], $info['is_dir'], $info['view']),
+			'link'			=> $this->getPreviewLink($info['path'], $info['is_dir'], $info['view'], $fileId),
 			'source'		=> '',
 			'isMimeTypeIcon' => true,
 		];
@@ -343,7 +344,7 @@ protected function getPreview($owner, $fileId, $filePath) {
 	protected function getPreviewFromPath($filePath, $info) {
 		$mimeType = $this->mimeTypeDetector->detectPath($filePath);
 		$preview = [
-			'link'			=> $this->getPreviewLink($info['path'], $info['is_dir'], $info['view']),
+			'link'			=> $this->getPreviewLink($info['path'], $info['is_dir'], $info['view'], $info['fileid']),
 			'source'		=> $this->getPreviewPathFromMimeType($mimeType),
 			'isMimeTypeIcon' => true,
 		];
@@ -368,9 +369,13 @@ protected function getPreviewPathFromMimeType($mimeType) {
 	 * @param string $path
 	 * @param bool $isDir
 	 * @param string $view
+	 * @param int $fileId
 	 * @return string
 	 */
-	protected function getPreviewLink($path, $isDir, $view) {
+	protected function getPreviewLink($path, $isDir, $view, $fileId) {
+		if ($fileId !== null && $fileId !== '') {
+			return $this->urlGenerator->linkToRoute('files.viewcontroller.showFile', ['fileId' => $fileId]);
+		}
 		$params = [
 			'dir' => $path,
 		];

diff --git a/lib/FilesHooks.php b/lib/FilesHooks.php
@@ -553,9 +553,9 @@ protected function addNotificationsForUser($user, $subject, $subjectParams, $fil
 
 		$selfAction = $user === $this->currentUser;
 		$app = $type === Files_Sharing::TYPE_SHARED ? 'files_sharing' : 'files';
-		$link = $this->urlGenerator->linkToRouteAbsolute('files.view.index', [
-			'dir' => ($isFile) ? \dirname($path) : $path,
-		]);
+		$link = $this->urlGenerator->linkToRouteAbsolute('files.viewcontroller.showFile', array(
+			'fileId' => $fileId
+		));
 
 		$objectType = ($fileId) ? 'files' : '';
 

diff --git a/lib/Formatter/FileFormatter.php b/lib/Formatter/FileFormatter.php
@@ -1,6 +1,7 @@
 <?php
 /**
  * @author Joas Schilling <[email protected]>
+ * @author Vincent Petry <[email protected]>
  *
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  * @license AGPL-3.0
@@ -72,28 +73,28 @@ public function format(IEvent $event, $parameter) {
 			$info = $this->infoCache->getInfoByPath($this->user, $param);
 		}
 
-		if ($info['is_dir']) {
-			$linkData = ['dir' => $info['path']];
+		$param = trim($param, '/');
+		if ($fileId !== '') {
+			$fileLink = $this->urlGenerator->linkToRouteAbsolute('files.viewcontroller.showFile', ['fileId' => $fileId]);
 		} else {
-			$parentDir = (\substr_count($info['path'], '/') === 1) ? '/' : \dirname($info['path']);
-			$fileName = \basename($info['path']);
-			$linkData = [
-				'dir' => $parentDir,
-				'scrollto' => $fileName,
-			];
-		}
+			if ($info['is_dir']) {
+				$linkData = ['dir' => $info['path']];
+			} else {
+				$parentDir = (\substr_count($info['path'], '/') === 1) ? '/' : \dirname($info['path']);
+				$fileName = \basename($info['path']);
+				$linkData = [
+					'dir' => $parentDir,
+					'scrollto' => $fileName,
+				];
+			}
 
-		if ($info['view'] !== '') {
-			$linkData['view'] = $info['view'];
-		}
+			if ($info['view'] !== '') {
+				$linkData['view'] = $info['view'];
+			}
 
-		$param = \trim($param, '/');
-		if ($param === '') {
-			$param = '/';
+			$fileLink = $this->urlGenerator->linkToRouteAbsolute('files.view.index', $linkData);
 		}
 
-		$fileLink = $this->urlGenerator->linkToRouteAbsolute('files.view.index', $linkData);
-
 		return '<file link="' . $fileLink . '" id="' . Util::sanitizeHTML($fileId) . '">' . Util::sanitizeHTML($param) . '</file>';
 	}
 

diff --git a/tests/Unit/Controller/OCSEndPointTest.php b/tests/Unit/Controller/OCSEndPointTest.php
@@ -769,4 +769,12 @@ public function testGetPreviewLink($path, $isDir, $view, $expected) {
 
 		$this->invokePrivate($this->controller, 'getPreviewLink', [$path, $isDir, $view]);
 	}
+
+	public function testGetPreviewLinkFileId() {
+		$this->urlGenerator->expects($this->once())
+			->method('linkToRoute')
+			->with('files.viewcontroller.showFile', ['fileId' => 123]);
+
+		$this->invokePrivate($this->controller, 'getPreviewLink', [123]);
+	}
 }
diff --git a/tests/Unit/FilesHooksTest.php b/tests/Unit/FilesHooksTest.php
@@ -871,7 +871,7 @@ public function testAddNotificationsForUser($user, $subject, $parameter, $fileId
 
 		$this->urlGenerator->expects($this->once())
 			->method('linkToRouteAbsolute')
-			->with('files.view.index', ['dir' => $urlPath])
+			->with('files.viewcontroller.showFile', ['fileId' => $fileId])
 			->willReturn('routeToFilesIndex');
 
 		$event = $this->getMockBuilder('OCP\Activity\IEvent')

diff --git a/tests/Unit/Formatter/FileFormatterTest.php b/tests/Unit/Formatter/FileFormatterTest.php
@@ -95,18 +95,29 @@ public function dataFormat() {
 			'exists'	=> true,
 			'view'		=> 'trashbin',
 		];
+		$trash2 = [
+			'path'		=> '/test2',
+			'is_dir'	=> true,
+			'exists'	=> true,
+			'view'		=> 'trashbin',
+			'fileid'	=> 42
+		];
 
 		return [
-			['user1', '/test1', false, [], '<file link="apps/files/?dir=%2F&scrollto=test1" id="">test1</file>'],
-			['user1', '/test1', true, [], '<file link="apps/files/?dir=%2Ftest1" id="">test1</file>'],
-			['user1', '/', true, [], '<file link="apps/files/?dir=%2F" id="">/</file>'],
-			['user1', '/test1/test2', false, [], '<file link="apps/files/?dir=%2Ftest1&scrollto=test2" id="">test1/test2</file>'],
-			['user1', '/test1/test2', true, [], '<file link="apps/files/?dir=%2Ftest1%2Ftest2" id="">test1/test2</file>'],
+			['user1', '/test1', false, [], '<file link="apps/files/?dir=%2F&scrollto=test1" id="">test1</file>', 'files.view.index'],
+			['user1', '/test1', true, [], '<file link="apps/files/?dir=%2Ftest1" id="">test1</file>', 'files.view.index'],
+			['user1', '/', true, [], '<file link="apps/files/?dir=%2F" id="">/</file>', 'files.view.index'],
+			['user1', '/test1/test2', false, [], '<file link="apps/files/?dir=%2Ftest1&scrollto=test2" id="">test1/test2</file>', 'files.view.index'],
+			['user1', '/test1/test2', true, [], '<file link="apps/files/?dir=%2Ftest1%2Ftest2" id="">test1/test2</file>', 'files.view.index'],
+
+			['user1', [42 => '/test1/test2'], false, [], '<file link="f/42" id="42">test1/test2</file>', 'files.viewcontroller.showFile'],
+			['user1', [42 => '/test1/test2'], true, [], '<file link="f/42" id="42">test1/test2</file>', 'files.viewcontroller.showFile'],
 
-			['user1', '/test1/test2', false, $trash0, '<file link="apps/files/?dir=%2F&scrollto=test2&view=trashbin" id="42">test1/test2</file>'],
-			['user1', '/test1/test2', true, $trash1, '<file link="apps/files/?dir=%2Ftest2&view=trashbin" id="42">test1/test2</file>'],
+			['user1', '/test1/test2', false, $trash0, '<file link="apps/files/?dir=%2F&scrollto=test2&view=trashbin" id="42">test1/test2</file>', 'files.view.index'],
+			['user1', '/test1/test2', true, $trash1, '<file link="apps/files/?dir=%2Ftest2&view=trashbin" id="42">test1/test2</file>', 'files.view.index'],
+			['user1', '/test1/test2', true, $trash2, '<file link="f/42" id="42">test1/test2</file>', 'files.viewcontroller.showFile'],
 
-			['user2', '/test1', false, [], '<file link="apps/files/?dir=%2F&scrollto=test1" id="">test1</file>'],
+			['user2', '/test1', false, [], '<file link="apps/files/?dir=%2F&scrollto=test1" id="">test1</file>', 'files.view.index'],
 		];
 	}
 
@@ -119,7 +130,7 @@ public function dataFormat() {
 	 * @param array $info
 	 * @param string $expected
 	 */
-	public function testFormat($user, $parameter, $isDir, array $info, $expected) {
+	public function testFormat($user, $parameter, $isDir, array $info, $expected, $routeName) {
 		/** @var \OCP\Activity\IEvent|\PHPUnit_Framework_MockObject_MockObject $event */
 		$event = $this->getMockBuilder('OCP\Activity\IEvent')
 			->disableOriginalConstructor()
@@ -138,8 +149,8 @@ public function testFormat($user, $parameter, $isDir, array $info, $expected) {
 
 		$this->urlGenerator->expects($this->once())
 			->method('linkToRouteAbsolute')
-			->with('files.view.index', $this->anything())
-			->willReturnCallback(function ($route, $parameters) {
+			->with($routeName, $this->anything())
+			->willReturnCallback(function($route, $parameters) {
 				$paramList = [];
 				foreach ($parameters as $key => $value) {
 					$paramList[] = $key . '=' . \urlencode($value);