From bcc5c97963992a729a2d598c6945bccdf6c6d18c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?ANDRE=20s=C3=A9bastien?= <sandre@afi-sa.fr> Date: Fri, 30 Sep 2022 09:41:42 +0000 Subject: [PATCH] fix error in match for storm_query --- src/Storm/Model/Loader.php | 5 --- src/Storm/Query/Clause/Match.php | 15 ++++--- src/Storm/Query/Clause/MatchTerms.php | 23 +++++++---- src/Storm/Query/MatchBoolean.php | 18 +++++++-- src/Storm/Query/MatchRating.php | 30 +++++++------- tests/Storm/Test/LoaderQueryTest.php | 56 ++++++++++++++++++++++++++- 6 files changed, 107 insertions(+), 40 deletions(-) diff --git a/src/Storm/Model/Loader.php b/src/Storm/Model/Loader.php index ac40c5ae..afda9e42 100644 --- a/src/Storm/Model/Loader.php +++ b/src/Storm/Model/Loader.php @@ -498,9 +498,4 @@ class Storm_Model_Loader { public function fetchFirstForQuery(Storm_Query $query) : ?Storm_Model_Abstract { return $this->_findFirstFrom($this->fetchForQuery($query->limit(1))); } - - - public function greater(string $key, string $value) : Storm_Query_Clause { - return Storm_Query_Clause::greater($key, $value); - } } diff --git a/src/Storm/Query/Clause/Match.php b/src/Storm/Query/Clause/Match.php index f7ce65cb..e7f48b98 100644 --- a/src/Storm/Query/Clause/Match.php +++ b/src/Storm/Query/Clause/Match.php @@ -33,9 +33,7 @@ class Storm_Query_Clause_Match extends Storm_Query_Clause { $content = ''; foreach ($value->getTerms() as $key => $term) - $content = implode(' ', - array_filter([$content, - $term->getFormatDb($value->isStrict())])); + $content = implode(' ', array_filter([$content, $term->getFormatDb()])); $against = sprintf('AGAINST(%s%s)', Zend_Db_Table_Abstract::getDefaultAdapter() @@ -60,7 +58,7 @@ class Storm_Query_Clause_Match extends Storm_Query_Clause { foreach ($value->getTerms() as $term) { $new_compare = $term->getCompareValue($contents); - if ($value->isStrict() && 0 === $new_compare) + if ($term->isStrict() && 0 === $new_compare) return 0; $compare += $term->getCompareValue($contents); @@ -77,15 +75,16 @@ class Storm_Query_Clause_Match extends Storm_Query_Clause { || !($contents = $this->_getContents($model))) return false; + $contains_all = false; foreach ($value->getTerms() as $term) { - if ($value->isStrict() && !$term->containVolatile($contents)) + $contain = $term->containVolatile($contents); + if ($term->isStrict() && !$contain) return false; - if (!$value->isStrict() && $term->containVolatile($contents)) - return true; + $contains_all |= $contain; } - return $value->isStrict(); + return $contains_all; } diff --git a/src/Storm/Query/Clause/MatchTerms.php b/src/Storm/Query/Clause/MatchTerms.php index d255ac6b..8bccf0d9 100644 --- a/src/Storm/Query/Clause/MatchTerms.php +++ b/src/Storm/Query/Clause/MatchTerms.php @@ -29,24 +29,26 @@ class Storm_Query_Clause_MatchTerms { protected array $_values; protected bool $_exact; + protected bool $_strict; - public function __construct(array $values, bool $exact) { + public function __construct(array $values, bool $exact, bool $strict) { $this->_values = $values; $this->_exact = $exact; + $this->_strict = $strict; } - public function getFormatDb(bool $strict) : string { + public function getFormatDb() : string { if (!($count = count($this->_values))) return ''; - $format_db = $strict ? '+' : ''; - $values = $this->_getValues(); + $format_db = $this->isStrict() ? '+' : ''; + $values = $this->_implodeValues(); if ($this->_exact) return $format_db . '"' . $values . '"'; - $format_db .= (!$strict || 1 === $count) + $format_db .= (!$this->isStrict() || 1 === $count) ? '%s' : '(%s)'; @@ -56,7 +58,7 @@ class Storm_Query_Clause_MatchTerms { public function containVolatile(string $contents) : bool { if ($this->_exact) - return $this->_containValue($contents, $this->_getValues()); + return $this->_containValue($contents, $this->_implodeValues()); foreach ($this->_values as $value) if ($this->_containValue($contents, $value)) @@ -68,7 +70,7 @@ class Storm_Query_Clause_MatchTerms { public function getCompareValue(string $contents) : int { if ($this->_exact) - return preg_match_all('/\b' . $this->_getValues() . '\b/', $contents); + return preg_match_all('/\b' . $this->_implodeValues() . '\b/', $contents); $compare = 0; foreach ($this->_values as $value) @@ -80,12 +82,17 @@ class Storm_Query_Clause_MatchTerms { } + public function isStrict() : bool { + return $this->_strict; + } + + protected function _containValue(string $contents, string $value) : bool { return preg_match('/\b' . $value . '\b/', $contents); } - protected function _getValues() : string { + protected function _implodeValues() : string { return implode(' ', $this->_values); } } diff --git a/src/Storm/Query/MatchBoolean.php b/src/Storm/Query/MatchBoolean.php index 15ab7d81..9aa2df68 100644 --- a/src/Storm/Query/MatchBoolean.php +++ b/src/Storm/Query/MatchBoolean.php @@ -27,13 +27,25 @@ class Storm_Query_MatchBoolean extends Storm_Query_MatchRating { + protected bool $_boolean_mode = true; + public function __construct($array_or_key, bool $strict = false) { parent::__construct($array_or_key, $strict); - $this->_boolean_mode = true; + $this->_strict = $strict; } - public function exact($array_or_value) : self { - return $this->_addTerm($array_or_value, true); + public function exact($array_or_value, ?bool $strict = null) : self { + return $this->_addTerm($array_or_value, $strict, true); + } + + + protected function _createNewMatchTerms($array_or_value, ?bool $strict, bool $exact = false) : Storm_Query_Clause_MatchTerms { + if (null === $strict) + $strict = $this->_strict; + + return (new Storm_Query_Clause_MatchTerms($array_or_value, + $exact, + $strict)); } } diff --git a/src/Storm/Query/MatchRating.php b/src/Storm/Query/MatchRating.php index 8dfcb720..1a1ddfb9 100644 --- a/src/Storm/Query/MatchRating.php +++ b/src/Storm/Query/MatchRating.php @@ -29,16 +29,15 @@ class Storm_Query_MatchRating { protected string $_key; protected array $_terms; - protected bool $_boolean_mode; - protected bool $_strict; + protected bool $_boolean_mode = false; + protected bool $_strict = false; public function __construct($array_or_key, bool $strict = false) { $this->_key = is_array($array_or_key) ? implode(', ', $array_or_key) : $array_or_key; + $this->_terms = []; - $this->_boolean_mode = false; - $this->_strict = $strict; } @@ -47,11 +46,6 @@ class Storm_Query_MatchRating { } - public function isStrict() : bool { - return $this->_strict; - } - - public function getKey() : string { return $this->_key; } @@ -62,17 +56,25 @@ class Storm_Query_MatchRating { } - public function against($array_or_value) : self { - return $this->_addTerm($array_or_value); + public function against($array_or_value, ?bool $strict = null) : self { + return $this->_addTerm($array_or_value, $strict); } - protected function _addTerm($array_or_value, bool $exact = false) : self { + protected function _addTerm($array_or_value, + ?bool $strict, + bool $exact = false) : self { if (!is_array($array_or_value)) $array_or_value = explode(' ', $array_or_value); - $this->_terms [] = (new Storm_Query_Clause_MatchTerms($array_or_value, - $exact)); + $this->_terms [] = $this->_createNewMatchTerms($array_or_value, $strict, $exact); return $this; } + + + protected function _createNewMatchTerms($array_or_value, ?bool $strict, bool $exact = false) : Storm_Query_Clause_MatchTerms { + return (new Storm_Query_Clause_MatchTerms($array_or_value, + $exact, + false)); + } } diff --git a/tests/Storm/Test/LoaderQueryTest.php b/tests/Storm/Test/LoaderQueryTest.php index 297c8bdb..9d318043 100644 --- a/tests/Storm/Test/LoaderQueryTest.php +++ b/tests/Storm/Test/LoaderQueryTest.php @@ -263,7 +263,7 @@ class Storm_Test_LoaderQueryDbTest extends Storm_Test_LoaderQueryTestCase { /** @test */ - public function withClauseMatchStrictAndOneExactExpressionWithNoBooleanModeShouldReturnQuery() { + public function withClauseMatchStrictAndOneExactExpressionWithBooleanModeShouldReturnQuery() { Storm_Query::from(Storm_Test_Mock_User::class) ->match((new Storm_Query_MatchBoolean(['login', 'role'], true)) ->exact('ADMIN INVITE') @@ -273,6 +273,32 @@ class Storm_Test_LoaderQueryDbTest extends Storm_Test_LoaderQueryTestCase { $this->assertEquals(['MATCH(login, role) AGAINST(\'+"ADMIN INVITE" +HUGO\' IN BOOLEAN MODE)'], $this->_select->getAttributesForLastCallOn('where')); } + + + /** @test */ + public function withClauseMatchOneExactAndOneAgainstStrictInBooleanModeShouldReturnQuery() { + Storm_Query::from(Storm_Test_Mock_User::class) + ->match((new Storm_Query_MatchBoolean(['login', 'role'])) + ->exact('ADMIN INVITE', true) + ->against('HUGO')) + ->fetchAll(); + + $this->assertEquals(['MATCH(login, role) AGAINST(\'+"ADMIN INVITE" HUGO\' IN BOOLEAN MODE)'], + $this->_select->getAttributesForLastCallOn('where')); + } + + + /** @test */ + public function withClauseMatchOneExactStrictAndOneAgainstInBooleanModeShouldReturnQuery() { + Storm_Query::from(Storm_Test_Mock_User::class) + ->match((new Storm_Query_MatchBoolean(['login', 'role'])) + ->exact('ADMIN INVITE') + ->against('HUGO', true)) + ->fetchAll(); + + $this->assertEquals(['MATCH(login, role) AGAINST(\'"ADMIN INVITE" +HUGO\' IN BOOLEAN MODE)'], + $this->_select->getAttributesForLastCallOn('where')); + } } @@ -615,7 +641,7 @@ class Storm_Test_LoaderQueryVolatileClauseWhereTest extends Storm_Test_ModelTest /** @test */ - public function withMatchLevelAgainstExactDeuxiemeTroisiemeShouldAnswersOnlyUser_Admin() { + public function withMatchLevelFooExactDeuxiemeRedacteurShouldAnswersOnlyUser_Administrateur() { $query = Storm_Query::from(Storm_Test_LoaderQueryUser::class) ->match((new Storm_Query_MatchBoolean('level,foo')) ->exact('deuxieme redacteur')); @@ -626,6 +652,32 @@ class Storm_Test_LoaderQueryVolatileClauseWhereTest extends Storm_Test_ModelTest } + /** @test */ + public function withMatchLevelFooDeuxiemeAndAgainstRedacteurAnswers_AllUsers() { + $query = Storm_Query::from(Storm_Test_LoaderQueryUser::class) + ->match((new Storm_Query_MatchBoolean('level,foo')) + ->against('deuxieme') + ->against('redacteur')); + $this->assertEquals(['user_admin', 'user_administrateur', 'user_invite'], + (new Storm_Model_Collection($query->fetchAll())) + ->collect('login') + ->getArrayCopy()); + } + + + /** @test */ + public function withMatchLevelFooDeuxiemeAndAgainstStrictRedacteurAnswers_User_Admin_User_Administrateur() { + $query = Storm_Query::from(Storm_Test_LoaderQueryUser::class) + ->match((new Storm_Query_MatchBoolean('level,foo')) + ->against('deuxieme') + ->against('redacteur', true)); + $this->assertEquals(['user_admin', 'user_administrateur'], + (new Storm_Model_Collection($query->fetchAll())) + ->collect('login') + ->getArrayCopy()); + } + + /** @test */ public function withOrClause_LevelInviteOrFooLikePremierShouldAnswersUser_AdminAndUser_Invite() { $query = Storm_Query::from(Storm_Test_LoaderQueryUser::class) -- GitLab