Commits
-------
1953280 [MonologBridge] updated the class name from Monolog
96da7c8 [MonologBridge] Added the user agent check for the ChromePhpHandler
f7aa6c0 [MonologBridge] Added the Response-aware ChromePhpHandler
Discussion
----------
[MonologBridge] Added the Response-aware ChromePhpHandler
This adds an extended ChromePhpHandler based on the Response class to set the headers, similar to the extended FirePHPHandler.
This PR depends on Seldaek/monolog#58
---------------------------------------------------------------------------
by stof at 2012-02-20T16:36:47Z
@fabpot The monolog PR is merged now so this one is ready
---------------------------------------------------------------------------
by stloyd at 2012-02-20T17:11:14Z
@stof You need to rename file and class name to: [`ChromePHPHandler`](8d4ac5c0f7)
---------------------------------------------------------------------------
by fabpot at 2012-02-22T09:16:46Z
@stloyd is right. As per Symfony standard, you should use `ChromePhpHandler` for the Symfony class and `SymfonyPHPHandler` for the Monolog one.
---------------------------------------------------------------------------
by stof at 2012-02-22T09:22:27Z
@fabpot updated
Commits
-------
fb2bb65 [HttpFoundation] Fix session.cache_limiter is not set correctly
Discussion
----------
[HttpFoundation] Fix session.cache_limiter is not set correctly
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
Fixes a regression after the session refactoring where extra cache control http headers are sent.
This was previously handled by [calling session_cache_limiter(false) in NativeSessionStorage](https://github.com/symfony/symfony/blob/2.0/src/Symfony/Component/HttpFoundation/SessionStorage/NativeSessionStorage.php#L81)
---------------------------------------------------------------------------
by drak at 2012-02-21T12:23:48Z
@fabpot - this code can be merged imo.
Commits
-------
6fbd290 Improved unit tests for MemcacheSessionStorage
b4c5323 Added comma to array initializer, reverted permissions back to 644
3dd851a Use correct parameters
0e01418 Fix default if no serverpool is provided
2a65121 Fix several issues in MemccheSessionStorage which prevented it from being used correctly
Discussion
----------
Fix several issues in MemcacheSessionStorage
Apperently this could never have worked unless someone passed wrong arguments to the options.
---------------------------------------------------------------------------
by mazen at 2012-02-19T07:58:52Z
```
[marcel@development symfony]$ phpunit tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/MemcacheSessionStorageTest.php
PHPUnit 3.6.10 by Sebastian Bergmann.
Configuration read from /www/includes/vendor/symfony/phpunit.xml.dist
......
Time: 0 seconds, Memory: 3.75Mb
OK (6 tests, 11 assertions)
```
---------------------------------------------------------------------------
by lsmith77 at 2012-02-19T16:10:13Z
cc @drak
---------------------------------------------------------------------------
by drak at 2012-02-19T17:44:00Z
Looks like we could do with some tests for the constructor that also test the defaults and the internal properties. And also more extensively tests the mock to test the addServer behaviour.
---------------------------------------------------------------------------
by helmer at 2012-02-19T18:02:03Z
@mazen You've changed file permissions from 644->755 ..
---------------------------------------------------------------------------
by drak at 2012-02-21T12:25:11Z
@fabpot - with the extra tests added in 6fbd290 I believe this code is ready for merge.
Commits
-------
0a176eb [FrameworkBundle] Fix configuration errors
6745b28 [Config] Throw exceptions on invalid definition
fb27de0 [Config] cleanup
Discussion
----------
[Config] Cleanup, error detection, fixes
see #3357
---------------------------------------------------------------------------
by stloyd at 2012-02-15T10:56:00Z
@vicb As you added new exceptions, IMO you should add some tests to cover it.
---------------------------------------------------------------------------
by vicb at 2012-02-15T10:56:50Z
good point, I'll do.
---------------------------------------------------------------------------
by vicb at 2012-02-15T13:49:44Z
@stloyd that was a great idea, I realized I had miss a case. It has been added and should be covered by UT + fixes made.
I am done with the fixes, should be ready to merge.
And time to give the `PrototypedArrayNode` some more usability now.
Commits
-------
1cec4f5 [MonologBundle] added missing class to compile
Discussion
----------
[MonologBundle] added missing class to compile
`Symfony\Bridge\Monolog\Handler\DebugHandler` extends a class which was not being included in the compiled class file.
```
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: ~
Todo: ~
```
---------------------------------------------------------------------------
by stof at 2012-02-15T14:27:29Z
@kriswallsmith Can you send the same PR to the standalone repo for 2.1 ?
---------------------------------------------------------------------------
by kriswallsmith at 2012-02-15T14:30:05Z
Can I just commit to that repository directly? /ping @Seldaek
---------------------------------------------------------------------------
by stof at 2012-02-15T14:33:51Z
yeah indeed, you have the needed permissions, and the change is OK
Commits
-------
ed028d5 [WebProfilerBundle] Made is_ajax available to the view when rendering panels
Discussion
----------
[Profiler] Ajax
The first commit should be merged as `app` is not always accessible in the twig template due to the ways the templating system is used. Then there is currently no way to check if we are dealing with an ajax request in the view.
The second commit use ajax to load the panels. This should make the interface more responsive as you don't have to load the layout each time + the panels are cached. Loading via AJAX would also work if your panel does not extend the ajax layout (legacy support) - this would be less efficient though as you would load the layout and filter it out afterwards.
I am not sure if the second commit is worth merging, maybe it is useless ?
---------------------------------------------------------------------------
by stof at 2012-02-12T20:40:16Z
@vicb please rebase
---------------------------------------------------------------------------
by stof at 2012-02-13T17:48:48Z
@vicb just FYI, this conflicts with master so you will need to rebased it before it can be merged.
Otherwise, what are the remaining points ?
---------------------------------------------------------------------------
by vicb at 2012-02-13T17:57:27Z
I am still wondering if the second commit is a good idea or not ?
---------------------------------------------------------------------------
by vicb at 2012-02-13T18:28:17Z
@stof isn't the branch based on the latest master ?
---------------------------------------------------------------------------
by stof at 2012-02-13T19:32:52Z
Well, github tells me it cannot be merged automatically. so either there is a conflict, either their conflict detection failed last time you pushed.
---------------------------------------------------------------------------
by vicb at 2012-02-13T22:20:06Z
I did fail.
Should be ok now.
---------------------------------------------------------------------------
by fabpot at 2012-02-14T23:27:08Z
I'm -1 on the second commit.
---------------------------------------------------------------------------
by vicb at 2012-02-15T07:44:25Z
Thanks all for the feedback.
@fabpot ready !
---------------------------------------------------------------------------
by stof at 2012-02-15T07:46:53Z
@vicb not ready: you reverted all use of ``is_ajax`` in the templates (and you did not renamed it to the underscored name preferred by @fabpot)
---------------------------------------------------------------------------
by vicb at 2012-02-15T07:54:30Z
Well I did revert the use of "`isajax`" (prefer not to mix CS here, the scope of this PR is not to fix CS) because it is not used (this should be applied to the Doctrine profiler).
_What I mean is that `isajax` in all the Sf templates w/o the associated js is useless, basically all or nothing_
---------------------------------------------------------------------------
by vicb at 2012-02-15T08:26:41Z
btw @fabpot it makes me wonder if underscored variable names is a good idea, this will force us to mix (i.e. `is_ajax` vs `request.isxmlhttprequest`). What do you think ?
---------------------------------------------------------------------------
by fabpot at 2012-02-15T10:09:20Z
I still prefer `is_ajax` as it makes things more readable.
---------------------------------------------------------------------------
by vicb at 2012-02-15T10:16:13Z
At a larger scale how do fix the inconsistency described in my previous message ?
Options are:
* fix twig cs
* create twig cs specific to sf2
* don't fix (= keep & live with some inconsistency)
---------------------------------------------------------------------------
by stof at 2012-02-15T10:22:13Z
@vicb we also use underscores for variables used in the form themes. the official Twig CS are basically the one used by Sf2 in the form theme
---------------------------------------------------------------------------
by fabpot at 2012-02-15T10:24:46Z
I don't see any inconsistencies here. One a variable name and the other is a method call/property name. So, my vote is a don't fix.
---------------------------------------------------------------------------
by vicb at 2012-02-15T10:28:53Z
I agree but then we loose one advertised benefit a twig: _"Easy to learn: The syntax is easy to learn and has been optimized to allow web designers to get their job done fast without getting in their way"_.
The designers should now be aware of the underlying implementation (i.e. Am I dealing with a variable or a function ?)
Edit: race condition here... I agree with @stof
---------------------------------------------------------------------------
by stof at 2012-02-15T10:45:49Z
@vicb they see that ``isXmlHttpRequest`` is not a variable. They are accessing it on the ``request`` variable (well, recurse here to reach the variable)
---------------------------------------------------------------------------
by fabpot at 2012-02-15T10:46:57Z
variables and functions are underscored.
---------------------------------------------------------------------------
by vicb at 2012-02-15T10:51:28Z
I think that the beauty of Twig comes from the fact that designers do not have to wonder if "something" is an array / an object / a variable / a method / a property.
But never mind, I'll update the PR.
---------------------------------------------------------------------------
by vicb at 2012-02-15T10:55:06Z
@fabpot would you mind if I open a PR against twig to check for existence of `collector::getNotCalledListeners()` when a designer writes `collector.not_called_listeners`, then we are all happy ?
---------------------------------------------------------------------------
by vicb at 2012-02-15T11:21:55Z
ready !
---------------------------------------------------------------------------
by fabpot at 2012-02-15T11:31:50Z
The problem is that the `Twig_Template::getAttribute()` is already the bottleneck
Commits
-------
b95284e [Profiler] Fix memcache(d)
Discussion
----------
[Profiler] Fix memcache(d) storages
This fixes an ambiguity...
The memcache(d) storages have a `$lifetime` option. The name indicates that we are talking about a ttl (in seconds). This is wrong is `$lifetime` > 2592000 (=30 days), see http://fr.php.net/manual/en/memcache.set.php.
Doctrine is also [affected](e9ab2d2cca).
The ambiguity also exists in the session storage but to a lesser extend as those storage directly use memcache(d) options rather than a `$lifetime`. @drak could you confirm ?
Hopefully the Cache Component will get it right (#3211).
Commits
-------
d077ede [HttpFoundation] Increase test coverage.
cbb3e69 [HttpFoundation] Increase test coverage.
Discussion
----------
[HttpFoundation] Increase session test coverage.
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
Commits
-------
c754f28 [DoctrineBridge] Rename data fixtures loader class
af84805 [DoctrineBridge] Suggest doctrine/data-fixtures dependency
e4243a1 [DoctrineBridge] Add common data fixtures loader
Discussion
----------
[DoctrineBridge] Add common data fixtures loader
Symfony does not depend on doctrine/data-fixtures, but having this class in the bridge would enable DoctrineMongoDBBundle (and possibly others) to load fixtures without requiring DoctrineFixturesBundle to be installed.
Additionally, DoctrineFixturesBundle seems to only consist of this class and a command for loading ORM fixtures. With this in the bridge, we can possibly eliminate DoctrineFixturesBundle altogether by merging its command into DoctrineBundle.
---------------------------------------------------------------------------
by stof at 2012-02-11T19:40:17Z
The reason to have a separate bundle for the ORM fixtures was that the ORM is released whereas the DataFixtures library is still in alpha versions. So we wanted to avoid having it in Symfony itself for the 2.0 release. It could maybe change now that we have the bundle in a separate repo.
The other solution could be to put all commands related to fixtures in DoctrineFixturesBundle but IIRC @beberlei rejected a PR trying to make the same command work for ORM and PHPCR.
@beberlei what do you think about these suggestions ? And what is missing in DataFixtures to release it ? It has not changed recently except for the addition of the typehint and an update of the PHPCR purger
---------------------------------------------------------------------------
by fabpot at 2012-02-14T23:30:23Z
The Symfony bridges provide integration between a third-party library and Symfony components. IIUC, this PR is only about Doctrine and as such it is not in the scope of the bridge. It should be done "somewhere" in the Doctrine namespace (what about common for instance?).
---------------------------------------------------------------------------
by stof at 2012-02-14T23:34:19Z
@fabpot no it is not a Doctrine-only code. This extended loader is about integrating the Doctrine DataFixtures library with the DI component to allow fixtures to be container-aware (it does absolutely nothing else fancy btw). So this *is* in the scope of the bridge.
---------------------------------------------------------------------------
by jmikola at 2012-02-15T00:40:12Z
I second @stof's point here. This class is specifically for loading fixtures into application with a service container. Likewise, that is why the base class it inherits is in the common data-fixtures library.
Since this is common to both ORM and ODM, the most logical home for it would be DoctrineCommonBundle, and I believe that's what the bridge is :)
---------------------------------------------------------------------------
by stof at 2012-02-15T01:53:17Z
@jmikola not even a DoctrimeCommonBundle IMO. This is not about integrating things with the fullstack framework but with one component
On the advice of @schmittjoh, this commit adds a LogoutException class for use by LogoutListener if the CSRF token is invalid.
The handling in the Security component's ExceptionListener is modeled after AccessDeniedException, which gets wrapped in an AccessDeniedHttpException in the absence of handler service or error page (I didn't think it was appropriate to re-use those for LogoutException).
Using "securitybundletest" as the default environment for the functional test's kernel causes a PHP fatal error redeclaring the class "appSecuritybundletestDebugProjectContainer" when multiple tests (with unique names) are executed. In lieu of forcing tests to specify their own environment explicitly, we can simply append the test name into the environment.
Note: this bug may be related to PHPUnit executing multiple tests within the same process.
As each firewall is configured, its logout listener (if any) will be registered with the LogoutUrlHelper service. In a template, this helper may be used to generate relative or absolute URL's to a particular firewall's logout path. A CSRF token will be appended to the URL as necessary.
The Twig extension composes the helper service to avoid code duplication (see: #2999).
This adds several new options to the logout listener, modeled after the form_login listener:
* csrf_parameter
* intention
* csrf_provider
The "csrf_parameter" and "intention" have default values if omitted. By default, "csrf_provider" is empty and CSRF validation is disabled in LogoutListener (preserving BC). If a service ID is given for "csrf_provider", CSRF validation will be enabled. Invalid tokens will result in an InvalidCsrfTokenException being thrown before any logout handlers are invoked.
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
-------
9d6eb82 [Routing] Fix a bug in the TraceableUrlMatcher
9fc8d28 [FrameworkBundle] Fix a bug in the RedirectableUrlMatcher
4fcf9ef [Routing] Small optimization in the UrlMatcher
abc2141 [Routing] Added a missing property declaration
d86e1eb [Routing] Remove a weird dependency
Discussion
----------
[Routing] Remove a dependency on a derived class, fixes, optim
Subset of #3296 which should be acceptable.
Travis is happy.
The side effect of removing the dependency is that the `UrlMatcher` does not throw an exception any more when the scheme does not match the required scheme. I think it is better because:
* it removes a dependency on a derived class,
* it was an undocumented "feature",
* other thrown excs are component specific while this one was raw SPL.
---------------------------------------------------------------------------
by vicb at 2012-02-09T14:43:02Z
let me know what should go in 2.0 as well.
Commits
-------
b3fd2fa [Propel] Added Propel to Stopwatch
Discussion
----------
[Propel] Added Propel to Stopwatch
I've added the Stopwatch feature, everything is ready on the PropelBundle.
The trick is to log `prepare` queries in Propel, that way we got first the prepared statement, and then the executed query. That's why there is a `$isPrepare` boolean.
I kept BC if people don't update the PropelBundle too.
William
---------------------------------------------------------------------------
by stof at 2012-02-14T12:16:51Z
@willdurand toggling a flag for each call seems a bit hackish to me. Is there no better way to do it ?
---------------------------------------------------------------------------
by willdurand at 2012-02-14T12:21:38Z
Unfortunately no... But it's quite safe as we cannot change logged methods.
There is neighter start/stop methods, nor typed messages.
Le 14 févr. 2012 à 13:16, Christophe Coevoet<reply@reply.github.com> a écrit :
> @willdurand toggling a flag for each call seems a bit hackish to me. Is there no better way to do it ?
>
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/symfony/symfony/pull/3352#issuecomment-3959592
---------------------------------------------------------------------------
by stof at 2012-02-14T12:26:04Z
@willdurand then let's use this for propel 1. But please improve the logging interface for Propel 2 :)
---------------------------------------------------------------------------
by willdurand at 2012-02-14T12:34:28Z
Sure! I've added that on my todolist…
2012/2/14 Christophe Coevoet <
reply@reply.github.com
>
> @willdurand then let's use this for propel 1. But please improve the
> logging interface for Propel 2 :)
>
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/symfony/symfony/pull/3352#issuecomment-3959729
>
Commits
-------
e5edf5a [Console] Fixed CS
8abf506 [Console] Added abbreviation into search for bad command / namespace
c6203bc [Console] Added namespace suggest on bad namespace name
117359a [Console] fixed CS according to PR comment
dd0d97e [Console] Added suggest on bad command name
Discussion
----------
[Console] Added suggest on bad command name
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: namespace ?
Added something like in `git` : if user type a wrong command and if a close alternative exists, Command compenent will display a list of similar command(s).
Note : It does not work with namespace. If this PR will be merged, I could work on namespace.
see : https://github.com/fabpot/Twig/blob/master/lib/Twig/Environment.php#L1003
---------------------------------------------------------------------------
by fabpot at 2012-02-11T18:54:49Z
I think we need it to also work on namespace before merging. Is it possible?
---------------------------------------------------------------------------
by henrikbjorn at 2012-02-11T19:01:06Z
could maybe use similar_text ?
---------------------------------------------------------------------------
by lyrixx at 2012-02-11T19:01:55Z
Yes.
I will work on it asap
---------------------------------------------------------------------------
by lyrixx at 2012-02-11T20:06:43Z
I added code for namespace
@henrikbjorn I did the same logic as in twig.
---------------------------------------------------------------------------
by lyrixx at 2012-02-11T20:27:48Z
Note : Travis tests failed : http://travis-ci.org/#!/lyrixx/symfony/builds/663216
```before_script: Execution of 'php vendors.php' took longer than 600 seconds and was terminated.
Consider rewriting your stuff in AssemblyScript, we've heard it handles Web Scale™```
But tests are OK on my laptop
---------------------------------------------------------------------------
by stof at 2012-02-11T20:41:15Z
Well, it may be due to github issues during the setup of the vendors. There is some issues regularly because of the DDoS attack.
---------------------------------------------------------------------------
by lyrixx at 2012-02-11T20:58:07Z
Yes, i guessed it :-) that's why i notice it work on my laptop
---------------------------------------------------------------------------
by fabpot at 2012-02-11T23:11:08Z
This code won't work if you use abbreviations instead of the full namespace or command name.
---------------------------------------------------------------------------
by lyrixx at 2012-02-12T23:30:04Z
I added code to manage abbreviations. But I'm not sure what you are expecting. Can you try it and give me some feedback ?
P.S. : Travis failed again, but tests pass on my laptop.
Commits
-------
8935dec Added support for SVG mime type
Discussion
----------
Added support for SVG mime type
Hi, MimeTypeExtensionGuesser doesn't have a default type for SVG files, I've added this in.
Craig
Commits
-------
beb4fc0 [WIP][Locale] StubIntlDateFormatter::parse was throwing exception instead of returning Boolean false like intl implementation
b61dff7 fixed CS
Discussion
----------
[WIP][Locale] StubIntlDateFormatter::parse was throwing exception instead of returning Boolean false like intl implementation
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: ![travis.ci](https://secure.travis-ci.org/eriksencosta/symfony.png?branch=ticket_2781)
Fixes the following tickets: #2781
Todo: A test fail in 32 bit environment, executed tests only with PHP 5.3.2 and ext-intl ICU 4.2 based
Failed test:
1) Symfony\Tests\Component\Locale\Stub\StubIntlDateFormatterTest::testFormatWithDefaultTimezoneIntl
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'1969-12-31 21:00:00'
+'1969-12-31 16:00:00'
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
Commits
-------
3dd3d58 [EventListener] Fix an issue with sub-requests
71bf279 cleanup
acdb325 [StopWatch] Provide a cleaner API
acd1287 [Stopwatch] rename the section event to avoid collisions
eb540be [Profiler] Allow profiling the terminate event
4ccdc53 [HttpKernel] Cleanup of PdoProfilerStorage
814876f [HttpKernel] Tweak the code of the ProfilerListener
Discussion
----------
[Profiler] Allow profiling the terminate event
![Travis](https://secure.travis-ci.org/vicb/symfony.png?branch=profiler.terminate)
This PR is mainly about allowing to profile the terminate event (i.e. see it in the timeline panel)
There are some other tweaks.
---------------------------------------------------------------------------
by vicb at 2012-02-02T14:43:20Z
please don't merge for now. good question. bad answer.
---------------------------------------------------------------------------
by vicb at 2012-02-06T15:05:46Z
While first commits were focused on problem solving, the last brings a clean API with the ability to re-open an existing section in order to add events (re-setting event origins and merging them were just hacks).
Should be ready to be merged.
_Edit: Sorry, couldn't resist adding a private helper class again!_
---------------------------------------------------------------------------
by stof at 2012-02-06T18:30:09Z
@vicb you should stop adding such classes defined in the same file. Otherwise we will have to change the CS (and to stop telling we respect the PSR-0 standard)
---------------------------------------------------------------------------
by vicb at 2012-02-06T18:33:36Z
Once again PSR-0 is about autoloading which is exactly why I do not want in such cases. CS are an other matter and yes I think they should be changed to allow this (and I am going to submit a PR right now).
The only argument I could accept is whether this class should be private or not.
---------------------------------------------------------------------------
by vicb at 2012-02-06T19:57:06Z
Thanks for your valuable feedback @stof
---------------------------------------------------------------------------
by fabpot at 2012-02-11T20:53:03Z
Have you tested it on a project? Because it breaks my simple examples (where I have some sub-requests).
---------------------------------------------------------------------------
by vicb at 2012-02-12T09:47:23Z
my bad, should be ok now.
Commits
-------
6e9f886 [WebProfilerBundle] moved variable initialization from condition
Discussion
----------
[WebProfilerBundle] moved variable initialization from condition
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
I got fatal error for the original condition on PHP 5.3.8:
Fatal error: Call to a member function getFlashBag() on a non-object in /home/context/httpd/vendor/symfony/src/Symfony/Bundle/WebProfilerBundle/Controller/ProfilerController.php on line 150
Commits
-------
cde34fd [Form] Throwing an AlreadyBoundException in `add`, `remove`, `setParent`, `bind` and `setData` if called on a bound form
Discussion
----------
[Form] Throwing an AlreadyBoundException in `add`, `remove`, `setParent`, `bind` and `setData` if called on a bound form
Bug fix: yes
Feature addition: no
Backwards compatibility break: **yes**
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue3022)
The above mentioned methods now throw an exception because when invoked on a bound form they might cause strange side effects. You should rely on event listeners instead of modifying bound forms.
See also #3022
Commits
-------
88b826d [Propel] Fixed typo, removed useless use statement, used getData() instead of casting a PropelCollection
46d28cd [Propel] Fixed the CollectionToArray transformer
1f20fb1 [Propel] Removed useless code
3910735 [Propel] Avoid to duplicate objects
d69144c [Propel] Refactored the CollectionToArray transformer
1706671 [Propel] Fixed naming to reflect Doctrine bridge
1f277df [Propel] Removed useless ModelToIdTransformer
Discussion
----------
Cleaned the propel bridge (+ fixes)
I've fixed the `ModelChoiceList` with `multiple=true`, and I removed useless code.
This PR will ensure everything works fine, but it requires the following fix for Propel: https://github.com/propelorm/Propel/pull/286.
---------------------------------------------------------------------------
by willdurand at 2012-02-11T20:04:10Z
@cedriclombardot this PR will fix your issues with Sf2 + Propel in your admingen
@bschussek nevermind my comments on Twitter, it seems ok now
Commits
-------
cb6fdb1 [HttpFoundation] removed Session::close()
c59d880 Docblocks.
b8df162 Correct instanceof condition.
8a01dd5 renamed getFlashes() to getFlashBag() to avoid clashes
282d3ae updated CHANGELOG for 2.1
0f6c50a [HttpFoundation] added some method for a better BC
146a502 [FrameworkBundle] added some service aliases to avoid some BC breaks
93d81a1 [HttpFoundation] removed configuration for session storages in session.xml as we cannot provide a way to configure them (like before this PR anyway)
74ccf70 reverted 5b7ef11650 (Simplify session storage class names now we have a separate namespace for sessions)
91f4f8a [HttpFoundation] changed default flash bag to auto-expires to keep BC
0494250 removed unused use statements
7878a0a [HttpFoundation] renamed pop() to all() and getAll() to all()
0d2745f [HttpFoundation] Remove constants from FlashBagInterface
dad60ef [HttpFoundation] Add back get defaults and small clean-up.
5b7ef11 [HttpFoundation] Simplify session storage class names now we have a separate namespace for sessions.
27530cb [HttpFoundation] Moved session related classes to own sub-namespace.
4683915 [HttpFoundation] Free bags from session storage and move classes to their own namespaces.
d64939a [DoctrineBridge] Refactored driver for changed interface.
f9951a3 Fixed formatting.
398acc9 [HttpFoundation] Reworked flashes to maintain same behaviour as in Symfony 2.0
f98f9ae [HttpFoundation] Refactor for DRY code.
9dd4dbe Documentation, changelogs and coding standards.
1ed6ee3 [DoctribeBridge][SecurityBundle][WebProfiler] Refactor code for HttpFoundation changes.
7aaf024 [FrameworkBundle] Refactored code for changes to HttpFoundation component.
669bc96 [HttpFoundation] Added pure Memcache, Memcached and Null storage drivers.
e185c8d [HttpFoundation] Refactored component for session workflow.
85b5c43 [HttpFoundation] Added drivers for PHP native session save handlers, files, sqlite, memcache and memcached.
57ef984 [HttpFoundation] Added unit and functional testing session storage objects.
3a263dc [HttpFoundation] Introduced session storage base class and interfaces.
c969423 [HttpFoundation] Added FlashBagInterface and concrete implementation.
39288bc [HttpFoundation] Added AttributesInterface and AttributesBagInterface and concrete implementations.
Discussion
----------
[2.1][HttpFoundation] Refactor session handling and flash messages
Bug fix: yes
Feature addition: yes
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: #2607, #2591, #2717, #2773
References the following tickets: #2592, #2543, #2541, #2510, #2714, #2684
Todo: -
__Introduction__
This extensive PR is a refactor with minimal BC breaks of the `[HttpFoundation]` component's session management which fixes several issues in the current implementation. This PR includes all necessary changes to other bundles and components is documented in the `CHANGELOG-2.1` and `UPGRADING-2.1`.
__Summary of Changes__
__Session:__
- Session object now implements `SessionInterface`
__Attributes:__
- Attributes now handled by `AttributeBagInterface`
- Added two AttributeBag implementations: `AttributeBag` replicates the current Symfony2 attributes behaviour, and the second, `NamespacedAttributeBag` introduces structured namespaced representation using '/' in the key. Both are BC. `FrameworkBundle` defaults to the old behaviour.
__Flash messages:__
- Flash messages now handled by `FlashBagInterface`
- Introduced `FlashBag` which changes the way flash messages expire, they now expire on use rather than automatically, useful for ESI.
- Introduced `AutoExpireFlashBag` (default) which replicates the old automatic expiry behaviour of flash messages.
__Session Storage:__
- Introduced a base object, `AbstractSessionStorage` for session storage drivers
- Introduced a `SessionSaveHandlerInterface` when using custom session save handlers
- Introduced a `NullSessionStorage` driver which allows for unsaved sessions
- Introduced new session storage drivers for Memcache and Memcached
- Introduced new session storage drivers for PHP's native SQLite, Memcache and Memcached support
__General:__
- Fixed bugs where session attributes are not saved and all cases where flash messages would get lost
- Wrote new tests, refactored related existing tests and increased test coverage extensively.
__Rationale/Details__
I'll explain more detail in the following sections.
__Unit Tests__
All unit and functional tests pass.
__Note on Functional Testing__
I've introduced `MockFileSessionStorage` which replaces `FilesystemSessionStorage` to emulate a PHP session for functional testing. Essentially the same methodology of functional testing has been maintained but without interrupting the other session storage drivers interaction with real PHP sessions. The service is now called `session.storage.mock_file`.
__Session Workflow__
PHP sessions follow a specific workflow which is not being followed by the current session management implementation and is responsible for some unpredictable bugs and behaviours.
Basically, PHP session workflow is as follows: `open`, `read`, `write`, `close`. In between these can occur, `destroy` and `garbage collection`. These actions are handled by `session save handlers` and one is always registered in all cases. By default, the `files` save handler (internally to PHP) is registered by PHP at the start of code execution.
PHP offers the possibility to change the save handler to another internal type, for example one provided by a PHP extension (think SQLite, Memcache etc), or you can register a user type and set your own handlers. However __in all cases__ PHP requires the handlers.
The handlers are called when the following things occur:
- `open` and `read` when `session_start()` or the session autostarts when PHP outputs some display
- `destroy` when `session_regenerate_id(true)` is called
- `write` and `close` when PHP shuts down or when `session_write_close()` is called
- `garbage collection` is called randomly according to configurable probability
The next very important aspect of this PR is that `$_SESSION` plays an important part in this workflow because the contents of the $_SESSION is populated __after__ the `read` handler returns the previously saved serialised session data. The `write` handler is sent the serialised `$_SESSION` contents for save. Please note the serialisation is a different format to `serialize()`.
For this reason, any session implementation cannot get rid of using `$_SESSION`.
I wrote more details on this issue [here](https://github.com/symfony/symfony/issues/2607#issuecomment-2858300)
In order to make writing session storage drivers simple, I created a light base class `AbstractSessionStorage` and the `SessionSaveHandlerInterface` which allows you to quickly write native and custom save handler drivers.
__Flash Messages [BC BREAK]__
Flash messages currently allow representation of a single message per `$name`. Fabien designed the original system so that `$name` was equivalent to flash message type. The current PR changes the fact that Flash messages expire explicitly when retrieved for display to the user as opposed to immediately on the next page load.
The last issue fixes potential cases when flash messages are lost due to an unexpected intervening page-load (an error for example). The API `get()` has a flag which allows you to override the `clear()` action.
__Flash message translation__
This PR does not cover translation of flash messages because messages should be translated before calling the flash message API. This is because flash messages are used to present messages to the user after a specific action, and in any case, immediately on the next page load. Since we know the locale of the request in every case we can translate the message before storing. Secondly, translation is simply a string manipulation. Translation API calls should always have the raw untranslated string present because it allows for extraction of translation catalogs. For a complete answer see my answer [here](https://github.com/symfony/symfony/pull/2543#issuecomment-2858707)
__Session attribute and structured namespacing__
__This has been implemented without changing the current default behaviour__ but details are below for the alternative:
Attributes are currently stored in a flat array which limits the potential of session attributes:
Here are some examples to see why this 'structured namespace' methodology is extremely convenient over using a flat system. Let's look at an example with csrf tokens. Let's say we have multiple csrftokens stored by form ID (allowing multiple forms on the page and tabbed browsing).
If we're using a flat system, you might have
'tokens' => array('a' => 'a6c1e0b6',
'b' => 'f4a7b1f3')
With a flat system when you get the key `tokens`, you will get back an array, so now you have to analyse the array. So if you simply want to add another token, you have to follow three steps: get the session attribute `tokens`, have to add to the array, and lastly set the entire array back to the session.
$tokens = $session->get('tokens');
$tokens['c'] = $value;
$session->set('tokens', $tokens);
Doable, but you can see it's pretty long winded.
With structured namespacing you can simply do:
$session->set('c', $value, '/tokens');
There are several ways to implement this, either with an additional `$namespace` argument, or by treating a character in the `$key` as a namespacer. `NamespacedAttributeBag` treats `/` as a namespacer so you can represent `user.tokens/a` for example. The namespace character is configurable in `NamespacedAttributeBag`.
---------------------------------------------------------------------------
by marijn at 2011-12-18T15:43:17Z
I haven't read the code yet but the description from this PR and your line of thought seem very well structured.
Seems like a big +1 for me.
---------------------------------------------------------------------------
by lsmith77 at 2011-12-19T16:01:19Z
@deviantintegral could you look over this to see if it really addresses everything you wanted with PR #2510 ?
---------------------------------------------------------------------------
by deviantintegral at 2011-12-24T20:12:03Z
I've read through the documentation and upgrade notes, and I can't see anything that's obviously missing from #2510. Being able to support multiple flashes per type is the most important, and the API looks reasonable to me. Drupal does support supressing repeat messages, but that can easily be implemented in our code unless there's a compelling case for it to be a part of Symfony.
I wonder if PHP memcache support is required in Symfony given the availability of memcached. I'm not familiar with how other parts of Symfony handle it, but there is often quite a bit of confusion between the two PHP extensions. It could be simpler to remove one, or add a bit of info describing or linking to why there are two nearly identical classes.
Is it possible to make one class inherit from the other (memcached is a child of memcache)?
---------------------------------------------------------------------------
by Fristi at 2011-12-24T20:29:46Z
Interesting, maybe add: session events as I did with the current impl: https://github.com/Fristi/SessionBundle
---------------------------------------------------------------------------
by drak at 2011-12-25T00:50:03Z
@deviantintegral - I agree about the confusion between memcache and memcached but actually, it is necessary to support both because `memcached` is not available everywhere. For example on Debian Lenny and RHEL/CentOS 5, only memcache is available by default. This would preclude a massive amount of shared hosting environments. Also, it is not possible to inherit one from the other, they are completely different drivers.
@Fristi - I also thought about the events, but they do not belong as part of the standalone component as this would create a coupling to the event dispatcher. The way you have done it, ie, in a bundle is the right way to achieve it.
---------------------------------------------------------------------------
by matheo at 2011-12-25T01:12:00Z
Impressive work, looks like a big improvement and deserves a big +1
---------------------------------------------------------------------------
by datiecher at 2011-12-26T11:57:12Z
Took some time to grok all the changes in this PR but all in all it is a keeper. Specially the new flash message API, it's really nicer to work with it then the previous one.
Nicely done @drak!
---------------------------------------------------------------------------
by lsmith77 at 2012-01-02T15:00:00Z
@fabpot did you have time to review this yet? with all the work @drak has done its important that he gets some feedback soon. its clear this PR breaks BC in ways we never wanted to allow. but i think this PR also clearly explains why its necessary none the less.
---------------------------------------------------------------------------
by drak at 2012-01-02T15:41:53Z
@fabpot - I have removed the WIP status from this PR now and rebased against the current master branch.
---------------------------------------------------------------------------
by Tobion at 2012-01-07T07:13:38Z
From what I read from the IRC chat logs, the main concern of @fabpot is whether we really need multiple flash messages per type. I'm in favor of this PR and just want to add one point to this discussion.
At the moment you can add multiple flash messages of different type/category/identifier. For example you can specify one error message and one info message after an operation. I think most agree that this can be usefull.
But then it makes semantically no sense that you currently cannot add 2 info messages. This approach feels a bit half-done.
So I think this PR eliminates this paradox.
---------------------------------------------------------------------------
by drak at 2012-01-07T09:11:07Z
For reference there is also a discussion started by @lsmith77 on the mailing list at https://groups.google.com/forum/#!topic/symfony-devs/cy4wokD0mQI
---------------------------------------------------------------------------
by dlsniper at 2012-01-07T16:02:15Z
@drak I could also add the next scenario that I currently have to live with, in addition to @lsmith77 ones.
I had this issue while working on our shopping cart implementation for a customer where the customer wanted to show the unavailability of the items as different lines in the 'flash-error' section of the cart. We had to set an array as the 'flash message' in order to display that information.
So in this case for example having the flash messages types as array would actually make more sense that sending an array to the flasher. Plus the the other issue we had was that we also wanted to add another error in the message but we had to do a check to see if the flash message is an array already or we need to make it an array.
I think it's better not to impose a limit of this sort and let the users be able to handle every scenario, even if some are rare, rather that forcing users to overcome limitations such as these.
I really hope this PR gets approved faster and thanks everyone for their hard work :)
---------------------------------------------------------------------------
by Tobion at 2012-01-07T21:01:07Z
@dlsniper I think you misinterpreted my point.
---------------------------------------------------------------------------
by dlsniper at 2012-01-07T21:04:04Z
@Tobion I'm sorry I did that, I'll edit the message asap. Seems no sleep in 26 hours can cause brain not to function as intended :)
---------------------------------------------------------------------------
by lsmith77 at 2012-02-01T14:38:52Z
FYI the drupal guys are liking this PR (including the flash changes):
http://drupal.org/node/335411
---------------------------------------------------------------------------
by drak at 2012-02-01T14:51:33Z
@lsmith77 Fabien asked me to remove the changes to the flash messages so that they are as before - i.e. only one flash per name/type /cc @fabpot
---------------------------------------------------------------------------
by fabpot at 2012-02-01T14:58:23Z
To be clear, I've asked to split this PR in two parts:
* one about the session refactoring (which is non-controversial and should be merged ASAP)
* this one with only the flash refactoring
---------------------------------------------------------------------------
by drak at 2012-02-02T11:29:26Z
@fabpot this is ready to be merged now. I will open a separate PR later today for the flash messages as a bucket.
---------------------------------------------------------------------------
by fabpot at 2012-02-02T11:34:39Z
I must have missed something, but I still see a lot of changes related to the flash messages.
---------------------------------------------------------------------------
by drak at 2012-02-02T11:39:10Z
When I spoke to you you said you wanted to make the commit with flash messages with one message per name/type rather than multiple. The old flash messages behaviour is 100% maintained in `AutoExpireFlashBag` which can be the default in the framework if you wish. The `FlashBag` implementation makes Symfony2 ESI compatible.
---------------------------------------------------------------------------
by stof at 2012-02-02T11:47:38Z
@drak splitting into 2 PRs means you should not refactor the flash messages in this one but in the dedicated one.
---------------------------------------------------------------------------
by drak at 2012-02-02T12:29:43Z
@stof Yes. I discussed with Fabien over chat there are basically no changes
to flashes in `FlashBag` and `AutoExpireFlashBag` maintains the exact
behaviour as before. The FlashBag just introduces ESI compatible flashes.
There is no way to refactor the sessions without moving the flash messages
to their own bag. The next PR will propose the changes to flashes that
allow multiple messages per name/type. I can size the PR down a little
more removing the new storage drivers and so on to make the PR smaller but
that's really as far as I can go. To be clear, while the API has changed a
little for flashes, the behaviour is the same.
Rename ArraySessionStorage to make it clear the session is a mock for testing purposes only.
Has BC class for ArraySessionStorage
Added sanity check when starting the session.
Fixed typos and incorrect php extension test method
session_module_name() also sets session.save_handler, so must use extension_loaded() to check if module exist
or not.
Respect autostart settings.
Session object now implements SessionInterface to make it more portable.
AbstractSessionStorage and SessionSaveHandlerInterface now makes implementation
of session storage drivers simple and easy to write for both custom save handlers
and native php save handlers and respect the PHP session workflow.
This commit outsources the flash message processing to it's own interface.
Overall flash messages now can have multiple flash types and each type can
store multiple messages. For convenience there are now four flash types
by default, INFO, NOTICE, WARNING and ERROR.
There are two concrete implementations: one preserving the old behaviour of
flash messages expiring exactly after one page load, regardless of being
displayed or not; and the other where flash messages persist until explicitly
popped.
This commit outsources session attribute storage to it's own class.
There are two concrete implementations, one with structured namespace storage and the other
without.
Commits
-------
ac59db7 cleanup
64ea95d [WebProfilerBundle] Add redirection info to the router panel
826bd23 [FrameworkBundle] fix phpDoc of ControllerResolver::createController()
e3cf37f [HttpFoundation] RedirectResponse: add the ability to retrieve the target URL, add unit tests
50c85ae [WebProfiler] Add info to the router panel
Discussion
----------
[WIP][Profiler] Routing
former #3206 part 3 (depends on part 1 - #3280)
The goal of this PR is to fix#3264 by adding redirection infos on the router panel.
Done:
* Add info on the target url / route
To do:
* Display an accurate URL matching process (when using the RedirectableUrlMatcher)
Commits
-------
0d4d7e0 [WebProfilerBundle] Make the toolbar use the common JS
a440279 [WebProfilerBundle] Adds panel pages
762d90d [Profiler] Buid a common infrastructure
Discussion
----------
[Profiler] Provide a common infrastructure
former #3206 part 3
* base JS (provides ajax, toggle, css class helpers),
* panel pages (used only by the Doctrine panel for now).
Successfuly tested with the (future version of the) Doctrine panel.
Commits
-------
22c8f80 [Form] Fixed issues mentioned in the PR comments
3b1b570 [Form] Fixed: The "date", "time" and "datetime" types can be initialized with \DateTime objects
88ef52d [Form] Improved FormType::getDefaultOptions() to see default options defined in parent types
b9facfc [Form] Removed undefined variables in exception constructor
Discussion
----------
[Form] Fixed: "date", "time" and "datetime" fields can be initialized with \DateTime objects
Bug fix: yes
Feature addition: yes
Backwards compatibility break: **yes**
Symfony2 tests pass: yes
Fixes the following tickets: #3288
Todo: -
![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue3288)
Fixed exception that was thrown when doing:
$builder->add('createdAt', 'date', array('data' => new \DateTime()));
On a side note, the options passed to `FieldType::getDefaultOptions` now always also contain the default options of any parent types. This is necessary if you want to be independent of how `getDefaultOptions` is implemented in the parent type and still rely on the already defined values.
As a result, `FieldType::getParent` doesn't see default options anymore. This shouldn't be a big problem, because this method only relies on options in few cases. If it does, options now need to be checked for existence with `isset` before being used (BC break).
---------------------------------------------------------------------------
by bschussek at 2012-02-09T16:14:46Z
@fabpot Ready to merge.
---------------------------------------------------------------------------
by bschussek at 2012-02-10T12:15:04Z
@fabpot Ready to merge
Commits
-------
da2447e [Form] Fixed MergeCollectionListener when used with a custom property path
b56502f0 [Form] Added getParent() to PropertyPath
7e5104e [Form] Fixed MergeCollectionListener for the case that the form's data is updated by the data mapper (as happening in CollectionType)
Discussion
----------
[Form] Fixed MergeCollectionListener for the case that the form's data is updated by the data mapper
Bug fix: yes
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #3293, #1499
Todo: -
![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue3293)
This fixes CollectionType to properly use adders and removers. Apart from that, adders and removers now work with custom property paths. PropertyPath was extended for a method `getParent`.
---------------------------------------------------------------------------
by bschussek at 2012-02-10T07:42:13Z
@fabpot Ready to merge.
Commits
-------
0a4519d [Form] Fixed duplicate errors on forms with "error_bubbling"=false
Discussion
----------
[Form] Fixed duplicate errors on forms with "error_bubbling"=false
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #2308
Todo: -
![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue2308)
Commits
-------
6a45a41 [Form] Fixed Form::bindRequest() when used on a form without children
Discussion
----------
[Form] Fixed Form::bindRequest() when used on a form without children
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #2553
Todo: -
![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue2553)
Commits
-------
411a0cc [Validator] Added GroupSequenceProvider to changelog
815c769 [Validator] Renamed getValidationGroups to getGroupSequence
d84a2e4 [Validator] Updated test expectations
9f2310b [Validator] Fixed typos, renamed hasGroupSequenceProvider
e0d2828 [Validator] GroupSequenceProvider tests improved, configuration changed
c3b04a3 [Validator] Changed GroupSequenceProvider implementation
6c4455f [Validator] Added GroupSequenceProvider
Discussion
----------
[Validator] Added GroupSequenceProvider
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: ![](https://secure.travis-ci.org/blogsh/symfony.png?branch=dynamic_group_sequence)
As discussed in #3114 I implemented the "GroupSequenceProvider" pattern for the validator component. It allows the user to select certain validation groups based on the current state of an object. Here is an example:
/**
* @Assert\GroupSequenceProvider("UserGroupSequnceProvider")
*/
class User
{
/**
* @Assert\NotBlank(groups={"Premium"})
*/
public function getAddress();
public function hasPremiumSubscription();
}
class UserGroupSequenceProvider implements GroupSequenceProviderInterface
{
public function getValidationGroups($user)
{
if ($user->hasPremiumSubscription()) {
return array('User', 'Premium');
} else {
return array('User');
}
}
}
With this patch there are two mechanisms to define the group sequence now. Either you can use @GroupSequence to define a static order of validation groups or you can use @GroupSequenceProvider to create dynamic validation group arrays.
The ClassMetadata therefore has methods now which implement quite similar things. The question is whether it would make sense to interpret the static group sequence as a special case and create something like a DefaultGroupSequenceProvider or StaticGroupSequenceProvider which is assigned by default. This would cause a BC break inside the validator component.
---------------------------------------------------------------------------
by bschussek at 2012-01-28T13:39:54Z
I like the implementation, but I think we should differ a little bit from Java here.
1. `GroupSequenceProviderInterface` should be implemented by the domain classes themselves (`User`), not by a separate class.
2. As such, the parameter `$object` from `getValidationGroups($object)` can be removed
3. `ClassMetadata::setGroupSequenceProvider()` should accept a boolean to activate/deactivate this functionality. Also the check for the interface (does the underlying class implement it?) should be done here
Apart from that, special cases need to be treated:
* A definition of a group sequence and a group sequence provider in the same `ClassMetadata` should not be allowed. Either of them must not be set.
* Metadata loaders must take care of settings made by parent classes. If `Animal` is extended by `Dog`, `Animal` defines a group sequence (or group sequence provider) and `Dog` a group sequence provider (or group sequence), only the setting of `Dog` should apply
---------------------------------------------------------------------------
by blogsh at 2012-01-28T21:25:37Z
Changes of the latest commit:
- GroupSequenceProviderInterface has to be implemented by the domain class
- The annotation/configuration options let the user define whether the provider is activated or not (is this neccessary at all?)
- An error is thrown if the user wants to use static group sequences and the provider simultaneously
At the moment neither the static group sequence nor the provider is inherited from parent classes or interfaces. I don't know if it would make sense to enable this feature. There could be problems if a user wants to define a static group sequence in the parent class and a sequence provider in the child class.
---------------------------------------------------------------------------
by bschussek at 2012-01-30T13:07:04Z
> There could be problems if a user wants to define a static group sequence in the parent class and a sequence provider in the child class.
In this case, the setting in the child class should override the setting of the parent class.
But we can leave this open for now. As it seems, [this issue is unresolved in Hibernate as well](https://hibernate.onjira.com/browse/HV-467).
---------------------------------------------------------------------------
by blogsh at 2012-01-30T22:54:41Z
Okay, finally I managed to upload the latest commit. If you got a bunch of notifications or so I'm sorry, but I had to revert some accidental changes in the commit :(
I've rewritten the tests and have removed the "active" setting in the XML configuration.
---------------------------------------------------------------------------
by blogsh at 2012-02-02T15:24:01Z
Okay, typos are fixed now and `hasGroupSequenceProvider` has been renamed to `isGroupSequenceProvider`. I also had to adjust some tests after the rebase with master.
---------------------------------------------------------------------------
by bschussek at 2012-02-03T09:25:19Z
Looks good.
@fabpot 👍
---------------------------------------------------------------------------
by fabpot at 2012-02-03T09:46:52Z
Can you add a note in the CHANGELOG before I merge? Thanks.
---------------------------------------------------------------------------
by blogsh at 2012-02-09T12:31:27Z
@fabpot done
Commits
-------
ba3b321 [ClassLoader] Added a class map file generator utility
Discussion
----------
[ClassLoader] Added a class map file generator utility
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #2407
---------------------------------------------------------------------------
by lsmith77 at 2011-11-30T19:33:52Z
@thesalla do you have time/interest to finish this up?
---------------------------------------------------------------------------
by thesalla at 2011-12-01T09:55:50Z
sure (will attempt at least), but no sooner than tomorrow
---------------------------------------------------------------------------
by stof at 2011-12-12T20:41:41Z
@thesalla @fabpot what is the state of this PR ?
---------------------------------------------------------------------------
by thesalla at 2011-12-17T19:36:58Z
If you don't have any other suggestions or ideas, I'd consider it finished.
---------------------------------------------------------------------------
by lsmith77 at 2012-01-24T09:27:12Z
ping
---------------------------------------------------------------------------
by fabpot at 2012-02-02T08:21:38Z
@thesalla: Can you rename the ClassMapDumper class to ClassMapGenerator and then squash your commit before the merge? Thanks.
---------------------------------------------------------------------------
by thesalla at 2012-02-04T12:55:48Z
@fabpot done.
Commits
-------
928e352 Change the array access used in UniqueEntityValidator
Discussion
----------
[DoctrineBridge] Change the array access used in UniqueEntityValidator
MongoDB ODM Cursor does not implement ArrayAccess and therefor using
`$result[0]` will fail. `reset()` rewinds the array and returns the
first element value.
DoctrineMongoDBBundle#77
/cc @beberlei
---------------------------------------------------------------------------
by henrikbjorn at 2012-02-06T08:09:25Z
Made a mistake, the findBy call would still return a cursor. Changed the findBy to findOneBy we only want one result anyway.
---------------------------------------------------------------------------
by stof at 2012-02-06T08:11:26Z
findOneBy is wrong: you will not detect duplicate anymore if you return a single element
---------------------------------------------------------------------------
by henrikbjorn at 2012-02-06T08:28:03Z
@stof before it was only the first result that was used anyway so if it had found 3 results it would only have used the first one. Performance is apparently the biggest issue with findOneBy so this have been reverted.
---------------------------------------------------------------------------
by stof at 2012-02-06T08:31:17Z
@henrikbjorn no, we use other results too: if ``count()`` is not 1, the validation fails
---------------------------------------------------------------------------
by stof at 2012-02-06T08:36:06Z
Btw, running the testsuite would have tell you there is an issue when using findOneBy as it was breaking a test :)
---------------------------------------------------------------------------
by bschussek at 2012-02-06T10:45:44Z
Why doesn't this validator use a SELECT COUNT anyway?
---------------------------------------------------------------------------
by stof at 2012-02-06T10:57:00Z
@bschussek because we need to check if an existing value is for the same object or another one. Otherwise, the valdiation would fail as soon as you are editing an existing object without changing the unique value
---------------------------------------------------------------------------
by henrikbjorn at 2012-02-06T11:03:40Z
@stof @bschussek other changes that should be done? Else it should be ready to be merged ?
---------------------------------------------------------------------------
by henrikbjorn at 2012-02-06T13:00:57Z
@stof done an rebased.
---------------------------------------------------------------------------
by bschussek at 2012-02-06T13:04:44Z
👍 /cc @fabpot
Commits
-------
b65a997 [Form] Added test ChoiceList::testGetChoicesForValuesCorrectOrderingOfResult for correct ordering check
a60daff [Form] Fixed incorrect sorting in ChoiceList::getChoicesForValues
Discussion
----------
[Form] Fixed incorrect sorting in ChoiceList::getChoicesForValues
ChoiceList::getChoicesForValues return collection with sorting, that not corresponding to the sorting for input keys
---------------------------------------------------------------------------
by bschussek at 2012-02-03T21:40:49Z
Can you adapt the test cases to fail for this case please? (probably by use of assertSame instead of assertEquals in one of the existing tests)
---------------------------------------------------------------------------
by grizlik at 2012-02-04T08:41:44Z
I need to create a new test or modify an existing one?
---------------------------------------------------------------------------
by bschussek at 2012-02-04T09:58:04Z
Something like this in ChoiceListTest:
public function testGetChoicesForValuesReverseOrderedValues()
{
$values = array('2', '1');
$this->assertSame(array($this->obj3, $this->obj2), $this->list->getChoicesForValues($values));
}
Commits
-------
78c1451 [Validator] Throwing exception in ConstraintValidator::setMessage() if initialize() wasn't called
Discussion
----------
[Validator] Throwing exception in ConstraintValidator::setMessage() if initialize() wasn't called
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue3269)
---------------------------------------------------------------------------
by Maks3w at 2012-02-05T09:52:21Z
One problem was that. Now I initialized the unit test with a execution
context mockup and works good.
Another problem I found is in the rules for upgrade from 2.0 to
2.1:
In v2.0: with setMessages() only one argument is mandatory ($message)
In v2.1: With the new way, addViolation(), then you need 2 arguments.
But, in v2.0 with the new way, addViolation, required 3 arguments.
So for preserve forward and backward compatibility, you need pass 3
arguments to addViolation() I think that it is helpfull and could be written in the
doc.
At the last one ¿What is the purpose of the third argument? ¿Can I pass
any object as invalid value?
Thanks
---------------------------------------------------------------------------
by bschussek at 2012-02-05T10:45:16Z
The last argument should always be the validated value that caused the constraint to fail.
Commits
-------
80682ba Fixed CS
007de8c [Tests] [Propel] Added some tests for the ModelChoiceList class
8257efd [Propel] Refactored the whole ModelChoiceList
Discussion
----------
Fix propel bridge
Ok, so I rewrote the `ModelChoiceList` based on the `EntityChoiceList`, and I provided the same test suite.
Sorry for the last PR, it was a fix to avoid fatal..
Cheers,
William
fixed cs
Small refactoring for Finder support
If class name found, return
Find multiple classes and namespaces in the same file
fixed problems with inheritance and non-php files
Renamed ClassMapDumper to ClassMapGenerator
fixed error with splfileinfo
Commits
-------
4847d3a renamed command
e97af0b code fixes
df94282 [FrameworkBundle] removed unnecessary DebugCommand
fa32885 [SecurityBundle] added configuration info
2f8ad93 [MonologBundle] added configuration info
9757958 [FrameworkBundle] added configuration info
58939f1 [TwigBundle] added configuration docs
8dc40e4 [FrameworkBundle] added config:dump console command
Discussion
----------
Config dump command
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #1663
Todo: add more config info/examples
[![Build Status](https://secure.travis-ci.org/kbond/symfony.png?branch=config_dump_command)](http://travis-ci.org/kbond/symfony)
This is a config dump command based on the additions of PR #1099. This was initially part of that PR and there is some discussion there about it (https://github.com/symfony/symfony/pull/1099)
### Usage:
1. dump by root node: ``app/console config:dump framework``
2. dump by bundle name: ``app/console config:dump FrameworkBundle``
A few issues/notes:
* Only dumps to yaml
* Only 1 configuration per bundle (this was brought by @stof here: https://github.com/symfony/symfony/pull/1099#issuecomment-1242993)
* Works out of the box for most bundles but not ones that use a non-standard ``Configuration`` class (such as the assetic bundle). In this case ``Extension::getConfiguration()`` must be configurated.
I have used it to create some (most) of the config reference docs. It works fine but I find it somewhat crude, any suggestions to improve it would be appreciated.
---------------------------------------------------------------------------
by kbond at 2012-01-24T21:00:43Z
Few more issues:
1. Should I abstract the logic to a "normalizer" class that converts the Configuration class into a manageable array? I struggle with this idea because isn't that what ``TreeBuilder`` basically is?
2. @stof made a good point that ``config:dump`` doesn't really describe what this does. Would dumping your config be useful? Perhaps ``config:dump framework`` dumps the config for your project while ``config:dump --ref framework`` dumps the default reference?
---------------------------------------------------------------------------
by stof at 2012-01-24T21:18:15Z
@kbond you cannot really dump the config. Part of it does not go through these extensions at all. And it does not make much sense anyway IMO.
the command as is does the right job IMO (i.e. dumping a reference for the extension). But its name should be improved
---------------------------------------------------------------------------
by kbond at 2012-01-24T21:20:51Z
``config:reference`` perhaps?
---------------------------------------------------------------------------
by fabpot at 2012-02-02T10:05:19Z
This command is about displaying the default configuration for a given bundle. So, what about `config:dump-reference`? As I understand, the command name is the last element to figure out before merging, right?
---------------------------------------------------------------------------
by stof at 2012-02-02T10:19:49Z
@fabpot indeed.
---------------------------------------------------------------------------
by stof at 2012-02-02T10:34:16Z
and +1 for ``config:dump-reference``
---------------------------------------------------------------------------
by Tobion at 2012-02-02T12:08:03Z
why not use the words you chose yourself: `config:dump-default`
I think it's more explicit.
Commits
-------
8321593 [Form] DRYed ChoiceType
0753cee [Form] Fixed read_only attribute for expanded fields
Discussion
----------
[Form] Fixed read_only attribute for expanded fields
Expanded choice widgets lose the knowledge of read_only attribute value.
Fixes bug introduced by #3193
---------------------------------------------------------------------------
by helmer at 2012-02-02T16:24:50Z
Please hold before merging, @bschussek had some thoughts about my changes in ``ChoiceType``, waiting for feedback.
---------------------------------------------------------------------------
by bschussek at 2012-02-02T16:33:12Z
I'm fine with the refactoring then, but please split it into two commits at least. The changes in ChoiceType have nothing in common with the actual issue here.
---------------------------------------------------------------------------
by helmer at 2012-02-02T19:40:39Z
Tests added.
---------------------------------------------------------------------------
by bschussek at 2012-02-03T10:14:32Z
Great, thanks.
@fabpot 👍
Commits
-------
8714d79 [Form] Simplified code in MergeCollectionListener
8ab982a [Form] Fixed: Custom add and remove method are not invoked if disallowed
02f61ad [Form] Renamed choice and collection options "adder_prefix" and "remover_prefix" to "add_method" and "remove_method" and allowed to specify full method names
b393774 [Form] Used direct method access in MergeCollectionListener instead of Reflection to avoid problems when using class hierarchies
d208f4e [Form] Made it possible to use models with only either addXxx() or removeXxx()
Discussion
----------
[Form] Fixed edge cases in MergeCollectionListener
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue3239)
Fixes an issue mentioned in the comments of #3239
see https://github.com/symfony/symfony/pull/3239#issuecomment-3776312
---------------------------------------------------------------------------
by bschussek at 2012-02-02T12:12:17Z
Wait a minute before merging this.
---------------------------------------------------------------------------
by bschussek at 2012-02-02T13:01:55Z
@fabpot Ready to merge
Commits
-------
9b0245b [Form] Made prefix of adder and remover method configurable. Adders and removers are not called if "by_reference" is disabled.
49d1464 [Form] Implemented MergeCollectionListener which calls addXxx() and removeXxx() in your model if found
7837f50 [Form] Added FormUtil::singularify()
Discussion
----------
[Form] Forms now call addXxx() and removeXxx() in your model
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #1540
Todo: adapt documentation
![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue1540)
Adds functionality for calling `addXxx` and `removeXxx` method in your model. All types returning a collection of values are affected: "collection", "choice" (with multiple selection) and "entity" (with multiple selection).
Example:
class Article
{
public function addTag($tag) { ... }
public function removeTag($tag) { ... }
public function getTags($tag) { ... }
}
And the controller:
$form = $this->createFormBuilder($article)
->add('tags')
->getForm();
Upon modifying the form, addTag() and removeTag() are now called appropiately.
---------------------------------------------------------------------------
by stof at 2012-02-01T18:23:49Z
Really great
---------------------------------------------------------------------------
by vicb at 2012-02-01T18:24:04Z
Great !!
Two suggestions:
* make "add" and "remove" configurable,
* introduce a base class for the remove listeners with (final?) `::getSubscribedEvents()` and `::getEventPriorities()`
---------------------------------------------------------------------------
by haswalt at 2012-02-01T18:57:46Z
+1 this
---------------------------------------------------------------------------
by daFish at 2012-02-01T19:54:46Z
+1
---------------------------------------------------------------------------
by michelsalib at 2012-02-01T20:55:37Z
Can wait to have it!
It will save lots of time trying to solve WTF effects and making workarounds.
---------------------------------------------------------------------------
by bschussek at 2012-02-02T09:37:12Z
@vicb: Your first point is done. The second, I don't understand.
---------------------------------------------------------------------------
by stof at 2012-02-02T09:40:50Z
@bschussek your branch conflicts with master according to github
---------------------------------------------------------------------------
by vicb at 2012-02-02T09:52:40Z
@bschussek my point is that I can stand hard-coded priorities which are error prone. A better solution might be to introduce constants (in `FormEvents` / `FormEventPriorities` ?) with meaningful names.
---------------------------------------------------------------------------
by bschussek at 2012-02-02T10:21:52Z
@stof Rebased
@vicb I know, but who is responsible for managing priorities? There is no central entitty that can do this. (btw this is a general problem of the priority system of the EventDispatcher)
@fabpot Ready to merge.
---------------------------------------------------------------------------
by vicb at 2012-02-02T10:23:28Z
@bschussek doesn't each form has is own dispatcher so there is no need for a global registry here, something local to the form could be good enough.
The listener is used by the Collection type as well as the Choice and Entity type (with multiple
selection). The effect is that you can have for example this model:
class Article
{
public function addTag($tag) { ... }
public function removeTag($tag) { ... }
public function getTags($tag) { ... }
}
You can create a form for the article with a field "tags" of either type "collection" or "choice"
(or "entity"). The field will correctly use the three methods of the model for displaying and
editing tags.
Commits
-------
2e4ebe4 [Validator] Renamed methods addViolationAtRelativePath() and getAbsolutePropertyPath() in ExecutionContext
9153f0e [Validator] Deprecated ConstraintValidator methods setMessage(), getMessageTemplate() and getMessageParameters()
0417282 [Validator] Fixed typos
a30a679 [Validator] Made ExecutionContext immutable and introduced new class GlobalExecutionContext
fe85bbd [Validator] Simplified ExecutionContext::addViolation(), added ExecutionContext::addViolationAt()
f77fd41 [Form] Fixed typos
1fc615c Fixed string access by curly brace to bracket
a103c28 [Validator] The Collection constraint adds "missing" and "extra" errors to the individual fields now
f904a9e [Validator] Fixed: GraphWalker does not add constraint violation if error message is empty
1dd302c [Validator] Fixed ConstraintViolationList::__toString() to not include dots in the output if the root is empty
1678a3d [Validator] Fixed: Validator::validateValue() propagates empty validation root instead of the provided value
Discussion
----------
[Validator] Improved "missing" and "extra" errors of Collection constraint
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #2615
Todo: -
![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue2615)
Instead of a single violation
Array:
The fields "foo", "bar" are missing
various violations are now generated.
Array[foo]:
This field is missing
Array[bar]:
This field is missing
Apart from that, the PR contains various minor fixes to the Validator.
---------------------------------------------------------------------------
by bschussek at 2012-02-02T09:14:52Z
@fabpot Ready for merge.
Commits
-------
7cecb4e [Form] added support for parent of FormBuilder
Discussion
----------
[Form] added support for parent of FormBuilder
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
In some cases of building forms, it is good to know the attributes of the parent builder. In my case I want to automaticaly add the subscriber to the fields, whose parent builder has a concrete attribute.
Replace #2882
Commits
-------
e9b4c58 [Console] Enable process isolantion in Shell
Discussion
----------
[Console] Enable process isolantion in Shell
Bug fix: no
BC break: no
Feature addition: yes
Symfony2 test pass: yes
Fixes the following tickets: #2848#2847
Todo: Write unit tests
See tickets for reference, need help with unit testing, because I don't know how to test this :)
---------------------------------------------------------------------------
by canni at 2011-12-16T09:36:32Z
I've tested this with different scenarios like "inception" (invoking shell from shell - will not work) ;) and others, everything seems to work great.
As I have no idea on how to pack this with unit testing some help needed, also as I don't have any windows in home ;) need someone to test it on MS os.
And we should decide, do we want process isolation by default? (This will not break the BC, break only the "expected behavior" - colorful output and "interactivity")
---------------------------------------------------------------------------
by canni at 2011-12-18T15:14:26Z
I've rebased this branch to match current `HEAD` and I've added usage of new process builder, for better portability an shell arg escaping.
---------------------------------------------------------------------------
by fabpot at 2012-02-02T08:28:32Z
@canni: Can you squash your commits before I merge this PR? Thanks.
---------------------------------------------------------------------------
by canni at 2012-02-02T09:07:16Z
@fabpot @stof done.
Bug fix: no
BC break: no
Feature addition: yes
Symfony2 test pass: yes
Fixes the following tickets: #2848#2847
Todo: -
See tickets for reference, need help with testing, because I don't know how to test this :)
Commits
-------
de253dd [Form] read_only and disabled attributes
Discussion
----------
[Form] read_only and disabled attributes (closes#1974)
1. Removed ``readOnly`` property from ``Form``, as it is no longer required
2. Introduced ``disabled`` property to ``Form``, behaves exactly like ``readOnly`` used to
3. Added ``disabled`` property to fields, defaults to ``false``, renders as ``disabled="disabled"``
4. A field with positive ``read_only`` property now renders as ``readonly="readonly"``
---------------------------------------------------------------------------
by helmer at 2012-01-26T17:46:17Z
I changed ``Form`` and ``FormBuilder`` property ``readOnly`` to ``disabled``. On second thought, this is perhaps not such good change - while readOnly somewhat implied the use-case, disabled no longer does.
Perhaps something else, like ``bindable`` (as not to confuse with read_only attribute of Fields)?
@bschussek, others, any thoughts?
---------------------------------------------------------------------------
by bschussek at 2012-01-31T06:53:59Z
Please prefix commits with the affected component, if applicable.
---------------------------------------------------------------------------
by helmer at 2012-01-31T08:41:03Z
@bschussek Prefixed. Please also see see to [this question](https://github.com/symfony/symfony/pull/3193#issuecomment-3673074)
Commits
-------
6090dee [FormType] Adopted MoneyTypeTest::testMoneyPatternWorksForYen to CS
e814d27 [FormType] Fixed broken MoneyType regexp for JPY
Discussion
----------
[Bugfix][Form] Fixed broken MoneyType regexp for JPY
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: ![Build Status](https://secure.travis-ci.org/ManuelKiessling/symfony.png?branch=ticket_3124) Fixes the following tickets: #3124
Todo: -
The regexp in MoneyType doesn't work if currency format has no decimal
(like JPY) and doesn't work either if the currency symbol is unicode
This change fixes both issues and adds a unit test
Commits
-------
601d462 [Form] Added getValidators() to Form class
Discussion
----------
[Form] Added getValidators() to Form class
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
I am working implementing client side validation in a bundle that adds validation rules via a form builder. I am currently unable to pull the validators from the Form class which is making the implementation incredibly difficult.
I noticed that the transformers are currently exposed and based on some feedback I got on IRC I am guessing these weren't exposed simply because no one needed them at the time.
---------------------------------------------------------------------------
by mlively at 2011-12-28T20:54:41Z
Side note: It would be incredibly helpful to have this in the current branch of symfony. I implemented it on master because it is a new feature, but I would be more than happy to patch it into 2.0 if you would like.
---------------------------------------------------------------------------
by canni at 2012-01-11T10:15:46Z
👍
As this is new feature it cannot fit into 2.0 series, but 2.1 is just few clicks ahead, maybe this feature will fit into ;)
---------------------------------------------------------------------------
by bschussek at 2012-01-31T08:23:05Z
ping @fabpot
👍
---------------------------------------------------------------------------
by stof at 2012-01-31T08:51:26Z
@mlively could you rebase your branch ? it conflicts with the current master
---------------------------------------------------------------------------
by mlively at 2012-01-31T21:38:39Z
Yes, I can do that might be later today.
Commits
-------
2374e54 Break paths in exceptions hard with css if necessary
Discussion
----------
Exceptions: Break source-paths with CSS
Sometimes in exceptions absolute paths of files are pretty long and need more than one line.
eg.: `/Volumes/Macintosh HD/Users/blabla/Sites/project/files/src/SomeProjectsName/SomeProjectsNameFrontendBundle/Controller/CreateController.php`
This should be displayed within the width of the `h1`.
Using CSS `word-break: break-all;`.
Commits
-------
b228942 fix for entity choice list when ->loaded is false and the class name is an entity shorthand name and updated tests to work with refactored choicelist
Discussion
----------
fix for entity choice list when ->loaded is false and the class name is ...
...an entity shorthand name
This bug was reintroduced after the latest choicelist refactoring and was originally fixed in 231e79ce0f
---------------------------------------------------------------------------
by stof at 2012-02-01T15:17:37Z
Please also add a unit test for this to avoid further regressions
---------------------------------------------------------------------------
by stof at 2012-02-01T17:05:31Z
btw, a better fix would be to put the real class name in ``$this->class`` to avoid doing it each time (you are doing it in a loop btw). We don't need to keep the short notation anyway
---------------------------------------------------------------------------
by Burgov at 2012-02-01T17:19:01Z
@stof done, thanks for the comments
Commits
-------
4bc0c67 [HttpFoundation] fix a small copy and paste error
Discussion
----------
Just a small copy and paste error...
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets:
Todo:
Commits
-------
7b79cc2 Fixing typo in XLIFF Dumper
Discussion
----------
Fixing typo in XLIFF Dumper
There was a typo in the service name for the XLIFF Dumper
A new ExecutionContext is now created everytime that GraphWalker::walkConstraint() is
launched. Because of this, a validator B launched from within a validator A can't break
A anymore by changing the context.
Because we have a new ExecutionContext for every constraint validation, there is no point
in modifying its state anymore. Because of this it is now immutable.
Commits
-------
57cc531 [Form] Improved PHPDocs of choice lists
9e7e2af [Form] Fixed PHPDoc: Used {@inheritdoc} where applicable
2c530cc Fixed typos in UPGRADE file
7899bea Added examples to UPGRADE
d346ae6 Improved choice list sections of UPGRADE and CHANGELOG
a676598 [Form] Added class LazyChoiceList
Discussion
----------
[Form] Added LazyChoiceList
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue3156)
Adds a ChoiceList implementation that satisfies people who formerly extended ArrayChoiceList and loaded choices lazily in its `load` method.
---------------------------------------------------------------------------
by craue at 2012-01-30T12:56:49Z
👍
---------------------------------------------------------------------------
by craue at 2012-01-30T14:55:38Z
Not sure if it's a bug in this PR or in #3156, but the labels get replaced by their keys:
```php
<?php
use Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceList;
use Symfony\Component\Form\Extension\Core\ChoiceList\LazyChoiceList;
use Symfony\Component\Form\Extension\Core\ChoiceList\SimpleChoiceList;
class MyChoiceList extends LazyChoiceList {
protected function loadChoiceList() {
$choices = array(
'bla' => 'blaaaaaahhhh',
);
return new SimpleChoiceList($choices, array(), ChoiceList::COPY_CHOICE, ChoiceList::COPY_CHOICE);
}
public function getChoices() {
$choices = parent::getChoices();
// $choices is array('bla' => 'bla')
return $choices;
}
}
```
If it's not this PR, I can of course open a new ticket for that. But I'm only working with `LazyChoiceList`s.
---------------------------------------------------------------------------
by stof at 2012-01-30T16:07:41Z
@craue the ``SimpleChoiceList`` is an implementation using the same string as label and value. If you need different ones, you need to use the ``ChoiceList`` implementation.
---------------------------------------------------------------------------
by craue at 2012-01-30T16:22:06Z
@stof: That would make `SimpleChoiceList` useless for almost any case. Are you sure?
---------------------------------------------------------------------------
by craue at 2012-01-30T16:33:31Z
The bug even occurs when using
```
return new ChoiceList(array_keys($choices), array_values($choices), array(), ChoiceList::COPY_CHOICE, ChoiceList::COPY_CHOICE);
```
---------------------------------------------------------------------------
by stof at 2012-01-30T16:40:19Z
well, the SimpleChoiceList is for simple cases (thus its naming) where you want the same for the label and the values. And if you look at the class, you will see it extends the ChoiceList implementation which is the generic one.
---------------------------------------------------------------------------
by craue at 2012-01-30T16:53:36Z
For me, "simple" would mean that it just takes the array given, using keys as indices and values as labels. No fancy stuff messing around with anything. :D But, is there anything wrong in this code or in mine? @bschussek: Please enlighten me.
---------------------------------------------------------------------------
by bschussek at 2012-01-30T17:16:58Z
You are both wrong :) `getChoices` does not return the choices with their associated labels anymore. What you get now are the choice indices as array keys and the choice values as array values. How both are determined depends on the index and value generation strategy, which, in your case, are both COPY_CHOICE.
The difference between simple and complex choice lists is, that simple choice lists can only contain scalar values as choices, while other choice lists (such as ObjectChoiceList, EntityChoiceList) may contain objects as choices.
Choice labels are now stored in ChoiceView objects, which are returned by the various `get*Views` methods.
---------------------------------------------------------------------------
by craue at 2012-01-30T18:07:43Z
It's pretty annoying having provided an array with keys and values when initializing the `ChoiceList` but being unable to retrieve it again. Guess I just over-used or even abused those choice lists as kind of (not only form related) lookup tables.
---------------------------------------------------------------------------
by bschussek at 2012-01-30T19:27:21Z
@craue: What's your use case?
---------------------------------------------------------------------------
by craue at 2012-01-30T20:10:16Z
I just used choice lists extensively, even for not directly form-related stuff. In one case, I'm using two of them (which are also used individually) to build up a third one. That went well using the old `ArrayChoiceList`s and their `getChoices` method. Just thinking about creating another set of model classes which just contain my lists. So for only one select field in a form it'll take three classes then: (A) a list, (B) a choice list based on A, (C) a choice form type based on B. Oh well ... :D
---------------------------------------------------------------------------
by craue at 2012-01-30T21:31:32Z
Anyway, this PR for `LazyChoiceList` is great, so please merge it. ;)
---------------------------------------------------------------------------
by craue at 2012-01-31T14:00:46Z
@bschussek: Is it ready to be merged?
---------------------------------------------------------------------------
by bschussek at 2012-01-31T16:59:17Z
Yes
Commits
-------
9db6c8d print info about environment and debug mode when running the `CacheWarmupCommand`
Discussion
----------
print info about environment and debug mode when running the `CacheWarmupCommand`
Adapted from `CacheClearCommand`. Replaces #3212.
Commits
-------
8dc78bd [Form] Fixed YODA issues
600cec7 [Form] Added missing entries to CHANGELOG and UPGRADE
b154f7c [Form] Fixed docblock and unneeded use statement
399af27 [Form] Implemented checks to assert that values and indices generated in choice lists match their requirements
5f6f75c [Form] Fixed outstanding issues mentioned in the PR
7c70976 [Form] Fixed text in UPGRADE file
c26b47a [Form] Made query parameter name generated by ORMQueryBuilderLoader unique
18f92cd [Form] Fixed double choice fixing
f533ef0 [Form] Added ChoiceView class for passing choice-related data to the view
d72900e [Form] Incorporated changes suggested in PR comments
28d2f6d Removed duplicated lines from UPGRADE file
e1fc5a5 [Form] Restricted form names to specific characters to (1) fix generation of HTML IDs and to (2) avoid problems with property paths.
87b16e7 [Form] Greatly improved ChoiceListInterface and all of its implementations
Discussion
----------
[Form] Improved ChoiceList implementation and made form naming more restrictive
Bug fix: yes
Feature addition: yes
Backwards compatibility break: **yes**
Symfony2 tests pass: yes
Fixes the following tickets: #2869, #3021, #1919, #3153
Todo: adapt documentation
![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue1919)
The changes in this PR are primarily motivated by the fact that invalid form/field names lead to various problems.
1. When a name contains any characters that are not permitted in HTML "id" attributes, these are invalid
2. When a name contains periods ("."), form validation is broken, because they confuse the property path resolution
3. Since choices in expanded choice fields are directly translated to field names, choices applying to either 1. or 2. lead to problems. But choices should be unrestricted.
4. Unless a choice field is not expanded and does not allow multiple selection, it is not possible to use empty strings as choices, which might be desirable in some occasions.
The solution to these problems is to
* Restrict form names to disallow unpermitted characters (solves 1. and 2.)
* Generate integer indices to be stored in the HTML "id" and "name" attributes and map them to the choices (solves 3.). Can be reverted to the old behaviour by setting the option "index_generation" to ChoiceList::COPY_CHOICE
* Generate integer values to be stored in the HTML "value" attribute and map them to the choices (solves 4.). Can be reverted to the old behaviour by setting the option "value_generation" to ChoiceList::COPY_CHOICE
Apart from these fixes, it is now possible to write more flexible choice lists. One of these is `ObjectChoiceList`, which allows to use objects as choices and is bundled in the core. `EntityChoiceList` has been made an extension of this class.
$form = $this->createFormBuilder()
->add('object', 'choice', array(
'choice_list' => new ObjectChoiceList(
array($obj1, $obj2, $obj3, $obj4),
// property path determining the choice label (optional)
'name',
// preferred choices (optional)
array($obj2, $obj3),
// property path for object grouping (optional)
'category',
// property path for value generation (optional)
'id',
// property path for index generation (optional)
'id'
)
))
->getForm()
;
---------------------------------------------------------------------------
by kriswallsmith at 2012-01-19T18:09:09Z
Rather than passing `choices` and a `choice_labels` arrays to the view would it make sense to introduce a `ChoiceView` class and pass one array of objects?
---------------------------------------------------------------------------
by stof at 2012-01-22T15:32:36Z
@bschussek can you update your PR according to the feedback (and rebase it as it conflicts according to github) ?
---------------------------------------------------------------------------
by bschussek at 2012-01-24T00:15:42Z
@kriswallsmith fixed
Fixed all outstanding issues. Would be glad if someone could review again, otherwise this PR is ready to merge.
---------------------------------------------------------------------------
by fabpot at 2012-01-25T15:17:59Z
Is it ready to be merged?
---------------------------------------------------------------------------
by Tobion at 2012-01-25T15:35:50Z
Yes I think so. He said it's ready to be merged when reviewed.
---------------------------------------------------------------------------
by bschussek at 2012-01-26T02:30:36Z
Yes.
---------------------------------------------------------------------------
by bschussek at 2012-01-28T12:39:00Z
Fixed outstanding issues. Ready for merge.
Commits
-------
a52c675 [WebProfilerBundle] Improve the logger panel
Discussion
----------
[WebProfilerBundle] Improve the logger panel
No more need to hit 'refresh'
Commits
-------
b177786 Make twig optimizations configurable
Discussion
----------
optimizations not configurable
Valid option for twig but missing in the configuration. I am currently hardsetting this in my own bundle.
Commits
-------
b879397 [Profiler] Optimize time panel IS
d4300b9 [WebProfilerBundle] Tweak the time view
416a2a4 [Stopwatch] Fix some logic
8c3505e [Profiler] Tweak PHPDoc
3bcd154 [HttpKernel] Tweak the Profile class - DRY
Discussion
----------
[Profiler] Stopwatch related tweaks
* Some fixes in the stopwatch logic,
* Some JS fixes,
* Make use of modern JS.
The regexp in MoneyType doesn't work if currency format has no decimal
(like JPY) and doesn't work either if the currency symbol is unicode
This change fixes both issues and adds a unit test
Commits
-------
ed9c348 Authentication(Success|Failure)Handler can now return null
Discussion
----------
[Security] Authentication(Success|Failure)Handler can now return null
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Related to the following ticket: #838
[![Build Status](https://secure.travis-ci.org/odolbeau/symfony.png)](http://travis-ci.org/odolbeau/symfony)
Correct me if I'm wrong but for now it's not possible to handle Authentication(Success|Failure) in some case only (for example to handle XmlHttpRequest on login form).
With this change, if the handler return null, the default behavior is kept.
---------------------------------------------------------------------------
by stof at 2012-01-24T17:28:49Z
👍
Commits
-------
43e0db5 [DomCrawler] Add support for multivalued form fields (fix#1579, #3012)
Discussion
----------
[DomCrawler] Support for multivalued fields
This is a tentative fix for #1579 by @kriswallsmith, also see #3012 for more info.
Any feedback is appreciated.
---------------------------------------------------------------------------
by vicb at 2012-01-05T08:44:51Z
@stof thanks for the valuable feedback I think most of it should be implemented should we use this solution.
The one thing I don't agree is PSR-0, I don't want this class to be public, that's is just a "private" helper class.
There are also missing type hints in the helper class, that should be added.
---------------------------------------------------------------------------
by alessandro1997 at 2012-01-05T10:05:15Z
Well, @vicb, I think it's up to the developer to not use "private" classes. Just write it in the documentation. But declaring two classes in the same file would be a big violation of the standards.
---------------------------------------------------------------------------
by vicb at 2012-01-05T11:28:53Z
What "standard"s ?
PSR-0 is about auto-loading, I don't want/need this to be autoloaded.
Sf coding standards ? Well relying on a developer reading the doc is more error prone than the current implementation. I sometimes favor pragmatism over theory.
edit: I am not trying to say I am right here but only that I don't see any added value in moving the helper class to a dedicated file. I appreciate any feedback, really.
---------------------------------------------------------------------------
by fabpot at 2012-01-06T11:55:09Z
FYI, we already have such a "private" class in https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Security/Http/Firewall/DigestAuthenticationListener.php#L135
---------------------------------------------------------------------------
by vicb at 2012-01-06T16:36:04Z
@alessandro1997 if you need an example on why it is not safe to rely on developers reading comments, see #2892
---------------------------------------------------------------------------
by vicb at 2012-01-09T22:19:52Z
@fabpot I am waiting for your feedback on the [proposed API](https://github.com/symfony/symfony/pull/3035/files#L1R57) before finishing this PR.
---------------------------------------------------------------------------
by drak at 2012-01-10T05:12:16Z
@fabpot
> FYI, we already have such a "private" class in https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Security/Http/Firewall/DigestAuthenticationListener.php#L135
Why on is that necessary, it could just be another class file in the namespace. Unless you are making some kind of forward compatibility, e.g. for with a new class in PHP 5.4 then I see no reason to do that.
---------------------------------------------------------------------------
by vicb at 2012-01-10T07:40:32Z
What would be a good reason not to allow "private" classes ?
If the Sf coding standards are the only good reason let's change them then.
[Java](http://stackoverflow.com/questions/968347/can-a-java-file-have-more-than-one-class) and [ActionScript3](http://livedocs.adobe.com/flex/3/html/help.html?content=03_Language_and_Syntax_05.html) allow such construction
I would no say any better than the above link on Stack Overflow:
> The purpose of including multiple classes in one source file is to bundle related support functionality (internal data structures, support classes, etc) together with the main public class. Note that it is always ok not to do this -- the only effect is on the readability (or not) of your code.
---------------------------------------------------------------------------
by Tobion at 2012-01-10T09:35:09Z
There are also many private classes in the test cases.
---------------------------------------------------------------------------
by stof at 2012-01-10T13:29:08Z
@Tobion for tests, it is logical because there is no autoloader for the test classes.
---------------------------------------------------------------------------
by vicb at 2012-01-10T13:31:53Z
@stof by definition you do not want a "private" class to be autoloaded anyway.
---------------------------------------------------------------------------
by alessandro1997 at 2012-01-10T14:11:42Z
Sure, but what you're doing here is just making instantiating the class a bit more difficult. If a stubborn developer wants to use it, then he (or her) can include the file manually or autoload the "main class".
PHP does NOT have support for private/inner classes, and, until it does, all classes should be istantiable normally.
---------------------------------------------------------------------------
by stof at 2012-01-10T14:23:30Z
@vicb what about someone wanting to serialize the object ? (well, serializing is not the issue. unserializing is)
---------------------------------------------------------------------------
by vicb at 2012-01-10T14:57:52Z
@alessandro1997 you are absolutely right, it's not meant to be instantiated from the outside (it's **private**). You could argue the same with private properties & methods (using Reflection). Dead-end.
@stof Is unserializing really an issue as the file would have been loaded already ?
---------------------------------------------------------------------------
by fabpot at 2012-01-22T09:38:13Z
@vicb: I'm fine with the proposed API, but I fail to see why it would be more BC than #3012.
---------------------------------------------------------------------------
by vicb at 2012-01-22T10:06:56Z
For BC I have to check #3012 again but at some point if I remember correctly the public API had changed (not sure about the latest version in your branch)
By introducing the private helper class, it is quite easy to see that the public API is not modified by this PR.
Next steps:
* Stof the code,
* Add/fix phpdoc,
* Add tests for the helper class,
* Add/refactor tests for the `Form` class.
@fabpot if you agree with the above steps it could be ready sometime next week.
---------------------------------------------------------------------------
by fabpot at 2012-01-22T10:21:16Z
The API is perhaps not changed but the behavior will certainly changed. I agree with your steps.
---------------------------------------------------------------------------
by vicb at 2012-01-22T10:45:10Z
Which leads to the question: should we consider this as a change in behavior (2.1) or a bug fix (2.0) ?
_I am thinking of a form with multiple fields named `field[]`_
---------------------------------------------------------------------------
by fabpot at 2012-01-22T11:32:04Z
@vicb: this change should be done on master
---------------------------------------------------------------------------
by vicb at 2012-01-24T07:59:40Z
Should be ready now, let me know when I should squash after review.
---------------------------------------------------------------------------
by fabpot at 2012-01-24T08:18:03Z
@vicb: yes, can you squash your commits?
---------------------------------------------------------------------------
by vicb at 2012-01-24T08:29:58Z
@fabpot done
ad (1): HTML4 "id" attributes are limited to strings starting with a letter and containing only letters, digits, underscores, hyphens, periods and colons.
ad (2): Property paths contain three special characters needed for correct parsing: left/right bracket and period.
The rules for form naming are:
* Names may start with a letter, a digit or an underscore. Leading digits or underscores will be stripped from the "id" attributes.
* Names must only contain letters, digits, underscores, hyphens and colons.
* Root forms may have an empty name.
Solves #1919 and #3021 on a wider scope.
Commits
-------
e37783f [DoctrineBridge] Refactored the query sanitization in the collector
3b260d2 Refactored the collector to separate the loggers per connection
Discussion
----------
Doctrine collector
Bug fix: no
Feature addition: yes
Backwards compatibility break: yes (for the end user, it will require deleting old profiler data)
Symfony2 tests pass: yes ![Build Status](https://secure.travis-ci.org/stof/symfony.png?branch=doctrine_collector)
This refactors the Doctrine collector to allow implementing doctrine/DoctrineBundle#7
The first commit splits the logging of queries per connection to be able to know which connection was used instead of using a shared stack.
The second commit refactors the sanitation of the parameters to apply the DBAL conversion and then keep the param whenever possible (i.e. when we are sure it is serializable). Such queries will then be explainable in the profiler as we will be able to use the parameters again. Due to the way PDO works, the only cases where we would get an unexplainable queries due to the parameters are queries using a LOB parameter (as it is a resource) or broken queries (passing an object to PDO for instance). And this second case does not make sense to explain the query of course.
---------------------------------------------------------------------------
by stof at 2012-01-23T12:32:16Z
Merging this PR should be synchronized with the DoctrineBundle PR due to the BC break in the collector
Commits
-------
4a797df Oracle issues
81d73bb Oracle issues
2316b21 Oracle issues
315bfc4 just update
b20b15b Oracle 10 issues
Discussion
----------
Oracle issues
updated with some adjustments required by stof
---------------------------------------------------------------------------
by fabpot at 2011-12-13T07:24:12Z
@schmittjoh: Can you have a look at this PR?
---------------------------------------------------------------------------
by fabpot at 2011-12-24T08:19:37Z
Can you squash your commit before I merge your PR? Thanks.
Commits
-------
753c067 [FrameworkBundle] added $view['form']->csrfToken() helper
e1aced8 [Twig] added {{ csrf_token() }} helper
Discussion
----------
[Twig] [FrameworkBundle] added CSRF token helper
I've added a templating helper and Twig function for generating a CSRF token without the overhead of creating a form.
```html+jinja
<form action="{{ path('user_delete', { 'id': user.id }) }}" method="post">
<input type="hidden" name="_method" value="delete">
<input type="hidden" name="_token" value="{{ csrf_token('delete_user_' ~ user.id) }}">
<button type="submit">delete</button>
</form>
```
```php
<?php
class UserController extends Controller
{
public function delete(User $user, Request $request)
{
$csrfProvider = $this->get('form.csrf_provider');
if (!$csrfProvider->isCsrfTokenValid('delete_user_'.$user->getId(), $request->request->get('_token')) {
throw new RuntimeException('CSRF attack detected.');
}
// etc...
}
}
```
The test that is failing on Travis appears to be unrelated, but I may be wrong?
```
1) Symfony\Bundle\SecurityBundle\Tests\Functional\LocalizedRoutesAsPathTest::testLoginLogoutProcedure with data set #1 ('de')
RuntimeException: OUTPUT:
Catchable fatal error: Argument 3 passed to Symfony\Bundle\FrameworkBundle\Controller\TraceableControllerResolver::__construct() must be an instance of Symfony\Component\HttpKernel\Debug\Stopwatch, instance of Symfony\Bundle\FrameworkBundle\Controller\ControllerNameParser given, called in /tmp/2.1.0-DEV/StandardFormLogin/cache/securitybundletest/appSecuritybundletestDebugProjectContainer.php on line 94 and defined in /home/vagrant/builds/kriswallsmith/symfony/src/Symfony/Bundle/FrameworkBundle/Controller/TraceableControllerResolver.php on line 37
```
---------------------------------------------------------------------------
by pablodip at 2012-01-10T14:18:45Z
As you don't need forms to use the csrf provider, how about putting its service without the form prefix? It could even make sense to put the CsrfProvider as a component since you can use it standalone and in more cases than only forms. It would be a small component though.
---------------------------------------------------------------------------
by Tobion at 2012-01-10T17:54:14Z
I think it would be more clear to generate the token in the controller. Doing so in the template will spread the CSRF intention across template and controller. So I don't think this extension is necessary.
---------------------------------------------------------------------------
by kriswallsmith at 2012-01-10T17:58:14Z
@pablodip I'm open to the idea of a Csrf component. This would be a good place for some nonce classes as well.
@Tobion I disagree. One use case is for a list of users, each with a delete form. Iterating over the users in the controller and generating a token for each, just to iterate over them again in the view is a waste and adds complexity.
---------------------------------------------------------------------------
by Tobion at 2012-01-10T18:05:14Z
I see. But I don't understand why the intention needs to be different for each user to delete. Usually the intention is the same for each form type. I thought this is enough.
---------------------------------------------------------------------------
by kriswallsmith at 2012-01-10T18:06:13Z
Yes, a static intention would suffice.
---------------------------------------------------------------------------
by Tobion at 2012-01-10T18:07:08Z
Then your use case is not valid anymore.
---------------------------------------------------------------------------
by Tobion at 2012-01-10T18:12:25Z
I would suggest to make a cookbook article out of it about how to create a simple form without the form component.
And include such things as validating the result using the validator component and checking the CSRF.
---------------------------------------------------------------------------
by kriswallsmith at 2012-01-10T21:32:50Z
This helper makes it easier to use CSRF protection without a form and we should make it as easy as possible. Spreading the intention across controller and template is not concerning to me. Either way, a cookbook entry is a great idea.
---------------------------------------------------------------------------
by Tobion at 2012-01-10T21:47:12Z
Well, it's just one line more without this helper. So I disagree it makes it really easier when you know how to use the CsrfProvider which is a pre-condition anyway since you must still validate its correctness by hand.
---------------------------------------------------------------------------
by kriswallsmith at 2012-01-13T13:24:15Z
Another use case is when rendering a page with a bunch of simple buttons with different intentions: delete user, delete comment, follow, unfollow... Creating all of these in the controller just leads to spaghetti.
---------------------------------------------------------------------------
by jwage at 2012-01-17T21:55:53Z
👍 lots of use cases for something like this @OpenSky
Commits
-------
92f820a Renamed registerConstraints to loadDynamicValidatorMetadata
dd12ff8 CS fix, getConstraints renamed
09c1911 [Validator] Improved dynamic constraints
54cb6e4 [Validator] Added dynamic constraints
Discussion
----------
[Validator] Dynamic constraints
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
By now the Validator component is based on a per-class configuration of
constraints, but in some cases it might be neccessary to add new constraints
dynamically at runtime.
This pull request adds a "ConstraintProviderInterface" to the Validator component. If an object is validated that implements this interface the method "getConstraints" is used to add dynamic constraints:
class User implements ConstraintProviderInterface
{
protected $isPremium;
protected $paymentInformation;
public function getConstraints(ClassMetadata $metadata)
{
if ($this->isPremium) {
$metadata->addPropertyConstraint('paymentInformation', new NotBlank());
}
}
}
---------------------------------------------------------------------------
by alexandresalome at 2012-01-15T11:20:04Z
Related to #1151
---------------------------------------------------------------------------
by canni at 2012-01-16T09:22:28Z
👍
---------------------------------------------------------------------------
by bschussek at 2012-01-16T12:32:44Z
I think this is a good addition. I think we still have a naming problem though. When constraints are loaded using a static method, the default name for the loader method is `loadValidatorMetadata`. Since the method for dynamic constraint loading is basically the same, I think the two names should be related.
Solution (1): Rename the method in your interface to `loadDynamicValidatorMetadata`. Ugly and long.
class MyClass implements ConstraintProviderInterface
{
public static loadValidatorMetadata(ClassMetadata $metadata) ...
public loadDynamicValidatorMetadata(ClassMetadata $metadata) ...
}
Solution (2): Rename the default method name in `StaticMethodLoader` to `registerConstraints` and adjust the docs. Breaks BC.
class MyClass implements ConstraintProviderInterface
{
public static registerConstraints(ClassMetadata $metadata) ...
public registerDynamicConstraints(ClassMetadata $metadata) ...
}
@fabpot: Are we allowed to break BC here? If not, we should probably stick to (1).
---------------------------------------------------------------------------
by fabpot at 2012-01-16T12:36:14Z
I would prefer to not break BC if possible.
---------------------------------------------------------------------------
by blogsh at 2012-01-16T15:25:46Z
So "loadDynamicValidatorMetadata" would be the best solution?
---------------------------------------------------------------------------
by althaus at 2012-01-17T13:39:19Z
>So "loadDynamicValidatorMetadata" would be the best solution?
Sounds fine for me based on @bschussek's comment.
Commits
-------
9cb513f Now… no more tabs!
7f34643 [Pull Request 3134] Improved code based on comments
90abc0f [Serializer][XmlEncoder] add CDATA padding only if necessary
Discussion
----------
[Serializer][XmlEncoder] add CDATA padding only if necessary
Changed XML encoder so CDATA padding is only added to value if necessary.
---------------------------------------------------------------------------
by fabpot at 2012-01-17T21:34:59Z
You should add some unit tests.
Commits
-------
0b7e2e0 Support for DELETE method in forms
Discussion
----------
[Form] Support DELETE HTTP verb
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: none
Todo: -
As `Symfony\Component\HttpFoundation\Request` already support DELETE requests nicely by parsing the request for us, support for the HTTPs DELETE verb can be easily done.
---------------------------------------------------------------------------
by mvrhov at 2012-01-20T06:00:49Z
This is wrong. The body for DELETE method is supposed to be empty or if present ignored.
Also the DELETE is supposed to remove the resource identified by uri, so the same code as for GET should be executed.
---------------------------------------------------------------------------
by lstrojny at 2012-01-20T08:56:22Z
I don’t think that’s the case. The HTTP standard does not state explicitly that DELETE does not have a body. See this [StackOverflow thread](http://stackoverflow.com/questions/2539394/rest-http-delete-and-parameters)
Commits
-------
9e55cda Only call recover() when spool is a Swift_FileSpool
d2a0c74 Use if/else instead of ternary operator
15c666b Add a "recover-timeout" option to allow recovering messages that have taken too long to send
Discussion
----------
[SwiftmailerBundle] Add a "recover-timeout" option to swiftmailer:spool:send
This would allow for easy resending of messages that were marked as being sent, but for whatever reason were never actually sent.
Commits
-------
f6b3ea2 New validation messages and translated to Serbian language.
Discussion
----------
New validation messages and translated to Serbian language.
It would be nice for translators to be notified somehow when new validation messages appear. I copied those from French translation, not sure if that is the right way to go?
Also, in addition, I would like to contribute sr@latin translation. To explain, Serbian language have dual alphabet, both cyrillic and latin. I'm not sure if Symfony locale supports locale variants? Can you suggest right translation file name for this?
---------------------------------------------------------------------------
by stof at 2012-01-21T19:20:31Z
Please send the ids up to 41 to the 2.0 branch. Only 42 and above are new in 2.1
---------------------------------------------------------------------------
by stof at 2012-01-21T19:23:48Z
Regarding serbian latin translations, there is an issue here: both cyrillic and latin serbian share the same locale id ``sr_SP``
---------------------------------------------------------------------------
by stof at 2012-01-21T19:33:01Z
ok, looking a bit more about it, it seems like the right way to handle this is to use ``sr_Latn`` and ``sr_Cyrl`` for the 2 variants
---------------------------------------------------------------------------
by umpirsky at 2012-01-21T20:28:37Z
But ids 42 and above can be merged to master (2.1), right?
I think they share `sr_RS`, not `sr_SP` as you said.
So, `validators.sr.xlf` should be renamed to `validators.sr_Cyrl.xlf` and for latig added `validators.sr_Latn.xlf`?
---------------------------------------------------------------------------
by stof at 2012-01-21T21:00:18Z
yeah, but previous ids should be merged in 2.0 first to avoid merge conflicts later
---------------------------------------------------------------------------
by umpirsky at 2012-01-21T22:37:15Z
Done https://github.com/symfony/symfony/pull/3168
* 2.0:
Updated Serbian translation.
fixed CS
[Locale][Testing] Fixed breaking tests if 'intl' extension is not installed (#3139)
[Bridge] [Twig] fixed typo in a comment of the Twig FormExtension extension.
Commits
-------
0513eb1 [Form] Pass translation domain to the sub-forms when choice list is expanded
Discussion
----------
[Form] Pass translation domain to the sub-forms when choice list is expanded
* Bug fix: yes
* Tests pass: yes
* Feature addition: no
* BC compatibility break: no
When you have a select list with ``translation_domain``, you loose translations by expanding the list.
---------------------------------------------------------------------------
by stof at 2012-01-21T14:55:31Z
👍
---------------------------------------------------------------------------
by fabpot at 2012-01-21T16:51:17Z
Why not doing that in the 2.0 branch instead?
---------------------------------------------------------------------------
by stof at 2012-01-21T17:26:32Z
@fabpot because the support of translation domains is a 2.1 feature
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
This is cleanup after enabling empty form names, now form with empty name
will not render the default `id="form"` container attribute.
Developers can extend/override this behaviour by standard form theming methods.
Commits
-------
7e14a56 [Locale] Removed unneccesary semi-colon
cacc880 [Bugfix][Locale] Fixed incomplete Locale data loading
Discussion
----------
[Bugfix][Locale] Fixed incomplete Locale data loading
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: ![Build Status](https://secure.travis-ci.org/ManuelKiessling/symfony.png) Fixes the following tickets: #3090
Todo: -
Sublocales like de_CH returned only incomplete results for
getDisplayCountries(), getDisplayLanguages() and getDisplayLocales(),
consisting only of results specific for this sublocale, but without the
results of their respective parent locale
This PR was https://github.com/symfony/symfony/pull/3106 before - reopened it as a new PR because the commits were too chaotic.
Commits
-------
e6e3da5 [Validator] Improved test coverage of CollectionValidator and reduced test code duplication
509c7bf [Validator] Moved Optional and Required constraints to dedicated sub namespace.
bf59018 [Validator] Removed @api-tag from Optional and Required constraint, since these two are new.
6641f3e [Validator] Added constraints Optional and Required for the CollectionValidator
Discussion
----------
[Validator] Improve support for optional/required fields in Collection constraint
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: none
Todo: none
![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=collection-validator)
Improves the `Collection` constraint to test on a more granular level if entries of the collection are optional or required. Before this could only be set using the "allowExtraFields" and "allowMissingFields" options, but these are very general and limited.
The former syntax - without Optional or Required - is still supported.
Usage:
$array = array(
'name' => 'Bernhard',
'birthdate' => '1970-01-01',
);
$validator->validate($array, null, new Collection(array(
'name' => new Required(),
'birthdate' => new Optional(),
));
// you can also pass additional constraints for the fields
$validator->validate($array, null, new Collection(array(
'name' => new Required(array(
new Type('string'),
new MinLength(3),
)),
'birthdate' => new Optional(new Date()),
));
---------------------------------------------------------------------------
by canni at 2012-01-15T20:22:17Z
@bschussek I've rewritten a lot of test code for Collection validator in 2.0 branch and also had modified validator itself, as it had a bug #3078, consider waiting with this PR till fabpot will merge 2.0 back into master, as there will be code conflicts :)
---------------------------------------------------------------------------
by Koc at 2012-01-15T23:13:04Z
Does it helps to #2615 ?
---------------------------------------------------------------------------
by fabpot at 2012-01-16T06:44:53Z
@canni: I've just merged 2.0 into master.
---------------------------------------------------------------------------
by bschussek at 2012-01-16T12:05:19Z
@fabpot: Rebased. I also fixed the CS issues mentioned by @stof.
Commits
-------
f3c413d add missing class var; add phpdocs
Discussion
----------
add missing class var; add phpdocs
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
---------------------------------------------------------------------------
by fabpot at 2012-01-16T11:12:27Z
We don't document properties, especially private ones.
---------------------------------------------------------------------------
by vicb at 2012-01-16T11:20:44Z
Good doc always help and should be accepted even for private properties.
However sometimes doc isn't necessary: `The digest algorithm to use` does not bring more information than the name itself `MessageDigestPasswordEncoder::algorithm`, the `@var` annotation could be useful - even more for objects & arrays.
---------------------------------------------------------------------------
by gimler at 2012-01-16T11:37:54Z
i have remove the private property comments.
Sublocales like de_CH returned only incomplete results for
getDisplayCountries(), getDisplayLanguages() and getDisplayLocales(),
consisting only of results specific for this sublocale, but without the
results of their respective parent locale
Commits
-------
7961014 [Yaml][Parser] changes according review
efce640 [Yaml][Parser] throw an exception if not readable
Discussion
----------
[Yaml][Parser] throw an exception if service file not readable.
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #3077
Todo: -
---------------------------------------------------------------------------
by makasim at 2012-01-13T15:49:49Z
@fabpot done
Commits
-------
e23d452 Add info about BC Break to CHANGELOG-2.1
d7ffeb5 Add some more tests, and enforce boolean return value of interface implementations.
9d3a49f When method name is `hasUserChanged` the return boolean should be true (to match question semantics) and false when user has not changed, this commits inverts return statements.
c57b528 Add note about `AdvancedUserInterface`.
3682f62 Refactor `isUserChanged` to `hasUserChanged`
56db4a1 Change names to Equatable
680b108 Suggested fixes ;)
9386583 [BC Break][Security] Moved user comparsion logic out of UserInterface As discussed on IRC meetings and in PR #2669 I came up with implementation. This is option2, I think more elegant.
Discussion
----------
[BC Break][Security][Option2] Moved user comparsion logic out of UserInterface
As discussed on IRC meetings and in PR #2669 I came up with implementation.
This is option2, I think more elegant.
BC break: yes
Feature addition: no/feature move
Symfony2 test pass: yes
Symfony2 test written: yes
Todo: decide about naming
[![Build Status](https://secure.travis-ci.org/canni/symfony.png)](http://travis-ci.org/canni/symfony)
---------------------------------------------------------------------------
by schmittjoh at 2011-12-19T19:33:24Z
This looks much better than the previous PR. Thanks!
One thing, we also discussed this on Doctrine, the name "comparable" is used in most programming languages to perform a real compare operation that is ">", "<", or "=". In this case though, we are specifically interested in equality of two objects (we cannot establish a natural order between these objects). Java has no such interface as all objects naturally have an equals() method, .NET uses "Equatable" which looks a bit odd. Not sure if there are better names.
---------------------------------------------------------------------------
by canni at 2011-12-19T19:34:52Z
I think this is best of "both worlds" we have nice full-featured implementation suitable for most, and if someone needs advanced compare logic just implements interface. @stof @schmittjoh, what do you think?
---------------------------------------------------------------------------
by stof at 2011-12-19T19:36:55Z
@canni I already commented on the code, and I agree with @schmittjoh that the naming can be confusing
---------------------------------------------------------------------------
by jmikola at 2011-12-20T17:33:22Z
I don't mean to bikeshed, but I strongly agree with @schmittjoh about implications of "compare". I'm not concerned with the interface name so much as I am with `compareUser()`. Given that this method returns a boolean, I think it's best to prefix it with `is` (e.g. `isSameUser`, `isUserEqualTo`) or `equals` (e.g. `equalsUser`).
In this PR, the Token class is implementing the interface, so I think having "User" in the method name is a good idea. Naturally, if the interface was intended for User classes, we could do without it.
---------------------------------------------------------------------------
by canni at 2011-12-20T19:00:00Z
@jmikola in this PR Token class does not implement any additional interface, and `compareUser` is `private` and used internally. I don't stand still after this names, I'll update PR as soon as some decision about naming will be done.
---------------------------------------------------------------------------
by jmikola at 2011-12-21T02:29:59Z
@canni: My mistake, I got confused between the Token method and interface method, which you've since renamed in canni/symfony@fcfcd1087b.
---------------------------------------------------------------------------
by mvrhov at 2011-12-21T06:09:45Z
hm. Now I'm going to bike shed. Wouldn't the proper function name be hasUserChanged?
---------------------------------------------------------------------------
by stof at 2011-12-21T10:58:38Z
it would probably be bettter. The meaning of ``true`` and ``false`` would then be the opposite of the current ones but this is not an issue IMO as it is a different method
---------------------------------------------------------------------------
by jstout24 at 2011-12-27T18:08:49Z
@canni nice job
---------------------------------------------------------------------------
by fabpot at 2011-12-30T14:59:11Z
The method `isUserChanged()` must be rename. What about `hasUserChanged()` as @mvrhov suggested or `isUserDifferent()`?
---------------------------------------------------------------------------
by canni at 2012-01-02T11:44:05Z
@fabpot done.
---------------------------------------------------------------------------
by fabpot at 2012-01-02T18:13:40Z
The only missing thing I can think of is adding some unit tests.
---------------------------------------------------------------------------
by canni at 2012-01-10T20:16:25Z
@fabpot is there anything more you think that should done in this PR?
---------------------------------------------------------------------------
by stof at 2012-01-10T20:38:46Z
@canni can you rebase your branch ? it conflicts with the current master according to github
---------------------------------------------------------------------------
by canni at 2012-01-10T20:56:55Z
@stof done.
---------------------------------------------------------------------------
by fabpot at 2012-01-12T18:06:00Z
@canni: Can you just add some information in the CHANGELOG and in the UPGRADE file? That's all I need to merge this PR now. Thanks a lot.
---------------------------------------------------------------------------
by canni at 2012-01-12T18:16:32Z
@fabpot done, and no problem :)
Commits
-------
78ce60c Add config as required
10b3cde [FrameworkBundle] Add missing dependency and recommended libraries fixes#3094
Discussion
----------
[FrameworkBundle] Add missing dependency and recommended libraries
Fixes#3094
---------------------------------------------------------------------------
by fabpot at 2012-01-12T17:31:14Z
You forgot the dependency on config? Is it on purpose
---------------------------------------------------------------------------
by henrikbjorn at 2012-01-12T17:39:20Z
the config is recommended package on the DependencyInjection component.
---------------------------------------------------------------------------
by stof at 2012-01-12T17:40:56Z
@henrikbjorn yeah, but it is *required* dependency for FrameworkBundle, not only recommended
---------------------------------------------------------------------------
by henrikbjorn at 2012-01-12T17:41:56Z
well it will install it by default as it is recommended.
---------------------------------------------------------------------------
by stof at 2012-01-12T17:43:05Z
@henrikbjorn yeah, but it will be skipped if the user asks to avoid recommended packages (the flag is not implemented yet IIRC) which would break FrameworkBundle as it requires the component
Commits
-------
fe62401 optimized string starts with checks
Discussion
----------
optimized string starts with checks
Doing this with strpos() is slightly faster than substr().
```
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
```
---------------------------------------------------------------------------
by vicb at 2012-01-11T19:58:27Z
How faster ? even if the string is long and do not contain an occurrence of the sub-string ?
Looks like micro-(not)-optimizations to me.
---------------------------------------------------------------------------
by kriswallsmith at 2012-01-11T20:04:26Z
The difference is about 0.1s when repeated 1M times.
---------------------------------------------------------------------------
by vicb at 2012-01-11T20:08:12Z
% would be better (machine & env independant), what string size, what match offset ?
I personally vote against (`substr` is more meaningful to me and I do not like micro-optims)
---------------------------------------------------------------------------
by kriswallsmith at 2012-01-11T20:12:34Z
I personally consider this a coding standard but don't want to bikeshed here :)
---------------------------------------------------------------------------
by vicb at 2012-01-11T20:28:08Z
I have [tried](https://gist.github.com/1596588) at home.
`strpos ` **is** faster unless you have a very long string, probably because you do not need to create a new string, interesting, thanks for the tip.
---------------------------------------------------------------------------
by Tobion at 2012-01-11T22:40:18Z
I think strpos() is more useful. Say you want to change the string you have to replace 2 variables (the text and the length parameter) when using substr(). It could also introduce bugs when they don't match. With strpos() it's only the text.
---------------------------------------------------------------------------
by robocoder at 2012-01-11T22:43:22Z
alternate micro-optimization that doesn't create a temporary string:
```
strncmp($v, "@", 1) === 0
```
---------------------------------------------------------------------------
by Tobion at 2012-01-11T22:47:12Z
@robocoder probably the fastest solution but needs to be benchmarked
Commits
-------
7f7f82a [HttpKernel] removed unnecessary regex
Discussion
----------
[HttpKernel] removed unnecessary regex
The pattern was also flawed because of the unescaped `.`
```
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
```
Commits
-------
7f7c2a7 Add prof-of-concept test, this test will fail without changes in previous commit
253eeba [BugFix][Validator] Fix for PHP incosistent behaviour of ArrayAccess
Discussion
----------
[BugFix][Validator] Fix for PHP incosistent behaviour of ArrayAccess
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #2779
Todo: -
[![Build Status](https://secure.travis-ci.org/canni/symfony.png)](http://travis-ci.org/canni/symfony)
Because PHP function `array_key_exists` is buggy, it works great with native
PHP `ArrayObject` instances, but hand written implementations of `ArrayAccess`
and `Traversable` objects will fail to work with `CollectionValidator`
Tests from second commit are valid use cases, but without this change, they will fail.
Commits
-------
aa58330 [Form] fixed flawed condition
Discussion
----------
[Form] fixed flawed condition
The validate() method always returns an object. The test is whether there are violations in that object.
```
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
```
---------------------------------------------------------------------------
by fabpot at 2012-01-10T21:22:10Z
What about removing the if condition altogether?
---------------------------------------------------------------------------
by kriswallsmith at 2012-01-10T21:23:55Z
This way we avoid creating an `ArrayIterator` for no reason.
Commits
-------
c0ad1ac [HttpKernel] Minor fixes in the Stopwatch
Discussion
----------
[HttpKernel] Minor fixes in the Stopwatch
Not a breakthrough, fixing `'0'` handling at 2 places, some re factoring (fluid interface)
As discussed on IRC meetings and in PR #2669 I came up with implementation.
This is option2, I think more elegant.
BC break: yes
Feature addition: no/feature move
Symfony2 test pass: yes
Symfony2 test written: yes
Todo: feedback needed
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #2779
Todo: -
Because PHP function `array_key_exists` is buggy, it works great with native
PHP `ArrayObject` instances, but hand written implementations of `ArrayAccess`
and `Traversable` objects will fail to work with `CollectionValidator`
Commits
-------
af32590 [FrameworkBundle] Use only _route_params to generate redirect routes
Discussion
----------
[FrameworkBundle] Use only _route_params to generate redirect routes
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Routes in RedirectController are generated using all request attributes, which is inconvenient since I abuse request attributes to store other things (device types and such) relevant to the app. It renders the RedirectController useless since it adds unrelated query parameters to URLs it creates.
Commits
-------
c8bafcf Updated validators.sk.xlf file
Discussion
----------
[FrameworkBundle] Slovak translations updated
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
---------------------------------------------------------------------------
by stof at 2012-01-09T20:33:53Z
can you send the ids below 41 to the 2.0 branch ? Only 42-28 are new for 2.1
---------------------------------------------------------------------------
by pulzarraider at 2012-01-09T20:39:42Z
Done, see #3073.
fix CS
fix CS + remove unneeded else
add documentation, change protected methods as private
rename var
throw exception for invalid name, index fix
memcache profiler storage support added, fix CS and minor bugs
fix CS
removed unneeded else
- memcached support added
- improved performance (serialization, index)
updated code to last version of Profiler
Commits
-------
c7ab9ba [Console] Allow redefinition of application options descriptions
Discussion
----------
[Console] Allow redefinition of application options descriptions
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
This allows you to redefine an `InputOption` as long as it keeps the same semantic (same default, same name, same alias, same modes). There are two purposes:
- Modifying the description with a more accurate one
- Making sure the option appears in your commands' help
Concrete example: I often want to provide a verbose version of commands. It's an elegant and very common pattern, but I basically can't document what is going to happen if you do `--verbose` since the base Application already defines `--verbose`. Also the `--verbose` option does not appear when you do `console <command> --help`, which means people probably won't think of using that option.
Commits
-------
8ee9161 [Security] Adding more extensive PHPDoc to UserInterface, AdvancedUserInterface and UserProviderInterface
Discussion
----------
More extensive PHPDoc for Security interfaces
Hey guys!
We've started to get into the habit of documenting interfaces and methods in the official docs. I think these things should be omitted from the documentation entirely, and replaced with a link to API docs that rock (I've started doing this already).
This PR just takes some of the details we have in the docs and pushes them back as PHPDoc. I use `@see`, `<code>` and changed a particular `@throws` to have a FQ class name since there's no `use` statement.
Thanks!
---------------------------------------------------------------------------
by weaverryan at 2012/01/07 20:24:15 -0800
Ok, updated and I think it's clearer now.
---------------------------------------------------------------------------
by fabpot at 2012/01/07 23:29:45 -0800
@weaverryan Great! I think that's a really good idea to document interfaces in the API, that makes a lot of sense.
---------------------------------------------------------------------------
by maastermedia at 2012/01/08 02:10:04 -0800
+1 Symfony API needs that atention also, yes. Thank you.
---------------------------------------------------------------------------
by lsmith77 at 2012/01/08 11:45:04 -0800
@fabpot: but then we should also add a list of interfaces to the API http://screencast.com/t/vu4Tljkri0
Commits
-------
10ecaba slovenian validators.xlf updated
Discussion
----------
Validators.sl.xlf updated
PR sent intentionally to symfony:master because of different translations set in translations.XYZ.xlf and translations.XYZ.xliff in current 2.0 branch.
Commits
-------
c9129e5 Fix Console tests on windows
Discussion
----------
Fix Console tests on windows (2.0)
Rebased #3059 on 2.0 - once this is applied I will rebase it again because some of the fixes could not be applied to 2.0.
Commits
-------
1728493 made the assertions in the RequestTest more explicit and improved PHPDoc
Discussion
----------
made the assertions in the RequestTest more explicit and improved PHPDoc
as discussed in 4f1edb50d8 (comments)
Commits
-------
9441c46 [DependencyInjection] PhpDumper, fixes#2730
Discussion
----------
[DependencyInjection] PhpDumper, fixes#2730
Hey, this PR fixes#2730, if no parameters are set, the constructor wont get passed a ParameterBag
Bug fix: yes (#2730)
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
3rd and last try ;) this time i think its all fine
BC Break: no
Feature addition: yes
Symfony2 tests pass: yes
Fixes the following issues: #2790
Todo: need more testing
This PR enables usage of empty string as a form name (only at root level).
Commits
-------
1cd74ec Added norwegian translations of validators
Discussion
----------
Added norwegian translations of validators
---------------------------------------------------------------------------
by stof at 2011/12/29 10:14:43 -0800
Can you send a PR to the 2.0 branch instead of master to add these translation for the ids 1 to 41 (missing in your PR btw) ? and then another PR to master for the ids 42 to 48 which are new for 2.1 ?
---------------------------------------------------------------------------
by antonbabenko at 2011/12/29 10:59:39 -0800
Ok, will do, but where can I find the correct original one ? I took german file as the most complete. Some languages have different amount of phrases and sources.
---------------------------------------------------------------------------
by javiereguiluz at 2011/12/29 11:23:04 -0800
@antonbabenko you can use the Spanish translation as an example (it was updated very recently and I initially made the same mistake ;) ):
* #2968 for 2.0 branch (added id 41)
* #2969 for master branch (added ids 42 - 48)
---------------------------------------------------------------------------
by antonbabenko at 2011/12/29 11:28:03 -0800
Thanks Javier.
Commits
-------
7c8bd3d [FrameworkBundle] Invalid composer ref fix
Discussion
----------
[FrameworkBundle] Invalid composer ref fix
Changes the `composer.json` reference in the FrameworkBundle to use the `symfony/translation` package rather than the current `symfony/translator` (which doesn't exist).
Commits
-------
261325d Cast $query['params'] to array to ensure it is a valid argument for the foreach.
Discussion
----------
[DoctrineBridge] Account for $query['params'] being null in DoctrineDataCollector's sanitizeQueries method
I was getting this error because inside the sanitizeQueries method of DoctrineDataCollector, there's no handling for $query['params'] being null.
ErrorException: Warning: Invalid argument supplied for foreach() in .../vendor/symfony/src/Symfony/Bridge/Doctrine/DataCollector/DoctrineDataCollector.php line 91
Commits
-------
0ed3497 [FrameworkBundle][translations] Updated Catalan translation
Discussion
----------
[FrameworkBundle][translations] Updated Catalan translation
Added some translations
---------------------------------------------------------------------------
by stof at 2012/01/04 19:14:29 -0800
Can you send the trans-unit 41 to the 2.0 branch as it is already part of the 2.0 release (ids 42 and above are new for 2.1)
---------------------------------------------------------------------------
by franmomu at 2012/01/05 00:53:49 -0800
Of course, I didn't realize
Apache expects the response to already be in chunked format in that case,
which causes it to not deliver the streamed body.
If no Content-Length is set on the response, web servers will automatically
switch to chunked Transfer-Encoding, and handle the chunking for you.
Nginx does not share the issue that apache has, but will add the Content-
Length header too.
Commits
-------
c37c145 [SecurityBundle] Only throw exception if check_path looks like an url
Discussion
----------
[SecurityBundle] Only throw exception if check_path looks like an url
fixes#2954 and enables the usage of route names like you can in all other paths
Commits
-------
20f96bd Fix CS
2f47cca [Propel1] Add a PropelExtension
Discussion
----------
[Propel1] Add a PropelExtension
The propel extension allow to use propel specific type (ModelType) outside of
Symfony2.
---------------------------------------------------------------------------
by rouffj at 2012/01/01 15:36:10 -0800
@Stof Fixed :).
---------------------------------------------------------------------------
by willdurand at 2012/01/02 07:38:22 -0800
👍
Commits
-------
887c0e9 moved EngineInterface::stream() to a new StreamingEngineInterface to keep BC with 2.0
473741b added the possibility to change a StreamedResponse callback after its creation
8717d44 moved a test in the constructor
e44b8ba made some cosmetic changes
0038d1b [HttpFoundation] added support for streamed responses
Discussion
----------
[HttpFoundation] added support for streamed responses
To stream a Response, use the StreamedResponse class instead of the
standard Response class:
$response = new StreamedResponse(function () {
echo 'FOO';
});
$response = new StreamedResponse(function () {
echo 'FOO';
}, 200, array('Content-Type' => 'text/plain'));
As you can see, a StreamedResponse instance takes a PHP callback instead of
a string for the Response content. It's up to the developer to stream the
response content from the callback with standard PHP functions like echo.
You can also use flush() if needed.
From a controller, do something like this:
$twig = $this->get('templating');
return new StreamedResponse(function () use ($templating) {
$templating->stream('BlogBundle:Annot:streamed.html.twig');
}, 200, array('Content-Type' => 'text/html'));
If you are using the base controller, you can use the stream() method instead:
return $this->stream('BlogBundle:Annot:streamed.html.twig');
You can stream an existing file by using the PHP built-in readfile() function:
new StreamedResponse(function () use ($file) {
readfile($file);
}, 200, array('Content-Type' => 'image/png');
Read http://php.net/flush for more information about output buffering in PHP.
Note that you should do your best to move all expensive operations to
be "activated/evaluated/called" during template evaluation.
Templates
---------
If you are using Twig as a template engine, everything should work as
usual, even if are using template inheritance!
However, note that streaming is not supported for PHP templates. Support
is impossible by design (as the layout is rendered after the main content).
Exceptions
----------
Exceptions thrown during rendering will be rendered as usual except that
some content might have been rendered already.
Limitations
-----------
As the getContent() method always returns false for streamed Responses, some
event listeners won't work at all:
* Web debug toolbar is not available for such Responses (but the profiler works fine);
* ESI is not supported.
Also note that streamed responses cannot benefit from HTTP caching for obvious
reasons.
---------------------------------------------------------------------------
by Seldaek at 2011/12/21 06:34:13 -0800
Just an idea: what about exposing flush() to twig? Possibly in a way that it will not call it if the template is not streaming. That way you could always add a flush() after your </head> tag to make sure that goes out as fast as possible, but it wouldn't mess with non-streamed responses. Although it appears flush() doesn't affect output buffers, so I guess it doesn't need anything special.
When you say "ESI is not supported.", that means only the AppCache right? I don't see why this would affect Varnish, but then again as far as I know Varnish will buffer if ESI is used so the benefit of streaming there is non-existent.
---------------------------------------------------------------------------
by cordoval at 2011/12/21 08:04:21 -0800
wonder what the use case is for streaming a response, very interesting.
---------------------------------------------------------------------------
by johnkary at 2011/12/21 08:19:48 -0800
@cordoval Common use cases are present fairly well by this RailsCast video: http://railscasts.com/episodes/266-http-streaming
Essentially it allows faster fetching of web assets (JS, CSS, etc) located in the <head></head>, allowing those assets to be fetched as soon as possible before the remainder of the content body is computed and sent to the browser. The end goal is to improve page load speed.
There are other uses cases too like making large body content available quickly to the service consuming it. Think if you were monitoring a live feed of JSON data of newest Twitter comments.
---------------------------------------------------------------------------
by lsmith77 at 2011/12/21 08:54:35 -0800
How does this relate the limitations mentioned in:
http://yehudakatz.com/2010/09/07/automatic-flushing-the-rails-3-1-plan/
Am I right to understand that due to how twig works we are not really streaming the content pieces when we call render(), but instead the entire template with its layout is rendered and only then will we flush? or does it mean that the render call will work its way to the top level layout template and form then on it can send the content until it hits another block, which it then first renders before it continues to send the data?
---------------------------------------------------------------------------
by stof at 2011/12/21 09:02:53 -0800
@lsmith77 this is why the ``stream`` method calls ``display`` in Twig instead of ``render``. ``display`` uses echo to print the output of the template line by line (and blocks are simply method calls in the middle). Look at your compiled templates to see it (the ``doDisplay`` method)
Rendering a template with Twig simply use an output buffer around the rendering.
---------------------------------------------------------------------------
by fabpot at 2011/12/21 09:24:33 -0800
@lsmith77: We don't have the Rails problem thanks to Twig as the order of execution is the right one by default (the layout is executed first); it means that we can have the flush feature without any change to how the core works. As @stof mentioned, we are using `display`, not `render`, so we are streaming your templates for byte one.
---------------------------------------------------------------------------
by fabpot at 2011/12/21 09:36:41 -0800
@Seldaek: yes, I meant ESI with the PHP reverse proxy.
---------------------------------------------------------------------------
by fabpot at 2011/12/21 09:37:34 -0800
@Seldaek: I have `flush()` support for Twig on my todo-list. As you mentioned, It should be trivial to implement.
---------------------------------------------------------------------------
by fzaninotto at 2011/12/21 09:48:18 -0800
How do streaming responses deal with assets that must be called in the head, but are declared in the body?
---------------------------------------------------------------------------
by fabpot at 2011/12/21 09:52:12 -0800
@fzaninotto: What do you mean?
With Twig, your layout is defined with blocks ("holes"). These blocks are overridden by child templates, but evaluated as they are encountered in the layout. So, everything works as expected.
As noted in the commit message, this does not work with PHP templates for the problems mentioned in the Rails post (as the order of execution is not the right one -- the child template is first evaluated and then the layout).
---------------------------------------------------------------------------
by fzaninotto at 2011/12/21 10:07:35 -0800
I was referring to using Assetic. Not sure if this compiles to Twig the same way as javascript and stylesheet blocks placed in the head - and therefore executed in the right way.
---------------------------------------------------------------------------
by fabpot at 2011/12/21 10:34:59 -0800
@Seldaek: I've just added a `flush` tag in Twig 1.5: 1d6dfad4f5
---------------------------------------------------------------------------
by catchamonkey at 2011/12/21 13:29:22 -0800
I'm really happy you've got this into the core, it's a great feature to have! Good work.
* 2.0:
[Tests] Skip segfaulting form test
Rename test file
[BrowserKit] added missing @return PHPDoc for the Client::submit() method.
also test PHP 5.3.2, since this is the official lowest supported PHP version
Commits
-------
85ca8e3 ParameterBag no longer resolves parameters that have spaces.
99011ca Added tests for ParameterBag parameters with spaces
Discussion
----------
[DependencyInjection] Parameters with spaces are not resolved
Bug fix: yes
Feature addition: no
Backwards compatibility break: no (not likely, according to convention)
Symfony2 tests pass: yes
Fixes the following tickets: #2884
`ParameterBag` currently resolves anything between two `%` signs, which creates issues for any parameters in the DIC that are legitimate text. This PR enforces the [documented parameter convention](http://symfony.com/doc/2.0/book/service_container.html#service-parameters) so that only `%parameters.with.no_spaces%` are resolved.
I was considering using instead `^%([^\w\._-]+)%$`, but felt that was too constricting & could easily introduce issues with existing applications.
Commits
-------
41950a6 [WebProfilerBundle] add margin-bottom to caption
Discussion
----------
[WebProfilerBundle] add margin-bottom to caption
---------------------------------------------------------------------------
by fabpot at 2011/12/26 13:15:57 -0800
What does it fix?
---------------------------------------------------------------------------
by havvg at 2011/12/27 02:46:16 -0800
Just a minor design issue with table captions.
Without: http://dl.dropbox.com/u/548684/PR2957/without-margin.png
With: http://dl.dropbox.com/u/548684/PR2957/with-margin.png
I currently hold it in a custom css, but thought it is generic enough to be put into the bundle.
---------------------------------------------------------------------------
by henrikbjorn at 2011/12/27 04:03:53 -0800
@havvg What custom bundle is that ? and what does it show? look interesting
---------------------------------------------------------------------------
by havvg at 2011/12/27 04:40:18 -0800
@henrikbjorn It's a bundle (not published yet, https://github.com/havvg/HavvgCloudcontrolBundle) adding features to fully utilize applications on http://cloudcontrol.com PaaS.
Commits
-------
3f2e1b0 [Console] Updated tests to reflect the change from `program` to `application`
2b64944 [Console][Output] Fixed some minor typos and grammatical errors
96997f1 [Console][Input] Added missing PHPDoc and fixed some minor typos and grammatical errors
855b8af [Console][Helper] Added missing PHPDoc and fixed some minor typos and grammatical errors
3ad02bd [Console][Formatter] Added missing PHPDoc @throws and fixed some minor typos and grammatical errors
33e3f11 [Console] Added a missing PHPDoc and replaced `program` by `application`
Discussion
----------
[Console] Fixed and completed PHPDoc
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
* Fixed minor typos and grammatical errors
* Added missing PHPDoc for some methods
* Added missing @throws
Commits
-------
1bc10e5 Forgotten </trans-unit> tag added
Discussion
----------
Forgotten </trans-unit> tag added
on src/Symfony/Bundle/FrameworkBundle/Resources/translations/validators.es.xlf
Commits
-------
aacb2de use the forward compat version in the Filesystem service
Discussion
----------
use the forward compat version in the Filesystem service
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: ![Build Status](https://secure.travis-ci.org/lsmith77/symfony.png?branch=forward_compat_filesystem)
Fixes the following tickets: -
by changing the service it should fix any type hints for the Filesystem class inside 2.1, but it shouldn't affect anyone still type hinting the old location in 2.0 since the new forward compat file extends the old file.
See
00c988bf0c (commitcomment-820879)
---------------------------------------------------------------------------
by tobiassjosten at 2011/12/26 18:41:45 -0800
👍
Commits
-------
cae7db0 Be more tolerant and also accept <esi:include ...></esi:include>, also if it is not 100% standards compliant.
Discussion
----------
Be more tolerant and also accept <esi:include ...></esi:include>
I know this is not 100% standards compliant, but:
We need to do some XHTML processing on the output using PHP's DOM extension and the underlying libxml2.
libxml2 seems to be unable to keep the <esi:include /> tag as such and will expand it to ```<esi:include ...></esi:include>```.
Note this has nothing to do with having LIBXML_NOEMPTYTAG set (http://php.net/manual/de/domdocument.savexml.php). Rather it seems to be a problem for libxml that it cannot recognize <esi:include> as an "EMPTY" tag (in the DTD sense) because it is not defined in a standard xhtml1-strict DTD.
Commits
-------
79793e4 Coding standards and removing whitespace.
Discussion
----------
Coding standards and removing whitespace.
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Please note `2.0` cs passes, this is specifically for the `master` branch.
Commits
-------
6e98730 added forwards compatibility for the Filesystem component
Discussion
----------
added forwards compatibility for the Filesystem component
see #2949 and #2946
---------------------------------------------------------------------------
by willdurand at 2011/12/25 05:24:01 -0800
👍
---------------------------------------------------------------------------
by ManuelAC at 2011/12/25 06:38:44 -0800
👍
---------------------------------------------------------------------------
by NAYZO at 2011/12/25 07:45:55 -0800
👍
---------------------------------------------------------------------------
by avalanche123 at 2011/12/25 09:52:59 -0800
I think extracting interface and committing it to both branches is preferable.
---------------------------------------------------------------------------
by lsmith77 at 2011/12/25 12:17:59 -0800
what interface?
---------------------------------------------------------------------------
by avalanche123 at 2011/12/25 12:19:03 -0800
FilesystemInterface
---------------------------------------------------------------------------
by lsmith77 at 2011/12/25 12:22:31 -0800
an interface doesn't really relate to this PR .. aka its a separate topic. the purpose of this PR is to make it easier to write code targeting with 2.0 and 2.1 with minimal impact.
---------------------------------------------------------------------------
by willdurand at 2011/12/25 12:23:51 -0800
Seems overkill to add an interface here. I dont think there are different implementations.
Le 25 déc. 2011 à 21:19, Bulat Shakirzyanov<reply@reply.github.com> a écrit :
> FilesystemInterface
>
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/symfony/symfony/pull/2956#issuecomment-3271576
---------------------------------------------------------------------------
by avalanche123 at 2011/12/25 12:28:49 -0800
@lsmith77 point taken, makes sense
@willdurand there are plenty of alternatives to default filesystem (databases, s3, etc.)
---------------------------------------------------------------------------
by willdurand at 2011/12/25 12:52:58 -0800
Gaufrette (KnpLabs) is suitable for what you describe. I don't know if this component is designed to handle various adapters.
This is just a layer for files manipulations on disk.
Le 25 déc. 2011 à 21:28, Bulat Shakirzyanov<reply@reply.github.com> a écrit :
> @lsmith77 point taken, makes sense
> @willdurand there are plenty of alternatives to default filesystem (databases, s3, etc.)
>
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/symfony/symfony/pull/2956#issuecomment-3271596