From fd9dc6f647fa001df2414ffb0f1c46058a8eeae0 Mon Sep 17 00:00:00 2001 From: evazion Date: Fri, 28 Jul 2017 17:10:10 -0500 Subject: [PATCH 1/5] expunge: decrement upload and note/post update counts (fix #2062). --- app/models/note.rb | 2 -- app/models/note_version.rb | 2 +- app/models/post.rb | 2 +- app/models/post_archive.rb | 2 +- app/models/upload.rb | 1 - test/unit/post_test.rb | 27 ++++++++++++++++++++++++--- 6 files changed, 27 insertions(+), 9 deletions(-) diff --git a/app/models/note.rb b/app/models/note.rb index 00142815a..a20371366 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -147,8 +147,6 @@ class Note < ApplicationRecord def create_version return unless versioned_attributes_changed? - User.where(id: CurrentUser.id).update_all("note_update_count = note_update_count + 1") - CurrentUser.reload if merge_version? merge_version diff --git a/app/models/note_version.rb b/app/models/note_version.rb index 1ec07def1..c9c34a183 100644 --- a/app/models/note_version.rb +++ b/app/models/note_version.rb @@ -1,6 +1,6 @@ class NoteVersion < ApplicationRecord before_validation :initialize_updater - belongs_to :updater, :class_name => "User" + belongs_to :updater, :class_name => "User", :counter_cache => "note_update_count" scope :for_user, lambda {|user_id| where("updater_id = ?", user_id)} attr_accessible :note_id, :x, :y, :width, :height, :body, :updater_id, :updater_ip_addr, :is_active, :post_id, :html_id, :version diff --git a/app/models/post.rb b/app/models/post.rb index b389aa37b..e1056668f 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -36,7 +36,7 @@ class Post < ApplicationRecord belongs_to :updater, :class_name => "User" belongs_to :approver, :class_name => "User" - belongs_to :uploader, :class_name => "User" + belongs_to :uploader, :class_name => "User", :counter_cache => "post_upload_count" belongs_to :parent, :class_name => "Post" has_one :upload, :dependent => :destroy has_one :artist_commentary, :dependent => :destroy diff --git a/app/models/post_archive.rb b/app/models/post_archive.rb index bd2bf3a22..abdc7f98f 100644 --- a/app/models/post_archive.rb +++ b/app/models/post_archive.rb @@ -2,7 +2,7 @@ class PostArchive < ApplicationRecord extend Memoist belongs_to :post - belongs_to :updater, class_name: "User" + belongs_to :updater, class_name: "User", counter_cache: "post_update_count" def self.enabled? Danbooru.config.aws_sqs_archives_url.present? diff --git a/app/models/upload.rb b/app/models/upload.rb index 8563b1c99..c1b3adf14 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -139,7 +139,6 @@ class Upload < ApplicationRecord post = convert_to_post post.distribute_files if post.save - User.where(id: CurrentUser.id).update_all("post_upload_count = post_upload_count + 1") create_artist_commentary(post) if include_artist_commentary? ugoira_service.save_frame_data(post) if is_ugoira? notify_cropper(post) diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index 13b19c30a..f1151b706 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -61,6 +61,25 @@ class PostTest < ActiveSupport::TestCase assert_equal(0, Favorite.for_user(@user.id).where("post_id = ?", @post.id).count) end + should "decrement the uploader's upload count" do + assert_difference("@post.uploader.reload.post_upload_count", -1) do + @post.expunge! + end + end + + should "decrement the user's note update count" do + FactoryGirl.create(:note, post: @post) + assert_difference(["@post.uploader.reload.note_update_count"], -1) do + @post.expunge! + end + end + + should "decrement the user's post update count" do + assert_difference(["@post.uploader.reload.post_update_count"], -1) do + @post.expunge! + end + end + should "remove the post from iqdb" do mock_iqdb_service! Post.iqdb_sqs_service.expects(:send_message).with("remove\n#{@post.id}") @@ -1162,11 +1181,13 @@ class PostTest < ActiveSupport::TestCase should "increment the updater's post_update_count" do PostArchive.sqs_service.stubs(:merge?).returns(false) post = FactoryGirl.create(:post, :tag_string => "aaa bbb ccc") - CurrentUser.reload - assert_difference("CurrentUser.post_update_count", 1) do + # XXX in the test environment the update count gets bumped twice: and + # once by Post#post_update_count, and once by the counter cache. in + # production the counter cache doesn't bump the count, because + # versions are created on a separate server. + assert_difference("CurrentUser.user.reload.post_update_count", 2) do post.update_attributes(:tag_string => "zzz") - CurrentUser.reload end end From af42740ca91c0c5364ccf1ade4b4f99a06d0f5a1 Mon Sep 17 00:00:00 2001 From: evazion Date: Thu, 3 Aug 2017 22:45:10 -0500 Subject: [PATCH 2/5] expunge: decrement user favorite counts. --- app/models/favorite.rb | 15 +-------------- app/models/post.rb | 4 +++- test/unit/post_test.rb | 7 +++++++ 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/app/models/favorite.rb b/app/models/favorite.rb index 0db481935..545fe9102 100644 --- a/app/models/favorite.rb +++ b/app/models/favorite.rb @@ -4,19 +4,6 @@ class Favorite < ApplicationRecord scope :for_user, lambda {|user_id| where("user_id % 100 = #{user_id.to_i % 100} and user_id = #{user_id.to_i}")} attr_accessible :user_id, :post_id - # this is necessary because there's no trigger for deleting favorites - def self.destroy_all(user_id: nil, post_id: nil) - if user_id && post_id - connection.execute("delete from favorites_#{user_id % 100} where user_id = #{user_id} and post_id = #{post_id}") - elsif user_id - connection.execute("delete from favorites_#{user_id % 100} where user_id = #{user_id}") - elsif post_id - 0.upto(99) do |uid| - connection.execute("delete from favorites_#{uid} where post_id = #{post_id}") - end - end - end - def self.add(post:, user:) Favorite.transaction do User.where(:id => user.id).select("id").lock("FOR UPDATE NOWAIT").first @@ -40,7 +27,7 @@ class Favorite < ApplicationRecord User.where(:id => user.id).select("id").lock("FOR UPDATE NOWAIT").first return unless Favorite.for_user(user.id).where(:user_id => user.id, :post_id => post_id).exists? - Favorite.destroy_all(user_id: user.id, post_id: post_id) + Favorite.for_user(user.id).delete_all(post_id: post_id) Post.where(:id => post_id).update_all("fav_count = fav_count - 1") post.delete_user_from_fav_string(user.id) if post User.where(:id => user.id).update_all("favorite_count = favorite_count - 1") diff --git a/app/models/post.rb b/app/models/post.rb index e1056668f..48acc5676 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -1038,7 +1038,9 @@ class Post < ApplicationRecord end def remove_from_favorites - Favorite.destroy_all(post_id: self.id) + favorites.find_each do |fav| + remove_favorite!(fav.user) + end end def remove_from_fav_groups diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index f1151b706..d2a208102 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -80,6 +80,13 @@ class PostTest < ActiveSupport::TestCase end end + should "decrement the user's favorite count" do + @post.add_favorite!(@post.uploader) + assert_difference(["@post.uploader.reload.favorite_count"], -1) do + @post.expunge! + end + end + should "remove the post from iqdb" do mock_iqdb_service! Post.iqdb_sqs_service.expects(:send_message).with("remove\n#{@post.id}") From 5a6cc8481798760385b0996c29a28ee435615d58 Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 2 Aug 2017 18:25:01 -0500 Subject: [PATCH 3/5] favorites: don't regen fav count when adding favorites. Don't randomly regen the fav count when favoriting a post. This was a workaround for #1210 that is no longer needed. --- app/models/user.rb | 9 --------- test/unit/user_test.rb | 13 ------------- 2 files changed, 22 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 72f766edd..b2aba2ec8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -286,17 +286,8 @@ class User < ApplicationRecord Favorite.where("user_id % 100 = #{id % 100} and user_id = #{id}").order("id desc") end - def clean_favorite_count? - favorite_count < 0 || Kernel.rand(100) < [Math.log(favorite_count, 2), 5].min - end - - def clean_favorite_count! - update_column(:favorite_count, Favorite.for_user(id).count) - end - def add_favorite!(post) Favorite.add(post: post, user: self) - clean_favorite_count! if clean_favorite_count? end def remove_favorite!(post) diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index c2868596a..1ef2320e6 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -38,19 +38,6 @@ class UserTest < ActiveSupport::TestCase end end - context "favoriting a post" do - setup do - @user.update_column(:favorite_count, 999) - @user.stubs(:clean_favorite_count?).returns(true) - @post = FactoryGirl.create(:post) - end - - should "periodically clean the favorite_count" do - @user.add_favorite!(@post) - assert_equal(1, @user.favorite_count) - end - end - context "that has been invited by a mod" do setup do @mod = FactoryGirl.create(:moderator_user) From 1f3bafc0616a57ced652181dd0be029cf4ed77fa Mon Sep 17 00:00:00 2001 From: r888888888 Date: Wed, 13 Sep 2017 14:19:54 -0700 Subject: [PATCH 4/5] delegate removal from favorites and updating of user fav counts to delayed job --- app/models/favorite.rb | 9 ++++++++- app/models/favorite_group.rb | 18 ++++++++++++------ app/models/post.rb | 18 +++++++----------- test/unit/post_test.rb | 1 + 4 files changed, 28 insertions(+), 18 deletions(-) diff --git a/app/models/favorite.rb b/app/models/favorite.rb index 545fe9102..d8537b029 100644 --- a/app/models/favorite.rb +++ b/app/models/favorite.rb @@ -14,10 +14,17 @@ class Favorite < ApplicationRecord post.append_user_to_fav_string(user.id) User.where(:id => user.id).update_all("favorite_count = favorite_count + 1") user.favorite_count += 1 - # post.fav_count += 1 # this is handled in Post#clean_fav_string! end end + def self.purge_post(post_id, user_ids) + 0.upto(99) do |uid| + Favorite.where("user_id % 100 = ?", uid).delete_all(post_id: post_id) + end + + User.where(:id => user_ids).update_all("favorite_count = favorite_count - 1") + end + def self.remove(user:, post: nil, post_id: nil) Favorite.transaction do if post && post_id.nil? diff --git a/app/models/favorite_group.rb b/app/models/favorite_group.rb index 833fca988..d9d847392 100644 --- a/app/models/favorite_group.rb +++ b/app/models/favorite_group.rb @@ -172,18 +172,24 @@ class FavoriteGroup < ApplicationRecord self.post_count = post_id_array.size end - def add!(post) - return if contains?(post.id) + def add!(post_id) + return if contains?(post_id) clear_post_id_array - update_attributes(:post_ids => add_number_to_string(post.id, post_ids)) + update_attributes(:post_ids => add_number_to_string(post_id, post_ids)) end - def remove!(post) - return unless contains?(post.id) + def self.purge_post(post_id) + for_post(post_id).find_each do |group| + group.remove!(post_id) + end + end + + def remove!(post_id) + return unless contains?(post_id) clear_post_id_array - update_attributes(:post_ids => remove_number_from_string(post.id, post_ids)) + update_attributes(:post_ids => remove_number_from_string(post_id, post_ids)) end def add_number_to_string(number, string) diff --git a/app/models/post.rb b/app/models/post.rb index 48acc5676..1c567b36a 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -815,19 +815,19 @@ class Post < ApplicationRecord when /^-favgroup:(\d+)$/i favgroup = FavoriteGroup.where("id = ?", $1.to_i).for_creator(CurrentUser.user.id).first - favgroup.remove!(self) if favgroup + favgroup.remove!(id) if favgroup when /^-favgroup:(.+)$/i favgroup = FavoriteGroup.named($1).for_creator(CurrentUser.user.id).first - favgroup.remove!(self) if favgroup + favgroup.remove!(id) if favgroup when /^favgroup:(\d+)$/i favgroup = FavoriteGroup.where("id = ?", $1.to_i).for_creator(CurrentUser.user.id).first - favgroup.add!(self) if favgroup + favgroup.add!(id) if favgroup when /^favgroup:(.+)$/i favgroup = FavoriteGroup.named($1).for_creator(CurrentUser.user.id).first - favgroup.add!(self) if favgroup + favgroup.add!(id) if favgroup end end end @@ -1038,15 +1038,12 @@ class Post < ApplicationRecord end def remove_from_favorites - favorites.find_each do |fav| - remove_favorite!(fav.user) - end + Favorite.delay.purge_post(id, fav_string.scan(/\d+/)) + PostVote.where(post_id: id).delete_all end def remove_from_fav_groups - FavoriteGroup.for_post(id).find_each do |group| - group.remove!(self) - end + FavoriteGroup.delay.purge_post(id) end end @@ -1382,7 +1379,6 @@ class Post < ApplicationRecord transaction do Post.without_timeout do ModAction.log("permanently deleted post ##{id}") - #delete!("Permanently deleted post ##{id}", :without_mod_action => true) give_favorites_to_parent update_children_on_destroy diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index d2a208102..419e2bd33 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -21,6 +21,7 @@ class PostTest < ActiveSupport::TestCase CurrentUser.user = @user CurrentUser.ip_addr = "127.0.0.1" mock_saved_search_service! + ImageCropper.stubs(:enabled?).returns(false) end def teardown From 0bfd201973122f3edda410c540d0af1af71bd50f Mon Sep 17 00:00:00 2001 From: r888888888 Date: Wed, 13 Sep 2017 14:47:42 -0700 Subject: [PATCH 5/5] simplify logic --- app/models/favorite.rb | 8 -------- app/models/post.rb | 4 +++- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/app/models/favorite.rb b/app/models/favorite.rb index d8537b029..3b95696c7 100644 --- a/app/models/favorite.rb +++ b/app/models/favorite.rb @@ -17,14 +17,6 @@ class Favorite < ApplicationRecord end end - def self.purge_post(post_id, user_ids) - 0.upto(99) do |uid| - Favorite.where("user_id % 100 = ?", uid).delete_all(post_id: post_id) - end - - User.where(:id => user_ids).update_all("favorite_count = favorite_count - 1") - end - def self.remove(user:, post: nil, post_id: nil) Favorite.transaction do if post && post_id.nil? diff --git a/app/models/post.rb b/app/models/post.rb index 1c567b36a..b8960cc26 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -1038,7 +1038,9 @@ class Post < ApplicationRecord end def remove_from_favorites - Favorite.delay.purge_post(id, fav_string.scan(/\d+/)) + Favorite.delete_all(post_id: id) + user_ids = fav_string.scan(/\d+/) + User.where(:id => user_ids).update_all("favorite_count = favorite_count - 1") PostVote.where(post_id: id).delete_all end