update unit tests

Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>

5 files changed, 74 insertions, 37 deletions

diff --git a/apps/files_sharing/lib/Controller/ShareesAPIController.php b/apps/files_sharing/lib/Controller/ShareesAPIController.php
index c60e873b826..ee95661d4c6 100644
--- a/apps/files_sharing/lib/Controller/ShareesAPIController.php
+++ b/apps/files_sharing/lib/Controller/ShareesAPIController.php
@@ -223,7 +223,13 @@ class ShareesAPIController extends OCSController {
 	}
 
 	protected function isRemoteGroupSharingAllowed(string $itemType): bool {
-		return true;
+		try {
+			// FIXME: static foo makes unit testing unnecessarily difficult
+			$backend = \OC\Share\Share::getBackend($itemType);
+			return $backend->isShareTypeAllowed(Share::SHARE_TYPE_REMOTE_GROUP);
+		} catch (\Exception $e) {
+			return false;
+		}
 	}
 
 
diff --git a/apps/files_sharing/lib/External/Manager.php b/apps/files_sharing/lib/External/Manager.php
index 9f8acbce4cb..a3b037e4982 100644
--- a/apps/files_sharing/lib/External/Manager.php
+++ b/apps/files_sharing/lib/External/Manager.php
@@ -276,15 +276,16 @@ class Manager {
 			$mountPoint = Files::buildNotExistingFileName($shareFolder, $share['name']);
 			$mountPoint = Filesystem::normalizePath($mountPoint);
 			$hash = md5($mountPoint);
+			$userShareAccepted = false;
 
-			if($share['share_type'] === Share::SHARE_TYPE_USER) {
+			if((int)$share['share_type'] === Share::SHARE_TYPE_USER) {
 				$acceptShare = $this->connection->prepare('
 				UPDATE `*PREFIX*share_external`
 				SET `accepted` = ?,
 					`mountpoint` = ?,
 					`mountpoint_hash` = ?
 				WHERE `id` = ? AND `user` = ?');
-				$updated = $acceptShare->execute(array(1, $mountPoint, $hash, $id, $this->uid));
+				$userShareAccepted = $acceptShare->execute(array(1, $mountPoint, $hash, $id, $this->uid));
 			} else {
 				$result = $this->writeShareToDb(
 					$share['remote'],
@@ -297,9 +298,8 @@ class Manager {
 					$share['remote_id'],
 					$id,
 					$share['share_type']);
-				// TODO group share, add additional row for the user who accepted it
 			}
-			if ($updated === true) {
+			if ($userShareAccepted === true) {
 				$this->sendFeedbackToRemote($share['remote'], $share['share_token'], $share['remote_id'], 'accept');
 				\OC_Hook::emit(Share::class, 'federated_share_added', ['server' => $share['remote']]);
 				$result = true;
diff --git a/apps/files_sharing/lib/ShareBackend/File.php b/apps/files_sharing/lib/ShareBackend/File.php
index bb28c1a33ac..bd32741e67e 100644
--- a/apps/files_sharing/lib/ShareBackend/File.php
+++ b/apps/files_sharing/lib/ShareBackend/File.php
@@ -195,6 +195,10 @@ class File implements \OCP\Share_Backend_File_Dependent {
 			return $this->federatedShareProvider->isOutgoingServer2serverShareEnabled();
 		}
 
+		if ($shareType === \OCP\Share::SHARE_TYPE_REMOTE_GROUP) {
+			return $this->federatedShareProvider->isOutgoingServer2serverGroupShareEnabled();
+		}
+
 		return true;
 	}
 
diff --git a/apps/files_sharing/tests/Controller/ShareesAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareesAPIControllerTest.php
index 98ae953a318..d8c706da03c 100644
--- a/apps/files_sharing/tests/Controller/ShareesAPIControllerTest.php
+++ b/apps/files_sharing/tests/Controller/ShareesAPIControllerTest.php
@@ -85,120 +85,125 @@ class ShareesAPIControllerTest extends TestCase {
 
 	public function dataSearch() {
 		$noRemote = [Share::SHARE_TYPE_USER, Share::SHARE_TYPE_GROUP, Share::SHARE_TYPE_EMAIL];
-		$allTypes = [Share::SHARE_TYPE_USER, Share::SHARE_TYPE_GROUP, Share::SHARE_TYPE_REMOTE, Share::SHARE_TYPE_EMAIL];
+		$allTypes = [Share::SHARE_TYPE_USER, Share::SHARE_TYPE_GROUP, Share::SHARE_TYPE_REMOTE, Share::SHARE_TYPE_REMOTE_GROUP, Share::SHARE_TYPE_EMAIL];
 
 		return [
-			[[], '', 'yes', true, true, $noRemote, false, true, true],
+			[[], '', 'yes', true, true, true, $noRemote, false, true, true],
 
 			// Test itemType
 			[[
 				'search' => '',
-			], '', 'yes', true, true, $noRemote, false, true, true],
+			], '', 'yes', true, true, true, $noRemote, false, true, true],
 			[[
 				'search' => 'foobar',
-			], '', 'yes', true, true, $noRemote, false, true, true],
+			], '', 'yes', true, true, true, $noRemote, false, true, true],
 			[[
 				'search' => 0,
-			], '', 'yes', true, true, $noRemote, false, true, true],
+			], '', 'yes', true, true, true, $noRemote, false, true, true],
 
 			// Test itemType
 			[[
 				'itemType' => '',
-			], '', 'yes', true, true, $noRemote, false, true, true],
+			], '', 'yes', true, true, true, $noRemote, false, true, true],
 			[[
 				'itemType' => 'folder',
-			], '', 'yes', true, true, $allTypes, false, true, true],
+			], '', 'yes', true, true, true, $allTypes, false, true, true],
 			[[
 				'itemType' => 0,
-			], '', 'yes', true, true, $noRemote, false, true, true],
-
+			], '', 'yes', true, true , true, $noRemote, false, true, true],
 			// Test shareType
 			[[
 				'itemType' => 'call',
-			], '', 'yes', true, true, $noRemote, false, true, true],
+			], '', 'yes', true, true, true, $noRemote, false, true, true],
 			[[
 				'itemType' => 'folder',
-			], '', 'yes', true, true, $allTypes, false, true, true],
+			], '', 'yes', true, true, true, $allTypes, false, true, true],
 			[[
 				'itemType' => 'folder',
 				'shareType' => 0,
-			], '', 'yes', true, false, [0], false, true, true],
+			], '', 'yes', true, true, false, [0], false, true, true],
 			[[
 				'itemType' => 'folder',
 				'shareType' => '0',
-			], '', 'yes', true, false, [0], false, true, true],
+			], '', 'yes', true, true, false, [0], false, true, true],
 			[[
 				'itemType' => 'folder',
 				'shareType' => 1,
-			], '', 'yes', true, false, [1], false, true, true],
+			], '', 'yes', true, true, false, [1], false, true, true],
 			[[
 				'itemType' => 'folder',
 				'shareType' => 12,
-			], '', 'yes', true, false, [], false, true, true],
+			], '', 'yes', true, true, false, [], false, true, true],
 			[[
 				'itemType' => 'folder',
 				'shareType' => 'foobar',
-			], '', 'yes', true, true, $allTypes, false, true, true],
+			], '', 'yes', true, true, true, $allTypes, false, true, true],
+
 			[[
 				'itemType' => 'folder',
 				'shareType' => [0, 1, 2],
-			], '', 'yes', false, false, [0, 1], false, true, true],
+			], '', 'yes', false, false, false, [0, 1], false, true, true],
 			[[
 				'itemType' => 'folder',
 				'shareType' => [0, 1],
-			], '', 'yes', false, false, [0, 1], false, true, true],
+			], '', 'yes', false, false, false, [0, 1], false, true, true],
+			[[
+				'itemType' => 'folder',
+				'shareType' => $allTypes,
+			], '', 'yes', true, true, true, $allTypes, false, true, true],
 			[[
 				'itemType' => 'folder',
 				'shareType' => $allTypes,
-			], '', 'yes', true, true, $allTypes, false, true, true],
+			], '', 'yes', false, false, false, [0, 1], false, true, true],
 			[[
 				'itemType' => 'folder',
 				'shareType' => $allTypes,
-			], '', 'yes', false, false, [0, 1], false, true, true],
+			], '', 'yes', true, false, false, [0, 6], false, true, false],
 			[[
 				'itemType' => 'folder',
 				'shareType' => $allTypes,
-			], '', 'yes', true, false, [0, 6], false, true, false],
+			], '', 'yes', false, false, true, [0, 4], false, true, false],
 			[[
 				'itemType' => 'folder',
 				'shareType' => $allTypes,
-			], '', 'yes', false, true, [0, 4], false, true, false],
+			], '', 'yes', true, true, false, [0, 6, 9], false, true, false],
 
 			// Test pagination
 			[[
 				'itemType' => 'folder',
 				'page' => 1,
-			], '', 'yes', true, true, $allTypes, false, true, true],
+			], '', 'yes', true, true, true, $allTypes, false, true, true],
 			[[
 				'itemType' => 'folder',
 				'page' => 10,
-			], '', 'yes', true, true, $allTypes, false, true, true],
+			], '', 'yes', true, true, true, $allTypes, false, true, true],
 
 			// Test perPage
 			[[
 				'itemType' => 'folder',
 				'perPage' => 1,
-			], '', 'yes', true, true, $allTypes, false, true, true],
+			], '', 'yes', true, true, true, $allTypes, false, true, true],
 			[[
 				'itemType' => 'folder',
 				'perPage' => 10,
-			], '', 'yes', true, true, $allTypes, false, true, true],
+			], '', 'yes', true, true, true, $allTypes, false, true, true],
 
 			// Test $shareWithGroupOnly setting
 			[[
 				'itemType' => 'folder',
-			], 'no', 'yes',  true, true, $allTypes, false, true, true],
+			], 'no', 'yes',  true, true, true, $allTypes, false, true, true],
 			[[
 				'itemType' => 'folder',
-			], 'yes', 'yes', true, true, $allTypes, true, true, true],
+			], 'yes', 'yes', true, true, true, $allTypes, true, true, true],
 
 			// Test $shareeEnumeration setting
 			[[
 				'itemType' => 'folder',
-			], 'no', 'yes',  true, true, $allTypes, false, true, true],
+			], 'no', 'yes',  true, true, true, $allTypes, false, true, true],
 			[[
 				'itemType' => 'folder',
-			], 'no', 'no', true, true, $allTypes, false, false, true],
+			], 'no', 'no', true, true, true, $allTypes, false, false, true],
+
 		];
 	}
 
@@ -209,13 +214,15 @@ class ShareesAPIControllerTest extends TestCase {
 	 * @param string $apiSetting
 	 * @param string $enumSetting
 	 * @param bool $remoteSharingEnabled
+	 * @param bool $isRemoteGroupSharingEnabled
 	 * @param bool $emailSharingEnabled
 	 * @param array $shareTypes
 	 * @param bool $shareWithGroupOnly
 	 * @param bool $shareeEnumeration
 	 * @param bool $allowGroupSharing
+	 * @throws OCSBadRequestException
 	 */
-	public function testSearch($getData, $apiSetting, $enumSetting, $remoteSharingEnabled, $emailSharingEnabled, $shareTypes, $shareWithGroupOnly, $shareeEnumeration, $allowGroupSharing) {
+	public function testSearch($getData, $apiSetting, $enumSetting, $remoteSharingEnabled, $isRemoteGroupSharingEnabled, $emailSharingEnabled, $shareTypes, $shareWithGroupOnly, $shareeEnumeration, $allowGroupSharing) {
 		$search = isset($getData['search']) ? $getData['search'] : '';
 		$itemType = isset($getData['itemType']) ? $getData['itemType'] : 'irrelevant';
 		$page = isset($getData['page']) ? $getData['page'] : 1;
@@ -251,7 +258,7 @@ class ShareesAPIControllerTest extends TestCase {
 				$this->shareManager,
 				$this->collaboratorSearch
 			])
-			->setMethods(['isRemoteSharingAllowed', 'shareProviderExists'])
+			->setMethods(['isRemoteSharingAllowed', 'shareProviderExists', 'isRemoteGroupSharingAllowed'])
 			->getMock();
 
 		$this->collaboratorSearch->expects($this->once())
@@ -264,6 +271,13 @@ class ShareesAPIControllerTest extends TestCase {
 			->with($itemType)
 			->willReturn($remoteSharingEnabled);
 
+
+		$sharees->expects($this->any())
+			->method('isRemoteGroupSharingAllowed')
+			->with($itemType)
+			->willReturn($isRemoteGroupSharingEnabled);
+
+
 		$this->shareManager->expects($this->any())
 			->method('shareProviderExists')
 			->with(\OCP\Share::SHARE_TYPE_EMAIL)
diff --git a/apps/files_sharing/tests/External/ManagerTest.php b/apps/files_sharing/tests/External/ManagerTest.php
index 2cc88160a8c..753836dd58e 100644
--- a/apps/files_sharing/tests/External/ManagerTest.php
+++ b/apps/files_sharing/tests/External/ManagerTest.php
@@ -35,6 +35,8 @@ use OCA\Files_Sharing\Tests\TestCase;
 use OCP\Federation\ICloudFederationFactory;
 use OCP\Federation\ICloudFederationProviderManager;
 use OCP\Http\Client\IClientService;
+use OCP\IGroupManager;
+use OCP\IUserManager;
 use Test\Traits\UserTrait;
 
 /**
@@ -62,6 +64,12 @@ class ManagerTest extends TestCase {
 	/** @var ICloudFederationFactory|\PHPUnit_Framework_MockObject_MockObject */
 	private $cloudFederationFactory;
 
+	/** @var \PHPUnit_Framework_MockObject_MockObject|IGroupManager */
+	private $groupManager;
+
+	/** @var \PHPUnit_Framework_MockObject_MockObject|IUserManager */
+	private $userManager;
+
 	private $uid;
 
 	/**
@@ -81,6 +89,8 @@ class ManagerTest extends TestCase {
 			->disableOriginalConstructor()->getMock();
 		$this->cloudFederationProviderManager = $this->createMock(ICloudFederationProviderManager::class);
 		$this->cloudFederationFactory = $this->createMock(ICloudFederationFactory::class);
+		$this->groupManager = $this->createMock(IGroupManager::class);
+		$this->userManager = $this->createMock(IUserManager::class);
 
 		$this->manager = $this->getMockBuilder(Manager::class)
 			->setConstructorArgs(
@@ -93,6 +103,8 @@ class ManagerTest extends TestCase {
 					\OC::$server->query(\OCP\OCS\IDiscoveryService::class),
 					$this->cloudFederationProviderManager,
 					$this->cloudFederationFactory,
+					$this->groupManager,
+					$this->userManager,
 					$this->uid
 				]
 			)->setMethods(['tryOCMEndPoint'])->getMock();
@@ -117,6 +129,7 @@ class ManagerTest extends TestCase {
 			'password' => '',
 			'name' => '/SharedFolder',
 			'owner' => 'foobar',
+			'shareType' => \OCP\Share::SHARE_TYPE_USER,
 			'accepted' => false,
 			'user' => $this->uid,
 		];