Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SOLR-16903: Migrate all tests using File to NIO Path #3263

Draft
wants to merge 17 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@
import static org.apache.solr.bench.BaseBenchState.log;

import com.codahale.metrics.Meter;
import java.io.File;
import java.io.IOException;
import java.io.OutputStream;
import java.io.PrintStream;
import java.lang.management.ManagementFactory;
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.nio.file.FileSystems;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
Expand Down Expand Up @@ -130,8 +130,7 @@ public void tearDown(BenchmarkParams benchmarkParams) throws Exception {
benchmarkParams.getBenchmark() + ".txt");
Files.createDirectories(metricsResults.getParent());

cluster.dumpMetrics(
metricsResults.getParent().toFile(), metricsResults.getFileName().toString());
cluster.dumpMetrics(metricsResults.getParent(), metricsResults.getFileName().toString());
}

/**
Expand Down Expand Up @@ -170,8 +169,14 @@ private void logClusterDirectorySize() throws IOException {
long clusterSize =
Files.walk(node)
.filter(Files::isRegularFile)
.map(Path::toFile)
.mapToLong(File::length)
.mapToLong(
file -> {
try {
return Files.size(file);
} catch (IOException e) {
throw new RuntimeException(e);
}
})
.sum();
log("mini cluster node size (bytes) " + node + " " + clusterSize);
} catch (IOException e) {
Expand Down Expand Up @@ -553,7 +558,9 @@ public static ModifiableSolrParams params(ModifiableSolrParams params, String...
*/
public static Path getFile(String name) {
final URL url =
MiniClusterState.class.getClassLoader().getResource(name.replace(File.separatorChar, '/'));
MiniClusterState.class
.getClassLoader()
.getResource(name.replace(FileSystems.getDefault().getSeparator(), "/"));
if (url != null) {
try {
return Path.of(url.toURI());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@
*/
package org.apache.solr.blockcache;

import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.lang.invoke.MethodHandles;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Set;
import org.apache.lucene.index.IndexFileNames;
import org.apache.lucene.store.Directory;
Expand Down Expand Up @@ -289,12 +290,12 @@ String getFileCacheName(String name) throws IOException {

private long getFileModified(String name) throws IOException {
if (in instanceof FSDirectory) {
File directory = ((FSDirectory) in).getDirectory().toFile();
File file = new File(directory, name);
if (!file.exists()) {
Path directory = ((FSDirectory) in).getDirectory();
Path file = directory.resolve(name);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just purely cosmentic, but should this really be path instead of file? In all the places?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. I like 'file' to differentiate from a 'dir'. 'path' is generic if could be either.

if (!Files.exists(file)) {
throw new FileNotFoundException("File [" + name + "] not found");
}
return file.lastModified();
return Files.getLastModifiedTime(file).toMillis();
} else {
throw new UnsupportedOperationException();
}
Expand Down
3 changes: 1 addition & 2 deletions solr/core/src/java/org/apache/solr/cli/CreateTool.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import org.apache.commons.cli.CommandLine;
import org.apache.commons.cli.Option;
import org.apache.commons.cli.Options;
import org.apache.commons.io.FileUtils;
import org.apache.commons.io.file.PathUtils;
import org.apache.solr.cli.CommonCLIOptions.DefaultValues;
import org.apache.solr.client.solrj.SolrClient;
Expand Down Expand Up @@ -190,7 +189,7 @@ protected void createCore(CommandLine cli, SolrClient solrClient) throws Excepti
"Failed to create new core instance directory: " + coreInstanceDir.toAbsolutePath());
}

FileUtils.copyDirectoryToDirectory(confDir.toFile(), coreInstanceDir.toFile());
PathUtils.copyDirectory(confDir, coreInstanceDir);

echoIfVerbose(
"\nCopying configuration to new core instance directory:\n"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,11 @@

import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;

import java.io.File;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import org.apache.commons.io.FileUtils;
import org.apache.commons.io.file.PathUtils;
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrQuery;
import org.apache.solr.client.solrj.embedded.EmbeddedSolrServer;
Expand All @@ -41,9 +40,9 @@ public class AnalysisAfterCoreReloadTest extends SolrTestCaseJ4 {

@BeforeClass
public static void beforeClass() throws Exception {
String tmpSolrHome = createTempDir().toFile().getAbsolutePath();
FileUtils.copyDirectory(new File(TEST_HOME()), new File(tmpSolrHome).getAbsoluteFile());
initCore("solrconfig.xml", "schema.xml", new File(tmpSolrHome).getAbsolutePath());
Path tmpSolrHome = createTempDir().toAbsolutePath();
PathUtils.copyDirectory(TEST_HOME(), tmpSolrHome);
initCore("solrconfig.xml", "schema.xml", tmpSolrHome);
}

@AfterClass
Expand Down
39 changes: 24 additions & 15 deletions solr/core/src/test/org/apache/solr/SolrInfoBeanTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@
*/
package org.apache.solr;

import java.io.File;
import java.net.URI;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Enumeration;
import java.util.List;
import java.util.stream.Stream;
import org.apache.lucene.tests.util.TestUtil;
import org.apache.solr.core.SolrInfoBean;
import org.apache.solr.handler.admin.LukeRequestHandler;
Expand Down Expand Up @@ -88,30 +90,37 @@ public void testCallMBeanInfo() throws Exception {
}

private static List<Class<?>> getClassesForPackage(String pckgname) throws Exception {
ArrayList<File> directories = new ArrayList<>();
ArrayList<Path> directories = new ArrayList<>();
ClassLoader cld = h.getCore().getResourceLoader().getClassLoader();
String path = pckgname.replace('.', '/');
Enumeration<URL> resources = cld.getResources(path);
while (resources.hasMoreElements()) {
final URI uri = resources.nextElement().toURI();
if (!"file".equalsIgnoreCase(uri.getScheme())) continue;
final File f = new File(uri);
final Path f = Path.of(uri);
directories.add(f);
}

ArrayList<Class<?>> classes = new ArrayList<>();
for (File directory : directories) {
if (directory.exists()) {
String[] files = directory.list();
for (String file : files) {
if (file.endsWith(".class")) {
String clazzName = file.substring(0, file.length() - 6);
// exclude Test classes that happen to be in these packages.
// class.ForName'ing some of them can cause trouble.
if (!clazzName.endsWith("Test") && !clazzName.startsWith("Test")) {
classes.add(Class.forName(pckgname + '.' + clazzName));
}
}
for (Path directory : directories) {
if (Files.exists(directory)) {
try (Stream<Path> files = Files.list(directory)) {
files.forEach(
(file) -> {
String fileName = file.getFileName().toString();
if (fileName.endsWith(".class")) {
String clazzName = fileName.substring(0, fileName.length() - 6);
// exclude Test classes that happen to be in these packages.
// class.ForName'ing some of them can cause trouble.
if (!clazzName.endsWith("Test") && !clazzName.startsWith("Test")) {
try {
classes.add(Class.forName(pckgname + '.' + clazzName));
} catch (ClassNotFoundException e) {
throw new RuntimeException(e);
}
}
}
});
}
}
}
Expand Down
23 changes: 11 additions & 12 deletions solr/core/src/test/org/apache/solr/SolrTestCaseJ4Test.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,9 @@
*/
package org.apache.solr;

import java.io.File;
import java.nio.file.Files;
import java.nio.file.Path;
import org.apache.commons.io.FileUtils;
import org.apache.commons.io.file.PathUtils;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.junit.AfterClass;
import org.junit.BeforeClass;
Expand All @@ -31,24 +30,24 @@ public class SolrTestCaseJ4Test extends SolrTestCaseJ4 {
public static void beforeClass() throws Exception {
// Create a temporary directory that holds a core NOT named "collection1". Use the smallest
// configuration sets we can, so we don't copy that much junk around.
String tmpSolrHome = createTempDir().toFile().getAbsolutePath();
Path tmpSolrHome = createTempDir().toAbsolutePath();

Path subHome = Path.of(tmpSolrHome, "core0", "conf");
Path subHome = tmpSolrHome.resolve("core0/conf");
Files.createDirectories(subHome);
String top = SolrTestCaseJ4.TEST_HOME() + "/collection1/conf";
Files.copy(Path.of(top, "schema-tiny.xml"), subHome.resolve("schema-tiny.xml"));
Files.copy(Path.of(top, "solrconfig-minimal.xml"), subHome.resolve("solrconfig-minimal.xml"));
Path top = SolrTestCaseJ4.TEST_HOME().resolve("collection1/conf");
Files.copy(top.resolve("schema-tiny.xml"), subHome.resolve("schema-tiny.xml"));
Files.copy(top.resolve("solrconfig-minimal.xml"), subHome.resolve("solrconfig-minimal.xml"));
Files.copy(
Path.of(top, "solrconfig.snippet.randomindexconfig.xml"),
top.resolve("solrconfig.snippet.randomindexconfig.xml"),
subHome.resolve("solrconfig.snippet.randomindexconfig.xml"));

FileUtils.copyDirectory(new File(tmpSolrHome, "core0"), new File(tmpSolrHome, "core1"));
PathUtils.copyDirectory(tmpSolrHome.resolve("core0"), tmpSolrHome.resolve("core1"));
// Core discovery will default to the name of the dir the core.properties file is in. So if
// everything else is OK as defaults, just the _presence_ of this file is sufficient.
FileUtils.touch(new File(tmpSolrHome, "core0/core.properties"));
FileUtils.touch(new File(tmpSolrHome, "core1/core.properties"));
PathUtils.touch(tmpSolrHome.resolve("core0/core.properties"));
PathUtils.touch(tmpSolrHome.resolve("core1/core.properties"));

Files.copy(getFile("solr/solr.xml"), Path.of(tmpSolrHome, "solr.xml"));
Files.copy(getFile("solr/solr.xml"), tmpSolrHome.resolve("solr.xml"));

initCore("solrconfig-minimal.xml", "schema-tiny.xml", tmpSolrHome, "core1");
}
Expand Down
2 changes: 1 addition & 1 deletion solr/core/src/test/org/apache/solr/TestCpuTimeSearch.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public static void setupSolr() throws Exception {
System.setProperty(AllowListUrlChecker.DISABLE_URL_ALLOW_LIST, "true");

Path configSet = createTempDir("configSet");
copyMinConf(configSet.toFile());
copyMinConf(configSet);
solrRule.startSolr(LuceneTestCase.createTempDir());

solrRule.newCollection("core1").withConfigSet(configSet.toString()).create();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public static void beforeTest() throws Exception {
Files.createDirectories(dataDir);
Files.createDirectories(confDir);

Files.copy(Path.of(SolrTestCaseJ4.TEST_HOME(), "solr.xml"), homeDir.resolve("solr.xml"));
Files.copy(SolrTestCaseJ4.TEST_HOME().resolve("solr.xml"), homeDir.resolve("solr.xml"));
String src_dir = TEST_HOME() + "/collection1/conf";
Files.copy(Path.of(src_dir, "schema-tiny.xml"), confDir.resolve("schema.xml"));
Files.copy(
Expand All @@ -71,7 +71,7 @@ public static void beforeTest() throws Exception {
Properties nodeProperties = new Properties();
// this sets the property for jetty starting SolrDispatchFilter
if (System.getProperty("solr.data.dir") == null) {
nodeProperties.setProperty("solr.data.dir", createTempDir().toFile().getCanonicalPath());
nodeProperties.setProperty("solr.data.dir", createTempDir().toRealPath().toString());
}

solrClientTestRule.startSolr(homeDir, nodeProperties, JettyConfig.builder().build());
Expand Down
11 changes: 5 additions & 6 deletions solr/core/src/test/org/apache/solr/TestTolerantSearch.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
*/
package org.apache.solr;

import java.io.File;
import java.io.IOException;
import java.io.OutputStream;
import java.nio.file.Files;
Expand Down Expand Up @@ -45,23 +44,23 @@ public class TestTolerantSearch extends SolrJettyTestBase {
private static String shard1;
private static String shard2;

private static File createSolrHome() throws Exception {
private static Path createSolrHome() throws Exception {
Path workDir = createTempDir();
setupJettyTestHome(workDir.toFile(), "collection1");
setupJettyTestHome(workDir, "collection1");
Files.copy(
Path.of(SolrTestCaseJ4.TEST_HOME() + "/collection1/conf/solrconfig-tolerant-search.xml"),
workDir.resolve("collection1").resolve("conf").resolve("solrconfig.xml"),
StandardCopyOption.REPLACE_EXISTING);
FileUtils.copyDirectory(
workDir.resolve("collection1").toFile(), workDir.resolve("collection2").toFile());
return workDir.toFile();
return workDir;
}

@BeforeClass
public static void createThings() throws Exception {
systemSetPropertySolrDisableUrlAllowList("true");
File solrHome = createSolrHome();
createAndStartJetty(solrHome.getAbsolutePath());
Path solrHome = createSolrHome();
createAndStartJetty(solrHome.toAbsolutePath());
String url = getBaseUrl();
collection1 = getHttpSolrClient(url, "collection1");
collection2 = getHttpSolrClient(url, "collection2");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
package org.apache.solr.blockcache;

import com.github.benmanes.caffeine.cache.Caffeine;
import java.io.File;
import java.io.IOException;
import java.lang.invoke.MethodHandles;
import java.nio.file.Path;
import java.util.Map;
import java.util.Random;
import org.apache.lucene.store.Directory;
Expand Down Expand Up @@ -99,16 +99,16 @@ public void releaseResources() {}
private static final int MAX_BUFFER_SIZE = 12000;
private static final int MAX_NUMBER_OF_READS = 20000;
private BlockDirectory directory;
private File file;
private Path file;
private Random random;
private MapperCache mapperCache;

@Override
@Before
public void setUp() throws Exception {
super.setUp();
file = createTempDir().toFile();
FSDirectory dir = FSDirectory.open(new File(file, "base").toPath());
file = createTempDir();
FSDirectory dir = FSDirectory.open(file.resolve("base"));
mapperCache = new MapperCache();

if (random().nextBoolean()) {
Expand Down Expand Up @@ -138,7 +138,7 @@ public void tearDown() throws Exception {

@Test
public void testEOF() throws IOException {
Directory fsDir = FSDirectory.open(new File(file, "normal").toPath());
Directory fsDir = FSDirectory.open(file.resolve("normal"));
String name = "test.eof";
createFile(name, fsDir, directory);
long fsLength = fsDir.fileLength(name);
Expand Down Expand Up @@ -170,7 +170,7 @@ public void testRandomAccessWrites() throws IOException {
int i = 0;
try {
for (; i < 10; i++) {
Directory fsDir = FSDirectory.open(new File(file, "normal").toPath());
Directory fsDir = FSDirectory.open(file.resolve("normal"));
String name = getName();
createFile(name, fsDir, directory);
assertInputsEquals(name, fsDir, directory);
Expand Down Expand Up @@ -253,9 +253,9 @@ private String getName() {
return Long.toUnsignedString(random.nextLong());
}

public static void rm(File file) {
public static void rm(Path file) {
try {
IOUtils.rm(file.toPath());
IOUtils.rm(file);
} catch (Throwable ignored) {
// TODO: should this class care if a file couldnt be deleted?
// this just emulates previous behavior, where only SecurityException would be handled.
Expand Down
Loading