From b025f07fb7090acfe9d168974b4ce2e66f973e30 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 25 Nov 2015 13:53:31 +0100 Subject: [PATCH] Make Cache\Updater per storage --- apps/dav/lib/connector/sabre/file.php | 4 +- apps/encryption/lib/migration.php | 2 +- apps/files_trashbin/lib/trashbin.php | 6 +- lib/private/files/cache/updater.php | 168 ++++++++---------- lib/private/files/storage/common.php | 12 ++ lib/private/files/storage/storage.php | 8 + lib/private/files/storage/wrapper/wrapper.php | 7 + lib/private/files/view.php | 63 ++++--- tests/lib/files/cache/updater.php | 41 ++--- 9 files changed, 161 insertions(+), 150 deletions(-) diff --git a/apps/dav/lib/connector/sabre/file.php b/apps/dav/lib/connector/sabre/file.php index ef7b9891dc..c66f627c0a 100644 --- a/apps/dav/lib/connector/sabre/file.php +++ b/apps/dav/lib/connector/sabre/file.php @@ -192,7 +192,7 @@ class File extends Node implements IFile { } // since we skipped the view we need to scan and emit the hooks ourselves - $this->fileView->getUpdater()->update($this->path); + $storage->getUpdater()->update($internalPath); if ($view) { $this->emitPostHooks($exists); @@ -438,7 +438,7 @@ class File extends Node implements IFile { $this->fileView->changeLock($targetPath, ILockingProvider::LOCK_SHARED); // since we skipped the view we need to scan and emit the hooks ourselves - $this->fileView->getUpdater()->update($targetPath); + $targetStorage->getUpdater()->update($targetInternalPath); $this->emitPostHooks($exists, $targetPath); diff --git a/apps/encryption/lib/migration.php b/apps/encryption/lib/migration.php index 1a7c2e9877..0b691624a0 100644 --- a/apps/encryption/lib/migration.php +++ b/apps/encryption/lib/migration.php @@ -50,7 +50,7 @@ class Migration { */ public function __construct(IConfig $config, View $view, IDBConnection $connection, ILogger $logger) { $this->view = $view; - $this->view->getUpdater()->disable(); + $this->view->disableCacheUpdate(); $this->connection = $connection; $this->moduleId = \OCA\Encryption\Crypto\Encryption::ID; $this->config = $config; diff --git a/apps/files_trashbin/lib/trashbin.php b/apps/files_trashbin/lib/trashbin.php index 8f0fe745a4..9497553079 100644 --- a/apps/files_trashbin/lib/trashbin.php +++ b/apps/files_trashbin/lib/trashbin.php @@ -235,7 +235,7 @@ class Trashbin { return false; } - $ownerView->getUpdater()->rename('/files/' . $ownerPath, $trashPath); + $trashStorage->getUpdater()->renameFromStorage($sourceStorage, $sourceInternalPath, $trashInternalPath); if ($sizeOfAddedFiles !== false) { $size = $sizeOfAddedFiles; @@ -323,7 +323,7 @@ class Trashbin { $result = $targetStorage->moveFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); if ($result) { - $view->getUpdater()->rename($source, $target); + $targetStorage->getUpdater()->renameFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); } return $result; } @@ -345,7 +345,7 @@ class Trashbin { $result = $targetStorage->copyFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); if ($result) { - $view->getUpdater()->update($target); + $targetStorage->getUpdater()->update($targetInternalPath); } return $result; } diff --git a/lib/private/files/cache/updater.php b/lib/private/files/cache/updater.php index 85dcc8589d..4d11fa3a25 100644 --- a/lib/private/files/cache/updater.php +++ b/lib/private/files/cache/updater.php @@ -29,8 +29,6 @@ namespace OC\Files\Cache; /** * Update the cache and propagate changes * - * Unlike most other classes an Updater is not related to a specific storage but handles updates for all storages in a users filesystem. - * This is needed because the propagation of mtime and etags need to cross storage boundaries */ class Updater { /** @@ -39,21 +37,33 @@ class Updater { protected $enabled = true; /** - * @var \OC\Files\View + * @var \OC\Files\Storage\Storage */ - protected $view; + protected $storage; /** - * @var \OC\Files\Cache\ChangePropagator + * @var \OC\Files\Cache\Propagator */ protected $propagator; /** - * @param \OC\Files\View $view the view the updater works on, usually the view of the logged in user + * @var Scanner */ - public function __construct($view) { - $this->view = $view; - $this->propagator = new ChangePropagator($view); + protected $scanner; + + /** + * @var Cache + */ + protected $cache; + + /** + * @param \OC\Files\Storage\Storage $storage + */ + public function __construct(\OC\Files\Storage\Storage $storage) { + $this->storage = $storage; + $this->propagator = $storage->getPropagator(); + $this->scanner = $storage->getScanner(); + $this->cache = $storage->getCache(); } /** @@ -73,7 +83,7 @@ class Updater { /** * Get the propagator for etags and mtime for the view the updater works on * - * @return ChangePropagator + * @return Propagator */ public function getPropagator() { return $this->propagator; @@ -89,8 +99,7 @@ class Updater { if (Scanner::isPartialFile($path)) { return; } - $this->propagator->addChange($path); - $this->propagator->propagateChanges($time); + $this->propagator->propagateChange($path, $time); } /** @@ -103,20 +112,14 @@ class Updater { if (!$this->enabled or Scanner::isPartialFile($path)) { return; } - /** - * @var \OC\Files\Storage\Storage $storage - * @var string $internalPath - */ - list($storage, $internalPath) = $this->view->resolvePath($path); - if ($storage) { - $this->propagator->addChange($path); - $cache = $storage->getCache($internalPath); - $scanner = $storage->getScanner($internalPath); - $data = $scanner->scan($internalPath, Scanner::SCAN_SHALLOW, -1, false); - $this->correctParentStorageMtime($storage, $internalPath); - $cache->correctFolderSize($internalPath, $data); - $this->propagator->propagateChanges($time); + if (is_null($time)) { + $time = time(); } + + $data = $this->scanner->scan($path, Scanner::SCAN_SHALLOW, -1, false); + $this->correctParentStorageMtime($path); + $this->cache->correctFolderSize($path, $data); + $this->propagator->propagateChange($path, $time); } /** @@ -128,87 +131,71 @@ class Updater { if (!$this->enabled or Scanner::isPartialFile($path)) { return; } - /** - * @var \OC\Files\Storage\Storage $storage - * @var string $internalPath - */ - list($storage, $internalPath) = $this->view->resolvePath($path); - if ($storage) { - $parent = dirname($internalPath); - if ($parent === '.') { - $parent = ''; - } - $this->propagator->addChange($path); - $cache = $storage->getCache($internalPath); - $cache->remove($internalPath); - $cache->correctFolderSize($parent); - $this->correctParentStorageMtime($storage, $internalPath); - $this->propagator->propagateChanges(); + + $parent = dirname($path); + if ($parent === '.') { + $parent = ''; } + + $this->cache->remove($path); + $this->cache->correctFolderSize($parent); + $this->correctParentStorageMtime($path); + $this->propagator->propagateChange($path, time()); } /** * Rename a file or folder in the cache and update the size, etag and mtime of the parent folders * + * @param \OC\Files\Storage\Storage $sourceStorage * @param string $source * @param string $target */ - public function rename($source, $target) { + public function renameFromStorage(\OC\Files\Storage\Storage $sourceStorage, $source, $target) { if (!$this->enabled or Scanner::isPartialFile($source) or Scanner::isPartialFile($target)) { return; } - /** - * @var \OC\Files\Storage\Storage $sourceStorage - * @var \OC\Files\Storage\Storage $targetStorage - * @var string $sourceInternalPath - * @var string $targetInternalPath - */ - list($sourceStorage, $sourceInternalPath) = $this->view->resolvePath($source); - // if it's a moved mountpoint we dont need to do anything - if ($sourceInternalPath === '') { - return; - } - list($targetStorage, $targetInternalPath) = $this->view->resolvePath($target); - if ($sourceStorage && $targetStorage) { - $targetCache = $targetStorage->getCache($sourceInternalPath); - if ($sourceStorage->getCache($sourceInternalPath)->inCache($sourceInternalPath)) { - if ($targetCache->inCache($targetInternalPath)) { - $targetCache->remove($targetInternalPath); - } - if ($sourceStorage === $targetStorage) { - $targetCache->move($sourceInternalPath, $targetInternalPath); - } else { - $targetCache->moveFromCache($sourceStorage->getCache(), $sourceInternalPath, $targetInternalPath); - } + $time = time(); + + $sourceCache = $sourceStorage->getCache($source); + $sourceUpdater = $sourceStorage->getUpdater(); + $sourcePropagator = $sourceStorage->getPropagator(); + + if ($sourceCache->inCache($source)) { + if ($this->cache->inCache($target)) { + $this->cache->remove($target); } - if (pathinfo($sourceInternalPath, PATHINFO_EXTENSION) !== pathinfo($targetInternalPath, PATHINFO_EXTENSION)) { - // handle mime type change - $mimeType = $targetStorage->getMimeType($targetInternalPath); - $fileId = $targetCache->getId($targetInternalPath); - $targetCache->update($fileId, array('mimetype' => $mimeType)); + if ($sourceStorage === $this->storage) { + $this->cache->move($source, $target); + } else { + $this->cache->moveFromCache($sourceCache, $source, $target); } - - $targetCache->correctFolderSize($sourceInternalPath); - $targetCache->correctFolderSize($targetInternalPath); - $this->correctParentStorageMtime($sourceStorage, $sourceInternalPath); - $this->correctParentStorageMtime($targetStorage, $targetInternalPath); - $this->updateStorageMTimeOnly($targetStorage, $targetInternalPath); - $this->propagator->addChange($source); - $this->propagator->addChange($target); - $this->propagator->propagateChanges(); } + + if (pathinfo($source, PATHINFO_EXTENSION) !== pathinfo($target, PATHINFO_EXTENSION)) { + // handle mime type change + $mimeType = $this->storage->getMimeType($target); + $fileId = $this->cache->getId($target); + $this->cache->update($fileId, ['mimetype' => $mimeType]); + } + + $sourceCache->correctFolderSize($source); + $this->cache->correctFolderSize($target); + $sourceUpdater->correctParentStorageMtime($source); + $this->correctParentStorageMtime($target); + $this->updateStorageMTimeOnly($target); + $sourcePropagator->propagateChange($source, $time); + $this->propagator->propagateChange($target, $time); } - private function updateStorageMTimeOnly($storage, $internalPath) { - $cache = $storage->getCache(); - $fileId = $cache->getId($internalPath); + private function updateStorageMTimeOnly($internalPath) { + $fileId = $this->cache->getId($internalPath); if ($fileId !== -1) { - $cache->update( + $this->cache->update( $fileId, [ 'mtime' => null, // this magic tells it to not overwrite mtime - 'storage_mtime' => $storage->filemtime($internalPath) + 'storage_mtime' => $this->storage->filemtime($internalPath) ] ); } @@ -217,20 +204,13 @@ class Updater { /** * update the storage_mtime of the direct parent in the cache to the mtime from the storage * - * @param \OC\Files\Storage\Storage $storage * @param string $internalPath */ - private function correctParentStorageMtime($storage, $internalPath) { - $cache = $storage->getCache(); - $parentId = $cache->getParentId($internalPath); + public function correctParentStorageMtime($internalPath) { + $parentId = $this->cache->getParentId($internalPath); $parent = dirname($internalPath); if ($parentId != -1) { - $cache->update($parentId, array('storage_mtime' => $storage->filemtime($parent))); + $this->cache->update($parentId, array('storage_mtime' => $this->storage->filemtime($parent))); } } - - public function __destruct() { - // propagate any leftover changes - $this->propagator->propagateChanges(); - } } diff --git a/lib/private/files/storage/common.php b/lib/private/files/storage/common.php index 3772b442b5..0cd67e343f 100644 --- a/lib/private/files/storage/common.php +++ b/lib/private/files/storage/common.php @@ -39,6 +39,7 @@ namespace OC\Files\Storage; use OC\Files\Cache\Cache; use OC\Files\Cache\Propagator; use OC\Files\Cache\Scanner; +use OC\Files\Cache\Updater; use OC\Files\Filesystem; use OC\Files\Cache\Watcher; use OCP\Files\FileNameTooLongException; @@ -67,6 +68,7 @@ abstract class Common implements Storage { protected $watcher; protected $propagator; protected $storageCache; + protected $updater; protected $mountOptions = []; @@ -363,6 +365,16 @@ abstract class Common implements Storage { return $this->propagator; } + public function getUpdater($storage = null) { + if (!$storage) { + $storage = $this; + } + if (!isset($this->updater)) { + $this->updater = new Updater($storage); + } + return $this->updater; + } + public function getStorageCache($storage = null) { if (!$storage) { $storage = $this; diff --git a/lib/private/files/storage/storage.php b/lib/private/files/storage/storage.php index 3399c71789..fb59752ede 100644 --- a/lib/private/files/storage/storage.php +++ b/lib/private/files/storage/storage.php @@ -75,6 +75,14 @@ interface Storage extends \OCP\Files\Storage { */ public function getPropagator($storage = null); + /** + * get a updater instance for the cache + * + * @param \OC\Files\Storage\Storage (optional) the storage to pass to the watcher + * @return \OC\Files\Cache\Updater + */ + public function getUpdater($storage = null); + /** * @return \OC\Files\Cache\Storage */ diff --git a/lib/private/files/storage/wrapper/wrapper.php b/lib/private/files/storage/wrapper/wrapper.php index 5327b94211..81284c7aa6 100644 --- a/lib/private/files/storage/wrapper/wrapper.php +++ b/lib/private/files/storage/wrapper/wrapper.php @@ -437,6 +437,13 @@ class Wrapper implements \OC\Files\Storage\Storage { return $this->storage->getPropagator($storage); } + public function getUpdater($storage = null) { + if (!$storage) { + $storage = $this; + } + return $this->storage->getUpdater($storage); + } + /** * @return \OC\Files\Cache\Storage */ diff --git a/lib/private/files/view.php b/lib/private/files/view.php index 3ad9d36a38..7854790d0e 100644 --- a/lib/private/files/view.php +++ b/lib/private/files/view.php @@ -46,6 +46,7 @@ namespace OC\Files; use Icewind\Streams\CallbackWrapper; use OC\Files\Cache\Updater; use OC\Files\Mount\MoveableMount; +use OC\Files\Storage\Storage; use OC\User\User; use OCP\Files\FileNameTooLongException; use OCP\Files\InvalidCharacterInPathException; @@ -76,9 +77,6 @@ class View { /** @var string */ private $fakeRoot = ''; - /** @var \OC\Files\Cache\Updater */ - protected $updater; - /** * @var \OCP\Lock\ILockingProvider */ @@ -86,6 +84,8 @@ class View { private $lockingEnabled; + private $updaterEnabled = true; + /** * @param string $root * @throws \Exception If $root contains an invalid path @@ -99,7 +99,6 @@ class View { } $this->fakeRoot = $root; - $this->updater = new Updater($this); $this->lockingProvider = \OC::$server->getLockingProvider(); $this->lockingEnabled = !($this->lockingProvider instanceof \OC\Lock\NoopLockingProvider); } @@ -286,6 +285,35 @@ class View { } } + public function disableCacheUpdate() { + $this->updaterEnabled = false; + } + + public function enableCacheUpdate() { + $this->updaterEnabled = true; + } + + protected function writeUpdate(Storage $storage, $internalPath, $time = null) { + if ($this->updaterEnabled) { + if (is_null($time)) { + $time = time(); + } + $storage->getUpdater()->update($internalPath, $time); + } + } + + protected function removeUpdate(Storage $storage, $internalPath) { + if ($this->updaterEnabled) { + $storage->getUpdater()->remove($internalPath); + } + } + + protected function renameUpdate(Storage $sourceStorage, Storage $targetStorage, $sourceInternalPath, $targetInternalPath) { + if ($this->updaterEnabled) { + $targetStorage->getUpdater()->renameFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); + } + } + /** * @param string $path * @return bool|mixed @@ -569,7 +597,7 @@ class View { fclose($target); fclose($data); - $this->updater->update($path); + $this->writeUpdate($storage, $internalPath); $this->changeLock($path, ILockingProvider::LOCK_SHARED); @@ -703,10 +731,11 @@ class View { if ((Cache\Scanner::isPartialFile($path1) && !Cache\Scanner::isPartialFile($path2)) && $result !== false) { // if it was a rename from a part file to a regular file it was a write and not a rename operation - $this->updater->update($path2); + + $this->writeUpdate($storage2, $internalPath2); } else if ($result) { if ($internalPath1 !== '') { // dont do a cache update for moved mounts - $this->updater->rename($path1, $path2); + $this->renameUpdate($storage1, $storage2, $internalPath1, $internalPath2); } } @@ -803,7 +832,7 @@ class View { $result = $storage2->copyFromStorage($storage1, $internalPath1, $internalPath2); } - $this->updater->update($path2); + $this->writeUpdate($storage2, $internalPath2); $this->changeLock($path2, ILockingProvider::LOCK_SHARED); $lockTypePath2 = ILockingProvider::LOCK_SHARED; @@ -1013,6 +1042,7 @@ class View { } $run = $this->runHooks($hooks, $path); + /** @var \OC\Files\Storage\Storage $storage */ list($storage, $internalPath) = Filesystem::resolvePath($absolutePath . $postFix); if ($run and $storage) { if (in_array('write', $hooks) || in_array('delete', $hooks)) { @@ -1034,13 +1064,13 @@ class View { } if (in_array('delete', $hooks) and $result) { - $this->updater->remove($path); + $this->removeUpdate($storage, $internalPath); } if (in_array('write', $hooks) and $operation !== 'fopen') { - $this->updater->update($path); + $this->writeUpdate($storage, $internalPath); } if (in_array('touch', $hooks)) { - $this->updater->update($path, $extraParam); + $this->writeUpdate($storage, $internalPath, $extraParam); } if ((in_array('write', $hooks) || in_array('delete', $hooks)) && ($operation !== 'fopen' || $result === false)) { @@ -1205,7 +1235,7 @@ class View { } else if (!Cache\Scanner::isPartialFile($internalPath) && $watcher->needsUpdate($internalPath, $data)) { $this->lockFile($relativePath, ILockingProvider::LOCK_SHARED); $watcher->update($internalPath, $data); - $this->updater->propagate($relativePath); + $storage->getPropagator()->propagateChange($internalPath, time()); $data = $cache->get($internalPath); $this->unlockFile($relativePath, ILockingProvider::LOCK_SHARED); } @@ -1242,7 +1272,7 @@ class View { if ($storage) { $data = $this->getCacheEntry($storage, $internalPath, $relativePath); - if(!is_array($data)) { + if (!is_array($data)) { return false; } @@ -1691,13 +1721,6 @@ class View { ); } - /** - * @return Updater - */ - public function getUpdater() { - return $this->updater; - } - /** * @param string $path * @param string $fileName diff --git a/tests/lib/files/cache/updater.php b/tests/lib/files/cache/updater.php index b7e76aeace..807dcd596f 100644 --- a/tests/lib/files/cache/updater.php +++ b/tests/lib/files/cache/updater.php @@ -39,9 +39,7 @@ class Updater extends \Test\TestCase { $this->loginAsUser(); $this->storage = new Temporary(array()); - Filesystem::mount($this->storage, array(), '/'); - $this->view = new View(''); - $this->updater = new \OC\Files\Cache\Updater($this->view); + $this->updater = $this->storage->getUpdater(); $this->cache = $this->storage->getCache(); } @@ -56,7 +54,7 @@ class Updater extends \Test\TestCase { $this->storage->file_put_contents('foo.txt', 'bar'); $this->assertFalse($this->cache->inCache('foo.txt')); - $this->updater->update('/foo.txt'); + $this->updater->update('foo.txt'); $this->assertTrue($this->cache->inCache('foo.txt')); $cached = $this->cache->get('foo.txt'); @@ -65,7 +63,9 @@ class Updater extends \Test\TestCase { } public function testUpdatedFile() { - $this->view->file_put_contents('/foo.txt', 'bar'); + $this->storage->file_put_contents('foo.txt', 'bar'); + $this->updater->update('foo.txt'); + $cached = $this->cache->get('foo.txt'); $this->assertEquals(3, $cached['size']); $this->assertEquals('text/plain', $cached['mimetype']); @@ -131,7 +131,7 @@ class Updater extends \Test\TestCase { $this->assertTrue($this->cache->inCache('foo.txt')); $this->assertFalse($this->cache->inCache('bar.txt')); - $this->updater->rename('foo.txt', 'bar.txt'); + $this->updater->renameFromStorage($this->storage, 'foo.txt', 'bar.txt'); $this->assertFalse($this->cache->inCache('foo.txt')); $this->assertTrue($this->cache->inCache('bar.txt')); @@ -149,7 +149,7 @@ class Updater extends \Test\TestCase { $cached = $this->cache->get('bar.txt'); - $this->updater->rename('foo.txt', 'bar.txt'); + $this->updater->renameFromStorage($this->storage, 'foo.txt', 'bar.txt'); $this->assertFalse($this->cache->inCache('foo.txt')); $this->assertTrue($this->cache->inCache('bar.txt')); @@ -186,7 +186,7 @@ class Updater extends \Test\TestCase { // some storages (like Dropbox) change storage mtime on rename $this->storage->touch('sub2/bar.txt', $testmtime); - $this->updater->rename('sub/foo.txt', 'sub2/bar.txt'); + $this->updater->renameFromStorage($this->storage, 'sub/foo.txt', 'sub2/bar.txt'); $cachedTargetParent = $this->cache->get('sub2'); $cachedTarget = $this->cache->get('sub2/bar.txt'); @@ -212,26 +212,6 @@ class Updater extends \Test\TestCase { $this->assertFalse($this->cache->inCache('foo.txt')); } - public function testMoveDisabled() { - $this->storage->file_put_contents('foo.txt', 'qwerty'); - $this->updater->update('foo.txt'); - - $this->assertTrue($this->cache->inCache('foo.txt')); - $this->assertFalse($this->cache->inCache('bar.txt')); - $cached = $this->cache->get('foo.txt'); - - $this->storage->rename('foo.txt', 'bar.txt'); - - $this->assertTrue($this->cache->inCache('foo.txt')); - $this->assertFalse($this->cache->inCache('bar.txt')); - - $this->updater->disable(); - $this->updater->rename('foo.txt', 'bar.txt'); - - $this->assertTrue($this->cache->inCache('foo.txt')); - $this->assertFalse($this->cache->inCache('bar.txt')); - } - public function testMoveCrossStorage() { $storage2 = new Temporary(array()); $cache2 = $storage2->getCache(); @@ -251,7 +231,7 @@ class Updater extends \Test\TestCase { $this->assertTrue($this->cache->inCache('foo.txt')); $this->assertFalse($cache2->inCache('bar.txt')); - $this->updater->rename('foo.txt', 'bar/bar.txt'); + $storage2->getUpdater()->renameFromStorage($this->storage, 'foo.txt', 'bar.txt'); $this->assertFalse($this->cache->inCache('foo.txt')); $this->assertTrue($cache2->inCache('bar.txt')); @@ -288,7 +268,8 @@ class Updater extends \Test\TestCase { $cached[] = $this->cache->get('foo/bar/bar.txt'); // add extension to trigger the possible mimetype change - $this->view->rename('/foo', '/bar/foo.b'); + $storage2->moveFromStorage($this->storage, 'foo', 'foo.b'); + $storage2->getUpdater()->renameFromStorage($this->storage, 'foo', 'foo.b'); $this->assertFalse($this->cache->inCache('foo')); $this->assertFalse($this->cache->inCache('foo/foo.txt'));