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

[BUG] hasMany and hasOne relation not working with ObjectId #2788

Open
florianJacques opened this issue Mar 21, 2024 · 3 comments
Open

[BUG] hasMany and hasOne relation not working with ObjectId #2788

florianJacques opened this issue Mar 21, 2024 · 3 comments

Comments

@florianJacques
Copy link
Contributor

Hello
The _id autocast is a problem on HasOne and HasMany relations, as it prevents comparisons with ObjectId

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

    if ($value instanceof Binary) {
        return (string) $value->getData();
    }

    return $value;
}

Indeed, this relationship always returns me null

public function accessToken(): HasOne
{
    return $this->hasOne(AccessToken::class, 'userId', '_id');
}

But if I remove the line

// Convert ObjectID to string.
if ($value instanceof ObjectID) {
    return (string) $value;
}

The relation working

@florianJacques
Copy link
Contributor Author

florianJacques commented Mar 21, 2024

I propose this solution to get around the problem without breaking the existing system.
What do you think?

protected bool $castKey = true;

public function getIdAttribute($value = null): mixed
{
    return $this->castKey ? parent::getIdAttribute($value) : $value;
}

public function accessTokens(): HasMany
  {
      $this->castKey = false;
      $hasMany =  $this->hasMany(AccessToken::class, 'userId', '_id');
      $this->castKey = true;

      return $hasMany;
  }

Unfortunately does not work with method....

I don't know why you put a default attribute mutator on the id, I wonder if it shouldn't be removed.

@masterbater
Copy link
Contributor

#2753 (comment)

I have a simple replication for this issue

@tarranjones
Copy link

tarranjones commented Jan 17, 2025

The only way I've found to stop _id from auto casting is to capture it before mutateAttribute strips the leading underscore.
By the time getIdAttribute is called we do not know which property was called id or _id.

The following patch will return a string for $mode->id and a ObjectID for $model->_id.
This fixes the hasOne hasMany issues without needing to specify additional parameters.

is_string($model->id); // true
$model->_id instanceof ObjectId; // true
<?php

declare(strict_types=1);

namespace App\MongoDB\Eloquent;

use MongoDB\BSON\ObjectId;
use MongoDB\Laravel\Eloquent\Model;

/**
 * Trait IdAttributePatch
 * @link https://github.com/mongodb/laravel-mongodb/issues/2753
 * @link https://github.com/mongodb/laravel-mongodb/issues/2788
 * @mixin Model
 * @property ObjectId $_id
 * @property string $id
 */
trait IdAttributePatch
{
    /**
     * Determine if a get mutator exists for an attribute.
     *
     * @param  string  $key
     * @return bool
     */
    public function hasGetMutator($key): bool
    {
        return $key === '_id' || parent::hasGetMutator($key);
    }

    /**
     * Get the value of an attribute using its mutator.
     *
     * @param  string  $key
     * @param  mixed  $value
     * @return mixed
     */
    protected function mutateAttribute($key, $value): mixed
    {
        if ($key === '_id') {
            // replicate getIdAttribute but without the (string) casting
            return $value ?: ($this->attributes['_id'] ?? $value);
        }
        return parent::mutateAttribute($key, $value);
    }

    public function getQueueableId(): string
    {
        return (string) parent::getKey();
    }
}
`

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants