From 1441a37891d261f250ae7d79048d3a4e09b8881a Mon Sep 17 00:00:00 2001 From: eileen Date: Tue, 10 Mar 2020 14:13:44 +1300 Subject: [PATCH] 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 --- CRM/Contact/BAO/Contact/Utils.php | 5 +- CRM/Utils/String.php | 14 ++++++ Civi/Test/Api3TestTrait.php | 8 +++ tests/phpunit/api/v3/ContactTest.php | 73 +++++++++++++++++++++++++--- 4 files changed, 91 insertions(+), 9 deletions(-) diff --git a/CRM/Contact/BAO/Contact/Utils.php b/CRM/Contact/BAO/Contact/Utils.php index 2e43667b7325..b32745f6706d 100644 --- 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 index 1c878580af9a..9b2652a28a92 100644 --- 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 index 79e6e8e53cd7..f42e7f3b493b 100644 --- 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 index aa64a3b52905..aa3ad5573ada 100644 --- 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,19 +4288,29 @@ 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']); @@ -4304,8 +4318,20 @@ public function testContactGreetingsCreate() { /** * 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,37 @@ public function testContactGreetingsCreateWithCustomField() { $this->customGroupDelete($ids['custom_group_id']); } + /** + * Test that smarty variables are parsed if they exist in the greeting template. + * + * In this test we have both a Civi token & a Smarty token and we check both are processed. + * + * @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', '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 */