@@ -68,7 +68,7 @@ module MongoMapper end def nullify - criteria = FinderOptions.to_mongo_criteria(scoped_conditions) + criteria = FinderOptions.to_mongo_criteria(klass, scoped_conditions) all(criteria).each do |doc| doc.update_attributes self.foreign_key => nil end lib/mongo_mapper/associations/many_documents_proxy.rb @@ -95,14 +95,14 @@ module MongoMapper end def find_by_id(id) - criteria = FinderOptions.to_mongo_criteria(:_id => id) + criteria = FinderOptions.to_mongo_criteria(self, :_id => id) if doc = collection.find_one(criteria) new(doc) end end def count(conditions={}) - collection.find(FinderOptions.to_mongo_criteria(conditions)).count + collection.find(FinderOptions.to_mongo_criteria(self, conditions)).count end def exists?(conditions={}) @@ -133,12 +133,12 @@ module MongoMapper end def delete(*ids) - criteria = FinderOptions.to_mongo_criteria(:_id => ids.flatten) + criteria = FinderOptions.to_mongo_criteria(self, :_id => ids.flatten) collection.remove(criteria) end def delete_all(conditions={}) - criteria = FinderOptions.to_mongo_criteria(conditions) + criteria = FinderOptions.to_mongo_criteria(self, conditions) collection.remove(criteria) end @@ -191,6 +191,14 @@ module MongoMapper class_eval { before_save :update_timestamps } end + def single_collection_inherited? + keys.has_key?('_type') && single_collection_inherited_superclass? + end + + def single_collection_inherited_superclass? + superclass.respond_to?(:keys) && superclass.keys.has_key?('_type') + end + protected def method_missing(method, *args) finder = DynamicFinder.new(method) @@ -221,7 +229,7 @@ module MongoMapper end def find_every(options) - criteria, options = FinderOptions.new(options).to_a + criteria, options = FinderOptions.new(self, options).to_a collection.find(criteria, options).to_a.map do |doc| begin klass = doc['_type'].present? ? doc['_type'].constantize : self @@ -304,7 +312,7 @@ module MongoMapper def destroy return false if frozen? - criteria = FinderOptions.to_mongo_criteria(:_id => id) + criteria = FinderOptions.to_mongo_criteria(self.class, :_id => id) collection.remove(criteria) unless new? freeze end lib/mongo_mapper/document.rb @@ -1,9 +1,9 @@ module MongoMapper - class FinderOptions - attr_reader :options - - def self.to_mongo_criteria(conditions, parent_key=nil) + class FinderOptions + def self.to_mongo_criteria(model, conditions, parent_key=nil) criteria = {} + add_sci_scope(model, criteria) + conditions.each_pair do |field, value| field = field_normalized(field) case value @@ -15,7 +15,7 @@ module MongoMapper {'$in' => value} end when Hash - criteria[field] = to_mongo_criteria(value, field) + criteria[field] = to_mongo_criteria(model, value, field) else criteria[field] = value end @@ -23,8 +23,15 @@ module MongoMapper criteria end + + # adds _type single collection inheritance scope for models that need it + def self.add_sci_scope(model, criteria) + if model.single_collection_inherited? + criteria[:_type] = model.to_s + end + end - def self.to_mongo_options(options) + def self.to_mongo_options(model, options) options = options.dup { :fields => to_mongo_fields(options.delete(:fields) || options.delete(:select)), @@ -44,9 +51,13 @@ module MongoMapper OptionKeys = [:fields, :select, :skip, :offset, :limit, :sort, :order] - def initialize(options) + attr_reader :model, :options + + def initialize(model, options) raise ArgumentError, "FinderOptions must be a hash" unless options.is_a?(Hash) + @model = model + options = options.symbolize_keys @options, @conditions = {}, options.delete(:conditions) || {} @@ -60,11 +71,11 @@ module MongoMapper end def criteria - self.class.to_mongo_criteria(@conditions) + self.class.to_mongo_criteria(model, @conditions) end def options - self.class.to_mongo_options(@options) + self.class.to_mongo_options(model, @options) end def to_a lib/mongo_mapper/finder_options.rb @@ -12,7 +12,7 @@ def run_all_tests end def related_test_files(path) - Dir['test/**/*.rb'].select { |file| file =~ /#{File.basename(path)}/ } + Dir['test/**/*.rb'].select { |file| file =~ /test_#{File.basename(path)}/ } end watch('test/test_helper\.rb') { run_all_tests } specs.watchr @@ -948,38 +948,43 @@ class DocumentTest < Test::Unit::TestCase class ::DocParent include MongoMapper::Document key :_type, String + key :name, String end - - class ::DocChild < ::DocParent; end DocParent.collection.clear - + + class ::DocDaughter < ::DocParent; end + class ::DocSon < ::DocParent; end + class ::DocGrandSon < ::DocSon; end + @parent = DocParent.new({:name => "Daddy Warbucks"}) - @child = DocChild.new({:name => "Little Orphan Annie"}) + @daughter = DocDaughter.new({:name => "Little Orphan Annie"}) end teardown do - Object.send :remove_const, 'DocParent' if defined?(::DocParent) - Object.send :remove_const, 'DocChild' if defined?(::DocChild) + Object.send :remove_const, 'DocParent' if defined?(::DocParent) + Object.send :remove_const, 'DocDaughter' if defined?(::DocDaughter) + Object.send :remove_const, 'DocSon' if defined?(::DocSon) + Object.send :remove_const, 'DocGrandSon' if defined?(::DocGrandSon) end should "use the same collection in the subclass" do - DocChild.collection.name.should == DocParent.collection.name + DocDaughter.collection.name.should == DocParent.collection.name end should "assign the class name into the _type property" do @parent._type.should == 'DocParent' - @child._type.should == 'DocChild' + @daughter._type.should == 'DocDaughter' end should "load the document with the assigned type" do @parent.save - @child.save - + @daughter.save + collection = DocParent.find(:all) collection.size.should == 2 collection.first.should be_kind_of(DocParent) collection.first.name.should == "Daddy Warbucks" - collection.last.should be_kind_of(DocChild) + collection.last.should be_kind_of(DocDaughter) collection.last.name.should == "Little Orphan Annie" end @@ -992,6 +997,45 @@ class DocumentTest < Test::Unit::TestCase collection.last.should == doc collection.last.should be_kind_of(DocParent) end + + should "find scoped to class" do + john = DocSon.create(:name => 'John') + steve = DocSon.create(:name => 'Steve') + steph = DocDaughter.create(:name => 'Steph') + carrie = DocDaughter.create(:name => 'Carrie') + + DocGrandSon.all(:order => 'name').should == [] + DocSon.all(:order => 'name').should == [john, steve] + DocDaughter.all(:order => 'name').should == [carrie, steph] + DocParent.all(:order => 'name').should == [carrie, john, steph, steve] + end + + should "count scoped to class" do + john = DocSon.create(:name => 'John') + steve = DocSon.create(:name => 'Steve') + steph = DocDaughter.create(:name => 'Steph') + carrie = DocDaughter.create(:name => 'Carrie') + + DocGrandSon.count.should == 0 + DocSon.count.should == 2 + DocDaughter.count.should == 2 + DocParent.count.should == 4 + end + + should "know if it is single_collection_inherited?" do + DocParent.single_collection_inherited?.should be_false + + DocDaughter.single_collection_inherited?.should be_true + DocSon.single_collection_inherited?.should be_true + end + + should "know if single_collection_inherited_superclass?" do + DocParent.single_collection_inherited_superclass?.should be_false + + DocDaughter.single_collection_inherited_superclass?.should be_true + DocSon.single_collection_inherited_superclass?.should be_true + DocGrandSon.single_collection_inherited_superclass?.should be_true + end end context "timestamping" do test/functional/test_document.rb @@ -68,12 +68,6 @@ class Message belongs_to :room end -class Answer - include MongoMapper::Document - - key :body, String -end - class Enter < Message; end class Exit < Message; end class Chat < Message; end @@ -85,6 +79,12 @@ class Room many :messages, :polymorphic => true end +class Answer + include MongoMapper::Document + + key :body, String +end + class Project include MongoMapper::Document test/models.rb @@ -1,73 +1,87 @@ require 'test_helper' +require 'models' class FinderOptionsTest < Test::Unit::TestCase include MongoMapper should "raise error if provided something other than a hash" do - lambda { FinderOptions.new }.should raise_error(ArgumentError) - lambda { FinderOptions.new(1) }.should raise_error(ArgumentError) + lambda { FinderOptions.new(Room) }.should raise_error(ArgumentError) + lambda { FinderOptions.new(Room, 1) }.should raise_error(ArgumentError) end should "symbolize the keys of the hash provided" do - FinderOptions.new('offset' => 1).options.keys.map do |key| + FinderOptions.new(Room, 'offset' => 1).options.keys.map do |key| key.should be_instance_of(Symbol) end end context "#criteria" do should "convert conditions to criteria" do - FinderOptions.expects(:to_mongo_criteria).with(:foo => 1).returns({}) - FinderOptions.new(:conditions => {:foo => 1}).criteria + FinderOptions.expects(:to_mongo_criteria).with(Room, :foo => 1).returns({}) + FinderOptions.new(Room, :conditions => {:foo => 1}).criteria end end context "#options" do should "convert options to mongo options" do - FinderOptions.expects(:to_mongo_options).with(:order => 'foo asc', :select => 'foo,bar').returns({}) - FinderOptions.new(:order => 'foo asc', :select => 'foo,bar').options + FinderOptions.expects(:to_mongo_options).with(Room, :order => 'foo asc', :select => 'foo,bar').returns({}) + FinderOptions.new(Room, :order => 'foo asc', :select => 'foo,bar').options end end context "Converting conditions to criteria" do + should "not add _type to query if model does not have superclass that is single collection inherited" do + FinderOptions.to_mongo_criteria(Message, :foo => 'bar').should == { + :foo => 'bar' + } + end + + should "automatically add _type to query if model is single collection inherited" do + FinderOptions.to_mongo_criteria(Enter, :foo => 'bar').should == { + :foo => 'bar', + :_type => 'Enter' + } + end + should "work with simple criteria" do - FinderOptions.to_mongo_criteria(:foo => 'bar').should == { + FinderOptions.to_mongo_criteria(Room, :foo => 'bar').should == { :foo => 'bar' } - FinderOptions.to_mongo_criteria(:foo => 'bar', :baz => 'wick').should == { + FinderOptions.to_mongo_criteria(Room, :foo => 'bar', :baz => 'wick').should == { :foo => 'bar', :baz => 'wick' } end should "convert id to _id" do - FinderOptions.to_mongo_criteria(:id => '1').should == { + FinderOptions.to_mongo_criteria(Room, :id => '1').should == { :_id => '1' } end should "use $in for arrays" do - FinderOptions.to_mongo_criteria(:foo => [1,2,3]).should == { + FinderOptions.to_mongo_criteria(Room, :foo => [1,2,3]).should == { :foo => {'$in' => [1,2,3]} } end should "not use $in for arrays if already using array operator" do - FinderOptions.to_mongo_criteria(:foo => {'$all' => [1,2,3]}).should == { + FinderOptions.to_mongo_criteria(Room, :foo => {'$all' => [1,2,3]}).should == { :foo => {'$all' => [1,2,3]} } - FinderOptions.to_mongo_criteria(:foo => {'$any' => [1,2,3]}).should == { + FinderOptions.to_mongo_criteria(Room, :foo => {'$any' => [1,2,3]}).should == { :foo => {'$any' => [1,2,3]} } end should "work arbitrarily deep" do - FinderOptions.to_mongo_criteria(:foo => {:bar => [1,2,3]}).should == { + FinderOptions.to_mongo_criteria(Room, :foo => {:bar => [1,2,3]}).should == { :foo => {:bar => {'$in' => [1,2,3]}} } - FinderOptions.to_mongo_criteria(:foo => {:bar => {'$any' => [1,2,3]}}).should == { + FinderOptions.to_mongo_criteria(Room, :foo => {:bar => {'$any' => [1,2,3]}}).should == { :foo => {:bar => {'$any' => [1,2,3]}} } end @@ -76,166 +90,166 @@ class FinderOptionsTest < Test::Unit::TestCase context "ordering" do should "single field with ascending direction" do sort = [['foo', 1]] - FinderOptions.to_mongo_options(:order => 'foo asc')[:sort].should == sort - FinderOptions.to_mongo_options(:order => 'foo ASC')[:sort].should == sort + FinderOptions.to_mongo_options(Room, :order => 'foo asc')[:sort].should == sort + FinderOptions.to_mongo_options(Room, :order => 'foo ASC')[:sort].should == sort end should "single field with descending direction" do sort = [['foo', -1]] - FinderOptions.to_mongo_options(:order => 'foo desc')[:sort].should == sort - FinderOptions.to_mongo_options(:order => 'foo DESC')[:sort].should == sort + FinderOptions.to_mongo_options(Room, :order => 'foo desc')[:sort].should == sort + FinderOptions.to_mongo_options(Room, :order => 'foo DESC')[:sort].should == sort end should "convert field without direction to ascending" do sort = [['foo', 1]] - FinderOptions.to_mongo_options(:order => 'foo')[:sort].should == sort + FinderOptions.to_mongo_options(Room, :order => 'foo')[:sort].should == sort end should "convert multiple fields with directions" do sort = [['foo', -1], ['bar', 1], ['baz', -1]] - FinderOptions.to_mongo_options(:order => 'foo desc, bar asc, baz desc')[:sort].should == sort + FinderOptions.to_mongo_options(Room, :order => 'foo desc, bar asc, baz desc')[:sort].should == sort end should "convert multiple fields with some missing directions" do sort = [['foo', -1], ['bar', 1], ['baz', 1]] - FinderOptions.to_mongo_options(:order => 'foo desc, bar, baz')[:sort].should == sort + FinderOptions.to_mongo_options(Room, :order => 'foo desc, bar, baz')[:sort].should == sort end should "just use sort if sort and order are present" do sort = [['$natural', 1]] - FinderOptions.to_mongo_options(:sort => sort, :order => 'foo asc')[:sort].should == sort + FinderOptions.to_mongo_options(Room, :sort => sort, :order => 'foo asc')[:sort].should == sort end should "convert natural in order to proper" do sort = [['$natural', 1]] - FinderOptions.to_mongo_options(:order => '$natural asc')[:sort].should == sort + FinderOptions.to_mongo_options(Room, :order => '$natural asc')[:sort].should == sort sort = [['$natural', -1]] - FinderOptions.to_mongo_options(:order => '$natural desc')[:sort].should == sort + FinderOptions.to_mongo_options(Room, :order => '$natural desc')[:sort].should == sort end should "work for natural order ascending" do - FinderOptions.to_mongo_options(:sort => {'$natural' => 1})[:sort]['$natural'].should == 1 + FinderOptions.to_mongo_options(Room, :sort => {'$natural' => 1})[:sort]['$natural'].should == 1 end should "work for natural order descending" do - FinderOptions.to_mongo_options(:sort => {'$natural' => -1})[:sort]['$natural'].should == -1 + FinderOptions.to_mongo_options(Room, :sort => {'$natural' => -1})[:sort]['$natural'].should == -1 end end context "skip" do should "default to 0" do - FinderOptions.to_mongo_options({})[:skip].should == 0 + FinderOptions.to_mongo_options(Room, {})[:skip].should == 0 end should "use skip provided" do - FinderOptions.to_mongo_options(:skip => 2)[:skip].should == 2 + FinderOptions.to_mongo_options(Room, :skip => 2)[:skip].should == 2 end should "covert string to integer" do - FinderOptions.to_mongo_options(:skip => '2')[:skip].should == 2 + FinderOptions.to_mongo_options(Room, :skip => '2')[:skip].should == 2 end should "convert offset to skip" do - FinderOptions.to_mongo_options(:offset => 1)[:skip].should == 1 + FinderOptions.to_mongo_options(Room, :offset => 1)[:skip].should == 1 end end context "limit" do should "default to 0" do - FinderOptions.to_mongo_options({})[:limit].should == 0 + FinderOptions.to_mongo_options(Room, {})[:limit].should == 0 end should "use limit provided" do - FinderOptions.to_mongo_options(:limit => 2)[:limit].should == 2 + FinderOptions.to_mongo_options(Room, :limit => 2)[:limit].should == 2 end should "covert string to integer" do - FinderOptions.to_mongo_options(:limit => '2')[:limit].should == 2 + FinderOptions.to_mongo_options(Room, :limit => '2')[:limit].should == 2 end end context "fields" do should "default to nil" do - FinderOptions.to_mongo_options({})[:fields].should be(nil) + FinderOptions.to_mongo_options(Room, {})[:fields].should be(nil) end should "be converted to nil if empty string" do - FinderOptions.to_mongo_options(:fields => '')[:fields].should be(nil) + FinderOptions.to_mongo_options(Room, :fields => '')[:fields].should be(nil) end should "be converted to nil if []" do - FinderOptions.to_mongo_options(:fields => [])[:fields].should be(nil) + FinderOptions.to_mongo_options(Room, :fields => [])[:fields].should be(nil) end should "should work with array" do - FinderOptions.to_mongo_options({:fields => %w(a b)})[:fields].should == %w(a b) + FinderOptions.to_mongo_options(Room, {:fields => %w(a b)})[:fields].should == %w(a b) end should "convert comma separated list to array" do - FinderOptions.to_mongo_options({:fields => 'a, b'})[:fields].should == %w(a b) + FinderOptions.to_mongo_options(Room, {:fields => 'a, b'})[:fields].should == %w(a b) end should "also work as select" do - FinderOptions.new(:select => %w(a b)).options[:fields].should == %w(a b) + FinderOptions.new(Room, :select => %w(a b)).options[:fields].should == %w(a b) end end context "Condition auto-detection" do should "know :conditions are criteria" do - finder = FinderOptions.new(:conditions => {:foo => 'bar'}) + finder = FinderOptions.new(Room, :conditions => {:foo => 'bar'}) finder.criteria.should == {:foo => 'bar'} finder.options.keys.should_not include(:conditions) end should "know fields is an option" do - finder = FinderOptions.new(:fields => ['foo']) + finder = FinderOptions.new(Room, :fields => ['foo']) finder.options[:fields].should == ['foo'] finder.criteria.keys.should_not include(:fields) end # select gets converted to fields so just checking keys should "know select is an option" do - finder = FinderOptions.new(:select => 'foo') + finder = FinderOptions.new(Room, :select => 'foo') finder.options.keys.should include(:sort) finder.criteria.keys.should_not include(:select) finder.criteria.keys.should_not include(:fields) end should "know skip is an option" do - finder = FinderOptions.new(:skip => 10) + finder = FinderOptions.new(Room, :skip => 10) finder.options[:skip].should == 10 finder.criteria.keys.should_not include(:skip) end # offset gets converted to skip so just checking keys should "know offset is an option" do - finder = FinderOptions.new(:offset => 10) + finder = FinderOptions.new(Room, :offset => 10) finder.options.keys.should include(:skip) finder.criteria.keys.should_not include(:skip) finder.criteria.keys.should_not include(:offset) end should "know limit is an option" do - finder = FinderOptions.new(:limit => 10) + finder = FinderOptions.new(Room, :limit => 10) finder.options[:limit].should == 10 finder.criteria.keys.should_not include(:limit) end should "know sort is an option" do - finder = FinderOptions.new(:sort => [['foo', 1]]) + finder = FinderOptions.new(Room, :sort => [['foo', 1]]) finder.options[:sort].should == [['foo', 1]] finder.criteria.keys.should_not include(:sort) end # order gets converted to sort so just checking keys should "know order is an option" do - finder = FinderOptions.new(:order => 'foo') + finder = FinderOptions.new(Room, :order => 'foo') finder.options.keys.should include(:sort) finder.criteria.keys.should_not include(:sort) end should "work with full range of things" do - finder_options = FinderOptions.new({ + finder_options = FinderOptions.new(Room, { :foo => 'bar', :baz => true, :sort => [['foo', 1]], test/unit/test_finder_options.rb 4cca5bca1edef3a30bfbd0916ed3b22eb3e57a43 John Nunemaker nunemaker@gmail.com http://github.com/jnunemaker/mongomapper/commit/32c454d1cc7853bb6de5e58dd108cd04f0536ba3 32c454d1cc7853bb6de5e58dd108cd04f0536ba3 2009-10-22T14:03:30-07:00 2009-10-22T14:03:30-07:00 Models with _type key are no properly scoped when performing find and count. Fixes #47. bc9527956ef1e436cede2287e73553b4d8fa3db7 John Nunemaker nunemaker@gmail.com