From 8085ca4cc4e0d38e33b623613b36ec0fe03e5a9f Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Tue, 29 Oct 2019 16:56:06 +0100 Subject: [PATCH 1/3] Get all shares as iterable Sometimes we need all shares or rather a specific subset of shares but creating dedicated functions is a pain. This just returns an iterable object for all shares so we can loop over them without allocating all the memory on the system. It should not be used by any user called code. But in an occ command or background job it is fine IMO. Signed-off-by: Roeland Jago Douma --- .../lib/FederatedShareProvider.php | 27 +++++++++++++++++++ apps/sharebymail/lib/ShareByMailProvider.php | 25 +++++++++++++++++ lib/private/Share20/DefaultShareProvider.php | 26 ++++++++++++++++++ lib/private/Share20/Manager.php | 7 +++++ lib/public/Share/IManager.php | 11 ++++++++ lib/public/Share/IShareProvider.php | 8 ++++++ 6 files changed, 104 insertions(+) diff --git a/apps/federatedfilesharing/lib/FederatedShareProvider.php b/apps/federatedfilesharing/lib/FederatedShareProvider.php index 67089cba9d..4d93dec18f 100644 --- a/apps/federatedfilesharing/lib/FederatedShareProvider.php +++ b/apps/federatedfilesharing/lib/FederatedShareProvider.php @@ -1100,4 +1100,31 @@ class FederatedShareProvider implements IShareProvider { return ['remote' => $remote]; } + + public function getAllShares(): iterable { + $qb = $this->dbConnection->getQueryBuilder(); + + $qb->select('*') + ->from('share') + ->where( + $qb->expr()->orX( + $qb->expr()->eq('share_type', $qb->createNamedParameter(\OCP\Share\IShare::TYPE_REMOTE)), + $qb->expr()->eq('share_type', $qb->createNamedParameter(\OCP\Share\IShare::TYPE_REMOTE_GROUP)) + ) + ); + + $cursor = $qb->execute(); + while($data = $cursor->fetch()) { + try { + $share = $this->createShareObject($data); + } catch (InvalidShare $e) { + continue; + } catch (ShareNotFound $e) { + continue; + } + + yield $share; + } + $cursor->closeCursor(); + } } diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php index 9bfe5e733f..903df175e9 100644 --- a/apps/sharebymail/lib/ShareByMailProvider.php +++ b/apps/sharebymail/lib/ShareByMailProvider.php @@ -1166,4 +1166,29 @@ class ShareByMailProvider implements IShareProvider { return ['public' => $mail]; } + public function getAllShares(): iterable { + $qb = $this->dbConnection->getQueryBuilder(); + + $qb->select('*') + ->from('share') + ->where( + $qb->expr()->orX( + $qb->expr()->eq('share_type', $qb->createNamedParameter(\OCP\Share\IShare::TYPE_EMAIL)) + ) + ); + + $cursor = $qb->execute(); + while($data = $cursor->fetch()) { + try { + $share = $this->createShareObject($data); + } catch (InvalidShare $e) { + continue; + } catch (ShareNotFound $e) { + continue; + } + + yield $share; + } + $cursor->closeCursor(); + } } diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php index aea50dfcdb..05b3094e6a 100644 --- a/lib/private/Share20/DefaultShareProvider.php +++ b/lib/private/Share20/DefaultShareProvider.php @@ -1382,4 +1382,30 @@ class DefaultShareProvider implements IShareProvider { } } + + public function getAllShares(): iterable { + $qb = $this->dbConn->getQueryBuilder(); + + $qb->select('*') + ->from('share') + ->where( + $qb->expr()->orX( + $qb->expr()->eq('share_type', $qb->createNamedParameter(\OCP\Share\IShare::TYPE_USER)), + $qb->expr()->eq('share_type', $qb->createNamedParameter(\OCP\Share\IShare::TYPE_GROUP)), + $qb->expr()->eq('share_type', $qb->createNamedParameter(\OCP\Share\IShare::TYPE_LINK)) + ) + ); + + $cursor = $qb->execute(); + while($data = $cursor->fetch()) { + try { + $share = $this->createShare($data); + } catch (InvalidShare $e) { + continue; + } + + yield $share; + } + $cursor->closeCursor(); + } } diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 2e8e6f9a3a..df537062e4 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -1671,4 +1671,11 @@ class Manager implements IManager { return true; } + public function getAllShares(): iterable { + $providers = $this->factory->getAllProviders(); + + foreach ($providers as $provider) { + yield from $provider->getAllShares(); + } + } } diff --git a/lib/public/Share/IManager.php b/lib/public/Share/IManager.php index 302be52332..0c47eb730f 100644 --- a/lib/public/Share/IManager.php +++ b/lib/public/Share/IManager.php @@ -385,4 +385,15 @@ interface IManager { */ public function shareProviderExists($shareType); + /** + * @Internal + * + * Get all the shares as iterable to reduce memory overhead + * Note, since this opens up database cursors the iterable should + * be fully itterated. + * + * @return iterable + */ + public function getAllShares(): iterable; + } diff --git a/lib/public/Share/IShareProvider.php b/lib/public/Share/IShareProvider.php index 6731bf8882..da29a7b98a 100644 --- a/lib/public/Share/IShareProvider.php +++ b/lib/public/Share/IShareProvider.php @@ -217,4 +217,12 @@ interface IShareProvider { * @since 12 */ public function getAccessList($nodes, $currentAccess); + + /** + * Get all the shares in this provider returned as iterable to reduce memory + * overhead + * + * @return iterable + */ + public function getAllShares(): iterable; } From ea55f8eeddd49b86e5547bc8389aa8942d33d021 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 30 Oct 2019 14:26:37 +0100 Subject: [PATCH 2/3] fixup! Get all shares as iterable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- lib/public/Share/IManager.php | 1 + lib/public/Share/IShareProvider.php | 1 + 2 files changed, 2 insertions(+) diff --git a/lib/public/Share/IManager.php b/lib/public/Share/IManager.php index 0c47eb730f..8bb7291d6b 100644 --- a/lib/public/Share/IManager.php +++ b/lib/public/Share/IManager.php @@ -393,6 +393,7 @@ interface IManager { * be fully itterated. * * @return iterable + * @since 18.0.0 */ public function getAllShares(): iterable; diff --git a/lib/public/Share/IShareProvider.php b/lib/public/Share/IShareProvider.php index da29a7b98a..c881592826 100644 --- a/lib/public/Share/IShareProvider.php +++ b/lib/public/Share/IShareProvider.php @@ -223,6 +223,7 @@ interface IShareProvider { * overhead * * @return iterable + * @since 18.0.0 */ public function getAllShares(): iterable; } From 2f49806c201c0aaaf400e84a7580d0789c90dc57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 30 Oct 2019 14:27:17 +0100 Subject: [PATCH 3/3] Add unit tests for "getAllShares()" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- .../lib/Share20/DefaultShareProviderTest.php | 165 ++++++++++++++++++ tests/lib/Share20/ManagerTest.php | 51 ++++++ 2 files changed, 216 insertions(+) diff --git a/tests/lib/Share20/DefaultShareProviderTest.php b/tests/lib/Share20/DefaultShareProviderTest.php index f5f710cbbd..8f725022ed 100644 --- a/tests/lib/Share20/DefaultShareProviderTest.php +++ b/tests/lib/Share20/DefaultShareProviderTest.php @@ -2742,4 +2742,169 @@ class DefaultShareProviderTest extends \Test\TestCase { $u5->delete(); $g1->delete(); } + + public function testGetAllShares() { + $qb = $this->dbConn->getQueryBuilder(); + + $qb->insert('share') + ->values([ + 'share_type' => $qb->expr()->literal(\OCP\Share::SHARE_TYPE_USER), + 'share_with' => $qb->expr()->literal('sharedWith1'), + 'uid_owner' => $qb->expr()->literal('shareOwner1'), + 'uid_initiator' => $qb->expr()->literal('sharedBy1'), + 'item_type' => $qb->expr()->literal('file'), + 'file_source' => $qb->expr()->literal(42), + 'file_target' => $qb->expr()->literal('myTarget1'), + 'permissions' => $qb->expr()->literal(13), + ]); + $qb->execute(); + + $id1 = $qb->getLastInsertId(); + + $qb->insert('share') + ->values([ + 'share_type' => $qb->expr()->literal(\OCP\Share::SHARE_TYPE_GROUP), + 'share_with' => $qb->expr()->literal('sharedWith2'), + 'uid_owner' => $qb->expr()->literal('shareOwner2'), + 'uid_initiator' => $qb->expr()->literal('sharedBy2'), + 'item_type' => $qb->expr()->literal('file'), + 'file_source' => $qb->expr()->literal(43), + 'file_target' => $qb->expr()->literal('myTarget2'), + 'permissions' => $qb->expr()->literal(14), + ]); + $qb->execute(); + + $id2 = $qb->getLastInsertId(); + + $qb->insert('share') + ->values([ + 'share_type' => $qb->expr()->literal(\OCP\Share::SHARE_TYPE_LINK), + 'token' => $qb->expr()->literal('token3'), + 'uid_owner' => $qb->expr()->literal('shareOwner3'), + 'uid_initiator' => $qb->expr()->literal('sharedBy3'), + 'item_type' => $qb->expr()->literal('file'), + 'file_source' => $qb->expr()->literal(44), + 'file_target' => $qb->expr()->literal('myTarget3'), + 'permissions' => $qb->expr()->literal(15), + ]); + $qb->execute(); + + $id3 = $qb->getLastInsertId(); + + $qb->insert('share') + ->values([ + 'share_type' => $qb->expr()->literal(\OCP\Share::SHARE_TYPE_EMAIL), + 'share_with' => $qb->expr()->literal('shareOwner4'), + 'token' => $qb->expr()->literal('token4'), + 'uid_owner' => $qb->expr()->literal('shareOwner4'), + 'uid_initiator' => $qb->expr()->literal('sharedBy4'), + 'item_type' => $qb->expr()->literal('file'), + 'file_source' => $qb->expr()->literal(45), + 'file_target' => $qb->expr()->literal('myTarget4'), + 'permissions' => $qb->expr()->literal(16), + ]); + $qb->execute(); + + $id4 = $qb->getLastInsertId(); + + $qb->insert('share') + ->values([ + 'share_type' => $qb->expr()->literal(\OCP\Share::SHARE_TYPE_LINK), + 'token' => $qb->expr()->literal('token5'), + 'uid_owner' => $qb->expr()->literal('shareOwner5'), + 'uid_initiator' => $qb->expr()->literal('sharedBy5'), + 'item_type' => $qb->expr()->literal('file'), + 'file_source' => $qb->expr()->literal(46), + 'file_target' => $qb->expr()->literal('myTarget5'), + 'permissions' => $qb->expr()->literal(17), + ]); + $qb->execute(); + + $id5 = $qb->getLastInsertId(); + + $ownerPath1 = $this->createMock(File::class); + $shareOwner1Folder = $this->createMock(Folder::class); + $shareOwner1Folder->method('getById')->willReturn([$ownerPath1]); + + $ownerPath2 = $this->createMock(File::class); + $shareOwner2Folder = $this->createMock(Folder::class); + $shareOwner2Folder->method('getById')->willReturn([$ownerPath2]); + + $ownerPath3 = $this->createMock(File::class); + $shareOwner3Folder = $this->createMock(Folder::class); + $shareOwner3Folder->method('getById')->willReturn([$ownerPath3]); + + $ownerPath4 = $this->createMock(File::class); + $shareOwner4Folder = $this->createMock(Folder::class); + $shareOwner4Folder->method('getById')->willReturn([$ownerPath4]); + + $ownerPath5 = $this->createMock(File::class); + $shareOwner5Folder = $this->createMock(Folder::class); + $shareOwner5Folder->method('getById')->willReturn([$ownerPath5]); + + $this->rootFolder + ->method('getUserFolder') + ->will($this->returnValueMap( + [ + ['shareOwner1', $shareOwner1Folder], + ['shareOwner2', $shareOwner2Folder], + ['shareOwner3', $shareOwner3Folder], + ['shareOwner4', $shareOwner4Folder], + ['shareOwner5', $shareOwner5Folder], + ] + )); + + $shares = iterator_to_array($this->provider->getAllShares()); + $this->assertEquals(4, count($shares)); + + $share = $shares[0]; + + // We fetch the node so the root folder is eventually called + + $this->assertEquals($id1, $share->getId()); + $this->assertEquals(\OCP\Share::SHARE_TYPE_USER, $share->getShareType()); + $this->assertEquals('sharedWith1', $share->getSharedWith()); + $this->assertEquals('sharedBy1', $share->getSharedBy()); + $this->assertEquals('shareOwner1', $share->getShareOwner()); + $this->assertEquals($ownerPath1, $share->getNode()); + $this->assertEquals(13, $share->getPermissions()); + $this->assertEquals(null, $share->getToken()); + $this->assertEquals('myTarget1', $share->getTarget()); + + $share = $shares[1]; + + $this->assertEquals($id2, $share->getId()); + $this->assertEquals(\OCP\Share::SHARE_TYPE_GROUP, $share->getShareType()); + $this->assertEquals('sharedWith2', $share->getSharedWith()); + $this->assertEquals('sharedBy2', $share->getSharedBy()); + $this->assertEquals('shareOwner2', $share->getShareOwner()); + $this->assertEquals($ownerPath2, $share->getNode()); + $this->assertEquals(14, $share->getPermissions()); + $this->assertEquals(null, $share->getToken()); + $this->assertEquals('myTarget2', $share->getTarget()); + + $share = $shares[2]; + + $this->assertEquals($id3, $share->getId()); + $this->assertEquals(\OCP\Share::SHARE_TYPE_LINK, $share->getShareType()); + $this->assertEquals(null, $share->getSharedWith()); + $this->assertEquals('sharedBy3', $share->getSharedBy()); + $this->assertEquals('shareOwner3', $share->getShareOwner()); + $this->assertEquals($ownerPath3, $share->getNode()); + $this->assertEquals(15, $share->getPermissions()); + $this->assertEquals('token3', $share->getToken()); + $this->assertEquals('myTarget3', $share->getTarget()); + + $share = $shares[3]; + + $this->assertEquals($id5, $share->getId()); + $this->assertEquals(\OCP\Share::SHARE_TYPE_LINK, $share->getShareType()); + $this->assertEquals(null, $share->getSharedWith()); + $this->assertEquals('sharedBy5', $share->getSharedBy()); + $this->assertEquals('shareOwner5', $share->getShareOwner()); + $this->assertEquals($ownerPath5, $share->getNode()); + $this->assertEquals(17, $share->getPermissions()); + $this->assertEquals('token5', $share->getToken()); + $this->assertEquals('myTarget5', $share->getTarget()); + } } diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index 6f46d69d8d..4a7686acd3 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -3685,6 +3685,57 @@ class ManagerTest extends \Test\TestCase { $this->assertSame($expected['users'], $result['users']); } + + public function testGetAllShares() { + $factory = new DummyFactory2($this->createMock(IServerContainer::class)); + + $manager = new Manager( + $this->logger, + $this->config, + $this->secureRandom, + $this->hasher, + $this->mountManager, + $this->groupManager, + $this->l, + $this->l10nFactory, + $factory, + $this->userManager, + $this->rootFolder, + $this->eventDispatcher, + $this->mailer, + $this->urlGenerator, + $this->defaults + ); + + $factory->setProvider($this->defaultProvider); + $extraProvider = $this->createMock(IShareProvider::class); + $factory->setSecondProvider($extraProvider); + + $share1 = $this->createMock(IShare::class); + $share2 = $this->createMock(IShare::class); + $share3 = $this->createMock(IShare::class); + $share4 = $this->createMock(IShare::class); + + $this->defaultProvider->method('getAllShares') + ->willReturnCallback(function() use ($share1, $share2) { + yield $share1; + yield $share2; + }); + $extraProvider->method('getAllShares') + ->willReturnCallback(function() use ($share3, $share4) { + yield $share3; + yield $share4; + }); + + // "yield from", used in "getAllShares()", does not reset the keys, so + // "use_keys" has to be disabled to collect all the values while + // ignoring the keys returned by the generator. + $result = iterator_to_array($manager->getAllShares(), $use_keys = false); + + $expects = [$share1, $share2, $share3, $share4]; + + $this->assertSame($expects, $result); + } } class DummyFactory implements IProviderFactory {