Introduce BsonUtil.mutableDeepCopy
#1081
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rozza
approved these changes
Feb 8, 2023
Member
rozza
left a comment
rozza
left a comment
There was a problem hiding this comment.
LGTM - please also add a Jira ticket to associate with this change.
Collaborator
|
I'm not so sure about this, and would like to discuss it in the context of a Jira ticket. |
Member
Author
stIncMale
force-pushed
the
rawBsonDocumentClone
branch
from
d1a0c31 to
a525a64
Compare
Member
Author
|
Instead of modifying |
stIncMale
changed the title
BsonDocument.clone returns a mutable deep copyUtil.mutableDeepCopy
rozza
reviewed
Feb 10, 2023
Member
There was a problem hiding this comment.
Not sure I like the lack of description in name of the Util class - suggest BsonDocumentHelper or BsonDocumentUtil. Will make it more discoverable in the future.
I like the tests use a mix of BsonValue (BsonDocumentWrapper, RawBsonDocument etc) types.
Other than the name - LGTM
JAVA-4874
Member
Author
|
@rozza Renamed to |
stIncMale
changed the title
Util.mutableDeepCopyBsonUtil.mutableDeepCopy
jyemin
reviewed
Feb 13, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently we have at least two places (that I know of) where we rely on
BsonDocument.clonereturning not only a deep copy, but mutable deep copy:AbstractConstructibleBson.newMergedClientEncryption.createEncryptedCollectionintroduced in Add theClientEncryption.createEncryptedCollectionhelper method #1079.@rozza pointed out that
RawBsonDocumentbreaks the "mutable" part and can come from users: thecloneIsDeepCopyAndMutabletest added in this PR demonstrates this.RawBsonDocument.clonereturnsBsonDocument(notRawBsonDocument), and documents nothing about its behavior (neither doesBsonDocument.clone). So while there may be different approaches of solving the problem, these facts allow for the straightforward approach to fix the problem proposed in this PR.JAVA-4874