Merge branch 'main' of https://github.com/opensearch-project/OpenSearch…

… into main

Signed-off-by: Andreas [email protected]

.gitignore

@@ -10,6 +10,7 @@ out/
 # include shared intellij config
 !.idea/inspectionProfiles/Project_Default.xml
 !.idea/runConfigurations/Debug_OpenSearch.xml
+!.idea/vcs.xml
 
 # These files are generated in the main tree by IntelliJ
 benchmarks/src/main/generated/*

TESTING.md

@@ -421,9 +421,11 @@ Unit tests are the preferred way to test some functionality: most of the time th
 
 The reason why `OpenSearchSingleNodeTestCase` exists is that all our components used to be very hard to set up in isolation, which had led us to having a number of integration tests but close to no unit tests. `OpenSearchSingleNodeTestCase` is a workaround for this issue which provides an easy way to spin up a node and get access to components that are hard to instantiate like `IndicesService`. Whenever practical, you should prefer unit tests.
 
-Many tests extend `OpenSearchIntegTestCase`, mostly because this is how most tests used to work in the early days of Elasticsearch. However, the complexity of these tests tends to make them hard to debug. Whenever the functionality that is being tested isn’t intimately dependent on how OpenSearch behaves as a cluster, it is recommended to write unit tests or REST tests instead.
+Finally, if the the functionality under test needs to be run in a cluster, there are two test classes to consider:
+  * `OpenSearchRestTestCase` will connect to an external cluster. This is a good option if the tests cases don't rely on a specific configuration of the test cluster. A test cluster is set up as part of the Gradle task running integration tests, and test cases using this class can connect to it. The configuration of the cluster is provided in the Gradle files.
+  * `OpenSearchIntegTestCase` will create a local cluster as part of each test case. The configuration of the cluster is controlled by the test class. This is a good option if different tests cases depend on different cluster configurations, as it would be impractical (and limit parallelization) to keep re-configuring (and re-starting) the external cluster for each test case. A good example of when this class might come in handy is for testing security features, where different cluster configurations are needed to fully test each one.
 
-In short, most new functionality should come with unit tests, and optionally REST tests to test integration.
+In short, most new functionality should come with unit tests, and optionally integration tests using either an external cluster or a local one if there's a need for more specific cluster configurations, as those are more costly and harder to maintain/debug.
 
 ### Refactor code to make it easier to test
 

buildSrc/build.gradle

@@ -116,7 +116,7 @@ dependencies {
   api 'com.avast.gradle:gradle-docker-compose-plugin:0.14.12'
   api 'org.apache.maven:maven-model:3.6.2'
   api 'com.networknt:json-schema-validator:1.0.36'
-  api 'com.fasterxml.jackson.core:jackson-databind:2.12.5'
+  api "com.fasterxml.jackson.core:jackson-databind:${props.getProperty('jackson')}"
 
   testFixturesApi "junit:junit:${props.getProperty('junit')}"
   testFixturesApi "com.carrotsearch.randomizedtesting:randomizedtesting-runner:${props.getProperty('randomizedrunner')}"

buildSrc/src/main/java/org/opensearch/gradle/testclusters/StandaloneRestIntegTestTask.java

@@ -31,6 +31,7 @@
 
 package org.opensearch.gradle.testclusters;
 
+import groovy.lang.Closure;
 import org.opensearch.gradle.FileSystemOperationsAware;
 import org.opensearch.gradle.test.Fixture;
 import org.opensearch.gradle.util.GradleUtils;
@@ -60,6 +61,7 @@
 public class StandaloneRestIntegTestTask extends Test implements TestClustersAware, FileSystemOperationsAware {
 
     private Collection<OpenSearchCluster> clusters = new HashSet<>();
+    private Closure<Void> beforeStart;
 
     public StandaloneRestIntegTestTask() {
         this.getOutputs()
@@ -86,6 +88,18 @@ public StandaloneRestIntegTestTask() {
             );
     }
 
+    // Hook for executing any custom logic before starting the task.
+    public void setBeforeStart(Closure<Void> closure) {
+        beforeStart = closure;
+    }
+
+    @Override
+    public void beforeStart() {
+        if (beforeStart != null) {
+            beforeStart.call(this);
+        }
+    }
+
     @Override
     public int getMaxParallelForks() {
         return 1;

buildSrc/version.properties

@@ -9,7 +9,7 @@ bundled_jdk = 17.0.1+12
 # optional dependencies
 spatial4j         = 0.7
 jts               = 1.15.0
-jackson           = 2.12.5
+jackson           = 2.12.6
 snakeyaml         = 1.26
 icu4j             = 62.1
 supercsv          = 2.4.0
@@ -35,9 +35,9 @@ httpasyncclient   = 4.1.4
 commonslogging    = 1.1.3
 commonscodec      = 1.13
 hamcrest          = 2.1
-mockito           = 4.2.0
+mockito           = 4.3.1
 objenesis         = 3.2
-bytebuddy         = 1.12.6
+bytebuddy         = 1.12.7
 
 # benchmark dependencies
 jmh               = 1.19

client/rest-high-level/src/main/java/org/opensearch/client/IndicesClient.java

@@ -40,7 +40,6 @@
 import org.opensearch.action.admin.indices.delete.DeleteIndexRequest;
 import org.opensearch.action.admin.indices.flush.FlushRequest;
 import org.opensearch.action.admin.indices.flush.FlushResponse;
-import org.opensearch.action.admin.indices.flush.SyncedFlushRequest;
 import org.opensearch.action.admin.indices.forcemerge.ForceMergeRequest;
 import org.opensearch.action.admin.indices.forcemerge.ForceMergeResponse;
 import org.opensearch.action.admin.indices.open.OpenIndexRequest;
@@ -931,53 +930,6 @@ public Cancellable flushAsync(FlushRequest flushRequest, RequestOptions options,
         );
     }
 
-    /**
-     * Initiate a synced flush manually using the synced flush API.
-     *
-     * @param syncedFlushRequest the request
-     * @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
-     * @return the response
-     * @throws IOException in case there is a problem sending the request or parsing back the response
-     * @deprecated synced flush is deprecated and will be removed in 8.0.
-     * Use {@link #flush(FlushRequest, RequestOptions)} instead.
-     */
-    @Deprecated
-    public SyncedFlushResponse flushSynced(SyncedFlushRequest syncedFlushRequest, RequestOptions options) throws IOException {
-        return restHighLevelClient.performRequestAndParseEntity(
-            syncedFlushRequest,
-            IndicesRequestConverters::flushSynced,
-            options,
-            SyncedFlushResponse::fromXContent,
-            emptySet()
-        );
-    }
-
-    /**
-     * Asynchronously initiate a synced flush manually using the synced flush API.
-     *
-     * @param syncedFlushRequest the request
-     * @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
-     * @param listener the listener to be notified upon request completion
-     * @return cancellable that may be used to cancel the request
-     * @deprecated synced flush is deprecated and will be removed in 8.0.
-     * Use {@link #flushAsync(FlushRequest, RequestOptions, ActionListener)} instead.
-     */
-    @Deprecated
-    public Cancellable flushSyncedAsync(
-        SyncedFlushRequest syncedFlushRequest,
-        RequestOptions options,
-        ActionListener<SyncedFlushResponse> listener
-    ) {
-        return restHighLevelClient.performRequestAsyncAndParseEntity(
-            syncedFlushRequest,
-            IndicesRequestConverters::flushSynced,
-            options,
-            SyncedFlushResponse::fromXContent,
-            listener,
-            emptySet()
-        );
-    }
-
     /**
      * Retrieve the settings of one or more indices.
      *

client/rest-high-level/src/main/java/org/opensearch/client/IndicesRequestConverters.java

@@ -42,7 +42,6 @@
 import org.opensearch.action.admin.indices.cache.clear.ClearIndicesCacheRequest;
 import org.opensearch.action.admin.indices.delete.DeleteIndexRequest;
 import org.opensearch.action.admin.indices.flush.FlushRequest;
-import org.opensearch.action.admin.indices.flush.SyncedFlushRequest;
 import org.opensearch.action.admin.indices.forcemerge.ForceMergeRequest;
 import org.opensearch.action.admin.indices.open.OpenIndexRequest;
 import org.opensearch.action.admin.indices.refresh.RefreshRequest;
@@ -322,15 +321,6 @@ static Request flush(FlushRequest flushRequest) {
         return request;
     }
 
-    static Request flushSynced(SyncedFlushRequest syncedFlushRequest) {
-        String[] indices = syncedFlushRequest.indices() == null ? Strings.EMPTY_ARRAY : syncedFlushRequest.indices();
-        Request request = new Request(HttpPost.METHOD_NAME, RequestConverters.endpoint(indices, "_flush/synced"));
-        RequestConverters.Params parameters = new RequestConverters.Params();
-        parameters.withIndicesOptions(syncedFlushRequest.indicesOptions());
-        request.addParameters(parameters.asMap());
-        return request;
-    }
-
     static Request forceMerge(ForceMergeRequest forceMergeRequest) {
         String[] indices = forceMergeRequest.indices() == null ? Strings.EMPTY_ARRAY : forceMergeRequest.indices();
         Request request = new Request(HttpPost.METHOD_NAME, RequestConverters.endpoint(indices, "_forcemerge"));