Depreciate injection of Request instances (#200)

| Q | A | ------------- | --- | Branch? | master | BC breaks? | yes (small) | Deprecations? | yes | Tests pass? | yes This depreciates the injection of `Request` instances in order to totally avoid this practice in 2.0. That concerns most of our Event classes. Warning triggered by Scrutinizer: > _Bug_ You have injected the Request via parameter $request. This is generally not recommended as there might be multiple instances during a request cycle (f.e. when using sub-requests). Instead, it is recommended to inject the RequestStack and retrieve the current request each time you need it via getCurrentRequest(). The proposed deprecation way consists into: - adding a default value (null) for constructors/setters arguments type hinted as `Request` (BC break) - document the corresponding properties+members as deprecated - trigger a `E_USER_DEPRECATED` notice when the deprecated methods are called with a Request as argument. The recommended alternative is to inject the RequestStack instead. For `Event` classes, the user will be responsible of injecting the request stack into their event listeners to be able to use it. All changed classes are instantiated internally, I think this should not be an hard BC breaking for most people. Any thoughts? ---------- - [x] Pre-remove the practice from our code (when RequestStack exists) - [x] Documentation needs to be updated with an example of `@request_stack` injection&usage
lexik · Jul 25, 2016 · 0906948 · 0906948
1 parent aea1122
commit 0906948
Show file tree

Hide file tree

Showing 14 changed files with 140 additions and 79 deletions.
diff --git a/DependencyInjection/Compiler/RequestCompilerPass.php b/DependencyInjection/Compiler/RequestCompilerPass.php
diff --git a/Event/AuthenticationFailureEvent.php b/Event/AuthenticationFailureEvent.php
@@ -17,6 +17,8 @@ class AuthenticationFailureEvent extends Event
 {
     /**
      * @var Request
+     *
+     * @deprecated since 1.7, removed in 2.0
      */
     protected $request;
 
@@ -31,22 +33,33 @@ class AuthenticationFailureEvent extends Event
     protected $response;
 
     /**
-     * @param Request                 $request
+     * @param Request|null            $request   Deprecated
      * @param AuthenticationException $exception
      * @param Response                $response
      */
-    public function __construct(Request $request, AuthenticationException $exception, Response $response)
+    public function __construct(Request $request = null, AuthenticationException $exception, Response $response)
     {
-        $this->request   = $request;
+        if (null !== $request && class_exists('Symfony\Component\HttpFoundation\RequestStack')) {
+            @trigger_error(sprintf('Passing a Request instance as first argument of %s() is deprecated since version 1.7 and will be removed in 2.0.%sInject the "@request_stack" service in your event listener instead.', __METHOD__, PHP_EOL), E_USER_DEPRECATED);
+
+            $this->request = $request;
+        }
+
         $this->exception = $exception;
         $this->response  = $response;
     }
 
     /**
+     * @deprecated since 1.7, removed in 2.0
+     *
      * @return Request
      */
     public function getRequest()
     {
+        if (class_exists('Symfony\Component\HttpFoundation\RequestStack')) {
+            @trigger_error(sprintf('Method %s() is deprecated since version 1.7 and will be removed in 2.0.%sUse  Symfony\Component\HttpFoundation\RequestStack::getCurrentRequest() instead.', __METHOD__, PHP_EOL), E_USER_DEPRECATED);
+        }
+
         return $this->request;
     }
 

diff --git a/Event/AuthenticationSuccessEvent.php b/Event/AuthenticationSuccessEvent.php
@@ -27,6 +27,8 @@ class AuthenticationSuccessEvent extends Event
 
     /**
      * @var Request
+     *
+     * @deprecated since 1.7, removed in 2.0
      */
     protected $request;
 
@@ -38,11 +40,17 @@ class AuthenticationSuccessEvent extends Event
     /**
      * @param array         $data
      * @param UserInterface $user
-     * @param Request       $request
+     * @param Request|null  $request Deprecated
      * @param Response      $response
      */
-    public function __construct(array $data, UserInterface $user, Request $request, Response $response)
+    public function __construct(array $data, UserInterface $user, Request $request = null, Response $response)
     {
+        if (null !== $request && class_exists('Symfony\Component\HttpFoundation\RequestStack')) {
+            @trigger_error(sprintf('Passing a Request instance as first argument of %s() is deprecated since version 1.7 and will be removed in 2.0.%sInject the "@request_stack" service in your event listener instead.', __METHOD__, PHP_EOL), E_USER_DEPRECATED);
+
+            $this->request = $request;
+        }
+
         $this->data = $data;
         $this->user = $user;
         $this->request = $request;
@@ -74,10 +82,16 @@ public function getUser()
     }
 
     /**
+     * @deprecated since 1.7, removed in 2.0
+     *
      * @return Request
      */
     public function getRequest()
     {
+        if (class_exists('Symfony\Component\HttpFoundation\RequestStack')) {
+            @trigger_error(sprintf('Method %s() is deprecated since version 1.7 and will be removed in 2.0.%sUse  Symfony\Component\HttpFoundation\RequestStack::getCurrentRequest() instead.', __METHOD__, PHP_EOL), E_USER_DEPRECATED);
+        }
+
         return $this->request;
     }
 

diff --git a/Event/JWTAuthenticatedEvent.php b/Event/JWTAuthenticatedEvent.php
@@ -21,12 +21,7 @@ class JWTAuthenticatedEvent extends Event
      * @var TokenInterface
      */
     protected $token;
-
-    /**
-     * @var Request
-     */
-    protected $request;
-
+
     /**
      * @param array          $payload
      * @param TokenInterface $token

diff --git a/Event/JWTCreatedEvent.php b/Event/JWTCreatedEvent.php
@@ -24,19 +24,26 @@ class JWTCreatedEvent extends Event
 
     /**
      * @var Request
+     *
+     * @deprecated since 1.7, removed in 2.0
      */
     protected $request;
 
     /**
      * @param array         $data
      * @param UserInterface $user
-     * @param Request       $request
+     * @param Request|null  $request Deprecated
      */
     public function __construct(array $data, UserInterface $user, Request $request = null)
     {
-        $this->data    = $data;
-        $this->user    = $user;
-        $this->request = $request;
+        if (null !== $request && class_exists('Symfony\Component\HttpFoundation\RequestStack')) {
+            @trigger_error(sprintf('Passing a Request instance as first argument of %s() is deprecated since version 1.7 and will be removed in 2.0.%sInject the "@request_stack" service in your event listener instead.', __METHOD__, PHP_EOL), E_USER_DEPRECATED);
+
+            $this->request = $request;
+        }
+
+        $this->data = $data;
+        $this->user = $user;
     }
 
     /**
@@ -64,10 +71,16 @@ public function getUser()
     }
 
     /**
+     * @deprecated since 1.7, removed in 2.0
+     *
      * @return Request
      */
     public function getRequest()
     {
+        if (class_exists('Symfony\Component\HttpFoundation\RequestStack')) {
+            @trigger_error(sprintf('Method %s() is deprecated since version 1.7 and will be removed in 2.0.%sUse  Symfony\Component\HttpFoundation\RequestStack::getCurrentRequest() instead.', __METHOD__, PHP_EOL), E_USER_DEPRECATED);
+        }
+
         return $this->request;
     }
 }
diff --git a/Event/JWTDecodedEvent.php b/Event/JWTDecodedEvent.php
@@ -19,6 +19,8 @@ class JWTDecodedEvent extends Event
 
     /**
      * @var Request
+     *
+     * @deprecated since 1.7, removed in 2.0
      */
     protected $request;
 
@@ -28,13 +30,18 @@ class JWTDecodedEvent extends Event
     protected $isValid;
 
     /**
-     * @param array   $payload
-     * @param Request $request
+     * @param array        $payload
+     * @param Request|null $request Deprecated
      */
     public function __construct(array $payload, Request $request = null)
     {
+        if (null !== $request && class_exists('Symfony\Component\HttpFoundation\RequestStack')) {
+            @trigger_error(sprintf('Passing a Request instance as first argument of %s() is deprecated since version 1.7 and will be removed in 2.0.%sInject the "@request_stack" service in your event listener instead.', __METHOD__, PHP_EOL), E_USER_DEPRECATED);
+
+            $this->request = $request;
+        }
+
         $this->payload = $payload;
-        $this->request = $request;
         $this->isValid = true;
     }
 
@@ -47,10 +54,16 @@ public function getPayload()
     }
 
     /**
+     * @deprecated since 1.7, removed in 2.0
+     *
      * @return Request
      */
     public function getRequest()
     {
+        if (class_exists('Symfony\Component\HttpFoundation\RequestStack')) {
+            @trigger_error(sprintf('Method %s() is deprecated since version 1.7 and will be removed in 2.0.%sUse  Symfony\Component\HttpFoundation\RequestStack::getCurrentRequest() instead.', __METHOD__, PHP_EOL), E_USER_DEPRECATED);
+        }
+
         return $this->request;
     }
 

diff --git a/Event/JWTFailureEventInterface.php b/Event/JWTFailureEventInterface.php
@@ -8,7 +8,7 @@
 
 /**
  * Interface for event classes that are dispatched when a JWT cannot be authenticated.
- * 
+ *
  * @author Robin Chalas <[email protected]>
  */
 interface JWTFailureEventInterface
@@ -19,6 +19,8 @@ interface JWTFailureEventInterface
     public function getResponse();
 
     /**
+     * @deprecated since 1.7, removed in 2.0
+     *
      * @return Request
      */
     public function getRequest();

diff --git a/Event/JWTNotFoundEvent.php b/Event/JWTNotFoundEvent.php
@@ -7,21 +7,26 @@
 use Symfony\Component\Security\Core\Exception\AuthenticationException;
 
 /**
- * JWTNotFoundEvent event is dispatched when a JWT cannot be found in a request 
+ * JWTNotFoundEvent event is dispatched when a JWT cannot be found in a request
  * covered by a firewall secured via lexik_jwt.
  *
  * @author Robin Chalas <[email protected]>
  */
 class JWTNotFoundEvent extends AuthenticationFailureEvent implements JWTFailureEventInterface
 {
     /**
-     * @param Request                      $request
+     * @param Request|null                 $request   Deprecated
      * @param AuthenticationException|null $exception
      * @param Response|null                $response
      */
-    public function __construct(Request $request, AuthenticationException $exception = null, Response $response = null)
+    public function __construct(Request $request = null, AuthenticationException $exception = null, Response $response = null)
     {
-        $this->request   = $request;
+        if (null !== $request && class_exists('Symfony\Component\HttpFoundation\RequestStack')) {
+            @trigger_error(sprintf('Passing a Request instance as first argument of %s() is deprecated since version 1.7 and will be removed in 2.0.%sInject the "@request_stack" service in your event listener instead.', __METHOD__, PHP_EOL), E_USER_DEPRECATED);
+
+            $this->request = $request;
+        }
+
         $this->exception = $exception;
         $this->response  = $response;
     }

diff --git a/LexikJWTAuthenticationBundle.php b/LexikJWTAuthenticationBundle.php
@@ -25,7 +25,5 @@ public function build(ContainerBuilder $container)
         /** @var SecurityExtension $extension */
         $extension = $container->getExtension('security');
         $extension->addSecurityListenerFactory(new JWTFactory());
-
-        $container->addCompilerPass(new RequestCompilerPass());
     }
 }
diff --git a/Resources/doc/2-data-customization.md b/Resources/doc/2-data-customization.md
@@ -13,6 +13,7 @@ but you can add your own data.
 services:
     acme_api.event.jwt_created_listener:
         class: Acme\Bundle\ApiBundle\EventListener\JWTCreatedListener
+        arguments: [ '@request_stack' ] # Symfony 2.4+
         tags:
             - { name: kernel.event_listener, event: lexik_jwt_authentication.on_jwt_created, method: onJWTCreated }
 ```
@@ -21,18 +22,38 @@ Example 1 : add client ip to the encoded payload
 
 ``` php
 // Acme\Bundle\ApiBundle\EventListener\JWTCreatedListener.php
+
+use Symfony\Component\HttpFoundation\RequestStack;
+
 class JWTCreatedListener
 {
+    /**
+     * @var RequestStack
+     */
+    private $requestStack;
+
+    /**
+     * @param RequestStack $requestStack
+     */
+    public function __construct(RequestStack $requestStack)
+    {
+        $this->requestStack = $requestStack;
+    }
+
+    // ...
+
     /**
      * @param JWTCreatedEvent $event
      *
      * @return void
      */
     public function onJWTCreated(JWTCreatedEvent $event)
     {
-        if (!($request = $event->getRequest())) {
-            return;
-        }
+        // Symfony < 2.4
+        $request = $event->getRequest();
+
+        // Symfony 2.4+
+        $request = $this->requestStack->getCurrentRequest();
 
         $payload       = $event->getData();
         $payload['ip'] = $request->getClientIp();
@@ -47,7 +68,7 @@ Example 2 : override token expiration date calcul to be more flexible
 ``` php
 // Acme\Bundle\ApiBundle\EventListener\JWTCreatedListener.php
 class JWTCreatedListener
-{
+{  
     /**
      * @param JWTCreatedEvent $event
      *
@@ -75,6 +96,7 @@ You can access the jwt payload once it has been decoded to perform you own addit
 services:
     acme_api.event.jwt_decoded_listener:
         class: Acme\Bundle\ApiBundle\EventListener\JWTDecodedListener
+        arguments: [ '@request_stack' ] # Symfony 2.4+
         tags:
             - { name: kernel.event_listener, event: lexik_jwt_authentication.on_jwt_decoded, method: onJWTDecoded }
 ```
@@ -85,19 +107,22 @@ Example 3 : check client ip the decoded payload (from example 1)
 // Acme\Bundle\ApiBundle\EventListener\JWTDecodedListener.php
 class JWTDecodedListener
 {
+    // ...
+
     /**
      * @param JWTDecodedEvent $event
      *
      * @return void
      */
     public function onJWTDecoded(JWTDecodedEvent $event)
     {
-        if (!($request = $event->getRequest())) {
-            return;
-        }
-
-        $payload = $event->getPayload();
+        // Symfony < 2.4
         $request = $event->getRequest();
+
+        // Symfony 2.4+
+        $request = $this->requestStack->getCurrentRequest();
+
+        $payload = $event->getPayload();
 
         if (!isset($payload['ip']) || $payload['ip'] !== $request->getClientIp()) {
             $event->markAsInvalid();