Merge pull request #1019 from nextcloud/stable10_956
[Stable10] When using permalinks don't error out if file id can't be found
This commit is contained in:
Joas Schilling
GitHub
commit
2d3c29c66a
|
|
@ -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;
|
||||
|
|
@ -42,6 +42,7 @@ use OCP\IUserSession;
|
|||
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
|
||||
use OCP\Files\Folder;
|
||||
use OCP\App\IAppManager;
|
||||
use OC\AppFramework\Http\Request;
|
||||
|
||||
/**
|
||||
* Class ViewController
|
||||
|
|
@ -67,7 +68,7 @@ class ViewController extends Controller {
|
|||
protected $userSession;
|
||||
/** @var IAppManager */
|
||||
protected $appManager;
|
||||
/** @var \OCP\Files\Folder */
|
||||
/** @var IRootFolder */
|
||||
protected $rootFolder;
|
||||
|
||||
/**
|
||||
|
|
@ -80,7 +81,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 +92,7 @@ class ViewController extends Controller {
|
|||
EventDispatcherInterface $eventDispatcherInterface,
|
||||
IUserSession $userSession,
|
||||
IAppManager $appManager,
|
||||
Folder $rootFolder
|
||||
IRootFolder $rootFolder
|
||||
) {
|
||||
parent::__construct($appName, $request);
|
||||
$this->appName = $appName;
|
||||
|
|
@ -143,15 +144,14 @@ class ViewController extends Controller {
|
|||
* @param string $dir
|
||||
* @param string $view
|
||||
* @param string $fileid
|
||||
* @return TemplateResponse
|
||||
* @return TemplateResponse|RedirectResponse
|
||||
*/
|
||||
public function index($dir = '', $view = '', $fileid = null) {
|
||||
$fileNotFound = false;
|
||||
public function index($dir = '', $view = '', $fileid = null, $fileNotFound = false) {
|
||||
if ($fileid !== null) {
|
||||
try {
|
||||
return $this->showFile($fileid);
|
||||
} catch (NotFoundException $e) {
|
||||
$fileNotFound = true;
|
||||
return new RedirectResponse($this->urlGenerator->linkToRoute('files.view.index', ['fileNotFound' => true]));
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -277,13 +277,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 = [];
|
||||
|
||||
|
|
|
|||
|
|
@ -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->getMock('\OCP\Files\Folder');
|
||||
$this->rootFolder = $this->getMockBuilder('\OCP\Files\IRootFolder')->getMock();
|
||||
$this->viewController = $this->getMockBuilder('\OCA\Files\Controller\ViewController')
|
||||
->setConstructorArgs([
|
||||
'files',
|
||||
|
|
@ -305,18 +304,8 @@ class ViewControllerTest extends TestCase {
|
|||
$this->assertEquals($expected, $this->viewController->index('MyDir', 'MyView'));
|
||||
}
|
||||
|
||||
public function showFileMethodProvider() {
|
||||
return [
|
||||
[true],
|
||||
[false],
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider showFileMethodProvider
|
||||
*/
|
||||
public function testShowFileRouteWithFolder($useShowFile) {
|
||||
$node = $this->getMock('\OCP\Files\Folder');
|
||||
public function testShowFileRouteWithFolder() {
|
||||
$node = $this->getMockBuilder('\OCP\Files\Folder')->getMock();
|
||||
$node->expects($this->once())
|
||||
->method('getPath')
|
||||
->will($this->returnValue('/testuser1/files/test/sub'));
|
||||
|
|
@ -324,8 +313,8 @@ class ViewControllerTest extends TestCase {
|
|||
$baseFolder = $this->getMock('\OCP\Files\Folder');
|
||||
|
||||
$this->rootFolder->expects($this->once())
|
||||
->method('get')
|
||||
->with('testuser1/files/')
|
||||
->method('getUserFolder')
|
||||
->with('testuser1')
|
||||
->will($this->returnValue($baseFolder));
|
||||
|
||||
$baseFolder->expects($this->at(0))
|
||||
|
|
@ -344,18 +333,11 @@ 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'));
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider showFileMethodProvider
|
||||
*/
|
||||
public function testShowFileRouteWithFile($useShowFile) {
|
||||
$parentNode = $this->getMock('\OCP\Files\Folder');
|
||||
public function testShowFileRouteWithFile() {
|
||||
$parentNode = $this->getMockBuilder('\OCP\Files\Folder')->getMock();
|
||||
$parentNode->expects($this->once())
|
||||
->method('getPath')
|
||||
->will($this->returnValue('testuser1/files/test'));
|
||||
|
|
@ -363,8 +345,8 @@ class ViewControllerTest extends TestCase {
|
|||
$baseFolder = $this->getMock('\OCP\Files\Folder');
|
||||
|
||||
$this->rootFolder->expects($this->once())
|
||||
->method('get')
|
||||
->with('testuser1/files/')
|
||||
->method('getUserFolder')
|
||||
->with('testuser1')
|
||||
->will($this->returnValue($baseFolder));
|
||||
|
||||
$node = $this->getMock('\OCP\Files\File');
|
||||
|
|
@ -391,21 +373,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'));
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider showFileMethodProvider
|
||||
*/
|
||||
public function testShowFileRouteWithInvalidFileId($useShowFile) {
|
||||
$baseFolder = $this->getMock('\OCP\Files\Folder');
|
||||
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))
|
||||
|
|
@ -413,21 +388,17 @@ class ViewControllerTest extends TestCase {
|
|||
->with(123)
|
||||
->will($this->returnValue([]));
|
||||
|
||||
if ($useShowFile) {
|
||||
$this->setExpectedException('OCP\Files\NotFoundException');
|
||||
$this->viewController->showFile(123);
|
||||
} else {
|
||||
$this->urlGenerator->expects($this->once())
|
||||
->method('linkToRoute')
|
||||
->with('files.view.index', ['fileNotFound' => true])
|
||||
->willReturn('redirect.url');
|
||||
|
||||
$response = $this->viewController->index('MyDir', 'MyView', '123');
|
||||
$this->assertInstanceOf('OCP\AppFramework\Http\TemplateResponse', $response);
|
||||
$params = $response->getParams();
|
||||
$this->assertEquals(1, $params['fileNotFound']);
|
||||
}
|
||||
$this->assertInstanceOf('OCP\AppFramework\Http\RedirectResponse', $response);
|
||||
$this->assertEquals('redirect.url', $response->getRedirectURL());
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider showFileMethodProvider
|
||||
*/
|
||||
public function testShowFileRouteWithTrashedFile($useShowFile) {
|
||||
public function testShowFileRouteWithTrashedFile() {
|
||||
$this->appManager->expects($this->once())
|
||||
->method('isEnabledForUser')
|
||||
->with('files_trashbin')
|
||||
|
|
@ -442,8 +413,8 @@ class ViewControllerTest extends TestCase {
|
|||
$baseFolderTrash = $this->getMock('\OCP\Files\Folder');
|
||||
|
||||
$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')
|
||||
|
|
@ -479,10 +450,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'));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -117,9 +117,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
|
||||
|
|
|
|||
Loading…
Reference in New Issue