The regexp in MoneyType doesn't work if currency format has no decimal
(like JPY) and doesn't work either if the currency symbol is unicode
This change fixes both issues and adds a unit test
ad (1): HTML4 "id" attributes are limited to strings starting with a letter and containing only letters, digits, underscores, hyphens, periods and colons.
ad (2): Property paths contain three special characters needed for correct parsing: left/right bracket and period.
The rules for form naming are:
* Names may start with a letter, a digit or an underscore. Leading digits or underscores will be stripped from the "id" attributes.
* Names must only contain letters, digits, underscores, hyphens and colons.
* Root forms may have an empty name.
Solves #1919 and #3021 on a wider scope.
Commits
-------
753c067 [FrameworkBundle] added $view['form']->csrfToken() helper
e1aced8 [Twig] added {{ csrf_token() }} helper
Discussion
----------
[Twig] [FrameworkBundle] added CSRF token helper
I've added a templating helper and Twig function for generating a CSRF token without the overhead of creating a form.
```html+jinja
<form action="{{ path('user_delete', { 'id': user.id }) }}" method="post">
<input type="hidden" name="_method" value="delete">
<input type="hidden" name="_token" value="{{ csrf_token('delete_user_' ~ user.id) }}">
<button type="submit">delete</button>
</form>
```
```php
<?php
class UserController extends Controller
{
public function delete(User $user, Request $request)
{
$csrfProvider = $this->get('form.csrf_provider');
if (!$csrfProvider->isCsrfTokenValid('delete_user_'.$user->getId(), $request->request->get('_token')) {
throw new RuntimeException('CSRF attack detected.');
}
// etc...
}
}
```
The test that is failing on Travis appears to be unrelated, but I may be wrong?
```
1) Symfony\Bundle\SecurityBundle\Tests\Functional\LocalizedRoutesAsPathTest::testLoginLogoutProcedure with data set #1 ('de')
RuntimeException: OUTPUT:
Catchable fatal error: Argument 3 passed to Symfony\Bundle\FrameworkBundle\Controller\TraceableControllerResolver::__construct() must be an instance of Symfony\Component\HttpKernel\Debug\Stopwatch, instance of Symfony\Bundle\FrameworkBundle\Controller\ControllerNameParser given, called in /tmp/2.1.0-DEV/StandardFormLogin/cache/securitybundletest/appSecuritybundletestDebugProjectContainer.php on line 94 and defined in /home/vagrant/builds/kriswallsmith/symfony/src/Symfony/Bundle/FrameworkBundle/Controller/TraceableControllerResolver.php on line 37
```
---------------------------------------------------------------------------
by pablodip at 2012-01-10T14:18:45Z
As you don't need forms to use the csrf provider, how about putting its service without the form prefix? It could even make sense to put the CsrfProvider as a component since you can use it standalone and in more cases than only forms. It would be a small component though.
---------------------------------------------------------------------------
by Tobion at 2012-01-10T17:54:14Z
I think it would be more clear to generate the token in the controller. Doing so in the template will spread the CSRF intention across template and controller. So I don't think this extension is necessary.
---------------------------------------------------------------------------
by kriswallsmith at 2012-01-10T17:58:14Z
@pablodip I'm open to the idea of a Csrf component. This would be a good place for some nonce classes as well.
@Tobion I disagree. One use case is for a list of users, each with a delete form. Iterating over the users in the controller and generating a token for each, just to iterate over them again in the view is a waste and adds complexity.
---------------------------------------------------------------------------
by Tobion at 2012-01-10T18:05:14Z
I see. But I don't understand why the intention needs to be different for each user to delete. Usually the intention is the same for each form type. I thought this is enough.
---------------------------------------------------------------------------
by kriswallsmith at 2012-01-10T18:06:13Z
Yes, a static intention would suffice.
---------------------------------------------------------------------------
by Tobion at 2012-01-10T18:07:08Z
Then your use case is not valid anymore.
---------------------------------------------------------------------------
by Tobion at 2012-01-10T18:12:25Z
I would suggest to make a cookbook article out of it about how to create a simple form without the form component.
And include such things as validating the result using the validator component and checking the CSRF.
---------------------------------------------------------------------------
by kriswallsmith at 2012-01-10T21:32:50Z
This helper makes it easier to use CSRF protection without a form and we should make it as easy as possible. Spreading the intention across controller and template is not concerning to me. Either way, a cookbook entry is a great idea.
---------------------------------------------------------------------------
by Tobion at 2012-01-10T21:47:12Z
Well, it's just one line more without this helper. So I disagree it makes it really easier when you know how to use the CsrfProvider which is a pre-condition anyway since you must still validate its correctness by hand.
---------------------------------------------------------------------------
by kriswallsmith at 2012-01-13T13:24:15Z
Another use case is when rendering a page with a bunch of simple buttons with different intentions: delete user, delete comment, follow, unfollow... Creating all of these in the controller just leads to spaghetti.
---------------------------------------------------------------------------
by jwage at 2012-01-17T21:55:53Z
👍 lots of use cases for something like this @OpenSky
Commits
-------
92f820a Renamed registerConstraints to loadDynamicValidatorMetadata
dd12ff8 CS fix, getConstraints renamed
09c1911 [Validator] Improved dynamic constraints
54cb6e4 [Validator] Added dynamic constraints
Discussion
----------
[Validator] Dynamic constraints
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
By now the Validator component is based on a per-class configuration of
constraints, but in some cases it might be neccessary to add new constraints
dynamically at runtime.
This pull request adds a "ConstraintProviderInterface" to the Validator component. If an object is validated that implements this interface the method "getConstraints" is used to add dynamic constraints:
class User implements ConstraintProviderInterface
{
protected $isPremium;
protected $paymentInformation;
public function getConstraints(ClassMetadata $metadata)
{
if ($this->isPremium) {
$metadata->addPropertyConstraint('paymentInformation', new NotBlank());
}
}
}
---------------------------------------------------------------------------
by alexandresalome at 2012-01-15T11:20:04Z
Related to #1151
---------------------------------------------------------------------------
by canni at 2012-01-16T09:22:28Z
👍
---------------------------------------------------------------------------
by bschussek at 2012-01-16T12:32:44Z
I think this is a good addition. I think we still have a naming problem though. When constraints are loaded using a static method, the default name for the loader method is `loadValidatorMetadata`. Since the method for dynamic constraint loading is basically the same, I think the two names should be related.
Solution (1): Rename the method in your interface to `loadDynamicValidatorMetadata`. Ugly and long.
class MyClass implements ConstraintProviderInterface
{
public static loadValidatorMetadata(ClassMetadata $metadata) ...
public loadDynamicValidatorMetadata(ClassMetadata $metadata) ...
}
Solution (2): Rename the default method name in `StaticMethodLoader` to `registerConstraints` and adjust the docs. Breaks BC.
class MyClass implements ConstraintProviderInterface
{
public static registerConstraints(ClassMetadata $metadata) ...
public registerDynamicConstraints(ClassMetadata $metadata) ...
}
@fabpot: Are we allowed to break BC here? If not, we should probably stick to (1).
---------------------------------------------------------------------------
by fabpot at 2012-01-16T12:36:14Z
I would prefer to not break BC if possible.
---------------------------------------------------------------------------
by blogsh at 2012-01-16T15:25:46Z
So "loadDynamicValidatorMetadata" would be the best solution?
---------------------------------------------------------------------------
by althaus at 2012-01-17T13:39:19Z
>So "loadDynamicValidatorMetadata" would be the best solution?
Sounds fine for me based on @bschussek's comment.
Commits
-------
0b7e2e0 Support for DELETE method in forms
Discussion
----------
[Form] Support DELETE HTTP verb
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: none
Todo: -
As `Symfony\Component\HttpFoundation\Request` already support DELETE requests nicely by parsing the request for us, support for the HTTPs DELETE verb can be easily done.
---------------------------------------------------------------------------
by mvrhov at 2012-01-20T06:00:49Z
This is wrong. The body for DELETE method is supposed to be empty or if present ignored.
Also the DELETE is supposed to remove the resource identified by uri, so the same code as for GET should be executed.
---------------------------------------------------------------------------
by lstrojny at 2012-01-20T08:56:22Z
I don’t think that’s the case. The HTTP standard does not state explicitly that DELETE does not have a body. See this [StackOverflow thread](http://stackoverflow.com/questions/2539394/rest-http-delete-and-parameters)
* 2.0:
Updated Serbian translation.
fixed CS
[Locale][Testing] Fixed breaking tests if 'intl' extension is not installed (#3139)
[Bridge] [Twig] fixed typo in a comment of the Twig FormExtension extension.
Symfony\Tests\Component\Locale\LocaleTest->testGetDisplayCountriesReturnsFullListForSubLocale()
fails with a fatal error if the PHP extension 'intl' is not installed on the system.
Added a check which skips the affected tests if the extension is not available.
Fixes#3139
Commits
-------
e6e3da5 [Validator] Improved test coverage of CollectionValidator and reduced test code duplication
509c7bf [Validator] Moved Optional and Required constraints to dedicated sub namespace.
bf59018 [Validator] Removed @api-tag from Optional and Required constraint, since these two are new.
6641f3e [Validator] Added constraints Optional and Required for the CollectionValidator
Discussion
----------
[Validator] Improve support for optional/required fields in Collection constraint
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: none
Todo: none
Improves the `Collection` constraint to test on a more granular level if entries of the collection are optional or required. Before this could only be set using the "allowExtraFields" and "allowMissingFields" options, but these are very general and limited.
The former syntax - without Optional or Required - is still supported.
Usage:
$array = array(
'name' => 'Bernhard',
'birthdate' => '1970-01-01',
);
$validator->validate($array, null, new Collection(array(
'name' => new Required(),
'birthdate' => new Optional(),
));
// you can also pass additional constraints for the fields
$validator->validate($array, null, new Collection(array(
'name' => new Required(array(
new Type('string'),
new MinLength(3),
)),
'birthdate' => new Optional(new Date()),
));
---------------------------------------------------------------------------
by canni at 2012-01-15T20:22:17Z
@bschussek I've rewritten a lot of test code for Collection validator in 2.0 branch and also had modified validator itself, as it had a bug #3078, consider waiting with this PR till fabpot will merge 2.0 back into master, as there will be code conflicts :)
---------------------------------------------------------------------------
by Koc at 2012-01-15T23:13:04Z
Does it helps to #2615 ?
---------------------------------------------------------------------------
by fabpot at 2012-01-16T06:44:53Z
@canni: I've just merged 2.0 into master.
---------------------------------------------------------------------------
by bschussek at 2012-01-16T12:05:19Z
@fabpot: Rebased. I also fixed the CS issues mentioned by @stof.
Sublocales like de_CH returned only incomplete results for
getDisplayCountries(), getDisplayLanguages() and getDisplayLocales(),
consisting only of results specific for this sublocale, but without the
results of their respective parent locale
Commits
-------
e23d452 Add info about BC Break to CHANGELOG-2.1
d7ffeb5 Add some more tests, and enforce boolean return value of interface implementations.
9d3a49f When method name is `hasUserChanged` the return boolean should be true (to match question semantics) and false when user has not changed, this commits inverts return statements.
c57b528 Add note about `AdvancedUserInterface`.
3682f62 Refactor `isUserChanged` to `hasUserChanged`
56db4a1 Change names to Equatable
680b108 Suggested fixes ;)
9386583 [BC Break][Security] Moved user comparsion logic out of UserInterface As discussed on IRC meetings and in PR #2669 I came up with implementation. This is option2, I think more elegant.
Discussion
----------
[BC Break][Security][Option2] Moved user comparsion logic out of UserInterface
As discussed on IRC meetings and in PR #2669 I came up with implementation.
This is option2, I think more elegant.
BC break: yes
Feature addition: no/feature move
Symfony2 test pass: yes
Symfony2 test written: yes
Todo: decide about naming
[](http://travis-ci.org/canni/symfony)
---------------------------------------------------------------------------
by schmittjoh at 2011-12-19T19:33:24Z
This looks much better than the previous PR. Thanks!
One thing, we also discussed this on Doctrine, the name "comparable" is used in most programming languages to perform a real compare operation that is ">", "<", or "=". In this case though, we are specifically interested in equality of two objects (we cannot establish a natural order between these objects). Java has no such interface as all objects naturally have an equals() method, .NET uses "Equatable" which looks a bit odd. Not sure if there are better names.
---------------------------------------------------------------------------
by canni at 2011-12-19T19:34:52Z
I think this is best of "both worlds" we have nice full-featured implementation suitable for most, and if someone needs advanced compare logic just implements interface. @stof @schmittjoh, what do you think?
---------------------------------------------------------------------------
by stof at 2011-12-19T19:36:55Z
@canni I already commented on the code, and I agree with @schmittjoh that the naming can be confusing
---------------------------------------------------------------------------
by jmikola at 2011-12-20T17:33:22Z
I don't mean to bikeshed, but I strongly agree with @schmittjoh about implications of "compare". I'm not concerned with the interface name so much as I am with `compareUser()`. Given that this method returns a boolean, I think it's best to prefix it with `is` (e.g. `isSameUser`, `isUserEqualTo`) or `equals` (e.g. `equalsUser`).
In this PR, the Token class is implementing the interface, so I think having "User" in the method name is a good idea. Naturally, if the interface was intended for User classes, we could do without it.
---------------------------------------------------------------------------
by canni at 2011-12-20T19:00:00Z
@jmikola in this PR Token class does not implement any additional interface, and `compareUser` is `private` and used internally. I don't stand still after this names, I'll update PR as soon as some decision about naming will be done.
---------------------------------------------------------------------------
by jmikola at 2011-12-21T02:29:59Z
@canni: My mistake, I got confused between the Token method and interface method, which you've since renamed in canni/symfony@fcfcd1087b.
---------------------------------------------------------------------------
by mvrhov at 2011-12-21T06:09:45Z
hm. Now I'm going to bike shed. Wouldn't the proper function name be hasUserChanged?
---------------------------------------------------------------------------
by stof at 2011-12-21T10:58:38Z
it would probably be bettter. The meaning of ``true`` and ``false`` would then be the opposite of the current ones but this is not an issue IMO as it is a different method
---------------------------------------------------------------------------
by jstout24 at 2011-12-27T18:08:49Z
@canni nice job
---------------------------------------------------------------------------
by fabpot at 2011-12-30T14:59:11Z
The method `isUserChanged()` must be rename. What about `hasUserChanged()` as @mvrhov suggested or `isUserDifferent()`?
---------------------------------------------------------------------------
by canni at 2012-01-02T11:44:05Z
@fabpot done.
---------------------------------------------------------------------------
by fabpot at 2012-01-02T18:13:40Z
The only missing thing I can think of is adding some unit tests.
---------------------------------------------------------------------------
by canni at 2012-01-10T20:16:25Z
@fabpot is there anything more you think that should done in this PR?
---------------------------------------------------------------------------
by stof at 2012-01-10T20:38:46Z
@canni can you rebase your branch ? it conflicts with the current master according to github
---------------------------------------------------------------------------
by canni at 2012-01-10T20:56:55Z
@stof done.
---------------------------------------------------------------------------
by fabpot at 2012-01-12T18:06:00Z
@canni: Can you just add some information in the CHANGELOG and in the UPGRADE file? That's all I need to merge this PR now. Thanks a lot.
---------------------------------------------------------------------------
by canni at 2012-01-12T18:16:32Z
@fabpot done, and no problem :)
As discussed on IRC meetings and in PR #2669 I came up with implementation.
This is option2, I think more elegant.
BC break: yes
Feature addition: no/feature move
Symfony2 test pass: yes
Symfony2 test written: yes
Todo: feedback needed
fix CS
fix CS + remove unneeded else
add documentation, change protected methods as private
rename var
throw exception for invalid name, index fix
memcache profiler storage support added, fix CS and minor bugs
fix CS
removed unneeded else
- memcached support added
- improved performance (serialization, index)
updated code to last version of Profiler
Commits
-------
c7ab9ba [Console] Allow redefinition of application options descriptions
Discussion
----------
[Console] Allow redefinition of application options descriptions
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
This allows you to redefine an `InputOption` as long as it keeps the same semantic (same default, same name, same alias, same modes). There are two purposes:
- Modifying the description with a more accurate one
- Making sure the option appears in your commands' help
Concrete example: I often want to provide a verbose version of commands. It's an elegant and very common pattern, but I basically can't document what is going to happen if you do `--verbose` since the base Application already defines `--verbose`. Also the `--verbose` option does not appear when you do `console <command> --help`, which means people probably won't think of using that option.
Commits
-------
9441c46 [DependencyInjection] PhpDumper, fixes#2730
Discussion
----------
[DependencyInjection] PhpDumper, fixes#2730
Hey, this PR fixes#2730, if no parameters are set, the constructor wont get passed a ParameterBag
Bug fix: yes (#2730)
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
3rd and last try ;) this time i think its all fine
Apache expects the response to already be in chunked format in that case,
which causes it to not deliver the streamed body.
If no Content-Length is set on the response, web servers will automatically
switch to chunked Transfer-Encoding, and handle the chunking for you.
Nginx does not share the issue that apache has, but will add the Content-
Length header too.
Commits
-------
3ef0594 [HttpKernel] Fix missing $method argument in MongoDbProfilerStorageTest
Discussion
----------
[HttpKernel] Fix missing $method argument in MongoDbProfilerStorageTest
```
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
```
This test case fixes failures related to a signature change for ProfilerStorageInterface::find(). See: 1aef4e806b and 5f2226807c.
This was easily missed as the test case is skipped unless a MongoDB server is running.
Commits
-------
887c0e9 moved EngineInterface::stream() to a new StreamingEngineInterface to keep BC with 2.0
473741b added the possibility to change a StreamedResponse callback after its creation
8717d44 moved a test in the constructor
e44b8ba made some cosmetic changes
0038d1b [HttpFoundation] added support for streamed responses
Discussion
----------
[HttpFoundation] added support for streamed responses
To stream a Response, use the StreamedResponse class instead of the
standard Response class:
$response = new StreamedResponse(function () {
echo 'FOO';
});
$response = new StreamedResponse(function () {
echo 'FOO';
}, 200, array('Content-Type' => 'text/plain'));
As you can see, a StreamedResponse instance takes a PHP callback instead of
a string for the Response content. It's up to the developer to stream the
response content from the callback with standard PHP functions like echo.
You can also use flush() if needed.
From a controller, do something like this:
$twig = $this->get('templating');
return new StreamedResponse(function () use ($templating) {
$templating->stream('BlogBundle:Annot:streamed.html.twig');
}, 200, array('Content-Type' => 'text/html'));
If you are using the base controller, you can use the stream() method instead:
return $this->stream('BlogBundle:Annot:streamed.html.twig');
You can stream an existing file by using the PHP built-in readfile() function:
new StreamedResponse(function () use ($file) {
readfile($file);
}, 200, array('Content-Type' => 'image/png');
Read http://php.net/flush for more information about output buffering in PHP.
Note that you should do your best to move all expensive operations to
be "activated/evaluated/called" during template evaluation.
Templates
---------
If you are using Twig as a template engine, everything should work as
usual, even if are using template inheritance!
However, note that streaming is not supported for PHP templates. Support
is impossible by design (as the layout is rendered after the main content).
Exceptions
----------
Exceptions thrown during rendering will be rendered as usual except that
some content might have been rendered already.
Limitations
-----------
As the getContent() method always returns false for streamed Responses, some
event listeners won't work at all:
* Web debug toolbar is not available for such Responses (but the profiler works fine);
* ESI is not supported.
Also note that streamed responses cannot benefit from HTTP caching for obvious
reasons.
---------------------------------------------------------------------------
by Seldaek at 2011/12/21 06:34:13 -0800
Just an idea: what about exposing flush() to twig? Possibly in a way that it will not call it if the template is not streaming. That way you could always add a flush() after your </head> tag to make sure that goes out as fast as possible, but it wouldn't mess with non-streamed responses. Although it appears flush() doesn't affect output buffers, so I guess it doesn't need anything special.
When you say "ESI is not supported.", that means only the AppCache right? I don't see why this would affect Varnish, but then again as far as I know Varnish will buffer if ESI is used so the benefit of streaming there is non-existent.
---------------------------------------------------------------------------
by cordoval at 2011/12/21 08:04:21 -0800
wonder what the use case is for streaming a response, very interesting.
---------------------------------------------------------------------------
by johnkary at 2011/12/21 08:19:48 -0800
@cordoval Common use cases are present fairly well by this RailsCast video: http://railscasts.com/episodes/266-http-streaming
Essentially it allows faster fetching of web assets (JS, CSS, etc) located in the <head></head>, allowing those assets to be fetched as soon as possible before the remainder of the content body is computed and sent to the browser. The end goal is to improve page load speed.
There are other uses cases too like making large body content available quickly to the service consuming it. Think if you were monitoring a live feed of JSON data of newest Twitter comments.
---------------------------------------------------------------------------
by lsmith77 at 2011/12/21 08:54:35 -0800
How does this relate the limitations mentioned in:
http://yehudakatz.com/2010/09/07/automatic-flushing-the-rails-3-1-plan/
Am I right to understand that due to how twig works we are not really streaming the content pieces when we call render(), but instead the entire template with its layout is rendered and only then will we flush? or does it mean that the render call will work its way to the top level layout template and form then on it can send the content until it hits another block, which it then first renders before it continues to send the data?
---------------------------------------------------------------------------
by stof at 2011/12/21 09:02:53 -0800
@lsmith77 this is why the ``stream`` method calls ``display`` in Twig instead of ``render``. ``display`` uses echo to print the output of the template line by line (and blocks are simply method calls in the middle). Look at your compiled templates to see it (the ``doDisplay`` method)
Rendering a template with Twig simply use an output buffer around the rendering.
---------------------------------------------------------------------------
by fabpot at 2011/12/21 09:24:33 -0800
@lsmith77: We don't have the Rails problem thanks to Twig as the order of execution is the right one by default (the layout is executed first); it means that we can have the flush feature without any change to how the core works. As @stof mentioned, we are using `display`, not `render`, so we are streaming your templates for byte one.
---------------------------------------------------------------------------
by fabpot at 2011/12/21 09:36:41 -0800
@Seldaek: yes, I meant ESI with the PHP reverse proxy.
---------------------------------------------------------------------------
by fabpot at 2011/12/21 09:37:34 -0800
@Seldaek: I have `flush()` support for Twig on my todo-list. As you mentioned, It should be trivial to implement.
---------------------------------------------------------------------------
by fzaninotto at 2011/12/21 09:48:18 -0800
How do streaming responses deal with assets that must be called in the head, but are declared in the body?
---------------------------------------------------------------------------
by fabpot at 2011/12/21 09:52:12 -0800
@fzaninotto: What do you mean?
With Twig, your layout is defined with blocks ("holes"). These blocks are overridden by child templates, but evaluated as they are encountered in the layout. So, everything works as expected.
As noted in the commit message, this does not work with PHP templates for the problems mentioned in the Rails post (as the order of execution is not the right one -- the child template is first evaluated and then the layout).
---------------------------------------------------------------------------
by fzaninotto at 2011/12/21 10:07:35 -0800
I was referring to using Assetic. Not sure if this compiles to Twig the same way as javascript and stylesheet blocks placed in the head - and therefore executed in the right way.
---------------------------------------------------------------------------
by fabpot at 2011/12/21 10:34:59 -0800
@Seldaek: I've just added a `flush` tag in Twig 1.5: 1d6dfad4f5
---------------------------------------------------------------------------
by catchamonkey at 2011/12/21 13:29:22 -0800
I'm really happy you've got this into the core, it's a great feature to have! Good work.
* 2.0:
[Tests] Skip segfaulting form test
Rename test file
[BrowserKit] added missing @return PHPDoc for the Client::submit() method.
also test PHP 5.3.2, since this is the official lowest supported PHP version
Commits
-------
85ca8e3 ParameterBag no longer resolves parameters that have spaces.
99011ca Added tests for ParameterBag parameters with spaces
Discussion
----------
[DependencyInjection] Parameters with spaces are not resolved
Bug fix: yes
Feature addition: no
Backwards compatibility break: no (not likely, according to convention)
Symfony2 tests pass: yes
Fixes the following tickets: #2884
`ParameterBag` currently resolves anything between two `%` signs, which creates issues for any parameters in the DIC that are legitimate text. This PR enforces the [documented parameter convention](http://symfony.com/doc/2.0/book/service_container.html#service-parameters) so that only `%parameters.with.no_spaces%` are resolved.
I was considering using instead `^%([^\w\._-]+)%$`, but felt that was too constricting & could easily introduce issues with existing applications.
To stream a Response, use the StreamedResponse class instead of the
standard Response class:
$response = new StreamedResponse(function () {
echo 'FOO';
});
$response = new StreamedResponse(function () {
echo 'FOO';
}, 200, array('Content-Type' => 'text/plain'));
As you can see, a StreamedResponse instance takes a PHP callback instead of
a string for the Response content. It's up to the developer to stream the
response content from the callback with standard PHP functions like echo.
You can also use flush() if needed.
From a controller, do something like this:
$twig = $this->get('templating');
return new StreamedResponse(function () use ($templating) {
$templating->stream('BlogBundle:Annot:streamed.html.twig');
}, 200, array('Content-Type' => 'text/html'));
If you are using the base controller, you can use the stream() method instead:
return $this->stream('BlogBundle:Annot:streamed.html.twig');
You can stream an existing file by using the PHP built-in readfile() function:
new StreamedResponse(function () use ($file) {
readfile($file);
}, 200, array('Content-Type' => 'image/png');
Read http://php.net/flush for more information about output buffering in PHP.
Note that you should do your best to move all expensive operations to
be "activated/evaluated/called" during template evaluation.
Templates
---------
If you are using Twig as a template engine, everything should work as
usual, even if are using template inheritance!
However, note that streaming is not supported for PHP templates. Support
is impossible by design (as the layout is rendered after the main content).
Exceptions
----------
Exceptions thrown during rendering will be rendered as usual except that
some content might have been rendered already.
Limitations
-----------
As the getContent() method always returns false for streamed Responses, some
event listeners won't work at all:
* Web debug toolbar is not available for such Responses (but the profiler works fine);
* ESI is not supported.
Also note that streamed responses cannot benefit from HTTP caching for obvious
reasons.
Commits
-------
49d2685 [Form] Add default validation to TextType field (and related)
Discussion
----------
[Form] Add default transformer to TextType field (and related)
Bug fix: yes&no (?)
Feature addition: yes (?)
BC break: no
Symfony2 tests pass: yes
Fixes the following tickets: #1962.
---------------------------------------------------------------------------
by stloyd at 2011/12/19 03:43:37 -0800
@fabpot ping ;-)
---------------------------------------------------------------------------
by fabpot at 2011/12/19 10:58:20 -0800
Is it really needed? I have a feeling that it enforces unneeded constraints, but I can be wrong of course.
---------------------------------------------------------------------------
by hlecorche at 2011/12/20 02:31:03 -0800
It's needed because with TextType field, and without the ValueToStringTransformer, the user data (when sending the form) can be an array !!!
For example:
- if there is a TextType field
- and if there is a MaxLengthValidator
- and if the user data (when sending the form) is an array
So the exception "Expected argument of type string, array given in src\Symfony\Component\Validator\Constraints\MaxLengthValidator.php at line 40" is thrown
Commits
-------
3ae976c fixed CS
84ad40d added cache clear hook
Discussion
----------
[Cache][2.1] Added cache clear hook
Allows bundles to hook into the `cache:clear` command by using the `kernel.cache_clearer` tag instead of using the `event_dispatcher` service.
See #1884
Bug fix: No
Feature addition: Yes
Backwards compatibility break: No
Symfony2 tests pass: Yes
Fixes the following tickets: #1884
References the following tickets: #1884
---------------------------------------------------------------------------
by dustin10 at 2011/12/16 11:03:54 -0800
Rebased to squash all commits into one.
---------------------------------------------------------------------------
by lsmith77 at 2011/12/17 05:27:29 -0800
@fabpot: we figured that priorities wouldn't be needed for cleaning .. haven't tested the PR, but conceptually it looks good to me and aside from the priority stuff its modeled after the cache warners.
---------------------------------------------------------------------------
by dustin10 at 2011/12/19 09:46:26 -0800
@fabpot Updated to pass cache dir to `clear` method.
---------------------------------------------------------------------------
by dustin10 at 2011/12/19 10:02:21 -0800
@stof and @fabpot Another thought I just had. Should the `$this->getContainer()->get('cache_clearer')->clear($realCacheDir);` call in the `CacheClearCommand` be done before the warming?
---------------------------------------------------------------------------
by stof at 2011/12/19 10:03:59 -0800
indeed. the clearing should be done before the warming.
---------------------------------------------------------------------------
by dustin10 at 2011/12/19 10:19:28 -0800
Squashed all commits into one. Let me know if there is anything else.
---------------------------------------------------------------------------
by dustin10 at 2011/12/19 10:31:50 -0800
Fixed extra lines.
Commits
-------
200ed54 [DoctrineBridge] Extracted the common type and made the choice list generic
3c81b62 [Doctrine] Cleanup and move loader into its own method
7646a5b [Doctrine] Dont allow null in ORMQueryBuilderLoader
988c2a5 Adjust check
3b5c617 [DoctrineBridge] Remove large parts of the EntityChoiceList code that were completly unnecessary (code was unreachable).
b919d92 [DoctrineBridge] Optimize fetching of entities to use WHERE IN and fix other inefficencies.
517eebc [DoctrineBridge] Refactor entity choice list to be ORM independant using an EntityLoader interface.
Discussion
----------
[DoctrineBridge] Refactor EntityChoiceList
Bug fix: no
Feature addition: yes
Backwards compatibility break: no (99%)
Symfony2 tests pass: yes
This decouples the ORM from the EntityChoiceList and makes its much more flexible. Instead of having a "query_builder" to do smart or complex queries you can now create "loader" instances which can arbitrarily help loading entities.
Additionally i removed lots of code that was unnecessary and not used by the current code.
There is a slight BC break in that the EntityChoiceList class is now accepting an EntityLoaderInterface instead of a querybuilder. However that class was nested inside the EntityType and should not be widely used or overwritten.
The abstract class DoctrineType is meant to be used as base class by other Doctrine project to share the logic by simply using a different type name and a different loader implementation.
This PR replaces #2728.
---------------------------------------------------------------------------
by beberlei at 2011/12/19 09:20:43 -0800
Thanks for doing the last refactorings, this is now fine from my side as well.
Commits
-------
8713c2d [DoctrineBridge][DoctrineBundle] Refactored the DBAL logging
Discussion
----------
[DoctrineBridge][DoctrineBundle] Refactored the DBAL logging
This allows enabling the logging and the profiling separately. This is useful for instance when doing batch processing leading to memory issue because of the profiling. In such case, the only solution currently is to disable the logging totally whereas disabling only the use of the profiler would allow seeing the queries in the logs (and the profiles are not collected in the CLI anyway).
I'm not sure about the place where the ``Stopwatch`` should be used. Keeping it in the ``DbalLogger`` with Monolog was the easy way as it allows using the DBAL class directly to collect queries for the profiler but technically the ``Stopwatch`` is used by the profiler.
the bundle changes are part of the PR to avoid letting the bundle in a broken state. I will also submit them to the doctrine/DoctrineBundle repo
---------------------------------------------------------------------------
by fabpot at 2011/12/16 02:33:05 -0800
Tests do not pass for me (even after upgrading the Doctrine deps to their latest versions):
There was 1 error:
1) Symfony\Bundle\DoctrineBundle\Tests\ContainerTest::testContainer
Argument 1 passed to Doctrine\DBAL\Logging\LoggerChain::addLogger() must implement interface Doctrine\DBAL\Logging\SQLLogger, instance of Doctrine\Dbal\Logging\DebugStack given
vendor/doctrine-dbal/lib/Doctrine/DBAL/Logging/LoggerChain.php:39
src/Symfony/Component/DependencyInjection/ContainerBuilder.php:777
src/Symfony/Component/DependencyInjection/ContainerBuilder.php:349
src/Symfony/Bundle/DoctrineBundle/Tests/ContainerTest.php:22
---------------------------------------------------------------------------
by stof at 2011/12/16 04:24:46 -0800
this is weird. DebugStack implements the interface, and the test passes for me
---------------------------------------------------------------------------
by fabpot at 2011/12/16 05:30:11 -0800
actually, the test pass when I run the `ContainerTest.php` file alone, but fail when I'm running the whole test suite.
---------------------------------------------------------------------------
by stof at 2011/12/16 05:39:15 -0800
I'm not able to run the whole testsuite. With intl enabled, it fails before the first test somewhere in the Locale component tests (Intl seems to be broken on 5.3.8 on windows as using the constructor of the intl classes gives me ``null`` in the variable). And after disabling intl, I got an error about allowed memory size exhausted during the EntityType tests
---------------------------------------------------------------------------
by stloyd at 2011/12/16 05:42:09 -0800
@stof And here goes Travis with help! ;-)
Just log in there, enable hook for your symfony repo, force an push, and watch result at: http://travis-ci.org/#!/stof/symfony
---------------------------------------------------------------------------
by stof at 2011/12/16 05:47:57 -0800
Note: when running only the testsuite for bundles, I also get such an error after about 450 of the 500 tests. It seems like the garbage collector does not clean the memory between tests...
---------------------------------------------------------------------------
by stof at 2011/12/16 05:52:08 -0800
anyway, the error seems really weird as the class implements the interface. I don't see how it could be passed without implementing it
---------------------------------------------------------------------------
by stof at 2011/12/16 06:10:43 -0800
@stloyd Travis allows me to see that this issue is not specific to @fabpot's computer. But it does not allow me to debug the test as I only get the result of the test, which does not make any sense
* 2.0:
[FrameworkBundle] Added functional tests.
[Form] Added missing use statements (closes#2880)
[Console] Improve input definition output for Boolean defaults
[SecurityBundle] Changed environment to something unique.
2879: missing space between catch and the brace
#2688: Entities are generated in wrong folder (doctrine:generate:entities Namespace)
[TwigBundle] Fix the exception message escaping
Commits
-------
7c2f11f Merge pull request #1 from pminnieur/post_response
9f4391f [HttpKernel] fixed DocBlocks
2a61714 [HttpKernel] added PostResponseEvent dispatching to HttpKernel
915f440 [HttpKernel] removed BC breaks, introduced new TerminableInterface
7efe4bc [HttpKernel] Add Kernel::terminate() and HttpKernel::terminate() for post-response logic
Discussion
----------
[HttpKernel] Add Kernel::terminate() and HttpKernel::terminate() for post-response logic
This came out of a discussion on IRC about doing stuff post-response, and the fact that right now there is no best practice, and it basically requires adding code after the `->send()` call.
It's an attempt at fixing it in an official way. Of course terminate() would need to be called explicitly, and added to the front controllers, but then it offers a standard way for everyone to listen on that event and do things without slowing down the user response.
---------------------------------------------------------------------------
by stof at 2011/12/06 02:41:26 -0800
We discussed it on IRC and I suggested a way to avoid the BC break of the interface: adding a new interface (``TerminableInterface`` or whatever better name you find) containing this method.
HttpKernel, Kernel and HttpCache can then implement it without breaking the existing apps using the component (Kernel and HttpCache would need an instanceof check to see if the inner kernel implements the method)
For Symfony2 users it will mean they have to change their front controller to benefit from the new event of course, but this is easy to do.
Btw, Silex can then be able to use it without *any* change for the end users as it can be done inside ``Application::run()``
---------------------------------------------------------------------------
by pminnieur at 2011/12/06 11:47:03 -0800
@Seldaek: I opened a pull request so that the discussion on IRC is fulfilled and no BC breaks exist: https://github.com/Seldaek/symfony/pull/1/files
---------------------------------------------------------------------------
by fabpot at 2011/12/07 07:59:49 -0800
Any real-world use case for this?
---------------------------------------------------------------------------
by Seldaek at 2011/12/07 08:10:31 -0800
Doing slow stuff after the user got his response back without having to implement a message queue. I believe @pminnieur wanted to use it to send logs to loggly?
---------------------------------------------------------------------------
by pminnieur at 2011/12/07 09:08:41 -0800
Its a good practice to defer code execution without the introduction of a new software layer (like gearman, amqp, whatever tools people use to defer code execution) which may be way too much just for the goal of having fast responses, whatever my code does.
My real world use case which made me miss this feature the first time:
> I have a calendar with a scheduled Event. For a given period of time, several Event entities will be created, coupled to the scheduled event (the schedule Event just keeps track of `startDate`, `endDate` and the `dateInterval`). Let's say we want this scheduled Event to be on every Monday-Friday, on a weekly basis, for the next 10 years.
This means I have to create `10*52*5` Event entities before I could even think about sending a simple redirect response. If I could defer code execution, I'd only save the scheduled Event, send the redirect response and after that, I create the `10*52*5` entities.
The other use case was loggly, yes. Sending logging data over the wire before the response is send doesn't make sense in my eyes, so it could be deferred after the response is send (this especially sucks if loggly fails and i get a 500 --the frontend/public user is not interested in a working logging facility, he wants his responses).
---------------------------------------------------------------------------
by mvrhov at 2011/12/07 10:07:03 -0800
This would help significantly, but the real problem, that your process is busy and unavailable for the next request, is still there.
---------------------------------------------------------------------------
by fabpot at 2011/12/07 10:15:18 -0800
I think this is the wrong solution for a real problem.
Saying "Its a good practice to defer code execution without the introduction of a new software layer" is just wrong.
It is definitely a good practice to defer code execution, but you should use the right tool for the job.
I'm -1.
---------------------------------------------------------------------------
by pminnieur at 2011/12/07 10:25:44 -0800
It should just give a possibility to put unimportant but heavy lifting code behind the send request with ease. With little effort people could benefit from the usage of `fastcgi_finish_request` without introducing new software, using `register_shutdown_function` or using `__destruct `(which works for simple things, but may act weird with dependencies).
It should not simulate node.js ;-) I agree that the real problem is not solved, but small problems could be solved easily. I personally don't want to setup RabbitMQ or whatever, maintain my crontab or any other software that may allow me to defer code execution.
---------------------------------------------------------------------------
by Seldaek at 2011/12/08 01:08:32 -0800
@fabpot: one could say that on shared hostings it is still useful because they generally don't give you gearman or \*MQs. Anyway I think it'd be nice to really complete the HttpKernel event cycle.
---------------------------------------------------------------------------
by pminnieur at 2011/12/08 01:48:57 -0800
not only on shared hostings, sometimes teams/projects just don't have the resources or knowledge or time to setup such an infrastructure.
---------------------------------------------------------------------------
by videlalvaro at 2011/12/08 01:53:06 -0800
I can say we used `fastcgi_finish_request` quite a lot at poppen with symfony 1.x. It certainly helped us to send data to Graphite, save XHProf runs, send data to RabbitMQ, and so on.
For example we used to connect to RabbitMQ and send the messages _after_ calling `fastcgi_finish_request` so the user never had to wait for stuff like that.
Also keep in mind that if you are using Gearman or RabbitMQ or whatever tool you use to defer code execution… you are not deferring the network connection handling, sending data over the wire and what not. I know this is obvious but is often overlooked.
So it would be nice to have an standard way of doing this.
---------------------------------------------------------------------------
by henrikbjorn at 2011/12/13 01:42:23 -0800
This could have been useful recently while implementing a "Poor mans cronjob" system. The solution was to do a custom Response object and do the stuff after send have been called with a Connection: Close header and ignore_user_abort(); (Yes very ugly)
Previously, Boolean defaults were printed as strings, which lead to true and false being printed as "1" and "", respectively. With this change, they are now printed as "true" and "false".
Commits
-------
0776b50 removed supports(De)Serializiation()
72b9083 SerializerAwareNormalizer now only implements SerializerAwareInterface
97389fa use Serializer specific RuntimeException
cb495fd added additional unit tests for deserialization
967531f fixed various typos from the refactoring
067242d updated serializer tests to use the new interfaces
d811e29 CS fix
351eaa8 require a (de)normalizer inside the (de)normalizable interfaces instead of a serializer
c3d6123 re-added supports(de)normalization()
078f7f3 more typo fixes
c3a711d abstract class children should also implement dernormalization
2a6741c typo fix
d021dc8 refactored encoder handling to use the supports*() methods to determine which encoder handles what format
f8e2787 refactored Normalizer interfaces
58bd0f5 refactored the EncoderInterface
b0daf35 split off an EncoderInterface and NormalizerInterface from SerializerInterface
Discussion
----------
[Serializer] split off an EncoderInterface and NormalizerInterface from SerializerInte
Bug fix: no
Feature addition: no
Backwards compatibility break: yes (but not inside a stable API)
Symfony2 tests pass: 
Fixes the following tickets: #2153
The purpose is to make it easier for other implementations that only implement parts of the interface due to different underlying approaches like the JMSSerializerBundle.
---------------------------------------------------------------------------
by schmittjoh at 2011/11/01 03:36:17 -0700
Actually, you can keep the current interface and I will just provide an adapter, sth like the following:
```php
<?php
class SymfonyAdapter implements SymfonyInterface
{
public function __construct(BundleInterface $serializer) { /* ... */ }
// symfony serializer methods mapped to bundle methods
}
```
I like to provide an adapter instead of implementing the interface directly since the bundle can be used standalone right now, and I don't want to add a dependency on the component just for the sake of the interface.
However, I do not completely see the purpose of the component. When would someone be recommended to use it? Everything the component does, the bundles does at the same level with the same complexity or simplicity (however you want to view that).
---------------------------------------------------------------------------
by lsmith77 at 2011/11/01 03:40:55 -0700
standalone in what way? you mean even out of the context of Symfony? In that context imho you should ship that code outside of a Bundle.
Regardless, how will that adaptor work? How would you implement methods like ``getEncoder()``? Afaik you can't and this is what this PR is about, splitting the interface to enable people to more finely specify what they provide.
---------------------------------------------------------------------------
by schmittjoh at 2011/11/01 04:03:56 -0700
I would just throw exceptions when something is not supported.
The more important question though is what is the goal of the component in the long-term, i.e. what problems is it supposed to solve, or in which cases should it be used?
Because right now it seems to me - correct me if I'm wrong - that the only purpose is that people don't have to install an extra library. However, that might even be frustrating for users because they need to migrate their code to the bundle as soon as they need to customize the serialization process which you need in 99% of the cases. For deserialization, the situation in the component is even worse. So, if my assessment is correct here (i.e. component to get started fast, if you need more migrate to the bundle), I think it would be better and less painful to have them start with the bundle right away.
---------------------------------------------------------------------------
by lsmith77 at 2011/11/01 04:15:10 -0700
Well then imho it would be better to split the interface.
I think the serializer component is sufficient for many situations and imho its easier to grok. Furthermore the normalizer/encoder concept it can be used in situations where JMSSerializerBundle cannot be used.
And splitting up the interfaces has exactly the goal of reducing the "frustrations" caused by out growing the the component.
---------------------------------------------------------------------------
by schmittjoh at 2011/11/01 04:29:39 -0700
I don't agree, but it's a subjective thing anyway.
So, whatever interface you come up with (preferably as few methods as possible), I will provide an adapter for it.
---------------------------------------------------------------------------
by fabpot at 2011/11/07 08:45:25 -0800
What's the status of this PR?
---------------------------------------------------------------------------
by lsmith77 at 2011/11/07 10:28:14 -0800
from my POV its good to go. but would like a nod from someone else in terms of the naming of the new interfaces
On 07.11.2011, at 17:45, Fabien Potencier <reply@reply.github.com> wrote:
> What's the status of this PR?
>
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/symfony/symfony/pull/2530#issuecomment-2655889
---------------------------------------------------------------------------
by stof at 2011/11/08 11:37:40 -0800
@lsmith77 what about doing the same for the ``NormalizerInterface`` instead of adding a new interface with a confusing name ? The Serializer class could implement ``Normalizer\NormalizerInterface`` by adding the 2 needed methods instead of duplicating part of the interface.
The next step is to refactor the Serializer class so that it choose the encoder and the decoder based on the ``support*`` methods. But this could probably be done in a separate PR.
---------------------------------------------------------------------------
by lsmith77 at 2011/11/08 11:51:27 -0800
yeah .. i wanted to do that once we are in agreement on the encoder stuff. question then is if we should again split off Denormalization. i guess yes.
---------------------------------------------------------------------------
by lsmith77 at 2011/11/08 12:06:34 -0800
ok done ..
---------------------------------------------------------------------------
by lsmith77 at 2011/11/08 12:59:51 -0800
i guess the next big task is to add more tests .. had to fix way too few unit tests with all this shuffling around .. will also help validating the concept. i should also test this out in a production application.
---------------------------------------------------------------------------
by lsmith77 at 2011/11/14 13:27:48 -0800
@ericclemmons can you also have a look at this PR and potentially help me adding tests?
---------------------------------------------------------------------------
by fabpot at 2011/12/07 07:32:06 -0800
@lsmith77: Is it ready to be merged? Should I wait for more unit tests?
---------------------------------------------------------------------------
by lsmith77 at 2011/12/07 07:34:56 -0800
If you merge it I am afraid I might get lazy and not write tests. This is why I changed the topic to WIP. I promise to finish this on the weekend.
Note however I was planning to write the tests for 2.0 and send them via a separate PR.
Once that PR is merged into 2.0 and master. I would then refactor them to work for this PR.
This way both 2.0 and master will have tests.
---------------------------------------------------------------------------
by fabpot at 2011/12/07 07:42:15 -0800
@lsmith77: sounds good. Thanks.
---------------------------------------------------------------------------
by lsmith77 at 2011/12/11 12:02:12 -0800
@fabpot ok i am done from my end.
@schmittjoh would be great if you could look over the final interfaces one time and give your blessing that you will indeed be able to provide implementations for these interfaces inside JMSSerializerBundle (even if just via an adapter)
---------------------------------------------------------------------------
by stof at 2011/12/12 12:43:49 -0800
@schmittjoh can you take a look as requested by @lsmith77 ?
---------------------------------------------------------------------------
by schmittjoh at 2011/12/13 03:33:23 -0800
Are the supports methods necessary? This is what I'm using in the bundle:
https://github.com/schmittjoh/JMSSerializerBundle/blob/master/Serializer/SerializerInterface.php
---------------------------------------------------------------------------
by lsmith77 at 2011/12/13 04:08:49 -0800
@schmittjoh without them determining if something is supported will always require an exception, which is pretty expensive. especially if one iterates over a data structure this can cause a lot of overhead.
---------------------------------------------------------------------------
by schmittjoh at 2011/12/13 04:24:18 -0800
my question was more if you have a real-world use case where this is useful?
On Tue, Dec 13, 2011 at 1:08 PM, Lukas Kahwe Smith <
reply@reply.github.com
> wrote:
> @schmittjoh without them determining if something is supported will always
> require an exception, which is pretty expensive. especially if one iterates
> over a data structure this can cause a lot of overhead.
>
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/symfony/symfony/pull/2530#issuecomment-3122157
>
---------------------------------------------------------------------------
by lsmith77 at 2011/12/13 04:28:08 -0800
yes .. this serializer .. since it traverses the tree and decides what is the current normalizer one by one (aka not via visitors as in your implementation). without the supports*() methods it would need to have the normalizer throw exceptions, but this is not exceptional, its the normal code flow to have to iterate to find the correct normalizer.
---------------------------------------------------------------------------
by schmittjoh at 2011/12/13 04:30:36 -0800
can we split it off into a second interface?
On Tue, Dec 13, 2011 at 1:28 PM, Lukas Kahwe Smith <
reply@reply.github.com
> wrote:
> yes .. this serializer .. since it traverses the tree and decides what is
> the current normalizer one by one (aka not via visitors as in your
> implementation). without the supports*() methods it would need to have the
> normalizer throw exceptions, but this is not exceptional, its the normal
> code flow to have to iterate to find the correct normalizer.
>
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/symfony/symfony/pull/2530#issuecomment-3122315
>
---------------------------------------------------------------------------
by lsmith77 at 2011/12/13 04:33:27 -0800
hmm .. i guess we could .. these methods in a way are implementation specific and are mainly public because its different objects interacting with each other, though for users of the lib they can also be convenient at times.
---------------------------------------------------------------------------
by lsmith77 at 2011/12/14 09:13:53 -0800
ok i reviewed things again and just removed those two methods, since the possibility for these methods to be feasible is too tied to the implementation and for this particular implementation supportsEncoding() and supportsDecoding() are still available.
so all ready to be merged ..
---------------------------------------------------------------------------
by lsmith77 at 2011/12/14 09:15:44 -0800
hmm i realized one thing just now:
cb495fd7a3
that commit should also be included in 2.0 .. i am not sure what the most elegant way is to make that happen ..
---------------------------------------------------------------------------
by fabpot at 2011/12/14 10:10:16 -0800
@lsmith77: commit cb495fd7a3 cannot be cherry picked in 2.0 as is as the tests do not pass: "Fatal error: Call to undefined method Symfony\Component\Serializer\Serializer::supportsDenormalization() in tests/Symfony/Tests/Component/Serializer/SerializerTest.php on line 150"
---------------------------------------------------------------------------
by lsmith77 at 2011/12/14 10:11:55 -0800
ah of course .. i just removed that method :) .. then never mind .. all is well.
Commits
-------
4d64d90 Allow empty result; change default *choices* value to **null** instead of **array()**. - added *testEmptyChoicesAreManaged* test - `null` as default value for choices. - is_array() used to test if choices are user-defined. - `null` as default value in __construct too. - `null` as default value for choices in EntityType.
Discussion
----------
[Doctrine][Bridge] EntityType: Allow empty result; default `choices` value changed to null
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
References the following tickets: #2504
- added *testEmptyChoicesAreManaged* test
- `null` as default value for choices.
- is_array() used to test if choices are user-defined.
- `null` as default value in __construct too.
- `null` as default value for choices in EntityType.
I squashed commits from PR #2504 as requested.
- added *testEmptyChoicesAreManaged* test
- `null` as default value for choices.
- is_array() used to test if choices are user-defined.
- `null` as default value in __construct too.
- `null` as default value for choices in EntityType.
Commits
-------
600066e [Templating] fixed 'scheme://' not detected as absolute path
e6f2687 [HttpKernel] fixed 'scheme://' not detected as absolute path
b50ac5b [Config] fixed 'scheme://' not detected as absolute path
Discussion
----------
[Config][HttpKernel][Templating] 'scheme://' paths not detected as absolute
Bug fix: yes
Feature addition: no
Backwards compatibility break: no (99%)
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
The method ```isAbsolutePath``` does not detect URL schemes as absolute. This makes imposible the use of wrappers to access remote files or the use of files (mostly configuration or templates) stored on phar archives (uses the scheme ```phar://``` in the path).
Three classes implement this methods: ```Symfony\Component\Config\FileLocator```, ```Symfony\Component\HttpKernel\Util\Filesystem``` and ```Symfony\Component\Templating\Loader\FilesytemLoader```. All are updated. Also includes a new check on all related tests (```Symfony\Component\HttpKernel\Util\Filesystem``` lacks of test).
Commits
-------
5f22268 [Profiler] Sync with master
1aef4e8 Adds collecting info about request method and allowing searching by it
Discussion
----------
[WebProfiler] Add ability to filter data by request method
Bug fix: no
Feature addition: yes
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: #1515
For discussion & description checkout: #1515 & #2279
---------------------------------------------------------------------------
by fabpot at 2011/12/11 10:02:41 -0800
After merging this PR, the toolbar is not displayed anymore for me.
---------------------------------------------------------------------------
by stof at 2011/12/12 14:18:20 -0800
@fabpot the toolbar works for me using this branch
Commits
-------
b1ca0cd added several tests to the serializer (mainly for deserialization)
Discussion
----------
Serializer tests
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: [](http://travis-ci.org/lsmith77/symfony)
Fixes the following tickets: -
The state of the serializer tests wasn't as bad as I thought.
Was mostly missing tests for some edge cases as well as deserialization.
Once this is merged to 2.0 and master, I will rebase #2530 and make sure the tests still pass.
Commits
-------
3759ff0 [Locale] StubNumberFormatter allow to parse 64bit number in 64bit mode
Discussion
----------
[Locale] StubNumberFormatter allow to parse 64bit number in 64bit mode
Bug fix: yes
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: [](http://travis-ci.org/stealth35/symfony)
Fixes the following tickets: #2735
---------------------------------------------------------------------------
by stealth35 at 2011/12/01 06:47:32 -0800
@Seldaek should be better now
---------------------------------------------------------------------------
by stealth35 at 2011/12/02 04:22:42 -0800
@fabpot done
---------------------------------------------------------------------------
by fabpot at 2011/12/02 04:28:24 -0800
Tests do not pas for me (on a Mac):
1) Symfony\Tests\Component\Locale\Stub\StubNumberFormatterTest::testParseTypeInt64StubWith64BitIntegerInPhp64Bit
->parse() TYPE_INT64 does not use true 64 bit integers, using only the 32 bit range.
Failed asserting that 2147483648 matches expected -2147483648.
.../tests/Symfony/Tests/Component/Locale/Stub/StubNumberFormatterTest.php:819
---------------------------------------------------------------------------
by stealth35 at 2011/12/02 04:50:20 -0800
@fabpot, could you send me the return of this code
``` php
<?php
$formatter = new \NumberFormatter('en', \NumberFormatter::DECIMAL);
$value = $formatter->parse('2,147,483,648', \NumberFormatter::TYPE_INT64);
var_dump($value);
$value = $formatter->parse('-2,147,483,649', \NumberFormatter::TYPE_INT64);
var_dump($value);
```
---------------------------------------------------------------------------
by fabpot at 2011/12/02 06:06:21 -0800
int(-2147483648)
int(2147483647)
---------------------------------------------------------------------------
by stealth35 at 2011/12/02 06:10:28 -0800
It's nosens, but the Stub should follow Intl ... so I fix that
---------------------------------------------------------------------------
by stealth35 at 2011/12/11 08:48:25 -0800
It's OK now
Commits
-------
bb0d202 Switched sanitizeParameter() for existing varToString()-method; now always stores a string representation of each parameter
4fe4dfd Fixed vendor version mismatch in tests
28730e9 [DoctrineBridge] Added unit tests
4535abe [DoctrineBridge] Fixed attempt to serialize non-serializable values
Discussion
----------
[DoctrineBridge] Fixed attempt to serialize non-serializable values
Bug fix: yes
Feature addition: no
Backwards compatibility break: no (99%)
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
The Doctrine DBAL type system does not pose any restrictions on the php-types of parameters in queries. Hence one could write a doctrine-type that uses a resource or an `\SplFileInfo` as its corresponding php-type. Parameters of these types are logged in the `DoctrineDataCollector` however, which is then serialized in the profiler. Since resources or `\SplFileInfo` variables cannot be serialized this throws an exception.
This PR fixes this problem (for known cases) by sanitizing the query parameters to only contain serializable types. The `isNotSerializable`-check surely is not complete yet, but more non-serializable classes can be added on a case-by-case basis.
---------------------------------------------------------------------------
by fabpot at 2011/12/07 07:04:43 -0800
Tests do not pass for me.
Furthermore, let's reuse what we already have in the framework (see https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpKernel/HttpKernel.php#L187 -- yes you can just copy/paster the existing code).
---------------------------------------------------------------------------
by aboks at 2011/12/09 01:41:14 -0800
@fabpot I fixed the tests (seems I had the wrong vendor versions in my copy) and reused the `varToString()`-code. This introduces a tiny BC break in the rare case that someone writes his own templates for the web profiler (the parameters returned by the data collector are now always a string; could be any type before).
After merging this PR, merging 2.0 into master would give a merge conflict and failing tests (because of the changes related to the introduction of the `ManagerRegistry` interface). To prevent this, please merge #2820 into master directly after merging this PR (so before merging 2.0 into master). After that 2.0 can be cleanly merged into master.
---------------------------------------------------------------------------
by stof at 2011/12/09 03:43:38 -0800
it is not a BC break. Using ``yaml_encode`` on a string will not break the template
This changes helps the common use case of fetching the current user and better complies with the Law of Demeter (http://en.wikipedia.org/wiki/Law_of_Demeter).
Before (still works):
$token = $context->getToken();
$user = $token ? $token->getUser() : null;
After:
$user = $context->getUser();
Commits
-------
e06cea9 [HttpFoundation] Cookie values should not be restricted
Discussion
----------
[HttpFoundation] Cookie values should not be restricted
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
The restriction I removed makes no sense IMO because we do not use setrawcookie() to send cookies. setrawcookie() does throw a warning when the cookie value contains incorrect characters, but not setcookie(). The latter will just urlencode() the value so it becomes valid. This is also what is done by `Cookie::__toString`, so this could be used in combination with header() to just send raw cookies that are valid, even with values that are invalid in their decoded form.
PHP urldecodes cookies on input, so it all works fine.
Commits
-------
da0773c Note DependencyInjection exception changes in the 2.1 changelog
9a090bc [DependencyInjection] Fix class check and failure message in PhpDumper test
2334596 [DependencyInjection] Use component's SPL classes in dumped service container
3c02ea2 [DependencyInjection] Use exception class for API doc generation
47256ea [DependencyInjection] Make exceptions consistent when ContainerBuilder is frozen
b7300d2 [DependencyInjection] Fix up @throws documentation
123f514 [DependencyInjection] Use component-specific SPL exceptions
cf2ca9b [DependencyInjection] Create additional SPL exceptions
ba8322e [DependencyInjection] Format base exception classes consistently
Discussion
----------
[DependencyInjection] Refactor usage of SPL exceptions
```
Bug fix: no
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: -
```
This was something discussed on the mailing list recently and a topic at the last IRC meeting. The DependencyInjection component was what I had in mind while discussing this with @lsmith a few weeks ago, as I found that it already had some local SPL exceptions, which weren't being used directly (only as base classes for the compiler exceptions). This PR replaces uses of core SPL exceptions with the component's equivalents. I've listed it as BC-breaking to be safe, but I don't think it should cause any trouble, since the new exceptions are sub-classes of those originally used.
I purposely avoided changing the exceptions in the dumped PHP container. If we agree that's worth doing, it would be a trivial addition to the PR.
---------------------------------------------------------------------------
by jmikola at 2011/12/04 22:38:47 -0800
One question I came across while implementing this PR: the doc blocks in ContainerInterface reference InvalidArgumentException (actually the component's local SPL equivalent), but there is no practical need for that class to be used in the file. How will the API doc generator handle this? Do we need to change the doc block reference to the full class path?
---------------------------------------------------------------------------
by fabpot at 2011/12/05 01:20:31 -0800
Why have you not replaced the exception in the dumped container code? I don't see any drawback.
---------------------------------------------------------------------------
by stof at 2011/12/05 03:39:25 -0800
it would even be better to be consistent in the generated code
---------------------------------------------------------------------------
by jmikola at 2011/12/05 09:48:05 -0800
I'll update the exceptions in the dumped container code as well. Thanks for the feedback.
---------------------------------------------------------------------------
by jmikola at 2011/12/05 10:41:21 -0800
Ok, this should be ready to review and merge. Tests needed some updating, but they still pass.
Commits
-------
541888d [EventDispatcher] Added missing object destruction in test tearDown() and removed duplicated tests.
Discussion
----------
[EventDispatcher] Merge two test cases into one.
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
Ref 667c24d73d (commitcomment-766966)
---------------------------------------------------------------------------
by stof at 2011/12/04 19:43:32 -0800
``testGetName`` could also be removed as it is already tested by ``getSetName``, and same for the event dispatcher.
Basically, you cannot test the setter without testing the associated getter so no need for 2 tests for them as it is forced to be duplicate for part of them (and no need to use so long names IMO)
---------------------------------------------------------------------------
by drak at 2011/12/05 00:49:22 -0800
I've refactored the test to remove duplication, I pushed as a rebase so the new commit is 541888d referencing the same discussion.
Commits
-------
7c1cbb9 [Config] Use LoaderResolverInterface for type-hinting
48b084e fixed typo
8ad94fb merged branch hhamon/doctrine_bridge_cs (PR #2775)
240796e [Bridge] [Doctrine] fixed coding conventions.
7cfc392 check for session before trying to authentication details
648fae7 merged branch proofek/domcrawlerform-radiodisabled (PR #2768)
3976b7a [DoctrineBridge] fixed CS
9a04783 merged branch beberlei/SecurityEntityRepositoryIdentifierFix (PR #2765)
3c83b89 [DoctrineBridge] Catch user-error when the identifier is not serialized with the User entity.
36c7d03 Fixed GH-2720 - Fix disabled atrribute handling for radio form elements
Discussion
----------
[Config] Use LoaderResolverInterface for type-hinting
```
Bug fix: no
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: -
```
I've listed this as a BC break because we're changing the argument type-hint, but I think it's unlikely to affect anyone.
Commits
-------
36c7d03 Fixed GH-2720 - Fix disabled atrribute handling for radio form elements
Discussion
----------
Fixed GH-2720 - Fix disabled atrribute handling for radio form elements
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: - GH-2720
I wasn't really sure about the correct approach. This one is very minimalistic and following the existing concept of not duplicating nodes of the same name, but only storing multiple values for the same node. If you think that should be changed, let me know. Hints appreciated.
Thanks
Commits
-------
8710a13 Added example to the change log file
c9a2b49 Fixed xml encoder test script, and group `item` tags into an array
a0561e5 Replaced `item` with `*item` when parsing XML string
Discussion
----------
Replaced `item` with `*item` when parsing XML string
---------------------------------------------------------------------------
by fabpot at 2011/11/23 22:14:12 -0800
Tests do not pass:
1) Symfony\Tests\Component\Serializer\Encoder\XmlEncoderTest::testDecode
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
'key2' => 'val'
- 'A B' => 'bar'
'Barry' => Array (...)
+ 'item' => Array (...)
)
'qux' => '1'
)
.../tests/Symfony/Tests/Component/Serializer/Encoder/XmlEncoderTest.php:173
---------------------------------------------------------------------------
by fabpot at 2011/11/24 22:57:37 -0800
I don't understand the patch anymore. I don't see any use of `*item` in the code.
---------------------------------------------------------------------------
by excelwebzone at 2011/11/24 23:04:07 -0800
I run some testing and you can't use '*item' XML parser reject it. So I modified it to convert it to an array.. Look at the test script change
---------------------------------------------------------------------------
by fabpot at 2011/11/24 23:13:30 -0800
So, you probably need to change the CHANGELOG as well? You should add an example which shows a before/after example.
---------------------------------------------------------------------------
by excelwebzone at 2011/11/24 23:15:51 -0800
Yes, forgot to change that..
---------------------------------------------------------------------------
by fabpot at 2011/11/25 01:27:42 -0800
ping @Seldaek, @lsmith77
---------------------------------------------------------------------------
by Seldaek at 2011/11/25 04:16:43 -0800
There are other meta-names available in the XmlEncoder, @-something for attributes, then there is something happening with a # but I'm not quite sure what. I'm just saying, maybe *item isn't the best name, if it introduces a third metacharacter. Apart from that I'm fine with it.
---------------------------------------------------------------------------
by excelwebzone at 2011/11/25 08:45:31 -0800
Maybe we can rename it to `wildcard` instead
---------------------------------------------------------------------------
by excelwebzone at 2011/11/25 15:12:09 -0800
Any chance we can push this throw?
---------------------------------------------------------------------------
by lsmith77 at 2011/11/27 04:06:25 -0800
here is the old PR #2682
@Seldaek: i think your comment was made for an older version of the patch.
overall I am fine with the change, the Serializer component takes a fairly simple approach. it is also not designed to really produce XML or JSON cleanly from the same data. it will really only be able to output a clean API for one or the other with the same data structure.
---------------------------------------------------------------------------
by excelwebzone at 2011/12/01 06:25:24 -0800
@fabpot can we merge this change
Commits
-------
63e2a99 [CssSelector] Fixed Issue for XPathExprOr: missing prefix in string conversion
Discussion
----------
[CssSelector] Issue for XPathExprOr: missing prefix in string conversion
Hi there,
I created a small and dumb test for the issue. I looked at the original implementation and i think the problem is, that private properties are used in the parent class for xPathExprOr. so that the prefix cannot be accessd with $this->prefix in XPathExprOr
However I think the distribution for the prefix should be put in the parts of the or-sub-expressions the way it is shown in the test.
Hope this helps.
Best regards
Philipp
Commits
-------
c1a798e Another style fix.
eba9de1 Code style fix.
0b1abb3 1. Separated testing diacritic letters with additional check for mbstring in Symfony\Tests\Component\Console\Helper\FormatterHelperTest.php. 2. Added check if French locale was correctly set, skip the test otherwise in Symfony\Tests\Component\Yaml\InlineTest.php. 3. Inverted check for Windows platform since testing finding php with suffixes has meaning only in Windows in Symfony\Tests\Component\Process\PhpExecutableFinderTest.php.
Discussion
----------
Test fixes for ci.qa.php.net 2.0
This is remake of https://github.com/symfony/symfony/pull/2749 against 2.0 as requested by stof. Code style fix also applied.
Original message below:
My name is Shein Alexey and I'm writing on behalf of php-qa team. As you probably know we've set up build server for testing major php versions (5.3, 5.4 and trunk for now) on the url http://ci.qa.php.net. There's also an initiative also check these versions against major php projects with good test coverage like symfony.
In our symfony builds some tests constantly fail (see http://ci.qa.php.net/view/php-userland/job/php-symfony2/420/testReport/ for example) so here are the fixes I come up with:
Separated testing diacritic letters with additional check for mbstring in Symfony\Tests\Component\Console\Helper\FormatterHelperT
Added check if French locale was correctly set, skip the test otherwise in Symfony\Tests\Component\Yaml\InlineTest.php.
Inverted check for Windows platform since testing finding php with suffixes has meaning only in Windows in Symfony\Tests\Component\Process\PhpExecutableFinderTest.php.
There is also erratic test Symfony\Tests\Component\HttpKernel\HttpCache\HttpCacheTest:testFetchesFullResponseWhenCacheStaleAndNoValidatorsPresent which randomly fails when Age header is non-zero, I'm not sure what to do here, maybe this check should be deleted since page can have age greater than zero.
Let me know what you think.
Thank you.
Bug fix: yes
Feature add: no
Symfony2 tests pass: yes
Symfony2 tests added: yes
In general without this exception generated by php dumper container class, will cause PHP fatal error, bacause method call will look like this: `$instance->(/* arguments*/);`.
2. Added check if French locale was correctly set, skip the test otherwise in Symfony\Tests\Component\Yaml\InlineTest.php.
3. Inverted check for Windows platform since testing finding php with suffixes has meaning only in Windows in Symfony\Tests\Component\Process\PhpExecutableFinderTest.php.
Commits
-------
7a8e1b3 ChoiceType flattens nested Choices when expanded
82b0b37 Added ChoiceType test for flattening grouped Choices when expanded
Discussion
----------
Fixed#2545 - ChoiceType flattens grouped Choices when expanded
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #2545
**Problem**: With PR #2464, a bug was discovered when nested choices threw a exception during rendering.
**Solution**: Nested choices are flattened prior to creating checkbox/radio fields.
Commits
-------
11b6156 updated unittest
a931e21 get correct client IP from X-forwarded-for header
Discussion
----------
[HttpFoundation] Get correct client IP when using trusted proxy (Varnish)
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
Note: This is reopened PR #2686 for 2.0 branch.
If using trusted proxy (Varnish, ...) the client IP must be identified from X-Forwarded-For header. The header has de-facto standard format:
X-Forwarded-For : client1, proxy1, proxy2,
where the value is a comma+space separated list of IP addresses, the left-most being the farthest downstream client, and each successive proxy that passed the request adding the IP address where it received the request from. See: http://en.wikipedia.org/wiki/X-Forwarded-For
Function getClientIp should return only one client IP, not a list of all nonimportant IPs as it's now. Similar example can be seen in Cake framework: http://api.cakephp.org/view_source/request-handler-component/#line-477
There are many ways how to chose the first IP from X-Forwarded-For header. Any other faster and more reliable way is welcome.
