From 84560a6f44746b89bbb4ae3e9791497d0ab85564 Mon Sep 17 00:00:00 2001 From: Sobuno Date: Tue, 31 Dec 2024 10:16:27 +0100 Subject: [PATCH 01/27] WIP --- app/Providers/SearchServiceProvider.php | 11 + app/Support/Search/GdbotsQueryParser.php | 80 +++++++ app/Support/Search/OperatorQuerySearch.php | 149 ++++++------ app/Support/Search/QueryParser.php | 171 ++++++++++++++ app/Support/Search/QueryParserInterface.php | 112 +++++++++ ...ractQueryParserInterfaceParseQueryTest.php | 213 ++++++++++++++++++ .../GdbotsQueryParserParseQueryTest.php | 24 ++ .../Search/QueryParserParseQueryTest.php | 25 ++ 8 files changed, 699 insertions(+), 86 deletions(-) create mode 100644 app/Support/Search/GdbotsQueryParser.php create mode 100644 app/Support/Search/QueryParser.php create mode 100644 app/Support/Search/QueryParserInterface.php create mode 100644 tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php create mode 100644 tests/unit/Support/Search/GdbotsQueryParserParseQueryTest.php create mode 100644 tests/unit/Support/Search/QueryParserParseQueryTest.php diff --git a/app/Providers/SearchServiceProvider.php b/app/Providers/SearchServiceProvider.php index 0bb2b1056b..627d9f0820 100644 --- a/app/Providers/SearchServiceProvider.php +++ b/app/Providers/SearchServiceProvider.php @@ -24,6 +24,8 @@ declare(strict_types=1); namespace FireflyIII\Providers; use FireflyIII\Support\Search\OperatorQuerySearch; +use FireflyIII\Support\Search\QueryParser; +use FireflyIII\Support\Search\QueryParserInterface; use FireflyIII\Support\Search\SearchInterface; use Illuminate\Foundation\Application; use Illuminate\Support\ServiceProvider; @@ -43,6 +45,15 @@ class SearchServiceProvider extends ServiceProvider */ public function register(): void { + $this->app->bind( + QueryParserInterface::class, + static function (Application $app) { + /** @var QueryParser $queryParser */ + $queryParser = app(QueryParser::class); + return $queryParser; + } + ); + $this->app->bind( SearchInterface::class, static function (Application $app) { diff --git a/app/Support/Search/GdbotsQueryParser.php b/app/Support/Search/GdbotsQueryParser.php new file mode 100644 index 0000000000..c8249c455a --- /dev/null +++ b/app/Support/Search/GdbotsQueryParser.php @@ -0,0 +1,80 @@ +parser = new BaseQueryParser(); + } + + /** + * @return Node[] + * @throws FireflyException + */ + public function parse(string $query): array + { + try { + $result = $this->parser->parse($query); + return array_map( + fn(GdbotsNode\Node $node) => $this->convertNode($node), + $result->getNodes() + ); + } catch (\LogicException|\TypeError $e) { + fwrite(STDERR, "Setting up GdbotsQueryParserTest\n"); + dd('Creating GdbotsQueryParser'); + app('log')->error($e->getMessage()); + app('log')->error(sprintf('Could not parse search: "%s".', $query)); + + throw new FireflyException(sprintf('Invalid search value "%s". See the logs.', e($query)), 0, $e); + } + } + + private function convertNode(GdbotsNode\Node $node): Node + { + switch (true) { + case $node instanceof GdbotsNode\Word: + return new Word($node->getValue()); + + case $node instanceof GdbotsNode\Field: + return new Field( + $node->getValue(), + (string) $node->getNode()->getValue(), + BoolOperator::PROHIBITED === $node->getBoolOperator() + ); + + case $node instanceof GdbotsNode\Subquery: + return new Subquery( + array_map( + fn(GdbotsNode\Node $subNode) => $this->convertNode($subNode), + $node->getNodes() + ) + ); + + case $node instanceof GdbotsNode\Phrase: + case $node instanceof GdbotsNode\Numbr: + case $node instanceof GdbotsNode\Date: + case $node instanceof GdbotsNode\Url: + case $node instanceof GdbotsNode\Hashtag: + case $node instanceof GdbotsNode\Mention: + case $node instanceof GdbotsNode\Emoticon: + case $node instanceof GdbotsNode\Emoji: + return new Word((string) $node->getValue()); + + default: + throw new FireflyException( + sprintf('Unsupported node type: %s', get_class($node)) + ); + } + } +} diff --git a/app/Support/Search/OperatorQuerySearch.php b/app/Support/Search/OperatorQuerySearch.php index 8683d4e289..ca989bd911 100644 --- a/app/Support/Search/OperatorQuerySearch.php +++ b/app/Support/Search/OperatorQuerySearch.php @@ -41,20 +41,6 @@ use FireflyIII\Repositories\Tag\TagRepositoryInterface; use FireflyIII\Repositories\UserGroups\Currency\CurrencyRepositoryInterface; use FireflyIII\Support\ParseDateString; use FireflyIII\User; -use Gdbots\QueryParser\Enum\BoolOperator; -use Gdbots\QueryParser\Node\Date; -use Gdbots\QueryParser\Node\Emoji; -use Gdbots\QueryParser\Node\Emoticon; -use Gdbots\QueryParser\Node\Field; -use Gdbots\QueryParser\Node\Hashtag; -use Gdbots\QueryParser\Node\Mention; -use Gdbots\QueryParser\Node\Node; -use Gdbots\QueryParser\Node\Numbr; -use Gdbots\QueryParser\Node\Phrase; -use Gdbots\QueryParser\Node\Subquery; -use Gdbots\QueryParser\Node\Url; -use Gdbots\QueryParser\Node\Word; -use Gdbots\QueryParser\QueryParser; use Illuminate\Pagination\LengthAwarePaginator; use Illuminate\Support\Collection; @@ -145,10 +131,11 @@ class OperatorQuerySearch implements SearchInterface public function parseQuery(string $query): void { app('log')->debug(sprintf('Now in parseQuery(%s)', $query)); - $parser = new QueryParser(); + $parser = app(QueryParserInterface::class); + app('log')->debug(sprintf('Using %s as implementation for QueryParserInterface', get_class($parser))); try { - $query1 = $parser->parse($query); + $nodes = $parser->parse($query); } catch (\LogicException|\TypeError $e) { app('log')->error($e->getMessage()); app('log')->error(sprintf('Could not parse search: "%s".', $query)); @@ -156,9 +143,9 @@ class OperatorQuerySearch implements SearchInterface throw new FireflyException(sprintf('Invalid search value "%s". See the logs.', e($query)), 0, $e); } - app('log')->debug(sprintf('Found %d node(s)', count($query1->getNodes()))); - foreach ($query1->getNodes() as $searchNode) { - $this->handleSearchNode($searchNode); + app('log')->debug(sprintf('Found %d node(s)', count($nodes))); + foreach ($nodes as $node) { + $this->handleSearchNode($node); } // add missing information @@ -173,81 +160,71 @@ class OperatorQuerySearch implements SearchInterface * * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ - private function handleSearchNode(Node $searchNode): void + private function handleSearchNode(Node $node): void { - $class = get_class($searchNode); - app('log')->debug(sprintf('Now in handleSearchNode(%s)', $class)); + app('log')->debug(sprintf('Now in handleSearchNode(%s)', get_class($node))); - switch ($class) { - default: - app('log')->error(sprintf('Cannot handle node %s', $class)); - - throw new FireflyException(sprintf('Firefly III search can\'t handle "%s"-nodes', $class)); - - case Subquery::class: - // loop all notes in subquery: - foreach ($searchNode->getNodes() as $subNode) { // @phpstan-ignore-line PHPStan thinks getNodes() does not exist but it does. - $this->handleSearchNode($subNode); // let's hope it's not too recursive - } - - break; - - case Word::class: - case Phrase::class: - case Numbr::class: - case Url::class: - case Date::class: - case Hashtag::class: - case Emoticon::class: - case Emoji::class: - case Mention::class: - $allWords = (string) $searchNode->getValue(); - app('log')->debug(sprintf('Add words "%s" to search string, because Node class is "%s"', $allWords, $class)); + switch (true) { + case $node instanceof Word: + $allWords = (string) $node->getValue(); + app('log')->debug(sprintf('Add words "%s" to search string', $allWords)); $this->words[] = $allWords; - break; - case Field::class: - app('log')->debug(sprintf('Now handle Node class %s', $class)); + case $node instanceof Field: + $this->handleFieldNode($node); + break; - /** @var Field $searchNode */ - // used to search for x:y - $operator = strtolower($searchNode->getValue()); - $value = $searchNode->getNode()->getValue(); - $prohibited = BoolOperator::PROHIBITED === $searchNode->getBoolOperator(); - $context = config(sprintf('search.operators.%s.needs_context', $operator)); + case $node instanceof Subquery: + foreach ($node->getNodes() as $subNode) { + $this->handleSearchNode($subNode); + } + break; - // is an operator that needs no context, and value is false, then prohibited = true. - if ('false' === $value && in_array($operator, $this->validOperators, true) && false === $context && !$prohibited) { - $prohibited = true; - $value = 'true'; - } - // if the operator is prohibited, but the value is false, do an uno reverse - if ('false' === $value && $prohibited && in_array($operator, $this->validOperators, true) && false === $context) { - $prohibited = false; - $value = 'true'; - } + default: + app('log')->error(sprintf('Cannot handle node %s', get_class($node))); + throw new FireflyException(sprintf('Firefly III search can\'t handle "%s"-nodes', get_class($node))); + } + } - // must be valid operator: - if ( - in_array($operator, $this->validOperators, true) - && $this->updateCollector($operator, (string) $value, $prohibited)) { - $this->operators->push( - [ - 'type' => self::getRootOperator($operator), - 'value' => (string) $value, - 'prohibited' => $prohibited, - ] - ); - app('log')->debug(sprintf('Added operator type "%s"', $operator)); - } - if (!in_array($operator, $this->validOperators, true)) { - app('log')->debug(sprintf('Added INVALID operator type "%s"', $operator)); - $this->invalidOperators[] = [ - 'type' => $operator, - 'value' => (string) $value, - ]; - } + /** + * @throws FireflyException + */ + private function handleFieldNode(Field $node): void + { + $operator = strtolower($node->getOperator()); + $value = $node->getValue(); + $prohibited = $node->isProhibited(); + + $context = config(sprintf('search.operators.%s.needs_context', $operator)); + + // is an operator that needs no context, and value is false, then prohibited = true. + if ('false' === $value && in_array($operator, $this->validOperators, true) && false === $context && !$prohibited) { + $prohibited = true; + $value = 'true'; + } + // if the operator is prohibited, but the value is false, do an uno reverse + if ('false' === $value && $prohibited && in_array($operator, $this->validOperators, true) && false === $context) { + $prohibited = false; + $value = 'true'; + } + + // must be valid operator: + if (in_array($operator, $this->validOperators, true)) { + if ($this->updateCollector($operator, (string)$value, $prohibited)) { + $this->operators->push([ + 'type' => self::getRootOperator($operator), + 'value' => (string)$value, + 'prohibited' => $prohibited, + ]); + app('log')->debug(sprintf('Added operator type "%s"', $operator)); + } + } else { + app('log')->debug(sprintf('Added INVALID operator type "%s"', $operator)); + $this->invalidOperators[] = [ + 'type' => $operator, + 'value' => (string)$value, + ]; } } diff --git a/app/Support/Search/QueryParser.php b/app/Support/Search/QueryParser.php new file mode 100644 index 0000000000..ba07f2a1c9 --- /dev/null +++ b/app/Support/Search/QueryParser.php @@ -0,0 +1,171 @@ +query = $query; + $this->position = 0; + return $this->parseQuery(); + } + + private function parseQuery(): array + { + $nodes = []; + + while ($this->position < strlen($this->query)) { + $this->skipWhitespace(); + + if ($this->position >= strlen($this->query)) { + break; + } + + // Handle subquery + if ($this->query[$this->position] === '(') { + $nodes[] = $this->parseSubquery(); + continue; + } + + // Handle field operator + if ($this->isStartOfField()) { + $nodes[] = $this->parseField(); + continue; + } + + // Handle word + $nodes[] = $this->parseWord(); + } + + return $nodes; + } + + private function parseSubquery(): Subquery + { + $this->position++; // Skip opening parenthesis + $nodes = []; + + while ($this->position < strlen($this->query)) { + $this->skipWhitespace(); + + if ($this->query[$this->position] === ')') { + $this->position++; // Skip closing parenthesis + break; + } + + if ($this->query[$this->position] === '(') { + $nodes[] = $this->parseSubquery(); + continue; + } + + if ($this->isStartOfField()) { + $nodes[] = $this->parseField(); + continue; + } + + $nodes[] = $this->parseWord(); + } + + return new Subquery($nodes); + } + + private function parseField(): Field + { + $prohibited = false; + if ($this->query[$this->position] === '-') { + $prohibited = true; + $this->position++; + } + + $operator = ''; + while ($this->position < strlen($this->query) && $this->query[$this->position] !== ':') { + $operator .= $this->query[$this->position]; + $this->position++; + } + $this->position++; // Skip colon + + $value = ''; + $inQuotes = false; + while ($this->position < strlen($this->query)) { + $char = $this->query[$this->position]; + + if ($char === '"' && !$inQuotes) { + $inQuotes = true; + $this->position++; + continue; + } + + if ($char === '"' && $inQuotes) { + $inQuotes = false; + $this->position++; + break; + } + + if (!$inQuotes && ($char === ' ' || $char === ')')) { + break; + } + + $value .= $char; + $this->position++; + } + + return new Field(trim($operator), trim($value), $prohibited); + } + + private function parseWord(): Word + { + $word = ''; + while ($this->position < strlen($this->query)) { + $char = $this->query[$this->position]; + if ($char === ' ' || $char === '(' || $char === ')') { + break; + } + $word .= $char; + $this->position++; + } + return new Word(trim($word)); + } + + private function isStartOfField(): bool + { + $pos = $this->position; + if ($this->query[$pos] === '-') { + $pos++; + } + + // Look ahead for a colon that's not inside quotes + $inQuotes = false; + while ($pos < strlen($this->query)) { + if ($this->query[$pos] === '"') { + $inQuotes = !$inQuotes; + } + if ($this->query[$pos] === ':' && !$inQuotes) { + return true; + } + if ($this->query[$pos] === ' ' && !$inQuotes) { + return false; + } + $pos++; + } + return false; + } + + private function skipWhitespace(): void + { + while ($this->position < strlen($this->query) && $this->query[$this->position] === ' ') { + $this->position++; + } + } +} diff --git a/app/Support/Search/QueryParserInterface.php b/app/Support/Search/QueryParserInterface.php new file mode 100644 index 0000000000..d56494ce70 --- /dev/null +++ b/app/Support/Search/QueryParserInterface.php @@ -0,0 +1,112 @@ +value = $value; + } + + public function getValue(): string + { + return $this->value; + } + + public function __toString(): string + { + return $this->value; + } +} + +/** + * Represents a field operator with value (e.g. amount:100) + */ +class Field extends Node +{ + private string $operator; + private string $value; + private bool $prohibited; + + public function __construct(string $operator, string $value, bool $prohibited = false) + { + $this->operator = $operator; + $this->value = $value; + $this->prohibited = $prohibited; + } + + public function getOperator(): string + { + return $this->operator; + } + + public function getValue(): string + { + return $this->value; + } + + public function isProhibited(): bool + { + return $this->prohibited; + } + + public function __toString(): string + { + return ($this->prohibited ? '-' : '') . $this->operator . ':' . $this->value; + } +} + +/** + * Represents a subquery (group of nodes) + */ +class Subquery extends Node +{ + /** @var Node[] */ + private array $nodes; + + /** + * @param Node[] $nodes + */ + public function __construct(array $nodes) + { + $this->nodes = $nodes; + } + + /** + * @return Node[] + */ + public function getNodes(): array + { + return $this->nodes; + } + + public function __toString(): string + { + return '(' . implode(' ', array_map(fn($node) => (string)$node, $this->nodes)) . ')'; + } +} diff --git a/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php b/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php new file mode 100644 index 0000000000..bb02a95b03 --- /dev/null +++ b/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php @@ -0,0 +1,213 @@ + + * + * This file is part of Firefly III (https://github.com/firefly-iii). + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +declare(strict_types=1); + +namespace Tests\unit\Support; + +use FireflyIII\Support\Search\Field; +use FireflyIII\Support\Search\QueryParserInterface; +use FireflyIII\Support\Search\Word; +use FireflyIII\Support\Search\Subquery; +use Tests\integration\TestCase; + + +abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase +{ + abstract protected function createParser(): QueryParserInterface; + + protected function setUp(): void + { + parent::setUp(); + } + + public function __construct(string $name) + { + parent::__construct($name); + } + + public function testGivenEmptyStringWhenParsingQueryThenReturnsEmptyArray(): void + { + $result = $this->createParser()->parse(''); + + $this->assertIsArray($result); + $this->assertEmpty($result); + } + + public function testGivenProhibitedFieldOperatorWhenParsingQueryThenReturnsFieldNode(): void + { + $result = $this->createParser()->parse('-amount:100'); + + $this->assertIsArray($result); + $this->assertCount(1, $result); + $this->assertInstanceOf(Field::class, $result[0]); + $this->assertTrue($result[0]->isProhibited()); + $this->assertEquals('amount', $result[0]->getOperator()); + $this->assertEquals('100', $result[0]->getValue()); + } + + /*public function testGivenNestedSubqueryWhenParsingQueryThenReturnsSubqueryNode(): void + { + $result = $this->createParser()->parse('(amount:100 (description_contains:"test payment" -has_attachments:true))'); + + $this->assertIsArray($result); + $this->assertCount(1, $result); + $this->assertInstanceOf(Subquery::class, $result[0]); + + $nodes = $result[0]->getNodes(); + $this->assertCount(2, $nodes); + + $this->assertInstanceOf(Field::class, $nodes[0]); + $this->assertEquals('amount', $nodes[0]->getOperator()); + $this->assertEquals('100', $nodes[0]->getValue()); + + $this->assertInstanceOf(Subquery::class, $nodes[1]); + $subNodes = $nodes[1]->getNodes(); + $this->assertCount(2, $subNodes); + + $this->assertInstanceOf(Field::class, $subNodes[0]); + $this->assertEquals('description_contains', $subNodes[0]->getOperator()); + $this->assertEquals('test payment', $subNodes[0]->getValue()); + + $this->assertInstanceOf(Field::class, $subNodes[1]); + $this->assertTrue($subNodes[1]->isProhibited()); + $this->assertEquals('has_attachments', $subNodes[1]->getOperator()); + $this->assertEquals('true', $subNodes[1]->getValue()); + }*/ + + public function testGivenSimpleWordWhenParsingQueryThenReturnsWordNode(): void + { + $result = $this->createParser()->parse('groceries'); + + $this->assertIsArray($result); + $this->assertCount(1, $result); + $this->assertInstanceOf(Word::class, $result[0]); + $this->assertEquals('groceries', $result[0]->getValue()); + } + + public function testGivenMultipleWordsWhenParsingQueryThenReturnsWordNodes(): void + { + $result = $this->createParser()->parse('groceries shopping market'); + + $this->assertIsArray($result); + $this->assertCount(3, $result); + + $this->assertInstanceOf(Word::class, $result[0]); + $this->assertEquals('groceries', $result[0]->getValue()); + + $this->assertInstanceOf(Word::class, $result[1]); + $this->assertEquals('shopping', $result[1]->getValue()); + + $this->assertInstanceOf(Word::class, $result[2]); + $this->assertEquals('market', $result[2]->getValue()); + } + + public function testGivenMixedWordsAndOperatorsWhenParsingQueryThenReturnsCorrectNodes(): void + { + $result = $this->createParser()->parse('groceries amount:50 shopping'); + + $this->assertIsArray($result); + $this->assertCount(3, $result); + + $this->assertInstanceOf(Word::class, $result[0]); + $this->assertEquals('groceries', $result[0]->getValue()); + + $this->assertInstanceOf(Field::class, $result[1]); + $this->assertEquals('amount', $result[1]->getOperator()); + $this->assertEquals('50', $result[1]->getValue()); + + $this->assertInstanceOf(Word::class, $result[2]); + $this->assertEquals('shopping', $result[2]->getValue()); + } + + public function testGivenQuotedValueWithSpacesWhenParsingQueryThenReturnsFieldNode(): void + { + $result = $this->createParser()->parse('description_contains:"shopping at market"'); + + $this->assertInstanceOf(Field::class, $result[0]); + $this->assertEquals('description_contains', $result[0]->getOperator()); + $this->assertEquals('shopping at market', $result[0]->getValue()); + } + + public function testGivenDecimalNumberWhenParsingQueryThenReturnsFieldNode(): void + { + $result = $this->createParser()->parse('amount:123.45'); + + $this->assertInstanceOf(Field::class, $result[0]); + $this->assertEquals('amount', $result[0]->getOperator()); + $this->assertEquals('123.45', $result[0]->getValue()); + } + + public function testGivenBooleanOperatorWhenParsingQueryThenReturnsFieldNode(): void + { + $result = $this->createParser()->parse('has_any_category:true'); + + $this->assertInstanceOf(Field::class, $result[0]); + $this->assertEquals('has_any_category', $result[0]->getOperator()); + $this->assertEquals('true', $result[0]->getValue()); + } + + /*public function testGivenIncompleteFieldOperatorWhenParsingQueryThenHandlesGracefully(): void + { + $result = $this->createParser()->parse('amount:'); + + $this->assertInstanceOf(Field::class, $result[0]); + $this->assertEquals('amount', $result[0]->getOperator()); + $this->assertEquals('', $result[0]->getValue()); + }*/ + + public function testGivenUnterminatedQuoteWhenParsingQueryThenHandlesGracefully(): void + { + $result = $this->createParser()->parse('description_contains:"unterminated'); + + $this->assertInstanceOf(Field::class, $result[0]); + $this->assertEquals('description_contains', $result[0]->getOperator()); + $this->assertEquals('unterminated', $result[0]->getValue()); + } + + public function testGivenWordFollowedBySubqueryWithoutSpaceWhenParsingQueryThenReturnsCorrectNodes(): void +{ + $result = $this->createParser()->parse('groceries(amount:100 description_contains:"test")'); + + $this->assertIsArray($result); + $this->assertCount(2, $result); + + // Test the word node + $this->assertInstanceOf(Word::class, $result[0]); + $this->assertEquals('groceries', $result[0]->getValue()); + + // Test the subquery node + $this->assertInstanceOf(Subquery::class, $result[1]); + $nodes = $result[1]->getNodes(); + $this->assertCount(2, $nodes); + + // Test first field in subquery + $this->assertInstanceOf(Field::class, $nodes[0]); + $this->assertEquals('amount', $nodes[0]->getOperator()); + $this->assertEquals('100', $nodes[0]->getValue()); + + // Test second field in subquery + $this->assertInstanceOf(Field::class, $nodes[1]); + $this->assertEquals('description_contains', $nodes[1]->getOperator()); + $this->assertEquals('test', $nodes[1]->getValue()); +} +} diff --git a/tests/unit/Support/Search/GdbotsQueryParserParseQueryTest.php b/tests/unit/Support/Search/GdbotsQueryParserParseQueryTest.php new file mode 100644 index 0000000000..c0e6fbd71d --- /dev/null +++ b/tests/unit/Support/Search/GdbotsQueryParserParseQueryTest.php @@ -0,0 +1,24 @@ + Date: Tue, 31 Dec 2024 10:40:32 +0100 Subject: [PATCH 02/27] Fix namespace --- .../Search/AbstractQueryParserInterfaceParseQueryTest.php | 2 +- tests/unit/Support/Search/GdbotsQueryParserParseQueryTest.php | 4 ++-- tests/unit/Support/Search/QueryParserParseQueryTest.php | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php b/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php index bb02a95b03..8c4758d2e1 100644 --- a/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php +++ b/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php @@ -22,7 +22,7 @@ declare(strict_types=1); -namespace Tests\unit\Support; +namespace Tests\unit\Support\Search; use FireflyIII\Support\Search\Field; use FireflyIII\Support\Search\QueryParserInterface; diff --git a/tests/unit/Support/Search/GdbotsQueryParserParseQueryTest.php b/tests/unit/Support/Search/GdbotsQueryParserParseQueryTest.php index c0e6fbd71d..897d9ecb01 100644 --- a/tests/unit/Support/Search/GdbotsQueryParserParseQueryTest.php +++ b/tests/unit/Support/Search/GdbotsQueryParserParseQueryTest.php @@ -1,10 +1,10 @@ Date: Wed, 1 Jan 2025 04:17:55 +0100 Subject: [PATCH 03/27] More WIP --- app/Support/Search/OperatorQuerySearch.php | 13 +- app/Support/Search/QueryParser2.php | 136 +++++++++++ app/Support/Search/QueryParserInterface.php | 23 +- ...ractQueryParserInterfaceParseQueryTest.php | 226 ++++++++++++++---- .../Search/QueryParserParseQueryTest.php | 4 +- 5 files changed, 341 insertions(+), 61 deletions(-) create mode 100644 app/Support/Search/QueryParser2.php diff --git a/app/Support/Search/OperatorQuerySearch.php b/app/Support/Search/OperatorQuerySearch.php index ca989bd911..596aedd7d0 100644 --- a/app/Support/Search/OperatorQuerySearch.php +++ b/app/Support/Search/OperatorQuerySearch.php @@ -166,9 +166,15 @@ class OperatorQuerySearch implements SearchInterface switch (true) { case $node instanceof Word: - $allWords = (string) $node->getValue(); - app('log')->debug(sprintf('Add words "%s" to search string', $allWords)); - $this->words[] = $allWords; + $word = (string) $node->getValue(); + if($node->isProhibited()) { + app('log')->debug(sprintf('Exclude word "%s" from search string', $word)); + $this->prohibitedWords[] = $word; + } else { + app('log')->debug(sprintf('Add word "%s" to search string', $word)); + $this->words[] = $word; + } + break; case $node instanceof Field: @@ -176,6 +182,7 @@ class OperatorQuerySearch implements SearchInterface break; case $node instanceof Subquery: + //TODO: Handle Subquery prohibition, i.e. flip all prohibition flags inside the subquery foreach ($node->getNodes() as $subNode) { $this->handleSearchNode($subNode); } diff --git a/app/Support/Search/QueryParser2.php b/app/Support/Search/QueryParser2.php new file mode 100644 index 0000000000..76e2a816e4 --- /dev/null +++ b/app/Support/Search/QueryParser2.php @@ -0,0 +1,136 @@ +query = $query; + $this->position = 0; + return $this->parseQuery(); + } + + private function parseQuery(): array + { + $nodes = []; + $token = $this->buildNextNode(); + + while ($token !== null) { + $nodes[] = $token; + $token = $this->buildNextNode(); + } + + return $nodes; + } + + private function buildNextNode(): ?Node + { + $tokenUnderConstruction = ''; + $inQuotes = false; + $fieldName = ''; + $prohibited = false; + + while ($this->position < strlen($this->query)) { + $char = $this->query[$this->position]; + + // If we're in a quoted string, we treat all characters except another quote as ordinary characters + if ($inQuotes) { + if($char !== '"') { + $tokenUnderConstruction .= $char; + $this->position++; + continue; + } else { + $this->position++; + return $this->createNode($tokenUnderConstruction, $fieldName, $prohibited); + } + } + + switch ($char) { + case '-': + if ($tokenUnderConstruction === '') { + // A minus sign at the beginning of a token indicates prohibition + $prohibited = true; + } else { + // In any other location, it's just a normal character + $tokenUnderConstruction .= $char; + } + break; + + case '"': + if ($tokenUnderConstruction === '') { + // A quote sign at the beginning of a token indicates the start of a quoted string + $inQuotes = true; + } else { + // In any other location, it's just a normal character + $tokenUnderConstruction .= $char; + } + break; + + case '(': + if ($tokenUnderConstruction === '') { + // A left parentheses at the beginning of a token indicates the start of a subquery + $this->position++; + return new Subquery($this->parseQuery(), $prohibited); + } else { + // In any other location, it's just a normal character + $tokenUnderConstruction .= $char; + } + break; + + case ')': + if ($tokenUnderConstruction !== '') { + $this->position++; + return $this->createNode($tokenUnderConstruction, $fieldName, $prohibited); + } + $this->position++; + return null; + + + case ':': + if ($tokenUnderConstruction !== '') { + // If we meet a colon with a left-hand side string, we know we're in a field and are about to set up the value + $fieldName = $tokenUnderConstruction; + $tokenUnderConstruction = ''; + } else { + // In any other location, it's just a normal character + $tokenUnderConstruction .= $char; + } + break; + + case ' ': + // A space indicates the end of a token construction if non-empty, otherwise it's just ignored + if ($tokenUnderConstruction !== '') { + $this->position++; + return $this->createNode($tokenUnderConstruction, $fieldName, $prohibited); + } + break; + + default: + $tokenUnderConstruction .= $char; + } + + $this->position++; + } + + return $fieldName !== '' || $tokenUnderConstruction !== '' + ? $this->createNode($tokenUnderConstruction, $fieldName, $prohibited) + : null; + } + + private function createNode(string $token, string $fieldName, bool $prohibited): Node + { + if (strlen($fieldName) > 0) { + return new Field(trim($fieldName), trim($token), $prohibited); + } + return new Word(trim($token), $prohibited); + } +} diff --git a/app/Support/Search/QueryParserInterface.php b/app/Support/Search/QueryParserInterface.php index d56494ce70..93f88d1e75 100644 --- a/app/Support/Search/QueryParserInterface.php +++ b/app/Support/Search/QueryParserInterface.php @@ -27,10 +27,12 @@ abstract class Node class Word extends Node { private string $value; + private bool $prohibited; - public function __construct(string $value) + public function __construct(string $value, bool $prohibited = false) { $this->value = $value; + $this->prohibited = $prohibited; } public function getValue(): string @@ -38,9 +40,14 @@ class Word extends Node return $this->value; } + public function isProhibited(): bool + { + return $this->prohibited; + } + public function __toString(): string { - return $this->value; + return ($this->prohibited ? '-' : '') . $this->value; } } @@ -89,12 +96,15 @@ class Subquery extends Node /** @var Node[] */ private array $nodes; + private bool $prohibited; + /** * @param Node[] $nodes */ - public function __construct(array $nodes) + public function __construct(array $nodes, bool $prohibited = false) { $this->nodes = $nodes; + $this->prohibited = $prohibited; } /** @@ -105,8 +115,13 @@ class Subquery extends Node return $this->nodes; } + public function isProhibited(): bool + { + return $this->prohibited; + } + public function __toString(): string { - return '(' . implode(' ', array_map(fn($node) => (string)$node, $this->nodes)) . ')'; + return ($this->prohibited ? '-' : '') . '(' . implode(' ', array_map(fn($node) => (string)$node, $this->nodes)) . ')'; } } diff --git a/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php b/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php index 8c4758d2e1..5b27e4caeb 100644 --- a/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php +++ b/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php @@ -65,35 +65,6 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase $this->assertEquals('100', $result[0]->getValue()); } - /*public function testGivenNestedSubqueryWhenParsingQueryThenReturnsSubqueryNode(): void - { - $result = $this->createParser()->parse('(amount:100 (description_contains:"test payment" -has_attachments:true))'); - - $this->assertIsArray($result); - $this->assertCount(1, $result); - $this->assertInstanceOf(Subquery::class, $result[0]); - - $nodes = $result[0]->getNodes(); - $this->assertCount(2, $nodes); - - $this->assertInstanceOf(Field::class, $nodes[0]); - $this->assertEquals('amount', $nodes[0]->getOperator()); - $this->assertEquals('100', $nodes[0]->getValue()); - - $this->assertInstanceOf(Subquery::class, $nodes[1]); - $subNodes = $nodes[1]->getNodes(); - $this->assertCount(2, $subNodes); - - $this->assertInstanceOf(Field::class, $subNodes[0]); - $this->assertEquals('description_contains', $subNodes[0]->getOperator()); - $this->assertEquals('test payment', $subNodes[0]->getValue()); - - $this->assertInstanceOf(Field::class, $subNodes[1]); - $this->assertTrue($subNodes[1]->isProhibited()); - $this->assertEquals('has_attachments', $subNodes[1]->getOperator()); - $this->assertEquals('true', $subNodes[1]->getValue()); - }*/ - public function testGivenSimpleWordWhenParsingQueryThenReturnsWordNode(): void { $result = $this->createParser()->parse('groceries'); @@ -166,14 +137,23 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase $this->assertEquals('true', $result[0]->getValue()); } - /*public function testGivenIncompleteFieldOperatorWhenParsingQueryThenHandlesGracefully(): void + public function testGivenFieldOperatorWithBlankValueWhenParsingQueryThenReturnsCorrectNodes(): void { $result = $this->createParser()->parse('amount:'); $this->assertInstanceOf(Field::class, $result[0]); $this->assertEquals('amount', $result[0]->getOperator()); $this->assertEquals('', $result[0]->getValue()); - }*/ + } + + public function testGivenFieldOperatorWithEmptyQuotedStringWhenParsingQueryThenReturnsCorrectNodes(): void + { + $result = $this->createParser()->parse('amount:""'); + + $this->assertInstanceOf(Field::class, $result[0]); + $this->assertEquals('amount', $result[0]->getOperator()); + $this->assertEquals('', $result[0]->getValue()); + } public function testGivenUnterminatedQuoteWhenParsingQueryThenHandlesGracefully(): void { @@ -184,30 +164,172 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase $this->assertEquals('unterminated', $result[0]->getValue()); } - public function testGivenWordFollowedBySubqueryWithoutSpaceWhenParsingQueryThenReturnsCorrectNodes(): void -{ - $result = $this->createParser()->parse('groceries(amount:100 description_contains:"test")'); + public function testGivenWordFollowedBySubqueryWhenParsingQueryThenReturnsCorrectNodes(): void + { + $result = $this->createParser()->parse('groceries (amount:100 description_contains:"test")'); - $this->assertIsArray($result); - $this->assertCount(2, $result); + $this->assertIsArray($result); + $this->assertCount(2, $result); - // Test the word node - $this->assertInstanceOf(Word::class, $result[0]); - $this->assertEquals('groceries', $result[0]->getValue()); + // Test the word node + $this->assertInstanceOf(Word::class, $result[0]); + $this->assertEquals('groceries', $result[0]->getValue()); - // Test the subquery node - $this->assertInstanceOf(Subquery::class, $result[1]); - $nodes = $result[1]->getNodes(); - $this->assertCount(2, $nodes); + // Test the subquery node + $this->assertInstanceOf(Subquery::class, $result[1]); + $nodes = $result[1]->getNodes(); + $this->assertCount(2, $nodes); - // Test first field in subquery - $this->assertInstanceOf(Field::class, $nodes[0]); - $this->assertEquals('amount', $nodes[0]->getOperator()); - $this->assertEquals('100', $nodes[0]->getValue()); + // Test first field in subquery + $this->assertInstanceOf(Field::class, $nodes[0]); + $this->assertEquals('amount', $nodes[0]->getOperator()); + $this->assertEquals('100', $nodes[0]->getValue()); - // Test second field in subquery - $this->assertInstanceOf(Field::class, $nodes[1]); - $this->assertEquals('description_contains', $nodes[1]->getOperator()); - $this->assertEquals('test', $nodes[1]->getValue()); -} + // Test second field in subquery + $this->assertInstanceOf(Field::class, $nodes[1]); + $this->assertEquals('description_contains', $nodes[1]->getOperator()); + $this->assertEquals('test', $nodes[1]->getValue()); + } + + public function testGivenMultipleFieldsWithQuotedValuesWhenParsingQueryThenReturnsFieldNodes(): void + { + $result = $this->createParser()->parse('description:"shopping at market" notes:"paid in cash" category:"groceries and food"'); + + $this->assertIsArray($result); + $this->assertCount(3, $result); + + $this->assertInstanceOf(Field::class, $result[0]); + $this->assertEquals('description', $result[0]->getOperator()); + $this->assertEquals('shopping at market', $result[0]->getValue()); + + $this->assertInstanceOf(Field::class, $result[1]); + $this->assertEquals('notes', $result[1]->getOperator()); + $this->assertEquals('paid in cash', $result[1]->getValue()); + + $this->assertInstanceOf(Field::class, $result[2]); + $this->assertEquals('category', $result[2]->getOperator()); + $this->assertEquals('groceries and food', $result[2]->getValue()); + } + + public function testGivenSubqueryAfterFieldValueWhenParsingQueryThenReturnsCorrectNodes(): void + { + $result = $this->createParser()->parse('amount:100 (description:"market" category:food)'); + + $this->assertIsArray($result); + $this->assertCount(2, $result); + + $this->assertInstanceOf(Field::class, $result[0]); + $this->assertEquals('amount', $result[0]->getOperator()); + $this->assertEquals('100', $result[0]->getValue()); + + $this->assertInstanceOf(Subquery::class, $result[1]); + $nodes = $result[1]->getNodes(); + $this->assertCount(2, $nodes); + + $this->assertInstanceOf(Field::class, $nodes[0]); + $this->assertEquals('description', $nodes[0]->getOperator()); + $this->assertEquals('market', $nodes[0]->getValue()); + + $this->assertInstanceOf(Field::class, $nodes[1]); + $this->assertEquals('category', $nodes[1]->getOperator()); + $this->assertEquals('food', $nodes[1]->getValue()); + } + + public function testGivenMultipleFieldsWithQuotedValuesWithoutSpacesWhenParsingQueryThenReturnsFieldNodes(): void + { + $result = $this->createParser()->parse('description:"shopping market"category:"groceries"notes:"cash payment"'); + + $this->assertIsArray($result); + $this->assertCount(3, $result); + + $this->assertInstanceOf(Field::class, $result[0]); + $this->assertEquals('description', $result[0]->getOperator()); + $this->assertEquals('shopping market', $result[0]->getValue()); + + $this->assertInstanceOf(Field::class, $result[1]); + $this->assertEquals('category', $result[1]->getOperator()); + $this->assertEquals('groceries', $result[1]->getValue()); + + $this->assertInstanceOf(Field::class, $result[2]); + $this->assertEquals('notes', $result[2]->getOperator()); + $this->assertEquals('cash payment', $result[2]->getValue()); + } + + public function testGivenStringWithSingleQuoteInMiddleWhenParsingQueryThenReturnsWordNode(): void + { + $result = $this->createParser()->parse('stringWithSingle"InMiddle'); + + $this->assertIsArray($result); + $this->assertCount(1, $result); + $this->assertInstanceOf(Word::class, $result[0]); + $this->assertEquals('stringWithSingle"InMiddle', $result[0]->getValue()); + } + + public function testGivenWordStartingWithColonWhenParsingQueryThenReturnsWordNode(): void + { + $result = $this->createParser()->parse(':startingWithColon'); + + $this->assertIsArray($result); + $this->assertCount(1, $result); + $this->assertInstanceOf(Word::class, $result[0]); + $this->assertEquals(':startingWithColon', $result[0]->getValue()); + } + + public function testGivenComplexNestedSubqueriesWhenParsingQueryThenReturnsCorrectNodes(): void + { + $result = $this->createParser()->parse('shopping (amount:50 market (-category:food word description:"test phrase" (has_notes:true)))'); + + $this->assertIsArray($result); + $this->assertCount(2, $result); + + // Test the first word node + $this->assertInstanceOf(Word::class, $result[0]); + $this->assertEquals('shopping', $result[0]->getValue()); + + // Test first level subquery + $this->assertInstanceOf(Subquery::class, $result[1]); + /** @var Subquery $firstLevelSubquery */ + $firstLevelSubquery = $result[1]; + $level1Nodes = $firstLevelSubquery->getNodes(); + $this->assertCount(3, $level1Nodes); + + // Test field in first level + $this->assertInstanceOf(Field::class, $level1Nodes[0]); + $this->assertEquals('amount', $level1Nodes[0]->getOperator()); + $this->assertEquals('50', $level1Nodes[0]->getValue()); + + // Test word in first level + $this->assertInstanceOf(Word::class, $level1Nodes[1]); + $this->assertEquals('market', $level1Nodes[1]->getValue()); + + // Test second level subquery + $this->assertInstanceOf(Subquery::class, $level1Nodes[2]); + $level2Nodes = $level1Nodes[2]->getNodes(); + $this->assertCount(4, $level2Nodes); + + // Test prohibited field in second level + $this->assertInstanceOf(Field::class, $level2Nodes[0]); + $this->assertTrue($level2Nodes[0]->isProhibited()); + $this->assertEquals('category', $level2Nodes[0]->getOperator()); + $this->assertEquals('food', $level2Nodes[0]->getValue()); + + // Test word in second level + $this->assertInstanceOf(Word::class, $level2Nodes[1]); + $this->assertEquals('word', $level2Nodes[1]->getValue()); + + // Test field with quoted value in second level + $this->assertInstanceOf(Field::class, $level2Nodes[2]); + $this->assertEquals('description', $level2Nodes[2]->getOperator()); + $this->assertEquals('test phrase', $level2Nodes[2]->getValue()); + + // Test third level subquery + $this->assertInstanceOf(Subquery::class, $level2Nodes[3]); + $level3Nodes = $level2Nodes[3]->getNodes(); + $this->assertCount(1, $level3Nodes); + + // Test field in third level + $this->assertInstanceOf(Field::class, $level3Nodes[0]); + $this->assertEquals('has_notes', $level3Nodes[0]->getOperator()); + $this->assertEquals('true', $level3Nodes[0]->getValue()); + } } diff --git a/tests/unit/Support/Search/QueryParserParseQueryTest.php b/tests/unit/Support/Search/QueryParserParseQueryTest.php index a5c4e85963..f05301f29c 100644 --- a/tests/unit/Support/Search/QueryParserParseQueryTest.php +++ b/tests/unit/Support/Search/QueryParserParseQueryTest.php @@ -2,7 +2,7 @@ namespace Tests\unit\Support\Search; -use FireflyIII\Support\Search\QueryParser; +use FireflyIII\Support\Search\QueryParser2; use FireflyIII\Support\Search\QueryParserInterface; use Tests\unit\Support\Search\AbstractQueryParserInterfaceParseQueryTest; @@ -20,6 +20,6 @@ final class QueryParserParseQueryTest extends AbstractQueryParserInterfaceParseQ { protected function createParser(): QueryParserInterface { - return new QueryParser(); + return new QueryParser2(); } } From 0e3d779e241db92c394d987756a23f47d911a9b2 Mon Sep 17 00:00:00 2001 From: Sobuno Date: Wed, 1 Jan 2025 04:27:44 +0100 Subject: [PATCH 04/27] Remove V1 --- app/Support/Search/QueryParser.php | 171 ----------------------------- 1 file changed, 171 deletions(-) delete mode 100644 app/Support/Search/QueryParser.php diff --git a/app/Support/Search/QueryParser.php b/app/Support/Search/QueryParser.php deleted file mode 100644 index ba07f2a1c9..0000000000 --- a/app/Support/Search/QueryParser.php +++ /dev/null @@ -1,171 +0,0 @@ -query = $query; - $this->position = 0; - return $this->parseQuery(); - } - - private function parseQuery(): array - { - $nodes = []; - - while ($this->position < strlen($this->query)) { - $this->skipWhitespace(); - - if ($this->position >= strlen($this->query)) { - break; - } - - // Handle subquery - if ($this->query[$this->position] === '(') { - $nodes[] = $this->parseSubquery(); - continue; - } - - // Handle field operator - if ($this->isStartOfField()) { - $nodes[] = $this->parseField(); - continue; - } - - // Handle word - $nodes[] = $this->parseWord(); - } - - return $nodes; - } - - private function parseSubquery(): Subquery - { - $this->position++; // Skip opening parenthesis - $nodes = []; - - while ($this->position < strlen($this->query)) { - $this->skipWhitespace(); - - if ($this->query[$this->position] === ')') { - $this->position++; // Skip closing parenthesis - break; - } - - if ($this->query[$this->position] === '(') { - $nodes[] = $this->parseSubquery(); - continue; - } - - if ($this->isStartOfField()) { - $nodes[] = $this->parseField(); - continue; - } - - $nodes[] = $this->parseWord(); - } - - return new Subquery($nodes); - } - - private function parseField(): Field - { - $prohibited = false; - if ($this->query[$this->position] === '-') { - $prohibited = true; - $this->position++; - } - - $operator = ''; - while ($this->position < strlen($this->query) && $this->query[$this->position] !== ':') { - $operator .= $this->query[$this->position]; - $this->position++; - } - $this->position++; // Skip colon - - $value = ''; - $inQuotes = false; - while ($this->position < strlen($this->query)) { - $char = $this->query[$this->position]; - - if ($char === '"' && !$inQuotes) { - $inQuotes = true; - $this->position++; - continue; - } - - if ($char === '"' && $inQuotes) { - $inQuotes = false; - $this->position++; - break; - } - - if (!$inQuotes && ($char === ' ' || $char === ')')) { - break; - } - - $value .= $char; - $this->position++; - } - - return new Field(trim($operator), trim($value), $prohibited); - } - - private function parseWord(): Word - { - $word = ''; - while ($this->position < strlen($this->query)) { - $char = $this->query[$this->position]; - if ($char === ' ' || $char === '(' || $char === ')') { - break; - } - $word .= $char; - $this->position++; - } - return new Word(trim($word)); - } - - private function isStartOfField(): bool - { - $pos = $this->position; - if ($this->query[$pos] === '-') { - $pos++; - } - - // Look ahead for a colon that's not inside quotes - $inQuotes = false; - while ($pos < strlen($this->query)) { - if ($this->query[$pos] === '"') { - $inQuotes = !$inQuotes; - } - if ($this->query[$pos] === ':' && !$inQuotes) { - return true; - } - if ($this->query[$pos] === ' ' && !$inQuotes) { - return false; - } - $pos++; - } - return false; - } - - private function skipWhitespace(): void - { - while ($this->position < strlen($this->query) && $this->query[$this->position] === ' ') { - $this->position++; - } - } -} From be2d3f3637de4698d9eb10427ef5ace4ceff471e Mon Sep 17 00:00:00 2001 From: Sobuno Date: Wed, 1 Jan 2025 04:28:06 +0100 Subject: [PATCH 05/27] Rename QueryParser2 to QueryParser --- app/Support/Search/{QueryParser2.php => QueryParser.php} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename app/Support/Search/{QueryParser2.php => QueryParser.php} (98%) diff --git a/app/Support/Search/QueryParser2.php b/app/Support/Search/QueryParser.php similarity index 98% rename from app/Support/Search/QueryParser2.php rename to app/Support/Search/QueryParser.php index 76e2a816e4..e25e7f2afe 100644 --- a/app/Support/Search/QueryParser2.php +++ b/app/Support/Search/QueryParser.php @@ -7,7 +7,7 @@ namespace FireflyIII\Support\Search; /** * Query parser class */ -class QueryParser2 implements QueryParserInterface +class QueryParser implements QueryParserInterface { private string $query; private int $position = 0; From e38e4574adf40f33ba30b43ce0bdfd97b865d7a5 Mon Sep 17 00:00:00 2001 From: Sobuno Date: Wed, 1 Jan 2025 04:28:18 +0100 Subject: [PATCH 06/27] Fix warnings in tests and add some more --- ...ractQueryParserInterfaceParseQueryTest.php | 408 +++++++++++------- 1 file changed, 247 insertions(+), 161 deletions(-) diff --git a/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php b/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php index 5b27e4caeb..06e67a1713 100644 --- a/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php +++ b/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php @@ -30,7 +30,6 @@ use FireflyIII\Support\Search\Word; use FireflyIII\Support\Search\Subquery; use Tests\integration\TestCase; - abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase { abstract protected function createParser(): QueryParserInterface; @@ -60,9 +59,11 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase $this->assertIsArray($result); $this->assertCount(1, $result); $this->assertInstanceOf(Field::class, $result[0]); - $this->assertTrue($result[0]->isProhibited()); - $this->assertEquals('amount', $result[0]->getOperator()); - $this->assertEquals('100', $result[0]->getValue()); + /** @var Field $field */ + $field = $result[0]; + $this->assertTrue($field->isProhibited()); + $this->assertEquals('amount', $field->getOperator()); + $this->assertEquals('100', $field->getValue()); } public function testGivenSimpleWordWhenParsingQueryThenReturnsWordNode(): void @@ -72,7 +73,9 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase $this->assertIsArray($result); $this->assertCount(1, $result); $this->assertInstanceOf(Word::class, $result[0]); - $this->assertEquals('groceries', $result[0]->getValue()); + /** @var Word $word */ + $word = $result[0]; + $this->assertEquals('groceries', $word->getValue()); } public function testGivenMultipleWordsWhenParsingQueryThenReturnsWordNodes(): void @@ -83,13 +86,19 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase $this->assertCount(3, $result); $this->assertInstanceOf(Word::class, $result[0]); - $this->assertEquals('groceries', $result[0]->getValue()); + /** @var Word $word1 */ + $word1 = $result[0]; + $this->assertEquals('groceries', $word1->getValue()); $this->assertInstanceOf(Word::class, $result[1]); - $this->assertEquals('shopping', $result[1]->getValue()); + /** @var Word $word2 */ + $word2 = $result[1]; + $this->assertEquals('shopping', $word2->getValue()); $this->assertInstanceOf(Word::class, $result[2]); - $this->assertEquals('market', $result[2]->getValue()); + /** @var Word $word3 */ + $word3 = $result[2]; + $this->assertEquals('market', $word3->getValue()); } public function testGivenMixedWordsAndOperatorsWhenParsingQueryThenReturnsCorrectNodes(): void @@ -100,14 +109,20 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase $this->assertCount(3, $result); $this->assertInstanceOf(Word::class, $result[0]); - $this->assertEquals('groceries', $result[0]->getValue()); + /** @var Word $word1 */ + $word1 = $result[0]; + $this->assertEquals('groceries', $word1->getValue()); $this->assertInstanceOf(Field::class, $result[1]); - $this->assertEquals('amount', $result[1]->getOperator()); - $this->assertEquals('50', $result[1]->getValue()); + /** @var Field $field */ + $field = $result[1]; + $this->assertEquals('amount', $field->getOperator()); + $this->assertEquals('50', $field->getValue()); $this->assertInstanceOf(Word::class, $result[2]); - $this->assertEquals('shopping', $result[2]->getValue()); + /** @var Word $word2 */ + $word2 = $result[2]; + $this->assertEquals('shopping', $word2->getValue()); } public function testGivenQuotedValueWithSpacesWhenParsingQueryThenReturnsFieldNode(): void @@ -115,100 +130,10 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase $result = $this->createParser()->parse('description_contains:"shopping at market"'); $this->assertInstanceOf(Field::class, $result[0]); - $this->assertEquals('description_contains', $result[0]->getOperator()); - $this->assertEquals('shopping at market', $result[0]->getValue()); - } - - public function testGivenDecimalNumberWhenParsingQueryThenReturnsFieldNode(): void - { - $result = $this->createParser()->parse('amount:123.45'); - - $this->assertInstanceOf(Field::class, $result[0]); - $this->assertEquals('amount', $result[0]->getOperator()); - $this->assertEquals('123.45', $result[0]->getValue()); - } - - public function testGivenBooleanOperatorWhenParsingQueryThenReturnsFieldNode(): void - { - $result = $this->createParser()->parse('has_any_category:true'); - - $this->assertInstanceOf(Field::class, $result[0]); - $this->assertEquals('has_any_category', $result[0]->getOperator()); - $this->assertEquals('true', $result[0]->getValue()); - } - - public function testGivenFieldOperatorWithBlankValueWhenParsingQueryThenReturnsCorrectNodes(): void - { - $result = $this->createParser()->parse('amount:'); - - $this->assertInstanceOf(Field::class, $result[0]); - $this->assertEquals('amount', $result[0]->getOperator()); - $this->assertEquals('', $result[0]->getValue()); - } - - public function testGivenFieldOperatorWithEmptyQuotedStringWhenParsingQueryThenReturnsCorrectNodes(): void - { - $result = $this->createParser()->parse('amount:""'); - - $this->assertInstanceOf(Field::class, $result[0]); - $this->assertEquals('amount', $result[0]->getOperator()); - $this->assertEquals('', $result[0]->getValue()); - } - - public function testGivenUnterminatedQuoteWhenParsingQueryThenHandlesGracefully(): void - { - $result = $this->createParser()->parse('description_contains:"unterminated'); - - $this->assertInstanceOf(Field::class, $result[0]); - $this->assertEquals('description_contains', $result[0]->getOperator()); - $this->assertEquals('unterminated', $result[0]->getValue()); - } - - public function testGivenWordFollowedBySubqueryWhenParsingQueryThenReturnsCorrectNodes(): void - { - $result = $this->createParser()->parse('groceries (amount:100 description_contains:"test")'); - - $this->assertIsArray($result); - $this->assertCount(2, $result); - - // Test the word node - $this->assertInstanceOf(Word::class, $result[0]); - $this->assertEquals('groceries', $result[0]->getValue()); - - // Test the subquery node - $this->assertInstanceOf(Subquery::class, $result[1]); - $nodes = $result[1]->getNodes(); - $this->assertCount(2, $nodes); - - // Test first field in subquery - $this->assertInstanceOf(Field::class, $nodes[0]); - $this->assertEquals('amount', $nodes[0]->getOperator()); - $this->assertEquals('100', $nodes[0]->getValue()); - - // Test second field in subquery - $this->assertInstanceOf(Field::class, $nodes[1]); - $this->assertEquals('description_contains', $nodes[1]->getOperator()); - $this->assertEquals('test', $nodes[1]->getValue()); - } - - public function testGivenMultipleFieldsWithQuotedValuesWhenParsingQueryThenReturnsFieldNodes(): void - { - $result = $this->createParser()->parse('description:"shopping at market" notes:"paid in cash" category:"groceries and food"'); - - $this->assertIsArray($result); - $this->assertCount(3, $result); - - $this->assertInstanceOf(Field::class, $result[0]); - $this->assertEquals('description', $result[0]->getOperator()); - $this->assertEquals('shopping at market', $result[0]->getValue()); - - $this->assertInstanceOf(Field::class, $result[1]); - $this->assertEquals('notes', $result[1]->getOperator()); - $this->assertEquals('paid in cash', $result[1]->getValue()); - - $this->assertInstanceOf(Field::class, $result[2]); - $this->assertEquals('category', $result[2]->getOperator()); - $this->assertEquals('groceries and food', $result[2]->getValue()); + /** @var Field $field */ + $field = $result[0]; + $this->assertEquals('description_contains', $field->getOperator()); + $this->assertEquals('shopping at market', $field->getValue()); } public function testGivenSubqueryAfterFieldValueWhenParsingQueryThenReturnsCorrectNodes(): void @@ -219,60 +144,97 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase $this->assertCount(2, $result); $this->assertInstanceOf(Field::class, $result[0]); - $this->assertEquals('amount', $result[0]->getOperator()); - $this->assertEquals('100', $result[0]->getValue()); + /** @var Field $field */ + $field = $result[0]; + $this->assertEquals('amount', $field->getOperator()); + $this->assertEquals('100', $field->getValue()); $this->assertInstanceOf(Subquery::class, $result[1]); - $nodes = $result[1]->getNodes(); + /** @var Subquery $subquery */ + $subquery = $result[1]; + $nodes = $subquery->getNodes(); $this->assertCount(2, $nodes); $this->assertInstanceOf(Field::class, $nodes[0]); - $this->assertEquals('description', $nodes[0]->getOperator()); - $this->assertEquals('market', $nodes[0]->getValue()); + /** @var Field $field1 */ + $field1 = $nodes[0]; + $this->assertEquals('description', $field1->getOperator()); + $this->assertEquals('market', $field1->getValue()); $this->assertInstanceOf(Field::class, $nodes[1]); - $this->assertEquals('category', $nodes[1]->getOperator()); - $this->assertEquals('food', $nodes[1]->getValue()); + /** @var Field $field2 */ + $field2 = $nodes[1]; + $this->assertEquals('category', $field2->getOperator()); + $this->assertEquals('food', $field2->getValue()); } - public function testGivenMultipleFieldsWithQuotedValuesWithoutSpacesWhenParsingQueryThenReturnsFieldNodes(): void + public function testGivenWordFollowedBySubqueryWhenParsingQueryThenReturnsCorrectNodes(): void { - $result = $this->createParser()->parse('description:"shopping market"category:"groceries"notes:"cash payment"'); + $result = $this->createParser()->parse('groceries (amount:100 description_contains:"test")'); $this->assertIsArray($result); - $this->assertCount(3, $result); + $this->assertCount(2, $result); - $this->assertInstanceOf(Field::class, $result[0]); - $this->assertEquals('description', $result[0]->getOperator()); - $this->assertEquals('shopping market', $result[0]->getValue()); + $this->assertInstanceOf(Word::class, $result[0]); + /** @var Word $word */ + $word = $result[0]; + $this->assertEquals('groceries', $word->getValue()); - $this->assertInstanceOf(Field::class, $result[1]); - $this->assertEquals('category', $result[1]->getOperator()); - $this->assertEquals('groceries', $result[1]->getValue()); + $this->assertInstanceOf(Subquery::class, $result[1]); + /** @var Subquery $subquery */ + $subquery = $result[1]; + $nodes = $subquery->getNodes(); + $this->assertCount(2, $nodes); - $this->assertInstanceOf(Field::class, $result[2]); - $this->assertEquals('notes', $result[2]->getOperator()); - $this->assertEquals('cash payment', $result[2]->getValue()); + $this->assertInstanceOf(Field::class, $nodes[0]); + /** @var Field $field1 */ + $field1 = $nodes[0]; + $this->assertEquals('amount', $field1->getOperator()); + $this->assertEquals('100', $field1->getValue()); + + $this->assertInstanceOf(Field::class, $nodes[1]); + /** @var Field $field2 */ + $field2 = $nodes[1]; + $this->assertEquals('description_contains', $field2->getOperator()); + $this->assertEquals('test', $field2->getValue()); } - public function testGivenStringWithSingleQuoteInMiddleWhenParsingQueryThenReturnsWordNode(): void + public function testGivenNestedSubqueryWhenParsingQueryThenReturnsSubqueryNode(): void { - $result = $this->createParser()->parse('stringWithSingle"InMiddle'); + $result = $this->createParser()->parse('(amount:100 (description_contains:"test payment" -has_attachments:true))'); $this->assertIsArray($result); $this->assertCount(1, $result); - $this->assertInstanceOf(Word::class, $result[0]); - $this->assertEquals('stringWithSingle"InMiddle', $result[0]->getValue()); - } + $this->assertInstanceOf(Subquery::class, $result[0]); + /** @var Subquery $outerSubquery */ + $outerSubquery = $result[0]; + $nodes = $outerSubquery->getNodes(); + $this->assertCount(2, $nodes); - public function testGivenWordStartingWithColonWhenParsingQueryThenReturnsWordNode(): void - { - $result = $this->createParser()->parse(':startingWithColon'); + $this->assertInstanceOf(Field::class, $nodes[0]); + /** @var Field $field1 */ + $field1 = $nodes[0]; + $this->assertEquals('amount', $field1->getOperator()); + $this->assertEquals('100', $field1->getValue()); - $this->assertIsArray($result); - $this->assertCount(1, $result); - $this->assertInstanceOf(Word::class, $result[0]); - $this->assertEquals(':startingWithColon', $result[0]->getValue()); + $this->assertInstanceOf(Subquery::class, $nodes[1]); + /** @var Subquery $innerSubquery */ + $innerSubquery = $nodes[1]; + $subNodes = $innerSubquery->getNodes(); + $this->assertCount(2, $subNodes); + + $this->assertInstanceOf(Field::class, $subNodes[0]); + /** @var Field $field2 */ + $field2 = $subNodes[0]; + $this->assertEquals('description_contains', $field2->getOperator()); + $this->assertEquals('test payment', $field2->getValue()); + + $this->assertInstanceOf(Field::class, $subNodes[1]); + /** @var Field $field3 */ + $field3 = $subNodes[1]; + $this->assertTrue($field3->isProhibited()); + $this->assertEquals('has_attachments', $field3->getOperator()); + $this->assertEquals('true', $field3->getValue()); } public function testGivenComplexNestedSubqueriesWhenParsingQueryThenReturnsCorrectNodes(): void @@ -282,54 +244,178 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase $this->assertIsArray($result); $this->assertCount(2, $result); - // Test the first word node $this->assertInstanceOf(Word::class, $result[0]); - $this->assertEquals('shopping', $result[0]->getValue()); + /** @var Word $word */ + $word = $result[0]; + $this->assertEquals('shopping', $word->getValue()); - // Test first level subquery $this->assertInstanceOf(Subquery::class, $result[1]); /** @var Subquery $firstLevelSubquery */ $firstLevelSubquery = $result[1]; $level1Nodes = $firstLevelSubquery->getNodes(); $this->assertCount(3, $level1Nodes); - // Test field in first level $this->assertInstanceOf(Field::class, $level1Nodes[0]); - $this->assertEquals('amount', $level1Nodes[0]->getOperator()); - $this->assertEquals('50', $level1Nodes[0]->getValue()); + /** @var Field $field1 */ + $field1 = $level1Nodes[0]; + $this->assertEquals('amount', $field1->getOperator()); + $this->assertEquals('50', $field1->getValue()); - // Test word in first level $this->assertInstanceOf(Word::class, $level1Nodes[1]); - $this->assertEquals('market', $level1Nodes[1]->getValue()); + /** @var Word $word2 */ + $word2 = $level1Nodes[1]; + $this->assertEquals('market', $word2->getValue()); - // Test second level subquery $this->assertInstanceOf(Subquery::class, $level1Nodes[2]); - $level2Nodes = $level1Nodes[2]->getNodes(); + /** @var Subquery $secondLevelSubquery */ + $secondLevelSubquery = $level1Nodes[2]; + $level2Nodes = $secondLevelSubquery->getNodes(); $this->assertCount(4, $level2Nodes); - // Test prohibited field in second level $this->assertInstanceOf(Field::class, $level2Nodes[0]); - $this->assertTrue($level2Nodes[0]->isProhibited()); - $this->assertEquals('category', $level2Nodes[0]->getOperator()); - $this->assertEquals('food', $level2Nodes[0]->getValue()); + /** @var Field $field2 */ + $field2 = $level2Nodes[0]; + $this->assertTrue($field2->isProhibited()); + $this->assertEquals('category', $field2->getOperator()); + $this->assertEquals('food', $field2->getValue()); - // Test word in second level $this->assertInstanceOf(Word::class, $level2Nodes[1]); - $this->assertEquals('word', $level2Nodes[1]->getValue()); + /** @var Word $word3 */ + $word3 = $level2Nodes[1]; + $this->assertEquals('word', $word3->getValue()); - // Test field with quoted value in second level $this->assertInstanceOf(Field::class, $level2Nodes[2]); - $this->assertEquals('description', $level2Nodes[2]->getOperator()); - $this->assertEquals('test phrase', $level2Nodes[2]->getValue()); + /** @var Field $field3 */ + $field3 = $level2Nodes[2]; + $this->assertEquals('description', $field3->getOperator()); + $this->assertEquals('test phrase', $field3->getValue()); - // Test third level subquery $this->assertInstanceOf(Subquery::class, $level2Nodes[3]); - $level3Nodes = $level2Nodes[3]->getNodes(); + /** @var Subquery $thirdLevelSubquery */ + $thirdLevelSubquery = $level2Nodes[3]; + $level3Nodes = $thirdLevelSubquery->getNodes(); $this->assertCount(1, $level3Nodes); - // Test field in third level $this->assertInstanceOf(Field::class, $level3Nodes[0]); - $this->assertEquals('has_notes', $level3Nodes[0]->getOperator()); - $this->assertEquals('true', $level3Nodes[0]->getValue()); + /** @var Field $field4 */ + $field4 = $level3Nodes[0]; + $this->assertEquals('has_notes', $field4->getOperator()); + $this->assertEquals('true', $field4->getValue()); + } + + public function testGivenProhibitedWordWhenParsingQueryThenReturnsWordNode(): void + { + $result = $this->createParser()->parse('-groceries'); + + $this->assertIsArray($result); + $this->assertCount(1, $result); + $this->assertInstanceOf(Word::class, $result[0]); + /** @var Word $word */ + $word = $result[0]; + $this->assertTrue($word->isProhibited()); + $this->assertEquals('groceries', $word->getValue()); + } + + public function testGivenQuotedWordWhenParsingQueryThenReturnsWordNode(): void + { + $result = $this->createParser()->parse('"test phrase"'); + + $this->assertIsArray($result); + $this->assertCount(1, $result); + $this->assertInstanceOf(Word::class, $result[0]); + /** @var Word $word */ + $word = $result[0]; + $this->assertEquals('test phrase', $word->getValue()); + } + + public function testGivenProhibitedQuotedWordWhenParsingQueryThenReturnsWordNode(): void + { + $result = $this->createParser()->parse('-"test phrase"'); + + $this->assertIsArray($result); + $this->assertCount(1, $result); + $this->assertInstanceOf(Word::class, $result[0]); + /** @var Word $word */ + $word = $result[0]; + $this->assertTrue($word->isProhibited()); + $this->assertEquals('test phrase', $word->getValue()); + } + + public function testGivenMultipleFieldsWhenParsingQueryThenReturnsFieldNodes(): void + { + $result = $this->createParser()->parse('amount:100 category:food description:"test phrase"'); + + $this->assertIsArray($result); + $this->assertCount(3, $result); + + $this->assertInstanceOf(Field::class, $result[0]); + /** @var Field $field1 */ + $field1 = $result[0]; + $this->assertEquals('amount', $field1->getOperator()); + $this->assertEquals('100', $field1->getValue()); + + $this->assertInstanceOf(Field::class, $result[1]); + /** @var Field $field2 */ + $field2 = $result[1]; + $this->assertEquals('category', $field2->getOperator()); + $this->assertEquals('food', $field2->getValue()); + + $this->assertInstanceOf(Field::class, $result[2]); + /** @var Field $field3 */ + $field3 = $result[2]; + $this->assertEquals('description', $field3->getOperator()); + $this->assertEquals('test phrase', $field3->getValue()); + } + + public function testGivenProhibitedSubqueryWhenParsingQueryThenReturnsSubqueryNode(): void + { + $result = $this->createParser()->parse('-(amount:100 category:food)'); + + $this->assertIsArray($result); + $this->assertCount(1, $result); + $this->assertInstanceOf(Subquery::class, $result[0]); + /** @var Subquery $subquery */ + $subquery = $result[0]; + $this->assertTrue($subquery->isProhibited()); + + $nodes = $subquery->getNodes(); + $this->assertCount(2, $nodes); + + $this->assertInstanceOf(Field::class, $nodes[0]); + /** @var Field $field1 */ + $field1 = $nodes[0]; + $this->assertEquals('amount', $field1->getOperator()); + $this->assertEquals('100', $field1->getValue()); + + $this->assertInstanceOf(Field::class, $nodes[1]); + /** @var Field $field2 */ + $field2 = $nodes[1]; + $this->assertEquals('category', $field2->getOperator()); + $this->assertEquals('food', $field2->getValue()); + } + + public function testGivenWordWithMultipleSpacesWhenParsingQueryThenRetainsSpaces(): void + { + $result = $this->createParser()->parse('"multiple spaces"'); + + $this->assertIsArray($result); + $this->assertCount(1, $result); + $this->assertInstanceOf(Word::class, $result[0]); + /** @var Word $word */ + $word = $result[0]; + $this->assertEquals('multiple spaces', $word->getValue()); + } + + public function testGivenFieldWithMultipleSpacesInValueWhenParsingQueryThenRetainsSpaces(): void + { + $result = $this->createParser()->parse('description:"multiple spaces here"'); + + $this->assertIsArray($result); + $this->assertCount(1, $result); + $this->assertInstanceOf(Field::class, $result[0]); + /** @var Field $field */ + $field = $result[0]; + $this->assertEquals('description', $field->getOperator()); + $this->assertEquals('multiple spaces here', $field->getValue()); } } From 0eca1c8d039f9ff19cc5f91f967bb5b95dd0f57a Mon Sep 17 00:00:00 2001 From: Sobuno Date: Wed, 1 Jan 2025 04:29:31 +0100 Subject: [PATCH 07/27] Fix test --- tests/unit/Support/Search/QueryParserParseQueryTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/Support/Search/QueryParserParseQueryTest.php b/tests/unit/Support/Search/QueryParserParseQueryTest.php index f05301f29c..a5c4e85963 100644 --- a/tests/unit/Support/Search/QueryParserParseQueryTest.php +++ b/tests/unit/Support/Search/QueryParserParseQueryTest.php @@ -2,7 +2,7 @@ namespace Tests\unit\Support\Search; -use FireflyIII\Support\Search\QueryParser2; +use FireflyIII\Support\Search\QueryParser; use FireflyIII\Support\Search\QueryParserInterface; use Tests\unit\Support\Search\AbstractQueryParserInterfaceParseQueryTest; @@ -20,6 +20,6 @@ final class QueryParserParseQueryTest extends AbstractQueryParserInterfaceParseQ { protected function createParser(): QueryParserInterface { - return new QueryParser2(); + return new QueryParser(); } } From 631ed4956aa758c9a592d3d13735aa829394ab32 Mon Sep 17 00:00:00 2001 From: Sobuno Date: Wed, 1 Jan 2025 05:08:01 +0100 Subject: [PATCH 08/27] Handle right parenthesis behaviour correctly --- app/Support/Search/QueryParser.php | 74 ++++++++++++++----- ...ractQueryParserInterfaceParseQueryTest.php | 56 ++++++++++++++ 2 files changed, 112 insertions(+), 18 deletions(-) diff --git a/app/Support/Search/QueryParser.php b/app/Support/Search/QueryParser.php index e25e7f2afe..9005671c71 100644 --- a/app/Support/Search/QueryParser.php +++ b/app/Support/Search/QueryParser.php @@ -5,7 +5,25 @@ declare(strict_types=1); namespace FireflyIII\Support\Search; /** - * Query parser class + * Represents a result from parsing a query node + * + * Contains the parsed node and a flag indicating if this is the end of the query. + * Used to handle subquery parsing and termination. + */ +class NodeResult +{ + public function __construct( + public readonly ?Node $node, + public readonly bool $isQueryEnd + ) { + } +} + + +/** + * Single-pass parser that processes query strings into structured nodes. + * Scans each character once (O(n)) to build field searches, quoted strings, + * prohibited terms and nested subqueries without backtracking. */ class QueryParser implements QueryParserInterface { @@ -16,23 +34,26 @@ class QueryParser implements QueryParserInterface { $this->query = $query; $this->position = 0; - return $this->parseQuery(); + return $this->parseQuery(false); } - private function parseQuery(): array + private function parseQuery(bool $isSubquery): array { $nodes = []; - $token = $this->buildNextNode(); + $nodeResult = $this->buildNextNode($isSubquery); - while ($token !== null) { - $nodes[] = $token; - $token = $this->buildNextNode(); + while ($nodeResult->node !== null) { + $nodes[] = $nodeResult->node; + if($nodeResult->isQueryEnd) { + break; + } + $nodeResult = $this->buildNextNode($isSubquery); } return $nodes; } - private function buildNextNode(): ?Node + private function buildNextNode(bool $isSubquery): NodeResult { $tokenUnderConstruction = ''; $inQuotes = false; @@ -44,13 +65,16 @@ class QueryParser implements QueryParserInterface // If we're in a quoted string, we treat all characters except another quote as ordinary characters if ($inQuotes) { - if($char !== '"') { + if ($char !== '"') { $tokenUnderConstruction .= $char; $this->position++; continue; } else { $this->position++; - return $this->createNode($tokenUnderConstruction, $fieldName, $prohibited); + return new NodeResult( + $this->createNode($tokenUnderConstruction, $fieldName, $prohibited), + false + ); } } @@ -79,7 +103,10 @@ class QueryParser implements QueryParserInterface if ($tokenUnderConstruction === '') { // A left parentheses at the beginning of a token indicates the start of a subquery $this->position++; - return new Subquery($this->parseQuery(), $prohibited); + return new NodeResult( + new Subquery($this->parseQuery(true), $prohibited), + false + ); } else { // In any other location, it's just a normal character $tokenUnderConstruction .= $char; @@ -87,12 +114,20 @@ class QueryParser implements QueryParserInterface break; case ')': - if ($tokenUnderConstruction !== '') { + // A right parentheses while in a subquery means the subquery ended, + // thus also signaling the end of any node currently being built + if ($isSubquery) { $this->position++; - return $this->createNode($tokenUnderConstruction, $fieldName, $prohibited); + return new NodeResult( + $tokenUnderConstruction !== '' + ? $this->createNode($tokenUnderConstruction, $fieldName, $prohibited) + : null, + true + ); } - $this->position++; - return null; + // In any other location, it's just a normal character + $tokenUnderConstruction .= $char; + break; case ':': @@ -110,7 +145,10 @@ class QueryParser implements QueryParserInterface // A space indicates the end of a token construction if non-empty, otherwise it's just ignored if ($tokenUnderConstruction !== '') { $this->position++; - return $this->createNode($tokenUnderConstruction, $fieldName, $prohibited); + return new NodeResult( + $this->createNode($tokenUnderConstruction, $fieldName, $prohibited), + false + ); } break; @@ -121,9 +159,9 @@ class QueryParser implements QueryParserInterface $this->position++; } - return $fieldName !== '' || $tokenUnderConstruction !== '' + return new NodeResult($tokenUnderConstruction !== '' || $fieldName !== '' ? $this->createNode($tokenUnderConstruction, $fieldName, $prohibited) - : null; + : null, true); } private function createNode(string $token, string $fieldName, bool $prohibited): Node diff --git a/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php b/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php index 06e67a1713..6e44903321 100644 --- a/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php +++ b/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php @@ -418,4 +418,60 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase $this->assertEquals('description', $field->getOperator()); $this->assertEquals('multiple spaces here', $field->getValue()); } + + public function testGivenUnmatchedRightParenthesisWhenParsingQueryThenTreatsAsCharacter(): void + { + $result = $this->createParser()->parse('test)word'); + + $this->assertIsArray($result); + $this->assertCount(1, $result); + $this->assertInstanceOf(Word::class, $result[0]); + /** @var Word $word */ + $word = $result[0]; + $this->assertEquals('test)word', $word->getValue()); + } + + public function testGivenUnmatchedRightParenthesisInFieldWhenParsingQueryThenTreatsAsCharacter(): void + { + $result = $this->createParser()->parse('description:test)phrase'); + + $this->assertIsArray($result); + $this->assertCount(1, $result); + $this->assertInstanceOf(Field::class, $result[0]); + /** @var Field $field */ + $field = $result[0]; + $this->assertEquals('description', $field->getOperator()); + $this->assertEquals('test)phrase', $field->getValue()); + } + + public function testGivenSubqueryFollowedByWordWhenParsingQueryThenReturnsCorrectNodes(): void + { + $result = $this->createParser()->parse('(amount:100 category:food) shopping'); + + $this->assertIsArray($result); + $this->assertCount(2, $result); + + $this->assertInstanceOf(Subquery::class, $result[0]); + /** @var Subquery $subquery */ + $subquery = $result[0]; + $nodes = $subquery->getNodes(); + $this->assertCount(2, $nodes); + + $this->assertInstanceOf(Field::class, $nodes[0]); + /** @var Field $field1 */ + $field1 = $nodes[0]; + $this->assertEquals('amount', $field1->getOperator()); + $this->assertEquals('100', $field1->getValue()); + + $this->assertInstanceOf(Field::class, $nodes[1]); + /** @var Field $field2 */ + $field2 = $nodes[1]; + $this->assertEquals('category', $field2->getOperator()); + $this->assertEquals('food', $field2->getValue()); + + $this->assertInstanceOf(Word::class, $result[1]); + /** @var Word $word */ + $word = $result[1]; + $this->assertEquals('shopping', $word->getValue()); + } } From 50e07d422f5dcce76c93c412b0e92d995bc2948d Mon Sep 17 00:00:00 2001 From: Sobuno Date: Wed, 1 Jan 2025 05:13:12 +0100 Subject: [PATCH 09/27] Small QoL additions --- app/Support/Search/QueryParser.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/Support/Search/QueryParser.php b/app/Support/Search/QueryParser.php index 9005671c71..19a1c678ca 100644 --- a/app/Support/Search/QueryParser.php +++ b/app/Support/Search/QueryParser.php @@ -14,7 +14,7 @@ class NodeResult { public function __construct( public readonly ?Node $node, - public readonly bool $isQueryEnd + public readonly bool $isSubqueryEnd ) { } } @@ -30,6 +30,7 @@ class QueryParser implements QueryParserInterface private string $query; private int $position = 0; + /** @return Node[] */ public function parse(string $query): array { $this->query = $query; @@ -37,6 +38,7 @@ class QueryParser implements QueryParserInterface return $this->parseQuery(false); } + /** @return Node[] */ private function parseQuery(bool $isSubquery): array { $nodes = []; @@ -44,7 +46,7 @@ class QueryParser implements QueryParserInterface while ($nodeResult->node !== null) { $nodes[] = $nodeResult->node; - if($nodeResult->isQueryEnd) { + if($nodeResult->isSubqueryEnd) { break; } $nodeResult = $this->buildNextNode($isSubquery); From b3f374f4ea795d662640d5cdc547712d3cacaf92 Mon Sep 17 00:00:00 2001 From: Sobuno Date: Wed, 1 Jan 2025 05:27:13 +0100 Subject: [PATCH 10/27] Introduce assertion helpers to make tests more readable --- ...ractQueryParserInterfaceParseQueryTest.php | 359 ++++++------------ 1 file changed, 108 insertions(+), 251 deletions(-) diff --git a/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php b/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php index 6e44903321..3c44014535 100644 --- a/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php +++ b/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php @@ -28,6 +28,7 @@ use FireflyIII\Support\Search\Field; use FireflyIII\Support\Search\QueryParserInterface; use FireflyIII\Support\Search\Word; use FireflyIII\Support\Search\Subquery; +use FireflyIII\Support\Search\Node; use Tests\integration\TestCase; abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase @@ -58,12 +59,7 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase $this->assertIsArray($result); $this->assertCount(1, $result); - $this->assertInstanceOf(Field::class, $result[0]); - /** @var Field $field */ - $field = $result[0]; - $this->assertTrue($field->isProhibited()); - $this->assertEquals('amount', $field->getOperator()); - $this->assertEquals('100', $field->getValue()); + $this->assertIsField($result[0], 'amount', '100', true); } public function testGivenSimpleWordWhenParsingQueryThenReturnsWordNode(): void @@ -72,10 +68,7 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase $this->assertIsArray($result); $this->assertCount(1, $result); - $this->assertInstanceOf(Word::class, $result[0]); - /** @var Word $word */ - $word = $result[0]; - $this->assertEquals('groceries', $word->getValue()); + $this->assertIsWord($result[0], 'groceries'); } public function testGivenMultipleWordsWhenParsingQueryThenReturnsWordNodes(): void @@ -84,21 +77,9 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase $this->assertIsArray($result); $this->assertCount(3, $result); - - $this->assertInstanceOf(Word::class, $result[0]); - /** @var Word $word1 */ - $word1 = $result[0]; - $this->assertEquals('groceries', $word1->getValue()); - - $this->assertInstanceOf(Word::class, $result[1]); - /** @var Word $word2 */ - $word2 = $result[1]; - $this->assertEquals('shopping', $word2->getValue()); - - $this->assertInstanceOf(Word::class, $result[2]); - /** @var Word $word3 */ - $word3 = $result[2]; - $this->assertEquals('market', $word3->getValue()); + $this->assertIsWord($result[0], 'groceries'); + $this->assertIsWord($result[1], 'shopping'); + $this->assertIsWord($result[2], 'market'); } public function testGivenMixedWordsAndOperatorsWhenParsingQueryThenReturnsCorrectNodes(): void @@ -107,33 +88,18 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase $this->assertIsArray($result); $this->assertCount(3, $result); - - $this->assertInstanceOf(Word::class, $result[0]); - /** @var Word $word1 */ - $word1 = $result[0]; - $this->assertEquals('groceries', $word1->getValue()); - - $this->assertInstanceOf(Field::class, $result[1]); - /** @var Field $field */ - $field = $result[1]; - $this->assertEquals('amount', $field->getOperator()); - $this->assertEquals('50', $field->getValue()); - - $this->assertInstanceOf(Word::class, $result[2]); - /** @var Word $word2 */ - $word2 = $result[2]; - $this->assertEquals('shopping', $word2->getValue()); + $this->assertIsWord($result[0], 'groceries'); + $this->assertIsField($result[1], 'amount', '50'); + $this->assertIsWord($result[2], 'shopping'); } public function testGivenQuotedValueWithSpacesWhenParsingQueryThenReturnsFieldNode(): void { $result = $this->createParser()->parse('description_contains:"shopping at market"'); - $this->assertInstanceOf(Field::class, $result[0]); - /** @var Field $field */ - $field = $result[0]; - $this->assertEquals('description_contains', $field->getOperator()); - $this->assertEquals('shopping at market', $field->getValue()); + $this->assertIsArray($result); + $this->assertCount(1, $result); + $this->assertIsField($result[0], 'description_contains', 'shopping at market'); } public function testGivenSubqueryAfterFieldValueWhenParsingQueryThenReturnsCorrectNodes(): void @@ -142,30 +108,13 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase $this->assertIsArray($result); $this->assertCount(2, $result); + $this->assertIsField($result[0], 'amount', '100'); - $this->assertInstanceOf(Field::class, $result[0]); - /** @var Field $field */ - $field = $result[0]; - $this->assertEquals('amount', $field->getOperator()); - $this->assertEquals('100', $field->getValue()); - - $this->assertInstanceOf(Subquery::class, $result[1]); - /** @var Subquery $subquery */ - $subquery = $result[1]; - $nodes = $subquery->getNodes(); - $this->assertCount(2, $nodes); - - $this->assertInstanceOf(Field::class, $nodes[0]); - /** @var Field $field1 */ - $field1 = $nodes[0]; - $this->assertEquals('description', $field1->getOperator()); - $this->assertEquals('market', $field1->getValue()); - - $this->assertInstanceOf(Field::class, $nodes[1]); - /** @var Field $field2 */ - $field2 = $nodes[1]; - $this->assertEquals('category', $field2->getOperator()); - $this->assertEquals('food', $field2->getValue()); + $expectedNodes = [ + new Field('description', 'market'), + new Field('category', 'food') + ]; + $this->assertIsSubquery($result[1], $expectedNodes); } public function testGivenWordFollowedBySubqueryWhenParsingQueryThenReturnsCorrectNodes(): void @@ -175,28 +124,13 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase $this->assertIsArray($result); $this->assertCount(2, $result); - $this->assertInstanceOf(Word::class, $result[0]); - /** @var Word $word */ - $word = $result[0]; - $this->assertEquals('groceries', $word->getValue()); + $this->assertIsWord($result[0], 'groceries'); - $this->assertInstanceOf(Subquery::class, $result[1]); - /** @var Subquery $subquery */ - $subquery = $result[1]; - $nodes = $subquery->getNodes(); - $this->assertCount(2, $nodes); - - $this->assertInstanceOf(Field::class, $nodes[0]); - /** @var Field $field1 */ - $field1 = $nodes[0]; - $this->assertEquals('amount', $field1->getOperator()); - $this->assertEquals('100', $field1->getValue()); - - $this->assertInstanceOf(Field::class, $nodes[1]); - /** @var Field $field2 */ - $field2 = $nodes[1]; - $this->assertEquals('description_contains', $field2->getOperator()); - $this->assertEquals('test', $field2->getValue()); + $expectedNodes = [ + new Field('amount', '100'), + new Field('description_contains', 'test') + ]; + $this->assertIsSubquery($result[1], $expectedNodes); } public function testGivenNestedSubqueryWhenParsingQueryThenReturnsSubqueryNode(): void @@ -205,36 +139,16 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase $this->assertIsArray($result); $this->assertCount(1, $result); - $this->assertInstanceOf(Subquery::class, $result[0]); - /** @var Subquery $outerSubquery */ - $outerSubquery = $result[0]; - $nodes = $outerSubquery->getNodes(); - $this->assertCount(2, $nodes); - $this->assertInstanceOf(Field::class, $nodes[0]); - /** @var Field $field1 */ - $field1 = $nodes[0]; - $this->assertEquals('amount', $field1->getOperator()); - $this->assertEquals('100', $field1->getValue()); - - $this->assertInstanceOf(Subquery::class, $nodes[1]); - /** @var Subquery $innerSubquery */ - $innerSubquery = $nodes[1]; - $subNodes = $innerSubquery->getNodes(); - $this->assertCount(2, $subNodes); - - $this->assertInstanceOf(Field::class, $subNodes[0]); - /** @var Field $field2 */ - $field2 = $subNodes[0]; - $this->assertEquals('description_contains', $field2->getOperator()); - $this->assertEquals('test payment', $field2->getValue()); - - $this->assertInstanceOf(Field::class, $subNodes[1]); - /** @var Field $field3 */ - $field3 = $subNodes[1]; - $this->assertTrue($field3->isProhibited()); - $this->assertEquals('has_attachments', $field3->getOperator()); - $this->assertEquals('true', $field3->getValue()); + $innerSubqueryNodes = [ + new Field('description_contains', 'test payment'), + new Field('has_attachments', 'true', true) + ]; + $outerSubqueryNodes = [ + new Field('amount', '100'), + new Subquery($innerSubqueryNodes) + ]; + $this->assertIsSubquery($result[0], $outerSubqueryNodes); } public function testGivenComplexNestedSubqueriesWhenParsingQueryThenReturnsCorrectNodes(): void @@ -244,63 +158,22 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase $this->assertIsArray($result); $this->assertCount(2, $result); - $this->assertInstanceOf(Word::class, $result[0]); - /** @var Word $word */ - $word = $result[0]; - $this->assertEquals('shopping', $word->getValue()); + $this->assertIsWord($result[0], 'shopping'); - $this->assertInstanceOf(Subquery::class, $result[1]); - /** @var Subquery $firstLevelSubquery */ - $firstLevelSubquery = $result[1]; - $level1Nodes = $firstLevelSubquery->getNodes(); - $this->assertCount(3, $level1Nodes); + $expectedLevel2 = [ + new Field('category', 'food', true), + new Word('word'), + new Field('description', 'test phrase'), + new Subquery([new Field('has_notes', 'true')]) + ]; - $this->assertInstanceOf(Field::class, $level1Nodes[0]); - /** @var Field $field1 */ - $field1 = $level1Nodes[0]; - $this->assertEquals('amount', $field1->getOperator()); - $this->assertEquals('50', $field1->getValue()); + $expectedLevel1 = [ + new Field('amount', '50'), + new Word('market'), + new Subquery($expectedLevel2) + ]; - $this->assertInstanceOf(Word::class, $level1Nodes[1]); - /** @var Word $word2 */ - $word2 = $level1Nodes[1]; - $this->assertEquals('market', $word2->getValue()); - - $this->assertInstanceOf(Subquery::class, $level1Nodes[2]); - /** @var Subquery $secondLevelSubquery */ - $secondLevelSubquery = $level1Nodes[2]; - $level2Nodes = $secondLevelSubquery->getNodes(); - $this->assertCount(4, $level2Nodes); - - $this->assertInstanceOf(Field::class, $level2Nodes[0]); - /** @var Field $field2 */ - $field2 = $level2Nodes[0]; - $this->assertTrue($field2->isProhibited()); - $this->assertEquals('category', $field2->getOperator()); - $this->assertEquals('food', $field2->getValue()); - - $this->assertInstanceOf(Word::class, $level2Nodes[1]); - /** @var Word $word3 */ - $word3 = $level2Nodes[1]; - $this->assertEquals('word', $word3->getValue()); - - $this->assertInstanceOf(Field::class, $level2Nodes[2]); - /** @var Field $field3 */ - $field3 = $level2Nodes[2]; - $this->assertEquals('description', $field3->getOperator()); - $this->assertEquals('test phrase', $field3->getValue()); - - $this->assertInstanceOf(Subquery::class, $level2Nodes[3]); - /** @var Subquery $thirdLevelSubquery */ - $thirdLevelSubquery = $level2Nodes[3]; - $level3Nodes = $thirdLevelSubquery->getNodes(); - $this->assertCount(1, $level3Nodes); - - $this->assertInstanceOf(Field::class, $level3Nodes[0]); - /** @var Field $field4 */ - $field4 = $level3Nodes[0]; - $this->assertEquals('has_notes', $field4->getOperator()); - $this->assertEquals('true', $field4->getValue()); + $this->assertIsSubquery($result[1], $expectedLevel1); } public function testGivenProhibitedWordWhenParsingQueryThenReturnsWordNode(): void @@ -334,11 +207,7 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase $this->assertIsArray($result); $this->assertCount(1, $result); - $this->assertInstanceOf(Word::class, $result[0]); - /** @var Word $word */ - $word = $result[0]; - $this->assertTrue($word->isProhibited()); - $this->assertEquals('test phrase', $word->getValue()); + $this->assertIsWord($result[0], 'test phrase', true); } public function testGivenMultipleFieldsWhenParsingQueryThenReturnsFieldNodes(): void @@ -347,24 +216,9 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase $this->assertIsArray($result); $this->assertCount(3, $result); - - $this->assertInstanceOf(Field::class, $result[0]); - /** @var Field $field1 */ - $field1 = $result[0]; - $this->assertEquals('amount', $field1->getOperator()); - $this->assertEquals('100', $field1->getValue()); - - $this->assertInstanceOf(Field::class, $result[1]); - /** @var Field $field2 */ - $field2 = $result[1]; - $this->assertEquals('category', $field2->getOperator()); - $this->assertEquals('food', $field2->getValue()); - - $this->assertInstanceOf(Field::class, $result[2]); - /** @var Field $field3 */ - $field3 = $result[2]; - $this->assertEquals('description', $field3->getOperator()); - $this->assertEquals('test phrase', $field3->getValue()); + $this->assertIsField($result[0], 'amount', '100'); + $this->assertIsField($result[1], 'category', 'food'); + $this->assertIsField($result[2], 'description', 'test phrase'); } public function testGivenProhibitedSubqueryWhenParsingQueryThenReturnsSubqueryNode(): void @@ -373,25 +227,12 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase $this->assertIsArray($result); $this->assertCount(1, $result); - $this->assertInstanceOf(Subquery::class, $result[0]); - /** @var Subquery $subquery */ - $subquery = $result[0]; - $this->assertTrue($subquery->isProhibited()); - $nodes = $subquery->getNodes(); - $this->assertCount(2, $nodes); - - $this->assertInstanceOf(Field::class, $nodes[0]); - /** @var Field $field1 */ - $field1 = $nodes[0]; - $this->assertEquals('amount', $field1->getOperator()); - $this->assertEquals('100', $field1->getValue()); - - $this->assertInstanceOf(Field::class, $nodes[1]); - /** @var Field $field2 */ - $field2 = $nodes[1]; - $this->assertEquals('category', $field2->getOperator()); - $this->assertEquals('food', $field2->getValue()); + $expectedNodes = [ + new Field('amount', '100'), + new Field('category', 'food') + ]; + $this->assertIsSubquery($result[0], $expectedNodes, true); } public function testGivenWordWithMultipleSpacesWhenParsingQueryThenRetainsSpaces(): void @@ -400,10 +241,7 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase $this->assertIsArray($result); $this->assertCount(1, $result); - $this->assertInstanceOf(Word::class, $result[0]); - /** @var Word $word */ - $word = $result[0]; - $this->assertEquals('multiple spaces', $word->getValue()); + $this->assertIsWord($result[0], 'multiple spaces'); } public function testGivenFieldWithMultipleSpacesInValueWhenParsingQueryThenRetainsSpaces(): void @@ -412,11 +250,7 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase $this->assertIsArray($result); $this->assertCount(1, $result); - $this->assertInstanceOf(Field::class, $result[0]); - /** @var Field $field */ - $field = $result[0]; - $this->assertEquals('description', $field->getOperator()); - $this->assertEquals('multiple spaces here', $field->getValue()); + $this->assertIsField($result[0], 'description', 'multiple spaces here'); } public function testGivenUnmatchedRightParenthesisWhenParsingQueryThenTreatsAsCharacter(): void @@ -425,10 +259,7 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase $this->assertIsArray($result); $this->assertCount(1, $result); - $this->assertInstanceOf(Word::class, $result[0]); - /** @var Word $word */ - $word = $result[0]; - $this->assertEquals('test)word', $word->getValue()); + $this->assertIsWord($result[0], 'test)word'); } public function testGivenUnmatchedRightParenthesisInFieldWhenParsingQueryThenTreatsAsCharacter(): void @@ -437,11 +268,7 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase $this->assertIsArray($result); $this->assertCount(1, $result); - $this->assertInstanceOf(Field::class, $result[0]); - /** @var Field $field */ - $field = $result[0]; - $this->assertEquals('description', $field->getOperator()); - $this->assertEquals('test)phrase', $field->getValue()); + $this->assertIsField($result[0], 'description', 'test)phrase'); } public function testGivenSubqueryFollowedByWordWhenParsingQueryThenReturnsCorrectNodes(): void @@ -451,27 +278,57 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase $this->assertIsArray($result); $this->assertCount(2, $result); - $this->assertInstanceOf(Subquery::class, $result[0]); - /** @var Subquery $subquery */ - $subquery = $result[0]; - $nodes = $subquery->getNodes(); - $this->assertCount(2, $nodes); + $expectedNodes = [ + new Field('amount', '100'), + new Field('category', 'food') + ]; + $this->assertIsSubquery($result[0], $expectedNodes); + $this->assertIsWord($result[1], 'shopping'); + } - $this->assertInstanceOf(Field::class, $nodes[0]); - /** @var Field $field1 */ - $field1 = $nodes[0]; - $this->assertEquals('amount', $field1->getOperator()); - $this->assertEquals('100', $field1->getValue()); + private function assertIsWord(Node $node, string $expectedValue, bool $prohibited = false): void + { + $this->assertInstanceOf(Word::class, $node); + /** @var Word $node */ + $this->assertEquals($expectedValue, $node->getValue()); + if ($prohibited) { + $this->assertTrue($node->isProhibited()); + } + } - $this->assertInstanceOf(Field::class, $nodes[1]); - /** @var Field $field2 */ - $field2 = $nodes[1]; - $this->assertEquals('category', $field2->getOperator()); - $this->assertEquals('food', $field2->getValue()); + private function assertIsField( + Node $node, + string $expectedOperator, + string $expectedValue, + bool $prohibited = false + ): void { + $this->assertInstanceOf(Field::class, $node); + /** @var Field $node */ + $this->assertEquals($expectedOperator, $node->getOperator()); + $this->assertEquals($expectedValue, $node->getValue()); + if ($prohibited) { + $this->assertTrue($node->isProhibited()); + } + } - $this->assertInstanceOf(Word::class, $result[1]); - /** @var Word $word */ - $word = $result[1]; - $this->assertEquals('shopping', $word->getValue()); + private function assertIsSubquery(Node $node, array $expectedNodes, bool $prohibited = false): void + { + $this->assertInstanceOf(Subquery::class, $node); + /** @var Subquery $node */ + $this->assertCount(count($expectedNodes), $node->getNodes()); + if ($prohibited) { + $this->assertTrue($node->isProhibited()); + } + + foreach ($expectedNodes as $index => $expected) { + $actual = $node->getNodes()[$index]; + if ($expected instanceof Word) { + $this->assertIsWord($actual, $expected->getValue(), $expected->isProhibited()); + } elseif ($expected instanceof Field) { + $this->assertIsField($actual, $expected->getOperator(), $expected->getValue(), $expected->isProhibited()); + } elseif ($expected instanceof Subquery) { + $this->assertIsSubquery($actual, $expected->getNodes(), $expected->isProhibited()); + } + } } } From 9d9dffee746880ce1191393eedc1d2a589a41e78 Mon Sep 17 00:00:00 2001 From: Sobuno Date: Wed, 1 Jan 2025 05:31:25 +0100 Subject: [PATCH 11/27] Always assert prohibited field --- .../AbstractQueryParserInterfaceParseQueryTest.php | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php b/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php index 3c44014535..ee71c819e4 100644 --- a/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php +++ b/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php @@ -291,9 +291,7 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase $this->assertInstanceOf(Word::class, $node); /** @var Word $node */ $this->assertEquals($expectedValue, $node->getValue()); - if ($prohibited) { - $this->assertTrue($node->isProhibited()); - } + $this->assertEquals($prohibited, $node->isProhibited()); } private function assertIsField( @@ -306,9 +304,7 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase /** @var Field $node */ $this->assertEquals($expectedOperator, $node->getOperator()); $this->assertEquals($expectedValue, $node->getValue()); - if ($prohibited) { - $this->assertTrue($node->isProhibited()); - } + $this->assertEquals($prohibited, $node->isProhibited()); } private function assertIsSubquery(Node $node, array $expectedNodes, bool $prohibited = false): void @@ -316,9 +312,7 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase $this->assertInstanceOf(Subquery::class, $node); /** @var Subquery $node */ $this->assertCount(count($expectedNodes), $node->getNodes()); - if ($prohibited) { - $this->assertTrue($node->isProhibited()); - } + $this->assertEquals($prohibited, $node->isProhibited()); foreach ($expectedNodes as $index => $expected) { $actual = $node->getNodes()[$index]; From 9729434926d175b0bf7e291b85c1baef1fcd8c88 Mon Sep 17 00:00:00 2001 From: Sobuno Date: Wed, 1 Jan 2025 05:36:22 +0100 Subject: [PATCH 12/27] Allow choosing QueryParser implementation --- app/Providers/SearchServiceProvider.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/app/Providers/SearchServiceProvider.php b/app/Providers/SearchServiceProvider.php index 627d9f0820..dccd90ddec 100644 --- a/app/Providers/SearchServiceProvider.php +++ b/app/Providers/SearchServiceProvider.php @@ -23,6 +23,7 @@ declare(strict_types=1); namespace FireflyIII\Providers; +use FireflyIII\Support\Search\GdbotsQueryParser; use FireflyIII\Support\Search\OperatorQuerySearch; use FireflyIII\Support\Search\QueryParser; use FireflyIII\Support\Search\QueryParserInterface; @@ -47,10 +48,13 @@ class SearchServiceProvider extends ServiceProvider { $this->app->bind( QueryParserInterface::class, - static function (Application $app) { - /** @var QueryParser $queryParser */ - $queryParser = app(QueryParser::class); - return $queryParser; + static function () { + $implementation = env('QUERY_PARSER_IMPLEMENTATION', 'default'); + + return match($implementation) { + 'new' => app(QueryParser::class), + default => app(GdbotsQueryParser::class), + }; } ); From 57b064f590a84a72d6400042037f92d4c8df1a0a Mon Sep 17 00:00:00 2001 From: Sobuno Date: Wed, 1 Jan 2025 14:18:19 +0100 Subject: [PATCH 13/27] Remove incorrect LICENSE message at top of file --- ...ractQueryParserInterfaceParseQueryTest.php | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php b/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php index ee71c819e4..47efe3a6ed 100644 --- a/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php +++ b/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php @@ -1,25 +1,5 @@ - * - * This file is part of Firefly III (https://github.com/firefly-iii). - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as - * published by the Free Software Foundation, either version 3 of the - * License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - */ - declare(strict_types=1); namespace Tests\unit\Support\Search; From 92f5cca65b8d71edeed39702b6b27ab1b50137e7 Mon Sep 17 00:00:00 2001 From: Sobuno Date: Wed, 1 Jan 2025 14:18:56 +0100 Subject: [PATCH 14/27] Add QueryParser tests to correct group --- tests/unit/Support/Search/GdbotsQueryParserParseQueryTest.php | 2 +- tests/unit/Support/Search/QueryParserParseQueryTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/Support/Search/GdbotsQueryParserParseQueryTest.php b/tests/unit/Support/Search/GdbotsQueryParserParseQueryTest.php index 897d9ecb01..8f00f7a65d 100644 --- a/tests/unit/Support/Search/GdbotsQueryParserParseQueryTest.php +++ b/tests/unit/Support/Search/GdbotsQueryParserParseQueryTest.php @@ -9,7 +9,7 @@ use Tests\unit\Support\Search\AbstractQueryParserInterfaceParseQueryTest; /** * @group unit-test * @group support - * @group navigation + * @group search * * @internal * diff --git a/tests/unit/Support/Search/QueryParserParseQueryTest.php b/tests/unit/Support/Search/QueryParserParseQueryTest.php index a5c4e85963..c85131b586 100644 --- a/tests/unit/Support/Search/QueryParserParseQueryTest.php +++ b/tests/unit/Support/Search/QueryParserParseQueryTest.php @@ -10,7 +10,7 @@ use Tests\unit\Support\Search\AbstractQueryParserInterfaceParseQueryTest; /** * @group unit-test * @group support - * @group navigation + * @group search * * @internal * From c44b827922e5843ff1351dfee0b1f9f2dd3abe3e Mon Sep 17 00:00:00 2001 From: Sobuno Date: Thu, 2 Jan 2025 20:03:39 +0100 Subject: [PATCH 15/27] Fix environment-based selection of Query Parser --- app/Providers/SearchServiceProvider.php | 4 ++-- config/search.php | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/Providers/SearchServiceProvider.php b/app/Providers/SearchServiceProvider.php index dccd90ddec..944348218d 100644 --- a/app/Providers/SearchServiceProvider.php +++ b/app/Providers/SearchServiceProvider.php @@ -48,8 +48,8 @@ class SearchServiceProvider extends ServiceProvider { $this->app->bind( QueryParserInterface::class, - static function () { - $implementation = env('QUERY_PARSER_IMPLEMENTATION', 'default'); + static function (): GdbotsQueryParser|QueryParser { + $implementation = config('search.query_parser'); return match($implementation) { 'new' => app(QueryParser::class), diff --git a/config/search.php b/config/search.php index 304cd080a0..249fabd363 100644 --- a/config/search.php +++ b/config/search.php @@ -253,4 +253,8 @@ return [ 'destination_balance_lt' => ['alias' => false, 'needs_context' => true], 'destination_balance_is' => ['alias' => false, 'needs_context' => true], ], + /** + * Which query parser to use - 'new' or 'legacy' + */ + 'query_parser' => env('QUERY_PARSER_IMPLEMENTATION', 'legacy'), ]; From 78f9f7e2dd5df5b069c0b69d22059ac87992c8ed Mon Sep 17 00:00:00 2001 From: Sobuno Date: Thu, 2 Jan 2025 20:06:11 +0100 Subject: [PATCH 16/27] Add QUERY_PARSER_IMPLEMENTATION in .env.example --- .env.example | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.env.example b/.env.example index e2e0b18720..358fec2d52 100644 --- a/.env.example +++ b/.env.example @@ -325,6 +325,12 @@ USE_RUNNING_BALANCE=false # FIREFLY_III_LAYOUT=v1 +# +# Which Query Parser implementation to use for the Search Engine and Rules +# 'new' is experimental, 'legacy' is the classic one +# +QUERY_PARSER_IMPLEMENTATION=legacy + # # Please make sure this URL matches the external URL of your Firefly III installation. # It is used to validate specific requests and to generate URLs in emails. From afc9ea08f379868e7f037938d2fb4a205e3edcba Mon Sep 17 00:00:00 2001 From: Sobuno Date: Thu, 2 Jan 2025 22:17:56 +0100 Subject: [PATCH 17/27] Rearrange structure --- app/Support/Search/OperatorQuerySearch.php | 5 + app/Support/Search/QueryParser/Field.php | 36 +++++ .../{ => QueryParser}/GdbotsQueryParser.php | 2 +- app/Support/Search/QueryParser/Node.php | 20 +++ .../Search/{ => QueryParser}/QueryParser.php | 2 +- .../QueryParser/QueryParserInterface.php | 13 ++ app/Support/Search/QueryParser/Subquery.php | 38 ++++++ app/Support/Search/QueryParser/Word.php | 29 ++++ app/Support/Search/QueryParserInterface.php | 127 ------------------ 9 files changed, 143 insertions(+), 129 deletions(-) create mode 100644 app/Support/Search/QueryParser/Field.php rename app/Support/Search/{ => QueryParser}/GdbotsQueryParser.php (98%) create mode 100644 app/Support/Search/QueryParser/Node.php rename app/Support/Search/{ => QueryParser}/QueryParser.php (99%) create mode 100644 app/Support/Search/QueryParser/QueryParserInterface.php create mode 100644 app/Support/Search/QueryParser/Subquery.php create mode 100644 app/Support/Search/QueryParser/Word.php delete mode 100644 app/Support/Search/QueryParserInterface.php diff --git a/app/Support/Search/OperatorQuerySearch.php b/app/Support/Search/OperatorQuerySearch.php index 596aedd7d0..4e9408a3a8 100644 --- a/app/Support/Search/OperatorQuerySearch.php +++ b/app/Support/Search/OperatorQuerySearch.php @@ -39,6 +39,11 @@ use FireflyIII\Repositories\Budget\BudgetRepositoryInterface; use FireflyIII\Repositories\Category\CategoryRepositoryInterface; use FireflyIII\Repositories\Tag\TagRepositoryInterface; use FireflyIII\Repositories\UserGroups\Currency\CurrencyRepositoryInterface; +use FireflyIII\Support\Search\QueryParser\QueryParserInterface; +use FireflyIII\Support\Search\QueryParser\Node; +use FireflyIII\Support\Search\QueryParser\Field; +use FireflyIII\Support\Search\QueryParser\Word; +use FireflyIII\Support\Search\QueryParser\Subquery; use FireflyIII\Support\ParseDateString; use FireflyIII\User; use Illuminate\Pagination\LengthAwarePaginator; diff --git a/app/Support/Search/QueryParser/Field.php b/app/Support/Search/QueryParser/Field.php new file mode 100644 index 0000000000..3028af9730 --- /dev/null +++ b/app/Support/Search/QueryParser/Field.php @@ -0,0 +1,36 @@ +operator = $operator; + $this->value = $value; + $this->prohibited = $prohibited; + } + + public function getOperator(): string + { + return $this->operator; + } + + public function getValue(): string + { + return $this->value; + } + + public function __toString(): string + { + return ($this->prohibited ? '-' : '') . $this->operator . ':' . $this->value; + } +} diff --git a/app/Support/Search/GdbotsQueryParser.php b/app/Support/Search/QueryParser/GdbotsQueryParser.php similarity index 98% rename from app/Support/Search/GdbotsQueryParser.php rename to app/Support/Search/QueryParser/GdbotsQueryParser.php index c8249c455a..8a56d0f6de 100644 --- a/app/Support/Search/GdbotsQueryParser.php +++ b/app/Support/Search/QueryParser/GdbotsQueryParser.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace FireflyIII\Support\Search; +namespace FireflyIII\Support\Search\QueryParser; use FireflyIII\Exceptions\FireflyException; use Gdbots\QueryParser\QueryParser as BaseQueryParser; diff --git a/app/Support/Search/QueryParser/Node.php b/app/Support/Search/QueryParser/Node.php new file mode 100644 index 0000000000..7dced2e759 --- /dev/null +++ b/app/Support/Search/QueryParser/Node.php @@ -0,0 +1,20 @@ +prohibited; + } +} diff --git a/app/Support/Search/QueryParser.php b/app/Support/Search/QueryParser/QueryParser.php similarity index 99% rename from app/Support/Search/QueryParser.php rename to app/Support/Search/QueryParser/QueryParser.php index 19a1c678ca..1d7f0658c9 100644 --- a/app/Support/Search/QueryParser.php +++ b/app/Support/Search/QueryParser/QueryParser.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace FireflyIII\Support\Search; +namespace FireflyIII\Support\Search\QueryParser; /** * Represents a result from parsing a query node diff --git a/app/Support/Search/QueryParser/QueryParserInterface.php b/app/Support/Search/QueryParser/QueryParserInterface.php new file mode 100644 index 0000000000..831baf1270 --- /dev/null +++ b/app/Support/Search/QueryParser/QueryParserInterface.php @@ -0,0 +1,13 @@ +nodes = $nodes; + $this->prohibited = $prohibited; + } + + /** + * @return Node[] + */ + public function getNodes(): array + { + return $this->nodes; + } + + + public function __toString(): string + { + return ($this->prohibited ? '-' : '') . '(' . implode(' ', array_map(fn($node) => (string)$node, $this->nodes)) . ')'; + } +} diff --git a/app/Support/Search/QueryParser/Word.php b/app/Support/Search/QueryParser/Word.php new file mode 100644 index 0000000000..d7c12b6976 --- /dev/null +++ b/app/Support/Search/QueryParser/Word.php @@ -0,0 +1,29 @@ +value = $value; + $this->prohibited = $prohibited; + } + + public function getValue(): string + { + return $this->value; + } + + public function __toString(): string + { + return ($this->prohibited ? '-' : '') . $this->value; + } +} diff --git a/app/Support/Search/QueryParserInterface.php b/app/Support/Search/QueryParserInterface.php deleted file mode 100644 index 93f88d1e75..0000000000 --- a/app/Support/Search/QueryParserInterface.php +++ /dev/null @@ -1,127 +0,0 @@ -value = $value; - $this->prohibited = $prohibited; - } - - public function getValue(): string - { - return $this->value; - } - - public function isProhibited(): bool - { - return $this->prohibited; - } - - public function __toString(): string - { - return ($this->prohibited ? '-' : '') . $this->value; - } -} - -/** - * Represents a field operator with value (e.g. amount:100) - */ -class Field extends Node -{ - private string $operator; - private string $value; - private bool $prohibited; - - public function __construct(string $operator, string $value, bool $prohibited = false) - { - $this->operator = $operator; - $this->value = $value; - $this->prohibited = $prohibited; - } - - public function getOperator(): string - { - return $this->operator; - } - - public function getValue(): string - { - return $this->value; - } - - public function isProhibited(): bool - { - return $this->prohibited; - } - - public function __toString(): string - { - return ($this->prohibited ? '-' : '') . $this->operator . ':' . $this->value; - } -} - -/** - * Represents a subquery (group of nodes) - */ -class Subquery extends Node -{ - /** @var Node[] */ - private array $nodes; - - private bool $prohibited; - - /** - * @param Node[] $nodes - */ - public function __construct(array $nodes, bool $prohibited = false) - { - $this->nodes = $nodes; - $this->prohibited = $prohibited; - } - - /** - * @return Node[] - */ - public function getNodes(): array - { - return $this->nodes; - } - - public function isProhibited(): bool - { - return $this->prohibited; - } - - public function __toString(): string - { - return ($this->prohibited ? '-' : '') . '(' . implode(' ', array_map(fn($node) => (string)$node, $this->nodes)) . ')'; - } -} From ebd30f48614858a150825947012f628ee0d064aa Mon Sep 17 00:00:00 2001 From: Sobuno Date: Thu, 2 Jan 2025 22:18:12 +0100 Subject: [PATCH 18/27] Move tests to match rearranged structure + standardize tests --- ...ractQueryParserInterfaceParseQueryTest.php | 308 ------------------ ...ractQueryParserInterfaceParseQueryTest.php | 278 ++++++++++++++++ .../GdbotsQueryParserParseQueryTest.php | 7 +- .../QueryParserParseQueryTest.php | 7 +- 4 files changed, 284 insertions(+), 316 deletions(-) delete mode 100644 tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php create mode 100644 tests/unit/Support/Search/QueryParser/AbstractQueryParserInterfaceParseQueryTest.php rename tests/unit/Support/Search/{ => QueryParser}/GdbotsQueryParserParseQueryTest.php (60%) rename tests/unit/Support/Search/{ => QueryParser}/QueryParserParseQueryTest.php (60%) diff --git a/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php b/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php deleted file mode 100644 index 47efe3a6ed..0000000000 --- a/tests/unit/Support/Search/AbstractQueryParserInterfaceParseQueryTest.php +++ /dev/null @@ -1,308 +0,0 @@ -createParser()->parse(''); - - $this->assertIsArray($result); - $this->assertEmpty($result); - } - - public function testGivenProhibitedFieldOperatorWhenParsingQueryThenReturnsFieldNode(): void - { - $result = $this->createParser()->parse('-amount:100'); - - $this->assertIsArray($result); - $this->assertCount(1, $result); - $this->assertIsField($result[0], 'amount', '100', true); - } - - public function testGivenSimpleWordWhenParsingQueryThenReturnsWordNode(): void - { - $result = $this->createParser()->parse('groceries'); - - $this->assertIsArray($result); - $this->assertCount(1, $result); - $this->assertIsWord($result[0], 'groceries'); - } - - public function testGivenMultipleWordsWhenParsingQueryThenReturnsWordNodes(): void - { - $result = $this->createParser()->parse('groceries shopping market'); - - $this->assertIsArray($result); - $this->assertCount(3, $result); - $this->assertIsWord($result[0], 'groceries'); - $this->assertIsWord($result[1], 'shopping'); - $this->assertIsWord($result[2], 'market'); - } - - public function testGivenMixedWordsAndOperatorsWhenParsingQueryThenReturnsCorrectNodes(): void - { - $result = $this->createParser()->parse('groceries amount:50 shopping'); - - $this->assertIsArray($result); - $this->assertCount(3, $result); - $this->assertIsWord($result[0], 'groceries'); - $this->assertIsField($result[1], 'amount', '50'); - $this->assertIsWord($result[2], 'shopping'); - } - - public function testGivenQuotedValueWithSpacesWhenParsingQueryThenReturnsFieldNode(): void - { - $result = $this->createParser()->parse('description_contains:"shopping at market"'); - - $this->assertIsArray($result); - $this->assertCount(1, $result); - $this->assertIsField($result[0], 'description_contains', 'shopping at market'); - } - - public function testGivenSubqueryAfterFieldValueWhenParsingQueryThenReturnsCorrectNodes(): void - { - $result = $this->createParser()->parse('amount:100 (description:"market" category:food)'); - - $this->assertIsArray($result); - $this->assertCount(2, $result); - $this->assertIsField($result[0], 'amount', '100'); - - $expectedNodes = [ - new Field('description', 'market'), - new Field('category', 'food') - ]; - $this->assertIsSubquery($result[1], $expectedNodes); - } - - public function testGivenWordFollowedBySubqueryWhenParsingQueryThenReturnsCorrectNodes(): void - { - $result = $this->createParser()->parse('groceries (amount:100 description_contains:"test")'); - - $this->assertIsArray($result); - $this->assertCount(2, $result); - - $this->assertIsWord($result[0], 'groceries'); - - $expectedNodes = [ - new Field('amount', '100'), - new Field('description_contains', 'test') - ]; - $this->assertIsSubquery($result[1], $expectedNodes); - } - - public function testGivenNestedSubqueryWhenParsingQueryThenReturnsSubqueryNode(): void - { - $result = $this->createParser()->parse('(amount:100 (description_contains:"test payment" -has_attachments:true))'); - - $this->assertIsArray($result); - $this->assertCount(1, $result); - - $innerSubqueryNodes = [ - new Field('description_contains', 'test payment'), - new Field('has_attachments', 'true', true) - ]; - $outerSubqueryNodes = [ - new Field('amount', '100'), - new Subquery($innerSubqueryNodes) - ]; - $this->assertIsSubquery($result[0], $outerSubqueryNodes); - } - - public function testGivenComplexNestedSubqueriesWhenParsingQueryThenReturnsCorrectNodes(): void - { - $result = $this->createParser()->parse('shopping (amount:50 market (-category:food word description:"test phrase" (has_notes:true)))'); - - $this->assertIsArray($result); - $this->assertCount(2, $result); - - $this->assertIsWord($result[0], 'shopping'); - - $expectedLevel2 = [ - new Field('category', 'food', true), - new Word('word'), - new Field('description', 'test phrase'), - new Subquery([new Field('has_notes', 'true')]) - ]; - - $expectedLevel1 = [ - new Field('amount', '50'), - new Word('market'), - new Subquery($expectedLevel2) - ]; - - $this->assertIsSubquery($result[1], $expectedLevel1); - } - - public function testGivenProhibitedWordWhenParsingQueryThenReturnsWordNode(): void - { - $result = $this->createParser()->parse('-groceries'); - - $this->assertIsArray($result); - $this->assertCount(1, $result); - $this->assertInstanceOf(Word::class, $result[0]); - /** @var Word $word */ - $word = $result[0]; - $this->assertTrue($word->isProhibited()); - $this->assertEquals('groceries', $word->getValue()); - } - - public function testGivenQuotedWordWhenParsingQueryThenReturnsWordNode(): void - { - $result = $this->createParser()->parse('"test phrase"'); - - $this->assertIsArray($result); - $this->assertCount(1, $result); - $this->assertInstanceOf(Word::class, $result[0]); - /** @var Word $word */ - $word = $result[0]; - $this->assertEquals('test phrase', $word->getValue()); - } - - public function testGivenProhibitedQuotedWordWhenParsingQueryThenReturnsWordNode(): void - { - $result = $this->createParser()->parse('-"test phrase"'); - - $this->assertIsArray($result); - $this->assertCount(1, $result); - $this->assertIsWord($result[0], 'test phrase', true); - } - - public function testGivenMultipleFieldsWhenParsingQueryThenReturnsFieldNodes(): void - { - $result = $this->createParser()->parse('amount:100 category:food description:"test phrase"'); - - $this->assertIsArray($result); - $this->assertCount(3, $result); - $this->assertIsField($result[0], 'amount', '100'); - $this->assertIsField($result[1], 'category', 'food'); - $this->assertIsField($result[2], 'description', 'test phrase'); - } - - public function testGivenProhibitedSubqueryWhenParsingQueryThenReturnsSubqueryNode(): void - { - $result = $this->createParser()->parse('-(amount:100 category:food)'); - - $this->assertIsArray($result); - $this->assertCount(1, $result); - - $expectedNodes = [ - new Field('amount', '100'), - new Field('category', 'food') - ]; - $this->assertIsSubquery($result[0], $expectedNodes, true); - } - - public function testGivenWordWithMultipleSpacesWhenParsingQueryThenRetainsSpaces(): void - { - $result = $this->createParser()->parse('"multiple spaces"'); - - $this->assertIsArray($result); - $this->assertCount(1, $result); - $this->assertIsWord($result[0], 'multiple spaces'); - } - - public function testGivenFieldWithMultipleSpacesInValueWhenParsingQueryThenRetainsSpaces(): void - { - $result = $this->createParser()->parse('description:"multiple spaces here"'); - - $this->assertIsArray($result); - $this->assertCount(1, $result); - $this->assertIsField($result[0], 'description', 'multiple spaces here'); - } - - public function testGivenUnmatchedRightParenthesisWhenParsingQueryThenTreatsAsCharacter(): void - { - $result = $this->createParser()->parse('test)word'); - - $this->assertIsArray($result); - $this->assertCount(1, $result); - $this->assertIsWord($result[0], 'test)word'); - } - - public function testGivenUnmatchedRightParenthesisInFieldWhenParsingQueryThenTreatsAsCharacter(): void - { - $result = $this->createParser()->parse('description:test)phrase'); - - $this->assertIsArray($result); - $this->assertCount(1, $result); - $this->assertIsField($result[0], 'description', 'test)phrase'); - } - - public function testGivenSubqueryFollowedByWordWhenParsingQueryThenReturnsCorrectNodes(): void - { - $result = $this->createParser()->parse('(amount:100 category:food) shopping'); - - $this->assertIsArray($result); - $this->assertCount(2, $result); - - $expectedNodes = [ - new Field('amount', '100'), - new Field('category', 'food') - ]; - $this->assertIsSubquery($result[0], $expectedNodes); - $this->assertIsWord($result[1], 'shopping'); - } - - private function assertIsWord(Node $node, string $expectedValue, bool $prohibited = false): void - { - $this->assertInstanceOf(Word::class, $node); - /** @var Word $node */ - $this->assertEquals($expectedValue, $node->getValue()); - $this->assertEquals($prohibited, $node->isProhibited()); - } - - private function assertIsField( - Node $node, - string $expectedOperator, - string $expectedValue, - bool $prohibited = false - ): void { - $this->assertInstanceOf(Field::class, $node); - /** @var Field $node */ - $this->assertEquals($expectedOperator, $node->getOperator()); - $this->assertEquals($expectedValue, $node->getValue()); - $this->assertEquals($prohibited, $node->isProhibited()); - } - - private function assertIsSubquery(Node $node, array $expectedNodes, bool $prohibited = false): void - { - $this->assertInstanceOf(Subquery::class, $node); - /** @var Subquery $node */ - $this->assertCount(count($expectedNodes), $node->getNodes()); - $this->assertEquals($prohibited, $node->isProhibited()); - - foreach ($expectedNodes as $index => $expected) { - $actual = $node->getNodes()[$index]; - if ($expected instanceof Word) { - $this->assertIsWord($actual, $expected->getValue(), $expected->isProhibited()); - } elseif ($expected instanceof Field) { - $this->assertIsField($actual, $expected->getOperator(), $expected->getValue(), $expected->isProhibited()); - } elseif ($expected instanceof Subquery) { - $this->assertIsSubquery($actual, $expected->getNodes(), $expected->isProhibited()); - } - } - } -} diff --git a/tests/unit/Support/Search/QueryParser/AbstractQueryParserInterfaceParseQueryTest.php b/tests/unit/Support/Search/QueryParser/AbstractQueryParserInterfaceParseQueryTest.php new file mode 100644 index 0000000000..3d2a5426dc --- /dev/null +++ b/tests/unit/Support/Search/QueryParser/AbstractQueryParserInterfaceParseQueryTest.php @@ -0,0 +1,278 @@ + [ + 'query' => '', + 'expected' => [] + ], + 'simple word' => [ + 'query' => 'grocaeries', + 'expected' => [new Word('groceries')] + ], + 'prohibited word' => [ + 'query' => '-groceries', + 'expected' => [new Word('groceries', true)] + ], + 'prohibited field' => [ + 'query' => '-amount:100', + 'expected' => [new Field('amount', '100', true)] + ], + 'quoted word' => [ + 'query' => '"test phrase"', + 'expected' => [new Word('test phrase')] + ], + 'prohibited quoted word' => [ + 'query' => '-"test phrase"', + 'expected' => [new Word('test phrase', true)] + ], + 'multiple words' => [ + 'query' => 'groceries shopping market', + 'expected' => [ + new Word('groceries'), + new Word('shopping'), + new Word('market') + ] + ], + 'field operator' => [ + 'query' => 'amount:100', + 'expected' => [new Field('amount', '100')] + ], + 'quoted field value with single space' => [ + 'query' => 'description:"test phrase"', + 'expected' => [new Field('description', 'test phrase')] + ], + 'multiple fields' => [ + 'query' => 'amount:100 category:food', + 'expected' => [ + new Field('amount', '100'), + new Field('category', 'food') + ] + ], + 'simple subquery' => [ + 'query' => '(amount:100 category:food)', + 'expected' => [ + new Subquery([ + new Field('amount', '100'), + new Field('category', 'food') + ]) + ] + ], + 'prohibited subquery' => [ + 'query' => '-(amount:100 category:food)', + 'expected' => [ + new Subquery([ + new Field('amount', '100'), + new Field('category', 'food') + ], true) + ] + ], + 'nested subquery' => [ + 'query' => '(amount:100 (description:"test" category:food))', + 'expected' => [ + new Subquery([ + new Field('amount', '100'), + new Subquery([ + new Field('description', 'test'), + new Field('category', 'food') + ]) + ]) + ] + ], + 'mixed words and operators' => [ + 'query' => 'groceries amount:50 shopping', + 'expected' => [ + new Word('groceries'), + new Field('amount', '50'), + new Word('shopping') + ] + ], + 'subquery after field value' => [ + 'query' => 'amount:100 (description:"market" category:food)', + 'expected' => [ + new Field('amount', '100'), + new Subquery([ + new Field('description', 'market'), + new Field('category', 'food') + ]) + ] + ], + 'word followed by subquery' => [ + 'query' => 'groceries (amount:100 description_contains:"test")', + 'expected' => [ + new Word('groceries'), + new Subquery([ + new Field('amount', '100'), + new Field('description_contains', 'test') + ]) + ] + ], + 'nested subquery with prohibited field' => [ + 'query' => '(amount:100 (description_contains:"test payment" -has_attachments:true))', + 'expected' => [ + new Subquery([ + new Field('amount', '100'), + new Subquery([ + new Field('description_contains', 'test payment'), + new Field('has_attachments', 'true', true) + ]) + ]) + ] + ], + 'complex nested subqueries' => [ + 'query' => 'shopping (amount:50 market (-category:food word description:"test phrase" (has_notes:true)))', + 'expected' => [ + new Word('shopping'), + new Subquery([ + new Field('amount', '50'), + new Word('market'), + new Subquery([ + new Field('category', 'food', true), + new Word('word'), + new Field('description', 'test phrase'), + new Subquery([ + new Field('has_notes', 'true') + ]) + ]) + ]) + ] + ], + 'word with multiple spaces' => [ + 'query' => '"multiple spaces"', + 'expected' => [new Word('multiple spaces')] + ], + 'field with multiple spaces in value' => [ + 'query' => 'description:"multiple spaces here"', + 'expected' => [new Field('description', 'multiple spaces here')] + ], + 'unmatched right parenthesis in word' => [ + 'query' => 'test)word', + 'expected' => [new Word('test)word')] + ], + 'unmatched right parenthesis in field' => [ + 'query' => 'description:test)phrase', + 'expected' => [new Field('description', 'test)phrase')] + ], + 'subquery followed by word' => [ + 'query' => '(amount:100 category:food) shopping', + 'expected' => [ + new Subquery([ + new Field('amount', '100'), + new Field('category', 'food') + ]), + new Word('shopping') + ] + ] + ]; + } + + /** + * @dataProvider queryDataProvider + * @param string $query The query string to parse + * @param array $expected The expected parse result + */ + public function testQueryParsing(string $query, array $expected): void + { + $result = $this->createParser()->parse($query); + + $this->assertNodesMatch($expected, $result); + } + + private function assertNodesMatch(array $expected, array $actual): void + { + $this->assertCount(count($expected), $actual); + + foreach ($expected as $index => $expectedNode) { + $actualNode = $actual[$index]; + $this->assertNodeMatches($expectedNode, $actualNode); + } + } + + private function assertNodeMatches(Node $expected, Node $actual): void + { + $this->assertInstanceOf(get_class($expected), $actual); + $this->assertEquals($expected->isProhibited(), $actual->isProhibited()); + + match (get_class($expected)) { + Word::class => $this->assertWordMatches($expected, $actual), + Field::class => $this->assertFieldMatches($expected, $actual), + Subquery::class => $this->assertSubqueryMatches($expected, $actual), + default => throw new \InvalidArgumentException(sprintf( + 'Unexpected node type: %s', + get_class($expected) + )) + }; + } + + private function assertWordMatches(Word $expected, Word $actual): void + { + $this->assertEquals($expected->getValue(), $actual->getValue()); + } + + private function assertFieldMatches(Field $expected, Field $actual): void + { + $this->assertEquals($expected->getOperator(), $actual->getOperator()); + $this->assertEquals($expected->getValue(), $actual->getValue()); + } + + private function assertSubqueryMatches(Subquery $expected, Subquery $actual): void + { + $this->assertNodesMatch($expected->getNodes(), $actual->getNodes()); + } + + private function assertIsWord(Node $node, string $expectedValue, bool $prohibited = false): void + { + $this->assertInstanceOf(Word::class, $node); + /** @var Word $node */ + $this->assertEquals($expectedValue, $node->getValue()); + $this->assertEquals($prohibited, $node->isProhibited()); + } + + private function assertIsField( + Node $node, + string $expectedOperator, + string $expectedValue, + bool $prohibited = false + ): void { + $this->assertInstanceOf(Field::class, $node); + /** @var Field $node */ + $this->assertEquals($expectedOperator, $node->getOperator()); + $this->assertEquals($expectedValue, $node->getValue()); + $this->assertEquals($prohibited, $node->isProhibited()); + } + + private function assertIsSubquery(Node $node, array $expectedNodes, bool $prohibited = false): void + { + $this->assertInstanceOf(Subquery::class, $node); + /** @var Subquery $node */ + $this->assertCount(count($expectedNodes), $node->getNodes()); + $this->assertEquals($prohibited, $node->isProhibited()); + + foreach ($expectedNodes as $index => $expected) { + $actual = $node->getNodes()[$index]; + if ($expected instanceof Word) { + $this->assertIsWord($actual, $expected->getValue(), $expected->isProhibited()); + } elseif ($expected instanceof Field) { + $this->assertIsField($actual, $expected->getOperator(), $expected->getValue(), $expected->isProhibited()); + } elseif ($expected instanceof Subquery) { + $this->assertIsSubquery($actual, $expected->getNodes(), $expected->isProhibited()); + } + } + } +} diff --git a/tests/unit/Support/Search/GdbotsQueryParserParseQueryTest.php b/tests/unit/Support/Search/QueryParser/GdbotsQueryParserParseQueryTest.php similarity index 60% rename from tests/unit/Support/Search/GdbotsQueryParserParseQueryTest.php rename to tests/unit/Support/Search/QueryParser/GdbotsQueryParserParseQueryTest.php index 8f00f7a65d..c5880fe98c 100644 --- a/tests/unit/Support/Search/GdbotsQueryParserParseQueryTest.php +++ b/tests/unit/Support/Search/QueryParser/GdbotsQueryParserParseQueryTest.php @@ -1,10 +1,9 @@ Date: Thu, 2 Jan 2025 22:40:50 +0100 Subject: [PATCH 19/27] Give a bit better error messages in tests --- app/Support/Search/QueryParser/Subquery.php | 2 +- ...ractQueryParserInterfaceParseQueryTest.php | 80 ++++++++++--------- 2 files changed, 42 insertions(+), 40 deletions(-) diff --git a/app/Support/Search/QueryParser/Subquery.php b/app/Support/Search/QueryParser/Subquery.php index 9336ce0a62..f43c1f123a 100644 --- a/app/Support/Search/QueryParser/Subquery.php +++ b/app/Support/Search/QueryParser/Subquery.php @@ -33,6 +33,6 @@ class Subquery extends Node public function __toString(): string { - return ($this->prohibited ? '-' : '') . '(' . implode(' ', array_map(fn($node) => (string)$node, $this->nodes)) . ')'; + return ($this->prohibited ? '-' : '') . '[' . implode(' ', array_map(fn($node) => (string)$node, $this->nodes)) . ']'; } } diff --git a/tests/unit/Support/Search/QueryParser/AbstractQueryParserInterfaceParseQueryTest.php b/tests/unit/Support/Search/QueryParser/AbstractQueryParserInterfaceParseQueryTest.php index 3d2a5426dc..1a1d668a7b 100644 --- a/tests/unit/Support/Search/QueryParser/AbstractQueryParserInterfaceParseQueryTest.php +++ b/tests/unit/Support/Search/QueryParser/AbstractQueryParserInterfaceParseQueryTest.php @@ -23,7 +23,7 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase 'expected' => [] ], 'simple word' => [ - 'query' => 'grocaeries', + 'query' => 'groceries', 'expected' => [new Word('groceries')] ], 'prohibited word' => [ @@ -196,7 +196,13 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase private function assertNodesMatch(array $expected, array $actual): void { - $this->assertCount(count($expected), $actual); + $message = sprintf( + "Expected: %s\nActual: %s", + $this->formatNodes($expected), + $this->formatNodes($actual) + ); + + $this->assertCount(count($expected), $actual, $message); foreach ($expected as $index => $expectedNode) { $actualNode = $actual[$index]; @@ -206,8 +212,10 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase private function assertNodeMatches(Node $expected, Node $actual): void { - $this->assertInstanceOf(get_class($expected), $actual); - $this->assertEquals($expected->isProhibited(), $actual->isProhibited()); + $message = $this->formatAssertMessage($expected, $actual); + + $this->assertInstanceOf(get_class($expected), $actual, $message); + $this->assertEquals($expected->isProhibited(), $actual->isProhibited(), $message); match (get_class($expected)) { Word::class => $this->assertWordMatches($expected, $actual), @@ -220,15 +228,22 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase }; } + private function assertWordMatches(Word $expected, Word $actual): void { - $this->assertEquals($expected->getValue(), $actual->getValue()); + $message = $this->formatAssertMessage($expected, $actual); // Using your implementation + $this->assertEquals($expected->getValue(), $actual->getValue(), $message); } private function assertFieldMatches(Field $expected, Field $actual): void { - $this->assertEquals($expected->getOperator(), $actual->getOperator()); - $this->assertEquals($expected->getValue(), $actual->getValue()); + $message = sprintf( + "\nExpected field: %s\nActual field: %s", + $this->formatNode($expected), + $this->formatNode($actual) + ); + $this->assertEquals($expected->getOperator(), $actual->getOperator(), $message); + $this->assertEquals($expected->getValue(), $actual->getValue(), $message); } private function assertSubqueryMatches(Subquery $expected, Subquery $actual): void @@ -236,43 +251,30 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase $this->assertNodesMatch($expected->getNodes(), $actual->getNodes()); } - private function assertIsWord(Node $node, string $expectedValue, bool $prohibited = false): void + private function formatAssertMessage(Node $expected, Node $actual): string { - $this->assertInstanceOf(Word::class, $node); - /** @var Word $node */ - $this->assertEquals($expectedValue, $node->getValue()); - $this->assertEquals($prohibited, $node->isProhibited()); + return sprintf( + "\nExpected: %s\nActual: %s", + $this->formatNode($expected), + $this->formatNode($actual) + ); } - private function assertIsField( - Node $node, - string $expectedOperator, - string $expectedValue, - bool $prohibited = false - ): void { - $this->assertInstanceOf(Field::class, $node); - /** @var Field $node */ - $this->assertEquals($expectedOperator, $node->getOperator()); - $this->assertEquals($expectedValue, $node->getValue()); - $this->assertEquals($prohibited, $node->isProhibited()); + private function formatNode(Node $node): string + { + return sprintf( + '%s(%s)', + basename(str_replace('\\', '/', get_class($node))), + $node->__toString() + ); } - private function assertIsSubquery(Node $node, array $expectedNodes, bool $prohibited = false): void - { - $this->assertInstanceOf(Subquery::class, $node); - /** @var Subquery $node */ - $this->assertCount(count($expectedNodes), $node->getNodes()); - $this->assertEquals($prohibited, $node->isProhibited()); - foreach ($expectedNodes as $index => $expected) { - $actual = $node->getNodes()[$index]; - if ($expected instanceof Word) { - $this->assertIsWord($actual, $expected->getValue(), $expected->isProhibited()); - } elseif ($expected instanceof Field) { - $this->assertIsField($actual, $expected->getOperator(), $expected->getValue(), $expected->isProhibited()); - } elseif ($expected instanceof Subquery) { - $this->assertIsSubquery($actual, $expected->getNodes(), $expected->isProhibited()); - } - } + private function formatNodes(array $nodes): string + { + return '[' . implode(', ', array_map( + fn(Node $node) => $this->formatNode($node), + $nodes + )) . ']'; } } From af7a4b5d3da8b20430b88faf83bce56602721c16 Mon Sep 17 00:00:00 2001 From: Sobuno Date: Thu, 2 Jan 2025 23:19:21 +0100 Subject: [PATCH 20/27] Renaming of classes, making true recursive structure --- app/Support/Search/OperatorQuerySearch.php | 60 +++++++++++-------- .../QueryParser/{Field.php => FieldNode.php} | 2 +- .../Search/QueryParser/GdbotsQueryParser.php | 15 ++--- .../{Subquery.php => NodeGroup.php} | 6 +- .../Search/QueryParser/QueryParser.php | 27 +++++---- .../QueryParser/QueryParserInterface.php | 4 +- .../QueryParser/{Word.php => StringNode.php} | 4 +- 7 files changed, 66 insertions(+), 52 deletions(-) rename app/Support/Search/QueryParser/{Field.php => FieldNode.php} (96%) rename app/Support/Search/QueryParser/{Subquery.php => NodeGroup.php} (82%) rename app/Support/Search/QueryParser/{Word.php => StringNode.php} (76%) diff --git a/app/Support/Search/OperatorQuerySearch.php b/app/Support/Search/OperatorQuerySearch.php index 4e9408a3a8..74e518fd5c 100644 --- a/app/Support/Search/OperatorQuerySearch.php +++ b/app/Support/Search/OperatorQuerySearch.php @@ -41,9 +41,10 @@ use FireflyIII\Repositories\Tag\TagRepositoryInterface; use FireflyIII\Repositories\UserGroups\Currency\CurrencyRepositoryInterface; use FireflyIII\Support\Search\QueryParser\QueryParserInterface; use FireflyIII\Support\Search\QueryParser\Node; -use FireflyIII\Support\Search\QueryParser\Field; -use FireflyIII\Support\Search\QueryParser\Word; -use FireflyIII\Support\Search\QueryParser\Subquery; +use FireflyIII\Support\Search\QueryParser\FieldNode; +use FireflyIII\Support\Search\QueryParser\StringNode; +use FireflyIII\Support\Search\QueryParser\NodeGroup; + use FireflyIII\Support\ParseDateString; use FireflyIII\User; use Illuminate\Pagination\LengthAwarePaginator; @@ -140,7 +141,7 @@ class OperatorQuerySearch implements SearchInterface app('log')->debug(sprintf('Using %s as implementation for QueryParserInterface', get_class($parser))); try { - $nodes = $parser->parse($query); + $parsedQuery = $parser->parse($query); } catch (\LogicException|\TypeError $e) { app('log')->error($e->getMessage()); app('log')->error(sprintf('Could not parse search: "%s".', $query)); @@ -148,10 +149,8 @@ class OperatorQuerySearch implements SearchInterface throw new FireflyException(sprintf('Invalid search value "%s". See the logs.', e($query)), 0, $e); } - app('log')->debug(sprintf('Found %d node(s)', count($nodes))); - foreach ($nodes as $node) { - $this->handleSearchNode($node); - } + app('log')->debug(sprintf('Found %d node(s) at top-level', count($parsedQuery->getNodes()))); + $this->handleSearchNode($parsedQuery); // add missing information $this->collector->withBillInformation(); @@ -170,27 +169,16 @@ class OperatorQuerySearch implements SearchInterface app('log')->debug(sprintf('Now in handleSearchNode(%s)', get_class($node))); switch (true) { - case $node instanceof Word: - $word = (string) $node->getValue(); - if($node->isProhibited()) { - app('log')->debug(sprintf('Exclude word "%s" from search string', $word)); - $this->prohibitedWords[] = $word; - } else { - app('log')->debug(sprintf('Add word "%s" to search string', $word)); - $this->words[] = $word; - } - + case $node instanceof StringNode: + $this->handleStringNode($node); break; - case $node instanceof Field: + case $node instanceof FieldNode: $this->handleFieldNode($node); break; - case $node instanceof Subquery: - //TODO: Handle Subquery prohibition, i.e. flip all prohibition flags inside the subquery - foreach ($node->getNodes() as $subNode) { - $this->handleSearchNode($subNode); - } + case $node instanceof NodeGroup: + $this->handleNodeGroup($node); break; default: @@ -199,10 +187,32 @@ class OperatorQuerySearch implements SearchInterface } } + private function handleNodeGroup(NodeGroup $node): void + { + //TODO: Handle Subquery prohibition, i.e. flip all prohibition flags inside the subquery + foreach ($node->getNodes() as $subNode) { + $this->handleSearchNode($subNode); + } + } + + + + private function handleStringNode(StringNode $node): void + { + $string = (string) $node->getValue(); + if($node->isProhibited()) { + app('log')->debug(sprintf('Exclude string "%s" from search string', $string)); + $this->prohibitedWords[] = $string; + } else { + app('log')->debug(sprintf('Add string "%s" to search string', $string)); + $this->words[] = $string; + } + } + /** * @throws FireflyException */ - private function handleFieldNode(Field $node): void + private function handleFieldNode(FieldNode $node): void { $operator = strtolower($node->getOperator()); $value = $node->getValue(); diff --git a/app/Support/Search/QueryParser/Field.php b/app/Support/Search/QueryParser/FieldNode.php similarity index 96% rename from app/Support/Search/QueryParser/Field.php rename to app/Support/Search/QueryParser/FieldNode.php index 3028af9730..4fd847cd5d 100644 --- a/app/Support/Search/QueryParser/Field.php +++ b/app/Support/Search/QueryParser/FieldNode.php @@ -7,7 +7,7 @@ namespace FireflyIII\Support\Search\QueryParser; /** * Represents a field operator with value (e.g. amount:100) */ -class Field extends Node +class FieldNode extends Node { private string $operator; private string $value; diff --git a/app/Support/Search/QueryParser/GdbotsQueryParser.php b/app/Support/Search/QueryParser/GdbotsQueryParser.php index 8a56d0f6de..c580b694be 100644 --- a/app/Support/Search/QueryParser/GdbotsQueryParser.php +++ b/app/Support/Search/QueryParser/GdbotsQueryParser.php @@ -19,17 +19,18 @@ class GdbotsQueryParser implements QueryParserInterface } /** - * @return Node[] + * @return NodeGroup * @throws FireflyException */ - public function parse(string $query): array + public function parse(string $query): NodeGroup { try { $result = $this->parser->parse($query); - return array_map( + $nodes = array_map( fn(GdbotsNode\Node $node) => $this->convertNode($node), $result->getNodes() ); + return new NodeGroup($nodes); } catch (\LogicException|\TypeError $e) { fwrite(STDERR, "Setting up GdbotsQueryParserTest\n"); dd('Creating GdbotsQueryParser'); @@ -44,17 +45,17 @@ class GdbotsQueryParser implements QueryParserInterface { switch (true) { case $node instanceof GdbotsNode\Word: - return new Word($node->getValue()); + return new StringNode($node->getValue()); case $node instanceof GdbotsNode\Field: - return new Field( + return new FieldNode( $node->getValue(), (string) $node->getNode()->getValue(), BoolOperator::PROHIBITED === $node->getBoolOperator() ); case $node instanceof GdbotsNode\Subquery: - return new Subquery( + return new NodeGroup( array_map( fn(GdbotsNode\Node $subNode) => $this->convertNode($subNode), $node->getNodes() @@ -69,7 +70,7 @@ class GdbotsQueryParser implements QueryParserInterface case $node instanceof GdbotsNode\Mention: case $node instanceof GdbotsNode\Emoticon: case $node instanceof GdbotsNode\Emoji: - return new Word((string) $node->getValue()); + return new StringNode((string) $node->getValue()); default: throw new FireflyException( diff --git a/app/Support/Search/QueryParser/Subquery.php b/app/Support/Search/QueryParser/NodeGroup.php similarity index 82% rename from app/Support/Search/QueryParser/Subquery.php rename to app/Support/Search/QueryParser/NodeGroup.php index f43c1f123a..967c4d89d4 100644 --- a/app/Support/Search/QueryParser/Subquery.php +++ b/app/Support/Search/QueryParser/NodeGroup.php @@ -5,9 +5,11 @@ declare(strict_types=1); namespace FireflyIII\Support\Search\QueryParser; /** - * Represents a subquery (group of nodes) + * Represents a group of nodes. + * + * NodeGroups can be nested inside other NodeGroups, making them subqueries */ -class Subquery extends Node +class NodeGroup extends Node { /** @var Node[] */ private array $nodes; diff --git a/app/Support/Search/QueryParser/QueryParser.php b/app/Support/Search/QueryParser/QueryParser.php index 1d7f0658c9..f1e951f5ce 100644 --- a/app/Support/Search/QueryParser/QueryParser.php +++ b/app/Support/Search/QueryParser/QueryParser.php @@ -30,16 +30,16 @@ class QueryParser implements QueryParserInterface private string $query; private int $position = 0; - /** @return Node[] */ - public function parse(string $query): array + /** @return NodeGroup */ + public function parse(string $query): NodeGroup { $this->query = $query; $this->position = 0; - return $this->parseQuery(false); + return $this->buildNodeGroup(false); } - /** @return Node[] */ - private function parseQuery(bool $isSubquery): array + /** @return NodeGroup */ + private function buildNodeGroup(bool $isSubquery, bool $prohibited = false): NodeGroup { $nodes = []; $nodeResult = $this->buildNextNode($isSubquery); @@ -52,7 +52,7 @@ class QueryParser implements QueryParserInterface $nodeResult = $this->buildNextNode($isSubquery); } - return $nodes; + return new NodeGroup($nodes, $prohibited); } private function buildNextNode(bool $isSubquery): NodeResult @@ -105,8 +105,7 @@ class QueryParser implements QueryParserInterface if ($tokenUnderConstruction === '') { // A left parentheses at the beginning of a token indicates the start of a subquery $this->position++; - return new NodeResult( - new Subquery($this->parseQuery(true), $prohibited), + return new NodeResult($this->buildNodeGroup(true, $prohibited), false ); } else { @@ -161,16 +160,18 @@ class QueryParser implements QueryParserInterface $this->position++; } - return new NodeResult($tokenUnderConstruction !== '' || $fieldName !== '' - ? $this->createNode($tokenUnderConstruction, $fieldName, $prohibited) - : null, true); + $finalNode = $tokenUnderConstruction !== '' || $fieldName !== '' + ? $this->createNode($tokenUnderConstruction, $fieldName, $prohibited) + : null; + + return new NodeResult($finalNode, true); } private function createNode(string $token, string $fieldName, bool $prohibited): Node { if (strlen($fieldName) > 0) { - return new Field(trim($fieldName), trim($token), $prohibited); + return new FieldNode(trim($fieldName), trim($token), $prohibited); } - return new Word(trim($token), $prohibited); + return new StringNode(trim($token), $prohibited); } } diff --git a/app/Support/Search/QueryParser/QueryParserInterface.php b/app/Support/Search/QueryParser/QueryParserInterface.php index 831baf1270..00383af0ff 100644 --- a/app/Support/Search/QueryParser/QueryParserInterface.php +++ b/app/Support/Search/QueryParser/QueryParserInterface.php @@ -7,7 +7,7 @@ namespace FireflyIII\Support\Search\QueryParser; interface QueryParserInterface { /** - * @return Node[] + * @return NodeGroup */ - public function parse(string $query): array; + public function parse(string $query): NodeGroup; } diff --git a/app/Support/Search/QueryParser/Word.php b/app/Support/Search/QueryParser/StringNode.php similarity index 76% rename from app/Support/Search/QueryParser/Word.php rename to app/Support/Search/QueryParser/StringNode.php index d7c12b6976..387862da30 100644 --- a/app/Support/Search/QueryParser/Word.php +++ b/app/Support/Search/QueryParser/StringNode.php @@ -5,9 +5,9 @@ declare(strict_types=1); namespace FireflyIII\Support\Search\QueryParser; /** - * Represents a word in the search query, meaning either a single-word without spaces or a quote-delimited string + * Represents a string in the search query, meaning either a single-word without spaces or a quote-delimited string */ -class Word extends Node +class StringNode extends Node { private string $value; From a62916a63d03b73fb3f406a26b52b891f29dc4a1 Mon Sep 17 00:00:00 2001 From: Sobuno Date: Thu, 2 Jan 2025 23:36:00 +0100 Subject: [PATCH 21/27] Simplify further --- app/Support/Search/QueryParser/FieldNode.php | 5 - app/Support/Search/QueryParser/Node.php | 2 - app/Support/Search/QueryParser/NodeGroup.php | 6 - app/Support/Search/QueryParser/StringNode.php | 5 - ...ractQueryParserInterfaceParseQueryTest.php | 257 ++++++------------ 5 files changed, 87 insertions(+), 188 deletions(-) diff --git a/app/Support/Search/QueryParser/FieldNode.php b/app/Support/Search/QueryParser/FieldNode.php index 4fd847cd5d..5005d5fe57 100644 --- a/app/Support/Search/QueryParser/FieldNode.php +++ b/app/Support/Search/QueryParser/FieldNode.php @@ -28,9 +28,4 @@ class FieldNode extends Node { return $this->value; } - - public function __toString(): string - { - return ($this->prohibited ? '-' : '') . $this->operator . ':' . $this->value; - } } diff --git a/app/Support/Search/QueryParser/Node.php b/app/Support/Search/QueryParser/Node.php index 7dced2e759..3050bd82c7 100644 --- a/app/Support/Search/QueryParser/Node.php +++ b/app/Support/Search/QueryParser/Node.php @@ -9,8 +9,6 @@ namespace FireflyIII\Support\Search\QueryParser; */ abstract class Node { - abstract public function __toString(): string; - protected bool $prohibited; public function isProhibited(): bool diff --git a/app/Support/Search/QueryParser/NodeGroup.php b/app/Support/Search/QueryParser/NodeGroup.php index 967c4d89d4..8cf801dbbc 100644 --- a/app/Support/Search/QueryParser/NodeGroup.php +++ b/app/Support/Search/QueryParser/NodeGroup.php @@ -31,10 +31,4 @@ class NodeGroup extends Node { return $this->nodes; } - - - public function __toString(): string - { - return ($this->prohibited ? '-' : '') . '[' . implode(' ', array_map(fn($node) => (string)$node, $this->nodes)) . ']'; - } } diff --git a/app/Support/Search/QueryParser/StringNode.php b/app/Support/Search/QueryParser/StringNode.php index 387862da30..6164cc1de7 100644 --- a/app/Support/Search/QueryParser/StringNode.php +++ b/app/Support/Search/QueryParser/StringNode.php @@ -21,9 +21,4 @@ class StringNode extends Node { return $this->value; } - - public function __toString(): string - { - return ($this->prohibited ? '-' : '') . $this->value; - } } diff --git a/tests/unit/Support/Search/QueryParser/AbstractQueryParserInterfaceParseQueryTest.php b/tests/unit/Support/Search/QueryParser/AbstractQueryParserInterfaceParseQueryTest.php index 1a1d668a7b..f324a8661a 100644 --- a/tests/unit/Support/Search/QueryParser/AbstractQueryParserInterfaceParseQueryTest.php +++ b/tests/unit/Support/Search/QueryParser/AbstractQueryParserInterfaceParseQueryTest.php @@ -4,10 +4,10 @@ declare(strict_types=1); namespace Tests\unit\Support\Search\QueryParser; -use FireflyIII\Support\Search\QueryParser\Field; +use FireflyIII\Support\Search\QueryParser\FieldNode; use FireflyIII\Support\Search\QueryParser\QueryParserInterface; -use FireflyIII\Support\Search\QueryParser\Word; -use FireflyIII\Support\Search\QueryParser\Subquery; +use FireflyIII\Support\Search\QueryParser\StringNode; +use FireflyIII\Support\Search\QueryParser\NodeGroup; use FireflyIII\Support\Search\QueryParser\Node; use Tests\integration\TestCase; @@ -20,164 +20,164 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase return [ 'empty query' => [ 'query' => '', - 'expected' => [] + 'expected' => new NodeGroup([]) ], 'simple word' => [ 'query' => 'groceries', - 'expected' => [new Word('groceries')] + 'expected' => new NodeGroup([new StringNode('groceries')]) ], 'prohibited word' => [ 'query' => '-groceries', - 'expected' => [new Word('groceries', true)] + 'expected' => new NodeGroup([new StringNode('groceries', true)]) ], 'prohibited field' => [ 'query' => '-amount:100', - 'expected' => [new Field('amount', '100', true)] + 'expected' => new NodeGroup([new FieldNode('amount', '100', true)]) ], 'quoted word' => [ 'query' => '"test phrase"', - 'expected' => [new Word('test phrase')] + 'expected' => new NodeGroup([new StringNode('test phrase')]) ], 'prohibited quoted word' => [ 'query' => '-"test phrase"', - 'expected' => [new Word('test phrase', true)] + 'expected' => new NodeGroup([new StringNode('test phrase', true)]) ], 'multiple words' => [ 'query' => 'groceries shopping market', - 'expected' => [ - new Word('groceries'), - new Word('shopping'), - new Word('market') - ] + 'expected' => new NodeGroup([ + new StringNode('groceries'), + new StringNode('shopping'), + new StringNode('market') + ]) ], 'field operator' => [ 'query' => 'amount:100', - 'expected' => [new Field('amount', '100')] + 'expected' => new NodeGroup([new FieldNode('amount', '100')]) ], 'quoted field value with single space' => [ 'query' => 'description:"test phrase"', - 'expected' => [new Field('description', 'test phrase')] + 'expected' => new NodeGroup([new FieldNode('description', 'test phrase')]) ], 'multiple fields' => [ 'query' => 'amount:100 category:food', - 'expected' => [ - new Field('amount', '100'), - new Field('category', 'food') - ] + 'expected' => new NodeGroup([ + new FieldNode('amount', '100'), + new FieldNode('category', 'food') + ]) ], 'simple subquery' => [ 'query' => '(amount:100 category:food)', - 'expected' => [ - new Subquery([ - new Field('amount', '100'), - new Field('category', 'food') + 'expected' => new NodeGroup([ + new NodeGroup([ + new FieldNode('amount', '100'), + new FieldNode('category', 'food') ]) - ] + ]) ], 'prohibited subquery' => [ 'query' => '-(amount:100 category:food)', - 'expected' => [ - new Subquery([ - new Field('amount', '100'), - new Field('category', 'food') + 'expected' => new NodeGroup([ + new NodeGroup([ + new FieldNode('amount', '100'), + new FieldNode('category', 'food') ], true) - ] + ]) ], 'nested subquery' => [ 'query' => '(amount:100 (description:"test" category:food))', - 'expected' => [ - new Subquery([ - new Field('amount', '100'), - new Subquery([ - new Field('description', 'test'), - new Field('category', 'food') + 'expected' => new NodeGroup([ + new NodeGroup([ + new FieldNode('amount', '100'), + new NodeGroup([ + new FieldNode('description', 'test'), + new FieldNode('category', 'food') ]) ]) - ] + ]) ], 'mixed words and operators' => [ 'query' => 'groceries amount:50 shopping', - 'expected' => [ - new Word('groceries'), - new Field('amount', '50'), - new Word('shopping') - ] + 'expected' => new NodeGroup([ + new StringNode('groceries'), + new FieldNode('amount', '50'), + new StringNode('shopping') + ]) ], 'subquery after field value' => [ 'query' => 'amount:100 (description:"market" category:food)', - 'expected' => [ - new Field('amount', '100'), - new Subquery([ - new Field('description', 'market'), - new Field('category', 'food') + 'expected' => new NodeGroup([ + new FieldNode('amount', '100'), + new NodeGroup([ + new FieldNode('description', 'market'), + new FieldNode('category', 'food') ]) - ] + ]) ], 'word followed by subquery' => [ 'query' => 'groceries (amount:100 description_contains:"test")', - 'expected' => [ - new Word('groceries'), - new Subquery([ - new Field('amount', '100'), - new Field('description_contains', 'test') + 'expected' => new NodeGroup([ + new StringNode('groceries'), + new NodeGroup([ + new FieldNode('amount', '100'), + new FieldNode('description_contains', 'test') ]) - ] + ]) ], 'nested subquery with prohibited field' => [ 'query' => '(amount:100 (description_contains:"test payment" -has_attachments:true))', - 'expected' => [ - new Subquery([ - new Field('amount', '100'), - new Subquery([ - new Field('description_contains', 'test payment'), - new Field('has_attachments', 'true', true) + 'expected' => new NodeGroup([ + new NodeGroup([ + new FieldNode('amount', '100'), + new NodeGroup([ + new FieldNode('description_contains', 'test payment'), + new FieldNode('has_attachments', 'true', true) ]) ]) - ] + ]) ], 'complex nested subqueries' => [ 'query' => 'shopping (amount:50 market (-category:food word description:"test phrase" (has_notes:true)))', - 'expected' => [ - new Word('shopping'), - new Subquery([ - new Field('amount', '50'), - new Word('market'), - new Subquery([ - new Field('category', 'food', true), - new Word('word'), - new Field('description', 'test phrase'), - new Subquery([ - new Field('has_notes', 'true') + 'expected' => new NodeGroup([ + new StringNode('shopping'), + new NodeGroup([ + new FieldNode('amount', '50'), + new StringNode('market'), + new NodeGroup([ + new FieldNode('category', 'food', true), + new StringNode('word'), + new FieldNode('description', 'test phrase'), + new NodeGroup([ + new FieldNode('has_notes', 'true') ]) ]) ]) - ] + ]) ], 'word with multiple spaces' => [ 'query' => '"multiple spaces"', - 'expected' => [new Word('multiple spaces')] + 'expected' => new NodeGroup([new StringNode('multiple spaces')]) ], 'field with multiple spaces in value' => [ 'query' => 'description:"multiple spaces here"', - 'expected' => [new Field('description', 'multiple spaces here')] + 'expected' => new NodeGroup([new FieldNode('description', 'multiple spaces here')]) ], 'unmatched right parenthesis in word' => [ 'query' => 'test)word', - 'expected' => [new Word('test)word')] + 'expected' => new NodeGroup([new StringNode('test)word')]) ], 'unmatched right parenthesis in field' => [ 'query' => 'description:test)phrase', - 'expected' => [new Field('description', 'test)phrase')] + 'expected' => new NodeGroup([new FieldNode('description', 'test)phrase')]) ], 'subquery followed by word' => [ 'query' => '(amount:100 category:food) shopping', - 'expected' => [ - new Subquery([ - new Field('amount', '100'), - new Field('category', 'food') + 'expected' => new NodeGroup([ + new NodeGroup([ + new FieldNode('amount', '100'), + new FieldNode('category', 'food') ]), - new Word('shopping') - ] + new StringNode('shopping') + ]) ] ]; } @@ -185,96 +185,13 @@ abstract class AbstractQueryParserInterfaceParseQueryTest extends TestCase /** * @dataProvider queryDataProvider * @param string $query The query string to parse - * @param array $expected The expected parse result + * @param Node $expected The expected parse result */ - public function testQueryParsing(string $query, array $expected): void + public function testQueryParsing(string $query, Node $expected): void { - $result = $this->createParser()->parse($query); + $actual = $this->createParser()->parse($query); - $this->assertNodesMatch($expected, $result); - } + $this->assertEquals($expected, $actual); - private function assertNodesMatch(array $expected, array $actual): void - { - $message = sprintf( - "Expected: %s\nActual: %s", - $this->formatNodes($expected), - $this->formatNodes($actual) - ); - - $this->assertCount(count($expected), $actual, $message); - - foreach ($expected as $index => $expectedNode) { - $actualNode = $actual[$index]; - $this->assertNodeMatches($expectedNode, $actualNode); - } - } - - private function assertNodeMatches(Node $expected, Node $actual): void - { - $message = $this->formatAssertMessage($expected, $actual); - - $this->assertInstanceOf(get_class($expected), $actual, $message); - $this->assertEquals($expected->isProhibited(), $actual->isProhibited(), $message); - - match (get_class($expected)) { - Word::class => $this->assertWordMatches($expected, $actual), - Field::class => $this->assertFieldMatches($expected, $actual), - Subquery::class => $this->assertSubqueryMatches($expected, $actual), - default => throw new \InvalidArgumentException(sprintf( - 'Unexpected node type: %s', - get_class($expected) - )) - }; - } - - - private function assertWordMatches(Word $expected, Word $actual): void - { - $message = $this->formatAssertMessage($expected, $actual); // Using your implementation - $this->assertEquals($expected->getValue(), $actual->getValue(), $message); - } - - private function assertFieldMatches(Field $expected, Field $actual): void - { - $message = sprintf( - "\nExpected field: %s\nActual field: %s", - $this->formatNode($expected), - $this->formatNode($actual) - ); - $this->assertEquals($expected->getOperator(), $actual->getOperator(), $message); - $this->assertEquals($expected->getValue(), $actual->getValue(), $message); - } - - private function assertSubqueryMatches(Subquery $expected, Subquery $actual): void - { - $this->assertNodesMatch($expected->getNodes(), $actual->getNodes()); - } - - private function formatAssertMessage(Node $expected, Node $actual): string - { - return sprintf( - "\nExpected: %s\nActual: %s", - $this->formatNode($expected), - $this->formatNode($actual) - ); - } - - private function formatNode(Node $node): string - { - return sprintf( - '%s(%s)', - basename(str_replace('\\', '/', get_class($node))), - $node->__toString() - ); - } - - - private function formatNodes(array $nodes): string - { - return '[' . implode(', ', array_map( - fn(Node $node) => $this->formatNode($node), - $nodes - )) . ']'; } } From 0c955efa8be3f6d2ac3ba1fe9dde186fc2495dd7 Mon Sep 17 00:00:00 2001 From: Sobuno Date: Fri, 3 Jan 2025 00:07:57 +0100 Subject: [PATCH 22/27] Allow flipping of subqueries --- app/Providers/SearchServiceProvider.php | 6 ++--- app/Support/Search/OperatorQuerySearch.php | 28 ++++++++++++---------- app/Support/Search/QueryParser/Node.php | 17 +++++++++++-- 3 files changed, 34 insertions(+), 17 deletions(-) diff --git a/app/Providers/SearchServiceProvider.php b/app/Providers/SearchServiceProvider.php index 944348218d..98b5a0f850 100644 --- a/app/Providers/SearchServiceProvider.php +++ b/app/Providers/SearchServiceProvider.php @@ -23,10 +23,10 @@ declare(strict_types=1); namespace FireflyIII\Providers; -use FireflyIII\Support\Search\GdbotsQueryParser; +use FireflyIII\Support\Search\QueryParser\GdbotsQueryParser; use FireflyIII\Support\Search\OperatorQuerySearch; -use FireflyIII\Support\Search\QueryParser; -use FireflyIII\Support\Search\QueryParserInterface; +use FireflyIII\Support\Search\QueryParser\QueryParser; +use FireflyIII\Support\Search\QueryParser\QueryParserInterface; use FireflyIII\Support\Search\SearchInterface; use Illuminate\Foundation\Application; use Illuminate\Support\ServiceProvider; diff --git a/app/Support/Search/OperatorQuerySearch.php b/app/Support/Search/OperatorQuerySearch.php index 74e518fd5c..e6c445b231 100644 --- a/app/Support/Search/OperatorQuerySearch.php +++ b/app/Support/Search/OperatorQuerySearch.php @@ -150,7 +150,7 @@ class OperatorQuerySearch implements SearchInterface } app('log')->debug(sprintf('Found %d node(s) at top-level', count($parsedQuery->getNodes()))); - $this->handleSearchNode($parsedQuery); + $this->handleSearchNode($parsedQuery, $parsedQuery->isProhibited(false)); // add missing information $this->collector->withBillInformation(); @@ -164,21 +164,21 @@ class OperatorQuerySearch implements SearchInterface * * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ - private function handleSearchNode(Node $node): void + private function handleSearchNode(Node $node, $flipProhibitedFlag): void { app('log')->debug(sprintf('Now in handleSearchNode(%s)', get_class($node))); switch (true) { case $node instanceof StringNode: - $this->handleStringNode($node); + $this->handleStringNode($node, $flipProhibitedFlag); break; case $node instanceof FieldNode: - $this->handleFieldNode($node); + $this->handleFieldNode($node, $flipProhibitedFlag); break; case $node instanceof NodeGroup: - $this->handleNodeGroup($node); + $this->handleNodeGroup($node, $flipProhibitedFlag); break; default: @@ -187,20 +187,24 @@ class OperatorQuerySearch implements SearchInterface } } - private function handleNodeGroup(NodeGroup $node): void + private function handleNodeGroup(NodeGroup $node, $flipProhibitedFlag): void { - //TODO: Handle Subquery prohibition, i.e. flip all prohibition flags inside the subquery + $prohibited = $node->isProhibited($flipProhibitedFlag); + foreach ($node->getNodes() as $subNode) { - $this->handleSearchNode($subNode); + $this->handleSearchNode($subNode, $prohibited); } } - private function handleStringNode(StringNode $node): void + private function handleStringNode(StringNode $node, $flipProhibitedFlag): void { $string = (string) $node->getValue(); - if($node->isProhibited()) { + + $prohibited = $node->isProhibited($flipProhibitedFlag); + + if($prohibited) { app('log')->debug(sprintf('Exclude string "%s" from search string', $string)); $this->prohibitedWords[] = $string; } else { @@ -212,11 +216,11 @@ class OperatorQuerySearch implements SearchInterface /** * @throws FireflyException */ - private function handleFieldNode(FieldNode $node): void + private function handleFieldNode(FieldNode $node, $flipProhibitedFlag): void { $operator = strtolower($node->getOperator()); $value = $node->getValue(); - $prohibited = $node->isProhibited(); + $prohibited = $node->isProhibited($flipProhibitedFlag); $context = config(sprintf('search.operators.%s.needs_context', $operator)); diff --git a/app/Support/Search/QueryParser/Node.php b/app/Support/Search/QueryParser/Node.php index 3050bd82c7..e7ae29611b 100644 --- a/app/Support/Search/QueryParser/Node.php +++ b/app/Support/Search/QueryParser/Node.php @@ -11,8 +11,21 @@ abstract class Node { protected bool $prohibited; - public function isProhibited(): bool + /** + * Returns the prohibited status of the node, optionally inverted based on flipFlag + * + * Flipping is used when a node is inside a NodeGroup that has a prohibited status itself, causing inversion of the query parts inside + * + * @param bool $flipFlag When true, inverts the prohibited status + * @return bool The (potentially inverted) prohibited status + */ + public function isProhibited(bool $flipFlag): bool { - return $this->prohibited; + if ($flipFlag === true) { + return !$this->prohibited; + } else { + return $this->prohibited; + } + } } From 058a0f9fb229646fba855ba99010f0ad03e098e1 Mon Sep 17 00:00:00 2001 From: Sobuno Date: Fri, 3 Jan 2025 00:08:11 +0100 Subject: [PATCH 23/27] Show excluded words in UI --- app/Http/Controllers/SearchController.php | 3 ++- app/Support/Search/OperatorQuerySearch.php | 5 +++++ app/Support/Search/SearchInterface.php | 1 + resources/lang/en_US/firefly.php | 1 + resources/views/search/index.twig | 9 +++++++-- 5 files changed, 16 insertions(+), 3 deletions(-) diff --git a/app/Http/Controllers/SearchController.php b/app/Http/Controllers/SearchController.php index 6bf2204c3b..09eb820c54 100644 --- a/app/Http/Controllers/SearchController.php +++ b/app/Http/Controllers/SearchController.php @@ -84,11 +84,12 @@ class SearchController extends Controller // words from query and operators: $query = $searcher->getWordsAsString(); + $excludedWords = $searcher->getExcludedWordsAsString(); $operators = $searcher->getOperators(); $invalidOperators = $searcher->getInvalidOperators(); $subTitle = (string) trans('breadcrumbs.search_result', ['query' => $fullQuery]); - return view('search.index', compact('query', 'operators', 'page', 'rule', 'fullQuery', 'subTitle', 'ruleId', 'ruleChanged', 'invalidOperators')); + return view('search.index', compact('query', 'excludedWords', 'operators', 'page', 'rule', 'fullQuery', 'subTitle', 'ruleId', 'ruleChanged', 'invalidOperators')); } /** diff --git a/app/Support/Search/OperatorQuerySearch.php b/app/Support/Search/OperatorQuerySearch.php index e6c445b231..e37b90e282 100644 --- a/app/Support/Search/OperatorQuerySearch.php +++ b/app/Support/Search/OperatorQuerySearch.php @@ -123,6 +123,11 @@ class OperatorQuerySearch implements SearchInterface return implode(' ', $this->words); } + public function getExcludedWordsAsString(): string + { + return implode(' ', $this->prohibitedWords); + } + /** * @throws FireflyException */ diff --git a/app/Support/Search/SearchInterface.php b/app/Support/Search/SearchInterface.php index b9197e9daf..3604cacdd1 100644 --- a/app/Support/Search/SearchInterface.php +++ b/app/Support/Search/SearchInterface.php @@ -40,6 +40,7 @@ interface SearchInterface public function getOperators(): Collection; public function getWordsAsString(): string; + public function getExcludedWordsAsString(): string; public function hasModifiers(): bool; diff --git a/resources/lang/en_US/firefly.php b/resources/lang/en_US/firefly.php index bd38b2404d..65e810481c 100644 --- a/resources/lang/en_US/firefly.php +++ b/resources/lang/en_US/firefly.php @@ -330,6 +330,7 @@ return [ 'search_found_transactions' => 'Firefly III found :count transaction in :time seconds.|Firefly III found :count transactions in :time seconds.', 'search_found_more_transactions' => 'Firefly III found more than :count transactions in :time seconds.', 'search_for_query' => 'Firefly III is searching for transactions with all of these words in them: :query', + 'search_for_excluded_words' => 'Firefly III is searching for transactions with none of these words in them: :excluded_words', 'invalid_operators_list' => 'These search parameters are not valid and have been ignored.', // old diff --git a/resources/views/search/index.twig b/resources/views/search/index.twig index b48dfd069e..f4346c55b1 100644 --- a/resources/views/search/index.twig +++ b/resources/views/search/index.twig @@ -43,6 +43,11 @@ {{ trans('firefly.search_for_query', {query: query|escape})|raw }}

{% endif %} + {% if '' != excludedWords %} +

+ {{ trans('firefly.search_for_excluded_words', {excluded_words: excludedWords|escape})|raw }} +

+ {% endif %} {% if invalidOperators|length > 0 %}

{{ trans('firefly.invalid_operators_list') }}

@@ -70,7 +75,7 @@ - {% if query or operators|length > 0 %} + {% if query or excludedWords or operators|length > 0 %}
@@ -125,7 +130,7 @@
{% endif %} - {% if query == "" and operators|length == 0 %} + {% if query == "" and excludedWords == "" and operators|length == 0 %}
From 50aff9cfb64e592e42d818b8d3ae09654da64307 Mon Sep 17 00:00:00 2001 From: Sobuno Date: Fri, 3 Jan 2025 00:13:51 +0100 Subject: [PATCH 24/27] Make prohibited word search work --- app/Support/Search/OperatorQuerySearch.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/app/Support/Search/OperatorQuerySearch.php b/app/Support/Search/OperatorQuerySearch.php index e37b90e282..2bdbe40c5b 100644 --- a/app/Support/Search/OperatorQuerySearch.php +++ b/app/Support/Search/OperatorQuerySearch.php @@ -2774,7 +2774,7 @@ class OperatorQuerySearch implements SearchInterface public function searchTransactions(): LengthAwarePaginator { $this->parseTagInstructions(); - if (0 === count($this->getWords()) && 0 === count($this->getOperators())) { + if (0 === count($this->getWords()) && 0 === count($this->getExcludedWords()) && 0 === count($this->getOperators())) { return new LengthAwarePaginator([], 0, 5, 1); } @@ -2831,6 +2831,11 @@ class OperatorQuerySearch implements SearchInterface return $this->words; } + public function getExcludedWords(): array + { + return $this->prohibitedWords; + } + public function setDate(Carbon $date): void { $this->date = $date; From 36ec1daf3a2ca36964d2e84ca87aad46644c40b8 Mon Sep 17 00:00:00 2001 From: Sobuno Date: Fri, 3 Jan 2025 00:29:38 +0100 Subject: [PATCH 25/27] Update UI --- resources/lang/en_US/firefly.php | 6 ++--- resources/views/search/index.twig | 40 +++++++++++++++---------------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/resources/lang/en_US/firefly.php b/resources/lang/en_US/firefly.php index 65e810481c..f10e621e4f 100644 --- a/resources/lang/en_US/firefly.php +++ b/resources/lang/en_US/firefly.php @@ -329,8 +329,9 @@ return [ 'search_query' => 'Query', 'search_found_transactions' => 'Firefly III found :count transaction in :time seconds.|Firefly III found :count transactions in :time seconds.', 'search_found_more_transactions' => 'Firefly III found more than :count transactions in :time seconds.', - 'search_for_query' => 'Firefly III is searching for transactions with all of these words in them: :query', - 'search_for_excluded_words' => 'Firefly III is searching for transactions with none of these words in them: :excluded_words', + 'search_for_overview' => 'Firefly III is searching for transactions that fulfill all of the following conditions:', + 'search_for_query' => 'All of these words must be present: :query', + 'search_for_excluded_words' => 'None of these words may be present: :excluded_words', 'invalid_operators_list' => 'These search parameters are not valid and have been ignored.', // old @@ -730,7 +731,6 @@ return [ // Ignore this comment // END - 'modifiers_applies_are' => 'The following modifiers are applied to the search as well:', 'general_search_error' => 'An error occurred while searching. Please check the log files for more information.', 'search_box' => 'Search', 'search_box_intro' => 'Welcome to the search function of Firefly III. Enter your search query in the box. Make sure you check out the help file because the search is pretty advanced.', diff --git a/resources/views/search/index.twig b/resources/views/search/index.twig index f4346c55b1..844f00f16a 100644 --- a/resources/views/search/index.twig +++ b/resources/views/search/index.twig @@ -38,17 +38,31 @@ {% endif %} +

+ {{ trans('firefly.search_for_overview') |raw }} +

+
    {% if '' != query %} -

    - {{ trans('firefly.search_for_query', {query: query|escape})|raw }} -

    +
  • + {{- trans('firefly.search_for_query', {query: query|escape})|raw -}} +
    • {% endif %} {% if '' != excludedWords %} -

    - {{ trans('firefly.search_for_excluded_words', {excluded_words: excludedWords|escape})|raw }} -

    +
  • + {{- trans('firefly.search_for_excluded_words', {excluded_words: excludedWords|escape})|raw -}} +
    • {% endif %} + {% for operator in operators %} + {% if operator.prohibited %} +
  • {{- trans('firefly.search_modifier_not_'~operator.type, {value: operator.value}) -}}
    • + {% endif %} + {% if not operator.prohibited %} +
  • {{- trans('firefly.search_modifier_'~operator.type, {value: operator.value}) -}}
    • + {% endif %} + {% endfor %} +
+ {% if invalidOperators|length > 0 %}

{{ trans('firefly.invalid_operators_list') }}

    @@ -57,20 +71,6 @@ {% endfor %}
{% endif %} - - {% if operators|length > 0 %} -

{{ trans('firefly.modifiers_applies_are') }}

-
    - {% for operator in operators %} - {% if operator.prohibited %} -
  • {{ trans('firefly.search_modifier_not_'~operator.type, {value: operator.value}) }}
    • - {% endif %} - {% if not operator.prohibited %} -
  • {{ trans('firefly.search_modifier_'~operator.type, {value: operator.value}) }}
    • - {% endif %} - {% endfor %} -
- {% endif %}
From e33e3cc40fe970e596c7d94d044236cd3f51d4fe Mon Sep 17 00:00:00 2001 From: Sobuno Date: Fri, 3 Jan 2025 00:45:19 +0100 Subject: [PATCH 26/27] Tweak UI --- app/Http/Controllers/SearchController.php | 6 +++--- app/Support/Search/OperatorQuerySearch.php | 19 +++++++------------ app/Support/Search/SearchInterface.php | 3 ++- public/v1/css/firefly.css | 5 +++++ resources/views/search/index.twig | 20 ++++++++++++++------ 5 files changed, 31 insertions(+), 22 deletions(-) diff --git a/app/Http/Controllers/SearchController.php b/app/Http/Controllers/SearchController.php index 09eb820c54..ceab9169c5 100644 --- a/app/Http/Controllers/SearchController.php +++ b/app/Http/Controllers/SearchController.php @@ -83,13 +83,13 @@ class SearchController extends Controller $searcher->parseQuery($fullQuery); // words from query and operators: - $query = $searcher->getWordsAsString(); - $excludedWords = $searcher->getExcludedWordsAsString(); + $words = $searcher->getWords(); + $excludedWords = $searcher->getExcludedWords(); $operators = $searcher->getOperators(); $invalidOperators = $searcher->getInvalidOperators(); $subTitle = (string) trans('breadcrumbs.search_result', ['query' => $fullQuery]); - return view('search.index', compact('query', 'excludedWords', 'operators', 'page', 'rule', 'fullQuery', 'subTitle', 'ruleId', 'ruleChanged', 'invalidOperators')); + return view('search.index', compact('words', 'excludedWords', 'operators', 'page', 'rule', 'fullQuery', 'subTitle', 'ruleId', 'ruleChanged', 'invalidOperators')); } /** diff --git a/app/Support/Search/OperatorQuerySearch.php b/app/Support/Search/OperatorQuerySearch.php index 2bdbe40c5b..915e8ad26d 100644 --- a/app/Support/Search/OperatorQuerySearch.php +++ b/app/Support/Search/OperatorQuerySearch.php @@ -123,9 +123,14 @@ class OperatorQuerySearch implements SearchInterface return implode(' ', $this->words); } - public function getExcludedWordsAsString(): string + public function getWords(): array { - return implode(' ', $this->prohibitedWords); + return $this->words; + } + + public function getExcludedWords(): array + { + return $this->prohibitedWords; } /** @@ -2826,16 +2831,6 @@ class OperatorQuerySearch implements SearchInterface } } - public function getWords(): array - { - return $this->words; - } - - public function getExcludedWords(): array - { - return $this->prohibitedWords; - } - public function setDate(Carbon $date): void { $this->date = $date; diff --git a/app/Support/Search/SearchInterface.php b/app/Support/Search/SearchInterface.php index 3604cacdd1..3c9a78e19e 100644 --- a/app/Support/Search/SearchInterface.php +++ b/app/Support/Search/SearchInterface.php @@ -38,9 +38,10 @@ interface SearchInterface public function getModifiers(): Collection; public function getOperators(): Collection; + public function getWords(): array; public function getWordsAsString(): string; - public function getExcludedWordsAsString(): string; + public function getExcludedWords(): array; public function hasModifiers(): bool; diff --git a/public/v1/css/firefly.css b/public/v1/css/firefly.css index 2c93945e76..3b546ab095 100644 --- a/public/v1/css/firefly.css +++ b/public/v1/css/firefly.css @@ -258,6 +258,11 @@ span.twitter-typeahead { top: 46px !important; } +.search-word { + white-space: pre; + background-color: #f5f5f5; +} + /* .twitter-typeahead { width:100%; diff --git a/resources/views/search/index.twig b/resources/views/search/index.twig index 844f00f16a..07c5b20866 100644 --- a/resources/views/search/index.twig +++ b/resources/views/search/index.twig @@ -42,14 +42,22 @@ {{ trans('firefly.search_for_overview') |raw }}

    - {% if '' != query %} + {% if words|length > 0 %}
  • - {{- trans('firefly.search_for_query', {query: query|escape})|raw -}} + {{- trans('firefly.search_for_query', { + query: words + |map(word => '' ~ word|escape ~ '') + |join(' ') + })|raw -}}
    • {% endif %} - {% if '' != excludedWords %} + {% if excludedWords|length > 0 %}
  • - {{- trans('firefly.search_for_excluded_words', {excluded_words: excludedWords|escape})|raw -}} + {{- trans('firefly.search_for_excluded_words', { + excluded_words: excludedWords + |map(word => '' ~ word|escape ~ '') + |join(' ') + })|raw -}}
    • {% endif %} @@ -75,7 +83,7 @@
- {% if query or excludedWords or operators|length > 0 %} + {% if query|length > 0 or excludedWords|length > 0 or operators|length > 0 %}
@@ -130,7 +138,7 @@
{% endif %} - {% if query == "" and excludedWords == "" and operators|length == 0 %} + {% if query|length == 0 and excludedWords|length == 0 and operators|length == 0 %}
From dae7e7d507ed3fddfc7790a61b57eba3e46fa96c Mon Sep 17 00:00:00 2001 From: Sobuno Date: Fri, 3 Jan 2025 01:00:17 +0100 Subject: [PATCH 27/27] Handle excludedWords on Rule management pages as well --- .../Controllers/Rule/CreateController.php | 30 +++++++++++++------ app/Http/Controllers/Rule/EditController.php | 23 +++++++++++--- 2 files changed, 40 insertions(+), 13 deletions(-) diff --git a/app/Http/Controllers/Rule/CreateController.php b/app/Http/Controllers/Rule/CreateController.php index 9729dfe015..ab2e59b46c 100644 --- a/app/Http/Controllers/Rule/CreateController.php +++ b/app/Http/Controllers/Rule/CreateController.php @@ -89,16 +89,28 @@ class CreateController extends Controller // build triggers from query, if present. $query = (string) $request->get('from_query'); if ('' !== $query) { - $search = app(SearchInterface::class); + $search = app(SearchInterface::class); $search->parseQuery($query); - $words = $search->getWordsAsString(); - $operators = $search->getOperators()->toArray(); - if ('' !== $words) { - session()->flash('warning', trans('firefly.rule_from_search_words', ['string' => $words])); - $operators[] = [ - 'type' => 'description_contains', - 'value' => $words, - ]; + $words = $search->getWords(); + $excludedWords = $search->getExcludedWords(); + $operators = $search->getOperators()->toArray(); + if (count($words) > 0) { + session()->flash('warning', trans('firefly.rule_from_search_words', ['string' => implode('', $words)])); + foreach($words as $word) { + $operators[] = [ + 'type' => 'description_contains', + 'value' => $word, + ]; + } + } + if (count($excludedWords) > 0) { + session()->flash('warning', trans('firefly.rule_from_search_words', ['string' => implode('', $excludedWords)])); + foreach($excludedWords as $excludedWord) { + $operators[] = [ + 'type' => '-description_contains', + 'value' => $excludedWord, + ]; + } } $oldTriggers = $this->parseFromOperators($operators); } diff --git a/app/Http/Controllers/Rule/EditController.php b/app/Http/Controllers/Rule/EditController.php index a00df8ef4c..4c4e04b6fc 100644 --- a/app/Http/Controllers/Rule/EditController.php +++ b/app/Http/Controllers/Rule/EditController.php @@ -87,11 +87,26 @@ class EditController extends Controller if ('' !== $query) { $search = app(SearchInterface::class); $search->parseQuery($query); - $words = $search->getWordsAsString(); + $words = $search->getWords(); + $excludedWords = $search->getExcludedWords(); $operators = $search->getOperators()->toArray(); - if ('' !== $words) { - session()->flash('warning', trans('firefly.rule_from_search_words', ['string' => $words])); - $operators[] = ['type' => 'description_contains', 'value' => $words]; + if (count($words) > 0) { + session()->flash('warning', trans('firefly.rule_from_search_words', ['string' => implode('', $words)])); + foreach($words as $word) { + $operators[] = [ + 'type' => 'description_contains', + 'value' => $word, + ]; + } + } + if (count($excludedWords) > 0) { + session()->flash('warning', trans('firefly.rule_from_search_words', ['string' => implode('', $excludedWords)])); + foreach($excludedWords as $excludedWord) { + $operators[] = [ + 'type' => '-description_contains', + 'value' => $excludedWord, + ]; + } } $oldTriggers = $this->parseFromOperators($operators); }