Fixed bug with discriminator scoping problem

* Added specs for discriminator behavior
* Cleaned up discriminator setup to not rely on hooks

[#943 state:resolved]

Manifest.txt

@@ -86,6 +86,7 @@ spec/public/shared/collection_shared_spec.rb
 spec/public/shared/resource_shared_spec.rb
 spec/public/shared/sel_shared_spec.rb
 spec/public/transaction_spec.rb
+spec/public/types/discriminator_spec.rb
 spec/semipublic/adapters/abstract_adapter_spec.rb
 spec/semipublic/adapters/in_memory_adapter_spec.rb
 spec/semipublic/adapters/mysql_adapter_spec.rb

lib/dm-core/model.rb

@@ -74,7 +74,11 @@ def self.descendants
     #
     # @api private
     def descendants
-      @descendants ||= DescendantSet.new(self, Model.descendants << self)
+      @descendants ||=
+        begin
+          ancestor = base_model.equal?(self) ? Model : superclass
+          DescendantSet.new(self, ancestor.descendants << self)
+        end
     end
 
     ##
@@ -153,7 +157,6 @@ def self.extended(model)
       # TODO: document
       # @api private
       def inherited(target)
-        target.instance_variable_set(:@descendants,   DescendantSet.new(target, descendants << target))
         target.instance_variable_set(:@valid,         false)
         target.instance_variable_set(:@storage_names, @storage_names.dup)
         target.instance_variable_set(:@base_model,    base_model)

lib/dm-core/model/descendant_set.rb

@@ -53,6 +53,17 @@ def delete(model)
         @descendants.delete(model)
       end
 
+      ##
+      # Return an Array representation of descendants
+      #
+      # @return [Array]
+      #   the descendants
+      #
+      # @api private
+      def to_ary
+        @descendants.dup
+      end
+
       private
 
       ##

lib/dm-core/query.rb

@@ -1061,10 +1061,14 @@ def append_path(path, bind_value, model, operator)
     # TODO: document
     # @api private
     def normalize_bind_value(property_or_path, bind_value)
-      if bind_value.kind_of?(Proc)
+      if bind_value.respond_to?(:call)
         bind_value = bind_value.call
       end
 
+      if bind_value.respond_to?(:to_ary)
+        bind_value = bind_value.to_ary
+      end
+
       # TODO: update InclusionComparison so it knows how to do this
       if bind_value.instance_of?(Array)
         bind_value.uniq!

lib/dm-core/query/conditions/comparison.rb

@@ -262,7 +262,7 @@ def matches?(record)
         # @api private
         def typecast_value(value)
           if subject.respond_to?(:typecast) && value.respond_to?(:map)
-            value.map { |val| subject.typecast(val) }.sort_by { |val| Sort.new(val) }
+            value.map { |val| subject.typecast(val) }
           else
             value
           end

lib/dm-core/types/discriminator.rb

@@ -11,11 +11,22 @@ def self.bind(property)
         repository_name = property.repository_name
         model           = property.model
 
+        model.default_scope(repository_name).update(property.name => model.descendants)
+
         model.class_eval <<-RUBY, __FILE__, __LINE__ + 1
-          after_class_method :inherited, :add_scope_for_discriminator
+          extend Chainable
+
+          extendable do
+            def inherited(model)
+              set_discriminator_scope_for(model)
+              super
+            end
+
+            private
 
-          def self.add_scope_for_discriminator(retval, target)
-            target.default_scope(#{repository_name.inspect}).update(#{property.name.inspect} => target.descendants.to_a)
+            def set_discriminator_scope_for(model)
+              model.default_scope(#{repository_name.inspect}).update(#{property.name.inspect} => model.descendants)
+            end
           end
         RUBY
       end

spec/public/types/discriminator_spec.rb

@@ -0,0 +1,88 @@
+require File.expand_path(File.join(File.dirname(__FILE__), '..', '..', 'spec_helper'))
+
+describe DataMapper::Types::Discriminator do
+  before :all do
+    module ::Blog
+      class Article
+        include DataMapper::Resource
+
+        undef_method :type
+
+        property :id,    Serial
+        property :title, String, :nullable => false
+        property :type,  Discriminator
+      end
+
+      class Announcement < Article; end
+      class Release < Announcement; end
+    end
+
+    @article_model      = Blog::Article
+    @announcement_model = Blog::Announcement
+    @release_model      = Blog::Release
+  end
+
+  it 'should typecast to a Model' do
+    @article_model.properties[:type].typecast('Blog::Release').should equal(@release_model)
+  end
+
+  describe 'descendant tracking' do
+    it 'should set the descendants for the grandparent model' do
+      @article_model.descendants.to_a.should == [ @article_model, @announcement_model, @release_model ]
+    end
+
+    it 'should set the descendants for the parent model' do
+      @announcement_model.descendants.to_a.should == [ @announcement_model, @release_model ]
+    end
+
+    it 'should set the descendants for the child model' do
+      @release_model.descendants.to_a.should == [ @release_model ]
+    end
+  end
+
+  describe 'scoping' do
+    it 'should set the default scope for the grandparent model' do
+      @article_model.default_scope[:type].should equal(@article_model.descendants)
+    end
+
+    it 'should set the default scope for the parent model' do
+      @announcement_model.default_scope[:type].should equal(@announcement_model.descendants)
+    end
+
+    it 'should set the default scope for the child model' do
+      @release_model.default_scope[:type].should equal(@release_model.descendants)
+    end
+  end
+
+  supported_by :all do
+    before :all do
+      @skip = defined?(DataMapper::Adapters::YamlAdapter) && @adapter.kind_of?(DataMapper::Adapters::YamlAdapter)
+    end
+
+    before do
+      pending if @skip
+    end
+
+    before :all do
+      rescue_if 'TODO: fix YAML serialization/deserialization', @skip do
+        @announcement = @announcement_model.create(:title => 'Announcement')
+      end
+    end
+
+    it 'should persist the type' do
+      @announcement.model.get(*@announcement.key).type.should equal(@announcement_model)
+    end
+
+    it 'should be retrieved as an instance of the correct class' do
+      @announcement.model.get(*@announcement.key).should be_instance_of(@announcement_model)
+    end
+
+    it 'should include descendants in finders' do
+      @article_model.first.should eql(@announcement)
+    end
+
+    it 'should not include ancestors' do
+      @release_model.first.should be_nil
+    end
+  end
+end