From 6517c5225c0ce9dfc15f88db2e3ccb58e994bb97 Mon Sep 17 00:00:00 2001 From: Ephraim Berkovitch Date: Wed, 22 Feb 2017 00:21:05 +0200 Subject: [PATCH 1/4] Issue #1472 - duplication in primary key (_id) raising an exception now --- mongoengine/document.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/mongoengine/document.py b/mongoengine/document.py index b79e5e976..5b5d5b5fa 100644 --- a/mongoengine/document.py +++ b/mongoengine/document.py @@ -394,7 +394,10 @@ def _save_create(self, doc, force_insert, write_concern): if force_insert: return collection.insert(doc, **write_concern) - object_id = collection.save(doc, **write_concern) + #object_id = collection.save(doc, **write_concern) + # instead of using SAVE as in previous line, which has been deprecated in pymongo: + result = collection.insert_one(doc) + object_id = result.inserted_id # In PyMongo 3.0, the save() call calls internally the _update() call # but they forget to return the _id value passed back, therefore getting it back here From fef36c985f101d220c7741e3c7c06582c6b9aeef Mon Sep 17 00:00:00 2001 From: Ephraim Berkovitch Date: Wed, 22 Feb 2017 06:38:47 +0200 Subject: [PATCH 2/4] Issue #1472 - now all tests pass --- mongoengine/document.py | 5 +---- mongoengine/queryset/base.py | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/mongoengine/document.py b/mongoengine/document.py index 5b5d5b5fa..b79e5e976 100644 --- a/mongoengine/document.py +++ b/mongoengine/document.py @@ -394,10 +394,7 @@ def _save_create(self, doc, force_insert, write_concern): if force_insert: return collection.insert(doc, **write_concern) - #object_id = collection.save(doc, **write_concern) - # instead of using SAVE as in previous line, which has been deprecated in pymongo: - result = collection.insert_one(doc) - object_id = result.inserted_id + object_id = collection.save(doc, **write_concern) # In PyMongo 3.0, the save() call calls internally the _update() call # but they forget to return the _id value passed back, therefore getting it back here diff --git a/mongoengine/queryset/base.py b/mongoengine/queryset/base.py index 7e4856863..fde46ae63 100644 --- a/mongoengine/queryset/base.py +++ b/mongoengine/queryset/base.py @@ -286,7 +286,7 @@ def create(self, **kwargs): .. versionadded:: 0.4 """ - return self._document(**kwargs).save() + return self._document(**kwargs).save(force_insert=True) def first(self): """Retrieve the first object matching the query.""" From 1e37b3fd2226e571afcfca063cbf0e470d7385cd Mon Sep 17 00:00:00 2001 From: Ephraim Berkovitch Date: Wed, 22 Feb 2017 07:49:15 +0200 Subject: [PATCH 3/4] Issue #1472 - a new test for attempt of creating a duplicate primary key --- tests/document/indexes.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/document/indexes.py b/tests/document/indexes.py index af93e7db2..603350c69 100644 --- a/tests/document/indexes.py +++ b/tests/document/indexes.py @@ -792,6 +792,27 @@ class User(Document): User.drop_collection() + def test_unique_and_primary_create(self): + """Create a new record with a duplicate primary key + throws an exception + """ + + class User(Document): + name = StringField(primary_key=True) + password = StringField() + + User.drop_collection() + + User.objects.create(name='huangz', password='secret') + def duplicate_primary_key(): + User.objects.create(name='huangz', password='secret2') + self.assertRaises(NotUniqueError, duplicate_primary_key) + + self.assertEqual(User.objects.count(), 1) + self.assertEqual(User.objects.get().password, 'secret') + + User.drop_collection() + def test_index_with_pk(self): """Ensure you can use `pk` as part of a query""" From 26b059838b53b9b08f3547f3c5eb846749c769c8 Mon Sep 17 00:00:00 2001 From: Ephraim Berkovitch Date: Mon, 27 Feb 2017 12:22:51 +0200 Subject: [PATCH 4/4] Issue #1472 - clean uo the code and remove redundancies --- .gitignore | 1 + tests/document/indexes.py | 21 +-------------------- 2 files changed, 2 insertions(+), 20 deletions(-) diff --git a/.gitignore b/.gitignore index b180e87ef..048a2d192 100644 --- a/.gitignore +++ b/.gitignore @@ -17,3 +17,4 @@ tests/test_bugfix.py htmlcov/ venv venv3 +scratchpad diff --git a/tests/document/indexes.py b/tests/document/indexes.py index 603350c69..3afcebbaa 100644 --- a/tests/document/indexes.py +++ b/tests/document/indexes.py @@ -412,7 +412,6 @@ class MongoUser(User): User.ensure_indexes() info = User.objects._collection.index_information() self.assertEqual(sorted(info.keys()), ['_cls_1_user_guid_1', '_id_']) - User.drop_collection() def test_embedded_document_index(self): """Tests settings an index on an embedded document @@ -434,7 +433,6 @@ class BlogPost(Document): info = BlogPost.objects._collection.index_information() self.assertEqual(sorted(info.keys()), ['_id_', 'date.yr_-1']) - BlogPost.drop_collection() def test_list_embedded_document_index(self): """Ensure list embedded documents can be indexed @@ -461,7 +459,6 @@ class BlogPost(Document): post1 = BlogPost(title="Embedded Indexes tests in place", tags=[Tag(name="about"), Tag(name="time")]) post1.save() - BlogPost.drop_collection() def test_recursive_embedded_objects_dont_break_indexes(self): @@ -623,8 +620,6 @@ class BlogPost(Document): post3 = BlogPost(title='test3', date=Date(year=2010), slug='test') self.assertRaises(OperationError, post3.save) - BlogPost.drop_collection() - def test_unique_embedded_document(self): """Ensure that uniqueness constraints are applied to fields on embedded documents. """ @@ -652,8 +647,6 @@ class BlogPost(Document): sub=SubDocument(year=2010, slug='test')) self.assertRaises(NotUniqueError, post3.save) - BlogPost.drop_collection() - def test_unique_embedded_document_in_list(self): """ Ensure that the uniqueness constraints are applied to fields in @@ -684,8 +677,6 @@ class BlogPost(Document): self.assertRaises(NotUniqueError, post2.save) - BlogPost.drop_collection() - def test_unique_with_embedded_document_and_embedded_unique(self): """Ensure that uniqueness constraints are applied to fields on embedded documents. And work with unique_with as well. @@ -719,8 +710,6 @@ class BlogPost(Document): sub=SubDocument(year=2009, slug='test-1')) self.assertRaises(NotUniqueError, post3.save) - BlogPost.drop_collection() - def test_ttl_indexes(self): class Log(Document): @@ -768,13 +757,11 @@ class Customer(Document): raise AssertionError("We saved a dupe!") except NotUniqueError: pass - Customer.drop_collection() def test_unique_and_primary(self): """If you set a field as primary, then unexpected behaviour can occur. You won't create a duplicate but you will update an existing document. """ - class User(Document): name = StringField(primary_key=True, unique=True) password = StringField() @@ -790,13 +777,10 @@ class User(Document): self.assertEqual(User.objects.count(), 1) self.assertEqual(User.objects.get().password, 'secret2') - User.drop_collection() - def test_unique_and_primary_create(self): """Create a new record with a duplicate primary key throws an exception """ - class User(Document): name = StringField(primary_key=True) password = StringField() @@ -804,15 +788,12 @@ class User(Document): User.drop_collection() User.objects.create(name='huangz', password='secret') - def duplicate_primary_key(): + with self.assertRaises(NotUniqueError): User.objects.create(name='huangz', password='secret2') - self.assertRaises(NotUniqueError, duplicate_primary_key) self.assertEqual(User.objects.count(), 1) self.assertEqual(User.objects.get().password, 'secret') - User.drop_collection() - def test_index_with_pk(self): """Ensure you can use `pk` as part of a query"""