fix(storage): atomic put + tolerate DirectoryNotEmptyException in delete
This commit is contained in:
@@ -7,6 +7,7 @@ import java.io.IOException;
|
|||||||
import java.io.InputStream;
|
import java.io.InputStream;
|
||||||
import java.nio.file.Files;
|
import java.nio.file.Files;
|
||||||
import java.nio.file.Path;
|
import java.nio.file.Path;
|
||||||
|
import java.nio.file.StandardCopyOption;
|
||||||
|
|
||||||
public class FilesystemArtifactStore implements ArtifactStore {
|
public class FilesystemArtifactStore implements ArtifactStore {
|
||||||
|
|
||||||
@@ -24,9 +25,11 @@ public class FilesystemArtifactStore implements ArtifactStore {
|
|||||||
public String put(ArtifactCoordinates coords, InputStream bytes, long size) throws IOException {
|
public String put(ArtifactCoordinates coords, InputStream bytes, long size) throws IOException {
|
||||||
Path target = pathOf(coords);
|
Path target = pathOf(coords);
|
||||||
Files.createDirectories(target.getParent());
|
Files.createDirectories(target.getParent());
|
||||||
|
Path tmp = target.resolveSibling(target.getFileName() + ".tmp");
|
||||||
try (InputStream in = bytes) {
|
try (InputStream in = bytes) {
|
||||||
Files.copy(in, target, java.nio.file.StandardCopyOption.REPLACE_EXISTING);
|
Files.copy(in, tmp, StandardCopyOption.REPLACE_EXISTING);
|
||||||
}
|
}
|
||||||
|
Files.move(tmp, target, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE);
|
||||||
return target.toString();
|
return target.toString();
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -45,12 +48,17 @@ public class FilesystemArtifactStore implements ArtifactStore {
|
|||||||
Path target = pathOf(coords);
|
Path target = pathOf(coords);
|
||||||
Files.deleteIfExists(target);
|
Files.deleteIfExists(target);
|
||||||
// Sweep empty {appId}/v{n}/ then {appId}/ if now empty.
|
// Sweep empty {appId}/v{n}/ then {appId}/ if now empty.
|
||||||
|
// A concurrent put of a sibling version can create v{n+1}/ mid-sweep —
|
||||||
|
// DirectoryNotEmptyException just means "stop sweeping," which is what
|
||||||
|
// "remove empty parent dirs" semantically means.
|
||||||
Path versionDir = target.getParent();
|
Path versionDir = target.getParent();
|
||||||
if (versionDir != null && Files.isDirectory(versionDir) && isEmpty(versionDir)) {
|
if (versionDir != null && Files.isDirectory(versionDir) && isEmpty(versionDir)) {
|
||||||
Files.delete(versionDir);
|
try { Files.delete(versionDir); }
|
||||||
|
catch (java.nio.file.DirectoryNotEmptyException e) { return; }
|
||||||
Path appDir = versionDir.getParent();
|
Path appDir = versionDir.getParent();
|
||||||
if (appDir != null && Files.isDirectory(appDir) && isEmpty(appDir)) {
|
if (appDir != null && Files.isDirectory(appDir) && isEmpty(appDir)) {
|
||||||
Files.delete(appDir);
|
try { Files.delete(appDir); }
|
||||||
|
catch (java.nio.file.DirectoryNotEmptyException e) { /* leave it */ }
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -58,4 +58,35 @@ class FilesystemArtifactStoreTest {
|
|||||||
assertThat(store.locator(c))
|
assertThat(store.locator(c))
|
||||||
.isEqualTo(tmp.resolve("11111111-1111-1111-1111-111111111111/v7/app.jar").toString());
|
.isEqualTo(tmp.resolve("11111111-1111-1111-1111-111111111111/v7/app.jar").toString());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void putOverwritesExistingArtifactWithDifferentContent(@TempDir Path tmp) throws Exception {
|
||||||
|
FilesystemArtifactStore store = new FilesystemArtifactStore(tmp.toString());
|
||||||
|
ArtifactCoordinates c = new ArtifactCoordinates("default", UUID.randomUUID(), 1);
|
||||||
|
|
||||||
|
store.put(c, new ByteArrayInputStream("v1-bytes".getBytes()), 8);
|
||||||
|
store.put(c, new ByteArrayInputStream("v2-different-bytes".getBytes()), 18);
|
||||||
|
|
||||||
|
try (InputStream in = store.get(c)) {
|
||||||
|
assertThat(new String(in.readAllBytes())).isEqualTo("v2-different-bytes");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void deleteLeavesAppDirAloneWhenSiblingVersionExists(@TempDir Path tmp) throws Exception {
|
||||||
|
FilesystemArtifactStore store = new FilesystemArtifactStore(tmp.toString());
|
||||||
|
UUID appId = UUID.randomUUID();
|
||||||
|
ArtifactCoordinates v1 = new ArtifactCoordinates("default", appId, 1);
|
||||||
|
ArtifactCoordinates v2 = new ArtifactCoordinates("default", appId, 2);
|
||||||
|
|
||||||
|
store.put(v1, new ByteArrayInputStream("a".getBytes()), 1);
|
||||||
|
store.put(v2, new ByteArrayInputStream("b".getBytes()), 1);
|
||||||
|
|
||||||
|
store.delete(v1);
|
||||||
|
|
||||||
|
// v1/ should be gone, but v2/ and the appId/ dir should remain (sibling lives there)
|
||||||
|
assertThat(Files.exists(tmp.resolve(appId.toString()).resolve("v1"))).isFalse();
|
||||||
|
assertThat(Files.exists(tmp.resolve(appId.toString()).resolve("v2/app.jar"))).isTrue();
|
||||||
|
assertThat(Files.exists(tmp.resolve(appId.toString()))).isTrue();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user