Merge branch 'issue_#1271'

CHANGES.rst

@@ -10,9 +10,9 @@ New
 ~~~~~
 - ``NORMALIZE_ON_PATCH`` switches normalization on patch requests (`#1234`_)
 
-
 Fixed
 ~~~~~
+- Document count broken with concurrent requests (`#1271`_)
 - If ``ignore_fields`` contains a nested field, document is mutated (`#1266`_)
 - Crash with Werzeug >= 0.15.3 (`#1267`_)
 - Fix crash when trying to ignore a nested field that doesn't exist (`#1263`_)
@@ -22,6 +22,19 @@ Improved
 - Remove unsupported ``transparent_schema_rules`` option from docs (`#1264`_)
 - Bump (and pin) Wekzeug to 0.15.4 (`#1267`_)
 
+Breaking Changes
+~~~~~~~~~~~~~~~~
+
+No known breaking changes for the standard framework user. However, if you are
+consuming the developer API:
+
+- Be aware that ``io.base.DataLayer.find()`` signature has changed and an
+  optional ``perform_count`` argument has been added. The method return value
+  is now a tuple ``(cursor, count)``; ``cursor`` is the query result as
+  before while ``count`` is the document count, which is expected to have a
+  consistent value when ``perform_count = True``.
+
+.. _`#1271`: https://github.com/pyeve/eve/issues/1271
 .. _`#1266`: https://github.com/pyeve/eve/pull/1266
 .. _`#1234`: https://github.com/pyeve/eve/issues/1234
 .. _`#1267`: https://github.com/pyeve/eve/issues/1267

eve/io/base.py

@@ -117,7 +117,7 @@ def init_app(self, app):
         """
         raise NotImplementedError
 
-    def find(self, resource, req, sub_resource_lookup):
+    def find(self, resource, req, sub_resource_lookup, perform_count=True):
         """ Retrieves a set of documents (rows), matching the current request.
         Consumed when a request hits a collection/document endpoint
         (`/people/`).
@@ -134,6 +134,8 @@ def find(self, resource, req, sub_resource_lookup):
                     to support with your driver. For example ``eve.io.Mongo``
                     supports both Python and Mongo-like query syntaxes.
         :param sub_resource_lookup: sub-resource lookup from the endpoint url.
+        :param perform_count: wether a document count should be performed and
+                              returned to the client.
 
         .. versionchanged:: 0.3
            Support for sub-resources.

eve/io/mongo/mongo.py

@@ -141,7 +141,7 @@ def init_app(self, app):
         self.driver = PyMongos(self)
         self.mongo_prefix = None
 
-    def find(self, resource, req, sub_resource_lookup):
+    def find(self, resource, req, sub_resource_lookup, perform_count=True):
         """ Retrieves a set of documents matching a given request. Queries can
         be expressed in two different formats: the mongo query syntax, and the
         python syntax. The first kind of query would look like: ::
@@ -258,40 +258,37 @@ def find(self, resource, req, sub_resource_lookup):
         if projection:
             args["projection"] = projection
 
-        self.__last_target = self.pymongo(resource).db[datasource], spec
+        target = self.pymongo(resource).db[datasource]
         try:
-            self.__last_cursor = self.pymongo(resource).db[datasource].find(**args)
+            result = target.find(**args)
         except TypeError as e:
             # pymongo raises ValueError when invalid query paramenters are
             # included. We do our best to catch them beforehand but, especially
             # with key/value sort syntax, invalid ones might still slip in.
             self.app.logger.exception(e)
             abort(400, description=debug_error_message(str(e)))
 
-        return self.__last_cursor
-
-    @property
-    def last_documents_count(self):
-        if not self.__last_target:
-            return None
+        if perform_count:
+            try:
+                count = target.count_documents(spec)
+            except:
+                # fallback to deprecated method. this might happen when the query
+                # includes operators not supported by count_documents(). one
+                # documented use-case is when we're running on mongo 3.4 and below,
+                # which does not support $expr ($expr must replace $where # in
+                # count_documents()).
+
+                # 1. Mongo 3.6+; $expr: pass
+                # 2. Mongo 3.6+; $where: pass (via fallback)
+                # 3. Mongo 3.4; $where: pass (via fallback)
+                # 4. Mongo 3.4; $expr: fail (operator not supported by db)
+
+                # See: http://api.mongodb.com/python/current/api/pymongo/collection.html#pymongo.collection.Collection.count
+                count = target.count()
+        else:
+            count = None
 
-        try:
-            target, spec = self.__last_target
-            return target.count_documents(spec)
-        except:
-            # fallback to deprecated method. this might happen when the query
-            # includes operators not supported by count_documents(). one
-            # documented use-case is when we're running on mongo 3.4 and below,
-            # which does not support $expr ($expr must replace $where # in
-            # count_documents()).
-
-            # 1. Mongo 3.6+; $expr: pass
-            # 2. Mongo 3.6+; $where: pass (via fallback)
-            # 3. Mongo 3.4; $where: pass (via fallback)
-            # 4. Mongo 3.4; $expr: fail (operator not supported by db)
-
-            # See: http://api.mongodb.com/python/current/api/pymongo/collection.html#pymongo.collection.Collection.count
-            return self.__last_cursor.count()
+        return result, count
 
     def find_one(
         self,

eve/methods/common.py

@@ -849,7 +849,7 @@ def embedded_document(references, data_relation, field_name):
     :param data_relation: the relation schema definition.
     :param field_name: field name used in abort message only
 
-    .. versionadded:: 0.5
+)    .. versionadded:: 0.5
     """
     embedded_docs = []
 
@@ -892,9 +892,10 @@ def embedded_document(references, data_relation, field_name):
             data_relation, references
         )
         for subresource in subresources_query:
-            list_embedded_doc = list(
-                app.data.find(subresource, None, subresources_query[subresource])
+            result, _ = app.data.find(
+                subresource, None, subresources_query[subresource]
             )
+            list_embedded_doc = list(result)
 
             if not list_embedded_doc:
                 embedded_docs.extend(

eve/methods/delete.py

@@ -217,7 +217,8 @@ def delete(resource, **lookup):
         # get_document should always fetch soft deleted documents from the db
         # callers must handle soft deleted documents
         default_request.show_deleted = True
-    originals = list(app.data.find(resource, default_request, lookup))
+    result, _ = app.data.find(resource, default_request, lookup)
+    originals = list(result)
     if not originals:
         abort(404)
     # I add new callback as I want the framework to be retro-compatible

eve/methods/get.py

@@ -262,7 +262,9 @@ def _perform_find(resource, lookup):
     # If-Modified-Since disabled on collections (#334)
     req.if_modified_since = None
 
-    cursor = app.data.find(resource, req, lookup)
+    cursor, count = app.data.find(
+        resource, req, lookup, perform_count=not config.OPTIMIZE_PAGINATION_FOR_SPEED
+    )
     # If soft delete is enabled, data.find will not include items marked
     # deleted unless req.show_deleted is True
     for document in cursor:
@@ -279,10 +281,7 @@ def _perform_find(resource, lookup):
 
     response[config.ITEMS] = documents
 
-    if config.OPTIMIZE_PAGINATION_FOR_SPEED:
-        count = None
-    else:
-        count = app.data.last_documents_count
+    if count:
         headers.append((config.HEADER_TOTAL_COUNT, count))
 
     if config.DOMAIN[resource]["hateoas"]:
@@ -454,11 +453,11 @@ def getitem_internal(resource, **lookup):
             # default sort for 'all', required sort for 'diffs'
             req.sort = '[("%s", 1)]' % config.VERSION
         req.if_modified_since = None  # we always want the full history here
-        cursor = app.data.find(resource + config.VERSIONS, req, lookup)
+        cursor, count = app.data.find(resource + config.VERSIONS, req, lookup)
 
         # build all versions
         documents = []
-        if app.data.last_documents_count == 0:
+        if count == 0:
             # this is the scenario when the document existed before
             # document versioning got turned on
             documents.append(latest_doc)
@@ -510,7 +509,6 @@ def getitem_internal(resource, **lookup):
     if config.DOMAIN[resource]["hateoas"]:
         # use the id of the latest document for multi-document requests
         if cursor:
-            count = app.data.last_documents_count
             response[config.LINKS] = _pagination_links(
                 resource, req, count, latest_doc[resource_def["id_field"]]
             )

eve/tests/methods/delete.py

@@ -559,24 +559,24 @@ def test_softdelete_datalayer(self):
             # show_deleted == True is passed or if the deleted field is part of
             # the lookup
             req.show_deleted = False
-            self.app.data.find(self.known_resource, req, None)
-            undeleted_count = self.app.data.last_documents_count
+            _, undeleted_count = self.app.data.find(self.known_resource, req, None)
 
             req.show_deleted = True
-            self.app.data.find(self.known_resource, req, None)
-            self.assertEqual(undeleted_count, self.app.data.last_documents_count - 1)
+            _, challenge = self.app.data.find(self.known_resource, req, None)
+            self.assertEqual(undeleted_count, challenge - 1)
 
             req.show_deleted = False
-            self.app.data.find(self.known_resource, req, {self.deleted_field: True})
-            deleted_count = self.app.data.last_documents_count
+            _, deleted_count = self.app.data.find(
+                self.known_resource, req, {self.deleted_field: True}
+            )
             self.assertEqual(deleted_count, 1)
 
             # find_list_of_ids will return deleted documents if given their id
-            self.app.data.find_list_of_ids(
+            ids = self.app.data.find_list_of_ids(
                 self.known_resource, [ObjectId(self.item_id)]
             )
 
-            self.assertEqual(self.app.data.last_documents_count, 1)
+            self.assertEqual(str(ids[0]["_id"]), self.item_id)
 
     def test_softdelete_db_fields(self):
         """Documents created when soft delete is enabled should include and

eve/tests/methods/get.py

@@ -1051,13 +1051,13 @@ def test_cursor_extra_find(self):
         _find = self.app.data.find
         hits = {"total_hits": 0}
 
-        def find(resource, req, sub_resource_lookup):
+        def find(resource, req, sub_resource_lookup, perform_count=True):
             def extra(response):
                 response["_hits"] = hits
 
-            cursor = _find(resource, req, sub_resource_lookup)
+            cursor, _ = _find(resource, req, sub_resource_lookup)
             cursor.extra = extra
-            return cursor
+            return cursor, _
 
         self.app.data.find = find
         r, status = self.get(self.known_resource)