Skip to content

Commit 15df34f

Browse files
committed
(dev/core#217) PrevNext - Use more consistent cache-keys while adjusting filters
Suppose you run a search ("Find Contact", "Advanced Search", "Custom Search", etc). The result screen includes several elements (which we'll reference below): 1. Standard pagination (Previous/Next; First/Last; Jump-To) 2. Numerical option for page-size 3. Sortable columns 4. An alphabetical filter 5. Checkboxes As you work with these options, the content of the `civicrm_prevnext_cache` table may change. This patch does not substantively change what's in that cache, but makes the column `cacheKey` simpler and more consistent. Both Before and After (Unchanged) --------------------------------- * The form's qfKey identifies the current screen/filters/cache. * If you navigate to the next/previous page (`#1`) or adjust the page-size (`#2`), the content in `civicrm_prevnext_cache` remains the same (for the given qfKey). * If you change the sort column (`#3`) or alphabetic filter (`#4`), the content in `civicrm_prevnext_cache` is deleted and repopulated (for the given qfKey). * If you toggle a checkbox, the `civicrm_prevnext_cache.is_selected` property updates accordingly. These selections are retained when changing pages (`#1`/`#2`), but they're reset if you use sort or alphabet options (`#3`/`#4`). Before ------ * The content of `civicrm_prevnext_cache.cacheKey` takes one of two forms, depending on whether you're using an alphabetic filter (`#4`). * `civicrm search {qfKey}` (typical, without any alphabetic filter) * `civicrm search {qfKey}_alphabet` (less common, with an alphabetic filter) * The queries which read or delete the query-cache use a prefix+wildcard, i.e. `WHERE cacheKey LIKE 'civicrm search {qfKey}%'`. After ----- * The content of `civicrm_prevnext_cache.cacheKey` takes only one form * `civicrm search {qfKey}` * The queries which read or delete the query-cache use an exact match, i.e. `WHERE cacheKey = 'civicrm search {qfKey}'`.` * The text `_alphabet` does not appear in the PHP source folders (CRM, Civi, bin, api, extern, tests). Comments -------- In theory, one can imagine that it's desireable to keep the cached results for each of the sorted/filtered variants of the query. That might allow the user to quickly switch among different sortings and different alphabetic-filters, or it might allow some kind of clever management of the selections. But this is not so. As we see (both before and after), the substance of the cache is deleted whenever the user changes `#3`/`#4`. In reality, one user browsing a search screen corresponds to exactly one query-cache. As near as I can tell, the old code made the names change for no real reason at all. To observe the behavior empirically, I would twiddle the UI widgets and concurrently inspect the content of the cache tables. For example: ``` mysql> select group_name, path, FROM_BASE64(data), expired_date from civicrm_cache where path like 'civicrm search%'; select 'Total records in' as label, cacheKey, count(*), min(id), max(id) from civicrm_prevnext_cache group by cacheKey union select 'Selected records in ', cacheKey, count(*), min(id), max(id) from civicrm_prevnext_cache where is_selected=1 group by cacheKey; +------------------------------+------------------------------------------------------+--------------------------------------------------------------+--------------+ | group_name | path | FROM_BASE64(data) | expired_date | +------------------------------+------------------------------------------------------+--------------------------------------------------------------+--------------+ | CiviCRM Search PrevNextCache | civicrm search a8ed1e2039241c41457a88f65aa8a8ee_7845 | s:52:"civicrm search a8ed1e2039241c41457a88f65aa8a8ee_7845"; | NULL | +------------------------------+------------------------------------------------------+--------------------------------------------------------------+--------------+ 1 row in set (0.00 sec) +----------------------+------------------------------------------------------+----------+---------+---------+ | label | cacheKey | count(*) | min(id) | max(id) | +----------------------+------------------------------------------------------+----------+---------+---------+ | Total records in | civicrm search a8ed1e2039241c41457a88f65aa8a8ee_7845 | 6 | 787 | 792 | | Selected records in | civicrm search a8ed1e2039241c41457a88f65aa8a8ee_7845 | 1 | 789 | 789 | +----------------------+------------------------------------------------------+----------+---------+---------+ 2 rows in set (0.01 sec) ```
1 parent 3422e9b commit 15df34f

File tree

3 files changed

+21
-13
lines changed

3 files changed

+21
-13
lines changed

CRM/Contact/Form/Task.php

-3
Original file line numberDiff line numberDiff line change
@@ -165,9 +165,6 @@ public static function preProcessCommon(&$form) {
165165
if ((CRM_Utils_Array::value('radio_ts', self::$_searchFormValues) == 'ts_all') ||
166166
($form->_task == CRM_Contact_Task::SAVE_SEARCH)
167167
) {
168-
$sortByCharacter = $form->get('sortByCharacter');
169-
$cacheKey = ($sortByCharacter && $sortByCharacter != 'all') ? "{$cacheKey}_alphabet" : $cacheKey;
170-
171168
// since we don't store all contacts in prevnextcache, when user selects "all" use query to retrieve contacts
172169
// rather than prevnext cache table for most of the task actions except export where we rebuild query to fetch
173170
// final result set

CRM/Contact/Selector.php

-1
Original file line numberDiff line numberDiff line change
@@ -903,7 +903,6 @@ public function buildPrevNextCache($sort) {
903903
$countRow = Civi::service('prevnext')->getCount($cacheKey);
904904
// $sortByCharacter triggers a refresh in the prevNext cache
905905
if ($sortByCharacter && $sortByCharacter != 'all') {
906-
$cacheKey .= "_alphabet";
907906
$this->fillupPrevNextCache($sort, $cacheKey, 0, max(self::CACHE_SIZE, $pageSize));
908907
}
909908
elseif (($firstRecord + $pageSize) >= $countRow) {

CRM/Core/PrevNextCache/Sql.php

+21-9
Original file line numberDiff line numberDiff line change
@@ -97,37 +97,37 @@ public function markSelection($cacheKey, $action, $cIds = NULL) {
9797
if (is_array($cIds)) {
9898
$cIdFilter = "(" . implode(',', $cIds) . ")";
9999
$whereClause = "
100-
WHERE cacheKey LIKE %1
100+
WHERE cacheKey = %1
101101
AND (entity_id1 IN {$cIdFilter} OR entity_id2 IN {$cIdFilter})
102102
";
103103
}
104104
else {
105105
$whereClause = "
106-
WHERE cacheKey LIKE %1
106+
WHERE cacheKey = %1
107107
AND (entity_id1 = %2 OR entity_id2 = %2)
108108
";
109109
$params[2] = array("{$cIds}", 'Integer');
110110
}
111111
if ($action == 'select') {
112112
$whereClause .= "AND is_selected = 0";
113113
$sql = "UPDATE civicrm_prevnext_cache SET is_selected = 1 {$whereClause} {$entity_whereClause}";
114-
$params[1] = array("{$cacheKey}%", 'String');
114+
$params[1] = array($cacheKey, 'String');
115115
}
116116
elseif ($action == 'unselect') {
117117
$whereClause .= "AND is_selected = 1";
118118
$sql = "UPDATE civicrm_prevnext_cache SET is_selected = 0 {$whereClause} {$entity_whereClause}";
119-
$params[1] = array("%{$cacheKey}%", 'String');
119+
$params[1] = array($cacheKey, 'String');
120120
}
121121
// default action is reseting
122122
}
123123
elseif (!$cIds && $cacheKey && $action == 'unselect') {
124124
$sql = "
125125
UPDATE civicrm_prevnext_cache
126126
SET is_selected = 0
127-
WHERE cacheKey LIKE %1 AND is_selected = 1
127+
WHERE cacheKey = %1 AND is_selected = 1
128128
{$entity_whereClause}
129129
";
130-
$params[1] = array("{$cacheKey}%", 'String');
130+
$params[1] = array($cacheKey, 'String');
131131
}
132132
CRM_Core_DAO::executeQuery($sql, $params);
133133
}
@@ -157,12 +157,12 @@ public function getSelection($cacheKey, $action = 'get') {
157157
$actionGet = ($action == "get") ? " AND is_selected = 1 " : "";
158158
$sql = "
159159
SELECT entity_id1, entity_id2 FROM civicrm_prevnext_cache
160-
WHERE cacheKey LIKE %1
160+
WHERE cacheKey = %1
161161
$actionGet
162162
$entity_whereClause
163163
ORDER BY id
164164
";
165-
$params[1] = array("{$cacheKey}%", 'String');
165+
$params[1] = array($cacheKey, 'String');
166166

167167
$contactIds = array($cacheKey => array());
168168
$cIdDao = CRM_Core_DAO::executeQuery($sql, $params);
@@ -199,7 +199,19 @@ public function getPositions($cacheKey, $id1, $id2) {
199199
* @param string $entityTable
200200
*/
201201
public function deleteItem($id = NULL, $cacheKey = NULL, $entityTable = 'civicrm_contact') {
202-
CRM_Core_BAO_PrevNextCache::deleteItem($id, $cacheKey, $entityTable);
202+
$sql = "DELETE FROM civicrm_prevnext_cache WHERE entity_table = %1";
203+
$params = array(1 => array($entityTable, 'String'));
204+
205+
if (is_numeric($id)) {
206+
$sql .= " AND ( entity_id1 = %2 OR entity_id2 = %2 )";
207+
$params[2] = array($id, 'Integer');
208+
}
209+
210+
if (isset($cacheKey)) {
211+
$sql .= " AND cacheKey = %3";
212+
$params[3] = array($cacheKey, 'String');
213+
}
214+
CRM_Core_DAO::executeQuery($sql, $params);
203215
}
204216

205217
/**

0 commit comments

Comments
 (0)