From cbcdf69dc218a9767b2682c0a1825f9f36e78266 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 9 Nov 2016 16:14:54 +0100 Subject: [PATCH] only query substorages to calculate the final mtime/size/etag lazily Signed-off-by: Robin Appelman --- .../tests/SizePropagationTest.php | 3 +- lib/private/Files/FileInfo.php | 40 ++++++++++++++++++- lib/private/Files/View.php | 15 ++----- tests/lib/Files/Cache/UpdaterLegacyTest.php | 23 ++++++----- tests/lib/Files/ViewTest.php | 4 +- 5 files changed, 62 insertions(+), 23 deletions(-) diff --git a/apps/files_sharing/tests/SizePropagationTest.php b/apps/files_sharing/tests/SizePropagationTest.php index 04db505e8a..9d80633b46 100644 --- a/apps/files_sharing/tests/SizePropagationTest.php +++ b/apps/files_sharing/tests/SizePropagationTest.php @@ -102,6 +102,7 @@ class SizePropagationTest extends TestCase { $this->assertTrue($recipientView->file_exists('/sharedfolder/subfolder/foo.txt')); $recipientRootInfo = $recipientView->getFileInfo('', false); $recipientRootInfoWithMounts = $recipientView->getFileInfo('', true); + $oldRecipientSize = $recipientRootInfoWithMounts->getSize(); // when file changed as recipient $recipientView->file_put_contents('/sharedfolder/subfolder/foo.txt', 'foobar'); @@ -112,7 +113,7 @@ class SizePropagationTest extends TestCase { // but the size including mountpoints increases $newRecipientRootInfo = $recipientView->getFileInfo('', true); - $this->assertEquals($recipientRootInfoWithMounts->getSize() +3, $newRecipientRootInfo->getSize()); + $this->assertEquals($oldRecipientSize +3, $newRecipientRootInfo->getSize()); // size of owner's root increases $this->loginAsUser(self::TEST_FILES_SHARING_API_USER2); diff --git a/lib/private/Files/FileInfo.php b/lib/private/Files/FileInfo.php index d463abfadc..14a32ba8f7 100644 --- a/lib/private/Files/FileInfo.php +++ b/lib/private/Files/FileInfo.php @@ -31,6 +31,8 @@ namespace OC\Files; use OCP\Files\Cache\ICacheEntry; +use OCP\Files\Mount\IMountPoint; +use OCP\Files\Storage\IStorage; use OCP\IUser; class FileInfo implements \OCP\Files\FileInfo, \ArrayAccess { @@ -69,6 +71,13 @@ class FileInfo implements \OCP\Files\FileInfo, \ArrayAccess { */ private $childEtags = []; + /** + * @var IMountPoint[] + */ + private $subMounts = []; + + private $subMountsUsed = false; + /** * @param string|boolean $path * @param Storage\Storage $storage @@ -103,6 +112,10 @@ class FileInfo implements \OCP\Files\FileInfo, \ArrayAccess { return $this->getType(); } else if ($offset === 'etag') { return $this->getEtag(); + } else if ($offset === 'size') { + return $this->getSize(); + } else if ($offset === 'mtime') { + return $this->getMTime(); } elseif ($offset === 'permissions') { return $this->getPermissions(); } elseif (isset($this->data[$offset])) { @@ -165,6 +178,7 @@ class FileInfo implements \OCP\Files\FileInfo, \ArrayAccess { * @return string */ public function getEtag() { + $this->updateEntryfromSubMounts(); if (count($this->childEtags) > 0) { $combinedEtag = $this->data['etag'] . '::' . implode('::', $this->childEtags); return md5($combinedEtag); @@ -177,6 +191,7 @@ class FileInfo implements \OCP\Files\FileInfo, \ArrayAccess { * @return int */ public function getSize() { + $this->updateEntryfromSubMounts(); return isset($this->data['size']) ? $this->data['size'] : 0; } @@ -184,6 +199,7 @@ class FileInfo implements \OCP\Files\FileInfo, \ArrayAccess { * @return int */ public function getMTime() { + $this->updateEntryfromSubMounts(); return $this->data['mtime']; } @@ -316,12 +332,34 @@ class FileInfo implements \OCP\Files\FileInfo, \ArrayAccess { return $this->owner; } + /** + * @param IMountPoint[] $mounts + */ + public function setSubMounts(array $mounts) { + $this->subMounts = $mounts; + } + + private function updateEntryfromSubMounts() { + if ($this->subMountsUsed) { + return; + } + $this->subMountsUsed = true; + foreach ($this->subMounts as $mount) { + $subStorage = $mount->getStorage(); + if ($subStorage) { + $subCache = $subStorage->getCache(''); + $rootEntry = $subCache->get(''); + $this->addSubEntry($rootEntry, $mount->getMountPoint()); + } + } + } + /** * Add a cache entry which is the child of this folder * * Sets the size, etag and size to for cross-storage childs * - * @param array $data cache entry for the child + * @param array|ICacheEntry $data cache entry for the child * @param string $entryPath full path of the child entry */ public function addSubEntry($data, $entryPath) { diff --git a/lib/private/Files/View.php b/lib/private/Files/View.php index f36e2c2c64..3514c82969 100644 --- a/lib/private/Files/View.php +++ b/lib/private/Files/View.php @@ -56,6 +56,7 @@ use OCP\Files\Cache\ICacheEntry; use OCP\Files\FileNameTooLongException; use OCP\Files\InvalidCharacterInPathException; use OCP\Files\InvalidPathException; +use OCP\Files\Mount\IMountPoint; use OCP\Files\NotFoundException; use OCP\Files\ReservedWordException; use OCP\Files\UnseekableException; @@ -1353,18 +1354,10 @@ class View { //add the sizes of other mount points to the folder $extOnly = ($includeMountPoints === 'ext'); $mounts = Filesystem::getMountManager()->findIn($path); - foreach ($mounts as $mount) { + $info->setSubMounts(array_filter($mounts, function(IMountPoint $mount) use ($extOnly) { $subStorage = $mount->getStorage(); - if ($subStorage) { - // exclude shared storage ? - if ($extOnly && $subStorage instanceof \OCA\Files_Sharing\SharedStorage) { - continue; - } - $subCache = $subStorage->getCache(''); - $rootEntry = $subCache->get(''); - $info->addSubEntry($rootEntry, $mount->getMountPoint()); - } - } + return !($extOnly && $subStorage instanceof \OCA\Files_Sharing\SharedStorage); + })); } } diff --git a/tests/lib/Files/Cache/UpdaterLegacyTest.php b/tests/lib/Files/Cache/UpdaterLegacyTest.php index 7d247968ca..707ed70af2 100644 --- a/tests/lib/Files/Cache/UpdaterLegacyTest.php +++ b/tests/lib/Files/Cache/UpdaterLegacyTest.php @@ -128,24 +128,26 @@ class UpdaterLegacyTest extends \Test\TestCase { Filesystem::file_put_contents('folder/substorage/foo.txt', 'asd'); $this->assertTrue($cache2->inCache('foo.txt')); $cachedData = $cache2->get('foo.txt'); + $oldEtag = $substorageCachedData['etag']; $this->assertEquals(3, $cachedData['size']); $mtime = $cachedData['mtime']; $cachedData = $cache2->get(''); $this->assertInternalType('string', $substorageCachedData['etag']); $this->assertInternalType('string', $cachedData['etag']); - $this->assertNotSame($substorageCachedData['etag'], $cachedData['etag']); + $this->assertNotSame($oldEtag, $cachedData['etag']); $cachedData = $view->getFileInfo('folder'); $this->assertInternalType('string', $folderCachedData['etag']); $this->assertInternalType('string', $cachedData['etag']); - $this->assertNotSame($folderCachedData['etag'], $cachedData['etag']); + $this->assertNotSame($oldEtag, $cachedData['etag']); } public function testDelete() { $textSize = strlen("dummy file data\n"); $imageSize = filesize(\OC::$SERVERROOT . '/core/img/logo.png'); $rootCachedData = $this->cache->get(''); + $oldEtag = $rootCachedData['etag']; $this->assertEquals(3 * $textSize + $imageSize, $rootCachedData['size']); $this->assertTrue($this->cache->inCache('foo.txt')); @@ -155,7 +157,7 @@ class UpdaterLegacyTest extends \Test\TestCase { $this->assertEquals(2 * $textSize + $imageSize, $cachedData['size']); $this->assertInternalType('string', $rootCachedData['etag']); $this->assertInternalType('string', $cachedData['etag']); - $this->assertNotSame($rootCachedData['etag'], $cachedData['etag']); + $this->assertNotSame($oldEtag, $cachedData['etag']); $this->assertGreaterThanOrEqual($rootCachedData['mtime'], $cachedData['mtime']); $rootCachedData = $cachedData; @@ -164,14 +166,15 @@ class UpdaterLegacyTest extends \Test\TestCase { $cachedData = $this->cache->get(''); $this->assertInternalType('string', $rootCachedData['etag']); $this->assertInternalType('string', $cachedData['etag']); - $this->assertNotSame($rootCachedData['etag'], $cachedData['etag']); + $this->assertNotSame($oldEtag, $cachedData['etag']); $rootCachedData = $cachedData; + $oldEtag = $rootCachedData['etag']; Filesystem::rmdir('bar_folder'); $this->assertFalse($this->cache->inCache('bar_folder')); $cachedData = $this->cache->get(''); $this->assertInternalType('string', $rootCachedData['etag']); $this->assertInternalType('string', $cachedData['etag']); - $this->assertNotSame($rootCachedData['etag'], $cachedData['etag']); + $this->assertNotSame($oldEtag, $cachedData['etag']); $this->assertGreaterThanOrEqual($rootCachedData['mtime'], $cachedData['mtime']); } @@ -184,19 +187,20 @@ class UpdaterLegacyTest extends \Test\TestCase { $this->assertTrue($cache2->inCache('foo.txt')); $folderCachedData = $view->getFileInfo('folder'); $substorageCachedData = $cache2->get(''); + $oldEtag = $folderCachedData['etag']; Filesystem::unlink('folder/substorage/foo.txt'); $this->assertFalse($cache2->inCache('foo.txt')); $cachedData = $cache2->get(''); $this->assertInternalType('string', $substorageCachedData['etag']); $this->assertInternalType('string', $cachedData['etag']); - $this->assertNotSame($substorageCachedData['etag'], $cachedData['etag']); + $this->assertNotSame($substorageCachedData, $cachedData['etag']); $this->assertGreaterThanOrEqual($substorageCachedData['mtime'], $cachedData['mtime']); $cachedData = $view->getFileInfo('folder'); $this->assertInternalType('string', $folderCachedData['etag']); $this->assertInternalType('string', $cachedData['etag']); - $this->assertNotSame($folderCachedData['etag'], $cachedData['etag']); + $this->assertNotSame($oldEtag, $cachedData['etag']); $this->assertGreaterThanOrEqual($folderCachedData['mtime'], $cachedData['mtime']); } @@ -238,6 +242,7 @@ class UpdaterLegacyTest extends \Test\TestCase { $view = new View('/' . self::$user . '/files'); $this->assertTrue($cache2->inCache('foo.txt')); $folderCachedData = $view->getFileInfo('folder'); + $oldEtag = $folderCachedData['etag']; $substorageCachedData = $cache2->get(''); $fooCachedData = $cache2->get('foo.txt'); Filesystem::rename('folder/substorage/foo.txt', 'folder/substorage/bar.txt'); @@ -250,14 +255,14 @@ class UpdaterLegacyTest extends \Test\TestCase { $cachedData = $cache2->get(''); $this->assertInternalType('string', $substorageCachedData['etag']); $this->assertInternalType('string', $cachedData['etag']); - $this->assertNotSame($substorageCachedData['etag'], $cachedData['etag']); + $this->assertNotSame($oldEtag, $cachedData['etag']); // rename can cause mtime change - invalid assert // $this->assertEquals($mtime, $cachedData['mtime']); $cachedData = $view->getFileInfo('folder'); $this->assertInternalType('string', $folderCachedData['etag']); $this->assertInternalType('string', $cachedData['etag']); - $this->assertNotSame($folderCachedData['etag'], $cachedData['etag']); + $this->assertNotSame($oldEtag, $cachedData['etag']); // rename can cause mtime change - invalid assert // $this->assertEquals($mtime, $cachedData['mtime']); } diff --git a/tests/lib/Files/ViewTest.php b/tests/lib/Files/ViewTest.php index b0233116ed..4c26447238 100644 --- a/tests/lib/Files/ViewTest.php +++ b/tests/lib/Files/ViewTest.php @@ -842,10 +842,12 @@ class ViewTest extends \Test\TestCase { 'storage_mtime' => $past )); + $oldEtag = $oldFolderInfo->getEtag(); + $view->getFileInfo('/test/sub/storage/test.txt'); $newFolderInfo = $view->getFileInfo('/test'); - $this->assertNotEquals($newFolderInfo->getEtag(), $oldFolderInfo->getEtag()); + $this->assertNotEquals($newFolderInfo->getEtag(), $oldEtag); } /**