From eef66b9d620a35f945b8eecbeb6529a681ac00d2 Mon Sep 17 00:00:00 2001 From: Maher Al Ghoul Date: Thu, 24 Feb 2022 15:27:15 +0200 Subject: [PATCH 1/8] Fixed issue when trying to check if a multidimensional array is dirty attribute or not --- framework/db/BaseActiveRecord.php | 18 +++++++++++++++++- framework/helpers/BaseArrayHelper.php | 15 +++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/framework/db/BaseActiveRecord.php b/framework/db/BaseActiveRecord.php index 747ac1f28d9..a9a5d914dde 100644 --- a/framework/db/BaseActiveRecord.php +++ b/framework/db/BaseActiveRecord.php @@ -639,7 +639,7 @@ public function getDirtyAttributes($names = null) } } else { foreach ($this->_attributes as $name => $value) { - if (isset($names[$name]) && (!array_key_exists($name, $this->_oldAttributes) || $value !== $this->_oldAttributes[$name])) { + if (isset($names[$name]) && (!array_key_exists($name, $this->_oldAttributes) || $this->isAttributeDirty($name, $value))) { $attributes[$name] = $value; } } @@ -1760,4 +1760,20 @@ private function setRelationDependencies($name, $relation, $viaRelationName = nu $this->setRelationDependencies($name, $viaQuery, $viaRelationName); } } + + /** + * @param string $attribute + * @param mixed $value + * @return bool + */ + private function isAttributeDirty($attribute, $value) + { + $old_attribute = $this->oldAttributes[$attribute]; + if (is_array($value) && is_array($this->oldAttributes[$attribute])) { + ArrayHelper::sortMultidimensionalArray($value); + ArrayHelper::sortMultidimensionalArray($old_attribute); + } + + return $value !== $old_attribute; + } } diff --git a/framework/helpers/BaseArrayHelper.php b/framework/helpers/BaseArrayHelper.php index dcd28657c9e..a065a8ea3c4 100644 --- a/framework/helpers/BaseArrayHelper.php +++ b/framework/helpers/BaseArrayHelper.php @@ -999,4 +999,19 @@ public static function filter($array, $filters) return $result; } + + /** + * Sorts multidimensional array + * @param $array + * @return void + */ + public static function sortMultidimensionalArray(&$array) + { + ksort($array); + foreach ($array as &$item) { + if (is_array($item)) { + $item = self::sortMultidimensionalArray($item); + } + } + } } From 524d3bb08ed6ebfc6b3d316c4adc7b0f12fced95 Mon Sep 17 00:00:00 2001 From: Maher Al Ghoul Date: Sat, 26 Feb 2022 01:10:27 +0300 Subject: [PATCH 2/8] fixed issue in logic and added test --- framework/helpers/BaseArrayHelper.php | 13 +++++++++---- tests/framework/helpers/ArrayHelperTest.php | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/framework/helpers/BaseArrayHelper.php b/framework/helpers/BaseArrayHelper.php index a065a8ea3c4..93f6250bca2 100644 --- a/framework/helpers/BaseArrayHelper.php +++ b/framework/helpers/BaseArrayHelper.php @@ -7,9 +7,9 @@ namespace yii\helpers; -use ArrayAccess; -use Traversable; use Yii; +use Traversable; +use ArrayAccess; use yii\base\Arrayable; use yii\base\InvalidArgumentException; @@ -1007,10 +1007,15 @@ public static function filter($array, $filters) */ public static function sortMultidimensionalArray(&$array) { - ksort($array); + if (static::isIndexed($array)) { + sort($array); + } else { + ksort($array); + } + foreach ($array as &$item) { if (is_array($item)) { - $item = self::sortMultidimensionalArray($item); + self::sortMultidimensionalArray($item); } } } diff --git a/tests/framework/helpers/ArrayHelperTest.php b/tests/framework/helpers/ArrayHelperTest.php index 11f6ead433c..4cbc43aeea7 100644 --- a/tests/framework/helpers/ArrayHelperTest.php +++ b/tests/framework/helpers/ArrayHelperTest.php @@ -1436,6 +1436,24 @@ public function testArrayAccessWithMagicProperty() $this->assertEquals(42, ArrayHelper::getValue($model, 'magic')); $this->assertEquals('ta-da', ArrayHelper::getValue($model, 'moreMagic')); } + + public function testSortMultidimensionalArray() + { + // empty array + $empty_array = []; + ArrayHelper::sortMultidimensionalArray($empty_array); + $this->assertEquals([], $empty_array); + + // assoc array + $assoc_array = ['foo' => ['bar' => 1, 'baz' => 2]]; + ArrayHelper::sortMultidimensionalArray($assoc_array); + $this->assertEquals(['foo' => ['baz' => 2, 'bar' => 1]], $assoc_array); + + // index array + $index_array = [['bar' => 1], ['baz' => 2]]; + ArrayHelper::sortMultidimensionalArray($index_array); + $this->assertEquals([['baz' => 2], ['bar' => 1]], $index_array); + } } class Post1 From e69661c50d31ac7240ab5bfaebbb2e2eca84a3f8 Mon Sep 17 00:00:00 2001 From: Maher Al Ghoul Date: Sat, 26 Feb 2022 01:40:12 +0300 Subject: [PATCH 3/8] Updated test to only support PHP 7 and above --- tests/framework/helpers/ArrayHelperTest.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/framework/helpers/ArrayHelperTest.php b/tests/framework/helpers/ArrayHelperTest.php index 4cbc43aeea7..27ae7f3e8ef 100644 --- a/tests/framework/helpers/ArrayHelperTest.php +++ b/tests/framework/helpers/ArrayHelperTest.php @@ -1449,10 +1449,14 @@ public function testSortMultidimensionalArray() ArrayHelper::sortMultidimensionalArray($assoc_array); $this->assertEquals(['foo' => ['baz' => 2, 'bar' => 1]], $assoc_array); - // index array - $index_array = [['bar' => 1], ['baz' => 2]]; - ArrayHelper::sortMultidimensionalArray($index_array); - $this->assertEquals([['baz' => 2], ['bar' => 1]], $index_array); + if (PHP_VERSION_ID < 70000) { + $this->markTestSkipped('Can not be tested on PHP < 7.0'); + } else { + // index array + $index_array = [['bar' => 1], ['baz' => 2]]; + ArrayHelper::sortMultidimensionalArray($index_array); + $this->assertEquals([['baz' => 2], ['bar' => 1]], $index_array); + } } } From 1fccf01482888be76a066cf651c81054370e269d Mon Sep 17 00:00:00 2001 From: Maher Al Ghoul Date: Sun, 27 Feb 2022 10:57:11 +0300 Subject: [PATCH 4/8] Code refactoring --- framework/db/BaseActiveRecord.php | 4 +-- framework/helpers/BaseArrayHelper.php | 29 ++++++++++++--------- tests/framework/helpers/ArrayHelperTest.php | 8 +++--- 3 files changed, 23 insertions(+), 18 deletions(-) diff --git a/framework/db/BaseActiveRecord.php b/framework/db/BaseActiveRecord.php index a9a5d914dde..2e1ac43aa61 100644 --- a/framework/db/BaseActiveRecord.php +++ b/framework/db/BaseActiveRecord.php @@ -1770,8 +1770,8 @@ private function isAttributeDirty($attribute, $value) { $old_attribute = $this->oldAttributes[$attribute]; if (is_array($value) && is_array($this->oldAttributes[$attribute])) { - ArrayHelper::sortMultidimensionalArray($value); - ArrayHelper::sortMultidimensionalArray($old_attribute); + $value = ArrayHelper::recursiveSort($value); + $old_attribute = ArrayHelper::recursiveSort($old_attribute); } return $value !== $old_attribute; diff --git a/framework/helpers/BaseArrayHelper.php b/framework/helpers/BaseArrayHelper.php index 93f6250bca2..ebabad7e18a 100644 --- a/framework/helpers/BaseArrayHelper.php +++ b/framework/helpers/BaseArrayHelper.php @@ -1001,22 +1001,27 @@ public static function filter($array, $filters) } /** - * Sorts multidimensional array - * @param $array - * @return void + * Sorts array recursively. + * + * @param array $array An array passing by reference. + * @param callable|null $sorter The array sorter. If omitted, sort index array by values, sort assoc array by keys. + * @return array */ - public static function sortMultidimensionalArray(&$array) + public static function recursiveSort(array &$array, $sorter = null) { - if (static::isIndexed($array)) { - sort($array); - } else { - ksort($array); + foreach ($array as &$value) { + if (is_array($value)) { + self::recursiveSort($value, $sorter); + } } + unset($value); - foreach ($array as &$item) { - if (is_array($item)) { - self::sortMultidimensionalArray($item); - } + if ($sorter === null) { + $sorter = static::isIndexed($array) ? 'sort' : 'ksort'; } + + call_user_func_array($sorter, [&$array]); + + return $array; } } diff --git a/tests/framework/helpers/ArrayHelperTest.php b/tests/framework/helpers/ArrayHelperTest.php index 27ae7f3e8ef..9b86515885a 100644 --- a/tests/framework/helpers/ArrayHelperTest.php +++ b/tests/framework/helpers/ArrayHelperTest.php @@ -1437,16 +1437,16 @@ public function testArrayAccessWithMagicProperty() $this->assertEquals('ta-da', ArrayHelper::getValue($model, 'moreMagic')); } - public function testSortMultidimensionalArray() + public function testRecursiveSort() { // empty array $empty_array = []; - ArrayHelper::sortMultidimensionalArray($empty_array); + ArrayHelper::recursiveSort($empty_array); $this->assertEquals([], $empty_array); // assoc array $assoc_array = ['foo' => ['bar' => 1, 'baz' => 2]]; - ArrayHelper::sortMultidimensionalArray($assoc_array); + ArrayHelper::recursiveSort($assoc_array); $this->assertEquals(['foo' => ['baz' => 2, 'bar' => 1]], $assoc_array); if (PHP_VERSION_ID < 70000) { @@ -1454,7 +1454,7 @@ public function testSortMultidimensionalArray() } else { // index array $index_array = [['bar' => 1], ['baz' => 2]]; - ArrayHelper::sortMultidimensionalArray($index_array); + ArrayHelper::recursiveSort($index_array); $this->assertEquals([['baz' => 2], ['bar' => 1]], $index_array); } } From 33e15520e3a18e14d2193910f743ebb99fd5d46c Mon Sep 17 00:00:00 2001 From: Maher Al Ghoul Date: Thu, 3 Mar 2022 10:33:44 +0300 Subject: [PATCH 5/8] Updated Test Cases --- tests/framework/helpers/ArrayHelperTest.php | 80 ++++++++++++++++----- 1 file changed, 62 insertions(+), 18 deletions(-) diff --git a/tests/framework/helpers/ArrayHelperTest.php b/tests/framework/helpers/ArrayHelperTest.php index 9b86515885a..57ab1bf0fa1 100644 --- a/tests/framework/helpers/ArrayHelperTest.php +++ b/tests/framework/helpers/ArrayHelperTest.php @@ -1437,26 +1437,70 @@ public function testArrayAccessWithMagicProperty() $this->assertEquals('ta-da', ArrayHelper::getValue($model, 'moreMagic')); } - public function testRecursiveSort() + /** + * @dataProvider dataProviderRecursiveSort + * + * @return void + */ + public function testRecursiveSort($expected_result, $input_array) { - // empty array - $empty_array = []; - ArrayHelper::recursiveSort($empty_array); - $this->assertEquals([], $empty_array); - - // assoc array - $assoc_array = ['foo' => ['bar' => 1, 'baz' => 2]]; - ArrayHelper::recursiveSort($assoc_array); - $this->assertEquals(['foo' => ['baz' => 2, 'bar' => 1]], $assoc_array); + $actual = ArrayHelper::recursiveSort($input_array); + $this->assertEquals($expected_result, $actual); + } - if (PHP_VERSION_ID < 70000) { - $this->markTestSkipped('Can not be tested on PHP < 7.0'); - } else { - // index array - $index_array = [['bar' => 1], ['baz' => 2]]; - ArrayHelper::recursiveSort($index_array); - $this->assertEquals([['baz' => 2], ['bar' => 1]], $index_array); - } + /** + * Data provider for [[testRecursiveSort()]]. + * @return array test data + */ + public function dataProviderRecursiveSort() + { + return [ + //Normal index array + [ + [1, 2, 3, 4], + [4, 1, 3, 2] + ], + //Normal associative array + [ + ['a' => 1, 'b' => 2, 'c' => 3, 'd' => 4], + ['b' => 2, 'a' => 1, 'd' => 4, 'c' => 3], + ], + //Normal index array + [ + [1, 2, 3, 4], + [4, 1, 3, 2] + ], + //Multidimensional associative array + [ + [ + 'a' => ['a' => 1, 'b' => 2, 'c' => 3, 'd' => 4], + 'b' => ['a' => 1, 'b' => 2, 'c' => 3, 'd' => 4], + 'c' => ['a' => 1, 'b' => 2, 'c' => 3, 'd' => 4], + 'd' => ['a' => 1, 'b' => 2, 'c' => 3, 'd' => 4], + ], + [ + 'b' => ['a' => 1, 'd' => 4, 'b' => 2, 'c' => 3], + 'd' => ['b' => 2, 'c' => 3, 'a' => 1, 'd' => 4], + 'c' => ['c' => 3, 'a' => 1, 'd' => 4, 'b' => 2], + 'a' => ['d' => 4, 'b' => 2, 'c' => 3, 'a' => 1], + ], + ], + //Multidimensional associative array + [ + [ + 'a' => ['a' => 1, 'b' => 2, 'c' => 3, 'd' => ['a' => 1, 'b' => 2, 'c' => 3, 'd' => 4]], + 'b' => ['a' => 1, 'b' => 2, 'c' => ['a' => 1, 'b' => 2, 'c' => 3, 'd' => 4], 'd' => 4], + 'c' => ['a' => 1, 'b' => ['a' => 1, 'b' => 2, 'c' => 3, 'd' => 4], 'c' => 3, 'd' => 4], + 'd' => ['a' => ['a' => 1, 'b' => 2, 'c' => 3, 'd' => 4], 'b' => 2, 'c' => 3, 'd' => 4], + ], + [ + 'b' => ['a' => 1, 'd' => 4, 'b' => 2, 'c' => ['b' => 2, 'c' => 3, 'a' => 1, 'd' => 4]], + 'd' => ['b' => 2, 'c' => 3, 'a' => ['a' => 1, 'd' => 4, 'b' => 2, 'c' => 3], 'd' => 4], + 'c' => ['c' => 3, 'a' => 1, 'd' => 4, 'b' => ['c' => 3, 'a' => 1, 'd' => 4, 'b' => 2]], + 'a' => ['d' => ['d' => 4, 'b' => 2, 'c' => 3, 'a' => 1], 'b' => 2, 'c' => 3, 'a' => 1], + ] + ], + ]; } } From 4376edad57fd844be60716dc75f17daf5a113612 Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Sat, 16 Apr 2022 01:04:08 +0400 Subject: [PATCH 6/8] Update framework/helpers/BaseArrayHelper.php Co-authored-by: Bizley --- framework/helpers/BaseArrayHelper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework/helpers/BaseArrayHelper.php b/framework/helpers/BaseArrayHelper.php index ebabad7e18a..a968a562c6a 100644 --- a/framework/helpers/BaseArrayHelper.php +++ b/framework/helpers/BaseArrayHelper.php @@ -8,8 +8,8 @@ namespace yii\helpers; use Yii; -use Traversable; use ArrayAccess; +use Traversable; use yii\base\Arrayable; use yii\base\InvalidArgumentException; From 121cee18766b9899d9e9fb0821daa8862580a43d Mon Sep 17 00:00:00 2001 From: Maher Al Ghoul Date: Mon, 18 Apr 2022 12:10:53 +0300 Subject: [PATCH 7/8] Added to CHANGELOG.md --- framework/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md index b18fe5e0976..f83046eb0f1 100644 --- a/framework/CHANGELOG.md +++ b/framework/CHANGELOG.md @@ -4,6 +4,7 @@ Yii Framework 2 Change Log 2.0.46 under development ------------------------ +- Bug #19272: Fix bug in dirty attributes check on multidimensional array - Bug #19349: Fix PHP 8.1 error when attribute and label of `yii\grid\DataColumn` are empty (githubjeka) - Bug #19243: Handle `finfo_open` for tar.xz as `application/octet-stream` on PHP 8.1 (longthanhtran) - Bug #19235: Fix return type compatibility of `yii\web\SessionIterator` class methods for PHP 8.1 (virtual-designer) From 9c23fc8632ac51819ad4e5ee6cf236f2641f9a2c Mon Sep 17 00:00:00 2001 From: Bizley Date: Mon, 18 Apr 2022 11:42:59 +0200 Subject: [PATCH 8/8] Update CHANGELOG.md --- framework/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md index f83046eb0f1..748dfdba90e 100644 --- a/framework/CHANGELOG.md +++ b/framework/CHANGELOG.md @@ -4,7 +4,7 @@ Yii Framework 2 Change Log 2.0.46 under development ------------------------ -- Bug #19272: Fix bug in dirty attributes check on multidimensional array +- Bug #19272: Fix bug in dirty attributes check on multidimensional array (speedplli) - Bug #19349: Fix PHP 8.1 error when attribute and label of `yii\grid\DataColumn` are empty (githubjeka) - Bug #19243: Handle `finfo_open` for tar.xz as `application/octet-stream` on PHP 8.1 (longthanhtran) - Bug #19235: Fix return type compatibility of `yii\web\SessionIterator` class methods for PHP 8.1 (virtual-designer)