Merge pull request #5553 from paulbalandan/backtrace-collector

Refactor Database Collector display

Author: lonnieezell

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(),
+                'trace'     => $backtrace,
             ];
         }
     }
@@ -134,23 +142,39 @@ 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 = [];
+            $firstNonSystemLine = '';
+
+            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]';
+                }
+
+                // find the first trace line that does not originate from `system/`
+                if ($firstNonSystemLine === '' && strpos($line['file'], 'SYSTEMPATH') === false) {
+                    $firstNonSystemLine = $line['file'];
+                }
 
-            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']);
+                // 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 = empty($line) ? $traceLine : $line;
+
+                $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);
             }
 
             return [
@@ -159,8 +183,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);

system/Debug/Toolbar/Views/_database.tpl

@@ -10,13 +10,14 @@
         <tr class="{class}" title="{hover}" data-toggle="{qid}-trace">
             <td class="narrow">{duration}</td>
             <td>{! sql !}</td>
-            <td style="text-align: right">{trace-file}:<strong>{trace-line}</strong></td>
+            <td style="text-align: right"><strong>{trace-file}</strong></td>
         </tr>
         <tr class="muted" id="{qid}-trace" style="display:none">
             <td></td>
             <td colspan="2">
             {trace}
-                {file}:<strong>{line}</strong><br/>
+                {index}<strong>{file}</strong><br/>
+                {function}<br/><br/>
             {/trace}
             </td>
         </tr>

tests/system/Debug/Toolbar/Collectors/DatabaseTest.php

@@ -0,0 +1,61 @@
+<?php
+
+/**
+ * This file is part of CodeIgniter 4 framework.
+ *
+ * (c) CodeIgniter Foundation <[email protected]>
+ *
+ * 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('<strong>SHOW</strong> 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('<strong>SHOW</strong> 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
+            // but args MAY STILL BE present in internal calls
+            $this->assertArrayNotHasKey('object', $trace);
+        }
+    }
+}