From 4fdcad1327b4e54dfb77992c4d58cff99818b6a7 Mon Sep 17 00:00:00 2001 From: Mike Dirolf Date: Thu, 8 Oct 2009 10:02:54 -0400 Subject: [PATCH] cleanup and credits --- README.rdoc | 3 ++- lib/mongo/collection.rb | 6 +++--- lib/mongo/cursor.rb | 18 ++++++++++++----- lib/mongo/errors.rb | 2 +- lib/mongo/message/query_message.rb | 4 ++-- lib/mongo/query.rb | 2 +- lib/mongo/util/conversions.rb | 8 ++++---- test/test_cursor.rb | 22 ++++++++++++++------- test/test_db_api.rb | 31 ++++++++++++------------------ 9 files changed, 53 insertions(+), 43 deletions(-) diff --git a/README.rdoc b/README.rdoc index 4ece228..ea17007 100644 --- a/README.rdoc +++ b/README.rdoc @@ -320,8 +320,9 @@ Paul Dlug, paul.dlug@gmail.com * Generate _id on the client side if not provided * Collection#insert and Collection#save return _id -Durran Jordan and Les Hill, durran@gmail.com +Durran Jordan, durran@gmail.com * DB#collections +* Support for specifying sort order as array of [key, direction] pairs Cyril Mougel, cyril.mougel@gmail.com * Initial logging support diff --git a/lib/mongo/collection.rb b/lib/mongo/collection.rb index f4d8fcc..be33a31 100644 --- a/lib/mongo/collection.rb +++ b/lib/mongo/collection.rb @@ -91,9 +91,9 @@ module Mongo # :skip :: Number of documents to omit (from the start of the result set) # when returning the results # :limit :: Maximum number of records to return - # :sort :: Either hash of field names as keys and 1/-1 as values; 1 == - # ascending, -1 == descending, or array of field names (all - # assumed to be sorted in ascending order). + # :sort :: An array of [key, direction] pairs to sort by. Direction should + # be specified as Mongo::ASCENDING (or :ascending / :asc) or + # Mongo::DESCENDING (or :descending / :desc) # :hint :: See #hint. This option overrides the collection-wide value. # :snapshot :: If true, snapshot mode will be used for this query. # Snapshot mode assures no duplicates are returned, or diff --git a/lib/mongo/cursor.rb b/lib/mongo/cursor.rb index b470d5e..cbd3df1 100644 --- a/lib/mongo/cursor.rb +++ b/lib/mongo/cursor.rb @@ -75,16 +75,24 @@ module Mongo # Sort this cursor's result # - # Takes either a hash of field names as keys and 1/-1 as values; 1 == - # ascending, -1 == descending, or array of field names (all assumed to be - # sorted in ascending order). + # Takes either a single key and a direction, or an array of [key, + # direction] pairs. Directions should be specified as Mongo::ASCENDING + # or Mongo::DESCENDING (or :ascending or :descending) (or :asc or :desc). # - # Raises InvalidOperation if this cursor has already been used. + # Raises InvalidOperation if this cursor has already been used. Raises + # InvalidSortValueError if specified order is invalid. # # This method overrides any sort order specified in the Collection#find # method, and only the last sort applied has an effect - def sort(order) + def sort(key_or_list, direction=nil) check_modifiable + + if !direction.nil? + order = [[key_or_list, direction]] + else + order = key_or_list + end + @query.order_by = order self end diff --git a/lib/mongo/errors.rb b/lib/mongo/errors.rb index 365c2ef..3eefbe6 100644 --- a/lib/mongo/errors.rb +++ b/lib/mongo/errors.rb @@ -24,6 +24,6 @@ module Mongo # Raised when an invalid name is used. class InvalidName < RuntimeError; end - # Raised when the client supplies an invalid values for a sorting. + # Raised when the client supplies an invalid value to sort by. class InvalidSortValueError < RuntimeError; end end diff --git a/lib/mongo/message/query_message.rb b/lib/mongo/message/query_message.rb index 8778605..ae22f92 100644 --- a/lib/mongo/message/query_message.rb +++ b/lib/mongo/message/query_message.rb @@ -37,7 +37,7 @@ module Mongo if query.contains_special_fields sel = OrderedHash.new sel['query'] = query.selector - if query.order_by && query.order_by.length > 0 + if query.order_by order_by = query.order_by sel['orderby'] = case order_by when String then string_as_sort_parameters(order_by) @@ -47,7 +47,7 @@ module Mongo warn_if_deprecated(order_by) order_by else - raise "illegal order_by: is a #{query.order_by.class.name}, must be String, Array, Hash, or OrderedHash" + raise InvalidSortValueError.new("illegal order_by: is a #{query.order_by.class.name}, must be String, Array, Hash, or OrderedHash") end end sel['$hint'] = query.hint if query.hint && query.hint.length > 0 diff --git a/lib/mongo/query.rb b/lib/mongo/query.rb index 5559367..65fee6f 100644 --- a/lib/mongo/query.rb +++ b/lib/mongo/query.rb @@ -108,7 +108,7 @@ module Mongo end def contains_special_fields - (@order_by != nil && @order_by.length > 0) || @explain || @hint || @snapshot + @order_by || @explain || @hint || @snapshot end def to_s diff --git a/lib/mongo/util/conversions.rb b/lib/mongo/util/conversions.rb index c75fc2d..e5842a9 100644 --- a/lib/mongo/util/conversions.rb +++ b/lib/mongo/util/conversions.rb @@ -93,18 +93,18 @@ module Mongo #:nodoc: return 1 if ASCENDING.include?(val) return -1 if DESCENDING.include?(val) raise InvalidSortValueError.new( - "#{self} was supplied as a sort value when acceptable values are: " + - "ascending, asc, :ascending, :asc, 1, descending, desc, :descending, :desc, -1.") + "#{self} was supplied as a sort direction when acceptable values are: " + + "Mongo::ASCENDING, 'ascending', 'asc', :ascending, :asc, 1, Mongo::DESCENDING, 'descending', 'desc', :descending, :desc, -1.") end # This is the method to call when the client needs to be warned of # deprecation in the way sorting parameters are supplied. def warn_if_deprecated(value) unless value.is_a?(Array) && value.first.is_a?(Array) - warn("\nSorting has been deprecated in favor of a new syntax: \n" + + warn("Specifying sort order as #{value.inspect} has been deprecated in favor of a new syntax: \n" + " :sort => [['field1', '(ascending|descending)'], ['field2', '(ascending|descending)']]") end end end -end \ No newline at end of file +end diff --git a/test/test_cursor.rb b/test/test_cursor.rb index e989e3b..e7f093b 100644 --- a/test/test_cursor.rb +++ b/test/test_cursor.rb @@ -62,22 +62,30 @@ class CursorTest < Test::Unit::TestCase @@coll.clear 5.times{|x| @@coll.insert({"a" => x}) } - assert_kind_of Cursor, @@coll.find().sort({:a => 1}) + assert_kind_of Cursor, @@coll.find().sort(:a, 1) - assert_equal 0, @@coll.find().sort({:a => 1}).next_object["a"] - assert_equal 4, @@coll.find().sort({:a => -1}).next_object["a"] - assert_equal 0, @@coll.find().sort(["a"]).next_object["a"] + assert_equal 0, @@coll.find().sort(:a, 1).next_object["a"] + assert_equal 4, @@coll.find().sort(:a, -1).next_object["a"] + assert_equal 0, @@coll.find().sort([["a", :asc]]).next_object["a"] - assert_kind_of Cursor, @@coll.find().sort({:a => -1, :b => 1}) + assert_kind_of Cursor, @@coll.find().sort([[:a, -1], [:b, 1]]) - assert_equal 4, @@coll.find().sort({:a => 1}).sort({:a => -1}).next_object["a"] - assert_equal 0, @@coll.find().sort({:a => -1}).sort({:a => 1}).next_object["a"] + assert_equal 4, @@coll.find().sort(:a, 1).sort(:a, -1).next_object["a"] + assert_equal 0, @@coll.find().sort(:a, -1).sort(:a, 1).next_object["a"] cursor = @@coll.find() cursor.next_object() assert_raise InvalidOperation do cursor.sort(["a"]) end + + assert_raise InvalidSortValueError do + @@coll.find().sort(:a, 25).next_object + end + + assert_raise InvalidSortValueError do + @@coll.find().sort(25).next_object + end end def test_limit diff --git a/test/test_db_api.rb b/test/test_db_api.rb index aa6237c..c34aeba 100644 --- a/test/test_db_api.rb +++ b/test/test_db_api.rb @@ -149,7 +149,7 @@ class DBAPITest < Test::Unit::TestCase @@coll.insert('a' => 4, 'b' => 1) # Sorting (ascending) - docs = @@coll.find({'a' => { '$lt' => 10 }}, :sort => {'a' => 1}).to_a + docs = @@coll.find({'a' => { '$lt' => 10 }}, :sort => [['a', 1]]).to_a assert_equal 4, docs.size assert_equal 1, docs[0]['a'] assert_equal 2, docs[1]['a'] @@ -157,7 +157,7 @@ class DBAPITest < Test::Unit::TestCase assert_equal 4, docs[3]['a'] # Sorting (descending) - docs = @@coll.find({'a' => { '$lt' => 10 }}, :sort => {'a' => -1}).to_a + docs = @@coll.find({'a' => { '$lt' => 10 }}, :sort => [['a', -1]]).to_a assert_equal 4, docs.size assert_equal 4, docs[0]['a'] assert_equal 3, docs[1]['a'] @@ -187,16 +187,10 @@ class DBAPITest < Test::Unit::TestCase assert_equal 1, docs[2]['a'] assert_equal 3, docs[3]['a'] - # Sorting using empty array; no order guarantee (Mongo bug #898) but - # should not blow up. + # Sorting using empty array; no order guarantee should not blow up. docs = @@coll.find({'a' => { '$lt' => 10 }}, :sort => []).to_a assert_equal 4, docs.size - # Sorting using array of hashes; no order guarantee (Mongo bug #898) but - # should not blow up. - docs = @@coll.find({'a' => { '$lt' => 10 }}, :sort => [{'b' => 1}, {'a' => -1}]).to_a - assert_equal 4, docs.size - # Sorting using ordered hash. You can use an unordered one, but then the # order of the keys won't be guaranteed thus your sort won't make sense. oh = OrderedHash.new @@ -208,16 +202,15 @@ class DBAPITest < Test::Unit::TestCase assert_equal 2, docs[2]['a'] assert_equal 1, docs[3]['a'] - # TODO this will not pass due to known Mongo bug #898 -# oh = OrderedHash.new -# oh['b'] = -1 -# oh['a'] = 1 -# docs = @@coll.find({'a' => { '$lt' => 10 }}, :sort => oh).to_a -# assert_equal 4, docs.size -# assert_equal 1, docs[0]['a'] -# assert_equal 3, docs[1]['a'] -# assert_equal 2, docs[2]['a'] -# assert_equal 4, docs[3]['a'] + oh = OrderedHash.new + oh['b'] = -1 + oh['a'] = 1 + docs = @@coll.find({'a' => { '$lt' => 10 }}, :sort => oh).to_a + assert_equal 4, docs.size + assert_equal 1, docs[0]['a'] + assert_equal 3, docs[1]['a'] + assert_equal 2, docs[2]['a'] + assert_equal 4, docs[3]['a'] end def test_find_limits