Skip to content

Commit

Permalink
Privatise prevNextCache functions
Browse files Browse the repository at this point in the history 
The buildPrevNextCache function had handling for the extensionised legacy
searches mixed in with the core searches - this gives each
it's own function, with the custom logic in the one on the Custom Selectore
& removed from the core one. In addition I determined that these functions
were only called from within their own classes & made them private
  • Loading branch information
@eileenmcnaughton
eileenmcnaughton committed Jan 24, 2023
1 parent 0a41fd5 commit 689a576
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 30 deletions.
2 changes: 1 addition & 1 deletion CRM/Campaign/Selector/Search.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ public function &getRows($action, $offset, $rowCount, $sort, $output = NULL) {
/**
* @param $sort
*/
public function buildPrevNextCache($sort) {
private function buildPrevNextCache($sort) {
//for prev/next pagination
$crmPID = CRM_Utils_Request::retrieve('crmPID', 'Integer');

Expand Down
35 changes: 8 additions & 27 deletions CRM/Contact/Selector.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -864,7 +864,7 @@ public function &getRows($action, $offset, $rowCount, $sort, $output = NULL) {
*
* @return string
*/
public function buildPrevNextCache($sort) {
private function buildPrevNextCache($sort) {
$cacheKey = 'civicrm search ' . $this->_key;

// We should clear the cache in following conditions:
Expand Down Expand Up @@ -1027,18 +1027,9 @@ public function removeActions(&$rows) {
*
* @throws \CRM_Core_Exception
*/
public function fillupPrevNextCache($sort, $cacheKey, $start = 0, $end = self::CACHE_SIZE) {
$coreSearch = TRUE;
// For custom searches, use the contactIDs method
if (is_a($this, 'CRM_Contact_Selector_Custom')) {
$sql = $this->_search->contactIDs($start, $end, $sort, TRUE);
$coreSearch = FALSE;
}
// For core searches use the searchQuery method
else {
$sql = $this->_query->getSearchSQL($start, $end, $sort, FALSE, $this->_query->_includeContactIds,
private function fillupPrevNextCache($sort, $cacheKey, $start = 0, $end = self::CACHE_SIZE) {
$sql = $this->_query->getSearchSQL($start, $end, $sort, FALSE, $this->_query->_includeContactIds,
FALSE, TRUE);
}

// CRM-9096
// due to limitations in our search query writer, the above query does not work
Expand All @@ -1049,7 +1040,7 @@ public function fillupPrevNextCache($sort, $cacheKey, $start = 0, $end = self::C
// 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 = CRM_Core_DAO::composeQuery("SELECT DISTINCT %1, contact_a.id, contact_a.sort_name", [1 => [$cacheKey, 'String']]);
$selectSQL = CRM_Core_DAO::composeQuery('SELECT DISTINCT %1, contact_a.id, contact_a.sort_name', [1 => [$cacheKey, 'String']]);

$sql = str_ireplace(['SELECT contact_a.id as contact_id', 'SELECT contact_a.id as id'], $selectSQL, $sql);
$sql = str_ireplace('ORDER BY `contact_id`', 'ORDER BY `id`', $sql, $sql);
Expand All @@ -1058,19 +1049,9 @@ public function fillupPrevNextCache($sort, $cacheKey, $start = 0, $end = self::C
Civi::service('prevnext')->fillWithSql($cacheKey, $sql);
}
catch (\Exception $e) {
if ($coreSearch) {
// in the case of error, try rebuilding cache using full sql which is used for search selector display
// this fixes the bugs reported in CRM-13996 & CRM-14438
$this->rebuildPreNextCache($start, $end, $sort, $cacheKey);
}
else {
CRM_Core_Error::deprecatedFunctionWarning('Custom searches should return sql capable of filling the prevnext cache.');
// This will always show for CiviRules :-( as a) it orders by 'rule_label'
// which is not available in the query & b) it uses contact not contact_a
// as an alias.
// CRM_Core_Session::setStatus(ts('Query Failed'));
return;
}
// in the case of error, try rebuilding cache using full sql which is used for search selector display
// this fixes the bugs reported in CRM-13996 & CRM-14438
$this->rebuildPreNextCache($start, $end, $sort, $cacheKey);
}

if (Civi::service('prevnext') instanceof CRM_Core_PrevNextCache_Sql) {
Expand All @@ -1091,7 +1072,7 @@ public function fillupPrevNextCache($sort, $cacheKey, $start = 0, $end = self::C
* @param string $cacheKey
* Cache key.
*/
public function rebuildPreNextCache($start, $end, $sort, $cacheKey) {
private function rebuildPreNextCache($start, $end, $sort, $cacheKey): void {
// generate full SQL
$sql = $this->_query->searchQuery($start, $end, $sort, FALSE, $this->_query->_includeContactIds,
FALSE, FALSE, TRUE);
Expand Down
104 changes: 102 additions & 2 deletions ext/legacycustomsearches/CRM/Contact/Selector/Custom.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
*
* @package CRM
* @copyright CiviCRM LLC https://civicrm.org/licensing
* $Id: Selector.php 11510 2007-09-18 09:21:34Z lobo $
*/

/**
Expand All @@ -28,7 +27,7 @@ class CRM_Contact_Selector_Custom extends CRM_Contact_Selector {
*
* @var array
*/
public static $_links = NULL;
public static $_links;

/**
* We use desc to remind us what that column is, name is used in the tpl
Expand Down Expand Up @@ -360,6 +359,107 @@ public function &getRows($action, $offset, $rowCount, $sort, $output = NULL) {
return $rows;
}

/**
* @param CRM_Utils_Sort $sort
*
* @return string
* @throws \CRM_Core_Exception
*/
private function buildPrevNextCache($sort): string {
$cacheKey = 'civicrm search ' . $this->_key;

// We should clear the cache in following conditions:
// 1. when starting from scratch, i.e new search
// 2. if records are sorted

// get current page requested
$pageNum = CRM_Utils_Request::retrieve('crmPID', 'Integer');

// get the current sort order
$currentSortID = CRM_Utils_Request::retrieve('crmSID', 'String');

$session = CRM_Core_Session::singleton();

// get previous sort id
$previousSortID = $session->get('previousSortID');

// check for current != previous to ensure cache is not reset if paging is done without changing
// sort criteria
if (!$pageNum || (!empty($currentSortID) && $currentSortID != $previousSortID)) {
Civi::service('prevnext')->deleteItem(NULL, $cacheKey, 'civicrm_contact');
// this means it's fresh search, so set pageNum=1
if (!$pageNum) {
$pageNum = 1;
}
}

// set the current sort as previous sort
if (!empty($currentSortID)) {
$session->set('previousSortID', $currentSortID);
}

$pageSize = CRM_Utils_Request::retrieve('crmRowCount', 'Integer', CRM_Core_DAO::$_nullObject, FALSE, 50);
$firstRecord = ($pageNum - 1) * $pageSize;

//for alphabetic pagination selection save
$sortByCharacter = CRM_Utils_Request::retrieve('sortByCharacter', 'String');

//for text field pagination selection save
$countRow = Civi::service('prevnext')->getCount($cacheKey);
// $sortByCharacter triggers a refresh in the prevNext cache
if ($sortByCharacter && $sortByCharacter !== 'all') {
$this->fillPrevNextCache($sort, $cacheKey, 0, max(self::CACHE_SIZE, $pageSize));
}
elseif (($firstRecord + $pageSize) >= $countRow) {
$this->fillPrevNextCache($sort, $cacheKey, $countRow, max(self::CACHE_SIZE, $pageSize) + $firstRecord - $countRow);
}
return $cacheKey;
}

/**
* @param CRM_Utils_Sort $sort
* @param string $cacheKey
* @param int $start
* @param int $end
*
* @throws \CRM_Core_Exception
*/
public function fillPrevNextCache($sort, $cacheKey, $start = 0, $end = self::CACHE_SIZE): void {
$sql = $this->_search->contactIDs($start, $end, $sort, TRUE);

// CRM-9096
// due to limitations in our search query writer, the above query does not work
// in cases where the query is being sorted on a non-contact table
// this results in a fatal error :(
// see below for the gross hack of trapping the error and not filling
// the prev next cache in this situation
// 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 = CRM_Core_DAO::composeQuery("SELECT DISTINCT %1, contact_a.id, contact_a.sort_name", [1 => [$cacheKey, 'String']]);

$sql = str_ireplace(['SELECT contact_a.id as contact_id', 'SELECT contact_a.id as id'], $selectSQL, $sql);
$sql = str_ireplace('ORDER BY `contact_id`', 'ORDER BY `id`', $sql, $sql);

try {
Civi::service('prevnext')->fillWithSql($cacheKey, $sql);
}
catch (\Exception $e) {
CRM_Core_Error::deprecatedFunctionWarning('Custom searches should return sql capable of filling the prevnext cache.');
// This will always show for CiviRules :-( as a) it orders by 'rule_label'
// which is not available in the query & b) it uses contact not contact_a
// as an alias.
// CRM_Core_Session::setStatus(ts('Query Failed'));
return;
}

if (Civi::service('prevnext') instanceof CRM_Core_PrevNextCache_Sql) {
// SQL-backed prevnext cache uses an extra record for pruning the cache.
// Also ensure that caches stay alive for 2 days as per previous code
Civi::cache('prevNextCache')->set($cacheKey, $cacheKey, 60 * 60 * 24 * CRM_Core_PrevNextCache_Sql::cacheDays);
}
}

/**
* Given the current formValues, gets the query in local language.
*
Expand Down

0 comments on commit 689a576

Please sign in to comment.