From 81c4c1480428c30469fbfa8d94bd403be97f8503 Mon Sep 17 00:00:00 2001 From: eileen Date: Wed, 3 Jul 2019 13:20:52 +1200 Subject: [PATCH] Rewrite subTypeInfo to use caching mechanism This seems to be getting a lot of cache misses on prod - presumably due to a lack of sub types so fixing to use 'modern caching'; --- CRM/Contact/BAO/ContactType.php | 75 +++++++------------ CRM/Utils/System.php | 1 + Civi/Core/Container.php | 3 +- .../BAO/ContactType/ContactTypeTest.php | 10 +-- 4 files changed, 36 insertions(+), 53 deletions(-) diff --git a/CRM/Contact/BAO/ContactType.php b/CRM/Contact/BAO/ContactType.php index 8ee3011f2779..a4d7cc6af43f 100644 --- a/CRM/Contact/BAO/ContactType.php +++ b/CRM/Contact/BAO/ContactType.php @@ -149,65 +149,45 @@ public static function basicTypePairs($all = FALSE, $key = 'name') { * .. * @param bool $all * @param bool $ignoreCache - * @param bool $reset * * @return array * Array of sub type information */ - public static function subTypeInfo($contactType = NULL, $all = FALSE, $ignoreCache = FALSE, $reset = FALSE) { - static $_cache = NULL; - - if ($reset === TRUE) { - $_cache = NULL; - } - - if ($_cache === NULL) { - $_cache = []; - } - if ($contactType && !is_array($contactType)) { - $contactType = [$contactType]; - } - + public static function subTypeInfo($contactType = NULL, $all = FALSE, $ignoreCache = FALSE) { $argString = $all ? 'CRM_CT_STI_1_' : 'CRM_CT_STI_0_'; if (!empty($contactType)) { + $contactType = (array) $contactType; $argString .= implode('_', $contactType); } + if (!Civi::cache('contactTypes')->has($argString) || $ignoreCache) { + $ctWHERE = ''; + if (!empty($contactType)) { + $ctWHERE = " AND parent.name IN ('" . implode("','", $contactType) . "')"; + } - if ((!array_key_exists($argString, $_cache)) || $ignoreCache) { - $cache = CRM_Utils_Cache::singleton(); - $_cache[$argString] = $cache->get($argString); - if (!$_cache[$argString] || $ignoreCache) { - $_cache[$argString] = []; - - $ctWHERE = ''; - if (!empty($contactType)) { - $ctWHERE = " AND parent.name IN ('" . implode("','", $contactType) . "')"; - } - - $sql = " + $sql = " SELECT subtype.*, parent.name as parent, parent.label as parent_label FROM civicrm_contact_type subtype INNER JOIN civicrm_contact_type parent ON subtype.parent_id = parent.id WHERE subtype.name IS NOT NULL AND subtype.parent_id IS NOT NULL {$ctWHERE} "; - if ($all === FALSE) { - $sql .= " AND subtype.is_active = 1 AND parent.is_active = 1 ORDER BY parent.id"; - } - $dao = CRM_Core_DAO::executeQuery($sql, [], - FALSE, 'CRM_Contact_DAO_ContactType' - ); - while ($dao->fetch()) { - $value = []; - CRM_Core_DAO::storeValues($dao, $value); - $value['parent'] = $dao->parent; - $value['parent_label'] = $dao->parent_label; - $_cache[$argString][$dao->name] = $value; - } - - $cache->set($argString, $_cache[$argString]); + if ($all === FALSE) { + $sql .= " AND subtype.is_active = 1 AND parent.is_active = 1 ORDER BY parent.id"; } + $dao = CRM_Core_DAO::executeQuery($sql, [], + FALSE, 'CRM_Contact_DAO_ContactType' + ); + $values = []; + while ($dao->fetch()) { + $value = []; + CRM_Core_DAO::storeValues($dao, $value); + $value['parent'] = $dao->parent; + $value['parent_label'] = $dao->parent_label; + $values[$dao->name] = $value; + } + Civi::cache('contactTypes')->set($argString, $values); } - return $_cache[$argString]; + return Civi::cache('contactTypes')->get($argString); } /** @@ -378,6 +358,7 @@ public static function getSelectElements( $isSeparator = TRUE, $separator = '__' ) { + // @todo - use Cache class - ie like Civi::cache('contactTypes') static $_cache = NULL; if ($_cache === NULL) { @@ -467,6 +448,7 @@ public static function isaSubType($subType, $ignoreCache = FALSE) { * basicTypes. */ public static function getBasicType($subType) { + // @todo - use Cache class - ie like Civi::cache('contactTypes') static $_cache = NULL; if ($_cache === NULL) { $_cache = []; @@ -617,8 +599,9 @@ public static function del($contactTypeId) { FROM civicrm_navigation WHERE name = %1"; $params = [1 => ["New $name", 'String']]; - $dao = CRM_Core_DAO::executeQuery($sql, $params); + CRM_Core_DAO::executeQuery($sql, $params); CRM_Core_BAO_Navigation::resetNavigation(); + Civi::cache('contactTypes')->clear(); } return TRUE; } @@ -680,9 +663,7 @@ public static function add(&$params) { CRM_Core_BAO_Navigation::add($navigation); } CRM_Core_BAO_Navigation::resetNavigation(); - - // reset the cache after adding - self::subTypeInfo(NULL, FALSE, FALSE, TRUE); + Civi::cache('contactTypes')->clear(); return $contactType; } diff --git a/CRM/Utils/System.php b/CRM/Utils/System.php index 622e1aae9acc..53ccf5f2c6f5 100644 --- a/CRM/Utils/System.php +++ b/CRM/Utils/System.php @@ -1442,6 +1442,7 @@ public static function flushCache() { Civi::cache('groups')->flush(); Civi::cache('navigation')->flush(); Civi::cache('customData')->flush(); + Civi::cache('contactTypes')->clear(); CRM_Extension_System::singleton()->getCache()->flush(); CRM_Cxn_CiviCxnHttp::singleton()->getCache()->flush(); } diff --git a/Civi/Core/Container.php b/Civi/Core/Container.php index 9c78f0b3126c..0c67eb5a5373 100644 --- a/Civi/Core/Container.php +++ b/Civi/Core/Container.php @@ -159,6 +159,7 @@ public function createContainer() { 'navigation' => 'navigation', 'customData' => 'custom data', 'fields' => 'contact fields', + 'contactTypes' => 'contactTypes', ]; foreach ($basicCaches as $cacheSvc => $cacheGrp) { $definitionParams = [ @@ -168,7 +169,7 @@ public function createContainer() { // For Caches that we don't really care about the ttl for and/or maybe accessed // fairly often we use the fastArrayDecorator which improves reads and writes, these // caches should also not have concurrency risk. - $fastArrayCaches = ['groups', 'navigation', 'customData', 'fields']; + $fastArrayCaches = ['groups', 'navigation', 'customData', 'fields', 'contactTypes']; if (in_array($cacheSvc, $fastArrayCaches)) { $definitionParams['withArray'] = 'fast'; } diff --git a/tests/phpunit/CRM/Contact/BAO/ContactType/ContactTypeTest.php b/tests/phpunit/CRM/Contact/BAO/ContactType/ContactTypeTest.php index bb77e8b667ee..b6af1999903e 100644 --- a/tests/phpunit/CRM/Contact/BAO/ContactType/ContactTypeTest.php +++ b/tests/phpunit/CRM/Contact/BAO/ContactType/ContactTypeTest.php @@ -200,8 +200,7 @@ public function testAddInvalid3() { } /** - * Test del() with valid data - * success expected + * Test del() with valid data. */ public function testDel() { @@ -212,11 +211,12 @@ public function testDel() { 'is_active' => 1, ]; $subtype = CRM_Contact_BAO_ContactType::add($params); + $result = CRM_Contact_BAO_ContactType::subTypes(); + $this->assertEquals(TRUE, in_array($subtype->name, $result, TRUE)); + $this->callAPISuccess('ContactType', 'delete', ['id' => $subtype->id]); - $del = CRM_Contact_BAO_ContactType::del($subtype->id); $result = CRM_Contact_BAO_ContactType::subTypes(); - $this->assertEquals($del, TRUE); - $this->assertEquals(in_array($subtype->name, $result), TRUE); + $this->assertEquals(FALSE, in_array($subtype->name, $result, TRUE)); } /**