[2.3][Session] Give greater control over how and when session starts #7576
Conversation
This allows control over how the session start on demand works 0: no start on demand when bags are accessed 1: start session if bags are accessed 2: no start on demand when bags are accessed but still return bag contents
…rt on demand. 1. Gives user control over session start on demand mode. 2. Re-introduce flag to allow session listener to manually start session.
|
note: The tests pass, but there is an error with Travis on 5.3 build segfault. |
| @@ -184,6 +184,11 @@ private function addSessionSection(ArrayNodeDefinition $rootNode) | |||
| ->info('session configuration') | |||
| ->canBeUnset() | |||
| ->children() | |||
| ->booleanNode('auto_start')->info('Flag for SessionListener to start session') | |||
| ->defaultValue(true)->end() | |||
There was a problem hiding this comment.
you should use defaultTrue().
And the indentation should be 4 spaces per level. If you find the line too long, format it like this:
->booleanNode('auto_start')
->defaultTrue()
->info('Flag for SessionListener to start session')
->end()|
Your link to the doc PR is wrong. It is linking to a doc issue, not to a doc PR. |
|
@stof - does it have to be a PR rather than an issue? I thought the idea was so that a feature will not be forgotten. the eventual PR will reference that ticket. I'd prefer not to write docs until it's merged. |
|
@stof is right: we don't merge new features anymore without proper documentation. |
|
@fabpot - OK, that's fine. If the PR is acceptable now then I'll do the docs. |
| @@ -9,6 +9,9 @@ CHANGELOG | |||
| * added `TimedPhpEngine` | |||
| * added `--clean` option the the `translation:update` command | |||
| * added `http_method_override` option | |||
| * Reintroduce `auto_start` session config flag which control to `SessionListener` to manually start sessions | |||
There was a problem hiding this comment.
It would appear working with PHP sessions for too long is rewiring my brain. I'll fix it.
…n 2.2 and make component values consistent with FrameworkBundle's configuration options.
|
@fabpot - Documentation PR completed |
| $this->start(); | ||
| } elseif (!$this->started && self::NO_START_ON_DEMAND_STRICT === $this->mode) { | ||
| throw new \RuntimeException('Cannot access session bags because the session has been started.'); |
There was a problem hiding this comment.
I think the message should be 'Cannot access session bags because the session has not been started.'
You forgot a 'not' in there. Otherwise the messages does not make sense.
As requested by @fabpot
| @@ -36,7 +36,7 @@ | |||
| protected $saveHandlerName; | |||
|
|
|||
| /** | |||
| * Gets the session.save_handler name. | |||
| * Gets the session.save_handler name | |||
There was a problem hiding this comment.
why removing trailing dot here ?
There was a problem hiding this comment.
It was an error, I was removing them from the @param blocks. I think it doesn't matter though
There was a problem hiding this comment.
Consistency does matter. You should add them back. Sorry for the trouble.
This PR was merged into the master branch. Discussion ---------- [2.3][Session] Give greater control over how and when session starts | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | na | License | MIT | Doc PR | symfony/symfony-docs#2475 Refs #6036 Gives control over how start on demand works: allowing to turn it on or off and to allow bag access when session is off. Commits ------- f431cb0 Fix tests 1f521d8 Coding standards 2583c26 [HttpFoundation][FrameworkBundle] Keep save auto_start behaviour as in 2.2 and make component values consistent with FrameworkBundle's configuration options. ceaf69b [FrameworkBundle] Use more sophisticated validation and configuration. af0a140 [FrameworkBundle] Add configuration to allow control over session start on demand. 8fc2397 [HttpFoundation] Give control over how session start on demand.
|
Re-reading the comments on #6036 (#6036 (comment)), I'm not sure that this change is wise. So, I've reverted it for now. |
Refs #6036
Gives control over how start on demand works: allowing to turn it on or off and to allow bag access when session is off.