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

Defer: take @defer cases into account with normalized cache #3987

Merged
merged 4 commits into from
Apr 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@ class AdapterContext private constructor(
}

fun hasDeferredFragment(path: List<Any>, label: String?): Boolean {
return mergedDeferredFragmentIds?.contains(DeferredFragmentIdentifier(path, label)) == true
if (mergedDeferredFragmentIds == null) {
// By default, parse all deferred fragments - this is the case when parsing from the normalized cache.
return true
}
return mergedDeferredFragmentIds.contains(DeferredFragmentIdentifier(path, label))
martinbonnin marked this conversation as resolved.
Show resolved Hide resolved
}

class Builder {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ enum class FetchPolicy {
/**
* Try the cache, if that failed, try the network.
*
* An [ApolloCompositeException] is thrown if the data is not in the cache and the network call failed, otherwise 1 value is emitted.
* An [ApolloCompositeException] is thrown if the data is not in the cache and the network call failed.
* If coming from the cache 1 value is emitted, otherwise 1 or multiple values can be emitted from the network.
*
* This is the default behaviour.
*/
Expand All @@ -53,22 +54,23 @@ enum class FetchPolicy {
/**
* Try the network, if that failed, try the cache.
*
* An [ApolloCompositeException] is thrown if the network call failed and the data is not in the cache, otherwise 1 value is emitted.
* An [ApolloCompositeException] is thrown if the network call failed and the data is not in the cache.
* If coming from the network 1 or multiple values can be emitted, otherwise 1 value is emitted from the cache.
*/
NetworkFirst,

/**
* Only try the network.
*
* An [ApolloException] is thrown if the network call failed, otherwise 1 value is emitted.
* An [ApolloException] is thrown if the network call failed, otherwise 1 or multiple values can be emitted.
*/
NetworkOnly,

/**
* Try the cache, then also try the network.
*
* If the data is in the cache, it is emitted, if not, no exception is thrown at that point. Then the network call is made, and if
* successful, the value is emitted, if not, either an [ApolloCompositeException] (both cache miss and network failed) or an
* successful the value(s) are emitted, otherwise either an [ApolloCompositeException] (both cache miss and network failed) or an
* [ApolloException] (only network failed) is thrown.
*/
CacheAndNetwork,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ import com.apollographql.apollo3.interceptor.ApolloInterceptor
import com.apollographql.apollo3.interceptor.ApolloInterceptorChain
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.catch
import kotlinx.coroutines.flow.emitAll
import kotlinx.coroutines.flow.flow
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.singleOrNull
import kotlin.jvm.JvmName

Expand Down Expand Up @@ -64,34 +66,33 @@ val CacheFirstInterceptor = object : ApolloInterceptor {
return@flow
}

val networkResponse = chain.proceed(
val networkResponses = chain.proceed(
request = request
).catch {
if (it is ApolloException) {
networkException = it
} else {
throw it
}
}.singleOrNull()
}.map { response ->
Copy link
Contributor

Choose a reason for hiding this comment

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

This breaks the contract that CacheFirst emits only once. I guess there's no real way around this but the doc needs to be updated

response.newBuilder()
.cacheInfo(
response.cacheInfo!!
.newBuilder()
.cacheMissException(cacheException as? CacheMissException)
.build()
)
.build()
}

if (networkResponse != null) {
emit(
networkResponse.newBuilder()
.cacheInfo(
networkResponse.cacheInfo!!
.newBuilder()
.cacheMissException(cacheException as? CacheMissException)
.build()
)
.build()
emitAll(networkResponses)

if (networkException != null) {
throw ApolloCompositeException(
first = cacheException,
second = networkException
)
return@flow
}

throw ApolloCompositeException(
first = cacheException,
second = networkException
)
}
}
}
Expand All @@ -102,18 +103,18 @@ val NetworkFirstInterceptor = object : ApolloInterceptor {
var cacheException: ApolloException? = null
var networkException: ApolloException? = null

val networkResponse = chain.proceed(
val networkResponses = chain.proceed(
request = request
).catch {
if (it is ApolloException) {
networkException = it
} else {
throw it
}
}.singleOrNull()
}

if (networkResponse != null) {
emit(networkResponse)
emitAll(networkResponses)
if (networkException == null) {
return@flow
}

Expand Down Expand Up @@ -179,35 +180,36 @@ val CacheAndNetworkInterceptor = object : ApolloInterceptor {
emit(cacheResponse.newBuilder().isLast(false).build())
}

val networkResponse = chain.proceed(request)
val networkResponses = chain.proceed(request)
.catch {
if (it is ApolloException) {
networkException = it
} else {
throw it
}
}.singleOrNull()

if (networkResponse != null) {
emit(
networkResponse.newBuilder()
}
.map { response ->
response.newBuilder()
.cacheInfo(
networkResponse.cacheInfo!!
response.cacheInfo!!
.newBuilder()
.cacheMissException(cacheException as? CacheMissException)
.build()
)
.build()
)
return@flow
}
if (cacheException != null) {
throw ApolloCompositeException(
first = cacheException,
second = networkException
)
}

emitAll(networkResponses)

if (networkException != null) {
if (cacheException != null) {
throw ApolloCompositeException(
first = cacheException,
second = networkException
)
}
throw networkException!!
}
throw networkException!!
martinbonnin marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,13 @@ import com.apollographql.apollo3.interceptor.ApolloInterceptor
import com.apollographql.apollo3.interceptor.ApolloInterceptorChain
import com.apollographql.apollo3.mpp.currentTimeMillis
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.catch
import kotlinx.coroutines.flow.collect
import kotlinx.coroutines.flow.emitAll
import kotlinx.coroutines.flow.flow
import kotlinx.coroutines.flow.flowOn
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.flow.single
import kotlinx.coroutines.launch

internal class ApolloCacheInterceptor(
Expand Down Expand Up @@ -143,26 +146,43 @@ internal class ApolloCacheInterceptor(
/**
* This doesn't use [readFromNetwork] so that we can publish all keys all at once after the keys have been rolled back
*/
var response: ApolloResponse<D>? = null
var exception: ApolloException? = null
try {
response = chain.proceed(request).single()
} catch (e: ApolloException) {
exception = e
}
var networkException: ApolloException? = null
val networkResponses: Flow<ApolloResponse<D>> = chain.proceed(request)
.catch {
if (it is ApolloException) {
networkException = it
} else {
throw it
}
}

val optimisticKeys = if (optimisticData != null) {
store.rollbackOptimisticUpdates(request.requestUuid, publish = false)
} else {
emptySet()
}
var optimisticKeys: Set<String>? = null

if (response != null) {
maybeWriteToCache(request, response, customScalarAdapters, optimisticKeys)
var previousResponse: ApolloResponse<D>? = null
networkResponses.collect { response ->
if (optimisticData != null && previousResponse != null) {
throw ApolloException("Apollo: optimistic updates can only be applied with one network response")
}
previousResponse = response
if (optimisticKeys == null) optimisticKeys = if (optimisticData != null) {
store.rollbackOptimisticUpdates(request.requestUuid, publish = false)
} else {
emptySet()
}

maybeWriteToCache(request, response, customScalarAdapters, optimisticKeys!!)
emit(response)
} else {
store.publish(optimisticKeys)
throw exception!!
}

if (networkException != null) {
if (optimisticKeys == null) optimisticKeys = if (optimisticData != null) {
martinbonnin marked this conversation as resolved.
Show resolved Hide resolved
store.rollbackOptimisticUpdates(request.requestUuid, publish = false)
} else {
emptySet()
}

store.publish(optimisticKeys!!)
throw networkException!!
}
}
}
Expand All @@ -172,13 +192,11 @@ internal class ApolloCacheInterceptor(
val fetchFromCache = request.fetchFromCache

return flow {
emit(
if (fetchFromCache) {
readFromCache(request, customScalarAdapters)
} else {
readFromNetwork(request, chain, customScalarAdapters)
}
)
if (fetchFromCache) {
emit(readFromCache(request, customScalarAdapters))
} else {
emitAll(readFromNetwork(request, chain, customScalarAdapters))
}
}
}

Expand Down Expand Up @@ -238,18 +256,18 @@ internal class ApolloCacheInterceptor(
request: ApolloRequest<D>,
chain: ApolloInterceptorChain,
customScalarAdapters: CustomScalarAdapters,
): ApolloResponse<D> {
): Flow<ApolloResponse<D>> {
val startMillis = currentTimeMillis()
val networkResponse = chain.proceed(request).onEach {
return chain.proceed(request).onEach {
maybeWriteToCache(request, it, customScalarAdapters)
martinbonnin marked this conversation as resolved.
Show resolved Hide resolved
}.single()

return networkResponse.newBuilder()
.cacheInfo(
CacheInfo.Builder()
.networkStartMillis(startMillis)
.networkEndMillis(currentTimeMillis())
.build()
).build()
}.map { networkResponse ->
networkResponse.newBuilder()
.cacheInfo(
CacheInfo.Builder()
.networkStartMillis(startMillis)
.networkEndMillis(currentTimeMillis())
.build()
).build()
}
}
}
1 change: 1 addition & 0 deletions tests/defer/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ kotlin {
val commonMain by getting {
dependencies {
implementation("com.apollographql.apollo3:apollo-runtime")
implementation("com.apollographql.apollo3:apollo-normalized-cache")
}
}

Expand Down
15 changes: 12 additions & 3 deletions tests/defer/src/commonMain/graphql/operation.graphql
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
query WithFragmentSpreads {
query WithFragmentSpreadsQuery {
computers {
id
...ComputerFields @defer
}
}

fragment ComputerFields on Computer {
cpu
year
Expand All @@ -12,13 +13,13 @@ fragment ComputerFields on Computer {
...ScreenFields @defer(label: "a")
}
}

fragment ScreenFields on Screen {
isColor
}



query WithInlineFragments {
query WithInlineFragmentsQuery {
computers {
id
... on Computer @defer {
Expand All @@ -33,3 +34,11 @@ query WithInlineFragments {
}
}
}


mutation WithFragmentSpreadsMutation {
computers {
id
...ComputerFields @defer(label: "c")
}
}
6 changes: 6 additions & 0 deletions tests/defer/src/commonMain/graphql/schema.graphqls
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
type Query {
computers: [Computer!]!
}

type Mutation {
computers: [Computer!]!
}

type Computer {
id: ID!
cpu: String!
year: Int!
screen: Screen!
}

type Screen {
resolution: String!
isColor: Boolean!
Expand Down
Loading