Skip to content

Commit

Permalink
Replace DbalLogger by DebugMiddleware
Browse files Browse the repository at this point in the history
  • Loading branch information
l-vo authored and ostrolucky committed May 21, 2022
1 parent f9339b2 commit 9583584
Show file tree
Hide file tree
Showing 10 changed files with 704 additions and 40 deletions.
11 changes: 9 additions & 2 deletions DataCollector/DoctrineDataCollector.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use Doctrine\Persistence\ManagerRegistry;
use Doctrine\Persistence\Mapping\AbstractClassMetadataFactory;
use Symfony\Bridge\Doctrine\DataCollector\DoctrineDataCollector as BaseCollector;
use Symfony\Bridge\Doctrine\Middleware\Debug\DebugDataHolder;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Throwable;
Expand Down Expand Up @@ -64,12 +65,18 @@ class DoctrineDataCollector extends BaseCollector
/** @var bool */
private $shouldValidateSchema;

public function __construct(ManagerRegistry $registry, bool $shouldValidateSchema = true)
/** @psalm-suppress UndefinedClass */
public function __construct(ManagerRegistry $registry, bool $shouldValidateSchema = true, ?DebugDataHolder $debugDataHolder = null)
{
$this->registry = $registry;
$this->shouldValidateSchema = $shouldValidateSchema;

parent::__construct($registry);
if ($debugDataHolder === null) {
parent::__construct($registry);
} else {
/** @psalm-suppress TooManyArguments */
parent::__construct($registry, $debugDataHolder);
}
}

/**
Expand Down
65 changes: 50 additions & 15 deletions DependencyInjection/DoctrineExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
use Doctrine\DBAL\Connections\PrimaryReadReplicaConnection;
use Doctrine\DBAL\Driver\Middleware as MiddlewareInterface;
use Doctrine\DBAL\Logging\LoggerChain;
use Doctrine\DBAL\Logging\Middleware;
use Doctrine\DBAL\Sharding\PoolingShardConnection;
use Doctrine\DBAL\Sharding\PoolingShardManager;
use Doctrine\DBAL\Tools\Console\Command\ImportCommand;
Expand All @@ -34,6 +33,7 @@
use Symfony\Bridge\Doctrine\IdGenerator\UuidGenerator;
use Symfony\Bridge\Doctrine\Messenger\DoctrineClearEntityManagerWorkerSubscriber;
use Symfony\Bridge\Doctrine\Messenger\DoctrineTransactionMiddleware;
use Symfony\Bridge\Doctrine\Middleware\Debug\Middleware as SfDebugMiddleware;
use Symfony\Bridge\Doctrine\PropertyInfo\DoctrineExtractor;
use Symfony\Bridge\Doctrine\SchemaListener\DoctrineDbalCacheAdapterSchemaSubscriber;
use Symfony\Bridge\Doctrine\SchemaListener\MessengerTransportDoctrineSchemaSubscriber;
Expand Down Expand Up @@ -116,9 +116,6 @@ protected function dbalLoad(array $config, ContainerBuilder $container)
{
$loader = new XmlFileLoader($container, new FileLocator(__DIR__ . '/../Resources/config'));
$loader->load('dbal.xml');
$chainLogger = $container->getDefinition('doctrine.dbal.logger.chain');
$logger = new Reference('doctrine.dbal.logger');
$chainLogger->addArgument([$logger]);

if (class_exists(ImportCommand::class)) {
$container->register('doctrine.database_import_command', ImportDoctrineCommand::class)
Expand Down Expand Up @@ -147,12 +144,22 @@ protected function dbalLoad(array $config, ContainerBuilder $container)
$container->setParameter('doctrine.connections', $connections);
$container->setParameter('doctrine.default_connection', $this->defaultConnection);

$connWithLogging = [];
$connWithLogging = [];
$connWithProfiling = [];
$connWithBacktrace = [];
foreach ($config['connections'] as $name => $connection) {
if ($connection['logging']) {
$connWithLogging[] = $name;
}

if ($connection['profiling']) {
$connWithProfiling[] = $name;

if ($connection['profiling_collect_backtrace']) {
$connWithBacktrace[] = $name;
}
}

$this->loadDbalConnection($name, $connection, $container);
}

Expand All @@ -173,7 +180,7 @@ protected function dbalLoad(array $config, ContainerBuilder $container)
});
}

$this->useMiddlewaresIfAvailable($container, $connWithLogging);
$this->useMiddlewaresIfAvailable($container, $connWithLogging, $connWithProfiling, $connWithBacktrace);
}

/**
Expand All @@ -187,7 +194,9 @@ protected function loadDbalConnection($name, array $connection, ContainerBuilder
{
$configuration = $container->setDefinition(sprintf('doctrine.dbal.%s_connection.configuration', $name), new ChildDefinition('doctrine.dbal.connection.configuration'));
$logger = null;
if ($connection['logging']) {

/** @psalm-suppress UndefinedClass */
if (! interface_exists(MiddlewareInterface::class) && $connection['logging']) {
$logger = new Reference('doctrine.dbal.logger');
}

Expand All @@ -196,7 +205,7 @@ protected function loadDbalConnection($name, array $connection, ContainerBuilder
$dataCollectorDefinition = $container->getDefinition('data_collector.doctrine');
$dataCollectorDefinition->replaceArgument(1, $connection['profiling_collect_schema_errors']);

if ($connection['profiling']) {
if (! $this->isSfDebugMiddlewareAvailable() && $connection['profiling']) {
$profilingAbstractId = $connection['profiling_collect_backtrace'] ?
'doctrine.dbal.logger.backtrace' :
'doctrine.dbal.logger.profiling';
Expand Down Expand Up @@ -1116,24 +1125,50 @@ private function createArrayAdapterCachePool(ContainerBuilder $container, string
return $id;
}

/** @param string[] $connWithLogging */
private function useMiddlewaresIfAvailable(ContainerBuilder $container, array $connWithLogging): void
{
/**
* @param string[] $connWithLogging
* @param string[] $connWithProfiling
* @param string[] $connWithBacktrace
*/
private function useMiddlewaresIfAvailable(
ContainerBuilder $container,
array $connWithLogging,
array $connWithProfiling,
array $connWithBacktrace
): void {
/** @psalm-suppress UndefinedClass */
if (! class_exists(Middleware::class)) {
if (! interface_exists(MiddlewareInterface::class)) {
return;
}

$loader = new XmlFileLoader($container, new FileLocator(__DIR__ . '/../Resources/config'));
$loader->load('middlewares.xml');

$container
->getDefinition('doctrine.dbal.logger')
->replaceArgument(0, null);

$loader = new XmlFileLoader($container, new FileLocator(__DIR__ . '/../Resources/config'));
$loader->load('middlewares.xml');

$loggingMiddlewareAbstractDef = $container->getDefinition('doctrine.dbal.logging_middleware');
foreach ($connWithLogging as $connName) {
$loggingMiddlewareAbstractDef->addTag('doctrine.middleware', ['connection' => $connName]);
}

if ($this->isSfDebugMiddlewareAvailable()) {
$container->getDefinition('doctrine.debug_data_holder')->replaceArgument(0, $connWithBacktrace);
$debugMiddlewareAbstractDef = $container->getDefinition('doctrine.dbal.debug_middleware');
foreach ($connWithProfiling as $connName) {
$debugMiddlewareAbstractDef
->addTag('doctrine.middleware', ['connection' => $connName]);
}
} else {
$container->removeDefinition('doctrine.dbal.debug_middleware');
$container->removeDefinition('doctrine.debug_data_holder');
}
}

private function isSfDebugMiddlewareAvailable(): bool
{
/** @psalm-suppress UndefinedClass */
return interface_exists(MiddlewareInterface::class) && class_exists(SfDebugMiddleware::class);
}
}
92 changes: 92 additions & 0 deletions Middleware/BacktraceDebugDataHolder.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
<?php

namespace Doctrine\Bundle\DoctrineBundle\Middleware;

use Symfony\Bridge\Doctrine\Middleware\Debug\DebugDataHolder;
use Symfony\Bridge\Doctrine\Middleware\Debug\Query;

use function array_slice;
use function debug_backtrace;
use function in_array;

use const DEBUG_BACKTRACE_IGNORE_ARGS;

/** @psalm-suppress MissingDependency */
class BacktraceDebugDataHolder extends DebugDataHolder
{
/** @var string[] */
private $connWithBacktraces;

/** @var array<string, array<string, mixed>[]> */
private $backtraces = [];

/** @param string[] $connWithBacktraces */
public function __construct(array $connWithBacktraces)
{
$this->connWithBacktraces = $connWithBacktraces;
}

public function reset(): void
{
parent::reset();

$this->backtraces = [];
}

public function addQuery(string $connectionName, Query $query): void
{
parent::addQuery($connectionName, $query);

if (! in_array($connectionName, $this->connWithBacktraces, true)) {
return;
}

// array_slice to skip middleware calls in the trace
$this->backtraces[$connectionName][] = array_slice(debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS), 2);
}

/** @return array<string, array<string, mixed>[]> */
public function getData(): array
{
$dataWithBacktraces = [];

$data = parent::getData();
foreach ($data as $connectionName => $dataForConn) {
$dataWithBacktraces[$connectionName] = $this->getDataForConnection($connectionName, $dataForConn);
}

return $dataWithBacktraces;
}

/**
* @param mixed[][] $dataForConn
*
* @return mixed[][]
*/
private function getDataForConnection(string $connectionName, array $dataForConn): array
{
$data = [];

foreach ($dataForConn as $idx => $record) {
$data[] = $this->addBacktracesIfAvailable($connectionName, $record, $idx);
}

return $data;
}

/**
* @param mixed[] $record
*
* @return mixed[]
*/
private function addBacktracesIfAvailable(string $connectionName, array $record, int $idx): array
{
if (! isset($this->backtraces[$connectionName])) {
return $record;
}

$record['backtrace'] = $this->backtraces[$connectionName][$idx];

return $record;
}
}
37 changes: 37 additions & 0 deletions Middleware/DebugMiddleware.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?php

namespace Doctrine\Bundle\DoctrineBundle\Middleware;

use Doctrine\DBAL\Driver as DriverInterface;
use Doctrine\DBAL\Driver\Middleware;
use Symfony\Bridge\Doctrine\Middleware\Debug\DebugDataHolder;
use Symfony\Bridge\Doctrine\Middleware\Debug\Driver;
use Symfony\Component\Stopwatch\Stopwatch;

class DebugMiddleware implements Middleware, ConnectionNameAwareInterface
{
/** @var DebugDataHolder */
private $debugDataHolder;

/** @var Stopwatch|null */
private $stopwatch;

/** @var string */
private $connectionName = 'default';

public function __construct(DebugDataHolder $debugDataHolder, ?Stopwatch $stopwatch)
{
$this->debugDataHolder = $debugDataHolder;
$this->stopwatch = $stopwatch;
}

public function setConnectionName(string $name): void
{
$this->connectionName = $name;
}

public function wrap(DriverInterface $driver): DriverInterface
{
return new Driver($driver, $this->debugDataHolder, $this->stopwatch, $this->connectionName);
}
}
7 changes: 7 additions & 0 deletions Resources/config/middlewares.xml
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,12 @@
<argument type="service" id="logger" />
<tag name="monolog.logger" channel="doctrine" />
</service>
<service id="doctrine.debug_data_holder" class="Doctrine\Bundle\DoctrineBundle\Middleware\BacktraceDebugDataHolder">
<argument type="collection" />
</service>
<service id="doctrine.dbal.debug_middleware" class="Doctrine\Bundle\DoctrineBundle\Middleware\DebugMiddleware" abstract="true">
<argument type="service" id="doctrine.debug_data_holder" />
<argument type="service" id="debug.stopwatch" on-invalid="null" />
</service>
</services>
</container>
22 changes: 17 additions & 5 deletions Tests/ContainerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use Doctrine\Common\EventManager;
use Doctrine\DBAL\Configuration as DBALConfiguration;
use Doctrine\DBAL\Connection;
use Doctrine\DBAL\Driver\Middleware as MiddlewareInterface;
use Doctrine\DBAL\Logging\LoggerChain;
use Doctrine\DBAL\Tools\Console\Command\ImportCommand;
use Doctrine\DBAL\Types\Type;
Expand All @@ -21,6 +22,7 @@
use Symfony\Bridge\Doctrine\CacheWarmer\ProxyCacheWarmer;
use Symfony\Bridge\Doctrine\DataCollector\DoctrineDataCollector;
use Symfony\Bridge\Doctrine\Logger\DbalLogger;
use Symfony\Bridge\Doctrine\Middleware\Debug\Middleware as SfDebugMiddleware;
use Symfony\Bridge\Doctrine\PropertyInfo\DoctrineExtractor;
use Symfony\Bridge\Doctrine\Validator\Constraints\UniqueEntityValidator;
use Symfony\Bridge\Doctrine\Validator\DoctrineLoader;
Expand All @@ -32,16 +34,22 @@

class ContainerTest extends TestCase
{
/** @group legacy */
public function testContainerWithDbalOnly(): void
{
$kernel = new DbalTestKernel();
$kernel->boot();

$container = $kernel->getContainer();
$this->assertInstanceOf(
LoggerChain::class,
$container->get('doctrine.dbal.logger.chain.default')
);

/** @psalm-suppress UndefinedClass */
if (! interface_exists(MiddlewareInterface::class)) {
$this->assertInstanceOf(
LoggerChain::class,
$container->get('doctrine.dbal.logger.chain.default')
);
}

if (class_exists(ImportCommand::class)) {
self::assertTrue($container->has('doctrine.database_import_command'));
} else {
Expand All @@ -57,7 +65,11 @@ public function testContainer(): void

$container = $this->createXmlBundleTestContainer();

$this->assertInstanceOf(DbalLogger::class, $container->get('doctrine.dbal.logger'));
/** @psalm-suppress UndefinedClass */
if (! interface_exists(MiddlewareInterface::class) || ! class_exists(SfDebugMiddleware::class)) {
$this->assertInstanceOf(DbalLogger::class, $container->get('doctrine.dbal.logger'));
}

$this->assertInstanceOf(DoctrineDataCollector::class, $container->get('data_collector.doctrine'));
$this->assertInstanceOf(DBALConfiguration::class, $container->get('doctrine.dbal.default_connection.configuration'));
$this->assertInstanceOf(EventManager::class, $container->get('doctrine.dbal.default_connection.event_manager'));
Expand Down
6 changes: 6 additions & 0 deletions Tests/DependencyInjection/AbstractDoctrineExtensionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use Doctrine\Common\Cache\Psr6\DoctrineProvider;
use Doctrine\DBAL\Configuration;
use Doctrine\DBAL\Connections\PrimaryReadReplicaConnection;
use Doctrine\DBAL\Driver\Middleware;
use Doctrine\DBAL\DriverManager;
use Doctrine\ORM\EntityManager;
use Doctrine\ORM\EntityManagerInterface;
Expand Down Expand Up @@ -447,6 +448,11 @@ public function testLoadMultipleConnections(): void

public function testLoadLogging(): void
{
/** @psalm-suppress UndefinedClass */
if (interface_exists(Middleware::class)) {
$this->markTestSkipped('This test requires Debug middleware to not be activated');
}

$container = $this->loadContainer('dbal_logging');

$definition = $container->getDefinition('doctrine.dbal.log_connection.configuration');
Expand Down
Loading

0 comments on commit 9583584

Please sign in to comment.