[Feature] Customizeable current_user

[eric-hu]

It would be useful to have a generator function similar to the views generator so that the current_user loading could be customized. In my current situation, I'd like to be able to add some eager loading and nested eager loading to current_user, so that I could do something like

current_user.votes.find_by_post_id (1234)

without having to query the database

[josevalim]

I don't understand completely what you propose. Could you please explain it better? Do you want to be able to customize controller? If that is the case, you can simply override it in your application controller...

[eric-hu]

At this point I'm not sure how or where the helper current_user is loaded. I'd like to be able to do something like rails generate devise:helpers and have similar behavior to rails generate devise:views. I'm guessing that right now, current_user is loaded something like this

def current_user
  resource.find(:id)
end

I would like to be able to modify this line so I can optimize my application. For example, what I want right now is :

def current_user
  resource.includes(:moderator_permission, :post, :vote, :flag => {:post => {:user}}.find(:id)
end

This would optimize how many SQL statements I use to generate my views. My needs would be different in another application, but being able to customize the helper current_user allows me to optimize details as my application grows. I believe that others would make use of this functionality too.

[josevalim]

It is a method in the controller, you can override it and call super. BUT, you don't need to do eager loading here. Eager loading would be just important if you had a collection of users. The nested eager loading can be done when bringing the flags. You can give as default parameter in the association.

[eric-hu]

Ah okay, I was starting to think it wouldn't be that difficult. Thanks for confirming that I can override it.

I need eager loading on current_user.votes because I'm essentially calling this 30 times (for different post objects)

if current_user.votes.find_by_post_id(target_post.id)
  ...

Without eager loading, I get 30 of these for each page request:

  Vote Load (0.5ms)  SELECT "votes".* FROM "votes" WHERE "votes"."post_id" = 23252 AND ("votes".user_id = 11811) LIMIT 1

Where the only difference is the post_id. I could rearrange my Rails statement so that I could eager load Post.votes, but for my particular application I feel like that wouldn't scale well.

I'm not sure what you mean by "bringing the flags"--could you elaborate on that?

[josevalim]

Ah, I didn't know find_by_post_id would look into the cache. Maybe you can do something like this in your application controller:

def current_user
  super.tap { |u|
    u.posts
    u.magic
  }
end

I am not sure if user can be nil in this case... you can try it out. :)

[eric-hu]

I'm still new to eager loading, but I believe it prevents database lookups for all data you include, thus if we have

def current_user
  resource.find(:id, :include => :votes)
end

then the following code would still just be 2 sql statements (something like select * from Users where id = current_user.id, and select * from votes where user_id = current_user.id). This would be true whether we have 0 votes, 1 vote, or 100 (there may be an upper limit, but I'm not familiar enough with it yet).

current_user.votes.all.each do |v|
  puts v.id
end

However, as something like current_user.votes.first.category.category_name would generate an SQL request. I also want to be selective about when I use eager loading, as I could end up using cached data that's different from what's in the db.

I'm on rails 1.8.7, so I can't use tap, but I'll just use debugger to try to accomplish the same. Thanks for that code snippet! l Am I correct that current_#{resource} is defined by def current_#{mapping} in ./controllers/helpers.rb ?

[eric-hu]

After some more playing around, I discovered that you were right that ActiveRecord:Base.find does not use eager-loaded values.

However, current_user does not eager-load any associations. I tested this using the ruby-debug gem and enabling some extra SQL output to the logger with

ActiveSupport::Notifications.subscribe('sql.active_record') do |*args|
  $odd_or_even_queries = !$odd_or_even_queries
  color = $odd_or_even_queries ? ANSI[:CYAN] : ANSI[:MAGENTA]
  event = ActiveSupport::Notifications::Event.new(*args)
  time = "%.1fms" % event.duration
  name = event.payload[:name]
  sql = event.payload[:sql].gsub("\n", " ").squeeze(" ")
  puts " #{ANSI[:UNDERLINE]}#{color}#{name} (#{time})#{ANSI[:RESET]} #{sql}"
end

in my ~/.irbrc file. Then, setting debugger in my homepage controller, I logged into my application website and got the following

irb(#<HomeController:0x7f29f7daa160>):002:0> current_user.votes
 Vote Load (1.0ms) SELECT "votes".* FROM "votes" WHERE ("votes".user_id = 11818)
=> []
irb(#<HomeController:0x7f29f7daa160>):003:0> current_user.posts
 Post Load (0.8ms) SELECT "posts".* FROM "posts" WHERE ("posts".user_id = 11818)
=> []
irb(#<HomeController:0x7f29f7daa160>):004:0> current_user.posts
=> []

The second current_user.posts doesn't generate an SQL query since Rails caches that. I don't think I can modify current_user to eager load associations, though, since it appears to be a copy of the model object, provided by Warden.

I'd be willing to edit this conversation (for length) and make a writeup in the wiki, if you'd like. Since I'm still pretty new to Rails, I'm not sure how much of what I've discovered is obvious to most users.

[josevalim]

Thanks for pinging Back. It is indeed obvious for experienced users, but again, there is no need to eager load on users. You would have to eager load if you had an array of users instead.

[eric-hu]

No problem. When you say that there is no need to eager load a user, are you referring to the user associations? When I have has_many :posts in my User model, I don't see why I wouldn't want to eager load User.posts if I have many posts. Am I missing something?

[josevalim]

In your case, if you want to bring all posts, you just do @user.posts.all. However, when we refer to eager loading in Rails, we usually say that instead of doing this:

Post.all.each { |p| o.comments }

You should do this:

Post.all(:include => :comments).each { |p| o.comments }

So it does two queries instead of N + 1.

[eric-hu]

That part I understand and I agree it's entirely doable when working within the User model. I ran into issues when I wanted to do something in the view for Posts#index. Specifically, I wanted to display different vote links for each post based on whether current_user had already voted on a post.

I suppose I could create an instance variable @user = User.find (current_user, :includes => :post) in the Post controller. That seemed redundant to me when current_user seems to already be loaded and available.

[joeellis]

Came across this issue today myself on wanting to specifically eager load on current_user. In my case, a user has_many decks (study deck - flash card app) and when I do current_user.decks, Rails does an N+1 lookup.

I believe I understand what josevalim is saying for eager loading on other models in other cases, but there are specific times where one would want to eager-load associations on current_user (and Rails does not eager-load them automatically, at least not by default that I can see)

[josevalim]

current_user.decks will always eager load. Unless you are calling a method that makes it goes in batch. You can call current_user.decks.all to be sure. What would not be eager loaded is if you have something nested on decks, like cards. So you would have to do: current_user.decks.all(:include => :cards).

[joeellis]

Apologies, you are absolutely right about that, current_user.decks.all worked perfectly. Perhaps then that is what eric-hu needs to do, just call current_user.posts.all to solve his problem.

[eric-hu]

Yes, joeellis, that is one possible solution for me. I've extracted a lot of logic out of my views into helper methods, so I'm concerned that I'd run into performance issues.

I wanted to avoid making ~30 calls of generate_vote_link(current_user.posts.all) for each page load. I'm used to Java, where passing large objects to a functions repeatedly can be very expensive. This could be a non-issue in Ruby, so I can give it a shot and see.

Even if it is expensive, there are still workarounds to this. I could make my helper method less generic, which I'd prefer not to do but I will if it will give the best performance. My initial impression was that current_user was a global variable (or behaved like one) and could be easily modified to just eager load some associations in specific scopes.

[joeellis]

Not sure without seeing your code, but it sounds like if you're worried about scalability, all you'd need to do is just cache it once and then do your ~30 calls, and just make sure to update the cache when the user makes changes to its posts

On Thursday, March 24, 2011 at 3:20 PM, eric-hu wrote:

Yes, joeellis, that is one possible solution for me. I've extracted a lot of logic out of my views into helper methods, so I'm concerned that I'd run into performance issues.

I wanted to avoid making ~30 calls of generate_vote_link(current_user.posts.all) for each page load. I'm used to Java, where passing large objects to a functions repeatedly can be very expensive. This could be a non-issue in Ruby, so I can give it a shot and see.

Reply to this email directly or view it on GitHub:
#938 (comment)

[eric-hu]

Scalability is indeed the concern and your solution sounds like it'll work, but I think that's starting to go out of the scope of this issue.

Sorry for all the notifications, Jose, and thanks for your patience!