Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Session] Changed session.flash_bag service to publicly available#11957
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
jakzal commentedSep 19, 2014
I'd think twice before doing this in the core, especially since we're considering to get rid of a session as a service (see#10557). You can always register such a service in your project. Why do you say that type hinting with |
linaori commentedSep 19, 2014
@jakzal there's nothing concrete planned yet and this is an issue many programmers face. I have provided some cases below. Each case has its own problems, however, the ones coming back are:
In my PR I have made it possible to inject the Case 1: plain action issue publicfunctionsomeAction(Request$request){// just because SessionInterface doesn't have getFlashBagif (($session =$request->getSession())instanceof Session) {$session->getFlashBag()->set('a','b'); }} Case 2: Injection a session class SomeService{private$session;publicfunction__construct(SessionInterface$session) {$this->session =$session; }publicfunctionsomeMethod() {// have to do the same check as in case 1 because SessionInterface doesn't have it }} Case 3: Injection the session that deviates from the interface, not generic anymore class SomeService{private$session;publicfunction__construct(MyCustomSession$session) {$this->session =$session; }publicfunctionsomeMethod() {$this->session->getFlashBag()->setSomething(); }} |
stof commentedSep 19, 2014
Injecting a private service in your service is perfectly allowed and valid. What is forbidden for a service marked as private is retrieving it dynamically through |
linaori commentedSep 19, 2014
@stof correct. However, using that service as injection in its current state is not 100% trustworthy. By default it's passed to the Session. However, it's not guaranteed to be that session as the Worst case scenario: You put stuff in the FlashBag service, but Session is using a different one thus your values are not stored. |
stof commentedSep 19, 2014
@iltar in the case of FrameworkBundle, the service is already passed to the session, as it is there in the service definition |
linaori commentedSep 22, 2014
@stof again, correct. However, for custom session services, there is no way of properly defining a custom session other than overwriting the This PR solves the issue of Because this allows people to use the |
linaori commentedMay 22, 2015
Tobion commentedMay 22, 2015
I guess it's a topic for the next team meeting. |
Main issue is described in my ticket. The problem is that in order to get the flash bag, you have to either ignore the
SessionInterfaceonRequestor type hintSessioninstead ofSessionInterfaceduring injection. Neither solutions work nicely with custom implementations of the session.By defining
session.flash_bagas public service, you can just inject this as it's created bysession.getFlashBag. This makes sure that you always have the right bag, even if you decide to inject a custom flash bag in the session service.For the default implementation, the old variant is used as injection. I have left this as it is so that people can still choose to omit it when constructing or putting in their own flash bag, with their own key if desired. This PR should be 100% BC.