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

posts: stop updating fav_string attribute.

Browse Source 
Stop updating the fav_string attribute on posts. The column still exists
on the table, but is no longer used or updated.

Like the pool_string in 7d503f08, the fav_string was used in the past to
facilitate `fav:X` searches. Posts had a hidden fav_string column that
contained a list of every user who favorited the post. These were
treated like fake hidden tags on the post so that a search for `fav:X`
was treated like a tag search.

The fav_string attribute has been unused for search purposes for a while
now. It was only kept because of technicalities that required
departitioning the favorites table first (340e1008e) before it could be
removed. Basically, removing favorites with `@favorite.destroy` was
slow because Rails always deletes object by ID, but we didn't have an
index on favorites.id, and we couldn't easily add one until the
favorites table was departitioned.

Fixes #4652. See https://github.com/danbooru/danbooru/issues/4652#issuecomment-754993802
for more discussion of issues caused by the fav_string (in short: write
amplification, post table bloat, and favorite inconsistency problems).
This commit is contained in:
evazion
2021-10-07 23:32:38 -05:00
parent 5ce36b482f
commit 1653392361
17 changed files with 190 additions and 239 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
test/functional/explore/posts_controller_test.rb
View File

@@ -22,7 +22,7 @@ module Explore
context "#curated" do
should "render" do
@builder = create(:builder_user)
@post.add_favorite!(@builder)
Favorite.create!(post: @post, user: @builder)
get curated_explore_posts_path
assert_response :success
end

32
test/functional/favorites_controller_test.rb
View File

@@ -6,7 +6,7 @@ class FavoritesControllerTest < ActionDispatch::IntegrationTest
@user = create(:user)
@post = create(:post)
@faved_post = create(:post)
@faved_post.add_favorite!(@user)
create(:favorite, post: @faved_post, user: @user)
end
context "index action" do
@@ -33,30 +33,48 @@ class FavoritesControllerTest < ActionDispatch::IntegrationTest
context "create action" do
should "create a favorite for the current user" do
assert_difference("Favorite.count", 1) do
assert_difference [-> { @post.favorites.count }, -> { @post.reload.fav_count }, -> { @user.reload.favorite_count }], 1 do
post_auth favorites_path(post_id: @post.id), @user, as: :javascript
assert_response :redirect
end
end
should "not allow creating duplicate favorites" do
create(:favorite, post: @post, user: @user)
assert_no_difference [-> { @post.favorites.count }, -> { @post.reload.fav_count }, -> { @user.reload.favorite_count }] do
post_auth favorites_path(post_id: @post.id), @user, as: :javascript
assert_response :redirect
end
end
should "allow banned users to create favorites" do
assert_difference("Favorite.count", 1) do
post_auth favorites_path(post_id: @post.id), create(:banned_user), as: :javascript
@banned_user = create(:banned_user)
assert_difference [-> { @post.favorites.count }, -> { @post.reload.fav_count }, -> { @banned_user.reload.favorite_count }], 1 do
post_auth favorites_path(post_id: @post.id), @banned_user, as: :javascript
assert_response :redirect
end
end
should "not allow anonymous users to create favorites" do
assert_no_difference [-> { @post.favorites.count }, -> { @post.reload.fav_count }] do
post favorites_path(post_id: @post.id), as: :javascript
assert_response 403
end
end
end
context "destroy action" do
should "remove the favorite from the current user" do
assert_difference("Favorite.count", -1) do
should "remove the favorite for the current user" do
assert_difference [-> { @faved_post.favorites.count }, -> { @faved_post.reload.fav_count }, -> { @user.reload.favorite_count }], -1 do
delete_auth favorite_path(@faved_post.id), @user, as: :javascript
assert_response :redirect
end
end
should "allow banned users to destroy favorites" do
assert_difference("Favorite.count", -1) do
assert_difference [-> { @faved_post.favorites.count }, -> { @faved_post.reload.fav_count }, -> { @user.reload.favorite_count }], -1 do
delete_auth favorite_path(@faved_post.id), @user, as: :javascript
assert_response :redirect
end

2
test/functional/moderator/post/posts_controller_test.rb
View File

@@ -34,7 +34,7 @@ module Moderator
end
users = FactoryBot.create_list(:user, 2)
users.each do |u|
@child.add_favorite!(u)
Favorite.create!(post: @child, user: u)
@child.reload
end

6
test/jobs/delete_favorites_job_test.rb
View File

@@ -5,17 +5,17 @@ class DeleteFavoritesJobTest < ActiveJob::TestCase
should "delete all favorites" do
user = create(:user)
posts = create_list(:post, 3)
favorites = posts.each { |post| post.add_favorite!(user) }
favorites = posts.each { |post| Favorite.create!(post: post, user: user) }
assert_equal(3, user.favorite_count)
assert_equal(3, user.favorites.count)
assert_equal(3, Post.raw_tag_match("fav:#{user.id}").count)
assert_equal(3, Post.user_tag_match("fav:#{user.name}", user).count)
DeleteFavoritesJob.perform_now(user)
assert_equal(0, user.reload.favorite_count)
assert_equal(0, user.favorites.count)
assert_equal(0, Post.raw_tag_match("fav:#{user.id}").count)
assert_equal(0, Post.user_tag_match("fav:#{user.name}", user).count)
end
end
end

113
test/unit/favorite_test.rb
View File

@@ -2,50 +2,95 @@ require 'test_helper'
class FavoriteTest < ActiveSupport::TestCase
setup do
@user1 = FactoryBot.create(:user)
@user2 = FactoryBot.create(:user)
@p1 = FactoryBot.create(:post)
@p2 = FactoryBot.create(:post)
CurrentUser.user = @user1
CurrentUser.ip_addr = "127.0.0.1"
@user1 = create(:user)
@user2 = create(:user)
@p1 = create(:post)
@p2 = create(:post)
end
teardown do
CurrentUser.user = nil
CurrentUser.ip_addr = nil
end
context "Favorites: " do
context "removing a favorite" do
should "update the post and user favorite counts" do
fav = Favorite.create!(post: @p1, user: @user1)
context "A favorite" do
should "delete from all tables" do
@p1.add_favorite!(@user1)
assert_equal(1, @user1.favorite_count)
assert_equal(1, @user1.reload.favorite_count)
assert_equal(1, @p1.reload.fav_count)
Favorite.where(:user_id => @user1.id, :post_id => @p1.id).delete_all
assert_equal(0, Favorite.count)
Favorite.destroy_by(post: @p1, user: @user1)
assert_equal(0, @user1.reload.favorite_count)
assert_equal(0, @p1.reload.fav_count)
end
should "remove the upvote if the user could vote" do
@user = create(:gold_user)
@vote = create(:post_vote, post: @p1, user: @user, score: 1)
fav = Favorite.create!(post: @p1, user: @user)
assert_equal(1, @user.reload.favorite_count)
assert_equal(1, @p1.reload.fav_count)
assert_equal(1, @p1.reload.score)
assert(PostVote.positive.exists?(post: @p1, user: @user))
Favorite.destroy_by(post: @p1, user: @user)
assert_equal(0, @user.reload.favorite_count)
assert_equal(0, @p1.reload.fav_count)
assert_equal(0, @p1.reload.score)
refute(PostVote.positive.exists?(post: @p1, user: @user))
end
end
should "know which table it belongs to" do
@p1.add_favorite!(@user1)
@p2.add_favorite!(@user1)
@p1.add_favorite!(@user2)
context "adding a favorite" do
should "update the post and user favorite counts" do
Favorite.create!(post: @p1, user: @user1)
favorites = @user1.favorites.order("id desc")
assert_equal(2, favorites.count)
assert_equal(@p2.id, favorites[0].post_id)
assert_equal(@p1.id, favorites[1].post_id)
assert_equal(1, @user1.reload.favorite_count)
assert_equal(1, @p1.reload.fav_count)
end
favorites = @user2.favorites.order("id desc")
assert_equal(1, favorites.count)
assert_equal(@p1.id, favorites[0].post_id)
end
should "not upvote the post if the user can't vote" do
Favorite.create!(post: @p1, user: @user1)
should "not allow duplicates" do
@p1.add_favorite!(@user1)
error = assert_raises(Favorite::Error) { @p1.add_favorite!(@user1) }
assert_equal(1, @user1.reload.favorite_count)
assert_equal(1, @p1.reload.fav_count)
assert_equal(0, @p1.reload.score)
refute(PostVote.positive.exists?(post: @p1, user: @user1))
end
assert_equal("You have already favorited this post", error.message)
assert_equal(1, @user1.favorite_count)
should "upvote the post if the user can vote" do
@user = create(:gold_user)
Favorite.create!(post: @p1, user: @user)
assert_equal(1, @user.reload.favorite_count)
assert_equal(1, @p1.reload.fav_count)
assert_equal(1, @p1.reload.score)
assert(PostVote.positive.exists?(post: @p1, user: @user))
end
should "convert a downvote into an upvote if the post was downvoted" do
@user = create(:gold_user)
@vote = create(:post_vote, post: @p1, user: @user, score: -1)
assert_equal(-1, @p1.reload.score)
Favorite.create!(post: @p1, user: @user)
assert_equal(1, @user.reload.favorite_count)
assert_equal(1, @p1.reload.fav_count)
assert_equal(1, @p1.reload.score)
assert(PostVote.positive.exists?(post: @p1, user: @user))
refute(PostVote.negative.exists?(post: @p1, user: @user))
end
should "not allow duplicate favorites" do
@f1 = Favorite.create(post: @p1, user: @user1)
@f2 = Favorite.create(post: @p1, user: @user1)
assert_equal(["You have already favorited this post"], @f2.errors.full_messages)
assert_equal(1, @user1.reload.favorite_count)
assert_equal(1, @p1.reload.fav_count)
assert_equal(0, @p1.reload.score)
end
end
end
end

81
test/unit/post_test.rb
View File

@@ -33,7 +33,7 @@ class PostTest < ActiveSupport::TestCase
setup do
@upload = UploadService.new(FactoryBot.attributes_for(:jpg_upload)).start!
@post = @upload.post
Favorite.add(post: @post, user: @user)
Favorite.create!(post: @post, user: @user)
create(:favorite_group, post_ids: [@post.id])
perform_enqueued_jobs # perform IqdbAddPostJob
end
@@ -51,7 +51,9 @@ class PostTest < ActiveSupport::TestCase
should "remove all favorites" do
@post.expunge!
assert_equal(0, Favorite.for_user(@user.id).where("post_id = ?", @post.id).count)
assert_equal(0, @post.favorites.count)
assert_equal(0, @user.favorites.count)
assert_equal(0, @user.reload.favorite_count)
end
should "remove all favgroups" do
@@ -248,7 +250,7 @@ class PostTest < ActiveSupport::TestCase
p1 = FactoryBot.create(:post)
c1 = FactoryBot.create(:post, :parent_id => p1.id)
user = FactoryBot.create(:gold_user)
c1.add_favorite!(user)
create(:favorite, post: c1, user: user)
c1.delete!("test")
p1.reload
assert(Favorite.exists?(:post_id => c1.id, :user_id => user.id))
@@ -259,7 +261,7 @@ class PostTest < ActiveSupport::TestCase
p1 = FactoryBot.create(:post)
c1 = FactoryBot.create(:post, :parent_id => p1.id)
user = FactoryBot.create(:gold_user)
c1.add_favorite!(user)
create(:favorite, post: c1, user: user)
c1.delete!("test", :move_favorites => true)
p1.reload
assert(!Favorite.exists?(:post_id => c1.id, :user_id => user.id), "Child should not still have favorites")
@@ -278,7 +280,7 @@ class PostTest < ActiveSupport::TestCase
user = FactoryBot.create(:gold_user)
p1 = FactoryBot.create(:post)
c1 = FactoryBot.create(:post, :parent_id => p1.id)
c1.add_favorite!(user)
create(:favorite, post: c1, user: user)
assert_equal(true, p1.reload.has_active_children?)
c1.delete!("test", :move_favorites => true)
@@ -837,18 +839,18 @@ class PostTest < ActiveSupport::TestCase
context "for a fav" do
should "add/remove the current user to the post's favorite listing" do
@post.update(tag_string: "aaa fav:self")
assert_equal("fav:#{@user.id}", @post.fav_string)
assert_equal(1, @post.favorites.where(user: @user).count)
@post.update(tag_string: "aaa -fav:self")
assert_equal("", @post.fav_string)
assert_equal(0, @post.favorites.count)
end
should "not fail when the fav: metatag is used twice" do
@post.update(tag_string: "aaa fav:self fav:me")
assert_equal("fav:#{@user.id}", @post.fav_string)
assert_equal(1, @post.favorites.where(user: @user).count)
@post.update(tag_string: "aaa -fav:self -fav:me")
assert_equal("", @post.fav_string)
assert_equal(0, @post.favorites.count)
end
end
@@ -1531,33 +1533,33 @@ class PostTest < ActiveSupport::TestCase
setup do
@user = FactoryBot.create(:contributor_user)
@post = FactoryBot.create(:post)
@post.add_favorite!(@user)
create(:favorite, post: @post, user: @user)
@user.reload
end
should "decrement the user's favorite_count" do
assert_difference("@user.favorite_count", -1) do
@post.remove_favorite!(@user)
assert_difference("@user.reload.favorite_count", -1) do
Favorite.destroy_by(post: @post, user: @user)
end
end
should "decrement the post's score for gold users" do
assert_difference("@post.score", -1) do
@post.remove_favorite!(@user)
assert_difference("@post.reload.score", -1) do
Favorite.destroy_by(post: @post, user: @user)
end
end
should "not decrement the post's score for basic users" do
@member = FactoryBot.create(:user)
assert_no_difference("@post.score") { @post.add_favorite!(@member) }
assert_no_difference("@post.score") { @post.remove_favorite!(@member) }
assert_no_difference("@post.score") { create(:favorite, post: @post, user: @member) }
assert_no_difference("@post.score") { Favorite.destroy_by(post: @post, user: @member) }
end
should "not decrement the user's favorite_count if the user did not favorite the post" do
@post2 = FactoryBot.create(:post)
assert_no_difference("@user.favorite_count") do
@post2.remove_favorite!(@user)
Favorite.destroy_by(post: @post2, user: @user)
end
end
end
@@ -1568,53 +1570,22 @@ class PostTest < ActiveSupport::TestCase
@post = FactoryBot.create(:post)
end
should "periodically clean the fav_string" do
@post.update_column(:fav_string, "fav:1 fav:1 fav:1")
@post.update_column(:fav_count, 3)
@post.stubs(:clean_fav_string?).returns(true)
@post.append_user_to_fav_string(2)
assert_equal("fav:1 fav:2", @post.fav_string)
assert_equal(2, @post.fav_count)
end
should "increment the user's favorite_count" do
assert_difference("@user.favorite_count", 1) do
@post.add_favorite!(@user)
create(:favorite, post: @post, user: @user)
end
end
should "increment the post's score for gold users" do
@post.add_favorite!(@user)
assert_equal(1, @post.score)
create(:favorite, post: @post, user: @user)
assert_equal(1, @post.reload.score)
end
should "not increment the post's score for basic users" do
@member = FactoryBot.create(:user)
@post.add_favorite!(@member)
create(:favorite, post: @post, user: @member)
assert_equal(0, @post.score)
end
should "update the fav strings on the post" do
@post.add_favorite!(@user)
@post.reload
assert_equal("fav:#{@user.id}", @post.fav_string)
assert(Favorite.exists?(:user_id => @user.id, :post_id => @post.id))
assert_raises(Favorite::Error) { @post.add_favorite!(@user) }
@post.reload
assert_equal("fav:#{@user.id}", @post.fav_string)
assert(Favorite.exists?(:user_id => @user.id, :post_id => @post.id))
@post.remove_favorite!(@user)
@post.reload
assert_equal("", @post.fav_string)
assert(!Favorite.exists?(:user_id => @user.id, :post_id => @post.id))
@post.remove_favorite!(@user)
@post.reload
assert_equal("", @post.fav_string)
assert(!Favorite.exists?(:user_id => @user.id, :post_id => @post.id))
end
end
context "Moving favorites to a parent post" do
@@ -1625,8 +1596,8 @@ class PostTest < ActiveSupport::TestCase
@user1 = FactoryBot.create(:user, enable_private_favorites: true)
@gold1 = FactoryBot.create(:gold_user)
@child.add_favorite!(@user1)
@child.add_favorite!(@gold1)
create(:favorite, post: @child, user: @user1)
create(:favorite, post: @child, user: @gold1)
@child.give_favorites_to_parent
@child.reload
@@ -1636,12 +1607,10 @@ class PostTest < ActiveSupport::TestCase
should "move the favorites" do
assert_equal(0, @child.fav_count)
assert_equal(0, @child.favorites.count)
assert_equal("", @child.fav_string)
assert_equal([], @child.favorites.pluck(:user_id))
assert_equal(2, @parent.fav_count)
assert_equal(2, @parent.favorites.count)
assert_equal("fav:#{@user1.id} fav:#{@gold1.id}", @parent.fav_string)
assert_equal([@user1.id, @gold1.id], @parent.favorites.pluck(:user_id))
end

5
test/unit/user_deletion_test.rb
View File

@@ -69,13 +69,12 @@ class UserDeletionTest < ActiveSupport::TestCase
should "remove any favorites" do
@post = create(:post)
Favorite.add(post: @post, user: @user)
Favorite.create!(post: @post, user: @user)
perform_enqueued_jobs { @deletion.delete! }
assert_equal(0, Favorite.count)
assert_equal("", @post.reload.fav_string)
assert_equal(0, @post.fav_count)
assert_equal(0, @post.reload.fav_count)
end
end
end