diff --git a/UPGRADE-7.2.md b/UPGRADE-7.2.md index 5a615fb460..1ecfc60cb7 100644 --- a/UPGRADE-7.2.md +++ b/UPGRADE-7.2.md @@ -10,3 +10,10 @@ PagePartBundle -------------- - Not passing a HasPagePartsInterface as second parameter in PagePartEvent is deprecated and will be required in 8.0. + + +NodeBundle +----------------- + +- The node-bundle/multidomain-bundle has now some improved logic in the router itself. Using the old router logic is deprecated and the new will be the default in 8.0. + To enable the new and improved router, set the `kunstmaan_node.enable_improved_router` config to `true`. diff --git a/src/Kunstmaan/MultiDomainBundle/DependencyInjection/Configuration.php b/src/Kunstmaan/MultiDomainBundle/DependencyInjection/Configuration.php index bd52b3d98f..bdcb258e38 100644 --- a/src/Kunstmaan/MultiDomainBundle/DependencyInjection/Configuration.php +++ b/src/Kunstmaan/MultiDomainBundle/DependencyInjection/Configuration.php @@ -52,6 +52,7 @@ public function getConfigTreeBuilder(): TreeBuilder ->end() ->end() ->end() + ->end() ->end() ->end(); diff --git a/src/Kunstmaan/MultiDomainBundle/Router/DomainBasedLocaleRouter.php b/src/Kunstmaan/MultiDomainBundle/Router/DomainBasedLocaleRouter.php index 957b6827a6..08162a0d47 100644 --- a/src/Kunstmaan/MultiDomainBundle/Router/DomainBasedLocaleRouter.php +++ b/src/Kunstmaan/MultiDomainBundle/Router/DomainBasedLocaleRouter.php @@ -14,18 +14,15 @@ class DomainBasedLocaleRouter extends SlugRouter { - /** @var RouteCollection */ - protected $routeCollectionMultiLanguage; - /** - * @var array|null + * @var RouteCollection + * + * @deprecated routeCollectionMultiLanguage property is deprecated in 7.2 and will be removed in 8.0. There is no replacement for this property. */ - private $otherSite; + protected $routeCollectionMultiLanguage; - /** - * @var array - */ - private $cachedNodeTranslations = []; + private ?array $otherSite = null; + private array $cachedNodeTranslations = []; /** * Generate an url for a supplied route @@ -36,7 +33,7 @@ class DomainBasedLocaleRouter extends SlugRouter */ public function generate($name, $parameters = [], $referenceType = UrlGeneratorInterface::ABSOLUTE_PATH): string { - if ('_slug' === $name && $this->isMultiLanguage() && $this->isMultiDomainHost()) { + if ('_slug' === $name && $this->isMultiDomainHost() && $this->isMultiLanguage()) { $locale = isset($parameters['_locale']) ? $parameters['_locale'] : $this->getRequestLocale(); $reverseLocaleMap = $this->getReverseLocaleMap(); @@ -47,8 +44,6 @@ public function generate($name, $parameters = [], $referenceType = UrlGeneratorI if (isset($parameters['otherSite'])) { $this->otherSite = $this->domainConfiguration->getFullHostById($parameters['otherSite']); - } else { - $this->otherSite = null; } $this->urlGenerator = new UrlGenerator( @@ -80,13 +75,8 @@ public function match($pathinfo): array $result = $urlMatcher->match($pathinfo); if (!empty($result)) { // Remap locale for front-end requests - if ($this->isMultiDomainHost() - && $this->isMultiLanguage() - && !$result['preview'] - ) { - $localeMap = $this->getLocaleMap(); - $locale = $result['_locale']; - $result['_locale'] = $localeMap[$locale]; + if (!$result['preview'] && $this->isMultiDomainHost() && $this->isMultiLanguage()) { + $result['_locale'] = $this->getLocaleMap()[$result['_locale']]; } $nodeTranslation = $this->getNodeTranslation($result); @@ -104,13 +94,7 @@ public function match($pathinfo): array */ protected function getRequestLocale() { - $request = $this->getMasterRequest(); - $locale = $this->getDefaultLocale(); - if (!\is_null($request)) { - $locale = $request->getLocale(); - } - - return $locale; + return $this->getMasterRequest()?->getLocale() ?: $this->getDefaultLocale(); } /** @@ -176,15 +160,19 @@ private function getReverseLocaleMap(): array */ public function getRouteCollection(): RouteCollection { - if (($this->otherSite && $this->isMultiLanguage($this->otherSite['host'])) || (!$this->otherSite && $this->isMultiLanguage())) { - if (!$this->routeCollectionMultiLanguage) { - $this->routeCollectionMultiLanguage = new RouteCollection(); + if (!$this->enabledImprovedRouter) { + trigger_deprecation('kunstmaan/multidomain-bundle', '7.2', 'Not enabling the improved router is deprecated and the changed and improved router will be the default in 8.0. Set the "kunstmaan_node.enable_improved_router" config to true.'); - $this->addMultiLangPreviewRoute(); - $this->addMultiLangSlugRoute(); - } + if (($this->otherSite && $this->isMultiLanguage($this->otherSite['host'])) || (!$this->otherSite && $this->isMultiLanguage())) { + if (!$this->routeCollectionMultiLanguage) { + $this->routeCollectionMultiLanguage = new RouteCollection(); - return $this->routeCollectionMultiLanguage; + $this->addMultiLangPreviewRoute(); + $this->addMultiLangSlugRoute(); + } + + return $this->routeCollectionMultiLanguage; + } } if (!$this->routeCollection) { @@ -211,6 +199,8 @@ protected function addSlugRoute() */ protected function addMultiLangPreviewRoute() { + trigger_deprecation('kunstmaan/multidomain-bundle', '7.2', 'This method is deprecated and will be removed in 8.0.'); + $routeParameters = $this->getPreviewRouteParameters(); $this->addMultiLangRoute('_slug_preview', $routeParameters); } @@ -220,6 +210,8 @@ protected function addMultiLangPreviewRoute() */ protected function addMultiLangSlugRoute() { + trigger_deprecation('kunstmaan/multidomain-bundle', '7.2', 'This method is deprecated and will be removed in 8.0.'); + $routeParameters = $this->getSlugRouteParameters(); $this->addMultiLangRoute('_slug', $routeParameters); } @@ -229,6 +221,8 @@ protected function addMultiLangSlugRoute() */ protected function addMultiLangRoute($name, array $parameters = []) { + trigger_deprecation('kunstmaan/multidomain-bundle', '7.2', 'This method is deprecated and will be removed in 8.0.'); + $this->routeCollectionMultiLanguage->add( $name, new Route( @@ -251,26 +245,25 @@ protected function getSlugRouteParameters() '_controller' => SlugController::class . '::slugAction', 'preview' => false, 'url' => '', - '_locale' => $this->getDefaultLocale(), ]; $slugRequirements = [ 'url' => $this->getSlugPattern(), ]; - $locales = []; - // If other site provided and multilingual, get the locales from the host config. + $locales = []; if ($this->otherSite && $this->isMultiLanguage($this->otherSite['host'])) { $locales = $this->getHostLocales(); - } elseif ($this->isMultiLanguage() && !$this->otherSite) { + } elseif (!$this->otherSite && $this->isMultiLanguage()) { $locales = $this->getFrontendLocales(); } - // Make locale availables for the slug routes. + // Make locale available for the slug routes. if (!empty($locales)) { $slugPath = '/{_locale}' . $slugPath; - unset($slugDefaults['_locale']); - $slugRequirements['_locale'] = $this->getEscapedLocales($this->getHostLocales()); + $slugRequirements['_locale'] = $this->getEscapedLocales(!$this->otherSite ? $locales : $this->getHostLocales()); + } else { + $slugDefaults['_locale'] = $this->getDefaultLocale(); } return [ diff --git a/src/Kunstmaan/MultiDomainBundle/Tests/Router/DomainBasedLocaleRouterTest.php b/src/Kunstmaan/MultiDomainBundle/Tests/Router/DomainBasedLocaleRouterTest.php index 7d7f372538..b3993020e2 100644 --- a/src/Kunstmaan/MultiDomainBundle/Tests/Router/DomainBasedLocaleRouterTest.php +++ b/src/Kunstmaan/MultiDomainBundle/Tests/Router/DomainBasedLocaleRouterTest.php @@ -97,6 +97,7 @@ public function testAddMultiLangSlugRoute() $request = $this->getRequest('http://singlelangdomain.tld/'); $object = new DomainBasedLocaleRouter($domainConfiguration, $this->getRequestStack($request), $this->getEntityManager(new NodeTranslation()), 'admin'); + $object->enabledImprovedRouter(true); $mirror = new \ReflectionClass(DomainBasedLocaleRouter::class); $property = $mirror->getProperty('otherSite'); @@ -140,6 +141,7 @@ public function testGetRouteCollection() /** @var Container $container */ $object = new DomainBasedLocaleRouter($domainConfiguration, $this->getRequestStack($request), $this->getEntityManager(new NodeTranslation()), 'admin'); + $object->enabledImprovedRouter(true); $mirror = new \ReflectionClass(DomainBasedLocaleRouter::class); $property = $mirror->getProperty('otherSite'); $property->setAccessible(true); @@ -216,6 +218,9 @@ private function getNodeTranslationRepository($nodeTranslation = null) private function getDomainBasedLocaleRouter($request, $nodeTranslation = null) { - return new DomainBasedLocaleRouter($this->getDomainConfiguration(), $this->getRequestStack($request), $this->getEntityManager($nodeTranslation), 'admin'); + $router = new DomainBasedLocaleRouter($this->getDomainConfiguration(), $this->getRequestStack($request), $this->getEntityManager($nodeTranslation), 'admin'); + $router->enabledImprovedRouter(true); + + return $router; } } diff --git a/src/Kunstmaan/NodeBundle/DependencyInjection/Configuration.php b/src/Kunstmaan/NodeBundle/DependencyInjection/Configuration.php index d75987b283..f68289c1e1 100644 --- a/src/Kunstmaan/NodeBundle/DependencyInjection/Configuration.php +++ b/src/Kunstmaan/NodeBundle/DependencyInjection/Configuration.php @@ -57,6 +57,7 @@ public function getConfigTreeBuilder(): TreeBuilder ->booleanNode('enabled')->defaultFalse()->end() ->end() ->end() + ->booleanNode('enable_improved_router')->defaultFalse()->end() ->end(); return $treeBuilder; diff --git a/src/Kunstmaan/NodeBundle/DependencyInjection/KunstmaanNodeExtension.php b/src/Kunstmaan/NodeBundle/DependencyInjection/KunstmaanNodeExtension.php index 3fcf64c64d..20fa7b0bd1 100644 --- a/src/Kunstmaan/NodeBundle/DependencyInjection/KunstmaanNodeExtension.php +++ b/src/Kunstmaan/NodeBundle/DependencyInjection/KunstmaanNodeExtension.php @@ -42,6 +42,13 @@ public function load(array $configs, ContainerBuilder $container): void $loader->load('services.yml'); $loader->load('commands.yml'); + + $enableImprovedRouter = $config['enable_improved_router'] ?? false; + if (!$enableImprovedRouter) { + trigger_deprecation('kunstmaan/node-bundle', '7.2', 'Not setting the "kunstmaan_node.enable_improved_router" config to true is deprecated, it will always be true in 8.0.'); + } + $slugRouter = $container->findDefinition('kunstmaan_node.slugrouter'); + $slugRouter->addMethodCall('enabledImprovedRouter', [$enableImprovedRouter]); } public function prepend(ContainerBuilder $container): void diff --git a/src/Kunstmaan/NodeBundle/Router/SlugRouter.php b/src/Kunstmaan/NodeBundle/Router/SlugRouter.php index a7ca579f11..260a8c4d72 100644 --- a/src/Kunstmaan/NodeBundle/Router/SlugRouter.php +++ b/src/Kunstmaan/NodeBundle/Router/SlugRouter.php @@ -52,6 +52,13 @@ class SlugRouter implements RouterInterface /** @var string */ protected $slugPattern; + /** + * NEXT_MAJOR: Remove property + * + * @internal + */ + protected bool $enabledImprovedRouter = false; + public function __construct( DomainConfigurationInterface $domainConfiguration, RequestStack $requestStack, @@ -186,7 +193,6 @@ protected function getPreviewRouteParameters() '_controller' => SlugController::class . '::slugAction', 'preview' => true, 'url' => '', - '_locale' => $this->getDefaultLocale(), ]; $previewRequirements = [ 'url' => $this->getSlugPattern(), @@ -194,8 +200,9 @@ protected function getPreviewRouteParameters() if ($this->isMultiLanguage()) { $previewPath = '/{_locale}' . $previewPath; - unset($previewDefaults['_locale']); $previewRequirements['_locale'] = $this->getEscapedLocales($this->getBackendLocales()); + } else { + $previewDefaults['_locale'] = $this->getDefaultLocale(); } return [ @@ -217,7 +224,6 @@ protected function getSlugRouteParameters() '_controller' => SlugController::class . '::slugAction', 'preview' => false, 'url' => '', - '_locale' => $this->getDefaultLocale(), ]; $slugRequirements = [ 'url' => $this->getSlugPattern(), @@ -225,8 +231,9 @@ protected function getSlugRouteParameters() if ($this->isMultiLanguage()) { $slugPath = '/{_locale}' . $slugPath; - unset($slugDefaults['_locale']); $slugRequirements['_locale'] = $this->getEscapedLocales($this->getFrontendLocales()); + } else { + $slugDefaults['_locale'] = $this->getDefaultLocale(); } return [ @@ -345,4 +352,14 @@ protected function getEscapedLocales($locales) return implode('|', $escapedLocales); } + + /** + * NEXT_MAJOR: Remove method + * + * @interal + */ + public function enabledImprovedRouter(bool $enabled): void + { + $this->enabledImprovedRouter = $enabled; + } } diff --git a/src/Kunstmaan/NodeBundle/Tests/DependencyInjection/ConfigurationTest.php b/src/Kunstmaan/NodeBundle/Tests/DependencyInjection/ConfigurationTest.php index 6adef0d66f..997ab156b7 100644 --- a/src/Kunstmaan/NodeBundle/Tests/DependencyInjection/ConfigurationTest.php +++ b/src/Kunstmaan/NodeBundle/Tests/DependencyInjection/ConfigurationTest.php @@ -30,6 +30,7 @@ public function testConfigGeneratesAsExpected() 'threshold' => 35, ], 'enable_permissions' => true, + 'enable_improved_router' => false, ]; $this->assertProcessedConfigurationEquals([$array], $array); diff --git a/src/Kunstmaan/NodeBundle/Tests/DependencyInjection/KunstmaanNodeExtensionTest.php b/src/Kunstmaan/NodeBundle/Tests/DependencyInjection/KunstmaanNodeExtensionTest.php index 761a6afe1d..1d22ac9271 100644 --- a/src/Kunstmaan/NodeBundle/Tests/DependencyInjection/KunstmaanNodeExtensionTest.php +++ b/src/Kunstmaan/NodeBundle/Tests/DependencyInjection/KunstmaanNodeExtensionTest.php @@ -4,10 +4,13 @@ use Kunstmaan\NodeBundle\DependencyInjection\KunstmaanNodeExtension; use Matthias\SymfonyDependencyInjectionTest\PhpUnit\AbstractExtensionTestCase; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\DependencyInjection\Extension\ExtensionInterface; class KunstmaanNodeExtensionTest extends AbstractExtensionTestCase { + use ExpectDeprecationTrait; + /** * @return ExtensionInterface[] */ @@ -19,7 +22,7 @@ protected function getContainerExtensions(): array public function testCorrectParametersHaveBeenSet() { $this->container->setParameter('twig.form.resources', []); - $this->load(); + $this->load(['enable_improved_router' => true]); $this->assertContainerBuilderHasParameter('twig.form.resources'); $this->assertContainerBuilderHasParameter('kunstmaan_node.show_add_homepage', true); @@ -30,4 +33,14 @@ public function testCorrectParametersHaveBeenSet() $this->assertContainerBuilderHasParameter('kunstmaan_node.version_timeout', 3600); $this->assertContainerBuilderHasParameter('kunstmaan_node.url_chooser.lazy_increment', 2); } + + /** + * @group legacy + */ + public function testImprovedRouterConfigDeprecation() + { + $this->expectDeprecation('Since kunstmaan/node-bundle 7.2: Not setting the "kunstmaan_node.enable_improved_router" config to true is deprecated, it will always be true in 8.0.'); + $this->container->setParameter('twig.form.resources', []); + $this->load(); + } }