From e45acc412b90837385a54b309fccf09a1aa5757d Mon Sep 17 00:00:00 2001 From: Clemens Schwaighofer Date: Thu, 18 May 2023 15:08:45 +0900 Subject: [PATCH] Add better error reporting to DB\IO for query with params On error with query with params the query was sent to the server and if ther query itself is ok but there is a problem with the parameters a wrong error message ($1 not found) will be returned Add pg_last_error reporting to catch this too. Update both error reporting to return not string and prefix combined but prefix + error string in array In error return check that both strings are not equal, so we do not return the same error string twice. Also default set dbh variable in the PgSQL class to false so it will skip last error report if there is no dbh set yet. Bug fix for db query with params debug output. if there are more than 9 entries the $1 of eg $10 is replaced with $1 entry again. Changed to '#' instead '$' to avoid this. Other: ACL\Login: replace EOM with HTML config.master: replace list() with [] Add single DB tester where we can test single db calls without adding more to the general test run --- 4dev/tests/DB/CoreLibsDBIOTest.php | 48 ++++---- www/admin/class_test.db.php | 85 +++++++++----- www/admin/class_test.db.single.php | 105 ++++++++++++++++++ www/admin/class_test.php | 1 + www/configs/config.master.php | 2 +- www/lib/CoreLibs/ACL/Login.php | 8 +- www/lib/CoreLibs/DB/IO.php | 37 ++++-- .../DB/SQL/Interface/SqlFunctions.php | 13 ++- www/lib/CoreLibs/DB/SQL/PgSQL.php | 37 ++++-- 9 files changed, 262 insertions(+), 74 deletions(-) create mode 100644 www/admin/class_test.db.single.php diff --git a/4dev/tests/DB/CoreLibsDBIOTest.php b/4dev/tests/DB/CoreLibsDBIOTest.php index 0ee6e878..20e90fda 100644 --- a/4dev/tests/DB/CoreLibsDBIOTest.php +++ b/4dev/tests/DB/CoreLibsDBIOTest.php @@ -156,7 +156,7 @@ final class CoreLibsDBIOTest extends TestCase $db->dbExec("DROP TABLE test_meta"); } // uid is for internal reference tests - $base_table = <<dbExec( // primary key name is table + '_id' - <<dbExec( - <<dbExec( - << false, 'array dims' => 0, 'is enum' => false, - 'is base' => 1, + 'is base' => true, 'is composite' => false, - 'is pesudo' => false, 'description' => '', + 'is pseudo' => false ], 'row_2' => [ 'num' => 2, @@ -1355,10 +1355,10 @@ final class CoreLibsDBIOTest extends TestCase 'has default' => false, 'array dims' => 0, 'is enum' => false, - 'is base' => 1, + 'is base' => true, 'is composite' => false, - 'is pesudo' => false, 'description' => '', + 'is pseudo' => false ] ] ], @@ -1374,10 +1374,10 @@ final class CoreLibsDBIOTest extends TestCase 'has default' => false, 'array dims' => 0, 'is enum' => false, - 'is base' => 1, + 'is base' => true, 'is composite' => false, - 'is pesudo' => false, 'description' => '', + 'is pseudo' => false ], 'row_2' => [ 'num' => 2, @@ -1387,10 +1387,10 @@ final class CoreLibsDBIOTest extends TestCase 'has default' => false, 'array dims' => 0, 'is enum' => false, - 'is base' => 1, + 'is base' => true, 'is composite' => false, - 'is pesudo' => false, 'description' => '', + 'is pseudo' => false ] ] ], @@ -4425,16 +4425,16 @@ final class CoreLibsDBIOTest extends TestCase ] ] ], - // same but as EOM - 'single insert (PK), EOM string' => [ - << [ + << [ - << [ + <<dbGetReturningExt()); // same as above but use an EOM string $some_time = time(); -$query = <<dbExec($query); print "EOM STRING DIRECT INSERT STATUS: " . Support::printToString($status) . " |
" . "QUERY: " . $db->dbGetQuery() . " |
" @@ -167,21 +167,21 @@ print "DIRECT INSERT PREVIOUS INSERTED: " . print_r($db->dbReturnRow("SELECT test_foo_id, test FROM test_foo " . "WHERE test_foo_id = " . (int)$last_insert_pk), true) . "
"; $__last_insert_pk = (int)$last_insert_pk; -$query = <<dbReturnRow($query), true) . "
"; print "LAST ERROR: " . $db->dbGetLastError() . "
"; print "
"; -$query = <<dbReturnRowParams( $query, @@ -208,7 +208,7 @@ foreach (['pk_name', 'count', 'query', 'returning_id'] as $key) { print "KEY: " . $key . ': ' . $db->dbGetPrepareCursorValue('ins_test_foo', $key) . "
"; } -$query = <<dbPrepare("ins_test_foo_eom", $query); $status = $db->dbExecute("ins_test_foo_eom", ['EOM BAR TEST ' . time()]); print "EOM STRING PREPARE INSERT[ins_test_foo_eom] STATUS: " . Support::printToString($status) . " |
" @@ -235,7 +235,7 @@ print "EOM STRING EXEC PARAMS INSERT STATUS: " . Support::printToString($status) . "RETURNING RETURN: " . print_r($db->dbGetReturningArray(), true) . "
"; // I/S Query -$query_insert = <<dbExecParams( $query_insert, @@ -313,18 +313,51 @@ print "EOM STRING EXEC RETURN TEST: " . print_r( [$__last_insert_id] ) ) . "
"; +// params > 10 for debug +// error catcher +$query_insert = <<dbExecParams($query_insert, $query_params); +echo "*
"; +echo "EOM STRING WITH MORE THAN 10 PARAMETERS: " + . Support::printToString($query_params) . " |
" + . " |
" + . "PRIMARY KEY: " . Support::printToString($db->dbGetInsertPK()) . " | " + . "RETURNING EXT: " . print_r($db->dbGetReturningExt(), true) . " | " + . "RETURNING RETURN: " . print_r($db->dbGetReturningArray(), true) + . "ERROR: " . $db->dbGetLastError(true) . "
"; echo "
"; // binary insert tests $filename = $db->dbEscapeLiteral('class_test.db.php'); $rand_bin_uid = $db->dbEscapeLiteral(\CoreLibs\Create\Uids::uniqIdShort()); $binary_data = $db->dbEscapeBytea(file_get_contents('class_test.db.php') ?: ''); -$query = <<dbExec($query); $__last_insert_id = $db->dbGetInsertPK(); print "BINARY DATA INSERT: " @@ -336,13 +369,13 @@ print "BINARY DATA INSERT: " . "ERROR: " . $db->dbGetLastError(true) . "
"; echo "*
"; -$query = <<dbExecParams($query, [$filename, $rand_bin_uid, $binary_data]); $__last_insert_id = $db->dbGetInsertPK(); print "BINARY DATA INSERT PARAMS: " @@ -380,7 +413,7 @@ print "DIRECT MULTIPLE INSERT WITH RETURN STATUS: " . Support::printToString($st $t_1 = time(); $t_2 = time(); $t_3 = time(); -$query = <<dbExec($query); print "EOM STRING DIRECT MULTIPLE INSERT WITH RETURN STATUS: " . Support::printToString($status) . " |
" . "QUERY: " . $db->dbGetQuery() . " |
" @@ -422,7 +455,7 @@ print "UPDATE WITH PK " . Support::printToString($last_insert_pk) . "RETURNING ARRAY: " . print_r($db->dbGetReturningArray(), true) . "
"; // UPDATE BUT EOM STYLE $status = $db->dbExecParams( - <<dbExecParams( tset_foo_id = ? RETURNING test_foo.test, string_a - EOM, + SQL, ['SOMETHING DIFFERENT EOM', (string)rand(1, 100)] ); print "UPDATE EOM WITH PK " . Support::printToString($last_insert_pk) @@ -524,27 +557,27 @@ echo "
"; print "EOM STYLE STRINGS
"; $test_bar = $db->dbEscapeLiteral('SOMETHING DIFFERENT'); // Test EOM block -$q = <<dbReturn($q))) { print "ROW: 
" . print_r($res, true) . "

"; } echo "
"; print "DB RETURN PARAMS
"; -$q = <<dbReturnParams($q, ['SOMETHING DIFFERENT'])) ) { @@ -632,14 +665,14 @@ print "Wrote to DB tabel $table with data " . print_r($data, true) . " and got p $query = "SELECT type, sdate, integer FROM foobar"; $data = $db->dbReturnArray($query, true); print "RETURN ARRAY: " . $db->dbGetNumRows() . ", Full foobar list: 
" . print_r($data, true) . "

"; -$query = <<dbReturnArrayParams($query, ['schmalz'], true); print "RETURN ARRAY PARAMS: " . $db->dbGetNumRows() . ", Full foobar list: 
"
 	. print_r($data, true) . "

"; diff --git a/www/admin/class_test.db.single.php b/www/admin/class_test.db.single.php new file mode 100644 index 00000000..c70edb4c --- /dev/null +++ b/www/admin/class_test.db.single.php @@ -0,0 +1,105 @@ + BASE . LOG, + 'file_id' => $LOG_FILE_ID, + // add file date + 'print_file_date' => true, + // set debug and print flags + 'debug_all' => $DEBUG_ALL ?? true, + 'echo_all' => $ECHO_ALL, + 'print_all' => $PRINT_ALL ?? true, +]); +// db connection and attach logger +$db = new CoreLibs\DB\IO(DB_CONFIG, $log); +$db->log->debug('START', '=============================>'); + +$PAGE_NAME = 'TEST CLASS: DB SINGLE'; +print ""; +print "" . $PAGE_NAME . ""; +print ""; +print ''; +print ''; +print '

' . $PAGE_NAME . '

'; + +print "LOGFILE NAME: " . $db->log->getSetting('log_file_name') . "
"; +print "LOGFILE ID: " . $db->log->getSetting('log_file_id') . "
"; +print "DBINFO: " . $db->dbInfo() . "
"; +// DB client encoding +print "DB client encoding: " . $db->dbGetEncoding() . "
"; +print "DB search path: " . $db->dbGetSchema() . "
"; + +$to_db_version = '15.2'; +print "VERSION DB: " . $db->dbVersion() . "
"; +print "SERVER ENCODING: " . $db->dbVersionInfo('server_encoding') . "
"; +if (($dbh = $db->dbGetDbh()) instanceof \PgSql\Connection) { + print "ALL OUTPUT [TEST]: 
" . print_r(pg_version($dbh), true) . "

"; +} else { + print "NO DB HANDLER
"; +} + +// params > 10 for debug +// error catcher +$query_insert = <<dbExecParams($query_insert, $query_params); +echo "*
"; +echo "EOM STRING WITH MORE THAN 10 PARAMETERS: " + . Support::printToString($query_params) . " |
" + . " |
" + . "PRIMARY KEY: " . Support::printToString($db->dbGetInsertPK()) . " | " + // . "RETURNING EXT: " . Support::printToString($db->dbGetReturningExt()) . " | " + . "RETURNING RETURN: " . Support::printToString($db->dbGetReturningArray()) + . "ERROR: " . $db->dbGetLastError(true) . "
"; +echo "
"; + +// error message +print $log->printErrorMsg(); + +print ""; + +// __END__ diff --git a/www/admin/class_test.php b/www/admin/class_test.php index 5fd5ec47..9c2b453c 100644 --- a/www/admin/class_test.php +++ b/www/admin/class_test.php @@ -73,6 +73,7 @@ print "TEST CLASS"; print ""; print ''; +print ''; print ''; print ''; print ''; diff --git a/www/configs/config.master.php b/www/configs/config.master.php index 551dbb75..51ed8267 100644 --- a/www/configs/config.master.php +++ b/www/configs/config.master.php @@ -185,7 +185,7 @@ if (file_exists(BASE . CONFIGS . 'config.path.php')) { // live frontend pages // ** missing live domains ** // get the name without the port -list($HOST_NAME) = array_pad(explode(':', $_SERVER['HTTP_HOST'], 2), 2, null); +[$HOST_NAME] = array_pad(explode(':', $_SERVER['HTTP_HOST'], 2), 2, null); // set HOST name define('HOST_NAME', $HOST_NAME); // BAIL ON MISSING MASTER SITE CONFIG diff --git a/www/lib/CoreLibs/ACL/Login.php b/www/lib/CoreLibs/ACL/Login.php index c4126be7..92a87028 100644 --- a/www/lib/CoreLibs/ACL/Login.php +++ b/www/lib/CoreLibs/ACL/Login.php @@ -1608,7 +1608,7 @@ class Login // TODO: submit or JS to set target page as ajax call // NOTE: for the HTML block I ignore line lengths // phpcs:disable - $this->login_template['password_change'] = <<login_template['password_change'] = << @@ -1626,7 +1626,7 @@ class Login

{TITLE_PASSWORD_CHANGE}

{PASSWORD_CHANGE_SHOW} -EOM; +HTML; // phpcs:enable } if ($this->password_forgot) { @@ -1650,7 +1650,7 @@ EOM; // now check templates // TODO: submit or JS to set target page as ajax call if (!$this->login_template['template']) { - $this->login_template['template'] = <<login_template['template'] = << @@ -1712,7 +1712,7 @@ h3 { font-size: 18px; } -EOM; +HTML; } } diff --git a/www/lib/CoreLibs/DB/IO.php b/www/lib/CoreLibs/DB/IO.php index f637aa70..6d0f1e5c 100644 --- a/www/lib/CoreLibs/DB/IO.php +++ b/www/lib/CoreLibs/DB/IO.php @@ -735,7 +735,10 @@ class IO */ private function __dbErrorPreprocessor(\PgSql\Result|false $cursor = false): array { - $pg_error_string = ''; + $db_prefix = ''; + $db_error_string = ''; + $db_prefix_last = ''; + $db_error_string_last = ''; // 1 = self/__dbErrorPreprocessor, 2 = __dbError, __dbWarning, // 3+ == actual source // loop until we get a null, build where called chain @@ -749,16 +752,31 @@ class IO if ($where_called === null) { $where_called = '[Unknown Method]'; } + [$db_prefix_last, $db_error_string_last] = $this->db_functions->__dbPrintLastError(); if ($cursor !== false) { - $pg_error_string = $this->db_functions->__dbPrintError($cursor); + [$db_prefix, $db_error_string] = $this->db_functions->__dbPrintError($cursor); } if ($cursor === false && method_exists($this->db_functions, '__dbPrintError')) { - $pg_error_string = $this->db_functions->__dbPrintError(); + [$db_prefix, $db_error_string] = $this->db_functions->__dbPrintError(); } - if ($pg_error_string) { - $this->__dbDebug('db', $pg_error_string, 'DB_ERROR', $where_called); + // prefix the master if not the same + if ( + !empty($db_error_string_last) && + trim($db_error_string) != trim($db_error_string_last) + ) { + $db_error_string = + $db_prefix_last . ' ' . $db_error_string_last . ';' + . $db_prefix . ' ' . $db_error_string; + } elseif (!empty($db_error_string)) { + $db_error_string = $db_prefix . ' ' . $db_error_string; } - return [$where_called, $pg_error_string]; + if ($db_error_string) { + $this->__dbDebug('db', $db_error_string, 'DB_ERROR', $where_called); + } + return [ + $where_called, + $db_error_string + ]; } /** @@ -902,11 +920,14 @@ class IO // because the placeholders start with $ and at 1, // we need to increase each key and prefix it with a $ char for ($i = 0, $iMax = count($keys); $i < $iMax; $i++) { - $keys[$i] = '$' . ($keys[$i] + 1); + // note: if I use $ here, the str_replace will + // replace it again. eg $11 '$1'1would be replaced with $1 again // prefix data set with parameter pos - $data[$i] = $keys[$i] . ':' . ($data[$i] === null ? + $data[$i] = '#' . ($keys[$i] + 1) . ':' . ($data[$i] === null ? '"NULL"' : (string)$data[$i] ); + // search part + $keys[$i] = '$' . ($keys[$i] + 1); } // simply replace the $1, $2, ... with the actual data and return it return str_replace( diff --git a/www/lib/CoreLibs/DB/SQL/Interface/SqlFunctions.php b/www/lib/CoreLibs/DB/SQL/Interface/SqlFunctions.php index 5aa7fdb5..7e81a164 100644 --- a/www/lib/CoreLibs/DB/SQL/Interface/SqlFunctions.php +++ b/www/lib/CoreLibs/DB/SQL/Interface/SqlFunctions.php @@ -209,10 +209,17 @@ interface SqlFunctions /** * Undocumented function * - * @param \PgSql\Result|false $cursor - * @return string + * @return array{0:string,1:string} */ - public function __dbPrintError(\PgSql\Result|false $cursor = false): string; + public function __dbPrintLastError(): array; + + /** + * Undocumented function + * + * @param \PgSql\Result|false $cursor + * @return array{0:string,1:string} + */ + public function __dbPrintError(\PgSql\Result|false $cursor = false): array; /** * Undocumented function diff --git a/www/lib/CoreLibs/DB/SQL/PgSQL.php b/www/lib/CoreLibs/DB/SQL/PgSQL.php index 9f42f76a..2d9b34eb 100644 --- a/www/lib/CoreLibs/DB/SQL/PgSQL.php +++ b/www/lib/CoreLibs/DB/SQL/PgSQL.php @@ -61,7 +61,7 @@ class PgSQL implements Interface\SqlFunctions /** @var string */ private $last_error_query; /** @var \PgSql\Connection|false */ - private $dbh; + private $dbh = false; /** * queries last error query and returns true or false if error was set @@ -532,18 +532,37 @@ class PgSQL implements Interface\SqlFunctions return $this->dbh; } + /** + * Returns last error for active cursor + * + * @return array{0:string,1:string} prefix, error string + */ + public function __dbPrintLastError(): array + { + if (is_bool($this->dbh)) { + return ['', '']; + } + if (!empty($error_message = pg_last_error($this->dbh))) { + return [ + '-PostgreSQL-Error-Last-', + $error_message + ]; + } + return ['', '']; + } + /** * reads the last error for this cursor and returns * html formatted string with error name * * @param \PgSql\Result|false $cursor cursor - * or null - * @return string error string + * or null + * @return array{0:string,1:string} prefix, error string */ - public function __dbPrintError(\PgSql\Result|false $cursor = false): string + public function __dbPrintError(\PgSql\Result|false $cursor = false): array { if (is_bool($this->dbh)) { - return ''; + return ['', '']; } // run the query again for the error result here if ((is_bool($cursor)) && $this->last_error_query) { @@ -552,10 +571,12 @@ class PgSQL implements Interface\SqlFunctions $cursor = pg_get_result($this->dbh); } if ($cursor && $error_str = pg_result_error($cursor)) { - return '-PostgreSQL-Error- ' - . $error_str; + return [ + '-PostgreSQL-Error-', + $error_str + ]; } else { - return ''; + return ['', '']; } }