Remove calls & deprecate CRM_Core_BAO_PrevNextCache::setItem
authoreileen <emcnaughton@wikimedia.org>
Fri, 6 Mar 2020 00:36:23 +0000 (13:36 +1300)
committereileen <emcnaughton@wikimedia.org>
Fri, 6 Mar 2020 00:41:31 +0000 (13:41 +1300)
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
CRM/Dedupe/Finder.php
CRM/Dedupe/Merger.php
tests/phpunit/CRM/Core/BAO/PrevNextCacheTest.php

index e7aa7b4382f1a16fa3aa2d37bcdca7e3bda5586d..ca53363abf82427667e353bb0dcf762ef76d8450 100644 (file)
@@ -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'));
index f2626bcb328ed8eebeb8cf7edf87d3552b90426b..dac4bf1359fbf63b00fec751f54248eebb3bd0ed 100644 (file)
@@ -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;
   }
index c5f063d1dfd42c5845138ac3a6291ba8f6bd6057..00f2901fa5c9e22d93c25b8f3d158cb16c66a4a6 100644 (file)
@@ -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']]);
   }
 
   /**
index 2c3a6175dc987d5edbcbf01588945dd99ebe530c..3c1e92fc8b63e9aa8338aa0d611fc11552e1e099 100644 (file)
@@ -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);
-  }
-
 }