diff --git a/lib/private/connector/sabre/file.php b/lib/private/connector/sabre/file.php index cfa1cacdd0..8e4460ef3b 100644 --- a/lib/private/connector/sabre/file.php +++ b/lib/private/connector/sabre/file.php @@ -161,13 +161,37 @@ class File extends Node implements IFile { } try { + $view = \OC\Files\Filesystem::getView(); + $run = true; + if ($view) { + $hookPath = $view->getRelativePath($this->fileView->getAbsolutePath($this->path)); + + if (!$exists) { + \OC_Hook::emit(\OC\Files\Filesystem::CLASSNAME, \OC\Files\Filesystem::signal_create, array( + \OC\Files\Filesystem::signal_param_path => $hookPath, + \OC\Files\Filesystem::signal_param_run => &$run, + )); + } else { + \OC_Hook::emit(\OC\Files\Filesystem::CLASSNAME, \OC\Files\Filesystem::signal_update, array( + \OC\Files\Filesystem::signal_param_path => $hookPath, + \OC\Files\Filesystem::signal_param_run => &$run, + )); + } + \OC_Hook::emit(\OC\Files\Filesystem::CLASSNAME, \OC\Files\Filesystem::signal_write, array( + \OC\Files\Filesystem::signal_param_path => $hookPath, + \OC\Files\Filesystem::signal_param_run => &$run, + )); + } + if ($needsPartFile) { // rename to correct path try { - $renameOkay = $storage->moveFromStorage($partStorage, $internalPartPath, $internalPath); - $fileExists = $storage->file_exists($internalPath); - if ($renameOkay === false || $fileExists === false) { - \OC_Log::write('webdav', '\OC\Files\Filesystem::rename() failed', \OC_Log::ERROR); + if ($run) { + $renameOkay = $storage->moveFromStorage($partStorage, $internalPartPath, $internalPath); + $fileExists = $storage->file_exists($internalPath); + } + if (!$run || $renameOkay === false || $fileExists === false) { + \OC_Log::write('webdav', 'renaming part file to final file failed', \OC_Log::ERROR); $partStorage->unlink($internalPartPath); throw new Exception('Could not rename part file to final file'); } @@ -180,9 +204,7 @@ class File extends Node implements IFile { // since we skipped the view we need to scan and emit the hooks ourselves $partStorage->getScanner()->scanFile($internalPath); - $view = \OC\Files\Filesystem::getView(); if ($view) { - $hookPath = $view->getRelativePath($this->fileView->getAbsolutePath($this->path)); $this->fileView->getUpdater()->propagate($hookPath); if (!$exists) { \OC_Hook::emit(\OC\Files\Filesystem::CLASSNAME, \OC\Files\Filesystem::signal_post_create, array( diff --git a/tests/lib/connector/sabre/file.php b/tests/lib/connector/sabre/file.php index ee9c20fd9c..6602a2df24 100644 --- a/tests/lib/connector/sabre/file.php +++ b/tests/lib/connector/sabre/file.php @@ -8,8 +8,35 @@ namespace Test\Connector\Sabre; +use Test\HookHelper; +use OC\Files\Filesystem; + class File extends \Test\TestCase { + /** + * @var string + */ + private $user; + + public function setUp() { + parent::setUp(); + + \OC_Hook::clear(); + + $this->user = $this->getUniqueID('user_'); + $userManager = \OC::$server->getUserManager(); + $userManager->createUser($this->user, 'pass'); + + $this->loginAsUser($this->user); + } + + public function tearDown() { + $userManager = \OC::$server->getUserManager(); + $userManager->get($this->user)->delete(); + + parent::tearDown(); + } + private function getStream($string) { $stream = fopen('php://temp', 'r+'); fwrite($stream, $string); @@ -23,7 +50,7 @@ class File extends \Test\TestCase { public function testSimplePutFails() { // setup $storage = $this->getMock('\OC\Files\Storage\Local', ['fopen'], [['datadir' => \OC::$server->getTempManager()->getTemporaryFolder()]]); - $view = $this->getMock('\OC\Files\View', array('file_put_contents', 'getRelativePath', 'resolvePath'), array()); + $view = $this->getMock('\OC\Files\View', array('getRelativePath', 'resolvePath'), array()); $view->expects($this->any()) ->method('resolvePath') ->will($this->returnValue(array($storage, ''))); @@ -45,45 +72,166 @@ class File extends \Test\TestCase { $file->put('test data'); } - public function testPutSingleFileShare() { - // setup - $stream = fopen('php://temp', 'w+'); - $storage = $this->getMock('\OC\Files\Storage\Local', ['fopen'], [['datadir' => \OC::$server->getTempManager()->getTemporaryFolder()]]); - $view = $this->getMock('\OC\Files\View', array('file_put_contents', 'getRelativePath', 'resolvePath'), array()); - $view->expects($this->any()) - ->method('resolvePath') - ->will($this->returnValue(array($storage, ''))); - $view->expects($this->any()) - ->method('getRelativePath') - ->will($this->returnValue('')); - $view->expects($this->any()) - ->method('file_put_contents') - ->with('') - ->will($this->returnValue(true)); - $storage->expects($this->once()) - ->method('fopen') - ->will($this->returnValue($stream)); + private function doPut($path, $viewRoot = null) { + $view = \OC\Files\Filesystem::getView(); + if (!is_null($viewRoot)) { + $view = new \OC\Files\View($viewRoot); + } else { + $viewRoot = '/' . $this->user . '/files'; + } - $info = new \OC\Files\FileInfo('/foo.txt', null, null, array( - 'permissions' => \OCP\Constants::PERMISSION_ALL - ), null); + $info = new \OC\Files\FileInfo( + $viewRoot . '/' . ltrim($path, '/'), + null, + null, + ['permissions' => \OCP\Constants::PERMISSION_ALL], + null + ); $file = new \OC\Connector\Sabre\File($view, $info); $this->assertNotEmpty($file->put($this->getStream('test data'))); } + /** + * Test putting a single file + */ + public function testPutSingleFile() { + $this->doPut('/foo.txt'); + } + + /** + * Test that putting a file triggers create hooks + */ + public function testPutSingleFileTriggersHooks() { + HookHelper::setUpHooks(); + + $this->doPut('/foo.txt'); + + $this->assertCount(4, HookHelper::$hookCalls); + $this->assertHookCall( + HookHelper::$hookCalls[0], + Filesystem::signal_create, + '/foo.txt' + ); + $this->assertHookCall( + HookHelper::$hookCalls[1], + Filesystem::signal_write, + '/foo.txt' + ); + $this->assertHookCall( + HookHelper::$hookCalls[2], + Filesystem::signal_post_create, + '/foo.txt' + ); + $this->assertHookCall( + HookHelper::$hookCalls[3], + Filesystem::signal_post_write, + '/foo.txt' + ); + } + + /** + * Test that putting a file triggers update hooks + */ + public function testPutOverwriteFileTriggersHooks() { + $view = \OC\Files\Filesystem::getView(); + $view->file_put_contents('/foo.txt', 'some content that will be replaced'); + + HookHelper::setUpHooks(); + + $this->doPut('/foo.txt'); + + $this->assertCount(4, HookHelper::$hookCalls); + $this->assertHookCall( + HookHelper::$hookCalls[0], + Filesystem::signal_update, + '/foo.txt' + ); + $this->assertHookCall( + HookHelper::$hookCalls[1], + Filesystem::signal_write, + '/foo.txt' + ); + $this->assertHookCall( + HookHelper::$hookCalls[2], + Filesystem::signal_post_update, + '/foo.txt' + ); + $this->assertHookCall( + HookHelper::$hookCalls[3], + Filesystem::signal_post_write, + '/foo.txt' + ); + } + + /** + * Test that putting a file triggers hooks with the correct path + * if the passed view was chrooted (can happen with public webdav + * where the root is the share root) + */ + public function testPutSingleFileTriggersHooksDifferentRoot() { + $view = \OC\Files\Filesystem::getView(); + $view->mkdir('noderoot'); + + HookHelper::setUpHooks(); + + // happens with public webdav where the view root is the share root + $this->doPut('/foo.txt', '/' . $this->user . '/files/noderoot'); + + $this->assertCount(4, HookHelper::$hookCalls); + $this->assertHookCall( + HookHelper::$hookCalls[0], + Filesystem::signal_create, + '/noderoot/foo.txt' + ); + $this->assertHookCall( + HookHelper::$hookCalls[1], + Filesystem::signal_write, + '/noderoot/foo.txt' + ); + $this->assertHookCall( + HookHelper::$hookCalls[2], + Filesystem::signal_post_create, + '/noderoot/foo.txt' + ); + $this->assertHookCall( + HookHelper::$hookCalls[3], + Filesystem::signal_post_write, + '/noderoot/foo.txt' + ); + } + + public static function cancellingHook($params) { + self::$hookCalls[] = array( + 'signal' => Filesystem::signal_post_create, + 'params' => $params + ); + } + + /** + * Test put file with cancelled hook + * + * @expectedException \Sabre\DAV\Exception + */ + public function testPutSingleFileCancelPreHook() { + \OCP\Util::connectHook( + Filesystem::CLASSNAME, + Filesystem::signal_create, + '\Test\HookHelper', + 'cancellingCallback' + ); + + $this->doPut('/foo.txt'); + } + /** * @expectedException \Sabre\DAV\Exception */ public function testSimplePutFailsOnRename() { // setup $view = $this->getMock('\OC\Files\View', - array('file_put_contents', 'rename', 'getRelativePath', 'filesize')); - $view->expects($this->any()) - ->method('file_put_contents') - ->withAnyParameters() - ->will($this->returnValue(true)); + array('rename', 'getRelativePath', 'filesize')); $view->expects($this->any()) ->method('rename') ->withAnyParameters() @@ -113,11 +261,7 @@ class File extends \Test\TestCase { */ public function testSimplePutInvalidChars() { // setup - $view = $this->getMock('\OC\Files\View', array('file_put_contents', 'getRelativePath')); - $view->expects($this->any()) - ->method('file_put_contents') - ->will($this->returnValue(false)); - + $view = $this->getMock('\OC\Files\View', array('getRelativePath')); $view->expects($this->any()) ->method('getRelativePath') ->will($this->returnValue('/*')); @@ -157,11 +301,7 @@ class File extends \Test\TestCase { public function testUploadAbort() { // setup $view = $this->getMock('\OC\Files\View', - array('file_put_contents', 'rename', 'getRelativePath', 'filesize')); - $view->expects($this->any()) - ->method('file_put_contents') - ->withAnyParameters() - ->will($this->returnValue(true)); + array('rename', 'getRelativePath', 'filesize')); $view->expects($this->any()) ->method('rename') ->withAnyParameters() @@ -248,4 +388,20 @@ class File extends \Test\TestCase { // action $file->delete(); } + + /** + * Asserts hook call + * + * @param array $callData hook call data to check + * @param string $signal signal name + * @param string $hookPath hook path + */ + protected function assertHookCall($callData, $signal, $hookPath) { + $this->assertEquals($signal, $callData['signal']); + $params = $callData['params']; + $this->assertEquals( + $hookPath, + $params[Filesystem::signal_param_path] + ); + } } diff --git a/tests/lib/hookhelper.php b/tests/lib/hookhelper.php new file mode 100644 index 0000000000..93411bd068 --- /dev/null +++ b/tests/lib/hookhelper.php @@ -0,0 +1,112 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace Test; + +use OC\Files\Filesystem; + +/** + * Helper class to register hooks on + */ +class HookHelper { + public static $hookCalls; + + public static function setUpHooks() { + self::clear(); + \OCP\Util::connectHook( + Filesystem::CLASSNAME, + Filesystem::signal_create, + '\Test\HookHelper', + 'createCallback' + ); + \OCP\Util::connectHook( + Filesystem::CLASSNAME, + Filesystem::signal_update, + '\Test\HookHelper', + 'updateCallback' + ); + \OCP\Util::connectHook( + Filesystem::CLASSNAME, + Filesystem::signal_write, + '\Test\HookHelper', + 'writeCallback' + ); + + \OCP\Util::connectHook( + Filesystem::CLASSNAME, + Filesystem::signal_post_create, + '\Test\HookHelper', + 'postCreateCallback' + ); + \OCP\Util::connectHook( + Filesystem::CLASSNAME, + Filesystem::signal_post_update, + '\Test\HookHelper', + 'postUpdateCallback' + ); + \OCP\Util::connectHook( + Filesystem::CLASSNAME, + Filesystem::signal_post_write, + '\Test\HookHelper', + 'postWriteCallback' + ); + } + + public static function clear() { + self::$hookCalls = []; + } + + public static function createCallback($params) { + self::$hookCalls[] = array( + 'signal' => Filesystem::signal_create, + 'params' => $params + ); + } + + public static function updateCallback($params) { + self::$hookCalls[] = array( + 'signal' => Filesystem::signal_update, + 'params' => $params + ); + } + + public static function writeCallback($params) { + self::$hookCalls[] = array( + 'signal' => Filesystem::signal_write, + 'params' => $params + ); + } + + public static function postCreateCallback($params) { + self::$hookCalls[] = array( + 'signal' => Filesystem::signal_post_create, + 'params' => $params + ); + } + + public static function postUpdateCallback($params) { + self::$hookCalls[] = array( + 'signal' => Filesystem::signal_post_update, + 'params' => $params + ); + } + + public static function postWriteCallback($params) { + self::$hookCalls[] = array( + 'signal' => Filesystem::signal_post_write, + 'params' => $params + ); + } + + /** + * Callback that sets the run paramter to false + */ + public static function cancellingCallback($params) { + $params[Filesystem::signal_param_run] = false; + } +}