From b0fcd6ccc2ce66c811373d62b127e043e8908e8a Mon Sep 17 00:00:00 2001 From: BenCodez <17074231+BenCodez@users.noreply.github.com> Date: Sat, 3 Oct 2026 09:59:43 -0600 Subject: [PATCH 1/3] Preserve SQL acquisition failures in user storage --- .../api/user/userstorage/mysql/MySQL.java | 24 +++++++------- .../user/storage/sql/MysqlUserBackend.java | 24 +++++++------- .../sql/MysqlBackendReviewRegressionTest.java | 33 +++++++++++++++++++ docs/shared-sql-user-access.md | 15 +++++++++ 4 files changed, 72 insertions(+), 24 deletions(-) diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/userstorage/mysql/MySQL.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/userstorage/mysql/MySQL.java index 19f5288b4..9c8cd3cc5 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/userstorage/mysql/MySQL.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/userstorage/mysql/MySQL.java @@ -112,7 +112,7 @@ public boolean isUseBatchUpdates() { } public void loadData() { - try (Connection con = mysql.getConnectionManager().getConnection()) { + try (Connection con = mysql.getConnectionManager().getConnectionChecked()) { useBatchUpdates = con != null && con.getMetaData().supportsBatchUpdates(); } catch (SQLException e) { debug(e); @@ -160,7 +160,7 @@ public boolean containsKeyQuery(String uuid) { return false; } String sql = "SELECT 1 FROM " + qi(tableName) + " WHERE " + qi("uuid") + " = ?::uuid LIMIT 1;"; - try (Connection conn = mysql.getConnectionManager().getConnection(); + try (Connection conn = mysql.getConnectionManager().getConnectionChecked(); PreparedStatement ps = conn.prepareStatement(sql)) { ps.setString(1, uuid); try (ResultSet rs = ps.executeQuery()) { @@ -225,7 +225,7 @@ public void forEachUser(java.util.function.BiConsumer> p + " ASC " + " LIMIT " + pageSize + ";"; } - try (Connection conn = mysql.getConnectionManager().getConnection(); + try (Connection conn = mysql.getConnectionManager().getConnectionChecked(); PreparedStatement ps = conn.prepareStatement(sqlStr, ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY)) { @@ -435,7 +435,7 @@ public String getUUIDOrThrow(String playerName) throws SQLException { String query = "SELECT " + qi("uuid") + " FROM " + qi(tableName) + " WHERE " + qi("PlayerName") + "=?;"; plugin.devDebug("DB QUERY: " + query); - try (Connection conn = mysql.getConnectionManager().getConnection(); + try (Connection conn = mysql.getConnectionManager().getConnectionChecked(); PreparedStatement sql = conn.prepareStatement(query)) { sql.setString(1, playerName); try (ResultSet rs = sql.executeQuery()) { @@ -462,7 +462,7 @@ public ArrayList getNumbersInColumn(String column) { String sqlStr = "SELECT " + qi(column) + " FROM " + qi(tableName) + ";"; plugin.devDebug("DB QUERY: " + sqlStr); - try (Connection conn = mysql.getConnectionManager().getConnection(); + try (Connection conn = mysql.getConnectionManager().getConnectionChecked(); PreparedStatement sql = conn.prepareStatement(sqlStr); ResultSet rs = sql.executeQuery()) { @@ -481,7 +481,7 @@ public ArrayList getRowsNameQuery() { String sqlStr = "SELECT " + qi("PlayerName") + " FROM " + qi(tableName) + ";"; plugin.devDebug("DB QUERY: " + sqlStr); - try (Connection conn = mysql.getConnectionManager().getConnection(); + try (Connection conn = mysql.getConnectionManager().getConnectionChecked(); PreparedStatement sql = conn.prepareStatement(sqlStr); ResultSet rs = sql.executeQuery()) { @@ -500,7 +500,7 @@ public ArrayList getRowsQuery() { String sqlStr = "SELECT " + qi("uuid") + " FROM " + qi(tableName) + ";"; plugin.devDebug("DB QUERY: " + sqlStr); - try (Connection conn = mysql.getConnectionManager().getConnection(); + try (Connection conn = mysql.getConnectionManager().getConnectionChecked(); PreparedStatement sql = conn.prepareStatement(sqlStr); ResultSet rs = sql.executeQuery()) { @@ -529,7 +529,7 @@ public ConcurrentHashMap getRowsUUIDNameQuery() { String sqlStr = "SELECT " + qi("uuid") + ", " + qi("PlayerName") + " FROM " + qi(tableName) + ";"; plugin.devDebug("DB QUERY: " + sqlStr); - try (Connection conn = mysql.getConnectionManager().getConnection(); + try (Connection conn = mysql.getConnectionManager().getConnectionChecked(); PreparedStatement sql = conn.prepareStatement(sqlStr); ResultSet rs = sql.executeQuery()) { @@ -576,7 +576,7 @@ public void deletePlayer(String uuid) { String q = "DELETE FROM " + qi(tableName) + " WHERE " + qi("uuid") + "=?;"; plugin.devDebug("DB QUERY: " + q); - try (Connection conn = mysql.getConnectionManager().getConnection(); + try (Connection conn = mysql.getConnectionManager().getConnectionChecked(); PreparedStatement statement = conn.prepareStatement(q)) { bindUuid(statement, 1, uuid); statement.executeUpdate(); @@ -600,7 +600,7 @@ public void executeQuery(String str) { } public void executeQueryReturn(String str) { - try (Connection conn = mysql.getConnectionManager().getConnection(); + try (Connection conn = mysql.getConnectionManager().getConnectionChecked(); PreparedStatement sql = conn.prepareStatement(str); ResultSet rs = sql.executeQuery()) { plugin.devDebug("DB QUERY: " + str); @@ -614,7 +614,7 @@ public HashMap> getAllQuery() { String query = "SELECT * FROM " + qi(tableName) + ";"; plugin.devDebug("DB QUERY: " + query); - try (Connection conn = mysql.getConnectionManager().getConnection(); + try (Connection conn = mysql.getConnectionManager().getConnectionChecked(); PreparedStatement sql = conn.prepareStatement(query); ResultSet rs = sql.executeQuery()) { @@ -687,7 +687,7 @@ public ArrayList getExactQuery(Column column) { plugin.devDebug("DB QUERY: " + query); - try (Connection conn = mysql.getConnectionManager().getConnection(); + try (Connection conn = mysql.getConnectionManager().getConnectionChecked(); PreparedStatement sql = conn.prepareStatement(query)) { if (dbType == DbType.POSTGRESQL && "uuid".equalsIgnoreCase(column.getName())) { bindUuid(sql, 1, column.getValue().getString()); diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/core/user/storage/sql/MysqlUserBackend.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/core/user/storage/sql/MysqlUserBackend.java index e94cef04f..966c56734 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/core/user/storage/sql/MysqlUserBackend.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/core/user/storage/sql/MysqlUserBackend.java @@ -58,7 +58,7 @@ public SqlUserStorage user(UUID uuid) { requireAdmissionOpen(); JdbcSqlUserStorage.Dialect dialect = JdbcSqlUserStorage.Dialect.fromDbType(table.getMysql().getConnectionManager().getDbType()); SqlUserStorage delegate = new JdbcSqlUserStorage(UserStorage.MYSQL, uuid, table.getTableName(), schema, - () -> table.getMysql().getConnectionManager().getConnection(), dialect, logger); + () -> table.getMysql().getConnectionManager().getConnectionChecked(), dialect, logger); return new SqlUserStorage() { @Override public List readRow(UserStorage storage) { return withOperation(() -> delegate.readRow(storage)); } @Override public boolean contains(UserStorage storage) { return withOperation(() -> delegate.contains(storage)); } @@ -102,7 +102,7 @@ private List readUserPage(String cursor) { + (cursor == null ? "" : " AND " + uuidColumn + " > ?") + " ORDER BY " + uuidColumn + " ASC LIMIT ?"; JdbcSqlUserStorage.Dialect dialect = JdbcSqlUserStorage.Dialect.fromDbType(table.getDbType()); - try (Connection connection = table.getMysql().getConnectionManager().getConnection(); PreparedStatement statement = connection.prepareStatement(sql)) { + try (Connection connection = table.getMysql().getConnectionManager().getConnectionChecked(); PreparedStatement statement = connection.prepareStatement(sql)) { int index = 1; if (cursor != null) { if (dialect == JdbcSqlUserStorage.Dialect.POSTGRESQL) dialect.bindUuid(statement, index++, UUID.fromString(cursor)); else statement.setString(index++, cursor); @@ -239,7 +239,7 @@ void ensureUuidType() { + uuidType + " USING NULLIF(" + uuidColumn + ", '')::uuid;" : "ALTER TABLE " + quote(tableName) + " MODIFY " + uuidColumn + " " + normaliseTypeForDb(uuidType) + ";"; - try (Connection connection = getMysql().getConnectionManager().getConnection(); PreparedStatement statement = connection.prepareStatement(sql)) { statement.executeUpdate(); } + try (Connection connection = getMysql().getConnectionManager().getConnectionChecked(); PreparedStatement statement = connection.prepareStatement(sql)) { statement.executeUpdate(); } catch (SQLException ddlFailure) { try { if (columnNeedsAlter(SqlUserSchema.UUID_COLUMN, uuidType)) throw ddlFailure; } catch (SQLException inspectionFailure) { @@ -251,7 +251,7 @@ void ensureUuidType() { } void ensureUuidUnique() { - try (Connection connection = getMysql().getConnectionManager().getConnection()) { + try (Connection connection = getMysql().getConnectionManager().getConnectionChecked()) { java.sql.DatabaseMetaData metadata = connection.getMetaData(); if (metadata == null || hasUniqueUuidConstraint(connection, metadata)) return; String indexName = tableName + "_uuid_unique"; @@ -326,7 +326,7 @@ void ensureColumn(SqlUserSchema.ColumnDefinition column) { rememberColumn(column); return; } String sql = "ALTER TABLE " + quote(tableName) + " ADD COLUMN " + quote(column.name()) + " " + normaliseTypeForDb(column.sqlType()) + ";"; - try (Connection connection = getMysql().getConnectionManager().getConnection(); PreparedStatement statement = connection.prepareStatement(sql)) { statement.executeUpdate(); } + try (Connection connection = getMysql().getConnectionManager().getConnectionChecked(); PreparedStatement statement = connection.prepareStatement(sql)) { statement.executeUpdate(); } catch (SQLException ddlFailure) { if (!isDuplicateColumn(ddlFailure)) throw ddlFailure; try { @@ -347,7 +347,7 @@ void ensureColumn(SqlUserSchema.ColumnDefinition column) { private void renamePostgresColumn(String storedName, String requestedName) throws SQLException { String sql = "ALTER TABLE " + quote(tableName) + " RENAME COLUMN " + quote(storedName) + " TO " + quote(requestedName) + ";"; - try (Connection connection = getMysql().getConnectionManager().getConnection(); PreparedStatement statement = connection.prepareStatement(sql)) { statement.executeUpdate(); } + try (Connection connection = getMysql().getConnectionManager().getConnectionChecked(); PreparedStatement statement = connection.prepareStatement(sql)) { statement.executeUpdate(); } catch (SQLException renameFailure) { String current = findRegisteredColumn(requestedName); if (!requestedName.equals(current)) throw renameFailure; } } @@ -390,7 +390,7 @@ private void migrateRetainedColumnToString(String storedName, + (attributes.comment() == null || attributes.comment().isEmpty() ? "" : " COMMENT '" + quoteMysqlLiteral(attributes.comment()) + "'") + ";"; - try (Connection connection = getMysql().getConnectionManager().getConnection(); + try (Connection connection = getMysql().getConnectionManager().getConnectionChecked(); PreparedStatement statement = connection.prepareStatement(sql)) { statement.executeUpdate(); } catch (SQLException ddlFailure) { @@ -412,7 +412,7 @@ private void migrateRetainedColumnToString(String storedName, .append(" SET DEFAULT (").append(defaultExpression).append(")::text") .append(declaredStorageType ? "::" + targetType : ""); sql.append(';'); - try (Connection connection = getMysql().getConnectionManager().getConnection(); + try (Connection connection = getMysql().getConnectionManager().getConnectionChecked(); PreparedStatement statement = connection.prepareStatement(sql.toString())) { statement.executeUpdate(); } catch (SQLException ddlFailure) { @@ -585,7 +585,7 @@ private static int[] declaredTypeParameters(String sqlType) { private MysqlColumnAttributes mysqlColumnAttributes(String name) throws SQLException { String sql = "SELECT IS_NULLABLE, COLUMN_DEFAULT, EXTRA, COLUMN_COMMENT, COLUMN_TYPE " + "FROM information_schema.COLUMNS WHERE TABLE_SCHEMA=DATABASE() AND TABLE_NAME=? AND COLUMN_NAME=?"; - try (Connection connection = getMysql().getConnectionManager().getConnection(); + try (Connection connection = getMysql().getConnectionManager().getConnectionChecked(); PreparedStatement statement = connection.prepareStatement(sql)) { statement.setString(1, tableName); statement.setString(2, name); @@ -643,7 +643,7 @@ private String postgresColumnDefault(String name) throws SQLException { + "AND default_value.adnum=attribute.attnum " + "WHERE attribute.attrelid=pg_catalog.to_regclass(?) AND attribute.attname=? " + "AND attribute.attnum>0 AND NOT attribute.attisdropped"; - try (Connection connection = getMysql().getConnectionManager().getConnection(); + try (Connection connection = getMysql().getConnectionManager().getConnectionChecked(); PreparedStatement statement = connection.prepareStatement(sql)) { statement.setString(1, regclass); statement.setString(2, name); @@ -654,7 +654,7 @@ private String postgresColumnDefault(String name) throws SQLException { } private RegisteredColumnType registeredColumnType(String name) throws SQLException { - try (Connection connection = getMysql().getConnectionManager().getConnection(); + try (Connection connection = getMysql().getConnectionManager().getConnectionChecked(); PreparedStatement statement = connection.prepareStatement( "SELECT * FROM " + quote(tableName) + " WHERE 1=0"); ResultSet result = statement.executeQuery()) { @@ -675,7 +675,7 @@ private void rememberColumn(SqlUserSchema.ColumnDefinition column) { } private String findRegisteredColumn(String name) throws SQLException { - try (Connection connection = getMysql().getConnectionManager().getConnection(); PreparedStatement statement = connection.prepareStatement("SELECT * FROM " + quote(tableName) + " WHERE 1=0"); ResultSet result = statement.executeQuery()) { + try (Connection connection = getMysql().getConnectionManager().getConnectionChecked(); PreparedStatement statement = connection.prepareStatement("SELECT * FROM " + quote(tableName) + " WHERE 1=0"); ResultSet result = statement.executeQuery()) { ResultSetMetaData metadata = result.getMetaData(); String exactMatch = null; String foldedMatch = null; int foldedMatches = 0; for (int i = 1; i <= metadata.getColumnCount(); i++) { String storedName = metadata.getColumnName(i); diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/core/user/storage/sql/MysqlBackendReviewRegressionTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/core/user/storage/sql/MysqlBackendReviewRegressionTest.java index e1fe5a07e..d64961f0d 100644 --- a/AdvancedCore/src/test/java/com/bencodez/advancedcore/core/user/storage/sql/MysqlBackendReviewRegressionTest.java +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/core/user/storage/sql/MysqlBackendReviewRegressionTest.java @@ -9,6 +9,10 @@ import java.sql.ResultSet; import java.sql.ResultSetMetaData; import java.sql.SQLException; +import java.sql.SQLTransientConnectionException; +import java.util.UUID; +import com.bencodez.advancedcore.api.user.UserStorage; +import com.bencodez.simpleapi.sql.data.DataValueInt; import java.util.ArrayList; import java.util.List; import java.util.Locale; @@ -119,6 +123,35 @@ private void existingSpelling(DbType type, String name) throws Exception { fixture.assertClosed(); } + @Test void sqlRequiredOperationsPreserveAcquisitionTimeoutAndRecover() throws Exception { + for (DbType type : new DbType[]{DbType.MYSQL, DbType.MARIADB, DbType.POSTGRESQL}) { + Fixture fixture = new Fixture(type, "Points"); + try (var managers = fixture.managers(); var backend = fixture.open()) { + ConnectionManager manager = managers.constructed().get(0); + var user = backend.user(UUID.randomUUID()); + SQLTransientConnectionException timeout = new SQLTransientConnectionException("Pool exhausted", "08001"); + doReturn(null).when(manager).getConnection(); + doThrow(timeout).when(manager).getConnectionChecked(); + List operations = List.of( + () -> user.readRow(UserStorage.MYSQL), + () -> user.contains(UserStorage.MYSQL), + () -> user.delete(UserStorage.MYSQL), + () -> user.write(UserStorage.MYSQL, "Points", new DataValueInt(7)), + () -> user.transaction(UserStorage.MYSQL, transaction -> fail("No transaction work before acquisition")), + backend::enumerateUsers); + for (var operation : operations) { + assertSame(timeout, assertThrows(IllegalStateException.class, operation).getCause(), type.name()); + assertTrue(backend.isOpen(), "Timeout must not close backend"); + } + doAnswer(ignored -> fixture.connection()).when(manager).getConnectionChecked(); + assertTrue(user.readRow(UserStorage.MYSQL).isEmpty()); + assertFalse(user.contains(UserStorage.MYSQL)); + assertTrue(backend.enumerateUsers().isEmpty()); + } + fixture.assertClosed(); + } + } + private static final class Fixture { final DbType type; final String storedColumn; diff --git a/docs/shared-sql-user-access.md b/docs/shared-sql-user-access.md index 45a786011..373e69c31 100644 --- a/docs/shared-sql-user-access.md +++ b/docs/shared-sql-user-access.md @@ -72,3 +72,18 @@ If storage is unavailable, leave publication invalid and report the refresh failure. Cache-only/no-database fetch modes retain their explicit default-return contract. This is snapshot coherence, not a guarantee that another backend cannot commit a later update or that external side effects share the SQL transaction. + +## SQL acquisition failures + +The headless MySQL/MariaDB/PostgreSQL user backend and Bukkit user-table SQL +paths acquire connections with SimpleAPI's additive `getConnectionChecked()`. +A pool timeout reaches the existing SQL failure handling with its original +`SQLException` cause instead of dereferencing a null connection. Shared user +operations wrap that cause in `IllegalStateException`; failed acquisition never +enters the transaction callback, replaces the pool, or closes the backend. +Operations may be retried after a lease is released. + +The public legacy SimpleAPI `getConnection()` retains its nullable failure +contract for existing callers. Rebuild both AdvancedCore and shaded VotingPlugin +consumers against SimpleAPI containing merged PR #106 before validating the +packaged storage API. From 630aaff484c770bcc4e04b1d72032cac1d1588f2 Mon Sep 17 00:00:00 2001 From: BenCodez <17074231+BenCodez@users.noreply.github.com> Date: Sat, 3 Oct 2026 13:28:37 -0600 Subject: [PATCH 2/3] Propagate PostgreSQL UUID lookup SQL failures --- .../api/user/userstorage/mysql/MySQL.java | 2 +- .../PostgresUuidLookupFailureTest.java | 104 ++++++++++++++++++ docs/shared-sql-user-access.md | 5 +- 3 files changed, 109 insertions(+), 2 deletions(-) create mode 100644 AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/storage/PostgresUuidLookupFailureTest.java diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/userstorage/mysql/MySQL.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/userstorage/mysql/MySQL.java index 9c8cd3cc5..5fc32be88 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/userstorage/mysql/MySQL.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/userstorage/mysql/MySQL.java @@ -168,7 +168,7 @@ public boolean containsKeyQuery(String uuid) { } } catch (SQLException e) { debug(e); - return false; + throw new IllegalStateException("Failed to check UUID existence", e); } } diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/storage/PostgresUuidLookupFailureTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/storage/PostgresUuidLookupFailureTest.java new file mode 100644 index 000000000..86f03e571 --- /dev/null +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/storage/PostgresUuidLookupFailureTest.java @@ -0,0 +1,104 @@ +package com.bencodez.advancedcore.tests.storage; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +import java.lang.reflect.Field; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.SQLTransientConnectionException; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; + +import org.junit.jupiter.api.Test; + +import com.bencodez.advancedcore.AdvancedCorePlugin; +import com.bencodez.advancedcore.api.user.UserStorage; +import com.bencodez.advancedcore.api.user.userstorage.mysql.MySQL; +import com.bencodez.advancedcore.bukkit.user.storage.BukkitSqlUserStorage; +import com.bencodez.simpleapi.sql.mysql.AbstractSqlTable; +import com.bencodez.simpleapi.sql.mysql.ConnectionManager; +import com.bencodez.simpleapi.sql.mysql.DbType; + +class PostgresUuidLookupFailureTest { + private static final String UUID = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"; + + @Test void legacyFacadePreservesAcquisitionCauseAndRecovers() throws Exception { + Fixture f = new Fixture(); + SQLException timeout = new SQLTransientConnectionException("pool exhausted", "08001"); + when(f.manager.getConnectionChecked()).thenThrow(timeout); + assertSame(timeout, assertThrows(IllegalStateException.class, + () -> f.storage.contains(UserStorage.MYSQL)).getCause()); + verify(f.plugin).debug(timeout); + + f.successfulLookup(false); + assertFalse(f.storage.contains(UserStorage.MYSQL)); + when(f.result.next()).thenReturn(true); + assertTrue(f.storage.contains(UserStorage.MYSQL)); + verify(f.statement, times(2)).setString(1, UUID); + verify(f.connection, times(2)).close(); + verify(f.statement, times(2)).close(); + verify(f.result, times(2)).close(); + verify(f.manager, never()).close(); + } + + @Test void queryFailurePreservesCauseAndClosesBorrowedResources() throws Exception { + Fixture f = new Fixture(); + f.successfulLookup(false); + SQLException failure = new SQLException("query failed", "08006"); + when(f.statement.executeQuery()).thenThrow(failure); + assertSame(failure, assertThrows(IllegalStateException.class, + () -> f.table.containsKeyQuery(UUID)).getCause()); + verify(f.connection).close(); + verify(f.statement).close(); + verify(f.plugin).debug(failure); + } + + @Test void malformedUuidAndCachedIdentityDoNotAcquireConnection() throws Exception { + Fixture f = new Fixture(); + assertFalse(f.table.containsKeyQuery("invalid-uuid")); + f.uuids.add(UUID); + assertTrue(f.storage.contains(UserStorage.MYSQL)); + verifyNoInteractions(f.manager); + } + + private static final class Fixture { + final MySQL table = mock(MySQL.class, CALLS_REAL_METHODS); + final AdvancedCorePlugin plugin = mock(AdvancedCorePlugin.class); + final ConnectionManager manager = mock(ConnectionManager.class); + final Connection connection = mock(Connection.class); + final PreparedStatement statement = mock(PreparedStatement.class); + final ResultSet result = mock(ResultSet.class); + final Set uuids = ConcurrentHashMap.newKeySet(); + final BukkitSqlUserStorage storage = new BukkitSqlUserStorage(() -> plugin, () -> UUID); + + Fixture() throws Exception { + var wrapper = mock(com.bencodez.simpleapi.sql.mysql.MySQL.class); + when(wrapper.getConnectionManager()).thenReturn(manager); + set(AbstractSqlTable.class, "mysql", wrapper); + set(AbstractSqlTable.class, "dbType", DbType.POSTGRESQL); + set(AbstractSqlTable.class, "tableName", "Users"); + set(MySQL.class, "plugin", plugin); + set(MySQL.class, "uuids", uuids); + // An existing cache entry avoids the separate whole-table refresh path. + uuids.add("11111111-2222-3333-4444-555555555555"); + when(plugin.getMysql()).thenReturn(table); + } + + void successfulLookup(boolean exists) throws Exception { + doReturn(connection).when(manager).getConnectionChecked(); + when(connection.prepareStatement("SELECT 1 FROM \"Users\" WHERE \"uuid\" = ?::uuid LIMIT 1;")) + .thenReturn(statement); + when(statement.executeQuery()).thenReturn(result); + when(result.next()).thenReturn(exists); + } + + void set(Class owner, String name, Object value) throws Exception { + Field field = owner.getDeclaredField(name); + field.setAccessible(true); + field.set(table, value); + } + } +} diff --git a/docs/shared-sql-user-access.md b/docs/shared-sql-user-access.md index 373e69c31..a8566827c 100644 --- a/docs/shared-sql-user-access.md +++ b/docs/shared-sql-user-access.md @@ -81,7 +81,10 @@ A pool timeout reaches the existing SQL failure handling with its original `SQLException` cause instead of dereferencing a null connection. Shared user operations wrap that cause in `IllegalStateException`; failed acquisition never enters the transaction callback, replaces the pool, or closes the backend. -Operations may be retried after a lease is released. +Operations may be retried after a lease is released. The Bukkit PostgreSQL UUID +existence lookup also propagates acquisition and query failures with their cause; +only a successful lookup without a row means absent. Malformed UUID input still +returns false before acquiring a connection, and cached identities remain local. The public legacy SimpleAPI `getConnection()` retains its nullable failure contract for existing callers. Rebuild both AdvancedCore and shaded VotingPlugin From 2304bff3fa976b373ba5636aec3d6e4a02df1ae0 Mon Sep 17 00:00:00 2001 From: BenCodez <17074231+BenCodez@users.noreply.github.com> Date: Sat, 3 Oct 2026 14:59:05 -0600 Subject: [PATCH 3/3] Preserve Bukkit shared SQL read and delete failures --- .../api/user/userstorage/mysql/MySQL.java | 21 +++ .../user/storage/BukkitSqlUserBackend.java | 4 +- .../BukkitSqlFailurePropagationTest.java | 150 ++++++++++++++++++ .../user/BukkitUserRuntimeBootstrapTest.java | 10 +- docs/shared-sql-user-access.md | 6 + 5 files changed, 184 insertions(+), 7 deletions(-) create mode 100644 AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/storage/BukkitSqlFailurePropagationTest.java diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/userstorage/mysql/MySQL.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/userstorage/mysql/MySQL.java index 5fc32be88..dfda1db2b 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/userstorage/mysql/MySQL.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/userstorage/mysql/MySQL.java @@ -573,6 +573,15 @@ public ConcurrentHashMap getRowsUUIDNameQuery() { // ------------------------- public void deletePlayer(String uuid) { + deletePlayer(uuid, false); + } + + /** Delete for shared storage: a failed operation must not acknowledge cache removal. */ + public void deletePlayerStrict(String uuid) { + deletePlayer(uuid, true); + } + + private void deletePlayer(String uuid, boolean strict) { String q = "DELETE FROM " + qi(tableName) + " WHERE " + qi("uuid") + "=?;"; plugin.devDebug("DB QUERY: " + q); @@ -582,6 +591,8 @@ public void deletePlayer(String uuid) { statement.executeUpdate(); } catch (SQLException | IllegalArgumentException e) { debug(e); + if (strict) throw new IllegalStateException("Failed to delete SQL user", e); + return; } uuids.remove(uuid); @@ -680,7 +691,16 @@ public ArrayList getExact(String uuid) { return getExactQuery(new Column("uuid", new DataValueString(uuid))); } + /** Read for shared storage: never publish a schema-only snapshot after SQL failure. */ + public ArrayList getExactStrict(String uuid) { + return getExactQuery(new Column("uuid", new DataValueString(uuid)), true); + } + public ArrayList getExactQuery(Column column) { + return getExactQuery(column, false); + } + + private ArrayList getExactQuery(Column column, boolean strict) { ArrayList result = new ArrayList<>(); String query = "SELECT * FROM " + qi(tableName) + " WHERE " + qi(column.getName()) + "=?;"; @@ -731,6 +751,7 @@ public ArrayList getExactQuery(Column column) { } } catch (SQLException | ArrayIndexOutOfBoundsException e) { debug(e); + if (strict) throw new IllegalStateException("Failed to read SQL user", e); } for (String col : getColumns()) { diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/bukkit/user/storage/BukkitSqlUserBackend.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/bukkit/user/storage/BukkitSqlUserBackend.java index 1122412e3..904e92b49 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/bukkit/user/storage/BukkitSqlUserBackend.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/bukkit/user/storage/BukkitSqlUserBackend.java @@ -133,7 +133,7 @@ private IllegalStateException enumerationFailure(String storage, Throwable failu private List read(UserStorage storage, UUID uuid) { requireOpen(); requireStorage(storage); - if (storage == UserStorage.MYSQL) return mysql().getExact(uuid.toString()); + if (storage == UserStorage.MYSQL) return mysql().getExactStrict(uuid.toString()); synchronized (sqliteOperations) { return table().getExact(primary(uuid)); } } @@ -147,7 +147,7 @@ private boolean contains(UserStorage storage, UUID uuid) { private void delete(UserStorage storage, UUID uuid) { requireOpen(); requireStorage(storage); - if (storage == UserStorage.MYSQL) mysql().deletePlayer(uuid.toString()); + if (storage == UserStorage.MYSQL) mysql().deletePlayerStrict(uuid.toString()); else synchronized (sqliteOperations) { table().delete(primary(uuid)); } } diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/storage/BukkitSqlFailurePropagationTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/storage/BukkitSqlFailurePropagationTest.java new file mode 100644 index 000000000..621f5b40d --- /dev/null +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/storage/BukkitSqlFailurePropagationTest.java @@ -0,0 +1,150 @@ +package com.bencodez.advancedcore.tests.storage; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; + +import java.lang.reflect.Field; +import java.sql.*; +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; + +import org.junit.jupiter.api.Test; + +import com.bencodez.advancedcore.AdvancedCorePlugin; +import com.bencodez.advancedcore.api.user.UserStorage; +import com.bencodez.advancedcore.api.user.userstorage.mysql.MySQL; +import com.bencodez.advancedcore.bukkit.user.storage.BukkitSqlUserBackend; +import com.bencodez.advancedcore.core.user.runtime.SharedUserDataRuntime; +import com.bencodez.advancedcore.core.user.runtime.UserCacheOwner; +import com.bencodez.simpleapi.sql.mysql.AbstractSqlTable; +import com.bencodez.simpleapi.sql.mysql.ConnectionManager; +import com.bencodez.simpleapi.sql.mysql.DbType; +import com.bencodez.advancedcore.api.player.UuidLookup; + +class BukkitSqlFailurePropagationTest { + private static final UUID ID = UUID.fromString("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"); + + @Test void failedReadCannotPublishSnapshotAndRetryCanReadEmptyRowMysql() throws Exception { failedReadCannotPublishSnapshotAndRetryCanReadEmptyRow(DbType.MYSQL); } + @Test void failedReadCannotPublishSnapshotAndRetryCanReadEmptyRowPostgres() throws Exception { failedReadCannotPublishSnapshotAndRetryCanReadEmptyRow(DbType.POSTGRESQL); } + + private void failedReadCannotPublishSnapshotAndRetryCanReadEmptyRow(DbType dialect) throws Exception { + Fixture f = new Fixture(dialect); + SQLException failure = new SQLTransientConnectionException("pool exhausted", "08001"); + when(f.manager.getConnectionChecked()).thenThrow(failure); + assertSame(failure, assertThrows(IllegalStateException.class, () -> f.runtime.populate(ID)).getCause()); + verify(f.cache, never()).completePopulation(any(), any(), any()); + verify(f.cache, never()).populate(any(), any()); + f.connect(); + when(f.statement.executeQuery()).thenReturn(f.result); + when(f.result.next()).thenReturn(false); + doAnswer(call -> call.getArgument(1)).when(f.cache).completePopulation(eq(ID), any(), any()); + assertTrue(f.runtime.populate(ID).isEmpty()); + verify(f.cache).completePopulation(eq(ID), argThat(java.util.Map::isEmpty), any()); + verify(f.connection).close(); + verify(f.statement).close(); + verify(f.result).close(); + verify(f.manager, never()).close(); + } + + @Test void failedDeleteKeepsIdentityAndCacheAndRetryCompletesMysql() throws Exception { failedDeleteKeepsIdentityAndCacheAndRetryCompletes(DbType.MYSQL); } + @Test void failedDeleteKeepsIdentityAndCacheAndRetryCompletesPostgres() throws Exception { failedDeleteKeepsIdentityAndCacheAndRetryCompletes(DbType.POSTGRESQL); } + + private void failedDeleteKeepsIdentityAndCacheAndRetryCompletes(DbType dialect) throws Exception { + Fixture f = new Fixture(dialect); + SQLException failure = new SQLTransientConnectionException("pool exhausted", "08001"); + when(f.manager.getConnectionChecked()).thenThrow(failure); + assertSame(failure, assertThrows(IllegalStateException.class, () -> f.runtime.remove(ID)).getCause()); + assertTrue(f.uuids.contains(ID.toString())); + assertTrue(f.names.contains("Player")); + verify(f.cache, never()).remove(ID); + verify(f.cache).cancelRemoval(ID); + verify(f.table, never()).clearCacheBasic(); + f.connect(); + doNothing().when(f.table).clearCacheBasic(); + try (var lookup = mockStatic(UuidLookup.class)) { + UuidLookup resolver = mock(UuidLookup.class); + lookup.when(UuidLookup::getInstance).thenReturn(resolver); + when(resolver.getCachedName(ID.toString())).thenReturn("Player"); + f.runtime.remove(ID); + } + verify(f.statement).executeUpdate(); + verify(f.cache).remove(ID); + assertFalse(f.uuids.contains(ID.toString())); + assertFalse(f.names.contains("Player")); + verify(f.connection).close(); + verify(f.statement).close(); + verify(f.manager, never()).close(); + } + + @Test void queryFailuresAlsoAbortReadAndDeleteMysql() throws Exception { queryFailuresAlsoAbortReadAndDelete(DbType.MYSQL); } + @Test void queryFailuresAlsoAbortReadAndDeletePostgres() throws Exception { queryFailuresAlsoAbortReadAndDelete(DbType.POSTGRESQL); } + + private void queryFailuresAlsoAbortReadAndDelete(DbType dialect) throws Exception { + Fixture f = new Fixture(dialect); + f.connect(); + SQLException failure = new SQLException("query failed", "08006"); + when(f.statement.executeQuery()).thenThrow(failure); + when(f.statement.executeUpdate()).thenThrow(failure); + assertSame(failure, assertThrows(IllegalStateException.class, () -> f.runtime.populate(ID)).getCause()); + assertSame(failure, assertThrows(IllegalStateException.class, () -> f.runtime.remove(ID)).getCause()); + verify(f.cache, never()).completePopulation(any(), any(), any()); + verify(f.cache, never()).remove(ID); + assertTrue(f.uuids.contains(ID.toString())); + assertTrue(f.names.contains("Player")); + verify(f.connection, times(2)).close(); + verify(f.statement, times(2)).close(); + } + + @Test void legacyReadFallbackRemainsAndFailedLegacyDeleteKeepsIdentity() throws Exception { + Fixture f = new Fixture(DbType.MYSQL); + when(f.manager.getConnectionChecked()).thenThrow(new SQLTransientConnectionException("pool exhausted")); + doReturn(java.util.List.of("PlayerName")).when(f.table).getColumns(); + var row = f.table.getExact(ID.toString()); + assertEquals(1, row.size()); + assertEquals("PlayerName", row.get(0).getName()); + f.table.deletePlayer(ID.toString()); + assertTrue(f.uuids.contains(ID.toString())); + assertTrue(f.names.contains("Player")); + verify(f.table, never()).clearCacheBasic(); + } + + private static final class Fixture { + final MySQL table = mock(MySQL.class, CALLS_REAL_METHODS); + final AdvancedCorePlugin plugin = mock(AdvancedCorePlugin.class); + final ConnectionManager manager = mock(ConnectionManager.class); + final Connection connection = mock(Connection.class); + final PreparedStatement statement = mock(PreparedStatement.class); + final ResultSet result = mock(ResultSet.class); + final UserCacheOwner cache = mock(UserCacheOwner.class); + final Set uuids = ConcurrentHashMap.newKeySet(); + final Set names = ConcurrentHashMap.newKeySet(); + final SharedUserDataRuntime runtime; + + Fixture(DbType dialect) throws Exception { + var wrapper = mock(com.bencodez.simpleapi.sql.mysql.MySQL.class); + when(wrapper.getConnectionManager()).thenReturn(manager); + set(AbstractSqlTable.class, "mysql", wrapper); + set(AbstractSqlTable.class, "dbType", dialect); + set(AbstractSqlTable.class, "tableName", "Users"); + set(MySQL.class, "plugin", plugin); + set(MySQL.class, "uuids", uuids); + set(MySQL.class, "names", names); + uuids.add(ID.toString()); + names.add("Player"); + runtime = new SharedUserDataRuntime(new BukkitSqlUserBackend(plugin, UserStorage.MYSQL, table, null), cache); + } + + void connect() throws Exception { + doReturn(connection).when(manager).getConnectionChecked(); + when(connection.prepareStatement(anyString())).thenReturn(statement); + } + + void set(Class owner, String name, Object value) throws Exception { + Field field = owner.getDeclaredField(name); + field.setAccessible(true); + field.set(table, value); + } + } +} diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/BukkitUserRuntimeBootstrapTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/BukkitUserRuntimeBootstrapTest.java index 5f4b96f3c..dafce1567 100644 --- a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/BukkitUserRuntimeBootstrapTest.java +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/BukkitUserRuntimeBootstrapTest.java @@ -76,7 +76,7 @@ class BukkitUserRuntimeBootstrapTest { assertThrows(IllegalStateException.class, () -> manager.withSharedSqlBackend(UUID.randomUUID(), (storage, user) -> user.readRow(storage))); } - verify(mysql, never()).getExact(anyString()); + verify(mysql, never()).getExactStrict(anyString()); } finally { CountDownLatch retired = new CountDownLatch(1); assertTrue(manager.closeSharedRuntimeAsync(retired::countDown)); @@ -90,14 +90,14 @@ class BukkitUserRuntimeBootstrapTest { MySQL mysql = mock(MySQL.class); when(plugin.getStorageType()).thenReturn(UserStorage.MYSQL); when(plugin.getMysql()).thenReturn(mysql); - when(mysql.getExact(anyString())).thenReturn(new ArrayList<>()); + when(mysql.getExactStrict(anyString())).thenReturn(new ArrayList<>()); UserDataManager manager = new UserDataManager(plugin); try { BukkitUserRuntimeBootstrap.bindAfterStorageInitialization(plugin, manager); assertTrue(manager.hasSharedRuntime()); UUID uuid = UUID.randomUUID(); manager.withSharedSqlBackend(uuid, (storage, user) -> user.readRow(storage)); - verify(mysql).getExact(uuid.toString()); + verify(mysql).getExactStrict(uuid.toString()); CountDownLatch retired = new CountDownLatch(1); assertTrue(manager.closeSharedRuntimeAsync(retired::countDown)); @@ -127,7 +127,7 @@ class BukkitUserRuntimeBootstrapTest { AtomicReference configured = new AtomicReference<>(UserStorage.MYSQL); when(plugin.getStorageType()).thenAnswer(ignored -> configured.get()); when(plugin.getMysql()).thenReturn(mysql); - when(mysql.getExact(anyString())).thenReturn(new ArrayList<>()); + when(mysql.getExactStrict(anyString())).thenReturn(new ArrayList<>()); UserDataManager manager = new UserDataManager(plugin); try { BukkitUserRuntimeBootstrap.bindAfterStorageInitialization(plugin, manager); @@ -139,7 +139,7 @@ class BukkitUserRuntimeBootstrapTest { return user.readRow(storage); }); - verify(mysql).getExact(uuid.toString()); + verify(mysql).getExactStrict(uuid.toString()); verify(plugin, never()).getSQLiteUserTable(); } finally { CountDownLatch retired = new CountDownLatch(1); diff --git a/docs/shared-sql-user-access.md b/docs/shared-sql-user-access.md index a8566827c..fd396158e 100644 --- a/docs/shared-sql-user-access.md +++ b/docs/shared-sql-user-access.md @@ -90,3 +90,9 @@ The public legacy SimpleAPI `getConnection()` retains its nullable failure contract for existing callers. Rebuild both AdvancedCore and shaded VotingPlugin consumers against SimpleAPI containing merged PR #106 before validating the packaged storage API. + +The Bukkit shared MySQL backend uses `getExactStrict` and `deletePlayerStrict` +to surface acquisition and query failures with their original cause. A failed read +cannot publish a schema-only/default snapshot, and a failed delete cannot remove +identity or runtime caches. Existing `getExact`/`getExactQuery` callers retain +the legacy read fallback; legacy deletion also leaves caches intact on SQL failure.