From 4790125edf9c6bf41e50e1f0ba04c6dc2978b423 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Wed, 16 Aug 2023 13:51:21 +0200 Subject: [PATCH 1/5] Attempt to remove automatic conversion --- src/Eloquent/Builder.php | 5 +++ src/Eloquent/Casts/ObjectId.php | 6 +-- src/Eloquent/Model.php | 74 +++++++++---------------------- src/Query/Builder.php | 34 +------------- src/Relations/EmbedsOneOrMany.php | 3 +- tests/EmbeddedRelationsTest.php | 4 +- tests/ModelTest.php | 13 +++--- tests/Models/Address.php | 7 +++ tests/Models/User.php | 2 + 9 files changed, 48 insertions(+), 100 deletions(-) diff --git a/src/Eloquent/Builder.php b/src/Eloquent/Builder.php index 84e93b83..0d7816e3 100644 --- a/src/Eloquent/Builder.php +++ b/src/Eloquent/Builder.php @@ -241,4 +241,9 @@ protected function ensureOrderForCursorPagination($shouldReverse = false) ]; })->values(); } + + public function whereKey($id) + { + return parent::whereKey($this->model->convertKey($id)); + } } diff --git a/src/Eloquent/Casts/ObjectId.php b/src/Eloquent/Casts/ObjectId.php index bf34bea2..3961d23d 100644 --- a/src/Eloquent/Casts/ObjectId.php +++ b/src/Eloquent/Casts/ObjectId.php @@ -37,10 +37,8 @@ public function get($model, string $key, $value, array $attributes) */ public function set($model, string $key, $value, array $attributes) { - if ($value instanceof BSONObjectId) { - return $value; - } + $value = $value instanceof BSONObjectId ? $value : new BSONObjectId($value); - return new BSONObjectId($value); + return [$key => $value]; } } diff --git a/src/Eloquent/Model.php b/src/Eloquent/Model.php index 2d985f62..320f8102 100644 --- a/src/Eloquent/Model.php +++ b/src/Eloquent/Model.php @@ -15,8 +15,6 @@ use Illuminate\Support\Str; use function in_array; use Jenssegers\Mongodb\Query\Builder as QueryBuilder; -use MongoDB\BSON\Binary; -use MongoDB\BSON\ObjectID; use MongoDB\BSON\UTCDateTime; use function uniqid; @@ -52,30 +50,6 @@ abstract class Model extends BaseModel */ protected $parentRelation; - /** - * Custom accessor for the model's id. - * - * @param mixed $value - * @return mixed - */ - public function getIdAttribute($value = null) - { - // If we don't have a value for 'id', we will use the MongoDB '_id' value. - // This allows us to work with models in a more sql-like way. - if (! $value && array_key_exists('_id', $this->attributes)) { - $value = $this->attributes['_id']; - } - - // Convert ObjectID to string. - if ($value instanceof ObjectID) { - return (string) $value; - } elseif ($value instanceof Binary) { - return (string) $value->getData(); - } - - return $value; - } - /** * @inheritdoc */ @@ -190,12 +164,7 @@ protected function getAttributeFromArray($key) public function setAttribute($key, $value) { // Convert _id to ObjectID. - if ($key == '_id' && is_string($value)) { - $builder = $this->newBaseQueryBuilder(); - - $value = $builder->convertKey($value); - } // Support keys in dot notation. - elseif (Str::contains($key, '.')) { + if (Str::contains($key, '.')) { // Store to a temporary key, then move data to the actual key $uniqueKey = uniqid($key); parent::setAttribute($uniqueKey, $value); @@ -209,28 +178,6 @@ public function setAttribute($key, $value) return parent::setAttribute($key, $value); } - /** - * @inheritdoc - */ - public function attributesToArray() - { - $attributes = parent::attributesToArray(); - - // Because the original Eloquent never returns objects, we convert - // MongoDB related objects to a string representation. This kind - // of mimics the SQL behaviour so that dates are formatted - // nicely when your models are converted to JSON. - foreach ($attributes as $key => &$value) { - if ($value instanceof ObjectID) { - $value = (string) $value; - } elseif ($value instanceof Binary) { - $value = (string) $value->getData(); - } - } - - return $attributes; - } - /** * @inheritdoc */ @@ -568,4 +515,23 @@ protected function addCastAttributesToArray(array $attributes, array $mutatedAtt return $attributes; } + + /** @internal */ + public function convertKey($value) + { + if (! $this->hasCast($this->primaryKey)) { + return $value; + } + + return $this->castAttribute($this->primaryKey, $value); + } + + protected function getClassCastableAttributeValue($key, $value) + { + // The class cast cache does not play nice with database values that + // already are objects, so we need to manually unset it + unset($this->classCastCache[$key]); + + return parent::getClassCastableAttributeValue($key, $value); + } } diff --git a/src/Query/Builder.php b/src/Query/Builder.php index dd448ed0..1b361502 100644 --- a/src/Query/Builder.php +++ b/src/Query/Builder.php @@ -188,7 +188,7 @@ public function hint($index) */ public function find($id, $columns = []) { - return $this->where('_id', '=', $this->convertKey($id))->first($columns); + return parent::find($id, $columns); } /** @@ -884,25 +884,6 @@ protected function performUpdate($query, array $options = []) return 0; } - /** - * Convert a key to ObjectID if needed. - * - * @param mixed $id - * @return mixed - */ - public function convertKey($id) - { - if (is_string($id) && strlen($id) === 24 && ctype_xdigit($id)) { - return new ObjectID($id); - } - - if (is_string($id) && strlen($id) === 16 && preg_match('~[^\x20-\x7E\t\r\n]~', $id) > 0) { - return new Binary($id, Binary::TYPE_UUID); - } - - return $id; - } - /** * @inheritdoc */ @@ -950,19 +931,6 @@ protected function compileWheres(): array } } - // Convert id's. - if (isset($where['column']) && ($where['column'] == '_id' || Str::endsWith($where['column'], '._id'))) { - // Multiple values. - if (isset($where['values'])) { - foreach ($where['values'] as &$value) { - $value = $this->convertKey($value); - } - } // Single value. - elseif (isset($where['value'])) { - $where['value'] = $this->convertKey($where['value']); - } - } - // Convert DateTime values to UTCDateTime. if (isset($where['value'])) { if (is_array($where['value'])) { diff --git a/src/Relations/EmbedsOneOrMany.php b/src/Relations/EmbedsOneOrMany.php index dedae591..929baef9 100644 --- a/src/Relations/EmbedsOneOrMany.php +++ b/src/Relations/EmbedsOneOrMany.php @@ -245,8 +245,7 @@ protected function getForeignKeyValue($id) $id = $id->getKey(); } - // Convert the id to MongoId if necessary. - return $this->toBase()->convertKey($id); + return $id; } /** diff --git a/tests/EmbeddedRelationsTest.php b/tests/EmbeddedRelationsTest.php index 972572cc..34d2af6b 100644 --- a/tests/EmbeddedRelationsTest.php +++ b/tests/EmbeddedRelationsTest.php @@ -60,7 +60,7 @@ public function testEmbedsManySave() $this->assertInstanceOf(DateTime::class, $address->created_at); $this->assertInstanceOf(DateTime::class, $address->updated_at); $this->assertNotNull($address->_id); - $this->assertIsString($address->_id); + $this->assertInstanceOf(ObjectId::class, $address->_id); $raw = $address->getAttributes(); $this->assertInstanceOf(ObjectId::class, $raw['_id']); @@ -183,7 +183,7 @@ public function testEmbedsManyCreate() $user = User::create([]); $address = $user->addresses()->create(['city' => 'Bruxelles']); $this->assertInstanceOf(Address::class, $address); - $this->assertIsString($address->_id); + $this->assertInstanceOf(ObjectId::class, $address->_id); $this->assertEquals(['Bruxelles'], $user->addresses->pluck('city')->all()); $raw = $address->getAttributes(); diff --git a/tests/ModelTest.php b/tests/ModelTest.php index 1042a07b..78cc833c 100644 --- a/tests/ModelTest.php +++ b/tests/ModelTest.php @@ -86,6 +86,7 @@ public function testUpdate(): void /** @var User $check */ $check = User::find($user->_id); + $this->assertInstanceOf(User::class, $check); $check->age = 36; $check->save(); @@ -395,22 +396,24 @@ public static function provideId(): iterable yield 'ObjectID' => [ 'model' => User::class, 'id' => $objectId, - 'expected' => (string) $objectId, - 'expectedFound' => true, + 'expected' => $objectId, + // Not found as the keyType is "string" + 'expectedFound' => false, ]; $binaryUuid = new Binary(hex2bin('0c103357380648c9a84b867dcb625cfb'), Binary::TYPE_UUID); yield 'BinaryUuid' => [ 'model' => User::class, 'id' => $binaryUuid, - 'expected' => (string) $binaryUuid, - 'expectedFound' => true, + 'expected' => $binaryUuid, + // Not found as the keyType is "string" + 'expectedFound' => false, ]; yield 'cast as BinaryUuid' => [ 'model' => IdIsBinaryUuid::class, 'id' => $binaryUuid, - 'expected' => (string) $binaryUuid, + 'expected' => $binaryUuid, 'expectedFound' => true, ]; diff --git a/tests/Models/Address.php b/tests/Models/Address.php index 1050eb0e..c05f5757 100644 --- a/tests/Models/Address.php +++ b/tests/Models/Address.php @@ -4,13 +4,20 @@ namespace Jenssegers\Mongodb\Tests\Models; +use Jenssegers\Mongodb\Eloquent\Casts\ObjectId as ObjectIdCast; use Jenssegers\Mongodb\Eloquent\Model as Eloquent; use Jenssegers\Mongodb\Relations\EmbedsMany; +use MongoDB\BSON\ObjectId; class Address extends Eloquent { protected $connection = 'mongodb'; protected static $unguarded = true; + protected $keyType = ObjectId::class; + + protected $casts = [ + '_id' => ObjectIdCast::class, + ]; public function addresses(): EmbedsMany { diff --git a/tests/Models/User.php b/tests/Models/User.php index f559af47..e3f1420e 100644 --- a/tests/Models/User.php +++ b/tests/Models/User.php @@ -12,6 +12,7 @@ use Illuminate\Database\Eloquent\Casts\Attribute; use Illuminate\Notifications\Notifiable; use Illuminate\Support\Str; +use Jenssegers\Mongodb\Eloquent\Casts\ObjectId as ObjectIdCast; use Jenssegers\Mongodb\Eloquent\HybridRelations; use Jenssegers\Mongodb\Eloquent\Model as Eloquent; @@ -38,6 +39,7 @@ class User extends Eloquent implements AuthenticatableContract, CanResetPassword protected $connection = 'mongodb'; protected $casts = [ + '_id' => ObjectIdCast::class, 'birthday' => 'datetime', 'entry.date' => 'datetime', 'member_status' => MemberStatus::class, From 6946c7ab4c86edc7968fc91f102098e133351a73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Thu, 17 Aug 2023 13:09:59 +0200 Subject: [PATCH 2/5] Apply cast on eloquent queries --- src/Eloquent/Builder.php | 60 ++++++++++++++++++++++++++++----- src/Eloquent/Casts/ObjectId.php | 28 ++++++++++++--- src/Eloquent/Model.php | 49 ++++++++++++++++++++++++--- src/Query/Builder.php | 17 ++++------ tests/ModelTest.php | 25 +++++++------- 5 files changed, 138 insertions(+), 41 deletions(-) diff --git a/src/Eloquent/Builder.php b/src/Eloquent/Builder.php index 0d7816e3..dd52bff5 100644 --- a/src/Eloquent/Builder.php +++ b/src/Eloquent/Builder.php @@ -2,7 +2,10 @@ namespace Jenssegers\Mongodb\Eloquent; +use Illuminate\Contracts\Support\Arrayable; +use Illuminate\Database\Concerns\BuildsQueries; use Illuminate\Database\Eloquent\Builder as EloquentBuilder; +use Illuminate\Database\Eloquent\Model; use Jenssegers\Mongodb\Helpers\QueriesRelationships; use MongoDB\Driver\Cursor; use MongoDB\Model\BSONDocument; @@ -153,14 +156,6 @@ public function decrement($column, $amount = 1, array $extra = []) return parent::decrement($column, $amount, $extra); } - /** - * @inheritdoc - */ - public function chunkById($count, callable $callback, $column = '_id', $alias = null) - { - return parent::chunkById($count, $callback, $column, $alias); - } - /** * @inheritdoc */ @@ -219,6 +214,37 @@ public function getConnection() return $this->query->getConnection(); } + /** + * @see \Illuminate\Database\Query\Builder::forPageBeforeId() + */ + public function forPageBeforeId($perPage = 15, $lastId = 0, $column = null) + { + if (! $column || $column === $this->model->getKeyName()) { + $column = $this->model->getKeyName(); + if ($lastId !== null) { + $lastId = $this->model->castKeyForDatabase($lastId); + } + } + + return parent::forPageBeforeId($perPage, $lastId, $column); + } + + /** + * @see \Illuminate\Database\Query\Builder::forPageAfterId() + * @see BuildsQueries::chunkById() for usage + */ + public function forPageAfterId($perPage = 15, $lastId = 0, $column = null) + { + if (! $column || $column === $this->model->getKeyName()) { + $column = $this->model->getKeyName(); + if ($lastId !== null) { + $lastId = $this->model->castKeyForDatabase($lastId); + } + } + + return parent::forPageAfterId($perPage, $lastId, $column); + } + /** * @inheritdoc */ @@ -244,6 +270,22 @@ protected function ensureOrderForCursorPagination($shouldReverse = false) public function whereKey($id) { - return parent::whereKey($this->model->convertKey($id)); + if ($id instanceof Model) { + $id = $id->getKey(); + } + + if (is_array($id) || $id instanceof Arrayable) { + $id = array_map(function ($id) { + return $this->model->castKeyForDatabase($id); + }, $id); + + $this->query->whereIn($this->model->getQualifiedKeyName(), $id); + + return $this; + } + + $id = $this->model->castKeyForDatabase($id); + + return $this->where($this->model->getQualifiedKeyName(), '=', $id); } } diff --git a/src/Eloquent/Casts/ObjectId.php b/src/Eloquent/Casts/ObjectId.php index 3961d23d..6eebf4d8 100644 --- a/src/Eloquent/Casts/ObjectId.php +++ b/src/Eloquent/Casts/ObjectId.php @@ -5,11 +5,19 @@ use Illuminate\Contracts\Database\Eloquent\CastsAttributes; use Jenssegers\Mongodb\Eloquent\Model; use MongoDB\BSON\ObjectId as BSONObjectId; +use MongoDB\Driver\Exception\InvalidArgumentException; +/** + * Store the value as an ObjectId in the database. This cast should be used for _id fields. + * The value read from the database will not be transformed. + * + * @extends CastsAttributes + */ class ObjectId implements CastsAttributes { /** * Cast the given value. + * Nothing will be done here, the value should already be an ObjectId in the database. * * @param Model $model * @param string $key @@ -19,25 +27,37 @@ class ObjectId implements CastsAttributes */ public function get($model, string $key, $value, array $attributes) { - if (! $value instanceof BSONObjectId) { - return $value; + if ($value instanceof BSONObjectId && $model->getKeyName() === $key && $model->getKeyType() === 'string') { + return (string) $value; } - return (string) $value; + return $value; } /** * Prepare the given value for storage. + * The value will be converted to an ObjectId. * * @param Model $model * @param string $key * @param mixed $value * @param array $attributes * @return mixed + * + * @throws \RuntimeException when the value is not an ObjectID or a valid ID string. */ public function set($model, string $key, $value, array $attributes) { - $value = $value instanceof BSONObjectId ? $value : new BSONObjectId($value); + if (! $value instanceof BSONObjectId) { + if (! is_string($value)) { + throw new \RuntimeException(sprintf('Invalid BSON ObjectID provided for %s[%s]. "string" or %s expected, got "%s". Remove the ObjectId cast if you need to store other types of values.', get_class($model), $key, BSONObjectId::class, get_debug_type($value))); + } + try { + $value = new BSONObjectId($value); + } catch (InvalidArgumentException $e) { + throw new \RuntimeException(sprintf('Invalid BSON ObjectID provided for %s[%s]: %s. Remove the ObjectID cast if you need to store string values.', get_class($model), $key, $value), 0, $e); + } + } return [$key => $value]; } diff --git a/src/Eloquent/Model.php b/src/Eloquent/Model.php index 320f8102..c999a2ec 100644 --- a/src/Eloquent/Model.php +++ b/src/Eloquent/Model.php @@ -145,13 +145,24 @@ public function getAttribute($key) return parent::getAttribute($key); } + protected function throwMissingAttributeExceptionIfApplicable($key) + { + // Fallback to "_id" if "id" is not set. + if ($key === 'id') { + // Should be deprecated? + return $this->getAttribute('_id'); + } + + parent::throwMissingAttributeExceptionIfApplicable($key); + } + /** * @inheritdoc */ protected function getAttributeFromArray($key) { // Support keys in dot notation. - if (Str::contains($key, '.')) { + if (str_contains($key, '.')) { return Arr::get($this->attributes, $key); } @@ -238,7 +249,7 @@ public function drop($columns) } // Perform unset only on current document - return $this->newQuery()->where($this->getKeyName(), $this->getKey())->unset($columns); + return $this->newQuery()->whereKey($this->getKey())->unset($columns); } /** @@ -449,6 +460,26 @@ protected function isGuardableColumn($key) return true; } + /** + * @see \Jenssegers\Mongodb\Query\Builder::whereIn() + * Add a "where in" clause to the query. + * + * @param \Illuminate\Contracts\Database\Query\Expression|string $column + * @param mixed $values + * @param string $boolean + * @param bool $not + * @return $this + */ + public function whereIn($column, $values, $boolean = 'and', $not = false) + { + $args = func_get_args(); + if ($column === $this->getKeyName()) { + $args[1] = array_map(fn ($value) => $this->castKeyForDatabase($value), $args[1]); + } + + return parent::__call(__FUNCTION__, $args); + } + /** * @inheritdoc */ @@ -517,13 +548,21 @@ protected function addCastAttributesToArray(array $attributes, array $mutatedAtt } /** @internal */ - public function convertKey($value) + public function castKeyForDatabase($value) { - if (! $this->hasCast($this->primaryKey)) { + $key = $this->primaryKey; + + if (! $this->hasCast($key)) { + return $value; + } + + if (! $this->isClassCastable($key)) { return $value; } + $caster = $this->resolveCasterClass($key); + $attributes = $this->normalizeCastClassResponse($key, $caster->set($this, $key, $value, [])); - return $this->castAttribute($this->primaryKey, $value); + return $attributes[$key]; } protected function getClassCastableAttributeValue($key, $value) diff --git a/src/Query/Builder.php b/src/Query/Builder.php index 1b361502..22e27623 100644 --- a/src/Query/Builder.php +++ b/src/Query/Builder.php @@ -12,8 +12,6 @@ use Illuminate\Support\LazyCollection; use Illuminate\Support\Str; use Jenssegers\Mongodb\Connection; -use MongoDB\BSON\Binary; -use MongoDB\BSON\ObjectID; use MongoDB\BSON\Regex; use MongoDB\BSON\UTCDateTime; use MongoDB\Driver\Cursor; @@ -188,7 +186,7 @@ public function hint($index) */ public function find($id, $columns = []) { - return parent::find($id, $columns); + return $this->where('_id', '=', $id)->first($columns); } /** @@ -657,14 +655,6 @@ public function decrement($column, $amount = 1, array $extra = [], array $option return $this->increment($column, -1 * $amount, $extra, $options); } - /** - * @inheritdoc - */ - public function chunkById($count, callable $callback, $column = '_id', $alias = null) - { - return parent::chunkById($count, $callback, $column, $alias); - } - /** * @inheritdoc */ @@ -907,6 +897,11 @@ public function where($column, $operator = null, $value = null, $boolean = 'and' return parent::where(...$params); } + protected function defaultKeyName(): string + { + return '_id'; + } + /** * Compile the where array. * diff --git a/tests/ModelTest.php b/tests/ModelTest.php index 78cc833c..f03c4280 100644 --- a/tests/ModelTest.php +++ b/tests/ModelTest.php @@ -62,8 +62,8 @@ public function testInsert(): void $this->assertTrue(isset($user->_id)); $this->assertIsString($user->_id); - $this->assertNotEquals('', (string) $user->_id); - $this->assertNotEquals(0, strlen((string) $user->_id)); + $this->assertNotEquals('', $user->_id); + $this->assertNotEquals(0, strlen($user->_id)); $this->assertInstanceOf(Carbon::class, $user->created_at); $raw = $user->getAttributes(); @@ -123,14 +123,14 @@ public function testManualStringId(): void $this->assertInstanceOf(ObjectID::class, $raw['_id']); $user = new User; - $user->_id = 'customId'; + $user->_id = '64d9e303455918c12204cfb5'; $user->name = 'John Doe'; $user->title = 'admin'; $user->age = 35; $user->save(); $this->assertTrue($user->exists); - $this->assertEquals('customId', $user->_id); + $this->assertEquals('64d9e303455918c12204cfb5', $user->_id); $raw = $user->getAttributes(); $this->assertIsString($raw['_id']); @@ -275,7 +275,8 @@ public function testDestroy(): void $user->age = 35; $user->save(); - User::destroy((string) $user->_id); + $this->assertIsString($user->_id); + User::destroy($user->_id); $this->assertEquals(0, User::count()); } @@ -396,9 +397,9 @@ public static function provideId(): iterable yield 'ObjectID' => [ 'model' => User::class, 'id' => $objectId, - 'expected' => $objectId, - // Not found as the keyType is "string" - 'expectedFound' => false, + // $keyType is string, so the ObjectID caster convert to string + 'expected' => (string) $objectId, + 'expectedFound' => true, ]; $binaryUuid = new Binary(hex2bin('0c103357380648c9a84b867dcb625cfb'), Binary::TYPE_UUID); @@ -466,7 +467,7 @@ public function testToArray(): void $this->assertEquals(['_id', 'created_at', 'name', 'type', 'updated_at'], $keys); $this->assertIsString($array['created_at']); $this->assertIsString($array['updated_at']); - $this->assertIsString($array['_id']); + $this->assertInstanceOf(ObjectID::class, $array['_id']); } public function testUnset(): void @@ -719,20 +720,20 @@ public function testPushPull(): void $user->push('tags', 'tag2', true); $this->assertEquals(['tag1', 'tag1', 'tag2'], $user->tags); - $user = User::where('_id', $user->_id)->first(); + $user = User::find($user->_id); $this->assertEquals(['tag1', 'tag1', 'tag2'], $user->tags); $user->pull('tags', 'tag1'); $this->assertEquals(['tag2'], $user->tags); - $user = User::where('_id', $user->_id)->first(); + $user = User::find($user->_id)->first(); $this->assertEquals(['tag2'], $user->tags); $user->push('tags', 'tag3'); $user->pull('tags', ['tag2', 'tag3']); $this->assertEquals([], $user->tags); - $user = User::where('_id', $user->_id)->first(); + $user = User::find($user->_id)->first(); $this->assertEquals([], $user->tags); } From 8ca9dc4b6d4e0465efd69d65c96d8c81a0f0c1f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Thu, 17 Aug 2023 13:47:14 +0200 Subject: [PATCH 3/5] Update src/Query/Builder.php --- src/Query/Builder.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Query/Builder.php b/src/Query/Builder.php index 22e27623..3f7c7c53 100644 --- a/src/Query/Builder.php +++ b/src/Query/Builder.php @@ -186,7 +186,11 @@ public function hint($index) */ public function find($id, $columns = []) { - return $this->where('_id', '=', $id)->first($columns); + /** + * Remove this method when this is fixed in Laravel. + * @see https://github.com/laravel/framework/pull/48089 + */ + return $this->where($this->defaultKeyName(), '=', $id)->first($columns); } /** From 4d4d456cc7c6fe363adee64a19f32f6113fc26d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Fri, 18 Aug 2023 12:03:02 +0200 Subject: [PATCH 4/5] Use ObjectId by default for Model _id --- src/Eloquent/Model.php | 3 ++- src/Query/Builder.php | 2 +- src/Relations/BelongsToMany.php | 12 +++++++----- src/Relations/EmbedsOneOrMany.php | 10 ++++++++-- tests/RelationsTest.php | 8 ++++---- tests/TestCase.php | 15 +++++++++++++++ 6 files changed, 37 insertions(+), 13 deletions(-) diff --git a/src/Eloquent/Model.php b/src/Eloquent/Model.php index c999a2ec..3833c0b8 100644 --- a/src/Eloquent/Model.php +++ b/src/Eloquent/Model.php @@ -2,6 +2,7 @@ namespace Jenssegers\Mongodb\Eloquent; +use MongoDB\BSON\ObjectId; use function array_key_exists; use DateTimeInterface; use function explode; @@ -41,7 +42,7 @@ abstract class Model extends BaseModel * * @var string */ - protected $keyType = 'string'; + protected $keyType = ObjectId::class; /** * The parent relation instance. diff --git a/src/Query/Builder.php b/src/Query/Builder.php index 3f7c7c53..0585bb6f 100644 --- a/src/Query/Builder.php +++ b/src/Query/Builder.php @@ -872,7 +872,7 @@ protected function performUpdate($query, array $options = []) $wheres = $this->compileWheres(); $result = $this->collection->updateMany($wheres, $query, $options); if (1 == (int) $result->isAcknowledged()) { - return $result->getModifiedCount() ? $result->getModifiedCount() : $result->getUpsertedCount(); + return $result->getModifiedCount() ?: $result->getUpsertedCount(); } return 0; diff --git a/src/Relations/BelongsToMany.php b/src/Relations/BelongsToMany.php index 824a4509..46a88cb4 100644 --- a/src/Relations/BelongsToMany.php +++ b/src/Relations/BelongsToMany.php @@ -195,14 +195,14 @@ public function attach($id, array $attributes = [], $touch = true) $query = $this->newRelatedQuery(); - $query->whereIn($this->related->getKeyName(), (array) $id); + $query->whereIn($this->related->getKeyName(), is_array($id) ? $id : [$id]); // Attach the new parent id to the related model. $query->push($this->foreignPivotKey, $this->parent->getKey(), true); } // Attach the new ids to the parent model. - $this->parent->push($this->getRelatedKey(), (array) $id, true); + $this->parent->push($this->getRelatedKey(), is_array($id) ? $id : [$id], true); if ($touch) { $this->touchIfTouching(); @@ -215,7 +215,7 @@ public function attach($id, array $attributes = [], $touch = true) public function detach($ids = [], $touch = true) { if ($ids instanceof Model) { - $ids = (array) $ids->getKey(); + $ids = $ids->getKey(); } $query = $this->newRelatedQuery(); @@ -223,7 +223,9 @@ public function detach($ids = [], $touch = true) // If associated IDs were passed to the method we will only delete those // associations, otherwise all of the association ties will be broken. // We'll return the numbers of affected rows when we do the deletes. - $ids = (array) $ids; + if (! is_array($ids)) { + $ids = [$ids]; + } // Detach all ids from the parent model. $this->parent->pull($this->getRelatedKey(), $ids); @@ -257,7 +259,7 @@ protected function buildDictionary(Collection $results) foreach ($results as $result) { foreach ($result->$foreign as $item) { - $dictionary[$item][] = $result; + $dictionary[(string) $item][] = $result; } } diff --git a/src/Relations/EmbedsOneOrMany.php b/src/Relations/EmbedsOneOrMany.php index 929baef9..0b3f00e1 100644 --- a/src/Relations/EmbedsOneOrMany.php +++ b/src/Relations/EmbedsOneOrMany.php @@ -212,9 +212,15 @@ protected function getEmbedded() $attributes = $this->parent->getAttributes(); // Get embedded models form parent attributes. - $embedded = isset($attributes[$this->localKey]) ? (array) $attributes[$this->localKey] : null; + if (!isset($attributes[$this->localKey])) { + return null; + } + + if (is_array($attributes[$this->localKey])) { + return $attributes[$this->localKey]; + } - return $embedded; + return [$attributes[$this->localKey]]; } /** diff --git a/tests/RelationsTest.php b/tests/RelationsTest.php index 66c27583..758d05e2 100644 --- a/tests/RelationsTest.php +++ b/tests/RelationsTest.php @@ -217,8 +217,8 @@ public function testBelongsToMany(): void $client = Client::Where('name', '=', 'Buffet Bar Inc.')->first(); // Assert they are attached - $this->assertContains($client->_id, $user->client_ids); - $this->assertContains($user->_id, $client->user_ids); + $this->assertContainsObjectId($client->_id, $user->client_ids); + $this->assertContainsObjectId($user->_id, $client->user_ids); $this->assertCount(2, $user->clients); $this->assertCount(2, $client->users); @@ -348,8 +348,8 @@ public function testBelongsToManyCustom(): void $this->assertArrayHasKey('groups', $user->getAttributes()); // Assert they are attached - $this->assertContains($group->_id, $user->groups->pluck('_id')->toArray()); - $this->assertContains($user->_id, $group->users->pluck('_id')->toArray()); + $this->assertContainsObjectId($group->_id, $user->groups->pluck('_id')->toArray()); + $this->assertContainsObjectId($user->_id, $group->users->pluck('_id')->toArray()); $this->assertEquals($group->_id, $user->groups()->first()->_id); $this->assertEquals($user->_id, $group->users()->first()->_id); } diff --git a/tests/TestCase.php b/tests/TestCase.php index 51121d52..29781f14 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -11,10 +11,25 @@ use Jenssegers\Mongodb\MongodbServiceProvider; use Jenssegers\Mongodb\Tests\Models\User; use Jenssegers\Mongodb\Validation\ValidationServiceProvider; +use MongoDB\BSON\ObjectId; use Orchestra\Testbench\TestCase as OrchestraTestCase; class TestCase extends OrchestraTestCase { + public static function assertContainsObjectId(ObjectId $expectedObjectId, array $objectIds, string $message = ''): void + { + self::assertNotEmpty($objectIds, $message ?: 'Failed asserting that array of object ids is not empty.'); + + foreach ($objectIds as $objectId) { + if ($objectId == $expectedObjectId) { + // Found successfully + return; + } + } + + self::fail($message ?: sprintf('Failed asserting that ObjectId(%s) was found in [ObjectId(%s)].', $expectedObjectId, implode('), ObjectId(', $objectIds).')')); + } + /** * Get application providers. * From 33ce71e8f636f7d6e1eecacc1459f391b8bfa2ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Sat, 19 Aug 2023 00:06:01 +0200 Subject: [PATCH 5/5] cast --- src/Relations/BelongsToMany.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Relations/BelongsToMany.php b/src/Relations/BelongsToMany.php index 46a88cb4..5c7e0894 100644 --- a/src/Relations/BelongsToMany.php +++ b/src/Relations/BelongsToMany.php @@ -325,7 +325,7 @@ protected function formatSyncList(array $records) if (! is_array($attributes)) { [$id, $attributes] = [$attributes, []]; } - $results[$id] = $attributes; + $results[(string) $id] = $attributes; } return $results;