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

[belgoros]

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.

[Phil Edelbrock]

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

[Rob Jonson]

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.

[belgoros]

On Wednesday, 10 October 2018 10:52:33 UTC+2, Rob Jonson wrote:

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.

[belgoros]

On Wednesday, 10 October 2018 14:54:38 UTC+2, belgoros wrote:

On Wednesday, 10 October 2018 10:52:33 UTC+2, Rob Jonson wrote:

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 !

[Rob Jonson]

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.)

 

--

Hobbyist Software is a trading name of Hobbyist Software Limited. Registered office 12 Fraley Rd, Bristol, BS93BS. Registered in England. Company no:7876492

[s.cambour]

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.