When using permalinks don't error out if file id can't be found

Fixes #952

* Use only the index route (since it went to showFile anyways)
* Fix tests
* Use getUserFolder to force init of users mounts

diff --git a/apps/files/lib/Controller/ViewController.php b/apps/files/lib/Controller/ViewController.php
index 779a2c7aadc9d3416b5c343ffc61ea156a347719..9d26c048368ec04b1063df6b74921819df65b36c 100644 (file)
--- a/apps/files/lib/Controller/ViewController.php
+++ b/apps/files/lib/Controller/ViewController.php
@@ -27,11 +27,11 @@
 
 namespace OCA\Files\Controller;
 
-use OC\AppFramework\Http\Request;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http\ContentSecurityPolicy;
 use OCP\AppFramework\Http\RedirectResponse;
 use OCP\AppFramework\Http\TemplateResponse;
+use OCP\Files\IRootFolder;
 use OCP\Files\NotFoundException;
 use OCP\IConfig;
 use OCP\IL10N;
@@ -67,7 +67,7 @@ class ViewController extends Controller {
        protected $userSession;
        /** @var IAppManager */
        protected $appManager;
-       /** @var \OCP\Files\Folder */
+       /** @var IRootFolder */
        protected $rootFolder;
 
        /**
@@ -80,7 +80,7 @@ class ViewController extends Controller {
         * @param EventDispatcherInterface $eventDispatcherInterface
         * @param IUserSession $userSession
         * @param IAppManager $appManager
-        * @param Folder $rootFolder
+        * @param IRootFolder $rootFolder
         */
        public function __construct($appName,
                                                                IRequest $request,
@@ -91,7 +91,7 @@ class ViewController extends Controller {
                                                                EventDispatcherInterface $eventDispatcherInterface,
                                                                IUserSession $userSession,
                                                                IAppManager $appManager,
-                                                               Folder $rootFolder
+                                                               IRootFolder $rootFolder
        ) {
                parent::__construct($appName, $request);
                $this->appName = $appName;
@@ -267,13 +267,10 @@ class ViewController extends Controller {
         * @param string $fileId file id to show
         * @return RedirectResponse redirect response or not found response
         * @throws \OCP\Files\NotFoundException
-        *
-        * @NoCSRFRequired
-        * @NoAdminRequired
         */
-       public function showFile($fileId) {
+       private function showFile($fileId) {
                $uid = $this->userSession->getUser()->getUID();
-               $baseFolder = $this->rootFolder->get($uid . '/files/');
+               $baseFolder = $this->rootFolder->getUserFolder($uid);
                $files = $baseFolder->getById($fileId);
                $params = [];
 

diff --git a/apps/files/tests/Controller/ViewControllerTest.php b/apps/files/tests/Controller/ViewControllerTest.php
index 12ff779d6f14eed70113d1aace7ea0605fba67af..0ffe66c55920c8d656b2ae5f9cb635f1832ba4ec 100644 (file)
--- a/apps/files/tests/Controller/ViewControllerTest.php
+++ b/apps/files/tests/Controller/ViewControllerTest.php
@@ -27,7 +27,7 @@ namespace OCA\Files\Tests\Controller;
 
 use OCA\Files\Controller\ViewController;
 use OCP\AppFramework\Http;
-use OCP\Files\NotFoundException;
+use OCP\Files\IRootFolder;
 use OCP\IUser;
 use OCP\Template;
 use Test\TestCase;
@@ -39,7 +39,6 @@ use OCP\IL10N;
 use OCP\IConfig;
 use OCP\IUserSession;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
-use OCP\Files\Folder;
 use OCP\App\IAppManager;
 
 /**
@@ -48,27 +47,27 @@ use OCP\App\IAppManager;
  * @package OCA\Files\Tests\Controller
  */
 class ViewControllerTest extends TestCase {
-       /** @var IRequest */
+       /** @var IRequest|\PHPUnit_Framework_MockObject_MockObject */
        private $request;
-       /** @var IURLGenerator */
+       /** @var IURLGenerator|\PHPUnit_Framework_MockObject_MockObject */
        private $urlGenerator;
        /** @var INavigationManager */
        private $navigationManager;
        /** @var IL10N */
        private $l10n;
-       /** @var IConfig */
+       /** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */
        private $config;
        /** @var EventDispatcherInterface */
        private $eventDispatcher;
-       /** @var ViewController */
+       /** @var ViewController|\PHPUnit_Framework_MockObject_MockObject */
        private $viewController;
        /** @var IUser */
        private $user;
        /** @var IUserSession */
        private $userSession;
-       /** @var IAppManager */
+       /** @var IAppManager|\PHPUnit_Framework_MockObject_MockObject */
        private $appManager;
-       /** @var Folder */
+       /** @var IRootFolder|\PHPUnit_Framework_MockObject_MockObject */
        private $rootFolder;
 
        public function setUp() {
@@ -88,7 +87,7 @@ class ViewControllerTest extends TestCase {
                $this->userSession->expects($this->any())
                        ->method('getUser')
                        ->will($this->returnValue($this->user));
-               $this->rootFolder = $this->getMockBuilder('\OCP\Files\Folder')->getMock();
+               $this->rootFolder = $this->getMockBuilder('\OCP\Files\IRootFolder')->getMock();
                $this->viewController = $this->getMockBuilder('\OCA\Files\Controller\ViewController')
                        ->setConstructorArgs([
                        'files',
@@ -265,17 +264,7 @@ class ViewControllerTest extends TestCase {
                $this->assertEquals($expected, $this->viewController->index('MyDir', 'MyView'));
        }
 
-       public function showFileMethodProvider() {
-               return [
-                       [true],
-                       [false],
-               ];
-       }
-
-       /**
-        * @dataProvider showFileMethodProvider
-        */
-       public function testShowFileRouteWithFolder($useShowFile) {
+       public function testShowFileRouteWithFolder() {
                $node = $this->getMockBuilder('\OCP\Files\Folder')->getMock();
                $node->expects($this->once())
                        ->method('getPath')
@@ -284,8 +273,8 @@ class ViewControllerTest extends TestCase {
                $baseFolder = $this->getMockBuilder('\OCP\Files\Folder')->getMock();
 
                $this->rootFolder->expects($this->once())
-                       ->method('get')
-                       ->with('testuser1/files/')
+                       ->method('getUserFolder')
+                       ->with('testuser1')
                        ->will($this->returnValue($baseFolder));
 
                $baseFolder->expects($this->at(0))
@@ -304,17 +293,10 @@ class ViewControllerTest extends TestCase {
                        ->will($this->returnValue('/apps/files/?dir=/test/sub'));
 
                $expected = new Http\RedirectResponse('/apps/files/?dir=/test/sub');
-               if ($useShowFile) {
-                       $this->assertEquals($expected, $this->viewController->showFile(123));
-               } else {
-                       $this->assertEquals($expected, $this->viewController->index('/whatever', '', '123'));
-               }
+               $this->assertEquals($expected, $this->viewController->index('/whatever', '', '123'));
        }
 
-       /**
-        * @dataProvider showFileMethodProvider
-        */
-       public function testShowFileRouteWithFile($useShowFile) {
+       public function testShowFileRouteWithFile() {
                $parentNode = $this->getMockBuilder('\OCP\Files\Folder')->getMock();
                $parentNode->expects($this->once())
                        ->method('getPath')
@@ -323,8 +305,8 @@ class ViewControllerTest extends TestCase {
                $baseFolder = $this->getMockBuilder('\OCP\Files\Folder')->getMock();
 
                $this->rootFolder->expects($this->once())
-                       ->method('get')
-                       ->with('testuser1/files/')
+                       ->method('getUserFolder')
+                       ->with('testuser1')
                        ->will($this->returnValue($baseFolder));
 
                $node = $this->getMockBuilder('\OCP\Files\File')->getMock();
@@ -351,21 +333,14 @@ class ViewControllerTest extends TestCase {
                        ->will($this->returnValue('/apps/files/?dir=/test/sub&scrollto=somefile.txt'));
 
                $expected = new Http\RedirectResponse('/apps/files/?dir=/test/sub&scrollto=somefile.txt');
-               if ($useShowFile) {
-                       $this->assertEquals($expected, $this->viewController->showFile(123));
-               } else {
-                       $this->assertEquals($expected, $this->viewController->index('/whatever', '', '123'));
-               }
+               $this->assertEquals($expected, $this->viewController->index('/whatever', '', '123'));
        }
 
-       /**
-        * @dataProvider showFileMethodProvider
-        */
-       public function testShowFileRouteWithInvalidFileId($useShowFile) {
+       public function testShowFileRouteWithInvalidFileId() {
                $baseFolder = $this->getMockBuilder('\OCP\Files\Folder')->getMock();
                $this->rootFolder->expects($this->once())
-                       ->method('get')
-                       ->with('testuser1/files/')
+                       ->method('getUserFolder')
+                       ->with('testuser1')
                        ->will($this->returnValue($baseFolder));
 
                $baseFolder->expects($this->at(0))
@@ -373,21 +348,13 @@ class ViewControllerTest extends TestCase {
                        ->with(123)
                        ->will($this->returnValue([]));
 
-               if ($useShowFile) {
-                       $this->setExpectedException('OCP\Files\NotFoundException');
-                       $this->viewController->showFile(123);
-               } else {
-                       $response = $this->viewController->index('MyDir', 'MyView', '123');
-                       $this->assertInstanceOf('OCP\AppFramework\Http\TemplateResponse', $response);
-                       $params = $response->getParams();
-                       $this->assertEquals(1, $params['fileNotFound']);
-               }
+               $response = $this->viewController->index('MyDir', 'MyView', '123');
+               $this->assertInstanceOf('OCP\AppFramework\Http\TemplateResponse', $response);
+               $params = $response->getParams();
+               $this->assertEquals(1, $params['fileNotFound']);
        }
 
-       /**
-        * @dataProvider showFileMethodProvider
-        */
-       public function testShowFileRouteWithTrashedFile($useShowFile) {
+       public function testShowFileRouteWithTrashedFile() {
                $this->appManager->expects($this->once())
                        ->method('isEnabledForUser')
                        ->with('files_trashbin')
@@ -402,8 +369,8 @@ class ViewControllerTest extends TestCase {
                $baseFolderTrash = $this->getMockBuilder('\OCP\Files\Folder')->getMock();
 
                $this->rootFolder->expects($this->at(0))
-                       ->method('get')
-                       ->with('testuser1/files/')
+                       ->method('getUserFolder')
+                       ->with('testuser1')
                        ->will($this->returnValue($baseFolderFiles));
                $this->rootFolder->expects($this->at(1))
                        ->method('get')
@@ -439,10 +406,6 @@ class ViewControllerTest extends TestCase {
                        ->will($this->returnValue('/apps/files/?view=trashbin&dir=/test.d1462861890/sub&scrollto=somefile.txt'));
 
                $expected = new Http\RedirectResponse('/apps/files/?view=trashbin&dir=/test.d1462861890/sub&scrollto=somefile.txt');
-               if ($useShowFile) {
-                       $this->assertEquals($expected, $this->viewController->showFile(123));
-               } else {
-                       $this->assertEquals($expected, $this->viewController->index('/whatever', '', '123'));
-               }
+               $this->assertEquals($expected, $this->viewController->index('/whatever', '', '123'));
        }
 }

diff --git a/core/routes.php b/core/routes.php
index b4868c14cf3143f971f65628a0e466fd10e26685..a148ee091e6665ed65ca075352f6c160756266e8 100644 (file)
--- a/core/routes.php
+++ b/core/routes.php
@@ -121,9 +121,9 @@ $this->create('core_ajax_update', '/core/ajax/update.php')
        ->actionInclude('core/ajax/update.php');
 
 // File routes
-$this->create('files.viewcontroller.showFile', '/f/{fileId}')->action(function($urlParams) {
+$this->create('files.viewcontroller.showFile', '/f/{fileid}')->action(function($urlParams) {
        $app = new \OCA\Files\AppInfo\Application($urlParams);
-       $app->dispatch('ViewController', 'showFile');
+       $app->dispatch('ViewController', 'index');
 });
 
 // Sharing routes