Deprecate passing string to Search::addIndex, hasIndex and addIndices (…

…#2106) Co-authored-by: Fran Moreno <franmomu@gmail.com>
ruflin · Aug 2, 2022 · 8a3cf51 · 8a3cf51
1 parent 88fb974
commit 8a3cf51
Show file tree

Hide file tree

Showing 5 changed files with 248 additions and 11 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * Added coverage check to CI by @franmomu [#2071](https://github.com/ruflin/Elastica/pull/2071)
 * Added `string` as a valid type for `data` in `Request`  by @franmomu [#2078](https://github.com/ruflin/Elastica/pull/2078)
 * Added missing `throws` PHPDoc tags by @franmomu [#2077](https://github.com/ruflin/Elastica/pull/2077)
+* Added `Search::addIndexByName()`, `Search::hasIndexByName()` and `Search::addIndicesByName()` by @franmomu [#2103](https://github.com/ruflin/Elastica/pull/2103)
 
 ### Changed
 * Updated `symfony/phpunit-bridge` to `6.0` by @franmomu [#2067](https://github.com/ruflin/Elastica/pull/2067)
@@ -28,6 +29,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Deprecated
 * Deprecated `Elastica\Reindex::WAIT_FOR_COMPLETION_FALSE`, use a boolean as parameter instead by @franmomu [#2070](https://github.com/ruflin/Elastica/pull/2070)
 * Passing anything else than a boolean as 1st argument to `Reindex::setWaitForCompletion`, pass a boolean instead by @franmomu [#2070](https://github.com/ruflin/Elastica/pull/2070)
+* Deprecated passing a `string` as 1st argument to `Search::addIndex()` and `Search::hasIndex()`, pass an Index instance instead by @franmomu [#2103](https://github.com/ruflin/Elastica/pull/2103)
+* Deprecated passing an array of `string` as 1st argument to `Search::addIndices()`, use an array of Index instances by @franmomu [#2103](https://github.com/ruflin/Elastica/pull/2103)
+
 ### Removed
 * Removed `egeloen/http-adapter` as suggested package since the project is abandoned by @franmomu [#2069](https://github.com/ruflin/Elastica/pull/2069)
 * Removed `0` as valid request data using Analyze API by @franmomu [#2068](https://github.com/ruflin/Elastica/pull/2068)

diff --git a/src/Search.php b/src/Search.php
@@ -80,39 +80,87 @@ public function __construct(Client $client, ?BuilderInterface $builder = null)
     /**
      * Adds a index to the list.
      *
-     * @param Index|string $index Index object or string
+     * @param Index $index Index object or string
      *
      * @throws InvalidException
      */
     public function addIndex($index): self
     {
         if ($index instanceof Index) {
             $index = $index->getName();
+        } else {
+            \trigger_deprecation(
+                'ruflin/elastica',
+                '7.2.0',
+                'Passing a string as 1st argument to "%s()" is deprecated, pass an Index instance or use "addIndexByName" instead. It will throw a %s in 8.0.',
+                __METHOD__,
+                \TypeError::class
+            );
         }
 
         if (!\is_scalar($index)) {
             throw new InvalidException('Invalid param type');
         }
 
-        $this->_indices[] = (string) $index;
+        return $this->addIndexByName((string) $index);
+    }
+
+    /**
+     * Adds an index to the list.
+     */
+    public function addIndexByName(string $index): self
+    {
+        $this->_indices[] = $index;
 
         return $this;
     }
 
     /**
      * Add array of indices at once.
      *
-     * @param Index[]|string[] $indices
+     * @param Index[] $indices
      */
     public function addIndices(array $indices = []): self
     {
         foreach ($indices as $index) {
+            if (\is_string($index)) {
+                \trigger_deprecation(
+                    'ruflin/elastica',
+                    '7.2.0',
+                    'Passing a array of strings as 1st argument to "%s()" is deprecated, pass an array of Indexes or use "addIndicesByName" instead. It will throw a %s in 8.0.',
+                    __METHOD__,
+                    \TypeError::class
+                );
+                $this->addIndexByName($index);
+
+                continue;
+            }
+
+            if (!$index instanceof Index) {
+                throw new InvalidException('Invalid param type for addIndices(), expected Index[]');
+            }
+
             $this->addIndex($index);
         }
 
         return $this;
     }
 
+    /**
+     * @param string[] $indices
+     */
+    public function addIndicesByName(array $indices = []): self
+    {
+        foreach ($indices as $index) {
+            if (!\is_string($index)) {
+                throw new InvalidException('Invalid param type for addIndicesByName(), expected string[]');
+            }
+            $this->addIndexByName($index);
+        }
+
+        return $this;
+    }
+
     /**
      * @param AbstractQuery|AbstractSuggest|array|Collapse|Query|string|Suggest $query
      */
@@ -213,14 +261,27 @@ public function hasIndices(): bool
     }
 
     /**
-     * @param Index|string $index
+     * @param Index $index
      */
     public function hasIndex($index): bool
     {
         if ($index instanceof Index) {
             $index = $index->getName();
+        } else {
+            \trigger_deprecation(
+                'ruflin/elastica',
+                '7.2.0',
+                'Passing a string as 1st argument to "%s()" is deprecated, pass an Index instance or use "hasIndexByName" instead. It will throw a %s in 8.0.',
+                __METHOD__,
+                \TypeError::class
+            );
         }
 
+        return $this->hasIndexByName($index);
+    }
+
+    public function hasIndexByName(string $index): bool
+    {
         return \in_array($index, $this->_indices, true);
     }
 

diff --git a/tests/IndexTest.php b/tests/IndexTest.php
@@ -765,7 +765,7 @@ public function testCreateSearch(): void
         $this->assertEquals($expected, $search->getQuery()->toArray());
         $this->assertEquals(['test'], $search->getIndices());
         $this->assertTrue($search->hasIndices());
-        $this->assertTrue($search->hasIndex('test'));
+        $this->assertTrue($search->hasIndexByName('test'));
         $this->assertTrue($search->hasIndex($index));
     }
 

diff --git a/tests/ScrollTest.php b/tests/ScrollTest.php
@@ -108,7 +108,7 @@ public function testEmptyScroll(): void
     public function testScrollWithIgnoreUnavailable(): void
     {
         $search = $this->_prepareSearch();
-        $search->addIndex('unavailable_index');
+        $search->addIndexByName('unavailable_index');
         $search->setOption($search::OPTION_SEARCH_IGNORE_UNAVAILABLE, 'true');
         $scroll = new Scroll($search);
         $count = 1;

diff --git a/tests/SearchTest.php b/tests/SearchTest.php
@@ -18,12 +18,15 @@
 use Elastica\Search;
 use Elastica\Suggest;
 use Elastica\Test\Base as BaseTest;
+use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
 
 /**
  * @internal
  */
 class SearchTest extends BaseTest
 {
+    use ExpectDeprecationTrait;
+
     /**
      * @group unit
      */
@@ -58,13 +61,109 @@ public function testAddIndex(): void
 
         $this->assertContains($index1->getName(), $indices);
         $this->assertContains($index2->getName(), $indices);
+    }
+
+    /**
+     * @group functional
+     */
+    public function testAddIndexByName(): void
+    {
+        $client = $this->_getClient();
+        $search = new Search($client);
+
+        $search->addIndexByName('index1');
+        $indices = $search->getIndices();
+
+        $this->assertCount(1, $indices);
+        $this->assertContains('index1', $indices);
+    }
+
+    /**
+     * @group functional
+     * @group legacy
+     */
+    public function testAddIndexTriggersDeprecationWithString(): void
+    {
+        $client = $this->_getClient();
+        $search = new Search($client);
+
+        $index1 = $this->_createIndex();
+
+        $search->addIndex($index1);
+        $indices = $search->getIndices();
+
+        $this->assertCount(1, $indices);
 
-        // Add string
-        $search->addIndex('test3');
+        $this->assertContains($index1->getName(), $indices);
+
+        $this->expectDeprecation('Since ruflin/elastica 7.2.0: Passing a string as 1st argument to "Elastica\Search::addIndex()" is deprecated, pass an Index instance or use "addIndexByName" instead. It will throw a TypeError in 8.0.');
+
+        $search->addIndex('test2');
         $indices = $search->getIndices();
 
-        $this->assertCount(3, $indices);
-        $this->assertContains('test3', $indices);
+        $this->assertCount(2, $indices);
+        $this->assertContains('test2', $indices);
+    }
+
+    /**
+     * @group functional
+     */
+    public function testHasIndex(): void
+    {
+        $client = $this->_getClient();
+        $search = new Search($client);
+
+        $index1 = $this->_createIndex();
+        $index2 = $this->_createIndex();
+
+        $this->assertFalse($search->hasIndex($index1));
+        $this->assertFalse($search->hasIndex($index2));
+
+        $search->addIndex($index1);
+        $search->addIndex($index2);
+
+        $this->assertTrue($search->hasIndex($index1));
+        $this->assertTrue($search->hasIndex($index2));
+    }
+
+    /**
+     * @group functional
+     */
+    public function testHasIndexByName(): void
+    {
+        $client = $this->_getClient();
+        $search = new Search($client);
+
+        $indexName1 = 'index1';
+        $indexName2 = 'index2';
+
+        $this->assertFalse($search->hasIndexByName($indexName1));
+        $this->assertFalse($search->hasIndexByName($indexName2));
+
+        $search->addIndexByName($indexName1);
+        $search->addIndexByName($indexName2);
+
+        $this->assertTrue($search->hasIndexByName($indexName1));
+        $this->assertTrue($search->hasIndexByName($indexName2));
+    }
+
+    /**
+     * @group functional
+     * @group legacy
+     */
+    public function testHasIndexTriggersDeprecationWithString(): void
+    {
+        $client = $this->_getClient();
+        $search = new Search($client);
+
+        $indexName = 'index1';
+
+        $this->expectDeprecation('Since ruflin/elastica 7.2.0: Passing a string as 1st argument to "Elastica\Search::hasIndex()" is deprecated, pass an Index instance or use "hasIndexByName" instead. It will throw a TypeError in 8.0.');
+
+        $this->assertFalse($search->hasIndex($indexName));
+
+        $search->addIndexByName($indexName);
+        $this->assertTrue($search->hasIndex($indexName));
     }
 
     /**
@@ -75,9 +174,53 @@ public function testAddIndices(): void
         $client = $this->_getClient();
         $search = new Search($client);
 
+        $indices = [
+            $client->getIndex('elastica_test1'),
+            $client->getIndex('elastica_test2'),
+        ];
+
+        $search->addIndices($indices);
+        $this->assertCount(2, $search->getIndices());
+    }
+
+    /**
+     * @group unit
+     */
+    public function testAddIndicesWithInvalidParametersThrowsException(): void
+    {
+        $client = $this->_getClient();
+        $search = new Search($client);
+
+        $this->expectException(InvalidException::class);
+        $search->addIndices([new \stdClass()]);
+    }
+
+    /**
+     * @group unit
+     */
+    public function testAddIndicesByName(): void
+    {
+        $client = $this->_getClient();
+        $search = new Search($client);
+        $search->addIndicesByName(['elastica_test1', 'elastica_test2']);
+
+        $this->assertCount(2, $search->getIndices());
+    }
+
+    /**
+     * @group unit
+     * @group legacy
+     */
+    public function testAddIndicesTriggersDeprecationWithIndexAsString(): void
+    {
+        $client = $this->_getClient();
+        $search = new Search($client);
+
         $indices = [];
         $indices[] = $client->getIndex('elastica_test1');
-        $indices[] = $client->getIndex('elastica_test2');
+        $indices[] = 'elastica_test2';
+
+        $this->expectDeprecation('Since ruflin/elastica 7.2.0: Passing a array of strings as 1st argument to "Elastica\Search::addIndices()" is deprecated, pass an array of Indexes or use "addIndicesByName" instead. It will throw a TypeError in 8.0.');
 
         $search->addIndices($indices);
 
@@ -87,6 +230,19 @@ public function testAddIndices(): void
     /**
      * @group unit
      */
+    public function testAddIndicesByNameWithInvalidParametersThrowsException(): void
+    {
+        $client = $this->_getClient();
+        $search = new Search($client);
+
+        $this->expectException(InvalidException::class);
+        $search->addIndicesByName([new \stdClass()]);
+    }
+
+    /**
+     * @group unit
+     * @group legacy
+     */
     public function testAddIndexInvalid(): void
     {
         $this->expectException(InvalidException::class);
@@ -97,6 +253,22 @@ public function testAddIndexInvalid(): void
         $search->addIndex(new \stdClass());
     }
 
+    /**
+     * @group unit
+     * @group legacy
+     */
+    public function testAddNumericIndex(): void
+    {
+        $client = $this->_getClient();
+        $search = new Search($client);
+
+        $this->expectDeprecation('Since ruflin/elastica 7.2.0: Passing a string as 1st argument to "Elastica\Search::addIndex()" is deprecated, pass an Index instance or use "addIndexByName" instead. It will throw a TypeError in 8.0.');
+
+        $search->addIndex(1);
+
+        $this->assertContains('1', $search->getIndices(), 'Make sure it has been added and converted to string');
+    }
+
     /**
      * @group functional
      */