Repository: ruflin/Elastica (php)

Issue:
the post_filter is unset in Query if Terms is used post_filter
$query = new \Elastica\Query();

// set Terms as post_filter
$terms = new \Elastica\Query\Terms("key", ['term']);
$query->setPostFilter($terms);

// Query::toArray() will unset post_filter because count($terms) is 0 now, thus always ignoring the post_filter
$search->setQuery($query);
$search->search();


the _params should be set correctly in the construct
Terms:__construct()
{
    ...
    if ($terms)
        $this->setParam($this->field, $this->terms)
}

Or, the temporary workaround is to call $terms->toArray() right after the Terms is initialized.


Pull request description:
Use params to configure terms query
Fix #2031 

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5349ea55..a369bef4 100755
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -32,6 +32,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * Removed remaining `_type` field usages [#2017](https://github.com/ruflin/Elastica/pull/2017)
 ### Fixed
 * Fixed type-hint for `Elastica\Search::setOptionsAndQuery()` [#2009](https://github.com/ruflin/Elastica/pull/2009)
+* Fixed terms query params resolution [#2032](https://github.com/ruflin/Elastica/pull/2032)
 ### Security
 
 ## [7.1.2](https://github.com/ruflin/Elastica/compare/7.1.1...7.1.2)
diff --git a/src/Query/Terms.php b/src/Query/Terms.php
index eb80089d..c0af77d6 100644
--- a/src/Query/Terms.php
+++ b/src/Query/Terms.php
@@ -19,16 +19,6 @@ class Terms extends AbstractQuery
      */
     private $field;
 
-    /**
-     * @var array<float|int|string>
-     */
-    private $terms;
-
-    /**
-     * @var string[]|null
-     */
-    private $lookup;
-
     /**
      * @param array<bool|float|int|string> $terms Terms list, leave empty if building a terms-lookup query
      */
@@ -39,7 +29,7 @@ class Terms extends AbstractQuery
         }
 
         $this->field = $field;
-        $this->terms = $terms;
+        $this->setTerms($terms);
     }
 
     /**
@@ -49,9 +39,7 @@ class Terms extends AbstractQuery
      */
     public function setTerms(array $terms): self
     {
-        $this->terms = $terms;
-
-        return $this;
+        return $this->setParam($this->field, $terms);
     }
 
     /**
@@ -65,37 +53,21 @@ class Terms extends AbstractQuery
             throw new \TypeError(\sprintf('Argument 1 passed to "%s()" must be a scalar, %s given.', __METHOD__, \is_object($term) ? \get_class($term) : \gettype($term)));
         }
 
-        $this->terms[] = $term;
+        $terms = $this->getParam($this->field);
 
-        return $this;
+        if (isset($terms['index'])) {
+            throw new InvalidException('Mixed terms and terms lookup are not allowed.');
+        }
+
+        return $this->addParam($this->field, $term);
     }
 
     public function setTermsLookup(string $index, string $id, string $path): self
     {
-        $this->lookup = [
+        return $this->setParam($this->field, [
             'index' => $index,
             'id' => $id,
             'path' => $path,
-        ];
-
-        return $this;
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function toArray(): array
-    {
-        if (null !== $this->lookup && \count($this->terms)) {
-            throw new InvalidException('Unable to build Terms query: only one of terms or lookup properties should be set');
-        }
-
-        if (null !== $this->lookup) {
-            $this->setParam($this->field, $this->lookup);
-        } else {
-            $this->setParam($this->field, $this->terms);
-        }
-
-        return parent::toArray();
+        ]);
     }
 }


Test patch:
diff --git a/tests/Query/TermsTest.php b/tests/Query/TermsTest.php
index 2d1ee570..4f0bf1e5 100644
--- a/tests/Query/TermsTest.php
+++ b/tests/Query/TermsTest.php
@@ -17,17 +17,21 @@ class TermsTest extends BaseTest
      */
     public function testSetTermsLookup(): void
     {
-        $terms = [
-            'index' => 'index_name',
-            'id' => '1',
-            'path' => 'terms',
+        $expected = [
+            'terms' => [
+                'name' => [
+                    'index' => 'index_name',
+                    'id' => '1',
+                    'path' => 'terms',
+                ],
+            ],
         ];
 
-        $query = new Terms('name');
-        $query->setTermsLookup('index_name', '1', 'terms');
+        $query = (new Terms('name'))
+            ->setTermsLookup('index_name', '1', 'terms')
+        ;
 
-        $data = $query->toArray();
-        $this->assertEquals($terms, $data['terms']['name']);
+        $this->assertSame($expected, $query->toArray());
     }
 
     /**
@@ -35,11 +39,13 @@ class TermsTest extends BaseTest
      */
     public function testInvalidParams(): void
     {
-        $query = new Terms('field', ['aaa', 'bbb']);
-        $query->setTermsLookup('index', '1', 'path');
-
         $this->expectException(InvalidException::class);
-        $query->toArray();
+        $this->expectExceptionMessage('Mixed terms and terms lookup are not allowed.');
+
+        (new Terms('field', ['aaa', 'bbb']))
+            ->setTermsLookup('index', '1', 'path')
+            ->addTerm('ccc')
+        ;
     }
 
     /**
@@ -48,6 +54,8 @@ class TermsTest extends BaseTest
     public function testEmptyField(): void
     {
         $this->expectException(InvalidException::class);
+        $this->expectExceptionMessage('Terms field name has to be set');
+
         new Terms('');
     }