Commits
-------
db2d773 [FrameworkBundle] Improve the TemplateLocator exception message
Discussion
----------
Template locator/exception message
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Improve the error message to include the error message from the File Locator which is more accurate : the File Locator might also look in some fallback folder(s) (i.e. `%kernel.root_dir%/Resources`)
Commits
-------
40e5b60 [FrameworkBundle] added return type for getContainer()
Discussion
----------
added return type for getContainer()
IDE type inference support
Commits
-------
3759ff0 [Locale] StubNumberFormatter allow to parse 64bit number in 64bit mode
Discussion
----------
[Locale] StubNumberFormatter allow to parse 64bit number in 64bit mode
Bug fix: yes
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: [](http://travis-ci.org/stealth35/symfony)
Fixes the following tickets: #2735
---------------------------------------------------------------------------
by stealth35 at 2011/12/01 06:47:32 -0800
@Seldaek should be better now
---------------------------------------------------------------------------
by stealth35 at 2011/12/02 04:22:42 -0800
@fabpot done
---------------------------------------------------------------------------
by fabpot at 2011/12/02 04:28:24 -0800
Tests do not pas for me (on a Mac):
1) Symfony\Tests\Component\Locale\Stub\StubNumberFormatterTest::testParseTypeInt64StubWith64BitIntegerInPhp64Bit
->parse() TYPE_INT64 does not use true 64 bit integers, using only the 32 bit range.
Failed asserting that 2147483648 matches expected -2147483648.
.../tests/Symfony/Tests/Component/Locale/Stub/StubNumberFormatterTest.php:819
---------------------------------------------------------------------------
by stealth35 at 2011/12/02 04:50:20 -0800
@fabpot, could you send me the return of this code
``` php
<?php
$formatter = new \NumberFormatter('en', \NumberFormatter::DECIMAL);
$value = $formatter->parse('2,147,483,648', \NumberFormatter::TYPE_INT64);
var_dump($value);
$value = $formatter->parse('-2,147,483,649', \NumberFormatter::TYPE_INT64);
var_dump($value);
```
---------------------------------------------------------------------------
by fabpot at 2011/12/02 06:06:21 -0800
int(-2147483648)
int(2147483647)
---------------------------------------------------------------------------
by stealth35 at 2011/12/02 06:10:28 -0800
It's nosens, but the Stub should follow Intl ... so I fix that
---------------------------------------------------------------------------
by stealth35 at 2011/12/11 08:48:25 -0800
It's OK now
Commits
-------
bb0d202 Switched sanitizeParameter() for existing varToString()-method; now always stores a string representation of each parameter
4fe4dfd Fixed vendor version mismatch in tests
28730e9 [DoctrineBridge] Added unit tests
4535abe [DoctrineBridge] Fixed attempt to serialize non-serializable values
Discussion
----------
[DoctrineBridge] Fixed attempt to serialize non-serializable values
Bug fix: yes
Feature addition: no
Backwards compatibility break: no (99%)
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
The Doctrine DBAL type system does not pose any restrictions on the php-types of parameters in queries. Hence one could write a doctrine-type that uses a resource or an `\SplFileInfo` as its corresponding php-type. Parameters of these types are logged in the `DoctrineDataCollector` however, which is then serialized in the profiler. Since resources or `\SplFileInfo` variables cannot be serialized this throws an exception.
This PR fixes this problem (for known cases) by sanitizing the query parameters to only contain serializable types. The `isNotSerializable`-check surely is not complete yet, but more non-serializable classes can be added on a case-by-case basis.
---------------------------------------------------------------------------
by fabpot at 2011/12/07 07:04:43 -0800
Tests do not pass for me.
Furthermore, let's reuse what we already have in the framework (see https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpKernel/HttpKernel.php#L187 -- yes you can just copy/paster the existing code).
---------------------------------------------------------------------------
by aboks at 2011/12/09 01:41:14 -0800
@fabpot I fixed the tests (seems I had the wrong vendor versions in my copy) and reused the `varToString()`-code. This introduces a tiny BC break in the rare case that someone writes his own templates for the web profiler (the parameters returned by the data collector are now always a string; could be any type before).
After merging this PR, merging 2.0 into master would give a merge conflict and failing tests (because of the changes related to the introduction of the `ManagerRegistry` interface). To prevent this, please merge #2820 into master directly after merging this PR (so before merging 2.0 into master). After that 2.0 can be cleanly merged into master.
---------------------------------------------------------------------------
by stof at 2011/12/09 03:43:38 -0800
it is not a BC break. Using ``yaml_encode`` on a string will not break the template
This changes helps the common use case of fetching the current user and better complies with the Law of Demeter (http://en.wikipedia.org/wiki/Law_of_Demeter).
Before (still works):
$token = $context->getToken();
$user = $token ? $token->getUser() : null;
After:
$user = $context->getUser();
Commits
-------
e06cea9 [HttpFoundation] Cookie values should not be restricted
Discussion
----------
[HttpFoundation] Cookie values should not be restricted
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
The restriction I removed makes no sense IMO because we do not use setrawcookie() to send cookies. setrawcookie() does throw a warning when the cookie value contains incorrect characters, but not setcookie(). The latter will just urlencode() the value so it becomes valid. This is also what is done by `Cookie::__toString`, so this could be used in combination with header() to just send raw cookies that are valid, even with values that are invalid in their decoded form.
PHP urldecodes cookies on input, so it all works fine.
Commits
-------
59397cf [DoctrineBridge] Generalize EntityValidator to work with any validation service and against any Common ClassMetadata provider
Discussion
----------
[DoctrineBridge] Generalize EntityValidator to work with any validation ...
...service and against any Common ClassMetadata provider. Also decoupled the Bridge from its implicit dependency on the "doctrine.orm.vaildator.unique" service.
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: no
Commits
-------
70e9332 added check for invalid user providers
Discussion
----------
[security] added check for invalid user providers
I've added an exception if an invalid user provider is passed into a context listener.
The FOSFacebookBundle configuration has been documented wrong for a long time. This exception prevent users from configuring the security layer incorrectly.
```
Bug fix: no
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: ~
Todo: ~
```
---------------------------------------------------------------------------
by fabpot at 2011/12/06 00:40:08 -0800
What about doing that in the 2.0 branch?
---------------------------------------------------------------------------
by kriswallsmith at 2011/12/06 03:23:59 -0800
It is possible it will break some applications -- a non-user provider that is not first in the array may never be called.
Commits
-------
da0773c Note DependencyInjection exception changes in the 2.1 changelog
9a090bc [DependencyInjection] Fix class check and failure message in PhpDumper test
2334596 [DependencyInjection] Use component's SPL classes in dumped service container
3c02ea2 [DependencyInjection] Use exception class for API doc generation
47256ea [DependencyInjection] Make exceptions consistent when ContainerBuilder is frozen
b7300d2 [DependencyInjection] Fix up @throws documentation
123f514 [DependencyInjection] Use component-specific SPL exceptions
cf2ca9b [DependencyInjection] Create additional SPL exceptions
ba8322e [DependencyInjection] Format base exception classes consistently
Discussion
----------
[DependencyInjection] Refactor usage of SPL exceptions
```
Bug fix: no
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: -
```
This was something discussed on the mailing list recently and a topic at the last IRC meeting. The DependencyInjection component was what I had in mind while discussing this with @lsmith a few weeks ago, as I found that it already had some local SPL exceptions, which weren't being used directly (only as base classes for the compiler exceptions). This PR replaces uses of core SPL exceptions with the component's equivalents. I've listed it as BC-breaking to be safe, but I don't think it should cause any trouble, since the new exceptions are sub-classes of those originally used.
I purposely avoided changing the exceptions in the dumped PHP container. If we agree that's worth doing, it would be a trivial addition to the PR.
---------------------------------------------------------------------------
by jmikola at 2011/12/04 22:38:47 -0800
One question I came across while implementing this PR: the doc blocks in ContainerInterface reference InvalidArgumentException (actually the component's local SPL equivalent), but there is no practical need for that class to be used in the file. How will the API doc generator handle this? Do we need to change the doc block reference to the full class path?
---------------------------------------------------------------------------
by fabpot at 2011/12/05 01:20:31 -0800
Why have you not replaced the exception in the dumped container code? I don't see any drawback.
---------------------------------------------------------------------------
by stof at 2011/12/05 03:39:25 -0800
it would even be better to be consistent in the generated code
---------------------------------------------------------------------------
by jmikola at 2011/12/05 09:48:05 -0800
I'll update the exceptions in the dumped container code as well. Thanks for the feedback.
---------------------------------------------------------------------------
by jmikola at 2011/12/05 10:41:21 -0800
Ok, this should be ready to review and merge. Tests needed some updating, but they still pass.
Commits
-------
7c1cbb9 [Config] Use LoaderResolverInterface for type-hinting
48b084e fixed typo
8ad94fb merged branch hhamon/doctrine_bridge_cs (PR #2775)
240796e [Bridge] [Doctrine] fixed coding conventions.
7cfc392 check for session before trying to authentication details
648fae7 merged branch proofek/domcrawlerform-radiodisabled (PR #2768)
3976b7a [DoctrineBridge] fixed CS
9a04783 merged branch beberlei/SecurityEntityRepositoryIdentifierFix (PR #2765)
3c83b89 [DoctrineBridge] Catch user-error when the identifier is not serialized with the User entity.
36c7d03 Fixed GH-2720 - Fix disabled atrribute handling for radio form elements
Discussion
----------
[Config] Use LoaderResolverInterface for type-hinting
```
Bug fix: no
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: -
```
I've listed this as a BC break because we're changing the argument type-hint, but I think it's unlikely to affect anyone.
Some methods previously threw LogicExceptions when the ContainerBuilder was frozen. Using BadMethodCallException (a descendant of LogicException) in all cases improves consistency and preserves BC.
This replaces existing use of core SPL exceptions with the equivalent classes defined within the component. Although method documentation has been changed, this change should be BC since the component-specific SPL exceptions extend their core counterpart.
This commit purposely omits any changes to the PhpDumper, which throws several core SPL exceptions.
Commits
-------
36c7d03 Fixed GH-2720 - Fix disabled atrribute handling for radio form elements
Discussion
----------
Fixed GH-2720 - Fix disabled atrribute handling for radio form elements
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: - GH-2720
I wasn't really sure about the correct approach. This one is very minimalistic and following the existing concept of not duplicating nodes of the same name, but only storing multiple values for the same node. If you think that should be changed, let me know. Hints appreciated.
Thanks
Commits
-------
8710a13 Added example to the change log file
c9a2b49 Fixed xml encoder test script, and group `item` tags into an array
a0561e5 Replaced `item` with `*item` when parsing XML string
Discussion
----------
Replaced `item` with `*item` when parsing XML string
---------------------------------------------------------------------------
by fabpot at 2011/11/23 22:14:12 -0800
Tests do not pass:
1) Symfony\Tests\Component\Serializer\Encoder\XmlEncoderTest::testDecode
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
'key2' => 'val'
- 'A B' => 'bar'
'Barry' => Array (...)
+ 'item' => Array (...)
)
'qux' => '1'
)
.../tests/Symfony/Tests/Component/Serializer/Encoder/XmlEncoderTest.php:173
---------------------------------------------------------------------------
by fabpot at 2011/11/24 22:57:37 -0800
I don't understand the patch anymore. I don't see any use of `*item` in the code.
---------------------------------------------------------------------------
by excelwebzone at 2011/11/24 23:04:07 -0800
I run some testing and you can't use '*item' XML parser reject it. So I modified it to convert it to an array.. Look at the test script change
---------------------------------------------------------------------------
by fabpot at 2011/11/24 23:13:30 -0800
So, you probably need to change the CHANGELOG as well? You should add an example which shows a before/after example.
---------------------------------------------------------------------------
by excelwebzone at 2011/11/24 23:15:51 -0800
Yes, forgot to change that..
---------------------------------------------------------------------------
by fabpot at 2011/11/25 01:27:42 -0800
ping @Seldaek, @lsmith77
---------------------------------------------------------------------------
by Seldaek at 2011/11/25 04:16:43 -0800
There are other meta-names available in the XmlEncoder, @-something for attributes, then there is something happening with a # but I'm not quite sure what. I'm just saying, maybe *item isn't the best name, if it introduces a third metacharacter. Apart from that I'm fine with it.
---------------------------------------------------------------------------
by excelwebzone at 2011/11/25 08:45:31 -0800
Maybe we can rename it to `wildcard` instead
---------------------------------------------------------------------------
by excelwebzone at 2011/11/25 15:12:09 -0800
Any chance we can push this throw?
---------------------------------------------------------------------------
by lsmith77 at 2011/11/27 04:06:25 -0800
here is the old PR #2682
@Seldaek: i think your comment was made for an older version of the patch.
overall I am fine with the change, the Serializer component takes a fairly simple approach. it is also not designed to really produce XML or JSON cleanly from the same data. it will really only be able to output a clean API for one or the other with the same data structure.
---------------------------------------------------------------------------
by excelwebzone at 2011/12/01 06:25:24 -0800
@fabpot can we merge this change
Commits
-------
0be8820 [FrameworkBundle] fixed small typo in Czech translation
Discussion
----------
[FrameworkBundle] fixed small typo in Czech translation
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
fixed small typo in Czech translation in validators.cs.xliff
Commits
-------
4a8f101b Fixed problem with multiple occurences of a given namespace. fix#2688
Discussion
----------
[Console] [Doctrine] Fixed: Entities are generated in wrong folder (doctrine:generate:entities Namespace)
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: no
Fixes the following tickets: 2688
Todo: -
Bug-description:
In our project we have some bundles from the same root-namespace "ABC" stored under "vendors/bundles/ABC" and some bundles under "src/ABC".
Running the command "$php app/console doctrine:generate:entities ABC" from the commandline generates the entities from "src/ABC/" under "vendors/bundles/ABC/" or vice versa depending on their order in the bundle array in the Appkernel.php.
The error does not occur when the entities are generated by bundlename or by classname.
Bugfix:-description:
Bugfix was to get the path for each entity class once more before generating the entities. Before the bugfix the path was taken form the first entity class of the namespace without checking if the following entities have a different path.
Commits
-------
63e2a99 [CssSelector] Fixed Issue for XPathExprOr: missing prefix in string conversion
Discussion
----------
[CssSelector] Issue for XPathExprOr: missing prefix in string conversion
Hi there,
I created a small and dumb test for the issue. I looked at the original implementation and i think the problem is, that private properties are used in the parent class for xPathExprOr. so that the prefix cannot be accessd with $this->prefix in XPathExprOr
However I think the distribution for the prefix should be put in the parts of the or-sub-expressions the way it is shown in the test.
Hope this helps.
Best regards
Philipp
Commits
-------
c4303f3 TraceableEventDispatcher uses 'Closure' as the StopWatch name for Closures
0b5499e TraceableEventDispatcher test for Closures
Discussion
----------
[FrameworkBundle] (PR) TraceableEventDispatcher creates NOTICE on Closures
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #2743
Currently a `\Closure` will cause the error:
> Notice: Undefined index: class in ...\TraceableEventDispatcher`
The test will fail in the event this Notice occurs & passes when the Closure fires correctly.
**Note**: If you use error suppression (`@$dispatcher->dispatch('onFooEvent')`), then the notice does not occur (as expected) and the listener fires correctly. This, in my opinion, is expected behavior.
If you'd like this change rebased against another branch, let me know...
Commits
-------
9e6a10a [Form] Added FormError::getMessage() and use it in Form class
Discussion
----------
[Form] Added FormError::getMessage()
---------------------------------------------------------------------------
by ericclemmons at 2011/11/29 18:38:40 -0800
Should this go through the translator, similar to how `field_errors` renders error messages?
> https://github.com/symfony/symfony/blob/master/src/Symfony/Bridge/Twig/Resources/views/Form/form_div_layout.html.twig#L253
---------------------------------------------------------------------------
by stof at 2011/11/29 23:11:24 -0800
``getErrorsAsString`` is there for a debugging purpose so injecting the translator in the Form class just for it seems wrong. And the logic used here is exactly what the identity translator does.
Bug fix: yes
Feature add: no
Symfony2 tests pass: yes
Symfony2 tests added: yes
In general without this exception generated by php dumper container class, will cause PHP fatal error, bacause method call will look like this: `$instance->(/* arguments*/);`.
Commits
-------
6d5b296 Adjust doctrine requirements
628cce1 Adjust composer files to strictly require known to work packages
Discussion
----------
Adjust composer files to strictly require known to work packages
I also bumped doctrine common to 2.2, because it seems 2.1 doesn't have the AbstractManagerRegistry stuff that is used by symfony 2.1. Please someone correct me if this is wrong.
---------------------------------------------------------------------------
by fabpot at 2011/11/25 07:59:53 -0800
It should be done on the 2.0 branch too, no?
---------------------------------------------------------------------------
by Seldaek at 2011/11/26 16:11:25 -0800
Well as we discussed the 2.0 branch is not a priority right now, but yes if someone wants to port the changes to 2.0 it'd be good.
When controller is a Closure ControllerResolver::getArguments tries to
make a ReflectionMethod of the __invoke method. But because it's an
internal function, the parameters method isDefaultValueAvailable will
return always false, even if isOptional return true.
Commits
-------
09562df Update CHANGELOG for 2.1, describe new auth events
cf09c2d added authentication success/failure events
Discussion
----------
[Security] Implementation of a "failed login" event, replaces: PR #1307
As I have to use this feature I have completed its implementation.
Bugfix: no
Feature addition: yes
Symfopny2 tests pass: yes
Replaces/closes PR: #1307
---------------------------------------------------------------------------
by schmittjoh at 2011/11/18 23:57:56 -0800
Usually, this event is used for the wrong reasons (to customize what happens on authentication failure). Can you move your implementation to the AuthenticationProviderManager instead?
see https://github.com/schmittjoh/symfony/blob/master/src/Symfony/Component/Security/Core/Authentication/AuthenticationProviderManager.php#L103
---------------------------------------------------------------------------
by canni at 2011/11/19 06:00:36 -0800
Good point :) I'll not rewrite yours work, I've cherry-picked yours commits. (BTW you added call to `setEventDispatcher` on `security.authentication.manager` to commit related to some different work ;)
---------------------------------------------------------------------------
by fabpot at 2011/11/22 00:12:19 -0800
The new files are missing the LICENSE header. As far as I can see, @schmittjoh fork has a different license from the Symfony one. This needs to be clarified before I can merge this PR.
---------------------------------------------------------------------------
by schmittjoh at 2011/11/22 01:53:09 -0800
No biggy, MIT is fine here.
---------------------------------------------------------------------------
by canni at 2011/11/22 01:57:51 -0800
@fabpot done
---------------------------------------------------------------------------
by fabpot at 2011/11/22 02:22:47 -0800
@canni: Can you update the CHANGELOG file (to reference the changes and the BC breaks -- like the move of KernelEvents for instance).
---------------------------------------------------------------------------
by canni at 2011/11/22 02:40:33 -0800
@fabpot: no problem & done
PS I haven't realized that namespace change of `SecurityEvents` is actually a BC Break, thx for pointing this.
---------------------------------------------------------------------------
by fabpot at 2011/11/22 03:06:17 -0800
@canni: What about keeping a `SecurityEvents` class in the `Http` namespace that just extends the new one. That way, we don't break BC.
---------------------------------------------------------------------------
by canni at 2011/11/22 03:53:01 -0800
@fabpot: that will force us to remove `final` keyword form one of classes.
Maybe we can add new, not extending class e.g.: `GeneralSecurityEvents` or `AuthenticationEvents`, that way we dont break BC and dont introduce confusion in naming?
---------------------------------------------------------------------------
by canni at 2011/11/22 05:53:15 -0800
@fabpot: I've removed the BC break, and squashed schmittjoh commits, to keep things nice and clear.
I've changed Schema.php to not use Restrict on delete/update since
oracle report it as missing keyword. Both restrict and no action on
oracle seems to be redundant and used by default. So the output query
can't use it. I've also changed Schema construct to accept a
SchemaConfig parameter. InitAcl was changed to pass on new Schema a
SchemaConfig generated by SchemaManager, I did that because acl command
was generating names with more than 30 characters and Oracle doesn't
accept, this seems to solve the problem and init:acl works properly.
Commits
-------
e7215ae Fix composer.json
Discussion
----------
Fix composer.json files
>=2.1 does not include the 2.1.0-dev versions, since those are lesser than 2.1.0 stable, so nothing is really working right now.
Commits
-------
11b6156 updated unittest
a931e21 get correct client IP from X-forwarded-for header
Discussion
----------
[HttpFoundation] Get correct client IP when using trusted proxy (Varnish)
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
Note: This is reopened PR #2686 for 2.0 branch.
If using trusted proxy (Varnish, ...) the client IP must be identified from X-Forwarded-For header. The header has de-facto standard format:
X-Forwarded-For : client1, proxy1, proxy2,
where the value is a comma+space separated list of IP addresses, the left-most being the farthest downstream client, and each successive proxy that passed the request adding the IP address where it received the request from. See: http://en.wikipedia.org/wiki/X-Forwarded-For
Function getClientIp should return only one client IP, not a list of all nonimportant IPs as it's now. Similar example can be seen in Cake framework: http://api.cakephp.org/view_source/request-handler-component/#line-477
There are many ways how to chose the first IP from X-Forwarded-For header. Any other faster and more reliable way is welcome.
Commits
-------
b6bf018 tweaked error handling for the forward compatibility
dd606b5 added note about the purpose of this class
c1426ba added locale handling forward compatibility
10eed30 added MessageDataCollector forward compatibility
Discussion
----------
Forward compat
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #2522
Commits
-------
78e9b2f [Form] Fixed textarea_widget (W3C standards)
Discussion
----------
[Form] Fixed textarea_widget (W3C standards)
Textarea widget included the "pattern" attribute but is not valid by W3C standards.
(See PR 2666 - New PR because rebase inside the 2.0 branch)
---------------------------------------------------------------------------
by fabpot at 2011/11/18 09:01:41 -0800
@hlecorche: Thanks for your work on this issue. Can you update the unit tests to be sure that this case is covered? If you're not comfortable with this, just tell me and I will do it myself
---------------------------------------------------------------------------
by hlecorche at 2011/11/19 02:51:06 -0800
@fabpot: I did'nt commited because I am not sure. I changed the "tests/Symfony/Tests/Component/Form/AbstractLayoutTest.php" file :
public function testTextarea()
{
$form = $this->factory->createNamed('textarea', 'na&me', 'foo&bar', array(
'property_path' => 'name',
'pattern' => 'foo',
));
$this->assertWidgetMatchesXpath($form->createView(), array(),
'/textarea
[@name="na&me"]
[not(@pattern)]
[.="foo&bar"]
'
);
}
Is it correct?
Commits
-------
36cebf0 Fix infinite loop on circullar reference in form factory
Discussion
----------
[BugFix][Form]Throw exception on form name circulal ref
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Closes: #2673
When FormType method `getName()` returns the same value as `getParent()` we're asking about trouble, and land into infinite loop.
Commits
-------
5b30812 See this issue : https://github.com/symfony/symfony/issues/2433
Discussion
----------
See this issue : https://github.com/symfony/symfony/issues/2433
I changed the access speficiers to `protected`, which makes easier to extend this class if one needs to like I did.
---------------------------------------------------------------------------
by greg0ire at 2011/11/10 06:55:12 -0800
Precision on the problem I had : I wanted to use a `CollectionType` and display a collection element attribute as the label for this element. I had no choice but to extend `ResizeFormListener` and `CollectionType`.
Commits
-------
57e1aeb Fixed undefined index notice in readProperty() method (PropertyPath)
Discussion
----------
Fixed undefined index notice in readProperty() method (PropertyPath)
Hi,
For some reasons, I get `notice` errors on `readProperty()` with Propel:
Notice: Undefined index: 0 in /Users/william/projects/Propel/testProjects/symfony2/vendor/symfony/src/Symfony/Component/Form/Util/PropertyPath.php line 284
The `PropelObjectCollection` implements `ArrayAccess`, the `readProperty()` method does not check if the given `index` exists so the `notice` error is thrown. I suppose to check whether the index exists or not has to be added.
Regards,
William
---------------------------------------------------------------------------
by fabpot at 2011/09/27 23:42:07 -0700
The patch is probably not what we want to do. First, I suppose that you are not creating the propertyPath by hand. If that is the case, we need to understand why the property path does not exist. Then, even if we might want to check the existence of the index, if it does not exist, we should probably throw an exception instead of just ignoring the problem.
---------------------------------------------------------------------------
by willdurand at 2011/09/28 01:14:49 -0700
My bad. This is a Propel bug due to `ArrayObject`. It throws a notice error if the index is not found in `offsetGet()` which is wrong according to the `ArrayAccess` interface. If the index is not found, we have to return `null`.
@fabpot Are you agree with that (for the `null` value) ?
---------------------------------------------------------------------------
by fabpot at 2011/09/28 01:17:09 -0700
My point is that it should never happen under normal circumstances.
---------------------------------------------------------------------------
by willdurand at 2011/09/28 01:23:55 -0700
@fabpot Not sure to get it.
The fact is that it tries to get the value (`getValue()`) of a fresh object, just added to the `collection` when I'm submitting a form with a `CollectionType` and a new entry in it.
I mean it tries to get this new object (not yet persisted, not yet in the collection) in the collection (`getValue()` -> `readProperty()`) which implements `ArrayAccess` but this object cannot be in the collection at this time.
Am I wrong ?
And, without this notice error thrown by Propel, I probably never opened this issue...
---------------------------------------------------------------------------
by willdurand at 2011/09/29 06:40:34 -0700
@fabpot: you can try this example: http://www.propelorm.org/cookbook/symfony2/mastering-symfony2-forms-with-propel.html#manytomany_relations in order to make your own tests. Will it be enough?
As I said, it throws a weird notice for the reasons above.
---------------------------------------------------------------------------
by jaugustin at 2011/10/04 12:58:10 -0700
any news on this ?
@fabpot did you have time to look at the test case ?
---------------------------------------------------------------------------
by cedriclombardot at 2011/11/09 14:29:42 -0800
@fabpot: can we have news about this ?
Commits
-------
47ebf08 Fix some bugs
fad825e Add DoctrineValidationPass to DoctrineBundle#buildContainer
a064acd Implement feature to add validations based on the Manager-Type (ORM, MongoDB, CouchDB)
Discussion
----------
[WIP] Validation on a Doctrine Manager Basis
Hello,
we have had problems before with validation that is "persistence" related. Unique-validators or any other validation that is based on services that depend on persistence.
The problem is two-fold:
1. In annotations you cannot define validators for all persistence layers you support, because then users need them all installed.
2. In XML/YAML the same is true, since there is only one validation.xml or validation.yml file looked for.
Now one solution is to have three model classes that extend from a base class to get around this (like FOSUserBundle does) but that is cumbersome. This PR provides a new solution that is Doctrine specific (and takes the responsibility out of the Core).
Each Doctrine Bundle (ORM, CouchDB, MongoDB, PHPCR) can add this compiler pass with a manager type name:
$container->addCompilerPass(new DoctrineValidationPass('orm'));
This leads to the compiler pass searching for additional validation files "Resources/config/validation.orm.yml" and "Resources/configvalidation.orm.xml".
My first idea was to put this into the Resources/config/doctrine folder as well, but then it is detected as mapping file of course.
Regarding tests, this is not easily testable without a full fledged bundle setup, i tested this inside Acme Demo Bundle, however for a good unit-test we probably need a filesystem abstraction testing layer. Has anyone a good idea how to test this without having to setup another test-bundle? I can't use the one from DoctrineBundle since this code is in the Bridge.
---------------------------------------------------------------------------
by fabpot at 2011/11/13 23:12:06 -0800
@beberlei: Is it still WIP?
---------------------------------------------------------------------------
by beberlei at 2011/11/15 10:47:49 -0800
@fabpot it is complete, but it has no tests, that was the WIP part. :-)
---------------------------------------------------------------------------
by mvrhov at 2011/11/15 23:56:11 -0800
I wanted to refactor how validation is managed today, so it could do one validation file per class, same as with Doctrine but @stof pointed me to this PR. I still find this a great idea as the validation is easier to find.
```php
foreach ($container->getParameter('kernel.bundles') as $bundle) {
$reflection = new \ReflectionClass($bundle);
$bundleDir = dirname($reflection->getFilename());
//check for per class validation files
if (is_dir($dir = $bundleDir . '/Resources/config/validation')) {
$finder = new Finder();
$finder
->name('*'.$extension)
->in($dir);
foreach ($finder as $file) {
$files[] = realpath($file);
$container->addResource(new FileResource($file));
}
}
//global validation file?
if (is_file($file = $bundleDir . '/Resources/config/validation'.$extension)) {
$files[] = realpath($file);
$container->addResource(new FileResource($file));
}
}
```
Commits
-------
fc4e628 [Config] added append to the node builder
Discussion
----------
[Config] added append to the node builder
Bug fix: no
Feature addition: yes
BC break: no
test pass: true
the problem is that i can only append a node if i have an array node, but if the current node is the node builder i can not append a node (and i think the node builder is like a array node).
---------------------------------------------------------------------------
by tecbot at 2011/11/16 23:23:16 -0800
@fabpot: any chance to merge this?
Commits
-------
fabe818 [EventDispatcher] Add reference to the EventDispatcher on the Event
Discussion
----------
[EventDispatcher] Add reference to the EventDispatcher on the Event
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
I don't like registering event listeners unless they are really used, it seems wasteful. So I tend to register listeners for the response event in other listeners, only when they will be required. @stof has [brought to my attention](fb243ace83 (commitcomment-696467)) that this may cause issues in Silex or any other situation where event listeners are not lazy loaded, since it creates a circular reference in that case.
With this PR, avoiding the circular reference is possible, without bloating the response listener with unnecessary "do I need to do anything?" code.
---------------------------------------------------------------------------
by schmittjoh at 2011/11/06 05:28:39 -0800
Did you do any benchmarks? It's just a feeling, but registering a listener at runtime might be more expensive than just having it always executed.
Also, I find these dynamic listeners a bit of a code smell. They are not easily testable, and the control flow is harder to track. Besides, you do not take into account subrequests which might happen in between.
---------------------------------------------------------------------------
by Seldaek at 2011/11/06 09:34:27 -0800
I don't see why it would be slower, if it's a commonly fired event yes you blast away the `sorted` listener cache every time you add one, but most of the time those optional listeners are for the response, which is typically not sorted yet when you add the listener, so I don't think there is any overhead.
As for the code smell, of course it's a matter of preference, but I have the opposite view on control flow, I find it weird that listeners are registered when they are not used in the end, while doing it my way I think it's more clear what happens.
For sub-requests, I'm not sure what you mean. In this instance, and in most response listeners I have seen, the sub-requests are always ignored anyway by the listener.
---------------------------------------------------------------------------
by drak at 2011/11/10 06:07:45 -0800
I don't see how loading up the dispatcher with a bunch of callables can be expensive - it's just loading an array basically.
Wouldn't it be better to have a separate `DispatcherAwareEvent extends Event`
class DispatcherAwareEvent extends Event
{
protected $dispatcher;
public function setDispatcher(EventDispatcher $dispatcher)
{
$this->dispatcher = $dispatcher;
}
public function getDispatcher()
{
return $this->dispatcher;
}
This can then be used as a base class for what you need `MyEvent extends DispatcherAwareEvent`
$event = new MyEvent($dispatcher, $foo);
$dispatcher->dispatch($event);
---------------------------------------------------------------------------
by Seldaek at 2011/11/10 06:18:57 -0800
@drak: Every event is part the event dispatching system and therefore should be aware of the dispatcher imo. It's not like the ContainerAwareInterface which is gluing things that do not especially have to know about the DIC together.
If we do that, then we have to start arguing every time we need the dispatcher in a given event, because the original author did not think it was necessary, and then that will only make it into the next minor version, etc. Not fun at all.
---------------------------------------------------------------------------
by drak at 2011/11/10 06:36:26 -0800
By the way, the event dispatchers looks to be pretty well optimized given the fact that it only sorts listeners if they are called, and then only once.
---------------------------------------------------------------------------
by drak at 2011/11/10 12:33:28 -0800
It just seems weird. I mean, following on, why isn't the event name a compulsory parameter also? - again, you can say both ways, if you need it, make it part of your custom Event class, or since it's a required param to be able to dispatch an event in the first place, make it part of the base Event class. All I'm saying it it seems suspicious when it could be achieved a different way.
For example, you could inject the dispatcher into the listener itself and then the event handler could access the dispatcher if it needs:
class MyListener
{
public function __construct(EventDispatcher $eventDispatcher)
{
//...
}
public function someListener(Event $event)
{
//...
}
}
---------------------------------------------------------------------------
by stof at 2011/11/10 15:20:07 -0800
@drak The issue when injecting the dispatcher in the listener is described in the issue: circular dependency: you need to create the dispatcher before the listener (as it is injected in it) and when the listener is not lazy-loaded (in Silex for instance), you need to create it before the dispatcher.
---------------------------------------------------------------------------
by drak at 2011/11/10 21:15:45 -0800
Indeed, although it might not unreasonable to expect to create the dispatcher first... but anyway I'm convinced!
Injecting the dispatcher could lead to some __very interesting possibilities__ as standard. While we are at it though, we should have a getter and setter for `$name` in the `Event` class and `$event->setName($eventname)` in the `dispatch()` method. Allowing an event to know it's name is very useful. It allows a single listener to be registered for multiple names, and even makes the event object reusable. I don't know why $name was removed, it was in the Symfony 1 dispatcher and while the new dispatcher is brilliant from an OO point of view, missing the name as standard is a big shame.
+1 from me.
Commits
-------
413756c [BC break][SecurityBundle] Changed the way to register factories
Discussion
----------
[BC break][SecurityBundle] Changed the way to register factories
As discussed in #2454, this changes the way to register the factories to let each bundles register the factories it provides.
Commits
-------
2adc36c [Security] renamed security option to erase_credentials
104b697 [Security] added configurable option security.erase_credentials_from_token
ede55d2 [Security] added configuration parameter for AuthorizationManagerProvider
Discussion
----------
[Security] added configuration parameter to AuthorizationManagerProvider
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: 2657
Todo: -
---------------------------------------------------------------------------
by fabpot at 2011/11/16 10:30:34 -0800
You should also add a configurable option under the `security` entry to ease the configurability.
Commits
-------
5e4b7cb Renamed Propel Bridge: Propel => Propel1
cdad7ab Introducing the Propel Bridge
Discussion
----------
Introducing the Propel Bridge
Basic bridge with stable components for Propel.
This should not affect anything except the need to maintain this code (even if the most part is safe thanks to `@api` tag). As Propel future is linked to Symfony2, to maintain this bridge should not be a problem.
Don't flame me for this proposal, I do that "knowingly".
Regards,
William.
---------------------------------------------------------------------------
by stof at 2011/09/15 12:22:12 -0700
IMO, it would be better to put this code in a repo owned by the Propel organization than in the core.
---------------------------------------------------------------------------
by Richtermeister at 2011/09/15 15:33:02 -0700
Yay, great to see this, very much looking forward to using Propel again.
---------------------------------------------------------------------------
by GromNaN at 2011/09/16 01:37:53 -0700
+1 for @stof proposition.The Silex Core should stay lightweight.
Silex definitely need a site or at least a page in the doc to list all the community extensions.
---------------------------------------------------------------------------
by odino at 2011/09/16 01:39:40 -0700
Silex? :)
---------------------------------------------------------------------------
by GromNaN at 2011/09/16 01:43:20 -0700
Oups, I was looking at Silex PR and got this PR.
---------------------------------------------------------------------------
by lsmith77 at 2011/10/08 01:01:56 -0700
@willdurand we should maybe make this a topic for the next IRC meeting.
---------------------------------------------------------------------------
by willdurand at 2011/10/09 10:26:22 -0700
Agreed :)
---------------------------------------------------------------------------
by willdurand at 2011/11/03 14:18:02 -0700
Good to go ?
---------------------------------------------------------------------------
by willdurand at 2011/11/04 03:34:37 -0700
Just removed the `Query` class. /cc @Stof
Anything else?
---------------------------------------------------------------------------
by willdurand at 2011/11/05 08:45:34 -0700
@fabpot : good to merge ?
The PropelBundle has a `bridge` branch. I'm ready, I'm just waiting you merge this PR to tag the PropelBundle for Symfony 2.0, and after that I'll merge the `bridge` branch into the `master`.
---------------------------------------------------------------------------
by fabpot at 2011/11/16 22:32:42 -0800
What about renaming the directory from `Propel` to `Propel1`? That way, we will be able to have a `Propel` bridge for Propel 2.0.
---------------------------------------------------------------------------
by willdurand at 2011/11/16 23:18:56 -0800
It won't be consistent with the PropelBundle but I guess we don't have any other choice.. So I'm +1 for that.
If it's ok, I'll update this PR, just tell me.
---------------------------------------------------------------------------
by fabpot at 2011/11/17 07:09:58 -0800
yes, +1 for renaming
---------------------------------------------------------------------------
by lsmith77 at 2011/11/17 07:11:45 -0800
Why rename it to ``Propel1``? I think its enough to eventually add a ``Propel2``.
---------------------------------------------------------------------------
by willdurand at 2011/11/17 07:14:05 -0800
`Propel1` is for BC.
`Propel` will be the Propel's future :)
---------------------------------------------------------------------------
by lsmith77 at 2011/11/17 07:17:02 -0800
sounds like a bad idea .. and what happens when you come out with ``Propel3`` ?
---------------------------------------------------------------------------
by stof at 2011/11/17 07:17:31 -0800
@willdurand Maybe the bundle should renamed the same way, for consistency and to let ``PropelBundle`` for the Propel 2 one ? (but this should probably be discussed in another issue tracker)
---------------------------------------------------------------------------
by willdurand at 2011/11/17 07:30:21 -0800
That way we'll be able to handle both Propel 1 & 2 without BC break. You may want to upgrade Symfony2 but not Propel nor PropelBundle. Propel1 bridge has a limited lifetime.
@stof : the PropelBundle will be tagged and a branch will probably appear for Propel1 compatibility.
---------------------------------------------------------------------------
by stof at 2011/11/17 07:34:10 -0800
@willdurand if Symfony provides a Propel bridge using the same namespace for Propel2 and then Propel3, this means that the Sf2 update changing the bridge to use the Propel3 code will make Sf2 incompatible with Propel2 even if you have a tag for Propel2 in the PropelBundle (as you will need to downgrade Symfony to the older tag too). As long as bridges are in the main Symfony repo, they are upgraded the same time Symfony is upgraded and they can bump the requirements.
---------------------------------------------------------------------------
by willdurand at 2011/11/17 07:37:13 -0800
Yes but Propel 1 is frozen, almost dead as we won't add any new features.
Propel2 is the future and there is no plan for a Propel3 which will break BC.
---------------------------------------------------------------------------
by willdurand at 2011/11/17 07:57:05 -0800
Updated!
Commits
-------
c89d45b Fix cs
Discussion
----------
Fix cs
---------------------------------------------------------------------------
by rouffj at 2011/11/17 23:30:29 -0800
lsmith77: ok, so i revert my change only on RequestMatcher file ?
Commits
-------
6d7e6a8 [DoctrineBundle] Enhance error reporting during mapping validation when nested exceptions occur.
Discussion
----------
Doctrine info mapping exception
Better error handling when nested exceptions occur (which is commonly possible with reflection errors).
---------------------------------------------------------------------------
by fabpot at 2011/11/16 02:10:13 -0800
It should probably be done on the 2.0 branch, no?
---------------------------------------------------------------------------
by beberlei at 2011/11/18 02:11:06 -0800
Its not necessarily a bugfix but an enhancement.
Commits
-------
d2195cc Fixed phpdoc and updated the changelog
9e41ff4 [SecurityBundle] Added a validation rule
b107a3f [SecurityBundle] Refactored the configuration
633f0e9 [DoctrineBundle] Moved the entity provider service to DoctrineBundle
74732dc [SecurityBundle] Added a way to extend the providers section of the config
Discussion
----------
[WIP][SecurityBundle] Added a way to extend the providers section of the config
Bug fix: no
Feature addition: yes
BC break: <del>no (for now)</del> yes
Tests pass: yes
This adds a way to extend the ``providers`` section of the security config so that other bundles can hook their stuff into it. An example is available in DoctrineBundle which is now responsible to handle the entity provider (<del>needs some cleanup as the service definition is still in SecurityBundle currently</del>). This will allow PropelBundle to provide a ``propel:`` provider for instance.
In order to keep BC with the existing configuration for the in-memory and the chain providers, I had to allow using a prototyped node instead of forcing using an array node with childrens. This introduces some issues:
- impossible to validate easily that a provider uses only one setup as prototyped node always have a default value (the empty array)
- the ``getFixableKey`` method is needed in the interface to support the XML format by pluralizing the name.
Here is my non-BC proposal for the configuration to clean this:
```yaml
security:
providers:
first:
memory: # BC break here by adding a level before the users
users:
joe: { password: foobar, roles: ROLE_USER }
john: { password: foobarbaz, roles: ROLE_USER }
second:
entity: # this one is BC
class: Acme\DemoBundle\Entity\User
third:
id: my_custom_provider # also BC
fourth:
chain: # BC break by adding a level before the providers
providers: [first, second, third]
```
What do you think about it ? Do we need to keep the BC in the config of the bundle or no ?
Btw note that the way to register the factories used by the firewall section should be refactored using the new way to provide extension points in the extensions (as done here) instead of relying on the end user to register factories, which would probably mean a BC break anyway.
---------------------------------------------------------------------------
by lsmith77 at 2011/10/23 09:19:23 -0700
i don't think we should keep BC. the security config is complex as is .. having BC stuff in there will just make it even harder and confusing.
---------------------------------------------------------------------------
by willdurand at 2011/10/23 09:41:25 -0700
Is the security component tagged with `@api` ?
So basically, we just have to create a factory (`ModelFactory` for instance) and to register it in the `security` extension, right ? Seems quite simple to extend and much better than the hardcoded version…
Why did you call the method to pluralize a key `getFixableKey` ?
---------------------------------------------------------------------------
by beberlei at 2011/10/23 14:48:26 -0700
Changing security config will introduce risk for users. We should avoid that
---------------------------------------------------------------------------
by stof at 2011/10/23 15:34:47 -0700
@beberlei as the config is validated, it will simply give them an exception during the loading of the config if they don't update their config.
---------------------------------------------------------------------------
by stof at 2011/10/24 01:01:42 -0700
@schmittjoh @fabpot Could you give your mind about it ?
---------------------------------------------------------------------------
by stof at 2011/10/31 17:08:12 -0700
@fabpot @schmittjoh ping
---------------------------------------------------------------------------
by stof at 2011/11/11 14:08:18 -0800
I updated the PR by implementing my proposal as the latest IRC meeting agreed that we don't need to keep the BC for this change. This allows to add the validation rule now.
---------------------------------------------------------------------------
by stof at 2011/11/16 11:16:06 -0800
@fabpot ping
---------------------------------------------------------------------------
by fabpot at 2011/11/16 22:29:05 -0800
@stof: Before merging, you must also add information about how to upgrade in the CHANGELOG-2.1.md file.
---------------------------------------------------------------------------
by stof at 2011/11/17 00:01:23 -0800
@fabpot done
The Firewall is now executed after the Router. This was needed to have access
to the locale and other request attributes that are set by the Router. This
change implies that all Firewall specific URLs have proper (empty) routes like
`/login_check` and `/logout`.
Commits
-------
f9a65ba Redirect to default_target_path if use_referer is true and the referer is the login_path.
Discussion
----------
Login redirect
Bug fix: no
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes
Redirect to default_target_path if use_referer is true and the referer is the login_path.
---------------------------------------------------------------------------
by Seldaek at 2011/10/30 10:52:38 -0700
👍
---------------------------------------------------------------------------
by stealth35 at 2011/10/30 11:04:16 -0700
@snc BC break ?
---------------------------------------------------------------------------
by snc at 2011/10/30 12:11:39 -0700
Well I'm sure it is never intended by a developer to be redirected to the login page after logging in but it could be possible that the controller which displays the login form handles this case, so my change would break it.
Commits
-------
79ae3fc [Form] fixed radio and checkbox when data is not bool
Discussion
----------
[Form] fixed checkbox view
The checkbox view was being built based on app data, not client data. This fixes it.
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
---------------------------------------------------------------------------
by fabpot at 2011/11/16 13:31:09 -0800
`RadioType` suffers from the same problem, no?
---------------------------------------------------------------------------
by kriswallsmith at 2011/11/16 13:32:50 -0800
Yeah, I'll fix that too.
---------------------------------------------------------------------------
by kriswallsmith at 2011/11/16 13:43:29 -0800
Updated to include `RadioType`.
Commits
-------
8399574 Fixes a small php doc issue of Symfony\Component\Console\Command\Command::setDefinition()
Discussion
----------
Fixes a small php doc issue of Symfony\Component\Console\Command\Command::setDefinition()
Have setDefinition() accept InputDefinition instead of Definition.
Commits
-------
f83ef1e [DoctrineBundle] Fix tests - incorrect class names (copy paste error most probably)
Discussion
----------
[DoctrineBundle] Fix tests - incorrect class names
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Some classes are incorrectly named resulting in failed tests for DoctrineBundle, looks like a copy paste error.
Commits
-------
470f872 [FrameworkBundle] fixed formatting of farsi translation file
Discussion
----------
[FrameworkBundle] fixed formatting of farsi translation file
It now matches the other languages translation files.
* 2.0:
[HttpKernel] fixed Content-Length header when using ESI tags (closes#2623)
[HttpFoundation] added an exception to MimeTypeGuesser::guess() when no guesser are available (closes#2636)
[Security] fixed HttpUtils::checkRequestPath() to not catch all exceptions (closes#2637)
[DoctrineBundle] added missing default parameters, needed to setup and use DBAL without ORM
[Transation] Fix grammar.
[TwigBundle] Fix trace to not show 'in at line' when file/line are empty.
Commits
-------
4858fbe [TwigBundle] Fix trace to not show 'in at line' when file/line are empty.
Discussion
----------
[TwigBundle] Fix trace to not show 'in at line' when file/line are empty.
Occasionally I saw call stacks where file/line are empty in the raw exception object, but the trace.html.twig file was still showing 'in at line' with empty values. I believe this fixes that.
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
---------------------------------------------------------------------------
by fabpot at 2011/11/09 22:49:35 -0800
The current code looks correct to me. Can you try with the latest version of Twig to be sure that you don't have this issue because of a bug in Twig that has been fixed since then?
---------------------------------------------------------------------------
by dpb587 at 2011/11/10 07:20:34 -0800
Thanks for the feedback. Should I change my patch to check that both are defined and non-empty, such as `trace.file is defined and trace.file and trace.line is defined and trace.line`?
I think the issue is that I'm seeing the file and line keys are defined but empty. I created another branch with a pseudo-test case that shows a little more information. Using symfony-standard with symfony in deps as follows and symfony/twig removed from deps.lock.
[symfony]
git=git://github.com/dpb587/symfony.git
version=origin/patch-trace-debug
Then running the following:
phpunit -c app/ vendor/symfony/tests/Symfony/Tests/Bundle/TwigBundle/Controller/ExceptionController.php
The test (is backwards) and passes, dumping the following (this call happens right after a call_user_func):
Array
(
[namespace] => Symfony\Bundle\FrameworkBundle\EventListener
[short_class] => RouterListener
[class] => Symfony\Bundle\FrameworkBundle\EventListener\RouterListener
[type] => ->
[function] => onKernelRequest
[file] =>
[line] =>
[args] => Array
(
[0] => Array
(
[0] => object
[1] => Symfony\Component\HttpKernel\Event\GetResponseEvent
)
)
)
I saw the same results with two php versions:
PHP 5.3.3-7+squeeze3 with Suhosin-Patch (cli) (built: Jun 28 2011 08:24:40)
Copyright (c) 1997-2009 The PHP Group
Zend Engine v2.3.0, Copyright (c) 1998-2010 Zend Technologies
with Xdebug v2.1.1, Copyright (c) 2002-2011, by Derick Rethans
with Suhosin v0.9.32.1, Copyright (c) 2007-2010, by SektionEins GmbH
PHP 5.3.8 (cli) (built: Nov 4 2011 05:43:22)
Copyright (c) 1997-2011 The PHP Group
Zend Engine v2.3.0, Copyright (c) 1998-2011 Zend Technologies
with the ionCube PHP Loader v4.0.10, Copyright (c) 2002-2011, by ionCube Ltd., and
with Zend Guard Loader v3.3, Copyright (c) 1998-2010, by Zend Technologies
Sorry if I'm simply missing something or doing something silly.
---------------------------------------------------------------------------
by fabpot at 2011/11/11 13:12:21 -0800
`trace.file is defined and trace.file and trace.line is defined and trace.line` looks good to me.
* 2.0:
[Form] fixed previous merge
[Form] simplified previous merge
Also identify FirePHP by the X-FirePHP-Version header
[TwigBundle] Extract output buffer cleaning to method
[TwigBundle] Do not clean output buffering below initial level
Fixed rendering of FileType (value is not a valid attribute for input[type=file])
Added tests for string fix in DateTimeToArrayTransformer (8351a11286).
Added check for array fields to be integers in reverseTransform method. This prevents checkdate from getting strings as arguments and throwing incorrect ErrorException when submitting form with malformed (string) data in, for example, Date field. #2609
[Translation] removed unneeded methods
[Translation] added detection for circular references when adding a fallback catalogue
[DomCrawler] trim URI in getURI
[Yaml][Tests] Fixed missing locale string for Windows platforms which caused test to fail
The configuration is now cleaner by avoiding using prototyped nodes
as additional keys. This is a BC break for existing providers.
- MemoryProvider:
security:
providers:
my_provider:
memory: # this level has been added
users:
# ...
- ChainProvider:
security:
providers:
my_provider:
chain: # This level has been added
providers:
# ...
Commits
-------
29e12af [TwigBundle] Extract output buffer cleaning to method
ed1a6c2 [TwigBundle] Do not clean output buffering below initial level
Discussion
----------
[TwigBundle] Do not clean output buffering below initial level
This resulted in issues with PHPUnit 3.6, which will buffer all output and clean them in the end. Since
we cleaned their buffer, the subsequent clean would raise a warning. This is documented in [issue 390](https://github.com/sebastianbergmann/phpunit/issues/390) of
the PHPUnit tracker.
Closes#2531.
This also affects FOSRestBundle's ExceptionController /cc @lsmith.
---------------------------------------------------------------------------
by fabpot at 2011/11/11 07:33:24 -0800
I have a similar fix locally but I have not merged it yet as it looks a bit dirty (but I've not a better idea yet). Anyway, your PR is better than mine as you've added some unit tests already.
Commits
-------
e83e00a Fixed rendering of FileType (value is not a valid attribute for input[type=file])
Discussion
----------
Fixed rendering of FileType
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
According to the W3C validator, `value` is not a valid attribute for `input[type=file]`.
---------------------------------------------------------------------------
by fabpot at 2011/11/10 23:01:24 -0800
Instead of creating yet another block, what about modifying the `field_widget` to not render the `value` attribute if the value is empty? Also, the PHP template must be fixed too.
---------------------------------------------------------------------------
by jalliot at 2011/11/11 02:02:52 -0800
@fabpot Changed ;)
Commits
-------
2582fcb Added tests for string fix in DateTimeToArrayTransformer (8351a11286).
8351a11 Added check for array fields to be integers in reverseTransform method. This prevents checkdate from getting strings as arguments and throwing incorrect ErrorException when submitting form with malformed (string) data in, for example, Date field. #2609
Discussion
----------
Fix for #2609
Second take for fix for #2609, hope it's ok now. Tests are failing without my fix and passing with it.
This resulted in issues with PHPUnit 3.6, which will buffer all output and clean them in the end. Since
we cleaned their buffer, the subsequent clean would raise a warning. This is documented in issue 390 of
the PHPUnit tracker.
Closes#2531.
Commits
-------
0fe8ac6 [EventDispatcher] Removed unused argument.
Discussion
----------
[EventDispatcher] Removed unused parameter in doDispatch().
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Removed unused parameter.
---------------------------------------------------------------------------
by Seldaek at 2011/11/10 02:30:01 -0800
A simple $foo($arg) is faster than call_user_func, fine, but with all the if/else logic here are you sure it is still faster?
---------------------------------------------------------------------------
by drak at 2011/11/10 06:44:41 -0800
call_user_func is inordinately slow, I'll make some benchmarks. The if logic here is pretty, two conditionals for array callables or one conditional for closures/functions.
---------------------------------------------------------------------------
by drak at 2011/11/10 12:45:27 -0800
After doing some benchmarks, there are savings to be made in all cases except when the listener is an instantiated object when it's marginally slower, so I've reverted these changes and made this PR just to remove the unused argument in `doDispatch()`. You can see the benchmark code here: https://gist.github.com/1356129.
---------------------------------------------------------------------------
by drak at 2011/11/10 12:46:55 -0800
This PR is ready for consideration.
Commits
-------
4d80ebd Remove security token if user was deleted, is disabled or locked to prevent infinite redirect loops to the login path (fixes#1798).
Discussion
----------
[2.1] Fix for issue 1798
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #1798
This is a simplified PR of #2528 for the master branch.
* 2.0:
Added a class to the logs ol element to prevent hiding it when toggling an exception (fixes#2589).
Remove only the security token instead of the session cookie.
Clear session cookie if user was deleted, is disabled or locked to prevent infinite redirect loops to the login path (fixes#1798).
Commits
-------
f9befb6 Remove only the security token instead of the session cookie.
348bccb Clear session cookie if user was deleted, is disabled or locked to prevent infinite redirect loops to the login path (fixes#1798).
Discussion
----------
Fix for issue 1798
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Clear session cookie if user was deleted, is disabled or locked to prevent infinite redirect loops to the login path (fixes#1798).
---------------------------------------------------------------------------
by snc at 2011/11/01 04:01:49 -0700
@stof I have changed the code so that it only removes the token... do we still need any hook support?
---------------------------------------------------------------------------
by stof at 2011/11/01 04:07:17 -0700
well, the hook is for your own use case but it would be for 2.1 only anyway, not for 2.0
---------------------------------------------------------------------------
by snc at 2011/11/07 15:11:52 -0800
Now that #2414 is merged to 2.1, this could be simplified for the master branch...
Commits
-------
269a5e6 Added the ablity to get a requests ContentType
Discussion
----------
Added getContentType
I've added the ability for Symfony\Component\HttpFoundation\Request to return the ContentType from serverBag this uses the $formats array to determine if the requested ContentType is valid.
---------------------------------------------------------------------------
by ericclemmons at 2011/11/03 20:00:51 -0700
Have you considered squashing a couple of your commits? They seem doubled up.
Trivial, I know, but it will make each commit stand on its own (instead of appearing as a typo correction)
---------------------------------------------------------------------------
by thomasbibb at 2011/11/04 02:02:36 -0700
done.
---------------------------------------------------------------------------
by ericclemmons at 2011/11/04 07:25:20 -0700
You may need to do a `git push -f origin master`. Check the commits tab to see the duplicate history:
> https://github.com/symfony/symfony/pull/2559/commits
Wheeeee, rebasing is fun!
---------------------------------------------------------------------------
by thomasbibb at 2011/11/04 12:26:06 -0700
There we got thats better :)
---------------------------------------------------------------------------
by ericclemmons at 2011/11/04 12:55:07 -0700
👍 Now let's see if it gets approved by @fabpot :)
---------------------------------------------------------------------------
by thomasbibb at 2011/11/06 03:39:12 -0800
I've removed the space between the method name and the parenthesis.
---------------------------------------------------------------------------
by thomasbibb at 2011/11/06 04:05:15 -0800
done.
---------------------------------------------------------------------------
by fabpot at 2011/11/06 23:44:22 -0800
Can you added some unit tests?
Commits
-------
d974a4a Merge pull request #4 from stealth35/test_mo_loader
cf05646 delete useless tests
19f9de9 [Translation] fix gettext tests
965f2bf Merge pull request #3 from stealth35/test_mo_loader
9c2a26d [Translation] add Mo loader tests
9af2342 [Translation] Added the gettext loaders
Discussion
----------
[Translation] Added the gettext loaders
This is the squashed version of the work done by @xaav in #634.
@stealth35 you said you will work on the dumpers. do you have some stuff on it ?
---------------------------------------------------------------------------
by drak at 2011/10/24 19:28:43 -0700
Is there any more progress with this?
---------------------------------------------------------------------------
by stealth35 at 2011/10/25 00:57:19 -0700
I work on the dumpers, but the Po loader is wrong, caus' the Po ressource can be multiline,
msgid ""
"Here is an example of how one might continue a very long string\n"
"for the common case the string represents multi-line output.\n"
http://www.gnu.org/software/gettext/manual/gettext.html#PO-Files
Anyway the Po format is an intermediate format to Mo file, (like .txt to .res file for ICU), IMO we can just support the real gettext format : Mo
---------------------------------------------------------------------------
by stealth35 at 2011/11/03 02:00:24 -0700
@stof The MO Dumper is ready (stealth35/symfony@f2d1d5b4de), should we keep the PO format ?
---------------------------------------------------------------------------
by fabpot at 2011/11/07 08:50:59 -0800
@stealth35: The PO is what people will use for their translations. They will then dump it to MO. So, we need both PO and MO loaders and dumpers.
---------------------------------------------------------------------------
by stealth35 at 2011/11/08 01:25:39 -0800
@fabpot, I'm ready for both dumpers, you can merge this, and I'll open a PR for the dumpers
---------------------------------------------------------------------------
by fabpot at 2011/11/08 22:37:47 -0800
I've just had a look at this PR code again and I see that the unit tests are pretty slim. Is it possible to add some tests for the mo loader?
---------------------------------------------------------------------------
by stealth35 at 2011/11/09 01:15:25 -0800
@fabpot test send to @stof ✌️
---------------------------------------------------------------------------
by stof at 2011/11/09 02:22:55 -0800
and merged in this branch
---------------------------------------------------------------------------
by fabpot at 2011/11/09 02:39:09 -0800
The tests do not pass for me:
There was 1 error:
1) Symfony\Tests\Component\Translation\Loader\MoFileLoaderTest::testLoadDoesNothingIfEmpty
InvalidArgumentException: MO stream content has an invalid format.
/Users/fabien/work/symfony/git/symfony/src/Symfony/Component/Translation/Loader/MoFileLoader.php:79
/Users/fabien/work/symfony/git/symfony/src/Symfony/Component/Translation/Loader/MoFileLoader.php:46
/Users/fabien/work/symfony/git/symfony/tests/Symfony/Tests/Component/Translation/Loader/MoFileLoaderTest.php:34
--
There was 1 failure:
1) Symfony\Tests\Component\Translation\Loader\PoFileLoaderTest::testLoad
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
Array (
- 'foo' => 'bar'
)
/Users/fabien/work/symfony/git/symfony/tests/Symfony/Tests/Component/Translation/Loader/PoFileLoaderTest.php:25
Commits
-------
d08ec5e Add DelegatingValidator tests
e1822e7 Enable dynamic set of validation groups by a callback or Closure
Discussion
----------
[Form][Validator] Enable dynamic set of validation groups based on callback
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Symfony2 tests written for new feature: yes
closes tickets: #2498#1151
This will allow developer to pass a Closure or a callback array as a Validation groups option of a form. Eg:
```
class ClientType extends AbstarctType
{
// ...
public function getDefaultOptions(array $options)
{
return array(
'validation_groups' => function(FormInterface $form){
// return array of validation groups based on submitted user data (data is after transform)
$data = $form->getData();
if($data->getType() == Entity\Client::TYPE_PERSON)
return array('Default', 'person');
else
return array('Default', 'company');
},
);
}
// ...
}
```
```
class ClientType extends AbstarctType
{
// ...
public function getDefaultOptions(array $options)
{
return array(
'validation_groups' => array(
'Acme\\AcmeBundle\\Entity\\Client',
'determineValidationGroups'
),
);
}
// ...
}
```
This will make developers life easier !
---------------------------------------------------------------------------
by schmittjoh at 2011/10/27 06:39:56 -0700
Does that work if your ClientType were added to another form type?
e.g.
```php
<?php
class MyComplexType extends AbstractType
{
public function buildForm(FormBuilder $builder, array $options)
{
$builder->add('client', new ClientType());
}
// ...
}
```
---------------------------------------------------------------------------
by canni at 2011/10/27 06:44:33 -0700
This is doing nothing more than injecting array of validation groups, should work, but I have not tested this use case.
---------------------------------------------------------------------------
by canni at 2011/10/28 01:58:26 -0700
PHPUnit output
```
OK, but incomplete or skipped tests!
Tests: 5011, Assertions: 12356, Incomplete: 36, Skipped: 32.
```
---------------------------------------------------------------------------
by canni at 2011/11/02 11:37:47 -0700
Now functionality is complete, test are written, and implementation is clean. :)
---------------------------------------------------------------------------
by stloyd at 2011/11/02 11:50:44 -0700
Can tou `squash` your commits ? Thanks.
---------------------------------------------------------------------------
by canni at 2011/11/02 11:58:41 -0700
Done
---------------------------------------------------------------------------
by fabpot at 2011/11/07 07:51:18 -0800
Can you add some tests for the `DelegatingValidator` class, which is where we can ensure that the new feature actually works as expected?
---------------------------------------------------------------------------
by canni at 2011/11/07 13:53:16 -0800
OK, I've written proof-of-concept tests, also I've squashed few commits to make things clear.
Personally I think this should go straight into 2.0 series, as it do not beak BC, and a feature is really nice to use.
---------------------------------------------------------------------------
by stof at 2011/11/07 14:17:15 -0800
@canni the 2.0 branch is for bug fixes, not for new features. This is the difference between maintenance releases and minor releases.
Commits
-------
57040a8 document exceptions on the interfaces, inheritDoc on implementations, use statements instead of fully qualified exception names
Discussion
----------
Fix routing exception doc
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
This is only documentation cleanup for the url matcher and generator interfaces as they did not properly reflect the exceptions that are expected to be thrown. when the exceptions are not respected, unexpected behaviour can happen.
---------------------------------------------------------------------------
by dbu at 2011/11/07 00:49:08 -0800
ok, changed that. thought in an interface definition we would not want to "use" exceptions just for doc, but its changed now.
i rebased on the upstream master and got this additional commit about portuguese translations, no idea why this happened. and my local git diff does not show it either. is it creating problems in the pull request or can you still merge?
---------------------------------------------------------------------------
by fabpot at 2011/11/07 01:11:31 -0800
@dbu: Can you squash your commits and remove a commit that is not related to this PR? Thanks a lot.
---------------------------------------------------------------------------
by dbu at 2011/11/07 23:49:37 -0800
ok, done. thanks to you, i learn a lot about git :-)
for the record: i did git rebase --soft on the commit id before my first commit, then git reset HEAD and git checkout on all the files not related to my changes, then commit again with the consolidated message, then git push --force origin fix-routing-exception-doc
Commits
-------
7346896 Changed Serialized#supportsNormalization to PRIVATE
e851efc Updated SerializerTest with "normalizeTraversable" & "testNormalizeGivesPriorityToInterfaceOverTraversable"
d789f94 Serializer#normalize gives precedence to objects that support normalization
9e6ba9a Added protected Serializer#supportsNormalization
Discussion
----------
[Serializer] `normalize` should use supported normalizer before Traversable
Bug fix: yes
Feature addition: no
Backwards compatibility break: no (discussion needed)
Symfony2 tests pass: yes
Fixes the following tickets: #2295
**Same as PR #2539, except rebased onto `2.0`**
Should I abstract out a `supportsDenormalization` function just for symmetry?
Commits
-------
ab9caa0 [Security] Check for request's session before attempting writes.
dabff0e [Security] Support removing tokens from a session.
Discussion
----------
[Security] Support removing tokens from a session.
Currently there is no way to remove a session's security token without invalidating the entire session and all its data (the ContextListener will only update the session if a token is non-null and non-anonymous). This patch fixes that.
I consider this a bug and I found no tests to prove otherwise. Let me know if I'm mistaken. Originally mentioned at https://groups.google.com/d/topic/symfony-devs/ojLvh0WUbfo/discussion
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
---------------------------------------------------------------------------
by ms937 at 2011/10/24 05:19:21 -0700
This change looks good to me. In fact I'm using similar patch in my app and it works as intended. Also, several other people requested this on the mailing list. Could someone from Symfony team merge this? Thanks.
This will enable developer, to set a callback or a closure as a `'validation_groups'` form option,
this is usefull when we have to determine validation groups based on a client submitted data.
Commits
-------
78883f9 Allow syntax like ``{% render "AcmeDemoBundle:Frontend/Default:index" %}``
Discussion
----------
Allow syntax like ``{% render "AcmeDemoBundle:Frontend/Default:index" %}`
Allow syntax like ``{% render "AcmeDemoBundle:Frontend/Default:index" %}``
Bug fix: yes
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #2424
---------------------------------------------------------------------------
by stof at 2011/10/18 07:44:48 -0700
@docteurklein still the same issue. github says it conflicts. Are you sure you fetched the latest version ?
Thus it should be sent to 2.0 IMO as it is a bugfix
---------------------------------------------------------------------------
by docteurklein at 2011/10/18 07:51:21 -0700
@stof Yes, i'm pretty sure I followed the patches sending flow. (http://symfony.com/doc/2.0/contributing/code/patches.html
My tools are telling me it's ok.
I then merged it into master without any problem, which is up to date with upstream.
Sorry for the inconvenience.
I'll try to send it to 2.0 branch.
---------------------------------------------------------------------------
by docteurklein at 2011/10/18 07:53:52 -0700
@stof, what's wrong with https://github.com/docteurklein/symfony/commits/ticket_2424 ?
---------------------------------------------------------------------------
by stof at 2011/10/18 08:28:21 -0700
hmm, seems like github has an issue when determining if it conflicts or not. It's sad
---------------------------------------------------------------------------
by henrikbjorn at 2011/10/20 09:49:56 -0700
Dosent this already work ? as classes are namespaces the / should be a \ i think ?
Works for routes at least.
Commits
-------
09e1e60 Grammar changes.
b4d0f4b Grammar changes
a0e62f0 Fix typos.
e6627fb Docblocks.
Discussion
----------
[HttpFoundation] Docblocks.
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: n/a
Fixes the following tickets: -
Added missing docblocks and type hints. Improves use with IDE and auto-generated API docs.
Commits
-------
ffa537c replace occurences of "an UserInteface" with "a UserInterface"
Discussion
----------
replace occurences of "an UserInteface" with "a UserInterface"
Commits
-------
fbd2a0e make suggested changes for default value
c507b1d update variable name to match the option name
b53f000 add the ability to set the form prototype name in CollectionType. this will aid in handling nested collections in forms
Discussion
----------
[Form] Collection
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: [#1324](https://github.com/symfony/symfony/issues/1324)
add the ability to set the form prototype name in CollectionType. this will aid in handling nested collections in forms
---------------------------------------------------------------------------
by IamPersistent at 2011/10/17 02:54:45 -0700
Actually, as an afterthought, I looked over at the issues and this is basically https://github.com/symfony/symfony/issues/1324 just adding an extra option instead of using the prototype option.
@stloyd, my thought was handling the case where someone was "clever" enough to set the 'prototype_name' option to "". But I could be over-thinking :)
---------------------------------------------------------------------------
by stloyd at 2011/10/17 03:00:23 -0700
@IamPersistent IMO if someone is setting this option, he should be aware of that problem, also AFAIK `$$$$` could be *valid* name too ;-)
---------------------------------------------------------------------------
by IamPersistent at 2011/10/17 03:02:14 -0700
@stloyd, I'm fine with changing it, I'll wait to see what everyone else has to say about this request vs using the prototype option for the name in 1324
---------------------------------------------------------------------------
by IamPersistent at 2011/10/17 03:28:49 -0700
@stloyd, @stof, I made the suggested changes, now I suppose, let the debate begin over PR1324 vs this
---------------------------------------------------------------------------
by stloyd at 2011/10/17 03:47:53 -0700
IMO this PR makes changing prototype name in more clean way, so I would prefer this one over that proposed in #1324.
Commits
-------
89cd64a Set error code when number cannot be parsed. Fixes#2389
Discussion
----------
Set error code when number cannot be parsed in StubNumberFormatter
The stub implementation of NumberFormatter never sets an error code and instead always returns the "no-error" code. This causes unexpected results when a transformer gives it bad arguments, such as transforming the input value to boolean false and causing exceptions further down the line, as in #2389.
Instead, it should set an error code when appropriate and return it when requested so that a TransformationFailedException can be raised and the input value left unaltered.
There may be other instances where an error should be set. This covers the common use case of non-numeric input.
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #2389
Commits
-------
c9d05d7 Let NumberFormatter handle integer type casting
Discussion
----------
Let NumberFormatter handle integer type casting
The integer to localised string transformer is currently casting everything it gets to an integer, even if it is not a number. This responsibility should be passed off to NumberFormatter.
Partially addresses #2389 by not mistakenly typecasting a boolean false into an integer 0
---------------------------------------------------------------------------
by mrtorrent at 2011/10/30 15:04:28 -0700
Apologies, forgot the template:
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #2389 (partial)
Commits
-------
05a4e9d [Validators][Type] Added support for ctype_* functions + tests
Discussion
----------
[Validators][Type] Added support for ctype_* functions + tests
Bug fix: no
Feature addition: yes
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: -
Not sure if the ctype_* functions support should be included in the AssertTypeValidator or in a dedicated one.
What do you think ?
Commits
-------
cbd0c3c [Config] Implemented Serializable on resources
Discussion
----------
[Config] Implemented Serializable on resources
bug fix: no
feature addition: no
BC break: not really. you simply need to clear your cache in debug mode
test passes: yes
Serializing private variables without implementing Serializable uses far more space as it prepend the fully qualified class name before the name of each property. This change allows keeping the meta file smaller.
Commits
-------
6cb7acf CS - camelCase & curly braces
d9b7abb Added EntityChoiceList test for `group_by` and invalid, deep property paths
e6554d6 Removed Closure support from group_by (PropertyPath strings only)
037933a CS - (String) renamed to (string)
7ad0f05 Added group_by test for EntityType
882482a Added group_by tests for EntityChoiceList
040e988 `EntityChoiceList` now supports grouping of entities by property path or closure
b171a6a Added `group_by` to EntityType
Discussion
----------
[Doctrine] [Form] EntityType+EntityChoiceList supports grouping choices
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #1735
Per the discussion in #1735, `EntityType` does not immediately support grouping options, though I updated support for it in `EntityChoiceList` in fb9d951b1d.
This PR accomplishes the following:
* Adds optional `group_by` property to `EntityType` that supports either a `PropertyPath` or a `\Closure` that is evaluated on the entity choices
* Support for groups is added via the constructor in `EntityChoiceList`
* Groups are created prior to `EntityChoiceList#loadEntities` via a new `groupEntities` function
* Added tests for `EntityChoiceList`
* Added test for `EntityType` `group_by` support
*There is an alternative version that only modifies `EntityType`, but that requires the addition of `EntityType#buildView(...)`, which is messy, IMO: https://github.com/ericclemmons/symfony/compare/master...1735-entity_type_group_by*
---------------------------------------------------------------------------
by fabpot at 2011/10/25 01:48:23 -0700
ping @beberlei
---------------------------------------------------------------------------
by beberlei at 2011/10/25 03:06:05 -0700
I didnt run the tests, but generally this looks very good and is a good extension.
---------------------------------------------------------------------------
by beberlei at 2011/11/01 06:25:09 -0700
@fabpot i revewied this and it looks very good, tests all pass, i think this is a very nice addition.
Commits
-------
149bdb9 Enhance error messages with regard to form type properties.
Discussion
----------
[Form] Enhanced error messages
With this patch its much easier to see what went wrong with a form type and what the possible options are.
Also added tests for this error conditions.
Commits
-------
661421f [Doctrine] Remove AbstractDoctrineBundle and move code into Doctrine Bridge
Discussion
----------
[WIP] [Doctrine] Remove abtract doctrine bundle
Remove AbstractDoctrineBundle and move code into Doctrine Bridge. It is a BC break because all the "other" Doctrine Bundles MongoDB ODM, CouchDB ODM and PHPCR need to be updated to cope with this.
I will prepare PRs for them aswell and then remove the [WIP] here.
Bug fix: no
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: #2463
---------------------------------------------------------------------------
by beberlei at 2011/10/26 12:32:51 -0700
Here are all 3 PRs, can we coordinate on merging them somehow?
https://github.com/symfony/DoctrineMongoDBBundle/pull/50https://github.com/symfony-cmf/symfony-cmf/pull/118https://github.com/doctrine/DoctrineCouchDBBundle/pull/4
---------------------------------------------------------------------------
by beberlei at 2011/10/26 12:33:38 -0700
Ping @lsmith77 @jwage
---------------------------------------------------------------------------
by lsmith77 at 2011/10/26 12:35:29 -0700
all good for me ..
---------------------------------------------------------------------------
by stof at 2011/10/26 14:58:01 -0700
Well, this does not fix#2463. A change done in the bridge will still be able to break the service definitions of the other bundles or require tricky stuff to keep different versions of the logic.
---------------------------------------------------------------------------
by beberlei at 2011/10/26 22:49:39 -0700
@stof true, that is what https://github.com/doctrine/common/pull/71 will be about.
---------------------------------------------------------------------------
by stloyd at 2011/10/26 23:51:25 -0700
Comment just for linking cross PRs and for watching ;-) doctrine/common#71
---------------------------------------------------------------------------
by lsmith77 at 2011/10/31 08:18:45 -0700
please add forward compatibly into symfony 2.0 as per #2522
---------------------------------------------------------------------------
by beberlei at 2011/11/01 05:11:34 -0700
This doesn't make sense imho, since the number of people using the doctrine extension is 4 and everybody also prepared their pull request for this.
The problem is really that we need a branch for the MongoDB Bundle
---------------------------------------------------------------------------
by beberlei at 2011/11/01 05:40:49 -0700
@fabpot i created a branch in MongoDBBundle for 2.0 so this is ready to be merged.
While Definition::getArgument() could be used to fetch replaced values, it relied upon bad comparison logic (e.g. "index_1" > 1). Additionally, storing original arguments and replacements in the same array makes Definition::getArguments()'s bounds check unreliable. A single argument and its replacement would count twice, allowing getArgument(2) to pass the bounds check and result in an array index error.
With this new method, fetching of replacement arguments is more straightforward and bounds checking functions as it should.
Commits
-------
d3f137b cosmetic tweak
2877883 anything in front of ;q= is part of the mime type, anything after may be ignored
Discussion
----------
[HttpFoundation] fix splitHttpAcceptHeader() parsing of parameters
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
anything in front of ;q= is part of the mime type, anything after may be ignored
see http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.1
---------------------------------------------------------------------------
by lsmith77 at 2011/10/09 04:00:12 -0700
i must admit .. i am not 100% that my implemention is correct either .. but i am sure the current one isn't.
---------------------------------------------------------------------------
by lsmith77 at 2011/10/09 07:57:33 -0700
@fabpot: I am also not sure if getFormat() should optionally not support matching parameters, aka anything before ``;q=..``
Commits
-------
0907111 session data needs to be encoded because it can contain non binary safe characters e.g null. Fixes#2067
Discussion
----------
session data needs to be encoded because it can contain non binary safe characters e.g null., part 2
Bug fix: yes
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: #2067
I'm marking this as a compatibility break because session table should be cleared and even if not cleared all currently logged in users will be logged out.
This is the fix for a same issue in DBAL session storage made against master.
---------------------------------------------------------------------------
by schmittjoh at 2011/10/12 02:44:19 -0700
If I understand this correctly, only the PgSqlPlatform is affected by this. What do you think about adding an ``ìnstanceof PgSqlPlatform`` check?
---------------------------------------------------------------------------
by mvrhov at 2011/10/12 03:47:52 -0700
It's the same for sqlite, it just happens that mysql escapes \0, so we can say it's driver dependent.
The Drupal guys had the same issue http://drupal.org/node/690746 , they changed to column type to bytea for pgsql and for mysql to blob, also in Drupal report you can find that storing this into a session hash_file('md5', 'CHANGELOG.txt', TRUE) will trigger the similar problem in mysql.
The other thing to consider is what I mentioned in original bugreport, e.g igbinary as default serializer for session data.
Commits
-------
edfa29b session data needs to be encoded because it can contain non binary safe characters e.g null. Fixes#2067
Discussion
----------
session data needs to be encoded because it can contain non binary safe characters e.g null.
Bug fix: yes
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: #2067
I'm marking this as a compatibility break because session table should be cleared and even if not cleared all currently logged in users will be logged out.
---------------------------------------------------------------------------
by mvrhov at 2011/10/11 12:52:25 -0700
P.S. I know there was a talk about doctrine based session storage but I cannot find this in core. It probably has the same problem.
---------------------------------------------------------------------------
by eventhorizonpl at 2011/10/11 14:34:08 -0700
Thanks for tracking down and fixing this issue!
Best regards,
Michal
---------------------------------------------------------------------------
by stof at 2011/10/11 16:24:18 -0700
@mvrhov The Doctrine based storage is only available in master, not in 2.0
Commits
-------
c46fe16 Adding "sf-toolbar" class to toolbar wrapper to make it possible to check for its existance prior to js instantiation (in order to change page layout when profiler toolbar is enabled)
Discussion
----------
[WebProfilerBundle] Adding "sf-toolbar" class to toolbar wrapper to make it possible to check
Adding "sf-toolbar" class to toolbar wrapper to make it possible to check for its existance prior to js instantiation (in order to change page layout when profiler toolbar is enabled)
Commits
-------
808088a added the ability to use dot and single quotes in the keys and values
Discussion
----------
[2.0][Bugfix][DependencyInjection] added the ability to use dot and single quotes in the keys and values
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
We can not set a specific combination of dots and single quotes in the values and keys of the arguments. I.E.
```xml
<?xml version="1.0" ?>
<container xmlns="http://symfony.com/schema/dic/services"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://symfony.com/schema/dic/serviceshttp://symfony.com/schema/dic/services/services-1.0.xsd">
<services>
<service id="key_escaper" class="MyNamespace\MyClass">
<call method="setCollection">
<argument type="collection">
<argument key="only dot">.</argument>
<argument key="concatenation as value">.''.</argument>
<argument key="concatenation from the start line">''.</argument>
<argument key=".">is the same problem for the keys?</argument>
</argument>
</call>
</service>
</services>
</container>
```
As a result we have such a dump:
```php
<?php
class appDevDev_formapro1DebugProjectContainer extends Container
{
protected function getKeyEscaperService()
{
$this->services['key_escaper'] = $instance = new \MyNamespace\MyClass();
$instance->setCollection(array('only dot' => , 'concatenation as value' => '.\'\, 'concatenation from the start line' => '\'\, => 'is the same problem for the keys?'));
return $instance;
}
}
```
Commits
-------
27d0809 [MonologBridge] Adjust for Monolog 1.0.2
Discussion
----------
[MonologBridge] Adjust for Monolog 1.0.2
This is BC, it just turns off headers in browsers that don't have Firebug, I have had problems in IE with more than 150 log entries just crashing the page.
Commits
-------
6b872cf Check if cache_warmer service is available before doing the actual cache warmup
Discussion
----------
fix cache warump exception when service is not available
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: N/A
fixes [Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException]
You have requested a non-existent service "cache_warmer". in console when FrameworkBundle is removed from kernel.
Commits
-------
6b872cf Check if cache_warmer service is available before doing the actual cache warmup
40fb76d [Framework] removed wrong listener
Discussion
----------
fix cache warump exception when service is not available
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: N/A
fixes [Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException]
You have requested a non-existent service "cache_warmer". in console when FrameworkBundle is removed from kernel.
Commits
-------
96235a6 cs fix
46a69f1 define a WarmableInterface and only warm the cache if it implements warmable to allow replacing the core router. this fixes#2422. combining routers will only really work when #2450 is merged too.
Discussion
----------
define a WarmableInterface and only warm the cache if it implements warmable
define a WarmableInterface and only warm the cache if it implements warmable to allow replacing the core router. this fixes#2422. combining routers will only really work when #2450 is merged too.
Bug fix: yes
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes*
Fixes the following tickets: #2422
(*) tests:
success for
phpunit src/Symfony/Bundle/FrameworkBundle/Tests/
and
phpunit tests/Symfony/Tests/Component/Routing/
but when running all tests, i have to put gc_disable() into autoload to avoid segmentation fault and then get (after 3000 successful tests):
PHP Fatal error: Call to undefined method Symfony\Tests\Component\HttpKernel\Debug\StopwatchTest::assertCount() in /home/david/liip/symfony-cmf/cmf-sandbox/vendor/symfony/tests/Symfony/Tests/Component/HttpKernel/Debug/StopwatchTest.php on line 84
---------------------------------------------------------------------------
by stof at 2011/10/22 08:46:31 -0700
@dbu assertCount is new in PHP 3.6. It seems like the test of the new 2.1 feature has been written using 3.6-RC.
---------------------------------------------------------------------------
by dbu at 2011/10/22 09:40:07 -0700
@stof: ah, thanks for the hint. i hope you mean php 3.4 and not 3.6? but i assume symfony 2.1 should be able to run on 3.3, right?
anyways, the tests run through if i disable the StopWatch, so i guess we can consider the tests succeeding.
---------------------------------------------------------------------------
by stof at 2011/10/22 09:41:28 -0700
this is a method of PHPUnit TestCase class. I was talking about the PHPUnit version
Commits
-------
6b02ffb fix a typo in the routing dependency injection configuration. the request_context service is named router, not routing
Discussion
----------
fix a typo in the routing dependency injection configuration
fix a typo in the routing dependency injection configuration. the request_context service is named router.request_context , not routing.request_context
Bug fix: yes
Feature addition: no
Backwards compatibility break: ? - should not unless somebody relied on the bug
Symfony2 tests pass: yes
Fixes the following tickets: no ticket
i ran
phpunit src/Symfony/Bundle/FrameworkBundle/Tests/
and
phpunit tests/Symfony/Tests/Component/Routing/
If a request listener returns a response before calling the profiler
listener, the request will not be added in the stack leading to an error
during the handling of the kernel.response event. The profiler listener
should ideally be run first.
That allows projects that only use HttpFoundation and not HttpKernel to be able to
enforce the HTTP specification "rules".
$request = Request::createFromGlobals();
$response = new Response();
// do whatever you want with the Respons
// enforce HTTP spec
$response->prepare($request);
$response->send();
Within Symfony2, the prepare method is automatically called by the ResponseListener.
Commits
-------
ae342c7 unified toolbar.css
a7e4e70 unified profiler.css
09fe09e unified and corrected exception_layout.css
3a1674b unified and corrected exception.css
Discussion
----------
Unified and corrected CSS markup
Unified (spaces, braces, quotes, indention) and corrected (missing semicolon) the CSS markup.
Did not change any semantic, only markup!
@fabpot: New pull based on symfony:2.0 and changed formatting style as agreed in #2405
Commits
-------
1768544 Changed debug output to say Service Id rather than name for the service ids
Discussion
----------
Changed debug output to say Service Id rather than name for the service i
Changed debug output to say Service Id rather than name for the service ids
Commits
-------
9546ee6 Removed the IndexedReader
ea2cf73 Refactored the validator initializer
c6063ec [DoctrineBundle] Updated the code to use the new registry
a1784c2 [DoctrineBridge] Updated the code to use the new registry
Discussion
----------
Doctrine registry
This updates the Doctrine bridge and DoctrineBundle to use the new ManagerRegistry interface and its methods instead of using the old methods which are marked as deprecated.
Commits
-------
18cca22 cleaned up the use statements in the Registry class and RegistryInterface
Discussion
----------
cleaned up the use statements in the Registry class and RegistryInterface
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
just cosmetics ..
Commits
-------
63d2ce2 [FrameworkBundle] Fixed the ckeck for the router class
Discussion
----------
[FrameworkBundle] Fixed the ckeck for the router class
The getRouteCollection method is now part of the RouterInterface so the
command should accept any implementation of the interface instead of just
the implementations extending the core one.
The ORM now supports using a standard reader and does the needed logic
internally.
The IndexedReader is also available in Common for people needing it.
The getRouteCollection method is now part of the RouterInterface so the
command should accept any implementation of the interface instead of just
the implementations extending the core one.
Commits
-------
41ab1f7 updated RegistryInterface for https://github.com/doctrine/common/pull/68
Discussion
----------
[Doctrine] updated RegistryInterface
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
We tweaked the ManagerRegistry interface in https://github.com/doctrine/common/pull/68
So this PR just fixes the RegistryInterface as the PR was merged
Commits
-------
dc5772d use ORM master
4364463 use doctrine-common master
55b572d fixed getting the alias for a namespace
2b89e15 use getObjectNamespace() in getEntityNamespace()
0217a0e updated base class name
e8f3c21 updated vendors to point to lsmith77's fork of doctrine-common until its merged
6e87d01 fix tests
13c2f33 added a default implementation of the ManagerRegistry integrating the container
Discussion
----------
[Doctrine] added a default implementation of the ManagerRegistry
Bug fix: no
Feature addition: yes
Backwards compatibility break: yes (minor change in the interface see below)
Symfony2 tests pass: yes
Fixes the following tickets: -
added a default implementation of the ManagerRegistry integrating the container
attempted to maintain BC as good as possible, but RegistryInterface::getRepository() had to be dropped from RegistryInterface. Its still part of the ManagerRegistry, so its only a BC break for people using RegistryInterface to create their own implementation as I ran into https://bugs.php.net/bug.php?id=43200
all implementation (ORM/ODM) will need to match the changes to the ClassMetadataFactory interface
ORM, PHPCR, CouchDB have been upgraded already.
The Bundles also need to be updated. ORM is covered with this PR, I have a PR ready for PHPCR:
https://github.com/symfony-cmf/symfony-cmf/pull/108
also note that before merging the change to vendors.php needs to be fixed to point to the right repo again
For MongoDB it currently does not yet have a registry and I can take care of CouchDB once this is all merged.
---------------------------------------------------------------------------
by lsmith77 at 2011/09/23 00:40:07 -0700
still a few failing tests and details still need to be discussed ..
---------------------------------------------------------------------------
by lsmith77 at 2011/09/23 00:53:23 -0700
ok .. tests are passing now
---------------------------------------------------------------------------
by lsmith77 at 2011/10/11 10:27:52 -0700
ok Doctrine/ORM updates are done .. PR updated .. ready to be merged.
Commits
-------
1b57727 removed unused use statements
fd67c78 updated implementation to re-use the existing build() method
59e2e97 improves extensibility between bundles
Discussion
----------
[RFC] Improving extensibility between bundles
This is a quick draft for improving extensibility between different bundles.
The idea behind this is that an extension can provide configurable settings that other bundles can change.
---------------------------------------------------------------------------
by Seldaek at 2011/10/07 13:28:13 -0700
I am not yet sure what I would use it for, but I like the idea.
---------------------------------------------------------------------------
by lsmith77 at 2011/10/07 13:45:19 -0700
can you show a bit more how to use this?
---------------------------------------------------------------------------
by lsmith77 at 2011/10/07 13:47:38 -0700
oh it appears this is an example?
f4e76640a0 (diff-9)
---------------------------------------------------------------------------
by schmittjoh at 2011/10/07 13:57:00 -0700
yes
On Fri, Oct 7, 2011 at 10:47 PM, Lukas Kahwe Smith <
reply@reply.github.com>wrote:
> oh it appears this is an example?
>
>
> f4e76640a0 (diff-9)
>
> --
> Reply to this email directly or view it on GitHub:
> https://github.com/symfony/symfony/pull/2349#issuecomment-2328078
>
---------------------------------------------------------------------------
by schmittjoh at 2011/10/11 01:11:39 -0700
@fabpot, do you have an opinion on this, 👍👎?
Commits
-------
73312ab [Validator] The Type constraint now accepts the "Boolean" type instead of "boolean".
Discussion
----------
[Validator] The Type constraint now accepts the "Boolean" type instead of
[Validator] The Type constraint now accepts the "Boolean" type instead of "boolean".
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
---------------------------------------------------------------------------
by stloyd at 2011/10/11 03:43:24 -0700
As this is bugfix only, this should be made againts __2.0__ branch.
---------------------------------------------------------------------------
by hhamon at 2011/10/11 05:03:52 -0700
@stloyd I don't really know if it's a bug fix or an improvement. We could not use "Boolean" but we could use "boolean" or "bool" only. So that's why I just improved this constraint validator.
* 2.0:
[BrowserKit] fixed cookie updates from Response (the URI here is not the base URI, so it should not be used to determine the default values missing in the cookie, closes#2309)
Commits
-------
ba6bd4b [HttpKernel] Fix the FileProfileStorage, according to the tests
1654473 [HttpKernel] Create a test to outline the problem with file storage
Discussion
----------
[HttpKernel] Fix the file storage
The file storage was not correctly fetching children back.
* First the test showing the problem
* Second the fix to the test
Solution is to add a file for each stored profile file, containing the list of children tokens.
Commits
-------
249164d fixed typos
f4b7805 Edited src/Symfony/Bundle/FrameworkBundle/Resources/translations/validators.hy.xliff via GitHub
4cc1ed0 Edited src/Symfony/Bundle/FrameworkBundle/Resources/translations/validators.hy.xliff via GitHub
6a23e5f fixed typos
5b9d92a Fixed some errors
Discussion
----------
Translation of Framework Bundle in Armenian
fixed some typos
Without this, the exception is eventually thrown in EntityChoiceList::getIdentifierValues() with a very a message that doesn't really suit the real error: "Entities passed to the choice field must be managed"
Commits
-------
f49bbb7 updated
319fd9c Updated the file
0dc8c3f Edited src/Symfony/Bundle/FrameworkBundle/Resources/translations/validators.hy.xliff via GitHub
25d7596 Added 42-48 messages to Armenian translation
Discussion
----------
Armenian Translation (new)
updated armenian translation file for Framework Bundle
Commits
-------
05663ec First
Discussion
----------
Armenian Translation
I added armenian translation to Framework Bundle Validators
---------------------------------------------------------------------------
by stof at 2011/10/08 13:48:31 -0700
Can you do a PR based on the 2.0 branch instead ?
---------------------------------------------------------------------------
by tigranazatyan at 2011/10/08 13:51:03 -0700
I do not have skills using github, please do it yourself:)
The locale management does not require sessions anymore.
In the Symfony2 spirit, the locale should be part of your URLs. If this is the case
(via the special _locale request attribute), Symfony will store it in the request
(getLocale()).
This feature is now also configurable/replaceable at will as everything is now managed
by the new LocaleListener event listener.
How to upgrade:
The default locale configuration has been moved from session to the main configuration:
Before:
framework:
session:
default_locale: en
After:
framework:
default_locale: en
Whenever you want to get the current locale, call getLocale() on the request (was on the
session before).
Commits
-------
3f8e8c9 fixes a session max lifetime handling
3abb7f3 fixed some conflicts with garbage collection
a1888b2 added a dbal session storage
Discussion
----------
Dbal session storage
Adds a session storage based on Doctrine DBAL.
---------------------------------------------------------------------------
by lsmith77 at 2011/09/14 13:39:28 -0700
guess it would be nice to then provide a service inside the DoctrineBundle that reuses a global DoctrineDBAL connection, guess the connection to use would then need to be configured via the doctrine app config.
---------------------------------------------------------------------------
by schmittjoh at 2011/09/14 13:42:34 -0700
I haven't found a sane way to provide automatic configuration that's why I left this to the user to implement. It's also relatively easy:
```yml
services:
dbal_session_storage:
class: Symfony\Bridge\Doctrine\HttpFoundation\DbalSessionStorage
arguments: [@database_connection]
framework:
session: { storage_id: dbal_session_storage }
---------------------------------------------------------------------------
by stof at 2011/09/14 13:57:48 -0700
@lsmith77 There is an issue about reusing another DBAL connection: if the transaction is aborted by the ORM, the session could be aborted too as you cannot have 2 independent transactions AFAIK
---------------------------------------------------------------------------
by lsmith77 at 2011/09/14 13:59:57 -0700
not sure how this is relevant. i mean why does the transaction need to be left open? just begin, write, commit ..
---------------------------------------------------------------------------
by stof at 2011/09/14 14:02:39 -0700
and what if the transaction of the ORM is still opened (let's say you use SimpleThingsTransactionalBundle) ?
---------------------------------------------------------------------------
by lsmith77 at 2011/09/14 14:06:12 -0700
well thats a bit of an edge case imho. also i wonder if SimpleThingsTransactionalBundle shouldn't make sure that its transaction is closed before the session is written.
---------------------------------------------------------------------------
by schmittjoh at 2011/09/14 14:06:56 -0700
It closes them.
On Wed, Sep 14, 2011 at 11:06 PM, Lukas Kahwe Smith <
reply@reply.github.com>wrote:
> well thats a bit of an edge case imho. also i wonder if
> SimpleThingsTransactionalBundle shouldn't make sure that its transaction is
> closed before the session is written.
>
> --
> Reply to this email directly or view it on GitHub:
> https://github.com/symfony/symfony/pull/2182#issuecomment-2098100
>
---------------------------------------------------------------------------
by stof at 2011/09/14 14:15:02 -0700
@schmittjoh Does it close them **before** writing the session ?
---------------------------------------------------------------------------
by schmittjoh at 2011/09/14 14:44:15 -0700
I think either commit, or rollback is called, but @beberlei can probably answer that better.
Anyway, it is not really related to this PR because it is up to the user which connection is used.
---------------------------------------------------------------------------
by stof at 2011/09/14 14:58:48 -0700
I know that one of them is called. But if they are called after the session is persisted to the DB, a rollback is an issue as it will rollback the session persistence as well.
The PR is indeed fine. What need to be changed is the doc about how to use it, to advocate using a separate connection instead of the default one (which is used in your example)
---------------------------------------------------------------------------
by schmittjoh at 2011/09/15 02:57:34 -0700
There is no doc yet, but lets see what @fabpot thinks of all of this first.
---------------------------------------------------------------------------
by fabpot at 2011/09/15 04:57:57 -0700
Any benefits over using the PDO session storage?
---------------------------------------------------------------------------
by lsmith77 at 2011/09/15 05:00:50 -0700
cleaner code, potentially better support for niche RDBMS, centralized logging and finally afaik DoctrineDBAL has emulation for nested transactions.
---------------------------------------------------------------------------
by schmittjoh at 2011/09/15 05:11:00 -0700
The benefits (for me) are:
- logging queries
- better interoperability with existing build processes (migrations)
- better database interoperability
- re-using existing connection (I don't have the problem that Stof mentioned above)
---------------------------------------------------------------------------
by beberlei at 2011/09/15 06:18:22 -0700
The nested transactions is the problem here as @stof already said. If you reuse the default connection and use nested transactions that fail then this will make your session not save. That is why the docs should recommend you create a second connection for a dbal based session storage, even if it is using the same database. The PDO session storage would be a second connection besides DBAL aswell.
I like the migrations/schema hook though to create the table automatically through schema commands.
---------------------------------------------------------------------------
by fabpot at 2011/09/15 10:45:31 -0700
ok, looks good to me. Can you add some documentation about its usage (like the possible keys for options)? Is it possible to add some tests too?
---------------------------------------------------------------------------
by jdreesen at 2011/09/22 06:34:11 -0700
why did you close this?
---------------------------------------------------------------------------
by schmittjoh at 2011/09/30 06:26:12 -0700
I can't put more time into this PR, however I'm using it for some time already, and there shouldn't be any major issues as it is basically copy/paste from the PDO session.
Commits
-------
8bd0e42 [Form] Use proper parent (text) for EmailType and TextareaType
Discussion
----------
[2.0][Form] Use proper parent (text) for EmailType and TextareaType
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Commits
-------
95049ef [Form] Added type check to `ScalarToChoiceTransformer`
Discussion
----------
[2.0][Form] Added type check to ScalarToChoiceTransformer
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Commits
-------
5473d3b [Translation] Allow use of UTF-8 encoded catalogues into non-UTF-8 applications
deb6dea [Translation] Add failing tests to verify that UTF-8 lang files can't be used with another charset
Discussion
----------
Allow use of UTF-8 catalogues in non-UTF-8 applications
This is #2313 but targetting the master branch.
Bug fix: yes
Feature addition: ?:)
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
The problem I'm having is that, while porting an existing app, we are using UTF-8 everywhere to have a migration path ready, but the current application and DB is still in ISO-8859-1, which means translations containing accented chars are broken.
Also, we didn't hit the issue yet since we don't use forms much, but I imagine we would have similar issues with core translations for the validator which are all UTF-8 encoded.
Note that I explicitly suppressed this conversion in case your application is setup as UTF-8, to make sure most people are not affected by any slow down this introduces.
Commits
-------
8404b35 Updated indonesian translations for trans-unit 47 and 48
Discussion
----------
Updated indonesian translations for trans-unit 47 and 48
Commits
-------
ee0fe7a Added guessers for Size and SizeLength constraints
Discussion
----------
Added guessers for Size and SizeLength constraints
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
---------------------------------------------------------------------------
by jalliot at 2011/09/30 13:40:37 -0700
BTW, I've noticed that some constraints currently don't have guessers in 2.0:
* ``False`` (which could be guessed as a checkbox)
* ``True`` (which could be guessed as a required checkbox)
* ``Choice`` (which could be guessed as a choice type with medium confidence and with the choice list being guessed as the list provided for the constraint)
Are there any reasons why this is not implemented in 2.0 or should I try to make a PR for it?
There is also the ``Collection`` case but I guess it would be too difficult for this one...
Commits
-------
f9b2be9 Added french translation for SizeLength and UserPassword constraints
Discussion
----------
Added french translation for SizeLength and UserPassword constraints
---------------------------------------------------------------------------
by fabpot at 2011/09/30 13:45:46 -0700
47 is already taken for "This value should be the user current password". Should be 48.
---------------------------------------------------------------------------
by jalliot at 2011/09/30 13:59:01 -0700
@fabpot Fixed and added translation for 47 as well.
Commits
-------
0e00e3f [DoctrineBundle] CS
0c4b793 [DoctrineBundle] Fixed performances issues on "On-demand" proxy file generation
e866a67 [DoctrineBundle] Tries to auto-generate the missing proxy files on the autoloader
Discussion
----------
[DoctrineBundle] Tries to auto-generate the missing proxy files on the autoloaded
See:
https://github.com/symfony/symfony/issues/1965https://github.com/symfony/symfony/issues/1535
This fix is not really clean and there's maybe a factorizing work to do on it, but this work and avoid me spending my day deleting session cookies each time I clear cache.
---------------------------------------------------------------------------
by stloyd at 2011/08/23 10:37:28 -0700
You should follow Symfony2 CS (http://symfony.com/doc/current/contributing/code/standards.html).
---------------------------------------------------------------------------
by ruudk at 2011/09/26 02:50:13 -0700
+1
---------------------------------------------------------------------------
by fabpot at 2011/09/27 07:01:51 -0700
It looks like a bug fix, so this PR should be closed and a new one based on the 2.0 branch should be open. @beberlei: are you fine with this patch?
---------------------------------------------------------------------------
by beberlei at 2011/09/29 04:24:22 -0700
What is this for? I dont understand the bug and the solution here screams cache slam.
---------------------------------------------------------------------------
by beberlei at 2011/09/29 04:34:02 -0700
Ok i get the problem but the solution is still a monsterous hack. Can we find a real solution to the problem? There has to be one.
---------------------------------------------------------------------------
by Gregwar at 2011/09/29 04:34:25 -0700
@beberlei, when an user is authenticated for instance, there can be proxies serialized in session.
Si if you clear the cache in dev environment you'll get an error because the matching proxy classes won't exist and you'll be forced to clear your cookies and reauth, which can be annoying
---------------------------------------------------------------------------
by Gregwar at 2011/09/29 04:38:45 -0700
@beberlei, yes, I agree that we should do something more elegant, but the problem is that when PHP "meet" the proxy class we can't really know what was the "original" class it is supposed to extend
And as @schmittjoh said, this will only be executed very rarely
---------------------------------------------------------------------------
by beberlei at 2011/09/29 05:18:35 -0700
You agree you want something more suitable and still want this to be merged?
To ease the immediate pain wr could allow this however only in debug mode. A real solution here is maybe to move the proxy files out of the env folders. Rhey dont depend ont the env after all.
---------------------------------------------------------------------------
by stloyd at 2011/09/29 05:21:33 -0700
Proxy is not depending on env, but generation of proxy is... So this solution will be hard IMO, or even unacceptable...
---------------------------------------------------------------------------
by Gregwar at 2011/09/29 05:25:39 -0700
@beberlei what I meant is that I agree that's dirty but I don't think of anything better to solve this...
---------------------------------------------------------------------------
by fabpot at 2011/09/29 06:23:03 -0700
Even if the current patch is not the best solution, we should probably apply it to fix the problem and think about a better solution afterwards. Does it sound good for everybody?
Commits
-------
3223c5a Removed now useless test
21cf0ac Backported new behaviour from PR #2148 and removed check for interface at run-time
8b240d4 Implementation of kernel.event_subscriber tag for services.
Discussion
----------
Added missing kernel.event_subscriber tag (closes#2012)
This PR adds a ``kernel.event_subscriber`` tag which allows to register services as event subscribers in the same way ``kernel.event_listener`` allows to register them as event listeners.
The service is still lazy loaded and the DIC does not need to be recompiled for every modification in the service's code.
There is one important thing to remember:
If the service is created by a factory, the class parameter **MUST** reflect the real class of the service, although it is not needed at the moment for the DIC. For that issue, we could either forbid services created by factories or add a note to the documentation.
This PR closes#2012.
---------------------------------------------------------------------------
by jalliot at 2011/08/24 06:42:18 -0700
I'm not sure the test is good enough so feel free to add some more.
---------------------------------------------------------------------------
by jalliot at 2011/08/25 03:46:20 -0700
I re-implemented the check for EventSubscriberInterface in ContainerAwareEventDispatcher because I think the overhead is minimum and it allows to use this method even without the tag (at run-time).
I also added some tests for RegisterKernelListenersPass.
---------------------------------------------------------------------------
by stof at 2011/09/04 02:42:00 -0700
@jalliot Your branch conflicts with the current master. could you rebase it ?
---------------------------------------------------------------------------
by jalliot at 2011/09/04 02:57:03 -0700
Rebased
---------------------------------------------------------------------------
by jalliot at 2011/09/13 02:19:46 -0700
@fabpot What do you think about this PR? At the moment, the subscribers are not really usable in Symfony2 because of the lack of this tag.
---------------------------------------------------------------------------
by fabpot at 2011/09/13 04:17:46 -0700
I don't like subscribers. There are other PRs on adding more support for them, but the reality is that they are complex for no added benefit. I'm wondering if it wouldn't be better to just remove them altogether.
---------------------------------------------------------------------------
by jalliot at 2011/09/13 04:38:20 -0700
@fabpot Well I prefer listeners too but I think that if Symfony2 does support subscribers (which it does at the moment), it should do it properly and completely, thus allowing to register subscriber services like here or to register several methods for one same event like in #2148.
But I guess that if you merged those 2 PRs (well actually this one would have to be modified first if #2148 is merged but I'll do it then), many use cases would be covered and people should stop asking for more support :) (except maybe for removing the static modifier but this would be wrong IMO and prevent entirely this PR).
---------------------------------------------------------------------------
by fabpot at 2011/09/28 11:47:10 -0700
@jalliot: #2148 has been merged. Can you update this PR accordingly? thanks.
---------------------------------------------------------------------------
by jalliot at 2011/09/28 12:00:44 -0700
Sure thing. Will do it as well as removing the check for the interface tonight or tomorrow :)
---------------------------------------------------------------------------
by jalliot at 2011/09/29 08:53:17 -0700
@fabpot Check for interface removed and #2148 merged. Also rebased on latest master.
---------------------------------------------------------------------------
by fabpot at 2011/09/29 09:09:11 -0700
Tests do not pass.
---------------------------------------------------------------------------
by jalliot at 2011/09/29 09:18:48 -0700
@fabpot Fixed
Commits
-------
731b28b [composer] add missing deps for FrameworkBundle
9c8f100 [composer] change ext/intl to the new ext-intl syntax
d535afe [composer] fix monolog-bridge composer.json, add more inter-component deps
9ade639 [composer] add composer.json
Discussion
----------
Composer
This PR adds a composer.json file for [composer](https://github.com/composer/composer) ([more info](packagist.org/about-composer)).
For discussion you can also go into #composer-dev on freenode and argue with naderman, seldaek and everzet.
---------------------------------------------------------------------------
by naderman at 2011/09/26 15:51:51 -0700
You haven't entered any keywords, they might come in handy when searching for packages on packagist.
But really this is just a +1 ;-)
---------------------------------------------------------------------------
by stof at 2011/09/26 16:12:21 -0700
See my comments on your previous (non-rebased) commit: f1c0242b5a
---------------------------------------------------------------------------
by igorw at 2011/09/27 00:04:36 -0700
Following dependencies do not have a composer.json yet: Twig, Doctrine (orm, dbal, common), swiftmailer.
Also missing from the standard edition: assetic, twig-extensions, jsm-metadata, SensioFrameworkExtraBundle, JMSSecurityExtraBundle, SensioDistributionBundle, SensioGeneratorBundle, AsseticBundle.
The point is, those can be added later on. Having the components composerized is already a leap forward. Also, doctrine depends on some symfony components, we've got to start somewhere.
---------------------------------------------------------------------------
by Seldaek at 2011/09/27 00:36:41 -0700
Also, just for information, the plan is to have `symfony/framework-bundle` be the "framework", with all dependencies to doctrine etc, though we should really only have strict requirements in there, and then in symfony-standard we ship a composer.json that requires the framework-bundle, doctrine-orm and things like that that are not essential to core. Otherwise people don't have a choice about what they use anymore.
Just a comment btw, the json is invalid, all / should be escaped. However json_decode is nice enough to parse those without complaining, browsers do too, even Crockford's json2.js does, so I'm not sure if we should privilege readability over strictness, since it seems nobody really cares about this escaping.
---------------------------------------------------------------------------
by igorw at 2011/09/27 00:41:39 -0700
So, I've implemented all of @stof's suggestions, except (for reasons stated above):
* doctrine to DoctrineBundle
* swiftmailer to SwiftmailerBundle
* twig to TwigBundle
* doctrine-common to Validator
* FrameworkBundle (what exactly does it depend on?)
---------------------------------------------------------------------------
by stof at 2011/09/27 00:52:31 -0700
@igorw at least HttpKernel, Routing, Templating, EventDispatcher, Doctrine Common (annotations cannot be disabled), Translator, Form (optional), Validator (optional), Console (optional). See the service definitions to see the others
@Seldaek FrameworkBundle does not depend on Doctrine, except for Common
---------------------------------------------------------------------------
by beberlei at 2011/09/27 03:15:34 -0700
What does the symfony/ or ext/ prefix control in composer?
---------------------------------------------------------------------------
by Seldaek at 2011/09/27 03:33:52 -0700
symfony/ is just the (mandatory) vendor namespace. Also ext/ has been renamed to ext- now, so it's not in any vendor, and should avoid potential issues.
---------------------------------------------------------------------------
by beberlei at 2011/09/27 05:07:03 -0700
@Seldaek Mandatory? So every package name is "vendor/package"? I like that because previously i thought package names are not namespaced, and thus clashes could occur between different communities easily.
---------------------------------------------------------------------------
by Seldaek at 2011/09/27 05:16:20 -0700
@beberlei: Mandatory. As of yesterday http://packagist.org/ will tell you you have an invalid package name if there's no slash in it. See 1306d1ca82 (diff-3)
* 2.0:
[Validator] added support for grapheme_strlen when mbstring is not installed but intl is installed
removed separator of choice widget when the separator is null
Commits
-------
060d75d [Console] fixed CS and simplified code
a5ff635 [Console] added support for interactive shell without readline extension
Discussion
----------
[2.1] [Console] added support for interactive shell without readline extension
---------------------------------------------------------------------------
by lmcd at 2011/06/30 15:12:56 -0700
Very useful! Pre-installed PHP on OS X ships without readline.
---------------------------------------------------------------------------
by stof at 2011/06/30 15:24:47 -0700
@lmcd You can already use the console, just not the shell.
---------------------------------------------------------------------------
by lmcd at 2011/06/30 15:27:00 -0700
@stof Yep I know, but it would be nice to use the shell :)
---------------------------------------------------------------------------
by hason at 2011/07/01 01:54:34 -0700
@stof Interactive generators will not work on Windows and Mac OS X without these simple changes. I think this is bad news.
---------------------------------------------------------------------------
by Seldaek at 2011/07/01 01:58:59 -0700
Yup, it's definitely a good addition.
---------------------------------------------------------------------------
by Seldaek at 2011/07/01 01:59:26 -0700
@fabpot: I'm not sure why this is tagged 2.1, but IMO this should go in now.
---------------------------------------------------------------------------
by stof at 2011/07/01 01:59:44 -0700
@hason This is not true. The shell is only used when running the console with the ``-s`` option. A command can be interactive without using the shell (I'm on Windows)
---------------------------------------------------------------------------
by lmcd at 2011/07/01 02:08:17 -0700
@stof Yea true, the interactive generators are fine without readline, however I don't think this dependancy should prevent people using the shell on Win/Mac.
---------------------------------------------------------------------------
by hason at 2011/07/01 02:28:10 -0700
@stof This is good news :) I was hasty.
---------------------------------------------------------------------------
by lmcd at 2011/07/04 02:03:36 -0700
I'm feeling inclined to remove readline entirely for 2.1 and roll our own solution to support things like this in the shell: http://lmcd.me/v/sf-autocomplete-2.mov
---------------------------------------------------------------------------
by stof at 2011/09/04 04:55:37 -0700
what is the status of this PR ?
Commits
-------
5146a1f [EventDispatcher] Added possibility for subscribers to subscribe several times for same event
Discussion
----------
[EventDispatcher] Added possibility for subscribers to subscribe several times for same event
[EventDispatcher] Added possibility for subscribers to subscribe several times for same event
closes#2146
And it is of course fully BC :)
---------------------------------------------------------------------------
by jalliot at 2011/09/09 17:34:07 -0700
If merged, #2021 will have to reflect the change too
Commits
-------
dd20f01 Fixed assets:install to use a relative path instead of an absolute
Discussion
----------
[2.1] Fixed "assets:install" to create relative instead of absolute symlinks
This is a fairly simple fix so that the symlinks are relative to the resources rather than an absolute path that breaks from machine-to-machine or upon deployment.
We were trying to figure out why styles were messed up for other contributors, but then found that the paths were hard-coded for my machine :)
---------------------------------------------------------------------------
by ericclemmons at 2011/06/04 09:44:11 -0700
Any other thoughts/updates on this?
---------------------------------------------------------------------------
by fabpot at 2011/06/04 22:31:39 -0700
We have such a feature in symfony1 and IIRC it does not work very well. One problem is when you use symlink for some bundles. Then, you should not use a relative symlink as there is a common path between the two.
---------------------------------------------------------------------------
by ericclemmons at 2011/06/05 09:55:00 -0700
Sorry, I didn't think that we would be an issue since the Bundle "Best Practices" states to not include other bundles as dependencies.
If absolute links are a must, then the next alternative is for collaborators to add "/web/bundles" to .gitignore and each person run "assets:install" upon installation/update.
I was personally hoping there were a way to have this versioned for easier deployment.
On Jun 5, 2011, at 12:31 AM, fabpot<reply@reply.github.com> wrote:
> We have such a feature in symfony1 and IIRC it does not work very well. One problem is when you use symlink for some bundles. Then, you should not use a relative symlink as there is a common path between the two.
>
> --
> Reply to this email directly or view it on GitHub:
> https://github.com/symfony/symfony/pull/1173#issuecomment-1303600
---------------------------------------------------------------------------
by henrikbjorn at 2011/06/27 04:56:58 -0700
``` php
<?php
// ...
->addOption('relative', null, InputOption::VALUE_NONE, 'The --symlink option will generate relative paths')
// ...
```
and just default to absolute paths ?
---------------------------------------------------------------------------
by ericclemmons at 2011/06/27 08:37:50 -0700
I'm very supportive of that compromise. Up to @fabpot if I should add this back in, since relative paths were apparently problematic with symfony1.
---------------------------------------------------------------------------
by sbusch at 2011/07/15 08:46:01 -0700
+1
I'm developing on Mac and the files are mounted on a Linux box which serves the project. The paths are not the same on those two systems. If I accidentally install assets on my Mac the absolute paths won't work on the Linux webserver.
Other scenario: one teammate could add those symlinks by accident to the git repository, which breaks all other installations.
Relative symlinks could help a lot in these cases.
---------------------------------------------------------------------------
by ericclemmons at 2011/07/15 08:47:53 -0700
@sbusch Your issues are the same as mine, which prompted this ticket :)
Until this gets @fabpot's blessing, it's best to simply add `web/bundles` to your `.gitignore` file and tell your users to always run `assets:install --symlink` each time they pull down code & something breaks ;)
---------------------------------------------------------------------------
by sbusch at 2011/07/15 08:58:33 -0700
The handling (calculation) of relative symlinks IMO fits better to the `symlink()` method of `\Symfony\Component\HttpKernel\Util\Filesystem`. Possible method signature:
symfony/src/Symfony/Component/HttpKernel/Util/Filesystem.php:
```php
<?php
// ...
/**
* Creates a symbolic link or copy a directory.
*
* @param string $originDir The origin directory path
* @param string $targetDir The symbolic link name
* @param Boolean $copyOnWindows Whether to copy files if on Windows
* @param Boolean $makeRelative Whether to try to create a relative link
*/
public function symlink($originDir, $targetDir, $copyOnWindows = false, $makeRelative = false)
{
```
And what about changing the `--symlink` option to optionally have a value, instead of adding a new depending option? E.g. `--symlink[=absolute|relative]`, with "absolute" as default:
symfony/src/Symfony/Bundle/FrameworkBundle/Command/AssetsInstallCommand.php:
```php
<?php
// ...
class AssetsInstallCommand extends ContainerAwareCommand
{
/**
* @see Command
*/
protected function configure()
{
$this
->setDefinition(array(
new InputArgument('target', InputArgument::REQUIRED, 'The target directory (usually "web")'),
))
->addOption('symlink', null, InputOption::VALUE_OPTIONAL, 'Symlinks the assets instead of copying it. Allowed values: "absolute" (default) and "relative".', 'absolute')
->setHelp(<<<EOT
The <info>assets:install</info> command installs bundle assets into a given
directory (e.g. the web directory).
<info>./app/console assets:install web [--symlink]</info>
A "bundles" directory will be created inside the target directory, and the
"Resources/public" directory of each bundle will be copied into it.
To create a symlink to each bundle instead of copying its assets, use the
<info>--symlink</info> option. Use <info>--symlink=relative</info> for relative symlinks.
EOT
)
->setName('assets:install')
;
}
/**
* @see Command
*
* @throws \InvalidArgumentException When the target directory does not exist
*/
protected function execute(InputInterface $input, OutputInterface $output)
{
if (!is_dir($input->getArgument('target'))) {
throw new \InvalidArgumentException(sprintf('The target directory "%s" does not exist.', $input->getArgument('target')));
}
if ($input->hasOption('symlink'))
{
if (!function_exists('symlink')) {
throw new \InvalidArgumentException('The symlink() function is not available on your system. You need to install the assets without the --symlink option.');
}
if (!in_array($input->getOption('symlink'), array('absolute', 'relative'))) {
throw new \InvalidArgumentException(sprintf('Invalid value "%s" for option "symlink"', $input->getOption('symlink')));
}
}
$filesystem = $this->getContainer()->get('filesystem');
// Create the bundles directory otherwise symlink will fail.
$filesystem->mkdir($input->getArgument('target').'/bundles/', 0777);
foreach ($this->getContainer()->get('kernel')->getBundles() as $bundle) {
$originDir = $bundle->getPath().'/Resources/public';
if (is_dir($originDir)) {
$targetDir = $input->getArgument('target').'/bundles/'.preg_replace('/bundle$/', '', strtolower($bundle->getName()));
$output->writeln(sprintf('Installing assets for <comment>%s</comment> into <comment>%s</comment>', $bundle->getNamespace(), $targetDir));
$filesystem->remove($targetDir);
if ($input->hasOption('symlink')) {
$filesystem->symlink($originDir, $targetDir, false, $input->getOption('symlink') == 'relative');
} else {
$filesystem->mkdir($targetDir, 0777);
$filesystem->mirror($originDir, $targetDir);
}
}
}
}
```
---------------------------------------------------------------------------
by sbusch at 2011/07/15 09:04:46 -0700
@ericclemmons: yes, that's our current workaround. I started with manually converting absolute links to relative ones, but that quickly got very annoying ;-)
After that I tried to implement the generation of relative links by myself (where the proposals of my previous comment come from) until I found your PR.
---------------------------------------------------------------------------
by henrikbjorn at 2011/07/18 00:20:38 -0700
@sbush if it defaults to something how would you turn it off ?
---------------------------------------------------------------------------
by stof at 2011/07/18 00:26:16 -0700
@henrikbjorn a default value for the option is used when using ``--symlink`` without the value. If you don't use the option at all, it is disabled.
---------------------------------------------------------------------------
by stof at 2011/07/18 11:58:29 -0700
In fact no. The default value seems to be also used when the option is not set at all. @fabpot is this intended ?
---------------------------------------------------------------------------
by Seldaek at 2011/07/19 05:18:29 -0700
Symlinks on windows, although technically possible, don't quite work with PHP on most setups. Also git doesn't seem to support them either on windows (not sure why not). For those reasons, and although I'm sure this doesn't apply to every project, I would recommend you just have everyone run `assets:install [--symlink]` on their local machine, and make that command run on the server as part of your deployment process.
---------------------------------------------------------------------------
by ericclemmons at 2011/07/19 06:15:34 -0700
Nobody is even entertaining --relative?
On Jul 19, 2011, at 7:18 AM, Seldaek<reply@reply.github.com> wrote:
> Symlinks on windows, although technically possible, don't quite work with PHP on most setups. Also git doesn't seem to support them either on windows (not sure why not). For those reasons, and although I'm sure this doesn't apply to every project, I would recommend you just have everyone run `assets:install [--symlink]` on their local machine, and make that command run on the server as part of your deployment process.
>
> --
> Reply to this email directly or view it on GitHub:
> https://github.com/symfony/symfony/pull/1173#issuecomment-1606463
---------------------------------------------------------------------------
by Gregwar at 2011/08/10 08:56:27 -0700
I agree with the idea of proposing a --relative option, I'm currently working on a Samba mounted filesystem and I'm forced to create manually symlinks to get things working since the paths are not the same
Commits
-------
908a7a3 [HttpFoundation] Fix bug in clearCookie/removeCookie not clearing cookies set with a default '/' path, unless it was explicitly specified
Discussion
----------
[HttpFoundation] Fix bug in clearCookie/removeCookie not clearing cookies
[HttpFoundation] Fix bug in clearCookie/removeCookie not clearing cookies set with a default '/' path, unless it was explicitly specified
---------------------------------------------------------------------------
by Seldaek at 2011/08/02 10:31:44 -0700
The reason is that Cookie::__construct defaults to '/' btw, so if you don't specify it, and then call clearCookie without specifying again, the paths don't match.
---------------------------------------------------------------------------
by Koc at 2011/08/07 00:06:13 -0700
I think that correctrly use base path. Is it possible?
For example we have 2 apps
* site.com/app1/index.php
* site.com/app2/index.php
and app2 will remove cookies of app1
---------------------------------------------------------------------------
by Seldaek at 2011/08/07 02:58:10 -0700
IMO if people want that they should specify the path manually, by default cookies are always set for the entire host and I think it should stay like that.
---------------------------------------------------------------------------
by Koc at 2011/08/07 04:26:47 -0700
It is hard to specify path manually everywhere when set/remove cookies.
---------------------------------------------------------------------------
by Seldaek at 2011/09/27 07:01:43 -0700
@fabpot: ping? You said this was ok, but it was never merged.
Commits
-------
f7bf7b5 fixed condition
181332b added a Controller:getUser() shortcut to recover the current user
Discussion
----------
[2.1] added a Controller:getUser() shortcut to recover the current user
Commits
-------
c4a0f79 Updates according to suggestions.
6aec789 Added tests.
54454ba Added generic filtering to ParameterBag.
Discussion
----------
Added generic filtering to ParameterBag.
Adds filtering convenience using PHP's filter_var() e.g.
$request->get->filter($key, '', false, FITLER_SANITIZE_STRING);
See http://php.net/manual/en/filter.filters.php for capabilities.
---------------------------------------------------------------------------
by GromNaN at 2011/09/25 15:41:50 -0700
What is the use case ?
---------------------------------------------------------------------------
by drak at 2011/09/25 15:52:19 -0700
Input variable validation/sanitization. ParameterBag has a few built in like `getAlnum()` for example. This method offer's PHP's full filtering and sanitization suite.
---------------------------------------------------------------------------
by fabpot at 2011/09/27 00:56:41 -0700
Can you add some unit tests for this new feature?
---------------------------------------------------------------------------
by drak at 2011/09/27 00:58:56 -0700
Sure thing.
---------------------------------------------------------------------------
by drak at 2011/09/27 01:07:03 -0700
Before I make the commit, is the method name ok for you or would you prefer it is called `getFiltered()`?
---------------------------------------------------------------------------
by fabpot at 2011/09/27 01:13:46 -0700
`filter` sounds good to me.
---------------------------------------------------------------------------
by drak at 2011/09/27 02:37:01 -0700
I've added some tests.
---------------------------------------------------------------------------
by stloyd at 2011/09/27 02:42:42 -0700
@drak IMO you must check that user don't use unknown filter and/or flags for filter.
---------------------------------------------------------------------------
by drak at 2011/09/27 02:48:38 -0700
@stloyd - I'm not sure that's practical at all, this is a wrapper for a built-in PHP function and I don't understand why we would need validate arguments for a PHP function - it's the coder's job to use the API correctly - none of the inputs to this function are coming from a web request. It would also mean that the API would need to keep track of any upstream changes to constants in the PHP engine (which are just integers after all). It's really just not practical.
---------------------------------------------------------------------------
by stealth35 at 2011/09/27 05:16:50 -0700
@drak it's could be cool to use `filter_id` ✌️
if (is_string) {
$filter = filter_id($filter);
}
---------------------------------------------------------------------------
by drak at 2011/09/27 07:05:42 -0700
@stealth35 regarding this
if (is_string) {
$filter = filter_id($filter);
}
I believe strongly in the use of IDEs when coding and autocomplete nicely provides when you type `FILTER_`. Additionally, `filter_id()` only works on filters, but not for the flags, so I'm not entirely sure how useful it would be overall compared to using a good IDE (which you need when working with complex frameworks anyhow, imo :)
---------------------------------------------------------------------------
by drak at 2011/09/27 07:30:10 -0700
Ok check it now.
Commits
-------
d675c28 [FrameworkBundle] Use Router instead of RouterInterface
ae7ae8d [FrameworkBundle] Moved router_listener from web to router.xml since it depends on the router
35a9023 [FrameworkBundle] Added isEnabled to Router commands, fixes#1467536d979 [Console] Added Command::isEnabled method that defines whether to add the command or not
Discussion
----------
[2.1] [Console] Added Command::isEnabled method
This addresses #1467.
The idea is to allow commands to evaluate whether they can run or not, since they are automatically registered.
- It's useful for the two router:* commands since they're optional (router can be disabled), but part of the FrameworkBundle that is not really optional.
- It could be useful for third party code as well.
- It's BC.
- aa95bb0d395810b29a3e654673e130736d9d1080 should address the issue in #1467, while the other commits just make sure the command is not registered at all if the router isn't standard.
One issue remains though:
- A few other services like twig helpers get the `ròuter` injected, this means that if there is really **no** router service defined, there is still an error. I'm not sure how to fix those beyond adding `on-invalid="null"` but I'm not sure if that's desirable. I guess we could argue that the router is a big candidate for replacement/suppression, and as such it should be truly optional, but if we do it I don't know where it'll lead. I don't want to end up in a situation where half the dependencies are optional to support every possible combination. @fabpot wdyt?
---------------------------------------------------------------------------
by kriswallsmith at 2011/06/28 16:19:46 -0700
I'd rather see us not register a command instead of register and then disable it. Can we do the same thing you've done here in the bundle's registerCommands() method?
---------------------------------------------------------------------------
by Seldaek at 2011/06/28 16:51:36 -0700
Note that it's never really registered. During the registration it's checked and skipped if not enabled.
However, doing it as you suggest means overriding/copy-pasting all the code from the core Bundle class, which I don't like so much. It also means adding code specific to those two commands in a somewhat unrelated place, which I also don't like.
I'm not saying the current solution is perfect, but from the alternatives I considered, it's the best I have found.
---------------------------------------------------------------------------
by stof at 2011/09/04 04:58:04 -0700
@Seldaek your branch conflicts with master. could you rebase it ?
@fabpot what do you think about this PR ?
---------------------------------------------------------------------------
by Seldaek at 2011/09/04 08:39:05 -0700
Rebased
Builds upon aead4a9836180cabae4d47fe27c634dcd79ac8f2, which prematurely removed request scoping from the assets templating helper in all cases. The helper need only be request-scoped if one or more request-scoped packages (e.g. PathPackages) are injected into it. This change makes it possible to utilize the assets helper outside of a request (e.g. during a console script).
To ensure that the assets helper is not assigned a request scope, all asset base URL's must be defined for all packages (default and any named) and both protocols: HTTP and SSL. The included test config fixtures concisely accomplish this by specifying a single HTTPS URL as the base URL for our default and named package, since FrameworkExtension's Configuration conveniently registers this URL for both protocols.
No other helpers have request scope and the assets helper's parameters don't appear to depend on the request in any way, so this appears to be unnecessary. As-is, request scope here prevents use of the assets helper from a console command that may need to internally render a template.
Adds filtering convenience using PHP's filter_var() e.g.
`$request->get->filter($key, '', false, FITLER_SANITIZE_STRING);`
See http://php.net/manual/en/filter.filters.php for capabilities.
* 2.0:
bumped Symfony version to 2.0.3-DEV
updated VERSION for 2.0.2
update CONTRIBUTORS for 2.0.2
updated CHANGELOG for 2.0.2
updated vendors for 2.0.2
merged branch helmer/target_path (PR #2228)
Commits
-------
022a9a7 [Security] Make saving target_path extendible
Discussion
----------
[Security] Make saving target_path extendible
The problem lies in how Security component handles ``target_path`` - the latest request URI is always stored. This can lead to problems in following scenarios:
a) The response type of the request is not HTML (think JSON, XML ..)
b) The URI matches a route that does not listen to HTTP GET
I opened a [PR](https://github.com/symfony/symfony/pull/604) months ago, to partly solve scenario A, which did not make it. Now I am proposing a different solution - user can extend ``ExceptionListener`` and override the logic behind setting the ``target_path`` to match his precise needs.
In my simplified scenario, I would be using:
```
protected function setTargetPath(Request $request)
{
if ($request->isXmlHttpRequest() || 'GET' !== $request->getMethod()) {
return;
}
$request->getSession()->set('_security.target_path', $request->getUri());
}
```
@Seldaek, @schmittjoh, @lsmith77, thoughts?
---------------------------------------------------------------------------
by Seldaek at 2011/09/21 02:37:02 -0700
Seems like a better solution for flexibility's sake. Would be quite awesome if you could add a cookbook entry to symfony/symfony-docs about this, otherwise I'm afraid we'll have to explain it over and over again :)
---------------------------------------------------------------------------
by helmer at 2011/09/21 03:38:57 -0700
[Cookbook](b22c5e666e) entry done. Perhaps though I rushed ahead ..
---------------------------------------------------------------------------
by Seldaek at 2011/09/21 03:52:01 -0700
Thanks. You can already do a pull request against symfony-docs, just reference this pull request in it so it's not merged before this is merged.
Commits
-------
67c33a8 Rebased with master, and fixed wrong behavior with proper tests coverage
f8a6a4b Be sure that both fields have same value for required option in RepeatedType
0679220 Additional test coverage for changes in RepeatedType
b23d47d moved options test form from class->method scope
5fe5556 fixed accidental permission change
a969434 [Form] fixed CS, merged options, added tests
8819db3 [Form] Allow setting different options to repeating fields
Discussion
----------
[2.1] [Form] Allow setting different options at RepeatedType fields
This an test covered version of #1348 (rebased with master).
---------------------------------------------------------------------------
by stloyd at 2011/06/27 04:18:19 -0700
@fabpot What do you think about this ? I'm just not sure that we should allow setting `required` per field, IMO better would be forcing this option from default `$options['options']` and ignore that field in `$options['first_options']` and/or `$options['second_options']`.
---------------------------------------------------------------------------
by stloyd at 2011/07/02 00:00:04 -0700
@fabpot ping.
---------------------------------------------------------------------------
by fabpot at 2011/07/06 05:45:56 -0700
Let's discuss this new feature for 2.1.
---------------------------------------------------------------------------
by stloyd at 2011/08/24 01:12:59 -0700
Rebased with master.
---------------------------------------------------------------------------
by stof at 2011/09/04 05:02:42 -0700
@fabpot What do you think about this feature ? It is now time to discuss it :)
---------------------------------------------------------------------------
by fabpot at 2011/09/22 00:18:29 -0700
Tests do not pass.
---------------------------------------------------------------------------
by stloyd at 2011/09/24 01:54:42 -0700
@fabpot Should be ok now.
Commits
-------
afc0971 make it easier to customize the cache lookup in the TemplateLocator
Discussion
----------
make it easier to customize the cache lookup in the TemplateLocator
---------------------------------------------------------------------------
by fabpot at 2011/09/22 01:21:56 -0700
Do you any use case for that?
---------------------------------------------------------------------------
by lsmith77 at 2011/09/22 01:27:06 -0700
Yes in the ThemeBundle we override the default cache to include the key + theme:
https://github.com/liip/liipthemebundle/pull/9/files#L7R53
---------------------------------------------------------------------------
by lsmith77 at 2011/09/22 01:29:18 -0700
though i just thought about it .. if we do not put this into 2.0, it might not be worth the trouble.
---------------------------------------------------------------------------
by pjedrzejewski at 2011/09/22 02:19:42 -0700
+1 for this.
attempted to maintain BC as good as possible, but RegistryInterface::getRepository() had to be dropped from RegistryInterface. Its still part of the ManagerRegistry, so its only a BC break for people using RegistryInterface to create their own implementation
Commits
-------
f4784f7 [DomCrawler] Submit on a <form> node
Discussion
----------
DomCrawler - ability to submit a form that doesn't have any buttons
The proposed modification allows to submit above a <form> tag.
Using the DomCrawler component (among others), I have to interact with a remote site that has a form without a submit button (submitted automatically by javascript). This prompted the quick fix I'm sending. Please tell me if there is anything I should do differently and I'll modify it.
Thanks :)
---------------------------------------------------------------------------
by fabpot at 2011/09/12 00:46:07 -0700
Looks good to me. Can you add some unit tests for this new behavior? Thanks.
---------------------------------------------------------------------------
by jc- at 2011/09/12 02:27:25 -0700
Honored to meet you. I'm trying to run the test suite but 24 tests fail even without my commit. I'll try to get it sorted out and submit tests for this ASAP.
Commits
-------
6d8c4a8 change nested collection indentation from 2 to 4
Discussion
----------
Changed Yaml Dumper nested collection indentation
This PR changes the dumpers nested collection indentation from 2 to 4 which seems to be the standard.
Commits
-------
022a9a7 [Security] Make saving target_path extendible
Discussion
----------
[Security] Make saving target_path extendible
The problem lies in how Security component handles ``target_path`` - the latest request URI is always stored. This can lead to problems in following scenarios:
a) The response type of the request is not HTML (think JSON, XML ..)
b) The URI matches a route that does not listen to HTTP GET
I opened a [PR](https://github.com/symfony/symfony/pull/604) months ago, to partly solve scenario A, which did not make it. Now I am proposing a different solution - user can extend ``ExceptionListener`` and override the logic behind setting the ``target_path`` to match his precise needs.
In my simplified scenario, I would be using:
```
protected function setTargetPath(Request $request)
{
if ($request->isXmlHttpRequest() || 'GET' !== $request->getMethod()) {
return;
}
$request->getSession()->set('_security.target_path', $request->getUri());
}
```
@Seldaek, @schmittjoh, @lsmith77, thoughts?
---------------------------------------------------------------------------
by Seldaek at 2011/09/21 02:37:02 -0700
Seems like a better solution for flexibility's sake. Would be quite awesome if you could add a cookbook entry to symfony/symfony-docs about this, otherwise I'm afraid we'll have to explain it over and over again :)
---------------------------------------------------------------------------
by helmer at 2011/09/21 03:38:57 -0700
[Cookbook](b22c5e666e) entry done. Perhaps though I rushed ahead ..
---------------------------------------------------------------------------
by Seldaek at 2011/09/21 03:52:01 -0700
Thanks. You can already do a pull request against symfony-docs, just reference this pull request in it so it's not merged before this is merged.
Commits
-------
e1aabd2 [FrameworkBundle] Updated the Russian translations.
Discussion
----------
[FrameworkBundle] Updated the Russian translations.
Added translations for a new error messages of image validator.
Added translations for the user password validator.
Commits
-------
fea74db Added information page with better messages for Profiler
Discussion
----------
[WebProfiler] Added information page with better messages
---------------------------------------------------------------------------
by stloyd at 2011/08/30 23:29:27 -0700
@fabpot Any decision about this ?
Commits
-------
132fbe3 [WebProfilerBundle] Merge position and css_position
defdb82 [WebProfilerBundle] Remove unused line
69a50ab [WebProfilerBundle] Add the posibility to specify position of toolbar
Discussion
----------
[WebProfilerBundle] Add the posibility to specify position of toolbar
I'm facing a project where everything is at the bottom of the page, positioned with CSS.
This PR adds the possibility to specify the position of the toolbar, with configuration :
web_profiler:
toolbar: true
intercept_redirects: false
css_position: top
---------------------------------------------------------------------------
by stof at 2011/09/06 12:11:27 -0700
Looking at the code rendering the toolbar, there is already a ``position`` parameter used to display the toolbar on top in the profiler. Maybe we could look into reusing it instead of having a second one named ``css_position``.
But the phpdoc says ``bottom, normal, or null -- automatically guessed``. Using ``bottom`` will result in ``position: bottom`` in the CSS, which is broken. @fabpot is it simply a left-over of a previous version ?
---------------------------------------------------------------------------
by alexandresalome at 2011/09/16 00:56:11 -0700
I merged parameters and changed the documentation for 3 values : ```bottom, top or normal```.
Commits
-------
4ca09a9 [Validator] Validate object with it's own entity manager by default
Discussion
----------
[Validator] Validate object with it's own entity manager by default
While working with multiple object managers the default validation uses default entity manager, requiring the dev to create own validation services for each entity using other entity managers. This commit causes entity to be validated (by default) by it's own entity manager, while still preserving a possiblity to specifiy $constraint->em.
This is was a pull request to 2.0, changed to master according to stof's suggestion.
Commits
-------
7d3c2df [SecurityBundle] added a validator for the user password
Discussion
----------
[SecurityBundle] added a validator for the user password
This validator is useful when you want to validate that an input value
is equal to the user current password (in a form where the user can change
his password for instance).
Note that this should not be used to validate a login form as this is
done automatically by the built-in security mechanism.
---------------------------------------------------------------------------
by Palleas at 2011/09/21 08:36:14 -0700
This is kinda what I wrote for my project 2 days ago, I'm definitely +1 on this ;-)
---------------------------------------------------------------------------
by stealth35 at 2011/09/21 08:45:55 -0700
👍
Commits
-------
e13998d updated indonesian translation for image validator
Discussion
----------
Updated indonesian translation for image validator
---------------------------------------------------------------------------
by stof at 2011/09/21 10:49:48 -0700
Can you send a PR to the 2.0 branch for the trans-unit 41 ?
---------------------------------------------------------------------------
by alifity at 2011/09/21 11:05:48 -0700
Hi @stof, I've sent another PR for trans-unit-41 :)
This validator is useful when you want to validate that an input value
is equal to the user current password (in a form where the user can change
his password for instance).
Note that this should not be used to validate a login form as this is
done automatically by the built-in security mechanism.
Commits
-------
a0329c3 Added lifetime/cleanup support.
365e73a Fixed the find() method and changed the way the profile data is stored.
beeec5e Allow socket dsn (for example mongodb:///tmp/mongodb-27017.sock).
218eaba Fixed storage of time value.
85c3806 Added support for sorting by time like other profiler storage implementations.
73692c6 Fixed MongoDbProfilerStorage::find() when passing empty parameters.
4cd2dec Use token as identifier to make usage of the automatically created index.
Discussion
----------
[2.1] [HttpKernel] MongoDB profiler updates
I fixed one issue within the MongoDbProfilerStorage::find() function and made some more changes.
---------------------------------------------------------------------------
by snc at 2011/09/11 02:28:35 -0700
Please don't merge this in yet. There are some more commits pending...
---------------------------------------------------------------------------
by fabpot at 2011/09/14 01:07:39 -0700
@snc: is it ready for a merge?
---------------------------------------------------------------------------
by snc at 2011/09/14 01:20:32 -0700
Unfortunately not... while testing I found out that the currently merged in implementation does not work completely. The web profiler search function errors because the find function returns the wrong data. I fixed this already but now I have some strange "maximum function nesting level reached" errors when viewing profiles with children. I will work on it later today.
---------------------------------------------------------------------------
by snc at 2011/09/14 13:27:50 -0700
Now only one thing is missing... the generated container code looks like this:
`$this->services['profiler'] = $instance = new \Symfony\Component\HttpKernel\Profiler\Profiler(new \Symfony\Component\HttpKernel\Profiler\MongoDbProfilerStorage('mongodb://localhost/sf2-mongo-profiler/profiler', '', '', 86400), $a);`
The current constructor only uses the first parameter (dsn). What about the username, password and lifetime? Username and passwort can already be passed via the dsn... but the lifetime feature is not part of the interface... should I implement it?
---------------------------------------------------------------------------
by fabpot at 2011/09/15 11:03:02 -0700
The `lifetime` is used to cleanup the database (see https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpKernel/Profiler/PdoProfilerStorage.php#L136). So, it should probably be implemented for MongoDB as well (but it can probably be done in another PR).
---------------------------------------------------------------------------
by snc at 2011/09/19 13:42:52 -0700
Sorry for the delay, lifetime support is now implemented. What do you think about an AbstractProfilerStorageTest class to share some testing code between the different implementations (of cause in a separate PR)?
Commits
-------
83199ae [FrameworkBundle] Fix unintuitive merging behavior for assets_base_urls
4061114 [FrameworkBundle] fixes unintuitive merging behavior
Discussion
----------
[FrameworkBundle] fixes unintuitive merging behavior
---------------------------------------------------------------------------
by fabpot at 2011/09/16 10:04:53 -0700
I think this is a "bug", no? If this is the case, then we need to fix the 2.0 branch.
---------------------------------------------------------------------------
by schmittjoh at 2011/09/17 00:34:14 -0700
It is a change in behavior, but whether or how you want to merge this is really up to you.
---------------------------------------------------------------------------
by jmikola at 2011/09/19 08:09:26 -0700
I was about to create a PR for this very same quirk, as this was causing my CDN's for various environments to all get merged together. I think we can get away with merging it directly to 2.0 since `assets_base_urls` are hardly covered in the documentation at all.
Once this gets merged, I wouldn't mind writing up a blurb on them to explain how the shorthand syntax works and this merging strategy.
---------------------------------------------------------------------------
by jmikola at 2011/09/19 08:28:21 -0700
I just noticed this PR only fixes the `base_urls` config option under `packages`. We should also correct this behavior for `assets_base_urls`, which appears further up in FrameworkBundle's Configuration.php file.
---------------------------------------------------------------------------
by jmikola at 2011/09/19 08:44:57 -0700
@schmittjoh: I have the second commit for this sitting in https://github.com/jmikola/symfony/tree/configFix (rebased on your branch) if you'd prefer to merge that into your branch to update this PR.
---------------------------------------------------------------------------
by schmittjoh at 2011/09/19 08:55:42 -0700
Merged it in. Thanks.
---------------------------------------------------------------------------
by fabpot at 2011/09/19 09:01:27 -0700
ok, I'm going to merge this into master.
@jmikola: Can you submit documentation for the new way?
Commits
-------
b9b6084 [FrameworkBundle] Updated Czech translations for image validator
Discussion
----------
[FrameworkBundle] Updated Czech translations for image validator
---------------------------------------------------------------------------
by stof at 2011/09/10 12:10:08 -0700
Could you send a PR against the 2.0 branch for the trans-unit up to 41 which are already part of 2.0 ? Adding 42 to 46 should then still be done in master as they are new for 2.1
Commits
-------
c6b15b3 [WebProfilerBundle] Variables only used once
847c665 [WebProfilerBundle] Use panel URL for debugging toolbar
fe13a6c [WebProfilerBundle] Fix CS
fe76d74 [WebProfilerBundle] Propose to open debug toolbar request in an error occured.
Discussion
----------
[WebProfilerBundle] Propose to open debug toolbar request in an error occ
[WebProfilerBundle] Propose to open debug toolbar request in an error occured.
This is very useful when creating data collectors and an error occurs
when redering the toolbar via XHR.
The only problem is that the exception page renders the toolbar via JS, too.
---------------------------------------------------------------------------
by stof at 2011/09/17 05:26:10 -0700
IMO, you should propose to open the link to the profiler (if the profiler is enabled) instead of opening the toolbar alone
---------------------------------------------------------------------------
by alexandresalome at 2011/09/17 05:34:50 -0700
Thanks @stof
---------------------------------------------------------------------------
by alexandresalome at 2011/09/17 05:50:11 -0700
4 commits for 2 lines... I should change my job. Thanks @stof, again
Commits
-------
9fe87be More explicit default value for assets_version_format
Discussion
----------
Fixed default asset version format
This is not needed as it is already the value that is set when null in ``Symfony\Component\Templating\Asset\Package`` but that would make it clearer for people who just read the configuration.
Commits
-------
78c630c Added access to Doctrine's ValidateSchema command from the console. See ticket 2200.
Discussion
----------
Added access to Doctrine's ValidateSchema command from the console.
See ticket 2200.
Add a trivial Proxy wrapper class to access Doctrine's ValidateSchema() command from app/console
* 2.0:
[HttpKernel] fixed typo
fixed previous merge, done the same change to other occurences
fixes usage of mb_*
Profiler session import fixed.
[Process] workaround a faulty implementation of is_executable on Windows
Swedish translation fix.
[Locale] Fix#2179 StubIntlDateFormatter support yy format
Fixed fourth argument of Filesystem->mirror()
Commits
-------
046a125 [FrameworkBundle] Set the file storage as default storage for Symfony2
Discussion
----------
[FrameworkBundle] Set the file storage as default storage for Symfony2
---------------------------------------------------------------------------
by Seldaek at 2011/09/16 07:41:48 -0700
What's the reasoning behind this? Is SQLite causing issues? Isn't it faster than file storage?
---------------------------------------------------------------------------
by alexandresalome at 2011/09/16 07:50:50 -0700
3 reasons :
* The file storage is faster than the SQLite storage (see https://github.com/symfony/symfony/pull/1772)
* It's the only reason why Symfony2 is dependent of SQLite
* SQLite profiler has problems with concurrency (especially when concurrent access occurs).
---------------------------------------------------------------------------
by Seldaek at 2011/09/16 07:51:50 -0700
Ok sorry I missed the other PR. Sounds great :)
Commits
-------
95dc7e1 Fixed fourth argument of Filesystem->mirror()
Discussion
----------
Fixed fourth argument of Filesystem->mirror()
See #2027 and #2033 for discussion.
@fabpot said that we don't want to use symlink at all on Windows so if this is confirmed, we should also change ``Filesystem->symlink()`` implementation.
---------------------------------------------------------------------------
by alexandresalome at 2011/09/16 08:29:40 -0700
Tested on Windows, OK for me
Commits
-------
8e2cbe6 fixes usage of mb_*
Discussion
----------
Fixes usage of mb_strlen
---------------------------------------------------------------------------
by Seldaek at 2011/09/16 05:33:45 -0700
This will fail if the mbstring ext isn't enabled, you should still test for the mb_ function first.
Commits
-------
13b77bf Treat defaults enclosed between % as parameters from dic This allows as to define default like this
Discussion
----------
[Route] Treat defaults enclosed between % as parameters from dic
This allows as to define default like this
foo:
pattern: /{_locale}/login
defaults:
_controller: my_login_controller:loginAction
_locale: %session.default_locale%
---------------------------------------------------------------------------
by lsmith77 at 2011/08/10 07:00:14 -0700
this is a bit of a BC break .. but in general it does address a huge need for being able to make routes more easily configurable.
also didnt check this, but we should make sure that this doesnt open any security issues.
---------------------------------------------------------------------------
by maoueh at 2011/08/10 08:31:18 -0700
Hi,
There is an issue pending for the same feature here #1718. Maybe this one could be linked to #1718 somehow.
Regards,
Matt
---------------------------------------------------------------------------
by fabpot at 2011/09/12 23:51:53 -0700
@lsmith77: Why is it a BC break?
---------------------------------------------------------------------------
by lsmith77 at 2011/09/13 00:04:46 -0700
well its only a BC break on the off chance that someone puts stuff enclosed in % in the defaults atm and does not expect them to be interpreted as parameters. not very likely. but at the least we might want to first check if the parameter exists before replacing it.
---------------------------------------------------------------------------
by mvrhov at 2011/09/13 23:28:48 -0700
So, do I check if parameter exists inside dic and throw a notice if not, or do I just fix the comment?
Commits
-------
e6e5146 [Translation] now support ResourceBundle
Discussion
----------
[2.1][Translation] now support ResourceBundle
support `.res` and `.dat` bundles
---------------------------------------------------------------------------
by marijn at 2011/09/08 08:59:39 -0700
There are a few references to `ressource`, I guess that is a typo...
---------------------------------------------------------------------------
by stealth35 at 2011/09/08 09:13:32 -0700
@marijn thank, done
---------------------------------------------------------------------------
by fabpot at 2011/09/11 00:42:37 -0700
Is it possible to add a dumper like we have for all other loaders?
---------------------------------------------------------------------------
by stof at 2011/09/11 03:46:39 -0700
Btw, you need to rebase your branch as it conflicts with master
---------------------------------------------------------------------------
by stealth35 at 2011/09/11 04:04:23 -0700
@fabpot it's more difficult (or the easy way it's to use `exec` with `derb`), I can create the text resources
@stof oki
---------------------------------------------------------------------------
by fabpot at 2011/09/11 23:52:19 -0700
@stealth35: Can you remove the `@api` tags? We will review what is included into the public API later on. thanks.
---------------------------------------------------------------------------
by stealth35 at 2011/09/12 04:18:07 -0700
@fabpot done
Commits
-------
aecfd0a [HttpFoundation] Support user and password in url
Discussion
----------
[HttpFoundation] Support user and password in url
Allow `http://user:password@test.com` for url request and `basic auth`
Added Methods for `Request`
- `getUser`
- `getPassword `
---------------------------------------------------------------------------
by stealth35 at 2011/08/26 07:52:57 -0700
@seldaek @stof thanks guys it's done
---------------------------------------------------------------------------
by stof at 2011/08/28 11:52:11 -0700
btw, you should rebase your branch on top of the upstream master branch as it conflicts
---------------------------------------------------------------------------
by stealth35 at 2011/08/28 13:15:55 -0700
@stof done
---------------------------------------------------------------------------
by stof at 2011/09/04 02:09:11 -0700
@stealth35 you made an error when rebasing: you merged the previous version of the branch instead of forcing the push, thus duplicating all commits in the PR (rebasing rewrites the history so it creates new commits). Could you clean your branch ?
---------------------------------------------------------------------------
by stealth35 at 2011/09/05 02:00:17 -0700
@stof 👍
---------------------------------------------------------------------------
by stealth35 at 2011/09/06 08:21:00 -0700
should be 2.0 ?
---------------------------------------------------------------------------
by fabpot at 2011/09/11 23:52:30 -0700
@stealth35: Can you remove the `@api` tags? We will review what is included into the public API later on. thanks.
---------------------------------------------------------------------------
by stealth35 at 2011/09/12 04:02:01 -0700
@fabpot done
Commits
-------
a0a97c6 Removed executable bits from all php files
Discussion
----------
Removed executable bits from all PHP files
Some files had a file mode of 755 and this PR changes them to 644. The reason behind this is that git always thinks that those files are changed when accessing the repository via a samba share on windows (tested with PhpStorm).
---------------------------------------------------------------------------
by fabpot at 2011/09/09 05:51:30 -0700
That was on my radar too. Can you do the same for the 2.0 branch?
Commits
-------
ef322f6 -- add command that extracts translation messages from templates
Discussion
----------
[2.1] Extracting translation messages from templates
As seen here #1283 and here #2045, I push the command that extract translation from templates.
There are still a lot of new things here, but it seems more manageable.
---------------------------------------------------------------------------
by stof at 2011/09/04 02:04:40 -0700
@michelsalib Could you try to refactor the code to make it more flexible by moving the creating of the file to the dumpers to support other outputs (database...) ?
---------------------------------------------------------------------------
by michelsalib at 2011/09/04 02:35:50 -0700
You are right, I shall do it tonight.
---------------------------------------------------------------------------
by michelsalib at 2011/09/04 11:53:35 -0700
I just pushed a refactoring that should allow more flexibility. Dumpers are now responsive for writing the files. This way it is now possible to implement the DumperInterface that dump to a database and add it to the TranslationWriter.
I updated the tests accordingly.
---------------------------------------------------------------------------
by fabpot at 2011/09/05 23:27:27 -0700
To be consistent with other dumpers in the framework, the dumpers extending `FileDumper` should use `FileDumper` as a suffix like in `YmlFileDumper`.
---------------------------------------------------------------------------
by fabpot at 2011/09/05 23:41:12 -0700
A general note on PHPDoc: The first line of a phpdoc ends with a dot and starts with a present verb like in `Extracts translation messages from template files.`
---------------------------------------------------------------------------
by michelsalib at 2011/09/06 01:23:31 -0700
I fixed most of the remarks. I just need to go through the phpdoc (in a few minutes).
---------------------------------------------------------------------------
by stloyd at 2011/09/06 01:28:55 -0700
@michelsalib you should use `git rebase` (see [docs](http://symfony.com/doc/current/contributing/code/patches.html#id1)) instead of `git merge`.
---------------------------------------------------------------------------
by michelsalib at 2011/09/06 01:31:06 -0700
@stloyd sorry. I will rebase (squash) everything when I am finished.
---------------------------------------------------------------------------
by kaiwa at 2011/09/06 01:31:18 -0700
Hey, it might be a little bit late, but may i ask a not code-related question?
Is it correct that the `--force` option only means to write the output to a file instead of writing it to stdout?
If so,
1. is it semantically correct? I mean... i'm not a native english speaker, but from unix programs i'm used to interpret a `--force` option as something like "overwrite", "ignore errors" or "suppress warnings". An option which is used in case of trouble most time. Feels confusing to me to be forced ;-) to use the `--force` option for simply writing to files.
2. does it makes sense to have a default behaviour instead of requiring the user to give either `--force` or `--dump-messages`? In which cases does the user wants to dump the messages to the console? Is it only for debugging / to review the messages?
---------------------------------------------------------------------------
by michelsalib at 2011/09/06 01:33:58 -0700
@kaiwa Your concerns seems perfectly right. Initially I just wanted to mimic the `doctrine:schema:update` command. But it can be changed.
@fabpot what do you think ?
---------------------------------------------------------------------------
by michelsalib at 2011/09/06 02:01:22 -0700
@stloyd I tried to do a `git rebase` and I am quite lost. I think I messed something when merge instead of rebase. How can I clean/squash my PR properly ?
---------------------------------------------------------------------------
by stloyd at 2011/09/06 02:11:29 -0700
@michelsalib for now just work as it is ;-) When I back from work I will try to help you to rebase it properly.
---------------------------------------------------------------------------
by michelsalib at 2011/09/06 02:12:17 -0700
@stloyd Thank you !
---------------------------------------------------------------------------
by stloyd at 2011/09/06 12:39:18 -0700
@michelsalib I was trying to rebase your code and revert it this _bad_ commit (merge), but without success.
IMO best way would be making _brand new_ branch based on symfony/master, and the `git cherry-pick` ([hint](http://ariejan.net/2010/06/10/cherry-picking-specific-commits-from-another-branch)) commits you need (almost all in this PR but without this one with merge), then you will need to apply again _by hand_ changes from commit fce24c7fa2 (this clean up you have done there).
I'm sorry I can't give you more help...
---------------------------------------------------------------------------
by michelsalib at 2011/09/07 09:41:36 -0700
@stloyd I finally succeed to fix this PR. Thanks to your help! I must admit the whole thing with `cherry-pick` command was quite epic.
@fabpot Don't be sorry, I am about to fix the PHPDoc. I shall squash right after.
---------------------------------------------------------------------------
by michelsalib at 2011/09/07 10:07:20 -0700
I just squashed and did a last polish of the code. You might want to read it again before merge.
---------------------------------------------------------------------------
by fabpot at 2011/09/07 11:40:48 -0700
ok, code looks really good now. I think the only missing thing is some more unit tests (for the extractors for instance).
---------------------------------------------------------------------------
by michelsalib at 2011/09/07 12:18:59 -0700
Thanks, I'll look into it tomorrow.
---------------------------------------------------------------------------
by michelsalib at 2011/09/08 15:13:10 -0700
Hi,
I just added unit tests for both extractors (php and yaml).
Concerning the yaml extractor, the test is quite tricky because I need to mock the twig environment while returning a twig tree that contain a trans block and a trans filter. But it work fine with as little side effects as possible. Also I am not sure that the test should be in the TwigBundle.
IMO, the PR seems ready.
---------------------------------------------------------------------------
by stof at 2011/09/08 15:34:41 -0700
As the extractor is in bridge, the tests should be in the folder containing the tests for the bridge in tests/
---------------------------------------------------------------------------
by michelsalib at 2011/09/08 15:41:45 -0700
Thanks @stof, it is now fixed.
---------------------------------------------------------------------------
by michelsalib at 2011/09/09 00:48:47 -0700
thanks @stloyd, it is fixed now.
---------------------------------------------------------------------------
by michelsalib at 2011/09/09 01:25:24 -0700
Fixed again ;)
-- add missing files
-- tweak translation command files
-- dumpers are now responsive for writting the files
-- moved the twig extractor the bridge
-- clear temp files after unit tests
-- check the presence of dumper in translation writer
-- General cleaning of the code
-- clean phpDoc
-- fix PHPDoc
-- fixing class name in configuration
-- add unit tests for extractors (php and twig)
-- moved test to correct location
-- polish the code
-- polish the code
Commits
-------
b7c2a2e [FrameworkBundle] fixed typo in German validator translation
Discussion
----------
[FrameworkBundle] fixed typo in German validator translation
Commits
-------
8198078 updated italian translation for validators
Discussion
----------
updated italian translation in validators.it.xliff
---------------------------------------------------------------------------
by stof at 2011/09/06 14:06:39 -0700
could you do the update up-to the id 41 against the 2.0 branch (as they should be there) ? Only 42 and up are new for 2.1
---------------------------------------------------------------------------
by stloyd at 2011/09/06 14:12:28 -0700
@micheleorselli, as @stof mentioned you should make two PRs. One against __2.0__ to fix translation issues, and second against __master__ to add those new ones, available only in __2.1+__
---------------------------------------------------------------------------
by micheleorselli at 2011/09/06 15:04:17 -0700
@stof @stloyd: translation for 2.0 branch are updated in PR #2118
* 2.0:
[Validator] Sync polish translation
[FrameworkBundle] sanitize target arg in asset:install command
few optimisations for XliffFileLoader and XmlFileLoader
[FrameworkBundle] Sync the Russian translations
[FrameworkBundle] Added Dutch validator translation for trans-unit 41
[FrameworkBundle] Updated German validator translation
[FrameworkBundle] Fixed a typo in the translation file per @PeymanHR
Commits
-------
34494b3 whitespace fixes
1a86a4a Refactor mime-type to file extension guessing
e7481a3 Decouple mime-type to extension logic from File class
Discussion
----------
[2.1] Decouple mimetype-to-extension logic from File class
This allows guessing the extension from a given mime type
without requiring the existence of a local file.
If a file's meta information (mime-type, etc.) is already available (i.e. it's
been extracted once and stored in some persistent data store), it would be
nice to be able to make a best-guess on the extension based on the known mime-type.
A concrete use case of this is for the symfony-cmf, where a file has been stored
in the jackrabbit data store. When delivering this file or saving it to disk, we'd like to
use an extension that's created based on the known mime type of the file.
---------------------------------------------------------------------------
by brki at 2011/06/21 04:35:13 -0700
Now implemented similarly to the existing MimeTypeGuesser.
---------------------------------------------------------------------------
by brki at 2011/06/21 07:51:22 -0700
whitespace removed
---------------------------------------------------------------------------
by stof at 2011/09/04 05:04:54 -0700
@fabpot @brki what is the status of this PR ?
Commits
-------
89f4791 Fixed CS
a348efe Removed trailing whitespaces
135531a Replaced setExpectedException() methods by annotations
6ad83e7 Updated according to PR review
2de243c * Added Fixtures 2x2px test.gif image file * Updated tests for using fixture image instead of imagecreate function
a5a2dfa [ImageValidator] Added dedicated ImageValidator class with min width, max width, min height and max height validations
Discussion
----------
[2.1] [ImageValidator] Improved ImageValidator + tests
Added dedicated ImageValidator class with min width, max width, min height and max height validations
---------------------------------------------------------------------------
by kriswallsmith at 2011/06/04 10:32:56 -0700
This is nice but doesn't belong in the core as it adds a dependency on GD.
---------------------------------------------------------------------------
by benjamindulau at 2011/06/04 10:41:02 -0700
@kriswallsmith i'm not sure, php.net doc says :
Note:
This function does not require the GD image library.
Well, that could also be a mistake :)
---------------------------------------------------------------------------
by pborreli at 2011/06/04 11:17:25 -0700
i think @benjamindulau is right
```
$ php -m
[PHP Modules]
bcmath
calendar
com_dotnet
Core
ctype
date
dom
ereg
filter
ftp
hash
iconv
json
libxml
mcrypt
mhash
mysqlnd
odbc
pcre
PDO
Phar
Reflection
session
SimpleXML
SPL
sqlite3
standard
tokenizer
wddx
xdebug
xml
xmlreader
xmlwriter
zip
zlib
```
```
$ php -r "var_dump(getimagesize(__DIR__.'/src/Symfony/Bundle/FrameworkBund
le/Resources/public/images/blue_picto_less.gif'));"
array(7) {
[0]=>
int(18)
[1]=>
int(18)
[2]=>
int(1)
[3]=>
string(22) "width="18" height="18""
["bits"]=>
int(5)
["channels"]=>
int(3)
["mime"]=>
string(9) "image/gif"
}
```
---------------------------------------------------------------------------
by benjamindulau at 2011/06/04 11:22:57 -0700
However, the tests use imagejpeg and imagedestroy, that's not valid.
But, i'm not sure how to correctly test the validator then.
---------------------------------------------------------------------------
by benjamindulau at 2011/06/04 11:31:51 -0700
Can i use an image from FrameworkBundle resources ?
---------------------------------------------------------------------------
by pborreli at 2011/06/06 03:47:24 -0700
just add your own image inside Fixture folder (smaller the better, 1x1, 1x2 ..)
---------------------------------------------------------------------------
by stof at 2011/09/04 05:27:42 -0700
@benjamindulau could you rebase your PR and update it according to the comments ?
---------------------------------------------------------------------------
by benjamindulau at 2011/09/04 06:31:34 -0700
Yep, i think i can find some time today to do that
---------------------------------------------------------------------------
by benjamindulau at 2011/09/04 09:27:29 -0700
I've updated the PR.
A big thank to Stof who helped me with my git mess ;-)
Commits
-------
dea46c7 [Translation] support CsvDumper [Translation] support CsvDumper [Translation] support CsvDumper [Translation] support CsvDumper [Translation] support CsvDumper
Discussion
----------
[2.1][Translation] support CsvDumper
---------------------------------------------------------------------------
by michelsalib at 2011/09/05 04:56:47 -0700
The DumperInterface is probably going to change soon, it should be considered when merging : #2051.
Commits
-------
f2761dd Fix typo and include suggestion by Stof
4ac380e Adjust QtTranslations patch and include QtTranslationsDumper + test aswell
5712798 Adjust QtTranslationLoader to throw RuntimeException
21b29c2 Merge symfony/master
6bf43a1 [Translation] Add .ts as file extension to search for Qt Translation files.
5808715 [Translation] Add Qt Translation component with tests
Discussion
----------
[2.1] Qt translations
Add support for QT translations - it has a GUI and www.crowdin.net supports it.
---------------------------------------------------------------------------
by stof at 2011/05/30 07:24:48 -0700
You also need to register it in FrameworkBundle.
---------------------------------------------------------------------------
by tristanbes at 2011/06/26 12:08:47 -0700
crowdin seems to be a cool service. didn't know about this website. thanks
---------------------------------------------------------------------------
by fabpot at 2011/07/11 02:58:31 -0700
Just for the record: I've just removed all usage of `\Exception` in Symfony2 master:
6a7359389d
---------------------------------------------------------------------------
by beberlei at 2011/07/11 03:02:14 -0700
I will adjust the PR.
---------------------------------------------------------------------------
by tristanbes at 2011/08/28 11:10:02 -0700
Any news @beberlei ?
---------------------------------------------------------------------------
by beberlei at 2011/08/29 01:07:28 -0700
Yes, i have to allocate some time. Havent managed to do so yet.
---------------------------------------------------------------------------
by stof at 2011/09/04 05:28:16 -0700
@beberlei the PR also need to be rebased as it conflicts with master.
Commits
-------
41b7a19 Updated the tests so that tests will be marked as skipped when there is no MongoDB server present!
233c7db Updated the code to follow the symfony coding standards
7b24de5 Updated the code to follow the symfony coding standard using stof his remarks
fbcbdde - Fixed a small bug - Updated some phpdoc
00fdfec Added a MongoDbProfilerStorage engine
Discussion
----------
[2.1] [HttpKernel] MongoDb storage for Profiler
As a documentbased database like MongoDB is [supposedly fantastic in logging](http://blog.mongodb.org/post/172254834/mongodb-is-fantastic-for-logging) I implemented a storage engine for the profiler that should enable us to use this database as storage for this.
Activate it using this way:
framework:
profiler:
dsn: mongodb://user:pass@location/database/collection
---------------------------------------------------------------------------
by stof at 2011/07/24 11:23:06 -0700
btw, the MongoDB session storage has already be rejected from the core so this should probably be moved to the DoctrineMongoDBBundle (even if it uses only the PHP extension and not Doctrine). @fabpot thoughts about this ?
---------------------------------------------------------------------------
by Wotre at 2011/07/24 11:52:56 -0700
Just my personal opinion, if it is prefered that way I will move this into the DoctrineMongoDBBundle.
While it is reasonable to bundle all Mongo related things together, I do believe that in the case of logging we want to avoid as many depencies as possible. Some exceptions can occur pretty early inside the framework, and it would be a shame if those aren't logged because this layer is written on top of doctrine. I'm not exactly familliar enough with the symfony internals as I only started using it a few days ago, but I can imagine that this can make a difference with some exceptions.
---------------------------------------------------------------------------
by stof at 2011/07/24 11:59:10 -0700
I don't ask you to use Doctrine in this code. It is fine to use the extension directly if it is enough.
Btw, the profiler is *not* used early. :)
---------------------------------------------------------------------------
by Wotre at 2011/07/26 10:45:05 -0700
So... Any final remark whether this should be moved to [the DoctrineMongoDBBundle](https://github.com/symfony/DoctrineMongoDBBundle) or not?
If it has to be moved, any comment on where in that bundle this should be put?
Also, if it has to be moved, how can we arrange the configuration using DI? Currently I've put a line in the FrameworkExtension file to use this engine for everything with a $dsn starting with mongodb; I imagine this kind of ugly depency can't really exist between the FrameworkBundle and another one.
Although it seems completely illogical to me, I will move it, but I do need some directions on how to elegantly do this...
---------------------------------------------------------------------------
by stof at 2011/07/26 11:03:04 -0700
@fabpot what do you think ?
---------------------------------------------------------------------------
by stof at 2011/09/04 01:28:48 -0700
@fabpot what do you think about the place where this should be done ?
Commits
-------
9f0bd03 [HttpKernel] Update tests for FileProfilerStorage
b7032bc [HttpKernel] Update FileProfileStorage to search from EOF
188a5fa [HttpKernel] Override the existing tokens in FileProfilerStorage
b1b1424 [HttpKernel] Delete folders in the profiler cache
88bc3ec [HttpKernel] Fixes standards of FileProfilerStorage
affe66c Merge remote-tracking branch 'origin/master' into new-profiler-storage
ea916c3 [HttpKernel] Coding convention for the file profiler storage
9ae2c8d [HttpKernel] CS in file storage
b415efd [HttpKernel] Add a test for semicolon in file storage test
1c1215f [HttpKernel] Use subfolders for better storage in file storage of profiler
4b1dc1f [HttpKernel] Fix the folder attribute of file storage to private
70f73e1 [HttpKernel] Fix tests for the file storage of profiler
d5313d9 [HttpKernel] Add tests for the file profiler storage
09fc0a2 [HttpKernel] Add Symfony credits to the file storage class for the profiler
d1d5892 [HttpKernel] Finalize the file storage for the profiler
2f65cf2 Add POC for file storage system
Discussion
----------
[2.1] [HttpKernel] File storage for profiler
Symfony2 has some problems when dealing with multiple concurrency queries in the SQLite storage, resulting in a timeout error or terrible lack.
I've implemented after discussions with @fabpot a filesystem storage.
Enable it in your project with :
framework:
profiler:
dsn: "file:%kernel.cache_dir%/profiler"
I also studied the possibility to store only big data string in files and rest in the SQLite, but not concluant.
Results of my measures (4 concurrency, 120 total) :
* SQLite with data : 1057ms
* SQLite without data : 615ms
* MySQL : 40ms
* This File storage : 54ms
---------------------------------------------------------------------------
by alexandresalome at 2011/07/22 12:01:10 -0700
An idea for the find method : a csv file containing ip;url;token
The iteration could be done over a big file, without loading the whole file in memory.
---------------------------------------------------------------------------
by alexandresalome at 2011/07/23 14:22:32 -0700
OK new version, with as explained previously : a CSV file containing the index + file for each profile.
The speed is similar to the speed of MySQL, and no memory overhead should occur with this solution.
---------------------------------------------------------------------------
by alexandresalome at 2011/07/23 14:37:14 -0700
Hm... Created tests, duplicated from SqliteProfilerStorageTest.
Any idea on how to put this code in common ? Is it usual to create a base class for 2 tests ?
---------------------------------------------------------------------------
by alexandresalome at 2011/07/23 14:48:39 -0700
Just tested with 24.000 requests, the 24.001'th request still takes less than 50ms to execute.
The index file is about 2Mb, and iterating the whole file is fast.
---------------------------------------------------------------------------
by alexandresalome at 2011/07/23 14:53:19 -0700
I've filled the file with 120Mb of data, requests are still less than 50ms for executing.
Iterating the index takes more than 30s (so it crashed), but it's because of the amount of lines. 30 seconds = 1,400,000 lines in this computer. The file = 1,500,000 lines
---------------------------------------------------------------------------
by alexandresalome at 2011/07/23 14:56:54 -0700
I've tested it with Linux, is someone can test with Windows
---------------------------------------------------------------------------
by stloyd at 2011/07/24 00:32:32 -0700
IMO to speedup it a bit more and not end up with "crash" (to not end with "limit" of files per directory, also to many files in dir slow down every OS) you should use same method to write as Twig, split up files in to directories. If you do this you can speed up index more, because you can create smaller one per directory.
Also you should fix CS (coding standards).
---------------------------------------------------------------------------
by stloyd at 2011/07/24 02:10:20 -0700
Tested on Win 7, seems ok. Similar speed to sqlite, dunno why ;-) but used a bit less of memory.
---------------------------------------------------------------------------
by alexandresalome at 2011/07/24 02:13:21 -0700
Did you tried with concurrent requests ? It makes sense when you use assetic
and your browser hits the application 4 times simultaneously for CSS
generation
---------------------------------------------------------------------------
by alexandresalome at 2011/07/24 02:17:23 -0700
I used Apache Benchmark for producing results :
ab -c 4 -n 120 URL
---------------------------------------------------------------------------
by alexandresalome at 2011/07/24 02:56:55 -0700
OK I used subfolders, based on last characters (because the first part of token is mostly the same between queries.
---------------------------------------------------------------------------
by stof at 2011/09/04 01:27:15 -0700
@fabpot any news about it ? Can it be merged ?
Commits
-------
8d50c16 few optimisations for XliffFileLoader and XmlFileLoader
Discussion
----------
few optimisations for XliffFileLoader and XmlFileLoader
- file_put_contents + file_get_contents -> copy
- use stripos insteed preg_match
- removed useless `$tmpfiles` in XliffFileLoader
Commits
-------
3a7e038 [FrameworkBundle] sanitize target arg in asset:install command
Discussion
----------
[FrameworkBundle] sanitize target arg in asset:install command
`php app/console assets:install web/`
(removed tailing /)
before
`Installing assets for Symfony\Bundle\FrameworkBundle into web//bundles/framework`
after
`Installing assets for Symfony\Bundle\FrameworkBundle into web/bundles/framework`
Commits
-------
d19f1d7 [Doctrine] Fix UniqueEntityValidator reporting a false positive by ignoring multiple query results
Discussion
----------
[Doctrine] Fix UniqueEntityValidator reporting a false positive by ignoring multiple query results
An entity should only be considered unique if its search criteria returns no matches or a single, identical entity. Multiple results indicates that conflicting entities exist.
Note: the DoctrineMongoDBBundle's unique validator checks identifier values if the object strict-equality check is false. This may be a worthwhile improvement, as it would prevent reporting a validation error for an enttiy which is going to overwrite its conflicting counter-part in the database.
---------------------------------------------------------------------------
by jmikola at 2011/09/01 14:23:27 -0700
This is the Doctrine bridge equivalent for my fix to DoctrineMongoDBBundle: https://github.com/symfony/DoctrineMongoDBBundle/pull/42
---------------------------------------------------------------------------
by fabpot at 2011/09/02 00:13:52 -0700
As this is a bug fix, can you base your PR on the symfony/2.0 branch? Thanks.
* EvanK-patch-1:
Per the [documentation][1], the `NotBlank` constraint should be using the `empty` language construct, otherwise it will not trigger on, for example, a boolean false from an unchecked checkbox field.
An entity should only be considered unique if its search criteria returns no matches or a single, identical entity. Multiple results indicates that conflicting entities exist.
Commits
-------
6bd1749 Fixed a bug when multiple expanded choices would render unchecked because of the Form Framework's strict type checking.
Discussion
----------
[DoctrineBridge] Entities to array transformer
Fixed a bug when multiple expanded choices would render unchecked because of the Form Framework's strict type checking.
---------------------------------------------------------------------------
by fabpot at 2011/08/31 09:01:47 -0700
Looks good to me. Can you squash your commits before I merge? Thanks.
Check that $mode in InputArgument::__construct() is not below 1 to actually look if any of the flags are set.
Check that $mode in InputOption::__construct() is not below 1 to actually look if any of the flags are set.
Check for the correct parameter type, as in InputOption (integer).
InputArgumentTest: Added test for negative integer $mode parameter input in constructor.
InputOptionTest: Added test for negative integer $mode parameter input in constructor.
Commits
-------
eb8f3cb added uniqueEntity message translation (fr)
df9f223 added missing french translations
f4c133e removed trailing dot to make it consistent with other validator messages
Discussion
----------
[Translation] Unique Entity message
I've added the translation of uniqueEntity validation message, I've used ``trans-unit id="41"`` which seems to be unused
Doctrine caches annotations. For methods, it uses PHP reflection and the getDeclaringClass() to create
a unique cache key. Unfortunately, if you have 2 classes that extend another one, the cache will be shared.
It's not a problem except that before this patch, the default route name was also cached (as the cache is serialized
after we changed the object). So, all other classes inherited this default route name. The fix is quite easy:
just don't change the read annotation object.
From the PHP CHANGELOG:
The flag ENT_SUBSTITUTE makes invalid multibyte sequences be replaced by
U+FFFD (UTF-8) or &#FFFD; by htmlspecialchars and htmlentities. It is an
alternative to the default behavior, which just returns an empty string and to
ENT_IGNORE, which is a security risk. The behavior follows the recommendations
of Unicode Technical Report #36.
Commits
-------
ae3b128 [ClassLoader] Support for autoloading include_path incl. tests.
Discussion
----------
Autoload
GH Issue #1823
---------------------------------------------------------------------------
by stof at 2011/07/29 00:42:10 -0700
note that another fix was proposed in #1852 but this implementation is cleaner IMO
---------------------------------------------------------------------------
by henrikbjorn at 2011/08/12 01:57:45 -0700
@fabpot @stof any suggestions? need this kind of badly
---------------------------------------------------------------------------
by stof at 2011/08/12 02:06:54 -0700
for me it is fine. I guess you need to wait the end of @fabpot's holydays to see it merged.
---------------------------------------------------------------------------
by henrikbjorn at 2011/08/25 02:24:13 -0700
Added tests in the hope it will make it in soon :)
---------------------------------------------------------------------------
by henrikbjorn at 2011/08/29 03:31:08 -0700
Any other requests / suggestions ?
---------------------------------------------------------------------------
by stof at 2011/08/29 03:36:15 -0700
could you rebase the PR ? Github says that it conflicts.
---------------------------------------------------------------------------
by henrikbjorn at 2011/08/29 04:11:43 -0700
Should be rebased now or that is what git cli says :)
---------------------------------------------------------------------------
by henrikbjorn at 2011/08/29 04:16:28 -0700
And squashed.
Commits
-------
cc098a3 [HttpKernel] Add support for xdebug.file_link_format to Debug\ExceptionHandler.php
Discussion
----------
[HttpKernel] Add support for xdebug.file_link_format to Debug\ExceptionHandler
Format file and line as url, if xdebug.file_link_format is set. Inspired by #1893
Commits
-------
020fa51 [RedirectResponse] Added missing `doctype` and `title` tag
Discussion
----------
[RedirectResponse] Added missing `doctype` and `title` tag
Commits
-------
ea0db2d [HttpFoundation] Remove useless ContentTypeMimeTypeGuesser
Discussion
----------
[2.1] [HttpFoundation] Remove useless ContentTypeMimeTypeGuesser
`mime_content_type` exists just for the compat between the old PHP 5.2
`mime_magic` extension and `file_info` extension
---------------------------------------------------------------------------
by fabpot at 2011/08/19 05:31:25 -0700
I will merge it in 2.1 as some people might rely on it.
---------------------------------------------------------------------------
by stealth35 at 2011/08/19 05:46:02 -0700
ok in the meantime, we can invert the guesser checker :
```php
/**
* Registers all natively provided mime type guessers
*/
private function __construct()
{
if (FileBinaryMimeTypeGuesser::isSupported()) {
$this->register(new FileBinaryMimeTypeGuesser());
}
if (FileinfoMimeTypeGuesser::isSupported()) {
$this->register(new FileinfoMimeTypeGuesser());
}
if (ContentTypeMimeTypeGuesser::isSupported()) {
$this->register(new ContentTypeMimeTypeGuesser());
}
}
```
---------------------------------------------------------------------------
by stloyd at 2011/08/19 05:48:38 -0700
@stealth35 You should make new PR for change you mentioned above.
---------------------------------------------------------------------------
by stealth35 at 2011/08/19 05:53:12 -0700
@stloyd done PR #1989
EDIT : forget this
Commits
-------
007e395 do not set a default CONTENT_TYPE for PATCH
fa2c027 Added support for the PATCH method
Discussion
----------
[2.1] [HttpFoundation] Added support for the PATCH method
http://tools.ietf.org/html/rfc2068#section-19.6.1.1http://tools.ietf.org/html/rfc5789
---------------------------------------------------------------------------
by Seldaek at 2011/08/07 03:23:20 -0700
According to the spec it seems that PATCH requests shouldn't be of application/x-www-form-urlencoded content-type so it shouldn't match the first if, and in the second it's probably wrong to default to application/x-www-form-urlencoded, no?
---------------------------------------------------------------------------
by lsmith77 at 2011/08/07 03:31:48 -0700
Hmm you are right. I assumed the diff would be encoded as ``application/x-www-form-urlencoded`` but there indeed is no indication of that in the spec. But given that the second case would still need some sort of handling for PATCH, just not sure what exactly ``$defaults['CONTENT_TYPE']`` should be set to.
---------------------------------------------------------------------------
by Seldaek at 2011/08/07 03:48:53 -0700
As I understand it, a PATCH request must specify a content-type or it's invalid, so we could just skip the second behavior if no content-type is present.
As your first link says:
The list of differences is in a format defined by the media type of the entity (e.g.,
"application/diff") and MUST include sufficient information to allow
the server to recreate the changes necessary to convert the original
version of the resource to the desired version.
Sounds like PATCH is highly application specific, and not so standardized, probably because it's not very useful for most purposes.
---------------------------------------------------------------------------
by lsmith77 at 2011/08/07 04:02:43 -0700
Yes, but to me this means that the patch is actually correct aside from the fact that its setting a default Content-Type, which I just corrected (not sure if this use of switch is ok with our coding style). Now if the Content-Type does end up being ``application/x-www-form-urlencoded`` then I would say its correct to decode it.
Commits
-------
24bacdc Ignore VCS files in assets:install command (closes#2025)
Discussion
----------
Ignore VCS files in assets:install command (closes#2025)
---------------------------------------------------------------------------
by stloyd at 2011/08/25 06:10:22 -0700
`ignoreVCS` is set to `true` by default, AFAIK also `getIterator()` is not needed.
---------------------------------------------------------------------------
by jalliot at 2011/08/25 06:30:32 -0700
@stloyd I knew about ``ignoreVCS`` defaulting to ``true`` but I thought it made it clearer but you're right it's not really useful.
As for ``getIterator`` I thought the conversion couldn't be made automatically on a method call like here but apparently it works so I changed it.
Thanks.
---------------------------------------------------------------------------
by tiagojsag at 2011/08/25 08:41:02 -0700
This approach creates another problem: the already existing VCS files are deleted when the command is executed, which makes at least SVN throw errors.
---------------------------------------------------------------------------
by jalliot at 2011/08/25 08:50:55 -0700
@tiagojsag If you remove the call to ``remove`` on line 83, does everything work?
Because I'm not really sure we need to remove the entire dir first since ``mirror`` should be able to adapt itself.
BTW, wouldn't it be better if you didn't commit the ``web/bundles`` dir in your SVN and instead ask to call the ``assets:install`` command each time?
---------------------------------------------------------------------------
by stof at 2011/08/25 08:58:16 -0700
Great news about SVN: the incoming 1.7 version stops adding a ``.svn`` folder in every directory but uses a single one at the root of the project (like git does for instance), solving this sort of issues about copying files :)
@tiagojsag this command has always removed the old asset folders before copying the new ones, and there is not real mean to do otherwise by keeping things simple. You could consider ignoring the ``vendor/bundles`` folder in the SVN and running the command when doing checkout (thus allowing devs to use symlinks if they want)
---------------------------------------------------------------------------
by tiagojsag at 2011/08/25 09:01:39 -0700
yes, that was the solution I was using before submitting this bug report. I also agree that it's the simplest and fastest way to address this, provided that docs get updated, so that no one spends their time trying to figure out why files are not synced with their svn repo.
---------------------------------------------------------------------------
by jalliot at 2011/08/25 09:03:11 -0700
@stof That's really great to hear!
But still this PR should be merged to avoid legacy files from current versions of SVN or other VCS.
---------------------------------------------------------------------------
by stof at 2011/08/25 09:04:31 -0700
@jalliot sure. My comment was mainly about the opposite issue raised by @tiagojsag
Since the key was previously concatenating service ID and method without a separator, it's possible that two different listeners could conflict (e.g. service/method pairs: foo/bar and fo/obar).
Commits
-------
89f477e [WebProfilerBundle] Throw exception if a collector template isn't found
6ca72cf [WebProfilerBundle] Allow .html.twig in collector template names
Discussion
----------
WDT debugging
While implementing collectors I did a mistake in the template name and it never told me, so I was left wondering why my stuff didn't show up. Not so nice IMO. Also the first commit is to allow template names to be specified fully. I don't see why this shouldn't be allowed, since it is the way you specify templates everywhere else.
* domcrawler-disabled-fields:
[DomCrawler] fixed disabled fields in forms (they are available in the DOM, but their values are not submitted -- whereas before, they were simply removed from the DOM)
$node->hasAttribute('disabled') sf2 should not create disagreement between implementation and practice for a crawler. If sahi real browser can find an element that is disabled, then sf2 should too. https://github.com/Behat/Mink/pull/58#issuecomment-1712459
Commits
-------
e294211 [DomCrawler] Removed unused document property in Form
Discussion
----------
[DomCrawler] Removed unused document property in Form
Commits
-------
8a980bd $node->hasAttribute('disabled') sf2 should not create disagreement between implementation and practice for a crawler. If sahi real browser can find an element that is disabled, then sf2 should too. https://github.com/Behat/Mink/pull/58#issuecomment-1712459
Discussion
----------
$node->hasAttribute('disabled') sf2 should not create disagreement betwee
$node->hasAttribute('disabled') sf2 should not create disagreement between implementation and practice for a crawler. If sahi real browser can find an element that is disabled, then sf2 should too.
https://github.com/Behat/Mink/pull/58#issuecomment-1712459
---------------------------------------------------------------------------
by cordoval at 2011/08/09 20:34:56 -0700
@fabpot please let me know if this is going to be in sometime soon or not, just wondering why it is deviating ...
---------------------------------------------------------------------------
by fabpot at 2011/08/23 01:11:42 -0700
I have just checked in a browser and the Symfony2 implementation is actually the right one.
Try this in a browser:
<form action='#' method="post">
<input name="foo" disabled="disabled" value="foo" />
<input name="bar" value="bar" />
<input type="submit" />
</form>
<?php
print_r($_POST);
// output: Array ( [bar] => bar ) when the form is submitted
And here is the discussion about it in the HTML4 spec: http://www.w3.org/TR/html4/interact/forms.html#h-17.12:
"In this example, the INPUT element is disabled. Therefore, it cannot receive user input nor will its value be submitted with the form."
And the same is tru for HTML5: http://www.w3.org/TR/html5/association-of-controls-and-forms.html#constructing-form-data-set
---------------------------------------------------------------------------
by cordoval at 2011/08/23 01:29:53 -0700
@fabpot I guess you got my scenario wrong. I am not trying to submit any form. I am just happen to have a disabled box that is checked and I want to read with the DOM Crawler that is checked. Not to submit or anything but for the purposes of testing.
Please consider also that this request comes from asserting values using behat mink, mink is fully dependent on sf2 driver for when it is used except it is told to use a different driver like a real browser like sahi. When testing in chrome and firefox, the verification with the DOM is made that the disabled box is checked properly. Symfony2 DOM Crawler however misses that spot for that use.
Even in the case where Symfony2 DOM Crawler component would have been thought not for this purpose of testing, or further for this particular scenario it would be good to make it more reusable for this kind of scenario.
Just saying....
---------------------------------------------------------------------------
by fabpot at 2011/08/23 02:00:34 -0700
Indeed, I didn't get your issue right. So, basically, all fields should be in the form, but the disabled field values should not be submitted (that makes sense).
I've prepared a fix in this patch: e8852586073bc23d4a41f4cd9cbe0d17a2f0c76d which is in the symfony/domcrawler-disabled-fields branch for now as I don't know if we can make this change in 2.0 or if we need to put it in 2.1.
---------------------------------------------------------------------------
by cordoval at 2011/08/23 02:15:01 -0700
oh no I was hoping to enter the authors, you already did the fix :'(
Commits
-------
e9d2a67 CS
3a64b08 Search in others user providers when a user is not found in the first user provider and throws the right exception.
Discussion
----------
Chain user provider doesn't search in all user providers
I commit these changes because Chain user provider doesn't search in all user providers.
Example with the Acme/DemoBundle:
// security.yml
...
providers:
chain_provider:
providers: [in_memory, in_memory_extend]
in_memory_extend:
users:
admin2: { password: adminpass2, roles: [ 'ROLE_ADMIN' ] }
in_memory:
users:
user: { password: userpass, roles: [ 'ROLE_USER' ] }
...
firewalls:
...
secured_area:
pattern: ^/demo/secured/
provider: chain_provider OR in_memory_extend
...
We can see these logs :
security.INFO: User "admin2" has been authenticated successfully [] []
security.DEBUG: Write SecurityContext in the session [] []
security.DEBUG: Read SecurityContext from the session [] []
security.DEBUG: Reloading user from user provider. [] []
security.WARNING: Username "admin2" could not be found. [] []
The new code search in others user providers when a user is not found in the first user provider and throws the right exception.
---------------------------------------------------------------------------
by lsmith77 at 2011/08/14 12:20:04 -0700
I wonder if it should be a provider option to continue on a failed user lookup. I can see cases where you really dont want to iterate over all providers and others where you do.
---------------------------------------------------------------------------
by Abhoryo at 2011/08/14 17:27:16 -0700
If someone need a provider like you describe, he can create one.
Here we talk about a chain user provider.
Doc : [using-multiple-user-providers](http://symfony.com/doc/current/book/security.html#using-multiple-user-providers)
We can read in the doc: "The chain_provider will, in turn, try to load the user from both the in_memory and user_db providers."
But its not the case right now.
Commits
-------
c29fa9d [Form] Fix for treatment zero as empty data. Closes#1986
Discussion
----------
[Form] Fix for treatment zero as empty data. Closes#1986
For more info please read #1986.
Commits
-------
8d48cea [EventDispatcher] Change the license of EventDispatcher from LGPL to MIT
Discussion
----------
[EventDispatcher] Change the license of EventDispatcher from LGPL to MIT
It was previously agreed to re-license the Doctrine2 based
EventDispatcher refactoring to use the MIT license. However, the files
still retain the LGPL license notice.
This commit changes the license to MIT.
---------------------------------------------------------------------------
by fabpot at 2011/08/21 05:55:00 -0700
That's right but I would prefer that the PR comes from someone of the Doctrine core team like @beberlei or @jwage.
---------------------------------------------------------------------------
by fabpot at 2011/08/21 05:55:28 -0700
or at least, they can perhaps acknowledge this PR.
---------------------------------------------------------------------------
by beberlei at 2011/08/22 00:11:20 -0700
Acknowledged
It was previously agreed to re-license the Doctrine2 based
EventDispatcher refactoring to use the MIT license. However, the files
still retain the LGPL license notice.
This commit changes the license to MIT.
Commits
-------
d880db2 [Form] Test covered fix for invalid date (13 month/31.02.2011 etc.) send to transformer. Closes#1755df74f49 Patched src/Symfony/Component/Form/Extension/Core/DataTransformer/DateTimeToArrayTransformer.php to throw an exception when an invalid date is passed for transformation (e.g. 31st February)
Discussion
----------
[Form] Fix for "DateTimeToArrayTransformer" with invalid dates
Hey,
this is test covered fix from @mdavis1982 (closes#1755)
---------------------------------------------------------------------------
by stloyd at 2011/08/16 01:31:32 -0700
@fabpot Can we have this fix merged ?
Commits
-------
34a1b53 [HttpFoundation] Do not save session in Session::__destroy() when saved already
Discussion
----------
[HttpFoundation] Saving session data in __destroy() has a side effect on functional tests
Having functional test with several non-insulated requests, TestSessionListener invokes session saving at the end of every request. But instance of Session remains in memory until it's collected by garbage collector which saves the same data again in __destroy() method. The problem is that session object can get collected after other requests changed session data (e. g. user logged in) resulting in former data overwriting the latter.
Commits
-------
275da0d [Validator] changed 'self' to 'static' for child class to override pattern constant
Discussion
----------
[Validator] change 'self::' to 'static::' for PATTERN constant overridable in child classes
In TimeValidator and UrlValidator, PATTERN constant is not used with late static bind(static::) while DateValidator supports it.
Commits
-------
80d1718 [Fix] Email() constraints now guess as 'email' field type
Discussion
----------
[Fix] Email() constraints now guess as 'email' field type
I don't know what this was set to "text"
Commits
-------
e88ecbb [Form] Fixed a typo in AbstractType phpdoc
Discussion
----------
[Form] Fixed a typo in AbstractType phpdoc
This PR is a new version of PR #1862.
Original comment :
Hi,
Nothing really awesome, but I fixed a typo in some phpdoc of the AbstractType class.
Commits
-------
09c41d3 [Security] Fixed incorrect merge of two modifications (53f5c23c and 85199677) to AclVoter
Discussion
----------
[Security] Fixed incorrect merge of two modifications to AclVoter
It seems two modifications to `AclVoter` (53f5c23c and 85199677) have been merged incorrectly, leading to a method call on an object that is known to be `null` and a fatal error when running the tests
Commits
-------
ee5b9ce [SwiftmailerBundle] Allow non-file spools
Discussion
----------
[SwiftmailerBundle] Allow non-file spools
Actually if I have the following configuration:
swiftmailer:
spool:
type: not_file
path: some_path
The DIC compiler will complain:
'The service "swiftmailer.spool.file" has a dependency on
a non-existent parameter "swiftmailer.spool.file.path"
Because the file spool service is declared no matter the spool type configured.
And it requires the file.path, which is not available.
This patch aims to load the file spooler only if required by the
configuration.
---------------------------------------------------------------------------
by cystbear at 2011/07/29 16:36:04 -0700
Nice catch Thibault.
Commits
-------
86f888f fix https default port check
Discussion
----------
fix https default port check
---------------------------------------------------------------------------
by Abhoryo at 2011/08/03 03:26:15 -0700
I think it's better to delete $httpsPort variable from the prototype and use only $httpPort variable.
public function urlRedirectAction($path, $permanent = false, $scheme = null, $httpPort = 80)
...
$port = '';
if (('http' === $scheme && 80 != $httpPort) || ('https' === $scheme && 443 != $httpPort)) {
$port = ':'.$httpPort;
}
But if this method is already used with the $httpsPort variable elsewhere, your change is ok with me.
---------------------------------------------------------------------------
by gimler at 2011/08/03 04:52:08 -0700
You can use different ports for http and https so when you call the function $scheme = null than it use the $request->getScheme() so you must add both ports so i think it is not a good idea to merge the http and https vars.
---------------------------------------------------------------------------
by gimler at 2011/08/03 04:53:17 -0700
damn sorry i have accidentally close the pull request ;(
---------------------------------------------------------------------------
by stof at 2011/08/03 05:13:24 -0700
I agree with @gimler. Merging them as a single parameter does not make sense here
---------------------------------------------------------------------------
by Abhoryo at 2011/08/03 05:33:12 -0700
I've juste think it's weird to set a useless parameter ($httpPort) when you want to use the last parameter ($httpsPort).
And I don't think someone want http protocole on 433 or https on 80 ?
---------------------------------------------------------------------------
by stof at 2011/08/03 05:35:16 -0700
@Abhoryo what if you are using this controller in a general way, without knowing by advance if the handled request is a secure one ? You need both parameters.
If you need to change the https port by keeping the default http port, you indeed need to pass it but blame PHP: it does not support named parameters.
---------------------------------------------------------------------------
by Abhoryo at 2011/08/03 06:02:18 -0700
Ok, right.
Commits
-------
4f9d229 The trace argument value could be string ("*DEEP NESTED ARRAY*")
6e7439e expanded namespaces within phpdoc (special for PhpStorm)
f0a6ee5 merge from master
8519967 Calling supportsClass from vote to find out if we can vote
Discussion
----------
The trace argument of an exception can be string (*DEEP NESTED ARRAY*) but with an array type specified
It leads to the exception of a foreach loop:
Invalid argument supplied for foreach() /.../vendor/symfony/src/Symfony/Bundle/FrameworkBundle/Templating/Helper/CodeHelper.php:103
Commits
-------
ae55a98 Added $format in serialize() method, to keep consistence and give a hint to the normalizer.
Discussion
----------
Added $format in serialize() method, to keep consistence and give a hint
Added $format in serialize() method, to keep consistence and give a hint to the normalizer.
---------------------------------------------------------------------------
by Seldaek at 2011/08/12 02:06:19 -0700
👍
Commits
-------
81fb8e1 [DomCrawler] fix finding charset in addContent
Discussion
----------
[DomCrawler] fix finding charset in addContent
According to http://www.ietf.org/rfc/rfc2045.txt content type can include other field after charset. So they should be cut.
This allows as to define default like this
foo:
pattern: /{_locale}/login
defaults:
_controller: my_login_controller:loginAction
_locale: %session.default_locale%
Commits
-------
e78bc32 Fixed: Notice: Undefined index: enable_annotations in ...
Discussion
----------
Fixed: Notice: Undefined index: enable_annotations in ...
---------------------------------------------------------------------------
by stloyd at 2011/08/04 03:57:49 -0700
IMO `isset()` should be good enough here.
---------------------------------------------------------------------------
by stof at 2011/08/04 04:18:20 -0700
I don't see how such a notice could occur. There is a default value for this node so as soon as the validation node exists, there will be a value for this node. Could you give an example of configuration that causes the notice ?
---------------------------------------------------------------------------
by mvrhov at 2011/08/04 04:23:33 -0700
I don't have any validation node set up in my config in such a case it seems that sub nodes doesn't get build. So it seems that I found a bug in config builder.
---------------------------------------------------------------------------
by stof at 2011/08/04 04:54:05 -0700
This is in fact due to a hackish stuff in the DI extension. It changes the configuration after using the Config component to merge them in the case where you enable the forms without enabling the validator, to force enabling it.
Commits
-------
ba6a09d [DoctrineBundle] Adding a message in doctrine:generate:entities to notify people when a backup file is created
Discussion
----------
[DoctrineBundle] Adding a message in doctrine:generate:entities to notify
Hey guys!
This adds a message to the doctrine:generate:entities command when a backup file is created. This is because the backup file causes confusion in some cases (where did it come from?) and in rare cases - for reasons I don't know yet - the backup file causes "Cannot redeclare class ..." errors.
This is a not a BC-break, but of course could potentially cause an issue if there's some edge case that line 112 doesn't consider. For that reason, I'm pulling against master instead of 2.0.
Thanks!
Commits
-------
6738d2b [FrameworkBundle] Adding information about exactly which cache is being cleared.
Discussion
----------
[FrameworkBundle] Adding information about env being cleared
Hey guys!
I think the `cache:clear` confuses some people - they're expecting it to wipe out any and all cache (not just the cache for a specific env+debug mode). So, this adds details on *what* is being cleared, which should at least help.
I'll also put more information into the docs.
Thanks!
---------------------------------------------------------------------------
by jmikola at 2011/08/07 18:48:48 -0700
👍 on dumping the environment at output. Does the debug option mean anything in this context, though?
---------------------------------------------------------------------------
by weaverryan at 2011/08/07 19:57:18 -0700
I can't think of a spot where it makes a difference, but of course it *could* theoretically make a difference. The command's "help" message seems to indicate that it should be treated like there's a difference, so I followed suit.
But yes, environment is the big concern, hopefully nobody gets too hung up on the debug - one of those things where I think we technically need it, but practically don't.
---------------------------------------------------------------------------
by brikou at 2011/08/08 00:44:00 -0700
@weaverryan it would also be interesting to display the env used also for router:debug and probably other commands
---------------------------------------------------------------------------
by stof at 2011/08/08 01:13:58 -0700
@weaverryan It does make a difference when warùing up the cache (which is done by default when clearing it): many service definitions change according to the debug flag, so dumping the debug container seems a weird idea (the first request will need to dump the non-debug container as they have different names). But bigger issue: some cache warmer depend of the debug state (the Doctrine one IIRC) so the warming-up would do weird things if you run the prod CLI in debug mode.
Commits
-------
b8ee401 Making the english smoother in command help description
Discussion
----------
Making the english smoother in command help description
Pretty simple :)
Thanks!
Commits
-------
c0571fc [ClassLoader] Improve exception messages of the debug class loader
Discussion
----------
[ClassLoader] Improve exception messages of the debug class loader
---------------------------------------------------------------------------
by Seldaek at 2011/07/31 14:01:40 -0700
Ok, I updated this to just clarify the message, because when I got the issue after some serious copy-paste coding, I thought it was quite confusing - it seems to imply you mistyped the class name when using the class, and therefore it was not found, while the typo is in the class's file itself.
Actually if I have the following configuration:
swiftmailer:
spool:
type: not_file
path: some_path
The DIC compiler will complain:
'The service "swiftmailer.spool.file" has a dependency on
a non-existent parameter "swiftmailer.spool.file.path"
Because the file spool service is declared no matter the spool type configured.
And it requires the file.path, which is not available.
This patch aims to load the file spooler only if required by the
configuration.
Commits
-------
cf598de [FrameworkBundle] Updated the Chinese translations by @heccjj
e16ddcf [FrameworkBundle] Renamed validators.cn.xliff to validators.zh_CN.xliff
62da90a [FrameworkBundle] Fixed the Chinese translations by @heccjj
057cf2f Edited src/Symfony/Bundle/FrameworkBundle/Resources/translations/validators.cn.xliff via GitHub
Discussion
----------
[FrameworkBundle] Updated the Chinese translations
Commits
-------
7d5785e Fixed message 8
8ef3647 Updated danish validator translation
Discussion
----------
Updated Danish translation of validator strings
Just an updated Danish translation as requested.
---------------------------------------------------------------------------
by yethee at 2011/07/27 23:25:11 -0700
@cvaldemar, can you fix message #8?
Commits
-------
c558b78 avoid rendering the `ChoiceType` separator if all `choices` are `preferred_choices`
Discussion
----------
avoid rendering the `ChoiceType` separator if all `choices` are `preferred_choices`
---------------------------------------------------------------------------
by fabpot at 2011/07/24 00:51:21 -0700
The same change should be made to the PHP template.
---------------------------------------------------------------------------
by fabpot at 2011/07/25 00:31:39 -0700
I forgot to ask you to add some unit tests too. Thanks.
---------------------------------------------------------------------------
by craue at 2011/07/25 10:23:34 -0700
Are you asking for PHPUnit tests? If so, unfortunately, I'm not able to add those because I haven't used PHPUnit yet. ;)
---------------------------------------------------------------------------
by lenar at 2011/07/25 12:47:51 -0700
I would prefer ```choises | length``` without spaces as everywhere else.
---------------------------------------------------------------------------
by lenar at 2011/07/25 12:50:32 -0700
@fabpot: Since <option disabled> is unclickable in browser (by HTML spec) this really doesn't change anything (something not there is as unclickable) except the the look when rendered. I have hard time to imagine what could become unit-testable here by this change.
---------------------------------------------------------------------------
by stof at 2011/07/25 13:03:47 -0700
@lenar unit testing is not about what the browser could do. What should be unit-tested is that an example will only preferred choices does not output the separator, which is exactly what this PR is about
---------------------------------------------------------------------------
by stof at 2011/07/25 13:04:03 -0700
@lenar unit testing is not about what the browser could do. What should be unit-tested is that an example will only preferred choices does not output the separator, which is exactly what this PR is about
---------------------------------------------------------------------------
by lenar at 2011/07/25 13:08:33 -0700
@stof: ok, put this way you are definitely right.
---------------------------------------------------------------------------
by craue at 2011/07/25 13:37:50 -0700
@lenar: You're right about the spaces. I'm using them in my projects but will remove them here for the sake of consistency.
---------------------------------------------------------------------------
by stloyd at 2011/07/25 13:40:40 -0700
@craue I will write today/tomorrow test to cover your code and send you PR.
---------------------------------------------------------------------------
by craue at 2011/07/25 14:00:26 -0700
@stloyd: That would be nice. But I'm still not that familiar with Git(Hub). Is there anything I have to take care of?
Also, I'd like to squash my three commits into one ... if this is possible for an open PR and if I find out how to do that easily. :D
---------------------------------------------------------------------------
by fabpot at 2011/07/26 00:18:22 -0700
@craue: yes, you should squash your commits into one and use `--force` when you push (the PR will automatically be updated accordingly).
Commits
-------
85c0087 [TwigBridge] Made the locale configurable for the trans and transchoice tags
3ea31a0 [TwigBridge] Made the locale configurable for the trans and transchoice filters
Discussion
----------
Trans locale
This allows setting the locale when translating in a Twig template. This was already allowed in the Translator and in the PHP templates
Commits
-------
0832f4d Updated Persian translation
2a4fca8 translated validators resources into Persian
Discussion
----------
Persian translation
Added Persian validator translations
Commits
-------
be4b77d Updated Romanian translation
Discussion
----------
Updated Romanian translation
Updated Romanian translation for validation messages to match the latest messages.
Commits
-------
321dd45 Updated spanish and catalan translations
Discussion
----------
Updated spanish and catalan translations
Added new translations based on the indonesian updated file.
Commits
-------
9d8e6f6 [FrameworkBundle] Changed TraceableEventDispatcher to log calls to event listeners _before_ actually calling them
Discussion
----------
[FrameworkBundle] Log calls to event listereners _before_ calling them
The current implementation of `TraceableEventDispatcher` logs calls to event listeners _after_ actually calling them. This leads to strange logs when an event listener triggers another event.
For example, if I attach some `LoginListener` to the `security.interactive_login`-event, the log will look something like this:
<pre>
...
User "myusername" has been authenticated successfully
Notified event "security.interactive_login" to listener "MyVendor\MyBundle\EventListener\LoginListener::onSecurityInteractiveLogin".
Notified event "kernel.request" to listener "Symfony\Component\Security\Http\Firewall::onKernelRequest".
...
</pre>
From the logs it looks like the `kernel.request` event was fired after the user was authenticated, whereas it was actually the listener to `kernel.request` that caused the user to be authenticated.
By logging the call to the event listener _before_ calling it, the logs will look like this:
<pre>
...
Notified event "kernel.request" to listener "Symfony\Component\Security\Http\Firewall::onKernelRequest".
Notified event "security.interactive_login" to listener "MyVendor\MyBundle\EventListener\LoginListener::onSecurityInteractiveLogin".
User "myusername" has been authenticated successfully
...
</pre>
In my opinion this makes the causal relationship between the events clearer.
---------------------------------------------------------------------------
by stof at 2011/07/24 11:18:48 -0700
👍 for this.
Commits
-------
5219f81 Using the $status parameter instead of fixed value when creating a RedirectResponse.
Discussion
----------
Using the $status parameter instead of fixed value
I checked the usages and the optional `$status` parameter is never used, so maybe another option would be to remove the parameter completely...
---------------------------------------------------------------------------
by jaugustin at 2011/07/25 03:11:00 -0700
maybe you could test that $status is a valid redirect code
---------------------------------------------------------------------------
by stloyd at 2011/07/25 04:40:21 -0700
@jaugustin This check is already included in `RedirectResponse` class.
Commits
-------
266e60e Don't tell a lie to every WebServers
Discussion
----------
Please don't tell a lie to every WebServers
Fake Useragent name should be only in test case .
Commits
-------
03c7cfe UrlGenerator no longer appends '?' if query string is empty
Discussion
----------
UrlGenerator no longer appends '?' if query string is empty
If you generate a URL using null parameters (`array('foo' => null, 'bar' => null')`), `http_build_query` returns an empty string, resulting in a trailing `?` at the end of the generated URL.
This fixes that so that, if there are `$extra` params & `http_build_query` is empty, the URL is no longer appended.
---------------------------------------------------------------------------
by fabpot at 2011/07/22 10:15:26 -0700
Can you add unit tests?
---------------------------------------------------------------------------
by ericclemmons at 2011/07/22 10:52:21 -0700
Yes sir, will do.
-Eric Clemmons
Sent from my iPad Nano
On Jul 22, 2011, at 12:15 PM, fabpot<reply@reply.github.com> wrote:
> Can you add unit tests?
>
> --
> Reply to this email directly or view it on GitHub:
> https://github.com/symfony/symfony/pull/1773#issuecomment-1633515
---------------------------------------------------------------------------
by ericclemmons at 2011/07/22 11:55:30 -0700
**Added passing test.**
Currently `master` fails test:
```
1) Symfony\Tests\Component\Routing\Generator\UrlGeneratorTest::testUrlWithNullExtraParameters
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-http://localhost/app.php/testing
+http://localhost/app.php/testing?
//tests/Symfony/Tests/Component/Routing/Generator/UrlGeneratorTest.php:114
```
If not, as classes can be loaded during the boot, there is no way to be sure that
a class will not be already loaded by a third party bundle.
If the Kernel is already booted, we don't included the compiled classes.
Revert "[Form] CollectionType now checks for data_class parameter instead of only class."
This reverts commit 2e024f87a3.
Conflicts:
tests/Symfony/Tests/Component/Form/Extension/Core/Type/CollectionTypeTest.php
Revert "[Form] Added ObjectFactoryListener. Fixes #1746."
This reverts commit 0327beb0b9.
Conflicts:
tests/Symfony/Tests/Component/Form/Extension/Core/Type/CollectionTypeTest.php
Commits
-------
eae6a77 fixed wrong case
d0a175bfixes#1659f300ede fixes several bugs
a4f05ac added some tests
Discussion
----------
Http util fixes
Fixes several bugs in the http utils.
Please don't add anymore features without sufficient tests. Especially for the Security\Http namespace, regressions are very likely otherwise.
---------------------------------------------------------------------------
by fabpot at 2011/07/19 22:37:26 -0700
Tests do not pass for me:
There were 2 errors:
1) Symfony\Bundle\SecurityBundle\Tests\Functional\LocalizedRoutesAsPathTest::testLoginLogoutProcedure with data set #0 ('en')
InvalidArgumentException: The current node list is empty.
.../src/Symfony/Component/DomCrawler/Crawler.php:604
.../src/Symfony/Bundle/SecurityBundle/Tests/Functional/LocalizedRoutesAsPathTest.php:16
2) Symfony\Bundle\SecurityBundle\Tests\Functional\LocalizedRoutesAsPathTest::testLoginLogoutProcedure with data set #1 ('de')
InvalidArgumentException: The current node list is empty.
.../src/Symfony/Component/DomCrawler/Crawler.php:604
.../src/Symfony/Bundle/SecurityBundle/Tests/Functional/LocalizedRoutesAsPathTest.php:16
--
There were 4 failures:
1) Symfony\Bundle\SecurityBundle\Tests\Functional\LocalizedRoutesAsPathTest::testAccessRestrictedResource with data set #0 ('en')
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-http://localhost/en/login
+http://localhost/login
.../src/Symfony/Bundle/Securitybundle/Tests/Functional/WebTestCase.php:22
.../src/Symfony/Bundle/SecurityBundle/Tests/Functional/LocalizedRoutesAsPathTest.php:38
2) Symfony\Bundle\SecurityBundle\Tests\Functional\LocalizedRoutesAsPathTest::testAccessRestrictedResource with data set #1 ('de')
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-http://localhost/de/login
+http://localhost/login
.../src/Symfony/Bundle/Securitybundle/Tests/Functional/WebTestCase.php:22
.../src/Symfony/Bundle/SecurityBundle/Tests/Functional/LocalizedRoutesAsPathTest.php:38
3) Symfony\Bundle\SecurityBundle\Tests\Functional\LocalizedRoutesAsPathTest::testAccessRestrictedResourceWithForward with data set #0 ('en')
HTTP/1.0 302 Found
Cache-Control: no-cache
Content-Length: 299
Content-Type: text/html; charset=UTF-8
Date: Wed, 20 Jul 2011 05:36:27 GMT
Location: http://localhost/login
Set-Cookie: PHPSESSID=11c9c6a7e7620e13bddef223a5ba46d9; path=/; domain=
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<meta http-equiv="refresh" content="1;url=http://localhost/login" />
</head>
<body>
Redirecting to <a href="http://localhost/login">http://localhost/login</a>.
</body>
</html>
Failed asserting that <integer:0> matches expected <integer:1>.
.../src/Symfony/Bundle/SecurityBundle/Tests/Functional/LocalizedRoutesAsPathTest.php:50
4) Symfony\Bundle\SecurityBundle\Tests\Functional\LocalizedRoutesAsPathTest::testAccessRestrictedResourceWithForward with data set #1 ('de')
HTTP/1.0 302 Found
Cache-Control: no-cache
Content-Length: 299
Content-Type: text/html; charset=UTF-8
Date: Wed, 20 Jul 2011 05:36:28 GMT
Location: http://localhost/login
Set-Cookie: PHPSESSID=2bbe63786a088471ade3717917f4ba4f; path=/; domain=
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<meta http-equiv="refresh" content="1;url=http://localhost/login" />
</head>
<body>
Redirecting to <a href="http://localhost/login">http://localhost/login</a>.
</body>
</html>
Failed asserting that <integer:0> matches expected <integer:1>.
.../src/Symfony/Bundle/SecurityBundle/Tests/Functional/LocalizedRoutesAsPathTest.php:50
---------------------------------------------------------------------------
by schmittjoh at 2011/07/19 23:47:29 -0700
I fixed a wrong case, but I couldn't reproduce the other errors (tested on Ubuntu).
My guess is that the temporary directory on your machine couldn't be deleted for some reason, and the test runs with the configuration of some of the previous tests.
---------------------------------------------------------------------------
by fabpot at 2011/07/20 00:28:41 -0700
That does not make any difference for me. For instance, in `LocalizedRoutesAsPathTest::testLoginLogoutProcedure()`, the first request to `'/'.$locale.'/login'` returns the following Response:
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<meta http-equiv="refresh" content="1;url=http://localhost/login" />
</head>
<body>
Redirecting to <a href="http://localhost/login">http://localhost/login</a>.
</body>
</html>
---------------------------------------------------------------------------
by schmittjoh at 2011/07/20 00:31:34 -0700
That's weird, did you make sure that the temporary directory does not exist?
``rm -Rf /tmp/StandardFormLogin/``
On Wed, Jul 20, 2011 at 9:28 AM, fabpot <
reply@reply.github.com>wrote:
> That does not make any difference for me. For instance, in
> `LocalizedRoutesAsPathTest::testLoginLogoutProcedure()`, the first request
> to `'/'.$locale.'/login'` returns the following Response:
>
> <html>
> <head>
> <meta http-equiv="Content-Type" content="text/html;
> charset=utf-8" />
> <meta http-equiv="refresh" content="1;url=
> http://localhost/login" />
> </head>
> <body>
> Redirecting to <a href="http://localhost/login">
> http://localhost/login</a>.
> </body>
> </html>
>
> --
> Reply to this email directly or view it on GitHub:
> https://github.com/symfony/symfony/pull/1739#issuecomment-1613504
>
---------------------------------------------------------------------------
by fabpot at 2011/07/20 00:33:40 -0700
Yes, I've just checked and the directory does not exist.
---------------------------------------------------------------------------
by schmittjoh at 2011/07/20 00:39:55 -0700
Sorry, I can't reproduce it on Ubuntu and unless someone wants to sponsor me a Mac, there is not much I can do.
Commits
-------
2e024f8 [Form] CollectionType now checks for data_class parameter instead of only class.
0327beb [Form] Added ObjectFactoryListener. Fixes#1746.
Discussion
----------
[Form] Added ObjectFactoryListener. Fixes#1746.
---------------------------------------------------------------------------
by marcw at 2011/07/21 09:32:17 -0700
This patch also fixes a validation issue because it was impossible for the validator to validate an array.
---------------------------------------------------------------------------
by stof at 2011/07/21 09:47:46 -0700
yeah, using the data_class of the prototype would be great
This commit also fixes exception pages when Twig is not enabled as a templating engine.
Instead of just displaying the raw Twig template as before, we now fallback to the default
exception handler introduced some time ago.
when esi is enabled and internal uris are generated for esi-tags, an
attribute-array consisting entirely of null-values isn't handled correctly.
The reason is that php's `http_build_query()`-method outputs an empty string
for such arrays:
http_build_query(array('foo' => '')) == 'foo='
http_build_query(array('foo' => null)) == ''
In the latter case, the generation of an URI in `HttpKernel::generateInternalUri()`
generates an URI that could not be matched by the corresponding route (ex.
`_internal/Controller/.html` opposed to `_internal/Controller/none.html` which
should be expected).
This commit adds a possible solution as well as a simple test for this issue.
Commits
-------
9bcce9f fix tests
fc4787a fix non-extensible router
Discussion
----------
Router fix
Right now, the router is hard to overwrite (you need always a compiler pass). This commit fixes this.
---------------------------------------------------------------------------
by fabpot at 2011/07/18 01:15:36 -0700
Why do you need a complier pass to override the router?
---------------------------------------------------------------------------
by schmittjoh at 2011/07/18 01:47:47 -0700
How would you suggest to overwrite it?
Basically, I want to do something like this:
```yml
services:
router:
parent: router.default
class: MyClass
calls:
- [moreDeps, []]
```
---------------------------------------------------------------------------
by Seldaek at 2011/07/18 05:07:19 -0700
Then maybe we should somehow support redefining services with the same name while keeping the old one as parent, otherwise we need this foo.default for every service out there?
---------------------------------------------------------------------------
by fabpot at 2011/07/18 06:30:34 -0700
as @Seldeak said, why do that for the router and not all services?
---------------------------------------------------------------------------
by schmittjoh at 2011/07/18 06:38:39 -0700
I have designed the SecurityBundle this way where extension is encouraged.
---------------------------------------------------------------------------
by schmittjoh at 2011/07/18 11:15:57 -0700
I should add that this is mainly a problem for services where you still want to use the semantic configuration that is provided by the bundle. For services which are not configured by the extension, this is not so much of an issue.
Anyway, if you don't want to merge it, just close the PR. I have no problem with using a compiler pass.
---------------------------------------------------------------------------
by fabpot at 2011/07/18 11:55:11 -0700
We already have such a case with translator and translator.real. I will review the existing services to see where it makes sense to implement the same strategy.
---------------------------------------------------------------------------
by Seldaek at 2011/07/18 12:20:55 -0700
I guess you'd do it anyway, but we should pick a winner between .real and .default
---------------------------------------------------------------------------
by lsmith77 at 2011/07/18 12:26:52 -0700
I would prefer ".default" as ".real" always confused me.
Commits
-------
95011ce [HttpFoundation] Fixed creation of requests without a path.
Discussion
----------
[HttpFoundation] Fixed creation of requests without a path.
Providing urls with no path led to php warning that the index 'path' is
not set. This patch initializes 'path' if no path is set.
Commits
-------
8e169e4 Improved performance when assetic's use_controller is enabled
Discussion
----------
Improved performance when assetic's use_controller is enabled
When assetic's use_controller is enabled, assetic has to loop through all the templates and create TemplateReferences through the assetic FileResource. This in turn causes a lot of calls to getLogicalName() leading to 5x the calls to the TemplateReference's get() (16k in my case). By accessing the protected field directly compared to using get() we achieve better performance during development (33% in my case).
---------------------------------------------------------------------------
by beberlei at 2011/07/18 11:22:43 -0700
+1 - assetic is a huge performance drain for my app in dev mode aswell.
Commits
-------
5d17b92 [DoctrineBridge] Optimized the mapping drivers
Discussion
----------
[DoctrineBridge] Optimized the mapping drivers
This avoids loading all mappings when calling ``isTransient`` as it adds an overhead.
---------------------------------------------------------------------------
by beberlei at 2011/07/18 11:35:36 -0700
Its much better than the original one.
---------------------------------------------------------------------------
by stof at 2011/07/18 11:53:09 -0700
For the mapping defined in a specific class, this simply checks if the file exists (like Doctrine's implementation) but reusing the other method to keep the code DRY.
There is still an overhead when using a global mapping file but I don't see a way to avoid loading the file.
Commits
-------
872d308 [DoctrineBundle] Allowed to configure dql functions without using the multiple entity mangers syntax
Discussion
----------
[DoctrineBundle] Allowed to configure dql functions using the single EM syntax
[DoctrineBundle] Allowed to configure dql functions without using the multiple entity managers syntax
Commits
-------
b9bdab8 DoctrineAclCache unserialize sets the acl to the wrong field
Discussion
----------
DoctrineAclCache unserialize sets the acl to the wrong field
Upon unserialize of the acl, the acl is currently set to the id field which should be a string. Currently it passes the acl object into the id field which causes the following error upon unserialize.
Warning: Illegal offset type in isset or empty in Symfony/Component/Security/Acl/Dbal/AclProvider.php line 404
This is because at line 404, $ace->getId() returns an Acl object not an id and the acl field in $ace is null.
if (isset($this->loadedAces[$ace->getId()])) {
---------------------------------------------------------------------------
by fabpot at 2011/07/16 09:31:42 -0700
@schmittjoh?
---------------------------------------------------------------------------
by schmittjoh at 2011/07/16 23:19:06 -0700
Yes, this fix is good.
Commits
-------
d37ff15 removed unused code
2d3051f tabs -> spaces
2c224ce improves the exception message, and removes unnecessary constraint to only allow strings inside strings
d0b056c fixes a bug where getParameterBag() always returns null
Discussion
----------
Fixes a bug in PHPDumper, and in parameter resolving
Commits
-------
eb85cc5 fixes a bug where the cookie was wrongly considered expired
Discussion
----------
fixes a bug where the cookie was wrongly considered expired
On a related note, what do you think about adding some more functional tests here? Not only phpunit, but I would also suggest to add behat tests since there are a lot of things which are not picked up by the in process request emulation, but only by a real client.
@fabpot, @everzet, what do you think?
Commits
-------
5e80c68 fixes a naming inconsistency
8cfca15 added change to upgrade file
4123ec4 updated some missing references
Discussion
----------
Fix inconsistent naming
---------------------------------------------------------------------------
by jalliot at 2011/07/15 08:15:01 -0700
I think you forgot one commit (the one effectively changing Session and that you reverted in the main repo)
---------------------------------------------------------------------------
by schmittjoh at 2011/07/15 09:07:17 -0700
You're right, fixed now.
When assetic's use_controller is enabled, assetic has to loop through all the templates and create TemplateReferences through the assetic FileResource. This in turn causes a lot of calls to getLogicalName() leading to 5x the calls to the TemplateReference's get() (16k in my case). By accessing the protected field directly compared to using get() we achieve better performance during development (33% in my case).
Upon unserialize of the acl, the acl is currently set to the id field which should be a string. Currently it passes the acl object into the id field which causes the following error.
Warning: Illegal offset type in isset or empty in Symfony/Component/Security/Acl/Dbal/AclProvider.php line 404
Commits
-------
71cfb56 Thrown a \RuntimeException in RequestMatcher::checkIp6() if PHP is compiled with the option "disable-ipv6"
Discussion
----------
[HttpFoundation] Problem with RequestMatcher if PHP is compiled with the option "disable-ipv6"
Thrown a \RuntimeException in RequestMatcher::checkIp6() if PHP is compiled with the option "disable-ipv6".
Commits
-------
05cc24c [Yaml] Wrap numeric strings in quotes when dumping
Discussion
----------
[Yaml] Wrap numeric strings in quotes when dumping
This addresses an obscure case where a hash string (actually a commit-ish, "686e444") was dumped to YAML as an unquoted string value. It was later parsed from YAML as an exponential numeric and changed to ".Inf".
This commit should not change the existing behavior when dumping non-string numerics. It also doesn't appear to disturb any of the other test cases. I realize it's a huge edge case, so I'm open to discussion.
The alternative to this fix was an ugly `preg_replace()` to apply quoting around the commit-ish after dumping. I would look forward to removing that :)
This addresses an obscure case where a hash string (actually a commit-ish, "686e444") was dumped to YAML as an unquoted string value. It was later parsed from YAML as an exponential numeric and changed to ".Inf".
Commits
-------
29e4063 [Security] changed order of checks to check for more specific things first
Discussion
----------
[Security] changed order of checks
Commits
-------
61de80d [FrameworkBundle] Updated the Dutch validator translations for the changes in 95f7eedd63
Discussion
----------
[FrameworkBundle] Updated the Dutch validator translations
[FrameworkBundle] Updated the Dutch validator translations for the changes in 95f7eedd63
Commits
-------
64e9263 Updated UPDATE.md
7cf891a Renamed variable returned and used self in place of static for constants
f91f4dd Added the possibility to set cookies with the same name for different domains and paths for Symfony\Component\HttpFoundation\ResponseHeaderBag
f08eeb4 Moved managing cookies of HeaderBag in ResponseHeaderBag
Discussion
----------
[HttpFoundation] Cookies management in ResponseHeaderBag
Fixed cookies management in `Symfony\Component\HttpFoundation\HeaderBag` and `Symfony\Component\HttpFoundation\ResponseHeaderBag`
Commits
-------
26ff05bfixes#1538
Discussion
----------
fixes#1538
Constructor of Symfony\Component\Security\Acl\Domain\RoleSecurityIdentity
--------------------------------------------------------------------------------------------------------
currently it check if the argument is instance of Symfony\Component\Security\Core\Role\Role by
``if ($role instanceof Role)``
Maybe it should be changed to
``if ($role instanceof RoleInterface)``
Because if we use another Role class which implements RoleInterface
it dosen't work when we check access, it will throw a *NoAceFoundException* when vote
Commits
-------
66c698a Updated update.md
b33e1ba [SecurityBundle] Reported namespace change of EntityUserProvider
26e96c4 [DoctrineBridge] Changed namespace of EntityUserProvider[B
1633cb3 [Security] Moved EntityUserProvider to Doctrine Bridge
Discussion
----------
[2.1] Moved EntityUserProvider class from Security Component to Doctrine Bridge
I guess that this should better be placed in the Doctrine Bridge.
---------------------------------------------------------------------------
by henrikbjorn at 2011/07/13 00:00:39 -0700
Missing entry in UPDATE.md
---------------------------------------------------------------------------
by schmittjoh at 2011/07/13 00:34:10 -0700
What is our distribution strategy for these bridges?
---------------------------------------------------------------------------
by stof at 2011/07/13 00:53:29 -0700
@schmittjoh Each bridge (Doctrine, Twig, Monolog) is available as a separate repo just like the components.
---------------------------------------------------------------------------
by schmittjoh at 2011/07/13 00:57:41 -0700
I've just checked, but we don't seem to have PEAR packages for these.
If we make this change, then it should still be easy to install the component because there is no benefit of moving this class there. The bridges are mainly useful for extracting code from bundles.
---------------------------------------------------------------------------
by stof at 2011/07/13 01:02:42 -0700
@fabpot any plan to add the bridges on the PEAR channel ?
---------------------------------------------------------------------------
by fabpot at 2011/07/13 02:27:05 -0700
@stof: Bridges will be added with the RC5 release.
Commits
-------
ad2b224 [FrameworkBundle] Updated Russian translations.
95f7eed [FrameworkBundle] Fixed messages of the Choice constraint in all translations.
Discussion
----------
[FrameworkBundle] Fixes for all translations
Fixed the source messages of the choice contstraint for all translations, and re-sort messages.
And also updated Russian translations, added translations for all constraints.
Commits
-------
91cfb24 Made namespace for Doctrine cache unique to each environment
Discussion
----------
Made the cache namespace for the Doctrine cache extension unique to each environment
The current cache namespace for the Doctrine adapters is unique on an entity manager and the kernel root directory; however, it causes problems as settings for entity managers can differ between environments, so the addition of the environment into the namespace would address the issue.
$namespace = 'sf2orm_'.$entityManager['name'].'_'.md5($container->getParameter('kernel.root_dir'));
Solves issues with the cache not being valid for the DBAL ACL provider using D2 as the cache and switching environments.
An exception has been thrown during the rendering of a template ("Warning: Illegal offset type in isset or empty in ../vendor/symfony/src/Symfony/Component/Security/Acl/Dbal/AclProvider.php line 404")
Commits
-------
df34e0e [FrameworkBundle] Fix for setting a custom file link format (fixes#1652)
Discussion
----------
[FrameworkBundle] Fix for setting a custom file link format (fixes#1652)
[FrameworkBundle] Fix for setting a custom file link format (fixes#1652)
Solves issues with the cache not being valid for the DBAL ACL provider using D2 as the cache and switching environments.
An exception has been thrown during the rendering of a template ("Warning: Illegal offset type in isset or empty in ../vendor/symfony/src/Symfony/Component/Security/Acl/Dbal/AclProvider.php line 404")
Commits
-------
2a24603 [Routing] Allow multiple `@Route` annotations with a default name on a single method (fixes#1647)
Discussion
----------
[Routing] Allow multiple `@Route` annotations with a default name
[Routing] Allow multiple `@Route` annotations with a default name on a single method (fixes#1647)
Before this change, the default name would be the same for multiple `@Route` with a default name on the same method. Then only the last declared route is active.
The defaults names are (for consecutive `@Route`s):
* former_default,
* former_default_1,
* former_name,
* former_default_2,
* ...
The FrameworkExtraBundle needs to be updated in sync with this PR: https://github.com/sensio/SensioFrameworkExtraBundle/pull/50
Commits
-------
95ca258 Fixed problems with running processes returns wrong exitcode (-1) on Linux.
Discussion
----------
#1636
Fixed problems with running processes returns wrong exitcode (-1) on Linux.
---------------------------------------------------------------------------
by schmittjoh at 2011/07/11 11:07:42 -0700
This doesn't concern this PR specifically, but can we somehow start adding tests for this class? The process component seems like the by far most unstable code that we have.
---------------------------------------------------------------------------
by fabpot at 2011/07/11 23:07:43 -0700
@schmittjoh: Problem is that the behavior highly depends on the platform and PHP version.
---------------------------------------------------------------------------
by fabpot at 2011/07/11 23:14:23 -0700
@schmittjoh: By the way, we have added a bunch of tests recently (2d29a82412 (tests)/Symfony/Tests/Component/Process). Things are also "unstable" because the behavior depends on the amount of input, the amount of output, and when things happens. That means that it is also impossible to cover all possible cases reliably.
Commits
-------
d34caee merged and unified blocks `field_label` and `form_label` into `generic_label`
Discussion
----------
merged and unified blocks `field_label` and `form_label` into `generic_label`
Commits
-------
11369eb Fixed phpdoc
dbe1854 Added a AccessDeniedHttpException to wrap the AccessDeniedException.
Discussion
----------
Added a AccessDeniedHttpException to wrap the AccessDeniedException.
This is a proposal to fix#1631
It wraps the AccessDeniedException in an AccessDeniedHttpException when the firewall is not able to handle it itself. This allows getting a 403 response using the standard exception listener in this case.
Note that the app should not throw the AccessDeniedHttpException itself but keep using the AccessDeniedException to let the Security component check if the user is already fully authenticated or if it should give a chance to authenticate.
---------------------------------------------------------------------------
by fabpot at 2011/07/11 07:10:12 -0700
For reference, I've tried something more radical some time ago here: https://github.com/symfony/symfony/pull/369.
---------------------------------------------------------------------------
by stof at 2011/07/11 07:22:07 -0700
my implementation is what @schmittjoh suggested in the comments on your PR.
Commits
-------
52fdd53 [Bridge/Twig] Add required class to labels that match required fields
Discussion
----------
[Bridge/Twig] Add required class to labels that match required fields
I have used this to simply style labels that are required with a red star behind them using this CSS:
``` css
label.required::after {
content: " *";
color: #c00;
}
```
The problem is that you can't use `input[required] + label::after` as a selector since the label is typically rendered before the input. There is no way to check for an element that is *followed by* another, only elements *following*.
Of course this CSS in particular won't work except in the latest browsers, but you could still use the `label.required` selector to add a background image and so on. I think this is a very common use case and therefore I think it'd benefit the core framework.
---------------------------------------------------------------------------
by fabpot at 2011/07/05 01:27:49 -0700
Can you also update the PHP templates?
---------------------------------------------------------------------------
by schmittjoh at 2011/07/05 01:43:33 -0700
How about namespacing these css classes, like for example "sf-form-required"?
---------------------------------------------------------------------------
by stloyd at 2011/07/05 01:50:58 -0700
I would prefer an @schmittjoh naming, or even adding ability to setup it thought options.
---------------------------------------------------------------------------
by fabpot at 2011/07/05 01:54:36 -0700
Please, do not add more options. Prefix with `sf` is actually a good idea but people will argue that this is not a good idea because it gives too much information about the technology used to create the website (that's one of the things that came up pretty often in symfony1).
---------------------------------------------------------------------------
by schmittjoh at 2011/07/05 02:00:11 -0700
An option is not such a good idea imo since you likely want to have a uniform naming strategy across your entire site. How about adding a new service CssNamingStrategy?
---------------------------------------------------------------------------
by stloyd at 2011/07/05 02:01:19 -0700
Then this can be some simpler one not giving such informations i.e.: `form-label-required`, `label-required`, `framework-form-required`, `form-required` or whatever else ;-)
---------------------------------------------------------------------------
by fabpot at 2011/07/05 02:16:41 -0700
It cannot be configurable as it would potentially break bundles that come with stylesheets.
---------------------------------------------------------------------------
by stloyd at 2011/07/05 02:21:10 -0700
IMO if we decide to add this one, we could add same to `inputs/selects/etc` with `required` option.
---------------------------------------------------------------------------
by schmittjoh at 2011/07/05 02:21:59 -0700
I think it can, consider an interface like this:
```php
interface CssNamingStrategyInterface
{
function getCssName($class);
}
```
This will give people a lot of flexibility, and it also does allow them to exclude classes which for example are provided by third-party bundles.
---------------------------------------------------------------------------
by Seldaek at 2011/07/05 02:47:54 -0700
Wow guys, if this turns into a full blown class generator solution, I'm happy to close the PR.
"required" is not a name that's commonly used for main page elements, it's typically associated with forms, and therefore I don't see the need to make it unnecessary longer/namespaced. Similarly I don't see the need to add it to the input/select/.., because they already have an attribute, which you can very easily select as: `input[required]` in CSS. That works everywhere except IE6, but we can't build for the future on very old browsers. If you really want support for IE6, you can override the templates imo. But core should be looking forward, as it already is with HTML5, form markup, etc.
As for calling it form-label-required or label-required, again, I don't see the benefit, you can use `label.required` if you want to avoid conflicts with non-label elements having a required class, or a safer `form .required`. There are plenty of options in CSS itself, let's not make this overly complex.
---------------------------------------------------------------------------
by schmittjoh at 2011/07/05 02:52:17 -0700
see
http://code.google.com/intl/de-DE/speed/page-speed/docs/rendering.html#UseEfficientCSSSelectors
On Tue, Jul 5, 2011 at 11:47 AM, Seldaek <
reply@reply.github.com>wrote:
> Wow guys, if this turns into a full blown class generator solution, I'm
> happy to close the PR.
>
> "required" is not a name that's commonly used for main page elements, it's
> typically associated with forms, and therefore I don't see the need to make
> it unnecessary longer/namespaced. Similarly I don't see the need to add it
> to the input/select/.., because they already have an attribute, which you
> can very easily select as: `input[required]` in CSS. That works everywhere
> except IE6, but we can't build for the future on very old browsers. If you
> really want support for IE6, you can override the templates imo. But core
> should be looking forward, as it already is with HTML5, form markup, etc.
>
> As for calling it form-label-required or label-required, again, I don't see
> the benefit, you can use `label.required` if you want to avoid conflicts
> with non-label elements having a required class, or a safer `form
> .required`. There are plenty of options in CSS itself, let's not make this
> overly complex.
>
> --
> Reply to this email directly or view it on GitHub:
> https://github.com/symfony/symfony/pull/1519#issuecomment-1502560
>
---------------------------------------------------------------------------
by Seldaek at 2011/07/05 03:11:50 -0700
Really? Come on, we're talking about forms, it's not like you have billions of form/input tags per page that have to be parsed by the browser when you select that. Also you don't have to select the elements, if you want true performance just use no stylesheet, your users will thank you.
---------------------------------------------------------------------------
by schmittjoh at 2011/07/05 03:30:40 -0700
Your CSS selectors not only affect the performance of form elements, but of all elements that have a "required" class. Likewise, the same applies if we decide to add more classes.
Why close the door for people who care about performance? We can easily avoid this by making the css class more specific as suggested earlier. The idea with the renaming strategy is one step further and allows people to "obfuscate" which tool was used to generate the form, or do additional optimizations like shortening the css name.
---------------------------------------------------------------------------
by lenar at 2011/07/05 03:34:34 -0700
@Seldaek: Just for remark I've seen matrix forms spanning multiple screenfuls horizontally and vertically
containing tens of thousands inputs. Not pretty, but they do exist. Basically "poor" man's/company's excel
emulation or something.
---------------------------------------------------------------------------
by Seldaek at 2011/07/05 04:19:13 -0700
@schmittjoh, @lenar: We're catering to the most common use case, for which this will be more than fast enough. Small/medium scale websites don't have to optimize on CSS rules parsing, they usually have much bigger issues to deal with. If you really care about it, overriding the block to remove the class is just as easy as it was to for me to add it, but IMO this is the edge case.
---------------------------------------------------------------------------
by schmittjoh at 2011/07/05 05:37:19 -0700
IMO Symfony should follow best practices by encouraging to use class selectors, and not tag selectors for the reasons explained on the page I linked.
Anyway, I think everybody made his points. Time for @fabpot to make a decision :)
Commits
-------
22a49f1 Better docstring for FormError constructor
Discussion
----------
Better docstring for FormError constructor
Better docs for placeholder format of FormError.
Commits
-------
24e0d71 [FrameworkBundle] Fix a translatable string from the Form default validator
30d348d [Form] Make the default invalid message translatable
Discussion
----------
[Form] Translation
The first commit adds the ability to customize the default message when the form is invalid:
* Make it an option in the form builder,
* Allow placeholders in the message,
* The default value `This value is not valid` exists in the translation files.
The second commit updates a source string in the XLIFF files to make it translatable. All translations should be updated accordingly. The source string is from the [default validator](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/Extension/Core/Validator/DefaultValidator.php#L27).
This PR should fix The issue #997.
---------------------------------------------------------------------------
by fabpot at 2011/07/11 01:53:05 -0700
The first commit is not about making the message translatable, but to make it customizable (as the message is used for very different purposes depending on the Type).
---------------------------------------------------------------------------
by fabpot at 2011/07/11 01:55:11 -0700
The "This value is not valid" string should be added to the translation files too.
---------------------------------------------------------------------------
by vicb at 2011/07/11 02:02:51 -0700
@fabpot it was not translatable as the name was hardcoded in the message (instead of using a placeholder). So yes it becomes translatable now (and "customize"able as explained in the PR message).
I have also removed the form name from the default message as I don't think it brings any added value.
`This value is not valid` already exists in the translation files (see id=24).
Commits
-------
f7d0f65 RFC2616 changes
b9a218a [HttpFoundation] set Content-Length header to the length of content
Discussion
----------
[HttpFoundation] set Content-Length header to the length of content
I can't think of why this could be bad but if somebody knows please chime in.
The good thing is that with this change keepalive will work out of the box.
---------------------------------------------------------------------------
by Seldaek at 2011/07/06 05:34:51 -0700
That sounds like a great change. I think it might explain/fix the issues I've encountered with AppCache on my production box. Never had time to look into it, but IIRC I noticed the missing Content-Length, and it seemed to load forever.
---------------------------------------------------------------------------
by fabpot at 2011/07/06 06:46:50 -0700
The `Content-Length` is automatically added by servers like Apache. Moreover, sometimes, you should not add it: http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.4
---------------------------------------------------------------------------
by lenar at 2011/07/06 07:54:45 -0700
It is not added automatically by default. Yes, in case of Apache it is actually added if deflate module is enabled and if that module decides to compress the content (decision based on content-type).
About RFC2616: I will read it and add changes to this PR if applicable.
---------------------------------------------------------------------------
by fabpot at 2011/07/06 08:38:14 -0700
e943fde2ef
---------------------------------------------------------------------------
by Seldaek at 2011/07/06 08:45:22 -0700
@lenar all you have to do is skip setting the Content-Length for `1xx`, `204`, and `304` responses I believe.
---------------------------------------------------------------------------
by Seldaek at 2011/07/06 08:46:54 -0700
But this should maybe be done in sendHeaders() à la `fixContentType`, because you can't be sure about the statusCode before that.
---------------------------------------------------------------------------
by lenar at 2011/07/06 13:55:33 -0700
I propose this based on what I read and understood from RFC2616.
---------------------------------------------------------------------------
by mheleniak at 2011/07/10 03:57:26 -0700
+1
Commits
-------
e6a2d76 delay resolving values of extensions config until all files are loaded
Discussion
----------
[DependencyInjection] Delay resolving values of extensions' config until all files are loaded
This addresses #1598.
This delays resolving of extensions' configuration until all files are loaded. Without this, overriding parameters do not work for extensions.
Commits
-------
f359e3d Updated italian translations
Discussion
----------
Updated and corrected italian translations
---------------------------------------------------------------------------
by davideborsatto at 2011/07/10 12:41:29 -0700
Previously they were not translated that good, plus there were a few typos here and there.
Commits
-------
d53f312 changed "should" to "must" in message 6 and 7. also moved message 32 as replacement of message 5, according to note by yethee
e151c80 updated japanese translations
Discussion
----------
updated japanese translations
Please update japanese translations.
I picked up some messages from the latest sources of validation classes on your master branch, because translations for many other languages seems not to contain messages for recent validator classes, such as Image,Ip,Locale,Language.
Thanks in advance!
---------------------------------------------------------------------------
by yethee at 2011/07/10 09:23:01 -0700
> "One or more..." is on message#33 in my commit, should I locate #33 just after #5?
Not necessarily, the order of messages is not important, AFAIK.
Commits
-------
9714cfc Fixes fatal error when intl module is not installed.
Discussion
----------
Session instantiation breaks with Fatal error if intl module is not installed.
A check for class_exists in setPhpDefaultLocale() fixes this, though I got the feeling it should be resolved with a proxy or subscriber object; setPhpDefaultLocale feels like a hack now.
---------------------------------------------------------------------------
by stealth35 at 2011/07/10 06:32:43 -0700
Locale::setDefault don't throw any exception
Maybe just :
```php
if (class_exists('Locale', false)) {
return \Locale::setDefault($this->locale);
} else {
return false;
}
```
