From 973ab182ad5f35c191f6d418a041aa08a56a259a Mon Sep 17 00:00:00 2001 From: "John Paul E. Balandan, CPA" Date: Fri, 7 Jan 2022 10:48:08 +0800 Subject: [PATCH 1/3] Refactor Database Collector display --- system/Debug/Toolbar/Collectors/Database.php | 56 +++++++++++------ system/Debug/Toolbar/Views/_database.tpl | 5 +- .../Debug/Toolbar/Collectors/DatabaseTest.php | 61 +++++++++++++++++++ 3 files changed, 103 insertions(+), 19 deletions(-) create mode 100644 tests/system/Debug/Toolbar/Collectors/DatabaseTest.php diff --git a/system/Debug/Toolbar/Collectors/Database.php b/system/Debug/Toolbar/Collectors/Database.php index 520ddc7c5dc6..d7e14d291b68 100644 --- a/system/Debug/Toolbar/Collectors/Database.php +++ b/system/Debug/Toolbar/Collectors/Database.php @@ -89,7 +89,7 @@ public static function collect(Query $query) 'query' => $query, 'string' => $queryString, 'duplicate' => in_array($queryString, array_column(static::$queries, 'string', null), true), - 'trace' => debug_backtrace(), + 'trace' => debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS), ]; } } @@ -134,23 +134,46 @@ public function display(): array $data['queries'] = array_map(static function (array $query) { $isDuplicate = $query['duplicate'] === true; - // Find the first line that doesn't include `system` in the backtrace - $line = []; - - foreach ($query['trace'] as &$traceLine) { - // Clean up the file paths - $traceLine['file'] = str_ireplace(APPPATH, 'APPPATH/', $traceLine['file']); - $traceLine['file'] = str_ireplace(SYSTEMPATH, 'SYSTEMPATH/', $traceLine['file']); - if (defined('VENDORPATH')) { - // VENDORPATH is not defined unless `vendor/autoload.php` exists - $traceLine['file'] = str_ireplace(VENDORPATH, 'VENDORPATH/', $traceLine['file']); + foreach ($query['trace'] as $index => &$line) { + // simplify file and line + if (isset($line['file'])) { + $line['file'] = clean_path($line['file']) . ':' . $line['line']; + unset($line['line']); + } else { + $line['file'] = '[internal function]'; + } + + // simplify function call + if (isset($line['class'])) { + $line['function'] = $line['class'] . $line['type'] . $line['function']; + unset($line['class'], $line['type']); } - $traceLine['file'] = str_ireplace(ROOTPATH, 'ROOTPATH/', $traceLine['file']); - if (strpos($traceLine['file'], 'SYSTEMPATH') !== false) { - continue; + if (strrpos($line['function'], '{closure}') === false) { + $line['function'] .= '()'; + } + + $line['function'] = str_repeat(chr(0xC2) . chr(0xA0), 8) . $line['function']; + + // add index numbering padded with nonbreaking space + $indexPadded = str_pad(sprintf('%d', $index + 1), 3, ' ', STR_PAD_LEFT); + $indexPadded = preg_replace('/\s/', chr(0xC2) . chr(0xA0), $indexPadded); + + $line['index'] = $indexPadded . str_repeat(chr(0xC2) . chr(0xA0), 4); + } + + // remove the caller trace which is duplicated as the last item + array_pop($query['trace']); + + // Find the first line that doesn't include `system` in the backtrace + $firstNonSystemLine = ''; + + foreach ($query['trace'] as $line) { + if (strpos($line['file'], 'SYSTEMPATH') === false) { + $firstNonSystemLine = $line['file']; + + break; } - $line = empty($line) ? $traceLine : $line; } return [ @@ -159,8 +182,7 @@ public function display(): array 'duration' => ((float) $query['query']->getDuration(5) * 1000) . ' ms', 'sql' => $query['query']->debugToolbarDisplay(), 'trace' => $query['trace'], - 'trace-file' => str_replace(ROOTPATH, '/', $line['file'] ?? ''), - 'trace-line' => $line['line'] ?? '', + 'trace-file' => $firstNonSystemLine, 'qid' => md5($query['query'] . microtime()), ]; }, static::$queries); diff --git a/system/Debug/Toolbar/Views/_database.tpl b/system/Debug/Toolbar/Views/_database.tpl index a2f5bd9808f1..1bd9b8a88405 100644 --- a/system/Debug/Toolbar/Views/_database.tpl +++ b/system/Debug/Toolbar/Views/_database.tpl @@ -10,13 +10,14 @@ {duration} {! sql !} - {trace-file}:{trace-line} + {trace-file} {trace} - {file}:{line}
+ {index}{file}
+ {function}

{/trace} diff --git a/tests/system/Debug/Toolbar/Collectors/DatabaseTest.php b/tests/system/Debug/Toolbar/Collectors/DatabaseTest.php new file mode 100644 index 000000000000..17ae1cb3d143 --- /dev/null +++ b/tests/system/Debug/Toolbar/Collectors/DatabaseTest.php @@ -0,0 +1,61 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace CodeIgniter\Debug\Toolbar\Collectors; + +use CodeIgniter\Database\Query; +use CodeIgniter\Test\CIUnitTestCase; +use PHPUnit\Framework\MockObject\MockObject; + +/** + * @internal + */ +final class DatabaseTest extends CIUnitTestCase +{ + public function testDisplay(): void + { + /** @var MockObject&Query $query */ + $query = $this->getMockBuilder(Query::class) + ->disableOriginalConstructor() + ->getMock(); + + // set mock returns + $query->method('getQuery')->willReturn('SHOW TABLES;'); + $query->method('debugToolbarDisplay')->willReturn('SHOW TABLES;'); + $query->method('getDuration')->with(5)->willReturn('1.23456'); + + Database::collect($query); // <== $query will be called here + $collector = new Database(); + + $queries = $collector->display()['queries']; + + $this->assertSame('1234.56 ms', $queries[0]['duration']); + $this->assertSame('SHOW TABLES;', $queries[0]['sql']); + $this->assertSame(clean_path(__FILE__) . ':35', $queries[0]['trace-file']); + + foreach ($queries[0]['trace'] as $i => $trace) { + // since we added the index numbering + $this->assertArrayHasKey('index', $trace); + $this->assertSame( + sprintf('%s', $i + 1), + preg_replace(sprintf('/%s/', chr(0xC2) . chr(0xA0)), '', $trace['index']) + ); + + // since we merged file & line together + $this->assertArrayNotHasKey('line', $trace); + $this->assertArrayHasKey('file', $trace); + + // since we dropped object & args in the backtrace for performance + $this->assertArrayNotHasKey('object', $trace); + $this->assertArrayNotHasKey('args', $trace); + } + } +} From 1a52fbf7cb113358820de8cd86f85d263c42a357 Mon Sep 17 00:00:00 2001 From: "John Paul E. Balandan, CPA" Date: Fri, 7 Jan 2022 16:56:59 +0800 Subject: [PATCH 2/3] Do not reuse `$line` outside of foreach --- system/Debug/Toolbar/Collectors/Database.php | 21 +++++++------------ .../Debug/Toolbar/Collectors/DatabaseTest.php | 2 +- 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/system/Debug/Toolbar/Collectors/Database.php b/system/Debug/Toolbar/Collectors/Database.php index d7e14d291b68..a589c8fdabb9 100644 --- a/system/Debug/Toolbar/Collectors/Database.php +++ b/system/Debug/Toolbar/Collectors/Database.php @@ -134,6 +134,8 @@ public function display(): array $data['queries'] = array_map(static function (array $query) { $isDuplicate = $query['duplicate'] === true; + $firstNonSystemLine = ''; + foreach ($query['trace'] as $index => &$line) { // simplify file and line if (isset($line['file'])) { @@ -143,6 +145,11 @@ public function display(): array $line['file'] = '[internal function]'; } + // find the first trace line that does not originate from `system/` + if ($firstNonSystemLine === '' && strpos($line['file'], 'SYSTEMPATH') === false) { + $firstNonSystemLine = $line['file']; + } + // simplify function call if (isset($line['class'])) { $line['function'] = $line['class'] . $line['type'] . $line['function']; @@ -162,20 +169,6 @@ public function display(): array $line['index'] = $indexPadded . str_repeat(chr(0xC2) . chr(0xA0), 4); } - // remove the caller trace which is duplicated as the last item - array_pop($query['trace']); - - // Find the first line that doesn't include `system` in the backtrace - $firstNonSystemLine = ''; - - foreach ($query['trace'] as $line) { - if (strpos($line['file'], 'SYSTEMPATH') === false) { - $firstNonSystemLine = $line['file']; - - break; - } - } - return [ 'hover' => $isDuplicate ? 'This query was called more than once.' : '', 'class' => $isDuplicate ? 'duplicate' : '', diff --git a/tests/system/Debug/Toolbar/Collectors/DatabaseTest.php b/tests/system/Debug/Toolbar/Collectors/DatabaseTest.php index 17ae1cb3d143..3b139d9f2d7e 100644 --- a/tests/system/Debug/Toolbar/Collectors/DatabaseTest.php +++ b/tests/system/Debug/Toolbar/Collectors/DatabaseTest.php @@ -54,8 +54,8 @@ public function testDisplay(): void $this->assertArrayHasKey('file', $trace); // since we dropped object & args in the backtrace for performance + // but args MAY STILL BE present in internal calls $this->assertArrayNotHasKey('object', $trace); - $this->assertArrayNotHasKey('args', $trace); } } } From 1f3d5255bbeb08eef454fb61c1ac11f69ef1b59a Mon Sep 17 00:00:00 2001 From: "John Paul E. Balandan, CPA" Date: Fri, 7 Jan 2022 20:36:14 +0800 Subject: [PATCH 3/3] Remove first two arrays in trace when called in browser --- system/Debug/Toolbar/Collectors/Database.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/system/Debug/Toolbar/Collectors/Database.php b/system/Debug/Toolbar/Collectors/Database.php index a589c8fdabb9..26cb02ddfeb0 100644 --- a/system/Debug/Toolbar/Collectors/Database.php +++ b/system/Debug/Toolbar/Collectors/Database.php @@ -85,11 +85,19 @@ public static function collect(Query $query) if (count(static::$queries) < $max) { $queryString = $query->getQuery(); + $backtrace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS); + + if (! is_cli()) { + // when called in the browser, the first two trace arrays + // are from the DB event trigger, which are unneeded + $backtrace = array_slice($backtrace, 2); + } + static::$queries[] = [ 'query' => $query, 'string' => $queryString, 'duplicate' => in_array($queryString, array_column(static::$queries, 'string', null), true), - 'trace' => debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS), + 'trace' => $backtrace, ]; } }