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.

diff --git a/CRM/Core/DAO.php b/CRM/Core/DAO.php
index 554d89b11f7ece9875650097e45863f75fa2403e..eb8a7499a36821b07afd11f643d69f4de4afa1e5 100644 (file)
--- 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 a6033fc823a52166ce96f921faada6df753df797..a92c2218cc165f862af02839efbe6c3c76cae760 100644 (file)
--- 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");
   }
 
 }