From 60e00aa36fe7c9645e9f648f242b109a6e476a0d Mon Sep 17 00:00:00 2001 From: bwaidelich Date: Wed, 16 Mar 2022 17:53:13 +0100 Subject: [PATCH] squashed commit --- .../Mvc/Routing/Dto/UriConstraints.php | 16 --- .../Mvc/Routing/RouterCachingService.php | 119 +++++------------- .../Tests/Unit/Mvc/Routing/RouteTest.php | 6 +- .../Mvc/Routing/RouterCachingServiceTest.php | 50 ++++---- 4 files changed, 57 insertions(+), 134 deletions(-) diff --git a/Neos.Flow/Classes/Mvc/Routing/Dto/UriConstraints.php b/Neos.Flow/Classes/Mvc/Routing/Dto/UriConstraints.php index ed685ab30d..6d9e04ec59 100644 --- a/Neos.Flow/Classes/Mvc/Routing/Dto/UriConstraints.php +++ b/Neos.Flow/Classes/Mvc/Routing/Dto/UriConstraints.php @@ -284,22 +284,6 @@ public function withPathSuffix(string $pathSuffix, bool $prepend = false): self return new static($newConstraints); } - /** - * Returns the URI path constraint, which consists of the path and query string parts, or NULL if none was set - * - * @return string|null - * @deprecated With Flow 7.0, use toUri()->getPath() and/or toUri()->getQuery() instead. @see toUri() - */ - public function getPathConstraint(): ?string - { - $pathPart = $this->constraints[self::CONSTRAINT_PATH] ?? null; - $queryPart = $this->constraints[self::CONSTRAINT_QUERY_STRING] ?? null; - if ($pathPart === null && $queryPart === null) { - return null; - } - return $pathPart . ($queryPart ? '?' . $queryPart : ''); - } - /** * Applies all constraints of this instance to the given $templateUri and returns a new UriInterface instance * satisfying all of the constraints (see example above) diff --git a/Neos.Flow/Classes/Mvc/Routing/RouterCachingService.php b/Neos.Flow/Classes/Mvc/Routing/RouterCachingService.php index 9dc80254fc..6d43d7ff3a 100644 --- a/Neos.Flow/Classes/Mvc/Routing/RouterCachingService.php +++ b/Neos.Flow/Classes/Mvc/Routing/RouterCachingService.php @@ -11,6 +11,7 @@ * source code. */ +use Neos\Cache\Exception as CacheException; use Neos\Flow\Annotations as Flow; use Neos\Cache\CacheAwareInterface; use Neos\Cache\Frontend\VariableFrontend; @@ -22,7 +23,6 @@ use Neos\Flow\ObjectManagement\ObjectManagerInterface; use Neos\Flow\Persistence\PersistenceManagerInterface; use Neos\Utility\Arrays; -use Neos\Flow\Validation\Validator\UuidValidator; use Psr\Log\LoggerInterface; /** @@ -33,26 +33,26 @@ class RouterCachingService { /** - * @var VariableFrontend * @Flow\Inject + * @var VariableFrontend */ protected $routeCache; /** - * @var VariableFrontend * @Flow\Inject + * @var VariableFrontend */ protected $resolveCache; /** - * @var PersistenceManagerInterface * @Flow\Inject + * @var PersistenceManagerInterface */ protected $persistenceManager; /** - * @var LoggerInterface * @Flow\Inject(name="Neos.Flow:SystemLogger") + * @var LoggerInterface */ protected $logger; @@ -69,17 +69,9 @@ class RouterCachingService protected $routingSettings; /** - * @param LoggerInterface $logger - */ - public function injectLogger(LoggerInterface $logger) - { - $this->logger = $logger; - } - - /** - * @return void + * @throws CacheException */ - public function initializeObject() + public function initializeObject(): void { // flush routing caches if in Development context & routing settings changed if ($this->objectManager->getContext()->isDevelopment() && $this->routeCache->get('routingSettings') !== $this->routingSettings) { @@ -91,7 +83,6 @@ public function initializeObject() /** * Checks the cache for the given RouteContext and returns the result or false if no matching ache entry was found * - * @param RouteContext $routeContext * @return array|boolean the cached route values or false if no cache entry was found */ public function getCachedMatchResults(RouteContext $routeContext) @@ -107,18 +98,14 @@ public function getCachedMatchResults(RouteContext $routeContext) /** * Stores the $matchResults in the cache * - * @param RouteContext $routeContext - * @param array $matchResults - * @param RouteTags|null $matchedTags - * @return void + * @throws CacheException */ - public function storeMatchResults(RouteContext $routeContext, array $matchResults, RouteTags $matchedTags = null) + public function storeMatchResults(RouteContext $routeContext, array $matchResults, RouteTags $matchedTags = null): void { if ($this->containsObject($matchResults)) { return; } - - $tags = $this->generateRouteTags(RequestInformationHelper::getRelativeRequestPath($routeContext->getHttpRequest()), $matchResults); + $tags = $this->generateRouteTagsFromUriPath(RequestInformationHelper::getRelativeRequestPath($routeContext->getHttpRequest())); if ($matchedTags !== null) { $tags = array_unique(array_merge($matchedTags->getTags(), $tags)); } @@ -128,8 +115,7 @@ public function storeMatchResults(RouteContext $routeContext, array $matchResult /** * Checks the cache for the given ResolveContext and returns the cached UriConstraints if a cache entry is found * - * @param ResolveContext $resolveContext - * @return UriConstraints|boolean the cached URI or false if no cache entry was found + * @return UriConstraints|bool the cached URI or false if no cache entry was found */ public function getCachedResolvedUriConstraints(ResolveContext $resolveContext) { @@ -142,13 +128,9 @@ public function getCachedResolvedUriConstraints(ResolveContext $resolveContext) /** * Stores the resolved UriConstraints in the cache together with the $routeValues - * - * @param ResolveContext $resolveContext - * @param UriConstraints $uriConstraints - * @param RouteTags|null $resolvedTags - * @return void + * @throws CacheException */ - public function storeResolvedUriConstraints(ResolveContext $resolveContext, UriConstraints $uriConstraints, RouteTags $resolvedTags = null) + public function storeResolvedUriConstraints(ResolveContext $resolveContext, UriConstraints $uriConstraints, RouteTags $resolvedTags = null): void { $routeValues = $this->convertObjectsToHashes($resolveContext->getRouteValues()); if ($routeValues === null) { @@ -156,39 +138,37 @@ public function storeResolvedUriConstraints(ResolveContext $resolveContext, UriC } $cacheIdentifier = $this->buildResolveCacheIdentifier($resolveContext, $routeValues); - $tags = $this->generateRouteTags((string)$uriConstraints->toUri(), $routeValues); + $tags = $this->generateRouteTagsFromUriPath((string)$uriConstraints->toUri()); if ($resolvedTags !== null) { $tags = array_unique(array_merge($resolvedTags->getTags(), $tags)); } $this->resolveCache->set($cacheIdentifier, $uriConstraints, $tags); } - /** - * @param string $uriPath - * @param array $routeValues - * @return array - */ - protected function generateRouteTags($uriPath, $routeValues) + private function generateRouteTagsFromUriPath(string $uriPath): array { - $uriPath = trim($uriPath, '/'); - $tags = $this->extractUuids($routeValues); + $tags = []; $path = ''; - $uriPath = explode('/', $uriPath); - foreach ($uriPath as $uriPathSegment) { + foreach (explode('/', trim($uriPath, '/')) as $uriPathSegment) { $path .= '/' . $uriPathSegment; $path = trim($path, '/'); $tags[] = md5($path); } - return $tags; } + /** + * @deprecated with Flow 8.0 - This method is no longer used! It is just kept in order to keep AOP aspects from failing + */ + protected function generateRouteTags($uriPath, $routeValues) + { + return []; + } + /** * Flushes 'route' and 'resolve' caches. - * - * @return void */ - public function flushCaches() + public function flushCaches(): void { $this->routeCache->flush(); $this->resolveCache->flush(); @@ -196,11 +176,8 @@ public function flushCaches() /** * Flushes 'findMatchResults' and 'resolve' caches for the given $tag - * - * @param string $tag - * @return void */ - public function flushCachesByTag($tag) + public function flushCachesByTag(string $tag): void { $this->routeCache->flushByTag($tag); $this->resolveCache->flushByTag($tag); @@ -208,11 +185,8 @@ public function flushCachesByTag($tag) /** * Flushes 'findMatchResults' caches that are tagged with the given $uriPath - * - * @param string $uriPath - * @return void */ - public function flushCachesForUriPath($uriPath) + public function flushCachesForUriPath(string $uriPath): void { $uriPathTag = md5(trim($uriPath, '/')); $this->flushCachesByTag($uriPathTag); @@ -221,10 +195,9 @@ public function flushCachesForUriPath($uriPath) /** * Checks if the given subject contains an object * - * @param mixed $subject - * @return boolean true if $subject contains an object, otherwise false + * @return bool true if $subject contains an object, otherwise false */ - protected function containsObject($subject) + private function containsObject($subject): bool { if (is_object($subject)) { return true; @@ -244,9 +217,9 @@ protected function containsObject($subject) * Recursively converts objects in an array to their identifiers * * @param array $routeValues the array to be processed - * @return array the modified array or NULL if $routeValues contain an object and its identifier could not be determined + * @return array|null the modified array or NULL if $routeValues contain an object and its identifier could not be determined */ - protected function convertObjectsToHashes(array $routeValues) + private function convertObjectsToHashes(array $routeValues): ?array { foreach ($routeValues as &$value) { if (is_object($value)) { @@ -271,37 +244,11 @@ protected function convertObjectsToHashes(array $routeValues) /** * Generates the Resolve cache identifier for the given Request - * - * @param ResolveContext $resolveContext - * @param array $routeValues - * @return string */ - protected function buildResolveCacheIdentifier(ResolveContext $resolveContext, array $routeValues) + private function buildResolveCacheIdentifier(ResolveContext $resolveContext, array $routeValues): string { Arrays::sortKeysRecursively($routeValues); return md5(sprintf('abs:%s|prefix:%s|routeValues:%s', $resolveContext->isForceAbsoluteUri() ? 1 : 0, $resolveContext->getUriPathPrefix(), trim(http_build_query($routeValues), '/'))); } - - /** - * Helper method to generate tags by taking all UUIDs contained - * in the given $routeValues or $matchResults - * - * @param array $values - * @return array - */ - protected function extractUuids(array $values) - { - $uuids = []; - foreach ($values as $value) { - if (is_string($value)) { - if (preg_match(UuidValidator::PATTERN_MATCH_UUID, $value) !== 0) { - $uuids[] = $value; - } - } elseif (is_array($value)) { - $uuids = array_merge($uuids, $this->extractUuids($value)); - } - } - return $uuids; - } } diff --git a/Neos.Flow/Tests/Unit/Mvc/Routing/RouteTest.php b/Neos.Flow/Tests/Unit/Mvc/Routing/RouteTest.php index 829242c16b..8185f5db85 100644 --- a/Neos.Flow/Tests/Unit/Mvc/Routing/RouteTest.php +++ b/Neos.Flow/Tests/Unit/Mvc/Routing/RouteTest.php @@ -919,8 +919,8 @@ public function resolvesAppendsDefaultValuesOfOptionalUriPartsToResolvedUriPathC $this->routeValues = ['baz' => 'bazValue']; $this->resolveRouteValues($this->routeValues); - $expectedResult = 'foo/barDefaultValue/bazvalue'; - $actualResult = $this->route->getResolvedUriConstraints()->getPathConstraint(); + $expectedResult = '/foo/barDefaultValue/bazvalue'; + $actualResult = $this->route->getResolvedUriConstraints()->toUri()->getPath(); self::assertSame($expectedResult, $actualResult); } @@ -973,7 +973,7 @@ public function resolvedUriConstraintsIsEmptyAfterUnsuccessfulResolve() $this->routeValues = ['differentKey' => 'value1']; self::assertFalse($this->resolveRouteValues($this->routeValues)); - self::assertNull($this->route->getResolvedUriConstraints()->getPathConstraint()); + self::assertSame('/', $this->route->getResolvedUriConstraints()->toUri()->getPath()); } /** diff --git a/Neos.Flow/Tests/Unit/Mvc/Routing/RouterCachingServiceTest.php b/Neos.Flow/Tests/Unit/Mvc/Routing/RouterCachingServiceTest.php index f278196770..03ef189638 100644 --- a/Neos.Flow/Tests/Unit/Mvc/Routing/RouterCachingServiceTest.php +++ b/Neos.Flow/Tests/Unit/Mvc/Routing/RouterCachingServiceTest.php @@ -84,7 +84,7 @@ class RouterCachingServiceTest extends UnitTestCase */ protected function setUp(): void { - $this->routerCachingService = $this->getAccessibleMock(RouterCachingService::class, ['dummy']); + $this->routerCachingService = new RouterCachingService(); $this->mockRouteCache = $this->getMockBuilder(VariableFrontend::class)->disableOriginalConstructor()->getMock(); $this->inject($this->routerCachingService, 'routeCache', $this->mockRouteCache); @@ -121,7 +121,7 @@ public function initializeObjectDoesNotFlushCachesInProductionContext() $this->mockRouteCache->expects(self::never())->method('flush'); $this->mockResolveCache->expects(self::never())->method('flush'); - $this->routerCachingService->_call('initializeObject'); + $this->routerCachingService->initializeObject(); } /** @@ -141,7 +141,7 @@ public function initializeDoesNotFlushCachesInDevelopmentContextIfRoutingSetting $this->mockRouteCache->expects(self::never())->method('flush'); $this->mockResolveCache->expects(self::never())->method('flush'); - $this->routerCachingService->_call('initializeObject'); + $this->routerCachingService->initializeObject(); } /** @@ -162,7 +162,7 @@ public function initializeFlushesCachesInDevelopmentContextIfRoutingSettingsHave $this->mockRouteCache->expects(self::once())->method('flush'); $this->mockResolveCache->expects(self::once())->method('flush'); - $this->routerCachingService->_call('initializeObject'); + $this->routerCachingService->initializeObject(); } /** @@ -176,34 +176,31 @@ public function initializeFlushesCachesInDevelopmentContextIfRoutingSettingsWher $this->mockRouteCache->expects(self::once())->method('flush'); $this->mockResolveCache->expects(self::once())->method('flush'); - $this->routerCachingService->_call('initializeObject'); + $this->routerCachingService->initializeObject(); } /** - * Data provider for containsObjectDetectsObjectsInVariousSituations() + * Data provider for storeMatchResultsSkipsResultsWithObjects() */ - public function containsObjectDetectsObjectsInVariousSituationsDataProvider() + public function storeMatchResultsSkipsResultsWithObjectsDataProvider() { $object = new \stdClass(); return [ - [true, $object], - [true, ['foo' => $object]], - [true, ['foo' => 'bar', 'baz' => $object]], - [true, ['foo' => ['bar' => ['baz' => 'quux', 'here' => $object]]]], - [false, 'no object'], - [false, ['foo' => 'no object']], - [false, true] + [[$object]], + [['foo' => $object]], + [['foo' => 'bar', 'baz' => $object]], + [['foo' => ['bar' => ['baz' => 'quux', 'here' => $object]]]], ]; } /** - * @dataProvider containsObjectDetectsObjectsInVariousSituationsDataProvider() + * @dataProvider storeMatchResultsSkipsResultsWithObjectsDataProvider() * @test */ - public function containsObjectDetectsObjectsInVariousSituations($expectedResult, $subject) + public function storeMatchResultsSkipsResultsWithObjects(array $matchResults) { - $actualResult = $this->routerCachingService->_call('containsObject', $subject); - self::assertSame($expectedResult, $actualResult); + $this->mockRouteCache->expects(self::never())->method('set'); + $this->routerCachingService->storeMatchResults(new RouteContext($this->mockHttpRequest, RouteParameters::createEmpty()), $matchResults); } /** @@ -247,14 +244,14 @@ public function storeMatchResultsDoesNotStoreMatchResultsInCacheIfTheyContainObj /** * @test */ - public function storeMatchExtractsUuidsAndTheHashedUriPathToCacheTags() + public function storeMatchExtractsTheHashedUriPathToCacheTags() { $uuid1 = '550e8400-e29b-11d4-a716-446655440000'; $uuid2 = '302abe9c-7d07-4200-a868-478586019290'; $matchResults = ['some' => ['matchResults' => ['uuid', $uuid1]], 'foo' => $uuid2]; $routeContext = new RouteContext($this->mockHttpRequest, RouteParameters::createEmpty()); - $this->mockRouteCache->expects(self::once())->method('set')->with($routeContext->getCacheEntryIdentifier(), $matchResults, [$uuid1, $uuid2, md5('some'), md5('some/route'), md5('some/route/path')]); + $this->mockRouteCache->expects(self::once())->method('set')->with($routeContext->getCacheEntryIdentifier(), $matchResults, [md5('some'), md5('some/route'), md5('some/route/path')]); $this->routerCachingService->storeMatchResults($routeContext, $matchResults); } @@ -303,7 +300,7 @@ public function storeResolvedUriConstraintsConvertsObjectsToHashesToGenerateRout $this->mockPersistenceManager->expects(self::once())->method('getIdentifierByObject')->with($mockObject)->will(self::returnValue($mockUuid)); $resolvedUriConstraints = UriConstraints::create()->withPath('path'); - $this->mockResolveCache->expects(self::once())->method('set')->with($cacheIdentifier, $resolvedUriConstraints, [$mockUuid, md5('path')]); + $this->mockResolveCache->expects(self::once())->method('set')->with($cacheIdentifier, $resolvedUriConstraints, [md5('path')]); $this->routerCachingService->storeResolvedUriConstraints(new ResolveContext($this->mockUri, $routeValues, false, '', RouteParameters::createEmpty()), $resolvedUriConstraints); } @@ -311,7 +308,7 @@ public function storeResolvedUriConstraintsConvertsObjectsToHashesToGenerateRout /** * @test */ - public function storeResolvedUriConstraintsExtractsUuidsToCacheTags() + public function storeResolvedUriConstraintsExtractsPathHashesToCacheTags() { $uuid1 = '550e8400-e29b-11d4-a716-446655440000'; $uuid2 = '302abe9c-7d07-4200-a868-478586019290'; @@ -319,14 +316,9 @@ public function storeResolvedUriConstraintsExtractsUuidsToCacheTags() $resolveContext = new ResolveContext($this->mockUri, $routeValues, false, '', RouteParameters::createEmpty()); $resolvedUriConstraints = UriConstraints::create()->withPath('some/request/path'); - /** @var RouterCachingService|\PHPUnit\Framework\MockObject\MockObject $routerCachingService */ - $routerCachingService = $this->getAccessibleMock(RouterCachingService::class, ['buildResolveCacheIdentifier']); - $routerCachingService->expects(self::atLeastOnce())->method('buildResolveCacheIdentifier')->with($resolveContext, $routeValues)->will(self::returnValue('cacheIdentifier')); - $this->inject($routerCachingService, 'resolveCache', $this->mockResolveCache); - - $this->mockResolveCache->expects(self::once())->method('set')->with('cacheIdentifier', $resolvedUriConstraints, [$uuid1, $uuid2, md5('some'), md5('some/request'), md5('some/request/path')]); + $this->mockResolveCache->expects(self::once())->method('set')->with(self::anything(), $resolvedUriConstraints, [md5('some'), md5('some/request'), md5('some/request/path')]); - $routerCachingService->storeResolvedUriConstraints($resolveContext, $resolvedUriConstraints); + $this->routerCachingService->storeResolvedUriConstraints($resolveContext, $resolvedUriConstraints); } /**