active-hash / active_hash

A readonly ActiveRecord-esque base class that lets you use a hash, a Yaml file or a custom file as the datasource
MIT License
1.2k stars 179 forks source link

Prevent object creation if ID exists #232

Closed senhalil closed 3 years ago

senhalil commented 3 years ago

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 commented 3 years ago

moved to https://github.com/zilkey/active_hash/pull/233