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

Fix #730 compatibility with web profiler 4 #735

Merged
merged 7 commits into from
Nov 24, 2017
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
16 changes: 10 additions & 6 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,28 +11,32 @@ php:
- 5.6
- 7.0
- 7.1
- 7.2
- nightly

matrix:
include:
- php: 5.5
env: deps=low SYMFONY_DEPRECATIONS_HELPER=weak
- php: 5.6
env: SYMFONY_VERSION="2.8.*@dev"
- php: 5.6
env: deps=dev
env: SYMFONY_VERSION="2.8.*"
- php: 7.0
env: SYMFONY_VERSION="3.2.*"
- php: 7.1
env: stability=RC SYMFONY_DEPRECATIONS_HELPER=weak SYMFONY_VERSION="3.4.*"
- php: 7.2
env: stability=alpha
allow_failures:
- php: nightly
- env: deps=dev

env:
global:
- deps=no

before_install:
- composer self-update
- if [ "$SYMFONY_VERSION" != "" ]; then composer require --no-update symfony/symfony:${SYMFONY_VERSION}; fi;
- if [ "$deps" = "dev" ]; then perl -pi -e 's/^}$/,"minimum-stability":"dev"}/' composer.json; fi;
- if [ "$SYMFONY_VERSION" != "" ]; then jq "(.require, .\"require-dev\")|=(with_entries(if .key|test(\"^symfony/\") then .value|=\"${SYMFONY_VERSION}\" else . end))" composer.json|ex -sc 'wq!composer.json' /dev/stdin; fi;
- if [ ! -z "$stability" ]; then perl -pi -e "s/^}$/,\"minimum-stability\":\"$stability\"}/" composer.json; fi;

install:
- if [ "$deps" = "low" ]; then composer --prefer-lowest --prefer-stable update; else composer update; fi;
Expand Down
12 changes: 12 additions & 0 deletions DataCollector/DoctrineDataCollector.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
use Symfony\Bridge\Doctrine\DataCollector\DoctrineDataCollector as BaseCollector;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\VarDumper\Cloner\Data;

/**
* DoctrineDataCollector.
Expand Down Expand Up @@ -136,6 +137,17 @@ public function collect(Request $request, Response $response, \Exception $except
}
}

// HttpKernel < 3.2 compatibility layer
if (method_exists($this, 'cloneVar')) {
// Might be good idea to replicate this block in doctrine bridge so we can drop this from here after some time.
// This code is compatible with such change, because cloneVar is supposed to check if input is already cloned.
foreach ($this->data['queries'] as &$queries) {
foreach ($queries as &$query) {
$query['params'] = $this->cloneVar($query['params']);
}
}
}

$this->data['entities'] = $entities;
$this->data['errors'] = $errors;
$this->data['caches'] = $caches;
Expand Down
1 change: 1 addition & 0 deletions DependencyInjection/Compiler/EntityListenerPass.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ public function process(ContainerBuilder $container)
}

$resolver = $container->findDefinition($resolverId);
$resolver->setPublic(true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be better to use a ServiceLocator (by changing the lazy-loading listener resolver) rather than forcing to use public services

Copy link
Member Author

@ostrolucky ostrolucky Nov 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what do you mean, although I don't want to change behaviour. Tests expect this service to be public, meaning having it directly accessible from container. Making it otherwise would be BC break. I have done this change only to fix tests. I think it was concretely this one

$this->assertTrue($container->getDefinition('doctrine.orm.em1_entity_listener_resolver')->isPublic());


if (isset($attributes['entity']) && isset($attributes['event'])) {
$this->attachToListener($container, $name, $id, $attributes);
Expand Down
41 changes: 31 additions & 10 deletions Resources/views/Collector/db.html.twig
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
{% extends app.request.isXmlHttpRequest ? '@WebProfiler/Profiler/ajax_layout.html.twig' : '@WebProfiler/Profiler/layout.html.twig' %}
{% extends request.isXmlHttpRequest ? '@WebProfiler/Profiler/ajax_layout.html.twig' : '@WebProfiler/Profiler/layout.html.twig' %}

{% import _self as helper %}

{% block toolbar %}
{% if collector.querycount > 0 or collector.invalidEntityCount > 0 %}
Expand Down Expand Up @@ -113,8 +115,8 @@
{{ render(controller('DoctrineBundle:Profiler:explain', {
token: token,
panel: 'db',
connectionName: app.request.query.get('connection'),
query: app.request.query.get('query')
connectionName: request.query.get('connection'),
query: request.query.get('query')
})) }}
{% else %}
{{ block('queries') }}
Expand Down Expand Up @@ -238,7 +240,7 @@
{{ query.sql|doctrine_pretty_query(highlight_only = true) }}

<div>
<strong class="font-normal text-small">Parameters</strong>: {{ profiler_dump(query.params) }}
<strong class="font-normal text-small">Parameters</strong>: {{ profiler_dump(query.params, 2) }}
</div>

<div class="text-small font-normal">
Expand Down Expand Up @@ -280,7 +282,7 @@
<p>There are no configured database connections.</p>
</div>
{% else %}
{{ include('@WebProfiler/Profiler/table.html.twig', { data: collector.connections, labels: ['Name', 'Service'] }, with_context = false ) }}
{{ helper.render_simple_table('Name', 'Service', collector.connections) }}
{% endif %}

<h2>Entity Managers</h2>
Expand All @@ -290,7 +292,7 @@
<p>There are no configured entity managers.</p>
</div>
{% else %}
{{ include('@WebProfiler/Profiler/table.html.twig', { data: collector.managers, labels: ['Name', 'Service'] }, with_context = false ) }}
{{ helper.render_simple_table('Name', 'Service', collector.managers) }}
{% endif %}

<h2>Second Level Cache</h2>
Expand All @@ -306,7 +308,7 @@
</div>
{% else %}
{% if profiler_markup_version == 1 %}
{{ include('@WebProfiler/Profiler/table.html.twig', { data: collector.cacheCounts }, with_context = false) }}
{{ helper.render_simple_table('Key', 'Value', collector.cacheCounts) }}
{% else %}
<div class="metrics">
<div class="metric">
Expand All @@ -328,17 +330,17 @@

{% if collector.cacheRegions.hits %}
<h3>Number of cache hits</h3>
{{ include('@WebProfiler/Profiler/table.html.twig', { data: collector.cacheRegions.hits }, with_context = false) }}
{{ helper.render_simple_table('Region', 'Hits', collector.cacheRegions.hits) }}
{% endif %}

{% if collector.cacheRegions.misses %}
<h3>Number of cache misses</h3>
{{ include('@WebProfiler/Profiler/table.html.twig', { data: collector.cacheRegions.misses }, with_context = false) }}
{{ helper.render_simple_table('Region', 'Misses', collector.cacheRegions.misses) }}
{% endif %}

{% if collector.cacheRegions.puts %}
<h3>Number of cache puts</h3>
{{ include('@WebProfiler/Profiler/table.html.twig', { data: collector.cacheRegions.puts }, with_context = false) }}
{{ helper.render_simple_table('Region', 'Puts', collector.cacheRegions.puts) }}
{% endif %}
{% endif %}
{% endif %}
Expand Down Expand Up @@ -463,3 +465,22 @@

//]]></script>
{% endblock %}

{% macro render_simple_table(label1, label2, data) %}
<table>
<thead>
<tr>
<th scope="col" class="key">{{ label1 }}</th>
<th scope="col">{{ label2 }}</th>
</tr>
</thead>
<tbody>
{% for key, value in data %}
<tr>
<th scope="row">{{ key }}</th>
<td>{{ value }}</td>
</tr>
{% endfor %}
</tbody>
</table>
{% endmacro %}
21 changes: 11 additions & 10 deletions Tests/DependencyInjection/AbstractDoctrineExtensionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
use PHPUnit\Framework\TestCase;
use Symfony\Bridge\Doctrine\DependencyInjection\CompilerPass\RegisterEventListenersAndSubscribersPass;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\Compiler\ResolveChildDefinitionsPass;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag;
Expand Down Expand Up @@ -348,13 +349,13 @@ public function testLoadMultipleConnections()
$this->assertInstanceOf('Symfony\Component\DependencyInjection\Reference', $arguments[1]);
$this->assertEquals('doctrine.orm.em2_configuration', (string) $arguments[1]);

$definition = $container->getDefinition($container->getAlias('doctrine.orm.em1_metadata_cache'));
$definition = $container->getDefinition((string) $container->getAlias('doctrine.orm.em1_metadata_cache'));
$this->assertEquals('%doctrine_cache.xcache.class%', $definition->getClass());

$definition = $container->getDefinition($container->getAlias('doctrine.orm.em1_query_cache'));
$definition = $container->getDefinition((string) $container->getAlias('doctrine.orm.em1_query_cache'));
$this->assertEquals('%doctrine_cache.array.class%', $definition->getClass());

$definition = $container->getDefinition($container->getAlias('doctrine.orm.em1_result_cache'));
$definition = $container->getDefinition((string) $container->getAlias('doctrine.orm.em1_result_cache'));
$this->assertEquals('%doctrine_cache.array.class%', $definition->getClass());
}

Expand All @@ -376,18 +377,18 @@ public function testEntityManagerMetadataCacheDriverConfiguration()
{
$container = $this->loadContainer('orm_service_multiple_entity_managers');

$definition = $container->getDefinition($container->getAlias('doctrine.orm.em1_metadata_cache'));
$definition = $container->getDefinition((string) $container->getAlias('doctrine.orm.em1_metadata_cache'));
$this->assertDICDefinitionClass($definition, '%doctrine_cache.xcache.class%');

$definition = $container->getDefinition($container->getAlias('doctrine.orm.em2_metadata_cache'));
$definition = $container->getDefinition((string) $container->getAlias('doctrine.orm.em2_metadata_cache'));
$this->assertDICDefinitionClass($definition, '%doctrine_cache.apc.class%');
}

public function testEntityManagerMemcacheMetadataCacheDriverConfiguration()
{
$container = $this->loadContainer('orm_service_simple_single_entity_manager');

$definition = $container->getDefinition($container->getAlias('doctrine.orm.default_metadata_cache'));
$definition = $container->getDefinition((string) $container->getAlias('doctrine.orm.default_metadata_cache'));
$this->assertDICDefinitionClass($definition, '%doctrine_cache.memcache.class%');
$this->assertDICDefinitionMethodCallOnce($definition, 'setMemcache',
array(new Reference('doctrine_cache.services.doctrine.orm.default_metadata_cache.connection'))
Expand All @@ -404,7 +405,7 @@ public function testEntityManagerRedisMetadataCacheDriverConfigurationWithDataba
{
$container = $this->loadContainer('orm_service_simple_single_entity_manager_redis');

$definition = $container->getDefinition($container->getAlias('doctrine.orm.default_metadata_cache'));
$definition = $container->getDefinition((string) $container->getAlias('doctrine.orm.default_metadata_cache'));
$this->assertDICDefinitionClass($definition, '%doctrine_cache.redis.class%');
$this->assertDICDefinitionMethodCallOnce($definition, 'setRedis',
array(new Reference('doctrine_cache.services.doctrine.orm.default_metadata_cache_redis.connection'))
Expand All @@ -420,7 +421,7 @@ public function testDependencyInjectionImportsOverrideDefaults()
{
$container = $this->loadContainer('orm_imports');

$cacheDefinition = $container->getDefinition($container->getAlias('doctrine.orm.default_metadata_cache'));
$cacheDefinition = $container->getDefinition((string) $container->getAlias('doctrine.orm.default_metadata_cache'));
$this->assertEquals('%doctrine_cache.apc.class%', $cacheDefinition->getClass());

$configDefinition = $container->getDefinition('doctrine.orm.default_configuration');
Expand Down Expand Up @@ -604,7 +605,7 @@ public function testSecondLevelCache()
$loggerChainDef = $container->getDefinition('doctrine.orm.default_second_level_cache.logger_chain');
$loggerStatisticsDef = $container->getDefinition('doctrine.orm.default_second_level_cache.logger_statistics');
$myQueryRegionDef = $container->getDefinition('doctrine.orm.default_second_level_cache.region.my_query_region_filelock');
$cacheDriverDef = $container->getDefinition($container->getAlias('doctrine.orm.default_second_level_cache.region_cache_driver'));
$cacheDriverDef = $container->getDefinition((string) $container->getAlias('doctrine.orm.default_second_level_cache.region_cache_driver'));
$configDef = $container->getDefinition('doctrine.orm.default_configuration');
$myEntityRegionArgs = $myEntityRegionDef->getArguments();
$myQueryRegionArgs = $myQueryRegionDef->getArguments();
Expand Down Expand Up @@ -1072,7 +1073,7 @@ private function assertDICDefinitionNoMethodCall(Definition $definition, $method

private function compileContainer(ContainerBuilder $container)
{
$container->getCompilerPassConfig()->setOptimizationPasses(array(new ResolveDefinitionTemplatesPass()));
$container->getCompilerPassConfig()->setOptimizationPasses(array(class_exists(ResolveChildDefinitionsPass::class) ? new ResolveChildDefinitionsPass() : new ResolveDefinitionTemplatesPass()));
$container->getCompilerPassConfig()->setRemovingPasses(array());
$container->compile();
}
Expand Down
9 changes: 5 additions & 4 deletions Tests/DependencyInjection/DoctrineExtensionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Version;
use PHPUnit\Framework\TestCase;
use Symfony\Component\DependencyInjection\Compiler\ResolveChildDefinitionsPass;
use Symfony\Component\DependencyInjection\Compiler\ResolveDefinitionTemplatesPass;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Definition;
Expand Down Expand Up @@ -356,13 +357,13 @@ public function testDependencyInjectionConfigurationDefaults()
$this->assertEquals('doctrine.orm.default_entity_listener_resolver', (string) $calls[12][1][0]);
}

$definition = $container->getDefinition($container->getAlias('doctrine.orm.default_metadata_cache'));
$definition = $container->getDefinition((string) $container->getAlias('doctrine.orm.default_metadata_cache'));
$this->assertEquals('%doctrine_cache.array.class%', $definition->getClass());

$definition = $container->getDefinition($container->getAlias('doctrine.orm.default_query_cache'));
$definition = $container->getDefinition((string) $container->getAlias('doctrine.orm.default_query_cache'));
$this->assertEquals('%doctrine_cache.array.class%', $definition->getClass());

$definition = $container->getDefinition($container->getAlias('doctrine.orm.default_result_cache'));
$definition = $container->getDefinition((string) $container->getAlias('doctrine.orm.default_result_cache'));
$this->assertEquals('%doctrine_cache.array.class%', $definition->getClass());
}

Expand Down Expand Up @@ -817,7 +818,7 @@ private function assertDICDefinitionMethodCallOnce(Definition $definition, $meth

private function compileContainer(ContainerBuilder $container)
{
$container->getCompilerPassConfig()->setOptimizationPasses(array(new ResolveDefinitionTemplatesPass()));
$container->getCompilerPassConfig()->setOptimizationPasses(array(class_exists(ResolveChildDefinitionsPass::class) ? new ResolveChildDefinitionsPass() : new ResolveDefinitionTemplatesPass()));
$container->getCompilerPassConfig()->setRemovingPasses(array());
$container->compile();
}
Expand Down
86 changes: 86 additions & 0 deletions Tests/ProfilerTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
<?php

namespace Doctrine\Bundle\DoctrineBundle\Tests;


use Doctrine\Bundle\DoctrineBundle\DataCollector\DoctrineDataCollector;
use Doctrine\Bundle\DoctrineBundle\Twig\DoctrineExtension;
use Doctrine\Common\Persistence\ManagerRegistry;
use Doctrine\DBAL\Logging\DebugStack;
use PHPUnit\Framework\TestCase as BaseTestCase;
use Symfony\Bridge\Twig\Extension\CodeExtension;
use Symfony\Bridge\Twig\Extension\HttpKernelExtension;
use Symfony\Bridge\Twig\Extension\HttpKernelRuntime;
use Symfony\Bridge\Twig\Extension\RoutingExtension;
use Symfony\Bundle\WebProfilerBundle\Twig\WebProfilerExtension;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\DataCollector\RequestDataCollector;
use Symfony\Component\HttpKernel\Fragment\FragmentHandler;
use Symfony\Component\HttpKernel\Profiler\Profile;
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;

class ProfilerTest extends BaseTestCase
{
/** @var DebugStack */
private $logger;
/** @var \Twig_Environment */
private $twig;
/** @var DoctrineDataCollector */
private $collector;

public function setUp()
{
$this->logger = new DebugStack();
$registry = $this->getMockBuilder(ManagerRegistry::class)->getMock();
$registry->expects($this->once())->method('getManagers')->willReturn([]);
$this->collector = new DoctrineDataCollector($registry);
$this->collector->addLogger('foo', $this->logger);

$twigLoaderFilesystem = new \Twig_Loader_Filesystem(__DIR__.'/../Resources/views/Collector');
$twigLoaderFilesystem->addPath(__DIR__.'/../vendor/symfony/web-profiler-bundle/Resources/views', 'WebProfiler');
$this->twig = new \Twig_Environment($twigLoaderFilesystem, ['debug' => true, 'strict_variables' => true]);

$this->twig->addExtension(new CodeExtension('', '', ''));
$this->twig->addExtension(new RoutingExtension($this->getMockBuilder(UrlGeneratorInterface::class)->getMock()));
$this->twig->addExtension(new HttpKernelExtension($this->getMockBuilder(FragmentHandler::class)->disableOriginalConstructor()->getMock()));
$this->twig->addExtension(new WebProfilerExtension());
$this->twig->addExtension(new DoctrineExtension());

$loader = $this->getMockBuilder(\Twig_RuntimeLoaderInterface::class)->getMock();
$loader->method('load')->willReturn($this->getMockBuilder(HttpKernelRuntime::class)->disableOriginalConstructor()->getMock());
$this->twig->addRuntimeLoader($loader);
}

public function testRender()
{
$this->logger->queries = [
[
'sql' => 'SELECT * FROM foo WHERE bar IN (?, ?)',
'params' => ['foo', 'bar'],
'executionMS' => 1,
],
];

$this->collector->collect($request = new Request(['group' => '0']), $response = new Response());

$profile = new Profile('foo');

// This is only needed for WebProfilerBundle=3.2, remove when support for it is dropped
$requestCollector = new RequestDataCollector();
$requestCollector->collect($request, $response);
$profile->addCollector($requestCollector);

$output = $this->twig->render('db.html.twig', [
'request' => $request,
'token' => 'foo',
'page' => 'foo',
'profile' => $profile,
'collector' => $this->collector,
'queries' => $this->logger->queries,
]);

$output = str_replace(["\e[37m", "\e[0m", "\e[32;1m", "\e[34;1m"], "", $output);
$this->assertContains("SELECT * FROM foo WHERE bar IN ('foo', 'bar');", $output);
}
}
3 changes: 1 addition & 2 deletions Tests/ServiceRepositoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,7 @@ public function testRepositoryServiceWiring()
$this->expectExceptionMessageRegExp($message);
} else {
// PHPUnit 4 compat
$this->setExpectedException(\RuntimeException::class);
$this->setExpectedExceptionMessage($message);
$this->setExpectedExceptionRegExp(\RuntimeException::class, $message);
}
}

Expand Down
Loading