[JAVA-4874] BsonDocument.clone should return a mutable deep copy, or we need a different mechanism of achieving that Created: 08/Feb/23  Updated: 28/Oct/23  Resolved: 13/Feb/23

Status: Closed
Project: Java Driver
Component/s: BSON
Affects Version/s: None
Fix Version/s: 4.10.0

Type: Bug Priority: Minor - P4
Reporter: Valentin Kavalenka Assignee: Valentin Kavalenka
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Backwards Compatibility: Fully Compatible
Documentation Changes: Not Needed
Documentation Changes Summary:

1. What would you like to communicate to the user about this feature?
2. Would you like the user to see examples of the syntax and/or executable code and its output?
3. Which versions of the driver/connector does this apply to?


 Description   

Currently we have at least two places (that I know of) where we rely on BsonDocument.clone returning not only a deep copy, but mutable deep copy:

1. AbstractConstructibleBson.newMerged
2. The new method ClientEncryption.createEncryptedCollection introduced in https://github.com/mongodb/mongo-java-driver/pull/1079.

ross@mongodb.com pointed out that RawBsonDocument breaks the "mutable" part and can come from users.



 Comments   
Comment by Githook User [ 13/Feb/23 ]

Author:

{'name': 'Valentin Kovalenko', 'email': 'valentin.kovalenko@mongodb.com', 'username': 'stIncMale'}

Message: Introduce `BsonUtil.mutableDeepCopy` (#1081)

JAVA-4874
Branch: master
https://github.com/mongodb/mongo-java-driver/commit/a7489266d5eced73ce129357ec1782accccf0a5a

Comment by Valentin Kavalenka [ 09/Feb/23 ]

The idea here was to make it as efficient as possible
...
The same goes for RawBsonDocument.

If performance matters, I wonder why we clone bytes in RawBsonDocument.clone, given that RawBsonDocument is documented to be immutable.

Could we instead say that, if you need an actual instance of BsonDocument that is a mutable copy of the source, it's your responsibility to do so using a BsonDocument constructor?

I am not sure users really need something like this, otherwise we would have had requests from them by now. Let's introduce a mutableDeepCopy(BsonDocument) internal utility method. And if users ever request something like that, we can expose it via BsonDocument.

Comment by Jeffrey Yemin [ 09/Feb/23 ]

The original commit that added support for cloning provides some more context on the intent. Specifically:

This is in order to facilitate upcoming CommandListener implementations to clone BsonDocument instances representing commands and command responses.

The idea here was to make it as efficient as possible for the driver to include the command and command reply in the events, but still provide a way for event listeners to efficiently copy those documents for later use, in the event they are needed outside the scope of the event listener method. Hence the Javadoc for CommandStartedEvent#getCommand:

The document is only usable within the method that delivered the event. If it's needed for longer, it must be cloned via {@link Object#clone()}.

It should probably also indicate that the document is read-only, and in practice it is, because it's actually an instance of com.mongodb.internal.connection.ByteBufBsonDocument.

I don't think we should change the clone method of that class to return a mutable BsonDocument, as it would change the performance characteristics of the method significantly. The same goes for RawBsonDocument.

Could we instead say that, if you need an actual instance of BsonDocument that is a mutable copy of the source, it's your responsibility to do so using a BsonDocument constructor? Currently, it doesn't seem like there is a convenient one to use, but we could add one.

Generated at Thu Feb 08 09:03:11 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.