Kotlin coroutine support #2524
Kotlin coroutine support #2524
Conversation
|
@anderssv Feedback very much welcome. I am not exactly a Kotlin person so I kind of puzzled this together with some docs, common sense and trial-and-error. This could benefit from review / feedback from people that actually use Jdbi with Kotlin. |
hgschmie
force-pushed
the
handle-coroutines
branch
from
7b51c4b
to
2ca1365
Compare
|
This addresses #2522 |
| return decoratedCallback.withHandle(h); | ||
| } finally { | ||
| threadHandleSupplier.remove(); | ||
| getHandleSupplierHolder().clearHandleSupplier(); |
There was a problem hiding this comment.
should you look up the handle supplier holder here again? seems like the lookup can be done once, and you always set / clear on the same instance.
There was a problem hiding this comment.
can we move the review of the handle supplier holder code to #2523 ?
| @@ -521,16 +532,16 @@ public <X extends Exception> void useTransaction(final TransactionIsolationLevel | |||
| */ | |||
| public <R, E, X extends Exception> R withExtension(Class<E> extensionType, ExtensionCallback<R, E, X> callback) | |||
| throws X { | |||
| HandleSupplier handleSupplier = threadHandleSupplier.get(); | |||
| HandleSupplier handleSupplier = getHandleSupplierHolder().getHandleSupplier(); | |||
There was a problem hiding this comment.
same here, feels like we could re-use the same handleSupplierHolder
| * @param <T> type parameter | ||
| */ | ||
| @Alpha | ||
| public <T extends HandleSupplierHolder> T getHandleSupplierHolder() { |
There was a problem hiding this comment.
Since this is not intended as public API, can we restrict visibility? Also, I think @Alpha does not need to be applied to non-public api, since it is always subject to change at any time.
There was a problem hiding this comment.
Unfortunately not. This is specifically intended to be used by the Kotlin code to allow doing this:
fun Jdbi.attachToContext(): CoroutineContext.Element = this.getHandleSupplierHolder<CoroutineAwareHandleSupplierHolder>().contextElement()This allows attaching the Jdbi object to a coroutine context so that the handles in the internal threadlocal are moved around correctly.
The extension function above is (in terms of java code) a hidden static function (so basically it is
public static CoroutineContext.Element attachToContext() {
return Jdbi<CoroutineAwareHandleSupplierHolder>getHandleSupplierHolder().contextElement();
}"patched" into the Jdbi class. And that is only possible with public members (because the extension method lives in a different package (org.jdbi.v3.core.kotlin) and is not an actual member of the hierarchy, so neither protected nor package scope would work).
| */ | ||
| @Alpha | ||
| public <T extends HandleSupplierHolder> T getHandleSupplierHolder() { | ||
| return (T) handleSupplierHolder.updateAndGet(x -> x != null ? x : config.get(Handles.class).getHandleSupplierHolder()); |
There was a problem hiding this comment.
this is a little weird if you ever try to change the handle supplier holder, since this reference will never update.
There was a problem hiding this comment.
It allows changing the implementation class to be modified by calling
Jdbi jdbi = Jdbi.create(...);
jdbi.getConfig(Handles.class).setHandleSupplierHolderClass(MyMagicHandleSupplierHolder.class)
... more codeIf the implementation would be created when the Jdbi is created, it could not be changed by configuration. If we allow changing at any time, there is a good chance that we might leak handles.
Open for better ideas, this is what I came up with.
hgschmie
force-pushed
the
handle-coroutines
branch
2 times, most recently
from
4c1f145
to
32264da
Compare
|
keep this in draft until we have hashed out #2523 and I figured out the sporadically failing tests. |
hgschmie
force-pushed
the
handle-coroutines
branch
10 times, most recently
from
d0c4a20
to
cac69ea
Compare
hgschmie
force-pushed
the
handle-coroutines
branch
3 times, most recently
from
cbdaa0f
to
e2cdcc9
Compare
Allows using Jdbi objects (handles and extension objects) with kotlin coroutines. Objects can be shared between coroutines (even across threads) as long as the caller can guarantee that not multiple coroutines use them at the same time.
hgschmie
force-pushed
the
handle-coroutines
branch
from
e2cdcc9
to
763edf3
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
|
Awesome work guys! Sorry I didn't see the request for input earlier. I have only a rudimentary understanding of Coroutines myself, but on the surface it looks good to me. May I suggest popping by the Kotlin Slack and see if anyone in the coroutines channel has any input? :) |
Support Kotlin coroutines
Allows using Jdbi objects (handles and extension objects) with kotlin coroutines. Objects can be shared between coroutines (even across threads) as long as the caller can guarantee that not multiple coroutines use them at the same time.