diff --git a/src/main/java/net/tfminecraft/vfbuilders/data/Database.java b/src/main/java/net/tfminecraft/vfbuilders/data/Database.java index 43afe2b..ad5e692 100644 --- a/src/main/java/net/tfminecraft/vfbuilders/data/Database.java +++ b/src/main/java/net/tfminecraft/vfbuilders/data/Database.java @@ -16,38 +16,134 @@ import java.lang.reflect.Type; import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.nio.file.Path; import java.nio.file.StandardCopyOption; +import java.time.Duration; import java.util.*; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; public class Database { - private static final File file = new File(VFBuilders.plugin.getDataFolder(), "data/stations.json"); private static final Gson gson = new GsonBuilder().setPrettyPrinting().create(); + /** Held while a snapshot file is written; package-visible so tests can hold the writer. */ + static final Object WRITE_LOCK = new Object(); + private static final AtomicLong snapshots = new AtomicLong(); + // Guarded by WRITE_LOCK: the newest snapshot that has started writing. + private static long newestWrite; + private static ExecutorService writer; + + /** The stations file, under the plugin's current data folder. */ + static Path path() { + return file().toPath(); + } + + private static File file() { + return new File(VFBuilders.plugin.getDataFolder(), "data/stations.json"); + } + public static void saveStations(Map stations) { saveStationsNow(stations); } + /** + * Writes the stations durably before returning. Callers that must know the state is on disk + * (construction start, completion, cancellation, station removal, disable) use this. + */ public static boolean saveStationsNow(Map stations) { try { - List dataList = new ArrayList<>(); - - for (ActiveStation station : stations.values()) { - StationData data = new StationData(); - data.location = toSerializableLoc(station.getLocation()); - data.stationId = station.getStation().getId(); // Save station identifier - data.spawnLocation = station.hasSpawnLocation() ? toSerializableLoc(station.getSpawnLocation()) : null; - data.blueprintId = station.hasBlueprint() ? station.getBlueprint().getId() : null; - data.timeLeft = station.getTimeLeft(); - data.outputUuid = station.getOutputUuid(); - if (station.hasBlueprint()) { - data.constructorUuid = toText(station.getConstructorUuid()); - data.constructorCharacterId = toText(station.getConstructorCharacterId()); - } + return write(snapshot(stations)); + } catch (RuntimeException e) { + e.printStackTrace(); + return false; + } + } + + /** + * Snapshots the stations on the calling thread and writes them durably on the station writer + * thread. A snapshot taken earlier than one already written is dropped, so a later synchronous + * save is never overwritten by an older queued one. + */ + public static CompletableFuture saveStationsLater(Map stations) { + Snapshot snapshot; + try { + snapshot = snapshot(stations); + } catch (RuntimeException e) { + e.printStackTrace(); + return CompletableFuture.completedFuture(false); + } + return CompletableFuture.supplyAsync(() -> write(snapshot), writer()); + } + + /** Finishes queued writes and stops the writer thread; the next background save starts a new one. */ + public static void close() { + close(Duration.ofSeconds(30)); + } - dataList.add(data); + static void close(Duration timeout) { + ExecutorService current; + synchronized (Database.class) { + current = writer; + writer = null; + } + if (current == null) return; + current.shutdown(); + try { + if (!current.awaitTermination(timeout.toMillis(), TimeUnit.MILLISECONDS)) { + VFBuilders.plugin.getLogger().warning( + "Station writer still busy after " + timeout.toSeconds() + "s; the last synchronous save stands."); + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + + private static synchronized ExecutorService writer() { + if (writer == null) { + writer = Executors.newSingleThreadExecutor(task -> { + Thread thread = new Thread(task, "VFBuilders station writer"); + thread.setDaemon(true); + return thread; + }); + } + return writer; + } + + private static Snapshot snapshot(Map stations) { + List dataList = new ArrayList<>(); + + for (ActiveStation station : stations.values()) { + StationData data = new StationData(); + data.location = toSerializableLoc(station.getLocation()); + data.stationId = station.getStation().getId(); // Save station identifier + data.spawnLocation = station.hasSpawnLocation() ? toSerializableLoc(station.getSpawnLocation()) : null; + data.blueprintId = station.hasBlueprint() ? station.getBlueprint().getId() : null; + data.timeLeft = station.getTimeLeft(); + data.outputUuid = station.getOutputUuid(); + if (station.hasBlueprint()) { + data.constructorUuid = toText(station.getConstructorUuid()); + data.constructorCharacterId = toText(station.getConstructorCharacterId()); } + dataList.add(data); + } + return new Snapshot(snapshots.incrementAndGet(), file(), dataList); + } + + private static boolean write(Snapshot snapshot) { + synchronized (WRITE_LOCK) { + if (snapshot.sequence() < newestWrite) return true; // A newer snapshot already wrote. + newestWrite = snapshot.sequence(); + return writeFile(snapshot.target(), snapshot.stations()); + } + } + + private static boolean writeFile(File file, List dataList) { + try { var directory = file.toPath().getParent(); Files.createDirectories(directory); var temporary = Files.createTempFile(directory, "stations-", ".tmp"); @@ -70,9 +166,13 @@ public static boolean saveStationsNow(Map stations) { } } + // The target is resolved on the calling thread, so the writer thread never reads plugin state. + private record Snapshot(long sequence, File target, List stations) {} + public static HashMap loadStations() { HashMap loaded = new HashMap<>(); + File file = file(); if (!file.exists()) return loaded; try (Reader reader = new FileReader(file)) { diff --git a/src/main/java/net/tfminecraft/vfbuilders/managers/StationManager.java b/src/main/java/net/tfminecraft/vfbuilders/managers/StationManager.java index 6f1a0c9..775ca21 100644 --- a/src/main/java/net/tfminecraft/vfbuilders/managers/StationManager.java +++ b/src/main/java/net/tfminecraft/vfbuilders/managers/StationManager.java @@ -82,7 +82,8 @@ public void start() { @Override public void run() { if (stations.values().stream().anyMatch(ActiveStation::hasBlueprint)) { - Database.saveStationsNow(stations); + // Only countdown progress changes between the synchronous checkpoints. + Database.saveStationsLater(stations); } } }.runTaskTimer(VFBuilders.plugin, 600L, 600L); @@ -93,6 +94,7 @@ public void stop() { station.removeDisplay(); } Database.saveStations(stations); + Database.close(); } public void rebindAfterReload(Runnable reloadDefinitions) { diff --git a/src/test/java/net/tfminecraft/vfbuilders/DatabaseFixture.java b/src/test/java/net/tfminecraft/vfbuilders/DatabaseFixture.java index 4ebf571..6f69291 100644 --- a/src/test/java/net/tfminecraft/vfbuilders/DatabaseFixture.java +++ b/src/test/java/net/tfminecraft/vfbuilders/DatabaseFixture.java @@ -4,8 +4,8 @@ import java.nio.file.Files; import java.nio.file.Path; -/** One data directory per JVM, matching Database's static path lifetime. */ -final class DatabaseFixture { +/** One shared data directory per JVM for tests that save stations. */ +public final class DatabaseFixture { private static final Path ROOT; static { @@ -16,7 +16,7 @@ final class DatabaseFixture { } } - static Path folder() throws IOException { + public static Path folder() throws IOException { return Files.createDirectories(ROOT); } } diff --git a/src/test/java/net/tfminecraft/vfbuilders/SchedulerTest.java b/src/test/java/net/tfminecraft/vfbuilders/SchedulerTest.java index d03af0d..7735ef9 100644 --- a/src/test/java/net/tfminecraft/vfbuilders/SchedulerTest.java +++ b/src/test/java/net/tfminecraft/vfbuilders/SchedulerTest.java @@ -75,15 +75,18 @@ void ticksPauseIncompleteWorkFinishReadyWorkAndRefreshOnlyLoadedChunks() throws verify(waiting).rebindDefinitions(); db.clearInvocations(); server.getScheduler().performTicks(579); - db.verify(() -> Database.saveStationsNow(map)); + db.verify(() -> Database.saveStationsLater(map)); + db.verify(() -> Database.saveStationsNow(map), never()); when(ready.hasBlueprint()).thenReturn(false); when(waiting.hasBlueprint()).thenReturn(false); when(detached.hasBlueprint()).thenReturn(false); db.clearInvocations(); server.getScheduler().performTicks(600); - db.verify(() -> Database.saveStationsNow(map), never()); + db.verify(() -> Database.saveStationsLater(map), never()); manager.stop(); - db.verify(() -> Database.saveStations(map)); + var order = inOrder(Database.class); + order.verify(db, () -> Database.saveStations(map)); + order.verify(db, Database::close); } finally { MockBukkit.unmock(); VFBuilders.plugin = null; diff --git a/src/test/java/net/tfminecraft/vfbuilders/data/StationWriterTest.java b/src/test/java/net/tfminecraft/vfbuilders/data/StationWriterTest.java new file mode 100644 index 0000000..62d47c1 --- /dev/null +++ b/src/test/java/net/tfminecraft/vfbuilders/data/StationWriterTest.java @@ -0,0 +1,125 @@ +package net.tfminecraft.vfbuilders.data; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +import java.nio.file.*; +import java.time.Duration; +import java.util.*; +import java.util.concurrent.*; +import java.util.logging.Logger; +import net.tfminecraft.vfbuilders.DatabaseFixture; +import net.tfminecraft.vfbuilders.VFBuilders; +import net.tfminecraft.vfbuilders.core.*; +import org.bukkit.*; +import org.junit.jupiter.api.*; + +class StationWriterTest { + private Path file; + + @BeforeEach + void plugin() throws Exception { + Path folder = DatabaseFixture.folder(); + VFBuilders.plugin = mock(VFBuilders.class); + when(VFBuilders.plugin.getDataFolder()).thenReturn(folder.toFile()); + file = Database.path(); + Files.createDirectories(file.getParent()); + } + + @AfterEach + void cleanUp() throws Exception { + Database.close(); + VFBuilders.plugin = null; + Files.deleteIfExists(file); + } + + private static Map stations(int timeLeft) { + var world = mock(World.class); + when(world.getName()).thenReturn("build-world"); + var loc = new Location(world, 3, 64, 5); + var definition = mock(Station.class); + when(definition.getId()).thenReturn("dock"); + var station = mock(ActiveStation.class); + when(station.getLocation()).thenReturn(loc); + when(station.getStation()).thenReturn(definition); + when(station.getTimeLeft()).thenReturn(timeLeft); + return Map.of(loc, station); + } + + @Test + void backgroundSaveWritesTheSnapshotTakenOnTheCallingThread() throws Exception { + var stations = stations(41); + var station = stations.values().iterator().next(); + var saved = Database.saveStationsLater(stations); + // Later changes are not part of the queued snapshot. + when(station.getTimeLeft()).thenReturn(7); + assertTrue(saved.get(5, TimeUnit.SECONDS)); + assertTrue(Files.readString(file).contains("\"timeLeft\": 41")); + try (var paths = Files.list(file.getParent())) { + assertEquals(List.of(file), paths.toList()); + } + } + + @Test + void queuedOlderSnapshotNeverOverwritesALaterSynchronousSave() throws Exception { + CompletableFuture older; + synchronized (Database.WRITE_LOCK) { + older = Database.saveStationsLater(stations(30)); + // The writer thread is now waiting for the lock; this synchronous save is newer. + assertTrue(Database.saveStationsNow(stations(12))); + } + assertTrue(older.get(5, TimeUnit.SECONDS)); + assertTrue(Files.readString(file).contains("\"timeLeft\": 12")); + assertTrue(Database.saveStationsLater(stations(11)).get(5, TimeUnit.SECONDS)); + assertTrue(Files.readString(file).contains("\"timeLeft\": 11")); + } + + @Test + void snapshotFailureIsReportedWithoutQueueingAWrite() throws Exception { + Files.writeString(file, "[]"); + var detached = mock(ActiveStation.class); + when(detached.getLocation()).thenReturn(new Location(null, 1, 2, 3)); + assertFalse(Database.saveStationsLater(Map.of(new Location(null, 1, 2, 3), detached)).get()); + assertEquals("[]", Files.readString(file)); + } + + @Test + void closeDrainsQueuedWritesAndTheWriterRestartsAfterwards() throws Exception { + Database.close(); // Nothing started yet. + CompletableFuture queued; + synchronized (Database.WRITE_LOCK) { + queued = Database.saveStationsLater(stations(9)); + } + Database.close(); + assertTrue(queued.isDone()); + assertTrue(queued.get()); + assertTrue(Files.readString(file).contains("\"timeLeft\": 9")); + assertTrue(Database.saveStationsLater(stations(8)).get(5, TimeUnit.SECONDS)); + assertTrue(Files.readString(file).contains("\"timeLeft\": 8")); + } + + @Test + void closeGivesUpWaitingForAStuckWriter() throws Exception { + var logger = mock(Logger.class); + when(VFBuilders.plugin.getLogger()).thenReturn(logger); + CompletableFuture stuck; + synchronized (Database.WRITE_LOCK) { + stuck = Database.saveStationsLater(stations(5)); + Database.close(Duration.ofMillis(50)); + verify(logger).warning(contains("Station writer still busy")); + } + assertTrue(stuck.get(5, TimeUnit.SECONDS)); + } + + @Test + void interruptedCloseKeepsTheInterruptFlag() throws Exception { + CompletableFuture pending; + synchronized (Database.WRITE_LOCK) { + pending = Database.saveStationsLater(stations(4)); + Thread.currentThread().interrupt(); + Database.close(); + assertTrue(Thread.interrupted()); + } + assertTrue(pending.get(5, TimeUnit.SECONDS)); + } +}