From d558d70894f0e90c20e0cb8f128a684a3980accb Mon Sep 17 00:00:00 2001
From: Emily Stolfo <emily@10gen.com>
Date: Sat, 22 Aug 2015 20:36:11 +0200
Subject: [PATCH 01/16] RUBY-1006 Refactor View and Readable to allow for
 arbtirary query modifiers

---
 lib/mongo/collection/view.rb                  |   5 +-
 lib/mongo/collection/view/immutable.rb        |  10 +-
 lib/mongo/collection/view/map_reduce.rb       |   2 +-
 lib/mongo/collection/view/readable.rb         | 102 ++++++------
 spec/mongo/collection/view/map_reduce_spec.rb |  28 ++--
 spec/mongo/collection/view_spec.rb            | 151 +++++++++++++++++-
 spec/mongo/collection_spec.rb                 |   6 +-
 7 files changed, 227 insertions(+), 77 deletions(-)

diff --git a/lib/mongo/collection/view.rb b/lib/mongo/collection/view.rb
index e48ef7e750..a77ff9e0c0 100644
--- a/lib/mongo/collection/view.rb
+++ b/lib/mongo/collection/view.rb
@@ -127,8 +127,9 @@ def hash
       def initialize(collection, selector = {}, options = {})
         validate_doc!(selector)
         @collection = collection
-        @selector = selector.dup
-        @options = options.dup
+        @modifiers = options[:modifiers] ? options[:modifiers].dup : {}
+        @selector = selector[:$query] ? parse_selector(selector) : selector.dup
+        @options = parse_options(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..cc5d8baa2d 100644
--- a/lib/mongo/collection/view/immutable.rb
+++ b/lib/mongo/collection/view/immutable.rb
@@ -22,16 +22,16 @@ 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))
         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)
           new(options.dup).tap do |view|
             view.send(:flags).push(flag)
diff --git a/lib/mongo/collection/view/map_reduce.rb b/lib/mongo/collection/view/map_reduce.rb
index 35e55f764b..906d220f18 100644
--- a/lib/mongo/collection/view/map_reduce.rb
+++ b/lib/mongo/collection/view/map_reduce.rb
@@ -159,7 +159,7 @@ 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)
           }
diff --git a/lib/mongo/collection/view/readable.rb b/lib/mongo/collection/view/readable.rb
index a6cfb45812..8f6ca12916 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
@@ -117,7 +110,7 @@ def batch_size(batch_size = nil)
         #
         # @since 2.0.0
         def comment(comment = nil)
-          configure(:comment, comment)
+          configure_modifier(:comment, 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(:hint, hint)
         end
 
         # The max number of docs to return from the query.
@@ -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(:max_scan, 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(:max_value, 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(:min_value, value)
         end
 
         # The server normally times out idle cursors after an inactivity period
@@ -315,7 +308,7 @@ def read(value = nil)
         #
         # @since 2.1.0
         def return_key(value = nil)
-          configure(:return_key, value)
+          configure_modifier(:return_key, 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(:show_disk_loc, value)
         end
 
         # The number of docs to skip before returning results.
@@ -360,7 +353,7 @@ def skip(number = nil)
         #
         # @since 2.0.0
         def snapshot(value = nil)
-          configure(:snapshot, value)
+          configure_modifier(:snapshot, 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(:sort, 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(:modifiers => 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(:max_time_ms, 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,28 @@ def parallel_scan(cursor_count)
           end
         end
 
+        def parse_options(opts)
+          opts.each.reduce({}) do |o, (k, v)|
+            if SPECIAL_FIELDS[k]
+              @modifiers[SPECIAL_FIELDS[k]] = v
+            else
+              o[k] = v
+            end
+            o
+          end
+        end
+
+        def parse_selector(sel)
+          sel.each.reduce({}) do |s, (k, v)|
+            if k[0] == '$'
+              @modifiers[k] = v
+            else
+              s[k] = v
+            end
+            s
+          end
+        end
+
         def query_options
           {
             :project => projection,
@@ -451,8 +463,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 +477,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 = { :$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/spec/mongo/collection/view/map_reduce_spec.rb b/spec/mongo/collection/view/map_reduce_spec.rb
index 4987a7a73c..e93a9c2270 100644
--- a/spec/mongo/collection/view/map_reduce_spec.rb
+++ b/spec/mongo/collection/view/map_reduce_spec.rb
@@ -400,20 +400,20 @@
     end
   end
 
-  context 'when sort is set on the view' do
-
-    let(:sort) do
-      { name: -1 }
-    end
-
-    let(:view_options) do
-      { sort: sort }
-    end
-
-    it 'includes the sort object in the operation spec' do
-      expect(map_reduce.send(:map_reduce_spec)[:selector][:sort]).to be(sort)
-    end
-  end
+  # context 'when sort is set on the view' do
+  #
+  #   let(:sort) do
+  #     { name: -1 }
+  #   end
+  #
+  #   let(:view_options) do
+  #     { sort: sort }
+  #   end
+  #
+  #   it 'includes the sort object in the operation spec' do
+  #     expect(map_reduce.send(:map_reduce_spec)[:selector][:sort]).to be(sort)
+  #   end
+  # end
 
   context 'when the collection has a read preference' do
 
diff --git a/spec/mongo/collection/view_spec.rb b/spec/mongo/collection/view_spec.rb
index 042c4f0c59..7a2a90dc1e 100644
--- a/spec/mongo/collection/view_spec.rb
+++ b/spec/mongo/collection/view_spec.rb
@@ -18,6 +18,149 @@
     authorized_collection.delete_many
   end
 
+  context 'when query modifiers are provided' do
+
+    context 'when a selector has a query modifier' do
+
+      let(:selector) do
+        { :$query => { a: 1 }, :$someMod => 100 }
+      end
+
+      let(:options) do
+        {}
+      end
+
+      let(:modifiers) do
+        { :$query => { a: 1 }, :$someMod => 100 }
+      end
+
+      let(:parsed_selector) do
+        {}
+      end
+
+      let(:query_selector) do
+        { :$query => { a: 1 }, :$someMod => 100 }
+      end
+
+      it 'sets the modifiers' do
+        expect(view.instance_variable_get(:@modifiers)).to eq(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 a modifiers document is provided in the options' do
+
+      let(:selector) do
+        { a: 1 }
+      end
+
+      let(:options) do
+        { :modifiers => { :$someMod => 100 } }
+      end
+
+      let(:modifiers) do
+        { :$someMod => 100 }
+      end
+
+      let(:parsed_selector) do
+        { a: 1 }
+      end
+
+      let(:query_selector) do
+        { :$query => { a: 1 }, :$someMod => 100 }
+      end
+
+      it 'sets the modifiers' do
+        expect(view.instance_variable_get(:@modifiers)).to eq(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(:modifiers) do
+          { :$orderby => { a: Mongo::Index::ASCENDING } }
+        end
+
+        let(:parsed_selector) do
+          { a: 1 }
+        end
+
+        let(:query_selector) do
+          { :$query => { a: 1 }, :$orderby => { a: Mongo::Index::ASCENDING } }
+        end
+
+        it 'sets the modifiers' do
+          expect(view.instance_variable_get(:@modifiers)).to eq(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, 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(:modifiers) do
+            { :$query => { a: 1 }, :$someMod => 100, :$orderby => { a: Mongo::Index::ASCENDING } }
+          end
+
+          let(:parsed_selector) do
+            { b: 2 }
+          end
+
+          let(:query_selector) do
+            { :$query => { a: 1 }, :$someMod => 100, :$orderby => { a: Mongo::Index::ASCENDING } }
+          end
+
+          it 'sets the modifiers' do
+            expect(view.instance_variable_get(:@modifiers)).to eq(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
+  end
+
   describe '#==' do
 
     context 'when the other object is not a collection view' do
@@ -544,12 +687,10 @@
               }
             end
 
-            it 'overrides the modifier value with the option value' do
-              expect(query_spec[:selector][:$query]).to eq(selector)
-            end
+            # it 'overrides the modifier value with the option value' do
+            #   expect(query_spec[:selector][:$query]).to eq(selector)
+            # end
           end
-
-
         end
       end
 
diff --git a/spec/mongo/collection_spec.rb b/spec/mongo/collection_spec.rb
index 79868966c7..18bdc72f19 100644
--- a/spec/mongo/collection_spec.rb
+++ b/spec/mongo/collection_spec.rb
@@ -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 be(options[:comment])
         end
       end
 
@@ -527,7 +527,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 be(options[:max_time_ms])
         end
       end
 
@@ -593,7 +593,7 @@
         end
 
         it 'returns a view with :sort set' do
-          expect(view.options[:sort]).to be(options[:sort])
+          expect(view.modifiers[:$orderby]).to be(options[:sort])
         end
       end
     end

From 162cabc00402afabd8fa3c6bfce6c01848b61e02 Mon Sep 17 00:00:00 2001
From: Emily Stolfo <emily@10gen.com>
Date: Sat, 22 Aug 2015 21:31:53 +0200
Subject: [PATCH 02/16] RUBY-1006 Use  in modifiers if present

---
 spec/mongo/collection/view_spec.rb | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/spec/mongo/collection/view_spec.rb b/spec/mongo/collection/view_spec.rb
index 7a2a90dc1e..d02b3dac34 100644
--- a/spec/mongo/collection/view_spec.rb
+++ b/spec/mongo/collection/view_spec.rb
@@ -687,9 +687,9 @@
               }
             end
 
-            # it 'overrides the modifier value with the option value' do
-            #   expect(query_spec[:selector][:$query]).to eq(selector)
-            # end
+            it 'overrides the modifier value with the option value' do
+              expect(query_spec[:selector][:$query]).to eq(options[:modifiers][:$query])
+            end
           end
         end
       end

From 936f6915fed688a8ea4830c4f312a516dc49136f Mon Sep 17 00:00:00 2001
From: Emily Stolfo <emily@10gen.com>
Date: Sat, 22 Aug 2015 23:18:45 +0200
Subject: [PATCH 03/16] RUBY-1006 extra tests for immutability with modifiers

---
 spec/mongo/collection/view/immutable_spec.rb | 69 ++++++++++++++++++++
 spec/mongo/collection/view_spec.rb           | 36 +++++++++-
 2 files changed, 104 insertions(+), 1 deletion(-)
 create mode 100644 spec/mongo/collection/view/immutable_spec.rb

diff --git a/spec/mongo/collection/view/immutable_spec.rb b/spec/mongo/collection/view/immutable_spec.rb
new file mode 100644
index 0000000000..3b631c0057
--- /dev/null
+++ b/spec/mongo/collection/view/immutable_spec.rb
@@ -0,0 +1,69 @@
+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_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 does have 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_spec.rb b/spec/mongo/collection/view_spec.rb
index d02b3dac34..df0604e6ab 100644
--- a/spec/mongo/collection/view_spec.rb
+++ b/spec/mongo/collection/view_spec.rb
@@ -540,7 +540,7 @@
         end
       end
 
-      context 'when the cluster is sharded', if: sharded? do
+      context 'when the cluster is sharded' do
 
         before do
           allow(authorized_collection.cluster).to receive(:sharded?).and_return(true)
@@ -552,6 +552,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
+            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

From e313940938e217b3e55e0f313db39af1dba6eef6 Mon Sep 17 00:00:00 2001
From: Emily Stolfo <emily@10gen.com>
Date: Sat, 22 Aug 2015 23:29:14 +0200
Subject: [PATCH 04/16] RUBY-1006 Account for the view setting sort in the
 modifiers document when doing mp

---
 lib/mongo/collection/view/map_reduce.rb       |  6 +++-
 spec/mongo/collection/view/map_reduce_spec.rb | 28 +++++++++----------
 2 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/lib/mongo/collection/view/map_reduce.rb b/lib/mongo/collection/view/map_reduce.rb
index 906d220f18..bee1d20caf 100644
--- a/lib/mongo/collection/view/map_reduce.rb
+++ b/lib/mongo/collection/view/map_reduce.rb
@@ -161,10 +161,14 @@ def map_reduce_spec
               :reduce => reduce,
               :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/spec/mongo/collection/view/map_reduce_spec.rb b/spec/mongo/collection/view/map_reduce_spec.rb
index e93a9c2270..4987a7a73c 100644
--- a/spec/mongo/collection/view/map_reduce_spec.rb
+++ b/spec/mongo/collection/view/map_reduce_spec.rb
@@ -400,20 +400,20 @@
     end
   end
 
-  # context 'when sort is set on the view' do
-  #
-  #   let(:sort) do
-  #     { name: -1 }
-  #   end
-  #
-  #   let(:view_options) do
-  #     { sort: sort }
-  #   end
-  #
-  #   it 'includes the sort object in the operation spec' do
-  #     expect(map_reduce.send(:map_reduce_spec)[:selector][:sort]).to be(sort)
-  #   end
-  # end
+  context 'when sort is set on the view' do
+
+    let(:sort) do
+      { name: -1 }
+    end
+
+    let(:view_options) do
+      { sort: sort }
+    end
+
+    it 'includes the sort object in the operation spec' do
+      expect(map_reduce.send(:map_reduce_spec)[:selector][:sort]).to be(sort)
+    end
+  end
 
   context 'when the collection has a read preference' do
 

From 314e6445b4971abb88b7234cc95d6172c3b07d27 Mon Sep 17 00:00:00 2001
From: Emily Stolfo <emily@10gen.com>
Date: Sun, 23 Aug 2015 08:46:07 +0200
Subject: [PATCH 05/16] RUBY-1006 Simpler methods for setting up view options

---
 lib/mongo/collection/view.rb          |  6 +++---
 lib/mongo/collection/view/readable.rb | 30 ++++++++++++---------------
 spec/mongo/collection_spec.rb         | 26 ++++++++++++++---------
 3 files changed, 32 insertions(+), 30 deletions(-)

diff --git a/lib/mongo/collection/view.rb b/lib/mongo/collection/view.rb
index a77ff9e0c0..b45f21903a 100644
--- a/lib/mongo/collection/view.rb
+++ b/lib/mongo/collection/view.rb
@@ -127,9 +127,9 @@ def hash
       def initialize(collection, selector = {}, options = {})
         validate_doc!(selector)
         @collection = collection
-        @modifiers = options[:modifiers] ? options[:modifiers].dup : {}
-        @selector = selector[:$query] ? parse_selector(selector) : selector.dup
-        @options = parse_options(options)
+        @selector = selector.dup
+        @options = options.dup
+        setup
       end
 
       # Get a human-readable string representation of +View+.
diff --git a/lib/mongo/collection/view/readable.rb b/lib/mongo/collection/view/readable.rb
index 8f6ca12916..1eaedc2426 100644
--- a/lib/mongo/collection/view/readable.rb
+++ b/lib/mongo/collection/view/readable.rb
@@ -431,26 +431,22 @@ def parallel_scan(cursor_count)
           end
         end
 
-        def parse_options(opts)
-          opts.each.reduce({}) do |o, (k, v)|
-            if SPECIAL_FIELDS[k]
-              @modifiers[SPECIAL_FIELDS[k]] = v
-            else
-              o[k] = v
-            end
-            o
-          end
+        def setup
+          setup_options
+          setup_selector
         end
 
-        def parse_selector(sel)
-          sel.each.reduce({}) do |s, (k, v)|
-            if k[0] == '$'
-              @modifiers[k] = v
-            else
-              s[k] = v
-            end
-            s
+        def setup_options
+          @modifiers = @options[:modifiers] ? @options.delete(:modifiers).dup : {}
+          @options.keys.each { |k| @modifiers.merge!(SPECIAL_FIELDS[k] => @options.delete(k)) if SPECIAL_FIELDS[k] }
+          @options.freeze
+        end
+
+        def setup_selector
+          if @selector[:$query]
+            @selector.keys.each { |k| @modifiers.merge!(k => @selector.delete(k)) if k[0] == '$' }
           end
+          @selector.freeze
         end
 
         def query_options
diff --git a/spec/mongo/collection_spec.rb b/spec/mongo/collection_spec.rb
index 18bdc72f19..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.modifiers[:$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.modifiers[:$maxTimeMS]).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.modifiers[:$orderby]).to be(options[:sort])
+          expect(view.modifiers[:$orderby]).to eq(options[:sort])
         end
       end
     end

From fb3430be5d9c0108739e89b728222c930219e2fc Mon Sep 17 00:00:00 2001
From: Emily Stolfo <emily@10gen.com>
Date: Sun, 23 Aug 2015 09:41:02 +0200
Subject: [PATCH 06/16] RUBY-1006 Don't loose the modifiers document when
 setting a new option

---
 lib/mongo/collection/view/immutable.rb       |  2 +-
 spec/mongo/collection/view/immutable_spec.rb | 36 +++++++++++++++++++-
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/lib/mongo/collection/view/immutable.rb b/lib/mongo/collection/view/immutable.rb
index cc5d8baa2d..3d64729f7a 100644
--- a/lib/mongo/collection/view/immutable.rb
+++ b/lib/mongo/collection/view/immutable.rb
@@ -24,7 +24,7 @@ module Immutable
 
         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)
diff --git a/spec/mongo/collection/view/immutable_spec.rb b/spec/mongo/collection/view/immutable_spec.rb
index 3b631c0057..a203e5a1c8 100644
--- a/spec/mongo/collection/view/immutable_spec.rb
+++ b/spec/mongo/collection/view/immutable_spec.rb
@@ -18,6 +18,40 @@
     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
@@ -35,7 +69,7 @@
       end
     end
 
-    context 'when the options does have a modifiers document' do
+    context 'when the options has a modifiers document' do
 
       let(:options) do
         { modifiers: { :$maxTimeMS => 500 } }

From 1a5fffb58ffedb8243ff577d3b9d1210ad4d5ec3 Mon Sep 17 00:00:00 2001
From: Emily Stolfo <emily@10gen.com>
Date: Sun, 23 Aug 2015 09:45:36 +0200
Subject: [PATCH 07/16] RUBY-1006 Don't clone the view a million times by using
 the fluent API

---
 lib/mongo/grid/fs_bucket.rb   | 12 +++++++++---
 lib/mongo/grid/stream/read.rb |  4 ++--
 2 files changed, 11 insertions(+), 5 deletions(-)

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
 

From 641ccdfeb3e9907633d9b590e178a50e238c813e Mon Sep 17 00:00:00 2001
From: Emily Stolfo <emily@10gen.com>
Date: Sun, 23 Aug 2015 09:57:19 +0200
Subject: [PATCH 08/16] RUBY-1006 Dup options and selectors in setup methods

---
 lib/mongo/collection/view.rb          |  2 --
 lib/mongo/collection/view/readable.rb | 10 ++++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/mongo/collection/view.rb b/lib/mongo/collection/view.rb
index b45f21903a..87d8b324ba 100644
--- a/lib/mongo/collection/view.rb
+++ b/lib/mongo/collection/view.rb
@@ -127,8 +127,6 @@ def hash
       def initialize(collection, selector = {}, options = {})
         validate_doc!(selector)
         @collection = collection
-        @selector = selector.dup
-        @options = options.dup
         setup
       end
 
diff --git a/lib/mongo/collection/view/readable.rb b/lib/mongo/collection/view/readable.rb
index 1eaedc2426..8f9f2a62ba 100644
--- a/lib/mongo/collection/view/readable.rb
+++ b/lib/mongo/collection/view/readable.rb
@@ -432,17 +432,19 @@ def parallel_scan(cursor_count)
         end
 
         def setup
-          setup_options
-          setup_selector
+          setup_options(options)
+          setup_selector(selector)
         end
 
-        def setup_options
+        def setup_options(opts)
+          @options = opts.dup
           @modifiers = @options[:modifiers] ? @options.delete(:modifiers).dup : {}
           @options.keys.each { |k| @modifiers.merge!(SPECIAL_FIELDS[k] => @options.delete(k)) if SPECIAL_FIELDS[k] }
           @options.freeze
         end
 
-        def setup_selector
+        def setup_selector(sel)
+          @selector = sel.dup
           if @selector[:$query]
             @selector.keys.each { |k| @modifiers.merge!(k => @selector.delete(k)) if k[0] == '$' }
           end

From 0f36650ed230658bdae2bda57d85dac6e58c2408 Mon Sep 17 00:00:00 2001
From: Emily Stolfo <emily@10gen.com>
Date: Sun, 23 Aug 2015 10:18:32 +0200
Subject: [PATCH 09/16] RUBY-1006 Account for when nil is passed in for a
 selector or options

---
 lib/mongo/collection/view.rb          |  2 +-
 lib/mongo/collection/view/readable.rb | 10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/mongo/collection/view.rb b/lib/mongo/collection/view.rb
index 87d8b324ba..cc15686ecf 100644
--- a/lib/mongo/collection/view.rb
+++ b/lib/mongo/collection/view.rb
@@ -127,7 +127,7 @@ def hash
       def initialize(collection, selector = {}, options = {})
         validate_doc!(selector)
         @collection = collection
-        setup
+        setup(selector, options)
       end
 
       # Get a human-readable string representation of +View+.
diff --git a/lib/mongo/collection/view/readable.rb b/lib/mongo/collection/view/readable.rb
index 8f9f2a62ba..fe3b693afa 100644
--- a/lib/mongo/collection/view/readable.rb
+++ b/lib/mongo/collection/view/readable.rb
@@ -431,20 +431,20 @@ def parallel_scan(cursor_count)
           end
         end
 
-        def setup
-          setup_options(options)
-          setup_selector(selector)
+        def setup(sel, opts)
+          setup_options(opts)
+          setup_selector(sel)
         end
 
         def setup_options(opts)
-          @options = opts.dup
+          @options = opts ? opts.dup : {}
           @modifiers = @options[:modifiers] ? @options.delete(:modifiers).dup : {}
           @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.dup
+          @selector = sel ? sel.dup : {}
           if @selector[:$query]
             @selector.keys.each { |k| @modifiers.merge!(k => @selector.delete(k)) if k[0] == '$' }
           end

From 6ebc1e65a4a9ae22a8e935cce160557ee8591d12 Mon Sep 17 00:00:00 2001
From: Emily Stolfo <emily@10gen.com>
Date: Mon, 24 Aug 2015 12:00:59 +0200
Subject: [PATCH 10/16] RUBY-1006 Use BSON::Document for  selector

---
 lib/mongo/collection/view/readable.rb |  2 +-
 lib/mongo/protocol/query.rb           |  2 +-
 spec/mongo/collection/view_spec.rb    | 10 +++++-----
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/mongo/collection/view/readable.rb b/lib/mongo/collection/view/readable.rb
index fe3b693afa..324752c667 100644
--- a/lib/mongo/collection/view/readable.rb
+++ b/lib/mongo/collection/view/readable.rb
@@ -479,7 +479,7 @@ def read_pref_formatted
         end
 
         def special_selector
-          sel = { :$query => selector }.merge(modifiers)
+          sel = BSON::Document.new(:$query => selector).merge!(modifiers)
           sel[:$readPreference] = read_pref_formatted unless read_pref_formatted.nil?
           sel
         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_spec.rb b/spec/mongo/collection/view_spec.rb
index df0604e6ab..5c9e9dd24f 100644
--- a/spec/mongo/collection/view_spec.rb
+++ b/spec/mongo/collection/view_spec.rb
@@ -39,7 +39,7 @@
       end
 
       let(:query_selector) do
-        { :$query => { a: 1 }, :$someMod => 100 }
+        BSON::Document.new(:$query => { a: 1 }, :$someMod => 100)
       end
 
       it 'sets the modifiers' do
@@ -74,7 +74,7 @@
       end
 
       let(:query_selector) do
-        { :$query => { a: 1 }, :$someMod => 100 }
+        BSON::Document.new(:$query => { a: 1 }, :$someMod => 100)
       end
 
       it 'sets the modifiers' do
@@ -108,7 +108,7 @@
         end
 
         let(:query_selector) do
-          { :$query => { a: 1 }, :$orderby => { a: Mongo::Index::ASCENDING } }
+          BSON::Document.new(:$query => { a: 1 }, :$orderby => { a: Mongo::Index::ASCENDING })
         end
 
         it 'sets the modifiers' do
@@ -142,7 +142,7 @@
           end
 
           let(:query_selector) do
-            { :$query => { a: 1 }, :$someMod => 100, :$orderby => { a: Mongo::Index::ASCENDING } }
+            BSON::Document.new(:$query => { a: 1 }, :$someMod => 100, :$orderby => { a: Mongo::Index::ASCENDING })
           end
 
           it 'sets the modifiers' do
@@ -564,7 +564,7 @@
           end
 
           let(:formatted_read_pref) do
-            Mongo::ServerSelector.get(mode: :secondary).to_mongos
+            BSON::Document.new(Mongo::ServerSelector.get(mode: :secondary).to_mongos)
           end
 
           it 'adds the formatted read preference to the selector' do

From c0ff8357a828a885bd799ba53980ead5c00327b1 Mon Sep 17 00:00:00 2001
From: Emily Stolfo <emily@10gen.com>
Date: Mon, 24 Aug 2015 15:50:57 +0200
Subject: [PATCH 11/16] RUBY-1006 Make modifiers a BSON::Document as well

---
 lib/mongo/collection/view/readable.rb         |   4 +-
 spec/mongo/collection/view/map_reduce_spec.rb |   4 +-
 spec/mongo/collection/view/readable_spec.rb   |   8 +-
 spec/mongo/collection/view_spec.rb            | 115 +++++++++++-------
 spec/mongo/collection_spec.rb                 |   2 +-
 5 files changed, 78 insertions(+), 55 deletions(-)

diff --git a/lib/mongo/collection/view/readable.rb b/lib/mongo/collection/view/readable.rb
index 324752c667..940c6b35cf 100644
--- a/lib/mongo/collection/view/readable.rb
+++ b/lib/mongo/collection/view/readable.rb
@@ -438,14 +438,14 @@ def setup(sel, opts)
 
         def setup_options(opts)
           @options = opts ? opts.dup : {}
-          @modifiers = @options[:modifiers] ? @options.delete(:modifiers).dup : {}
+          @modifiers = @options[:modifiers] ? BSON::Document.new(@options.delete(:modifiers)) : 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]
+          if @selector[:$query] || @selector["$query"]
             @selector.keys.each { |k| @modifiers.merge!(k => @selector.delete(k)) if k[0] == '$' }
           end
           @selector.freeze
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..4940b63d35 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
 
@@ -638,7 +638,7 @@
     context 'when a modifiers document is specified' do
 
       let(:new_modifiers) do
-        { :modifiers => { :$orderby => Mongo::Index::DESCENDING } }
+        BSON::Document.new(:modifiers => { :$orderby => Mongo::Index::DESCENDING })
       end
 
       it 'sets the new_modifiers document' do
@@ -654,7 +654,7 @@
     context 'when a modifiers document is not specified' do
 
       it 'returns the modifiers value' do
-        expect(view.modifiers).to eq(options[:modifiers])
+        expect(view.modifiers).to eq(BSON::Document.new(options[:modifiers]))
       end
     end
   end
diff --git a/spec/mongo/collection/view_spec.rb b/spec/mongo/collection/view_spec.rb
index 5c9e9dd24f..cb482fa4d0 100644
--- a/spec/mongo/collection/view_spec.rb
+++ b/spec/mongo/collection/view_spec.rb
@@ -22,16 +22,12 @@
 
     context 'when a selector has a query modifier' do
 
-      let(:selector) do
-        { :$query => { a: 1 }, :$someMod => 100 }
-      end
-
       let(:options) do
         {}
       end
 
-      let(:modifiers) do
-        { :$query => { a: 1 }, :$someMod => 100 }
+      let(:expected_modifiers) do
+        BSON::Document.new(selector)
       end
 
       let(:parsed_selector) do
@@ -39,19 +35,46 @@
       end
 
       let(:query_selector) do
-        BSON::Document.new(:$query => { a: 1 }, :$someMod => 100)
+        BSON::Document.new(selector)
       end
 
-      it 'sets the modifiers' do
-        expect(view.instance_variable_get(:@modifiers)).to eq(modifiers)
-      end
+      context 'when the $query key is a string' do
+
+        let(:selector) do
+          { "$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
 
-      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)
+      context 'when the $query key is a symbol' do
+
+        let(:selector) do
+          { :$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
       end
     end
 
@@ -65,8 +88,8 @@
         { :modifiers => { :$someMod => 100 } }
       end
 
-      let(:modifiers) do
-        { :$someMod => 100 }
+      let(:expected_modifiers) do
+        BSON::Document.new(options[:modifiers])
       end
 
       let(:parsed_selector) do
@@ -78,7 +101,7 @@
       end
 
       it 'sets the modifiers' do
-        expect(view.instance_variable_get(:@modifiers)).to eq(modifiers)
+        expect(view.instance_variable_get(:@modifiers)).to eq(expected_modifiers)
       end
 
       it 'removes the modifiers from the selector' do
@@ -99,8 +122,8 @@
           { :sort =>  { a: Mongo::Index::ASCENDING }, :modifiers => { :$orderby => { a: Mongo::Index::DESCENDING } } }
         end
 
-        let(:modifiers) do
-          { :$orderby => { a: Mongo::Index::ASCENDING } }
+        let(:expected_modifiers) do
+          BSON::Document.new(:$orderby => options[:sort])
         end
 
         let(:parsed_selector) do
@@ -108,11 +131,11 @@
         end
 
         let(:query_selector) do
-          BSON::Document.new(:$query => { a: 1 }, :$orderby => { a: Mongo::Index::ASCENDING })
+          BSON::Document.new(:$query => selector, :$orderby => { a: Mongo::Index::ASCENDING })
         end
 
         it 'sets the modifiers' do
-          expect(view.instance_variable_get(:@modifiers)).to eq(modifiers)
+          expect(view.instance_variable_get(:@modifiers)).to eq(expected_modifiers)
         end
 
         it 'removes the modifiers from the selector' do
@@ -122,40 +145,40 @@
         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
+      context 'when modifiers, options and a query modifier are provided' do
 
-          let(:selector) do
-            { b: 2, :$query => { a: 1 }, :$someMod => 100 }
-          end
+        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(:options) do
+          { :sort =>  { a: Mongo::Index::ASCENDING }, :modifiers => { :$someMod => true, :$orderby => { a: Mongo::Index::DESCENDING } } }
+        end
 
-          let(:modifiers) do
-            { :$query => { a: 1 }, :$someMod => 100, :$orderby => { a: Mongo::Index::ASCENDING } }
-          end
+        let(:expected_modifiers) do
+          BSON::Document.new(:$query => { a: 1 }, :$orderby => { a: Mongo::Index::ASCENDING }, :$someMod => 100)
+        end
 
-          let(:parsed_selector) do
-            { b: 2 }
-          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
+        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(modifiers)
-          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 '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
+        it 'creates the correct query selector' do
+          expect(view.send(:query_spec)[:selector]).to eq(query_selector)
         end
       end
     end
diff --git a/spec/mongo/collection_spec.rb b/spec/mongo/collection_spec.rb
index 99e9b48321..36d2912ad6 100644
--- a/spec/mongo/collection_spec.rb
+++ b/spec/mongo/collection_spec.rb
@@ -540,7 +540,7 @@
         end
 
         it 'returns a view with modifiers set' do
-          expect(view.modifiers).to eq(options[:modifiers])
+          expect(view.modifiers).to eq(BSON::Document.new(options[:modifiers]))
         end
 
         it 'dups the modifiers hash' do

From 9ec2266a079f13a6f520a6481d0f7aaa2122e243 Mon Sep 17 00:00:00 2001
From: Emily Stolfo <emily@10gen.com>
Date: Mon, 24 Aug 2015 15:59:13 +0200
Subject: [PATCH 12/16] RUBY-1006 Don't rely on mocking to test sending a query
 to a sharded cluster

---
 spec/mongo/collection/view_spec.rb | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/spec/mongo/collection/view_spec.rb b/spec/mongo/collection/view_spec.rb
index cb482fa4d0..b66d9ef807 100644
--- a/spec/mongo/collection/view_spec.rb
+++ b/spec/mongo/collection/view_spec.rb
@@ -563,10 +563,9 @@
         end
       end
 
-      context 'when the cluster is sharded' do
+      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
 

From cf08e2a22bd48b83e22af6dcf1d9adc3a6d72201 Mon Sep 17 00:00:00 2001
From: Emily Stolfo <emily@10gen.com>
Date: Mon, 24 Aug 2015 16:04:01 +0200
Subject: [PATCH 13/16] RUBY-1006 Extra space and quote usage

---
 lib/mongo/collection/view/readable.rb | 2 +-
 spec/mongo/collection/view_spec.rb    | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/mongo/collection/view/readable.rb b/lib/mongo/collection/view/readable.rb
index 940c6b35cf..005570b1cf 100644
--- a/lib/mongo/collection/view/readable.rb
+++ b/lib/mongo/collection/view/readable.rb
@@ -445,7 +445,7 @@ def setup_options(opts)
 
         def setup_selector(sel)
           @selector = sel ? sel.dup : {}
-          if @selector[:$query] || @selector["$query"]
+          if @selector[:$query] || @selector['$query']
             @selector.keys.each { |k| @modifiers.merge!(k => @selector.delete(k)) if k[0] == '$' }
           end
           @selector.freeze
diff --git a/spec/mongo/collection/view_spec.rb b/spec/mongo/collection/view_spec.rb
index b66d9ef807..4867f6da0d 100644
--- a/spec/mongo/collection/view_spec.rb
+++ b/spec/mongo/collection/view_spec.rb
@@ -749,7 +749,6 @@
           end
         end
       end
-
     end
 
     context 'when there are no special fields' do

From 58dc44d36b7cea904daa6c4001c426c4525ea64f Mon Sep 17 00:00:00 2001
From: Emily Stolfo <emily@10gen.com>
Date: Mon, 24 Aug 2015 16:28:11 +0200
Subject: [PATCH 14/16] RUBY-1006 Only initialize modifiers to BSON::Document,
 don't convert

---
 lib/mongo/collection/view/readable.rb       |  2 +-
 spec/mongo/collection/view/readable_spec.rb |  4 ++--
 spec/mongo/collection/view_spec.rb          | 14 +++++++++++---
 spec/mongo/collection_spec.rb               |  2 +-
 4 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/lib/mongo/collection/view/readable.rb b/lib/mongo/collection/view/readable.rb
index 005570b1cf..9356325818 100644
--- a/lib/mongo/collection/view/readable.rb
+++ b/lib/mongo/collection/view/readable.rb
@@ -438,7 +438,7 @@ def setup(sel, opts)
 
         def setup_options(opts)
           @options = opts ? opts.dup : {}
-          @modifiers = @options[:modifiers] ? BSON::Document.new(@options.delete(:modifiers)) : BSON::Document.new
+          @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
diff --git a/spec/mongo/collection/view/readable_spec.rb b/spec/mongo/collection/view/readable_spec.rb
index 4940b63d35..9d46ba1818 100644
--- a/spec/mongo/collection/view/readable_spec.rb
+++ b/spec/mongo/collection/view/readable_spec.rb
@@ -638,7 +638,7 @@
     context 'when a modifiers document is specified' do
 
       let(:new_modifiers) do
-        BSON::Document.new(:modifiers => { :$orderby => Mongo::Index::DESCENDING })
+        { :modifiers => { :$orderby => Mongo::Index::DESCENDING } }
       end
 
       it 'sets the new_modifiers document' do
@@ -654,7 +654,7 @@
     context 'when a modifiers document is not specified' do
 
       it 'returns the modifiers value' do
-        expect(view.modifiers).to eq(BSON::Document.new(options[:modifiers]))
+        expect(view.modifiers).to eq(options[:modifiers])
       end
     end
   end
diff --git a/spec/mongo/collection/view_spec.rb b/spec/mongo/collection/view_spec.rb
index 4867f6da0d..f17242b2f7 100644
--- a/spec/mongo/collection/view_spec.rb
+++ b/spec/mongo/collection/view_spec.rb
@@ -44,6 +44,10 @@
           { "$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
@@ -64,6 +68,10 @@
           { :$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
@@ -89,7 +97,7 @@
       end
 
       let(:expected_modifiers) do
-        BSON::Document.new(options[:modifiers])
+        options[:modifiers]
       end
 
       let(:parsed_selector) do
@@ -123,7 +131,7 @@
         end
 
         let(:expected_modifiers) do
-          BSON::Document.new(:$orderby => options[:sort])
+          { :$orderby => options[:sort] }
         end
 
         let(:parsed_selector) do
@@ -158,7 +166,7 @@
         end
 
         let(:expected_modifiers) do
-          BSON::Document.new(:$query => { a: 1 }, :$orderby => { a: Mongo::Index::ASCENDING }, :$someMod => 100)
+          { :$query => { a: 1 }, :$orderby => { a: Mongo::Index::ASCENDING }, :$someMod => 100 }
         end
 
         let(:parsed_selector) do
diff --git a/spec/mongo/collection_spec.rb b/spec/mongo/collection_spec.rb
index 36d2912ad6..99e9b48321 100644
--- a/spec/mongo/collection_spec.rb
+++ b/spec/mongo/collection_spec.rb
@@ -540,7 +540,7 @@
         end
 
         it 'returns a view with modifiers set' do
-          expect(view.modifiers).to eq(BSON::Document.new(options[:modifiers]))
+          expect(view.modifiers).to eq(options[:modifiers])
         end
 
         it 'dups the modifiers hash' do

From 3a2d40f1161ff3e776a504837a10561032412c92 Mon Sep 17 00:00:00 2001
From: Emily Stolfo <emily@10gen.com>
Date: Mon, 24 Aug 2015 16:46:11 +0200
Subject: [PATCH 15/16] RUBY-1006 use __method__ for consistency

---
 lib/mongo/collection/view/readable.rb | 34 +++++++++++++--------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/lib/mongo/collection/view/readable.rb b/lib/mongo/collection/view/readable.rb
index 9356325818..9d72918984 100644
--- a/lib/mongo/collection/view/readable.rb
+++ b/lib/mongo/collection/view/readable.rb
@@ -92,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.
@@ -110,7 +110,7 @@ def batch_size(batch_size = nil)
         #
         # @since 2.0.0
         def comment(comment = nil)
-          configure_modifier(:comment, comment)
+          configure_modifier(__method__, comment)
         end
 
         # Get a count of matching documents in the collection.
@@ -174,7 +174,7 @@ def distinct(field_name, options={})
         #
         # @since 2.0.0
         def hint(hint = nil)
-          configure_modifier(:hint, hint)
+          configure_modifier(__method__, hint)
         end
 
         # The max number of docs to return from the query.
@@ -188,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.
@@ -218,7 +218,7 @@ def map_reduce(map, reduce, options = {})
         #
         # @since 2.0.0
         def max_scan(value = nil)
-          configure_modifier(:max_scan, value)
+          configure_modifier(__method__, value)
         end
 
         # Set the maximum value to search.
@@ -232,7 +232,7 @@ def max_scan(value = nil)
         #
         # @since 2.1.0
         def max_value(value = nil)
-          configure_modifier(:max_value, value)
+          configure_modifier(__method__, value)
         end
 
         # Set the minimum value to search.
@@ -246,7 +246,7 @@ def max_value(value = nil)
         #
         # @since 2.1.0
         def min_value(value = nil)
-          configure_modifier(:min_value, value)
+          configure_modifier(__method__, value)
         end
 
         # The server normally times out idle cursors after an inactivity period
@@ -259,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.
@@ -278,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.
@@ -294,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.
@@ -308,7 +308,7 @@ def read(value = nil)
         #
         # @since 2.1.0
         def return_key(value = nil)
-          configure_modifier(:return_key, value)
+          configure_modifier(__method__, value)
         end
 
         # Set whether the disk location should be shown for each document.
@@ -323,7 +323,7 @@ def return_key(value = nil)
         #
         # @since 2.0.0
         def show_disk_loc(value = nil)
-          configure_modifier(:show_disk_loc, value)
+          configure_modifier(__method__, value)
         end
 
         # The number of docs to skip before returning results.
@@ -338,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.
@@ -353,7 +353,7 @@ def skip(number = nil)
         #
         # @since 2.0.0
         def snapshot(value = nil)
-          configure_modifier(:snapshot, value)
+          configure_modifier(__method__, value)
         end
 
         # The key and direction pairs by which the result set will be sorted.
@@ -368,7 +368,7 @@ def snapshot(value = nil)
         #
         # @since 2.0.0
         def sort(spec = nil)
-          configure_modifier(:sort, spec)
+          configure_modifier(__method__, spec)
         end
 
         # “meta” operators that let you modify the output or behavior of a query.
@@ -383,7 +383,7 @@ def sort(spec = nil)
         # @since 2.1.0
         def modifiers(doc = nil)
           return @modifiers if doc.nil?
-          new(options.merge(:modifiers => doc))
+          new(options.merge(__method__ => doc))
         end
 
         # A cumulative time limit in milliseconds for processing operations on a cursor.
@@ -397,7 +397,7 @@ def modifiers(doc = nil)
         #
         # @since 2.1.0
         def max_time_ms(max = nil)
-          configure_modifier(:max_time_ms, max)
+          configure_modifier(__method__, max)
         end
 
         private

From 2df4d9a62cb454ffe95d1a7da5e29b319e49e4f4 Mon Sep 17 00:00:00 2001
From: Emily Stolfo <emily@10gen.com>
Date: Mon, 24 Aug 2015 16:46:37 +0200
Subject: [PATCH 16/16] RUBY-1006 Freeze modifiers hash

---
 lib/mongo/collection/view/readable.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/mongo/collection/view/readable.rb b/lib/mongo/collection/view/readable.rb
index 9d72918984..c1be99ffd6 100644
--- a/lib/mongo/collection/view/readable.rb
+++ b/lib/mongo/collection/view/readable.rb
@@ -448,6 +448,7 @@ def setup_selector(sel)
           if @selector[:$query] || @selector['$query']
             @selector.keys.each { |k| @modifiers.merge!(k => @selector.delete(k)) if k[0] == '$' }
           end
+          @modifiers.freeze
           @selector.freeze
         end