From: Tim Otten Date: Wed, 10 Jul 2019 02:21:58 +0000 (-0700) Subject: GenCode, Cache::cleanKey() - Fix deploop during clean initialization X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=77f080cbe1c0062631f43244d7f37edfc133c0b7;p=civicrm-core.git GenCode, Cache::cleanKey() - Fix deploop during clean initialization Overview -------- Suppose you have a clean codebase with no DAO files, and you try to run `xml/GenCode.php`. This currently crashes. The problem is not symptomatic day-to-day because we commit DAOs, so it's very rare to run a full/clean initialization. However, it does get in the way of stress-testing the correctness of GenCode. Before ------ Since 76e697a9d750d568d0d12c10dd8173f656e8bb1e, I believe we've had a dependency-loop in the clean-gencode scenario, which works as follows: * We don't have any DAOs, so we run `GenCode`. * Running `GenCode` does a partial bootstrap (i.e. `CRM_Core_Config::singleton($loadFromDB===FALSE)`) * The partial bootstrap creates some thread-local caches (`Arraycache`) * Before creating the cache, it normalizes the name by calling `CRM_Core_BAO_Cache::cleanKey()` * `CRM_Core_BAO_Cache` extends `CRM_Core_DAO_Cache` * `CRM_Core_BAO_Cache` doesn't exist - that's why we called `GenCode` at the beginning. After ----- This migrates the utility function `cleanKey()` to another class which is always available and does not rely on DAOs. --- diff --git a/CRM/ACL/BAO/ACL.php b/CRM/ACL/BAO/ACL.php index 0051422252..abf87247bb 100644 --- a/CRM/ACL/BAO/ACL.php +++ b/CRM/ACL/BAO/ACL.php @@ -851,7 +851,7 @@ SELECT g.* $aclKeys = array_keys($acls); $aclKeys = implode(',', $aclKeys); - $cacheKey = CRM_Core_BAO_Cache::cleanKey("$tableName-$aclKeys"); + $cacheKey = CRM_Utils_Cache::cleanKey("$tableName-$aclKeys"); $cache = CRM_Utils_Cache::singleton(); $ids = $cache->get($cacheKey); if (!$ids) { diff --git a/CRM/Contact/BAO/ContactType.php b/CRM/Contact/BAO/ContactType.php index b0d25258ef..8ee3011f27 100644 --- a/CRM/Contact/BAO/ContactType.php +++ b/CRM/Contact/BAO/ContactType.php @@ -387,7 +387,7 @@ WHERE type.name IS NOT NULL $argString = $all ? 'CRM_CT_GSE_1' : 'CRM_CT_GSE_0'; $argString .= $isSeparator ? '_1' : '_0'; $argString .= $separator; - $argString = CRM_Core_BAO_Cache::cleanKey($argString); + $argString = CRM_Utils_Cache::cleanKey($argString); if (!array_key_exists($argString, $_cache)) { $cache = CRM_Utils_Cache::singleton(); $_cache[$argString] = $cache->get($argString); diff --git a/CRM/Core/BAO/Cache.php b/CRM/Core/BAO/Cache.php index ef2bc60eee..d5b4ade2f2 100644 --- a/CRM/Core/BAO/Cache.php +++ b/CRM/Core/BAO/Cache.php @@ -475,24 +475,11 @@ class CRM_Core_BAO_Cache extends CRM_Core_DAO_Cache { * @return string * Ex: '_abcd1234abcd1234' or 'ab_xx/cd_xxef'. * A similar key, but suitable for use with PSR-16-compliant cache providers. + * @deprecated + * @see CRM_Utils_Cache::cleanKey() */ public static function cleanKey($key) { - if (!is_string($key) && !is_int($key)) { - throw new \RuntimeException("Malformed cache key"); - } - - $maxLen = 64; - $escape = '-'; - - if (strlen($key) >= $maxLen) { - return $escape . md5($key); - } - - $r = preg_replace_callback(';[^A-Za-z0-9_\.];', function($m) use ($escape) { - return $escape . dechex(ord($m[0])); - }, $key); - - return strlen($r) >= $maxLen ? $escape . md5($key) : $r; + return CRM_Utils_Cache::cleanKey($key); } } diff --git a/CRM/Core/BAO/Cache/Psr16.php b/CRM/Core/BAO/Cache/Psr16.php index ecf107831a..efc723a3fa 100644 --- a/CRM/Core/BAO/Cache/Psr16.php +++ b/CRM/Core/BAO/Cache/Psr16.php @@ -95,7 +95,7 @@ class CRM_Core_BAO_Cache_Psr16 { 'path' => $path, ]); } - return self::getGroup($group)->get(CRM_Core_BAO_Cache::cleanKey($path)); + return self::getGroup($group)->get(CRM_Utils_Cache::cleanKey($path)); } /** @@ -138,7 +138,7 @@ class CRM_Core_BAO_Cache_Psr16 { ]); } self::getGroup($group) - ->set(CRM_Core_BAO_Cache::cleanKey($path), $data, self::TTL); + ->set(CRM_Utils_Cache::cleanKey($path), $data, self::TTL); } /** @@ -153,7 +153,7 @@ class CRM_Core_BAO_Cache_Psr16 { // FIXME: Generate a general deprecation notice. if ($path) { - self::getGroup($group)->delete(CRM_Core_BAO_Cache::cleanKey($path)); + self::getGroup($group)->delete(CRM_Utils_Cache::cleanKey($path)); } else { self::getGroup($group)->clear(); diff --git a/CRM/Core/BAO/PrevNextCache.php b/CRM/Core/BAO/PrevNextCache.php index 416709e59a..04c9d5dfd4 100644 --- a/CRM/Core/BAO/PrevNextCache.php +++ b/CRM/Core/BAO/PrevNextCache.php @@ -447,7 +447,7 @@ WHERE c.group_name = %1 AND c.created_date < date_sub( NOW( ), INTERVAL %2 day ) "; $params = [ - 1 => [CRM_Core_BAO_Cache::cleanKey('CiviCRM Search PrevNextCache'), 'String'], + 1 => [CRM_Utils_Cache::cleanKey('CiviCRM Search PrevNextCache'), 'String'], 2 => [$cacheTimeIntervalDays, 'Integer'], ]; CRM_Core_DAO::executeQuery($sql, $params); diff --git a/CRM/Core/PseudoConstant.php b/CRM/Core/PseudoConstant.php index 5df5e7f275..df29c7f3f5 100644 --- a/CRM/Core/PseudoConstant.php +++ b/CRM/Core/PseudoConstant.php @@ -530,7 +530,7 @@ class CRM_Core_PseudoConstant { $key = 'id', $force = NULL ) { - $cacheKey = CRM_Core_BAO_Cache::cleanKey("CRM_PC_{$name}_{$all}_{$key}_{$retrieve}_{$filter}_{$condition}_{$orderby}"); + $cacheKey = CRM_Utils_Cache::cleanKey("CRM_PC_{$name}_{$all}_{$key}_{$retrieve}_{$filter}_{$condition}_{$orderby}"); $cache = CRM_Utils_Cache::singleton(); $var = $cache->get($cacheKey); if ($var !== NULL && empty($force)) { diff --git a/CRM/Utils/Cache.php b/CRM/Utils/Cache.php index 26bdc6c3b7..93e292127f 100644 --- a/CRM/Utils/Cache.php +++ b/CRM/Utils/Cache.php @@ -183,7 +183,7 @@ class CRM_Utils_Cache { $types = (array) $params['type']; if (!empty($params['name'])) { - $params['name'] = CRM_Core_BAO_Cache::cleanKey($params['name']); + $params['name'] = self::cleanKey($params['name']); } foreach ($types as $type) { @@ -220,6 +220,37 @@ class CRM_Utils_Cache { throw new CRM_Core_Exception("Failed to instantiate cache. No supported cache type found. " . print_r($params, 1)); } + /** + * Normalize a cache key. + * + * This bridges an impedance mismatch between our traditional caching + * and PSR-16 -- PSR-16 accepts a narrower range of cache keys. + * + * @param string $key + * Ex: 'ab/cd:ef' + * @return string + * Ex: '_abcd1234abcd1234' or 'ab_xx/cd_xxef'. + * A similar key, but suitable for use with PSR-16-compliant cache providers. + */ + public static function cleanKey($key) { + if (!is_string($key) && !is_int($key)) { + throw new \RuntimeException("Malformed cache key"); + } + + $maxLen = 64; + $escape = '-'; + + if (strlen($key) >= $maxLen) { + return $escape . md5($key); + } + + $r = preg_replace_callback(';[^A-Za-z0-9_\.];', function($m) use ($escape) { + return $escape . dechex(ord($m[0])); + }, $key); + + return strlen($r) >= $maxLen ? $escape . md5($key) : $r; + } + /** * Assert that a key is well-formed. * diff --git a/CRM/Utils/Cache/SqlGroup.php b/CRM/Utils/Cache/SqlGroup.php index fe34b5290c..840886c65e 100644 --- a/CRM/Utils/Cache/SqlGroup.php +++ b/CRM/Utils/Cache/SqlGroup.php @@ -215,7 +215,7 @@ class CRM_Utils_Cache_SqlGroup implements CRM_Utils_Cache_Interface { CRM_Utils_Cache::assertValidKey($key); // If we are triggering a deletion of a prevNextCache key in the civicrm_cache tabl // Alssure that the relevant prev_next_cache values are also removed. - if ($this->group == CRM_Core_BAO_Cache::cleanKey('CiviCRM Search PrevNextCache')) { + if ($this->group == CRM_Utils_Cache::cleanKey('CiviCRM Search PrevNextCache')) { Civi::service('prevnext')->deleteItem(NULL, $key); } CRM_Core_DAO::executeQuery("DELETE FROM {$this->table} WHERE {$this->where($key)}"); @@ -225,7 +225,7 @@ class CRM_Utils_Cache_SqlGroup implements CRM_Utils_Cache_Interface { } public function flush() { - if ($this->group == CRM_Core_BAO_Cache::cleanKey('CiviCRM Search PrevNextCache') && + if ($this->group == CRM_Utils_Cache::cleanKey('CiviCRM Search PrevNextCache') && Civi::service('prevnext') instanceof CRM_Core_PrevNextCache_Sql) { // Use the standard PrevNextCache cleanup function here not just delete from civicrm_cache CRM_Core_BAO_PrevNextCache::cleanupCache(); diff --git a/tests/phpunit/CRM/Core/BAO/CacheTest.php b/tests/phpunit/CRM/Core/BAO/CacheTest.php index 99abb9e649..537556c0e0 100644 --- a/tests/phpunit/CRM/Core/BAO/CacheTest.php +++ b/tests/phpunit/CRM/Core/BAO/CacheTest.php @@ -109,7 +109,7 @@ class CRM_Core_BAO_CacheTest extends CiviUnitTestCase { * @dataProvider getCleanKeyExamples */ public function testCleanKeys($inputKey, $expectKey) { - $actualKey = CRM_Core_BAO_Cache::cleanKey($inputKey); + $actualKey = CRM_Utils_Cache::cleanKey($inputKey); $this->assertEquals($expectKey, $actualKey); }