Merge pull request #8132 from owncloud/issue/8131

Issue/8131 Fix emitting of filesystem related hooks

lib/private/files/filesystem.php

@@ -118,6 +118,22 @@ class Filesystem {
 	 */
 	const signal_post_write = 'post_write';
 
+	/**
+	 * signal emitted before file/dir update
+	 *
+	 * @param string $path
+	 * @param bool $run changing this flag to false in hook handler will cancel event
+	 */
+	const signal_update = 'update';
+
+	/**
+	 * signal emitted after file/dir update
+	 *
+	 * @param string $path
+	 * @param bool $run changing this flag to false in hook handler will cancel event
+	 */
+	const signal_post_update = 'post_update';
+
 	/**
 	 * signal emits when reading file/dir
 	 *

lib/private/files/view.php

@@ -271,6 +271,39 @@ class View {
 		return $this->basicOperation('file_get_contents', $path, array('read'));
 	}
 
+	protected function emit_file_hooks_pre($exists, $path, &$run) {
+		if (!$exists) {
+			\OC_Hook::emit(Filesystem::CLASSNAME, Filesystem::signal_create, array(
+				Filesystem::signal_param_path => $this->getHookPath($path),
+				Filesystem::signal_param_run => &$run,
+			));
+		} else {
+			\OC_Hook::emit(Filesystem::CLASSNAME, Filesystem::signal_update, array(
+				Filesystem::signal_param_path => $this->getHookPath($path),
+				Filesystem::signal_param_run => &$run,
+			));
+		}
+		\OC_Hook::emit(Filesystem::CLASSNAME, Filesystem::signal_write, array(
+			Filesystem::signal_param_path => $this->getHookPath($path),
+			Filesystem::signal_param_run => &$run,
+		));
+	}
+
+	protected function emit_file_hooks_post($exists, $path) {
+		if (!$exists) {
+			\OC_Hook::emit(Filesystem::CLASSNAME, Filesystem::signal_post_create, array(
+				Filesystem::signal_param_path => $this->getHookPath($path),
+			));
+		} else {
+			\OC_Hook::emit(Filesystem::CLASSNAME, Filesystem::signal_post_update, array(
+				Filesystem::signal_param_path => $this->getHookPath($path),
+			));
+		}
+		\OC_Hook::emit(Filesystem::CLASSNAME, Filesystem::signal_post_write, array(
+			Filesystem::signal_param_path => $this->getHookPath($path),
+		));
+	}
+
 	public function file_put_contents($path, $data) {
 		if (is_resource($data)) { //not having to deal with streams in file_put_contents makes life easier
 			$absolutePath = Filesystem::normalizePath($this->getAbsolutePath($path));
@@ -282,24 +315,7 @@ class View {
 				$exists = $this->file_exists($path);
 				$run = true;
 				if ($this->shouldEmitHooks($path)) {
-					if (!$exists) {
+					$this->emit_file_hooks_pre($exists, $path, $run);
-						\OC_Hook::emit(
-							Filesystem::CLASSNAME,
-							Filesystem::signal_create,
-							array(
-								Filesystem::signal_param_path => $this->getHookPath($path),
-								Filesystem::signal_param_run => &$run
-							)
-						);
-					}
-					\OC_Hook::emit(
-						Filesystem::CLASSNAME,
-						Filesystem::signal_write,
-						array(
-							Filesystem::signal_param_path => $this->getHookPath($path),
-							Filesystem::signal_param_run => &$run
-						)
-					);
 				}
 				if (!$run) {
 					return false;
@@ -313,18 +329,7 @@ class View {
 						Updater::writeHook(array(
 							'path' => $this->getHookPath($path)
 						));
-						if (!$exists) {
+						$this->emit_file_hooks_post($exists, $path);
-							\OC_Hook::emit(
-								Filesystem::CLASSNAME,
-								Filesystem::signal_post_create,
-								array(Filesystem::signal_param_path => $this->getHookPath($path))
-							);
-						}
-						\OC_Hook::emit(
-							Filesystem::CLASSNAME,
-							Filesystem::signal_post_write,
-							array(Filesystem::signal_param_path => $this->getHookPath($path))
-						);
 					}
 					\OC_FileProxy::runPostProxies('file_put_contents', $absolutePath, $count);
 					return $result;
@@ -335,7 +340,7 @@ class View {
 				return false;
 			}
 		} else {
-			$hooks = ($this->file_exists($path)) ? array('write') : array('create', 'write');
+			$hooks = ($this->file_exists($path)) ? array('update', 'write') : array('create', 'write');
 			return $this->basicOperation('file_put_contents', $path, $hooks, $data);
 		}
 	}
@@ -378,6 +383,7 @@ class View {
 		) {
 			$path1 = $this->getRelativePath($absolutePath1);
 			$path2 = $this->getRelativePath($absolutePath2);
+			$exists = $this->file_exists($path2);
 
 			if ($path1 == null or $path2 == null) {
 				return false;
@@ -385,13 +391,7 @@ class View {
 			$run = true;
 			if ($this->shouldEmitHooks() && (Cache\Scanner::isPartialFile($path1) && !Cache\Scanner::isPartialFile($path2))) {
 				// if it was a rename from a part file to a regular file it was a write and not a rename operation
-				\OC_Hook::emit(
+				$this->emit_file_hooks_pre($exists, $path2, $run);
-					Filesystem::CLASSNAME, Filesystem::signal_write,
-					array(
-						Filesystem::signal_param_path => $this->getHookPath($path2),
-						Filesystem::signal_param_run => &$run
-					)
-				);
 			} elseif ($this->shouldEmitHooks()) {
 				\OC_Hook::emit(
 					Filesystem::CLASSNAME, Filesystem::signal_rename,
@@ -448,13 +448,7 @@ class View {
 				if ($this->shouldEmitHooks() && (Cache\Scanner::isPartialFile($path1) && !Cache\Scanner::isPartialFile($path2)) && $result !== false) {
 					// if it was a rename from a part file to a regular file it was a write and not a rename operation
 					Updater::writeHook(array('path' => $this->getHookPath($path2)));
-					\OC_Hook::emit(
+					$this->emit_file_hooks_post($exists, $path2);
-						Filesystem::CLASSNAME,
-						Filesystem::signal_post_write,
-						array(
-							Filesystem::signal_param_path => $this->getHookPath($path2),
-						)
-					);
 				} elseif ($this->shouldEmitHooks() && $result !== false) {
 					Updater::renameHook(array(
 						'oldpath' => $this->getHookPath($path1),
@@ -507,26 +501,7 @@ class View {
 						Filesystem::signal_param_run => &$run
 					)
 				);
-				if ($run and !$exists) {
+				$this->emit_file_hooks_pre($exists, $path2, $run);
-					\OC_Hook::emit(
-						Filesystem::CLASSNAME,
-						Filesystem::signal_create,
-						array(
-							Filesystem::signal_param_path => $this->getHookPath($path2),
-							Filesystem::signal_param_run => &$run
-						)
-					);
-				}
-				if ($run) {
-					\OC_Hook::emit(
-						Filesystem::CLASSNAME,
-						Filesystem::signal_write,
-						array(
-							Filesystem::signal_param_path => $this->getHookPath($path2),
-							Filesystem::signal_param_run => &$run
-						)
-					);
-				}
 			}
 			if ($run) {
 				$mp1 = $this->getMountPoint($path1 . $postFix1);
@@ -566,18 +541,7 @@ class View {
 							Filesystem::signal_param_newpath => $this->getHookPath($path2)
 						)
 					);
-					if (!$exists) {
+					$this->emit_file_hooks_post($exists, $path2);
-						\OC_Hook::emit(
-							Filesystem::CLASSNAME,
-							Filesystem::signal_post_create,
-							array(Filesystem::signal_param_path => $this->getHookPath($path2))
-						);
-					}
-					\OC_Hook::emit(
-						Filesystem::CLASSNAME,
-						Filesystem::signal_post_write,
-						array(Filesystem::signal_param_path => $this->getHookPath($path2))
-					);
 				}
 				return $result;
 			} else {

tests/lib/files/view.php

@@ -394,13 +394,12 @@ class View extends \PHPUnit_Framework_TestCase {
 		$this->assertNull($this->hookPath);
 
 		$subView->file_put_contents('/foo.txt', 'asd');
-		$this->assertNotNull($this->hookPath);
 		$this->assertEquals('/substorage/foo.txt', $this->hookPath);
 	}
 
 	private $hookPath;
 
-	function dummyHook($params) {
+	public function dummyHook($params) {
 		$this->hookPath = $params['path'];
 	}
 
@@ -442,12 +441,6 @@ class View extends \PHPUnit_Framework_TestCase {
 		return $storage;
 	}
 
-	private $createHookPath;
-
-	function dummyCreateHook($params) {
-		$this->createHookPath = $params['path'];
-	}
-
 	/**
 	 * @medium
 	 */
@@ -466,23 +459,50 @@ class View extends \PHPUnit_Framework_TestCase {
 		$this->assertNull($this->hookPath);
 	}
 
+	private $hookWritePath;
+	private $hookCreatePath;
+	private $hookUpdatePath;
+
+	public function dummyHookWrite($params) {
+		$this->hookWritePath = $params['path'];
+	}
+
+	public function dummyHookUpdate($params) {
+		$this->hookUpdatePath = $params['path'];
+	}
+
+	public function dummyHookCreate($params) {
+		$this->hookCreatePath = $params['path'];
+	}
+
 	public function testEditNoCreateHook() {
 		$storage1 = $this->getTestStorage();
 		$storage2 = $this->getTestStorage();
 		$defaultRoot = \OC\Files\Filesystem::getRoot();
 		\OC\Files\Filesystem::mount($storage1, array(), '/');
 		\OC\Files\Filesystem::mount($storage2, array(), $defaultRoot);
-		\OC_Hook::connect('OC_Filesystem', 'post_create', $this, 'dummyCreateHook');
+		\OC_Hook::connect('OC_Filesystem', 'post_create', $this, 'dummyHookCreate');
+		\OC_Hook::connect('OC_Filesystem', 'post_update', $this, 'dummyHookUpdate');
+		\OC_Hook::connect('OC_Filesystem', 'post_write', $this, 'dummyHookWrite');
 
 		$view = new \OC\Files\View($defaultRoot);
-		$this->hookPath = null;
+		$this->hookWritePath = $this->hookUpdatePath = $this->hookCreatePath = null;
 
 		$view->file_put_contents('/asd.txt', 'foo');
-		$this->assertEquals('/asd.txt', $this->createHookPath);
+		$this->assertEquals('/asd.txt', $this->hookCreatePath);
-		$this->createHookPath = null;
+		$this->assertNull($this->hookUpdatePath);
+		$this->assertEquals('/asd.txt', $this->hookWritePath);
+
+		$this->hookWritePath = $this->hookUpdatePath = $this->hookCreatePath = null;
 
 		$view->file_put_contents('/asd.txt', 'foo');
-		$this->assertNull($this->createHookPath);
+		$this->assertNull($this->hookCreatePath);
+		$this->assertEquals('/asd.txt', $this->hookUpdatePath);
+		$this->assertEquals('/asd.txt', $this->hookWritePath);
+
+		\OC_Hook::clear('OC_Filesystem', 'post_create');
+		\OC_Hook::clear('OC_Filesystem', 'post_update');
+		\OC_Hook::clear('OC_Filesystem', 'post_write');
 	}
 
 	/**