You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by kr...@apache.org on 2023/01/09 19:40:55 UTC
[solr] branch main updated: SOLR-15787: Fix FileSystemConfigSetService test failure on Windows (#1239)
This is an automated email from the ASF dual-hosted git repository.
krisden pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new 5d724b80f1e SOLR-15787: Fix FileSystemConfigSetService test failure on Windows (#1239)
5d724b80f1e is described below
commit 5d724b80f1e76d2768946e43986e1e77b3de9fcf
Author: Nazerke Seidan <se...@gmail.com>
AuthorDate: Mon Jan 9 14:40:49 2023 -0500
SOLR-15787: Fix FileSystemConfigSetService test failure on Windows (#1239)
Co-authored-by: Nazerke Seidan <ns...@salesforce.com>
Co-authored-by: Kevin Risden <kr...@apache.org>
---
.../org/apache/solr/core/ConfigSetService.java | 8 ++++----
.../solr/core/FileSystemConfigSetService.java | 24 +++++++++++++++-------
.../org/apache/solr/core/backup/BackupManager.java | 8 +++++++-
.../org/apache/solr/core/TestConfigSetService.java | 19 +++++++----------
4 files changed, 35 insertions(+), 24 deletions(-)
diff --git a/solr/core/src/java/org/apache/solr/core/ConfigSetService.java b/solr/core/src/java/org/apache/solr/core/ConfigSetService.java
index 5d9fbf30054..da8c8b90877 100644
--- a/solr/core/src/java/org/apache/solr/core/ConfigSetService.java
+++ b/solr/core/src/java/org/apache/solr/core/ConfigSetService.java
@@ -399,7 +399,7 @@ public abstract class ConfigSetService {
* to true then file will be overwritten
*
* @param configName the name to give the config
- * @param fileName the name of the file
+ * @param fileName the name of the file with '/' used as the file path separator
* @param data the content of the file
* @param overwriteOnExists if true then file will be overwritten
* @throws SolrException if file exists and overwriteOnExists == false
@@ -420,7 +420,7 @@ public abstract class ConfigSetService {
* Download a file from config If the file does not exist, it returns null
*
* @param configName the name of the config
- * @param filePath the file to download
+ * @param filePath the file to download with '/' as the separator
* @return the content of the file
*/
public abstract byte[] downloadFileFromConfig(String configName, String filePath)
@@ -453,7 +453,7 @@ public abstract class ConfigSetService {
* Delete files in config
*
* @param configName the name of the config
- * @param filesToDelete a list of file paths to delete
+ * @param filesToDelete a list of file paths to delete using '/' as file path separator
*/
public abstract void deleteFilesFromConfig(String configName, List<String> filesToDelete)
throws IOException;
@@ -488,7 +488,7 @@ public abstract class ConfigSetService {
* lexicographically e.g. solrconfig.xml, lang/, lang/stopwords_en.txt
*
* @param configName the config name
- * @return list of file name paths in the config
+ * @return list of file name paths in the config with '/' uses as file path separators
*/
public abstract List<String> getAllConfigFiles(String configName) throws IOException;
diff --git a/solr/core/src/java/org/apache/solr/core/FileSystemConfigSetService.java b/solr/core/src/java/org/apache/solr/core/FileSystemConfigSetService.java
index 01dcdf8f524..75d8e288834 100644
--- a/solr/core/src/java/org/apache/solr/core/FileSystemConfigSetService.java
+++ b/solr/core/src/java/org/apache/solr/core/FileSystemConfigSetService.java
@@ -94,7 +94,7 @@ public class FileSystemConfigSetService extends ConfigSetService {
Path configDir = getConfigDir(configName);
Objects.requireNonNull(filesToDelete);
for (String fileName : filesToDelete) {
- Path file = configDir.resolve(fileName);
+ Path file = configDir.resolve(normalizePathToOsSeparator(fileName));
if (Files.exists(file)) {
if (Files.isDirectory(file)) {
deleteDir(file);
@@ -146,7 +146,7 @@ public class FileSystemConfigSetService extends ConfigSetService {
public void uploadFileToConfig(
String configName, String fileName, byte[] data, boolean overwriteOnExists)
throws IOException {
- Path filePath = getConfigDir(configName).resolve(fileName);
+ Path filePath = getConfigDir(configName).resolve(normalizePathToOsSeparator(fileName));
if (!Files.exists(filePath) || overwriteOnExists) {
Files.write(filePath, data);
}
@@ -218,7 +218,7 @@ public class FileSystemConfigSetService extends ConfigSetService {
@Override
public byte[] downloadFileFromConfig(String configName, String fileName) throws IOException {
- Path filePath = getConfigDir(configName).resolve(fileName);
+ Path filePath = getConfigDir(configName).resolve(normalizePathToOsSeparator(fileName));
byte[] data = null;
try {
data = Files.readAllBytes(filePath);
@@ -234,13 +234,13 @@ public class FileSystemConfigSetService extends ConfigSetService {
List<String> filePaths = new ArrayList<>();
Files.walkFileTree(
configDir,
- new SimpleFileVisitor<Path>() {
+ new SimpleFileVisitor<>() {
@Override
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)
throws IOException {
// don't include hidden (.) files
- if (!Files.isHidden(file)) {
- filePaths.add(configDir.relativize(file).toString());
+ if (!Files.isHidden(file) && !METADATA_FILE.equals(file.getFileName().toString())) {
+ filePaths.add(normalizePathToForwardSlash(configDir.relativize(file).toString()));
return FileVisitResult.CONTINUE;
}
return FileVisitResult.CONTINUE;
@@ -250,7 +250,9 @@ public class FileSystemConfigSetService extends ConfigSetService {
public FileVisitResult postVisitDirectory(Path dir, IOException ioException) {
String relativePath = configDir.relativize(dir).toString();
if (!relativePath.isEmpty()) {
- filePaths.add(relativePath + "/");
+ // We always want to have a trailing forward slash on a directory to
+ // match the normalization to forward slashes everywhere.
+ filePaths.add(relativePath + '/');
}
return FileVisitResult.CONTINUE;
}
@@ -259,6 +261,14 @@ public class FileSystemConfigSetService extends ConfigSetService {
return filePaths;
}
+ private String normalizePathToForwardSlash(String path) {
+ return path.replace(configSetBase.getFileSystem().getSeparator(), "/");
+ }
+
+ private String normalizePathToOsSeparator(String path) {
+ return path.replace("/", configSetBase.getFileSystem().getSeparator());
+ }
+
protected Path locateInstanceDir(CoreDescriptor cd) {
String configSet = cd.getConfigSet();
if (configSet == null) return cd.getInstanceDir();
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 faae2b9a70c..0d6cd77f3df 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
@@ -17,6 +17,7 @@
package org.apache.solr.core.backup;
import com.google.common.base.Preconditions;
+import java.io.File;
import java.io.IOException;
import java.io.OutputStream;
import java.io.OutputStreamWriter;
@@ -329,10 +330,15 @@ public class BackupManager {
private void downloadConfigToRepo(ConfigSetService configSetService, String configName, URI dir)
throws IOException {
List<String> filePaths = configSetService.getAllConfigFiles(configName);
+ // getAllConfigFiles always separates file paths with '/'
for (String filePath : filePaths) {
- URI uri = repository.resolve(dir, filePath);
+ // Replace '/' to ensure that propre file is resolved for writing.
+ URI uri = repository.resolve(dir, filePath.replace('/', File.separatorChar));
+ // checking for '/' is correct for a directory since ConfigSetService#getAllConfigFiles
+ // always separates file paths with '/'
if (!filePath.endsWith("/")) {
log.debug("Writing file {}", filePath);
+ // ConfigSetService#downloadFileFromConfig requires '/' in fle path separator
byte[] data = configSetService.downloadFileFromConfig(configName, filePath);
try (OutputStream os = repository.createOutput(uri)) {
os.write(data);
diff --git a/solr/core/src/test/org/apache/solr/core/TestConfigSetService.java b/solr/core/src/test/org/apache/solr/core/TestConfigSetService.java
index 07cc09b91a2..e1f3787fc88 100644
--- a/solr/core/src/test/org/apache/solr/core/TestConfigSetService.java
+++ b/solr/core/src/test/org/apache/solr/core/TestConfigSetService.java
@@ -30,7 +30,6 @@ import java.util.function.Supplier;
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.cloud.ZkConfigSetService;
import org.apache.solr.cloud.ZkTestServer;
-import org.apache.solr.common.cloud.SolrZkClient;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.Test;
@@ -39,18 +38,15 @@ public class TestConfigSetService extends SolrTestCaseJ4 {
private final ConfigSetService configSetService;
private static ZkTestServer zkServer;
- private static SolrZkClient zkClient;
@BeforeClass
public static void startZkServer() throws Exception {
zkServer = new ZkTestServer(createTempDir("zkData"));
zkServer.run();
- zkClient = new SolrZkClient(zkServer.getZkAddress("/solr"), 10000);
}
@AfterClass
public static void shutdownZkServer() throws IOException, InterruptedException {
- zkClient.close();
if (null != zkServer) {
zkServer.shutdown();
}
@@ -62,11 +58,10 @@ public class TestConfigSetService extends SolrTestCaseJ4 {
}
@ParametersFactory
- @SuppressWarnings("rawtypes")
- public static Iterable<Supplier[]> parameters() {
+ public static Iterable<Supplier<?>[]> parameters() {
return Arrays.asList(
- new Supplier[][] {
- {() -> new ZkConfigSetService(zkClient)},
+ new Supplier<?>[][] {
+ {() -> new ZkConfigSetService(zkServer.getZkClient())},
{() -> new FileSystemConfigSetService(createTempDir("configsets"))}
});
}
@@ -109,15 +104,15 @@ public class TestConfigSetService extends SolrTestCaseJ4 {
List<String> configFiles = configSetService.getAllConfigFiles(configName);
assertEquals(
- configFiles.toString(),
- "[file1, file2, solrconfig.xml, subdir/, subdir/file3, subdir/file4]");
+ configFiles,
+ List.of("file1", "file2", "solrconfig.xml", "subdir/", "subdir/file3", "subdir/file4"));
List<String> configs = configSetService.listConfigs();
- assertEquals(configs.toString(), "[testconfig]");
+ assertEquals(configs, List.of("testconfig"));
configSetService.copyConfig(configName, "testconfig.AUTOCREATED");
List<String> copiedConfigFiles = configSetService.getAllConfigFiles("testconfig.AUTOCREATED");
- assertEquals(configFiles.toString(), (copiedConfigFiles.toString()));
+ assertEquals(configFiles, copiedConfigFiles);
assertEquals(2, configSetService.listConfigs().size());