From bfdc13a4bbb36299b1430de2497a7a9006231145 Mon Sep 17 00:00:00 2001 From: evazion Date: Tue, 4 Aug 2020 10:58:42 -0500 Subject: [PATCH] Fix #4574: "unknown keyword: :tag_count" when changing tag category. Bug: Tag#update_post_category_counts effectively called `update_all("tag_count_general" => 123, :tag_count => 456)`, which led to an argument error due to the :tag_count symbol being treated as a keyword argument. Fix: Don't mess around with `update_all`, just regenerate the tag counts then `save!` the post. We have to do this in an after_save callback because we need the updated category to be in the database for `set_tag_counts` to work. Delete `fix_post_counts` because it was unused. This is most likely a regression caused by the upgrade of the newrelic_rpm gem to 6.12. This can only be reproduced in production when using Newrelic and it didn't happen before the upgrade. Presumably Newrelic is splatting the arguments when it calls `update_all`, which causes symbol keys to be treated as keywords. --- app/models/post.rb | 12 ++---------- app/models/tag.rb | 14 ++++++-------- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/app/models/post.rb b/app/models/post.rb index d401bdf71..34a929e4c 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -400,24 +400,16 @@ class Post < ApplicationRecord set_tag_count(category, self.send("tag_count_#{category}") + 1) end - def set_tag_counts(disable_cache = true) + def set_tag_counts self.tag_count = 0 TagCategory.categories.each {|x| set_tag_count(x, 0)} - categories = Tag.categories_for(tag_array, :disable_caching => disable_cache) + categories = Tag.categories_for(tag_array, disable_caching: true) categories.each_value do |category| self.tag_count += 1 inc_tag_count(TagCategory.reverse_mapping[category]) end end - def fix_post_counts(post) - post.set_tag_counts(false) - if post.changes_saved? - args = Hash[TagCategory.categories.map {|x| ["tag_count_#{x}", post.send("tag_count_#{x}")]}].update(:tag_count => post.tag_count) - post.update_columns(args) - end - end - def merge_old_changes reset_tag_array_cache @removed_tags = [] diff --git a/app/models/tag.rb b/app/models/tag.rb index 2dc756445..731f2aee7 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -10,8 +10,8 @@ class Tag < ApplicationRecord validates :name, tag_name: true, on: :name validates_inclusion_of :category, in: TagCategory.category_ids - before_save :update_category_cache, if: :category_changed? - before_save :update_category_post_counts, if: :category_changed? + after_save :update_category_cache, if: :saved_change_to_category? + after_save :update_category_post_counts, if: :saved_change_to_category? scope :empty, -> { where("tags.post_count <= 0") } scope :nonempty, -> { where("tags.post_count > 0") } @@ -162,12 +162,10 @@ class Tag < ApplicationRecord end def update_category_post_counts - Post.with_timeout(30_000, nil, :tags => name) do - Post.raw_tag_match(name).where("true /* Tag#update_category_post_counts */").find_each do |post| - post.reload - post.set_tag_counts(false) - args = TagCategory.categories.map {|x| ["tag_count_#{x}", post.send("tag_count_#{x}")]}.to_h.update(:tag_count => post.tag_count) - Post.where(:id => post.id).update_all(args) + Post.with_timeout(30_000) do + Post.raw_tag_match(name).find_each do |post| + post.set_tag_counts + post.save! end end end