Skip to content

Commit fdbf26e

Browse files
BenCodezBenCodez
authored
Merge pull request #111 from BenCodez/fix/sql-initialization-lifecycle
Handle SQL connection initialization failures safely
2 parents d491453 + cd3f650 commit fdbf26e

7 files changed

Lines changed: 396 additions & 20 deletions

File tree

‎SimpleAPI/src/main/java/com/bencodez/simpleapi/sql/mysql/AbstractSqlTable.java‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -129,19 +129,28 @@ public void severe(String string) {
129129
}
130130
};
131131

132-
boolean ok = this.mysql.connect(config);
132+
boolean ok = this.mysql.connect(config, false);
133133
if (!ok) {
134-
logSevere("Failed to connect to database. host=" + config.getHostName() + " db=" + config.getDatabase());
134+
IllegalStateException failure = new IllegalStateException("Failed to initialize SQL connection pool",
135+
this.mysql.getLastConnectFailure());
136+
try { mysql.disconnect(); }
137+
catch (RuntimeException | Error cleanupFailure) { failure.addSuppressed(cleanupFailure); }
138+
throw failure;
135139
}
136140

137141
// Keep the old behavior: USE db for MySQL/MariaDB
138142
if (this.dbType == DbType.MYSQL || this.dbType == DbType.MARIADB) {
139-
try {
140-
new Query(mysql, "USE `" + config.getDatabase() + "`;").executeUpdate();
141-
} catch (SQLException e) {
142-
logSevere("Failed to send USE database query: " + config.getDatabase()
143-
+ " (DB may still work depending on JDBC URL). Error: " + e.getMessage());
144-
debug(e);
143+
// Query.executeUpdate retains legacy logging/zero-result behavior on SQL
144+
// failure. Constructor setup needs a checked path without subclass hooks.
145+
try (Connection connection = mysql.getConnectionManager().getConnectionChecked();
146+
PreparedStatement statement = connection.prepareStatement("USE " + qi(config.getDatabase()) + ";")) {
147+
statement.executeUpdate();
148+
} catch (SQLException | RuntimeException | Error failure) {
149+
try { mysql.disconnect(); }
150+
catch (RuntimeException | Error cleanupFailure) { failure.addSuppressed(cleanupFailure); }
151+
if (failure instanceof RuntimeException unchecked) { throw unchecked; }
152+
if (failure instanceof Error fatal) { throw fatal; }
153+
throw new IllegalStateException("Failed to initialize SQL database selection", failure);
145154
}
146155
}
147156

‎SimpleAPI/src/main/java/com/bencodez/simpleapi/sql/mysql/ConnectionManager.java‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,12 @@ public class ConnectionManager {
3131

3232
@Getter
3333
private volatile HikariDataSource dataSource;
34+
/** The original initialization failure from the most recent open attempt. */
35+
private volatile Exception lastOpenFailure;
36+
37+
public Exception getLastOpenFailure() {
38+
return lastOpenFailure;
39+
}
3440

3541
@Getter
3642
@Setter
@@ -193,7 +199,7 @@ public Connection getConnectionChecked() throws SQLException {
193199
HikariDataSource current;
194200
synchronized (this) {
195201
if (isClosed() && !open()) {
196-
throw new SQLException("Unable to initialize SQL connection pool");
202+
throw new SQLException("Unable to initialize SQL connection pool", lastOpenFailure);
197203
}
198204
current = dataSource;
199205
}
@@ -384,6 +390,7 @@ private void rejectConflictingPostgreSqlTlsOptions(String extra) {
384390
// --- Pool Configuration ---
385391

386392
public synchronized boolean open() {
393+
lastOpenFailure = null;
387394
try {
388395
// If someone only set legacy flag but not dbType explicitly, keep it consistent
389396
if (dbType == null) {
@@ -438,7 +445,7 @@ public synchronized boolean open() {
438445
replaceDataSource(replacement);
439446
return true;
440447
} catch (Exception e) {
441-
e.printStackTrace();
448+
lastOpenFailure = e;
442449
return false;
443450
}
444451
}

‎SimpleAPI/src/main/java/com/bencodez/simpleapi/sql/mysql/MySQL.java‎

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@ public abstract class MySQL {
1111
private ConnectionManager connectionManager;
1212
private int maxConnections;
1313
private ExecutorService threadPool;
14+
private volatile Exception lastConnectFailure;
15+
16+
public Exception getLastConnectFailure() {
17+
return lastConnectFailure;
18+
}
1419

1520
/**
1621
* Create a new MySQL object with a default of 10 maximum threads.
@@ -31,6 +36,12 @@ public MySQL(int maxConnections) {
3136
}
3237

3338
public boolean connect(MysqlConfig config) {
39+
return connect(config, true);
40+
}
41+
42+
/** Connect without subclass diagnostics when called by a superclass constructor. */
43+
public boolean connect(MysqlConfig config, boolean emitDiagnostics) {
44+
lastConnectFailure = null;
3445
// (Re)size thread pool to match MaxConnections from config
3546
this.maxConnections = Math.max(1, config.getMaxThreads());
3647
if (threadPool != null) {
@@ -89,9 +100,13 @@ public boolean connect(MysqlConfig config) {
89100
}
90101

91102
boolean ok = connectionManager.open();
103+
if (!ok) {
104+
lastConnectFailure = connectionManager.getLastOpenFailure();
105+
// Legacy callers can retry through the connection manager. The owner
106+
// disconnects explicitly when construction is aborted instead.
107+
}
92108

93-
// Optional debug output
94-
if (config.isDebug()) {
109+
if (emitDiagnostics && config.isDebug()) {
95110
if (ok) {
96111
debug("DB connected. type=" + connectionManager.getDbType() + " host=" + config.getHostName() + " db="
97112
+ config.getDatabase() + " maxPool=" + maxConnections);
@@ -100,7 +115,6 @@ public boolean connect(MysqlConfig config) {
100115
+ " Check host/port/credentials and timeouts.");
101116
}
102117
}
103-
104118
return ok;
105119
}
106120

@@ -112,12 +126,17 @@ public boolean connect(MysqlConfig config) {
112126
* Close all connections and the data source.
113127
*/
114128
public void disconnect() {
115-
if (connectionManager != null) {
116-
connectionManager.close();
117-
}
118-
if (threadPool != null) {
119-
threadPool.shutdown();
120-
}
129+
try {
130+
if (connectionManager != null) connectionManager.close();
131+
} catch (RuntimeException | Error failure) {
132+
// Pool cleanup and worker retirement are independent responsibilities.
133+
try { if (threadPool != null) threadPool.shutdown(); }
134+
catch (RuntimeException | Error cleanup) {
135+
if (cleanup != failure) failure.addSuppressed(cleanup);
136+
}
137+
throw failure;
138+
}
139+
if (threadPool != null) threadPool.shutdown();
121140
}
122141

123142
/**

‎SimpleAPI/src/test/java/com/bencodez/simpleapi/sql/mysql/AbstractSqlTableInitializationTest.java‎

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
package com.bencodez.simpleapi.sql.mysql;
2+
3+
import static org.junit.jupiter.api.Assertions.*;
4+
import static org.mockito.Mockito.*;
5+
6+
import java.sql.Connection;
7+
import java.sql.Driver;
8+
import java.sql.DriverPropertyInfo;
9+
import java.sql.SQLException;
10+
import java.util.Properties;
11+
import java.util.logging.Logger;
12+
13+
import org.junit.jupiter.api.Test;
14+
15+
import com.bencodez.simpleapi.sql.mysql.config.MysqlConfig;
16+
17+
class AbstractSqlTableInitializationTest {
18+
@Test
19+
void deferredExistingPoolConstructorDoesNotNeedSubclassState() {
20+
MySQL mysql = mock(MySQL.class);
21+
ConnectionManager manager = mock(ConnectionManager.class);
22+
when(mysql.getConnectionManager()).thenReturn(manager);
23+
when(manager.getDbType()).thenReturn(DbType.MYSQL);
24+
DeferredTable table = new DeferredTable("votes", mysql);
25+
assertFalse(table.initialized);
26+
table.initialized = true;
27+
assertEquals("votes", table.getTableName());
28+
}
29+
@Test
30+
void authenticationFailureStopsConstructorBeforeSubclassHooksOrQueries() {
31+
RejectingTable.hookCalls = 0;
32+
RejectingDriver.connectionAttempts = 0;
33+
MysqlConfig config = config();
34+
config.setDriver(RejectingDriver.class.getName());
35+
36+
IllegalStateException failure = assertThrows(IllegalStateException.class,
37+
() -> new RejectingTable("votes", config, true));
38+
assertTrue(hasCause(failure, AUTH_FAILURE));
39+
assertEquals(0, RejectingTable.hookCalls);
40+
assertTrue(RejectingDriver.connectionAttempts > 0);
41+
}
42+
43+
@Test void databaseSelectionFailureClosesPoolWithoutCallingSubclassHooks() throws Exception {
44+
SQLException denied = new SQLException("controlled database selection rejected", "42000");
45+
RejectingTable.hookCalls = 0;
46+
try (var managers = mockConstruction(ConnectionManager.class, (manager, context) -> {
47+
when(manager.open()).thenReturn(true);
48+
Connection connection = mock(Connection.class);
49+
when(manager.getConnectionChecked()).thenReturn(connection);
50+
when(connection.prepareStatement(anyString())).thenThrow(denied);
51+
})) {
52+
IllegalStateException failure = assertThrows(IllegalStateException.class,
53+
() -> new RejectingTable("votes", config(), true));
54+
assertSame(denied, failure.getCause());
55+
assertEquals(0, RejectingTable.hookCalls);
56+
verify(managers.constructed().get(0)).close();
57+
}
58+
}
59+
60+
@Test void uncheckedDatabaseSelectionFailuresKeepIdentityAndCleanupFailures() throws Exception {
61+
for (Throwable failure : new Throwable[] {new IllegalArgumentException("driver rejected selection"),
62+
new LinkageError("driver linkage failure")}) {
63+
RuntimeException cleanupFailure = new IllegalStateException("controlled cleanup failure");
64+
RejectingTable.hookCalls = 0;
65+
try (var managers = mockConstruction(ConnectionManager.class, (manager, context) -> {
66+
when(manager.open()).thenReturn(true);
67+
Connection connection = mock(Connection.class);
68+
when(manager.getConnectionChecked()).thenReturn(connection);
69+
when(connection.prepareStatement(anyString())).thenThrow(failure);
70+
doThrow(cleanupFailure).when(manager).close();
71+
})) {
72+
Throwable thrown = assertThrows(failure.getClass(),
73+
() -> new RejectingTable("votes", config(), true));
74+
assertSame(failure, thrown);
75+
assertArrayEquals(new Throwable[] {cleanupFailure}, thrown.getSuppressed());
76+
assertEquals(0, RejectingTable.hookCalls);
77+
verify(managers.constructed().get(0)).close();
78+
verify(managers.constructed().get(0).getConnectionChecked()).close();
79+
}
80+
}
81+
}
82+
83+
@Test void databaseSelectionEscapesQuotedDatabaseNames() throws Exception {
84+
MysqlConfig config = config();
85+
config.setDatabase("vote`data");
86+
try (var managers = mockConstruction(ConnectionManager.class, (manager, context) -> {
87+
when(manager.open()).thenReturn(true);
88+
Connection connection = mock(Connection.class);
89+
when(manager.getConnectionChecked()).thenReturn(connection);
90+
when(connection.prepareStatement("USE `vote``data`;"))
91+
.thenReturn(mock(java.sql.PreparedStatement.class));
92+
})) {
93+
AbstractSqlTable table = new AbstractSqlTable("votes", config, true, true) {
94+
public String getPrimaryKeyColumn() { return "id"; }
95+
public String buildCreateTableSql(DbType type) { return "CREATE TABLE votes (id TEXT)"; }
96+
public void logSevere(String message) { fail("early hook"); }
97+
public void logInfo(String message) { fail("early hook"); }
98+
public void debug(Throwable failure) { fail("early hook"); }
99+
public void debug(String message) { fail("early hook"); }
100+
};
101+
verify(managers.constructed().get(0).getConnectionChecked())
102+
.prepareStatement("USE `vote``data`;");
103+
table.close();
104+
}
105+
}
106+
107+
private static MysqlConfig config() {
108+
MysqlConfig config = new MysqlConfig();
109+
config.setHostName("127.0.0.1");
110+
config.setPort(3306);
111+
config.setUser("user");
112+
config.setPass("password");
113+
config.setDatabase("database");
114+
config.setMinimumIdle(0);
115+
config.setMaxThreads(1);
116+
config.setConnectionTimeout(1000);
117+
return config;
118+
}
119+
120+
private static boolean hasCause(Throwable failure, String message) {
121+
for (Throwable current = failure; current != null; current = current.getCause()) {
122+
if (message.equals(current.getMessage())) return true;
123+
}
124+
return false;
125+
}
126+
127+
private static final String AUTH_FAILURE = "controlled authentication rejected";
128+
129+
private static final class DeferredTable extends AbstractSqlTable {
130+
boolean initialized;
131+
DeferredTable(String name, MySQL mysql) { super(name, mysql, true); }
132+
public String getPrimaryKeyColumn() { return "id"; }
133+
public String buildCreateTableSql(DbType type) { return "CREATE TABLE votes (id TEXT)"; }
134+
public void logSevere(String message) { if (!initialized) fail("early hook"); }
135+
public void logInfo(String message) { if (!initialized) fail("early hook"); }
136+
public void debug(Throwable failure) { if (!initialized) fail("early hook"); }
137+
public void debug(String message) { if (!initialized) fail("early hook"); }
138+
}
139+
140+
private static final class RejectingTable extends AbstractSqlTable {
141+
static int hookCalls;
142+
143+
RejectingTable(String name, MysqlConfig config, boolean debug) {
144+
super(name, config, debug);
145+
}
146+
147+
public String getPrimaryKeyColumn() { return "id"; }
148+
public String buildCreateTableSql(DbType type) { hookCalls++; return "CREATE TABLE votes (id TEXT)"; }
149+
public void logSevere(String message) { hookCalls++; }
150+
public void logInfo(String message) { hookCalls++; }
151+
public void debug(Throwable failure) { hookCalls++; }
152+
public void debug(String message) { hookCalls++; }
153+
}
154+
155+
public static final class RejectingDriver implements Driver {
156+
static int connectionAttempts;
157+
158+
public Connection connect(String url, Properties properties) throws SQLException {
159+
connectionAttempts++;
160+
throw new SQLException(AUTH_FAILURE);
161+
}
162+
public boolean acceptsURL(String url) { return url != null && url.startsWith("jdbc:"); }
163+
public DriverPropertyInfo[] getPropertyInfo(String url, Properties properties) { return new DriverPropertyInfo[0]; }
164+
public int getMajorVersion() { return 1; }
165+
public int getMinorVersion() { return 0; }
166+
public boolean jdbcCompliant() { return false; }
167+
public Logger getParentLogger() { return Logger.getGlobal(); }
168+
}
169+
}

0 commit comments

Comments
 (0)