Refactor Database Collector display

codeigniter4 · Jan 7, 2022 · 1fa5c86 · 1fa5c86
1 parent cb0b371
commit 1fa5c86
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 19 deletions.
diff --git 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,24 @@ 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 &$line) {
+                if (isset($line['file'])) {
+                    $line['file'] = clean_path($line['file']) . ':' . $line['line'];
+                    unset($line['line']);
+                } else {
+                    $line['file'] = '[internal function]';
                 }
-                $traceLine['file'] = str_ireplace(ROOTPATH, 'ROOTPATH/', $traceLine['file']);
+            }
+
+            // 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'];
 
-                if (strpos($traceLine['file'], 'SYSTEMPATH') !== false) {
-                    continue;
+                    break;
                 }
-                $line = empty($line) ? $traceLine : $line;
             }
 
             return [
@@ -159,8 +160,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
@@ -10,13 +10,13 @@
         <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">{trace-file}</td>
         </tr>
         <tr class="muted" id="{qid}-trace" style="display:none">
             <td></td>
             <td colspan="2">
             {trace}
-                {file}:<strong>{line}</strong><br/>
+                {file}<br/>
             {/trace}
             </td>
         </tr>

diff --git a/tests/system/Debug/Toolbar/Collectors/DatabaseTest.php b/tests/system/Debug/Toolbar/Collectors/DatabaseTest.php
@@ -0,0 +1,54 @@
+<?php
+
+/**
+ * This file is part of CodeIgniter 4 framework.
+ *
+ * (c) CodeIgniter Foundation <admin@codeigniter.com>
+ *
+ * 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 $trace) {
+            // 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);
+        }
+    }
+}