Updated Relationship#initialize to use positional arguments

* Refactored Relationship#child_key and Relationship#parent_key
* Caught other places where variables were named "something_inst", but
  were really "something_resource".  Updated the names accordingly.
* Added note for further refactoring of Relationship#initialize
* Fixed rdoc generation
* Minor documentation updates

README

@@ -46,8 +46,7 @@ Ready for something amazing? The following example executes only two queries.
 
   zoos = Zoo.all
   first = zoos.first
-  first.exhibits
-  # Loads the exhibits for all the Zoo objects in the zoos variable.
+  first.exhibits  # Loads the exhibits for all the Zoo objects in the zoos variable.
 
 Pretty impressive huh? The idea is that you aren't going to load a set of
 objects and use only an association in just one of them. This should hold up
@@ -125,7 +124,8 @@ need other comparisons though? Try these:
 
   Zoo.first(:name => 'Galveston')
 
-  # 'gt' means greater-than. We also do 'lt'.  Person.all(:age.gt => 30)
+  # 'gt' means greater-than. We also do 'lt'.
+  Person.all(:age.gt => 30)
 
   # 'gte' means greather-than-or-equal-to. We also do 'lte'.
   Person.all(:age.gte => 30)
@@ -135,7 +135,8 @@ need other comparisons though? Try these:
   # If the value of a pair is an Array, we do an IN-clause for you.
   Person.all(:name.like => 'S%', :id => [1, 2, 3, 4, 5])
 
-  # An alias for Zoo.find(11) Zoo[11]
+  # An alias for Zoo.find(11)
+  Zoo[11]
 
   # Does a NOT IN () clause for you.
   Person.all(:name.not => ['bob','rick','steve'])

Rakefile

@@ -60,7 +60,7 @@ PACKAGE_FILES = Pathname.glob([
 ]).reject { |path| path =~ /(\/db|Makefile|\.bundle|\.log|\.o)\z/ }
 
 DOCUMENTED_FILES = PACKAGE_FILES.reject do |path|
-  path.directory? || path =~ /(^spec|\/spec|\/swig\_)/
+  path.directory? || path.to_s.match(/(?:^spec|\/spec|\/swig\_)/)
 end
 
 PROJECT = "dm-core"
@@ -74,7 +74,7 @@ rd = Rake::RDocTask.new do |rdoc|
   rdoc.rdoc_dir = 'doc'
   rdoc.title = "DataMapper -- An Object/Relational Mapper for Ruby"
   rdoc.options << '--line-numbers' << '--inline-source' << '--main' << 'README'
-  rdoc.rdoc_files.include(*DOCUMENTED_FILES)
+  rdoc.rdoc_files.include(*DOCUMENTED_FILES.map { |file| file.to_s })
 end
 
 gem_spec = Gem::Specification.new do |s|

lib/data_mapper/associations/many_to_many.rb

@@ -10,8 +10,10 @@ def many_to_many(name, options = {})
         relationships[name] = Relationship.new(
           name,
           options[:repository_name] || self.repository.name,
-          [ DataMapper::Inflection.demodulize(self.name), nil ],
-          [ target,                                       nil ]
+          DataMapper::Inflection.demodulize(self.name),
+          nil,
+          target,
+          nil
         )
       end
 

lib/data_mapper/associations/many_to_one.rb

@@ -11,8 +11,10 @@ def many_to_one(name, options = {})
         relationships[name] = Relationship.new(
           name,
           options[:repository_name] || repository.name,
-          [ DataMapper::Inflection.demodulize(self.name), nil ],
-          [ target,                                       nil ]
+          DataMapper::Inflection.demodulize(self.name),
+          nil,
+          target,
+          nil
         )
 
         class_eval <<-EOS, __FILE__, __LINE__

lib/data_mapper/associations/one_to_many.rb

@@ -13,8 +13,10 @@ def one_to_many(name, options = {})
         relationships[name] = Relationship.new(
           DataMapper::Inflection.underscore(model_name).to_sym,
           options[:repository_name] || repository.name,
-          [ source,     nil ],
-          [ model_name, nil ]
+          source,
+          nil,
+          model_name,
+          nil
         )
 
         class_eval <<-EOS, __FILE__, __LINE__

lib/data_mapper/associations/one_to_one.rb

@@ -10,8 +10,10 @@ def one_to_one(name, options = {})
         relationships[name] = Relationship.new(
           DataMapper::Inflection.underscore(model_name).to_sym,
           options[:repository_name] || repository.name,
-          [ child,      nil ],
-          [ model_name, nil ]
+          child,
+          nil,
+          model_name,
+          nil
         )
 
         class_eval <<-EOS, __FILE__, __LINE__

lib/data_mapper/associations/relationship.rb

@@ -4,80 +4,67 @@ class Relationship
 
       attr_reader :name, :repository_name
 
-      # +child+ is the FK, +parent+ is the PK. Please refer to:
+      # +child_model and child_properties refers to the FK, parent_model
+      # and parent_properties refer to the PK.  For more information:
       # http://edocs.bea.com/kodo/docs41/full/html/jdo_overview_mapping_join.html
       # I wash my hands of it!
-      #
-      # TODO: should repository_name be removed? it would allow relationships across multiple
-      # repositories (if query supports it)
-
-      # XXX: why not break up child and parent arguments so the method definition becomes:
-      #   initialize(name, repository_name, child_model, child_key, parent_model, parent_key, &loader)
-      # The *_key arguments could be Arrays of symbols or a PropertySet object
-      def initialize(name, repository_name, child, parent, &loader)
-
-        unless child.is_a?(Array) && child.size == 2
-          raise ArgumentError.new("child should be an Array of [model_name, property_name] but was #{child.inspect}")
-        end
 
-        unless parent.is_a?(Array) && parent.size == 2
-          raise ArgumentError.new("parent should be an Array of [model_name, property_name] but was #{parent.inspect}")
-        end
-
-        @name               = name
-        @repository_name    = repository_name
-        @child              = child
-        @parent             = parent
-        @loader             = loader
+      # FIXME: should we replace child_* and parent_* arguments with two
+      # Arrays of Property objects?  This would allow syntax like:
+      #
+      #   belongs_to = DataMapper::Associations::Relationship.new(
+      #     :manufacturer,
+      #     :relationship_spec,
+      #     Vehicle.properties.slice(:manufacturer_id)
+      #     Manufacturer.properties.slice(:id)
+      #   )
+      def initialize(name, repository_name, child_model, child_properties, parent_model, parent_properties, &loader)
+        @name              = name
+        @repository_name   = repository_name
+        @child_model       = child_model
+        @child_properties  = child_properties   # may be nil
+        @parent_model      = parent_model
+        @parent_properties = parent_properties  # may be nil
+        @loader            = loader
       end
 
-      def child_key      
+      def child_key
         @child_key ||= begin
-          child_key        = PropertySet.new
           model_properties = child_model.properties(@repository_name)
-          parent_keys      = parent_key.to_a
 
-          if child_property_names
-            child_property_names.zip(parent_keys).each do |property_name,parent_property|
-              child_key << (model_properties[property_name] || child_model.property(property_name, parent_property.type))
-            end
-          else
-            # Default to the parent key we're binding to prefixed with the
-            # association name.
-            parent_key.each do |property|
-              property_name = "#{@name}_#{property.name}"
-              child_key << (model_properties[property_name] || child_model.property(property_name.to_sym, property.type))
-            end
+          child_key = parent_key.zip(@child_properties || []).map do |parent_property,property_name|
+            # TODO: use something similar to DM::NamingConventions to determine the property name
+            property_name ||= "#{@name}_#{parent_property.name}".to_sym
+            model_properties[property_name] || child_model.property(property_name, parent_property.type)
           end
 
-          child_key
+          PropertySet.new(child_key)
         end
       end
 
       def parent_key
         @parent_key ||= begin
-          parent_key       = PropertySet.new
           model_properties = parent_model.properties(@repository_name)
 
-          keys = if parent_property_names
-            model_properties.slice(*parent_property_names)
+          parent_key = if @parent_properties
+            model_properties.slice(*@parent_properties)
           else
             model_properties.key
           end
 
-          parent_key.add(*keys)
+          PropertySet.new(parent_key)
         end
       end
 
-      def with_child(child_inst, association, &loader)
-        association.new(self, child_inst, lambda {
-          loader.call(repository(@repository_name), child_key, parent_key, parent_model, child_inst)
+      def with_child(child_resource, association, &loader)
+        association.new(self, child_resource, lambda {
+          loader.call(repository(@repository_name), child_key, parent_key, parent_model, child_resource)
         })
       end
 
-      def with_parent(parent_inst, association, &loader)
-        association.new(self, parent_inst, lambda {
-          loader.call(repository(@repository_name), child_key, parent_key, child_model, parent_inst)
+      def with_parent(parent_resource, association, &loader)
+        association.new(self, parent_resource, lambda {
+          loader.call(repository(@repository_name), child_key, parent_key, child_model, parent_resource)
         })
       end
 
@@ -86,21 +73,11 @@ def attach_parent(child, parent)
       end
 
       def parent_model
-        @parent.at(0).to_class
+        @parent_model.to_class
       end
 
       def child_model
-        @child.at(0).to_class
-      end
-
-      private
-
-      def child_property_names
-        @child.at(1)
-      end
-
-      def parent_property_names
-        @parent.at(1)
+        @child_model.to_class
       end
     end # class Relationship
   end # module Associations

lib/data_mapper/property_set.rb

@@ -96,7 +96,7 @@ def inspect
 
     private
 
-    def initialize(*properties, &block)
+    def initialize(properties = [], &block)
       @entries      = properties
       @property_for = Hash.new do |h,k|
         case k

spec/associations/relationship_spec.rb

@@ -11,10 +11,12 @@
     belongs_to = DataMapper::Associations::Relationship.new(
       :manufacturer,
       :relationship_spec,
-      ['Vehicle', [:manufacturer_id]],
-      ['Manufacturer', nil ]
-      )
-
+      'Vehicle',
+      [ :manufacturer_id ],
+      'Manufacturer',
+      nil
+    )
+
     belongs_to.should respond_to(:name)
     belongs_to.should respond_to(:repository_name)
     belongs_to.should respond_to(:child_key)
@@ -25,10 +27,12 @@
     belongs_to = DataMapper::Associations::Relationship.new(
       :manufacturer,
       :relationship_spec,
-      ['Vehicle', [:manufacturer_id]],
-      ['Manufacturer', [:id]]
-      )
-
+      'Vehicle',
+      [ :manufacturer_id ],
+      'Manufacturer',
+      [ :id ]
+    )
+
     belongs_to.name.should == :manufacturer
     belongs_to.repository_name.should == :relationship_spec
 
@@ -43,9 +47,11 @@
     has_many = DataMapper::Associations::Relationship.new(
       :models,
       :relationship_spec,
-      ['Vehicle', nil],
-      ['Manufacturer', nil]
-      )
+      'Vehicle',
+      nil,
+      'Manufacturer',
+      nil
+    )
 
     has_many.name.should == :models
     has_many.repository_name.should == :relationship_spec

spec/integration/query_spec.rb

@@ -210,10 +210,14 @@ class Vehicle
       end
 
       it 'should accept a DM::Assoc::Relationship as a link' do
-        factory = DataMapper::Associations::Relationship.new( :factory,
-                                                        repository(:sqlite3),
-                                                        ['Vehicle',[:factory_id]],
-                                                        ['Factory',[:id]])
+        factory = DataMapper::Associations::Relationship.new(
+          :factory,
+          repository(:sqlite3),
+          'Vehicle',
+          [ :factory_id ],
+          'Factory',
+          [ :id ]
+        )
         query = DataMapper::Query.new(Vehicle,:links => [factory])
         results = @adapter.read_set(repository(:sqlite3),query)
         results.should have(1).entries
@@ -232,10 +236,14 @@ class Vehicle
       end
 
       it 'should accept a mixture of items as a set of links' do
-        region = DataMapper::Associations::Relationship.new(:region,
-                                                        repository(:sqlite3),
-                                                        ['Factory',[:region_id]],
-                                                        ['Region',[:id]])
+        region = DataMapper::Associations::Relationship.new(
+          :region,
+          repository(:sqlite3),
+          'Factory',
+          [ :region_id ],
+          'Region',
+          [ :id ]
+        )
         query = DataMapper::Query.new(Vehicle,:links => ['factory',region])
         results = @adapter.read_set(repository(:sqlite3),query)
         results.should have(1).entries