Fix sort configuration and criteria preparation

CHANGELOG.md

@@ -9,6 +9,8 @@
   the data reader (@vjik)
 - New #153: Add `Compare::withValue()` method (@vjik)
 - Chg #154: Raise minimum required PHP version to 8.1 (@vjik)
+- Bug #155: Fix `Sort` configuration preparation (@vjik)
+- Bug #155: Fix same named order fields in `Sort` were not overriding previous ones (@vjik)
 
 ## 1.0.1 January 25, 2023
 

src/Reader/Sort.php

@@ -41,7 +41,12 @@
  * @psalm-type TSortFieldItem = array<string, int>
  * @psalm-type TConfigItem = array{asc: TSortFieldItem, desc: TSortFieldItem, default: "asc"|"desc"}
  * @psalm-type TConfig = array<string, TConfigItem>
- * @psalm-type TUserConfig = array<int, string>|array<string, array<string, int|string>>
+ * @psalm-type TUserConfigItem = array{
+ *     asc?: int|"asc"|"desc"|array<string, int|"asc"|"desc">,
+ *     desc?: int|"asc"|"desc"|array<string, int|"asc"|"desc">,
+ *     default?: "asc"|"desc"
+ * }
+ * @psalm-type TUserConfig = array<int, string>|array<string, TUserConfigItem>
  */
 final class Sort
 {
@@ -81,17 +86,43 @@ private function __construct(private bool $ignoreExtraFields, array $config)
                 throw new InvalidArgumentException('Invalid config format.');
             }
 
-            if (is_string($fieldConfig)) {
+            if (is_int($fieldName)) {
+                /** @var string $fieldConfig */
                 $fieldName = $fieldConfig;
                 $fieldConfig = [];
+            } else {
+                /** @psalm-var TUserConfigItem $fieldConfig */
+                foreach ($fieldConfig as $key => &$criteria) {
+                    // 'default' => 'asc' or 'desc'
+                    if ($key === 'default') {
+                        continue;
+                    }
+                    // 'asc'/'desc' => SORT_*
+                    if (is_int($criteria)) {
+                        continue;
+                    }
+                    // 'asc'/'desc' => 'asc' or 'asc'/'desc' => 'desc'
+                    if (is_string($criteria)) {
+                        $criteria = [$fieldName => $criteria === 'desc' ? SORT_DESC : SORT_ASC];
+                        continue;
+                    }
+                    // 'asc'/'desc' => ['field' => SORT_*|'asc'|'desc']
+                    foreach ($criteria as &$subCriteria) {
+                        if (is_string($subCriteria)) {
+                            $subCriteria = $subCriteria === 'desc' ? SORT_DESC : SORT_ASC;
+                        }
+                    }
+                }
             }
 
-            /** @psalm-var TConfig $fieldConfig */
-            $normalizedConfig[$fieldName] = array_merge([
-                'asc' => [$fieldName => SORT_ASC],
-                'desc' => [$fieldName => SORT_DESC],
-                'default' => 'asc',
-            ], $fieldConfig);
+            $normalizedConfig[$fieldName] = array_merge(
+                [
+                    'asc' => [$fieldName => SORT_ASC],
+                    'desc' => [$fieldName => SORT_DESC],
+                    'default' => 'asc',
+                ],
+                $fieldConfig,
+            );
         }
 
         /** @psalm-var TConfig $normalizedConfig */
@@ -272,7 +303,7 @@ public function getOrderAsString(): string
      * when obtaining the data i.e. a list of real fields along with their order directions.
      *
      * @return array Sorting criteria.
-     * @psalm-return array<string, "asc"|"desc"|int>
+     * @psalm-return array<string, int>
      */
     public function getCriteria(): array
     {
@@ -281,13 +312,13 @@ public function getCriteria(): array
 
         foreach ($this->currentOrder as $field => $direction) {
             if (array_key_exists($field, $config)) {
-                $criteria += $config[$field][$direction];
+                $criteria = array_merge($criteria, $config[$field][$direction]);
                 unset($config[$field]);
             } else {
                 if ($this->ignoreExtraFields) {
                     continue;
                 }
-                $criteria += [$field => $direction];
+                $criteria = array_merge($criteria, [$field => $direction === 'desc' ? SORT_DESC : SORT_ASC]);
             }
         }
 

tests/Reader/SortTest.php

@@ -5,6 +5,7 @@
 namespace Yiisoft\Data\Tests\Reader;
 
 use InvalidArgumentException;
+use PHPUnit\Framework\Attributes\DataProvider;
 use Yiisoft\Data\Reader\Sort;
 use Yiisoft\Data\Tests\TestCase;
 
@@ -117,12 +118,18 @@ public function testOnlyModeGetCriteriaWithEmptyConfig(): void
 
     public function testAnyModeGetCriteriaWithEmptyConfig(): void
     {
-        $sort = Sort::any([])->withOrder([
+        $sort = Sort::any()->withOrder([
             'a' => 'desc',
             'b' => 'asc',
         ]);
 
-        $this->assertSame(['a' => 'desc', 'b' => 'asc'], $sort->getCriteria());
+        $this->assertSame(
+            [
+                'a' => SORT_DESC,
+                'b' => SORT_ASC,
+            ],
+            $sort->getCriteria(),
+        );
     }
 
     public function testGetCriteria(): void
@@ -158,10 +165,13 @@ public function testAnyModeGetCriteriaWhenAnyFieldConflictsWithConfig(): void
             ],
         ])->withOrderString('-bee,b,a,-foo');
 
-        $this->assertSame([
-            'bee' => 'desc',
-            'foo' => 'asc',
-        ], $sort->getCriteria());
+        $this->assertSame(
+            [
+                'bee' => SORT_ASC,
+                'foo' => SORT_DESC,
+            ],
+            $sort->getCriteria(),
+        );
     }
 
     public function testGetCriteriaDefaults(): void
@@ -265,4 +275,62 @@ public function testIgnoreExtraFields(): void
             $sort->getCriteria()
         );
     }
+
+    public static function dataPrepareConfig(): array
+    {
+        return [
+            [
+                [
+                    'a' => SORT_ASC,
+                    'b' => SORT_DESC,
+                ],
+                [
+                    'a',
+                    'b' => ['asc' => 'desc'],
+                ],
+            ],
+            [
+                [
+                    'a' => SORT_ASC,
+                ],
+                [
+                    'a' => [
+                        'asc' => 'desc',
+                        'desc' => 'asc',
+                        'default' => 'desc',
+                    ],
+                ],
+            ],
+            [
+                [
+                    'a' => SORT_ASC,
+                ],
+                [
+                    'a' => [
+                        'asc' => SORT_DESC,
+                        'desc' => 'asc',
+                        'default' => 'desc',
+                    ],
+                ],
+            ],
+            [
+                [
+                    'x' => SORT_ASC,
+                ],
+                [
+                    'a' => [
+                        'default' => 'desc',
+                        'desc' => ['x' => 'asc'],
+                    ],
+                ],
+            ],
+        ];
+    }
+
+    #[DataProvider('dataPrepareConfig')]
+    public function testPrepareConfig(array $expected, array $config): void
+    {
+        $sort = Sort::only($config);
+        $this->assertSame($expected, $sort->getCriteria());
+    }
 }