Create and document Java sync improved bulk write API

[stIncMale]

Documentation:

• https://www.mongodb.com/docs/upcoming/reference/command/bulkWrite
• https://github.com/mongodb/specifications/blob/master/source/crud/bulk-write.md

API changes (only new program elements were introduced):

1. com.mongodb.client.model.bulk
   1.1. ClientWriteModel, ClientNamespacedWriteModel. a) Note how unlike WriteModel, it does not have a type parameter specifying the document type. Initially I implemented that (48b9614), but then I removed it (af854ed) because I don't think it is needed in this case. b) The reason behind introducing two write model types is that we are considering the possibility of exposing the new operation at the collection level, where each model must not have a namespace associated with it.
   1.2. ClientInsertOneModel, ClientUpdateOneModel, ClientUpdateManyModel, ClientReplaceOneModel, ClientDeleteOneModel, ClientDeleteManyModel.
   1.3. ClientDeleteOptions, ClientReplaceOptions, ClientUpdateOptions.
   1.4. ClientBulkWriteOptions.
2. com.mongodb.client.result.bulk
   2.2. ClientDeleteResult.
   2.3. ClientInsertOneResult.
   2.4. ClientUpdateResult.
   2.5. ClientBulkWriteResult.
3. com.mongodb.ClientBulkWriteException.
4. com.mongodb.client.MongoCluster.bulkWrite (4 overloads).

BULK-TODO code comments:

The BULK-TODO code comments will be turned into comments (where possible) in the PR to merge JAVA-4586_bulk-write into master, which will be created once this PR is approved and merged into JAVA-4586_bulk-write.

JAVA-5527

[stIncMale]

I'd prefer to half the number of overloads of methods in ClientWriteModel and the number of new MongoCluster.bulkWrite overloads by allowing to pass @Nullable options. However, the existing bulk write API does not allow null options, and I followed that in the new API.

[stIncMale]

Maybe reviewers have thoughts on this comment. My understanding is that the existing bulk write API does not wrap the top-level errors in BulkWriteException, but the new API wraps the top-level errors in ClientBulkWriteException. Given that the driver uses error code and labels to implement some of its functionality, the question seems reasonable. I don't currently have an answer.

[jyemin]

I will have to think about this more. I'm not sure.

[stIncMale]

These are all the error labels the driver knows about and uses: TransientTransactionError, UnknownTransactionCommitResult, RetryableWriteError, NoWritesPerformed.

This question splits into two:

1. What error codes/labels should be copied to ClientBulkWriteException from ClientBulkWriteException.error (the top-level error) for consumption by applications?
   • Simply copying some labels seems to be just incorrect, for example, RetryableWriteError - even if a specific bulkWrite command within a bulk operation can be retried, it does not mean that all previous commands can also be repeated when an application attempts to retry the whole operation. The same reasoning seems to apply to NoWritesPerformed. However, if no bulkWrite commands succeeded, then copying both of these labels seems safe.
   • I don't think we should copy any error codes for consumption by applications.
2. What error codes/labels should be copied to ClientBulkWriteException from ClientBulkWriteException.error (the top-level error) for consumption by the driver?
   • Given that ClientBulkWriteException will be created by ClientBulkWriteOperation.execute, only the following driver code will have a chance to handle labels and the code from this exception:
     • MongoClusterImpl.OperationExecutorImpl.execute - reads TransientTransactionError, UnknownTransactionCommitResult, and code 91.
     • ClientSessionImpl.withTransaction - reads TransientTransactionError, UnknownTransactionCommitResult.
   • It seems safe to copy the code if its 91.
   • It seems safe to copy the label UnknownTransactionCommitResult.
   • I am not sure about copying TransientTransactionError. @jyemin does this label always result in driver aborting the transaction?

[rozza]

Seems safest not to copy any error codes / error labels to the top level Exception.

If a user needs to inspect them can't they use the writeErrors map?

[stIncMale]

Seems safest not to copy any error codes / error labels to the top level Exception.

I agree that we should not copy error codes/labels for consumption by applications. However, we may still decide to do that for consumption by the driver (and if we do so, the applications will be able to observe that).

I am OK with not copying anything, and instead updating the logic of MongoClusterImpl.OperationExecutorImpl.execute, ClientSessionImpl.withTransaction, and their reactive counterparts such that they look at ClientBulkWriteException.getError.

[stIncMale]

If a user needs to inspect them can't they use the writeErrors map?

I don't think that ClientBulkWriteException.getWriteErrors is relevant here. Only the code/labels from the top-level error seem to make sense to consider in the context of copying them to ClientBulkWriteException.

[stIncMale]

Done in dbf9a26, f026ee3.

[stIncMale]

Note that this throwing behavior is explicitly allowed by the specification.

[vbabanin]

1. Handling unacknowledged writes:

We are currently throwing an UnsupportedOperationException whenever an unacknowledged write occurs, rendering all methods ineffective under these conditions.

Ideally, API users would check status of isAcknowladged() before operations that might throw exceptions. However, in practice, I believe some users might overlook or misunderstand these requirements, leading to runtime exceptions.

The server, to my understanding, does not send a reply in cases of unacknowladged write. We might consider the API which handles such misuse more gracefully. For instance, wrapping ClientBulkWriteResult in an Optional within the bulkWrite method could indicate an unacknowledged write. This approach prevents the API from throwing exceptions, potentially reducing runtime issues in user applications. Using Optional might make the API more intuitive. This also seems to be allowed by the spec.

2. API consistency

If we shift the status-checking approach of unacknowledged writes to use Optional, it could further unify the API. Currently, we have Optional<BsonValue> getUpsertedId(); which returns a non-empty value only if an upsert occurred. ClientUpdateResult lacks a status checking method. Similarly, we could make ClientBulkWriteResult more consistent by returning Optional from methods instead of throwing UnsupportedOperationException in cases when it is not verbose result, aligning it with what is already done in ClientUpdateResult. This way the API should be more consistent. This also seems to be allowed by the spec.

3. Further consideration with VerboseResult (which i would personally prefer)

In recent discussions, @stIncMale proposed introducing a VerboseResult within ClientBulkWriteResult, which would be encapsulated in Optional. This design would contain all the current methods that throw unsupported exceptions related to verbose result and would align seamlessly with the use of Optional in ClientUpdateResult and Optional for unacknowledged write, enhancing the intuitiveness and cleanliness of the API.
(If I’ve missed any aspects of this approach, @stIncMale, please feel free to correct me.)

This approach would require deviation from the specification. However, current requirements in specification seem to be too restrictive.

4. Balancing legacy compatibility and API improvements:
   While maintaining consistency with our 'legacy' bulk API aids in user transition, doing so could lead to outdated designs. If we focus on aligning with legacy standards, we risk repeating old patterns and missing opportunities to modernize our API.

I understand these approaches might have been considered previously. I would appreciate hearing your thoughts.

@stIncMale @rozza @jyemin

[stIncMale]

See also https://mongodb.slack.com/archives/C035ZJL6CQN/p1724354467489519.

Done in 395af7a.

[rozza]

Good work @vbabanin and @stIncMale

[stIncMale]

See https://github.com/mongodb/specifications/blob/master/source/crud/bulk-write.md#is-bulkwrite-supported-on-atlas-serverless.

[stIncMale]

@jyemin, @rozza, @vbabanin, please re-review.

[jyemin]

Excellent work, Valentin.

Just one Javadoc nit, but LGTM either way.

[vbabanin]

While Verbose accurately suggests that it provides more detailed or extended information than plain result, using an adjective as a class or interface name is somewhat unconventional in Java spec and the name might not immediately convey its purpose.

I would suggest noun-based naming, just couple of examples: VerboseBulkResult/ExtendedBulkResult

[stIncMale]

The intent was to refer to it as ClientBulkWriteResult.Verbose. I updated the code where this was not done, and following is how the rendered API docs look like

Do you still think this type should be renamed? If yes, then VerboseResults is the name I'll use.

[stIncMale]

Done in b6702a6.

[vbabanin]

Looks great!