Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent duplicate ids #4

Merged
merged 3 commits into from
Jul 15, 2021
Merged

Conversation

senhalil
Copy link

AH doesn't check if the ID is in use during object creation. This leads to a different behaviour between data= and multiple calls to the create method.

(byebug) Models::Point.data = [ {id:1}, {id:1}]
*** ActiveHash::IdError Exception: Duplicate ID found for record {:associated_stops=>[], :id=>1}

nil
(byebug) Models::Point.create({id:1})
#<Models::Point:0x000055f893355e30 @attributes={:id=>1, :associated_stops=>[]}>
(byebug) Models::Point.create({id:1})
#<Models::Point:0x000055f893369b60 @attributes={:id=>1, :associated_stops=>[]}>
(byebug) Models::Point.find(1)
#<Models::Point:0x000055f89330b560 @attributes={:id=>1, :associated_stops=>[]}>  
                ^^^^^^^^^^^^^^^^^^ This is the very first object created by `data=`

Due to stringification of IDs it is not possible to retrieve an object if their id happens to be an integer as a string.

(byebug) Models::Point.create({id:1})
#<Models::Point:0x000055c977b1a918 @attributes={:id=>1, :associated_stops=>[]}>
(byebug) Models::Point.create({id:"1"})
#<Models::Point:0x000055c977b25f98 @attributes={:id=>"1", :associated_stops=>[]}>
(byebug) Models::Point.find("1")
#<Models::Point:0x000055c977b1a918 @attributes={:id=>1, :associated_stops=>[]}>
(byebug) Models::Point.find(1)
#<Models::Point:0x000055c977b1a918 @attributes={:id=>1, :associated_stops=>[]}>

The PR removes the ID stringification logic and add a validate_unique_id check to the create method.

@senhalil senhalil requested a review from braktar July 12, 2021 12:18
@senhalil senhalil force-pushed the prevent_duplicate_ids branch from 4229b74 to b1ff92f Compare July 12, 2021 12:31
@braktar braktar merged commit d04b246 into Mapotempo:dev Jul 15, 2021
@senhalil senhalil deleted the prevent_duplicate_ids branch July 15, 2021 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants