Permalink
Browse files

Always access relationships in order of definition

This refactoring introduces a few new set classes
that are used to store relationships, properties
and ultimately also descendant sets.

  # Implements set behavior and
  # keeps track of insertion order
  DataMapper::OrderedSet

  # Uses an OrderedSet to keep track of
  # objects that must respond_to?(:name)
  DataMapper::SubjectSet "has a" OrderedSet

SubjectSet forms the base class for the already
present DataMapper::PropertySet and the newly added
DataMapper::RelationshipSet.

  PropertySet     < SubjectSet
  RelationshipSet < SubjectSet

This makes sure that common behavior between these
two classes is shared, and that RelationshipSet now
remembers the order in which relationships were added
too. Previously, relationships were stored inside a
hash, which would loose order on any not ruby-1.9
implementation.

By using an ordered set to store the relationships,
we can now provide consistent foreign key property
order when automigrating models (or fetching records
using DataMapper::Model#get).

This means that it's finally save to always omit an
explicit property declaration when defining any
relationship either with #belongs_to or #has.
Previously, it wasn't save to call Model#get on
a model with a CPK that consists of FK properties
(a join table). This was because Model#get relies
on the key property order which was undefined
before this patch because it simply used a Hash
internally.

[#1398 state:resolved]
  • Loading branch information...
1 parent 532d4a5 commit e97e9af2021660dc422a035468456ddeadd499fc @snusnu snusnu committed Jan 13, 2011
Showing with 2,125 additions and 107 deletions.
  1. +106 −1 dm-core.gemspec
  2. +24 −6 lib/dm-core.rb
  3. +1 −1 lib/dm-core/associations/relationship.rb
  4. +1 −1 lib/dm-core/collection.rb
  5. +1 −1 lib/dm-core/model.rb
  6. +2 −2 lib/dm-core/model/property.rb
  7. +12 −6 lib/dm-core/model/relationship.rb
  8. +19 −76 lib/dm-core/property_set.rb
  9. +3 −3 lib/dm-core/query.rb
  10. +1 −1 lib/dm-core/query/path.rb
  11. +58 −0 lib/dm-core/relationship_set.rb
  12. +3 −3 lib/dm-core/resource.rb
  13. +1 −1 lib/dm-core/resource/state.rb
  14. +2 −0 lib/dm-core/spec/shared/public/property_spec.rb
  15. +383 −0 lib/dm-core/support/ordered_set.rb
  16. +252 −0 lib/dm-core/support/subject_set.rb
  17. +1 −1 spec/public/model/property_spec.rb
  18. +1 −1 spec/semipublic/property/lookup_spec.rb
  19. +1 −1 spec/semipublic/property_spec.rb
  20. +26 −0 spec/unit/data_mapper/ordered_set/append_spec.rb
  21. +24 −0 spec/unit/data_mapper/ordered_set/clear_spec.rb
  22. +28 −0 spec/unit/data_mapper/ordered_set/delete_spec.rb
  23. +19 −0 spec/unit/data_mapper/ordered_set/each_spec.rb
  24. +20 −0 spec/unit/data_mapper/ordered_set/empty_spec.rb
  25. +22 −0 spec/unit/data_mapper/ordered_set/entries_spec.rb
  26. +51 −0 spec/unit/data_mapper/ordered_set/eql_spec.rb
  27. +84 −0 spec/unit/data_mapper/ordered_set/equal_value_spec.rb
  28. +12 −0 spec/unit/data_mapper/ordered_set/hash_spec.rb
  29. +23 −0 spec/unit/data_mapper/ordered_set/include_spec.rb
  30. +28 −0 spec/unit/data_mapper/ordered_set/index_spec.rb
  31. +32 −0 spec/unit/data_mapper/ordered_set/initialize_spec.rb
  32. +36 −0 spec/unit/data_mapper/ordered_set/merge_spec.rb
  33. +24 −0 spec/unit/data_mapper/ordered_set/shared/append_spec.rb
  34. +9 −0 spec/unit/data_mapper/ordered_set/shared/clear_spec.rb
  35. +25 −0 spec/unit/data_mapper/ordered_set/shared/delete_spec.rb
  36. +17 −0 spec/unit/data_mapper/ordered_set/shared/each_spec.rb
  37. +9 −0 spec/unit/data_mapper/ordered_set/shared/empty_spec.rb
  38. +9 −0 spec/unit/data_mapper/ordered_set/shared/entries_spec.rb
  39. +9 −0 spec/unit/data_mapper/ordered_set/shared/include_spec.rb
  40. +13 −0 spec/unit/data_mapper/ordered_set/shared/index_spec.rb
  41. +28 −0 spec/unit/data_mapper/ordered_set/shared/initialize_spec.rb
  42. +28 −0 spec/unit/data_mapper/ordered_set/shared/merge_spec.rb
  43. +13 −0 spec/unit/data_mapper/ordered_set/shared/size_spec.rb
  44. +11 −0 spec/unit/data_mapper/ordered_set/shared/to_ary_spec.rb
  45. +27 −0 spec/unit/data_mapper/ordered_set/size_spec.rb
  46. +23 −0 spec/unit/data_mapper/ordered_set/to_ary_spec.rb
  47. +47 −0 spec/unit/data_mapper/subject_set/append_spec.rb
  48. +34 −0 spec/unit/data_mapper/subject_set/clear_spec.rb
  49. +40 −0 spec/unit/data_mapper/subject_set/delete_spec.rb
  50. +30 −0 spec/unit/data_mapper/subject_set/each_spec.rb
  51. +31 −0 spec/unit/data_mapper/subject_set/empty_spec.rb
  52. +31 −0 spec/unit/data_mapper/subject_set/entries_spec.rb
  53. +34 −0 spec/unit/data_mapper/subject_set/get_spec.rb
  54. +32 −0 spec/unit/data_mapper/subject_set/include_spec.rb
  55. +33 −0 spec/unit/data_mapper/subject_set/named_spec.rb
  56. +18 −0 spec/unit/data_mapper/subject_set/shared/append_spec.rb
  57. +9 −0 spec/unit/data_mapper/subject_set/shared/clear_spec.rb
  58. +9 −0 spec/unit/data_mapper/subject_set/shared/delete_spec.rb
  59. +9 −0 spec/unit/data_mapper/subject_set/shared/each_spec.rb
  60. +9 −0 spec/unit/data_mapper/subject_set/shared/empty_spec.rb
  61. +9 −0 spec/unit/data_mapper/subject_set/shared/entries_spec.rb
  62. +9 −0 spec/unit/data_mapper/subject_set/shared/get_spec.rb
  63. +9 −0 spec/unit/data_mapper/subject_set/shared/include_spec.rb
  64. +9 −0 spec/unit/data_mapper/subject_set/shared/named_spec.rb
  65. +13 −0 spec/unit/data_mapper/subject_set/shared/size_spec.rb
  66. +9 −0 spec/unit/data_mapper/subject_set/shared/to_ary_spec.rb
  67. +44 −0 spec/unit/data_mapper/subject_set/shared/values_at_spec.rb
  68. +42 −0 spec/unit/data_mapper/subject_set/size_spec.rb
  69. +34 −0 spec/unit/data_mapper/subject_set/to_ary_spec.rb
  70. +57 −0 spec/unit/data_mapper/subject_set/values_at_spec.rb
  71. +1 −1 spec/unit/hash_spec.rb
  72. +1 −1 spec/unit/module_spec.rb
View
@@ -9,7 +9,7 @@ Gem::Specification.new do |s|
s.required_rubygems_version = Gem::Requirement.new(">= 0") if s.respond_to? :required_rubygems_version=
s.authors = ["Dan Kubb"]
- s.date = %q{2010-11-25}
+ s.date = %q{2011-01-12}
s.description = %q{Faster, Better, Simpler.}
s.email = %q{dan.kubb@gmail.com}
s.extra_rdoc_files = [
@@ -79,6 +79,7 @@ Gem::Specification.new do |s|
"lib/dm-core/query/operator.rb",
"lib/dm-core/query/path.rb",
"lib/dm-core/query/sort.rb",
+ "lib/dm-core/relationship_set.rb",
"lib/dm-core/repository.rb",
"lib/dm-core/resource.rb",
"lib/dm-core/resource/state.rb",
@@ -110,7 +111,9 @@ Gem::Specification.new do |s|
"lib/dm-core/support/local_object_space.rb",
"lib/dm-core/support/logger.rb",
"lib/dm-core/support/naming_conventions.rb",
+ "lib/dm-core/support/ordered_set.rb",
"lib/dm-core/support/subject.rb",
+ "lib/dm-core/support/subject_set.rb",
"lib/dm-core/type.rb",
"lib/dm-core/types/boolean.rb",
"lib/dm-core/types/decimal.rb",
@@ -203,6 +206,57 @@ Gem::Specification.new do |s|
"spec/spec_helper.rb",
"spec/support/properties/huge_integer.rb",
"spec/unit/array_spec.rb",
+ "spec/unit/data_mapper/ordered_set/append_spec.rb",
+ "spec/unit/data_mapper/ordered_set/clear_spec.rb",
+ "spec/unit/data_mapper/ordered_set/delete_spec.rb",
+ "spec/unit/data_mapper/ordered_set/each_spec.rb",
+ "spec/unit/data_mapper/ordered_set/empty_spec.rb",
+ "spec/unit/data_mapper/ordered_set/entries_spec.rb",
+ "spec/unit/data_mapper/ordered_set/eql_spec.rb",
+ "spec/unit/data_mapper/ordered_set/equal_value_spec.rb",
+ "spec/unit/data_mapper/ordered_set/hash_spec.rb",
+ "spec/unit/data_mapper/ordered_set/include_spec.rb",
+ "spec/unit/data_mapper/ordered_set/index_spec.rb",
+ "spec/unit/data_mapper/ordered_set/initialize_spec.rb",
+ "spec/unit/data_mapper/ordered_set/merge_spec.rb",
+ "spec/unit/data_mapper/ordered_set/shared/append_spec.rb",
+ "spec/unit/data_mapper/ordered_set/shared/clear_spec.rb",
+ "spec/unit/data_mapper/ordered_set/shared/delete_spec.rb",
+ "spec/unit/data_mapper/ordered_set/shared/each_spec.rb",
+ "spec/unit/data_mapper/ordered_set/shared/empty_spec.rb",
+ "spec/unit/data_mapper/ordered_set/shared/entries_spec.rb",
+ "spec/unit/data_mapper/ordered_set/shared/include_spec.rb",
+ "spec/unit/data_mapper/ordered_set/shared/index_spec.rb",
+ "spec/unit/data_mapper/ordered_set/shared/initialize_spec.rb",
+ "spec/unit/data_mapper/ordered_set/shared/merge_spec.rb",
+ "spec/unit/data_mapper/ordered_set/shared/size_spec.rb",
+ "spec/unit/data_mapper/ordered_set/shared/to_ary_spec.rb",
+ "spec/unit/data_mapper/ordered_set/size_spec.rb",
+ "spec/unit/data_mapper/ordered_set/to_ary_spec.rb",
+ "spec/unit/data_mapper/subject_set/append_spec.rb",
+ "spec/unit/data_mapper/subject_set/clear_spec.rb",
+ "spec/unit/data_mapper/subject_set/delete_spec.rb",
+ "spec/unit/data_mapper/subject_set/each_spec.rb",
+ "spec/unit/data_mapper/subject_set/empty_spec.rb",
+ "spec/unit/data_mapper/subject_set/entries_spec.rb",
+ "spec/unit/data_mapper/subject_set/get_spec.rb",
+ "spec/unit/data_mapper/subject_set/include_spec.rb",
+ "spec/unit/data_mapper/subject_set/named_spec.rb",
+ "spec/unit/data_mapper/subject_set/shared/append_spec.rb",
+ "spec/unit/data_mapper/subject_set/shared/clear_spec.rb",
+ "spec/unit/data_mapper/subject_set/shared/delete_spec.rb",
+ "spec/unit/data_mapper/subject_set/shared/each_spec.rb",
+ "spec/unit/data_mapper/subject_set/shared/empty_spec.rb",
+ "spec/unit/data_mapper/subject_set/shared/entries_spec.rb",
+ "spec/unit/data_mapper/subject_set/shared/get_spec.rb",
+ "spec/unit/data_mapper/subject_set/shared/include_spec.rb",
+ "spec/unit/data_mapper/subject_set/shared/named_spec.rb",
+ "spec/unit/data_mapper/subject_set/shared/size_spec.rb",
+ "spec/unit/data_mapper/subject_set/shared/to_ary_spec.rb",
+ "spec/unit/data_mapper/subject_set/shared/values_at_spec.rb",
+ "spec/unit/data_mapper/subject_set/size_spec.rb",
+ "spec/unit/data_mapper/subject_set/to_ary_spec.rb",
+ "spec/unit/data_mapper/subject_set/values_at_spec.rb",
"spec/unit/hash_spec.rb",
"spec/unit/hook_spec.rb",
"spec/unit/lazy_array_spec.rb",
@@ -302,6 +356,57 @@ Gem::Specification.new do |s|
"spec/spec_helper.rb",
"spec/support/properties/huge_integer.rb",
"spec/unit/array_spec.rb",
+ "spec/unit/data_mapper/ordered_set/append_spec.rb",
+ "spec/unit/data_mapper/ordered_set/clear_spec.rb",
+ "spec/unit/data_mapper/ordered_set/delete_spec.rb",
+ "spec/unit/data_mapper/ordered_set/each_spec.rb",
+ "spec/unit/data_mapper/ordered_set/empty_spec.rb",
+ "spec/unit/data_mapper/ordered_set/entries_spec.rb",
+ "spec/unit/data_mapper/ordered_set/eql_spec.rb",
+ "spec/unit/data_mapper/ordered_set/equal_value_spec.rb",
+ "spec/unit/data_mapper/ordered_set/hash_spec.rb",
+ "spec/unit/data_mapper/ordered_set/include_spec.rb",
+ "spec/unit/data_mapper/ordered_set/index_spec.rb",
+ "spec/unit/data_mapper/ordered_set/initialize_spec.rb",
+ "spec/unit/data_mapper/ordered_set/merge_spec.rb",
+ "spec/unit/data_mapper/ordered_set/shared/append_spec.rb",
+ "spec/unit/data_mapper/ordered_set/shared/clear_spec.rb",
+ "spec/unit/data_mapper/ordered_set/shared/delete_spec.rb",
+ "spec/unit/data_mapper/ordered_set/shared/each_spec.rb",
+ "spec/unit/data_mapper/ordered_set/shared/empty_spec.rb",
+ "spec/unit/data_mapper/ordered_set/shared/entries_spec.rb",
+ "spec/unit/data_mapper/ordered_set/shared/include_spec.rb",
+ "spec/unit/data_mapper/ordered_set/shared/index_spec.rb",
+ "spec/unit/data_mapper/ordered_set/shared/initialize_spec.rb",
+ "spec/unit/data_mapper/ordered_set/shared/merge_spec.rb",
+ "spec/unit/data_mapper/ordered_set/shared/size_spec.rb",
+ "spec/unit/data_mapper/ordered_set/shared/to_ary_spec.rb",
+ "spec/unit/data_mapper/ordered_set/size_spec.rb",
+ "spec/unit/data_mapper/ordered_set/to_ary_spec.rb",
+ "spec/unit/data_mapper/subject_set/append_spec.rb",
+ "spec/unit/data_mapper/subject_set/clear_spec.rb",
+ "spec/unit/data_mapper/subject_set/delete_spec.rb",
+ "spec/unit/data_mapper/subject_set/each_spec.rb",
+ "spec/unit/data_mapper/subject_set/empty_spec.rb",
+ "spec/unit/data_mapper/subject_set/entries_spec.rb",
+ "spec/unit/data_mapper/subject_set/get_spec.rb",
+ "spec/unit/data_mapper/subject_set/include_spec.rb",
+ "spec/unit/data_mapper/subject_set/named_spec.rb",
+ "spec/unit/data_mapper/subject_set/shared/append_spec.rb",
+ "spec/unit/data_mapper/subject_set/shared/clear_spec.rb",
+ "spec/unit/data_mapper/subject_set/shared/delete_spec.rb",
+ "spec/unit/data_mapper/subject_set/shared/each_spec.rb",
+ "spec/unit/data_mapper/subject_set/shared/empty_spec.rb",
+ "spec/unit/data_mapper/subject_set/shared/entries_spec.rb",
+ "spec/unit/data_mapper/subject_set/shared/get_spec.rb",
+ "spec/unit/data_mapper/subject_set/shared/include_spec.rb",
+ "spec/unit/data_mapper/subject_set/shared/named_spec.rb",
+ "spec/unit/data_mapper/subject_set/shared/size_spec.rb",
+ "spec/unit/data_mapper/subject_set/shared/to_ary_spec.rb",
+ "spec/unit/data_mapper/subject_set/shared/values_at_spec.rb",
+ "spec/unit/data_mapper/subject_set/size_spec.rb",
+ "spec/unit/data_mapper/subject_set/to_ary_spec.rb",
+ "spec/unit/data_mapper/subject_set/values_at_spec.rb",
"spec/unit/hash_spec.rb",
"spec/unit/hook_spec.rb",
"spec/unit/lazy_array_spec.rb",
View
@@ -321,11 +321,12 @@ def self.finalize
end
private
+
# @api private
def self.finalize_model(model)
name = model.name
repository_name = model.repository_name
- relationships = model.relationships(repository_name).values
+ relationships = model.relationships(repository_name)
if name.to_s.strip.empty?
raise IncompleteModelError, "#{model.inspect} must have a name"
@@ -336,15 +337,32 @@ def self.finalize_model(model)
raise IncompleteModelError, "#{name} must have at least one property or many to one relationship to be valid"
end
- # initialize join models and target keys
+ # Initialize all foreign key properties established by relationships
relationships.each do |relationship|
- relationship.child_key
- relationship.through if relationship.respond_to?(:through)
- relationship.via if relationship.respond_to?(:via)
+ case relationship
+ when Associations::ManyToOne::Relationship
+ # Initialize the foreign key property this "many to one"
+ # relationship uses to persist itself
+ relationship.child_key
+ when Associations::ManyToMany::Relationship
+ # Initialize the chain of "many to many" relationships
+ relationship.through if relationship.respond_to?(:through)
+ relationship.via if relationship.respond_to?(:via)
+ else
+ # If this is a "one to one" or "one to many" relationship, initialize
+ # the inverse many to one relationships explicitly before initializing
+ # other relationships. This makes sure that foreign key properties always
+ # appear in the order they were declared.
+ relationship.child_model.relationships.each do |remote_relationship|
+ if remote_relationship.kind_of?(Associations::ManyToOne::Relationship)
+ remote_relationship.child_key
+ end
+ end
+ end
end
if model.key(repository_name).empty?
raise IncompleteModelError, "#{name} must have a key to be valid"
end
end
-end
+end # module DataMapper
@@ -388,7 +388,7 @@ def inverse
return @inverse
end
- relationships = target_model.relationships(relative_target_repository_name).values
+ relationships = target_model.relationships(relative_target_repository_name)
@inverse = relationships.detect { |relationship| inverse?(relationship) } ||
invert
@@ -941,7 +941,7 @@ def destroy!
#
# @api public
def respond_to?(method, include_private = false)
- super || model.respond_to?(method) || relationships.key?(method)
+ super || model.respond_to?(method) || relationships.named?(method)
end
# Checks if all the resources have no changes to save
@@ -789,7 +789,7 @@ def assert_valid(force = false) # :nodoc:
# initialize join models and target keys
@relationships.values.each do |relationships|
- relationships.values.each do |relationship|
+ relationships.each do |relationship|
relationship.child_key
relationship.through if relationship.respond_to?(:through)
relationship.via if relationship.respond_to?(:via)
@@ -20,7 +20,7 @@ def inherited(model)
@properties.each do |repository_name, properties|
model_properties = model.properties(repository_name)
- properties.each { |property| model_properties[property.name] ||= property }
+ properties.each { |property| model_properties << property }
end
super
@@ -98,7 +98,7 @@ def property(name, type, options = {})
# added after the child classes' properties have been copied from
# the parent
descendants.each do |descendant|
- descendant.properties(repository_name)[name] ||= property
+ descendant.properties(repository_name) << property
end
create_reader_for(property)
@@ -1,6 +1,8 @@
# TODO: update Model#respond_to? to return true if method_method missing
# would handle the message
+require 'dm-core/relationship_set'
+
module DataMapper
module Model
module Relationship
@@ -31,7 +33,7 @@ def inherited(model)
@relationships.each do |repository_name, relationships|
model_relationships = model.relationships(repository_name)
- relationships.each { |name, relationship| model_relationships[name] ||= relationship }
+ relationships.each { |relationship| model_relationships << relationship }
end
super
@@ -53,7 +55,7 @@ def relationships(repository_name = default_repository_name)
default_repository_name = self.default_repository_name
@relationships[repository_name] ||= if repository_name == default_repository_name
- Mash.new
+ RelationshipSet.new
else
relationships(default_repository_name).dup
end
@@ -124,10 +126,12 @@ def has(cardinality, name, *args)
Associations::OneToOne::Relationship
end
- relationship = relationships(repository_name)[name] = klass.new(name, model, self, options)
+ relationship = klass.new(name, model, self, options)
+
+ relationships(repository_name) << relationship
descendants.each do |descendant|
- descendant.relationships(repository_name)[name] ||= relationship
+ descendant.relationships(repository_name) << relationship
end
create_relationship_reader(relationship)
@@ -179,10 +183,12 @@ def belongs_to(name, *args)
options[:child_repository_name] = repository_name
options[:parent_repository_name] = options.delete(:repository)
- relationship = relationships(repository_name)[name] = Associations::ManyToOne::Relationship.new(name, self, model, options)
+ relationship = Associations::ManyToOne::Relationship.new(name, self, model, options)
+
+ relationships(repository_name) << relationship
descendants.each do |descendant|
- descendant.relationships(repository_name)[name] ||= relationship
+ descendant.relationships(repository_name) << relationship
end
create_relationship_reader(relationship)
Oops, something went wrong.

1 comment on commit e97e9af

@solnic
Contributor
solnic commented on e97e9af Jan 14, 2011

You made it!

Please sign in to comment.