Skip to content

Commit b62d687

Browse files
authored
[local] Write files atomically to avoid 500s on concurrent reads (#30)
1 parent 94d5622 commit b62d687

2 files changed

Lines changed: 22 additions & 1 deletion

File tree

source/file-server/src/main/java/nl/aerius/fileserver/local/LocalFileStorageSevice.java

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,17 @@ public void putFile(final String uuid, final String filename, final long size, f
6666
Files.createDirectory(uuidPath);
6767
}
6868
final Path file = filePath(uuidPath, filename);
69-
Files.copy(in, file, StandardCopyOption.REPLACE_EXISTING);
69+
// Write to a temporary file in the same directory and then atomically move it into place. A plain
70+
// Files.copy(REPLACE_EXISTING) deletes the target before recreating it, which exposes a window where a
71+
// concurrent read sees the file missing or partially written and fails with a 500. ATOMIC_MOVE (a rename)
72+
// ensures readers always observe either the complete old file or the complete new one.
73+
final Path tempFile = Files.createTempFile(uuidPath, filename + ".", ".tmp");
74+
try {
75+
Files.copy(in, tempFile, StandardCopyOption.REPLACE_EXISTING);
76+
Files.move(tempFile, file, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING);
77+
} finally {
78+
Files.deleteIfExists(tempFile);
79+
}
7080
}
7181

7282
@Override

source/file-server/src/test/java/nl/aerius/fileserver/local/LocalFileStorageSeviceTest.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
*/
1717
package nl.aerius.fileserver.local;
1818

19+
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
1920
import static org.junit.jupiter.api.Assertions.assertEquals;
2021
import static org.junit.jupiter.api.Assertions.assertFalse;
2122
import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -82,6 +83,16 @@ void testPutFileOverwrite() throws IOException {
8283
assertEquals(overwriteContent, Files.readString(expectedFile.toPath()), "Content of file should be as expected.");
8384
}
8485

86+
@Test
87+
void testPutFileLeavesNoTempFile() throws IOException {
88+
// The atomic-write path stages the content in a temp file before moving it into place; ensure that
89+
// temp file is always cleaned up so it can't leak into directory listings or repeated overwrites.
90+
service.putFile(UUID_CODE, FILENAME, 10, null, new ByteArrayInputStream(CONTENT.getBytes()));
91+
service.putFile(UUID_CODE, FILENAME, 0, null, new ByteArrayInputStream(CONTENT.getBytes()));
92+
final String[] storedFiles = expectedFile.getParentFile().list();
93+
assertArrayEquals(new String[] {FILENAME}, storedFiles, "Only the target file should remain, no temp residue.");
94+
}
95+
8596
@Test
8697
void testGetFile() throws IOException {
8798
writeTempFile();

0 commit comments

Comments
 (0)