8000 [HttpFoundation] MongoDbSessionHandler supports auto expiry via configurable expiry_field by catchamonkey · Pull Request #11510 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[HttpFoundation] MongoDbSessionHandler supports auto expiry via configurable expiry_field #11510

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 11 commits into from
Closed

[HttpFoundation] MongoDbSessionHandler supports auto expiry via configurable expiry_field #11510

Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
bb336a5
[HttpFOundation] Added support for auto expiry in MongoSbSessionHandler
catchamonkey Jul 29, 2014
796b18a
Fixes an issue in the mongo handler where the wrong option was used f…
catchamonkey Jul 29, 2014
2d8b2bd
[HttpFoundation] Fixes unit test following the recent update
catchamonkey Jul 29, 2014
4d42f32
Fixes usage of short array syntax to support 5.3
catchamonkey Jul 29, 2014
cb5e562
Use strict comparisons on boolean checks
catchamonkey Jul 29, 2014
d748743
Switched to using yoda expressions
catchamonkey Jul 30, 2014
2f8742e
Return early from the gc method when expiry indexes are used
catchamonkey Aug 1, 2014
fc962db
Modifies the gc tests to use positive values
catchamonkey Aug 1, 2014
b55c63b
Now longer creating the index during write, but have documented it
catchamonkey Aug 4, 2014
88b256c
Corrects error in comment
catchamonkey Aug 4, 2014
9744f61
Removes will and with expectations from method tests that are never c…
catchamonkey Aug 11, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Use strict comparisons on boolean checks
  • Loading branch information
@catchamonkey
catchamonkey committed Jul 29, 2014
commit cb5e5625dfddbddb2050ed3b6c06a425e6b441e8
4 changes: 2 additions & 2 deletions src/Symfony/Component/HttpFoundation/Session/Storage/Handler/MongoDbSessionHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ public function gc($maxlifetime)
*
* See: http://docs.mongodb.org/manual/tutorial/expire-data/
*/
if ($this->options['expiry_field'] == false) {
if ($this->options['expiry_field'] === false) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should use Yoda expressions:

false === $this->options['expiry_field']

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, will modify.

$time = new \MongoDate(time() - $maxlifetime);

$this->getCollection()->remove(array(
Expand All @@ -132,7 +132,7 @@ public function write($sessionId, $data)
$this->options['time_field'] => new \MongoDate(),
);

if ($this->options['expiry_field'] != false) {
if ($this->options['expiry_field'] !== false) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

$expiry = new \MongoDate(time() + (int) ini_get('session.gc_maxlifetime'));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Tobion: In #11508 (comment), you mentioned that the session lifetime option in Symfony sets the session.gc_maxlifetime INI value. Do you have a reference for this? I couldn't find it.

One concern I have is that we're storing the option's value from a particular point in time in this calculated value. If the PHP configuration changes because session lifetimes need to be extended site-wide, this storage layer is still going to delete those records based on the original time. I would assume other storage layers would simply delay their gc() step and allow session data to persist. That seems like a caveat of using TTL indexes with expireAfterSeconds of 0 and not relying on PHP for garbage collection.

Just for context, ZF's MongoDB session handler stores the lifetime (i.e. session.gc_maxlifetime) with each write. That implementation requires that the session lifetime is kept in sync with the TTL index's expireAfterSeconds option, but it ensures that behavior between MongoDB's deletion routine and PHP's garbage collection are consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the PHP configuration changes because session lifetimes need to be extended site-wide, this storage layer is still going to delete those records based on the original time.

But only for records that have not had activity, as they will have the new expireAt set based on whatever the maxlifetime may have been changed to.

Not sure how much of an issue this actually is.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But only for records that have not had activity, as they will have the new expireAt set based on whatever the maxlifetime may have been changed to.

Good point! I missed that in my initial comment. No objection then, provided Symfony is OK with reading the session.gc_maxlifetime INI value here (I'm still curious what @Tobion was referring to).

$fields[$this->options['expiry_field']] = $expiry;

Expand Down
0