Skip to content

Commit

Permalink
Drop Flyweight pattern
Browse files Browse the repository at this point in the history
There is a growing need for types with parameters, and that pattern was
probably used out of concerns that are no longer relevant anyway.
  • Loading branch information
greg0ire committed Nov 27, 2021
1 parent bb13b5f commit dadfde0
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 47 deletions.
7 changes: 7 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
54 changes: 54 additions & 0 deletions docs/en/reference/types.rst
Original file line number Diff line number Diff line change
Expand Up @@ -909,3 +909,57 @@ 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::

<?php
namespace My\Project\Types;

use Doctrine\DBAL\Types\Type;
use Doctrine\DBAL\Platforms\AbstractPlatform;

final class EncryptedType extends StringType
{
const MONEY = 'money'; // modify to match your type name

public function __construct(
private string $cipher,
private string $secretName,
private SecretStore $secretStore
) {
}

public function convertToPHPValue($value, AbstractPlatform $platform): string
{
return openssl_decrypt(
$value,
$this->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::

<?php
Type::getTypeRegistry()->addType('encrypted', new EncryptedType(
'aes-256-cbc-hmac-sha256',
'/path/to/secret',
$secretStore
));
7 changes: 0 additions & 7 deletions src/Exception.php
Original file line number Diff line number Diff line change
Expand Up @@ -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))
);
}
}
1 change: 0 additions & 1 deletion src/Types/Type.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()).
*
Expand Down
35 changes: 11 additions & 24 deletions src/Types/TypeRegistry.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,15 @@

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<string, Type> Map of type names and their corresponding flyweight objects. */
/** @var array<string, Type> Map of type names and their corresponding objects. */
private $instances;

/**
Expand Down Expand Up @@ -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);
}

Expand All @@ -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;
}

Expand All @@ -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;
}

Expand All @@ -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;
}
}
21 changes: 6 additions & 15 deletions tests/Types/TypeRegistryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand All @@ -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
Expand All @@ -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);
Expand Down

0 comments on commit dadfde0

Please sign in to comment.