* 2.2: (26 commits)
[FrameworkBundle] Fixes invalid serialized objects in cache
remove dead code in yaml component
Fixed typo in UPGRADE-2.2
fixed typo
RedisProfilerStorage wrong db-number/index-number selected
[DependencyInjection] added a test for the previous merge (refs #7261)
Unset loading[$id] in ContainerBuilder on exception
Default validation message translation fix.
remove() should not use deprecated getParent() so it does not trigger deprecation internally
adjust routing tests to not use prefix in addCollection
add test for uniqueness of resources
added tests for addDefaults, addRequirements, addOptions
adjust RouteCollectionTest for the addCollection change and refactor the tests to only skip the part that really needs the config component
added tests for remove() that wasnt covered yet and special route name
refactor interator test that was still assuming a tree
adjust tests to no use addPrefix with options
adjusted tests to not use RouteCollection::getPrefix
[Routing] trigger deprecation warning for deprecated features that will be removed in 2.3
[Console] fixed StringInput binding
[Console] added string input test
...
This PR was merged into the master branch.
Commits
-------
0ef08f5 [Translator] fixed inconsistency in Translator
Discussion
----------
[2.3] [Translator] fixed inconsistency in Translator
| Q | A
| ------------- | ---
| Bug fix? | no
| New feature? | yes
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | n/a
| License | MIT
| Doc PR | n/a
---------------------------------------------------------------------------
by stof at 2013-02-17T20:44:32Z
shouldn't you make ``setFallbackLocale`` call ``setFallbackLocales`` ?
---------------------------------------------------------------------------
by fabpot at 2013-02-18T14:04:39Z
@stof: done
---------------------------------------------------------------------------
by stof at 2013-02-18T14:18:10Z
shoudn't you also update the place where the method is used (in FrameworkBundle probably) to use the non-deprecated one ?
HttpContentRenderer has been renamed to FragmentHandler.
The RendererStrategy subnamespace has been renamed to Fragment.
The strategy classes now have Fragment in their names.
ProxyRouterListener has been renamed to FragmentListener
The router_proxy configuration entry has been renamed to fragments.
This PR was merged into the master branch.
Commits
-------
76fefe3 updated CHANGELOG and UPGRADE files
f7da1f0 added some unit tests (and fixed some bugs)
f17f586 moved the container aware HTTP kernel to the HttpKernel component
2eea768 moved the deprecation logic calls outside the new HttpContentRenderer class
bd102c5 made the content renderer work even when ESI is disabled or when no templating engine is available (the latter being mostly useful when testing)
a8ea4e4 [FrameworkBundle] deprecated HttpKernel::forward() (it is only used once now and not part of any interface anyway)
1240690 [HttpKernel] made the strategy a regular parameter in HttpContentRenderer::render()
adc067e [FrameworkBundle] made some services private
1f1392d [HttpKernel] simplified and enhanced code managing the hinclude strategy
403bb06 [HttpKernel] added missing phpdoc and tweaked existing ones
892f00f [HttpKernel] added a URL signer mechanism for hincludes
a0c49c3 [TwigBridge] added a render_* function to ease usage of custom rendering strategies
9aaceb1 moved the logic from HttpKernel in FrameworkBundle to the HttpKernel component
Discussion
----------
[WIP] Kernel refactor
Currently, the handling of sub-requests (including ESI and hinclude) is mostly done in FrameworkBundle. It makes these important features harder to implement for people using only HttpKernel (like Drupal and Silex for instance).
This PR moves the code to HttpKernel instead. The code has also been refactored to allow easier integration of other rendering strategies (refs #6108).
The internal route has been re-introduced but it can only be used for trusted IPs (so for the internal rendering which is managed by Symfony itself, or by a trusted reverse proxy like Varnish for ESI handling). For the hinclude strategy, when using a controller, the URL is automatically signed (see #6463).
The usage of a listener instead of a controller to handle internal sub-requests speeds up things quite a lot as it saves one sub-request handling. In Symfony 2.0 and 2.1, the handling of a sub-request actually creates two sub-requests.
Rendering a sub-request from a controller can be done with the following code:
```jinja
{# default strategy #}
{{ render(path("partial")) }}
{{ render(controller("SomeBundle:Controller:partial")) }}
{# ESI strategy #}
{{ render(path("partial"), { strategy: 'esi' }) }}
{{ render(controller("SomeBundle:Controller:partial"), { strategy: 'esi' }) }}
{# hinclude strategy #}
{{ render(path("default1"), { strategy: 'hinclude' }) }}
```
The second commit allows to simplify the calls a little bit thanks to some nice syntactic sugar:
```jinja
{# default strategy #}
{{ render(path("partial")) }}
{{ render(controller("SomeBundle:Controller:partial")) }}
{# ESI strategy #}
{{ render_esi(path("partial")) }}
{{ render_esi(controller("SomeBundle:Controller:partial")) }}
{# hinclude strategy #}
{{ render_hinclude(path("default1")) }}
```
---------------------------------------------------------------------------
by fabpot at 2013-01-03T17:58:49Z
I've just pushed a new version of the code that actually works in my browser (but I've not yet written any unit tests). I've updated the PR description accordingly.
All comments welcome!
---------------------------------------------------------------------------
by Koc at 2013-01-03T20:11:43Z
what about `render(controller="SomeBundle:Controller:partial", strategy="esi")`?
---------------------------------------------------------------------------
by stof at 2013-01-04T09:01:01Z
shouldn't we have interfaces for the UriSigner and the HttpContentRenderer ?
---------------------------------------------------------------------------
by lsmith77 at 2013-01-04T19:28:09Z
btw .. as mentioned in #6213 i think it would make sense to refactor the HttpCache to use a cache layer to allow more flexibility in where to cache the data (including clustering) and better invalidation. as such if you are refactoring HttpKernel .. it might also make sense to explore splitting off HttpCache.
---------------------------------------------------------------------------
by fabpot at 2013-01-04T19:30:07Z
@lsmith77 This is a totally different topic. This PR is just about moving things from FrameworkBundle to HttpKernel to make them more reusable outside of the full-stack framework.
---------------------------------------------------------------------------
by fabpot at 2013-01-05T09:39:52Z
I think this PR is almost ready now. I still need to update the docs and add some unit tests. Any other comments on the whole approach? The class names? The `controller` function thingy? The URI signer mechanism? The proxy protection for the internal controller? The proxy to handle internal routes?
---------------------------------------------------------------------------
by sstok at 2013-01-05T10:08:25Z
Looks good to me 👍
---------------------------------------------------------------------------
by sdboyer at 2013-01-07T18:17:08Z
@Crell asked me to weigh in, since i'm one of the Drupal folks who's likely to work most with this.
i think i've grokked about 60% of the big picture here, and i'm generally happy with what i see. the assumption that the HInclude strategy makes about working with templates probably isn't one that we'll be able to use (and so, would need to write our own), but that's not a big deal since the whole goal here is to make strategies pluggable.
so, yeah. +1.
---------------------------------------------------------------------------
by winzou at 2013-01-09T20:21:44Z
Just for my information: will this PR be merged for 2.2 version? Thanks.
---------------------------------------------------------------------------
by stof at 2013-01-09T20:41:04Z
@winzou according to the blog post announcing the beta 1 release, yes. It is explicitly listed as being one of the reason to make it a beta instead of the first RC.
---------------------------------------------------------------------------
by winzou at 2013-01-09T20:49:36Z
OK thanks, I've totally skipped this blog post.
---------------------------------------------------------------------------
by fabpot at 2013-01-10T15:26:15Z
I've just added a bunch of unit tests and fix some bugs I found while writing the tests.
This PR was merged into the master branch.
Commits
-------
73db84f [Security] Move translations file to 'security' domain
324703a [Security] Switch to English messages as message keys
aa74769 [Security] Fix CS + unreachable code
2d7a7ba [Security] Fix `AuthenticationException` serialization
50d5724 [Security] Introduced `UsernameNotFoundException#get/setUsername`
39da27a [Security] Removed `get/setExtraInformation`, added `get/set(Token|User)`
837ae15 [Security] Add note about changed constructor to changelog
d6c57cf [FrameworkBundle] Register security exception translations
d7129b9 [Security] Fix exception constructors called in `UserChecker`
0038fbb [Security] Add initial translations for AccountStatusException childs
50e2cfc [Security] Add custom `getMessageKey` AccountStatusException childs
1147977 [Security] Fix InsufficientAuthenticationException constructor calls
79430b8 [Security] Fix AuthenticationServiceException constructor calls
42cced4 [Security] Fix AuthenticationException constructor calls
963a1d7 [Security] Add initial translations for the exceptions
ed6eed4 [Security] Add `getMessageKey` and `getMessageData` to auth exceptions
694c47c [Security] Change signature of `AuthenticationException` to match `\Exception`
Discussion
----------
[2.2][Security] AuthenticationException enhancements
Bug fix: semi
Feature addition: yes
Backwards compatibility break: yes
Symfony2 tests pass: [![Build Status](https://secure.travis-ci.org/asm89/symfony.png?branch=issue-837)](http://travis-ci.org/asm89/symfony)
Fixes the following tickets: #837
License of the code: MIT
This PR adds the functionality discussed in #837 and changes the constructor of the `AuthenticationException` to match that of `\Exception`. This PR will allow developers to show a translated (save) authentication exception message to the user. :)
*Todo:*
- Add some functional test to check that the exceptions can indeed be translated?
- Get feedback on the current English messages
---------------------------------------------------------------------------
by asm89 at 2012-07-15T14:04:11Z
ping @schmittjoh
---------------------------------------------------------------------------
by schmittjoh at 2012-07-15T14:57:32Z
Looks good to me.
While you are at the exceptions, I think we can also get rid of the "extra information" thing and replace it by explicit getters/setters. Mostly that will mean adding set/getToken, set/getUser, set/getUsername. Bundles might add custom exceptions which have other data. This will make it a bit more useful and predictable.
---------------------------------------------------------------------------
by asm89 at 2012-07-15T15:40:45Z
@schmittjoh I removed the `get/setExtraInformation` and added the more explicit getters/setters as you suggested.
---------------------------------------------------------------------------
by asm89 at 2012-07-15T19:33:15Z
@fabpot Did you reschedule this for 2.2? Why? It was originally a 2.1 ticket. I think it is an important one because at the moment there is no reliable way to show users the cause of an `AuthenticationException` without the threat of exposing sensitive information. This issue has been around for a while, see the original issue this PR refers to, or for example [this TODO comment in FOSUB](https://github.com/FriendsOfSymfony/FOSUserBundle/blob/master/Controller/SecurityController.php#L37).
The PR itself is ready to merge now. My only question that remains is about whether the actual translations should be functional tested?
---------------------------------------------------------------------------
by fabpot at 2012-07-15T19:43:19Z
We need to stop at some point. If not, we never release anything. beta3 was scheduled for today and I don't plan any other one before the first RC and I won't have time to review this PR next week. So, if you, @schmittjoh, @vicb, @stof, and a few other core devs "validate" this PR, I might consider merging it before 2.1.
---------------------------------------------------------------------------
by asm89 at 2012-07-15T19:46:09Z
@fabpot I totally agree with your point of view. I just have been trying to pickup some security issues that were still open. :)
---------------------------------------------------------------------------
by stof at 2012-07-15T19:50:29Z
This looks good to me
---------------------------------------------------------------------------
by asm89 at 2012-08-12T09:06:24Z
Since the beta period is over I assume the window was missed to get this security related PR in 2.1. If I have feedback from @fabpot I'll still try to make it mergeable asap though.
---------------------------------------------------------------------------
by fabpot at 2012-08-13T10:10:32Z
@asm89 This would indeed be considered for merging in 2.2.
---------------------------------------------------------------------------
by Antek88 at 2012-10-03T10:30:46Z
+1
---------------------------------------------------------------------------
by stof at 2012-10-04T21:27:15Z
@asm89 could you rebase this PR ? It conflicts with master
---------------------------------------------------------------------------
by fabpot at 2012-10-05T17:16:44Z
What's the status of this PR? @asm89 Have you taken all the feedback into account?
---------------------------------------------------------------------------
by stof at 2012-10-13T17:48:48Z
@asm89 ping
---------------------------------------------------------------------------
by fabpot at 2012-10-29T09:48:40Z
@asm89 If you don't have time, I can finish the work on this PR, but can you just tell me what's left?
---------------------------------------------------------------------------
by asm89 at 2012-10-29T10:02:22Z
I can pick this up, but I have two outstanding questions:
- One about adding `::create()`? https://github.com/symfony/symfony/pull/4935#discussion_r1358297
- And what is the final verdict on the messages? https://github.com/symfony/symfony/pull/4935#discussion_r1165701 The initial idea was that the exception itself have an exception message which is plain english and informative for the developer. If you want to display the 'safe' user messages you have the optional dependency on the translator. There is a comparison made with the Validator component, but in my opinion that's a different case because the violations always contain the message directed at the user and have no plain english message for the developer. Apart from that the Validator component contains it's own code for replacing `{{ }}` variables in messages (duplication? not as flexible as the translator). Concluding I'd opt for: optional dependency on translator component if you want to show 'safe' user messages + message keys.
@schmittjoh Any things to add?
---------------------------------------------------------------------------
by schmittjoh at 2012-10-29T10:14:09Z
Message keys sound good to me. I wouldn't add the ``create`` method for now.
On Mon, Oct 29, 2012 at 11:02 AM, Alexander <notifications@github.com>wrote:
> I can pick this up, but I have two outstanding questions:
>
> - One about adding ::create()? symfony/symfony#4935<https://github.com/symfony/symfony/issues/4935#discussion_r1358297>
> - And what is the final verdict on the messages? symfony/symfony#4935<https://github.com/symfony/symfony/issues/4935#discussion_r1165701>The initial idea was that the exception itself have an exception message
> which is plain english and informative for the developer. If you want to
> display the 'safe' user messages you have the optional dependency on the
> translator. There is a comparison made with the Validator component, but in
> my opinion that's a different case because the violations always contain
> the message directed at the user and have no plain english message for the
> developer. Apart from that the Validator component contains it's own code
> for replacing {{ }} variables in messages (duplication? not as
> flexible as the translator). Concluding I'd opt for: optional dependency on
> translator component if you want to show 'safe' user messages + message
> keys.
>
> @schmittjoh <https://github.com/schmittjoh> Any things to add?
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/symfony/symfony/pull/4935#issuecomment-9861016>.
>
>
---------------------------------------------------------------------------
by fabpot at 2012-10-29T10:27:37Z
As I said in the discussion about the translations, I'm -1 for the message keys to be consistent with how we manage translations everywhere else in the framework.
---------------------------------------------------------------------------
by stof at 2012-10-29T10:30:50Z
@fabpot When we changed the English translation for the validation errors in 2.1, we had to tag the commit as a BC rbeak as it was changing the source for all other translations. And if you look at the state of the files now, you will see that we are *not* using the English as source anymore in some places as some validation errors have a pluralized translation but the source has not been changed.
So I think using a key is more future-proof.
---------------------------------------------------------------------------
by asm89 at 2012-10-30T19:44:49Z
Any final decision on this? On one hand I have @stof and @schmittjoh +1 on message keys, on the other @fabpot -1. I guess it's your call @fabpot.
Edit: also @vicb seemed to be +1 on message keys earlier on.
---------------------------------------------------------------------------
by drak at 2012-11-01T20:19:00Z
I am also -1, I agree with @fabpot
---------------------------------------------------------------------------
by asm89 at 2012-11-12T09:38:51Z
@fabpot Can you please give a definite answer on this? I personally think @stof and @vicb have good points to do message keys, but with all these different people +1 and -1'ing the PR I'm lost on what it should actually do.
---------------------------------------------------------------------------
by asm89 at 2012-11-14T09:59:06Z
ping @fabpot
---------------------------------------------------------------------------
by asm89 at 2012-11-26T10:01:27Z
ping @fabpot We talked about this in Berlin. Any final thoughts on the PR? :) One idea was to do message keys + opt depend on the translator component if you want to use them, or use your own implementation.
---------------------------------------------------------------------------
by fabpot at 2012-11-26T14:01:37Z
The conclusion is: keep using plain English.
On Mon, Nov 26, 2012 at 11:01 AM, Alexander <notifications@github.com>wrote:
> ping @fabpot <https://github.com/fabpot> We talked about this in Berlin.
> Any final thoughts on the PR? :) One idea was to do message keys + opt
> depend on the translator component if you want to use them, or use your own
> implementation.
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/symfony/symfony/pull/4935#issuecomment-10709997>.
>
>
---------------------------------------------------------------------------
by Inori at 2012-11-26T15:00:22Z
is this final? if not, then +1 for message keys
---------------------------------------------------------------------------
by vicb at 2012-11-27T22:33:47Z
@fabpot I can't understand why we keep discussing this for months as this implementation use *both* keys and plain Englis, ie using keys is optional ( if it was not it would not be an issue according to #6129)
---------------------------------------------------------------------------
by asm89 at 2013-01-02T21:43:46Z
@fabpot @vicb I'll rebase this PR, fix the comments and refactor the message keys to use plain English + {{ }} syntax for the placeholders.
---------------------------------------------------------------------------
by asm89 at 2013-01-07T15:00:58Z
@fabpot If I fix this tonight, will it make the beta?
---------------------------------------------------------------------------
by fabpot at 2013-01-07T15:53:00Z
yes, definitely.
---------------------------------------------------------------------------
by asm89 at 2013-01-07T20:13:38Z
@fabpot I switched the implementation to English messages instead of message keys and fixed the final comments + rebased. Anything you want me to do after this?
Still happy with `getMessageKey()`?
* 2.1:
fixed typo
[FrameworkBundle] fixed ESI calls
[FrameworkBundle] fixed ESI calls
bumped Symfony version to 2.1.6-DEV
updated VERSION for 2.1.5
updated CHANGELOG for 2.1.5
bumped Symfony version to 2.0.21-DEV
[FrameworkBundle] fixed trusted_proxies configuration for some edge cases
[FrameworkBundle] fixed XSD for the trusted-proxies setting
updated VERSION for 2.0.20
update CONTRIBUTORS for 2.0.20
updated CHANGELOG for 2.0.20
Conflicts:
src/Symfony/Bundle/FrameworkBundle/HttpKernel.php
src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php
src/Symfony/Component/HttpKernel/Kernel.php
* 2.0:
bumped Symfony version to 2.0.21-DEV
[FrameworkBundle] fixed trusted_proxies configuration for some edge cases
[FrameworkBundle] fixed XSD for the trusted-proxies setting
updated VERSION for 2.0.20
update CONTRIBUTORS for 2.0.20
updated CHANGELOG for 2.0.20
Conflicts:
CONTRIBUTORS.md
src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php
src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/full.xml
src/Symfony/Component/HttpKernel/Kernel.php
* 2.1:
[FrameworkBundle] fixed broken tests
[FrameworkBundle] Fixed logic under test environment.
[Session] Added exception to save method
[Session] Fixed a bug with the TestListener
Added comment
[FrameworkBundle] Added tests for trusted_proxies configuration.
[FrameworkBundle] Added a check on file mime type for CodeHelper::fileExcerpt()
checked for a potentially missing key
[FrameworkBundle] used the new method for trusted proxies
remove realpath call
Conflicts:
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
* 2.0:
Added comment
[FrameworkBundle] Added tests for trusted_proxies configuration.
[FrameworkBundle] Added a check on file mime type for CodeHelper::fileExcerpt()
checked for a potentially missing key
[FrameworkBundle] used the new method for trusted proxies
remove realpath call
Conflicts:
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
This PR was merged into the master branch.
Commits
-------
7f16c1f [HttpKernel] Add DI extension configs as ressources when possible
Discussion
----------
[HttpKernel] Add DI extension configs as ressources when possible
/cc @rdohms @richardmiller
---------------------------------------------------------------------------
by vicb at 2012-11-30T11:57:48Z
btw @fabpot what about having a base class for `Extension` in the DI ? Would make it easier to re-use it when using standalone components, Di and (the suggested) Config as the greatest part of the class is not HttpKernel specific.
---------------------------------------------------------------------------
by fabpot at 2012-12-06T08:47:28Z
@vicb your suggestion makes sense.
Can you also explain the goal of this PR?
---------------------------------------------------------------------------
by vicb at 2012-12-06T09:01:58Z
The goal of this PR is to avoid having to sfcc when you modify a DI extension configuration. I think @rdohms got trapped.
---------------------------------------------------------------------------
by vicb at 2012-12-06T09:08:08Z
see https://twitter.com/rdohms/status/274059267428978688
---------------------------------------------------------------------------
by stof at 2012-12-06T09:20:54Z
I thought about it several times but never took time to implement it. It is annoying to have to clear the cache when you modify a default value in the Configuration class. So +1
The ContainerAwareTraceableEventDispatcher class was tied to both the
Symfony container and the HttpKernel profiler. It made it non reusable
in another context.
The new TraceableEventDispatcher only keeps the HttpKernel profiler
integration and is able to wrap any other event dispatcher. It makes it
reusable in frameworks using the Symfony HttpKernel component like
Silex.
The only drawback is that we don't have access to the listener
priorities in the collected data anymore (but the listeners are still
ordered correctly). The change is still worth it I think.
Commits
-------
22e9036 updated CHANGELOG
bafe890 [FrameworkBundle] changed Client::enableProfiler() behavior to fail silently when the profiler is not available (it makes it easier to write functional tests)
f41872b [FrameworkBundle] added a way to enable the profiler for the very next request in functional tests (closes#4307)
67b91e5 [HttpKernel] added a way to enable a disable Profiler
Discussion
----------
[2.2] added a way to enable the profiler for the very next request in a functional test
Bug fix: yes/no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #4307
Todo: -
License of the code: MIT
Documentation PR: should be done before merging
After merging this PR, we need to disable the profiler in the test environment in Symfony SE.
The charset was configurable in a configuration file but it never worked:
framework:
charset: ISO-8859-1
Now, like for the cache and log dirs, you can configure the charset by
overriding the getCharset() method in the app kernel:
public function getCharset()
{
return 'ISO-8859-1';
}
Commits
-------
23bb668 [FrameworkBundle][SecurityBundle] updated configuration to new method names
8775f2c [Config] replaced setInfo(), setExample() with more generic attributes
Discussion
----------
[Config] replaced setInfo(), setExample() with more generic attributes
This replaces ``setInfo`` and ``setExample`` with a more generic attribute system which provides more flexibility and is more future prove.
I have kept the specialized ``setInfo`` and ``setExample`` methods because they are a bit shorter, and also a good demonstration of what the system could be used for. However for consistency, I have renamed them to ``info()`` and ``example()`` respectively.
---------------------------------------------------------------------------
by travisbot at 2012-05-26T17:37:06Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1442720) (merged 8775f2c1 into 9e951991).
---------------------------------------------------------------------------
by stof at 2012-05-26T17:42:02Z
and you forgot to update FrameworkBundle
---------------------------------------------------------------------------
by travisbot at 2012-05-26T17:46:37Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1442764) (merged 23bb668e into 9e951991).
Revert service back to session.storage.native
Rename session.storage.native_file to session.handler.native_file (which is the default so no BC break from 2.0)
Commits
-------
cea2c7e removed unneeded local variable
924f378 updated changelog
72d5805 changed route name
41cc0d6 [FrameworkBundle] added support for HInclude
Discussion
----------
[FrameworkBundle] added support for HInclude
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: discuss
Example: https://github.com/kbond/symfony-standard/tree/hinclude
**Reopened this as I broke #2903**
References:
- http://groups.google.com/group/symfony-devs/browse_thread/thread/b74e587d6f2f87b0
- http://groups.google.com/group/symfony-devs/browse_thread/thread/8776a9833d4a5f79
- #2903
- #2865
[![Build Status](https://secure.travis-ci.org/kbond/symfony.png?branch=hinclude)](http://travis-ci.org/kbond/symfony)
---------------------------------------------------------------------------
by kbond at 2012-02-11T20:27:22Z
unless there is anything else I think this is ready, want me to squash again?
---------------------------------------------------------------------------
by fabpot at 2012-02-11T21:07:33Z
@kbond: Can you add some information about the changes in the CHANGELOG?
---------------------------------------------------------------------------
by Tobion at 2012-02-11T21:33:32Z
Do I see it correctly that we cannot set a default template on a per hinclude tag basis? But only global?
That's not really usefull when javascript is disabled because it should resemble the content to be included as an alternative.
---------------------------------------------------------------------------
by stof at 2012-02-11T21:42:15Z
@Tobion currently it is not possible. But changing the content on a tag basis may require changing the way the render tag look like (as there is no content in the tag currently) so this needs further discussion and @fabpot said he wants to merge a first implementation without it. See the discussion above.
Commits
-------
7474293 memcache profiler storage support added
Discussion
----------
[HttpKernel] [FrameworkBundle] Memcache(d) Profiler Storage added
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
There are 2 memcache PHP extensions: Memcache and MemcacheD (with "D" at the end) - both are supported.
How to use Memcache Profiler Storage (Memcache php extension is used):
change (or add if there isn't) "dsn" in framework/profiler section in config_dev.yml
```
...
framework:
...
profiler:
...
dsn: memcache://127.0.0.1/11211
...
```
How to use Memcached Profiler Storage (MemcacheD php extension is used):
change "dsn" in framework/profiler section in config_dev.yml
```
...
framework:
...
profiler:
...
dsn: memcached://127.0.0.1/11211
...
```
Last changes:
- memcached support addedd
- optimized performance (serialization done in extension, index is created with ```append``` function)
- updated to last version of Profiler (find by method, avoid duplications)
- done squash on commits
---------------------------------------------------------------------------
by stloyd at 2011-12-01T23:36:02Z
You need to add check for index name size, AFAIK memcache will fail if key is longer than 250 characters.
Also please do an `squash` for all those commits.
---------------------------------------------------------------------------
by pulzarraider at 2011-12-02T00:15:28Z
@stloyd Thanks. I will add the check for key length.
I am just starting with git. Could you please add some tutorial about squash to a documentation page: http://symfony.com/doc/2.0/contributing/code/patches.html ? It will help me (and maybe some others) to do it correct way.
---------------------------------------------------------------------------
by stof at 2011-12-02T00:19:01Z
http://help.github.com/rebase/
---------------------------------------------------------------------------
by pulzarraider at 2011-12-03T18:56:11Z
Thanks @stof, rebase done.
---------------------------------------------------------------------------
by dlsniper at 2011-12-11T14:00:17Z
Hi,
Would it be possible to either use Memcached instead of Memcache or make it configurable to use either Memcache or Memcached?
I've did a little digging on the benefits of using Memcached over Memcache (like for example: http://stackoverflow.com/questions/1442411/using-memcache-vs-memcached-with-phphttp://devzone.zend.com/1869/zendcon-sessions-episode-040-memcached-the-better-memcache-interface/ ) and maybe this will also help in not having two extensions installed for people who are using Memcached already.
Regards.
---------------------------------------------------------------------------
by pulzarraider at 2011-12-11T16:15:58Z
@dlsniper thanks for great comment. I will add memcached support.
---------------------------------------------------------------------------
by stof at 2011-12-12T20:49:00Z
@pulzarraider what is the status of this PR ? Is it still a WIP ?
---------------------------------------------------------------------------
by pulzarraider at 2011-12-12T22:58:48Z
@stof Yes, it's still WIP. I'm working on a memcached (with D at the end) support. It will be finished in the next few days.
---------------------------------------------------------------------------
by dlsniper at 2011-12-15T12:51:52Z
@pulzarraider if I can help you with the PR let me know.
---------------------------------------------------------------------------
by pulzarraider at 2012-01-08T20:22:24Z
@dlsniper @stof I've finally added memcached support and done some optimizations. Memcache(d) profiler storage is now ready.
---------------------------------------------------------------------------
by dlsniper at 2012-01-08T22:12:29Z
I'm glad you finished this @pulzarraider
Thanks! for your hard work!
+1 for this PR
@stof, @fabpot is it good to go on master?
---------------------------------------------------------------------------
by pulzarraider at 2012-01-28T19:45:56Z
@stof, @fabpot ping
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
-------
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.
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).
Builds upon aead4a9836180cabae4d47fe27c634dcd79ac8f2, which prematurely removed request scoping from the assets templating helper in all cases. The helper need only be request-scoped if one or more request-scoped packages (e.g. PathPackages) are injected into it. This change makes it possible to utilize the assets helper outside of a request (e.g. during a console script).
To ensure that the assets helper is not assigned a request scope, all asset base URL's must be defined for all packages (default and any named) and both protocols: HTTP and SSL. The included test config fixtures concisely accomplish this by specifying a single HTTPS URL as the base URL for our default and named package, since FrameworkExtension's Configuration conveniently registers this URL for both protocols.
Commits
-------
83199ae [FrameworkBundle] Fix unintuitive merging behavior for assets_base_urls
4061114 [FrameworkBundle] fixes unintuitive merging behavior
Discussion
----------
[FrameworkBundle] fixes unintuitive merging behavior
---------------------------------------------------------------------------
by fabpot at 2011/09/16 10:04:53 -0700
I think this is a "bug", no? If this is the case, then we need to fix the 2.0 branch.
---------------------------------------------------------------------------
by schmittjoh at 2011/09/17 00:34:14 -0700
It is a change in behavior, but whether or how you want to merge this is really up to you.
---------------------------------------------------------------------------
by jmikola at 2011/09/19 08:09:26 -0700
I was about to create a PR for this very same quirk, as this was causing my CDN's for various environments to all get merged together. I think we can get away with merging it directly to 2.0 since `assets_base_urls` are hardly covered in the documentation at all.
Once this gets merged, I wouldn't mind writing up a blurb on them to explain how the shorthand syntax works and this merging strategy.
---------------------------------------------------------------------------
by jmikola at 2011/09/19 08:28:21 -0700
I just noticed this PR only fixes the `base_urls` config option under `packages`. We should also correct this behavior for `assets_base_urls`, which appears further up in FrameworkBundle's Configuration.php file.
---------------------------------------------------------------------------
by jmikola at 2011/09/19 08:44:57 -0700
@schmittjoh: I have the second commit for this sitting in https://github.com/jmikola/symfony/tree/configFix (rebased on your branch) if you'd prefer to merge that into your branch to update this PR.
---------------------------------------------------------------------------
by schmittjoh at 2011/09/19 08:55:42 -0700
Merged it in. Thanks.
---------------------------------------------------------------------------
by fabpot at 2011/09/19 09:01:27 -0700
ok, I'm going to merge this into master.
@jmikola: Can you submit documentation for the new way?
Commits
-------
9fe87be More explicit default value for assets_version_format
Discussion
----------
Fixed default asset version format
This is not needed as it is already the value that is set when null in ``Symfony\Component\Templating\Asset\Package`` but that would make it clearer for people who just read the configuration.
-- add missing files
-- tweak translation command files
-- dumpers are now responsive for writting the files
-- moved the twig extractor the bridge
-- clear temp files after unit tests
-- check the presence of dumper in translation writer
-- General cleaning of the code
-- clean phpDoc
-- fix PHPDoc
-- fixing class name in configuration
-- add unit tests for extractors (php and twig)
-- moved test to correct location
-- polish the code
-- polish the code
Commits
-------
41b7a19 Updated the tests so that tests will be marked as skipped when there is no MongoDB server present!
233c7db Updated the code to follow the symfony coding standards
7b24de5 Updated the code to follow the symfony coding standard using stof his remarks
fbcbdde - Fixed a small bug - Updated some phpdoc
00fdfec Added a MongoDbProfilerStorage engine
Discussion
----------
[2.1] [HttpKernel] MongoDb storage for Profiler
As a documentbased database like MongoDB is [supposedly fantastic in logging](http://blog.mongodb.org/post/172254834/mongodb-is-fantastic-for-logging) I implemented a storage engine for the profiler that should enable us to use this database as storage for this.
Activate it using this way:
framework:
profiler:
dsn: mongodb://user:pass@location/database/collection
---------------------------------------------------------------------------
by stof at 2011/07/24 11:23:06 -0700
btw, the MongoDB session storage has already be rejected from the core so this should probably be moved to the DoctrineMongoDBBundle (even if it uses only the PHP extension and not Doctrine). @fabpot thoughts about this ?
---------------------------------------------------------------------------
by Wotre at 2011/07/24 11:52:56 -0700
Just my personal opinion, if it is prefered that way I will move this into the DoctrineMongoDBBundle.
While it is reasonable to bundle all Mongo related things together, I do believe that in the case of logging we want to avoid as many depencies as possible. Some exceptions can occur pretty early inside the framework, and it would be a shame if those aren't logged because this layer is written on top of doctrine. I'm not exactly familliar enough with the symfony internals as I only started using it a few days ago, but I can imagine that this can make a difference with some exceptions.
---------------------------------------------------------------------------
by stof at 2011/07/24 11:59:10 -0700
I don't ask you to use Doctrine in this code. It is fine to use the extension directly if it is enough.
Btw, the profiler is *not* used early. :)
---------------------------------------------------------------------------
by Wotre at 2011/07/26 10:45:05 -0700
So... Any final remark whether this should be moved to [the DoctrineMongoDBBundle](https://github.com/symfony/DoctrineMongoDBBundle) or not?
If it has to be moved, any comment on where in that bundle this should be put?
Also, if it has to be moved, how can we arrange the configuration using DI? Currently I've put a line in the FrameworkExtension file to use this engine for everything with a $dsn starting with mongodb; I imagine this kind of ugly depency can't really exist between the FrameworkBundle and another one.
Although it seems completely illogical to me, I will move it, but I do need some directions on how to elegantly do this...
---------------------------------------------------------------------------
by stof at 2011/07/26 11:03:04 -0700
@fabpot what do you think ?
---------------------------------------------------------------------------
by stof at 2011/09/04 01:28:48 -0700
@fabpot what do you think about the place where this should be done ?
Commits
-------
9f0bd03 [HttpKernel] Update tests for FileProfilerStorage
b7032bc [HttpKernel] Update FileProfileStorage to search from EOF
188a5fa [HttpKernel] Override the existing tokens in FileProfilerStorage
b1b1424 [HttpKernel] Delete folders in the profiler cache
88bc3ec [HttpKernel] Fixes standards of FileProfilerStorage
affe66c Merge remote-tracking branch 'origin/master' into new-profiler-storage
ea916c3 [HttpKernel] Coding convention for the file profiler storage
9ae2c8d [HttpKernel] CS in file storage
b415efd [HttpKernel] Add a test for semicolon in file storage test
1c1215f [HttpKernel] Use subfolders for better storage in file storage of profiler
4b1dc1f [HttpKernel] Fix the folder attribute of file storage to private
70f73e1 [HttpKernel] Fix tests for the file storage of profiler
d5313d9 [HttpKernel] Add tests for the file profiler storage
09fc0a2 [HttpKernel] Add Symfony credits to the file storage class for the profiler
d1d5892 [HttpKernel] Finalize the file storage for the profiler
2f65cf2 Add POC for file storage system
Discussion
----------
[2.1] [HttpKernel] File storage for profiler
Symfony2 has some problems when dealing with multiple concurrency queries in the SQLite storage, resulting in a timeout error or terrible lack.
I've implemented after discussions with @fabpot a filesystem storage.
Enable it in your project with :
framework:
profiler:
dsn: "file:%kernel.cache_dir%/profiler"
I also studied the possibility to store only big data string in files and rest in the SQLite, but not concluant.
Results of my measures (4 concurrency, 120 total) :
* SQLite with data : 1057ms
* SQLite without data : 615ms
* MySQL : 40ms
* This File storage : 54ms
---------------------------------------------------------------------------
by alexandresalome at 2011/07/22 12:01:10 -0700
An idea for the find method : a csv file containing ip;url;token
The iteration could be done over a big file, without loading the whole file in memory.
---------------------------------------------------------------------------
by alexandresalome at 2011/07/23 14:22:32 -0700
OK new version, with as explained previously : a CSV file containing the index + file for each profile.
The speed is similar to the speed of MySQL, and no memory overhead should occur with this solution.
---------------------------------------------------------------------------
by alexandresalome at 2011/07/23 14:37:14 -0700
Hm... Created tests, duplicated from SqliteProfilerStorageTest.
Any idea on how to put this code in common ? Is it usual to create a base class for 2 tests ?
---------------------------------------------------------------------------
by alexandresalome at 2011/07/23 14:48:39 -0700
Just tested with 24.000 requests, the 24.001'th request still takes less than 50ms to execute.
The index file is about 2Mb, and iterating the whole file is fast.
---------------------------------------------------------------------------
by alexandresalome at 2011/07/23 14:53:19 -0700
I've filled the file with 120Mb of data, requests are still less than 50ms for executing.
Iterating the index takes more than 30s (so it crashed), but it's because of the amount of lines. 30 seconds = 1,400,000 lines in this computer. The file = 1,500,000 lines
---------------------------------------------------------------------------
by alexandresalome at 2011/07/23 14:56:54 -0700
I've tested it with Linux, is someone can test with Windows
---------------------------------------------------------------------------
by stloyd at 2011/07/24 00:32:32 -0700
IMO to speedup it a bit more and not end up with "crash" (to not end with "limit" of files per directory, also to many files in dir slow down every OS) you should use same method to write as Twig, split up files in to directories. If you do this you can speed up index more, because you can create smaller one per directory.
Also you should fix CS (coding standards).
---------------------------------------------------------------------------
by stloyd at 2011/07/24 02:10:20 -0700
Tested on Win 7, seems ok. Similar speed to sqlite, dunno why ;-) but used a bit less of memory.
---------------------------------------------------------------------------
by alexandresalome at 2011/07/24 02:13:21 -0700
Did you tried with concurrent requests ? It makes sense when you use assetic
and your browser hits the application 4 times simultaneously for CSS
generation
---------------------------------------------------------------------------
by alexandresalome at 2011/07/24 02:17:23 -0700
I used Apache Benchmark for producing results :
ab -c 4 -n 120 URL
---------------------------------------------------------------------------
by alexandresalome at 2011/07/24 02:56:55 -0700
OK I used subfolders, based on last characters (because the first part of token is mostly the same between queries.
---------------------------------------------------------------------------
by stof at 2011/09/04 01:27:15 -0700
@fabpot any news about it ? Can it be merged ?
This commit also fixes exception pages when Twig is not enabled as a templating engine.
Instead of just displaying the raw Twig template as before, we now fallback to the default
exception handler introduced some time ago.
Commits
-------
9bcce9f fix tests
fc4787a fix non-extensible router
Discussion
----------
Router fix
Right now, the router is hard to overwrite (you need always a compiler pass). This commit fixes this.
---------------------------------------------------------------------------
by fabpot at 2011/07/18 01:15:36 -0700
Why do you need a complier pass to override the router?
---------------------------------------------------------------------------
by schmittjoh at 2011/07/18 01:47:47 -0700
How would you suggest to overwrite it?
Basically, I want to do something like this:
```yml
services:
router:
parent: router.default
class: MyClass
calls:
- [moreDeps, []]
```
---------------------------------------------------------------------------
by Seldaek at 2011/07/18 05:07:19 -0700
Then maybe we should somehow support redefining services with the same name while keeping the old one as parent, otherwise we need this foo.default for every service out there?
---------------------------------------------------------------------------
by fabpot at 2011/07/18 06:30:34 -0700
as @Seldeak said, why do that for the router and not all services?
---------------------------------------------------------------------------
by schmittjoh at 2011/07/18 06:38:39 -0700
I have designed the SecurityBundle this way where extension is encouraged.
---------------------------------------------------------------------------
by schmittjoh at 2011/07/18 11:15:57 -0700
I should add that this is mainly a problem for services where you still want to use the semantic configuration that is provided by the bundle. For services which are not configured by the extension, this is not so much of an issue.
Anyway, if you don't want to merge it, just close the PR. I have no problem with using a compiler pass.
---------------------------------------------------------------------------
by fabpot at 2011/07/18 11:55:11 -0700
We already have such a case with translator and translator.real. I will review the existing services to see where it makes sense to implement the same strategy.
---------------------------------------------------------------------------
by Seldaek at 2011/07/18 12:20:55 -0700
I guess you'd do it anyway, but we should pick a winner between .real and .default
---------------------------------------------------------------------------
by lsmith77 at 2011/07/18 12:26:52 -0700
I would prefer ".default" as ".real" always confused me.
The current implementation is not ready for inclusion in 2.0. It has several
known problems (security, not possible to disable it, not "cloud-compatible",
...) and it's not a must have feature anyway.
Some references:
* Security issue in FileType: https://github.com/symfony/symfony/issues/1001
* Validation fails on file, still stored in TemporaryStorage: https://github.com/symfony/symfony/issues/908
* Add a size argument & ability to configure TemporaryStorage: https://github.com/symfony/symfony/pull/748
This feature should be reworked and discussed for inclusion in 2.1.
Here are the new simplified rules:
* Required cache warmers are *always* executed when the Kernel boots for the first time;
* Optional cache warmers are *only* executed from the CLI via cache:warmup
These new rules means that all the configuration settings for the cache
warmers have been removed. So, if you want the best performance, remember to
warmup the cache when going to production.
This also fixed quite a few bugs.
* Seldaek/events:
[EventDispatcher] Removed temporary code
[FrameworkBundle] Improved code readability
[FrameworkBundle] Clarified code and fixed regression
Update Core and Security events to latest model
[EventDispatcher] Allow registration of arbitrary callbacks
[EventDispatcher] Remove useless code
[EventDispatcher] Minor memory optimization to getListeners()
[FrameworkBundle] Small optimization, remove some function calls
This in effect removes the direct link between event name and the method name on the handler.
Any callback can be given as a handler and the event name becomes an arbitrary string. Allowing for easier namespacing (see next commit)