From 5cca021ce7286027899bd58311b98de96b20fcc9 Mon Sep 17 00:00:00 2001 From: Noah Stapp Date: Fri, 27 Sep 2024 15:10:05 -0400 Subject: [PATCH 1/5] PYTHON-4786 - Fix UpdateResult.did_upsert TypeError --- pymongo/results.py | 5 +++-- test/test_results.py | 13 +++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/pymongo/results.py b/pymongo/results.py index b34f6c4926..00e4af8c8c 100644 --- a/pymongo/results.py +++ b/pymongo/results.py @@ -171,9 +171,10 @@ def upserted_id(self) -> Any: @property def did_upsert(self) -> bool: - """Whether or not an upsert took place.""" + """Whether an upsert took place.""" assert self.__raw_result is not None - return len(self.__raw_result.get("upserted", {})) > 0 + result = self.__raw_result.get("upserted") + return result is not None and len(str(result)) > 0 class DeleteResult(_WriteResult): diff --git a/test/test_results.py b/test/test_results.py index 19e086a9a5..e74e5107f6 100644 --- a/test/test_results.py +++ b/test/test_results.py @@ -122,6 +122,19 @@ def test_update_result(self): self.assertEqual(raw_result["n"], result.matched_count) self.assertEqual(raw_result["nModified"], result.modified_count) self.assertEqual(raw_result["upserted"], result.upserted_id) + self.assertEqual(result.did_upsert, False) + + raw_result_upserted = { + "n": 1, + "nModified": 1, + "upserted": [ + {"index": 5, "_id": 1}, + ], + } + self.repr_test(UpdateResult, raw_result_upserted) + + result = UpdateResult(raw_result_upserted, True) + self.assertEqual(result.did_upsert, True) result = UpdateResult(raw_result, False) self.assertEqual(raw_result, result.raw_result) From 3fc288e0ed96b3bc2fd58117d98855cc666e520b Mon Sep 17 00:00:00 2001 From: Noah Stapp Date: Mon, 30 Sep 2024 16:26:17 -0400 Subject: [PATCH 2/5] Fix did_upsert logic --- pymongo/results.py | 3 +-- test/test_results.py | 17 +++++++++++++---- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/pymongo/results.py b/pymongo/results.py index 00e4af8c8c..0685683aef 100644 --- a/pymongo/results.py +++ b/pymongo/results.py @@ -173,8 +173,7 @@ def upserted_id(self) -> Any: def did_upsert(self) -> bool: """Whether an upsert took place.""" assert self.__raw_result is not None - result = self.__raw_result.get("upserted") - return result is not None and len(str(result)) > 0 + return "upserted" in self.__raw_result class DeleteResult(_WriteResult): diff --git a/test/test_results.py b/test/test_results.py index e74e5107f6..deb09d7ed4 100644 --- a/test/test_results.py +++ b/test/test_results.py @@ -122,20 +122,29 @@ def test_update_result(self): self.assertEqual(raw_result["n"], result.matched_count) self.assertEqual(raw_result["nModified"], result.modified_count) self.assertEqual(raw_result["upserted"], result.upserted_id) - self.assertEqual(result.did_upsert, False) + self.assertEqual(result.did_upsert, True) - raw_result_upserted = { + raw_result_2 = { "n": 1, "nModified": 1, "upserted": [ {"index": 5, "_id": 1}, ], } - self.repr_test(UpdateResult, raw_result_upserted) + self.repr_test(UpdateResult, raw_result_2) - result = UpdateResult(raw_result_upserted, True) + result = UpdateResult(raw_result_2, True) self.assertEqual(result.did_upsert, True) + raw_result_3 = { + "n": 1, + "nModified": 1, + } + self.repr_test(UpdateResult, raw_result_3) + + result = UpdateResult(raw_result_3, True) + self.assertEqual(result.did_upsert, False) + result = UpdateResult(raw_result, False) self.assertEqual(raw_result, result.raw_result) error_msg = "A value for .* is not available when" From edca524d251fb22cc6b945e5c1c2156831a19b93 Mon Sep 17 00:00:00 2001 From: Noah Stapp Date: Tue, 1 Oct 2024 11:07:52 -0400 Subject: [PATCH 3/5] Add integration tests --- test/asynchronous/test_client_bulk_write.py | 41 +++++++++++++++++++++ test/asynchronous/test_collection.py | 13 +++++++ test/test_client_bulk_write.py | 41 +++++++++++++++++++++ test/test_collection.py | 13 +++++++ 4 files changed, 108 insertions(+) diff --git a/test/asynchronous/test_client_bulk_write.py b/test/asynchronous/test_client_bulk_write.py index 3a17299453..6b3a784f05 100644 --- a/test/asynchronous/test_client_bulk_write.py +++ b/test/asynchronous/test_client_bulk_write.py @@ -549,6 +549,47 @@ async def test_returns_error_if_auto_encryption_configured(self): "bulk_write does not currently support automatic encryption", context.exception._message ) + @async_client_context.require_version_min(8, 0, 0, -24) + @async_client_context.require_no_serverless + @unittest.skipUnless(_HAVE_PYMONGOCRYPT, "pymongocrypt is not installed") + async def test_upserted_result(self): + client = await self.async_rs_or_single_client() + + collection = client.db["coll"] + self.addAsyncCleanup(collection.drop) + await collection.drop() + + models = [] + models.append( + UpdateOne( + namespace="db.coll", + filter={"_id": "a"}, + update={"$set": {"x": 1}}, + upsert=True, + ) + ) + models.append( + UpdateOne( + namespace="db.coll", + filter={"_id": None}, + update={"$set": {"x": 1}}, + upsert=True, + ) + ) + models.append( + UpdateOne( + namespace="db.coll", + filter={"_id": None}, + update={"$set": {"x": 1}}, + ) + ) + result = await client.bulk_write(models=models, verbose_results=True) + + self.assertEqual(result.upserted_count, 2) + self.assertEqual(result.update_results[0].did_upsert, True) + self.assertEqual(result.update_results[1].did_upsert, True) + self.assertEqual(result.update_results[2].did_upsert, False) + # https://github.com/mongodb/specifications/blob/master/source/client-side-operations-timeout/tests/README.md#11-multi-batch-bulkwrites class TestClientBulkWriteCSOT(AsyncIntegrationTest): diff --git a/test/asynchronous/test_collection.py b/test/asynchronous/test_collection.py index 74a4a5151d..612090b69f 100644 --- a/test/asynchronous/test_collection.py +++ b/test/asynchronous/test_collection.py @@ -1444,6 +1444,19 @@ async def test_update_one(self): self.assertRaises(InvalidOperation, lambda: result.upserted_id) self.assertFalse(result.acknowledged) + async def test_update_result(self): + db = self.db + await db.drop_collection("test") + + result = await db.test.update_one({"x": 0}, {"$inc": {"x": 1}}, upsert=True) + self.assertEqual(result.did_upsert, True) + + result = await db.test.update_one({"_id": None, "x": 0}, {"$inc": {"x": 1}}, upsert=True) + self.assertEqual(result.did_upsert, True) + + result = await db.test.update_one({"_id": None}, {"$inc": {"x": 1}}) + self.assertEqual(result.did_upsert, False) + async def test_update_many(self): db = self.db await db.drop_collection("test") diff --git a/test/test_client_bulk_write.py b/test/test_client_bulk_write.py index ebbdc74c1c..2b2a63c977 100644 --- a/test/test_client_bulk_write.py +++ b/test/test_client_bulk_write.py @@ -549,6 +549,47 @@ def test_returns_error_if_auto_encryption_configured(self): "bulk_write does not currently support automatic encryption", context.exception._message ) + @client_context.require_version_min(8, 0, 0, -24) + @client_context.require_no_serverless + @unittest.skipUnless(_HAVE_PYMONGOCRYPT, "pymongocrypt is not installed") + def test_upserted_result(self): + client = self.rs_or_single_client() + + collection = client.db["coll"] + self.addCleanup(collection.drop) + collection.drop() + + models = [] + models.append( + UpdateOne( + namespace="db.coll", + filter={"_id": "a"}, + update={"$set": {"x": 1}}, + upsert=True, + ) + ) + models.append( + UpdateOne( + namespace="db.coll", + filter={"_id": None}, + update={"$set": {"x": 1}}, + upsert=True, + ) + ) + models.append( + UpdateOne( + namespace="db.coll", + filter={"_id": None}, + update={"$set": {"x": 1}}, + ) + ) + result = client.bulk_write(models=models, verbose_results=True) + + self.assertEqual(result.upserted_count, 2) + self.assertEqual(result.update_results[0].did_upsert, True) + self.assertEqual(result.update_results[1].did_upsert, True) + self.assertEqual(result.update_results[2].did_upsert, False) + # https://github.com/mongodb/specifications/blob/master/source/client-side-operations-timeout/tests/README.md#11-multi-batch-bulkwrites class TestClientBulkWriteCSOT(IntegrationTest): diff --git a/test/test_collection.py b/test/test_collection.py index dab59cf1b2..a2c3b0b0b6 100644 --- a/test/test_collection.py +++ b/test/test_collection.py @@ -1429,6 +1429,19 @@ def test_update_one(self): self.assertRaises(InvalidOperation, lambda: result.upserted_id) self.assertFalse(result.acknowledged) + def test_update_result(self): + db = self.db + db.drop_collection("test") + + result = db.test.update_one({"x": 0}, {"$inc": {"x": 1}}, upsert=True) + self.assertEqual(result.did_upsert, True) + + result = db.test.update_one({"_id": None, "x": 0}, {"$inc": {"x": 1}}, upsert=True) + self.assertEqual(result.did_upsert, True) + + result = db.test.update_one({"_id": None}, {"$inc": {"x": 1}}) + self.assertEqual(result.did_upsert, False) + def test_update_many(self): db = self.db db.drop_collection("test") From 86589c717ba41e130c4033a5d1581627f9c88da0 Mon Sep 17 00:00:00 2001 From: Noah Stapp Date: Tue, 1 Oct 2024 16:48:32 -0400 Subject: [PATCH 4/5] Remove unneeded requirement --- test/asynchronous/test_client_bulk_write.py | 1 - test/test_client_bulk_write.py | 1 - 2 files changed, 2 deletions(-) diff --git a/test/asynchronous/test_client_bulk_write.py b/test/asynchronous/test_client_bulk_write.py index 852932dedd..9464337809 100644 --- a/test/asynchronous/test_client_bulk_write.py +++ b/test/asynchronous/test_client_bulk_write.py @@ -552,7 +552,6 @@ async def test_returns_error_if_auto_encryption_configured(self): @async_client_context.require_version_min(8, 0, 0, -24) @async_client_context.require_no_serverless - @unittest.skipUnless(_HAVE_PYMONGOCRYPT, "pymongocrypt is not installed") async def test_upserted_result(self): client = await self.async_rs_or_single_client() diff --git a/test/test_client_bulk_write.py b/test/test_client_bulk_write.py index e0dabcf5e9..58b5015dd2 100644 --- a/test/test_client_bulk_write.py +++ b/test/test_client_bulk_write.py @@ -552,7 +552,6 @@ def test_returns_error_if_auto_encryption_configured(self): @client_context.require_version_min(8, 0, 0, -24) @client_context.require_no_serverless - @unittest.skipUnless(_HAVE_PYMONGOCRYPT, "pymongocrypt is not installed") def test_upserted_result(self): client = self.rs_or_single_client() From d8894cfef2b29dafcea38a3a3450bb04a887f696 Mon Sep 17 00:00:00 2001 From: Noah Stapp Date: Tue, 1 Oct 2024 16:57:22 -0400 Subject: [PATCH 5/5] Add versionadded --- pymongo/results.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pymongo/results.py b/pymongo/results.py index 0685683aef..d17ff1c3ea 100644 --- a/pymongo/results.py +++ b/pymongo/results.py @@ -171,7 +171,10 @@ def upserted_id(self) -> Any: @property def did_upsert(self) -> bool: - """Whether an upsert took place.""" + """Whether an upsert took place. + + .. versionadded:: 4.9 + """ assert self.__raw_result is not None return "upserted" in self.__raw_result