Fixed relationship inheritance so inferred FK is based on base relati…

…onship

* Spec relationship inheritance behavior
* Added #inherited_by to 1:m, m:1 and m:m relationships for handling
  reinitializing a relationship in a subclass.
* Added ManyToMany::Relationship#invert
* Refactored Relationship comparison logic

[#921 state:resolved]

lib/dm-core/associations/many_to_many.rb

@@ -112,6 +112,16 @@ def query
           @many_to_many_query ||= super.merge(:links => links).freeze
         end
 
+        # TODO: document
+        # @api private
+        def inherited_by(model)
+          relationship = super
+          if explicit_through_relationship? || target_model?
+            relationship.instance_variable_set(:@through, through.inherited_by(model))
+          end
+          relationship
+        end
+
         private
 
         # TODO: document
@@ -187,10 +197,27 @@ def one_to_many_options
           { :parent_key => source_key.map { |property| property.name } }
         end
 
+        # Returns the inverse relationship class
+        #
+        # @api private
+        def inverse_class
+          self.class
+        end
+
+        # TODO: document
+        # @api private
+        def invert
+          inverse_class.new(inverse_name, parent_model, child_model, inverted_options)
+        end
+
         # TODO: document
         # @api private
-        def inverse
-          raise NotImplementedError, "#{self.class}#inverse not implemented"
+        def inverted_options
+          options.only(*OPTIONS - [ :min, :max ]).update(
+            :child_key  => parent_key.map { |property| property.name },
+            :parent_key => child_key.map  { |property| property.name },
+            :inverse    => self
+          )
         end
 
         # Returns collection class used by this type of

lib/dm-core/associations/many_to_one.rb

@@ -111,6 +111,12 @@ def set(source, target)
           set!(source, target)
         end
 
+        # TODO: document
+        # @api private
+        def inherited_by(model)
+          self.class.new(name, model, parent_model_name, options_with_inverse)
+        end
+
         private
 
         # Initializes the relationship, always using max cardinality of 1.

lib/dm-core/associations/one_to_many.rb

@@ -90,6 +90,12 @@ def set(source, targets)
           get!(source).replace(targets)
         end
 
+        # TODO: document
+        # @api private
+        def inherited_by(model)
+          self.class.new(name, child_model_name, model, options_with_inverse)
+        end
+
         private
 
         # TODO: document
@@ -178,8 +184,11 @@ def inverse_name
         #
         # @api semipublic
         def property_prefix
-          # TODO: try to use the inverse relationship name if possible
-          Extlib::Inflection.underscore(Extlib::Inflection.demodulize(parent_model.base_model.name)).to_sym
+          if @inverse
+            inverse.name
+          else
+            inverse_name
+          end
         end
       end # class Relationship
 

lib/dm-core/associations/relationship.rb

@@ -385,13 +385,13 @@ def ==(other)
       # @api semipublic
       def inverse
         @inverse ||= target_model.relationships(relative_target_repository_name).values.detect do |relationship|
-          relationship.kind_of?(inverse_class)   &&
-          cmp_repository?(relationship, :child)  &&
-          cmp_repository?(relationship, :parent) &&
-          cmp_model?(relationship, :child)       &&
-          cmp_model?(relationship, :parent)      &&
-          cmp_key?(relationship, :child)         &&
-          cmp_key?(relationship, :parent)
+          relationship.kind_of?(inverse_class)                 &&
+          cmp_repository?(relationship, :==, :source, :target) &&
+          cmp_repository?(relationship, :==, :target, :source) &&
+          cmp_model?(relationship,      :==, :source, :target) &&
+          cmp_model?(relationship,      :==, :target, :source) &&
+          cmp_key?(relationship,        :==, :source, :target) &&
+          cmp_key?(relationship,        :==, :target, :source)
 
           # TODO: match only when the Query is empty, or is the same as the
           # default scope for the target model
@@ -544,30 +544,40 @@ def inverted_options
         )
       end
 
+      # TODO: document
+      # @api private
+      def options_with_inverse
+        if child_model? && parent_model?
+          options.merge(:inverse => inverse)
+        else
+          options
+        end
+      end
+
       # TODO: document
       # @api private
       def cmp?(other, operator)
-        unless cmp_repository?(other, :child, operator)
+        unless cmp_repository?(other, operator, :source)
           return false
         end
 
-        unless cmp_repository?(other, :parent, operator)
+        unless cmp_repository?(other, operator, :target)
           return false
         end
 
-        unless cmp_model?(other, :child, operator)
+        unless cmp_model?(other, operator, :source)
           return false
         end
 
-        unless cmp_model?(other, :parent, operator)
+        unless cmp_model?(other, operator, :target)
           return false
         end
 
-        unless cmp_key?(other, :child, operator)
+        unless cmp_key?(other, operator, :source)
           return false
         end
 
-        unless cmp_key?(other, :parent, operator)
+        unless cmp_key?(other, operator, :target)
           return false
         end
 
@@ -580,16 +590,14 @@ def cmp?(other, operator)
 
       # TODO: document
       # @api private
-      def cmp_repository?(other, type, operator = :==)
-        method = "#{type}_repository_name".to_sym
-
+      def cmp_repository?(other, operator, self_type, other_type = self_type)
         # if either repository is nil, then the relationship is relative,
         # and the repositories are considered equivalent
-        unless repository_name = send(method)
+        unless repository_name = send("#{self_type}_repository_name")
           return true
         end
 
-        unless other_repository_name = other.send(method)
+        unless other_repository_name = other.send("#{other_type}_repository_name")
           return true
         end
 
@@ -602,19 +610,16 @@ def cmp_repository?(other, type, operator = :==)
 
       # TODO: document
       # @api private
-      def cmp_model?(other, type, operator = :==)
-        method  = "#{type}_model".to_sym
-        defined = "#{method}?".to_sym
-
-        unless send(defined)
+      def cmp_model?(other, operator, self_type, other_type = self_type)
+        unless send("#{self_type}_model?")
           return false
         end
 
-        unless other.send(defined)
+        unless other.send("#{other_type}_model?")
           return false
         end
 
-        unless send(method).send(operator, other.send(method))
+        unless send("#{self_type}_model").send(operator, other.send("#{other_type}_model"))
           return false
         end
 
@@ -623,18 +628,16 @@ def cmp_model?(other, type, operator = :==)
 
       # TODO: document
       # @api private
-      def cmp_key?(other, type, operator = :==)
-        method = "#{type}_key".to_sym
-
-        unless send("#{method}?")
+      def cmp_key?(other, operator, self_type, other_type = self_type)
+        unless send("#{self_type}_key?")
           return true
         end
 
-        unless other.send("#{method}?")
+        unless other.send("#{other_type}_key?")
           return true
         end
 
-        unless send(method).send(operator, other.send(method))
+        unless send("#{self_type}_key").send(operator, other.send("#{other_type}_key"))
           return false
         end
 

lib/dm-core/model.rb

@@ -648,13 +648,8 @@ def assert_valid # :nodoc:
       # initialize join models and target keys
       @relationships.each_value do |relationships|
         relationships.each_value do |relationship|
-          # TODO: remove m:m test below when inverse works for it
-          if relationship.respond_to?(:resource_for)
-            relationship.child_key
-          elsif !relationship.kind_of?(Associations::ManyToMany::Relationship)
-            relationship.inverse.child_key
-          end
-
+          relationship.child_key
+          relationship.inverse.child_key
           relationship.through if relationship.respond_to?(:through)
         end
       end

lib/dm-core/model/relationship.rb

@@ -34,12 +34,7 @@ def inherited(model)
             dup = duped_relationships[repository_name] ||= Mash.new
 
             relationships.each do |name, relationship|
-              dup[name] = relationship.class.new(
-                relationship.name,
-                relationship.child_model_name  == self.name ? model : relationship.child_model_name,
-                relationship.parent_model_name == self.name ? model : relationship.parent_model_name,
-                relationship.options.dup
-              )
+              dup[name] = relationship.inherited_by(model)
             end
           end
 
@@ -138,7 +133,13 @@ def has(cardinality, name, *args)
           Associations::OneToOne::Relationship
         end
 
-        relationships(repository.name)[name] = klass.new(name, model, self, options)
+        relationship = relationships(repository.name)[name] = klass.new(name, model, self, options)
+
+        descendants.each do |descendant|
+          descendant.relationships(repository.name)[name] ||= relationship.inherited_by(descendant)
+        end
+
+        relationship
       end
 
       ##
@@ -183,11 +184,19 @@ def belongs_to(name, *args)
 
         model ||= options.delete(:model)
 
+        repository_name = repository.name
+
         # TODO: change to source_repository_name and target_respository_name
-        options[:child_repository_name]  = repository.name
+        options[:child_repository_name]  = repository_name
         options[:parent_repository_name] = options.delete(:repository)
 
-        relationships(repository.name)[name] = Associations::ManyToOne::Relationship.new(name, self, model, options)
+        relationship = relationships(repository.name)[name] = Associations::ManyToOne::Relationship.new(name, self, model, options)
+
+        descendants.each do |descendant|
+          descendant.relationships(repository.name)[name] ||= relationship.inherited_by(descendant)
+        end
+
+        relationship
       end
 
       private