(dev/core#217) PrevNext - Remove references to entity_table and entity_id2 from service
authorTim Otten <totten@civicrm.org>
Sun, 8 Jul 2018 02:31:36 +0000 (19:31 -0700)
committerTim Otten <totten@civicrm.org>
Thu, 16 Aug 2018 23:28:23 +0000 (16:28 -0700)
The PrevNext service/interface aims to be a replaceable component for use
the search-caching (but not deduping).

The interface that we produced from refactoring includes several references
to `entity_table` and `entity_id2` -- these values are part of the SQL
table, and they're needed for dedupe, but they don't seem to convey anything
meaningful for search-caching.  Including these fields makes the interface
more complicated -- which will make it hard to implement other variants.

The general gist of this commit is that we no longer fill those two columns,
and we no longer read them.

In a couple functions, we split the new OOP implementation
(`CRM_Core_PrevNextCache_Sql`) from the traditional static BAO
implementation (`CRM_Core_BAO_PrevNext`) so that we can omit these fields.

CRM/Campaign/Selector/Search.php
CRM/Contact/Selector.php
CRM/Core/PrevNextCache/Interface.php
CRM/Core/PrevNextCache/Sql.php

index 28c2b15b6d7961c6899021582a5765012b16e5fc..535a78349547fb580b15f66e9ba8123cdb56d2d0 100644 (file)
@@ -282,7 +282,7 @@ class CRM_Campaign_Selector_Search extends CRM_Core_Selector_Base implements CRM
       );
       list($select, $from) = explode(' FROM ', $sql);
       $selectSQL = "
-      SELECT 'civicrm_contact', contact_a.id, contact_a.id, '$cacheKey', contact_a.display_name
+      SELECT '$cacheKey', contact_a.id, contact_a.display_name
 FROM {$from}
 ";
 
index ad5bcdd0b07ef6772d06ee64b8fe68bf35a9cbe0..f048612ab7116eb803f6c86ded746d321315ddd2 100644 (file)
@@ -1038,7 +1038,7 @@ class CRM_Contact_Selector extends CRM_Core_Selector_Base implements CRM_Core_Se
     // the other alternative of running the FULL query will just be incredibly inefficient
     // and slow things down way too much on large data sets / complex queries
 
-    $selectSQL = "SELECT DISTINCT 'civicrm_contact', contact_a.id, contact_a.id, '$cacheKey', contact_a.sort_name";
+    $selectSQL = "SELECT DISTINCT '$cacheKey', contact_a.id, contact_a.sort_name";
 
     $sql = str_replace(array("SELECT contact_a.id as contact_id", "SELECT contact_a.id as id"), $selectSQL, $sql);
     try {
index c71b57ed24f05c0afb07f9adc9c1993205cde1a3..edaf0d6778ce9bae1658773d6d249c458f1b7a4c 100644 (file)
@@ -39,7 +39,7 @@ interface CRM_Core_PrevNextCache_Interface {
    * @param string $cacheKey
    * @param string $sql
    *   A SQL query. The query *MUST* be a SELECT statement which yields
-   *   the following columns (in order): entity_table, entity_id1, entity_id2, cacheKey, data
+   *   the following columns (in order): cacheKey, entity_id1, data
    * @return bool
    */
   public function fillWithSql($cacheKey, $sql);
@@ -50,9 +50,7 @@ interface CRM_Core_PrevNextCache_Interface {
    * @param string $cacheKey
    * @param array $rows
    *   A list of cache records. Each record should have keys:
-   *    - entity_table
    *    - entity_id1
-   *    - entity_id2
    *    - data
    * @return bool
    */
@@ -89,20 +87,24 @@ interface CRM_Core_PrevNextCache_Interface {
    *
    * @param string $cacheKey
    * @param int $id1
-   * @param int $id2
    *
    * @return array
+   *   List of neighbors.
+   *   [
+   *     'foundEntry' => 1,
+   *     'prev' => ['id1' => 123, 'data'=>'foo'],
+   *     'next' => ['id1' => 456, 'data'=>'foo'],
+   *   ]
    */
-  public function getPositions($cacheKey, $id1, $id2);
+  public function getPositions($cacheKey, $id1);
 
   /**
    * Delete an item from the prevnext cache table based on the entity.
    *
    * @param int $id
    * @param string $cacheKey
-   * @param string $entityTable
    */
-  public function deleteItem($id = NULL, $cacheKey = NULL, $entityTable = 'civicrm_contact');
+  public function deleteItem($id = NULL, $cacheKey = NULL);
 
   /**
    * Get count of matching rows.
index 6483fda8acac9561b4c460348ec4dd0c5dba8384..5a5897353332e0ee6b74fe2d7dcd08d152271beb 100644 (file)
@@ -37,13 +37,13 @@ class CRM_Core_PrevNextCache_Sql implements CRM_Core_PrevNextCache_Interface {
    *
    * @param string $sql
    *   A SQL query. The query *MUST* be a SELECT statement which yields
-   *   the following columns (in order): entity_table, entity_id1, entity_id2, cacheKey, data
+   *   the following columns (in order): cacheKey, entity_id1, data
    * @return bool
    * @throws CRM_Core_Exception
    */
   public function fillWithSql($cacheKey, $sql) {
     $insertSQL = "
-INSERT INTO civicrm_prevnext_cache ( entity_table, entity_id1, entity_id2, cacheKey, data )
+INSERT INTO civicrm_prevnext_cache (cacheKey, entity_id1, data)
 ";
     $result = CRM_Core_DAO::executeQuery($insertSQL . $sql, [], FALSE, NULL, FALSE, TRUE, TRUE);
     if (is_a($result, 'DB_Error')) {
@@ -59,9 +59,7 @@ INSERT INTO civicrm_prevnext_cache ( entity_table, entity_id1, entity_id2, cache
 
     $insert = CRM_Utils_SQL_Insert::into('civicrm_prevnext_cache')
       ->columns([
-        'entity_table',
         'entity_id1',
-        'entity_id2',
         'cacheKey',
         'data'
       ]);
@@ -85,14 +83,11 @@ INSERT INTO civicrm_prevnext_cache ( entity_table, entity_id1, entity_id2, cache
    *   To unselect all contact IDs, use NULL.
    */
   public function markSelection($cacheKey, $action, $cIds = NULL) {
-    $entity_table = 'civicrm_contact';
-
     if (!$cacheKey) {
       return;
     }
     $params = array();
 
-    $entity_whereClause = " AND entity_table = '{$entity_table}'";
     if ($cIds && $cacheKey && $action) {
       if (is_array($cIds)) {
         $cIdFilter = "(" . implode(',', $cIds) . ")";
@@ -110,12 +105,12 @@ AND (entity_id1 = %2 OR entity_id2 = %2)
       }
       if ($action == 'select') {
         $whereClause .= "AND is_selected = 0";
-        $sql = "UPDATE civicrm_prevnext_cache SET is_selected = 1 {$whereClause} {$entity_whereClause}";
+        $sql = "UPDATE civicrm_prevnext_cache SET is_selected = 1 {$whereClause}";
         $params[1] = array($cacheKey, 'String');
       }
       elseif ($action == 'unselect') {
         $whereClause .= "AND is_selected = 1";
-        $sql = "UPDATE civicrm_prevnext_cache SET is_selected = 0 {$whereClause} {$entity_whereClause}";
+        $sql = "UPDATE civicrm_prevnext_cache SET is_selected = 0 {$whereClause}";
         $params[1] = array($cacheKey, 'String');
       }
       // default action is reseting
@@ -125,7 +120,6 @@ AND (entity_id1 = %2 OR entity_id2 = %2)
 UPDATE civicrm_prevnext_cache
 SET    is_selected = 0
 WHERE  cacheKey = %1 AND is_selected = 1
-       {$entity_whereClause}
 ";
       $params[1] = array($cacheKey, 'String');
     }
@@ -145,21 +139,17 @@ WHERE  cacheKey = %1 AND is_selected = 1
    * @return array|NULL
    */
   public function getSelection($cacheKey, $action = 'get') {
-    $entity_table = 'civicrm_contact';
-
     if (!$cacheKey) {
       return NULL;
     }
     $params = array();
 
-    $entity_whereClause = " AND entity_table = '{$entity_table}'";
     if ($cacheKey && ($action == 'get' || $action == 'getall')) {
       $actionGet = ($action == "get") ? " AND is_selected = 1 " : "";
       $sql = "
-SELECT entity_id1, entity_id2 FROM civicrm_prevnext_cache
+SELECT entity_id1 FROM civicrm_prevnext_cache
 WHERE cacheKey = %1
       $actionGet
-      $entity_whereClause
 ORDER BY id
 ";
       $params[1] = array($cacheKey, 'String');
@@ -167,9 +157,7 @@ ORDER BY id
       $contactIds = array($cacheKey => array());
       $cIdDao = CRM_Core_DAO::executeQuery($sql, $params);
       while ($cIdDao->fetch()) {
-        if ($cIdDao->entity_id1 == $cIdDao->entity_id2) {
-          $contactIds[$cacheKey][$cIdDao->entity_id1] = 1;
-        }
+        $contactIds[$cacheKey][$cIdDao->entity_id1] = 1;
       }
       return $contactIds;
     }
@@ -180,15 +168,46 @@ ORDER BY id
    *
    * @param string $cacheKey
    * @param int $id1
-   * @param int $id2
-   *
-   * NOTE: I don't really get why there are two ID columns, but we'll
-   * keep passing them through as a matter of safe-refactoring.
    *
    * @return array
    */
-  public function getPositions($cacheKey, $id1, $id2) {
-    return CRM_Core_BAO_PrevNextCache::getPositions($cacheKey, $id1, $id2);
+  public function getPositions($cacheKey, $id1) {
+    $mergeId = CRM_Core_DAO::singleValueQuery(
+      "SELECT id FROM civicrm_prevnext_cache WHERE cacheKey = %2 AND entity_id1 = %1",
+      [
+        1 => [$id1, 'Integer'],
+        2 => [$cacheKey, 'String'],
+      ]
+    );
+
+    $pos = ['foundEntry' => 0];
+    if ($mergeId) {
+      $pos['foundEntry'] = 1;
+
+      $sql = "SELECT pn.id, pn.entity_id1, pn.entity_id2, pn.data FROM civicrm_prevnext_cache pn ";
+      $wherePrev = " WHERE pn.id < %1 AND pn.cacheKey = %2 ORDER BY ID DESC LIMIT 1";
+      $whereNext = " WHERE pn.id > %1 AND pn.cacheKey = %2 ORDER BY ID ASC LIMIT 1";
+      $p = [
+        1 => [$mergeId, 'Integer'],
+        2 => [$cacheKey, 'String'],
+      ];
+
+      $dao = CRM_Core_DAO::executeQuery($sql . $wherePrev, $p);
+      if ($dao->fetch()) {
+        $pos['prev']['id1'] = $dao->entity_id1;
+        $pos['prev']['mergeId'] = $dao->id;
+        $pos['prev']['data'] = $dao->data;
+      }
+
+      $dao = CRM_Core_DAO::executeQuery($sql . $whereNext, $p);
+      if ($dao->fetch()) {
+        $pos['next']['id1'] = $dao->entity_id1;
+        $pos['next']['mergeId'] = $dao->id;
+        $pos['next']['data'] = $dao->data;
+      }
+    }
+    return $pos;
+
   }
 
   /**
@@ -196,11 +215,10 @@ ORDER BY id
    *
    * @param int $id
    * @param string $cacheKey
-   * @param string $entityTable
    */
-  public function deleteItem($id = NULL, $cacheKey = NULL, $entityTable = 'civicrm_contact') {
-    $sql = "DELETE FROM civicrm_prevnext_cache WHERE  entity_table = %1";
-    $params = array(1 => array($entityTable, 'String'));
+  public function deleteItem($id = NULL, $cacheKey = NULL) {
+    $sql = "DELETE FROM civicrm_prevnext_cache WHERE (1)";
+    $params = array();
 
     if (is_numeric($id)) {
       $sql .= " AND ( entity_id1 = %2 OR entity_id2 = %2 )";
@@ -221,7 +239,9 @@ ORDER BY id
    * @return int
    */
   public function getCount($cacheKey) {
-    return CRM_Core_BAO_PrevNextCache::getCount($cacheKey, NULL, "entity_table = 'civicrm_contact'");
+    $query = "SELECT COUNT(*) FROM civicrm_prevnext_cache pn WHERE pn.cacheKey = %1";
+    $params = [1 => [$cacheKey, 'String']];
+    return (int) CRM_Core_DAO::singleValueQuery($query, $params, TRUE, FALSE);
   }
 
 }