From 9d1be0bbaf7d8af442fc8d908baff743cc823a98 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 28 Oct 2014 15:57:08 +0100 Subject: [PATCH 1/3] get the source path and owner in a pre hook and the target path and owner in a post hook --- apps/files_versions/lib/hooks.php | 33 ++++++++++++--- apps/files_versions/lib/versions.php | 46 +++++++++++++++++--- apps/files_versions/tests/versions.php | 58 +++++++++++++++++++++++++- 3 files changed, 124 insertions(+), 13 deletions(-) diff --git a/apps/files_versions/lib/hooks.php b/apps/files_versions/lib/hooks.php index 1a584232ba..024cb6a3c3 100644 --- a/apps/files_versions/lib/hooks.php +++ b/apps/files_versions/lib/hooks.php @@ -16,12 +16,14 @@ class Hooks { public static function connectHooks() { // Listen to write signals - \OCP\Util::connectHook('OC_Filesystem', 'write', "OCA\Files_Versions\Hooks", "write_hook"); + \OCP\Util::connectHook('OC_Filesystem', 'write', 'OCA\Files_Versions\Hooks', 'write_hook'); // Listen to delete and rename signals - \OCP\Util::connectHook('OC_Filesystem', 'post_delete', "OCA\Files_Versions\Hooks", "remove_hook"); - \OCP\Util::connectHook('OC_Filesystem', 'delete', "OCA\Files_Versions\Hooks", "pre_remove_hook"); - \OCP\Util::connectHook('OC_Filesystem', 'rename', "OCA\Files_Versions\Hooks", "rename_hook"); - \OCP\Util::connectHook('OC_Filesystem', 'copy', "OCA\Files_Versions\Hooks", "copy_hook"); + \OCP\Util::connectHook('OC_Filesystem', 'post_delete', 'OCA\Files_Versions\Hooks', 'remove_hook'); + \OCP\Util::connectHook('OC_Filesystem', 'delete', 'OCA\Files_Versions\Hooks', 'pre_remove_hook'); + \OCP\Util::connectHook('OC_Filesystem', 'post_rename', 'OCA\Files_Versions\Hooks', 'rename_hook'); + \OCP\Util::connectHook('OC_Filesystem', 'post_copy', 'OCA\Files_Versions\Hooks', 'copy_hook'); + \OCP\Util::connectHook('OC_Filesystem', 'rename', 'OCA\Files_Versions\Hooks', 'pre_renameOrCopy_hook'); + \OCP\Util::connectHook('OC_Filesystem', 'copy', 'OCA\Files_Versions\Hooks', 'pre_renameOrCopy_hook'); } /** @@ -102,4 +104,25 @@ class Hooks { } } + /** + * Remember owner and the owner path of the source file. + * If the file already exists, then it was a upload of a existing file + * over the web interface and we call Storage::store() directly + * + * @param array $params array with oldpath and newpath + * + */ + public static function pre_renameOrCopy_hook($params) { + if (\OCP\App::isEnabled('files_versions')) { + + $view = new \OC\Files\View(\OCP\User::getUser() . '/files'); + if ($view->file_exists($params['newpath'])) { + Storage::store($params['newpath']); + } else { + Storage::setSourcePathAndUser($params['oldpath']); + } + + } + } + } diff --git a/apps/files_versions/lib/versions.php b/apps/files_versions/lib/versions.php index bdb2689694..35e79569cd 100644 --- a/apps/files_versions/lib/versions.php +++ b/apps/files_versions/lib/versions.php @@ -24,6 +24,8 @@ class Storage { // files for which we can remove the versions after the delete operation was successful private static $deletedFiles = array(); + private static $sourcePathAndUser = array(); + private static $max_versions_per_interval = array( //first 10sec, one version every 2sec 1 => array('intervalEndsAfter' => 10, 'step' => 2), @@ -50,6 +52,34 @@ class Storage { return array($uid, $filename); } + /** + * remeber the owner and the owner path of the source file + * + * @param string $source source path + */ + public static function setSourcePathAndUser($source) { + list($uid, $path) = self::getUidAndFilename($source); + self::$sourcePathAndUser[$source] = array('uid' => $uid, 'path' => $path); + } + + /** + * gets the owner and the owner path from the source path + * + * @param string $source source path + * @return array with user id and path + */ + public static function getSourcePathAndUser($source) { + + if (isset(self::$sourcePathAndUser[$source])) { + $uid = self::$sourcePathAndUser[$source]['uid']; + $path = self::$sourcePathAndUser[$source]['path']; + unset(self::$sourcePathAndUser[$source]); + } else { + $uid = $path = false; + } + return array($uid, $path); + } + /** * get current size of all versions from a given user * @@ -180,16 +210,20 @@ class Storage { * @param string $operation can be 'copy' or 'rename' */ public static function renameOrCopy($old_path, $new_path, $operation) { - list($uid, $oldpath) = self::getUidAndFilename($old_path); + list($uid, $oldpath) = self::getSourcePathAndUser($old_path); + + // it was a upload of a existing file if no old path exists + // in this case the pre-hook already called the store method and we can + // stop here + if ($oldpath === false) { + return true; + } + list($uidn, $newpath) = self::getUidAndFilename($new_path); $versions_view = new \OC\Files\View('/'.$uid .'/files_versions'); $files_view = new \OC\Files\View('/'.$uid .'/files'); - // if the file already exists than it was a upload of a existing file - // over the web interface -> store() is the right function we need here - if ($files_view->file_exists($newpath)) { - return self::store($new_path); - } + if ( $files_view->is_dir($oldpath) && $versions_view->is_dir($oldpath) ) { $versions_view->$operation($oldpath, $newpath); diff --git a/apps/files_versions/tests/versions.php b/apps/files_versions/tests/versions.php index 558c8dfcb8..6dfba6bcf2 100644 --- a/apps/files_versions/tests/versions.php +++ b/apps/files_versions/tests/versions.php @@ -30,18 +30,28 @@ require_once __DIR__ . '/../lib/versions.php'; class Test_Files_Versioning extends \PHPUnit_Framework_TestCase { const TEST_VERSIONS_USER = 'test-versions-user'; + const TEST_VERSIONS_USER2 = 'test-versions-user2'; const USERS_VERSIONS_ROOT = '/test-versions-user/files_versions'; private $rootView; public static function setUpBeforeClass() { + + // clear share hooks + \OC_Hook::clear('OCP\\Share'); + \OC::registerShareHooks(); + \OCA\Files_Versions\Hooks::connectHooks(); + \OCP\Util::connectHook('OC_Filesystem', 'setup', '\OC\Files\Storage\Shared', 'setup'); + // create test user + self::loginHelper(self::TEST_VERSIONS_USER2, true); self::loginHelper(self::TEST_VERSIONS_USER, true); } public static function tearDownAfterClass() { // cleanup test user \OC_User::deleteUser(self::TEST_VERSIONS_USER); + \OC_User::deleteUser(self::TEST_VERSIONS_USER2); } function setUp() { @@ -222,7 +232,7 @@ class Test_Files_Versioning extends \PHPUnit_Framework_TestCase { $this->rootView->file_put_contents($v2, 'version2'); // execute rename hook of versions app - \OCA\Files_Versions\Storage::renameOrCopy("test.txt", "test2.txt", 'rename'); + \OC\Files\Filesystem::rename("test.txt", "test2.txt"); $this->assertFalse($this->rootView->file_exists($v1)); $this->assertFalse($this->rootView->file_exists($v2)); @@ -234,6 +244,50 @@ class Test_Files_Versioning extends \PHPUnit_Framework_TestCase { \OC\Files\Filesystem::unlink('test2.txt'); } + function testRenameInSharedFolder() { + + \OC\Files\Filesystem::mkdir('folder1'); + \OC\Files\Filesystem::mkdir('folder1/folder2'); + \OC\Files\Filesystem::file_put_contents("folder1/test.txt", "test file"); + + $fileInfo = \OC\Files\Filesystem::getFileInfo('folder1'); + + $t1 = time(); + // second version is two weeks older, this way we make sure that no + // version will be expired + $t2 = $t1 - 60 * 60 * 24 * 14; + + $this->rootView->mkdir(self::USERS_VERSIONS_ROOT . '/folder1'); + // create some versions + $v1 = self::USERS_VERSIONS_ROOT . '/folder1/test.txt.v' . $t1; + $v2 = self::USERS_VERSIONS_ROOT . '/folder1/test.txt.v' . $t2; + $v1Renamed = self::USERS_VERSIONS_ROOT . '/folder1/folder2/test.txt.v' . $t1; + $v2Renamed = self::USERS_VERSIONS_ROOT . '/folder1/folder2/test.txt.v' . $t2; + + $this->rootView->file_put_contents($v1, 'version1'); + $this->rootView->file_put_contents($v2, 'version2'); + + \OCP\Share::shareItem('folder', $fileInfo['fileid'], \OCP\Share::SHARE_TYPE_USER, self::TEST_VERSIONS_USER2, OCP\PERMISSION_ALL); + + self::loginHelper(self::TEST_VERSIONS_USER2); + + $this->assertTrue(\OC\Files\Filesystem::file_exists('folder1/test.txt')); + + // execute rename hook of versions app + \OC\Files\Filesystem::rename('/folder1/test.txt', '/folder1/folder2/test.txt'); + + self::loginHelper(self::TEST_VERSIONS_USER2); + + $this->assertFalse($this->rootView->file_exists($v1)); + $this->assertFalse($this->rootView->file_exists($v2)); + + $this->assertTrue($this->rootView->file_exists($v1Renamed)); + $this->assertTrue($this->rootView->file_exists($v2Renamed)); + + //cleanup + \OC\Files\Filesystem::unlink('/folder1/folder2/test.txt'); + } + function testCopy() { \OC\Files\Filesystem::file_put_contents("test.txt", "test file"); @@ -253,7 +307,7 @@ class Test_Files_Versioning extends \PHPUnit_Framework_TestCase { $this->rootView->file_put_contents($v2, 'version2'); // execute copy hook of versions app - \OCA\Files_Versions\Storage::renameOrCopy("test.txt", "test2.txt", 'copy'); + \OC\Files\Filesystem::copy("test.txt", "test2.txt"); $this->assertTrue($this->rootView->file_exists($v1)); $this->assertTrue($this->rootView->file_exists($v2)); From 206cb5ba632baa70a0317f5412c306708e4788d1 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Tue, 28 Oct 2014 23:32:57 +0100 Subject: [PATCH 2/3] Fix typo --- apps/files_versions/lib/versions.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/files_versions/lib/versions.php b/apps/files_versions/lib/versions.php index 35e79569cd..82e0ecc3e2 100644 --- a/apps/files_versions/lib/versions.php +++ b/apps/files_versions/lib/versions.php @@ -53,7 +53,7 @@ class Storage { } /** - * remeber the owner and the owner path of the source file + * Remember the owner and the owner path of the source file * * @param string $source source path */ @@ -63,7 +63,7 @@ class Storage { } /** - * gets the owner and the owner path from the source path + * Gets the owner and the owner path from the source path * * @param string $source source path * @return array with user id and path From ebe1d3df0a51f3517ea14ea4d3ba9c770f4f85c1 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 31 Oct 2014 16:42:54 +0100 Subject: [PATCH 3/3] don't move versions if only the mount point was renamed --- apps/files_versions/lib/hooks.php | 10 ++++++ apps/files_versions/tests/versions.php | 43 ++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/apps/files_versions/lib/hooks.php b/apps/files_versions/lib/hooks.php index 024cb6a3c3..5398046312 100644 --- a/apps/files_versions/lib/hooks.php +++ b/apps/files_versions/lib/hooks.php @@ -115,6 +115,16 @@ class Hooks { public static function pre_renameOrCopy_hook($params) { if (\OCP\App::isEnabled('files_versions')) { + // if we rename a movable mount point, then the versions don't have + // to be renamed + $absOldPath = \OC\Files\Filesystem::normalizePath('/' . \OCP\User::getUser() . '/files' . $params['oldpath']); + $manager = \OC\Files\Filesystem::getMountManager(); + $mount = $manager->find($absOldPath); + $internalPath = $mount->getInternalPath($absOldPath); + if ($internalPath === '' and $mount instanceof \OC\Files\Mount\MoveableMount) { + return; + } + $view = new \OC\Files\View(\OCP\User::getUser() . '/files'); if ($view->file_exists($params['newpath'])) { Storage::store($params['newpath']); diff --git a/apps/files_versions/tests/versions.php b/apps/files_versions/tests/versions.php index 6dfba6bcf2..761cc07d9f 100644 --- a/apps/files_versions/tests/versions.php +++ b/apps/files_versions/tests/versions.php @@ -288,6 +288,49 @@ class Test_Files_Versioning extends \PHPUnit_Framework_TestCase { \OC\Files\Filesystem::unlink('/folder1/folder2/test.txt'); } + function testRenameSharedFile() { + + \OC\Files\Filesystem::file_put_contents("test.txt", "test file"); + + $fileInfo = \OC\Files\Filesystem::getFileInfo('test.txt'); + + $t1 = time(); + // second version is two weeks older, this way we make sure that no + // version will be expired + $t2 = $t1 - 60 * 60 * 24 * 14; + + $this->rootView->mkdir(self::USERS_VERSIONS_ROOT); + // create some versions + $v1 = self::USERS_VERSIONS_ROOT . '/test.txt.v' . $t1; + $v2 = self::USERS_VERSIONS_ROOT . '/test.txt.v' . $t2; + // the renamed versions should not exist! Because we only moved the mount point! + $v1Renamed = self::USERS_VERSIONS_ROOT . '/test2.txt.v' . $t1; + $v2Renamed = self::USERS_VERSIONS_ROOT . '/test2.txt.v' . $t2; + + $this->rootView->file_put_contents($v1, 'version1'); + $this->rootView->file_put_contents($v2, 'version2'); + + \OCP\Share::shareItem('file', $fileInfo['fileid'], \OCP\Share::SHARE_TYPE_USER, self::TEST_VERSIONS_USER2, OCP\PERMISSION_ALL); + + self::loginHelper(self::TEST_VERSIONS_USER2); + + $this->assertTrue(\OC\Files\Filesystem::file_exists('test.txt')); + + // execute rename hook of versions app + \OC\Files\Filesystem::rename('test.txt', 'test2.txt'); + + self::loginHelper(self::TEST_VERSIONS_USER); + + $this->assertTrue($this->rootView->file_exists($v1)); + $this->assertTrue($this->rootView->file_exists($v2)); + + $this->assertFalse($this->rootView->file_exists($v1Renamed)); + $this->assertFalse($this->rootView->file_exists($v2Renamed)); + + //cleanup + \OC\Files\Filesystem::unlink('/test.txt'); + } + function testCopy() { \OC\Files\Filesystem::file_put_contents("test.txt", "test file");