From 3e000f9be1d7e4fdd93317b6da0ab4a0e07b5431 Mon Sep 17 00:00:00 2001 From: Emmanuel Leblond Date: Thu, 21 May 2015 17:35:51 +0200 Subject: [PATCH 1/7] Raise error if save_condition fails #991 --- mongoengine/document.py | 6 +++++- tests/document/instance.py | 11 +++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/mongoengine/document.py b/mongoengine/document.py index 060919ca..2c4872e9 100644 --- a/mongoengine/document.py +++ b/mongoengine/document.py @@ -266,7 +266,8 @@ class Document(BaseDocument): to cascading saves. Implies ``cascade=True``. :param _refs: A list of processed references used in cascading saves :param save_condition: only perform save if matching record in db - satisfies condition(s) (e.g., version number) + satisfies condition(s) (e.g., version number). + Raises :class:`OperationError` if the conditions are not satisfied .. versionchanged:: 0.5 In existing documents it only saves changed fields using @@ -348,6 +349,9 @@ class Document(BaseDocument): upsert = save_condition is None last_error = collection.update(select_dict, update_query, upsert=upsert, **write_concern) + if save_condition is not None and last_error['nModified'] == 0: + raise OperationError('Race condition preventing' + ' document update detected') created = is_new_object(last_error) if cascade is None: diff --git a/tests/document/instance.py b/tests/document/instance.py index 2cfdef65..3ccabacd 100644 --- a/tests/document/instance.py +++ b/tests/document/instance.py @@ -954,11 +954,12 @@ class InstanceTest(unittest.TestCase): self.assertEqual(w1.save_id, UUID(1)) self.assertEqual(w1.count, 0) - # mismatch in save_condition prevents save + # mismatch in save_condition prevents save and raise exception flip(w1) self.assertTrue(w1.toggle) self.assertEqual(w1.count, 1) - w1.save(save_condition={'save_id': UUID(42)}) + self.assertRaises(OperationError, + w1.save, save_condition={'save_id': UUID(42)}) w1.reload() self.assertFalse(w1.toggle) self.assertEqual(w1.count, 0) @@ -986,7 +987,8 @@ class InstanceTest(unittest.TestCase): self.assertEqual(w1.count, 2) flip(w2) flip(w2) - w2.save(save_condition={'save_id': old_id}) + self.assertRaises(OperationError, + w2.save, save_condition={'save_id': old_id}) w2.reload() self.assertFalse(w2.toggle) self.assertEqual(w2.count, 2) @@ -998,7 +1000,8 @@ class InstanceTest(unittest.TestCase): self.assertTrue(w1.toggle) self.assertEqual(w1.count, 3) flip(w1) - w1.save(save_condition={'count__gte': w1.count}) + self.assertRaises(OperationError, + w1.save, save_condition={'count__gte': w1.count}) w1.reload() self.assertTrue(w1.toggle) self.assertEqual(w1.count, 3) From 682db9b81f24d9160bc5707296ac3a1b507f200a Mon Sep 17 00:00:00 2001 From: Emmanuel Leblond Date: Tue, 2 Jun 2015 17:44:57 +0200 Subject: [PATCH 2/7] Add versionchanged to document save_condition --- mongoengine/document.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/mongoengine/document.py b/mongoengine/document.py index 2c4872e9..f798780e 100644 --- a/mongoengine/document.py +++ b/mongoengine/document.py @@ -266,7 +266,7 @@ class Document(BaseDocument): to cascading saves. Implies ``cascade=True``. :param _refs: A list of processed references used in cascading saves :param save_condition: only perform save if matching record in db - satisfies condition(s) (e.g., version number). + satisfies condition(s) (e.g. version number). Raises :class:`OperationError` if the conditions are not satisfied .. versionchanged:: 0.5 @@ -285,6 +285,8 @@ class Document(BaseDocument): .. versionchanged:: 0.8.5 Optional save_condition that only overwrites existing documents if the condition is satisfied in the current db record. + .. versionchanged:: 0.10 + :class:`OperationError` exception raised if save_condition fails. """ signals.pre_save.send(self.__class__, document=self) From cca0222e1d415346eb1ee439ddba21daaf530851 Mon Sep 17 00:00:00 2001 From: Emmanuel Leblond Date: Tue, 2 Jun 2015 17:45:10 +0200 Subject: [PATCH 3/7] Update AUTHORS --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index 37170ffa..f64093d8 100644 --- a/AUTHORS +++ b/AUTHORS @@ -223,3 +223,4 @@ that much better: * Kiryl Yermakou (https://github.com/rma4ok) * Matthieu Rigal (https://github.com/MRigal) * Charanpal Dhanjal (https://github.com/charanpald) + * Emmanuel Leblond (https://github.com/touilleMan) From 9c917c3bd389a222b3400dc61fb95de4c322cb75 Mon Sep 17 00:00:00 2001 From: Emmanuel Leblond Date: Tue, 2 Jun 2015 17:48:31 +0200 Subject: [PATCH 4/7] Update changelog --- docs/changelog.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 42d98597..52e76ffc 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -20,6 +20,7 @@ Changes in 0.9.X - DEV - Support for PyMongo 3+ #946 - Fix for issue where FileField deletion did not free space in GridFS. - No_dereference() not respected on embedded docs containing reference. #517 +- Document save raise an exception if save_condition fails Changes in 0.9.0 ================ From 4034ab41829ba7da18af5dc2e8cc1544dbff8614 Mon Sep 17 00:00:00 2001 From: Emmanuel Leblond Date: Mon, 8 Jun 2015 18:40:28 +0200 Subject: [PATCH 5/7] Clean save_condition exception implementation and related tests --- mongoengine/document.py | 2 +- tests/document/instance.py | 15 +++++++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/mongoengine/document.py b/mongoengine/document.py index f798780e..eedd01d2 100644 --- a/mongoengine/document.py +++ b/mongoengine/document.py @@ -351,7 +351,7 @@ class Document(BaseDocument): upsert = save_condition is None last_error = collection.update(select_dict, update_query, upsert=upsert, **write_concern) - if save_condition is not None and last_error['nModified'] == 0: + if not upsert and last_error['nModified'] == 0: raise OperationError('Race condition preventing' ' document update detected') created = is_new_object(last_error) diff --git a/tests/document/instance.py b/tests/document/instance.py index 3ccabacd..29692c20 100644 --- a/tests/document/instance.py +++ b/tests/document/instance.py @@ -958,8 +958,9 @@ class InstanceTest(unittest.TestCase): flip(w1) self.assertTrue(w1.toggle) self.assertEqual(w1.count, 1) - self.assertRaises(OperationError, - w1.save, save_condition={'save_id': UUID(42)}) + self.assertRaisesRegexp(OperationError, + "Race condition preventing document update detected", + w1.save, save_condition={'save_id': UUID(42)}) w1.reload() self.assertFalse(w1.toggle) self.assertEqual(w1.count, 0) @@ -987,8 +988,9 @@ class InstanceTest(unittest.TestCase): self.assertEqual(w1.count, 2) flip(w2) flip(w2) - self.assertRaises(OperationError, - w2.save, save_condition={'save_id': old_id}) + self.assertRaisesRegexp(OperationError, + "Race condition preventing document update detected", + w2.save, save_condition={'save_id': old_id}) w2.reload() self.assertFalse(w2.toggle) self.assertEqual(w2.count, 2) @@ -1000,8 +1002,9 @@ class InstanceTest(unittest.TestCase): self.assertTrue(w1.toggle) self.assertEqual(w1.count, 3) flip(w1) - self.assertRaises(OperationError, - w1.save, save_condition={'count__gte': w1.count}) + self.assertRaisesRegexp(OperationError, + "Race condition preventing document update detected", + w1.save, save_condition={'count__gte': w1.count}) w1.reload() self.assertTrue(w1.toggle) self.assertEqual(w1.count, 3) From 153c239c9bcb9acbc3f87c0d89995a128e4e934f Mon Sep 17 00:00:00 2001 From: Emmanuel Leblond Date: Thu, 11 Jun 2015 14:36:51 +0200 Subject: [PATCH 6/7] Replace assertRaisesRegexp by assertRaises (python2.6 compatibility) --- tests/document/instance.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/tests/document/instance.py b/tests/document/instance.py index 29692c20..e1710b9f 100644 --- a/tests/document/instance.py +++ b/tests/document/instance.py @@ -958,8 +958,7 @@ class InstanceTest(unittest.TestCase): flip(w1) self.assertTrue(w1.toggle) self.assertEqual(w1.count, 1) - self.assertRaisesRegexp(OperationError, - "Race condition preventing document update detected", + self.assertRaises(OperationError, w1.save, save_condition={'save_id': UUID(42)}) w1.reload() self.assertFalse(w1.toggle) @@ -988,8 +987,7 @@ class InstanceTest(unittest.TestCase): self.assertEqual(w1.count, 2) flip(w2) flip(w2) - self.assertRaisesRegexp(OperationError, - "Race condition preventing document update detected", + self.assertRaises(OperationError, w2.save, save_condition={'save_id': old_id}) w2.reload() self.assertFalse(w2.toggle) @@ -1002,8 +1000,7 @@ class InstanceTest(unittest.TestCase): self.assertTrue(w1.toggle) self.assertEqual(w1.count, 3) flip(w1) - self.assertRaisesRegexp(OperationError, - "Race condition preventing document update detected", + self.assertRaises(OperationError, w1.save, save_condition={'count__gte': w1.count}) w1.reload() self.assertTrue(w1.toggle) From cd7a9345ec624094e3811bf0c5ec2ed92d4ccc4d Mon Sep 17 00:00:00 2001 From: Emmanuel Leblond Date: Thu, 11 Jun 2015 14:45:19 +0200 Subject: [PATCH 7/7] Add issue related in changelog.rst --- docs/changelog.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 52e76ffc..65e236ba 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -20,7 +20,7 @@ Changes in 0.9.X - DEV - Support for PyMongo 3+ #946 - Fix for issue where FileField deletion did not free space in GridFS. - No_dereference() not respected on embedded docs containing reference. #517 -- Document save raise an exception if save_condition fails +- Document save raise an exception if save_condition fails #1005 Changes in 0.9.0 ================