From bf417055c05bb022ba5f249b5413e0faf9458fea Mon Sep 17 00:00:00 2001 From: Divesh Pahuja Date: Tue, 14 May 2019 15:17:03 +0200 Subject: [PATCH] Updates for SensioFrameworkExtraBundle latest version(^5.2) (#4348) * routing annotations deprecation fix * Twig_Extension deprecations fix * Update configurations * Add legacy implementation for templates backward compatibility * review code changes * Fix tests * review fixes * Refactored template engine in LegacyTemplateListener * Symfony 4 compatibility --- .../Compiler/LegacyTemplatePass.php | 50 ++++++ .../Compiler/RoutingLoaderPass.php | 41 +++++ .../PimcoreCoreExtension.php | 25 --- .../EventListener/LegacyTemplateListener.php | 120 ++++++++++++++ .../TemplateControllerListener.php | 7 - bundles/CoreBundle/PimcoreCoreBundle.php | 4 + .../Resources/config/pimcore/default.yml | 4 + .../Templating/LegacyTemplateGuesser.php | 146 ++++++++++++++++++ composer.json | 2 +- lib/Controller/Config/ConfigNormalizer.php | 2 - .../Loader/AnnotatedRouteControllerLoader.php | 2 +- lib/Twig/Extension/AssetCompressExtension.php | 3 +- lib/Twig/Extension/DocumentTagExtension.php | 3 +- lib/Twig/Extension/DumpExtension.php | 3 +- lib/Twig/Extension/GlossaryExtension.php | 3 +- lib/Twig/Extension/HelpersExtension.php | 4 +- lib/Twig/Extension/NavigationExtension.php | 3 +- lib/Twig/Extension/PimcoreObjectExtension.php | 3 +- lib/Twig/Extension/SubrequestExtension.php | 3 +- .../Extension/TemplatingHelperExtension.php | 3 +- lib/Twig/Extension/WebsiteConfigExtension.php | 3 +- 21 files changed, 388 insertions(+), 46 deletions(-) create mode 100644 bundles/CoreBundle/DependencyInjection/Compiler/LegacyTemplatePass.php create mode 100644 bundles/CoreBundle/DependencyInjection/Compiler/RoutingLoaderPass.php create mode 100644 bundles/CoreBundle/EventListener/LegacyTemplateListener.php create mode 100644 bundles/CoreBundle/Templating/LegacyTemplateGuesser.php diff --git a/bundles/CoreBundle/DependencyInjection/Compiler/LegacyTemplatePass.php b/bundles/CoreBundle/DependencyInjection/Compiler/LegacyTemplatePass.php new file mode 100644 index 00000000000..57b2c1bc8f6 --- /dev/null +++ b/bundles/CoreBundle/DependencyInjection/Compiler/LegacyTemplatePass.php @@ -0,0 +1,50 @@ +hasDefinition('sensio_framework_extra.view.guesser')) { + $definition = $container->getDefinition('sensio_framework_extra.view.guesser'); + $definition + ->setPublic(true) + ->setClass(LegacyTemplateGuesser::class) + ->addArgument(new Reference('templating')); + } + + + if ($container->hasDefinition('sensio_framework_extra.view.listener')) { + $definition = $container->getDefinition('sensio_framework_extra.view.listener'); + $definition + ->setClass(LegacyTemplateListener::class) + ->addMethodCall('setTemplateEngine', [ + new Reference('templating') + ]); + } + } +} diff --git a/bundles/CoreBundle/DependencyInjection/Compiler/RoutingLoaderPass.php b/bundles/CoreBundle/DependencyInjection/Compiler/RoutingLoaderPass.php new file mode 100644 index 00000000000..3d623c22893 --- /dev/null +++ b/bundles/CoreBundle/DependencyInjection/Compiler/RoutingLoaderPass.php @@ -0,0 +1,41 @@ +hasDefinition('routing.loader.annotation')) { + return; + } + + $definition = $container->getDefinition('routing.loader.annotation'); + $definition->setClass(AnnotatedRouteControllerLoader::class); + } +} diff --git a/bundles/CoreBundle/DependencyInjection/PimcoreCoreExtension.php b/bundles/CoreBundle/DependencyInjection/PimcoreCoreExtension.php index 867bcf611ab..229ef83dedb 100644 --- a/bundles/CoreBundle/DependencyInjection/PimcoreCoreExtension.php +++ b/bundles/CoreBundle/DependencyInjection/PimcoreCoreExtension.php @@ -25,7 +25,6 @@ use Pimcore\Migrations\Configuration\ConfigurationFactory; use Pimcore\Model\Document\Tag\Loader\PrefixLoader as DocumentTagPrefixLoader; use Pimcore\Model\Factory; -use Pimcore\Routing\Loader\AnnotatedRouteControllerLoader; use Pimcore\Sitemap\EventListener\SitemapGeneratorListener; use Pimcore\Targeting\ActionHandler\DelegatingActionHandler; use Pimcore\Targeting\DataLoaderInterface; @@ -81,8 +80,6 @@ public function loadInternal(array $config, ContainerBuilder $container) // TODO only extract what we need as parameter? $container->setParameter('pimcore.config', $config); - $this->setAnnotationRouteControllerLoader($container); - $loader = new YamlFileLoader( $container, new FileLocator(__DIR__ . '/../Resources/config') @@ -502,28 +499,6 @@ private function addContextRoutes(ContainerBuilder $container, array $config) } } - /** - * Set annotation loader to our own implementation normalizing admin routes: converts the prefix - * pimcore_pimcoreadmin_ to just pimcore_admin_ - * - * @param ContainerBuilder $container - */ - private function setAnnotationRouteControllerLoader(ContainerBuilder $container) - { - $parameter = 'sensio_framework_extra.routing.loader.annot_class.class'; - - // make sure the parameter is not dropped by sensio framework extra bundle - // if this exception is thrown, implement the class override in a compiler pass - if (!$container->hasParameter($parameter)) { - throw new RuntimeException(sprintf( - 'The sensio framework extra bundle removed support for the "%s" parameter', - $parameter - )); - } - - $container->setParameter($parameter, AnnotatedRouteControllerLoader::class); - } - /** * The security component disallows definition of firewalls and access_control entries from different files to enforce * security. However this limits our possibilities how to provide a security config for the admin area while making diff --git a/bundles/CoreBundle/EventListener/LegacyTemplateListener.php b/bundles/CoreBundle/EventListener/LegacyTemplateListener.php new file mode 100644 index 00000000000..caeab376e9b --- /dev/null +++ b/bundles/CoreBundle/EventListener/LegacyTemplateListener.php @@ -0,0 +1,120 @@ +templateEngine; + } + + /** + * @param EngineInterface $templateEngine + */ + public function setTemplateEngine(EngineInterface $templateEngine): void + { + $this->templateEngine = $templateEngine; + } + + /** + * @inheritdoc + */ + public function onKernelView(GetResponseForControllerResultEvent $event) + { + + /* @var Template $template */ + $request = $event->getRequest(); + $template = $request->attributes->get('_template'); + + if (!$template instanceof Template) { + return; + } + + + $parameters = $event->getControllerResult(); + $owner = $template->getOwner(); + list($controller, $action) = $owner; + + // when the annotation declares no default vars and the action returns + // null, all action method arguments are used as default vars + if (null === $parameters) { + $parameters = $this->resolveDefaultParameters($request, $template, $controller, $action); + } + + // attempt to render the actual response + $templating = $this->getTemplateEngine(); + + if ($template->isStreamable()) { + $callback = function () use ($templating, $template, $parameters) { + return $templating->stream($template->getTemplate(), $parameters); + }; + + $event->setResponse(new StreamedResponse($callback)); + } + + // make sure the owner (controller+dependencies) is not cached or stored elsewhere + $template->setOwner([]); + + $event->setResponse($templating->renderResponse($template->getTemplate(), $parameters)); + } + + /** + * {@inheritdoc} + */ + public static function getSubscribedEvents() + { + return [ + KernelEvents::CONTROLLER => ['onKernelController', -128], + KernelEvents::VIEW => 'onKernelView', + ]; + } + + private function resolveDefaultParameters(Request $request, Template $template, $controller, $action) + { + $parameters = []; + $arguments = $template->getVars(); + + if (0 === \count($arguments)) { + $r = new \ReflectionObject($controller); + + $arguments = []; + foreach ($r->getMethod($action)->getParameters() as $param) { + $arguments[] = $param; + } + } + + // fetch the arguments of @Template.vars or everything if desired + // and assign them to the designated template + foreach ($arguments as $argument) { + if ($argument instanceof \ReflectionParameter) { + $parameters[$name = $argument->getName()] = !$request->attributes->has($name) && $argument->isDefaultValueAvailable() ? $argument->getDefaultValue() : $request->attributes->get($name); + } else { + $parameters[$argument] = $request->attributes->get($argument); + } + } + + return $parameters; + } +} diff --git a/bundles/CoreBundle/EventListener/TemplateControllerListener.php b/bundles/CoreBundle/EventListener/TemplateControllerListener.php index fb268eaaee0..6da912e80e8 100644 --- a/bundles/CoreBundle/EventListener/TemplateControllerListener.php +++ b/bundles/CoreBundle/EventListener/TemplateControllerListener.php @@ -104,14 +104,7 @@ public function onKernelView(GetResponseForControllerResultEvent $event) $template = new Template([]); $template->setOwner($controller); - $template->setEngine($engine); $templateReference = $guesser->guessTemplateName($controller, $request, $engine); - - // Only AppBundle should use templates inside app folder - if ($templateReference->get('bundle') === 'AppBundle') { - $templateReference->set('bundle', ''); - } - $template->setTemplate($templateReference); // inject Template annotation into the request - will be used by SensioFrameworkExtraBundle diff --git a/bundles/CoreBundle/PimcoreCoreBundle.php b/bundles/CoreBundle/PimcoreCoreBundle.php index 6b8efc095e8..37c8bc0152c 100644 --- a/bundles/CoreBundle/PimcoreCoreBundle.php +++ b/bundles/CoreBundle/PimcoreCoreBundle.php @@ -18,6 +18,7 @@ use Pimcore\Bundle\CoreBundle\DependencyInjection\Compiler\CacheCollectorPass; use Pimcore\Bundle\CoreBundle\DependencyInjection\Compiler\DebugStopwatchPass; use Pimcore\Bundle\CoreBundle\DependencyInjection\Compiler\DoctrineMigrationsParametersPass; +use Pimcore\Bundle\CoreBundle\DependencyInjection\Compiler\LegacyTemplatePass; use Pimcore\Bundle\CoreBundle\DependencyInjection\Compiler\LongRunningHelperPass; use Pimcore\Bundle\CoreBundle\DependencyInjection\Compiler\MonologPsrLogMessageProcessorPass; use Pimcore\Bundle\CoreBundle\DependencyInjection\Compiler\MonologPublicLoggerPass; @@ -27,6 +28,7 @@ use Pimcore\Bundle\CoreBundle\DependencyInjection\Compiler\PimcoreGlobalTemplatingVariablesPass; use Pimcore\Bundle\CoreBundle\DependencyInjection\Compiler\RegisterImageOptimizersPass; use Pimcore\Bundle\CoreBundle\DependencyInjection\Compiler\RegisterMaintenanceTaskPass; +use Pimcore\Bundle\CoreBundle\DependencyInjection\Compiler\RoutingLoaderPass; use Pimcore\Bundle\CoreBundle\DependencyInjection\Compiler\ServiceControllersPass; use Pimcore\Bundle\CoreBundle\DependencyInjection\Compiler\SessionConfiguratorPass; use Pimcore\Bundle\CoreBundle\DependencyInjection\Compiler\TargetingOverrideHandlersPass; @@ -86,5 +88,7 @@ public function build(ContainerBuilder $container) $container->addCompilerPass(new WorkflowPass()); $container->addCompilerPass(new RegisterImageOptimizersPass()); $container->addCompilerPass(new RegisterMaintenanceTaskPass()); + $container->addCompilerPass(new RoutingLoaderPass()); + $container->addCompilerPass(new LegacyTemplatePass()); } } diff --git a/bundles/CoreBundle/Resources/config/pimcore/default.yml b/bundles/CoreBundle/Resources/config/pimcore/default.yml index 1729b64144a..a4916ddbe57 100644 --- a/bundles/CoreBundle/Resources/config/pimcore/default.yml +++ b/bundles/CoreBundle/Resources/config/pimcore/default.yml @@ -499,3 +499,7 @@ presta_sitemap: lastmod: ~ priority: ~ changefreq: ~ + +sensio_framework_extra: + router: + annotations: false diff --git a/bundles/CoreBundle/Templating/LegacyTemplateGuesser.php b/bundles/CoreBundle/Templating/LegacyTemplateGuesser.php new file mode 100644 index 00000000000..43ff00881e0 --- /dev/null +++ b/bundles/CoreBundle/Templating/LegacyTemplateGuesser.php @@ -0,0 +1,146 @@ +kernel = $kernel; + $this->controllerPatterns = $controllerPatterns; + $this->templateEngine = $templateEngine; + + parent::__construct($kernel, $controllerPatterns); + } + + /** + * @inheritdoc + */ + public function guessTemplateName($controller, Request $request, $engine = 'php') + { + //first lookup for new template name(snake_case) + //if not found then use legacy guesser template name(camelCase) + $templateReference = parent::guessTemplateName($controller, $request); + + // Only AppBundle should use templates inside app folder + if (0 === strpos($templateReference,'@') && substr(explode('/',$templateReference)[0],1) === 'App') { + $templateReference = str_replace('@App/','',$templateReference); + } + + //update view file format(not supported by Sensio), if engine is php + if($engine == 'php') { + $templateReference = str_replace('.twig','.php',$templateReference); + } + + if($this->templateEngine->exists($templateReference)) { + return $templateReference; + } + + if (is_object($controller) && method_exists($controller, '__invoke')) { + $controller = array($controller, '__invoke'); + } elseif (!is_array($controller)) { + throw new \InvalidArgumentException(sprintf('First argument of %s must be an array callable or an object defining the magic method __invoke. "%s" given.', __METHOD__, gettype($controller))); + } + $className = $this->getRealClass(\get_class($controller[0])); + + $matchController = null; + foreach ($this->controllerPatterns as $pattern) { + if (preg_match($pattern, $className, $tempMatch)) { + $matchController = $tempMatch; + break; + } + } + if (null === $matchController) { + throw new \InvalidArgumentException(sprintf('The "%s" class does not look like a controller class (its FQN must match one of the following regexps: "%s")', \get_class($controller[0]), implode('", "', $this->controllerPatterns))); + } + + if ($controller[1] === '__invoke') { + $matchAction = $matchController; + $matchController = null; + } elseif (!preg_match('/^(.+)Action$/', $controller[1], $matchAction)) { + $matchAction = array(null, $controller[1]); + } + + $bundle = $this->getBundleForClass($className); + if ($bundle) { + while ($bundleName = $bundle->getName()) { + if (!method_exists($bundle,'getParent') || (null === $parentBundleName = $bundle->getParent())) { + $bundleName = $bundle->getName(); + break; + } + $bundles = $this->kernel->getBundle($parentBundleName, false); + $bundle = array_pop($bundles); + } + } else { + $bundleName = null; + } + + $legacyTemplateReference = new TemplateReference($bundleName, $matchController[1], $matchAction[1], $request->getRequestFormat(), $engine); + + // Only AppBundle should use templates inside app folder + if ($legacyTemplateReference->get('bundle') === 'AppBundle') { + $legacyTemplateReference->set('bundle', ''); + } + + if(!$this->templateEngine->exists($legacyTemplateReference->getLogicalName())) { + throw new \InvalidArgumentException(sprintf('The template "%s" and fallback: "%s" does not exist.',$templateReference, $legacyTemplateReference)); + } + + return $legacyTemplateReference; + } + + /** + * @inheritdoc + */ + protected function getBundleForClass($class) + { + $reflectionClass = new \ReflectionClass($class); + $bundles = $this->kernel->getBundles(); + do { + $namespace = $reflectionClass->getNamespaceName(); + foreach ($bundles as $bundle) { + if (0 === strpos($namespace, $bundle->getNamespace())) { + return $bundle; + } + } + $reflectionClass = $reflectionClass->getParentClass(); + } while ($reflectionClass); + } + + private static function getRealClass(string $class): string + { + if (false === $pos = strrpos($class, '\\'.Proxy::MARKER.'\\')) { + return $class; + } + + return substr($class, $pos + Proxy::MARKER_LENGTH + 2); + } +} diff --git a/composer.json b/composer.json index 985a3a1cbcf..2e200915239 100644 --- a/composer.json +++ b/composer.json @@ -76,7 +76,7 @@ "presta/sitemap-bundle": "^1.5", "ramsey/uuid": "^3.8", "sabre/dav": "^3.2", - "sensio/framework-extra-bundle": "^3.0.2", + "sensio/framework-extra-bundle": "^5.2", "sensio/generator-bundle": "^3.0", "sensiolabs/ansi-to-html": "^1.1", "symfony-cmf/routing-bundle": "^2.0", diff --git a/lib/Controller/Config/ConfigNormalizer.php b/lib/Controller/Config/ConfigNormalizer.php index c99c3d5af0e..fbf95a86ce2 100644 --- a/lib/Controller/Config/ConfigNormalizer.php +++ b/lib/Controller/Config/ConfigNormalizer.php @@ -209,9 +209,7 @@ public function normalizeTemplateName(string $template = null) $parts = explode('/', $template); $template = array_pop($parts); - // ucfirst to match views/Content - TODO should we remove this? $path = implode('/', $parts); - $path = ucfirst($path); } return sprintf('%s/%s', $path, $template); diff --git a/lib/Routing/Loader/AnnotatedRouteControllerLoader.php b/lib/Routing/Loader/AnnotatedRouteControllerLoader.php index 7323f96f089..9f947ab5ba4 100644 --- a/lib/Routing/Loader/AnnotatedRouteControllerLoader.php +++ b/lib/Routing/Loader/AnnotatedRouteControllerLoader.php @@ -14,7 +14,7 @@ namespace Pimcore\Routing\Loader; -use Sensio\Bundle\FrameworkExtraBundle\Routing\AnnotatedRouteControllerLoader as BaseAnnotatedRouteControllerLoader; +use Symfony\Bundle\FrameworkBundle\Routing\AnnotatedRouteControllerLoader as BaseAnnotatedRouteControllerLoader; /** * Normalizes autogenerated admin routes to pimcore_admin_ and pimcore_api_ prefixes diff --git a/lib/Twig/Extension/AssetCompressExtension.php b/lib/Twig/Extension/AssetCompressExtension.php index e2b73b25036..8b1015b59f4 100644 --- a/lib/Twig/Extension/AssetCompressExtension.php +++ b/lib/Twig/Extension/AssetCompressExtension.php @@ -18,8 +18,9 @@ namespace Pimcore\Twig\Extension; use Pimcore\Twig\TokenParser\AssetCompressParser; +use Twig\Extension\AbstractExtension; -class AssetCompressExtension extends \Twig_Extension +class AssetCompressExtension extends AbstractExtension { public function getTokenParsers(): array { diff --git a/lib/Twig/Extension/DocumentTagExtension.php b/lib/Twig/Extension/DocumentTagExtension.php index 49ef6eb8a61..923a84e0bb6 100644 --- a/lib/Twig/Extension/DocumentTagExtension.php +++ b/lib/Twig/Extension/DocumentTagExtension.php @@ -17,8 +17,9 @@ use Pimcore\Model\Document\PageSnippet; use Pimcore\Model\Document\Tag\BlockInterface; use Pimcore\Templating\Renderer\TagRenderer; +use Twig\Extension\AbstractExtension; -class DocumentTagExtension extends \Twig_Extension +class DocumentTagExtension extends AbstractExtension { /** * @var TagRenderer diff --git a/lib/Twig/Extension/DumpExtension.php b/lib/Twig/Extension/DumpExtension.php index e8aa62a21cd..67881c36af3 100644 --- a/lib/Twig/Extension/DumpExtension.php +++ b/lib/Twig/Extension/DumpExtension.php @@ -20,9 +20,10 @@ use Symfony\Component\VarDumper\Cloner\VarCloner; use Symfony\Component\VarDumper\Dumper\AbstractDumper; use Symfony\Component\VarDumper\Dumper\HtmlDumper; +use Twig\Extension\AbstractExtension; use Twig\TwigFunction; -class DumpExtension extends \Twig_Extension +class DumpExtension extends AbstractExtension { public function getFunctions() { diff --git a/lib/Twig/Extension/GlossaryExtension.php b/lib/Twig/Extension/GlossaryExtension.php index ad3a298120a..241689015a9 100644 --- a/lib/Twig/Extension/GlossaryExtension.php +++ b/lib/Twig/Extension/GlossaryExtension.php @@ -19,8 +19,9 @@ use Pimcore\Templating\Helper\Glossary; use Pimcore\Twig\TokenParser\GlossaryTokenParser; +use Twig\Extension\AbstractExtension; -class GlossaryExtension extends \Twig_Extension +class GlossaryExtension extends AbstractExtension { /** * @var Glossary diff --git a/lib/Twig/Extension/HelpersExtension.php b/lib/Twig/Extension/HelpersExtension.php index 4b1992a11d8..cfca9767f6d 100644 --- a/lib/Twig/Extension/HelpersExtension.php +++ b/lib/Twig/Extension/HelpersExtension.php @@ -17,10 +17,12 @@ namespace Pimcore\Twig\Extension; +use Twig\Extension\AbstractExtension; + /** * Simple helpers that do not need a dedicated extension */ -class HelpersExtension extends \Twig_Extension +class HelpersExtension extends AbstractExtension { public function getTests() { diff --git a/lib/Twig/Extension/NavigationExtension.php b/lib/Twig/Extension/NavigationExtension.php index 48207c9ec42..2e974b54988 100644 --- a/lib/Twig/Extension/NavigationExtension.php +++ b/lib/Twig/Extension/NavigationExtension.php @@ -21,8 +21,9 @@ use Pimcore\Navigation\Container; use Pimcore\Navigation\Renderer\RendererInterface; use Pimcore\Templating\Helper\Navigation; +use Twig\Extension\AbstractExtension; -class NavigationExtension extends \Twig_Extension +class NavigationExtension extends AbstractExtension { /** * @var Navigation diff --git a/lib/Twig/Extension/PimcoreObjectExtension.php b/lib/Twig/Extension/PimcoreObjectExtension.php index c4abc609973..b1a6022767e 100644 --- a/lib/Twig/Extension/PimcoreObjectExtension.php +++ b/lib/Twig/Extension/PimcoreObjectExtension.php @@ -21,8 +21,9 @@ use Pimcore\Model\DataObject; use Pimcore\Model\Document; use Pimcore\Model\Site; +use Twig\Extension\AbstractExtension; -class PimcoreObjectExtension extends \Twig_Extension +class PimcoreObjectExtension extends AbstractExtension { public function getFunctions() { diff --git a/lib/Twig/Extension/SubrequestExtension.php b/lib/Twig/Extension/SubrequestExtension.php index 952e15782f3..baaf0f594c5 100644 --- a/lib/Twig/Extension/SubrequestExtension.php +++ b/lib/Twig/Extension/SubrequestExtension.php @@ -16,8 +16,9 @@ use Pimcore\Templating\Helper\Action; use Pimcore\Templating\Helper\Inc; +use Twig\Extension\AbstractExtension; -class SubrequestExtension extends \Twig_Extension +class SubrequestExtension extends AbstractExtension { /** * @var Inc diff --git a/lib/Twig/Extension/TemplatingHelperExtension.php b/lib/Twig/Extension/TemplatingHelperExtension.php index 6f4b5ab5af7..7a29b8392d1 100644 --- a/lib/Twig/Extension/TemplatingHelperExtension.php +++ b/lib/Twig/Extension/TemplatingHelperExtension.php @@ -19,12 +19,13 @@ use Pimcore\Templating\PhpEngine; use Symfony\Component\Templating\Helper\HelperInterface; +use Twig\Extension\AbstractExtension; /** * Delegates calls to PHP templating helpers. Use this only with templating helpers which do not rely * on PHP rendering! */ -class TemplatingHelperExtension extends \Twig_Extension +class TemplatingHelperExtension extends AbstractExtension { /** * @var PhpEngine diff --git a/lib/Twig/Extension/WebsiteConfigExtension.php b/lib/Twig/Extension/WebsiteConfigExtension.php index 6b632a82e1b..ecfec5d95c1 100644 --- a/lib/Twig/Extension/WebsiteConfigExtension.php +++ b/lib/Twig/Extension/WebsiteConfigExtension.php @@ -18,8 +18,9 @@ namespace Pimcore\Twig\Extension; use Pimcore\Config; +use Twig\Extension\AbstractExtension; -class WebsiteConfigExtension extends \Twig_Extension +class WebsiteConfigExtension extends AbstractExtension { public function getFunctions() {