diff --git a/lib/mongo/collection/view.rb b/lib/mongo/collection/view.rb index e48ef7e750..cc15686ecf 100644 --- a/lib/mongo/collection/view.rb +++ b/lib/mongo/collection/view.rb @@ -127,8 +127,7 @@ def hash def initialize(collection, selector = {}, options = {}) validate_doc!(selector) @collection = collection - @selector = selector.dup - @options = options.dup + setup(selector, options) end # Get a human-readable string representation of +View+. diff --git a/lib/mongo/collection/view/immutable.rb b/lib/mongo/collection/view/immutable.rb index ebaab9855f..3d64729f7a 100644 --- a/lib/mongo/collection/view/immutable.rb +++ b/lib/mongo/collection/view/immutable.rb @@ -22,14 +22,14 @@ module Immutable private - # @api private - # - # @note In the including class, the method #immutable needs to be - # implemented in order to define how a new class of that type needs to - # be instantiated. def configure(field, value) return options[field] if value.nil? - new(options.merge(field => value)) + new(options.merge(field => value, :modifiers => @modifiers)) + end + + def configure_modifier(field, value) + return @modifiers[Readable::SPECIAL_FIELDS[field]] if value.nil? + new(options.merge(:modifiers => @modifiers.merge(Readable::SPECIAL_FIELDS[field] => value))) end def configure_flag(flag) diff --git a/lib/mongo/collection/view/map_reduce.rb b/lib/mongo/collection/view/map_reduce.rb index 35e55f764b..bee1d20caf 100644 --- a/lib/mongo/collection/view/map_reduce.rb +++ b/lib/mongo/collection/view/map_reduce.rb @@ -159,12 +159,16 @@ def map_reduce_spec :mapreduce => collection.name, :map => map, :reduce => reduce, - :query => view.selector[:$query] || view.selector, + :query => view.modifiers[:$query] || view.selector, :out => { inline: 1 } - }.merge(options).merge(view.options) + }.merge(options).merge(view_options) } end + def view_options + view.sort ? view.options.merge(:sort => view.sort) : view.options + end + def new(options) MapReduce.new(view, map, reduce, options) end diff --git a/lib/mongo/collection/view/readable.rb b/lib/mongo/collection/view/readable.rb index a6cfb45812..c1be99ffd6 100644 --- a/lib/mongo/collection/view/readable.rb +++ b/lib/mongo/collection/view/readable.rb @@ -21,30 +21,23 @@ class View # @since 2.0.0 module Readable - # Special fields and their getters for the query selector. + # Special fields and their option names for the query selector. # # @since 2.0.0 SPECIAL_FIELDS = { - :$query => :selector, - :$readPreference => :read_pref_formatted, - :$orderby => :sort, - :$hint => :hint, - :$comment => :comment, - :$snapshot => :snapshot, - :$maxScan => :max_scan, - :$max => :max_value, - :$min => :min_value, - :$maxTimeMS => :max_time_ms, - :$returnKey => :return_key, - :$showDiskLoc => :show_disk_loc, - :$explain => :explained? + :sort => :$orderby, + :hint => :$hint, + :comment => :$comment, + :snapshot => :$snapshot, + :max_scan => :$maxScan, + :max_value => :$max, + :min_value => :$min, + :max_time_ms => :$maxTimeMS, + :return_key => :$returnKey, + :show_disk_loc => :$showDiskLoc, + :explain => :$explain }.freeze - # Get just the names of the special fields. - # - # @since 2.1.0 - SPECIAL_FIELD_OPTION_NAMES = SPECIAL_FIELDS.values.freeze - # Options to cursor flags mapping. # # @since 2.1.0 @@ -99,7 +92,7 @@ def allow_partial_results # # @since 2.0.0 def batch_size(batch_size = nil) - configure(:batch_size, batch_size) + configure(__method__, batch_size) end # Associate a comment with the query. @@ -117,7 +110,7 @@ def batch_size(batch_size = nil) # # @since 2.0.0 def comment(comment = nil) - configure(:comment, comment) + configure_modifier(__method__, comment) end # Get a count of matching documents in the collection. @@ -181,7 +174,7 @@ def distinct(field_name, options={}) # # @since 2.0.0 def hint(hint = nil) - configure(:hint, hint) + configure_modifier(__method__, hint) end # The max number of docs to return from the query. @@ -195,7 +188,7 @@ def hint(hint = nil) # # @since 2.0.0 def limit(limit = nil) - configure(:limit, limit) + configure(__method__, limit) end # Execute a map/reduce operation on the collection view. @@ -225,7 +218,7 @@ def map_reduce(map, reduce, options = {}) # # @since 2.0.0 def max_scan(value = nil) - configure(:max_scan, value) + configure_modifier(__method__, value) end # Set the maximum value to search. @@ -239,7 +232,7 @@ def max_scan(value = nil) # # @since 2.1.0 def max_value(value = nil) - configure(:max_value, value) + configure_modifier(__method__, value) end # Set the minimum value to search. @@ -253,7 +246,7 @@ def max_value(value = nil) # # @since 2.1.0 def min_value(value = nil) - configure(:min_value, value) + configure_modifier(__method__, value) end # The server normally times out idle cursors after an inactivity period @@ -266,7 +259,7 @@ def min_value(value = nil) # # @since 2.0.0 def no_cursor_timeout - configure_flag(:no_cursor_timeout) + configure_flag(__method__) end # The fields to include or exclude from each doc in the result set. @@ -285,7 +278,7 @@ def no_cursor_timeout # @since 2.0.0 def projection(document = nil) validate_doc!(document) if document - configure(:projection, document) + configure(__method__, document) end # The read preference to use for the query. @@ -301,7 +294,7 @@ def projection(document = nil) # @since 2.0.0 def read(value = nil) return default_read if value.nil? - configure(:read, value.is_a?(Hash) ? ServerSelector.get(value) : value) + configure(__method__, value.is_a?(Hash) ? ServerSelector.get(value) : value) end # Set whether to return only the indexed field or fields. @@ -315,7 +308,7 @@ def read(value = nil) # # @since 2.1.0 def return_key(value = nil) - configure(:return_key, value) + configure_modifier(__method__, value) end # Set whether the disk location should be shown for each document. @@ -330,7 +323,7 @@ def return_key(value = nil) # # @since 2.0.0 def show_disk_loc(value = nil) - configure(:show_disk_loc, value) + configure_modifier(__method__, value) end # The number of docs to skip before returning results. @@ -345,7 +338,7 @@ def show_disk_loc(value = nil) # # @since 2.0.0 def skip(number = nil) - configure(:skip, number) + configure(__method__, number) end # Set the snapshot value for the view. @@ -360,7 +353,7 @@ def skip(number = nil) # # @since 2.0.0 def snapshot(value = nil) - configure(:snapshot, value) + configure_modifier(__method__, value) end # The key and direction pairs by which the result set will be sorted. @@ -375,7 +368,7 @@ def snapshot(value = nil) # # @since 2.0.0 def sort(spec = nil) - configure(:sort, spec) + configure_modifier(__method__, spec) end # “meta” operators that let you modify the output or behavior of a query. @@ -389,7 +382,8 @@ def sort(spec = nil) # # @since 2.1.0 def modifiers(doc = nil) - configure(:modifiers, doc) + return @modifiers if doc.nil? + new(options.merge(__method__ => doc)) end # A cumulative time limit in milliseconds for processing operations on a cursor. @@ -403,7 +397,7 @@ def modifiers(doc = nil) # # @since 2.1.0 def max_time_ms(max = nil) - configure(:max_time_ms, max) + configure_modifier(__method__, max) end private @@ -421,10 +415,6 @@ def flags end end - def has_special_fields? - contains_modifiers? || explained? || cluster.sharded? - end - def parallel_scan(cursor_count) server = read.select_server(cluster) Operation::ParallelScan.new( @@ -441,6 +431,27 @@ def parallel_scan(cursor_count) end end + def setup(sel, opts) + setup_options(opts) + setup_selector(sel) + end + + def setup_options(opts) + @options = opts ? opts.dup : {} + @modifiers = @options[:modifiers] ? @options.delete(:modifiers).dup : BSON::Document.new + @options.keys.each { |k| @modifiers.merge!(SPECIAL_FIELDS[k] => @options.delete(k)) if SPECIAL_FIELDS[k] } + @options.freeze + end + + def setup_selector(sel) + @selector = sel ? sel.dup : {} + if @selector[:$query] || @selector['$query'] + @selector.keys.each { |k| @modifiers.merge!(k => @selector.delete(k)) if k[0] == '$' } + end + @modifiers.freeze + @selector.freeze + end + def query_options { :project => projection, @@ -451,8 +462,12 @@ def query_options } end + def requires_special_selector? + !modifiers.empty? || cluster.sharded? + end + def query_spec - sel = has_special_fields? ? special_selector : selector + sel = requires_special_selector? ? special_selector : selector { :selector => sel, :read => read, :options => query_options, @@ -461,26 +476,18 @@ def query_spec end def read_pref_formatted - read.to_mongos + @read_formatted ||= read.to_mongos end def special_selector - SPECIAL_FIELDS.reduce({}) do |hash, (key, method)| - value = send(method) || (options[:modifiers] && options[:modifiers][key]) - hash[key] = value unless value.nil? - hash - end + sel = BSON::Document.new(:$query => selector).merge!(modifiers) + sel[:$readPreference] = read_pref_formatted unless read_pref_formatted.nil? + sel end def validate_doc!(doc) raise Error::InvalidDocument.new unless doc.respond_to?(:keys) end - - def contains_modifiers? - modifiers || options.keys.any? do |key| - SPECIAL_FIELD_OPTION_NAMES.include?(key) - end - end end end end diff --git a/lib/mongo/grid/fs_bucket.rb b/lib/mongo/grid/fs_bucket.rb index 387ba24fe0..1a9a5384ca 100644 --- a/lib/mongo/grid/fs_bucket.rb +++ b/lib/mongo/grid/fs_bucket.rb @@ -261,13 +261,19 @@ def download_to_stream(id, io) # # @since 2.1.0 def open_download_stream_by_name(filename, opts = {}, &block) - view = files_collection.find(:filename => filename).projection(_id: 1) revision = opts.fetch(:revision, -1) if revision < 0 - file_doc = view.sort('uploadDate' => Mongo::Index::DESCENDING).skip(revision.abs - 1).limit(-1).first + skip = revision.abs - 1 + sort = { 'uploadDate' => Mongo::Index::DESCENDING } else - file_doc = view.sort('uploadDate' => Mongo::Index::ASCENDING).skip(revision).limit(-1).first + skip = revision + sort = { 'uploadDate' => Mongo::Index::ASCENDING } end + file_doc = files_collection.find({ filename: filename} , + projection: { _id: 1 }, + sort: sort, + skip: skip, + limit: -1).first unless file_doc raise Error::FileNotFound.new(filename, :filename) unless opts[:revision] raise Error::InvalidFileRevision.new(filename, opts[:revision]) diff --git a/lib/mongo/grid/stream/read.rb b/lib/mongo/grid/stream/read.rb index d4e0fe5f2d..279a6edeec 100644 --- a/lib/mongo/grid/stream/read.rb +++ b/lib/mongo/grid/stream/read.rb @@ -49,8 +49,8 @@ class Read # @since 2.1.0 def initialize(fs, options) @fs = fs - @options = options - @file_id = options[:file_id] + @options = options.dup + @file_id = @options.delete(:file_id) @open = true end diff --git a/lib/mongo/protocol/query.rb b/lib/mongo/protocol/query.rb index 3c8dfa15b7..5ba8e0519f 100644 --- a/lib/mongo/protocol/query.rb +++ b/lib/mongo/protocol/query.rb @@ -258,7 +258,7 @@ def op_command def find_command document = BSON::Document.new document[:find] = collection - document[:filter] = filter.key?(:$query) ? filter[:$query] : filter + document[:filter] = filter[:$query] ? filter[:$query] : filter OPTION_MAPPINGS.each do |legacy, option| document[option] = options[legacy] unless options[legacy].nil? end diff --git a/spec/mongo/collection/view/immutable_spec.rb b/spec/mongo/collection/view/immutable_spec.rb new file mode 100644 index 0000000000..a203e5a1c8 --- /dev/null +++ b/spec/mongo/collection/view/immutable_spec.rb @@ -0,0 +1,103 @@ +require 'spec_helper' + +describe Mongo::Collection::View::Immutable do + + let(:selector) do + {} + end + + let(:options) do + {} + end + + let(:view) do + Mongo::Collection::View.new(authorized_collection, selector, options) + end + + after do + authorized_collection.delete_many + end + + describe '#configure' do + + context 'when the options has a modifiers document' do + + let(:options) do + { modifiers: { :$maxTimeMS => 500 } } + end + + let(:new_view) do + view.projection(_id: 1) + end + + it 'returns a new view' do + expect(view).not_to be(new_view) + end + + it 'creates a new options hash' do + expect(view.options).not_to be(new_view.options) + end + + it 'keeps the modifier fields already in the options hash' do + expect(new_view.modifiers[:$maxTimeMS]).to eq(500) + end + + it 'sets the option' do + expect(new_view.projection).to eq(_id: 1) + end + + it 'creates a new modifiers document' do + expect(view.modifiers).not_to be(new_view.modifiers) + end + end + end + + describe '#configure_modifier' do + + let(:new_view) do + view.sort('x' => Mongo::Index::ASCENDING) + end + + context 'when the options does not have a modifiers document' do + + it 'returns a new view' do + expect(view).not_to be(new_view) + end + + it 'returns a new view with the modifiers document containing the option' do + expect(new_view.modifiers[:$orderby]).to eq({ 'x' => Mongo::Index::ASCENDING }) + end + end + + context 'when the options has a modifiers document' do + + let(:options) do + { modifiers: { :$maxTimeMS => 500 } } + end + + it 'returns a new view' do + expect(view).not_to be(new_view) + end + + it 'creates a new options hash' do + expect(view.options).not_to be(new_view.options) + end + + it 'keeps the fields already in the options hash and merges in the new one' do + expect(new_view.modifiers[:$maxTimeMS]).to eq(500) + end + + it 'sets the new value in the new view modifier document' do + expect(new_view.modifiers[:$orderby]).to eq('x' => Mongo::Index::ASCENDING) + end + + it 'returns that value when the corresponding option method is called' do + expect(new_view.sort).to eq({ 'x' => Mongo::Index::ASCENDING }) + end + + it 'creates a new modifiers document' do + expect(view.modifiers).not_to be(new_view.modifiers) + end + end + end +end diff --git a/spec/mongo/collection/view/map_reduce_spec.rb b/spec/mongo/collection/view/map_reduce_spec.rb index 4987a7a73c..09c6f3be94 100644 --- a/spec/mongo/collection/view/map_reduce_spec.rb +++ b/spec/mongo/collection/view/map_reduce_spec.rb @@ -213,7 +213,7 @@ end it 'includes the selector in the operation spec' do - expect(map_reduce.send(:map_reduce_spec)[:selector][:query]).to eq(selector[:$query]) + expect(map_reduce.send(:map_reduce_spec)[:selector][:query]).to eq(BSON::Document.new(selector[:$query])) end end end @@ -411,7 +411,7 @@ end it 'includes the sort object in the operation spec' do - expect(map_reduce.send(:map_reduce_spec)[:selector][:sort]).to be(sort) + expect(map_reduce.send(:map_reduce_spec)[:selector][:sort][:name]).to eq(sort[:name]) end end diff --git a/spec/mongo/collection/view/readable_spec.rb b/spec/mongo/collection/view/readable_spec.rb index d05e393638..9d46ba1818 100644 --- a/spec/mongo/collection/view/readable_spec.rb +++ b/spec/mongo/collection/view/readable_spec.rb @@ -472,7 +472,7 @@ end it 'sets the value in the options' do - expect(new_view.max_value).to eq(_id: 1) + expect(new_view.max_value).to eq('_id' => 1) end end @@ -483,7 +483,7 @@ end it 'sets the value in the options' do - expect(new_view.min_value).to eq(_id: 1) + expect(new_view.min_value).to eq('_id' => 1) end end diff --git a/spec/mongo/collection/view_spec.rb b/spec/mongo/collection/view_spec.rb index 042c4f0c59..f17242b2f7 100644 --- a/spec/mongo/collection/view_spec.rb +++ b/spec/mongo/collection/view_spec.rb @@ -18,6 +18,180 @@ authorized_collection.delete_many end + context 'when query modifiers are provided' do + + context 'when a selector has a query modifier' do + + let(:options) do + {} + end + + let(:expected_modifiers) do + BSON::Document.new(selector) + end + + let(:parsed_selector) do + {} + end + + let(:query_selector) do + BSON::Document.new(selector) + end + + context 'when the $query key is a string' do + + let(:selector) do + { "$query" => { a: 1 }, :$someMod => 100 } + end + + let(:expected_modifiers) do + BSON::Document.new(selector) + end + + it 'sets the modifiers' do + expect(view.instance_variable_get(:@modifiers)).to eq(expected_modifiers) + end + + it 'removes the modifiers from the selector' do + expect(view.selector).to eq(parsed_selector) + end + + it 'creates the correct query selector' do + expect(view.send(:query_spec)[:selector]).to eq(query_selector) + end + + end + + context 'when the $query key is a symbol' do + + let(:selector) do + { :$query => { a: 1 }, :$someMod => 100 } + end + + let(:expected_modifiers) do + BSON::Document.new(selector) + end + + it 'sets the modifiers' do + expect(view.instance_variable_get(:@modifiers)).to eq(expected_modifiers) + end + + it 'removes the modifiers from the selector' do + expect(view.selector).to eq(parsed_selector) + end + + it 'creates the correct query selector' do + expect(view.send(:query_spec)[:selector]).to eq(query_selector) + end + end + end + + context 'when a modifiers document is provided in the options' do + + let(:selector) do + { a: 1 } + end + + let(:options) do + { :modifiers => { :$someMod => 100 } } + end + + let(:expected_modifiers) do + options[:modifiers] + end + + let(:parsed_selector) do + { a: 1 } + end + + let(:query_selector) do + BSON::Document.new(:$query => { a: 1 }, :$someMod => 100) + end + + it 'sets the modifiers' do + expect(view.instance_variable_get(:@modifiers)).to eq(expected_modifiers) + end + + it 'removes the modifiers from the selector' do + expect(view.selector).to eq(parsed_selector) + end + + it 'creates the correct query selector' do + expect(view.send(:query_spec)[:selector]).to eq(query_selector) + end + + context 'when modifiers and options are both provided' do + + let(:selector) do + { a: 1 } + end + + let(:options) do + { :sort => { a: Mongo::Index::ASCENDING }, :modifiers => { :$orderby => { a: Mongo::Index::DESCENDING } } } + end + + let(:expected_modifiers) do + { :$orderby => options[:sort] } + end + + let(:parsed_selector) do + { a: 1 } + end + + let(:query_selector) do + BSON::Document.new(:$query => selector, :$orderby => { a: Mongo::Index::ASCENDING }) + end + + it 'sets the modifiers' do + expect(view.instance_variable_get(:@modifiers)).to eq(expected_modifiers) + end + + it 'removes the modifiers from the selector' do + expect(view.selector).to eq(parsed_selector) + end + + it 'creates the correct query selector' do + expect(view.send(:query_spec)[:selector]).to eq(query_selector) + end + end + + context 'when modifiers, options and a query modifier are provided' do + + let(:selector) do + { b: 2, :$query => { a: 1 }, :$someMod => 100 } + end + + let(:options) do + { :sort => { a: Mongo::Index::ASCENDING }, :modifiers => { :$someMod => true, :$orderby => { a: Mongo::Index::DESCENDING } } } + end + + let(:expected_modifiers) do + { :$query => { a: 1 }, :$orderby => { a: Mongo::Index::ASCENDING }, :$someMod => 100 } + end + + let(:parsed_selector) do + { b: 2 } + end + + let(:query_selector) do + BSON::Document.new(:$query => { a: 1 }, :$someMod => 100, :$orderby => { a: Mongo::Index::ASCENDING }) + end + + it 'sets the modifiers' do + expect(view.instance_variable_get(:@modifiers)).to eq(expected_modifiers) + end + + it 'removes the modifiers from the selector' do + expect(view.selector).to eq(parsed_selector) + end + + it 'creates the correct query selector' do + expect(view.send(:query_spec)[:selector]).to eq(query_selector) + end + end + end + end + describe '#==' do context 'when the other object is not a collection view' do @@ -400,7 +574,6 @@ context 'when the cluster is sharded', if: sharded? do before do - allow(authorized_collection.cluster).to receive(:sharded?).and_return(true) expect(view).to receive(:special_selector).and_call_original end @@ -409,6 +582,40 @@ expect(doc).to have_key('field') end end + + context 'when there is a read preference' do + + let(:collection) do + authorized_collection.with(read: { mode: :secondary}) + end + + let(:view) do + described_class.new(collection, selector, options) + end + + let(:formatted_read_pref) do + BSON::Document.new(Mongo::ServerSelector.get(mode: :secondary).to_mongos) + end + + it 'adds the formatted read preference to the selector' do + expect(view.send(:query_spec)[:selector][:$readPreference]).to eq(formatted_read_pref) + end + end + + context 'when the read preference is primary' do + + let(:collection) do + authorized_collection.with(read: { mode: :primary}) + end + + let(:view) do + described_class.new(collection, selector, options) + end + + it 'does not add the formatted read preference to the selector' do + expect(view.send(:query_spec)[:selector][:$readPreference]).to be(nil) + end + end end context 'when a modifier document is provided' do @@ -545,14 +752,11 @@ end it 'overrides the modifier value with the option value' do - expect(query_spec[:selector][:$query]).to eq(selector) + expect(query_spec[:selector][:$query]).to eq(options[:modifiers][:$query]) end end - - end end - end context 'when there are no special fields' do diff --git a/spec/mongo/collection_spec.rb b/spec/mongo/collection_spec.rb index 79868966c7..99e9b48321 100644 --- a/spec/mongo/collection_spec.rb +++ b/spec/mongo/collection_spec.rb @@ -494,7 +494,7 @@ end it 'returns a view with :batch_size set' do - expect(view.options[:batch_size]).to be(options[:batch_size]) + expect(view.options[:batch_size]).to eq(options[:batch_size]) end end @@ -505,7 +505,7 @@ end it 'returns a view with :comment set' do - expect(view.options[:comment]).to be(options[:comment]) + expect(view.modifiers[:$comment]).to eq(options[:comment]) end end @@ -516,10 +516,12 @@ end it 'returns a view with :cursor_type set' do - expect(view.options[:cursor_type]).to be(options[:cursor_type]) + expect(view.options[:cursor_type]).to eq(options[:cursor_type]) end end + #limit + context 'when provided :max_time_ms' do let(:options) do @@ -527,7 +529,7 @@ end it 'returns a view with :max_time_ms set' do - expect(view.options[:max_time_ms]).to be(options[:max_time_ms]) + expect(view.modifiers[:$maxTimeMS]).to eq(options[:max_time_ms]) end end @@ -538,7 +540,11 @@ end it 'returns a view with modifiers set' do - expect(view.options[:modifiers]).to be(options[:modifiers]) + expect(view.modifiers).to eq(options[:modifiers]) + end + + it 'dups the modifiers hash' do + expect(view.modifiers).not_to be(options[:modifiers]) end end @@ -549,7 +555,7 @@ end it 'returns a view with :no_cursor_timeout set' do - expect(view.options[:no_cursor_timeout]).to be(options[:no_cursor_timeout]) + expect(view.options[:no_cursor_timeout]).to eq(options[:no_cursor_timeout]) end end @@ -560,7 +566,7 @@ end it 'returns a view with :oplog_replay set' do - expect(view.options[:oplog_replay]).to be(options[:oplog_replay]) + expect(view.options[:oplog_replay]).to eq(options[:oplog_replay]) end end @@ -571,7 +577,7 @@ end it 'returns a view with :projection set' do - expect(view.options[:projection]).to be(options[:projection]) + expect(view.options[:projection]).to eq(options[:projection]) end end @@ -582,7 +588,7 @@ end it 'returns a view with :skip set' do - expect(view.options[:skip]).to be(options[:skip]) + expect(view.options[:skip]).to eq(options[:skip]) end end @@ -593,7 +599,7 @@ end it 'returns a view with :sort set' do - expect(view.options[:sort]).to be(options[:sort]) + expect(view.modifiers[:$orderby]).to eq(options[:sort]) end end end