Merge pull request #793 from DirectoryTree/fix-model-filter-behaviour

Align model query builder filter behaviour with root query builder

Author: stevebauman

src/Query/Builder.php

@@ -17,7 +17,6 @@
 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;
@@ -30,6 +29,7 @@ class Builder
 {
     use BuildsQueries;
     use EscapesValues;
+    use ExtractsNestedFilters;
 
     public const TYPE_SEARCH = 'search';
 
@@ -788,30 +788,6 @@ public function orFilter(Closure $closure): static
         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.
      */

src/Query/ExtractsNestedFilters.php

@@ -0,0 +1,33 @@
+<?php
+
+namespace LdapRecord\Query;
+
+use LdapRecord\Query\Filter\Filter;
+use LdapRecord\Query\Filter\GroupFilter;
+
+trait ExtractsNestedFilters
+{
+    /**
+     * 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;
+    }
+}

src/Query/Model/Builder.php

@@ -13,6 +13,7 @@
 use LdapRecord\Models\Types\ActiveDirectory;
 use LdapRecord\Query\Builder as QueryBuilder;
 use LdapRecord\Query\BuildsQueries;
+use LdapRecord\Query\ExtractsNestedFilters;
 use LdapRecord\Query\Filter\AndGroup;
 use LdapRecord\Query\Filter\Filter;
 use LdapRecord\Query\Filter\Not;
@@ -28,6 +29,7 @@
 class Builder
 {
     use BuildsQueries;
+    use ExtractsNestedFilters;
     use ForwardsCalls;
 
     /**
@@ -728,7 +730,9 @@ public function andFilter(Closure $closure): static
         $query = $this->newNestedModelInstance($closure);
 
         if ($filter = $query->getQuery()->getFilter()) {
-            $this->query->addFilter(new AndGroup($filter));
+            $this->query->addFilter(new AndGroup(
+                ...$this->extractNestedFilters($filter)
+            ), wrap: false);
         }
 
         return $this;
@@ -742,7 +746,9 @@ public function orFilter(Closure $closure): static
         $query = $this->newNestedModelInstance($closure);
 
         if ($filter = $query->getQuery()->getFilter()) {
-            $this->query->addFilter(new OrGroup($filter), 'or');
+            $this->query->addFilter(new OrGroup(
+                ...$this->extractNestedFilters($filter)
+            ), wrap: false);
         }
 
         return $this;

tests/Unit/Models/ModelHasManyTest.php

@@ -227,7 +227,7 @@ public function test_only_related_with_many_relation_object_classes()
     {
         // Scopes are wrapped in their own AndGroup for isolation
         $this->assertEquals(
-            '(&(&(|(objectclass=top)(objectclass=person)(objectclass=organizationalperson)(objectclass=user))(|(objectclass=top)(objectclass=group))))',
+            '(&(|(objectclass=top)(objectclass=person)(objectclass=organizationalperson)(objectclass=user))(|(objectclass=top)(objectclass=group)))',
             (new ModelHasManyStubWithManyRelated)->relation()->onlyRelated()->getQuery()->getUnescapedQuery()
         );
     }

tests/Unit/Models/ModelQueryTest.php

@@ -82,7 +82,7 @@ public function test_new_queries_apply_object_class_scopes()
 
         // Scopes are wrapped in their own AndGroup for isolation
         $this->assertEquals(
-            '(&(&(objectclass=foo)(objectclass=bar)(objectclass=baz)))',
+            '(&(objectclass=foo)(objectclass=bar)(objectclass=baz))',
             ModelWithObjectClassStub::query()->getUnescapedQuery()
         );
     }

tests/Unit/Models/ModelScopeTest.php

@@ -167,7 +167,7 @@ public function test_scopes_remain_enforced_with_complex_queries()
     {
         // The scope (foo=bar) must always be present and enforced at the root AND level
         $this->assertEquals(
-            '(&(|(name=\4a\6f\68\6e)(name=\4a\61\6e\65))(active=\74\72\75\65)(&(foo=bar)))',
+            '(&(&(|(name=\4a\6f\68\6e)(name=\4a\61\6e\65))(active=\74\72\75\65))(&(foo=bar)))',
             ModelWithGlobalScopeTestStub::query()
                 ->where('name', '=', 'John')
                 ->orWhere('name', '=', 'Jane')

tests/Unit/Query/Model/ActiveDirectoryBuilderTest.php

@@ -165,4 +165,28 @@ public function test_add_select_with_variadic_arguments()
         $this->assertContains('mail', $selects);
         $this->assertContains('objectguid', $selects);
     }
+
+    public function test_or_filter_extracts_filters_from_nested_query()
+    {
+        $b = $this->newBuilder();
+
+        $query = $b->orFilter(function ($query) {
+            $query->whereEquals('foo', '1');
+            $query->whereEquals('foo', '2');
+        })->getUnescapedQuery();
+
+        $this->assertEquals('(|(foo=1)(foo=2))', $query);
+    }
+
+    public function test_and_filter_extracts_filters_from_nested_query()
+    {
+        $b = $this->newBuilder();
+
+        $query = $b->andFilter(function ($query) {
+            $query->whereEquals('foo', '1');
+            $query->whereEquals('foo', '2');
+        })->getUnescapedQuery();
+
+        $this->assertEquals('(&(foo=1)(foo=2))', $query);
+    }
 }