search

cashapp / sqldelight

SQLDelight - Generates typesafe Kotlin APIs from SQL
https://cashapp.github.io/sqldelight/
Apache License 2.0
6.01k stars 501 forks source link

Add a SqlDriver for Androidx KMP SQLiteDriver #5283

Open eygraber opened 3 weeks ago

eygraber commented 3 weeks ago

Androidx has a SQLiteDriver that targets Android, iOS, Mac, Linux, and JVM Desktop.

This PR provides a SqlDriver that wraps it. Currently the implementation only works for Android and JVM (it uses a ThreadLocal and I wanted to get some feedback before expanding that functionality to the native targets).

eygraber commented 3 weeks ago

Once I'm here, this line from AndroidSqliteDriver tripped me up. I can't figure out in what scenario the previous connection should be closed, because if identifier != null then either there was no connection in the cache and a new one is made, or the connection was reused and removed from the cache so there's nothing to close.

JakeWharton commented 3 weeks ago

Once I'm here, this line from AndroidSqliteDriver tripped me up. I can't figure out in what scenario the previous connection should be closed, because if identifier != null then either there was no connection in the cache and a new one is made, or the connection was reused and removed from the cache so there's nothing to close.

Two threads racing at the same time.

eygraber commented 3 weeks ago

Two threads racing at the same time.

Should've been my first guess :sweat_smile:

cache4k doesn't have the same semantics as LruCache so to handle that you need to listen for an Update event, and if the values don't match each other, close the old one.

eygraber commented 3 weeks ago

Some weird stuff going on with tests that I'm working through.

Also had an idea for how to make this work on native. Should I do that here, or have a follow up PR?

eygraber commented 3 weeks ago

Got a native implementation in. I don't have the most experience with Native APIs, but I think this should clean up after itself since in endTransaction eventually transactions.set(enclosingTransaction) will get called with a null value (I think).

I'm also good with removing that part for now. In any case this should be ready to go.