Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow custom SqlDriver to support multi-platform encryption #4504

Closed
izakvdhoven opened this issue Nov 10, 2022 · 5 comments
Closed

Allow custom SqlDriver to support multi-platform encryption #4504

izakvdhoven opened this issue Nov 10, 2022 · 5 comments

Comments

@izakvdhoven
Copy link

izakvdhoven commented Nov 10, 2022

Use case

I need to be able to encrypt the normalised cache on non-Android targets (specifically iOS).

SqlDelight supports configuring a SqlDriver for encryption:

val configuration = DatabaseConfiguration(
    encryptionConfig = DatabaseConfiguration.Encryption("MyCacheEncryptionKey"),
    ...
)

val driver: SqlDriver = NativeSqliteDriver(configuration)

Describe the solution you'd like

I would like for the SqlNormalizedCacheFactory constructor to be made public, to allow supplying a customised SqlDriver when instantiating SqlNormalizedCacheFactory.

I understand that the constructor is internal to allow SqlDelight to remain an implementation detail, but this hides functionality already supported by the dependency.

Perhaps an overloaded constructor that allows passing an api-agnostic encryption config (eg, SqlNormalisedCacheEncryptionConfig) would solve the problem without leaking the SqlDelight dependency?

@martinbonnin
Copy link
Contributor

It might be ok to expose SQLDelight transitively. After all, we're doing it for Okio and OkHttp already so it would be somewhat consistent.

The only thing that makes this code complicated is the expect/actual declaration. It's currently possible to create a SQLiteNormalizedCache from commonMain code by passing a name but in your case, I guess the code that creates the drivers is platform specific anyway so there's no need for a expect constructor(driver: SqlDriver), right?

@izakvdhoven
Copy link
Author

the code that creates the drivers is platform specific anyway so there's no need for a expect constructor(driver: SqlDriver), right?

Correct.

SqlNormalizedCacheFactory already has a default constructor (not expect-actual) defined in androidMain, appleMain and jvmMain

actual class SqlNormalizedCacheFactory internal constructor(
    private val driver: SqlDriver,
) : NormalizedCacheFactory() {

Simply making this default constructor public should solve my problem without incurring additional overhead elsewhere.

@martinbonnin
Copy link
Contributor

martinbonnin commented Nov 10, 2022

Want to open a PR about this? (you'll most likely have to run ./gradlew apiDump to update the public API).

@izakvdhoven
Copy link
Author

Happy to take a stab at it.

Thanks!

@martinbonnin
Copy link
Contributor

Fixed with #4806

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants