Commits
-------
0b78fdf Only call registerCommand on bundles that is an instance of Bundle
Discussion
----------
Only call registerCommand on bundles that is an instance of Bundle
Fixes GH-5133
---------------------------------------------------------------------------
by travisbot at 2012-08-01T09:41:05Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/2008252) (merged 0b78fdff into 1da896dc).
---------------------------------------------------------------------------
by henrikbjorn at 2012-08-01T10:05:00Z
Build failed because of HTTP request error.
---------------------------------------------------------------------------
by lsmith77 at 2012-08-01T11:31:08Z
wondering if it would be good if you could include the commit from #5133 in this PR .. then we get the test and the fix at once.
OptionsResolver#validateOptionsCompleteness would already have thrown exception if the option were required, so this should only affect something explicitly marked as optional
Commits
-------
4ae54e3 [Composer] Bumped doctrine/orm to 2.2.3
Discussion
----------
[Composer] Bumped doctrine/orm to 2.2.3
fix#4966
---------------------------------------------------------------------------
by stloyd at 2012-07-31T15:01:41Z
You should also _bump_ Security component [deps](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Security/composer.json#L28).
---------------------------------------------------------------------------
by vicb at 2012-07-31T15:05:03Z
The security does not depend on `doctrine/orm`
Commits
-------
b982883 [Form] Moved FormHelper back to FrameworkBundle
cb62d05 [Form] [Validator] Fixed issues mentioned in the PR
2185ca8 [Validator] Added entry point "Validation" for more convenient usage outside of Symfony2
ed87361 [Form] Moved FormHelper creation to TemplatingExtension
87ccb6a [Form] Added entry point "Forms" for more convenient usage outside of Symfony
Discussion
----------
[Form] [Validator] Added more convenient entry points for stand-alone usage
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
This PR greatly simplifies the usage of the Form and Validator component when used outside of Symfony2. Check out the below code to get an idea about the simplified usage:
```php
<?php
use Symfony\Component\Validator\Validation;
use Symfony\Component\Validator\Mapping\Cache\ApcCache;
use Symfony\Component\Form\Forms;
use Symfony\Component\Form\Extension\HttpFoundation\HttpFoundationExtension;
use Symfony\Component\Form\Extension\Csrf\CsrfExtension;
use Symfony\Component\Form\Extension\Csrf\CsrfProvider\SessionCsrfProvider;
use Symfony\Component\Form\Extension\Templating\TemplatingExtension;
use Symfony\Component\Form\Extension\Validator\ValidatorExtension;
use Symfony\Component\HttpFoundation\Session;
use Symfony\Component\Templating\PhpEngine;
$session = new Session();
$secret = 'V8a5Z97e...';
$csrfProvider = new SessionCsrfProvider($session, $secret);
$engine = new PhpEngine(/* ... snap ... */);
$validator = Validation::createValidator();
// or
$validator = Validation::createValidatorBuilder()
->addXmlMapping('path/to/mapping.xml')
->addYamlMapping('path/to/mapping.yml')
->addMethodMapping('loadValidatorMetadata')
->enableAnnotationMapping()
->setMetadataCache(new ApcCache())
->getValidator();
$formFactory = Forms::createFormFactory();
// or
$formFactory = Forms::createFormFactoryBuilder()
// custom types, if you're too lazy to create an extension :)
->addType(new PersonType())
->addType(new PhoneNumberType())
->addTypeExtension(new FormTypeHelpTextExtension())
// desired extensions (CoreExtension is loaded by default)
->addExtension(new HttpFoundationExtension())
->addExtension(new CsrfExtension($csrfProvider))
->addExtension(new TemplatingExtension($engine, $csrfProvider, array(
'FormBundle:Form'
))
->addExtension(new ValidatorExtension($validator))
->getFormFactory();
$form = $formFactory->createBuilder()
->add('firstName', 'text')
->add('lastName', 'text')
->add('age', 'integer')
->add('gender', 'choice', array(
'choices' => array('m' => 'Male', 'f' => 'Female'),
))
->getForm();
if (isset($_POST[$form->getName()])) {
$form->bind($_POST[$form->getName()]);
if ($form->isValid()) {
// do stuff
}
}
return $engine->render('AcmeHelloBundle:Hello:index.html.php', array(
'form' => $form->createView(),
));
```
---------------------------------------------------------------------------
by bschussek at 2012-07-30T10:08:42Z
I should maybe add a comment about the benefits of this change, in case they are not self-explanatory:
* class construction with default configuration is now a one-liner
* userland code is decoupled from core implementations → userland code doesn't break if we change constructor signatures
* easier to understand, since many core classes are now created internally
* easy to discover the possible settings → just look at (FormFactory|Validator)BuilderInterface
* usage of custom interface implementations is supported, just like before
---------------------------------------------------------------------------
by fabpot at 2012-07-31T08:18:53Z
The new syntax is great.
I have one comment though about this PR about support of PHP as a templating system (support for Twig is provided by the bridge and it was already easy to configure Twig as a templating system for forms -- see Silex for instance).
The `FormHelper` has been moved into the Form component. This helper is only useful when using the PHP templating system (which is not what we recommend people to use), but the default templates are still in the Framework bundle. So using the Form component as standalone with PHP as a templating system still requires to install the bundle to get access to the default templates. Am I missing something? Do we want to move the PHP templates to the Form component too?
---------------------------------------------------------------------------
by stof at 2012-07-31T08:28:28Z
@fabpot it is even worse than that: the FormHelper currently uses the theme by using ``$theme . ':' . $block . '.html.php`` IIRC. This is not compatible with the default template name parser of the component expecting a path. And the FrameworkBundle template name parser does not support accessing a template outside a bundle AFAIK.
So moving the templating to the component would require some refactoring in the FormHelper and the template name parser. However, I think it is worth it. Some people complained that using the form rendering (outside the full-stack framework) was requiring either setting up Twig with the bridge, or adding FrameworkBundle in the project (which means including most of the code of the full-stack framework). Having the Templating rendering in the standalone component could be a great idea
---------------------------------------------------------------------------
by fabpot at 2012-07-31T08:42:53Z
But then, I don't want to promote the Templating component or the PHP templating system. Twig is always a better alternative and this should be what people use most of the time, PHP being the rare exception.
Anyway, we are too close from the first 2.1 RC, so any big refactoring will have to wait for 2.2.
---------------------------------------------------------------------------
by stof at 2012-07-31T09:02:10Z
then maybe we should keep the FormHelper in FrameworkBundle for now as it is tied to the FrameworkBundle template name parser anyway currently.
---------------------------------------------------------------------------
by bschussek at 2012-07-31T14:22:35Z
> it it is even worse than that: the FormHelper currently uses the theme by using ``$theme . ':' . $block . '.html.php`` IIRC. This is not compatible with the default template name parser of the component expecting a path.
This is why the templates are still in FrameworkBundle. I think they should be moved too, but then we have to change
* the default theme to an absolute file path
* the FrameworkBundle name parser to accept absolute paths
I think this can wait until 2.2. Baby steps.
> I don't want to promote the Templating component or the PHP templating system.
We can both promote Twig while making Templating as easy to use as possible. If people want to use Templating, they probably have a reason. We don't have to make their lives more painful than necessary.
Btw: Templating is a *lot* faster for rendering forms than Twig. On Denis' form, Templating takes 1.15 seconds while Twig takes 2.
About moving the helpers, we have two choices:
* Move each helper to the respective component. This would not require new releases of the Templating component when we add more helpers in other component.
* Move all helpers to Templating. This does not make that much sense for Form, as then Form has support for Templating (TemplatingRendererEngine) and Templating has support for Form (FormHelper), which is a bit weird. I personally prefer a stacked architecture, where Templating is at the bottom and Form-agnostic, and Form (or any other component) builds upon that.
I'm fine with both approaches. I'll move FormHelper back to FrameworkBundle, and we can decide for a direction in 2.2.
---------------------------------------------------------------------------
by bschussek at 2012-07-31T14:36:30Z
Done.
Commits
-------
03bbaaf [Routing] Add an interface for configuring strict_parameters
Discussion
----------
[RFC][Routing] Add an interface for configuring strict_parameters
This is a proposal to fix#4697 (related to #4592).
The main point left to discuss was the name of the interface, which is now `LenientInterface`. We could change the name to anything else is someone has a better idea.
@stof @Tobion what do you think ?
---------------------------------------------------------------------------
by stof at 2012-07-30T16:34:20Z
@vicb I already said I had no idea to name it, and it has not changed. :)
So let's wait for other people to see if they have a better idea
---------------------------------------------------------------------------
by breerly at 2012-07-30T16:38:38Z
Maybe `PermissibleInterface` or `PermissiveInterface`.
---------------------------------------------------------------------------
by Partugal at 2012-07-30T17:00:09Z
`StrictUrlGeneratorInterface`, `StrictParametersInterface` or `StrictInterface`
---------------------------------------------------------------------------
by pborreli at 2012-07-30T17:04:46Z
👍 for `PermissiveInterface`
---------------------------------------------------------------------------
by stof at 2012-07-30T17:07:59Z
yes, because the Router currently can only use this interface to set it to ``not-strict``. It assumes that the url generator is already strict by default (which is probably a bad assumption btw as the base class for the generated generator can be changed)
---------------------------------------------------------------------------
by pborreli at 2012-07-30T17:09:33Z
@stof thx, got it
---------------------------------------------------------------------------
by Partugal at 2012-07-30T17:10:03Z
this interface realize setting Strict by setStrictParameters, and get by getStrictParameters, and imho named it by `Strictable` is more logic
---------------------------------------------------------------------------
by pborreli at 2012-07-30T17:11:07Z
@Partugal let's try to find an english term :)
---------------------------------------------------------------------------
by Partugal at 2012-07-30T17:11:31Z
)
---------------------------------------------------------------------------
by breerly at 2012-07-30T17:15:23Z
@Partugal I like using "able" in interface names because it describes a behavior instead of a noun. This type of naming makes following the Interface Segregation Principle easy to follow. Good work.
---------------------------------------------------------------------------
by vicb at 2012-07-30T18:24:26Z
As explained by @stof I did not consider `StrictInterface` because as of now the interface is used to disabled the strict bevahior (which is enabled by default).
I am not satisfied with `PermissiveInterface` / `LenientInterface` because implementing this interface does not mean that the generator will be permissive but only that the behavior is configurable - yes I did consider `Configurable` but the term is a too vague.
---------------------------------------------------------------------------
by breerly at 2012-07-30T18:35:45Z
I see. Perhaps ```StrictConfigurableInterface``` would do the trick.
---------------------------------------------------------------------------
by Tobion at 2012-07-30T21:02:21Z
I think renaming strict_parameters to `strict_requirements` is the way to go because it determines how requirements are handled when generating a URL. Also we should allow another option:
strict_requirements = true: throw exception for mismatching requirements
strict_requirements = null: return null as URL for mismatching requirements and log it.
strict_requirements = false: return the URL with the given parameters without checking the requirements and don't log it.
(Maybe use constants for these).
The Interface I would then call `ConfigurableRequirementsInterface` or `RequirementsHandlingInterface`.
---------------------------------------------------------------------------
by vicb at 2012-07-31T07:23:24Z
Thanks all for the feeback, this is what is now implemented:
- A `ConfigurableRequirementsInterface` that should be implemented by generators that can be configure not to throw an exception when the parameters do not match the requirements,
- The interface has two methods: `setStrictRequirements()` and `isStrictRequirements()`,
- `setStrictRequirements()` always gets called to configure the generator (whatever the option value is)
Note: The Router option name has not changed (i.e. `strict_parameters`)
Does that fit everyone ?
---------------------------------------------------------------------------
by vicb at 2012-07-31T07:39:22Z
So the option name is now consistent (`strict_requirements`) with the interface. We should sync the change [in the standard edition](https://github.com/symfony/symfony-standard/blob/master/app/config/config.yml#L11) if we agree to merge this.
---------------------------------------------------------------------------
by fabpot at 2012-07-31T07:51:47Z
@vicb you forgot to rename the property in `UrlGenerator` as @stof mentioned above.
---------------------------------------------------------------------------
by vicb at 2012-07-31T07:59:57Z
@fabpot fixed. If the code is ok, I'll squash the commits and open a PR on symfony-standard
Commits
-------
a47922b [OptionsResolver] Fix Options::has() when the value is null
Discussion
----------
[OptionsResolver] Fix Options::has() when the value is null
`isset()` would have returned `false` when the value is `null`
Commits
-------
03c3712 [Filesystem] Fixed 2 tests throwing error on windows
3689bb8 [Filesystem] Fixed 3 failing tests on windows
Discussion
----------
[Filesystem] Fixed 5 tests on windows
Fixing 3 test expecting wrong folders :
```
-'C:\Users\pascal\AppData\Local\Temp\\1343425847694\file'
+'C:\Users\pascal\AppData\Local\Temp\1343425847694\file'
```
Fixed 2 tests on Windows caused by symlink function throwing error when first argument is not existent :
```
symlink(): Could not fetch file information(error 2)
```
Commits
-------
f402a16 [FrameworkBundle] AssetsInstallCommand. Made 'web' as a default folder.
Discussion
----------
[FrameworkBundle] AssetsInstallCommand. Made 'web' as a default folder.
Bug fix: no
Feature addition: yes
Backwards compatibility break: not sure
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
License of the code: MIT
>'The target directory (usually "web")'
It is indeed a folder that's usually used to install assets, why not making it as a default value?
Commits
-------
ae6016c [Finder] Workaround for FilterIterator-FilesystemIterator-rewind issue
Discussion
----------
[Finder] Workaround for the problem with rewind of FilterIterator with inner FilesystemIterator.
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #4922
Todo: -
License of the code: MIT
Documentation PR: -
---------------------------------------------------------------------------
by stof at 2012-07-20T10:28:05Z
Please add some tests
---------------------------------------------------------------------------
by alebo at 2012-07-24T09:50:36Z
Any feedback yet? The new commit includes tests.
Commits
-------
4d09907 [Serializer] Add a docblock to help type hinting
Discussion
----------
[Serializer] Add a docblock to help type hinting
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets:
Todo:
License of the code: MIT
Documentation PR:
Commits
-------
9e28593 fixed error on oracle db related to clob data. https://github.com/symfony/symfony/issues/3255
Discussion
----------
fixed error on pdosession storage for oracle db. Related to clob data
Did a change on 2.0 branch to fix this error
https://github.com/symfony/symfony/issues/3255
Tested on mysql and Oracle DB.
I also ran a phpunit test before commit:
phpunit tests/Symfony/Tests/Component/HttpFoundation/SessionStorage/PdoSessionStorageTest.php
PHPUnit 3.6.10 by Sebastian Bergmann.
Time: 0 seconds, Memory: 2.75Mb
OK (3 tests, 5 assertions)
Time: 0 seconds, Memory: 2.75Mb
Commits
-------
d30943c [FrameworkBundle] Switched to parameters for request context host and scheme
Discussion
----------
[FrameworkBundle] Switched to parameters for request context host and scheme
Commits
-------
d858f7b [OptionsResolver] Optimized previous values of a lazy option not to be evaluated if the second argument is not defined
8a338cb [OptionsResolver] Micro-optimization
e659f0e [OptionsResolver] Improved the performance of normalizers
Discussion
----------
[OptionsResolver] Improved the performance of normalizers
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
Normalizers are now stored in the Options instance only once. Previously, normalizers were stored in Options upon resolving, which meant that they were added a lot of time if the same resolver was used for many different options arrays.
This improvement led to an improvement of 30ms on http://advancedform.gpserver.dk/app_dev.php/taxclasses/1
---------------------------------------------------------------------------
by beberlei at 2012-07-26T13:34:23Z
@bschussek do you have the code for this forms somewhere btw?
---------------------------------------------------------------------------
by bschussek at 2012-07-26T13:54:52Z
@beberlei https://github.com/stof/symfony-standard/tree/twig_forms
Normalizers are now stored in the Options instance only once. Previously,
normalizers were stored in Options upon resolving, which meant that
they were added a lot of time if the same resolver was used for many
different options arrays.
This improvement led to an improvement of 30ms on
advancedform.gpserver.dk/app_dev.php/taxclasses/1
Commits
-------
1d6611b [Filesystem] Fixed tests on system without posix
Discussion
----------
[Filesystem] Fixed tests on system without posix
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: master is broken
Fixes the following tickets: None
License of the code: MIT
Commits
-------
fb002d8 [Form] Fixed variable passing from outer to inner blocks of the same FormView instance
Discussion
----------
[Form] Fixed variable passing from outer to inner blocks of the same FormView instance
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #5029
Todo: -
This PR fixes two bugs.
The first bug is described in #5029. The second parameter to the "form_label" function in Twig, if given, always overwrote whatever label was defined previously.
```
{# null would overwrite whatever is currently set #}
form_label(form, null, { ... })
```
The second bug affected passing variables from outer to inner blocks. In the following example, "label_attr" would not be forwarded to the "form_label" function.
```
form_row(form, { "label_attr": { "class": "my_class" }})
```
Both bugs are fixed now.
Commits
-------
0ea3769 Fix not recognized "type" option exception
Discussion
----------
[Form] Fixed not recognized "type" option exception
The exception about not recognized "type" option was raised when "date", "datetime", "time" type was guessed by validator type guesser using the date related constraint.
---------------------------------------------------------------------------
by bschussek at 2012-07-25T11:30:23Z
Thanks! 👍
Commits
-------
686bf6b [Form] Made original data of a form and choices accessible in templates
Discussion
----------
[Form] Made original data of a form and choices accessible in templates
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #4171
Todo: -
Now you can access the normalized data of a form in the template:
```
form.vars.data
```
You can also access the original data of a choice, for example the entities in an entity type:
```
choice.data
```
The exception about not recognized "type" option was raised when "date", "datetime", "time" type was guessed by validator type guesser using the date related constraint.
Commits
-------
307d99c [Security] Fixed use_referer option not working properly when login_path is a route name
Discussion
----------
[Security] Fixed use_referer option not working properly when login_path...
... is a route name
When use_referer is set to true and the request comes from the login page,
the user should not be redirected to the login form again (the referer) but
to the default_target_path. The problem arises when our login_path option
is not a path but a route name, as the ```getUriForPath()``` method is not
made to create routes from route names.
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: [![Build Status](https://secure.travis-ci.org/acasademont/symfony.png?branch=master)](http://travis-ci.org/acasademont/symfony)
Fixes the following tickets: -
Todo: -
License of the code: MIT
Documentation PR: -
---------------------------------------------------------------------------
by stloyd at 2012-07-24T16:24:28Z
👍
When use_referer is set to true and the request comes from the login page,
the user should not be redirected to the login form again (the referer) but
to the default_target_path. The problem arises when our login_path option
is not a path but a route name, as the ```getUriForPath()``` method is not
made to create routes from route names.
Commits
-------
ff273af [HttpFoundation][Sessions] Micro-optimization
9bf3cb4 [HttpFoundation][Sessions] Add support for extended save_path for native files save handler
Discussion
----------
[Sessions] Add support for extended save_path for native files save handler
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #4908
Todo: -
License of the code: MIT
Documentation PR: -
Commits
-------
dd2aa54 [Form] Disabled manual singulars in PropertyPath until the syntax is finalized
Discussion
----------
[Form] Disabled manual singulars in PropertyPath until the syntax is finalized
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
Commits
-------
dc3a680 [Form] Improved FormRenderer API to reduce the size of the function call stack during rendering
Discussion
----------
[Form] Improved FormRenderer API to decrease the function call stack
Bug fix: no
Feature addition: no
Backwards compatibility break: **yes**
Symfony2 tests pass: yes
Fixes the following tickets: #4962, #4973
Todo: -
This PR reduces the function call stack size when rendering by directly calling the methods `renderBlock` and `searchAndRenderBlock` (formerly `renderSection`) and removing the delegating methods `render(Widget|Label|Row|...)`.
It breaks BC in that PHP templates now need to pass the FormView instance to `block` (formerly `renderBlock`). This is necessary, otherwise that function may behave buggy in special circumstances.
Otherwise this PR cleans up API method and parameter names to improve clarity.
Commits
-------
24b764e [Form] Fixed issues mentioned in the PR
9216816 [Form] Turned Twig filters into tests
310f985 [Form] Added a layer of 2.0 BC methods to FormView and updated UPGRADE and CHANGELOG
5984b18 [Form] Precalculated the closure for deciding whether a choice is selected (PHP +30ms, Twig +30ms)
5dc3c39 [Form] Moved the access to templating helpers out of the choice loop for performance reasons (PHP +100ms)
0ef9acb [Form] Moved the method isChoiceSelected() to the ChoiceView class (PHP +150ms)
8b72766 [Form] Tweaked the generation of option tags for performance (PHP +200ms, Twig +50ms)
400c95b [Form] Replace methods in ChoiceView by public properties (PHP +100ms, Twig +400ms)
d072f35 [Form] The properties of FormView are now accessed directly in order to increase performance (PHP +200ms, Twig +150ms)
Discussion
----------
[Form] Made FormView and ChoiceView properties public for performance reasons
Bug fix: no
Feature addition: no
Backwards compatibility break: **yes**
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
This PR changes the access to properties of `FormView` and `ChoiceView` objects from getters to direct property accesses. On [my example form](http://advancedform.gpserver.dk/app_dev.php/taxclasses/1) this improves rendering performance for **300ms** with PHP templates and **550ms** with Twig on my local machine.
Unfortunately, this breaks BC both with 2.0 and with the current master in Form Types and PHP templates. Twig templates are not affected by this change.
2.0:
```
$formView->set('my_var', 'foobar');
$formView->get('my_var');
$formView->getChild('childName');
$formView['childName'];
```
master:
```
$formView->setVar('my_var', 'foobar');
$formView->getVar('my_var');
$formView->get('childName');
$formView['childName'];
```
this PR:
```
$formView->vars['my_var'] = 'foobar';
$formView->vars['my_var'];
$formView->children['childName'];
$formView['childName'];
```
Should we add methods to keep BC with 2.0?
The second part of this PR contains improvements to the rendering of choice fields. These gain another **~500ms** for PHP templates and **80ms** for Twig. These improvements are BC, unless you overwrote the block "choice_widget_options" in your form themes which then needs to be adapted.
**Update:**
The PR now includes a BC layer for 2.0.
---------------------------------------------------------------------------
by stof at 2012-07-21T11:37:41Z
@bschussek couldn't we keep the getters and setters for BC even if the rendering accesses the public properties directly ?
---------------------------------------------------------------------------
by bschussek at 2012-07-21T11:52:33Z
@stof A BC layer for 2.0 is now included. People who upgraded to master already unfortunately need to adapt their code.
---------------------------------------------------------------------------
by sstok at 2012-07-21T12:40:57Z
👍
Commits
-------
134cc84 [Security] Fix DocBlock of attemptAuthentication
Discussion
----------
[Security] Fix DocBlock of attemptAuthentication
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets:
Todo: -
License of the code: MIT
Documentation PR: -
Commits
-------
9dc2011 Late static factory method
Discussion
----------
Late static factory method
When using `Symfony\CS\Finder\DefaultFinder::create()`, we lose all `Symfony\CS\Finder\DefaultFinder::__construct()` properties because main `Finder` does not use late static binding.
This commit resolves the issue.
Commits
-------
d4f4038 [Form] Reduced the number of setData() calls by deferring a Form's initialization (+40ms)
Discussion
----------
[Form] Reduced the number of setData() calls
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
This PR decreases the number of expensive `setData()` calls on `Form` instances by deferring the form's initialization with default data to the first call to a `get*Data()` method. If `setData()` is called manually before invoking `get*Data()`, the initialization with the default data will not take place.
Before:
```
$form = new Form($config); // implicit setData($config->getData());
$form->setData($object); // setData() is now called twice
```
After:
```
$form = new Form($config); // no implicit setData()
$form->getData(); // implicit setData($config->getData())
// or
$form = new Form($config);
$form->setData($object);
$form->getData(); // setData() was called only once
```
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
-------
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
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
-------
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).
Commits
-------
610c602 [OptionsResolver] Slightly tweaked the performance of the Options class
Discussion
----------
[OptionsResolver] Slightly tweaked the performance of the Options class
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
Commits
-------
0d0a968 [Templating] Cached the result of escape() in order to improve performance (+470ms)
Discussion
----------
[Templating] Cached the result of escape() in order to improve performance
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
This improvement gains **400ms** of rendering speed on [this particular example page](http://advancedform.gpserver.dk/app_dev.php/taxclasses/1).
---------------------------------------------------------------------------
by lsmith77 at 2012-07-16T17:36:50Z
i guess we don't have to be concerned with increased memory usage here .. if at all we could offer a clear cache method in case someone is f.e. using this to generate tons of messages in a cron job.
---------------------------------------------------------------------------
by henrikbjorn at 2012-07-17T06:39:52Z
The example form is broken.
---------------------------------------------------------------------------
by bschussek at 2012-07-17T07:21:26Z
The source code for the form can be found [here](https://github.com/stof/symfony-standard/blob/twig_forms/src/AdvancedForm/CoreBundle/Form/TaxClassType.php).
---------------------------------------------------------------------------
by henrikbjorn at 2012-07-17T07:28:11Z
But i am guessing this is only for php not twig :P
---------------------------------------------------------------------------
by bschussek at 2012-07-17T07:41:07Z
Obviously..
Commits
-------
8f99be3 [DoctrineBridge] Fixed the type guesser for doctrine 2.3
Discussion
----------
[DoctrineBridge] Fixed the type guesser for doctrine 2.3
Doctrine 2.3 now uses the drivers moved to Common, so the exception was not catched anymore and was breaking the guessing when a non-entity was used.
---------------------------------------------------------------------------
by craue at 2012-07-16T14:54:30Z
👍
---------------------------------------------------------------------------
by ddeboer at 2012-07-17T20:07:57Z
👍
---------------------------------------------------------------------------
by stof at 2012-07-17T20:17:01Z
@fabpot please merge this as 2.1 is currently broken when you rely on the form guessers for unmapped classes
Commits
-------
17ca9b6 [Form] Fixed DoctrineType to use getManagerForClass() if no EM name is given
Discussion
----------
[Form] Fixed DoctrineType to use getManagerForClass() if no EM name is given
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #4125
Todo: -
---------------------------------------------------------------------------
by stof at 2012-07-17T08:16:59Z
👍
Commits
-------
b4d1bdf [Form] added a bc break note about the tag alias matching
Discussion
----------
[Form] added a bc break note about the tag alias matching
6489a65960 is a BC break if we were relying on the previous behavior.
Commits
-------
1f2f866 fixed the serialization of the SwitchUserRole
b55930a [Security] Implemented the Serializable interface in the Role class
Discussion
----------
[Security] Implemented the Serializable interface in the Role class
The Role class is serialized in the session for each role of the user. Implementing the Serializable interface allows to reduce the size of the data.
Commits
-------
df2406f [Security] Add note to changelog about BC break
01b2e39 [Security] Extract default logout success handling logic
Discussion
----------
[Security] Extract default logout success handling logic
Bug fix: no
Feature addition: no
Backwards compatibility break: yes, small one for people using the component
Symfony2 tests pass: [![Build Status](https://secure.travis-ci.org/asm89/symfony.png?branch=default-logout-success-handler)](http://travis-ci.org/asm89/symfony)
License of the code: MIT
As discussed earlier with @fabpot and @schmittjoh. This PR extracts the default logout success handling logic to a separate class that users can extend.
Note: build status is red, but that is because of a failing performance test in the form component? ..
Commits
-------
7d53909 Earlier PHP output buffer flush for non FPM environments
Discussion
----------
Earlier PHP output buffer flush for non FPM environments
In the Response::send() method you are calling the fastcgi_finish_request() in case it exists. This will provide a respectful performance boost when you have significant work being done by listeners acting on kernel terminal events; Sadly you are forgetting people that don't use FPM doing this.
The performance boost for a Vanilla PHP is not much: flushing earlier potentially helps higher layers such as the HTTPd or potential other cache layers: the sooner their buffer gets filled, the sooner they release information to the browser, even if the output buffer is still open. The explicit flush() is supposed to do exactly this.
Commits
-------
33f29ed [Form] '@group benchmark' for form performance tests
Discussion
----------
[Form] '@group benchmark' for form performance tests
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: [![Build Status](https://secure.travis-ci.org/asm89/symfony.png?branch=form-performance)](http://travis-ci.org/asm89/symfony)
License of the code: MIT
I think a PR or note about this has been rejected before, but since build statuses on PRs sometimes seem to fail if travis is busy I think moving the form performance tests to `@group benchmark` should be reconsidered.
Edit: even master is currently failing on this
Commits
-------
07992d3 [Validator] Added inheritDoc phpdoc for validate methods
Discussion
----------
[Validator] Added inheritDoc phpdoc for validate methods
Was instructed by @stof to do this for a PR on comparison validators and noticed none of the validators used inheritDoc.
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: n/a
Todo: I haven't looked around too much, but I assume if none of the validators followed this standard that there would be a fair few other classes not using. Obviously not a big issue though
License of the code: MIT
Documentation PR: n/a
Commits
-------
5ae0da0 Update changelogs
ff91b9a [FrameworkBundle] Make FlashBag the default.
Discussion
----------
[2.1][FrameworkBundle] Make FlashBag the default.
Bug fix: no
Feature addition: yes
Backwards compatibility break: yes (but only technically)
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
License of the code: MIT
The difference between `AutoExpireFlashBag` and `FlashBag` is simply that the first will expire flashes regardless of being displayed on the next pageload. This can result in lost messages. It was created simply for BC with 2.0.
`FlashBag` expires flashes once they are retrieved. This also makes it ESI compatible.
/cc @lsmith77
---------------------------------------------------------------------------
by jalliot at 2012-07-14T18:13:40Z
+1!
You should add it to the changelog and upgrade files though :)
Commits
-------
480ab14 Further improving the MessageSelector exception
Discussion
----------
Further improving the MessageSelector exception
Hey guys!
The goal is just to give the users a better starting point when they see this exception.
See previous change in #4173 and conversation in #4207
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #4207
Todo: -
License of the code: MIT
Documentation PR: n/a
Thanks!
---------------------------------------------------------------------------
by stof at 2012-07-14T23:11:17Z
Shouldn't it be done in 2.0 instead ?
---------------------------------------------------------------------------
by weaverryan at 2012-07-15T00:15:42Z
I decided to go against 2.1 when I saw that #4173 was against 2.0. If we do it against 2.0, it'll cause a conflict when 2.0 is merged into master - seemed like too much trouble for such a small change.
Commits
-------
12bdec3 Moved the NormalizationAwareInterface check to the ChainEncoder
28e137c [Serializer] Added a ChainEncoder and a ChainDecoder
Discussion
----------
[Serializer] Added a ChainEncoder and a ChainDecoder
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: [![Build Status](https://secure.travis-ci.org/stof/symfony.png?branch=serializer_improvement)](http://travis-ci.org/stof/symfony)
Fixes the following tickets: -
Todo: -
These classes contains the logic previously defined in the Serializer itself to handle the choice of a serializer. This allows reusing it when using only the encoding part of the component, without having to use the Serializer class (which is not as handy to bootstrap when you want to use only encoders and decoders as normalizers come first)
I was wondering if these classes should have adders but I kept the constructor injection only to be consistent with the current code (encoders cannot be registered after the instantiation) and to avoid implementing the SerializerAwareInterface in them (to allow injecting the Serializer in serializer-aware encoders and decoders added later).
Note that this change is fully BC as it only changes the internal implementation of the Serializer.
---------------------------------------------------------------------------
by fabpot at 2012-07-14T11:07:32Z
ping @lsmith77 @Seldaek
---------------------------------------------------------------------------
by Seldaek at 2012-07-14T15:17:42Z
After a quick look, I'd say +1
Commits
-------
dbd169f [Form] Error in the SimpleFormTest case.
Discussion
----------
[Form] Error in the SimpleFormTest case.
Symfony2 tests pass: yes
---------------------------------------------------------------------------
by bschussek at 2012-07-14T13:25:28Z
Thanks, looks like a copy paste error. @fabpot 👍
Commits
-------
6489a65 [Form] Added an exception for invalid type services
Discussion
----------
[Form] Added an exception for invalid type services
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes [![Build Status](https://secure.travis-ci.org/stof/symfony.png?branch=form_safeguard)](http://travis-ci.org/stof/symfony) (Travis fails randomly for the performance test)
Fixes the following tickets: -
Todo: -
Before the introduction of the FormRegistry, the getName() method was
never used for types registered through the DI container. The
FormRegistry now uses the getName() method and missconfigured services
will trigger a notice.
This was reported in FriendsOfSymfony/FOSCommentBundle#234
Before the introduction of the FormRegistry, the getName() method was
never used for types registered through the DI container. The
FormRegistry now uses the getName() method and missconfigured services
will trigger a notice.
This was reported in FriendsOfSymfony/FOSCommentBundle#234
These classes contains the logic previously defined in the Serializer
itself to handle the choice of a serializer. This allows reusing it when
using only the encoding part of the component.
Commits
-------
b3958af [Validator] [Resources] fixed french validator translated messages for the french locale.
Discussion
----------
[Validator] [Resources] fixed french validator translated messages for the french locale
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #4900
Commits
-------
77b4349 [WebProfiler] Improved logger panel
Discussion
----------
[WebProfiler] Improved logger panel
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
* Add missing priority levels from filter
* Display priority level in front of each log entry
* Add a yellow background for warnings and use the red one for errors and above (previously only errors)
* Display `No logs available for this priority.` when filter is too restrictive
Commits
-------
23d8735 Added NativeFileSessionHandler to classes to compile .
12d6ae7 Removed FileSessionHandler from FrameworkExtension to stop compiling a file that does not exist.
Discussion
----------
[FrameworkBundle] Removed FileSessionHandler from FrameworkExtension to stop compiling a file that does not exist.
PR #4899 removed FileSessionHandler which caused a class not found error from FrameworkBundle after the cache was created. This PR will fix it.
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Todo: -
License of the code: MIT
---------------------------------------------------------------------------
by stof at 2012-07-13T19:12:51Z
you should add the NativeSessionHandler class in the list instead as it replaces it
---------------------------------------------------------------------------
by tystr at 2012-07-13T19:14:29Z
+1
---------------------------------------------------------------------------
by zachbadgett at 2012-07-13T19:15:55Z
Done
Commits
-------
cd7835d [Form] Cached the form type hierarchy in order to improve performance
2ca753b [Form] Fixed choice list hashing in DoctrineType
2bf4d6c [Form] Fixed FormFactory not to set "data" option if not explicitely given
7149d26 [Form] Removed invalid PHPDoc text
Discussion
----------
[Form] WIP Improved performance of form building
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: **Update the Silex extension**
This PR is work in progress and up for discussion. It increases the performance of FormFactory::createForm() on a specific, heavy-weight form from **0.848** to **0.580** seconds.
Before, the FormFactory had to traverse the hierarchy and calculate the default options of each FormType everytime a form was created of that type.
Now, FormTypes are wrapped within instances of a new class `ResolvedFormType`, which caches the parent type, the type's extensions and its default options.
The updated responsibilities: `FormFactory` is a registry and proxy for `ResolvedFormType` objects, `FormType` specifies how a form can be built on a specific layer of the type hierarchy (e.g. "form", or "date", etc.) and `ResolvedFormType` *does the actual building* across all layers of the hierarchy (by delegating to the parent type, which delegates to its parent type etc.).
---------------------------------------------------------------------------
by schmittjoh at 2012-07-12T18:25:40Z
Maybe ResolvedFormType
---------------------------------------------------------------------------
by jmather at 2012-07-13T02:56:38Z
I really like ResolvedFormType. That's the naming method I took for my tag parser that handes the same conceptual issue.
---------------------------------------------------------------------------
by axelarge at 2012-07-13T05:25:00Z
ResolvedFormType sounds very clear.
This change is great and I desperately hope to see more of this kind
---------------------------------------------------------------------------
by Baachi at 2012-07-13T06:41:26Z
Yes `ResolvedFormType` sounds good :) 👍
---------------------------------------------------------------------------
by fabpot at 2012-07-13T07:11:33Z
I like `ResolvedFormType` as well.
---------------------------------------------------------------------------
by henrikbjorn at 2012-07-13T07:46:48Z
👍 `ResolvedFormType` :shipit:
---------------------------------------------------------------------------
by stof at 2012-07-13T18:01:51Z
This looks good to me