jaculabilis/danbooru
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

comments: minimize sql queries.

Browse Source 
Certain parts of comment rendering triggered sql queries that we didn't
really need to do. Rework things to avoid this.

* Preload comment creators in order to display commenter names with link_to_user.

* Preload comment votes in order to display "undo vote" links. Only preload
  votes for members since anonymous users can't vote and don't have "undo
  vote" links.

* Rework various conditionals to do the filtering in Ruby so that we
  avoid issuing any extra queries in sql.

* Avoid issuing any queries at all when the post doesn't have any
  comments (when last_commented_at is blank).
This commit is contained in:
evazion
2019-08-20 20:53:40 -05:00
parent 2cf929ad6e
commit b283281e5e
9 changed files with 30 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
app/controllers/comments_controller.rb
View File

@@ -74,7 +74,10 @@ private
def index_by_post
@posts = Post.where("last_comment_bumped_at IS NOT NULL").tag_match(params[:tags]).reorder("last_comment_bumped_at DESC NULLS LAST").paginate(params[:page], :limit => 5, :search_count => params[:search])
@posts.each # hack to force rails to eager load
@posts = @posts.includes(comments: [:creator])
@posts = @posts.includes(comments: [:votes]) if CurrentUser.is_member?
respond_with(@posts) do |format|
format.xml do
render :xml => @posts.to_xml(:root => "posts")

4
app/controllers/posts_controller.rb
View File

@@ -23,6 +23,10 @@ class PostsController < ApplicationController
def show
@post = Post.find(params[:id])
@comments = @post.last_commented_at.present? ? @post.comments : Comment.none
@comments = @comments.includes(:creator)
@comments = @comments.includes(:votes) if CurrentUser.is_member?
include_deleted = @post.is_deleted? || (@post.parent_id.present? && @post.parent.is_deleted?) || CurrentUser.user.show_deleted_children?
@parent_post_set = PostSets::PostRelationship.new(@post.parent_id, :include_deleted => include_deleted)
@children_post_set = PostSets::PostRelationship.new(@post.id, :include_deleted => include_deleted)

27
app/models/comment.rb
View File

@@ -22,26 +22,6 @@ class Comment < ApplicationRecord
)
module SearchMethods
def recent
reorder("comments.id desc").limit(6)
end
def hidden(user)
if user.is_moderator?
where("(score < ? and is_sticky = false) or is_deleted = true", user.comment_threshold)
else
where("score < ? and is_sticky = false", user.comment_threshold)
end
end
def visible(user)
if user.is_moderator?
where("(score >= ? or is_sticky = true) and is_deleted = false", user.comment_threshold)
else
where("score >= ? or is_sticky = true", user.comment_threshold)
end
end
def deleted
where("comments.is_deleted = true")
end
@@ -172,9 +152,16 @@ class Comment < ApplicationRecord
end
def voted_by?(user)
return false if user.is_anonymous?
user.id.in?(votes.map(&:user_id))
end
def visible_by?(user, show_thresholded: false, show_deleted: false)
return false if is_deleted? && !show_deleted && !user.is_moderator?
return false if score < user.comment_threshold && !is_sticky? && !show_thresholded
true
end
def hidden_attributes
super + [:body_index]
end

2
app/models/post.rb
View File

@@ -51,7 +51,7 @@ class Post < ApplicationRecord
has_many :appeals, :class_name => "PostAppeal", :dependent => :destroy
has_many :votes, :class_name => "PostVote", :dependent => :destroy
has_many :notes, :dependent => :destroy
has_many :comments, -> {includes(:creator, :updater).order("comments.id")}, :dependent => :destroy
has_many :comments, -> {order("comments.id")}, :dependent => :destroy
has_many :children, -> {order("posts.id")}, :class_name => "Post", :foreign_key => "parent_id"
has_many :approvals, :class_name => "PostApproval", :dependent => :destroy
has_many :disapprovals, :class_name => "PostDisapproval", :dependent => :destroy

2
app/views/application/_update_notice.html.erb
View File

@@ -1,6 +1,6 @@
<%# record, interval %>
<% if record.respond_to?(:updater) && record.updater != record.creator %>
<% if record.respond_to?(:updater) && record.updater_id != record.creator_id %>
<p class="info">Updated by <%= link_to_user record.updater %> <%= time_ago_in_words_tagged(record.updated_at) %></p>
<% elsif record.updated_at - record.created_at > (local_assigns[:interval] || 5.minutes) %>
<p class="info">Updated <%= time_ago_in_words_tagged(record.updated_at) %></p>

12
app/views/comments/_index_by_post.html.erb
View File

@@ -5,19 +5,17 @@
</div>
<% end %>
<% if @posts.empty? %>
<% if @posts.blank? %>
<%= render "post_sets/blank" %>
<% end %>
<% @posts.select {|x| x.visible?}.each do |post| %>
<% if CurrentUser.is_moderator? || post.comments.undeleted.exists? %>
<% @posts.select(&:visible?).each do |post| %>
<% if post.comments.any? { |c| c.visible_by?(CurrentUser.user, show_thresholded: true) } %>
<%= content_tag(:div, { id: "post_#{post.id}", class: ["post", *PostPresenter.preview_class(post)].join(" ") }.merge(PostPresenter.data_attributes(post))) do %>
<div class="preview">
<% if post.visible? %>
<%= link_to(image_tag(post.preview_file_url), post_path(post)) %>
<% end %>
<%= link_to(image_tag(post.preview_file_url), post_path(post)) %>
</div>
<%= render "comments/partials/index/list", :post => post, :comments => post.comments.visible(CurrentUser.user).recent.reverse, :show_header => true %>
<%= render "comments/partials/index/list", post: post, comments: post.comments.select { |c| c.visible_by?(CurrentUser.user) }.last(6), page: :comments %>
<div class="clearfix"></div>
<% end %>
<% end %>

16
app/views/comments/partials/index/_list.html.erb
View File

@@ -1,10 +1,10 @@
<div class="comments-for-post" data-post-id="<%= post.id %>">
<% if show_header %>
<% if page == :comments %>
<%= render "comments/partials/index/header", :post => post %>
<% end %>
<div class="row notices">
<% if post.comments.hidden(CurrentUser.user).count > 0 || (params[:controller] == "comments" && post.comments.count > 6) %>
<% if (post.last_commented_at.present? && post.comments.any? { |c| !c.visible_by?(CurrentUser.user) }) || (page == :comments && post.comments.size > 6) %>
<span class="info" id="threshold-comments-notice-for-<%= post.id %>">
<%= link_to "Show all comments", comments_path(:post_id => post.id), :remote => true %>
</span>
@@ -12,14 +12,12 @@
</div>
<div class="list-of-comments">
<% if comments.empty? %>
<% if post.last_commented_at.present? %>
<p>There are no visible comments.</p>
<% else %>
<p>There are no comments.</p>
<% end %>
<% if comments.present? %>
<%= render partial: "comments/partials/show/comment", collection: comments %>
<% elsif post.last_commented_at.present? %>
<p>There are no visible comments.</p>
<% else %>
<%= render :partial => "comments/partials/show/comment", :collection => comments %>
<p>There are no comments.</p>
<% end %>
</div>

2
app/views/posts/show.html.erb
View File

@@ -120,7 +120,7 @@
<% if !CurrentUser.user.is_builder? %>
<h2>Before commenting, read the <%= link_to "how to comment guide", wiki_pages_path(:search => {:title => "howto:comment"}) %>.</h2>
<% end %>
<%= render "comments/partials/index/list", :comments => @post.comments.visible(CurrentUser.user).includes(:creator), :post => @post, :show_header => false %>
<%= render "comments/partials/index/list", comments: @comments, post: @post, page: :post %>
</section>
<section id="notes" style="display: none;">

20
test/unit/comment_test.rb
View File

@@ -252,26 +252,6 @@ class CommentTest < ActiveSupport::TestCase
end
end
context "that is below the score threshold" do
should "be hidden if not stickied" do
user = FactoryBot.create(:user, :comment_threshold => 0)
post = FactoryBot.create(:post)
comment = FactoryBot.create(:comment, :post => post, :score => -5)
assert_equal([comment], post.comments.hidden(user))
assert_equal([], post.comments.visible(user))
end
should "be visible if stickied" do
user = FactoryBot.create(:user, :comment_threshold => 0)
post = FactoryBot.create(:post)
comment = FactoryBot.create(:comment, :post => post, :score => -5, :is_sticky => true)
assert_equal([], post.comments.hidden(user))
assert_equal([comment], post.comments.visible(user))
end
end
context "that is quoted" do
should "strip [quote] tags correctly" do
comment = FactoryBot.create(:comment, body: <<-EOS.strip_heredoc)