Bug: Entity::hasChanged() is unreliable for casts

[rmilecki]

PHP Version

8.0

CodeIgniter4 Version

4.1.9

CodeIgniter4 Installation Method

Manual (zip or tar.gz)

Which operating systems have you tested for this bug?

Linux

Which server did you use?

cgi-fcgi

Database

No response

What happened?

The problem is that \CodeIgniter\Entity::hasChanged() may return true even if there isn't any data change.

This problem is caused by two facts:

1. Casting is performed only when reading properties
2. hasChanged() ignores all casts (it compares $original with $attributes directly, bypassing castAs())

Steps to Reproduce

/* This matches what \CodeIgniter\Database\BaseResult does */
$data = [
    'id' => '1',
    'name' => 'John',
    'age' => '35',
];
$entity = new class ($data) extends \CodeIgniter\Entity {
    protected $casts = [
        'id'   => 'integer',
        'name' => 'string',
        'age'  => 'integer',
    ];
};
$entity->syncOriginal();

var_dump($entity->age);
var_dump($entity->hasChanged());
$entity->age = 35;
var_dump($entity->age);
var_dump($entity->hasChanged());

Expected Output

int(35)
bool(false)
int(35)
bool(false)

Anything else?

Current output:

int(35)
bool(false)
int(35)
bool(true)

[kenjis]

@rmilecki Thank you for reporting.

#5678 indicates that if you use fill(), you need to pass the same typed value from the database.

Do you think this bug can be fixed?

[kenjis]

Ref #2441 (comment)

[kenjis]

\CodeIgniter\Entity is deprecated. Please use CodeIgniter\Entity\Entity class instead.

[kenjis]

Workaround:

<?php

namespace App\Controllers;

class Home extends BaseController
{
    public function index()
    {
        $data = [
            'id' => '1',
            'name' => 'John',
            'age' => '35',
        ];
        $entity = new class ($data) extends \CodeIgniter\Entity\Entity {
            protected $casts = [
                'id'   => 'integer',
                'name' => 'string',
                'age'  => 'integer',
            ];

            protected function setAge($value)
            {
                $this->attributes['age'] = (string) $value;
            }
        };
        $entity->syncOriginal();

        var_dump($entity->age);
        var_dump($entity->hasChanged());
        $entity->age = 35;
        var_dump($entity->age);
        var_dump($entity->hasChanged());
    }
}

[Patricio-Byte-Solution]

Hello how are you.
I think this problem would be solved if the hasChanged() method were rewritten as follows.

    
    public function hasChanged(?string $key = null) : bool
    {
        // If no parameter was given then check all attributes
        if ($key === null) {
            foreach ($this->attributes as $key => $value) {
                // It's a new element
                if (! array_key_exists($key, $this->original)) {
                    return true;
                }

                if ($this->hasChangedWithCast($key)) {
                    return true;
                }
            }
            return false;
        }
    
        // Key doesn't exist in either
        if (! array_key_exists($key, $this->original) && ! array_key_exists($key, $this->attributes)) {
            return false;
        }

        // It's a new element
        if (! array_key_exists($key, $this->original) && array_key_exists($key, $this->attributes)) {
            return true;
        }

        return $this->hasChangedWithCast($key);
    }

    public function hasChangedWithCast(string $key) : bool
    {
        $original   = $this->castAs($this->original[$key], $key, 'get');
        $actual     = $this->castAs($this->attributes[$key], $key, 'get');
        $changed    = false;

        if ($original !== $actual) {
            $changed = true;            
            // At the moment they are not the same.
            // If a property has a Cast defined.
            if (!empty($this->casts[$key])) {
                $type = $this->casts[$key];
                // If it is a numeric Cast
                if (in_array($type, ['int','float','double'])) {
                    // To compare numbers better convert them to strings.
                    if ((string)$original === (string)$actual) {
                        $changed = false;
                    }
                }
            }
        }
        return $changed;
    }

I created a class called BaseEntity that extends from Entity and in it I added this method.

[kenjis]

@Patricio-Byte-Solution Could you send a PR to review your code?

[iRedds]

Instead of adding a mutator to set a value or extending the InetegerCast class, or adding your own class to cast types, you change the logic of the hasChange() method. Why?

[Patricio-Byte-Solution]

Hi @iRedds.
The problem with that is that when an entity is loaded as a result of a database lookup, the convert handlers are not applied.

[iRedds]

I don't understand you.
From the database, you get all the data as a string.
So you need to convert the new value to a string.
As @kenjis showed.

protected function setAge($value)
{
   $this->attributes['age'] = (string) $value;
}

[kenjis]

@iRedds if we cast both values and then compare them, we can solve the problem of the type difference?

[iRedds]

@kenjis This will only work with primitive types.
For example, new A !== new A will return true even if the data (string) is the same.
Or (bool) 'false' === (bool) 'true'