From afd1958520567b4d1bae3e6cd3116bcb869843bb Mon Sep 17 00:00:00 2001 From: CJ Burkey Date: Thu, 23 May 2024 13:33:48 -0400 Subject: [PATCH] Organize and cleanup --- build.gradle.kts | 2 + .../com/cjburkey/claimchunk/ClaimChunk.java | 2 +- .../data/{conversion => }/IDataConverter.java | 2 +- .../data/conversion/ConvertJsonToMySQL.java | 30 ---- .../data/conversion/ConvertMySQLToJson.java | 38 ----- .../data/newdata/MySQLDataHandler.java | 2 +- .../data/sqlite/SqLiteDataHandler.java | 2 +- .../sqlite/SqLiteTableMigrationManager.java | 2 +- .../claimchunk/data/sqlite/SqLiteWrapper.java | 93 ++++------ .../claimchunk/data/sqlite/SqlDataChunk.java | 5 - .../cjburkey/claimchunk/TestSQLPlease.java | 161 +++++++++++++++--- 11 files changed, 172 insertions(+), 167 deletions(-) rename src/main/java/com/cjburkey/claimchunk/data/{conversion => }/IDataConverter.java (97%) delete mode 100644 src/main/java/com/cjburkey/claimchunk/data/conversion/ConvertJsonToMySQL.java delete mode 100644 src/main/java/com/cjburkey/claimchunk/data/conversion/ConvertMySQLToJson.java diff --git a/build.gradle.kts b/build.gradle.kts index aa0c43ce..276bc604 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -42,6 +42,7 @@ object DepData { const val JAVAX_PERSISTENCE_VERSION = "2.1.0" const val JAVAX_TRANSACTION_VERSION = "1.1" const val SANS_ORM_VERSION = "3.17" + const val SLF4J_VERSION = "1.7.25" // Directories const val TEST_SERVER_DIR = "run" @@ -301,6 +302,7 @@ dependencies { implementation("javax.transaction:transaction-api:${DepData.JAVAX_TRANSACTION_VERSION}") implementation("com.github.h-thurow:q2o:${DepData.SANS_ORM_VERSION}") + testImplementation("org.slf4j:slf4j-simple:${DepData.SLF4J_VERSION}") testImplementation("org.junit.jupiter:junit-jupiter:${DepData.JUNIT_VERSION}") testRuntimeOnly("org.junit.platform:junit-platform-launcher:${DepData.JUNIT_LAUNCHER_VERSION}") } diff --git a/src/main/java/com/cjburkey/claimchunk/ClaimChunk.java b/src/main/java/com/cjburkey/claimchunk/ClaimChunk.java index 3a50b367..be35503e 100644 --- a/src/main/java/com/cjburkey/claimchunk/ClaimChunk.java +++ b/src/main/java/com/cjburkey/claimchunk/ClaimChunk.java @@ -6,7 +6,7 @@ import com.cjburkey.claimchunk.cmd.*; import com.cjburkey.claimchunk.config.ClaimChunkWorldProfileHandler; import com.cjburkey.claimchunk.config.ccconfig.*; -import com.cjburkey.claimchunk.data.conversion.IDataConverter; +import com.cjburkey.claimchunk.data.IDataConverter; import com.cjburkey.claimchunk.data.newdata.*; import com.cjburkey.claimchunk.data.sqlite.SqLiteDataHandler; import com.cjburkey.claimchunk.event.*; diff --git a/src/main/java/com/cjburkey/claimchunk/data/conversion/IDataConverter.java b/src/main/java/com/cjburkey/claimchunk/data/IDataConverter.java similarity index 97% rename from src/main/java/com/cjburkey/claimchunk/data/conversion/IDataConverter.java rename to src/main/java/com/cjburkey/claimchunk/data/IDataConverter.java index 28a6afef..ea7cb898 100644 --- a/src/main/java/com/cjburkey/claimchunk/data/conversion/IDataConverter.java +++ b/src/main/java/com/cjburkey/claimchunk/data/IDataConverter.java @@ -1,4 +1,4 @@ -package com.cjburkey.claimchunk.data.conversion; +package com.cjburkey.claimchunk.data; import com.cjburkey.claimchunk.data.newdata.IClaimChunkDataHandler; diff --git a/src/main/java/com/cjburkey/claimchunk/data/conversion/ConvertJsonToMySQL.java b/src/main/java/com/cjburkey/claimchunk/data/conversion/ConvertJsonToMySQL.java deleted file mode 100644 index e55a6e7b..00000000 --- a/src/main/java/com/cjburkey/claimchunk/data/conversion/ConvertJsonToMySQL.java +++ /dev/null @@ -1,30 +0,0 @@ -package com.cjburkey.claimchunk.data.conversion; - -import com.cjburkey.claimchunk.ClaimChunk; -import com.cjburkey.claimchunk.data.newdata.JsonDataHandler; -import com.cjburkey.claimchunk.data.newdata.MySQLDataHandler; - -@SuppressWarnings("unused") -public class ConvertJsonToMySQL implements IDataConverter> { - - private final ClaimChunk claimChunk; - - public ConvertJsonToMySQL(ClaimChunk claimChunk) { - this.claimChunk = claimChunk; - } - - @Override - public MySQLDataHandler convert(JsonDataHandler oldDataHandler) throws Exception { - // Create and a new MySQL data handler - MySQLDataHandler newDataHandler = new MySQLDataHandler<>(claimChunk, null, null); - - // Initialize the new data handler - newDataHandler.init(); - - // Convert from the old data handler to the new one - IDataConverter.copyConvert(oldDataHandler, newDataHandler); - - // Return the new data handler - return newDataHandler; - } -} diff --git a/src/main/java/com/cjburkey/claimchunk/data/conversion/ConvertMySQLToJson.java b/src/main/java/com/cjburkey/claimchunk/data/conversion/ConvertMySQLToJson.java deleted file mode 100644 index 2f05d8c0..00000000 --- a/src/main/java/com/cjburkey/claimchunk/data/conversion/ConvertMySQLToJson.java +++ /dev/null @@ -1,38 +0,0 @@ -package com.cjburkey.claimchunk.data.conversion; - -import com.cjburkey.claimchunk.ClaimChunk; -import com.cjburkey.claimchunk.data.newdata.JsonDataHandler; -import com.cjburkey.claimchunk.data.newdata.MySQLDataHandler; - -import java.io.File; - -@SuppressWarnings("unused") -public class ConvertMySQLToJson implements IDataConverter, JsonDataHandler> { - - private final ClaimChunk claimChunk; - private final File claimedChunksFile; - private final File joinedPlayersFile; - - public ConvertMySQLToJson( - ClaimChunk claimChunk, File claimedChunksFile, File joinedPlayersFile) { - this.claimChunk = claimChunk; - this.claimedChunksFile = claimedChunksFile; - this.joinedPlayersFile = joinedPlayersFile; - } - - @Override - public JsonDataHandler convert(MySQLDataHandler oldDataHandler) throws Exception { - // Create and a new MySQL data handler - JsonDataHandler newDataHandler = - new JsonDataHandler(claimChunk, claimedChunksFile, joinedPlayersFile); - - // Initialize the new data handler - newDataHandler.init(); - - // Convert from the old data handler to the new one - IDataConverter.copyConvert(oldDataHandler, newDataHandler); - - // Return the new data handler - return newDataHandler; - } -} diff --git a/src/main/java/com/cjburkey/claimchunk/data/newdata/MySQLDataHandler.java b/src/main/java/com/cjburkey/claimchunk/data/newdata/MySQLDataHandler.java index ebf57dea..205c9fad 100644 --- a/src/main/java/com/cjburkey/claimchunk/data/newdata/MySQLDataHandler.java +++ b/src/main/java/com/cjburkey/claimchunk/data/newdata/MySQLDataHandler.java @@ -7,7 +7,7 @@ import com.cjburkey.claimchunk.chunk.ChunkPlayerPermissions; import com.cjburkey.claimchunk.chunk.ChunkPos; import com.cjburkey.claimchunk.chunk.DataChunk; -import com.cjburkey.claimchunk.data.conversion.IDataConverter; +import com.cjburkey.claimchunk.data.IDataConverter; import com.cjburkey.claimchunk.player.FullPlayerData; import com.cjburkey.claimchunk.player.SimplePlayerData; diff --git a/src/main/java/com/cjburkey/claimchunk/data/sqlite/SqLiteDataHandler.java b/src/main/java/com/cjburkey/claimchunk/data/sqlite/SqLiteDataHandler.java index a9386043..d206fc19 100644 --- a/src/main/java/com/cjburkey/claimchunk/data/sqlite/SqLiteDataHandler.java +++ b/src/main/java/com/cjburkey/claimchunk/data/sqlite/SqLiteDataHandler.java @@ -237,7 +237,7 @@ public void givePlayerAccess( DataChunk chunkData = claimedChunks.get(chunk); if (chunkData != null) { chunkData.playerPermissions.put(accessor, permissions); - sqLiteWrapper.updateOrInsertPlayerAccess(chunk, accessor, permissions.permissionFlags); + sqLiteWrapper.setPlayerAccess(chunk, accessor, permissions.permissionFlags); } } diff --git a/src/main/java/com/cjburkey/claimchunk/data/sqlite/SqLiteTableMigrationManager.java b/src/main/java/com/cjburkey/claimchunk/data/sqlite/SqLiteTableMigrationManager.java index 915ae516..50b7e895 100644 --- a/src/main/java/com/cjburkey/claimchunk/data/sqlite/SqLiteTableMigrationManager.java +++ b/src/main/java/com/cjburkey/claimchunk/data/sqlite/SqLiteTableMigrationManager.java @@ -48,11 +48,11 @@ FOREIGN KEY(owner_uuid) REFERENCES player_data(player_uuid) Q2Sql.executeUpdate( """ CREATE TABLE IF NOT EXISTS chunk_permissions ( - perm_id INTEGER PRIMARY KEY, chunk_id INTEGER NOT NULL, other_player_uuid TEXT NOT NULL, permission_bits INTEGER NOT NULL, + PRIMARY KEY(chunk_id, other_player_uuid) FOREIGN KEY(chunk_id) REFERENCES chunk_data(chunk_id), FOREIGN KEY(other_player_uuid) REFERENCES player_data(player_uuid) ) STRICT diff --git a/src/main/java/com/cjburkey/claimchunk/data/sqlite/SqLiteWrapper.java b/src/main/java/com/cjburkey/claimchunk/data/sqlite/SqLiteWrapper.java index b9f7716a..466d35a9 100644 --- a/src/main/java/com/cjburkey/claimchunk/data/sqlite/SqLiteWrapper.java +++ b/src/main/java/com/cjburkey/claimchunk/data/sqlite/SqLiteWrapper.java @@ -95,18 +95,14 @@ INSERT INTO chunk_permissions ( """; // Better way to do this? - String permsValsSql = - chunk.playerPermissions.entrySet().stream() - .map( - ignored -> - replaceChunkIdQuery( - """ - (%%SELECT_CHUNK_ID_SQL%%, ?, ?) - """)) - .collect(Collectors.joining(",")); - - try (PreparedStatement statement = - connection.prepareStatement(permsInsertPrefixSql + permsValsSql)) { + ArrayList params = new ArrayList<>(); + for (int i = 0; i < chunk.playerPermissions.size(); i++) { + params.add("(%%SELECT_CHUNK_ID_SQL%%, ?, ?)"); + } + String finalSql = + chunkIdQuery(permsInsertPrefixSql + String.join(",", params)); + + try (PreparedStatement statement = connection.prepareStatement(finalSql)) { int currentParam = 1; for (Map.Entry entry : chunk.playerPermissions.entrySet()) { @@ -129,7 +125,7 @@ public void removeClaimedChunk(ChunkPos chunk) { // Remove all granted permissions for the chunk try (PreparedStatement statement = connection.prepareStatement( - replaceChunkIdQuery( + chunkIdQuery( """ DELETE FROM chunk_permissions WHERE chunk_id=%%SELECT_CHUNK_ID_SQL%% @@ -254,60 +250,28 @@ public void setPlayerExtraMaxClaims(UUID player, int extraMaxClaims) { }); } - public void updateOrInsertPlayerAccess(ChunkPos chunk, UUID accessor, int permissionFlags) { - // Check if the access already exists + public void setPlayerAccess(ChunkPos chunk, UUID accessor, int permissionFlags) { SqlClosure.sqlExecute( connection -> { - final boolean accessExists; try (PreparedStatement statement = connection.prepareStatement( - replaceChunkIdQuery( + chunkIdQuery( """ -SELECT COUNT(*) FROM chunk_permissions -WHERE other_player_uuid=? AND chunk_id=%%SELECT_CHUNK_ID_SQL%% -"""))) { - statement.setString(1, accessor.toString()); - setChunkPosParams(statement, 2, chunk); - ResultSet resultSet = statement.executeQuery(); - accessExists = resultSet.next() && resultSet.getInt(1) > 0; - } - - // If the entry already exists, update the permission bits - if (accessExists) { - try (PreparedStatement statement = - connection.prepareStatement( - replaceChunkIdQuery( - """ - UPDATE chunk_permissions - SET permission_bits=? - WHERE chunk_id=%%SELECT_CHUNK_ID_SQL%% - AND other_player_uuid=? - """))) { - statement.setInt(1, permissionFlags); - int next = setChunkPosParams(statement, 2, chunk); - statement.setString(next, accessor.toString()); - statement.execute(); - } - } else { - try (PreparedStatement statement = - connection.prepareStatement( - replaceChunkIdQuery( - """ - INSERT INTO chunk_permissions ( - chunk_id, - other_player_uuid, - permission_bits - ) VALUES ( - %%SELECT_CHUNK_ID_SQL%%, ?, ? - ) - """))) { - int next = setChunkPosParams(statement, 1, chunk); - statement.setString(next, accessor.toString()); - statement.setInt(next + 1, permissionFlags); - statement.execute(); - } + INSERT INTO chunk_permissions ( + chunk_id, + other_player_uuid, + permission_bits + ) VALUES ( + %%SELECT_CHUNK_ID_SQL%%, ?, ? + ) + ON CONFLICT(chunk_id, other_player_uuid) DO + UPDATE SET permission_bits=excluded.permission_bits + """))) { + int next = setChunkPosParams(statement, 1, chunk); + statement.setString(next, accessor.toString()); + statement.setInt(next + 1, permissionFlags); + statement.execute(); } - return null; }); } @@ -317,14 +281,15 @@ public void removePlayerAccess(ChunkPos chunk, UUID accessor) { connection -> { try (PreparedStatement statement = connection.prepareStatement( - replaceChunkIdQuery( + chunkIdQuery( """ DELETE FROM chunk_permissions - WHERE chunk_id=%%SELECT_CHUNK_ID_SQL%%, + WHERE chunk_id=%%SELECT_CHUNK_ID_SQL%% AND other_player_uuid=? """))) { int next = setChunkPosParams(statement, 1, chunk); statement.setString(next, accessor.toString()); + statement.execute(); return null; } }); @@ -408,7 +373,7 @@ private int setChunkPosParams( return worldParameterNum + 3; } - private String replaceChunkIdQuery(String sql) { + private String chunkIdQuery(String sql) { return SELECT_CHUNK_ID_SQL_PATTERN.matcher(sql).replaceAll(SELECT_CHUNK_ID_SQL); } } diff --git a/src/main/java/com/cjburkey/claimchunk/data/sqlite/SqlDataChunk.java b/src/main/java/com/cjburkey/claimchunk/data/sqlite/SqlDataChunk.java index 302bc4d4..de6b20f8 100644 --- a/src/main/java/com/cjburkey/claimchunk/data/sqlite/SqlDataChunk.java +++ b/src/main/java/com/cjburkey/claimchunk/data/sqlite/SqlDataChunk.java @@ -5,11 +5,6 @@ @Table(name = "chunk_data") public class SqlDataChunk { - @Id - @Column(name = "chunk_id") - @GeneratedValue(strategy = GenerationType.IDENTITY) - public int chunkId; - @Column(name = "chunk_world") public String world; diff --git a/src/test/java/com/cjburkey/claimchunk/TestSQLPlease.java b/src/test/java/com/cjburkey/claimchunk/TestSQLPlease.java index a6d83a43..6162204b 100644 --- a/src/test/java/com/cjburkey/claimchunk/TestSQLPlease.java +++ b/src/test/java/com/cjburkey/claimchunk/TestSQLPlease.java @@ -1,5 +1,8 @@ package com.cjburkey.claimchunk; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; + import com.cjburkey.claimchunk.chunk.ChunkPlayerPermissions; import com.cjburkey.claimchunk.chunk.ChunkPos; import com.cjburkey.claimchunk.chunk.DataChunk; @@ -12,22 +15,41 @@ import java.io.File; import java.util.Collection; import java.util.HashMap; -import java.util.Objects; +import java.util.Map; import java.util.UUID; class TestSQLPlease { @Test - void ensureNoDataLoss() { - File dbFile = randomDbFile(); - dbFile.deleteOnExit(); + void ensureColumnExistsMethodWorks() { + // Must create the wrapper to initialize (and deinitialize) connection + try (TestQlWrap ignoredWrapper = new TestQlWrap()) { + // Make sure that instantiating SqLiteWrapper created the tables + assert SqLiteTableMigrationManager.columnExists("player_data", "player_uuid"); + assert SqLiteTableMigrationManager.columnExists("chunk_data", "owner_uuid"); + assert SqLiteTableMigrationManager.columnExists("chunk_permissions", "permission_bits"); + assert !SqLiteTableMigrationManager.columnExists("chunk_hell", "permission_bits"); + assert !SqLiteTableMigrationManager.columnExists("player_data", "fake_col"); + } + } - try (SqLiteWrapper ignoredWrapper = new SqLiteWrapper(dbFile, false)) { + @Test + void ensureNoDataLoss() { + try (TestQlWrap wrapper = new TestQlWrap()) { // Add a random player - UUID plyUuid = UUID.randomUUID(); - ignoredWrapper.addPlayer( + UUID ply1Uuid = UUID.randomUUID(); + UUID ply2Uuid = UUID.randomUUID(); + wrapper.sql.addPlayer( + new FullPlayerData( + ply1Uuid, "SomeGuysName", null, System.currentTimeMillis(), true, 0)); + wrapper.sql.addPlayer( new FullPlayerData( - plyUuid, "SomeGuysName", null, System.currentTimeMillis(), true, 0)); + ply2Uuid, + "OtherPersonsName", + "queenshit", + System.currentTimeMillis(), + false, + 0)); // Make fake accessors and permissions UUID accessorUuid1 = UUID.randomUUID(); @@ -37,40 +59,129 @@ void ensureNoDataLoss() { // Add a chunk to the player and give the permissions to the other players ChunkPos chunkPos = new ChunkPos("world", 10, -3); - DataChunk chunkData = new DataChunk(chunkPos, plyUuid, new HashMap<>(), false); + DataChunk chunkData = new DataChunk(chunkPos, ply1Uuid, new HashMap<>(), false); chunkData.playerPermissions.put(accessorUuid1, permissions1); chunkData.playerPermissions.put(accessorUuid2, permissions2); - ignoredWrapper.addClaimedChunk(chunkData); + wrapper.sql.addClaimedChunk(chunkData); + + // Make sure both players get loaded + Collection players = wrapper.sql.getAllPlayers(); + assertEquals(2, players.size()); + assert players.stream() + .allMatch(ply -> ply.player.equals(ply1Uuid) || ply.player.equals(ply2Uuid)); + assert players.stream().anyMatch(ply -> "queenshit".equals(ply.chunkName)); // Load the chunk after adding it - Collection loadedChunks = ignoredWrapper.getAllChunks(); + Collection loadedChunks = wrapper.sql.getAllChunks(); DataChunk loadedChunk = loadedChunks.iterator().next(); - Objects.requireNonNull(loadedChunk); + assertNotNull(loadedChunk); // Make sure the chunk exists when we load from the database - assert loadedChunk.player.equals(plyUuid) && loadedChunk.chunk.equals(chunkPos); + assert loadedChunk.player.equals(ply1Uuid) && loadedChunk.chunk.equals(chunkPos); // Make sure the chunk permission got loaded correctly - assert Objects.equals(permissions1, loadedChunk.playerPermissions.get(accessorUuid1)); - assert Objects.equals(permissions2, loadedChunk.playerPermissions.get(accessorUuid2)); + assertEquals(permissions1, loadedChunk.playerPermissions.get(accessorUuid1)); + assertEquals(permissions2, loadedChunk.playerPermissions.get(accessorUuid2)); } } @Test - void ensureSomeColumnsExistsAfterInitializing() { - File dbFile = randomDbFile(); - dbFile.deleteOnExit(); + void multiplePermissions() { + try (TestQlWrap wrapper = new TestQlWrap()) { + UUID owner = UUID.randomUUID(); + UUID accessor1 = UUID.randomUUID(); + UUID accessor2 = UUID.randomUUID(); + ChunkPos chunk = new ChunkPos("world", 824, -29); + DataChunk chunkData = new DataChunk(chunk, owner, new HashMap<>(), false); + chunkData.playerPermissions.put(accessor1, new ChunkPlayerPermissions(0b01)); + chunkData.playerPermissions.put(accessor2, new ChunkPlayerPermissions(0b10)); - try (SqLiteWrapper ignoredWrapper = new SqLiteWrapper(dbFile, false)) { - // Make sure that instantiating SqLiteWrapper created the tables - assert SqLiteTableMigrationManager.columnExists("player_data", "player_uuid"); - assert SqLiteTableMigrationManager.columnExists("chunk_data", "owner_uuid"); - assert SqLiteTableMigrationManager.columnExists("chunk_permissions", "permission_bits"); - assert !SqLiteTableMigrationManager.columnExists("chunk_hell", "permission_bits"); - assert !SqLiteTableMigrationManager.columnExists("player_data", "fake_col"); + // Add the players + wrapper.sql.addPlayer( + new FullPlayerData( + owner, "PersonHere", null, System.currentTimeMillis(), true, 0)); + wrapper.sql.addPlayer( + new FullPlayerData( + accessor1, "PersonThere", null, System.currentTimeMillis(), true, 0)); + wrapper.sql.addPlayer( + new FullPlayerData( + accessor2, "AnotherOne", null, System.currentTimeMillis(), true, 0)); + + // Add the chunk + wrapper.sql.addClaimedChunk(chunkData); + + // Load the chunk and make sure it contains both accessors + Map loadedPerms = + wrapper.sql.getAllChunks().iterator().next().playerPermissions; + assert loadedPerms.containsKey(accessor1); + assert loadedPerms.containsKey(accessor2); + } + } + + @Test + void insertOrUpdatePermission() { + try (TestQlWrap wrapper = new TestQlWrap()) { + UUID owner = UUID.randomUUID(); + UUID accessor = UUID.randomUUID(); + ChunkPos chunk = new ChunkPos("world", 824, -29); + int flags1 = 0b10101001; + int flags2 = 0b01010100; + + // Add the players and the chunk + wrapper.sql.addPlayer( + new FullPlayerData( + owner, "PersonHere", null, System.currentTimeMillis(), true, 0)); + wrapper.sql.addPlayer( + new FullPlayerData( + accessor, "PersonThere", null, System.currentTimeMillis(), true, 0)); + wrapper.sql.addClaimedChunk(new DataChunk(chunk, owner, new HashMap<>(), false)); + + // Insert the permission and check it + wrapper.sql.setPlayerAccess(chunk, accessor, flags1); + assertEquals( + flags1, + wrapper.sql + .getAllChunks() + .iterator() + .next() + .playerPermissions + .get(accessor) + .permissionFlags); + + // Update the permission and check it + wrapper.sql.setPlayerAccess(chunk, accessor, flags2); + assertEquals( + flags2, + wrapper.sql + .getAllChunks() + .iterator() + .next() + .playerPermissions + .get(accessor) + .permissionFlags); + + // Remove the permission and make sure there aren't any permissions now + wrapper.sql.removePlayerAccess(chunk, accessor); + assert wrapper.sql.getAllChunks().iterator().next().playerPermissions.isEmpty(); } } protected static File randomDbFile() { return new File(UUID.randomUUID() + ".tmp.sqlite3"); } + + static class TestQlWrap implements AutoCloseable { + SqLiteWrapper sql; + File dbFile; + + TestQlWrap() { + dbFile = randomDbFile(); + sql = new SqLiteWrapper(dbFile, false); + dbFile.deleteOnExit(); + } + + @Override + public void close() { + sql.close(); + } + } }