lucene-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From uschind...@apache.org
Subject [1/2] lucene-solr:master: [SOLR-9444] Fix path usage for cloud backup/restore
Date Fri, 02 Sep 2016 12:15:32 GMT
Repository: lucene-solr
Updated Branches:
  refs/heads/master e203c9af9 -> d9c0f2c6b


[SOLR-9444] Fix path usage for cloud backup/restore

- Refactored code using URI.getPath() API to use URI instance
  uniformly.
- During serialization of URI instance, used toASCIIString() API
  to generate the appropriate String representation.
- Updated the unit test to use whitespaces in the backup directory path


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/e138462a
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/e138462a
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/e138462a

Branch: refs/heads/master
Commit: e138462a82800be3811017062868051c14e560e6
Parents: e13f7ae
Author: Hrishikesh Gadre <hgadre@cloudera.com>
Authored: Sun Aug 28 14:48:24 2016 -0700
Committer: Hrishikesh Gadre <hgadre@cloudera.com>
Committed: Thu Sep 1 16:28:19 2016 -0700

----------------------------------------------------------------------
 .../java/org/apache/solr/cloud/BackupCmd.java   |  6 ++--
 .../java/org/apache/solr/cloud/RestoreCmd.java  |  6 ++--
 .../apache/solr/core/backup/BackupManager.java  | 34 +++++++++---------
 .../backup/repository/BackupRepository.java     | 14 ++++++--
 .../backup/repository/HdfsBackupRepository.java | 29 +++++++++++++---
 .../repository/LocalFileSystemRepository.java   | 36 ++++++++++++--------
 .../apache/solr/handler/ReplicationHandler.java | 12 ++++---
 .../org/apache/solr/handler/RestoreCore.java    |  6 ++--
 .../org/apache/solr/handler/SnapShooter.java    | 11 +++---
 .../solr/handler/admin/CoreAdminOperation.java  |  7 ++--
 .../cloud/TestLocalFSCloudBackupRestore.java    | 10 +++++-
 11 files changed, 112 insertions(+), 59 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e138462a/solr/core/src/java/org/apache/solr/cloud/BackupCmd.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/BackupCmd.java b/solr/core/src/java/org/apache/solr/cloud/BackupCmd.java
index 679cb07..648eee8 100644
--- a/solr/core/src/java/org/apache/solr/cloud/BackupCmd.java
+++ b/solr/core/src/java/org/apache/solr/cloud/BackupCmd.java
@@ -62,7 +62,6 @@ public class BackupCmd implements OverseerCollectionMessageHandler.Cmd {
     ShardHandler shardHandler = ocmh.shardHandlerFactory.getShardHandler();
     String asyncId = message.getStr(ASYNC);
     String repo = message.getStr(CoreAdminParams.BACKUP_REPOSITORY);
-    String location = message.getStr(CoreAdminParams.BACKUP_LOCATION);
 
     Map<String, String> requestMap = new HashMap<>();
     Instant startTime = Instant.now();
@@ -72,7 +71,8 @@ public class BackupCmd implements OverseerCollectionMessageHandler.Cmd {
     BackupManager backupMgr = new BackupManager(repository, ocmh.zkStateReader, collectionName);
 
     // Backup location
-    URI backupPath = repository.createURI(location, backupName);
+    URI location = repository.createURI(message.getStr(CoreAdminParams.BACKUP_LOCATION));
+    URI backupPath = repository.resolve(location, backupName);
 
     //Validating if the directory already exists.
     if (repository.exists(backupPath)) {
@@ -94,7 +94,7 @@ public class BackupCmd implements OverseerCollectionMessageHandler.Cmd {
       params.set(CoreAdminParams.ACTION, CoreAdminParams.CoreAdminAction.BACKUPCORE.toString());
       params.set(NAME, slice.getName());
       params.set(CoreAdminParams.BACKUP_REPOSITORY, repo);
-      params.set(CoreAdminParams.BACKUP_LOCATION, backupPath.getPath()); // note: index dir
will be here then the "snapshot." + slice name
+      params.set(CoreAdminParams.BACKUP_LOCATION, backupPath.toASCIIString()); // note: index
dir will be here then the "snapshot." + slice name
       params.set(CORE_NAME_PROP, coreName);
 
       ocmh.sendShardRequest(replica.getNodeName(), params, shardHandler, asyncId, requestMap);

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e138462a/solr/core/src/java/org/apache/solr/cloud/RestoreCmd.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/RestoreCmd.java b/solr/core/src/java/org/apache/solr/cloud/RestoreCmd.java
index af2215c..63d5686 100644
--- a/solr/core/src/java/org/apache/solr/cloud/RestoreCmd.java
+++ b/solr/core/src/java/org/apache/solr/cloud/RestoreCmd.java
@@ -79,13 +79,13 @@ public class RestoreCmd implements OverseerCollectionMessageHandler.Cmd
{
     ShardHandler shardHandler = ocmh.shardHandlerFactory.getShardHandler();
     String asyncId = message.getStr(ASYNC);
     String repo = message.getStr(CoreAdminParams.BACKUP_REPOSITORY);
-    String location = message.getStr(CoreAdminParams.BACKUP_LOCATION);
     Map<String, String> requestMap = new HashMap<>();
 
     CoreContainer cc = ocmh.overseer.getZkController().getCoreContainer();
     BackupRepository repository = cc.newBackupRepository(Optional.ofNullable(repo));
 
-    URI backupPath = repository.createURI(location, backupName);
+    URI location = repository.createURI(message.getStr(CoreAdminParams.BACKUP_LOCATION));
+    URI backupPath = repository.resolve(location, backupName);
     ZkStateReader zkStateReader = ocmh.zkStateReader;
     BackupManager backupMgr = new BackupManager(repository, zkStateReader, restoreCollectionName);
 
@@ -195,7 +195,7 @@ public class RestoreCmd implements OverseerCollectionMessageHandler.Cmd
{
       ModifiableSolrParams params = new ModifiableSolrParams();
       params.set(CoreAdminParams.ACTION, CoreAdminParams.CoreAdminAction.RESTORECORE.toString());
       params.set(NAME, "snapshot." + slice.getName());
-      params.set(CoreAdminParams.BACKUP_LOCATION, backupPath.getPath());
+      params.set(CoreAdminParams.BACKUP_LOCATION, backupPath.toASCIIString());
       params.set(CoreAdminParams.BACKUP_REPOSITORY, repo);
 
       ocmh.sliceCmd(clusterState, params, null, slice, shardHandler, asyncId, requestMap);

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e138462a/solr/core/src/java/org/apache/solr/core/backup/BackupManager.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/core/backup/BackupManager.java b/solr/core/src/java/org/apache/solr/core/backup/BackupManager.java
index 51227e8..e650553 100644
--- a/solr/core/src/java/org/apache/solr/core/backup/BackupManager.java
+++ b/solr/core/src/java/org/apache/solr/core/backup/BackupManager.java
@@ -87,12 +87,12 @@ public class BackupManager {
    * @return the configuration parameters for the specified backup.
    * @throws IOException In case of errors.
    */
-  public Properties readBackupProperties(String backupLoc, String backupId) throws IOException
{
+  public Properties readBackupProperties(URI backupLoc, String backupId) throws IOException
{
     Preconditions.checkNotNull(backupLoc);
     Preconditions.checkNotNull(backupId);
 
     // Backup location
-    URI backupPath = repository.createURI(backupLoc, backupId);
+    URI backupPath = repository.resolve(backupLoc, backupId);
     if (!repository.exists(backupPath)) {
       throw new SolrException(ErrorCode.SERVER_ERROR, "Couldn't restore since doesn't exist:
" + backupPath);
     }
@@ -113,8 +113,8 @@ public class BackupManager {
    * @param props The backup properties
    * @throws IOException in case of I/O error
    */
-  public void writeBackupProperties(String backupLoc, String backupId, Properties props)
throws IOException {
-    URI dest = repository.createURI(backupLoc, backupId, BACKUP_PROPS_FILE);
+  public void writeBackupProperties(URI backupLoc, String backupId, Properties props) throws
IOException {
+    URI dest = repository.resolve(backupLoc, backupId, BACKUP_PROPS_FILE);
     try (Writer propsWriter = new OutputStreamWriter(repository.createOutput(dest), StandardCharsets.UTF_8))
{
       props.store(propsWriter, "Backup properties file");
     }
@@ -128,10 +128,10 @@ public class BackupManager {
    * @return the meta-data information for the backed-up collection.
    * @throws IOException in case of errors.
    */
-  public DocCollection readCollectionState(String backupLoc, String backupId, String collectionName)
throws IOException {
+  public DocCollection readCollectionState(URI backupLoc, String backupId, String collectionName)
throws IOException {
     Preconditions.checkNotNull(collectionName);
 
-    URI zkStateDir = repository.createURI(backupLoc, backupId, ZK_STATE_DIR);
+    URI zkStateDir = repository.resolve(backupLoc, backupId, ZK_STATE_DIR);
     try (IndexInput is = repository.openInput(zkStateDir, COLLECTION_PROPS_FILE, IOContext.DEFAULT))
{
       byte[] arr = new byte[(int) is.length()]; // probably ok since the json file should
be small.
       is.readBytes(arr, 0, (int) is.length());
@@ -149,9 +149,9 @@ public class BackupManager {
    * @param collectionState The collection meta-data to be stored.
    * @throws IOException in case of I/O errors.
    */
-  public void writeCollectionState(String backupLoc, String backupId, String collectionName,
+  public void writeCollectionState(URI backupLoc, String backupId, String collectionName,
                                    DocCollection collectionState) throws IOException {
-    URI dest = repository.createURI(backupLoc, backupId, ZK_STATE_DIR, COLLECTION_PROPS_FILE);
+    URI dest = repository.resolve(backupLoc, backupId, ZK_STATE_DIR, COLLECTION_PROPS_FILE);
     try (OutputStream collectionStateOs = repository.createOutput(dest)) {
       collectionStateOs.write(Utils.toJSON(Collections.singletonMap(collectionName, collectionState)));
     }
@@ -166,9 +166,9 @@ public class BackupManager {
    * @param targetConfigName  The name of the config to be created.
    * @throws IOException in case of I/O errors.
    */
-  public void uploadConfigDir(String backupLoc, String backupId, String sourceConfigName,
String targetConfigName)
+  public void uploadConfigDir(URI backupLoc, String backupId, String sourceConfigName, String
targetConfigName)
       throws IOException {
-    URI source = repository.createURI(backupLoc, backupId, ZK_STATE_DIR, CONFIG_STATE_DIR,
sourceConfigName);
+    URI source = repository.resolve(backupLoc, backupId, ZK_STATE_DIR, CONFIG_STATE_DIR,
sourceConfigName);
     String zkPath = ZkConfigManager.CONFIGS_ZKNODE + "/" + targetConfigName;
     uploadToZk(zkStateReader.getZkClient(), source, zkPath);
   }
@@ -181,10 +181,10 @@ public class BackupManager {
    * @param configName The name of the config to be saved.
    * @throws IOException in case of I/O errors.
    */
-  public void downloadConfigDir(String backupLoc, String backupId, String configName) throws
IOException {
-    URI dest = repository.createURI(backupLoc, backupId, ZK_STATE_DIR, CONFIG_STATE_DIR,
configName);
-    repository.createDirectory(repository.createURI(backupLoc, backupId, ZK_STATE_DIR));
-    repository.createDirectory(repository.createURI(backupLoc, backupId, ZK_STATE_DIR, CONFIG_STATE_DIR));
+  public void downloadConfigDir(URI backupLoc, String backupId, String configName) throws
IOException {
+    URI dest = repository.resolve(backupLoc, backupId, ZK_STATE_DIR, CONFIG_STATE_DIR, configName);
+    repository.createDirectory(repository.resolve(backupLoc, backupId, ZK_STATE_DIR));
+    repository.createDirectory(repository.resolve(backupLoc, backupId, ZK_STATE_DIR, CONFIG_STATE_DIR));
     repository.createDirectory(dest);
 
     downloadFromZK(zkStateReader.getZkClient(), ZkConfigManager.CONFIGS_ZKNODE + "/" + configName,
dest);
@@ -201,11 +201,11 @@ public class BackupManager {
         if (children.size() == 0) {
           log.info("Writing file {}", file);
           byte[] data = zkClient.getData(zkPath + "/" + file, null, null, true);
-          try (OutputStream os = repository.createOutput(repository.createURI(dir.getPath(),
file))) {
+          try (OutputStream os = repository.createOutput(repository.resolve(dir, file)))
{
             os.write(data);
           }
         } else {
-          downloadFromZK(zkClient, zkPath + "/" + file, repository.createURI(dir.getPath(),
file));
+          downloadFromZK(zkClient, zkPath + "/" + file, repository.resolve(dir, file));
         }
       }
     } catch (KeeperException | InterruptedException e) {
@@ -221,7 +221,7 @@ public class BackupManager {
 
     for (String file : repository.listAll(sourceDir)) {
       String zkNodePath = destZkPath + "/" + file;
-      URI path = repository.createURI(sourceDir.getPath(), file);
+      URI path = repository.resolve(sourceDir, file);
       PathType t = repository.getPathType(path);
       switch (t) {
         case FILE: {

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e138462a/solr/core/src/java/org/apache/solr/core/backup/repository/BackupRepository.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/core/backup/repository/BackupRepository.java
b/solr/core/src/java/org/apache/solr/core/backup/repository/BackupRepository.java
index 8950ce7..875be18 100644
--- a/solr/core/src/java/org/apache/solr/core/backup/repository/BackupRepository.java
+++ b/solr/core/src/java/org/apache/solr/core/backup/repository/BackupRepository.java
@@ -57,13 +57,23 @@ public interface BackupRepository extends NamedListInitializedPlugin,
Closeable
   <T> T getConfigProperty(String name);
 
   /**
-   * This method creates a URI using the specified path components (as method arguments).
+   * This method returns the URI representation for the specified path.
+   * Note - the specified path could be a fully qualified URI OR a relative path for a file-system.
    *
+   * @param path The path specified by the user.
+   * @return the URI representation of the user supplied value
+   */
+   URI createURI(String path);
+
+  /**
+   * This method resolves a URI using the specified path components (as method arguments).
+   *
+   * @param baseUri The base URI to use for creating the path
    * @param pathComponents
    *          The directory (or file-name) to be included in the URI.
    * @return A URI containing absolute path
    */
-  URI createURI(String... pathComponents);
+  URI resolve(URI baseUri, String... pathComponents);
 
   /**
    * This method checks if the specified path exists in this repository.

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e138462a/solr/core/src/java/org/apache/solr/core/backup/repository/HdfsBackupRepository.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/core/backup/repository/HdfsBackupRepository.java
b/solr/core/src/java/org/apache/solr/core/backup/repository/HdfsBackupRepository.java
index bb148de..f12d9fd 100644
--- a/solr/core/src/java/org/apache/solr/core/backup/repository/HdfsBackupRepository.java
+++ b/solr/core/src/java/org/apache/solr/core/backup/repository/HdfsBackupRepository.java
@@ -20,6 +20,7 @@ package org.apache.solr.core.backup.repository;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.net.URI;
+import java.net.URISyntaxException;
 
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.FileStatus;
@@ -88,11 +89,31 @@ public class HdfsBackupRepository implements BackupRepository {
   }
 
   @Override
-  public URI createURI(String... pathComponents) {
-    Path result = baseHdfsPath;
-    for (String p : pathComponents) {
-      result = new Path(result, p);
+  public URI createURI(String location) {
+    Preconditions.checkNotNull(location);
+
+    URI result = null;
+    try {
+      result = new URI(location);
+      if (!result.isAbsolute()) {
+        result = resolve(this.baseHdfsPath.toUri(), location);
+      }
+    } catch (URISyntaxException ex) {
+      result = resolve(this.baseHdfsPath.toUri(), location);
+    }
+
+    return result;
+  }
+
+  @Override
+  public URI resolve(URI baseUri, String... pathComponents) {
+    Preconditions.checkArgument(baseUri.isAbsolute());
+
+    Path result = new Path(baseUri);
+    for (String path : pathComponents) {
+      result = new Path(result, path);
     }
+
     return result.toUri();
   }
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e138462a/solr/core/src/java/org/apache/solr/core/backup/repository/LocalFileSystemRepository.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/core/backup/repository/LocalFileSystemRepository.java
b/solr/core/src/java/org/apache/solr/core/backup/repository/LocalFileSystemRepository.java
index 86c4110..4ac2558 100644
--- a/solr/core/src/java/org/apache/solr/core/backup/repository/LocalFileSystemRepository.java
+++ b/solr/core/src/java/org/apache/solr/core/backup/repository/LocalFileSystemRepository.java
@@ -20,19 +20,20 @@ package org.apache.solr.core.backup.repository;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.net.URI;
+import java.net.URISyntaxException;
 import java.nio.file.FileVisitResult;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.nio.file.SimpleFileVisitor;
 import java.nio.file.attribute.BasicFileAttributes;
+
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.store.FSDirectory;
 import org.apache.lucene.store.IOContext;
 import org.apache.lucene.store.IndexInput;
 import org.apache.lucene.store.NoLockFactory;
 import org.apache.lucene.store.SimpleFSDirectory;
-import org.apache.lucene.util.Constants;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.core.DirectoryFactory;
 
@@ -58,21 +59,28 @@ public class LocalFileSystemRepository implements BackupRepository {
   }
 
   @Override
-  public URI createURI(String... pathComponents) {
-    Preconditions.checkArgument(pathComponents.length > 0);
-
-    String basePath = Preconditions.checkNotNull(pathComponents[0]);
-    // Note the URI.getPath() invocation on Windows platform generates an invalid URI.
-    // Refer to http://stackoverflow.com/questions/9834776/java-nio-file-path-issue
-    // Since the caller may have used this method to generate the string representation
-    // for the pathComponents, we implement a work-around specifically for Windows platform
-    // to remove the leading '/' character.
-    if (Constants.WINDOWS) {
-      basePath = basePath.replaceFirst("^/(.:/)", "$1");
+  public URI createURI(String location) {
+    Preconditions.checkNotNull(location);
+
+    URI result = null;
+    try {
+      result = new URI(location);
+      if (!result.isAbsolute()) {
+        result = Paths.get(location).toUri();
+      }
+    } catch (URISyntaxException ex) {
+      result = Paths.get(location).toUri();
     }
 
-    Path result = Paths.get(basePath);
-    for (int i = 1; i < pathComponents.length; i++) {
+    return result;
+  }
+
+  @Override
+  public URI resolve(URI baseUri, String... pathComponents) {
+    Preconditions.checkArgument(pathComponents.length > 0);
+
+    Path result = Paths.get(baseUri);
+    for (int i = 0; i < pathComponents.length; i++) {
       result = result.resolve(pathComponents[i]);
     }
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e138462a/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java b/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
index aee3b97..84e1ba2 100644
--- a/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
@@ -443,14 +443,15 @@ public class ReplicationHandler extends RequestHandlerBase implements
SolrCoreAw
       location = core.getDataDir();
     }
 
+    URI locationUri = repo.createURI(location);
+
     //If name is not provided then look for the last unnamed( the ones with the snapshot.timestamp
format)
     //snapshot folder since we allow snapshots to be taken without providing a name. Pick
the latest timestamp.
     if (name == null) {
-      URI basePath = repo.createURI(location);
-      String[] filePaths = repo.listAll(basePath);
+      String[] filePaths = repo.listAll(locationUri);
       List<OldBackupDirectory> dirs = new ArrayList<>();
       for (String f : filePaths) {
-        OldBackupDirectory obd = new OldBackupDirectory(basePath, f);
+        OldBackupDirectory obd = new OldBackupDirectory(locationUri, f);
         if (obd.getTimestamp().isPresent()) {
           dirs.add(obd);
         }
@@ -465,7 +466,7 @@ public class ReplicationHandler extends RequestHandlerBase implements
SolrCoreAw
       name = "snapshot." + name;
     }
 
-    RestoreCore restoreCore = new RestoreCore(repo, core, location, name);
+    RestoreCore restoreCore = new RestoreCore(repo, core, locationUri, name);
     try {
       MDC.put("RestoreCore.core", core.getName());
       MDC.put("RestoreCore.backupLocation", location);
@@ -561,7 +562,8 @@ public class ReplicationHandler extends RequestHandlerBase implements
SolrCoreAw
       }
 
       // small race here before the commit point is saved
-      SnapShooter snapShooter = new SnapShooter(repo, core, location, params.get(NAME), commitName);
+      URI locationUri = repo.createURI(location);
+      SnapShooter snapShooter = new SnapShooter(repo, core, locationUri, params.get(NAME),
commitName);
       snapShooter.validateCreateSnapshot();
       snapShooter.createSnapAsync(indexCommit, numberToKeep, (nl) -> snapShootDetails
= nl);
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e138462a/solr/core/src/java/org/apache/solr/handler/RestoreCore.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/handler/RestoreCore.java b/solr/core/src/java/org/apache/solr/handler/RestoreCore.java
index 6aef35c..62cb93f 100644
--- a/solr/core/src/java/org/apache/solr/handler/RestoreCore.java
+++ b/solr/core/src/java/org/apache/solr/handler/RestoreCore.java
@@ -44,11 +44,11 @@ public class RestoreCore implements Callable<Boolean> {
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
   private final String backupName;
-  private final String backupLocation;
+  private final URI backupLocation;
   private final SolrCore core;
   private final BackupRepository backupRepo;
 
-  public RestoreCore(BackupRepository backupRepo, SolrCore core, String location, String
name) {
+  public RestoreCore(BackupRepository backupRepo, SolrCore core, URI location, String name)
{
     this.backupRepo = backupRepo;
     this.core = core;
     this.backupLocation = location;
@@ -62,7 +62,7 @@ public class RestoreCore implements Callable<Boolean> {
 
   public boolean doRestore() throws Exception {
 
-    URI backupPath = backupRepo.createURI(backupLocation, backupName);
+    URI backupPath = backupRepo.resolve(backupLocation, backupName);
     SimpleDateFormat dateFormat = new SimpleDateFormat(SnapShooter.DATE_FMT, Locale.ROOT);
     String restoreIndexName = "restore." + dateFormat.format(new Date());
     String restoreIndexPath = core.getDataDir() + restoreIndexName;

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e138462a/solr/core/src/java/org/apache/solr/handler/SnapShooter.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/handler/SnapShooter.java b/solr/core/src/java/org/apache/solr/handler/SnapShooter.java
index e12649d..52f4889 100644
--- a/solr/core/src/java/org/apache/solr/handler/SnapShooter.java
+++ b/solr/core/src/java/org/apache/solr/handler/SnapShooter.java
@@ -19,6 +19,7 @@ package org.apache.solr.handler;
 import java.io.IOException;
 import java.lang.invoke.MethodHandles;
 import java.net.URI;
+import java.nio.file.Paths;
 import java.text.SimpleDateFormat;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -75,17 +76,17 @@ public class SnapShooter {
     } else {
       snapDirStr = core.getCoreDescriptor().getInstanceDir().resolve(location).normalize().toString();
     }
-    initialize(new LocalFileSystemRepository(), core, snapDirStr, snapshotName, null);
+    initialize(new LocalFileSystemRepository(), core, Paths.get(snapDirStr).toUri(), snapshotName,
null);
   }
 
-  public SnapShooter(BackupRepository backupRepo, SolrCore core, String location, String
snapshotName, String commitName) {
+  public SnapShooter(BackupRepository backupRepo, SolrCore core, URI location, String snapshotName,
String commitName) {
     initialize(backupRepo, core, location, snapshotName, commitName);
   }
 
-  private void initialize(BackupRepository backupRepo, SolrCore core, String location, String
snapshotName, String commitName) {
+  private void initialize(BackupRepository backupRepo, SolrCore core, URI location, String
snapshotName, String commitName) {
     this.solrCore = Preconditions.checkNotNull(core);
     this.backupRepo = Preconditions.checkNotNull(backupRepo);
-    this.baseSnapDirPath = backupRepo.createURI(Preconditions.checkNotNull(location)).normalize();
+    this.baseSnapDirPath = location;
     this.snapshotName = snapshotName;
     if (snapshotName != null) {
       directoryName = "snapshot." + snapshotName;
@@ -93,7 +94,7 @@ public class SnapShooter {
       SimpleDateFormat fmt = new SimpleDateFormat(DATE_FMT, Locale.ROOT);
       directoryName = "snapshot." + fmt.format(new Date());
     }
-    this.snapshotDirPath = backupRepo.createURI(location, directoryName);
+    this.snapshotDirPath = backupRepo.resolve(location, directoryName);
     this.commitName = commitName;
   }
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e138462a/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java b/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java
index e4103c5..dfc7a6f 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java
@@ -18,6 +18,7 @@ package org.apache.solr.handler.admin;
 
 import java.io.IOException;
 import java.lang.invoke.MethodHandles;
+import java.net.URI;
 import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -803,8 +804,9 @@ enum CoreAdminOperation implements CoreAdminOp {
       // parameter is not supplied, the latest index commit is backed-up.
       String commitName = params.get(CoreAdminParams.COMMIT_NAME);
 
+      URI locationUri = repository.createURI(location);
       try (SolrCore core = it.handler.coreContainer.getCore(cname)) {
-        SnapShooter snapShooter = new SnapShooter(repository, core, location, name, commitName);
+        SnapShooter snapShooter = new SnapShooter(repository, core, locationUri, name, commitName);
         // validateCreateSnapshot will create parent dirs instead of throw; that choice is
dubious.
         //  But we want to throw. One reason is that
         //  this dir really should, in fact must, already exist here if triggered via a collection
backup on a shared
@@ -847,8 +849,9 @@ enum CoreAdminOperation implements CoreAdminOp {
           + " parameter or as a default repository property");
     }
 
+    URI locationUri = repository.createURI(location);
     try (SolrCore core = it.handler.coreContainer.getCore(cname)) {
-      RestoreCore restoreCore = new RestoreCore(repository, core, location, name);
+      RestoreCore restoreCore = new RestoreCore(repository, core, locationUri, name);
       boolean success = restoreCore.doRestore();
       if (!success) {
         throw new SolrException(ErrorCode.SERVER_ERROR, "Failed to restore core=" + core.getName());

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e138462a/solr/core/src/test/org/apache/solr/cloud/TestLocalFSCloudBackupRestore.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/TestLocalFSCloudBackupRestore.java b/solr/core/src/test/org/apache/solr/cloud/TestLocalFSCloudBackupRestore.java
index db68913..da8e767 100644
--- a/solr/core/src/test/org/apache/solr/cloud/TestLocalFSCloudBackupRestore.java
+++ b/solr/core/src/test/org/apache/solr/cloud/TestLocalFSCloudBackupRestore.java
@@ -24,12 +24,20 @@ import org.junit.BeforeClass;
  * such file-system would be exposed via local file-system API.
  */
 public class TestLocalFSCloudBackupRestore extends AbstractCloudBackupRestoreTestCase {
+  private static String backupLocation;
 
   @BeforeClass
   public static void setupClass() throws Exception {
     configureCluster(NUM_SHARDS)// nodes
         .addConfig("conf1", TEST_PATH().resolve("configsets").resolve("cloud-minimal").resolve("conf"))
         .configure();
+
+    boolean whitespacesInPath = random().nextBoolean();
+    if (whitespacesInPath) {
+      backupLocation = createTempDir("my backup").toFile().getAbsolutePath();
+    } else {
+      backupLocation = createTempDir("mybackup").toFile().getAbsolutePath();
+    }
   }
 
   @Override
@@ -44,6 +52,6 @@ public class TestLocalFSCloudBackupRestore extends AbstractCloudBackupRestoreTes
 
   @Override
   public String getBackupLocation() {
-    return createTempDir().toFile().getAbsolutePath();
+    return backupLocation;
   }
 }


Mime
View raw message