From b302592a6435b4f9bf18719b04314d9f463d54da Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 14 Jan 2015 16:44:30 +0100 Subject: [PATCH 01/12] Small cleanup of rename code --- lib/private/files/view.php | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/lib/private/files/view.php b/lib/private/files/view.php index ab7a7d3db9..475a9d35af 100644 --- a/lib/private/files/view.php +++ b/lib/private/files/view.php @@ -616,26 +616,26 @@ class View { } if ($run) { $this->verifyPath(dirname($path2), basename($path2)); + + $mount1 = $this->getMount($path1); + $mount2 = $this->getMount($path2); + $storage1 = $mount1->getStorage(); + $storage2 = $mount1->getStorage(); + $internalPath1 = $mount1->getInternalPath($absolutePath1); + $internalPath2 = $mount2->getInternalPath($absolutePath2); - $mp1 = $this->getMountPoint($path1 . $postFix1); - $mp2 = $this->getMountPoint($path2 . $postFix2); - $manager = Filesystem::getMountManager(); - $mount = $manager->find($absolutePath1 . $postFix1); - $storage1 = $mount->getStorage(); - $internalPath1 = $mount->getInternalPath($absolutePath1 . $postFix1); - list($storage2, $internalPath2) = Filesystem::resolvePath($absolutePath2 . $postFix2); - if ($internalPath1 === '' and $mount instanceof MoveableMount) { + if ($internalPath1 === '' and $mount1 instanceof MoveableMount) { if ($this->isTargetAllowed($absolutePath2)) { /** - * @var \OC\Files\Mount\MountPoint | \OC\Files\Mount\MoveableMount $mount + * @var \OC\Files\Mount\MountPoint | \OC\Files\Mount\MoveableMount $mount1 */ - $sourceMountPoint = $mount->getMountPoint(); - $result = $mount->moveMount($absolutePath2); - $manager->moveMount($sourceMountPoint, $mount->getMountPoint()); + $sourceMountPoint = $mount1->getMountPoint(); + $result = $mount1->moveMount($absolutePath2); + $manager->moveMount($sourceMountPoint, $mount1->getMountPoint()); } else { $result = false; } - } elseif ($mp1 == $mp2) { + } elseif ($storage1 == $storage2) { if ($storage1) { $result = $storage1->rename($internalPath1, $internalPath2); } else { From 8575bb2cb9f57d2b7e6d9e9e48e7c85485a7c63b Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 16 Jan 2015 13:32:42 +0100 Subject: [PATCH 02/12] Move cross storage copy logic to the storage --- lib/private/files/storage/common.php | 48 ++++++++++ lib/private/files/storage/wrapper/wrapper.php | 20 +++++ lib/private/files/view.php | 87 +++---------------- lib/public/files/storage.php | 16 ++++ 4 files changed, 97 insertions(+), 74 deletions(-) diff --git a/lib/private/files/storage/common.php b/lib/private/files/storage/common.php index ed85d3c07c..164225de3e 100644 --- a/lib/private/files/storage/common.php +++ b/lib/private/files/storage/common.php @@ -525,4 +525,52 @@ abstract class Common implements Storage { public function getMountOption($name, $default = null) { return isset($this->mountOptions[$name]) ? $this->mountOptions[$name] : $default; } + /** + * @param \OCP\Files\Storage $sourceStorage + * @param string $sourceInternalPath + * @param string $targetInternalPath + * @param bool $preserveMtime + * @return bool + */ + public function copyFromStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath, $preserveMtime = false) { + if ($sourceStorage->is_dir($sourceInternalPath)) { + $dh = $sourceStorage->opendir($sourceInternalPath); + $result = $this->mkdir($targetInternalPath); + if (is_resource($dh)) { + while (($file = readdir($dh)) !== false) { + if (!Filesystem::isIgnoredDir($file)) { + $result &= $this->copyFromStorage($sourceStorage, $sourceInternalPath . '/' . $file, $targetInternalPath . '/' . $file); + } + } + } + } else { + $source = $sourceStorage->fopen($sourceInternalPath, 'r'); + $target = $this->fopen($targetInternalPath, 'w'); + list(, $result) = \OC_Helper::streamCopy($source, $target); + if ($preserveMtime) { + $this->touch($targetInternalPath, $sourceStorage->filemtime($sourceInternalPath)); + } + fclose($source); + fclose($target); + } + return $result; + } + + /** + * @param \OCP\Files\Storage $sourceStorage + * @param string $sourceInternalPath + * @param string $targetInternalPath + * @return bool + */ + public function moveFromStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath) { + $result = $this->copyFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath, true); + if ($result) { + if ($sourceStorage->is_dir($sourceInternalPath)) { + $sourceStorage->rmdir($sourceInternalPath); + } else { + $sourceStorage->unlink($sourceInternalPath); + } + } + return $result; + } } diff --git a/lib/private/files/storage/wrapper/wrapper.php b/lib/private/files/storage/wrapper/wrapper.php index 6550313f71..2552c926e0 100644 --- a/lib/private/files/storage/wrapper/wrapper.php +++ b/lib/private/files/storage/wrapper/wrapper.php @@ -505,4 +505,24 @@ class Wrapper implements \OC\Files\Storage\Storage { public function verifyPath($path, $fileName) { $this->storage->verifyPath($path, $fileName); } + + /** + * @param \OCP\Files\Storage $sourceStorage + * @param string $sourceInternalPath + * @param string $targetInternalPath + * @return bool + */ + public function copyFromStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath) { + return $this->storage->copyFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); + } + + /** + * @param \OCP\Files\Storage $sourceStorage + * @param string $sourceInternalPath + * @param string $targetInternalPath + * @return bool + */ + public function moveFromStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath) { + return $this->storage->moveFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); + } } diff --git a/lib/private/files/view.php b/lib/private/files/view.php index 475a9d35af..84164e2a24 100644 --- a/lib/private/files/view.php +++ b/lib/private/files/view.php @@ -584,8 +584,6 @@ class View { * @return bool|mixed */ public function rename($path1, $path2) { - $postFix1 = (substr($path1, -1, 1) === '/') ? '/' : ''; - $postFix2 = (substr($path2, -1, 1) === '/') ? '/' : ''; $absolutePath1 = Filesystem::normalizePath($this->getAbsolutePath($path1)); $absolutePath2 = Filesystem::normalizePath($this->getAbsolutePath($path2)); if ( @@ -620,7 +618,7 @@ class View { $mount1 = $this->getMount($path1); $mount2 = $this->getMount($path2); $storage1 = $mount1->getStorage(); - $storage2 = $mount1->getStorage(); + $storage2 = $mount2->getStorage(); $internalPath1 = $mount1->getInternalPath($absolutePath1); $internalPath2 = $mount2->getInternalPath($absolutePath2); @@ -642,34 +640,9 @@ class View { $result = false; } } else { - if ($this->is_dir($path1)) { - $result = $this->copy($path1, $path2, true); - if ($result === true) { - $result = $storage1->rmdir($internalPath1); - } - } else { - $source = $this->fopen($path1 . $postFix1, 'r'); - $target = $this->fopen($path2 . $postFix2, 'w'); - list(, $result) = \OC_Helper::streamCopy($source, $target); - if ($result !== false) { - $this->touch($path2, $this->filemtime($path1)); - } - - // close open handle - especially $source is necessary because unlink below will - // throw an exception on windows because the file is locked - fclose($source); - fclose($target); - - if ($result !== false) { - $result &= $storage1->unlink($internalPath1); - } else { - // delete partially written target file - $storage2->unlink($internalPath2); - // delete cache entry that was created by fopen - $storage2->getCache()->remove($internalPath2); - } - } + $result = $storage2->moveFromStorage($storage1, $internalPath1, $internalPath2); } + \OC_FileProxy::runPostProxies('rename', $absolutePath1, $absolutePath2); if ((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 $this->updater->update($path2); @@ -708,8 +681,6 @@ class View { * @return bool|mixed */ public function copy($path1, $path2, $preserveMtime = false) { - $postFix1 = (substr($path1, -1, 1) === '/') ? '/' : ''; - $postFix2 = (substr($path2, -1, 1) === '/') ? '/' : ''; $absolutePath1 = Filesystem::normalizePath($this->getAbsolutePath($path1)); $absolutePath2 = Filesystem::normalizePath($this->getAbsolutePath($path2)); if ( @@ -738,52 +709,20 @@ class View { $this->emit_file_hooks_pre($exists, $path2, $run); } if ($run) { - $mp1 = $this->getMountPoint($path1 . $postFix1); - $mp2 = $this->getMountPoint($path2 . $postFix2); - if ($mp1 == $mp2) { - list($storage, $internalPath1) = Filesystem::resolvePath($absolutePath1 . $postFix1); - list(, $internalPath2) = Filesystem::resolvePath($absolutePath2 . $postFix2); - if ($storage) { - $result = $storage->copy($internalPath1, $internalPath2); - if (!$result) { - // delete partially written target file - $storage->unlink($internalPath2); - $storage->getCache()->remove($internalPath2); - } + $mount1 = $this->getMount($path1); + $mount2 = $this->getMount($path2); + $storage1 = $mount1->getStorage(); + $internalPath1 = $mount1->getInternalPath($absolutePath1); + $storage2 = $mount2->getStorage(); + $internalPath2 = $mount2->getInternalPath($absolutePath2); + if ($mount1->getMountPoint() == $mount2->getMountPoint()) { + if ($storage1) { + $result = $storage1->copy($internalPath1, $internalPath2); } else { $result = false; } } else { - if ($this->is_dir($path1) && ($dh = $this->opendir($path1))) { - $result = $this->mkdir($path2); - if ($preserveMtime) { - $this->touch($path2, $this->filemtime($path1)); - } - if (is_resource($dh)) { - while (($file = readdir($dh)) !== false) { - if (!Filesystem::isIgnoredDir($file)) { - if (!$this->copy($path1 . '/' . $file, $path2 . '/' . $file, $preserveMtime)) { - $result = false; - } - } - } - } - } else { - list($storage2, $internalPath2) = Filesystem::resolvePath($absolutePath2 . $postFix2); - $source = $this->fopen($path1 . $postFix1, 'r'); - $target = $this->fopen($path2 . $postFix2, 'w'); - list(, $result) = \OC_Helper::streamCopy($source, $target); - if($result && $preserveMtime) { - $this->touch($path2, $this->filemtime($path1)); - } - fclose($source); - fclose($target); - if (!$result) { - // delete partially written target file - $storage2->unlink($internalPath2); - $storage2->getCache()->remove($internalPath2); - } - } + $result = $storage2->copyFromStorage($storage1, $internalPath1, $internalPath2); } $this->updater->update($path2); if ($this->shouldEmitHooks() && $result !== false) { diff --git a/lib/public/files/storage.php b/lib/public/files/storage.php index 8a20eff2d9..bac2c95ebc 100644 --- a/lib/public/files/storage.php +++ b/lib/public/files/storage.php @@ -358,4 +358,20 @@ interface Storage { * @throws InvalidPathException */ public function verifyPath($path, $fileName); + + /** + * @param \OCP\Files\Storage $sourceStorage + * @param string $sourceInternalPath + * @param string $targetInternalPath + * @return bool + */ + public function copyFromStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath); + + /** + * @param \OCP\Files\Storage $sourceStorage + * @param string $sourceInternalPath + * @param string $targetInternalPath + * @return bool + */ + public function moveFromStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath); } From 31e94708f86e21174e4bcfaf33f645dbba1efc1e Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 16 Jan 2015 13:33:17 +0100 Subject: [PATCH 03/12] Improve cross storage copy between local storages --- lib/private/files/storage/local.php | 36 +++++++++++++++ tests/lib/files/view.php | 69 ++++++++++++++++++++++++++++- 2 files changed, 103 insertions(+), 2 deletions(-) diff --git a/lib/private/files/storage/local.php b/lib/private/files/storage/local.php index 6bd9b4401d..8815361bc1 100644 --- a/lib/private/files/storage/local.php +++ b/lib/private/files/storage/local.php @@ -353,5 +353,41 @@ if (\OC_Util::runningOnWindows()) { return parent::getETag($path); } } + + /** + * @param \OCP\Files\Storage $sourceStorage + * @param string $sourceInternalPath + * @param string $targetInternalPath + * @return bool + */ + public function copyFromStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath) { + if($sourceStorage->instanceOfStorage('\OC\Files\Storage\Local')){ + /** + * @var \OC\Files\Storage\Local $sourceStorage + */ + $rootStorage = new Local(['datadir' => '/']); + return $rootStorage->copy($sourceStorage->getSourcePath($sourceInternalPath), $this->getSourcePath($targetInternalPath)); + } else { + return parent::copyFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); + } + } + + /** + * @param \OCP\Files\Storage $sourceStorage + * @param string $sourceInternalPath + * @param string $targetInternalPath + * @return bool + */ + public function moveFromStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath) { + if ($sourceStorage->instanceOfStorage('\OC\Files\Storage\Local')) { + /** + * @var \OC\Files\Storage\Local $sourceStorage + */ + $rootStorage = new Local(['datadir' => '/']); + return $rootStorage->rename($sourceStorage->getSourcePath($sourceInternalPath), $this->getSourcePath($targetInternalPath)); + } else { + return parent::moveFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); + } + } } } diff --git a/tests/lib/files/view.php b/tests/lib/files/view.php index 2ea9e8de78..3b5e3aa3b0 100644 --- a/tests/lib/files/view.php +++ b/tests/lib/files/view.php @@ -8,6 +8,7 @@ namespace Test\Files; use OC\Files\Cache\Watcher; +use OC\Files\Storage\Common; use OC\Files\Mount\MountPoint; use OC\Files\Storage\Temporary; @@ -17,6 +18,26 @@ class TemporaryNoTouch extends \OC\Files\Storage\Temporary { } } +class TemporaryNoCross extends \OC\Files\Storage\Temporary { + public function copyFromStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath) { + return Common::copyFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); + } + + public function moveFromStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath) { + return Common::moveFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); + } +} + +class TemporaryNoLocal extends \OC\Files\Storage\Temporary { + public function instanceOfStorage($className) { + if($className === '\OC\Files\Storage\Local') { + return false; + } else { + return parent::instanceOfStorage($className); + } + } +} + class View extends \Test\TestCase { /** * @var \OC\Files\Storage\Storage[] $storages @@ -291,9 +312,31 @@ class View extends \Test\TestCase { /** * @medium */ - function testCopyBetweenStorages() { + function testCopyBetweenStorageNoCross() { + $storage1 = $this->getTestStorage(true, '\Test\Files\TemporaryNoCross'); + $storage2 = $this->getTestStorage(true, '\Test\Files\TemporaryNoCross'); + $this->copyBetweenStorages($storage1, $storage2); + } + + /** + * @medium + */ + function testCopyBetweenStorageCross() { $storage1 = $this->getTestStorage(); $storage2 = $this->getTestStorage(); + $this->copyBetweenStorages($storage1, $storage2); + } + + /** + * @medium + */ + function testCopyBetweenStorageCrossNonLocal() { + $storage1 = $this->getTestStorage(true, '\Test\Files\TemporaryNoLocal'); + $storage2 = $this->getTestStorage(true, '\Test\Files\TemporaryNoLocal'); + $this->copyBetweenStorages($storage1, $storage2); + } + + function copyBetweenStorages($storage1, $storage2) { \OC\Files\Filesystem::mount($storage1, array(), '/'); \OC\Files\Filesystem::mount($storage2, array(), '/substorage'); @@ -315,9 +358,31 @@ class View extends \Test\TestCase { /** * @medium */ - function testMoveBetweenStorages() { + function testMoveBetweenStorageNoCross() { + $storage1 = $this->getTestStorage(true, '\Test\Files\TemporaryNoCross'); + $storage2 = $this->getTestStorage(true, '\Test\Files\TemporaryNoCross'); + $this->moveBetweenStorages($storage1, $storage2); + } + + /** + * @medium + */ + function testMoveBetweenStorageCross() { $storage1 = $this->getTestStorage(); $storage2 = $this->getTestStorage(); + $this->moveBetweenStorages($storage1, $storage2); + } + + /** + * @medium + */ + function testMoveBetweenStorageCrossNonLocal() { + $storage1 = $this->getTestStorage(true, '\Test\Files\TemporaryNoLocal'); + $storage2 = $this->getTestStorage(true, '\Test\Files\TemporaryNoLocal'); + $this->moveBetweenStorages($storage1, $storage2); + } + + function moveBetweenStorages($storage1, $storage2) { \OC\Files\Filesystem::mount($storage1, array(), '/'); \OC\Files\Filesystem::mount($storage2, array(), '/substorage'); From d726db74590a35529dadbde45a375b8f50d9ec34 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 19 Jan 2015 13:44:37 +0100 Subject: [PATCH 04/12] Implement move/copyFromStorage for shared storage --- apps/files_sharing/lib/sharedstorage.php | 28 ++++++++++ apps/files_sharing/tests/sharedstorage.php | 64 ++++++++++++++++++++-- 2 files changed, 86 insertions(+), 6 deletions(-) diff --git a/apps/files_sharing/lib/sharedstorage.php b/apps/files_sharing/lib/sharedstorage.php index 6e3abb1f56..a339637552 100644 --- a/apps/files_sharing/lib/sharedstorage.php +++ b/apps/files_sharing/lib/sharedstorage.php @@ -583,4 +583,32 @@ class Shared extends \OC\Files\Storage\Common implements ISharedStorage { return $result; } + private function resolvePath($path) { + $source = $this->getSourcePath($path); + return \OC\Files\Filesystem::resolvePath($source); + } + + /** + * @param \OCP\Files\Storage $sourceStorage + * @param string $sourceInternalPath + * @param string $targetInternalPath + * @return bool + */ + public function copyFromStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath) { + /** @var \OCP\Files\Storage $targetStorage */ + list($targetStorage, $targetInternalPath) = $this->resolvePath($targetInternalPath); + $targetStorage->copyFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); + } + + /** + * @param \OCP\Files\Storage $sourceStorage + * @param string $sourceInternalPath + * @param string $targetInternalPath + * @return bool + */ + public function moveFromStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath) { + /** @var \OCP\Files\Storage $targetStorage */ + list($targetStorage, $targetInternalPath) = $this->resolvePath($targetInternalPath); + $targetStorage->moveFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); + } } diff --git a/apps/files_sharing/tests/sharedstorage.php b/apps/files_sharing/tests/sharedstorage.php index a8ce92775d..14afdcf9cd 100644 --- a/apps/files_sharing/tests/sharedstorage.php +++ b/apps/files_sharing/tests/sharedstorage.php @@ -117,21 +117,21 @@ class Test_Files_Sharing_Storage extends OCA\Files_sharing\Tests\TestCase { $this->assertTrue($user2View->file_exists($this->folder)); // create part file - $result = $user2View->file_put_contents($this->folder. '/foo.txt.part', 'some test data'); + $result = $user2View->file_put_contents($this->folder . '/foo.txt.part', 'some test data'); $this->assertTrue(is_int($result)); // rename part file to real file - $result = $user2View->rename($this->folder. '/foo.txt.part', $this->folder. '/foo.txt'); + $result = $user2View->rename($this->folder . '/foo.txt.part', $this->folder . '/foo.txt'); $this->assertTrue($result); // check if the new file really exists - $this->assertTrue($user2View->file_exists( $this->folder. '/foo.txt')); + $this->assertTrue($user2View->file_exists($this->folder . '/foo.txt')); // check if the rename also affected the owner self::loginHelper(self::TEST_FILES_SHARING_API_USER1); - $this->assertTrue($this->view->file_exists( $this->folder. '/foo.txt')); + $this->assertTrue($this->view->file_exists($this->folder . '/foo.txt')); //cleanup \OCP\Share::unshare('folder', $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_USER, @@ -144,7 +144,7 @@ class Test_Files_Sharing_Storage extends OCA\Files_sharing\Tests\TestCase { $fileinfoFile = $this->view->getFileInfo($this->filename); $folderSize = $this->view->filesize($this->folder); - $file1Size = $this->view->filesize($this->folder. $this->filename); + $file1Size = $this->view->filesize($this->folder . $this->filename); $file2Size = $this->view->filesize($this->filename); $result = \OCP\Share::shareItem('folder', $fileinfoFolder['fileid'], \OCP\Share::SHARE_TYPE_USER, @@ -373,11 +373,63 @@ class Test_Files_Sharing_Storage extends OCA\Files_sharing\Tests\TestCase { $this->assertTrue($rootView->file_exists('/' . self::TEST_FILES_SHARING_API_USER3 . '/files/' . $this->filename)); // make sure we didn't double setup shares for user 2 or mounted the shares for user 3 in user's 2 home - $this->assertFalse($rootView->file_exists('/' . self::TEST_FILES_SHARING_API_USER2 . '/files/' . $this->folder .' (2)')); + $this->assertFalse($rootView->file_exists('/' . self::TEST_FILES_SHARING_API_USER2 . '/files/' . $this->folder . ' (2)')); $this->assertFalse($rootView->file_exists('/' . self::TEST_FILES_SHARING_API_USER2 . '/files/' . $this->filename)); //cleanup self::loginHelper(self::TEST_FILES_SHARING_API_USER1); $this->view->unlink($this->folder); } + + public function testCopyFromStorage() { + $folderInfo = $this->view->getFileInfo($this->folder); + self::loginHelper(self::TEST_FILES_SHARING_API_USER1); + + // share 2 different files with 2 different users + \OCP\Share::shareItem('folder', $folderInfo['fileid'], \OCP\Share::SHARE_TYPE_USER, + self::TEST_FILES_SHARING_API_USER2, 31); + + self::loginHelper(self::TEST_FILES_SHARING_API_USER2); + $view = new \OC\Files\View('/' . self::TEST_FILES_SHARING_API_USER2 . '/files'); + $this->assertTrue($view->file_exists($this->folder)); + + /** + * @var \OCP\Files\Storage $sharedStorage + */ + list($sharedStorage,) = $view->resolvePath($this->folder); + $this->assertInstanceOf('OCA\Files_Sharing\ISharedStorage', $sharedStorage); + + $sourceStorage = new \OC\Files\Storage\Temporary(array()); + $sourceStorage->file_put_contents('foo.txt', 'asd'); + + $sharedStorage->copyFromStorage($sourceStorage, 'foo.txt', 'bar.txt'); + $this->assertTrue($sharedStorage->file_exists('bar.txt')); + $this->assertEquals('asd', $sharedStorage->file_get_contents('bar.txt')); + } + + public function testMoveFromStorage() { + $folderInfo = $this->view->getFileInfo($this->folder); + self::loginHelper(self::TEST_FILES_SHARING_API_USER1); + + // share 2 different files with 2 different users + \OCP\Share::shareItem('folder', $folderInfo['fileid'], \OCP\Share::SHARE_TYPE_USER, + self::TEST_FILES_SHARING_API_USER2, 31); + + self::loginHelper(self::TEST_FILES_SHARING_API_USER2); + $view = new \OC\Files\View('/' . self::TEST_FILES_SHARING_API_USER2 . '/files'); + $this->assertTrue($view->file_exists($this->folder)); + + /** + * @var \OCP\Files\Storage $sharedStorage + */ + list($sharedStorage,) = $view->resolvePath($this->folder); + $this->assertInstanceOf('OCA\Files_Sharing\ISharedStorage', $sharedStorage); + + $sourceStorage = new \OC\Files\Storage\Temporary(array()); + $sourceStorage->file_put_contents('foo.txt', 'asd'); + + $sharedStorage->moveFromStorage($sourceStorage, 'foo.txt', 'bar.txt'); + $this->assertTrue($sharedStorage->file_exists('bar.txt')); + $this->assertEquals('asd', $sharedStorage->file_get_contents('bar.txt')); + } } From d16ee4138d80c757485b67abad4ecaa8146cc1a3 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 19 Jan 2015 16:23:33 +0100 Subject: [PATCH 05/12] Fix test --- apps/files_sharing/tests/sharedstorage.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/apps/files_sharing/tests/sharedstorage.php b/apps/files_sharing/tests/sharedstorage.php index 14afdcf9cd..a1469a7468 100644 --- a/apps/files_sharing/tests/sharedstorage.php +++ b/apps/files_sharing/tests/sharedstorage.php @@ -397,7 +397,7 @@ class Test_Files_Sharing_Storage extends OCA\Files_sharing\Tests\TestCase { * @var \OCP\Files\Storage $sharedStorage */ list($sharedStorage,) = $view->resolvePath($this->folder); - $this->assertInstanceOf('OCA\Files_Sharing\ISharedStorage', $sharedStorage); + $this->assertTrue($sharedStorage->instanceOfStorage('OCA\Files_Sharing\ISharedStorage')); $sourceStorage = new \OC\Files\Storage\Temporary(array()); $sourceStorage->file_put_contents('foo.txt', 'asd'); @@ -405,6 +405,9 @@ class Test_Files_Sharing_Storage extends OCA\Files_sharing\Tests\TestCase { $sharedStorage->copyFromStorage($sourceStorage, 'foo.txt', 'bar.txt'); $this->assertTrue($sharedStorage->file_exists('bar.txt')); $this->assertEquals('asd', $sharedStorage->file_get_contents('bar.txt')); + + self::loginHelper(self::TEST_FILES_SHARING_API_USER1); + $this->view->unlink($this->folder); } public function testMoveFromStorage() { @@ -423,7 +426,7 @@ class Test_Files_Sharing_Storage extends OCA\Files_sharing\Tests\TestCase { * @var \OCP\Files\Storage $sharedStorage */ list($sharedStorage,) = $view->resolvePath($this->folder); - $this->assertInstanceOf('OCA\Files_Sharing\ISharedStorage', $sharedStorage); + $this->assertTrue($sharedStorage->instanceOfStorage('OCA\Files_Sharing\ISharedStorage')); $sourceStorage = new \OC\Files\Storage\Temporary(array()); $sourceStorage->file_put_contents('foo.txt', 'asd'); @@ -431,5 +434,8 @@ class Test_Files_Sharing_Storage extends OCA\Files_sharing\Tests\TestCase { $sharedStorage->moveFromStorage($sourceStorage, 'foo.txt', 'bar.txt'); $this->assertTrue($sharedStorage->file_exists('bar.txt')); $this->assertEquals('asd', $sharedStorage->file_get_contents('bar.txt')); + + self::loginHelper(self::TEST_FILES_SHARING_API_USER1); + $this->view->unlink($this->folder); } } From c4ec8fbeff926e459317829695af1354b4c8ad61 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 19 Jan 2015 16:25:28 +0100 Subject: [PATCH 06/12] Make getSourcePath accessible for storage wrappers --- lib/private/files/storage/local.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/files/storage/local.php b/lib/private/files/storage/local.php index 8815361bc1..175b33b032 100644 --- a/lib/private/files/storage/local.php +++ b/lib/private/files/storage/local.php @@ -322,7 +322,7 @@ if (\OC_Util::runningOnWindows()) { * @param string $path * @return string */ - protected function getSourcePath($path) { + public function getSourcePath($path) { $fullPath = $this->datadir . $path; return $fullPath; } From d26c6cab90b15bcf9cbd0d95015d8ad2405f7914 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 9 Feb 2015 15:15:03 +0100 Subject: [PATCH 07/12] properly return false if we cant delete the source file --- apps/files_trashbin/tests/storage.php | 9 +++++---- lib/private/files/storage/common.php | 4 ++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/apps/files_trashbin/tests/storage.php b/apps/files_trashbin/tests/storage.php index d5bd7c318d..d1468522dc 100644 --- a/apps/files_trashbin/tests/storage.php +++ b/apps/files_trashbin/tests/storage.php @@ -202,12 +202,13 @@ class Storage extends \Test\TestCase { $cache = $storage->getCache(); - Filesystem::mount($storage, [], '/' . $this->user . '/files'); + Filesystem::mount($storage, [], '/' . $this->user); + $storage->mkdir('files'); $this->userView->file_put_contents('test.txt', 'foo'); - $this->assertTrue($storage->file_exists('test.txt')); + $this->assertTrue($storage->file_exists('files/test.txt')); $this->assertFalse($this->userView->unlink('test.txt')); - $this->assertTrue($storage->file_exists('test.txt')); - $this->assertTrue($cache->inCache('test.txt')); + $this->assertTrue($storage->file_exists('files/test.txt')); + $this->assertTrue($cache->inCache('files/test.txt')); // file should not be in the trashbin $results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/files/'); diff --git a/lib/private/files/storage/common.php b/lib/private/files/storage/common.php index 164225de3e..ca898bcc0b 100644 --- a/lib/private/files/storage/common.php +++ b/lib/private/files/storage/common.php @@ -566,9 +566,9 @@ abstract class Common implements Storage { $result = $this->copyFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath, true); if ($result) { if ($sourceStorage->is_dir($sourceInternalPath)) { - $sourceStorage->rmdir($sourceInternalPath); + $result &= $sourceStorage->rmdir($sourceInternalPath); } else { - $sourceStorage->unlink($sourceInternalPath); + $result &= $sourceStorage->unlink($sourceInternalPath); } } return $result; From 404773940daf9c5a686b92d65dd39fb44d766050 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 5 Mar 2015 11:49:15 +0100 Subject: [PATCH 08/12] Detect storage full when doing cross storage copy/move --- lib/private/files/storage/wrapper/quota.php | 39 +++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/lib/private/files/storage/wrapper/quota.php b/lib/private/files/storage/wrapper/quota.php index 3c0fda98da..92d749a814 100644 --- a/lib/private/files/storage/wrapper/quota.php +++ b/lib/private/files/storage/wrapper/quota.php @@ -55,9 +55,14 @@ class Quota extends Wrapper { /** * @param string $path + * @param \OC\Files\Storage\Storage $storage */ - protected function getSize($path) { - $cache = $this->getCache(); + protected function getSize($path, $storage = null) { + if (is_null($storage)) { + $cache = $this->getCache(); + } else { + $cache = $storage->getCache(); + } $data = $cache->get($path); if (is_array($data) and isset($data['size'])) { return $data['size']; @@ -141,4 +146,34 @@ class Quota extends Wrapper { return $source; } } + + /** + * @param \OCP\Files\Storage $sourceStorage + * @param string $sourceInternalPath + * @param string $targetInternalPath + * @return bool + */ + public function copyFromStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath) { + $free = $this->free_space(''); + if ($free < 0 or $this->getSize($sourceInternalPath, $sourceStorage) < $free) { + return $this->storage->copyFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); + } else { + return false; + } + } + + /** + * @param \OCP\Files\Storage $sourceStorage + * @param string $sourceInternalPath + * @param string $targetInternalPath + * @return bool + */ + public function moveFromStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath) { + $free = $this->free_space(''); + if ($free < 0 or $this->getSize($sourceInternalPath, $sourceStorage) < $free) { + return $this->storage->moveFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); + } else { + return false; + } + } } From 0772e3b4c11a387dfcd774caf1018d73106cd064 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 5 Mar 2015 11:49:31 +0100 Subject: [PATCH 09/12] Properly handle copy/move failures in cross storage copy/move --- lib/private/files/storage/common.php | 13 ++++++++++--- tests/lib/files/view.php | 19 ++++++++++++++----- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/lib/private/files/storage/common.php b/lib/private/files/storage/common.php index ca898bcc0b..66ed713e22 100644 --- a/lib/private/files/storage/common.php +++ b/lib/private/files/storage/common.php @@ -537,7 +537,7 @@ abstract class Common implements Storage { $dh = $sourceStorage->opendir($sourceInternalPath); $result = $this->mkdir($targetInternalPath); if (is_resource($dh)) { - while (($file = readdir($dh)) !== false) { + while ($result and ($file = readdir($dh)) !== false) { if (!Filesystem::isIgnoredDir($file)) { $result &= $this->copyFromStorage($sourceStorage, $sourceInternalPath . '/' . $file, $targetInternalPath . '/' . $file); } @@ -547,13 +547,20 @@ abstract class Common implements Storage { $source = $sourceStorage->fopen($sourceInternalPath, 'r'); $target = $this->fopen($targetInternalPath, 'w'); list(, $result) = \OC_Helper::streamCopy($source, $target); - if ($preserveMtime) { + if ($result and $preserveMtime) { $this->touch($targetInternalPath, $sourceStorage->filemtime($sourceInternalPath)); } fclose($source); fclose($target); + + if (!$result) { + // delete partially written target file + $this->unlink($targetInternalPath); + // delete cache entry that was created by fopen + $this->getCache()->remove($targetInternalPath); + } } - return $result; + return (bool)$result; } /** diff --git a/tests/lib/files/view.php b/tests/lib/files/view.php index 3b5e3aa3b0..269b8b23e7 100644 --- a/tests/lib/files/view.php +++ b/tests/lib/files/view.php @@ -944,8 +944,19 @@ class View extends \Test\TestCase { private function doTestCopyRenameFail($operation) { $storage1 = new Temporary(array()); - $storage2 = new Temporary(array()); - $storage2 = new \OC\Files\Storage\Wrapper\Quota(array('storage' => $storage2, 'quota' => 9)); + /** @var \PHPUnit_Framework_MockObject_MockObject | \OC\Files\Storage\Temporary $storage2 */ + $storage2 = $this->getMockBuilder('\Test\Files\TemporaryNoCross') + ->setConstructorArgs([[]]) + ->setMethods(['fopen']) + ->getMock(); + + $storage2->expects($this->any()) + ->method('fopen') + ->will($this->returnCallback(function($path, $mode) use($storage2) { + $source = fopen($storage2->getSourcePath($path), $mode); + return \OC\Files\Stream\Quota::wrap($source, 9); + })); + $storage1->mkdir('sub'); $storage1->file_put_contents('foo.txt', '0123456789ABCDEFGH'); $storage1->mkdir('dirtomove'); @@ -976,10 +987,8 @@ class View extends \Test\TestCase { $this->assertFalse($view->$operation('/test/dirtomove/', '/test/sub/storage/dirtomove/')); // since the move failed, the full source tree is kept $this->assertTrue($storage1->file_exists('dirtomove/indir1.txt')); - // but the target file stays - $this->assertTrue($storage2->file_exists('dirtomove/indir1.txt')); - // second file not moved/copied $this->assertTrue($storage1->file_exists('dirtomove/indir2.txt')); + // second file not moved/copied $this->assertFalse($storage2->file_exists('dirtomove/indir2.txt')); $this->assertFalse($storage2->getCache()->get('dirtomove/indir2.txt')); From d4c91dc8356a3514487e2357c1882cdf9eaa2090 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 19 Mar 2015 15:26:47 +0100 Subject: [PATCH 10/12] add missing return for shared --- apps/files_sharing/lib/sharedstorage.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/files_sharing/lib/sharedstorage.php b/apps/files_sharing/lib/sharedstorage.php index a339637552..ce8758a6d8 100644 --- a/apps/files_sharing/lib/sharedstorage.php +++ b/apps/files_sharing/lib/sharedstorage.php @@ -597,7 +597,7 @@ class Shared extends \OC\Files\Storage\Common implements ISharedStorage { public function copyFromStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath) { /** @var \OCP\Files\Storage $targetStorage */ list($targetStorage, $targetInternalPath) = $this->resolvePath($targetInternalPath); - $targetStorage->copyFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); + return $targetStorage->copyFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); } /** @@ -609,6 +609,6 @@ class Shared extends \OC\Files\Storage\Common implements ISharedStorage { public function moveFromStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath) { /** @var \OCP\Files\Storage $targetStorage */ list($targetStorage, $targetInternalPath) = $this->resolvePath($targetInternalPath); - $targetStorage->moveFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); + return $targetStorage->moveFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); } } From c29419e6d7e5a47a063a598dc2300275e344f893 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 7 Apr 2015 17:53:40 +0200 Subject: [PATCH 11/12] fix rebase issue --- lib/private/files/view.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/private/files/view.php b/lib/private/files/view.php index 84164e2a24..313237c5f5 100644 --- a/lib/private/files/view.php +++ b/lib/private/files/view.php @@ -614,7 +614,8 @@ class View { } if ($run) { $this->verifyPath(dirname($path2), basename($path2)); - + + $manager = Filesystem::getMountManager(); $mount1 = $this->getMount($path1); $mount2 = $this->getMount($path2); $storage1 = $mount1->getStorage(); @@ -642,7 +643,6 @@ class View { } else { $result = $storage2->moveFromStorage($storage1, $internalPath1, $internalPath2); } - \OC_FileProxy::runPostProxies('rename', $absolutePath1, $absolutePath2); if ((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 $this->updater->update($path2); From 0f21303b751291188733e24b5f213053ea96a368 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 14 Apr 2015 12:35:53 +0200 Subject: [PATCH 12/12] a bit more phpdoc --- apps/files_sharing/lib/sharedstorage.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/apps/files_sharing/lib/sharedstorage.php b/apps/files_sharing/lib/sharedstorage.php index ce8758a6d8..ffaa542c87 100644 --- a/apps/files_sharing/lib/sharedstorage.php +++ b/apps/files_sharing/lib/sharedstorage.php @@ -583,6 +583,12 @@ class Shared extends \OC\Files\Storage\Common implements ISharedStorage { return $result; } + /** + * Resolve the path for the source of the share + * + * @param string $path + * @return array + */ private function resolvePath($path) { $source = $this->getSourcePath($path); return \OC\Files\Filesystem::resolvePath($source);