doctrine / DoctrineBundle

Symfony Bundle for Doctrine ORM and DBAL
https://www.doctrine-project.org/projects/doctrine-bundle.html
MIT License
4.72k stars 453 forks source link

[RFC] Introduce official support for dynamic connections #597

Closed marcospassos closed 6 years ago

marcospassos commented 7 years ago

There are a lot of open issues (200+ only on Stackoverflow) about how to determine the database credentials based on runtime information. It's even more relevante for supporting multi-tenant architectures with data isolation. Based on this, I'd like to propose a new approach to address this issue.

This approach envolves introducing a new interface called OptionsProvider, as well as a new provider configuration option under connections:

doctrine:
    dbal:
        default_connection: default
        connections:
            default:
                provider: 'acme.database_configuration_provider'
                driver:   'default_driver'
                host:     'default_host'
                port:     'default_port''
                charset:  'default_charset
interface OptionsProvider
{
    public function getOptions();
}

When a provider is specified, any configuration defined under this connection is used as default, later merged to the provided options.

In addition, a LazyConnection decorator must be introduced. In the same vein as the Doctrine LazyCollection, LazyConnection is a connection decorator that postpones the connection initialization until it cannot do that anymore. It allows us keeping everything as is, including the Doctrine cache warmer.

class LazyConnection implements Connection {
    private $connection;
    private $factory;
    private $optionsProvider;
    private $configuration;
    private $eventManager;
    private $mappingTypes;

    public function __construct(ConnectionFactory $factory, OptionsProvider $optionsProvider, Configuration $configuration,  EventManager $manager, array $mappingTypes)
    {
        $this->factory = $factory;
        $this->optionsProvider = $optionsProvider;
        $this->connection = $configuration;
        $this->eventManager = $manager;
        $this->mappingTypes = $mappingTypes;
    }

    public function setProvider(OptionProvider $optionsProvider)
    {
        if ($this->connection !== null) {
            throw new \BadMethodCallException('Connection is already initialized');
        }

        $this->optionsProvider = $optionsProvider;
    }

    protected function load() {
        if ($this->connection == null) {
            $this->connection = $this->factory->createConnection(
                $this->optionsProvider->getOptions(),
                $this->configuration,
                $this->eventManager,
                $this->mappingTypes
            );
        }

        return $this->connection;
    }

    function prepare($prepareString)
    {
        return $this->load()->prepare($prepareString);
    }

    function query()
    {
        return $this->load()->query();
    }

   // ...
}

Now, multi-tenant applications that need to configure the connection according to the request can achieve such result just implementing an OptionProvider:

class RequestBasedOptionsProvider implements OptionsProvider
{
    private $requestStack;
    private $repository;

    public function __construct(RequestStack $stack, EntityRepository $repository)
    {
        $this->requestStack = $stack;
        $this->repository = $repository;
    }

    public function getOptions()
    {
        $request = $this->requestStack->getMasterRequest();
        $namespace = $request->get('tenant');
        $account = $this->repository->findOneBy(['tenant' => $tenant]);

        if ($account == null) {
            throw new \RuntimeException(sprintf('Unrecognized tenant "%s"', $tenant));
        }

        return $account->getDatabaseConfiguration();
    }
}

Note If the provider isn't ready for providing the options, it should throw an exception. The default values are intended only to provide common options such as default port, host, etc. So, there should be no way for creating a connection using only the default values.

Even for more complex cases, such as specifying the provider prior to run a Doctrine console command, it's simple to be achieved:

class StaticOptionsProvider implements OptionsProvider
{
    private $options;

    public function __construct(array $options)
    {
        $this->options = $options;
    }

    public function getOptions()
    {
        return $this->options;
    }
}
class DoctrineCommandEventListener
{
    const DOCTRINE_COMMAND_NAMESPACE = 'Doctrine\Bundle\DoctrineBundle\Command';

    private $connection;
    private $repository;

    public function __construct(EntityRepository $repository, LazyConnection $connection)
    {
        $this->repository = $repository;
        $this->connection = $connection;
    }

    public function onConsoleCommand(ConsoleCommandEvent $event)
    {
        $command = $event->getCommand();
        $input = $event->getInput();

        if (strpos(get_class($command), self::DOCTRINE_COMMAND_NAMESPACE) !== 0) {
            return;
        }

        $option = new InputOption('tenant', null, InputOption::VALUE_OPTIONAL, 'The tenant ID', null);
        $definition = $command->getDefinition();
        $definition->addOption($option);

        $input->bind($command->getDefinition());
        $id = $input->getOption('tenant');

        if ($id !== null) {
            $account = $this->repository->findOneBy(['tenant' => $id]);

            if ($account == null) {
                throw new InvalidArgumentException(sprintf(
                    'Tenant "%s" does not exist.', $id
                ));
            }

            $configuration = $account->getDatabaseConfiguration();
            $provider = new StaticOptionsProvider($configuration);

            $output = $event->getOutput();
            $output->writeln(sprintf('<info>Current account: %s</info>', $id));

            $this->connection->setProvider($provider);
        }
    }
}

This proposal requires minor changes to the current code base and can bring a lot of possibilities for Symfony developers.

marcospassos commented 7 years ago

@stof @Ocramius guys, any thoughts?

stof commented 7 years ago

The issue with this proposal is that it is very easy to end up calling your lazy connection at a time the dynamic options are not yet available, and so creating it with the default options rather than the dynamic ones (and throwing an exception when setting the updated options with your code above). I'd rather not ship this in the bundle directly.

marcospassos commented 7 years ago

If the provider isn't ready for providing the options, it should throw an exception. The default values are intended only to provide common options such as default port, host, etc. So, there should be no way for creating a connection using only the default values.

stof commented 7 years ago

@marcospassos and then, everything could be broken if some code triggers the usage of the connection before the time your code makes it working. This is a very good reason not to ship it by default IMO.

marcospassos commented 7 years ago

What the difference between doing this and using environment variables, like Symfony just introduced? I could just implement a provider chain that uses a EnvironmentVariableProvider as fallback, right? Besides this, a multi-tenant application is ran assuming some premisses. Context dependency is one of them.

Do you see any of better way for achieving such result?

stof commented 7 years ago

@marcospassos environment variables will be usable at any time during the request processing. So even if the entity manager triggers a call on the connection very early, reading the env variable at this time is OK.

marcospassos commented 7 years ago

So, using a ChainProvider with a fallback to EnvironmentVariableProvider is OK as well, right?

class ChainOptionProvider implements OptionProvider {
    private $providers = [];

    public function __construct(array $providers = [])
    {
        $this->providers = $providers;
    }

    public function getOption() {
        foreach ($this->providers as $provider) {
            try {
                return $provider->getOptions();
            } catch (Exception $exception) {
                continue;
            }
        }

        throw new RuntimeException('Unable to resolve options');
    }
}

$chain = new ChainOptionProvider([new RequestBasedProvider(), new EnvironmentVariableProvider()]);
stof commented 7 years ago

@marcospassos except that if the request does not have the info yet (due to getting the connection too early), the env variables will be used and then the whole request handling will use this connection created with the default settings from the environment.

marcospassos commented 7 years ago

Hmm, I got it. I'll think in how to handle it.

Do have any good idea of how to tie it?

marcospassos commented 7 years ago

For reference, follows how Hibernate handles it: https://docs.jboss.org/hibernate/orm/4.2/devguide/en-US/html/ch16.html https://docs.jboss.org/hibernate/orm/4.2/javadocs/org/hibernate/context/spi/CurrentTenantIdentifierResolver.html http://docs.jboss.org/hibernate/orm/4.2/javadocs/org/hibernate/service/jdbc/connections/spi/MultiTenantConnectionProvider.html

kimhemsoe commented 6 years ago

Labeling as "wont fix" for now. I believe this feature require some thoughts before we go ahead and implementing it.