Skip useless duplicates in query order render

src/Field.php

@@ -199,7 +199,9 @@ public function normalize($value)
             $value = $this->normalizeUsingTypecast($value);
 
             if ($value === null) {
-                if (!$this->nullable || $this->required) {
+                if ($this->required) {
+                    throw new Exception('Must not be empty');
+                } elseif (!$this->nullable) {
                     throw new Exception('Must not be null');
                 }
 
@@ -264,7 +266,7 @@ public function normalize($value)
             } elseif ($this->values) {
                 if ($value === '') {
                     $value = null;
-                } elseif ((!is_string($value) && !is_int($value)) || !array_key_exists($value, $this->values)) {
+                } elseif ((!is_string($value) && !is_int($value)) || !isset($this->values[$value])) {
                     throw new Exception('Value is not one of the allowed values: ' . implode(', ', array_keys($this->values)));
                 }
             }

src/Persistence/Sql/Query.php

@@ -490,7 +490,7 @@ public function having($field, $cond = null, $value = null)
      *
      * @param string $kind 'where' or 'having'
      *
-     * @return array<int, string>
+     * @return list<string>
      */
     protected function _subrenderWhere($kind): array
     {
@@ -880,7 +880,7 @@ protected function _renderOrder(): ?string
             $x[] = $this->consume($arg, self::ESCAPE_IDENTIFIER_SOFT) . ($desc ? (' ' . $desc) : '');
         }
 
-        return ' order by ' . implode(', ', array_reverse($x));
+        return ' order by ' . implode(', ', array_unique(array_reverse($x)));
     }
 
     // }}}

tests/ConditionSqlTest.php

@@ -7,6 +7,7 @@
 use Atk4\Data\Exception;
 use Atk4\Data\Model;
 use Atk4\Data\Schema\TestCase;
+use Atk4\Data\ValidationException;
 
 class ConditionSqlTest extends TestCase
 {
@@ -366,7 +367,8 @@ public function testDateConditionFailure(): void
         $m->addField('name');
         $m->addField('date', ['type' => 'date']);
 
-        $this->expectException(Exception::class);
+        $this->expectException(ValidationException::class);
+        $this->expectExceptionMessage('Must be scalar');
         $m->tryLoadBy('name', new \DateTime('08-12-1982'));
     }
 

tests/FieldTest.php

@@ -72,10 +72,12 @@ public function testNotNullableNullException(): void
         $m = new Model();
         $m->addField('foo', ['nullable' => false]);
         $m = $m->createEntity();
+
         $m->set('foo', 'abc');
         $m->set('foo', '');
 
         $this->expectException(ValidationException::class);
+        $this->expectExceptionMessage('Must not be null');
         $m->set('foo', null);
     }
 
@@ -86,6 +88,18 @@ public function testRequiredNullException(): void
         $m = $m->createEntity();
 
         $this->expectException(ValidationException::class);
+        $this->expectExceptionMessage('Must not be empty');
+        $m->set('foo', null);
+    }
+
+    public function testNotNullableAndRequiredNullException(): void
+    {
+        $m = new Model();
+        $m->addField('foo', ['nullable' => false, 'required' => true]);
+        $m = $m->createEntity();
+
+        $this->expectException(ValidationException::class);
+        $this->expectExceptionMessage('Must not be empty');
         $m->set('foo', null);
     }
 
@@ -96,6 +110,7 @@ public function testRequiredStringEmptyException(): void
         $m = $m->createEntity();
 
         $this->expectException(ValidationException::class);
+        $this->expectExceptionMessage('Must not be empty');
         $m->set('foo', '');
     }
 
@@ -108,6 +123,7 @@ public function testRequiredBinaryEmptyException(): void
         $m->set('foo', '0');
 
         $this->expectException(ValidationException::class);
+        $this->expectExceptionMessage('Must not be empty');
         $m->set('foo', '');
     }
 
@@ -118,6 +134,7 @@ public function testRequiredStringZeroException(): void
         $m = $m->createEntity();
 
         $this->expectException(ValidationException::class);
+        $this->expectExceptionMessage('Must not be empty');
         $m->set('foo', '0');
     }
 
@@ -159,6 +176,7 @@ public function testNotNullableNullInsertException(): void
         $m->addField('surname');
 
         $this->expectException(Exception::class);
+        $this->expectExceptionMessage('Must not be null');
         $m->insert(['surname' => 'qq']);
     }
 
@@ -174,7 +192,8 @@ public function testRequiredStringEmptyInsertException(): void
         $m->addField('name', ['required' => true]);
         $m->addField('surname');
 
-        $this->expectException(Exception::class);
+        $this->expectException(ValidationException::class);
+        $this->expectExceptionMessage('Must not be empty');
         $m->insert(['surname' => 'qq', 'name' => '']);
     }
 
@@ -191,7 +210,8 @@ public function testNotNullableNullLoadException(): void
         $m->addField('surname');
         $m = $m->load(1);
 
-        $this->expectException(Exception::class);
+        $this->expectException(ValidationException::class);
+        $this->expectExceptionMessage('Must not be null');
         $m->save(['name' => null]);
     }
 
@@ -262,17 +282,18 @@ public function testCaption(): void
         );
     }
 
-    public function testReadOnly1(): void
+    public function testReadOnlyException(): void
     {
         $m = new Model();
         $m->addField('foo', ['readOnly' => true]);
         $m = $m->createEntity();
 
         $this->expectException(Exception::class);
+        $this->expectExceptionMessage('Attempting to change read-only field');
         $m->set('foo', 'bar');
     }
 
-    public function testReadOnly2(): void
+    public function testReadOnlySetSameValue(): void
     {
         $m = new Model();
         $m->addField('foo', ['readOnly' => true, 'default' => 'abc']);
@@ -287,7 +308,8 @@ public function testEnum1(): void
         $m->addField('foo', ['enum' => ['foo', 'bar']]);
         $m = $m->createEntity();
 
-        $this->expectException(Exception::class);
+        $this->expectException(ValidationException::class);
+        $this->expectExceptionMessage('Value is not one of the allowed values: foo, bar');
         $m->set('foo', 'xx');
     }
 
@@ -323,7 +345,7 @@ public function testEnum3(): void
         $m->addField('foo', ['enum' => [1, 'bar']]);
         $m = $m->createEntity();
 
-        $this->expectException(Exception::class);
+        $this->expectException(ValidationException::class);
         $this->expectExceptionMessage('Must not be boolean type');
         $m->set('foo', true);
     }
@@ -344,7 +366,8 @@ public function testValues1(): void
         $m->addField('foo', ['values' => ['foo', 'bar']]);
         $m = $m->createEntity();
 
-        $this->expectException(Exception::class);
+        $this->expectException(ValidationException::class);
+        $this->expectExceptionMessage('Value is not one of the allowed values: 0, 1');
         $m->set('foo', 4);
     }
 
@@ -367,7 +390,8 @@ public function testValues3(): void
         $m->addField('foo', ['values' => [1 => 'bar']]);
         $m = $m->createEntity();
 
-        $this->expectException(Exception::class);
+        $this->expectException(ValidationException::class);
+        $this->expectExceptionMessage('Value is not one of the allowed values: 1');
         $m->set('foo', 'bar');
     }
 
@@ -484,6 +508,7 @@ public function testNonExistingField(): void
         $m = $m->createEntity();
 
         $this->expectException(Exception::class);
+        $this->expectExceptionMessage('Field is not defined');
         $m->set('baz', 'bar');
     }
 
@@ -844,7 +869,8 @@ public function testSetNull(): void
         self::assertNull($m->get('b'));
         self::assertNull($m->get('c'));
 
-        $this->expectException(Exception::class);
+        $this->expectException(ValidationException::class);
+        $this->expectExceptionMessage('Must not be empty');
         $m->set('c', null);
     }
 
@@ -878,6 +904,7 @@ public function testEntityFieldPair(): void
         self::assertNull($entityBarField->get());
 
         $this->expectException(Exception::class);
+        $this->expectExceptionMessage('Must not be null');
         $entityBarField->set(null);
     }
 

tests/Persistence/Sql/WithDb/SelectTest.php

@@ -451,6 +451,20 @@ public function testImportAndAutoincrement(): void
         ], $m->export());
     }
 
+    public function testOrderDuplicate(): void
+    {
+        $query = $this->q('employee')->field('name')
+            ->order('id')
+            ->order('name')
+            ->order('surname')
+            ->order('name');
+
+        self::assertSame(
+            [['name' => 'Charlie'], ['name' => 'Harry'], ['name' => 'Jack'], ['name' => 'Oliver']],
+            $query->getRows()
+        );
+    }
+
     public function testSubqueryWithOrderAndLimit(): void
     {
         $subQuery = $this->q('employee');
@@ -477,5 +491,7 @@ public function testSubqueryWithOrderAndLimit(): void
             [['name' => 'Charlie'], ['name' => 'Harry'], ['name' => 'Jack'], ['name' => 'Oliver']],
             $query->getRows()
         );
+
+        self::assertSame([['surname', 'desc']], $subQuery->args['order']);
     }
 }

tests/SerializeTest.php

@@ -66,6 +66,7 @@ public function testSerializeErrorJson2(): void
         $dbData[] = &$dbData;
 
         $this->expectException(Exception::class);
+        $this->expectExceptionMessage('Typecast save error: Could not convert PHP type \'array\' to \'json\', as an \'Recursion detected\'');
         $this->db->typecastSaveRow($m, ['data' => ['foo' => 'bar', 'recursive' => $dbData]]);
     }