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] 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 0d7816e..dd52bff 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 3961d23..6eebf4d 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 320f810..c999a2e 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 1b36150..22e2762 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 78cc833..f03c428 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); }