Skip to content

Commit

Permalink
Skip useless duplicates in query order render (#1141)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mvorisek
mvorisek authored Oct 14, 2023
1 parent 609ea70 commit 6fe8b48
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 14 deletions.
6 changes: 4 additions & 2 deletions src/Field.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}

Expand Down Expand Up @@ -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)));
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/Persistence/Sql/Query.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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)));
}

// }}}
Expand Down
4 changes: 3 additions & 1 deletion tests/ConditionSqlTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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'));
}

Expand Down
45 changes: 36 additions & 9 deletions tests/FieldTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -96,6 +110,7 @@ public function testRequiredStringEmptyException(): void
$m = $m->createEntity();

$this->expectException(ValidationException::class);
$this->expectExceptionMessage('Must not be empty');
$m->set('foo', '');
}

Expand All @@ -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', '');
}

Expand All @@ -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');
}

Expand Down Expand Up @@ -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']);
}

Expand All @@ -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' => '']);
}

Expand All @@ -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]);
}

Expand Down Expand Up @@ -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']);
Expand All @@ -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');
}

Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}

Expand All @@ -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');
}

Expand Down Expand Up @@ -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');
}

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}

Expand Down
16 changes: 16 additions & 0 deletions tests/Persistence/Sql/WithDb/SelectTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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']);
}
}
1 change: 1 addition & 0 deletions tests/SerializeTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]]);
}

Expand Down

0 comments on commit 6fe8b48

Please sign in to comment.