diff --git a/CHANGELOG.md b/CHANGELOG.md index efc2e01..ed2fab3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,3 +52,6 @@ # Version 5.3.0 * Changed salesforce_ar_sync to use bang (!) versions of Restforce CRUD methods to raise errors on Salesforce save failures instead of returning false. + +# Version 5.3.1 +* Fix race condition in app-level upsert by retrying on ActiveRecord::RecordNotUnique \ No newline at end of file diff --git a/lib/salesforce_ar_sync/salesforce_sync.rb b/lib/salesforce_ar_sync/salesforce_sync.rb index 7efffc2..5082661 100644 --- a/lib/salesforce_ar_sync/salesforce_sync.rb +++ b/lib/salesforce_ar_sync/salesforce_sync.rb @@ -71,24 +71,49 @@ module ClassMethods def salesforce_update(attributes = {}) raise ArgumentError, "#{salesforce_id_attribute_name} parameter required" if attributes[salesforce_id_attribute_name].blank? + attempt_salesforce_update(attributes) + end + + private + + def find_record(attributes) data_source = unscoped_updates ? unscoped : self object = data_source.find_by(salesforce_id: attributes[salesforce_id_attribute_name]) object ||= data_source.find_by(activerecord_web_id_attribute_name => attributes[salesforce_web_id_attribute_name]) if salesforce_sync_web_id? && attributes[salesforce_web_id_attribute_name] + return object if object.present? - if !object && additional_lookup_fields + if additional_lookup_fields additional_lookup_fields.each do |attribute_name, salesforce_attribute_name| object = data_source.find_by(attribute_name => attributes[salesforce_attribute_name]) if attributes[salesforce_attribute_name] + break if object end + return object end + end - if object.nil? - object = new + def build_record(attributes) + new.tap do |object| salesforce_default_attributes_for_create.merge(salesforce_id: attributes[salesforce_id_attribute_name]).each_pair do |k, v| object.send("#{k}=", v) end end + end + + def attempt_salesforce_update(attributes, retried: false) + object = find_record(attributes) || build_record(attributes) + return unless object + + object.salesforce_process_update(attributes) if salesforce_stale?(object, attributes[:SystemModstamp]) + rescue ActiveRecord::RecordNotUnique + raise if retried + + attempt_salesforce_update(attributes, retried: true) + end + + def salesforce_stale?(object, modstamp) + return false if modstamp.blank? - object.salesforce_process_update(attributes) if object && (object.salesforce_updated_at.nil? || (object.salesforce_updated_at && object.salesforce_updated_at < Time.parse(attributes[:SystemModstamp]))) + object.salesforce_updated_at.nil? || object.salesforce_updated_at < Time.parse(modstamp) end end diff --git a/lib/salesforce_ar_sync/version.rb b/lib/salesforce_ar_sync/version.rb index 11e387c..cfb6242 100644 --- a/lib/salesforce_ar_sync/version.rb +++ b/lib/salesforce_ar_sync/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module SalesforceArSync - VERSION = '5.3.0' + VERSION = '5.3.1' end diff --git a/spec/salesforce_ar_sync/salesforce_ar_sync_spec.rb b/spec/salesforce_ar_sync/salesforce_ar_sync_spec.rb index e2bba45..d1f191e 100644 --- a/spec/salesforce_ar_sync/salesforce_ar_sync_spec.rb +++ b/spec/salesforce_ar_sync/salesforce_ar_sync_spec.rb @@ -137,13 +137,48 @@ class TestSyncable < ActiveRecord::Base it 'looks for unscoped records when unscoped_updates is set' do sf_id = 1 contact = Contact.new(salesforce_id: sf_id) + modstamp = '2012-03-26T19:54:50.000Z' allow(Contact).to receive(:unscoped_updates).and_return(true) expect(contact).to receive(:salesforce_process_update) { nil } expect(Contact).to receive(:unscoped).and_return(self) expect(self).to receive(:find_by).with(salesforce_id: sf_id).and_return(contact) - Contact.salesforce_update(Id: sf_id) + Contact.salesforce_update(Id: sf_id, SystemModstamp: modstamp) + end + + context 'when an existing record is not found but ActiveRecord::RecordNotUnique is raised on save' do + let(:contact) do + Contact.new( + first_name: 'Bob', + last_name: 'Smith', + phone: '519 555-1212', + email: 'bsmith@example.com', + salesforce_skip_sync: true + ) + end + + before do + allow(Contact).to receive(:salesforce_sync_web_id?).and_return(false) + allow(Contact).to receive(:unscoped_updates?).and_return(false) + # simulate the record being created after the find but before the save + allow(Contact).to receive(:new).and_return(contact) + allow(contact).to receive(:save!).once do + contact.save + raise(ActiveRecord::RecordNotUnique) + end + end + + it 'retries and updates the found record' do + expect(Contact).to receive(:new).once + expect(Contact).to receive(:find_by).with(salesforce_id: 321).twice.and_call_original + + expect { + Contact.salesforce_update(Id: 321, SystemModstamp: '2024-01-01T00:00:00.000Z') + }.not_to raise_exception + + expect(Contact.where(salesforce_id: 321)).to exist + end end end