Add Response as possible return type of the method because the method AbstractAuthenticationListener::handle() test if $returnValue is an instance of Response (line 148).
Commits
-------
4eb54a0 update CHANGELOG
db9ea09 [Doctrine] [Bridge] fix repositoryMethod test
2a6c222 Add a customRepository option to the uniqueEntity validator
Discussion
----------
[Doctrine] [Bridge] Add a "repositoryMethod" option to the uniqueEntity validator
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: ~
Todo: ~
License of the code: MIT
Documentation PR: ~
This allows to configure the repository method used to verify uniqueness of entity.
Before, it was always using `findBy`.
---------------------------------------------------------------------------
by fabpot at 2012-07-20T05:35:28Z
Can you add a note in the CHANGELOG?
---------------------------------------------------------------------------
by docteurklein at 2012-07-20T07:17:08Z
@fabpot done.
Commits
-------
ed8823c [HttpFoundation] Allow setting an unknown status code without specifying a text
Discussion
----------
[HttpFoundation] Allow setting an unknown status code without specifying...
... a text
fix#4978
Commits
-------
16a980b [Validator] Fix bug order for constraint, property, getter and group-sequence-provider in validation.xml
Discussion
----------
[Validator] Fix bug order for constraint, property, getter and group-seq...
Actually, there is a bug that force developers to write validation.xml file with the following nodes order:
- constraint
- property
- getter
So that's not possible to have the following XML (because I need to write my property(ies) first).
```xml
<?xml version="1.0" encoding="UTF-8" ?>
<constraint-mapping xmlns="http://symfony.com/schema/dic/constraint-mapping"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://symfony.com/schema/dic/constraint-mappinghttp://symfony.com/schema/dic/constraint-mapping/constraint-mapping-1.0.xsd">
<class name="Application\Eko\MyBundle\Entity\MyEntity">
<getter property="isBar">
<constraint name="True">
<option name="message">My error message</option>
</constraint>
</getter>
<property name="foo">
<constraint name="NotBlank" />
</property>
</class>
</constraint-mapping>
```
The XML below result in the following exception:
```
[ERROR 1871] Element '{http://symfony.com/schema/dic/constraint-mapping}property': This element is not expected. Expected is ( {http://symfony.com/schema/dic/constraint-mapping}getter ). (in /var/www/myproject/src/Application/Eko/MyBundle/Resources/config/validation.xml - line 14, column 0)
```
This is due to the sequence element that needs to respect the order given in the schema file.
The choice element is doing the same thing and permit to have a free order of elements so I have replaced the sequence by a choice element.
For more information: http://www.w3.org/TR/xmlschema-0/#ref17
Commits
-------
c81b2ad [Form] Rename UnmodifiableFormConfig to ImmutableFormConfig
274eb9e [EventDispatcher] Rename UnmodifiableEventDispatcher to ImmutableEventDispatcher
Discussion
----------
Rename unmodifiable to immutable
Maybe it's just me, but it sounded really wrong. The EventDispatcher one was added in 2.1 so no BC break. I don't know about the Form one, but I guess it's just used internally anyway.
Commits
-------
39157a8 [Security] fixes multiple overlapping definitions of DefaultFailureHandler and DefaultSuccessHandler in AbstractFactory
Discussion
----------
[Security] fixes multiple overlapping definitions of DefaultFailureHandler and DefaultSuccessHandler in AbstractFactory
If more than one listener extends AbstractFactory, you'll have multiple calls to createAuthenticationFailureHandler and createAuthenticationSuccessHandler with the same id.
Implicitly it's going to use the one generated by the last factory generating unexpected behavior.
This is related to commits 915704c071 and c6aa392df7
Commits
-------
f5e03a3 Changed $request->setDefaultLocale() to $request->setLocale() in custom LocaleListener.
Discussion
----------
Changed $request->setDefaultLocale() to $request->setLocale() in custom ...
...LocaleListener.
---------------------------------------------------------------------------
by mpiecko at 2012-07-19T13:05:07Z
This is based on my comments from https://github.com/symfony/symfony/pull/4692#issuecomment-7096255
Commits
-------
310c458 [Process] Fixed a problem on RHEL5 where the exit code was incorrect
Discussion
----------
[Process] Fixed a problem on RHEL5 where the exit code was incorrect
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
License of the code: MIT
RHEL5 will intermittently result in an exit code of -1 due to `proc_get_status()` being called after the process has completed but outside of `updateStatus()` which saves the exit code.
See composer/composer#876
Commits
-------
3bb975c [WebProfilerBundle] Fixed white pixel in toolbar info
Discussion
----------
[WebProfilerBundle] Fixed white pixel in toolbar info
Fixed this ![white pixel](http://www7.pic-upload.de/18.07.12/l7ve9xrevwc.png)
Tested on IE9/FF/Chrome
RHEL5 will intermittently result in an exit code of -1 [1] due to
proc_get_status() being called after the process has completed
but outside of updateStatus() which saves the exit code.
[1]: https://github.com/composer/composer/issues/876
Commits
-------
96638f4 [UPGRADE] Tweaking formatting error with event listener section. Also tweaking values
Discussion
----------
[UPGRADE] Tweaking formatting error with event listener section and information
Hey guys!
This is just a documentation change based on sha: 5da1bc6a4f
Check out the details in the commit message - I'm not sure if this is right, but the original commit seemed a bit odd compared to what I was seeing in the code. There was also a fix to the formatting.
If I've missed something, I'm happy to change the commit as needed.
Thanks!
---------------------------------------------------------------------------
by stof at 2012-07-14T21:53:12Z
There is no equivalent to early_kernel_request for the router listener as both methods have been merged together (it was separated so that the initialization of the RequestContext was done before the firewall, but the whole logic is before the firewall now)
Commits
-------
89975ef Added more verbose message for exception when form types have wrong getName method
Discussion
----------
Added more verbose message for exception when form types have wrong getName
PR based on discussion of commit introduces exception when custom form type have no getName method. 6489a65960
---------------------------------------------------------------------------
by biozshock at 2012-07-17T08:15:35Z
@tyx thanks. Fixed. ;)
---------------------------------------------------------------------------
by fabpot at 2012-07-17T20:46:57Z
Can you squash your commits before I merge? Thanks.
---------------------------------------------------------------------------
by noetix at 2012-07-17T23:49:51Z
I think the variables should have quotes around them. It seems like all other error messages have them.
From: The type name specified for the service form.type.translatable does not match the actual name. Expected translatable_field, given translator
To: The type name specified for the service "form.type.translatable" does not match the actual name. Expected "translatable_field", given "translator"
---------------------------------------------------------------------------
by biozshock at 2012-07-18T09:39:01Z
@fabpot Done.
@noetix Thanks for remark.
Commits
-------
1f33756 Update master
Discussion
----------
Allow HttpKernel ->render to be called directly
Hi,
I faced this problem in a funny situation.
Working on tests of individual components, I want to be able to test sub-requests. These elements are not mapped directly, relying on `FrameworkBundle` routing internal to achieve its goal.
The nature of my application leads to fully decoupled bundles, being a website responsible to define everything, from app configuration to routing elements. That way, an individual bundle controller don't know the route it should call, leading the test to be harder to do.
Together with this situation, it is also required in my testing scenario to be able to test the real world execution, then being an ESI include (a sub-request). This test then needs to be able to directly call controller to be rendered. That said, `HttpKernel` provides an API that does exactly what is required to achieve my goal, calling `render()` which triggers the sub-request and also accepts options which allows me to test the real world scenarios.
But as soon as you trigger the method, `ProfileListener` intercepts the kernel response to collect profiling information. Under this specific situation, since you called directly `render`, there's no master request, then leading the test to fail with the following PHP warning:
```
Warning: SplObjectStorage::offsetExists() expects parameter 1 to be object, boolean given in /var/www/nde/vendor/symfony/symfony/src/Symfony/Component/HttpKernel/EventListener/ProfilerListener.php line 120
```
To fix that, all that is needed is a check for a possible parent request. That's the purpose of this patch. =)
---------------------------------------------------------------------------
by stof at 2012-07-17T15:11:13Z
This looks good to me, but as said on IRC, a test should be added to avoid regressions
Commits
-------
1474aa5 [Form] Fixed consideration of Twig's template inheritance and added another performance-improving check
b4ec7f5 Fixed my rubbish English
d11f8b5 [Form] Fixed passing of variables in the FormRenderer
629093e [Form] Extracted common parts of FormHelper and FormExtension into separate classes
216c539 [Form] Implemented a more intelligent caching strategy in FormHelper (PHP +100ms, Twig +100ms)
Discussion
----------
[Form] Merged FormHelper and FormExtension and implemented a better caching strategy
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
This PR extracts common parts of `FormHelper` and `FormExtension` into implementations of the new interfaces `FormRendererInterface` and `FormRendererEngineInterface`. The implemented `AbstractRendererEngine` features a more intelligent caching strategy than the one used before. When this strategy was implemented directly in `FormHelper`, the performance of [this specific, heavy form](http://advancedform.gpserver.dk/app_dev.php/taxclasses/1) could be improved from **2.5** to **2.25 seconds** on my machine for PHP templates.
Due to the abstraction and delegation, the performance gain is not that big anymore, but we still have a performance gain of about **0.1 seconds** for both PHP and Twig in the above example. The second, big improvement of this PR is maintainability - the differences between PHP and Twig templates are now contained in relatively small classes - and extendability (it is very easy now to support different template engines).
---------------------------------------------------------------------------
by stof at 2012-07-14T13:47:19Z
should a similar refactoring be done for the [Twig rendering](https://github.com/symfony/symfony/blob/master/src/Symfony/Bridge/Twig/Extension/FormExtension.php) ?
---------------------------------------------------------------------------
by bschussek at 2012-07-14T13:49:25Z
Yes. I would like to merge the common parts of Twig's FormExtension and PHP's FormHelper into an abstract class. Before that I need to have a [working, heavy Twig Form](https://twitter.com/webmozart/status/224135287377371138) in order to measure whether I don't actually decrease the performance with Twig. Can you help me there?
---------------------------------------------------------------------------
by vicb at 2012-07-16T21:48:24Z
Would it make sense to create a 'renderer' folder in the form component and move related classes there ?
---------------------------------------------------------------------------
by stof at 2012-07-16T22:06:58Z
@vicb It makes sense to keep the Twig renderer in the brisge. This is what the bridge is about. Moving the Twig class to the component would not be consistent. And the PHP renderer is already in the component (but it could make sense to move the helper from FrameworkBundle to the TemplatingExtension of the Form component though)
---------------------------------------------------------------------------
by vicb at 2012-07-16T22:16:50Z
@stof I was only referring to the classes located in the Component/Form folder.
---------------------------------------------------------------------------
by vicb at 2012-07-16T22:27:27Z
Overall I don't really know what to think of this PR. PHP and Twig use a different way to support blocks:
- PHP has one block per file,
- Twig could have many blocks per templates.
I am not sure if this PR is optimal for Twig and improves maintainability ?
---------------------------------------------------------------------------
by stof at 2012-07-16T22:46:11Z
@vicb it avoids duplicating the whole rendering logic for each engine (there is at least a third one in [SmartyBundle](https://github.com/noiselabs/SmartyBundle/blob/master/Extension/FormExtension.php) btw)
---------------------------------------------------------------------------
by bschussek at 2012-07-17T07:16:42Z
@vicb I don't think a renderer subfolder makes sense. The interfaces belong to the main namespace, and then the subfolder would only contain two classes.
Considering maintainability for Twig, I think that this PR in fact increases it. TwigExtension before always had to check the whole type hierarchy, while now the code in AbstractRendererEngine makes sure that this process is speeded up.
Before:
```
load _some_entity_field_label:
- check _some_entity_field_label
- check entity_label
- check choice_label
- check form_label
load _some_other_entity_field_label
- check _some_other_entity_field_label
- check entity_label
- check choice_label
- check form_label
a.s.o.
```
After:
```
load _some_entity_field_label:
- check _some_entity_field_label
- check entity_label (hits the cache if entity_label was checked before)
- check choice_label (hits the cache if choice_label was checked before)
- check form_label
load _some_other_entity_field_label
- check _some_other_entity_field_label
- check entity_label (now definitely hits the cache)
a.s.o.
```
Since many fields share the same ancestors in the inheritance tree, this definitely improves performance.
As can also be deducted here, custom block names such as `_some_entity_field_label` are now a major drawback. There is nothing we can cache for them, so they need to be checked for every individual block that we load. Removing this feature surprisingly gains no performance for Twig (I need to investigate why at some point), but it speeds up rendering for **250ms** using the PHP engine on [this example form](advancedform.gpserver.dk/app_dev.php/taxclasses/1), dropping the rendering time from 1.25 to 1 sec on my local machine. I'm not sure what we should do here.
---------------------------------------------------------------------------
by stof at 2012-07-17T07:21:31Z
@bschussek could it be possible to have an implementation checking the custom block and another one skipping it ? This way, the user could disable this feature when he does not need it.
---------------------------------------------------------------------------
by bschussek at 2012-07-17T07:38:34Z
@stof It would be possible to add a switch to `FormRenderer` that controls whether custom blocks are checked or not.
If this switch is disabled by default, we break BC. If this switch is enabled by default, it will be pretty useless. People will start designing away for custom blocks, and once they want to improve performance, they can't turn off the switch anymore because it would require too many changes.
---------------------------------------------------------------------------
by stof at 2012-07-17T08:08:38Z
@fabpot what do you think about it ?
---------------------------------------------------------------------------
by bschussek at 2012-07-17T08:41:43Z
Another option that just came to mind is to remove inheritance checks for anything but _widget and _row. I.e., if we render `entity_widget`, check
```
_id_widget
entity_widget
choice_widget
form_widget
```
But if we render `entity_label`, only check
```
_id_label
form_label
```
This improves PHP Templating for **170ms** and Twig for **20ms**. We gain another **150ms** for PHP Templating and **~15ms** for Twig if we also restrict custom fields (_id_widget) to the _widget and _row suffixes (it's really hard to tweak the renderer for Twig.. I think a lot of its performance bottlenecks lie in Twig itself).
Do you have any data on how often blocks other than _widget and _row are customized for specific types/IDs?
---------------------------------------------------------------------------
by stof at 2012-07-17T09:47:38Z
Well, I think most of the time other blocks are not even customized based on the type :)
---------------------------------------------------------------------------
by Tobion at 2012-07-17T14:32:39Z
From my experience rendering the form components individually is easier and more flexible than customizing by ID or type.
But there are still use cases for customizing like library-like bundles (e.g. Bootstrap).