From 3b4ad5ab55033528ad2383d01a8fdbc81c7f96b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Tue, 1 Apr 2025 16:43:59 +0200 Subject: [PATCH 1/4] PHPORM-255 Add a method to disable renaming the id to _id field in embedded documents --- src/Connection.php | 15 +++++++++++++ src/Query/Builder.php | 28 ++++++++++++++++-------- tests/Query/BuilderTest.php | 43 +++++++++++++++++++++++++++++-------- 3 files changed, 68 insertions(+), 18 deletions(-) diff --git a/src/Connection.php b/src/Connection.php index 4dd04120d..47594b168 100644 --- a/src/Connection.php +++ b/src/Connection.php @@ -53,6 +53,9 @@ class Connection extends BaseConnection private ?CommandSubscriber $commandSubscriber = null; + /** @var bool Whether to rename the rename "id" into "_id" for embedded documents. */ + private bool $renameEmbeddedIdField = true; + /** * Create a new database connection instance. */ @@ -395,6 +398,18 @@ public function __call($method, $parameters) return $this->db->$method(...$parameters); } + /** Set whether to rename "id" field into "_id" for embedded documents. */ + public function setRenameEmbeddedIdField(bool $rename): void + { + $this->renameEmbeddedIdField = $rename; + } + + /** Get whether to rename "id" field into "_id" for embedded documents. */ + public function getRenameEmbeddedIdField(): bool + { + return $this->renameEmbeddedIdField; + } + /** * Return the server version of one of the MongoDB servers: primary for * replica sets and standalone, and the selected server for sharded clusters. diff --git a/src/Query/Builder.php b/src/Query/Builder.php index 07a3483b0..477eac969 100644 --- a/src/Query/Builder.php +++ b/src/Query/Builder.php @@ -29,6 +29,7 @@ use MongoDB\Builder\Type\SearchOperatorInterface; use MongoDB\Driver\Cursor; use MongoDB\Driver\ReadPreference; +use MongoDB\Laravel\Connection; use Override; use RuntimeException; use stdClass; @@ -83,6 +84,7 @@ use function trait_exists; use function var_export; +/** @method Connection getConnection() */ class Builder extends BaseBuilder { private const REGEX_DELIMITERS = ['/', '#', '~']; @@ -124,6 +126,8 @@ class Builder extends BaseBuilder */ public $options = []; + private ?bool $renameEmbeddedIdField; + /** * All of the available clause operators. * @@ -1764,9 +1768,9 @@ public function orWhereIntegerNotInRaw($column, $values, $boolean = 'and') throw new BadMethodCallException('This method is not supported by MongoDB'); } - private function aliasIdForQuery(array $values): array + private function aliasIdForQuery(array $values, bool $root = true): array { - if (array_key_exists('id', $values)) { + if (array_key_exists('id', $values) && ($root || $this->getConnection()->getRenameEmbeddedIdField())) { if (array_key_exists('_id', $values) && $values['id'] !== $values['_id']) { throw new InvalidArgumentException('Cannot have both "id" and "_id" fields.'); } @@ -1793,7 +1797,7 @@ private function aliasIdForQuery(array $values): array } // ".id" subfield are alias for "._id" - if (str_ends_with($key, '.id')) { + if (str_ends_with($key, '.id') && ($root || $this->getConnection()->getRenameEmbeddedIdField())) { $newkey = substr($key, 0, -3) . '._id'; if (array_key_exists($newkey, $values) && $value !== $values[$newkey]) { throw new InvalidArgumentException(sprintf('Cannot have both "%s" and "%s" fields.', $key, $newkey)); @@ -1806,7 +1810,7 @@ private function aliasIdForQuery(array $values): array foreach ($values as &$value) { if (is_array($value)) { - $value = $this->aliasIdForQuery($value); + $value = $this->aliasIdForQuery($value, false); } elseif ($value instanceof DateTimeInterface) { $value = new UTCDateTime($value); } @@ -1824,10 +1828,13 @@ private function aliasIdForQuery(array $values): array * * @template T of array|object */ - public function aliasIdForResult(array|object $values): array|object + public function aliasIdForResult(array|object $values, bool $root = true): array|object { if (is_array($values)) { - if (array_key_exists('_id', $values) && ! array_key_exists('id', $values)) { + if ( + array_key_exists('_id', $values) && ! array_key_exists('id', $values) + && ($root || $this->getConnection()->getRenameEmbeddedIdField()) + ) { $values['id'] = $values['_id']; unset($values['_id']); } @@ -1837,13 +1844,16 @@ public function aliasIdForResult(array|object $values): array|object $values[$key] = Date::instance($value->toDateTime()) ->setTimezone(new DateTimeZone(date_default_timezone_get())); } elseif (is_array($value) || is_object($value)) { - $values[$key] = $this->aliasIdForResult($value); + $values[$key] = $this->aliasIdForResult($value, false); } } } if ($values instanceof stdClass) { - if (property_exists($values, '_id') && ! property_exists($values, 'id')) { + if ( + property_exists($values, '_id') && ! property_exists($values, 'id') + && ($root || $this->getConnection()->getRenameEmbeddedIdField()) + ) { $values->id = $values->_id; unset($values->_id); } @@ -1853,7 +1863,7 @@ public function aliasIdForResult(array|object $values): array|object $values->{$key} = Date::instance($value->toDateTime()) ->setTimezone(new DateTimeZone(date_default_timezone_get())); } elseif (is_array($value) || is_object($value)) { - $values->{$key} = $this->aliasIdForResult($value); + $values->{$key} = $this->aliasIdForResult($value, false); } } } diff --git a/tests/Query/BuilderTest.php b/tests/Query/BuilderTest.php index 20b5a12fb..6e68d42c7 100644 --- a/tests/Query/BuilderTest.php +++ b/tests/Query/BuilderTest.php @@ -12,7 +12,6 @@ use Illuminate\Tests\Database\DatabaseQueryBuilderTest; use InvalidArgumentException; use LogicException; -use Mockery as m; use MongoDB\BSON\Regex; use MongoDB\BSON\UTCDateTime; use MongoDB\Driver\ReadPreference; @@ -39,7 +38,7 @@ public function testMql(array $expected, Closure $build, ?string $requiredMethod $this->markTestSkipped(sprintf('Method "%s::%s()" does not exist.', Builder::class, $requiredMethod)); } - $builder = $build(self::getBuilder()); + $builder = $build($this->getBuilder()); $this->assertInstanceOf(Builder::class, $builder); $mql = $builder->toMql(); @@ -1447,7 +1446,7 @@ function (Builder $elemMatchQuery): void { #[DataProvider('provideExceptions')] public function testException($class, $message, Closure $build): void { - $builder = self::getBuilder(); + $builder = $this->getBuilder(); $this->expectException($class); $this->expectExceptionMessage($message); @@ -1545,7 +1544,7 @@ public static function provideExceptions(): iterable #[DataProvider('getEloquentMethodsNotSupported')] public function testEloquentMethodsNotSupported(Closure $callback) { - $builder = self::getBuilder(); + $builder = $this->getBuilder(); $this->expectException(BadMethodCallException::class); $this->expectExceptionMessage('This method is not supported by MongoDB'); @@ -1600,12 +1599,38 @@ public static function getEloquentMethodsNotSupported() yield 'orWhereIntegerNotInRaw' => [fn (Builder $builder) => $builder->orWhereIntegerNotInRaw('id', ['1a', 2])]; } - private static function getBuilder(): Builder + public function testRenameEmbeddedIdFieldCanBeDisabled() { - $connection = m::mock(Connection::class); - $processor = m::mock(Processor::class); - $connection->shouldReceive('getSession')->andReturn(null); - $connection->shouldReceive('getQueryGrammar')->andReturn(new Grammar($connection)); + $builder = $this->getBuilder(false); + $this->assertFalse($builder->getConnection()->getRenameEmbeddedIdField()); + + $mql = $builder + ->where('id', '=', 10) + ->where('nested.id', '=', 20) + ->where('embed', '=', ['id' => 30]) + ->toMql(); + + $this->assertEquals([ + 'find' => [ + [ + '$and' => [ + ['_id' => 10], + ['nested.id' => 20], + ['embed' => ['id' => 30]], + ], + ], + ['typeMap' => ['root' => 'object', 'document' => 'array']], + ], + ], $mql); + } + + private function getBuilder(bool $renameEmbeddedIdField = true): Builder + { + $connection = $this->createStub(Connection::class); + $connection->method('getRenameEmbeddedIdField')->willReturn($renameEmbeddedIdField); + $processor = $this->createStub(Processor::class); + $connection->method('getSession')->willReturn(null); + $connection->method('getQueryGrammar')->willReturn(new Grammar($connection)); return new Builder($connection, null, $processor); } From f7809069deddd5767147cc1efe0208801d0bd048 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Wed, 2 Apr 2025 11:12:17 +0200 Subject: [PATCH 2/4] Use $connection property --- src/Query/Builder.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Query/Builder.php b/src/Query/Builder.php index 477eac969..529c873ef 100644 --- a/src/Query/Builder.php +++ b/src/Query/Builder.php @@ -84,7 +84,7 @@ use function trait_exists; use function var_export; -/** @method Connection getConnection() */ +/** @property Connection $connection */ class Builder extends BaseBuilder { private const REGEX_DELIMITERS = ['/', '#', '~']; @@ -1770,7 +1770,7 @@ public function orWhereIntegerNotInRaw($column, $values, $boolean = 'and') private function aliasIdForQuery(array $values, bool $root = true): array { - if (array_key_exists('id', $values) && ($root || $this->getConnection()->getRenameEmbeddedIdField())) { + if (array_key_exists('id', $values) && ($root || $this->connection->getRenameEmbeddedIdField())) { if (array_key_exists('_id', $values) && $values['id'] !== $values['_id']) { throw new InvalidArgumentException('Cannot have both "id" and "_id" fields.'); } @@ -1797,7 +1797,7 @@ private function aliasIdForQuery(array $values, bool $root = true): array } // ".id" subfield are alias for "._id" - if (str_ends_with($key, '.id') && ($root || $this->getConnection()->getRenameEmbeddedIdField())) { + if (str_ends_with($key, '.id') && ($root || $this->connection->getRenameEmbeddedIdField())) { $newkey = substr($key, 0, -3) . '._id'; if (array_key_exists($newkey, $values) && $value !== $values[$newkey]) { throw new InvalidArgumentException(sprintf('Cannot have both "%s" and "%s" fields.', $key, $newkey)); @@ -1833,7 +1833,7 @@ public function aliasIdForResult(array|object $values, bool $root = true): array if (is_array($values)) { if ( array_key_exists('_id', $values) && ! array_key_exists('id', $values) - && ($root || $this->getConnection()->getRenameEmbeddedIdField()) + && ($root || $this->connection->getRenameEmbeddedIdField()) ) { $values['id'] = $values['_id']; unset($values['_id']); @@ -1852,7 +1852,7 @@ public function aliasIdForResult(array|object $values, bool $root = true): array if ($values instanceof stdClass) { if ( property_exists($values, '_id') && ! property_exists($values, 'id') - && ($root || $this->getConnection()->getRenameEmbeddedIdField()) + && ($root || $this->connection->getRenameEmbeddedIdField()) ) { $values->id = $values->_id; unset($values->_id); From 48c14cccb63383ad9b0dc413976d579cf3600025 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Mon, 7 Apr 2025 20:31:25 +0200 Subject: [PATCH 3/4] Add the option "rename_embedded_id_field" in database connection config --- src/Connection.php | 4 +++- src/Query/Builder.php | 2 -- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Connection.php b/src/Connection.php index 47594b168..29b72ae44 100644 --- a/src/Connection.php +++ b/src/Connection.php @@ -54,7 +54,7 @@ class Connection extends BaseConnection private ?CommandSubscriber $commandSubscriber = null; /** @var bool Whether to rename the rename "id" into "_id" for embedded documents. */ - private bool $renameEmbeddedIdField = true; + private bool $renameEmbeddedIdField; /** * Create a new database connection instance. @@ -83,6 +83,8 @@ public function __construct(array $config) $this->useDefaultSchemaGrammar(); $this->useDefaultQueryGrammar(); + + $this->renameEmbeddedIdField = $config['rename_embedded_id_field'] ?? true; } /** diff --git a/src/Query/Builder.php b/src/Query/Builder.php index 529c873ef..7d0fdce74 100644 --- a/src/Query/Builder.php +++ b/src/Query/Builder.php @@ -126,8 +126,6 @@ class Builder extends BaseBuilder */ public $options = []; - private ?bool $renameEmbeddedIdField; - /** * All of the available clause operators. * From 68e57f4e7ca0bea792b6f78d814899b41821e923 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Tue, 8 Apr 2025 21:56:38 +0200 Subject: [PATCH 4/4] Fix aliasing _id in embedded documents + add test --- src/Eloquent/Builder.php | 3 +- tests/QueryBuilderTest.php | 89 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 1 deletion(-) diff --git a/src/Eloquent/Builder.php b/src/Eloquent/Builder.php index aabc526f7..f3ffd7012 100644 --- a/src/Eloquent/Builder.php +++ b/src/Eloquent/Builder.php @@ -18,6 +18,7 @@ use MongoDB\Model\BSONDocument; use function array_key_exists; +use function array_map; use function array_replace; use function collect; use function is_array; @@ -237,7 +238,7 @@ public function raw($value = null) // Convert MongoCursor results to a collection of models. if ($results instanceof CursorInterface) { $results->setTypeMap(['root' => 'array', 'document' => 'array', 'array' => 'array']); - $results = $this->query->aliasIdForResult(iterator_to_array($results)); + $results = array_map(fn ($document) => $this->query->aliasIdForResult($document), iterator_to_array($results)); return $this->model->hydrate($results); } diff --git a/tests/QueryBuilderTest.php b/tests/QueryBuilderTest.php index 46beebab1..1233cda75 100644 --- a/tests/QueryBuilderTest.php +++ b/tests/QueryBuilderTest.php @@ -20,10 +20,12 @@ use MongoDB\BSON\UTCDateTime; use MongoDB\Collection; use MongoDB\Driver\Cursor; +use MongoDB\Driver\CursorInterface; use MongoDB\Driver\Monitoring\CommandFailedEvent; use MongoDB\Driver\Monitoring\CommandStartedEvent; use MongoDB\Driver\Monitoring\CommandSubscriber; use MongoDB\Driver\Monitoring\CommandSucceededEvent; +use MongoDB\Laravel\Connection; use MongoDB\Laravel\Query\Builder; use MongoDB\Laravel\Tests\Models\Item; use MongoDB\Laravel\Tests\Models\User; @@ -336,6 +338,93 @@ public function testRaw() $this->assertEquals('Jane Doe', $results[0]->name); } + public function testRawResultRenameId() + { + $connection = DB::connection('mongodb'); + self::assertInstanceOf(Connection::class, $connection); + + $date = Carbon::createFromDate(1986, 12, 31)->setTime(12, 0, 0); + User::insert([ + ['id' => 1, 'name' => 'Jane Doe', 'address' => ['id' => 11, 'city' => 'Ghent'], 'birthday' => $date], + ['id' => 2, 'name' => 'John Doe', 'address' => ['id' => 12, 'city' => 'Brussels'], 'birthday' => $date], + ]); + + // Using raw database query, result is not altered + $results = $connection->table('users')->raw(fn (Collection $collection) => $collection->find([])); + self::assertInstanceOf(CursorInterface::class, $results); + $results = $results->toArray(); + self::assertCount(2, $results); + + self::assertObjectHasProperty('_id', $results[0]); + self::assertObjectNotHasProperty('id', $results[0]); + self::assertSame(1, $results[0]->_id); + + self::assertObjectHasProperty('_id', $results[0]->address); + self::assertObjectNotHasProperty('id', $results[0]->address); + self::assertSame(11, $results[0]->address->_id); + + self::assertInstanceOf(UTCDateTime::class, $results[0]->birthday); + + // Using Eloquent query, result is transformed + self::assertTrue($connection->getRenameEmbeddedIdField()); + $results = User::raw(fn (Collection $collection) => $collection->find([])); + self::assertInstanceOf(LaravelCollection::class, $results); + self::assertCount(2, $results); + + $attributes = $results->first()->getAttributes(); + self::assertArrayHasKey('id', $attributes); + self::assertArrayNotHasKey('_id', $attributes); + self::assertSame(1, $attributes['id']); + + self::assertArrayHasKey('id', $attributes['address']); + self::assertArrayNotHasKey('_id', $attributes['address']); + self::assertSame(11, $attributes['address']['id']); + + self::assertEquals($date, $attributes['birthday']); + + // Single result + $result = User::raw(fn (Collection $collection) => $collection->findOne([], ['typeMap' => ['root' => 'object', 'document' => 'array']])); + self::assertInstanceOf(User::class, $result); + + $attributes = $result->getAttributes(); + self::assertArrayHasKey('id', $attributes); + self::assertArrayNotHasKey('_id', $attributes); + self::assertSame(1, $attributes['id']); + + self::assertArrayHasKey('id', $attributes['address']); + self::assertArrayNotHasKey('_id', $attributes['address']); + self::assertSame(11, $attributes['address']['id']); + + // Change the renameEmbeddedIdField option + $connection->setRenameEmbeddedIdField(false); + + $results = User::raw(fn (Collection $collection) => $collection->find([])); + self::assertInstanceOf(LaravelCollection::class, $results); + self::assertCount(2, $results); + + $attributes = $results->first()->getAttributes(); + self::assertArrayHasKey('id', $attributes); + self::assertArrayNotHasKey('_id', $attributes); + self::assertSame(1, $attributes['id']); + + self::assertArrayHasKey('_id', $attributes['address']); + self::assertArrayNotHasKey('id', $attributes['address']); + self::assertSame(11, $attributes['address']['_id']); + + // Single result + $result = User::raw(fn (Collection $collection) => $collection->findOne([])); + self::assertInstanceOf(User::class, $result); + + $attributes = $result->getAttributes(); + self::assertArrayHasKey('id', $attributes); + self::assertArrayNotHasKey('_id', $attributes); + self::assertSame(1, $attributes['id']); + + self::assertArrayHasKey('_id', $attributes['address']); + self::assertArrayNotHasKey('id', $attributes['address']); + self::assertSame(11, $attributes['address']['_id']); + } + public function testPush() { $id = DB::table('users')->insertGetId([