Skip to content

[3.1] [WebProfilerBundle] [DX] Feature allow forward and redirection detection in wdt #17589

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

Merged
merged 4 commits into from
Mar 31, 2016
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
4 changes: 3 additions & 1 deletion src/Symfony/Bundle/FrameworkBundle/Controller/Controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,10 @@ protected function generateUrl($route, $parameters = array(), $referenceType = U
*/
protected function forward($controller, array $path = array(), array $query = array())
{
$request = $this->container->get('request_stack')->getCurrentRequest();
$path['_forwarded'] = $request->attributes;
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be moved to an event listener instead. Otherwise you couldn't track subrequests triggered by user code that doesn't use the base controller class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@xabbuh, the point is not to catch subrequests as profiler already does, we can see them in the request panel.
This feature is about catching the "main" forward", using this special method, as it should be used only once in most use case to quickly click the profile token link of the original request in the wdt.

But I can try to go further, how would you imagine the presentation of sub requests token in the wdt ?

$path['_controller'] = $controller;
$subRequest = $this->container->get('request_stack')->getCurrentRequest()->duplicate($query, null, $path);
$subRequest = $request->duplicate($query, null, $path);

return $this->container->get('http_kernel')->handle($subRequest, HttpKernelInterface::SUB_REQUEST);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bundle\FrameworkBundle\DataCollector;

use Symfony\Component\HttpFoundation\ParameterBag;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\DataCollector\RequestDataCollector as BaseRequestCollector;
use Symfony\Component\HttpKernel\Event\FilterControllerEvent;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;

/**
* RequestDataCollector.
*
* @author Jules Pietri <jusles@heahprod.com>
*/
class RequestDataCollector extends BaseRequestCollector implements EventSubscriberInterface
{
/**
* {@inheritdoc}
*/
public function collect(Request $request, Response $response, \Exception $exception = null)
{
parent::collect($request, $response, $exception);

if ($parentRequestAttributes = $request->attributes->get('_forwarded')) {
if ($parentRequestAttributes instanceof ParameterBag) {
$parentRequestAttributes->set('_forward_token', $response->headers->get('x-debug-token'));
}
}
if ($request->attributes->has('_forward_controller')) {
$this->data['forward'] = array(
'token' => $request->attributes->get('_forward_token'),
'controller' => $this->parseController($request->attributes->get('_forward_controller')),
);
}
}

/**
* Gets the parsed forward controller.
*
* @return array|bool An array with keys 'token' the forward profile token, and
* 'controller' the parsed forward controller, false otherwise
*/
public function getForward()
{
return isset($this->data['forward']) ? $this->data['forward'] : false;
}

public function onKernelController(FilterControllerEvent $event)
{
$this->controllers[$event->getRequest()] = $event->getController();

if ($parentRequestAttributes = $event->getRequest()->attributes->get('_forwarded')) {
if ($parentRequestAttributes instanceof ParameterBag) {
$parentRequestAttributes->set('_forward_controller', $event->getController());
}
}
}

/**
* {@inheritdoc}
*/
public function getName()
{
return 'request';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
<call method="setKernel"><argument type="service" id="kernel" on-invalid="ignore" /></call>
</service>

<service id="data_collector.request" class="Symfony\Component\HttpKernel\DataCollector\RequestDataCollector">
<service id="data_collector.request" class="Symfony\Bundle\FrameworkBundle\DataCollector\RequestDataCollector">
<tag name="kernel.event_subscriber" />
<tag name="data_collector" template="@WebProfiler/Collector/request.html.twig" id="request" priority="335" />
</service>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,59 +2,99 @@

{% block toolbar %}
{% set request_handler %}
{% if collector.controller.class is defined %}
{% set link = collector.controller.file|file_link(collector.controller.line) %}
{% if link %}<a href="{{ link }}" title="{{ collector.controller.file }}">{% else %}<span>{% endif %}
{% import _self as helper %}
{{ helper.set_handler(collector.controller) }}
{% endset %}

{{ collector.controller.class|abbr_class|striptags }}
{% if collector.redirect %}
{% set redirect_handler %}
{% import _self as helper %}
{{ helper.set_handler(collector.redirect.controller, collector.redirect.route, 'GET' != collector.redirect.method ? collector.redirect.method) }}
{% endset %}
{% endif %}

{%- if collector.controller.method -%}
&nbsp;::&nbsp;{{ collector.controller.method }}
{%- endif -%}

{% if link %}</a>{% else %}</span>{% endif %}
{% else %}
<span>{{ collector.controller }}</span>
{% endif %}
{% endset %}
{% if collector.forward %}
{% set forward_handler %}
{% import _self as helper %}
{{ helper.set_handler(collector.forward.controller) }}
{% endset %}
{% endif %}

{% set request_status_code_color = (collector.statuscode >= 400) ? 'red' : (collector.statuscode >= 300) ? 'yellow' : 'green' %}

{% set icon %}
<span class="sf-toolbar-status sf-toolbar-status-{{ request_status_code_color }}">{{ collector.statuscode }}</span>
{% if collector.route %}
<span class="sf-toolbar-label">@</span>
{% if collector.redirect %}{{ include('@WebProfiler/Icon/redirect.svg') }}{% endif %}
{% if collector.forward %}{{ include('@WebProfiler/Icon/forward.svg') }}{% endif %}
<span class="sf-toolbar-label">{{ 'GET' != collector.method ? collector.method }} @</span>
<span class="sf-toolbar-value sf-toolbar-info-piece-additional">{{ collector.route }}</span>
{% endif %}
{% endset %}

{% set text %}
<div class="sf-toolbar-info-piece">
<b>HTTP status</b>
<span>{{ collector.statuscode }} {{ collector.statustext }}</span>
</div>
<div class="sf-toolbar-info-group">
<div class="sf-toolbar-info-piece">
<b>HTTP status</b>
<span>{{ collector.statuscode }} {{ collector.statustext }}</span>
</div>

<div class="sf-toolbar-info-piece">
<b>Controller</b>
<span>{{ request_handler }}</span>
</div>
{% if 'GET' != collector.method -%}
<div class="sf-toolbar-info-piece">
<b>Method</b>
<span>{{ collector.method }}</span>
</div>
{%- endif %}

{% if collector.controller.class is defined %}
<div class="sf-toolbar-info-piece">
<b>Controller class</b>
<span>{{ collector.controller.class }}</span>
<b>Controller</b>
<span>{{ request_handler }}</span>
</div>
{% endif %}

<div class="sf-toolbar-info-piece">
<b>Route name</b>
<span>{{ collector.route|default('NONE') }}</span>
</div>
{% if collector.controller.class is defined -%}
<div class="sf-toolbar-info-piece">
<b>Controller class</b>
<span>{{ collector.controller.class }}</span>
</div>
{%- endif %}

<div class="sf-toolbar-info-piece">
<b>Has session</b>
<span>{% if collector.sessionmetadata|length %}yes{% else %}no{% endif %}</span>
<div class="sf-toolbar-info-piece">
<b>Route name</b>
<span>{{ collector.route|default('NONE') }}</span>
</div>

<div class="sf-toolbar-info-piece">
<b>Has session</b>
<span>{% if collector.sessionmetadata|length %}yes{% else %}no{% endif %}</span>
</div>
</div>

{% if redirect_handler is defined -%}
<div class="sf-toolbar-info-group">
<div class="sf-toolbar-info-piece">
<b>
<span class="sf-toolbar-redirection-status sf-toolbar-status-yellow">{{ collector.redirect.status_code }}</span>
Redirect from
</b>
<span>
{{ redirect_handler }}
(<a href="{{ path('_profiler', { token: collector.redirect.token }) }}">{{ collector.redirect.token }}</a>)
</span>
</div>
</div>
{% endif %}

{% if forward_handler is defined %}
<div class="sf-toolbar-info-group">
<div class="sf-toolbar-info-piece">
<b>Forwarded to</b>
<span>
{{ forward_handler }}
(<a href="{{ path('_profiler', { token: collector.forward.token }) }}">{{ collector.forward.token }}</a>)
</span>
</div>
</div>
{% endif %}
{% endset %}

{{ include('@WebProfiler/Profiler/toolbar_item.html.twig', { link: profiler_url }) }}
Expand Down Expand Up @@ -224,3 +264,22 @@
{% endif %}
</div>
{% endblock %}

{% macro set_handler(controller, route, method) %}
{% if controller.class is defined -%}
{%- if method|default(false) %}<span class="sf-toolbar-status sf-toolbar-redirection-method">{{ method }}</span>{% endif -%}
{%- set link = controller.file|file_link(controller.line) %}
{%- if link %}<a href="{{ link }}" title="{{ controller.file }}">{% else %}<span>{% endif %}

{%- if route|default(false) -%}
@{{ route }}
{%- else -%}
{{- controller.class|abbr_class|striptags -}}
{{- controller.method ? ' :: ' ~ controller.method -}}
{%- endif -%}

{%- if link %}</a>{% else %}</span>{% endif %}
{%- else -%}
<span>{{ route|default(controller) }}</span>
{%- endif %}
{% endmacro %}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,47 @@
{% endif %}
</h2>

{% set request_collector = profile.collectors.request|default(false) %}
{% if request_collector is defined and request_collector.redirect -%}
{%- set redirect = request_collector.redirect -%}
{%- set controller = redirect.controller -%}
{%- set redirect_route = '@' ~ redirect.route %}
<dl class="metadata">
<dt>
<span class="label">{{ redirect.status_code }}</span>
Redirect from
</dt>
<dd>
{{ 'GET' != redirect.method ? redirect.method }}
{% if redirect.controller.class is defined -%}
{%- set link = controller.file|file_link(controller.line) -%}
{% if link %}<a href="{{ link }}" title="{{ controller.file }}">{% endif -%}
{{ redirect_route }}
{%- if link %}</a>{% endif -%}
{%- else -%}
{{ redirect_route }}
{%- endif %}
(<a href="{{ path('_profiler', { token: redirect.token }) }}">{{ redirect.token }}</a>)
</dd>
</dl>
{%- endif %}

{% if request_collector and request_collector.forward and request_collector.forward.controller.class is defined -%}
{%- set forward = request_collector.forward -%}
{%- set controller = forward.controller -%}
<dl class="metadata">
<dt>Forwarded to</dt>
<dd>
{% set link = controller.file|file_link(controller.line) -%}
{%- if link %}<a href="{{ link }}" title="{{ controller.file }}">{% endif -%}
{{- controller.class|abbr_class|striptags -}}
{{- controller.method ? ' :: ' ~ controller.method }}
{%- if link %}</a>{% endif %}
(<a href="{{ path('_profiler', { token: forward.token }) }}">{{ forward.token }}</a>)
</dd>
</dl>
{%- endif %}

<dl class="metadata">
<dt>Method</dt>
<dd>{{ profile.method|upper }}</dd>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -483,11 +483,11 @@ tr.status-warning td {
#summary .status-error { background: {{ colors.error|raw }}; }

#summary .status-success h2,
#summary .status-success h2 a,
#summary .status-success a,
#summary .status-warning h2,
#summary .status-warning h2 a,
#summary .status-warning a,
#summary .status-error h2,
#summary .status-error h2 a {
#summary .status-error a {
color: #FFF;
}

Expand All @@ -510,6 +510,10 @@ tr.status-warning td {
margin: 0 1.5em 0 0;
}

#summary dl.metadata .label {
background: rgba(255, 255, 255, 0.2);
}

{# Sidebar
========================================================================= #}
#sidebar {
Expand Down
Loading