Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHPORM-100 Support query on numerical field names #2642

Merged
merged 9 commits into from
Oct 19, 2023
8 changes: 8 additions & 0 deletions src/Eloquent/Model.php
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,8 @@ public function getAttribute($key)
return null;
}

$key = (string) $key;
GromNaN marked this conversation as resolved.
Show resolved Hide resolved

// An unset attribute is null or throw an exception.
if (isset($this->unset[$key])) {
return $this->throwMissingAttributeExceptionIfApplicable($key);
Expand All @@ -194,6 +196,8 @@ public function getAttribute($key)
/** @inheritdoc */
protected function getAttributeFromArray($key)
{
$key = (string) $key;

// Support keys in dot notation.
if (str_contains($key, '.')) {
return Arr::get($this->attributes, $key);
Expand All @@ -205,6 +209,8 @@ protected function getAttributeFromArray($key)
/** @inheritdoc */
public function setAttribute($key, $value)
{
$key = (string) $key;

// Convert _id to ObjectID.
if ($key === '_id' && is_string($value)) {
$builder = $this->newBaseQueryBuilder();
Expand Down Expand Up @@ -314,6 +320,8 @@ public function originalIsEquivalent($key)
/** @inheritdoc */
public function offsetUnset($offset): void
{
$offset = (string) $offset;

if (str_contains($offset, '.')) {
// Update the field in the subdocument
Arr::forget($this->attributes, $offset);
Expand Down
48 changes: 37 additions & 11 deletions src/Query/Builder.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,12 @@
use MongoDB\BSON\UTCDateTime;
use MongoDB\Driver\Cursor;
use RuntimeException;
use Stringable;

use function array_fill_keys;
use function array_is_list;
use function array_key_exists;
use function array_map;
use function array_merge;
use function array_merge_recursive;
use function array_values;
use function array_walk_recursive;
use function assert;
Expand All @@ -46,7 +45,11 @@
use function implode;
use function in_array;
use function is_array;
use function is_bool;
use function is_callable;
use function is_float;
use function is_int;
use function is_null;
use function is_string;
use function md5;
use function preg_match;
Expand All @@ -60,6 +63,7 @@
use function strlen;
use function strtolower;
use function substr;
use function var_export;

class Builder extends BaseBuilder
{
Expand Down Expand Up @@ -665,7 +669,7 @@
{
// Use $set as default operator for field names that are not in an operator
foreach ($values as $key => $value) {
if (str_starts_with($key, '$')) {
if (is_string($key) && str_starts_with($key, '$')) {
continue;
}

Expand Down Expand Up @@ -952,7 +956,20 @@
return $id;
}

/** @inheritdoc */
/**
* Add a basic where clause to the query.
*
* If 1 argument, the signature is: where(array|Closure $where)
* If 2 arguments, the signature is: where(string $column, mixed $value)
* If 3 arguments, the signature is: where(string $colum, string $operator, mixed $value)
*
* @param Closure|string|array $column
* @param mixed $operator
* @param mixed $value
* @param string $boolean
*
* @return $this
*/
public function where($column, $operator = null, $value = null, $boolean = 'and')
{
$params = func_get_args();
Expand All @@ -966,8 +983,12 @@
}
}

if (func_num_args() === 1 && is_string($column)) {
throw new ArgumentCountError(sprintf('Too few arguments to function %s("%s"), 1 passed and at least 2 expected when the 1st is a string.', __METHOD__, $column));
if (func_num_args() === 1 && ! is_array($column) && ! is_callable($column)) {
GromNaN marked this conversation as resolved.
Show resolved Hide resolved
throw new ArgumentCountError(sprintf('Too few arguments to function %s(%s), 1 passed and at least 2 expected when the 1st is not an array or a callable', __METHOD__, var_export($column, true)));
}

if (is_float($column) || is_bool($column) || is_null($column)) {

Check failure on line 990 in src/Query/Builder.php

View workflow job for this annotation

GitHub Actions / phpcs

The use of function is_null() is forbidden
throw new InvalidArgumentException(sprintf('First argument of %s must be a field path as "string". Got "%s"', __METHOD__, get_debug_type($column)));
}

return parent::where(...$params);
Expand Down Expand Up @@ -998,17 +1019,15 @@
}

// Convert column name to string to use as array key
if (isset($where['column']) && $where['column'] instanceof Stringable) {
if (isset($where['column'])) {
$where['column'] = (string) $where['column'];
}

// Convert id's.
if (isset($where['column']) && ($where['column'] === '_id' || str_ends_with($where['column'], '._id'))) {
if (isset($where['values'])) {
// Multiple values.
foreach ($where['values'] as &$value) {
GromNaN marked this conversation as resolved.
Show resolved Hide resolved
$value = $this->convertKey($value);
}
$where['values'] = array_map($this->convertKey(...), $where['values']);
} elseif (isset($where['value'])) {
// Single value.
$where['value'] = $this->convertKey($where['value']);
Expand Down Expand Up @@ -1076,7 +1095,14 @@
}

// Merge the compiled where with the others.
$compiled = array_merge_recursive($compiled, $result);
// array_merge_recursive can't be used here because it converts int keys to sequential int.
GromNaN marked this conversation as resolved.
Show resolved Hide resolved
foreach ($result as $key => $value) {
if (in_array($key, ['$and', '$or', '$nor'])) {
$compiled[$key] = array_merge($compiled[$key] ?? [], $value);
} else {
$compiled[$key] = $value;
}
}
}

return $compiled;
Expand Down
16 changes: 16 additions & 0 deletions tests/ModelTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -971,4 +971,20 @@ public function testEnumCast(): void
$this->assertSame(MemberStatus::Member->value, $check->getRawOriginal('member_status'));
$this->assertSame(MemberStatus::Member, $check->member_status);
}

public function testNumericFieldName(): void
{
$user = new User();
jmikola marked this conversation as resolved.
Show resolved Hide resolved
$user->{1} = 'one';
$user->{2} = ['3' => 'two.three'];
$user->save();

$found = User::where(1, 'one')->first();
$this->assertInstanceOf(User::class, $found);
$this->assertEquals('one', $found[1]);

$found = User::where('2.3', 'two.three')->first();
$this->assertInstanceOf(User::class, $found);
$this->assertEquals([3 => 'two.three'], $found[2]);
}
}
19 changes: 18 additions & 1 deletion tests/Query/BuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,11 @@ public static function provideQueryBuilderToMql(): iterable
fn (Builder $builder) => $builder->where('foo', 'bar'),
];

yield 'find with numeric field name' => [
['find' => [['123' => 'bar'], []]],
fn (Builder $builder) => $builder->where(123, 'bar'),
];

yield 'where with single array of conditions' => [
[
'find' => [
Expand Down Expand Up @@ -1175,10 +1180,16 @@ public static function provideExceptions(): iterable

yield 'find with single string argument' => [
ArgumentCountError::class,
'Too few arguments to function MongoDB\Laravel\Query\Builder::where("foo"), 1 passed and at least 2 expected when the 1st is a string',
'Too few arguments to function MongoDB\Laravel\Query\Builder::where(\'foo\'), 1 passed and at least 2 expected when the 1st is not an array',
fn (Builder $builder) => $builder->where('foo'),
];

yield 'find with single numeric argument' => [
ArgumentCountError::class,
'Too few arguments to function MongoDB\Laravel\Query\Builder::where(123), 1 passed and at least 2 expected when the 1st is not an array',
fn (Builder $builder) => $builder->where(123),
];

yield 'where regex not starting with /' => [
LogicException::class,
'Missing expected starting delimiter in regular expression "^ac/me$", supported delimiters are: / # ~',
Expand Down Expand Up @@ -1208,6 +1219,12 @@ public static function provideExceptions(): iterable
'Invalid time format, expected HH:MM:SS, HH:MM or HH, got "stdClass"',
fn (Builder $builder) => $builder->whereTime('created_at', new stdClass()),
];

yield 'where invalid column type' => [
InvalidArgumentException::class,
'First argument of MongoDB\Laravel\Query\Builder::where must be a field path as "string". Got "float"',
fn (Builder $builder) => $builder->where(2.3, '>', 1),
];
}

/** @dataProvider getEloquentMethodsNotSupported */
Expand Down