8000 [HttpKernel] Added support for timings in ArgumentValueResolvers by linaori · Pull Request #26833 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[HttpKernel] Added support for timings in ArgumentValueResolvers #26833

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

Merged
merged 1 commit into from
Apr 20, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
[HttpKernel] Added support for timings in ArgumentValueResolvers
  • Loading branch information
Iltar van der Berg authored and fabpot committed Apr 20, 2018
commit 1c0d8928aa78f0eb18cc812b63c3feb1c6feebd9
2 changes: 2 additions & 0 deletions src/Symfony/Component/HttpKernel/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ CHANGELOG
* added orphaned events support to `EventDataCollector`
* `ExceptionListener` now logs and collects exceptions at priority `2048` (previously logged at `-128` and collected at `0`)
* Deprecated `service:action` syntax with a single colon to reference controllers. Use `service::method` instead.
* Added the ability to profile individual argument value resolvers via the
`Symfony\Component\HttpKernel\Controller\ArgumentResolver\TraceableValueResolver`

4.0.0
-----
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
<?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\Component\HttpKernel\Controller\ArgumentResolver;

use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\Controller\ArgumentValueResolverInterface;
use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata;
use Symfony\Component\Stopwatch\Stopwatch;

/**
* Provides timing information via the stopwatch.
*
* @author Iltar van der Berg <kjarli@gmail.com>
*/
final class TraceableValueResolver implements ArgumentValueResolverInterface
{
private $inner;
private $stopwatch;

public function __construct(ArgumentValueResolverInterface $inner, ?Stopwatch $stopwatch = null)
{
$this->inner = $inner;
$this->stopwatch = $stopwatch ?? new Stopwatch();
}

/**
* {@inheritdoc}
*/
public function supports(Request $request, ArgumentMetadata $argument): bool
{
$method = \get_class($this->inner).'::'.__FUNCTION__;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could also just do $method = \get_class($this->inner).'::supports'; and resolve for the other. Not sure what is preferred. Opinions?

Copy link
Contributor

Choose a reason for hiding this comment

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

👍for __FUNCTION__

$this->stopwatch->start($method);

$return = $this->inner->supports($request, $argument);

$this->stopwatch->stop($method);

return $return;
}

/**
* {@inheritdoc}
*/
public function resolve(Request $request, ArgumentMetadata $argument): iterable
{
$method = \get_class($this->inner).'::'.__FUNCTION__;
$this->stopwatch->start($method);

yield from $this->inner->resolve($request, $argument);

$this->stopwatch->stop($method);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\Compiler\PriorityTaggedServiceTrait;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\HttpKernel\Controller\ArgumentResolver\TraceableValueResolver;
use Symfony\Component\Stopwatch\Stopwatch;

/**
* Gathers and configures the argument value resolvers.
Expand All @@ -40,9 +44,20 @@ public function process(ContainerBuilder $container)
return;
}

$resolvers = $this->findAndSortTaggedServices($this->argumentValueResolverTag, $container);
Copy link
Member

Choose a reason for hiding this comment

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

assignment does not seem necessary, leftover?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's actually being used for new IteratorArgument($resolvers). I initially wasn't able to use this as I needed the id, but casting the reference to a string gives me exactly that, saving a call to the container.

Copy link
Member

Choose a reason for hiding this comment

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

oops, missed the loop!


if ($container->getParameter('kernel.debug') && class_exists(Stopwatch::class)) {
foreach ($resolvers as $resolverReference) {
$id = (string) $resolverReference;
$container->register("debug.$id", TraceableValueResolver::class)
->setDecoratedService($id)
->setArguments(array(new Reference("debug.$id.inner"), new Reference('debug.stopwatch', ContainerInterface::NULL_ON_INVALID_REFERENCE)));
}
}

$container
->getDefinition($this->argumentResolverService)
->replaceArgument(1, new IteratorArgument($this->findAndSortTaggedServices($this->argumentValueResolverTag, $container)))
->replaceArgument(1, new IteratorArgument($resolvers))
;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
<?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\Component\HttpKernel\Tests\Controller\ArgumentResolver;

use PHPUnit\Framework\TestCase;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\Controller\ArgumentResolver\TraceableValueResolver;
use Symfony\Component\HttpKernel\Controller\ArgumentValueResolverInterface;
use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata;
use Symfony\Component\Stopwatch\Stopwatch;

class TraceableValueResolverTest extends TestCase
{
public function testTimingsInSupports()
{
$stopwatch = new Stopwatch();
$resolver = new TraceableValueResolver(new ResolverStub(), $stopwatch);
$argument = new ArgumentMetadata('dummy', 'string', false, false, null);
$request = new Request();

$this->assertTrue($resolver->supports($request, $argument));

$event = $stopwatch->getEvent(ResolverStub::class.'::supports');
$this->assertCount(1, $event->getPeriods());
}

public function testTimingsInResolve()
{
$stopwatch = new Stopwatch();
$resolver = new TraceableValueResolver(new ResolverStub(), $stopwatch);
$argument = new ArgumentMetadata('dummy', 'string', false, false, null);
$request = new Request();

$iterable = $resolver->resolve($request, $argument);

foreach ($iterable as $index => $resolved) {
$event = $stopwatch->getEvent(ResolverStub::class.'::resolve');
$this->assertTrue($event->isStarted());
$this->assertEmpty($event->getPeriods());
switch ($index) {
case 0:
$this->assertEquals('first', $resolved);
break;
case 1:
$this->assertEquals('second', $resolved);
break;
}
}

$event = $stopwatch->getEvent(ResolverStub::class.'::resolve');
$this->assertCount(1, $event->getPeriods());
}
}

class ResolverStub implements ArgumentValueResolverInterface
{
public function supports(Request $request, ArgumentMetadata $argument)
{
return true;
}

public function resolve(Request $request, ArgumentMetadata $argument)
{
yield 'first';
yield 'second';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,50 @@ public function testServicesAreOrderedAccordingToPriority()
$container->register($id)->addTag('controller.argument_value_resolver', $tag);
}

$container->setParameter('kernel.debug', false);

(new ControllerArgumentValueResolverPass())->process($container);
$this->assertEquals($expected, $definition->getArgument(1)->getValues());

$this->assertFalse($container->hasDefinition('n1.traceable'));
$this->assertFalse($container->hasDefinition('n2.traceable'));
$this->assertFalse($container->hasDefinition('n3.traceable'));
}

public function testInDebug()
{
$services = array(
'n3' => array(array()),
'n1' => array(array('priority' => 200)),
'n2' => array(array('priority' => 100)),
);

$expected = array(
new Reference('n1'),
new Reference('n2'),
new Reference('n3'),
);

$definition = new Definition(ArgumentResolver::class, array(null, array()));
$container = new ContainerBuilder();
$container->setDefinition('argument_resolver', $definition);

foreach ($services as $id => list($tag)) {
$container->register($id)->addTag('controller.argument_value_resolver', $tag);
}

$container->setParameter('kernel.debug', true);

(new ControllerArgumentValueResolverPass())->process($container);
$this->assertEquals($expected, $definition->getArgument(1)->getValues());

$this->assertTrue($container->hasDefinition('debug.n1'));
$this->assertTrue($container->hasDefinition('debug.n2'));
$this->assertTrue($container->hasDefinition('debug.n3'));

$this->assertTrue($container->hasDefinition('n1'));
$this->assertTrue($container->hasDefinition('n2'));
$this->assertTrue($container->hasDefinition('n3'));
}

public function testReturningEmptyArrayWhenNoService()
Expand All @@ -52,6 +94,8 @@ public function testReturningEmptyArrayWhenNoService()
$container = new ContainerBuilder();
$container->setDefinition('argument_resolver', $definition);

$container->setParameter('kernel.debug', false);

(new ControllerArgumentValueResolverPass())->process($container);
$this->assertEquals(array(), $definition->getArgument(1)->getValues());
}
Expand Down
0