From de6fef357b83f481ec3ed7375306f9a0f89b2926 Mon Sep 17 00:00:00 2001 From: Mike Dirolf Date: Wed, 30 Sep 2009 10:49:08 -0400 Subject: [PATCH] driver was sending hard limits where it should've sent soft, and vice-versa. fix and add tests for this --- README.rdoc | 3 + lib/mongo/cursor.rb | 14 +++-- lib/mongo/db.rb | 4 +- lib/mongo/message/query_message.rb | 2 +- test/test_collection.rb | 33 ++++++++++ test/test_cursor.rb | 96 +++++++++++++----------------- 6 files changed, 91 insertions(+), 61 deletions(-) diff --git a/README.rdoc b/README.rdoc index 704aa4c..de15583 100644 --- a/README.rdoc +++ b/README.rdoc @@ -335,6 +335,9 @@ Kyle Banker, banker on github Michael Bernstein, mrb on github * #sort method for Cursor instances +Paulo Ahahgon, pahagon on github +* removed hard limit + = License Copyright 2008-2009 10gen Inc. diff --git a/lib/mongo/cursor.rb b/lib/mongo/cursor.rb index 2cc08e9..b470d5e 100644 --- a/lib/mongo/cursor.rb +++ b/lib/mongo/cursor.rb @@ -173,7 +173,6 @@ module Mongo # Collection#find for details. def close @db.send_to_db(KillCursorsMessage.new(@cursor_id)) if @cursor_id - @cache = [] @cursor_id = 0 @closed = true end @@ -207,8 +206,15 @@ module Mongo @result_flags = header_buf.get_int @cursor_id = header_buf.get_long @starting_from = header_buf.get_int - @n_returned = header_buf.get_int - @n_remaining = @n_returned + @n_remaining = header_buf.get_int + if @n_received + @n_received += @n_remaining + else + @n_received = @n_remaining + end + if @query.number_to_return > 0 and @n_received >= @query.number_to_return + close() + end end def num_remaining @@ -267,7 +273,7 @@ module Mongo end def to_s - "DBResponse(flags=#@result_flags, cursor_id=#@cursor_id, start=#@starting_from, n_returned=#@n_returned)" + "DBResponse(flags=#@result_flags, cursor_id=#@cursor_id, start=#@starting_from)" end def check_modifiable diff --git a/lib/mongo/db.rb b/lib/mongo/db.rb index 573b618..6398ebb 100644 --- a/lib/mongo/db.rb +++ b/lib/mongo/db.rb @@ -525,8 +525,6 @@ module Mongo # DB commands need to be ordered, so selector must be an OrderedHash # (or a Hash with only one element). What DB commands really need is # that the "command" key be first. - # - # Do not call this. Intended for driver use only. def db_command(selector, use_admin_db=false) if !selector.kind_of?(OrderedHash) if !selector.kind_of?(Hash) || selector.keys.length > 1 @@ -535,7 +533,7 @@ module Mongo end q = Query.new(selector) - q.number_to_return = 1 + q.number_to_return = -1 query(Collection.new(self, SYSTEM_COMMAND_COLLECTION), q, use_admin_db).next_object end diff --git a/lib/mongo/message/query_message.rb b/lib/mongo/message/query_message.rb index 3c14291..b53307a 100644 --- a/lib/mongo/message/query_message.rb +++ b/lib/mongo/message/query_message.rb @@ -31,7 +31,7 @@ module Mongo write_int(0) write_string("#{db_name}.#{collection_name}") write_int(query.number_to_skip) - write_int(-query.number_to_return) # Negative means hard limit + write_int(query.number_to_return) sel = query.selector if query.contains_special_fields sel = OrderedHash.new diff --git a/test/test_collection.rb b/test/test_collection.rb index b237a50..3fe5a69 100644 --- a/test/test_collection.rb +++ b/test/test_collection.rb @@ -216,6 +216,39 @@ class TestCollection < Test::Unit::TestCase assert_equal 5, @@test.find({}, :skip => 3, :limit => 5).to_a.length end + def test_large_limit + 2000.times do |i| + @@test.insert("x" => i, "y" => "mongomongo" * 1000) + end + + assert_equal 2000, @@test.count + + i = 0 + y = 0 + @@test.find({}, :limit => 1900).each do |doc| + i += 1 + y += doc["x"] + end + + assert_equal 1900, i + assert_equal 1804050, y + end + + def test_small_limit + @@test.insert("x" => "hello world") + @@test.insert("x" => "goodbye world") + + assert_equal 2, @@test.count + + x = 0 + @@test.find({}, :limit => 1).each do |doc| + x += 1 + assert_equal "hello world", doc["x"] + end + + assert_equal 1, x + end + def test_group_with_scope @@test.save("a" => 1) @@test.save("b" => 1) diff --git a/test/test_cursor.rb b/test/test_cursor.rb index efecd4d..e989e3b 100644 --- a/test/test_cursor.rb +++ b/test/test_cursor.rb @@ -167,69 +167,59 @@ class CursorTest < Test::Unit::TestCase end def test_refill_via_get_more - begin - assert_equal 1, @@coll.count - 1000.times { |i| - assert_equal 1 + i, @@coll.count - @@coll.insert('a' => i) - } + assert_equal 1, @@coll.count + 1000.times { |i| + assert_equal 1 + i, @@coll.count + @@coll.insert('a' => i) + } - assert_equal 1001, @@coll.count - count = 0 - @@coll.find.each { |obj| - count += obj['a'] - } - assert_equal 1001, @@coll.count + assert_equal 1001, @@coll.count + count = 0 + @@coll.find.each { |obj| + count += obj['a'] + } + assert_equal 1001, @@coll.count - # do the same thing again for debugging - assert_equal 1001, @@coll.count - count2 = 0 - @@coll.find.each { |obj| - count2 += obj['a'] - } - assert_equal 1001, @@coll.count + # do the same thing again for debugging + assert_equal 1001, @@coll.count + count2 = 0 + @@coll.find.each { |obj| + count2 += obj['a'] + } + assert_equal 1001, @@coll.count - assert_equal count, count2 - assert_equal 499501, count - rescue Test::Unit::AssertionFailedError => ex - p @@db.collection_names - Process.exit 1 - end + assert_equal count, count2 + assert_equal 499501, count end def test_refill_via_get_more_alt_coll - begin - coll = @@db.collection('test-alt-coll') - coll.clear - coll.insert('a' => 1) # collection not created until it's used - assert_equal 1, coll.count + coll = @@db.collection('test-alt-coll') + coll.clear + coll.insert('a' => 1) # collection not created until it's used + assert_equal 1, coll.count - 1000.times { |i| - assert_equal 1 + i, coll.count - coll.insert('a' => i) - } + 1000.times { |i| + assert_equal 1 + i, coll.count + coll.insert('a' => i) + } - assert_equal 1001, coll.count - count = 0 - coll.find.each { |obj| - count += obj['a'] - } - assert_equal 1001, coll.count + assert_equal 1001, coll.count + count = 0 + coll.find.each { |obj| + count += obj['a'] + } + assert_equal 1001, coll.count - # do the same thing again for debugging - assert_equal 1001, coll.count - count2 = 0 - coll.find.each { |obj| - count2 += obj['a'] - } - assert_equal 1001, coll.count + # do the same thing again for debugging + assert_equal 1001, coll.count + count2 = 0 + coll.find.each { |obj| + count2 += obj['a'] + } + assert_equal 1001, coll.count - assert_equal count, count2 - assert_equal 499501, count - rescue Test::Unit::AssertionFailedError => ex - p @@db.collection_names - Process.exit 1 - end + assert_equal count, count2 + assert_equal 499501, count end def test_close_after_query_sent