RUBY-2220 Move find command construction code into operations layer

 (#2275)

* RUBY-2220 Move find command construction code into operations layer

* fix explain test on old sharded cluster servers

* remove read preference

* 2706 again

Co-authored-by: Oleg Pudeyev <code@olegp.name>

Author: p-mongo

docs/reference/crud-operations.txt

@@ -260,6 +260,32 @@ when querying and their corresponding methods as examples.
          # MongoDB 2.6
          client[:artists].find.explain(verbose: true)
 
+       The explain operation supports ``:session`` and ``:read``
+       (for read preference) options. To specify these options for a single
+       explain operation, they must be given to the ``find`` method as
+       follows:
+     
+       .. code-block:: ruby
+
+         client[:artists].find({}, session: session).explain
+
+         client[:artists].find({}, read: {mode: :secondary_preferred}).explain
+       
+       If the read preference option is specified on the client or on the
+       collection, it will be passed to the explain operation:
+     
+       .. code-block:: ruby
+
+         client[:artists, read: {mode: :secondary_preferred}].find.explain
+       
+       Note that the session option is not accepted when creating a collection
+       object.
+       
+       The explain command does not support passing the read concern option.
+       If the read concern is specifed on the client or collection level, or
+       if the read concern is specified as a find option, it will NOT be passed
+       by the driver to the explain command.
+       
        .. note::
 
          The information returned by the server for the ``explain`` command

lib/mongo/collection.rb

@@ -237,7 +237,7 @@ def create(opts = {})
       # TODO put the list of read options in a class-level constant when
       # we figure out what the full set of them is.
       options = Hash[self.options.reject do |key, value|
-        %w(read read_preference).include?(key.to_s)
+        %w(read read_preference read_concern).include?(key.to_s)
       end]
       operation = { :create => name }.merge(options)
       operation.delete(:write)
@@ -253,12 +253,12 @@ def create(opts = {})
           raise Error::UnsupportedCollation
         end
 
-        Operation::Create.new({
-                                selector: operation,
-                                db_name: database.name,
-                                write_concern: write_concern,
-                                session: session,
-                                }).execute(server, context: Operation::Context.new(client: client, session: session))
+        Operation::Create.new(
+          selector: operation,
+          db_name: database.name,
+          write_concern: write_concern,
+          session: session,
+        ).execute(server, context: Operation::Context.new(client: client, session: session))
       end
     end
 
@@ -332,8 +332,8 @@ def drop(opts = {})
     # @option options [ true, false ] :no_cursor_timeout The server normally times out idle
     #   cursors after an inactivity period (10 minutes) to prevent excess memory use.
     #   Set this option to prevent that.
-    # @option options [ true, false ] :oplog_replay Internal replication use only - driver
-    #   should not set.
+    # @option options [ true, false ] :oplog_replay For internal replication
+    #   use only, applications should not set this option.
     # @option options [ Hash ] :projection The fields to include or exclude from each doc
     #   in the result set.
     # @option options [ Session ] :session The session to use.

lib/mongo/collection/view.rb

@@ -141,6 +141,8 @@ def hash
       # @option options [ Hash ] :read The read preference to use for the
       #   query. If none is provided, the collection's default read preference
       #   is used.
+      # @option options [ Hash ] :read_concern The read concern to use for
+      #   the query.
       # @option options [ true | false ] :show_disk_loc Return disk location
       #   info as a field in each doc.
       # @option options [ Integer ] :skip The number of documents to skip.
@@ -153,7 +155,19 @@ def hash
       def initialize(collection, filter = {}, options = {})
         validate_doc!(filter)
         @collection = collection
-        parse_parameters!(BSON::Document.new(filter), BSON::Document.new(options))
+
+        filter = BSON::Document.new(filter)
+        options = BSON::Document.new(options)
+
+        # This is when users pass $query in filter and other modifiers
+        # alongside?
+        query = filter.delete(:$query)
+        # This makes modifiers contain the filter if filter wasn't
+        # given via $query but as top-level keys, presumably
+        # downstream code ignores non-modifier keys in the modifiers?
+        modifiers = filter.merge(options.delete(:modifiers) || {})
+        @filter = (query || filter).freeze
+        @options = Operation::Find::Builder::Modifiers.map_driver_options(modifiers).merge!(options).freeze
       end
 
       # Get a human-readable string representation of +View+.
@@ -189,13 +203,6 @@ def initialize_copy(other)
         @filter = other.filter.dup
       end
 
-      def parse_parameters!(filter, options)
-        query = filter.delete(QUERY)
-        modifiers = (filter || {}).merge(options.delete(MODIFIERS) || {})
-        @filter = (query || filter).freeze
-        @options = Builder::Modifiers.map_driver_options(modifiers).merge!(options).freeze
-      end
-
       def new(options)
         View.new(collection, filter, options)
       end

lib/mongo/collection/view/builder.rb

@@ -17,7 +17,3 @@
 
 require 'mongo/collection/view/builder/aggregation'
 require 'mongo/collection/view/builder/map_reduce'
-require 'mongo/collection/view/builder/op_query'
-require 'mongo/collection/view/builder/find_command'
-require 'mongo/collection/view/builder/flags'
-require 'mongo/collection/view/builder/modifiers'

lib/mongo/collection/view/builder/find_command.rb

@@ -71,36 +71,6 @@ def initialize(map, reduce, view, options)
             @options = options
           end
 
-          # Get the specification for issuing a find command on the map/reduce
-          # results.
-          #
-          # @example Get the command specification.
-          #   builder.command_specification
-          #
-          # @return [ Hash ] The specification.
-          #
-          # @since 2.2.0
-          def command_specification
-            {
-              selector: find_command,
-              db_name: query_database,
-              read: read,
-              session: options[:session]
-            }
-          end
-
-          # Get the specification for the document query after a map/reduce.
-          #
-          # @example Get the query specification.
-          #   builder.query_specification
-          #
-          # @return [ Hash ] The specification.
-          #
-          # @since 2.2.0
-          def query_specification
-            { selector: {}, options: {}, db_name: query_database, coll_name: query_collection }
-          end
-
           # Get the specification to pass to the map/reduce operation.
           #
           # @example Get the specification.
@@ -113,6 +83,8 @@ def specification
             spec = {
               selector: map_reduce_command,
               db_name: database.name,
+              # Note that selector just above may also have a read preference
+              # specified, per the #map_reduce_command method below.
               read: read,
               session: options[:session]
             }
@@ -121,46 +93,36 @@ def specification
 
           private
 
-          OUT_ACTIONS = [ :replace, :merge, :reduce ].freeze
-
           def write?(spec)
             if out = spec[:selector][:out]
               out.is_a?(String) ||
                 (out.respond_to?(:keys) && out.keys.first.to_s.downcase != View::MapReduce::INLINE)
             end
           end
 
-          def find_command
-            BSON::Document.new('find' => query_collection, 'filter' => {})
-          end
-
           def map_reduce_command
             command = BSON::Document.new(
               :mapReduce => collection.name,
               :map => map,
               :reduce => reduce,
               :query => filter,
-              :out => { inline: 1 }
+              :out => { inline: 1 },
             )
+            # Shouldn't this use self.read ?
             if collection.read_concern
               command[:readConcern] = Options::Mapper.transform_values_to_strings(
                 collection.read_concern)
             end
             command.merge!(view_options)
+            # Read preference isn't simply passed in the command payload
+            # (it may need to be converted to wire protocol flags)
+            # so remove it here and hopefully it's handled elsewhere.
+            # If not, RUBY-2706.
+            command.delete(:read)
             command.merge!(Options::Mapper.transform_documents(options, MAPPINGS))
             command
           end
 
-          def query_database
-            options[:out].respond_to?(:keys) && options[:out][:db] ? options[:out][:db] : database.name
-          end
-
-          def query_collection
-            if options[:out].respond_to?(:keys)
-              options[:out][OUT_ACTIONS.find { |action| options[:out][action] }]
-            end || options[:out]
-          end
-
           def view_options
             @view_options ||= (opts = view.options.dup
                                opts.delete(:session)