From bd5a483c854d3e8f5bc7cf39427e0815949e5ebf Mon Sep 17 00:00:00 2001 From: Simon Erkelens Date: Fri, 20 Oct 2023 12:16:02 +1300 Subject: [PATCH] Improve delete create index (#63) * Add ability to delete an index on configure or indexing * ShouldClear always needs the request --- src/Indexes/ElasticIndex.php | 39 ++++++++++++++ src/Tasks/ElasticConfigureTask.php | 51 +++++++++---------- src/Tasks/ElasticIndexTask.php | 14 ++++- tests/unit/Indexes/ElasticIndexTest.php | 15 ++++++ tests/unit/Tasks/ElasticConfigureTaskTest.php | 10 ++++ tests/unit/Tasks/ElasticIndexTaskTest.php | 5 ++ 6 files changed, 104 insertions(+), 30 deletions(-) diff --git a/src/Indexes/ElasticIndex.php b/src/Indexes/ElasticIndex.php index 142adb3..64d3d18 100644 --- a/src/Indexes/ElasticIndex.php +++ b/src/Indexes/ElasticIndex.php @@ -18,8 +18,11 @@ use Firesphere\ElasticSearch\Services\ElasticCoreService; use Firesphere\ElasticSearch\Traits\IndexTraits\BaseIndexTrait; use Firesphere\SearchBackend\Indexes\CoreIndex; +use Firesphere\SearchBackend\Traits\LoggerTrait; use Firesphere\SearchBackend\Traits\QueryTraits\QueryFilterTrait; use LogicException; +use Psr\Container\NotFoundExceptionInterface; +use SilverStripe\Control\HTTPRequest; use SilverStripe\Core\Config\Configurable; use SilverStripe\Core\Extensible; use SilverStripe\Core\Injector\Injectable; @@ -41,6 +44,7 @@ abstract class ElasticIndex extends CoreIndex use Injectable; use QueryFilterTrait; use BaseIndexTrait; + use LoggerTrait; /** * @var array @@ -86,6 +90,41 @@ public function init() $this->initFromConfig($config); } + + /** + * @param HTTPRequest|null $request + * @return bool + * @throws ClientResponseException + * @throws MissingParameterException + * @throws NotFoundExceptionInterface + * @throws ServerResponseException + */ + public function deleteIndex(HTTPRequest $request): bool + { + $deleteResult = false; + if ($this->shouldClear($request) && $this->indexExists()) { + $this->getLogger()->info(sprintf('Clearing index %s', $this->getIndexName())); + $deleteResult = $this->client + ->indices() + ->delete(['index' => $this->getIndexName()]) + ->asBool(); + } + + return $deleteResult; + } + + /** + * @param HTTPRequest $request + * @return bool + */ + private function shouldClear(HTTPRequest $request): bool + { + $var = $request->getVar('clear'); + + return !empty($var); + } + + /** * @return bool * @throws ClientResponseException diff --git a/src/Tasks/ElasticConfigureTask.php b/src/Tasks/ElasticConfigureTask.php index 5eba3e3..3a0dd63 100644 --- a/src/Tasks/ElasticConfigureTask.php +++ b/src/Tasks/ElasticConfigureTask.php @@ -18,6 +18,7 @@ use Firesphere\ElasticSearch\Indexes\ElasticIndex; use Firesphere\ElasticSearch\Services\ElasticCoreService; use Firesphere\SearchBackend\Helpers\FieldResolver; +use Firesphere\SearchBackend\Indexes\CoreIndex; use Firesphere\SearchBackend\Traits\LoggerTrait; use Psr\Container\NotFoundExceptionInterface; use SilverStripe\Control\HTTPRequest; @@ -36,14 +37,23 @@ class ElasticConfigureTask extends BuildTask { use LoggerTrait; - /** - * @var bool[] - */ - public $result; /** * @var string URLSegment */ private static $segment = 'ElasticConfigureTask'; + /** + * DBHTML and DBText etc. should never be made sortable + * It doesn't make sense for large text objects + * @var string[] + */ + private static $unSsortables = [ + 'HTML', + 'Text' + ]; + /** + * @var bool[] + */ + public $result; /** * @var string Title */ @@ -57,16 +67,6 @@ class ElasticConfigureTask extends BuildTask */ protected $service; - /** - * DBHTML and DBText etc. should never be made sortable - * It doesn't make sense for large text objects - * @var string[] - */ - private static $unSsortables = [ - 'HTML', - 'Text' - ]; - /** * @throws NotFoundExceptionInterface */ @@ -95,13 +95,8 @@ public function run($request) try { /** @var ElasticIndex $instance */ $instance = Injector::inst()->get($index, false); - - if ($request->getVar('clear') && $instance->indexExists()) { - $this->getLogger()->info(sprintf('Clearing index %s', $instance->getIndexName())); - $deleteResult = $this->service->getClient()->indices()->delete(['index' => $instance->getIndexName()]); - $result[] = $deleteResult->asBool(); - } - + // If delete in advance, do so + $instance->deleteIndex($request); $configResult = $this->configureIndex($instance); $result[] = $configResult->asBool(); } catch (Exception $error) { @@ -123,26 +118,26 @@ public function run($request) } /** - * Update/create a store - * @param ElasticIndex $instance + * Update/create a single index. + * @param ElasticIndex $index * @return Elasticsearch * @throws ClientResponseException * @throws MissingParameterException - * @throws ServerResponseException * @throws NotFoundExceptionInterface + * @throws ServerResponseException */ - protected function configureIndex($instance): Elasticsearch + public function configureIndex(CoreIndex $index): Elasticsearch { - $indexName = $instance->getIndexName(); + $indexName = $index->getIndexName(); - $instanceConfig = $this->createConfigForIndex($instance); + $instanceConfig = $this->createConfigForIndex($index); $mappings = $this->convertForJSON($instanceConfig); $body = ['index' => $indexName]; $client = $this->service->getClient(); - $method = $this->getMethod($instance); + $method = $this->getMethod($index); $msg = "%s index %s"; $msgType = 'Updating'; if ($method === 'create') { diff --git a/src/Tasks/ElasticIndexTask.php b/src/Tasks/ElasticIndexTask.php index c612f99..4ca51be 100644 --- a/src/Tasks/ElasticIndexTask.php +++ b/src/Tasks/ElasticIndexTask.php @@ -9,7 +9,10 @@ namespace Firesphere\ElasticSearch\Tasks; +use Elastic\Elasticsearch\Exception\ClientResponseException; use Elastic\Elasticsearch\Exception\HttpClientException; +use Elastic\Elasticsearch\Exception\MissingParameterException; +use Elastic\Elasticsearch\Exception\ServerResponseException; use Exception; use Firesphere\ElasticSearch\Indexes\ElasticIndex; use Firesphere\ElasticSearch\Services\ElasticCoreService; @@ -103,10 +106,13 @@ public function __construct() /** * @param HTTPRequest $request - * @return int|void + * @return bool|int + * @throws HttpClientException * @throws HttpException * @throws NotFoundExceptionInterface - * @throws HttpClientException + * @throws ClientResponseException + * @throws MissingParameterException + * @throws ServerResponseException */ public function run($request) { @@ -124,6 +130,10 @@ public function run($request) if (!count($classes)) { continue; } + // If clearing, also configure + if ($index->deleteIndex($request)) { + (new ElasticConfigureTask())->configureIndex($index); + } // Get the groups $groups = $this->indexClassForIndex($classes, $isGroup, $group); diff --git a/tests/unit/Indexes/ElasticIndexTest.php b/tests/unit/Indexes/ElasticIndexTest.php index 06af0ef..89fd567 100644 --- a/tests/unit/Indexes/ElasticIndexTest.php +++ b/tests/unit/Indexes/ElasticIndexTest.php @@ -5,6 +5,8 @@ use App\src\SearchIndex; use Elastic\Elasticsearch\Client; use Firesphere\ElasticSearch\Indexes\ElasticIndex; +use Firesphere\ElasticSearch\Tasks\ElasticIndexTask; +use SilverStripe\Control\HTTPRequest; use SilverStripe\Core\Config\Config; use SilverStripe\Dev\SapphireTest; @@ -119,4 +121,17 @@ public function testIndexExists() { $this->assertNotNull($this->index->indexExists()); } + + public function testNoClearIndex() + { + $request = new HTTPRequest('GET', 'dev/tasks/ElasticIndexTask'); + $isCleared = $this->index->deleteIndex($request); + $this->assertFalse($isCleared); + $request = new HTTPRequest('GET', 'dev/tasks/ElasticIndexTask', ['clear' => true]); + $isCleared = $this->index->deleteIndex($request); + $this->assertTrue($isCleared); + + // Ensure everything is back in place. + (new ElasticIndexTask())->run($request); + } } diff --git a/tests/unit/Tasks/ElasticConfigureTaskTest.php b/tests/unit/Tasks/ElasticConfigureTaskTest.php index 9937358..652182d 100644 --- a/tests/unit/Tasks/ElasticConfigureTaskTest.php +++ b/tests/unit/Tasks/ElasticConfigureTaskTest.php @@ -2,6 +2,7 @@ namespace Firesphere\ElasticSearch\Tests\unit\Tasks; +use App\src\SearchIndex; use Firesphere\ElasticSearch\Services\ElasticCoreService; use Firesphere\ElasticSearch\Tasks\ElasticConfigureTask; use SilverStripe\Control\HTTPRequest; @@ -27,4 +28,13 @@ public function testRun() $this->assertNotContains(false, $task->result); } + + public function testConfigureIndex() + { + $index = new SearchIndex(); + $task = new ElasticConfigureTask(); + $result = $task->configureIndex($index); + + $this->assertTrue($result->asBool()); + } } diff --git a/tests/unit/Tasks/ElasticIndexTaskTest.php b/tests/unit/Tasks/ElasticIndexTaskTest.php index c4a82e2..6653864 100644 --- a/tests/unit/Tasks/ElasticIndexTaskTest.php +++ b/tests/unit/Tasks/ElasticIndexTaskTest.php @@ -35,6 +35,11 @@ public function testRun() $request = new HTTPRequest('GET', 'dev/tasks/ElasticIndexTask'); $result = $task->run($request); + $this->assertGreaterThan(0, $result); + $this->assertinstanceOf(ElasticIndex::class, $task->getIndex()); + $request = new HTTPRequest('GET', 'dev/tasks/ElasticIndexTask', ['clear' => true]); + $result = $task->run($request); + $this->assertGreaterThan(0, $result); $this->assertinstanceOf(ElasticIndex::class, $task->getIndex()); }