From 53b8b75b95872b5d77cefa3a81b1e5cb49f89898 Mon Sep 17 00:00:00 2001 From: Jude Hungerford Date: Fri, 13 Nov 2020 16:44:23 +1100 Subject: [PATCH] fix style issues --- CRM/Utils/SQL/Select.php | 14 ++++----- Civi/API/SelectQuery.php | 21 ++++++++----- .../api/v3/CustomFieldTooManyJoinsTest.php | 31 +++++++++---------- 3 files changed, 35 insertions(+), 31 deletions(-) diff --git a/CRM/Utils/SQL/Select.php b/CRM/Utils/SQL/Select.php index 5ffe68fdc814..d516c0f0789f 100644 --- a/CRM/Utils/SQL/Select.php +++ b/CRM/Utils/SQL/Select.php @@ -476,7 +476,7 @@ public function isEmpty($parts = NULL) { /** * @return array - * The set of joins for this query + * The set of joins for this query */ public function getJoins() { return $this->joins; @@ -487,18 +487,18 @@ public function getJoins() { * The string to search for * * @return bool - * Do any of the WHERE, GROUP BY, HAVING, or ORDER BY clauses contain $needle? + * Do any of the WHERE, GROUP BY, HAVING, or ORDER BY clauses contain $needle? */ public function latterClausesContainString($needle) { foreach ([$this->wheres, $this->groupBys, $this->havings, $this->orderBys] as $clauseName => $clauseType) { foreach ($clauseType as $clauseKey => $clauseDetail) { - if (strpos($clauseKey, $needle) !== false) { - return true; + if (strpos($clauseKey, $needle) !== FALSE) { + return TRUE; break 2; } } } - return false; + return FALSE; } /** @@ -510,8 +510,8 @@ public function latterClausesContainString($needle) { */ public function removeTable($key) { foreach ($this->selects as $count => $select) { - if (strpos($select, $key) !== false) { - unset($this->selects[$count]); // According to docs, unset does not reindex the array + if (strpos($select, $key) !== FALSE) { + unset($this->selects[$count]); } } unset($this->joins[$key]); diff --git a/Civi/API/SelectQuery.php b/Civi/API/SelectQuery.php index e8868cc92636..fee1ed617b8e 100644 --- a/Civi/API/SelectQuery.php +++ b/Civi/API/SelectQuery.php @@ -28,8 +28,13 @@ abstract class SelectQuery { const - SQL_JOIN_LIMIT = 60, // 60 joins plus the table in the "from" clause equals 61, the join limit for MySQL. - MAX_JOINS = 4, // Note that this refers to join depth, not total joins. + + // 60 joins plus the table in the "from" clause equals 61, the join limit for MySQL. + SQL_JOIN_LIMIT = 60, + + // Note that this refers to join depth, not total joins. + MAX_JOINS = 4, + MAIN_TABLE_ALIAS = 'a'; /** @@ -171,7 +176,7 @@ private function classifyJoins() { $mandatory = false; foreach ($this->joins as $check_key => $check_join) { // check the order - haystack, needle - if ((strpos($key, $check_join) !== false) + if ((strpos($key, $check_join) !== false) || ($this->query->latterClausesContainString($key))) { $mandatory = true; break; @@ -180,7 +185,7 @@ private function classifyJoins() { if ($mandatory) { $classified['mandatory'][$key] = $join; } - else if (!isset($this->joins[$key])) { + elseif (!isset($this->joins[$key])) { $classified['depends'][$key] = $join; } else { @@ -198,7 +203,7 @@ private function classifyJoins() { */ private function addJoinDependants($joins, $depends) { $additional = []; - foreach($joins as $joinKey => $join) { + foreach ($joins as $joinKey => $join) { foreach ($depends as $dependKey => $dependJoin) { if ($joinKey != $dependKey && (strpos($dependJoin, $joinKey) !== false)) { $additional[$dependKey] = $dependJoin; @@ -223,10 +228,10 @@ private function addJoinDependants($joins, $depends) { /** * Split the set of classified joins into multiple groups, each representing - * a query whose results can be collated (hopefully) into the result we + * a query whose results can be collated (hopefully) into the result we * would have had if we could run a query with an arbitrary number of joins. * Note that this does not work if the query contains GROUP BY. - * + * * @param array $classified * This is the output of $this->classifyJoins() * @return array @@ -293,7 +298,7 @@ private function splitQueryRunAndCollate() { foreach ($split_queries as $query) { $sql = $query->toSQL(); $result_dao = \CRM_Core_DAO::executeQuery($sql); - while ($result_dao->fetch()) { // FIXME I have very low confidence that this will produce the desired outcome without understanding and tuning + while ($result_dao->fetch()) { if (in_array('count_rows', $this->select)) { return (int) $result_dao->c; } diff --git a/tests/phpunit/api/v3/CustomFieldTooManyJoinsTest.php b/tests/phpunit/api/v3/CustomFieldTooManyJoinsTest.php index 6a116646785a..0333fbfd0da0 100644 --- a/tests/phpunit/api/v3/CustomFieldTooManyJoinsTest.php +++ b/tests/phpunit/api/v3/CustomFieldTooManyJoinsTest.php @@ -20,7 +20,7 @@ class api_v3_CustomFieldTooManyJoinsTest extends CiviUnitTestCase { protected $createdCustomGroups = []; - function testTooManyJoins() { + public function testTooManyJoins() { $activityTypes = civicrm_api3('Activity', 'getoptions', [ 'sequential' => 1, 'field' => "activity_type_id", @@ -32,7 +32,7 @@ function testTooManyJoins() { for ($i = 0; $i < 130; $i++) { $customGroup = $this->customGroupCreate([ 'extends' => 'Activity', - 'title' => "Test join limit $i" + 'title' => "Test join limit $i", ]); $this->createdCustomGroups[] = $customGroup; $customField = $this->customFieldCreate([ @@ -49,24 +49,23 @@ function testTooManyJoins() { * @throws \CRM_Core_Exception */ public function tearDown() { - foreach($this->createdCustomGroups as $customGroup) { + foreach ($this->createdCustomGroups as $customGroup) { $this->customGroupDelete($customGroup['id']); } parent::tearDown(); } -// Suggestion from Totten in developer chat -// function testTooManyJoins() { -// $act = callApiSuccess('Activity', 'create', array(...)); -// for ($i = 0; $i < 70; $i++) { -// $cg = callApiSuccess('CustomGroup', 'create', array(...'label' => "My custom group $i"...)); -// $cf = callApiSuccess('CustomField', 'create', array(...'label' => "My custom field $i", 'custom_group_id' => $cg['id']...)); -// } -// callApiSuccess('Activity', 'get', array('id' => $act['id'])); -// } -// function tearDown() { -// // destroy the ~70 CustomGroups -// } + // Suggestion from Totten in developer chat + // function testTooManyJoins() { + // $act = callApiSuccess('Activity', 'create', array(...)); + // for ($i = 0; $i < 70; $i++) { + // $cg = callApiSuccess('CustomGroup', 'create', array(...'label' => "My custom group $i"...)); + // $cf = callApiSuccess('CustomField', 'create', array(...'label' => "My custom field $i", 'custom_group_id' => $cg['id']...)); + // } + // callApiSuccess('Activity', 'get', array('id' => $act['id'])); + // } + // function tearDown() { + // // destroy the ~70 CustomGroups + // } } -