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

use configurable keys for keeping static connections #296

Merged
merged 2 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,51 @@ dama_doctrine_test:
connection_a: true
```

#### Controlling how connections are kept statically in the current php process

By default, every configured doctrine DBAL connection will have its own driver connection that is managed in the current php process.
In case you need to customize this behavior you can choose different "connection keys" that are used to select driver connections.

Example for 2 connections that will re-use the same driver connection instance:

```yaml
doctrine:
dbal:
connections:
default:
url: '%database.url1%'
foo:
url: '%database.url2%'
dama_doctrine_test:
connection_keys:
# assigning the same key will result in the same internal driver connection being re-used for both DBAL connections
default: custom_key
foo: custom_key
```

**Since v8.1.0**: For connections with read/write replicas the bundle will use the **same** underlying driver connection by default for the primary and also for replicas. This addresses an [issue](https://github.com/dmaicher/doctrine-test-bundle/issues/289) where inconsistencies happened when reading/writing to different connections. This can also be customized as follows:

```yaml
doctrine:
dbal:
connections:
default:
url: '%database.url%'
replicas:
replica_one:
url: '%database.url_replica%'
dama_doctrine_test:
connection_keys:
# assigning different keys will result in separate internal driver connection being used for primary and replica
default:
primary: custom_key_primary
replicas:
replica_one: custom_key_primary_replica
```

### Example

An example usage can be seen within the functional tests included in this bundle: https://github.com/dmaicher/doctrine-test-bundle/tree/master/tests
Expand Down
1 change: 0 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
],
"require": {
"php": "^7.4 || ^8.0",
"ext-json": "*",
"doctrine/dbal": "^3.3 || ^4.0",
"doctrine/doctrine-bundle": "^2.2.2",
"psr/cache": "^1.0 || ^2.0 || ^3.0",
Expand Down
3 changes: 2 additions & 1 deletion makefile
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ test_phpunit_legacy: tests/Functional/app/parameters.yml
vendor/bin/phpunit -c tests/phpunit.9.xml tests/

phpstan:
vendor/bin/phpstan analyse -c phpstan.neon -a vendor/autoload.php -l 5 src tests
vendor/bin/phpstan analyse -c phpstan.neon -a vendor/autoload.php -l 8 src
vendor/bin/phpstan analyse -c phpstan.neon -a vendor/autoload.php -l 5 tests

behat:
vendor/bin/behat -c tests/behat.yml -fprogress
Expand Down
49 changes: 49 additions & 0 deletions src/DAMA/DoctrineTestBundle/DependencyInjection/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
class Configuration implements ConfigurationInterface
{
public const ENABLE_STATIC_CONNECTION = 'enable_static_connection';
public const CONNECTION_KEYS = 'connection_keys';
public const STATIC_META_CACHE = 'enable_static_meta_data_cache';
public const STATIC_QUERY_CACHE = 'enable_static_query_cache';

Expand Down Expand Up @@ -45,9 +46,57 @@ public function getConfigTreeBuilder(): TreeBuilder
->end()
->booleanNode(self::STATIC_META_CACHE)->defaultTrue()->end()
->booleanNode(self::STATIC_QUERY_CACHE)->defaultTrue()->end()
->arrayNode(self::CONNECTION_KEYS)
->normalizeKeys(false)
->variablePrototype()
->end()
->validate()
->ifTrue(function ($value) {
if ($value === null) {
return false;
}

if (!is_array($value)) {
return true;
}

foreach ($value as $k => $v) {
if (!is_string($k) || !(is_string($v) || is_array($v))) {
return true;
}

if (!is_array($v)) {
continue;
}

if (count($v) !== 2
|| !is_string($v['primary'] ?? null)
|| !is_array($v['replicas'] ?? null)
|| !$this->isAssocStringArray($v['replicas'])
) {
return true;
}
}

return false;
})
->thenInvalid('Must be array<string, string|array{primary: string, replicas: array<string, string>}>')
->end()
->end()
->end()
;

return $treeBuilder;
}

private function isAssocStringArray(array $value): bool
{
foreach ($value as $k => $v) {
if (!is_string($k) || !is_string($v)) {
return false;
}
}

return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,9 @@ public function load(array $configs, ContainerBuilder $container): void
'dama.'.Configuration::ENABLE_STATIC_CONNECTION,
$config[Configuration::ENABLE_STATIC_CONNECTION]
);
$container->setParameter(
'dama.'.Configuration::CONNECTION_KEYS,
$config[Configuration::CONNECTION_KEYS] ?? [],
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ class DoctrineTestCompilerPass implements CompilerPassInterface
{
public function process(ContainerBuilder $container): void
{
$transactionalBehaviorEnabledConnections = $this->getTransactionEnabledConnectionNames($container);
$container->register('dama.doctrine.dbal.middleware', Middleware::class);
$cacheNames = [];

Expand All @@ -29,11 +28,15 @@ public function process(ContainerBuilder $container): void
$cacheNames[] = 'doctrine.orm.%s_query_cache';
}

$connectionNames = array_keys($container->getParameter('doctrine.connections'));
/** @var array<string, mixed> $connections */
$connections = $container->getParameter('doctrine.connections');
$connectionNames = array_keys($connections);
$transactionalBehaviorEnabledConnections = $this->getTransactionEnabledConnectionNames($container, $connectionNames);
$connectionKeys = $this->getConnectionKeys($container, $connectionNames);

foreach ($connectionNames as $name) {
if (in_array($name, $transactionalBehaviorEnabledConnections, true)) {
$this->modifyConnectionService($container, $name);
$this->modifyConnectionService($container, $connectionKeys[$name] ?? null, $name);
}

foreach ($cacheNames as $cacheName) {
Expand All @@ -56,19 +59,25 @@ public function process(ContainerBuilder $container): void
$container->getParameterBag()->remove('dama.'.Configuration::ENABLE_STATIC_CONNECTION);
$container->getParameterBag()->remove('dama.'.Configuration::STATIC_META_CACHE);
$container->getParameterBag()->remove('dama.'.Configuration::STATIC_QUERY_CACHE);
$container->getParameterBag()->remove('dama.'.Configuration::CONNECTION_KEYS);
}

private function modifyConnectionService(ContainerBuilder $container, string $name): void
/**
* @param string|array{primary: string, replicas: array<string, string>}|null $connectionKey
*/
private function modifyConnectionService(ContainerBuilder $container, $connectionKey, string $name): void
{
$connectionDefinition = $container->getDefinition(sprintf('doctrine.dbal.%s_connection', $name));

if (!$this->hasSavepointsEnabled($connectionDefinition)) {
throw new \LogicException(sprintf('This bundle relies on savepoints for nested database transactions. You need to enable "use_savepoints" on the Doctrine DBAL config for connection "%s".', $name));
}

/** @var array<string, mixed> $connectionOptions */
$connectionOptions = $connectionDefinition->getArgument(0);
$connectionDefinition->replaceArgument(
0,
$this->getModifiedConnectionOptions($connectionDefinition->getArgument(0), $name),
$this->getModifiedConnectionOptions($connectionOptions, $connectionKey, $name),
);

$connectionConfig = $container->getDefinition(sprintf('doctrine.dbal.%s_connection.configuration', $name));
Expand All @@ -90,27 +99,44 @@ private function modifyConnectionService(ContainerBuilder $container, string $na
$connectionConfig->setMethodCalls($methodCalls);
}

private function getModifiedConnectionOptions(array $options, string $name): array
{
$connectionOptions = array_merge($options, [
'dama.keep_static' => true,
'dama.connection_name' => $name,
]);

if (isset($connectionOptions['primary'])) {
$connectionOptions['primary'] = array_merge($connectionOptions['primary'], [
'dama.keep_static' => true,
'dama.connection_name' => $name,
]);
}

if (isset($connectionOptions['replica']) && is_array($connectionOptions['replica'])) {
foreach ($connectionOptions['replica'] as $replicaName => &$replica) {
$replica = array_merge($replica, [
'dama.keep_static' => true,
'dama.connection_name' => sprintf('%s.%s', $name, $replicaName),
]);
/**
* @param array<string, mixed> $connectionOptions
* @param string|array{primary: string, replicas: array<string, string>}|null $connectionKey
*
* @return array<string, mixed>
*/
private function getModifiedConnectionOptions(
array $connectionOptions,
$connectionKey,
string $name
): array {
if (!isset($connectionOptions['primary'])) {
if (is_array($connectionKey)) {
throw new \InvalidArgumentException(sprintf('Connection key for connection "%s" must be a string', $name));
}

$connectionOptions['dama.connection_key'] = $connectionKey ?? $name;

return $connectionOptions;
}

$connectionOptions['dama.connection_key'] = $connectionKey['primary'] ?? $connectionKey ?? $name;
$connectionOptions['primary']['dama.connection_key'] = $connectionOptions['dama.connection_key'];

if (!is_array($connectionOptions['replica'] ?? null)) {
return $connectionOptions;
}

$replicaKeys = [];
if (isset($connectionKey['replicas'])) {
/** @var array<string> $definedReplicaNames */
$definedReplicaNames = array_keys($connectionOptions['replica']);
$this->validateConnectionNames(array_keys($connectionKey['replicas']), $definedReplicaNames);
$replicaKeys = $connectionKey['replicas'];
}

foreach ($connectionOptions['replica'] as $replicaName => &$replicaOptions) {
$replicaOptions['dama.connection_key'] = $replicaKeys[$replicaName] ?? $connectionOptions['dama.connection_key'];
}

return $connectionOptions;
Expand All @@ -123,11 +149,12 @@ private function registerStaticCache(
): void {
$cache = new Definition();
$namespace = sha1($cacheServiceId);
$originalServiceClass = $originalCacheServiceDefinition->getClass();

if (is_a($originalCacheServiceDefinition->getClass(), CacheItemPoolInterface::class, true)) {
if ($originalServiceClass !== null && is_a($originalServiceClass, CacheItemPoolInterface::class, true)) {
$cache->setClass(Psr6StaticArrayCache::class);
$cache->setArgument(0, $namespace); // make sure we have no key collisions
} elseif (is_a($originalCacheServiceDefinition->getClass(), Cache::class, true)) {
} elseif ($originalServiceClass !== null && is_a($originalServiceClass, Cache::class, true)) {
throw new \InvalidArgumentException(sprintf('Configuring "%s" caches is not supported anymore. Upgrade to PSR-6 caches instead.', Cache::class));
} else {
throw new \InvalidArgumentException(sprintf('Unsupported cache class "%s" found on service "%s".', $originalCacheServiceDefinition->getClass(), $cacheServiceId));
Expand All @@ -140,14 +167,15 @@ private function registerStaticCache(
}

/**
* @param string[] $connectionNames
*
* @return string[]
*/
private function getTransactionEnabledConnectionNames(ContainerBuilder $container): array
private function getTransactionEnabledConnectionNames(ContainerBuilder $container, array $connectionNames): array
{
/** @var bool|array $enableStaticConnectionsConfig */
/** @var bool|array<string, bool> $enableStaticConnectionsConfig */
$enableStaticConnectionsConfig = $container->getParameter('dama.'.Configuration::ENABLE_STATIC_CONNECTION);

$connectionNames = array_keys($container->getParameter('doctrine.connections'));
if (is_array($enableStaticConnectionsConfig)) {
$this->validateConnectionNames(array_keys($enableStaticConnectionsConfig), $connectionNames);
}
Expand Down Expand Up @@ -193,4 +221,18 @@ private function hasSavepointsEnabled(Definition $connectionDefinition): bool

return false;
}

/**
* @param string[] $connectionNames
*
* @return array<string, string|array{primary: string, replicas: array<string, string>}>
*/
private function getConnectionKeys(ContainerBuilder $container, array $connectionNames): array
{
/** @var array<string, string> $connectionKeys */
$connectionKeys = $container->getParameter('dama.'.Configuration::CONNECTION_KEYS);
$this->validateConnectionNames(array_keys($connectionKeys), $connectionNames);

return $connectionKeys;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ public function getItem($key): CacheItemInterface
return $this->adapter->getItem($key);
}

/**
* @return iterable<CacheItemInterface>
*/
public function getItems(array $keys = []): iterable
{
return $this->adapter->getItems($keys);
Expand Down
10 changes: 4 additions & 6 deletions src/DAMA/DoctrineTestBundle/Doctrine/DBAL/StaticDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
class StaticDriver extends Driver\Middleware\AbstractDriverMiddleware
{
/**
* @var Connection[]
* @var array<string, Connection>
*/
private static $connections = [];

Expand All @@ -21,14 +21,12 @@ class StaticDriver extends Driver\Middleware\AbstractDriverMiddleware

public function connect(array $params): Connection
{
if (!self::isKeepStaticConnections()
|| !isset($params['dama.keep_static'])
|| !$params['dama.keep_static']
) {
if (!self::isKeepStaticConnections() || !isset($params['dama.connection_key'])) {
return parent::connect($params);
}

$key = sha1(json_encode($params));
/** @var string $key */
$key = $params['dama.connection_key'];

if (!isset(self::$connections[$key])) {
self::$connections[$key] = parent::connect($params);
Expand Down
Loading