From e1c519d764c556d5bc38ec1c8031a7900eca930e Mon Sep 17 00:00:00 2001
From: Seamus Lee <seamuslee001@gmail.com>
Date: Sat, 29 Jun 2019 12:15:37 +1000
Subject: [PATCH] [REF] Cleanup usage of CRM_Core_BAO_PrevNextCache::setItem
 and deprecate passing in array of values

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
---
 CRM/Core/BAO/PrevNextCache.php                | 44 ++++++++++++++++---
 CRM/Dedupe/Finder.php                         |  3 +-
 CRM/Dedupe/Merger.php                         |  4 +-
 .../CRM/Core/BAO/PrevNextCacheTest.php        | 18 ++++++++
 4 files changed, 58 insertions(+), 11 deletions(-)

diff --git a/CRM/Core/BAO/PrevNextCache.php b/CRM/Core/BAO/PrevNextCache.php
index 04c9d5dfd4..165b130e92 100644
--- 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 7b6c936e72..8d5e4462c4 100644
--- 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 a57d64e100..42507b9e7d 100644
--- 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 2c532d33ea..39c491dc40 100644
--- 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);
+  }
+
 }
-- 
2.25.1