Merge pull request #15360 from owncloud/cross-storage-move-cache

Preserve cache data when doing a cross storage move

7 files changed, 151 insertions, 52 deletions

diff --git a/apps/files_sharing/lib/cache.php b/apps/files_sharing/lib/cache.php
index e9b2addea38..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']];
 	}
 
 	/**
@@ -415,8 +412,12 @@ class Shared_Cache extends Cache {
 		} else {
 			// bubble up to source cache
 			$sourceCache = $this->getSourceCache($path);
-			$parent = dirname($this->files[$path]);
-			$sourceCache->correctFolderSize($parent);
+			if (isset($this->files[$path])) {
+				$parent = dirname($this->files[$path]);
+				if ($sourceCache) {
+					$sourceCache->correctFolderSize($parent);
+				}
+			}
 		}
 	}
 
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 c5e118946e5..f00177d9c5b 100644
--- a/lib/private/files/cache/cache.php
+++ b/lib/private/files/cache/cache.php
@@ -435,32 +435,57 @@ class Cache {
 	 * @param string $target
 	 */
 	public function move($source, $target) {
+		$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
+	 *
+	 * @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
-		$source = $this->normalize($source);
-		$target = $this->normalize($target);
+		$sourcePath = $this->normalize($sourcePath);
+		$targetPath = $this->normalize($targetPath);
 
-		$sourceData = $this->get($source);
+		$sourceData = $sourceCache->get($sourcePath);
 		$sourceId = $sourceData['fileid'];
-		$newParentId = $this->getParentId($target);
+		$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, array($this->getNumericStorageId(), $source . '/%'));
+			$result = \OC_DB::executeAudited($sql, [$sourceStorageId, $sourcePath . '/%']);
 			$childEntries = $result->fetchAll();
-			$sourceLength = strlen($source);
+			$sourceLength = strlen($sourcePath);
 			\OC_DB::beginTransaction();
-			$query = \OC_DB::prepare('UPDATE `*PREFIX*filecache` SET `path` = ?, `path_hash` = ? WHERE `fileid` = ?');
+			$query = \OC_DB::prepare('UPDATE `*PREFIX*filecache` SET `storage` = ?, `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']));
+				$newTargetPath = $targetPath . substr($child['path'], $sourceLength);
+				\OC_DB::executeAudited($query, [$targetStorageId, $newTargetPath, md5($newTargetPath), $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));
+		$sql = 'UPDATE `*PREFIX*filecache` SET `storage` =  ?, `path` = ?, `path_hash` = ?, `name` = ?, `parent` =? WHERE `fileid` = ?';
+		\OC_DB::executeAudited($sql, [$targetStorageId, $targetPath, md5($targetPath), basename($targetPath), $newParentId, $sourceId]);
 	}
 
 	/**
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) {
+			$targetCache = $targetStorage->getCache($sourceInternalPath);
+			if ($targetCache->inCache($targetInternalPath)) {
+				$targetCache->remove($targetInternalPath);
+			}
 			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);
+				$targetCache->move($sourceInternalPath, $targetInternalPath);
 			} else {
-				$this->remove($source);
-				$this->update($target);
+				$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/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,
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']);
+		}
+	}
 }
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());
 	}