From ec19219788e978e0e3f15a71031ffc3d3585c0c3 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Sat, 7 Jul 2018 19:31:36 -0700 Subject: [PATCH] (dev/core#217) PrevNext - Remove references to entity_table and entity_id2 from service The PrevNext service/interface aims to be a replaceable component for use the search-caching (but not deduping). The interface that we produced from refactoring includes several references to `entity_table` and `entity_id2` -- these values are part of the SQL table, and they're needed for dedupe, but they don't seem to convey anything meaningful for search-caching. Including these fields makes the interface more complicated -- which will make it hard to implement other variants. The general gist of this commit is that we no longer fill those two columns, and we no longer read them. In a couple functions, we split the new OOP implementation (`CRM_Core_PrevNextCache_Sql`) from the traditional static BAO implementation (`CRM_Core_BAO_PrevNext`) so that we can omit these fields. --- CRM/Campaign/Selector/Search.php | 2 +- CRM/Contact/Selector.php | 2 +- CRM/Core/PrevNextCache/Interface.php | 16 +++--- CRM/Core/PrevNextCache/Sql.php | 78 +++++++++++++++++----------- 4 files changed, 60 insertions(+), 38 deletions(-) diff --git a/CRM/Campaign/Selector/Search.php b/CRM/Campaign/Selector/Search.php index 28c2b15b6d..535a783495 100644 --- a/CRM/Campaign/Selector/Search.php +++ b/CRM/Campaign/Selector/Search.php @@ -282,7 +282,7 @@ class CRM_Campaign_Selector_Search extends CRM_Core_Selector_Base implements CRM ); list($select, $from) = explode(' FROM ', $sql); $selectSQL = " - SELECT 'civicrm_contact', contact_a.id, contact_a.id, '$cacheKey', contact_a.display_name + SELECT '$cacheKey', contact_a.id, contact_a.display_name FROM {$from} "; diff --git a/CRM/Contact/Selector.php b/CRM/Contact/Selector.php index ad5bcdd0b0..f048612ab7 100644 --- a/CRM/Contact/Selector.php +++ b/CRM/Contact/Selector.php @@ -1038,7 +1038,7 @@ class CRM_Contact_Selector extends CRM_Core_Selector_Base implements CRM_Core_Se // the other alternative of running the FULL query will just be incredibly inefficient // and slow things down way too much on large data sets / complex queries - $selectSQL = "SELECT DISTINCT 'civicrm_contact', contact_a.id, contact_a.id, '$cacheKey', contact_a.sort_name"; + $selectSQL = "SELECT DISTINCT '$cacheKey', contact_a.id, contact_a.sort_name"; $sql = str_replace(array("SELECT contact_a.id as contact_id", "SELECT contact_a.id as id"), $selectSQL, $sql); try { diff --git a/CRM/Core/PrevNextCache/Interface.php b/CRM/Core/PrevNextCache/Interface.php index c71b57ed24..edaf0d6778 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): entity_table, entity_id1, entity_id2, cacheKey, data + * the following columns (in order): cacheKey, entity_id1, data * @return bool */ public function fillWithSql($cacheKey, $sql); @@ -50,9 +50,7 @@ interface CRM_Core_PrevNextCache_Interface { * @param string $cacheKey * @param array $rows * A list of cache records. Each record should have keys: - * - entity_table * - entity_id1 - * - entity_id2 * - data * @return bool */ @@ -89,20 +87,24 @@ interface CRM_Core_PrevNextCache_Interface { * * @param string $cacheKey * @param int $id1 - * @param int $id2 * * @return array + * List of neighbors. + * [ + * 'foundEntry' => 1, + * 'prev' => ['id1' => 123, 'data'=>'foo'], + * 'next' => ['id1' => 456, 'data'=>'foo'], + * ] */ - public function getPositions($cacheKey, $id1, $id2); + public function getPositions($cacheKey, $id1); /** * Delete an item from the prevnext cache table based on the entity. * * @param int $id * @param string $cacheKey - * @param string $entityTable */ - public function deleteItem($id = NULL, $cacheKey = NULL, $entityTable = 'civicrm_contact'); + public function deleteItem($id = NULL, $cacheKey = NULL); /** * Get count of matching rows. diff --git a/CRM/Core/PrevNextCache/Sql.php b/CRM/Core/PrevNextCache/Sql.php index 6483fda8ac..5a58973533 100644 --- a/CRM/Core/PrevNextCache/Sql.php +++ b/CRM/Core/PrevNextCache/Sql.php @@ -37,13 +37,13 @@ class CRM_Core_PrevNextCache_Sql implements CRM_Core_PrevNextCache_Interface { * * @param string $sql * A SQL query. The query *MUST* be a SELECT statement which yields - * the following columns (in order): entity_table, entity_id1, entity_id2, cacheKey, data + * the following columns (in order): cacheKey, entity_id1, data * @return bool * @throws CRM_Core_Exception */ public function fillWithSql($cacheKey, $sql) { $insertSQL = " -INSERT INTO civicrm_prevnext_cache ( entity_table, entity_id1, entity_id2, cacheKey, data ) +INSERT INTO civicrm_prevnext_cache (cacheKey, entity_id1, data) "; $result = CRM_Core_DAO::executeQuery($insertSQL . $sql, [], FALSE, NULL, FALSE, TRUE, TRUE); if (is_a($result, 'DB_Error')) { @@ -59,9 +59,7 @@ INSERT INTO civicrm_prevnext_cache ( entity_table, entity_id1, entity_id2, cache $insert = CRM_Utils_SQL_Insert::into('civicrm_prevnext_cache') ->columns([ - 'entity_table', 'entity_id1', - 'entity_id2', 'cacheKey', 'data' ]); @@ -85,14 +83,11 @@ INSERT INTO civicrm_prevnext_cache ( entity_table, entity_id1, entity_id2, cache * To unselect all contact IDs, use NULL. */ public function markSelection($cacheKey, $action, $cIds = NULL) { - $entity_table = 'civicrm_contact'; - if (!$cacheKey) { return; } $params = array(); - $entity_whereClause = " AND entity_table = '{$entity_table}'"; if ($cIds && $cacheKey && $action) { if (is_array($cIds)) { $cIdFilter = "(" . implode(',', $cIds) . ")"; @@ -110,12 +105,12 @@ AND (entity_id1 = %2 OR entity_id2 = %2) } if ($action == 'select') { $whereClause .= "AND is_selected = 0"; - $sql = "UPDATE civicrm_prevnext_cache SET is_selected = 1 {$whereClause} {$entity_whereClause}"; + $sql = "UPDATE civicrm_prevnext_cache SET is_selected = 1 {$whereClause}"; $params[1] = array($cacheKey, 'String'); } elseif ($action == 'unselect') { $whereClause .= "AND is_selected = 1"; - $sql = "UPDATE civicrm_prevnext_cache SET is_selected = 0 {$whereClause} {$entity_whereClause}"; + $sql = "UPDATE civicrm_prevnext_cache SET is_selected = 0 {$whereClause}"; $params[1] = array($cacheKey, 'String'); } // default action is reseting @@ -125,7 +120,6 @@ AND (entity_id1 = %2 OR entity_id2 = %2) UPDATE civicrm_prevnext_cache SET is_selected = 0 WHERE cacheKey = %1 AND is_selected = 1 - {$entity_whereClause} "; $params[1] = array($cacheKey, 'String'); } @@ -145,21 +139,17 @@ WHERE cacheKey = %1 AND is_selected = 1 * @return array|NULL */ public function getSelection($cacheKey, $action = 'get') { - $entity_table = 'civicrm_contact'; - if (!$cacheKey) { return NULL; } $params = array(); - $entity_whereClause = " AND entity_table = '{$entity_table}'"; if ($cacheKey && ($action == 'get' || $action == 'getall')) { $actionGet = ($action == "get") ? " AND is_selected = 1 " : ""; $sql = " -SELECT entity_id1, entity_id2 FROM civicrm_prevnext_cache +SELECT entity_id1 FROM civicrm_prevnext_cache WHERE cacheKey = %1 $actionGet - $entity_whereClause ORDER BY id "; $params[1] = array($cacheKey, 'String'); @@ -167,9 +157,7 @@ ORDER BY id $contactIds = array($cacheKey => array()); $cIdDao = CRM_Core_DAO::executeQuery($sql, $params); while ($cIdDao->fetch()) { - if ($cIdDao->entity_id1 == $cIdDao->entity_id2) { - $contactIds[$cacheKey][$cIdDao->entity_id1] = 1; - } + $contactIds[$cacheKey][$cIdDao->entity_id1] = 1; } return $contactIds; } @@ -180,15 +168,46 @@ ORDER BY id * * @param string $cacheKey * @param int $id1 - * @param int $id2 - * - * NOTE: I don't really get why there are two ID columns, but we'll - * keep passing them through as a matter of safe-refactoring. * * @return array */ - public function getPositions($cacheKey, $id1, $id2) { - return CRM_Core_BAO_PrevNextCache::getPositions($cacheKey, $id1, $id2); + public function getPositions($cacheKey, $id1) { + $mergeId = CRM_Core_DAO::singleValueQuery( + "SELECT id FROM civicrm_prevnext_cache WHERE cacheKey = %2 AND entity_id1 = %1", + [ + 1 => [$id1, 'Integer'], + 2 => [$cacheKey, 'String'], + ] + ); + + $pos = ['foundEntry' => 0]; + if ($mergeId) { + $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"; + $p = [ + 1 => [$mergeId, 'Integer'], + 2 => [$cacheKey, 'String'], + ]; + + $dao = CRM_Core_DAO::executeQuery($sql . $wherePrev, $p); + if ($dao->fetch()) { + $pos['prev']['id1'] = $dao->entity_id1; + $pos['prev']['mergeId'] = $dao->id; + $pos['prev']['data'] = $dao->data; + } + + $dao = CRM_Core_DAO::executeQuery($sql . $whereNext, $p); + if ($dao->fetch()) { + $pos['next']['id1'] = $dao->entity_id1; + $pos['next']['mergeId'] = $dao->id; + $pos['next']['data'] = $dao->data; + } + } + return $pos; + } /** @@ -196,11 +215,10 @@ ORDER BY id * * @param int $id * @param string $cacheKey - * @param string $entityTable */ - public function deleteItem($id = NULL, $cacheKey = NULL, $entityTable = 'civicrm_contact') { - $sql = "DELETE FROM civicrm_prevnext_cache WHERE entity_table = %1"; - $params = array(1 => array($entityTable, 'String')); + public function deleteItem($id = NULL, $cacheKey = NULL) { + $sql = "DELETE FROM civicrm_prevnext_cache WHERE (1)"; + $params = array(); if (is_numeric($id)) { $sql .= " AND ( entity_id1 = %2 OR entity_id2 = %2 )"; @@ -221,7 +239,9 @@ ORDER BY id * @return int */ public function getCount($cacheKey) { - return CRM_Core_BAO_PrevNextCache::getCount($cacheKey, NULL, "entity_table = 'civicrm_contact'"); + $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); } } -- 2.25.1