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

[FrameworkBundle] Remove deprecate session service #41321

Merged
merged 5 commits into from Sep 7, 2021

Conversation

jderusse
Copy link
Member

@jderusse jderusse commented May 20, 2021

Q A
Branch? 6.0
Bug fix? no
New feature? no
Deprecations? no
Tickets -
License MIT
Doc PR -

To ease code review this PR only removes code for session.

Blocked by #41343

@carsonbot carsonbot added this to the 6.0 milestone May 20, 2021
@jderusse jderusse marked this pull request as draft May 20, 2021 09:13
@jderusse jderusse force-pushed the deprec-session branch 2 times, most recently from c1b0432 to 67cb63a Compare May 20, 2021 16:42
@carsonbot

This comment has been minimized.

@carsonbot carsonbot changed the title Remove deprecate session service [FrameworkBundle] Remove deprecate session service May 24, 2021
@nicolas-grekas
Copy link
Member

Rebase unlocked

@jderusse jderusse force-pushed the deprec-session branch 4 times, most recently from b7a5d3f to cb32334 Compare May 29, 2021 11:09
nicolas-grekas added a commit that referenced this pull request Jun 1, 2021
…r (jderusse)

This PR was merged into the 5.4 branch.

Discussion
----------

[HttpKernel] Provide migration path for TestSessionListener

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

When working on #41321 I realized that #41343 was wrong for `TestSessionListener`.
Injecting the factory in both SessionListener AND TestSessionListener would generate 2 distinguished session instances which is the opposite of the expected behavior: TestSessionListener needs the session used by the request to set the right sessionId in the cookie.

This PR fallback to `request->getSession()` when there is no session injected in the container (ie: FrameworkBundlke:6)

Commits
-------

b56606e Provide migration path for TestSessionListener
@fabpot
Copy link
Member

fabpot commented Jul 4, 2021

@jderusse Is it still in draft mode?

@jderusse jderusse force-pushed the deprec-session branch 2 times, most recently from ed01205 to 1d26674 Compare July 4, 2021 15:49
@jderusse jderusse force-pushed the deprec-session branch 5 times, most recently from 7bccfce to e8d240b Compare July 5, 2021 06:00
@jderusse jderusse marked this pull request as ready for review July 6, 2021 15:19
@jderusse
Copy link
Member Author

jderusse commented Jul 6, 2021

💚 green

@jderusse jderusse force-pushed the deprec-session branch 6 times, most recently from 4ceb105 to 2127454 Compare September 6, 2021 20:21
nicolas-grekas added a commit that referenced this pull request Sep 6, 2021
…nListener (jderusse)

This PR was merged into the 5.4 branch.

Discussion
----------

Make test.session.listener compatible with SF 6.0 SessionListener

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

Fix flipped tests for #41321

In Symfony 6.0, the session_listener does not use `session` service anymore, it needs a `session_factory` instead.

note: the `session_listener` service is properly configured:
https://github.com/symfony/symfony/blob/0d61de7576d0d6807924fada3df508c5d05d8f2f/src/Symfony/Bundle/FrameworkBundle/Resources/config/session.php#L146-L150

Commits
-------

0943685 Make test.session.listener compatible with SF 6.0 SessionListener
@jderusse jderusse force-pushed the deprec-session branch 2 times, most recently from 3f9f3c1 to b2efa34 Compare September 6, 2021 21:58
jderusse and others added 2 commits September 7, 2021 12:04
…re 6.0 (jderusse)

This PR was merged into the 5.4 branch.

Discussion
----------

Fix tests when security-http 5.4 uses security-core 6.0

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

see falling test in flipped tests for 6.0 branch: https://github.com/symfony/symfony/pull/41321/checks?check_run_id=3528227747#step:8:3572

Commits
-------

58f09e5 Fix tests when security-http 5.4 uses security-core 6.0
nicolas-grekas and others added 3 commits September 7, 2021 12:19
* 5.4:
  cs fix
  Fix tests when security-http 5.4 uses security-core 6.0
@nicolas-grekas
Copy link
Member

Thank you @jderusse.

@nicolas-grekas nicolas-grekas merged commit 062ac36 into symfony:6.0 Sep 7, 2021
@jderusse jderusse deleted the deprec-session branch September 7, 2021 11:17
@fabpot fabpot mentioned this pull request Nov 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants