diff --git a/app/models/otp_requests_tracker.rb b/app/models/otp_requests_tracker.rb index 64ae71195e0..e95ca79b878 100644 --- a/app/models/otp_requests_tracker.rb +++ b/app/models/otp_requests_tracker.rb @@ -1,13 +1,9 @@ class OtpRequestsTracker < ApplicationRecord def self.find_or_create_with_phone_and_confirmed(phone, phone_confirmed) - tries = 1 - phone_fingerprint = Pii::Fingerprinter.fingerprint(phone.strip) - - where(phone_fingerprint: phone_fingerprint, phone_confirmed: phone_confirmed). - first_or_create(otp_send_count: 0, otp_last_sent_at: Time.zone.now) - rescue ActiveRecord::RecordNotUnique - retry unless (tries -= 1).zero? - raise + create_or_find_by( + phone_fingerprint: Pii::Fingerprinter.fingerprint(phone.strip), + phone_confirmed: phone_confirmed, + ) end def self.atomic_increment(id) diff --git a/spec/models/otp_requests_tracker_spec.rb b/spec/models/otp_requests_tracker_spec.rb index a1a1868425d..75230f999e0 100644 --- a/spec/models/otp_requests_tracker_spec.rb +++ b/spec/models/otp_requests_tracker_spec.rb @@ -26,26 +26,13 @@ end context 'match not found' do - it 'creates new record with otp_send_count = 0 and otp_last_sent_at = current time' do + it 'creates new record with otp_send_count = 0' do expect { OtpRequestsTracker.find_or_create_with_phone_and_confirmed(phone, true) }. to change(OtpRequestsTracker, :count).by(1) existing = OtpRequestsTracker.where(phone_fingerprint: phone_fingerprint).first expect(existing.otp_send_count).to eq 0 - expect(existing.otp_last_sent_at).to be_within(2.seconds).of(Time.zone.now) - end - end - - context 'race condition' do - it 'retries once, then raises ActiveRecord::RecordNotUnique' do - tracker = OtpRequestsTracker.new - allow(OtpRequestsTracker).to receive(:where). - and_raise(ActiveRecord::RecordNotUnique.new(tracker)) - - expect(OtpRequestsTracker).to receive(:where).exactly(:once) - expect { OtpRequestsTracker.find_or_create_with_phone_and_confirmed(phone, true) }. - to raise_error ActiveRecord::RecordNotUnique end end end diff --git a/spec/services/otp_rate_limiter_spec.rb b/spec/services/otp_rate_limiter_spec.rb index 8c0fddd6835..dc1eae240ae 100644 --- a/spec/services/otp_rate_limiter_spec.rb +++ b/spec/services/otp_rate_limiter_spec.rb @@ -47,6 +47,7 @@ phone, false, ) + otp_rate_limiter.increment old_otp_last_sent_at = tracker.reload.otp_last_sent_at otp_rate_limiter.increment new_otp_last_sent_at = tracker.reload.otp_last_sent_at