fix style issues

civicrm · Nov 13, 2020 · 53b8b75 · 53b8b75
1 parent eb82c6a
commit 53b8b75
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 31 deletions.
diff --git 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
@@ -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
@@ -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
+  // }
 
 }
-