From db71db1ae56d5b96719539efff449d2fd50faf35 Mon Sep 17 00:00:00 2001 From: Alexey Kravchenko <42177964+kraal-spryker@users.noreply.github.com> Date: Tue, 20 Aug 2024 12:48:48 +0300 Subject: [PATCH] FRW-2465 Fixed performance problem related to ZED/Merchant portal navigation. (#11007) FRW-2465 Fixed performance problem related to ZED/Merchant portal navigation. --- .../Zed/Router/Business/Router/Router.php | 100 +++++++++++- .../Router/Business/RouterBusinessFactory.php | 5 + .../Router/Business/RouterFacadeInterface.php | 40 +++-- src/Spryker/Zed/Router/RouterConfig.php | 20 +++ .../Business/Router/BackofficeRouterTest.php | 148 ++++++++++++++++++ .../Router/_support/Helper/RouterHelper.php | 2 +- .../Router/_support/RouterBusinessTester.php | 47 ++++++ tests/SprykerTest/Zed/Router/codeception.yml | 1 + 8 files changed, 338 insertions(+), 25 deletions(-) create mode 100644 tests/SprykerTest/Zed/Router/Business/Router/BackofficeRouterTest.php diff --git a/src/Spryker/Zed/Router/Business/Router/Router.php b/src/Spryker/Zed/Router/Business/Router/Router.php index e394b02..cc28d4b 100644 --- a/src/Spryker/Zed/Router/Business/Router/Router.php +++ b/src/Spryker/Zed/Router/Business/Router/Router.php @@ -7,6 +7,8 @@ namespace Spryker\Zed\Router\Business\Router; +use Spryker\Zed\Router\Business\Route\Route; +use Spryker\Zed\Router\RouterConfig; use Spryker\Zed\RouterExtension\Dependency\Plugin\RouterEnhancerAwareInterface; use Symfony\Component\Config\ConfigCacheFactory; use Symfony\Component\Config\Loader\LoaderInterface; @@ -14,10 +16,26 @@ use Symfony\Component\Routing\Generator\UrlGeneratorInterface; use Symfony\Component\Routing\Matcher\RequestMatcherInterface; use Symfony\Component\Routing\Matcher\UrlMatcherInterface; +use Symfony\Component\Routing\RouteCollection; use Symfony\Component\Routing\Router as SymfonyRouter; class Router extends SymfonyRouter implements RouterInterface, WarmableInterface { + /** + * @var string + */ + protected const ROUTE_CACHE_PATH = '/url_generating_routes.php'; + + /** + * @var string + */ + protected const CACHE_DIR_PATH_PLACEHOLDER = '%s%s'; + + /** + * @var string + */ + protected const OPTION_CACHE_DIR = 'cache_dir'; + /** * @var array<\Spryker\Zed\RouterExtension\Dependency\Plugin\RouterEnhancerPluginInterface> */ @@ -28,16 +46,33 @@ class Router extends SymfonyRouter implements RouterInterface, WarmableInterface */ protected $configCacheFactory; + /** + * @var \Spryker\Zed\Router\RouterConfig + */ + protected RouterConfig $routerConfig; + + /** + * @var array|null + */ + protected static ?array $cache = []; + /** * @param \Symfony\Component\Config\Loader\LoaderInterface $loader * @param mixed $resource + * @param \Spryker\Zed\Router\RouterConfig $routerConfig * @param array<\Spryker\Zed\RouterExtension\Dependency\Plugin\RouterEnhancerPluginInterface> $routerEnhancerPlugins * @param array $options */ - public function __construct(LoaderInterface $loader, $resource, array $routerEnhancerPlugins, array $options = []) - { + public function __construct( + LoaderInterface $loader, + $resource, + RouterConfig $routerConfig, + array $routerEnhancerPlugins, + array $options = [] + ) { parent::__construct($loader, $resource, $options); + $this->routerConfig = $routerConfig; $this->routerEnhancerPlugins = $routerEnhancerPlugins; } @@ -57,6 +92,67 @@ public function getMatcher(): UrlMatcherInterface|RequestMatcherInterface return $this->matcher; } + /** + * @return \Symfony\Component\Routing\RouteCollection + */ + public function getRouteCollection(): RouteCollection + { + $cachePath = sprintf(static::CACHE_DIR_PATH_PLACEHOLDER, $this->options[static::OPTION_CACHE_DIR], static::ROUTE_CACHE_PATH); + if (!$this->routerConfig->isRoutingCacheEnabled() || !file_exists($cachePath)) { + return parent::getRouteCollection(); + } + + $routes = static::getSymfonyCompiledRoutes($cachePath); + + return $this->mapRouteDataArrayCollectionToRouteCollection($routes); + } + + /** + * @param array $route + * + * @return \Spryker\Zed\Router\Business\Route\Route + */ + protected function mapRouteDataArrayToRouteObject(array $route): Route + { + [, $defaults, , $pathProperties] = $route; + [, $path] = array_pop($pathProperties); + $routeObject = new Route($path); + $routeObject->addDefaults($defaults); + + return $routeObject; + } + + /** + * @param array $routes + * + * @return \Symfony\Component\Routing\RouteCollection + */ + protected function mapRouteDataArrayCollectionToRouteCollection(array $routes): RouteCollection + { + $routeCollection = new RouteCollection(); + + foreach ($routes as $name => $route) { + $routeObject = $this->mapRouteDataArrayToRouteObject($route); + $routeCollection->add($name, $routeObject); + } + + return $routeCollection; + } + + /** + * @param string $path + * + * @return array + */ + public static function getSymfonyCompiledRoutes(string $path): array + { + if (!isset(static::$cache[$path])) { + static::$cache[$path] = require $path; + } + + return static::$cache[$path] ??= require $path; + } + /** * @param \Symfony\Component\Routing\Matcher\UrlMatcherInterface $matcher * diff --git a/src/Spryker/Zed/Router/Business/RouterBusinessFactory.php b/src/Spryker/Zed/Router/Business/RouterBusinessFactory.php index 0b3a001..e9bcec4 100644 --- a/src/Spryker/Zed/Router/Business/RouterBusinessFactory.php +++ b/src/Spryker/Zed/Router/Business/RouterBusinessFactory.php @@ -71,6 +71,7 @@ public function createBackofficeRouter(): RouterInterface return new Router( $this->createClosureLoader(), $this->createBackofficeRouterResource(), + $this->getConfig(), $this->getBackofficeRouterEnhancerPlugins(), $this->getConfig()->getBackofficeRouterConfiguration(), ); @@ -84,6 +85,7 @@ public function createMerchantPortalRouter(): RouterInterface return new Router( $this->createClosureLoader(), $this->createMerchantPortalRouterResource(), + $this->getConfig(), $this->getMerchantPortalRouterEnhancerPlugins(), $this->getConfig()->getMerchantPortalRouterConfiguration(), ); @@ -153,6 +155,7 @@ public function createBackendGatewayRouter(): RouterInterface return new Router( $this->createClosureLoader(), $this->createBackendGatewayRouterResource(), + $this->getConfig(), $this->getBackendGatewayRouterEnhancerPlugins(), $this->getConfig()->getBackendGatewayRouterConfiguration(), ); @@ -256,6 +259,7 @@ public function createZedRouter(): RouterInterface return new Router( $this->createClosureLoader(), $this->createResource(), + $this->getConfig(), $this->getRouterEnhancerPlugins(), $this->getConfig()->getRouterConfiguration(), ); @@ -277,6 +281,7 @@ public function createZedDevelopmentRouter(): RouterInterface return new Router( $this->createClosureLoader(), $this->createResource(), + $this->getConfig(), $this->getRouterEnhancerPlugins(), $this->getConfig()->getDevelopmentRouterConfiguration(), ); diff --git a/src/Spryker/Zed/Router/Business/RouterFacadeInterface.php b/src/Spryker/Zed/Router/Business/RouterFacadeInterface.php index 5bdd978..f647af8 100644 --- a/src/Spryker/Zed/Router/Business/RouterFacadeInterface.php +++ b/src/Spryker/Zed/Router/Business/RouterFacadeInterface.php @@ -16,14 +16,14 @@ interface RouterFacadeInterface { /** + * Specification: + * - Returns a ChainRouter which is added to the Application. + * - Executes {@link \Spryker\Zed\RouterExtension\Dependency\Plugin\RouterPluginInterface} plugin stack to add Router to the ChainRouter. + * * @api * * @internal * - * Specification: - * - Returns a ChainRouter which is added to the Application. - * - Uses RouterExtensionPluginInterfaces to add Router to the ChainRouter. - * * @return \Spryker\Zed\Router\Business\Router\ChainRouter */ public function getBackofficeChainRouter(): ChainRouter; @@ -53,14 +53,14 @@ public function getMerchantPortalChainRouter(): ChainRouter; public function getBackofficeRouter(): RouterInterface; /** + * Specification: + * - Returns a ChainRouter which is added to the BackendGateway Application. + * - Executes {@link \Spryker\Zed\RouterExtension\Dependency\Plugin\RouterPluginInterface} plugin stack to add Router to the ChainRouter. + * * @api * * @internal * - * Specification: - * - Returns a ChainRouter which is added to the BackendGateway Application. - * - Uses RouterExtensionPluginInterfaces to add Router to the ChainRouter. - * * @return \Spryker\Zed\Router\Business\Router\ChainRouter */ public function getBackendGatewayChainRouter(): ChainRouter; @@ -78,10 +78,6 @@ public function getBackendGatewayChainRouter(): ChainRouter; public function getMerchantPortalRouter(): RouterInterface; /** - * @api - * - * @internal - * * Specification: * - Returns Router which handles BackendGateway routes. * @@ -96,7 +92,7 @@ public function getBackendGatewayRouter(): RouterInterface; /** * Specification: * - Returns a ChainRouter which is added to the BackendApi Application. - * - Uses RouterExtensionPluginInterfaces to add Router to the ChainRouter. + * - Executes {@link \Spryker\Zed\RouterExtension\Dependency\Plugin\RouterPluginInterface} plugin stack to add Router to the ChainRouter. * * @api * @@ -137,26 +133,26 @@ public function warmUpMerchantPortalRouterCache(): void; public function warmUpBackendGatewayRouterCache(): void; /** + * Specification: + * - Returns Router which handles Zed routes. + * * @api * * @internal * - * Specification: - * - Returns Router which handles Zed routes. - * * @return \Spryker\Zed\Router\Business\Router\RouterInterface */ public function getZedRouter(): RouterInterface; /** + * Specification: + * - Returns a ChainRouter which is added to the Application. + * - Executes {@link \Spryker\Zed\RouterExtension\Dependency\Plugin\RouterPluginInterface} plugin stack to add Router to the ChainRouter. + * * @api * * @internal * - * Specification: - * - Returns a ChainRouter which is added to the Application. - * - Uses RouterExtensionPluginInterfaces to add Router to the ChainRouter. - * * @return \Spryker\Zed\Router\Business\Router\ChainRouter */ public function getRouter(): ChainRouter; @@ -174,11 +170,11 @@ public function getRouter(): ChainRouter; public function getZedFallbackRouter(): RouterInterface; /** - * @api - * * Specification: * - Builds the cache for the Router. * + * @api + * * @return void */ public function cacheWarmUp(): void; diff --git a/src/Spryker/Zed/Router/RouterConfig.php b/src/Spryker/Zed/Router/RouterConfig.php index 8b63983..f03e8e5 100644 --- a/src/Spryker/Zed/Router/RouterConfig.php +++ b/src/Spryker/Zed/Router/RouterConfig.php @@ -23,6 +23,11 @@ class RouterConfig extends AbstractBundleConfig */ protected const DIRECTORY_NAME_MERCHANT_PORTAL_APPLICATION = 'MerchantPortalApplication'; + /** + * @var bool + */ + protected const IS_ROUTING_CACHE_ENABLED = false; + /** * Specification: * - Returns a Router configuration which makes use of a Router cache. @@ -308,4 +313,19 @@ public function getNotAllowedBackofficeControllerDirectories(): array static::DIRECTORY_NAME_MERCHANT_PORTAL_APPLICATION, ]; } + + /** + * Specification: + * - Returns whether the Router cache is enabled. + * + * @api + * + * @deprecated Cache will be enabled by default in next major. + * + * @return bool + */ + public function isRoutingCacheEnabled(): bool + { + return static::IS_ROUTING_CACHE_ENABLED; + } } diff --git a/tests/SprykerTest/Zed/Router/Business/Router/BackofficeRouterTest.php b/tests/SprykerTest/Zed/Router/Business/Router/BackofficeRouterTest.php new file mode 100644 index 0000000..2e497d3 --- /dev/null +++ b/tests/SprykerTest/Zed/Router/Business/Router/BackofficeRouterTest.php @@ -0,0 +1,148 @@ +tester->cleanCache(); + } + + /** + * @return void + */ + public function testGetBackofficeRouteCollectionReturnsRoutesFromLoaderWhenCacheDirIsNull(): void + { + // Arrange + $options = [static::OPTION_CACHE_DIR => null]; + $routeCollection = new RouteCollection(); + $routeCollection->add('test1', new Route('test1')); + $router = $this->mockDependencies($routeCollection, $options); + + // Act + $routes = $router->getRouteCollection()->all(); + + // Assert + $this->tester->assertIsArray($routes); + $this->tester->assertNotEmpty($routes); + $this->tester->assertArrayHasKey('test1', $routes); + } + + /** + * @return void + */ + public function testGetBackofficeRouteCollectionRetrievesRoutesFromCachePath(): void + { + // Arrange + $options = [static::OPTION_CACHE_DIR => $this->tester->getCacheDir()]; + + $routeCollection = new RouteCollection(); + $routeCollection->add('test1', new Route('test1')); + $router = $this->mockDependencies($routeCollection, $options, true); + + // Act + $generatedRoutes = $router->getRouteCollection()->all(); + + // Assert + $this->tester->assertIsArray($generatedRoutes); + $this->tester->assertArrayHasKey('test1', $generatedRoutes); + } + + /** + * @return void + */ + public function testGetBackofficeRouteCollectionHandlesEmptyRouteCollectionGracefully(): void + { + // Arrange + $options = [static::OPTION_CACHE_DIR => $this->tester->getCacheDir()]; + + $routeCollection = new RouteCollection(); + $router = $this->mockDependencies($routeCollection, $options, true); + + // Act + $generatedRoutes = $router->getRouteCollection()->all(); + + // Assert + $this->assertIsArray($generatedRoutes); + $this->assertEmpty($generatedRoutes); + } + + /** + * @param \Spryker\Zed\Router\Business\Route\RouteCollection $routeCollection + * @param array $options + * @param bool $withCache + * + * @return \Spryker\Zed\Router\Business\Router\Router + */ + protected function mockDependencies( + RouteCollection $routeCollection, + array $options = [], + bool $withCache = false + ): Router { + $loader = $this->getMockBuilder(LoaderInterface::class) + ->disableOriginalConstructor() + ->getMock(); + $loader->method('load') + ->willReturn($routeCollection); + /** @var \Spryker\Zed\Router\RouterConfig $configMock */ + $configMock = $this->tester->getModuleConfig(); + $router = new Router($loader, 'resource', $configMock, [], $options); + + if ($withCache) { + $configCacheFactory = $this->createMock(ConfigCacheFactory::class); + $configCache = $this->createMock(ConfigCacheInterface::class); + $cacheFile = $this->tester->getCacheFileName(); + + $configCache->method('getPath')->willReturn($cacheFile); + $configCacheFactory->method('cache')->willReturn($configCache); + $router->setConfigCacheFactory($configCacheFactory); + } + + return $router; + } +} diff --git a/tests/SprykerTest/Zed/Router/_support/Helper/RouterHelper.php b/tests/SprykerTest/Zed/Router/_support/Helper/RouterHelper.php index 331f357..eb5ca1e 100644 --- a/tests/SprykerTest/Zed/Router/_support/Helper/RouterHelper.php +++ b/tests/SprykerTest/Zed/Router/_support/Helper/RouterHelper.php @@ -164,7 +164,7 @@ public function addRoute( $resource = function () { return $this->getRouteCollection(); }; - $router = new Router($loader, $resource, []); + $router = new Router($loader, $resource, $this->getConfig(), []); $chainRouter->add($router); } diff --git a/tests/SprykerTest/Zed/Router/_support/RouterBusinessTester.php b/tests/SprykerTest/Zed/Router/_support/RouterBusinessTester.php index fc9b2eb..323f927 100644 --- a/tests/SprykerTest/Zed/Router/_support/RouterBusinessTester.php +++ b/tests/SprykerTest/Zed/Router/_support/RouterBusinessTester.php @@ -26,4 +26,51 @@ class RouterBusinessTester extends Actor { use _generated\RouterBusinessTesterActions; + + /** + * @var int + */ + protected const MODULE_ROOT_DIRECTORY_LEVEL = 5; + + /** + * @var string + */ + protected const CACHE_DIR = '/tests/_data/cache/'; + + /** + * @var string + */ + protected const CACHE_FILE = 'url_generating_routes.php'; + + /** + * @return void + */ + public function cleanCache(): void + { + $dirName = $this->getCacheDir(); + if (file_exists($dirName . static::CACHE_FILE)) { + array_map('unlink', glob("$dirName/*.*", GLOB_NOSORT)); + rmdir($dirName); + } + } + + /** + * @return string + */ + public function getCacheDir(): string + { + $moduleRoot = realpath( + dirname(__DIR__, static::MODULE_ROOT_DIRECTORY_LEVEL), + ); + + return $moduleRoot . static::CACHE_DIR; + } + + /** + * @return string + */ + public function getCacheFileName(): string + { + return $this->getCacheDir() . static::CACHE_FILE; + } } diff --git a/tests/SprykerTest/Zed/Router/codeception.yml b/tests/SprykerTest/Zed/Router/codeception.yml index b578a07..99fb32c 100644 --- a/tests/SprykerTest/Zed/Router/codeception.yml +++ b/tests/SprykerTest/Zed/Router/codeception.yml @@ -27,6 +27,7 @@ suites: enabled: - Asserts - \SprykerTest\Shared\Testify\Helper\ConfigHelper + - \SprykerTest\Shared\Testify\Helper\LocatorHelper Communication: path: ./Communication