Skip expensive smarty Processing

On digging into speed & disk use issues I find that significant disk space is being used on smartyProcessing of greetings. The same also seems to be true of display names. This is a first shot across the bows but I'm also investigating 1) also doing an early return before replaceGreetingTemplate 2) changing the smarty ->fetch call to pass 'eval' rather than string. Despite the gut-reaction the docs identify this as correct https://www.smarty.net/docs/en/resources.string.tpl and eval is not eval From my local profiling this code also kicks in on delete & it's likely we will actually see a decrease in overall test time on this
civicrm · Mar 10, 2020 · 7557090 · 7557090
1 parent fff6d9b
commit 7557090
Show file tree

Hide file tree

Showing 4 changed files with 89 additions and 9 deletions.
diff --git a/CRM/Contact/BAO/Contact/Utils.php b/CRM/Contact/BAO/Contact/Utils.php
@@ -1117,7 +1117,10 @@ public static function getTokensRequiredForContactGreetings($contactParams) {
    */
   public static function processGreetingTemplate(&$templateString, $contactDetails, $contactID, $className) {
     CRM_Utils_Token::replaceGreetingTokens($templateString, $contactDetails, $contactID, $className, TRUE);
-
+    if (!CRM_Utils_String::stringContainsTokens($templateString)) {
+      // Skip expensive smarty processing.
+      return;
+    }
     $smarty = CRM_Core_Smarty::singleton();
     $templateString = $smarty->fetch("string:$templateString");
   }

diff --git a/CRM/Utils/String.php b/CRM/Utils/String.php
@@ -971,4 +971,18 @@ public static function pluralize($str) {
     }
   }
 
+  /**
+   * Generic check as to whether any tokens are in the given string.
+   *
+   * It might be a smarty token OR a CiviCRM token. In both cases the
+   * absence of a '{' indicates no token is present.
+   *
+   * @param string $string
+   *
+   * @return bool
+   */
+  public static function stringContainsTokens(string $string) {
+    return strpos($string, '{') !== FALSE;
+  }
+
 }
diff --git a/Civi/Test/Api3TestTrait.php b/Civi/Test/Api3TestTrait.php
@@ -399,6 +399,14 @@ public function runApi4Legacy($v3Entity, $v3Action, $v3Params = []) {
           unset($options['return'][$name]);
         }
       }
+
+      if ($name === 'option_group_id' && isset($v3Params[$name]) && !is_numeric($v3Params[$name])) {
+        // This is a per field hack (bad) but we can't solve everything at once
+        // & a cleverer way turned out to be too much for this round.
+        // Being in the test class it's tested....
+        $v3Params['option_group.name'] = $v3Params['option_group_id'];
+        unset($v3Params['option_group_id']);
+      }
     }
 
     switch ($v3Action) {

diff --git a/tests/phpunit/api/v3/ContactTest.php b/tests/phpunit/api/v3/ContactTest.php
@@ -118,6 +118,8 @@ public function tearDown() {
    * @param int $version
    *
    * @dataProvider versionThreeAndFour
+   *
+   * @throws \CRM_Core_Exception
    */
   public function testAddCreateIndividual($version) {
     $this->_apiversion = $version;
@@ -144,6 +146,8 @@ public function testAddCreateIndividual($version) {
    * Test that it is possible to prevent cache clearing via option.
    *
    * Cache clearing is bypassed if 'options' => array('do_not_reset_cache' => 1 is used.
+   *
+   * @throws \CRM_Core_Exception
    */
   public function testCreateIndividualNoCacheClear() {
 
@@ -4284,28 +4288,50 @@ public function testCreateCommunicationStylePassed() {
 
   /**
    * Test that creating a contact with various contact greetings works.
-   * V3 Only.
+   *
+   * @param int $version
+   *
+   * @dataProvider versionThreeAndFour
+   * @throws \CRM_Core_Exception
    */
-  public function testContactGreetingsCreate() {
+  public function testContactGreetingsCreate($version) {
+    $this->_apiversion = $version;
+    // Api v4 takes a return parameter like postal_greeting_display which matches the field.
+    // v3 has a customised parameter 'postal_greeting'. The v4 parameter is more correct so
+    // we will not change it to match v3. The keyString value allows the test to support both.
+    $keyString = $version === 4 ? '_display' : '';
+
     $contact = $this->callAPISuccess('Contact', 'create', ['first_name' => 'Alan', 'last_name' => 'MouseMouse', 'contact_type' => 'Individual']);
-    $contact = $this->callAPISuccessGetSingle('Contact', ['id' => $contact['id'], 'return' => 'postal_greeting']);
+    $contact = $this->callAPISuccessGetSingle('Contact', ['id' => $contact['id'], 'return' => 'postal_greeting' . $keyString]);
     $this->assertEquals('Dear Alan', $contact['postal_greeting_display']);
 
     $contact = $this->callAPISuccess('Contact', 'create', ['id' => $contact['id'], 'postal_greeting_id' => 2]);
-    $contact = $this->callAPISuccessGetSingle('Contact', ['id' => $contact['id'], 'return' => 'postal_greeting']);
+    $contact = $this->callAPISuccessGetSingle('Contact', ['id' => $contact['id'], 'return' => 'postal_greeting' . $keyString]);
     $this->assertEquals('Dear Alan MouseMouse', $contact['postal_greeting_display']);
 
     $contact = $this->callAPISuccess('Contact', 'create', ['organization_name' => 'Alan\'s Show', 'contact_type' => 'Organization']);
-    $contact = $this->callAPISuccessGetSingle('Contact', ['id' => $contact['id'], 'return' => 'postal_greeting, addressee, email_greeting']);
+    $contact = $this->callAPISuccessGetSingle('Contact', ['id' => $contact['id'], 'return' => "postal_greeting{$keyString}, addressee{$keyString}, email_greeting{$keyString}"]);
     $this->assertEquals('', $contact['postal_greeting_display']);
     $this->assertEquals('', $contact['email_greeting_display']);
     $this->assertEquals('Alan\'s Show', $contact['addressee_display']);
   }
 
   /**
    * Test that creating a contact with various contact greetings works.
+   *
+   * @param int $version
+   *
+   * @dataProvider versionThreeAndFour
+   *
+   * @throws \CRM_Core_Exception
    */
-  public function testContactGreetingsCreateWithCustomField() {
+  public function testContactGreetingsCreateWithCustomField($version) {
+    $this->_apiversion = $version;
+    // Api v4 takes a return parameter like postal_greeting_display which matches the field.
+    // v3 has a customised parameter 'postal_greeting'. The v4 parameter is more correct so
+    // we will not change it to match v3. The keyString value allows the test to support both.
+    $keyString = $version === 4 ? '_display' : '';
+
     $ids = $this->entityCustomGroupWithSingleFieldCreate(__FUNCTION__, __FILE__);
     $contact = $this->callAPISuccess('Contact', 'create', ['first_name' => 'Alan', 'contact_type' => 'Individual', 'custom_' . $ids['custom_field_id'] => 'Mice']);
 
@@ -4319,12 +4345,12 @@ public function testContactGreetingsCreateWithCustomField() {
 
     // Update contact & see if postal greeting now reflects the new string.
     $this->callAPISuccess('Contact', 'create', ['id' => $contact['id'], 'last_name' => 'MouseyMousey']);
-    $contact = $this->callAPISuccessGetSingle('Contact', ['id' => $contact['id'], 'return' => 'postal_greeting']);
+    $contact = $this->callAPISuccessGetSingle('Contact', ['id' => $contact['id'], 'return' => 'postal_greeting' . $keyString]);
     $this->assertEquals('Dear Alan Mice', $contact['postal_greeting_display']);
 
     // Set contact to have no postal greeting & check it is correct.
     $this->callAPISuccess('Contact', 'create', ['id' => $contact['id'], 'postal_greeting_id' => 'null']);
-    $contact = $this->callAPISuccessGetSingle('Contact', ['id' => $contact['id'], 'return' => 'postal_greeting']);
+    $contact = $this->callAPISuccessGetSingle('Contact', ['id' => $contact['id'], 'return' => 'postal_greeting' . $keyString]);
     $this->assertEquals('', $contact['postal_greeting_display']);
 
     //Cleanup
@@ -4333,6 +4359,35 @@ public function testContactGreetingsCreateWithCustomField() {
     $this->customGroupDelete($ids['custom_group_id']);
   }
 
+  /**
+   * Test that smarty variables are parsed if they exist in the greeting template.
+   *
+   * @param int $version
+   *
+   * @dataProvider versionThreeAndFour
+   * @throws \CRM_Core_Exception
+   */
+  public function testGreetingParseSmarty($version) {
+    $this->_apiversion = $version;
+    // Api v4 takes a return parameter like postal_greeting_display which matches the field.
+    // v3 has a customised parameter 'postal_greeting'. The v4 parameter is more correct so
+    // we will not change it to match v3. The keyString value allows the test to support both.
+    $keyString = $version === 4 ? '_display' : '';
+    $postalOption = $this->callAPISuccessGetSingle('OptionValue', ['option_group_id' => 'postal_greeting', 'option_group_id' => 'postal_greeting', 'filter' => 1, 'is_default' => 1]);
+    $this->callAPISuccess('OptionValue', 'create', [
+      'id' => $postalOption['id'],
+      'name' => "Dear {contact.first_name} {if \'{contact.first_name}\' === \'Tim\'}The Wise{/if}",
+      'label' => "Dear {contact.first_name} {if '{contact.first_name}' === 'Tim'} The Wise{/if}",
+    ]);
+    $contactID = $this->individualCreate();
+    $contact = $this->callAPISuccessGetSingle('Contact', ['id' => $contactID, 'return' => 'postal_greeting' . $keyString]);
+    $this->assertEquals('Dear Anthony', $contact['postal_greeting_display']);
+
+    $this->callAPISuccess('Contact', 'create', ['id' => $contactID, 'first_name' => 'Tim']);
+    $contact = $this->callAPISuccessGetSingle('Contact', ['id' => $contactID, 'return' => 'postal_greeting' . $keyString]);
+    $this->assertEquals('Dear Tim The Wise', $contact['postal_greeting_display']);
+  }
+
   /**
    * Test getunique api call for Contact entity
    */