Skip to content
Snippets Groups Projects
Commit 3a6cfcc5 authored by Andrew Chappell's avatar Andrew Chappell
Browse files

Add further test case and refactor

parent a20ab8f6
No related branches found
No related tags found
No related merge requests found
Pipeline #197766 passed with warnings
Stage: build
Stage: validate
Stage: test
Hide whitespace changes
Inline Side-by-side
src/SearchAPI/BackendClient.php
+ 65
− 80
View file @ 3a6cfcc5
......@@ -217,57 +217,20 @@ class BackendClient implements BackendClientInterface {
}
}
protected function array_diff_assoc_recursive($array1, $array2)
{
$difference = array();
foreach ($array1 as $key => $value) {
if (is_array($value)) {
if (!isset($array2[$key]) || !is_array($array2[$key])) {
$difference[$key] = $value;
} else {
$new_diff = $this->array_diff_assoc_recursive($value, $array2[$key]);
if (!empty($new_diff)) {
$difference[$key] = $new_diff;
}
}
} elseif (!array_key_exists($key, $array2) || $array2[$key] !== $value) {
$difference[$key] = $value;
}
}
return $difference;
}
/**
* {@inheritdoc}
*/
public function updateIndex(IndexInterface $index): void {
if ($this->indexExists($index)) {
$mapping = $this->client->indices()->getMapping([
'index' => $this->getIndexId($index),
]);
$params = $this->fieldParamsBuilder->mapFieldParams($this->getIndexId($index), $index);
// Before comparing, convert the new properties to
// a json format returned as an associative array,
// as the putMappings method expects an empty object,
// but the getMapping returns an empty array.
// Ensure float values are preserved as json_encode
// doesn't preserve them by default.
$result = $this->array_diff_assoc_recursive(
$mapping[$this->getIndexId($index)]['mappings']['properties'],
json_decode(
json_encode($params['body']['properties'], JSON_PRESERVE_ZERO_FRACTION),
TRUE
)
);
if (!empty($result)) {
$result = $this->indexNeedsClearing($index);
if ($result) {
$index->clear();
}
else {
$index->reindex();
}
$this->updateSettings($index);
$this->updateFieldMapping($index);
if (!$result) {
$index->reindex();
}
}
else {
$this->addIndex($index);
......@@ -294,44 +257,6 @@ class BackendClient implements BackendClientInterface {
}
}
/**
* Checks if the recently updated index had any fields changed.
*
* @param \Drupal\search_api\IndexInterface $index
* The index that was just updated.
*
* @return bool
* TRUE if any of the fields were updated, FALSE otherwise.
*/
protected function indexFieldsUpdated(IndexInterface $index): bool {
if (!isset($index->original)) {
return TRUE;
}
/** @var \Drupal\search_api\IndexInterface $original */
$original = $index->original;
$old_fields = $original->getFields();
$new_fields = $index->getFields();
if (!$old_fields && !$new_fields) {
return FALSE;
}
foreach ($new_fields as $field) {
if (!isset($old_fields[$field->getFieldIdentifier()])) {
continue;
}
$old_field = $old_fields[$field->getFieldIdentifier()];
if ($field->getSettings() != $old_field->getSettings()) {
return TRUE;
}
if ($field->getConfiguration() != $old_field->getConfiguration()) {
return TRUE;
}
}
return FALSE;
}
/**
* {@inheritdoc}
*/
......@@ -428,4 +353,64 @@ class BackendClient implements BackendClientInterface {
}
}
/**
* Determine whether the index needs clearing.
*
* OpenSearch does not allow changing existing field
* mappings with data but does allow adding new fields.
* OpenSearch has dynamic mappings which means it will automatically
* add types to fields based on the indexed data if no mapping
* is explicitly provided. It will also override the mapping
* if it is wrong, e.g. if you provide a string type but
* the indexed data for the field is a float it will override the
* mapping. In order to make sure the index is not cleared for minor
* changes It's important to make sure the types for fields are correct
* and that any custom fields are explicitly mapped.
*/
private function indexNeedsClearing(IndexInterface $index): bool {
$openSearchMapping = $this->client->indices()->getMapping([
'index' => $this->getIndexId($index),
]);
$drupalMapping = $this->fieldParamsBuilder->mapFieldParams($this->getIndexId($index), $index);
// Recursively check for differences in the mappings between the
// $openSearchMapping and $drupalMapping arrays. Before comparing,
// convert $drupalMapping to a json format returned as an
// associative array, so it is in the same format as the openSearchMapping.
// For example the putMappings method on the OS client expects an empty
// object but getMapping returns an empty array. Ensure float values are
// preserved as json_encode doesn't preserve them by default.
$result = $this->arrayDiffAssocRecursive(
$openSearchMapping[$this->getIndexId($index)]['mappings']['properties'],
json_decode(
json_encode($drupalMapping['body']['properties'], JSON_PRESERVE_ZERO_FRACTION),
TRUE
)
);
return !empty($result);
}
/**
* Recursively diff an associative array to find differences.
*/
private function arrayDiffAssocRecursive($array1, $array2): array {
$difference = [];
foreach ($array1 as $key => $value) {
if (is_array($value)) {
if (!isset($array2[$key]) || !is_array($array2[$key])) {
$difference[$key] = $value;
}
else {
$newDiff = $this->arrayDiffAssocRecursive($value, $array2[$key]);
if (!empty($newDiff)) {
$difference[$key] = $newDiff;
}
}
}
elseif (!array_key_exists($key, $array2) || $array2[$key] !== $value) {
$difference[$key] = $value;
}
}
return $difference;
}
}
tests/src/Kernel/OpenSearchBackendTest.php
+ 78
− 8
View file @ 3a6cfcc5
......@@ -6,7 +6,6 @@ use Drupal\field\Entity\FieldConfig;
use Drupal\field\Entity\FieldStorageConfig;
use Drupal\search_api\Entity\Index;
use Drupal\search_api\Entity\Server;
use Drupal\search_api\Item\Field;
use Drupal\search_api\Query\QueryInterface;
use Drupal\search_api_opensearch_test\EventSubscriber\OpenSearchEventSubscribers;
use Drupal\Tests\search_api\Functional\ExampleContentTrait;
......@@ -41,6 +40,11 @@ class OpenSearchBackendTest extends BackendTestBase {
*/
protected $indexId = 'test_opensearch_index';
/**
* The index prefix.
*/
protected string $prefix = 'test_';
/**
* {@inheritdoc}
*/
......@@ -52,6 +56,18 @@ class OpenSearchBackendTest extends BackendTestBase {
]);
}
/**
* {@inheritDoc}
*/
public function tearDown(): void {
$server = Server::load($this->serverId);
/** @var \Drupal\search_api_opensearch\Plugin\search_api\backend\OpenSearchBackend $backend */
$backend = $server->getBackend();
$osClient = $backend->getClient();
$osClient->indices()->delete(['index' => $this->prefix . $this->indexId]);
parent::tearDown();
}
/**
* Tests various indexing scenarios for the search backend.
*
......@@ -71,6 +87,12 @@ class OpenSearchBackendTest extends BackendTestBase {
$this->searchSuccess();
}
/**
* Tests making changes to fields on the index.
*
* Adding a new field should not clear the index
* but changing an existing field mapping should.
*/
public function testUpdatingIndex(): void {
$server = Server::load($this->serverId);
/** @var \Drupal\search_api_opensearch\Plugin\search_api\backend\OpenSearchBackend $backend */
......@@ -90,8 +112,7 @@ class OpenSearchBackendTest extends BackendTestBase {
$result = $this->queryAllResults($osClient);
$this->assertEquals(5, $result['hits']['total']['value']);
// Now check that tracker is reset but items remain on
// server when a new field is added.
// Add a new field to the index.
FieldStorageConfig::create([
'field_name' => 'new_field',
'entity_type' => 'entity_test_mulrev_changed',
......@@ -112,29 +133,78 @@ class OpenSearchBackendTest extends BackendTestBase {
]);
$index->addField($field);
$index->save();
// Now check that the tracker is reset but items remain on
// server.
$this->assertEquals(0, $tracker->getIndexedItemsCount());
$result = $this->queryAllResults($osClient);
$this->assertEquals(5, $result['hits']['total']['value']);
// Changing an existing fields type should clear the index.
// Re-index the items for next test.
$this->indexItems($this->indexId);
sleep(6);
$this->assertEquals(5, $tracker->getIndexedItemsCount());
$result = $this->queryAllResults($osClient);
$this->assertEquals(5, $result['hits']['total']['value']);
// Changing the fields type should clear the index completely
// as OS doesn't allow changes to existing fields with data.
$field = $index->getField('new_field');
$field->setType('fulltext');
$field->setType('text');
$index->addField($field);
$index->save();
$this->assertEquals(0, $tracker->getIndexedItemsCount());
$result = $this->queryAllResults($osClient);
$this->assertEquals(0, $result['hits']['total']['value']);
// Re-index the items for next test.
$this->indexItems($this->indexId);
sleep(6);
$this->assertEquals(5, $tracker->getIndexedItemsCount());
$result = $this->queryAllResults($osClient);
$this->assertEquals(5, $result['hits']['total']['value']);
// Add another new field and change the first field
// back to its original type.
FieldStorageConfig::create([
'field_name' => 'new_field_2',
'entity_type' => 'entity_test_mulrev_changed',
'type' => 'string',
])->save();
FieldConfig::create([
'entity_type' => 'entity_test_mulrev_changed',
'field_name' => 'new_field_2',
'bundle' => 'item',
])->save();
$fieldsHelper = \Drupal::getContainer()->get('search_api.fields_helper');
$field = $fieldsHelper->createField($index, 'new_field_2', [
'label' => 'New field 2',
'type' => 'string',
'property_path' => 'new_field_2',
'datasource_id' => 'entity:entity_test_mulrev_changed',
]);
$index->addField($field);
$existingField = $index->getField('new_field');
$existingField->setType('string');
$index->addField($existingField);
$index->save();
// The index should be completely cleared because we have both
// a new field and a changed field.
$this->assertEquals(0, $tracker->getIndexedItemsCount());
$result = $this->queryAllResults($osClient);
$this->assertEquals(0, $result['hits']['total']['value']);
}
protected function queryAllResults(Client $client) {
/**
* Returns the OS response for all documents.
*/
protected function queryAllResults(Client $client): array {
return $client->search([
'index' => 'test_' . $this->indexId,
'index' => $this->prefix . $this->indexId,
'body' => [
'query' => [
'match_all' => new \StdClass,
'match_all' => new \StdClass(),
],
],
'track_total_hits' => TRUE,
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment