From 95b128d73dc9ef1778d3c093b70d66401b126bb0 Mon Sep 17 00:00:00 2001 From: Christian Koop Date: Tue, 24 Oct 2023 03:00:27 +0200 Subject: [PATCH] fix: Properly migrate the database related code to the new core's API See previously commits for context tl;dr The database migration introduced a lot of changed behaviour and breaking changes. The plugin essentially could never have worked and nobody reported the issue (or tested it). I've completely redone the code migration, keeping changes for the plugin internals to a minimum. Hopefully I didn't overlook anything... --- EpicAnchors-Plugin/pom.xml | 1 + .../epicanchors/AnchorManagerImpl.java | 10 +- .../com/craftaro/epicanchors/EpicAnchors.java | 31 +++--- ...taManager.java => AnchorsDataManager.java} | 99 +++++++++---------- ...on.java => LegacyYamlAnchorsMigrator.java} | 23 ++--- .../files/migration/_1_InitialMigration.java | 23 ++--- .../epicanchors/listener/AnchorListener.java | 2 +- 7 files changed, 90 insertions(+), 99 deletions(-) rename EpicAnchors-Plugin/src/main/java/com/craftaro/epicanchors/files/{DataManager.java => AnchorsDataManager.java} (72%) rename EpicAnchors-Plugin/src/main/java/com/craftaro/epicanchors/files/migration/{AnchorMigration.java => LegacyYamlAnchorsMigrator.java} (85%) diff --git a/EpicAnchors-Plugin/pom.xml b/EpicAnchors-Plugin/pom.xml index 1d44dfc..e7c07dd 100644 --- a/EpicAnchors-Plugin/pom.xml +++ b/EpicAnchors-Plugin/pom.xml @@ -56,6 +56,7 @@ false **/nms/v*/** + **/core/third_party/org/h2/** diff --git a/EpicAnchors-Plugin/src/main/java/com/craftaro/epicanchors/AnchorManagerImpl.java b/EpicAnchors-Plugin/src/main/java/com/craftaro/epicanchors/AnchorManagerImpl.java index 7c19a56..94f69b4 100644 --- a/EpicAnchors-Plugin/src/main/java/com/craftaro/epicanchors/AnchorManagerImpl.java +++ b/EpicAnchors-Plugin/src/main/java/com/craftaro/epicanchors/AnchorManagerImpl.java @@ -12,7 +12,7 @@ import com.craftaro.core.utils.TimeUtils; import com.craftaro.epicanchors.api.Anchor; import com.craftaro.epicanchors.api.AnchorAccessCheck; import com.craftaro.epicanchors.api.AnchorManager; -import com.craftaro.epicanchors.files.DataManager; +import com.craftaro.epicanchors.files.AnchorsDataManager; import com.craftaro.epicanchors.files.Settings; import com.craftaro.epicanchors.utils.Callback; import com.craftaro.epicanchors.utils.UpdateCallback; @@ -47,7 +47,7 @@ public class AnchorManagerImpl implements AnchorManager { private static final String HOLOGRAM_PREFIX = "Anchor_"; private final SongodaPlugin plugin; - private final DataManager dataManager; + private final AnchorsDataManager dataManager; private final Map> anchors = new HashMap<>(3); private final Set visualizedChunk = new HashSet<>(); @@ -55,7 +55,7 @@ public class AnchorManagerImpl implements AnchorManager { private boolean ready; - public AnchorManagerImpl(SongodaPlugin plugin, DataManager dataManager) { + public AnchorManagerImpl(SongodaPlugin plugin, AnchorsDataManager dataManager) { this.plugin = Objects.requireNonNull(plugin); this.dataManager = Objects.requireNonNull(dataManager); } @@ -105,7 +105,7 @@ public class AnchorManagerImpl implements AnchorManager { if (callback != null) { callback.accept(ex); } else { - Utils.logException(this.plugin, ex, "SQLite"); + Utils.logException(this.plugin, ex, "H2"); } } }); @@ -234,7 +234,7 @@ public class AnchorManagerImpl implements AnchorManager { if (callback != null) { callback.accept(ex, null); } else { - Utils.logException(this.plugin, ex, "SQLite"); + Utils.logException(this.plugin, ex, "H2"); } } else { Bukkit.getScheduler().runTask(this.plugin, () -> { diff --git a/EpicAnchors-Plugin/src/main/java/com/craftaro/epicanchors/EpicAnchors.java b/EpicAnchors-Plugin/src/main/java/com/craftaro/epicanchors/EpicAnchors.java index 154089e..88df5a2 100644 --- a/EpicAnchors-Plugin/src/main/java/com/craftaro/epicanchors/EpicAnchors.java +++ b/EpicAnchors-Plugin/src/main/java/com/craftaro/epicanchors/EpicAnchors.java @@ -4,8 +4,6 @@ import com.craftaro.core.SongodaCore; import com.craftaro.core.SongodaPlugin; import com.craftaro.core.commands.CommandManager; import com.craftaro.core.configuration.Config; -import com.craftaro.core.database.DatabaseConnector; -import com.craftaro.core.database.SQLiteConnector; import com.craftaro.core.gui.GuiManager; import com.craftaro.core.hooks.EconomyManager; import com.craftaro.core.hooks.HologramManager; @@ -16,9 +14,9 @@ import com.craftaro.epicanchors.commands.sub.GiveCommand; import com.craftaro.epicanchors.commands.sub.ReloadCommand; import com.craftaro.epicanchors.commands.sub.SettingsCommand; import com.craftaro.epicanchors.commands.sub.ShowCommand; -import com.craftaro.epicanchors.files.DataManager; +import com.craftaro.epicanchors.files.AnchorsDataManager; import com.craftaro.epicanchors.files.Settings; -import com.craftaro.epicanchors.files.migration.AnchorMigration; +import com.craftaro.epicanchors.files.migration.LegacyYamlAnchorsMigrator; import com.craftaro.epicanchors.files.migration._1_InitialMigration; import com.craftaro.epicanchors.listener.AnchorListener; import com.craftaro.epicanchors.listener.BlockListener; @@ -38,7 +36,7 @@ public final class EpicAnchors extends SongodaPlugin { private GuiManager guiManager; private AnchorManagerImpl anchorManager; - private DataManager dataManager; + private AnchorsDataManager dataManager; @Override public void onPluginLoad() { @@ -48,14 +46,8 @@ public final class EpicAnchors extends SongodaPlugin { public void onPluginEnable() { SongodaCore.registerPlugin(this, 31, XMaterial.END_PORTAL_FRAME); - // Initialize database - this.getLogger().info("Initializing SQLite..."); - DatabaseConnector dbCon = new SQLiteConnector(this); - this.dataManager = new DataManager(dbCon, this); - AnchorMigration anchorMigration = new AnchorMigration(dbCon, this.dataManager, new _1_InitialMigration()); - anchorMigration.runMigrations(); + initializeDataManager(); - anchorMigration.migrateLegacyData(this); this.anchorManager = new AnchorManagerImpl(this, this.dataManager); EpicAnchorsApi.initApi(this.anchorManager); @@ -150,4 +142,19 @@ public final class EpicAnchors extends SongodaPlugin { public AnchorManager getAnchorManager() { return this.anchorManager; } + + @Override + public Config getDatabaseConfig() { + Config staticDatabaseConfig = new Config(); + staticDatabaseConfig.set("Connection Settings.Type", "H2"); + staticDatabaseConfig.set("Connection Settings.Pool Size", 1); + return staticDatabaseConfig; + } + + private void initializeDataManager() { + super.initDatabase(new _1_InitialMigration()); + + this.dataManager = new AnchorsDataManager(this); + LegacyYamlAnchorsMigrator.migrateLegacyData(this, this.dataManager); + } } diff --git a/EpicAnchors-Plugin/src/main/java/com/craftaro/epicanchors/files/DataManager.java b/EpicAnchors-Plugin/src/main/java/com/craftaro/epicanchors/files/AnchorsDataManager.java similarity index 72% rename from EpicAnchors-Plugin/src/main/java/com/craftaro/epicanchors/files/DataManager.java rename to EpicAnchors-Plugin/src/main/java/com/craftaro/epicanchors/files/AnchorsDataManager.java index 1daf7c0..999c595 100644 --- a/EpicAnchors-Plugin/src/main/java/com/craftaro/epicanchors/files/DataManager.java +++ b/EpicAnchors-Plugin/src/main/java/com/craftaro/epicanchors/files/AnchorsDataManager.java @@ -1,17 +1,16 @@ package com.craftaro.epicanchors.files; -import com.craftaro.core.database.DataManagerAbstract; +import com.craftaro.core.SongodaPlugin; import com.craftaro.core.database.DatabaseConnector; import com.craftaro.epicanchors.AnchorImpl; import com.craftaro.epicanchors.api.Anchor; -import com.craftaro.epicanchors.files.migration.AnchorMigration; +import com.craftaro.epicanchors.files.migration.LegacyYamlAnchorsMigrator; import com.craftaro.epicanchors.utils.Callback; import com.craftaro.epicanchors.utils.UpdateCallback; import com.craftaro.epicanchors.utils.Utils; import org.bukkit.Bukkit; import org.bukkit.Location; import org.bukkit.World; -import org.bukkit.plugin.Plugin; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -28,38 +27,35 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; -public class DataManager extends DataManagerAbstract { +public class AnchorsDataManager { private final ExecutorService thread = Executors.newSingleThreadExecutor(); + private final SongodaPlugin plugin; - private final String anchorTable; - - public DataManager(DatabaseConnector databaseConnector, Plugin plugin) { - super(databaseConnector, plugin); - - this.anchorTable = getTableName(super.getTablePrefix(), "anchors"); + public AnchorsDataManager(SongodaPlugin plugin) { + this.plugin = plugin; } public void close() { - if (!this.thread.isShutdown()) { - this.thread.shutdown(); - - try { - if (!this.thread.awaitTermination(60, TimeUnit.SECONDS)) { - // Try stopping the thread forcefully (there is basically no hope left for the data) - this.thread.shutdownNow(); - } - } catch (InterruptedException ex) { - Utils.logException(super.plugin, ex); - } - - this.databaseConnector.closeConnection(); + if (this.thread.isShutdown()) { + return; } + + this.thread.shutdown(); + try { + if (!this.thread.awaitTermination(60, TimeUnit.SECONDS)) { + // Try stopping the thread forcefully (there is basically no hope left for the data) + this.thread.shutdownNow(); + } + } catch (InterruptedException ex) { + Utils.logException(this.plugin, ex); + } + + this.plugin.getDataManager().shutdown(); } public void exists(@NotNull String worldName, int x, int y, int z, @NotNull Callback callback) { - this.databaseConnector.connect((con) -> { - try (PreparedStatement ps = con.prepareStatement("SELECT id FROM " + this.anchorTable + - " WHERE world_name =? AND x =? AND y =? AND z=?;")) { + getDatabaseConnector().connect((con) -> { + try (PreparedStatement ps = con.prepareStatement("SELECT id FROM " + getAnchorTable() + " WHERE world_name =? AND x =? AND y =? AND z=?;")) { ps.setString(1, worldName); ps.setInt(2, x); ps.setInt(3, y); @@ -77,9 +73,8 @@ public class DataManager extends DataManagerAbstract { public void getAnchors(@Nullable World world, @NotNull Callback> callback) { List result = new ArrayList<>(); - this.databaseConnector.connect((con) -> { - try (PreparedStatement ps = con.prepareStatement("SELECT * FROM " + this.anchorTable + - (world != null ? " WHERE world_name =?" : "") + ";")) { + getDatabaseConnector().connect((con) -> { + try (PreparedStatement ps = con.prepareStatement("SELECT * FROM " + getAnchorTable() + (world != null ? " WHERE world_name =?" : "") + ";")) { if (world != null) { ps.setString(1, world.getName()); } @@ -102,11 +97,10 @@ public class DataManager extends DataManagerAbstract { } public void insertAnchor(Location loc, UUID owner, int ticks, Callback callback) { - this.databaseConnector.connect((con) -> { - try (PreparedStatement ps = con.prepareStatement("INSERT INTO " + this.anchorTable + - "(owner, world_name,x,y,z, ticks_left) VALUES (?,?,?,?,?, ?);");// Future SQLite version might support 'RETURNING *' - PreparedStatement psFetch = con.prepareStatement("SELECT * FROM " + this.anchorTable + - " WHERE world_name =? AND x =? AND y =? AND z=?;")) { + getDatabaseConnector().connect((con) -> { + try (PreparedStatement ps = con.prepareStatement("INSERT INTO " + getAnchorTable() + "(owner, world_name,x,y,z, ticks_left) VALUES (?,?,?,?,?, ?);"); // Future SQLite version might support 'RETURNING *' + PreparedStatement psFetch = con.prepareStatement("SELECT * FROM " + getAnchorTable() + " WHERE world_name =? AND x =? AND y =? AND z=?;") + ) { ps.setString(1, owner != null ? owner.toString() : null); ps.setString(2, Objects.requireNonNull(loc.getWorld()).getName()); @@ -137,15 +131,14 @@ public class DataManager extends DataManagerAbstract { }); } - public void migrateAnchor(List anchorEntries, UpdateCallback callback) { - this.databaseConnector.connect((con) -> { + public void migrateAnchor(List anchorEntries, UpdateCallback callback) { + getDatabaseConnector().connect((con) -> { con.setAutoCommit(false); SQLException err = null; - try (PreparedStatement ps = con.prepareStatement("INSERT INTO " + this.anchorTable + - "(world_name,x,y,z, ticks_left) VALUES (?,?,?,?, ?);")) { - for (AnchorMigration.LegacyAnchorEntry entry : anchorEntries) { + try (PreparedStatement ps = con.prepareStatement("INSERT INTO " + getAnchorTable() + "(world_name,x,y,z, ticks_left) VALUES (?,?,?,?, ?);")) { + for (LegacyYamlAnchorsMigrator.LegacyAnchorEntry entry : anchorEntries) { ps.setString(1, entry.worldName); ps.setInt(2, entry.x); ps.setInt(3, entry.y); @@ -182,14 +175,13 @@ public class DataManager extends DataManagerAbstract { } public void updateAnchors(Collection anchors, UpdateCallback callback) { - this.databaseConnector.connect((con) -> { + getDatabaseConnector().connect((con) -> { con.setAutoCommit(false); SQLException err = null; for (Anchor anchor : anchors) { - try (PreparedStatement ps = con.prepareStatement("UPDATE " + this.anchorTable + - " SET ticks_left =? WHERE id =?;")) { + try (PreparedStatement ps = con.prepareStatement("UPDATE " + getAnchorTable() + " SET ticks_left =? WHERE id =?;")) { ps.setInt(1, anchor.getTicksLeft()); ps.setInt(2, anchor.getDbId()); @@ -220,9 +212,8 @@ public class DataManager extends DataManagerAbstract { public void deleteAnchorAsync(Anchor anchor, UpdateCallback callback) { this.thread.execute(() -> - this.databaseConnector.connect((con) -> { - try (PreparedStatement ps = con.prepareStatement("DELETE FROM " + this.anchorTable + - " WHERE id =?;")) { + getDatabaseConnector().connect((con) -> { + try (PreparedStatement ps = con.prepareStatement("DELETE FROM " + getAnchorTable() + " WHERE id =?;")) { ps.setInt(1, anchor.getDbId()); ps.executeUpdate(); @@ -235,14 +226,16 @@ public class DataManager extends DataManagerAbstract { ); } - public static String getTableName(String prefix, String name) { - String result = prefix + name; + public String getAnchorTable() { + return getAnchorTable(this.plugin.getDataManager().getTablePrefix()); + } - if (!result.matches("[a-z0-9_]+")) { - throw new IllegalStateException("The generated table name '" + result + "' contains invalid characters"); - } + public static String getAnchorTable(String prefix) { + return prefix + "anchors"; + } - return result; + private DatabaseConnector getDatabaseConnector() { + return this.plugin.getDataManager().getDatabaseConnector(); } private Anchor extractAnchor(ResultSet rs) throws SQLException { @@ -261,7 +254,7 @@ public class DataManager extends DataManagerAbstract { if (callback != null) { callback.accept(ex); } else if (ex != null) { - Utils.logException(this.plugin, ex, "SQLite"); + Utils.logException(this.plugin, ex, "H2"); } } @@ -269,7 +262,7 @@ public class DataManager extends DataManagerAbstract { if (callback != null) { callback.accept(ex, null); } else { - Utils.logException(this.plugin, ex, "SQLite"); + Utils.logException(this.plugin, ex, "H2"); } } } diff --git a/EpicAnchors-Plugin/src/main/java/com/craftaro/epicanchors/files/migration/AnchorMigration.java b/EpicAnchors-Plugin/src/main/java/com/craftaro/epicanchors/files/migration/LegacyYamlAnchorsMigrator.java similarity index 85% rename from EpicAnchors-Plugin/src/main/java/com/craftaro/epicanchors/files/migration/AnchorMigration.java rename to EpicAnchors-Plugin/src/main/java/com/craftaro/epicanchors/files/migration/LegacyYamlAnchorsMigrator.java index ed9483a..48b9df6 100644 --- a/EpicAnchors-Plugin/src/main/java/com/craftaro/epicanchors/files/migration/AnchorMigration.java +++ b/EpicAnchors-Plugin/src/main/java/com/craftaro/epicanchors/files/migration/LegacyYamlAnchorsMigrator.java @@ -2,10 +2,7 @@ package com.craftaro.epicanchors.files.migration; import com.craftaro.core.configuration.Config; import com.craftaro.core.configuration.ConfigSection; -import com.craftaro.core.database.DataMigration; -import com.craftaro.core.database.DataMigrationManager; -import com.craftaro.core.database.DatabaseConnector; -import com.craftaro.epicanchors.files.DataManager; +import com.craftaro.epicanchors.files.AnchorsDataManager; import com.craftaro.epicanchors.utils.ThreadSync; import org.bukkit.Location; import org.bukkit.plugin.Plugin; @@ -18,16 +15,8 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.logging.Level; -public class AnchorMigration extends DataMigrationManager { - private final DataManager dataManager; - - public AnchorMigration(DatabaseConnector databaseConnector, DataManager dataManager, DataMigration... migrations) { - super(databaseConnector, dataManager, migrations); - - this.dataManager = dataManager; - } - - public void migrateLegacyData(Plugin plugin) { +public class LegacyYamlAnchorsMigrator { + public static void migrateLegacyData(Plugin plugin, AnchorsDataManager dataManager) { long start = System.nanoTime(); AtomicBoolean abortMigration = new AtomicBoolean(false); @@ -67,7 +56,7 @@ public class AnchorMigration extends DataMigrationManager { int z = Location.locToBlock(Double.parseDouble(locArgs[3])); int finalTicksLeft = ticksLeft; - this.dataManager.exists(worldName, x, y, z, (ex, anchorExists) -> { + dataManager.exists(worldName, x, y, z, (ex, anchorExists) -> { if (ex == null) { if (anchorExists) { cfgSection.set(locationStr, null); @@ -94,7 +83,7 @@ public class AnchorMigration extends DataMigrationManager { if (!abortMigration.get()) { int finalMigratedAnchors = migratedAnchors; - this.dataManager.migrateAnchor(anchorQueue, ex -> { + dataManager.migrateAnchor(anchorQueue, ex -> { long end = System.nanoTime(); if (ex == null) { @@ -114,7 +103,7 @@ public class AnchorMigration extends DataMigrationManager { } } - private String[] deserializeLegacyLocation(String str) { + private static String[] deserializeLegacyLocation(String str) { if (str == null || str.isEmpty()) { return new String[0]; } diff --git a/EpicAnchors-Plugin/src/main/java/com/craftaro/epicanchors/files/migration/_1_InitialMigration.java b/EpicAnchors-Plugin/src/main/java/com/craftaro/epicanchors/files/migration/_1_InitialMigration.java index ac135f1..f260e66 100644 --- a/EpicAnchors-Plugin/src/main/java/com/craftaro/epicanchors/files/migration/_1_InitialMigration.java +++ b/EpicAnchors-Plugin/src/main/java/com/craftaro/epicanchors/files/migration/_1_InitialMigration.java @@ -1,7 +1,7 @@ package com.craftaro.epicanchors.files.migration; import com.craftaro.core.database.DataMigration; -import com.craftaro.epicanchors.files.DataManager; +import com.craftaro.epicanchors.files.AnchorsDataManager; import java.sql.Connection; import java.sql.SQLException; @@ -15,16 +15,17 @@ public class _1_InitialMigration extends DataMigration { @Override public void migrate(Connection connection, String tablePrefix) throws SQLException { try (Statement statement = connection.createStatement()) { - statement.execute("CREATE TABLE " + DataManager.getTableName(tablePrefix, "anchors") + "(" + - "id INTEGER NOT NULL," + - "world_name TEXT NOT NULL," + - "x INTEGER NOT NULL," + - "y INTEGER NOT NULL," + - "z INTEGER NOT NULL," + - "ticks_left INTEGER NOT NULL," + - "owner VARCHAR(36)," + - "PRIMARY KEY(id AUTOINCREMENT)" + - ");"); + statement.execute( + "CREATE TABLE " + AnchorsDataManager.getAnchorTable(tablePrefix) + + "(" + + "id INTEGER NOT NULL PRIMARY KEY auto_increment," + + "world_name TEXT NOT NULL," + + "x INTEGER NOT NULL," + + "y INTEGER NOT NULL," + + "z INTEGER NOT NULL," + + "ticks_left INTEGER NOT NULL," + + "owner VARCHAR(36)" + + ");"); } } } diff --git a/EpicAnchors-Plugin/src/main/java/com/craftaro/epicanchors/listener/AnchorListener.java b/EpicAnchors-Plugin/src/main/java/com/craftaro/epicanchors/listener/AnchorListener.java index 58a2ce6..f1776f0 100644 --- a/EpicAnchors-Plugin/src/main/java/com/craftaro/epicanchors/listener/AnchorListener.java +++ b/EpicAnchors-Plugin/src/main/java/com/craftaro/epicanchors/listener/AnchorListener.java @@ -49,7 +49,7 @@ public class AnchorListener implements Listener { this.plugin.getAnchorManager().createAnchor(e.getBlock().getLocation(), e.getPlayer().getUniqueId(), ticksLeft, (ex, result) -> { if (ex != null) { - Utils.logException(this.plugin, ex, "SQLite"); + Utils.logException(this.plugin, ex, "H2"); e.getPlayer().sendMessage("Error creating anchor!"); // TODO Bukkit.getScheduler().runTask(this.plugin, () -> {