Skip to content
This repository was archived by the owner on Aug 22, 2023. It is now read-only.

PHPORM-49 Implement Query\Builder::whereNot by encapsulating into $not #13

Merged
merged 6 commits into from
Jul 19, 2023
Merged

PHPORM-49 Implement Query\Builder::whereNot by encapsulating into $not #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3d269ef
PHPORM-49 Implement whereNot by encapsulating into $not
GromNaN Jul 13, 2023
70022ab
Fix nested "or not"
GromNaN Jul 13, 2023
e6cba45
Improve comment
GromNaN Jul 17, 2023
df20bb6
Apply suggestions from code review
GromNaN Jul 19, 2023
f732731
Review
GromNaN Jul 19, 2023
a188374
Update docs
GromNaN Jul 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,17 @@ All notable changes to this project will be documented in this file.

## [Unreleased]

- Add classes to cast ObjectId and UUID instances [#1](https://github.com/GromNaN/laravel-mongodb-private/pull/1) by [@alcaeus](https://github.com/alcaeus).
- Add `Query\Builder::toMql()` to simplify comprehensive query tests [#6](https://github.com/GromNaN/laravel-mongodb-private/pull/6) by [@GromNaN](https://github.com/GromNaN).
- Fix `Query\Builder::whereNot` to use MongoDB [`$not`](https://www.mongodb.com/docs/manual/reference/operator/query/not/) operator [#13](https://github.com/GromNaN/laravel-mongodb-private/pull/13) by [@GromNaN](https://github.com/GromNaN).
- Fix `Query\Builder::whereBetween` to accept `Carbon\Period` object [#10](https://github.com/GromNaN/laravel-mongodb-private/pull/10) by [@GromNaN](https://github.com/GromNaN).
- Throw an exception for unsupported `Query\Builder` methods [#9](https://github.com/GromNaN/laravel-mongodb-private/pull/9) by [@GromNaN](https://github.com/GromNaN).
- Throw an exception when `Query\Builder::orderBy()` is used with invalid direction [#7](https://github.com/GromNaN/laravel-mongodb-private/pull/7) by [@GromNaN](https://github.com/GromNaN).
- Throw an exception when `Query\Builder::push()` is used incorrectly [#5](https://github.com/GromNaN/laravel-mongodb-private/pull/5) by [@GromNaN](https://github.com/GromNaN).

## [3.9.2] - 2022-09-01

### Addded
### Added
- Add single word name mutators [#2438](https://github.com/jenssegers/laravel-mongodb/pull/2438) by [@RosemaryOrchard](https://github.com/RosemaryOrchard) & [@mrneatly](https://github.com/mrneatly).

### Fixed
Expand Down
6 changes: 6 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,12 @@ $users =
->get();
```

**NOT statements**

```php
$users = User::whereNot('age', '>', 18)->get();
```

**whereIn**

```php
Expand Down
19 changes: 13 additions & 6 deletions src/Query/Builder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1019,19 +1019,26 @@ protected function compileWheres(): array
}
}

// The next item in a "chain" of wheres devices the boolean of the
// first item. So if we see that there are multiple wheres, we will
// use the operator of the next where.
if ($i == 0 && count($wheres) > 1 && $where['boolean'] == 'and') {
$where['boolean'] = $wheres[$i + 1]['boolean'];
// In a sequence of "where" clauses, the logical operator of the
// first "where" is determined by the 2nd "where".
// $where['boolean'] = "and", "or", "and not" or "or not"
if ($i == 0 && count($wheres) > 1
&& str_starts_with($where['boolean'], 'and')
&& str_starts_with($wheres[$i + 1]['boolean'], 'or')
) {
$where['boolean'] = 'or'.(str_ends_with($where['boolean'], 'not') ? ' not' : '');
Copy link
Owner Author

@GromNaN GromNaN Jul 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$where['boolean'] can be and, or, and not or or not. It is used as-this into SQL queries. I tried several implementations to split into vars that are easier to use later, but using str_ functions is the simplest.

}

// We use different methods to compile different wheres.
$method = "compileWhere{$where['type']}";
$result = $this->{$method}($where);

if (str_ends_with($where['boolean'], 'not')) {
$result = ['$not' => $result];
}

// Wrap the where with an $or operator.
if ($where['boolean'] == 'or') {
if (str_starts_with($where['boolean'], 'or')) {
$result = ['$or' => [$result]];
}

Expand Down
182 changes: 182 additions & 0 deletions tests/Query/BuilderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,17 @@ public static function provideQueryBuilderToMql(): iterable
fn (Builder $builder) => $builder->where('foo', 'bar'),
];

yield 'where with single array of conditions' => [
['find' => [
['$and' => [
['foo' => 1],
['bar' => 2],
]],
[], // options
]],
fn (Builder $builder) => $builder->where(['foo' => 1, 'bar' => 2]),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beyond the scope of this PR I'm sure, but I don't understand why this results in an $and with separate conditions instead of ['find' => ['foo' => 1, 'bar' => 2]]. Something else to address in query builder optimization follow-up work I suppose.

];

yield 'find > date' => [
['find' => [['foo' => ['$gt' => new UTCDateTime($date)]], []]],
fn (Builder $builder) => $builder->where('foo', '>', $date),
Expand All @@ -65,6 +76,177 @@ public static function provideQueryBuilderToMql(): iterable
fn (Builder $builder) => $builder->limit(10)->offset(5)->select('foo', 'bar'),
];

/** @see DatabaseQueryBuilderTest::testBasicWhereNot() */
yield 'whereNot (multiple)' => [
['find' => [
['$and' => [
['$not' => ['name' => 'foo']],
['$not' => ['name' => ['$ne' => 'bar']]],
]],
[], // options
]],
fn (Builder $builder) => $builder
->whereNot('name', 'foo')
->whereNot('name', '<>', 'bar'),
Comment on lines +89 to +90
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is likely beyond the scope of this PR, but there's room for improvement here. Rather than unconditionally wrap the query with $not, we could just use a more appropriate operator. For instance:

  • where() assumes an equality match, so whereNot() could use $ne instead of a direct value match (as we have here) or $eq.
  • whereNot() with a <> operator could utilize $eq or a direct value match instead.

If you agree, perhaps we can track this in a new PHPORM ticket. I wouldn't limit the query optimization to $not. It should probably be a much larger task to scope out for the entire query builder.

];

/** @see DatabaseQueryBuilderTest::testBasicOrWheres() */
yield 'where orWhere' => [
['find' => [
['$or' => [
['id' => 1],
['email' => 'foo'],
]],
[], // options
]],
fn (Builder $builder) => $builder
->where('id', '=', 1)
->orWhere('email', '=', 'foo'),
];

/** @see DatabaseQueryBuilderTest::testBasicOrWhereNot() */
yield 'orWhereNot' => [
['find' => [
['$or' => [
['$not' => ['name' => 'foo']],
['$not' => ['name' => ['$ne' => 'bar']]],
]],
[], // options
]],
fn (Builder $builder) => $builder
->orWhereNot('name', 'foo')
->orWhereNot('name', '<>', 'bar'),
];

yield 'whereNot orWhere' => [
['find' => [
['$or' => [
['$not' => ['name' => 'foo']],
['name' => ['$ne' => 'bar']],
]],
[], // options
]],
fn (Builder $builder) => $builder
->whereNot('name', 'foo')
->orWhere('name', '<>', 'bar'),
];

/** @see DatabaseQueryBuilderTest::testWhereNot() */
yield 'whereNot callable' => [
['find' => [
['$not' => ['name' => 'foo']],
[], // options
]],
fn (Builder $builder) => $builder
->whereNot(fn (Builder $q) => $q->where('name', 'foo')),
];

yield 'where whereNot' => [
['find' => [
['$and' => [
['name' => 'bar'],
['$not' => ['email' => 'foo']],
]],
[], // options
]],
fn (Builder $builder) => $builder
->where('name', '=', 'bar')
->whereNot(function (Builder $q) {
$q->where('email', '=', 'foo');
}),
];

yield 'whereNot (nested)' => [
['find' => [
['$not' => [
'$and' => [
['name' => 'foo'],
['$not' => ['email' => ['$ne' => 'bar']]],
],
]],
[], // options
]],
fn (Builder $builder) => $builder
->whereNot(function (Builder $q) {
$q->where('name', '=', 'foo')
->whereNot('email', '<>', 'bar');
}),
];

yield 'orWhere orWhereNot' => [
['find' => [
['$or' => [
['name' => 'bar'],
['$not' => ['email' => 'foo']],
]],
[], // options
]],
fn (Builder $builder) => $builder
->orWhere('name', '=', 'bar')
->orWhereNot(function (Builder $q) {
$q->where('email', '=', 'foo');
}),
];

yield 'where orWhereNot' => [
['find' => [
['$or' => [
['name' => 'bar'],
['$not' => ['email' => 'foo']],
]],
[], // options
]],
fn (Builder $builder) => $builder
->where('name', '=', 'bar')
->orWhereNot('email', '=', 'foo'),
];

/** @see DatabaseQueryBuilderTest::testWhereNotWithArrayConditions() */
yield 'whereNot with arrays of single condition' => [
['find' => [
['$not' => [
'$and' => [
['foo' => 1],
['bar' => 2],
],
]],
[], // options
]],
fn (Builder $builder) => $builder
->whereNot([['foo', 1], ['bar', 2]]),
];

yield 'whereNot with single array of conditions' => [
['find' => [
['$not' => [
'$and' => [
['foo' => 1],
['bar' => 2],
],
]],
[], // options
]],
fn (Builder $builder) => $builder
->whereNot(['foo' => 1, 'bar' => 2]),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also relates to my earlier comment about query optimization.

];

yield 'whereNot with arrays of single condition with operator' => [
['find' => [
['$not' => [
'$and' => [
['foo' => 1],
['bar' => ['$lt' => 2]],
],
]],
[], // options
]],
fn (Builder $builder) => $builder
->whereNot([
['foo', 1],
['bar', '<', 2],
]),
];

/** @see DatabaseQueryBuilderTest::testOrderBys() */
yield 'orderBy multiple columns' => [
['find' => [[], ['sort' => ['email' => 1, 'age' => -1]]]],
Expand Down