From ec64610cde0337aad3da6bad77a6c84cfe36f0fe Mon Sep 17 00:00:00 2001 From: Bob Silvern Date: Tue, 28 Nov 2023 14:39:06 -0800 Subject: [PATCH] dev/core#4761 - CRM\Core\DAO - Generate unique name, case-insensitve This duplicates on the 5.68 branch changes made in PR-28168 to master. --- CRM/Core/DAO.php | 89 ++++++++++++++----- .../v4/SearchDisplay/SearchDisplayTest.php | 21 +++-- 2 files changed, 81 insertions(+), 29 deletions(-) diff --git a/CRM/Core/DAO.php b/CRM/Core/DAO.php index 554d89b11f..eb8a7499a3 100644 --- a/CRM/Core/DAO.php +++ b/CRM/Core/DAO.php @@ -3412,27 +3412,76 @@ SELECT contact_id // No label supplied, do nothing return; } + + // Strip unsafe characters and trim to max length, allowing room for a + // unique suffix composed of an underscore + 4 alphanumeric chars, + // supporting up to 36^4=1,679,616 unique names for any given value of + // $label. Half that amount could be considered the working limit, as + // much above that the time to find a non-existent suffix becomes + // unacceptable. + $maxSuffixLen = 5; $maxLen = static::getSupportedFields()['name']['maxlength'] ?? 255; - // Strip unsafe characters and trim to max length (-3 characters leaves room for a unique suffix) - $name = CRM_Utils_String::munge($label, '_', $maxLen - 3); - - // Find existing records with the same name - $sql = new CRM_Utils_SQL_Select($this::getTableName()); - $sql->select(['id', 'name']); - $sql->where('name LIKE @name', ['@name' => $name . '%']); - // Include all fields that are part of the index - foreach (array_diff($indexNameWith, ['name']) as $field) { - $sql->where("`$field` = @val", ['@val' => $this->$field]); - } - $query = $sql->toSQL(); - $existing = self::executeQuery($query)->fetchMap('id', 'name'); - $dupes = 0; - $suffix = ''; - // Add unique suffix if existing records have the same name - while (in_array($name . $suffix, $existing)) { - $suffix = '_' . ++$dupes; - } - $this->name = $name . $suffix; + $name = CRM_Utils_String::munge($label, '_', $maxLen - $maxSuffixLen); + + // Define an arbitrary limit on how many guesses we will perform before + // throwing an exception. This would occur only in some unanticipated use + // case. + $max_guesses = 36 ^ ($maxSuffixLen - 1); + + $guesses_per_loop = 5; + $guess_count = 0; + + do { + // Make an initial attempt to guess a unique name by searching for + // 5 candidates (the original $name plus $name with 4 random suffixes). + // If all of these happen to exist in the table, we'll keep trying, + // doubling the number of guesses each time through the loop. + for ($i = 0; $i < $guesses_per_loop; $i++, $guess_count++) { + $suffix = $guess_count == 0 ? '' : + '_' . CRM_Utils_String::createRandom($maxSuffixLen - 1, 'abcdefghijklmnopqrstuvwxyz0123456789'); + $candidates[$i] = $name . $suffix; + } + + $sql = new CRM_Utils_SQL_Select($this::getTableName()); + $sql->select(['id', 'LOWER(name) name_lc']); + $sql->where('name IN (@candidates)', ['@candidates' => $candidates]); + + // Narrow the search by specifying the value of any additional fields + // that may be part of the index. + foreach (array_diff($indexNameWith, ['name']) as $field) { + $sql->where("`$field` = @val", ['@val' => $this->$field]); + } + $query = $sql->toSQL(); + + // Search the table for our candidates using case-sensitivity determined + // by the collation of the name column -- case-insensitive by default. + // Array $existing_lc will contains all the candidates found in the table, + // converted to lower-case. + $existing_lc = self::executeQuery($query)->fetchMap('id', 'name_lc'); + + if (count($existing_lc) < $guesses_per_loop) { + // Not all of our candidates were found in the table, so we'll search + // for the first element of $candidates that wasn't found. This search + // is performed case-insensitive to ensure that the selected candidate + // is unique with both ci and cs collation of the name column. If the + // original (unsuffixed) value of $name doesn't exist in the table, then + // that value will be our selected candidate. + foreach ($candidates as $c) { + if (!in_array(strtolower($c), $existing_lc)) { + $this->name = $c; + return; + } + } + } + else { + // All candidates were found in the table. Try harder next time. + $guesses_per_loop = min(1000, $guesses_per_loop * 2); + + if ($guess_count > $max_guesses) { + throw new CRM_Core_Exception("CRM_Core_DAO::makeNameFromLabel failed to generate a unique name for label $label."); + } + } + } while (1); } /** diff --git a/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchDisplayTest.php b/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchDisplayTest.php index a6033fc823..a92c2218cc 100644 --- a/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchDisplayTest.php +++ b/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchDisplayTest.php @@ -74,10 +74,12 @@ class SearchDisplayTest extends \PHPUnit\Framework\TestCase implements HeadlessI ->addValue('label', 'My test search') ->execute()->first(); // Name will be created from munged label - $this->assertEquals('My_test_search', $savedSearch0['name']); - // Name will have _1, _2, etc. appended to ensure it's unique - $this->assertEquals('My_test_search_1', $savedSearch1['name']); - $this->assertEquals('My_test_search_2', $savedSearch2['name']); + $this->assertEquals('My_test_search', $savedSearch0['name'], "SavedSearch 0"); + // Name will have _r appended to ensure it's unique, where r is a string of + // random chars. + $this->assertEquals('My_test_search_', substr($savedSearch1['name'], 0, 15), "SavedSearch 1"); + $this->assertEquals('My_test_search_', substr($savedSearch2['name'], 0, 15), "SavedSearch 2"); + $this->assertNotSame($savedSearch1['name'], $savedSearch2['name'], "SavedSearch 1,2"); $display0 = SearchDisplay::create() ->addValue('saved_search_id', $savedSearch0['id']) @@ -95,11 +97,12 @@ class SearchDisplayTest extends \PHPUnit\Framework\TestCase implements HeadlessI ->addValue('type', 'table') ->execute()->first(); // Name will be created from munged label - $this->assertEquals('My_test_display', $display0['name']); - // Name will have _1 appended to ensure it's unique to savedSearch0 - $this->assertEquals('My_test_display_1', $display1['name']); - // This is for a different saved search so doesn't need a number appended - $this->assertEquals('My_test_display', $display2['name']); + $this->assertEquals('My_test_display', $display0['name'], "SearchDisplay 0"); + // Name will have _r appended (r is random string) to ensure it's unique to + // savedSearch0. + $this->assertEquals('My_test_display_', substr($display1['name'], 0, 16), "SearchDisplay 1"); + // This is for a different saved search so doesn't need a suffix appended + $this->assertEquals('My_test_display', $display2['name'], "SearchDisplay 2"); } } -- 2.25.1