[REF] Cleanup usage of CRM_Core_BAO_PrevNextCache::setItem and deprecate passing...

Fix conversion from old style format to new and add in unit test of conversion and confirm it stores data correctly in the table

Reformat Function as per Patrick's comments

Update doc block and add in comment detailing the change in function param

Fix Test

diff --git a/CRM/Core/BAO/PrevNextCache.php b/CRM/Core/BAO/PrevNextCache.php
index 04c9d5dfd4bc9f89dc86756053f2ccfb1d472601..165b130e9297c38e1632267480d776638b7a4356 100644 (file)
--- a/CRM/Core/BAO/PrevNextCache.php
+++ b/CRM/Core/BAO/PrevNextCache.php
@@ -338,14 +338,46 @@ FROM   civicrm_prevnext_cache pn
   }
 
   /**
-   * @param $values
+   * @param string $sqlValues string of SQLValues to insert
+   * @return array
    */
-  public static function setItem($values) {
-    $insert = "INSERT INTO civicrm_prevnext_cache ( entity_table, entity_id1, entity_id2, cachekey, data ) VALUES \n";
-    $query = $insert . implode(",\n ", $values);
+  public static function convertSetItemValues($sqlValues) {
+    $closingBrace = strpos($sqlValues, ')') - strlen($sqlValues);
+    $valueArray = array_map('trim', explode(', ', substr($sqlValues, strpos($sqlValues, '(') + 1, $closingBrace - 1)));
+    foreach ($valueArray as $key => &$value) {
+      // remove any quotes from values.
+      if (substr($value, 0, 1) == "'") {
+        $valueArray[$key] = substr($value, 1, -1);
+      }
+    }
+    return $valueArray;
+  }
 
-    //dump the dedupe matches in the prevnext_cache table
-    CRM_Core_DAO::executeQuery($query);
+  /**
+   * @param array|string $entity_table
+   * @param int $entity_id1
+   * @param int $entity_id2
+   * @param string $cacheKey
+   * @param string $data
+   */
+  public static function setItem($entity_table = NULL, $entity_id1 = NULL, $entity_id2 = NULL, $cacheKey = NULL, $data = NULL) {
+    // 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'));
+      foreach ($values as $value) {
+        $valueArray = self::convertSetItemValues($value);
+        self::setItem($valueArray[0], $valueArray[1], $valueArray[2], $valueArray[3], $valueArray[4]);
+      }
+    }
+    else {
+      CRM_Core_DAO::executeQuery("INSERT INTO civicrm_prevnext_cache (entity_table, entity_id1, entity_id2, cacheKey, data) VALUES
+        (%1, %2, %3, %4, '{$data}')", [
+          1 => [$entity_table, 'String'],
+          2 => [$entity_id1, 'Integer'],
+          3 => [$entity_id2, 'Integer'],
+          4 => [$cacheKey, 'String'],
+        ]);
+    }
   }
 
   /**

diff --git a/CRM/Dedupe/Finder.php b/CRM/Dedupe/Finder.php
index 7b6c936e72dfedcd603ca3912055beecdb0f8755..8d5e4462c47b27d5069020eeb34f6d9b593a1f2d 100644 (file)
--- a/CRM/Dedupe/Finder.php
+++ b/CRM/Dedupe/Finder.php
@@ -376,9 +376,8 @@ class CRM_Dedupe_Finder {
       ];
 
       $data = CRM_Core_DAO::escapeString(serialize($row));
-      $values[] = " ( 'civicrm_contact', $dstID, $srcID, '$cacheKeyString', '$data' ) ";
+      CRM_Core_BAO_PrevNextCache::setItem('civicrm_contact', $dstID, $srcID, $cacheKeyString, $data);
     }
-    CRM_Core_BAO_PrevNextCache::setItem($values);
     return $mainContacts;
   }
 

diff --git a/CRM/Dedupe/Merger.php b/CRM/Dedupe/Merger.php
index a57d64e1001de5254a66b4e6d665f8f2d5457e09..42507b9e7d0b3b76c4e031d063f6d7ac8cfba8a9 100644 (file)
--- a/CRM/Dedupe/Merger.php
+++ b/CRM/Dedupe/Merger.php
@@ -772,9 +772,7 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
     ];
     $data = CRM_Core_DAO::escapeString(serialize($data));
 
-    $values = [];
-    $values[] = " ( 'civicrm_contact', 0, 0, '{$cacheKeyString}_stats', '$data' ) ";
-    CRM_Core_BAO_PrevNextCache::setItem($values);
+    CRM_Core_BAO_PrevNextCache::setItem('civicrm_contact', 0, 0, $cacheKeyString . '_stats', $data);
   }
 
   /**

diff --git a/tests/phpunit/CRM/Core/BAO/PrevNextCacheTest.php b/tests/phpunit/CRM/Core/BAO/PrevNextCacheTest.php
index 2c532d33ea83a0993bb1609920dcd9c05bf2d6f3..39c491dc4086ec7068c6ddb881313ff835bc28ad 100644 (file)
--- a/tests/phpunit/CRM/Core/BAO/PrevNextCacheTest.php
+++ b/tests/phpunit/CRM/Core/BAO/PrevNextCacheTest.php
@@ -62,4 +62,22 @@ class CRM_Core_BAO_PrevNextCacheTest extends CiviUnitTestCase {
     $this->quickCleanup(array('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);
+  }
+
 }