Fix detection of unaliased identifiers (#72)

Added test for implicit conditions and fix detection of unaliased identifiers in the query
UseMuffin · Feb 4, 2025 · bbdaab7 · bbdaab7
1 parent baaf4d5
commit bbdaab7
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 12 deletions.
diff --git a/src/Model/Behavior/TrashBehavior.php b/src/Model/Behavior/TrashBehavior.php
@@ -6,8 +6,7 @@
 use ArrayObject;
 use Cake\Core\Configure;
 use Cake\Core\Exception\CakeException;
-use Cake\Database\Expression\BetweenExpression;
-use Cake\Database\Expression\ComparisonExpression;
+use Cake\Database\Expression\FieldInterface;
 use Cake\Database\Expression\IdentifierExpression;
 use Cake\Database\Query\SelectQuery;
 use Cake\Datasource\EntityInterface;
@@ -181,41 +180,39 @@ public function beforeFind(EventInterface $event, SelectQuery $query, ArrayObjec
             return;
         }
 
-        $field = $this->getTrashField();
-
-        if ($this->shouldAddTrashCondition($query, $field)) {
-            $query->andWhere([$field . ' IS' => null]);
+        if ($this->shouldAddTrashCondition($query)) {
+            $query->andWhere([$this->getTrashField() . ' IS' => null]);
         }
     }
 
     /**
      * Whether we need to add the trash condition to the query
      *
      * @param \Cake\ORM\Query\SelectQuery $query Query.
-     * @param string $field Trash field
      * @return bool
      */
-    protected function shouldAddTrashCondition(SelectQuery $query, string $field): bool
+    protected function shouldAddTrashCondition(SelectQuery $query): bool
     {
+        $fieldIdentifiers = [$this->getTrashField(false), $this->getTrashField()];
         $addCondition = true;
 
-        $query->traverseExpressions(function ($expression) use (&$addCondition, $field): void {
+        $query->traverseExpressions(function ($expression) use (&$addCondition, $fieldIdentifiers): void {
             if (!$addCondition) {
                 return;
             }
 
             if (
                 $expression instanceof IdentifierExpression
-                && $expression->getIdentifier() === $field
+                && in_array($expression->getIdentifier(), $fieldIdentifiers, true)
             ) {
                 $addCondition = false;
 
                 return;
             }
 
             if (
-                ($expression instanceof ComparisonExpression || $expression instanceof BetweenExpression)
-                && $expression->getField() === $field
+                $expression instanceof FieldInterface
+                && in_array($expression->getField(), $fieldIdentifiers, true)
             ) {
                 $addCondition = false;
             }

diff --git a/tests/TestCase/Model/Behavior/TrashBehaviorTest.php b/tests/TestCase/Model/Behavior/TrashBehaviorTest.php
@@ -361,6 +361,17 @@ public function testTrashNonAccessibleProperty()
         $this->assertCount(3, $this->Articles->find('withTrashed'));
     }
 
+    public function testFindWithImplicitCondition()
+    {
+        $this->assertCount(2, $this->Articles->find()->where([
+            'trashed IS NOT' => null,
+        ]));
+
+        $this->assertCount(2, $this->Articles->find()->where([
+            'Articles.trashed IS NOT' => null,
+        ]));
+    }
+
     /**
      * Test it can find only trashed records.
      *