From 515594ebf3a0f6165a0ebbb5ef798873aae0672e Mon Sep 17 00:00:00 2001 From: Kyle Banker Date: Mon, 23 Nov 2009 18:09:13 -0500 Subject: [PATCH] Completed deprecation of old sort options, :offset, and Collection#clear --- lib/mongo/collection.rb | 19 ++++------------- lib/mongo/connection.rb | 1 + lib/mongo/cursor.rb | 15 ++++++------- lib/mongo/util/conversions.rb | 40 ++++------------------------------- test/test_collection.rb | 8 ------- test/test_conversions.rb | 4 ++-- test/test_db_api.rb | 21 ++++-------------- 7 files changed, 21 insertions(+), 87 deletions(-) diff --git a/lib/mongo/collection.rb b/lib/mongo/collection.rb index 704d629..45917ea 100644 --- a/lib/mongo/collection.rb +++ b/lib/mongo/collection.rb @@ -110,14 +110,10 @@ module Mongo def find(selector={}, options={}) fields = options.delete(:fields) fields = ["_id"] if fields && fields.empty? - skip = options.delete(:offset) || nil - if !skip.nil? - warn "the :offset option to find is deprecated and will be removed. please use :skip instead" - end - skip = options.delete(:skip) || skip || 0 - limit = options.delete(:limit) || 0 - sort = options.delete(:sort) - hint = options.delete(:hint) + skip = options.delete(:skip) || skip || 0 + limit = options.delete(:limit) || 0 + sort = options.delete(:sort) + hint = options.delete(:hint) snapshot = options.delete(:snapshot) if options[:timeout] == false && !block_given? raise ArgumentError, "Timeout can be set to false only when #find is invoked with a block." @@ -227,13 +223,6 @@ module Mongo "db.#{@db.name}.remove(#{selector.inspect})") end - # Remove all records. - # DEPRECATED: please use Collection#remove instead. - def clear - warn "Collection#clear is deprecated. Please use Collection#remove instead." - remove({}) - end - # Update a single document in this collection. # # :selector :: a hash specifying elements which must be present for a document to be updated. Note: diff --git a/lib/mongo/connection.rb b/lib/mongo/connection.rb index dcf592a..258a46a 100644 --- a/lib/mongo/connection.rb +++ b/lib/mongo/connection.rb @@ -278,6 +278,7 @@ module Mongo # Creates a new socket and tries to connect to master. # If successful, sets @host and @port to master and returns the socket. def connect_to_master + close @host = @port = nil for node_pair in @nodes host, port = *node_pair diff --git a/lib/mongo/cursor.rb b/lib/mongo/cursor.rb index 4bba2cb..7140636 100644 --- a/lib/mongo/cursor.rb +++ b/lib/mongo/cursor.rb @@ -67,7 +67,7 @@ module Mongo # servers, next request will re-open on master server. if err == "not master" raise ConnectionFailure, err - @db.close + @connection.close end raise OperationFailure, err @@ -301,7 +301,7 @@ module Mongo close_cursor_if_query_complete end - # Run query first time we request an object from the wire + # Run query the first time we request an object from the wire def send_query_if_needed if @query_run false @@ -349,14 +349,11 @@ module Mongo def formatted_order_clause case @order - when String then string_as_sort_parameters(@order) - when Symbol then symbol_as_sort_parameters(@order) - when Array then array_as_sort_parameters(@order) - when Hash # Should be an ordered hash, but this message doesn't care - warn_if_deprecated(@order) - @order + when String, Symbol then string_as_sort_parameters(@order) + when Array then array_as_sort_parameters(@order) else - raise InvalidSortValueError, "Illegal order_by, '#{@order.class.name}'; must be String, Array, Hash, or OrderedHash" + raise InvalidSortValueError, "Illegal sort clause, '#{@order.class.name}'; must be of the form " + + "[['field1', '(ascending|descending)'], ['field2', '(ascending|descending)']]" end end diff --git a/lib/mongo/util/conversions.rb b/lib/mongo/util/conversions.rb index e5842a9..971d7cb 100644 --- a/lib/mongo/util/conversions.rb +++ b/lib/mongo/util/conversions.rb @@ -19,7 +19,7 @@ module Mongo #:nodoc: # objects to mongo-friendly parameters. module Conversions - ASCENDING = ["ascending", "asc", "1"] + ASCENDING = ["ascending", "asc", "1"] DESCENDING = ["descending", "desc", "-1"] # Converts the supplied +Array+ to a +Hash+ to pass to mongo as @@ -28,17 +28,9 @@ module Mongo #:nodoc: # # Example: # - # *DEPRECATED - # - # array_as_sort_parameters(["field1", "field2"]) => - # { "field1" => "1", "field2" => "1" } - # - # *New Syntax: - # # array_as_sort_parameters([["field1", :asc], ["field2", :desc]]) => # { "field1" => 1, "field2" => -1} def array_as_sort_parameters(value) - warn_if_deprecated(value) order_by = OrderedHash.new value.each do |param| if (param.class.name == "String") @@ -50,7 +42,7 @@ module Mongo #:nodoc: order_by end - # Converts the supplied +String+ to a +Hash+ to pass to mongo as + # Converts the supplied +String+ or +Symbol+ to a +Hash+ to pass to mongo as # a sorting parameter with ascending order. If the +String+ # is empty then an empty +Hash+ will be returned. # @@ -61,22 +53,8 @@ module Mongo #:nodoc: # string_as_sort_parameters("field") => { "field" => 1 } # string_as_sort_parameters("") => {} def string_as_sort_parameters(value) - warn_if_deprecated(value) - return {} if value.empty? - { value => 1 } - end - - # Converts the supplied +Symbol+ to a +Hash+ to pass to mongo as - # a sorting parameter with ascending order. - # - # Example: - # - # *DEPRECATED - # - # symbol_as_sort_parameters(:field) => { "field" => 1 } - def symbol_as_sort_parameters(value) - warn_if_deprecated(value) - { value.to_s => 1 } + return {} if (str = value.to_s).empty? + { str => 1 } end # Converts the +String+, +Symbol+, or +Integer+ to the @@ -96,15 +74,5 @@ module Mongo #:nodoc: "#{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("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 diff --git a/test/test_collection.rb b/test/test_collection.rb index 34f1efc..8ecdf1e 100644 --- a/test/test_collection.rb +++ b/test/test_collection.rb @@ -290,9 +290,6 @@ class TestCollection < Test::Unit::TestCase @@test.save(:foo => i) end - # TODO remove test for deprecated :offset option - assert_equal 5, @@test.find({}, :offset => 5).next_object()["foo"] - assert_equal 5, @@test.find({}, :skip => 5).next_object()["foo"] assert_equal nil, @@test.find({}, :skip => 10).next_object() @@ -387,11 +384,6 @@ class TestCollection < Test::Unit::TestCase assert_equal 0, @collection.find.count end - should "remove all records if deprecated clear is used" do - @collection.clear - assert_equal 0, @collection.find.count - end - should "remove only matching records" do @collection.remove({:name => "Jones"}) assert_equal 1, @collection.size diff --git a/test/test_conversions.rb b/test/test_conversions.rb index 8d5a91f..ab3e28c 100644 --- a/test/test_conversions.rb +++ b/test/test_conversions.rb @@ -28,7 +28,7 @@ class ConversionsTest < Test::Unit::TestCase end def test_symbol_as_sort_parameters - params = symbol_as_sort_parameters(:field) + params = string_as_sort_parameters(:field) assert_equal({ "field" => 1 }, params) end @@ -118,4 +118,4 @@ class ConversionsTest < Test::Unit::TestCase end end -end \ No newline at end of file +end diff --git a/test/test_db_api.rb b/test/test_db_api.rb index b291c04..f046a97 100644 --- a/test/test_db_api.rb +++ b/test/test_db_api.rb @@ -167,7 +167,7 @@ class DBAPITest < Test::Unit::TestCase assert_equal 1, docs[3]['a'] # Sorting using array of names; assumes ascending order. - docs = @@coll.find({'a' => { '$lt' => 10 }}, :sort => ['a']).to_a + docs = @@coll.find({'a' => { '$lt' => 10 }}, :sort => 'a').to_a assert_equal 4, docs.size assert_equal 1, docs[0]['a'] assert_equal 2, docs[1]['a'] @@ -197,22 +197,9 @@ class DBAPITest < Test::Unit::TestCase # order of the keys won't be guaranteed thus your sort won't make sense. oh = OrderedHash.new oh['a'] = -1 - docs = @@coll.find({'a' => { '$lt' => 10 }}, :sort => oh).to_a - assert_equal 4, docs.size - assert_equal 4, docs[0]['a'] - assert_equal 3, docs[1]['a'] - assert_equal 2, docs[2]['a'] - assert_equal 1, 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'] + assert_raise InvalidSortValueError do + docs = @@coll.find({'a' => { '$lt' => 10 }}, :sort => oh).to_a + end end def test_find_limits