From 1443004d5c36b16e913d7728ef41e9325a4b0f6e Mon Sep 17 00:00:00 2001 From: eileenmcnaugton Date: Mon, 16 May 2016 11:18:27 +1200 Subject: [PATCH] CRM-18142 fix use of uninidexed TIMESTAMPDIFF function This removes function comparisons in favour of direct comparisons on cache date functions Conflicts: CRM/Contact/BAO/GroupContactCache.php Civi/ActionSchedule/RecipientBuilder.php --- CRM/ACL/BAO/Cache.php | 9 ++---- CRM/Contact/BAO/GroupContactCache.php | 46 ++++++++++++++++----------- 2 files changed, 31 insertions(+), 24 deletions(-) diff --git a/CRM/ACL/BAO/Cache.php b/CRM/ACL/BAO/Cache.php index 6f88774dec..e632024142 100644 --- a/CRM/ACL/BAO/Cache.php +++ b/CRM/ACL/BAO/Cache.php @@ -147,17 +147,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 c105e87fa0..7f91329460 100644 --- a/CRM/Contact/BAO/GroupContactCache.php +++ b/CRM/Contact/BAO/GroupContactCache.php @@ -68,18 +68,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)"; @@ -265,7 +265,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'; } @@ -303,7 +302,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 @@ -328,10 +327,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) { @@ -360,12 +360,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)) { @@ -449,10 +447,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\Core\Container::singleton()->get('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" @@ -703,6 +701,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. * @@ -710,7 +720,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")); } -- 2.25.1