From afb40db91677d553ae637efa159d8b2229748f0a Mon Sep 17 00:00:00 2001 From: Jan Nedbal Date: Wed, 26 Jun 2024 14:03:27 +0200 Subject: [PATCH] Avoid real connection for type inference --- README.md | 3 - extension.neon | 2 - src/Doctrine/Driver/DriverDetector.php | 157 +++++++----------- .../Doctrine/Query/QueryResultTypeWalker.php | 71 +------- ...eryResultTypeWalkerFetchTypeMatrixTest.php | 151 +++-------------- tests/Platform/UnknownDriver.php | 26 +++ .../Doctrine/ORM/EntityColumnRuleTest.php | 2 +- 7 files changed, 118 insertions(+), 294 deletions(-) create mode 100644 tests/Platform/UnknownDriver.php diff --git a/README.md b/README.md index e59c66c3..bba1119d 100644 --- a/README.md +++ b/README.md @@ -132,9 +132,6 @@ Most DQL features are supported, including `GROUP BY`, `DISTINCT`, all flavors o Whether e.g. `SUM(e.column)` is fetched as `float`, `numeric-string` or `int` highly [depends on drivers, their setup and PHP version](https://github.com/janedbal/php-database-drivers-fetch-test). This extension autodetects your setup and provides quite accurate results for `pdo_mysql`, `mysqli`, `pdo_sqlite`, `sqlite3`, `pdo_pgsql` and `pgsql`. -Sadly, this autodetection often needs real database connection, so in order to utilize precise types, your `objectManagerLoader` need to be able to connect to real database. - -If you are using `bleedingEdge`, the connection failure is propagated. If not, it will be silently ignored and the type will be `mixed` or an union of possible types. ### Supported methods diff --git a/extension.neon b/extension.neon index 2f8daea1..fe686a66 100644 --- a/extension.neon +++ b/extension.neon @@ -91,8 +91,6 @@ services: - class: PHPStan\Doctrine\Driver\DriverDetector - arguments: - failOnInvalidConnection: %featureToggles.bleedingEdge% - class: PHPStan\Reflection\Doctrine\DoctrineSelectableClassReflectionExtension - diff --git a/src/Doctrine/Driver/DriverDetector.php b/src/Doctrine/Driver/DriverDetector.php index 674585b6..0c9e5df0 100644 --- a/src/Doctrine/Driver/DriverDetector.php +++ b/src/Doctrine/Driver/DriverDetector.php @@ -14,14 +14,8 @@ use Doctrine\DBAL\Driver\PgSQL\Driver as PgSQLDriver; use Doctrine\DBAL\Driver\SQLite3\Driver as SQLite3Driver; use Doctrine\DBAL\Driver\SQLSrv\Driver as SqlSrvDriver; -use mysqli; -use PDO; -use SQLite3; -use Throwable; -use function get_resource_type; -use function is_resource; -use function method_exists; -use function strpos; +use function get_class; +use function is_a; class DriverDetector { @@ -38,139 +32,114 @@ class DriverDetector public const SQLITE3 = 'sqlite3'; public const SQLSRV = 'sqlsrv'; - /** @var bool */ - private $failOnInvalidConnection; - - public function __construct(bool $failOnInvalidConnection) + /** + * @return self::*|null + */ + public function detect(Connection $connection): ?string { - $this->failOnInvalidConnection = $failOnInvalidConnection; + $driver = $connection->getDriver(); + + return $this->deduceFromDriverClass(get_class($driver)) ?? $this->deduceFromParams($connection); } - public function failsOnInvalidConnection(): bool + /** + * @return array + */ + public function detectDriverOptions(Connection $connection): array { - return $this->failOnInvalidConnection; + return $connection->getParams()['driverOptions'] ?? []; } /** * @return self::*|null */ - public function detect(Connection $connection): ?string + private function deduceFromDriverClass(string $driverClass): ?string { - $driver = $connection->getDriver(); - - if ($driver instanceof MysqliDriver) { + if (is_a($driverClass, MysqliDriver::class, true)) { return self::MYSQLI; } - if ($driver instanceof PdoMysqlDriver) { + if (is_a($driverClass, PdoMysqlDriver::class, true)) { return self::PDO_MYSQL; } - if ($driver instanceof PdoSQLiteDriver) { + if (is_a($driverClass, PdoSQLiteDriver::class, true)) { return self::PDO_SQLITE; } - if ($driver instanceof PdoSqlSrvDriver) { + if (is_a($driverClass, PdoSqlSrvDriver::class, true)) { return self::PDO_SQLSRV; } - if ($driver instanceof PdoOciDriver) { + if (is_a($driverClass, PdoOciDriver::class, true)) { return self::PDO_OCI; } - if ($driver instanceof PdoPgSQLDriver) { + if (is_a($driverClass, PdoPgSQLDriver::class, true)) { return self::PDO_PGSQL; } - if ($driver instanceof SQLite3Driver) { + if (is_a($driverClass, SQLite3Driver::class, true)) { return self::SQLITE3; } - if ($driver instanceof PgSQLDriver) { + if (is_a($driverClass, PgSQLDriver::class, true)) { return self::PGSQL; } - if ($driver instanceof SqlSrvDriver) { + if (is_a($driverClass, SqlSrvDriver::class, true)) { return self::SQLSRV; } - if ($driver instanceof Oci8Driver) { + if (is_a($driverClass, Oci8Driver::class, true)) { return self::OCI8; } - if ($driver instanceof IbmDb2Driver) { + if (is_a($driverClass, IbmDb2Driver::class, true)) { return self::IBM_DB2; } - // fallback to connection-based detection when driver is wrapped by middleware - - if (!method_exists($connection, 'getNativeConnection')) { - return null; // dbal < 3.3 (released in 2022-01) - } - - try { - $nativeConnection = $connection->getNativeConnection(); - } catch (Throwable $e) { - if ($this->failOnInvalidConnection) { - throw $e; - } - return null; // connection cannot be established - } - - if ($nativeConnection instanceof mysqli) { - return self::MYSQLI; - } - - if ($nativeConnection instanceof SQLite3) { - return self::SQLITE3; - } - - if ($nativeConnection instanceof \PgSql\Connection) { - return self::PGSQL; - } - - if ($nativeConnection instanceof PDO) { - $driverName = $nativeConnection->getAttribute(PDO::ATTR_DRIVER_NAME); - - if ($driverName === 'mysql') { - return self::PDO_MYSQL; - } - - if ($driverName === 'sqlite') { - return self::PDO_SQLITE; - } - - if ($driverName === 'pgsql') { - return self::PDO_PGSQL; - } - - if ($driverName === 'oci') { // semi-verified (https://stackoverflow.com/questions/10090709/get-current-pdo-driver-from-existing-connection/10090754#comment12923198_10090754) - return self::PDO_OCI; - } + return null; + } - if ($driverName === 'sqlsrv') { - return self::PDO_SQLSRV; + /** + * @return self::*|null + */ + private function deduceFromParams(Connection $connection): ?string + { + $params = $connection->getParams(); + + if (isset($params['driver'])) { + switch ($params['driver']) { + case 'pdo_mysql': + return self::PDO_MYSQL; + case 'pdo_sqlite': + return self::PDO_SQLITE; + case 'pdo_pgsql': + return self::PDO_PGSQL; + case 'pdo_oci': + return self::PDO_OCI; + case 'oci8': + return self::OCI8; + case 'ibm_db2': + return self::IBM_DB2; + case 'pdo_sqlsrv': + return self::PDO_SQLSRV; + case 'mysqli': + return self::MYSQLI; + case 'pgsql': // @phpstan-ignore-line never matches on PHP 7.3- with old dbal + return self::PGSQL; + case 'sqlsrv': + return self::SQLSRV; + case 'sqlite3': // @phpstan-ignore-line never matches on PHP 7.3- with old dbal + return self::SQLITE3; + default: + return null; } } - if (is_resource($nativeConnection)) { - $resourceType = get_resource_type($nativeConnection); - - if (strpos($resourceType, 'oci') !== false) { // not verified - return self::OCI8; - } - - if (strpos($resourceType, 'db2') !== false) { // not verified - return self::IBM_DB2; - } - - if (strpos($resourceType, 'SQL Server Connection') !== false) { - return self::SQLSRV; - } - - if (strpos($resourceType, 'pgsql link') !== false) { - return self::PGSQL; - } + if (isset($params['driverClass'])) { + return $this->deduceFromDriverClass($params['driverClass']); } return null; diff --git a/src/Type/Doctrine/Query/QueryResultTypeWalker.php b/src/Type/Doctrine/Query/QueryResultTypeWalker.php index f2e8c051..e5040fb8 100644 --- a/src/Type/Doctrine/Query/QueryResultTypeWalker.php +++ b/src/Type/Doctrine/Query/QueryResultTypeWalker.php @@ -14,7 +14,6 @@ use Doctrine\ORM\Query\ParserResult; use Doctrine\ORM\Query\SqlWalker; use PDO; -use PDOException; use PHPStan\Doctrine\Driver\DriverDetector; use PHPStan\Php\PhpVersion; use PHPStan\ShouldNotHappenException; @@ -41,7 +40,6 @@ use PHPStan\Type\TypeCombinator; use PHPStan\Type\TypeTraverser; use PHPStan\Type\UnionType; -use Throwable; use function array_key_exists; use function array_map; use function array_values; @@ -55,7 +53,6 @@ use function is_numeric; use function is_object; use function is_string; -use function method_exists; use function serialize; use function sprintf; use function stripos; @@ -108,6 +105,9 @@ class QueryResultTypeWalker extends SqlWalker /** @var DriverDetector::*|null */ private $driverType; + /** @var array */ + private $driverOptions; + /** * Map of all components/classes that appear in the DQL query. * @@ -130,8 +130,6 @@ class QueryResultTypeWalker extends SqlWalker /** @var bool */ private $hasGroupByClause; - /** @var bool */ - private $failOnInvalidConnection; /** * @param Query $query @@ -224,8 +222,10 @@ public function __construct($query, $parserResult, array $queryComponents) is_object($driverDetector) ? get_class($driverDetector) : gettype($driverDetector) )); } - $this->driverType = $driverDetector->detect($this->em->getConnection()); - $this->failOnInvalidConnection = $driverDetector->failsOnInvalidConnection(); + $connection = $this->em->getConnection(); + + $this->driverType = $driverDetector->detect($connection); + $this->driverOptions = $driverDetector->detectDriverOptions($connection); parent::__construct($query, $parserResult, $queryComponents); } @@ -2042,20 +2042,10 @@ private function hasAggregateWithoutGroupBy(): bool private function shouldStringifyExpressions(Type $type): TrinaryLogic { if (in_array($this->driverType, [DriverDetector::PDO_MYSQL, DriverDetector::PDO_PGSQL, DriverDetector::PDO_SQLITE], true)) { - try { - $nativeConnection = $this->getNativeConnection(); - assert($nativeConnection instanceof PDO); - } catch (Throwable $e) { // connection cannot be established - if ($this->failOnInvalidConnection) { - throw $e; - } - return TrinaryLogic::createMaybe(); - } - - $stringifyFetches = $this->isPdoStringifyEnabled($nativeConnection); + $stringifyFetches = isset($this->driverOptions[PDO::ATTR_STRINGIFY_FETCHES]) ? (bool) $this->driverOptions[PDO::ATTR_STRINGIFY_FETCHES] : false; if ($this->driverType === DriverDetector::PDO_MYSQL) { - $emulatedPrepares = $this->isPdoEmulatePreparesEnabled($nativeConnection); + $emulatedPrepares = isset($this->driverOptions[PDO::ATTR_EMULATE_PREPARES]) ? (bool) $this->driverOptions[PDO::ATTR_EMULATE_PREPARES] : true; if ($stringifyFetches) { return TrinaryLogic::createYes(); @@ -2105,49 +2095,6 @@ private function shouldStringifyExpressions(Type $type): TrinaryLogic return TrinaryLogic::createMaybe(); } - private function isPdoStringifyEnabled(PDO $pdo): bool - { - // this fails for most PHP versions, see https://github.com/php/php-src/issues/12969 - // working since 8.2.15 and 8.3.2 - try { - return (bool) $pdo->getAttribute(PDO::ATTR_STRINGIFY_FETCHES); - } catch (PDOException $e) { - $selectOne = $pdo->query('SELECT 1'); - if ($selectOne === false) { - return false; // this should not happen, just return attribute default value - } - $one = $selectOne->fetchColumn(); - - // string can be returned due to old PHP used or because ATTR_STRINGIFY_FETCHES is enabled, - // but it should not matter as it behaves the same way - // (the attribute is there to maintain BC) - return is_string($one); - } - } - - private function isPdoEmulatePreparesEnabled(PDO $pdo): bool - { - return (bool) $pdo->getAttribute(PDO::ATTR_EMULATE_PREPARES); - } - - /** - * @return object|resource|null - */ - private function getNativeConnection() - { - $connection = $this->em->getConnection(); - - if (method_exists($connection, 'getNativeConnection')) { - return $connection->getNativeConnection(); - } - - if ($connection->getWrappedConnection() instanceof PDO) { - return $connection->getWrappedConnection(); - } - - return null; - } - private function isSupportedDriver(): bool { return in_array($this->driverType, [ diff --git a/tests/Platform/QueryResultTypeWalkerFetchTypeMatrixTest.php b/tests/Platform/QueryResultTypeWalkerFetchTypeMatrixTest.php index 4543b9eb..0d1e3b34 100644 --- a/tests/Platform/QueryResultTypeWalkerFetchTypeMatrixTest.php +++ b/tests/Platform/QueryResultTypeWalkerFetchTypeMatrixTest.php @@ -77,9 +77,6 @@ final class QueryResultTypeWalkerFetchTypeMatrixTest extends PHPStanTestCase private const CONFIG_NO_EMULATE = 'pdo_no_emulate'; private const CONFIG_STRINGIFY_NO_EMULATE = 'pdo_stringify_no_emulate'; - private const INVALID_CONNECTION = 'invalid_connection'; - private const INVALID_CONNECTION_UNKNOWN_DRIVER = 'invalid_connection_and_unknown_driver'; - private const CONNECTION_CONFIGS = [ self::CONFIG_DEFAULT => [], self::CONFIG_STRINGIFY => [ @@ -582,8 +579,6 @@ public function testUnsupportedDriver( } /** - * Connection failure test - * * @param array $data * @param mixed $mysqlExpectedResult * @param mixed $sqliteExpectedResult @@ -594,93 +589,7 @@ public function testUnsupportedDriver( * * @dataProvider provideCases */ - public function testKnownDriverUnknownSetupDefault( - array $data, - string $dqlTemplate, - Type $mysqlExpectedType, - ?Type $sqliteExpectedType, - ?Type $pdoPgsqlExpectedType, - ?Type $pgsqlExpectedType, - ?Type $mssqlExpectedType, - $mysqlExpectedResult, - $sqliteExpectedResult, - $pdoPgsqlExpectedResult, - $pgsqlExpectedResult, - $mssqlExpectedResult, - string $stringify - ): void - { - $this->performDriverTest( - 'pdo_mysql', - self::CONFIG_DEFAULT, - $data, - $dqlTemplate, - (string) $this->dataName(), - PHP_VERSION_ID, - $this->determineTypeForKnownDriverUnknownSetup($mysqlExpectedType, $stringify), - $mysqlExpectedResult, - $stringify, - self::INVALID_CONNECTION - ); - } - - /** - * Connection failure test - * - * @param array $data - * @param mixed $mysqlExpectedResult - * @param mixed $sqliteExpectedResult - * @param mixed $pdoPgsqlExpectedResult - * @param mixed $pgsqlExpectedResult - * @param mixed $mssqlExpectedResult - * @param self::STRINGIFY_* $stringify - * - * @dataProvider provideCases - */ - public function testKnownDriverUnknownSetupStringify( - array $data, - string $dqlTemplate, - Type $mysqlExpectedType, - ?Type $sqliteExpectedType, - ?Type $pdoPgsqlExpectedType, - ?Type $pgsqlExpectedType, - ?Type $mssqlExpectedType, - $mysqlExpectedResult, - $sqliteExpectedResult, - $pdoPgsqlExpectedResult, - $pgsqlExpectedResult, - $mssqlExpectedResult, - string $stringify - ): void - { - $this->performDriverTest( - 'pdo_mysql', - self::CONFIG_STRINGIFY, - $data, - $dqlTemplate, - (string) $this->dataName(), - PHP_VERSION_ID, - $this->determineTypeForKnownDriverUnknownSetup($mysqlExpectedType, $stringify), - $mysqlExpectedResult, - $stringify, - self::INVALID_CONNECTION - ); - } - - /** - * Connection failure test - * - * @param array $data - * @param mixed $mysqlExpectedResult - * @param mixed $sqliteExpectedResult - * @param mixed $pdoPgsqlExpectedResult - * @param mixed $pgsqlExpectedResult - * @param mixed $mssqlExpectedResult - * @param self::STRINGIFY_* $stringify - * - * @dataProvider provideCases - */ - public function testUnknownDriverUnknownSetupDefault( + public function testUnknownDriver( array $data, string $dqlTemplate, Type $mysqlExpectedType, @@ -706,13 +615,11 @@ public function testUnknownDriverUnknownSetupDefault( $this->determineTypeForUnknownDriverUnknownSetup($mysqlExpectedType, $stringify), $mysqlExpectedResult, $stringify, - self::INVALID_CONNECTION_UNKNOWN_DRIVER + true ); } /** - * Connection failure test - * * @param array $data * @param mixed $mysqlExpectedResult * @param mixed $sqliteExpectedResult @@ -723,7 +630,7 @@ public function testUnknownDriverUnknownSetupDefault( * * @dataProvider provideCases */ - public function testUnknownDriverUnknownSetupStringify( + public function testUnknownDriverStringify( array $data, string $dqlTemplate, Type $mysqlExpectedType, @@ -749,7 +656,7 @@ public function testUnknownDriverUnknownSetupStringify( $this->determineTypeForUnknownDriverUnknownSetup($mysqlExpectedType, $stringify), $mysqlExpectedResult, $stringify, - self::INVALID_CONNECTION_UNKNOWN_DRIVER + true ); } @@ -4267,7 +4174,6 @@ public static function provideCases(): iterable * @param mixed $expectedFirstResult * @param array $data * @param self::STRINGIFY_* $stringification - * @param self::INVALID_*|null $invalidConnectionSetup */ private function performDriverTest( string $driver, @@ -4279,7 +4185,7 @@ private function performDriverTest( ?Type $expectedInferredType, $expectedFirstResult, string $stringification, - ?string $invalidConnectionSetup = null + bool $useUnknownDriverForInference = false ): void { $connectionParams = [ @@ -4299,12 +4205,12 @@ private function performDriverTest( $result = $query->getSingleResult(); $realResultType = ConstantTypeHelper::getTypeFromValue($result); - if ($invalidConnectionSetup !== null) { - $inferredType = $this->getInferredType($this->cloneQueryAndInjectInvalidConnection($query, $driver, $invalidConnectionSetup), false); - } else { - $inferredType = $this->getInferredType($query, true); + if ($useUnknownDriverForInference) { + $query = $this->cloneQueryAndInjectConnectionWithUnknownPdoMysqlDriver($query); } + $inferredType = $this->getInferredType($query); + } catch (Throwable $e) { if ($expectedInferredType === null) { return; @@ -4327,13 +4233,13 @@ private function performDriverTest( )); } - $driverDetector = new DriverDetector(true); - $driverType = $driverDetector->detect($query->getEntityManager()->getConnection()); + $driverDetector = new DriverDetector(); + $driverType = $driverDetector->detect($connection); $stringify = $this->shouldStringify($stringification, $driverType, $phpVersion, $configName); if ( $stringify - && $invalidConnectionSetup === null // do not stringify, we already passed union with stringified one above + && !$useUnknownDriverForInference // do not stringify, we already passed union with stringified one above ) { $expectedInferredType = self::stringifyType($expectedInferredType); } @@ -4420,7 +4326,7 @@ private function getQuery( /** * @param Query $query */ - private function getInferredType(Query $query, bool $failOnInvalidConnection): Type + private function getInferredType(Query $query): Type { $typeBuilder = new QueryResultTypeBuilder(); $phpVersion = new PhpVersion(PHP_VERSION_ID); // @phpstan-ignore-line ctor not in bc promise @@ -4429,7 +4335,7 @@ private function getInferredType(Query $query, bool $failOnInvalidConnection): T $typeBuilder, self::getContainer()->getByType(DescriptorRegistry::class), $phpVersion, - new DriverDetector($failOnInvalidConnection) + new DriverDetector() ); return $typeBuilder->getResultType(); @@ -4883,29 +4789,19 @@ private function shouldStringify(string $stringification, ?string $driverType, i /** * @param Query $query - * @param self::INVALID_* $invalidSetup * @return Query */ - private function cloneQueryAndInjectInvalidConnection(Query $query, string $driver, string $invalidSetup): Query + private function cloneQueryAndInjectConnectionWithUnknownPdoMysqlDriver(Query $query): Query { if ($query->getDQL() === null) { throw new LogicException('Query does not have DQL'); } - $connectionConfig = new DbalConfiguration(); - - if ($invalidSetup === self::INVALID_CONNECTION_UNKNOWN_DRIVER) { - $connectionConfig->setMiddlewares([ - new Middleware($this->createMock(LoggerInterface::class)), // ensures DriverType fallback detection is used - ]); - } + $connection = DriverManager::getConnection([ + 'driverClass' => UnknownDriver::class, + 'serverVersion' => $this->getSampleServerVersionForDriver('pdo_mysql'), + ]); - $serverVersion = $this->getSampleServerVersionForDriver($driver); - $connection = DriverManager::getConnection([ // @phpstan-ignore-line ignore dynamic driver - 'driver' => $driver, - 'user' => 'invalid', - 'serverVersion' => $serverVersion, // otherwise the connection fails while trying to determine the platform - ], $connectionConfig); $entityManager = new EntityManager($connection, $this->createOrmConfig()); $newQuery = new Query($entityManager); $newQuery->setDQL($query->getDQL()); @@ -4934,15 +4830,6 @@ private function createOrmConfig(): Configuration return $config; } - private function determineTypeForKnownDriverUnknownSetup(Type $originalExpectedType, string $stringify): Type - { - if ($stringify === self::STRINGIFY_NONE) { - return $originalExpectedType; - } - - return TypeCombinator::union($originalExpectedType, self::stringifyType($originalExpectedType)); - } - private function determineTypeForUnknownDriverUnknownSetup(Type $originalExpectedType, string $stringify): Type { if ($stringify === self::STRINGIFY_NONE) { diff --git a/tests/Platform/UnknownDriver.php b/tests/Platform/UnknownDriver.php new file mode 100644 index 00000000..52c408e0 --- /dev/null +++ b/tests/Platform/UnknownDriver.php @@ -0,0 +1,26 @@ +