From 9c7f357029bd7526b30ddfe993f1c3afe4e7373d Mon Sep 17 00:00:00 2001 From: Nicolas Date: Sat, 17 Nov 2012 12:39:08 +0100 Subject: [PATCH 1/7] Check if bool operators in find conditions are empty. --- lib/Cake/Model/Datasource/DboSource.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/Cake/Model/Datasource/DboSource.php b/lib/Cake/Model/Datasource/DboSource.php index 623b7a01f..f4fe7d777 100644 --- a/lib/Cake/Model/Datasource/DboSource.php +++ b/lib/Cake/Model/Datasource/DboSource.php @@ -2464,6 +2464,10 @@ class DboSource extends DataSource { $not = 'NOT '; } + if (empty($value)) { + continue; + } + if (empty($value[1])) { if ($not) { $out[] = $not . '(' . $value[0] . ')'; From 1b704fb7cf4d3b18e883a9979fe0e87983c7dec4 Mon Sep 17 00:00:00 2001 From: Nicolas Date: Sat, 17 Nov 2012 19:34:02 +0100 Subject: [PATCH 2/7] Unit tests added. --- .../Case/Model/Datasource/DboSourceTest.php | 82 +++---------------- 1 file changed, 13 insertions(+), 69 deletions(-) diff --git a/lib/Cake/Test/Case/Model/Datasource/DboSourceTest.php b/lib/Cake/Test/Case/Model/Datasource/DboSourceTest.php index c38e1d085..0faec562a 100644 --- a/lib/Cake/Test/Case/Model/Datasource/DboSourceTest.php +++ b/lib/Cake/Test/Case/Model/Datasource/DboSourceTest.php @@ -159,6 +159,19 @@ class DboSourceTest extends CakeTestCase { $this->assertEquals(' WHERE 1 = 1', $result); } +/** + * test that booleans work on empty set. + * + * @return void + */ + public function testBooleanEmptyConditionsParsing() { + $result = $this->testDb->conditions(array('OR' => array())); + $this->assertEquals(' WHERE 1 = 1', $result, 'empty conditions failed %s'); + + $result = $this->testDb->conditions(array('OR' => array('OR' => array()))); + $this->assertEquals(' WHERE 1 = 1', $result, 'nested empty conditions failed %s'); + } + /** * test that order() will accept objects made from DboSource::expression * @@ -1103,73 +1116,4 @@ class DboSourceTest extends CakeTestCase { $this->assertEquals($expected, $result); } -/** - * Test conditionKeysToString() - * - * @return void - */ - public function testConditionKeysToString() { - $Article = ClassRegistry::init('Article'); - $conn = $this->getMock('MockPDO', array('quote')); - $db = new DboTestSource; - $db->setConnection($conn); - - $conn->expects($this->at(0)) - ->method('quote') - ->will($this->returnValue('just text')); - - $conditions = array('Article.name' => 'just text'); - $result = $db->conditionKeysToString($conditions, true, $Article); - $expected = "Article.name = just text"; - $this->assertEquals($expected, $result[0]); - - $conn->expects($this->at(0)) - ->method('quote') - ->will($this->returnValue('just text')); - $conn->expects($this->at(1)) - ->method('quote') - ->will($this->returnValue('other text')); - - $conditions = array('Article.name' => array('just text', 'other text')); - $result = $db->conditionKeysToString($conditions, true, $Article); - $expected = "Article.name IN (just text, other text)"; - $this->assertEquals($expected, $result[0]); - } - -/** - * Test conditionKeysToString() with virtual field - * - * @return void - */ - public function testConditionKeysToStringVirtualField() { - $Article = ClassRegistry::init('Article'); - $Article->virtualFields = array( - 'extra' => 'something virtual' - ); - $conn = $this->getMock('MockPDO', array('quote')); - $db = new DboTestSource; - $db->setConnection($conn); - - $conn->expects($this->at(0)) - ->method('quote') - ->will($this->returnValue('just text')); - - $conditions = array('Article.extra' => 'just text'); - $result = $db->conditionKeysToString($conditions, true, $Article); - $expected = "(" . $Article->virtualFields['extra'] . ") = just text"; - $this->assertEquals($expected, $result[0]); - - $conn->expects($this->at(0)) - ->method('quote') - ->will($this->returnValue('just text')); - $conn->expects($this->at(1)) - ->method('quote') - ->will($this->returnValue('other text')); - - $conditions = array('Article.extra' => array('just text', 'other text')); - $result = $db->conditionKeysToString($conditions, true, $Article); - $expected = "(" . $Article->virtualFields['extra'] . ") IN (just text, other text)"; - $this->assertEquals($expected, $result[0]); - } - } From a77e46cbfda7786e1ff51f84995e7b90900c77d4 Mon Sep 17 00:00:00 2001 From: Nicolas Date: Sat, 17 Nov 2012 23:14:40 +0100 Subject: [PATCH 3/7] Added accidentally removed tests back in. --- .../Case/Model/Datasource/DboSourceTest.php | 73 ++++++++++++++++++- 1 file changed, 71 insertions(+), 2 deletions(-) diff --git a/lib/Cake/Test/Case/Model/Datasource/DboSourceTest.php b/lib/Cake/Test/Case/Model/Datasource/DboSourceTest.php index 0faec562a..e049be480 100644 --- a/lib/Cake/Test/Case/Model/Datasource/DboSourceTest.php +++ b/lib/Cake/Test/Case/Model/Datasource/DboSourceTest.php @@ -166,10 +166,10 @@ class DboSourceTest extends CakeTestCase { */ public function testBooleanEmptyConditionsParsing() { $result = $this->testDb->conditions(array('OR' => array())); - $this->assertEquals(' WHERE 1 = 1', $result, 'empty conditions failed %s'); + $this->assertEquals(' WHERE 1 = 1', $result, 'empty conditions failed'); $result = $this->testDb->conditions(array('OR' => array('OR' => array()))); - $this->assertEquals(' WHERE 1 = 1', $result, 'nested empty conditions failed %s'); + $this->assertEquals(' WHERE 1 = 1', $result, 'nested empty conditions failed'); } /** @@ -1116,4 +1116,73 @@ class DboSourceTest extends CakeTestCase { $this->assertEquals($expected, $result); } +/** + * Test conditionKeysToString() + * + * @return void + */ + public function testConditionKeysToString() { + $Article = ClassRegistry::init('Article'); + $conn = $this->getMock('MockPDO', array('quote')); + $db = new DboTestSource; + $db->setConnection($conn); + + $conn->expects($this->at(0)) + ->method('quote') + ->will($this->returnValue('just text')); + + $conditions = array('Article.name' => 'just text'); + $result = $db->conditionKeysToString($conditions, true, $Article); + $expected = "Article.name = just text"; + $this->assertEquals($expected, $result[0]); + + $conn->expects($this->at(0)) + ->method('quote') + ->will($this->returnValue('just text')); + $conn->expects($this->at(1)) + ->method('quote') + ->will($this->returnValue('other text')); + + $conditions = array('Article.name' => array('just text', 'other text')); + $result = $db->conditionKeysToString($conditions, true, $Article); + $expected = "Article.name IN (just text, other text)"; + $this->assertEquals($expected, $result[0]); + } + +/** + * Test conditionKeysToString() with virtual field + * + * @return void + */ + public function testConditionKeysToStringVirtualField() { + $Article = ClassRegistry::init('Article'); + $Article->virtualFields = array( + 'extra' => 'something virtual' + ); + $conn = $this->getMock('MockPDO', array('quote')); + $db = new DboTestSource; + $db->setConnection($conn); + + $conn->expects($this->at(0)) + ->method('quote') + ->will($this->returnValue('just text')); + + $conditions = array('Article.extra' => 'just text'); + $result = $db->conditionKeysToString($conditions, true, $Article); + $expected = "(" . $Article->virtualFields['extra'] . ") = just text"; + $this->assertEquals($expected, $result[0]); + + $conn->expects($this->at(0)) + ->method('quote') + ->will($this->returnValue('just text')); + $conn->expects($this->at(1)) + ->method('quote') + ->will($this->returnValue('other text')); + + $conditions = array('Article.extra' => array('just text', 'other text')); + $result = $db->conditionKeysToString($conditions, true, $Article); + $expected = "(" . $Article->virtualFields['extra'] . ") IN (just text, other text)"; + $this->assertEquals($expected, $result[0]); + } + } From 0d298614239072f9a817f7b903d5472ef913b977 Mon Sep 17 00:00:00 2001 From: mark_story Date: Wed, 21 Nov 2012 21:52:23 -0500 Subject: [PATCH 4/7] Fix order for finds This should solve more occasional errors in postgres on travis. --- lib/Cake/Test/Case/Model/ModelReadTest.php | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/lib/Cake/Test/Case/Model/ModelReadTest.php b/lib/Cake/Test/Case/Model/ModelReadTest.php index 6d4b29c24..65e7f90eb 100644 --- a/lib/Cake/Test/Case/Model/ModelReadTest.php +++ b/lib/Cake/Test/Case/Model/ModelReadTest.php @@ -5400,7 +5400,9 @@ class ModelReadTest extends BaseModelTest { )); $Post->Tag->primaryKey = 'tag'; - $result = $Post->find('all'); + $result = $Post->find('all', array( + 'order' => 'Post.id ASC', + )); $expected = array( array( 'Post' => array( @@ -5615,7 +5617,9 @@ class ModelReadTest extends BaseModelTest { $Project = new Project(); $Project->recursive = 3; - $result = $Project->find('all'); + $result = $Project->find('all', array( + 'order' => 'Project.id ASC', + )); $expected = array( array( 'Project' => array( @@ -5729,7 +5733,9 @@ class ModelReadTest extends BaseModelTest { $TestModel = new Home(); $TestModel->recursive = 2; - $result = $TestModel->find('all'); + $result = $TestModel->find('all', array( + 'order' => 'Home.id ASC', + )); $expected = array( array( 'Home' => array( @@ -5842,7 +5848,9 @@ class ModelReadTest extends BaseModelTest { $MyUser = new MyUser(); $MyUser->recursive = 2; - $result = $MyUser->find('all'); + $result = $MyUser->find('all', array( + 'order' => 'MyUser.id ASC' + )); $expected = array( array( 'MyUser' => array('id' => '1', 'firstname' => 'userA'), @@ -6033,7 +6041,7 @@ class ModelReadTest extends BaseModelTest { $fullDebug = $this->db->fullDebug; $this->db->fullDebug = true; $TestModel->recursive = 6; - $result = $TestModel->find('all', null, null, 'CategoryThread.id ASC'); + $result = $TestModel->find('all'); $expected = array( array( 'CategoryThread' => array( From 8ef30344825686e9bdce78caccc87c0b75a6cf2e Mon Sep 17 00:00:00 2001 From: Sam Mousa Date: Sat, 24 Nov 2012 12:22:17 +0100 Subject: [PATCH 5/7] Added the table name to the cacheKey used by DboSource::fields. #3394 --- lib/Cake/Model/Datasource/DboSource.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Cake/Model/Datasource/DboSource.php b/lib/Cake/Model/Datasource/DboSource.php index 623b7a01f..8cf997a78 100644 --- a/lib/Cake/Model/Datasource/DboSource.php +++ b/lib/Cake/Model/Datasource/DboSource.php @@ -2282,7 +2282,8 @@ class DboSource extends DataSource { $virtualFields, $fields, $quote, - ConnectionManager::getSourceName($this) + ConnectionManager::getSourceName($this), + $model->table ); $cacheKey = md5(serialize($cacheKey)); if ($return = $this->cacheMethod(__FUNCTION__, $cacheKey)) { From e04e0a0ec89ba95eaaa6dc51b1612a2909722018 Mon Sep 17 00:00:00 2001 From: mark_story Date: Wed, 21 Nov 2012 22:26:37 -0500 Subject: [PATCH 6/7] Use non-mocked methods for tests This makes reducing off by 1 second errors much easier. --- lib/Cake/Test/Case/View/MediaViewTest.php | 122 +++++++++------------- 1 file changed, 47 insertions(+), 75 deletions(-) diff --git a/lib/Cake/Test/Case/View/MediaViewTest.php b/lib/Cake/Test/Case/View/MediaViewTest.php index 8660a119b..a027c60e1 100644 --- a/lib/Cake/Test/Case/View/MediaViewTest.php +++ b/lib/Cake/Test/Case/View/MediaViewTest.php @@ -36,7 +36,10 @@ class MediaViewTest extends CakeTestCase { public function setUp() { parent::setUp(); $this->MediaView = $this->getMock('MediaView', array('_isActive', '_clearBuffer', '_flushBuffer')); - $this->MediaView->response = $this->getMock('CakeResponse'); + $this->MediaView->response = $this->getMock( + 'CakeResponse', + array('send', 'cache', 'type', 'download', 'statusCode') + ); } /** @@ -83,20 +86,6 @@ class MediaViewTest extends CakeTestCase { ->with('css') ->will($this->returnArgument(0)); - $this->MediaView->response->expects($this->at(1)) - ->method('header') - ->with(array( - 'Date' => gmdate('D, d M Y H:i:s', time()) . ' GMT', - 'Expires' => '0', - 'Cache-Control' => 'private, must-revalidate, post-check=0, pre-check=0', - 'Pragma' => 'no-cache' - )); - - $this->MediaView->response->expects($this->at(2)) - ->method('header') - ->with(array( - 'Content-Length' => 31 - )); $this->MediaView->response->expects($this->once())->method('send'); $this->MediaView->expects($this->once())->method('_clearBuffer'); $this->MediaView->expects($this->once())->method('_flushBuffer'); @@ -106,6 +95,16 @@ class MediaViewTest extends CakeTestCase { $output = ob_get_clean(); $this->assertEquals('this is the test asset css file', $output); $this->assertTrue($result !== false); + + $headers = $this->MediaView->response->header(); + $this->assertEquals(31, $headers['Content-Length']); + $this->assertEquals(0, $headers['Expires']); + $this->assertEquals( + 'private, must-revalidate, post-check=0, pre-check=0', + $headers['Cache-Control'] + ); + $this->assertEquals('no-cache', $headers['Pragma']); + $this->assertContains(gmdate('D, d M Y H:i', time()), $headers['Date']); } /** @@ -130,32 +129,10 @@ class MediaViewTest extends CakeTestCase { ->with('ini') ->will($this->returnValue(false)); - $this->MediaView->response->expects($this->at(1)) - ->method('header') - ->with($this->logicalAnd( - $this->arrayHasKey('Date'), - $this->arrayHasKey('Expires'), - $this->arrayHasKey('Cache-Control'), - $this->arrayHasKey('Pragma'), - $this->contains('0'), - $this->contains('private, must-revalidate, post-check=0, pre-check=0'), - $this->contains('no-cache') - )); - $this->MediaView->response->expects($this->once()) ->method('download') ->with('no_section.ini'); - $this->MediaView->response->expects($this->at(3)) - ->method('header') - ->with(array( - 'Accept-Ranges' => 'bytes' - )); - - $this->MediaView->response->expects($this->at(4)) - ->method('header') - ->with('Content-Length', 35); - $this->MediaView->response->expects($this->once())->method('send'); $this->MediaView->expects($this->once())->method('_clearBuffer'); $this->MediaView->expects($this->once())->method('_flushBuffer'); @@ -168,6 +145,17 @@ class MediaViewTest extends CakeTestCase { if ($currentUserAgent !== null) { $_SERVER['HTTP_USER_AGENT'] = $currentUserAgent; } + + $headers = $this->MediaView->response->header(); + $this->assertEquals(35, $headers['Content-Length']); + $this->assertEquals(0, $headers['Expires']); + $this->assertEquals('bytes', $headers['Accept-Ranges']); + $this->assertEquals( + 'private, must-revalidate, post-check=0, pre-check=0', + $headers['Cache-Control'] + ); + $this->assertEquals('no-cache', $headers['Pragma']); + $this->assertContains(gmdate('D, d M Y H:i', time()), $headers['Date']); } /** @@ -193,15 +181,6 @@ class MediaViewTest extends CakeTestCase { ->will($this->returnValue(false)); $this->MediaView->response->expects($this->at(1)) - ->method('header') - ->with(array( - 'Date' => gmdate('D, d M Y H:i:s', time()) . ' GMT', - 'Expires' => '0', - 'Cache-Control' => 'private, must-revalidate, post-check=0, pre-check=0', - 'Pragma' => 'no-cache' - )); - - $this->MediaView->response->expects($this->at(2)) ->method('type') ->with('application/octetstream') ->will($this->returnValue(false)); @@ -210,16 +189,6 @@ class MediaViewTest extends CakeTestCase { ->method('download') ->with('no_section.ini'); - $this->MediaView->response->expects($this->at(4)) - ->method('header') - ->with(array( - 'Accept-Ranges' => 'bytes' - )); - - $this->MediaView->response->expects($this->at(5)) - ->method('header') - ->with('Content-Length', 35); - $this->MediaView->response->expects($this->once())->method('send'); $this->MediaView->expects($this->once())->method('_clearBuffer'); $this->MediaView->expects($this->once())->method('_flushBuffer'); @@ -232,6 +201,17 @@ class MediaViewTest extends CakeTestCase { if ($currentUserAgent !== null) { $_SERVER['HTTP_USER_AGENT'] = $currentUserAgent; } + + $headers = $this->MediaView->response->header(); + $this->assertEquals(35, $headers['Content-Length']); + $this->assertEquals(0, $headers['Expires']); + $this->assertEquals('bytes', $headers['Accept-Ranges']); + $this->assertEquals( + 'private, must-revalidate, post-check=0, pre-check=0', + $headers['Cache-Control'] + ); + $this->assertEquals('no-cache', $headers['Pragma']); + $this->assertContains(gmdate('D, d M Y H:i', time()), $headers['Date']); } /** @@ -258,15 +238,6 @@ class MediaViewTest extends CakeTestCase { ->will($this->returnValue(false)); $this->MediaView->response->expects($this->at(1)) - ->method('header') - ->with(array( - 'Date' => gmdate('D, d M Y H:i:s', time()) . ' GMT', - 'Expires' => '0', - 'Cache-Control' => 'private, must-revalidate, post-check=0, pre-check=0', - 'Pragma' => 'no-cache' - )); - - $this->MediaView->response->expects($this->at(2)) ->method('type') ->with('application/force-download') ->will($this->returnValue(false)); @@ -275,16 +246,6 @@ class MediaViewTest extends CakeTestCase { ->method('download') ->with('config.ini'); - $this->MediaView->response->expects($this->at(4)) - ->method('header') - ->with(array( - 'Accept-Ranges' => 'bytes' - )); - - $this->MediaView->response->expects($this->at(5)) - ->method('header') - ->with('Content-Length', 35); - $this->MediaView->response->expects($this->once())->method('send'); $this->MediaView->expects($this->once())->method('_clearBuffer'); $this->MediaView->expects($this->once())->method('_flushBuffer'); @@ -297,6 +258,17 @@ class MediaViewTest extends CakeTestCase { if ($currentUserAgent !== null) { $_SERVER['HTTP_USER_AGENT'] = $currentUserAgent; } + + $headers = $this->MediaView->response->header(); + $this->assertEquals(35, $headers['Content-Length']); + $this->assertEquals(0, $headers['Expires']); + $this->assertEquals('bytes', $headers['Accept-Ranges']); + $this->assertEquals( + 'private, must-revalidate, post-check=0, pre-check=0', + $headers['Cache-Control'] + ); + $this->assertEquals('no-cache', $headers['Pragma']); + $this->assertContains(gmdate('D, d M Y H:i', time()), $headers['Date']); } /** From 3083b01f7d6a756416d493a8fc8c23819e33c20b Mon Sep 17 00:00:00 2001 From: mark_story Date: Sat, 24 Nov 2012 15:38:42 -0500 Subject: [PATCH 7/7] Fix exceptions being thrown in beforeFilter breaking error pages. If an exception was raised in the AppController::beforeFilter(), requests for content-type responses would render as HTML. Extracting the startupProcess() allows us to keep a reference to the error controller, which can be used to force startup RequestHandlerComponent if its enabled. Fixes #3389 --- lib/Cake/Controller/CakeErrorController.php | 2 -- lib/Cake/Error/ExceptionRenderer.php | 4 ++++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/Cake/Controller/CakeErrorController.php b/lib/Cake/Controller/CakeErrorController.php index c1835fd6e..53f500217 100644 --- a/lib/Cake/Controller/CakeErrorController.php +++ b/lib/Cake/Controller/CakeErrorController.php @@ -66,8 +66,6 @@ class CakeErrorController extends AppController { if ($this->Components->enabled('Security')) { $this->Components->disable('Security'); } - $this->startupProcess(); - $this->_set(array('cacheAction' => false, 'viewPath' => 'Errors')); } diff --git a/lib/Cake/Error/ExceptionRenderer.php b/lib/Cake/Error/ExceptionRenderer.php index 3be08d26c..b315a36cd 100644 --- a/lib/Cake/Error/ExceptionRenderer.php +++ b/lib/Cake/Error/ExceptionRenderer.php @@ -150,7 +150,11 @@ class ExceptionRenderer { $response = new CakeResponse(array('charset' => Configure::read('App.encoding'))); try { $controller = new CakeErrorController($request, $response); + $controller->startupProcess(); } catch (Exception $e) { + if (!empty($controller) && $controller->Components->enabled('RequestHandler')) { + $controller->RequestHandler->startup($controller); + } } if (empty($controller)) { $controller = new Controller($request, $response);