From 3bcde7f1f972a436d68fd2c995bbd84975291696 Mon Sep 17 00:00:00 2001 From: eileen Date: Fri, 6 Mar 2020 13:36:23 +1300 Subject: [PATCH] Remove calls & deprecate CRM_Core_BAO_PrevNextCache::setItem We have this function which 1) is mildly misleading - it appears to be generic to the prevnext table but is actually only relevant to deduping as searches no longer use it 2) is about 50% deprecated and 3) the remainder is a single insert 4) is called from 2 places which use it a little differently I think it's not really adding much value - I was going to remove the deprecated code but I think in fact the goal should be to remove the whole function. In general I think code on CRM_Core_BAO_PrevNextCache that is really dedupe-only code should be on the dedupe classes. The history is that prevnext was created for searches and kindof twisted to support dedupe as well but now search doesn't use much of what is in the BAO class (if any) --- CRM/Core/BAO/PrevNextCache.php | 3 +++ CRM/Dedupe/Finder.php | 12 +++++++++--- CRM/Dedupe/Merger.php | 9 +++++---- .../phpunit/CRM/Core/BAO/PrevNextCacheTest.php | 18 ------------------ 4 files changed, 17 insertions(+), 25 deletions(-) diff --git a/CRM/Core/BAO/PrevNextCache.php b/CRM/Core/BAO/PrevNextCache.php index e7aa7b4382..ca53363abf 100644 --- a/CRM/Core/BAO/PrevNextCache.php +++ b/CRM/Core/BAO/PrevNextCache.php @@ -338,6 +338,8 @@ FROM civicrm_prevnext_cache pn } /** + * @deprecated + * * @param array|string $entity_table * @param int $entity_id1 * @param int $entity_id2 @@ -345,6 +347,7 @@ FROM civicrm_prevnext_cache pn * @param string $data */ public static function setItem($entity_table = NULL, $entity_id1 = NULL, $entity_id2 = NULL, $cacheKey = NULL, $data = NULL) { + CRM_Core_Error::deprecatedFunctionWarning('Deprecated function'); // If entity table is an array we are passing in an older format where this function only had 1 param $values. We put a deprecation warning. if (!empty($entity_table) && is_array($entity_table)) { Civi::log()->warning('Deprecated code path. Values should not be set this is going away in the future in favour of specific function params for each column.', array('civi.tag' => 'deprecated')); diff --git a/CRM/Dedupe/Finder.php b/CRM/Dedupe/Finder.php index f2626bcb32..dac4bf1359 100644 --- a/CRM/Dedupe/Finder.php +++ b/CRM/Dedupe/Finder.php @@ -340,16 +340,22 @@ class CRM_Dedupe_Finder { } $mainContacts[] = $row = [ - 'dstID' => $dstID, + 'dstID' => (int) $dstID, 'dstName' => $displayNames[$dstID], - 'srcID' => $srcID, + 'srcID' => (int) $srcID, 'srcName' => $displayNames[$srcID], 'weight' => $dupes[2], 'canMerge' => TRUE, ]; $data = CRM_Core_DAO::escapeString(serialize($row)); - CRM_Core_BAO_PrevNextCache::setItem('civicrm_contact', $dstID, $srcID, $cacheKeyString, $data); + CRM_Core_DAO::executeQuery("INSERT INTO civicrm_prevnext_cache (entity_table, entity_id1, entity_id2, cacheKey, data) VALUES + ('civicrm_contact', %1, %2, %3, '{$data}')", [ + 1 => [$dstID, 'Integer'], + 2 => [$srcID, 'Integer'], + 3 => [$cacheKeyString, 'String'], + ] + ); } return $mainContacts; } diff --git a/CRM/Dedupe/Merger.php b/CRM/Dedupe/Merger.php index c5f063d1df..00f2901fa5 100644 --- a/CRM/Dedupe/Merger.php +++ b/CRM/Dedupe/Merger.php @@ -772,12 +772,13 @@ INNER JOIN civicrm_membership membership2 ON membership1.membership_type_id = m // store the updated stats $data = [ - 'merged' => $merged, - 'skipped' => $skipped, + 'merged' => (int) $merged, + 'skipped' => (int) $skipped, ]; - $data = CRM_Core_DAO::escapeString(serialize($data)); + $data = serialize($data); - CRM_Core_BAO_PrevNextCache::setItem('civicrm_contact', 0, 0, $cacheKeyString . '_stats', $data); + CRM_Core_DAO::executeQuery("INSERT INTO civicrm_prevnext_cache (entity_table, entity_id1, entity_id2, cacheKey, data) VALUES + ('civicrm_contact', 0, 0, %1, '{$data}')", [1 => [$cacheKeyString . '_stats', 'String']]); } /** diff --git a/tests/phpunit/CRM/Core/BAO/PrevNextCacheTest.php b/tests/phpunit/CRM/Core/BAO/PrevNextCacheTest.php index 2c3a6175dc..3c1e92fc8b 100644 --- a/tests/phpunit/CRM/Core/BAO/PrevNextCacheTest.php +++ b/tests/phpunit/CRM/Core/BAO/PrevNextCacheTest.php @@ -46,22 +46,4 @@ class CRM_Core_BAO_PrevNextCacheTest extends CiviUnitTestCase { $this->quickCleanup(['civicrm_prevnext_cache']); } - public function testSetItem() { - $cacheKeyString = 'TestCacheKeyString'; - $data = '1234afgbghh'; - $values = []; - $values[] = " ( 'civicrm_contact', 0, 0, '{$cacheKeyString}_stats', '$data' ) "; - $valueArray = CRM_Core_BAO_PrevNextCache::convertSetItemValues($values[0]); - // verify as SetItem would do that it converts the original values style into a sensible array format - $this->assertEquals(['civicrm_contact', 0, 0, 'TestCacheKeyString_stats', '1234afgbghh'], $valueArray); - CRM_Core_BAO_PrevNextCache::setItem($valueArray[0], $valueArray[1], $valueArray[2], $valueArray[3], $valueArray[4]); - $dao = new CRM_Core_BAO_PrevNextCache(); - $dao->cacheKey = 'TestCacheKeyString_stats'; - $dao->find(TRUE); - $this->assertEquals('1234afgbghh', $dao->data); - $this->assertEquals(0, $dao->entity_id1); - $this->assertEquals(0, $dao->entity_id2); - $this->assertEquals('civicrm_contact', $dao->entity_table); - } - } -- 2.25.1