From 4ec195f9c87ab71d8b12ba6225d8d3e29568c981 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kurre=20Sta=CC=8Ahlberg?= Date: Tue, 27 Jun 2017 11:46:51 +0300 Subject: [PATCH 01/18] Fix error when default value is reported as CURRENT_TIMESTAMP() with parenthesis --- lib/Cake/Model/Datasource/Database/Mysql.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Cake/Model/Datasource/Database/Mysql.php b/lib/Cake/Model/Datasource/Database/Mysql.php index 95a89ec16..3a9c12961 100644 --- a/lib/Cake/Model/Datasource/Database/Mysql.php +++ b/lib/Cake/Model/Datasource/Database/Mysql.php @@ -352,7 +352,8 @@ class Mysql extends DboSource { if (in_array($fields[$column->Field]['type'], $this->fieldParameters['unsigned']['types'], true)) { $fields[$column->Field]['unsigned'] = $this->_unsigned($column->Type); } - if (in_array($fields[$column->Field]['type'], array('timestamp', 'datetime')) && strtoupper($column->Default) === 'CURRENT_TIMESTAMP') { + if (in_array($fields[$column->Field]['type'], array('timestamp', 'datetime')) && + in_array(strtoupper($column->Default), array('CURRENT_TIMESTAMP', 'CURRENT_TIMESTAMP()'))) { $fields[$column->Field]['default'] = null; } if (!empty($column->Key) && isset($this->index[$column->Key])) { From 32f6b96060217f36817ffabce12118ab01cd5757 Mon Sep 17 00:00:00 2001 From: mark_story Date: Sun, 2 Jul 2017 11:06:39 -0400 Subject: [PATCH 02/18] Fix formatting. --- lib/Cake/Model/Datasource/Database/Mysql.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Cake/Model/Datasource/Database/Mysql.php b/lib/Cake/Model/Datasource/Database/Mysql.php index 3a9c12961..7d7c2df25 100644 --- a/lib/Cake/Model/Datasource/Database/Mysql.php +++ b/lib/Cake/Model/Datasource/Database/Mysql.php @@ -353,7 +353,8 @@ class Mysql extends DboSource { $fields[$column->Field]['unsigned'] = $this->_unsigned($column->Type); } if (in_array($fields[$column->Field]['type'], array('timestamp', 'datetime')) && - in_array(strtoupper($column->Default), array('CURRENT_TIMESTAMP', 'CURRENT_TIMESTAMP()'))) { + in_array(strtoupper($column->Default), array('CURRENT_TIMESTAMP', 'CURRENT_TIMESTAMP()')) + ) { $fields[$column->Field]['default'] = null; } if (!empty($column->Key) && isset($this->index[$column->Key])) { From 31fd4217b1de33855202f9eb29f82c7d46a80147 Mon Sep 17 00:00:00 2001 From: Val Bancer Date: Tue, 4 Jul 2017 23:01:17 +0200 Subject: [PATCH 03/18] more PaginatorComponent unit tests --- .../Component/PaginatorComponentTest.php | 96 ++++++++++++++++++- 1 file changed, 95 insertions(+), 1 deletion(-) diff --git a/lib/Cake/Test/Case/Controller/Component/PaginatorComponentTest.php b/lib/Cake/Test/Case/Controller/Component/PaginatorComponentTest.php index 8499f3912..5b30514f3 100644 --- a/lib/Cake/Test/Case/Controller/Component/PaginatorComponentTest.php +++ b/lib/Cake/Test/Case/Controller/Component/PaginatorComponentTest.php @@ -23,6 +23,8 @@ App::uses('PaginatorComponent', 'Controller/Component'); App::uses('CakeRequest', 'Network'); App::uses('CakeResponse', 'Network'); +require_once dirname(dirname(dirname(__FILE__))) . DS . 'Model' . DS . 'models.php'; + /** * PaginatorTestController class * @@ -288,7 +290,8 @@ class PaginatorComponentTest extends CakeTestCase { * * @var array */ - public $fixtures = array('core.post', 'core.comment', 'core.author'); + public $fixtures = array('core.post', 'core.comment', 'core.author', + 'core.translated_article', 'core.translate_article', 'core.user'); /** * setup @@ -402,6 +405,97 @@ class PaginatorComponentTest extends CakeTestCase { $this->assertSame(2, $Controller->params['paging']['PaginatorControllerPost']['count']); } + public function testPaginateNotCondition() { + $Controller = new PaginatorTestController($this->request); + $Controller->uses = array('PaginatorControllerPost', 'PaginatorControllerComment'); + $Controller->request->params['pass'] = array('1'); + $Controller->request->query = array(); + $Controller->constructClasses(); + $Controller->Paginator->settings = array( + 'conditions' => array( + 'NOT' => array('PaginatorAuthor.user' => 'mariano'), + ), + ); + $result = $Controller->Paginator->paginate('PaginatorControllerPost'); + $this->assertEquals('larry', $result[0]['PaginatorAuthor']['user']); + $this->assertCount(1, $result); + } + + public function testPaginateI18nConditionUser() { + $Request = new CakeRequest('articles/index'); + $Controller = new PaginatorTestController($Request); + $Controller->uses = array('TranslatedArticle'); + $Controller->constructClasses(); + $Controller->TranslatedArticle->locale = 'eng'; + $Controller->Paginator->settings = array( + 'recursive' => 0, + 'conditions' => array( + 'User.user' => 'mariano', + ), + ); + $result = $Controller->Paginator->paginate('TranslatedArticle'); + $this->assertEquals('Title (eng) #1', $result[0]['TranslatedArticle']['title']); + $this->assertEquals('mariano', $result[0]['User']['user']); + $this->assertEquals('Title (eng) #3', $result[1]['TranslatedArticle']['title']); + $this->assertEquals('mariano', $result[1]['User']['user']); + $this->assertCount(2, $result); + } + + public function testPaginateI18nConditionTitle() { + $Request = new CakeRequest('articles/index'); + $Controller = new PaginatorTestController($Request); + $Controller->uses = array('TranslatedArticle'); + $Controller->constructClasses(); + $Controller->TranslatedArticle->locale = 'eng'; + $Controller->Paginator->settings = array( + 'recursive' => 0, + 'conditions' => array( + 'I18n__title.content' => 'Title (eng) #3', + ), + ); + $result = $Controller->Paginator->paginate('TranslatedArticle'); + $this->assertEquals('Title (eng) #3', $result[0]['TranslatedArticle']['title']); + $this->assertCount(1, $result); + } + + public function testPaginateI18nConditionNotTitle() { + $Request = new CakeRequest('articles/index'); + $Controller = new PaginatorTestController($Request); + $Controller->uses = array('TranslatedArticle'); + $Controller->constructClasses(); + $Controller->TranslatedArticle->locale = 'eng'; + $Controller->Paginator->settings = array( + 'recursive' => 0, + 'conditions' => array( + 'I18n__title.content !=' => '', + ), + ); + $result = $Controller->Paginator->paginate('TranslatedArticle'); + $this->assertEquals('Title (eng) #1', $result[0]['TranslatedArticle']['title']); + $this->assertEquals('Title (eng) #2', $result[1]['TranslatedArticle']['title']); + $this->assertEquals('Title (eng) #3', $result[2]['TranslatedArticle']['title']); + $this->assertCount(3, $result); + } + + public function testPaginateI18nConditionNotTitleArraySyntax() { + $Request = new CakeRequest('articles/index'); + $Controller = new PaginatorTestController($Request); + $Controller->uses = array('TranslatedArticle'); + $Controller->constructClasses(); + $Controller->TranslatedArticle->locale = 'eng'; + $Controller->Paginator->settings = array( + 'recursive' => 0, + 'conditions' => array( + 'NOT' => array('I18n__title.content' => ''), + ), + ); + $result = $Controller->Paginator->paginate('TranslatedArticle'); + $this->assertEquals('Title (eng) #1', $result[0]['TranslatedArticle']['title']); + $this->assertEquals('Title (eng) #2', $result[1]['TranslatedArticle']['title']); + $this->assertEquals('Title (eng) #3', $result[2]['TranslatedArticle']['title']); + $this->assertCount(3, $result); + } + /** * Test that non-numeric values are rejected for page, and limit * From 50334679d649b03a0b29051f75bc6f9c1b6b6097 Mon Sep 17 00:00:00 2001 From: Val Bancer Date: Wed, 5 Jul 2017 22:40:41 +0200 Subject: [PATCH 04/18] added a unit test --- .../Component/PaginatorComponentTest.php | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/lib/Cake/Test/Case/Controller/Component/PaginatorComponentTest.php b/lib/Cake/Test/Case/Controller/Component/PaginatorComponentTest.php index 5b30514f3..6bc0a1e4c 100644 --- a/lib/Cake/Test/Case/Controller/Component/PaginatorComponentTest.php +++ b/lib/Cake/Test/Case/Controller/Component/PaginatorComponentTest.php @@ -496,6 +496,26 @@ class PaginatorComponentTest extends CakeTestCase { $this->assertCount(3, $result); } + public function testPaginateI18nConditionNotTitleWithLimit() { + $Request = new CakeRequest('articles/index'); + $Controller = new PaginatorTestController($Request); + $Controller->uses = array('TranslatedArticle'); + $Controller->constructClasses(); + $Controller->TranslatedArticle->locale = 'eng'; + $Controller->Paginator->settings = array( + 'recursive' => 0, + 'conditions' => array( + 'NOT' => array('I18n__title.content' => ''), + ), + 'limit' => 2, + ); + $result = $Controller->Paginator->paginate('TranslatedArticle'); + $this->assertEquals('Title (eng) #1', $result[0]['TranslatedArticle']['title']); + $this->assertEquals('Title (eng) #2', $result[1]['TranslatedArticle']['title']); + $this->assertEquals('Title (eng) #3', $result[2]['TranslatedArticle']['title']); + $this->assertCount(3, $result); + } + /** * Test that non-numeric values are rejected for page, and limit * From 85e0ebd7fd65e1e44cd0349daf589797f5445185 Mon Sep 17 00:00:00 2001 From: Val Bancer Date: Wed, 5 Jul 2017 23:22:58 +0200 Subject: [PATCH 05/18] more unit tests added --- .../Component/PaginatorComponentTest.php | 5 +- lib/Cake/Test/Case/Model/ModelReadTest.php | 72 +++++++++++++++++++ lib/Cake/Test/Case/Model/ModelTestBase.php | 1 + 3 files changed, 75 insertions(+), 3 deletions(-) diff --git a/lib/Cake/Test/Case/Controller/Component/PaginatorComponentTest.php b/lib/Cake/Test/Case/Controller/Component/PaginatorComponentTest.php index 6bc0a1e4c..4a980630b 100644 --- a/lib/Cake/Test/Case/Controller/Component/PaginatorComponentTest.php +++ b/lib/Cake/Test/Case/Controller/Component/PaginatorComponentTest.php @@ -505,15 +505,14 @@ class PaginatorComponentTest extends CakeTestCase { $Controller->Paginator->settings = array( 'recursive' => 0, 'conditions' => array( - 'NOT' => array('I18n__title.content' => ''), + 'NOT' => array('I18n__title.content' => ''), ), 'limit' => 2, ); $result = $Controller->Paginator->paginate('TranslatedArticle'); $this->assertEquals('Title (eng) #1', $result[0]['TranslatedArticle']['title']); $this->assertEquals('Title (eng) #2', $result[1]['TranslatedArticle']['title']); - $this->assertEquals('Title (eng) #3', $result[2]['TranslatedArticle']['title']); - $this->assertCount(3, $result); + $this->assertCount(2, $result); } /** diff --git a/lib/Cake/Test/Case/Model/ModelReadTest.php b/lib/Cake/Test/Case/Model/ModelReadTest.php index 11e9ffa90..bac344d04 100644 --- a/lib/Cake/Test/Case/Model/ModelReadTest.php +++ b/lib/Cake/Test/Case/Model/ModelReadTest.php @@ -6685,6 +6685,62 @@ class ModelReadTest extends BaseModelTest { $this->assertEquals($expected, $result); } + public function testFindAllI18nConditions() { + $this->loadFixtures('TranslateArticle', 'TranslatedArticle', 'User'); + $TestModel = new TranslatedArticle(); + $TestModel->cacheQueries = false; + $TestModel->locale = 'eng'; + $options = array( + 'recursive' => 0, + 'conditions' => array( + 'NOT' => array('I18n__title.content' => ''), + ), + 'limit' => 2, + ); + $result = $TestModel->find('all', $options); + $expected = array( + array( + 'TranslatedArticle' => array( + 'id' => '1', + 'user_id' => '1', + 'published' => 'Y', + 'created' => '2007-03-18 10:39:23', + 'updated' => '2007-03-18 10:41:31', + 'locale' => 'eng', + 'title' => 'Title (eng) #1', + 'body' => 'Body (eng) #1', + ), + 'User' => array( + 'id' => '1', + 'user' => 'mariano', + 'password' => '5f4dcc3b5aa765d61d8327deb882cf99', + 'created' => '2007-03-17 01:16:23', + 'updated' => '2007-03-17 01:18:31', + ), + ), + array( + 'TranslatedArticle' => array( + 'id' => '2', + 'user_id' => '3', + 'published' => 'Y', + 'created' => '2007-03-18 10:41:23', + 'updated' => '2007-03-18 10:43:31', + 'locale' => 'eng', + 'title' => 'Title (eng) #2', + 'body' => 'Body (eng) #2', + ), + 'User' => array( + 'id' => '3', + 'user' => 'larry', + 'password' => '5f4dcc3b5aa765d61d8327deb882cf99', + 'created' => '2007-03-17 01:20:23', + 'updated' => '2007-03-17 01:22:31', + ), + ), + ); + $this->assertEquals($expected, $result); + } + /** * test find('list') method * @@ -7118,6 +7174,22 @@ class ModelReadTest extends BaseModelTest { $this->assertEquals($expected, $result); } + public function testFindCountI18nConditions() { + $this->loadFixtures('TranslateArticle', 'TranslatedArticle', 'User'); + $TestModel = new TranslatedArticle(); + $TestModel->cacheQueries = false; + $TestModel->locale = 'eng'; + $options = array( + 'recursive' => 0, + 'conditions' => array( + 'NOT' => array('I18n__title.content' => ''), + ), + 'limit' => 2, + ); + $result = $TestModel->find('count', $options); + $this->assertEquals(2, $result); + } + /** * Test that find('first') does not use the id set to the object. * diff --git a/lib/Cake/Test/Case/Model/ModelTestBase.php b/lib/Cake/Test/Case/Model/ModelTestBase.php index 59f3353c3..f0f75a912 100644 --- a/lib/Cake/Test/Case/Model/ModelTestBase.php +++ b/lib/Cake/Test/Case/Model/ModelTestBase.php @@ -73,6 +73,7 @@ abstract class BaseModelTest extends CakeTestCase { 'core.bidding', 'core.bidding_message', 'core.site', 'core.domain', 'core.domains_site', 'core.uuidnativeitem', 'core.uuidnativeportfolio', 'core.uuidnativeitems_uuidnativeportfolio', 'core.uuidnativeitems_uuidnativeportfolio_numericid', + 'core.translated_article', 'core.translate_article', ); /** From 76ab1f4537db2213c37aa05f00fad9677887fa92 Mon Sep 17 00:00:00 2001 From: Val Bancer Date: Thu, 6 Jul 2017 00:03:00 +0200 Subject: [PATCH 06/18] more unit tests --- lib/Cake/Test/Case/Model/ModelReadTest.php | 138 +++++++++++++++++++++ 1 file changed, 138 insertions(+) diff --git a/lib/Cake/Test/Case/Model/ModelReadTest.php b/lib/Cake/Test/Case/Model/ModelReadTest.php index bac344d04..8c7148d89 100644 --- a/lib/Cake/Test/Case/Model/ModelReadTest.php +++ b/lib/Cake/Test/Case/Model/ModelReadTest.php @@ -6425,6 +6425,144 @@ class ModelReadTest extends BaseModelTest { $this->assertEquals($expected, $result); } + public function testBuildQueryAllI18nConditions() { + $this->loadFixtures('TranslateArticle', 'TranslatedArticle', 'User'); + $TestModel = new TranslatedArticle(); + $TestModel->cacheQueries = false; + $TestModel->locale = 'eng'; + $expected = array( + 'conditions' => array( + 'NOT' => array('I18n__title.content' => ''), + ), + 'fields' => null, + 'joins' => array( + array( + 'type' => 'INNER', + 'alias' => 'I18n__title', + 'table' => (object)array( + 'tablePrefix' => '', + 'table' => 'article_i18n', + 'schemaName' => 'test', + ), + 'conditions' => array( + 'TranslatedArticle.id' => (object)array( + 'type' => 'identifier', + 'value' => 'I18n__title.foreign_key', + ), + 'I18n__title.model' => 'TranslatedArticle', + 'I18n__title.field' => 'title', + 'I18n__title.locale' => 'eng', + ), + ), + array( + 'type' => 'INNER', + 'alias' => 'I18n__body', + 'table' => (object)array( + 'tablePrefix' => '', + 'table' => 'article_i18n', + 'schemaName' => 'test', + ), + 'conditions' => array( + 'TranslatedArticle.id' => (object)array( + 'type' => 'identifier', + 'value' => 'I18n__body.foreign_key', + ), + 'I18n__body.model' => 'TranslatedArticle', + 'I18n__body.field' => 'body', + 'I18n__body.locale' => 'eng', + ), + ), + ), + 'limit' => 2, + 'offset' => null, + 'order' => array( + 'TranslatedArticle.id' => 'ASC', + ), + 'page' => 1, + 'group' => null, + 'callbacks' => true, + 'recursive' => 0, + ); + $query= array( + 'recursive' => 0, + 'conditions' => array( + 'NOT' => array('I18n__title.content' => ''), + ), + 'limit' => 2, + ); + $result = $TestModel->buildQuery('all', $query); + $this->assertEquals($expected, $result); + } + + public function testBuildQueryCountI18nConditions() { + $this->loadFixtures('TranslateArticle', 'TranslatedArticle', 'User'); + $TestModel = new TranslatedArticle(); + $TestModel->cacheQueries = false; + $TestModel->locale = 'eng'; + $expected = array( + 'conditions' => array( + 'NOT' => array('I18n__title.content' => ''), + ), + 'fields' => 'COUNT(DISTINCT(`TranslatedArticle`.`id`)) AS count', + 'joins' => array( + array( + 'type' => 'INNER', + 'alias' => 'I18n__title', + 'table' => (object)array( + 'tablePrefix' => '', + 'table' => 'article_i18n', + 'schemaName' => 'test', + ), + 'conditions' => array( + 'TranslatedArticle.id' => (object)array( + 'type' => 'identifier', + 'value' => 'I18n__title.foreign_key', + ), + 'I18n__title.model' => 'TranslatedArticle', + 'I18n__title.field' => 'title', + 'I18n__title.locale' => 'eng', + ), + ), + array( + 'type' => 'INNER', + 'alias' => 'I18n__body', + 'table' => (object)array( + 'tablePrefix' => '', + 'table' => 'article_i18n', + 'schemaName' => 'test', + ), + 'conditions' => array( + 'TranslatedArticle.id' => (object)array( + 'type' => 'identifier', + 'value' => 'I18n__body.foreign_key', + ), + 'I18n__body.model' => 'TranslatedArticle', + 'I18n__body.field' => 'body', + 'I18n__body.locale' => 'eng', + ), + ), + ), + 'limit' => 2, + 'offset' => null, + 'order' => array( + 0 => false, + ), + 'page' => 1, + 'group' => null, + 'callbacks' => true, + 'recursive' => 0, + ); + $query= array( + 'recursive' => 0, + 'conditions' => array( + 'NOT' => array('I18n__title.content' => ''), + ), + 'limit' => 2, + ); + $result = $TestModel->buildQuery('count', $query); + $this->assertEquals($expected, $result); + } + /** * test find('all') method * From d9f21174364ce6946f7863480acde2c8adc1d1b0 Mon Sep 17 00:00:00 2001 From: Val Bancer Date: Fri, 7 Jul 2017 00:41:04 +0200 Subject: [PATCH 07/18] more unit tests --- .../Model/Behavior/TranslateBehaviorTest.php | 161 ++++++++++++++++++ 1 file changed, 161 insertions(+) diff --git a/lib/Cake/Test/Case/Model/Behavior/TranslateBehaviorTest.php b/lib/Cake/Test/Case/Model/Behavior/TranslateBehaviorTest.php index 80a9c5a45..37c42465b 100644 --- a/lib/Cake/Test/Case/Model/Behavior/TranslateBehaviorTest.php +++ b/lib/Cake/Test/Case/Model/Behavior/TranslateBehaviorTest.php @@ -1443,4 +1443,165 @@ class TranslateBehaviorTest extends CakeTestCase { $this->assertEquals('name', $results[0]['TranslateTestModel']['field']); } + public function testBeforeFindAllI18nConditions() { + $this->loadFixtures('TranslateArticle', 'TranslatedArticle', 'User'); + $TestModel = new TranslatedArticle(); + $TestModel->cacheQueries = false; + $TestModel->locale = 'eng'; + $expected = array( + 'conditions' => array( + 'NOT' => array('I18n__title.content' => ''), + ), + 'fields' => null, + 'joins' => array( + array( + 'type' => 'INNER', + 'alias' => 'I18n__title', + 'table' => (object)array( + 'tablePrefix' => '', + 'table' => 'article_i18n', + 'schemaName' => 'test', + ), + 'conditions' => array( + 'TranslatedArticle.id' => (object)array( + 'type' => 'identifier', + 'value' => 'I18n__title.foreign_key', + ), + 'I18n__title.model' => 'TranslatedArticle', + 'I18n__title.field' => 'title', + 'I18n__title.locale' => 'eng', + ), + ), + array( + 'type' => 'INNER', + 'alias' => 'I18n__body', + 'table' => (object)array( + 'tablePrefix' => '', + 'table' => 'article_i18n', + 'schemaName' => 'test', + ), + 'conditions' => array( + 'TranslatedArticle.id' => (object)array( + 'type' => 'identifier', + 'value' => 'I18n__body.foreign_key', + ), + 'I18n__body.model' => 'TranslatedArticle', + 'I18n__body.field' => 'body', + 'I18n__body.locale' => 'eng', + ), + ), + ), + 'limit' => 2, + 'offset' => null, + 'order' => array( + 'TranslatedArticle.id' => 'ASC', + ), + 'page' => 1, + 'group' => null, + 'callbacks' => true, + 'recursive' => 0, + ); + $query = array( + 'conditions' => array( + 'NOT' => array( + 'I18n__title.content' => '', + ), + ), + 'fields' => null, + 'joins' => array(), + 'limit' => 2, + 'offset' => null, + 'order' => array( + 'TranslatedArticle.id' => 'ASC', + ), + 'page' => 1, + 'group' => null, + 'callbacks' => true, + 'recursive' => 0, + ); + $TranslateBehavior = ClassRegistry::getObject('TranslateBehavior'); + $result = $TranslateBehavior->beforeFind($TestModel, $query); + $this->assertEquals($expected, $result); + } + + public function testBeforeFindCountI18nConditions() { + $this->loadFixtures('TranslateArticle', 'TranslatedArticle', 'User'); + $TestModel = new TranslatedArticle(); + $TestModel->cacheQueries = false; + $TestModel->locale = 'eng'; + $expected = array( + 'conditions' => array( + 'NOT' => array('I18n__title.content' => ''), + ), + 'fields' => 'COUNT(DISTINCT(`TranslatedArticle`.`id`)) AS count', + 'joins' => array( + array( + 'type' => 'INNER', + 'alias' => 'I18n__title', + 'table' => (object)array( + 'tablePrefix' => '', + 'table' => 'article_i18n', + 'schemaName' => 'test', + ), + 'conditions' => array( + 'TranslatedArticle.id' => (object)array( + 'type' => 'identifier', + 'value' => 'I18n__title.foreign_key', + ), + 'I18n__title.model' => 'TranslatedArticle', + 'I18n__title.field' => 'title', + 'I18n__title.locale' => 'eng', + ), + ), + array( + 'type' => 'INNER', + 'alias' => 'I18n__body', + 'table' => (object)array( + 'tablePrefix' => '', + 'table' => 'article_i18n', + 'schemaName' => 'test', + ), + 'conditions' => array( + 'TranslatedArticle.id' => (object)array( + 'type' => 'identifier', + 'value' => 'I18n__body.foreign_key', + ), + 'I18n__body.model' => 'TranslatedArticle', + 'I18n__body.field' => 'body', + 'I18n__body.locale' => 'eng', + ), + ), + ), + 'limit' => 2, + 'offset' => null, + 'order' => array( + 0 => false, + ), + 'page' => 1, + 'group' => null, + 'callbacks' => true, + 'recursive' => 0, + ); + $query= array( + 'conditions' => array( + 'NOT' => array( + 'I18n__title.content' => '', + ) + ), + 'fields' => 'COUNT(*) AS `count`', + 'joins' => array(), + 'limit' => 2, + 'offset' => null, + 'order' => array( + 0 => false + ), + 'page' => 1, + 'group' => null, + 'callbacks' => true, + 'recursive' => 0, + ); + $TranslateBehavior = ClassRegistry::getObject('TranslateBehavior'); + $result = $TranslateBehavior->beforeFind($TestModel, $query); + $this->assertEquals($expected, $result); + } } From 344061532325ed6ac54b1f0795e9ab9c7f813844 Mon Sep 17 00:00:00 2001 From: Val Bancer Date: Fri, 7 Jul 2017 01:20:09 +0200 Subject: [PATCH 08/18] Refactroing in TranslateBehavior. Some code was moved to protected methods, simplified the coditions. --- lib/Cake/Model/Behavior/TranslateBehavior.php | 72 ++++++++++++------- 1 file changed, 48 insertions(+), 24 deletions(-) diff --git a/lib/Cake/Model/Behavior/TranslateBehavior.php b/lib/Cake/Model/Behavior/TranslateBehavior.php index 88d9018c9..203bf9b55 100644 --- a/lib/Cake/Model/Behavior/TranslateBehavior.php +++ b/lib/Cake/Model/Behavior/TranslateBehavior.php @@ -147,28 +147,45 @@ class TranslateBehavior extends ModelBehavior { $this->_joinTable = $joinTable; $this->_runtimeModel = $RuntimeModel; - if (is_string($query['fields']) && $query['fields'] === "COUNT(*) AS {$db->name('count')}") { - $query['fields'] = "COUNT(DISTINCT({$db->name($Model->escapeField())})) {$db->alias}count"; - $query['joins'][] = array( - 'type' => $this->runtime[$Model->alias]['joinType'], - 'alias' => $RuntimeModel->alias, - 'table' => $joinTable, - 'conditions' => array( - $Model->escapeField() => $db->identifier($RuntimeModel->escapeField('foreign_key')), - $RuntimeModel->escapeField('model') => $Model->name, - $RuntimeModel->escapeField('locale') => $locale - ) - ); - $conditionFields = $this->_checkConditions($Model, $query); - foreach ($conditionFields as $field) { - $query = $this->_addJoin($Model, $query, $field, $field, $locale); + if (is_string($query['fields'])) { + if ($query['fields'] === "COUNT(*) AS {$db->name('count')}") { + $query['fields'] = "COUNT(DISTINCT({$db->name($Model->escapeField())})) {$db->alias}count"; + $query['joins'][] = array( + 'type' => $this->runtime[$Model->alias]['joinType'], + 'alias' => $RuntimeModel->alias, + 'table' => $joinTable, + 'conditions' => array( + $Model->escapeField() => $db->identifier($RuntimeModel->escapeField('foreign_key')), + $RuntimeModel->escapeField('model') => $Model->name, + $RuntimeModel->escapeField('locale') => $locale + ) + ); + $conditionFields = $this->_checkConditions($Model, $query); + foreach ($conditionFields as $field) { + $query = $this->_addJoin($Model, $query, $field, $field, $locale); + } + unset($this->_joinTable, $this->_runtimeModel); + return $query; + } else { + $query['fields'] = CakeText::tokenize($query['fields']); } - unset($this->_joinTable, $this->_runtimeModel); - return $query; - } elseif (is_string($query['fields'])) { - $query['fields'] = CakeText::tokenize($query['fields']); } + $addFields = $this->_getFields($Model, $query); + $this->runtime[$Model->alias]['virtualFields'] = $Model->virtualFields; + $query = $this->_addAllJoins($Model, $query, $addFields); + $this->runtime[$Model->alias]['beforeFind'] = $addFields; + unset($this->_joinTable, $this->_runtimeModel); + return $query; + } +/** + * Gets fields to be retrieved. + * + * @param Model $Model The model being worked on. + * @param array $query The query array to take fields from. + * @return array The fields. + */ + protected function _getFields(Model $Model, $query) { $fields = array_merge( $this->settings[$Model->alias], $this->runtime[$Model->alias]['fields'] @@ -191,15 +208,24 @@ class TranslateBehavior extends ModelBehavior { } } } + return $addFields; + } - $this->runtime[$Model->alias]['virtualFields'] = $Model->virtualFields; +/** + * Appends all necessary joins for translated fields. + * + * @param Model $Model The model being worked on. + * @param array $query The query array to append joins to. + * @param string $addFields The fields being joined. + * @return array The modified query + */ + protected function _addAllJoins(Model $Model, $query, $addFields) { + $locale = $this->_getLocale($Model); if ($addFields) { foreach ($addFields as $_f => $field) { $aliasField = is_numeric($_f) ? $field : $_f; - foreach (array($aliasField, $Model->alias . '.' . $aliasField) as $_field) { $key = array_search($_field, (array)$query['fields']); - if ($key !== false) { unset($query['fields'][$key]); } @@ -207,8 +233,6 @@ class TranslateBehavior extends ModelBehavior { $query = $this->_addJoin($Model, $query, $field, $aliasField, $locale); } } - $this->runtime[$Model->alias]['beforeFind'] = $addFields; - unset($this->_joinTable, $this->_runtimeModel); return $query; } From f0bbcb3ffcd720a9202a5a9d04d838092e7dac29 Mon Sep 17 00:00:00 2001 From: Val Bancer Date: Sat, 8 Jul 2017 16:51:32 +0200 Subject: [PATCH 09/18] fixed parsing of conditions with 'NOT' in TranslateBehavior --- lib/Cake/Model/Behavior/TranslateBehavior.php | 21 ++++++++-- .../Model/Behavior/TranslateBehaviorTest.php | 41 +++++++++---------- lib/Cake/Test/Case/Model/ModelReadTest.php | 37 ++++++++--------- 3 files changed, 56 insertions(+), 43 deletions(-) diff --git a/lib/Cake/Model/Behavior/TranslateBehavior.php b/lib/Cake/Model/Behavior/TranslateBehavior.php index 203bf9b55..07e2635f2 100644 --- a/lib/Cake/Model/Behavior/TranslateBehavior.php +++ b/lib/Cake/Model/Behavior/TranslateBehavior.php @@ -245,11 +245,26 @@ class TranslateBehavior extends ModelBehavior { * @return array The list of translated fields that are in the conditions. */ protected function _checkConditions(Model $Model, $query) { - $conditionFields = array(); if (empty($query['conditions']) || (!empty($query['conditions']) && !is_array($query['conditions']))) { - return $conditionFields; + return array(); } - foreach ($query['conditions'] as $col => $val) { + return $this->_getConditionFields($Model, $query['conditions']); + } + + /** + * Extracts condition field names recursively. + * + * @param Model $Model The model being read. + * @param array $conditions The conditions array. + * @return array The list of condition fields. + */ + protected function _getConditionFields(Model $Model, $conditions) { + $conditionFields = array(); + foreach ($conditions as $col => $val) { + if (is_array($val)) { + $subConditionFields = $this->_getConditionFields($Model, $val); + $conditionFields = array_merge($conditionFields, $subConditionFields); + } foreach ($this->settings[$Model->alias] as $field => $assoc) { if (is_numeric($field)) { $field = $assoc; diff --git a/lib/Cake/Test/Case/Model/Behavior/TranslateBehaviorTest.php b/lib/Cake/Test/Case/Model/Behavior/TranslateBehaviorTest.php index 37c42465b..5f74583c2 100644 --- a/lib/Cake/Test/Case/Model/Behavior/TranslateBehaviorTest.php +++ b/lib/Cake/Test/Case/Model/Behavior/TranslateBehaviorTest.php @@ -1460,7 +1460,7 @@ class TranslateBehaviorTest extends CakeTestCase { 'table' => (object)array( 'tablePrefix' => '', 'table' => 'article_i18n', - 'schemaName' => 'test', + 'schemaName' => 'cakephp_test', ), 'conditions' => array( 'TranslatedArticle.id' => (object)array( @@ -1478,7 +1478,7 @@ class TranslateBehaviorTest extends CakeTestCase { 'table' => (object)array( 'tablePrefix' => '', 'table' => 'article_i18n', - 'schemaName' => 'test', + 'schemaName' => 'cakephp_test', ), 'conditions' => array( 'TranslatedArticle.id' => (object)array( @@ -1535,13 +1535,30 @@ class TranslateBehaviorTest extends CakeTestCase { ), 'fields' => 'COUNT(DISTINCT(`TranslatedArticle`.`id`)) AS count', 'joins' => array( + array( + 'type' => 'INNER', + 'alias' => 'TranslateArticleModel', + 'table' => (object)array( + 'tablePrefix' => '', + 'table' => 'article_i18n', + 'schemaName' => 'cakephp_test', + ), + 'conditions' => array( + '`TranslatedArticle`.`id`' => (object)array( + 'type' => 'identifier', + 'value' => '`TranslateArticleModel`.`foreign_key`', + ), + '`TranslateArticleModel`.`model`' => 'TranslatedArticle', + '`TranslateArticleModel`.`locale`' => 'eng', + ), + ), array( 'type' => 'INNER', 'alias' => 'I18n__title', 'table' => (object)array( 'tablePrefix' => '', 'table' => 'article_i18n', - 'schemaName' => 'test', + 'schemaName' => 'cakephp_test', ), 'conditions' => array( 'TranslatedArticle.id' => (object)array( @@ -1553,24 +1570,6 @@ class TranslateBehaviorTest extends CakeTestCase { 'I18n__title.locale' => 'eng', ), ), - array( - 'type' => 'INNER', - 'alias' => 'I18n__body', - 'table' => (object)array( - 'tablePrefix' => '', - 'table' => 'article_i18n', - 'schemaName' => 'test', - ), - 'conditions' => array( - 'TranslatedArticle.id' => (object)array( - 'type' => 'identifier', - 'value' => 'I18n__body.foreign_key', - ), - 'I18n__body.model' => 'TranslatedArticle', - 'I18n__body.field' => 'body', - 'I18n__body.locale' => 'eng', - ), - ), ), 'limit' => 2, 'offset' => null, diff --git a/lib/Cake/Test/Case/Model/ModelReadTest.php b/lib/Cake/Test/Case/Model/ModelReadTest.php index 8c7148d89..f2c1713b9 100644 --- a/lib/Cake/Test/Case/Model/ModelReadTest.php +++ b/lib/Cake/Test/Case/Model/ModelReadTest.php @@ -6505,6 +6505,23 @@ class ModelReadTest extends BaseModelTest { ), 'fields' => 'COUNT(DISTINCT(`TranslatedArticle`.`id`)) AS count', 'joins' => array( + array( + 'type' => 'INNER', + 'alias' => 'TranslateArticleModel', + 'table' => (object)array( + 'tablePrefix' => '', + 'table' => 'article_i18n', + 'schemaName' => 'cakephp_test', + ), + 'conditions' => array( + '`TranslatedArticle`.`id`' => (object)array( + 'type' => 'identifier', + 'value' => '`TranslateArticleModel`.`foreign_key`', + ), + '`TranslateArticleModel`.`model`' => 'TranslatedArticle', + '`TranslateArticleModel`.`locale`' => 'eng', + ), + ), array( 'type' => 'INNER', 'alias' => 'I18n__title', @@ -6523,24 +6540,6 @@ class ModelReadTest extends BaseModelTest { 'I18n__title.locale' => 'eng', ), ), - array( - 'type' => 'INNER', - 'alias' => 'I18n__body', - 'table' => (object)array( - 'tablePrefix' => '', - 'table' => 'article_i18n', - 'schemaName' => 'test', - ), - 'conditions' => array( - 'TranslatedArticle.id' => (object)array( - 'type' => 'identifier', - 'value' => 'I18n__body.foreign_key', - ), - 'I18n__body.model' => 'TranslatedArticle', - 'I18n__body.field' => 'body', - 'I18n__body.locale' => 'eng', - ), - ), ), 'limit' => 2, 'offset' => null, @@ -7325,7 +7324,7 @@ class ModelReadTest extends BaseModelTest { 'limit' => 2, ); $result = $TestModel->find('count', $options); - $this->assertEquals(2, $result); + $this->assertEquals(3, $result); } /** From 794ce22f379e2ed4128570dec4c2361bf0ffaad7 Mon Sep 17 00:00:00 2001 From: Val Bancer Date: Sun, 9 Jul 2017 19:24:51 +0200 Subject: [PATCH 10/18] fixed unit tests and docs --- lib/Cake/Model/Behavior/TranslateBehavior.php | 2 +- lib/Cake/Test/Case/Model/ModelReadTest.php | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/Cake/Model/Behavior/TranslateBehavior.php b/lib/Cake/Model/Behavior/TranslateBehavior.php index 07e2635f2..8223b7e87 100644 --- a/lib/Cake/Model/Behavior/TranslateBehavior.php +++ b/lib/Cake/Model/Behavior/TranslateBehavior.php @@ -216,7 +216,7 @@ class TranslateBehavior extends ModelBehavior { * * @param Model $Model The model being worked on. * @param array $query The query array to append joins to. - * @param string $addFields The fields being joined. + * @param array $addFields The fields being joined. * @return array The modified query */ protected function _addAllJoins(Model $Model, $query, $addFields) { diff --git a/lib/Cake/Test/Case/Model/ModelReadTest.php b/lib/Cake/Test/Case/Model/ModelReadTest.php index f2c1713b9..3b6a54b5f 100644 --- a/lib/Cake/Test/Case/Model/ModelReadTest.php +++ b/lib/Cake/Test/Case/Model/ModelReadTest.php @@ -6442,7 +6442,7 @@ class ModelReadTest extends BaseModelTest { 'table' => (object)array( 'tablePrefix' => '', 'table' => 'article_i18n', - 'schemaName' => 'test', + 'schemaName' => 'cakephp_test', ), 'conditions' => array( 'TranslatedArticle.id' => (object)array( @@ -6460,7 +6460,7 @@ class ModelReadTest extends BaseModelTest { 'table' => (object)array( 'tablePrefix' => '', 'table' => 'article_i18n', - 'schemaName' => 'test', + 'schemaName' => 'cakephp_test', ), 'conditions' => array( 'TranslatedArticle.id' => (object)array( @@ -6528,7 +6528,7 @@ class ModelReadTest extends BaseModelTest { 'table' => (object)array( 'tablePrefix' => '', 'table' => 'article_i18n', - 'schemaName' => 'test', + 'schemaName' => 'cakephp_test', ), 'conditions' => array( 'TranslatedArticle.id' => (object)array( From d72c2d7e0e2900736d87683ba576187c31e612dc Mon Sep 17 00:00:00 2001 From: Val Bancer Date: Sun, 9 Jul 2017 20:05:53 +0200 Subject: [PATCH 11/18] fixed code style, skipped pgsql and sqlite incompartible tests --- lib/Cake/Model/Behavior/TranslateBehavior.php | 14 +++++++------- .../Case/Model/Behavior/TranslateBehaviorTest.php | 4 +++- lib/Cake/Test/Case/Model/ModelReadTest.php | 6 ++++-- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/lib/Cake/Model/Behavior/TranslateBehavior.php b/lib/Cake/Model/Behavior/TranslateBehavior.php index 8223b7e87..740fcccad 100644 --- a/lib/Cake/Model/Behavior/TranslateBehavior.php +++ b/lib/Cake/Model/Behavior/TranslateBehavior.php @@ -251,13 +251,13 @@ class TranslateBehavior extends ModelBehavior { return $this->_getConditionFields($Model, $query['conditions']); } - /** - * Extracts condition field names recursively. - * - * @param Model $Model The model being read. - * @param array $conditions The conditions array. - * @return array The list of condition fields. - */ +/** + * Extracts condition field names recursively. + * + * @param Model $Model The model being read. + * @param array $conditions The conditions array. + * @return array The list of condition fields. + */ protected function _getConditionFields(Model $Model, $conditions) { $conditionFields = array(); foreach ($conditions as $col => $val) { diff --git a/lib/Cake/Test/Case/Model/Behavior/TranslateBehaviorTest.php b/lib/Cake/Test/Case/Model/Behavior/TranslateBehaviorTest.php index 5f74583c2..251e8a8bb 100644 --- a/lib/Cake/Test/Case/Model/Behavior/TranslateBehaviorTest.php +++ b/lib/Cake/Test/Case/Model/Behavior/TranslateBehaviorTest.php @@ -1444,6 +1444,7 @@ class TranslateBehaviorTest extends CakeTestCase { } public function testBeforeFindAllI18nConditions() { + $this->skipIf(!$this->db instanceof Mysql, 'This test is only compatible with Mysql.'); $this->loadFixtures('TranslateArticle', 'TranslatedArticle', 'User'); $TestModel = new TranslatedArticle(); $TestModel->cacheQueries = false; @@ -1525,6 +1526,7 @@ class TranslateBehaviorTest extends CakeTestCase { } public function testBeforeFindCountI18nConditions() { + $this->skipIf(!$this->db instanceof Mysql, 'This test is only compatible with Mysql.'); $this->loadFixtures('TranslateArticle', 'TranslatedArticle', 'User'); $TestModel = new TranslatedArticle(); $TestModel->cacheQueries = false; @@ -1581,7 +1583,7 @@ class TranslateBehaviorTest extends CakeTestCase { 'callbacks' => true, 'recursive' => 0, ); - $query= array( + $query = array( 'conditions' => array( 'NOT' => array( 'I18n__title.content' => '', diff --git a/lib/Cake/Test/Case/Model/ModelReadTest.php b/lib/Cake/Test/Case/Model/ModelReadTest.php index 3b6a54b5f..e993fc9d4 100644 --- a/lib/Cake/Test/Case/Model/ModelReadTest.php +++ b/lib/Cake/Test/Case/Model/ModelReadTest.php @@ -6426,6 +6426,7 @@ class ModelReadTest extends BaseModelTest { } public function testBuildQueryAllI18nConditions() { + $this->skipIf(!$this->db instanceof Mysql, 'This test is only compatible with Mysql.'); $this->loadFixtures('TranslateArticle', 'TranslatedArticle', 'User'); $TestModel = new TranslatedArticle(); $TestModel->cacheQueries = false; @@ -6483,7 +6484,7 @@ class ModelReadTest extends BaseModelTest { 'callbacks' => true, 'recursive' => 0, ); - $query= array( + $query = array( 'recursive' => 0, 'conditions' => array( 'NOT' => array('I18n__title.content' => ''), @@ -6495,6 +6496,7 @@ class ModelReadTest extends BaseModelTest { } public function testBuildQueryCountI18nConditions() { + $this->skipIf(!$this->db instanceof Mysql, 'This test is only compatible with Mysql.'); $this->loadFixtures('TranslateArticle', 'TranslatedArticle', 'User'); $TestModel = new TranslatedArticle(); $TestModel->cacheQueries = false; @@ -6551,7 +6553,7 @@ class ModelReadTest extends BaseModelTest { 'callbacks' => true, 'recursive' => 0, ); - $query= array( + $query = array( 'recursive' => 0, 'conditions' => array( 'NOT' => array('I18n__title.content' => ''), From bdb8e9b6d5fc0c7490de803654a484044cf31e54 Mon Sep 17 00:00:00 2001 From: Val Bancer Date: Mon, 10 Jul 2017 23:12:35 +0200 Subject: [PATCH 12/18] added composer scripts like in cake 3 --- composer.json | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 5b3a2624f..3fa7dd20a 100644 --- a/composer.json +++ b/composer.json @@ -30,5 +30,13 @@ }, "bin": [ "lib/Cake/Console/cake" - ] + ], + "scripts": { + "check": [ + "@cs-check", + "@test" + ], + "cs-check": "./vendors/bin/phpcs -p --extensions=php --standard=CakePHP ./lib/Cake", + "test": "./lib/Cake/Console/cake test core AllTests --stderr" + } } From 2290b612f830782ee506ef34d4f71a366f1b1aab Mon Sep 17 00:00:00 2001 From: Val Bancer Date: Tue, 11 Jul 2017 00:14:08 +0200 Subject: [PATCH 13/18] set order in HABTM in unit tests to make it stable in mysql 5.7 --- lib/Cake/Test/Case/Model/models.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/Cake/Test/Case/Model/models.php b/lib/Cake/Test/Case/Model/models.php index babc5e7ff..d9b770162 100644 --- a/lib/Cake/Test/Case/Model/models.php +++ b/lib/Cake/Test/Case/Model/models.php @@ -260,7 +260,13 @@ class Article extends CakeTestModel { * * @var array */ - public $hasAndBelongsToMany = array('Tag'); + public $hasAndBelongsToMany = array( + 'Tag' => array( + 'order' => array( + 'Tag.id' => 'ASC', + ), + ), + ); /** * validate property From 16acacfa2785aef7dc298e206002db5f79000194 Mon Sep 17 00:00:00 2001 From: Val Bancer Date: Tue, 11 Jul 2017 00:31:41 +0200 Subject: [PATCH 14/18] process-timeout set to 0 in composer to let the tests run as much as necessary --- composer.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 3fa7dd20a..f8ae1f3fa 100644 --- a/composer.json +++ b/composer.json @@ -26,7 +26,8 @@ "cakephp/cakephp-codesniffer": "^1.0.0" }, "config": { - "vendor-dir": "vendors/" + "vendor-dir": "vendors/", + "process-timeout": 0 }, "bin": [ "lib/Cake/Console/cake" From 0d68007e5c7b8e18f91771c7dbfba89682bfb57e Mon Sep 17 00:00:00 2001 From: mark_story Date: Tue, 11 Jul 2017 10:01:08 -0400 Subject: [PATCH 15/18] Revert changes in 2290b612f830782ee506ef34d4f71a366f1b1aab I think they broke the builds in our CI environments. Refs #10894 --- lib/Cake/Test/Case/Model/models.php | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/lib/Cake/Test/Case/Model/models.php b/lib/Cake/Test/Case/Model/models.php index d9b770162..babc5e7ff 100644 --- a/lib/Cake/Test/Case/Model/models.php +++ b/lib/Cake/Test/Case/Model/models.php @@ -260,13 +260,7 @@ class Article extends CakeTestModel { * * @var array */ - public $hasAndBelongsToMany = array( - 'Tag' => array( - 'order' => array( - 'Tag.id' => 'ASC', - ), - ), - ); + public $hasAndBelongsToMany = array('Tag'); /** * validate property From 5cc0d7a5cf8eaea1cf4a7a829f580ae77460a39e Mon Sep 17 00:00:00 2001 From: Val Bancer Date: Wed, 12 Jul 2017 00:46:02 +0200 Subject: [PATCH 16/18] Some tests refactored. Skipped non-compartible tests in MySQL ONLY_FULL_GROUP_BY mode. --- lib/Cake/Test/Case/Model/ModelReadTest.php | 170 ++++++++++++--------- 1 file changed, 100 insertions(+), 70 deletions(-) diff --git a/lib/Cake/Test/Case/Model/ModelReadTest.php b/lib/Cake/Test/Case/Model/ModelReadTest.php index 11e9ffa90..1f2562553 100644 --- a/lib/Cake/Test/Case/Model/ModelReadTest.php +++ b/lib/Cake/Test/Case/Model/ModelReadTest.php @@ -192,6 +192,81 @@ class ModelReadTest extends BaseModelTest { $this->assertEquals($expected, $result); } + public function skipIfIsStrictGroupBy() { + $isOnlyFullGroupBy = false; + if($this->db instanceof Mysql) { + $sqlMode = $this->db->query('SELECT @@sql_mode AS sql_mode;'); + if (strpos($sqlMode[0][0]['sql_mode'], 'ONLY_FULL_GROUP_BY') > -1) { + $isOnlyFullGroupBy = true; + } + } + $isStrictGroupBy = $isOnlyFullGroupBy || + $this->db instanceof Postgres || + $this->db instanceof Sqlite || + $this->db instanceof Oracle || + $this->db instanceof Sqlserver; + $message = 'Postgres, Oracle, SQLite, SQL Server and MySQL in ONLY_FULL_GROUP_BY ' . + 'mode have strict GROUP BY and are incompatible with this test.'; + $this->skipIf($isStrictGroupBy, $message); + } + +/** + * testGroupByAndOrder method + * + * This test will never pass with Postgres or Oracle as all fields in a select must be + * part of an aggregate function or in the GROUP BY statement. + * + * @return void + */ + public function testGroupByAndOrder() { + $this->skipIfIsStrictGroupBy(); + $this->loadFixtures('Project', 'Thread', 'Message'); + $Thread = new Thread(); + $result = $Thread->find('all', array( + 'group' => 'Thread.project_id', + 'order' => 'Thread.id ASC', + )); + $expected = array ( + array ( + 'Thread' => array ( + 'id' => 1, + 'project_id' => 1, + 'name' => 'Project 1, Thread 1', + ), + 'Project' => array ( + 'id' => 1, + 'name' => 'Project 1', + ), + 'Message' => array ( + array ( + 'id' => 1, + 'thread_id' => 1, + 'name' => 'Thread 1, Message 1', + ), + ), + ), + array ( + 'Thread' => array ( + 'id' => 3, + 'project_id' => 2, + 'name' => 'Project 2, Thread 1', + ), + 'Project' => array ( + 'id' => 2, + 'name' => 'Project 2', + ), + 'Message' => array ( + array ( + 'id' => 3, + 'thread_id' => 3, + 'name' => 'Thread 3, Message 1', + ), + ), + ), + ); + $this->assertEquals($expected, $result); + } + /** * testGroupBy method * @@ -201,55 +276,12 @@ class ModelReadTest extends BaseModelTest { * @return void */ public function testGroupBy() { - $isStrictGroupBy = $this->db instanceof Postgres || $this->db instanceof Sqlite || $this->db instanceof Oracle || $this->db instanceof Sqlserver; - $message = 'Postgres, Oracle, SQLite and SQL Server have strict GROUP BY and are incompatible with this test.'; - - $this->skipIf($isStrictGroupBy, $message); + $this->skipIfIsStrictGroupBy(); $this->loadFixtures('Project', 'Product', 'Thread', 'Message', 'Bid'); $Thread = new Thread(); $Product = new Product(); - $result = $Thread->find('all', array( - 'group' => 'Thread.project_id', - 'order' => 'Thread.id ASC' - )); - - $expected = array( - array( - 'Thread' => array( - 'id' => 1, - 'project_id' => 1, - 'name' => 'Project 1, Thread 1' - ), - 'Project' => array( - 'id' => 1, - 'name' => 'Project 1' - ), - 'Message' => array( - array( - 'id' => 1, - 'thread_id' => 1, - 'name' => 'Thread 1, Message 1' - ))), - array( - 'Thread' => array( - 'id' => 3, - 'project_id' => 2, - 'name' => 'Project 2, Thread 1' - ), - 'Project' => array( - 'id' => 2, - 'name' => 'Project 2' - ), - 'Message' => array( - array( - 'id' => 3, - 'thread_id' => 3, - 'name' => 'Thread 3, Message 1' - )))); - $this->assertEquals($expected, $result); - $rows = $Thread->find('all', array( 'group' => 'Thread.project_id', 'fields' => array('Thread.project_id', 'COUNT(*) AS total') @@ -8159,6 +8191,29 @@ class ModelReadTest extends BaseModelTest { $this->assertEquals($expected, $result); } + public function testVirtualFieldsMysqlGroup() { + $this->skipIf(!($this->db instanceof Mysql), 'The rest of virtualFields test only compatible with Mysql.'); + $this->skipIfIsStrictGroupBy(); + $this->loadFixtures('Post'); + $Post = ClassRegistry::init('Post'); + $Post->create(); + $Post->virtualFields = array( + 'low_title' => 'lower(Post.title)', + 'unique_test_field' => 'COUNT(Post.id)', + ); + $expectation = array( + 'Post' => array( + 'low_title' => 'first post', + 'unique_test_field' => 1, + ), + ); + $result = $Post->find('first', array( + 'fields' => array_keys($Post->virtualFields), + 'group' => array('low_title'), + )); + $this->assertEquals($expectation, $result); + } + /** * testVirtualFieldsMysql() * @@ -8169,42 +8224,17 @@ class ModelReadTest extends BaseModelTest { */ public function testVirtualFieldsMysql() { $this->skipIf(!($this->db instanceof Mysql), 'The rest of virtualFields test only compatible with Mysql.'); - - $this->loadFixtures('Post', 'Author'); - $Post = ClassRegistry::init('Post'); - - $Post->create(); - $Post->virtualFields = array( - 'low_title' => 'lower(Post.title)', - 'unique_test_field' => 'COUNT(Post.id)' - ); - - $expectation = array( - 'Post' => array( - 'low_title' => 'first post', - 'unique_test_field' => 1 - ) - ); - - $result = $Post->find('first', array( - 'fields' => array_keys($Post->virtualFields), - 'group' => array('low_title') - )); - - $this->assertEquals($expectation, $result); - + $this->loadFixtures('Author'); $Author = ClassRegistry::init('Author'); $Author->virtualFields = array( 'full_name' => 'CONCAT(Author.user, " ", Author.id)' ); - $result = $Author->find('first', array( 'conditions' => array('Author.user' => 'mariano'), 'fields' => array('Author.password', 'Author.full_name'), 'recursive' => -1 )); $this->assertTrue(isset($result['Author']['full_name'])); - $result = $Author->find('first', array( 'conditions' => array('Author.user' => 'mariano'), 'fields' => array('Author.full_name', 'Author.password'), From d71bc4acae601176e3e4a755e9f245a18736c976 Mon Sep 17 00:00:00 2001 From: Val Bancer Date: Wed, 12 Jul 2017 01:00:33 +0200 Subject: [PATCH 17/18] Fixed code style --- lib/Cake/Test/Case/Model/ModelReadTest.php | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/Cake/Test/Case/Model/ModelReadTest.php b/lib/Cake/Test/Case/Model/ModelReadTest.php index 1f2562553..82690ae22 100644 --- a/lib/Cake/Test/Case/Model/ModelReadTest.php +++ b/lib/Cake/Test/Case/Model/ModelReadTest.php @@ -194,7 +194,7 @@ class ModelReadTest extends BaseModelTest { public function skipIfIsStrictGroupBy() { $isOnlyFullGroupBy = false; - if($this->db instanceof Mysql) { + if ($this->db instanceof Mysql) { $sqlMode = $this->db->query('SELECT @@sql_mode AS sql_mode;'); if (strpos($sqlMode[0][0]['sql_mode'], 'ONLY_FULL_GROUP_BY') > -1) { $isOnlyFullGroupBy = true; @@ -226,19 +226,19 @@ class ModelReadTest extends BaseModelTest { 'group' => 'Thread.project_id', 'order' => 'Thread.id ASC', )); - $expected = array ( - array ( - 'Thread' => array ( + $expected = array( + array( + 'Thread' => array( 'id' => 1, 'project_id' => 1, 'name' => 'Project 1, Thread 1', ), - 'Project' => array ( + 'Project' => array( 'id' => 1, 'name' => 'Project 1', ), - 'Message' => array ( - array ( + 'Message' => array( + array( 'id' => 1, 'thread_id' => 1, 'name' => 'Thread 1, Message 1', @@ -246,17 +246,17 @@ class ModelReadTest extends BaseModelTest { ), ), array ( - 'Thread' => array ( + 'Thread' => array( 'id' => 3, 'project_id' => 2, 'name' => 'Project 2, Thread 1', ), - 'Project' => array ( + 'Project' => array( 'id' => 2, 'name' => 'Project 2', ), - 'Message' => array ( - array ( + 'Message' => array( + array( 'id' => 3, 'thread_id' => 3, 'name' => 'Thread 3, Message 1', From d1c3cca924d923f98d7d51268f3db1c42c8c0dd8 Mon Sep 17 00:00:00 2001 From: Val Bancer Date: Wed, 12 Jul 2017 20:42:06 +0200 Subject: [PATCH 18/18] makes the test more stable --- lib/Cake/Test/Case/Model/ModelWriteTest.php | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/lib/Cake/Test/Case/Model/ModelWriteTest.php b/lib/Cake/Test/Case/Model/ModelWriteTest.php index 378469b55..52f99908f 100644 --- a/lib/Cake/Test/Case/Model/ModelWriteTest.php +++ b/lib/Cake/Test/Case/Model/ModelWriteTest.php @@ -1673,8 +1673,19 @@ class ModelWriteTest extends BaseModelTest { $this->assertFalse(empty($result)); $this->assertEquals($data['Tag'], $result['Tag']); - $TestModel->unbindModel(array('belongsTo' => array('User'), 'hasMany' => array('Comment'))); - $result = $TestModel->find('first', array('fields' => array('id', 'user_id', 'title', 'body'), 'conditions' => array('Article.id' => 2))); + $TestModel->unbindModel(array( + 'belongsTo' => array('User'), + 'hasMany' => array('Comment'), + )); + $TestModel->bindModel(array( + 'hasAndBelongsToMany' => array( + 'Tag' => array('order' => 'Tag.id'), + ), + ), false); + $result = $TestModel->find('first', array( + 'fields' => array('id', 'user_id', 'title', 'body'), + 'conditions' => array('Article.id' => 2), + )); $expected = array( 'Article' => array( 'id' => '2', @@ -1694,7 +1705,9 @@ class ModelWriteTest extends BaseModelTest { 'tag' => 'tag2', 'created' => '2007-03-18 12:24:23', 'updated' => '2007-03-18 12:26:31' - ))); + ), + ), + ); $this->assertEquals($expected, $result); $data = array('Article' => array('id' => '2'), 'Tag' => array('Tag' => array(2, 3)));