Stops async db saving on shutdown. Adds JSON backup. (#1558)

* Stops async db saving on shutdown. Adds JSON backup.

* Fixes test failures and added Util class tests.
This commit is contained in:
tastybento 2020-11-01 13:58:29 -08:00 committed by GitHub
parent f7f36179d3
commit ba903bdaca
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 179 additions and 104 deletions

View File

@ -203,8 +203,8 @@ public class BentoBox extends JavaPlugin {
// Save islands & players data every X minutes
Bukkit.getScheduler().runTaskTimer(instance, () -> {
playersManager.asyncSaveAll();
islandsManager.asyncSaveAll();
playersManager.saveAll();
islandsManager.saveAll();
}, getSettings().getDatabaseBackupPeriod() * 20 * 60L, getSettings().getDatabaseBackupPeriod() * 20 * 60L);
// Make sure all flag listeners are registered.
@ -281,6 +281,9 @@ public class BentoBox extends JavaPlugin {
@Override
public void onDisable() {
// Stop all async database tasks
shutdown = true;
if (addonsManager != null) {
addonsManager.disableAddons();
}
@ -291,8 +294,6 @@ public class BentoBox extends JavaPlugin {
if (islandsManager != null) {
islandsManager.shutdown();
}
// Close all async database tasks
shutdown = true;
}
/**

View File

@ -36,6 +36,7 @@ public abstract class AbstractDatabaseHandler<T> {
* Async save task that runs repeatedly
*/
private BukkitTask asyncSaveTask;
private boolean inSave;
protected boolean shutdown;
@ -100,26 +101,20 @@ public abstract class AbstractDatabaseHandler<T> {
if (!plugin.isEnabled()) return;
// Run async queue
processQueue = new ConcurrentLinkedQueue<>();
asyncSaveTask = Bukkit.getScheduler().runTaskAsynchronously(plugin, () -> {
// Loop continuously
while (!shutdown || !processQueue.isEmpty()) {
while (!processQueue.isEmpty()) {
asyncSaveTask = Bukkit.getScheduler().runTaskTimerAsynchronously(plugin, () -> {
// Check shutdown
if(shutdown || plugin.isShutdown()) {
// Cancel - this will only get called if the plugin is shutdown separately to the server
databaseConnector.closeConnection(dataObject);
asyncSaveTask.cancel();
} else if (!inSave && !processQueue.isEmpty()) {
inSave = true;
while(!processQueue.isEmpty()) {
processQueue.poll().run();
}
// Shutdown flag
shutdown = plugin.isShutdown();
// Clear the queue and then sleep
try {
Thread.sleep(25);
} catch (InterruptedException e) {
plugin.logError("Thread sleep error " + e.getMessage());
Thread.currentThread().interrupt();
}
inSave = false;
}
// Cancel
asyncSaveTask.cancel();
databaseConnector.closeConnection(dataObject);
});
}, 0L, 1L);
}
protected AbstractDatabaseHandler() {}

View File

@ -112,6 +112,7 @@ public class JSONDatabaseHandler<T> extends AbstractJSONDatabaseHandler<T> {
return completableFuture;
}
String path = DATABASE_FOLDER_NAME + File.separator + dataObject.getSimpleName();
String backupPath = DATABASE_FOLDER_NAME + "_backup" + File.separator + dataObject.getSimpleName();
// Obtain the value of uniqueId within the instance (which must be a DataObject)
PropertyDescriptor propertyDescriptor = new PropertyDescriptor("uniqueId", dataObject);
@ -124,26 +125,37 @@ public class JSONDatabaseHandler<T> extends AbstractJSONDatabaseHandler<T> {
tableFolder.mkdirs();
}
File backupTableFolder = new File(plugin.getDataFolder(), backupPath);
if (!backupTableFolder.exists()) {
backupTableFolder.mkdirs();
}
String toStore = getGson().toJson(instance);
if (plugin.isEnabled()) {
// Async
processQueue.add(() -> store(completableFuture, toStore, file, tableFolder, fileName));
processQueue.add(() -> store(completableFuture, toStore, file, tableFolder, backupTableFolder, fileName, true));
} else {
// Sync
store(completableFuture, toStore, file, tableFolder, fileName);
store(completableFuture, toStore, file, tableFolder, backupTableFolder, fileName, false);
}
return completableFuture;
}
private void store(CompletableFuture<Boolean> completableFuture, String toStore, File file, File tableFolder, String fileName) {
try (FileWriter fileWriter = new FileWriter(file)) {
File tmpFile = new File(tableFolder, fileName + ".bak");
if (file.exists()) {
// Make a backup of file
private void store(CompletableFuture<Boolean> completableFuture, String toStore, File file, File tableFolder, File backupTableFolder, String fileName, boolean async) {
// Do not save anything if plug is disabled and this was an async request
if (async && !plugin.isEnabled()) return;
File tmpFile = new File(backupTableFolder, fileName);
if (file.exists()) {
// Make a backup of file
try {
Files.copy(file.toPath(), tmpFile.toPath(), StandardCopyOption.REPLACE_EXISTING);
} catch (IOException e) {
plugin.logError("Could not backup JSON file: " + tableFolder.getName() + " " + fileName + " " + e.getMessage());
}
}
try (FileWriter fileWriter = new FileWriter(file)) {
fileWriter.write(toStore);
Files.deleteIfExists(tmpFile.toPath());
completableFuture.complete(true);
} catch (IOException e) {
plugin.logError("Could not save JSON file: " + tableFolder.getName() + " " + fileName + " " + e.getMessage());

View File

@ -169,12 +169,19 @@ public class SQLDatabaseHandler<T> extends AbstractJSONDatabaseHandler<T> {
}
// This has to be on the main thread to avoid concurrent modification errors
String toStore = getGson().toJson(instance);
// Async
processQueue.add(() -> store(completableFuture, instance.getClass().getName(), toStore, sqlConfig.getSaveObjectSQL()));
if (plugin.isEnabled()) {
// Async
processQueue.add(() -> store(completableFuture, instance.getClass().getName(), toStore, sqlConfig.getSaveObjectSQL(), true));
} else {
// Sync
store(completableFuture, instance.getClass().getName(), toStore, sqlConfig.getSaveObjectSQL(), false);
}
return completableFuture;
}
private void store(CompletableFuture<Boolean> completableFuture, String name, String toStore, String sb) {
private void store(CompletableFuture<Boolean> completableFuture, String name, String toStore, String sb, boolean async) {
// Do not save anything if plug is disabled and this was an async request
if (async && !plugin.isEnabled()) return;
try (PreparedStatement preparedStatement = connection.prepareStatement(sb)) {
preparedStatement.setString(1, toStore);
preparedStatement.setString(2, toStore);

View File

@ -617,7 +617,7 @@ public class YamlDatabaseHandler<T> extends AbstractDatabaseHandler<T> {
}
if (clazz.equals(World.class)) {
// Get world by name - may be null...
value = Bukkit.getServer().getWorld((String)value);
value = Bukkit.getWorld((String)value);
}
// Enums
if (Enum.class.isAssignableFrom(clazz)) {

View File

@ -230,7 +230,6 @@ public class PortalTeleportationListener implements Listener {
e.setTo(plugin.getIWM().getNetherWorld(overWorld).getSpawnLocation());
} else {
// Teleport to standard nether
plugin.logDebug("Standard nether spawn = " + plugin.getIWM().getNetherWorld(fromWorld).getSpawnLocation());
new SafeSpotTeleport.Builder(plugin)
.entity(e.getPlayer())
.location(plugin.getIWM().getNetherWorld(fromWorld).getSpawnLocation())

View File

@ -6,7 +6,6 @@ import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Objects;
@ -30,7 +29,6 @@ import org.bukkit.entity.Player;
import org.bukkit.entity.PufferFish;
import org.bukkit.inventory.ItemStack;
import org.bukkit.permissions.PermissionAttachmentInfo;
import org.bukkit.scheduler.BukkitTask;
import org.bukkit.util.Vector;
import org.eclipse.jdt.annotation.NonNull;
import org.eclipse.jdt.annotation.Nullable;
@ -99,10 +97,6 @@ public class IslandsManager {
@NonNull
private List<String> deletedIslands;
private Set<String> toSave = new HashSet<>();
private BukkitTask task;
/**
* Islands Manager
* @param plugin - plugin
@ -1238,24 +1232,6 @@ public class IslandsManager {
}
}
/**
* Saves all the players at a rate of 1 per tick. Used as a backup.
* @since 1.8.0
*/
public void asyncSaveAll() {
if (!toSave.isEmpty()) return;
// Get a list of ID's to save
toSave = new HashSet<>(islandCache.getAllIslandIds());
Iterator<String> it = toSave.iterator();
task = Bukkit.getScheduler().runTaskTimer(plugin, () -> {
if (plugin.isEnabled() && it.hasNext()) {
getIslandById(it.next()).ifPresent(this::save);
} else {
toSave.clear();
task.cancel();
}
}, 0L, 1L);
}
/**
* Puts a player in a team. Removes them from their old island if required.
* @param teamIsland - team island

View File

@ -4,16 +4,13 @@ import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Map;
import java.util.Set;
import java.util.UUID;
import org.bukkit.Bukkit;
import org.bukkit.Location;
import org.bukkit.World;
import org.bukkit.entity.Player;
import org.bukkit.scheduler.BukkitTask;
import org.eclipse.jdt.annotation.NonNull;
import org.eclipse.jdt.annotation.Nullable;
@ -32,8 +29,6 @@ public class PlayersManager {
private Map<UUID, Players> playerCache;
private Set<UUID> inTeleport;
private Set<UUID> toSave = new HashSet<>();
private BukkitTask task;
/**
* Provides a memory cache of online player information
@ -76,25 +71,6 @@ public class PlayersManager {
Collections.unmodifiableCollection(playerCache.values()).forEach(handler::saveObjectAsync);
}
/**
* Saves all the players at a rate of 1 per tick. Used as a backup.
* @since 1.8.0
*/
public void asyncSaveAll() {
if (!toSave.isEmpty()) return;
// Get a list of ID's to save
toSave = new HashSet<>(playerCache.keySet());
Iterator<UUID> it = toSave.iterator();
task = Bukkit.getScheduler().runTaskTimer(plugin, () -> {
if (plugin.isEnabled() && it.hasNext()) {
this.save(it.next());
} else {
toSave.clear();
task.cancel();
}
}, 0L, 1L);
}
public void shutdown(){
saveAll();
playerCache.clear();

View File

@ -109,7 +109,7 @@ public class Util {
}
final String[] parts = s.split(":");
if (parts.length == 6) {
final World w = Bukkit.getServer().getWorld(parts[0]);
final World w = Bukkit.getWorld(parts[0]);
if (w == null) {
return null;
}

View File

@ -4,6 +4,7 @@ import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.isA;
@ -26,8 +27,6 @@ import java.util.logging.Logger;
import org.bukkit.Bukkit;
import org.bukkit.Location;
import org.bukkit.Server;
import org.bukkit.World;
import org.bukkit.configuration.file.YamlConfiguration;
import org.bukkit.scheduler.BukkitScheduler;
import org.bukkit.scheduler.BukkitTask;
@ -93,12 +92,7 @@ public class YamlDatabaseHandlerTest {
PowerMockito.mockStatic(Bukkit.class);
when(Bukkit.getScheduler()).thenReturn(scheduler);
when(scheduler.runTaskAsynchronously(any(), any(Runnable.class))).thenReturn(task);
Server server = mock(Server.class);
World world = mock(World.class);
when(world.getName()).thenReturn("cleanroom");
when(server.getWorld(anyString())).thenReturn(world);
when(Bukkit.getServer()).thenReturn(server);
when(scheduler.runTaskTimerAsynchronously(any(), any(Runnable.class), anyLong(), anyLong())).thenReturn(task);
// A YAML file representing island
uuid = UUID.randomUUID();
@ -303,7 +297,7 @@ public class YamlDatabaseHandlerTest {
*/
@Test
public void testYamlDatabaseHandler() {
verify(scheduler).runTaskAsynchronously(eq(plugin), registerLambdaCaptor.capture());
verify(scheduler).runTaskTimerAsynchronously(eq(plugin), registerLambdaCaptor.capture(), eq(0L), eq(1L));
Runnable lamda = registerLambdaCaptor.getValue();
// Cannot run with true otherwise it'll infinite loop
when(plugin.isShutdown()).thenReturn(true);

View File

@ -4,6 +4,7 @@ import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
@ -11,19 +12,31 @@ import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.UUID;
import org.bukkit.Bukkit;
import org.bukkit.Location;
import org.bukkit.Server;
import org.bukkit.World;
import org.bukkit.World.Environment;
import org.bukkit.block.BlockFace;
import org.bukkit.command.ConsoleCommandSender;
import org.bukkit.entity.Player;
import org.bukkit.util.Vector;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.stubbing.Answer;
import org.powermock.api.mockito.PowerMockito;
import org.powermock.core.classloader.annotations.PrepareForTest;
import org.powermock.modules.junit4.PowerMockRunner;
@ -40,6 +53,8 @@ import world.bentobox.bentobox.managers.IslandWorldManager;
@PrepareForTest( { Bukkit.class })
public class UtilTest {
private static final String[] NAMES = {"adam", "ben", "cara", "dave", "ed", "frank", "freddy", "george", "harry", "ian", "joe"};
@Mock
private BentoBox plugin;
@Mock
@ -74,11 +89,27 @@ public class UtilTest {
when(location.getYaw()).thenReturn(10F);
when(location.getPitch()).thenReturn(20F);
PowerMockito.mockStatic(Bukkit.class);
PowerMockito.mockStatic(Bukkit.class, Mockito.RETURNS_MOCKS);
Server server = mock(Server.class);
when(Bukkit.getServer()).thenReturn(server);
when(server.getWorld(Mockito.anyString())).thenReturn(world);
when(Bukkit.getWorld(anyString())).thenReturn(world);
when(Bukkit.getConsoleSender()).thenReturn(sender);
// Bukkit - online players
Map<UUID, String> online = new HashMap<>();
Set<Player> onlinePlayers = new HashSet<>();
for (int j = 0; j < NAMES.length; j++) {
Player p1 = mock(Player.class);
UUID uuid = UUID.randomUUID();
when(p1.getUniqueId()).thenReturn(uuid);
when(p1.getName()).thenReturn(NAMES[j]);
online.put(uuid, NAMES[j]);
onlinePlayers.add(p1);
}
when(Bukkit.getOnlinePlayers()).then((Answer<Set<Player>>) invocation -> onlinePlayers);
when(user.isPlayer()).thenReturn(true);
}
@After
@ -91,7 +122,7 @@ public class UtilTest {
*/
@Test
public void testGetServerVersion() {
assertEquals("bukkit",Util.getServerVersion());
assertEquals("bukkit", Util.getServerVersion());
}
/**
@ -156,7 +187,17 @@ public class UtilTest {
*/
@Test
public void testGetOnlinePlayerList() {
//fail("Not yet implemented"); // TODO
assertEquals("Online players, null", 11, Util.getOnlinePlayerList(null).size());
assertEquals("Online players, not user", 11, Util.getOnlinePlayerList(mock(User.class)).size());
Player p = mock(Player.class);
// Can't see (default)
when(p.canSee(any(Player.class))).thenReturn(false);
when(user.getPlayer()).thenReturn(p);
assertEquals("Online players, cannot see", 0, Util.getOnlinePlayerList(user).size());
// Can see
when(p.canSee(any(Player.class))).thenReturn(true);
assertEquals("Online players, cannot see", 11, Util.getOnlinePlayerList(user).size());
}
/**
@ -164,7 +205,19 @@ public class UtilTest {
*/
@Test
public void testTabLimit() {
//fail("Not yet implemented"); // TODO
List<String> list = new ArrayList<>();
assertTrue(Util.tabLimit(list, "").isEmpty());
list.add("alpha");
list.add("bravo");
list.add("charlie");
list.add("delta");
list.add("epsilon");
assertEquals(5, Util.tabLimit(list, "").size());
assertEquals(1, Util.tabLimit(list, "a").size());
assertEquals(1, Util.tabLimit(list, "b").size());
assertEquals(1, Util.tabLimit(list, "c").size());
assertEquals(1, Util.tabLimit(list, "d").size());
assertEquals(1, Util.tabLimit(list, "e").size());
}
/**
@ -172,7 +225,7 @@ public class UtilTest {
*/
@Test
public void testXyz() {
//fail("Not yet implemented"); // TODO
assertEquals("34,67,54", Util.xyz(new Vector(34, 67, 54)));
}
/**
@ -215,7 +268,21 @@ public class UtilTest {
*/
@Test
public void testGetWorld() {
//fail("Not yet implemented"); // TODO
assertNull(Util.getWorld(null));
when(world.getEnvironment()).thenReturn(Environment.NORMAL);
when(world.getName()).thenReturn("world_name");
when(Bukkit.getWorld(eq("world_name"))).thenReturn(world);
assertEquals(world, Util.getWorld(world));
// Nether
World nether = mock(World.class);
when(nether.getEnvironment()).thenReturn(Environment.NETHER);
when(nether.getName()).thenReturn("world_name_nether");
assertEquals("Nether", world, Util.getWorld(nether));
// End
World end = mock(World.class);
when(end.getEnvironment()).thenReturn(Environment.THE_END);
when(end.getName()).thenReturn("world_name_the_end");
assertEquals("End", world, Util.getWorld(end));
}
/**
@ -223,7 +290,55 @@ public class UtilTest {
*/
@Test
public void testBlockFaceToFloat() {
//fail("Not yet implemented"); // TODO
for (BlockFace bf : BlockFace.values()) {
float r = Util.blockFaceToFloat(bf);
switch (bf) {
case EAST:
assertEquals(90F, r, 0);
break;
case EAST_NORTH_EAST:
assertEquals(67.5F, r, 0);
break;
case NORTH_EAST:
assertEquals(45F, r, 0);
break;
case NORTH_NORTH_EAST:
assertEquals(22.5F, r, 0);
break;
case NORTH_NORTH_WEST:
assertEquals(337.5F, r, 0);
break;
case NORTH_WEST:
assertEquals(315F, r, 0);
break;
case SOUTH:
assertEquals(180F, r, 0);
break;
case SOUTH_EAST:
assertEquals(135F, r, 0);
break;
case SOUTH_SOUTH_EAST:
assertEquals(157.5F, r, 0);
break;
case SOUTH_SOUTH_WEST:
assertEquals(202.5F, r, 0);
break;
case SOUTH_WEST:
assertEquals(225F, r, 0);
break;
case WEST:
assertEquals(270F, r, 0);
break;
case WEST_NORTH_WEST:
assertEquals(292.5F, r, 0);
break;
case WEST_SOUTH_WEST:
assertEquals(247.5F, r, 0);
break;
default:
assertEquals(0F, r, 0);
}
}
}
@Test