feat(carddav): Allow advanced search for contacts

Signed-off-by: Benjamin Gaussorgues <benjamin.gaussorgues@nextcloud.com>

3 files changed, 78 insertions, 31 deletions

diff --git a/apps/dav/lib/CardDAV/CardDavBackend.php b/apps/dav/lib/CardDAV/CardDavBackend.php
index e5ddeb9b4e1..613ec16921f 100644
--- a/apps/dav/lib/CardDAV/CardDavBackend.php
+++ b/apps/dav/lib/CardDAV/CardDavBackend.php
@@ -53,6 +53,7 @@ use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IDBConnection;
 use OCP\IGroupManager;
 use OCP\IUserManager;
+use OC\Search\Filter\DateTimeFilter;
 use PDO;
 use Sabre\CardDAV\Backend\BackendInterface;
 use Sabre\CardDAV\Backend\SyncSupport;
@@ -1109,7 +1110,15 @@ class CardDavBackend implements BackendInterface, SyncSupport {
 	 * @param string $pattern
 	 * @param array $searchProperties
 	 * @param array $options
-	 * @psalm-param array{types?: bool, escape_like_param?: bool, limit?: int, offset?: int, wildcard?: bool} $options
+	 * @psalm-param array{
+	 *   types?: bool,
+	 *   escape_like_param?: bool,
+	 *   limit?: int,
+	 *   offset?: int,
+	 *   wildcard?: bool,
+	 *   since?: DateTimeFilter|null,
+	 *   until?: DateTimeFilter|null,
+	 * } $options
 	 * @return array
 	 */
 	private function searchByAddressBookIds(array $addressBookIds,
@@ -1130,32 +1139,31 @@ class CardDavBackend implements BackendInterface, SyncSupport {
 			return [];
 		}
 
-		$propertyOr = $query2->expr()->orX();
-		foreach ($searchProperties as $property) {
-			if ($escapePattern) {
+		if ($escapePattern) {
+			$searchProperties = array_filter($searchProperties, function ($property) use ($pattern) {
 				if ($property === 'EMAIL' && str_contains($pattern, ' ')) {
 					// There can be no spaces in emails
-					continue;
+					return false;
 				}
 
 				if ($property === 'CLOUD' && preg_match('/[^a-zA-Z0-9 :_.@\/\-\']/', $pattern) === 1) {
 					// There can be no chars in cloud ids which are not valid for user ids plus :/
 					// worst case: CA61590A-BBBC-423E-84AF-E6DF01455A53@https://my.nxt/srv/
-					continue;
+					return false;
 				}
-			}
 
-			$propertyOr->add($query2->expr()->eq('cp.name', $query2->createNamedParameter($property)));
+				return true;
+			});
 		}
 
-		if ($propertyOr->count() === 0) {
+		if (empty($searchProperties)) {
 			return [];
 		}
 
 		$query2->selectDistinct('cp.cardid')
 			->from($this->dbCardsPropertiesTable, 'cp')
 			->andWhere($addressBookOr)
-			->andWhere($propertyOr);
+			->andWhere($query2->expr()->in('cp.name', $query2->createNamedParameter($searchProperties, IQueryBuilder::PARAM_STR_ARRAY)));
 
 		// No need for like when the pattern is empty
 		if ('' !== $pattern) {
@@ -1167,7 +1175,6 @@ class CardDavBackend implements BackendInterface, SyncSupport {
 				$query2->andWhere($query2->expr()->ilike('cp.value', $query2->createNamedParameter('%' . $this->db->escapeLikeParameter($pattern) . '%')));
 			}
 		}
-
 		if (isset($options['limit'])) {
 			$query2->setMaxResults($options['limit']);
 		}
@@ -1175,6 +1182,29 @@ class CardDavBackend implements BackendInterface, SyncSupport {
 			$query2->setFirstResult($options['offset']);
 		}
 
+		if (isset($options['since']) || isset($options['until'])) {
+			$query2->join('cp', $this->dbCardsPropertiesTable, 'cp_bday', 'cp.cardid = cp_bday.cardid');
+			$query2->andWhere($query2->expr()->eq('cp_bday.name', $query2->createNamedParameter('BDAY')));
+			/**
+			 * FIXME Find a way to match only 4 last digits
+			 * BDAY can be --1018 without year or 20001019 with it
+			 * $bDayOr = $query2->expr()->orX();
+			 * if ($options['since'] instanceof DateTimeFilter) {
+			 * $bDayOr->add(
+			 * $query2->expr()->gte('SUBSTR(cp_bday.value, -4)',
+			 * $query2->createNamedParameter($options['since']->get()->format('md')))
+			 * );
+			 * }
+			 * if ($options['until'] instanceof DateTimeFilter) {
+			 * $bDayOr->add(
+			 * $query2->expr()->lte('SUBSTR(cp_bday.value, -4)',
+			 * $query2->createNamedParameter($options['until']->get()->format('md')))
+			 * );
+			 * }
+			 * $query2->andWhere($bDayOr);
+			 */
+		}
+
 		$result = $query2->execute();
 		$matches = $result->fetchAll();
 		$result->closeCursor();
@@ -1410,7 +1440,7 @@ class CardDavBackend implements BackendInterface, SyncSupport {
 		$maxId = (int) $result->fetchOne();
 		$result->closeCursor();
 		if (!$maxId || $maxId < $keep) {
-		    return 0;
+			return 0;
 		}
 
 		$query = $this->db->getQueryBuilder();
diff --git a/apps/dav/lib/Search/ContactsSearchProvider.php b/apps/dav/lib/Search/ContactsSearchProvider.php
index 6b0fea1b70a..57e69c676e0 100644
--- a/apps/dav/lib/Search/ContactsSearchProvider.php
+++ b/apps/dav/lib/Search/ContactsSearchProvider.php
@@ -32,19 +32,23 @@ use OCP\App\IAppManager;
 use OCP\IL10N;
 use OCP\IURLGenerator;
 use OCP\IUser;
-use OCP\Search\IProvider;
+use OCP\Search\FilterDefinition;
+use OCP\Search\IFilteringProvider;
 use OCP\Search\ISearchQuery;
 use OCP\Search\SearchResult;
 use OCP\Search\SearchResultEntry;
 use Sabre\VObject\Component\VCard;
 use Sabre\VObject\Reader;
 
-class ContactsSearchProvider implements IProvider {
+class ContactsSearchProvider implements IFilteringProvider {
+	private static array $searchPropertiesRestricted = [
+		'N',
+		'FN',
+		'NICKNAME',
+		'EMAIL',
+	];
 
-	/**
-	 * @var string[]
-	 */
-	private static $searchProperties = [
+	private static array $searchProperties = [
 		'N',
 		'FN',
 		'NICKNAME',
@@ -78,9 +82,6 @@ class ContactsSearchProvider implements IProvider {
 		return $this->l10n->t('Contacts');
 	}
 
-	/**
-	 * @inheritDoc
-	 */
 	public function getOrder(string $route, array $routeParameters): int {
 		if ($route === 'contacts.Page.index') {
 			return -1;
@@ -88,9 +89,6 @@ class ContactsSearchProvider implements IProvider {
 		return 25;
 	}
 
-	/**
-	 * @inheritDoc
-	 */
 	public function search(IUser $user, ISearchQuery $query): SearchResult {
 		if (!$this->appManager->isEnabledForUser('contacts', $user)) {
 			return SearchResult::complete($this->getName(), []);
@@ -106,13 +104,15 @@ class ContactsSearchProvider implements IProvider {
 		$searchResults = $this->backend->searchPrincipalUri(
 			$principalUri,
 			$query->getFilter('term')?->get() ?? '',
-			self::$searchProperties,
+			$query->getFilter('title-only')?->get() ? self::$searchPropertiesRestricted : self::$searchProperties,
 			[
 				'limit' => $query->getLimit(),
 				'offset' => $query->getCursor(),
 				'since' => $query->getFilter('since'),
 				'until' => $query->getFilter('until'),
-			]
+				'person' => $query->getFilter('person'),
+				'company' => $query->getFilter('company'),
+			],
 		);
 		$formattedResults = \array_map(function (array $contactRow) use ($addressBooksById):SearchResultEntry {
 			$addressBook = $addressBooksById[$contactRow['addressbookid']];
@@ -165,9 +165,6 @@ class ContactsSearchProvider implements IProvider {
 		);
 	}
 
-	/**
-	 * @param VCard $vCard
-	 */
 	protected function generateSubline(VCard $vCard): string {
 		$emailAddresses = $vCard->select('EMAIL');
 		if (!is_array($emailAddresses) || empty($emailAddresses)) {
@@ -176,4 +173,24 @@ class ContactsSearchProvider implements IProvider {
 
 		return (string)$emailAddresses[0];
 	}
+
+	public function getSupportedFilters(): array {
+		return [
+			'term',
+			'since',
+			'until',
+			'person',
+			'title-only',
+		];
+	}
+
+	public function getAlternateIds(): array {
+		return [];
+	}
+
+	public function getCustomFilters(): array {
+		return [
+			new FilterDefinition('company'),
+		];
+	}
 }
diff --git a/apps/dav/tests/unit/Search/ContactsSearchProviderTest.php b/apps/dav/tests/unit/Search/ContactsSearchProviderTest.php
index b91612ae5be..f87a935f8b8 100644
--- a/apps/dav/tests/unit/Search/ContactsSearchProviderTest.php
+++ b/apps/dav/tests/unit/Search/ContactsSearchProviderTest.php
@@ -159,7 +159,7 @@ class ContactsSearchProviderTest extends TestCase {
 			]);
 		$this->backend->expects($this->once())
 			->method('searchPrincipalUri')
-			->with('principals/users/john.doe', 'search term',
+			->with('principals/users/john.doe', '',
 				[
 					'N',
 					'FN',
@@ -171,7 +171,7 @@ class ContactsSearchProviderTest extends TestCase {
 					'ORG',
 					'NOTE',
 				],
-				['limit' => 5, 'offset' => 20])
+				['limit' => 5, 'offset' => 20, 'since' => null, 'until' => null,  'person' => null, 'company' => null])
 			->willReturn([
 				[
 					'addressbookid' => 99,