Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

debugbar logging requests using microtime #5643

Closed
wants to merge 28 commits into from
Closed
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
69899fb
debugbar logging requests using microtime
maniaba Feb 2, 2022
6d4f8e5
debugbar logging requests using microtime
maniaba Feb 2, 2022
0e51d78
debugbar logging requests using microtime
maniaba Feb 2, 2022
c10bd47
debugbar logging requests using microtime
maniaba Feb 2, 2022
317d697
debugbar logging requests using microtime
maniaba Feb 2, 2022
58f6184
debugbar logging requests using microtime
maniaba Feb 2, 2022
9abed8f
fix datetime expects string, float given.
maniaba Feb 2, 2022
4bc62cb
fix datetime expects string, float given.
maniaba Feb 2, 2022
060c576
fix Strict comparison using
maniaba Feb 2, 2022
b6b86ff
update comments and add assertSame for active row
maniaba Feb 3, 2022
f2d55e1
test corrections
maniaba Feb 3, 2022
4ca2159
user guide changelog
maniaba Feb 4, 2022
4e13df7
Change parameter method History::setFiles() type from float to string
maniaba Feb 6, 2022
329daac
Merge branch 'develop' into develop
maniaba Feb 7, 2022
bf3263f
Fix regex pattern
maniaba Feb 15, 2022
8088318
Sync
maniaba Feb 23, 2022
acdaec1
Sync
maniaba Feb 23, 2022
4b97c22
Merge remote-tracking branch 'upstream/develop' into develop
maniaba Feb 23, 2022
10074e5
update changelogs
maniaba Feb 23, 2022
4c2ca8d
Merge branch 'develop' of https://github.com/maniaba/CodeIgniter4 int…
maniaba Feb 23, 2022
fb8fa1a
Update v4.2.0.rst
maniaba Feb 23, 2022
3e47505
Update v4.2.0.rst
maniaba Feb 23, 2022
bff4c56
Merge branch 'develop' of https://github.com/maniaba/CodeIgniter4 int…
maniaba Feb 23, 2022
070265c
Merge branch 'develop' into develop
maniaba Feb 24, 2022
cbfe5b4
Update system/Debug/Toolbar.php
maniaba Feb 24, 2022
4809c60
Update system/Debug/Toolbar/Collectors/History.php
maniaba Feb 24, 2022
282297c
Update system/Debug/Toolbar.php
maniaba Feb 24, 2022
c2b511a
Update toolbar.css
maniaba Feb 24, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions admin/css/debug-toolbar/toolbar.scss
Original file line number Diff line number Diff line change
Expand Up @@ -488,8 +488,8 @@
width: 70px;
}

.debug-bar-width140p {
width: 140px;
.debug-bar-width190p {
width: 190px;
}

.debug-bar-width20e {
Expand Down
2 changes: 1 addition & 1 deletion system/Debug/Toolbar.php
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ public function prepare(?RequestInterface $request = null, ?ResponseInterface $r
helper('filesystem');

// Updated to time() so we can get history
maniaba marked this conversation as resolved.
Show resolved Hide resolved
$time = time();
$time = number_format(microtime(true), 4, '.', '');
paulbalandan marked this conversation as resolved.
Show resolved Hide resolved

if (! is_dir(WRITEPATH . 'debugbar')) {
mkdir(WRITEPATH . 'debugbar', 0777);
Expand Down
23 changes: 16 additions & 7 deletions system/Debug/Toolbar/Collectors/History.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

namespace CodeIgniter\Debug\Toolbar\Collectors;

use DateTime;

/**
* History collector
*/
Expand Down Expand Up @@ -56,10 +58,10 @@ class History extends BaseCollector
/**
* Specify time limit & file count for debug history.
*
* @param int $current Current history time
* @param int $limit Max history files
* @param float $current Current history time
* @param int $limit Max history files
*/
public function setFiles(int $current, int $limit = 20)
public function setFiles(float $current, int $limit = 20)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to break the signature here? I think we could just pass around the microtime as an int and then do number_format right before outputting.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is BC break.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the limitations, I think it is acceptable to change the signature albeit a breaking change. Let's wait for other's opinions. @kenjis @MGatner ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mind (necessary) BC in minor version up if it is documented.
See https://forum.codeigniter.com/thread-80892-post-392846.html#pid392846

By the way, is float enough?
It seems only 4 digits like 1643882644.5667 in my PHP.

Copy link
Contributor Author

@maniaba maniaba Feb 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe change this to a string, and make the file name look like "debugbar_{microtime} _ {session_value} .json" or add session_value to the contents of the file?
Then we will solve the request "when multiple developers work on different modules I suggest the option that developers can filter requests by session so that everyone can see it's own requests made by their browser."
I suggest adding a checkbox to the history tab, which would filter sessions by the browser.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a good refinement to the Debug Toolbar, but let us limit this PR to adding the microtime only. We can add the session and filtering in another PR.

Copy link
Member

@kenjis kenjis Feb 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think type string is better. It is actually (a part of) filename, not time.

{
$filenames = glob(WRITEPATH . 'debugbar/debugbar_*.json');

Expand All @@ -81,14 +83,21 @@ public function setFiles(int $current, int $limit = 20)

$contents = @json_decode($contents);
if (json_last_error() === JSON_ERROR_NONE) {
preg_match_all('/\d+/', $filename, $time);
$time = (int) end($time[0]);
preg_match_all('/debugbar_(.*)\.json/s', $filename, $time, PREG_SET_ORDER);
$time = number_format((float) end($time[0]), 4, '.', '');
maniaba marked this conversation as resolved.
Show resolved Hide resolved

// Prevent throw exception is old debugbar time format
if ($dateTime = DateTime::createFromFormat('U.u', $time)) {
maniaba marked this conversation as resolved.
Show resolved Hide resolved
$dateTime = $dateTime->format('Y-m-d H:i:s.u');
} else {
$dateTime = date('Y-m-d H:i:s.u', (int) $time);
}

// Debugbar files shown in History Collector
$files[] = [
'time' => $time,
'datetime' => date('Y-m-d H:i:s', $time),
'active' => $time === $current,
'datetime' => $dateTime,
'active' => (float) $time === $current,
'status' => $contents->vars->response->statusCode,
'method' => $contents->method,
'url' => $contents->url,
Expand Down
2 changes: 1 addition & 1 deletion system/Debug/Toolbar/Views/_history.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
<td class="debug-bar-width70p">
<button class="ci-history-load" data-time="{time}">Load</button>
</td>
<td class="debug-bar-width140p">{datetime}</td>
<td class="debug-bar-width190p">{datetime}</td>
<td>{status}</td>
<td>{method}</td>
<td>{url}</td>
Expand Down
4 changes: 2 additions & 2 deletions system/Debug/Toolbar/Views/toolbar.css
Original file line number Diff line number Diff line change
Expand Up @@ -631,8 +631,8 @@
.debug-bar-width70p {
width: 70px; }

.debug-bar-width140p {
width: 140px; }
.debug-bar-width190p {
width: 190px;}

.debug-bar-width20e {
width: 20em; }
Expand Down
90 changes: 90 additions & 0 deletions tests/system/Debug/Toolbar/Collectors/HistoryTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
<?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\Test\CIUnitTestCase;
use DateTime;

/**
* @internal
*/
final class HistoryTest extends CIUnitTestCase
{
private const STEP = 0.0001;

protected float $time;
maniaba marked this conversation as resolved.
Show resolved Hide resolved

protected function setUp(): void
{
parent::setUp();
$this->time = number_format(microtime(true), 4, '.', '');
}

protected function createDummyDebugbarJson()
maniaba marked this conversation as resolved.
Show resolved Hide resolved
{
$time = $this->time;
$path = WRITEPATH . 'debugbar' . DIRECTORY_SEPARATOR . "debugbar_{$time}.json";

$dummyData = [
'vars' => [
'response' => [
'statusCode' => 200,
'contentType' => 'text/html; charset=UTF-8',
],
],
'method' => 'get',
'url' => 'localhost',
'isAJAX' => false,
];
// create 10 dummy debugbar json files
for ($i = 0; $i < 20; $i++) {
maniaba marked this conversation as resolved.
Show resolved Hide resolved
$path = str_replace($time, number_format($time - self::STEP, 4, '.', ''), $path);
file_put_contents($path, json_encode($dummyData));
$time = number_format($time - self::STEP, 4, '.', '');
maniaba marked this conversation as resolved.
Show resolved Hide resolved
}
}

protected function tearDown(): void
{
command('debugbar:clear');
}

public function testSetFiles()
{
$time = $this->time;
// test dir is now populated with json
$this->createDummyDebugbarJson();

$history = new History();
$history->setFiles($this->time, 20);
$this->assertIsArray($history->display());
$this->assertArrayHasKey('files', $history->display());
$this->assertNotEmpty($history->display(), 'Dummy Debugbar data is empty');
maniaba marked this conversation as resolved.
Show resolved Hide resolved

$time = number_format($time - self::STEP, 4, '.', '');

foreach ($history->display()['files'] as $request) {
$this->assertSame($request, [
'time' => $time,
'datetime' => DateTime::createFromFormat('U.u', $time)->format('Y-m-d H:i:s.u'),
'active' => false,
'status' => 200,
'method' => 'get',
'url' => 'localhost',
'isAJAX' => 'No',
'contentType' => 'text/html; charset=UTF-8',

], 'Date format fail');
$time = number_format($time - self::STEP, 4, '.', '');
maniaba marked this conversation as resolved.
Show resolved Hide resolved
}
}
}