Commit Graph

2131 Commits

Author SHA1 Message Date
Fabien Potencier
745b9a6d6c [HttpKernel] fixed function support in ControllerResolver (closes #3331) 2012-02-12 00:34:53 +01:00
Fabien Potencier
5efbd9f50e [HttpFoundation] fixed Request::create() when passing arguments as an array (closes #3314) 2012-02-12 00:26:10 +01:00
Fabien Potencier
48414000bb merged branch drak/session_refactor (PR #2853)
Commits
-------

cb6fdb1 [HttpFoundation] removed Session::close()
c59d880 Docblocks.
b8df162 Correct instanceof condition.
8a01dd5 renamed getFlashes() to getFlashBag() to avoid clashes
282d3ae updated CHANGELOG for 2.1
0f6c50a [HttpFoundation] added some method for a better BC
146a502 [FrameworkBundle] added some service aliases to avoid some BC breaks
93d81a1 [HttpFoundation] removed configuration for session storages in session.xml as we cannot provide a way to configure them (like before this PR anyway)
74ccf70 reverted 5b7ef11650 (Simplify session storage class names now we have a separate namespace for sessions)
91f4f8a [HttpFoundation] changed default flash bag to auto-expires to keep BC
0494250 removed unused use statements
7878a0a [HttpFoundation] renamed pop() to all() and getAll() to all()
0d2745f [HttpFoundation] Remove constants from FlashBagInterface
dad60ef [HttpFoundation] Add back get defaults and small clean-up.
5b7ef11 [HttpFoundation] Simplify session storage class names now we have a separate namespace for sessions.
27530cb [HttpFoundation] Moved session related classes to own sub-namespace.
4683915 [HttpFoundation] Free bags from session storage and move classes to their own namespaces.
d64939a [DoctrineBridge] Refactored driver for changed interface.
f9951a3 Fixed formatting.
398acc9 [HttpFoundation] Reworked flashes to maintain same behaviour as in Symfony 2.0
f98f9ae [HttpFoundation] Refactor for DRY code.
9dd4dbe Documentation, changelogs and coding standards.
1ed6ee3 [DoctribeBridge][SecurityBundle][WebProfiler] Refactor code for HttpFoundation changes.
7aaf024 [FrameworkBundle] Refactored code for changes to HttpFoundation component.
669bc96 [HttpFoundation] Added pure Memcache, Memcached and Null storage drivers.
e185c8d [HttpFoundation] Refactored component for session workflow.
85b5c43 [HttpFoundation] Added drivers for PHP native session save handlers, files, sqlite, memcache and memcached.
57ef984 [HttpFoundation] Added unit and functional testing session storage objects.
3a263dc [HttpFoundation] Introduced session storage base class and interfaces.
c969423 [HttpFoundation] Added FlashBagInterface and concrete implementation.
39288bc [HttpFoundation] Added AttributesInterface and AttributesBagInterface and concrete implementations.

Discussion
----------

[2.1][HttpFoundation] Refactor session handling and flash messages

Bug fix: yes
Feature addition: yes
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: #2607, #2591, #2717, #2773
References the following tickets: #2592, #2543, #2541, #2510, #2714, #2684
Todo: -

__Introduction__

This extensive PR is a refactor with minimal BC breaks of the `[HttpFoundation]` component's session management which fixes several issues in the current implementation.  This PR includes all necessary changes to other bundles and components is documented in the `CHANGELOG-2.1` and `UPGRADING-2.1`.

__Summary of Changes__

__Session:__
  - Session object now implements `SessionInterface`

__Attributes:__
  - Attributes now handled by `AttributeBagInterface`
  - Added two AttributeBag implementations: `AttributeBag` replicates the current Symfony2 attributes behaviour, and the second, `NamespacedAttributeBag` introduces structured namespaced representation using '/' in the key.  Both are BC.  `FrameworkBundle` defaults to the old behaviour.

__Flash messages:__
  - Flash messages now handled by `FlashBagInterface`
  - Introduced `FlashBag` which changes the way flash messages expire, they now expire on use rather than automatically, useful for ESI.
  - Introduced `AutoExpireFlashBag` (default) which replicates the old automatic expiry behaviour of flash messages.

__Session Storage:__
  - Introduced a base object, `AbstractSessionStorage` for session storage drivers
  - Introduced a `SessionSaveHandlerInterface` when using custom session save handlers
  - Introduced a `NullSessionStorage` driver which allows for unsaved sessions
  - Introduced new session storage drivers for Memcache and Memcached
  - Introduced new session storage drivers for PHP's native SQLite, Memcache and Memcached support

__General:__
  - Fixed bugs where session attributes are not saved and all cases where flash messages would get lost
  - Wrote new tests, refactored related existing tests and increased test coverage extensively.

__Rationale/Details__

I'll explain more detail in the following sections.

__Unit Tests__

All unit and functional tests pass.

__Note on Functional Testing__

I've introduced `MockFileSessionStorage` which replaces `FilesystemSessionStorage` to emulate a PHP session for functional testing.  Essentially the same methodology of functional testing has been maintained but without interrupting the other session storage drivers interaction with real PHP sessions.  The service is now called `session.storage.mock_file`.

__Session Workflow__

PHP sessions follow a specific workflow which is not being followed by the current session management implementation and is responsible for some unpredictable bugs and behaviours.

Basically, PHP session workflow is as follows: `open`, `read`, `write`, `close`.  In between these can occur, `destroy` and `garbage collection`.  These actions are handled by `session save handlers` and one is always registered in all cases.  By default, the `files` save handler (internally to PHP) is registered by PHP at the start of code execution.

PHP offers the possibility to change the save handler to another internal type, for example one provided by a PHP extension (think SQLite, Memcache etc), or you can register a user type and set your own handlers.  However __in all cases__ PHP requires the handlers.

The handlers are called when the following things occur:

  - `open` and `read` when `session_start()` or the session autostarts when PHP outputs some display
  - `destroy` when `session_regenerate_id(true)` is called
  - `write` and `close` when PHP shuts down or when `session_write_close()` is called
  - `garbage collection` is called randomly according to configurable probability

The next very important aspect of this PR is that `$_SESSION` plays an important part in this workflow because the contents of the $_SESSION is populated __after__ the `read` handler returns the previously saved serialised session data.  The `write` handler is sent the serialised `$_SESSION` contents for save.  Please note the serialisation is a different format to `serialize()`.

For this reason, any session implementation cannot get rid of using `$_SESSION`.

I wrote more details on this issue [here](https://github.com/symfony/symfony/issues/2607#issuecomment-2858300)

In order to make writing session storage drivers simple, I created a light base class `AbstractSessionStorage` and the `SessionSaveHandlerInterface` which allows you to quickly write native and custom save handler drivers.

__Flash Messages [BC BREAK]__

Flash messages currently allow representation of a single message per `$name`.  Fabien designed the original system so that `$name` was equivalent to flash message type.  The current PR changes the fact that Flash messages expire explicitly when retrieved for display to the user as opposed to immediately on the next page load.

The last issue fixes potential cases when flash messages are lost due to an unexpected intervening page-load (an error for example).  The API `get()` has a flag which allows you to override the `clear()` action.

__Flash message translation__

This PR does not cover translation of flash messages because  messages should be translated before calling the flash message API.  This is because flash messages are used to present messages to the user after a specific action, and in any case, immediately on the next page load.  Since we know the locale of the request in every case we can translate the message before storing.  Secondly, translation is simply a string manipulation.  Translation API calls should always have the raw untranslated string present because it allows for extraction of translation catalogs.  For a complete answer see my answer [here](https://github.com/symfony/symfony/pull/2543#issuecomment-2858707)

__Session attribute and structured namespacing__

__This has been implemented without changing the current default behaviour__ but details are below for the alternative:

Attributes are currently stored in a flat array which limits the potential of session attributes:

Here are some examples to see why this 'structured namespace' methodology is extremely convenient over using a flat system.  Let's look at an example with csrf tokens.  Let's say we have multiple csrftokens stored by form ID (allowing multiple forms on the page and tabbed browsing).

If we're using a flat system, you might have

    'tokens' => array('a' => 'a6c1e0b6',
                      'b' => 'f4a7b1f3')

With a flat system when you get the key `tokens`, you will get back an array, so now you have to analyse the array.  So if you simply want to add another token, you have to follow three steps: get the session attribute `tokens`, have to add to the array, and lastly set the entire array back to the session.

    $tokens = $session->get('tokens');
    $tokens['c'] = $value;
    $session->set('tokens', $tokens);

Doable, but you can see it's pretty long winded.

With structured namespacing you can simply do:

    $session->set('c', $value, '/tokens');

There are several ways to implement this, either with an additional `$namespace` argument, or by treating a character in the `$key` as a namespacer.  `NamespacedAttributeBag` treats `/` as a namespacer so you can represent `user.tokens/a` for example.  The namespace character is configurable in `NamespacedAttributeBag`.

---------------------------------------------------------------------------

by marijn at 2011-12-18T15:43:17Z

I haven't read the code yet but the description from this PR and your line of thought seem very well structured.
Seems like a big +1 for me.

---------------------------------------------------------------------------

by lsmith77 at 2011-12-19T16:01:19Z

@deviantintegral could you look over this to see if it really addresses everything you wanted with PR #2510 ?

---------------------------------------------------------------------------

by deviantintegral at 2011-12-24T20:12:03Z

I've read through the documentation and upgrade notes, and I can't see anything that's obviously missing from #2510. Being able to support multiple flashes per type is the most important, and the API looks reasonable to me. Drupal does support supressing repeat messages, but that can easily be implemented in our code unless there's a compelling case for it to be a part of Symfony.

I wonder if PHP memcache support is required in Symfony given the availability of memcached. I'm not familiar with how other parts of Symfony handle it, but there is often quite a bit of confusion between the two PHP extensions. It could be simpler to remove one, or add a bit of info describing or linking to why there are two nearly identical classes.

Is it possible to make one class inherit from the other (memcached is a child of memcache)?

---------------------------------------------------------------------------

by Fristi at 2011-12-24T20:29:46Z

Interesting, maybe add: session events as I did with the current impl: https://github.com/Fristi/SessionBundle

---------------------------------------------------------------------------

by drak at 2011-12-25T00:50:03Z

@deviantintegral - I agree about the confusion between memcache and memcached but actually, it is necessary to support both because `memcached` is not available everywhere.  For example on Debian Lenny and RHEL/CentOS 5, only memcache is available by default.  This would preclude a massive amount of shared hosting environments.  Also, it is not possible to inherit one from the other, they are completely different drivers.

@Fristi - I also thought about the events, but they do not belong as part of the standalone component as this would create a coupling to the event dispatcher.  The way you have done it, ie, in a bundle is the right way to achieve it.

---------------------------------------------------------------------------

by matheo at 2011-12-25T01:12:00Z

Impressive work, looks like a big improvement and deserves a big +1

---------------------------------------------------------------------------

by datiecher at 2011-12-26T11:57:12Z

Took some time to grok all the changes in this PR but all in all it is a keeper. Specially the new flash message API, it's really nicer to work with it then the previous one.

Nicely done @drak!

---------------------------------------------------------------------------

by lsmith77 at 2012-01-02T15:00:00Z

@fabpot did you have time to review this yet? with all the work @drak has done its important that he gets some feedback soon. its clear this PR breaks BC in ways we never wanted to allow. but i think this PR also clearly explains why its necessary none the less.

---------------------------------------------------------------------------

by drak at 2012-01-02T15:41:53Z

@fabpot - I have removed the WIP status from this PR now and rebased against the current master branch.

---------------------------------------------------------------------------

by Tobion at 2012-01-07T07:13:38Z

From what I read from the IRC chat logs, the main concern of @fabpot is whether we really need multiple flash messages per type. I'm in favor of this PR and just want to add one point to this discussion.
At the moment you can add multiple flash messages of different type/category/identifier. For example you can specify one error message and one info message after an operation. I think most agree that this can be usefull.
But then it makes semantically no sense that you currently cannot add 2 info messages. This approach feels a bit half-done.
So I think this PR eliminates this paradox.

---------------------------------------------------------------------------

by drak at 2012-01-07T09:11:07Z

For reference there is also a discussion started by @lsmith77 on the mailing list at https://groups.google.com/forum/#!topic/symfony-devs/cy4wokD0mQI

---------------------------------------------------------------------------

by dlsniper at 2012-01-07T16:02:15Z

@drak I could also add the next scenario that I currently have to live with, in addition to @lsmith77 ones.

I had this issue while working on our shopping cart implementation for a customer where the customer wanted to show the unavailability of the items as different lines in the 'flash-error' section of the cart. We had to set an array as the 'flash message' in order to display that information.

So in this case for example having the flash messages types as array would actually make more sense that sending an array to the flasher. Plus the the other issue we had was that we also wanted to add another error in the message but we had to do a check to see if the flash message is an array already or we need to make it an array.

I think it's better not to impose a limit of this sort and let the users be able to handle every scenario, even if some are rare, rather that forcing users to overcome limitations such as these.

I really hope this PR gets approved faster and thanks everyone for their hard work :)

---------------------------------------------------------------------------

by Tobion at 2012-01-07T21:01:07Z

@dlsniper I think you misinterpreted my point.

---------------------------------------------------------------------------

by dlsniper at 2012-01-07T21:04:04Z

@Tobion I'm sorry I did that, I'll edit the message asap. Seems no sleep in 26 hours can cause brain not to function as intended :)

---------------------------------------------------------------------------

by lsmith77 at 2012-02-01T14:38:52Z

FYI the drupal guys are liking this PR (including the flash changes):
http://drupal.org/node/335411

---------------------------------------------------------------------------

by drak at 2012-02-01T14:51:33Z

@lsmith77 Fabien asked me to remove the changes to the flash messages so that they are as before - i.e. only one flash per name/type /cc @fabpot

---------------------------------------------------------------------------

by fabpot at 2012-02-01T14:58:23Z

To be clear, I've asked to split this PR in two parts:

 * one about the session refactoring (which is non-controversial and should be merged ASAP)
 * this one with only the flash refactoring

---------------------------------------------------------------------------

by drak at 2012-02-02T11:29:26Z

@fabpot this is ready to be merged now.  I will open a separate PR later today for the flash messages as a bucket.

---------------------------------------------------------------------------

by fabpot at 2012-02-02T11:34:39Z

I must have missed something, but I still see a lot of changes related to the flash messages.

---------------------------------------------------------------------------

by drak at 2012-02-02T11:39:10Z

When I spoke to you you said you wanted to make the commit with flash messages with one message per name/type rather than multiple.  The old flash messages behaviour is 100% maintained in `AutoExpireFlashBag` which can be the default in the framework if you wish.  The `FlashBag` implementation makes Symfony2 ESI compatible.

---------------------------------------------------------------------------

by stof at 2012-02-02T11:47:38Z

@drak splitting into 2 PRs means you should not refactor the flash messages in this one but in the dedicated one.

---------------------------------------------------------------------------

by drak at 2012-02-02T12:29:43Z

@stof Yes. I discussed with Fabien over chat there are basically no changes
to flashes in `FlashBag` and `AutoExpireFlashBag` maintains the exact
behaviour as before.  The FlashBag just introduces ESI compatible flashes.
 There is no way to refactor the sessions without moving the flash messages
to their own bag.  The next PR will propose the changes to flashes that
allow multiple messages per name/type.  I can size the PR down a little
more removing the new storage drivers and so on to make the PR smaller but
that's really as far as I can go.  To be clear, while the API has changed a
little for flashes, the behaviour is the same.
2012-02-11 23:58:28 +01:00
Fabien Potencier
74ccf7062a reverted 5b7ef11650 (Simplify session
storage class names now we have a separate namespace for sessions)
2012-02-11 12:04:50 +01:00
Fabien Potencier
04942502a5 removed unused use statements 2012-02-11 11:53:03 +01:00
Fabien Potencier
7878a0a11a [HttpFoundation] renamed pop() to all() and getAll() to all() 2012-02-11 11:53:00 +01:00
Thomas Chmielowiec
dbaddbb7a8 [Form] Allow empty choices array for ChoiceType 2012-02-11 10:04:40 +01:00
Drak
0d2745f750 [HttpFoundation] Remove constants from FlashBagInterface
As requested by fabpot.
Corrected a few mistakes in the documentation.
2012-02-11 11:24:43 +05:45
Drak
dad60efccc [HttpFoundation] Add back get defaults and small clean-up.
Changed read-only method names from get*() to peek*()

Typo
2012-02-11 11:24:39 +05:45
Drak
5b7ef11650 [HttpFoundation] Simplify session storage class names now we have a separate namespace for sessions. 2012-02-11 11:24:35 +05:45
Drak
27530cbb1e [HttpFoundation] Moved session related classes to own sub-namespace. 2012-02-11 11:24:31 +05:45
Drak
468391525a [HttpFoundation] Free bags from session storage and move classes to their own namespaces. 2012-02-11 11:24:26 +05:45
Drak
398acc9e9f [HttpFoundation] Reworked flashes to maintain same behaviour as in Symfony 2.0 2012-02-11 11:24:15 +05:45
Drak
f98f9ae8ff [HttpFoundation] Refactor for DRY code.
Rename ArraySessionStorage to make it clear the session is a mock for testing purposes only.
Has BC class for ArraySessionStorage
Added sanity check when starting the session.
Fixed typos and incorrect php extension test method
session_module_name() also sets session.save_handler, so must use extension_loaded() to check if module exist
or not.
Respect autostart settings.
2012-02-11 11:24:11 +05:45
Drak
669bc96c7f [HttpFoundation] Added pure Memcache, Memcached and Null storage drivers. 2012-02-11 11:21:22 +05:45
Drak
e185c8d63b [HttpFoundation] Refactored component for session workflow. 2012-02-11 11:21:18 +05:45
Drak
85b5c43c7a [HttpFoundation] Added drivers for PHP native session save handlers, files, sqlite, memcache and memcached. 2012-02-11 11:21:14 +05:45
Drak
57ef984e95 [HttpFoundation] Added unit and functional testing session storage objects. 2012-02-11 11:21:10 +05:45
Drak
3a263dc088 [HttpFoundation] Introduced session storage base class and interfaces.
Session object now implements SessionInterface to make it more portable.

AbstractSessionStorage and SessionSaveHandlerInterface now makes implementation
of session storage drivers simple and easy to write for both custom save handlers
and native php save handlers and respect the PHP session workflow.
2012-02-11 11:21:06 +05:45
Drak
c9694237d2 [HttpFoundation] Added FlashBagInterface and concrete implementation.
This commit outsources the flash message processing to it's own interface.

Overall flash messages now can have multiple flash types and each type can
store multiple messages.  For convenience there are now four flash types
by default, INFO, NOTICE, WARNING and ERROR.

There are two concrete implementations: one preserving the old behaviour of
flash messages expiring exactly after one page load, regardless of being
displayed or not; and the other where flash messages persist until explicitly
popped.
2012-02-11 11:21:02 +05:45
Drak
39288bcdaa [HttpFoundation] Added AttributesInterface and AttributesBagInterface and concrete implementations.
This commit outsources session attribute storage to it's own class.
There are two concrete implementations, one with structured namespace storage and the other
without.
2012-02-11 11:20:58 +05:45
Fabien Potencier
92cb685ebc fixed CS 2012-02-10 13:35:11 +01:00
Fabien Potencier
5ca472bd45 merged branch vicb/profiler/routing (PR #3283)
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)
2012-02-10 13:31:26 +01:00
Fabien Potencier
be2e67c1ab merged branch bschussek/issue3288 (PR #3290)
Commits
-------

22c8f80 [Form] Fixed issues mentioned in the PR comments
3b1b570 [Form] Fixed: The "date", "time" and "datetime" types can be initialized with \DateTime objects
88ef52d [Form] Improved FormType::getDefaultOptions() to see default options defined in parent types
b9facfc [Form] Removed undefined variables in exception constructor

Discussion
----------

[Form] Fixed: "date", "time" and "datetime" fields can be initialized with \DateTime objects

Bug fix: yes
Feature addition: yes
Backwards compatibility break: **yes**
Symfony2 tests pass: yes
Fixes the following tickets: #3288
Todo: -

![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue3288)

Fixed exception that was thrown when doing:

    $builder->add('createdAt', 'date', array('data' => new \DateTime()));

On a side note, the options passed to `FieldType::getDefaultOptions` now always also contain the default options of any parent types. This is necessary if you want to be independent of how `getDefaultOptions` is implemented in the parent type and still rely on the already defined values.

As a result, `FieldType::getParent` doesn't see default options anymore. This shouldn't be a big problem, because this method only relies on options in few cases. If it does, options now need to be checked for existence with `isset` before being used (BC break).

---------------------------------------------------------------------------

by bschussek at 2012-02-09T16:14:46Z

@fabpot Ready to merge.

---------------------------------------------------------------------------

by bschussek at 2012-02-10T12:15:04Z

@fabpot Ready to merge
2012-02-10 13:16:49 +01:00
Fabien Potencier
78e6a2f734 merged branch bschussek/issue3293 (PR #3315)
Commits
-------

da2447e [Form] Fixed MergeCollectionListener when used with a custom property path
b56502f0 [Form] Added getParent() to PropertyPath
7e5104e [Form] Fixed MergeCollectionListener for the case that the form's data is updated by the data mapper (as happening in CollectionType)

Discussion
----------

[Form] Fixed MergeCollectionListener for the case that the form's data is updated by the data mapper

Bug fix: yes
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #3293, #1499
Todo: -

![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue3293)

This fixes CollectionType to properly use adders and removers. Apart from that, adders and removers now work with custom property paths. PropertyPath was extended for a method `getParent`.

---------------------------------------------------------------------------

by bschussek at 2012-02-10T07:42:13Z

@fabpot Ready to merge.
2012-02-10 13:02:02 +01:00
Fabien Potencier
c614be74c8 merged branch bschussek/issue2308 (PR #3320)
Commits
-------

0a4519d [Form] Fixed duplicate errors on forms with "error_bubbling"=false

Discussion
----------

[Form] Fixed duplicate errors on forms with "error_bubbling"=false

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #2308
Todo: -

![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue2308)
2012-02-10 12:59:38 +01:00
Bernhard Schussek
0a4519d103 [Form] Fixed duplicate errors on forms with "error_bubbling"=false 2012-02-10 11:41:19 +01:00
Bernhard Schussek
6a45a415a1 [Form] Fixed Form::bindRequest() when used on a form without children 2012-02-10 10:36:16 +01:00
Bernhard Schussek
da2447e118 [Form] Fixed MergeCollectionListener when used with a custom property path 2012-02-09 18:29:42 +01:00
Bernhard Schussek
b56502f023 [Form] Added getParent() to PropertyPath 2012-02-09 18:11:16 +01:00
Bernhard Schussek
7e5104e09b [Form] Fixed MergeCollectionListener for the case that the form's data is updated by the data mapper (as happening in CollectionType) 2012-02-09 18:11:10 +01:00
Bernhard Schussek
22c8f8087c [Form] Fixed issues mentioned in the PR comments 2012-02-09 17:13:33 +01:00
Fabien Potencier
11e3516583 merged branch blogsh/dynamic_group_sequence (PR #3199)
Commits
-------

411a0cc [Validator] Added GroupSequenceProvider to changelog
815c769 [Validator] Renamed getValidationGroups to getGroupSequence
d84a2e4 [Validator] Updated test expectations
9f2310b [Validator] Fixed typos, renamed hasGroupSequenceProvider
e0d2828 [Validator] GroupSequenceProvider tests improved, configuration changed
c3b04a3 [Validator] Changed GroupSequenceProvider implementation
6c4455f [Validator] Added GroupSequenceProvider

Discussion
----------

[Validator] Added GroupSequenceProvider

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: ![](https://secure.travis-ci.org/blogsh/symfony.png?branch=dynamic_group_sequence)

As discussed in #3114 I implemented the "GroupSequenceProvider" pattern for the validator component. It allows the user to select certain validation groups based on the current state of an object. Here is an example:

    /**
     * @Assert\GroupSequenceProvider("UserGroupSequnceProvider")
     */
    class User
    {
        /**
         * @Assert\NotBlank(groups={"Premium"})
         */
        public function getAddress();

        public function hasPremiumSubscription();
    }

    class UserGroupSequenceProvider implements GroupSequenceProviderInterface
    {
        public function getValidationGroups($user)
        {
            if ($user->hasPremiumSubscription()) {
                return array('User', 'Premium');
            } else {
                return array('User');
            }
        }
    }

With this patch there are two mechanisms to define the group sequence now. Either you can use @GroupSequence to define a static order of validation groups or you can use @GroupSequenceProvider to create dynamic validation group arrays.
The ClassMetadata therefore has methods now which implement quite similar things. The question is whether it would make sense to interpret the static group sequence as a special case and create something like a DefaultGroupSequenceProvider or StaticGroupSequenceProvider which is assigned by default. This would cause a BC break inside the validator component.

---------------------------------------------------------------------------

by bschussek at 2012-01-28T13:39:54Z

I like the implementation, but I think we should differ a little bit from Java here.

1. `GroupSequenceProviderInterface` should be implemented by the domain classes themselves (`User`), not by a separate class.
2. As such, the parameter `$object` from `getValidationGroups($object)` can be removed
3. `ClassMetadata::setGroupSequenceProvider()` should accept a boolean to activate/deactivate this functionality. Also the check for the interface (does the underlying class implement it?) should be done here

Apart from that, special cases need to be treated:

* A definition of a group sequence and a group sequence provider in the same `ClassMetadata` should not be allowed. Either of them must not be set.
* Metadata loaders must take care of settings made by parent classes. If `Animal` is extended by `Dog`, `Animal` defines a group sequence (or group sequence provider) and `Dog` a group sequence provider (or group sequence), only the setting of `Dog` should apply

---------------------------------------------------------------------------

by blogsh at 2012-01-28T21:25:37Z

Changes of the latest commit:

- GroupSequenceProviderInterface has to be implemented by the domain class
- The annotation/configuration options let the user define whether the provider is activated or not (is this neccessary at all?)
- An error is thrown if the user wants to use static group sequences and the provider simultaneously

At the moment neither the static group sequence nor the provider is inherited from parent classes or interfaces. I don't know if it would make sense to enable this feature. There could be problems if a user wants to define a static group sequence in the parent class and a sequence provider in the child class.

---------------------------------------------------------------------------

by bschussek at 2012-01-30T13:07:04Z

> There could be problems if a user wants to define a static group sequence in the parent class and a sequence provider in the child class.

In this case, the setting in the child class should override the setting of the parent class.

But we can leave this open for now. As it seems, [this issue is unresolved in Hibernate as well](https://hibernate.onjira.com/browse/HV-467).

---------------------------------------------------------------------------

by blogsh at 2012-01-30T22:54:41Z

Okay, finally I managed to upload the latest commit. If you got a bunch of notifications or so I'm sorry, but I had to revert some accidental changes in the commit :(

I've rewritten the tests and have removed the "active" setting in the XML configuration.

---------------------------------------------------------------------------

by blogsh at 2012-02-02T15:24:01Z

Okay, typos are fixed now and `hasGroupSequenceProvider` has been renamed to `isGroupSequenceProvider`. I also had to adjust some tests after the rebase with master.

---------------------------------------------------------------------------

by bschussek at 2012-02-03T09:25:19Z

Looks good.

@fabpot 👍

---------------------------------------------------------------------------

by fabpot at 2012-02-03T09:46:52Z

Can you add a note in the CHANGELOG before I merge? Thanks.

---------------------------------------------------------------------------

by blogsh at 2012-02-09T12:31:27Z

@fabpot done
2012-02-09 13:39:47 +01:00
Bernhard Schussek
3b1b57030b [Form] Fixed: The "date", "time" and "datetime" types can be initialized with \DateTime objects 2012-02-07 11:10:02 +01:00
Victor Berchet
e3cf37fe84 [HttpFoundation] RedirectResponse: add the ability to retrieve the target URL, add unit tests 2012-02-06 19:09:24 +01:00
Fabien Potencier
8c11e9baa7 merged branch thesalla/mapfiledumper (PR #2471)
Commits
-------

ba3b321 [ClassLoader] Added a class map file generator utility

Discussion
----------

[ClassLoader] Added a class map file generator utility

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #2407

---------------------------------------------------------------------------

by lsmith77 at 2011-11-30T19:33:52Z

@thesalla do you have time/interest to finish this up?

---------------------------------------------------------------------------

by thesalla at 2011-12-01T09:55:50Z

sure (will attempt at least), but no sooner than tomorrow

---------------------------------------------------------------------------

by stof at 2011-12-12T20:41:41Z

@thesalla @fabpot what is the state of this PR ?

---------------------------------------------------------------------------

by thesalla at 2011-12-17T19:36:58Z

If you don't have any other suggestions or ideas, I'd consider it finished.

---------------------------------------------------------------------------

by lsmith77 at 2012-01-24T09:27:12Z

ping

---------------------------------------------------------------------------

by fabpot at 2012-02-02T08:21:38Z

@thesalla: Can you rename the ClassMapDumper class to ClassMapGenerator and then squash your commit before the merge? Thanks.

---------------------------------------------------------------------------

by thesalla at 2012-02-04T12:55:48Z

@fabpot done.
2012-02-06 17:45:10 +01:00
Fabien Potencier
9855886dc5 merged branch grizlik/master (PR #3261)
Commits
-------

b65a997 [Form] Added test ChoiceList::testGetChoicesForValuesCorrectOrderingOfResult for correct ordering check
a60daff [Form] Fixed incorrect sorting in ChoiceList::getChoicesForValues

Discussion
----------

[Form] Fixed incorrect sorting in ChoiceList::getChoicesForValues

ChoiceList::getChoicesForValues return collection with sorting, that not corresponding to the sorting for input keys

---------------------------------------------------------------------------

by bschussek at 2012-02-03T21:40:49Z

Can you adapt the test cases to fail for this case please? (probably by use of assertSame instead of assertEquals in one of the existing tests)

---------------------------------------------------------------------------

by grizlik at 2012-02-04T08:41:44Z

I need to create a new test or modify an existing one?

---------------------------------------------------------------------------

by bschussek at 2012-02-04T09:58:04Z

Something like this in ChoiceListTest:

    public function testGetChoicesForValuesReverseOrderedValues()
    {
        $values = array('2', '1');
        $this->assertSame(array($this->obj3, $this->obj2), $this->list->getChoicesForValues($values));
    }
2012-02-06 09:56:24 +01:00
Fabien Potencier
c75ca12140 merged branch bschussek/issue3269 (PR #3270)
Commits
-------

78c1451 [Validator] Throwing exception in ConstraintValidator::setMessage() if initialize() wasn't called

Discussion
----------

[Validator] Throwing exception in ConstraintValidator::setMessage() if initialize() wasn't called

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -

![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue3269)

---------------------------------------------------------------------------

by Maks3w at 2012-02-05T09:52:21Z

One problem was that. Now I initialized the unit test with a execution
context mockup and works good.

Another problem I found is in the rules for upgrade from 2.0 to
2.1:

In v2.0: with setMessages() only one argument is mandatory ($message)
In v2.1: With the new way, addViolation(), then you need 2 arguments.
But, in v2.0 with the new way, addViolation, required 3 arguments.

So for preserve forward and backward compatibility, you need pass 3
arguments to addViolation()  I think that it is helpfull and could be written in the
doc.

At the last one ¿What is the purpose of the third argument? ¿Can I pass
any object as invalid value?

Thanks

---------------------------------------------------------------------------

by bschussek at 2012-02-05T10:45:16Z

The last argument should always be the validated value that caused the constraint to fail.
2012-02-06 09:53:04 +01:00
William DURAND
007de8c265 [Tests] [Propel] Added some tests for the ModelChoiceList class 2012-02-05 23:08:07 +01:00
Bernhard Schussek
78c145196b [Validator] Throwing exception in ConstraintValidator::setMessage() if initialize() wasn't called 2012-02-05 10:03:22 +01:00
Gyula Sallai
ba3b321842 [ClassLoader] Added a class map file generator utility
fixed cs

Small refactoring for Finder support

If class name found, return

Find multiple classes and namespaces in the same file

fixed problems with inheritance and non-php files

Renamed ClassMapDumper to ClassMapGenerator

fixed error with splfileinfo
2012-02-04 13:48:25 +01:00
grizlik
b65a9972fb [Form] Added test ChoiceList::testGetChoicesForValuesCorrectOrderingOfResult for correct ordering check 2012-02-04 14:14:29 +04:00
Fabien Potencier
6e60967827 [DoctrineBridge] fixed a unit test after the 2.0 merge 2012-02-04 08:08:27 +01:00
Fabien Potencier
b1148e334f merged 2.0 2012-02-04 08:03:45 +01:00
Fabien Potencier
5251177002 merged branch helmer/readonly_fix (PR #3258)
Commits
-------

8321593 [Form] DRYed ChoiceType
0753cee [Form] Fixed read_only attribute for expanded fields

Discussion
----------

[Form] Fixed read_only attribute for expanded fields

Expanded choice widgets lose the knowledge of read_only attribute value.

Fixes bug introduced by #3193

---------------------------------------------------------------------------

by helmer at 2012-02-02T16:24:50Z

Please hold before merging, @bschussek had some thoughts about my changes in ``ChoiceType``, waiting for feedback.

---------------------------------------------------------------------------

by bschussek at 2012-02-02T16:33:12Z

I'm fine with the refactoring then, but please split it into two commits at least. The changes in ChoiceType have nothing in common with the actual issue here.

---------------------------------------------------------------------------

by helmer at 2012-02-02T19:40:39Z

Tests added.

---------------------------------------------------------------------------

by bschussek at 2012-02-03T10:14:32Z

Great, thanks.

@fabpot 👍
2012-02-04 07:59:57 +01:00
Helmer Aaviksoo
0753cee11a [Form] Fixed read_only attribute for expanded fields 2012-02-03 11:49:06 +02:00
Sebastian Hörl
815c769292 [Validator] Renamed getValidationGroups to getGroupSequence 2012-02-02 20:27:50 +01:00
Sebastian Hörl
d84a2e4bdf [Validator] Updated test expectations 2012-02-02 20:27:50 +01:00
Sebastian Hörl
9f2310b2f8 [Validator] Fixed typos, renamed hasGroupSequenceProvider 2012-02-02 20:27:50 +01:00
Sebastian Hörl
e0d28284fc [Validator] GroupSequenceProvider tests improved, configuration changed 2012-02-02 20:27:50 +01:00