SOLR-14653: Replace usages of org.apache.hadoop.fs.Path that are used to construct Zookeeper paths

solr/core/src/java/org/apache/solr/cloud/ShardLeaderElectionContextBase.java

@@ -22,14 +22,13 @@ import java.lang.invoke.MethodHandles;
 import java.util.List;
 import java.util.ArrayList;
 
-// DO NOT SUBSTITUTE java nio Path here, see SOLR-13939
-import org.apache.hadoop.fs.Path;
 import org.apache.solr.cloud.overseer.OverseerAction;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.SolrException.ErrorCode;
 import org.apache.solr.common.cloud.Replica;
 import org.apache.solr.common.cloud.SolrZkClient;
 import org.apache.solr.common.cloud.ZkCmdExecutor;
+import org.apache.solr.common.cloud.ZkMaintenanceUtils;
 import org.apache.solr.common.cloud.ZkNodeProps;
 import org.apache.solr.common.cloud.ZkStateReader;
 import org.apache.solr.common.util.RetryUtil;
@@ -72,8 +71,7 @@ class ShardLeaderElectionContextBase extends ElectionContext {
     this.shardId = shardId;
     this.collection = collection;
 
-    // Fails on Windows if you use nio Paths.get here
-    String parent = new Path(leaderPath).getParent().toString();
+    String parent = ZkMaintenanceUtils.getZkParent(leaderPath);
     ZkCmdExecutor zcmd = new ZkCmdExecutor(30000);
     // only if /collections/{collection} exists already do we succeed in creating this path
     log.info("make sure parent is created {}", parent);
@@ -101,8 +99,8 @@ class ShardLeaderElectionContextBase extends ElectionContext {
           // version whenever a leader registers.
           log.debug("Removing leader registration node on cancel: {} {}", leaderPath, leaderZkNodeParentVersion);
           List<Op> ops = new ArrayList<>(2);
-          // Fails on Windows if you use nio Paths.get here
-          ops.add(Op.check(new Path(leaderPath).getParent().toString(), leaderZkNodeParentVersion));
+          String parent = ZkMaintenanceUtils.getZkParent(leaderPath);
+          ops.add(Op.check(parent, leaderZkNodeParentVersion));
           ops.add(Op.delete(leaderPath, -1));
           zkClient.multi(ops, true);
         } catch (InterruptedException e) {
@@ -122,8 +120,7 @@ class ShardLeaderElectionContextBase extends ElectionContext {
       throws KeeperException, InterruptedException, IOException {
     // register as leader - if an ephemeral is already there, wait to see if it goes away
 
-    // Fails on Windows if you use nio Paths.get here
-    String parent = new Path(leaderPath).getParent().toString();
+    String parent = ZkMaintenanceUtils.getZkParent(leaderPath);
     try {
       RetryUtil.retryOnThrowable(NodeExistsException.class, 60000, 5000, () -> {
         synchronized (lock) {

solr/core/src/java/org/apache/solr/handler/admin/ConfigSetsHandler.java

@@ -29,7 +29,6 @@ import java.util.zip.ZipInputStream;
 
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang3.StringUtils;
-import org.apache.hadoop.fs.Path;
 import org.apache.solr.api.Api;
 import org.apache.solr.client.solrj.SolrResponse;
 import org.apache.solr.cloud.OverseerSolrResponse;
@@ -153,7 +152,7 @@ public class ConfigSetsHandler extends RequestHandlerBase implements PermissionN
     }
 
     SolrZkClient zkClient = coreContainer.getZkController().getZkClient();
-    String configPathInZk = ZkConfigManager.CONFIGS_ZKNODE + Path.SEPARATOR + configSetName;
+    String configPathInZk = ZkConfigManager.CONFIGS_ZKNODE + "/" + configSetName;
 
     if (zkClient.exists(configPathInZk, true)) {
       throw new SolrException(ErrorCode.BAD_REQUEST,

solr/core/src/test/org/apache/solr/cloud/api/collections/TestLocalFSCloudBackupRestore.java

@@ -20,7 +20,6 @@ import java.io.IOException;
 import java.io.OutputStream;
 import java.net.URI;
 
-import org.apache.hadoop.fs.Path;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.store.IOContext;
 import org.apache.lucene.store.IndexInput;
@@ -63,7 +62,7 @@ public class TestLocalFSCloudBackupRestore extends AbstractCloudBackupRestoreTes
         .addConfig("confFaulty", TEST_PATH().resolve("configsets").resolve("cloud-minimal").resolve("conf"))
         .withSolrXml(solrXml)
         .configure();
-    cluster.getZkClient().delete(ZkConfigManager.CONFIGS_ZKNODE + Path.SEPARATOR + "confFaulty" + Path.SEPARATOR + "solrconfig.xml", -1, true);
+    cluster.getZkClient().delete(ZkConfigManager.CONFIGS_ZKNODE + "/" + "confFaulty" + "/" + "solrconfig.xml", -1, true);
 
     boolean whitespacesInPath = random().nextBoolean();
     if (whitespacesInPath) {

solr/solrj/src/java/org/apache/solr/common/cloud/ZkMaintenanceUtils.java

@@ -33,6 +33,7 @@ import java.util.TreeSet;
 import java.util.function.Predicate;
 import java.util.regex.Pattern;
 
+import org.apache.commons.lang.StringUtils;
 import org.apache.solr.client.solrj.SolrServerException;
 import org.apache.zookeeper.KeeperException;
 import org.apache.zookeeper.data.Stat;
@@ -48,6 +49,7 @@ public class ZkMaintenanceUtils {
   private static final String ZKNODE_DATA_FILE = "zknode.data";
 
   private ZkMaintenanceUtils() {} // don't let it be instantiated, all methods are static.
+
   /**
    * Lists a ZNode child and (optionally) the znodes of all the children. No data is dumped.
    *
@@ -69,7 +71,7 @@ public class ZkMaintenanceUtils {
     }
 
     StringBuilder sb = new StringBuilder();
-    
+
     if (recurse == false) {
       for (String node : zkClient.getChildren(root, null, true)) {
         if (node.equals("zookeeper") == false) {
@@ -78,7 +80,7 @@ public class ZkMaintenanceUtils {
       }
       return sb.toString();
     }
-    
+
     traverseZkTree(zkClient, root, VISIT_ORDER.VISIT_PRE, znode -> {
       if (znode.startsWith("/zookeeper")) return; // can't do anything with this node!
       int iPos = znode.lastIndexOf("/");
@@ -105,8 +107,8 @@ public class ZkMaintenanceUtils {
    * @throws InterruptedException Thread interrupted
    */
   public static void zkTransfer(SolrZkClient zkClient, String src, Boolean srcIsZk,
-                         String dst, Boolean dstIsZk, 
-                         Boolean recurse) throws SolrServerException, KeeperException, InterruptedException, IOException {
+                                String dst, Boolean dstIsZk,
+                                Boolean recurse) throws SolrServerException, KeeperException, InterruptedException, IOException {
 
     if (srcIsZk == false && dstIsZk == false) {
       throw new SolrServerException("One or both of source or destination must specify ZK nodes.");
@@ -274,7 +276,7 @@ public class ZkMaintenanceUtils {
       }
     }
   }
-  
+
   public static void uploadToZK(SolrZkClient zkClient, final Path fromPath, final String zkPath,
                                 final Pattern filenameExclusions) throws IOException {
 
@@ -284,7 +286,7 @@ public class ZkMaintenanceUtils {
     }
 
     final Path rootPath = Paths.get(path);
-        
+
     if (!Files.exists(rootPath))
       throw new IOException("Path " + rootPath + " does not exist");
 
@@ -418,7 +420,24 @@ public class ZkMaintenanceUtils {
     }
   }
 
+  // Get the parent path. This is really just the string before the last slash (/)
+  // Will return empty string if there are no slashes.
+  // Will return empty string if the path is just "/"
+  // Will return empty string if the path is just ""
+  public static String getZkParent(String path) {
+    // Remove trailing slash if present.
+    if (StringUtils.endsWith(path, "/")) {
+      path = StringUtils.substringBeforeLast(path, "/");
+    }
+    if (StringUtils.contains(path, "/") == false) {
+      return "";
+    }
+    return (StringUtils.substringBeforeLast(path, "/"));
+  }
+
   // Take into account Windows file separators when making a Znode's name.
+  // Used particularly when uploading configsets since the path we're copying
+  // up may be a file path.
   public static String createZkNodeName(String zkRoot, Path root, Path file) {
     String relativePath = root.relativize(file).toString();
     // Windows shenanigans

solr/solrj/src/test/org/apache/solr/common/util/TestZkMaintenanceUtils.java

@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.common.util;
+
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.cloud.ZkMaintenanceUtils;
+import org.junit.Test;
+
+public class TestZkMaintenanceUtils extends SolrTestCaseJ4 {
+  @Test
+  public void testPaths() {
+    assertEquals("Unexpected path construction"
+        , ""
+        , ZkMaintenanceUtils.getZkParent(null));
+
+    assertEquals("Unexpected path construction"
+        , "this/is/a"
+        , ZkMaintenanceUtils.getZkParent("this/is/a/path"));
+
+    assertEquals("Unexpected path construction"
+        , "/root"
+        , ZkMaintenanceUtils.getZkParent("/root/path/"));
+
+    assertEquals("Unexpected path construction"
+        , ""
+        , ZkMaintenanceUtils.getZkParent("/"));
+
+    assertEquals("Unexpected path construction"
+        , ""
+        , ZkMaintenanceUtils.getZkParent(""));
+
+    assertEquals("Unexpected path construction"
+        , ""
+        , ZkMaintenanceUtils.getZkParent("noslashesinstring"));
+
+    assertEquals("Unexpected path construction"
+        , ""
+        , ZkMaintenanceUtils.getZkParent("/leadingslashonly"));
+
+  }
+}