Skip to content
This repository has been archived by the owner on Aug 22, 2023. It is now read-only.

PHPORM-8 Remove automatic _id conversion, apply Cast to queries #23

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 55 additions & 8 deletions src/Eloquent/Builder.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
GromNaN marked this conversation as resolved.
Show resolved Hide resolved
{
return parent::chunkById($count, $callback, $column, $alias);
}

/**
* @inheritdoc
*/
Expand Down Expand Up @@ -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
*/
Expand All @@ -241,4 +267,25 @@ protected function ensureOrderForCursorPagination($shouldReverse = false)
];
})->values();
}

public function whereKey($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);
}
}
30 changes: 24 additions & 6 deletions src/Eloquent/Casts/ObjectId.php
Original file line number Diff line number Diff line change
Expand Up @@ -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<BSONObjectId, BSONObjectId>
*/
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
Expand All @@ -19,28 +27,38 @@ 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') {
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ IMPORTANT ⚠️

The ObjectId cast should not be used with anything else than ObjectId key type. Otherwise there is a mismatch in the type when using Model->getKey() to query the database.

This impacts queries using the query builder and relations.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This must be revisited to support SQL<->MongoDB relations.

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)
{
if ($value instanceof BSONObjectId) {
return $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 new BSONObjectId($value);
return [$key => $value];
}
}
117 changes: 61 additions & 56 deletions src/Eloquent/Model.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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.
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a feature that needs to be kept: Allow to use a custom 'id' field as id.

// 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
*/
Expand Down Expand Up @@ -171,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);
}

Expand All @@ -190,12 +175,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);
Expand All @@ -209,28 +189,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
*/
Expand Down Expand Up @@ -291,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);
}

/**
Expand Down Expand Up @@ -502,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
*/
Expand Down Expand Up @@ -568,4 +546,31 @@ protected function addCastAttributesToArray(array $attributes, array $mutatedAtt

return $attributes;
}

/** @internal */
public function castKeyForDatabase($value)
{
$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 $attributes[$key];
}

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);
}
}
Loading
Loading