From e13fa54be8c1c972ee824819a598f5538fb6c3f9 Mon Sep 17 00:00:00 2001 From: eileen Date: Wed, 22 May 2019 18:36:46 +1200 Subject: [PATCH] dev/core#982 add dedupe.getstatistics api Per gitlab the intent is to add a bunch of api so get the logic out of the form into somewhere more accessible. Following agreement on this I would change all calls to CRM_Dedupe_Merger::getMergeStats to call this api and get rid of the call to CRM_Core_BAO_PrevNextCache::retrieve from getMergeStats since that retrieve function really does differrent stuff & is being kinda pointlessly overloaded here. --- CRM/Core/BAO/PrevNextCache.php | 22 +++ CRM/Dedupe/Merger.php | 18 ++- api/v3/Dedupe.php | 150 ++++++++++++++++++ api/v3/utils.php | 5 + tests/phpunit/CRM/Dedupe/MergerTest.php | 12 +- tests/phpunit/api/v3/ContactTest.php | 1 + .../phpunit/api/v3/SyntaxConformanceTest.php | 1 + 7 files changed, 200 insertions(+), 9 deletions(-) create mode 100644 api/v3/Dedupe.php diff --git a/CRM/Core/BAO/PrevNextCache.php b/CRM/Core/BAO/PrevNextCache.php index bdc8e44ba1..7679ff9977 100644 --- a/CRM/Core/BAO/PrevNextCache.php +++ b/CRM/Core/BAO/PrevNextCache.php @@ -496,4 +496,26 @@ AND c.created_date < date_sub( NOW( ), INTERVAL %2 day ) ]; } + /** + * Generate and assign an arbitrary value to a field of a test object. + * + * This specifically supports testing the dedupe use case. + * + * @param string $fieldName + * @param array $fieldDef + * @param int $counter + * The globally-unique ID of the test object. + */ + protected function assignTestValue($fieldName, &$fieldDef, $counter) { + if ($fieldName === 'cacheKey') { + $this->cacheKey = 'merge_' . rand(); + return; + } + if ($fieldName === 'data') { + $this->data = serialize([]); + return; + } + parent::assignTestValue($fieldName, $fieldDef, $counter); + } + } diff --git a/CRM/Dedupe/Merger.php b/CRM/Dedupe/Merger.php index 684393da6c..c7c6d0ce73 100644 --- a/CRM/Dedupe/Merger.php +++ b/CRM/Dedupe/Merger.php @@ -755,13 +755,13 @@ INNER JOIN civicrm_membership membership2 ON membership1.membership_type_id = m } // get previous stats - $previousStats = CRM_Core_BAO_PrevNextCache::retrieve("{$cacheKeyString}_stats"); + $previousStats = CRM_Dedupe_Merger::getMergeStats($cacheKeyString); if (!empty($previousStats)) { - if ($previousStats[0]['merged']) { - $merged = $merged + $previousStats[0]['merged']; + if ($previousStats['merged']) { + $merged = $merged + $previousStats['merged']; } - if ($previousStats[0]['skipped']) { - $skipped = $skipped + $previousStats[0]['skipped']; + if ($previousStats['skipped']) { + $skipped = $skipped + $previousStats['skipped']; } } @@ -796,13 +796,15 @@ INNER JOIN civicrm_membership membership2 ON membership1.membership_type_id = m * * @return array * Array of how many were merged and how many were skipped. + * + * @throws \CiviCRM_API3_Exception */ public static function getMergeStats($cacheKeyString) { - $stats = CRM_Core_BAO_PrevNextCache::retrieve("{$cacheKeyString}_stats"); + $stats = civicrm_api3('Dedupe', 'get', ['cachekey' => "{$cacheKeyString}_stats", 'sequential' => 1])['values']; if (!empty($stats)) { - $stats = $stats[0]; + return $stats[0]['data']; } - return $stats; + return []; } /** diff --git a/api/v3/Dedupe.php b/api/v3/Dedupe.php new file mode 100644 index 0000000000..e98f1d14ff --- /dev/null +++ b/api/v3/Dedupe.php @@ -0,0 +1,150 @@ +where(['merge_data_restriction' => "cachekey LIKE 'merge_%'"]); + + if (isset($params['cachekey'])) { + // This is so bad. We actually have a camel case field name in the DB. Don't do that. + // Intercept the pain here. + $params['cacheKey'] = $params['cachekey']; + unset($params['cachekey']); + } + + $options = _civicrm_api3_get_options_from_params($params, TRUE, 'PrevNextCache', 'get'); + $result = _civicrm_api3_basic_get('CRM_Core_BAO_PrevNextCache', $params, FALSE, 'PrevNextCache', $sql); + + if ($options['is_count']) { + return civicrm_api3_create_success($result, $params, 'PrevNextCache', 'get'); + } + foreach ($result as $index => $values) { + if (isset($values['data']) && !empty($values['data'])) { + $result[$index]['data'] = unserialize($values['data']); + } + if (isset($values['cacheKey'])) { + $result[$index]['cachekey'] = $result[$index]['cacheKey']; + unset($result[$index]['cacheKey']); + } + } + return civicrm_api3_create_success($result, $params, 'PrevNextCache'); +} + +/** + * Get rows for getting dedupe cache records. + * + * @param array $params + */ +function _civicrm_api3_dedupe_get_spec(&$params) { + $params = CRM_Core_DAO_PrevNextCache::fields(); + $params['id']['api.aliases'] = ['dedupe_id']; +} + +/** + * Delete rows for any cached attempted merges on the passed criteria. + * + * @param array $params + * + * @return array + * + * @throws \API_Exception + * @throws \Civi\API\Exception\UnauthorizedException + */ +function civicrm_api3_dedupe_delete($params) { + return _civicrm_api3_basic_delete('CRM_Core_BAO_PrevNextCache', $params); +} + +/** + * Get the statistics for any cached attempted merges on the passed criteria. + * + * @param array $params + * + * @return array + * @throws \API_Exception + * @throws \Civi\API\Exception\UnauthorizedException + */ +function civicrm_api3_dedupe_create($params) { + return _civicrm_api3_basic_create('CRM_Core_BAO_PrevNextCache', $params, 'PrevNextCache'); +} + +/** + * Get the statistics for any cached attempted merges on the passed criteria. + * + * @param array $params + * + * @return array + * @throws \CiviCRM_API3_Exception + */ +function civicrm_api3_dedupe_getstatistics($params) { + $stats = CRM_Dedupe_Merger::getMergeStats(CRM_Dedupe_Merger::getMergeCacheKeyString( + $params['rule_group_id'], + CRM_Utils_Array::value('group_id', $params), + CRM_Utils_Array::value('criteria', $params, []), + CRM_Utils_Array::value('check_permissions', $params, []) + )); + return civicrm_api3_create_success($stats); +} + +/** + * Adjust Metadata for Create action. + * + * The metadata is used for setting defaults, documentation & validation. + * + * @param array $params + * Array of parameters determined by getfields. + */ +function _civicrm_api3_dedupe_getstatistics_spec(&$params) { + $params['rule_group_id'] = [ + 'title' => ts('Rule Group ID'), + 'api.required' => TRUE, + 'type' => CRM_Utils_Type::T_INT, + ]; + $params['group_id'] = [ + 'title' => ts('Group ID'), + 'api.required' => FALSE, + 'type' => CRM_Utils_Type::T_INT, + ]; + $params['criteria'] = [ + 'title' => ts('Criteria'), + 'description' => ts('Dedupe search criteria, as parsable by v3 Contact.get api'), + ]; + +} diff --git a/api/v3/utils.php b/api/v3/utils.php index 1e5d383640..18fcf6c0dd 100644 --- a/api/v3/utils.php +++ b/api/v3/utils.php @@ -381,6 +381,11 @@ function _civicrm_api3_get_BAO($name) { // has enhanced access to other entities. $name = 'Contribution'; } + if ($name === 'Dedupe') { + // Dedupe is a pseudoentity for PrevNextCache - but accessing dedupe related info + // not the other cache info like search results (which could in fact be in Redis or another cache engine) + $name = 'PrevNextCache'; + } $dao = _civicrm_api3_get_DAO($name); if (!$dao) { return NULL; diff --git a/tests/phpunit/CRM/Dedupe/MergerTest.php b/tests/phpunit/CRM/Dedupe/MergerTest.php index 78dffbff29..c55f188324 100644 --- a/tests/phpunit/CRM/Dedupe/MergerTest.php +++ b/tests/phpunit/CRM/Dedupe/MergerTest.php @@ -177,7 +177,6 @@ class CRM_Dedupe_MergerTest extends CiviUnitTestCase { $select = ['pn.is_selected' => 'is_selected']; $cacheKeyString = CRM_Dedupe_Merger::getMergeCacheKeyString($dao->id, $this->_groupId); $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.'); // mark first two pairs as selected @@ -191,6 +190,13 @@ class CRM_Dedupe_MergerTest extends CiviUnitTestCase { $result = CRM_Dedupe_Merger::batchMerge($dao->id, $this->_groupId, 'safe', 5, 1); $this->assertEquals(count($result['merged']), 2, 'Check number of merged pairs.'); + $stats = $this->callAPISuccess('Dedupe', 'getstatistics', [ + 'group_id' => $this->_groupId, + 'rule_group_id' => $dao->id, + 'check_permissions' => TRUE, + ])['values']; + $this->assertEquals(['merged' => 2, 'skipped' => 0], $stats); + // retrieve pairs from prev next cache table $pnDupePairs = CRM_Core_BAO_PrevNextCache::retrieve($cacheKeyString, NULL, NULL, 0, 0, $select); $this->assertEquals(count($pnDupePairs), 1, 'Check number of remaining dupe pairs in prev next cache.'); @@ -248,6 +254,10 @@ class CRM_Dedupe_MergerTest extends CiviUnitTestCase { $result = CRM_Dedupe_Merger::batchMerge($dao->id, $this->_groupId, 'safe', 5, 2); $this->assertEquals(count($result['merged']), 3, 'Check number of merged pairs.'); + $stats = $this->callAPISuccess('Dedupe', 'getstatistics', [ + 'rule_group_id' => $dao->id, + 'group_id' => $this->_groupId, + ]); // retrieve pairs from prev next cache table $pnDupePairs = CRM_Core_BAO_PrevNextCache::retrieve($cacheKeyString, NULL, NULL, 0, 0, $select); $this->assertEquals(count($pnDupePairs), 0, 'Check number of remaining dupe pairs in prev next cache.'); diff --git a/tests/phpunit/api/v3/ContactTest.php b/tests/phpunit/api/v3/ContactTest.php index d0d5ed7466..4e0c6f1f3f 100644 --- a/tests/phpunit/api/v3/ContactTest.php +++ b/tests/phpunit/api/v3/ContactTest.php @@ -87,6 +87,7 @@ class api_v3_ContactTest extends CiviUnitTestCase { 'civicrm_group_contact', 'civicrm_saved_search', 'civicrm_group_contact_cache', + 'civicrm_prevnext_cache', ); $this->quickCleanup($tablesToTruncate, TRUE); diff --git a/tests/phpunit/api/v3/SyntaxConformanceTest.php b/tests/phpunit/api/v3/SyntaxConformanceTest.php index e22e8eea0c..da5ecebd53 100644 --- a/tests/phpunit/api/v3/SyntaxConformanceTest.php +++ b/tests/phpunit/api/v3/SyntaxConformanceTest.php @@ -887,6 +887,7 @@ class api_v3_SyntaxConformanceTest extends CiviUnitTestCase { 'SmsProvider' => 'Provider', 'AclRole' => 'EntityRole', 'MailingEventQueue' => 'Queue', + 'Dedupe' => 'PrevNextCache', ]; $usableName = !empty($entitiesWithNamingIssues[$entityName]) ? $entitiesWithNamingIssues[$entityName] : $entityName; -- 2.25.1