Merge pull request #434 from cloudant/fix-document-cm-error-bug

Stop document context manager from saving if an error is raised.

Author: smithsz

CHANGES.md

@@ -1,7 +1,9 @@
 # Unreleased
 
-- [IMPROVED] Updated `Getting started` section with a `get_query_result` example. 
+- [FIXED] Bug where document context manager performed remote save despite
+  uncaught exceptions being raised inside `with` block.
 - [FIXED] Fixed parameter type of `selector` in docstring.
+- [IMPROVED] Updated `Getting started` section with a `get_query_result` example.
 
 # 2.11.0 (2019-01-21)
 

docs/conf.py

@@ -21,6 +21,8 @@
 # documentation root, use os.path.abspath to make it absolute, like shown here.
 #sys.path.insert(0, os.path.abspath('.'))
 
+sys.path.insert(0, os.path.abspath('../src'))
+
 # -- General configuration ------------------------------------------------
 
 # If your documentation needs a minimal Sphinx version, state it here.

docs/getting_started.rst

@@ -441,41 +441,45 @@ multiple updates to a single document. Note that we don't save to the server
 after each update. We only save once to the server upon exiting the ``Document``
 context manager.
 
- .. code-block:: python
+.. warning:: Uncaught exceptions inside the ``with`` block will prevent your
+             document changes being saved to the remote server. However, changes
+             will still be applied to your local document object.
 
-    from cloudant import cloudant
-    from cloudant.document import Document
+.. code-block:: python
 
-    with cloudant(USERNAME, PASSWORD, account=ACCOUNT_NAME) as client:
+   from cloudant import cloudant
+   from cloudant.document import Document
 
-        my_database = client.create_database('my_database')
+   with cloudant(USERNAME, PASSWORD, account=ACCOUNT_NAME) as client:
 
-        # Upon entry into the document context, fetches the document from the
-        # remote database, if it exists. Upon exit from the context, saves the
-        # document to the remote database with changes made within the context
-        # or creates a new document.
-        with Document(database, 'julia006') as document:
-            # If document exists, it's fetched from the remote database
-            # Changes are made locally
-            document['name'] = 'Julia'
-            document['age'] = 6
-            # The document is saved to the remote database
-
-        # Display a Document
-        print(my_database['julia30'])
-    
-        # Delete the database
-        client.delete_database('my_database')
+       my_database = client.create_database('my_database')
 
-        print('Databases: {0}'.format(client.all_dbs()))
+       # Upon entry into the document context, fetches the document from the
+       # remote database, if it exists. Upon exit from the context, saves the
+       # document to the remote database with changes made within the context
+       # or creates a new document.
+       with Document(database, 'julia006') as document:
+           # If document exists, it's fetched from the remote database
+           # Changes are made locally
+           document['name'] = 'Julia'
+           document['age'] = 6
+           # The document is saved to the remote database
+
+       # Display a Document
+       print(my_database['julia30'])
+
+       # Delete the database
+       client.delete_database('my_database')
+
+       print('Databases: {0}'.format(client.all_dbs()))
 
 Always use the ``_deleted`` document property to delete a document from within
 a ``Document`` context manager. For example:
 
- .. code-block:: python
+.. code-block:: python
 
-    with Document(my_database, 'julia30') as doc:
-        doc['_deleted'] = True
+   with Document(my_database, 'julia30') as doc:
+       doc['_deleted'] = True
 
 *You can also delete non underscore prefixed document keys to reduce the size of the request.*
 

src/cloudant/document.py

@@ -341,12 +341,13 @@ def __enter__(self):
 
         return self
 
-    def __exit__(self, *args):
+    def __exit__(self, exc_type, exc_value, traceback):
         """
-        Support context like editing of document fields.  Handles context exit
-        logic.  Executes a Document.save() upon exit.
+        Support context like editing of document fields. Handles context exit
+        logic. Executes a `Document.save()` upon exit if no exception occurred.
         """
-        self.save()
+        if exc_type is None:
+            self.save()
 
     def __setitem__(self, key, value):
         """

tests/unit/document_tests.py

@@ -622,6 +622,47 @@ def test_document_context_manager_no_doc_id(self):
         self.assertTrue(doc['_rev'].startswith('1-'))
         self.assertEqual(self.db['julia006'], doc)
 
+    def test_document_context_manager_creation_failure_on_error(self):
+        """
+        Test that the document context manager skips document creation if there
+        is an error.
+        """
+        with self.assertRaises(ZeroDivisionError), Document(self.db, 'julia006') as doc:
+            doc['name'] = 'julia'
+            doc['age'] = 6
+            raise ZeroDivisionError()
+
+        doc = Document(self.db, 'julia006')
+        try:
+            doc.fetch()
+        except requests.HTTPError as err:
+            self.assertEqual(err.response.status_code, 404)
+        else:
+            self.fail('Above statement should raise a HTTPError.')
+
+    def test_document_context_manager_update_failure_on_error(self):
+        """
+        Test that the document context manager skips document update if there
+        is an error.
+        """
+        # Create the document.
+        doc = Document(self.db, 'julia006')
+        doc['name'] = 'julia'
+        doc['age'] = 6
+        doc.save()
+
+        # Make a document update and then raise an error.
+        with self.assertRaises(ZeroDivisionError), Document(self.db, 'julia006') as doc:
+            doc['age'] = 7
+            raise ZeroDivisionError()
+
+        # Assert the change persists locally.
+        self.assertEqual(doc['age'], 7)
+
+        # Assert the document has not been saved to remote server.
+        self.assertTrue(doc['_rev'].startswith('1-'))
+        self.assertEqual(self.db['julia006']['age'], 6)
+
     def test_document_context_manager_doc_create(self):
         """
         Test that the document context manager will create a doc if it does