From 4accccf8e3643b7b1651a08d6631c0555a4b8254 Mon Sep 17 00:00:00 2001 From: Benjamin Gaussorgues Date: Thu, 19 Oct 2023 14:41:18 +0200 Subject: [PATCH] feat(carddav): Allow advanced search for contacts Signed-off-by: Benjamin Gaussorgues --- apps/dav/lib/CardDAV/CardDavBackend.php | 54 ++++++++++++++----- .../dav/lib/Search/ContactsSearchProvider.php | 51 ++++++++++++------ .../Search/ContactsSearchProviderTest.php | 4 +- 3 files changed, 78 insertions(+), 31 deletions(-) diff --git a/apps/dav/lib/CardDAV/CardDavBackend.php b/apps/dav/lib/CardDAV/CardDavBackend.php index e5ddeb9b4e198..613ec16921f16 100644 --- a/apps/dav/lib/CardDAV/CardDavBackend.php +++ b/apps/dav/lib/CardDAV/CardDavBackend.php @@ -53,6 +53,7 @@ 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 @@ public function searchPrincipalUri(string $principalUri, * @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 @@ private function searchByAddressBookIds(array $addressBookIds, 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 @@ private function searchByAddressBookIds(array $addressBookIds, $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 @@ private function searchByAddressBookIds(array $addressBookIds, $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 @@ public function pruneOutdatedSyncTokens(int $keep = 10_000): int { $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 6b0fea1b70a87..57e69c676e091 100644 --- a/apps/dav/lib/Search/ContactsSearchProvider.php +++ b/apps/dav/lib/Search/ContactsSearchProvider.php @@ -32,19 +32,23 @@ 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 @@ public function getName(): string { 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 @@ public function getOrder(string $route, array $routeParameters): int { 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 @@ public function search(IUser $user, ISearchQuery $query): SearchResult { $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 @@ protected function getDeepLinkToContactsApp( ); } - /** - * @param VCard $vCard - */ protected function generateSubline(VCard $vCard): string { $emailAddresses = $vCard->select('EMAIL'); if (!is_array($emailAddresses) || empty($emailAddresses)) { @@ -176,4 +173,24 @@ protected function generateSubline(VCard $vCard): string { 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 b91612ae5becc..f87a935f8b8b1 100644 --- a/apps/dav/tests/unit/Search/ContactsSearchProviderTest.php +++ b/apps/dav/tests/unit/Search/ContactsSearchProviderTest.php @@ -159,7 +159,7 @@ public function testSearch(): void { ]); $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 @@ public function testSearch(): void { 'ORG', 'NOTE', ], - ['limit' => 5, 'offset' => 20]) + ['limit' => 5, 'offset' => 20, 'since' => null, 'until' => null, 'person' => null, 'company' => null]) ->willReturn([ [ 'addressbookid' => 99,