Commit Graph

8505 Commits

Author SHA1 Message Date
Fabien Potencier
3e70646a67 merged branch drak/session_interface (PR #3342)
Commits
-------

137b002 [HttpFoundation] Make SessionHandlerInterface compatible with PHP 5.4's SessionHandlerInterface

Discussion
----------

[HttpFoundation] Make Sessions compatible with PHP 5.4's SessioHandlerInterface

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
2012-02-13 07:10:21 +01:00
Drak
137b0026b7 [HttpFoundation] Make SessionHandlerInterface compatible with PHP 5.4's SessionHandlerInterface 2012-02-13 11:48:55 +05:45
Fabien Potencier
a375c1289e merged branch drak/session_interface (PR #3342)
Commits
-------

388a9c2 [HttpFoundation] Make SessionHandlerInterface compatible with PHP 5.4's SessionHandlerInterface

Discussion
----------

[HttpFoundation] Make Sessions compatible with PHP 5.4's SessioHandlerInterface

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
2012-02-13 07:02:01 +01:00
Drak
388a9c2861 [HttpFoundation] Make SessionHandlerInterface compatible with PHP 5.4's SessionHandlerInterface 2012-02-13 11:35:29 +05:45
Grégoire Pineau
e5edf5ab86 [Console] Fixed CS 2012-02-13 00:29:28 +01:00
Grégoire Pineau
8abf50639a [Console] Added abbreviation into search for bad command / namespace 2012-02-13 00:16:28 +01:00
Fabien Potencier
deb22470f6 merged branch kbond/config-dump-changelog (PR #3339)
Commits
-------

c02c959 updated changelog with config:dump-reference command

Discussion
----------

updated changelog with config:dump-reference command
2012-02-12 17:51:49 +01:00
Kevin Bond
c02c959aae updated changelog with config:dump-reference command 2012-02-12 10:19:38 -05:00
Fabien Potencier
7401338e46 merged branch drak/session_memcached_tests (PR #3338)
Commits
-------

cab1060 [HttpFoundation] Add tests for session memcache/d storage drivers.

Discussion
----------

[HttpFoundation] Add tests for session memcache/d storage drivers.

Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
2012-02-12 15:52:42 +01:00
Drak
cab1060a76 [HttpFoundation] Add tests for session memcache/d storage drivers. 2012-02-12 20:08:50 +05:45
Kevin Bond
cea2c7e60f removed unneeded local variable 2012-02-12 08:57:10 -05:00
Fabien Potencier
fc7d0110f7 [HttpFoundation] removed Serializable from SessionInterface
If you need to serialize the session, you need to get the bags and
serialize them instead.
2012-02-12 14:51:23 +01:00
Fabien Potencier
574f2542df [HttpKernel] fixed flashes in the request data collector 2012-02-12 14:51:17 +01:00
Fabien Potencier
8365675fc2 fixed exception in the profile/WDT when no route matches 2012-02-12 13:51:16 +01:00
Fabien Potencier
1e79e30c7b added back flashes into the request data collector (and in the web profiler) 2012-02-12 13:37:15 +01:00
Fabien Potencier
e43e14987f updated CHANGELOG for 2.1 2012-02-12 13:29:30 +01:00
Fabien Potencier
e986b9b7e5 merged branch pulzarraider/memcache_profiler_storage (PR #2766)
Commits
-------

7474293 memcache profiler storage support added

Discussion
----------

[HttpKernel] [FrameworkBundle] Memcache(d) Profiler Storage added

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

There are 2 memcache PHP extensions: Memcache and MemcacheD (with "D" at the end) - both are supported.

How to use Memcache Profiler Storage (Memcache php extension is used):
change (or add if there isn't) "dsn" in framework/profiler section in config_dev.yml

```
...
framework:
    ...
    profiler:
        ...
        dsn: memcache://127.0.0.1/11211
...
```

How to use Memcached Profiler Storage (MemcacheD php extension is used):
change "dsn" in framework/profiler section in config_dev.yml

```
...
framework:
    ...
    profiler:
        ...
        dsn: memcached://127.0.0.1/11211
...
```

Last changes:
- memcached support addedd
- optimized performance (serialization done in extension, index is created with ```append``` function)
- updated to last version of Profiler (find by method, avoid duplications)
- done squash on commits

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

by stloyd at 2011-12-01T23:36:02Z

You need to add check for index name size, AFAIK memcache will fail if key is longer than 250 characters.

Also please do an `squash` for all those commits.

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

by pulzarraider at 2011-12-02T00:15:28Z

@stloyd Thanks. I will add the check for key length.

I am just starting with git. Could you please add some tutorial about squash to a documentation page: http://symfony.com/doc/2.0/contributing/code/patches.html ? It will help me (and maybe some others) to do it correct way.

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

by stof at 2011-12-02T00:19:01Z

http://help.github.com/rebase/

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

by pulzarraider at 2011-12-03T18:56:11Z

Thanks @stof, rebase done.

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

by dlsniper at 2011-12-11T14:00:17Z

Hi,

Would it be possible to either use Memcached instead of Memcache or make it configurable to use either Memcache or Memcached?
I've did a little digging on the benefits of using Memcached over Memcache (like for example: http://stackoverflow.com/questions/1442411/using-memcache-vs-memcached-with-php http://devzone.zend.com/1869/zendcon-sessions-episode-040-memcached-the-better-memcache-interface/ ) and maybe this will also help in not having two extensions installed for people who are using Memcached already.

Regards.

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

by pulzarraider at 2011-12-11T16:15:58Z

@dlsniper  thanks for great comment. I will add memcached support.

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

by stof at 2011-12-12T20:49:00Z

@pulzarraider what is the status of this PR ? Is it still a WIP ?

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

by pulzarraider at 2011-12-12T22:58:48Z

@stof Yes, it's still WIP. I'm working on a memcached (with D at the end) support. It will be finished in the next few days.

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

by dlsniper at 2011-12-15T12:51:52Z

@pulzarraider if I can help you with the PR let me know.

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

by pulzarraider at 2012-01-08T20:22:24Z

@dlsniper @stof I've finally added memcached support and done some optimizations. Memcache(d) profiler storage is now ready.

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

by dlsniper at 2012-01-08T22:12:29Z

I'm glad you finished this @pulzarraider
Thanks! for your hard work!

+1 for this PR

@stof, @fabpot is it good to go on master?

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

by pulzarraider at 2012-01-28T19:45:56Z

@stof, @fabpot ping
2012-02-12 13:26:06 +01:00
Fabien Potencier
ebe0ebba63 merged branch yethee/upgrade (PR #3337)
Commits
-------

04919a2 Fixed formatting in UPGRADE file

Discussion
----------

Fixed formatting in UPGRADE file
2012-02-12 13:18:00 +01:00
Deni
04919a2978 Fixed formatting in UPGRADE file 2012-02-12 16:14:56 +04:00
Fabien Potencier
7995b80bad merged branch vicb/profiler.terminate (PR #3223)
Commits
-------

3dd3d58 [EventListener] Fix an issue with sub-requests
71bf279 cleanup
acdb325 [StopWatch] Provide a cleaner API
acd1287 [Stopwatch] rename the section event to avoid collisions
eb540be [Profiler] Allow profiling the terminate event
4ccdc53 [HttpKernel] Cleanup of PdoProfilerStorage
814876f [HttpKernel] Tweak the code of the ProfilerListener

Discussion
----------

[Profiler] Allow profiling the terminate event

![Travis](https://secure.travis-ci.org/vicb/symfony.png?branch=profiler.terminate)

This PR is mainly about allowing to profile the terminate event (i.e. see it in the timeline panel)

There are some other tweaks.

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

by vicb at 2012-02-02T14:43:20Z

please don't merge for now. good question. bad answer.

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

by vicb at 2012-02-06T15:05:46Z

While first commits were focused on problem solving, the last brings a clean API with the ability to re-open an existing section in order to add events (re-setting event origins and merging them were just hacks).

Should be ready to be merged.

_Edit: Sorry, couldn't resist adding a private helper class again!_

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

by stof at 2012-02-06T18:30:09Z

@vicb you should stop adding such classes defined in the same file. Otherwise we will have to change the CS (and to stop telling we respect the PSR-0 standard)

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

by vicb at 2012-02-06T18:33:36Z

Once again PSR-0 is about autoloading which is exactly why I do not want in such cases. CS are an other matter and yes I think they should be changed to allow this (and I am going to submit a PR right now).

The only argument I could accept is whether this class should be private or not.

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

by vicb at 2012-02-06T19:57:06Z

Thanks for your valuable feedback @stof

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

by fabpot at 2012-02-11T20:53:03Z

Have you tested it on a project? Because it breaks my simple examples (where I have some sub-requests).

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

by vicb at 2012-02-12T09:47:23Z

my bad, should be ok now.
2012-02-12 13:12:18 +01:00
Fabien Potencier
8adefb55e8 merged branch yethee/profile_toolbar (PR #3336)
Commits
-------

6e9f886 [WebProfilerBundle] moved variable initialization from condition

Discussion
----------

[WebProfilerBundle] moved variable initialization from condition

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

I got fatal error for the original condition on PHP 5.3.8:

    Fatal error: Call to a member function getFlashBag() on a non-object in /home/context/httpd/vendor/symfony/src/Symfony/Bundle/WebProfilerBundle/Controller/ProfilerController.php on line 150
2012-02-12 13:10:29 +01:00
Fabien Potencier
802fe644fb merged branch stof/phpdoc (PR #3332)
Commits
-------

8e34f43 Fixed the phpdoc for the Response class

Discussion
----------

Fixed the phpdoc for the Response class

This adds the missing phpdoc for some return values.
2012-02-12 13:09:50 +01:00
Fabien Potencier
1da8deee4e merged branch drak/session_memcache_tests (PR #3335)
Commits
-------

fe870be [HttpFoundation] Added tests for memcache/d storage drivers.

Discussion
----------

[WIP][2.1][HttpFoundation] Add tests for session memcache/d storage drivers.

__[WIP] pending merge of PR 3333, no review please.__

Bug fix: no
Feature addition: no
Backwards compatibility break: no
__Symfony2 tests pass: no__
Fixes the following tickets: -
Todo: -
2012-02-12 13:08:22 +01:00
Fabien Potencier
f6ee66f75a merged branch stof/session_cleanup (PR #3333)
Commits
-------

2c767d1 [HttpFoundation] Fixed closeSession for the Memcached storage
ec44e68 [HttpFoundation] Fixed the use of the prefix for the Memcached storage
5808773 [HttpFoundation] Fixed a typo and updated the phpdoc for the session
0550bef Removed methods duplicated from parent class

Discussion
----------

Session cleanup

This cleans the phpdoc of the refactored session by using inheritdoc in all appropriate places. For the SessionInterface, I added the ``@api`` tag in all methods as the Session class had them.
It also fixes a few typos in the variable names.

I figured a few things:

- the Session class implements some methods that are not part of the SessionInterface. Is it intended or simply a left-over ?
- the MemcachedSessionStorage uses a ``prefix`` property which does not exist. This is clearly a copy-paste from the MemcacheSessionStorage which has it. It also call the ``Memcached#close`` method which does not exist according to PhpStorm. @drak could you check this class ? I don't know Memcached at all.
- as I said on the refactoring PR, the Serializable implementation of the Session class seems totally wrong as the SessionStorage is not serializable.

/cc @drak

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

by drak at 2012-02-12T02:09:38Z

@stof there is a ticket about this, the problem exists from Symfony 2.0 also - refs #3000 PDOSessionStorage

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

by drak at 2012-02-12T02:10:12Z

@stof I will look at the memcache issue and make a quick PR

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

by stof at 2012-02-12T02:11:07Z

@drak the issue could exist with any SessionStorage as your SessionStorageInterface does not enforce making it serializable

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

by stof at 2012-02-12T02:14:15Z

@drak note that this PR already fixes 2 typo in the memcached storage. It seems like some tests are missing for it as they should have been found (they would have raised a notice in the constructor)

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

by drak at 2012-02-12T02:14:50Z

Afaik, only PDO is not serializable@ but the `Serializable` is on the SessionInterface.  The problem is with #3000, the serialization is necessary but impossible.

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

by stof at 2012-02-12T02:21:42Z

@drak what about a Mongo storage or things like that ?

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

by drak at 2012-02-12T02:23:58Z

@stof Since you've started this PR would you mind removing `$prefix.` from the `*session()` methods in `MemcachedSessionStorage` - they are not required because Memcached handles prefixes itself.  I'll write some tests in another PR for them.  I guess I omitted it because at the time I didn't have them on my test env.

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

by drak at 2012-02-12T02:24:35Z

@stof - Let's take the serialization issue to #3000 because it's being discussed there.

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

by stof at 2012-02-12T02:27:27Z

@drak what about the ``close()`` method which does not exist in the Memcached class ?

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

by drak at 2012-02-12T03:58:11Z

The method just needs to `return true;`.
2012-02-12 13:06:54 +01:00
Fabien Potencier
84d9551159 merged branch drak/session_tests (PR #3334)
Commits
-------

805dd76 [HttpFoundation] Added tests for Session class.

Discussion
----------

[HttpFoundation] Added tests for Session class.

Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
2012-02-12 13:04:27 +01:00
Christophe Coevoet
2c767d163b [HttpFoundation] Fixed closeSession for the Memcached storage 2012-02-12 12:08:35 +01:00
Deni
6e9f8863af [WebProfilerBundle] moved variable initialization from condition 2012-02-12 14:50:52 +04:00
Drak
fe870beae3 [HttpFoundation] Added tests for memcache/d storage drivers. 2012-02-12 16:06:54 +05:45
Victor Berchet
3dd3d582c4 [EventListener] Fix an issue with sub-requests 2012-02-12 10:44:50 +01:00
Drak
805dd76fcd [HttpFoundation] Added tests for Session class. 2012-02-12 10:23:04 +05:45
Christophe Coevoet
ec44e6831a [HttpFoundation] Fixed the use of the prefix for the Memcached storage 2012-02-12 04:00:20 +01:00
Christophe Coevoet
8e34f43723 Fixed the phpdoc for the Response class 2012-02-12 02:04:15 +01:00
Christophe Coevoet
5808773b51 [HttpFoundation] Fixed a typo and updated the phpdoc for the session 2012-02-12 01:56:08 +01:00
Christophe Coevoet
0550bef14d Removed methods duplicated from parent class 2012-02-12 01:20:46 +01:00
Fabien Potencier
7e4f4dcdf9 merged branch bschussek/issue3022 (PR #3322)
Commits
-------

cde34fd [Form] Throwing an AlreadyBoundException in `add`, `remove`, `setParent`, `bind` and `setData` if called on a bound form

Discussion
----------

[Form] Throwing an AlreadyBoundException in `add`, `remove`, `setParent`, `bind` and `setData` if called on a bound form

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

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

The above mentioned methods now throw an exception because when invoked on a bound form they might cause strange side effects. You should rely on event listeners instead of modifying bound forms.

See also #3022
2012-02-12 00:48:39 +01:00
Fabien Potencier
79166bdd28 [FrameworkBundle] added a missing class in the compile cache 2012-02-12 00:45:48 +01:00
Fabien Potencier
77844d8cbd fixed UPGRADE file 2012-02-12 00:44:10 +01:00
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
a899548d79 merged branch willdurand/fix-propel-bridge (PR #3329)
Commits
-------

88b826d [Propel] Fixed typo, removed useless use statement, used getData() instead of casting a PropelCollection
46d28cd [Propel] Fixed the CollectionToArray transformer
1f20fb1 [Propel] Removed useless code
3910735 [Propel] Avoid to duplicate objects
d69144c [Propel] Refactored the CollectionToArray transformer
1706671 [Propel] Fixed naming to reflect Doctrine bridge
1f277df [Propel] Removed useless ModelToIdTransformer

Discussion
----------

Cleaned the propel bridge (+ fixes)

I've fixed the `ModelChoiceList` with `multiple=true`, and I removed useless code.

This PR will ensure everything works fine, but it requires the following fix for Propel: https://github.com/propelorm/Propel/pull/286.

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

by willdurand at 2012-02-11T20:04:10Z

@cedriclombardot this PR will fix your issues with Sf2 + Propel in your admingen
@bschussek nevermind my comments on Twitter, it seems ok now
2012-02-12 00:06:59 +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
William DURAND
88b826d4f6 [Propel] Fixed typo, removed useless use statement, used getData() instead of casting a PropelCollection 2012-02-11 23:16:27 +01:00
Jeremy Mikola
c754f28fcb [DoctrineBridge] Rename data fixtures loader class 2012-02-11 16:48:09 -05:00
Kevin Bond
924f378242 updated changelog 2012-02-11 16:20:40 -05:00
Jeremy Mikola
af848051ea [DoctrineBridge] Suggest doctrine/data-fixtures dependency 2012-02-11 16:15:54 -05:00
Kevin Bond
72d580594f changed route name 2012-02-11 15:24:20 -05:00
Grégoire Pineau
c6203bcffa [Console] Added namespace suggest on bad namespace name 2012-02-11 21:01:00 +01:00
Jeremy Mikola
e4243a1d32 [DoctrineBridge] Add common data fixtures loader 2012-02-11 14:17:18 -05:00
William DURAND
46d28cd55d [Propel] Fixed the CollectionToArray transformer 2012-02-11 18:52:37 +01:00
Grégoire Pineau
117359a196 [Console] fixed CS according to PR comment 2012-02-11 16:33:06 +01:00