diff --git a/src/Query/Builder.php b/src/Query/Builder.php index b6b951f3..bf52fc5b 100644 --- a/src/Query/Builder.php +++ b/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 - */ - 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. */ diff --git a/src/Query/ExtractsNestedFilters.php b/src/Query/ExtractsNestedFilters.php new file mode 100644 index 00000000..e2781303 --- /dev/null +++ b/src/Query/ExtractsNestedFilters.php @@ -0,0 +1,33 @@ + + */ + 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; + } +} diff --git a/src/Query/Model/Builder.php b/src/Query/Model/Builder.php index eefe8f1e..c9c71ba4 100644 --- a/src/Query/Model/Builder.php +++ b/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; diff --git a/tests/Unit/Models/ModelHasManyTest.php b/tests/Unit/Models/ModelHasManyTest.php index fee64350..45ad08ac 100644 --- a/tests/Unit/Models/ModelHasManyTest.php +++ b/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() ); } diff --git a/tests/Unit/Models/ModelQueryTest.php b/tests/Unit/Models/ModelQueryTest.php index d231b558..2d7350da 100644 --- a/tests/Unit/Models/ModelQueryTest.php +++ b/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() ); } diff --git a/tests/Unit/Models/ModelScopeTest.php b/tests/Unit/Models/ModelScopeTest.php index 77c2b139..1d1ebcf9 100644 --- a/tests/Unit/Models/ModelScopeTest.php +++ b/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') diff --git a/tests/Unit/Query/Model/ActiveDirectoryBuilderTest.php b/tests/Unit/Query/Model/ActiveDirectoryBuilderTest.php index 159e3009..8764118c 100644 --- a/tests/Unit/Query/Model/ActiveDirectoryBuilderTest.php +++ b/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); + } }