From f28f5380295b16d89f81994fc990924b8f15fa20 Mon Sep 17 00:00:00 2001
From: Vincent Petry <pvince81@owncloud.com>
Date: Wed, 16 Mar 2016 10:15:41 +0100
Subject: [PATCH] Do not fire pre/post hooks twice on chunk upload
---
apps/dav/lib/connector/sabre/file.php | 4 +-
apps/dav/tests/unit/connector/sabre/file.php | 69 ++++++++++++++++++++
lib/private/filechunking.php | 48 ++------------
3 files changed, 76 insertions(+), 45 deletions(-)
diff --git a/apps/dav/lib/connector/sabre/file.php b/apps/dav/lib/connector/sabre/file.php
index 59b3a6e7483..6b698c6e5a9 100644
--- a/apps/dav/lib/connector/sabre/file.php
+++ b/apps/dav/lib/connector/sabre/file.php
@@ -433,7 +433,7 @@ class File extends Node implements IFile {
list($partStorage, $partInternalPath) = $this->fileView->resolvePath($partFile);
- $chunk_handler->file_assemble($partStorage, $partInternalPath, $this->fileView->getAbsolutePath($targetPath));
+ $chunk_handler->file_assemble($partStorage, $partInternalPath);
// here is the final atomic rename
$renameOkay = $targetStorage->moveFromStorage($partStorage, $partInternalPath, $targetInternalPath);
@@ -452,7 +452,7 @@ class File extends Node implements IFile {
}
} else {
// assemble directly into the final file
- $chunk_handler->file_assemble($targetStorage, $targetInternalPath, $this->fileView->getAbsolutePath($targetPath));
+ $chunk_handler->file_assemble($targetStorage, $targetInternalPath);
}
// allow sync clients to send the mtime along in a header
diff --git a/apps/dav/tests/unit/connector/sabre/file.php b/apps/dav/tests/unit/connector/sabre/file.php
index 8bbef225483..eab7ece159c 100644
--- a/apps/dav/tests/unit/connector/sabre/file.php
+++ b/apps/dav/tests/unit/connector/sabre/file.php
@@ -422,6 +422,75 @@ class File extends \Test\TestCase {
);
}
+ /**
+ * Test that putting a file with chunks triggers create hooks
+ */
+ public function testPutChunkedFileTriggersHooks() {
+ HookHelper::setUpHooks();
+
+ $_SERVER['HTTP_OC_CHUNKED'] = true;
+ $this->assertNull($this->doPut('/foo.txt-chunking-12345-2-0'));
+ $this->assertNotEmpty($this->doPut('/foo.txt-chunking-12345-2-1'));
+
+ $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 chunked file triggers update hooks
+ */
+ public function testPutOverwriteChunkedFileTriggersHooks() {
+ $view = \OC\Files\Filesystem::getView();
+ $view->file_put_contents('/foo.txt', 'some content that will be replaced');
+
+ HookHelper::setUpHooks();
+
+ $_SERVER['HTTP_OC_CHUNKED'] = true;
+ $this->assertNull($this->doPut('/foo.txt-chunking-12345-2-0'));
+ $this->assertNotEmpty($this->doPut('/foo.txt-chunking-12345-2-1'));
+
+ $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'
+ );
+ }
+
public static function cancellingHook($params) {
self::$hookCalls[] = array(
'signal' => Filesystem::signal_post_create,
diff --git a/lib/private/filechunking.php b/lib/private/filechunking.php
index d6ff160870d..604a607336c 100644
--- a/lib/private/filechunking.php
+++ b/lib/private/filechunking.php
@@ -150,59 +150,21 @@ class OC_FileChunking {
* Assembles the chunks into the file specified by the path.
* Also triggers the relevant hooks and proxies.
*
- * @param \OC\Files\Storage\Storage $storage
+ * @param \OC\Files\Storage\Storage $storage storage
* @param string $path target path relative to the storage
- * @param string $absolutePath
- * @return bool assembled file size or false if file could not be created
+ * @return bool true on success or false if file could not be created
*
* @throws \OC\ServerNotAvailableException
*/
- public function file_assemble($storage, $path, $absolutePath) {
- $data = '';
+ public function file_assemble($storage, $path) {
// use file_put_contents as method because that best matches what this function does
if (\OC\Files\Filesystem::isValidPath($path)) {
- $exists = $storage->file_exists($path);
- $run = true;
- $hookPath = \OC\Files\Filesystem::getView()->getRelativePath($absolutePath);
- 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
- )
- );
- }
- 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(!$run) {
- return false;
- }
$target = $storage->fopen($path, 'w');
- if($target) {
+ if ($target) {
$count = $this->assemble($target);
fclose($target);
- if(!$exists) {
- OC_Hook::emit(
- \OC\Files\Filesystem::CLASSNAME,
- \OC\Files\Filesystem::signal_post_create,
- array( \OC\Files\Filesystem::signal_param_path => $hookPath)
- );
- }
- OC_Hook::emit(
- \OC\Files\Filesystem::CLASSNAME,
- \OC\Files\Filesystem::signal_post_write,
- array( \OC\Files\Filesystem::signal_param_path => $hookPath)
- );
return $count > 0;
- }else{
+ } else {
return false;
}
}
--
GitLab