What is the right way to use find_or_create_by to avoid duplicate records in Rails 5.2 API app

rubyonrails-talk
1

I can’t figure out the right way to use find_or_create_by method which is not atomic. In short, I have a before_action filter is used to either to find or create a User by its username.

def user

if decoded_auth_token && decoded_auth_token[:sub]

@user ||= User.find_or_create_by!(username: decoded_auth_token[:sub])

Rails.logger.silence do

@user.update_column(:token, http_auth_header)

end

@user

end

rescue ActiveRecord::RecordInvalid => e

raise(

ExceptionHandler::InvalidToken,

(“#{Message.invalid_token} #{e.message}”)

)

end

The problem is that the above method is called twice by different threads: 2 requests com from a JS front-end app, and I have the situation when 2 Users are created with the same username.

I tried to apply the suggested solution and wrap the method call in a transaction and use retry:

begin
  user = User.transaction(requires_new: true) do
    User.find_or_create_by(username: some_value)
end
rescue ActiveRecord::RecordNotUnique
  retry
end

**call_some_method to update other user attributes in User model:**

**    def update_user_info(options)
identifier = normalize_identifier(options[:sitenumber])
update(
first_name: options[:givenName],
last_name: options[:sn],
shop_identifier: identifier,
shop: user_shop(identifier)
)
end**

but it creates nevertheless the duplicate record. What am I missing ?

Used Rails version: 5.2.0

Ruby: 2.5.0

Thank you.

2

I would at least validate at the SQL level for anything that should NEVER happen. It sounds like you might have a bit of a race condition, but your DB should keep integrity in any event if set up right. (What’s DB backend are you using?)

Good luck!

Phil

3

You need to enforce the uniqueness at the database level, then catch errors.

More info on transactions here:

iiuc - transaction enforces that all changes within the transaction are applied (or none).

it doesn’t enforce that changes in transaction B are applied before a lookup in transaction A happens

looks like you could also work with a lock, though the database enforcement seems more natural.

4

You need to enforce the uniqueness at the database level, then catch errors.

https://robots.thoughtbot.com/the-perils-of-uniqueness-validations

More info on transactions here:

https://makandracards.com/makandra/31937-differences-between-transactions-and-locking

iiuc - transaction enforces that all changes within the transaction are applied (or none).

it doesn’t enforce that changes in transaction B are applied before a lookup in transaction A happens

looks like you could also work with a lock, though the database enforcement seems more natural.

Thank you very much, Rob ! I will add a unique index to Users#username column (I had one but without unique option).

What about wrapping the call into transaction ? Will it be correct to call retry ? If I got it right, retry will take another call to

User.find_or_create_by(username: some_value)

``

but this time will fetch the existing record ? Or not ?

Thank you.

5

You need to enforce the uniqueness at the database level, then catch errors.

https://robots.thoughtbot.com/the-perils-of-uniqueness-validations

More info on transactions here:

https://makandracards.com/makandra/31937-differences-between-transactions-and-locking

iiuc - transaction enforces that all changes within the transaction are applied (or none).

it doesn’t enforce that changes in transaction B are applied before a lookup in transaction A happens

looks like you could also work with a lock, though the database enforcement seems more natural.

Thank you very much, Rob ! I will add a unique index to Users#username column (I had one but without unique option).

What about wrapping the call into transaction ? Will it be correct to call retry ? If I got it right, retry will take another call to

User.find_or_create_by(username: some_value)

``

but this time will fetch the existing record ? Or not ?

Thank you.

Finally, it works as needed. Here is the working version which avoids users creation with duplicate username:

def user raise(ExceptionHandler::InvalidToken, (“#{Message.invalid_token}”)) unless decoded_auth_token[:sub].present? begin User.transaction(requires_new: true) do @user ||= User.find_or_create_by(username: decoded_auth_token[:sub]) Rails.logger.silence do @user.update_column(:token, http_auth_header) end end rescue ActiveRecord::RecordNotUnique # will retry to find/create a user to avoid thread racing and # creating users with duplicate username retry end

@user

end

``

Thanks a lot, Rob !

6

Thank you very much, Rob ! I will add a unique index to Users#username column (I had one but without unique option).

What about wrapping the call into transaction ? Will it be correct to call retry ? If I got it right, retry will take another call to

User.find_or_create_by(username: some_value)

``

but this time will fetch the existing record ? Or not ?

find_or_create by is really simple

def find_or_create_by(attributes, &block)
      find_by(attributes) || create(attributes, &block)
    end

I don’t think you need any transaction here. If you hit retry - that’s because there is a conflicting record.

if you retry find_or_create_by - then the find_by part will succeed (because there is a conflicting record.)

7

Exactly, an Ember app hits 2 Rails end-points (controllers), both protected and need a current user instance. I should find or create a user by username, - this is how the current user is initialized. Without transaction I always had 2 Users with duplicate username.