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

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

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.

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/rubyonrails-talk/579b1de8-8456-46d0-889f-f4569557ae44%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

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

Phil Edelbrock


On Oct 9, 2018, at 2:03 PM, belgoros <[hidden email]> wrote:

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.


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


--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/rubyonrails-talk/4091C5AB-9B4C-42A6-A073-4038A5FDCC77%40gmail.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

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

Rob Jonson
In reply to this post by belgoros
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.



On Tuesday, 9 October 2018 22:03:35 UTC+1, belgoros wrote:
I can't figure out the right way to use find_or_create_by method which is <a href="https://api.rubyonrails.org/classes/ActiveRecord/Relation.html#method-i-find_or_create_by" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fapi.rubyonrails.org%2Fclasses%2FActiveRecord%2FRelation.html%23method-i-find_or_create_by\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNG0sCmJ9kW0-Hi2suUWKWYg51Df1w&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fapi.rubyonrails.org%2Fclasses%2FActiveRecord%2FRelation.html%23method-i-find_or_create_by\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNG0sCmJ9kW0-Hi2suUWKWYg51Df1w&#39;;return true;">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.

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/rubyonrails-talk/dc933e3d-1276-4958-a938-addc6863de03%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

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

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.
<a href="https://robots.thoughtbot.com/the-perils-of-uniqueness-validations" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Frobots.thoughtbot.com%2Fthe-perils-of-uniqueness-validations\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNF1rFvi5V8HY1hBwJkX58-sXJjvhw&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Frobots.thoughtbot.com%2Fthe-perils-of-uniqueness-validations\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNF1rFvi5V8HY1hBwJkX58-sXJjvhw&#39;;return true;">https://robots.thoughtbot.com/the-perils-of-uniqueness-validations

More info on transactions here:
<a href="https://makandracards.com/makandra/31937-differences-between-transactions-and-locking" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fmakandracards.com%2Fmakandra%2F31937-differences-between-transactions-and-locking\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNFnat60dHfao7sKaJ3wvF3IYbRgKw&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fmakandracards.com%2Fmakandra%2F31937-differences-between-transactions-and-locking\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNFnat60dHfao7sKaJ3wvF3IYbRgKw&#39;;return true;">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.



On Tuesday, 9 October 2018 22:03:35 UTC+1, belgoros wrote:
I can't figure out the right way to use find_or_create_by method which is <a href="https://api.rubyonrails.org/classes/ActiveRecord/Relation.html#method-i-find_or_create_by" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fapi.rubyonrails.org%2Fclasses%2FActiveRecord%2FRelation.html%23method-i-find_or_create_by\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNG0sCmJ9kW0-Hi2suUWKWYg51Df1w&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fapi.rubyonrails.org%2Fclasses%2FActiveRecord%2FRelation.html%23method-i-find_or_create_by\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNG0sCmJ9kW0-Hi2suUWKWYg51Df1w&#39;;return true;">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.

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/rubyonrails-talk/117734f0-3d20-40aa-b5d4-d25bc6710e70%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

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

Confused Vorlon


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

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/rubyonrails-talk/CACTOy%2BrAwW%3DwbUyvAJJDjsp%2BPPM2qCZJ-KuGVBCyoj6Rq-Ufew%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

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

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

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/rubyonrails-talk/CAJGQ%3DvYjeOc-5N0Lyr6C251FTLwhn5D6HmiAX00bDxmU5iwmFw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.