Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make sure SpoonSession is initialised before starting symfony session #2023

Merged
merged 2 commits into from Apr 13, 2017

Conversation

StijnVrolijk
Copy link
Contributor

@StijnVrolijk StijnVrolijk commented Apr 10, 2017

Type

  • Critical bugfix

Resolves the following issues

Symfony sessions did not work if they where started before the spoon sessions

Pull request description

In most cases we will only access the symfony sessions after SpoonSession has started. But in the cases where that is not the case the symfony sessions didn't work because of some stuff happening This will fix that issue by making sure that when we start the symfony sessions the SpoonSessions also have started.

app/Kernel.php Outdated
SpoonSession::start();

/** @var \Symfony\Component\HttpFoundation\Session\Session $session */
$session = $this->getContainer()->get('session');
Copy link
Member

Choose a reason for hiding this comment

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

I would wrap this method still in a check where you check if the service exists, I think that in cases like a console command it doesnt exist, maybe in those cases also skipp strarting SpoonSession

Copy link
Member

Choose a reason for hiding this comment

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

Also, starting a session, means a cookies, which sucks for caching.

Copy link
Member

Choose a reason for hiding this comment

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

In my opinion, the session should only be started if it is needed.

Copy link
Member

@carakas carakas Apr 13, 2017

Choose a reason for hiding this comment

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

@tijsverkoyen this should be fixed in 3513b16 and that should also address my issue

@carakas carakas force-pushed the fork-symfony-session-bridge branch from dd950fc to 779099c Compare April 13, 2017 11:14
@carakas carakas changed the title Manually initialize both session Make sure SpoonSession is initialised before starting symfony session Apr 13, 2017
@carakas carakas merged commit fa5ed04 into master Apr 13, 2017
@carakas carakas deleted the fork-symfony-session-bridge branch April 13, 2017 12:14
@carakas carakas added this to the 4.5.3 milestone Apr 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants