How to hide RwLockMappedWriteGuard boilerplate returned from func

help
1

Hello everyone!
I have structure like this:

#[derive(Default, Debug)]
pub struct ActionContextImpl {
    authorization: RwLock<Option<AuthorizationContext>>,
    transaction: RwLock<Option<TransactionContext>>,
}

which implements trait:

#[async_trait]
pub trait ActionContext: Debug + Send + Sync + 'static {
    async fn authorization(&self) -> RwLockReadGuard<Option<AuthorizationContext>>;
    async fn transaction(&self) -> RwLockMappedWriteGuard<Option<TransactionContext>>;
    async fn extend(&self, extension: ActionContextExtension);
}

all time when i want to get transaction which contained in TransactionContext i require to write quite code in place, and cant find way to encapsulate these code inside function because this brokes rust rules.
TransactionContext struct looks like this:

#[derive(Debug)]
pub struct TransactionContext(Box<dyn Any + Send + Sync>);

impl TransactionContext {
    pub fn new(tx_context: impl TransactionContextMarker) -> TransactionContext {
        Self(Box::new(tx_context))
    }
    pub fn get_mut(&mut self) -> &mut Box<dyn Any + Send + Sync> {
        &mut self.0
    }
    pub fn into_inner(self) -> Box<dyn Any + Send + Sync> {
        self.0
    }
}

and code to extract transaction:

let mut guard = ctx.transaction().await;
let tx = guard.as_mut()
    .ok_or(anyhow!("Transaction context not exist."))?
    .get_mut()
    .downcast_mut::<TransactionContextImpl>()
    .ok_or(anyhow!("Can not downcast any to transaction context implementation."))?
    .get_transaction();

get_transaction() function just get .0 private field of pub(crate) struct TransactionContextImpl(Transaction<'static, Postgres>); which implements TransactionContextMarker.
So can i somehow encapsulate in func this code which extract and downcast transaction? Also not good thing that i always need to separate guard variable, because it has ownership on Transaction contained inside it. I think its little verbose)

P.S. i was try to use owning_ref but cant figure out how to use it for my case cos i use tokio's RwLock which not have implementations of unsafe traits required by owning_ref.

2

Here are some thoughts.

You can't do it all in the ActionContext::transaction method, because you need to return a lock guard that must exist while you're using its contents. And then you need to extract/downcast the transaction as you've shown. So there are at least two steps needed.

But you could create a TransactionGuard type that is returned by ActionContext::transaction. TransactionGuard could contain the lock guard, and have a TransactionGuard::get_transaction method that does the extraction/downcast. I think this would encapsulate most of the boilerplate.

If you've tried this and gotten errors, please post the code and errors, and people here can help.

Another thing that caught my attention is that you're using tokio's RwLock which is a sign that something might be wrong with the design. It is very rarely needed since there are very few cases where it makes sense to hold a lock across awaits, which is the purpose of the tokio lock. Normally the std RwLock is used instead. If you're interested in exploring this, please describe the purpose of the lock and how you're using it, and people here can suggest alternatives. This may allow you to simplify some things.

4 Likes
3

If you're always downcasting to the same type (or set of types), consider a design where you don't type erase to Box<dyn Any + ..> and instead return a concrete base type, associated type, or enum. Might be a deep rewrite or impossible if you don't own the traits though.

You can encapsulate some logic into extension traits.

pub trait RwLockMappedWriteGuardExt {
    fn downcast_mut<T: Any>(&mut self) -> Result<&mut T, anyhow::Error>;
}

impl RwLockMappedWriteGuardExt for RwLockMappedWriteGuard<'_, Option<TransactionContext>> {
    fn downcast_mut<T: Any>(&mut self) -> Result<&mut T, anyhow::Error> {
        self.as_mut()
            .ok_or(anyhow!("Transaction context not exist."))?
            .get_mut()
            .downcast_mut::<T>()
            .ok_or_else(|| { anyhow!(
                "Can not downcast any to {}.",
                std::any::type_name::<T>(),
            )})
    }
}

async fn example(ctx: impl ActionContext) -> Result<(), anyhow::Error>{
    let mut guard = ctx.transaction().await;
    let tx = guard
        .downcast_mut::<TransactionContextImpl>()?
        .get_transaction();
    Ok(())
}
3 Likes
4

Thanks for your response! I really like your idea of using TransactionGuard; it's very close to what I was trying to achieve with owning_ref. However, since TransactionGuard is my own type, I will not have issues with trait bounds. I'll definitely give your suggestion a try!

Currently, I'm using RwLocks as a quick and dirty solution, but for AuthorizationContext, I'm replaced it with OnceLock. I understand your point and will rethink my approach. I believe I can use std::RwLock or std::Mutex<RefCell<T>> here. I just need interior mutability. Since I'm not spawning other tasks that make database requests within the scope of a single HTTP request, the transaction lock is acquired sequentially, and other threads shouldn't be blocked by a locked transaction. Thanks again!)

1 Like
5

Thanks for your response! I need type erasure because I don't want to expose the concrete type of the transaction outside the database adapter. The ActionContext object moves through all layers of the application, so it's intentional to apply a ports-and-adapters architecture. This design used for generic transaction management in the service layer, with the concrete implementation of transaction manager contained within the database adapter.

I also like the idea of trait extension. I'm interested in trying this solution and comparing it with @ jumpnbrownweasel's solution with the guard wrapper. Thanks!

6

This post is getting off-topic I feel, so you can ignore it if you like. Probably I just don't understand your overall architecture.

I'm not sure I understand. Is there literally one type[1] you're trying to protect? This makes it look like it could be many types...

impl TransactionContext {
    pub fn new(tx_context: impl TransactionContextMarker) -> TransactionContext {
        Self(Box::new(tx_context))
    }

...but you haven't shared TransactionalContextMarker so it's hard to say. What is clear is that the code you've asked about is assuming there is one particular type inside. And the field of TranactionContext at least is not public.

Incidentally, you also have

    pub fn get_mut(&mut self) -> &mut Box<dyn Any + Send + Sync> {
        &mut self.0
    }

Are you aware that this allows swapping out the inner item?[2]

fn someone_you_are_trying_to_hide_impl_details_from(tc: &mut TransactionContext) {
    let _yoink = std::mem::replace(tc.get_mut(), Box::new(()));
}

Maybe you wanted to return &mut dyn ...

  1. or a fixed set ↩︎

  2. And with an erased type that doesn't necessarily implement TransactionContextMarker, unless that's literally : Send + Sync + 'static and blanket-implemented for all possible Sized implementors. ↩︎

1 Like
7

I'm protecting sqlx's Transaction<'_, Postgres>. I split the project into several crates: a couple of domain crates and one crate per adapter, each implementing specific port traits from the domain crate. My domain crates don't depend on sqlx or any concrete transaction type. Instead, the concrete transaction type is created and injected into ActionContext by concrete implementation of TransactionManager (generic type of TransactionManager trait i use in service layer) within the sqlx adapter, and it's also downcasted in concrete type in repository's concrete implementation, which is also contained within the sqlx adapter crate. I'm the only user of this code, and this encapsulation is intended to maintain separation of concerns between the different layers (crates) of the application.

pub trait TransactionContextMarker: Any + Send + Sync + 'static {}

TransactionContextMarker is a marker trait for concrete implementations of the transaction context. I use it to ensure that the TransactionContext::new() function is protected from creating a TransactionContext containing a type that implements Any but isn't expected.

Regarding the need for get_mut(), it's necessary because sqlx requires &mut Transaction to execute queries. I see this isn't an ideal sound interface, but so far, I haven't found a better solution).

8

Thanks for spelling it out more!

This will probably work fine:

-    pub fn get_mut(&mut self) -> &mut Box<dyn Any + Send + Sync> {
-        &mut self.0
+    pub fn get_mut(&mut self) -> &mut (dyn Any + Send + Sync + 'static) {
+        &mut *self.0
     }
2 Likes