Merge pull request #627 from FriendsOfSymfony/final-polish

constructor argument promotion and cleanup test type declarations
FriendsOfSymfony · May 4, 2024 · e737c1a · e737c1a
2 parents 3f6fe79 + e726e95
commit e737c1a
Show file tree

Hide file tree

Showing 55 changed files with 204 additions and 328 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,9 +10,9 @@ Changelog
   Remove all configuration you have at `fos_http_cache.tags.annotations`
 * Make `fastly` and `cloudflare` clients lazy loaded to support Symfony secrets that are only available at runtime, but
   not yet when the container is built.
-* Removed deprecated `FOS\HttpCacheBundle\UserContext\AnonymousRequestMatcher`, use the 
-  `FOS\HttpCache\UserContext\AnonymousRequestMatcher` class.
-* Removed deprecated `ContextInvalidationLogoutHandler`, use `ContextInvalidationSessionLogoutHandler` instead.
+* Removed deprecated `FOS\HttpCacheBundle\UserContext\AnonymousRequestMatcher`, use the class from the `FOSHttpCache`
+  component instead: `FOS\HttpCache\UserContext\AnonymousRequestMatcher`.
+* Removed obsolete `ContextInvalidationLogoutHandler`, use `ContextInvalidationSessionLogoutHandler` instead.
 * Fixed service loading to not fail when Twig is not available.
 
 2.x

diff --git a/src/CacheManager.php b/src/CacheManager.php
@@ -24,26 +24,16 @@
  */
 class CacheManager extends CacheInvalidator
 {
-    private ProxyClient $cache;
-
-    private UrlGeneratorInterface $urlGenerator;
-
     /**
      * What type of urls to generate.
      */
     private int $generateUrlType = UrlGeneratorInterface::ABSOLUTE_PATH;
 
-    /**
-     * Constructor.
-     *
-     * @param ProxyClient           $cache        HTTP cache proxy client
-     * @param UrlGeneratorInterface $urlGenerator Symfony route generator
-     */
-    public function __construct(ProxyClient $cache, UrlGeneratorInterface $urlGenerator)
-    {
+    public function __construct(
+        private readonly ProxyClient $cache,
+        private readonly UrlGeneratorInterface $urlGenerator
+    ) {
         parent::__construct($cache);
-        $this->cache = $cache;
-        $this->urlGenerator = $urlGenerator;
     }
 
     /**

diff --git a/src/DependencyInjection/Compiler/HashGeneratorPass.php b/src/DependencyInjection/Compiler/HashGeneratorPass.php
@@ -46,7 +46,7 @@ public function process(ContainerBuilder $container): void
         }
 
         krsort($prioritisedTags, SORT_NUMERIC);
-        $prioritisedProviders = call_user_func_array('array_merge', $prioritisedTags);
+        $prioritisedProviders = array_merge(...$prioritisedTags);
 
         $providers = [];
         foreach ($prioritisedProviders as $id) {

diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php
@@ -11,6 +11,7 @@
 
 namespace FOS\HttpCacheBundle\DependencyInjection;
 
+use DateTime;
 use FOS\HttpCache\ProxyClient\Varnish;
 use FOS\HttpCache\SymfonyCache\PurgeListener;
 use FOS\HttpCache\SymfonyCache\PurgeTagsListener;
@@ -35,17 +36,9 @@
  */
 final class Configuration implements ConfigurationInterface
 {
-    /**
-     * @var bool
-     */
-    private $debug;
-
-    /**
-     * @param bool $debug Whether to use the debug mode
-     */
-    public function __construct($debug)
-    {
-        $this->debug = $debug;
+    public function __construct(
+        private readonly bool $debug
+    ) {
     }
 
     public function getConfigTreeBuilder(): TreeBuilder
@@ -113,7 +106,7 @@ function ($v) {
                     }
 
                     if (isset($v['proxy_client']['default'])
-                        && in_array($v['proxy_client']['default'], ['varnish', 'symfony', 'noop'])
+                        && \in_array($v['proxy_client']['default'], ['varnish', 'symfony', 'noop'])
                     ) {
                         $v['user_context']['logout_handler']['enabled'] = true;
 
@@ -325,7 +318,7 @@ private function addCacheControlSection(ArrayNodeDefinition $rootNode): void
                     ->scalarNode('last_modified')
                         ->validate()
                             ->ifTrue(function ($v) {
-                                if (is_string($v)) {
+                                if (\is_string($v)) {
                                     new \DateTime($v);
                                 }
 

diff --git a/src/EventListener/AttributesListener.php b/src/EventListener/AttributesListener.php
@@ -18,7 +18,7 @@
 final class AttributesListener implements EventSubscriberInterface
 {
     public function __construct(
-        private ControllerResolverInterface $controllerResolver
+        private readonly ControllerResolverInterface $controllerResolver
     ) {
     }
 

diff --git a/src/EventListener/CacheControlListener.php b/src/EventListener/CacheControlListener.php
@@ -48,20 +48,15 @@ final class CacheControlListener implements EventSubscriberInterface
      */
     private array $rulesMap = [];
 
-    /**
-     * If not empty, add a debug header with that name to all responses,
-     * telling the cache proxy to add debug output.
-     *
-     * @var string|false Name of the header or false to add no header
-     */
-    private string|false $debugHeader;
-
-    /**
-     * @param string|false $debugHeader Header to set to trigger debugging, or false to send no header
-     */
-    public function __construct(string|false $debugHeader = false)
-    {
-        $this->debugHeader = $debugHeader;
+    public function __construct(
+        /**
+         * If not empty, add a debug header with that name to all responses,
+         * telling the cache proxy to add debug output.
+         *
+         * @var string|false Name of the header or false to add no header
+         */
+        private readonly string|false $debugHeader = false
+    ) {
     }
 
     public static function getSubscribedEvents(): array

diff --git a/src/EventListener/InvalidationListener.php b/src/EventListener/InvalidationListener.php
@@ -35,24 +35,12 @@
  */
 final class InvalidationListener extends AbstractRuleListener implements EventSubscriberInterface
 {
-    private CacheManager $cacheManager;
-    private UrlGeneratorInterface $urlGenerator;
-    private ?ExpressionLanguage $expressionLanguage;
-    private RuleMatcherInterface $mustInvalidateRule;
-
-    /**
-     * Constructor.
-     */
     public function __construct(
-        CacheManager $cacheManager,
-        UrlGeneratorInterface $urlGenerator,
-        RuleMatcherInterface $mustInvalidateRule,
-        ?ExpressionLanguage $expressionLanguage = null
+        private readonly CacheManager $cacheManager,
+        private readonly UrlGeneratorInterface $urlGenerator,
+        private readonly RuleMatcherInterface $mustInvalidateRule,
+        private ?ExpressionLanguage $expressionLanguage = null
     ) {
-        $this->cacheManager = $cacheManager;
-        $this->urlGenerator = $urlGenerator;
-        $this->mustInvalidateRule = $mustInvalidateRule;
-        $this->expressionLanguage = $expressionLanguage;
     }
 
     /**
@@ -76,7 +64,7 @@ public function onKernelTerminate(TerminateEvent $event): void
 
         try {
             $this->cacheManager->flush();
-        } catch (ExceptionCollection $e) {
+        } catch (ExceptionCollection) {
             // swallow exception
             // there is the fos_http_cache.event_listener.log to log them
         }
@@ -89,7 +77,7 @@ public function onKernelException(): void
     {
         try {
             $this->cacheManager->flush();
-        } catch (ExceptionCollection $e) {
+        } catch (ExceptionCollection) {
             // swallow exception
             // there is the fos_http_cache.event_listener.log to log them
         }

diff --git a/src/EventListener/SwitchUserListener.php b/src/EventListener/SwitchUserListener.php
@@ -18,11 +18,9 @@
 
 final class SwitchUserListener implements EventSubscriberInterface
 {
-    private UserContextInvalidator $invalidator;
-
-    public function __construct(UserContextInvalidator $invalidator)
-    {
-        $this->invalidator = $invalidator;
+    public function __construct(
+        private readonly UserContextInvalidator $invalidator
+    ) {
     }
 
     public static function getSubscribedEvents(): array

diff --git a/src/EventListener/TagListener.php b/src/EventListener/TagListener.php
@@ -29,27 +29,13 @@
  */
 final class TagListener extends AbstractRuleListener implements EventSubscriberInterface
 {
-    private CacheManager $cacheManager;
-    private SymfonyResponseTagger $symfonyResponseTagger;
-    private ?ExpressionLanguage $expressionLanguage;
-    private RuleMatcherInterface $mustInvalidateRule;
-    private RuleMatcherInterface $cacheableRule;
-
-    /**
-     * Constructor.
-     */
     public function __construct(
-        CacheManager $cacheManager,
-        SymfonyResponseTagger $tagHandler,
-        RuleMatcherInterface $cacheableRule,
-        RuleMatcherInterface $mustInvalidateRule,
-        ?ExpressionLanguage $expressionLanguage = null
+        private readonly CacheManager $cacheManager,
+        private readonly SymfonyResponseTagger $symfonyResponseTagger,
+        private readonly RuleMatcherInterface $cacheableRule,
+        private readonly RuleMatcherInterface $mustInvalidateRule,
+        private ?ExpressionLanguage $expressionLanguage = null
     ) {
-        $this->cacheManager = $cacheManager;
-        $this->symfonyResponseTagger = $tagHandler;
-        $this->cacheableRule = $cacheableRule;
-        $this->mustInvalidateRule = $mustInvalidateRule;
-        $this->expressionLanguage = $expressionLanguage;
     }
 
     /**
@@ -115,19 +101,19 @@ private function getAttributeTags(Request $request): array
             return [];
         }
 
-        $tags = [];
+        $tagArrays = [];
         foreach ($tagConfigurations as $tagConfiguration) {
             if (null !== $tagConfiguration->getExpression()) {
-                $tags[] = $this->evaluateTag(
+                $tagArrays[] = [$this->evaluateTag(
                     $tagConfiguration->getExpression(),
                     $request
-                );
+                )];
             } else {
-                $tags = array_merge($tags, $tagConfiguration->getTags());
+                $tagArrays[] = $tagConfiguration->getTags();
             }
         }
 
-        return $tags;
+        return array_merge(...$tagArrays);
     }
 
     /**

diff --git a/src/EventListener/UserContextListener.php b/src/EventListener/UserContextListener.php
@@ -36,45 +36,30 @@
  */
 final class UserContextListener implements EventSubscriberInterface
 {
-    private RequestMatcherInterface $requestMatcher;
-    private HashGenerator $hashGenerator;
-
-    /**
-     * If the response tagger is set, the hash lookup response is tagged with the session id for later invalidation.
-     */
-    private ?ResponseTagger $responseTagger;
-
     private array $options;
 
-    /**
-     * Whether the application has a session listener and therefore could
-     * require the AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER.
-     */
-    private bool $hasSessionListener;
-
     private bool $wasAnonymous = false;
 
-    /**
-     * Used to exclude anonymous requests (no authentication nor session) from user hash sanity check.
-     * It prevents issues when the hash generator that is used returns a customized value for anonymous users,
-     * that differs from the documented, hardcoded one.
-     */
-    private ?RequestMatcherInterface $anonymousRequestMatcher;
-
     public function __construct(
-        RequestMatcherInterface $requestMatcher,
-        HashGenerator $hashGenerator,
-        ?RequestMatcherInterface $anonymousRequestMatcher = null,
-        ?ResponseTagger $responseTagger = null,
+        private readonly RequestMatcherInterface $requestMatcher,
+        private readonly HashGenerator $hashGenerator,
+        /**
+         * Used to exclude anonymous requests (no authentication nor session) from user hash sanity check.
+         * It prevents issues when the hash generator that is used returns a customized value for anonymous users,
+         * that differs from the documented, hardcoded one.
+         */
+        private readonly ?RequestMatcherInterface $anonymousRequestMatcher = null,
+        /**
+         * If the response tagger is set, the hash lookup response is tagged with the session id for later invalidation.
+         */
+        private readonly ?ResponseTagger $responseTagger = null,
         array $options = [],
-        bool $hasSessionListener = true
+        /**
+         * Whether the application has a session listener and therefore could
+         * require the AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER.
+         */
+        private readonly bool $hasSessionListener = true
     ) {
-        $this->requestMatcher = $requestMatcher;
-        $this->hashGenerator = $hashGenerator;
-        $this->anonymousRequestMatcher = $anonymousRequestMatcher;
-        $this->responseTagger = $responseTagger;
-        $this->hasSessionListener = $hasSessionListener;
-
         $resolver = new OptionsResolver();
         $resolver->setDefaults([
             'user_identifier_headers' => ['Cookie', 'Authorization'],

diff --git a/src/Http/RequestMatcher/QueryStringRequestMatcher.php b/src/Http/RequestMatcher/QueryStringRequestMatcher.php
@@ -19,14 +19,12 @@
  */
 final class QueryStringRequestMatcher implements RequestMatcherInterface
 {
-    /**
-     * @var string Regular expression to match the query string part of the request url
-     */
-    private string $queryString;
-
-    public function __construct(string $queryString)
-    {
-        $this->queryString = $queryString;
+    public function __construct(
+        /**
+         * @var string Regular expression to match the query string part of the request url
+         */
+        private readonly string $queryString
+    ) {
     }
 
     public function matches(Request $request): bool

diff --git a/src/Http/ResponseMatcher/ExpressionResponseMatcher.php b/src/Http/ResponseMatcher/ExpressionResponseMatcher.php
@@ -16,18 +16,15 @@
 
 final class ExpressionResponseMatcher implements ResponseMatcherInterface
 {
-    private ?ExpressionLanguage $expressionLanguage;
-    private string $expression;
-
-    public function __construct(string $expression, ?ExpressionLanguage $expressionLanguage = null)
-    {
-        $this->expression = $expression;
-        $this->expressionLanguage = $expressionLanguage;
+    public function __construct(
+        private readonly string $expression,
+        private ?ExpressionLanguage $expressionLanguage = null
+    ) {
     }
 
-    public function matches(Response $response)
+    public function matches(Response $response): bool
     {
-        return $this->getExpressionLanguage()->evaluate(
+        return (bool) $this->getExpressionLanguage()->evaluate(
             $this->expression,
             ['response' => $response]
         );

diff --git a/src/Http/ResponseMatcher/NonErrorResponseMatcher.php b/src/Http/ResponseMatcher/NonErrorResponseMatcher.php
@@ -21,7 +21,7 @@
  */
 final class NonErrorResponseMatcher implements ResponseMatcherInterface
 {
-    public function matches(Response $response)
+    public function matches(Response $response): bool
     {
         return $response->getStatusCode() >= 200
             && $response->getStatusCode() < 400;

diff --git a/src/Http/ResponseMatcher/ResponseMatcherInterface.php b/src/Http/ResponseMatcher/ResponseMatcherInterface.php
@@ -17,8 +17,6 @@ interface ResponseMatcherInterface
 {
     /**
      * Determines whether the response matches the rule.
-     *
-     * @return bool
      */
-    public function matches(Response $response);
+    public function matches(Response $response): bool;
 }