session aware model methods in to_json cause n+1 queries

ruby-on-rails ruby ruby-on-rails-4 activerecord rails-activerecord

89 观看

2回复

1233 作者的声誉

This is an optimization question for an existing application, I've made the code generic to both make it annonmous and also easier to understand, instead of our proprietary models I'm describing a Forum discussion type situation. I've modified all this code for this example and not tested it, so if there are any typos I apologize, I'll try to fix them if they are pointed out to me.

Lets say I have a rails app with four models: Event, User, Forum, and Post.

the important relationships are as follows:

  • User has many events.
  • Forum has many posts.
  • Post has many events.

The front end is a single page javascript app, so all database data needs to be returned in json format.

Context:

  • when a User clicks on a post, an event is created with the name 'Show' which marks the post as no longer new.
  • The user needs to be logged in to see which posts are new clicking on a forum calls the following endpoint:
  • There are multiple users so the events able is a many to many relationship between posts and users.

example.com/forum/15/all_posts

heres the relevant code:

Forum Controller:

#forums_controller.rb
def all_posts
    current_user = User.find(session[:user_id])
    forum = Forum.includes(:posts).where(id: params[:id]).take

    forum.posts.each do |post|
        post.current_user = current_user
    end

     render json: forum.to_json(
        include: [
            { posts: {
                methods: [:is_new]
            }}
        ]
     )
end

Posts model:

#post.rb (posts model)

has_many :events
attr_accessor :current_user

def is_new
    if current_user #user may not be logged in
        !!self.events.where(user_id: current_user.id, name: 'Show').take
    else
        false
    end
end

the model is where the action is at, so we've tried to keep logic out of the controller, but since the session is not available in the model we end up with this crazy work around of adding current_user as an attr_accessor so that methods can return the appropriate data for the user in question.... I don't like this but I've never come up with a better way to do it. We've repeated this pattern elsewhere and I would love to hear alternatives.

Here's my problem:

The call to is_new is used on the front end to determine what posts to hi-light but it's also triggering an n+1 scenario If there are 10 posts, this endpoint would net me a total 12 queries which is no good if my events table is huge. If I moved all the logic to the controller I could probably do this in 2 queries.

in short I have two questions:

  1. MOST IMPORTANT: How can I fix this n+1 situation?
  2. Is there a better way in general? I don't like needing an each loop before calling to_json I don't find this pattern to be elegant or easy to understand. at the same time I don't want to move all the code into the controller. What is the rails way to do this?
作者: denodster 的来源 发布者: 2017 年 12 月 27 日

回应 2


1

1696 作者的声誉

If working with scope is an option, I will try something like:

class Post < ApplicationRecord
  scope :is_new, -> { where(user_id: current_user.id, name: 'Show') } if current_user.id?
end

If is a better option to send the current_user in your case, you can also do it:

class Post < ApplicationRecord
  scope :is_new, ->(current_user) {...}
end
作者: Alejandro Montilla 发布者: 2017 年 12 月 27 日

0

435 作者的声誉

This is just pseudo-code to give an example:

First Answer

When I posted this I forgot you are rendering json from ForumsController.

Post

scope :for_user, -> (user = nil) do 
  includes(events: :users).where(users: {id: user.id}) if user
end

def is_new_for_user?(user = nil)
  return true if user.nil?
  self.events.empty?{ |e| e.name == 'Show' }
end

PostController

def index
  @posts = Post.for_user(current_user)
end

posts/index.html.erb

...
<% if post.is_new_for_user?(current_user) %>
  ...
<% end
...

Second Answer

This is still pseudo-code. I didn't test anything.

Forum

scope :for_user, -> (user = nil) do
  if user
    includes(posts: [events: :users]).where(users: {id: user.id})
  else
    includes(:posts)
  end
end

ForumsController

def all_posts
    current_user = User.find(session[:user_id])
    forum = Forum.for_user(current_user).where(id: params[:id]).take

     render json: forum.to_json(
        include: [
            { posts: {
                methods: [:is_new_for_user?(current_user)]
            }}
        ]
     )
end
作者: Jacob Vanus 发布者: 2017 年 12 月 27 日
32x32