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
This commit is contained in:
Roeland Jago Douma 2016-08-19 08:15:30 +02:00
parent a83c5e8b00
commit 54f79a28f6
No known key found for this signature in database
GPG Key ID: 1E152838F164D13B
3 changed files with 35 additions and 75 deletions

View File

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

View File

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

View File

@ -121,9 +121,9 @@ $this->create('core_ajax_update', '/core/ajax/update.php')
->actionInclude('core/ajax/update.php'); ->actionInclude('core/ajax/update.php');
// File routes // 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 = new \OCA\Files\AppInfo\Application($urlParams);
$app->dispatch('ViewController', 'showFile'); $app->dispatch('ViewController', 'index');
}); });
// Sharing routes // Sharing routes