add atomic appearance updates on vote / skip

Luke Baker
1 parent 9675033d
Showing 3 changed files with 43 additions and 19 deletions Show diff stats
app/models/visitor.rb
db/migrate/20111017171903_add_index_on_answerable_to_appearances.rb
db/schema.rb
@@ -21,18 +21,6 @@ class Visitor &lt; ActiveRecord::Base
     prompt = options.delete(:prompt)
     ordinality = (options.delete(:direction) == "left") ? 0 : 1
-    
-    if options[:appearance_lookup] 
-       @appearance = prompt.appearances.find_by_lookup(options.delete(:appearance_lookup))
-       return nil unless @appearance # don't allow people to fake appearance lookups
-
-       if @appearance.answered?
-          options.merge!(:valid_record => false)
-          options.merge!(:validity_information => "Appearance #{@appearance.id} already answered")
-       else
-          options.merge!(:appearance => @appearance)
-       end
-    end
     if options.delete(:skip_fraud_protection)
        last_answered_appearance = self.appearances.find(:first,
@@ -43,6 +31,13 @@ class Visitor &lt; ActiveRecord::Base
               options.merge!(:validity_information => "Fraud protection: last visitor action was a skip")
        end
     end
+
+    associate_appearance = false
+    if options[:appearance_lookup] 
+       @appearance = prompt.appearances.find_by_lookup(options.delete(:appearance_lookup))
+       return nil unless @appearance # don't allow people to fake appearance lookups
+      associate_appearance = true
+    end
     choice = prompt.choices[ordinality] #we need to guarantee that the choices are in the right order (by position)
     other_choices = prompt.choices - [choice]
@@ -51,6 +46,8 @@ class Visitor &lt; ActiveRecord::Base
     options.merge!(:question_id => prompt.question_id, :prompt_id => prompt.id, :voter_id=> self.id, :choice_id => choice.id, :loser_choice_id => loser_choice.id) 
     v = votes.create!(options)
+    safely_associate_appearance(v, @appearance) if associate_appearance
+    v
   end
   def skip!(options)
@@ -58,18 +55,35 @@ class Visitor &lt; ActiveRecord::Base
     prompt = options.delete(:prompt)
+    associate_appearance = false
     if options[:appearance_lookup]
       @appearance = prompt.appearances.find_by_lookup(options.delete(:appearance_lookup))
       return nil unless @appearance
-       if @appearance.answered?
-          options.merge!(:valid_record => false)
-          options.merge!(:validity_information => "Appearance #{@appearance.id} already answered")
-       else
-          options.merge!(:appearance => @appearance)
-       end
+      associate_appearance = true
     end
     options.merge!(:question_id => prompt.question_id, :prompt_id => prompt.id, :skipper_id => self.id)
     prompt_skip = skips.create!(options)
+    if associate_appearance
+      safely_associate_appearance(prompt_skip, @appearance)
+    end
+    prompt_skip
   end
+
+  # Safely associates appearance with object, but making sure no other object
+  # is already associated wit this appearance. object is either vote or skip.
+  def safely_associate_appearance(object, appearance)
+    # Manually update Appearance with id to ensure no double votes for a
+    # single appearance.  Only update the answerable_id if it is NULL.
+    # If we can't find any rows to update, then this object should be invalid.
+    rows_updated = Appearance.update_all("answerable_id = #{object.id}, answerable_type = '#{object.class.to_s}'", "id = #{appearance.id} AND answerable_id IS NULL")
+    if rows_updated === 1
+      # update relationship the ActiveRecord way, now
+      # that we know it is safe to do so
+      object.update_attributes!(:appearance => appearance)
+    else
+      object.update_attributes!(:valid_record => false, :validity_information => "Appearance #{appearance.id} already answered")
+    end
+  end
+
 end
@@ -0,0 +1,9 @@
+class AddIndexOnAnswerableToAppearances < ActiveRecord::Migration
+  def self.up
+    add_index :appearances, [:answerable_id, :answerable_type]
+  end
+
+  def self.down
+    remove_index :appearances, [:answerable_id, :answerable_type]
+  end
+end
@@ -9,7 +9,7 @@
 #
 # It's strongly recommended to check this file into your version control system.
-ActiveRecord::Schema.define(:version => 20110628160608) do
+ActiveRecord::Schema.define(:version => 20111017171903) do
   create_table "appearances", :force => true do |t|
     t.integer  "voter_id"
@@ -25,6 +25,7 @@ ActiveRecord::Schema.define(:version =&gt; 20110628160608) do
     t.string   "validity_information"
   end
+  add_index "appearances", ["answerable_id", "answerable_type"], :name => "index_appearances_on_answerable_id_and_answerable_type"
   add_index "appearances", ["lookup"], :name => "index_appearances_on_lookup"
   add_index "appearances", ["prompt_id"], :name => "index_appearances_on_prompt_id"
   add_index "appearances", ["question_id", "voter_id"], :name => "index_appearances_on_question_id_voter_id"
@@ -0,0 +1,9 @@		@@ -0,0 +1,9 @@
	1	+class AddIndexOnAnswerableToAppearances < ActiveRecord::Migration
	2	+ def self.up
	3	+ add_index :appearances, [:answerable_id, :answerable_type]
	4	+ end
	5	+
	6	+ def self.down
	7	+ remove_index :appearances, [:answerable_id, :answerable_type]
	8	+ end
	9	+end