diff --git a/CHANGELOG-1.9.md b/CHANGELOG-1.9.md index 38b1ae4102..e2729e4425 100644 --- a/CHANGELOG-1.9.md +++ b/CHANGELOG-1.9.md @@ -1,5 +1,18 @@ # CHANGELOG FOR `1.9.X` +## v1.9.0-BETA.3 (2021-02-15) + +#### Details + +- [#12313](https://github.com/Sylius/Sylius/issues/12313) Change package type from "project" to "library" ([@vvasiloi](https://github.com/vvasiloi)) +- [#12322](https://github.com/Sylius/Sylius/issues/12322) update documentation for creating custom model ([@arti0090](https://github.com/arti0090)) +- [#12328](https://github.com/Sylius/Sylius/issues/12328) update documentation for creating custom resource controller ([@arti0090](https://github.com/arti0090)) +- [#12331](https://github.com/Sylius/Sylius/issues/12331) Add note about removing Twig route config in the UPGRADE-1.9 file ([@GSadee](https://github.com/GSadee)) +- [#12332](https://github.com/Sylius/Sylius/issues/12332) Reorganize UPGRADE-1.9 file to extract steps for Symfony v5.2 ([@GSadee](https://github.com/GSadee)) +- [#12336](https://github.com/Sylius/Sylius/issues/12336) Fix timeout on 404 Not Found errors when using templating and Symfony 4.4 ([@pamil](https://github.com/pamil)) +- [#12338](https://github.com/Sylius/Sylius/issues/12338) [DOCS] Updating the controller customization ([@Roshyo](https://github.com/Roshyo)) +- [#12342](https://github.com/Sylius/Sylius/issues/12342) Fixes according to composer require commands in the UPGRADE-1.9 file ([@GSadee](https://github.com/GSadee)) + ## v1.9.0-BETA.2 (2021-02-10) #### Details diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 51bb8b9d0c..cbe268d63d 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -28,5 +28,11 @@ parameters: - 'src/Sylius/Bundle/ApiBundle/DependencyInjection/Compiler/ReflectionExtractorHotfixPass.php' - 'src/Sylius/Bundle/ApiBundle/PropertyInfo/Extractor/ReflectionExtractor.php' + # Symfony 4.4-specific issues + - 'src/Sylius/Bundle/CoreBundle/DependencyInjection/Compiler/CircularDependencyBreakingExceptionListenerPass.php' + - 'src/Sylius/Bundle/CoreBundle/EventListener/CircularDependencyBreakingExceptionListener.php' + - 'src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/CircularDependencyBreakingExceptionListenerPassTest.php' + - 'src/Sylius/Bundle/CoreBundle/Tests/Listener/CircularDependencyBreakingExceptionListenerTest.php' + ignoreErrors: - '/Access to an undefined property Doctrine\\Common\\Collections\\ArrayCollection/' diff --git a/psalm.xml b/psalm.xml index af5b8ed304..b2e9d1456a 100644 --- a/psalm.xml +++ b/psalm.xml @@ -91,6 +91,8 @@ + + diff --git a/src/Sylius/Bundle/CoreBundle/DependencyInjection/Compiler/CircularDependencyBreakingExceptionListenerPass.php b/src/Sylius/Bundle/CoreBundle/DependencyInjection/Compiler/CircularDependencyBreakingExceptionListenerPass.php new file mode 100644 index 0000000000..5327c416b5 --- /dev/null +++ b/src/Sylius/Bundle/CoreBundle/DependencyInjection/Compiler/CircularDependencyBreakingExceptionListenerPass.php @@ -0,0 +1,40 @@ +has('twig.exception_listener')) { + return; + } + + $definition = new Definition(CircularDependencyBreakingExceptionListener::class); + $definition->setDecoratedService('twig.exception_listener'); + $definition->addArgument(new Reference(CircularDependencyBreakingExceptionListener::class .'.inner')); + + $container->setDefinition( + CircularDependencyBreakingExceptionListener::class, + $definition + ); + } +} diff --git a/src/Sylius/Bundle/CoreBundle/EventListener/CircularDependencyBreakingErrorListener.php b/src/Sylius/Bundle/CoreBundle/EventListener/CircularDependencyBreakingErrorListener.php index 0d49886084..8aa0a078a8 100644 --- a/src/Sylius/Bundle/CoreBundle/EventListener/CircularDependencyBreakingErrorListener.php +++ b/src/Sylius/Bundle/CoreBundle/EventListener/CircularDependencyBreakingErrorListener.php @@ -19,6 +19,8 @@ use Symfony\Component\HttpKernel\Event\ExceptionEvent; use Symfony\Component\HttpKernel\EventListener\ErrorListener; /** + * For Symfony 5+. + * * `Symfony\Component\HttpKernel\EventListener\ErrorListener::onKernelException` happens to set previous * exception for a wrapper exception. This is meant to improve DX while debugging nested exceptions, * but also creates some issues. @@ -45,7 +47,7 @@ use Symfony\Component\HttpKernel\EventListener\ErrorListener; */ final class CircularDependencyBreakingErrorListener extends ErrorListener { - /** @var CircularDependencyBreakingErrorListener */ + /** @var ErrorListener */ private $decoratedListener; public function __construct(ErrorListener $decoratedListener) @@ -61,6 +63,7 @@ final class CircularDependencyBreakingErrorListener extends ErrorListener public function onKernelException(ExceptionEvent $event, string $eventName = null, EventDispatcherInterface $eventDispatcher = null): void { try { + /** @psalm-suppress TooManyArguments */ $this->decoratedListener->onKernelException($event, $eventName, $eventDispatcher); } catch (\Throwable $throwable) { $this->breakCircularDependency($throwable); diff --git a/src/Sylius/Bundle/CoreBundle/EventListener/CircularDependencyBreakingExceptionListener.php b/src/Sylius/Bundle/CoreBundle/EventListener/CircularDependencyBreakingExceptionListener.php new file mode 100644 index 0000000000..741b536b43 --- /dev/null +++ b/src/Sylius/Bundle/CoreBundle/EventListener/CircularDependencyBreakingExceptionListener.php @@ -0,0 +1,96 @@ +getPrevious()->getPrevious()->getPrevious() === $exception` + * + * That exception is rethrown and other listeners like `Symfony\Component\Security\Http\Firewall\ExceptionListener` + * try to deal with an exception and all their previous ones, causing infinite loops. + * + * This fix only works if "framework.templating" setting DOES NOT include "twig". Otherwise, TwigBundle + * registers deprecated `Symfony\Component\HttpKernel\EventListener\ExceptionListener`, removes the non-deprecated + * "exception_listener" service, so that the issue still persists. + * + * This listener behaves as a decorator, but also extends the original ExceptionListener, because of yet another + * listener `ApiPlatform\Core\EventListener\ExceptionListener` requires the original class. + * + * @internal + * + * @see \Symfony\Component\HttpKernel\EventListener\ExceptionListener + * + * @psalm-suppress UndefinedClass + * @psalm-suppress MissingDependency + */ +final class CircularDependencyBreakingExceptionListener extends ExceptionListener +{ + /** @var ExceptionListener */ + private $decoratedListener; + + public function __construct(ExceptionListener $decoratedListener) + { + $this->decoratedListener = $decoratedListener; + } + + public function logKernelException(GetResponseForExceptionEvent $event): void + { + $this->decoratedListener->logKernelException($event); + } + + public function onKernelException(GetResponseForExceptionEvent $event): void + { + try { + $this->decoratedListener->onKernelException($event); + } catch (\Throwable $throwable) { + $this->breakCircularDependency($throwable); + + throw $throwable; + } + } + + private function breakCircularDependency(\Throwable $throwable): void + { + $throwables = []; + + do { + $throwables[] = $throwable; + + if (in_array($throwable->getPrevious(), $throwables, true)) { + $this->removePreviousFromThrowable($throwable); + } + + $throwable = $throwable->getPrevious(); + } while (null !== $throwable); + } + + private function removePreviousFromThrowable(\Throwable $throwable): void + { + $previous = new \ReflectionProperty($throwable instanceof \Exception ? \Exception::class : \Error::class, 'previous'); + $previous->setAccessible(true); + $previous->setValue($throwable, null); + } +} diff --git a/src/Sylius/Bundle/CoreBundle/SyliusCoreBundle.php b/src/Sylius/Bundle/CoreBundle/SyliusCoreBundle.php index b7ffad871a..0290d5caf2 100644 --- a/src/Sylius/Bundle/CoreBundle/SyliusCoreBundle.php +++ b/src/Sylius/Bundle/CoreBundle/SyliusCoreBundle.php @@ -22,6 +22,7 @@ use Doctrine\Inflector\Rules\Transformations; use Doctrine\Inflector\Rules\Word; use Sylius\Bundle\CoreBundle\DependencyInjection\Compiler\BackwardsCompatibility\ResolveShopUserTargetEntityPass; use Sylius\Bundle\CoreBundle\DependencyInjection\Compiler\CircularDependencyBreakingErrorListenerPass; +use Sylius\Bundle\CoreBundle\DependencyInjection\Compiler\CircularDependencyBreakingExceptionListenerPass; use Sylius\Bundle\CoreBundle\DependencyInjection\Compiler\IgnoreAnnotationsPass; use Sylius\Bundle\CoreBundle\DependencyInjection\Compiler\LazyCacheWarmupPass; use Sylius\Bundle\CoreBundle\DependencyInjection\Compiler\LiipImageFiltersPass; @@ -62,6 +63,7 @@ final class SyliusCoreBundle extends AbstractResourceBundle parent::build($container); $container->addCompilerPass(new CircularDependencyBreakingErrorListenerPass()); + $container->addCompilerPass(new CircularDependencyBreakingExceptionListenerPass()); $container->addCompilerPass(new LazyCacheWarmupPass()); $container->addCompilerPass(new RegisterTaxCalculationStrategiesPass()); $container->addCompilerPass(new TranslatableEntityLocalePass()); diff --git a/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/CircularDependencyBreakingErrorListenerPassTest.php b/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/CircularDependencyBreakingErrorListenerPassTest.php index fd2926d692..cfbabf80a1 100644 --- a/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/CircularDependencyBreakingErrorListenerPassTest.php +++ b/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/CircularDependencyBreakingErrorListenerPassTest.php @@ -17,6 +17,7 @@ use Matthias\SymfonyDependencyInjectionTest\PhpUnit\AbstractCompilerPassTestCase use Sylius\Bundle\CoreBundle\DependencyInjection\Compiler\CircularDependencyBreakingErrorListenerPass; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Definition; +use Sylius\Bundle\CoreBundle\EventListener\CircularDependencyBreakingErrorListener; final class CircularDependencyBreakingErrorListenerPassTest extends AbstractCompilerPassTestCase { @@ -27,14 +28,14 @@ final class CircularDependencyBreakingErrorListenerPassTest extends AbstractComp $this->compile(); - $this->assertContainerBuilderHasService('Sylius\Bundle\CoreBundle\EventListener\CircularDependencyBreakingErrorListener'); + $this->assertContainerBuilderHasService(CircularDependencyBreakingErrorListener::class); } public function it_does_nothing_when_exception_listener_is_not_registered(): void { $this->compile(); - $this->assertContainerBuilderNotHasService('Sylius\Bundle\CoreBundle\EventListener\CircularDependencyBreakingErrorListener'); + $this->assertContainerBuilderNotHasService(CircularDependencyBreakingErrorListener::class); } protected function registerCompilerPass(ContainerBuilder $container): void diff --git a/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/CircularDependencyBreakingExceptionListenerPassTest.php b/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/CircularDependencyBreakingExceptionListenerPassTest.php new file mode 100644 index 0000000000..a360acbaca --- /dev/null +++ b/src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/CircularDependencyBreakingExceptionListenerPassTest.php @@ -0,0 +1,45 @@ +container->setDefinition('twig.exception_listener', new Definition('ExceptionListener')); + + $this->compile(); + + $this->assertContainerBuilderHasService(CircularDependencyBreakingExceptionListener::class); + } + + public function it_does_nothing_when_exception_listener_is_not_registered(): void + { + $this->compile(); + + $this->assertContainerBuilderNotHasService(CircularDependencyBreakingExceptionListener::class); + } + + protected function registerCompilerPass(ContainerBuilder $container): void + { + $container->addCompilerPass(new CircularDependencyBreakingExceptionListenerPass()); + } +} diff --git a/src/Sylius/Bundle/CoreBundle/Tests/Listener/CircularDependencyBreakingErrorListenerTest.php b/src/Sylius/Bundle/CoreBundle/Tests/Listener/CircularDependencyBreakingErrorListenerTest.php new file mode 100644 index 0000000000..0d1d8eb673 --- /dev/null +++ b/src/Sylius/Bundle/CoreBundle/Tests/Listener/CircularDependencyBreakingErrorListenerTest.php @@ -0,0 +1,148 @@ +createMock(ErrorListener::class); + $listener = new CircularDependencyBreakingErrorListener($decoratedListener); + + $event = $this->createExceptionEvent(); + + $secondException = new \Exception('Second'); + $firstException = new \Exception('First', 0, $secondException); + $this->setPreviousForException($secondException, $firstException); + + $decoratedListener->method('onKernelException')->willThrowException($firstException); + + // Pre-assert + Assert::assertSame($secondException, $firstException->getPrevious()); + Assert::assertSame($firstException, $secondException->getPrevious()); + + // Act + $throwable = null; + + try { + $listener->onKernelException($event); + } catch (\Throwable $throwable) { + } + + // Assert + Assert::assertNotNull($throwable); + Assert::assertSame($firstException, $throwable); + Assert::assertSame($secondException, $firstException->getPrevious()); + Assert::assertSame(null, $secondException->getPrevious()); + } + + /** @test */ + public function it_breaks_more_complex_circular_dependencies_in_exceptions(): void + { + // Arrange + $decoratedListener = $this->createMock(ErrorListener::class); + $listener = new CircularDependencyBreakingErrorListener($decoratedListener); + + $event = $this->createExceptionEvent(); + + $fourthException = new \Exception('Fourth'); + $thirdException = new \Exception('Third', 0, $fourthException); + $secondException = new \Exception('Second', 0, $thirdException); + $firstException = new \Exception('First', 0, $secondException); + $this->setPreviousForException($fourthException, $secondException); + + $decoratedListener->method('onKernelException')->willThrowException($firstException); + + // Pre-assert + Assert::assertSame($secondException, $firstException->getPrevious()); + Assert::assertSame($thirdException, $secondException->getPrevious()); + Assert::assertSame($fourthException, $thirdException->getPrevious()); + Assert::assertSame($secondException, $fourthException->getPrevious()); + + // Act + $throwable = null; + + try { + $listener->onKernelException($event); + } catch (\Throwable $throwable) { + } + + // Assert + Assert::assertNotNull($throwable); + Assert::assertSame($firstException, $throwable); + Assert::assertSame($secondException, $firstException->getPrevious()); + Assert::assertSame($thirdException, $secondException->getPrevious()); + Assert::assertSame($fourthException, $thirdException->getPrevious()); + Assert::assertSame(null, $fourthException->getPrevious()); + } + + /** @test */ + public function it_does_nothing_when_circular_dependencies_are_not_found(): void + { + // Arrange + $decoratedListener = $this->createMock(ErrorListener::class); + $listener = new CircularDependencyBreakingErrorListener($decoratedListener); + + $event = $this->createExceptionEvent(); + + $secondException = new \Exception('Second'); + $firstException = new \Exception('First', 0, $secondException); + + $decoratedListener->method('onKernelException')->willThrowException($firstException); + + // Pre-assert + Assert::assertSame($secondException, $firstException->getPrevious()); + Assert::assertSame(null, $secondException->getPrevious()); + + // Act + $throwable = null; + + try { + $listener->onKernelException($event); + } catch (\Throwable $throwable) { + } + + // Assert + Assert::assertNotNull($throwable); + Assert::assertSame($firstException, $throwable); + Assert::assertSame($secondException, $firstException->getPrevious()); + Assert::assertSame(null, $secondException->getPrevious()); + } + + private function setPreviousForException(\Exception $exception, ?\Exception $previous): void + { + $property = new \ReflectionProperty(\Exception::class, 'previous'); + $property->setAccessible(true); + $property->setValue($exception, $previous); + } + + private function createExceptionEvent(): ExceptionEvent + { + $kernel = $this->createMock(HttpKernelInterface::class); + $request = $this->createMock(Request::class); + $exception = new \Exception(); + + return new ExceptionEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST, $exception); + } +} diff --git a/src/Sylius/Bundle/CoreBundle/Tests/Listener/CircularDependencyBreakingExceptionListenerTest.php b/src/Sylius/Bundle/CoreBundle/Tests/Listener/CircularDependencyBreakingExceptionListenerTest.php index 732756bf41..d191bc028a 100644 --- a/src/Sylius/Bundle/CoreBundle/Tests/Listener/CircularDependencyBreakingExceptionListenerTest.php +++ b/src/Sylius/Bundle/CoreBundle/Tests/Listener/CircularDependencyBreakingExceptionListenerTest.php @@ -15,10 +15,10 @@ namespace Sylius\Bundle\CoreBundle\Tests\Listener; use PHPUnit\Framework\Assert; use PHPUnit\Framework\TestCase; -use Sylius\Bundle\CoreBundle\EventListener\CircularDependencyBreakingErrorListener; +use Sylius\Bundle\CoreBundle\EventListener\CircularDependencyBreakingExceptionListener; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Event\ExceptionEvent; -use Symfony\Component\HttpKernel\EventListener\ErrorListener; +use Symfony\Component\HttpKernel\EventListener\ExceptionListener; use Symfony\Component\HttpKernel\HttpKernelInterface; final class CircularDependencyBreakingExceptionListenerTest extends TestCase @@ -27,8 +27,8 @@ final class CircularDependencyBreakingExceptionListenerTest extends TestCase public function it_breaks_circular_dependencies_in_exceptions(): void { // Arrange - $decoratedListener = $this->createMock(ErrorListener::class); - $listener = new CircularDependencyBreakingErrorListener($decoratedListener); + $decoratedListener = $this->createMock(ExceptionListener::class); + $listener = new CircularDependencyBreakingExceptionListener($decoratedListener); $event = $this->createExceptionEvent(); @@ -61,8 +61,8 @@ final class CircularDependencyBreakingExceptionListenerTest extends TestCase public function it_breaks_more_complex_circular_dependencies_in_exceptions(): void { // Arrange - $decoratedListener = $this->createMock(ErrorListener::class); - $listener = new CircularDependencyBreakingErrorListener($decoratedListener); + $decoratedListener = $this->createMock(ExceptionListener::class); + $listener = new CircularDependencyBreakingExceptionListener($decoratedListener); $event = $this->createExceptionEvent(); @@ -101,8 +101,8 @@ final class CircularDependencyBreakingExceptionListenerTest extends TestCase public function it_does_nothing_when_circular_dependencies_are_not_found(): void { // Arrange - $decoratedListener = $this->createMock(ErrorListener::class); - $listener = new CircularDependencyBreakingErrorListener($decoratedListener); + $decoratedListener = $this->createMock(ExceptionListener::class); + $listener = new CircularDependencyBreakingExceptionListener($decoratedListener); $event = $this->createExceptionEvent();