Skip to content

Commit

Permalink
Merge pull request #838 from City-of-Helsinki/UHF-10719-es-errors
Browse files Browse the repository at this point in the history
UHF-10719: Upgrade drupal/elasticsearch_connector major version
  • Loading branch information
hyrsky authored Nov 8, 2024
2 parents 07fd66b + f7e9de3 commit 1706882
Show file tree
Hide file tree
Showing 13 changed files with 96 additions and 134 deletions.
6 changes: 5 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"drupal/diff": "^1.0",
"drupal/easy_breadcrumb": "^2.0",
"drupal/editoria11y": "^2.0",
"drupal/elasticsearch_connector": "^7.0@alpha",
"drupal/elasticsearch_connector": "^8.0@alpha",
"drupal/entity_browser": "^2.5",
"drupal/entity_usage": "^2.0@beta",
"drupal/eu_cookie_compliance": "1.24",
Expand Down Expand Up @@ -65,9 +65,13 @@
"drupal/views_bulk_edit": "^2.7",
"drupal/views_bulk_operations": "^4.1",
"drupal/stage_file_proxy": "^2",
"ruflin/elastica": "^8.0",
"league/uri": "^6.0",
"php": "^8.1"
},
"require-dev": {
"dg/bypass-finals": "^1.0"
},
"conflict": {
"drupal/core": "<10.3",
"drupal/core-composer-scaffold": "<10.3",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ function helfi_paragraphs_news_list_paragraph_view(
$query = $storage
->getQuery();
$query
->condition('_language', $entity->language()->getId())
->condition('search_api_language', $entity->language()->getId())
->range(0, $entity->getLimit());

$termFilters = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@ services:
class: Drupal\helfi_paragraphs_news_list\ClientBuilder

helfi_paragraphs_news_list.elastic_client:
class: Elasticsearch\Client
class: Elastic\Elasticsearch\Client
factory: ['@helfi_paragraphs_news_list.elastic_client_factory', 'create']
15 changes: 5 additions & 10 deletions modules/helfi_paragraphs_news_list/src/ClientBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
use Drupal\helfi_api_base\Environment\EnvironmentResolverInterface;
use Drupal\helfi_api_base\Environment\Project;
use Drupal\helfi_api_base\Environment\ServiceEnum;
use Elasticsearch\Client;
use Elasticsearch\ClientBuilder as ElasticClientBuilder;
use Elastic\Elasticsearch\Client;
use Elastic\Elasticsearch\ClientBuilder as ElasticClientBuilder;

/**
* The client builder factory.
Expand All @@ -25,13 +25,12 @@ final class ClientBuilder {
public function __construct(
private readonly EnvironmentResolverInterface $environmentResolver,
) {

}

/**
* Creates a new client instance.
*
* @return \Elasticsearch\Client
* @return \Elastic\Elasticsearch\Client
* The client.
*/
public function create() : Client {
Expand All @@ -51,13 +50,9 @@ public function create() : Client {
return ElasticClientBuilder::create()
->setSSLVerification($service->protocol === 'https')
->setHosts([
[
'host' => $service->domain,
'port' => $service->port,
'scheme' => $service->protocol,
],
$service->getAddress(),
])
->setConnectionParams([
->setHttpClientOptions([
'client' => [
'timeout' => 1,
'connect_timeout' => 1,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
<?php

declare(strict_types=1);

namespace Drupal\helfi_paragraphs_news_list;

use Drupal\Core\Utility\Error;
use Drupal\external_entities\ExternalEntityInterface;
use Drupal\external_entities\StorageClient\ExternalEntityStorageClientBase;
use Elasticsearch\Client;
use Elasticsearch\Common\Exceptions\ElasticsearchException;
use Elastic\Elasticsearch\Client;
use Elastic\Elasticsearch\Exception\ElasticsearchException;
use Elastic\Transport\Exception\TransportException;
use Psr\Log\LoggerInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;

Expand All @@ -25,7 +28,7 @@ abstract class ElasticExternalEntityBase extends ExternalEntityStorageClientBase
/**
* The elastic client.
*
* @var \Elasticsearch\Client
* @var \Elastic\Elasticsearch\Client
*/
protected Client $client;

Expand Down Expand Up @@ -78,9 +81,9 @@ protected function request(
array $parameters,
) : array {
try {
return $this->client->search($parameters);
return $this->client->search($parameters)->asArray();
}
catch (ElasticsearchException $e) {
catch (ElasticsearchException | TransportException $e) {
Error::logException($this->logger, $e);
}
return [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
namespace Drupal\Tests\helfi_paragraphs_news_list\Kernel\ExternalEntityStorage;

use Drupal\helfi_paragraphs_news_list\Entity\ExternalEntity\News;
use Elasticsearch\Client;
use Elastic\Elasticsearch\Client;
use Prophecy\Argument;

/**
Expand All @@ -29,32 +29,35 @@ public function testLoadMultiple() : void {
$client = $this->prophesize(Client::class);
$client->search(Argument::any())
->shouldBeCalled()
->willReturn([], [
'hits' => [
->willReturn(
$this->createElasticsearchResponse([]),
$this->createElasticsearchResponse([
'hits' => [
// Working item.
[
'_source' => [
'uuid_langcode' => ['123'],
'uuid' => ['uuid-123'],
'title' => ['test title'],
'field_news_groups' => ['Test groups'],
'field_news_item_tags' => ['Test tag'],
'field_news_neighbourhoods' => ['Test neighbourhood'],
'url' => ['https://localhost'],
'published_at' => [1234567],
'short_title' => ['test shorttitle'],
'hits' => [
// Working item.
[
'_source' => [
'uuid_langcode' => ['123'],
'uuid' => ['uuid-123'],
'title' => ['test title'],
'field_news_groups' => ['Test groups'],
'field_news_item_tags' => ['Test tag'],
'field_news_neighbourhoods' => ['Test neighbourhood'],
'url' => ['https://localhost'],
'published_at' => [1234567],
'short_title' => ['test shorttitle'],
],
],
],
// Missing uuid_langcode field.
[
'_source' => [
'uuid' => 'uuid-321',
// Missing uuid_langcode field.
[
'_source' => [
'uuid' => 'uuid-321',
],
],
],
],
],
]);
]),
);
$client->search(Argument::any())
->shouldBeCalled();
$sut = $this->getSut($client->reveal());
Expand Down Expand Up @@ -87,7 +90,7 @@ public function testQuery(): void {
],
])
->shouldBeCalled()
->willReturn([]);
->willReturn($this->createElasticsearchResponse([]));
// Test sort.
$client->search([
'index' => 'news',
Expand All @@ -99,7 +102,7 @@ public function testQuery(): void {
],
])
->shouldBeCalled()
->willReturn([]);
->willReturn($this->createElasticsearchResponse([]));
// Test filters.
$client->search([
'index' => 'news',
Expand Down Expand Up @@ -129,7 +132,7 @@ public function testQuery(): void {
],
])
->shouldBeCalled()
->willReturn([]);
->willReturn($this->createElasticsearchResponse([]));
$this->getSut($client->reveal())->getQuery()->accessCheck(FALSE)->execute();
$this->getSut($client->reveal())
->getQuery()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
use Drupal\Core\Entity\EntityTypeManagerInterface;
use Drupal\Tests\helfi_paragraphs_news_list\Kernel\KernelTestBase;
use Drupal\external_entities\ExternalEntityStorage;
use Elasticsearch\Client;
use Elasticsearch\Common\Exceptions\BadRequest400Exception;
use Elastic\Elasticsearch\Client;
use Elastic\Elasticsearch\Exception\ClientResponseException;
use Prophecy\Argument;

/**
Expand All @@ -27,7 +27,7 @@ abstract protected function getStorageName() : string;
/**
* Gets the SUT.
*
* @param \Elasticsearch\Client $client
* @param \Elastic\Elasticsearch\Client $client
* The client mock.
*
* @return \Drupal\external_entities\ExternalEntityStorage
Expand All @@ -47,7 +47,7 @@ public function getSut(Client $client) : ExternalEntityStorage {
public function testRequestException() : void {
$client = $this->prophesize(Client::class);
$client->search(Argument::any())
->willThrow(new BadRequest400Exception('Message'));
->willThrow(new ClientResponseException('Message'));
$sut = $this->getSut($client->reveal());
$this->assertEmpty($sut->loadMultiple([123]));
$this->assertEmpty($sut->getQuery()->accessCheck(FALSE)->execute());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
namespace Drupal\Tests\helfi_paragraphs_news_list\Kernel\ExternalEntityStorage;

use Drupal\helfi_paragraphs_news_list\Entity\ExternalEntity\Term;
use Elasticsearch\Client;
use Elastic\Elasticsearch\Client;
use Prophecy\Argument;

/**
Expand All @@ -28,7 +28,7 @@ public function testLoadMultiple() : void {
$client = $this->prophesize(Client::class);
$client->search(Argument::any())
->shouldBeCalled()
->willReturn([], [
->willReturn($this->createElasticsearchResponse([]), $this->createElasticsearchResponse([
'hits' => [
'hits' => [
// Working item.
Expand All @@ -48,7 +48,7 @@ public function testLoadMultiple() : void {
],
],
],
]);
]));
$client->search(Argument::any())
->shouldBeCalled();
$sut = $this->getSut($client->reveal());
Expand Down Expand Up @@ -85,7 +85,7 @@ public function testQuery(): void {
],
])
->shouldBeCalled()
->willReturn([]);
->willReturn($this->createElasticsearchResponse([]));
// Test sort.
$client->search([
'index' => 'news_terms',
Expand All @@ -103,7 +103,7 @@ public function testQuery(): void {
],
])
->shouldBeCalled()
->willReturn([]);
->willReturn($this->createElasticsearchResponse([]));
// Test filters.
$client->search([
'index' => 'news_terms',
Expand Down Expand Up @@ -134,7 +134,7 @@ public function testQuery(): void {
],
])
->shouldBeCalled()
->willReturn([]);
->willReturn($this->createElasticsearchResponse([]));
$this->getSut($client->reveal())
->getQuery()
->accessCheck(FALSE)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@

namespace Drupal\Tests\helfi_paragraphs_news_list\Kernel;

use DG\BypassFinals;
use Drupal\KernelTests\KernelTestBase as CoreKernelTestBase;
use Elastic\Elasticsearch\Response\Elasticsearch;

/**
* Kernel test base for news feed list tests.
*/
class KernelTestBase extends CoreKernelTestBase {
abstract class KernelTestBase extends CoreKernelTestBase {

/**
* {@inheritdoc}
Expand All @@ -33,6 +35,9 @@ class KernelTestBase extends CoreKernelTestBase {
* {@inheritdoc}
*/
protected function setUp(): void {
// https://github.com/elastic/elasticsearch-php/issues/1227.
BypassFinals::enable();

parent::setUp();
$this->installConfig(['system', 'paragraphs', 'external_entities']);
$this->installEntitySchema('user');
Expand All @@ -46,4 +51,16 @@ protected function setUp(): void {
$this->installConfig('paragraphs');
}

/**
* Mocks elasticsearch response.
*
* @param array $response
* Response as an array.
*/
protected function createElasticsearchResponse(array $response): Elasticsearch {
$mock = $this->prophesize(Elasticsearch::class);
$mock->asArray()->willReturn($response);
return $mock->reveal();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@
use Drupal\Tests\helfi_api_base\Traits\EnvironmentResolverTrait;
use Drupal\helfi_api_base\Environment\EnvironmentResolver;
use Drupal\helfi_paragraphs_news_list\ClientBuilder;
use Elasticsearch\Client;
use Elastic\Elasticsearch\Client;

/**
* Tests elastic client builder.
*
* @group helfi_api_base
* @group helfi_paragraphs_news_list
*/
class ClientBuilderTest extends UnitTestCase {

Expand Down
1 change: 0 additions & 1 deletion modules/helfi_react_search/helfi_react_search.services.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,3 @@ services:
- { timeout: '%helfi_react_search.request_timeout%' }

Drupal\helfi_react_search\LinkedEvents: ~
Drupal\helfi_react_search\EventSubscriber\ElasticIndexSubscriber: ~
Loading

0 comments on commit 1706882

Please sign in to comment.