From 3dfef53ccd4dc97bdd28556d0519947d6f08a3a3 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 16 Feb 2024 12:34:07 +0100 Subject: [PATCH] perf: cache path by id to speedup getting nodes by id Signed-off-by: Robin Appelman --- lib/private/Files/Node/Root.php | 30 +++++++++++++- lib/private/Server.php | 5 ++- tests/lib/Files/Node/FileTest.php | 25 +++++++----- tests/lib/Files/Node/FolderTest.php | 46 +++++++++++----------- tests/lib/Files/Node/HookConnectorTest.php | 10 ++++- tests/lib/Files/Node/IntegrationTest.php | 10 ++++- tests/lib/Files/Node/NodeTest.php | 19 +++++++-- tests/lib/Files/Node/RootTest.php | 27 ++++++++++--- 8 files changed, 124 insertions(+), 48 deletions(-) diff --git a/lib/private/Files/Node/Root.php b/lib/private/Files/Node/Root.php index bc76640966e43..71bc7a6da6bf0 100644 --- a/lib/private/Files/Node/Root.php +++ b/lib/private/Files/Node/Root.php @@ -49,6 +49,8 @@ use OCP\Files\Node as INode; use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; +use OCP\ICache; +use OCP\ICacheFactory; use OCP\IUser; use OCP\IUserManager; use Psr\Log\LoggerInterface; @@ -81,6 +83,7 @@ class Root extends Folder implements IRootFolder { private LoggerInterface $logger; private IUserManager $userManager; private IEventDispatcher $eventDispatcher; + private ICache $pathByIdCache; /** * @param Manager $manager @@ -94,7 +97,8 @@ public function __construct( IUserMountCache $userMountCache, LoggerInterface $logger, IUserManager $userManager, - IEventDispatcher $eventDispatcher + IEventDispatcher $eventDispatcher, + ICacheFactory $cacheFactory, ) { parent::__construct($this, $view, ''); $this->mountManager = $manager; @@ -107,6 +111,7 @@ public function __construct( $eventDispatcher->addListener(FilesystemTornDownEvent::class, function () { $this->userFolderCache = new CappedMemoryCache(); }); + $this->pathByIdCache = $cacheFactory->createLocal('path-by-id'); } /** @@ -406,7 +411,28 @@ public function getUserMountCache() { } public function getFirstNodeByIdInPath(int $id, string $path): ?INode { - return current($this->getByIdInPath($id, $path)); + // scope the cache by user, so we don't return nodes for different users + if ($this->user) { + $cachedPath = $this->pathByIdCache->get($this->user->getUID() . '::' . $id); + if ($cachedPath && str_starts_with($path, $cachedPath)) { + // getting the node by path is significantly cheaper than finding it by id + $node = $this->get($cachedPath); + // by validating that the cached path still has the requested fileid we can work around the need to invalidate the cached path + // if the cached path is invalid or a different file now we fall back to the uncached logic + if ($node && $node->getId() === $id) { + return $node; + } + } + } + $node = current($this->getByIdInPath($id, $path)); + if (!$node) { + return null; + } + + if ($this->user) { + $this->pathByIdCache->set($this->user->getUID() . '::' . $id, $node->getPath()); + } + return $node; } /** diff --git a/lib/private/Server.php b/lib/private/Server.php index 1aedd7d06ac56..c956da4fdd80e 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -448,14 +448,17 @@ public function __construct($webRoot, \OC\Config $config) { $this->registerService('RootFolder', function (ContainerInterface $c) { $manager = \OC\Files\Filesystem::getMountManager(); $view = new View(); + /** @var IUserSession $userSession */ + $userSession = $c->get(IUserSession::class); $root = new Root( $manager, $view, - null, + $userSession->getUser(), $c->get(IUserMountCache::class), $this->get(LoggerInterface::class), $this->get(IUserManager::class), $this->get(IEventDispatcher::class), + $this->get(ICacheFactory::class), ); $previewConnector = new \OC\Preview\WatcherConnector( diff --git a/tests/lib/Files/Node/FileTest.php b/tests/lib/Files/Node/FileTest.php index 218e253172746..b245f5e2db540 100644 --- a/tests/lib/Files/Node/FileTest.php +++ b/tests/lib/Files/Node/FileTest.php @@ -39,7 +39,7 @@ protected function getViewDeleteMethod() { public function testGetContent() { /** @var \OC\Files\Node\Root|\PHPUnit\Framework\MockObject\MockObject $root */ $root = $this->getMockBuilder('\OC\Files\Node\Root') - ->setConstructorArgs([$this->manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher]) + ->setConstructorArgs([$this->manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) ->getMock(); $hook = function ($file) { @@ -69,7 +69,7 @@ public function testGetContentNotPermitted() { /** @var \OC\Files\Node\Root|\PHPUnit\Framework\MockObject\MockObject $root */ $root = $this->getMockBuilder('\OC\Files\Node\Root') - ->setConstructorArgs([$this->manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher]) + ->setConstructorArgs([$this->manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) ->getMock(); $root->expects($this->any()) @@ -88,7 +88,7 @@ public function testGetContentNotPermitted() { public function testPutContent() { /** @var \OC\Files\Node\Root|\PHPUnit\Framework\MockObject\MockObject $root */ $root = $this->getMockBuilder('\OC\Files\Node\Root') - ->setConstructorArgs([$this->manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher]) + ->setConstructorArgs([$this->manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) ->getMock(); $root->expects($this->any()) @@ -115,7 +115,7 @@ public function testPutContentNotPermitted() { /** @var \OC\Files\Node\Root|\PHPUnit\Framework\MockObject\MockObject $root */ $root = $this->getMockBuilder('\OC\Files\Node\Root') - ->setConstructorArgs([$this->manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher]) + ->setConstructorArgs([$this->manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) ->getMock(); $this->view->expects($this->once()) @@ -130,7 +130,7 @@ public function testPutContentNotPermitted() { public function testGetMimeType() { /** @var \OC\Files\Node\Root|\PHPUnit\Framework\MockObject\MockObject $root */ $root = $this->getMockBuilder('\OC\Files\Node\Root') - ->setConstructorArgs([$this->manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher]) + ->setConstructorArgs([$this->manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) ->getMock(); $this->view->expects($this->once()) @@ -154,7 +154,8 @@ public function testFOpenRead() { $this->userMountCache, $this->logger, $this->userManager, - $this->eventDispatcher + $this->eventDispatcher, + $this->cacheFactory, ); $hook = function ($file) { @@ -190,7 +191,8 @@ public function testFOpenWrite() { $this->userMountCache, $this->logger, $this->userManager, - $this->eventDispatcher + $this->eventDispatcher, + $this->cacheFactory, ); $hooksCalled = 0; $hook = function ($file) use (&$hooksCalled) { @@ -230,7 +232,8 @@ public function testFOpenReadNotPermitted() { $this->userMountCache, $this->logger, $this->userManager, - $this->eventDispatcher + $this->eventDispatcher, + $this->cacheFactory, ); $hook = function ($file) { throw new \Exception('Hooks are not supposed to be called'); @@ -256,7 +259,8 @@ public function testFOpenReadWriteNoReadPermissions() { $this->userMountCache, $this->logger, $this->userManager, - $this->eventDispatcher + $this->eventDispatcher, + $this->cacheFactory, ); $hook = function () { throw new \Exception('Hooks are not supposed to be called'); @@ -282,7 +286,8 @@ public function testFOpenReadWriteNoWritePermissions() { $this->userMountCache, $this->logger, $this->userManager, - $this->eventDispatcher + $this->eventDispatcher, + $this->cacheFactory, ); $hook = function () { throw new \Exception('Hooks are not supposed to be called'); diff --git a/tests/lib/Files/Node/FolderTest.php b/tests/lib/Files/Node/FolderTest.php index 0bcf69c5c1303..a219222d008e1 100644 --- a/tests/lib/Files/Node/FolderTest.php +++ b/tests/lib/Files/Node/FolderTest.php @@ -68,7 +68,7 @@ public function testGetDirectoryContent() { * @var \OC\Files\View | \PHPUnit\Framework\MockObject\MockObject $view */ $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) ->getMock(); $root->expects($this->any()) ->method('getUser') @@ -101,7 +101,7 @@ public function testGet() { $manager = $this->createMock(Manager::class); $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) ->getMock(); $root->expects($this->any()) ->method('getUser') @@ -120,7 +120,7 @@ public function testNodeExists() { $manager = $this->createMock(Manager::class); $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) ->getMock(); $root->expects($this->any()) ->method('getUser') @@ -140,7 +140,7 @@ public function testNodeExistsNotExists() { $manager = $this->createMock(Manager::class); $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) ->getMock(); $root->expects($this->any()) ->method('getUser') @@ -158,7 +158,7 @@ public function testNewFolder() { $manager = $this->createMock(Manager::class); $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) ->getMock(); $root->expects($this->any()) ->method('getUser') @@ -182,7 +182,7 @@ public function testNewFolderDeepParent() { $manager = $this->createMock(Manager::class); $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) ->getMock(); $root->expects($this->any()) ->method('getUser') @@ -209,7 +209,7 @@ public function testNewFolderNotPermitted() { $manager = $this->createMock(Manager::class); $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) ->getMock(); $root->method('getUser') ->willReturn($this->user); @@ -226,7 +226,7 @@ public function testNewFile() { $manager = $this->createMock(Manager::class); $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) ->getMock(); $root->expects($this->any()) ->method('getUser') @@ -253,7 +253,7 @@ public function testNewFileNotPermitted() { $manager = $this->createMock(Manager::class); $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) ->getMock(); $root->method('getUser') ->willReturn($this->user); @@ -270,7 +270,7 @@ public function testGetFreeSpace() { $manager = $this->createMock(Manager::class); $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) ->getMock(); $root->method('getUser') ->willReturn($this->user); @@ -287,7 +287,7 @@ public function testSearch() { $manager = $this->createMock(Manager::class); $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) ->getMock(); $root->method('getUser') ->willReturn($this->user); @@ -328,7 +328,7 @@ public function testSearchInRoot() { $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) ->setMethods(['getUser', 'getMountsIn', 'getMount']) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) ->getMock(); $root->expects($this->any()) ->method('getUser') @@ -368,7 +368,7 @@ public function testSearchInStorageRoot() { $manager = $this->createMock(Manager::class); $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) ->getMock(); $root->method('getUser') ->willReturn($this->user); @@ -408,7 +408,7 @@ public function testSearchSubStorages() { $manager = $this->createMock(Manager::class); $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) ->getMock(); $root->expects($this->any()) ->method('getUser') @@ -478,7 +478,7 @@ public function testGetById() { $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) ->setMethods(['getMountsIn', 'getMount']) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) ->getMock(); $storage = $this->createMock(\OC\Files\Storage\Storage::class); $mount = new MountPoint($storage, '/bar'); @@ -525,7 +525,7 @@ public function testGetByIdMountRoot() { $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) ->setMethods(['getMountsIn', 'getMount']) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) ->getMock(); $storage = $this->createMock(\OC\Files\Storage\Storage::class); $mount = new MountPoint($storage, '/bar'); @@ -568,7 +568,7 @@ public function testGetByIdOutsideFolder() { $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) ->setMethods(['getMountsIn', 'getMount']) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) ->getMock(); $storage = $this->createMock(\OC\Files\Storage\Storage::class); $mount = new MountPoint($storage, '/bar'); @@ -610,7 +610,7 @@ public function testGetByIdMultipleStorages() { $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) ->setMethods(['getMountsIn', 'getMount']) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) ->getMock(); $storage = $this->createMock(\OC\Files\Storage\Storage::class); $mount1 = new MountPoint($storage, '/bar'); @@ -672,7 +672,7 @@ public function testGetUniqueName($name, $existingFiles, $expected) { $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) ->setMethods(['getUser', 'getMountsIn', 'getMount']) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) ->getMock(); $view->expects($this->any()) @@ -697,7 +697,7 @@ public function testRecent(): void { /** @var \PHPUnit\Framework\MockObject\MockObject|\OC\Files\Node\Root $root */ $root = $this->getMockBuilder(Root::class) ->setMethods(['getUser', 'getMountsIn', 'getMount']) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) ->getMock(); /** @var \PHPUnit\Framework\MockObject\MockObject|\OC\Files\FileInfo $folderInfo */ $folderInfo = $this->getMockBuilder(FileInfo::class) @@ -762,7 +762,7 @@ public function testRecentFolder() { /** @var \PHPUnit\Framework\MockObject\MockObject|\OC\Files\Node\Root $root */ $root = $this->getMockBuilder(Root::class) ->setMethods(['getUser', 'getMountsIn', 'getMount']) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) ->getMock(); /** @var \PHPUnit\Framework\MockObject\MockObject|\OC\Files\FileInfo $folderInfo */ $folderInfo = $this->getMockBuilder(FileInfo::class) @@ -826,7 +826,7 @@ public function testRecentJail() { /** @var \PHPUnit\Framework\MockObject\MockObject|\OC\Files\Node\Root $root */ $root = $this->getMockBuilder(Root::class) ->setMethods(['getUser', 'getMountsIn', 'getMount']) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) ->getMock(); /** @var \PHPUnit\Framework\MockObject\MockObject|\OC\Files\FileInfo $folderInfo */ $folderInfo = $this->getMockBuilder(FileInfo::class) @@ -910,7 +910,7 @@ public function testSearchSubStoragesLimitOffset(int $offset, int $limit, array $manager = $this->createMock(Manager::class); $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) ->getMock(); $root->expects($this->any()) ->method('getUser') diff --git a/tests/lib/Files/Node/HookConnectorTest.php b/tests/lib/Files/Node/HookConnectorTest.php index 0501c175a5f6a..80a94462bba2c 100644 --- a/tests/lib/Files/Node/HookConnectorTest.php +++ b/tests/lib/Files/Node/HookConnectorTest.php @@ -13,6 +13,7 @@ use OC\Files\Node\Root; use OC\Files\Storage\Temporary; use OC\Files\View; +use OC\Memcache\ArrayCache; use OCP\EventDispatcher\GenericEvent as APIGenericEvent; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Events\Node\AbstractNodeEvent; @@ -30,6 +31,7 @@ use OCP\Files\Events\Node\NodeTouchedEvent; use OCP\Files\Events\Node\NodeWrittenEvent; use OCP\Files\Node; +use OCP\ICacheFactory; use OCP\IUserManager; use Psr\Log\LoggerInterface; use Symfony\Component\EventDispatcher\GenericEvent; @@ -67,6 +69,11 @@ protected function setUp(): void { // this will setup the FS $this->loginAsUser($this->userId); $this->registerMount($this->userId, new Temporary(), '/' . $this->userId . '/files/'); + $cacheFactory = $this->createMock(ICacheFactory::class); + $cacheFactory->method('createLocal') + ->willReturnCallback(function () { + return new ArrayCache(); + }); $this->view = new View(); $this->root = new Root( Filesystem::getMountManager(), @@ -75,7 +82,8 @@ protected function setUp(): void { \OC::$server->getUserMountCache(), $this->createMock(LoggerInterface::class), $this->createMock(IUserManager::class), - $this->createMock(IEventDispatcher::class) + $this->createMock(IEventDispatcher::class), + $cacheFactory, ); $this->eventDispatcher = \OC::$server->query(IEventDispatcher::class); } diff --git a/tests/lib/Files/Node/IntegrationTest.php b/tests/lib/Files/Node/IntegrationTest.php index 5ef5a134e1bb2..dacc65ad4d120 100644 --- a/tests/lib/Files/Node/IntegrationTest.php +++ b/tests/lib/Files/Node/IntegrationTest.php @@ -11,8 +11,10 @@ use OC\Files\Node\Root; use OC\Files\Storage\Temporary; use OC\Files\View; +use OC\Memcache\ArrayCache; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Mount\IMountManager; +use OCP\ICacheFactory; use OCP\IUserManager; use Psr\Log\LoggerInterface; use Test\Traits\UserTrait; @@ -52,6 +54,11 @@ protected function setUp(): void { $user = $this->createUser($this->getUniqueID('user'), ''); $this->loginAsUser($user->getUID()); + $cacheFactory = $this->createMock(ICacheFactory::class); + $cacheFactory->method('createLocal') + ->willReturnCallback(function () { + return new ArrayCache(); + }); $this->view = new View(); $this->root = new Root( @@ -61,7 +68,8 @@ protected function setUp(): void { \OC::$server->getUserMountCache(), $this->createMock(LoggerInterface::class), $this->createMock(IUserManager::class), - $this->createMock(IEventDispatcher::class) + $this->createMock(IEventDispatcher::class), + $cacheFactory, ); $storage = new Temporary([]); $subStorage = new Temporary([]); diff --git a/tests/lib/Files/Node/NodeTest.php b/tests/lib/Files/Node/NodeTest.php index 4f782f28a36e9..85a7f8a82f3c4 100644 --- a/tests/lib/Files/Node/NodeTest.php +++ b/tests/lib/Files/Node/NodeTest.php @@ -11,12 +11,14 @@ use OC\Files\FileInfo; use OC\Files\Mount\Manager; use OC\Files\View; +use OC\Memcache\ArrayCache; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\IRootFolder; use OCP\Files\Mount\IMountPoint; use OCP\Files\Node; use OCP\Files\NotFoundException; use OCP\Files\Storage; +use OCP\ICacheFactory; use OCP\IUser; use OCP\IUserManager; use Psr\Log\LoggerInterface; @@ -43,6 +45,8 @@ abstract class NodeTest extends \Test\TestCase { protected $userManager; /** @var IEventDispatcher|\PHPUnit\Framework\MockObject\MockObject */ protected $eventDispatcher; + /** @var ICacheFactory|\PHPUnit\Framework\MockObject\MockObject */ + protected $cacheFactory; protected function setUp(): void { parent::setUp(); @@ -63,8 +67,13 @@ protected function setUp(): void { $this->logger = $this->createMock(LoggerInterface::class); $this->userManager = $this->createMock(IUserManager::class); $this->eventDispatcher = $this->createMock(IEventDispatcher::class); + $this->cacheFactory = $this->createMock(ICacheFactory::class); + $this->cacheFactory->method('createLocal') + ->willReturnCallback(function () { + return new ArrayCache(); + }); $this->root = $this->getMockBuilder('\OC\Files\Node\Root') - ->setConstructorArgs([$this->manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->eventDispatcher]) + ->setConstructorArgs([$this->manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) ->getMock(); } @@ -174,7 +183,8 @@ public function testDeleteHooks() { $this->userMountCache, $this->logger, $this->userManager, - $this->eventDispatcher + $this->eventDispatcher, + $this->cacheFactory, ); $root->listen('\OC\Files', 'preDelete', $preListener); @@ -422,7 +432,8 @@ public function testTouchHooks() { $this->userMountCache, $this->logger, $this->userManager, - $this->eventDispatcher + $this->eventDispatcher, + $this->cacheFactory, ); $root->listen('\OC\Files', 'preTouch', $preListener); $root->listen('\OC\Files', 'postTouch', $postListener); @@ -599,7 +610,7 @@ public function moveOrCopyProvider() { public function testMoveCopyHooks($operationMethod, $viewMethod, $preHookName, $postHookName) { /** @var IRootFolder|\PHPUnit\Framework\MockObject\MockObject $root */ $root = $this->getMockBuilder('\OC\Files\Node\Root') - ->setConstructorArgs([$this->manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher]) + ->setConstructorArgs([$this->manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) ->setMethods(['get']) ->getMock(); diff --git a/tests/lib/Files/Node/RootTest.php b/tests/lib/Files/Node/RootTest.php index e24283f0931ac..d9abe2955b11a 100644 --- a/tests/lib/Files/Node/RootTest.php +++ b/tests/lib/Files/Node/RootTest.php @@ -12,8 +12,10 @@ use OC\Files\Mount\Manager; use OC\Files\Node\Folder; use OC\Files\View; +use OC\Memcache\ArrayCache; use OCP\Cache\CappedMemoryCache; use OCP\EventDispatcher\IEventDispatcher; +use OCP\ICacheFactory; use OCP\IUser; use OCP\IUserManager; use Psr\Log\LoggerInterface; @@ -36,6 +38,8 @@ class RootTest extends \Test\TestCase { private $userManager; /** @var IEventDispatcher|\PHPUnit\Framework\MockObject\MockObject */ private $eventDispatcher; + /** @var ICacheFactory|\PHPUnit\Framework\MockObject\MockObject */ + protected $cacheFactory; protected function setUp(): void { parent::setUp(); @@ -50,6 +54,11 @@ protected function setUp(): void { $this->logger = $this->createMock(LoggerInterface::class); $this->userManager = $this->createMock(IUserManager::class); $this->eventDispatcher = $this->createMock(IEventDispatcher::class); + $this->cacheFactory = $this->createMock(ICacheFactory::class); + $this->cacheFactory->method('createLocal') + ->willReturnCallback(function () { + return new ArrayCache(); + }); } /** @@ -82,7 +91,8 @@ public function testGet() { $this->userMountCache, $this->logger, $this->userManager, - $this->eventDispatcher + $this->eventDispatcher, + $this->cacheFactory, ); $view->expects($this->once()) @@ -114,7 +124,8 @@ public function testGetNotFound() { $this->userMountCache, $this->logger, $this->userManager, - $this->eventDispatcher + $this->eventDispatcher, + $this->cacheFactory, ); $view->expects($this->once()) @@ -138,7 +149,8 @@ public function testGetInvalidPath() { $this->userMountCache, $this->logger, $this->userManager, - $this->eventDispatcher + $this->eventDispatcher, + $this->cacheFactory, ); $root->get('/../foo'); @@ -156,7 +168,8 @@ public function testGetNoStorages() { $this->userMountCache, $this->logger, $this->userManager, - $this->eventDispatcher + $this->eventDispatcher, + $this->cacheFactory, ); $root->get('/bar/foo'); @@ -170,7 +183,8 @@ public function testGetUserFolder() { $this->userMountCache, $this->logger, $this->userManager, - $this->eventDispatcher + $this->eventDispatcher, + $this->cacheFactory, ); $user = $this->createMock(IUser::class); $user @@ -211,7 +225,8 @@ public function testGetUserFolderWithNoUserObj() { $this->userMountCache, $this->logger, $this->userManager, - $this->eventDispatcher + $this->eventDispatcher, + $this->cacheFactory, ); $this->userManager ->expects($this->once())