Problem/Motivation

Config entity query is a little confused about case (in)sensitivity. This code should return the 'Content' view (from the standard profile) among the results, but it doesn't.

$query = \Drupal::entityQuery('view');
$query->condition('label', 'c', 'CONTAINS');
$result = $query->execute();
debug($result);

Proposed resolution

I assume that the condition value is lowercased before doing the comparison, but the actual values that it's checked against are not.

Remaining tasks

Investigate, propose a resolution.

User interface changes

None.

API changes

Not sure.

#2068655: Entity fields do not support case sensitive queries

CommentFileSizeAuthor
#4 2107309-4.patch6.9 KBamateescu
#4 interdiff.txt5 KBamateescu
#1 2107309-test-only.patch1.41 KBamateescu
#1 2107309.patch2.89 KBamateescu

Comments

amateescu’s picture

amateescu commented
Status: Active » Needs review
StatusFileSize
new 2107309.patch2.89 KB
new 2107309-test-only.patch1.41 KB

This should do it.

dawehner’s picture

dawehner
Primary language German
commented 
+++ b/core/lib/Drupal/Core/Config/Entity/Query/Condition.php
@@ -33,6 +34,15 @@ public function compile($configs) {
+
+        // Lowercase condition value(s) for case-insensitive matches.
+        if (is_array($condition['value'])) {
+          $condition['value'] = array_map('Drupal\Component\Utility\Unicode::strtolower', $condition['value']);
+        }
+        else {
+          $condition['value'] = Unicode::strtolower($condition['value']);
+        }
+

\Drupal\Core\Entity\QueryInterface::condition should document that the value is case-insensitive.

Status: Needs review » Needs work

The last submitted patch, 2107309.patch, failed testing.

amateescu’s picture

amateescu commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt5 KB
new 2107309-4.patch6.9 KB

Fixed those test fails and documented the case-insensitivity aspect.

damiankloip’s picture

damiankloip commented 
+++ b/core/lib/Drupal/Core/Config/Entity/Query/Condition.php
@@ -33,6 +34,15 @@ public function compile($configs) {
+
+        // Lowercase condition value(s) for case-insensitive matches.
+        if (is_array($condition['value'])) {
+          $condition['value'] = array_map('Drupal\Component\Utility\Unicode::strtolower', $condition['value']);
+        }
+        elseif (!is_bool($condition['value'])) {
+          $condition['value'] = Unicode::strtolower($condition['value']);
+        }
+

@@ -150,6 +160,11 @@ protected function matchArray(array $condition, array $data, array $needs_matchi
+      // We always want a case-insensitive match.
+      if (!is_bool($value)) {
+        $value = Unicode::strtolower($value);
+      }

what do you think about all of this logic living in match? Then it's all in one place.

Apart from that, this looks good to me!

amateescu’s picture

amateescu commented

I deliberately put them in separate places for performance reasons.

The condition array stays the same for the duration of the matching process, so it makes sense to update it as early as possible, while the match() method is invoked multiple times, but not for every property of the config entity, so it doesn't make sense to do the lowercase transformation on all values of the entity.

damiankloip’s picture

damiankloip commented
Status: Needs review » Reviewed & tested by the community

We talked about this on IRC, it makes sense to do the transformation in compile() for sure.

Has tests and the rest of the code looks good , so RTBC.

chx, this should get your attention anyway :)

chx’s picture

chx commented

This will be fine. (It doesn't get my attention. I no longer monitor the RTBC queue.)

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Updated wrong example.