Commits
-------
ac59db7 cleanup
64ea95d [WebProfilerBundle] Add redirection info to the router panel
826bd23 [FrameworkBundle] fix phpDoc of ControllerResolver::createController()
e3cf37f [HttpFoundation] RedirectResponse: add the ability to retrieve the target URL, add unit tests
50c85ae [WebProfiler] Add info to the router panel
Discussion
----------
[WIP][Profiler] Routing
former #3206 part 3 (depends on part 1 - #3280)
The goal of this PR is to fix#3264 by adding redirection infos on the router panel.
Done:
* Add info on the target url / route
To do:
* Display an accurate URL matching process (when using the RedirectableUrlMatcher)
Commits
-------
22c8f80 [Form] Fixed issues mentioned in the PR comments
3b1b570 [Form] Fixed: The "date", "time" and "datetime" types can be initialized with \DateTime objects
88ef52d [Form] Improved FormType::getDefaultOptions() to see default options defined in parent types
b9facfc [Form] Removed undefined variables in exception constructor
Discussion
----------
[Form] Fixed: "date", "time" and "datetime" fields can be initialized with \DateTime objects
Bug fix: yes
Feature addition: yes
Backwards compatibility break: **yes**
Symfony2 tests pass: yes
Fixes the following tickets: #3288
Todo: -
Fixed exception that was thrown when doing:
$builder->add('createdAt', 'date', array('data' => new \DateTime()));
On a side note, the options passed to `FieldType::getDefaultOptions` now always also contain the default options of any parent types. This is necessary if you want to be independent of how `getDefaultOptions` is implemented in the parent type and still rely on the already defined values.
As a result, `FieldType::getParent` doesn't see default options anymore. This shouldn't be a big problem, because this method only relies on options in few cases. If it does, options now need to be checked for existence with `isset` before being used (BC break).
---------------------------------------------------------------------------
by bschussek at 2012-02-09T16:14:46Z
@fabpot Ready to merge.
---------------------------------------------------------------------------
by bschussek at 2012-02-10T12:15:04Z
@fabpot Ready to merge
Commits
-------
411a0cc [Validator] Added GroupSequenceProvider to changelog
815c769 [Validator] Renamed getValidationGroups to getGroupSequence
d84a2e4 [Validator] Updated test expectations
9f2310b [Validator] Fixed typos, renamed hasGroupSequenceProvider
e0d2828 [Validator] GroupSequenceProvider tests improved, configuration changed
c3b04a3 [Validator] Changed GroupSequenceProvider implementation
6c4455f [Validator] Added GroupSequenceProvider
Discussion
----------
[Validator] Added GroupSequenceProvider
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: 
As discussed in #3114 I implemented the "GroupSequenceProvider" pattern for the validator component. It allows the user to select certain validation groups based on the current state of an object. Here is an example:
/**
* @Assert\GroupSequenceProvider("UserGroupSequnceProvider")
*/
class User
{
/**
* @Assert\NotBlank(groups={"Premium"})
*/
public function getAddress();
public function hasPremiumSubscription();
}
class UserGroupSequenceProvider implements GroupSequenceProviderInterface
{
public function getValidationGroups($user)
{
if ($user->hasPremiumSubscription()) {
return array('User', 'Premium');
} else {
return array('User');
}
}
}
With this patch there are two mechanisms to define the group sequence now. Either you can use @GroupSequence to define a static order of validation groups or you can use @GroupSequenceProvider to create dynamic validation group arrays.
The ClassMetadata therefore has methods now which implement quite similar things. The question is whether it would make sense to interpret the static group sequence as a special case and create something like a DefaultGroupSequenceProvider or StaticGroupSequenceProvider which is assigned by default. This would cause a BC break inside the validator component.
---------------------------------------------------------------------------
by bschussek at 2012-01-28T13:39:54Z
I like the implementation, but I think we should differ a little bit from Java here.
1. `GroupSequenceProviderInterface` should be implemented by the domain classes themselves (`User`), not by a separate class.
2. As such, the parameter `$object` from `getValidationGroups($object)` can be removed
3. `ClassMetadata::setGroupSequenceProvider()` should accept a boolean to activate/deactivate this functionality. Also the check for the interface (does the underlying class implement it?) should be done here
Apart from that, special cases need to be treated:
* A definition of a group sequence and a group sequence provider in the same `ClassMetadata` should not be allowed. Either of them must not be set.
* Metadata loaders must take care of settings made by parent classes. If `Animal` is extended by `Dog`, `Animal` defines a group sequence (or group sequence provider) and `Dog` a group sequence provider (or group sequence), only the setting of `Dog` should apply
---------------------------------------------------------------------------
by blogsh at 2012-01-28T21:25:37Z
Changes of the latest commit:
- GroupSequenceProviderInterface has to be implemented by the domain class
- The annotation/configuration options let the user define whether the provider is activated or not (is this neccessary at all?)
- An error is thrown if the user wants to use static group sequences and the provider simultaneously
At the moment neither the static group sequence nor the provider is inherited from parent classes or interfaces. I don't know if it would make sense to enable this feature. There could be problems if a user wants to define a static group sequence in the parent class and a sequence provider in the child class.
---------------------------------------------------------------------------
by bschussek at 2012-01-30T13:07:04Z
> There could be problems if a user wants to define a static group sequence in the parent class and a sequence provider in the child class.
In this case, the setting in the child class should override the setting of the parent class.
But we can leave this open for now. As it seems, [this issue is unresolved in Hibernate as well](https://hibernate.onjira.com/browse/HV-467).
---------------------------------------------------------------------------
by blogsh at 2012-01-30T22:54:41Z
Okay, finally I managed to upload the latest commit. If you got a bunch of notifications or so I'm sorry, but I had to revert some accidental changes in the commit :(
I've rewritten the tests and have removed the "active" setting in the XML configuration.
---------------------------------------------------------------------------
by blogsh at 2012-02-02T15:24:01Z
Okay, typos are fixed now and `hasGroupSequenceProvider` has been renamed to `isGroupSequenceProvider`. I also had to adjust some tests after the rebase with master.
---------------------------------------------------------------------------
by bschussek at 2012-02-03T09:25:19Z
Looks good.
@fabpot 👍
---------------------------------------------------------------------------
by fabpot at 2012-02-03T09:46:52Z
Can you add a note in the CHANGELOG before I merge? Thanks.
---------------------------------------------------------------------------
by blogsh at 2012-02-09T12:31:27Z
@fabpot done
Commits
-------
8714d79 [Form] Simplified code in MergeCollectionListener
8ab982a [Form] Fixed: Custom add and remove method are not invoked if disallowed
02f61ad [Form] Renamed choice and collection options "adder_prefix" and "remover_prefix" to "add_method" and "remove_method" and allowed to specify full method names
b393774 [Form] Used direct method access in MergeCollectionListener instead of Reflection to avoid problems when using class hierarchies
d208f4e [Form] Made it possible to use models with only either addXxx() or removeXxx()
Discussion
----------
[Form] Fixed edge cases in MergeCollectionListener
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
Fixes an issue mentioned in the comments of #3239
see https://github.com/symfony/symfony/pull/3239#issuecomment-3776312
---------------------------------------------------------------------------
by bschussek at 2012-02-02T12:12:17Z
Wait a minute before merging this.
---------------------------------------------------------------------------
by bschussek at 2012-02-02T13:01:55Z
@fabpot Ready to merge
The listener is used by the Collection type as well as the Choice and Entity type (with multiple
selection). The effect is that you can have for example this model:
class Article
{
public function addTag($tag) { ... }
public function removeTag($tag) { ... }
public function getTags($tag) { ... }
}
You can create a form for the article with a field "tags" of either type "collection" or "choice"
(or "entity"). The field will correctly use the three methods of the model for displaying and
editing tags.
Commits
-------
2e4ebe4 [Validator] Renamed methods addViolationAtRelativePath() and getAbsolutePropertyPath() in ExecutionContext
9153f0e [Validator] Deprecated ConstraintValidator methods setMessage(), getMessageTemplate() and getMessageParameters()
0417282 [Validator] Fixed typos
a30a679 [Validator] Made ExecutionContext immutable and introduced new class GlobalExecutionContext
fe85bbd [Validator] Simplified ExecutionContext::addViolation(), added ExecutionContext::addViolationAt()
f77fd41 [Form] Fixed typos
1fc615c Fixed string access by curly brace to bracket
a103c28 [Validator] The Collection constraint adds "missing" and "extra" errors to the individual fields now
f904a9e [Validator] Fixed: GraphWalker does not add constraint violation if error message is empty
1dd302c [Validator] Fixed ConstraintViolationList::__toString() to not include dots in the output if the root is empty
1678a3d [Validator] Fixed: Validator::validateValue() propagates empty validation root instead of the provided value
Discussion
----------
[Validator] Improved "missing" and "extra" errors of Collection constraint
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #2615
Todo: -
Instead of a single violation
Array:
The fields "foo", "bar" are missing
various violations are now generated.
Array[foo]:
This field is missing
Array[bar]:
This field is missing
Apart from that, the PR contains various minor fixes to the Validator.
---------------------------------------------------------------------------
by bschussek at 2012-02-02T09:14:52Z
@fabpot Ready for merge.
Commits
-------
e9b4c58 [Console] Enable process isolantion in Shell
Discussion
----------
[Console] Enable process isolantion in Shell
Bug fix: no
BC break: no
Feature addition: yes
Symfony2 test pass: yes
Fixes the following tickets: #2848#2847
Todo: Write unit tests
See tickets for reference, need help with unit testing, because I don't know how to test this :)
---------------------------------------------------------------------------
by canni at 2011-12-16T09:36:32Z
I've tested this with different scenarios like "inception" (invoking shell from shell - will not work) ;) and others, everything seems to work great.
As I have no idea on how to pack this with unit testing some help needed, also as I don't have any windows in home ;) need someone to test it on MS os.
And we should decide, do we want process isolation by default? (This will not break the BC, break only the "expected behavior" - colorful output and "interactivity")
---------------------------------------------------------------------------
by canni at 2011-12-18T15:14:26Z
I've rebased this branch to match current `HEAD` and I've added usage of new process builder, for better portability an shell arg escaping.
---------------------------------------------------------------------------
by fabpot at 2012-02-02T08:28:32Z
@canni: Can you squash your commits before I merge this PR? Thanks.
---------------------------------------------------------------------------
by canni at 2012-02-02T09:07:16Z
@fabpot @stof done.
Bug fix: no
BC break: no
Feature addition: yes
Symfony2 test pass: yes
Fixes the following tickets: #2848#2847
Todo: -
See tickets for reference, need help with testing, because I don't know how to test this :)
Commits
-------
de253dd [Form] read_only and disabled attributes
Discussion
----------
[Form] read_only and disabled attributes (closes#1974)
1. Removed ``readOnly`` property from ``Form``, as it is no longer required
2. Introduced ``disabled`` property to ``Form``, behaves exactly like ``readOnly`` used to
3. Added ``disabled`` property to fields, defaults to ``false``, renders as ``disabled="disabled"``
4. A field with positive ``read_only`` property now renders as ``readonly="readonly"``
---------------------------------------------------------------------------
by helmer at 2012-01-26T17:46:17Z
I changed ``Form`` and ``FormBuilder`` property ``readOnly`` to ``disabled``. On second thought, this is perhaps not such good change - while readOnly somewhat implied the use-case, disabled no longer does.
Perhaps something else, like ``bindable`` (as not to confuse with read_only attribute of Fields)?
@bschussek, others, any thoughts?
---------------------------------------------------------------------------
by bschussek at 2012-01-31T06:53:59Z
Please prefix commits with the affected component, if applicable.
---------------------------------------------------------------------------
by helmer at 2012-01-31T08:41:03Z
@bschussek Prefixed. Please also see see to [this question](https://github.com/symfony/symfony/pull/3193#issuecomment-3673074)
A new ExecutionContext is now created everytime that GraphWalker::walkConstraint() is
launched. Because of this, a validator B launched from within a validator A can't break
A anymore by changing the context.
Because we have a new ExecutionContext for every constraint validation, there is no point
in modifying its state anymore. Because of this it is now immutable.
Commits
-------
8dc78bd [Form] Fixed YODA issues
600cec7 [Form] Added missing entries to CHANGELOG and UPGRADE
b154f7c [Form] Fixed docblock and unneeded use statement
399af27 [Form] Implemented checks to assert that values and indices generated in choice lists match their requirements
5f6f75c [Form] Fixed outstanding issues mentioned in the PR
7c70976 [Form] Fixed text in UPGRADE file
c26b47a [Form] Made query parameter name generated by ORMQueryBuilderLoader unique
18f92cd [Form] Fixed double choice fixing
f533ef0 [Form] Added ChoiceView class for passing choice-related data to the view
d72900e [Form] Incorporated changes suggested in PR comments
28d2f6d Removed duplicated lines from UPGRADE file
e1fc5a5 [Form] Restricted form names to specific characters to (1) fix generation of HTML IDs and to (2) avoid problems with property paths.
87b16e7 [Form] Greatly improved ChoiceListInterface and all of its implementations
Discussion
----------
[Form] Improved ChoiceList implementation and made form naming more restrictive
Bug fix: yes
Feature addition: yes
Backwards compatibility break: **yes**
Symfony2 tests pass: yes
Fixes the following tickets: #2869, #3021, #1919, #3153
Todo: adapt documentation
The changes in this PR are primarily motivated by the fact that invalid form/field names lead to various problems.
1. When a name contains any characters that are not permitted in HTML "id" attributes, these are invalid
2. When a name contains periods ("."), form validation is broken, because they confuse the property path resolution
3. Since choices in expanded choice fields are directly translated to field names, choices applying to either 1. or 2. lead to problems. But choices should be unrestricted.
4. Unless a choice field is not expanded and does not allow multiple selection, it is not possible to use empty strings as choices, which might be desirable in some occasions.
The solution to these problems is to
* Restrict form names to disallow unpermitted characters (solves 1. and 2.)
* Generate integer indices to be stored in the HTML "id" and "name" attributes and map them to the choices (solves 3.). Can be reverted to the old behaviour by setting the option "index_generation" to ChoiceList::COPY_CHOICE
* Generate integer values to be stored in the HTML "value" attribute and map them to the choices (solves 4.). Can be reverted to the old behaviour by setting the option "value_generation" to ChoiceList::COPY_CHOICE
Apart from these fixes, it is now possible to write more flexible choice lists. One of these is `ObjectChoiceList`, which allows to use objects as choices and is bundled in the core. `EntityChoiceList` has been made an extension of this class.
$form = $this->createFormBuilder()
->add('object', 'choice', array(
'choice_list' => new ObjectChoiceList(
array($obj1, $obj2, $obj3, $obj4),
// property path determining the choice label (optional)
'name',
// preferred choices (optional)
array($obj2, $obj3),
// property path for object grouping (optional)
'category',
// property path for value generation (optional)
'id',
// property path for index generation (optional)
'id'
)
))
->getForm()
;
---------------------------------------------------------------------------
by kriswallsmith at 2012-01-19T18:09:09Z
Rather than passing `choices` and a `choice_labels` arrays to the view would it make sense to introduce a `ChoiceView` class and pass one array of objects?
---------------------------------------------------------------------------
by stof at 2012-01-22T15:32:36Z
@bschussek can you update your PR according to the feedback (and rebase it as it conflicts according to github) ?
---------------------------------------------------------------------------
by bschussek at 2012-01-24T00:15:42Z
@kriswallsmith fixed
Fixed all outstanding issues. Would be glad if someone could review again, otherwise this PR is ready to merge.
---------------------------------------------------------------------------
by fabpot at 2012-01-25T15:17:59Z
Is it ready to be merged?
---------------------------------------------------------------------------
by Tobion at 2012-01-25T15:35:50Z
Yes I think so. He said it's ready to be merged when reviewed.
---------------------------------------------------------------------------
by bschussek at 2012-01-26T02:30:36Z
Yes.
---------------------------------------------------------------------------
by bschussek at 2012-01-28T12:39:00Z
Fixed outstanding issues. Ready for merge.
Commits
-------
887c0e9 moved EngineInterface::stream() to a new StreamingEngineInterface to keep BC with 2.0
473741b added the possibility to change a StreamedResponse callback after its creation
8717d44 moved a test in the constructor
e44b8ba made some cosmetic changes
0038d1b [HttpFoundation] added support for streamed responses
Discussion
----------
[HttpFoundation] added support for streamed responses
To stream a Response, use the StreamedResponse class instead of the
standard Response class:
$response = new StreamedResponse(function () {
echo 'FOO';
});
$response = new StreamedResponse(function () {
echo 'FOO';
}, 200, array('Content-Type' => 'text/plain'));
As you can see, a StreamedResponse instance takes a PHP callback instead of
a string for the Response content. It's up to the developer to stream the
response content from the callback with standard PHP functions like echo.
You can also use flush() if needed.
From a controller, do something like this:
$twig = $this->get('templating');
return new StreamedResponse(function () use ($templating) {
$templating->stream('BlogBundle:Annot:streamed.html.twig');
}, 200, array('Content-Type' => 'text/html'));
If you are using the base controller, you can use the stream() method instead:
return $this->stream('BlogBundle:Annot:streamed.html.twig');
You can stream an existing file by using the PHP built-in readfile() function:
new StreamedResponse(function () use ($file) {
readfile($file);
}, 200, array('Content-Type' => 'image/png');
Read http://php.net/flush for more information about output buffering in PHP.
Note that you should do your best to move all expensive operations to
be "activated/evaluated/called" during template evaluation.
Templates
---------
If you are using Twig as a template engine, everything should work as
usual, even if are using template inheritance!
However, note that streaming is not supported for PHP templates. Support
is impossible by design (as the layout is rendered after the main content).
Exceptions
----------
Exceptions thrown during rendering will be rendered as usual except that
some content might have been rendered already.
Limitations
-----------
As the getContent() method always returns false for streamed Responses, some
event listeners won't work at all:
* Web debug toolbar is not available for such Responses (but the profiler works fine);
* ESI is not supported.
Also note that streamed responses cannot benefit from HTTP caching for obvious
reasons.
---------------------------------------------------------------------------
by Seldaek at 2011/12/21 06:34:13 -0800
Just an idea: what about exposing flush() to twig? Possibly in a way that it will not call it if the template is not streaming. That way you could always add a flush() after your </head> tag to make sure that goes out as fast as possible, but it wouldn't mess with non-streamed responses. Although it appears flush() doesn't affect output buffers, so I guess it doesn't need anything special.
When you say "ESI is not supported.", that means only the AppCache right? I don't see why this would affect Varnish, but then again as far as I know Varnish will buffer if ESI is used so the benefit of streaming there is non-existent.
---------------------------------------------------------------------------
by cordoval at 2011/12/21 08:04:21 -0800
wonder what the use case is for streaming a response, very interesting.
---------------------------------------------------------------------------
by johnkary at 2011/12/21 08:19:48 -0800
@cordoval Common use cases are present fairly well by this RailsCast video: http://railscasts.com/episodes/266-http-streaming
Essentially it allows faster fetching of web assets (JS, CSS, etc) located in the <head></head>, allowing those assets to be fetched as soon as possible before the remainder of the content body is computed and sent to the browser. The end goal is to improve page load speed.
There are other uses cases too like making large body content available quickly to the service consuming it. Think if you were monitoring a live feed of JSON data of newest Twitter comments.
---------------------------------------------------------------------------
by lsmith77 at 2011/12/21 08:54:35 -0800
How does this relate the limitations mentioned in:
http://yehudakatz.com/2010/09/07/automatic-flushing-the-rails-3-1-plan/
Am I right to understand that due to how twig works we are not really streaming the content pieces when we call render(), but instead the entire template with its layout is rendered and only then will we flush? or does it mean that the render call will work its way to the top level layout template and form then on it can send the content until it hits another block, which it then first renders before it continues to send the data?
---------------------------------------------------------------------------
by stof at 2011/12/21 09:02:53 -0800
@lsmith77 this is why the ``stream`` method calls ``display`` in Twig instead of ``render``. ``display`` uses echo to print the output of the template line by line (and blocks are simply method calls in the middle). Look at your compiled templates to see it (the ``doDisplay`` method)
Rendering a template with Twig simply use an output buffer around the rendering.
---------------------------------------------------------------------------
by fabpot at 2011/12/21 09:24:33 -0800
@lsmith77: We don't have the Rails problem thanks to Twig as the order of execution is the right one by default (the layout is executed first); it means that we can have the flush feature without any change to how the core works. As @stof mentioned, we are using `display`, not `render`, so we are streaming your templates for byte one.
---------------------------------------------------------------------------
by fabpot at 2011/12/21 09:36:41 -0800
@Seldaek: yes, I meant ESI with the PHP reverse proxy.
---------------------------------------------------------------------------
by fabpot at 2011/12/21 09:37:34 -0800
@Seldaek: I have `flush()` support for Twig on my todo-list. As you mentioned, It should be trivial to implement.
---------------------------------------------------------------------------
by fzaninotto at 2011/12/21 09:48:18 -0800
How do streaming responses deal with assets that must be called in the head, but are declared in the body?
---------------------------------------------------------------------------
by fabpot at 2011/12/21 09:52:12 -0800
@fzaninotto: What do you mean?
With Twig, your layout is defined with blocks ("holes"). These blocks are overridden by child templates, but evaluated as they are encountered in the layout. So, everything works as expected.
As noted in the commit message, this does not work with PHP templates for the problems mentioned in the Rails post (as the order of execution is not the right one -- the child template is first evaluated and then the layout).
---------------------------------------------------------------------------
by fzaninotto at 2011/12/21 10:07:35 -0800
I was referring to using Assetic. Not sure if this compiles to Twig the same way as javascript and stylesheet blocks placed in the head - and therefore executed in the right way.
---------------------------------------------------------------------------
by fabpot at 2011/12/21 10:34:59 -0800
@Seldaek: I've just added a `flush` tag in Twig 1.5: 1d6dfad4f5
---------------------------------------------------------------------------
by catchamonkey at 2011/12/21 13:29:22 -0800
I'm really happy you've got this into the core, it's a great feature to have! Good work.
To stream a Response, use the StreamedResponse class instead of the
standard Response class:
$response = new StreamedResponse(function () {
echo 'FOO';
});
$response = new StreamedResponse(function () {
echo 'FOO';
}, 200, array('Content-Type' => 'text/plain'));
As you can see, a StreamedResponse instance takes a PHP callback instead of
a string for the Response content. It's up to the developer to stream the
response content from the callback with standard PHP functions like echo.
You can also use flush() if needed.
From a controller, do something like this:
$twig = $this->get('templating');
return new StreamedResponse(function () use ($templating) {
$templating->stream('BlogBundle:Annot:streamed.html.twig');
}, 200, array('Content-Type' => 'text/html'));
If you are using the base controller, you can use the stream() method instead:
return $this->stream('BlogBundle:Annot:streamed.html.twig');
You can stream an existing file by using the PHP built-in readfile() function:
new StreamedResponse(function () use ($file) {
readfile($file);
}, 200, array('Content-Type' => 'image/png');
Read http://php.net/flush for more information about output buffering in PHP.
Note that you should do your best to move all expensive operations to
be "activated/evaluated/called" during template evaluation.
Templates
---------
If you are using Twig as a template engine, everything should work as
usual, even if are using template inheritance!
However, note that streaming is not supported for PHP templates. Support
is impossible by design (as the layout is rendered after the main content).
Exceptions
----------
Exceptions thrown during rendering will be rendered as usual except that
some content might have been rendered already.
Limitations
-----------
As the getContent() method always returns false for streamed Responses, some
event listeners won't work at all:
* Web debug toolbar is not available for such Responses (but the profiler works fine);
* ESI is not supported.
Also note that streamed responses cannot benefit from HTTP caching for obvious
reasons.
Commits
-------
8710a13 Added example to the change log file
c9a2b49 Fixed xml encoder test script, and group `item` tags into an array
a0561e5 Replaced `item` with `*item` when parsing XML string
Discussion
----------
Replaced `item` with `*item` when parsing XML string
---------------------------------------------------------------------------
by fabpot at 2011/11/23 22:14:12 -0800
Tests do not pass:
1) Symfony\Tests\Component\Serializer\Encoder\XmlEncoderTest::testDecode
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
'key2' => 'val'
- 'A B' => 'bar'
'Barry' => Array (...)
+ 'item' => Array (...)
)
'qux' => '1'
)
.../tests/Symfony/Tests/Component/Serializer/Encoder/XmlEncoderTest.php:173
---------------------------------------------------------------------------
by fabpot at 2011/11/24 22:57:37 -0800
I don't understand the patch anymore. I don't see any use of `*item` in the code.
---------------------------------------------------------------------------
by excelwebzone at 2011/11/24 23:04:07 -0800
I run some testing and you can't use '*item' XML parser reject it. So I modified it to convert it to an array.. Look at the test script change
---------------------------------------------------------------------------
by fabpot at 2011/11/24 23:13:30 -0800
So, you probably need to change the CHANGELOG as well? You should add an example which shows a before/after example.
---------------------------------------------------------------------------
by excelwebzone at 2011/11/24 23:15:51 -0800
Yes, forgot to change that..
---------------------------------------------------------------------------
by fabpot at 2011/11/25 01:27:42 -0800
ping @Seldaek, @lsmith77
---------------------------------------------------------------------------
by Seldaek at 2011/11/25 04:16:43 -0800
There are other meta-names available in the XmlEncoder, @-something for attributes, then there is something happening with a # but I'm not quite sure what. I'm just saying, maybe *item isn't the best name, if it introduces a third metacharacter. Apart from that I'm fine with it.
---------------------------------------------------------------------------
by excelwebzone at 2011/11/25 08:45:31 -0800
Maybe we can rename it to `wildcard` instead
---------------------------------------------------------------------------
by excelwebzone at 2011/11/25 15:12:09 -0800
Any chance we can push this throw?
---------------------------------------------------------------------------
by lsmith77 at 2011/11/27 04:06:25 -0800
here is the old PR #2682
@Seldaek: i think your comment was made for an older version of the patch.
overall I am fine with the change, the Serializer component takes a fairly simple approach. it is also not designed to really produce XML or JSON cleanly from the same data. it will really only be able to output a clean API for one or the other with the same data structure.
---------------------------------------------------------------------------
by excelwebzone at 2011/12/01 06:25:24 -0800
@fabpot can we merge this change
Commits
-------
413756c [BC break][SecurityBundle] Changed the way to register factories
Discussion
----------
[BC break][SecurityBundle] Changed the way to register factories
As discussed in #2454, this changes the way to register the factories to let each bundles register the factories it provides.
Commits
-------
d2195cc Fixed phpdoc and updated the changelog
9e41ff4 [SecurityBundle] Added a validation rule
b107a3f [SecurityBundle] Refactored the configuration
633f0e9 [DoctrineBundle] Moved the entity provider service to DoctrineBundle
74732dc [SecurityBundle] Added a way to extend the providers section of the config
Discussion
----------
[WIP][SecurityBundle] Added a way to extend the providers section of the config
Bug fix: no
Feature addition: yes
BC break: <del>no (for now)</del> yes
Tests pass: yes
This adds a way to extend the ``providers`` section of the security config so that other bundles can hook their stuff into it. An example is available in DoctrineBundle which is now responsible to handle the entity provider (<del>needs some cleanup as the service definition is still in SecurityBundle currently</del>). This will allow PropelBundle to provide a ``propel:`` provider for instance.
In order to keep BC with the existing configuration for the in-memory and the chain providers, I had to allow using a prototyped node instead of forcing using an array node with childrens. This introduces some issues:
- impossible to validate easily that a provider uses only one setup as prototyped node always have a default value (the empty array)
- the ``getFixableKey`` method is needed in the interface to support the XML format by pluralizing the name.
Here is my non-BC proposal for the configuration to clean this:
```yaml
security:
providers:
first:
memory: # BC break here by adding a level before the users
users:
joe: { password: foobar, roles: ROLE_USER }
john: { password: foobarbaz, roles: ROLE_USER }
second:
entity: # this one is BC
class: Acme\DemoBundle\Entity\User
third:
id: my_custom_provider # also BC
fourth:
chain: # BC break by adding a level before the providers
providers: [first, second, third]
```
What do you think about it ? Do we need to keep the BC in the config of the bundle or no ?
Btw note that the way to register the factories used by the firewall section should be refactored using the new way to provide extension points in the extensions (as done here) instead of relying on the end user to register factories, which would probably mean a BC break anyway.
---------------------------------------------------------------------------
by lsmith77 at 2011/10/23 09:19:23 -0700
i don't think we should keep BC. the security config is complex as is .. having BC stuff in there will just make it even harder and confusing.
---------------------------------------------------------------------------
by willdurand at 2011/10/23 09:41:25 -0700
Is the security component tagged with `@api` ?
So basically, we just have to create a factory (`ModelFactory` for instance) and to register it in the `security` extension, right ? Seems quite simple to extend and much better than the hardcoded version…
Why did you call the method to pluralize a key `getFixableKey` ?
---------------------------------------------------------------------------
by beberlei at 2011/10/23 14:48:26 -0700
Changing security config will introduce risk for users. We should avoid that
---------------------------------------------------------------------------
by stof at 2011/10/23 15:34:47 -0700
@beberlei as the config is validated, it will simply give them an exception during the loading of the config if they don't update their config.
---------------------------------------------------------------------------
by stof at 2011/10/24 01:01:42 -0700
@schmittjoh @fabpot Could you give your mind about it ?
---------------------------------------------------------------------------
by stof at 2011/10/31 17:08:12 -0700
@fabpot @schmittjoh ping
---------------------------------------------------------------------------
by stof at 2011/11/11 14:08:18 -0800
I updated the PR by implementing my proposal as the latest IRC meeting agreed that we don't need to keep the BC for this change. This allows to add the validation rule now.
---------------------------------------------------------------------------
by stof at 2011/11/16 11:16:06 -0800
@fabpot ping
---------------------------------------------------------------------------
by fabpot at 2011/11/16 22:29:05 -0800
@stof: Before merging, you must also add information about how to upgrade in the CHANGELOG-2.1.md file.
---------------------------------------------------------------------------
by stof at 2011/11/17 00:01:23 -0800
@fabpot done
The Firewall is now executed after the Router. This was needed to have access
to the locale and other request attributes that are set by the Router. This
change implies that all Firewall specific URLs have proper (empty) routes like
`/login_check` and `/logout`.
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.
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).
