Skip to content

Commit

Permalink
Fix races that lead to duplicate votes creation
Browse files Browse the repository at this point in the history
  • Loading branch information
fatkodima committed Nov 20, 2020
1 parent 585020d commit 7f88df8
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 21 deletions.
37 changes: 37 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# Change log

## master (unreleased)

### Bug Fixes

* [#127](https://github.com/ryanto/acts_as_votable/issues/127): Fix races that lead to duplicate votes creation. ([@fatkodima][])

For this to work, you need to update your database schema and existing data.
```ruby
# 1. Add a new column to the `votes` table.
add_column :votes, :uniqueness_token, :string

# 2. Manually remove erroneously created duplicate votes.

# 3. If you are using `duplicate: true` while voting in your codebase,
# update all those duplicated votes by setting a unique `uniqueness_token` for each of them,
# something like a unique hex value would suffice.

# 4. For other non duplicated votes update all of them setting the
# `uniqueness_token` field to "unique vote" string value.

# 5. Update `uniqueness_token` column to disallow NULL values.
change_column_null :votes, :uniqueness_token, false

# 6. Add a new unique index
add_index :votes, [:voter_id, :voter_type, :vote_scope, :votable_id, :votable_type, :uniqueness_token],
name: "index_votes_uniqueness", unique: true

# 7. Now you can delete the old index
remove_index :votes, [:voter_id, :voter_type, :vote_scope]
```

NOTE: Make sure you adapted that steps for your workload and run that commands safely
(adding/removing indexes concurrently, updating columns in batches, etc).

[@fatkodima]: https://github.com/fatkodima
10 changes: 10 additions & 0 deletions lib/acts_as_votable/votable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ def default_conditions
}
end

UNIQUE_VOTE_TOKEN = "unique vote"
private_constant :UNIQUE_VOTE_TOKEN

# voting
def vote_by(args = {})
return false if args[:voter].nil?
Expand All @@ -81,6 +84,13 @@ def vote_by(args = {})
voter: options[:voter],
vote_scope: options[:vote_scope]
)

if options[:duplicate]
require "securerandom"
vote.uniqueness_token = SecureRandom.hex
else
vote.uniqueness_token = UNIQUE_VOTE_TOKEN
end
else
# this voter is potentially changing his vote
vote = votes.last
Expand Down
5 changes: 3 additions & 2 deletions lib/acts_as_votable/vote.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ class Vote < ::ActiveRecord::Base
scope :for_type, ->(klass) { where(votable_type: klass.to_s) }
scope :by_type, ->(klass) { where(voter_type: klass.to_s) }

validates_presence_of :votable_id
validates_presence_of :voter_id
validates :votable_id, presence: true
validates :voter_id, presence: true,
uniqueness: { scope: [:voter_type, :vote_scope, :votable, :uniqueness_token] }
end
end
Original file line number Diff line number Diff line change
@@ -1,22 +1,20 @@
class ActsAsVotableMigration < ActiveRecord::Migration<%= migration_version %>
def self.up
def change
create_table :votes do |t|
t.references :votable, null: false, polymorphic: true, index: false
t.references :voter, null: false, polymorphic: true, index: false

t.references :votable, :polymorphic => true
t.references :voter, :polymorphic => true

t.boolean :vote_flag
t.boolean :vote_flag, null: false, default: true
t.string :vote_scope
t.integer :vote_weight
t.integer :vote_weight, null: false, default: 1
t.string :uniqueness_token, null: false # used for 'duplicate' feature

t.timestamps
end

add_index :votes, [:voter_id, :voter_type, :vote_scope]
add_index :votes, [:votable_id, :votable_type, :vote_scope]
end
t.index [:voter_id, :voter_type, :vote_scope, :votable_id, :votable_type, :uniqueness_token],
name: "index_votes_uniqueness", unique: true

def self.down
drop_table :votes
t.index [:votable_id, :votable_type, :vote_scope]
end
end
end
15 changes: 8 additions & 7 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,18 @@
t.references :votable, polymorphic: true
t.references :voter, polymorphic: true

t.boolean :vote_flag
t.boolean :vote_flag, null: false, default: true
t.string :vote_scope
t.integer :vote_weight
t.integer :vote_weight, null: false, default: 1
t.string :uniqueness_token, null: false

t.timestamps
end

add_index :votes, [:votable_id, :votable_type]
add_index :votes, [:voter_id, :voter_type]
add_index :votes, [:voter_id, :voter_type, :vote_scope]
add_index :votes, [:votable_id, :votable_type, :vote_scope]
t.index [:voter_id, :voter_type, :vote_scope, :votable_id, :votable_type, :uniqueness_token],
name: "index_votes_uniqueness", unique: true

t.index [:votable_id, :votable_type, :vote_scope]
end

create_table :voters do |t|
t.string :name
Expand Down

0 comments on commit 7f88df8

Please sign in to comment.