From 631ed4956aa758c9a592d3d13735aa829394ab32 Mon Sep 17 00:00:00 2001 From: Sobuno Date: Wed, 1 Jan 2025 05:08:01 +0100 Subject: [PATCH] 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()); + } }