PERF: Cache user badge count in user_stats table (#8610)
This means that we no longer need to run a `count()` on the user_badges table when serializing a user
This commit is contained in:
parent
c15d702ae6
commit
9348d2cfdf
|
@ -114,6 +114,7 @@ class Badge < ActiveRecord::Base
|
|||
|
||||
after_commit do
|
||||
SvgSprite.expire_cache
|
||||
UserStat.update_distinct_badge_count if saved_change_to_enabled?
|
||||
end
|
||||
|
||||
# fields that can not be edited on system badges
|
||||
|
|
|
@ -945,7 +945,7 @@ class User < ActiveRecord::Base
|
|||
end
|
||||
|
||||
def badge_count
|
||||
user_badges.select('distinct badge_id').count
|
||||
user_stat&.distinct_badge_count
|
||||
end
|
||||
|
||||
def featured_user_badges(limit = 3)
|
||||
|
|
|
@ -18,11 +18,13 @@ class UserBadge < ActiveRecord::Base
|
|||
|
||||
after_create do
|
||||
Badge.increment_counter 'grant_count', self.badge_id
|
||||
UserStat.update_distinct_badge_count self.user_id
|
||||
DiscourseEvent.trigger(:user_badge_granted, self.badge_id, self.user_id)
|
||||
end
|
||||
|
||||
after_destroy do
|
||||
Badge.decrement_counter 'grant_count', self.badge_id
|
||||
UserStat.update_distinct_badge_count self.user_id
|
||||
DiscourseEvent.trigger(:user_badge_removed, self.badge_id, self.user_id)
|
||||
end
|
||||
|
||||
|
|
|
@ -6,6 +6,7 @@ class UserStat < ActiveRecord::Base
|
|||
|
||||
def self.ensure_consistency!(last_seen = 1.hour.ago)
|
||||
reset_bounce_scores
|
||||
update_distinct_badge_count
|
||||
update_view_counts(last_seen)
|
||||
update_first_unread(last_seen)
|
||||
end
|
||||
|
@ -126,6 +127,29 @@ class UserStat < ActiveRecord::Base
|
|||
SQL
|
||||
end
|
||||
|
||||
def self.update_distinct_badge_count(user_id = nil)
|
||||
sql = <<~SQL
|
||||
UPDATE user_stats
|
||||
SET distinct_badge_count = x.distinct_badge_count
|
||||
FROM (
|
||||
SELECT users.id user_id, COUNT(distinct user_badges.badge_id) distinct_badge_count
|
||||
FROM users
|
||||
LEFT JOIN user_badges ON user_badges.user_id = users.id
|
||||
AND (user_badges.badge_id IN (SELECT id FROM badges WHERE enabled))
|
||||
GROUP BY users.id
|
||||
) x
|
||||
WHERE user_stats.user_id = x.user_id AND user_stats.distinct_badge_count <> x.distinct_badge_count
|
||||
SQL
|
||||
|
||||
sql = sql + " AND user_stats.user_id = #{user_id.to_i}" if user_id
|
||||
|
||||
DB.exec sql
|
||||
end
|
||||
|
||||
def update_distinct_badge_count
|
||||
self.class.update_distinct_badge_count(self.user_id)
|
||||
end
|
||||
|
||||
# topic_reply_count is a count of posts in other users' topics
|
||||
def update_topic_reply_count
|
||||
self.topic_reply_count = Topic
|
||||
|
|
|
@ -0,0 +1,19 @@
|
|||
# frozen_string_literal: true
|
||||
class AddDistinctBadgeCountToUserStats < ActiveRecord::Migration[6.0]
|
||||
def change
|
||||
add_column :user_stats, :distinct_badge_count, :integer, default: 0, null: false
|
||||
|
||||
execute <<~SQL
|
||||
UPDATE user_stats
|
||||
SET distinct_badge_count = x.distinct_badge_count
|
||||
FROM (
|
||||
SELECT users.id user_id, COUNT(distinct user_badges.badge_id) distinct_badge_count
|
||||
FROM users
|
||||
LEFT JOIN user_badges ON user_badges.user_id = users.id
|
||||
AND (user_badges.badge_id IN (SELECT id FROM badges WHERE enabled))
|
||||
GROUP BY users.id
|
||||
) x
|
||||
WHERE user_stats.user_id = x.user_id AND user_stats.distinct_badge_count <> x.distinct_badge_count
|
||||
SQL
|
||||
end
|
||||
end
|
|
@ -130,4 +130,56 @@ describe UserStat do
|
|||
end
|
||||
|
||||
end
|
||||
|
||||
describe 'update_distinct_badge_count' do
|
||||
fab!(:user) { Fabricate(:user) }
|
||||
let(:stat) { user.user_stat }
|
||||
fab!(:badge1) { Fabricate(:badge) }
|
||||
fab!(:badge2) { Fabricate(:badge) }
|
||||
|
||||
it "updates counts correctly" do
|
||||
expect(stat.reload.distinct_badge_count).to eq(0)
|
||||
BadgeGranter.grant(badge1, user)
|
||||
expect(stat.reload.distinct_badge_count).to eq(1)
|
||||
BadgeGranter.grant(badge1, user)
|
||||
expect(stat.reload.distinct_badge_count).to eq(1)
|
||||
BadgeGranter.grant(badge2, user)
|
||||
expect(stat.reload.distinct_badge_count).to eq(2)
|
||||
user.reload.user_badges.destroy_all
|
||||
expect(stat.reload.distinct_badge_count).to eq(0)
|
||||
end
|
||||
|
||||
it "can update counts for all users simultaneously" do
|
||||
user2 = Fabricate(:user)
|
||||
stat2 = user2.user_stat
|
||||
|
||||
BadgeGranter.grant(badge1, user)
|
||||
BadgeGranter.grant(badge1, user)
|
||||
BadgeGranter.grant(badge2, user)
|
||||
|
||||
BadgeGranter.grant(badge1, user2)
|
||||
|
||||
# Set some clearly incorrect values
|
||||
stat.update(distinct_badge_count: 999)
|
||||
stat2.update(distinct_badge_count: 999)
|
||||
|
||||
UserStat.ensure_consistency!
|
||||
|
||||
expect(stat.reload.distinct_badge_count).to eq(2)
|
||||
expect(stat2.reload.distinct_badge_count).to eq(1)
|
||||
end
|
||||
|
||||
it "ignores disabled badges" do
|
||||
BadgeGranter.grant(badge1, user)
|
||||
BadgeGranter.grant(badge2, user)
|
||||
expect(stat.reload.distinct_badge_count).to eq(2)
|
||||
|
||||
badge2.update(enabled: false)
|
||||
expect(stat.reload.distinct_badge_count).to eq(1)
|
||||
|
||||
badge2.update(enabled: true)
|
||||
expect(stat.reload.distinct_badge_count).to eq(2)
|
||||
end
|
||||
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue