[FRAM-161] Disallow usage of expression as column

.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

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

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.
      *

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<string,mixed>|callable(static):void $expression The expression to compile. Can be name of the column, array expression, or closure
+     * @param string|iterable<string,mixed>|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,14 +801,16 @@ 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);
         }
 
         if (!is_string($expression) && is_callable($expression)) {
             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<string, mixed> $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();

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

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(<<<MSG
-Error during execution of bulk write query : 
-- 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.
-
-MSG
-                , $e->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());
         }
     }