Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[FrameworkBundle][HttpKernel] Add session usage reporting in stateless mode#35732

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.

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

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:masterfrommtarld:feature/stateless
Feb 26, 2020

Conversation

@mtarld
Copy link
Contributor

@mtarldmtarld commentedFeb 15, 2020
edited
Loading

QA
Branch?master
Bug fix?no
New feature?yes
Deprecations?no
LicenseMIT
Doc PRsymfony/symfony-docs#13344

https://github.com/orgs/symfony/projects/1#card-30506005

Provide astateless attribute that forbid session usage.

Implementations

v1

  • New session proxy that allows session to be marked as disabled
  • New default route attribute:_stateless (automatically set by@Stateless)
  • On kernel controller event, if_stateless istrue, session is marked as disabled
  • Session listener is able to check if the session is disabled and prevent its creation

v2

  • New default route attribute:_stateless (automatically set by@Stateless)
  • On kernel response, check the session usage and if session was used when_stateless attribute is set totrue: Either throw an exception (debug enabled) or log a warning (debug disabled)

v3

  • New route default_stateless (automatically set bystateless route attribute)
  • On kernel response, check the session usage and if session was used when_stateless attribute is set totrue: Either throw an exception (debug enabled) or log a warning (debug disabled)

derrabus and MatTheCat reacted with thumbs up emoji
@mtarld
Copy link
ContributorAuthor

One thing I'm wondering, should we need theDisableableSessionProxy or could we use directlySession ?

@nicolas-grekas
Copy link
Member

The request attribute is a nice idea.
For the implementation, I'd like to propose another approach: the AbstractSessionListener could check the session usage index and report back when a stateless route increments the index, wdyt?
That would not be enough to know where the session was used, but that could be improved separately when resolvinghttps://github.com/orgs/symfony/projects/1#card-30499423

@mtarld
Copy link
ContributorAuthor

Yes it could be great, I'll give it a try

@mtarld
Copy link
ContributorAuthor

I had tried this approach but something came up to me.
Using this implementation, I think that we won't be able to prevent session start and usage as we are just reporting back. Therefore maybe session data could have been accessed or updated even if we report back session usage.
Do you see ?
But maybe I didn't get fully what you wanted to mean
WDYT ?

@chalasrchalasr added this to thenext milestoneFeb 16, 2020
@mtarld
Copy link
ContributorAuthor

@nicolas-grekas I updated code according to your approach.
Indeed the code is much cleaner, thanks !

But what should we do when session is used in a stateless mode ? Should we throw an exception ? Just log it ? Or even make that behavior configurable ?

@nicolas-grekas
Copy link
Member

Throw when no logger is passed, log when one is - and pass one when kernel.debug=false in the DI extension?

@mtarld
Copy link
ContributorAuthor

Ok, so now if debug is enabled, this will throw an exception. If not, it'll log a warning.

Copy link
Member

@nicolas-grekasnicolas-grekas left a comment
edited
Loading

Choose a reason for hiding this comment

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

LGTM, don't miss adding some tests and a CHANGELOG entry.

@mtarldmtarldforce-pushed thefeature/stateless branch 6 times, most recently fromf5d6f6c tob8e83bbCompareFebruary 18, 2020 17:46
@mtarldmtarld changed the title[FrameworkBundle][HttpFoundation] Add@Stateless annotation[FrameworkBundle][HttpFoundation] Add session usage reporting in stateless modeFeb 19, 2020
@nicolas-grekasnicolas-grekas changed the title[FrameworkBundle][HttpFoundation] Add session usage reporting in stateless mode[FrameworkBundle][HttpKernel] Add session usage reporting in stateless modeFeb 26, 2020
@fabpot
Copy link
Member

Thank you@mtarld.

@fabpotfabpot merged commit7995fed intosymfony:masterFeb 26, 2020
@mtarldmtarld deleted the feature/stateless branchFebruary 26, 2020 10:41
fabpot added a commit that referenced this pull requestFeb 29, 2020
This PR was merged into the 5.1-dev branch.Discussion----------[Routing] Add stateless route attribute| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Ticket|https://github.com/orgs/symfony/projects/1#card-30506005| License       | MIT| Doc PR        | TODOOn top of#35732Add a stateless attribute for:Routes in annotations```@route(stateless=true)```Yaml```ymlroute:  stateless: true```Xml```xml<route stateless="true" />```PHP configurator```php$route->stateless(true);```That stateless attribute is a shortcut for setting `_stateless` default attribute in route.Commits-------2da68ba [Routing] Add stateless route attribute
@nicolas-grekasnicolas-grekas modified the milestones:next,5.1May 4, 2020
@fabpotfabpot mentioned this pull requestMay 5, 2020
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@derrabusderrabusderrabus left review comments

@fabpotfabpotfabpot approved these changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

5.1

Development

Successfully merging this pull request may close these issues.

6 participants

@mtarld@nicolas-grekas@fabpot@derrabus@chalasr@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp