Extract duplicate code and move it to CollectionHandlingUtils (#2044)

Co-authored-by: Renato Haeberli
apache · Nov 27, 2023 · f78f865 · f78f865
1 parent c1cfaec
commit f78f865
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 32 deletions.
diff --git a/solr/core/src/java/org/apache/solr/cloud/OverseerConfigSetMessageHandler.java b/solr/core/src/java/org/apache/solr/cloud/OverseerConfigSetMessageHandler.java
@@ -16,6 +16,7 @@
  */
 package org.apache.solr.cloud;
 
+import static org.apache.solr.cloud.api.collections.CollectionHandlingUtils.addExceptionToNamedList;
 import static org.apache.solr.common.params.CommonParams.NAME;
 
 import java.lang.invoke.MethodHandles;
@@ -27,7 +28,6 @@
 import org.apache.solr.common.cloud.ZkStateReader;
 import org.apache.solr.common.params.ConfigSetParams;
 import org.apache.solr.common.util.NamedList;
-import org.apache.solr.common.util.SimpleOrderedMap;
 import org.apache.solr.core.CoreContainer;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -97,12 +97,7 @@ public OverseerSolrResponse processMessage(ZkNodeProps message, String operation
       } else {
         log.error("ConfigSet: {} operation: {} failed", configSetName, operation, e);
       }
-
-      results.add("Operation " + operation + " caused exception:", e);
-      SimpleOrderedMap<Object> nl = new SimpleOrderedMap<>();
-      nl.add("msg", e.getMessage());
-      nl.add("rspCode", e instanceof SolrException ? ((SolrException) e).code() : -1);
-      results.add("exception", nl);
+      addExceptionToNamedList(operation, e, results);
     }
     return new OverseerSolrResponse(results);
   }

diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/CollectionHandlingUtils.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/CollectionHandlingUtils.java
@@ -450,6 +450,26 @@ static void processResponse(
     }
   }
 
+  static void logFailedOperation(final Object operation, final Exception e, final String collName) {
+    if (collName == null) {
+      log.error("Operation {} failed", operation, e);
+    } else {
+      log.error("Collection {}, operation {} failed", collName, operation, e);
+    }
+  }
+
+  /***
+   * Creates a SimpleOrderedMap with the exception details and adds it to the results
+   */
+  public static void addExceptionToNamedList(
+      final Object operation, final Exception e, final NamedList<Object> results) {
+    results.add("Operation " + operation + " caused exception:", e);
+    SimpleOrderedMap<Object> nl = new SimpleOrderedMap<>();
+    nl.add("msg", e.getMessage());
+    nl.add("rspCode", e instanceof SolrException ? ((SolrException) e).code() : -1);
+    results.add("exception", nl);
+  }
+
   private static void addFailure(NamedList<Object> results, String key, Object value) {
     @SuppressWarnings("unchecked")
     SimpleOrderedMap<Object> failure = (SimpleOrderedMap<Object>) results.get("failure");

diff --git a/...va/org/apache/solr/cloud/api/collections/DistributedCollectionConfigSetCommandRunner.java b/...va/org/apache/solr/cloud/api/collections/DistributedCollectionConfigSetCommandRunner.java
@@ -17,6 +17,8 @@
 
 package org.apache.solr.cloud.api.collections;
 
+import static org.apache.solr.cloud.api.collections.CollectionHandlingUtils.addExceptionToNamedList;
+import static org.apache.solr.cloud.api.collections.CollectionHandlingUtils.logFailedOperation;
 import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP;
 import static org.apache.solr.common.cloud.ZkStateReader.REPLICA_PROP;
 import static org.apache.solr.common.cloud.ZkStateReader.SHARD_ID_PROP;
@@ -49,7 +51,6 @@
 import org.apache.solr.common.util.ExecutorUtil;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.common.util.Pair;
-import org.apache.solr.common.util.SimpleOrderedMap;
 import org.apache.solr.common.util.SolrNamedThreadFactory;
 import org.apache.solr.core.CoreContainer;
 import org.apache.solr.logging.MDCLoggingContext;
@@ -452,18 +453,8 @@ public OverseerSolrResponse call() {
         if (e instanceof InterruptedException) {
           Thread.currentThread().interrupt();
         }
-        // Output some error logs
-        if (collName == null) {
-          log.error("Operation {} failed", action, e);
-        } else {
-          log.error("Collection {}}, operation {} failed", collName, action, e);
-        }
-
-        results.add("Operation " + action + " caused exception:", e);
-        SimpleOrderedMap<Object> nl = new SimpleOrderedMap<>();
-        nl.add("msg", e.getMessage());
-        nl.add("rspCode", e instanceof SolrException ? ((SolrException) e).code() : -1);
-        results.add("exception", nl);
+        logFailedOperation(action, e, collName);
+        addExceptionToNamedList(action, e, results);
       }
 
       OverseerSolrResponse res = new OverseerSolrResponse(results);

diff --git a/...core/src/java/org/apache/solr/cloud/api/collections/OverseerCollectionMessageHandler.java b/...core/src/java/org/apache/solr/cloud/api/collections/OverseerCollectionMessageHandler.java
@@ -16,6 +16,8 @@
  */
 package org.apache.solr.cloud.api.collections;
 
+import static org.apache.solr.cloud.api.collections.CollectionHandlingUtils.addExceptionToNamedList;
+import static org.apache.solr.cloud.api.collections.CollectionHandlingUtils.logFailedOperation;
 import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP;
 import static org.apache.solr.common.cloud.ZkStateReader.REPLICA_PROP;
 import static org.apache.solr.common.cloud.ZkStateReader.SHARD_ID_PROP;
@@ -42,7 +44,6 @@
 import org.apache.solr.common.params.CollectionParams.CollectionAction;
 import org.apache.solr.common.util.ExecutorUtil;
 import org.apache.solr.common.util.NamedList;
-import org.apache.solr.common.util.SimpleOrderedMap;
 import org.apache.solr.common.util.SolrNamedThreadFactory;
 import org.apache.solr.common.util.TimeSource;
 import org.apache.solr.handler.component.HttpShardHandlerFactory;
@@ -134,17 +135,8 @@ public OverseerSolrResponse processMessage(ZkNodeProps message, String operation
       String collName = message.getStr("collection");
       if (collName == null) collName = message.getStr(NAME);
 
-      if (collName == null) {
-        log.error("Operation {} failed", operation, e);
-      } else {
-        log.error("Collection {}}, operation {} failed", collName, operation, e);
-      }
-
-      results.add("Operation " + operation + " caused exception:", e);
-      SimpleOrderedMap<Object> nl = new SimpleOrderedMap<>();
-      nl.add("msg", e.getMessage());
-      nl.add("rspCode", e instanceof SolrException ? ((SolrException) e).code() : -1);
-      results.add("exception", nl);
+      logFailedOperation(operation, e, collName);
+      addExceptionToNamedList(operation, e, results);
     }
     return new OverseerSolrResponse(results);
   }