diff --git a/docs/persistence/sql/expressions.rst b/docs/persistence/sql/expressions.rst index 680c182ff..2a6f91bca 100644 --- a/docs/persistence/sql/expressions.rst +++ b/docs/persistence/sql/expressions.rst @@ -299,21 +299,6 @@ parts of the query. You must not call them in normal circumstances. $query->consume('first_name'); // `first_name` $query->consume($other_query); // will merge parameters and return string -.. php:method:: escape($value) - - Creates new expression where $value appears escaped. Use this method as a - conventional means of specifying arguments when you think they might have - a nasty back-ticks or commas in the field names. I generally **discourage** - you from using this method. Example use would be:: - - $query->field('foo, bar'); // escapes and adds 2 fields to the query - $query->field($query->escape('foo, bar')); // adds field `foo, bar` to the query - $query->field(['foo, bar']); // adds single field `foo, bar` - - $query->order('foo desc'); // escapes and add `foo` desc to the query - $query->field($query->escape('foo desc')); // adds field `foo desc` to the query - $query->field(['foo desc']); // adds `foo` desc anyway - .. php:method:: escapeIdentifier($sql_code) Always surrounds `$sql code` with back-ticks. diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 3952d4cea..cc78f8240 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -14,7 +14,8 @@ - src + src + tests diff --git a/src/Persistence/Sql.php b/src/Persistence/Sql.php index 68a76c769..81d9f6931 100644 --- a/src/Persistence/Sql.php +++ b/src/Persistence/Sql.php @@ -168,28 +168,23 @@ protected function initPersistence(Model $model): void /** * Creates new Expression object from expression string. - * - * @param mixed $expr */ - public function expr(Model $model, $expr, array $args = []): Expression + public function expr(Model $model, string $template, array $arguments = []): Expression { - if (!is_string($expr)) { - return $this->getConnection()->expr($expr, $args); - } preg_replace_callback( '~\[\w*\]|\{\w*\}~', - function ($matches) use (&$args, $model) { + function ($matches) use ($model, &$arguments) { $identifier = substr($matches[0], 1, -1); - if ($identifier && !isset($args[$identifier])) { - $args[$identifier] = $model->getField($identifier); + if ($identifier !== '' && !isset($arguments[$identifier])) { + $arguments[$identifier] = $model->getField($identifier); } return $matches[0]; }, - $expr + $template ); - return $this->getConnection()->expr($expr, $args); + return $this->getConnection()->expr($template, $arguments); } /** diff --git a/src/Persistence/Sql/Connection.php b/src/Persistence/Sql/Connection.php index 9f27a69cb..2413f0e5c 100644 --- a/src/Persistence/Sql/Connection.php +++ b/src/Persistence/Sql/Connection.php @@ -298,9 +298,8 @@ public function dsql($properties = []): Query * Returns Expression object with connection already set. * * @param string|array $properties - * @param array $arguments */ - public function expr($properties = [], $arguments = null): Expression + public function expr($properties = [], array $arguments = []): Expression { $c = $this->expression_class; $e = new $c($properties, $arguments); diff --git a/src/Persistence/Sql/Expression.php b/src/Persistence/Sql/Expression.php index d7ae151d8..12de95f59 100644 --- a/src/Persistence/Sql/Expression.php +++ b/src/Persistence/Sql/Expression.php @@ -11,6 +11,7 @@ use Doctrine\DBAL\ParameterType; use Doctrine\DBAL\Platforms\OraclePlatform; use Doctrine\DBAL\Platforms\PostgreSQLPlatform; +use Doctrine\DBAL\Platforms\SqlitePlatform; use Doctrine\DBAL\Platforms\SQLServerPlatform; use Doctrine\DBAL\Result as DbalResult; @@ -74,39 +75,18 @@ class Expression implements Expressionable, \ArrayAccess * If $properties is passed as string, then it's treated as template. * * @param string|array $properties - * @param array $arguments */ - public function __construct($properties = [], $arguments = null) + public function __construct($properties = [], array $arguments = []) { - // save template if (is_string($properties)) { $properties = ['template' => $properties]; - } elseif (!is_array($properties)) { - throw (new Exception('Incorrect use of Expression constructor')) - ->addMoreInfo('properties', $properties) - ->addMoreInfo('arguments', $arguments); } - // supports passing template as property value without key 'template' - if (isset($properties[0])) { - $properties['template'] = $properties[0]; - unset($properties[0]); - } - - // save arguments - if ($arguments !== null) { - if (!is_array($arguments)) { - throw (new Exception('Expression arguments must be an array')) - ->addMoreInfo('properties', $properties) - ->addMoreInfo('arguments', $arguments); - } - $this->args['custom'] = $arguments; - } - - // deal with remaining properties foreach ($properties as $key => $val) { $this->{$key} = $val; } + + $this->args['custom'] = $arguments; } /** @@ -170,28 +150,10 @@ public function offsetUnset($offset): void * new expression to the same connection as the parent. * * @param string|array $properties - * @param array $arguments - * - * @return Expression */ - public function expr($properties = [], $arguments = null) + public function expr($properties = [], array $arguments = []): self { - if ($this->connection !== null) { - // TODO condition above always satisfied when connection is set - adjust tests, - // so connection is always set and remove the code below - return $this->connection->expr($properties, $arguments); - } - - // make a smart guess :) when connection is not set - if ($this instanceof Query) { - $e = new self($properties, $arguments); - } else { - $e = new static($properties, $arguments); - } - - $e->identifierEscapeChar = $this->identifierEscapeChar; - - return $e; + return $this->connection->expr($properties, $arguments); } /** @@ -281,21 +243,6 @@ protected function consume($expr, string $escapeMode = self::ESCAPE_PARAM) return $sql; } - /** - * Creates new expression where $value appears escaped. Use this - * method as a conventional means of specifying arguments when you - * think they might have a nasty back-ticks or commas in the field - * names. - * - * @param string $value - * - * @return Expression - */ - public function escape($value) - { - return $this->expr('{}', [$value]); - } - /** * Converts value into parameter and returns reference. Use only during * query rendering. Consider using `consume()` instead, which will @@ -315,7 +262,72 @@ protected function escapeParam($value): string } /** - * Escapes argument by adding backticks around it. + * This method should be used only when string value cannot be bound. + */ + protected function escapeStringLiteral(string $value): string + { + $platform = $this->connection->getDatabasePlatform(); + if ($platform instanceof PostgreSQLPlatform || $platform instanceof SQLServerPlatform) { + $dummyPersistence = new Persistence\Sql($this->connection); + if (\Closure::bind(fn () => $dummyPersistence->binaryTypeValueIsEncoded($value), null, Persistence\Sql::class)()) { + $value = \Closure::bind(fn () => $dummyPersistence->binaryTypeValueDecode($value), null, Persistence\Sql::class)(); + + if ($platform instanceof PostgreSQLPlatform) { + return 'decode(\'' . bin2hex($value) . '\', \'hex\')'; + } + + return 'CONVERT(VARBINARY(MAX), \'' . bin2hex($value) . '\', 2)'; + } + } + + $parts = []; + foreach (explode("\0", $value) as $i => $v) { + if ($i > 0) { + if ($platform instanceof PostgreSQLPlatform) { + // will raise SQL error, PostgreSQL does not support \0 character + $parts[] = 'convert_from(decode(\'00\', \'hex\'), \'UTF8\')'; + } elseif ($platform instanceof SQLServerPlatform) { + $parts[] = 'NCHAR(0)'; + } elseif ($platform instanceof OraclePlatform) { + $parts[] = 'CHR(0)'; + } else { + $parts[] = 'x\'00\''; + } + } + + if ($v !== '') { + $parts[] = '\'' . str_replace('\'', '\'\'', $v) . '\''; + } + } + if ($parts === []) { + $parts = ['\'\'']; + } + + $buildConcatSqlFx = function (array $parts) use (&$buildConcatSqlFx, $platform): string { + if (count($parts) > 1) { + $partsLeft = array_slice($parts, 0, intdiv(count($parts), 2)); + $partsRight = array_slice($parts, count($partsLeft)); + + $sqlLeft = $buildConcatSqlFx($partsLeft); + if ($platform instanceof SQLServerPlatform && count($partsLeft) === 1) { + $sqlLeft = 'CAST(' . $sqlLeft . ' AS NVARCHAR(MAX))'; + } + + return ($platform instanceof SqlitePlatform ? '(' : 'CONCAT(') + . $sqlLeft + . ($platform instanceof SqlitePlatform ? ' || ' : ', ') + . $buildConcatSqlFx($partsRight) + . ')'; + } + + return reset($parts); + }; + + return $buildConcatSqlFx($parts); + } + + /** + * Escapes identifier from argument. * This will allow you to use reserved SQL words as table or field * names such as "table" as well as other characters that SQL * permits in the identifiers (e.g. spaces or equation signs). @@ -459,7 +471,7 @@ public function getDebugQuery(): string } elseif (is_float($val)) { $replacement = self::castFloatToString($val); } elseif (is_string($val)) { - $replacement = '\'' . addslashes($val) . '\''; + $replacement = '\'' . str_replace('\'', '\'\'', $val) . '\''; } else { continue; } @@ -585,8 +597,7 @@ public function execute(object $connection = null, bool $fromExecuteStatement = } elseif (is_string($val)) { $type = ParameterType::STRING; - if ($platform instanceof PostgreSQLPlatform - || $platform instanceof SQLServerPlatform) { + if ($platform instanceof PostgreSQLPlatform || $platform instanceof SQLServerPlatform) { $dummyPersistence = new Persistence\Sql($this->connection); if (\Closure::bind(fn () => $dummyPersistence->binaryTypeValueIsEncoded($val), null, Persistence\Sql::class)()) { $val = \Closure::bind(fn () => $dummyPersistence->binaryTypeValueDecode($val), null, Persistence\Sql::class)(); @@ -689,11 +700,11 @@ private function castGetValue($v): ?string } // for PostgreSQL/Oracle CLOB/BLOB datatypes and PDO driver - if (is_resource($v) && get_resource_type($v) === 'stream' && ( - $this->connection->getDatabasePlatform() instanceof PostgreSQLPlatform - || $this->connection->getDatabasePlatform() instanceof OraclePlatform - )) { - $v = stream_get_contents($v); + if (is_resource($v) && get_resource_type($v) === 'stream') { + $platform = $this->connection->getDatabasePlatform(); + if ($platform instanceof PostgreSQLPlatform || $platform instanceof OraclePlatform) { + $v = stream_get_contents($v); + } } return $v; // throw a type error if not null nor string diff --git a/src/Persistence/Sql/Mssql/ExpressionTrait.php b/src/Persistence/Sql/Mssql/ExpressionTrait.php index 31a29b439..066b8e68b 100644 --- a/src/Persistence/Sql/Mssql/ExpressionTrait.php +++ b/src/Persistence/Sql/Mssql/ExpressionTrait.php @@ -10,16 +10,20 @@ trait ExpressionTrait { protected function escapeIdentifier(string $value): string { - return preg_replace('~\]([^\[\]\'"(){}]*?\])~s', '[$1', parent::escapeIdentifier($value)); + $res = parent::escapeIdentifier($value); + + return $this->identifierEscapeChar === ']' && str_starts_with($res, ']') && str_ends_with($res, ']') + ? '[' . substr($res, 1) + : $res; } public function render(): array { [$sql, $params] = parent::render(); - // convert all SQL strings to NVARCHAR, eg 'text' to N'text' - $sql = preg_replace_callback('~N?(\'(?:\'\'|\\\\\'|[^\'])*+\')~s', function ($matches) { - return 'N' . $matches[1]; + // convert all string literals to NVARCHAR, eg. 'text' to N'text' + $sql = preg_replace_callback('~N?\'(?:\'\'|\\\\\'|[^\'])*+\'~s', function ($matches) { + return (substr($matches[0], 0, 1) === 'N' ? '' : 'N') . $matches[0]; }, $sql); return [$sql, $params]; diff --git a/src/Persistence/Sql/Mssql/Query.php b/src/Persistence/Sql/Mssql/Query.php index 20e5ceb7b..209d97ff4 100644 --- a/src/Persistence/Sql/Mssql/Query.php +++ b/src/Persistence/Sql/Mssql/Query.php @@ -54,7 +54,7 @@ public function _render_limit(): ?string public function groupConcat($field, string $delimiter = ',') { - return $this->expr('string_agg({}, \'' . $delimiter . '\')', [$field]); + return $this->expr('string_agg({}, ' . $this->escapeStringLiteral($delimiter) . ')', [$field]); } public function exists() diff --git a/src/Persistence/Sql/Mysql/ExpressionTrait.php b/src/Persistence/Sql/Mysql/ExpressionTrait.php index a72078bdc..6c342c7a8 100644 --- a/src/Persistence/Sql/Mysql/ExpressionTrait.php +++ b/src/Persistence/Sql/Mysql/ExpressionTrait.php @@ -6,6 +6,11 @@ trait ExpressionTrait { + protected function escapeStringLiteral(string $value): string + { + return str_replace('\\', '\\\\', parent::escapeStringLiteral($value)); + } + protected function hasNativeNamedParamSupport(): bool { $dbalConnection = $this->connection->getConnection(); diff --git a/src/Persistence/Sql/Mysql/Query.php b/src/Persistence/Sql/Mysql/Query.php index 4c3e616b0..c8002cdb9 100644 --- a/src/Persistence/Sql/Mysql/Query.php +++ b/src/Persistence/Sql/Mysql/Query.php @@ -20,6 +20,6 @@ class Query extends BaseQuery public function groupConcat($field, string $delimiter = ',') { - return $this->expr('group_concat({} separator \'' . str_replace('\'', '\'\'', $delimiter) . '\')', [$field]); + return $this->expr('group_concat({} separator ' . $this->escapeStringLiteral($delimiter) . ')', [$field]); } } diff --git a/src/Persistence/Sql/Oracle/ExpressionTrait.php b/src/Persistence/Sql/Oracle/ExpressionTrait.php index e10c85f7a..33cc08e7e 100644 --- a/src/Persistence/Sql/Oracle/ExpressionTrait.php +++ b/src/Persistence/Sql/Oracle/ExpressionTrait.php @@ -6,8 +6,51 @@ trait ExpressionTrait { + /** + * Like mb_str_split() function, but split by length in bytes. + * + * @return array + */ + private function splitLongString(string $value, int $lengthBytes): array + { + $res = []; + $value = array_reverse(str_split($value, 2 * $lengthBytes)); + $i = count($value) - 1; + $buffer = ''; + while (true) { + if (strlen($buffer) <= $lengthBytes && $i >= 0) { + $buffer .= array_pop($value); + --$i; + } + + if (strlen($buffer) <= $lengthBytes) { + $res[] = $buffer; + $buffer = ''; + + break; + } + + $l = $lengthBytes; + for ($j = 0; $j < 4; ++$j) { + $ordNextChar = ord(substr($buffer, $l - $j, 1)); + if ($ordNextChar < 0x80 || $ordNextChar >= 0xC0) { + $l -= $j; + + break; + } + } + $res[] = substr($buffer, 0, $l); + $buffer = substr($buffer, $l); + } + + return $res; + } + protected function convertLongStringToClobExpr(string $value): Expression { + // Oracle (multibyte) string literal is limited to 1332 bytes + $parts = $this->splitLongString($value, 1000); + $exprArgs = []; $buildConcatExprFx = function (array $parts) use (&$buildConcatExprFx, &$exprArgs): string { if (count($parts) > 1) { @@ -22,18 +65,6 @@ protected function convertLongStringToClobExpr(string $value): Expression return 'TO_CLOB([])'; }; - // Oracle (multibyte) string literal is limited to 1332 bytes - $parts = []; - foreach (mb_str_split($value, 10_000) ?: [''] as $shorterValue) { // @phpstan-ignore-line https://github.com/phpstan/phpstan/issues/7580 - $lengthBytes = strlen($shorterValue); - $startBytes = 0; - do { - $part = mb_strcut($shorterValue, $startBytes, 1000); - $startBytes += strlen($part); - $parts[] = $part; - } while ($startBytes < $lengthBytes); - } - $expr = $buildConcatExprFx($parts); return $this->expr($expr, $exprArgs); // @phpstan-ignore-line diff --git a/src/Persistence/Sql/Oracle/PlatformTrait.php b/src/Persistence/Sql/Oracle/PlatformTrait.php index e302972a5..696d573b2 100644 --- a/src/Persistence/Sql/Oracle/PlatformTrait.php +++ b/src/Persistence/Sql/Oracle/PlatformTrait.php @@ -67,7 +67,7 @@ public function getCreateAutoincrementSql($name, $table, $start = 1) $pkSeq = \Closure::bind(fn () => $this->normalizeIdentifier($aiSequenceName), $this, OraclePlatform::class)()->getName(); $sqls[count($sqls) - 1] = $conn->expr( // else branch should be maybe (because of concurrency) put into after update trigger - str_replace('[pk_seq]', '\'' . $pkSeq . '\'', <<<'EOT' + str_replace('[pk_seq]', '\'' . str_replace('\'', '\'\'', $pkSeq) . '\'', <<<'EOT' CREATE TRIGGER {trigger} BEFORE INSERT OR UPDATE ON {table} diff --git a/src/Persistence/Sql/Query.php b/src/Persistence/Sql/Query.php index 9b8c41324..7b5fa753e 100644 --- a/src/Persistence/Sql/Query.php +++ b/src/Persistence/Sql/Query.php @@ -824,8 +824,8 @@ protected function _render_set_fields(): ?string $ret = []; if ($this->args['set']) { - foreach ($this->args['set'] as [$field/* , $value */ ]) { - $field = $this->consume($field, self::ESCAPE_IDENTIFIER); + foreach ($this->args['set'] as $pair) { + $field = $this->consume($pair[0], self::ESCAPE_IDENTIFIER); $ret[] = $field; } @@ -840,8 +840,8 @@ protected function _render_set_values(): ?string $ret = []; if ($this->args['set']) { - foreach ($this->args['set'] as [/* $field */, $value]) { - $value = $this->consume($value, self::ESCAPE_PARAM); + foreach ($this->args['set'] as $pair) { + $value = $this->consume($pair[1], self::ESCAPE_PARAM); $ret[] = $value; } @@ -1097,11 +1097,8 @@ public function dsql($properties = []) * method should be used for building parts of the query internally. * * @param string|array $properties - * @param array $arguments - * - * @return Expression */ - public function expr($properties = [], $arguments = null) + public function expr($properties = [], array $arguments = []): Expression { $c = $this->expression_class; $e = new $c($properties, $arguments); diff --git a/src/Persistence/Static_.php b/src/Persistence/Static_.php index 0e6853b4c..c3c75c254 100644 --- a/src/Persistence/Static_.php +++ b/src/Persistence/Static_.php @@ -27,7 +27,7 @@ class Static_ extends Array_ * * @param array $data Static data in one of supported formats */ - public function __construct(array $data = null) + public function __construct(array $data = []) { // chomp off first row, we will use it to deduct fields $row1 = reset($data); diff --git a/src/Reference/HasOneSql.php b/src/Reference/HasOneSql.php index f9e08dc37..4250846dd 100644 --- a/src/Reference/HasOneSql.php +++ b/src/Reference/HasOneSql.php @@ -65,9 +65,9 @@ public function addField(string $fieldName, string $theirFieldName = null, array * Add multiple expressions by calling addField several times. Fields * may contain 3 types of elements:. * - * [ 'name', 'surname' ] - will import those fields as-is - * [ 'full_name' => 'name', 'day_of_birth' => ['dob', 'type' => 'date'] ] - use alias and options - * [ ['dob', 'type' => 'date'] ] - use options + * ['name', 'surname'] - will import those fields as-is + * ['full_name' => 'name', 'day_of_birth' => ['dob', 'type' => 'date']] - use alias and options + * [['dob', 'type' => 'date']] - use options * * You may also use second param to specify parameters: * diff --git a/tests/JoinArrayTest.php b/tests/JoinArrayTest.php index da561c332..66da04302 100644 --- a/tests/JoinArrayTest.php +++ b/tests/JoinArrayTest.php @@ -49,9 +49,9 @@ public function testDirection(): void $this->expectException(Exception::class); // TODO not implemented yet, see https://github.com/atk4/data/issues/803 $j = $m->join('contact4.foo_id', ['master_field' => 'test_id', 'reverse' => true]); - $this->assertTrue($this->getProtected($j, 'reverse')); - $this->assertSame('test_id', $this->getProtected($j, 'master_field')); - $this->assertSame('foo_id', $this->getProtected($j, 'foreign_field')); + // $this->assertTrue($this->getProtected($j, 'reverse')); + // $this->assertSame('test_id', $this->getProtected($j, 'master_field')); + // $this->assertSame('foo_id', $this->getProtected($j, 'foreign_field')); } public function testJoinException(): void diff --git a/tests/JoinSqlTest.php b/tests/JoinSqlTest.php index 9f0e5d722..19a27c879 100644 --- a/tests/JoinSqlTest.php +++ b/tests/JoinSqlTest.php @@ -31,9 +31,9 @@ public function testDirection(): void $this->expectException(Exception::class); // TODO not implemented yet, see https://github.com/atk4/data/issues/803 $j = $m->join('contact4.foo_id', ['master_field' => 'test_id', 'reverse' => true]); - $this->assertTrue($this->getProtected($j, 'reverse')); - $this->assertSame('test_id', $this->getProtected($j, 'master_field')); - $this->assertSame('foo_id', $this->getProtected($j, 'foreign_field')); + // $this->assertTrue($this->getProtected($j, 'reverse')); + // $this->assertSame('test_id', $this->getProtected($j, 'master_field')); + // $this->assertSame('foo_id', $this->getProtected($j, 'foreign_field')); } public function testDirectionException(): void diff --git a/tests/Model/Smbo/Company.php b/tests/Model/Smbo/Company.php deleted file mode 100644 index 329c7b36a..000000000 --- a/tests/Model/Smbo/Company.php +++ /dev/null @@ -1,35 +0,0 @@ -join('contractor'); - $j_company = $j_contractor->join('company.contractor_id', ['prefix' => 'company_']); - - $j_contractor->addFields([ - ['name', 'actual' => 'legal_name'], - ]); - - $j_company->addFields([ - ['business_start', 'type' => 'date'], - ['director_name'], - ['vat_calculation_type', 'enum' => ['cash', 'invoice']], - ]); - - $this->addFields([ - ['is_vat_registered', 'type' => 'boolean'], - ]); - } -} diff --git a/tests/Model/Smbo/Contact.php b/tests/Model/Smbo/Contact.php deleted file mode 100644 index de94aa8ee..000000000 --- a/tests/Model/Smbo/Contact.php +++ /dev/null @@ -1,21 +0,0 @@ -addField('type', ['enum' => ['client', 'supplier']]); - - $this->addField('name'); - } -} diff --git a/tests/Model/Smbo/Document.php b/tests/Model/Smbo/Document.php index 467988797..c582a5c6d 100644 --- a/tests/Model/Smbo/Document.php +++ b/tests/Model/Smbo/Document.php @@ -14,10 +14,6 @@ protected function init(): void { parent::init(); - // Documest is sent from one Contact to Another - $this->hasOne('contact_from_id', ['model' => [Contact::class]]); - $this->hasOne('contact_to_id', ['model' => [Contact::class]]); - $this->addField('doc_type', ['enum' => ['invoice', 'payment']]); $this->addField('amount', ['type' => 'atk4_money']); diff --git a/tests/Persistence/Sql/ConnectionTest.php b/tests/Persistence/Sql/ConnectionTest.php index 04432a5bd..189fca185 100644 --- a/tests/Persistence/Sql/ConnectionTest.php +++ b/tests/Persistence/Sql/ConnectionTest.php @@ -35,9 +35,6 @@ public function getName() } } -/** - * @coversDefaultClass \Atk4\Data\Persistence\Sql\Connection - */ class ConnectionTest extends TestCase { public function testInit(): void diff --git a/tests/Persistence/Sql/ExceptionTest.php b/tests/Persistence/Sql/ExceptionTest.php index e4b0ba91e..297b9953a 100644 --- a/tests/Persistence/Sql/ExceptionTest.php +++ b/tests/Persistence/Sql/ExceptionTest.php @@ -8,14 +8,8 @@ use Atk4\Data\Persistence\Sql\Exception as SqlException; use Atk4\Data\Persistence\Sql\Expression; -/** - * @coversDefaultClass \Atk4\Data\Persistence\Sql\Exception - */ class ExceptionTest extends TestCase { - /** - * @covers ::__construct - */ public function testException1(): void { $this->expectException(SqlException::class); diff --git a/tests/Persistence/Sql/ExpressionTest.php b/tests/Persistence/Sql/ExpressionTest.php index 163a8c73b..01454ecac 100644 --- a/tests/Persistence/Sql/ExpressionTest.php +++ b/tests/Persistence/Sql/ExpressionTest.php @@ -10,10 +10,8 @@ use Atk4\Data\Persistence\Sql\Expressionable; use Atk4\Data\Persistence\Sql\Mysql; use Atk4\Data\Persistence\Sql\Query; +use Atk4\Data\Persistence\Sql\Sqlite; -/** - * @coversDefaultClass \Atk4\Data\Persistence\Sql\Expression - */ class ExpressionTest extends TestCase { /** @@ -24,46 +22,6 @@ public function e(...$args): Expression return new Expression(...$args); } - /** - * Test constructor exception - wrong 1st parameter. - * - * @covers ::__construct - */ - public function testConstructorException1st1(): void - { - $this->expectException(Exception::class); - $this->e(null); - } - - /** - * Test constructor exception - wrong 1st parameter. - * - * @covers ::__construct - */ - public function testConstructorException1st2(): void - { - $this->expectException(Exception::class); - $this->e(false); - } - - /** - * Test constructor exception - wrong 2nd parameter. - */ - public function testConstructorException2nd1(): void - { - $this->expectException(Exception::class); - $this->e('hello, []', false); - } - - /** - * Test constructor exception - wrong 2nd parameter. - */ - public function testConstructorException2nd2(): void - { - $this->expectException(Exception::class); - $this->e('hello, []', 'hello'); - } - /** * Test constructor exception - no arguments (template is not defined). */ @@ -75,8 +33,6 @@ public function testConstructorException0arg(): void /** * Testing parameter edge cases - empty strings and arrays etc. - * - * @covers ::__construct */ public function testConstructor1(): void { @@ -89,9 +45,6 @@ public function testConstructor1(): void /** * Testing simple template patterns without arguments. * Testing different ways how to pass template to constructor. - * - * @covers ::__construct - * @covers ::escapeIdentifier */ public function testConstructor2(): void { @@ -100,21 +53,11 @@ public function testConstructor2(): void 'now()', $this->e('now()')->render()[0] ); - // pass as array without key - $this->assertSame( - 'now()', - $this->e(['now()'])->render()[0] - ); // pass as array with template key $this->assertSame( 'now()', $this->e(['template' => 'now()'])->render()[0] ); - // pass as array without key - $this->assertSame( - ':a Name', - $this->e(['[] Name'], ['First'])->render()[0] - ); // pass as array with template key $this->assertSame( ':a Name', @@ -124,8 +67,6 @@ public function testConstructor2(): void /** * Testing template with simple arguments. - * - * @covers ::__construct */ public function testConstructor3(): void { @@ -144,8 +85,6 @@ public function testConstructor3(): void /** * Testing template with complex arguments. - * - * @covers ::__construct */ public function testConstructor4(): void { @@ -240,10 +179,6 @@ public function provideNoTemplatingInSqlStringData(): iterable /** * Test nested parameters. - * - * @covers ::__construct - * @covers ::escapeParam - * @covers ::getDebugQuery */ public function testNestedParams(): void { @@ -264,9 +199,6 @@ public function testNestedParams(): void /** * Tests where one expression with parameter is used within several other expressions. - * - * @covers ::__construct - * @covers ::render */ public function testNestedExpressions(): void { @@ -286,22 +218,8 @@ public function testNestedExpressions(): void $this->assertSame('Hello :a and good night', $s4); } - /** - * @covers ::__toString - */ - /* - public function testToStringException1(): void - { - $e = new MyBadExpression('Hello'); - $this->expectException(Exception::class); - $s = (string)$e; - } - */ - /** * expr() should return new Expression object and inherit connection from it. - * - * @covers ::expr */ public function testExpr(): void { @@ -311,10 +229,6 @@ public function testExpr(): void /** * Fully covers escapeIdentifier method. - * - * @covers ::escape - * @covers ::escapeIdentifier - * @covers ::escapeIdentifierSoft */ public function testEscape(): void { @@ -357,25 +271,10 @@ public function testEscape(): void '"users".*', $this->callProtected($this->e(), 'escapeIdentifierSoft', 'users.*') ); - - $this->assertSame( - '"first_name"', - $this->e()->escape('first_name')->render()[0] - ); - $this->assertSame( - '"first""_name"', - $this->e()->escape('first"_name')->render()[0] - ); - $this->assertSame( - '"first""_name {}"', - $this->e()->escape('first"_name {}')->render()[0] - ); } /** * Fully covers escapeParam method. - * - * @covers ::escapeParam */ public function testParam(): void { @@ -386,9 +285,6 @@ public function testParam(): void ], $e->render()); } - /** - * @covers ::consume - */ public function testConsume(): void { $constants = (new \ReflectionClass(Expression::class))->getConstants(); @@ -411,16 +307,22 @@ public function testConsume(): void $this->callProtected($this->e(), 'consume', new Query()) ); + $myField = new class() implements Expressionable { + public function getDsqlExpression(Expression $expr): Expression + { + return $expr->expr('"myfield"'); + } + }; + $e = $this->e('hello, []', [$myField]); + $e->connection = new Sqlite\Connection(); $this->assertSame( 'hello, "myfield"', - $this->e('hello, []', [new MyField()])->render()[0] + $e->render()[0] ); } /** * $escape_mode value is incorrect. - * - * @covers ::consume */ public function testConsumeException1(): void { @@ -430,8 +332,6 @@ public function testConsumeException1(): void /** * Only Expressionable objects may be used in Expression. - * - * @covers ::consume */ public function testConsumeException2(): void { @@ -439,12 +339,6 @@ public function testConsumeException2(): void $this->callProtected($this->e(), 'consume', new \stdClass()); } - /** - * @covers ::offsetExists - * @covers ::offsetGet - * @covers ::offsetSet - * @covers ::offsetUnset - */ public function testArrayAccess(): void { $e = $this->e('', ['parrot' => 'red', 'blue']); @@ -477,26 +371,17 @@ public function testArrayAccess(): void $this->assertSame('coalesce(year(now()) - year(birth_date), :a)', $age->render()[0]); } - /** - * Test IteratorAggregate implementation. - * - * @covers ::getRowsIterator - * - * @doesNotPerformAssertions - */ - public function testIteratorAggregate(): void - { - // TODO cannot test this without actual DB connection and executing expression - } - /** * Test for vendors that rely on JavaScript expressions, instead of parameters. - * - * @coversNothing */ public function testJsonExpression(): void { - $e = new JsonExpression('hello, [who]', ['who' => 'world']); + $e = new class('hello, [who]', ['who' => 'world']) extends Expression { + public function escapeParam($value): string + { + return json_encode($value); + } + }; $this->assertSame([ 'hello, "world"', @@ -504,24 +389,21 @@ public function testJsonExpression(): void ], $e->render()); } - /** - * Test var-dump code for codecoverage. - * - * @covers ::__debugInfo - * - * @doesNotPerformAssertions - */ public function testVarDump(): void { - $this->e('test')->__debugInfo(); + $this->assertSame( + 'test', + $this->e('test')->__debugInfo()['R'] + ); - $this->e(' [nosuchtag] ')->__debugInfo(); + $this->assertStringContainsString( + 'Expression could not render tag', + $this->e(' [nosuchtag] ')->__debugInfo()['R'] + ); } /** * Test reset(). - * - * @covers ::reset */ public function testReset(): void { @@ -536,30 +418,3 @@ public function testReset(): void $this->assertSame(['custom' => ['name' => 'John']], $this->getProtected($e, 'args')); } } - -// @codingStandardsIgnoreStart -class JsonExpression extends Expression -{ - public function escapeParam($value): string - { - return json_encode($value); - } -} -class MyField implements Expressionable -{ - public function getDsqlExpression(Expression $expr): Expression - { - return $expr->expr('"myfield"'); - } -} -/* -class MyBadExpression extends Expression -{ - public function getOne() - { - // should return string, but for test case we return array to get \Exception - return array(); - } -} -*/ -// @codingStandardsIgnoreEnd diff --git a/tests/Persistence/Sql/QueryTest.php b/tests/Persistence/Sql/QueryTest.php index 860216681..ecfd53350 100644 --- a/tests/Persistence/Sql/QueryTest.php +++ b/tests/Persistence/Sql/QueryTest.php @@ -7,16 +7,9 @@ use Atk4\Core\Phpunit\TestCase; use Atk4\Data\Persistence\Sql\Exception; use Atk4\Data\Persistence\Sql\Expression; -use Atk4\Data\Persistence\Sql\Mssql; use Atk4\Data\Persistence\Sql\Mysql; -use Atk4\Data\Persistence\Sql\Oracle; -use Atk4\Data\Persistence\Sql\Postgresql; use Atk4\Data\Persistence\Sql\Query; -use Atk4\Data\Persistence\Sql\Sqlite; -/** - * @coversDefaultClass \Atk4\Data\Persistence\Sql\Query - */ class QueryTest extends TestCase { /** @@ -27,9 +20,6 @@ public function q(...$args): Query return new Query(...$args); } - /** - * @covers ::__construct - */ public function testConstruct(): void { // passing properties in constructor @@ -41,8 +31,6 @@ public function testConstruct(): void /** * dsql() should return new Query object and inherit connection from it. - * - * @covers ::dsql */ public function testDsql(): void { @@ -52,8 +40,6 @@ public function testDsql(): void /** * field() should return $this Query for chaining. - * - * @covers ::field */ public function testFieldReturnValue(): void { @@ -63,9 +49,6 @@ public function testFieldReturnValue(): void /** * Testing field - basic cases. - * - * @covers ::_render_field - * @covers ::field */ public function testFieldBasic(): void { @@ -109,9 +92,6 @@ public function testFieldBasic(): void /** * Testing field - defaultField. - * - * @covers ::_render_field - * @covers ::field */ public function testFieldDefaultField(): void { @@ -139,9 +119,6 @@ public function testFieldDefaultField(): void /** * Testing field - basic cases. - * - * @covers ::_render_field - * @covers ::field */ public function testFieldExpression(): void { @@ -178,9 +155,6 @@ public function testFieldExpression(): void /** * Duplicate alias of field. - * - * @covers ::_set_args - * @covers ::field */ public function testFieldException1(): void { @@ -191,8 +165,6 @@ public function testFieldException1(): void /** * Alias is NOT mandatory when pass table as Expression. * - * @covers ::table - * * @doesNotPerformAssertions */ public function testTableException3(): void @@ -203,8 +175,6 @@ public function testTableException3(): void /** * Alias is IS mandatory when pass table as Query. - * - * @covers ::table */ public function testTableException4(): void { @@ -214,9 +184,6 @@ public function testTableException4(): void /** * Table aliases should be unique. - * - * @covers ::_set_args - * @covers ::table */ public function testTableException5(): void { @@ -228,9 +195,6 @@ public function testTableException5(): void /** * Table aliases should be unique. - * - * @covers ::_set_args - * @covers ::table */ public function testTableException6(): void { @@ -242,9 +206,6 @@ public function testTableException6(): void /** * Table aliases should be unique. - * - * @covers ::_set_args - * @covers ::table */ public function testTableException7(): void { @@ -256,9 +217,6 @@ public function testTableException7(): void /** * Table aliases should be unique. - * - * @covers ::_set_args - * @covers ::table */ public function testTableException8(): void { @@ -270,9 +228,6 @@ public function testTableException8(): void /** * Table aliases should be unique. - * - * @covers ::_set_args - * @covers ::table */ public function testTableException9(): void { @@ -284,8 +239,6 @@ public function testTableException9(): void /** * Table can't be set as sub-Query in Update query mode. - * - * @covers ::table */ public function testTableException10(): void { @@ -299,8 +252,6 @@ public function testTableException10(): void /** * Table can't be set as sub-Query in Insert query mode. - * - * @covers ::table */ public function testTableException11(): void { @@ -314,8 +265,6 @@ public function testTableException11(): void /** * Requesting non-existant query mode should throw exception. - * - * @covers ::mode */ public function testModeException1(): void { @@ -325,8 +274,6 @@ public function testModeException1(): void /** * table() should return $this Query for chaining. - * - * @covers ::table */ public function testTableReturnValue(): void { @@ -334,11 +281,6 @@ public function testTableReturnValue(): void $this->assertSame($q, $q->table('employee')); } - /** - * @covers ::_render_table - * @covers ::_render_table_noalias - * @covers ::table - */ public function testTableRender1(): void { // no table defined @@ -433,10 +375,6 @@ public function testTableRender1(): void ); } - /** - * @covers ::_render_table - * @covers ::table - */ public function testTableRender2(): void { // pass table as expression or query @@ -474,11 +412,6 @@ public function testTableRender2(): void ); } - /** - * @covers ::render - * @covers \Atk4\Data\Persistence\Sql\Expression::consume - * @covers \Atk4\Data\Persistence\Sql\Expression::render - */ public function testBasicRenderSubquery(): void { $age = new Expression('coalesce([age], [default_age])'); @@ -493,9 +426,6 @@ public function testBasicRenderSubquery(): void ); } - /** - * @covers \Atk4\Data\Persistence\Sql\Expression::getDebugQuery - */ public function testGetDebugQuery(): void { $age = new Expression('coalesce([age], [default_age], [foo], [bar])'); @@ -512,9 +442,6 @@ public function testGetDebugQuery(): void ); } - /** - * @covers ::__debugInfo - */ public function testVarDump(): void { $this->assertMatchesRegularExpression( @@ -523,9 +450,6 @@ public function testVarDump(): void ); } - /** - * @covers ::__debugInfo - */ public function testVarDump2(): void { $this->assertStringContainsString( @@ -534,9 +458,6 @@ public function testVarDump2(): void ); } - /** - * @covers ::__debugInfo - */ public function testVarDump3(): void { $this->assertStringContainsString( @@ -545,9 +466,6 @@ public function testVarDump3(): void ); } - /** - * @covers ::__debugInfo - */ public function testVarDump4(): void { // should throw exception "Table cannot be Query in UPDATE, INSERT etc. query modes" @@ -559,13 +477,6 @@ public function testVarDump4(): void ); } - /** - * @covers ::_render_field - * @covers ::_render_table - * @covers ::field - * @covers ::render - * @covers ::table - */ public function testUnionQuery(): void { // 1st query @@ -633,8 +544,6 @@ public function testUnionQuery(): void /** * where() should return $this Query for chaining. - * - * @covers ::where */ public function testWhereReturnValue(): void { @@ -644,8 +553,6 @@ public function testWhereReturnValue(): void /** * having() should return $this Query for chaining. - * - * @covers ::field */ public function testHavingReturnValue(): void { @@ -655,10 +562,6 @@ public function testHavingReturnValue(): void /** * Basic where() tests. - * - * @covers ::_render_where - * @covers ::_sub_render_where - * @covers ::where */ public function testWhereBasic(): void { @@ -812,10 +715,6 @@ public function provideWhereUnsupportedOperatorData(): iterable /** * Testing where() with special values - null, array, like. - * - * @covers ::_render_where - * @covers ::_sub_render_where - * @covers ::where */ public function testWhereSpecialValues(): void { @@ -861,9 +760,6 @@ public function testWhereSpecialValues(): void /** * Having basically is the same as where, so we can relax and trouhly test where() instead. - * - * @covers ::_render_having - * @covers ::having */ public function testBasicHaving(): void { @@ -883,9 +779,6 @@ public function testBasicHaving(): void /** * Test Limit. - * - * @covers ::_render_limit - * @covers ::limit */ public function testLimit(): void { @@ -901,9 +794,6 @@ public function testLimit(): void /** * Test Order. - * - * @covers ::_render_order - * @covers ::order */ public function testOrder(): void { @@ -967,11 +857,11 @@ public function testOrder(): void ); $this->assertSame( 'order by "* desc"', - $this->q('[order]')->order($this->q()->escape('* desc'))->render()[0] + $this->q('[order]')->order($this->q()->expr('{}', ['* desc']))->render()[0] ); $this->assertSame( 'order by "* desc {}"', - $this->q('[order]')->order($this->q()->escape('* desc {}'))->render()[0] + $this->q('[order]')->order($this->q()->expr('{}', ['* desc {}']))->render()[0] ); // custom sort order $this->assertSame( @@ -986,8 +876,6 @@ public function testOrder(): void /** * If first argument is array, second argument must not be used. - * - * @covers ::order */ public function testOrderException1(): void { @@ -997,9 +885,6 @@ public function testOrderException1(): void /** * Test Group. - * - * @covers ::_render_group - * @covers ::group */ public function testGroup(): void { @@ -1043,9 +928,6 @@ public function testGroup(): void ); } - /** - * Test groupConcat. - */ public function testGroupConcatException(): void { // doesn't support groupConcat by default @@ -1053,44 +935,19 @@ public function testGroupConcatException(): void $this->q()->groupConcat('foo'); } - /** - * Test groupConcat. - * - * @covers ::groupConcat - */ - public function testGroupConcat(): void - { - $q = new Mysql\Query(); - $this->assertSame('group_concat(`foo` separator \'-\')', $q->groupConcat('foo', '-')->render()[0]); - - $q = new Oracle\Query(); - $this->assertSame('listagg("foo", :a) within group (order by "foo")', $q->groupConcat('foo', '-')->render()[0]); - - $q = new Postgresql\Query(); - $this->assertSame('string_agg("foo", :a)', $q->groupConcat('foo', '-')->render()[0]); - - $q = new Sqlite\Query(); - $this->assertSame('group_concat("foo", :a)', $q->groupConcat('foo', '-')->render()[0]); - } - /** * Test expr(). - * - * @covers ::expr */ public function testExpr(): void { $this->assertSame(Expression::class, get_class($this->q()->expr('foo'))); $q = new Mysql\Query(); - $this->assertSame(\Atk4\Data\Persistence\Sql\Mysql\Expression::class, get_class($q->expr('foo'))); + $this->assertSame(Mysql\Expression::class, get_class($q->expr('foo'))); } /** * Test Join. - * - * @covers ::_render_join - * @covers ::join */ public function testJoin(): void { @@ -1149,10 +1006,6 @@ public function testJoin(): void /** * Combined execution of where() clauses. - * - * @covers ::_render_where - * @covers ::mode - * @covers ::where */ public function testCombinedWhere(): void { @@ -1202,13 +1055,6 @@ public function testCombinedWhere(): void /** * Test OrWhere and AndWhere without where condition. Should ignore them. - * - * @covers ::_render_andwhere - * @covers ::_render_orwhere - * @covers ::_render_where - * @covers ::andExpr - * @covers ::orExpr - * @covers ::where */ public function testEmptyOrAndWhere(): void { @@ -1225,13 +1071,6 @@ public function testEmptyOrAndWhere(): void /** * Test insert, update and delete templates. - * - * @covers ::_render_set - * @covers ::_render_set_fields - * @covers ::_render_set_values - * @covers ::mode - * @covers ::set - * @covers ::where */ public function testInsertDeleteUpdate(): void { @@ -1292,10 +1131,55 @@ public function testInsertDeleteUpdate(): void ); } + public function testMiscInsert(): void + { + $data = [ + 'id' => null, + 'system_id' => '3576', + 'system' => null, + 'created_dts' => 123, + 'contractor_from' => null, + 'contractor_to' => null, + 'vat_rate_id' => null, + 'currency_id' => null, + 'vat_period_id' => null, + 'journal_spec_id' => '147735', + 'job_id' => '9341', + 'nominal_id' => null, + 'root_nominal_code' => null, + 'doc_type' => null, + 'is_cn' => 'N', + 'doc_date' => null, + 'ref_no' => '940 testingqq11111', + 'po_ref' => null, + 'total_gross' => '100.00', + 'total_net' => null, + 'total_vat' => null, + 'exchange_rate' => 1.892134, + 'note' => null, + 'archive' => 'N', + 'fx_document_id' => null, + 'exchanged_total_net' => null, + 'exchanged_total_gross' => null, + 'exchanged_total_vat' => null, + 'exchanged_total_a' => null, + 'exchanged_total_b' => null, + ]; + + $q = $this->q(); + $q->mode('insert'); + foreach ($data as $k => $v) { + $q->set($k, $v); + } + + $this->assertSame( + 'insert into ("' . implode('", "', array_keys($data)) . '") values (:a, :b, :c, :d, :e, :f, :g, :h, :i, :j, :k, :l, :m, :n, :o, :p, :q, :r, :s, :t, :u, :v, :w, :x, :y, :z, :aa, :ab, :ac, :ad)', + $q->render()[0] + ); + } + /** * set() should return $this Query for chaining. - * - * @covers ::set */ public function testSetReturnValue(): void { @@ -1305,8 +1189,6 @@ public function testSetReturnValue(): void /** * Value of type array is not supported by SQL. - * - * @covers ::set */ public function testSetException1(): void { @@ -1317,8 +1199,6 @@ public function testSetException1(): void /** * Field name can be expression. * - * @covers ::set - * * @doesNotPerformAssertions */ public function testSetException2(): void @@ -1328,12 +1208,6 @@ public function testSetException2(): void /** * Test nested OR and AND expressions. - * - * @covers ::_render_andwhere - * @covers ::_render_orwhere - * @covers ::andExpr - * @covers ::orExpr - * @covers ::where */ public function testNestedOrAnd(): void { @@ -1404,8 +1278,6 @@ public function testNestedOrAndHavingWithWhereException(): void /** * Test reset(). - * - * @covers \Atk4\Data\Persistence\Sql\Expression::reset */ public function testReset(): void { @@ -1425,9 +1297,6 @@ public function testReset(): void /** * Test [option]. - * - * @covers ::_render_option - * @covers ::option */ public function testOption(): void { @@ -1465,11 +1334,6 @@ public function testOption(): void /** * Test caseExpr (normal). - * - * @covers ::_render_case - * @covers ::caseElse - * @covers ::caseExpr - * @covers ::caseWhen */ public function testCaseExprNormal(): void { @@ -1494,11 +1358,6 @@ public function testCaseExprNormal(): void /** * Test caseExpr (short form). - * - * @covers ::_render_case - * @covers ::caseElse - * @covers ::caseExpr - * @covers ::caseWhen */ public function testCaseExprShortForm(): void { @@ -1546,8 +1405,6 @@ public function testCaseExprException2(): void /** * Tests exprNow() method. - * - * @covers ::exprNow */ public function testExprNow(): void { @@ -1659,24 +1516,4 @@ public function testWith(): void $q->render()[0] ); } - - public function testExists(): void - { - $this->assertSame( - 'select exists (select * from "contacts" where "first_name" = :a)', - $this->q()->table('contacts')->where('first_name', 'John')->exists()->render()[0] - ); - - $q = new Oracle\Query(); - $this->assertSame( - 'select case when exists(select * from "contacts" where "first_name" = :xxaaaa) then 1 else 0 end from "DUAL"', - $q->table('contacts')->where('first_name', 'John')->exists()->render()[0] - ); - - $q = new Mssql\Query(); - $this->assertSame( - 'select case when exists(select * from [contacts] where [first_name] = :a) then 1 else 0 end', - $q->table('contacts')->where('first_name', 'John')->exists()->render()[0] - ); - } } diff --git a/tests/Persistence/Sql/RandomTest.php b/tests/Persistence/Sql/RandomTest.php deleted file mode 100644 index 7db8a08c7..000000000 --- a/tests/Persistence/Sql/RandomTest.php +++ /dev/null @@ -1,110 +0,0 @@ - null, - 'system_id' => '3576', - 'system' => null, - 'created_dts' => 123, - 'contractor_from' => null, - 'contractor_to' => null, - 'vat_rate_id' => null, - 'currency_id' => null, - 'vat_period_id' => null, - 'journal_spec_id' => '147735', - 'job_id' => '9341', - 'nominal_id' => null, - 'root_nominal_code' => null, - 'doc_type' => null, - 'is_cn' => 'N', - 'doc_date' => null, - 'ref_no' => '940 testingqq11111', - 'po_ref' => null, - 'total_gross' => '100.00', - 'total_net' => null, - 'total_vat' => null, - 'exchange_rate' => null, - 'note' => null, - 'archive' => 'N', - 'fx_document_id' => null, - 'exchanged_total_net' => null, - 'exchanged_total_gross' => null, - 'exchanged_total_vat' => null, - 'exchanged_total_a' => null, - 'exchanged_total_b' => null, - ]; - $q = $this->q(); - $q->mode('insert'); - foreach ($data as $key => $val) { - $q->set($key, $val); - } - $this->assertSame( - 'insert into ("' . implode('", "', array_keys($data)) . '") values (:a, :b, :c, :d, :e, :f, :g, :h, :i, :j, :k, :l, :m, :n, :o, :p, :q, :r, :s, :t, :u, :v, :w, :x, :y, :z, :aa, :ab, :ac, :ad)', - $q->render()[0] - ); - } - - /** - * Confirms that group concat works for all the SQL vendors we support. - */ - public function _groupConcatTest(string $expected, Query $q): void - { - $q->table('people'); - $q->group('age'); - - $q->field('age'); - $q->field($q->groupConcat('name', ',')); - - $q->groupConcat('name', ','); - - $this->assertSame($expected, $q->render()[0]); - } - - public function testGroupConcat(): void - { - $this->_groupConcatTest( - 'select `age`, group_concat(`name` separator \',\') from `people` group by `age`', - new Mysql\Query() - ); - - $this->_groupConcatTest( - 'select "age", group_concat("name", :a) from "people" group by "age"', - new Sqlite\Query() - ); - - $this->_groupConcatTest( - 'select "age", string_agg("name", :a) from "people" group by "age"', - new Postgresql\Query() - ); - - $this->_groupConcatTest( - 'select "age", listagg("name", :xxaaaa) within group (order by "name") from "people" group by "age"', - new Oracle\Query() - ); - } -} diff --git a/tests/Persistence/Sql/WithDb/SelectTest.php b/tests/Persistence/Sql/WithDb/SelectTest.php index 7cdd376ff..3314db5de 100644 --- a/tests/Persistence/Sql/WithDb/SelectTest.php +++ b/tests/Persistence/Sql/WithDb/SelectTest.php @@ -60,9 +60,8 @@ private function q($table = null, string $alias = null): Query /** * @param string|array $template - * @param array $args */ - private function e($template = [], array $args = null): Expression + private function e($template = [], array $args = []): Expression { return $this->c->expr($template, $args); } @@ -106,11 +105,9 @@ public function testBasicQueries(): void $this->q()->field(new Expression('2 + 2'), 'now')->getRows() ); - /* - * PostgreSQL needs to have values cast, to make the query work. - * But CAST(.. AS int) does not work in mysql. So we use two different tests.. - * (CAST(.. AS int) will work on mariaDB, whereas mysql needs it to be CAST(.. AS signed)) - */ + // PostgreSQL needs to have values cast, to make the query work. + // But CAST(.. AS int) does not work in Mysql. So we use two different tests.. + // (CAST(.. AS int) will work on MariaDB, whereas Mysql needs it to be CAST(.. AS signed)) if ($this->getDatabasePlatform() instanceof PostgreSQLPlatform) { $this->assertSame( [['now' => '6']], @@ -131,12 +128,8 @@ public function testBasicQueries(): void public function testExpression(): void { - /* - * PostgreSQL, at least versions before 10, needs to have the string cast to the - * correct datatype. - * But using CAST(.. AS CHAR) will return one single character on postgresql, but the - * entire string on mysql. - */ + // PostgreSQL, at least versions before 10, needs to have the string cast to the correct datatype. + // But using CAST(.. AS CHAR) will return a single character on PostgreSQL, but the entire string on MySQL. if ($this->getDatabasePlatform() instanceof PostgreSQLPlatform || $this->getDatabasePlatform() instanceof SQLServerPlatform) { $this->assertSame( 'foo', @@ -240,6 +233,77 @@ public function testWhereExpression(): void ); } + public function testGroupConcat(): void + { + $q = $this->q() + ->table('people') + ->group('age') + ->field('age') + ->field($this->q()->groupConcat('name', ',')); + + if ($this->getDatabasePlatform() instanceof MySQLPlatform) { + $this->assertSame([ + 'select `age`, group_concat(`name` separator \',\') from `people` group by `age`', + [], + ], $q->render()); + } elseif ($this->getDatabasePlatform() instanceof PostgreSQLPlatform) { + $this->assertSame([ + 'select "age", string_agg("name", :a) from "people" group by "age"', + [':a' => ','], + ], $q->render()); + } elseif ($this->getDatabasePlatform() instanceof SQLServerPlatform) { + $this->assertSame([ + 'select [age], string_agg([name], N\',\') from [people] group by [age]', + [], + ], $q->render()); + } elseif ($this->getDatabasePlatform() instanceof OraclePlatform) { + $this->assertSame([ + 'select "age", listagg("name", :xxaaaa) within group (order by "name") from "people" group by "age"', + [':xxaaaa' => ','], + ], $q->render()); + } else { + $this->assertSame([ + 'select "age", group_concat("name", :a) from "people" group by "age"', + [':a' => ','], + ], $q->render()); + } + } + + public function testExists(): void + { + $q = $this->q() + ->table('contacts') + ->where('first_name', 'John') + ->exists(); + + if ($this->getDatabasePlatform() instanceof MySQLPlatform) { + $this->assertSame([ + 'select exists (select * from `contacts` where `first_name` = :a)', + [':a' => 'John'], + ], $q->render()); + } elseif ($this->getDatabasePlatform() instanceof PostgreSQLPlatform) { + $this->assertSame([ + 'select exists (select * from "contacts" where "first_name" = :a)', + [':a' => 'John'], + ], $q->render()); + } elseif ($this->getDatabasePlatform() instanceof SQLServerPlatform) { + $this->assertSame([ + 'select case when exists(select * from [contacts] where [first_name] = :a) then 1 else 0 end', + [':a' => 'John'], + ], $q->render()); + } elseif ($this->getDatabasePlatform() instanceof OraclePlatform) { + $this->assertSame([ + 'select case when exists(select * from "contacts" where "first_name" = :xxaaaa) then 1 else 0 end from "DUAL"', + [':xxaaaa' => 'John'], + ], $q->render()); + } else { + $this->assertSame([ + 'select exists (select * from "contacts" where "first_name" = :a)', + [':a' => 'John'], + ], $q->render()); + } + } + public function testExecuteException(): void { $this->expectException(ExecuteException::class); diff --git a/tests/RandomTest.php b/tests/RandomTest.php index 62c35fabd..5cc0bff05 100644 --- a/tests/RandomTest.php +++ b/tests/RandomTest.php @@ -322,13 +322,14 @@ public function testUpdateCondition(): void $m->set('name', 'Peter'); try { + $this->expectException(Exception::class); + $m->save(); - $e = null; } catch (\Exception $e) { - } + $this->assertEquals($dbData, $this->getDb()); - $this->assertNotNull($e); - $this->assertEquals($dbData, $this->getDb()); + throw $e; + } } public function testHookBreakers(): void diff --git a/tests/ReferenceTest.php b/tests/ReferenceTest.php index 34baedd6c..76c841212 100644 --- a/tests/ReferenceTest.php +++ b/tests/ReferenceTest.php @@ -80,6 +80,7 @@ public function testRefName1(): void $order->addField('user_id'); $user->hasMany('Orders', ['model' => $order]); + $this->expectException(Exception::class); $user->hasMany('Orders', ['model' => $order]); } @@ -90,6 +91,7 @@ public function testRefName2(): void $user = new Model(null, ['table' => 'user']); $user->hasOne('user_id', ['model' => $user]); + $this->expectException(Exception::class); $user->hasOne('user_id', ['model' => $user]); } @@ -100,6 +102,7 @@ public function testRefName3(): void $order->addRef('archive', ['model' => function ($m) { return new $m(null, ['table' => $m->table . '_archive']); }]); + $this->expectException(Exception::class); $order->addRef('archive', ['model' => function ($m) { return new $m(null, ['table' => $m->table . '_archive']); diff --git a/tests/Schema/ModelTest.php b/tests/Schema/ModelTest.php index 192ddb36c..fecaa0d39 100644 --- a/tests/Schema/ModelTest.php +++ b/tests/Schema/ModelTest.php @@ -6,8 +6,10 @@ use Atk4\Data\Field\PasswordField; use Atk4\Data\Model; +use Atk4\Data\Persistence\Sql\Expression; use Atk4\Data\Schema\TestCase; use Doctrine\DBAL\Platforms\OraclePlatform; +use Doctrine\DBAL\Platforms\PostgreSQLPlatform; use Doctrine\DBAL\Platforms\SQLServerPlatform; class ModelTest extends TestCase @@ -214,6 +216,53 @@ public function testCharacterTypeFieldLong(string $type, bool $isBinary, int $le $this->assertSame(2, $row['id']); $this->assertSame(strlen($str), strlen($row['v'])); $this->assertTrue($str === $row['v']); + + // remove once https://github.com/php/php-src/issues/8928 is fixed + if (str_starts_with($_ENV['DB_DSN'], 'oci8') && $length > 1000) { + return; + } + + // functional test for Expression::escapeStringLiteral() method + $strRaw = $model->getPersistence()->typecastSaveField($model->getField('v'), $str); + $strRawSql = \Closure::bind(function () use ($model, $strRaw) { + return $model->expr('')->escapeStringLiteral($strRaw); + }, null, Expression::class)(); + $query = $this->db->getConnection()->dsql() + ->field($model->expr($strRawSql)); + $resRaw = $query->getOne(); + if ($this->getDatabasePlatform() instanceof OraclePlatform && $isBinary) { + $this->assertNotSame(strlen($str), strlen($resRaw)); + } else { + $this->assertSame(strlen($str), strlen($resRaw)); + $this->assertTrue($str === $resRaw); + } + $res = $model->getPersistence()->typecastLoadField($model->getField('v'), $resRaw); + $this->assertSame(strlen($str), strlen($res)); + $this->assertTrue($str === $res); + + if (!$isBinary) { + $str = $this->makePseudoRandomString($isBinary, $length); + + // PostgreSQL does not support \0 character + // https://stackoverflow.com/questions/1347646/postgres-error-on-insert-error-invalid-byte-sequence-for-encoding-utf8-0x0 + if ($this->getDatabasePlatform() instanceof PostgreSQLPlatform) { + $str = str_replace("\0", '-', $str); + } + + $this->assertSame($length, mb_strlen($str)); + $strSql = \Closure::bind(function () use ($model, $str) { + return $model->expr('')->escapeStringLiteral($str); + }, null, Expression::class)(); + $query = $this->db->getConnection()->dsql() + ->field($model->expr($strSql)); + $res = $query->getOne(); + if ($this->getDatabasePlatform() instanceof OraclePlatform && $length === 0) { + $this->assertNull($res); + } else { + $this->assertSame(strlen($str), strlen($res)); + $this->assertTrue($str === $res); + } + } } public function providerCharacterTypeFieldLongData(): array diff --git a/tests/ScopeTest.php b/tests/ScopeTest.php index ea27847f9..7627b38f6 100644 --- a/tests/ScopeTest.php +++ b/tests/ScopeTest.php @@ -131,11 +131,11 @@ public function testConditionToWords(): void $condition = new Condition('country_id/code', 'US'); - $this->assertEquals('User that has reference Country Id where Code is equal to \'US\'', $condition->toWords($user)); + $this->assertEquals('User that has reference Country ID where Code is equal to \'US\'', $condition->toWords($user)); $condition = new Condition('country_id', 2); - $this->assertEquals('Country Id is equal to 2 (\'Latvia\')', $condition->toWords($user)); + $this->assertEquals('Country ID is equal to 2 (\'Latvia\')', $condition->toWords($user)); if ($this->getDatabasePlatform() instanceof SqlitePlatform) { $condition = new Condition('name', $user->expr('[surname]')); @@ -145,7 +145,7 @@ public function testConditionToWords(): void $condition = new Condition('country_id', null); - $this->assertEquals('Country Id is equal to empty', $condition->toWords($user)); + $this->assertEquals('Country ID is equal to empty', $condition->toWords($user)); $condition = new Condition('name', '>', 'Test'); @@ -153,7 +153,7 @@ public function testConditionToWords(): void $condition = (new Condition('country_id', 2))->negate(); - $this->assertEquals('Country Id is not equal to 2 (\'Latvia\')', $condition->toWords($user)); + $this->assertEquals('Country ID is not equal to 2 (\'Latvia\')', $condition->toWords($user)); $condition = new Condition($user->getField('surname'), $user->getField('name')); diff --git a/tests/SmboTransferTest.php b/tests/SmboTransferTest.php index 9b5fcac0b..0999b2d85 100644 --- a/tests/SmboTransferTest.php +++ b/tests/SmboTransferTest.php @@ -6,7 +6,6 @@ use Atk4\Data\Schema\TestCase; use Atk4\Data\Tests\Model\Smbo\Account; -use Atk4\Data\Tests\Model\Smbo\Company; use Atk4\Data\Tests\Model\Smbo\Payment; use Atk4\Data\Tests\Model\Smbo\Transfer; @@ -24,8 +23,6 @@ protected function setUp(): void $this->createMigrator()->table('document') ->id() ->field('reference') - ->field('contact_from_id') - ->field('contact_to_id') ->field('doc_type') ->field('amount', ['type' => 'float']) ->create(); @@ -103,77 +100,4 @@ public function testRef(): void ['amount' => 20.0], ], $data); } - - /* - public function testBasicEntities(): void - { - // Create a new company - $company = new Company($this->db); - $company->set([ - 'name' => 'Test Company 1', - 'director_name' => 'Tester Little', - 'type' => 'Limited Company', - 'vat_registered' => true, - ]); - $company->save(); - - return; - - // Create two new clients, one is sole trader, other is limited company - $client = $company->ref('Client'); - [$john_id, $agile_id] = $m->insert([ - ['name' => 'John Smith Consulting', 'vat_registered' => false], - 'Agile Software Limited', - ]); - - // Insert a first, default invoice for our sole-trader - $john = $company->load($john_id); - $john_invoices = $john->ref('Invoice'); - $john_invoices->insertInvoice([ - 'ref_no' => 'INV1', - 'due_date' => (new Date())->add(new DateInterval('2w')), // due in 2 weeks - 'lines' => [ - ['descr' => 'Sold some sweets', 'total_gross' => 100.0], - ['descr' => 'Delivery', 'total_gross' => 10.0], - ], - ]); - - // Use custom method to create a sub-nominal - $company->ref('Nominal')->insertSubNominal('Sales', 'Discounted'); - - // Insert our second invoice using set referencing - $company->ref('Client')->load($agile_id)->refSet('Invoice')->insertInvoice([ - 'lines' => [ - [ - 'item_id' => $john->ref('Product')->insert('Cat Food'), - 'nominal' => 'Sales:Discounted', - 'total_net' => 50.0, - 'vat_rate' => 23, - // calculates total_gross at 61.5 - ], - [ - 'item_id' => $john->ref('Service')->insert('Delivery'), - 'total_net' => 10.0, - 'vat_rate' => '23%', - // calculates total_gross at 12.3 - ], - ], - ]); - - // Next we create bank account - $hsbc = $john->ref('Account')->set('name', 'HSBC')->save(); - - // And each of our invoices will have one new payment - foreach ($john_invoices as $invoice) { - $invoice->ref('Payment')->insert(['amount' => 10.2, 'bank_account_id' => $hsbc]); - } - - // Now let's execute report - $debt = $john->add(new Model_Report_Debtors()); - - // This should give us total amount owed by all clients: - // (100.0 + 10.0) + (61.5 + 12.3) - 10.2 * 2 - $this->assertSame(163.4, $debt->sum('amount')); - } - */ } diff --git a/tests/SubTypesTest.php b/tests/SubTypesTest.php index da35dee7d..cb8672144 100644 --- a/tests/SubTypesTest.php +++ b/tests/SubTypesTest.php @@ -171,12 +171,29 @@ public function testBasic(): void $this->assertInstanceOf(StTransaction_TransferIn::class, $current->ref('Transactions')->load(3)); $this->assertInstanceOf(StTransaction_Withdrawal::class, $current->ref('Transactions')->load(4)); - $classes = []; - foreach ($current->ref('Transactions') as $tr) { - $classes[] = get_class($tr); - } - sort($classes); - - $this->assertSame([StTransaction_TransferIn::class, StTransaction_Withdrawal::class], $classes); + $assertClassesFx = function (array $expectedClasses) use ($current): void { + $classes = []; + foreach ($current->ref('Transactions')->setOrder('id') as $tr) { + $classes[] = get_class($tr); + } + $this->assertSame($expectedClasses, $classes); + }; + + $assertClassesFx([ + StTransaction_TransferIn::class, + StTransaction_Withdrawal::class, + ]); + + $current->deposit(50); + $current->deposit(50); + $this->assertInstanceOf(StTransaction_Deposit::class, $current->ref('Transactions')->load(5)); + $this->assertInstanceOf(StTransaction_Deposit::class, $current->ref('Transactions')->load(5)); + + $assertClassesFx([ + StTransaction_TransferIn::class, + StTransaction_Withdrawal::class, + StTransaction_Deposit::class, + StTransaction_Deposit::class, + ]); } } diff --git a/tests/UserActionTest.php b/tests/UserActionTest.php index 7caacf7a3..b7d666d44 100644 --- a/tests/UserActionTest.php +++ b/tests/UserActionTest.php @@ -123,8 +123,9 @@ public function testPreview(): void public function testPreviewFail(): void { - $this->expectExceptionMessage('specify preview callback'); $client = new UaClient($this->pers); + + $this->expectExceptionMessage('specify preview callback'); $client->getUserAction('backup_clients')->preview(); } @@ -159,8 +160,9 @@ public function testAppliesTo3(): void public function testException1(): void { - $this->expectException(\Atk4\Core\Exception::class); $client = new UaClient($this->pers); + + $this->expectException(\Atk4\Core\Exception::class); $client->getUserAction('non_existant_action'); } @@ -224,9 +226,9 @@ public function testFieldsTooDirty1(): void $this->assertNotSame('Peter', $client->get('name')); $client->set('name', 'Peter'); $client->set('reminder_sent', true); + $this->expectExceptionMessage('dirty fields'); $client->getUserAction('change_details')->execute(); - $this->assertSame('Peter', $client->get('name')); } public function testFieldsIncorrect(): void @@ -238,9 +240,9 @@ public function testFieldsIncorrect(): void $this->assertNotSame('Peter', $client->get('name')); $client->set('name', 'Peter'); + $this->expectExceptionMessage('must be either array or boolean'); $client->getUserAction('change_details')->execute(); - $this->assertSame('Peter', $client->get('name')); } public function testConfirmation(): void diff --git a/tests/ValidationTest.php b/tests/ValidationTest.php index 9a6deab1c..0cc24a78e 100644 --- a/tests/ValidationTest.php +++ b/tests/ValidationTest.php @@ -95,12 +95,13 @@ public function testValidate4(): void try { $m->set('name', 'Python'); $m->set('domain', 'example.com'); + + $this->expectException(ValidationException::class); $m->save(); - $this->fail('Expected exception'); - } catch (\Atk4\Data\ValidationException $e) { + } catch (ValidationException $e) { $this->assertSame('This domain is reserved for examples only', $e->getParams()['errors']['domain']); - return; + throw $e; } } @@ -115,10 +116,9 @@ public function testValidate5(): void $m->save(); } - public function testValidateHook(): void + public function testValidateHook1(): void { $m = $this->m->createEntity(); - $m->onHook(Model::HOOK_VALIDATE, static function ($m) { if ($m->get('name') === 'C#') { return ['name' => 'No sharp objects allowed']; @@ -130,19 +130,38 @@ public function testValidateHook(): void try { $m->set('name', 'C#'); + + $this->expectException(ValidationException::class); $m->save(); - $this->fail('Expected exception'); - } catch (\Atk4\Data\ValidationException $e) { + } catch (ValidationException $e) { $this->assertSame('No sharp objects allowed', $e->errors['name']); + + throw $e; } + } + + public function testValidateHook2(): void + { + $m = $this->m->createEntity(); + $m->onHook(Model::HOOK_VALIDATE, static function ($m) { + if ($m->get('name') === 'C#') { + return ['name' => 'No sharp objects allowed']; + } + }); + + $m->set('name', 'Swift'); + $m->save(); try { $m->set('name', 'Python'); $m->set('domain', 'example.com'); + + $this->expectException(ValidationException::class); $m->save(); - $this->fail('Expected exception'); - } catch (\Atk4\Data\ValidationException $e) { + } catch (ValidationException $e) { $this->assertCount(2, $e->errors); + + throw $e; } } }