From 6842bb53eddfd3b9b07d2cf630be693a4b0cdaea Mon Sep 17 00:00:00 2001
From: "Donald A. Lobo" <lobo@civicrm.org>
Date: Mon, 14 Oct 2013 10:07:45 -0700
Subject: [PATCH] CRM-11794 - shorten trigger names and add random string to
ensure uniqness
----------------------------------------
* CRM-11794: Drop Trigger IF Exists fatals if custom table too long
http://issues.civicrm.org/jira/browse/CRM-11794
---
CRM/Core/BAO/CustomGroup.php | 2 +-
CRM/Core/DAO.php | 34 +++++++++++++++++++++-
CRM/Logging/Schema.php | 46 ++++++++++++++++--------------
tests/phpunit/CRM/Core/DAOTest.php | 20 +++++++++++++
4 files changed, 78 insertions(+), 24 deletions(-)
diff --git a/CRM/Core/BAO/CustomGroup.php b/CRM/Core/BAO/CustomGroup.php
index 83636eb6be..1922e6656d 100644
--- a/CRM/Core/BAO/CustomGroup.php
+++ b/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;
diff --git a/CRM/Core/DAO.php b/CRM/Core/DAO.php
index 3420ca100a..395552221f 100644
--- a/CRM/Core/DAO.php
+++ b/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}";
+ }
+
}
diff --git a/CRM/Logging/Schema.php b/CRM/Logging/Schema.php
index a43cc30340..bb7efcd098 100644
--- a/CRM/Logging/Schema.php
+++ b/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;
diff --git a/tests/phpunit/CRM/Core/DAOTest.php b/tests/phpunit/CRM/Core/DAOTest.php
index 946de3836c..d6eedd98fd 100644
--- a/tests/phpunit/CRM/Core/DAOTest.php
+++ b/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));
+ }
+
}
--
GitLab