CRM-18142 fix use of uninidexed TIMESTAMPDIFF function

This removes function comparisons in favour of direct comparisons on cache date functions

diff --git a/CRM/ACL/BAO/Cache.php b/CRM/ACL/BAO/Cache.php
index f0b67a20574f3327d430d6471a35cc5a77e800a4..11ac3d33f1a02f125e8dfd49afc04d7a4179d55d 100644 (file)
--- a/CRM/ACL/BAO/Cache.php
+++ b/CRM/ACL/BAO/Cache.php
@@ -145,17 +145,14 @@ WHERE contact_id = %1
     // reset any static caching
     self::$_cache = NULL;
 
-    // reset any db caching
-    $config = CRM_Core_Config::singleton();
-    $smartGroupCacheTimeout = CRM_Contact_BAO_GroupContactCache::smartGroupCacheTimeout();
-
     $query = "
 DELETE
 FROM   civicrm_acl_cache
 WHERE  modified_date IS NULL
-   OR  (TIMESTAMPDIFF(MINUTE, modified_date, NOW()) >= $smartGroupCacheTimeout)
+   OR  (modified_date <= %1)
 ";
-    CRM_Core_DAO::singleValueQuery($query);
+    $params = array(1 => array(CRM_Contact_BAO_GroupContactCache::getCacheInvalidDateTime(), 'String'));
+    CRM_Core_DAO::singleValueQuery($query, $params);
 
     // CRM_Core_DAO::singleValueQuery("TRUNCATE TABLE civicrm_acl_contact_cache"); // No, force-commits transaction
     // CRM_Core_DAO::singleValueQuery("DELETE FROM civicrm_acl_contact_cache"); // Transaction-safe

diff --git a/CRM/Contact/BAO/GroupContactCache.php b/CRM/Contact/BAO/GroupContactCache.php
index 2785d34c0e0394c47da46d250d08425d4c07db74..f74912c432c2d71f43502290f3b929e9b429847b 100644 (file)
--- a/CRM/Contact/BAO/GroupContactCache.php
+++ b/CRM/Contact/BAO/GroupContactCache.php
@@ -67,18 +67,18 @@ class CRM_Contact_BAO_GroupContactCache extends CRM_Contact_DAO_GroupContactCach
    *   the sql query which lists the groups that need to be refreshed
    */
   public static function groupRefreshedClause($groupIDClause = NULL, $includeHiddenGroups = FALSE) {
-    $smartGroupCacheTimeout = self::smartGroupCacheTimeout();
+    $smartGroupCacheTimeoutDateTime = self::getCacheInvalidDateTime();
 
     $query = "
 SELECT  g.id
 FROM    civicrm_group g
 WHERE   ( g.saved_search_id IS NOT NULL OR g.children IS NOT NULL )
 AND     g.is_active = 1
-AND     ( g.cache_date IS NULL OR
-          ( TIMESTAMPDIFF(MINUTE, g.cache_date, NOW()) >= $smartGroupCacheTimeout ) OR
-          ( NOW() >= g.refresh_date )
-        )
-";
+AND (
+  g.cache_date IS NULL
+  OR cache_date <= $smartGroupCacheTimeoutDateTime
+  OR NOW() >= g.refresh_date
+)";
 
     if (!$includeHiddenGroups) {
       $query .= "AND (g.is_hidden = 0 OR g.is_hidden IS NULL)";
@@ -264,7 +264,6 @@ AND    g.refresh_date IS NULL
     // only update cache entry if we had any values
     if ($processed) {
       // also update the group with cache date information
-      //make sure to give original timezone settings again.
       $now = date('YmdHis');
       $refresh = 'null';
     }
@@ -300,7 +299,7 @@ WHERE  id IN ( $groupIDs )
     static $invoked = FALSE;
 
     // typically this needs to happy only once per instance
-    // this is especially TRUE in import, where we dont need
+    // this is especially TRUE in import, where we don't need
     // to do this all the time
     // this optimization is done only when no groupID is passed
     // i.e. cache is reset for all groups
@@ -325,10 +324,11 @@ WHERE  id IN ( $groupIDs )
     }
 
     $refresh = NULL;
-    $params = array();
     $smartGroupCacheTimeout = self::smartGroupCacheTimeout();
-
-    $refreshTime = self::getRefreshDateTime();
+    $params = array(
+      1 => array(self::getCacheInvalidDateTime(), 'String'),
+      2 => array(self::getRefreshDateTime(), 'String'),
+    );
 
     if (!isset($groupID)) {
       if ($smartGroupCacheTimeout == 0) {
@@ -357,12 +357,10 @@ WHERE  g.cache_date <= %1
 ";
         $refresh = "
 UPDATE civicrm_group g
-SET    refresh_date = $refreshTime
-WHERE  g.cache_date > %1
+SET    refresh_date = %2
+WHERE  g.cache_date < %1
 AND    refresh_date IS NULL
 ";
-        $cacheTime = date('Y-m-d H-i-s', strtotime("- $smartGroupCacheTimeout minutes"));
-        $params = array(1 => array($cacheTime, 'String'));
       }
     }
     elseif (is_array($groupID)) {
@@ -446,10 +444,10 @@ WHERE  id = %1
       return;
     }
 
-    // grab a lock so other processes dont compete and do the same query
+    // grab a lock so other processes don't compete and do the same query
     $lock = Civi::lockManager()->acquire("data.core.group.{$groupID}");
     if (!$lock->isAcquired()) {
-      // this can cause inconsistent results since we dont know if the other process
+      // this can cause inconsistent results since we don't know if the other process
       // will fill up the cache before our calling routine needs it.
       // however this routine does not return the status either, so basically
       // its a "lets return and hope for the best"
@@ -706,6 +704,18 @@ ORDER BY   gc.contact_id, g.children
     }
   }
 
+  /**
+   * Get the datetime from which the cache should be considered invalid.
+   *
+   * Ie if the smartgroup cache timeout is 5 minutes ago then the cache is invalid if it was
+   * refreshed 6 minutes ago, but not if it was refreshed 4 minutes ago.
+   *
+   * @return string
+   */
+  public static function getCacheInvalidDateTime() {
+    return date('Ymdhis', strtotime("-" . self::smartGroupCacheTimeout() . " Minutes"));
+  }
+
   /**
    * Get the date when the cache should be refreshed from.
    *
@@ -713,7 +723,7 @@ ORDER BY   gc.contact_id, g.children
    *
    * @return string
    */
-  protected static function getRefreshDateTime() {
+  public static function getRefreshDateTime() {
     return date('Ymdhis', strtotime("+ " . self::smartGroupCacheTimeout() . " Minutes"));
   }
 

diff --git a/Civi/ActionSchedule/RecipientBuilder.php b/Civi/ActionSchedule/RecipientBuilder.php
index 450dcfe270fc3cf70773344fdf0a17d10aab0ba8..274eedce2493fb4c88625c72ba701c26ec924e3d 100644 (file)
--- a/Civi/ActionSchedule/RecipientBuilder.php
+++ b/Civi/ActionSchedule/RecipientBuilder.php
@@ -281,6 +281,7 @@ class RecipientBuilder {
       ->merge($this->prepareRepetitionEndFilter($query['casDateField']))
       ->where($this->actionSchedule->start_action_date ? $startDateClauses[0] : array())
       ->groupBy("reminder.contact_id, reminder.entity_id, reminder.entity_table")
+      // @todo replace use of timestampdiff with a direct comparison as TIMESTAMPDIFF cannot use an index.
       ->having("TIMESTAMPDIFF(HOUR, latest_log_time, CAST(!casNow AS datetime)) >= TIMESTAMPDIFF(HOUR, latest_log_time, DATE_ADD(latest_log_time, INTERVAL !casRepetitionInterval))")
       ->param(array(
         'casRepetitionInterval' => $this->parseRepetitionInterval(),
@@ -328,6 +329,7 @@ class RecipientBuilder {
         ->merge($this->prepareAddlFilter('c.id'))
         ->where("c.is_deleted = 0 AND c.is_deceased = 0")
         ->groupBy("reminder.contact_id")
+        // @todo replace use of timestampdiff with a direct comparison as TIMESTAMPDIFF cannot use an index.
         ->having("TIMESTAMPDIFF(HOUR, latest_log_time, CAST(!casNow AS datetime)) >= TIMESTAMPDIFF(HOUR, latest_log_time, DATE_ADD(latest_log_time, INTERVAL !casRepetitionInterval)")
         ->param(array(
           'casRepetitionInterval' => $this->parseRepetitionInterval(),