Open frederikhors opened 1 month ago
If we permit leaking transactions outside of repositories' scope, sqlx
has a trait called Executor
which is implemented for pools, connections, and transactions (some of them via deref):
async fn check_unique_constraint(exec: impl sqlx::Executor) {
sqlx::query!("...").fetch(exec).await
}
check_unique_constraint(&pool).await;
check_unique_constraint(&mut *conn).await;
check_unique_constraint(&mut *tx).await;
If you want check_unique_constraint
to control commits and rollbacks, you can replace Executor
with Acquire
:
async fn check_unique_constraint(conn: impl sqlx::Acquire) {
let mut tx = conn.acquire().await?;
sqlx::query!("...").fetch(&mut *tx).await
}
Now the hard part is to decide how you can provide a concrete type to use it in a trait's signature. Since generics are not allowed in object-safe traits, I can think of two approaches on the spot:
Use an associated type:
trait Repo {
type Conn;
async fn check_unique_constraint(c: Self::Conn);
}
type MyRepo = Arc<dyn Repo<Conn = MyConn>>;
type MyConn = Box<dyn sqlx::Connection>;
Now, this kinda beats the purpose of a repo, since we have to explicitly depend on sqlx::Connection
.
Use type erasure:
trait Repo {
async fn connect(&self) -> RepoConn;
async fn check_unique_constraint(&self, c: RepoConn);
}
Where
struct RepoConn(Box<dyn Any>);
impl RepoConn {
fn new<T: Any>(value: T) -> Self {
Self(Box::new(value))
}
fn downcast<T: Any>(self) -> Option<Box<T>> {
self.0.downcast().ok()
}
}
So that implementation can give out connections and get them back.
impl Repo for MyRepo {
async fn connect(&self) -> RepoConn {
let tx = self.pg_pool.begin().await.unwrap();
RepoConn::new(tx)
}
async fn check_unique_constraint(&self, c: RepoConn) {
let tx = c.downcast::<sqlx::Transaction>().expect("connection should be downcastable to `sqlx::Transaction`");
// Use `tx`.
}
}
Error handling omitted.
Granted, both of them are kinda suboptimal: the first one requires us to explicitly state associated type (making us depend on it), and the second loses compile-time safety (forcing us to do it at runtime via type erasure).
If we don't leak transactions outside of repositories, then we don't have this problem :)
Please correct me if I'm wrong or didn't think of another approach.
P.S. You can also do associated type with a trait bound, passing dyn RepoConn
around, but I don't think it's useful (?) since you need concrete type in the implementation itself.
Wow, thanks @TmLev. This is a lot to digest for a newbie like me!
I asked a very similar thing on SO a while back and this was the response, what do you two think?
If we permit leaking transactions outside of repositories' scope
I am studying this problem in depth because it is the last thing I have left to solve and it is FUNDAMENTAL above all to have a rapid and easy development (also considering testability).
Let's say there is a service with the following methods:
create_team()
create_player()
create_team_with_players()
and a repo with the same methods:
create_team()
create_player()
create_team_with_players()
(note each repo method know how to "persist" each model which is in turn composed of other models (complex structs) so I cannot simply repeat the persisting logic everywhere because is a lot)
I can, in order:
from service/create_team_with_players()
call repo/create_team_with_players()
call repo/create_team()
for each player call repo/create_player()
and as you can see I need to call each repo
method within a transaction (and I'm still not leaking transaction outside of repositories' scope).
But since each of that repo
method can be called from a service too I need to leak in the repo method signature the optional transaction.
So leaking outside the scope of the repositories is inevitable!
pub trait PlayerRepository: Send + Sync + Clone + 'static {
fn create_player(
&self,
db_conn: Option<&mut sqlx::PgConnection>,
req: &CreatePlayerRequest,
) -> impl Future<Output = Result<Player, CreatePlayerError>> + Send;
}
Am I right?
One way that I think can avoid the leak is to create repository methods without db_conn: Option<&mut sqlx::PgConnection>
and the same methods to implement in outbound/pg.rs
that can be called with a db_conn: Option<&mut sqlx::PgConnection>
, like this:
pub trait PlayerRepository: Send + Sync + Clone + 'static {
fn create_player(
&self,
req: &CreatePlayerRequest,
) -> impl Future<Output = Result<Player, CreatePlayerError>> + Send;
}
pub struct Pg {
pool: PgPool,
}
impl Pg {
async fn create_player(
&self,
db_conn: Option<&mut sqlx::PgConnection>,
req: &CreatePlayerRequest,
) -> Result<Player, CreatePlayerError>> {
// Save player using db_conn or self.pool
}
}
impl PlayerRepository for Pg {
fn create_player(
&self,
req: &CreatePlayerRequest,
) -> impl Future<Output = Result<Player, CreatePlayerError>> + Send {
self.create_player(None, req)
}
}
What do you think?
And @AngusGMorrison, is everything ok? Are you there? Are you writing juicy new episodes? Give us a silent sign of your presence! LOL 🤣
Hi dear and amazing @AngusGMorrison,
a doubt:
In the repository function
create_author
I need to call another repository method (example:check_if_author_name_is_already_taken(req.name())
) and I need to call it within the same transaction I'm creating on line 50.How to?
Plus the same method can be called not in a transaction too!
Should I have a new argument on the repository method?
An argument to indicate both a db connection or a db transaction?
Something like:
I'm asking for you help too, @TmLev.