diff --git a/lib/ahoy/database_store.rb b/lib/ahoy/database_store.rb index e93a999..8a9a0a4 100644 --- a/lib/ahoy/database_store.rb +++ b/lib/ahoy/database_store.rb @@ -1,7 +1,9 @@ module Ahoy class DatabaseStore < BaseStore def track_visit(data) - @visit = visit_model.create!(slice_data(visit_model, data)) + visit = visit_model.new(slice_data(visit_model, data)) + save_record!(visit) + @visit = visit rescue => e raise e unless unique_exception?(e) @@ -18,7 +20,7 @@ def track_event(data) event.visit = visit event.time = visit.started_at if event.time < visit.started_at begin - event.save! + save_record!(event) rescue => e raise e unless unique_exception?(e) end @@ -30,7 +32,7 @@ def track_event(data) def geocode(data) visit_token = data.delete(:visit_token) data = slice_data(visit_model, data) - if defined?(Mongoid::Document) && visit_model < Mongoid::Document + if mongoid?(visit_model) # upsert since visit might not be found due to eventual consistency visit_model.where(visit_token: visit_token).find_one_and_update({"$set": data}, {upsert: true}) elsif visit @@ -73,6 +75,28 @@ def visit_or_create(started_at: nil) protected + # Postgres aborts the entire transaction when a statement fails, so rescuing + # the unique violation is not enough when the caller is already inside a + # transaction -- the `visitable` macro creates visits from a before_create + # hook, and events are often tracked from model callbacks. Save inside a + # savepoint so the failed insert can be rolled back on its own. + # + # The exception has to escape the block for Active Record to issue + # ROLLBACK TO SAVEPOINT, so callers rescue outside of it. + def save_record!(record) + if mongoid?(record.class) + record.save! + else + record.class.transaction(requires_new: true) do + record.save! + end + end + end + + def mongoid?(model) + defined?(Mongoid::Document) && model < Mongoid::Document + end + def visit_model ::Ahoy::Visit end diff --git a/test/tracker_test.rb b/test/tracker_test.rb index f71fd0f..e3cda03 100644 --- a/test/tracker_test.rb +++ b/test/tracker_test.rb @@ -37,6 +37,26 @@ def test_no_cookies_no_request assert_nil event.user_id end + # Postgres aborts the entire transaction when a statement fails, so rescuing + # the unique violation in Ruby is not enough to leave the transaction usable. + # The `visitable` macro creates visits from a before_create hook, i.e. inside + # the record's own transaction. + def test_duplicate_visit_token_does_not_abort_transaction + skip if ENV["ADAPTER"] == "mongoid" + + visit_token = SecureRandom.uuid + Ahoy::Visit.create!(visit_token: visit_token, started_at: Time.current) + + ActiveRecord::Base.transaction do + # A concurrent request already created this visit. + Ahoy::Tracker.new(visit_token: visit_token).track_visit + User.create!(name: "Test") + end + + assert_equal 1, Ahoy::Visit.count + assert_equal 1, User.count + end + def test_user_option user = Struct.new(:id).new(123) ahoy = Ahoy::Tracker.new(user: user)