From 9522f35c069b1d8f0dc23ebcf9515aee95bd7d2d Mon Sep 17 00:00:00 2001 From: Thijs Van der Schaeghe Date: Wed, 7 Oct 2026 09:24:58 +0200 Subject: [PATCH] DB: the query log is opt-in; a connection failure throws Query log: Database::setQueryLogEnabled(true) turns it on (off by default, for every instance). When off nothing is stored, getAllQueries() returns an empty array and getLastQuery() returns null. When on it keeps the most recent Database::QUERY_LOG_LIMIT (5000) statements. getQueryCounter() and getOriginCounters() are unaffected, and a logger set with setLogger() still receives every statement. Connection: MySQL::connect() throws a DbException with the fixed message DbException::CONNECTION_FAILED instead of printing the error and exiting; isConnectionError() tells it apart from a failed statement. The driver's message and code are kept on the previous exception. A charset the server rejects is a connection failure too. The charset is set with mysqli::set_charset(). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DqZUi2UiBJP6rZoCSRcdpE --- claude.md | 3 + src/Neuron/DB/Database.php | 82 +++++++++++++------ src/Neuron/DB/MySQL.php | 102 +++++++++++++++++------ src/Neuron/Exceptions/DbException.php | 37 +++++++++ tests/DbExceptionTest.php | 27 +++++++ tests/DbQueryLogTest.php | 104 ++++++++++++++++++++++++ tests/FailingMySQL.php | 32 ++++++++ tests/FakeMysqliConnection.php | 27 +++++++ tests/LoggingTestDatabase.php | 17 ++++ tests/MySQLConnectFailureTest.php | 112 ++++++++++++++++++++++++++ tests/RecordingLogger.php | 19 +++++ 11 files changed, 510 insertions(+), 52 deletions(-) create mode 100644 tests/DbQueryLogTest.php create mode 100644 tests/FailingMySQL.php create mode 100644 tests/FakeMysqliConnection.php create mode 100644 tests/LoggingTestDatabase.php create mode 100644 tests/MySQLConnectFailureTest.php create mode 100644 tests/RecordingLogger.php diff --git a/claude.md b/claude.md index ea6dd77..3c05751 100644 --- a/claude.md +++ b/claude.md @@ -36,6 +36,9 @@ The primary input validation layer. All user input should be validated through ` ### Database Queries (`src/Neuron/DB/Query.php`) Always use parameterized queries via the `Query` class to prevent SQL injection. Never concatenate user input directly into SQL strings. +### Query log and connection errors (`src/Neuron/DB/Database.php`, `MySQL.php`) +The query log (`getAllQueries()`, `getLastQuery()`) is opt-in: call `Database::setQueryLogEnabled(true)` where the application shows it; it keeps the most recent `Database::QUERY_LOG_LIMIT` (5000) statements. `getQueryCounter()` and `getOriginCounters()` work either way. A connection failure throws a `DbException` with the fixed message `DbException::CONNECTION_FAILED` (`isConnectionError()`); the driver's error is the previous exception, for logging only. + ### Testing Tests are located in `tests/` and use PHPUnit. Database-dependent tests are grouped with `#[Group('database')]` and require a MySQL connection. CI runs tests excluding this group. diff --git a/src/Neuron/DB/Database.php b/src/Neuron/DB/Database.php index 91d02dd..ba3d4ea 100755 --- a/src/Neuron/DB/Database.php +++ b/src/Neuron/DB/Database.php @@ -10,6 +10,12 @@ abstract class Database protected $affected_rows = 0; protected $query_counter = 0; + /** The query log keeps at most this many statements (the most recent). */ + const QUERY_LOG_LIMIT = 5000; + + /** @var bool See setQueryLogEnabled (). */ + private static $queryLogEnabled = false; + protected $query_log = array (); protected $origin_counter = array (); @@ -125,7 +131,7 @@ public function rollback () } /** - * Just put a comment in the query log. + * Just put a comment in the query log (when it is enabled). * Does not connect to database. * @param $txt */ @@ -134,46 +140,51 @@ public function log ($txt) $this->addQueryLog ("/* " . $txt . " */"); } - protected function addQueryLog ($sSQL, $duration = null) + /** + * Keep the executed statements (with their bound values) in memory, so + * getAllQueries () and getLastQuery () can return them. Off by default: + * turn it on only where the application shows or inspects the log. + * Applies to every Database instance. + * @param bool $enabled + */ + public static function setQueryLogEnabled ($enabled) { - /* - if (! (defined ('DEBUG') && DEBUG) && !isset ($_GET['debug'])) - { - return; - } - */ + self::$queryLogEnabled = (bool) $enabled; + } - if (count ($this->query_log) > 5000) - { - array_shift ($this->query_log); - } + /** + * @return bool + */ + public static function isQueryLogEnabled () + { + return self::$queryLogEnabled; + } - $stacktrace = debug_backtrace (); + protected function addQueryLog ($sSQL, $duration = null) + { + $stacktrace = debug_backtrace (DEBUG_BACKTRACE_IGNORE_ARGS, 3); $origin = $stacktrace[1]; - //var_dump ($stacktrace[2]); - if (isset ($stacktrace[2]['class']) && $stacktrace[2]['class'] == 'Neuron\DB\Query') { $origin = $stacktrace[2]; } - $txt = '[' . number_format ($duration, 3) . ' s] '; - //$txt .= $origin['class'] . ' '; + // The origin counters hold no statement text, so they are always kept. + if (isset ($origin['file'])) + { + $this->increaseOriginCounter ($origin['file'], $origin['line']); + } - // QUery took longer than 1 second? - /* - if ($duration > 0.1) + if (!self::$queryLogEnabled && !isset ($this->logger)) { - $txt = '' . $txt . ''; + return; } - */ + $txt = '[' . number_format ((float) $duration, 3) . ' s] '; $txt .= trim ($sSQL); - $this->increaseOriginCounter ($origin['file'], $origin['line']); - //$txt .= '
' . $origin['file'] . ':' . $origin['line']; - + // A logger is a sink the application set itself; it is not gated. if (isset ($this->logger)) { $color = 'green'; @@ -185,14 +196,33 @@ protected function addQueryLog ($sSQL, $duration = null) $this->logger->log ('DB: ' . preg_replace('!\s+!', ' ', str_replace ("\t", " ", str_replace ("\n", "", $txt))), false, $color); } + if (!self::$queryLogEnabled) + { + return; + } + $this->query_log[] = $txt; + + if (count ($this->query_log) > self::QUERY_LOG_LIMIT) + { + array_shift ($this->query_log); + } } + /** + * @return string|null The most recent logged statement, null when the + * query log is off or empty. + */ public function getLastQuery () { - return $this->query_log[count ($this->query_log) - 1]; + $count = count ($this->query_log); + return $count > 0 ? $this->query_log[$count - 1] : null; } + /** + * @return string[] The logged statements; empty unless the query log was + * turned on with setQueryLogEnabled (true). + */ public function getAllQueries () { return $this->query_log; diff --git a/src/Neuron/DB/MySQL.php b/src/Neuron/DB/MySQL.php index 4ee45d6..1183aef 100755 --- a/src/Neuron/DB/MySQL.php +++ b/src/Neuron/DB/MySQL.php @@ -16,35 +16,85 @@ class MySQL extends Database private $connection; + /** + * @throws DbException with a fixed message when the connection or its + * charset cannot be set up. The driver's own error is the exception's + * previous; nothing is printed. + */ public function connect () { - if (!isset ($this->connection)) + if (isset ($this->connection)) { - Logger::getInstance ()->log ('Connecting database.'); - - try - { - $this->connection = new MySQLi - ( - Config::get ('database.mysql.host'), - Config::get ('database.mysql.username'), - Config::get ('database.mysql.password'), - Config::get ('database.mysql.database') - ); - - $this->connection->query ('SET names "' . Config::get ('database.mysql.charset') . '"'); - //$this->connection->query ("SET time_zone = '+00:00'"); - } - catch (Exception $e) - { - echo $e; - } - - if (mysqli_connect_errno ()) - { - printf ("Connect failed: %s\n", mysqli_connect_error()); - exit(); - } + return; + } + + Logger::getInstance ()->log ('Connecting database.'); + + try + { + $connection = $this->openConnection (); + $this->applyCharset ($connection, Config::get ('database.mysql.charset')); + } + catch (\Throwable $e) + { + throw DbException::connectionFailed ($e->getMessage (), $e->getCode ()); + } + + $this->connection = $connection; + } + + /** + * Open the connection described by the database.mysql config. + * @return mysqli + * @throws \Throwable when the connection cannot be made. + */ + protected function openConnection () + { + // Have mysqli throw, whatever the report mode: without it a failed + // connect raises a PHP warning that carries the host and the user. + $driver = new \mysqli_driver (); + $reportMode = $driver->report_mode; + mysqli_report (MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT); + + try + { + $connection = new MySQLi + ( + Config::get ('database.mysql.host'), + Config::get ('database.mysql.username'), + Config::get ('database.mysql.password'), + Config::get ('database.mysql.database') + ); + } + finally + { + mysqli_report ($reportMode); + } + + if ($connection->connect_errno) + { + throw new \RuntimeException ((string) $connection->connect_error, intval ($connection->connect_errno)); + } + + return $connection; + } + + /** + * Set the connection charset through the driver, so escaping uses it too. + * @param mysqli $connection + * @param string|null $charset nothing is set when empty + * @throws \Throwable when the server rejects the charset. + */ + protected function applyCharset ($connection, $charset) + { + if ($charset === null || $charset === '') + { + return; + } + + if (!$connection->set_charset ($charset)) + { + throw new \RuntimeException ((string) $connection->error, intval ($connection->errno)); } } diff --git a/src/Neuron/Exceptions/DbException.php b/src/Neuron/Exceptions/DbException.php index a51158a..bd5bb7e 100644 --- a/src/Neuron/Exceptions/DbException.php +++ b/src/Neuron/Exceptions/DbException.php @@ -10,8 +10,13 @@ class DbException /** MySQL error number for a UNIQUE / PRIMARY KEY violation. */ const DUPLICATE_ENTRY = 1062; + /** The message of every connection failure; it never carries driver detail. */ + const CONNECTION_FAILED = 'Database connection failed.'; + private $query; + private $connectionError = false; + private $mysqlErrorCode; /** @@ -29,6 +34,38 @@ public static function fromMysqliException (\mysqli_sql_exception $e, $query = n return $ex; } + /** + * The connection (or its charset) could not be set up. The message is + * fixed; what the driver reported is kept as the previous exception, for + * logging only: it can name the host and the user. + * @param string $driverMessage + * @param int|string $driverCode + * @return DbException + */ + public static function connectionFailed ($driverMessage, $driverCode = 0) + { + $code = is_numeric ($driverCode) ? intval ($driverCode) : 0; + + // A new exception rather than the driver's own: its stack trace does + // not hold the arguments of the connect call. + $cause = new \RuntimeException ((string) $driverMessage, $code); + + $ex = new self (self::CONNECTION_FAILED, $code, $cause); + $ex->setErrorCode ($code); + $ex->connectionError = true; + return $ex; + } + + /** + * True when the connection could not be set up, as opposed to a + * statement that failed. + * @return bool + */ + public function isConnectionError () + { + return $this->connectionError; + } + /** * @param string $query */ diff --git a/tests/DbExceptionTest.php b/tests/DbExceptionTest.php index d622565..d214210 100644 --- a/tests/DbExceptionTest.php +++ b/tests/DbExceptionTest.php @@ -31,4 +31,31 @@ public function testFromMysqliExceptionKeepsCodeQueryAndPrevious () $this->assertSame ($original, $e->getPrevious ()); $this->assertTrue ($e->isDuplicateEntry ()); } + + public function testConnectionFailedHasAFixedMessageAndKeepsTheCause () + { + $e = DbException::connectionFailed ("Can't connect to MySQL server on 'db.internal'", 2002); + + $this->assertSame ('Database connection failed.', $e->getMessage ()); + $this->assertSame (DbException::CONNECTION_FAILED, $e->getMessage ()); + $this->assertTrue ($e->isConnectionError ()); + $this->assertSame (2002, $e->getErrorCode ()); + $this->assertSame (2002, $e->getCode ()); + $this->assertNull ($e->getQuery ()); + $this->assertSame ("Can't connect to MySQL server on 'db.internal'", $e->getPrevious ()->getMessage ()); + $this->assertSame (2002, $e->getPrevious ()->getCode ()); + } + + public function testConnectionFailedAcceptsANonNumericCode () + { + $e = DbException::connectionFailed ('driver missing', 'HY000'); + + $this->assertSame (0, $e->getCode ()); + $this->assertSame ('Database connection failed.', $e->getMessage ()); + } + + public function testOtherErrorsAreNotConnectionErrors () + { + $this->assertFalse ((new DbException ('dup'))->setErrorCode (1062)->isConnectionError ()); + } } diff --git a/tests/DbQueryLogTest.php b/tests/DbQueryLogTest.php new file mode 100644 index 0000000..c7bda6a --- /dev/null +++ b/tests/DbQueryLogTest.php @@ -0,0 +1,104 @@ +assertFalse (Database::isQueryLogEnabled ()); + } + + public function testNothingIsStoredWhenTheLogIsOff () + { + $db = new LoggingTestDatabase (); + + $db->query ("UPDATE users SET password = 'secret-hash' WHERE id = 1"); + $db->log ('a comment'); + + $this->assertSame (array (), $db->getAllQueries ()); + $this->assertNull ($db->getLastQuery ()); + } + + public function testCountersKeepWorkingWhenTheLogIsOff () + { + $db = new LoggingTestDatabase (); + + $db->query ('SELECT 1'); + $db->query ('SELECT 2'); + + $this->assertSame (2, $db->getQueryCounter ()); + $this->assertCount (2, $db->getOriginCounters ()); + } + + public function testStatementsAreStoredWhenTheLogIsOn () + { + Database::setQueryLogEnabled (true); + $this->assertTrue (Database::isQueryLogEnabled ()); + + $db = new LoggingTestDatabase (); + $db->query ('SELECT 1'); + $db->log ('a comment'); + + $queries = $db->getAllQueries (); + $this->assertCount (2, $queries); + $this->assertStringContainsString ('SELECT 1', $queries[0]); + $this->assertStringContainsString ('/* a comment */', $queries[1]); + $this->assertSame ($queries[1], $db->getLastQuery ()); + } + + public function testTurningTheLogOffStopsStoringButKeepsWhatWasLogged () + { + Database::setQueryLogEnabled (true); + $db = new LoggingTestDatabase (); + $db->query ('SELECT 1'); + + Database::setQueryLogEnabled (false); + $db->query ('SELECT 2'); + + $this->assertCount (1, $db->getAllQueries ()); + + $db->flushLog (); + $this->assertSame (array (), $db->getAllQueries ()); + $this->assertSame (0, $db->getQueryCounter ()); + } + + public function testTheLogKeepsOnlyTheMostRecentStatements () + { + Database::setQueryLogEnabled (true); + $db = new LoggingTestDatabase (); + + $total = Database::QUERY_LOG_LIMIT + 3; + for ($i = 1; $i <= $total; $i ++) { + $db->query ('SELECT ' . $i); + } + + $queries = $db->getAllQueries (); + $this->assertSame (5000, Database::QUERY_LOG_LIMIT); + $this->assertCount (Database::QUERY_LOG_LIMIT, $queries); + $this->assertStringEndsWith ('SELECT 4', $queries[0]); + $this->assertStringEndsWith ('SELECT ' . $total, $queries[count ($queries) - 1]); + $this->assertSame ($total, $db->getQueryCounter ()); + } + + public function testALoggerStillReceivesStatementsWhenTheLogIsOff () + { + $logger = new RecordingLogger (); + $db = new LoggingTestDatabase (); + $db->setLogger ($logger); + + $db->query ('SELECT 1'); + + $this->assertCount (1, $logger->lines); + $this->assertStringContainsString ('SELECT 1', $logger->lines[0]); + $this->assertSame (array (), $db->getAllQueries ()); + } +} diff --git a/tests/FailingMySQL.php b/tests/FailingMySQL.php new file mode 100644 index 0000000..980408e --- /dev/null +++ b/tests/FailingMySQL.php @@ -0,0 +1,32 @@ +attempts ++; + + if ($this->openError) { + throw $this->openError; + } + + return $this->fakeConnection; + } +} diff --git a/tests/FakeMysqliConnection.php b/tests/FakeMysqliConnection.php new file mode 100644 index 0000000..00ffd87 --- /dev/null +++ b/tests/FakeMysqliConnection.php @@ -0,0 +1,27 @@ +charsets[] = $charset; + return $this->acceptCharset; + } +} diff --git a/tests/LoggingTestDatabase.php b/tests/LoggingTestDatabase.php new file mode 100644 index 0000000..35b9842 --- /dev/null +++ b/tests/LoggingTestDatabase.php @@ -0,0 +1,17 @@ +query_counter ++; + $this->addQueryLog ($sSQL, 0.001); + return 0; + } +} diff --git a/tests/MySQLConnectFailureTest.php b/tests/MySQLConnectFailureTest.php new file mode 100644 index 0000000..47a67d4 --- /dev/null +++ b/tests/MySQLConnectFailureTest.php @@ -0,0 +1,112 @@ +openError = new \RuntimeException (self::DRIVER_ERROR, 1045); + + ob_start (); + try { + $db->connect (); + $thrown = null; + } catch (DbException $e) { + $thrown = $e; + } + $output = ob_get_clean (); + + $this->assertSame ('', $output); + $this->assertInstanceOf (DbException::class, $thrown); + $this->assertSame (DbException::CONNECTION_FAILED, $thrown->getMessage ()); + $this->assertSame ('Database connection failed.', $thrown->getMessage ()); + $this->assertTrue ($thrown->isConnectionError ()); + $this->assertNull ($thrown->getQuery ()); + } + + public function testOriginalErrorStaysAvailableForLogging () + { + $db = new FailingMySQL (); + $db->openError = new \RuntimeException (self::DRIVER_ERROR, 1045); + + try { + $db->connect (); + $this->fail ('Expected a DbException'); + } catch (DbException $e) { + $this->assertSame (1045, $e->getErrorCode ()); + $this->assertNotNull ($e->getPrevious ()); + $this->assertSame (self::DRIVER_ERROR, $e->getPrevious ()->getMessage ()); + $this->assertSame (1045, $e->getPrevious ()->getCode ()); + $this->assertNotSame ($db->openError, $e->getPrevious ()); + } + } + + public function testErrorsThatAreNotExceptionsAreWrappedToo () + { + $db = new FailingMySQL (); + $db->openError = new \Error ('Class "mysqli" not found'); + + $this->expectException (DbException::class); + $this->expectExceptionMessage (DbException::CONNECTION_FAILED); + $db->connect (); + } + + public function testFailedConnectionIsNotKept () + { + $db = new FailingMySQL (); + $db->openError = new \RuntimeException (self::DRIVER_ERROR, 1045); + + try { + $db->connect (); + } catch (DbException $e) { + } + + $this->assertNull ($db->getConnection ()); + + $db->openError = null; + $db->fakeConnection = new FakeMysqliConnection (); + $db->connect (); + + $this->assertSame (2, $db->attempts); + $this->assertSame ($db->fakeConnection, $db->getConnection ()); + } + + public function testCharsetIsSetThroughTheDriver () + { + $db = new FailingMySQL (); + $db->fakeConnection = new FakeMysqliConnection (); + + $db->connect (); + $db->connect (); + + // example/config/database.php + $this->assertSame (array ('utf8'), $db->fakeConnection->charsets); + $this->assertSame (1, $db->attempts); + } + + public function testRejectedCharsetIsAConnectionFailure () + { + $db = new FailingMySQL (); + $db->fakeConnection = new FakeMysqliConnection (); + $db->fakeConnection->acceptCharset = false; + $db->fakeConnection->error = 'Unknown character set'; + $db->fakeConnection->errno = 2019; + + try { + $db->connect (); + $this->fail ('Expected a DbException'); + } catch (DbException $e) { + $this->assertSame (DbException::CONNECTION_FAILED, $e->getMessage ()); + $this->assertSame (2019, $e->getErrorCode ()); + $this->assertSame ('Unknown character set', $e->getPrevious ()->getMessage ()); + $this->assertNull ($db->getConnection ()); + } + } +} diff --git a/tests/RecordingLogger.php b/tests/RecordingLogger.php new file mode 100644 index 0000000..34fa8f4 --- /dev/null +++ b/tests/RecordingLogger.php @@ -0,0 +1,19 @@ +lines[] = $string; + } +}