Truly test pymongo v2.8 instead of v2.9.

Testing v2.9 was unexpected and it's not as important given v2.9 mostly
mostly just provided an upgrade path for v3.0, and we already test v3.x.

.travis.yml

@@ -1,3 +1,6 @@
+# Use a container-based environment
+sudo: false
+
 language: python
 
 python:
@@ -8,26 +11,41 @@ python:
 - pypy
 - pypy3
 
+# Test on PyMongo v2.7.x, v2.8.x, and v3.x
 env:
 - PYMONGO=2.7
 - PYMONGO=2.8
 - PYMONGO=3.0
-- PYMONGO=dev
 
 matrix:
   fast_finish: true
 
-before_install:
+services:
-- travis_retry sudo apt-key adv --keyserver hkp://keyserver.ubuntu.com:80 --recv 7F0CEB10
+- mongodb
-- echo 'deb http://downloads-distro.mongodb.org/repo/ubuntu-upstart dist 10gen' |
+
-  sudo tee /etc/apt/sources.list.d/mongodb.list
+addons:
-- travis_retry sudo apt-get update
+  apt:
-- travis_retry sudo apt-get install mongodb-org-server
+    sources:
+    - mongodb-upstart
+    packages:
+    - mongodb-org-server=2.6.9
+
+    # Optional dependencies for the ImageField and others
+    - python-dev
+    - python3-dev
+    - libopenjpeg-dev
+    - zlib1g-dev
+    - libjpeg-turbo8-dev
+    - libtiff4-dev
+    - libjpeg8-dev
+    - libfreetype6-dev
+    - liblcms2-dev
+    - libwebp-dev
+    - tcl8.5-dev
+    - tk8.5-dev
+    - python-tk
 
 install:
-- sudo apt-get install python-dev python3-dev libopenjpeg-dev zlib1g-dev libjpeg-turbo8-dev
-  libtiff4-dev libjpeg8-dev libfreetype6-dev liblcms2-dev libwebp-dev tcl8.5-dev tk8.5-dev
-  python-tk
 - travis_retry pip install --upgrade pip
 - travis_retry pip install coveralls
 - travis_retry pip install flake8
@@ -35,6 +53,9 @@ install:
 - travis_retry pip install "virtualenv<14.0.0"  # virtualenv>=14.0.0 has dropped Python 3.2 support (and pypy3 is based on py32)
 - travis_retry tox -e $(echo py$TRAVIS_PYTHON_VERSION-mg$PYMONGO | tr -d . | sed -e 's/pypypy/pypy/') -- -e test
 
+# Cache dependencies installed via pip
+cache: pip
+
 # Run flake8 for py27
 before_script:
 - if [[ $TRAVIS_PYTHON_VERSION == '2.7' ]]; then tox -e flake8; fi
@@ -45,7 +66,7 @@ script:
 # For now only submit coveralls for Python v2.7. Python v3.x currently shows
 # 0% coverage. That's caused by 'use_2to3', which builds the py3-compatible
 # code in a separate dir and runs tests on that.
-after_script:
+after_success:
 - if [[ $TRAVIS_PYTHON_VERSION == '2.7' ]]; then coveralls --verbose; fi
 
 notifications:
@@ -56,11 +77,22 @@ branches:
   - master
   - /^v.*$/
 
+# Whenever a new release is created via GitHub, publish it on PyPI.
 deploy:
   provider: pypi
   user: the_drow
   password:
     secure: QMyatmWBnC6ZN3XLW2+fTBDU4LQcp1m/LjR2/0uamyeUzWKdlOoh/Wx5elOgLwt/8N9ppdPeG83ose1jOz69l5G0MUMjv8n/RIcMFSpCT59tGYqn3kh55b0cIZXFT9ar+5cxlif6a5rS72IHm5li7QQyxexJIII6Uxp0kpvUmek=
+
+  # create a source distribution and a pure python wheel for faster installs
+  distributions: "sdist bdist_wheel"
+
+  # only deploy on tagged commits (aka GitHub releases) and only for the
+  # parent repo's builds running Python 2.7 along with dev PyMongo (we run
+  # Travis against many different Python and PyMongo versions and we don't
+  # want the deploy to occur multiple times).
   on:
     tags: true
     repo: MongoEngine/mongoengine
+    condition: "$PYMONGO = 3.0"
+    python: 2.7

README.rst

@@ -35,16 +35,22 @@ setup.py install``.
 
 Dependencies
 ============
-- pymongo>=2.7.1
+All of the dependencies can easily be installed via `pip <https://pip.pypa.io/>`_. At the very least, you'll need these two packages to use MongoEngine:
-- sphinx (optional - for documentation generation)
+
+- pymongo>=2.7.1
+- six>=1.10.0
+
+If you utilize a ``DateTimeField``, you might also use a more flexible date parser:
 
-Optional Dependencies
----------------------
-- **Image Fields**: Pillow>=2.0.0
 - dateutil>=2.1.0
 
-.. note
+If you need to use an ``ImageField`` or ``ImageGridFsProxy``:
-   MongoEngine always runs it's test suite against the latest patch version of each dependecy. e.g.: PyMongo 3.0.1
+
+- Pillow>=2.0.0
+
+If you want to generate the documentation (e.g. to contribute to it):
+
+- sphinx
 
 Examples
 ========

docs/changelog.rst

@@ -5,6 +5,8 @@ Changelog
 Development
 ===========
 - (Fill this out as you fix issues and develop you features).
+- POTENTIAL BREAKING CHANGE: Fixed limit/skip/hint/batch_size chaining #1476
+- POTENTIAL BREAKING CHANGE: Changed a public `QuerySet.clone_into` method to a private `QuerySet._clone_into` #1476
 - Fixed connecting to a replica set with PyMongo 2.x #1436
 - Fixed an obscure error message when filtering by `field__in=non_iterable`. #1237
 
@@ -13,6 +15,7 @@ Changes in 0.11.0
 - BREAKING CHANGE: Renamed `ConnectionError` to `MongoEngineConnectionError` since the former is a built-in exception name in Python v3.x. #1428
 - BREAKING CHANGE: Dropped Python 2.6 support. #1428
 - BREAKING CHANGE: `from mongoengine.base import ErrorClass` won't work anymore for any error from `mongoengine.errors` (e.g. `ValidationError`). Use `from mongoengine.errors import ErrorClass instead`. #1428
+- BREAKING CHANGE: Accessing a broken reference will raise a `DoesNotExist` error. In the past it used to return `None`. #1334
 - Fixed absent rounding for DecimalField when `force_string` is set. #1103
 
 Changes in 0.10.8

docs/guide/defining-documents.rst

@@ -361,11 +361,6 @@ Its value can take any of the following constants:
    In Django, be sure to put all apps that have such delete rule declarations in
    their :file:`models.py` in the :const:`INSTALLED_APPS` tuple.
 
-
-.. warning::
-   Signals are not triggered when doing cascading updates / deletes - if this
-   is required you must manually handle the update / delete.
-
 Generic reference fields
 ''''''''''''''''''''''''
 A second kind of reference field also exists,

docs/guide/signals.rst

@@ -142,11 +142,4 @@ cleaner looking while still allowing manual execution of the callback::
         modified = DateTimeField()
 
 
-ReferenceFields and Signals
----------------------------
-
-Currently `reverse_delete_rule` does not trigger signals on the other part of
-the relationship.  If this is required you must manually handle the
-reverse deletion.
-
 .. _blinker: http://pypi.python.org/pypi/blinker

docs/upgrade.rst

@@ -2,6 +2,20 @@
 Upgrading
 #########
 
+Development
+***********
+(Fill this out whenever you introduce breaking changes to MongoEngine)
+
+This release includes various fixes for the `BaseQuerySet` methods and how they
+are chained together. Since version 0.10.1 applying limit/skip/hint/batch_size
+to an already-existing queryset wouldn't modify the underlying PyMongo cursor.
+This has been fixed now, so you'll need to make sure that your code didn't rely
+on the broken implementation.
+
+Additionally, a public `BaseQuerySet.clone_into` has been renamed to a private
+`_clone_into`. If you directly used that method in your code, you'll need to
+rename its occurrences.
+
 0.11.0
 ******
 This release includes a major rehaul of MongoEngine's code quality and

mongoengine/base/datastructures.py

@@ -429,7 +429,7 @@ class StrictDict(object):
     def __eq__(self, other):
         return self.items() == other.items()
 
-    def __neq__(self, other):
+    def __ne__(self, other):
         return self.items() != other.items()
 
     @classmethod

mongoengine/base/fields.py

@@ -41,7 +41,7 @@ class BaseField(object):
         """
         :param db_field: The database field to store this field in
             (defaults to the name of the field)
-        :param name: Depreciated - use db_field
+        :param name: Deprecated - use db_field
         :param required: If the field is required. Whether it has to have a
             value or not. Defaults to False.
         :param default: (optional) The default value for this field if no value
@@ -81,6 +81,17 @@ class BaseField(object):
         self.sparse = sparse
         self._owner_document = None
 
+        # Validate the db_field
+        if isinstance(self.db_field, six.string_types) and (
+            '.' in self.db_field or
+            '\0' in self.db_field or
+            self.db_field.startswith('$')
+        ):
+            raise ValueError(
+                'field names cannot contain dots (".") or null characters '
+                '("\\0"), and they must not start with a dollar sign ("$").'
+            )
+
         # Detect and report conflicts between metadata and base properties.
         conflicts = set(dir(self)) & set(kwargs)
         if conflicts:

mongoengine/document.py

@@ -332,68 +332,20 @@ class Document(BaseDocument):
         signals.pre_save_post_validation.send(self.__class__, document=self,
                                               created=created, **signal_kwargs)
 
+        if self._meta.get('auto_create_index', True):
+            self.ensure_indexes()
+
         try:
-            collection = self._get_collection()
+            # Save a new document or update an existing one
-            if self._meta.get('auto_create_index', True):
-                self.ensure_indexes()
             if created:
-                if force_insert:
+                object_id = self._save_create(doc, force_insert, write_concern)
-                    object_id = collection.insert(doc, **write_concern)
-                else:
-                    object_id = collection.save(doc, **write_concern)
-                    # In PyMongo 3.0, the save() call calls internally the _update() call
-                    # but they forget to return the _id value passed back, therefore getting it back here
-                    # Correct behaviour in 2.X and in 3.0.1+ versions
-                    if not object_id and pymongo.version_tuple == (3, 0):
-                        pk_as_mongo_obj = self._fields.get(self._meta['id_field']).to_mongo(self.pk)
-                        object_id = (
-                            self._qs.filter(pk=pk_as_mongo_obj).first() and
-                            self._qs.filter(pk=pk_as_mongo_obj).first().pk
-                        )  # TODO doesn't this make 2 queries?
             else:
-                object_id = doc['_id']
+                object_id, created = self._save_update(doc, save_condition,
-                updates, removals = self._delta()
+                                                       write_concern)
-                # Need to add shard key to query, or you get an error
-                if save_condition is not None:
-                    select_dict = transform.query(self.__class__,
-                                                  **save_condition)
-                else:
-                    select_dict = {}
-                select_dict['_id'] = object_id
-                shard_key = self._meta.get('shard_key', tuple())
-                for k in shard_key:
-                    path = self._lookup_field(k.split('.'))
-                    actual_key = [p.db_field for p in path]
-                    val = doc
-                    for ak in actual_key:
-                        val = val[ak]
-                    select_dict['.'.join(actual_key)] = val
-
-                def is_new_object(last_error):
-                    if last_error is not None:
-                        updated = last_error.get('updatedExisting')
-                        if updated is not None:
-                            return not updated
-                    return created
-
-                update_query = {}
-
-                if updates:
-                    update_query['$set'] = updates
-                if removals:
-                    update_query['$unset'] = removals
-                if updates or removals:
-                    upsert = save_condition is None
-                    last_error = collection.update(select_dict, update_query,
-                                                   upsert=upsert, **write_concern)
-                    if not upsert and last_error['n'] == 0:
-                        raise SaveConditionError('Race condition preventing'
-                                                 ' document update detected')
-                    created = is_new_object(last_error)
 
             if cascade is None:
-                cascade = self._meta.get(
+                cascade = (self._meta.get('cascade', False) or
-                    'cascade', False) or cascade_kwargs is not None
+                           cascade_kwargs is not None)
 
             if cascade:
                 kwargs = {
@@ -406,6 +358,7 @@ class Document(BaseDocument):
                     kwargs.update(cascade_kwargs)
                 kwargs['_refs'] = _refs
                 self.cascade_save(**kwargs)
+
         except pymongo.errors.DuplicateKeyError as err:
             message = u'Tried to save duplicate unique keys (%s)'
             raise NotUniqueError(message % six.text_type(err))
@@ -418,16 +371,91 @@ class Document(BaseDocument):
                 raise NotUniqueError(message % six.text_type(err))
             raise OperationError(message % six.text_type(err))
 
+        # Make sure we store the PK on this document now that it's saved
         id_field = self._meta['id_field']
         if created or id_field not in self._meta.get('shard_key', []):
             self[id_field] = self._fields[id_field].to_python(object_id)
 
         signals.post_save.send(self.__class__, document=self,
                                created=created, **signal_kwargs)
+
         self._clear_changed_fields()
         self._created = False
+
         return self
 
+    def _save_create(self, doc, force_insert, write_concern):
+        """Save a new document.
+
+        Helper method, should only be used inside save().
+        """
+        collection = self._get_collection()
+
+        if force_insert:
+            return collection.insert(doc, **write_concern)
+
+        object_id = collection.save(doc, **write_concern)
+
+        # In PyMongo 3.0, the save() call calls internally the _update() call
+        # but they forget to return the _id value passed back, therefore getting it back here
+        # Correct behaviour in 2.X and in 3.0.1+ versions
+        if not object_id and pymongo.version_tuple == (3, 0):
+            pk_as_mongo_obj = self._fields.get(self._meta['id_field']).to_mongo(self.pk)
+            object_id = (
+                self._qs.filter(pk=pk_as_mongo_obj).first() and
+                self._qs.filter(pk=pk_as_mongo_obj).first().pk
+            )  # TODO doesn't this make 2 queries?
+
+        return object_id
+
+    def _save_update(self, doc, save_condition, write_concern):
+        """Update an existing document.
+
+        Helper method, should only be used inside save().
+        """
+        collection = self._get_collection()
+        object_id = doc['_id']
+        created = False
+
+        select_dict = {}
+        if save_condition is not None:
+            select_dict = transform.query(self.__class__, **save_condition)
+
+        select_dict['_id'] = object_id
+
+        # Need to add shard key to query, or you get an error
+        shard_key = self._meta.get('shard_key', tuple())
+        for k in shard_key:
+            path = self._lookup_field(k.split('.'))
+            actual_key = [p.db_field for p in path]
+            val = doc
+            for ak in actual_key:
+                val = val[ak]
+            select_dict['.'.join(actual_key)] = val
+
+        updates, removals = self._delta()
+        update_query = {}
+        if updates:
+            update_query['$set'] = updates
+        if removals:
+            update_query['$unset'] = removals
+        if updates or removals:
+            upsert = save_condition is None
+            last_error = collection.update(select_dict, update_query,
+                                           upsert=upsert, **write_concern)
+            if not upsert and last_error['n'] == 0:
+                raise SaveConditionError('Race condition preventing'
+                                         ' document update detected')
+            if last_error is not None:
+                updated_existing = last_error.get('updatedExisting')
+                if updated_existing is False:
+                    created = True
+                    # !!! This is bad, means we accidentally created a new,
+                    # potentially corrupted document. See
+                    # https://github.com/MongoEngine/mongoengine/issues/564
+
+        return object_id, created
+
     def cascade_save(self, **kwargs):
         """Recursively save any references and generic references on the
         document.

mongoengine/errors.py

@@ -50,8 +50,8 @@ class FieldDoesNotExist(Exception):
     or an :class:`~mongoengine.EmbeddedDocument`.
 
     To avoid this behavior on data loading,
-    you should the :attr:`strict` to ``False``
+    you should set the :attr:`strict` to ``False``
-    in the :attr:`meta` dictionnary.
+    in the :attr:`meta` dictionary.
     """
 
 

mongoengine/fields.py

@@ -888,10 +888,6 @@ class ReferenceField(BaseField):
 
         Foo.register_delete_rule(Bar, 'foo', NULLIFY)
 
-    .. note ::
-        `reverse_delete_rule` does not trigger pre / post delete signals to be
-        triggered.
-
     .. versionchanged:: 0.5 added `reverse_delete_rule`
     """
 

mongoengine/queryset/base.py

@@ -86,6 +86,7 @@ class BaseQuerySet(object):
         self._batch_size = None
         self.only_fields = []
         self._max_time_ms = None
+        self._comment = None
 
     def __call__(self, q_obj=None, class_check=True, read_preference=None,
                  **query):
@@ -706,39 +707,36 @@ class BaseQuerySet(object):
         with switch_db(self._document, alias) as cls:
             collection = cls._get_collection()
 
-        return self.clone_into(self.__class__(self._document, collection))
+        return self._clone_into(self.__class__(self._document, collection))
 
     def clone(self):
-        """Creates a copy of the current
+        """Create a copy of the current queryset."""
-          :class:`~mongoengine.queryset.QuerySet`
+        return self._clone_into(self.__class__(self._document, self._collection_obj))
 
-        .. versionadded:: 0.5
+    def _clone_into(self, new_qs):
+        """Copy all of the relevant properties of this queryset to
+        a new queryset (which has to be an instance of
+        :class:`~mongoengine.queryset.base.BaseQuerySet`).
         """
-        return self.clone_into(self.__class__(self._document, self._collection_obj))
+        if not isinstance(new_qs, BaseQuerySet):
-
-    def clone_into(self, cls):
-        """Creates a copy of the current
-          :class:`~mongoengine.queryset.base.BaseQuerySet` into another child class
-        """
-        if not isinstance(cls, BaseQuerySet):
             raise OperationError(
-                '%s is not a subclass of BaseQuerySet' % cls.__name__)
+                '%s is not a subclass of BaseQuerySet' % new_qs.__name__)
 
         copy_props = ('_mongo_query', '_initial_query', '_none', '_query_obj',
                       '_where_clause', '_loaded_fields', '_ordering', '_snapshot',
                       '_timeout', '_class_check', '_slave_okay', '_read_preference',
                       '_iter', '_scalar', '_as_pymongo', '_as_pymongo_coerce',
                       '_limit', '_skip', '_hint', '_auto_dereference',
-                      '_search_text', 'only_fields', '_max_time_ms')
+                      '_search_text', 'only_fields', '_max_time_ms', '_comment')
 
         for prop in copy_props:
             val = getattr(self, prop)
-            setattr(cls, prop, copy.copy(val))
+            setattr(new_qs, prop, copy.copy(val))
 
         if self._cursor_obj:
-            cls._cursor_obj = self._cursor_obj.clone()
+            new_qs._cursor_obj = self._cursor_obj.clone()
 
-        return cls
+        return new_qs
 
     def select_related(self, max_depth=1):
         """Handles dereferencing of :class:`~bson.dbref.DBRef` objects or
@@ -760,7 +758,11 @@ class BaseQuerySet(object):
         """
         queryset = self.clone()
         queryset._limit = n if n != 0 else 1
-        # Return self to allow chaining
+
+        # If a cursor object has already been created, apply the limit to it.
+        if queryset._cursor_obj:
+            queryset._cursor_obj.limit(queryset._limit)
+
         return queryset
 
     def skip(self, n):
@@ -771,6 +773,11 @@ class BaseQuerySet(object):
         """
         queryset = self.clone()
         queryset._skip = n
+
+        # If a cursor object has already been created, apply the skip to it.
+        if queryset._cursor_obj:
+            queryset._cursor_obj.skip(queryset._skip)
+
         return queryset
 
     def hint(self, index=None):
@@ -788,6 +795,11 @@ class BaseQuerySet(object):
         """
         queryset = self.clone()
         queryset._hint = index
+
+        # If a cursor object has already been created, apply the hint to it.
+        if queryset._cursor_obj:
+            queryset._cursor_obj.hint(queryset._hint)
+
         return queryset
 
     def batch_size(self, size):
@@ -801,6 +813,11 @@ class BaseQuerySet(object):
         """
         queryset = self.clone()
         queryset._batch_size = size
+
+        # If a cursor object has already been created, apply the batch size to it.
+        if queryset._cursor_obj:
+            queryset._cursor_obj.batch_size(queryset._batch_size)
+
         return queryset
 
     def distinct(self, field):
@@ -972,13 +989,31 @@ class BaseQuerySet(object):
     def order_by(self, *keys):
         """Order the :class:`~mongoengine.queryset.QuerySet` by the keys. The
         order may be specified by prepending each of the keys by a + or a -.
-        Ascending order is assumed.
+        Ascending order is assumed. If no keys are passed, existing ordering
+        is cleared instead.
 
         :param keys: fields to order the query results by; keys may be
             prefixed with **+** or **-** to determine the ordering direction
         """
         queryset = self.clone()
-        queryset._ordering = queryset._get_order_by(keys)
+
+        old_ordering = queryset._ordering
+        new_ordering = queryset._get_order_by(keys)
+
+        if queryset._cursor_obj:
+
+            # If a cursor object has already been created, apply the sort to it
+            if new_ordering:
+                queryset._cursor_obj.sort(new_ordering)
+
+            # If we're trying to clear a previous explicit ordering, we need
+            # to clear the cursor entirely (because PyMongo doesn't allow
+            # clearing an existing sort on a cursor).
+            elif old_ordering:
+                queryset._cursor_obj = None
+
+        queryset._ordering = new_ordering
+
         return queryset
 
     def comment(self, text):
@@ -1424,10 +1459,13 @@ class BaseQuerySet(object):
             raise StopIteration
 
         raw_doc = self._cursor.next()
+
         if self._as_pymongo:
             return self._get_as_pymongo(raw_doc)
-        doc = self._document._from_son(raw_doc,
+
-                                       _auto_dereference=self._auto_dereference, only_fields=self.only_fields)
+        doc = self._document._from_son(
+            raw_doc, _auto_dereference=self._auto_dereference,
+            only_fields=self.only_fields)
 
         if self._scalar:
             return self._get_scalar(doc)
@@ -1437,7 +1475,6 @@ class BaseQuerySet(object):
     def rewind(self):
         """Rewind the cursor to its unevaluated state.
 
-
         .. versionadded:: 0.3
         """
         self._iter = False
@@ -1487,43 +1524,54 @@ class BaseQuerySet(object):
 
     @property
     def _cursor(self):
-        if self._cursor_obj is None:
+        """Return a PyMongo cursor object corresponding to this queryset."""
 
-            # In PyMongo 3+, we define the read preference on a collection
+        # If _cursor_obj already exists, return it immediately.
-            # level, not a cursor level. Thus, we need to get a cloned
+        if self._cursor_obj is not None:
-            # collection object using `with_options` first.
+            return self._cursor_obj
-            if IS_PYMONGO_3 and self._read_preference is not None:
-                self._cursor_obj = self._collection\
-                    .with_options(read_preference=self._read_preference)\
-                    .find(self._query, **self._cursor_args)
-            else:
-                self._cursor_obj = self._collection.find(self._query,
-                                                         **self._cursor_args)
-            # Apply where clauses to cursor
-            if self._where_clause:
-                where_clause = self._sub_js_fields(self._where_clause)
-                self._cursor_obj.where(where_clause)
 
-            if self._ordering:
+        # Create a new PyMongo cursor.
-                # Apply query ordering
+        # XXX In PyMongo 3+, we define the read preference on a collection
-                self._cursor_obj.sort(self._ordering)
+        # level, not a cursor level. Thus, we need to get a cloned collection
-            elif self._ordering is None and self._document._meta['ordering']:
+        # object using `with_options` first.
-                # Otherwise, apply the ordering from the document model, unless
+        if IS_PYMONGO_3 and self._read_preference is not None:
-                # it's been explicitly cleared via order_by with no arguments
+            self._cursor_obj = self._collection\
-                order = self._get_order_by(self._document._meta['ordering'])
+                .with_options(read_preference=self._read_preference)\
-                self._cursor_obj.sort(order)
+                .find(self._query, **self._cursor_args)
+        else:
+            self._cursor_obj = self._collection.find(self._query,
+                                                     **self._cursor_args)
+        # Apply "where" clauses to cursor
+        if self._where_clause:
+            where_clause = self._sub_js_fields(self._where_clause)
+            self._cursor_obj.where(where_clause)
 
-            if self._limit is not None:
+        # Apply ordering to the cursor.
-                self._cursor_obj.limit(self._limit)
+        # XXX self._ordering can be equal to:
+        # * None if we didn't explicitly call order_by on this queryset.
+        # * A list of PyMongo-style sorting tuples.
+        # * An empty list if we explicitly called order_by() without any
+        #   arguments. This indicates that we want to clear the default
+        #   ordering.
+        if self._ordering:
+            # explicit ordering
+            self._cursor_obj.sort(self._ordering)
+        elif self._ordering is None and self._document._meta['ordering']:
+            # default ordering
+            order = self._get_order_by(self._document._meta['ordering'])
+            self._cursor_obj.sort(order)
 
-            if self._skip is not None:
+        if self._limit is not None:
-                self._cursor_obj.skip(self._skip)
+            self._cursor_obj.limit(self._limit)
 
-            if self._hint != -1:
+        if self._skip is not None:
-                self._cursor_obj.hint(self._hint)
+            self._cursor_obj.skip(self._skip)
 
-            if self._batch_size is not None:
+        if self._hint != -1:
-                self._cursor_obj.batch_size(self._batch_size)
+            self._cursor_obj.hint(self._hint)
+
+        if self._batch_size is not None:
+            self._cursor_obj.batch_size(self._batch_size)
 
         return self._cursor_obj
 
@@ -1698,7 +1746,13 @@ class BaseQuerySet(object):
         return ret
 
     def _get_order_by(self, keys):
-        """Creates a list of order by fields"""
+        """Given a list of MongoEngine-style sort keys, return a list
+        of sorting tuples that can be applied to a PyMongo cursor. For
+        example:
+
+        >>> qs._get_order_by(['-last_name', 'first_name'])
+        [('last_name', -1), ('first_name', 1)]
+        """
         key_list = []
         for key in keys:
             if not key:
@@ -1711,17 +1765,19 @@ class BaseQuerySet(object):
             direction = pymongo.ASCENDING
             if key[0] == '-':
                 direction = pymongo.DESCENDING
+
             if key[0] in ('-', '+'):
                 key = key[1:]
+
             key = key.replace('__', '.')
             try:
                 key = self._document._translate_field_name(key)
             except Exception:
+                # TODO this exception should be more specific
                 pass
+
             key_list.append((key, direction))
 
-        if self._cursor_obj and key_list:
-            self._cursor_obj.sort(key_list)
         return key_list
 
     def _get_scalar(self, doc):
@@ -1819,10 +1875,21 @@ class BaseQuerySet(object):
         return code
 
     def _chainable_method(self, method_name, val):
+        """Call a particular method on the PyMongo cursor call a particular chainable method
+        with the provided value.
+        """
         queryset = self.clone()
-        method = getattr(queryset._cursor, method_name)
+
-        method(val)
+        # Get an existing cursor object or create a new one
+        cursor = queryset._cursor
+
+        # Find the requested method on the cursor and call it with the
+        # provided value
+        getattr(cursor, method_name)(val)
+
+        # Cache the value on the queryset._{method_name}
         setattr(queryset, '_' + method_name, val)
+
         return queryset
 
     # Deprecated

mongoengine/queryset/queryset.py

@@ -136,13 +136,15 @@ class QuerySet(BaseQuerySet):
         return self._len
 
     def no_cache(self):
-        """Convert to a non_caching queryset
+        """Convert to a non-caching queryset
 
         .. versionadded:: 0.8.3 Convert to non caching queryset
         """
         if self._result_cache is not None:
             raise OperationError('QuerySet already cached')
-        return self.clone_into(QuerySetNoCache(self._document, self._collection))
+
+        return self._clone_into(QuerySetNoCache(self._document,
+                                                self._collection))
 
 
 class QuerySetNoCache(BaseQuerySet):
@@ -153,7 +155,7 @@ class QuerySetNoCache(BaseQuerySet):
 
         .. versionadded:: 0.8.3 Convert to caching queryset
         """
-        return self.clone_into(QuerySet(self._document, self._collection))
+        return self._clone_into(QuerySet(self._document, self._collection))
 
     def __repr__(self):
         """Provides the string representation of the QuerySet

tests/fields/fields.py

@@ -306,6 +306,24 @@ class FieldTest(unittest.TestCase):
         person.id = '497ce96f395f2f052a494fd4'
         person.validate()
 
+    def test_db_field_validation(self):
+        """Ensure that db_field doesn't accept invalid values."""
+
+        # dot in the name
+        with self.assertRaises(ValueError):
+            class User(Document):
+                name = StringField(db_field='user.name')
+
+        # name starting with $
+        with self.assertRaises(ValueError):
+            class User(Document):
+                name = StringField(db_field='$name')
+
+        # name containing a null character
+        with self.assertRaises(ValueError):
+            class User(Document):
+                name = StringField(db_field='name\0')
+
     def test_string_validation(self):
         """Ensure that invalid values cannot be assigned to string fields.
         """
@@ -1967,7 +1985,7 @@ class FieldTest(unittest.TestCase):
         self.assertEqual(content, User.objects.first().groups[0].content)
 
     def test_reference_miss(self):
-        """Ensure an exception is raised when dereferencing unknow document
+        """Ensure an exception is raised when dereferencing unknown document
         """
 
         class Foo(Document):
@@ -3973,30 +3991,25 @@ class FieldTest(unittest.TestCase):
         """Tests if a `FieldDoesNotExist` exception is raised when trying to
         instanciate a document with a field that's not defined.
         """
-
         class Doc(Document):
-            foo = StringField(db_field='f')
+            foo = StringField()
 
-        def test():
+        with self.assertRaises(FieldDoesNotExist):
             Doc(bar='test')
 
-        self.assertRaises(FieldDoesNotExist, test)
 
     def test_undefined_field_exception_with_strict(self):
         """Tests if a `FieldDoesNotExist` exception is raised when trying to
         instanciate a document with a field that's not defined,
         even when strict is set to False.
         """
-
         class Doc(Document):
-            foo = StringField(db_field='f')
+            foo = StringField()
             meta = {'strict': False}
 
-        def test():
+        with self.assertRaises(FieldDoesNotExist):
             Doc(bar='test')
 
-        self.assertRaises(FieldDoesNotExist, test)
-
     def test_long_field_is_considered_as_int64(self):
         """
         Tests that long fields are stored as long in mongo, even if long value

tests/queryset/queryset.py

@@ -106,58 +106,111 @@ class QuerySetTest(unittest.TestCase):
             list(BlogPost.objects(author2__name="test"))
 
     def test_find(self):
-        """Ensure that a query returns a valid set of results.
+        """Ensure that a query returns a valid set of results."""
-        """
+        user_a = self.Person.objects.create(name='User A', age=20)
-        self.Person(name="User A", age=20).save()
+        user_b = self.Person.objects.create(name='User B', age=30)
-        self.Person(name="User B", age=30).save()
 
         # Find all people in the collection
         people = self.Person.objects
         self.assertEqual(people.count(), 2)
         results = list(people)
+
         self.assertTrue(isinstance(results[0], self.Person))
         self.assertTrue(isinstance(results[0].id, (ObjectId, str, unicode)))
-        self.assertEqual(results[0].name, "User A")
+
+        self.assertEqual(results[0], user_a)
+        self.assertEqual(results[0].name, 'User A')
         self.assertEqual(results[0].age, 20)
-        self.assertEqual(results[1].name, "User B")
+
+        self.assertEqual(results[1], user_b)
+        self.assertEqual(results[1].name, 'User B')
         self.assertEqual(results[1].age, 30)
 
-        # Use a query to filter the people found to just person1
+        # Filter people by age
         people = self.Person.objects(age=20)
         self.assertEqual(people.count(), 1)
         person = people.next()
+        self.assertEqual(person, user_a)
         self.assertEqual(person.name, "User A")
         self.assertEqual(person.age, 20)
 
-        # Test limit
+    def test_limit(self):
+        """Ensure that QuerySet.limit works as expected."""
+        user_a = self.Person.objects.create(name='User A', age=20)
+        user_b = self.Person.objects.create(name='User B', age=30)
+
+        # Test limit on a new queryset
         people = list(self.Person.objects.limit(1))
         self.assertEqual(len(people), 1)
-        self.assertEqual(people[0].name, 'User A')
+        self.assertEqual(people[0], user_a)
 
-        # Test skip
+        # Test limit on an existing queryset
+        people = self.Person.objects
+        self.assertEqual(len(people), 2)
+        people2 = people.limit(1)
+        self.assertEqual(len(people), 2)
+        self.assertEqual(len(people2), 1)
+        self.assertEqual(people2[0], user_a)
+
+        # Test chaining of only after limit
+        person = self.Person.objects().limit(1).only('name').first()
+        self.assertEqual(person, user_a)
+        self.assertEqual(person.name, 'User A')
+        self.assertEqual(person.age, None)
+
+    def test_skip(self):
+        """Ensure that QuerySet.skip works as expected."""
+        user_a = self.Person.objects.create(name='User A', age=20)
+        user_b = self.Person.objects.create(name='User B', age=30)
+
+        # Test skip on a new queryset
         people = list(self.Person.objects.skip(1))
         self.assertEqual(len(people), 1)
-        self.assertEqual(people[0].name, 'User B')
+        self.assertEqual(people[0], user_b)
 
-        person3 = self.Person(name="User C", age=40)
+        # Test skip on an existing queryset
-        person3.save()
+        people = self.Person.objects
+        self.assertEqual(len(people), 2)
+        people2 = people.skip(1)
+        self.assertEqual(len(people), 2)
+        self.assertEqual(len(people2), 1)
+        self.assertEqual(people2[0], user_b)
+
+        # Test chaining of only after skip
+        person = self.Person.objects().skip(1).only('name').first()
+        self.assertEqual(person, user_b)
+        self.assertEqual(person.name, 'User B')
+        self.assertEqual(person.age, None)
+
+    def test_slice(self):
+        """Ensure slicing a queryset works as expected."""
+        user_a = self.Person.objects.create(name='User A', age=20)
+        user_b = self.Person.objects.create(name='User B', age=30)
+        user_c = self.Person.objects.create(name="User C", age=40)
 
         # Test slice limit
         people = list(self.Person.objects[:2])
         self.assertEqual(len(people), 2)
-        self.assertEqual(people[0].name, 'User A')
+        self.assertEqual(people[0], user_a)
-        self.assertEqual(people[1].name, 'User B')
+        self.assertEqual(people[1], user_b)
 
         # Test slice skip
         people = list(self.Person.objects[1:])
         self.assertEqual(len(people), 2)
-        self.assertEqual(people[0].name, 'User B')
+        self.assertEqual(people[0], user_b)
-        self.assertEqual(people[1].name, 'User C')
+        self.assertEqual(people[1], user_c)
 
         # Test slice limit and skip
         people = list(self.Person.objects[1:2])
         self.assertEqual(len(people), 1)
-        self.assertEqual(people[0].name, 'User B')
+        self.assertEqual(people[0], user_b)
+
+        # Test slice limit and skip on an existing queryset
+        people = self.Person.objects
+        self.assertEqual(len(people), 3)
+        people2 = people[1:2]
+        self.assertEqual(len(people2), 1)
+        self.assertEqual(people2[0], user_b)
 
         # Test slice limit and skip cursor reset
         qs = self.Person.objects[1:2]
@@ -168,6 +221,7 @@ class QuerySetTest(unittest.TestCase):
         self.assertEqual(len(people), 1)
         self.assertEqual(people[0].name, 'User B')
 
+        # Test empty slice
         people = list(self.Person.objects[1:1])
         self.assertEqual(len(people), 0)
 
@@ -187,12 +241,6 @@ class QuerySetTest(unittest.TestCase):
         self.assertEqual("[<Person: Person object>, <Person: Person object>]",
                          "%s" % self.Person.objects[51:53])
 
-        # Test only after limit
-        self.assertEqual(self.Person.objects().limit(2).only('name')[0].age, None)
-
-        # Test only after skip
-        self.assertEqual(self.Person.objects().skip(2).only('name')[0].age, None)
-
     def test_find_one(self):
         """Ensure that a query using find_one returns a valid result.
         """
@@ -1226,6 +1274,7 @@ class QuerySetTest(unittest.TestCase):
 
         BlogPost.drop_collection()
 
+        # default ordering should be used by default
         with db_ops_tracker() as q:
             BlogPost.objects.filter(title='whatever').first()
             self.assertEqual(len(q.get_ops()), 1)
@@ -1234,11 +1283,28 @@ class QuerySetTest(unittest.TestCase):
                 {'published_date': -1}
             )
 
+        # calling order_by() should clear the default ordering
         with db_ops_tracker() as q:
             BlogPost.objects.filter(title='whatever').order_by().first()
             self.assertEqual(len(q.get_ops()), 1)
             self.assertFalse('$orderby' in q.get_ops()[0]['query'])
 
+        # calling an explicit order_by should use a specified sort
+        with db_ops_tracker() as q:
+            BlogPost.objects.filter(title='whatever').order_by('published_date').first()
+            self.assertEqual(len(q.get_ops()), 1)
+            self.assertEqual(
+                q.get_ops()[0]['query']['$orderby'],
+                {'published_date': 1}
+            )
+
+        # calling order_by() after an explicit sort should clear it
+        with db_ops_tracker() as q:
+            qs = BlogPost.objects.filter(title='whatever').order_by('published_date')
+            qs.order_by().first()
+            self.assertEqual(len(q.get_ops()), 1)
+            self.assertFalse('$orderby' in q.get_ops()[0]['query'])
+
     def test_no_ordering_for_get(self):
         """ Ensure that Doc.objects.get doesn't use any ordering.
         """

tests/test_connection.py

@@ -296,6 +296,19 @@ class ConnectionTest(unittest.TestCase):
         conn = get_connection('t2')
         self.assertFalse(get_tz_awareness(conn))
 
+    def test_write_concern(self):
+        """Ensure write concern can be specified in connect() via
+        a kwarg or as part of the connection URI.
+        """
+        conn1 = connect(alias='conn1', host='mongodb://localhost/testing?w=1&j=true')
+        conn2 = connect('testing', alias='conn2', w=1, j=True)
+        if IS_PYMONGO_3:
+            self.assertEqual(conn1.write_concern.document, {'w': 1, 'j': True})
+            self.assertEqual(conn2.write_concern.document, {'w': 1, 'j': True})
+        else:
+            self.assertEqual(dict(conn1.write_concern), {'w': 1, 'j': True})
+            self.assertEqual(dict(conn2.write_concern), {'w': 1, 'j': True})
+
     def test_datetime(self):
         connect('mongoenginetest', tz_aware=True)
         d = datetime.datetime(2010, 5, 5, tzinfo=utc)

tox.ini

@@ -1,5 +1,5 @@
 [tox]
-envlist = {py26,py27,py33,py34,py35,pypy,pypy3}-{mg27,mg28},flake8
+envlist = {py27,py33,py34,py35,pypy,pypy3}-{mg27,mg28,mg30},flake8
 
 [testenv]
 commands =
@@ -7,12 +7,10 @@ commands =
 deps =
     nose
     mg27: PyMongo<2.8
-    mg28: PyMongo>=2.8,<3.0
+    mg28: PyMongo>=2.8,<2.9
     mg30: PyMongo>=3.0
-    mgdev: https://github.com/mongodb/mongo-python-driver/tarball/master
 setenv =
     PYTHON_EGG_CACHE = {envdir}/python-eggs
-passenv = windir
 
 [testenv:flake8]
 deps =