Merge pull request #1793 from dlobo/CRM-11794

CRM-11794 - shorten trigger names and add random string to ensure uniqne...

CRM/Core/BAO/CustomGroup.php

@@ -166,7 +166,7 @@ class CRM_Core_BAO_CustomGroup extends CRM_Core_DAO_CustomGroup {
     $group->save();
     if (!isset($params['id'])) {
       if (!isset($params['table_name'])) {
-        $munged_title = strtolower(CRM_Utils_String::munge($group->title, '_', 32));
+        $munged_title = strtolower(CRM_Utils_String::munge($group->title, '_', 42));
         $tableName = "civicrm_value_{$munged_title}_{$group->id}";
       }
       $group->table_name = $tableName;

CRM/Core/DAO.php

@@ -1650,7 +1650,8 @@ SELECT contact_id
         foreach ($events as $whenName => $parts) {
           $varString   = implode("\n", $parts['variables']);
           $sqlString   = implode("\n", $parts['sql']);
-          $triggerName = "{$tableName}_{$whenName}_{$eventName}";
+          $validName   = CRM_Core_DAO::shortenSQLName($tableName, 48, TRUE);
+          $triggerName = "{$validName}_{$whenName}_{$eventName}";
           $triggerSQL  = "CREATE TRIGGER $triggerName $whenName $eventName ON $tableName FOR EACH ROW BEGIN $varString $sqlString END";
 
           CRM_Core_DAO::executeQuery("DROP TRIGGER IF EXISTS $triggerName");
@@ -1895,4 +1896,35 @@ EOS;
       }
     }
   }
+
+  /**
+   * SQL has a limit of 64 characters on various names:
+   * table name, trigger name, column name ...
+   *
+   * For custom groups and fields we generated names from user entered input
+   * which can be longer than this length, this function helps with creating
+   * strings that meet various criteria.
+   *
+   * @param string $string - the string to be shortened
+   * @param int    $length - the max length of the string
+   */
+  public static function shortenSQLName($string, $length = 60, $makeRandom = FALSE) {
+    // early return for strings that meet the requirements
+    if (strlen($string) <= $length) {
+      return $string;
+    }
+
+    // easy return for calls that dont need a randomized uniq string
+    if (! $makeRandom) {
+      return substr($string, 0, $length);
+    }
+
+    // the string is longer than the length and we need a uniq string
+    // for the same tablename we need the same uniq string everytime
+    // hence we use md5 on the string, which is not random
+    // we'll append 16 characters to the end of the tableName
+    $md5string = substr(md5($string), 0, 16);
+    return substr($string, 0, $length - 17) . "_{$md5string}";
+  }
+
 }

CRM/Logging/Schema.php

@@ -46,11 +46,11 @@ class CRM_Logging_Schema {
     'logging/contribute/summary',
   );
 
-  //CRM-13028 / NYSS-6933 - table => array (cols) - to be excluded from the update statement 
+  //CRM-13028 / NYSS-6933 - table => array (cols) - to be excluded from the update statement
   private $exceptions = array(
     'civicrm_job'   => array('last_run'),
     'civicrm_group' => array('cache_date'),
-  ); 
+  );
 
   /**
    * Populate $this->tables and $this->logs with current db state.
@@ -153,15 +153,17 @@ AND    TABLE_NAME LIKE 'log_civicrm_%'
     }
 
     foreach ($tableNames as $table) {
+      $validName = CRM_Core_DAO::shortenSQLName($table, 48, TRUE);
+
       // before triggers
-      $dao->executeQuery("DROP TRIGGER IF EXISTS {$table}_before_insert");
-      $dao->executeQuery("DROP TRIGGER IF EXISTS {$table}_before_update");
-      $dao->executeQuery("DROP TRIGGER IF EXISTS {$table}_before_delete");
+      $dao->executeQuery("DROP TRIGGER IF EXISTS {$validName}_before_insert");
+      $dao->executeQuery("DROP TRIGGER IF EXISTS {$validName}_before_update");
+      $dao->executeQuery("DROP TRIGGER IF EXISTS {$validName}_before_delete");
 
      // after triggers
-      $dao->executeQuery("DROP TRIGGER IF EXISTS {$table}_after_insert");
-      $dao->executeQuery("DROP TRIGGER IF EXISTS {$table}_after_update");
-      $dao->executeQuery("DROP TRIGGER IF EXISTS {$table}_after_delete");
+      $dao->executeQuery("DROP TRIGGER IF EXISTS {$validName}_after_insert");
+      $dao->executeQuery("DROP TRIGGER IF EXISTS {$validName}_after_update");
+      $dao->executeQuery("DROP TRIGGER IF EXISTS {$validName}_after_delete");
     }
   }
 
@@ -356,7 +358,7 @@ AND    TABLE_NAME LIKE 'log_civicrm_%'
         $civiDB = $dao->_database;
       }
       CRM_Core_Error::ignoreException();
-      // NOTE: W.r.t Performance using one query to find all details and storing in static array is much faster 
+      // NOTE: W.r.t Performance using one query to find all details and storing in static array is much faster
       // than firing query for every given table.
       $query = "
 SELECT TABLE_SCHEMA, TABLE_NAME, COLUMN_NAME, DATA_TYPE, IS_NULLABLE, COLUMN_DEFAULT
@@ -371,11 +373,11 @@ WHERE  table_schema IN ('{$this->db}', '{$civiDB}')";
         if (!array_key_exists($dao->TABLE_NAME, $columnSpecs)) {
           $columnSpecs[$dao->TABLE_NAME] = array();
         }
-        $columnSpecs[$dao->TABLE_NAME][$dao->COLUMN_NAME] = 
+        $columnSpecs[$dao->TABLE_NAME][$dao->COLUMN_NAME] =
           array(
-              'COLUMN_NAME' => $dao->COLUMN_NAME, 
-              'DATA_TYPE'   => $dao->DATA_TYPE, 
-              'IS_NULLABLE' => $dao->IS_NULLABLE, 
+              'COLUMN_NAME' => $dao->COLUMN_NAME,
+              'DATA_TYPE'   => $dao->DATA_TYPE,
+              'IS_NULLABLE' => $dao->IS_NULLABLE,
               'COLUMN_DEFAULT' => $dao->COLUMN_DEFAULT
             );
       }
@@ -386,38 +388,38 @@ WHERE  table_schema IN ('{$this->db}', '{$civiDB}')";
   function columnsWithDiffSpecs($civiTable, $logTable) {
     $civiTableSpecs = $this->columnSpecsOf($civiTable);
     $logTableSpecs  = $this->columnSpecsOf($logTable);
-    
+
     $diff = array('ADD' => array(), 'MODIFY' => array(), 'OBSOLETE' => array());
-    
+
     // columns to be added
     $diff['ADD'] = array_diff(array_keys($civiTableSpecs), array_keys($logTableSpecs));
-    
+
     // columns to be modified
-    // NOTE: we consider only those columns for modifications where there is a spec change, and that the column definition 
+    // NOTE: we consider only those columns for modifications where there is a spec change, and that the column definition
     // wasn't deliberately modified by fixTimeStampAndNotNullSQL() method.
     foreach ($civiTableSpecs as $col => $colSpecs) {
       $specDiff = array_diff($civiTableSpecs[$col], $logTableSpecs[$col]);
       if (!empty($specDiff) && $col != 'id' && !array_key_exists($col, $diff['ADD'])) {
         // ignore 'id' column for any spec changes, to avoid any auto-increment mysql errors
         if ($civiTableSpecs[$col]['DATA_TYPE'] != $logTableSpecs[$col]['DATA_TYPE']) {
-          // if data-type is different, surely consider the column 
+          // if data-type is different, surely consider the column
           $diff['MODIFY'][] = $col;
-        } else if ($civiTableSpecs[$col]['IS_NULLABLE'] != $logTableSpecs[$col]['IS_NULLABLE'] && 
+        } else if ($civiTableSpecs[$col]['IS_NULLABLE'] != $logTableSpecs[$col]['IS_NULLABLE'] &&
           $logTableSpecs[$col]['IS_NULLABLE'] == 'NO') {
           // if is-null property is different, and log table's column is NOT-NULL, surely consider the column
           $diff['MODIFY'][] = $col;
-        } else if ($civiTableSpecs[$col]['COLUMN_DEFAULT'] != $logTableSpecs[$col]['COLUMN_DEFAULT'] && 
+        } else if ($civiTableSpecs[$col]['COLUMN_DEFAULT'] != $logTableSpecs[$col]['COLUMN_DEFAULT'] &&
           !strstr($civiTableSpecs[$col]['COLUMN_DEFAULT'], 'TIMESTAMP')) {
           // if default property is different, and its not about a timestamp column, consider it
           $diff['MODIFY'][] = $col;
         }
-      } 
+      }
     }
 
     // columns to made obsolete by turning into not-null
     $oldCols = array_diff(array_keys($logTableSpecs), array_keys($civiTableSpecs));
     foreach ($oldCols as $col) {
-      if (!in_array($col, array('log_date', 'log_conn_id', 'log_user_id', 'log_action')) && 
+      if (!in_array($col, array('log_date', 'log_conn_id', 'log_user_id', 'log_action')) &&
         $logTableSpecs[$col]['IS_NULLABLE'] == 'NO') {
         // if its a column present only in log table, not among those used by log tables for special purpose, and not-null
         $diff['OBSOLETE'][] = $col;

tests/phpunit/CRM/Core/DAOTest.php

@@ -159,4 +159,24 @@ class CRM_Core_DAOTest extends CiviUnitTestCase {
     $actualSql = CRM_Core_DAO::composeQuery($inputSql, $inputParams);
     $this->assertFalse(($expectSql == $actualSql));
   }
+
+  function sqlNameDataProvider() {
+    return array(
+      array('this is a long string', 30, FALSE, 'this is a long string'),
+
+      array('this is an even longer string which is exactly 60 character', 60, FALSE, 'this is an even longer string which is exactly 60 character'),
+      array('this is an even longer string which is exactly 60 character', 60, TRUE , 'this is an even longer string which is exactly 60 character'),
+
+      array('this is an even longer string which is a bit more than 60 character', 60, FALSE, 'this is an even longer string which is a bit more than 60 ch'),
+      array('this is an even longer string which is a bit more than 60 character', 60, TRUE , 'this is an even longer string which is a bi_c1cbd5198187eb96'),
+    );
+  }
+
+  /**
+   * @dataProvider sqlNameDataProvider
+   */
+  function testShortenSQLName($inputData, $length, $makeRandom, $expectedResult) {
+    $this->assertEquals($expectedResult, CRM_Core_DAO::shortenSQLName($inputData, $length, $makeRandom));
+  }
+
 }