[FrameworkBundle][WebProfilerBundle] Move ProfilerPass to the WebProfiler bundle #21368
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,14 +50,16 @@ | |
| "symfony/validator": "~3.2", | ||
| "symfony/yaml": "~3.2", | ||
| "symfony/property-info": "~2.8|~3.0", | ||
| "symfony/web-profiler-bundle": "~3.3", | ||
| "doctrine/annotations": "~1.0", | ||
| "phpdocumentor/reflection-docblock": "^3.0", | ||
| "twig/twig": "~1.26|~2.0" | ||
| }, | ||
| "conflict": { | ||
| "phpdocumentor/reflection-docblock": "<3.0", | ||
| "phpdocumentor/type-resolver": "<0.2.0", | ||
| "symfony/console": "<3.3", | ||
| "symfony/web-profiler-bundle": "<3.3" | ||
|
There was a problem hiding this comment.
There was a problem hiding this comment. That's what I figured in your comment . |
||
| }, | ||
| "suggest": { | ||
| "ext-apcu": "For best performance of the system caches", | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,11 @@ | ||
| CHANGELOG | ||
| ========= | ||
|
|
||
| 3.3.0 | ||
| ----- | ||
|
|
||
| * added `ProfilerPass` | ||
|
|
||
| 3.1.0 | ||
| ----- | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| <?php | ||
|
|
||
| /* | ||
| * This file is part of the Symfony package. | ||
| * | ||
| * (c) Fabien Potencier <fabien@symfony.com> | ||
| * | ||
| * For the full copyright and license information, please view the LICENSE | ||
| * file that was distributed with this source code. | ||
| */ | ||
|
|
||
| namespace Symfony\Bundle\WebProfilerBundle\DependencyInjection\Compiler; | ||
|
|
||
| use Symfony\Component\DependencyInjection\Reference; | ||
| use Symfony\Component\DependencyInjection\ContainerBuilder; | ||
| use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; | ||
| use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; | ||
|
|
||
| /** | ||
| * Adds tagged data_collector services to profiler service. | ||
| * | ||
| * @author Fabien Potencier <fabien@symfony.com> | ||
| */ | ||
| class ProfilerPass implements CompilerPassInterface | ||
| { | ||
| public function process(ContainerBuilder $container) | ||
| { | ||
| if (false === $container->hasDefinition('profiler')) { | ||
| return; | ||
| } | ||
|
|
||
| $definition = $container->getDefinition('profiler'); | ||
|
|
||
| $collectors = new \SplPriorityQueue(); | ||
| $order = PHP_INT_MAX; | ||
| foreach ($container->findTaggedServiceIds('data_collector') as $id => $attributes) { | ||
| $priority = isset($attributes[0]['priority']) ? $attributes[0]['priority'] : 0; | ||
| $template = null; | ||
|
|
||
| if (isset($attributes[0]['template'])) { | ||
| if (!isset($attributes[0]['id'])) { | ||
| throw new InvalidArgumentException(sprintf('Data collector service "%s" must have an id attribute in order to specify a template', $id)); | ||
| } | ||
| $template = array($attributes[0]['id'], $attributes[0]['template']); | ||
| } | ||
|
|
||
| $collectors->insert(array($id, $template), array($priority, --$order)); | ||
| } | ||
|
|
||
| $templates = array(); | ||
| foreach ($collectors as $collector) { | ||
| $definition->addMethodCall('add', array(new Reference($collector[0]))); | ||
| $templates[$collector[0]] = $collector[1]; | ||
| } | ||
|
|
||
| $container->setParameter('data_collector.templates', $templates); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| <?php | ||
|
|
||
| /* | ||
| * This file is part of the Symfony package. | ||
| * | ||
| * (c) Fabien Potencier <fabien@symfony.com> | ||
| * | ||
| * For the full copyright and license information, please view the LICENSE | ||
| * file that was distributed with this source code. | ||
| */ | ||
|
|
||
| namespace Symfony\Bundle\WebProfilerBundle\Tests\DependencyInjection\Compiler; | ||
|
|
||
| use Symfony\Bundle\WebProfilerBundle\DependencyInjection\Compiler\ProfilerPass; | ||
| use Symfony\Component\DependencyInjection\Definition; | ||
|
|
||
| class ProfilerPassTest extends \PHPUnit_Framework_TestCase | ||
| { | ||
| private $profilerDefinition; | ||
|
|
||
| protected function setUp() | ||
| { | ||
| $this->profilerDefinition = new Definition('ProfilerClass'); | ||
| } | ||
|
|
||
| /** | ||
| * Tests that collectors that specify a template but no "id" will throw | ||
| * an exception (both are needed if the template is specified). | ||
| * | ||
| * Thus, a fully-valid tag looks something like this: | ||
| * | ||
| * <tag name="data_collector" template="YourBundle:Collector:templatename" id="your_collector_name" /> | ||
| */ | ||
| public function testTemplateNoIdThrowsException() | ||
| { | ||
| // one service, with a template key, but no id | ||
| $services = array( | ||
| 'my_collector_service' => array(0 => array('template' => 'foo')), | ||
| ); | ||
|
|
||
| $builder = $this->createContainerMock($services); | ||
|
|
||
| $this->setExpectedException('InvalidArgumentException'); | ||
|
|
||
| $profilerPass = new ProfilerPass(); | ||
| $profilerPass->process($builder); | ||
| } | ||
|
|
||
| public function testValidCollector() | ||
| { | ||
| // one service, with a template key, but no id | ||
| $services = array( | ||
| 'my_collector_service' => array(0 => array('template' => 'foo', 'id' => 'my_collector')), | ||
| ); | ||
|
|
||
| $container = $this->createContainerMock($services); | ||
|
|
||
| // fake the getDefinition() to return a Profiler definition | ||
| $container->expects($this->atLeastOnce()) | ||
| ->method('getDefinition'); | ||
|
|
||
| // assert that the data_collector.templates parameter should be set | ||
| $container->expects($this->once()) | ||
| ->method('setParameter') | ||
| ->with('data_collector.templates', array('my_collector_service' => array('my_collector', 'foo'))); | ||
|
|
||
| $profilerPass = new ProfilerPass(); | ||
| $profilerPass->process($container); | ||
|
|
||
| // grab the method calls off of the "profiler" definition | ||
| $methodCalls = $this->profilerDefinition->getMethodCalls(); | ||
| $this->assertCount(1, $methodCalls); | ||
| $this->assertEquals('add', $methodCalls[0][0]); // grab the method part of the first call | ||
| } | ||
|
|
||
| private function createContainerMock($services) | ||
| { | ||
| $container = $this->getMockBuilder('Symfony\Component\DependencyInjection\ContainerBuilder')->setMethods(array('hasDefinition', 'getDefinition', 'findTaggedServiceIds', 'setParameter'))->getMock(); | ||
| $container->expects($this->any()) | ||
| ->method('hasDefinition') | ||
| ->with($this->equalTo('profiler')) | ||
| ->will($this->returnValue(true)); | ||
| $container->expects($this->any()) | ||
| ->method('getDefinition') | ||
| ->will($this->returnValue($this->profilerDefinition)); | ||
| $container->expects($this->atLeastOnce()) | ||
| ->method('findTaggedServiceIds') | ||
| ->will($this->returnValue($services)); | ||
|
|
||
| return $container; | ||
| } | ||
| } |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have to specify the FQCN of an alternative implementation (the already existing one) that will be used as a fallback to provide backwards compatibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're saying that I should write
and duplicate the code ?
or just say somewhere that I took this part of the code from #21283 which has already been reviewed ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or do I have to link to the old Implementation ?
or the last chance, make an if/else to import the new one or the old one ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xabbuh In fact this doesn't expect a fallback. For the AddConsoleCommandPass, we just added a conflict to the frameworkbundle so that it is always available (it was your suggestion #19443 (comment)). I started to do the same for the FormPass (#21283) and SerializerPass(#21293), is it ok to do the same here or should we fallback to the deprecated one and mute deprecation instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right. Adding the conflict rule should be the solution here too.