From 1ccb82217e5ae042776478f655de67255616a813 Mon Sep 17 00:00:00 2001 From: Bastien Jaillot Date: Mon, 6 Jan 2020 12:48:51 +0100 Subject: [PATCH] Add a feature switch to trigger SchemaValidator in Collector Calling SchemaValidator from DoctrineDataCollector can results in loading a lot of metadatas for relations of retated entities. On some profile, it can be up to 18% of the total (already heavy) pages. As there is already a command to get the schema validation, we can put this validation behind a feature switch `profiling_collect_schema_errors`. Command is: `bin console doctrine:schema:validate` Fixes #1115 --- DataCollector/DoctrineDataCollector.php | 43 +++++----- DependencyInjection/Configuration.php | 4 + DependencyInjection/DoctrineExtension.php | 10 ++- Resources/config/dbal.xml | 1 + Resources/config/schema/doctrine-1.0.xsd | 1 + Resources/doc/configuration.rst | 3 + Resources/views/Collector/db.html.twig | 78 ++++++++++--------- .../DoctrineDataCollectorTest.php | 21 ++++- .../Fixtures/config/xml/dbal_logging.xml | 5 ++ .../Fixtures/config/yml/dbal_logging.yml | 4 + 10 files changed, 110 insertions(+), 60 deletions(-) diff --git a/DataCollector/DoctrineDataCollector.php b/DataCollector/DoctrineDataCollector.php index 5d332a0f7..63c240cf5 100644 --- a/DataCollector/DoctrineDataCollector.php +++ b/DataCollector/DoctrineDataCollector.php @@ -26,9 +26,13 @@ class DoctrineDataCollector extends BaseCollector /** @var string[] */ private $groupedQueries; - public function __construct(ManagerRegistry $registry) + /** @var bool */ + private $shouldValidateSchema; + + public function __construct(ManagerRegistry $registry, bool $shouldValidateSchema = true) { - $this->registry = $registry; + $this->registry = $registry; + $this->shouldValidateSchema = $shouldValidateSchema; parent::__construct($registry); } @@ -59,25 +63,28 @@ public function collect(Request $request, Response $response, Throwable $excepti /** @var EntityManager $em */ foreach ($this->registry->getManagers() as $name => $em) { - $entities[$name] = []; - /** @var ClassMetadataFactory $factory */ - $factory = $em->getMetadataFactory(); - $validator = new SchemaValidator($em); - - /** @var ClassMetadataInfo $class */ - foreach ($factory->getLoadedMetadata() as $class) { - if (isset($entities[$name][$class->getName()])) { - continue; - } + if ($this->shouldValidateSchema) { + $entities[$name] = []; - $classErrors = $validator->validateClass($class); - $entities[$name][$class->getName()] = $class->getName(); + /** @var ClassMetadataFactory $factory */ + $factory = $em->getMetadataFactory(); + $validator = new SchemaValidator($em); - if (empty($classErrors)) { - continue; - } + /** @var ClassMetadataInfo $class */ + foreach ($factory->getLoadedMetadata() as $class) { + if (isset($entities[$name][$class->getName()])) { + continue; + } - $errors[$name][$class->getName()] = $classErrors; + $classErrors = $validator->validateClass($class); + $entities[$name][$class->getName()] = $class->getName(); + + if (empty($classErrors)) { + continue; + } + + $errors[$name][$class->getName()] = $classErrors; + } } /** @var Configuration $emConfig */ diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index dac5a1b57..8911783df 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -136,6 +136,10 @@ private function getDbalConnectionsNode() ->defaultValue(false) ->info('Enables collecting backtraces when profiling is enabled') ->end() + ->booleanNode('profiling_collect_schema_errors') + ->defaultValue(true) + ->info('Enables collecting schema errors when profiling is enabled') + ->end() ->scalarNode('server_version')->end() ->scalarNode('driver_class')->end() ->scalarNode('wrapper_class')->end() diff --git a/DependencyInjection/DoctrineExtension.php b/DependencyInjection/DoctrineExtension.php index 1a95a72a6..441c3f28b 100644 --- a/DependencyInjection/DoctrineExtension.php +++ b/DependencyInjection/DoctrineExtension.php @@ -128,7 +128,9 @@ protected function loadDbalConnection($name, array $connection, ContainerBuilder $profilingLoggerId = $profilingAbstractId . '.' . $name; $container->setDefinition($profilingLoggerId, new ChildDefinition($profilingAbstractId)); $profilingLogger = new Reference($profilingLoggerId); - $container->getDefinition('data_collector.doctrine')->addMethodCall('addLogger', [$name, $profilingLogger]); + $container->getDefinition('data_collector.doctrine') + ->addMethodCall('addLogger', [$name, $profilingLogger]) + ->replaceArgument(1, $connection['profiling_collect_schema_errors']); if ($logger !== null) { $chainLogger = new ChildDefinition('doctrine.dbal.logger.chain'); @@ -141,7 +143,11 @@ protected function loadDbalConnection($name, array $connection, ContainerBuilder $logger = $profilingLogger; } } - unset($connection['profiling'], $connection['profiling_collect_backtrace']); + unset( + $connection['profiling'], + $connection['profiling_collect_backtrace'], + $connection['profiling_collect_schema_errors'] + ); if (isset($connection['auto_commit'])) { $configuration->addMethodCall('setAutoCommit', [$connection['auto_commit']]); diff --git a/Resources/config/dbal.xml b/Resources/config/dbal.xml index ff323248e..cb4ae293c 100644 --- a/Resources/config/dbal.xml +++ b/Resources/config/dbal.xml @@ -41,6 +41,7 @@ + true diff --git a/Resources/config/schema/doctrine-1.0.xsd b/Resources/config/schema/doctrine-1.0.xsd index 58bab75a8..c188cc774 100644 --- a/Resources/config/schema/doctrine-1.0.xsd +++ b/Resources/config/schema/doctrine-1.0.xsd @@ -37,6 +37,7 @@ + diff --git a/Resources/doc/configuration.rst b/Resources/doc/configuration.rst index ad21f0b07..bee1474d0 100644 --- a/Resources/doc/configuration.rst +++ b/Resources/doc/configuration.rst @@ -105,6 +105,8 @@ Configuration Reference profiling: "%kernel.debug%" # When true, profiling also collects a backtrace for each query profiling_collect_backtrace: false + # When true, profiling also collects schema errors for each query + profiling_collect_schema_errors: true server_version: ~ driver_class: ~ @@ -465,6 +467,7 @@ Configuration Reference logging="%kernel.debug%" profiling="%kernel.debug%" profiling-collect-backtrace="false" + profiling-collect-schema-errors="true" server-version="" driver-class="" wrapper-class="" diff --git a/Resources/views/Collector/db.html.twig b/Resources/views/Collector/db.html.twig index 9dc79f6b5..5f699b8ea 100644 --- a/Resources/views/Collector/db.html.twig +++ b/Resources/views/Collector/db.html.twig @@ -387,47 +387,49 @@ {% endif %} {% endif %} -

Entities Mapping

+ {% if collector.entities|length > 0 %} +

Entities Mapping

- {% for manager, classes in collector.entities %} - {% if collector.managers|length > 1 %} -

{{ manager }} entity manager

- {% endif %} + {% for manager, classes in collector.entities %} + {% if collector.managers|length > 1 %} +

{{ manager }} entity manager

+ {% endif %} - {% if classes is empty %} -
-

No loaded entities.

-
- {% else %} - - - - - - - - - {% for class in classes %} - {% set contains_errors = collector.mappingErrors[manager] is defined and collector.mappingErrors[manager][class] is defined %} - - - + {% if classes is empty %} +
+

No loaded entities.

+
+ {% else %} +
ClassMapping errors
{{ class }} - {% if contains_errors %} -
    - {% for error in collector.mappingErrors[manager][class] %} -
  • {{ error }}
    • - {% endfor %} -
- {% else %} - No errors. - {% endif %} -
+ + + + - {% endfor %} - -
ClassMapping errors
- {% endif %} - {% endfor %} + + + {% for class in classes %} + {% set contains_errors = collector.mappingErrors[manager] is defined and collector.mappingErrors[manager][class] is defined %} + + {{ class }} + + {% if contains_errors %} +
    + {% for error in collector.mappingErrors[manager][class] %} +
  • {{ error }}
    • + {% endfor %} +
+ {% else %} + No errors. + {% endif %} + + + {% endfor %} + + + {% endif %} + {% endfor %} + {% endif %}