Extract type factory and registry from Type into TypeRegistry #3354

Majkl578 · 2018-11-24T02:08:58Z

Q	A
Type	improvement
BC Break	no
Related issues	#2841

Summary

This PR splits Type factory, registry and abstract ancestor for all implementors.
Factory and registry stuff is moved to new TypeRegistry which holds no default types on its own.

TODO

Make addType() and overrideType() accept instances or factories instead of class name strings? This would remove internal API dependency on Type's constructor and allow users use it.

Further scope

Add support type aliases on registry level (internally one level of indirection, not leaked outside registry).
Make TypeRegistry per-connection, so different platforms may declare different types.
Eliminate static access to Type::*() registry methods.
Deprecate and remove Type::getName() and leverage ~~TypeRegistry::getName()~~TypeRegistry::lookupName() instead.
Un@internalize TypeRegistry.

cc @greg0ire

lib/Doctrine/DBAL/Types/Type.php

lib/Doctrine/DBAL/Types/TypeRegistry.php

morozov · 2018-11-24T07:32:12Z

Make addType() and overrideType() accept instances or factories instead of class name strings? This would remove internal API dependency on Type's constructor and allow users use it.

Would it make sense to accept type instances instead? Otherwise, the factory would still depend on the constructor signatures which would have to be the same. Given that objects are lightweight, there shouldn't be much overhead in just registering them all at once.

lib/Doctrine/DBAL/Types/Type.php

Majkl578 · 2018-11-24T19:43:16Z

Pushed new version where TypeRegistry accepts instances directly.

lib/Doctrine/DBAL/Types/TypeRegistry.php

lib/Doctrine/DBAL/Types/Type.php

Taluu

👍

lib/Doctrine/DBAL/Types/TypeRegistry.php

lib/Doctrine/DBAL/Types/Type.php

tests/Doctrine/Tests/DBAL/Types/TypeTest.php

Majkl578 · 2019-04-13T12:32:44Z

Rebased onto current master and addressed remaining open discussion.

greg0ire · 2019-04-13T14:02:03Z

lib/Doctrine/DBAL/DBALException.php

+        return new self(
+            sprintf('Type of the class %s@%s is already registered.', get_class($type), spl_object_hash($type))
+        );
+    }


On one hand, not a big fan of making a big exception class that grows with time, on the other hand, if these exceptions are not meant to be caught but to help the user figure out their issue, maybe this is fine?

It's only for 2.x.

lib/Doctrine/DBAL/Types/Type.php

lib/Doctrine/DBAL/Types/TypeRegistry.php

@loilo

# Release v2.10.0 This is a minor release of Doctrine DBAL that aggregates over 70 fixes and improvements developed by 25 contributors over the last year. This release focuses on internal code quality improvement and deprecating the functionality identified for removal in the next major release. ## Backwards Compatibility Breaks This release introduces a minor BC break. Default column values are no longer handled as SQL expressions. They are converted to SQL literals (e.g. escaped). The previous behavior was not portable and was never by design. Clients must now specify default values in their initial form, not in the form of an SQL literal (e.g. escaped). **Before:** ```php $column->setDefault('Foo\\\\Bar\\\\Baz'); ``` **After:** ```php $column->setDefault('Foo\\Bar\\Baz'); ``` ## Deprecations - The usage of the `getDriver()`, `getDatabasePlatform()` and `getSchemaManager()` methods of the `ConnectionEventArgs` class has been deprecated. - The usage of the `getDatabasePlatform()` method of the `SchemaColumnDefinitionEventArgs` class has been deprecated. - The usage of the `getHost()`, `getPort()`, `getUsername()` and `getPassword()` methods of the `Connection` class has been deprecated. - Passing multiple SQL statements as an array to `SchemaAlterTableAddColumnEventArgs::addSql()` and the same method in other `SchemaEventArgs`-based classes is deprecated. - Calling `AbstractSchemaManager::tablesExist()` with a string argument is deprecated. - Calling `OracleSchemaManager::createDatabase()` without an argument or by passing `NULL` is deprecated. - Unused schema manager methods are deprecated. - `AbstractSchemaManager::_getPortableFunctionsList()`, - `AbstractSchemaManager::_getPortableFunctionDefinition()`, - `OracleSchemaManager::_getPortableFunctionDefinition()`, - `SqliteSchemaManager::_getPortableTableIndexDefinition()`. - The usage of `NULL` to indicate empty `$username` or `$password` when calling `Doctrine\DBAL\Driver::connect()` is deprecated. - Method `Doctrine\DBAL\Platforms::_getAlterTableIndexForeignKeySQL()` has been deprecated as no longer used. - Property `Doctrine\DBAL\Driver\OCI8\OCI8Statement::$_PARAM` has been deprecated as not used. - Method `Doctrine\DBAL\Driver::getName()` is deprecated. - The usage of user-provided `PDO` instance is deprecated. - `Type::*` constants are deprecated. - The `Doctrine\DBAL\Driver\SQLSrv\SQLSrvStatement::LAST_INSERT_ID_SQL` constant has been deprecated. - The constants in `Doctrine\DBAL\SQLParserUtils` have been deprecated. - The `Doctrine\DBAL\Logging\LoggerChain::addLogger` method has been deprecated. Please see the details in the [UPGRADE.md](UPGRADE.md) documentation. ## New Features and Improvements - [3674: Add missing MySQL 8.0 reserved keywords](doctrine#3674) thanks to @loilo - [3512: Support for comments on table in all databases](doctrine#3512) thanks to @moufmouf - [3418: Add column charset for MySql](doctrine#3418) thanks to @altertable **MySQL-related changes:** - [3668: Quote collation on MySQL](doctrine#3668) thanks to @staudenmeir - [3374: Clean up `MySqlPlatform::getListTableIndexesSQL()` fields](doctrine#3374) thanks to @BenMorel - [3311: Ensuring correct `ALTER TABLE` statement for creation of an `AUTO INCREMENT` column as new `PRIMARY KEY`](doctrine#3311) thanks to @arnegroskurth **Driver level changes:** - [3677: Relax statement type declaration](doctrine#3677) thanks to @greg0ire - [3521: Maintain platform parameter in connection params](doctrine#3521) thanks to @jwage and @Perf - [3588: Add commit result bool](doctrine#3588) thanks to @otazniksk **Schema management:** - [2960: Handle default values as values, not SQL expressions](doctrine#2960) thanks to @morozov **Types improvements:** - [3356: Extract constants for built-in types from Type to Types](doctrine#3356) thanks to @Majkl578 - [3354: Extract type factory and registry from Type into TypeRegistry](doctrine#3354) thanks to @Majkl578 **Compatibility with Symfony 5:** - [3706: add missing exit codes to ensure Symfony 5 compatibility](doctrine#3706) thanks to @dmaicher - [3563: Allow Symfony 5](doctrine#3563) thanks to @nicolas-grekas **Query Builder:** - [3696: Add support for `DISTINCT` clause](doctrine#3696) thanks to @bingo-soft **Logging:** - [3572: Make LoggerChain use constructor to add loggers instead of adder method](doctrine#3572) thanks to @ostrolucky **Code quality improvements:** - [3667: Phpstan fix backport](doctrine#3667) thanks to @morozov - [3663: Updated PHPStan to v0.11.15](doctrine#3663) thanks to @morozov - [3604: Updated Jetbrains PhpStorm stubs to 2019.1](doctrine#3604) thanks to @morozov - [3549: Removed the assertion which doesn't work with a user-provided PDO connection](doctrine#3549) thanks to @morozov - [3489: Update doctrine coding standard from 5.0 to 6.0](doctrine#3489) thanks to @amaabdou - [3481: Updated PHPStan to v0.11.3](doctrine#3481) thanks to @morozov - [3443: PHPStan Level 7](doctrine#3443) thanks to @morozov - [3442: PHPStan Level 6](doctrine#3442) thanks to @morozov - [3436: PHPStan Level 5](doctrine#3436) thanks to @morozov - [3435: PHPStan Level 4](doctrine#3435) thanks to @morozov - [3432: Updated PHPStan to v0.11](doctrine#3432) thanks to @morozov **Test suite improvements:** - [3705: Don't skip a test for sqlite](doctrine#3705) thanks to @Federkun - [3689: Updated PHPUnit to 8.4.1](doctrine#3689) thanks to @morozov - [3664: Refactor usage of MockBuilder's deprecated setMethods()](doctrine#3664) thanks to @baspeeters - [3643: Bumped PHPUnit requrement to ^8.3.3, removed dependency on symfony/phpunit-bridge](doctrine#3643) thanks to @morozov - [3609: Reworked the mocks generated by Prophecy using PHPUnit](doctrine#3609) thanks to @morozov - [3608: Added a unit test for Doctrine\DBAL\Logging\LoggerChain](doctrine#3608) thanks to @morozov - [3600: Updated PHPUnit to 8.2.1](doctrine#3600) thanks to @morozov - [3575: Enforced parameter and return value types in test classes](doctrine#3575) thanks to @morozov - [3566: Upgraded to PHPUnit 8.1.6 and reworked the remaining driver exception mock](doctrine#3566) thanks to @morozov - [3555: Removed the rest of mock classes](doctrine#3555) thanks to @morozov - [3546: Reworked driver exception tests](doctrine#3546) thanks to @morozov - [3530: Improve ExceptionTest::testConnectionExceptionSqLite](doctrine#3530) thanks to @jwage - [3474: Remove more hard-coded mock classes](doctrine#3474) thanks to @morozov - [3470: Replaced MockPlatform with the ones generated by PHPUnit](doctrine#3470) thanks to @morozov - [3468: Marked some test classes abstract](doctrine#3468) thanks to @morozov - [3446: Upgraded PHPUnit to 8.0](doctrine#3446) thanks to @morozov **Documentation improvements:** - [3616: Fix typo in docblock](doctrine#3616) thanks to @rdarcy1 - [3559: add .github/FUNDING.yml](doctrine#3559) thanks to @jwage - [3556: Removed 2.8 from README](doctrine#3556) thanks to @morozov - [3514: Expand list of keywords in composer.json](doctrine#3514) thanks to @Majkl578 - [3504: fix doctrine#3479 (typos in example-code in QueryBuilder)](doctrine#3504) thanks to @DavidBruchmann - [3503: Fix the branch alias for master](doctrine#3503) thanks to @stof - [3463: fixed a typo in PoolingShardConnection phpdoc](doctrine#3463) thanks to @adapik - [3408: Removed unused build files](doctrine#3408) thanks to @morozov - [3404: Link to Slack instead of Gitter](doctrine#3404) thanks to @greg0ire - [3376: Bump version to 2.10.0-DEV](doctrine#3376) thanks to @morozov **CI improvements:** - [3688: Temporarily disable the usage of PHPUnit 8.4 due to a regression](doctrine#3688) thanks to @morozov - [3654: fix Appveyor builds](doctrine#3654) thanks to @mmucklo - [3644: Using command line options to configure MySQL 8 instead of mounting a config file](doctrine#3644) thanks to @morozov - [3509: Enabled the build against IBM DB2 on PHP 7.3 on Travis CI](doctrine#3509) thanks to @morozov - [3528: Reworked SQL Server installer on Travis CI](doctrine#3528) thanks to @morozov - [3484: Use latest stable versions of sqlsrv and pdo&doctrine#95;sqlsrv on CI](doctrine#3484) thanks to @morozov - [3475: Make PHP 7.3 the primary PHP version on Travis CI](doctrine#3475) thanks to @morozov - [3473: Avoid database connection from PHPUnit data providers](doctrine#3473) thanks to @morozov - [3469: Replaced Xdebug with PCOV for code coverage](doctrine#3469) thanks to @morozov - [3405: Travis CI: PHP 7.3 tests on MySQL 8.0](doctrine#3405) thanks to @BenMorel - [3394: Grouped PHPStan and PHP&doctrine#95;CodeSniffer for parallel execution](doctrine#3394) thanks to @morozov - [3388: Require PHP 7.2, drop <7.2 in Composer & on CI](doctrine#3388) thanks to @morozov - [3122: Introduced a smoke testing phase on Travis to run SQLite tests and CS checks first](doctrine#3122) thanks to @morozov **Deprecations:** - [3708: New deprecations for 2.10](doctrine#3708) thanks to @morozov and @jwage - [3598: Deprecated some unused code bits](doctrine#3598) thanks to @morozov - [3558: Deprecated Driver::getName()](doctrine#3558) thanks to @morozov - [3554: The usage of user-provided PDO instance is deprecated](doctrine#3554) thanks to @morozov - [3542: Deprecate SQLSrvStatement::LAST&doctrine#95;INSERT&doctrine#95;ID&doctrine#95;SQL constant.](doctrine#3542) thanks to @jwage - [3541: Deprecate the public constants in SQLParserUtils](doctrine#3541) thanks to @jwage # gpg: Signature made Sun Nov 3 17:56:11 2019 # gpg: using RSA key 374EADAF543AE995 # gpg: Can't check signature: public key not found # Conflicts: # .travis.yml # README.md # lib/Doctrine/DBAL/Platforms/SQLAnywherePlatform.php # lib/Doctrine/DBAL/Schema/MySqlSchemaManager.php # lib/Doctrine/DBAL/Schema/Table.php # lib/Doctrine/DBAL/Version.php # phpcs.xml.dist # tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php # tests/Doctrine/Tests/DBAL/Functional/ExceptionTest.php # tests/Doctrine/Tests/DBAL/Functional/MasterSlaveConnectionTest.php # tests/Doctrine/Tests/DBAL/Functional/Platform/DefaultExpressionTest.php # tests/Doctrine/Tests/DBAL/Functional/ResultCacheTest.php # tests/Doctrine/Tests/DBAL/Platforms/SQLAnywherePlatformTest.php # tests/Doctrine/Tests/DBAL/Schema/ComparatorTest.php # tests/travis/mysql.docker.travis.xml # tests/travis/mysqli.docker.travis.xml

Majkl578 added WIP Improvement Types labels Nov 24, 2018

Majkl578 requested review from morozov, Ocramius and lcobucci November 24, 2018 02:08

morozov reviewed Nov 24, 2018

View reviewed changes

greg0ire reviewed Nov 24, 2018

View reviewed changes

lib/Doctrine/DBAL/Types/Type.php Show resolved Hide resolved

Majkl578 force-pushed the type-registry branch 5 times, most recently from c25a287 to f90912c Compare November 24, 2018 19:40

Majkl578 force-pushed the type-registry branch 2 times, most recently from 6ba1ab2 to ab9a6e8 Compare November 24, 2018 21:09

morozov reviewed Nov 24, 2018

View reviewed changes

lib/Doctrine/DBAL/Types/TypeRegistry.php Outdated Show resolved Hide resolved

Majkl578 force-pushed the type-registry branch 2 times, most recently from a0b2fd8 to f9a1dfb Compare November 24, 2018 21:59

tarlepp reviewed Nov 24, 2018

View reviewed changes

lib/Doctrine/DBAL/Types/Type.php Outdated Show resolved Hide resolved

Taluu previously approved these changes Nov 24, 2018

View reviewed changes

lib/Doctrine/DBAL/Types/TypeRegistry.php Show resolved Hide resolved

morozov reviewed Nov 25, 2018

View reviewed changes

lib/Doctrine/DBAL/Types/TypeRegistry.php Outdated Show resolved Hide resolved

morozov reviewed Nov 25, 2018

View reviewed changes

lib/Doctrine/DBAL/Types/TypeRegistry.php Outdated Show resolved Hide resolved

greg0ire reviewed Nov 25, 2018

View reviewed changes

lib/Doctrine/DBAL/Types/TypeRegistry.php Outdated Show resolved Hide resolved

tarlepp reviewed Nov 25, 2018

View reviewed changes

lib/Doctrine/DBAL/Types/Type.php Show resolved Hide resolved

enumag reviewed Nov 26, 2018

View reviewed changes

tests/Doctrine/Tests/DBAL/Types/TypeTest.php Show resolved Hide resolved

Majkl578 dismissed Taluu’s stale review via 0931f19 April 13, 2019 12:32

Majkl578 force-pushed the type-registry branch from f9a1dfb to 0931f19 Compare April 13, 2019 12:32

Majkl578 requested review from morozov and greg0ire April 13, 2019 12:32

Majkl578 removed the WIP label Apr 13, 2019

Majkl578 force-pushed the type-registry branch 3 times, most recently from 3390882 to 2db20f2 Compare April 13, 2019 13:44

greg0ire reviewed Apr 13, 2019

View reviewed changes

greg0ire requested changes Apr 13, 2019

View reviewed changes

lib/Doctrine/DBAL/Types/Type.php Show resolved Hide resolved

lib/Doctrine/DBAL/Types/TypeRegistry.php Outdated Show resolved Hide resolved

lib/Doctrine/DBAL/Types/TypeRegistry.php Outdated Show resolved Hide resolved

lib/Doctrine/DBAL/Types/TypeRegistry.php Outdated Show resolved Hide resolved

Majkl578 force-pushed the type-registry branch from 2db20f2 to 1c43037 Compare April 13, 2019 16:13

Extract type factory and registry from Type into TypeRegistry

089f32e

Majkl578 force-pushed the type-registry branch from 1c43037 to 089f32e Compare April 13, 2019 16:13

Majkl578 requested a review from greg0ire April 13, 2019 16:13

greg0ire approved these changes Apr 13, 2019

View reviewed changes

morozov approved these changes Apr 13, 2019

View reviewed changes

Ocramius approved these changes Apr 13, 2019

View reviewed changes

Ocramius added this to the 2.10.0 milestone Apr 13, 2019

Ocramius assigned morozov Apr 13, 2019

morozov merged commit 11b67bb into doctrine:master Apr 13, 2019

Majkl578 deleted the type-registry branch April 13, 2019 22:24

morozov mentioned this pull request Jan 30, 2021

Remove internal flag from TypeRegistry #4491

Merged

greg0ire mentioned this pull request Nov 20, 2021

Refactor type system #2841

Open

9 tasks

morozov mentioned this pull request Jul 22, 2022

Deprecate static type methods #5530

Closed

github-actions bot locked as resolved and limited conversation to collaborators Aug 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extract type factory and registry from Type into TypeRegistry #3354

Extract type factory and registry from Type into TypeRegistry #3354

Majkl578 commented Nov 24, 2018 •

edited

Loading

morozov commented Nov 24, 2018

Majkl578 commented Nov 24, 2018

Taluu left a comment

Majkl578 commented Apr 13, 2019

greg0ire Apr 13, 2019

Majkl578 Apr 13, 2019

Extract type factory and registry from Type into TypeRegistry #3354

Extract type factory and registry from Type into TypeRegistry #3354

Conversation

Majkl578 commented Nov 24, 2018 • edited Loading

Summary

TODO

Further scope

morozov commented Nov 24, 2018

Majkl578 commented Nov 24, 2018

Taluu left a comment

Choose a reason for hiding this comment

Majkl578 commented Apr 13, 2019

greg0ire Apr 13, 2019

Choose a reason for hiding this comment

Majkl578 Apr 13, 2019

Choose a reason for hiding this comment

Majkl578 commented Nov 24, 2018 •

edited

Loading