Skip to content

Add NormalizedCacheFactory.close() #67

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add NormalizedCacheFactory.close() #67

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Next version (unreleased)

PUT_CHANGELOG_HERE
- Add `NormalizedCacheFactory.close()` (#66)

# Version 0.0.4
_2024-11-07_
Expand Down
3 changes: 2 additions & 1 deletion normalized-cache-incubating/api/normalized-cache-incubating.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ public final class com/apollographql/cache/normalized/api/NormalizedCache$Compan
public final fun prettifyDump (Ljava/util/Map;)Ljava/lang/String;
}

public abstract class com/apollographql/cache/normalized/api/NormalizedCacheFactory {
public abstract class com/apollographql/cache/normalized/api/NormalizedCacheFactory : java/io/Closeable {
public fun <init> ()V
public abstract fun create ()Lcom/apollographql/cache/normalized/api/NormalizedCache;
}
Expand Down Expand Up @@ -523,6 +523,7 @@ public final class com/apollographql/cache/normalized/memory/MemoryCacheFactory
public fun <init> (IJ)V
public synthetic fun <init> (IJILkotlin/jvm/internal/DefaultConstructorMarker;)V
public final fun chain (Lcom/apollographql/cache/normalized/api/NormalizedCacheFactory;)Lcom/apollographql/cache/normalized/memory/MemoryCacheFactory;
public fun close ()V
public synthetic fun create ()Lcom/apollographql/cache/normalized/api/NormalizedCache;
public fun create ()Lcom/apollographql/cache/normalized/memory/MemoryCache;
}
Expand Down
3 changes: 2 additions & 1 deletion normalized-cache-incubating/api/normalized-cache-incubating.klib.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ abstract class com.apollographql.cache.normalized.api/CacheKeyResolver : com.apo
final fun resolveField(com.apollographql.cache.normalized.api/ResolverContext): kotlin/Any? // com.apollographql.cache.normalized.api/CacheKeyResolver.resolveField|resolveField(com.apollographql.cache.normalized.api.ResolverContext){}[0]
open fun listOfCacheKeysForField(com.apollographql.cache.normalized.api/ResolverContext): kotlin.collections/List<com.apollographql.cache.normalized.api/CacheKey?>? // com.apollographql.cache.normalized.api/CacheKeyResolver.listOfCacheKeysForField|listOfCacheKeysForField(com.apollographql.cache.normalized.api.ResolverContext){}[0]
}
abstract class com.apollographql.cache.normalized.api/NormalizedCacheFactory { // com.apollographql.cache.normalized.api/NormalizedCacheFactory|null[0]
abstract class com.apollographql.cache.normalized.api/NormalizedCacheFactory : okio/Closeable { // com.apollographql.cache.normalized.api/NormalizedCacheFactory|null[0]
abstract fun create(): com.apollographql.cache.normalized.api/NormalizedCache // com.apollographql.cache.normalized.api/NormalizedCacheFactory.create|create(){}[0]
constructor <init>() // com.apollographql.cache.normalized.api/NormalizedCacheFactory.<init>|<init>(){}[0]
}
Expand Down Expand Up @@ -275,6 +275,7 @@ final class com.apollographql.cache.normalized.memory/MemoryCache : com.apollogr
final class com.apollographql.cache.normalized.memory/MemoryCacheFactory : com.apollographql.cache.normalized.api/NormalizedCacheFactory { // com.apollographql.cache.normalized.memory/MemoryCacheFactory|null[0]
constructor <init>(kotlin/Int = ..., kotlin/Long = ...) // com.apollographql.cache.normalized.memory/MemoryCacheFactory.<init>|<init>(kotlin.Int;kotlin.Long){}[0]
final fun chain(com.apollographql.cache.normalized.api/NormalizedCacheFactory): com.apollographql.cache.normalized.memory/MemoryCacheFactory // com.apollographql.cache.normalized.memory/MemoryCacheFactory.chain|chain(com.apollographql.cache.normalized.api.NormalizedCacheFactory){}[0]
final fun close() // com.apollographql.cache.normalized.memory/MemoryCacheFactory.close|close(){}[0]
final fun create(): com.apollographql.cache.normalized.memory/MemoryCache // com.apollographql.cache.normalized.memory/MemoryCacheFactory.create|create(){}[0]
}
final class com.apollographql.cache.normalized/CacheInfo : com.apollographql.apollo.api/ExecutionContext.Element { // com.apollographql.cache.normalized/CacheInfo|null[0]
Expand Down
4 changes: 3 additions & 1 deletion ...ng/src/commonMain/kotlin/com/apollographql/cache/normalized/api/NormalizedCacheFactory.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package com.apollographql.cache.normalized.api

import okio.Closeable

/**
* A Factory used to construct an instance of a [NormalizedCache] configured with the custom scalar adapters set in
* ApolloClient.Builder#addCustomScalarAdapter(ScalarType, CustomScalarAdapter).
*/
abstract class NormalizedCacheFactory {
abstract class NormalizedCacheFactory : Closeable {

/**
* ApolloClient.Builder#addCustomScalarAdapter(ScalarType, CustomScalarAdapter).
Expand Down
4 changes: 4 additions & 0 deletions ...incubating/src/commonMain/kotlin/com/apollographql/cache/normalized/memory/MemoryCache.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,4 +176,8 @@ class MemoryCacheFactory @JvmOverloads constructor(
expireAfterMillis = expireAfterMillis,
)
}

override fun close() {
nextCacheFactory?.close()
}
}
5 changes: 4 additions & 1 deletion normalized-cache-sqlite-incubating/api/android/normalized-cache-sqlite-incubating.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,17 @@ public final class com/apollographql/cache/normalized/sql/SqlNormalizedCache : c
public fun remove (Ljava/lang/String;)I
}

public final class com/apollographql/cache/normalized/sql/SqlNormalizedCacheFactoryKt {
public static final fun SqlNormalizedCacheFactory (Lapp/cash/sqldelight/db/SqlDriver;)Lcom/apollographql/cache/normalized/api/NormalizedCacheFactory;
}

public final class com/apollographql/cache/normalized/sql/SqlNormalizedCacheFactory_androidKt {
public static final fun SqlNormalizedCacheFactory (Landroid/content/Context;)Lcom/apollographql/cache/normalized/api/NormalizedCacheFactory;
public static final fun SqlNormalizedCacheFactory (Landroid/content/Context;Ljava/lang/String;)Lcom/apollographql/cache/normalized/api/NormalizedCacheFactory;
public static final fun SqlNormalizedCacheFactory (Landroid/content/Context;Ljava/lang/String;Landroidx/sqlite/db/SupportSQLiteOpenHelper$Factory;)Lcom/apollographql/cache/normalized/api/NormalizedCacheFactory;
public static final fun SqlNormalizedCacheFactory (Landroid/content/Context;Ljava/lang/String;Landroidx/sqlite/db/SupportSQLiteOpenHelper$Factory;Lkotlin/jvm/functions/Function1;)Lcom/apollographql/cache/normalized/api/NormalizedCacheFactory;
public static final fun SqlNormalizedCacheFactory (Landroid/content/Context;Ljava/lang/String;Landroidx/sqlite/db/SupportSQLiteOpenHelper$Factory;Lkotlin/jvm/functions/Function1;Z)Lcom/apollographql/cache/normalized/api/NormalizedCacheFactory;
public static final fun SqlNormalizedCacheFactory (Landroid/content/Context;Ljava/lang/String;Landroidx/sqlite/db/SupportSQLiteOpenHelper$Factory;Lkotlin/jvm/functions/Function1;ZLjava/lang/Long;)Lcom/apollographql/cache/normalized/api/NormalizedCacheFactory;
public static final fun SqlNormalizedCacheFactory (Lapp/cash/sqldelight/db/SqlDriver;)Lcom/apollographql/cache/normalized/api/NormalizedCacheFactory;
public static final fun SqlNormalizedCacheFactory (Ljava/lang/String;)Lcom/apollographql/cache/normalized/api/NormalizedCacheFactory;
public static synthetic fun SqlNormalizedCacheFactory$default (Landroid/content/Context;Ljava/lang/String;Landroidx/sqlite/db/SupportSQLiteOpenHelper$Factory;Lkotlin/jvm/functions/Function1;ZLjava/lang/Long;ILjava/lang/Object;)Lcom/apollographql/cache/normalized/api/NormalizedCacheFactory;
public static synthetic fun SqlNormalizedCacheFactory$default (Ljava/lang/String;ILjava/lang/Object;)Lcom/apollographql/cache/normalized/api/NormalizedCacheFactory;
Expand Down
6 changes: 5 additions & 1 deletion normalized-cache-sqlite-incubating/api/jvm/normalized-cache-sqlite-incubating.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@ public final class com/apollographql/cache/normalized/sql/SqlNormalizedCache : c
public fun remove (Ljava/lang/String;)I
}

public final class com/apollographql/cache/normalized/sql/SqlNormalizedCacheFactory_jvmKt {
public final class com/apollographql/cache/normalized/sql/SqlNormalizedCacheFactoryKt {
public static final fun SqlNormalizedCacheFactory (Lapp/cash/sqldelight/db/SqlDriver;)Lcom/apollographql/cache/normalized/api/NormalizedCacheFactory;
}

public final class com/apollographql/cache/normalized/sql/SqlNormalizedCacheFactory_jvmKt {
public static final fun SqlNormalizedCacheFactory (Ljava/lang/String;)Lcom/apollographql/cache/normalized/api/NormalizedCacheFactory;
public static final fun SqlNormalizedCacheFactory (Ljava/lang/String;Ljava/lang/String;)Lcom/apollographql/cache/normalized/api/NormalizedCacheFactory;
public static final fun SqlNormalizedCacheFactory (Ljava/lang/String;Ljava/util/Properties;)Lcom/apollographql/cache/normalized/api/NormalizedCacheFactory;
Expand All @@ -19,6 +22,7 @@ public final class com/apollographql/cache/normalized/sql/SqlNormalizedCacheFact
}

public final class com/apollographql/cache/normalized/sql/TrimmableNormalizedCacheFactory : com/apollographql/cache/normalized/api/NormalizedCacheFactory {
public fun close ()V
public fun create ()Lcom/apollographql/cache/normalized/api/NormalizedCache;
}

Expand Down
12 changes: 2 additions & 10 deletions ...idMain/kotlin/com/apollographql/cache/normalized/sql/SqlNormalizedCacheFactory.android.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,11 @@ import androidx.sqlite.db.SupportSQLiteOpenHelper
import androidx.sqlite.db.framework.FrameworkSQLiteOpenHelperFactory
import com.apollographql.cache.normalized.api.NormalizedCacheFactory
import com.apollographql.cache.normalized.sql.internal.createDriver
import com.apollographql.cache.normalized.sql.internal.createRecordDatabase
import com.apollographql.cache.normalized.sql.internal.getSchema
import app.cash.sqldelight.driver.android.AndroidSqliteDriver
import app.cash.sqldelight.db.SqlDriver
import com.apollographql.cache.normalized.api.NormalizedCache

actual fun SqlNormalizedCacheFactory(driver: SqlDriver): NormalizedCacheFactory = object : NormalizedCacheFactory() {
override fun create(): NormalizedCache {
return SqlNormalizedCache(createRecordDatabase(driver))
}
}

actual fun SqlNormalizedCacheFactory(name: String?): NormalizedCacheFactory =
SqlNormalizedCacheFactory(createDriver(name, null, getSchema()))
SqlNormalizedCacheFactory(createDriver(name, null, getSchema()), manageDriver = true)

/**
* @param [name] Name of the database file, or null for an in-memory database (as per Android framework implementation).
Expand Down Expand Up @@ -52,4 +43,5 @@ fun SqlNormalizedCacheFactory(
useNoBackupDirectory = useNoBackupDirectory,
windowSizeBytes = windowSizeBytes,
),
manageDriver = true,
)
11 changes: 1 addition & 10 deletions ...ppleMain/kotlin/com/apollographql/cache/normalized/sql/SqlNormalizedCacheFactory.apple.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,9 @@
package com.apollographql.cache.normalized.sql

import app.cash.sqldelight.db.SqlDriver
import com.apollographql.cache.normalized.api.NormalizedCache
import com.apollographql.cache.normalized.api.NormalizedCacheFactory
import com.apollographql.cache.normalized.sql.internal.createDriver
import com.apollographql.cache.normalized.sql.internal.createRecordDatabase
import com.apollographql.cache.normalized.sql.internal.getSchema

actual fun SqlNormalizedCacheFactory(driver: SqlDriver): NormalizedCacheFactory = object : NormalizedCacheFactory() {
override fun create(): NormalizedCache {
return SqlNormalizedCache(createRecordDatabase(driver))
}
}

/**
* @param name the name of the database or null for an in-memory database
* @param baseDir the baseDirectory where to store the database.
Expand All @@ -22,6 +13,6 @@ actual fun SqlNormalizedCacheFactory(driver: SqlDriver): NormalizedCacheFactory
fun SqlNormalizedCacheFactory(
name: String?,
baseDir: String?,
): NormalizedCacheFactory = SqlNormalizedCacheFactory(createDriver(name, baseDir, getSchema()))
): NormalizedCacheFactory = SqlNormalizedCacheFactory(createDriver(name, baseDir, getSchema()), manageDriver = true)

actual fun SqlNormalizedCacheFactory(name: String?): NormalizedCacheFactory = SqlNormalizedCacheFactory(name, null)
15 changes: 14 additions & 1 deletion ...src/commonMain/kotlin/com/apollographql/cache/normalized/sql/SqlNormalizedCacheFactory.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package com.apollographql.cache.normalized.sql

import app.cash.sqldelight.db.SqlDriver
import com.apollographql.cache.normalized.api.NormalizedCache
import com.apollographql.cache.normalized.api.NormalizedCacheFactory
import com.apollographql.cache.normalized.sql.internal.createRecordDatabase

/**
* Creates a new [NormalizedCacheFactory] that uses a persistent cache based on Sqlite
Expand All @@ -15,4 +17,15 @@ import com.apollographql.cache.normalized.api.NormalizedCacheFactory
*/
expect fun SqlNormalizedCacheFactory(name: String? = "apollo.db"): NormalizedCacheFactory

expect fun SqlNormalizedCacheFactory(driver: SqlDriver): NormalizedCacheFactory
fun SqlNormalizedCacheFactory(driver: SqlDriver): NormalizedCacheFactory = SqlNormalizedCacheFactory(driver, false)

internal fun SqlNormalizedCacheFactory(driver: SqlDriver, manageDriver: Boolean): NormalizedCacheFactory =
Copy link
Contributor

@martinbonnin martinbonnin Nov 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The manageDriver here seems inconsistent. I would avoid having to manually define ownership. There are 2 cases:

  1. We want the connection to be shared accross multiple clients => expose it as a public property so the user can retrieve and close it manually if they want to. (see OkHttpClient.dispatcher.executorService.shutdown()).
  2. We want ApolloClient to own the connection => close it systematically.

Option 1. is not breaking current behaviour. On the other hand, 2. is probably what most users expect by default. At the end of the day, it all boils down to whether there's a good case for sharing a connection. I would intuitively say "no" but haven't thought that completely through.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like 1! If we expose the driver, there's no need for a close.

About 2, yeah currently it's possible to pass the same factory to multiple clients, which should probably be forbidden?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's possible to pass the same factory to multiple clients, which should probably be forbidden?

If we forbid sharing a connection between multiple clients, we might as well close the connection systematically when the client is closed. At first sight I would say this is what the majority of users expect.

Copy link
Collaborator Author

@BoD BoD Nov 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot that ApolloClient.close has no knowledge of ApolloStore 😅 To make this happen we could add a ApolloStore.manage(Closeable) sorry I meant ApolloClient.manage(Closeable)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either that or ApolloInterceptor.close() (HttpInterceptor has something like this already for batching IIRC)

object : NormalizedCacheFactory() {
override fun create(): NormalizedCache {
return SqlNormalizedCache(createRecordDatabase(driver))
}

override fun close() {
if (manageDriver) driver.close()
}
}
13 changes: 2 additions & 11 deletions ...rc/jvmMain/kotlin/com/apollographql/cache/normalized/sql/SqlNormalizedCacheFactory.jvm.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,11 @@
package com.apollographql.cache.normalized.sql

import app.cash.sqldelight.db.SqlDriver
import app.cash.sqldelight.driver.jdbc.sqlite.JdbcSqliteDriver
import com.apollographql.cache.normalized.api.NormalizedCache
import com.apollographql.cache.normalized.api.NormalizedCacheFactory
import com.apollographql.cache.normalized.sql.internal.createDriver
import com.apollographql.cache.normalized.sql.internal.createRecordDatabase
import com.apollographql.cache.normalized.sql.internal.getSchema
import java.util.Properties

actual fun SqlNormalizedCacheFactory(driver: SqlDriver): NormalizedCacheFactory = object : NormalizedCacheFactory() {
override fun create(): NormalizedCache {
return SqlNormalizedCache(createRecordDatabase(driver))
}
}

/**
* @param url Database connection URL in the form of `jdbc:sqlite:path` where `path` is either blank
* (creating an in-memory database) or a path to a file.
Expand All @@ -23,7 +14,7 @@ actual fun SqlNormalizedCacheFactory(driver: SqlDriver): NormalizedCacheFactory
fun SqlNormalizedCacheFactory(
url: String,
properties: Properties = Properties(),
): NormalizedCacheFactory = SqlNormalizedCacheFactory(JdbcSqliteDriver(url, properties))
): NormalizedCacheFactory = SqlNormalizedCacheFactory(JdbcSqliteDriver(url, properties), manageDriver = true)

/**
* @param name the name of the database or null for an in-memory database
Expand All @@ -34,6 +25,6 @@ fun SqlNormalizedCacheFactory(
fun SqlNormalizedCacheFactory(
name: String?,
baseDir: String?,
): NormalizedCacheFactory = SqlNormalizedCacheFactory(createDriver(name, baseDir, getSchema()))
): NormalizedCacheFactory = SqlNormalizedCacheFactory(createDriver(name, baseDir, getSchema()), manageDriver = true)

actual fun SqlNormalizedCacheFactory(name: String?): NormalizedCacheFactory = SqlNormalizedCacheFactory(name, null)
4 changes: 4 additions & 0 deletions .../jvmMain/kotlin/com/apollographql/cache/normalized/sql/TrimmableNormalizedCacheFactory.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ class TrimmableNormalizedCacheFactory internal constructor(

return SqlNormalizedCache(Blob2RecordDatabase(queries))
}

override fun close() {
driver.close()
}
}


2 changes: 2 additions & 0 deletions tests/normalized-cache/src/commonTest/kotlin/NormalizedCacheThreadingTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ class NormalizedCacheThreadingTest {
cacheCreateThreadName = currentThreadId()
return MemoryCacheFactory().create()
}

override fun close() {}
}).build()
assertNull(cacheCreateThreadName)

Expand Down
1 change: 1 addition & 0 deletions tests/normalized-cache/src/commonTest/kotlin/ThreadTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ class ThreadTests {
return MyNormalizedCache(mainThreadId)
}

override fun close() {}
}

@Test
Expand Down