Fixed TODO: Refactored setting of Cookies into CakeResponse.

lib/Cake/Controller/Component/CookieComponent.php

@@ -154,6 +154,13 @@ class CookieComponent extends Component {
 	protected $_expires = 0;
 
 /**
+ * A reference to the Controller's CakeResponse object
+ * 
+ * @var CakeResponse
+ */
+	protected $_response = null;
+
+/**
  * Constructor
  *
  * @param ComponentCollection $collection A ComponentCollection for this component
@@ -168,6 +175,20 @@ class CookieComponent extends Component {
 	}
 
 /**
+ * Initialize CookieComponent
+ * 
+ * @param Controller $controller
+ * @return void
+ */
+	public function initialize($controller) {
+		if (is_object($controller) && isset($controller->response)) {
+			$this->_response = $controller->response;
+		} else {
+			$this->_response = new CakeResponse(array('charset' => Configure::read('App.encoding')));
+		}
+	}
+
+/**
  * Start CookieComponent for use in the controller
  *
  * @param Controller $controller
@@ -369,10 +390,15 @@ class CookieComponent extends Component {
  * @return void
  */
 	protected function _write($name, $value) {
-		$this->_setcookie(
-			$this->name . $name, $this->_encrypt($value),
-			$this->_expires, $this->path, $this->domain, $this->secure, $this->httpOnly
-		);
+		$this->_response->cookie(array(
+			'name' => $this->name . $name,
+			'value' => $this->_encrypt($value),
+			'expire' => $this->_expires,
+			'path' => $this->path,
+			'domain' => $this->domain,
+			'secure' => $this->secure,
+			'httpOnly' => $this->httpOnly
+		));
 
 		if (!is_null($this->_reset)) {
 			$this->_expires = $this->_reset;
@@ -387,29 +413,15 @@ class CookieComponent extends Component {
  * @return void
  */
 	protected function _delete($name) {
-		$this->_setcookie(
-			$this->name . $name, '',
-			time() - 42000, $this->path, $this->domain, $this->secure, $this->httpOnly
-		);
-	}
-
-/**
- * Object wrapper for setcookie() so it can be mocked in unit tests.
- *
- * @todo Re-factor setting cookies into CakeResponse.  Cookies are part
- * of the HTTP response, and should be handled there.
- *
- * @param string $name Name of the cookie
- * @param string $value Value of the cookie
- * @param integer $expire Time the cookie expires in
- * @param string $path Path the cookie applies to
- * @param string $domain Domain the cookie is for.
- * @param boolean $secure Is the cookie https?
- * @param boolean $httpOnly Is the cookie available in the client?
- * @return void
- */
-	protected function _setcookie($name, $value, $expire, $path, $domain, $secure, $httpOnly = false) {
-		setcookie($name, $value, $expire, $path, $domain, $secure, $httpOnly);
+		$this->_response->cookie(array(
+			'name' => $this->name . $name,
+			'value' => '',
+			'expire' => time() - 42000,
+			'path' => $this->path,
+			'domain' => $this->domain,
+			'secure' => $this->secure,
+			'httpOnly' => $this->httpOnly
+		));
 	}
 
 /**

lib/Cake/Network/CakeResponse.php

@@ -326,6 +326,13 @@ class CakeResponse {
 	protected $_cacheDirectives = array();
 
 /**
+ * Holds cookies to be sent to the client
+ * 
+ * @var array
+ */
+	protected $_cookies = array();
+
+/**
  * Class constructor
  *
  * @param array $options list of parameters to setup the response. Possible values are:
@@ -361,6 +368,7 @@ class CakeResponse {
 		}
 
 		$codeMessage = $this->_statusCodes[$this->_status];
+		$this->_setCookies();
 		$this->_sendHeader("{$this->_protocol} {$this->_status} {$codeMessage}");
 		$this->_setContent();
 		$this->_setContentLength();
@@ -372,6 +380,22 @@ class CakeResponse {
 	}
 
 /**
+ * Sets the cookies that have been added via static method CakeResponse::addCookie()
+ * before any other output is sent to the client.
+ * Will set the cookies in the order they have been set.
+ * 
+ * @return void
+ */
+	protected function _setCookies() {
+		foreach ($this->_cookies as $name => $c) {
+			setcookie(
+				$name, $c['value'], $c['expire'], $c['path'],
+				$c['domain'], $c['secure'], $c['httpOnly']
+			);
+		}
+	}
+
+/**
  * Formats the Content-Type header based on the configured contentType and charset
  * the charset will only be set in the header if the response is of type text/*
  *
@@ -1060,4 +1084,67 @@ class CakeResponse {
 	public function __toString() {
 		return (string)$this->_body;
 	}
-}
+
+/**
+ * Getter/Setter for cookie configs
+ * 
+ * This method acts as a setter/getter depending on the type of the argument.
+ * If the method is called with no arguments, it returns all configurations.
+ * 
+ * If the method is called with a string as argument, it returns either the
+ * given configuration if it is set, or null, if it's not set.
+ * 
+ * If the method is called with an array as argument, it will set the cookie
+ * configuration to the cookie container.
+ * 
+ * ### Options (when setting a configuration)
+ *  - name: The Cookie name
+ *  - value: Value of the cookie
+ *  - expire: Time the cookie expires in
+ *  - path: Path the cookie applies to
+ *  - domain: Domain the cookie is for.
+ *  - secure: Is the cookie https?
+ *  - httpOnly: Is the cookie available in the client?
+ * 
+ * ## Examples
+ * 
+ * ### Getting all cookies
+ * 
+ * `$this->cookie()`
+ * 
+ * ### Getting a certain cookie configuration
+ * 
+ * `$this->cookie('MyCookie')`
+ * 
+ * ### Setting a cookie configuration
+ * 
+ * `$this->cookie((array) $config)`
+ * 
+ * @return mixed
+ */
+	public function cookie($config = null) {
+		if ($config === null) {
+			return $this->_cookies;
+		}
+
+		if (is_string($config)) {
+			if (!isset($this->_cookies[$config])) {
+				return null;
+			}
+			return $this->_cookies[$config];
+		}
+
+		$defaults = array(
+			'name' => 'CakeCookie[default]',
+			'value' => '',
+			'expire' => 0,
+			'path' => '/',
+			'domain' => '',
+			'secure' => false,
+			'httpOnly' => false
+		);
+		$config += $defaults;
+
+		$this->_cookies[$config['name']] = $config;
+	}
+}

lib/Cake/Test/Case/Controller/Component/CookieComponentTest.php

@@ -73,8 +73,8 @@ class CookieComponentTest extends CakeTestCase {
 	public function setUp() {
 		$_COOKIE = array();
 		$Collection = new ComponentCollection();
-		$this->Cookie = $this->getMock('CookieComponent', array('_setcookie'), array($Collection));
-		$this->Controller = new CookieComponentTestController();
+		$this->Cookie = new CookieComponent($Collection);
+		$this->Controller = new CookieComponentTestController(new CakeRequest(), new CakeResponse());
 		$this->Cookie->initialize($this->Controller);
 
 		$this->Cookie->name = 'CakeTestCookie';
@@ -176,8 +176,6 @@ class CookieComponentTest extends CakeTestCase {
  * @return void
  */
 	public function testWriteSimple() {
-		$this->Cookie->expects($this->once())->method('_setcookie');
-
 		$this->Cookie->write('Testing', 'value');
 		$result = $this->Cookie->read('Testing');
 
@@ -192,10 +190,17 @@ class CookieComponentTest extends CakeTestCase {
 	public function testWriteHttpOnly() {
 		$this->Cookie->httpOnly = true;
 		$this->Cookie->secure = false;
-		$this->Cookie->expects($this->once())->method('_setcookie')
-			->with('CakeTestCookie[Testing]', 'value', time() + 10, '/', '', false, true);
-
 		$this->Cookie->write('Testing', 'value', false);
+		$expected = array(
+			'name' => $this->Cookie->name.'[Testing]',
+			'value' => 'value',
+			'expire' => time() + 10,
+			'path' => '/',
+			'domain' => '',
+			'secure' => false,
+			'httpOnly' => true);
+		$result = $this->Controller->response->cookie($this->Cookie->name.'[Testing]');
+		$this->assertEquals($result, $expected);
 	}
 
 /**
@@ -206,10 +211,17 @@ class CookieComponentTest extends CakeTestCase {
 	public function testDeleteHttpOnly() {
 		$this->Cookie->httpOnly = true;
 		$this->Cookie->secure = false;
-		$this->Cookie->expects($this->once())->method('_setcookie')
-			->with('CakeTestCookie[Testing]', '', time() - 42000, '/', '', false, true);
-
 		$this->Cookie->delete('Testing', false);
+		$expected = array(
+			'name' => $this->Cookie->name.'[Testing]',
+			'value' => '',
+			'expire' => time() - 42000,
+			'path' => '/',
+			'domain' => '',
+			'secure' => false,
+			'httpOnly' => true);
+		$result = $this->Controller->response->cookie($this->Cookie->name.'[Testing]');
+		$this->assertEquals($result, $expected);
 	}
 
 /**
@@ -236,10 +248,17 @@ class CookieComponentTest extends CakeTestCase {
  */
 	public function testWriteArrayValues() {
 		$this->Cookie->secure = false;
-		$this->Cookie->expects($this->once())->method('_setcookie')
-			->with('CakeTestCookie[Testing]', '[1,2,3]', time() + 10, '/', '', false, false);
-
 		$this->Cookie->write('Testing', array(1, 2, 3), false);
+		$expected = array(
+			'name' => $this->Cookie->name.'[Testing]',
+			'value' => '[1,2,3]',
+			'expire' => time() + 10,
+			'path' => '/',
+			'domain' => '',
+			'secure' => false,
+			'httpOnly' => false);
+		$result = $this->Controller->response->cookie($this->Cookie->name.'[Testing]');
+		$this->assertEquals($result, $expected);
 	}
 
 /**

lib/Cake/Test/Case/Network/CakeResponseTest.php

@@ -173,22 +173,23 @@ class CakeResponseTest extends CakeTestCase {
 *
 */
 	public function testSend() {
-		$response = $this->getMock('CakeResponse', array('_sendHeader', '_sendContent'));
+		$response = $this->getMock('CakeResponse', array('_sendHeader', '_sendContent', '_setCookies'));
 		$response->header(array(
 			'Content-Language' => 'es',
 			'WWW-Authenticate' => 'Negotiate'
 		));
 		$response->body('the response body');
 		$response->expects($this->once())->method('_sendContent')->with('the response body');
-		$response->expects($this->at(0))
-			->method('_sendHeader')->with('HTTP/1.1 200 OK');
+		$response->expects($this->at(0))->method('_setCookies');
 		$response->expects($this->at(1))
-			->method('_sendHeader')->with('Content-Language', 'es');
+			->method('_sendHeader')->with('HTTP/1.1 200 OK');
 		$response->expects($this->at(2))
-			->method('_sendHeader')->with('WWW-Authenticate', 'Negotiate');
+			->method('_sendHeader')->with('Content-Language', 'es');
 		$response->expects($this->at(3))
-			->method('_sendHeader')->with('Content-Length', 17);
+			->method('_sendHeader')->with('WWW-Authenticate', 'Negotiate');
 		$response->expects($this->at(4))
+			->method('_sendHeader')->with('Content-Length', 17);
+		$response->expects($this->at(5))
 			->method('_sendHeader')->with('Content-Type', 'text/html; charset=UTF-8');
 		$response->send();
 	}
@@ -198,15 +199,16 @@ class CakeResponseTest extends CakeTestCase {
 *
 */
 	public function testSendChangingContentYype() {
-		$response = $this->getMock('CakeResponse', array('_sendHeader', '_sendContent'));
+		$response = $this->getMock('CakeResponse', array('_sendHeader', '_sendContent', '_setCookies'));
 		$response->type('mp3');
 		$response->body('the response body');
 		$response->expects($this->once())->method('_sendContent')->with('the response body');
-		$response->expects($this->at(0))
-			->method('_sendHeader')->with('HTTP/1.1 200 OK');
+		$response->expects($this->at(0))->method('_setCookies');
 		$response->expects($this->at(1))
-			->method('_sendHeader')->with('Content-Length', 17);
+			->method('_sendHeader')->with('HTTP/1.1 200 OK');
 		$response->expects($this->at(2))
+			->method('_sendHeader')->with('Content-Length', 17);
+		$response->expects($this->at(3))
 			->method('_sendHeader')->with('Content-Type', 'audio/mpeg');
 		$response->send();
 	}
@@ -216,15 +218,16 @@ class CakeResponseTest extends CakeTestCase {
 *
 */
 	public function testSendChangingContentType() {
-		$response = $this->getMock('CakeResponse', array('_sendHeader', '_sendContent'));
+		$response = $this->getMock('CakeResponse', array('_sendHeader', '_sendContent', '_setCookies'));
 		$response->type('mp3');
 		$response->body('the response body');
 		$response->expects($this->once())->method('_sendContent')->with('the response body');
-		$response->expects($this->at(0))
-			->method('_sendHeader')->with('HTTP/1.1 200 OK');
+		$response->expects($this->at(0))->method('_setCookies');
 		$response->expects($this->at(1))
-			->method('_sendHeader')->with('Content-Length', 17);
+			->method('_sendHeader')->with('HTTP/1.1 200 OK');
 		$response->expects($this->at(2))
+			->method('_sendHeader')->with('Content-Length', 17);
+		$response->expects($this->at(3))
 			->method('_sendHeader')->with('Content-Type', 'audio/mpeg');
 		$response->send();
 	}
@@ -234,13 +237,14 @@ class CakeResponseTest extends CakeTestCase {
 *
 */
 	public function testSendWithLocation() {
-		$response = $this->getMock('CakeResponse', array('_sendHeader', '_sendContent'));
+		$response = $this->getMock('CakeResponse', array('_sendHeader', '_sendContent', '_setCookies'));
 		$response->header('Location', 'http://www.example.com');
-		$response->expects($this->at(0))
-			->method('_sendHeader')->with('HTTP/1.1 302 Found');
+		$response->expects($this->at(0))->method('_setCookies');
 		$response->expects($this->at(1))
-			->method('_sendHeader')->with('Location', 'http://www.example.com');
+			->method('_sendHeader')->with('HTTP/1.1 302 Found');
 		$response->expects($this->at(2))
+			->method('_sendHeader')->with('Location', 'http://www.example.com');
+		$response->expects($this->at(3))
 			->method('_sendHeader')->with('Content-Type', 'text/html; charset=UTF-8');		
 		$response->send();
 	}
@@ -921,4 +925,87 @@ class CakeResponseTest extends CakeTestCase {
 		$response->expects($this->never())->method('notModified');
 		$this->assertFalse($response->checkNotModified(new CakeRequest));
 	}
+
+/**
+ * Test cookie setting
+ * 
+ * @return void
+ */
+	public function testCookieSettings() {
+		$response = new CakeResponse();
+		$cookie = array(
+			'name' => 'CakeTestCookie[Testing]'
+		);
+		$response->cookie($cookie);
+		$expected = array(
+			'name' => 'CakeTestCookie[Testing]',
+			'value' => '',
+			'expire' => 0,
+			'path' => '/',
+			'domain' => '',
+			'secure' => false,
+			'httpOnly' => false);
+		$result = $response->cookie('CakeTestCookie[Testing]');
+		$this->assertEqual($result, $expected);
+
+		$cookie = array(
+			'name' => 'CakeTestCookie[Testing2]',
+			'value' => '[a,b,c]',
+			'expire' => 1000,
+			'path' => '/test',
+			'secure' => true
+		);
+		$response->cookie($cookie);
+		$expected = array(
+			'CakeTestCookie[Testing]' => array(
+				'name' => 'CakeTestCookie[Testing]',
+				'value' => '',
+				'expire' => 0,
+				'path' => '/',
+				'domain' => '',
+				'secure' => false,
+				'httpOnly' => false
+			),
+			'CakeTestCookie[Testing2]' => array(
+				'name' => 'CakeTestCookie[Testing2]',
+				'value' => '[a,b,c]',
+				'expire' => 1000,
+				'path' => '/test',
+				'domain' => '',
+				'secure' => true,
+				'httpOnly' => false
+			)
+		);
+
+		$result = $response->cookie();
+		$this->assertEqual($result, $expected);
+
+		$cookie = $expected['CakeTestCookie[Testing]'];
+		$cookie['value'] = 'test';
+		$response->cookie($cookie);
+		$expected = array(
+			'CakeTestCookie[Testing]' => array(
+				'name' => 'CakeTestCookie[Testing]',
+				'value' => 'test',
+				'expire' => 0,
+				'path' => '/',
+				'domain' => '',
+				'secure' => false,
+				'httpOnly' => false
+			),
+			'CakeTestCookie[Testing2]' => array(
+				'name' => 'CakeTestCookie[Testing2]',
+				'value' => '[a,b,c]',
+				'expire' => 1000,
+				'path' => '/test',
+				'domain' => '',
+				'secure' => true,
+				'httpOnly' => false
+			)
+		);
+
+		$result = $response->cookie();
+		$this->assertEqual($result, $expected);
+	}
+
 }