CXX-1258 Keep cursor iterators in lockstep at end

This commit fixes a bug where iterators might not always compare
equal to cursor.end() when the cursor was actually exhausted.

It adds extensive tests for cursor lockstep and invariants.

It also substantially revises cursor/iterator documentation for
exhaustion, equality comparison, and tailable cursor behavior.

(cherry picked from commit 667a833)

Author: xdg

examples/mongocxx/CMakeLists.txt

@@ -35,6 +35,7 @@ set(MONGOCXX_EXAMPLES
     query.cpp
     query_projection.cpp
     remove.cpp
+    tailable_cursor.cpp
     update.cpp
     view_or_value_variant.cpp
 )

examples/mongocxx/tailable_cursor.cpp

@@ -0,0 +1,114 @@
+// Copyright 2017 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.
+
+#include <chrono>
+#include <iostream>
+#include <thread>
+
+#include <bsoncxx/builder/basic/document.hpp>
+#include <bsoncxx/json.hpp>
+
+#include <mongocxx/client.hpp>
+#include <mongocxx/collection.hpp>
+#include <mongocxx/database.hpp>
+#include <mongocxx/instance.hpp>
+#include <mongocxx/options/create_collection.hpp>
+#include <mongocxx/options/find.hpp>
+#include <mongocxx/uri.hpp>
+
+using bsoncxx::builder::basic::document;
+using bsoncxx::builder::basic::kvp;
+
+//
+// Document number counter for sample inserted documents.  This just
+// makes the tailed document more obviously in sequence.
+//
+
+static std::int32_t counter = 0;
+
+//
+// Drop and recreate capped collection. Inserts a doc as tailing an empty
+// collection returns a closed cursor.
+//
+// TODO CDRIVER-2093: After CDRIVER-2093 is fixed, we can provide better
+// diagnostics as to whether the cursor is alive or closed and won't need
+// to prime the collection with a document.
+//
+void init_capped_collection(mongocxx::client* conn, std::string name) {
+    auto db = (*conn)["test"];
+    auto coll = db[name];
+
+    coll.drop();
+    auto create_opts = mongocxx::options::create_collection{}.capped(true).size(1024 * 1024);
+    db.create_collection(name, create_opts);
+
+    document builder{};
+    builder.append(kvp("n", counter++));
+    db[name].insert_one(builder.extract());
+}
+
+//
+// Insert 5 documents so there are more documents to tail.
+//
+void insert_docs(mongocxx::collection* coll) {
+    std::cout << "Inserting batch... " << std::endl;
+    for (int j = 0; j < 5; j++) {
+        document builder{};
+        builder.append(kvp("n", counter++));
+        coll->insert_one(builder.extract());
+    }
+}
+
+int main(int, char**) {
+    // The mongocxx::instance constructor and destructor initialize and shut down the driver,
+    // respectively. Therefore, a mongocxx::instance must be created before using the driver and
+    // must remain alive for as long as the driver is in use.
+    mongocxx::instance inst{};
+    mongocxx::client conn{mongocxx::uri{}};
+    std::string name{"capped_coll"};
+
+    // Create the capped collection.
+    init_capped_collection(&conn, name);
+
+    // Construct a tailable cursor.
+    auto coll = conn["test"][name];
+    mongocxx::options::find opts{};
+    opts.cursor_type(mongocxx::cursor::type::k_tailable);
+    auto cursor = coll.find({}, opts);
+
+    // Loop "forever", or in this case, until we find >= 25 documents.
+    std::cout << "Tailing the collection..." << std::endl;
+    int docs_found = 0;
+    for (;;) {
+        // Loop over the cursor until no more documents are available.
+        // On the next iteration of the outer loop, if more documents
+        // are available, the tailable cursor will return them.
+        for (auto&& doc : cursor) {
+            std::cout << bsoncxx::to_json(doc) << std::endl;
+            docs_found++;
+        }
+
+        if (docs_found >= 25) {
+            break;
+        }
+
+        // No documents are available, so sleep a bit before trying again.
+        std::this_thread::sleep_for(std::chrono::milliseconds(100));
+
+        // For the sake of this example, add more documents before the next loop.
+        insert_docs(&coll);
+    }
+
+    return EXIT_SUCCESS;
+}

src/mongocxx/cursor.cpp

@@ -57,7 +57,6 @@ cursor::iterator& cursor::iterator::operator++() {
         throw_exception<query_exception>(error);
     } else {
         _cursor->_impl->mark_nothing_left();
-        _cursor = nullptr;  // Set iterator equal to end().
     }
     return *this;
 }
@@ -82,6 +81,14 @@ cursor::iterator::iterator(cursor* cursor) : _cursor(cursor) {
     operator++();
 }
 
+//
+// An iterator is exhausted if it is the end-iterator (_cursor == nullptr)
+// or if the underlying _cursor is marked exhausted.
+//
+bool cursor::iterator::is_exhausted() const {
+    return !_cursor || _cursor->_impl->is_exhausted();
+}
+
 const bsoncxx::document::view& cursor::iterator::operator*() const {
     return _cursor->_impl->doc;
 }
@@ -90,8 +97,13 @@ const bsoncxx::document::view* cursor::iterator::operator->() const {
     return &_cursor->_impl->doc;
 }
 
+//
+// Iterators are equal if they point to the same underlying _cursor or if they
+// both are "at the end".  We check for exhaustion first because the most
+// common check is `iter != cursor.end()`.
+//
 bool operator==(const cursor::iterator& lhs, const cursor::iterator& rhs) {
-    return lhs._cursor == rhs._cursor;
+    return ((rhs.is_exhausted() && lhs.is_exhausted()) || (lhs._cursor == rhs._cursor));
 }
 
 bool operator!=(const cursor::iterator& lhs, const cursor::iterator& rhs) {

src/mongocxx/cursor.hpp

@@ -60,17 +60,19 @@ class MONGOCXX_API cursor {
     /// returned points to the next remaining result, not the result of
     /// the original call to begin().
     ///
+    /// For a tailable cursor, when cursor.begin() == cursor.end(), no
+    /// documents are available.  Each call to cursor.begin() checks again
+    /// for newly-available documents.
+    ///
     /// @return the cursor::iterator
     ///
     /// @throws mongocxx::query_exception if the query failed
     ///
     iterator begin();
 
     ///
-    /// A cursor::iterator that points to the end of the results.  In the
-    /// case of a tailable cursor, this iterator will compare equal to an
-    /// exhausted tailable cursor iterator, even if more results are available
-    /// the next time the cursor is iterated.
+    /// A cursor::iterator indicating cursor exhaustion, meaning that
+    /// no documents are available from the cursor.
     ///
     /// @return the cursor::iterator
     ///
@@ -80,6 +82,7 @@ class MONGOCXX_API cursor {
     friend class collection;
     friend class client;
     friend class database;
+    friend class cursor::iterator;
 
     MONGOCXX_PRIVATE cursor(void* cursor_ptr,
                             bsoncxx::stdx::optional<type> cursor_type = bsoncxx::stdx::nullopt);
@@ -92,10 +95,19 @@ class MONGOCXX_API cursor {
 /// Class representing an input iterator of documents in a MongoDB cursor
 /// result set.
 ///
-/// All non-empty iterators derived from the same mongocxx::cursor move in
-/// lock-step.  Dereferencing any non-empty iterator always gives the first
-/// remaining document in the cursor.  Incrementing one iterator is equivalent
-/// to incrementing them all.
+/// All non-end iterators derived from the same mongocxx::cursor move in
+/// lock-step.  Dereferencing any non-end() iterator always gives the first
+/// remaining document in the cursor.  Incrementing one non-end iterator is
+/// equivalent to incrementing them all.
+///
+/// An iterator is 'exhausted' when no documents are available. An
+/// end-iterator is always exhausted. A non-end iterator is exhausted when the
+/// originating mongocxx::cursor has no more documents.  When an iterator is
+/// exhausted, it must not be dereferenced or incremented.
+///
+/// For iterators of a tailable cursor, calling cursor.begin() may revive an
+/// exhausted iterator so that it no longer compares equal to the
+/// end-iterator.
 ///
 class MONGOCXX_API cursor::iterator
     : public std::iterator<std::input_iterator_tag, bsoncxx::document::view> {
@@ -130,7 +142,8 @@ class MONGOCXX_API cursor::iterator
     ///
     /// @{
     ///
-    /// Compare two iterators for (in)-equality
+    /// Compare two iterators for (in)-equality.  Iterators compare equal if
+    /// they point to the same underlying cursor or if both are exhausted.
     ///
     /// @relates iterator
     ///
@@ -140,6 +153,8 @@ class MONGOCXX_API cursor::iterator
     /// @}
     ///
 
+    MONGOCXX_PRIVATE bool is_exhausted() const;
+
     MONGOCXX_PRIVATE explicit iterator(cursor* cursor);
 
     // If this pointer is null, the iterator is considered "past-the-end".

src/mongocxx/private/cursor.hh

@@ -34,6 +34,7 @@ class cursor::impl {
     impl(mongoc_cursor_t* cursor, bsoncxx::stdx::optional<cursor::type> cursor_type)
         : cursor_t(cursor),
           status{cursor ? state::k_pending : state::k_dead},
+          exhausted(!cursor),
           tailable{cursor && cursor_type && (*cursor_type == cursor::type::k_tailable ||
                                              *cursor_type == cursor::type::k_tailable_await)} {
     }
@@ -50,6 +51,10 @@ class cursor::impl {
         return status == state::k_dead;
     }
 
+    bool is_exhausted() const {
+        return exhausted;
+    }
+
     bool is_tailable() const {
         return tailable;
     }
@@ -61,16 +66,19 @@ class cursor::impl {
 
     void mark_nothing_left() {
         doc = bsoncxx::document::view{};
+        exhausted = true;
         status = tailable ? state::k_pending : state::k_dead;
     }
 
     void mark_started() {
         status = state::k_started;
+        exhausted = false;
     }
 
     mongoc_cursor_t* cursor_t;
     bsoncxx::document::view doc;
     state status;
+    bool exhausted;
     bool tailable;
 };