Browse Source

Make header(), version() and method() play nice with PSR7

Add tests ensuring that both setters can be read from the other reader
method.
Mark Story 10 years ago
parent
commit
92081faf1c

+ 1 - 1
src/Http/Client.php

@@ -421,7 +421,7 @@ class Client
     protected function _createRequest($method, $url, $data, $options)
     {
         $request = new Request();
-        $request->method($method)
+        $request = $request->withMethod($method)
             ->url($url)
             ->body($data);
 

+ 1 - 8
src/Http/Client/Message.php

@@ -142,13 +142,6 @@ class Message
     protected $_cookies = [];
 
     /**
-     * HTTP Version being used.
-     *
-     * @var string
-     */
-    protected $_version = '1.1';
-
-    /**
      * Normalize header names to Camel-Case form.
      *
      * @param string $name The header name to normalize.
@@ -191,7 +184,7 @@ class Message
      */
     public function version()
     {
-        return $this->_version;
+        return $this->protocol;
     }
 
     /**

+ 40 - 22
src/Http/Client/Request.php

@@ -14,6 +14,7 @@
 namespace Cake\Http\Client;
 
 use Cake\Core\Exception\Exception;
+use Psr\Http\Message\RequestInterface;
 use Zend\Diactoros\MessageTrait;
 use Zend\Diactoros\RequestTrait;
 
@@ -24,19 +25,12 @@ use Zend\Diactoros\RequestTrait;
  * for making requests.
  *
  */
-class Request extends Message
+class Request extends Message implements RequestInterface
 {
     use MessageTrait;
     use RequestTrait;
 
     /**
-     * The HTTP method to use.
-     *
-     * @var string
-     */
-    protected $_method = self::METHOD_GET;
-
-    /**
      * Request body to send.
      *
      * @var mixed
@@ -51,18 +45,30 @@ class Request extends Message
     protected $_url;
 
     /**
-     * Headers to be sent.
+     * Constructor
      *
-     * @var array
+     * Provides backwards compatible defaults for some properties.
      */
-    protected $_headers = [
-        'Connection' => 'close',
-        'User-Agent' => 'CakePHP'
-    ];
+    public function __construct()
+    {
+        $this->method = static::METHOD_GET;
+
+        $this->headerNames = [
+            'connection' => 'Connection',
+            'user-agent' => 'User-Agent',
+        ];
+        $this->headers = [
+            'Connection' => 'close',
+            'User-Agent' => 'CakePHP'
+        ];
+    }
 
     /**
      * Get/Set the HTTP method.
      *
+     * *Warning* This method mutates the request in-place for backwards
+     * compatibility issues, and is not part of the PSR7 interface.
+     *
      * @param string|null $method The method for the request.
      * @return $this|string Either this or the current method.
      * @throws \Cake\Core\Exception\Exception On invalid methods.
@@ -71,13 +77,13 @@ class Request extends Message
     public function method($method = null)
     {
         if ($method === null) {
-            return $this->_method;
+            return $this->method;
         }
         $name = get_called_class() . '::METHOD_' . strtoupper($method);
         if (!defined($name)) {
             throw new Exception('Invalid method type');
         }
-        $this->_method = $method;
+        $this->method = $method;
         return $this;
     }
 
@@ -120,22 +126,31 @@ class Request extends Message
      * $request->header(['Connection' => 'close', 'User-Agent' => 'CakePHP']);
      * ```
      *
+     * *Warning* This method mutates the request in-place for backwards
+     * compatibility issues, and is not part of the PSR7 interface.
+     *
      * @param string|array|null $name The name to get, or array of multiple values to set.
      * @param string|null $value The value to set for the header.
      * @return mixed Either $this when setting or header value when getting.
+     * @deprecated 3.3.0 Use withHeader() and getHeaderLine() instead.
      */
     public function header($name = null, $value = null)
     {
         if ($value === null && is_string($name)) {
-            $name = $this->_normalizeHeader($name);
-            return isset($this->_headers[$name]) ? $this->_headers[$name] : null;
+            $val = $this->getHeaderLine($name);
+            if ($val === '') {
+                return null;
+            }
+            return $val;
         }
+
         if ($value !== null && !is_array($name)) {
             $name = [$name => $value];
         }
         foreach ($name as $key => $val) {
-            $key = $this->_normalizeHeader($key);
-            $this->_headers[$key] = $val;
+            $normalized = strtolower($key);
+            $this->headers[$key] = (array)$val;
+            $this->headerNames[$normalized] = $key;
         }
         return $this;
     }
@@ -182,6 +197,9 @@ class Request extends Message
     /**
      * Get/Set HTTP version.
      *
+     * *Warning* This method mutates the request in-place for backwards
+     * compatibility issues, and is not part of the PSR7 interface.
+     *
      * @param string|null $version The HTTP version.
      * @return $this|string Either $this or the HTTP version.
      * @deprecated 3.3.0 Use getProtocolVersion() and withProtocolVersion() instead.
@@ -189,10 +207,10 @@ class Request extends Message
     public function version($version = null)
     {
         if ($version === null) {
-            return parent::version();
+            return $this->protocol;
         }
 
-        $this->_version = $version;
+        $this->protocol = $version;
         return $this;
     }
 }

+ 7 - 7
tests/TestCase/Network/Http/ClientTest.php

@@ -172,7 +172,7 @@ class ClientTest extends TestCase
             ->method('send')
             ->with($this->logicalAnd(
                 $this->isInstanceOf('Cake\Network\Http\Request'),
-                $this->attributeEqualTo('_method', Request::METHOD_GET),
+                $this->attributeEqualTo('method', Request::METHOD_GET),
                 $this->attributeEqualTo('_url', 'http://cakephp.org/test.html'),
                 $this->attributeEqualTo('_headers', $headers),
                 $this->attributeEqualTo('_cookies', $cookies)
@@ -201,7 +201,7 @@ class ClientTest extends TestCase
             ->method('send')
             ->with($this->logicalAnd(
                 $this->isInstanceOf('Cake\Network\Http\Request'),
-                $this->attributeEqualTo('_method', Request::METHOD_GET),
+                $this->attributeEqualTo('method', Request::METHOD_GET),
                 $this->attributeEqualTo('_url', 'http://cakephp.org/search?q=hi+there&Category%5Bid%5D%5B0%5D=2&Category%5Bid%5D%5B1%5D=3')
             ))
             ->will($this->returnValue([$response]));
@@ -262,7 +262,7 @@ class ClientTest extends TestCase
             ->method('send')
             ->with($this->logicalAnd(
                 $this->isInstanceOf('Cake\Network\Http\Request'),
-                $this->attributeEqualTo('_method', Request::METHOD_GET),
+                $this->attributeEqualTo('method', Request::METHOD_GET),
                 $this->attributeEqualTo('_url', 'http://cakephp.org/search'),
                 $this->attributeEqualTo('_body', 'some data')
             ))
@@ -319,7 +319,7 @@ class ClientTest extends TestCase
             ->method('send')
             ->with($this->logicalAnd(
                 $this->isInstanceOf('Cake\Network\Http\Request'),
-                $this->attributeEqualTo('_method', Request::METHOD_GET),
+                $this->attributeEqualTo('method', Request::METHOD_GET),
                 $this->attributeEqualTo('_url', 'http://cakephp.org/'),
                 $this->attributeEqualTo('_headers', $headers)
             ))
@@ -368,7 +368,7 @@ class ClientTest extends TestCase
             ->method('send')
             ->with($this->logicalAnd(
                 $this->isInstanceOf('Cake\Network\Http\Request'),
-                $this->attributeEqualTo('_method', $method),
+                $this->attributeEqualTo('method', $method),
                 $this->attributeEqualTo('_url', 'http://cakephp.org/projects/add')
             ))
             ->will($this->returnValue([$response]));
@@ -417,7 +417,7 @@ class ClientTest extends TestCase
         $mock->expects($this->once())
             ->method('send')
             ->with($this->logicalAnd(
-                $this->attributeEqualTo('_method', Request::METHOD_POST),
+                $this->attributeEqualTo('method', Request::METHOD_POST),
                 $this->attributeEqualTo('_body', $data),
                 $this->attributeEqualTo('_headers', $headers)
             ))
@@ -538,7 +538,7 @@ class ClientTest extends TestCase
             ->method('send')
             ->with($this->logicalAnd(
                 $this->isInstanceOf('Cake\Network\Http\Request'),
-                $this->attributeEqualTo('_method', Request::METHOD_HEAD),
+                $this->attributeEqualTo('method', Request::METHOD_HEAD),
                 $this->attributeEqualTo('_url', 'http://cakephp.org/search?q=hi+there')
             ))
             ->will($this->returnValue([$response]));

+ 62 - 0
tests/TestCase/Network/Http/RequestTest.php

@@ -49,6 +49,23 @@ class RequestTest extends TestCase
     }
 
     /**
+     * test method interop.
+     *
+     * @return void
+     */
+    public function testMethodInteroperability()
+    {
+        $request = new Request();
+        $this->assertSame($request, $request->method(Request::METHOD_GET));
+        $this->assertEquals(Request::METHOD_GET, $request->method());
+        $this->assertEquals(Request::METHOD_GET, $request->getMethod());
+
+        $request = $request->withMethod(Request::METHOD_GET);
+        $this->assertEquals(Request::METHOD_GET, $request->method());
+        $this->assertEquals(Request::METHOD_GET, $request->getMethod());
+    }
+
+    /**
      * test invalid method.
      *
      * @expectedException \Cake\Core\Exception\Exception
@@ -104,6 +121,35 @@ class RequestTest extends TestCase
     }
 
     /**
+     * Test the default headers
+     *
+     * @return void
+     */
+    public function testDefaultHeaders()
+    {
+        $request = new Request();
+        $this->assertEquals('CakePHP', $request->getHeaderLine('User-Agent'));
+        $this->assertEquals('close', $request->getHeaderLine('Connection'));
+    }
+
+    /**
+     * Test that header() and PSR7 methods play nice.
+     *
+     * @return void
+     */
+    public function testHeaderMethodInteroperability()
+    {
+        $request = new Request();
+        $request->header('Content-Type', 'application/json');
+        $this->assertEquals('application/json', $request->header('Content-Type'), 'Old getter should work');
+
+        $this->assertEquals('application/json', $request->getHeaderLine('Content-Type'), 'getHeaderLine works');
+        $this->assertEquals('application/json', $request->getHeaderLine('content-type'), 'getHeaderLine works');
+        $this->assertEquals(['application/json'], $request->getHeader('Content-Type'), 'getHeader works');
+        $this->assertEquals(['application/json'], $request->getHeader('content-type'), 'getHeader works');
+    }
+
+    /**
      * test cookie method.
      *
      * @return void
@@ -133,4 +179,20 @@ class RequestTest extends TestCase
 
         $this->assertSame('1.0', $request->version());
     }
+
+    /**
+     * test version interop.
+     *
+     * @return void
+     */
+    public function testVersionInteroperability()
+    {
+        $request = new Request();
+        $this->assertEquals('1.1', $request->version());
+        $this->assertEquals('1.1', $request->getProtocolVersion());
+
+        $request = $request->withProtocolVersion('1.0');
+        $this->assertEquals('1.0', $request->version());
+        $this->assertEquals('1.0', $request->getProtocolVersion());
+    }
 }