diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 3da4eb44e..c2c059200 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -28,10 +28,10 @@ parameters: # for Doctrine DBAL 2.x, remove the support once Doctrine ORM 2.10 is released # see https://github.com/doctrine/orm/issues/8526 - - message: '~^(Call to an undefined method Doctrine\\DBAL\\Driver\\Connection::getWrappedConnection\(\)\.|Caught class Doctrine\\DBAL\\DBALException not found\.|Call to static method notSupported\(\) on an unknown class Doctrine\\DBAL\\DBALException\.|Access to an undefined property Doctrine\\DBAL\\Driver\\PDO\\Connection::\$connection\.|Method Atk4\\Data\\Persistence\\Sql\\Expression::execute\(\) should return Doctrine\\DBAL\\Result\|PDOStatement but returns bool\.|Class Doctrine\\DBAL\\Platforms\\MySqlPlatform referenced with incorrect case: Doctrine\\DBAL\\Platforms\\MySQLPlatform\.)$~' + message: '~^(Call to an undefined method Doctrine\\DBAL\\Driver\\Connection::getWrappedConnection\(\)\.|Caught class Doctrine\\DBAL\\DBALException not found\.|Call to static method notSupported\(\) on an unknown class Doctrine\\DBAL\\DBALException\.|Access to an undefined property Doctrine\\DBAL\\Driver\\PDO\\Connection::\$connection\.|Parameter #1 \$dsn of class Doctrine\\DBAL\\Driver\\PDO\\SQLSrv\\Connection constructor expects string, Doctrine\\DBAL\\Driver\\PDO\\Connection given\.|Method Atk4\\Data\\Persistence\\Sql\\Expression::execute\(\) should return Doctrine\\DBAL\\Result\|PDOStatement but returns bool\.|Class Doctrine\\DBAL\\Platforms\\MySqlPlatform referenced with incorrect case: Doctrine\\DBAL\\Platforms\\MySQLPlatform\.)$~' path: '*' # count for DBAL 3.x matched in "src/Persistence/GenericPlatform.php" file - count: 11 + count: 12 # TODO these rules are generated, this ignores should be fixed in the code # for src/Schema/TestCase.php diff --git a/src/Persistence/GenericPlatform.php b/src/Persistence/GenericPlatform.php index 02aa5f95b..383db6a87 100644 --- a/src/Persistence/GenericPlatform.php +++ b/src/Persistence/GenericPlatform.php @@ -22,6 +22,7 @@ private function createNotSupportedException(): \Exception // DbalException once \Doctrine\DBAL\DBALException::notSupported('SQL'); \Doctrine\DBAL\DBALException::notSupported('SQL'); \Doctrine\DBAL\DBALException::notSupported('SQL'); + \Doctrine\DBAL\DBALException::notSupported('SQL'); } return \Doctrine\DBAL\DBALException::notSupported('SQL'); diff --git a/src/Persistence/Sql/BinaryTypeCompatibilityTypecastTrait.php b/src/Persistence/Sql/BinaryTypeCompatibilityTypecastTrait.php index 065e1dbe6..59a35b8f4 100644 --- a/src/Persistence/Sql/BinaryTypeCompatibilityTypecastTrait.php +++ b/src/Persistence/Sql/BinaryTypeCompatibilityTypecastTrait.php @@ -42,12 +42,19 @@ private function binaryTypeValueDecode(string $value): string throw new Exception('Unexpected binary value crc'); } - return hex2bin($hex); + $res = hex2bin($hex); + if ($this->binaryTypeValueIsEncoded($res)) { + throw new Exception('Unexpected double encoded binary value'); + } + + return $res; } private function binaryTypeIsEncodeNeeded(Type $type): bool { - // TODO PostgreSQL tests fail without binary compatibility typecast + // binary values for PostgreSQL and MSSQL databases are stored natively, but we need + // to encode first to hold the binary type info for PDO parameter type binding + $platform = $this->getDatabasePlatform(); if ($platform instanceof PostgreSQL94Platform || $platform instanceof SQLServer2012Platform @@ -76,7 +83,11 @@ public function typecastLoadField(Field $field, $value) $value = parent::typecastLoadField($field, $value); if ($value !== null && $this->binaryTypeIsEncodeNeeded($field->getTypeObject())) { - $value = $this->binaryTypeValueDecode($value); + // always decode for Oracle platform to assert the value is always encoded, + // on other platforms, binary values are stored natively + if ($this->getDatabasePlatform() instanceof OraclePlatform || $this->binaryTypeValueIsEncoded($value)) { + $value = $this->binaryTypeValueDecode($value); + } } return $value; diff --git a/src/Persistence/Sql/Connection.php b/src/Persistence/Sql/Connection.php index c2df6131f..9907dc158 100644 --- a/src/Persistence/Sql/Connection.php +++ b/src/Persistence/Sql/Connection.php @@ -132,22 +132,22 @@ public static function connect($dsn, $user = null, $password = null, $args = []) // If it's already PDO or DbalConnection object, then we simply use it if ($dsn instanceof \PDO) { $connectionClass = self::resolveConnectionClass($dsn->getAttribute(\PDO::ATTR_DRIVER_NAME)); - $connectionArg = $connectionClass::connectDbalConnection(['pdo' => $dsn]); + $dbalConnection = $connectionClass::connectDbalConnection(['pdo' => $dsn]); } elseif ($dsn instanceof DbalConnection) { /** @var \PDO */ $pdo = self::isComposerDbal2x() ? $dsn->getWrappedConnection() : $dsn->getWrappedConnection()->getWrappedConnection(); $connectionClass = self::resolveConnectionClass($pdo->getAttribute(\PDO::ATTR_DRIVER_NAME)); - $connectionArg = $dsn; + $dbalConnection = $dsn; } else { $dsn = static::normalizeDsn($dsn, $user, $password); $connectionClass = self::resolveConnectionClass($dsn['driverSchema']); - $connectionArg = $connectionClass::connectDbalConnection($dsn); + $dbalConnection = $connectionClass::connectDbalConnection($dsn); } return new $connectionClass(array_merge([ - 'connection' => $connectionArg, + 'connection' => $dbalConnection, ], $args)); } @@ -241,8 +241,13 @@ protected static function connectDbalConnection(array $dsn) \Closure::bind(function () use ($pdoConnection, $pdo): void { $pdoConnection->connection = $pdo; }, null, \Doctrine\DBAL\Driver\PDO\Connection::class)(); + $pdoAttrDriverName = $pdo->getAttribute(\PDO::ATTR_DRIVER_NAME); + if ($pdoAttrDriverName === 'sqlsrv') { + $pdoConnection = new \Doctrine\DBAL\Driver\PDO\SQLSrv\Connection($pdoConnection); + } + $dbalConnection = DriverManager::getConnection([ - 'driver' => 'pdo_' . $pdo->getAttribute(\PDO::ATTR_DRIVER_NAME), + 'driver' => 'pdo_' . $pdoAttrDriverName, ], null, (static::class)::createDbalEventManager()); \Closure::bind(function () use ($dbalConnection, $pdoConnection): void { $dbalConnection->_conn = $pdoConnection; diff --git a/src/Persistence/Sql/Expression.php b/src/Persistence/Sql/Expression.php index 5957b4ce2..deb55dcbe 100644 --- a/src/Persistence/Sql/Expression.php +++ b/src/Persistence/Sql/Expression.php @@ -5,11 +5,13 @@ namespace Atk4\Data\Persistence\Sql; use Atk4\Core\WarnDynamicPropertyTrait; +use Atk4\Data\Persistence\Sql as SqlPersistence; use Doctrine\DBAL\Connection as DbalConnection; use Doctrine\DBAL\Exception as DbalException; use Doctrine\DBAL\ParameterType; use Doctrine\DBAL\Platforms\OraclePlatform; use Doctrine\DBAL\Platforms\PostgreSQL94Platform; +use Doctrine\DBAL\Platforms\SQLServer2012Platform; use Doctrine\DBAL\Result as DbalResult; /** @@ -528,6 +530,15 @@ public function execute(object $connection = null): object $type = ParameterType::STRING; } elseif (is_string($val)) { $type = ParameterType::STRING; + + if ($this->connection->getDatabasePlatform() instanceof PostgreSQL94Platform + || $this->connection->getDatabasePlatform() instanceof SQLServer2012Platform) { + $dummySqlPersistence = new SqlPersistence($this->connection); + if (\Closure::bind(fn () => $dummySqlPersistence->binaryTypeValueIsEncoded($val), null, SqlPersistence::class)()) { + $val = \Closure::bind(fn () => $dummySqlPersistence->binaryTypeValueDecode($val), null, SqlPersistence::class)(); + $type = ParameterType::BINARY; + } + } } elseif (is_resource($val)) { throw new Exception('Resource type is not supported, set value as string instead'); } else { diff --git a/src/Persistence/Sql/Mssql/PlatformTrait.php b/src/Persistence/Sql/Mssql/PlatformTrait.php index b93890094..6be0675f1 100644 --- a/src/Persistence/Sql/Mssql/PlatformTrait.php +++ b/src/Persistence/Sql/Mssql/PlatformTrait.php @@ -6,19 +6,6 @@ trait PlatformTrait { - // SQL Server database requires explicit conversion when using binary column, - // workaround by using a standard non-binary column with custom encoding/typecast - - protected function getBinaryTypeDeclarationSQLSnippet($length, $fixed) - { - return $this->getVarcharTypeDeclarationSQLSnippet($length, $fixed); - } - - public function getBlobTypeDeclarationSQL(array $column) - { - return $this->getClobTypeDeclarationSQL($column); - } - // remove once https://github.com/doctrine/dbal/pull/4987 is fixed // and also $this->markDoctrineTypeCommented('text') below public function getClobTypeDeclarationSQL(array $column) @@ -32,8 +19,6 @@ protected function initializeCommentedDoctrineTypes() { parent::initializeCommentedDoctrineTypes(); - $this->markDoctrineTypeCommented('binary'); - $this->markDoctrineTypeCommented('blob'); $this->markDoctrineTypeCommented('text'); } diff --git a/tests/Schema/ModelTest.php b/tests/Schema/ModelTest.php index a48998194..abeb6435d 100644 --- a/tests/Schema/ModelTest.php +++ b/tests/Schema/ModelTest.php @@ -7,6 +7,7 @@ use Atk4\Data\Model; use Atk4\Data\Schema\TestCase; use Doctrine\DBAL\Platforms\OraclePlatform; +use Doctrine\DBAL\Platforms\SQLServer2012Platform; class ModelTest extends TestCase { @@ -186,7 +187,7 @@ private function makePseudoRandomString(bool $isBinary, int $lengthBytes): strin public function testCharacterTypeFieldLong(string $type, bool $isBinary, int $lengthBytes): void { if ($this->getDatabasePlatform() instanceof OraclePlatform) { - $lengthBytes = min($lengthBytes, 500); + $lengthBytes = min($lengthBytes, $type === 'binary' ? 100 : 500); } $str = $this->makePseudoRandomString($isBinary, $lengthBytes); @@ -200,7 +201,11 @@ public function testCharacterTypeFieldLong(string $type, bool $isBinary, int $le $this->createMigrator($model)->create(); - $model->import([['v' => $str . ($isBinary ? "\0" : '.')]]); + $model->import([['v' => $str . ( + // MSSQL database ignores trailing \0 characters even with binary comparison + // https://dba.stackexchange.com/questions/48660/comparing-binary-0x-and-0x00-turns-out-to-be-equal-on-sql-server + $isBinary ? $this->getDatabasePlatform() instanceof SQLServer2012Platform ? ' ' : "\0" : '.' + )]]); $model->import([['v' => $str]]); $model->addCondition('v', $str); @@ -218,7 +223,7 @@ public function providerCharacterTypeFieldLongData(): array { return [ ['string', false, 250], - ['binary', true, 100], + ['binary', true, 250], ['text', false, 256 * 1024], ['blob', true, 256 * 1024], ];