Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Move generic normalize exceptions to typecasting #1142

Merged
merged 4 commits into from
Oct 24, 2023
Merged

Move generic normalize exceptions to typecasting #1142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ae0a20f
Move generic normalize errors to typecasting
mvorisek Oct 24, 2023
c986c5e
skip only save typecast msg
mvorisek Oct 24, 2023
8a339ba
do not strip quotes from numbers in typecast
mvorisek Oct 24, 2023
049d612
move pretypecast to overrideable methods
mvorisek Oct 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 4 additions & 46 deletions src/Field.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,52 +146,6 @@ public function normalize($value)
case 'text':
$value = rtrim(preg_replace('~\r?\n|\r~', "\n", $value)); // normalize line-ends to LF and rtrim

break;
case 'boolean':
case 'integer':
$value = preg_replace('~\s+|[,`\']~', '', $value);

break;
case 'float':
case 'decimal':
case 'atk4_money':
$value = preg_replace('~\s+|[`\']|,(?=.*\.)~', '', $value);

break;
}

switch ($this->type) {
case 'boolean':
case 'integer':
case 'float':
case 'decimal':
case 'atk4_money':
if ($value === '') {
$value = null;
} elseif (!is_numeric($value)) {
throw new Exception('Must be numeric');
}

break;
}
} elseif ($value !== null) {
switch ($this->type) {
case 'string':
case 'text':
case 'integer':
case 'float':
case 'decimal':
case 'atk4_money':
if (is_bool($value)) {
throw new Exception('Must not be boolean type');
} elseif (is_int($value)) {
$value = (string) $value;
} elseif (is_float($value)) {
$value = Expression::castFloatToString($value);
} else {
throw new Exception('Must be scalar');
}

break;
}
}
Expand Down Expand Up @@ -278,6 +232,10 @@ public function normalize($value)
$messages[] = $e->getMessage();
} while ($e = $e->getPrevious());

if (count($messages) >= 2 && $messages[0] === 'Typecast save error') {
array_shift($messages);
}

throw (new ValidationException([$this->shortName => implode(': ', $messages)], $this->issetOwner() ? $this->getOwner() : null))
->addMoreInfo('field', $this);
}
Expand Down
69 changes: 69 additions & 0 deletions src/Persistence.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,71 @@ public function typecastLoadRow(Model $model, array $row): array
return $result;
}

/**
* @param mixed $value
*
* @return ($value is scalar ? scalar : mixed)
*/
private function _typecastPreField(Field $field, $value, bool $fromLoad)
{
if (is_string($value)) {
switch ($field->type) {
case 'boolean':
case 'integer':
$value = preg_replace('~\s+|,~', '', $value);

break;
case 'float':
case 'decimal':
case 'atk4_money':
$value = preg_replace('~\s+|,(?=.*\.)~', '', $value);

break;
}

switch ($field->type) {
case 'boolean':
case 'integer':
case 'float':
case 'decimal':
case 'atk4_money':
if ($value === '') {
$value = null;
} elseif (!is_numeric($value)) {
throw new Exception('Must be numeric');
}

break;
}
} elseif ($value !== null) {
switch ($field->type) {
case 'string':
case 'text':
case 'integer':
case 'float':
case 'decimal':
case 'atk4_money':
if (is_bool($value)) {
throw new Exception('Must not be bool type');
} elseif (is_int($value)) {
if ($fromLoad) {
$value = (string) $value;
}
} elseif (is_float($value)) {
if ($fromLoad) {
$value = Persistence\Sql\Expression::castFloatToString($value);
}
} else {
throw new Exception('Must be scalar');
}

break;
}
}

return $value;
}

/**
* Prepare value of a specific field by converting it to
* persistence-friendly format.
Expand Down Expand Up @@ -403,6 +468,8 @@ public function typecastLoadField(Field $field, $value)
*/
protected function _typecastSaveField(Field $field, $value)
{
$value = $this->_typecastPreField($field, $value, false);

if (in_array($field->type, ['json', 'object'], true) && $value === '') { // TODO remove later
return null;
}
Expand Down Expand Up @@ -445,6 +512,8 @@ protected function _typecastSaveField(Field $field, $value)
*/
protected function _typecastLoadField(Field $field, $value)
{
$value = $this->_typecastPreField($field, $value, true);

// TODO casting optionally to null should be handled by type itself solely
if ($value === '' && in_array($field->type, ['boolean', 'integer', 'float', 'decimal', 'datetime', 'date', 'time', 'json', 'object'], true)) {
return null;
Expand Down
6 changes: 3 additions & 3 deletions tests/ConditionSqlTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ public function testDateCondition(): void
$m->addField('name');
$m->addField('date', ['type' => 'date']);

$m = $m->loadBy('date', new \DateTime('08-12-1982'));
$m = $m->loadBy('date', new \DateTime('1982-12-08'));
self::assertSame('Sue', $m->get('name'));
}

Expand All @@ -349,7 +349,7 @@ public function testDateCondition2(): void
$m->addField('name');
$m->addField('date', ['type' => 'date']);

$m->addCondition('date', new \DateTime('08-12-1982'));
$m->addCondition('date', new \DateTime('1982-12-08'));
$m = $m->loadOne();
self::assertSame('Sue', $m->get('name'));
}
Expand All @@ -369,7 +369,7 @@ public function testDateConditionFailure(): void

$this->expectException(ValidationException::class);
$this->expectExceptionMessage('Must be scalar');
$m->tryLoadBy('name', new \DateTime('08-12-1982'));
$m->tryLoadBy('name', new \DateTime('1982-12-08'));
}

public function testAndFromArrayCondition(): void
Expand Down
97 changes: 32 additions & 65 deletions tests/FieldTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ public function testEnum3(): void
$m = $m->createEntity();

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

Expand Down Expand Up @@ -646,7 +646,7 @@ public function testNormalize(): void
self::assertTrue($m->get('boolean'));
}

public function testNormalizeException1(): void
public function testNormalizeStringArrayException(): void
{
$m = new Model();
$m->addField('foo', ['type' => 'string']);
Expand All @@ -657,117 +657,95 @@ public function testNormalizeException1(): void
$m->set('foo', []);
}

public function testNormalizeException2(): void
public function testNormalizeStringBoolException(): void
{
$m = new Model();
$m->addField('foo', ['type' => 'text']);
$m->addField('foo', ['type' => 'string']);
$m = $m->createEntity();

$this->expectException(ValidationException::class);
$this->expectExceptionMessage('Must be scalar');
$m->set('foo', []);
$this->expectExceptionMessageMatches('~^Must not be bool type$~');
$m->set('foo', false);
}

public function testNormalizeException3(): void
public function testNormalizeIntegerNumericException(): void
{
$m = new Model();
$m->addField('foo', ['type' => 'integer']);
$m = $m->createEntity();

$this->expectException(ValidationException::class);
$this->expectExceptionMessage('Must be scalar');
$m->set('foo', []);
$this->expectExceptionMessage('Must be numeric');
$m->set('foo', '1x');
}

public function testNormalizeException4(): void
public function testNormalizeFloatNumericException(): void
{
$m = new Model();
$m->addField('foo', ['type' => 'atk4_money']);
$m->addField('foo', ['type' => 'float']);
$m = $m->createEntity();

$this->expectException(ValidationException::class);
$this->expectExceptionMessage('Must be scalar');
$m->set('foo', []);
$this->expectExceptionMessage('Must be numeric');
$m->set('foo', '1x');
}

public function testNormalizeException5(): void
public function testNormalizeAtk4MoneyNumericException(): void
{
$m = new Model();
$m->addField('foo', ['type' => 'float']);
$m->addField('foo', ['type' => 'atk4_money']);
$m = $m->createEntity();

$this->expectException(ValidationException::class);
$this->expectExceptionMessage('Must be scalar');
$m->set('foo', []);
$this->expectExceptionMessage('Must be numeric');
$m->set('foo', '1x');
}

public function testNormalizeException6(): void
public function testNormalizeBooleanNumericException(): void
{
$m = new Model();
$m->addField('foo', ['type' => 'date']);
$m->addField('foo', ['type' => 'boolean']);
$m = $m->createEntity();

$this->expectException(ValidationException::class);
$this->expectExceptionMessage('Must be instance of DateTimeInterface');
$m->set('foo', []);
$this->expectExceptionMessage('Must be numeric');
$m->set('foo', '1x');
}

public function testNormalizeException7(): void
public function testNormalizeDateException(): void
{
$m = new Model();
$m->addField('foo', ['type' => 'datetime']);
$m->addField('foo', ['type' => 'date']);
$m = $m->createEntity();

$this->expectException(ValidationException::class);
$this->expectExceptionMessage('Must be instance of DateTimeInterface');
$m->set('foo', []);
$m->set('foo', '2000-01-01');
}

public function testNormalizeException8(): void
public function testNormalizeTimeException(): void
{
$m = new Model();
$m->addField('foo', ['type' => 'time']);
$m = $m->createEntity();

$this->expectException(ValidationException::class);
$this->expectExceptionMessage('Must be instance of DateTimeInterface');
$m->set('foo', []);
$m->set('foo', '20:00:00');
}

public function testNormalizeException9(): void
public function testNormalizeDatetimeException(): void
{
$m = new Model();
$m->addField('foo', ['type' => 'integer']);
$m = $m->createEntity();

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

public function testNormalizeException10(): void
{
$m = new Model();
$m->addField('foo', ['type' => 'atk4_money']);
$m = $m->createEntity();

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

public function testNormalizeException11(): void
{
$m = new Model();
$m->addField('foo', ['type' => 'float']);
$m->addField('foo', ['type' => 'datetime']);
$m = $m->createEntity();

$this->expectException(ValidationException::class);
$this->expectExceptionMessage('Must be numeric');
$m->set('foo', '123---456');
$this->expectExceptionMessage('Must be instance of DateTimeInterface');
$m->set('foo', '2000-01-01 20:00:00');
}

public function testNormalizeException12(): void
public function testNormalizeJsonException(): void
{
$m = new Model();
$m->addField('foo', ['type' => 'json']);
Expand All @@ -778,7 +756,7 @@ public function testNormalizeException12(): void
$m->set('foo', 'ABC');
}

public function testNormalizeException13(): void
public function testNormalizeObjectException(): void
{
$m = new Model();
$m->addField('foo', ['type' => 'object']);
Expand All @@ -789,17 +767,6 @@ public function testNormalizeException13(): void
$m->set('foo', 'ABC');
}

public function testNormalizeException14(): void
{
$m = new Model();
$m->addField('foo', ['type' => 'boolean']);
$m = $m->createEntity();

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

public function testAddFieldDirectly(): void
{
$model = new Model();
Expand Down
2 changes: 1 addition & 1 deletion tests/SerializeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +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->expectExceptionMessage('Could not convert PHP type \'array\' to \'json\', as an \'Recursion detected\'');
$this->db->typecastSaveRow($m, ['data' => ['foo' => 'bar', 'recursive' => $dbData]]);
}

Expand Down