Commit Graph

8863 Commits

Author SHA1 Message Date
Jordi Boggiano
076bd1e99f [HttpFoundation] Add create on StreamedResponse 2012-03-15 18:40:15 +01:00
Fabien Potencier
0ba5096fe6 fixed CS 2012-03-15 17:42:47 +01:00
Fabien Potencier
4c5c7bc91e merged branch Seldaek/chainableresp (PR #3605)
Commits
-------

ff13528 [HttpFoundation] Add create method to Json & Redirect responses
1c86ad7 [HttpFoundation] Add headers arg to RedirectResponse
873da43 [HttpFoundation] Add chainability to the Response class

Discussion
----------

Chainable responses

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes

---------------------------------------------------------------------------

by cboden at 2012-03-15T15:53:43Z

+1
2012-03-15 17:39:12 +01:00
Fabien Potencier
120c2a260f merged branch vicb/twig/form_theme (PR #3576)
Commits
-------

0c83c5d [Form] Alternate syntax for form_theme

Discussion
----------

[RFC][Form] Alternate syntax for form_theme

before
`{% form_theme form _self "::base.html.twig" %}`

after
`{% form_theme form with "::base.html.twig" %}`
`{% form_theme form with varTheme %}`
`{% form_theme form with [_self, "::base.html.twig"] %}`

_the former syntax is still supported_

---------------------------------------------------------------------------

by stof at 2012-03-12T15:42:32Z

do you really need ``with`` ?

---------------------------------------------------------------------------

by vicb at 2012-03-12T15:50:41Z

it's not needed but I find it more clear (It can be drop if a consensus is reached)

---------------------------------------------------------------------------

by fabpot at 2012-03-12T17:05:46Z

+1 for `with`. Documentation for master should be updated as well.

---------------------------------------------------------------------------

by Tobion at 2012-03-13T02:26:22Z

+1 for `with`, but the syntax without array like `{% form_theme form with "::base.html.twig" %}` should also be supported

---------------------------------------------------------------------------

by vicb at 2012-03-13T07:16:55Z

`[]` are nice as they clearly indicate the ability to use multiple themes (which I think is yet to be documented). We'll pick the most popular syntax only.

---------------------------------------------------------------------------

by stof at 2012-03-13T08:16:40Z

@vicb supporting a string instead of an array should be possible when you need only one element. supporting several ones and turning it into an array is the mistake we made for 2.0

---------------------------------------------------------------------------

by hhamon at 2012-03-13T08:16:45Z

+1 for the new syntax

---------------------------------------------------------------------------

by vicb at 2012-03-13T08:29:45Z

@stof @Tobion what about using the former syntax then ?

---------------------------------------------------------------------------

by Baachi at 2012-03-13T08:32:09Z

+1 for new syntax. But it should be possible to use strings instead of arrays.

---------------------------------------------------------------------------

by stof at 2012-03-13T08:33:07Z

@vicb Having one wyntax using ``with`` and the other without will confuse users IMO. this is why I suggested allowing to pass a Twig array without adding an extra word

---------------------------------------------------------------------------

by stof at 2012-03-13T08:40:02Z

@Baachi not stringS as it is precisely what we are trying to solve :)

---------------------------------------------------------------------------

by Baachi at 2012-03-13T08:42:03Z

Oh sry. I mean __string__. :)

---------------------------------------------------------------------------

by fabpot at 2012-03-13T11:16:30Z

+1 for supporting a string or an array with the new syntax as using only one element is probably the most common use case. But then, why not supporting any valid Twig expression?

---------------------------------------------------------------------------

by vicb at 2012-03-13T11:54:51Z

Something like the latest commit ? (Tests have to be updated).

@fabpot What is the best place to handle array / non-array ? This is currenlty handled in the node but the parser might be a better place.

---------------------------------------------------------------------------

by fabpot at 2012-03-13T13:23:08Z

@vicb: I would just remove the special array case in the node as it's not needed anymore.

---------------------------------------------------------------------------

by fabpot at 2012-03-13T13:24:15Z

... and update FormExtension::setTheme() to also accept a string in which case we convert it to an array there.

---------------------------------------------------------------------------

by schmittjoh at 2012-03-13T14:26:17Z

I'd prefer a named argument instead of an ubiquitous "with" keyword which does not really tell me what's coming next.

Something like ``{% form_theme _form templates=[a, b, c] %}``. This is pretty nicely done for the assetic tags "javascripts", and "stylesheets".

---------------------------------------------------------------------------

by Tobion at 2012-03-13T16:04:26Z

@schmittjoh it would only make sense if there are multiple named arguments. With only one available it seems redundant.
Also `{% form_theme _form templates="template.html.twig" %}` is bad.

---------------------------------------------------------------------------

by vicb at 2012-03-14T07:59:08Z

I tend to agree with @Tobion but I'll have a closer look at assetic to see if we can make things more consistent.

---------------------------------------------------------------------------

by Seldaek at 2012-03-14T10:36:15Z

This would be more consistent with assetic, but assetic isn't really consistent with anything else in twig, although I see the benefits in that particular case for swapping and omitting parameters.

---------------------------------------------------------------------------

by schmittjoh at 2012-03-14T15:49:37Z

My goal was not really consistency, but I simply find it more obvious,
self-explanatory and easier to understand if you name things explicitly. We
are using the "with" keyword in several places and each time something
different is expected.

To me explicit naming is superior, but just my 2c

On Wed, Mar 14, 2012 at 4:36 AM, Jordi Boggiano <
reply@reply.github.com
> wrote:

> This would be more consistent with assetic, but assetic isn't really
> consistent with anything else in twig, although I see the benefits in that
> particular case for swapping and omitting parameters.
>
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/symfony/symfony/pull/3576#issuecomment-4495732
>

---------------------------------------------------------------------------

by Tobion at 2012-03-14T16:48:01Z

When I first saw this tag I didn't understand the role of first parameter.
So if we use johannes suggestion it should rather be `{% form_theme form=myForm templates=[a, b, c] %}`

---------------------------------------------------------------------------

by mvrhov at 2012-03-14T18:09:09Z

Before we complicate this any further can I add another thing here.
Moving to dedicated issue: Inflexible form theming #3598

---------------------------------------------------------------------------

by vicb at 2012-03-14T18:20:54Z

@mvrhov that is not the good place to discuss this (both this particular issue and GH as this is a support request).

_Have you tried `{% form_theme form.subForm ... %}`_

---------------------------------------------------------------------------

by vicb at 2012-03-15T07:39:14Z

Where do you think we should go:

1. `{% form_theme form with [_self, "::base.html.twig"] %}`
2. `{% form_theme form=form src=[_self, "::base.html.twig"] %}`

Let's discuss the structure first & not the details (i.e. src vs templates).

---------------------------------------------------------------------------

by Baachi at 2012-03-15T07:52:51Z

I tend to ```{% form_theme form with [_self, "::base.html.twig"] %}```, because its more consistent to the twig syntax.

---------------------------------------------------------------------------

by fabpot at 2012-03-15T13:10:56Z

@vicb: I like 1) more than 2) as this how the built-in tags work.

To keep BC even further, can we just remove the `with` keyword? To make it BC, we just need to have a look at extra parameters and add it to an array if they exist.

---------------------------------------------------------------------------

by Tobion at 2012-03-15T13:19:52Z

For newcomers 2) is definitely easier to understand. But it would also only make sense if you can change the parameter order, so `{% form_theme form=form src=[_self, "::base.html.twig"] %}` == ` {% form_theme src=[_self, "::base.html.twig"] form=form %}`. At the same time it reduces consistency. So for experienced developers option 1) [without "with"] is less redundant and preferable.

---------------------------------------------------------------------------

by vicb at 2012-03-15T13:53:49Z

@fabpot removing the `with` will make `Parser::parsePostfixException()` scream when providing an array of themes.
2012-03-15 16:54:08 +01:00
Fabien Potencier
0aac3613e1 merged branch jankramer/master (PR #3478)
Commits
-------

e6577de Added a 'post validation' event to the form component.

Discussion
----------

[Form] Add post-validate event

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: n/a
Fixes the following tickets: n/a
Todo: n/a

---------------------------------------------------------------------------

by fabpot at 2012-03-02T20:34:18Z

ping @bschussek

---------------------------------------------------------------------------

by vicb at 2012-03-04T09:19:53Z

I think this is a good idea (It was something missing to properly handle PersistentFile i.e. you should not persist invalid files)

---------------------------------------------------------------------------

by vicb at 2012-03-09T22:35:26Z

@jankramer please remove the second commit from this PR (see http://symfony.com/doc/current/contributing/code/patches.html) in order to make this mergeable.

---------------------------------------------------------------------------

by jankramer at 2012-03-10T09:26:04Z

@vicb done, sorry about that commit: overlooked the fact that it was on the same branch...
2012-03-15 16:46:57 +01:00
Fabien Potencier
5840d05f13 merged branch vicb/twig_cfg/2.0 (PR #3545)
Commits
-------

eee5065 [TwigBundle] Workaround a flaw in the design of the configuration (normalization)

Discussion
----------

[TwigBundle] Workaround a flaw in the design of the configuration (norma...

...lization)

see #2823

@Seldaek please comment.

---------------------------------------------------------------------------

by Seldaek at 2012-03-09T20:52:47Z

It seems fine at first glance. I don't have time to look at it in detail right now sorry.
2012-03-15 16:42:06 +01:00
Jordi Boggiano
ff13528ad0 [HttpFoundation] Add create method to Json & Redirect responses 2012-03-15 16:28:15 +01:00
Jordi Boggiano
1c86ad78ee [HttpFoundation] Add headers arg to RedirectResponse 2012-03-15 16:27:52 +01:00
Jordi Boggiano
873da434cd [HttpFoundation] Add chainability to the Response class 2012-03-15 16:27:06 +01:00
Fabien Potencier
7a54fe41ca merged 2.0 2012-03-15 15:47:03 +01:00
Fabien Potencier
de5e80b65c merged branch xavierbriand/pr-datetime-type (PR #3604)
Commits
-------

1b395f5 Revert "Throw exception when "date_widget" option is not equal to "time_widget""

Discussion
----------

Reverts commit 3c2539 to remove exception when DateTypeType has differents date and time widgets

see https://github.com/symfony/symfony/pull/1419
2012-03-15 15:43:43 +01:00
Fabien Potencier
bbd686a685 merged branch igorw/json-response (PR #3375)
Commits
-------

5fa1c70 [json-response] Add a JsonResponse class for convenient JSON encoding

Discussion
----------

[json-response] Add a JsonResponse class for convenient JSON encoding

Usage example:

    $data = array(user => $user->toArray());
    return new JsonResponse($data);

---------------------------------------------------------------------------

by drak at 2012-02-16T11:51:11Z

@fabpot - maybe we could benefit with a bit more sub-namespacing in this component.  One for Response for example and probably one for Request.

---------------------------------------------------------------------------

by Seldaek at 2012-02-16T15:07:31Z

@drak Please no. Moving the session was already a pain IMO since it was type-hinted in a few places (lack of interface, and interface doesn't include flash stuff still). Creating BC breaks just for fun like that is annoying for interop of bundles. It doesn't matter whether we have 10 or 15 classes in one directory.

---------------------------------------------------------------------------

by drak at 2012-02-17T08:33:46Z

@francodacosta The most optimal place is `__toString()`.

@Saldaek It just looks like the whole namespace is getting more cluttered.  I suggest it because things like Request/Response objects are surely only going to grow over time.  There is always the possibility to make BC for moved and renamed classes so there doesn't have to be any extra complications for making things look cleaner. Anyway, just a thought :-)

---------------------------------------------------------------------------

by stof at 2012-02-17T14:47:40Z

@drak Changing the namespace of a class is a BC break. The request and the response are used in many more places than the Session so it would be a real pain to update this. And the component is tagged with ``@api`` so BC breaks are forbidden without a good reason. The session refactoring was one as it was really an issue in the implementation, but simply renaming the class is not.

---------------------------------------------------------------------------

by fabpot at 2012-03-05T15:03:53Z

I'm -1 for adding this to the core. It does not add much value and why add a special response for JSON and not other formats?

---------------------------------------------------------------------------

by Seldaek at 2012-03-05T18:38:05Z

I think it's useful because it's a class we need in almost every project, and I don't think we're alone. It's super simple but makes me wonder every time why I have to recreate it. I don't want an additional bundle just for 3lines of code. Similarly I would say a JsonpResponse would be great, or maybe just an optional $callback arg to the json response to enable jsonp mode.

I just had someone ask me on irc how to do JSONP so while I think it's obvious and I'm sure you'd think that too, it obviously isn't to newcomers. The Response stuff is hidden behind those render methods & such and people don't realize they can simply subclass. If a few examples were in core it would be both helpful for learning and useful on a day to day basis.

As for other formats, well JSON is typically used nowadays, except when you want more fancy XML APIs, but for that the JMSSerializerBundle + FOSRestBundle are superior and we can't achieve such things in a few lines of code. I could also see a BinaryResponse or DownloadResponse or such that has proper "force-download" headers and accepts any binary stream, but that's another debate.

---------------------------------------------------------------------------

by dragoonis at 2012-03-05T19:43:05Z

I'm +1 for the concept but not commenting on how it should be implemented I'll leave that to other people.

Typically when you want to force a download you have to do ``content-disposition: attachment; filename="filehere.pdf"``
Modifying some response headers and the likes automatically for the user by returning a DownloadResponse object would be very handy..

I'm +1 for @Seldaek's point about examples of sub-classing for specific use cases. It will help with demonstrating how to do custom stuff the right way rather than people coming up with their own contraptions.

---------------------------------------------------------------------------

by stof at 2012-03-05T20:14:39Z

btw, regarding the BinaryResponse, there is a pending PR about it: #2606

---------------------------------------------------------------------------

by simensen at 2012-03-05T21:07:33Z

I'm +1 for providing reference implementations fo custom Response cases. I wanted to find best practices for handling JSONP requests/responses and couldn't find anything at all on the topic. I thought maybe extending Response might be useful but wasn't sure if that could be done safely or should be done at all.

---------------------------------------------------------------------------

by lsmith77 at 2012-03-05T22:28:01Z

@stof i think @drak was suggesting moving the class, but leaving an empty class extending from the new class in the old location to maintain BC

---------------------------------------------------------------------------

by stof at 2012-03-05T23:55:36Z

@lsmith77 This would force Symfony to use the BC class so that it does not break all typehints in existing code

---------------------------------------------------------------------------

by lsmith77 at 2012-03-06T00:22:15Z

BC hacks are never nice .. the goal would just be to eventually have all those classes and more importantly all new ones in a subnamespace. actually it might be easier to just leave all the classes in the old location and create new ones extending from the old ones. anyway .. personally i am also not such a big fan of these specialized responses .. but i guess i see FOSRestBundle as the alternative answer which makes me biased.

---------------------------------------------------------------------------

by Seldaek at 2012-03-06T07:57:36Z

I'm using FOSRestBundle when it's needed, but when you just have a small scale app that needs one or two json responses for specialized stuff it is slightly overkill. And again, newcomers probably won't know about it, and encouraging using it for simple use cases isn't exactly the best learning curve we can provide.

---------------------------------------------------------------------------

by COil at 2012-03-06T23:12:15Z

+1 for this. I have implemented such a function in all my sf1 projects, it will be the same for sf2.

---------------------------------------------------------------------------

by fabpot at 2012-03-15T13:22:27Z

Closing this PR in favor of a cookbook that explains how a developer can override the default Response class (this JSON class being a good example). see symfony/symfony-docs#1159

---------------------------------------------------------------------------

by Seldaek at 2012-03-15T13:25:08Z

Meh. Forcing people to copy paste code from the cookbook in every second project isn't exactly a step forward with regard to ease of use and user-friendliness.

---------------------------------------------------------------------------

by Seldaek at 2012-03-15T13:26:48Z

I mean following this logic, things like the X509 authentication should just be put in cookbooks too because almost nobody needs that. We have tons of code in the framework, I don't get the resistance with adding such a simple class which makes code more expressive.

---------------------------------------------------------------------------

by fabpot at 2012-03-15T13:53:07Z

because X509 authentication is not easy to get it right. Sending a JSON response is as simple as it can get:

    new Response(json_encode($data), 200, array('Content-Type' => 'application/json'));

---------------------------------------------------------------------------

by marijn at 2012-03-15T13:54:25Z

Perhaps we need a `Symfony\Extensions\{Component}` namespace for things that don't necessarily belong in the core but are truly useful...

---------------------------------------------------------------------------

by Seldaek at 2012-03-15T14:03:40Z

I still fail to see why it doesn't belong in core.. There are tons of little helpers here and there, a base controller class made only of proxies, and then this gets turned down because it is simple to do it yourself? Sure it is simple, but it's repetitive and boring too. And while it's simple when you know your way around, some people aren't really sure how to do it.

The whole point of a framework is to avoid repetitive bullshit and be more productive. @fabpot do you have any real arguments against? I can see that you don't see a big use to it, fair enough, but do you see any downside at all?
2012-03-15 15:42:36 +01:00
Xavier Briand
1b395f5351 Revert "Throw exception when "date_widget" option is not equal to "time_widget""
This reverts commit 3c2539fccb.

Conflicts:

	tests/Symfony/Tests/Component/Form/Extension/Core/Type/DateTimeTypeTest.php
2012-03-15 15:32:52 +01:00
Fabien Potencier
c4df57212b merged branch meandmymonkey/switchuser-noexception (PR #3580)
Commits
-------

0e4f789 changed test config
a98d554 [SecurityBundle] Allow switching to the user that is already impersonated (fix #2554)

Discussion
----------

[Security] Disabled exception when switching to the user that is already impersonated

Bug fix: yes-ish
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #2554
Todo: -

---------------------------------------------------------------------------

by vicb at 2012-03-13T14:31:45Z

@meandmymonkey thank you for your work on this issue. Would you have time to add functional tests ?

---------------------------------------------------------------------------

by meandmymonkey at 2012-03-13T14:49:52Z

Probably not today, but during the next few days, yes, of course.

---------------------------------------------------------------------------

by meandmymonkey at 2012-03-14T18:05:19Z

@vicb @schmittjoh Writing the tests I noticed switching to an non-existent user will not raise an exception. While it's not a security issue, it should raise an error for completeness sake, shouldn't it?

---------------------------------------------------------------------------

by vicb at 2012-03-14T20:28:52Z

I think it should (throw an `AuthenticationCredentialsNotFoundException`). _btw there is an extra `sprintf` in the original code that could be remove when attempting to exit_

---------------------------------------------------------------------------

by meandmymonkey at 2012-03-14T21:13:16Z

The problem with throwing an  `AuthenticationCredentialsNotFoundException` (or any other security exception for that matter) is that it derives from `AuthenticationException`, which means it gets caught by the framework and redirects to the login form, which is not what we want in this case.

We need to throw something 500-ish at [L89](d40b3376ec/src/Symfony/Component/Security/Http/Firewall/SwitchUserListener.php (L89)), either a generic or a (new) custom Exception.

---------------------------------------------------------------------------

by meandmymonkey at 2012-03-14T21:43:57Z

IMHO a `LogicException`would be fine, like the one used at [L117](d40b3376ec/src/Symfony/Component/Security/Http/Firewall/SwitchUserListener.php (L117)), as the error is not really about a failed authentication.

---------------------------------------------------------------------------

by vicb at 2012-03-14T21:49:04Z

I agree and btw very good job on the tests !

---------------------------------------------------------------------------

by meandmymonkey at 2012-03-14T22:12:43Z

Thanks :)

---------------------------------------------------------------------------

by vicb at 2012-03-15T08:01:13Z

Could you squash the commits, prefix the commit message with `[SecurityBundle]` and add `(fix #2554)` at the end ?

---------------------------------------------------------------------------

by meandmymonkey at 2012-03-15T08:53:12Z

Done.

---------------------------------------------------------------------------

by vicb at 2012-03-15T09:19:09Z

@fabpot this PR looks good to me.

---------------------------------------------------------------------------

by fabpot at 2012-03-15T12:50:50Z

Tests do not pass when you run them all.

---------------------------------------------------------------------------

by meandmymonkey at 2012-03-15T13:41:45Z

@fabpot @vicb With this config change, they pass when run together.

What is weird though is that the reason seems to be that the config for the profiler gets overwritten when running all tests together, while being used correctly when run alone. Any idea what can cause this? They should be isolated from each other.

The new config from 0e4f789 works, but enables the profiler for all SecurityBundle Tests... which is not strictly necessary.
2012-03-15 14:53:33 +01:00
Andreas Hucks
0e4f789084 changed test config 2012-03-15 14:32:58 +01:00
Fabien Potencier
697befc6e9 [HttpFoundation] updated the list of known mime types based on the Apache HTTPD list (closes #3563) 2012-03-15 14:08:50 +01:00
Fabien Potencier
8a574ad078 merged branch fixe/patch-3 (PR #3597)
Commits
-------

ed218bb Fixed an "Array to string conversion" warning when using PHP 5.4. Also affects Symfony2 master.

Discussion
----------

[Config] Fixed an "Array to string conversion" warning when using PHP 5.4

This also affects Symfony2 master
2012-03-15 13:45:40 +01:00
Fabien Potencier
b5d027218b merged branch mvrhov/composerTypos (PR #3600)
Commits
-------

c3dc04a fixed typos in composer file

Discussion
----------

fixed typos in composer file
2012-03-15 11:22:52 +01:00
Miha Vrhovnik
c3dc04a9e8 fixed typos in composer file 2012-03-15 11:15:25 +01:00
Andreas Hucks
a98d554472 [SecurityBundle] Allow switching to the user that is already impersonated (fix #2554)
Disabled exception when switching to the user that is already impersonated, exception is now only thrown when trying to switch to a new user.

Added an Excption exception when switching fails because target user does not exist.

Added funtional tests for switching users.
2012-03-15 01:50:14 -07:00
Drak
5ae76f1e55 [HttpFoundation] Update documentation. 2012-03-15 12:16:26 +05:45
Drak
910b5c7f83 [HttpFoudation] CS, more tests and some optimization. 2012-03-15 12:15:54 +05:45
Drak
b0466e8bb4 [HttpFoundation] Refactored BC Session class methods.
If code has not be refactored for the new API's then
you would still be using the API with one message per $name.
2012-03-15 12:14:11 +05:45
Drak
84c2e3caf7 [HttpFoundation] Allow flash messages to have multiple messages per type. 2012-03-15 11:55:52 +05:45
Fabien Potencier
85d40686ab merged branch drak/sessionhandler (PR #3493)
Commits
-------

eb9bf05 [HttpFoundation] Remove hard coded assumptions and replace with API calls.
9a5fc65 [HttpFoundation] Add more tests.
68074a2 Changelog and upgrading changes.
7f33b33 Refactor SessionStorage to NativeSessionStorage.
b12ece0 [HttpFoundation][FrameworkBundle] Separate out mock session storage and stop polluting global namespace.
d687801 [HttpKernel] Mock must invoke constructor.
7b36d0c [DoctrineBridge][HttpFoundation] Refactored tests.
39526df [HttpFoundation] Refactor away options property.
21221f7 [FrameworkBundle] Make use of session API.
cb873b2 [HttpFoundation] Add tests and some CS/docblocks.
a6a9280 [DoctrineBridge] Refactor session storage to handler.
a1c678e [FrameworkBundle] Add session.handler service and handler_id configuration property.
1308312 [HttpFoundation] Add and relocate tests.
88b1170 [HttpFoundation] Refactor tests.
2257a3d [HttpFoundation] Move session handler classes.
0a064d8 [HttpFoundation] Refactor session handlers.
2326707 [HttpFoundation] Split session handler callbacks to separate object.
bb30a44 [HttpFoundation] Prepare to split out session handler callback from session storage.

Discussion
----------

[2.1] Support PHP 5.4 \SessionHandler

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -

This patch allows us to add services, like an encryption layer into any session handler without having to alter or inherit any code across any session handler, internal or custom.

The `\SessionHandler` class exposes internal PHP's native internal session save handlers like files, memcache, and sqlite by wrapping the internal callbacks through the class giving user-space the chance to intercept, override and filter them by inheriting from `\SessionHandler`.  I've written a pretty nice use-case at http://docs.php.net/sessionhandler which really shows the power of it. I never considered how to make proper use of the `\SessionHandler` in Symfony2 until I wrote the code example you see in that documentation and also because of the `AbstractSessionStorage` base class got in the way.

It's really trivial to enable support for this in Symfony2 but requires to separate out the actual handlers because inheritance is not suitable.

Obviously, the feature will only work with internal PHP-extension provided handlers under PHP 5.4 and will already work in PHP 5.3 with any custom handler (since they all implement `\SessionHandlerInterface`). Symfony2 will also be the first framework to support these amazing features :-D

The necessary changes are really small but beautiful:

The basic idea is this: 1d55d1ff14  removed inheritance and separates out the actual session handler callbacks - the part PHP processes internally.

This is supported by an internal proxy mechanism: 10a36c901e

In terms of BC, not much changes net from 2.0:

  - We can restore the deprecated service ID: `session.storage.native`
  - We add a new service ID `session.handler` (and configuration alias `handler_id`) for the actual session handlers.  This defaults to the renamed `session.handler.native_file` session handler (same behaviour just new name and as it's a default there is no BC break).

---------------------------------------------------------------------------

by fabpot at 2012-03-03T12:15:10Z

Looks good to me. Can you update the CHANGELOG and UPGRADE file accordingly and start to update the documentation at symfony/symfony-docs? Thanks for your work, the session handling in Symfony2 is starting to become amazing!

---------------------------------------------------------------------------

by drak at 2012-03-04T11:09:31Z

@fabpot I will start working on documentation this week and get the CHANGELOG/UPGRADING committed shortly.  I'll ping when done.

---------------------------------------------------------------------------

by drak at 2012-03-14T16:48:37Z

@fabpot - This PR is ready now.
2012-03-15 06:59:41 +01:00
Fabien Potencier
26441afb7b merged branch stloyd/optimal-images (PR #3573)
Commits
-------

6ad201f [FrameworkBundle + WebProfilerBundle] Optimized images and icons with PunnyPNG

Discussion
----------

[FrameworkBundle + WebProfilerBundle] Optimized images and icons with PunnyPNG

It's addition to #3571, in this one I optimized also some gifs, and replaced some of gifs with png, i.e. `spinner.gif` is almost 1kb smaller.

---------------------------------------------------------------------------

by vicb at 2012-03-12T10:27:59Z

-1 for .gif -> .png (the original images might be used by some 3rd party bundles)

---------------------------------------------------------------------------

by stloyd at 2012-03-12T10:36:51Z

@vicb Do you know any ? Because I'm not =)

Mainly because of change you mentioned this PR is on `master`, so if @fabpot would like it, I can also add note to `UPGRADE-2.1` (`CHANGELOG-2.1` ?) file about this change.

---------------------------------------------------------------------------

by stof at 2012-03-12T11:49:03Z

@stloyd you should update the toolbar templates which contain the base64 encoded version of the icons too so that they use the same version

---------------------------------------------------------------------------

by Tobion at 2012-03-12T12:09:29Z

+1 for png

---------------------------------------------------------------------------

by stloyd at 2012-03-12T12:13:18Z

@stof Added. Thanks for reminding =)

If this PR will be merged, I will send PRs into related bundles (Doctrine, Monolog, SwitfMailer).

---------------------------------------------------------------------------

by fabpot at 2012-03-13T11:04:42Z

I agree with @vicb. No need to convert gifs to pngs; the benefits are not worth the change.

---------------------------------------------------------------------------

by Tobion at 2012-03-13T11:35:56Z

I disagree strongly. A sophisticated framework such as symfony should promote web best practices. PNGs are simply better for icons and image compression is an important topic because of web traffic consumption. Google does not research in JPG replacement (webp) for no reason.

---------------------------------------------------------------------------

by fabpot at 2012-03-14T11:51:31Z

It's not about promoting best practices and I agree that using png would have been better. The point is to not break BC when it is not strictly needed.

---------------------------------------------------------------------------

by Tobion at 2012-03-14T12:14:33Z

How about not replacing gif but instead putting png next to them and using png by default for profiler etc.
So anybody who used these gif images (still don't know who that should be outside the framework, and these are not part of the API) could still use them.

---------------------------------------------------------------------------

by fabpot at 2012-03-14T13:48:44Z

looks like a good compromise to me.

---------------------------------------------------------------------------

by flyboarder at 2012-03-14T16:07:58Z

+1 for side-by-side , gif's can always be removed later.

---------------------------------------------------------------------------

by stloyd at 2012-03-14T22:32:50Z

@fabpot Rebased and restored `.gif` files.
2012-03-15 06:53:31 +01:00
Drak
eb9bf05637 [HttpFoundation] Remove hard coded assumptions and replace with API calls. 2012-03-15 09:51:39 +05:45
Joseph Bielawski
6ad201f5ba [FrameworkBundle + WebProfilerBundle] Optimized images and icons with PunnyPNG 2012-03-14 23:31:48 +01:00
Tiago Ribeiro
ed218bb1b2 Fixed an "Array to string conversion" warning when using PHP 5.4. Also affects Symfony2 master. 2012-03-14 18:05:51 +00:00
Drak
9a5fc659d7 [HttpFoundation] Add more tests. 2012-03-14 21:28:16 +05:45
Drak
68074a2be0 Changelog and upgrading changes. 2012-03-14 21:00:04 +05:45
Drak
7f33b33aa6 Refactor SessionStorage to NativeSessionStorage.
Native here refers to the fact the session storage interacts with real PHP sessions.
2012-03-14 20:59:57 +05:45
Drak
b12ece0ff7 [HttpFoundation][FrameworkBundle] Separate out mock session storage and stop polluting global namespace.
This makes mock sessions truly mock and not to interfere with global namespace.
Add getters and setters for session name and ID.
2012-03-14 20:32:06 +05:45
Drak
d687801142 [HttpKernel] Mock must invoke constructor. 2012-03-14 20:31:59 +05:45
Drak
7b36d0cc2b [DoctrineBridge][HttpFoundation] Refactored tests. 2012-03-14 20:30:06 +05:45
Drak
39526df67c [HttpFoundation] Refactor away options property.
It does not make sense to try and store session ini directives since they can be changes outside
of the class as they are part of the global state.

Coding stan
2012-03-14 20:30:05 +05:45
Drak
21221f7cf6 [FrameworkBundle] Make use of session API. 2012-03-14 20:30:05 +05:45
Drak
cb873b250b [HttpFoundation] Add tests and some CS/docblocks. 2012-03-14 20:29:58 +05:45
Drak
a6a9280a3b [DoctrineBridge] Refactor session storage to handler. 2012-03-14 20:27:23 +05:45
Drak
a1c678ecd7 [FrameworkBundle] Add session.handler service and handler_id configuration property.
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)
2012-03-14 20:21:40 +05:45
Drak
130831248d [HttpFoundation] Add and relocate tests. 2012-03-14 20:16:03 +05:45
Drak
88b1170356 [HttpFoundation] Refactor tests. 2012-03-14 20:15:59 +05:45
Drak
2257a3d4d6 [HttpFoundation] Move session handler classes. 2012-03-14 20:15:55 +05:45
Drak
0a064d8aa1 [HttpFoundation] Refactor session handlers. 2012-03-14 20:15:51 +05:45
Drak
23267077ff [HttpFoundation] Split session handler callbacks to separate object. 2012-03-14 20:15:48 +05:45
Drak
bb30a447c5 [HttpFoundation] Prepare to split out session handler callback from session storage. 2012-03-14 20:15:44 +05:45
Fabien Potencier
07d2d2e94a merged branch alan0101c/datatransformer-tz-fix (PR #3589)
Commits
-------

17c3482 fixed timezone bug in DateTimeToTimestampTransformer

Discussion
----------

[FIX]fixed timezone bug in DateTimeToTimestampTransformer

After several trials, I found out that the original code

```php
$dateTime = new \DateTime(sprintf("@%s %s", $value, $this->outputTimezone));
```
would create a DateTime object with timezone being '0000', even though $this->outputTimezone is set to my local timezone.

so I expanded the code a bit and it's working now.

PHP Test code,

```PHP
$d = new DateTime("@1234567890 Asia/Tokyo");
echo date_format($d, 'Y/m/d H:i:s')."\n";
echo $d->getTimezone()->getName()."\n";

$d = new DateTime("now Asia/Hong_Kong");
echo date_format($d, 'Y/m/d H:i:s')."\n";
echo $d->getTimezone()->getName()."\n";
```

The output is as followed:
2009/02/13 23:31:30
+00:00
2012/03/13 03:35:55
Asia/Hong_Kong

This could be a bug of PHP,

---------------------------------------------------------------------------

by stealth35 at 2012-03-13T15:54:31Z

👍
2012-03-14 13:07:13 +01:00
Fabien Potencier
e01106f2b3 merged branch havvg/master (PR #3588)
Commits
-------

235be43 fix PropelLogger stopwatch events

Discussion
----------

fix PropelLogger "Event Propel Query X is not started."

---------------------------------------------------------------------------

by willdurand at 2012-03-13T16:00:39Z

👍
2012-03-14 12:53:48 +01:00
Fabien Potencier
d4057aed08 merged branch Spea/2.0 (PR #3590)
Commits
-------

50cb486 Fixed proxy generation in the DoctrineBundle when using Doctrine >= 2.2.0

Discussion
----------

[DoctrineBundle] Fixed proxy generation with Doctrine >= 2.2.0

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes

The issue here was, that the name of the generated Proxy files have changed in Doctrine 2.2.0, thus the autoloader in the DoctrineBundle stoped working.

This PR fixes this issue by applying different string manipulations to the given class name depending on the currently used Doctrine version.

---------------------------------------------------------------------------

by fabpot at 2012-03-14T07:13:23Z

Can you squash your commits before I merge? Thanks.

---------------------------------------------------------------------------

by Spea at 2012-03-14T09:33:10Z

Should I open a new PR when squashed the commits? Because I don't know what happens when I force the remote repository to push my squashed commits.

---------------------------------------------------------------------------

by stloyd at 2012-03-14T09:48:05Z

First you should rebase (normally), then squash and push with `--force`, then GH will automaticaly update this PR (if you push into same branch on which bases this PR).

---------------------------------------------------------------------------

by Spea at 2012-03-14T10:04:30Z

Yeah I knew about the ```--force``` option. I just wasn't sure what happens when I do it. Thank you!

---------------------------------------------------------------------------

by Spea at 2012-03-14T10:14:23Z

Squashed commits.
2012-03-14 12:53:37 +01:00
Martin Parsiegla
50cb486b67 Fixed proxy generation in the DoctrineBundle when using Doctrine >= 2.2.0 2012-03-14 11:40:15 +01:00