From: Joas Schilling Date: Tue, 11 Apr 2017 10:40:36 +0000 (+0200) Subject: Adjust docs and make !$currentAccess simpler X-Git-Tag: v12.0.0beta1~146^2~9 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=629b7c0fc38d9a2c33805bfcf4359e263e4ae68f;p=nextcloud-server.git Adjust docs and make !$currentAccess simpler Signed-off-by: Joas Schilling --- diff --git a/apps/federatedfilesharing/lib/FederatedShareProvider.php b/apps/federatedfilesharing/lib/FederatedShareProvider.php index 617db4a45ac..34e02391e05 100644 --- a/apps/federatedfilesharing/lib/FederatedShareProvider.php +++ b/apps/federatedfilesharing/lib/FederatedShareProvider.php @@ -976,6 +976,9 @@ class FederatedShareProvider implements IShareProvider { return ($result === 'yes'); } + /** + * @inheritdoc + */ public function getAccessList($nodes, $currentAccess) { $ids = []; foreach ($nodes as $node) { @@ -993,6 +996,13 @@ class FederatedShareProvider implements IShareProvider { )); $cursor = $qb->execute(); + if ($currentAccess === false) { + $remote = $cursor->fetch() !== false; + $cursor->closeCursor(); + + return ['remote' => $remote]; + } + $remote = []; while ($row = $cursor->fetch()) { $remote[$row['share_with']] = [ diff --git a/apps/federatedfilesharing/tests/FederatedShareProviderTest.php b/apps/federatedfilesharing/tests/FederatedShareProviderTest.php index 788ae47fded..e01e02c83ba 100644 --- a/apps/federatedfilesharing/tests/FederatedShareProviderTest.php +++ b/apps/federatedfilesharing/tests/FederatedShareProviderTest.php @@ -805,6 +805,12 @@ class FederatedShareProviderTest extends \Test\TestCase { ->method('sendRemoteShare') ->willReturn(true); + $result = $this->provider->getAccessList([$file1], true); + $this->assertEquals(['remote' => []], $result); + + $result = $this->provider->getAccessList([$file1], false); + $this->assertEquals(['remote' => false], $result); + $share1 = $this->shareManager->newShare(); $share1->setSharedWith('user@server.com') ->setSharedBy($u1->getUID()) @@ -822,7 +828,6 @@ class FederatedShareProviderTest extends \Test\TestCase { $this->provider->create($share2); $result = $this->provider->getAccessList([$file1], true); - $this->assertEquals(['remote' => [ 'user@server.com' => [ 'token' => 'token1', @@ -833,6 +838,10 @@ class FederatedShareProviderTest extends \Test\TestCase { 'node_id' => $file1->getId(), ], ]], $result); + + $result = $this->provider->getAccessList([$file1], false); + $this->assertEquals(['remote' => true], $result); + $u1->delete(); } } diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php index 14ed12749b7..0f4a8dd87bc 100644 --- a/apps/sharebymail/lib/ShareByMailProvider.php +++ b/apps/sharebymail/lib/ShareByMailProvider.php @@ -834,6 +834,9 @@ class ShareByMailProvider implements IShareProvider { return $shares; } + /** + * @inheritdoc + */ public function getAccessList($nodes, $currentAccess) { $ids = []; foreach ($nodes as $node) { diff --git a/apps/sharebymail/tests/ShareByMailProviderTest.php b/apps/sharebymail/tests/ShareByMailProviderTest.php index 1b215016ee4..cd0600d3ae9 100644 --- a/apps/sharebymail/tests/ShareByMailProviderTest.php +++ b/apps/sharebymail/tests/ShareByMailProviderTest.php @@ -675,6 +675,13 @@ class ShareByMailProviderTest extends TestCase { $folder = $rootFolder->getUserFolder($u1->getUID())->newFolder('foo'); + $accessList = $provider->getAccessList([$folder], true); + $this->assertArrayHasKey('mail', $accessList); + $this->assertFalse($accessList['mail']); + $accessList = $provider->getAccessList([$folder], false); + $this->assertArrayHasKey('mail', $accessList); + $this->assertFalse($accessList['mail']); + $share1 = $this->shareManager->newShare(); $share1->setSharedWith('user@server.com') ->setSharedBy($u1->getUID()) diff --git a/lib/private/Encryption/File.php b/lib/private/Encryption/File.php index f6fd3382cb5..2bc0e014f0c 100644 --- a/lib/private/Encryption/File.php +++ b/lib/private/Encryption/File.php @@ -95,14 +95,14 @@ class File implements \OCP\Encryption\IFile { $this->cache[$parent] = $resultForParents; } $userIds = array_merge($userIds, $resultForParents['users']); - $public = $resultForParents['public'] || !empty($resultForParents['remote']); + $public = $resultForParents['public'] || $resultForParents['remote']; // Find out who, if anyone, is sharing the file if ($file !== null) { $resultForFile = $this->shareManager->getAccessList($file, false); $userIds = array_merge($userIds, $resultForFile['users']); - $public = $resultForFile['public'] || !empty($resultForFile['remote']) || $public; + $public = $resultForFile['public'] || $resultForFile['remote'] || $public; } // check if it is a group mount diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php index b28dc56f852..ed3651df9b2 100644 --- a/lib/private/Share20/DefaultShareProvider.php +++ b/lib/private/Share20/DefaultShareProvider.php @@ -1136,9 +1136,11 @@ class DefaultShareProvider implements IShareProvider { } $cursor->closeCursor(); - $users = array_map([$this, 'filterSharesOfUser'], $users); if ($currentAccess === true) { + $users = array_map([$this, 'filterSharesOfUser'], $users); $users = array_filter($users); + } else { + $users = array_keys($users); } return ['users' => $users, 'public' => $link]; diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 91fcb6af8fb..8b670da544b 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -1181,18 +1181,33 @@ class Manager implements IManager { * * Consider: * -root - * |-folder1 - * |-folder2 - * |-fileA + * |-folder1 (23) + * |-folder2 (32) + * |-fileA (42) * - * fileA is shared with user1 + * fileA is shared with user1 and user1@server1 * folder2 is shared with group2 (user4 is a member of group2) - * folder1 is shared with user2 + * folder1 is shared with user2 (renamed to "folder (1)") and user2@server2 * - * Then the access list will to '/folder1/folder2/fileA' is: + * Then the access list to '/folder1/folder2/fileA' with $currentAccess is: * [ - * users => ['user1' => ['node_id' => 42, 'node_path' => '/path'], 'user2' => [...]], - * remote => ['user1' => ['node_id' => 42, 'node_path' => '/path'], 'user2' => [...]], + * users => [ + * 'user1' => ['node_id' => 42, 'node_path' => '/fileA'], + * 'user4' => ['node_id' => 32, 'node_path' => '/folder2'], + * 'user2' => ['node_id' => 23, 'node_path' => '/folder (1)'], + * ], + * remote => [ + * 'user1@server1' => ['node_id' => 42, 'token' => 'SeCr3t'], + * 'user2@server2' => ['node_id' => 23, 'token' => 'FooBaR'], + * ], + * public => bool + * mail => bool + * ] + * + * The access list to '/folder1/folder2/fileA' **without** $currentAccess is: + * [ + * users => ['user1', 'user2', 'user4'], + * remote => bool, * public => bool * mail => bool * ] @@ -1207,7 +1222,11 @@ class Manager implements IManager { public function getAccessList(\OCP\Files\Node $path, $recursive = true, $currentAccess = false) { $owner = $path->getOwner()->getUID(); - $al = ['users' => [], 'remote' => [], 'public' => false]; + if ($currentAccess) { + $al = ['users' => [], 'remote' => [], 'public' => false]; + } else { + $al = ['users' => [], 'remote' => false, 'public' => false]; + } if (!$this->userManager->userExists($owner)) { return $al; } diff --git a/lib/public/Share/IManager.php b/lib/public/Share/IManager.php index 5f02e1d9804..31f04803066 100644 --- a/lib/public/Share/IManager.php +++ b/lib/public/Share/IManager.php @@ -197,18 +197,33 @@ interface IManager { * * Consider: * -root - * |-folder1 - * |-folder2 - * |-fileA + * |-folder1 (23) + * |-folder2 (32) + * |-fileA (42) * - * fileA is shared with user1 + * fileA is shared with user1 and user1@server1 * folder2 is shared with group2 (user4 is a member of group2) - * folder1 is shared with user2 + * folder1 is shared with user2 (renamed to "folder (1)") and user2@server2 * - * Then the access list will to '/folder1/folder2/fileA' is: + * Then the access list to '/folder1/folder2/fileA' with $currentAccess is: * [ - * users => ['user1' => ['node_id' => 42, 'node_path' => '/path'], 'user2' => [...]], - * remote => ['user1' => ['node_id' => 42, 'token' => 'ShareToken'], 'user2' => [...]], + * users => [ + * 'user1' => ['node_id' => 42, 'node_path' => '/fileA'], + * 'user4' => ['node_id' => 32, 'node_path' => '/folder2'], + * 'user2' => ['node_id' => 23, 'node_path' => '/folder (1)'], + * ], + * remote => [ + * 'user1@server1' => ['node_id' => 42, 'token' => 'SeCr3t'], + * 'user2@server2' => ['node_id' => 23, 'token' => 'FooBaR'], + * ], + * public => bool + * mail => bool + * ] + * + * The access list to '/folder1/folder2/fileA' **without** $currentAccess is: + * [ + * users => ['user1', 'user2', 'user4'], + * remote => bool, * public => bool * mail => bool * ] diff --git a/lib/public/Share/IShareProvider.php b/lib/public/Share/IShareProvider.php index bac1a5d87f0..31808206cf6 100644 --- a/lib/public/Share/IShareProvider.php +++ b/lib/public/Share/IShareProvider.php @@ -193,14 +193,8 @@ interface IShareProvider { /** * Get the access list to the array of provided nodes. - * Return will look like: - * - * [ - * users => ['user1' => ['node_id' => 42, 'node_path' => '/path'], 'user2' => [...]], - * remote => ['user1' => ['node_id' => 42, 'token' => 'ShareToken'], 'user2' => [...]], - * mail => bool - * public => bool - * ] + * + * @see IManager::getAccessList() for sample docs * * @param Node[] $nodes The list of nodes to get access for * @param bool $currentAccess If current access is required (like for removed shares that might get revived later) diff --git a/tests/lib/Share20/DefaultShareProviderTest.php b/tests/lib/Share20/DefaultShareProviderTest.php index 6c351f1a6d9..0fa8aa3d0c6 100644 --- a/tests/lib/Share20/DefaultShareProviderTest.php +++ b/tests/lib/Share20/DefaultShareProviderTest.php @@ -2462,6 +2462,10 @@ class DefaultShareProviderTest extends \Test\TestCase { $folder2 = $folder1->newFolder('baz'); $file1 = $folder2->newFile('bar'); + $result = $provider->getAccessList([$folder1, $folder2, $file1], false); + $this->assertCount(0, $result['users']); + $this->assertFalse($result['public']); + $shareManager = \OC::$server->getShareManager(); $share1 = $shareManager->newShare(); $share1->setNode($folder1) @@ -2503,10 +2507,10 @@ class DefaultShareProviderTest extends \Test\TestCase { $result = $provider->getAccessList([$folder1, $folder2, $file1], false); $this->assertCount(4, $result['users']); - $this->assertArrayHasKey('testShare2', $result['users']); - $this->assertArrayHasKey('testShare3', $result['users']); - $this->assertArrayHasKey('testShare4', $result['users']); - $this->assertArrayHasKey('testShare5', $result['users']); + $this->assertContains('testShare2', $result['users']); + $this->assertContains('testShare3', $result['users']); + $this->assertContains('testShare4', $result['users']); + $this->assertContains('testShare5', $result['users']); $this->assertTrue($result['public']); $provider->delete($share1); @@ -2549,6 +2553,10 @@ class DefaultShareProviderTest extends \Test\TestCase { $folder2 = $folder1->newFolder('baz'); $file1 = $folder2->newFile('bar'); + $result = $provider->getAccessList([$folder1, $folder2, $file1], false); + $this->assertCount(0, $result['users']); + $this->assertFalse($result['public']); + $shareManager = \OC::$server->getShareManager(); $share1 = $shareManager->newShare(); $share1->setNode($folder1)