Skip to content

Commit

Permalink
many 1 + n queries detected in prod by prosopite
Browse files Browse the repository at this point in the history
  • Loading branch information
pushcx committed Mar 15, 2024
1 parent 7af9d6f commit 0747697
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 78 deletions.
15 changes: 7 additions & 8 deletions app/controllers/stories_controller.rb
Expand Up @@ -228,8 +228,8 @@ def submit_suggestions
end

story_user = @story.user
inappropriate_tags = params[:story][:tags_a].reject { |t| t.to_s.strip == "" }
.map { |t| Tag.find_by tag: t }
inappropriate_tags = Tag
.where(tag: params[:story][:tags_a].reject { |t| t.to_s.blank? })
.reject { |t| t.can_be_applied_by?(story_user) }
if inappropriate_tags.length > 0
tag_error = ""
Expand Down Expand Up @@ -259,12 +259,11 @@ def submit_suggestions
dsug = true
end

sugtags = params[:story][:tags_a].reject { |t| t.to_s.strip == "" }
.sort
.reject { |t|
!Tag.find_by(tag: t).can_be_applied_by?(story_user)
}
if @story.tags_a.sort != sugtags
sugtags = Tag
.where(tag: params[:story][:tags_a].reject { |t| t.to_s.strip.blank? })
.reject { |t| !t.can_be_applied_by?(story_user) }
.map { |s| s.tag }
if @story.tags_a.sort != sugtags.sort
@story.save_suggested_tags_a_for_user!(sugtags, @user)
dsug = true
end
Expand Down
46 changes: 18 additions & 28 deletions app/models/comment.rb
Expand Up @@ -132,7 +132,7 @@ def as_json(_options = {})
{comment_plain: (is_gone? ? gone_text : :comment)},
:url,
:depth,
{commenting_user: :user}
{commenting_user: user.username}
]

js = {}
Expand Down Expand Up @@ -265,34 +265,24 @@ def deliver_notifications
end

def deliver_mention_notifications(notified = [])
to_notify = plaintext_comment.scan(/\B[@~]([\w\-]+)/).flatten.uniq
(to_notify - notified).each do |mention|
if notified.include? mention
next
end

if (u = User.active.find_by(username: mention))
if u.id == user.id
next
end

if u.email_mentions?
begin
EmailReplyMailer.mention(self, u).deliver_now
rescue => e
Rails.logger.error "error e-mailing #{u.email}: #{e}"
end
to_notify = plaintext_comment.scan(/\B[@~]([\w\-]+)/).flatten.uniq - notified - [user.username]
User.active.where(username: to_notify).find_each do |u|
if u.email_mentions?
begin
EmailReplyMailer.mention(self, u).deliver_now
rescue => e
Rails.logger.error "error e-mailing #{u.email}: #{e}"
end
end

if u.pushover_mentions?
u.pushover!(
title: "#{Rails.application.name} mention by " \
"#{user.username} on #{story.title}",
message: plaintext_comment,
url: url,
url_title: "Reply to #{user.username}"
)
end
if u.pushover_mentions?
u.pushover!(
title: "#{Rails.application.name} mention by " \
"#{user.username} on #{story.title}",
message: plaintext_comment,
url: url,
url_title: "Reply to #{user.username}"
)
end
end
end
Expand Down Expand Up @@ -563,7 +553,7 @@ def url
def vote_summary_for_user(u)
r_counts = {}
r_users = {}
votes.includes(:user).each do |v|
votes.includes(:user).find_each do |v|
r_counts[v.reason.to_s] ||= 0
r_counts[v.reason.to_s] += v.vote

Expand Down
2 changes: 1 addition & 1 deletion app/models/replying_comment.rb
Expand Up @@ -8,7 +8,7 @@ class ReplyingComment < ApplicationRecord
scope :for_user, ->(user_id) {
where(user_id: user_id)
.order(comment_created_at: :desc)
.preload(comment: [:story, :user])
.preload(comment: [:hat, {story: :user}, :user])
}
scope :unread_replies_for, ->(user_id) { for_user(user_id).where(is_unread: true) }
scope :comment_replies_for,
Expand Down
2 changes: 1 addition & 1 deletion app/models/search.rb
Expand Up @@ -226,7 +226,7 @@ def perform_comment_search!
end

def perform_story_search!
query = Story.base(@user).for_presentation
query = Story.base(@searcher).for_presentation

terms = []
n_domains = 0
Expand Down
62 changes: 22 additions & 40 deletions app/models/story.rb
Expand Up @@ -17,6 +17,7 @@ class Story < ApplicationRecord
autosave: true,
dependent: :destroy
has_many :suggested_taggings, dependent: :destroy
has_many :suggested_tags, source: :story, through: :suggested_taggings, dependent: :destroy
has_many :suggested_titles, dependent: :destroy
has_many :suggested_tagging_times,
-> { group(:tag_id).select("count(*) as times, tag_id").order("times desc") },
Expand All @@ -38,9 +39,9 @@ class Story < ApplicationRecord
has_many :savings, class_name: "SavedStory", inverse_of: :story, dependent: :destroy
has_one :story_text, foreign_key: :id, dependent: :destroy, inverse_of: :story

scope :base, ->(user) { includes(:tags).not_deleted(user).unmerged.mod_preload?(user) }
scope :base, ->(user) { includes(:story_text, :user).not_deleted(user).unmerged.mod_preload?(user) }
scope :for_presentation, -> {
includes(:domain, :user, taggings: :tag)
includes(:domain, :user, :tags, taggings: :tag)
}
scope :mod_preload?, ->(user) {
user.try(:is_moderator?) ? preload(:suggested_taggings, :suggested_titles) : all
Expand Down Expand Up @@ -310,7 +311,7 @@ def as_json(options = {})
{description: :markeddown_description},
{description_plain: :description},
:comments_url,
{submitter_user: :user},
{submitter_user: user.username},
:user_is_author,
{tags: tags.map(&:tag).sort}
]
Expand Down Expand Up @@ -403,8 +404,8 @@ def check_tags
u = editor || user

if u&.is_new? &&
(unpermitted = taggings.filter { |t| !t.tag.permit_by_new_users? }).any?
tags = unpermitted.map { |t| t.tag.tag }.to_sentence
(unpermitted = Tag.where(id: taggings.map(&:tag_id), permit_by_new_users: false)).any?
tags = unpermitted.map(&:tag).to_sentence
errors.add :base, <<-EXPLANATION
New users can't submit stories with the tag(s) #{tags}
because they're for meta discussion or prone to off-topic stories.
Expand Down Expand Up @@ -666,7 +667,10 @@ def tagging_changes
end

def tags_a
@_tags_a ||= taggings.reject(&:marked_for_destruction?).map { |t| t.tag.tag }
@_tags_a ||= taggings
.includes(:tag)
.reject(&:marked_for_destruction?)
.map { |t| t.tag.tag }
end

def tags_a=(new_tag_names_a)
Expand All @@ -676,45 +680,23 @@ def tags_a=(new_tag_names_a)
end
end

new_tag_names_a.uniq.each do |tag_name|
if tag_name.to_s != "" && !tags.exists?(tag: tag_name)
if (t = Tag.active.find_by(tag: tag_name))
# we can't lookup whether the user is allowed to use this tag yet
# because we aren't assured to have a user_id by now; we'll do it in
# the validation with check_tags
taggings.build(tag_id: t.id)
end
end
new_tags = Tag.where(tag: new_tag_names_a.uniq.compact_blank - tags.pluck(:tag))
new_tags.each do |t|
# we can't lookup whether the user is allowed to use this tag yet
# because we aren't assured to have a user_id by now; we'll do it in
# the validation with check_tags
taggings.build(tag_id: t.id)
end
end

def save_suggested_tags_a_for_user!(new_tag_names_a, user)
st = suggested_taggings.where(user_id: user.id)

st.each do |tagging|
if !new_tag_names_a.include?(tagging.tag.tag)
tagging.destroy!
end
end
suggested_taggings.where(user_id: user.id).destroy_all

st.reload

new_tag_names_a.each do |tag_name|
# XXX: AR bug? st.exists?(:tag => tag_name) does not work
if tag_name.to_s != "" && !st.map { |x| x.tag.tag }.include?(tag_name)
if (t = Tag.active.find_by(tag: tag_name)) &&
t.can_be_applied_by?(user)
tg = suggested_taggings.build
tg.user_id = user.id
tg.tag_id = t.id
tg.save!

st.reload
else
next
end
end
end
new_tags = Tag
.active
.where(tag: new_tag_names_a.uniq.compact_blank)
.map { |t| {user: user, tag: t} }
suggested_taggings.create!(new_tags)

# if enough users voted on the same set of replacement tags, do it
tag_votes = {}
Expand Down
6 changes: 6 additions & 0 deletions extras/markdowner.rb
Expand Up @@ -51,6 +51,11 @@ def self.walk_text_nodes(node, &block)
end

def self.postprocess_text_node(node)
# Allowing 1+n in username linkification in comments/bios because this works inorder on the
# parse tree rather than running once over the text. Proper fix: loop to find usernames, do one
# lookup, then loop again to manipulate the nodes for usernames that exist.
Prosopite.pause

while node
return unless node.string_content =~ /\B(@#{User::VALID_USERNAME})/o
before, user, after = $`, $1, $'
Expand Down Expand Up @@ -81,6 +86,7 @@ def self.postprocess_text_node(node)
node = nil
end
end
Prosopite.resume
end

def self.convert_images_to_links(node)
Expand Down

0 comments on commit 0747697

Please sign in to comment.