From 6e0a218d1110fcf076cc93277fd97a9d409778c9 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 10 Jun 2014 15:26:18 +0200 Subject: [PATCH 1/4] Repair broken parent link in the scanner --- lib/private/files/cache/scanner.php | 74 +++++++++++++++-------------- tests/lib/files/cache/scanner.php | 50 ++++++++++++++++++- 2 files changed, 87 insertions(+), 37 deletions(-) diff --git a/lib/private/files/cache/scanner.php b/lib/private/files/cache/scanner.php index 8c10d0f354f..f1c4313b014 100644 --- a/lib/private/files/cache/scanner.php +++ b/lib/private/files/cache/scanner.php @@ -105,48 +105,50 @@ class Scanner extends BasicEmitter { \OC_Hook::emit('\OC\Files\Cache\Scanner', 'scan_file', array('path' => $file, 'storage' => $this->storageId)); $data = $this->getData($file); if ($data) { - if ($file and !$parentExistsInCache) { - $parent = dirname($file); - if ($parent === '.' or $parent === '/') { - $parent = ''; - } - if (!$this->cache->inCache($parent)) { - $this->scanFile($parent); - } + $parent = dirname($file); + if ($parent === '.' or $parent === '/') { + $parent = ''; + } + $parentId = $this->cache->getId($parent); + if ($file and $parentId === -1) { + $parentData = $this->scanFile($parent); + $parentId = $parentData['fileid']; + } + if ($parent) { + $data['parent'] = $parentId; } - $newData = $data; $cacheData = $this->cache->get($file); - if ($cacheData) { - if (isset($cacheData['fileid'])) { - $this->permissionsCache->remove($cacheData['fileid']); - } - if ($reuseExisting) { - // prevent empty etag + if ( $cacheData and isset($cacheData['fileid'])) { + $this->permissionsCache->remove($cacheData['fileid']); + } + if ($cacheData and $reuseExisting) { + // prevent empty etag + if (empty($cacheData['etag'])) { + $etag = $data['etag']; + } else { $etag = $cacheData['etag']; - if (empty($etag)) { - $etag = $data['etag']; - } else { - $etag = $cacheData['etag']; + } + // only reuse data if the file hasn't explicitly changed + if (isset($data['storage_mtime']) && isset($cacheData['storage_mtime']) && $data['storage_mtime'] === $cacheData['storage_mtime']) { + $data['mtime'] = $cacheData['mtime']; + if (($reuseExisting & self::REUSE_SIZE) && ($data['size'] === -1)) { + $data['size'] = $cacheData['size']; } - // only reuse data if the file hasn't explicitly changed - if (isset($data['storage_mtime']) && isset($cacheData['storage_mtime']) && $data['storage_mtime'] === $cacheData['storage_mtime']) { - if (($reuseExisting & self::REUSE_SIZE) && ($data['size'] === -1)) { - $data['size'] = $cacheData['size']; - } - if ($reuseExisting & self::REUSE_ETAG) { - $data['etag'] = $etag; - } - } - // Only update metadata that has changed - $newData = array_diff_assoc($data, $cacheData); - if (isset($newData['etag'])) { - $cacheDataString = print_r($cacheData, true); - $dataString = print_r($data, true); - \OCP\Util::writeLog('OC\Files\Cache\Scanner', - "!!! No reuse of etag for '$file' !!! \ncache: $cacheDataString \ndata: $dataString", - \OCP\Util::DEBUG); + if ($reuseExisting & self::REUSE_ETAG) { + $data['etag'] = $etag; } } + // Only update metadata that has changed + $newData = array_diff_assoc($data, $cacheData); + if (isset($newData['etag'])) { + $cacheDataString = print_r($cacheData, true); + $dataString = print_r($data, true); + \OCP\Util::writeLog('OC\Files\Cache\Scanner', + "!!! No reuse of etag for '$file' !!! \ncache: $cacheDataString \ndata: $dataString", + \OCP\Util::DEBUG); + } + } else { + $newData = $data; } if (!empty($newData)) { $data['fileid'] = $this->addToCache($file, $newData); diff --git a/tests/lib/files/cache/scanner.php b/tests/lib/files/cache/scanner.php index 92d75aae907..3b86df51bab 100644 --- a/tests/lib/files/cache/scanner.php +++ b/tests/lib/files/cache/scanner.php @@ -199,7 +199,7 @@ class Scanner extends \PHPUnit_Framework_TestCase { $this->assertFalse($this->cache->inCache('folder/bar.txt')); } - public function testScanRemovedFile(){ + public function testScanRemovedFile() { $this->fillTestFolders(); $this->scanner->scan(''); @@ -230,4 +230,52 @@ class Scanner extends \PHPUnit_Framework_TestCase { $this->assertInternalType('string', $newData0['etag']); $this->assertNotEmpty($newData0['etag']); } + + public function testRepairParent() { + $this->fillTestFolders(); + $this->scanner->scan(''); + $this->assertTrue($this->cache->inCache('folder/bar.txt')); + $oldFolderId = $this->cache->getId('folder'); + + // delete the folder without removing the childs + $sql = 'DELETE FROM `*PREFIX*filecache` WHERE `fileid` = ?'; + \OC_DB::executeAudited($sql, array($oldFolderId)); + + $cachedData = $this->cache->get('folder/bar.txt'); + $this->assertEquals($oldFolderId, $cachedData['parent']); + $this->assertFalse($this->cache->inCache('folder')); + + $this->scanner->scan(''); + + $this->assertTrue($this->cache->inCache('folder')); + $newFolderId = $this->cache->getId('folder'); + $this->assertNotEquals($oldFolderId, $newFolderId); + + $cachedData = $this->cache->get('folder/bar.txt'); + $this->assertEquals($newFolderId, $cachedData['parent']); + } + + public function testRepairParentShallow() { + $this->fillTestFolders(); + $this->scanner->scan(''); + $this->assertTrue($this->cache->inCache('folder/bar.txt')); + $oldFolderId = $this->cache->getId('folder'); + + // delete the folder without removing the childs + $sql = 'DELETE FROM `*PREFIX*filecache` WHERE `fileid` = ?'; + \OC_DB::executeAudited($sql, array($oldFolderId)); + + $cachedData = $this->cache->get('folder/bar.txt'); + $this->assertEquals($oldFolderId, $cachedData['parent']); + $this->assertFalse($this->cache->inCache('folder')); + + $this->scanner->scan('folder', \OC\Files\Cache\Scanner::SCAN_SHALLOW); + + $this->assertTrue($this->cache->inCache('folder')); + $newFolderId = $this->cache->getId('folder'); + $this->assertNotEquals($oldFolderId, $newFolderId); + + $cachedData = $this->cache->get('folder/bar.txt'); + $this->assertEquals($newFolderId, $cachedData['parent']); + } } From e793a87954e19424f1f1a9d6c3b472c5029a587e Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 10 Jun 2014 15:37:43 +0200 Subject: [PATCH 2/4] add some comments --- lib/private/files/cache/scanner.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/private/files/cache/scanner.php b/lib/private/files/cache/scanner.php index f1c4313b014..afbf69fb1e2 100644 --- a/lib/private/files/cache/scanner.php +++ b/lib/private/files/cache/scanner.php @@ -110,6 +110,8 @@ class Scanner extends BasicEmitter { $parent = ''; } $parentId = $this->cache->getId($parent); + + // scan the parent if it's not in the cache (id -1) and the current file is not the root folder if ($file and $parentId === -1) { $parentData = $this->scanFile($parent); $parentId = $parentData['fileid']; From eeca726d28272978f3a3ce425edd28a09556ce54 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 10 Jun 2014 15:42:37 +0200 Subject: [PATCH 3/4] remove unused argument --- lib/private/files/cache/scanner.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/private/files/cache/scanner.php b/lib/private/files/cache/scanner.php index afbf69fb1e2..abba659931a 100644 --- a/lib/private/files/cache/scanner.php +++ b/lib/private/files/cache/scanner.php @@ -94,10 +94,9 @@ class Scanner extends BasicEmitter { * * @param string $file * @param int $reuseExisting - * @param bool $parentExistsInCache - * @return array with metadata of the scanned file + * @return array an array of metadata of the scanned file */ - public function scanFile($file, $reuseExisting = 0, $parentExistsInCache = false) { + public function scanFile($file, $reuseExisting = 0) { if (!self::isPartialFile($file) and !Filesystem::isFileBlacklisted($file) ) { @@ -248,7 +247,7 @@ class Scanner extends BasicEmitter { if (!Filesystem::isIgnoredDir($file)) { $newChildren[] = $file; try { - $data = $this->scanFile($child, $reuse, true); + $data = $this->scanFile($child, $reuse); if ($data) { if ($data['mimetype'] === 'httpd/unix-directory' and $recursive === self::SCAN_RECURSIVE) { $childQueue[] = $child; From 84222e30a76c690ab1d217a8e39f59feb958464e Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 16 Jun 2014 13:37:08 +0200 Subject: [PATCH 4/4] Fix unit test --- tests/lib/files/utils/scanner.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/lib/files/utils/scanner.php b/tests/lib/files/utils/scanner.php index 159a2a48677..5e5cc6ac128 100644 --- a/tests/lib/files/utils/scanner.php +++ b/tests/lib/files/utils/scanner.php @@ -112,7 +112,7 @@ class Scanner extends \PHPUnit_Framework_TestCase { $this->assertEquals(array('/foo', '/foo/folder', '/foo/folder/bar.txt', '/foo/foo.txt'), $changes); $this->assertEquals(array('/', '/foo', '/foo/folder'), $parents); - $cache->put('foo.txt', array('mtime' => time() - 50)); + $cache->put('foo.txt', array('storage_mtime' => time() - 50)); $propagator = $this->getMock('\OC\Files\Cache\ChangePropagator', array('propagateChanges'), array(), '', false); $scanner->setPropagator($propagator); @@ -128,7 +128,7 @@ class Scanner extends \PHPUnit_Framework_TestCase { $scanner->setPropagator($originalPropagator); $oldInfo = $cache->get(''); - $cache->put('foo.txt', array('mtime' => time() - 70)); + $cache->put('foo.txt', array('storage_mtime' => time() - 70)); $storage->file_put_contents('foo.txt', 'asdasd'); $scanner->scan('');