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.