Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Autoconfigure DBAL types #1867

Open
GromNaN opened this issue Feb 5, 2025 · 6 comments
Open

Autoconfigure DBAL types #1867

GromNaN opened this issue Feb 5, 2025 · 6 comments

Comments

@GromNaN
Copy link
Member

GromNaN commented Feb 5, 2025

Feature Request

What

When a project implements a Doctrine\DBAL\Types\Type, it's almost certain this needs to be declared in the configuration. Using ContainerBuilder::registerForAutoconfiguration(), it should be possible to automatically detect and register this classes.

Why

Remove some obvious configuration.

types:
custom: Acme\HelloBundle\MyCustomType

How

  • In DoctrineExtension, call $this->registerForAutoconfiguration() to add a tag.
  • Add a compiler pass that find service by tag name and append the classes names to the doctrine.dbal.connection_factory.types parameter.

Types are not scoped to the DBAL connection.

@stof
Copy link
Member

stof commented Feb 5, 2025

The reason this was not done is because defining types as services is what does not actually work (DBAL does not support using instances of types that are instantiated externally, and so we cannot make it use the instance instantiated by the container, which could have dependencies and so on).

Adding support for autoconfiguring a service as defining a type would encourage projects to register their types as services, but it would then make developers think that they can use any feature of the DI component for those services.

@GromNaN
Copy link
Member Author

GromNaN commented Feb 5, 2025

So we need a mechanism in Symfony to discover non-service classes, while ensuring that they are excluded from the DIC. And, if possible, throw an exception when we detect that they are trying to use them as service (let's discuss here symfony/symfony#59704).

The Doctrine\DBAL\Types\Type class has a final constructor, which makes dependency injection more difficult, although someone could try to use injection via a method.

@GromNaN
Copy link
Member Author

GromNaN commented Feb 6, 2025

Have you ever considered enabling dependency injection for DBAL types?

Using Type::getTypeRegistry()->register($name, $instance), it's possible to set an instance of Type instead of relying on Type::addType($name, $className) to instantiate the class.
Then ConnectionFactory can get a list of array{class:string}|Type and inject the instances when provided.

    private function initializeTypes(): void
    {
        foreach ($this->typesConfig as $typeName => $typeConfig) {
            if ($typeConfig instanceof Type) {
                if (Type::hasType($typeName)) {
                    Type::getTypeRegistry()->override($typeName, $typeConfig);
                } else {
                    Type::getTypeRegistry()->register($typeName, $typeConfig);
                }
            } else {
                if (Type::hasType($typeName)) {
                    Type::overrideType($typeName, $typeConfig['class']);
                } else {
                    Type::addType($typeName, $typeConfig['class']);
                }
            }
        }

@stof
Copy link
Member

stof commented Feb 6, 2025

Well, as long as Type has a final constructor enforcing that it has no arguments, it makes it hard to use proper DI.

@andrew-demb
Copy link

See this PR [1] to make the Type constructor not final

[1] doctrine/dbal#6705

@GromNaN
Copy link
Member Author

GromNaN commented Feb 6, 2025

Property or setter injections are still possible. That could evolve if we add a TypeInterface or remove the constructor constraint.

Even without DI, which was not the main idea behind this issue, that makes the auto-configuration of Type classes possible; and they would be services, as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants