Merge pull request #28209 from owncloud/stable10-improved-fed-user-se…

…arch

[Stable10] improved fed user search

apps/dav/lib/CardDAV/AddressBookImpl.php

@@ -86,11 +86,13 @@ public function getDisplayName() {
 	 * @param string $pattern which should match within the $searchProperties
 	 * @param array $searchProperties defines the properties within the query pattern should match
 	 * @param array $options - for future use. One should always have options!
+	 * @param int $limit
+	 * @param int $offset
 	 * @return array an array of contacts which are arrays of key-value-pairs
 	 * @since 5.0.0
 	 */
-	public function search($pattern, $searchProperties, $options) {
-		$results = $this->backend->search($this->getKey(), $pattern, $searchProperties);
+	public function search($pattern, $searchProperties, $options, $limit = null, $offset = null) {
+		$results = $this->backend->search($this->getKey(), $pattern, $searchProperties, $limit, $offset);
 
 		$vCards = [];
 		foreach ($results as $result) {

apps/dav/lib/CardDAV/CardDavBackend.php

@@ -790,9 +790,11 @@ public function updateShares(IShareable $shareable, $add, $remove) {
 	 * @param int $addressBookId
 	 * @param string $pattern which should match within the $searchProperties
 	 * @param array $searchProperties defines the properties within the query pattern should match
+	 * @param int $limit
+	 * @param int $offset
 	 * @return array an array of contacts which are arrays of key-value-pairs
 	 */
-	public function search($addressBookId, $pattern, $searchProperties) {
+	public function search($addressBookId, $pattern, $searchProperties, $limit = 100, $offset = 0) {
 		$query = $this->db->getQueryBuilder();
 		$query2 = $this->db->getQueryBuilder();
 		$query2->selectDistinct('cp.cardid')->from($this->dbCardsPropertiesTable, 'cp');
@@ -809,6 +811,9 @@ public function search($addressBookId, $pattern, $searchProperties) {
 		$query->select('c.carddata', 'c.uri')->from($this->dbCardsTable, 'c')
 			->where($query->expr()->in('c.id', $query->createFunction($query2->getSQL())));
 
+		$query->setFirstResult($offset)->setMaxResults($limit);
+		$query->orderBy('c.uri');
+
 		$result = $query->execute();
 		$cards = $result->fetchAll();
 

apps/dav/tests/unit/CardDAV/AddressBookImplTest.php

@@ -102,10 +102,10 @@ public function testSearch() {
 			->getMock();
 
 		$pattern = 'pattern';
-		$searchProperties = 'properties';
+		$searchProperties = ['properties'];
 
 		$this->backend->expects($this->once())->method('search')
-			->with($this->addressBookInfo['id'], $pattern, $searchProperties)
+			->with($this->addressBookInfo['id'], $pattern, $searchProperties, 10, 0)
 			->willReturn(
 				[
 					['uri' => 'foo.vcf', 'carddata' => 'cardData1'],
@@ -121,7 +121,7 @@ public function testSearch() {
 				['bar.vcf', $this->vCard]
 			)->willReturn('vCard');
 
-		$result = $addressBookImpl->search($pattern, $searchProperties, []);
+		$result = $addressBookImpl->search($pattern, $searchProperties, [], 10, 0);
 		$this->assertTrue((is_array($result)));
 		$this->assertSame(2, count($result));
 	}

apps/dav/tests/unit/CardDAV/CardDavBackendTest.php

@@ -466,7 +466,7 @@ public function testGetCardIdFailed() {
 	 * @param array $properties
 	 * @param array $expected
 	 */
-	public function testSearch($pattern, $properties, $expected) {
+	public function testSearch($pattern, $properties, $expected, $limit, $offset) {
 		/** @var VCard $vCards */
 		$vCards = [];
 		$vCards[0] = new VCard();
@@ -529,7 +529,7 @@ public function testSearch($pattern, $properties, $expected) {
 			);
 		$query->execute();
 
-		$result = $this->backend->search(0, $pattern, $properties);
+		$result = $this->backend->search(0, $pattern, $properties, $limit, $offset);
 
 		// check result
 		$this->assertSame(count($expected), count($result));
@@ -548,11 +548,13 @@ public function testSearch($pattern, $properties, $expected) {
 
 	public function dataTestSearch() {
 		return [
-				['John', ['FN'], [['uri0', 'John Doe'], ['uri1', 'John M. Doe']]],
-				['M. Doe', ['FN'], [['uri1', 'John M. Doe']]],
-				['Do', ['FN'], [['uri0', 'John Doe'], ['uri1', 'John M. Doe']]],
-				'check if duplicates are handled correctly' => ['John', ['FN', 'CLOUD'], [['uri0', 'John Doe'], ['uri1', 'John M. Doe']]],
-				'case insensitive' => ['john', ['FN'], [['uri0', 'John Doe'], ['uri1', 'John M. Doe']]]
+				['John', ['FN'], [['uri0', 'John Doe'], ['uri1', 'John M. Doe']], 100, 0],
+				['M. Doe', ['FN'], [['uri1', 'John M. Doe']], 100, 0],
+				['Do', ['FN'], [['uri0', 'John Doe'], ['uri1', 'John M. Doe']], 100, 0],
+				'check if duplicates are handled correctly' => ['John', ['FN', 'CLOUD'], [['uri0', 'John Doe'], ['uri1', 'John M. Doe']], 100, 0],
+				'case insensitive' => ['john', ['FN'], [['uri0', 'John Doe'], ['uri1', 'John M. Doe']], 100, 0],
+				'search limit' => ['John', ['FN'], [['uri0', 'John Doe']], 1, 0],
+				'search offset' => ['John', ['FN'], [['uri1', 'John M. Doe']], 1, 1],
 		];
 	}
 

apps/files_sharing/lib/Controller/ShareesController.php

@@ -291,58 +291,102 @@ protected function getGroups($search) {
 	 */
 	protected function getRemote($search) {
 		$this->result['remotes'] = [];
-
+		// Fetch remote search properties from app config
+		$searchProperties = explode(',', $this->config->getAppValue('dav', 'remote_search_properties', 'CLOUD,FN'));
 		// Search in contacts
-		//@todo Pagination missing
-		$addressBookContacts = $this->contactsManager->search($search, ['CLOUD', 'FN']);
+		$addressBookContacts = $this->contactsManager->search($search, $searchProperties, [], $this->limit, $this->offset);
 		$foundRemoteById = false;
 		foreach ($addressBookContacts as $contact) {
+
 			if (isset($contact['isLocalSystemBook'])) {
+				// We only want remote users
 				continue;
 			}
-			if (isset($contact['CLOUD'])) {
-				$cloudIds = $contact['CLOUD'];
-				if (!is_array($cloudIds)) {
-					$cloudIds = [$cloudIds];
+			if (!isset($contact['CLOUD'])) {
+				// we need a cloud id to setup a remote share
+				continue;
+			}
+
+			// we can have multiple cloud domains, always convert to an array
+			$cloudIds = $contact['CLOUD'];
+			if (!is_array($cloudIds)) {
+				$cloudIds = [$cloudIds];
+			}
+
+			$lowerSearch = strtolower($search);
+			foreach ($cloudIds as $cloudId) {
+				list(, $serverUrl) = $this->splitUserRemote($cloudId);
+
+
+				if (strtolower($cloudId) === $lowerSearch) {
+					$foundRemoteById = true;
+					// Save this as an exact match and continue with next CLOUD
+					$this->result['exact']['remotes'][] = [
+						'label' => $contact['FN'],
+						'value' => [
+							'shareType' => Share::SHARE_TYPE_REMOTE,
+							'shareWith' => $cloudId,
+							'server' => $serverUrl,
+						],
+					];
+					continue;
 				}
-				$lowerSearch = strtolower($search);
-				foreach ($cloudIds as $cloudId) {
-					list(, $serverUrl) = $this->splitUserRemote($cloudId);
-					if (strtolower($contact['FN']) === $lowerSearch || strtolower($cloudId) === $lowerSearch) {
-						if (strtolower($cloudId) === $lowerSearch) {
-							$foundRemoteById = true;
+
+				// CLOUD matching is done above
+				unset($searchProperties['CLOUD']);
+				foreach($searchProperties as $property) {
+					// do we even have this property for this contact/
+					if(!isset($contact[$property])) {
+						// Skip this property since our contact doesnt have it
+						continue;
+					}
+					// check if we have a match
+					$values = $contact[$property];
+					if(!is_array($values)) {
+						$values = [$values];
+					}
+					foreach($values as $value) {
+						// check if we have an exact match
+						if(strtolower($value) === $lowerSearch) {
+							$this->result['exact']['remotes'][] = [
+								'label' => $contact['FN'],
+								'value' => [
+									'shareType' => Share::SHARE_TYPE_REMOTE,
+									'shareWith' => $cloudId,
+									'server' => $serverUrl,
+								],
+							];
+
+							// Now skip to next CLOUD
+							continue 3;
 						}
-						$this->result['exact']['remotes'][] = [
-							'label' => $contact['FN'],
-							'value' => [
-								'shareType' => Share::SHARE_TYPE_REMOTE,
-								'shareWith' => $cloudId,
-								'server' => $serverUrl,
-							],
-						];
-					} else {
-						$this->result['remotes'][] = [
-							'label' => $contact['FN'],
-							'value' => [
-								'shareType' => Share::SHARE_TYPE_REMOTE,
-								'shareWith' => $cloudId,
-								'server' => $serverUrl,
-							],
-						];
 					}
 				}
+
+				// If we get here, we didnt find an exact match, so add to other matches
+				$this->result['remotes'][] = [
+					'label' => $contact['FN'],
+					'value' => [
+						'shareType' => Share::SHARE_TYPE_REMOTE,
+						'shareWith' => $cloudId,
+						'server' => $serverUrl,
+					],
+				];
+
 			}
 		}
 
+		// remove the exact user results if we dont allow autocomplete
 		if (!$this->shareeEnumeration) {
 			$this->result['remotes'] = [];
 		}
 
+
 		if (!$foundRemoteById && substr_count($search, '@') >= 1 && $this->offset === 0
 			// if an exact local user is found, only keep the remote entry if
 			// its domain does not matches the trusted domains
 			// (if it does, it is a user whose local login domain matches the ownCloud
-			// instance domain) 
+			// instance domain)
 			&& (empty($this->result['exact']['users'])
 				|| !$this->isInstanceDomain($search))
 		) {

apps/files_sharing/tests/API/ShareesTest.php

@@ -1233,6 +1233,34 @@ public function dataGetRemote() {
 					]
 				]
 			],
+			// #16 check email property is matched for remote users
+			[
+				'[email protected]',
+				[
+					[
+						'FN' => 'User3 @ Localhost',
+					],
+					[
+						'FN' => 'User2 @ Localhost',
+						'CLOUD' => [
+						],
+					],
+					[
+						'FN' => 'User @ Localhost',
+						'CLOUD' => [
+							'username@localhost',
+						],
+						'EMAIL' => '[email protected]'
+					],
+				],
+				true,
+				[
+					['label' => 'User @ Localhost', 'value' => ['shareType' => Share::SHARE_TYPE_REMOTE, 'shareWith' => 'username@localhost', 'server' => 'localhost']],
+					['label' => '[email protected]', 'value' => ['shareType' => Share::SHARE_TYPE_REMOTE, 'shareWith' => '[email protected]']]
+				],
+				[],
+				true,
+			],
 		];
 	}
 
@@ -1248,6 +1276,11 @@ public function dataGetRemote() {
 	 * @param array $previousExact
 	 */
 	public function testGetRemote($searchTerm, $contacts, $shareeEnumeration, $exactExpected, $expected, $reachedEnd, $previousExact = []) {
+
+		// Set the limit and offset for remote user searching
+		$this->invokePrivate($this->sharees, 'limit', [2]);
+		$this->invokePrivate($this->sharees, 'offset', [0]);
+
 		$this->config->expects($this->any())
 			->method('getSystemValue')
 			->with('trusted_domains')
@@ -1259,10 +1292,15 @@ public function testGetRemote($searchTerm, $contacts, $shareeEnumeration, $exact
 			$this->invokePrivate($this->sharees, 'result', [$result]);
 		}
 
+		$this->config->expects($this->once())
+			->method('getAppValue')
+			->with('dav', 'remote_search_properties')
+			->willReturn('EMAIL,CLOUD,FN');
+
 		$this->invokePrivate($this->sharees, 'shareeEnumeration', [$shareeEnumeration]);
 		$this->contactsManager->expects($this->any())
 			->method('search')
-			->with($searchTerm, ['CLOUD', 'FN'])
+			->with($searchTerm, ['EMAIL', 'CLOUD', 'FN'], [], 2, 0)
 			->willReturn($contacts);
 
 		$this->invokePrivate($this->sharees, 'getRemote', [$searchTerm]);

lib/private/ContactsManager.php

@@ -26,7 +26,9 @@
 
 namespace OC {
 
-	class ContactsManager implements \OCP\Contacts\IManager {
+	use OCP\Contacts\IManager;
+
+	class ContactsManager implements IManager {
 
 		/**
 		 * This function is used to search and find contacts within the users address books.
@@ -35,13 +37,15 @@ class ContactsManager implements \OCP\Contacts\IManager {
 		 * @param string $pattern which should match within the $searchProperties
 		 * @param array $searchProperties defines the properties within the query pattern should match
 		 * @param array $options - for future use. One should always have options!
+		 * @param int $limit
+		 * @param int $offset
 		 * @return array an array of contacts which are arrays of key-value-pairs
 		 */
-		public function search($pattern, $searchProperties = [], $options = []) {
+		public function search($pattern, $searchProperties = [], $options = [], $limit = 100, $offset = 0) {
 			$this->loadAddressBooks();
 			$result = [];
 			foreach($this->addressBooks as $addressBook) {
-				$r = $addressBook->search($pattern, $searchProperties, $options);
+				$r = $addressBook->search($pattern, $searchProperties, $options, $limit, $offset);
 				$contacts = [];
 				foreach($r as $c){
 					$c['addressbook-key'] = $addressBook->getKey();

lib/public/Contacts/IManager.php

@@ -92,10 +92,12 @@ interface IManager {
 		 * @param string $pattern which should match within the $searchProperties
 		 * @param array $searchProperties defines the properties within the query pattern should match
 		 * @param array $options - for future use. One should always have options!
+		 * @param int $limit
+		 * @param int $offset
 		 * @return array an array of contacts which are arrays of key-value-pairs
 		 * @since 6.0.0
 		 */
-		function search($pattern, $searchProperties = [], $options = []);
+		function search($pattern, $searchProperties = [], $options = [], $limit = null, $offset = null);
 
 		/**
 		 * This function can be used to delete the contact identified by the given id

lib/public/IAddressBook.php

@@ -55,10 +55,12 @@ public function getDisplayName();
 		 * @param string $pattern which should match within the $searchProperties
 		 * @param array $searchProperties defines the properties within the query pattern should match
 		 * @param array $options - for future use. One should always have options!
+		 * @param int $limit
+		 * @param int $offset
 		 * @return array an array of contacts which are arrays of key-value-pairs
 		 * @since 5.0.0
 		 */
-		public function search($pattern, $searchProperties, $options);
+		public function search($pattern, $searchProperties, $options, $limit = null, $offset = null);
 		//	// dummy results
 		//	return array(
 		//		array('id' => 0, 'FN' => 'Thomas Müller', 'EMAIL' => '[email protected]', 'GEO' => '37.386013;-122.082932'),