From 86b850ebd8f7843a8a85cbe8a48953811c5ef32e Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Sat, 7 Apr 2018 16:29:23 +0200 Subject: [PATCH 1/7] [HttpKernel] Dont create mock cookie for new sessions in tests --- .../EventListener/AbstractTestSessionListener.php | 2 +- .../Tests/EventListener/TestSessionListenerTest.php | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Component/HttpKernel/EventListener/AbstractTestSessionListener.php b/src/Symfony/Component/HttpKernel/EventListener/AbstractTestSessionListener.php index 0a153dd943..82061fd6ea 100644 --- a/src/Symfony/Component/HttpKernel/EventListener/AbstractTestSessionListener.php +++ b/src/Symfony/Component/HttpKernel/EventListener/AbstractTestSessionListener.php @@ -69,7 +69,7 @@ abstract class AbstractTestSessionListener implements EventSubscriberInterface $session->save(); } - if ($session instanceof Session ? !$session->isEmpty() || $session->getId() !== $this->sessionId : $wasStarted) { + if ($session instanceof Session ? !$session->isEmpty() || (null !== $this->sessionId && $session->getId() !== $this->sessionId) : $wasStarted) { $params = session_get_cookie_params(); $event->getResponse()->headers->setCookie(new Cookie($session->getName(), $session->getId(), 0 === $params['lifetime'] ? 0 : time() + $params['lifetime'], $params['path'], $params['domain'], $params['secure'], $params['httponly'])); $this->sessionId = $session->getId(); diff --git a/src/Symfony/Component/HttpKernel/Tests/EventListener/TestSessionListenerTest.php b/src/Symfony/Component/HttpKernel/Tests/EventListener/TestSessionListenerTest.php index 0a2263d5a8..22a2b71239 100644 --- a/src/Symfony/Component/HttpKernel/Tests/EventListener/TestSessionListenerTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/EventListener/TestSessionListenerTest.php @@ -13,7 +13,6 @@ namespace Symfony\Component\HttpKernel\Tests\EventListener; use PHPUnit\Framework\TestCase; use Symfony\Component\DependencyInjection\ServiceSubscriberInterface; -use Symfony\Component\HttpFoundation\Cookie; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Event\GetResponseEvent; @@ -46,6 +45,9 @@ class TestSessionListenerTest extends TestCase { $this->listener = $this->getMockForAbstractClass('Symfony\Component\HttpKernel\EventListener\AbstractTestSessionListener'); $this->session = $this->getSession(); + $this->listener->expects($this->any()) + ->method('getSession') + ->will($this->returnValue($this->session)); } public function testShouldSaveMasterRequestSession() @@ -95,7 +97,7 @@ class TestSessionListenerTest extends TestCase $this->fixSessionId('456'); $kernel = $this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(); - $request = Request::create('/', 'GET', array(), array(new Cookie('MOCKSESSID', '123'))); + $request = Request::create('/', 'GET', array(), array('MOCKSESSID' => '123')); $event = new GetResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST); $this->listener->onKernelRequest($event); From cd914209bd3acbc637c4b435626f003696ebacb7 Mon Sep 17 00:00:00 2001 From: Pascal Montoya <> Date: Fri, 6 Apr 2018 07:52:15 +0200 Subject: [PATCH 2/7] [Validator] Fix LazyLoadingMetadataFactory with PSR6Cache for non classname if tested values isn't an existing class --- .../Factory/LazyLoadingMetadataFactory.php | 8 ++++---- .../Factory/LazyLoadingMetadataFactoryTest.php | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/Symfony/Component/Validator/Mapping/Factory/LazyLoadingMetadataFactory.php b/src/Symfony/Component/Validator/Mapping/Factory/LazyLoadingMetadataFactory.php index 1a4f3074b6..84375ec694 100644 --- a/src/Symfony/Component/Validator/Mapping/Factory/LazyLoadingMetadataFactory.php +++ b/src/Symfony/Component/Validator/Mapping/Factory/LazyLoadingMetadataFactory.php @@ -90,6 +90,10 @@ class LazyLoadingMetadataFactory implements MetadataFactoryInterface return $this->loadedClasses[$class]; } + if (!class_exists($class) && !interface_exists($class)) { + throw new NoSuchMetadataException(sprintf('The class or interface "%s" does not exist.', $class)); + } + if (null !== $this->cache && false !== ($metadata = $this->cache->read($class))) { // Include constraints from the parent class $this->mergeConstraints($metadata); @@ -97,10 +101,6 @@ class LazyLoadingMetadataFactory implements MetadataFactoryInterface return $this->loadedClasses[$class] = $metadata; } - if (!class_exists($class) && !interface_exists($class)) { - throw new NoSuchMetadataException(sprintf('The class or interface "%s" does not exist.', $class)); - } - $metadata = new ClassMetadata($class); if (null !== $this->loader) { diff --git a/src/Symfony/Component/Validator/Tests/Mapping/Factory/LazyLoadingMetadataFactoryTest.php b/src/Symfony/Component/Validator/Tests/Mapping/Factory/LazyLoadingMetadataFactoryTest.php index b5d1a9dc84..de6852271e 100644 --- a/src/Symfony/Component/Validator/Tests/Mapping/Factory/LazyLoadingMetadataFactoryTest.php +++ b/src/Symfony/Component/Validator/Tests/Mapping/Factory/LazyLoadingMetadataFactoryTest.php @@ -149,6 +149,21 @@ class LazyLoadingMetadataFactoryTest extends TestCase $this->assertEquals($metadata, $factory->getMetadataFor(self::PARENT_CLASS)); } + /** + * @expectedException \Symfony\Component\Validator\Exception\NoSuchMetadataException + */ + public function testNonClassNameStringValues() + { + $testedValue = 'error@example.com'; + $loader = $this->getMockBuilder('Symfony\Component\Validator\Mapping\Loader\LoaderInterface')->getMock(); + $cache = $this->getMockBuilder('Symfony\Component\Validator\Mapping\Cache\CacheInterface')->getMock(); + $factory = new LazyLoadingMetadataFactory($loader, $cache); + $cache + ->expects($this->never()) + ->method('read'); + $factory->getMetadataFor($testedValue); + } + public function testMetadataCacheWithRuntimeConstraint() { $cache = $this->getMockBuilder('Symfony\Component\Validator\Mapping\Cache\CacheInterface')->getMock(); From 5198f435a047aad8d5ed119e918cf8c008ea65b7 Mon Sep 17 00:00:00 2001 From: Pascal Montoya Date: Mon, 9 Apr 2018 10:29:56 +0200 Subject: [PATCH 3/7] Disable autoloader call on interface_exists check --- .../Validator/Mapping/Factory/LazyLoadingMetadataFactory.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Validator/Mapping/Factory/LazyLoadingMetadataFactory.php b/src/Symfony/Component/Validator/Mapping/Factory/LazyLoadingMetadataFactory.php index 84375ec694..e088caa897 100644 --- a/src/Symfony/Component/Validator/Mapping/Factory/LazyLoadingMetadataFactory.php +++ b/src/Symfony/Component/Validator/Mapping/Factory/LazyLoadingMetadataFactory.php @@ -90,7 +90,7 @@ class LazyLoadingMetadataFactory implements MetadataFactoryInterface return $this->loadedClasses[$class]; } - if (!class_exists($class) && !interface_exists($class)) { + if (!class_exists($class) && !interface_exists($class, false)) { throw new NoSuchMetadataException(sprintf('The class or interface "%s" does not exist.', $class)); } From 7f398117f490b724dcfedd3663c76dfc49553392 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Sat, 7 Apr 2018 15:42:06 +0200 Subject: [PATCH 4/7] [DI] Improve error message for non-autowirable scalar argument --- .../Compiler/AutowirePass.php | 5 ++++- .../Tests/Compiler/AutowirePassTest.php | 22 ++++++++++++++++++- .../Fixtures/includes/autowiring_classes.php | 2 +- 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php index 8bfc3cd9d9..803490cecd 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php @@ -233,7 +233,10 @@ class AutowirePass extends AbstractRecursivePass if ($parameter->isOptional()) { continue; } - throw new AutowiringFailedException($this->currentId, sprintf('Cannot autowire service "%s": argument "$%s" of method "%s()" must have a type-hint or be given a value explicitly.', $this->currentId, $parameter->name, $class !== $this->currentId ? $class.'::'.$method : $method)); + $type = ProxyHelper::getTypeHint($reflectionMethod, $parameter, false); + $type = $type ? sprintf('is type-hinted "%s"', $type) : 'has no type-hint'; + + throw new AutowiringFailedException($this->currentId, sprintf('Cannot autowire service "%s": argument "$%s" of method "%s()" %s, you should configure its value explicitly.', $this->currentId, $parameter->name, $class !== $this->currentId ? $class.'::'.$method : $method, $type)); } // specifically pass the default value diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php index ee0b4baef8..924a490942 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php @@ -436,6 +436,7 @@ class AutowirePassTest extends TestCase // args are: A, Foo, Dunglas ->setArguments(array( 1 => new Reference('foo'), + 3 => array('bar'), )); (new ResolveClassPass())->process($container); @@ -447,6 +448,7 @@ class AutowirePassTest extends TestCase new TypedReference(A::class, A::class, MultipleArguments::class), new Reference('foo'), new TypedReference(Dunglas::class, Dunglas::class, MultipleArguments::class), + array('bar'), ), $definition->getArguments() ); @@ -454,12 +456,30 @@ class AutowirePassTest extends TestCase /** * @expectedException \Symfony\Component\DependencyInjection\Exception\AutowiringFailedException - * @expectedExceptionMessage Cannot autowire service "arg_no_type_hint": argument "$foo" of method "Symfony\Component\DependencyInjection\Tests\Compiler\MultipleArguments::__construct()" must have a type-hint or be given a value explicitly. + * @expectedExceptionMessage Cannot autowire service "arg_no_type_hint": argument "$bar" of method "Symfony\Component\DependencyInjection\Tests\Compiler\MultipleArguments::__construct()" is type-hinted "array", you should configure its value explicitly. */ public function testScalarArgsCannotBeAutowired() { $container = new ContainerBuilder(); + $container->register(A::class); + $container->register(Dunglas::class); + $container->register('arg_no_type_hint', __NAMESPACE__.'\MultipleArguments') + ->setArguments(array(1 => 'foo')) + ->setAutowired(true); + + (new ResolveClassPass())->process($container); + (new AutowirePass())->process($container); + } + + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\AutowiringFailedException + * @expectedExceptionMessage Cannot autowire service "arg_no_type_hint": argument "$foo" of method "Symfony\Component\DependencyInjection\Tests\Compiler\MultipleArguments::__construct()" has no type-hint, you should configure its value explicitly. + */ + public function testNoTypeArgsCannotBeAutowired() + { + $container = new ContainerBuilder(); + $container->register(A::class); $container->register(Dunglas::class); $container->register('arg_no_type_hint', __NAMESPACE__.'\MultipleArguments') diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/autowiring_classes.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/autowiring_classes.php index 990a873f34..6f93957753 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/autowiring_classes.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/autowiring_classes.php @@ -181,7 +181,7 @@ class NotGuessableArgumentForSubclass } class MultipleArguments { - public function __construct(A $k, $foo, Dunglas $dunglas) + public function __construct(A $k, $foo, Dunglas $dunglas, array $bar) { } } From adba79a6b01800bd7cc0bc9795dcb46539330f7f Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Mon, 9 Apr 2018 12:07:56 -0500 Subject: [PATCH 5/7] [Console] Don't go past exact matches when autocompleting --- .../Console/Helper/QuestionHelper.php | 2 +- .../Tests/Helper/QuestionHelperTest.php | 24 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/Console/Helper/QuestionHelper.php b/src/Symfony/Component/Console/Helper/QuestionHelper.php index bce0534ed0..36187f8cea 100644 --- a/src/Symfony/Component/Console/Helper/QuestionHelper.php +++ b/src/Symfony/Component/Console/Helper/QuestionHelper.php @@ -284,7 +284,7 @@ class QuestionHelper extends Helper foreach ($autocomplete as $value) { // If typed characters match the beginning chunk of value (e.g. [AcmeDe]moBundle) - if (0 === strpos($value, $ret) && $i !== strlen($value)) { + if (0 === strpos($value, $ret)) { $matches[$numMatches++] = $value; } } diff --git a/src/Symfony/Component/Console/Tests/Helper/QuestionHelperTest.php b/src/Symfony/Component/Console/Tests/Helper/QuestionHelperTest.php index d9bdb606ac..9f837d0f9e 100644 --- a/src/Symfony/Component/Console/Tests/Helper/QuestionHelperTest.php +++ b/src/Symfony/Component/Console/Tests/Helper/QuestionHelperTest.php @@ -160,6 +160,30 @@ class QuestionHelperTest extends TestCase $this->assertEquals('AsseticBundle', $dialog->ask($this->createInputInterfaceMock(), $this->createOutputInterface(), $question)); } + public function testAskWithAutocompleteWithExactMatch() + { + if (!$this->hasSttyAvailable()) { + $this->markTestSkipped('`stty` is required to test autocomplete functionality'); + } + + $inputStream = $this->getInputStream("b\n"); + + $possibleChoices = array( + 'a' => 'berlin', + 'b' => 'copenhagen', + 'c' => 'amsterdam', + ); + + $dialog = new QuestionHelper(); + $dialog->setInputStream($inputStream); + $dialog->setHelperSet(new HelperSet(array(new FormatterHelper()))); + + $question = new ChoiceQuestion('Please select a city', $possibleChoices); + $question->setMaxAttempts(1); + + $this->assertSame('b', $dialog->ask($this->createInputInterfaceMock(), $this->createOutputInterface(), $question)); + } + public function testAutocompleteWithTrailingBackslash() { if (!$this->hasSttyAvailable()) { From 40bd8bd797ae8921e2a04f71a1ba7e1d3c0c29a3 Mon Sep 17 00:00:00 2001 From: Normunds Date: Mon, 9 Apr 2018 15:54:29 +0300 Subject: [PATCH 6/7] Add d-block to bootstrap 4 alerts --- .../Twig/Resources/views/Form/bootstrap_4_layout.html.twig | 2 +- .../Form/Tests/AbstractBootstrap4HorizontalLayoutTest.php | 2 +- .../Component/Form/Tests/AbstractBootstrap4LayoutTest.php | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Symfony/Bridge/Twig/Resources/views/Form/bootstrap_4_layout.html.twig b/src/Symfony/Bridge/Twig/Resources/views/Form/bootstrap_4_layout.html.twig index df88a00011..1c5c390954 100644 --- a/src/Symfony/Bridge/Twig/Resources/views/Form/bootstrap_4_layout.html.twig +++ b/src/Symfony/Bridge/Twig/Resources/views/Form/bootstrap_4_layout.html.twig @@ -267,7 +267,7 @@ {% block form_errors -%} {%- if errors|length > 0 -%} - + {%- for error in errors -%} {{ 'Error'|trans({}, 'validators') }} {{ error.message }} diff --git a/src/Symfony/Component/Form/Tests/AbstractBootstrap4HorizontalLayoutTest.php b/src/Symfony/Component/Form/Tests/AbstractBootstrap4HorizontalLayoutTest.php index c906c6549a..016792e0ed 100644 --- a/src/Symfony/Component/Form/Tests/AbstractBootstrap4HorizontalLayoutTest.php +++ b/src/Symfony/Component/Form/Tests/AbstractBootstrap4HorizontalLayoutTest.php @@ -32,7 +32,7 @@ abstract class AbstractBootstrap4HorizontalLayoutTest extends AbstractBootstrap4 [ ./label[@for="name"] [ - ./span[@class="alert alert-danger"] + ./span[@class="alert alert-danger d-block"] [./span[@class="mb-0 d-block"] [./span[.="[trans]Error[/trans]"]] [./span[.="[trans]Error![/trans]"]] diff --git a/src/Symfony/Component/Form/Tests/AbstractBootstrap4LayoutTest.php b/src/Symfony/Component/Form/Tests/AbstractBootstrap4LayoutTest.php index beabaa21cd..2cadba0ba9 100644 --- a/src/Symfony/Component/Form/Tests/AbstractBootstrap4LayoutTest.php +++ b/src/Symfony/Component/Form/Tests/AbstractBootstrap4LayoutTest.php @@ -32,7 +32,7 @@ abstract class AbstractBootstrap4LayoutTest extends AbstractBootstrap3LayoutTest [ ./label[@for="name"] [ - ./span[@class="alert alert-danger"] + ./span[@class="alert alert-danger d-block"] [./span[@class="mb-0 d-block"] [./span[.="[trans]Error[/trans]"]] [./span[.="[trans]Error![/trans]"]] @@ -161,7 +161,7 @@ abstract class AbstractBootstrap4LayoutTest extends AbstractBootstrap3LayoutTest $this->assertMatchesXpath($html, '/span - [@class="alert alert-danger"] + [@class="alert alert-danger d-block"] [ ./span[@class="mb-0 d-block"] [./span[.="[trans]Error[/trans]"]] From 3b47441fd5be8714ea7086c581a2f8d07fc64173 Mon Sep 17 00:00:00 2001 From: Mathieu TUDISCO Date: Tue, 3 Apr 2018 15:10:16 +0200 Subject: [PATCH 7/7] [HttpKernel] Make ServiceValueResolver work if controller namespace starts with a backslash in routing --- .../ArgumentResolver/ServiceValueResolver.php | 12 +++++++++++- .../ServiceValueResolverTest.php | 19 +++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/ServiceValueResolver.php b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/ServiceValueResolver.php index c55564c046..7bc195f233 100644 --- a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/ServiceValueResolver.php +++ b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/ServiceValueResolver.php @@ -39,9 +39,15 @@ final class ServiceValueResolver implements ArgumentValueResolverInterface if (\is_array($controller) && \is_callable($controller, true) && \is_string($controller[0])) { $controller = $controller[0].'::'.$controller[1]; + } elseif (!\is_string($controller) || '' === $controller) { + return false; } - return \is_string($controller) && $this->container->has($controller) && $this->container->get($controller)->has($argument->getName()); + if ('\\' === $controller[0]) { + $controller = ltrim($controller, '\\'); + } + + return $this->container->has($controller) && $this->container->get($controller)->has($argument->getName()); } /** @@ -53,6 +59,10 @@ final class ServiceValueResolver implements ArgumentValueResolverInterface $controller = $controller[0].'::'.$controller[1]; } + if ('\\' === $controller[0]) { + $controller = ltrim($controller, '\\'); + } + yield $this->container->get($controller)->get($argument->getName()); } } diff --git a/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/ServiceValueResolverTest.php b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/ServiceValueResolverTest.php index b05828f5bf..7d34172ce3 100644 --- a/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/ServiceValueResolverTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/ServiceValueResolverTest.php @@ -47,6 +47,25 @@ class ServiceValueResolverTest extends TestCase $this->assertYieldEquals(array(new DummyService()), $resolver->resolve($request, $argument)); } + public function testExistingControllerWithATrailingBackSlash() + { + $resolver = new ServiceValueResolver(new ServiceLocator(array( + 'App\\Controller\\Mine::method' => function () { + return new ServiceLocator(array( + 'dummy' => function () { + return new DummyService(); + }, + )); + }, + ))); + + $request = $this->requestWithAttributes(array('_controller' => '\\App\\Controller\\Mine::method')); + $argument = new ArgumentMetadata('dummy', DummyService::class, false, false, null); + + $this->assertTrue($resolver->supports($request, $argument)); + $this->assertYieldEquals(array(new DummyService()), $resolver->resolve($request, $argument)); + } + public function testControllerNameIsAnArray() { $resolver = new ServiceValueResolver(new ServiceLocator(array(