From 87fa2251b91d697d18807604bef33d11431640bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Paris?= Date: Sat, 27 Nov 2021 14:21:05 +0100 Subject: [PATCH] Drop Flyweight pattern There is a growing need for types with parameters, and that pattern was probably used out of concerns that are no longer relevant anyway. --- UPGRADE.md | 7 +++++ docs/en/reference/types.rst | 52 ++++++++++++++++++++++++++++++++ psalm.xml.dist | 5 +++ src/Exception.php | 7 ----- src/Types/Type.php | 1 - src/Types/TypeRegistry.php | 35 +++++++-------------- tests/Types/TypeRegistryTest.php | 21 ++++--------- 7 files changed, 81 insertions(+), 47 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index e51562bd0ab..0d6527c5055 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -6,6 +6,13 @@ awareness about deprecated code. - Use of our low-overhead runtime deprecation API, details: https://github.com/doctrine/deprecations/ +# Upgrade to 3.3 + +## Deprecated `TypeRegistry::lookupName()`. + +Since `Type` instances are no longer flyweight, and might therefore be +registered more than once, it makes no sense to lookup the name of a given type. + # Upgrade to 3.2 ## Deprecated `SQLLogger` and its implementations. diff --git a/docs/en/reference/types.rst b/docs/en/reference/types.rst index 1cad37e9465..97e5d4309d3 100644 --- a/docs/en/reference/types.rst +++ b/docs/en/reference/types.rst @@ -909,3 +909,55 @@ hook it into the database platform: This would allow using a money type in the ORM for example and have Doctrine automatically convert it back and forth to the database. + +It is also possible to register type instances directly, in case you +need to pass parameters to your instance:: + + cipher, + $this->secretStore->fetch($this->secretName) + ); + } + + public function convertToDatabaseValue($value, AbstractPlatform $platform): string + { + return openssl_encrypt( + $value, + $this->cipher, + $this->secretStore->fetch($this->secretName) + ); + } + + public function getName(): string + { + return 'encrypted'; + } + } + +To do that, you can obtain the ``TypeRegistry`` singleton from ``Type`` +and register your type in it:: + + addType('encrypted', new EncryptedType( + 'aes-256-cbc-hmac-sha256', + '/path/to/secret', + $secretStore + )); diff --git a/psalm.xml.dist b/psalm.xml.dist index 4b194dda9f3..e4e1eafe512 100644 --- a/psalm.xml.dist +++ b/psalm.xml.dist @@ -194,6 +194,11 @@ See https://github.com/doctrine/dbal/pull/4897 --> + + diff --git a/src/Exception.php b/src/Exception.php index b8d7804d5d3..733d85d237c 100644 --- a/src/Exception.php +++ b/src/Exception.php @@ -137,11 +137,4 @@ public static function typeNotRegistered(Type $type): self sprintf('Type of the class %s@%s is not registered.', get_class($type), spl_object_hash($type)) ); } - - public static function typeAlreadyRegistered(Type $type): self - { - return new self( - sprintf('Type of the class %s@%s is already registered.', get_class($type), spl_object_hash($type)) - ); - } } diff --git a/src/Types/Type.php b/src/Types/Type.php index 03e5b824851..273d3c00a9a 100644 --- a/src/Types/Type.php +++ b/src/Types/Type.php @@ -130,7 +130,6 @@ private static function createTypeRegistry(): TypeRegistry /** * Factory method to create type instances. - * Type instances are implemented as flyweights. * * @param string $name The name of the type (as returned by getName()). * diff --git a/src/Types/TypeRegistry.php b/src/Types/TypeRegistry.php index ce33b951ae8..e2d1844d488 100644 --- a/src/Types/TypeRegistry.php +++ b/src/Types/TypeRegistry.php @@ -5,17 +5,16 @@ namespace Doctrine\DBAL\Types; use Doctrine\DBAL\Exception; +use Doctrine\Deprecations\Deprecation; use function array_search; -use function in_array; /** * The type registry is responsible for holding a map of all known DBAL types. - * The types are stored using the flyweight pattern so that one type only exists as exactly one instance. */ final class TypeRegistry { - /** @var array Map of type names and their corresponding flyweight objects. */ + /** @var array Map of type names and their corresponding objects. */ private $instances; /** @@ -43,13 +42,20 @@ public function get(string $name): Type /** * Finds a name for the given type. * + * @deprecated this method will be removed in 4.0.0 + * * @throws Exception */ public function lookupName(Type $type): string { - $name = $this->findTypeName($type); + Deprecation::trigger( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/pulls/5036', + 'This method will be removed in 4.0.0, since the same type can be registered twice.', + ); + $name = array_search($type, $this->instances, true); - if ($name === null) { + if ($name === false) { throw Exception::typeNotRegistered($type); } @@ -75,10 +81,6 @@ public function register(string $name, Type $type): void throw Exception::typeExists($name); } - if ($this->findTypeName($type) !== null) { - throw Exception::typeAlreadyRegistered($type); - } - $this->instances[$name] = $type; } @@ -93,10 +95,6 @@ public function override(string $name, Type $type): void throw Exception::typeNotFound($name); } - if (! in_array($this->findTypeName($type), [$name, null], true)) { - throw Exception::typeAlreadyRegistered($type); - } - $this->instances[$name] = $type; } @@ -111,15 +109,4 @@ public function getMap(): array { return $this->instances; } - - private function findTypeName(Type $type): ?string - { - $name = array_search($type, $this->instances, true); - - if ($name === false) { - return null; - } - - return $name; - } } diff --git a/tests/Types/TypeRegistryTest.php b/tests/Types/TypeRegistryTest.php index fa6f26e4da7..bd5e067d7f4 100644 --- a/tests/Types/TypeRegistryTest.php +++ b/tests/Types/TypeRegistryTest.php @@ -86,7 +86,7 @@ public function testRegister(): void self::assertSame($newType, $this->registry->get('some')); } - public function testRegisterWithAlradyRegisteredName(): void + public function testRegisterWithAlreadyRegisteredName(): void { $this->registry->register('some', new TextType()); @@ -99,9 +99,11 @@ public function testRegisterWithAlreadyRegisteredInstance(): void $newType = new TextType(); $this->registry->register('some', $newType); - - $this->expectException(Exception::class); - $this->registry->register('other', $newType); + $this->registry->register('sameButDifferent', $newType); + self::assertSame( + $this->registry->get('some'), + $this->registry->get('sameButDifferent') + ); } public function testOverride(): void @@ -125,17 +127,6 @@ public function testOverrideAllowsExistingInstance(): void self::assertSame($type, $this->registry->get('some')); } - public function testOverrideWithAlreadyRegisteredInstance(): void - { - $newType = new TextType(); - - $this->registry->register('first', $newType); - $this->registry->register('second', new StringType()); - - $this->expectException(Exception::class); - $this->registry->override('second', $newType); - } - public function testOverrideWithUnknownType(): void { $this->expectException(Exception::class);