From 452a8d9390c53bea12c10a56c7a369149e48b4e0 Mon Sep 17 00:00:00 2001 From: Kevin Papst Date: Mon, 5 May 2025 18:18:00 +0200 Subject: [PATCH] improved search with negation (#5453) --- phpstan.neon | 5 - src/Repository/ActivityRepository.php | 30 +-- src/Repository/CustomerRepository.php | 30 +-- src/Repository/InvoiceRepository.php | 31 +-- src/Repository/ProjectRepository.php | 30 +-- src/Repository/RepositorySearchTrait.php | 121 +++-------- src/Repository/Search/SearchConfiguration.php | 66 ++++++ .../Search/SearchConfigurationInterface.php | 36 ++++ src/Repository/Search/SearchHelper.php | 147 +++++++++++++ src/Repository/TagRepository.php | 21 +- src/Repository/TimesheetRepository.php | 30 +-- src/Repository/UserRepository.php | 43 +--- src/Utils/SearchTerm.php | 70 ++++-- src/Utils/SearchTermPart.php | 51 +++++ tests/Controller/ProjectControllerTest.php | 26 ++- .../SearchTermTransformerTest.php | 15 +- .../Search/SearchConfigurationTest.php | 40 ++++ tests/Repository/Search/SearchHelperTest.php | 199 ++++++++++++++++++ tests/Utils/SearchTermPartTest.php | 43 ++++ tests/Utils/SearchTermTest.php | 134 ++++++++++-- 20 files changed, 881 insertions(+), 287 deletions(-) create mode 100644 src/Repository/Search/SearchConfiguration.php create mode 100644 src/Repository/Search/SearchConfigurationInterface.php create mode 100644 src/Repository/Search/SearchHelper.php create mode 100644 src/Utils/SearchTermPart.php create mode 100644 tests/Repository/Search/SearchConfigurationTest.php create mode 100644 tests/Repository/Search/SearchHelperTest.php create mode 100644 tests/Utils/SearchTermPartTest.php diff --git a/phpstan.neon b/phpstan.neon index 8b2ba46c..f12fd6bf 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -4372,11 +4372,6 @@ parameters: count: 1 path: src/Utils/ReleaseVersion.php - - - message: "#^Method App\\\\Utils\\\\SearchTerm\\:\\:getSearchFields\\(\\) return type has no value type specified in iterable type array\\.$#" - count: 1 - path: src/Utils/SearchTerm.php - - message: "#^Cannot access offset 'pattern' on mixed\\.$#" count: 1 diff --git a/src/Repository/ActivityRepository.php b/src/Repository/ActivityRepository.php index 45f4977b..77670b91 100644 --- a/src/Repository/ActivityRepository.php +++ b/src/Repository/ActivityRepository.php @@ -21,6 +21,8 @@ use App\Repository\Paginator\PaginatorInterface; use App\Repository\Query\ActivityFormTypeQuery; use App\Repository\Query\ActivityQuery; use App\Repository\Query\ActivityQueryHydrate; +use App\Repository\Search\SearchConfiguration; +use App\Repository\Search\SearchHelper; use App\Utils\Pagination; use Doctrine\DBAL\ParameterType; use Doctrine\ORM\EntityRepository; @@ -35,8 +37,6 @@ use Doctrine\ORM\QueryBuilder; */ class ActivityRepository extends EntityRepository { - use RepositorySearchTrait; - /** * @param int[] $activityIds * @return array @@ -330,29 +330,17 @@ class ActivityRepository extends EntityRepository $this->addPermissionCriteria($qb, $query->getCurrentUser(), $query->getTeams(), $query->isGlobalsOnly()); - $this->addSearchTerm($qb, $query); + $configuration = new SearchConfiguration( + ['a.name', 'a.comment', 'a.number'], + ActivityMeta::class, + 'activity' + ); + $helper = new SearchHelper($configuration); + $helper->addSearchTerm($qb, $query); return $qb; } - private function getMetaFieldClass(): string - { - return ActivityMeta::class; - } - - private function getMetaFieldName(): string - { - return 'activity'; - } - - /** - * @return array - */ - private function getSearchableFields(): array - { - return ['a.name', 'a.comment', 'a.number']; - } - /** * @return int<0, max> */ diff --git a/src/Repository/CustomerRepository.php b/src/Repository/CustomerRepository.php index e42f94d9..5bbfd3ed 100644 --- a/src/Repository/CustomerRepository.php +++ b/src/Repository/CustomerRepository.php @@ -21,6 +21,8 @@ use App\Repository\Paginator\PaginatorInterface; use App\Repository\Query\CustomerFormTypeQuery; use App\Repository\Query\CustomerQuery; use App\Repository\Query\CustomerQueryHydrate; +use App\Repository\Search\SearchConfiguration; +use App\Repository\Search\SearchHelper; use App\Utils\Pagination; use Doctrine\DBAL\ParameterType; use Doctrine\ORM\EntityRepository; @@ -35,8 +37,6 @@ use Doctrine\ORM\QueryBuilder; */ class CustomerRepository extends EntityRepository { - use RepositorySearchTrait; - /** * @param int[] $customerIDs * @return array @@ -209,29 +209,17 @@ class CustomerRepository extends EntityRepository $this->addPermissionCriteria($qb, $query->getCurrentUser(), $query->getTeams()); - $this->addSearchTerm($qb, $query); + $configuration = new SearchConfiguration( + ['c.name', 'c.comment', 'c.company', 'c.vatId', 'c.number', 'c.contact', 'c.phone', 'c.email', 'c.address'], + CustomerMeta::class, + 'customer' + ); + $helper = new SearchHelper($configuration); + $helper->addSearchTerm($qb, $query); return $qb; } - private function getMetaFieldClass(): string - { - return CustomerMeta::class; - } - - private function getMetaFieldName(): string - { - return 'customer'; - } - - /** - * @return array - */ - private function getSearchableFields(): array - { - return ['c.name', 'c.comment', 'c.company', 'c.vatId', 'c.number', 'c.contact', 'c.phone', 'c.email', 'c.address']; - } - public function getPagerfantaForQuery(CustomerQuery $query): Pagination { return new Pagination($this->getPaginatorForQuery($query), $query); diff --git a/src/Repository/InvoiceRepository.php b/src/Repository/InvoiceRepository.php index 2991ad5a..946ab9e7 100644 --- a/src/Repository/InvoiceRepository.php +++ b/src/Repository/InvoiceRepository.php @@ -17,6 +17,8 @@ use App\Entity\User; use App\Repository\Paginator\PaginatorInterface; use App\Repository\Paginator\QueryPaginator; use App\Repository\Query\InvoiceArchiveQuery; +use App\Repository\Search\SearchConfiguration; +use App\Repository\Search\SearchHelper; use App\Utils\Pagination; use Doctrine\ORM\EntityRepository; use Doctrine\ORM\Mapping\ClassMetadata; @@ -28,8 +30,6 @@ use Doctrine\ORM\QueryBuilder; */ class InvoiceRepository extends EntityRepository { - use RepositorySearchTrait; - public function saveInvoice(Invoice $invoice): void { $entityManager = $this->getEntityManager(); @@ -235,30 +235,19 @@ class InvoiceRepository extends EntityRepository if ($query->hasSearchTerm()) { $qb->leftJoin('i.customer', 'customer'); - $this->addSearchTerm($qb, $query); + + $configuration = new SearchConfiguration( + ['i.comment', 'customer.name', 'customer.company'], + InvoiceMeta::class, + 'invoice' + ); + $helper = new SearchHelper($configuration); + $helper->addSearchTerm($qb, $query); } return $qb; } - private function getMetaFieldClass(): string - { - return InvoiceMeta::class; - } - - private function getMetaFieldName(): string - { - return 'invoice'; - } - - /** - * @return array - */ - private function getSearchableFields(): array - { - return ['i.comment', 'customer.name', 'customer.company']; - } - /** * @return int<0, max> */ diff --git a/src/Repository/ProjectRepository.php b/src/Repository/ProjectRepository.php index ced92a64..d23ee718 100644 --- a/src/Repository/ProjectRepository.php +++ b/src/Repository/ProjectRepository.php @@ -22,6 +22,8 @@ use App\Repository\Paginator\PaginatorInterface; use App\Repository\Query\ProjectFormTypeQuery; use App\Repository\Query\ProjectQuery; use App\Repository\Query\ProjectQueryHydrate; +use App\Repository\Search\SearchConfiguration; +use App\Repository\Search\SearchHelper; use App\Utils\Pagination; use DateTime; use Doctrine\DBAL\ParameterType; @@ -38,8 +40,6 @@ use Doctrine\ORM\QueryBuilder; */ class ProjectRepository extends EntityRepository { - use RepositorySearchTrait; - /** * @param int[] $projectIds * @return array @@ -281,29 +281,17 @@ class ProjectRepository extends EntityRepository $this->addPermissionCriteria($qb, $query->getCurrentUser()); - $this->addSearchTerm($qb, $query); + $configuration = new SearchConfiguration( + ['p.name', 'p.comment', 'p.orderNumber', 'p.number'], + ProjectMeta::class, + 'project' + ); + $helper = new SearchHelper($configuration); + $helper->addSearchTerm($qb, $query); return $qb; } - private function getMetaFieldClass(): string - { - return ProjectMeta::class; - } - - private function getMetaFieldName(): string - { - return 'project'; - } - - /** - * @return array - */ - private function getSearchableFields(): array - { - return ['p.name', 'p.comment', 'p.orderNumber', 'p.number']; - } - private function addProjectStartAndEndDate(QueryBuilder $qb, ?DateTime $begin, ?DateTime $end): Andx { $and = $qb->expr()->andX(); diff --git a/src/Repository/RepositorySearchTrait.php b/src/Repository/RepositorySearchTrait.php index 230979f5..18706c0c 100644 --- a/src/Repository/RepositorySearchTrait.php +++ b/src/Repository/RepositorySearchTrait.php @@ -10,20 +10,42 @@ namespace App\Repository; use App\Repository\Query\BaseQuery; +use App\Repository\Search\SearchConfiguration; +use App\Repository\Search\SearchHelper; use Doctrine\ORM\QueryBuilder; -trait RepositorySearchTrait +/** + * @deprecated since 2.34.0 use SearchHelper instead + */ +trait RepositorySearchTrait // @phpstan-ignore trait.unused { + /** + * The name of the meta-field class, e.g. ProjectMeta::class. + * Null if entity does not support meta-fields. + */ private function getMetaFieldClass(): ?string { return null; } + /** + * This is the attribute/field name of the parent class within the meta-field class from getMetaFieldClass() + * Null if entity does not support meta-fields. + */ private function getMetaFieldName(): ?string { return null; } + /** + * This is the attribute/field name of meta-field class within the entity + * Null if entity does not support meta-fields. + */ + private function getEntityFieldName(): ?string + { + return 'meta'; + } + /** * @return array */ @@ -32,97 +54,16 @@ trait RepositorySearchTrait return []; } - private function supportsMetaFields(): bool - { - /* @phpstan-ignore-next-line */ - return $this->getMetaFieldClass() !== null && $this->getMetaFieldName() !== null; - } - private function addSearchTerm(QueryBuilder $qb, BaseQuery $query): void { - $searchTerm = $query->getSearchTerm(); + $configuration = new SearchConfiguration( + $this->getSearchableFields(), + $this->getMetaFieldClass(), + $this->getMetaFieldName() + ); + $configuration->setEntityFieldName($this->getEntityFieldName()); - if ($searchTerm === null) { - return; - } - - if (!$this->supportsMetaFields() && !$searchTerm->hasSearchTerm()) { - return; - } - - $aliases = $qb->getRootAliases(); - if (!isset($aliases[0])) { - throw new RepositoryException('No alias was set before invoking addSearchTerm().'); - } - $rootAlias = $aliases[0]; - - $searchAnd = $qb->expr()->andX(); - - if ($this->supportsMetaFields()) { - $i = 0; - $a = 0; - $c = 0; - foreach ($searchTerm->getSearchFields() as $metaName => $metaValue) { - $and = $qb->expr()->andX(); - /** @var non-falsy-string&lowercase-string $alias */ - $alias = 'meta' . $a++; - $paramName = 'metaName' . $i++; - $paramValue = 'metaValue' . $c++; - $subqueryName = 'metaNotExists' . $metaName; - - if ($metaValue === '*') { - $qb->leftJoin($rootAlias . '.meta', $alias); - $and->add($qb->expr()->eq($alias . '.name', ':' . $paramName)); - $qb->setParameter($paramName, $metaName); - $and->add($qb->expr()->isNotNull($alias . '.value')); - } elseif ($metaValue === '~') { - $and->add( - \sprintf('NOT EXISTS(SELECT %s FROM %s %s WHERE %s.%s = %s.id)', $subqueryName, $this->getMetaFieldClass(), $subqueryName, $subqueryName, $this->getMetaFieldName(), $rootAlias) - ); - } elseif ($metaValue === '' || $metaValue === null) { - $qb->leftJoin($rootAlias . '.meta', $alias); - $and->add( - $qb->expr()->orX( - $qb->expr()->andX( - $qb->expr()->eq($alias . '.name', ':' . $paramName), - $qb->expr()->isNull($alias . '.value') - ), - \sprintf('NOT EXISTS(SELECT %s FROM %s %s WHERE %s.%s = %s.id)', $subqueryName, $this->getMetaFieldClass(), $subqueryName, $subqueryName, $this->getMetaFieldName(), $rootAlias) - ) - ); - $qb->setParameter($paramName, $metaName); - } else { - $qb->leftJoin($rootAlias . '.meta', $alias); - $and->add($qb->expr()->eq($alias . '.name', ':' . $paramName)); - $and->add($qb->expr()->like($alias . '.value', ':' . $paramValue)); - $qb->setParameter($paramName, $metaName); - $qb->setParameter($paramValue, '%' . $metaValue . '%'); - } - - $searchAnd->add($and); - } - } - - $fields = $this->getSearchableFields(); - - if ($searchTerm->hasSearchTerm() && \count($fields) > 0) { - $or = $qb->expr()->orX(); - $i = 0; - foreach ($fields as $field) { - $param = 'searchTerm' . $i++; - if (stripos($field, '.') === false) { - $field = $rootAlias . '.' . $field; - } - $or->add( - $qb->expr()->like($field, ':' . $param), - ); - $qb->setParameter($param, '%' . $searchTerm->getSearchTerm() . '%'); - } - $searchAnd->add($or); - } - - if ($searchAnd->count() > 0) { - $qb->andWhere($searchAnd); - } + $helper = new SearchHelper($configuration); + $helper->addSearchTerm($qb, $query); } } diff --git a/src/Repository/Search/SearchConfiguration.php b/src/Repository/Search/SearchConfiguration.php new file mode 100644 index 00000000..f8c0b0d8 --- /dev/null +++ b/src/Repository/Search/SearchConfiguration.php @@ -0,0 +1,66 @@ + $searchableFields + */ + public function __construct( + private readonly array $searchableFields = [], + private readonly ?string $metaFieldClass = null, + private readonly ?string $metaFieldName = null, + ) + { + } + + /** + * The name of the meta-field class, e.g. ProjectMeta::class. + * Null if entity does not support meta-fields. + */ + public function getMetaFieldClass(): ?string + { + return $this->metaFieldClass; + } + + /** + * This is the attribute/field name of the parent class within the meta-field class from getMetaFieldClass() + * Null if entity does not support meta-fields. + */ + public function getMetaFieldName(): ?string + { + return $this->metaFieldName; + } + + public function setEntityFieldName(string $entityFieldName): void + { + $this->entityFieldName = $entityFieldName; + } + + /** + * This is the attribute/field name of meta-field class within the entity + * Null if entity does not support meta-fields. + */ + public function getEntityFieldName(): ?string + { + return $this->entityFieldName; + } + + /** + * @return array + */ + public function getSearchableFields(): array + { + return $this->searchableFields; + } +} diff --git a/src/Repository/Search/SearchConfigurationInterface.php b/src/Repository/Search/SearchConfigurationInterface.php new file mode 100644 index 00000000..0111bfc8 --- /dev/null +++ b/src/Repository/Search/SearchConfigurationInterface.php @@ -0,0 +1,36 @@ + + */ + public function getSearchableFields(): array; +} diff --git a/src/Repository/Search/SearchHelper.php b/src/Repository/Search/SearchHelper.php new file mode 100644 index 00000000..e8cb4369 --- /dev/null +++ b/src/Repository/Search/SearchHelper.php @@ -0,0 +1,147 @@ +configuration->getMetaFieldClass() !== null && $this->configuration->getMetaFieldName() !== null && $this->configuration->getEntityFieldName() !== null; + } + + public function addSearchTerm(QueryBuilder $qb, BaseQuery $query): void + { + $searchTerm = $query->getSearchTerm(); + + if ($searchTerm === null) { + return; + } + + if (!$this->supportsMetaFields() && !$searchTerm->hasSearchTerm()) { + return; + } + + $aliases = $qb->getRootAliases(); + if (!isset($aliases[0])) { + throw new RepositoryException('No alias was set before invoking addSearchTerm().'); + } + + $rootAlias = $aliases[0]; + $searchAnd = $qb->expr()->andX(); + + if ($this->supportsMetaFields()) { + $metaFieldRef = $rootAlias . '.' . $this->configuration->getEntityFieldName(); + $i = 0; + $c = 0; + $j = 0; + foreach ($searchTerm->getParts() as $part) { + // we do NOT search for unspecific/global terms as of now, because it is not clear if the user wants that + if (($metaName = $part->getField()) === null) { + continue; + } + $alias = 'meta' . $j++; + $qb->leftJoin($metaFieldRef, $alias); + $metaValue = $part->getTerm(); + $paramName = 'metaName' . $i++; + $paramValue = 'metaValue' . $c++; + $subqueryName = 'metaNotExists' . $metaName; + $field = $alias . '.value'; + + $and = $qb->expr()->andX(); + + if ($metaValue === '*') { + $and->add($qb->expr()->eq($alias . '.name', ':' . $paramName)); + $and->add($qb->expr()->isNotNull($field)); + } elseif ($metaValue === '~') { + $and->add( + \sprintf('NOT EXISTS(SELECT %s FROM %s %s WHERE %s.%s = %s.id AND %s.name = :%s)', $subqueryName, $this->configuration->getMetaFieldClass(), $subqueryName, $subqueryName, $this->configuration->getMetaFieldName(), $rootAlias, $subqueryName, $paramName) + ); + } elseif ($metaValue === '') { + $and->add( + $qb->expr()->orX( + $qb->expr()->andX( + $qb->expr()->eq($alias . '.name', ':' . $paramName), + $qb->expr()->isNull($field) + ), + \sprintf('NOT EXISTS(SELECT %s FROM %s %s WHERE %s.%s = %s.id AND %s.name = :%s)', $subqueryName, $this->configuration->getMetaFieldClass(), $subqueryName, $subqueryName, $this->configuration->getMetaFieldName(), $rootAlias, $subqueryName, $paramName) + ) + ); + } else { + $and->add($qb->expr()->eq($alias . '.name', ':' . $paramName)); + if (!$part->isExcluded()) { + $and->add($qb->expr()->like($field, ':' . $paramValue)); + } else { + $and->add( + $qb->expr()->orX()->addMultiple([ + $qb->expr()->isNull($field), + $qb->expr()->notLike($field, ':' . $paramValue), + ]) + ); + } + $qb->setParameter($paramValue, '%' . $metaValue . '%'); + } + + $qb->setParameter($paramName, $metaName); + + $searchAnd->add($and); + } + } + + if ($searchTerm->hasSearchTerm()) { + $i = 0; + $fields = $this->configuration->getSearchableFields(); + $and = $qb->expr()->andX(); + foreach ($searchTerm->getParts() as $part) { + // currently only meta fields have a name, so we do not use them here + if ($part->getField() !== null) { + continue; + } + $or = $qb->expr()->orX(); + foreach ($fields as $field) { + if (stripos($field, '.') === false) { + $field = $rootAlias . '.' . $field; + } + $param = 'searchTerm' . $i++; + if ($part->isExcluded()) { + $searchAnd->add( + $qb->expr()->orX()->addMultiple([ + $qb->expr()->isNull($field), + $qb->expr()->notLike($field, ':' . $param), + ]) + ); + } else { + $or->add( + $qb->expr()->like($field, ':' . $param), + ); + } + $qb->setParameter($param, '%' . $part->getTerm() . '%'); + } + if ($or->count() > 0) { + $and->add($or); + } + } + if ($and->count() > 0) { + $searchAnd->add($and); + } + } + + if ($searchAnd->count() > 0) { + $qb->andWhere($searchAnd); + } + } +} diff --git a/src/Repository/TagRepository.php b/src/Repository/TagRepository.php index 44d03f5b..e2b13f5b 100644 --- a/src/Repository/TagRepository.php +++ b/src/Repository/TagRepository.php @@ -14,6 +14,8 @@ use App\Entity\Timesheet; use App\Repository\Paginator\QueryPaginator; use App\Repository\Query\TagFormTypeQuery; use App\Repository\Query\TagQuery; +use App\Repository\Search\SearchConfiguration; +use App\Repository\Search\SearchHelper; use App\Utils\Pagination; use Doctrine\DBAL\ParameterType; use Doctrine\ORM\EntityRepository; @@ -155,22 +157,9 @@ class TagRepository extends EntityRepository $qb->addOrderBy($orderBy, $query->getOrder()); - $searchTerm = $query->getSearchTerm(); - if ($searchTerm !== null) { - $searchAnd = $qb->expr()->andX(); - - if ($searchTerm->hasSearchTerm()) { - $searchAnd->add( - $qb->expr()->orX( - $qb->expr()->like('tag.name', ':searchTerm') - ) - ); - $qb->setParameter('searchTerm', '%' . $searchTerm->getSearchTerm() . '%'); - } - - if ($searchAnd->count() > 0) { - $qb->andWhere($searchAnd); - } + if ($query->hasSearchTerm()) { + $helper = new SearchHelper(new SearchConfiguration(['tag.name'])); + $helper->addSearchTerm($qb, $query); } return $qb; diff --git a/src/Repository/TimesheetRepository.php b/src/Repository/TimesheetRepository.php index 28e2ed9a..c5b2bc67 100644 --- a/src/Repository/TimesheetRepository.php +++ b/src/Repository/TimesheetRepository.php @@ -26,6 +26,8 @@ use App\Repository\Paginator\PaginatorInterface; use App\Repository\Query\TimesheetQuery; use App\Repository\Query\TimesheetQueryHint; use App\Repository\Result\TimesheetResult; +use App\Repository\Search\SearchConfiguration; +use App\Repository\Search\SearchHelper; use App\Utils\Pagination; use DateInterval; use DateTime; @@ -43,8 +45,6 @@ use InvalidArgumentException; */ class TimesheetRepository extends EntityRepository { - use RepositorySearchTrait; - /** @deprecated since 2.0.35 */ public const STATS_QUERY_DURATION = 'duration'; /** @deprecated since 2.0.35 */ @@ -646,7 +646,13 @@ class TimesheetRepository extends EntityRepository $requiresTeams = $this->addPermissionCriteria($qb, $query->getCurrentUser(), $query->getTeams()); - $this->addSearchTerm($qb, $query); + $configuration = new SearchConfiguration( + ['t.description'], + TimesheetMeta::class, + 'timesheet' + ); + $helper = new SearchHelper($configuration); + $helper->addSearchTerm($qb, $query); if ($requiresCustomer || $requiresProject || $requiresTeams) { $qb->leftJoin('t.project', 'p'); @@ -667,24 +673,6 @@ class TimesheetRepository extends EntityRepository return $qb; } - private function getMetaFieldClass(): string - { - return TimesheetMeta::class; - } - - private function getMetaFieldName(): string - { - return 'timesheet'; - } - - /** - * @return array - */ - private function getSearchableFields(): array - { - return ['t.description']; - } - /** * @return Timesheet[] */ diff --git a/src/Repository/UserRepository.php b/src/Repository/UserRepository.php index 189e5e71..c55110b3 100644 --- a/src/Repository/UserRepository.php +++ b/src/Repository/UserRepository.php @@ -20,6 +20,8 @@ use App\Repository\Paginator\PaginatorInterface; use App\Repository\Query\UserFormTypeQuery; use App\Repository\Query\UserQuery; use App\Repository\Query\VisibilityInterface; +use App\Repository\Search\SearchConfiguration; +use App\Repository\Search\SearchHelper; use App\Utils\Pagination; use Doctrine\DBAL\ParameterType; use Doctrine\DBAL\Types\Types; @@ -323,39 +325,14 @@ class UserRepository extends EntityRepository implements UserLoaderInterface, Us $qb->setParameter('system', $query->getSystemAccount(), Types::BOOLEAN); } - $searchTerm = $query->getSearchTerm(); - if ($searchTerm !== null) { - $searchAnd = $qb->expr()->andX(); - - foreach ($searchTerm->getSearchFields() as $metaName => $metaValue) { - $qb->leftJoin('u.preferences', 'meta'); - $searchAnd->add( - $qb->expr()->andX( - $qb->expr()->eq('meta.name', ':metaName'), - $qb->expr()->like('meta.value', ':metaValue') - ) - ); - $qb->setParameter('metaName', $metaName); - $qb->setParameter('metaValue', '%' . $metaValue . '%'); - } - - if ($searchTerm->hasSearchTerm()) { - $searchAnd->add( - $qb->expr()->orX( - $qb->expr()->like('u.alias', ':searchTerm'), - $qb->expr()->like('u.title', ':searchTerm'), - $qb->expr()->like('u.accountNumber', ':searchTerm'), - $qb->expr()->like('u.email', ':searchTerm'), - $qb->expr()->like('u.username', ':searchTerm') - ) - ); - $qb->setParameter('searchTerm', '%' . $searchTerm->getSearchTerm() . '%'); - } - - if ($searchAnd->count() > 0) { - $qb->andWhere($searchAnd); - } - } + $configuration = new SearchConfiguration( + ['u.alias', 'u.title', 'u.accountNumber', 'u.email', 'u.username'], + UserPreference::class, + 'user' + ); + $configuration->setEntityFieldName('preferences'); + $helper = new SearchHelper($configuration); + $helper->addSearchTerm($qb, $query); return $qb; } diff --git a/src/Utils/SearchTerm.php b/src/Utils/SearchTerm.php index 3348f95c..87fafb23 100644 --- a/src/Utils/SearchTerm.php +++ b/src/Utils/SearchTerm.php @@ -14,56 +14,92 @@ final class SearchTerm private string $originalTerm; private string $term; /** - * @var string[] + * @var SearchTermPart[] */ - private array $fields; + private array $parts = []; public function __construct(string $searchTerm) { $this->originalTerm = $searchTerm; $terms = explode(' ', $searchTerm); - $fields = []; $finalTerm = []; foreach ($terms as $term) { - $tmp = explode(':', $term); - if (\count($tmp) === 2) { - $fields[$tmp[0]] = $tmp[1]; - } else { - $finalTerm[] = $term; + $part = new SearchTermPart($term); + if ($part->getField() === null) { + $finalTerm[] = $part->getTerm(); } + $this->parts[] = $part; } $this->term = implode(' ', $finalTerm); - $this->fields = $fields; } + /** + * @deprecated since 2.34.0 + */ public function hasSearchField(string $name): bool { - return \array_key_exists($name, $this->fields); - } + @trigger_error('The SearchTerm::hasSearchField() method is deprecated and will be removed with 3.0', E_USER_DEPRECATED); - public function getSearchField(string $name): ?string - { - if (!$this->hasSearchField($name)) { - return null; + foreach ($this->parts as $part) { + if ($part->getField() === $name) { + return true; + } } - return $this->fields[$name]; + return false; } + /** + * @deprecated since 2.34.0 + */ + public function getSearchField(string $name): ?string + { + @trigger_error('The SearchTerm::getSearchField() method is deprecated and will be removed with 3.0', E_USER_DEPRECATED); + + foreach ($this->parts as $part) { + if ($part->getField() === $name) { + return $part->getTerm(); + } + } + + return null; + } + + /** + * @return array + */ public function getSearchFields(): array { - return $this->fields; + // TODO deprecated 3.0 - all places that use this method should use the RepositorySearchTrait instead (soft deprecation for plugins) + $fields = []; + foreach ($this->parts as $part) { + if (($field = $part->getField()) !== null) { + $fields[$field] = $part->getTerm(); + } + } + + return $fields; + } + + /** + * @return SearchTermPart[] + */ + public function getParts(): array + { + return $this->parts; } public function getSearchTerm(): string { + // TODO deprecated 3.0 - all places that use this method should use the RepositorySearchTrait instead (soft deprecation for plugins) return $this->term; } public function hasSearchTerm(): bool { + // TODO refactor and use the parts and check if any part has an emoty field name return $this->term !== ''; } diff --git a/src/Utils/SearchTermPart.php b/src/Utils/SearchTermPart.php new file mode 100644 index 00000000..d5b4558d --- /dev/null +++ b/src/Utils/SearchTermPart.php @@ -0,0 +1,51 @@ +field = $tmp[0]; + $term = $tmp[1] !== '""' ? $tmp[1] : ''; + } + } + + if (\strlen($term) > 1 && $term[0] === '!') { + $term = substr($term, 1); + $this->excluded = true; + } + + $this->term = $term; + } + + public function getTerm(): string + { + return $this->term; + } + + public function getField(): ?string + { + return $this->field; + } + + public function isExcluded(): bool + { + return $this->excluded; + } +} diff --git a/tests/Controller/ProjectControllerTest.php b/tests/Controller/ProjectControllerTest.php index 604daacb..13b8302b 100644 --- a/tests/Controller/ProjectControllerTest.php +++ b/tests/Controller/ProjectControllerTest.php @@ -72,9 +72,27 @@ class ProjectControllerTest extends AbstractControllerBaseTestCase $fixture = new ProjectFixtures(); $fixture->setAmount(5); - $fixture->setCallback(function (Project $project) { + $i = 0; + $fixture->setCallback(function (Project $project) use (&$i) { $project->setVisible(true); - $project->setComment('I am a foobar with tralalalala some more content'); + switch ($i++) { + case 0: + $project->setComment('I am a foo'); + break; + case 1: + $project->setComment('I am a foo with tralalalala some more content'); + break; + case 2: + $project->setComment('I am a barfoo with tralalalala some more content'); + break; + case 3: + $project->setName($project->getName() . ' with'); + $project->setComment('I am a foobar tralalalala some more content'); + break; + default: + $project->setComment('I am a foobar with tralalalala some more content'); + break; + } $project->setMetaField((new ProjectMeta())->setName('location')->setValue('homeoffice')); $project->setMetaField((new ProjectMeta())->setName('feature')->setValue('timetracking')); }); @@ -89,7 +107,7 @@ class ProjectControllerTest extends AbstractControllerBaseTestCase $form = $client->getCrawler()->filter('form.searchform')->form(); $client->submit($form, [ - 'searchTerm' => 'feature:timetracking foo', + 'searchTerm' => 'feature:timetracking foo with', 'visibility' => 1, 'customers' => [1], 'size' => 50, @@ -98,7 +116,7 @@ class ProjectControllerTest extends AbstractControllerBaseTestCase self::assertTrue($client->getResponse()->isSuccessful()); $this->assertHasDataTable($client); - $this->assertDataTableRowCount($client, 'datatable_project_admin', 5); + $this->assertDataTableRowCount($client, 'datatable_project_admin', 4); } public function testExportIsSecureForRole(): void diff --git a/tests/Form/DataTransformer/SearchTermTransformerTest.php b/tests/Form/DataTransformer/SearchTermTransformerTest.php index 7473c0c1..4d38c35a 100644 --- a/tests/Form/DataTransformer/SearchTermTransformerTest.php +++ b/tests/Form/DataTransformer/SearchTermTransformerTest.php @@ -47,7 +47,18 @@ class SearchTermTransformerTest extends TestCase self::assertEquals('hello world:xxxxx foo bar test:1234', $term->getOriginalSearch()); self::assertEquals('hello foo bar', $term->getSearchTerm()); self::assertEquals(['world' => 'xxxxx', 'test' => '1234'], $term->getSearchFields()); - self::assertEquals('xxxxx', $term->getSearchField('world')); - self::assertEquals('1234', $term->getSearchField('test')); + $expectedParts = [ + ['hello', null], + ['xxxxx', 'world'], + ['foo', null], + ['bar', null], + ['1234', 'test'], + ]; + $i = 0; + foreach ($term->getParts() as $part) { + $expected = $expectedParts[$i++]; + self::assertEquals($expected[0], $part->getTerm()); + self::assertEquals($expected[1], $part->getField()); + } } } diff --git a/tests/Repository/Search/SearchConfigurationTest.php b/tests/Repository/Search/SearchConfigurationTest.php new file mode 100644 index 00000000..730cfd3e --- /dev/null +++ b/tests/Repository/Search/SearchConfigurationTest.php @@ -0,0 +1,40 @@ +getMetaFieldClass()); + self::assertNull($sut->getMetaFieldName()); + self::assertEquals('meta', $sut->getEntityFieldName()); + self::assertEquals([], $sut->getSearchableFields()); + } + + public function testConstruct(): void + { + $fields = ['field1', 'field2']; + $sut = new SearchConfiguration($fields, 'SomeClassName', 'foo-bar'); + self::assertEquals($fields, $sut->getSearchableFields()); + self::assertEquals('SomeClassName', $sut->getMetaFieldClass()); + self::assertEquals('foo-bar', $sut->getMetaFieldName()); + + $sut->setEntityFieldName('customField'); + self::assertEquals('customField', $sut->getEntityFieldName()); + } +} diff --git a/tests/Repository/Search/SearchHelperTest.php b/tests/Repository/Search/SearchHelperTest.php new file mode 100644 index 00000000..92dd8d97 --- /dev/null +++ b/tests/Repository/Search/SearchHelperTest.php @@ -0,0 +1,199 @@ +createMock(QueryBuilder::class); + $qb->expects(self::never())->method('andWhere'); + + $query = new BaseQuery(); + + $configuration = new SearchConfiguration(); + $sut = new SearchHelper($configuration); + + $sut->addSearchTerm($qb, $query); + } + + public function testNoAliasThrowsException(): void + { + $qb = $this->createMock(QueryBuilder::class); + $qb->method('getRootAliases')->willReturn([]); + + $query = new BaseQuery(); + $query->setSearchTerm(new SearchTerm('foo')); + $configuration = new SearchConfiguration(); + $sut = new SearchHelper($configuration); + + $this->expectException(RepositoryException::class); + $this->expectExceptionMessage('No alias was set before invoking addSearchTerm().'); + + $sut->addSearchTerm($qb, $query); + } + + public function testSupportsMetaFieldsAddsMetaFieldConditions(): void + { + $em = $this->createMock(EntityManagerInterface::class); + $em->method('getExpressionBuilder')->willReturn(new Expr()); + $qb = new QueryBuilder($em); + $qb->from(Timesheet::class, 'testFoo'); + + $query = new BaseQuery(); + $query->setSearchTerm(new SearchTerm('metaField:value !foo test')); + $configuration = new SearchConfiguration(['bar', 'tmp'], 'MetaFieldClass', 'metaFieldName'); + $configuration->setEntityFieldName('entityFieldName'); + + $sut = new SearchHelper($configuration); + + $sut->addSearchTerm($qb, $query); + $parts = $qb->getDQLParts(); + + self::assertCount(9, $parts); + self::assertArrayHasKey('join', $parts); + self::assertIsArray($parts['join']); + self::assertCount(1, $parts['join']); + self::assertArrayHasKey('testFoo', $parts['join']); + self::assertArrayHasKey('where', $parts); + self::assertInstanceOf(Expr\Andx::class, $parts['where']); + $whereParts = $parts['where']; + self::assertEquals(1, $whereParts->count()); + + $whereAnd = $whereParts->getParts()[0]; + self::assertInstanceOf(Expr\Andx::class, $whereAnd); + self::assertCount(4, $whereAnd->getParts()); + + // meta fields + $where = $whereAnd->getParts()[0]; + self::assertInstanceOf(Expr\Andx::class, $where); + $compareParts = $where->getParts(); + self::assertCount(2, $compareParts); + + self::assertInstanceOf(Expr\Comparison::class, $compareParts[0]); + self::assertEquals('meta0.name', $compareParts[0]->getLeftExpr()); + self::assertEquals('=', $compareParts[0]->getOperator()); + self::assertEquals(':metaName0', $compareParts[0]->getRightExpr()); + + self::assertInstanceOf(Expr\Comparison::class, $compareParts[1]); + self::assertEquals('meta0.value', $compareParts[1]->getLeftExpr()); + self::assertEquals('LIKE', $compareParts[1]->getOperator()); + self::assertEquals(':metaValue0', $compareParts[1]->getRightExpr()); + + // negated search terms + $where = $whereAnd->getParts()[1]; + self::assertInstanceOf(Expr\Orx::class, $where); + $compareParts = $where->getParts(); + self::assertCount(2, $compareParts); + + self::assertEquals('testFoo.bar IS NULL', $compareParts[0]); + + self::assertInstanceOf(Expr\Comparison::class, $compareParts[1]); + self::assertEquals('testFoo.bar', $compareParts[1]->getLeftExpr()); + self::assertEquals('NOT LIKE', $compareParts[1]->getOperator()); + self::assertEquals(':searchTerm0', $compareParts[1]->getRightExpr()); + + $where = $whereAnd->getParts()[2]; + self::assertInstanceOf(Expr\Orx::class, $where); + $compareParts = $where->getParts(); + self::assertCount(2, $compareParts); + + self::assertEquals('testFoo.tmp IS NULL', $compareParts[0]); + + self::assertInstanceOf(Expr\Comparison::class, $compareParts[1]); + self::assertEquals('testFoo.tmp', $compareParts[1]->getLeftExpr()); + self::assertEquals('NOT LIKE', $compareParts[1]->getOperator()); + self::assertEquals(':searchTerm1', $compareParts[1]->getRightExpr()); + + // regular search terms + $where = $whereAnd->getParts()[3]; + self::assertInstanceOf(Expr\Andx::class, $where); + $compareParts = $where->getParts(); + self::assertCount(1, $compareParts); + self::assertInstanceOf(Expr\Orx::class, $compareParts[0]); + $orParts = $compareParts[0]->getParts(); + self::assertCount(2, $orParts); + + self::assertInstanceOf(Expr\Comparison::class, $orParts[0]); + self::assertEquals('testFoo.bar', $orParts[0]->getLeftExpr()); + self::assertEquals('LIKE', $orParts[0]->getOperator()); + self::assertEquals(':searchTerm2', $orParts[0]->getRightExpr()); + + self::assertInstanceOf(Expr\Comparison::class, $orParts[1]); + self::assertEquals('testFoo.tmp', $orParts[1]->getLeftExpr()); + self::assertEquals('LIKE', $orParts[1]->getOperator()); + self::assertEquals(':searchTerm3', $orParts[1]->getRightExpr()); + + /** @var array $parameters */ + $parameters = $qb->getParameters(); + self::assertCount(6, $parameters); + + self::assertInstanceOf(Parameter::class, $parameters[0]); + self::assertEquals('metaValue0', $parameters[0]->getName()); + self::assertEquals('%value%', $parameters[0]->getValue()); + self::assertEquals(2, $parameters[0]->getType()); + + self::assertInstanceOf(Parameter::class, $parameters[1]); + self::assertEquals('metaName0', $parameters[1]->getName()); + self::assertEquals('metaField', $parameters[1]->getValue()); + self::assertEquals(2, $parameters[1]->getType()); + + self::assertInstanceOf(Parameter::class, $parameters[2]); + self::assertEquals('searchTerm0', $parameters[2]->getName()); + self::assertEquals('%foo%', $parameters[2]->getValue()); + self::assertEquals(2, $parameters[2]->getType()); + + self::assertInstanceOf(Parameter::class, $parameters[3]); + self::assertEquals('searchTerm1', $parameters[3]->getName()); + self::assertEquals('%foo%', $parameters[3]->getValue()); + self::assertEquals(2, $parameters[3]->getType()); + + self::assertInstanceOf(Parameter::class, $parameters[4]); + self::assertEquals('searchTerm2', $parameters[4]->getName()); + self::assertEquals('%test%', $parameters[4]->getValue()); + self::assertEquals(2, $parameters[4]->getType()); + + self::assertInstanceOf(Parameter::class, $parameters[5]); + self::assertEquals('searchTerm3', $parameters[5]->getName()); + self::assertEquals('%test%', $parameters[5]->getValue()); + self::assertEquals(2, $parameters[5]->getType()); + } + + public function testSearchTermWithExcludedFieldAddsExclusionCondition(): void + { + $qb = $this->createMock(QueryBuilder::class); + $qb->method('expr')->willReturn(new Expr()); + $qb->method('getRootAliases')->willReturn(['root']); + $qb->expects(self::atLeastOnce())->method('andWhere'); + + $query = new BaseQuery(); + $query->setSearchTerm(new SearchTerm('!value')); + $configuration = new SearchConfiguration(['field1', 'field2']); + + $sut = new SearchHelper($configuration); + + $sut->addSearchTerm($qb, $query); + } +} diff --git a/tests/Utils/SearchTermPartTest.php b/tests/Utils/SearchTermPartTest.php new file mode 100644 index 00000000..156e2a68 --- /dev/null +++ b/tests/Utils/SearchTermPartTest.php @@ -0,0 +1,43 @@ +getTerm()); + self::assertEquals($field, $sut->getField()); + self::assertEquals($excluded, $sut->isExcluded()); + } +} diff --git a/tests/Utils/SearchTermTest.php b/tests/Utils/SearchTermTest.php index 511d9d15..d08424c8 100644 --- a/tests/Utils/SearchTermTest.php +++ b/tests/Utils/SearchTermTest.php @@ -22,11 +22,22 @@ class SearchTermTest extends TestCase $sut = new SearchTerm('foo bar test 1'); self::assertEquals('foo bar test 1', $sut->getSearchTerm()); self::assertEmpty($sut->getSearchFields()); - self::assertFalse($sut->hasSearchField('foo')); self::assertTrue($sut->hasSearchTerm()); - self::assertNull($sut->getSearchField('foo')); self::assertEquals('foo bar test 1', $sut->getOriginalSearch()); self::assertEquals('foo bar test 1', (string) $sut); + $expectedParts = [ + ['foo', null, false], + ['bar', null, false], + ['test', null, false], + ['1', null, false], + ]; + $i = 0; + foreach ($sut->getParts() as $part) { + $expected = $expectedParts[$i++]; + self::assertEquals($expected[0], $part->getTerm()); + self::assertEquals($expected[1], $part->getField()); + self::assertEquals($expected[2], $part->isExcluded()); + } } public function testWithMetaField(): void @@ -35,37 +46,130 @@ class SearchTermTest extends TestCase self::assertFalse($sut->hasSearchTerm()); self::assertEquals('', $sut->getSearchTerm()); self::assertNotEmpty($sut->getSearchFields()); - self::assertTrue($sut->hasSearchField('foo')); - self::assertEquals('bar', $sut->getSearchField('foo')); self::assertEquals(['foo' => 'bar'], $sut->getSearchFields()); self::assertEquals('foo:bar', $sut->getOriginalSearch()); + self::assertCount(1, $sut->getParts()); + $expectedParts = [ + ['foo', 'bar', false], + ]; + $i = 0; + foreach ($sut->getParts() as $part) { + $expected = $expectedParts[$i++]; + self::assertEquals($expected[0], $part->getField()); + self::assertEquals($expected[1], $part->getTerm()); + self::assertEquals($expected[2], $part->isExcluded()); + } } + /** + * @group legacy + */ public function testWithMultipleMetaFields(): void { $sut = new SearchTerm('foo:bar bar:foo'); self::assertFalse($sut->hasSearchTerm()); self::assertEquals('', $sut->getSearchTerm()); self::assertNotEmpty($sut->getSearchFields()); - self::assertTrue($sut->hasSearchField('foo')); - self::assertTrue($sut->hasSearchField('bar')); - self::assertEquals('bar', $sut->getSearchField('foo')); - self::assertEquals('foo', $sut->getSearchField('bar')); + self::assertFalse($sut->hasSearchField('test')); // @phpstan-ignore method.deprecated + self::assertTrue($sut->hasSearchField('foo')); // @phpstan-ignore method.deprecated + self::assertTrue($sut->hasSearchField('bar')); // @phpstan-ignore method.deprecated + self::assertEquals('bar', $sut->getSearchField('foo')); // @phpstan-ignore method.deprecated + self::assertEquals('foo', $sut->getSearchField('bar')); // @phpstan-ignore method.deprecated self::assertEquals(['foo' => 'bar', 'bar' => 'foo'], $sut->getSearchFields()); self::assertEquals('foo:bar bar:foo', $sut->getOriginalSearch()); + self::assertCount(2, $sut->getParts()); } public function testComplexWithMultipleAndDuplicateMetaFields(): void { - $sut = new SearchTerm('foo:bar hello bar:foo world test foo:bar wuff'); + $sut = new SearchTerm('foo:bar hello bar:!foo world test foo:bar2 wuff'); self::assertTrue($sut->hasSearchTerm()); self::assertEquals('hello world test wuff', $sut->getSearchTerm()); self::assertNotEmpty($sut->getSearchFields()); - self::assertTrue($sut->hasSearchField('foo')); - self::assertTrue($sut->hasSearchField('bar')); - self::assertEquals('bar', $sut->getSearchField('foo')); - self::assertEquals('foo', $sut->getSearchField('bar')); - self::assertEquals(['foo' => 'bar', 'bar' => 'foo'], $sut->getSearchFields()); - self::assertEquals('foo:bar hello bar:foo world test foo:bar wuff', $sut->getOriginalSearch()); + self::assertEquals(['foo' => 'bar2', 'bar' => 'foo'], $sut->getSearchFields()); + self::assertEquals('foo:bar hello bar:!foo world test foo:bar2 wuff', $sut->getOriginalSearch()); + self::assertCount(7, $sut->getParts()); + $expectedParts = [ + ['bar', 'foo', false], + ['hello', null, false], + ['foo', 'bar', true], + ['world', null, false], + ['test', null, false], + ['bar2', 'foo', false], + ['wuff', null, false], + ]; + $i = 0; + foreach ($sut->getParts() as $part) { + $expected = $expectedParts[$i++]; + self::assertEquals($expected[0], $part->getTerm()); + self::assertEquals($expected[1], $part->getField()); + self::assertEquals($expected[2], $part->isExcluded()); + } + } + + public function testIssue5221(): void + { + $sut = new SearchTerm('ABC-123: abcd: abcd'); + self::assertTrue($sut->hasSearchTerm()); + self::assertEquals('ABC-123: abcd: abcd', $sut->getSearchTerm()); + self::assertEmpty($sut->getSearchFields()); + self::assertEquals([], $sut->getSearchFields()); + self::assertEquals('ABC-123: abcd: abcd', $sut->getOriginalSearch()); + self::assertCount(3, $sut->getParts()); + $expectedParts = [ + ['ABC-123:', null, false], + ['abcd:', null, false], + ['abcd', null, false], + ]; + $i = 0; + foreach ($sut->getParts() as $part) { + $expected = $expectedParts[$i++]; + self::assertEquals($expected[0], $part->getTerm()); + self::assertEquals($expected[1], $part->getField()); + self::assertEquals($expected[2], $part->isExcluded()); + } + + $sut = new SearchTerm('1 : 1:.'); + self::assertTrue($sut->hasSearchTerm()); + self::assertEquals('1 :', $sut->getSearchTerm()); + self::assertNotEmpty($sut->getSearchFields()); + self::assertSame([1 => '.'], $sut->getSearchFields()); // this is weird, should be a string, but PHP seems to think otherwise + self::assertEquals('1 : 1:.', $sut->getOriginalSearch()); + self::assertCount(3, $sut->getParts()); + $expectedParts = [ + ['1', null, false], + [':', null, false], + ['.', '1', false], + ]; + $i = 0; + foreach ($sut->getParts() as $part) { + $expected = $expectedParts[$i++]; + self::assertEquals($expected[0], $part->getTerm()); + self::assertEquals($expected[1], $part->getField()); + self::assertEquals($expected[2], $part->isExcluded()); + } + } + + public function testEmptySearchTerm(): void + { + $sut = new SearchTerm('ABC-123:"" abcd:"" abcd'); + self::assertTrue($sut->hasSearchTerm()); + self::assertEquals('abcd', $sut->getSearchTerm()); + self::assertNotEmpty($sut->getSearchFields()); + self::assertEquals(['ABC-123' => '', 'abcd' => ''], $sut->getSearchFields()); + self::assertEquals('ABC-123:"" abcd:"" abcd', $sut->getOriginalSearch()); + self::assertCount(3, $sut->getParts()); + $expectedParts = [ + ['', 'ABC-123', false], + ['', 'abcd', false], + ['abcd', null, false], + ]; + $i = 0; + foreach ($sut->getParts() as $part) { + $expected = $expectedParts[$i++]; + self::assertEquals($expected[0], $part->getTerm()); + self::assertEquals($expected[1], $part->getField()); + self::assertEquals($expected[2], $part->isExcluded()); + } } }