PYTHON-677 - Switch internals to new WriteConcern class

Author: behackett

pymongo/collection.py

@@ -383,15 +383,16 @@ def gen():
                     ids.append(doc.get('_id'))
                     yield doc
 
-        safe, options = self._get_write_mode(kwargs)
+        concern = kwargs or self.write_concern
+        safe = concern.get("w") != 0
 
         if client.max_wire_version > 1 and safe:
             # Insert command
             command = SON([('insert', self.name),
                            ('ordered', not continue_on_error)])
 
-            if options:
-                command['writeConcern'] = options
+            if concern:
+                command['writeConcern'] = concern
 
             results = message._do_batched_write_command(
                     self.database.name + ".$cmd", _INSERT, command,
@@ -400,7 +401,7 @@ def gen():
         else:
             # Legacy batched OP_INSERT
             message._do_batched_insert(self.__full_name, gen(), check_keys,
-                                       safe, options, continue_on_error,
+                                       safe, concern, continue_on_error,
                                        self.uuid_subtype, client)
 
         if return_one:
@@ -515,7 +516,8 @@ def update(self, spec, document, upsert=False, manipulate=False,
         if manipulate:
             document = self.__database._fix_incoming(document, self)
 
-        safe, options = self._get_write_mode(kwargs)
+        concern = kwargs or self.write_concern
+        safe = concern.get("w") != 0
 
         if document:
             # If a top level key begins with '$' this is a modify operation
@@ -530,8 +532,8 @@ def update(self, spec, document, upsert=False, manipulate=False,
         if client.max_wire_version > 1 and safe:
             # Update command
             command = SON([('update', self.name)])
-            if options:
-                command['writeConcern'] = options
+            if concern:
+                command['writeConcern'] = concern
 
             docs = [SON([('q', spec), ('u', document),
                          ('multi', multi), ('upsert', upsert)])]
@@ -554,7 +556,7 @@ def update(self, spec, document, upsert=False, manipulate=False,
             # Legacy OP_UPDATE
             return client._send_message(
                 message.update(self.__full_name, upsert, multi,
-                               spec, document, safe, options,
+                               spec, document, safe, concern,
                                check_keys, self.uuid_subtype), safe)
 
     def drop(self):
@@ -640,7 +642,8 @@ def remove(self, spec_or_id=None, multi=True, **kwargs):
         if not isinstance(spec_or_id, dict):
             spec_or_id = {"_id": spec_or_id}
 
-        safe, options = self._get_write_mode(kwargs)
+        concern = kwargs or self.write_concern
+        safe = concern.get("w") != 0
 
         client = self.database.connection
 
@@ -649,8 +652,8 @@ def remove(self, spec_or_id=None, multi=True, **kwargs):
         if client.max_wire_version > 1 and safe:
             # Delete command
             command = SON([('delete', self.name)])
-            if options:
-                command['writeConcern'] = options
+            if concern:
+                command['writeConcern'] = concern
 
             docs = [SON([('q', spec_or_id), ('limit', int(not multi))])]
 
@@ -666,7 +669,7 @@ def remove(self, spec_or_id=None, multi=True, **kwargs):
             # Legacy OP_DELETE
             return client._send_message(
                 message.delete(self.__full_name, spec_or_id, safe,
-                               options, self.uuid_subtype, int(not multi)), safe)
+                               concern, self.uuid_subtype, int(not multi)), safe)
 
     def find_one(self, spec_or_id=None, *args, **kwargs):
         """Get a single document from the database.

pymongo/common.py

@@ -19,6 +19,7 @@
 
 from pymongo.auth import MECHANISMS
 from pymongo.errors import ConfigurationError
+from pymongo.write_concern import WriteConcern
 from bson.binary import (OLD_UUID_SUBTYPE, UUID_SUBTYPE,
                          JAVA_LEGACY, CSHARP_LEGACY)
 
@@ -245,8 +246,7 @@ def validate_read_preference_tags(name, value):
     return [tags]
 
 
-
-# jounal is an alias for j,
+# journal is an alias for j,
 # wtimeoutms is an alias for wtimeout,
 VALIDATORS = {
     'replicaset': validate_basestring,
@@ -310,22 +310,6 @@ def validate(option, value):
 ])
 
 
-class WriteConcern(dict):
-
-    def __init__(self, *args, **kwargs):
-        """A subclass of dict that overrides __setitem__ to
-        validate write concern options.
-        """
-        super(WriteConcern, self).__init__(*args, **kwargs)
-
-    def __setitem__(self, key, value):
-        if key not in WRITE_CONCERN_OPTIONS:
-            raise ConfigurationError("%s is not a valid write "
-                                     "concern option." % (key,))
-        key, value = validate(key, value)
-        super(WriteConcern, self).__setitem__(key, value)
-
-
 class BaseObject(object):
     """A base class that provides attributes and methods common
     to multiple pymongo classes.
@@ -337,20 +321,12 @@ def __init__(self, **options):
 
         self.__read_pref = read_preferences.ReadPreference.PRIMARY
         self.__uuid_subtype = OLD_UUID_SUBTYPE
-        self.__write_concern = WriteConcern()
+        self.__write_concern = None
         self.__set_options(options)
 
-    def __set_write_concern_option(self, option, value):
-        """Validates and sets getlasterror options for this
-        object (MongoClient, Database, Collection, etc.)
-        """
-        if value is None:
-            self.__write_concern.pop(option, None)
-        else:
-            self.__write_concern[option] = value
-
     def __set_options(self, options):
         """Validates and sets all options passed to this object."""
+        wc_opts = {}
         for option, value in options.iteritems():
             if option == 'read_preference':
                 self.__read_pref = validate_read_preference(option, value)
@@ -365,25 +341,20 @@ def __set_options(self, options):
             elif option == 'uuidrepresentation':
                 self.__uuid_subtype = validate_uuid_subtype(option, value)
             elif option in WRITE_CONCERN_OPTIONS:
-                if option == 'journal':
-                    self.__set_write_concern_option('j', value)
-                elif option == 'wtimeoutms':
-                    self.__set_write_concern_option('wtimeout', value)
+                if option == "journal":
+                    wc_opts["j"] = value
+                elif option == "wtimeoutms":
+                    wc_opts["wtimeout"] = value
                 else:
-                    self.__set_write_concern_option(option, value)
+                    wc_opts[option] = value
+        self.__write_concern = WriteConcern(**wc_opts)
 
     def __set_write_concern(self, value):
         """Property setter for write_concern."""
         if not isinstance(value, dict):
             raise ConfigurationError("write_concern must be an "
                                      "instance of dict or a subclass.")
-        # Make a copy here to avoid users accidentally setting the
-        # same dict on multiple instances.
-        wc = WriteConcern()
-        for k, v in value.iteritems():
-            # Make sure we validate each option.
-            wc[k] = v
-        self.__write_concern = wc
+        self.__write_concern = WriteConcern(**value)
 
     def __get_write_concern(self):
         """The default write concern for this instance.
@@ -435,7 +406,7 @@ def __get_write_concern(self):
         """
         # To support dict style access we have to return the actual
         # WriteConcern here, not a copy.
-        return self.__write_concern
+        return self.__write_concern.document
 
     write_concern = property(__get_write_concern, __set_write_concern)
 
@@ -477,22 +448,7 @@ def _get_wc_override(self):
         We don't want to override user write concern options if write concern
         is already enabled.
         """
-        if self.__write_concern.get('w') != 0:
+        if self.__write_concern.acknowledged:
             return {}
         return {'w': 1}
 
-    def _get_write_mode(self, options):
-        """Get the current write mode.
-
-        Determines if the current write is acknowledged or not based on the
-        inherited write_concern values, or passed options.
-
-        :Parameters:
-            - `options`: overriding write concern options.
-
-        .. versionadded:: 2.3
-        """
-        write_concern = options or self.__write_concern
-        if write_concern.get('w') == 0:
-            return False, {}
-        return True, write_concern

pymongo/write_concern.py

@@ -0,0 +1,105 @@
+# Copyright 2014 MongoDB, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Tools for working with write concerns."""
+
+from bson.py3compat import integer_types, string_type
+from pymongo.errors import ConfigurationError
+
+class WriteConcern(object):
+    """WriteConcern
+    
+    :Parameters:
+        - `w`: (integer or string) Used with replication, write operations
+          will block until they have been replicated to the specified number
+          or tagged set of servers. `w=<integer>` always includes the replica
+          set primary (e.g. w=3 means write to the primary and wait until
+          replicated to **two** secondaries). **w=0 disables acknowledgement
+          of write operations and can not be used with other write concern
+          options.**
+        - `wtimeout`: (integer) Used in conjunction with `w`. Specify a value
+          in milliseconds to control how long to wait for write propagation
+          to complete. If replication does not complete in the given
+          timeframe, a timeout exception is raised.
+        - `j`: If ``True`` block until write operations have been committed
+          to the journal. Cannot be used in combination with `fsync`. Prior
+          to MongoDB 2.6 this option was ignored if the server was running
+          without journaling. Starting with MongoDB 2.6 write operations will
+          fail with an exception if this option is used when the server is
+          running without journaling.
+        - `fsync`: If ``True`` and the server is running without journaling,
+          blocks until the server has synced all data files to disk. If the
+          server is running with journaling, this acts the same as the `j`
+          option, blocking until write operations have been committed to the
+          journal. Cannot be used in combination with `j`.
+    """
+
+    __slots__ = ("__document", "__acknowledged")
+
+    def __init__(self, w=None, wtimeout=None, j=None, fsync=None):
+        self.__document = {}
+        self.__acknowledged = True
+
+        if wtimeout is not None:
+            if not isinstance(wtimeout, integer_types):
+                raise ConfigurationError("wtimeout must be an integer")
+            self.__document["wtimeout"] = wtimeout
+
+        if j is not None:
+            if not isinstance(j, bool):
+                raise ConfigurationError("j must be True or False")
+            self.__document["j"] = j
+
+        if fsync is not None:
+            if not isinstance(fsync, bool):
+                raise ConfigurationError("fsync must be True or False")
+            if j and fsync:
+                raise ConfigurationError("Can't set both j "
+                                         "and fsync at the same time")
+            self.__document["fsync"] = fsync
+
+        if self.__document and w == 0:
+            raise ConfigurationError("Can not use w value "
+                                     "of 0 with other options")
+        if w is not None:
+            if isinstance(w, integer_types):
+                self.__acknowledged = w > 0
+            elif not isinstance(w, string_type):
+                raise ConfigurationError("w must be an integer or string")
+            self.__document["w"] = w
+
+    @property
+    def document(self):
+        """The document representation of this write concern.
+        """
+        return self.__document
+
+    @property
+    def acknowledged(self):
+        """If ``True`` write operations will wait for acknowledgement before
+        returning.
+        """
+        return self.__acknowledged
+
+    # This doesn't keep the options in order. Do we care?
+    def __repr__(self):
+        return ("WriteConcern(%s)" % (
+            ", ".join("%s=%s" % kvt for kvt in self.document.items()),))
+
+    def __eq__(self, other):
+        return self.document == other.document
+
+    def __ne__(self, other):
+        return self.document != other.document
+

test/test_collection.py

@@ -833,7 +833,7 @@ def test_insert_multiple_with_duplicate(self):
         )
 
         db.drop_collection("test")
-        db.write_concern['w'] = 0
+        db.write_concern = {"w": 0}
         db.test.ensure_index([('i', ASCENDING)], unique=True)
 
         # No error

test/test_common.py

@@ -217,18 +217,6 @@ def test_write_concern(self):
         coll.write_concern = wc
         self.assertEqual(wc.to_dict(), coll.write_concern)
 
-        def f():
-            c.write_concern = {'foo': 'bar'}
-        self.assertRaises(ConfigurationError, f)
-
-        def f():
-            c.write_concern['foo'] = 'bar'
-        self.assertRaises(ConfigurationError, f)
-
-        def f():
-            c.write_concern = [('foo', 'bar')]
-        self.assertRaises(ConfigurationError, f)
-
     def test_mongo_client(self):
         m = MongoClient(pair, w=0)
         coll = m.pymongo_test.write_concern_test