Merge pull request #16292 from owncloud/webdav-storage-fireprehooks

Fire prehooks when uploading directly to storage

3 files changed, 334 insertions, 44 deletions

diff --git a/lib/private/connector/sabre/file.php b/lib/private/connector/sabre/file.php
index cfa1cacdd0b..8e4460ef3b5 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 ee9c20fd9cb..6602a2df24f 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,28 +72,21 @@ 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));
-
-		$info = new \OC\Files\FileInfo('/foo.txt', null, null, array(
-			'permissions' => \OCP\Constants::PERMISSION_ALL
-		), null);
+	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(
+			$viewRoot . '/' . ltrim($path, '/'),
+			null,
+			null,
+			['permissions' => \OCP\Constants::PERMISSION_ALL],
+			null
+		);
 
 		$file = new \OC\Connector\Sabre\File($view, $info);
 
@@ -74,16 +94,144 @@ class File extends \Test\TestCase {
 	}
 
 	/**
+	 * 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 00000000000..93411bd068b
--- /dev/null
+++ b/tests/lib/hookhelper.php
@@ -0,0 +1,112 @@
+<?php
+/**
+ * Copyright (c) 2015 Vincent Petry <pvince81@owncloud.com>
+ * 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;
+	}
+}