8.18 upgrade

README.md

@@ -150,6 +150,10 @@ Result:
 ### Build
 
 Built with Java 17 and Gradle 8.10.2 (use the packaged gradlew included in this repo if you want to build yourself).
+Apply spotless code formatting with:
+``` shell
+./gradlew spotlessApply
+```
 
 ### Development Environment Setup
 

build.gradle

@@ -1,3 +1,7 @@
+/*
+Gradle build file for development building, testing, and packaging.
+*/
+
 buildscript {
     repositories {
         mavenCentral()
@@ -8,6 +12,11 @@ buildscript {
     }
 }
 
+// the spotless plugin is dedicated to format the code
+plugins {
+    id("com.diffplug.spotless") version "7.0.3"
+}
+
 repositories {
     mavenCentral()
     mavenLocal()
@@ -32,6 +41,18 @@ esplugin {
     noticeFile = rootProject.file('README.md')
 }
 
+// automatic formatting configuration (the same configuration as elasticsearch)
+spotless {
+    java {
+        importOrderFile('config/elastic.importorder') // import order file as exported from elastic
+        eclipse().configFile('config/formatterConfig.xml')
+        trimTrailingWhitespace()
+        target 'src/**/*.java'
+    }
+}
+
+check.dependsOn spotlessCheck
+
 // exclude junit from implementation to resolve a version conflict (but not from tests)
 configurations {
     implementation {
@@ -43,14 +64,21 @@ dependencies {
     implementation "org.elasticsearch:elasticsearch:${es_version}"
 
     yamlRestTestImplementation "org.elasticsearch.test:framework:${es_version}"
+    yamlRestTestImplementation "org.elasticsearch.test:yaml-rest-runner:${es_version}"
     // required for ESClientYamlSuiteTestCase; fixes "RestApiYamlIT > initializationError"
     yamlRestTestImplementation "org.apache.logging.log4j:log4j-core:2.17.1"
 }
 
-// Make sure the ES distribution used for rest tests is the "complete" variant
-// Otherwise weirds errors occur (like the geo_shape type is not handled)
 testClusters.configureEach {
-    testDistribution = 'DEFAULT'
-    // disable security to disable failing warnings
-    setting 'xpack.security.enabled', 'false'
+        // Use full distribution otherwise the geo_shape type is not fully handled
+        testDistribution = 'DEFAULT'
+
+        // Disable security (avoids annoying warnings/failures with xpack features)
+        setting 'xpack.security.enabled', 'false'
+
+        // Logging levels for debugging: logs are located in build/testclusters/runTask-0/logs/
+        setting 'logger._root', 'DEBUG'
+        setting 'logger.org.elasticsearch.plugins', 'DEBUG'
+        setting 'logger.org.elasticsearch.cluster', 'DEBUG'
+        setting 'logger.org.elasticsearch.cluster.metadata', 'TRACE'
 }

config/elastic.importorder

@@ -0,0 +1,8 @@
+#source: elasticsearch/build-conventions/elastic.importorder
+#Eclipse configuration for import order for Elasticsearch
+0=
+1=com
+2=org
+3=java
+4=javax
+5=\#

gradle.properties

@@ -1,2 +1,2 @@
-es_version = 7.17.28
-plugin_version = 7.17.28.0
+es_version = 8.18.2
+plugin_version = 8.18.2.0

gradle/wrapper/gradle-wrapper.properties

@@ -1,6 +1,6 @@
 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
-distributionUrl=https\://services.gradle.org/distributions/gradle-8.10.2-bin.zip
+distributionUrl=https\://services.gradle.org/distributions/gradle-8.14-bin.zip
 networkTimeout=10000
 validateDistributionUrl=true
 zipStoreBase=GRADLE_USER_HOME

gradlew

@@ -86,8 +86,7 @@ done
 # shellcheck disable=SC2034
 APP_BASE_NAME=${0##*/}
 # Discard cd standard output in case $CDPATH is set (https://github.com/gradle/gradle/issues/25036)
-APP_HOME=$( cd -P "${APP_HOME:-./}" > /dev/null && printf '%s
-' "$PWD" ) || exit
+APP_HOME=$( cd -P "${APP_HOME:-./}" > /dev/null && printf '%s\n' "$PWD" ) || exit
 
 # Use the maximum available, or set MAX_FD != -1 to use that value.
 MAX_FD=maximum
@@ -115,7 +114,7 @@ case "$( uname )" in                #(
   NONSTOP* )        nonstop=true ;;
 esac
 
-CLASSPATH=$APP_HOME/gradle/wrapper/gradle-wrapper.jar
+CLASSPATH="\\\"\\\""
 
 
 # Determine the Java command to use to start the JVM.
@@ -206,15 +205,15 @@ fi
 DEFAULT_JVM_OPTS='"-Xmx64m" "-Xms64m"'
 
 # Collect all arguments for the java command:
-#   * DEFAULT_JVM_OPTS, JAVA_OPTS, JAVA_OPTS, and optsEnvironmentVar are not allowed to contain shell fragments,
+#   * DEFAULT_JVM_OPTS, JAVA_OPTS, and optsEnvironmentVar are not allowed to contain shell fragments,
 #     and any embedded shellness will be escaped.
 #   * For example: A user cannot expect ${Hostname} to be expanded, as it is an environment variable and will be
 #     treated as '${Hostname}' itself on the command line.
 
 set -- \
         "-Dorg.gradle.appname=$APP_BASE_NAME" \
         -classpath "$CLASSPATH" \
-        org.gradle.wrapper.GradleWrapperMain \
+        -jar "$APP_HOME/gradle/wrapper/gradle-wrapper.jar" \
         "$@"
 
 # Stop when "xargs" is not available.

gradlew.bat

@@ -70,11 +70,11 @@ goto fail
 :execute
 @rem Setup the command line
 
-set CLASSPATH=%APP_HOME%\gradle\wrapper\gradle-wrapper.jar
+set CLASSPATH=
 
 
 @rem Execute Gradle
-"%JAVA_EXE%" %DEFAULT_JVM_OPTS% %JAVA_OPTS% %GRADLE_OPTS% "-Dorg.gradle.appname=%APP_BASE_NAME%" -classpath "%CLASSPATH%" org.gradle.wrapper.GradleWrapperMain %*
+"%JAVA_EXE%" %DEFAULT_JVM_OPTS% %JAVA_OPTS% %GRADLE_OPTS% "-Dorg.gradle.appname=%APP_BASE_NAME%" -classpath "%CLASSPATH%" -jar "%APP_HOME%\gradle\wrapper\gradle-wrapper.jar" %*
 
 :end
 @rem End local scope for the variables with windows NT shell

src/main/java/com/opendatasoft/elasticsearch/plugin/GeoPointClusteringAggregationPlugin.java

@@ -2,6 +2,7 @@
 
 import com.opendatasoft.elasticsearch.search.aggregations.bucket.geopointclustering.GeoPointClusteringAggregationBuilder;
 import com.opendatasoft.elasticsearch.search.aggregations.bucket.geopointclustering.InternalGeoPointClustering;
+
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.plugins.SearchPlugin;
 
@@ -13,12 +14,12 @@ public ArrayList<SearchPlugin.AggregationSpec> getAggregations() {
         ArrayList<SearchPlugin.AggregationSpec> r = new ArrayList<>();
 
         r.add(
-                new AggregationSpec(
-                        GeoPointClusteringAggregationBuilder.NAME,
-                        GeoPointClusteringAggregationBuilder::new,
-                        GeoPointClusteringAggregationBuilder::parse)
-                        .addResultReader(InternalGeoPointClustering::new)
-                        .setAggregatorRegistrar(GeoPointClusteringAggregationBuilder::registerAggregators)
+            new AggregationSpec(
+                GeoPointClusteringAggregationBuilder.NAME,
+                GeoPointClusteringAggregationBuilder::new,
+                GeoPointClusteringAggregationBuilder::parse
+            ).addResultReader(InternalGeoPointClustering::new)
+                .setAggregatorRegistrar(GeoPointClusteringAggregationBuilder::registerAggregators)
         );
 
         return r;

src/main/java/com/opendatasoft/elasticsearch/search/aggregations/bucket/geopointclustering/BucketReducer.java

@@ -0,0 +1,87 @@
+package com.opendatasoft.elasticsearch.search.aggregations.bucket.geopointclustering;
+
+import org.elasticsearch.common.geo.GeoPoint;
+import org.elasticsearch.core.Releasable;
+import org.elasticsearch.core.Releasables;
+import org.elasticsearch.search.aggregations.AggregationReduceContext;
+import org.elasticsearch.search.aggregations.AggregatorsReducer;
+import org.elasticsearch.search.aggregations.InternalAggregations;
+
+/**
+ *  Class for reducing a list of {@link com.opendatasoft.elasticsearch.search.aggregations.bucket.geopointclustering.InternalGeoPointClustering.Bucket} to a single {@link InternalAggregations} and the number of documents.
+ *  Inspired by {@link org.elasticsearch.search.aggregations.bucket.BucketReducer} but adapted for the
+ *  GeoPointClustering aggregation.
+ *  Particularity: it calculates a weighted centroid based on the document counts of the buckets. This weighted centroid
+ *  is then used to create the final cluster centroid in the reduce phase InternalGeoPointClustering::AggregatorReducer.get().
+ *
+ */
+public final class BucketReducer implements Releasable {
+
+    private final AggregatorsReducer aggregatorsReducer;
+    private final InternalGeoPointClustering.Bucket proto;
+    private long count = 0;
+    private double centroidLat = 0;
+    private double centroidLon = 0;
+
+    /**
+     * Untouched from the original BucketReducer.
+     */
+    public BucketReducer(InternalGeoPointClustering.Bucket proto, AggregationReduceContext context, int size) {
+        this.aggregatorsReducer = new AggregatorsReducer(proto.getAggregations(), context, size);
+        this.proto = proto;
+    }
+
+    /**
+     * Accepts a bucket and updates the count and weighted centroid.
+     * The centroid is calculated as the sum of latitudes and longitudes
+     * weighted by the document count of each bucket.
+     *
+     * @param bucket the bucket to accept
+     */
+    public void accept(InternalGeoPointClustering.Bucket bucket) {
+        count += bucket.getDocCount();
+        centroidLat += bucket.centroid.getLat() * bucket.getDocCount();
+        centroidLon += bucket.centroid.getLon() * bucket.getDocCount();
+        aggregatorsReducer.accept(bucket.getAggregations());
+    }
+
+    /**
+     * Untouched from the original BucketReducer.
+     */
+    public InternalGeoPointClustering.Bucket getProto() {
+        return proto;
+    }
+
+    /**
+     * Untouched from the original BucketReducer.
+     */
+    public InternalAggregations getAggregations() {
+        return aggregatorsReducer.get();
+    }
+
+    /**
+     * Untouched from the original BucketReducer.
+     */
+    public long getDocCount() {
+        return count;
+    }
+
+    /**
+     * Returns the weighted centroid of the buckets processed so far.
+     * The centroid is calculated as the sum of latitudes and longitudes
+     * weighted by the document count of each bucket.
+     *
+     * @return the weighted centroid as a {@link GeoPoint}
+     */
+    public GeoPoint getWeightedCentroid() {
+        return new GeoPoint(centroidLat / count, centroidLon / count);
+    }
+
+    /**
+     * Untouched from the original BucketReducer.
+     */
+    @Override
+    public void close() {
+        Releasables.close(aggregatorsReducer);
+    }
+}

src/main/java/com/opendatasoft/elasticsearch/search/aggregations/bucket/geopointclustering/GeoPointClustering.java

@@ -5,15 +5,14 @@
 import java.util.List;
 
 /**
- * A {@code geopoint_clustering} aggregation. Buckets key are centroids.
+ * A {@code geopoint_clustering} aggregation.
  */
 public interface GeoPointClustering extends MultiBucketsAggregation {
 
     /**
      * A bucket that is associated with a {@code geopoint_clustering} cell. The key of the bucket is the {@code geohash} of the cell
      */
-    interface Bucket extends MultiBucketsAggregation.Bucket {
-    }
+    interface Bucket extends MultiBucketsAggregation.Bucket {}
 
     /**
      * @return  The buckets of this aggregation (each bucket representing a cluster of points)