From 00771dd4657cc35e7a353a8e0d493814c54f27a4 Mon Sep 17 00:00:00 2001
From: Santeri Hurnanen <santeri.hurnanen@fame.fi>
Date: Fri, 17 Jan 2025 08:48:27 +0200
Subject: [PATCH 1/4] UHF-11002: Add NewsNeighbourhoods::loadByCoordinates()

---
 ..._entity_type.helfi_news_neighbourhoods.yml |  2 +
 .../helfi_paragraphs_news_list.install        |  8 +++
 .../src/ElasticExternalEntityBase.php         |  5 +-
 .../StorageClient/NewsNeighbourhoods.php      | 55 +++++++++++++++++++
 .../NewsNeighbourhoodsStorageClientTest.php   | 43 +++++++++++++++
 5 files changed, 111 insertions(+), 2 deletions(-)

diff --git a/modules/helfi_paragraphs_news_list/config/install/external_entities.external_entity_type.helfi_news_neighbourhoods.yml b/modules/helfi_paragraphs_news_list/config/install/external_entities.external_entity_type.helfi_news_neighbourhoods.yml
index fd5cedd73..6a5d05086 100644
--- a/modules/helfi_paragraphs_news_list/config/install/external_entities.external_entity_type.helfi_news_neighbourhoods.yml
+++ b/modules/helfi_paragraphs_news_list/config/install/external_entities.external_entity_type.helfi_news_neighbourhoods.yml
@@ -19,6 +19,8 @@ field_mapper_config:
       value: '$._source.name[0]'
     tid:
       value: '$._source.tid[0]'
+    location:
+      value: '$._source.field_location'
 storage_client_id: helfi_news_neighbourhoods
 storage_client_config: {  }
 persistent_cache_max_age: 86400
diff --git a/modules/helfi_paragraphs_news_list/helfi_paragraphs_news_list.install b/modules/helfi_paragraphs_news_list/helfi_paragraphs_news_list.install
index 755e6f684..9a2c49b2e 100644
--- a/modules/helfi_paragraphs_news_list/helfi_paragraphs_news_list.install
+++ b/modules/helfi_paragraphs_news_list/helfi_paragraphs_news_list.install
@@ -135,3 +135,11 @@ function helfi_paragraphs_news_list_update_9009() : void {
   \Drupal::service('helfi_platform_config.config_update_helper')
     ->update('helfi_paragraphs_news_list');
 }
+
+/**
+ * UHF-11002: Update news list external entities.
+ */
+function helfi_paragraphs_news_list_update_9010() : void {
+  \Drupal::service('helfi_platform_config.config_update_helper')
+    ->update('helfi_paragraphs_news_list');
+}
diff --git a/modules/helfi_paragraphs_news_list/src/ElasticExternalEntityBase.php b/modules/helfi_paragraphs_news_list/src/ElasticExternalEntityBase.php
index 31c02892a..5c06ca87e 100644
--- a/modules/helfi_paragraphs_news_list/src/ElasticExternalEntityBase.php
+++ b/modules/helfi_paragraphs_news_list/src/ElasticExternalEntityBase.php
@@ -183,10 +183,11 @@ protected function buildQuery(array $parameters, array $sorts) : array {
 
     $sortQuery = [];
     foreach ($sorts as $sort) {
-      ['field' => $field, 'direction' => $direction] = $sort;
+      $defaults = ['options' => []];
+      ['field' => $field, 'direction' => $direction, 'options' => $options] = $sort + $defaults;
       $fieldName = $this->getFieldMapping($field);
 
-      $sortQuery[$fieldName] = ['order' => strtolower($direction)];
+      $sortQuery[$fieldName] = ['order' => strtolower($direction)] + $options;
     }
 
     return [
diff --git a/modules/helfi_paragraphs_news_list/src/Plugin/ExternalEntities/StorageClient/NewsNeighbourhoods.php b/modules/helfi_paragraphs_news_list/src/Plugin/ExternalEntities/StorageClient/NewsNeighbourhoods.php
index 4b3502364..5ff1e0b86 100644
--- a/modules/helfi_paragraphs_news_list/src/Plugin/ExternalEntities/StorageClient/NewsNeighbourhoods.php
+++ b/modules/helfi_paragraphs_news_list/src/Plugin/ExternalEntities/StorageClient/NewsNeighbourhoods.php
@@ -20,4 +20,59 @@ final class NewsNeighbourhoods extends TermBase {
    */
   protected string $vid = 'news_neighbourhoods';
 
+  /**
+   * {@inheritdoc}
+   */
+  protected function getFieldMapping(string $field) : string {
+    return match($field) {
+      'location' => 'field_location',
+      default => parent::getFieldMapping($field),
+    };
+  }
+
+  /**
+   * Load nearest News neighbourhoods terms.
+   *
+   * Drupal query interface is not quite flexible enough to implement
+   * geo_distance sorting.
+   *
+   * @param float $lat
+   *   Latitude.
+   * @param float $lon
+   *   Longitude.
+   * @param int|null $start
+   *   (optional) The first item to return.
+   * @param int|null $length
+   *   (optional) The number of items to return.
+   *
+   * @return array
+   */
+  public function loadByCoordinates(
+    float $lat,
+    float $lon,
+    ?int $start = NULL,
+    ?int $length = NULL,
+  ): array {
+    return $this->query([], [
+      [
+        'field' => '_geo_distance',
+        'direction' => 'ASC',
+        'options' => [
+          $this->getFieldMapping('location') => [
+            'lat' => $lat,
+            'lon' => $lon,
+          ],
+          'unit' => 'km',
+          // Arc is more accurate, but within
+          // the city it should not matter.
+          'distance_type' => 'plane',
+          // What to do in case a field has several geo points.
+          'mode' => 'min',
+          // Unmapped field cause the search to fail.
+          'ignore_unmapped' => false
+        ],
+      ],
+    ], $start, $length);
+  }
+
 }
diff --git a/modules/helfi_paragraphs_news_list/tests/src/Kernel/ExternalEntityStorage/NewsNeighbourhoodsStorageClientTest.php b/modules/helfi_paragraphs_news_list/tests/src/Kernel/ExternalEntityStorage/NewsNeighbourhoodsStorageClientTest.php
index 328c899a3..d7cf288da 100644
--- a/modules/helfi_paragraphs_news_list/tests/src/Kernel/ExternalEntityStorage/NewsNeighbourhoodsStorageClientTest.php
+++ b/modules/helfi_paragraphs_news_list/tests/src/Kernel/ExternalEntityStorage/NewsNeighbourhoodsStorageClientTest.php
@@ -4,6 +4,9 @@
 
 namespace Drupal\Tests\helfi_paragraphs_news_list\Kernel\ExternalEntityStorage;
 
+use Drupal\helfi_paragraphs_news_list\Plugin\ExternalEntities\StorageClient\NewsNeighbourhoods;
+use Elastic\Elasticsearch\Client;
+
 /**
  * Tests news tags storage client.
  *
@@ -25,4 +28,44 @@ protected function getVid(): string {
     return 'news_neighbourhoods';
   }
 
+  /**
+   * Tests geo_distance sorting.
+   */
+  public function testLocationQuery(): void {
+    $client = $this->prophesize(Client::class);
+    // Test geo distance sort.
+    $client->search([
+      'index' => 'news_terms',
+      'body' => [
+        'sort' => [
+          '_geo_distance' => [
+            'order' => 'asc',
+            'field_location' => [
+              'lat' => 48.8584,
+              'lon' => 2.2945,
+            ],
+            'unit' => 'km',
+            'distance_type' => 'plane',
+            'mode' => 'min',
+            'ignore_unmapped' => FALSE,
+          ],
+        ],
+        'query' => [
+          'bool' => [
+            'must' => [
+              ['term' => ['vid' => $this->getVid()]],
+            ],
+          ],
+        ],
+      ],
+    ])
+      ->shouldBeCalled()
+      ->willReturn($this->createElasticsearchResponse([]));
+
+    $sut = $this->getSut($client->reveal())->getStorageClient();
+    $this->assertInstanceOf(NewsNeighbourhoods::class, $sut);
+
+    $sut->loadByCoordinates(48.8584, 2.2945);
+  }
+
 }

From d5947aa4fb1969c79583492bfe3daf0f69754bfa Mon Sep 17 00:00:00 2001
From: Santeri Hurnanen <santeri.hurnanen@fame.fi>
Date: Fri, 17 Jan 2025 08:48:30 +0200
Subject: [PATCH 2/4] UHF-11002: Use Query::setParameter instead

---
 .../src/ElasticExternalEntityBase.php         | 114 +++++++++++++-----
 .../StorageClient/NewsNeighbourhoods.php      |  45 -------
 .../NewsNeighbourhoodsStorageClientTest.php   |  54 ++++++---
 3 files changed, 123 insertions(+), 90 deletions(-)

diff --git a/modules/helfi_paragraphs_news_list/src/ElasticExternalEntityBase.php b/modules/helfi_paragraphs_news_list/src/ElasticExternalEntityBase.php
index 5c06ca87e..983dd18d8 100644
--- a/modules/helfi_paragraphs_news_list/src/ElasticExternalEntityBase.php
+++ b/modules/helfi_paragraphs_news_list/src/ElasticExternalEntityBase.php
@@ -135,6 +135,78 @@ protected function getFieldMapping(string $field) : string {
     return $field;
   }
 
+  /**
+   * Get callback that builds elasticsearch query fragment for given operator.
+   *
+   * @param ?string $op
+   *   Query operation.
+   *
+   * @return callable
+   *   Handler.
+   */
+  protected function getOperatorCallback(?string $op): callable {
+    return match($op) {
+      'IN' => static function (array $value, string $fieldName) : array {
+        $inGroup = [];
+        foreach ($value as $v) {
+          $inGroup[] = ['term' => [$fieldName => $v]];
+        }
+        return [
+          'query' => [
+            'bool' => [
+              'must' => [
+                ['bool' => ['should' => $inGroup]],
+              ],
+            ],
+          ],
+        ];
+      },
+      'CONTAINS' => static function (string $value, string $fieldName) : array {
+        return [
+          'query' => [
+            'bool' => [
+              'must' => [
+                [
+                  'regexp' => [
+                    $fieldName => [
+                      'value' => $value . '.*',
+                      'case_insensitive' => TRUE,
+                    ],
+                  ],
+                ],
+              ],
+            ],
+          ],
+        ];
+      },
+      'GEO_DISTANCE_SORT' => static function (array $value, string $fieldName) : array {
+        [$coordinates, $options] = $value;
+
+        return [
+          'sort' => [
+            [
+              '_geo_distance' => [
+                $fieldName => $coordinates,
+                ...$options,
+              ],
+            ],
+          ],
+        ];
+      },
+      default => static function (string|int|null $value, string $fieldName) : array {
+        return [
+          'query' => [
+            'bool' => [
+              'must' => [
+                ['term' => [$fieldName => $value]],
+              ],
+            ],
+          ],
+        ];
+      },
+    };
+  }
+
   /**
    * Builds the elastic query for given parameters.
    *
@@ -147,7 +219,7 @@ protected function getFieldMapping(string $field) : string {
    *   The query.
    */
   protected function buildQuery(array $parameters, array $sorts) : array {
-    $query = [];
+    $body = [];
 
     foreach ($parameters as $parameter) {
       ['field' => $field, 'value' => $value, 'operator' => $op] = $parameter;
@@ -156,46 +228,26 @@ protected function buildQuery(array $parameters, array $sorts) : array {
       if (!$value) {
         continue;
       }
-      $callback = match($op) {
-        'IN' => function (array $value, string $fieldName) : array {
-          $inGroup = [];
-          foreach ($value as $v) {
-            $inGroup[] = ['term' => [$fieldName => $v]];
-          }
-          return ['bool' => ['should' => $inGroup]];
-        },
-        'CONTAINS' => function (string $value, string $fieldName) : array {
-          return [
-            'regexp' => [
-              $fieldName => [
-                'value' => $value . '.*',
-                'case_insensitive' => TRUE,
-              ],
-            ],
-          ];
-        },
-        default => function (string|int|null $value, string $fieldName) : array {
-          return ['term' => [$fieldName => $value]];
-        },
-      };
-      $query['bool']['must'][] = $callback($value, $fieldName);
+
+      $callback = $this->getOperatorCallback($op);
+      $body = array_merge_recursive($body, $callback($value, $fieldName));
     }
 
     $sortQuery = [];
     foreach ($sorts as $sort) {
-      $defaults = ['options' => []];
-      ['field' => $field, 'direction' => $direction, 'options' => $options] = $sort + $defaults;
+      ['field' => $field, 'direction' => $direction] = $sort;
       $fieldName = $this->getFieldMapping($field);
 
-      $sortQuery[$fieldName] = ['order' => strtolower($direction)] + $options;
+      $sortQuery[$fieldName] = ['order' => strtolower($direction)];
     }
 
+    $body = array_merge_recursive($body, [
+      'sort' => $sortQuery,
+    ]);
+
     return [
       'index' => $this->index,
-      'body' => [
-        'sort' => $sortQuery,
-        'query' => $query,
-      ],
+      'body' => $body,
     ];
   }
 
diff --git a/modules/helfi_paragraphs_news_list/src/Plugin/ExternalEntities/StorageClient/NewsNeighbourhoods.php b/modules/helfi_paragraphs_news_list/src/Plugin/ExternalEntities/StorageClient/NewsNeighbourhoods.php
index 5ff1e0b86..86c54cd28 100644
--- a/modules/helfi_paragraphs_news_list/src/Plugin/ExternalEntities/StorageClient/NewsNeighbourhoods.php
+++ b/modules/helfi_paragraphs_news_list/src/Plugin/ExternalEntities/StorageClient/NewsNeighbourhoods.php
@@ -30,49 +30,4 @@ protected function getFieldMapping(string $field) : string {
     };
   }
 
-  /**
-   * Load nearest News neighbourhoods terms.
-   *
-   * Drupal query interface is not quite flexible enough to implement
-   * geo_distance sorting.
-   *
-   * @param float $lat
-   *   Latitude.
-   * @param float $lon
-   *   Longitude.
-   * @param int|null $start
-   *   (optional) The first item to return.
-   * @param int|null $length
-   *   (optional) The number of items to return.
-   *
-   * @return array
-   */
-  public function loadByCoordinates(
-    float $lat,
-    float $lon,
-    ?int $start = NULL,
-    ?int $length = NULL,
-  ): array {
-    return $this->query([], [
-      [
-        'field' => '_geo_distance',
-        'direction' => 'ASC',
-        'options' => [
-          $this->getFieldMapping('location') => [
-            'lat' => $lat,
-            'lon' => $lon,
-          ],
-          'unit' => 'km',
-          // Arc is more accurate, but within
-          // the city it should not matter.
-          'distance_type' => 'plane',
-          // What to do in case a field has several geo points.
-          'mode' => 'min',
-          // Unmapped field cause the search to fail.
-          'ignore_unmapped' => false
-        ],
-      ],
-    ], $start, $length);
-  }
-
 }
diff --git a/modules/helfi_paragraphs_news_list/tests/src/Kernel/ExternalEntityStorage/NewsNeighbourhoodsStorageClientTest.php b/modules/helfi_paragraphs_news_list/tests/src/Kernel/ExternalEntityStorage/NewsNeighbourhoodsStorageClientTest.php
index d7cf288da..605e2337c 100644
--- a/modules/helfi_paragraphs_news_list/tests/src/Kernel/ExternalEntityStorage/NewsNeighbourhoodsStorageClientTest.php
+++ b/modules/helfi_paragraphs_news_list/tests/src/Kernel/ExternalEntityStorage/NewsNeighbourhoodsStorageClientTest.php
@@ -4,7 +4,7 @@
 
 namespace Drupal\Tests\helfi_paragraphs_news_list\Kernel\ExternalEntityStorage;
 
-use Drupal\helfi_paragraphs_news_list\Plugin\ExternalEntities\StorageClient\NewsNeighbourhoods;
+use Drupal\external_entities\Entity\Query\External\Query;
 use Elastic\Elasticsearch\Client;
 
 /**
@@ -31,23 +31,25 @@ protected function getVid(): string {
   /**
    * Tests geo_distance sorting.
    */
-  public function testLocationQuery(): void {
+  public function testGeoDistanceQuery(): void {
     $client = $this->prophesize(Client::class);
     // Test geo distance sort.
     $client->search([
       'index' => 'news_terms',
       'body' => [
         'sort' => [
-          '_geo_distance' => [
-            'order' => 'asc',
-            'field_location' => [
-              'lat' => 48.8584,
-              'lon' => 2.2945,
+          [
+            '_geo_distance' => [
+              'field_location' => [
+                'lat' => 48.8584,
+                'lon' => 2.2945,
+              ],
+              'unit' => 'km',
+              'order' => 'asc',
+              'distance_type' => 'plane',
+              'mode' => 'min',
+              'ignore_unmapped' => FALSE,
             ],
-            'unit' => 'km',
-            'distance_type' => 'plane',
-            'mode' => 'min',
-            'ignore_unmapped' => FALSE,
           ],
         ],
         'query' => [
@@ -62,10 +64,34 @@ public function testLocationQuery(): void {
       ->shouldBeCalled()
       ->willReturn($this->createElasticsearchResponse([]));
 
-    $sut = $this->getSut($client->reveal())->getStorageClient();
-    $this->assertInstanceOf(NewsNeighbourhoods::class, $sut);
+    $query = $this->getSut($client->reveal())
+      ->getQuery();
 
-    $sut->loadByCoordinates(48.8584, 2.2945);
+    $this->assertInstanceOf(Query::class, $query);
+
+    // Drupal query interface is not quite flexible enough to support all the
+    // options and parameters geo_distance sort needs, so the implementation
+    // uses setParameter from external_entities Query class.
+    $query->setParameter('location', [
+      [
+        'lat' => 48.8584,
+        'lon' => 2.2945,
+      ],
+      [
+        'unit' => 'km',
+        // Geo distance sort direction.
+        'order' => 'asc',
+        // 'arc' is more accurate, but within
+        // a city it should not matter.
+        'distance_type' => 'plane',
+        // What to do in case a field has several geo points.
+        'mode' => 'min',
+        // Unmapped field cause the search to fail.
+        'ignore_unmapped' => FALSE,
+      ],
+    ], 'GEO_DISTANCE_SORT');
+
+    $query->accessCheck(FALSE)->execute();
   }
 
 }

From ee50819167175520c89018cdb384bc7431516704 Mon Sep 17 00:00:00 2001
From: Santeri Hurnanen <santeri.hurnanen@fame.fi>
Date: Fri, 17 Jan 2025 09:03:18 +0200
Subject: [PATCH 3/4] UHF-11002: Check null results in tests

This should not happen when client is not mocked
---
 .../src/ElasticExternalEntityBase.php                           | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/modules/helfi_paragraphs_news_list/src/ElasticExternalEntityBase.php b/modules/helfi_paragraphs_news_list/src/ElasticExternalEntityBase.php
index 983dd18d8..3ba491821 100644
--- a/modules/helfi_paragraphs_news_list/src/ElasticExternalEntityBase.php
+++ b/modules/helfi_paragraphs_news_list/src/ElasticExternalEntityBase.php
@@ -81,7 +81,7 @@ protected function request(
     array $parameters,
   ) : array {
     try {
-      return $this->client->search($parameters)->asArray();
+      return $this->client->search($parameters)?->asArray() ?? [];
     }
     catch (ElasticsearchException | TransportException $e) {
       Error::logException($this->logger, $e);

From 43149ebf6e4cf3cceda74192ac232319f21bd244 Mon Sep 17 00:00:00 2001
From: Santeri Hurnanen <santeri.hurnanen@fame.fi>
Date: Fri, 17 Jan 2025 09:14:55 +0200
Subject: [PATCH 4/4] UHF-11002: Update default query

---
 .../src/ElasticExternalEntityBase.php                        | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/modules/helfi_paragraphs_news_list/src/ElasticExternalEntityBase.php b/modules/helfi_paragraphs_news_list/src/ElasticExternalEntityBase.php
index 3ba491821..9a7d84935 100644
--- a/modules/helfi_paragraphs_news_list/src/ElasticExternalEntityBase.php
+++ b/modules/helfi_paragraphs_news_list/src/ElasticExternalEntityBase.php
@@ -219,7 +219,10 @@ protected function getOperatorCallback(?string $op): callable {
    *   The query.
    */
   protected function buildQuery(array $parameters, array $sorts) : array {
-    $body = [];
+    $body = [
+      'sort' => [],
+      'query' => [],
+    ];
 
     foreach ($parameters as $parameter) {
       ['field' => $field, 'value' => $value, 'operator' => $op] = $parameter;