Allow custom SupportSQLiteOpenHelper.Callback in the SqlNormalizedCacheFactory

[rohandhruva]

A quick PR to allow custom callbacks in the SqlNormalizedCacheFactory. This should allow us to get lifecycle events and apply custom database settings if needed. The eventual plan is to allow us to try something like this: https://stackoverflow.com/questions/65425352/sqldelight-slow-performance-compared-to-room

[netlify]

👷 Deploy request for apollo-android-docs pending review.

Visit the deploys page to approve it

Name	Link
🔨 Latest commit	7427688

[rohandhruva]

I just realized that this approach will mean that callers have to import sqldelight dependency to be able to delegate to the upstream AndroidSqliteDriver.Callback as well as get a reference to the schema.

I wonder if it's ok for apollo-kotlin to do the delegation to the upstream callback? Or does it feel like too much "business logic" at this point?

[BoD]

I wonder if it's ok for apollo-kotlin to do the delegation to the upstream callback? Or does it feel like too much "business logic" at this point?

I think doing so would avoid potential user mistake, and simplify the usage, right? I'm not 100% sure of the consequences of not calling the default sqldelight callback, but it looks like it would break (e.g. the schema wouldn't be created)? So if it must be called always, we might as well do it in the library?

[martinbonnin]

What about a configure function?

  @JvmOverloads
  constructor(
      context: Context,
      name: String? = "apollo.db",
      factory: SupportSQLiteOpenHelper.Factory = FrameworkSQLiteOpenHelperFactory(),
      configure: (SupportSQLiteDatabase) -> Unit,
      useNoBackupDirectory: Boolean = false,
  ) : this(
      AndroidSqliteDriver(
          getSchema(),
          context.applicationContext,
          name,
          factory,
          object : AndroidSqliteDriver.Callback(getSchema()) {
            override fun onConfigure(db: SupportSQLiteDatabase) {
              super.onConfigure(db)
              configure(db)
            }
          },
          useNoBackupDirectory = useNoBackupDirectory
      ),
  ) 

Doesn't leak sqldelight in the public API and mandates that calls to AndroidSqliteDriver.Callback are made?

[rohandhruva]

I think doing so would avoid potential user mistake, and simplify the usage, right? I'm not 100% sure of the consequences of not calling the default sqldelight callback, but it looks like it would break (e.g. the schema wouldn't be created)? So if it must be called always, we might as well do it in the library?

Yes, I think skipping the call to the upstream callback (AndroidSqliteDriver) is not an option. It is bound to cause a lot of other issues.

What about a configure function?
...
Doesn't leak sqldelight in the public API and mandates that calls to AndroidSqliteDriver.Callback are made?

I think that works for now, and I also like the fact that it does not introduce a sqldelight API as part of the apollo-kotlin API. However, in the future, if we end up needing to introducing more more of the callback's APIs, we'll need to add them all to the constructor (e.g. if someone wants a custom onCreate in the future).

Maybe it's ok to just have configure function for now..?

[martinbonnin]

we'll need to add them all to the constructor (e.g. if someone wants a custom onCreate in the future).

Let's revisit when that happens? My guiding principle here is that in the benefit of the doubt, the most restrictive API is safer. It's always easier to add a constructor than remove one

[BoD]

👍