Adjust docs and make !$currentAccess simpler

Signed-off-by: Joas Schilling <coding@schilljs.com>

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']] = [

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();
 	}
 }

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) {

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())

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

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];

lib/private/Share20/Manager.php

@@ -1181,18 +1181,33 @@ class Manager implements IManager {
 	 *
 	 * Consider:
 	 * -root
-	 * |-folder1
+	 * |-folder1 (23)
-	 *  |-folder2
+	 *  |-folder2 (32)
-	 *   |-fileA
+	 *   |-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' => [...]],
+	 *  users  => [
-	 *  remote => ['user1' => ['node_id' => 42, 'node_path' => '/path'], 'user2' => [...]],
+	 *      '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();
 
+		if ($currentAccess) {
 			$al = ['users' => [], 'remote' => [], 'public' => false];
+		} else {
+			$al = ['users' => [], 'remote' => false, 'public' => false];
+		}
 		if (!$this->userManager->userExists($owner)) {
 			return $al;
 		}

lib/public/Share/IManager.php

@@ -197,18 +197,33 @@ interface IManager {
 	 *
 	 * Consider:
 	 * -root
-	 * |-folder1
+	 * |-folder1 (23)
-	 *  |-folder2
+	 *  |-folder2 (32)
-	 *   |-fileA
+	 *   |-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' => [...]],
+	 *  users  => [
-	 *  remote => ['user1' => ['node_id' => 42, 'token' => 'ShareToken'], 'user2' => [...]],
+	 *      '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
 	 * ]

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:
 	 *
-	 * [
+	 * @see IManager::getAccessList() for sample docs
-	 *  users  => ['user1' => ['node_id' => 42, 'node_path' => '/path'], 'user2' => [...]],
-	 *  remote => ['user1' => ['node_id' => 42, 'token' => 'ShareToken'], 'user2' => [...]],
-	 *  mail   => bool
-	 *  public => bool
-	 * ]
 	 *
 	 * @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)

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->assertContains('testShare2', $result['users']);
-		$this->assertArrayHasKey('testShare3', $result['users']);
+		$this->assertContains('testShare3', $result['users']);
-		$this->assertArrayHasKey('testShare4', $result['users']);
+		$this->assertContains('testShare4', $result['users']);
-		$this->assertArrayHasKey('testShare5', $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)