From e222f153272694467be2b6d8505f4eef3065a0b1 Mon Sep 17 00:00:00 2001 From: Dave Pijuan-Nomura Date: Tue, 14 Apr 2026 15:53:53 -0400 Subject: [PATCH 1/4] Handle race condition --- lib/salesforce_ar_sync/salesforce_sync.rb | 28 +++++++++++++++---- .../salesforce_ar_sync_spec.rb | 28 +++++++++++++++++++ 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/lib/salesforce_ar_sync/salesforce_sync.rb b/lib/salesforce_ar_sync/salesforce_sync.rb index 7efffc2..6fe0a81 100644 --- a/lib/salesforce_ar_sync/salesforce_sync.rb +++ b/lib/salesforce_ar_sync/salesforce_sync.rb @@ -71,24 +71,42 @@ module ClassMethods def salesforce_update(attributes = {}) raise ArgumentError, "#{salesforce_id_attribute_name} parameter required" if attributes[salesforce_id_attribute_name].blank? + retried = false + begin + object = find_record(attributes) || build_record(attributes) + + object.salesforce_process_update(attributes) if object && (object.salesforce_updated_at.nil? || (object.salesforce_updated_at && object.salesforce_updated_at < Time.parse(attributes[:SystemModstamp]))) + rescue ActiveRecord::RecordNotUnique + if retried + raise + else + retried = true + retry + end + end + end + + 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 - - object.salesforce_process_update(attributes) if object && (object.salesforce_updated_at.nil? || (object.salesforce_updated_at && object.salesforce_updated_at < Time.parse(attributes[:SystemModstamp]))) end 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..3590b75 100644 --- a/spec/salesforce_ar_sync/salesforce_ar_sync_spec.rb +++ b/spec/salesforce_ar_sync/salesforce_ar_sync_spec.rb @@ -145,6 +145,34 @@ class TestSyncable < ActiveRecord::Base Contact.salesforce_update(Id: sf_id) 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 + let(:return_values) { [:raise, true] } + + 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!).exactly(2).times do + return_value = return_values.shift + return_value == :raise ? (contact.save and raise(ActiveRecord::RecordNotUnique)) : return_value + end + end + it 'retries and updates the found record' do + expect(Contact).to receive(:new).once + expect { Contact.salesforce_update(Id: 321, WebId__c: 432) }.not_to raise_exception + end + end end describe '.salesforce_id_attribute_name' do From 141e5807c36e793b10ac4dca6b7e11907b824bcf Mon Sep 17 00:00:00 2001 From: Alejandro Torres Date: Wed, 24 Jun 2026 18:06:14 -0300 Subject: [PATCH 2/4] Give better structure to the fix --- lib/salesforce_ar_sync/salesforce_sync.rb | 33 ++++++++++++++--------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/lib/salesforce_ar_sync/salesforce_sync.rb b/lib/salesforce_ar_sync/salesforce_sync.rb index 6fe0a81..5082661 100644 --- a/lib/salesforce_ar_sync/salesforce_sync.rb +++ b/lib/salesforce_ar_sync/salesforce_sync.rb @@ -71,21 +71,11 @@ module ClassMethods def salesforce_update(attributes = {}) raise ArgumentError, "#{salesforce_id_attribute_name} parameter required" if attributes[salesforce_id_attribute_name].blank? - retried = false - begin - object = find_record(attributes) || build_record(attributes) - - object.salesforce_process_update(attributes) if object && (object.salesforce_updated_at.nil? || (object.salesforce_updated_at && object.salesforce_updated_at < Time.parse(attributes[:SystemModstamp]))) - rescue ActiveRecord::RecordNotUnique - if retried - raise - else - retried = true - retry - end - end + 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]) @@ -108,6 +98,23 @@ def build_record(attributes) 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_updated_at.nil? || object.salesforce_updated_at < Time.parse(modstamp) + end end # if this instance variable is set to true, the salesforce_sync method will return without attempting From 939483cd68c3a7755e8d944e3b752cdd2643a66d Mon Sep 17 00:00:00 2001 From: Alejandro Torres Date: Thu, 25 Jun 2026 16:51:55 -0300 Subject: [PATCH 3/4] Fix spec and change some validations --- .../salesforce_ar_sync_spec.rb | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/spec/salesforce_ar_sync/salesforce_ar_sync_spec.rb b/spec/salesforce_ar_sync/salesforce_ar_sync_spec.rb index 3590b75..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,14 @@ 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 @@ -156,21 +157,27 @@ class TestSyncable < ActiveRecord::Base salesforce_skip_sync: true ) end - let(:return_values) { [:raise, true] } 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!).exactly(2).times do - return_value = return_values.shift - return_value == :raise ? (contact.save and raise(ActiveRecord::RecordNotUnique)) : return_value + 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.salesforce_update(Id: 321, WebId__c: 432) }.not_to raise_exception + 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 From 73bb52d75b3e4f3c94fe783b35c753c81709c3e6 Mon Sep 17 00:00:00 2001 From: Alejandro Torres Date: Tue, 30 Jun 2026 15:44:51 -0300 Subject: [PATCH 4/4] Add new version and changelog for new lib version --- CHANGELOG.md | 3 +++ lib/salesforce_ar_sync/version.rb | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) 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/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