Merge pull request #791 from DirectoryTree/and-or-behaviour

Fix `orFilter/andFilter` behaviour with nested clauses

Author: stevebauman

src/Query/Builder.php

@@ -17,6 +17,7 @@
 use LdapRecord\Query\Filter\AndGroup;
 use LdapRecord\Query\Filter\Factory;
 use LdapRecord\Query\Filter\Filter;
+use LdapRecord\Query\Filter\GroupFilter;
 use LdapRecord\Query\Filter\Not;
 use LdapRecord\Query\Filter\OrGroup;
 use LdapRecord\Query\Filter\Raw;
@@ -763,7 +764,9 @@ public function andFilter(Closure $closure): static
         $query = $this->newNestedInstance($closure);
 
         if ($filter = $query->getFilter()) {
-            $this->addFilter(new AndGroup($filter));
+            $this->addFilter(new AndGroup(
+                ...$this->extractNestedFilters($filter)
+            ), wrap: false);
         }
 
         return $this;
@@ -777,12 +780,38 @@ public function orFilter(Closure $closure): static
         $query = $this->newNestedInstance($closure);
 
         if ($filter = $query->getFilter()) {
-            $this->addFilter(new OrGroup($filter), 'or');
+            $this->addFilter(new OrGroup(
+                ...$this->extractNestedFilters($filter)
+            ), wrap: false);
         }
 
         return $this;
     }
 
+    /**
+     * Extract filters from a nested group filter for re-wrapping, preserving nested groups.
+     *
+     * @return array<Filter>
+     */
+    protected function extractNestedFilters(Filter $filter): array
+    {
+        if (! $filter instanceof GroupFilter) {
+            return [$filter];
+        }
+
+        $children = $filter->getFilters();
+
+        // If any child is a group, preserve the structure
+        foreach ($children as $child) {
+            if ($child instanceof GroupFilter) {
+                return $children;
+            }
+        }
+
+        // All children are non-groups, it's safe to unwrap.
+        return $children;
+    }
+
     /**
      * Add a nested 'not' filter to the query.
      */
@@ -1146,7 +1175,7 @@ public function orWhereNotEndsWith(string $attribute, string $value): static
     /**
      * Add a filter to the query.
      */
-    public function addFilter(Filter $filter, string $boolean = 'and'): static
+    public function addFilter(Filter $filter, string $boolean = 'and', bool $wrap = true): static
     {
         if (is_null($this->filter)) {
             $this->filter = $filter;
@@ -1157,11 +1186,11 @@ public function addFilter(Filter $filter, string $boolean = 'and'): static
         // Flatten same-type groups to avoid deeply nested structures.
         // Ex: AndGroup(AndGroup(a, b), c) becomes AndGroup(a, b, c)
         if ($boolean === 'or') {
-            $this->filter = $this->filter instanceof OrGroup
+            $this->filter = $this->filter instanceof OrGroup && $wrap
                 ? new OrGroup(...[...$this->filter->getFilters(), $filter])
                 : new OrGroup($this->filter, $filter);
         } else {
-            $this->filter = $this->filter instanceof AndGroup
+            $this->filter = $this->filter instanceof AndGroup && $wrap
                 ? new AndGroup(...[...$this->filter->getFilters(), $filter])
                 : new AndGroup($this->filter, $filter);
         }

tests/Unit/Query/BuilderTest.php

@@ -762,31 +762,43 @@ public function test_nested_or_filter()
     {
         $b = $this->newBuilder();
 
-        // orFilter wraps the nested query in an OrGroup
-        // Since where() creates an AndGroup, the result is OrGroup(AndGroup(...))
+        // orFilter combines the filters inside with OR
         $query = $b->orFilter(function ($query) {
             $query->where([
                 'one' => 'one',
                 'two' => 'two',
             ]);
         })->getUnescapedQuery();
 
-        $this->assertEquals('(|(&(one=one)(two=two)))', $query);
+        $this->assertEquals('(|(one=one)(two=two))', $query);
+    }
+
+    public function test_nested_or_filter_with_where_equals()
+    {
+        $b = $this->newBuilder();
+
+        // orFilter with whereEquals should combine with OR, not AND
+        $query = $b->orFilter(function ($query) {
+            $query->whereEquals('foo', '1');
+            $query->whereEquals('foo', '2');
+        })->getUnescapedQuery();
+
+        $this->assertEquals('(|(foo=1)(foo=2))', $query);
     }
 
     public function test_nested_and_filter()
     {
         $b = $this->newBuilder();
 
-        // andFilter wraps the nested query in an AndGroup
+        // andFilter combines the filters inside with AND
         $query = $b->andFilter(function ($query) {
             $query->where([
                 'one' => 'one',
                 'two' => 'two',
             ]);
         })->getUnescapedQuery();
 
-        $this->assertEquals('(&(&(one=one)(two=two)))', $query);
+        $this->assertEquals('(&(one=one)(two=two))', $query);
     }
 
     public function test_nested_not_filter()
@@ -820,7 +832,7 @@ public function test_nested_filters()
             ]);
         })->getUnescapedQuery();
 
-        $this->assertEquals('(&(|(&(one=one)(two=two)))(&(&(one=one)(two=two))))', $query);
+        $this->assertEquals('(&(|(one=one)(two=two))(&(one=one)(two=two)))', $query);
     }
 
     public function test_nested_filters_with_non_nested()
@@ -842,7 +854,32 @@ public function test_nested_filters_with_non_nested()
             'six' => 'six',
         ])->getUnescapedQuery();
 
-        $this->assertEquals('(&(|(&(one=one)(two=two)))(&(&(three=three)(four=four)))(five=five)(six=six))', $query);
+        $this->assertEquals('(&(|(one=one)(two=two))(&(three=three)(four=four))(five=five)(six=six))', $query);
+    }
+
+    public function test_deeply_nested_filters()
+    {
+        $b = $this->newBuilder();
+
+        // Complex query: enabled AND (user with verified OR service with trusted)
+        $query = $b->andFilter(function ($query) {
+            $query->whereEquals('enabled', 'true');
+            $query->orFilter(function ($q) {
+                $q->andFilter(function ($inner) {
+                    $inner->whereEquals('type', 'user');
+                    $inner->whereEquals('verified', 'true');
+                });
+                $q->andFilter(function ($inner) {
+                    $inner->whereEquals('type', 'service');
+                    $inner->whereEquals('trusted', 'true');
+                });
+            });
+        })->getUnescapedQuery();
+
+        $this->assertEquals(
+            '(&(enabled=true)(|(&(type=user)(verified=true))(&(type=service)(trusted=true))))',
+            $query
+        );
     }
 
     public function test_nested_builder_is_nested()