Skip to content

Commit

Permalink
feature #24289 [FrameworkBundle][HttpKernel] Reset profiler (derrabus)
Browse files Browse the repository at this point in the history
This PR was merged into the 3.4 branch.

Discussion
----------

[FrameworkBundle][HttpKernel] Reset profiler

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | #18244
| License       | MIT
| Doc PR        | N/A

This PR adds the ability to reset the profiler between requests. Furthermore, the profiler service has been tagged with the new `kernel.reset` tag from #24155. For this, I had to readd the ability to define multiple reset methods for a service.

Note: This PR requires twigphp/Twig#2560.

Commits
-------

8c39bf7 Reset profiler.
  • Loading branch information
fabpot committed Oct 5, 2017
2 parents d6b68e1 + 8c39bf7 commit 86684f1
Show file tree
Hide file tree
Showing 41 changed files with 426 additions and 37 deletions.
14 changes: 12 additions & 2 deletions UPGRADE-3.4.md
Expand Up @@ -63,6 +63,12 @@ Debug

* Support for stacked errors in the `ErrorHandler` is deprecated and will be removed in Symfony 4.0.

EventDispatcher
---------------

* Implementing `TraceableEventDispatcherInterface` without the `reset()` method
is deprecated and will be unsupported in 4.0.

Filesystem
----------

Expand Down Expand Up @@ -270,6 +276,10 @@ HttpKernel

* The `Symfony\Component\HttpKernel\Config\EnvParametersResource` class has been deprecated and will be removed in 4.0.

* Implementing `DataCollectorInterface` without a `reset()` method has been deprecated and will be unsupported in 4.0.

* Implementing `DebugLoggerInterface` without a `clear()` method has been deprecated and will be unsupported in 4.0.

* The `ChainCacheClearer::add()` method has been deprecated and will be removed in 4.0,
inject the list of clearers as a constructor argument instead.

Expand Down Expand Up @@ -320,11 +330,11 @@ SecurityBundle

* Deprecated the HTTP digest authentication: `HttpDigestFactory` will be removed in 4.0.
Use another authentication system like `http_basic` instead.

* Deprecated setting the `switch_user.stateless` option to false when the firewall is `stateless`.
Setting it to false will have no effect in 4.0.

* Not configuring explicitly the provider on a firewall is ambiguous when there is more than one registered provider.
* Not configuring explicitly the provider on a firewall is ambiguous when there is more than one registered provider.
Using the first configured provider is deprecated since 3.4 and will throw an exception on 4.0.
Explicitly configure the provider to use on your firewalls.

Expand Down
10 changes: 8 additions & 2 deletions UPGRADE-4.0.md
Expand Up @@ -192,6 +192,8 @@ EventDispatcher
* The `ContainerAwareEventDispatcher` class has been removed.
Use `EventDispatcher` with closure factories instead.

* The `reset()` method has been added to `TraceableEventDispatcherInterface`.

ExpressionLanguage
------------------

Expand Down Expand Up @@ -611,6 +613,10 @@ HttpKernel

* The `Symfony\Component\HttpKernel\Config\EnvParametersResource` class has been removed.

* The `reset()` method has been added to `Symfony\Component\HttpKernel\DataCollector\DataCollectorInterface`.

* The `clear()` method has been added to `Symfony\Component\HttpKernel\Log\DebugLoggerInterface`.

* The `ChainCacheClearer::add()` method has been removed,
inject the list of clearers as a constructor argument instead.

Expand Down Expand Up @@ -693,10 +699,10 @@ SecurityBundle

* Removed the HTTP digest authentication system. The `HttpDigestFactory` class
has been removed. Use another authentication system like `http_basic` instead.

* The `switch_user.stateless` option is now always true if the firewall is stateless.

* Not configuring explicitly the provider on a firewall is ambiguous when there is more than one registered provider.
* Not configuring explicitly the provider on a firewall is ambiguous when there is more than one registered provider.
The first configured provider is not used anymore and an exception is thrown instead.
Explicitly configure the provider to use on your firewalls.

Expand Down
2 changes: 1 addition & 1 deletion composer.json
Expand Up @@ -20,7 +20,7 @@
"ext-xml": "*",
"doctrine/common": "~2.4",
"fig/link-util": "^1.0",
"twig/twig": "~1.34|~2.4",
"twig/twig": "^1.35|^2.4.4",
"psr/cache": "~1.0",
"psr/container": "^1.0",
"psr/link": "^1.0",
Expand Down
Expand Up @@ -28,6 +28,10 @@ class DoctrineDataCollector extends DataCollector
private $registry;
private $connections;
private $managers;

/**
* @var DebugStack[]
*/
private $loggers = array();

public function __construct(ManagerRegistry $registry)
Expand Down Expand Up @@ -65,6 +69,16 @@ public function collect(Request $request, Response $response, \Exception $except
);
}

public function reset()
{
$this->data = array();

foreach ($this->loggers as $logger) {
$logger->queries = array();
$logger->currentQuery = 0;
}
}

public function getManagers()
{
return $this->data['managers'];
Expand Down
Expand Up @@ -101,6 +101,20 @@ public function testCollectQueryWithNoParams()
$this->assertTrue($collectedQueries['default'][1]['explainable']);
}

public function testReset()
{
$queries = array(
array('sql' => 'SELECT * FROM table1', 'params' => array(), 'types' => array(), 'executionMS' => 1),
);
$c = $this->createCollector($queries);
$c->collect(new Request(), new Response());

$c->reset();
$c->collect(new Request(), new Response());

$this->assertEquals(array('default' => array()), $c->getQueries());
}

/**
* @dataProvider paramProvider
*/
Expand Down
10 changes: 10 additions & 0 deletions src/Symfony/Bridge/Monolog/Logger.php
Expand Up @@ -45,6 +45,16 @@ public function countErrors()
return 0;
}

/**
* {@inheritdoc}
*/
public function clear()
{
if (($logger = $this->getDebugLogger()) && method_exists($logger, 'clear')) {
$logger->clear();
}
}

/**
* Returns a DebugLoggerInterface instance if one is registered with this logger.
*
Expand Down
9 changes: 9 additions & 0 deletions src/Symfony/Bridge/Monolog/Processor/DebugProcessor.php
Expand Up @@ -55,4 +55,13 @@ public function countErrors()
{
return $this->errorCount;
}

/**
* {@inheritdoc}
*/
public function clear()
{
$this->records = array();
$this->errorCount = 0;
}
}
13 changes: 13 additions & 0 deletions src/Symfony/Bridge/Monolog/Tests/LoggerTest.php
Expand Up @@ -128,4 +128,17 @@ public function testGetLogsWithDebugProcessor2()
$this->assertEquals('test', $record['message']);
$this->assertEquals(Logger::INFO, $record['priority']);
}

public function testClear()
{
$handler = new TestHandler();
$logger = new Logger('test', array($handler));
$logger->pushProcessor(new DebugProcessor());

$logger->addInfo('test');
$logger->clear();

$this->assertEmpty($logger->getLogs());
$this->assertSame(0, $logger->countErrors());
}
}
10 changes: 10 additions & 0 deletions src/Symfony/Bridge/Twig/DataCollector/TwigDataCollector.php
Expand Up @@ -44,6 +44,16 @@ public function collect(Request $request, Response $response, \Exception $except
{
}

/**
* {@inheritdoc}
*/
public function reset()
{
$this->profile->reset();
$this->computed = null;
$this->data = array();
}

/**
* {@inheritdoc}
*/
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Bridge/Twig/composer.json
Expand Up @@ -17,7 +17,7 @@
],
"require": {
"php": "^5.5.9|>=7.0.8",
"twig/twig": "~1.34|~2.4"
"twig/twig": "^1.35|^2.4.4"
},
"require-dev": {
"fig/link-util": "^1.0",
Expand Down
Expand Up @@ -609,9 +609,9 @@ private function registerProfilerConfiguration(array $config, ContainerBuilder $
}
}

if (!$config['collect']) {
$container->getDefinition('profiler')->addMethodCall('disable', array());
}
$container->getDefinition('profiler')
->addArgument($config['collect'])
->addTag('kernel.reset', array('method' => 'reset'));
}

/**
Expand Down
Expand Up @@ -203,6 +203,14 @@ public function collect(Request $request, Response $response, \Exception $except
}
}

/**
* {@inheritdoc}
*/
public function reset()
{
$this->data = array();
}

public function lateCollect()
{
$this->data = $this->cloneVar($this->data);
Expand Down
Expand Up @@ -53,6 +53,15 @@ public function collect(Request $request, Response $response, \Exception $except
$this->data['total']['statistics'] = $this->calculateTotalStatistics();
}

public function reset()
{
$this->data = array();
foreach ($this->instances as $instance) {
// Calling getCalls() will clear the calls.
$instance->getCalls();
}
}

public function lateCollect()
{
$this->data = $this->cloneVar($this->data);
Expand Down
5 changes: 5 additions & 0 deletions src/Symfony/Component/EventDispatcher/CHANGELOG.md
@@ -1,6 +1,11 @@
CHANGELOG
=========

3.4.0
-----

* Implementing `TraceableEventDispatcherInterface` without the `reset()` method has been deprecated.

3.3.0
-----

Expand Down
Expand Up @@ -212,6 +212,11 @@ public function getNotCalledListeners()
return $notCalled;
}

public function reset()
{
$this->called = array();
}

/**
* Proxies all method calls to the original event dispatcher.
*
Expand Down
Expand Up @@ -15,6 +15,8 @@

/**
* @author Fabien Potencier <fabien@symfony.com>
*
* @method reset() Resets the trace.
*/
interface TraceableEventDispatcherInterface extends EventDispatcherInterface
{
Expand Down
Expand Up @@ -124,6 +124,21 @@ public function testGetCalledListeners()
$this->assertEquals(array(), $tdispatcher->getNotCalledListeners());
}

public function testClearCalledListeners()
{
$tdispatcher = new TraceableEventDispatcher(new EventDispatcher(), new Stopwatch());
$tdispatcher->addListener('foo', function () {}, 5);

$tdispatcher->dispatch('foo');
$tdispatcher->reset();

$listeners = $tdispatcher->getNotCalledListeners();
$this->assertArrayHasKey('stub', $listeners['foo.closure']);
unset($listeners['foo.closure']['stub']);
$this->assertEquals(array(), $tdispatcher->getCalledListeners());
$this->assertEquals(array('foo.closure' => array('event' => 'foo', 'pretty' => 'closure', 'priority' => 5)), $listeners);
}

public function testGetCalledListenersNested()
{
$tdispatcher = null;
Expand Down
Expand Up @@ -72,12 +72,9 @@ class FormDataCollector extends DataCollector implements FormDataCollectorInterf
public function __construct(FormDataExtractorInterface $dataExtractor)
{
$this->dataExtractor = $dataExtractor;
$this->data = array(
'forms' => array(),
'forms_by_hash' => array(),
'nb_errors' => 0,
);
$this->hasVarDumper = class_exists(ClassStub::class);

$this->reset();
}

/**
Expand All @@ -87,6 +84,15 @@ public function collect(Request $request, Response $response, \Exception $except
{
}

public function reset()
{
$this->data = array(
'forms' => array(),
'forms_by_hash' => array(),
'nb_errors' => 0,
);
}

/**
* {@inheritdoc}
*/
Expand Down
Expand Up @@ -695,6 +695,36 @@ public function testCollectSubmittedDataExpandedFormsErrors()
$this->assertFalse(isset($child21Data['has_children_error']), 'The leaf data does not contains "has_children_error" property.');
}

public function testReset()
{
$form = $this->createForm('my_form');

$this->dataExtractor->expects($this->any())
->method('extractConfiguration')
->will($this->returnValue(array()));
$this->dataExtractor->expects($this->any())
->method('extractDefaultData')
->will($this->returnValue(array()));
$this->dataExtractor->expects($this->any())
->method('extractSubmittedData')
->with($form)
->will($this->returnValue(array('errors' => array('baz'))));

$this->dataCollector->buildPreliminaryFormTree($form);
$this->dataCollector->collectSubmittedData($form);

$this->dataCollector->reset();

$this->assertSame(
array(
'forms' => array(),
'forms_by_hash' => array(),
'nb_errors' => 0,
),
$this->dataCollector->getData()
);
}

private function createForm($name)
{
$builder = new FormBuilder($name, null, $this->dispatcher, $this->factory);
Expand Down
4 changes: 3 additions & 1 deletion src/Symfony/Component/HttpKernel/CHANGELOG.md
Expand Up @@ -14,7 +14,9 @@ CHANGELOG
* deprecated the `ChainCacheClearer::add()` method
* deprecated the `CacheaWarmerAggregate::add()` and `setWarmers()` methods
* made `CacheWarmerAggregate` and `ChainCacheClearer` classes final

* added the possibility to reset the profiler to its initial state
* deprecated data collectors without a `reset()` method
* deprecated implementing `DebugLoggerInterface` without a `clear()` method

3.3.0
-----
Expand Down
Expand Up @@ -26,6 +26,11 @@ public function collect(Request $request, Response $response, \Exception $except
// all collecting is done client side
}

public function reset()
{
// all collecting is done client side
}

public function getName()
{
return 'ajax';
Expand Down

0 comments on commit 86684f1

Please sign in to comment.