Move ImmutableOpenMap usage to j.u.Map

Signed-off-by: Junqiu Lei <junqiu@amazon.com>
opensearch-project · Jul 8, 2023 · 2943e01 · 2943e01
1 parent d90964d
commit 2943e01
Show file tree

Hide file tree

Showing 12 changed files with 38 additions and 37 deletions.
diff --git a/src/main/java/org/opensearch/knn/common/exception/DeleteModelWhenInTrainStateException.java b/src/main/java/org/opensearch/knn/common/exception/DeleteModelWhenInTrainStateException.java
@@ -6,7 +6,7 @@
 package org.opensearch.knn.common.exception;
 
 import org.opensearch.OpenSearchException;
-import org.opensearch.common.logging.LoggerMessageFormat;
+import org.opensearch.core.common.logging.LoggerMessageFormat;
 import org.opensearch.rest.RestStatus;
 
 /**

diff --git a/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java b/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java
@@ -6,6 +6,7 @@
 package org.opensearch.knn.index.query;
 
 import lombok.extern.log4j.Log4j2;
+import org.apache.commons.lang.StringUtils;
 import org.opensearch.Version;
 import org.opensearch.index.mapper.NumberFieldMapper;
 import org.opensearch.index.query.QueryBuilder;
@@ -19,7 +20,6 @@
 import org.apache.lucene.search.Query;
 import org.opensearch.core.ParseField;
 import org.opensearch.common.ParsingException;
-import org.opensearch.common.Strings;
 import org.opensearch.common.io.stream.StreamInput;
 import org.opensearch.common.io.stream.StreamOutput;
 import org.opensearch.core.xcontent.XContentBuilder;
@@ -68,7 +68,7 @@ public KNNQueryBuilder(String fieldName, float[] vector, int k) {
     }
 
     public KNNQueryBuilder(String fieldName, float[] vector, int k, QueryBuilder filter) {
-        if (Strings.isNullOrEmpty(fieldName)) {
+        if (StringUtils.isBlank(fieldName)) {
             throw new IllegalArgumentException("[" + NAME + "] requires fieldName");
         }
         if (vector == null) {

diff --git a/src/main/java/org/opensearch/knn/plugin/rest/RestDeleteModelHandler.java b/src/main/java/org/opensearch/knn/plugin/rest/RestDeleteModelHandler.java
@@ -11,9 +11,9 @@
 
 package org.opensearch.knn.plugin.rest;
 
+import org.apache.commons.lang.StringUtils;
 import com.google.common.collect.ImmutableList;
 import org.opensearch.client.node.NodeClient;
-import org.opensearch.common.Strings;
 import org.opensearch.knn.plugin.KNNPlugin;
 import org.opensearch.knn.plugin.transport.DeleteModelAction;
 import org.opensearch.knn.plugin.transport.DeleteModelRequest;
@@ -58,7 +58,7 @@ public List<Route> routes() {
     @Override
     protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) {
         String modelID = request.param(MODEL_ID);
-        if (!Strings.hasText(modelID)) {
+        if (StringUtils.isBlank(modelID)) {
             throw new IllegalArgumentException("model ID cannot be empty");
         }
         DeleteModelRequest deleteModelRequest = new DeleteModelRequest(modelID);

diff --git a/src/main/java/org/opensearch/knn/plugin/rest/RestGetModelHandler.java b/src/main/java/org/opensearch/knn/plugin/rest/RestGetModelHandler.java
@@ -12,8 +12,8 @@
 package org.opensearch.knn.plugin.rest;
 
 import com.google.common.collect.ImmutableList;
+import org.apache.commons.lang.StringUtils;
 import org.opensearch.client.node.NodeClient;
-import org.opensearch.common.Strings;
 import org.opensearch.knn.plugin.KNNPlugin;
 import org.opensearch.knn.plugin.transport.GetModelAction;
 import org.opensearch.knn.plugin.transport.GetModelRequest;
@@ -50,7 +50,7 @@ public List<Route> routes() {
     @Override
     protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException {
         String modelID = restRequest.param(MODEL_ID);
-        if (!Strings.hasText(modelID)) {
+        if (StringUtils.isBlank(modelID)) {
             throw new IllegalArgumentException("model ID cannot be empty");
         }
 

diff --git a/src/main/java/org/opensearch/knn/plugin/rest/RestKNNStatsHandler.java b/src/main/java/org/opensearch/knn/plugin/rest/RestKNNStatsHandler.java
@@ -6,12 +6,12 @@
 package org.opensearch.knn.plugin.rest;
 
 import lombok.AllArgsConstructor;
+import org.apache.commons.lang.StringUtils;
 import org.opensearch.knn.plugin.KNNPlugin;
 import org.opensearch.knn.plugin.transport.KNNStatsAction;
 import org.opensearch.knn.plugin.transport.KNNStatsRequest;
 import com.google.common.collect.ImmutableList;
 import org.opensearch.client.node.NodeClient;
-import org.opensearch.common.Strings;
 import org.opensearch.rest.BaseRestHandler;
 import org.opensearch.rest.RestRequest;
 import org.opensearch.rest.action.RestActions;
@@ -83,7 +83,7 @@ private KNNStatsRequest getRequest(RestRequest request) {
         // parse the nodes the user wants to query
         String[] nodeIdsArr = null;
         String nodesIdsStr = request.param("nodeId");
-        if (!Strings.isEmpty(nodesIdsStr)) {
+        if (StringUtils.isNotEmpty(nodesIdsStr)) {
             nodeIdsArr = nodesIdsStr.split(",");
         }
 
@@ -93,7 +93,7 @@ private KNNStatsRequest getRequest(RestRequest request) {
         // parse the stats the customer wants to see
         Set<String> statsSet = null;
         String statsStr = request.param("stat");
-        if (!Strings.isEmpty(statsStr)) {
+        if (StringUtils.isNotEmpty(statsStr)) {
             statsSet = new HashSet<>(Arrays.asList(statsStr.split(",")));
         }
 

diff --git a/src/main/java/org/opensearch/knn/plugin/rest/RestKNNWarmupHandler.java b/src/main/java/org/opensearch/knn/plugin/rest/RestKNNWarmupHandler.java
@@ -5,6 +5,7 @@
 
 package org.opensearch.knn.plugin.rest;
 
+import org.apache.commons.lang.StringUtils;
 import org.opensearch.knn.common.exception.KNNInvalidIndicesException;
 import org.opensearch.knn.plugin.KNNPlugin;
 import org.opensearch.knn.plugin.transport.KNNWarmupAction;
@@ -15,7 +16,6 @@
 import org.opensearch.client.node.NodeClient;
 import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
 import org.opensearch.cluster.service.ClusterService;
-import org.opensearch.common.Strings;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.index.Index;
 import org.opensearch.rest.BaseRestHandler;
@@ -81,7 +81,7 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
     }
 
     private KNNWarmupRequest createKNNWarmupRequest(RestRequest request) {
-        String[] indexNames = Strings.splitStringByCommaToArray(request.param("index"));
+        String[] indexNames = StringUtils.split(request.param("index"), ",");
         Index[] indices = indexNameExpressionResolver.concreteIndices(clusterService.state(), strictExpandOpen(), indexNames);
         List<String> invalidIndexNames = new ArrayList<>();
 

diff --git a/src/main/java/org/opensearch/knn/plugin/transport/DeleteModelRequest.java b/src/main/java/org/opensearch/knn/plugin/transport/DeleteModelRequest.java
@@ -11,9 +11,9 @@
 
 package org.opensearch.knn.plugin.transport;
 
+import org.apache.commons.lang.StringUtils;
 import org.opensearch.action.ActionRequest;
 import org.opensearch.action.ActionRequestValidationException;
-import org.opensearch.common.Strings;
 import org.opensearch.common.io.stream.StreamInput;
 import org.opensearch.common.io.stream.StreamOutput;
 
@@ -43,7 +43,7 @@ public void writeTo(StreamOutput out) throws IOException {
 
     @Override
     public ActionRequestValidationException validate() {
-        if (Strings.hasText(modelID)) {
+        if (StringUtils.isNotBlank(modelID)) {
             return null;
         }
         return addValidationError("Model id cannot be empty ", null);

diff --git a/src/main/java/org/opensearch/knn/plugin/transport/TrainingJobRouterTransportAction.java b/src/main/java/org/opensearch/knn/plugin/transport/TrainingJobRouterTransportAction.java
@@ -11,6 +11,7 @@
 
 package org.opensearch.knn.plugin.transport;
 
+import org.apache.commons.lang.StringUtils;
 import org.opensearch.action.ActionListener;
 import org.opensearch.action.ActionListenerResponseHandler;
 import org.opensearch.action.search.SearchRequest;
@@ -19,15 +20,15 @@
 import org.opensearch.client.Client;
 import org.opensearch.cluster.node.DiscoveryNode;
 import org.opensearch.cluster.service.ClusterService;
-import org.opensearch.common.Strings;
 import org.opensearch.common.ValidationException;
-import org.opensearch.common.collect.ImmutableOpenMap;
 import org.opensearch.common.inject.Inject;
 import org.opensearch.search.builder.SearchSourceBuilder;
 import org.opensearch.tasks.Task;
 import org.opensearch.transport.TransportRequestOptions;
 import org.opensearch.transport.TransportService;
 
+import java.util.Map;
+
 import static org.opensearch.knn.common.KNNConstants.BYTES_PER_KILOBYTES;
 import static org.opensearch.search.internal.SearchContext.DEFAULT_TERMINATE_AFTER;
 
@@ -94,7 +95,7 @@ protected DiscoveryNode selectNode(String preferredNode, TrainingJobRouteDecisio
 
         DiscoveryNode selectedNode = null;
 
-        ImmutableOpenMap<String, DiscoveryNode> eligibleNodes = clusterService.state().nodes().getDataNodes();
+        Map<String, DiscoveryNode> eligibleNodes = clusterService.state().nodes().getDataNodes();
         DiscoveryNode currentNode;
 
         for (TrainingJobRouteDecisionInfoNodeResponse response : jobInfo.getNodes()) {
@@ -107,7 +108,7 @@ protected DiscoveryNode selectNode(String preferredNode, TrainingJobRouteDecisio
             if (response.getTrainingJobCount() < 1) {
                 selectedNode = currentNode;
                 // Return right away if the user didnt pass a preferred node or this is the preferred node
-                if (Strings.isEmpty(preferredNode) || selectedNode.getId().equals(preferredNode)) {
+                if (StringUtils.isEmpty(preferredNode) || selectedNode.getId().equals(preferredNode)) {
                     return selectedNode;
                 }
             }

diff --git a/src/main/java/org/opensearch/knn/training/TrainingJob.java b/src/main/java/org/opensearch/knn/training/TrainingJob.java
@@ -11,9 +11,9 @@
 
 package org.opensearch.knn.training;
 
+import org.apache.commons.lang.StringUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
-import org.opensearch.common.Strings;
 import org.opensearch.common.UUIDs;
 import org.opensearch.knn.common.KNNConstants;
 import org.opensearch.knn.index.KNNSettings;
@@ -68,7 +68,7 @@ public TrainingJob(
         String description
     ) {
         // Generate random base64 string if one is not provided
-        this.modelId = Strings.hasText(modelId) ? modelId : UUIDs.randomBase64UUID();
+        this.modelId = StringUtils.isNotBlank(modelId) ? modelId : UUIDs.randomBase64UUID();
         this.knnMethodContext = Objects.requireNonNull(knnMethodContext, "MethodContext cannot be null.");
         this.nativeMemoryCacheManager = Objects.requireNonNull(nativeMemoryCacheManager, "NativeMemoryCacheManager cannot be null.");
         this.trainingDataEntryContext = Objects.requireNonNull(trainingDataEntryContext, "TrainingDataEntryContext cannot be null.");

diff --git a/src/test/java/org/opensearch/knn/plugin/transport/TrainingJobRouterTransportActionTests.java b/src/test/java/org/opensearch/knn/plugin/transport/TrainingJobRouterTransportActionTests.java
@@ -22,7 +22,6 @@
 import org.opensearch.cluster.node.DiscoveryNode;
 import org.opensearch.cluster.node.DiscoveryNodes;
 import org.opensearch.cluster.service.ClusterService;
-import org.opensearch.common.collect.ImmutableOpenMap;
 import org.opensearch.knn.KNNTestCase;
 import org.opensearch.knn.index.KNNMethodContext;
 import org.opensearch.search.SearchHit;
@@ -31,7 +30,9 @@
 
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 import static org.mockito.Mockito.any;
 import static org.mockito.Mockito.doAnswer;
@@ -45,7 +46,7 @@ public void testSingleNode_withCapacity() {
         // Mock datanodes in the cluster through mocking the cluster service
         List<String> nodeIds = ImmutableList.of("node-1");
 
-        ImmutableOpenMap<String, DiscoveryNode> discoveryNodesMap = generateDiscoveryNodes(nodeIds);
+        Map<String, DiscoveryNode> discoveryNodesMap = generateDiscoveryNodes(nodeIds);
         ClusterService clusterService = generateMockedClusterService(discoveryNodesMap);
 
         // Create a response to be returned with job route decision info
@@ -85,7 +86,7 @@ public void testSingleNode_withoutCapacity() {
         // Mock datanodes in the cluster through mocking the cluster service
         List<String> nodeIds = ImmutableList.of("node-1");
 
-        ImmutableOpenMap<String, DiscoveryNode> discoveryNodesMap = generateDiscoveryNodes(nodeIds);
+        Map<String, DiscoveryNode> discoveryNodesMap = generateDiscoveryNodes(nodeIds);
         ClusterService clusterService = generateMockedClusterService(discoveryNodesMap);
 
         // Create a response to be returned with job route decision info
@@ -125,7 +126,7 @@ public void testMultiNode_withCapacity() {
         // Mock datanodes in the cluster through mocking the cluster service
         List<String> nodeIds = ImmutableList.of("node-1", "node-2", "node-3");
 
-        ImmutableOpenMap<String, DiscoveryNode> discoveryNodesMap = generateDiscoveryNodes(nodeIds);
+        Map<String, DiscoveryNode> discoveryNodesMap = generateDiscoveryNodes(nodeIds);
         ClusterService clusterService = generateMockedClusterService(discoveryNodesMap);
 
         // Create a response to be returned with job route decision info
@@ -168,7 +169,7 @@ public void testMultiNode_withCapacity_withPreferredAvailable() {
 
         String preferredNode = nodeIds.get(2);
 
-        ImmutableOpenMap<String, DiscoveryNode> discoveryNodesMap = generateDiscoveryNodes(nodeIds);
+        Map<String, DiscoveryNode> discoveryNodesMap = generateDiscoveryNodes(nodeIds);
         ClusterService clusterService = generateMockedClusterService(discoveryNodesMap);
 
         // Create a response to be returned with job route decision info
@@ -211,7 +212,7 @@ public void testMultiNode_withCapacity_withoutPreferredAvailable() {
 
         String preferredNode = nodeIds.get(2);
 
-        ImmutableOpenMap<String, DiscoveryNode> discoveryNodesMap = generateDiscoveryNodes(nodeIds);
+        Map<String, DiscoveryNode> discoveryNodesMap = generateDiscoveryNodes(nodeIds);
         ClusterService clusterService = generateMockedClusterService(discoveryNodesMap);
 
         // Create a response to be returned with job route decision info
@@ -253,7 +254,7 @@ public void testMultiNode_withoutCapacity() {
         // Mock datanodes in the cluster through mocking the cluster service
         List<String> nodeIds = ImmutableList.of("node-1", "node-2", "node-3");
 
-        ImmutableOpenMap<String, DiscoveryNode> discoveryNodesMap = generateDiscoveryNodes(nodeIds);
+        Map<String, DiscoveryNode> discoveryNodesMap = generateDiscoveryNodes(nodeIds);
         ClusterService clusterService = generateMockedClusterService(discoveryNodesMap);
 
         // Create a response to be returned with job route decision info
@@ -338,19 +339,19 @@ public void testTrainingIndexSize() {
         transportAction.getTrainingIndexSizeInKB(trainingModelRequest, listener);
     }
 
-    private ImmutableOpenMap<String, DiscoveryNode> generateDiscoveryNodes(List<String> dataNodeIds) {
-        ImmutableOpenMap.Builder<String, DiscoveryNode> builder = ImmutableOpenMap.builder();
+    private Map<String, DiscoveryNode> generateDiscoveryNodes(List<String> dataNodeIds) {
+        Map<String, DiscoveryNode> nodes = new HashMap<>();
 
         for (String nodeId : dataNodeIds) {
             DiscoveryNode discoveryNode = mock(DiscoveryNode.class);
             when(discoveryNode.getId()).thenReturn(nodeId);
-            builder.put(nodeId, discoveryNode);
+            nodes.put(nodeId, discoveryNode);
         }
 
-        return builder.build();
+        return nodes;
     }
 
-    private ClusterService generateMockedClusterService(ImmutableOpenMap<String, DiscoveryNode> discoveryNodeMap) {
+    private ClusterService generateMockedClusterService(Map<String, DiscoveryNode> discoveryNodeMap) {
         DiscoveryNodes discoveryNodes = mock(DiscoveryNodes.class);
         when(discoveryNodes.getDataNodes()).thenReturn(discoveryNodeMap);
         ClusterState clusterState = mock(ClusterState.class);

diff --git a/src/test/java/org/opensearch/knn/plugin/transport/TrainingModelRequestTests.java b/src/test/java/org/opensearch/knn/plugin/transport/TrainingModelRequestTests.java
@@ -20,7 +20,6 @@
 import org.opensearch.cluster.node.DiscoveryNodes;
 import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.common.ValidationException;
-import org.opensearch.common.collect.ImmutableOpenMap;
 import org.opensearch.common.io.stream.BytesStreamOutput;
 import org.opensearch.knn.KNNTestCase;
 import org.opensearch.knn.common.KNNConstants;
@@ -529,7 +528,7 @@ public void testValidation_invalid_preferredNodeDoesNotExist() {
 
         // Empty set of data nodes to produce exception
         DiscoveryNodes discoveryNodes = mock(DiscoveryNodes.class);
-        when(discoveryNodes.getDataNodes()).thenReturn(ImmutableOpenMap.of());
+        when(discoveryNodes.getDataNodes()).thenReturn(Map.of());
 
         ClusterState clusterState = mock(ClusterState.class);
         when(clusterState.metadata()).thenReturn(metadata);
@@ -679,7 +678,7 @@ public void testValidation_valid_trainingIndexBuiltFromModel() {
         Metadata metadata = mock(Metadata.class);
         when(metadata.index(trainingIndex)).thenReturn(indexMetadata);
         DiscoveryNodes discoveryNodes = mock(DiscoveryNodes.class);
-        when(discoveryNodes.getDataNodes()).thenReturn(ImmutableOpenMap.of());
+        when(discoveryNodes.getDataNodes()).thenReturn(Map.of());
 
         ClusterState clusterState = mock(ClusterState.class);
         when(clusterState.metadata()).thenReturn(metadata);

diff --git a/src/testFixtures/java/org/opensearch/knn/KNNRestTestCase.java b/src/testFixtures/java/org/opensearch/knn/KNNRestTestCase.java
@@ -122,7 +122,7 @@ public static void dumpCoverage() throws IOException, MalformedObjectNameExcepti
         // jacoco.dir is set in esplugin-coverage.gradle, if it doesn't exist we don't
         // want to collect coverage so we can return early
         String jacocoBuildPath = System.getProperty("jacoco.dir");
-        if (Strings.isNullOrEmpty(jacocoBuildPath)) {
+        if (StringUtils.isBlank(jacocoBuildPath)) {
             return;
         }
 
@@ -657,7 +657,7 @@ protected void createModelSystemIndex() throws IOException {
     }
 
     protected void addModelToSystemIndex(String modelId, ModelMetadata modelMetadata, byte[] model) throws IOException {
-        assertFalse(Strings.isNullOrEmpty(modelId));
+        assertFalse(StringUtils.isBlank(modelId));
         String modelBase64 = Base64.getEncoder().encodeToString(model);
 
         Request request = new Request("POST", "/" + MODEL_INDEX_NAME + "/_doc/" + modelId + "?refresh=true");