From 16e6ef1dc78692ae290d76c2a964a37b9ef9dc7d Mon Sep 17 00:00:00 2001 From: Risto Lahtela Date: Sun, 24 Feb 2019 12:28:58 +0200 Subject: [PATCH] [#769, #928] Session save on server shutdown (#927) * ShutdownHook: No sessions to save check ShutdownHook now checks if it needs to save any sessions and does not start the database if no sessions are unsaved. * SessionCache.getActiveSessions() now immutable * [#769] Bukkit and Sponge server shutdown save Implemented following save procedure for Bukkit: - On plugin disable check if server is shutting down and save sessions - Shutdown hook triggered on JVM shutdown calls the same session save - Save clears sessions from cache, so the sessions are not saved twice Implemented following save procedure for Sponge: - Listen for GameStoppingServerEvent - On plugin disable ask listener if shutting down and save sessions - Shutdown hook triggered on JVM shutdown calls the same session save - Save clears sessions from cache, so the sessions are not saved twice Test: - Tests ShutdownSave on reload - Tests ShutdownSave on shutdown - Tests ShutdownSave on JVM shutdown --- .../plan/BukkitServerShutdownSave.java | 65 ++++++++ .../main/java/com/djrapitops/plan/Plan.java | 6 + .../djrapitops/plan/PlanBukkitComponent.java | 2 + .../bukkit/BukkitSuperClassBindingModule.java | 7 +- .../plan/system/tasks/BukkitTaskSystem.java | 2 + .../plan/system/tasks/BungeeTaskSystem.java | 2 + .../djrapitops/plan/ServerShutdownSave.java | 121 ++++++++++++++ .../com/djrapitops/plan/ShutdownHook.java | 69 +------- .../java/com/djrapitops/plan/db/SQLiteDB.java | 5 +- .../plan/system/cache/CacheSystem.java | 11 +- .../plan/system/cache/SessionCache.java | 3 +- .../plan/system/locale/lang/PluginLang.java | 1 + .../plan/system/processing/Processing.java | 33 +++- .../com/djrapitops/plan/ShutdownSaveTest.java | 155 ++++++++++++++++++ .../test/java/extension/PrintExtension.java | 43 +++++ .../test/java/utilities/TestConstants.java | 2 + .../test/java/utilities/TestResources.java | 20 ++- .../src/test/java/utilities/mocks/Mocker.java | 32 +--- .../java/com/djrapitops/plan/PlanSponge.java | 5 + .../djrapitops/plan/PlanSpongeComponent.java | 2 + .../plan/SpongeServerShutdownSave.java | 57 +++++++ .../sponge/SpongeSuperClassBindingModule.java | 5 + .../listeners/SpongeListenerSystem.java | 24 ++- .../plan/system/tasks/SpongeTaskSystem.java | 2 + .../plan/system/tasks/VelocityTaskSystem.java | 2 + 25 files changed, 561 insertions(+), 115 deletions(-) create mode 100644 Plan/bukkit/src/main/java/com/djrapitops/plan/BukkitServerShutdownSave.java create mode 100644 Plan/common/src/main/java/com/djrapitops/plan/ServerShutdownSave.java create mode 100644 Plan/common/src/test/java/com/djrapitops/plan/ShutdownSaveTest.java create mode 100644 Plan/common/src/test/java/extension/PrintExtension.java create mode 100644 Plan/sponge/src/main/java/com/djrapitops/plan/SpongeServerShutdownSave.java diff --git a/Plan/bukkit/src/main/java/com/djrapitops/plan/BukkitServerShutdownSave.java b/Plan/bukkit/src/main/java/com/djrapitops/plan/BukkitServerShutdownSave.java new file mode 100644 index 000000000..cfad8abe7 --- /dev/null +++ b/Plan/bukkit/src/main/java/com/djrapitops/plan/BukkitServerShutdownSave.java @@ -0,0 +1,65 @@ +/* + * This file is part of Player Analytics (Plan). + * + * Plan is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License v3 as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Plan is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Plan. If not, see . + */ +package com.djrapitops.plan; + +import com.djrapitops.plan.system.database.DBSystem; +import com.djrapitops.plan.utilities.java.Reflection; +import com.djrapitops.plugin.logging.console.PluginLogger; +import com.djrapitops.plugin.logging.error.ErrorHandler; + +import javax.inject.Inject; +import javax.inject.Singleton; + +/** + * ServerShutdownSave implementation for Bukkit based servers. + * + * @author Rsl1122 + */ +@Singleton +public class BukkitServerShutdownSave extends ServerShutdownSave { + + private final PluginLogger logger; + + @Inject + public BukkitServerShutdownSave( + DBSystem dbSystem, + PluginLogger logger, + ErrorHandler errorHandler + ) { + super(dbSystem, errorHandler); + this.logger = logger; + } + + @Override + protected boolean checkServerShuttingDownStatus() { + try { + return performCheck(); + } catch (Exception | NoClassDefFoundError | NoSuchFieldError e) { + logger.debug("Server shutdown check failed, using JVM ShutdownHook instead. Error: " + e.toString()); + return false; // ShutdownHook handles save in case this fails upon plugin disable. + } + } + + private boolean performCheck() { + // Special thanks to Fuzzlemann for figuring out the methods required for this check. + // https://github.com/Rsl1122/Plan-PlayerAnalytics/issues/769#issuecomment-433898242 + Class minecraftServerClass = Reflection.getMinecraftClass("MinecraftServer"); + Object minecraftServer = Reflection.getField(minecraftServerClass, "SERVER", minecraftServerClass).get(null); + + return Reflection.getField(minecraftServerClass, "isStopped", boolean.class).get(minecraftServer); + } +} \ No newline at end of file diff --git a/Plan/bukkit/src/main/java/com/djrapitops/plan/Plan.java b/Plan/bukkit/src/main/java/com/djrapitops/plan/Plan.java index bd51404dd..db94f8b93 100644 --- a/Plan/bukkit/src/main/java/com/djrapitops/plan/Plan.java +++ b/Plan/bukkit/src/main/java/com/djrapitops/plan/Plan.java @@ -40,6 +40,7 @@ public class Plan extends BukkitPlugin implements PlanPlugin { private PlanSystem system; private Locale locale; + private ServerShutdownSave serverShutdownSave; @Override public void onEnable() { @@ -47,6 +48,7 @@ public class Plan extends BukkitPlugin implements PlanPlugin { try { timings.start("Enable"); system = component.system(); + serverShutdownSave = component.serverShutdownSave(); locale = system.getLocaleSystem().getLocale(); system.enable(); @@ -88,6 +90,10 @@ public class Plan extends BukkitPlugin implements PlanPlugin { */ @Override public void onDisable() { + if (serverShutdownSave != null) { + logger.info(locale != null ? locale.getString(PluginLang.DISABLED_UNSAVED_SESSIONS) : PluginLang.DISABLED_UNSAVED_SESSIONS.getDefault()); + serverShutdownSave.performSave(); + } if (system != null) { system.disable(); } diff --git a/Plan/bukkit/src/main/java/com/djrapitops/plan/PlanBukkitComponent.java b/Plan/bukkit/src/main/java/com/djrapitops/plan/PlanBukkitComponent.java index 98ccaaa71..60d1223aa 100644 --- a/Plan/bukkit/src/main/java/com/djrapitops/plan/PlanBukkitComponent.java +++ b/Plan/bukkit/src/main/java/com/djrapitops/plan/PlanBukkitComponent.java @@ -53,6 +53,8 @@ public interface PlanBukkitComponent { PlanSystem system(); + ServerShutdownSave serverShutdownSave(); + @Component.Builder interface Builder { diff --git a/Plan/bukkit/src/main/java/com/djrapitops/plan/modules/bukkit/BukkitSuperClassBindingModule.java b/Plan/bukkit/src/main/java/com/djrapitops/plan/modules/bukkit/BukkitSuperClassBindingModule.java index 6d7705b48..cab6c3125 100644 --- a/Plan/bukkit/src/main/java/com/djrapitops/plan/modules/bukkit/BukkitSuperClassBindingModule.java +++ b/Plan/bukkit/src/main/java/com/djrapitops/plan/modules/bukkit/BukkitSuperClassBindingModule.java @@ -16,6 +16,8 @@ */ package com.djrapitops.plan.modules.bukkit; +import com.djrapitops.plan.BukkitServerShutdownSave; +import com.djrapitops.plan.ServerShutdownSave; import com.djrapitops.plan.system.database.BukkitDBSystem; import com.djrapitops.plan.system.database.DBSystem; import com.djrapitops.plan.system.importing.BukkitImportSystem; @@ -55,6 +57,9 @@ public interface BukkitSuperClassBindingModule { ListenerSystem bindBukkitListenerSystem(BukkitListenerSystem bukkitListenerSystem); @Binds - ImportSystem bindImportSsytem(BukkitImportSystem bukkitImportSystem); + ImportSystem bindImportSystem(BukkitImportSystem bukkitImportSystem); + + @Binds + ServerShutdownSave bindBukkitServerShutdownSave(BukkitServerShutdownSave bukkitServerShutdownSave); } \ No newline at end of file diff --git a/Plan/bukkit/src/main/java/com/djrapitops/plan/system/tasks/BukkitTaskSystem.java b/Plan/bukkit/src/main/java/com/djrapitops/plan/system/tasks/BukkitTaskSystem.java index b670fe42f..40b92c79b 100644 --- a/Plan/bukkit/src/main/java/com/djrapitops/plan/system/tasks/BukkitTaskSystem.java +++ b/Plan/bukkit/src/main/java/com/djrapitops/plan/system/tasks/BukkitTaskSystem.java @@ -33,6 +33,7 @@ import com.djrapitops.plugin.task.RunnableFactory; import org.bukkit.Bukkit; import javax.inject.Inject; +import javax.inject.Singleton; import java.util.Optional; import java.util.concurrent.TimeUnit; @@ -41,6 +42,7 @@ import java.util.concurrent.TimeUnit; * * @author Rsl1122 */ +@Singleton public class BukkitTaskSystem extends ServerTaskSystem { private final Plan plugin; diff --git a/Plan/bungeecord/src/main/java/com/djrapitops/plan/system/tasks/BungeeTaskSystem.java b/Plan/bungeecord/src/main/java/com/djrapitops/plan/system/tasks/BungeeTaskSystem.java index e53ee7f59..84542e410 100644 --- a/Plan/bungeecord/src/main/java/com/djrapitops/plan/system/tasks/BungeeTaskSystem.java +++ b/Plan/bungeecord/src/main/java/com/djrapitops/plan/system/tasks/BungeeTaskSystem.java @@ -28,6 +28,7 @@ import com.djrapitops.plugin.api.TimeAmount; import com.djrapitops.plugin.task.RunnableFactory; import javax.inject.Inject; +import javax.inject.Singleton; import java.util.concurrent.TimeUnit; /** @@ -35,6 +36,7 @@ import java.util.concurrent.TimeUnit; * * @author Rsl1122 */ +@Singleton public class BungeeTaskSystem extends TaskSystem { private final PlanBungee plugin; diff --git a/Plan/common/src/main/java/com/djrapitops/plan/ServerShutdownSave.java b/Plan/common/src/main/java/com/djrapitops/plan/ServerShutdownSave.java new file mode 100644 index 000000000..34cba3eda --- /dev/null +++ b/Plan/common/src/main/java/com/djrapitops/plan/ServerShutdownSave.java @@ -0,0 +1,121 @@ +/* + * This file is part of Player Analytics (Plan). + * + * Plan is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License v3 as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Plan is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Plan. If not, see . + */ +package com.djrapitops.plan; + +import com.djrapitops.plan.api.exceptions.database.DBInitException; +import com.djrapitops.plan.api.exceptions.database.DBOpException; +import com.djrapitops.plan.data.container.Session; +import com.djrapitops.plan.data.store.keys.SessionKeys; +import com.djrapitops.plan.db.Database; +import com.djrapitops.plan.db.access.transactions.events.ServerShutdownTransaction; +import com.djrapitops.plan.system.cache.SessionCache; +import com.djrapitops.plan.system.database.DBSystem; +import com.djrapitops.plugin.logging.L; +import com.djrapitops.plugin.logging.error.ErrorHandler; + +import java.util.Map; +import java.util.Optional; +import java.util.UUID; +import java.util.concurrent.ExecutionException; + +/** + * Class in charge of performing save operations when the server shuts down. + * + * @author Rsl1122 + */ +public abstract class ServerShutdownSave { + + private final DBSystem dbSystem; + private final ErrorHandler errorHandler; + private boolean shuttingDown = false; + + public ServerShutdownSave( + DBSystem dbSystem, + ErrorHandler errorHandler + ) { + this.dbSystem = dbSystem; + this.errorHandler = errorHandler; + } + + protected abstract boolean checkServerShuttingDownStatus(); + + public void serverIsKnownToBeShuttingDown() { + shuttingDown = true; + } + + public void performSave() { + if (!checkServerShuttingDownStatus() && !shuttingDown) { + return; + } + + Map activeSessions = SessionCache.getActiveSessions(); + if (activeSessions.isEmpty()) { + return; + } + + attemptSave(activeSessions); + + SessionCache.clear(); + } + + private void attemptSave(Map activeSessions) { + try { + prepareSessionsForStorage(activeSessions, System.currentTimeMillis()); + saveActiveSessions(activeSessions); + } catch (DBInitException e) { + errorHandler.log(L.ERROR, this.getClass(), e); + } catch (IllegalStateException ignored) { + /* Database is not initialized */ + } finally { + closeDatabase(dbSystem.getDatabase()); + } + } + + private void saveActiveSessions(Map activeSessions) { + Database database = dbSystem.getDatabase(); + if (database.getState() == Database.State.CLOSED) { + // Ensure that database is not closed when performing the transaction. + database.init(); + } + + saveSessions(activeSessions, database); + } + + private void prepareSessionsForStorage(Map activeSessions, long now) { + for (Session session : activeSessions.values()) { + Optional end = session.getValue(SessionKeys.END); + if (!end.isPresent()) { + session.endSession(now); + } + } + } + + private void saveSessions(Map activeSessions, Database database) { + try { + database.executeTransaction(new ServerShutdownTransaction(activeSessions.values())) + .get(); // Ensure that the transaction is executed before shutdown. + } catch (ExecutionException | DBOpException e) { + errorHandler.log(L.ERROR, this.getClass(), e); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + + private void closeDatabase(Database database) { + database.close(); + } +} \ No newline at end of file diff --git a/Plan/common/src/main/java/com/djrapitops/plan/ShutdownHook.java b/Plan/common/src/main/java/com/djrapitops/plan/ShutdownHook.java index c5ce27c71..b10c621f0 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/ShutdownHook.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/ShutdownHook.java @@ -16,21 +16,8 @@ */ package com.djrapitops.plan; -import com.djrapitops.plan.api.exceptions.database.DBInitException; -import com.djrapitops.plan.api.exceptions.database.DBOpException; -import com.djrapitops.plan.data.container.Session; -import com.djrapitops.plan.data.store.keys.SessionKeys; -import com.djrapitops.plan.db.Database; -import com.djrapitops.plan.db.access.transactions.events.ServerShutdownTransaction; -import com.djrapitops.plan.system.cache.SessionCache; -import com.djrapitops.plan.system.database.DBSystem; -import com.djrapitops.plugin.logging.L; -import com.djrapitops.plugin.logging.error.ErrorHandler; - import javax.inject.Inject; -import java.util.Map; -import java.util.Optional; -import java.util.UUID; +import javax.inject.Singleton; /** * Thread that is run when JVM shuts down. @@ -39,16 +26,16 @@ import java.util.UUID; * * @author Rsl1122 */ +@Singleton public class ShutdownHook extends Thread { private static ShutdownHook activated; - private final DBSystem dbSystem; - private final ErrorHandler errorHandler; + + private final ServerShutdownSave serverShutdownSave; @Inject - public ShutdownHook(DBSystem dbSystem, ErrorHandler errorHandler) { - this.dbSystem = dbSystem; - this.errorHandler = errorHandler; + public ShutdownHook(ServerShutdownSave serverShutdownSave) { + this.serverShutdownSave = serverShutdownSave; } private static boolean isActivated() { @@ -74,47 +61,7 @@ public class ShutdownHook extends Thread { @Override public void run() { - try { - Map activeSessions = SessionCache.getActiveSessions(); - prepareSessionsForStorage(activeSessions, System.currentTimeMillis()); - saveActiveSessions(activeSessions); - } catch (DBInitException e) { - errorHandler.log(L.ERROR, this.getClass(), e); - } catch (IllegalStateException ignored) { - /* Database is not initialized */ - } finally { - closeDatabase(dbSystem.getDatabase()); - } - } - - private void saveActiveSessions(Map activeSessions) { - Database database = dbSystem.getDatabase(); - if (database.getState() == Database.State.CLOSED) { - // Ensure that database is not closed when performing the transaction. - database.init(); - } - - saveSessions(activeSessions, database); - } - - private void prepareSessionsForStorage(Map activeSessions, long now) { - for (Session session : activeSessions.values()) { - Optional end = session.getValue(SessionKeys.END); - if (!end.isPresent()) { - session.endSession(now); - } - } - } - - private void saveSessions(Map activeSessions, Database database) { - try { - database.executeTransaction(new ServerShutdownTransaction(activeSessions.values())); - } catch (DBOpException e) { - errorHandler.log(L.ERROR, this.getClass(), e); - } - } - - private void closeDatabase(Database database) { - database.close(); + serverShutdownSave.serverIsKnownToBeShuttingDown(); + serverShutdownSave.performSave(); } } diff --git a/Plan/common/src/main/java/com/djrapitops/plan/db/SQLiteDB.java b/Plan/common/src/main/java/com/djrapitops/plan/db/SQLiteDB.java index 22612ad77..2c4f7d4f5 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/db/SQLiteDB.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/db/SQLiteDB.java @@ -69,6 +69,8 @@ public class SQLiteDB extends SQLDB { @Override public void setupDataSource() { try { + if (connection != null) connection.close(); + connection = getNewConnection(databaseFile); } catch (SQLException e) { throw new DBInitException(e.getMessage(), e); @@ -139,11 +141,12 @@ public class SQLiteDB extends SQLDB { @Override public void close() { + logger.debug("SQLite Connection close prompted by: " + ThrowableUtils.findCallerAfterClass(Thread.currentThread().getStackTrace(), SQLiteDB.class)); + super.close(); stopConnectionPingTask(); if (connection != null) { - logger.debug("SQLite Connection close prompted by: " + ThrowableUtils.findCallerAfterClass(Thread.currentThread().getStackTrace(), SQLiteDB.class)); logger.debug("SQLite " + dbName + ": Closed Connection"); MiscUtils.close(connection); } diff --git a/Plan/common/src/main/java/com/djrapitops/plan/system/cache/CacheSystem.java b/Plan/common/src/main/java/com/djrapitops/plan/system/cache/CacheSystem.java index 87c5d0d89..593c3c668 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/system/cache/CacheSystem.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/system/cache/CacheSystem.java @@ -30,11 +30,17 @@ import javax.inject.Singleton; @Singleton public class CacheSystem implements SubSystem { + private final SessionCache sessionCache; private final NicknameCache nicknameCache; private final GeolocationCache geolocationCache; @Inject - public CacheSystem(NicknameCache nicknameCache, GeolocationCache geolocationCache) { + public CacheSystem( + SessionCache sessionCache, + NicknameCache nicknameCache, + GeolocationCache geolocationCache + ) { + this.sessionCache = sessionCache; this.nicknameCache = nicknameCache; this.geolocationCache = geolocationCache; } @@ -58,4 +64,7 @@ public class CacheSystem implements SubSystem { return geolocationCache; } + public SessionCache getSessionCache() { + return sessionCache; + } } diff --git a/Plan/common/src/main/java/com/djrapitops/plan/system/cache/SessionCache.java b/Plan/common/src/main/java/com/djrapitops/plan/system/cache/SessionCache.java index f3e961011..f1df66f91 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/system/cache/SessionCache.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/system/cache/SessionCache.java @@ -18,6 +18,7 @@ package com.djrapitops.plan.system.cache; import com.djrapitops.plan.data.container.Session; import com.djrapitops.plan.data.store.keys.SessionKeys; +import com.google.common.collect.ImmutableMap; import javax.inject.Inject; import javax.inject.Singleton; @@ -42,7 +43,7 @@ public class SessionCache { } public static Map getActiveSessions() { - return ACTIVE_SESSIONS; + return ImmutableMap.copyOf(ACTIVE_SESSIONS); } public static void clear() { diff --git a/Plan/common/src/main/java/com/djrapitops/plan/system/locale/lang/PluginLang.java b/Plan/common/src/main/java/com/djrapitops/plan/system/locale/lang/PluginLang.java index 153525d3f..8bc27a971 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/system/locale/lang/PluginLang.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/system/locale/lang/PluginLang.java @@ -49,6 +49,7 @@ public enum PluginLang implements Lang { DISABLED_WEB_SERVER("Disable - WebServer", "Webserver has been disabled."), DISABLED_PROCESSING("Disable - Processing", "Processing critical unprocessed tasks. (${0})"), DISABLED_PROCESSING_COMPLETE("Disable - Processing Complete", "Processing complete."), + DISABLED_UNSAVED_SESSIONS("Disable - Unsaved Session Save", "Saving unfinished sessions.."), VERSION_NEWEST("Version - Latest", "You're using the latest version."), VERSION_AVAILABLE("Version - New", "New Release (${0}) is available ${1}"), diff --git a/Plan/common/src/main/java/com/djrapitops/plan/system/processing/Processing.java b/Plan/common/src/main/java/com/djrapitops/plan/system/processing/Processing.java index 6de27abec..85e5dea18 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/system/processing/Processing.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/system/processing/Processing.java @@ -16,7 +16,6 @@ */ package com.djrapitops.plan.system.processing; -import com.djrapitops.plan.api.exceptions.EnableException; import com.djrapitops.plan.system.SubSystem; import com.djrapitops.plan.system.locale.Locale; import com.djrapitops.plan.system.locale.lang.PluginLang; @@ -38,8 +37,8 @@ public class Processing implements SubSystem { private final PluginLogger logger; private final ErrorHandler errorHandler; - private final ExecutorService nonCriticalExecutor; - private final ExecutorService criticalExecutor; + private ExecutorService nonCriticalExecutor; + private ExecutorService criticalExecutor; @Inject public Processing( @@ -50,8 +49,12 @@ public class Processing implements SubSystem { this.locale = locale; this.logger = logger; this.errorHandler = errorHandler; - nonCriticalExecutor = Executors.newFixedThreadPool(6, new ThreadFactoryBuilder().setNameFormat("Plan Non critical-pool-%d").build()); - criticalExecutor = Executors.newFixedThreadPool(2, new ThreadFactoryBuilder().setNameFormat("Plan Critical-pool-%d").build()); + nonCriticalExecutor = createExecutor(6, "Plan Non critical-pool-%d"); + criticalExecutor = createExecutor(2, "Plan Critical-pool-%d"); + } + + private ExecutorService createExecutor(int i, String s) { + return Executors.newFixedThreadPool(i, new ThreadFactoryBuilder().setNameFormat(s).build()); } public void submit(Runnable runnable) { @@ -126,18 +129,28 @@ public class Processing implements SubSystem { } @Override - public void enable() throws EnableException { + public void enable() { if (nonCriticalExecutor.isShutdown()) { - throw new EnableException("Non Critical ExecutorService was shut down on enable"); + nonCriticalExecutor = createExecutor(6, "Plan Non critical-pool-%d"); } if (criticalExecutor.isShutdown()) { - throw new EnableException("Critical ExecutorService was shut down on enable"); + criticalExecutor = createExecutor(2, "Plan Critical-pool-%d"); } } @Override public void disable() { + shutdownNonCriticalExecutor(); + shutdownCriticalExecutor(); + ensureShutdown(); + logger.info(locale.get().getString(PluginLang.DISABLED_PROCESSING_COMPLETE)); + } + + private void shutdownNonCriticalExecutor() { nonCriticalExecutor.shutdown(); + } + + private void shutdownCriticalExecutor() { List criticalTasks = criticalExecutor.shutdownNow(); logger.info(locale.get().getString(PluginLang.DISABLED_PROCESSING, criticalTasks.size())); for (Runnable runnable : criticalTasks) { @@ -148,6 +161,9 @@ public class Processing implements SubSystem { errorHandler.log(L.WARN, this.getClass(), e); } } + } + + private void ensureShutdown() { try { if (!nonCriticalExecutor.isTerminated() && !nonCriticalExecutor.awaitTermination(1, TimeUnit.SECONDS)) { nonCriticalExecutor.shutdownNow(); @@ -161,6 +177,5 @@ public class Processing implements SubSystem { criticalExecutor.shutdownNow(); Thread.currentThread().interrupt(); } - logger.info(locale.get().getString(PluginLang.DISABLED_PROCESSING_COMPLETE)); } } diff --git a/Plan/common/src/test/java/com/djrapitops/plan/ShutdownSaveTest.java b/Plan/common/src/test/java/com/djrapitops/plan/ShutdownSaveTest.java new file mode 100644 index 000000000..5aba8a26f --- /dev/null +++ b/Plan/common/src/test/java/com/djrapitops/plan/ShutdownSaveTest.java @@ -0,0 +1,155 @@ +/* + * This file is part of Player Analytics (Plan). + * + * Plan is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License v3 as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Plan is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Plan. If not, see . + */ +package com.djrapitops.plan; + +import com.djrapitops.plan.data.container.Session; +import com.djrapitops.plan.data.time.GMTimes; +import com.djrapitops.plan.db.Database; +import com.djrapitops.plan.db.access.queries.objects.SessionQueries; +import com.djrapitops.plan.db.access.transactions.StoreServerInformationTransaction; +import com.djrapitops.plan.db.access.transactions.commands.RemoveEverythingTransaction; +import com.djrapitops.plan.db.access.transactions.events.PlayerRegisterTransaction; +import com.djrapitops.plan.db.access.transactions.events.WorldNameStoreTransaction; +import com.djrapitops.plan.system.cache.SessionCache; +import com.djrapitops.plan.system.database.DBSystem; +import com.djrapitops.plan.system.info.server.Server; +import com.djrapitops.plugin.logging.console.TestPluginLogger; +import com.djrapitops.plugin.logging.error.ConsoleErrorLogger; +import extension.PrintExtension; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.io.TempDir; +import org.junit.platform.runner.JUnitPlatform; +import org.junit.runner.RunWith; +import utilities.TestConstants; +import utilities.dagger.DaggerPlanPluginComponent; +import utilities.dagger.PlanPluginComponent; +import utilities.mocks.PlanPluginMocker; + +import java.nio.file.Path; +import java.util.UUID; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * Test ensures that unsaved sessions are saved on server shutdown. + * + * @author Rsl1122 + */ +@RunWith(JUnitPlatform.class) +@ExtendWith(PrintExtension.class) +public class ShutdownSaveTest { + + private boolean shutdownStatus; + private ServerShutdownSave underTest; + private Database database; + private SessionCache sessionCache; + + @BeforeEach + void setupShutdownSaveObject(@TempDir Path temporaryFolder) throws Exception { + PlanPluginComponent pluginComponent = DaggerPlanPluginComponent.builder().plan( + PlanPluginMocker.setUp() + .withDataFolder(temporaryFolder.resolve("ShutdownSaveTest").toFile()) + .withLogging() + .getPlanMock() + ).build(); + + database = pluginComponent.system().getDatabaseSystem().getSqLiteFactory().usingFileCalled("test"); + database.init(); + + sessionCache = pluginComponent.system().getCacheSystem().getSessionCache(); + + storeNecessaryInformation(); + placeSessionToCache(); + + DBSystem dbSystemMock = mock(DBSystem.class); + when(dbSystemMock.getDatabase()).thenReturn(database); + + underTest = new ServerShutdownSave(dbSystemMock, new ConsoleErrorLogger(new TestPluginLogger())) { + @Override + protected boolean checkServerShuttingDownStatus() { + return shutdownStatus; + } + }; + + shutdownStatus = false; + } + + @AfterEach + void tearDownPluginComponent() { + database.close(); + SessionCache.clear(); + } + + private void storeNecessaryInformation() throws Exception { + database.executeTransaction(new RemoveEverythingTransaction()); + + UUID serverUUID = TestConstants.SERVER_UUID; + UUID playerUUID = TestConstants.PLAYER_ONE_UUID; + String worldName = TestConstants.WORLD_ONE_NAME; + + database.executeTransaction(new StoreServerInformationTransaction(new Server(-1, serverUUID, "-", "", 0))); + database.executeTransaction(new PlayerRegisterTransaction(playerUUID, () -> 0L, TestConstants.PLAYER_ONE_NAME)); + database.executeTransaction(new WorldNameStoreTransaction(serverUUID, worldName)) + .get(); + } + + @Test + void sessionsAreNotSavedOnReload() { + shutdownStatus = false; + underTest.performSave(); + + database.init(); + assertTrue(database.query(SessionQueries.fetchAllSessions()).isEmpty()); + database.close(); + } + + @Test + void sessionsAreSavedOnServerShutdown() { + shutdownStatus = true; + underTest.performSave(); + + database.init(); + assertFalse(database.query(SessionQueries.fetchAllSessions()).isEmpty()); + database.close(); + } + + @Test + void sessionsAreSavedOnJVMShutdown() { + ShutdownHook shutdownHook = new ShutdownHook(underTest); + shutdownHook.run(); + + database.init(); + assertFalse(database.query(SessionQueries.fetchAllSessions()).isEmpty()); + database.close(); + } + + private void placeSessionToCache() { + UUID serverUUID = TestConstants.SERVER_UUID; + UUID playerUUID = TestConstants.PLAYER_ONE_UUID; + String worldName = TestConstants.WORLD_ONE_NAME; + + Session session = new Session(playerUUID, serverUUID, 0L, worldName, GMTimes.getGMKeyArray()[0]); + + sessionCache.cacheSession(playerUUID, session); + } +} \ No newline at end of file diff --git a/Plan/common/src/test/java/extension/PrintExtension.java b/Plan/common/src/test/java/extension/PrintExtension.java new file mode 100644 index 000000000..7684a76d6 --- /dev/null +++ b/Plan/common/src/test/java/extension/PrintExtension.java @@ -0,0 +1,43 @@ +/* + * This file is part of Player Analytics (Plan). + * + * Plan is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License v3 as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Plan is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Plan. If not, see . + */ +package extension; + +import org.junit.jupiter.api.extension.AfterTestExecutionCallback; +import org.junit.jupiter.api.extension.BeforeTestExecutionCallback; +import org.junit.jupiter.api.extension.ExtensionContext; + +import java.lang.reflect.Method; + +/** + * JUnit 5 Extension that prints what test is being run before each test. + * + * @author Rsl1122 + */ +public class PrintExtension implements BeforeTestExecutionCallback, AfterTestExecutionCallback { + + @Override + public void beforeTestExecution(ExtensionContext context) throws Exception { + String testName = context.getTestClass().map(Class::getSimpleName).orElse("?"); + String testMethodName = context.getTestMethod().map(Method::getName).orElse("?"); + System.out.println(">> " + testName + " - " + testMethodName); + } + + @Override + public void afterTestExecution(ExtensionContext context) throws Exception { + System.out.println(); + } +} \ No newline at end of file diff --git a/Plan/common/src/test/java/utilities/TestConstants.java b/Plan/common/src/test/java/utilities/TestConstants.java index a19daaccd..bb94d5aac 100644 --- a/Plan/common/src/test/java/utilities/TestConstants.java +++ b/Plan/common/src/test/java/utilities/TestConstants.java @@ -31,6 +31,8 @@ public class TestConstants { public static final String PLAYER_ONE_NAME = "Test_Player_one"; + public static final String WORLD_ONE_NAME = "World One"; + public static final int BUKKIT_MAX_PLAYERS = 20; public static final int BUNGEE_MAX_PLAYERS = 100; diff --git a/Plan/common/src/test/java/utilities/TestResources.java b/Plan/common/src/test/java/utilities/TestResources.java index ece77ef59..3ee084702 100644 --- a/Plan/common/src/test/java/utilities/TestResources.java +++ b/Plan/common/src/test/java/utilities/TestResources.java @@ -22,6 +22,7 @@ import java.io.*; import java.net.URI; import java.net.URISyntaxException; import java.net.URL; +import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -53,8 +54,10 @@ public class TestResources { } private static void copyResourceToFile(File toFile, InputStream testResource) { - try (InputStream in = testResource; - OutputStream out = new FileOutputStream(toFile)) { + try ( + InputStream in = testResource; + OutputStream out = Files.newOutputStream(toFile.toPath()) + ) { copy(in, out); } catch (IOException e) { throw new UncheckedIOException(e); @@ -62,8 +65,10 @@ public class TestResources { } private static void writeResourceToFile(File toFile, String resourcePath) { - try (InputStream in = PlanPlugin.class.getResourceAsStream(resourcePath); - OutputStream out = new FileOutputStream(toFile)) { + try ( + InputStream in = PlanPlugin.class.getResourceAsStream(resourcePath); + OutputStream out = Files.newOutputStream(toFile.toPath()) + ) { if (in == null) { throw new FileNotFoundException("Resource with name '" + resourcePath + "' not found"); } @@ -83,11 +88,10 @@ public class TestResources { } private static void createEmptyFile(File toFile) { - String path = toFile.getAbsolutePath(); try { - toFile.getParentFile().mkdirs(); - if (!toFile.exists() && !toFile.createNewFile()) { - throw new FileNotFoundException("Could not create file: " + path); + Files.createDirectories(toFile.toPath().getParent()); + if (!toFile.exists()) { + Files.createFile(toFile.toPath()); } } catch (IOException e) { throw new UncheckedIOException(e); diff --git a/Plan/common/src/test/java/utilities/mocks/Mocker.java b/Plan/common/src/test/java/utilities/mocks/Mocker.java index 4a72cc59f..b5bb39d9c 100644 --- a/Plan/common/src/test/java/utilities/mocks/Mocker.java +++ b/Plan/common/src/test/java/utilities/mocks/Mocker.java @@ -17,8 +17,11 @@ package utilities.mocks; import com.djrapitops.plan.PlanPlugin; +import utilities.TestResources; -import java.io.*; +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; import static org.mockito.Mockito.doReturn; @@ -34,42 +37,23 @@ abstract class Mocker { File getFile(String fileName) { // Read the resource from jar to a temporary file File file = new File(new File(planMock.getDataFolder(), "jar"), fileName); - try { - file.getParentFile().mkdirs(); - if (!file.exists() && !file.createNewFile()) { - throw new FileNotFoundException("Could not create file: " + fileName); - } - } catch (IOException e) { - throw new UncheckedIOException(e); - } - try (InputStream in = PlanPlugin.class.getResourceAsStream(fileName); - OutputStream out = new FileOutputStream(file)) { - - int read; - byte[] bytes = new byte[1024]; - - while ((read = in.read(bytes)) != -1) { - out.write(bytes, 0, read); - } - } catch (IOException e) { - throw new UncheckedIOException(e); - } + TestResources.copyResourceIntoFile(file, fileName); return file; } - private void withPluginFile(String fileName) throws FileNotFoundException { + private void withPluginFile(String fileName) throws IOException { if (planMock.getDataFolder() == null) { throw new IllegalStateException("withDataFolder needs to be called before setting files"); } try { File file = getFile("/" + fileName); - doReturn(new FileInputStream(file)).when(planMock).getResource(fileName); + doReturn(Files.newInputStream(file.toPath())).when(planMock).getResource(fileName); } catch (NullPointerException e) { System.out.println("File is missing! " + fileName); } } - void withPluginFiles() throws FileNotFoundException { + void withPluginFiles() throws IOException { for (String fileName : new String[]{ "bungeeconfig.yml", "config.yml", diff --git a/Plan/sponge/src/main/java/com/djrapitops/plan/PlanSponge.java b/Plan/sponge/src/main/java/com/djrapitops/plan/PlanSponge.java index 0560b94f4..4ec7ee461 100644 --- a/Plan/sponge/src/main/java/com/djrapitops/plan/PlanSponge.java +++ b/Plan/sponge/src/main/java/com/djrapitops/plan/PlanSponge.java @@ -63,6 +63,7 @@ public class PlanSponge extends SpongePlugin implements PlanPlugin { private File dataFolder; private PlanSystem system; private Locale locale; + private ServerShutdownSave serverShutdownSave; @Listener public void onServerStart(GameStartedServerEvent event) { @@ -79,6 +80,7 @@ public class PlanSponge extends SpongePlugin implements PlanPlugin { PlanSpongeComponent component = DaggerPlanSpongeComponent.builder().plan(this).build(); try { system = component.system(); + serverShutdownSave = component.serverShutdownSave(); locale = system.getLocaleSystem().getLocale(); system.enable(); @@ -112,6 +114,9 @@ public class PlanSponge extends SpongePlugin implements PlanPlugin { @Override public void onDisable() { + if (serverShutdownSave != null) { + serverShutdownSave.performSave(); + } if (system != null) { system.disable(); } diff --git a/Plan/sponge/src/main/java/com/djrapitops/plan/PlanSpongeComponent.java b/Plan/sponge/src/main/java/com/djrapitops/plan/PlanSpongeComponent.java index db8c01d30..c1bdf7ab4 100644 --- a/Plan/sponge/src/main/java/com/djrapitops/plan/PlanSpongeComponent.java +++ b/Plan/sponge/src/main/java/com/djrapitops/plan/PlanSpongeComponent.java @@ -53,6 +53,8 @@ public interface PlanSpongeComponent { PlanSystem system(); + ServerShutdownSave serverShutdownSave(); + @Component.Builder interface Builder { diff --git a/Plan/sponge/src/main/java/com/djrapitops/plan/SpongeServerShutdownSave.java b/Plan/sponge/src/main/java/com/djrapitops/plan/SpongeServerShutdownSave.java new file mode 100644 index 000000000..d74dafdb3 --- /dev/null +++ b/Plan/sponge/src/main/java/com/djrapitops/plan/SpongeServerShutdownSave.java @@ -0,0 +1,57 @@ +/* + * This file is part of Player Analytics (Plan). + * + * Plan is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License v3 as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Plan is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Plan. If not, see . + */ +package com.djrapitops.plan; + +import com.djrapitops.plan.system.database.DBSystem; +import com.djrapitops.plugin.logging.error.ErrorHandler; +import org.spongepowered.api.GameState; +import org.spongepowered.api.event.Listener; +import org.spongepowered.api.event.Order; +import org.spongepowered.api.event.game.state.GameStoppingServerEvent; + +import javax.inject.Inject; +import javax.inject.Singleton; + +/** + * ServerShutdownSave implementation for Sponge + * + * @author Rsl1122 + */ +@Singleton +public class SpongeServerShutdownSave extends ServerShutdownSave { + + private boolean shuttingDown = false; + + @Inject + public SpongeServerShutdownSave(DBSystem dbSystem, ErrorHandler errorHandler) { + super(dbSystem, errorHandler); + } + + @Override + protected boolean checkServerShuttingDownStatus() { + return shuttingDown; + } + + @Listener(order = Order.PRE) + public void onServerShutdown(GameStoppingServerEvent event) { + GameState state = event.getState(); + shuttingDown = state == GameState.SERVER_STOPPING + || state == GameState.GAME_STOPPING + || state == GameState.SERVER_STOPPED + || state == GameState.GAME_STOPPED; + } +} \ No newline at end of file diff --git a/Plan/sponge/src/main/java/com/djrapitops/plan/modules/sponge/SpongeSuperClassBindingModule.java b/Plan/sponge/src/main/java/com/djrapitops/plan/modules/sponge/SpongeSuperClassBindingModule.java index 7b62a6d17..01d61688d 100644 --- a/Plan/sponge/src/main/java/com/djrapitops/plan/modules/sponge/SpongeSuperClassBindingModule.java +++ b/Plan/sponge/src/main/java/com/djrapitops/plan/modules/sponge/SpongeSuperClassBindingModule.java @@ -16,6 +16,8 @@ */ package com.djrapitops.plan.modules.sponge; +import com.djrapitops.plan.ServerShutdownSave; +import com.djrapitops.plan.SpongeServerShutdownSave; import com.djrapitops.plan.system.database.DBSystem; import com.djrapitops.plan.system.database.SpongeDBSystem; import com.djrapitops.plan.system.importing.EmptyImportSystem; @@ -57,4 +59,7 @@ public interface SpongeSuperClassBindingModule { @Binds ImportSystem bindImportSystem(EmptyImportSystem emptyImportSystem); + @Binds + ServerShutdownSave bindSpongeServerShutdownSave(SpongeServerShutdownSave spongeServerShutdownSave); + } \ No newline at end of file diff --git a/Plan/sponge/src/main/java/com/djrapitops/plan/system/listeners/SpongeListenerSystem.java b/Plan/sponge/src/main/java/com/djrapitops/plan/system/listeners/SpongeListenerSystem.java index 09d1c1394..173eddf22 100644 --- a/Plan/sponge/src/main/java/com/djrapitops/plan/system/listeners/SpongeListenerSystem.java +++ b/Plan/sponge/src/main/java/com/djrapitops/plan/system/listeners/SpongeListenerSystem.java @@ -18,6 +18,7 @@ package com.djrapitops.plan.system.listeners; import com.djrapitops.plan.PlanPlugin; import com.djrapitops.plan.PlanSponge; +import com.djrapitops.plan.SpongeServerShutdownSave; import com.djrapitops.plan.api.events.PlanSpongeEnableEvent; import com.djrapitops.plan.system.listeners.sponge.*; import org.spongepowered.api.Sponge; @@ -36,16 +37,19 @@ public class SpongeListenerSystem extends ListenerSystem { private final SpongeGMChangeListener gmChangeListener; private final SpongePlayerListener playerListener; private final SpongeWorldChangeListener worldChangeListener; + private final SpongeServerShutdownSave spongeServerShutdownSave; @Inject - public SpongeListenerSystem(PlanSponge plugin, - SpongeAFKListener afkListener, - SpongeChatListener chatListener, - SpongeCommandListener commandListener, - SpongeDeathListener deathListener, - SpongeGMChangeListener gmChangeListener, - SpongePlayerListener playerListener, - SpongeWorldChangeListener worldChangeListener + public SpongeListenerSystem( + PlanSponge plugin, + SpongeAFKListener afkListener, + SpongeChatListener chatListener, + SpongeCommandListener commandListener, + SpongeDeathListener deathListener, + SpongeGMChangeListener gmChangeListener, + SpongePlayerListener playerListener, + SpongeWorldChangeListener worldChangeListener, + SpongeServerShutdownSave spongeServerShutdownSave ) { this.plugin = plugin; @@ -56,6 +60,7 @@ public class SpongeListenerSystem extends ListenerSystem { this.gmChangeListener = gmChangeListener; this.playerListener = playerListener; this.worldChangeListener = worldChangeListener; + this.spongeServerShutdownSave = spongeServerShutdownSave; } @Override @@ -67,7 +72,8 @@ public class SpongeListenerSystem extends ListenerSystem { deathListener, playerListener, gmChangeListener, - worldChangeListener + worldChangeListener, + spongeServerShutdownSave ); } diff --git a/Plan/sponge/src/main/java/com/djrapitops/plan/system/tasks/SpongeTaskSystem.java b/Plan/sponge/src/main/java/com/djrapitops/plan/system/tasks/SpongeTaskSystem.java index 57606b27e..517b8ec35 100644 --- a/Plan/sponge/src/main/java/com/djrapitops/plan/system/tasks/SpongeTaskSystem.java +++ b/Plan/sponge/src/main/java/com/djrapitops/plan/system/tasks/SpongeTaskSystem.java @@ -32,8 +32,10 @@ import org.spongepowered.api.Sponge; import org.spongepowered.api.scheduler.Task; import javax.inject.Inject; +import javax.inject.Singleton; import java.util.concurrent.TimeUnit; +@Singleton public class SpongeTaskSystem extends ServerTaskSystem { private final PlanSponge plugin; diff --git a/Plan/velocity/src/main/java/com/djrapitops/plan/system/tasks/VelocityTaskSystem.java b/Plan/velocity/src/main/java/com/djrapitops/plan/system/tasks/VelocityTaskSystem.java index f9502db37..bd09a6b07 100644 --- a/Plan/velocity/src/main/java/com/djrapitops/plan/system/tasks/VelocityTaskSystem.java +++ b/Plan/velocity/src/main/java/com/djrapitops/plan/system/tasks/VelocityTaskSystem.java @@ -28,6 +28,7 @@ import com.djrapitops.plugin.api.TimeAmount; import com.djrapitops.plugin.task.RunnableFactory; import javax.inject.Inject; +import javax.inject.Singleton; import java.util.concurrent.TimeUnit; /** @@ -35,6 +36,7 @@ import java.util.concurrent.TimeUnit; * * @author Rsl1122 */ +@Singleton public class VelocityTaskSystem extends TaskSystem { private final PlanVelocity plugin;