Don't wrap query spec when querying $cmd - PYTHON-403

Don't include $readPreferences when querying $cmd unless
the command is ok to be sent to a secondary

pymongo/cursor.py

@@ -18,7 +18,7 @@ from collections import deque
 from bson.code import Code
 from bson.son import SON
 from pymongo import helpers, message, read_preferences
-from pymongo.read_preferences import ReadPreference
+from pymongo.read_preferences import ReadPreference, secondary_ok_commands
 from pymongo.errors import (InvalidOperation,
                             AutoReconnect)
 
@@ -235,9 +235,30 @@ class Cursor(object):
         if operators:
             # Make a shallow copy so we can cleanly rewind or clone.
             spec = self.__spec.copy()
-            if "$query" not in spec:
+
+            # Only commands that can be run on secondaries should have any
+            # operators added to the spec.  Command queries can be issued
+            # by db.command or calling find_one on $cmd directly
+            is_cmd = self.collection.name == "$cmd"
+            if is_cmd:
+                # Don't change commands that can't be sent to secondaries
+                command_name = spec.keys()[0].lower()
+                if command_name not in secondary_ok_commands:
+                    return spec
+                elif command_name == 'mapreduce':
+                    # mapreduce shouldn't be changed if its not inline
+                    out = spec.get('out')
+                    if not isinstance(out, dict) or not out.get('inline'):
+                        return spec
+            elif "$query" not in spec:
                 # $query has to come first
                 spec = SON({"$query": spec})
+
+            if not isinstance(spec, SON):
+                # Ensure the spec is SON. As order is important this will
+                # ensure its set before merging in any extra operators.
+                spec = SON(spec)
+
             spec.update(operators)
             return spec
         # Have to wrap with $query if "query" is the first key.

pymongo/database.py

@@ -350,9 +350,9 @@ class Database(common.BaseObject):
         if isinstance(command, basestring):
             command = SON([(command, value)])
 
-        command_name = command.keys()[0]
+        command_name = command.keys()[0].lower()
         must_use_master = kwargs.pop('_use_master', False)
-        if command_name.lower() not in rp.secondary_ok_commands:
+        if command_name not in rp.secondary_ok_commands:
             must_use_master = True
 
         # Special-case: mapreduce can go to secondaries only if inline

test/test_cursor.py

@@ -15,7 +15,6 @@
 """Test the cursor module."""
 import unittest
 import random
-import warnings
 import sys
 import itertools
 sys.path[0:0] = [""]
@@ -25,12 +24,11 @@ from nose.plugins.skip import SkipTest
 from bson.code import Code
 from pymongo import (ASCENDING,
                      DESCENDING)
-from pymongo.cursor import Cursor
 from pymongo.database import Database
 from pymongo.errors import (InvalidOperation,
                             OperationFailure)
-from test.test_connection import get_connection
 from test import version
+from test.test_connection import get_connection
 
 
 class TestCursor(unittest.TestCase):
@@ -681,21 +679,21 @@ class TestCursor(unittest.TestCase):
 
         cursor = db.test.find(tailable=True)
 
-        db.test.insert({"x": 1})
+        db.test.insert({"x": 1}, safe=True)
         count = 0
         for doc in cursor:
             count += 1
             self.assertEqual(1, doc["x"])
         self.assertEqual(1, count)
 
-        db.test.insert({"x": 2})
+        db.test.insert({"x": 2}, safe=True)
         count = 0
         for doc in cursor:
             count += 1
             self.assertEqual(2, doc["x"])
         self.assertEqual(1, count)
 
-        db.test.insert({"x": 3})
+        db.test.insert({"x": 3}, safe=True)
         count = 0
         for doc in cursor:
             count += 1

test/test_read_preferences.py

@@ -18,11 +18,14 @@ import random
 import sys
 import unittest
 
+from nose.plugins.skip import SkipTest
+
 sys.path[0:0] = [""]
 
 from bson.son import SON
 from pymongo.replica_set_connection import ReplicaSetConnection
-from pymongo.read_preferences import ReadPreference, modes, MovingAverage
+from pymongo.read_preferences import (ReadPreference, modes, MovingAverage,
+                                      secondary_ok_commands)
 from pymongo.errors import ConfigurationError
 
 from test.test_replica_set_connection import TestConnectionReplicaSetBase
@@ -93,7 +96,7 @@ class TestReadPreferences(TestReadPreferencesBase):
     def test_secondary_preferred(self):
         self.assertReadsFrom('secondary',
             read_preference=ReadPreference.SECONDARY_PREFERRED)
-        
+
     def test_secondary_only(self):
         # Test deprecated mode SECONDARY_ONLY, which is now a synonym for
         # SECONDARY
@@ -135,7 +138,7 @@ class ReadPrefTester(ReplicaSetConnection):
 class TestCommandAndReadPreference(TestConnectionReplicaSetBase):
     def setUp(self):
         super(TestCommandAndReadPreference, self).setUp()
-        
+
         # Need auto_start_request False to avoid pinning members.
         self.c = ReadPrefTester(
             '%s:%s' % (host, port),
@@ -262,7 +265,7 @@ class TestCommandAndReadPreference(TestConnectionReplicaSetBase):
         self._test_fn(True, lambda: self.c.pymongo_test.command(
             'geoSearch', 'test', near=[33, 33], maxDistance=6,
             search={'type': 'restaurant' }, limit=30))
-        
+
         self._test_fn(True, lambda: self.c.pymongo_test.command(SON([
             ('geoSearch', 'test'), ('near', [33, 33]), ('maxDistance', 6),
             ('search', {'type': 'restaurant'}), ('limit', 30)])))
@@ -466,6 +469,37 @@ class TestMongosConnection(unittest.TestCase):
                     self.assertFalse(
                         '$readPreference' in cursor._Cursor__query_spec())
 
+    def test_only_secondary_ok_commands_have_read_prefs(self):
+        c = get_connection(read_preference=ReadPreference.SECONDARY)
+        is_mongos = utils.is_mongos(c)
+        if not is_mongos:
+            raise SkipTest("Only mongos have read_prefs added to the spec")
+
+        # Ensure secondary_ok_commands have readPreference
+        for cmd in secondary_ok_commands:
+            if cmd == 'mapreduce':  # map reduce is a special case
+                continue
+            command = SON([(cmd, 1)])
+            cursor = c.pymongo_test["$cmd"].find(command.copy())
+            command['$readPreference'] = {'mode': 'secondary'}
+            self.assertEqual(command, cursor._Cursor__query_spec())
+
+        # map_reduce inline should have read prefs
+        command = SON([('mapreduce', 'test'), ('out', {'inline': 1})])
+        cursor = c.pymongo_test["$cmd"].find(command.copy())
+        command['$readPreference'] = {'mode': 'secondary'}
+        self.assertEqual(command, cursor._Cursor__query_spec())
+
+        # map_reduce that outputs to a collection shouldn't have read prefs
+        command = SON([('mapreduce', 'test'), ('out', {'mrtest': 1})])
+        cursor = c.pymongo_test["$cmd"].find(command.copy())
+        self.assertEqual(command, cursor._Cursor__query_spec())
+
+        # Other commands shouldn't be changed
+        for cmd in ('drop', 'create', 'any-future-cmd'):
+            command = SON([(cmd, 1)])
+            cursor = c.pymongo_test["$cmd"].find(command.copy())
+            self.assertEqual(command, cursor._Cursor__query_spec())
 
 if __name__ == "__main__":
     unittest.main()