[9.x] Loose comparison causes the value not to be saved

[JBtje]

The issue has been addressed in #37961.

Given an attribute with the cast object or collection, the $model->isDirty() method returns the wrong value due to the use of loose comparison.

Example from issue #37961:

use Illuminate\Database\Eloquent\Model;

class Test extends Model
{
    protected $casts = [
        'collection' => 'collection',
    ];
    protected $fillable = [
        'collection',
    ];
}

$model = new Test(['collection' => ['key' => true]]);
$model->syncOriginal();
$model->fill(['collection' => ['key' => 'value']]);

$model->isDirty(); // false but should be true

// or
$model = new Test(['collection' => ['key' => null]]);
$model->syncOriginal();
$model->fill(['collection' => ['key' => false]]);

$model->isDirty(); // false but should be true

The solution proposed by @krnsptr link is in my opinion the best solution.

Furthermore, casting the attributes to object/collection prior to checking the values, makes no sense to me. We are interested in knowing if the content differs, the cast prior to this check does not provide useful additional information (only causes that you can't use strict comparison due to object being always different even with the same content). The cast also adds overhead.

[taylorotwell]

Here is what I think people will complain about here, and I believe this has been attempted in the past. An array with the same key / value pairs but the key / value pairs are in a different "order" is considered equal when using ==, but not equal when using ===:

[JBtje]

Personally, I stand with PHP on this, and think a change in order means that the value is no longer the same and should be updated.

Implementing it as is, the worst-case scenario would be an extra UPDATE and the updated_at attribute obtains a new timestamp due to the order change. Though, this can easily be solved by keeping the same order in the array… whereas the issue at hand only has less nice solutions.

If the order thing is a big deal, it would be possible to add a recursive ksort() in the check if the values don't match using strict comparison.

[taylorotwell]

I agree that the current situation seems a little worse than the change in behavior introduced by this PR. Will think about it over the weekend.

[iamgergo]

I've also met with this problem. Could it be a solution to use Arr::dot() and keep loose comparison at the same time? In that case nested array will be flattened and it should work, I guess.

Arr::dot($this->fromJson($attribute)) == Arr::dot($this->fromJson($original));

[JBtje]

I've also met with this problem. Could it be a solution to use Arr::dot() and keep loose comparison at the same time? In that case nested array will be flattened and it should work, I guess.

Arr::dot does not solve the problem at hand.

$attribute = [
  'b' => [
    'c' => 1
  ]
];
$original = [
  'b' => [
    'c' => true
  ]
];

Arr::dot($attribute) == Arr::dot($original);  // Returns true, should be false.

Since loose comparison is the cause of the problem

[iamgergo]

I see. Another solution that crossed my mind:

$a = Arr::dot($this->fromJson($attribute));
$o = Arr::dot($this->fromJson($original));

count($a) === count($o) && empty(array_diff_assoc($a, $o));

Note, comparing the count of the array elements is necessary, because let's say if $a is just a subset of $o (because some items were removed, but others were not changed), the array_diff_assoc will return an empty array.

What do you think about that solution?

---

Okay, I see what do you mean. This solution won't work because 1 will be handled as true while it should not be.

[JBtje]

Okay, I see what do you mean. This solution won't work because 1 will be handled as true while it should not be.

Yup, same for all other "loose" vs "strict" comparisons differences https://www.php.net/manual/en/types.comparisons.php

[DarkGhostHunter]

Can this be fixed by retrieving the items as an array, using ksort(), and then compare strictly?

[JBtje]

Can this be fixed by retrieving the items as an array, using ksort(), and then compare strictly?

Yes that would be possible (when using recursive ksort()). But it would also give extra overhead and the behaviour would deviate from L1957-L1958.

[tpetry]

I had to resolve this problem in my application too by patching the dirty logic.

Here is what I think people will complain about here, and I believe this has been attempted in the past. An array with the same key / value pairs but the key / value pairs are in a different "order" is considered equal when using ==, but not equal when using ===:

For PHP a different ordering will result in a === check being false, but the database acknowledges that these two JSON objects are the same despite it's ordering:

select CAST('{"a": 1, "b":2}' as JSON) = CAST('{"b": 2, "a":1}' as JSON) -- Result: 1

I've solved the problem in a backwards-compatible way by recursively sorting a collection/json array when array_is_list is false, which means the resulting variable will be a json object in the hierarchy.

[DarkGhostHunter]

May be this will need a flag or sort in the Model:

protected $strictComparison = ['my-array'].

This way users can opt-in when dealing with comparing model attributes.

[MasterRO94]

This PR breaks BC. If original $value is null then PHP Deprecated: json_decode(): Passing null to parameter #1 ($json) of type string is deprecated is thrown