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-17548: Switch all public Java APIs from File to Path #2907

Merged
merged 12 commits into from
Dec 19, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ protected LockFactory createLockFactory(String rawLockType) throws IOException {

@Override
public String normalize(String path) throws IOException {
return super.normalize(Path.of(path).toAbsolutePath().toString());
return super.normalize(Path.of(path).toAbsolutePath().normalize().toString());
}

@Override
Expand Down
73 changes: 2 additions & 71 deletions solr/core/src/java/org/apache/solr/handler/IndexFetcher.java
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,9 @@
import java.nio.channels.FileChannel;
import java.nio.charset.StandardCharsets;
import java.nio.file.FileStore;
import java.nio.file.FileSystems;
import java.nio.file.Files;
import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
import java.text.SimpleDateFormat;
import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -124,7 +122,6 @@
import org.apache.solr.security.AllowListUrlChecker;
import org.apache.solr.update.CommitUpdateCommand;
import org.apache.solr.update.UpdateShardHandler;
import org.apache.solr.util.FileUtils;
import org.apache.solr.util.IndexOutputOutputStream;
import org.apache.solr.util.RTimer;
import org.apache.solr.util.RefCounted;
Expand Down Expand Up @@ -1514,52 +1511,6 @@ private void copyTmpConfFiles2Conf(Path tmpconfDir) {
}
}

/**
* The tlog files are moved from the tmp dir to the tlog dir as an atomic filesystem operation. A
* backup of the old directory is maintained. If the directory move fails, it will try to revert
* the original tlog directory.
*/
private boolean copyTmpTlogFiles2Tlog(Path tmpTlogDir) {
Path tlogDir =
FileSystems.getDefault().getPath(solrCore.getUpdateHandler().getUpdateLog().getTlogDir());
Path backupTlogDir =
FileSystems.getDefault()
.getPath(
tlogDir.getParent().toAbsolutePath().toString(),
tmpTlogDir.getFileName().toString());

try {
Files.move(tlogDir, backupTlogDir, StandardCopyOption.ATOMIC_MOVE);
} catch (IOException e) {
log.error("Unable to rename: {} to: {}", tlogDir, backupTlogDir, e);
return false;
}

Path src =
FileSystems.getDefault()
.getPath(
backupTlogDir.toAbsolutePath().toString(), tmpTlogDir.getFileName().toString());
try {
Files.move(src, tlogDir, StandardCopyOption.ATOMIC_MOVE);
} catch (IOException e) {
log.error("Unable to rename: {} to: {}", src, tlogDir, e);

// In case of error, try to revert the original tlog directory
try {
Files.move(backupTlogDir, tlogDir, StandardCopyOption.ATOMIC_MOVE);
} catch (IOException e2) {
// bad, we were not able to revert the original tlog directory
throw new SolrException(
SolrException.ErrorCode.SERVER_ERROR,
"Unable to rename: " + backupTlogDir + " to: " + tlogDir);
}

return false;
}

return true;
}

private String getDateAsStr(Date d) {
return new SimpleDateFormat(SnapShooter.DATE_FMT, Locale.ROOT).format(d);
}
Expand Down Expand Up @@ -1602,23 +1553,6 @@ private Collection<Map<String, Object>> getModifiedConfFiles(
return nameVsFile.isEmpty() ? Collections.emptyList() : nameVsFile.values();
}

/**
* This simulates File.delete exception-wise, since this class has some strange behavior with it.
* The only difference is it returns null on success, throws SecurityException on
* SecurityException, otherwise returns Throwable preventing deletion (instead of false), for
* additional information.
*/
static Throwable delete(Path file) {
try {
Files.delete(file);
return null;
} catch (SecurityException e) {
throw e;
} catch (Throwable other) {
return other;
}
}

static boolean delTree(Path dir) {
try {
org.apache.lucene.util.IOUtils.rm(dir);
Expand Down Expand Up @@ -2049,16 +1983,13 @@ protected class DirectoryFileFetcher extends FileFetcher {
}

private static class LocalFsFile implements FileInterface {
private Path copy2Dir;

FileChannel fileChannel;
private FileOutputStream fileOutputStream;
Path file;

LocalFsFile(Path dir, String saveAs) throws IOException {
this.copy2Dir = dir;

this.file = Path.of(copy2Dir.toString(), saveAs);
this.file = dir.resolve(saveAs);

Path parentDir = this.file.getParent();
if (Files.notExists(parentDir)) {
Expand All @@ -2077,7 +2008,7 @@ private static class LocalFsFile implements FileInterface {

@Override
public void sync() throws IOException {
FileUtils.sync(file);
org.apache.lucene.util.IOUtils.fsync(file, false);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ public void configure(SolrResourceLoader loader, NamedList<String> initArgs)

@Override
public boolean exists(String storedResourceId) throws IOException {
return (Files.exists(Path.of(storageDir, storedResourceId)));
return Files.exists(Path.of(storageDir, storedResourceId));
}

@Override
Expand Down
14 changes: 8 additions & 6 deletions solr/core/src/java/org/apache/solr/update/UpdateLog.java
Copy link
Contributor

Choose a reason for hiding this comment

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

nice!

Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import com.codahale.metrics.Gauge;
import com.codahale.metrics.Meter;
import java.io.Closeable;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.UncheckedIOException;
import java.lang.invoke.MethodHandles;
Expand Down Expand Up @@ -733,12 +732,15 @@ private boolean updateFromOldTlogs(UpdateCommand cmd) {

public String[] getLogList(Path directory) {
final String prefix = TLOG_NAME + '.';
String[] names = directory.toFile().list((dir, name) -> name.startsWith(prefix));
if (names == null) {
throw new RuntimeException(new FileNotFoundException(directory.toFile().getAbsolutePath()));
try (Stream<Path> files = Files.list(directory)) {
return files
.map((file) -> file.getFileName().toString())
.filter((name) -> name.startsWith(prefix))
.sorted()
.toArray(String[]::new);
} catch (IOException e) {
throw new RuntimeException(e);
}
Arrays.sort(names);
return names;
}

public long getLastLogId() {
Expand Down
22 changes: 0 additions & 22 deletions solr/core/src/java/org/apache/solr/util/FileUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,38 +16,16 @@
*/
package org.apache.solr.util;

import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.RandomAccessFile;
import java.nio.channels.FileChannel;
import java.nio.file.Files;
import java.nio.file.Path;
import org.apache.commons.io.FileExistsException;

/** */
public class FileUtils {

/**
* Resolves a path relative a base directory.
*
* <p>This method does what "new File(base,path)" <b>Should</b> do, if it wasn't completely lame:
* If path is absolute, then a File for that path is returned; if it's not absolute, then a File
* is returned using "path" as a child of "base")
*/
public static Path resolvePath(Path base, String path) {
Path r = Path.of(path);
return r.isAbsolute() ? r : Path.of(base.toString(), path);
}

public static void copyFile(Path src, Path destination) throws IOException {
try (FileChannel in = new FileInputStream(src.toFile()).getChannel();
FileChannel out = new FileOutputStream(destination.toFile()).getChannel()) {
in.transferTo(0, in.size(), out);
}
}

/**
* Copied from Lucene's FSDirectory.fsync(String)
*
Expand Down
2 changes: 2 additions & 0 deletions solr/core/src/test/org/apache/solr/cli/PostToolTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,7 @@ public void testDoFilesMode() throws IOException {
postTool.recursive = 0;
postTool.dryRun = true;
postTool.solrUpdateUrl = URI.create("http://localhost:8983/solr/fake/update");
// TODO SOLR-8282 move to PATH
File dir = getFile("exampledocs").toFile();
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we just do a toString, maybe we can simplify this line???

Copy link
Contributor

Choose a reason for hiding this comment

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

matbe change postFiles to take in a list of paths?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Feels out of scope for now but agreed maybe we can simplify here. Unless you feel strongly about it, will add a TODO and I can do on a separate PR.

int num = postTool.postFiles(new String[] {dir.toString()}, 0, null, null);
assertEquals(2, num);
Expand All @@ -316,6 +317,7 @@ public void testRecursionAppliesToFilesMode() throws IOException {
postTool.recursive = 1; // This is the default
postTool.dryRun = true;
postTool.solrUpdateUrl = URI.create("http://localhost:8983/solr/fake/update");
// TODO SOLR-8282 move to PATH
File dir = getFile("exampledocs").toFile();
int num = postTool.postFiles(new String[] {dir.toString()}, 0, null, null);
assertEquals(2, num);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
import org.apache.solr.common.util.NamedList;
import org.apache.solr.embedded.JettyConfig;
import org.apache.solr.embedded.JettySolrRunner;
import org.apache.solr.util.FileUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -55,7 +54,7 @@ public final class ReplicationTestHelper {
"solr" + File.separator + "collection1" + File.separator + "conf" + File.separator;

public static JettySolrRunner createAndStartJetty(SolrInstance instance) throws Exception {
FileUtils.copyFile(
Files.copy(
Path.of(SolrTestCaseJ4.TEST_HOME(), "solr.xml"),
Path.of(instance.getHomeDir(), "solr.xml"));
Properties nodeProperties = new Properties();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
import org.apache.solr.client.solrj.impl.HttpSolrClient;
import org.apache.solr.embedded.JettyConfig;
import org.apache.solr.embedded.JettySolrRunner;
import org.apache.solr.util.FileUtils;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
Expand All @@ -72,7 +71,7 @@ public class TestReplicationHandlerBackup extends SolrJettyTestBase {

private static JettySolrRunner createAndStartJetty(ReplicationTestHelper.SolrInstance instance)
throws Exception {
FileUtils.copyFile(
Files.copy(
Path.of(SolrTestCaseJ4.TEST_HOME(), "solr.xml"),
Path.of(instance.getHomeDir(), "solr.xml"));
Properties nodeProperties = new Properties();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
import org.apache.solr.common.SolrInputDocument;
import org.apache.solr.embedded.JettyConfig;
import org.apache.solr.embedded.JettySolrRunner;
import org.apache.solr.util.FileUtils;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
Expand All @@ -57,7 +56,7 @@ public class TestRestoreCore extends SolrJettyTestBase {

private static JettySolrRunner createAndStartJetty(ReplicationTestHelper.SolrInstance instance)
throws Exception {
FileUtils.copyFile(
Files.copy(
Path.of(SolrTestCaseJ4.TEST_HOME(), "solr.xml"),
Path.of(instance.getHomeDir(), "solr.xml"));
Properties nodeProperties = new Properties();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@
public class ExternalFileFieldSortTest extends SolrTestCaseJ4 {

static void updateExternalFile() throws IOException {
final String testHome = SolrTestCaseJ4.getFile("solr/collection1").getParent().toString();
final Path testHome = SolrTestCaseJ4.getFile("solr/collection1").getParent();
String filename = "external_eff";
Files.copy(Path.of(testHome, filename), Path.of(h.getCore().getDataDir(), filename));
Files.copy(testHome.resolve(filename), Path.of(h.getCore().getDataDir(), filename));
}

private void addDocuments() {
Expand Down
9 changes: 0 additions & 9 deletions solr/core/src/test/org/apache/solr/util/FileUtilsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,6 @@

public class FileUtilsTest extends SolrTestCase {

@Test
public void testResolve() {
String cwd = Path.of(".").toAbsolutePath().toString();
assertEquals(Path.of("conf/data"), FileUtils.resolvePath(Path.of("conf"), "data"));
assertEquals(
Path.of(cwd + "/conf/data"), FileUtils.resolvePath(Path.of(cwd + "/conf"), "data"));
assertEquals(Path.of(cwd + "/data"), FileUtils.resolvePath(Path.of("conf"), cwd + "/data"));
}

@Test
public void testDetectsPathEscape() {
final var parent = Path.of(".");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
*/
package org.apache.solr.analysis;

import java.io.File;
import org.apache.commons.io.FileUtils;
import java.nio.file.Path;
import org.apache.commons.io.file.PathUtils;
import org.apache.solr.SolrTestCaseJ4;
import org.junit.BeforeClass;
import org.junit.Test;
Expand All @@ -32,9 +32,12 @@ public String getCoreName() {

@BeforeClass
public static void beforeTests() throws Exception {
File testHome = createTempDir().toFile();
FileUtils.copyDirectory(getFile("analysis-extras/solr").toFile(), testHome);
initCore("solrconfig-icucollate.xml", "schema-folding-extra.xml", testHome.getAbsolutePath());
Path testHome = createTempDir();
PathUtils.copyDirectory(getFile("analysis-extras/solr"), testHome);
initCore(
"solrconfig-icucollate.xml",
"schema-folding-extra.xml",
testHome.toAbsolutePath().toString());

int idx = 1;
// ICUFoldingFilterFactory
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,21 @@
*/
package org.apache.solr.schema;

import java.io.File;
import org.apache.commons.io.FileUtils;
import java.nio.file.Path;
import org.apache.commons.io.file.PathUtils;
import org.apache.solr.SolrTestCaseJ4;
import org.junit.BeforeClass;

/** Tests expert options of {@link ICUCollationField}. */
public class TestICUCollationFieldOptions extends SolrTestCaseJ4 {
@BeforeClass
public static void beforeClass() throws Exception {
File testHome = createTempDir().toFile();
FileUtils.copyDirectory(getFile("analysis-extras/solr").toFile(), testHome);
Path testHome = createTempDir();
PathUtils.copyDirectory(getFile("analysis-extras/solr"), testHome);
initCore(
"solrconfig-icucollate.xml", "schema-icucollateoptions.xml", testHome.getAbsolutePath());
"solrconfig-icucollate.xml",
"schema-icucollateoptions.xml",
testHome.toAbsolutePath().toString());
// add some docs
assertU(adoc("id", "1", "text", "foo-bar"));
assertU(adoc("id", "2", "text", "foo bar"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@

package org.apache.solr.update.processor;

import java.io.File;
import java.nio.file.Path;
import java.util.List;
import org.apache.commons.io.FileUtils;
import org.apache.commons.io.file.PathUtils;
import org.apache.solr.common.SolrInputDocument;
import org.junit.BeforeClass;
import org.junit.Test;
Expand All @@ -28,10 +28,12 @@ public class TestOpenNLPExtractNamedEntitiesUpdateProcessorFactory extends Updat

@BeforeClass
public static void beforeClass() throws Exception {
File testHome = createTempDir().toFile();
FileUtils.copyDirectory(getFile("analysis-extras/solr").toFile(), testHome);
Path testHome = createTempDir();
PathUtils.copyDirectory(getFile("analysis-extras/solr"), testHome);
initCore(
"solrconfig-opennlp-extract.xml", "schema-opennlp-extract.xml", testHome.getAbsolutePath());
"solrconfig-opennlp-extract.xml",
"schema-opennlp-extract.xml",
testHome.toAbsolutePath().toString());
}

@Test
Expand Down
Loading
Loading