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
-------
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.
* 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.
* 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
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.
Commits
-------
a245e15 [DomCrawler] trim URI in getURI
Discussion
----------
[DomCrawler] trim URI in getURI
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #2599
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
-------
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
-------
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
-------
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.
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
-------
fee217f Update tests/Symfony/Tests/Component/Process/ProcessTest.php
d58e682 GH-2287 - Skip Process:processTest on Windows due to PHP bug #60120
Discussion
----------
[Process] Test with pipes hang on windows
Rebased onto 2.0
This is a common case with optional Entity associations:
$user->getGroup()->getName()
The property path `group.name` (traversing from the User Entity), when
the User is not assigned to a Group, will throw an UnexpectedTypeException.
This is caught so that the User can still be listed as an option, just without
any nesting.
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=..``
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
-------
205f524 [Tests] Skip MimeTypeTest if running as root
d3e9104 [Tests] Skip Routing annotation tests when Doctrine is not available.
Discussion
----------
[Tests] Fix tests when some vendor is not available.
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.
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.
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
-------
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
-------
18a83c6 [Form] Simplified a bit `FormUtil` and extended test coverage
Discussion
----------
[2.0][Form] Simplified a bit `FormUtil` and extended test coverage
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
* 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
-------
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
-------
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
-------
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
-------
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)?
* 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
-------
777f876 [HttpFoundation] Added test that exposes error in session saving
Discussion
----------
[HttpFoundation] Added test that exposes error in session saving
Noticed this commit in the recent release:
https://github.com/symfony/symfony/commit/34a1b53
And noticed that the save in __destruct won't be fired if a manual save has been called. The testSavedOnDestructAfterManualSave test I've added fails on 2.0.1 but it passes with 2.0.0. An example:
$session->set('foo', 'value');
$session->__destruct(); // eventually called during shutdown, triggers a save
// During next request, $session->get('foo') returns 'value'
$session->set('foo', 'value');
$session->save();
$session->set('foo', 'newvalue');
$session->__destruct(); // eventually called during shutdown, however WON'T trigger save
// During next request, $session->get('foo') returns 'value'
In my eyes the save on destruction should still happen whether a save has been called manually or not - it's more predictable. But i can see arguments for the opposite as well.
If consensus is that this is a bug, I'm happy to provide a fix but wanted to get feedback on the 2 options:
* Save optimisation can be reverted
* Can make the save optimisation more intelligent so a write to storage is only done if something has changed. The best way to do this would be to close down the api and mark the session as invalid when an attribute is set for example, however all properties are currently protected and would need to be private, so it might break some people's extensions of session
---------------------------------------------------------------------------
by stof at 2011/09/04 02:13:52 -0700
@fabpot what about it ?
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
-------
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
-------
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.
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
-------
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
-------
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