Split logic and global usage and add tests for "searchSharees()"

apps/files_sharing/api/sharees.php

@@ -74,7 +74,7 @@ class Sharees {
 	 *
 	 * @return array possible sharees
 	 */
-	private function getUsers($search, $shareWithGroupOnly) {
+	protected function getUsers($search, $shareWithGroupOnly) {
 		$sharees = [];
 		
 		$users = [];
@@ -115,7 +115,7 @@ class Sharees {
 	 *
 	 * @return array possible sharees
 	 */
-	private function getGroups($search, $shareWithGroupOnly) {
+	protected function getGroups($search, $shareWithGroupOnly) {
 		$sharees = [];
 		$groups = $this->groupManager->search($search);
 		$groups = array_map(function (IGroup $group) { return $group->getGID(); }, $groups);
@@ -145,7 +145,7 @@ class Sharees {
 	 *
 	 * @return array possible sharees
 	 */
-	private function getRemote($search) {
+	protected function getRemote($search) {
 		$sharees = [];
 
 		if (substr_count($search, '@') >= 1) {
@@ -177,14 +177,37 @@ class Sharees {
 		return $sharees;
 	}
 
+	/**
+	 * @param array $params
+	 * @return \OC_OCS_Result
+	 */
 	public function search($params) {
-		$search = isset($_GET['search']) ? (string) $_GET['search'] : '';
-		$itemType = isset($_GET['itemType']) ? (string) $_GET['itemType'] : null;
-		$existingShares = isset($_GET['existingShares']) ? (array) $_GET['existingShares'] : [];
+		$search = isset($_GET['search']) ? (string)$_GET['search'] : '';
+		$itemType = isset($_GET['itemType']) ? (string)$_GET['itemType'] : null;
+		$existingShares = isset($_GET['existingShares']) ? (array)$_GET['existingShares'] : [];
 		$shareType = isset($_GET['shareType']) ? intval($_GET['shareType']) : null;
 		$page = !empty($_GET['page']) ? intval($_GET['page']) : 1;
 		$perPage = !empty($_GET['limit']) ? intval($_GET['limit']) : 200;
 
+		$shareWithGroupOnly = $this->appConfig->getValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes';
+
+		return $this->searchSharees($search, $itemType, $existingShares, $shareType, $page, $perPage, $shareWithGroupOnly);
+	}
+
+	/**
+	 * Testable search function that does not need globals
+	 *
+	 * @param string $search
+	 * @param string $itemType
+	 * @param array $existingShares
+	 * @param int $shareType
+	 * @param int $page
+	 * @param int $perPage
+	 * @param bool $shareWithGroupOnly
+	 * @return \OC_OCS_Result
+	 */
+	protected function searchSharees($search, $itemType, array $existingShares, $shareType, $page, $perPage, $shareWithGroupOnly) {
+
 		$sharedUsers = $sharedGroups = [];
 		if (!empty($existingShares)) {
 			if (!empty($existingShares[\OCP\Share::SHARE_TYPE_USER]) &&
@@ -203,8 +226,6 @@ class Sharees {
 			return new \OC_OCS_Result(null, 400, 'missing itemType');
 		}
 
-		$shareWithGroupOnly = $this->appConfig->getValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes' ? true : false;
-
 		$sharees = [];
 		// Get users
 		if ($shareType === null || $shareType === \OCP\Share::SHARE_TYPE_USER) {
@@ -267,7 +288,7 @@ class Sharees {
 	 * @param array $existingSharees
 	 * @return array
 	 */
-	private function filterSharees($potentialSharees, $existingSharees) {
+	protected function filterSharees($potentialSharees, $existingSharees) {
 		$sharees = array_map(function ($sharee) use ($existingSharees) {
 			return in_array($sharee['value']['shareWith'], $existingSharees) ? null : $sharee;
 		}, $potentialSharees);

apps/files_sharing/tests/api/sharees.php

@@ -344,39 +344,191 @@ class ShareesTest extends TestCase {
 		$this->assertEquals($expected, $users);
 	}
 
-//	public function testArguments() {
-//
-//	}
-//
-//	public function testsOnlyUsers() {
-//
-//	}
-//
-//	public function testOnlyGroups() {
-//
-//	}
-//
-//	public function testRemoteAddress() {
-//
-//	}
-//
-//	public function testRemoteFromContacts() {
-//
-//	}
-//
-//	public function testAll() {
-//
-//	}
-//
-//	public function testSorting() {
-//
-//	}
-//
-//	public function testPagination() {
-//
-//	}
-//
-//	public function testShareWithinGroup() {
-//
-//	}
+	public function dataSearchSharees() {
+		return [
+			['test', 'folder', [], null, 1, 2, false, [], [], [], [], 0, false],
+			['test', 'folder', [0 => ['test1'], 1 => ['test2 group']], null, 1, 2, false, [], [], [], [], 0, false],
+			// First page with 2 of 3 results
+			[
+				'test', 'folder', [], null, 1, 2, false, [
+					['label' => 'test One', 'value' => ['shareType' => \OCP\Share::SHARE_TYPE_USER, 'shareWith' => 'test1']],
+				], [
+					['label' => 'testgroup1', 'value' => ['shareType' => \OCP\Share::SHARE_TYPE_GROUP, 'shareWith' => 'testgroup1']],
+				], [
+					['label' => 'testz@remote', 'value' => ['shareType' => \OCP\Share::SHARE_TYPE_REMOTE, 'shareWith' => 'testz@remote']],
+				], [
+					['label' => 'test One', 'value' => ['shareType' => \OCP\Share::SHARE_TYPE_USER, 'shareWith' => 'test1']],
+					['label' => 'testgroup1', 'value' => ['shareType' => \OCP\Share::SHARE_TYPE_GROUP, 'shareWith' => 'testgroup1']],
+				], 3, true,
+			],
+			// Second page with the 3rd result
+			[
+				'test', 'folder', [], null, 2, 2, false, [
+					['label' => 'test One', 'value' => ['shareType' => \OCP\Share::SHARE_TYPE_USER, 'shareWith' => 'test1']],
+				], [
+					['label' => 'testgroup1', 'value' => ['shareType' => \OCP\Share::SHARE_TYPE_GROUP, 'shareWith' => 'testgroup1']],
+				], [
+					['label' => 'testz@remote', 'value' => ['shareType' => \OCP\Share::SHARE_TYPE_REMOTE, 'shareWith' => 'testz@remote']],
+				], [
+					['label' => 'testz@remote', 'value' => ['shareType' => \OCP\Share::SHARE_TYPE_REMOTE, 'shareWith' => 'testz@remote']],
+				], 3, false,
+			],
+			// Ingnoring already shared user
+			[
+				'test', 'folder', [\OCP\Share::SHARE_TYPE_USER => ['test1']], null, 1, 2, false, [
+					['label' => 'test One', 'value' => ['shareType' => \OCP\Share::SHARE_TYPE_USER, 'shareWith' => 'test1']],
+				], [
+					['label' => 'testgroup1', 'value' => ['shareType' => \OCP\Share::SHARE_TYPE_GROUP, 'shareWith' => 'testgroup1']],
+				], [
+					['label' => 'testz@remote', 'value' => ['shareType' => \OCP\Share::SHARE_TYPE_REMOTE, 'shareWith' => 'testz@remote']],
+				], [
+					['label' => 'testgroup1', 'value' => ['shareType' => \OCP\Share::SHARE_TYPE_GROUP, 'shareWith' => 'testgroup1']],
+					['label' => 'testz@remote', 'value' => ['shareType' => \OCP\Share::SHARE_TYPE_REMOTE, 'shareWith' => 'testz@remote']],
+				], 2, false,
+			],
+			// Share type restricted to user - Only one user
+			[
+				'test', 'folder', [], \OCP\Share::SHARE_TYPE_USER, 1, 2, false, [
+					['label' => 'test One', 'value' => ['shareType' => \OCP\Share::SHARE_TYPE_USER, 'shareWith' => 'test1']],
+				], null, null, [
+					['label' => 'test One', 'value' => ['shareType' => \OCP\Share::SHARE_TYPE_USER, 'shareWith' => 'test1']],
+				], 1, false,
+			],
+			// Share type restricted to user - Multipage result
+			[
+				'test', 'folder', [], \OCP\Share::SHARE_TYPE_USER, 1, 2, false, [
+					['label' => 'test 1', 'value' => ['shareType' => \OCP\Share::SHARE_TYPE_USER, 'shareWith' => 'test1']],
+					['label' => 'test 2', 'value' => ['shareType' => \OCP\Share::SHARE_TYPE_USER, 'shareWith' => 'test2']],
+					['label' => 'test 3', 'value' => ['shareType' => \OCP\Share::SHARE_TYPE_USER, 'shareWith' => 'test3']],
+				], null, null, [
+					['label' => 'test 1', 'value' => ['shareType' => \OCP\Share::SHARE_TYPE_USER, 'shareWith' => 'test1']],
+					['label' => 'test 2', 'value' => ['shareType' => \OCP\Share::SHARE_TYPE_USER, 'shareWith' => 'test2']],
+				], 3, true,
+			],
+			// Share type restricted to user - Only user already shared
+			[
+				'test', 'folder', [\OCP\Share::SHARE_TYPE_USER => ['test1']], \OCP\Share::SHARE_TYPE_USER, 1, 2, false, [
+					['label' => 'test One', 'value' => ['shareType' => \OCP\Share::SHARE_TYPE_USER, 'shareWith' => 'test1']],
+				], null, null, [], 0, false,
+			],
+		];
+	}
+
+	/**
+	 * @dataProvider dataSearchSharees
+	 *
+	 * @param string $searchTerm
+	 * @param string $itemType
+	 * @param array $existingShares
+	 * @param int $shareType
+	 * @param int $page
+	 * @param int $perPage
+	 * @param bool $shareWithGroupOnly
+	 * @param array $expected
+	 */
+	public function testSearchSharees($searchTerm, $itemType, array $existingShares, $shareType, $page, $perPage, $shareWithGroupOnly,
+									  $mockedUserResult, $mockedGroupsResult, $mockedRemotesResult, $expected, $totalItems, $nextLink) {
+		/** @var \PHPUnit_Framework_MockObject_MockObject|\OCA\Files_Sharing\API\Sharees $sharees */
+		$sharees = $this->getMockBuilder('\OCA\Files_Sharing\API\Sharees')
+			->setConstructorArgs([
+				$this->groupManager,
+				$this->userManager,
+				$this->contactsManager,
+				$this->getMockBuilder('OCP\IAppConfig')->disableOriginalConstructor()->getMock(),
+				$this->session,
+				$this->getMockBuilder('OCP\IURLGenerator')->disableOriginalConstructor()->getMock()
+			])
+			->setMethods(array('getUsers', 'getGroups', 'getRemote'))
+			->getMock();
+		$sharees->expects(($mockedUserResult === null) ? $this->never() : $this->once())
+			->method('getUsers')
+			->with($searchTerm, $shareWithGroupOnly)
+			->willReturn($mockedUserResult);
+		$sharees->expects(($mockedGroupsResult === null) ? $this->never() : $this->once())
+			->method('getGroups')
+			->with($searchTerm, $shareWithGroupOnly)
+			->willReturn($mockedGroupsResult);
+		$sharees->expects(($mockedRemotesResult === null) ? $this->never() : $this->once())
+			->method('getRemote')
+			->with($searchTerm)
+			->willReturn($mockedRemotesResult);
+
+		/** @var \OC_OCS_Result $ocs */
+		$ocs = $this->invokePrivate($sharees, 'searchSharees', [$searchTerm, $itemType, $existingShares, $shareType, $page, $perPage, $shareWithGroupOnly]);
+
+		$this->assertEquals($expected, $ocs->getData());
+
+		// Check number of total results
+		$meta = $ocs->getMeta();
+		$this->assertArrayHasKey('totalitems', $meta);
+		$this->assertSame($totalItems, $meta['totalitems']);
+
+		// Check if next link is set
+		if ($nextLink) {
+			$headers = $ocs->getHeaders();
+			$this->assertArrayHasKey('Link', $headers);
+			$this->assertStringStartsWith('<', $headers['Link']);
+			$this->assertStringEndsWith('> rel="next"', $headers['Link']);
+		}
+	}
+
+	public function testSearchShareesNoItemType() {
+		/** @var \OC_OCS_Result $ocs */
+		$ocs = $this->invokePrivate($this->sharees, 'searchSharees', ['', null, [], null, 0, 0, false]);
+
+		$this->assertSame(400, $ocs->getStatusCode(), 'Expected status code 400');
+		$this->assertSame([], $ocs->getData(), 'Expected that no data is send');
+
+		$meta = $ocs->getMeta();
+		$this->assertNotEmpty($meta);
+		$this->assertArrayHasKey('message', $meta);
+		$this->assertSame('missing itemType', $meta['message']);
+	}
+
+
+	public function dataFilterSharees() {
+		return [
+			[[], [], []],
+			[
+				[
+					['label' => 'Test One', 'value' => ['shareType' => \OCP\Share::SHARE_TYPE_USER, 'shareWith' => 'test1']],
+				],
+				[],
+				[
+					['label' => 'Test One', 'value' => ['shareType' => \OCP\Share::SHARE_TYPE_USER, 'shareWith' => 'test1']],
+				],
+			],
+			[
+				[
+					['label' => 'Test One', 'value' => ['shareType' => \OCP\Share::SHARE_TYPE_USER, 'shareWith' => 'test1']],
+					['label' => 'Test Two', 'value' => ['shareType' => \OCP\Share::SHARE_TYPE_USER, 'shareWith' => 'test2']],
+				],
+				['test1'],
+				[
+					1 => ['label' => 'Test Two', 'value' => ['shareType' => \OCP\Share::SHARE_TYPE_USER, 'shareWith' => 'test2']],
+				],
+			],
+			[
+				[
+					['label' => 'Test One', 'value' => ['shareType' => \OCP\Share::SHARE_TYPE_USER, 'shareWith' => 'test1']],
+					['label' => 'Test Two', 'value' => ['shareType' => \OCP\Share::SHARE_TYPE_USER, 'shareWith' => 'test2']],
+				],
+				['test2'],
+				[
+					0 => ['label' => 'Test One', 'value' => ['shareType' => \OCP\Share::SHARE_TYPE_USER, 'shareWith' => 'test1']],
+				],
+			],
+		];
+	}
+
+	/**
+	 * @dataProvider dataFilterSharees
+	 *
+	 * @param array $potentialSharees
+	 * @param array $existingSharees
+	 * @param array $expectedSharees
+	 */
+	public function testFilterSharees($potentialSharees, $existingSharees, $expectedSharees) {
+		$this->assertEquals($expectedSharees, $this->invokePrivate($this->sharees, 'filterSharees', [$potentialSharees, $existingSharees]));
+	}
 }