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

Let SessionMiddleware handle session saving #2863

Open
stveit opened this issue Mar 5, 2024 · 4 comments
Open

Let SessionMiddleware handle session saving #2863

stveit opened this issue Mar 5, 2024 · 4 comments

Comments

@stveit
Copy link
Contributor

stveit commented Mar 5, 2024

Related to #2804

Django lets SessionMiddleware save session during the response processing step. Saving on login was already done some places, but #2813 made it happen for all kinds of logins. Similarly some places save session after logging out, and #2828 makes it happen on all logouts.

Basically the save calls in clear_session and set_account should be removed. This does however cause at least 1 test to break, since it uses the Django test client from the client fixture, and the SessionMiddleware does not seem to run automatically during requests made using client.

@stveit
Copy link
Contributor Author

stveit commented Mar 8, 2024

An actual difference between calling save early and not: flush sets the session ID to None. calling save will generate a session ID. So removing the current extra save calls will make it so the session does not have a session ID until it reaches the SessionMiddleware's process_response function where save should be called

@stveit
Copy link
Contributor Author

stveit commented Mar 8, 2024

And it seems, depending on what subclass of SessionBase that is being used. save calls cycle_key, meaning it will get a new session ID every time save is called.

The one we use, SessionStore, only generates a new key on save if it doesnt already have one. So extra save calls should be safe in our instance

@stveit
Copy link
Contributor Author

stveit commented Apr 29, 2024

branch: save-in-middleware in my fork is a WIP attempt at this

@stveit
Copy link
Contributor Author

stveit commented Apr 29, 2024

big problem i had: get session middleware to run properly during testing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant