Skip to content

Commit

Permalink
PHPORM-47 Improve Builder::whereBetween to support CarbonPeriod and r…
Browse files Browse the repository at this point in the history 
…eject invalid array (mongodb#10)

The Query\Builder::whereBetween() method can be used like this:

whereBetween('date_field', [min, max])
whereBetween('date_field', collect([min, max]))
whereBetween('date_field', CarbonPeriod)

Laravel allows other formats: the $values array is flatten and the builder assumes there are at least 2 elements and ignore the others. It's a design that can lead to misunderstandings. I prefer to raise an exception when we have incorrect values, rather than trying to guess what the developer would like to do.

Support for CarbonPeriod was fixed in Laravel 10: laravel/framework#46720 because the query builder was taking the 1st 2 values of the iterator instead of the start & end dates.
  • Loading branch information
@GromNaN
GromNaN committed Jul 19, 2023
1 parent bd86f85 commit cf103ba
Show file tree
Hide file tree
Showing 2 changed files with 184 additions and 6 deletions.
27 changes: 22 additions & 5 deletions src/Query/Builder.php
View file
Expand Up @@ -2,6 +2,7 @@

namespace Jenssegers\Mongodb\Query;

use Carbon\CarbonPeriod;
use Closure;
use DateTimeInterface;
use Illuminate\Database\Query\Builder as BaseBuilder;
Expand Down Expand Up @@ -554,11 +555,20 @@ public function whereAll($column, array $values, $boolean = 'and', $not = false)

/**
* @inheritdoc
* @param list{mixed, mixed}|CarbonPeriod $values
*/
public function whereBetween($column, iterable $values, $boolean = 'and', $not = false)
{
$type = 'between';

if ($values instanceof Collection) {
$values = $values->all();
}

if (is_array($values) && (! array_is_list($values) || count($values) !== 2)) {
throw new \InvalidArgumentException('Between $values must be a list with exactly two elements: [min, max]');
}

$this->wheres[] = compact('column', 'type', 'boolean', 'values', 'not');

return $this;
Expand Down Expand Up @@ -995,11 +1005,18 @@ protected function compileWheres(): array
}
}
} elseif (isset($where['values'])) {
array_walk_recursive($where['values'], function (&$item, $key) {
if ($item instanceof DateTimeInterface) {
$item = new UTCDateTime($item);
}
});
if (is_array($where['values'])) {
array_walk_recursive($where['values'], function (&$item, $key) {
if ($item instanceof DateTimeInterface) {
$item = new UTCDateTime($item);
}
});
} elseif ($where['values'] instanceof CarbonPeriod) {
$where['values'] = [
new UTCDateTime($where['values']->getStartDate()),
new UTCDateTime($where['values']->getEndDate()),
];
}
}

// The next item in a "chain" of wheres devices the boolean of the
Expand Down
163 changes: 162 additions & 1 deletion tests/Query/BuilderTest.php
View file
Expand Up @@ -5,6 +5,7 @@
namespace Jenssegers\Mongodb\Tests\Query;

use DateTimeImmutable;
use Illuminate\Database\Eloquent\Collection;
use Illuminate\Tests\Database\DatabaseQueryBuilderTest;
use Jenssegers\Mongodb\Connection;
use Jenssegers\Mongodb\Query\Builder;
Expand Down Expand Up @@ -124,13 +125,142 @@ function (Builder $builder) {
->orderBy('score', ['$meta' => 'textScore']),
];

/** @see DatabaseQueryBuilderTest::testWhereBetweens() */
yield 'whereBetween array of numbers' => [
['find' => [['id' => ['$gte' => 1, '$lte' => 2]], []]],
fn (Builder $builder) => $builder->whereBetween('id', [1, 2]),
];

yield 'whereBetween nested array of numbers' => [
['find' => [['id' => ['$gte' => [1], '$lte' => [2, 3]]], []]],
fn (Builder $builder) => $builder->whereBetween('id', [[1], [2, 3]]),
];

$period = now()->toPeriod(now()->addMonth());
yield 'whereBetween CarbonPeriod' => [
['find' => [
['created_at' => ['$gte' => new UTCDateTime($period->start), '$lte' => new UTCDateTime($period->end)]],
[], // options
]],
fn (Builder $builder) => $builder->whereBetween('created_at', $period),
];

yield 'whereBetween collection' => [
['find' => [['id' => ['$gte' => 1, '$lte' => 2]], []]],
fn (Builder $builder) => $builder->whereBetween('id', collect([1, 2])),
];

/** @see DatabaseQueryBuilderTest::testOrWhereBetween() */
yield 'orWhereBetween array of numbers' => [
['find' => [
['$or' => [
['id' => 1],
['id' => ['$gte' => 3, '$lte' => 5]],
]],
[], // options
]],
fn (Builder $builder) => $builder
->where('id', '=', 1)
->orWhereBetween('id', [3, 5]),
];

/** @link https://www.mongodb.com/docs/manual/reference/bson-type-comparison-order/#arrays */
yield 'orWhereBetween nested array of numbers' => [
['find' => [
['$or' => [
['id' => 1],
['id' => ['$gte' => [4], '$lte' => [6, 8]]],
]],
[], // options
]],
fn (Builder $builder) => $builder
->where('id', '=', 1)
->orWhereBetween('id', [[4], [6, 8]]),
];

yield 'orWhereBetween collection' => [
['find' => [
['$or' => [
['id' => 1],
['id' => ['$gte' => 3, '$lte' => 4]],
]],
[], // options
]],
fn (Builder $builder) => $builder
->where('id', '=', 1)
->orWhereBetween('id', collect([3, 4])),
];

yield 'whereNotBetween array of numbers' => [
['find' => [
['$or' => [
['id' => ['$lte' => 1]],
['id' => ['$gte' => 2]],
]],
[], // options
]],
fn (Builder $builder) => $builder->whereNotBetween('id', [1, 2]),
];

/** @see DatabaseQueryBuilderTest::testOrWhereNotBetween() */
yield 'orWhereNotBetween array of numbers' => [
['find' => [
['$or' => [
['id' => 1],
['$or' => [
['id' => ['$lte' => 3]],
['id' => ['$gte' => 5]],
]],
]],
[], // options
]],
fn (Builder $builder) => $builder
->where('id', '=', 1)
->orWhereNotBetween('id', [3, 5]),
];

yield 'orWhereNotBetween nested array of numbers' => [
['find' => [
['$or' => [
['id' => 1],
['$or' => [
['id' => ['$lte' => [2, 3]]],
['id' => ['$gte' => [5]]],
]],
]],
[], // options
]],
fn (Builder $builder) => $builder
->where('id', '=', 1)
->orWhereNotBetween('id', [[2, 3], [5]]),
];

yield 'orWhereNotBetween collection' => [
['find' => [
['$or' => [
['id' => 1],
['$or' => [
['id' => ['$lte' => 3]],
['id' => ['$gte' => 4]],
]],
]],
[], // options
]],
fn (Builder $builder) => $builder
->where('id', '=', 1)
->orWhereNotBetween('id', collect([3, 4])),
];

yield 'distinct' => [
['distinct' => ['foo', [], []]],
fn (Builder $builder) => $builder->distinct('foo'),
];

yield 'groupBy' => [
['aggregate' => [[['$group' => ['_id' => ['foo' => '$foo'], 'foo' => ['$last' => '$foo']]]], []]],
['aggregate' => [
[['$group' => ['_id' => ['foo' => '$foo'], 'foo' => ['$last' => '$foo']]]],
[], // options
]],
fn (Builder $builder) => $builder->groupBy('foo'),
];
}
Expand All @@ -154,6 +284,37 @@ public static function provideExceptions(): iterable
'Order direction must be "asc" or "desc"',
fn (Builder $builder) => $builder->orderBy('_id', 'dasc'),
];

/** @see DatabaseQueryBuilderTest::testWhereBetweens */
yield 'whereBetween array too short' => [
\InvalidArgumentException::class,
'Between $values must be a list with exactly two elements: [min, max]',
fn (Builder $builder) => $builder->whereBetween('id', [1]),
];

yield 'whereBetween array too short (nested)' => [
\InvalidArgumentException::class,
'Between $values must be a list with exactly two elements: [min, max]',
fn (Builder $builder) => $builder->whereBetween('id', [[1, 2]]),
];

yield 'whereBetween array too long' => [
\InvalidArgumentException::class,
'Between $values must be a list with exactly two elements: [min, max]',
fn (Builder $builder) => $builder->whereBetween('id', [1, 2, 3]),
];

yield 'whereBetween collection too long' => [
\InvalidArgumentException::class,
'Between $values must be a list with exactly two elements: [min, max]',
fn (Builder $builder) => $builder->whereBetween('id', new Collection([1, 2, 3])),
];

yield 'whereBetween array is not a list' => [
\InvalidArgumentException::class,
'Between $values must be a list with exactly two elements: [min, max]',
fn (Builder $builder) => $builder->whereBetween('id', ['min' => 1, 'max' => 2]),
];
}

/** @dataProvider getEloquentMethodsNotSupported */
Expand Down

0 comments on commit cf103ba

Please sign in to comment.