Skip to content

Commit

Permalink
FIX: Upsert topic votes count atomically (#71)
Browse files Browse the repository at this point in the history
The `Topic#update_vote_count` method should be concurrency-safe to prevent unique constraint violation errors when multiple processes try to insert a topic_votes_count record for the same topic simultaneously.

Signed-off-by: OsamaSayegh <asooomaasoooma90@gmail.com>
  • Loading branch information
OsamaSayegh committed Jan 7, 2021
1 parent 148d76f commit 8dbc75f
Show file tree
Hide file tree
Showing 5 changed files with 96 additions and 2 deletions.
14 changes: 14 additions & 0 deletions app/models/discourse_voting/category_setting.rb
Expand Up @@ -38,3 +38,17 @@ def reset_voting_cache
end
end
end

# == Schema Information
#
# Table name: discourse_voting_category_settings
#
# id :bigint not null, primary key
# category_id :integer
# created_at :datetime not null
# updated_at :datetime not null
#
# Indexes
#
# index_discourse_voting_category_settings_on_category_id (category_id) UNIQUE
#
15 changes: 15 additions & 0 deletions app/models/discourse_voting/topic_vote_count.rb
Expand Up @@ -7,3 +7,18 @@ class TopicVoteCount < ActiveRecord::Base
belongs_to :topic
end
end

# == Schema Information
#
# Table name: discourse_voting_topic_vote_count
#
# id :bigint not null, primary key
# topic_id :integer
# votes_count :integer
# created_at :datetime not null
# updated_at :datetime not null
#
# Indexes
#
# index_discourse_voting_topic_vote_count_on_topic_id (topic_id) UNIQUE
#
16 changes: 16 additions & 0 deletions app/models/discourse_voting/vote.rb
Expand Up @@ -8,3 +8,19 @@ class Vote < ActiveRecord::Base
belongs_to :topic
end
end

# == Schema Information
#
# Table name: discourse_voting_votes
#
# id :bigint not null, primary key
# topic_id :integer
# user_id :integer
# archive :boolean default(FALSE)
# created_at :datetime not null
# updated_at :datetime not null
#
# Indexes
#
# index_discourse_voting_votes_on_user_id_and_topic_id (user_id,topic_id) UNIQUE
#
12 changes: 10 additions & 2 deletions lib/discourse_voting/topic_extension.rb
Expand Up @@ -27,8 +27,16 @@ def user_voted?(user)
def update_vote_count
count = self.votes.count

topic_vote_count = self.topic_vote_count || DiscourseVoting::TopicVoteCount.new(topic: self)
topic_vote_count.update!(votes_count: count)
DB.exec(<<~SQL, topic_id: self.id, votes_count: count)
INSERT INTO discourse_voting_topic_vote_count
(topic_id, votes_count, created_at, updated_at)
VALUES
(:topic_id, :votes_count, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)
ON CONFLICT (topic_id) DO UPDATE SET
votes_count = :votes_count,
updated_at = CURRENT_TIMESTAMP
WHERE discourse_voting_topic_vote_count.topic_id = :topic_id
SQL
end

def who_voted
Expand Down
41 changes: 41 additions & 0 deletions spec/lib/topic_extension_spec.rb
@@ -0,0 +1,41 @@
# frozen_string_literal: true

require 'rails_helper'

describe DiscourseVoting::TopicExtension do
let(:user) { Fabricate(:user) }
let(:user2) { Fabricate(:user) }

let(:topic) { Fabricate(:topic) }
let(:topic2) { Fabricate(:topic) }

before do
SiteSetting.voting_enabled = true
SiteSetting.voting_show_who_voted = true
end

describe '#update_vote_count' do
it 'upserts topic votes count' do
topic.update_vote_count
topic2.update_vote_count

expect(topic.reload.topic_vote_count.votes_count).to eq(0)
expect(topic2.reload.topic_vote_count.votes_count).to eq(0)

DiscourseVoting::Vote.create!(user: user, topic: topic)
topic.update_vote_count
topic2.update_vote_count

expect(topic.reload.topic_vote_count.votes_count).to eq(1)
expect(topic2.reload.topic_vote_count.votes_count).to eq(0)

DiscourseVoting::Vote.create!(user: user2, topic: topic)
DiscourseVoting::Vote.create!(user: user, topic: topic2)
topic.update_vote_count
topic2.update_vote_count

expect(topic.reload.topic_vote_count.votes_count).to eq(2)
expect(topic2.reload.topic_vote_count.votes_count).to eq(1)
end
end
end

0 comments on commit 8dbc75f

Please sign in to comment.