diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index 49d393f..ee3a6ed 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -1,6 +1,10 @@ name: Build -on: [push, pull_request] +on: + push: + branches: [ '[1-9].[0-9]+' ] + pull_request: + branches: [ '[1-9].[0-9]+' ] env: ELASTICSEARCH_HOST: '127.0.0.1:9200' @@ -45,9 +49,9 @@ jobs: sudo sysctl -w vm.max_map_count=262144 - name: Runs Elasticsearch - uses: elastic/elastic-github-actions/elasticsearch@master + uses: elastic/elastic-github-actions/elasticsearch@dc110609b1cb3024477ead739ca23ab547b8b9ff with: - stack-version: 7.17.1 + stack-version: 7.17.28 - name: Validate composer.json and composer.lock run: composer validate --strict @@ -67,9 +71,9 @@ jobs: fail-fast: false matrix: versions: - - es: '7.17.1' + - es: '7.17.28' lib: '7.0' - - es: '8.1.0' + - es: '8.19.12' lib: '8.0' name: Elasticsearch version ${{ matrix.versions.es }} @@ -97,7 +101,7 @@ jobs: sudo sysctl -w vm.max_map_count=262144 - name: Runs Elasticsearch ${{ matrix.versions.es }} - uses: elastic/elastic-github-actions/elasticsearch@master + uses: elastic/elastic-github-actions/elasticsearch@dc110609b1cb3024477ead739ca23ab547b8b9ff with: stack-version: ${{ matrix.versions.es }} security-enabled: false @@ -230,10 +234,10 @@ jobs: sudo sysctl -w fs.file-max=262144 sudo sysctl -w vm.max_map_count=262144 - - name: Runs Elasticsearch 8.1.0 - uses: elastic/elastic-github-actions/elasticsearch@master + - name: Runs Elasticsearch 8.19.12 + uses: elastic/elastic-github-actions/elasticsearch@dc110609b1cb3024477ead739ca23ab547b8b9ff with: - stack-version: 8.1.0 + stack-version: 8.19.12 security-enabled: false - name: Install dependencies diff --git a/src/Bundle/PrimeIndexerBundle.php b/src/Bundle/PrimeIndexerBundle.php index 7eb5eb3..b8190b0 100644 --- a/src/Bundle/PrimeIndexerBundle.php +++ b/src/Bundle/PrimeIndexerBundle.php @@ -12,7 +12,7 @@ */ class PrimeIndexerBundle extends Bundle { - public function build(ContainerBuilder $container) + public function build(ContainerBuilder $container): void { $container->addCompilerPass(new RegisterIndexFactoryCompilerPass()); $container->addCompilerPass(new RegisterIndexConfigurationCompilerPass()); diff --git a/src/Elasticsearch/Query/Compound/BooleanQuery.php b/src/Elasticsearch/Query/Compound/BooleanQuery.php index 4d468e2..47c59fe 100644 --- a/src/Elasticsearch/Query/Compound/BooleanQuery.php +++ b/src/Elasticsearch/Query/Compound/BooleanQuery.php @@ -4,6 +4,13 @@ use Bdf\Prime\Indexer\Elasticsearch\Grammar\ElasticsearchGrammarInterface; use Bdf\Prime\Indexer\Elasticsearch\Query\CompilableExpressionInterface; +use Closure; + +use function array_keys; +use function array_merge; +use function array_values; +use function count; +use function end; /** * A compound query with boolean combinations. @@ -54,6 +61,38 @@ public function filter($query): BooleanQuery return $this; } + /** + * Remove a filter matching the given predicate + * If multiple filters match the predicate, all of them will be removed + * + * Note: this method will only remove filters that have been added using the filter() method + * + * @param Closure(array|CompilableExpressionInterface):bool $predicate The predicate. Takes a filter as parameter and returns true if it should be removed + * + * @return bool true if at least one filter has been removed, false if no filter matched the predicate + * @see BooleanQuery::filter() To add a filter + */ + public function removeFilter(Closure $predicate): bool + { + $filters = $this->filter; + $hasChanged = false; + + foreach ($filters as $key => $filter) { + if ($predicate($filter)) { + unset($filters[$key]); + $hasChanged = true; + } + } + + if ($hasChanged) { + $this->filter = array_values($filters); + + return true; + } + + return false; + } + /** * The clause (query) must not appear in the matching documents. * diff --git a/src/Elasticsearch/Query/ElasticsearchQuery.php b/src/Elasticsearch/Query/ElasticsearchQuery.php index 22daf39..c518272 100644 --- a/src/Elasticsearch/Query/ElasticsearchQuery.php +++ b/src/Elasticsearch/Query/ElasticsearchQuery.php @@ -5,7 +5,6 @@ use Bdf\Collection\Stream\ArrayStream; use Bdf\Collection\Stream\StreamInterface; use Bdf\Collection\Util\OptionalInterface; -use Bdf\Prime\Connection\Result\ResultSetInterface; use Bdf\Prime\Indexer\Elasticsearch\Adapter\ClientInterface; use Bdf\Prime\Indexer\Elasticsearch\Adapter\Exception\ElasticsearchExceptionInterface; use Bdf\Prime\Indexer\Elasticsearch\Adapter\Response\SearchResults; @@ -24,8 +23,21 @@ use Bdf\Prime\Indexer\QueryInterface; use Bdf\Prime\Query\Contract\Limitable; use Bdf\Prime\Query\Contract\Orderable; -use Closure; +use Bdf\Prime\Query\Expression\ExpressionInterface as PrimeExpressionInterface; +use Bdf\Prime\Query\QueryInterface as PrimeQueryInterface; use Countable; +use InvalidArgumentException; +use TypeError; + +use function array_replace; +use function ceil; +use function explode; +use function is_array; +use function is_callable; +use function is_int; +use function is_iterable; +use function is_string; +use function trim; /** * Query for perform index search @@ -185,71 +197,96 @@ public function wrap(WrappingQueryInterface $wrapper) */ public function where($column, $operator = null, $value = null) { - /** @psalm-suppress PossiblyInvalidArgument */ + if ($column instanceof PrimeExpressionInterface) { + throw new InvalidArgumentException('Field cannot be a Prime expression on elasticsearch. Use a string, or whereRaw() instead.'); + } + return $this->buildWhere($column, $operator, $value, BooleanQuery::COMPOSITE_AND); } + /** + * {@inheritdoc} + */ + public function whereReplace(string $column, $operator = null, $value = null) + { + if ($value === null && (!is_string($operator) || !isset($this->operators[$operator]))) { + $value = $operator; + $operator = '='; + } + + $this->bool()->and()->removeFilter(fn ($filter) => $filter instanceof WhereFilter && $filter->column() === $column && $filter->operator() === $operator); + $this->bool()->and()->filter(new WhereFilter($column, $operator, $value)); + + return $this; + } + /** * {@inheritdoc} */ public function orWhere($column, $operator = null, $value = null) { - /** @psalm-suppress PossiblyInvalidArgument */ + if ($column instanceof PrimeExpressionInterface) { + throw new InvalidArgumentException('Field cannot be a Prime expression on elasticsearch. Use a string, or whereRaw() instead.'); + } + return $this->buildWhere($column, $operator, $value, BooleanQuery::COMPOSITE_OR); } /** * {@inheritdoc} - * - * @psalm-suppress MoreSpecificImplementedParamType */ - public function whereNull(string $column, string $type = BooleanQuery::COMPOSITE_AND) + public function whereNull($column, string $type = BooleanQuery::COMPOSITE_AND) { - /** @psalm-suppress PossiblyInvalidArgument */ + if (!is_string($column)) { + throw new InvalidArgumentException('Field name must be a string'); + } + return $this->whereRaw(new Missing($column), $type); } /** * {@inheritdoc} - * - * @psalm-suppress MoreSpecificImplementedParamType */ - public function whereNotNull(string $column, string $type = BooleanQuery::COMPOSITE_AND) + public function whereNotNull($column, string $type = BooleanQuery::COMPOSITE_AND) { - /** @psalm-suppress PossiblyInvalidArgument */ + if (!is_string($column)) { + throw new InvalidArgumentException('Field name must be a string'); + } + return $this->whereRaw(new Exists($column), $type); } /** * {@inheritdoc} - * - * @psalm-suppress MoreSpecificImplementedParamType */ - public function orWhereNull(string $column) + public function orWhereNull($column) { - /** @psalm-suppress PossiblyInvalidArgument */ return $this->whereNull($column, BooleanQuery::COMPOSITE_OR); } /** * {@inheritdoc} - * - * @psalm-suppress MoreSpecificImplementedParamType */ - public function orWhereNotNull(string $column) + public function orWhereNotNull($column) { - /** @psalm-suppress PossiblyInvalidArgument */ return $this->whereNotNull($column, BooleanQuery::COMPOSITE_OR); } /** * {@inheritdoc} * - * @param string|\Bdf\Prime\Query\QueryInterface|\Bdf\Prime\Query\Expression\ExpressionInterface|array|CompilableExpressionInterface $raw - * @psalm-suppress PossiblyInvalidArgument + * @param string|PrimeQueryInterface|\Bdf\Prime\Query\Expression\ExpressionInterface|array|CompilableExpressionInterface $raw */ public function whereRaw($raw, string $type = BooleanQuery::COMPOSITE_AND) { + if ($raw instanceof PrimeQueryInterface) { + throw new InvalidArgumentException('Cannot use Prime query as raw expression'); + } + + if ($raw instanceof PrimeExpressionInterface || is_string($raw)) { + throw new InvalidArgumentException('Prime expression are not supported by elasticsearch query. Use CompilableExpressionInterface instead.'); + } + switch ($type) { case BooleanQuery::COMPOSITE_AND: $this->bool()->and()->filter($raw); @@ -318,7 +355,7 @@ public function order($sort, ?string $order = 'asc') { if (!is_array($sort)) { if (!is_string($sort)) { - throw new \TypeError('$sort must be of type string or array'); + throw new TypeError('$sort must be of type string or array'); } $this->order = [$sort => $order]; @@ -338,7 +375,7 @@ public function addOrder($sort, ?string $order = 'asc') $this->order = array_replace($this->order, $sort); } else { if (!is_string($sort)) { - throw new \TypeError('$sort must be of type string or array'); + throw new TypeError('$sort must be of type string or array'); } $this->order[$sort] = $order; @@ -754,7 +791,7 @@ public function bool(): BooleanQuery /** * Build simple where expression * - * @param string|array|callable(static):void $expression The expression to compile. Can be name of the column, array expression, or closure + * @param string|iterable|callable(static):void $expression The expression to compile. Can be name of the column, array expression, or closure * @param string|mixed $operator The operator (if first argument is column name), or value if value is not given * @param mixed $value The comparison value if first argument is the column name * @param string $type The composite expression type (and/or) @@ -764,6 +801,8 @@ public function bool(): BooleanQuery private function buildWhere($expression, $operator, $value, $type) { if ($expression instanceof CompilableExpressionInterface) { + @trigger_error('Using CompilableExpressionInterface as expression is deprecated. Use whereRaw() instead.', E_USER_DEPRECATED); + return $this->whereRaw($expression, $type); } @@ -771,7 +810,7 @@ private function buildWhere($expression, $operator, $value, $type) return $this->nested($expression, $type); } - if (is_array($expression)) { + if (is_iterable($expression)) { return $this->buildArrayExpression($expression, $type); } @@ -795,12 +834,12 @@ private function buildWhere($expression, $operator, $value, $type) /** * Build array expression * - * @param array $expression + * @param iterable $expression * @param string $type * * @return $this */ - private function buildArrayExpression(array $expression, $type = BooleanQuery::COMPOSITE_AND) + private function buildArrayExpression(iterable $expression, $type = BooleanQuery::COMPOSITE_AND) { //nested expression $bool = new BooleanQuery(); diff --git a/src/Elasticsearch/Query/Filter/WhereFilter.php b/src/Elasticsearch/Query/Filter/WhereFilter.php index 20a5364..6cfdcc1 100644 --- a/src/Elasticsearch/Query/Filter/WhereFilter.php +++ b/src/Elasticsearch/Query/Filter/WhereFilter.php @@ -42,4 +42,20 @@ public function compile(ElasticsearchGrammarInterface $grammar): array { return $grammar->operator($this->column, $this->operator, $this->value); } + + /** + * Get the field name to filter + */ + public function column(): string + { + return $this->column; + } + + /** + * Get the used operator + */ + public function operator(): string + { + return $this->operator; + } } diff --git a/tests/Elasticsearch/Query/Bulk/ElasticsearchBulkQueryTest.php b/tests/Elasticsearch/Query/Bulk/ElasticsearchBulkQueryTest.php index c6624a9..07b9743 100644 --- a/tests/Elasticsearch/Query/Bulk/ElasticsearchBulkQueryTest.php +++ b/tests/Elasticsearch/Query/Bulk/ElasticsearchBulkQueryTest.php @@ -723,12 +723,8 @@ public function test_invalid_query_bulk() $this->fail('Expected exception to be thrown'); } catch (BulkWriteException $e) { $id = $e->errors()[0]['_id']; - $this->assertEquals(<<getMessage()); + $this->assertStringContainsString('Error during execution of bulk write query :', $e->getMessage()); + $this->assertStringContainsString("failed to parse field [enabled] of type [boolean] in document with id '{$id}'. Preview of field's value: 'invalid' Caused by: Failed to parse value [invalid] as only [true] or [false] are allowed.", $e->getMessage()); } } diff --git a/tests/Elasticsearch/Query/Compound/BooleanQueryTest.php b/tests/Elasticsearch/Query/Compound/BooleanQueryTest.php index b01227c..d8a65e3 100644 --- a/tests/Elasticsearch/Query/Compound/BooleanQueryTest.php +++ b/tests/Elasticsearch/Query/Compound/BooleanQueryTest.php @@ -6,6 +6,7 @@ use Bdf\Prime\Indexer\Elasticsearch\Query\Filter\MatchBoolean; use Bdf\Prime\Indexer\Elasticsearch\Query\Filter\Missing; use Bdf\Prime\Indexer\Elasticsearch\Query\Filter\Range; +use Bdf\Prime\Indexer\Elasticsearch\Query\Filter\WhereFilter; use Bdf\Prime\Indexer\Elasticsearch\Query\Filter\Wildcard; use PHPUnit\Framework\TestCase; @@ -43,6 +44,45 @@ public function test_simple() ], $bool->compile(new ElasticsearchGrammar())); } + public function test_removeFilter_not_matching() + { + $bool = new BooleanQuery(); + $bool + ->filter(new WhereFilter('name', '=', 'Paris')) + ->filter(new WhereFilter('zipCode', '=', '75000')) + ; + + $removed = clone $bool; + $this->assertFalse($removed->removeFilter(fn ($filter) => $filter->column() === 'not_found')); + $this->assertEquals($bool, $removed); + } + + public function test_removeFilter_matching() + { + $bool = new BooleanQuery(); + $bool + ->filter(new WhereFilter('name', '=', 'Paris')) + ->filter(new WhereFilter('zipCode', '=', '75000')) + ; + + $this->assertTrue($bool->removeFilter(fn ($filter) => $filter->column() === 'name')); + $this->assertEquals((new BooleanQuery())->filter(new WhereFilter('zipCode', '=', '75000')), $bool); + } + + public function test_removeFilter_matching_multiple() + { + $bool = new BooleanQuery(); + $bool + ->filter(new WhereFilter('name', '=', 'Paris')) + ->filter(new WhereFilter('zipCode', '=', '75000')) + ->filter(new WhereFilter('name', '>', 'A')) + ->filter(new WhereFilter('name', '<', 'Z')) + ; + + $this->assertTrue($bool->removeFilter(fn ($filter) => $filter->column() === 'name')); + $this->assertEquals((new BooleanQuery())->filter(new WhereFilter('zipCode', '=', '75000')), $bool); + } + /** * */ diff --git a/tests/Elasticsearch/Query/ElasticsearchCreateQueryTest.php b/tests/Elasticsearch/Query/ElasticsearchCreateQueryTest.php index 962c1cb..fa6c729 100644 --- a/tests/Elasticsearch/Query/ElasticsearchCreateQueryTest.php +++ b/tests/Elasticsearch/Query/ElasticsearchCreateQueryTest.php @@ -595,12 +595,8 @@ public function test_invalid_query_bulk() $this->fail('Expected exception to be thrown'); } catch (BulkWriteException $e) { $id = $e->errors()[0]['_id']; - $this->assertEquals(<<getMessage()); + $this->assertStringContainsString('Error during execution of bulk write query :', $e->getMessage()); + $this->assertStringContainsString("failed to parse field [enabled] of type [boolean] in document with id '{$id}'. Preview of field's value: 'invalid' Caused by: Failed to parse value [invalid] as only [true] or [false] are allowed.", $e->getMessage()); } } diff --git a/tests/Elasticsearch/Query/ElasticsearchQueryTest.php b/tests/Elasticsearch/Query/ElasticsearchQueryTest.php index 0764794..4fbc641 100644 --- a/tests/Elasticsearch/Query/ElasticsearchQueryTest.php +++ b/tests/Elasticsearch/Query/ElasticsearchQueryTest.php @@ -13,7 +13,11 @@ use Bdf\Prime\Indexer\Elasticsearch\Query\Result\ElasticsearchPaginator; use Bdf\Prime\Indexer\Exception\QueryExecutionException; use Bdf\Prime\Indexer\IndexTestCase; +use Bdf\Prime\Query\Expression\Attribute; +use Bdf\Prime\Query\Expression\Field; +use Bdf\Prime\Query\QueryInterface; use Elastic\Elasticsearch\Client; +use InvalidArgumentException; /** * Class ElasticsearchQueryTest @@ -64,6 +68,74 @@ public function test_simple_where() ); } + /** + * + */ + public function test_simple_whereReplace() + { + $query = $this->query->from('cities', 'city'); + + $this->assertEquals( + [ + 'query' => [ + 'bool' => [ + 'filter' => [ + ['term' => ['name' => 'Paris']] + ] + ] + ] + ], + $query->whereReplace('name', 'Paris')->compile() + ); + + $this->assertEquals( + [ + 'query' => [ + 'bool' => [ + 'filter' => [ + ['term' => ['name' => 'Avignon']] + ] + ] + ] + ], + $query->whereReplace('name', 'Avignon')->compile() + ); + } + + /** + * + */ + public function test_whereReplace_with_operator() + { + $query = $this->query->from('cities', 'city'); + + $this->assertEquals( + [ + 'query' => [ + 'bool' => [ + 'filter' => [ + ['range' => ['name' => ['gt' => 'A']]] + ] + ] + ] + ], + $query->whereReplace('name', '>', 'A')->compile() + ); + + $this->assertEquals( + [ + 'query' => [ + 'bool' => [ + 'filter' => [ + ['range' => ['name' => ['gt' => 'B']]] + ] + ] + ] + ], + $query->whereReplace('name', '>', 'B')->compile() + ); + } + /** * */ @@ -165,6 +237,63 @@ public function test_where_multiple() ); } + /** + * + */ + public function test_where_multiple_replace() + { + $query = $this->query->from('cities', 'city'); + + $this->assertEquals( + [ + 'query' => [ + 'bool' => [ + 'filter' => [ + ['term' => ['name' => 'Paris']], + ['term' => ['zipCode' => '75000']], + ] + ] + ] + ], + $query + ->where('name', 'Paris') + ->where('zipCode', '75000') + ->compile() + ); + + $this->assertEquals( + [ + 'query' => [ + 'bool' => [ + 'filter' => [ + ['term' => ['zipCode' => '75000']], + ['term' => ['name' => 'Avignon']], + ] + ] + ] + ], + $query + ->whereReplace('name', 'Avignon') + ->compile() + ); + + $this->assertEquals( + [ + 'query' => [ + 'bool' => [ + 'filter' => [ + ['term' => ['name' => 'Avignon']], + ['term' => ['zipCode' => '84001']], + ] + ] + ] + ], + $query + ->whereReplace('zipCode', '84001') + ->compile() + ); + } + /** * */ @@ -191,6 +320,26 @@ public function test_where_with_array() ); } + /** + * + */ + public function test_where_with_expression_not_supported() + { + $this->expectException(InvalidArgumentException::class); + + $this->query->from('cities', 'city')->where(new Attribute('foo')); + } + + /** + * + */ + public function test_orWhere_with_expression_not_supported() + { + $this->expectException(InvalidArgumentException::class); + + $this->query->from('cities', 'city')->orWhere(new Attribute('foo')); + } + /** * */ @@ -666,6 +815,20 @@ public function test_limit_getters() $this->assertTrue($this->query->hasPagination()); } + public function test_whereRaw_expression_not_supported() + { + $this->expectException(InvalidArgumentException::class); + + $this->query->whereRaw(new Attribute('foo')); + } + + public function test_whereRaw_query_not_supported() + { + $this->expectException(InvalidArgumentException::class); + + $this->query->whereRaw($this->createMock(QueryInterface::class)); + } + /** * */