This is an automated email from the ASF dual-hosted git repository. dsmiley pushed a commit to branch branch_9x in repository https://gitbox.apache.org/repos/asf/solr.git
commit 91a2e84a644ea149361fc56519720064d2b1562c Author: Nazerke Seidan <[email protected]> AuthorDate: Mon Jan 9 14:40:49 2023 -0500 SOLR-15787: Fix FileSystemConfigSetService test failure on Windows (#1239) Co-authored-by: Nazerke Seidan <[email protected]> Co-authored-by: Kevin Risden <[email protected]> --- .../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());
