Refactore new methods blacklisters and add documentation

Author: pyrech

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 * [BC BREAK] Remove SeoManagerInterface
 * Add blacklist behaviour to avoid useless fetcher runs
+* Add blacklisters checking for XHR or HTTP methods
 
 ## 0.4.0 (2017-09-12)
 

doc/symfony/README.md

@@ -111,7 +111,7 @@ Note:
 ### Blacklist
 
 You can blacklist some request/response to avoid fetcher to run (f.e. on non
-2xx response, on your admin, etc).
+2xx response, on your admin, on XHR calls, etc).
 
 There is some built-in blacklister that you can enable/configure via the config
 `blacklist`:

doc/symfony/available-blacklisters.md

@@ -4,6 +4,8 @@
 |---|---|---|
 | not_2xx | Blacklist if the response status code is not 2xx | None |
 | path | Blacklist if the request path matches the pattern | - "pattern": mandatory, the regex pattern to match |
+| not_method | Blacklist if the request HTTP method is in the accepted ones | - "method": mandatory, the HTTP method(s) to accept |
+| xml_http | Blacklist if the request is made through XmlHttpRequest | None |
 
 ## not_2xx
 
@@ -36,3 +38,31 @@ seo_override:
             type: path
             pattern: '^/(admin|account)'
 ```
+
+## not_method
+
+The `not_method` blacklister refuses request whose method is not in the
+configured `method` option. This option can be a single HTTP method or an array
+of methods:
+
+```yaml
+seo_override:
+    blacklist:
+        -
+            type: not_method
+            method: [GET, POST]
+            # if only one method should be accepted
+            # method: GET
+```
+
+## xml_http
+
+The `xml_http` blacklister refuses request which have been made through Ajax
+(XmlHttpRequest). This blacklister does not have any option so the short YAML
+syntax can be used:
+
+```yaml
+seo_override:
+    blacklist:
+        - xml_http
+```

src/Bridge/Symfony/Blacklister/GetMethodBlacklister.php src/Bridge/Symfony/Blacklister/NotMethodBlacklister.php

@@ -15,19 +15,27 @@
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 
-/**
- * Class GetBlacklister.
- */
-class GetMethodBlacklister implements Blacklister
+class NotMethodBlacklister implements Blacklister
 {
+    /** @var string[] */
+    private $methods;
+
     /**
-     * @param Request  $request
-     * @param Response $response
-     *
-     * @return bool
+     * @param string|string[] $method
      */
+    public function __construct($method)
+    {
+        $this->methods = (array) $method;
+    }
+
     public function isBlacklisted(Request $request, Response $response): bool
     {
-        return $request->isMethod('get');
+        foreach ($this->methods as $method) {
+            if ($request->isMethod($method)) {
+                return false;
+            }
+        }
+
+        return true;
     }
 }

src/Bridge/Symfony/Blacklister/PostMethodBlacklister.php

@@ -15,17 +15,8 @@
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 
-/**
- * Class XmlHttpBlacklister.
- */
 class XmlHttpBlacklister implements Blacklister
 {
-    /**
-     * @param Request  $request
-     * @param Response $response
-     *
-     * @return bool
-     */
     public function isBlacklisted(Request $request, Response $response): bool
     {
         return $request->isXmlHttpRequest();

src/Bridge/Symfony/Blacklister/XmlHttpBlacklister.php

@@ -52,17 +52,13 @@ services:
         tags:
             - { name: seo_override.blacklister, alias: not_2xx }
 
-    seo_override.blacklister.get_method:
-        class: Joli\SeoOverride\Bridge\Symfony\Blacklister\GetMethodBlacklister
-        public: false
-        tags:
-            - { name: seo_override.blacklister, alias: get_method }
-
-    seo_override.blacklister.post_method:
-        class: Joli\SeoOverride\Bridge\Symfony\Blacklister\PostMethodBlacklister
+    seo_override.blacklister.not_method:
+        class: Joli\SeoOverride\Bridge\Symfony\Blacklister\NotMethodBlacklister
         public: false
+        arguments:
+            - '' # Argument configured in the DIC extension
         tags:
-            - { name: seo_override.blacklister, alias: post_method }
+            - { name: seo_override.blacklister, alias: not_method, required_options: 'method' }
 
     seo_override.blacklister.xml_http:
         class: Joli\SeoOverride\Bridge\Symfony\Blacklister\XmlHttpBlacklister

src/Bridge/Symfony/Resources/config/services.yml

@@ -52,3 +52,5 @@ seo_override:
     blacklist:
         - not_2xx
         - { type: path, pattern: '^/admin' }
+        - { type: not_method, method: ['GET', 'POST'] }
+        - xml_http

tests/Functional/Fixtures/symfony/app/config/config.yml

@@ -186,16 +186,34 @@ public function test_it_does_not_override_seo_when_request_path_does_not_match()
         $this->assertSame($expected, $response->getContent());
     }
 
+    public function test_it_does_not_override_seo_when_request_use_not_allowed_action()
+    {
+        $response = $this->call('/', 'localhost', 'PUT');
+
+        $this->assertSame(200, $response->getStatusCode());
+        $this->assertSame(self::NOT_OVERRIDDEN_HOMEPAGE_CONTENT, $response->getContent());
+    }
+
+    public function test_it_does_not_override_seo_when_request_is_xhr()
+    {
+        $response = $this->call('/', 'localhost', 'GET', [
+            'X-Requested-With' => 'XMLHttpRequest',
+        ]);
+
+        $this->assertSame(200, $response->getStatusCode());
+        $this->assertSame(self::NOT_OVERRIDDEN_HOMEPAGE_CONTENT, $response->getContent());
+    }
+
     protected static function getKernelClass()
     {
         return AppKernel::class;
     }
 
-    private function call(string $uri, string $host)
+    private function call(string $uri, string $host, string $method = 'GET', array $server = [])
     {
-        $request = Request::create($uri, 'GET', [], [], [], [
-            'HTTP_HOST' => $host,
-        ]);
+        $server['HTTP_HOST'] = $host;
+
+        $request = Request::create($uri, $method, [], [], [], $server);
 
         return self::$kernel->handle($request);
     }

tests/Functional/SymfonyTest.php

@@ -0,0 +1,62 @@
+<?php
+
+/*
+ * This file is part of the SeoOverride project.
+ *
+ * (c) JoliCode <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Joli\SeoOverride\Tests\Unit\Bridge\Symfony\Blacklister;
+
+use Joli\SeoOverride\Bridge\Symfony\Blacklister\NotMethodBlacklister;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
+
+class NotMethodBlacklisterTest extends TestCase
+{
+    /** @var NotMethodBlacklisterTest */
+    private $blacklister;
+
+    public function setUp()
+    {
+        parent::setUp();
+    }
+
+    public function test_it_blacklists_not_accepted_method_requests()
+    {
+        $request = new Request();
+        $request->setMethod('put');
+
+        $blacklister = new NotMethodBlacklister(['GET', 'POST']);
+
+        self::assertTrue($blacklister->isBlacklisted($request, new Response()));
+    }
+
+    public function test_it_does_not_blacklist_accepted_method_requests()
+    {
+        $request = new Request();
+        $request->setMethod('get');
+
+        $blacklister = new NotMethodBlacklister('GET');
+
+        self::assertFalse($blacklister->isBlacklisted($request, new Response()));
+
+        $request = new Request();
+        $request->setMethod('get');
+
+        $blacklister = new NotMethodBlacklister(['GET', 'POST']);
+
+        self::assertFalse($blacklister->isBlacklisted($request, new Response()));
+
+        $request = new Request();
+        $request->setMethod('POST');
+
+        $blacklister = new NotMethodBlacklister(['GET', 'POST']);
+
+        self::assertFalse($blacklister->isBlacklisted($request, new Response()));
+    }
+}

tests/Unit/Bridge/Symfony/Blacklister/GetMethodBlackListerTest.php

@@ -36,7 +36,7 @@ public function test_it_blacklists_xml_http_requests()
         self::assertTrue($this->blacklister->isBlacklisted($request, new Response()));
     }
 
-    public function test_it_does_not_blacklist_xml_http_requests()
+    public function test_it_does_not_blacklist_not_xml_http_requests()
     {
         $request = new Request();