update unit tests

Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
2018-07-04 10:39:13 +02:00 · 2018-07-04 10:39:13 +02:00 · 3942d731d2
parent 5b06a7d773
commit 3942d731d2
7 changed files with 120 additions and 40 deletions
--- a/apps/federatedfilesharing/tests/FederatedShareProviderTest.php
+++ b/apps/federatedfilesharing/tests/FederatedShareProviderTest.php
@ -33,6 +33,7 @@ use OCA\FederatedFileSharing\AddressHandler;
 use OCA\FederatedFileSharing\FederatedShareProvider;
 use OCA\FederatedFileSharing\Notifications;
 use OCA\FederatedFileSharing\TokenHandler;
+use OCP\Federation\ICloudFederationProviderManager;
 use OCP\Federation\ICloudIdManager;
 use OCP\Files\File;
 use OCP\Files\IRootFolder;
@ -80,6 +81,8 @@ class FederatedShareProviderTest extends \Test\TestCase {
 	/** @var  ICloudIdManager */
 	private $cloudIdManager;

+	/** @var \PHPUnit_Framework_MockObject_MockObject|ICloudFederationProviderManager */
+	private $cloudFederationProviderManager;

 	public function setUp() {
 		parent::setUp();
@ -107,6 +110,8 @@ class FederatedShareProviderTest extends \Test\TestCase {

 		$this->userManager->expects($this->any())->method('userExists')->willReturn(true);

+		$this->cloudFederationProviderManager = $this->createMock(ICloudFederationProviderManager::class);
+
 		$this->provider = new FederatedShareProvider(
 			$this->connection,
 			$this->addressHandler,
@ -118,7 +123,8 @@ class FederatedShareProviderTest extends \Test\TestCase {
 			$this->config,
 			$this->userManager,
 			$this->cloudIdManager,
-			$this->gsConfig
+			$this->gsConfig,
+			$this->cloudFederationProviderManager
 		);

 		$this->shareManager = \OC::$server->getShareManager();
@ -141,6 +147,7 @@ class FederatedShareProviderTest extends \Test\TestCase {
 			->setSharedBy('sharedBy')
 			->setShareOwner('shareOwner')
 			->setPermissions(19)
+			->setShareType(\OCP\Share::SHARE_TYPE_REMOTE)
 			->setNode($node);

 		$this->tokenHandler->method('generateToken')->willReturn('token');
@ -212,6 +219,7 @@ class FederatedShareProviderTest extends \Test\TestCase {
 			->setSharedBy('sharedBy')
 			->setShareOwner('shareOwner')
 			->setPermissions(19)
+			->setShareType(\OCP\Share::SHARE_TYPE_REMOTE)
 			->setNode($node);

 		$this->tokenHandler->method('generateToken')->willReturn('token');
@ -268,6 +276,7 @@ class FederatedShareProviderTest extends \Test\TestCase {
 			->setSharedBy('sharedBy')
 			->setShareOwner('shareOwner')
 			->setPermissions(19)
+			->setShareType(\OCP\Share::SHARE_TYPE_REMOTE)
 			->setNode($node);

 		$this->tokenHandler->method('generateToken')->willReturn('token');
@ -367,6 +376,7 @@ class FederatedShareProviderTest extends \Test\TestCase {
 			->setSharedBy('sharedBy')
 			->setShareOwner('shareOwner')
 			->setPermissions(19)
+			->setShareType(\OCP\Share::SHARE_TYPE_REMOTE)
 			->setNode($node);

 		$this->tokenHandler->method('generateToken')->willReturn('token');
@ -417,7 +427,8 @@ class FederatedShareProviderTest extends \Test\TestCase {
 					$this->config,
 					$this->userManager,
 					$this->cloudIdManager,
-					$this->gsConfig
+					$this->gsConfig,
+					$this->cloudFederationProviderManager
 				]
 			)->setMethods(['sendPermissionUpdate'])->getMock();

@ -435,6 +446,7 @@ class FederatedShareProviderTest extends \Test\TestCase {
 			->setSharedBy($sharedBy)
 			->setShareOwner($owner)
 			->setPermissions(19)
+			->setShareType(\OCP\Share::SHARE_TYPE_REMOTE)
 			->setNode($node);

 		$this->tokenHandler->method('generateToken')->willReturn('token');
@ -505,6 +517,7 @@ class FederatedShareProviderTest extends \Test\TestCase {
 			->setSharedBy('sharedBy')
 			->setShareOwner('shareOwner')
 			->setPermissions(19)
+			->setShareType(\OCP\Share::SHARE_TYPE_REMOTE)
 			->setNode($node);
 		$this->provider->create($share);

@ -513,6 +526,7 @@ class FederatedShareProviderTest extends \Test\TestCase {
 			->setSharedBy('sharedBy2')
 			->setShareOwner('shareOwner')
 			->setPermissions(19)
+			->setShareType(\OCP\Share::SHARE_TYPE_REMOTE)
 			->setNode($node);
 		$this->provider->create($share2);

@ -543,6 +557,7 @@ class FederatedShareProviderTest extends \Test\TestCase {
 			->setSharedBy('sharedBy')
 			->setShareOwner('shareOwner')
 			->setPermissions(19)
+			->setShareType(\OCP\Share::SHARE_TYPE_REMOTE)
 			->setNode($node);
 		$this->provider->create($share);

@ -555,6 +570,7 @@ class FederatedShareProviderTest extends \Test\TestCase {
 			->setSharedBy('sharedBy')
 			->setShareOwner('shareOwner')
 			->setPermissions(19)
+			->setShareType(\OCP\Share::SHARE_TYPE_REMOTE)
 			->setNode($node2);
 		$this->provider->create($share2);

@ -584,6 +600,7 @@ class FederatedShareProviderTest extends \Test\TestCase {
 			->setSharedBy('shareOwner')
 			->setShareOwner('shareOwner')
 			->setPermissions(19)
+			->setShareType(\OCP\Share::SHARE_TYPE_REMOTE)
 			->setNode($node);
 		$this->provider->create($share);

@ -592,6 +609,7 @@ class FederatedShareProviderTest extends \Test\TestCase {
 			->setSharedBy('sharedBy')
 			->setShareOwner('shareOwner')
 			->setPermissions(19)
+			->setShareType(\OCP\Share::SHARE_TYPE_REMOTE)
 			->setNode($node);
 		$this->provider->create($share2);

@ -628,6 +646,7 @@ class FederatedShareProviderTest extends \Test\TestCase {
 			->setSharedBy('sharedBy')
 			->setShareOwner('shareOwner')
 			->setPermissions(19)
+			->setShareType(\OCP\Share::SHARE_TYPE_REMOTE)
 			->setNode($node);
 		$this->provider->create($share);

@ -636,6 +655,7 @@ class FederatedShareProviderTest extends \Test\TestCase {
 			->setSharedBy('sharedBy')
 			->setShareOwner('shareOwner')
 			->setPermissions(19)
+			->setShareType(\OCP\Share::SHARE_TYPE_REMOTE)
 			->setNode($node);
 		$this->provider->create($share2);

@ -826,6 +846,7 @@ class FederatedShareProviderTest extends \Test\TestCase {
 			->setSharedBy($u1->getUID())
 			->setShareOwner($u1->getUID())
 			->setPermissions(\OCP\Constants::PERMISSION_READ)
+			->setShareType(\OCP\Share::SHARE_TYPE_REMOTE)
 			->setNode($file1);
 		$this->provider->create($share1);

@ -834,6 +855,7 @@ class FederatedShareProviderTest extends \Test\TestCase {
 			->setSharedBy($u2->getUID())
 			->setShareOwner($u1->getUID())
 			->setPermissions(\OCP\Constants::PERMISSION_READ)
+			->setShareType(\OCP\Share::SHARE_TYPE_REMOTE)
 			->setNode($file2);
 		$this->provider->create($share2);

@ -880,6 +902,7 @@ class FederatedShareProviderTest extends \Test\TestCase {
 			->setSharedBy($u1->getUID())
 			->setShareOwner($u1->getUID())
 			->setPermissions(\OCP\Constants::PERMISSION_READ)
+			->setShareType(\OCP\Share::SHARE_TYPE_REMOTE)
 			->setNode($file1);
 		$this->provider->create($share1);

@ -888,6 +911,7 @@ class FederatedShareProviderTest extends \Test\TestCase {
 			->setSharedBy($u1->getUID())
 			->setShareOwner($u1->getUID())
 			->setPermissions(\OCP\Constants::PERMISSION_READ)
+			->setShareType(\OCP\Share::SHARE_TYPE_REMOTE)
 			->setNode($file1);
 		$this->provider->create($share2);

--- a/apps/federatedfilesharing/tests/Settings/AdminTest.php
+++ b/apps/federatedfilesharing/tests/Settings/AdminTest.php
@ -72,6 +72,10 @@ class AdminTest extends TestCase {
 			->expects($this->once())
 			->method('isIncomingServer2serverShareEnabled')
 			->willReturn($state);
+		$this->federatedShareProvider
+			->expects($this->once())
+			->method('isIncomingServer2serverShareEnabled')
+			->willReturn($state);
 		$this->federatedShareProvider
 			->expects($this->once())
 			->method('isLookupServerQueriesEnabled')
@ -80,6 +84,18 @@ class AdminTest extends TestCase {
 			->expects($this->once())
 			->method('isLookupServerUploadEnabled')
 			->willReturn($state);
+		$this->federatedShareProvider
+			->expects($this->once())
+			->method('isFederatedGroupSharingSupported')
+			->willReturn($state);
+		$this->federatedShareProvider
+			->expects($this->once())
+			->method('isOutgoingServer2serverGroupShareEnabled')
+			->willReturn($state);
+		$this->federatedShareProvider
+			->expects($this->once())
+			->method('isIncomingServer2serverGroupShareEnabled')
+			->willReturn($state);
 		$this->gsConfig->expects($this->once())->method('onlyInternalFederation')
 			->willReturn($state);

@ -88,7 +104,10 @@ class AdminTest extends TestCase {
 			'outgoingServer2serverShareEnabled' => $state,
 			'incomingServer2serverShareEnabled' => $state,
 			'lookupServerEnabled' => $state,
-			'lookupServerUploadEnabled' => $state
+			'lookupServerUploadEnabled' => $state,
+			'federatedGroupSharingSupported' => $state,
+			'outgoingServer2serverGroupShareEnabled' => $state,
+			'incomingServer2serverGroupShareEnabled' => $state,
 		];
 		$expected = new TemplateResponse('federatedfilesharing', 'settings-admin', $params, '');
 		$this->assertEquals($expected, $this->admin->getForm());
--- 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;
+		}
 	}


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

--- 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, $allTypes, false, true, true],
+			], '', 'yes', true, true, true, $allTypes, false, true, true],
 			[[
 				'itemType' => 'folder',
 				'shareType' => $allTypes,
-			], '', 'yes', false, false, [0, 1], false, true, true],
+			], '', 'yes', false, false, false, [0, 1], false, true, true],
 			[[
 				'itemType' => 'folder',
 				'shareType' => $allTypes,
-			], '', 'yes', true, false, [0, 6], false, true, false],
+			], '', 'yes', true, false, false, [0, 6], false, true, false],
 			[[
 				'itemType' => 'folder',
 				'shareType' => $allTypes,
-			], '', 'yes', false, true, [0, 4], false, true, false],
+			], '', 'yes', false, false, true, [0, 4], false, true, false],
+			[[
+				'itemType' => 'folder',
+				'shareType' => $allTypes,
+			], '', '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)
--- 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,
 		];