diff --git a/.github/workflows/test-unit.yml b/.github/workflows/test-unit.yml index b89a342af..5db9b2594 100644 --- a/.github/workflows/test-unit.yml +++ b/.github/workflows/test-unit.yml @@ -161,6 +161,7 @@ jobs: if [ -n "$LOG_COVERAGE" ]; then mv coverage/phpunit.cov coverage/phpunit-sqlite.cov; fi - name: "Run tests: MySQL - PDO" + if: success() || failure() env: DB_DSN: "pdo_mysql:host=mysql;dbname=atk4_test" DB_USER: atk4_test_user @@ -170,6 +171,7 @@ jobs: if [ -n "$LOG_COVERAGE" ]; then mv coverage/phpunit.cov coverage/phpunit-mysql-pdo.cov; fi - name: "Run tests: MySQL - mysqli" + if: success() || failure() env: DB_DSN: "mysqli:host=mysql;dbname=atk4_test" DB_USER: atk4_test_user @@ -179,6 +181,7 @@ jobs: if [ -n "$LOG_COVERAGE" ]; then mv coverage/phpunit.cov coverage/phpunit-mysql-mysqli.cov; fi - name: "Run tests: MySQL 5.6" + if: success() || failure() env: DB_DSN: "mysql:host=mysql56;dbname=atk4_test" DB_USER: atk4_test_user @@ -188,6 +191,7 @@ jobs: if [ -n "$LOG_COVERAGE" ]; then mv coverage/phpunit.cov coverage/phpunit-mysql56.cov; fi - name: "Run tests: MariaDB" + if: success() || failure() env: DB_DSN: "mysql:host=mariadb;dbname=atk4_test" DB_USER: atk4_test_user @@ -197,6 +201,7 @@ jobs: if [ -n "$LOG_COVERAGE" ]; then mv coverage/phpunit.cov coverage/phpunit-mariadb.cov; fi - name: "Run tests: PostgreSQL" + if: success() || failure() env: DB_DSN: "pgsql:host=postgres;dbname=atk4_test" DB_USER: atk4_test_user @@ -206,6 +211,7 @@ jobs: if [ -n "$LOG_COVERAGE" ]; then mv coverage/phpunit.cov coverage/phpunit-postgres.cov; fi - name: "Run tests: MSSQL" + if: success() || failure() env: DB_DSN: "sqlsrv:host=mssql;dbname=master;driverOptions[TrustServerCertificate]=1" DB_USER: sa @@ -215,7 +221,7 @@ jobs: if [ -n "$LOG_COVERAGE" ]; then mv coverage/phpunit.cov coverage/phpunit-mssql.cov; fi - name: "Run tests: Oracle - PDO (only for coverage or cron)" - if: env.LOG_COVERAGE || github.event_name == 'schedule' + if: (success() || failure()) && env.LOG_COVERAGE || github.event_name == 'schedule' env: DB_DSN: "pdo_oci:dbname=oracle/xe" DB_USER: system @@ -226,6 +232,7 @@ jobs: if [ -n "$LOG_COVERAGE" ]; then mv coverage/phpunit.cov coverage/phpunit-oracle-pdo.cov; fi - name: "Run tests: Oracle - OCI8" + if: success() || failure() env: DB_DSN: "oci8:dbname=oracle/xe" DB_USER: system diff --git a/src/Field.php b/src/Field.php index bb5eb334f..831726829 100644 --- a/src/Field.php +++ b/src/Field.php @@ -187,8 +187,10 @@ public function normalize($value) } else { throw new Exception('Must not be boolean type'); } - } elseif (is_scalar($value)) { + } elseif (is_int($value)) { $value = (string) $value; + } elseif (is_float($value)) { + $value = Expression::castFloatToString($value); } else { throw new Exception('Must be scalar'); } @@ -364,7 +366,12 @@ private function getValueForCompare($value): ?string return null; } - return (string) $this->typecastSaveField($value, true); + $res = $this->typecastSaveField($value, true); + if (is_float($res)) { + return Expression::castFloatToString($res); + } + + return (string) $res; } /** diff --git a/src/Model/Scope/Condition.php b/src/Model/Scope/Condition.php index d9d920d0a..9efdf38d6 100644 --- a/src/Model/Scope/Condition.php +++ b/src/Model/Scope/Condition.php @@ -395,8 +395,10 @@ protected function valueToWords(Model $model, $value): string if (is_bool($value)) { $valueStr = $value ? 'true' : 'false'; - } elseif (is_int($value) || is_float($value)) { - $valueStr = $value; + } elseif (is_int($value)) { + $valueStr = (string) $value; + } elseif (is_float($value)) { + $valueStr = Expression::castFloatToString($value); } else { $valueStr = '\'' . (string) $value . '\''; } diff --git a/src/Persistence/Sql/Expression.php b/src/Persistence/Sql/Expression.php index 06d06b3c7..5862153b4 100644 --- a/src/Persistence/Sql/Expression.php +++ b/src/Persistence/Sql/Expression.php @@ -385,15 +385,15 @@ function ($matches) use (&$nameless_count) { $identifier = substr($matches[0], 1, -1); - $escaping = null; + $escapeMode = null; if (substr($matches[0], 0, 1) === '[') { - $escaping = self::ESCAPE_PARAM; + $escapeMode = self::ESCAPE_PARAM; } elseif (substr($matches[0], 0, 1) === '{') { if (substr($matches[0], 1, 1) === '{') { - $escaping = self::ESCAPE_IDENTIFIER_SOFT; + $escapeMode = self::ESCAPE_IDENTIFIER_SOFT; $identifier = substr($identifier, 1, -1); } else { - $escaping = self::ESCAPE_IDENTIFIER; + $escapeMode = self::ESCAPE_IDENTIFIER; } } @@ -406,7 +406,7 @@ function ($matches) use (&$nameless_count) { $fx = '_render_' . $identifier; if (array_key_exists($identifier, $this->args['custom'])) { - $value = $this->consume($this->args['custom'][$identifier], $escaping); + $value = $this->consume($this->args['custom'][$identifier], $escapeMode); } elseif (method_exists($this, $fx)) { $value = $this->{$fx}(); } else { @@ -456,8 +456,10 @@ public function getDebugQuery(): string $replacement = 'NULL'; } elseif (is_bool($val)) { $replacement = $val ? '1' : '0'; - } elseif (is_int($val) || is_float($val)) { + } elseif (is_int($val)) { $replacement = (string) $val; + } elseif (is_float($val)) { + $replacement = self::castFloatToString($val); } elseif (is_string($val)) { $replacement = '\'' . addslashes($val) . '\''; } else { @@ -541,15 +543,15 @@ public function execute(object $connection = null): DbalResult return $connection->execute($this); } - [$query, $params] = $this->updateRenderBeforeExecute($this->render()); + [$sql, $params] = $this->updateRenderBeforeExecute($this->render()); $platform = $this->connection->getDatabasePlatform(); try { - $statement = $connection->prepare($query); + $statement = $connection->prepare($sql); foreach ($params as $key => $val) { - if (is_int($val)) { - $type = ParameterType::INTEGER; + if ($val === null) { + $type = ParameterType::NULL; } elseif (is_bool($val)) { if ($platform instanceof PostgreSQLPlatform) { $type = ParameterType::STRING; @@ -558,9 +560,10 @@ public function execute(object $connection = null): DbalResult $type = ParameterType::INTEGER; $val = $val ? 1 : 0; } - } elseif ($val === null) { - $type = ParameterType::NULL; + } elseif (is_int($val)) { + $type = ParameterType::INTEGER; } elseif (is_float($val)) { + $val = self::castFloatToString($val); $type = ParameterType::STRING; } elseif (is_string($val)) { $type = ParameterType::STRING; @@ -619,15 +622,31 @@ public function execute(object $connection = null): DbalResult // {{{ Result Querying + /** + * Cast float to string with lossless precision. + */ + final public static function castFloatToString(float $value): string + { + $precisionBackup = ini_get('precision'); + ini_set('precision', '-1'); + try { + return (string) $value; + } finally { + ini_set('precision', $precisionBackup); + } + } + /** * @param string|int|float|bool|null $v */ private function castGetValue($v): ?string { - if (is_int($v) || is_float($v)) { - return (string) $v; - } elseif (is_bool($v)) { + if (is_bool($v)) { return $v ? '1' : '0'; + } elseif (is_int($v)) { + return (string) $v; + } elseif (is_float($v)) { + return self::castFloatToString($v); } // for PostgreSQL/Oracle CLOB/BLOB datatypes and PDO driver diff --git a/src/Persistence/Sql/Mssql/Query.php b/src/Persistence/Sql/Mssql/Query.php index 2fd431264..f28d3823d 100644 --- a/src/Persistence/Sql/Mssql/Query.php +++ b/src/Persistence/Sql/Mssql/Query.php @@ -14,13 +14,24 @@ class Query extends BaseQuery protected $expression_class = Expression::class; - protected $template_insert = 'begin try' - . "\n" . 'insert[option] into [table_noalias] ([set_fields]) values ([set_values])' - . "\n" . 'end try begin catch if ERROR_NUMBER() = 544 begin' - . "\n" . 'set IDENTITY_INSERT [table_noalias] on' - . "\n" . 'insert[option] into [table_noalias] ([set_fields]) values ([set_values])' - . "\n" . 'set IDENTITY_INSERT [table_noalias] off' - . "\n" . 'end end catch'; + protected $template_insert = <<<'EOF' + begin try + insert[option] into [table_noalias] ([set_fields]) values ([set_values]); + end try begin catch + if ERROR_NUMBER() = 544 begin + set IDENTITY_INSERT [table_noalias] on; + begin try + insert[option] into [table_noalias] ([set_fields]) values ([set_values]); + set IDENTITY_INSERT [table_noalias] off; + end try begin catch + set IDENTITY_INSERT [table_noalias] off; + throw; + end catch + end else begin + throw; + end + end catch + EOF; public function _render_limit(): ?string { diff --git a/src/Persistence/Sql/Oracle/ExpressionTrait.php b/src/Persistence/Sql/Oracle/ExpressionTrait.php index 430df7063..70621af2e 100644 --- a/src/Persistence/Sql/Oracle/ExpressionTrait.php +++ b/src/Persistence/Sql/Oracle/ExpressionTrait.php @@ -6,7 +6,7 @@ trait ExpressionTrait { - protected function castLongStringToClobExpr(string $value): Expression + protected function convertLongStringToClobExpr(string $value): Expression { $exprArgs = []; $buildConcatExprFx = function (array $parts) use (&$buildConcatExprFx, &$exprArgs): string { @@ -50,7 +50,7 @@ protected function updateRenderBeforeExecute(array $render): array function ($matches) use ($params, &$newParams, &$newParamBase) { $value = $params[$matches[0]]; if (is_string($value) && strlen($value) > 4000) { - $expr = $this->castLongStringToClobExpr($value); + $expr = $this->convertLongStringToClobExpr($value); unset($value); [$exprSql, $exprParams] = $expr->render(); $sql = preg_replace_callback( diff --git a/src/Persistence/Sql/Query.php b/src/Persistence/Sql/Query.php index 18872e1d4..7df84bd12 100644 --- a/src/Persistence/Sql/Query.php +++ b/src/Persistence/Sql/Query.php @@ -651,7 +651,7 @@ protected function _sub_render_condition(array $row): string return '1 = 1'; // always true } - $value = '(' . implode(', ', array_map(function ($v) { return $this->escapeParam($v); }, $value)) . ')'; + $value = '(' . implode(', ', array_map(function ($v) { return $this->consume($v); }, $value)) . ')'; return $field . ' ' . $cond . ' ' . $value; } diff --git a/src/Persistence/Static_.php b/src/Persistence/Static_.php index 8f4916fde..4f77c430a 100644 --- a/src/Persistence/Static_.php +++ b/src/Persistence/Static_.php @@ -72,14 +72,14 @@ public function __construct(array $data = null) } // try to detect type of field by its value - if (is_int($value)) { - $def_types[] = ['type' => 'integer']; - } elseif ($value instanceof \DateTime) { - $def_types[] = ['type' => 'datetime']; - } elseif (is_bool($value)) { + if (is_bool($value)) { $def_types[] = ['type' => 'boolean']; + } elseif (is_int($value)) { + $def_types[] = ['type' => 'integer']; } elseif (is_float($value)) { $def_types[] = ['type' => 'float']; + } elseif ($value instanceof \DateTimeInterface) { + $def_types[] = ['type' => 'datetime']; } elseif (is_array($value)) { $def_types[] = ['type' => 'json']; } elseif (is_object($value)) { diff --git a/src/Schema/TestCase.php b/src/Schema/TestCase.php index e32b63f74..31913e91e 100644 --- a/src/Schema/TestCase.php +++ b/src/Schema/TestCase.php @@ -249,18 +249,20 @@ public function setDb(array $dbData, bool $importData = true): void } } - $first_row = current($data); + $firstRow = current($data); $idColumnName = null; - if ($first_row) { - $idColumnName = isset($first_row['_id']) ? '_id' : 'id'; + if ($firstRow) { + $idColumnName = isset($firstRow['_id']) ? '_id' : 'id'; $migrator->id($idColumnName); - foreach ($first_row as $field => $row) { + foreach ($firstRow as $field => $row) { if ($field === $idColumnName) { continue; } - if (is_int($row)) { + if (is_bool($row)) { + $fieldType = 'boolean'; + } elseif (is_int($row)) { $fieldType = 'integer'; } elseif (is_float($row)) { $fieldType = 'float'; @@ -278,23 +280,25 @@ public function setDb(array $dbData, bool $importData = true): void // import data if ($importData) { - $hasId = (bool) key($data); + $this->db->atomic(function () use ($tableName, $data, $idColumnName) { + $hasId = array_key_first($data) !== 0; - foreach ($data as $id => $row) { - $query = $this->db->dsql(); - if ($id === '_') { - continue; - } + foreach ($data as $id => $row) { + $query = $this->db->dsql(); + if ($id === '_') { + continue; + } - $query->table($tableName); - $query->setMulti($row); + $query->table($tableName); + $query->setMulti($row); - if (!isset($row[$idColumnName]) && $hasId) { - $query->set($idColumnName, $id); - } + if (!isset($row[$idColumnName]) && $hasId) { + $query->set($idColumnName, $id); + } - $query->mode('insert')->execute(); - } + $query->mode('insert')->execute(); + } + }); } } } diff --git a/src/ValidationException.php b/src/ValidationException.php index 380962faa..6dd54c5ef 100644 --- a/src/ValidationException.php +++ b/src/ValidationException.php @@ -25,7 +25,7 @@ public function __construct(array $errors, Model $model = null, $intent = null) if (count($errors) === 1) { parent::__construct(reset($errors)); - $this->addMoreInfo('field', key($errors)); + $this->addMoreInfo('field', array_key_first($errors)); } else { parent::__construct('Multiple unhandled validation errors'); $this->addMoreInfo('errors', $errors) diff --git a/tests/FolderTest.php b/tests/FolderTest.php index ae6652ced..5831cb843 100644 --- a/tests/FolderTest.php +++ b/tests/FolderTest.php @@ -33,27 +33,27 @@ public function testRate(): void { $this->setDb([ 'folder' => [ - ['parent_id' => 1, 'is_deleted' => 0, 'name' => 'Desktop'], - ['parent_id' => 1, 'is_deleted' => 0, 'name' => 'My Documents'], - ['parent_id' => 1, 'is_deleted' => 0, 'name' => 'My Videos'], - ['parent_id' => 1, 'is_deleted' => 0, 'name' => 'My Projects'], - ['parent_id' => 4, 'is_deleted' => 0, 'name' => 'Agile Data'], - ['parent_id' => 4, 'is_deleted' => 0, 'name' => 'DSQL'], - ['parent_id' => 4, 'is_deleted' => 0, 'name' => 'Agile Toolkit'], - ['parent_id' => 4, 'is_deleted' => 1, 'name' => 'test-project'], + ['parent_id' => 1, 'is_deleted' => false, 'name' => 'Desktop'], + ['parent_id' => 1, 'is_deleted' => false, 'name' => 'My Documents'], + ['parent_id' => 1, 'is_deleted' => false, 'name' => 'My Videos'], + ['parent_id' => 1, 'is_deleted' => false, 'name' => 'My Projects'], + ['parent_id' => 4, 'is_deleted' => false, 'name' => 'Agile Data'], + ['parent_id' => 4, 'is_deleted' => false, 'name' => 'DSQL'], + ['parent_id' => 4, 'is_deleted' => false, 'name' => 'Agile Toolkit'], + ['parent_id' => 4, 'is_deleted' => true, 'name' => 'test-project'], ], ]); $f = new Folder($this->db); $f = $f->load(4); - $this->assertEquals([ + $this->assertSame([ 'id' => 4, 'name' => 'My Projects', - 'count' => 3, + 'count' => '3', 'parent_id' => 1, 'parent' => 'Desktop', - 'is_deleted' => 0, + 'is_deleted' => false, ], $f->get()); } } diff --git a/tests/Schema/ModelTest.php b/tests/Schema/ModelTest.php index a03d610ba..9d6c37b4b 100644 --- a/tests/Schema/ModelTest.php +++ b/tests/Schema/ModelTest.php @@ -241,7 +241,7 @@ public function providerCharacterTypeFieldLongData(): array // is broken with long strings, oci8 driver is NOT affected, // CI images ghcr.io/mvorisek/image-php are patched // remove comment once https://github.com/php/php-src/pull/8018 is merged & released - ['text', false, 256 * 1024], + ['text', false, str_starts_with($_ENV['DB_DSN'], 'pdo_oci') && !isset($_ENV['CI']) ? 16 * 1024 : 256 * 1024], ['blob', true, 256 * 1024], ]; } diff --git a/tests/TypecastingTest.php b/tests/TypecastingTest.php index 1e9119202..42b21efdb 100644 --- a/tests/TypecastingTest.php +++ b/tests/TypecastingTest.php @@ -8,6 +8,7 @@ use Atk4\Data\Model; use Atk4\Data\Schema\TestCase; use Doctrine\DBAL\Platforms\OraclePlatform; +use Doctrine\DBAL\Platforms\SqlitePlatform; class TypecastingTest extends TestCase { @@ -37,10 +38,10 @@ public function testType(): void 'date' => '2013-02-20', 'datetime' => '2013-02-20 20:00:12.000000', 'time' => '12:00:50.000000', - 'boolean' => 1, + 'boolean' => true, 'integer' => '2940', 'money' => '8.20', - 'float' => '8.202343', + 'float' => 8.20234376757473, 'json' => '[1,2,3]', ], ], @@ -69,7 +70,7 @@ public function testType(): void $this->assertEquals(new \DateTime('1970-01-01 12:00:50'), $mm->get('time')); $this->assertSame(2940, $mm->get('integer')); $this->assertSame([1, 2, 3], $mm->get('json')); - $this->assertSame(8.202343, $mm->get('float')); + $this->assertSame(8.20234376757473, $mm->get('float')); $m->createEntity()->setMulti(array_diff_key($mm->get(), ['id' => true]))->save(); @@ -84,7 +85,7 @@ public function testType(): void 'boolean' => 1, 'integer' => 2940, 'money' => 8.2, - 'float' => 8.202343, + 'float' => 8.20234376757473, 'json' => '[1,2,3]', ], 2 => [ @@ -96,7 +97,7 @@ public function testType(): void 'boolean' => '1', 'integer' => '2940', 'money' => '8.2', - 'float' => '8.202343', + 'float' => 8.20234376757473, 'json' => '[1,2,3]', ], ], @@ -109,6 +110,15 @@ public function testType(): void unset($duplicate['id']); $this->assertEquals($first, $duplicate); + + $m->load(2)->set('float', 8.20234376757474)->save(); + $this->assertSame(8.20234376757474, $m->load(2)->get('float')); + $m->load(2)->set('float', 8.202343767574732)->save(); + // pdo_sqlite in truncating float, see https://github.com/php/php-src/issues/8510 + // fixed since PHP 8.1, but if converted in SQL to string explicitly, the result is still wrong + if (!$this->getDatabasePlatform() instanceof SqlitePlatform || \PHP_VERSION_ID >= 80100) { + $this->assertSame(8.202343767574732, $m->load(2)->get('float')); + } } public function testEmptyValues(): void @@ -239,11 +249,11 @@ public function testTypeCustom1(): void 'date' => '2013-02-20', 'datetime' => '2013-02-20 20:00:12.235689', 'time' => '12:00:50.235689', - 'b1' => '1', - 'b2' => '0', + 'b1' => true, + 'b2' => false, 'integer' => '2940', 'money' => '8.20', - 'float' => '8.202343', + 'float' => 8.20234376757473, ], ], ]; @@ -277,6 +287,7 @@ public function testTypeCustom1(): void $m->delete(1); unset($dbData['types'][0]); + $row['b2'] = '0'; // fix false == '0', see https://github.com/sebastianbergmann/phpunit/issues/4967, remove once fixed $row['money'] = '8.2'; // here it will loose last zero and that's as expected $dbData['types'][2] = array_merge(['id' => '2'], $row);