RUST-107 Convenient transactions #849
Conversation
| } | ||
| } | ||
|
|
||
| pub(crate) struct OpSessions<'a> { |
There was a problem hiding this comment.
The split here between OpSessions and OpRunner is to satisfy the borrow checker during recursive operation, in which distinct mutable borrows of each are needed.
| operation: &Operation, | ||
| mut sessions: OpSessions<'b>, | ||
| ) -> Option<Result<Option<bson::Bson>, crate::error::Error>> { | ||
| if operation.name == "withTransaction" { |
There was a problem hiding this comment.
I'm not thrilled about special-casing withTransaction by name, but I couldn't think of a better way to do this.
| return Some(operation.execute_recursive(self, sessions).await); | ||
| } | ||
|
|
||
| let db = match &operation.database_options { |
There was a problem hiding this comment.
Despite what github's diff says, the rest of the logic in this method is unchanged from the original loop.
| async fn run_operation( | ||
| &mut self, | ||
| operation: &Operation, | ||
| ) -> Option<Result<Option<bson::Bson>, crate::error::Error>> { |
There was a problem hiding this comment.
The return type here is a little complex because along with succeeding or failing an operation can also be skipped.
| @@ -87,6 +97,74 @@ pub(crate) struct Operation { | |||
| pub(crate) session: Option<String>, | |||
| } | |||
|
|
|||
| impl Operation { | |||
| pub(crate) fn assert_result_matches(&self, result: &Result<Option<Bson>>, description: &str) { | |||
There was a problem hiding this comment.
This was also moved unchanged from its original place in run_v2_test.
| async move { | ||
| for op in operations.iter() { | ||
| let sessions = OpSessions { | ||
| session0: Some(session), |
There was a problem hiding this comment.
This is literally the same pointer as session0 passed to execute_recursive, but to satisfy the borrow checker we need to re-bind it with the lifetime provided to the callback.
| options: impl Into<Option<TransactionOptions>>, | ||
| ) -> Result<R> | ||
| where | ||
| F: for<'a> FnMut(&'a mut ClientSession, &'a mut C) -> BoxFuture<'a, Result<R>>, |
There was a problem hiding this comment.
Is there a way we could rewrite this signature to avoid including BoxFuture? futures_core is a pre-1.0 crate, so I think we need to be cautious about including its API in our public API. I know BoxFuture specifically was broken a while ago to add the lifetime parameter.
There was a problem hiding this comment.
Hmm, that's unfortunate. The type erasure is necessary - there's no way to express for<'a> FnMut(&'a mut ClientSession, &'a mut C) -> impl Future<'a, Result<R>> currently without it. Since BoxFuture is just a type alias, I've duplicated the definition here instead of using the one from the futures_core crate.
src/client/session/mod.rs
Outdated
| @@ -561,6 +566,107 @@ impl ClientSession { | |||
| } | |||
| } | |||
|
|
|||
| /// Runs a callback inside a transaction. Transient transaction errors will cause the callback | |||
There was a problem hiding this comment.
Can we add a bit more detail here explaining that the transaction will be started and committed/aborted within this method?
| /// # Ok(()) | ||
| /// # } | ||
| /// ``` | ||
| pub async fn with_transaction<R, C, F>( |
There was a problem hiding this comment.
we'll also need to add this method to the sync API
There was a problem hiding this comment.
Good catch. Unfortunately, I had to duplicate the body of the function rather than just wrapping the async one to avoid nested block_on calls from within the callback.
|
Note that the evergreen failures are unrelated - testing against AWS auth is broken right now due to https://jira.mongodb.org/browse/BF-28319. |
RUST-107
This adds the convenient transactions API and associated tests. Running the v2 spec tests required quite a bit of refactoring in order to allow recursively running operations within the
withTransactionoperation.