Conversation
| $types = $container->getParameter('doctrine.dbal.connection_factory.types'); | ||
|
|
||
| foreach ($container->findTaggedServiceIds('doctrine.dbal.type') as $id => $tags) { | ||
| $types[$tags[0]['name']] = ['class' => $container->getDefinition($id)->getClass()]; |
There was a problem hiding this comment.
Should we have to loop over every tags? Even if the attribute is not repeatable, the service can be tagged directly using AutoconfigureTag.
There was a problem hiding this comment.
We should indeed process all tags, and throw a proper exception when the name is not provided in the tag instead of producing a notice.
| $types = $container->getParameter('doctrine.dbal.connection_factory.types'); | ||
|
|
||
| foreach ($container->findTaggedServiceIds('doctrine.dbal.type') as $id => $tags) { | ||
| $types[$tags[0]['name']] = ['class' => $container->getDefinition($id)->getClass()]; |
There was a problem hiding this comment.
Maybe it could be interesting to check if the name property exists.
There was a problem hiding this comment.
👌
I would also check if the class extends Doctrine\DBAL\Types\Type
src/Attribute/AsDoctrineType.php
Outdated
| use Attribute; | ||
|
|
||
| #[Attribute(Attribute::TARGET_CLASS)] | ||
| final readonly class AsDoctrineType |
There was a problem hiding this comment.
| final readonly class AsDoctrineType | |
| final readonly class AsDbalType |
Other doctrine projects might have a concept of type as well.
| $types = $container->getParameter('doctrine.dbal.connection_factory.types'); | ||
|
|
||
| foreach ($container->findTaggedServiceIds('doctrine.dbal.type') as $id => $tags) { | ||
| $types[$tags[0]['name']] = ['class' => $container->getDefinition($id)->getClass()]; |
There was a problem hiding this comment.
We should indeed process all tags, and throw a proper exception when the name is not provided in the tag instead of producing a notice.
| $types[$tags[0]['name']] = ['class' => $container->getDefinition($id)->getClass()]; | ||
| } | ||
|
|
||
| $container->setParameter('doctrine.dbal.connection_factory.types', $types); |
There was a problem hiding this comment.
I'm not sure this is the right approach currently. The proposed API would make people think they can use DI in their types by registering them as services, while the implementation won't use those services.
There is 2 possible ways there:
- implement proper support for using the services for the type instances, thanks to the DBAL 4.x feature allowing that
- register only the class as done here, but build that feature using resource tags (which helps a bit with the intent)
There was a problem hiding this comment.
Indeed, I missed it.
I change the implementation to use resource tags.
Thanks
There was a problem hiding this comment.
@stof @Jean-Beru I changed the first implementation to use resource tags when available (e.g symfony/dependency-injection >= 7.3), and fallback to tags otherwise.
Let me know if this is a good way.
3304bbe to
8f59bd1
Compare
8d6c4f7 to
e26bfe1
Compare
e26bfe1 to
ea214eb
Compare
|
|
||
| /** @phpstan-ignore function.alreadyNarrowedType */ | ||
| $attributes = method_exists($container, 'getAttributeAutoconfigurators') | ||
| ? array_map(static fn (array $arr) => $arr[0], $container->getAttributeAutoconfigurators()) |
There was a problem hiding this comment.
| ? array_map(static fn (array $arr) => $arr[0], $container->getAttributeAutoconfigurators()) | |
| ? array_column($container->getAttributeAutoconfigurators(), 0) |
?
There was a problem hiding this comment.
@Jean-Beru I used array_map to be consistent with the others similar tests.
Plus, the return type of the getAttributeAutoconfigurators() method is an array with the attribute FQCN as a key and the closure as first entry of the sub array :
array:2 [
"Doctrine\Bundle\DoctrineBundle\Attribute\AsDbalType" => array:1 [
0 => Closure(ChildDefinition $definition, AsDbalType $type): void^ {#4186
returnType: "void"
class: "Doctrine\Bundle\DoctrineBundle\DependencyInjection\DoctrineExtension"
}
]
"Doctrine\Bundle\DoctrineBundle\Attribute\AsMiddleware" => array:1 [
0 => Closure(ChildDefinition $definition, AsMiddleware $attribute): void^ {#7886
returnType: "void"
class: "Doctrine\Bundle\DoctrineBundle\DependencyInjection\DoctrineExtension"
}
]
]So calling array_column as is would not give us the expected behavior.
WDYT?
Description
This PR add a new AsDbalType attribute to automatically register the related class as a DBAL type.
Problem Solved
Currently, to register a custom DBAL type, it must be manually declared in the Doctrine configuration file:
This approach is verbose and requires the developer to manage configuration in addition to the type class itself.
Feature Added
The addition of the #[AsDbalType] attribute allows the application to automatically detect and register any class that uses this attribute.
Concrete Usage Example
Here is how you can register a custom type directly via the attribute, without touching the YAML configuration:
1. Creating the Custom Type
2. Usage in an Entity
You can then use this type directly in your entities:
Advantages
doctrine.yamlfor every new custom type.