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.

Sign up for GitHub

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

Jump to bottom

Why is the ||= removed? current_user method #93

Open
E36lewis opened this issue Apr 15, 2023 · 2 comments · May be fixed by #95
Open

Why is the ||= removed? current_user method #93

E36lewis opened this issue Apr 15, 2023 · 2 comments · May be fixed by #95

Comments

@E36lewis
Copy link

I thought the whole point of having this operator was to have the user memoized? With it removed yes the current user is cached into the db, but does that not add to response time? Are there pitfalls in keeping the Current.user ||= instead of the updated: Current.user =.

Looking for clarification, more detail on this. Thanks.
@stevepolitodesign
Copy link
Owner

Excellent question! I think the issue was that I was getting failing tests when I introduced af768c2, and just assumed memoization was the culprit. However, that assumption might be incorrect. I think I'll need to investigate if the current_user test helper needs to be updated.

rails-authentication-from-scratch/test/test_helper.rb

Lines 13 to 19 in 0416b6f

def current_user
if session[:current_active_session_id].present?
ActiveSession.find_by(id: session[:current_active_session_id])&.user
elsif cookies[:remember_token]
ActiveSession.find_by(remember_token: cookies[:remember_token])&.user
end
end

@passbe
Copy link

passbe commented Mar 12, 2024

I believe the tests are failing because using ||= inside current_user will not re-query the ActiveSession table. It will use the cached value inside Current.user. Therefore if a user deletes their current session current_user still returns a valid user indicating the session is still valid.

rails-authentication-from-scratch/app/controllers/active_sessions_controller.rb

Lines 4 to 16 in 0e9d1de

def destroy
@active_session = current_user.active_sessions.find(params[:id])
@active_session.destroy
if current_user
redirect_to account_path, notice: "Session deleted."
else
forget_active_session
reset_session
redirect_to root_path, notice: "Signed out."
end
end

mdchaney added a commit to mdchaney/rails-authentication-from-scratch that referenced this issue Jun 12, 2024
@mdchaney
Adds current_active_session helper.
100dc0c 
Closes stevepolitodesign#93, "Why is the ||= removed? current_user method".  This adds
memoization back to current_user / Current.user, plus causes log out
when the current session is destroyed.
@mdchaney mdchaney linked a pull request Jun 12, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@passbe @stevepolitodesign @E36lewis