Skip to content

Commit

Permalink
Merge pull request #12379 from totten/master-psr16-sqlgroup
Browse files Browse the repository at this point in the history
(dev/core#174) Full PSR-16 compliance for SqlGroup
  • Loading branch information
eileenmcnaughton authored Jul 2, 2018
2 parents 6936f14 + fd33fea commit 3962e60
Show file tree
Hide file tree
Showing 5 changed files with 165 additions and 29 deletions.
14 changes: 11 additions & 3 deletions CRM/Core/BAO/Cache.php
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ public static function restoreSessionFromCache($names) {
* @param bool $table
* @param bool $prevNext
*/
public static function cleanup($session = FALSE, $table = FALSE, $prevNext = FALSE) {
public static function cleanup($session = FALSE, $table = FALSE, $prevNext = FALSE, $expired = FALSE) {
// first delete all sessions more than 20 minutes old which are related to any potential transaction
$timeIntervalMins = (int) Civi::settings()->get('secure_cache_timeout_minutes');
if ($timeIntervalMins && $session) {
Expand Down Expand Up @@ -338,10 +338,10 @@ public static function cleanup($session = FALSE, $table = FALSE, $prevNext = FAL
$timeIntervalDays = 2;

if (mt_rand(1, 100000) % $cleanUpNumber == 0) {
$session = $table = $prevNext = TRUE;
$expired = $session = $table = $prevNext = TRUE;
}

if (!$session && !$table && !$prevNext) {
if (!$session && !$table && !$prevNext && !$expired) {
return;
}

Expand All @@ -363,6 +363,14 @@ public static function cleanup($session = FALSE, $table = FALSE, $prevNext = FAL
";
CRM_Core_DAO::executeQuery($sql);
}

if ($expired) {
$sql = "DELETE FROM civicrm_cache WHERE expired_date < %1";
$params = [
1 => [date(CRM_Utils_Cache_SqlGroup::TS_FMT, CRM_Utils_Time::getTimeRaw()), 'String'],
];
CRM_Core_DAO::executeQuery($sql, $params);
}
}

/**
Expand Down
130 changes: 107 additions & 23 deletions CRM/Utils/Cache/SqlGroup.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,11 @@
*/
class CRM_Utils_Cache_SqlGroup implements CRM_Utils_Cache_Interface {

// 6*60*60
const DEFAULT_TTL = 21600;

const TS_FMT = 'Y-m-d H:i:s';
use CRM_Utils_Cache_NaiveMultipleTrait; // TODO Consider native implementation.
use CRM_Utils_Cache_NaiveHasTrait; // TODO Native implementation

/**
* The host name of the memcached server.
Expand All @@ -56,7 +59,18 @@ class CRM_Utils_Cache_SqlGroup implements CRM_Utils_Cache_Interface {
/**
* @var array in-memory cache to optimize redundant get()s
*/
protected $frontCache;
protected $valueCache;

/**
* @var array in-memory cache to optimize redundant get()s
* Note: expiresCache[$key]===NULL means cache-miss
*/
protected $expiresCache;

/**
* @var string
*/
protected $table;

/**
* Constructor.
Expand All @@ -71,6 +85,7 @@ class CRM_Utils_Cache_SqlGroup implements CRM_Utils_Cache_Interface {
* @return \CRM_Utils_Cache_SqlGroup
*/
public function __construct($config) {
$this->table = CRM_Core_DAO_Cache::getTableName();
if (isset($config['group'])) {
$this->group = $config['group'];
}
Expand All @@ -83,7 +98,7 @@ public function __construct($config) {
else {
$this->componentID = NULL;
}
$this->frontCache = array();
$this->valueCache = array();
if (CRM_Utils_Array::value('prefetch', $config, TRUE)) {
$this->prefetch();
}
Expand All @@ -96,11 +111,47 @@ public function __construct($config) {
* @return bool
*/
public function set($key, $value, $ttl = NULL) {
if ($ttl !== NULL) {
throw new \RuntimeException("FIXME: " . __CLASS__ . "::set() should support non-NULL TTL");
CRM_Utils_Cache::assertValidKey($key);

$lock = Civi::lockManager()->acquire("cache.{$this->group}_{$key}._null");
if (!$lock->isAcquired()) {
throw new \CRM_Utils_Cache_CacheException("SqlGroup: Failed to acquire lock on cache key.");
}

$dataExists = CRM_Core_DAO::singleValueQuery("SELECT COUNT(*) FROM {$this->table} WHERE {$this->where($key)}");
$expires = CRM_Utils_Date::convertCacheTtlToExpires($ttl, self::DEFAULT_TTL);

$dataSerialized = CRM_Core_BAO_Cache::encode($value);

// This table has a wonky index, so we cannot use REPLACE or
// "INSERT ... ON DUPE". Instead, use SELECT+(INSERT|UPDATE).
if ($dataExists) {
$sql = "UPDATE {$this->table} SET data = %1, created_date = FROM_UNIXTIME(%2), expired_date = FROM_UNIXTIME(%3) WHERE {$this->where($key)}";
$args = array(
1 => array($dataSerialized, 'String'),
2 => array(time(), 'Positive'),
3 => array($expires, 'Positive'),
);
$dao = CRM_Core_DAO::executeQuery($sql, $args, FALSE, NULL, FALSE, FALSE);
}
else {
$sql = "INSERT INTO {$this->table} (group_name,path,data,created_date,expired_date) VALUES (%1,%2,%3,FROM_UNIXTIME(%4),FROM_UNIXTIME(%5))";
$args = array(
1 => [$this->group, 'String'],
2 => [$key, 'String'],
3 => [$dataSerialized, 'String'],
4 => [time(), 'Positive'],
5 => [$expires, 'Positive'],
);
$dao = CRM_Core_DAO::executeQuery($sql, $args, FALSE, NULL, FALSE, FALSE);
}
CRM_Core_BAO_Cache::setItem($value, $this->group, $key, $this->componentID);
$this->frontCache[$key] = $value;

$lock->release();

$dao->free();

$this->valueCache[$key] = CRM_Core_BAO_Cache::decode($dataSerialized);
$this->expiresCache[$key] = $expires;
return TRUE;
}

Expand All @@ -111,13 +162,21 @@ public function set($key, $value, $ttl = NULL) {
* @return mixed
*/
public function get($key, $default = NULL) {
if ($default !== NULL) {
throw new \RuntimeException("FIXME: " . __CLASS__ . "::get() only supports NULL default");
}
if (!array_key_exists($key, $this->frontCache)) {
$this->frontCache[$key] = CRM_Core_BAO_Cache::getItem($this->group, $key, $this->componentID);
CRM_Utils_Cache::assertValidKey($key);
if (!isset($this->expiresCache[$key]) || time() >= $this->expiresCache[$key]) {
$sql = "SELECT path, data, UNIX_TIMESTAMP(expired_date) as expires FROM {$this->table} WHERE " . $this->where($key);
$dao = CRM_Core_DAO::executeQuery($sql);
while ($dao->fetch()) {
$this->expiresCache[$key] = $dao->expires;
$this->valueCache[$key] = CRM_Core_BAO_Cache::decode($dao->data);
}
$dao->free();
}
return $this->frontCache[$key];
return (isset($this->expiresCache[$key]) && time() < $this->expiresCache[$key]) ? $this->reobjectify($this->valueCache[$key]) : $default;
}

private function reobjectify($value) {
return is_object($value) ? unserialize(serialize($value)) : $value;
}

/**
Expand All @@ -127,26 +186,35 @@ public function get($key, $default = NULL) {
* @return mixed
*/
public function getFromFrontCache($key, $default = NULL) {
return CRM_Utils_Array::value($key, $this->frontCache, $default);
if (isset($this->expiresCache[$key]) && time() < $this->expiresCache[$key] && $this->valueCache[$key]) {
return $this->reobjectify($this->valueCache[$key]);
}
else {
return $default;
}
}

public function has($key) {
$this->get($key);
return isset($this->expiresCache[$key]) && time() < $this->expiresCache[$key];
}

/**
* @param string $key
* @return bool
*/
public function delete($key) {
CRM_Core_BAO_Cache::deleteGroup($this->group, $key, FALSE);
CRM_Core_BAO_Cache::$_cache = NULL; // FIXME: remove multitier cache
CRM_Utils_Cache::singleton()->flush(); // FIXME: remove multitier cache
unset($this->frontCache[$key]);
CRM_Utils_Cache::assertValidKey($key);
CRM_Core_DAO::executeQuery("DELETE FROM {$this->table} WHERE {$this->where($key)}");
unset($this->valueCache[$key]);
unset($this->expiresCache[$key]);
return TRUE;
}

public function flush() {
CRM_Core_BAO_Cache::deleteGroup($this->group, NULL, FALSE);
CRM_Core_BAO_Cache::$_cache = NULL; // FIXME: remove multitier cache
CRM_Utils_Cache::singleton()->flush(); // FIXME: remove multitier cache
$this->frontCache = array();
CRM_Core_DAO::executeQuery("DELETE FROM {$this->table} WHERE {$this->where()}");
$this->valueCache = array();
$this->expiresCache = array();
return TRUE;
}

Expand All @@ -155,7 +223,23 @@ public function clear() {
}

public function prefetch() {
$this->frontCache = CRM_Core_BAO_Cache::getItems($this->group, $this->componentID);
$dao = CRM_Core_DAO::executeQuery("SELECT path, data, UNIX_TIMESTAMP(expired_date) AS expires FROM {$this->table} WHERE " . $this->where(NULL));
$this->valueCache = array();
$this->expiresCache = array();
while ($dao->fetch()) {
$this->valueCache[$dao->path] = CRM_Core_BAO_Cache::decode($dao->data);
$this->expiresCache[$dao->path] = $dao->expires;
}
$dao->free();
}

protected function where($path = NULL) {
$clauses = array();
$clauses[] = ('group_name = "' . CRM_Core_DAO::escapeString($this->group) . '"');
if ($path) {
$clauses[] = ('path = "' . CRM_Core_DAO::escapeString($path) . '"');
}
return $clauses ? implode(' AND ', $clauses) : '(1)';
}

}
5 changes: 3 additions & 2 deletions api/v3/Job.php
Original file line number Diff line number Diff line change
Expand Up @@ -619,14 +619,15 @@ function civicrm_api3_job_cleanup($params) {
$session = CRM_Utils_Array::value('session', $params, TRUE);
$tempTable = CRM_Utils_Array::value('tempTables', $params, TRUE);
$jobLog = CRM_Utils_Array::value('jobLog', $params, TRUE);
$expired = CRM_Utils_Array::value('expiredDbCache', $params, TRUE);
$prevNext = CRM_Utils_Array::value('prevNext', $params, TRUE);
$dbCache = CRM_Utils_Array::value('dbCache', $params, FALSE);
$memCache = CRM_Utils_Array::value('memCache', $params, FALSE);
$tplCache = CRM_Utils_Array::value('tplCache', $params, FALSE);
$wordRplc = CRM_Utils_Array::value('wordRplc', $params, FALSE);

if ($session || $tempTable || $prevNext) {
CRM_Core_BAO_Cache::cleanup($session, $tempTable, $prevNext);
if ($session || $tempTable || $prevNext || $expired) {
CRM_Core_BAO_Cache::cleanup($session, $tempTable, $prevNext, $expired);
}

if ($jobLog) {
Expand Down
4 changes: 3 additions & 1 deletion tests/phpunit/CRM/Utils/Cache/SqlGroupTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@ public function testTwoInstance() {
));
$fooValue = array('whiz' => 'bang', 'bar' => 3);
$a->set('foo', $fooValue);
$this->assertEquals($a->get('foo'), array('whiz' => 'bang', 'bar' => 3));
$getValue = $a->get('foo');
$expectValue = array('whiz' => 'bang', 'bar' => 3);
$this->assertEquals($getValue, $expectValue);

$b = new CRM_Utils_Cache_SqlGroup(array(
'group' => 'testTwoInstance',
Expand Down
41 changes: 41 additions & 0 deletions tests/phpunit/E2E/Cache/SqlGroupTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?php
/*
+--------------------------------------------------------------------+
| CiviCRM version 5 |
+--------------------------------------------------------------------+
| Copyright CiviCRM LLC (c) 2004-2018 |
+--------------------------------------------------------------------+
| This file is a part of CiviCRM. |
| |
| CiviCRM is free software; you can copy, modify, and distribute it |
| under the terms of the GNU Affero General Public License |
| Version 3, 19 November 2007 and the CiviCRM Licensing Exception. |
| |
| CiviCRM is distributed in the hope that it will be useful, but |
| WITHOUT ANY WARRANTY; without even the implied warranty of |
| MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. |
| See the GNU Affero General Public License for more details. |
| |
| You should have received a copy of the GNU Affero General Public |
| License along with this program; if not, contact CiviCRM LLC |
| at info[AT]civicrm[DOT]org. If you have questions about the |
| GNU Affero General Public License or the licensing of CiviCRM, |
| see the CiviCRM license FAQ at http://civicrm.org/licensing |
+--------------------------------------------------------------------+
*/

/**
* Verify that CRM_Utils_Cache_SqlGroup complies with PSR-16.
*
* @group e2e
*/
class E2E_Cache_SqlGroupTest extends E2E_Cache_CacheTestCase {

public function createSimpleCache() {
return CRM_Utils_Cache::create([
'name' => 'e2e sqlgroup test',
'type' => ['SqlGroup'],
]);
}

}

0 comments on commit 3962e60

Please sign in to comment.