Merge pull request #25392 from eileenmcnaughton/prev_next_divide

dev/core#4112 Privatise `prevNextCache` functions
civicrm · Feb 21, 2023 · 20c3cc4 · 20c3cc4
2 parents b07cd26 + f6c304b
commit 20c3cc4
Show file tree

Hide file tree

Showing 3 changed files with 111 additions and 30 deletions.
diff --git a/CRM/Campaign/Selector/Search.php b/CRM/Campaign/Selector/Search.php
@@ -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');
 

diff --git a/CRM/Contact/Selector.php b/CRM/Contact/Selector.php
@@ -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:
@@ -1033,18 +1033,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
@@ -1055,7 +1046,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);
@@ -1064,19 +1055,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) {
@@ -1097,7 +1078,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);

diff --git a/ext/legacycustomsearches/CRM/Contact/Selector/Custom.php b/ext/legacycustomsearches/CRM/Contact/Selector/Custom.php
@@ -13,7 +13,6 @@
  *
  * @package CRM
  * @copyright CiviCRM LLC https://civicrm.org/licensing
- * $Id: Selector.php 11510 2007-09-18 09:21:34Z lobo $
  */
 
 /**
@@ -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
@@ -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.
    *