From d7b3a1a35a47dd37f68f7819aa0789b794e352e5 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 1 Apr 2015 15:12:59 +0200 Subject: [PATCH 1/7] preserve cache data when doing a cross storage move --- lib/private/files/cache/cache.php | 37 +++++++++++++++ lib/private/files/cache/updater.php | 45 +++++++++--------- tests/lib/files/cache/updater.php | 74 +++++++++++++++++++++++++++++ 3 files changed, 133 insertions(+), 23 deletions(-) diff --git a/lib/private/files/cache/cache.php b/lib/private/files/cache/cache.php index c5e118946e5..fd4c5715a67 100644 --- a/lib/private/files/cache/cache.php +++ b/lib/private/files/cache/cache.php @@ -463,6 +463,43 @@ class Cache { \OC_DB::executeAudited($sql, array($target, md5($target), basename($target), $newParentId, $sourceId)); } + /** + * Move a file or folder in the cache + * + * @param \OC\Files\Cache\Cache $sourceCache + * @param string $sourcePath + * @param string $targetPath + * @throws \OC\DatabaseException + */ + public function moveFromCache(Cache $sourceCache, $sourcePath, $targetPath) { + // normalize source and target + $sourcePath = $this->normalize($sourcePath); + $targetPath = $this->normalize($targetPath); + + $sourceData = $sourceCache->get($sourcePath); + $sourceId = $sourceData['fileid']; + $newParentId = $this->getParentId($targetPath); + + if ($sourceData['mimetype'] === 'httpd/unix-directory') { + //find all child entries + $sql = 'SELECT `path`, `fileid` FROM `*PREFIX*filecache` WHERE `storage` = ? AND `path` LIKE ?'; + $result = \OC_DB::executeAudited($sql, [$sourceCache->getNumericStorageId(), $sourcePath . '/%']); + $childEntries = $result->fetchAll(); + $sourceLength = strlen($sourcePath); + \OC_DB::beginTransaction(); + $query = \OC_DB::prepare('UPDATE `*PREFIX*filecache` SET `storage` = ?, `path` = ?, `path_hash` = ? WHERE `fileid` = ?'); + + foreach ($childEntries as $child) { + $newTargetPath = $targetPath . substr($child['path'], $sourceLength); + \OC_DB::executeAudited($query, [$this->getNumericStorageId(), $newTargetPath, md5($newTargetPath), $child['fileid']]); + } + \OC_DB::commit(); + } + + $sql = 'UPDATE `*PREFIX*filecache` SET `storage` = ?, `path` = ?, `path_hash` = ?, `name` = ?, `parent` =? WHERE `fileid` = ?'; + \OC_DB::executeAudited($sql, [$this->getNumericStorageId(), $targetPath, md5($targetPath), basename($targetPath), $newParentId, $sourceId]); + } + /** * remove all entries for files that are stored on the storage from the cache */ diff --git a/lib/private/files/cache/updater.php b/lib/private/files/cache/updater.php index 5d2ac608cfd..14aecf01220 100644 --- a/lib/private/files/cache/updater.php +++ b/lib/private/files/cache/updater.php @@ -144,30 +144,29 @@ class Updater { list($targetStorage, $targetInternalPath) = $this->view->resolvePath($target); if ($sourceStorage && $targetStorage) { - if ($sourceStorage === $targetStorage) { - $cache = $sourceStorage->getCache($sourceInternalPath); - if ($cache->inCache($targetInternalPath)) { - $cache->remove($targetInternalPath); - } - $cache->move($sourceInternalPath, $targetInternalPath); - - if (pathinfo($sourceInternalPath, PATHINFO_EXTENSION) !== pathinfo($targetInternalPath, PATHINFO_EXTENSION)) { - // handle mime type change - $mimeType = $sourceStorage->getMimeType($targetInternalPath); - $fileId = $cache->getId($targetInternalPath); - $cache->update($fileId, array('mimetype' => $mimeType)); - } - - $cache->correctFolderSize($sourceInternalPath); - $cache->correctFolderSize($targetInternalPath); - $this->correctParentStorageMtime($sourceStorage, $sourceInternalPath); - $this->correctParentStorageMtime($targetStorage, $targetInternalPath); - $this->propagator->addChange($source); - $this->propagator->addChange($target); - } else { - $this->remove($source); - $this->update($target); + $targetCache = $targetStorage->getCache($sourceInternalPath); + if ($targetCache->inCache($targetInternalPath)) { + $targetCache->remove($targetInternalPath); } + if ($sourceStorage === $targetStorage) { + $targetCache->move($sourceInternalPath, $targetInternalPath); + } else { + $targetCache->moveFromCache($sourceStorage->getCache(), $sourceInternalPath, $targetInternalPath); + } + + if (pathinfo($sourceInternalPath, PATHINFO_EXTENSION) !== pathinfo($targetInternalPath, PATHINFO_EXTENSION)) { + // handle mime type change + $mimeType = $sourceStorage->getMimeType($targetInternalPath); + $fileId = $targetCache->getId($targetInternalPath); + $targetCache->update($fileId, array('mimetype' => $mimeType)); + } + + $targetCache->correctFolderSize($sourceInternalPath); + $targetCache->correctFolderSize($targetInternalPath); + $this->correctParentStorageMtime($sourceStorage, $sourceInternalPath); + $this->correctParentStorageMtime($targetStorage, $targetInternalPath); + $this->propagator->addChange($source); + $this->propagator->addChange($target); $this->propagator->propagateChanges(); } } diff --git a/tests/lib/files/cache/updater.php b/tests/lib/files/cache/updater.php index 7c3ebd5a6f9..726ee360479 100644 --- a/tests/lib/files/cache/updater.php +++ b/tests/lib/files/cache/updater.php @@ -172,4 +172,78 @@ class Updater extends \Test\TestCase { $this->assertTrue($this->cache->inCache('foo.txt')); $this->assertFalse($this->cache->inCache('bar.txt')); } + + public function testMoveCrossStorage() { + $storage2 = new Temporary(array()); + $cache2 = $storage2->getCache(); + Filesystem::mount($storage2, array(), '/bar'); + $this->storage->file_put_contents('foo.txt', 'qwerty'); + + $this->updater->update('foo.txt'); + + $this->assertTrue($this->cache->inCache('foo.txt')); + $this->assertFalse($cache2->inCache('bar.txt')); + $cached = $this->cache->get('foo.txt'); + + // "rename" + $storage2->file_put_contents('bar.txt', 'qwerty'); + $this->storage->unlink('foo.txt'); + + $this->assertTrue($this->cache->inCache('foo.txt')); + $this->assertFalse($cache2->inCache('bar.txt')); + + $this->updater->rename('foo.txt', 'bar/bar.txt'); + + $this->assertFalse($this->cache->inCache('foo.txt')); + $this->assertTrue($cache2->inCache('bar.txt')); + + $cachedTarget = $cache2->get('bar.txt'); + $this->assertEquals($cached['mtime'], $cachedTarget['mtime']); + $this->assertEquals($cached['size'], $cachedTarget['size']); + $this->assertEquals($cached['etag'], $cachedTarget['etag']); + $this->assertEquals($cached['fileid'], $cachedTarget['fileid']); + } + + public function testMoveFolderCrossStorage() { + $storage2 = new Temporary(array()); + $cache2 = $storage2->getCache(); + Filesystem::mount($storage2, array(), '/bar'); + $this->storage->mkdir('foo'); + $this->storage->mkdir('foo/bar'); + $this->storage->file_put_contents('foo/foo.txt', 'qwerty'); + $this->storage->file_put_contents('foo/bar.txt', 'foo'); + $this->storage->file_put_contents('foo/bar/bar.txt', 'qwertyuiop'); + + $this->storage->getScanner()->scan(''); + + $this->assertTrue($this->cache->inCache('foo/foo.txt')); + $this->assertTrue($this->cache->inCache('foo/bar.txt')); + $this->assertTrue($this->cache->inCache('foo/bar/bar.txt')); + $cached = []; + $cached[] = $this->cache->get('foo/foo.txt'); + $cached[] = $this->cache->get('foo/bar.txt'); + $cached[] = $this->cache->get('foo/bar/bar.txt'); + + $this->view->rename('/foo', '/bar/foo'); + + $this->assertFalse($this->cache->inCache('foo/foo.txt')); + $this->assertFalse($this->cache->inCache('foo/bar.txt')); + $this->assertFalse($this->cache->inCache('foo/bar/bar.txt')); + $this->assertTrue($cache2->inCache('foo/foo.txt')); + $this->assertTrue($cache2->inCache('foo/bar.txt')); + $this->assertTrue($cache2->inCache('foo/bar/bar.txt')); + + $cachedTarget = []; + $cachedTarget[] = $cache2->get('foo/foo.txt'); + $cachedTarget[] = $cache2->get('foo/bar.txt'); + $cachedTarget[] = $cache2->get('foo/bar/bar.txt'); + + foreach ($cached as $i => $old) { + $new = $cachedTarget[$i]; + $this->assertEquals($old['mtime'], $new['mtime']); + $this->assertEquals($old['size'], $new['size']); + $this->assertEquals($old['etag'], $new['etag']); + $this->assertEquals($old['fileid'], $new['fileid']); + } + } } From caadc8cdd9044236388dfb557ba21a5de7f147fc Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 1 Apr 2015 15:15:24 +0200 Subject: [PATCH 2/7] reuse cache move logic --- lib/private/files/cache/cache.php | 27 +-------------------------- 1 file changed, 1 insertion(+), 26 deletions(-) diff --git a/lib/private/files/cache/cache.php b/lib/private/files/cache/cache.php index fd4c5715a67..ee9da231090 100644 --- a/lib/private/files/cache/cache.php +++ b/lib/private/files/cache/cache.php @@ -435,32 +435,7 @@ class Cache { * @param string $target */ public function move($source, $target) { - // normalize source and target - $source = $this->normalize($source); - $target = $this->normalize($target); - - $sourceData = $this->get($source); - $sourceId = $sourceData['fileid']; - $newParentId = $this->getParentId($target); - - if ($sourceData['mimetype'] === 'httpd/unix-directory') { - //find all child entries - $sql = 'SELECT `path`, `fileid` FROM `*PREFIX*filecache` WHERE `storage` = ? AND `path` LIKE ?'; - $result = \OC_DB::executeAudited($sql, array($this->getNumericStorageId(), $source . '/%')); - $childEntries = $result->fetchAll(); - $sourceLength = strlen($source); - \OC_DB::beginTransaction(); - $query = \OC_DB::prepare('UPDATE `*PREFIX*filecache` SET `path` = ?, `path_hash` = ? WHERE `fileid` = ?'); - - foreach ($childEntries as $child) { - $targetPath = $target . substr($child['path'], $sourceLength); - \OC_DB::executeAudited($query, array($targetPath, md5($targetPath), $child['fileid'])); - } - \OC_DB::commit(); - } - - $sql = 'UPDATE `*PREFIX*filecache` SET `path` = ?, `path_hash` = ?, `name` = ?, `parent` =? WHERE `fileid` = ?'; - \OC_DB::executeAudited($sql, array($target, md5($target), basename($target), $newParentId, $sourceId)); + $this->moveFromCache($this, $source, $target); } /** From 6b5daca7b7732edc1a30bcca28827897d9558130 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 1 Apr 2015 16:12:01 +0200 Subject: [PATCH 3/7] check for source cache --- apps/files_sharing/lib/cache.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/files_sharing/lib/cache.php b/apps/files_sharing/lib/cache.php index e9b2addea38..187d3f38afc 100644 --- a/apps/files_sharing/lib/cache.php +++ b/apps/files_sharing/lib/cache.php @@ -416,7 +416,9 @@ class Shared_Cache extends Cache { // bubble up to source cache $sourceCache = $this->getSourceCache($path); $parent = dirname($this->files[$path]); - $sourceCache->correctFolderSize($parent); + if ($sourceCache) { + $sourceCache->correctFolderSize($parent); + } } } From 86886608256f6c480269366f83f83fa97594a5dd Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 7 Apr 2015 16:02:37 +0200 Subject: [PATCH 4/7] check that we know the parent --- apps/files_sharing/lib/cache.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/apps/files_sharing/lib/cache.php b/apps/files_sharing/lib/cache.php index 187d3f38afc..2f982ec9dc6 100644 --- a/apps/files_sharing/lib/cache.php +++ b/apps/files_sharing/lib/cache.php @@ -415,9 +415,11 @@ class Shared_Cache extends Cache { } else { // bubble up to source cache $sourceCache = $this->getSourceCache($path); - $parent = dirname($this->files[$path]); - if ($sourceCache) { - $sourceCache->correctFolderSize($parent); + if (isset($this->files[$path])) { + $parent = dirname($this->files[$path]); + if ($sourceCache) { + $sourceCache->correctFolderSize($parent); + } } } } From addfafd9da020466df1d1fac56b9f1eb30f79f68 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 9 Apr 2015 14:10:51 +0200 Subject: [PATCH 5/7] Fix moving mount points --- lib/private/files/view.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/private/files/view.php b/lib/private/files/view.php index ab7a7d3db9a..be14521990a 100644 --- a/lib/private/files/view.php +++ b/lib/private/files/view.php @@ -677,7 +677,9 @@ class View { $this->emit_file_hooks_post($exists, $path2); } } elseif ($result) { - $this->updater->rename($path1, $path2); + if ($internalPath1 !== '') { // dont do a cache update for moved mounts + $this->updater->rename($path1, $path2); + } if ($this->shouldEmitHooks($path1) and $this->shouldEmitHooks($path2)) { \OC_Hook::emit( Filesystem::CLASSNAME, From 01da6be4d6301cceb3a0c04106aacc7178783353 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 9 Apr 2015 15:13:56 +0200 Subject: [PATCH 6/7] upda tests --- tests/lib/files/node/integration.php | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/tests/lib/files/node/integration.php b/tests/lib/files/node/integration.php index 4e362607240..545793be54a 100644 --- a/tests/lib/files/node/integration.php +++ b/tests/lib/files/node/integration.php @@ -37,11 +37,6 @@ class IntegrationTests extends \Test\TestCase { \OC_Hook::clear('OC_Filesystem'); - \OC_Hook::connect('OC_Filesystem', 'post_write', '\OC\Files\Cache\Updater', 'writeHook'); - \OC_Hook::connect('OC_Filesystem', 'post_delete', '\OC\Files\Cache\Updater', 'deleteHook'); - \OC_Hook::connect('OC_Filesystem', 'post_rename', '\OC\Files\Cache\Updater', 'renameHook'); - \OC_Hook::connect('OC_Filesystem', 'post_touch', '\OC\Files\Cache\Updater', 'touchHook'); - $user = new User($this->getUniqueID('user'), new \OC_User_Dummy); $this->loginAsUser($user->getUID()); @@ -83,7 +78,7 @@ class IntegrationTests extends \Test\TestCase { $this->assertEquals('bar.txt', $file->getInternalPath()); $file->move('/substorage/bar.txt'); - $this->assertNotEquals($id, $file->getId()); + $this->assertEquals($id, $file->getId()); $this->assertEquals('qwerty', $file->getContent()); } From f605c985311fc652d1744704061af38f3ac62919 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 13 Apr 2015 17:09:18 +0200 Subject: [PATCH 7/7] Fix cross storage move with shared storages --- apps/files_sharing/lib/cache.php | 17 +++++++---------- apps/files_sharing/lib/sharedstorage.php | 5 ++++- lib/private/files/cache/cache.php | 19 ++++++++++++++++--- 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/apps/files_sharing/lib/cache.php b/apps/files_sharing/lib/cache.php index 2f982ec9dc6..49f5a4e8e7c 100644 --- a/apps/files_sharing/lib/cache.php +++ b/apps/files_sharing/lib/cache.php @@ -235,18 +235,15 @@ class Shared_Cache extends Cache { } /** - * Move a file or folder in the cache + * Get the storage id and path needed for a move * - * @param string $source - * @param string $target + * @param string $path + * @return array [$storageId, $internalPath] */ - public function move($source, $target) { - if ($cache = $this->getSourceCache($source)) { - $file = \OC_Share_Backend_File::getSource($target, $this->storage->getMountPoint(), $this->storage->getItemType()); - if ($file && isset($file['path'])) { - $cache->move($this->files[$source], $file['path']); - } - } + protected function getMoveInfo($path) { + $cache = $this->getSourceCache($path); + $file = \OC_Share_Backend_File::getSource($path, $this->storage->getMountPoint(), $this->storage->getItemType()); + return [$cache->getNumericStorageId(), $file['path']]; } /** diff --git a/apps/files_sharing/lib/sharedstorage.php b/apps/files_sharing/lib/sharedstorage.php index 6e3abb1f56c..c2253fcaa3c 100644 --- a/apps/files_sharing/lib/sharedstorage.php +++ b/apps/files_sharing/lib/sharedstorage.php @@ -327,7 +327,10 @@ class Shared extends \OC\Files\Storage\Common implements ISharedStorage { $targetFilename = basename($relPath2); list($user2, $path2) = \OCA\Files_Sharing\Helper::getUidAndFilename(dirname($relPath2)); $rootView = new \OC\Files\View(''); - return $rootView->rename('/' . $user1 . '/files/' . $path1, '/' . $user2 . '/files/' . $path2 . '/' . $targetFilename); + $rootView->getUpdater()->disable(); // dont update the cache here + $result = $rootView->rename('/' . $user1 . '/files/' . $path1, '/' . $user2 . '/files/' . $path2 . '/' . $targetFilename); + $rootView->getUpdater()->enable(); + return $result; } public function copy($path1, $path2) { diff --git a/lib/private/files/cache/cache.php b/lib/private/files/cache/cache.php index ee9da231090..f00177d9c5b 100644 --- a/lib/private/files/cache/cache.php +++ b/lib/private/files/cache/cache.php @@ -438,6 +438,16 @@ class Cache { $this->moveFromCache($this, $source, $target); } + /** + * Get the storage id and path needed for a move + * + * @param string $path + * @return array [$storageId, $internalPath] + */ + protected function getMoveInfo($path) { + return [$this->getNumericStorageId(), $path]; + } + /** * Move a file or folder in the cache * @@ -455,10 +465,13 @@ class Cache { $sourceId = $sourceData['fileid']; $newParentId = $this->getParentId($targetPath); + list($sourceStorageId, $sourcePath) = $sourceCache->getMoveInfo($sourcePath); + list($targetStorageId, $targetPath) = $this->getMoveInfo($targetPath); + if ($sourceData['mimetype'] === 'httpd/unix-directory') { //find all child entries $sql = 'SELECT `path`, `fileid` FROM `*PREFIX*filecache` WHERE `storage` = ? AND `path` LIKE ?'; - $result = \OC_DB::executeAudited($sql, [$sourceCache->getNumericStorageId(), $sourcePath . '/%']); + $result = \OC_DB::executeAudited($sql, [$sourceStorageId, $sourcePath . '/%']); $childEntries = $result->fetchAll(); $sourceLength = strlen($sourcePath); \OC_DB::beginTransaction(); @@ -466,13 +479,13 @@ class Cache { foreach ($childEntries as $child) { $newTargetPath = $targetPath . substr($child['path'], $sourceLength); - \OC_DB::executeAudited($query, [$this->getNumericStorageId(), $newTargetPath, md5($newTargetPath), $child['fileid']]); + \OC_DB::executeAudited($query, [$targetStorageId, $newTargetPath, md5($newTargetPath), $child['fileid']]); } \OC_DB::commit(); } $sql = 'UPDATE `*PREFIX*filecache` SET `storage` = ?, `path` = ?, `path_hash` = ?, `name` = ?, `parent` =? WHERE `fileid` = ?'; - \OC_DB::executeAudited($sql, [$this->getNumericStorageId(), $targetPath, md5($targetPath), basename($targetPath), $newParentId, $sourceId]); + \OC_DB::executeAudited($sql, [$targetStorageId, $targetPath, md5($targetPath), basename($targetPath), $newParentId, $sourceId]); } /**