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';
civicrm · Aug 2, 2019 · 42a9f59 · 42a9f59
1 parent f126dd0
commit 42a9f59
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 54 deletions.
diff --git 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/UF/Page/ProfileEditor.php b/CRM/UF/Page/ProfileEditor.php
@@ -50,7 +50,7 @@ public static function registerProfileScripts() {
             'phoneType' => CRM_Core_PseudoConstant::get('CRM_Core_DAO_Phone', 'phone_type_id'),
           ],
           'initialProfileList' => $ufGroups,
-          'contactSubTypes' => CRM_Contact_BAO_ContactType::subTypes(),
+          'contactTypes' => CRM_Contact_BAO_ContactType::subTypes(),
           'profilePreviewKey' => CRM_Core_Key::get('CRM_UF_Form_Inline_Preview', TRUE),
         ];
       })

diff --git 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
@@ -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
@@ -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));
   }
 
   /**