Issue #3409401 by Ihor Ukhan: Drupal\views\Plugin\views\HandlerBase::breakString works unexpectedly for a views with a huge amount of filters/parameters in a view.
Open
issue/drupal-3409401:3409401-drupalviewspluginviewshandlerbasebreakstring-works-unexpectedly into 11.x
2 open threads
Closes #3409401
Merge request reports
Activity
added 124 commits
-
883b37ec...69dc12e1 - 119 commits from branch
project:11.x - 80f04dcf - Issue #3409401 by Ihor Ukhan:...
- 6f290d9e - Issues/3409401: Add tests.
- 45ca5b23 - Issues/3409401: Add tests.
- 4b1f0cec - Added regex test from #8.
- 0ef55cdf - Issue #3409401: Addressed feedback on comments & Simplified the logic to handle empty strings
Toggle commit list-
883b37ec...69dc12e1 - 119 commits from branch
839 if (!strpos($str, '+') && !strpos($str, ',') && !strpos($str, ' ')) { 840 $value = [$str]; 841 // Default to 'and' for a single word. 842 $operator = 'and'; 843 } 844 // Check for 'and' operators (commas). 845 elseif (str_contains($str, ',')) { 846 $operator = 'and'; 847 $value = explode(',', $str); 848 } 849 // Check for 'or' operators (plus signs or spaces). 850 elseif (str_contains($str, '+') || str_contains($str, ' ')) { 851 $operator = 'or'; 852 // Replace plus signs with spaces and split. 853 $value = explode(' ', str_replace('+', ' ', $str)); 854 } - Comment on lines +837 to +854
I think there are some subtle ways in which this code is different from the preg version in HEAD.
- If $str is just a , or + or leads with a space
- the , and + / space order is swapped.
How about doing something like this:
830 if (trim($str) !== '') { 831 // Check if the string has only one word without any delimiters. 832 if (!strpos($str, '+') && !strpos($str, ',') && !strpos($str, ' ')) { 833 $value = [$str]; 834 // Default to 'and' for a single word. 835 $operator = 'and'; 836 } 837 // Check for 'and' operators (commas). 838 elseif (str_contains($str, ',')) { 839 $operator = 'and'; 840 $value = explode(',', $str); 841 } 842 // Check for 'or' operators (plus signs or spaces). 843 elseif (str_contains($str, '+') || str_contains($str, ' ')) { 844 $operator = 'or'; 845 // Replace plus signs with spaces and split. 846 $value = explode(' ', str_replace('+', ' ', $str)); 847 } 830 // Remove + and , characters from start and end of string. 831 $str = trim($str, " \n\r\t\v\0,+"); 832 if ($str !== '') { 833 // Check for 'or' operators (plus signs or spaces). 834 if (strpos($str, '+') || strpos($str, ' ')) { 835 $operator = 'or'; 836 // Replace plus signs with spaces and split. 837 $value = explode(' ', str_replace('+', ' ', $str)); 838 } 839 // Check for 'and' operators (commas) or if the string is one word. 840 else { 841 $operator = 'and'; 842 $value = explode(',', $str); 843 } It's less code and more efficient because you're doing less searching through the string. Plus it more closely matches the code in HEAD.
Edited by Alex Pott 
Thank you for the detailed feedback. I have tested the optimised code across all scenarios, and it is working as expected. However, I encountered one test failure. Upon further investigation, I found that when both space/plus and comma delimiters are present in the string (e.g., "word1 word2,word"), the first condition gets triggered, which causes the test to break. I will review this further and make necessary adjustments.
added 185 commits
-
0ef55cdf...4761995e - 180 commits from branch
project:11.x - 514134f8 - Issue #3409401 by Ihor Ukhan:...
- 977bba25 - Issues/3409401: Add tests.
- 72a1554a - Issues/3409401: Add tests.
- bc4c9985 - Added regex test from #8.
- 4f751274 - Issue #3409401: Addressed feedback on comments & Simplified the logic to handle empty strings
Toggle commit list-
0ef55cdf...4761995e - 180 commits from branch
added 269 commits
-
b778777f...c742c881 - 261 commits from branch
project:11.x - 56ba8787 - Issue #3409401 by Ihor Ukhan:...
- 89999420 - Issues/3409401: Add tests.
- 3c40a45a - Issues/3409401: Add tests.
- 4829a46f - Added regex test from #8.
- 7e78a154 - Issue #3409401: Addressed feedback on comments & Simplified the logic to handle empty strings
- 11f9934e - Addressed feedback
- 9c37cfb3 - Fix lint
- 4883b91c - Addressed feedback
Toggle commit list-
b778777f...c742c881 - 261 commits from branch
844 844 $operator = NULL; 845 845 $value = []; 846 846 847 // Determine if the string has 'or' operators (plus signs) or 'and' 848 // operators (commas) and split the string accordingly. 849 if (preg_match('/^([\w0-9-_\.]+[+ ]+)+[\w0-9-_\.]+$/u', $str)) { 850 // The '+' character in a query string may be parsed as ' '. 847 // Remove whitespace, + and , characters from start and end of string. -
Yes, this should have a comment.
// Regular expressions must not be used to divide the string. Some strings, // such as long strings, will fail. This is because the strings exceeds the // default PHP limits for PCRE function. See // https://www.php.net/manual/en/pcre.configuration.phpSomething like this would do.
Please register or sign in to reply