Allow replacement of PrevNextCache implementation (for search screens)
This is a major sub-task of #174 (closed) (Consistently use swappable cache interfaces).
Background
The PrevNextCache is used to temporarily save the results of a query. It is specifically used in the following scenarios:
- Contact searches (Related -- other derived searches and the "View Contact" result screen)
- Dedupe/merge
For this issue, our aim is specifically to allow the contact-searches to retain their results via Redis/Memcache (instead of MySQL).
Touch Points
We want to make it possible to swap the prev-next implementation, but the functionality of CRM_Core_BAO_PrevNextCache
is a bit more sophisticated than CRM_Utils_Cache_Interface
. So, first, we need to understand the places which touch the prev-next cache (before patching).
Q1: Where does it INSERT
into civicrm_prevnext_cache
?
CRM_Contact_Selector::fillupPrevNextCache
CRM_Campaign_Selector_Search::buildPrevNextCache
Q2: Where does it SELECT
from civicrm_prevnext_cache
?
CRM_Contact_BAO_Query::getCachedContacts
Q3: What are all the functions supported by the BAO? When are they relevant
- (Group A) Relevant to contact searches (and maybe also dedupe/merge)
public static function markSelection($cacheKey, $action = 'unselect', $cIds = NULL, $entity_table = 'civicrm_contact') {
public static function getSelection($cacheKey, $action = 'get', $entity_table = 'civicrm_contact') {
public static function getPositions($cacheKey, $id1, $id2, &$mergeId = NULL, $join = NULL, $where = NULL, $flip = FALSE) {
public static function deleteItem($id = NULL, $cacheKey = NULL, $entityTable = 'civicrm_contact') {
public static function getSelectedContacts() {
-
public static function getCount($cacheKey, $join = NULL, $where = NULL, $op = "=",
$params = array()) {
- (Group B) Only relevant to dedupe/merge
public static function flipPair(array $prevNextId, $onlySelected) {
public static function deletePair($id1, $id2, $cacheKey = NULL, $isViceVersa = FALSE, $entityTable = 'civicrm_contact') {
public static function markConflict($id1, $id2, $cacheKey, $conflicts) {
public static function setItem($values) {
public static function refillCache($rgid, $gid, $cacheKeyString, $criteria, $checkPermissions, $searchLimit = 0) {
public static function retrieve($cacheKey, $join = NULL, $whereClause = NULL, $offset = 0, $rowCount = 0, $select = array(),...)
- (Group C) Special, internal, or unused
public static function cleanupCache() {
public static function is_serialized($string) {
public static function buildSelectedContactPager(&$form, &$params) {
General Approach
- Define a service
Civi::service('prevnext')
which satisfies an interfaceCRM_Core_PrevNextCache_Interface
. - Create two implementations of the service:
-
CRM_Core_PrevNextCache_Sql
(which is pretty thin -- it generally delegates to the existing BAO code) CRM_Core_PrevNextCache_Memory
(which uses a memory-backed cache)-
CRM_Core_PrevNextCache_Redis
(which uses Redis for a memory-backed cache)
-
- The scope of the interface would include:
- Functionality needed by the inserters (
CRM_Contact_Selector::fillupPrevNextCache
andCRM_Campaign_Selector_Search::buildPrevNextCache
) - Functionality needed by the reader (
CRM_Contact_BAO_Query::getCachedContacts
) - Functionality from the BAO that's relevant to contact-searches (eg
markSelection
but notdeletePair
)
- Functionality needed by the inserters (
- In the contact-search use-cases, change to use the service instead of the BAO.
- Ex:
CRM_Core_BAO_PrevNextCache::markSelection(...)
=>Civi::service('prevnext')->markSelection(...)
- Ex:
Note: I initially considered a different relationship (where CRM_Core_BAO_PrevNextCache
remains the canonical/front-facing interface and CRM_Core_PrevNextCache_Interface
remains hidden behind the BAO). However, given that dedupe/merge needs to continue using the existing SQL implementation, it felt like that would get messier; it seemed cleaner to split them apart more clearly ("use-case (1) always invokes code (A), and use-case (2) always invokes code (B)" -- i.e. "all contact-search code invokes Civi::service('prevnext')
, and all dedupe code invokes PrevNext BAO").
PR List
-
12377 - General/overall patchset
- 12438 - CRM_Contact_Selector::getRows() - Use generator instead of DAO loop
- 12528 - Add skeletal PrevNextCache service
- 12543 - PrevNext - Probe for best available implementation (memory-backed or SQL-backed)
- 12544 - CRM_Utils_Cache_Redis::connect() - Allow pooling connections
- 12545 - PrevNext - Define and use fillWithSql()/fillWithArray()
- 12556 - Migrate selection methods
- 12558 - Allow swapping getPositions (etal) for contact-search
- 12663 - Use more consistent cache-keys while adjusting filters
- 12664 - Remove references to entity_table and entity_id2 from service. Add test.
- 12665 - Implement Redis. Decouple Query::getCachedContacts()
Test Scenarios
The prev-next cache has evolved a number of conditionals to handle edge-cases in which it's used. We should be testing these in a targeted/incremental way during development. Once we have a fairly complete PR, we should do a final/comprehensive pass through these use-cases.
-
Quick Search Box / Advanced Search / Search Builder: This is the main section of the code that we're targetting for revision.
- Execute a search with multiple pages of results
- Execute a search with one page of results
- Drill-down and view a contact in the search results. Navigate using "Previous" button, "Next" button, and the breadcrumb "Search Results"
- CRM-9096 Create a profile. Mark some fields as visible/enabled for listings. Run an "Advanced Search" and use this profile. Re-sort results.
- Select a handful of contacts on different pages. Execute a task (e.g. "Export") and ensure that it has the appropriate contacts.
- Select all contacts. Execute a task (e.g. "Export") and ensure that it has the appropriate contacts.
-
Custom Search: Most of the code is shared with above, but a few lines are different.
- Run a few custom searches. Use various sort/filter options.
-
CiviCampaign: This component has logic to fill the cache directly -- which doesn't use the same code-paths.
- Create a survey. Reserve respondents. Interview. Release. (When browsing respondents, try to sort.)
-
CiviRules: This component uses some of the code from contact-search, and there's a comment about special case.
- Browse the list of available rules.
- Dedupe/Merge: In principle, this code isn't supposed to be touched/changed by our PR, and there is test coverage over this functionality. OTOH, if one made a mistake (inappropriately changing shared code), there could be a regression. So do a couple tests with this anyway.