Skip to content

Commit

Permalink
[MRESOLVER-441] Undo FileUtils changes for non-Windows (#376)
Browse files Browse the repository at this point in the history
Undo changes to code block that worked without any issue on non-Windows OSes.

---

https://issues.apache.org/jira/browse/MRESOLVER-441
  • Loading branch information
cstamas committed Nov 22, 2023
1 parent 0187cf1 commit b60921f
Showing 1 changed file with 7 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,9 @@
import java.io.InputStream;
import java.io.OutputStream;
import java.nio.ByteBuffer;
import java.nio.channels.FileChannel;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
import java.nio.file.StandardOpenOption;
import java.util.concurrent.ThreadLocalRandom;
import java.util.concurrent.atomic.AtomicBoolean;

Expand All @@ -39,7 +37,7 @@
* @since 1.9.0
*/
public final class FileUtils {
// Logic borrowed from Commons-Lang3: we really need only this, to decide do we fsync on directories or not
// Logic borrowed from Commons-Lang3: we really need only this, to decide do we "atomic move" or not
private static final boolean IS_WINDOWS =
System.getProperty("os.name", "unknown").startsWith("Windows");

Expand All @@ -66,6 +64,11 @@ public interface CollocatedTempFile extends TempFile {
* Invocation of this method merely signals that caller ultimately wants temp file to replace the target
* file, but when this method returns, the move operation did not yet happen, it will happen when this
* instance is closed.
* <p>
* Invoking this method <em>without writing to temp file</em> {@link #getPath()} (thus, not creating a temp
* file to be moved) is considered a bug, a mistake of the caller. Caller of this method should ensure
* that this method is invoked ONLY when the temp file is created and moving it to its final place is
* required.
*/
void move() throws IOException;
}
Expand Down Expand Up @@ -125,13 +128,11 @@ public void move() {

@Override
public void close() throws IOException {
if (wantsMove.get() && Files.isReadable(tempFile)) {
if (wantsMove.get()) {
if (IS_WINDOWS) {
copy(tempFile, file);
} else {
fsyncFile(tempFile);
Files.move(tempFile, file, StandardCopyOption.ATOMIC_MOVE);
fsyncParent(tempFile);
}
}
Files.deleteIfExists(tempFile);
Expand All @@ -157,33 +158,6 @@ private static void copy(Path source, Path target) throws IOException {
}
}

/**
* Performs fsync: makes sure no OS "dirty buffers" exist for given file.
*
* @param target Path that must not be {@code null}, must exist as plain file.
*/
private static void fsyncFile(Path target) throws IOException {
try (FileChannel file = FileChannel.open(target, StandardOpenOption.WRITE)) {
file.force(true);
}
}

/**
* Performs directory fsync: not usable on Windows, but some other OSes may also throw, hence thrown IO exception
* is just ignored.
*
* @param target Path that must not be {@code null}, must exist as plain file, and must have parent.
*/
private static void fsyncParent(Path target) throws IOException {
try (FileChannel parent = FileChannel.open(target.getParent(), StandardOpenOption.READ)) {
try {
parent.force(true);
} catch (IOException e) {
// ignore
}
}
}

/**
* A file writer, that accepts a {@link Path} to write some content to. Note: the file denoted by path may exist,
* hence implementation have to ensure it is able to achieve its goal ("replace existing" option or equivalent
Expand Down

0 comments on commit b60921f

Please sign in to comment.