From c2c1c9bcd48a42411a168f6a6b7d4ea9383e2765 Mon Sep 17 00:00:00 2001 From: Dan Kubb Date: Thu, 2 Jul 2009 11:11:27 -0700 Subject: [PATCH] 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 | 1 + lib/dm-core/model.rb | 7 +- lib/dm-core/model/descendant_set.rb | 11 +++ lib/dm-core/query.rb | 6 +- lib/dm-core/query/conditions/comparison.rb | 2 +- lib/dm-core/types/discriminator.rb | 17 ++++- spec/public/types/discriminator_spec.rb | 88 ++++++++++++++++++++++ 7 files changed, 125 insertions(+), 7 deletions(-) create mode 100644 spec/public/types/discriminator_spec.rb diff --git a/Manifest.txt b/Manifest.txt index 3ea51d92..fa099ec1 100644 --- a/Manifest.txt +++ b/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 diff --git a/lib/dm-core/model.rb b/lib/dm-core/model.rb index 98c9f254..2080c9cb 100644 --- a/lib/dm-core/model.rb +++ b/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) diff --git a/lib/dm-core/model/descendant_set.rb b/lib/dm-core/model/descendant_set.rb index 5ca4f429..4ea4d31c 100644 --- a/lib/dm-core/model/descendant_set.rb +++ b/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 ## diff --git a/lib/dm-core/query.rb b/lib/dm-core/query.rb index 3d7048b2..d1ee6b6c 100644 --- a/lib/dm-core/query.rb +++ b/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! diff --git a/lib/dm-core/query/conditions/comparison.rb b/lib/dm-core/query/conditions/comparison.rb index 490129ca..8c001752 100644 --- a/lib/dm-core/query/conditions/comparison.rb +++ b/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 diff --git a/lib/dm-core/types/discriminator.rb b/lib/dm-core/types/discriminator.rb index cd265eee..adf0044c 100644 --- a/lib/dm-core/types/discriminator.rb +++ b/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 diff --git a/spec/public/types/discriminator_spec.rb b/spec/public/types/discriminator_spec.rb new file mode 100644 index 00000000..dd522b30 --- /dev/null +++ b/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