Merge pull request #394 from cloudant/393-conflict-retry

Stopped raising exception on conflict retry success

Author: ricellis

CHANGES.md

@@ -1,6 +1,7 @@
 # Unreleased
 
 - [NEW] Add new view parameters, `stable` and `update`, as keyword arguments to `get_view_result`.
+- [FIXED] Case where an exception was raised after successful retry when using `doc.update_field`.
 
 # 2.9.0 (2018-06-13)
 

src/cloudant/document.py

@@ -259,7 +259,8 @@ def _update_field(self, action, field, value, max_tries, tries=0):
             if tries < max_tries and ex.response.status_code == 409:
                 self._update_field(
                     action, field, value, max_tries, tries=tries+1)
-            raise
+            else:
+                raise
 
     def update_field(self, action, field, value, max_tries=10):
         """

tests/unit/document_tests.py

@@ -474,6 +474,62 @@ def test_update_field(self):
         self.assertTrue(doc['_rev'].startswith('2-'))
         self.assertEqual(doc['pets'], ['cat', 'dog', 'fish'])
 
+    @mock.patch('cloudant.document.Document.save')
+    def test_update_field_maxretries(self, m_save):
+        """
+        Test that conflict retries work for updating a single field.
+        """
+        # Create a doc
+        doc = Document(self.db, 'julia006')
+        doc['name'] = 'julia'
+        doc['age'] = 6
+        doc.create()
+        self.assertTrue(doc['_rev'].startswith('1-'))
+        self.assertEqual(doc['age'], 6)
+        # Mock conflicts when saving updates
+        m_save.side_effect = requests.HTTPError(response=mock.Mock(status_code=409, reason='conflict'))
+        # Tests that failing on retry eventually throws
+        with self.assertRaises(requests.HTTPError) as cm:
+            doc.update_field(doc.field_set, 'age', 7, max_tries=2)
+
+        # There is an off-by-one error for "max_tries"
+        # It really means max_retries i.e. 1 attempt
+        # followed by a max of 2 retries
+        self.assertEqual(m_save.call_count, 3)
+        self.assertEqual(cm.exception.response.status_code, 409)
+        self.assertEqual(cm.exception.response.reason, 'conflict')
+        # Fetch again before asserting, otherwise we assert against
+        # the locally updated age field
+        doc.fetch()
+        self.assertFalse(doc['_rev'].startswith('2-'))
+        self.assertNotEqual(doc['age'], 7)
+
+    def test_update_field_success_on_retry(self):
+        """
+        Test that conflict retries work for updating a single field.
+        """
+        # Create a doc
+        doc = Document(self.db, 'julia006')
+        doc['name'] = 'julia'
+        doc['age'] = 6
+        doc.create()
+        self.assertTrue(doc['_rev'].startswith('1-'))
+        self.assertEqual(doc['age'], 6)
+
+        # Mock when saving the document
+        # 1st call throw a 409
+        # 2nd call delegate to the real doc.save()
+        with mock.patch('cloudant.document.Document.save',
+                        side_effect=[requests.HTTPError(response=mock.Mock(status_code=409, reason='conflict')),
+                                     doc.save()]) as m_save:
+            # A list of side effects containing only 1 element
+            doc.update_field(doc.field_set, 'age', 7, max_tries=1)
+        # Two calls to save, one with a 409 and one that succeeds
+        self.assertEqual(m_save.call_count, 2)
+        # Check that the _rev and age field were updated
+        self.assertTrue(doc['_rev'].startswith('2-'))
+        self.assertEqual(doc['age'], 7)
+
     def test_delete_document_failure(self):
         """
         Test failure condition when attempting to remove a document