Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

PERF: Cache ranks for featured badges, to simplify user serialization (#8698)

This commit is contained in:
David Taylor 2020-01-14 14:26:49 +00:00 committed by GitHub
parent e474cda321
commit cff6e941de
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 157 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
app/jobs/scheduled/badge_grant.rb
View File

@ -22,6 +22,7 @@ module Jobs
end
BadgeGranter.revoke_ungranted_titles!
UserBadge.ensure_consistency! # Badge granter sometimes uses raw SQL, so hooks do not run. Clean up data
end
end

1
app/models/badge.rb
View File

@ -115,6 +115,7 @@ class Badge < ActiveRecord::Base
after_commit do
SvgSprite.expire_cache
UserStat.update_distinct_badge_count if saved_change_to_enabled?
UserBadge.ensure_consistency! if saved_change_to_enabled?
end
# fields that can not be edited on system badges

36
app/models/user.rb
View File

@ -20,8 +20,14 @@ class User < ActiveRecord::Base
has_many :user_actions
has_many :post_actions
has_many :user_badges, -> { where('user_badges.badge_id IN (SELECT id FROM badges WHERE enabled)') }, dependent: :destroy
DEFAULT_FEATURED_BADGE_COUNT = 3
has_many :user_badges, -> { for_enabled_badges }, dependent: :destroy
has_many :badges, through: :user_badges
has_many :default_featured_user_badges,
-> { for_enabled_badges.grouped_with_count.where("featured_rank <= ?", DEFAULT_FEATURED_BADGE_COUNT) },
class_name: "UserBadge"
has_many :email_logs, dependent: :delete_all
has_many :incoming_emails, dependent: :delete_all
has_many :post_timings
@ -961,28 +967,12 @@ class User < ActiveRecord::Base
user_stat&.distinct_badge_count
end
def featured_user_badges(limit = 3)
tl_badge_ids = Badge.trust_level_badge_ids
query = user_badges
.group(:badge_id)
.select(UserBadge.attribute_names.map { |x| "MAX(user_badges.#{x}) AS #{x}" },
'COUNT(*) AS "count"',
'MAX(badges.badge_type_id) AS badges_badge_type_id',
'MAX(badges.grant_count) AS badges_grant_count')
.joins(:badge)
.order('badges_badge_type_id ASC, badges_grant_count ASC, badge_id DESC')
.includes(:user, :granted_by, { badge: :badge_type }, post: :topic)
tl_badge = query.where("user_badges.badge_id IN (:tl_badge_ids)",
tl_badge_ids: tl_badge_ids)
.limit(1)
other_badges = query.where("user_badges.badge_id NOT IN (:tl_badge_ids)",
tl_badge_ids: tl_badge_ids)
.limit(limit)
(tl_badge + other_badges).take(limit)
def featured_user_badges(limit = DEFAULT_FEATURED_BADGE_COUNT)
if limit == DEFAULT_FEATURED_BADGE_COUNT
default_featured_user_badges
else
user_badges.grouped_with_count.where("featured_rank <= ?", limit)
end
end
def self.count_by_signup_date(start_date = nil, end_date = nil, group_id = nil)

54
app/models/user_badge.rb
View File

@ -7,6 +7,16 @@ class UserBadge < ActiveRecord::Base
belongs_to :notification, dependent: :destroy
belongs_to :post
scope :grouped_with_count, -> {
group(:badge_id, :user_id)
.select(UserBadge.attribute_names.map { |x| "MAX(user_badges.#{x}) AS #{x}" },
'COUNT(*) AS "count"')
.order('MAX(featured_rank) ASC')
.includes(:user, :granted_by, { badge: :badge_type }, post: :topic)
}
scope :for_enabled_badges, -> { where('user_badges.badge_id IN (SELECT id FROM badges WHERE enabled)') }
validates :badge_id,
presence: true,
uniqueness: { scope: :user_id },
@ -19,15 +29,58 @@ class UserBadge < ActiveRecord::Base
after_create do
Badge.increment_counter 'grant_count', self.badge_id
UserStat.update_distinct_badge_count self.user_id
UserBadge.update_featured_ranks! 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
UserBadge.update_featured_ranks! self.user_id
DiscourseEvent.trigger(:user_badge_removed, self.badge_id, self.user_id)
end
def self.ensure_consistency!
self.update_featured_ranks!
end
def self.update_featured_ranks!(user_id = nil)
query = <<~SQL
WITH featured_tl_badge AS -- Find the best trust level badge for each user
(
SELECT user_id, max(badge_id) as badge_id
FROM user_badges
WHERE badge_id IN (1,2,3,4)
#{"AND user_id = #{user_id.to_i}" if user_id}
GROUP BY user_id
),
ranks AS ( -- Take all user badges, group by user_id and badge_id, and calculate a rank for each one
SELECT
user_badges.user_id,
user_badges.badge_id,
RANK() OVER (
PARTITION BY user_badges.user_id -- Do a separate rank for each user
ORDER BY BOOL_OR(badges.enabled) DESC, -- Disabled badges last
MAX(featured_tl_badge.user_id) NULLS LAST, -- Best tl badge first
CASE WHEN user_badges.badge_id IN (1,2,3,4) THEN 1 ELSE 0 END ASC, -- Non-featured tl badges last
MAX(badges.badge_type_id) ASC,
MAX(badges.grant_count) ASC,
user_badges.badge_id DESC
) rank_number
FROM user_badges
INNER JOIN badges ON badges.id = user_badges.badge_id
LEFT JOIN featured_tl_badge ON featured_tl_badge.user_id = user_badges.user_id AND featured_tl_badge.badge_id = user_badges.badge_id
#{"WHERE user_badges.user_id = #{user_id.to_i}" if user_id}
GROUP BY user_badges.user_id, user_badges.badge_id
)
-- Now use that data to update the featured_rank column
UPDATE user_badges SET featured_rank = rank_number
FROM ranks WHERE ranks.badge_id = user_badges.badge_id AND ranks.user_id = user_badges.user_id AND featured_rank IS DISTINCT FROM rank_number
SQL
DB.exec query
end
private
def single_grant_badge?
@ -47,6 +100,7 @@ end
# post_id :integer
# notification_id :integer
# seq :integer default(0), not null
# featured_rank :integer
#
# Indexes
#

37
db/migrate/20200107161405_add_featured_rank_to_user_badges.rb Normal file
View File

@ -0,0 +1,37 @@
# frozen_string_literal: true
class AddFeaturedRankToUserBadges < ActiveRecord::Migration[6.0]
def change
add_column :user_badges, :featured_rank, :integer, null: true
execute <<~SQL
WITH featured_tl_badge AS -- Find the best trust level badge for each user
(
SELECT user_id, max(badge_id) as badge_id
FROM user_badges
WHERE badge_id IN (1,2,3,4)
GROUP BY user_id
),
ranks AS ( -- Take all user badges, group by user_id and badge_id, and calculate a rank for each one
SELECT
user_badges.user_id,
user_badges.badge_id,
RANK() OVER (
PARTITION BY user_badges.user_id -- Do a separate rank for each user
ORDER BY BOOL_OR(badges.enabled) DESC, -- Disabled badges last
MAX(featured_tl_badge.user_id) NULLS LAST, -- Best tl badge first
CASE WHEN user_badges.badge_id IN (1,2,3,4) THEN 1 ELSE 0 END ASC, -- Non-featured tl badges last
MAX(badges.badge_type_id) ASC,
MAX(badges.grant_count) ASC,
user_badges.badge_id DESC
) rank_number
FROM user_badges
INNER JOIN badges ON badges.id = user_badges.badge_id
LEFT JOIN featured_tl_badge ON featured_tl_badge.user_id = user_badges.user_id AND featured_tl_badge.badge_id = user_badges.badge_id
GROUP BY user_badges.user_id, user_badges.badge_id
)
-- Now use that data to update the featured_rank column
UPDATE user_badges SET featured_rank = rank_number
FROM ranks WHERE ranks.badge_id = user_badges.badge_id AND ranks.user_id = user_badges.user_id
SQL
end
end

46
spec/models/user_badge_spec.rb
View File

@ -16,4 +16,50 @@ describe UserBadge do
it { is_expected.to validate_uniqueness_of(:badge_id).scoped_to(:user_id) }
end
describe "featured rank" do
fab!(:user) { Fabricate(:user) }
fab!(:user_badge_tl1) { UserBadge.create!(badge_id: Badge::BasicUser, user: user, granted_by: Discourse.system_user, granted_at: Time.now) }
fab!(:user_badge_tl2) { UserBadge.create!(badge_id: Badge::Member, user: user, granted_by: Discourse.system_user, granted_at: Time.now) }
fab!(:user_badge_wiki) { UserBadge.create!(badge_id: Badge::WikiEditor, user: user, granted_by: Discourse.system_user, granted_at: Time.now) }
fab!(:user_badge_like) { UserBadge.create!(badge_id: Badge::FirstLike, user: user, granted_by: Discourse.system_user, granted_at: Time.now) }
it "gives user badges the correct rank" do
expect(user_badge_tl2.reload.featured_rank).to eq(1)
expect(user_badge_wiki.reload.featured_rank).to eq(2)
expect(user_badge_like.reload.featured_rank).to eq(3)
expect(user_badge_tl1.reload.featured_rank).to eq(4) # Previous trust level badges last
end
it "gives duplicate user_badges the same rank" do
ub1 = UserBadge.create!(badge_id: Badge::GreatTopic, user: user, granted_by: Discourse.system_user, granted_at: Time.now)
ub2 = UserBadge.create!(badge_id: Badge::GreatTopic, user: user, granted_by: Discourse.system_user, granted_at: Time.now, seq: 1)
expect(ub1.reload.featured_rank).to eq(2)
expect(ub2.reload.featured_rank).to eq(2)
end
it "skips disabled badges" do
user_badge_wiki.badge.update(enabled: false)
expect(user_badge_tl2.reload.featured_rank).to eq(1)
expect(user_badge_like.reload.featured_rank).to eq(2)
expect(user_badge_tl1.reload.featured_rank).to eq(3) # Previous trust level badges last
expect(user_badge_wiki.reload.featured_rank).to eq(4) # Disabled
end
it "can ensure consistency per user" do
user_badge_tl2.update_column(:featured_rank, 20) # Update without hooks
expect(user_badge_tl2.reload.featured_rank).to eq(20) # Double check
UserBadge.update_featured_ranks! user.id
expect(user_badge_tl2.reload.featured_rank).to eq(1)
end
it "can ensure consistency for all users" do
user_badge_tl2.update_column(:featured_rank, 20) # Update without hooks
expect(user_badge_tl2.reload.featured_rank).to eq(20) # Double check
UserBadge.update_featured_ranks!
expect(user_badge_tl2.reload.featured_rank).to eq(1)
end
end
end

13
spec/models/user_spec.rb
View File

@ -1706,15 +1706,12 @@ describe User do
describe "#featured_user_badges" do
fab!(:user) { Fabricate(:user) }
let!(:user_badge_tl1) { UserBadge.create(badge_id: 1, user: user, granted_by: Discourse.system_user, granted_at: Time.now) }
let!(:user_badge_tl2) { UserBadge.create(badge_id: 2, user: user, granted_by: Discourse.system_user, granted_at: Time.now) }
let!(:user_badge_tl1) { UserBadge.create(badge_id: Badge::BasicUser, user: user, granted_by: Discourse.system_user, granted_at: Time.now) }
let!(:user_badge_tl2) { UserBadge.create(badge_id: Badge::Member, user: user, granted_by: Discourse.system_user, granted_at: Time.now) }
let!(:user_badge_like) { UserBadge.create(badge_id: Badge::FirstLike, user: user, granted_by: Discourse.system_user, granted_at: Time.now) }
it 'should display highest trust level badge first' do
expect(user.featured_user_badges[0].badge_id).to eq(2)
end
it 'should display only 1 trust level badge' do
expect(user.featured_user_badges.length).to eq(1)
it 'should display badges in the correct order' do
expect(user.featured_user_badges.map(&:badge_id)).to eq([Badge::Member, Badge::FirstLike, Badge::BasicUser])
end
end