Commits
-------
fabe818 [EventDispatcher] Add reference to the EventDispatcher on the Event
Discussion
----------
[EventDispatcher] Add reference to the EventDispatcher on the Event
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
I don't like registering event listeners unless they are really used, it seems wasteful. So I tend to register listeners for the response event in other listeners, only when they will be required. @stof has [brought to my attention](fb243ace83 (commitcomment-696467)) that this may cause issues in Silex or any other situation where event listeners are not lazy loaded, since it creates a circular reference in that case.
With this PR, avoiding the circular reference is possible, without bloating the response listener with unnecessary "do I need to do anything?" code.
---------------------------------------------------------------------------
by schmittjoh at 2011/11/06 05:28:39 -0800
Did you do any benchmarks? It's just a feeling, but registering a listener at runtime might be more expensive than just having it always executed.
Also, I find these dynamic listeners a bit of a code smell. They are not easily testable, and the control flow is harder to track. Besides, you do not take into account subrequests which might happen in between.
---------------------------------------------------------------------------
by Seldaek at 2011/11/06 09:34:27 -0800
I don't see why it would be slower, if it's a commonly fired event yes you blast away the `sorted` listener cache every time you add one, but most of the time those optional listeners are for the response, which is typically not sorted yet when you add the listener, so I don't think there is any overhead.
As for the code smell, of course it's a matter of preference, but I have the opposite view on control flow, I find it weird that listeners are registered when they are not used in the end, while doing it my way I think it's more clear what happens.
For sub-requests, I'm not sure what you mean. In this instance, and in most response listeners I have seen, the sub-requests are always ignored anyway by the listener.
---------------------------------------------------------------------------
by drak at 2011/11/10 06:07:45 -0800
I don't see how loading up the dispatcher with a bunch of callables can be expensive - it's just loading an array basically.
Wouldn't it be better to have a separate `DispatcherAwareEvent extends Event`
class DispatcherAwareEvent extends Event
{
protected $dispatcher;
public function setDispatcher(EventDispatcher $dispatcher)
{
$this->dispatcher = $dispatcher;
}
public function getDispatcher()
{
return $this->dispatcher;
}
This can then be used as a base class for what you need `MyEvent extends DispatcherAwareEvent`
$event = new MyEvent($dispatcher, $foo);
$dispatcher->dispatch($event);
---------------------------------------------------------------------------
by Seldaek at 2011/11/10 06:18:57 -0800
@drak: Every event is part the event dispatching system and therefore should be aware of the dispatcher imo. It's not like the ContainerAwareInterface which is gluing things that do not especially have to know about the DIC together.
If we do that, then we have to start arguing every time we need the dispatcher in a given event, because the original author did not think it was necessary, and then that will only make it into the next minor version, etc. Not fun at all.
---------------------------------------------------------------------------
by drak at 2011/11/10 06:36:26 -0800
By the way, the event dispatchers looks to be pretty well optimized given the fact that it only sorts listeners if they are called, and then only once.
---------------------------------------------------------------------------
by drak at 2011/11/10 12:33:28 -0800
It just seems weird. I mean, following on, why isn't the event name a compulsory parameter also? - again, you can say both ways, if you need it, make it part of your custom Event class, or since it's a required param to be able to dispatch an event in the first place, make it part of the base Event class. All I'm saying it it seems suspicious when it could be achieved a different way.
For example, you could inject the dispatcher into the listener itself and then the event handler could access the dispatcher if it needs:
class MyListener
{
public function __construct(EventDispatcher $eventDispatcher)
{
//...
}
public function someListener(Event $event)
{
//...
}
}
---------------------------------------------------------------------------
by stof at 2011/11/10 15:20:07 -0800
@drak The issue when injecting the dispatcher in the listener is described in the issue: circular dependency: you need to create the dispatcher before the listener (as it is injected in it) and when the listener is not lazy-loaded (in Silex for instance), you need to create it before the dispatcher.
---------------------------------------------------------------------------
by drak at 2011/11/10 21:15:45 -0800
Indeed, although it might not unreasonable to expect to create the dispatcher first... but anyway I'm convinced!
Injecting the dispatcher could lead to some __very interesting possibilities__ as standard. While we are at it though, we should have a getter and setter for `$name` in the `Event` class and `$event->setName($eventname)` in the `dispatch()` method. Allowing an event to know it's name is very useful. It allows a single listener to be registered for multiple names, and even makes the event object reusable. I don't know why $name was removed, it was in the Symfony 1 dispatcher and while the new dispatcher is brilliant from an OO point of view, missing the name as standard is a big shame.
+1 from me.
* 2.0:
[HttpKernel] fixed Content-Length header when using ESI tags (closes#2623)
[HttpFoundation] added an exception to MimeTypeGuesser::guess() when no guesser are available (closes#2636)
[Security] fixed HttpUtils::checkRequestPath() to not catch all exceptions (closes#2637)
[DoctrineBundle] added missing default parameters, needed to setup and use DBAL without ORM
[Transation] Fix grammar.
[TwigBundle] Fix trace to not show 'in at line' when file/line are empty.
* 2.0:
[Form] fixed previous merge
[Form] simplified previous merge
Also identify FirePHP by the X-FirePHP-Version header
[TwigBundle] Extract output buffer cleaning to method
[TwigBundle] Do not clean output buffering below initial level
Fixed rendering of FileType (value is not a valid attribute for input[type=file])
Added tests for string fix in DateTimeToArrayTransformer (8351a11286).
Added check for array fields to be integers in reverseTransform method. This prevents checkdate from getting strings as arguments and throwing incorrect ErrorException when submitting form with malformed (string) data in, for example, Date field. #2609
[Translation] removed unneeded methods
[Translation] added detection for circular references when adding a fallback catalogue
[DomCrawler] trim URI in getURI
[Yaml][Tests] Fixed missing locale string for Windows platforms which caused test to fail
Commits
-------
e83e00a Fixed rendering of FileType (value is not a valid attribute for input[type=file])
Discussion
----------
Fixed rendering of FileType
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
According to the W3C validator, `value` is not a valid attribute for `input[type=file]`.
---------------------------------------------------------------------------
by fabpot at 2011/11/10 23:01:24 -0800
Instead of creating yet another block, what about modifying the `field_widget` to not render the `value` attribute if the value is empty? Also, the PHP template must be fixed too.
---------------------------------------------------------------------------
by jalliot at 2011/11/11 02:02:52 -0800
@fabpot Changed ;)
Commits
-------
2582fcb Added tests for string fix in DateTimeToArrayTransformer (8351a11286).
8351a11 Added check for array fields to be integers in reverseTransform method. This prevents checkdate from getting strings as arguments and throwing incorrect ErrorException when submitting form with malformed (string) data in, for example, Date field. #2609
Discussion
----------
Fix for #2609
Second take for fix for #2609, hope it's ok now. Tests are failing without my fix and passing with it.
Commits
-------
a245e15 [DomCrawler] trim URI in getURI
Discussion
----------
[DomCrawler] trim URI in getURI
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #2599
Commits
-------
d974a4a Merge pull request #4 from stealth35/test_mo_loader
cf05646 delete useless tests
19f9de9 [Translation] fix gettext tests
965f2bf Merge pull request #3 from stealth35/test_mo_loader
9c2a26d [Translation] add Mo loader tests
9af2342 [Translation] Added the gettext loaders
Discussion
----------
[Translation] Added the gettext loaders
This is the squashed version of the work done by @xaav in #634.
@stealth35 you said you will work on the dumpers. do you have some stuff on it ?
---------------------------------------------------------------------------
by drak at 2011/10/24 19:28:43 -0700
Is there any more progress with this?
---------------------------------------------------------------------------
by stealth35 at 2011/10/25 00:57:19 -0700
I work on the dumpers, but the Po loader is wrong, caus' the Po ressource can be multiline,
msgid ""
"Here is an example of how one might continue a very long string\n"
"for the common case the string represents multi-line output.\n"
http://www.gnu.org/software/gettext/manual/gettext.html#PO-Files
Anyway the Po format is an intermediate format to Mo file, (like .txt to .res file for ICU), IMO we can just support the real gettext format : Mo
---------------------------------------------------------------------------
by stealth35 at 2011/11/03 02:00:24 -0700
@stof The MO Dumper is ready (stealth35/symfony@f2d1d5b4de), should we keep the PO format ?
---------------------------------------------------------------------------
by fabpot at 2011/11/07 08:50:59 -0800
@stealth35: The PO is what people will use for their translations. They will then dump it to MO. So, we need both PO and MO loaders and dumpers.
---------------------------------------------------------------------------
by stealth35 at 2011/11/08 01:25:39 -0800
@fabpot, I'm ready for both dumpers, you can merge this, and I'll open a PR for the dumpers
---------------------------------------------------------------------------
by fabpot at 2011/11/08 22:37:47 -0800
I've just had a look at this PR code again and I see that the unit tests are pretty slim. Is it possible to add some tests for the mo loader?
---------------------------------------------------------------------------
by stealth35 at 2011/11/09 01:15:25 -0800
@fabpot test send to @stof ✌️
---------------------------------------------------------------------------
by stof at 2011/11/09 02:22:55 -0800
and merged in this branch
---------------------------------------------------------------------------
by fabpot at 2011/11/09 02:39:09 -0800
The tests do not pass for me:
There was 1 error:
1) Symfony\Tests\Component\Translation\Loader\MoFileLoaderTest::testLoadDoesNothingIfEmpty
InvalidArgumentException: MO stream content has an invalid format.
/Users/fabien/work/symfony/git/symfony/src/Symfony/Component/Translation/Loader/MoFileLoader.php:79
/Users/fabien/work/symfony/git/symfony/src/Symfony/Component/Translation/Loader/MoFileLoader.php:46
/Users/fabien/work/symfony/git/symfony/tests/Symfony/Tests/Component/Translation/Loader/MoFileLoaderTest.php:34
--
There was 1 failure:
1) Symfony\Tests\Component\Translation\Loader\PoFileLoaderTest::testLoad
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
Array (
- 'foo' => 'bar'
)
/Users/fabien/work/symfony/git/symfony/tests/Symfony/Tests/Component/Translation/Loader/PoFileLoaderTest.php:25
Commits
-------
d08ec5e Add DelegatingValidator tests
e1822e7 Enable dynamic set of validation groups by a callback or Closure
Discussion
----------
[Form][Validator] Enable dynamic set of validation groups based on callback
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Symfony2 tests written for new feature: yes
closes tickets: #2498#1151
This will allow developer to pass a Closure or a callback array as a Validation groups option of a form. Eg:
```
class ClientType extends AbstarctType
{
// ...
public function getDefaultOptions(array $options)
{
return array(
'validation_groups' => function(FormInterface $form){
// return array of validation groups based on submitted user data (data is after transform)
$data = $form->getData();
if($data->getType() == Entity\Client::TYPE_PERSON)
return array('Default', 'person');
else
return array('Default', 'company');
},
);
}
// ...
}
```
```
class ClientType extends AbstarctType
{
// ...
public function getDefaultOptions(array $options)
{
return array(
'validation_groups' => array(
'Acme\\AcmeBundle\\Entity\\Client',
'determineValidationGroups'
),
);
}
// ...
}
```
This will make developers life easier !
---------------------------------------------------------------------------
by schmittjoh at 2011/10/27 06:39:56 -0700
Does that work if your ClientType were added to another form type?
e.g.
```php
<?php
class MyComplexType extends AbstractType
{
public function buildForm(FormBuilder $builder, array $options)
{
$builder->add('client', new ClientType());
}
// ...
}
```
---------------------------------------------------------------------------
by canni at 2011/10/27 06:44:33 -0700
This is doing nothing more than injecting array of validation groups, should work, but I have not tested this use case.
---------------------------------------------------------------------------
by canni at 2011/10/28 01:58:26 -0700
PHPUnit output
```
OK, but incomplete or skipped tests!
Tests: 5011, Assertions: 12356, Incomplete: 36, Skipped: 32.
```
---------------------------------------------------------------------------
by canni at 2011/11/02 11:37:47 -0700
Now functionality is complete, test are written, and implementation is clean. :)
---------------------------------------------------------------------------
by stloyd at 2011/11/02 11:50:44 -0700
Can tou `squash` your commits ? Thanks.
---------------------------------------------------------------------------
by canni at 2011/11/02 11:58:41 -0700
Done
---------------------------------------------------------------------------
by fabpot at 2011/11/07 07:51:18 -0800
Can you add some tests for the `DelegatingValidator` class, which is where we can ensure that the new feature actually works as expected?
---------------------------------------------------------------------------
by canni at 2011/11/07 13:53:16 -0800
OK, I've written proof-of-concept tests, also I've squashed few commits to make things clear.
Personally I think this should go straight into 2.0 series, as it do not beak BC, and a feature is really nice to use.
---------------------------------------------------------------------------
by stof at 2011/11/07 14:17:15 -0800
@canni the 2.0 branch is for bug fixes, not for new features. This is the difference between maintenance releases and minor releases.
Commits
-------
7346896 Changed Serialized#supportsNormalization to PRIVATE
e851efc Updated SerializerTest with "normalizeTraversable" & "testNormalizeGivesPriorityToInterfaceOverTraversable"
d789f94 Serializer#normalize gives precedence to objects that support normalization
9e6ba9a Added protected Serializer#supportsNormalization
Discussion
----------
[Serializer] `normalize` should use supported normalizer before Traversable
Bug fix: yes
Feature addition: no
Backwards compatibility break: no (discussion needed)
Symfony2 tests pass: yes
Fixes the following tickets: #2295
**Same as PR #2539, except rebased onto `2.0`**
Should I abstract out a `supportsDenormalization` function just for symmetry?
Commits
-------
ab9caa0 [Security] Check for request's session before attempting writes.
dabff0e [Security] Support removing tokens from a session.
Discussion
----------
[Security] Support removing tokens from a session.
Currently there is no way to remove a session's security token without invalidating the entire session and all its data (the ContextListener will only update the session if a token is non-null and non-anonymous). This patch fixes that.
I consider this a bug and I found no tests to prove otherwise. Let me know if I'm mistaken. Originally mentioned at https://groups.google.com/d/topic/symfony-devs/ojLvh0WUbfo/discussion
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
---------------------------------------------------------------------------
by ms937 at 2011/10/24 05:19:21 -0700
This change looks good to me. In fact I'm using similar patch in my app and it works as intended. Also, several other people requested this on the mailing list. Could someone from Symfony team merge this? Thanks.
This will enable developer, to set a callback or a closure as a `'validation_groups'` form option,
this is usefull when we have to determine validation groups based on a client submitted data.
Commits
-------
fbd2a0e make suggested changes for default value
c507b1d update variable name to match the option name
b53f000 add the ability to set the form prototype name in CollectionType. this will aid in handling nested collections in forms
Discussion
----------
[Form] Collection
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: [#1324](https://github.com/symfony/symfony/issues/1324)
add the ability to set the form prototype name in CollectionType. this will aid in handling nested collections in forms
---------------------------------------------------------------------------
by IamPersistent at 2011/10/17 02:54:45 -0700
Actually, as an afterthought, I looked over at the issues and this is basically https://github.com/symfony/symfony/issues/1324 just adding an extra option instead of using the prototype option.
@stloyd, my thought was handling the case where someone was "clever" enough to set the 'prototype_name' option to "". But I could be over-thinking :)
---------------------------------------------------------------------------
by stloyd at 2011/10/17 03:00:23 -0700
@IamPersistent IMO if someone is setting this option, he should be aware of that problem, also AFAIK `$$$$` could be *valid* name too ;-)
---------------------------------------------------------------------------
by IamPersistent at 2011/10/17 03:02:14 -0700
@stloyd, I'm fine with changing it, I'll wait to see what everyone else has to say about this request vs using the prototype option for the name in 1324
---------------------------------------------------------------------------
by IamPersistent at 2011/10/17 03:28:49 -0700
@stloyd, @stof, I made the suggested changes, now I suppose, let the debate begin over PR1324 vs this
---------------------------------------------------------------------------
by stloyd at 2011/10/17 03:47:53 -0700
IMO this PR makes changing prototype name in more clean way, so I would prefer this one over that proposed in #1324.
Commits
-------
89cd64a Set error code when number cannot be parsed. Fixes#2389
Discussion
----------
Set error code when number cannot be parsed in StubNumberFormatter
The stub implementation of NumberFormatter never sets an error code and instead always returns the "no-error" code. This causes unexpected results when a transformer gives it bad arguments, such as transforming the input value to boolean false and causing exceptions further down the line, as in #2389.
Instead, it should set an error code when appropriate and return it when requested so that a TransformationFailedException can be raised and the input value left unaltered.
There may be other instances where an error should be set. This covers the common use case of non-numeric input.
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #2389
Commits
-------
c9d05d7 Let NumberFormatter handle integer type casting
Discussion
----------
Let NumberFormatter handle integer type casting
The integer to localised string transformer is currently casting everything it gets to an integer, even if it is not a number. This responsibility should be passed off to NumberFormatter.
Partially addresses #2389 by not mistakenly typecasting a boolean false into an integer 0
---------------------------------------------------------------------------
by mrtorrent at 2011/10/30 15:04:28 -0700
Apologies, forgot the template:
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #2389 (partial)
Commits
-------
05a4e9d [Validators][Type] Added support for ctype_* functions + tests
Discussion
----------
[Validators][Type] Added support for ctype_* functions + tests
Bug fix: no
Feature addition: yes
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: -
Not sure if the ctype_* functions support should be included in the AssertTypeValidator or in a dedicated one.
What do you think ?
Commits
-------
6cb7acf CS - camelCase & curly braces
d9b7abb Added EntityChoiceList test for `group_by` and invalid, deep property paths
e6554d6 Removed Closure support from group_by (PropertyPath strings only)
037933a CS - (String) renamed to (string)
7ad0f05 Added group_by test for EntityType
882482a Added group_by tests for EntityChoiceList
040e988 `EntityChoiceList` now supports grouping of entities by property path or closure
b171a6a Added `group_by` to EntityType
Discussion
----------
[Doctrine] [Form] EntityType+EntityChoiceList supports grouping choices
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #1735
Per the discussion in #1735, `EntityType` does not immediately support grouping options, though I updated support for it in `EntityChoiceList` in fb9d951b1d.
This PR accomplishes the following:
* Adds optional `group_by` property to `EntityType` that supports either a `PropertyPath` or a `\Closure` that is evaluated on the entity choices
* Support for groups is added via the constructor in `EntityChoiceList`
* Groups are created prior to `EntityChoiceList#loadEntities` via a new `groupEntities` function
* Added tests for `EntityChoiceList`
* Added test for `EntityType` `group_by` support
*There is an alternative version that only modifies `EntityType`, but that requires the addition of `EntityType#buildView(...)`, which is messy, IMO: https://github.com/ericclemmons/symfony/compare/master...1735-entity_type_group_by*
---------------------------------------------------------------------------
by fabpot at 2011/10/25 01:48:23 -0700
ping @beberlei
---------------------------------------------------------------------------
by beberlei at 2011/10/25 03:06:05 -0700
I didnt run the tests, but generally this looks very good and is a good extension.
---------------------------------------------------------------------------
by beberlei at 2011/11/01 06:25:09 -0700
@fabpot i revewied this and it looks very good, tests all pass, i think this is a very nice addition.
While Definition::getArgument() could be used to fetch replaced values, it relied upon bad comparison logic (e.g. "index_1" > 1). Additionally, storing original arguments and replacements in the same array makes Definition::getArguments()'s bounds check unreliable. A single argument and its replacement would count twice, allowing getArgument(2) to pass the bounds check and result in an array index error.
With this new method, fetching of replacement arguments is more straightforward and bounds checking functions as it should.
Commits
-------
fee217f Update tests/Symfony/Tests/Component/Process/ProcessTest.php
d58e682 GH-2287 - Skip Process:processTest on Windows due to PHP bug #60120
Discussion
----------
[Process] Test with pipes hang on windows
Rebased onto 2.0
This is a common case with optional Entity associations:
$user->getGroup()->getName()
The property path `group.name` (traversing from the User Entity), when
the User is not assigned to a Group, will throw an UnexpectedTypeException.
This is caught so that the User can still be listed as an option, just without
any nesting.
Commits
-------
d3f137b cosmetic tweak
2877883 anything in front of ;q= is part of the mime type, anything after may be ignored
Discussion
----------
[HttpFoundation] fix splitHttpAcceptHeader() parsing of parameters
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
anything in front of ;q= is part of the mime type, anything after may be ignored
see http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.1
---------------------------------------------------------------------------
by lsmith77 at 2011/10/09 04:00:12 -0700
i must admit .. i am not 100% that my implemention is correct either .. but i am sure the current one isn't.
---------------------------------------------------------------------------
by lsmith77 at 2011/10/09 07:57:33 -0700
@fabpot: I am also not sure if getFormat() should optionally not support matching parameters, aka anything before ``;q=..``
That allows projects that only use HttpFoundation and not HttpKernel to be able to
enforce the HTTP specification "rules".
$request = Request::createFromGlobals();
$response = new Response();
// do whatever you want with the Respons
// enforce HTTP spec
$response->prepare($request);
$response->send();
Within Symfony2, the prepare method is automatically called by the ResponseListener.
Commits
-------
205f524 [Tests] Skip MimeTypeTest if running as root
d3e9104 [Tests] Skip Routing annotation tests when Doctrine is not available.
Discussion
----------
[Tests] Fix tests when some vendor is not available.
Commits
-------
73312ab [Validator] The Type constraint now accepts the "Boolean" type instead of "boolean".
Discussion
----------
[Validator] The Type constraint now accepts the "Boolean" type instead of
[Validator] The Type constraint now accepts the "Boolean" type instead of "boolean".
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
---------------------------------------------------------------------------
by stloyd at 2011/10/11 03:43:24 -0700
As this is bugfix only, this should be made againts __2.0__ branch.
---------------------------------------------------------------------------
by hhamon at 2011/10/11 05:03:52 -0700
@stloyd I don't really know if it's a bug fix or an improvement. We could not use "Boolean" but we could use "boolean" or "bool" only. So that's why I just improved this constraint validator.
Commits
-------
ba6bd4b [HttpKernel] Fix the FileProfileStorage, according to the tests
1654473 [HttpKernel] Create a test to outline the problem with file storage
Discussion
----------
[HttpKernel] Fix the file storage
The file storage was not correctly fetching children back.
* First the test showing the problem
* Second the fix to the test
Solution is to add a file for each stored profile file, containing the list of children tokens.
The locale management does not require sessions anymore.
In the Symfony2 spirit, the locale should be part of your URLs. If this is the case
(via the special _locale request attribute), Symfony will store it in the request
(getLocale()).
This feature is now also configurable/replaceable at will as everything is now managed
by the new LocaleListener event listener.
How to upgrade:
The default locale configuration has been moved from session to the main configuration:
Before:
framework:
session:
default_locale: en
After:
framework:
default_locale: en
Whenever you want to get the current locale, call getLocale() on the request (was on the
session before).
Commits
-------
95049ef [Form] Added type check to `ScalarToChoiceTransformer`
Discussion
----------
[2.0][Form] Added type check to ScalarToChoiceTransformer
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Commits
-------
18a83c6 [Form] Simplified a bit `FormUtil` and extended test coverage
Discussion
----------
[2.0][Form] Simplified a bit `FormUtil` and extended test coverage
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
* 2.0:
[Validator] added support for grapheme_strlen when mbstring is not installed but intl is installed
removed separator of choice widget when the separator is null
Commits
-------
5146a1f [EventDispatcher] Added possibility for subscribers to subscribe several times for same event
Discussion
----------
[EventDispatcher] Added possibility for subscribers to subscribe several times for same event
[EventDispatcher] Added possibility for subscribers to subscribe several times for same event
closes#2146
And it is of course fully BC :)
---------------------------------------------------------------------------
by jalliot at 2011/09/09 17:34:07 -0700
If merged, #2021 will have to reflect the change too
Commits
-------
c4a0f79 Updates according to suggestions.
6aec789 Added tests.
54454ba Added generic filtering to ParameterBag.
Discussion
----------
Added generic filtering to ParameterBag.
Adds filtering convenience using PHP's filter_var() e.g.
$request->get->filter($key, '', false, FITLER_SANITIZE_STRING);
See http://php.net/manual/en/filter.filters.php for capabilities.
---------------------------------------------------------------------------
by GromNaN at 2011/09/25 15:41:50 -0700
What is the use case ?
---------------------------------------------------------------------------
by drak at 2011/09/25 15:52:19 -0700
Input variable validation/sanitization. ParameterBag has a few built in like `getAlnum()` for example. This method offer's PHP's full filtering and sanitization suite.
---------------------------------------------------------------------------
by fabpot at 2011/09/27 00:56:41 -0700
Can you add some unit tests for this new feature?
---------------------------------------------------------------------------
by drak at 2011/09/27 00:58:56 -0700
Sure thing.
---------------------------------------------------------------------------
by drak at 2011/09/27 01:07:03 -0700
Before I make the commit, is the method name ok for you or would you prefer it is called `getFiltered()`?
---------------------------------------------------------------------------
by fabpot at 2011/09/27 01:13:46 -0700
`filter` sounds good to me.
---------------------------------------------------------------------------
by drak at 2011/09/27 02:37:01 -0700
I've added some tests.
---------------------------------------------------------------------------
by stloyd at 2011/09/27 02:42:42 -0700
@drak IMO you must check that user don't use unknown filter and/or flags for filter.
---------------------------------------------------------------------------
by drak at 2011/09/27 02:48:38 -0700
@stloyd - I'm not sure that's practical at all, this is a wrapper for a built-in PHP function and I don't understand why we would need validate arguments for a PHP function - it's the coder's job to use the API correctly - none of the inputs to this function are coming from a web request. It would also mean that the API would need to keep track of any upstream changes to constants in the PHP engine (which are just integers after all). It's really just not practical.
---------------------------------------------------------------------------
by stealth35 at 2011/09/27 05:16:50 -0700
@drak it's could be cool to use `filter_id` ✌️
if (is_string) {
$filter = filter_id($filter);
}
---------------------------------------------------------------------------
by drak at 2011/09/27 07:05:42 -0700
@stealth35 regarding this
if (is_string) {
$filter = filter_id($filter);
}
I believe strongly in the use of IDEs when coding and autocomplete nicely provides when you type `FILTER_`. Additionally, `filter_id()` only works on filters, but not for the flags, so I'm not entirely sure how useful it would be overall compared to using a good IDE (which you need when working with complex frameworks anyhow, imo :)
---------------------------------------------------------------------------
by drak at 2011/09/27 07:30:10 -0700
Ok check it now.
Commits
-------
67c33a8 Rebased with master, and fixed wrong behavior with proper tests coverage
f8a6a4b Be sure that both fields have same value for required option in RepeatedType
0679220 Additional test coverage for changes in RepeatedType
b23d47d moved options test form from class->method scope
5fe5556 fixed accidental permission change
a969434 [Form] fixed CS, merged options, added tests
8819db3 [Form] Allow setting different options to repeating fields
Discussion
----------
[2.1] [Form] Allow setting different options at RepeatedType fields
This an test covered version of #1348 (rebased with master).
---------------------------------------------------------------------------
by stloyd at 2011/06/27 04:18:19 -0700
@fabpot What do you think about this ? I'm just not sure that we should allow setting `required` per field, IMO better would be forcing this option from default `$options['options']` and ignore that field in `$options['first_options']` and/or `$options['second_options']`.
---------------------------------------------------------------------------
by stloyd at 2011/07/02 00:00:04 -0700
@fabpot ping.
---------------------------------------------------------------------------
by fabpot at 2011/07/06 05:45:56 -0700
Let's discuss this new feature for 2.1.
---------------------------------------------------------------------------
by stloyd at 2011/08/24 01:12:59 -0700
Rebased with master.
---------------------------------------------------------------------------
by stof at 2011/09/04 05:02:42 -0700
@fabpot What do you think about this feature ? It is now time to discuss it :)
---------------------------------------------------------------------------
by fabpot at 2011/09/22 00:18:29 -0700
Tests do not pass.
---------------------------------------------------------------------------
by stloyd at 2011/09/24 01:54:42 -0700
@fabpot Should be ok now.
Commits
-------
a0329c3 Added lifetime/cleanup support.
365e73a Fixed the find() method and changed the way the profile data is stored.
beeec5e Allow socket dsn (for example mongodb:///tmp/mongodb-27017.sock).
218eaba Fixed storage of time value.
85c3806 Added support for sorting by time like other profiler storage implementations.
73692c6 Fixed MongoDbProfilerStorage::find() when passing empty parameters.
4cd2dec Use token as identifier to make usage of the automatically created index.
Discussion
----------
[2.1] [HttpKernel] MongoDB profiler updates
I fixed one issue within the MongoDbProfilerStorage::find() function and made some more changes.
---------------------------------------------------------------------------
by snc at 2011/09/11 02:28:35 -0700
Please don't merge this in yet. There are some more commits pending...
---------------------------------------------------------------------------
by fabpot at 2011/09/14 01:07:39 -0700
@snc: is it ready for a merge?
---------------------------------------------------------------------------
by snc at 2011/09/14 01:20:32 -0700
Unfortunately not... while testing I found out that the currently merged in implementation does not work completely. The web profiler search function errors because the find function returns the wrong data. I fixed this already but now I have some strange "maximum function nesting level reached" errors when viewing profiles with children. I will work on it later today.
---------------------------------------------------------------------------
by snc at 2011/09/14 13:27:50 -0700
Now only one thing is missing... the generated container code looks like this:
`$this->services['profiler'] = $instance = new \Symfony\Component\HttpKernel\Profiler\Profiler(new \Symfony\Component\HttpKernel\Profiler\MongoDbProfilerStorage('mongodb://localhost/sf2-mongo-profiler/profiler', '', '', 86400), $a);`
The current constructor only uses the first parameter (dsn). What about the username, password and lifetime? Username and passwort can already be passed via the dsn... but the lifetime feature is not part of the interface... should I implement it?
---------------------------------------------------------------------------
by fabpot at 2011/09/15 11:03:02 -0700
The `lifetime` is used to cleanup the database (see https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpKernel/Profiler/PdoProfilerStorage.php#L136). So, it should probably be implemented for MongoDB as well (but it can probably be done in another PR).
---------------------------------------------------------------------------
by snc at 2011/09/19 13:42:52 -0700
Sorry for the delay, lifetime support is now implemented. What do you think about an AbstractProfilerStorageTest class to share some testing code between the different implementations (of cause in a separate PR)?
* 2.0:
[HttpKernel] fixed typo
fixed previous merge, done the same change to other occurences
fixes usage of mb_*
Profiler session import fixed.
[Process] workaround a faulty implementation of is_executable on Windows
Swedish translation fix.
[Locale] Fix#2179 StubIntlDateFormatter support yy format
Fixed fourth argument of Filesystem->mirror()
Commits
-------
777f876 [HttpFoundation] Added test that exposes error in session saving
Discussion
----------
[HttpFoundation] Added test that exposes error in session saving
Noticed this commit in the recent release:
https://github.com/symfony/symfony/commit/34a1b53
And noticed that the save in __destruct won't be fired if a manual save has been called. The testSavedOnDestructAfterManualSave test I've added fails on 2.0.1 but it passes with 2.0.0. An example:
$session->set('foo', 'value');
$session->__destruct(); // eventually called during shutdown, triggers a save
// During next request, $session->get('foo') returns 'value'
$session->set('foo', 'value');
$session->save();
$session->set('foo', 'newvalue');
$session->__destruct(); // eventually called during shutdown, however WON'T trigger save
// During next request, $session->get('foo') returns 'value'
In my eyes the save on destruction should still happen whether a save has been called manually or not - it's more predictable. But i can see arguments for the opposite as well.
If consensus is that this is a bug, I'm happy to provide a fix but wanted to get feedback on the 2 options:
* Save optimisation can be reverted
* Can make the save optimisation more intelligent so a write to storage is only done if something has changed. The best way to do this would be to close down the api and mark the session as invalid when an attribute is set for example, however all properties are currently protected and would need to be private, so it might break some people's extensions of session
---------------------------------------------------------------------------
by stof at 2011/09/04 02:13:52 -0700
@fabpot what about it ?
Commits
-------
e6e5146 [Translation] now support ResourceBundle
Discussion
----------
[2.1][Translation] now support ResourceBundle
support `.res` and `.dat` bundles
---------------------------------------------------------------------------
by marijn at 2011/09/08 08:59:39 -0700
There are a few references to `ressource`, I guess that is a typo...
---------------------------------------------------------------------------
by stealth35 at 2011/09/08 09:13:32 -0700
@marijn thank, done
---------------------------------------------------------------------------
by fabpot at 2011/09/11 00:42:37 -0700
Is it possible to add a dumper like we have for all other loaders?
---------------------------------------------------------------------------
by stof at 2011/09/11 03:46:39 -0700
Btw, you need to rebase your branch as it conflicts with master
---------------------------------------------------------------------------
by stealth35 at 2011/09/11 04:04:23 -0700
@fabpot it's more difficult (or the easy way it's to use `exec` with `derb`), I can create the text resources
@stof oki
---------------------------------------------------------------------------
by fabpot at 2011/09/11 23:52:19 -0700
@stealth35: Can you remove the `@api` tags? We will review what is included into the public API later on. thanks.
---------------------------------------------------------------------------
by stealth35 at 2011/09/12 04:18:07 -0700
@fabpot done
Commits
-------
aecfd0a [HttpFoundation] Support user and password in url
Discussion
----------
[HttpFoundation] Support user and password in url
Allow `http://user:password@test.com` for url request and `basic auth`
Added Methods for `Request`
- `getUser`
- `getPassword `
---------------------------------------------------------------------------
by stealth35 at 2011/08/26 07:52:57 -0700
@seldaek @stof thanks guys it's done
---------------------------------------------------------------------------
by stof at 2011/08/28 11:52:11 -0700
btw, you should rebase your branch on top of the upstream master branch as it conflicts
---------------------------------------------------------------------------
by stealth35 at 2011/08/28 13:15:55 -0700
@stof done
---------------------------------------------------------------------------
by stof at 2011/09/04 02:09:11 -0700
@stealth35 you made an error when rebasing: you merged the previous version of the branch instead of forcing the push, thus duplicating all commits in the PR (rebasing rewrites the history so it creates new commits). Could you clean your branch ?
---------------------------------------------------------------------------
by stealth35 at 2011/09/05 02:00:17 -0700
@stof 👍
---------------------------------------------------------------------------
by stealth35 at 2011/09/06 08:21:00 -0700
should be 2.0 ?
---------------------------------------------------------------------------
by fabpot at 2011/09/11 23:52:30 -0700
@stealth35: Can you remove the `@api` tags? We will review what is included into the public API later on. thanks.
---------------------------------------------------------------------------
by stealth35 at 2011/09/12 04:02:01 -0700
@fabpot done
