Skip to content

Instantly share code, notes, and snippets.

@snusnu
Created December 10, 2010 00:57
Show Gist options
  • Save snusnu/735602 to your computer and use it in GitHub Desktop.
Save snusnu/735602 to your computer and use it in GitHub Desktop.
DM relationship order bug
## Problem
Currently, relationships are stored in a hash. This means that the order in which they were created gets lost.
* A model with a composite primary key that is made up of two foreign keys (e.g. a join table)
* Only the relationships are defined (using #belongs_to)
* Properties are *not* explicitly defined (using #property)
class PersonTask
include DataMapper::Resource
belongs_to :person
belongs_to :task
end
PersonTask.get(person_id, task_id)
The above call to #get might not work as expected. What we expect is that just like with properties, the two calls to #belongs_to establish two (foreign key) properties in the PersonTask model, first :person_id, then :task_id. Because the relationships are stored in a hash, there's no guarantee that we retrieve them in the same order we were putting them inside. At some point (before automigrating) DM "resolves" the two relationship definitions and finds out that it needs to establish properties for them. The problem now is that it didn't remember the order in which the relationships were defined, and thus doesn't know the order in which to define the respective properties. Since #get relies on the order in which the key properties were defined to map the arguments to properties, there's obviously a problem.
## Proposal
Currently, relationships are stored in a hash
https://github.com/datamapper/dm-core/blob/master/lib/dm-core/model/relationship.rb#L56
https://github.com/datamapper/dm-core/blob/master/lib/dm-core/model/relationship.rb#L21
There's already 2 set implementations in the codebase, properties are stored in a PropertySet
https://github.com/datamapper/dm-core/blob/master/lib/dm-core/property_set.rb
https://github.com/datamapper/dm-core/blob/master/lib/dm-core/model/property.rb#L131
and Model keeps track of its descendants in a DescendantSet:
https://github.com/datamapper/dm-core/blob/master/lib/dm-core/support/descendant_set.rb
https://github.com/datamapper/dm-core/blob/master/lib/dm-core/model.rb#L60
https://github.com/datamapper/dm-core/blob/master/lib/dm-core/model.rb#L79
The plan is to provide an OrderedSet class that provides both set semantics and retains insertion order. With that class available, we could probably have a SubjectSet class that "has a" OrderedSet and provides common API for all its descendants, being: PropertySet, RelationshipSet and DescendantSet. This refactoring would eliminate the bug explained above and would also clean up duplicated code (currently in PropertySet and DescendantSet, and in the new RelationshipSet)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment