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

Drop session from user class #2790

Merged
merged 1 commit into from
Apr 16, 2021
Merged

Drop session from user class #2790

merged 1 commit into from
Apr 16, 2021

Conversation

askvortsov1
Copy link
Sponsor Member

This was originally introduced inhttps://github.com/flarum/core/commit/3612ca7aca90b5d45310da1602256e247451cec3, but has not seen usage, since usually when the session needs to be modified, the request is available.

It causes issues with certain queue drivers, as it can't be serialized.

It's also not entirely accurate, as a user can have multiple sessions at once. Therefore, a given session is a property of the request, not of the user.

This was originally introduced inhttps://github.com/flarum/core/commit/3612ca7aca90b5d45310da1602256e247451cec3, but has not seen usage, since usually when the session needs to be modified, the request is available.

It causes issues with certain queue drivers, as it can't be serialized.

It's also not entirely accurate, as a user can have multiple sessions at once. Therefore, a given session is a property of the request, not of the user.
@askvortsov1 askvortsov1 added this to the 0.1 milestone Apr 16, 2021
Copy link
Member

@luceos luceos left a comment

Choose a reason for hiding this comment

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

The reason this causes issues in the Queue is that when a Job has payload that consists User(s), the Queue will try to serialize that. Serializing the User object will require serializing the session too; this causes a Serialization of Closure is not allowed error, see image.

One can circumvent that in many ways, the most obvious one is adding a __sleep and __wakeup implementation in the User class (or the session handler). But as we aren't really using the session on the User model anywhere in core, bundled or most community extensions it is best to simply detach this from the user.

image

@askvortsov1 askvortsov1 merged commit fb51fb4 into master Apr 16, 2021
@askvortsov1 askvortsov1 deleted the as/drop-user-session branch April 16, 2021 19:53
@askvortsov1 askvortsov1 removed this from the 1.0 milestone Apr 16, 2021
@askvortsov1
Copy link
Sponsor Member Author

Fixes #2784

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

Successfully merging this pull request may close these issues.

3 participants