-
Notifications
You must be signed in to change notification settings - Fork 1.1k
PYTHON-3075 bulk_write does not apply CodecOptions to upserted_ids result #840
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
8b3c736
2b9d74f
dcea844
17fa097
692c960
29c8195
40b6629
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,9 +15,13 @@ | |
"""Test the bulk API.""" | ||
|
||
import sys | ||
import uuid | ||
from bson.binary import UuidRepresentation | ||
from bson.codec_options import CodecOptions | ||
|
||
sys.path[0:0] = [""] | ||
|
||
from bson import Binary | ||
from bson.objectid import ObjectId | ||
from pymongo.common import partition_node | ||
from pymongo.errors import (BulkWriteError, | ||
|
@@ -376,6 +380,78 @@ def test_client_generated_upsert_id(self): | |
{'index': 2, '_id': 2}]}, | ||
result.bulk_api_result) | ||
|
||
def test_upsert_uuid_standard(self): | ||
options = CodecOptions(uuid_representation=UuidRepresentation.STANDARD) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice. Could you add a similar test to ensure that UuidRepresentation.UNSPECIFIED results in Binary instances? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As far as I can tell we can't do that because it raises a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes the test will need to convert the uuids to Binary manually before inserting them:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
coll = self.coll.with_options(codec_options=options) | ||
uuids = [uuid.uuid4() for _ in range(3)] | ||
result = coll.bulk_write([ | ||
UpdateOne({'_id': uuids[0]}, {'$set': {'a': 0}}, upsert=True), | ||
ReplaceOne({'a': 1}, {'_id': uuids[1]}, upsert=True), | ||
# This is just here to make the counts right in all cases. | ||
ReplaceOne({'_id': uuids[2]}, {'_id': uuids[2]}, upsert=True), | ||
]) | ||
self.assertEqualResponse( | ||
{'nMatched': 0, | ||
'nModified': 0, | ||
'nUpserted': 3, | ||
'nInserted': 0, | ||
'nRemoved': 0, | ||
'upserted': [{'index': 0, '_id': uuids[0]}, | ||
{'index': 1, '_id': uuids[1]}, | ||
{'index': 2, '_id': uuids[2]}]}, | ||
result.bulk_api_result) | ||
|
||
def test_upsert_uuid_unspecified(self): | ||
options = CodecOptions(uuid_representation=UuidRepresentation.UNSPECIFIED) | ||
coll = self.coll.with_options(codec_options=options) | ||
uuids = [Binary.from_uuid(uuid.uuid4()) for _ in range(3)] | ||
result = coll.bulk_write([ | ||
UpdateOne({'_id': uuids[0]}, {'$set': {'a': 0}}, upsert=True), | ||
ReplaceOne({'a': 1}, {'_id': uuids[1]}, upsert=True), | ||
# This is just here to make the counts right in all cases. | ||
ReplaceOne({'_id': uuids[2]}, {'_id': uuids[2]}, upsert=True), | ||
]) | ||
self.assertEqualResponse( | ||
{'nMatched': 0, | ||
'nModified': 0, | ||
'nUpserted': 3, | ||
'nInserted': 0, | ||
'nRemoved': 0, | ||
'upserted': [{'index': 0, '_id': uuids[0]}, | ||
{'index': 1, '_id': uuids[1]}, | ||
{'index': 2, '_id': uuids[2]}]}, | ||
result.bulk_api_result) | ||
|
||
def test_upsert_uuid_standard_subdocuments(self): | ||
options = CodecOptions(uuid_representation=UuidRepresentation.STANDARD) | ||
coll = self.coll.with_options(codec_options=options) | ||
ids = [ | ||
{'f': Binary(bytes(i)), 'f2': uuid.uuid4()} | ||
for i in range(3) | ||
] | ||
|
||
result = coll.bulk_write([ | ||
UpdateOne({'_id': ids[0]}, {'$set': {'a': 0}}, upsert=True), | ||
ReplaceOne({'a': 1}, {'_id': ids[1]}, upsert=True), | ||
# This is just here to make the counts right in all cases. | ||
ReplaceOne({'_id': ids[2]}, {'_id': ids[2]}, upsert=True), | ||
]) | ||
|
||
# The `Binary` values are returned as `bytes` objects. | ||
for _id in ids: | ||
_id['f'] = bytes(_id['f']) | ||
|
||
self.assertEqualResponse( | ||
{'nMatched': 0, | ||
'nModified': 0, | ||
'nUpserted': 3, | ||
'nInserted': 0, | ||
'nRemoved': 0, | ||
'upserted': [{'index': 0, '_id': ids[0]}, | ||
{'index': 1, '_id': ids[1]}, | ||
{'index': 2, '_id': ids[2]}]}, | ||
result.bulk_api_result) | ||
|
||
def test_single_ordered_batch(self): | ||
result = self.coll.bulk_write([ | ||
InsertOne({'a': 1}), | ||
|
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect we need a similar change in _EncryptedBulkWriteContext.execute().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't sure how to test that locally, waiting for evergreen