improved search with negation (#5453)

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

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());
+        }
     }
 }

tests/Repository/Search/SearchConfigurationTest.php

@@ -0,0 +1,40 @@
+<?php
+
+/*
+ * This file is part of the Kimai time-tracking app.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace App\Tests\Repository\Search;
+
+use App\Repository\Search\SearchConfiguration;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @covers \App\Repository\Search\SearchConfiguration
+ */
+class SearchConfigurationTest extends TestCase
+{
+    public function testDefaultConstruct(): void
+    {
+        $sut = new SearchConfiguration();
+        self::assertNull($sut->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());
+    }
+}

tests/Repository/Search/SearchHelperTest.php

@@ -0,0 +1,199 @@
+<?php
+
+/*
+ * This file is part of the Kimai time-tracking app.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace App\Tests\Repository\Search;
+
+use App\Entity\Timesheet;
+use App\Repository\Query\BaseQuery;
+use App\Repository\RepositoryException;
+use App\Repository\Search\SearchConfiguration;
+use App\Repository\Search\SearchHelper;
+use App\Utils\SearchTerm;
+use Doctrine\ORM\EntityManagerInterface;
+use Doctrine\ORM\Query\Expr;
+use Doctrine\ORM\Query\Parameter;
+use Doctrine\ORM\QueryBuilder;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @covers \App\Repository\Search\SearchHelper
+ */
+class SearchHelperTest extends TestCase
+{
+    public function testSearchTermIsNullDoesNotModifyQueryBuilder(): void
+    {
+        $qb = $this->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<Parameter> $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);
+    }
+}

tests/Utils/SearchTermPartTest.php

@@ -0,0 +1,43 @@
+<?php
+
+/*
+ * This file is part of the Kimai time-tracking app.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace App\Tests\Utils;
+
+use App\Utils\SearchTermPart;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @covers \App\Utils\SearchTermPart
+ */
+class SearchTermPartTest extends TestCase
+{
+    public static function getTestData(): array
+    {
+        return [
+            ['foo bar test 1', 'foo bar test 1', null, false],
+            ['foo:bar test 1', 'bar test 1', 'foo', false],
+            ['foo:!bar test 1', 'bar test 1', 'foo', true],
+            ['!bar', 'bar', null, true],
+            ['bar', 'bar', null, false],
+            ['hello:world', 'world', 'hello', false],
+            ['hello:!world', 'world', 'hello', true],
+        ];
+    }
+
+    /**
+     * @dataProvider getTestData
+     */
+    public function testSearchTerm(string $term, string $expected, ?string $field, bool $excluded): void
+    {
+        $sut = new SearchTermPart($term);
+        self::assertEquals($expected, $sut->getTerm());
+        self::assertEquals($field, $sut->getField());
+        self::assertEquals($excluded, $sut->isExcluded());
+    }
+}

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());
+        }
     }
 }