From 05b572a65057394580d91013c1686790ccb7b4f4 Mon Sep 17 00:00:00 2001 From: kenjis Date: Thu, 21 Jul 2022 16:53:52 +0900 Subject: [PATCH 1/3] fix: Entity::hasChanged() is unreliable for casts --- system/Entity/Entity.php | 60 ++++++++++++++++++++++++++---- tests/system/Entity/EntityTest.php | 45 ++++++++++++++++++++++ 2 files changed, 98 insertions(+), 7 deletions(-) diff --git a/system/Entity/Entity.php b/system/Entity/Entity.php index 65898c012dc9..34876c5d7028 100644 --- a/system/Entity/Entity.php +++ b/system/Entity/Entity.php @@ -227,7 +227,7 @@ public function toRawArray(bool $onlyChanged = false, bool $recursive = false): } foreach ($this->attributes as $key => $value) { - if (! $this->hasChanged($key)) { + if (! $this->hasChangedAttributes($key)) { continue; } @@ -268,22 +268,45 @@ public function hasChanged(?string $key = null): bool { // If no parameter was given then check all attributes if ($key === null) { - return $this->original !== $this->attributes; + return $this->hasChangedAttributes(); } - $dbColumn = $this->mapProperty($key); + $key = $this->mapProperty($key); + + return $this->hasChangedAttributes($key); + } + + /** + * Checks a attribute to see if it has changed since the entity + * was created. Or, without a parameter, checks if any + * attributes have changed. + * + * @param string|null $key key of $this->attributes + */ + private function hasChangedAttributes(?string $key = null): bool + { + // If no parameter was given then check all attributes + if ($key === null) { + foreach ($this->attributes as $key => $value) { + if ($this->isChanged($key)) { + return true; + } + } + + return false; + } // Key doesn't exist in either - if (! array_key_exists($dbColumn, $this->original) && ! array_key_exists($dbColumn, $this->attributes)) { + if (! array_key_exists($key, $this->original) && ! array_key_exists($key, $this->attributes)) { return false; } // It's a new element - if (! array_key_exists($dbColumn, $this->original) && array_key_exists($dbColumn, $this->attributes)) { + if (! array_key_exists($key, $this->original) && array_key_exists($key, $this->attributes)) { return true; } - return $this->original[$dbColumn] !== $this->attributes[$dbColumn]; + return $this->isChanged($key); } /** @@ -526,7 +549,6 @@ public function __get(string $key) // If a "`get` + $key" method exists, it is also a getter. $result = $this->{$method}(); } - // Otherwise return the protected property // if it exists. elseif (array_key_exists($dbColumn, $this->attributes)) { @@ -545,6 +567,30 @@ public function __get(string $key) return $result; } + /** + * Get cast value from the data array. + * + * @return mixed|null + */ + private function _getCastData(string $key, array $data) + { + $result = null; + + if (array_key_exists($key, $data)) { + $result = $this->castAs($data[$key], $key); + } + + return $result; + } + + /** + * Check if the key value is changed. + */ + private function isChanged(string $key): bool + { + return $this->_getCastData($key, $this->original) !== $this->_getCastData($key, $this->attributes); + } + /** * Returns true if a property exists names $key, or a getter method * exists named like for __get(). diff --git a/tests/system/Entity/EntityTest.php b/tests/system/Entity/EntityTest.php index dadea32b5680..dd1d865fe1a8 100644 --- a/tests/system/Entity/EntityTest.php +++ b/tests/system/Entity/EntityTest.php @@ -1032,6 +1032,51 @@ public function testHasChangedMappedChanged(): void $this->assertTrue($entity->hasChanged('createdAt')); } + /** + * @see https://github.com/codeigniter4/CodeIgniter4/issues/5905 + */ + public function testHasChangedCastsItem() + { + $data = [ + 'id' => '1', + 'name' => 'John', + 'age' => '35', + ]; + $entity = new class ($data) extends \CodeIgniter\Entity { + protected $casts = [ + 'id' => 'integer', + 'name' => 'string', + 'age' => 'integer', + ]; + }; + $entity->syncOriginal(); + + $entity->age = 35; + + $this->assertFalse($entity->hasChanged('age')); + } + + public function testHasChangedCastsWholeEntity() + { + $data = [ + 'id' => '1', + 'name' => 'John', + 'age' => '35', + ]; + $entity = new class ($data) extends \CodeIgniter\Entity { + protected $casts = [ + 'id' => 'integer', + 'name' => 'string', + 'age' => 'integer', + ]; + }; + $entity->syncOriginal(); + + $entity->age = 35; + + $this->assertFalse($entity->hasChanged()); + } + public function testHasChangedWholeEntity(): void { $entity = $this->getEntity(); From 68683c076ca1c51b00db544c96ed45815c140220 Mon Sep 17 00:00:00 2001 From: kenjis Date: Thu, 21 Jul 2022 17:10:49 +0900 Subject: [PATCH 2/3] refactor: run rector --- system/Entity/Entity.php | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/system/Entity/Entity.php b/system/Entity/Entity.php index 34876c5d7028..a92314faed71 100644 --- a/system/Entity/Entity.php +++ b/system/Entity/Entity.php @@ -287,7 +287,7 @@ private function hasChangedAttributes(?string $key = null): bool { // If no parameter was given then check all attributes if ($key === null) { - foreach ($this->attributes as $key => $value) { + foreach (array_keys($this->attributes) as $key) { if ($this->isChanged($key)) { return true; } @@ -574,13 +574,11 @@ public function __get(string $key) */ private function _getCastData(string $key, array $data) { - $result = null; - if (array_key_exists($key, $data)) { - $result = $this->castAs($data[$key], $key); + return $this->castAs($data[$key], $key); } - return $result; + return null; } /** From 2136183a6dc883261f869c00e39732e1a14221a3 Mon Sep 17 00:00:00 2001 From: kenjis Date: Thu, 21 Jul 2022 18:09:11 +0900 Subject: [PATCH 3/3] docs: fix PHPDoc types --- system/Entity/Entity.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/Entity/Entity.php b/system/Entity/Entity.php index a92314faed71..6687f6c577c4 100644 --- a/system/Entity/Entity.php +++ b/system/Entity/Entity.php @@ -570,7 +570,7 @@ public function __get(string $key) /** * Get cast value from the data array. * - * @return mixed|null + * @return array|bool|float|int|object|string|null */ private function _getCastData(string $key, array $data) {