The largest changes are caused by the associated upgrade of hyper to
1.0, including the removal of axum::Server (replaced by axum::serve)
and http body utilities (replaced by http-body-util). These changes
were purely mechanical.
A more subtle change is the requirement from http 1.0 for request
extensions to be Clone. The Lazy transaction wrapper is not Clone,
so it has been wrapped in an Arc, with Arc::get_mut being used to
obtain the necessary mutability. This feels like it might be buggy,
since other middleware could potentially clone and store request
extensions, without otherwise interfering with the transaction (meaning
the Tx extractor should still work). This needs subsequent
investigation before release.
84a49ae fix: fix buggy behaviour when cloning request extensions
Our naive update for axum@0.7 / hyper@1.0 led to buggy behaviour whereby
the Tx extractor would always fail with OverlappingExtractors if
there were any outstanding clones of the request extensions (see the new
test). Since request extensions now must all implement Clone, it's
possible that some middleware might wish to keep a clone of all request
extensions (e.g. for request inspection/tracing/debugging), rendering
Tx unusable with those middleware.
To fix it, we simplify the synchronisation by implementing Clone for
Slot and creating new Extension<DB> and LazyTransaction<DB> types
to replace TxSlot<DB> and Lazy<DB>.
Slot<T> is a wrapper around an Arc<Mutex<Option<T>>>, and as such it
can trivially implement Clone (there was some "pit of success"
considerations with the previous API intended to enforce proper usage,
but that is unnecesarily limiting given the underlying Mutex).
The Extension<DB> holds a Slot containing a LazyTransaction<DB>.
Extension<DB> is trivially clonable since Slot itself is. The
LazyTransaction<DB> then implements a simple "lazily acquired
transaction" protocol, making use of normal rust ownership and borrowing
rules to manage the transaction (i.e. it has no internal
synchronisation).
This makes the overall synchronisation picture much simpler: the
middleware future and all clones of the request extension hold a
reference to the same Slot. The Tx extractor obtains its copy of the
request extension and attempts to lease the inner LazyTransaction,
failing with OverlappingExtractors if the lease is already taken (this
is the only public invocation of lease, and so overlapping extractors
can be the only* cause of an absent transaction). If the lease is
successful, the extractor can acquire a transaction (if there's not one
already) and package it up for request handlers to then interact with.
Technically the transaction can be "stolen" from the Tx extractor by
committing explicitly, but this considered to create an endless
"overlap" in the current semantics.
The main caveat of this approach seems to be that the Tx extractor no
longer has type-level knowledge that it can access a Transaction - the
Transaction can only be accessed by matching on the LazyTransaction
state. This doesn't affect the API, but it could make bumping into a
panic more likely, or there may be performance implications (though
these would likely be dwarfed by the I/O involved in interacting with a
database).
1cec0ae refactor: ditch slot
It turns out the parking_lotarc_lock future does the main thing we
want from Slot, giving us 'static lock guards that unlock the value
on drop. The only missing functionality is the "stealing" we need to
obtain ownership in order to commit the transaction. Rather than
implementing this via Option, we add an additional state to
LazyTransaction and handle it there.
Ultimately this removes a lot of code, and makes the synchronisation
mechanism even less exotic.
9e9827f chore: upgrade axum to 0.7
The largest changes are caused by the associated upgrade of hyper to 1.0, including the removal of
axum::Server(replaced byaxum::serve) and http body utilities (replaced byhttp-body-util). These changes were purely mechanical.A more subtle change is the requirement from
http1.0 for request extensions to beClone. TheLazytransaction wrapper is notClone, so it has been wrapped in anArc, withArc::get_mutbeing used to obtain the necessary mutability. This feels like it might be buggy, since other middleware could potentially clone and store request extensions, without otherwise interfering with the transaction (meaning theTxextractor should still work). This needs subsequent investigation before release.84a49ae fix: fix buggy behaviour when cloning request extensions
Our naive update for axum@0.7 / hyper@1.0 led to buggy behaviour whereby the
Txextractor would always fail withOverlappingExtractorsif there were any outstanding clones of the request extensions (see the new test). Since request extensions now must all implementClone, it's possible that some middleware might wish to keep a clone of all request extensions (e.g. for request inspection/tracing/debugging), renderingTxunusable with those middleware.To fix it, we simplify the synchronisation by implementing
CloneforSlotand creating newExtension<DB>andLazyTransaction<DB>types to replaceTxSlot<DB>andLazy<DB>.Slot<T>is a wrapper around anArc<Mutex<Option<T>>>, and as such it can trivially implementClone(there was some "pit of success" considerations with the previous API intended to enforce proper usage, but that is unnecesarily limiting given the underlyingMutex).The
Extension<DB>holds aSlotcontaining aLazyTransaction<DB>.Extension<DB>is trivially clonable sinceSlotitself is. TheLazyTransaction<DB>then implements a simple "lazily acquired transaction" protocol, making use of normal rust ownership and borrowing rules to manage the transaction (i.e. it has no internal synchronisation).This makes the overall synchronisation picture much simpler: the middleware future and all clones of the request extension hold a reference to the same
Slot. TheTxextractor obtains its copy of the request extension and attempts toleasethe innerLazyTransaction, failing withOverlappingExtractorsif the lease is already taken (this is the only public invocation oflease, and so overlapping extractors can be the only* cause of an absent transaction). If the lease is successful, the extractor can acquire a transaction (if there's not one already) and package it up for request handlers to then interact with.Txextractor by committing explicitly, but this considered to create an endless "overlap" in the current semantics.The main caveat of this approach seems to be that the
Txextractor no longer has type-level knowledge that it can access aTransaction- theTransactioncan only be accessed by matching on theLazyTransactionstate. This doesn't affect the API, but it could make bumping into a panic more likely, or there may be performance implications (though these would likely be dwarfed by the I/O involved in interacting with a database).1cec0ae refactor: ditch
slotIt turns out the
parking_lotarc_lockfuture does the main thing we want fromSlot, giving us'staticlock guards that unlock the value on drop. The only missing functionality is the "stealing" we need to obtain ownership in order to commit the transaction. Rather than implementing this viaOption, we add an additional state toLazyTransactionand handle it there.Ultimately this removes a lot of code, and makes the synchronisation mechanism even less exotic.