Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorporate searchLimit in dedupe cacheKey #15185

Merged
merged 1 commit into from
Sep 2, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions CRM/Contact/Form/Merge.php
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public function preProcess() {
CRM_Core_Session::singleton()->pushUserContext($browseUrl);
}

$cacheKey = CRM_Dedupe_Merger::getMergeCacheKeyString($this->_rgid, $gid, json_decode($this->criteria, TRUE));
$cacheKey = CRM_Dedupe_Merger::getMergeCacheKeyString($this->_rgid, $gid, json_decode($this->criteria, TRUE), TRUE, $this->limit);

$join = CRM_Dedupe_Merger::getJoinOnDedupeTable();
$where = "de.id IS NULL";
Expand Down Expand Up @@ -331,7 +331,7 @@ public function postProcess() {
}

if ($this->next && $this->_mergeId) {
$cacheKey = CRM_Dedupe_Merger::getMergeCacheKeyString($this->_rgid, $this->_gid, json_decode($this->criteria, TRUE));
$cacheKey = CRM_Dedupe_Merger::getMergeCacheKeyString($this->_rgid, $this->_gid, json_decode($this->criteria, TRUE), TRUE, $this->limit);

$join = CRM_Dedupe_Merger::getJoinOnDedupeTable();
$where = "de.id IS NULL";
Expand Down
3 changes: 2 additions & 1 deletion CRM/Contact/Page/AJAX.php
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,7 @@ public static function getDedupes() {

$gid = CRM_Utils_Request::retrieve('gid', 'Positive');
$rgid = CRM_Utils_Request::retrieve('rgid', 'Positive');
$limit = CRM_Utils_Request::retrieveValue('limit', 'Positive', 0);
$null = NULL;
$criteria = CRM_Utils_Request::retrieve('criteria', 'Json', $null, FALSE, '{}');
$selected = CRM_Utils_Request::retrieveValue('selected', 'Boolean');
Expand All @@ -646,7 +647,7 @@ public static function getDedupes() {
}

$whereClause = $orderByClause = '';
$cacheKeyString = CRM_Dedupe_Merger::getMergeCacheKeyString($rgid, $gid, json_decode($criteria, TRUE));
$cacheKeyString = CRM_Dedupe_Merger::getMergeCacheKeyString($rgid, $gid, json_decode($criteria, TRUE), TRUE, $limit);

$searchRows = [];

Expand Down
6 changes: 3 additions & 3 deletions CRM/Contact/Page/DedupeFind.php
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,13 @@ public function run() {
'reset' => 1,
'rgid' => $rgid,
'gid' => $gid,
'limit' => $limit,
'limit' => (int) $limit,
'criteria' => $criteria,
];
$this->assign('urlQuery', CRM_Utils_System::makeQueryString($urlQry));
$this->assign('isSelected', $this->isSelected());
$criteria = json_decode($criteria, TRUE);
$cacheKeyString = CRM_Dedupe_Merger::getMergeCacheKeyString($rgid, $gid, $criteria);
$cacheKeyString = CRM_Dedupe_Merger::getMergeCacheKeyString($rgid, $gid, $criteria, TRUE, $limit);
$this->assign('cacheKey', $cacheKeyString);

if ($context == 'search') {
Expand All @@ -129,7 +129,7 @@ public function run() {
}
elseif ($action & CRM_Core_Action::MAP) {
// do a batch merge if requested
$result = CRM_Dedupe_Merger::batchMerge($rgid, $gid, 'safe', 75, 2, $criteria);
$result = CRM_Dedupe_Merger::batchMerge($rgid, $gid, 'safe', 75, 2, $criteria, TRUE, NULL, $limit);

$skippedCount = CRM_Utils_Request::retrieve('skipped', 'Positive', $this, FALSE, 0);
$skippedCount = $skippedCount + count($result['skipped']);
Expand Down
12 changes: 8 additions & 4 deletions CRM/Contact/Page/DedupeMerge.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public static function getRunner() {
];

$criteria = json_decode($criteria, TRUE);
$cacheKeyString = CRM_Dedupe_Merger::getMergeCacheKeyString($rgid, $gid, $criteria);
$cacheKeyString = CRM_Dedupe_Merger::getMergeCacheKeyString($rgid, $gid, $criteria, TRUE, $limit);

if ($mode == 'aggressive' && !CRM_Core_Permission::check('force merge duplicate contacts')) {
CRM_Core_Session::setStatus(ts('You do not have permission to force merge duplicate contact records'), ts('Permission Denied'), 'error');
Expand Down Expand Up @@ -98,7 +98,7 @@ public static function getRunner() {
for ($i = 1; $i <= ceil($total / self::BATCHLIMIT); $i++) {
$task = new CRM_Queue_Task(
['CRM_Contact_Page_DedupeMerge', 'callBatchMerge'],
[$rgid, $gid, $mode, self::BATCHLIMIT, $onlyProcessSelected, $criteria],
[$rgid, $gid, $mode, self::BATCHLIMIT, $onlyProcessSelected, $criteria, $limit],
"Processed " . $i * self::BATCHLIMIT . " pair of duplicates out of " . $total
);

Expand Down Expand Up @@ -132,11 +132,15 @@ public static function getRunner() {
* @param int $batchLimit
* @param int $isSelected
* @param array $criteria
* @param int $searchLimit
*
* @return int
*
* @throws \CRM_Core_Exception
* @throws \CiviCRM_API3_Exception
*/
public static function callBatchMerge(CRM_Queue_TaskContext $ctx, $rgid, $gid, $mode = 'safe', $batchLimit, $isSelected, $criteria) {
CRM_Dedupe_Merger::batchMerge($rgid, $gid, $mode, $batchLimit, $isSelected, $criteria, TRUE, FALSE);
public static function callBatchMerge(CRM_Queue_TaskContext $ctx, $rgid, $gid, $mode = 'safe', $batchLimit, $isSelected, $criteria, $searchLimit) {
CRM_Dedupe_Merger::batchMerge($rgid, $gid, $mode, $batchLimit, $isSelected, $criteria, TRUE, FALSE, $searchLimit);
return CRM_Queue_Task::TASK_SUCCESS;
}

Expand Down
2 changes: 1 addition & 1 deletion CRM/Core/BAO/PrevNextCache.php
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ public static function getCount($cacheKey, $join = NULL, $where = NULL, $op = "=
* @throws \CiviCRM_API3_Exception
*/
public static function refillCache($rgid, $gid, $criteria, $checkPermissions, $searchLimit = 0) {
$cacheKeyString = CRM_Dedupe_Merger::getMergeCacheKeyString($rgid, $gid, $criteria, $checkPermissions);
$cacheKeyString = CRM_Dedupe_Merger::getMergeCacheKeyString($rgid, $gid, $criteria, $checkPermissions, $searchLimit);

// 1. Clear cache if any
$sql = "DELETE FROM civicrm_prevnext_cache WHERE cachekey LIKE %1";
Expand Down
29 changes: 21 additions & 8 deletions CRM/Dedupe/Merger.php
Original file line number Diff line number Diff line change
Expand Up @@ -691,9 +691,17 @@ public static function retrieveFields($main, $other) {
* If not set explicitly this is calculated but it is preferred that it be set
* per comments on isSelected above.
*
* @param int $searchLimit
* Limit on number of contacts to search for duplicates for.
* This means that if the limit is 1000 then only duplicates for the first 1000 contacts
* matching criteria will be found and batchMerged (the number of merges could be less than or greater than 100)
*
* @return array|bool
*
* @throws \CRM_Core_Exception
* @throws \CiviCRM_API3_Exception
*/
public static function batchMerge($rgid, $gid = NULL, $mode = 'safe', $batchLimit = 1, $isSelected = 2, $criteria = [], $checkPermissions = TRUE, $reloadCacheIfEmpty = NULL) {
public static function batchMerge($rgid, $gid = NULL, $mode = 'safe', $batchLimit = 1, $isSelected = 2, $criteria = [], $checkPermissions = TRUE, $reloadCacheIfEmpty = NULL, $searchLimit = 0) {
$redirectForPerformance = ($batchLimit > 1) ? TRUE : FALSE;

if (!isset($reloadCacheIfEmpty)) {
Expand All @@ -706,7 +714,7 @@ public static function batchMerge($rgid, $gid = NULL, $mode = 'safe', $batchLimi
$dupePairs = self::getDuplicatePairs($rgid, $gid, $reloadCacheIfEmpty, $batchLimit, $isSelected, ($mode == 'aggressive'), $criteria, $checkPermissions);

$cacheParams = [
'cache_key_string' => self::getMergeCacheKeyString($rgid, $gid, $criteria, $checkPermissions),
'cache_key_string' => self::getMergeCacheKeyString($rgid, $gid, $criteria, $checkPermissions, $searchLimit),
// @todo stop passing these parameters in & instead calculate them in the merge function based
// on the 'real' params like $isRespectExclusions $batchLimit and $isSelected.
'join' => self::getJoinOnDedupeTable(),
Expand Down Expand Up @@ -1840,13 +1848,13 @@ public static function createMergeActivities($mainId, $otherId) {
* @throws \CiviCRM_API3_Exception
*/
public static function getDuplicatePairs($rule_group_id, $group_id, $reloadCacheIfEmpty, $batchLimit, $isSelected, $includeConflicts = TRUE, $criteria = [], $checkPermissions = TRUE, $searchLimit = 0) {
$dupePairs = self::getCachedDuplicateMatches($rule_group_id, $group_id, $batchLimit, $isSelected, $includeConflicts, $criteria, $checkPermissions);
$dupePairs = self::getCachedDuplicateMatches($rule_group_id, $group_id, $batchLimit, $isSelected, $includeConflicts, $criteria, $checkPermissions, $searchLimit);
if (empty($dupePairs) && $reloadCacheIfEmpty) {
// If we haven't found any dupes, probably cache is empty.
// Try filling cache and give another try. We don't need to specify include conflicts here are there will not be any
// until we have done some processing.
CRM_Core_BAO_PrevNextCache::refillCache($rule_group_id, $group_id, $criteria, $checkPermissions, $searchLimit);
return self::getCachedDuplicateMatches($rule_group_id, $group_id, $batchLimit, $isSelected, FALSE, $criteria, $checkPermissions);
return self::getCachedDuplicateMatches($rule_group_id, $group_id, $batchLimit, $isSelected, FALSE, $criteria, $checkPermissions, $searchLimit);
}
return $dupePairs;
}
Expand All @@ -1859,17 +1867,21 @@ public static function getDuplicatePairs($rule_group_id, $group_id, $reloadCache
* @param array $criteria
* Additional criteria to narrow down the merge group.
* Currently we are only supporting the key 'contact' within it.
*
* @param bool $checkPermissions
* Respect the users permissions.
* @param int $searchLimit
* Number of contacts to seek dupes for (we need this because if
* we change it the results won't be refreshed otherwise. Changing the limit
* from 100 to 1000 SHOULD result in a new dedupe search).
*
* @return string
*/
public static function getMergeCacheKeyString($rule_group_id, $group_id, $criteria = [], $checkPermissions = TRUE) {
public static function getMergeCacheKeyString($rule_group_id, $group_id, $criteria, $checkPermissions, $searchLimit) {
$contactType = CRM_Dedupe_BAO_RuleGroup::getContactTypeForRuleGroup($rule_group_id);
$cacheKeyString = "merge_{$contactType}";
$cacheKeyString .= $rule_group_id ? "_{$rule_group_id}" : '_0';
$cacheKeyString .= $group_id ? "_{$group_id}" : '_0';
$cacheKeyString .= '_' . (int) $searchLimit;
$cacheKeyString .= !empty($criteria) ? md5(serialize($criteria)) : '_0';
if ($checkPermissions) {
$contactID = CRM_Core_Session::getLoggedInContactID();
Expand Down Expand Up @@ -2487,12 +2499,13 @@ protected static function formatConflictArray($conflicts, $migrationInfo, $toKee
* @param bool $includeConflicts
* @param array $criteria
* @param int $checkPermissions
* @param int $searchLimit
*
* @return array
*/
protected static function getCachedDuplicateMatches($rule_group_id, $group_id, $batchLimit, $isSelected, $includeConflicts, $criteria, $checkPermissions) {
protected static function getCachedDuplicateMatches($rule_group_id, $group_id, $batchLimit, $isSelected, $includeConflicts, $criteria, $checkPermissions, $searchLimit = 0) {
return CRM_Core_BAO_PrevNextCache::retrieve(
self::getMergeCacheKeyString($rule_group_id, $group_id, $criteria, $checkPermissions),
self::getMergeCacheKeyString($rule_group_id, $group_id, $criteria, $checkPermissions, $searchLimit),
self::getJoinOnDedupeTable(),
self::getWhereString($isSelected),
0, $batchLimit,
Expand Down
8 changes: 7 additions & 1 deletion api/v3/Dedupe.php
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ function civicrm_api3_dedupe_getstatistics($params) {
$params['rule_group_id'],
CRM_Utils_Array::value('group_id', $params),
CRM_Utils_Array::value('criteria', $params, []),
CRM_Utils_Array::value('check_permissions', $params, [])
!empty($params['check_permissions']),
CRM_Utils_Array::value('search_limit', $params, 0)
));
return civicrm_api3_create_success($stats);
}
Expand Down Expand Up @@ -135,6 +136,11 @@ function _civicrm_api3_dedupe_getstatistics_spec(&$params) {
'title' => ts('Criteria'),
'description' => ts('Dedupe search criteria, as parsable by v3 Contact.get api'),
];
$spec['search_limit'] = [
'title' => ts('Number of contacts to look for matches for.'),
'type' => CRM_Utils_Type::T_INT,
'api.default' => (int) Civi::settings()->get('dedupe_default_limit'),
];

}

Expand Down
8 changes: 7 additions & 1 deletion api/v3/Job.php
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ function civicrm_api3_job_process_batch_merge($params) {
$gid = CRM_Utils_Array::value('gid', $params);
$mode = CRM_Utils_Array::value('mode', $params, 'safe');

$result = CRM_Dedupe_Merger::batchMerge($rule_group_id, $gid, $mode, 1, 2, CRM_Utils_Array::value('criteria', $params, []), CRM_Utils_Array::value('check_permissions', $params));
$result = CRM_Dedupe_Merger::batchMerge($rule_group_id, $gid, $mode, 1, 2, CRM_Utils_Array::value('criteria', $params, []), CRM_Utils_Array::value('check_permissions', $params), NULL, $params['search_limit']);

return civicrm_api3_create_success($result, $params);
}
Expand Down Expand Up @@ -571,6 +571,12 @@ function _civicrm_api3_job_process_batch_merge_spec(&$params) {
'description' => 'let the api decide which contact to retain and which to delete?',
'type' => CRM_Utils_Type::T_BOOLEAN,
];
$params['search_limit'] = [
'title' => ts('Number of contacts to look for matches for.'),
'type' => CRM_Utils_Type::T_INT,
'api.default' => (int) Civi::settings()->get('dedupe_default_limit'),
];

}

/**
Expand Down
4 changes: 2 additions & 2 deletions tests/phpunit/CRM/Dedupe/MergerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ public function testBatchMergeSelectedDuplicates() {

// Retrieve pairs from prev next cache table
$select = ['pn.is_selected' => 'is_selected'];
$cacheKeyString = CRM_Dedupe_Merger::getMergeCacheKeyString($dao->id, $this->_groupId);
$cacheKeyString = CRM_Dedupe_Merger::getMergeCacheKeyString($dao->id, $this->_groupId, [], TRUE, 0);
$pnDupePairs = CRM_Core_BAO_PrevNextCache::retrieve($cacheKeyString, NULL, NULL, 0, 0, $select);
$this->assertEquals(count($foundDupes), count($pnDupePairs), 'Check number of dupe pairs in prev next cache.');

Expand Down Expand Up @@ -245,7 +245,7 @@ public function testBatchMergeAllDuplicates() {

// Retrieve pairs from prev next cache table
$select = ['pn.is_selected' => 'is_selected'];
$cacheKeyString = CRM_Dedupe_Merger::getMergeCacheKeyString($dao->id, $this->_groupId);
$cacheKeyString = CRM_Dedupe_Merger::getMergeCacheKeyString($dao->id, $this->_groupId, [], TRUE, 0);
$pnDupePairs = CRM_Core_BAO_PrevNextCache::retrieve($cacheKeyString, NULL, NULL, 0, 0, $select);

$this->assertEquals(count($foundDupes), count($pnDupePairs), 'Check number of dupe pairs in prev next cache.');
Expand Down