From 783b4b21de9b1bd430be3611bb376680a4b3b218 Mon Sep 17 00:00:00 2001 From: Seamus Lee Date: Tue, 28 May 2019 08:45:33 +1000 Subject: [PATCH] Fix name of cache key column to be all lower case rather than camel case in civicrm_prevnext_cache Fix test failures --- CRM/Contact/Page/AJAX.php | 2 +- CRM/Core/BAO/PrevNextCache.php | 30 +++++++++---------- CRM/Core/DAO/PrevNextCache.php | 14 ++++----- CRM/Core/PrevNextCache/Interface.php | 2 +- CRM/Core/PrevNextCache/Sql.php | 28 ++++++++--------- CRM/Upgrade/Incremental/php/FiveFifteen.php | 8 +++++ api/v3/Dedupe.php | 11 ------- tests/phpunit/CRM/Contact/SelectorTest.php | 6 ++-- .../CRM/Core/BAO/SchemaHandlerTest.php | 2 +- xml/schema/Core/PrevNextCache.xml | 4 +-- 10 files changed, 52 insertions(+), 55 deletions(-) diff --git a/CRM/Contact/Page/AJAX.php b/CRM/Contact/Page/AJAX.php index a0df0b92ddb3..da32d7329f1b 100644 --- a/CRM/Contact/Page/AJAX.php +++ b/CRM/Contact/Page/AJAX.php @@ -1047,7 +1047,7 @@ public static function toggleDedupeSelect() { $params[2] = [$pnid, 'Integer']; } - $sql = "UPDATE civicrm_prevnext_cache SET is_selected = %1 WHERE {$whereClause} AND cacheKey LIKE %3"; + $sql = "UPDATE civicrm_prevnext_cache SET is_selected = %1 WHERE {$whereClause} AND cachekey LIKE %3"; CRM_Core_DAO::executeQuery($sql, $params); CRM_Utils_System::civiExit(); diff --git a/CRM/Core/BAO/PrevNextCache.php b/CRM/Core/BAO/PrevNextCache.php index 7679ff9977ef..216736f18bc4 100644 --- a/CRM/Core/BAO/PrevNextCache.php +++ b/CRM/Core/BAO/PrevNextCache.php @@ -58,7 +58,7 @@ public static function getPositions($cacheKey, $id1, $id2, &$mergeId = NULL, $jo $query = " SELECT id FROM civicrm_prevnext_cache -WHERE cacheKey = %3 AND +WHERE cachekey = %3 AND entity_id1 = %1 AND entity_id2 = %2 AND entity_table = 'civicrm_contact' @@ -87,7 +87,7 @@ public static function getPositions($cacheKey, $id1, $id2, &$mergeId = NULL, $jo 2 => [$cacheKey, 'String'], ]; $sql = "SELECT pn.id, pn.entity_id1, pn.entity_id2, pn.data FROM civicrm_prevnext_cache pn {$join} "; - $wherePrev = " WHERE pn.id < %1 AND pn.cacheKey = %2 {$where} ORDER BY ID DESC LIMIT 1"; + $wherePrev = " WHERE pn.id < %1 AND pn.cachekey = %2 {$where} ORDER BY ID DESC LIMIT 1"; $sqlPrev = $sql . $wherePrev; $dao = CRM_Core_DAO::executeQuery($sqlPrev, $p); @@ -98,7 +98,7 @@ public static function getPositions($cacheKey, $id1, $id2, &$mergeId = NULL, $jo $pos['prev']['data'] = $dao->data; } - $whereNext = " WHERE pn.id > %1 AND pn.cacheKey = %2 {$where} ORDER BY ID ASC LIMIT 1"; + $whereNext = " WHERE pn.id > %1 AND pn.cachekey = %2 {$where} ORDER BY ID ASC LIMIT 1"; $sqlNext = $sql . $whereNext; $dao = CRM_Core_DAO::executeQuery($sqlNext, $p); @@ -131,7 +131,7 @@ public static function deleteItem($id = NULL, $cacheKey = NULL, $entityTable = ' } if (isset($cacheKey)) { - $sql .= " AND cacheKey LIKE %3"; + $sql .= " AND cachekey LIKE %3"; $params[3] = ["{$cacheKey}%", 'String']; } CRM_Core_DAO::executeQuery($sql, $params); @@ -155,7 +155,7 @@ public static function deletePair($id1, $id2, $cacheKey = NULL) { $params[3] = [$id2, 'Integer']; if (isset($cacheKey)) { - $sql .= " AND cacheKey LIKE %4"; + $sql .= " AND cachekey LIKE %4"; // used % to address any row with conflict-cacheKey e.g "merge Individual_8_0_conflicts" $params[4] = ["{$cacheKey}%", 'String']; } @@ -182,7 +182,7 @@ public static function markConflict($id1, $id2, $cacheKey, $conflicts) { FROM civicrm_prevnext_cache pn WHERE ((pn.entity_id1 = %1 AND pn.entity_id2 = %2) OR (pn.entity_id1 = %2 AND pn.entity_id2 = %1)) AND - (cacheKey = %3 OR cacheKey = %4)"; + (cachekey = %3 OR cachekey = %4)"; $params = [ 1 => [$id1, 'Integer'], 2 => [$id2, 'Integer'], @@ -201,7 +201,7 @@ public static function markConflict($id1, $id2, $cacheKey, $conflicts) { $pncUp->id = $pncFind->id; if ($pncUp->find(TRUE)) { $pncUp->data = serialize($data); - $pncUp->cacheKey = "{$cacheKey}_conflicts"; + $pncUp->cachekey = "{$cacheKey}_conflicts"; $pncUp->save(); } } @@ -251,11 +251,11 @@ public static function retrieve($cacheKey, $join = NULL, $whereClause = NULL, $o $whereClause = " AND " . $whereClause; } if ($includeConflicts) { - $where = ' WHERE (pn.cacheKey = %1 OR pn.cacheKey = %2)' . $whereClause; + $where = ' WHERE (pn.cachekey = %1 OR pn.cachekey = %2)' . $whereClause; $params[2] = ["{$cacheKey}_conflicts", 'String']; } else { - $where = ' WHERE (pn.cacheKey = %1)' . $whereClause; + $where = ' WHERE (pn.cachekey = %1)' . $whereClause; } $query = " @@ -318,7 +318,7 @@ public static function is_serialized($string) { * @param $values */ public static function setItem($values) { - $insert = "INSERT INTO civicrm_prevnext_cache ( entity_table, entity_id1, entity_id2, cacheKey, data ) VALUES \n"; + $insert = "INSERT INTO civicrm_prevnext_cache ( entity_table, entity_id1, entity_id2, cachekey, data ) VALUES \n"; $query = $insert . implode(",\n ", $values); //dump the dedupe matches in the prevnext_cache table @@ -341,7 +341,7 @@ public static function getCount($cacheKey, $join = NULL, $where = NULL, $op = "= $query = " SELECT COUNT(*) FROM civicrm_prevnext_cache pn {$join} -WHERE (pn.cacheKey $op %1 OR pn.cacheKey $op %2) +WHERE (pn.cachekey $op %1 OR pn.cachekey $op %2) "; if ($where) { $query .= " AND {$where}"; @@ -385,7 +385,7 @@ public static function refillCache($rgid, $gid, $cacheKeyString, $criteria, $che } // 1. Clear cache if any - $sql = "DELETE FROM civicrm_prevnext_cache WHERE cacheKey LIKE %1"; + $sql = "DELETE FROM civicrm_prevnext_cache WHERE cachekey LIKE %1"; CRM_Core_DAO::executeQuery($sql, [1 => ["{$cacheKeyString}%", 'String']]); // FIXME: we need to start using temp tables / queries here instead of arrays. @@ -427,7 +427,7 @@ public static function cleanupCache() { $sql = " DELETE pn, c FROM civicrm_cache c -INNER JOIN civicrm_prevnext_cache pn ON c.path = pn.cacheKey +INNER JOIN civicrm_prevnext_cache pn ON c.path = pn.cachekey WHERE c.group_name = %1 AND c.created_date < date_sub( NOW( ), INTERVAL %2 day ) "; @@ -507,8 +507,8 @@ public static function getPrevNextBackends() { * The globally-unique ID of the test object. */ protected function assignTestValue($fieldName, &$fieldDef, $counter) { - if ($fieldName === 'cacheKey') { - $this->cacheKey = 'merge_' . rand(); + if ($fieldName === 'cachekey') { + $this->cachekey = 'merge_' . rand(); return; } if ($fieldName === 'data') { diff --git a/CRM/Core/DAO/PrevNextCache.php b/CRM/Core/DAO/PrevNextCache.php index 682f6984c40c..190c88939592 100644 --- a/CRM/Core/DAO/PrevNextCache.php +++ b/CRM/Core/DAO/PrevNextCache.php @@ -6,7 +6,7 @@ * * Generated from xml/schema/CRM/Core/PrevNextCache.xml * DO NOT EDIT. Generated by CRM_Core_CodeGen - * (GenCodeChecksum:dba140c3d2ece863c512ed688df2ebcb) + * (GenCodeChecksum:f67e7fde19a780da589bcf8a0fdd77bf) */ /** @@ -59,7 +59,7 @@ class CRM_Core_DAO_PrevNextCache extends CRM_Core_DAO { * * @var string */ - public $cacheKey; + public $cachekey; /** * cached snapshot of the serialized data @@ -137,14 +137,14 @@ public static function &fields() { 'bao' => 'CRM_Core_BAO_PrevNextCache', 'localizable' => 0, ], - 'cacheKey' => [ - 'name' => 'cacheKey', + 'cachekey' => [ + 'name' => 'cachekey', 'type' => CRM_Utils_Type::T_STRING, 'title' => ts('Cache Key'), 'description' => ts('Unique path name for cache element of the searched item'), 'maxlength' => 255, 'size' => CRM_Utils_Type::HUGE, - 'where' => 'civicrm_prevnext_cache.cacheKey', + 'where' => 'civicrm_prevnext_cache.cachekey', 'table_name' => 'civicrm_prevnext_cache', 'entity' => 'PrevNextCache', 'bao' => 'CRM_Core_BAO_PrevNextCache', @@ -246,14 +246,14 @@ public static function indices($localize = TRUE) { 'index_all' => [ 'name' => 'index_all', 'field' => [ - 0 => 'cacheKey', + 0 => 'cachekey', 1 => 'entity_id1', 2 => 'entity_id2', 3 => 'entity_table', 4 => 'is_selected', ], 'localizable' => FALSE, - 'sig' => 'civicrm_prevnext_cache::0::cacheKey::entity_id1::entity_id2::entity_table::is_selected', + 'sig' => 'civicrm_prevnext_cache::0::cachekey::entity_id1::entity_id2::entity_table::is_selected', ], ]; return ($localize && !empty($indices)) ? CRM_Core_DAO_AllCoreTables::multilingualize(__CLASS__, $indices) : $indices; diff --git a/CRM/Core/PrevNextCache/Interface.php b/CRM/Core/PrevNextCache/Interface.php index 54048d26eb2c..c11000710999 100644 --- a/CRM/Core/PrevNextCache/Interface.php +++ b/CRM/Core/PrevNextCache/Interface.php @@ -39,7 +39,7 @@ interface CRM_Core_PrevNextCache_Interface { * @param string $cacheKey * @param string $sql * A SQL query. The query *MUST* be a SELECT statement which yields - * the following columns (in order): cacheKey, entity_id1, data + * the following columns (in order): cachekey, entity_id1, data * @param array $sqlParams * An array of parameters to be used with $sql. * Use the same interpolation format as CRM_Core_DAO (composeQuery/executeQuery). diff --git a/CRM/Core/PrevNextCache/Sql.php b/CRM/Core/PrevNextCache/Sql.php index 3fed173b539b..86e1034ff364 100644 --- a/CRM/Core/PrevNextCache/Sql.php +++ b/CRM/Core/PrevNextCache/Sql.php @@ -37,7 +37,7 @@ class CRM_Core_PrevNextCache_Sql implements CRM_Core_PrevNextCache_Interface { * @param string $cacheKey * @param string $sql * A SQL query. The query *MUST* be a SELECT statement which yields - * the following columns (in order): cacheKey, entity_id1, data + * the following columns (in order): cachekey, entity_id1, data * @param array $sqlParams * An array of parameters to be used with $sql. * Use the same interpolation format as CRM_Core_DAO (composeQuery/executeQuery). @@ -48,7 +48,7 @@ class CRM_Core_PrevNextCache_Sql implements CRM_Core_PrevNextCache_Interface { */ public function fillWithSql($cacheKey, $sql, $sqlParams = []) { $insertSQL = " -INSERT INTO civicrm_prevnext_cache (cacheKey, entity_id1, data) +INSERT INTO civicrm_prevnext_cache (cachekey, entity_id1, data) "; $result = CRM_Core_DAO::executeQuery($insertSQL . $sql, $sqlParams, FALSE, NULL, FALSE, TRUE, TRUE); if (is_a($result, 'DB_Error')) { @@ -65,12 +65,12 @@ public function fillWithArray($cacheKey, $rows) { $insert = CRM_Utils_SQL_Insert::into('civicrm_prevnext_cache') ->columns([ 'entity_id1', - 'cacheKey', + 'cachekey', 'data', ]); foreach ($rows as &$row) { - $insert->row($row + ['cacheKey' => $cacheKey]); + $insert->row($row + ['cachekey' => $cacheKey]); } CRM_Core_DAO::executeQuery($insert->toSQL()); @@ -97,13 +97,13 @@ public function markSelection($cacheKey, $action, $ids = NULL) { if (is_array($ids)) { $cIdFilter = "(" . implode(',', $ids) . ")"; $whereClause = " -WHERE cacheKey = %1 +WHERE cachekey = %1 AND (entity_id1 IN {$cIdFilter} OR entity_id2 IN {$cIdFilter}) "; } else { $whereClause = " -WHERE cacheKey = %1 +WHERE cachekey = %1 AND (entity_id1 = %2 OR entity_id2 = %2) "; $params[2] = ["{$ids}", 'Integer']; @@ -124,7 +124,7 @@ public function markSelection($cacheKey, $action, $ids = NULL) { $sql = " UPDATE civicrm_prevnext_cache SET is_selected = 0 -WHERE cacheKey = %1 AND is_selected = 1 +WHERE cachekey = %1 AND is_selected = 1 "; $params[1] = [$cacheKey, 'String']; } @@ -153,7 +153,7 @@ public function getSelection($cacheKey, $action = 'get') { $actionGet = ($action == "get") ? " AND is_selected = 1 " : ""; $sql = " SELECT entity_id1 FROM civicrm_prevnext_cache -WHERE cacheKey = %1 +WHERE cachekey = %1 $actionGet ORDER BY id "; @@ -178,7 +178,7 @@ public function getSelection($cacheKey, $action = 'get') { */ public function getPositions($cacheKey, $id1) { $mergeId = CRM_Core_DAO::singleValueQuery( - "SELECT id FROM civicrm_prevnext_cache WHERE cacheKey = %2 AND entity_id1 = %1", + "SELECT id FROM civicrm_prevnext_cache WHERE cachekey = %2 AND entity_id1 = %1", [ 1 => [$id1, 'Integer'], 2 => [$cacheKey, 'String'], @@ -190,8 +190,8 @@ public function getPositions($cacheKey, $id1) { $pos['foundEntry'] = 1; $sql = "SELECT pn.id, pn.entity_id1, pn.entity_id2, pn.data FROM civicrm_prevnext_cache pn "; - $wherePrev = " WHERE pn.id < %1 AND pn.cacheKey = %2 ORDER BY ID DESC LIMIT 1"; - $whereNext = " WHERE pn.id > %1 AND pn.cacheKey = %2 ORDER BY ID ASC LIMIT 1"; + $wherePrev = " WHERE pn.id < %1 AND pn.cachekey = %2 ORDER BY ID DESC LIMIT 1"; + $whereNext = " WHERE pn.id > %1 AND pn.cachekey = %2 ORDER BY ID ASC LIMIT 1"; $p = [ 1 => [$mergeId, 'Integer'], 2 => [$cacheKey, 'String'], @@ -231,7 +231,7 @@ public function deleteItem($id = NULL, $cacheKey = NULL) { } if (isset($cacheKey)) { - $sql .= " AND cacheKey = %3"; + $sql .= " AND cachekey = %3"; $params[3] = [$cacheKey, 'String']; } CRM_Core_DAO::executeQuery($sql, $params); @@ -244,7 +244,7 @@ public function deleteItem($id = NULL, $cacheKey = NULL) { * @return int */ public function getCount($cacheKey) { - $query = "SELECT COUNT(*) FROM civicrm_prevnext_cache pn WHERE pn.cacheKey = %1"; + $query = "SELECT COUNT(*) FROM civicrm_prevnext_cache pn WHERE pn.cachekey = %1"; $params = [1 => [$cacheKey, 'String']]; return (int) CRM_Core_DAO::singleValueQuery($query, $params, TRUE, FALSE); } @@ -261,7 +261,7 @@ public function getCount($cacheKey) { public function fetch($cacheKey, $offset, $rowCount) { $cids = []; $dao = CRM_Utils_SQL_Select::from('civicrm_prevnext_cache pnc') - ->where('pnc.cacheKey = @cacheKey', ['cacheKey' => $cacheKey]) + ->where('pnc.cachekey = @cacheKey', ['cacheKey' => $cacheKey]) ->select('pnc.entity_id1 as cid') ->orderBy('pnc.id') ->limit($rowCount, $offset) diff --git a/CRM/Upgrade/Incremental/php/FiveFifteen.php b/CRM/Upgrade/Incremental/php/FiveFifteen.php index 2cfb6a2d3396..95248a1a9f3c 100644 --- a/CRM/Upgrade/Incremental/php/FiveFifteen.php +++ b/CRM/Upgrade/Incremental/php/FiveFifteen.php @@ -75,6 +75,14 @@ public function setPostUpgradeMessage(&$postUpgradeMessage, $rev) { public function upgrade_5_15_alpha1($rev) { $this->addTask(ts('Upgrade DB to %1: SQL', [1 => $rev]), 'runSql', $rev); $this->addTask('Fix errant deferred revenue settings', 'updateContributeSettings'); + $this->addTask('Fix cache key column name in prev next cache', 'fixCacheKeyColumnNamePrevNext'); + } + + public static function fixCacheKeyColumnNamePrevNext() { + CRM_Core_BAO_SchemaHandler::dropIndexIfExists('civicrm_prevnext_cache', 'index_all'); + CRM_Core_DAO::executeQuery("ALTER TABLE civicrm_prevnext_cache CHANGE COLUMN cacheKey cachekey VARCHAR(255) COMMENT 'Unique path name for cache element of the searched item'"); + CRM_Core_DAO::executeQuery("CREATE INDEX index_all ON civicrm_prevnext_cache (cachekey, entity_id1, entity_id2, entity_table, is_selected)"); + return TRUE; } } diff --git a/api/v3/Dedupe.php b/api/v3/Dedupe.php index e98f1d14fff5..bcd3ada43513 100644 --- a/api/v3/Dedupe.php +++ b/api/v3/Dedupe.php @@ -43,13 +43,6 @@ function civicrm_api3_dedupe_get($params) { $sql = CRM_Utils_SQL_Select::fragment(); $sql->where(['merge_data_restriction' => "cachekey LIKE 'merge_%'"]); - if (isset($params['cachekey'])) { - // This is so bad. We actually have a camel case field name in the DB. Don't do that. - // Intercept the pain here. - $params['cacheKey'] = $params['cachekey']; - unset($params['cachekey']); - } - $options = _civicrm_api3_get_options_from_params($params, TRUE, 'PrevNextCache', 'get'); $result = _civicrm_api3_basic_get('CRM_Core_BAO_PrevNextCache', $params, FALSE, 'PrevNextCache', $sql); @@ -60,10 +53,6 @@ function civicrm_api3_dedupe_get($params) { if (isset($values['data']) && !empty($values['data'])) { $result[$index]['data'] = unserialize($values['data']); } - if (isset($values['cacheKey'])) { - $result[$index]['cachekey'] = $result[$index]['cacheKey']; - unset($result[$index]['cacheKey']); - } } return civicrm_api3_create_success($result, $params, 'PrevNextCache'); } diff --git a/tests/phpunit/CRM/Contact/SelectorTest.php b/tests/phpunit/CRM/Contact/SelectorTest.php index 67e9a6321c32..fcf065fb8c01 100644 --- a/tests/phpunit/CRM/Contact/SelectorTest.php +++ b/tests/phpunit/CRM/Contact/SelectorTest.php @@ -155,8 +155,8 @@ public function testPrevNextCache() { // build cache key and use to it to fetch prev-next cache record $cacheKey = 'civicrm search ' . $key; $contacts = CRM_Utils_SQL_Select::from('civicrm_prevnext_cache') - ->select(['entity_id1', 'cacheKey']) - ->where("cacheKey = @key") + ->select(['entity_id1', 'cachekey']) + ->where("cachekey = @key") ->param('key', $cacheKey) ->execute() ->fetchAll(); @@ -164,7 +164,7 @@ public function testPrevNextCache() { // check the prevNext record matches $expectedEntry = [ 'entity_id1' => $contactID, - 'cacheKey' => $cacheKey, + 'cachekey' => $cacheKey, ]; $this->checkArrayEquals($contacts[0], $expectedEntry); } diff --git a/tests/phpunit/CRM/Core/BAO/SchemaHandlerTest.php b/tests/phpunit/CRM/Core/BAO/SchemaHandlerTest.php index b9584b15daf5..ca5345dd51bf 100644 --- a/tests/phpunit/CRM/Core/BAO/SchemaHandlerTest.php +++ b/tests/phpunit/CRM/Core/BAO/SchemaHandlerTest.php @@ -256,7 +256,7 @@ public function testPartialIndices() { ); CRM_Core_BAO_SchemaHandler::dropIndexIfExists('civicrm_prevnext_cache', 'index_all'); //Missing Column `is_selected`. - CRM_Core_DAO::executeQuery('CREATE INDEX index_all ON civicrm_prevnext_cache (cacheKey, entity_id1, entity_id2, entity_table)'); + CRM_Core_DAO::executeQuery('CREATE INDEX index_all ON civicrm_prevnext_cache (cachekey, entity_id1, entity_id2, entity_table)'); $missingIndices = CRM_Core_BAO_SchemaHandler::getMissingIndices(); $this->assertNotEmpty($missingIndices); diff --git a/xml/schema/Core/PrevNextCache.xml b/xml/schema/Core/PrevNextCache.xml index caee9ef7fa58..0af66d463b00 100644 --- a/xml/schema/Core/PrevNextCache.xml +++ b/xml/schema/Core/PrevNextCache.xml @@ -42,7 +42,7 @@ 3.4 - cacheKey + cachekey Cache Key varchar 255 @@ -66,7 +66,7 @@ index_all - cacheKey + cachekey entity_id1 entity_id2 entity_table