Framework: Model::is() gives false positive when models don't exist

[adamthehutt]

• Laravel Version: 5.8
• PHP Version: 7.3
• Database Driver & Version: MySQL 5.7

Description:

The Model::is($other) method is used to check if two model objects refer to the same database record. This works fine when both models exist and have been assigned primary keys. But if neither model has been persisted, it will always return true since null === null.

Steps To Reproduce:

$modelA = new Foo();
$modelB = new Foo();
var_dump($modelA->is($modelB));
// returns true but should return false

Proposed Fix:

Without primary keys to reference, there is no foolproof way to determine if the two model objects are really the same. However, we could make the test more accurate in this scenario by testing for identity with a === check in cases when the models don't exist.

    /**
     * Determine if two models have the same ID and belong to the same table.
     *
     * @param  \Illuminate\Database\Eloquent\Model|null  $model
     * @return bool
     */
    public function is($model)
    {
        return !$this->exists ? 
               $this === $model : 
               ! is_null($model) &&
               $this->getKey() === $model->getKey() &&
               $this->getTable() === $model->getTable() &&
               $this->getConnectionName() === $model->getConnectionName();
    }

[akiyamaSM]

"belong to the same table" does it include their existence or just they are both related to the same table?

[adamthehutt]

@akiyamaSM I'm not sure I understand your question. In both the existing and proposed versions of the method, the test includes checking for table/class, if that's what you mean.

[akiyamaSM]

It was a question about the description of the method, what they mean by "they blong to the same table".
do they mean there is a record already? or just that they are under the same of table. Hope I didn't make it worse.

[devcircus]

is() doesn't provide an answer to "does either model exist in the database", just that the model 'belongs' to the same table and has the same id(even if null). With Eloquent, all models belong to a table that is either specified on the model class or inferred from the name of the class.

$user = new User;
$user2 = new User;
$user->is($user2); //true

$user = new User;
$post = new Post;
$user->is($post); //false

You can use the exists property to check if the model is actually in the database.

$user = new User;
$user->exists; //false

$user = User::find(1);
$user->exists; //true (if indeed the 'find' call was successful.

[adamthehutt]

Yeah, I understand that. But is() should be testing to see if the two objects refer to the same record, which may or may not be the case, regardless of whether they've been persisted yet.

[akiyamaSM]

@adamthehutt I guess that's the key, if they are not yet persisted or at least one of them we can't compare, can we?

[adamthehutt]

Right, but at the very minimum it shouldn't be returning true automatically in that situation. I think testing for identity with === makes sense as a fallback.

[adamthehutt]

FWIW, the real solution to this problem is to implement an Identity Map for Eloquent, which would be amazingly useful.

[driesvints]

Since the PR was rejected I'm going to close this. As suggested, please do the check in userland.