Skip to content

[FrameworkBundle] Added configuration for additionnal request formats #8944

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

Closed
wants to merge 11 commits into from
Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ public function getConfigTreeBuilder()
$this->addProfilerSection($rootNode);
$this->addRouterSection($rootNode);
$this->addSessionSection($rootNode);
$this->addRequestSection($rootNode);
$this->addTemplatingSection($rootNode);
$this->addTranslatorSection($rootNode);
$this->addValidationSection($rootNode);
Expand Down Expand Up @@ -256,6 +257,31 @@ private function addSessionSection(ArrayNodeDefinition $rootNode)
;
}

private function addRequestSection(ArrayNodeDefinition $rootNode)
{
$rootNode
->children()
->arrayNode('request')
->info('request configuration')
->canBeUnset()
->fixXmlConfig('additional_format')
->children()
Copy link
Member

Choose a reason for hiding this comment

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

You are missing the ->fixXmlConfig('additional_format') call to make the prototyped node work properly for XML configs

->arrayNode('additional_formats')
->useAttributeAsKey('name')
->prototype('array')
Copy link
Member

Choose a reason for hiding this comment

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

you are missing useAttributeAsKey to make it an associative array

Copy link
Member

Choose a reason for hiding this comment

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

you are still missing the call to useAttributeAsKey. Please add a test using this feature in different formats to see that it is broken currently

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok thanks, will do soon

->beforeNormalization()
->ifTrue(function($v) { return !is_array($v); })
->then(function($v) { return array($v); })
->end()
->prototype('scalar')->end()
->end()
->end()
->end()
->end()
->end()
;
}

private function addTemplatingSection(ArrayNodeDefinition $rootNode)
{
$organizeUrls = function($urls) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,10 @@ public function load(array $configs, ContainerBuilder $container)
$this->registerSessionConfiguration($config['session'], $container, $loader);
}

if (isset($config['request'])) {
$this->registerRequestConfiguration($config['request'], $container, $loader);
}

$loader->load('security.xml');

if ($this->isConfigEnabled($container, $config['form'])) {
Expand Down Expand Up @@ -382,6 +386,21 @@ private function registerSessionConfiguration(array $config, ContainerBuilder $c
$container->setParameter('session.metadata.update_threshold', $config['metadata_update_threshold']);
}

/**
* Loads the request configuration.
*
* @param array $config A session configuration array
* @param ContainerBuilder $container A ContainerBuilder instance
* @param XmlFileLoader $loader An XmlFileLoader instance
*/
private function registerRequestConfiguration(array $config, ContainerBuilder $container, XmlFileLoader $loader)
{
if ($config['additional_formats']) {
$container->setParameter('request.additional_formats', $config['additional_formats']);
$loader->load('request.xml');
}
}

/**
* Loads the templating configuration.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
<?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\EventListener;

use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpKernel\KernelEvents;
use Symfony\Component\HttpKernel\Event\GetResponseEvent;

/**
* Set additional formats into the request
*
* @author Gildas Quemener <gildas.quemener@gmail.com>
*/
class AddRequestFormatsListener implements EventSubscriberInterface
{
/**
* @var array
*/
protected $formats;

/**
* @param array $formats
*/
public function __construct(array $formats)
{
$this->formats = $formats;
}

/**
* Set additionnal request formats
*
* @param GetResponseEvent $event
*/
public function onKernelRequest(GetResponseEvent $event)
{
foreach ($this->formats as $format => $mimeTypes) {
$event->getRequest()->setFormat($format, $mimeTypes);
}
}

/**
* {@inheritdoc}
*/
public static function getSubscribedEvents()
{
return array(
KernelEvents::REQUEST => 'onKernelRequest',
);
}
}
17 changes: 17 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/Resources/config/request.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?xml version="1.0" ?>

<container xmlns="http://symfony.com/schema/dic/services"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">

<parameters>
<parameter key="request.add_request_formats_listener.class">Symfony\Bundle\FrameworkBundle\EventListener\AddRequestFormatsListener</parameter>
</parameters>

<services>
<service id="request.add_request_formats_listener" class="%request.add_request_formats_listener.class%">
<tag name="kernel.event_subscriber" />
<argument>%request.additional_formats%</argument>
</service>
</services>
</container>
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
<xsd:element name="fragments" type="fragments" minOccurs="0" maxOccurs="1" />
<xsd:element name="profiler" type="profiler" minOccurs="0" maxOccurs="1" />
<xsd:element name="router" type="router" minOccurs="0" maxOccurs="1" />
<xsd:element name="request" type="request" minOccurs="0" maxOccurs="1" />
<xsd:element name="session" type="session" minOccurs="0" maxOccurs="1" />
<xsd:element name="templating" type="templating" minOccurs="0" maxOccurs="1" />
<xsd:element name="translator" type="translator" minOccurs="0" maxOccurs="1" />
Expand Down Expand Up @@ -101,6 +102,19 @@
<xsd:attribute name="save-path" type="xsd:string" />
</xsd:complexType>

<xsd:complexType name="request">
<xsd:sequence>
<xsd:element name="additional-format" type="additional-format" minOccurs="0" maxOccurs="unbounded" />
</xsd:sequence>
</xsd:complexType>

<xsd:complexType name="additional-format">
<xsd:choice minOccurs="1" maxOccurs="unbounded">
<xsd:element name="mime-type" type="xsd:string" />
</xsd:choice>
<xsd:attribute name="name" type="xsd:string" />
Copy link
Member

Choose a reason for hiding this comment

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

you should mark this attribute as required as we use it as key

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok thanks I'll fix it.

I'm still having issue finding the correct way to support https://github.com/symfony/symfony/pull/8944/files#L11R131 using xml.

@docteurklein rose up the fact that it might not be possible (gquemener@b41388e#commitcomment-4065417), but the FrameworkExtensionTest architecture suggest that all configuration format should behave identically. Is it okay if it's not the case?

I mean, is it ok if it's not possible to define multiple mime-types for a given format when using xml?

</xsd:complexType>

<xsd:complexType name="templating">
<xsd:sequence>
<xsd:element name="loader" type="xsd:string" minOccurs="0" maxOccurs="unbounded" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,5 +70,16 @@
'debug' => true,
'file_cache_dir' => '%kernel.cache_dir%/annotations',
),
'ide' => 'file%%link%%format'
'ide' => 'file%%link%%format',
'request' => array(
'additional_formats' => array(
'csv' => array(
'text/csv',
'text/plain',
),
'pdf' => array(
'application/pdf'
)
)
)
));
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?php

$container->loadFromExtension('framework', array(
'request' => array(
'additional_formats' => array(),
),
));
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,11 @@
<framework:translator enabled="true" fallback="fr" />
<framework:validation enabled="true" cache="apc" />
<framework:annotations cache="file" debug="true" file-cache-dir="%kernel.cache_dir%/annotations" />
<framework:request>
<framework:additional-format name="csv">
<framework:mime-type>text/csv</framework:mime-type>
<framework:mime-type>text/plain</framework:mime-type>
</framework:additional-format>
</framework:request>
</framework:config>
</container>
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?xml version="1.0" ?>

<container xmlns="http://symfony.com/schema/dic/services"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:framework="http://symfony.com/schema/dic/symfony"
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd
http://symfony.com/schema/dic/symfony http://symfony.com/schema/dic/symfony/symfony-1.0.xsd">

<framework:config>
<framework:request />
</framework:config>
</container>
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,7 @@ framework:
debug: true
file_cache_dir: %kernel.cache_dir%/annotations
ide: file%%link%%format
request:
additional_formats:
csv: ['text/csv', 'text/plain']
pdf: 'application/pdf'
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
framework:
request:
additional_formats: ~
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,21 @@ public function testNullSessionHandler()
$this->assertNull($container->getDefinition('session.storage.php_bridge')->getArgument(0));
}

public function testRequest()
{
$container = $this->createContainerFromFile('full');

$this->assertTrue($container->hasDefinition('request.add_request_formats_listener'), '->registerRequestConfiguration() loads request.xml');
$this->assertEquals(array('csv' => array('text/csv', 'text/plain'), 'pdf' => array('application/pdf')), $container->getParameter('request.additional_formats'));
}

public function testEmptyAdditionlRequestFormats()
{
$container = $this->createContainerFromFile('request');

$this->assertFalse($container->hasDefinition('request.add_request_formats_listener'), '->registerRequestConfiguration() does not load request.xml when no additional request formats are configured');
}

public function testTemplating()
{
$container = $this->createContainerFromFile('full');
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
<?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\Tests\EventListener;

use Symfony\Bundle\FrameworkBundle\EventListener\AddRequestFormatsListener;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\KernelEvents;

/**
* Test AddRequestFormatsListener class
*
* @author Gildas Quemener <gildas.quemener@gmail.com>
*/
class AddRequestFormatsListenerTest extends \PHPUnit_Framework_TestCase
{
/**
* @var AddRequestFormatsListener
*/
private $listener;

protected function setUp()
{
$this->listener = new AddRequestFormatsListener(array('csv' => array('text/csv', 'text/plain')));
}

protected function tearDown()
{
$this->listener = null;
}

public function testIsAnEventSubscriber()
{
$this->assertInstanceOf('Symfony\Component\EventDispatcher\EventSubscriberInterface', $this->listener);
}

public function testRegisteredEvent()
{
$this->assertEquals(array(
KernelEvents::REQUEST => 'onKernelRequest',
), AddRequestFormatsListener::getSubscribedEvents());
Copy link
Contributor

Choose a reason for hiding this comment

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

is this indentation correct?

}

public function testSetAdditionnalFormats()
{
$request = $this->getRequestMock();
$event = $this->getGetResponseEventMock($request);

$request->expects($this->once())
->method('setFormat')
->with('csv', array('text/csv', 'text/plain'));

$this->listener->onKernelRequest($event);
}

protected function getRequestMock()
{
return $this->getMock('Symfony\Component\HttpFoundation\Request');
}

protected function getGetResponseEventMock(Request $request)
{
$event = $this
->getMockBuilder('Symfony\Component\HttpKernel\Event\GetResponseEvent')
->disableOriginalConstructor()
->getMock();

$event->expects($this->any())
->method('getRequest')
->will($this->returnValue($request));

return $event;
}
}