Skip to content

Commit

Permalink
Cover DefaultLoadingHandler
Browse files Browse the repository at this point in the history
  • Loading branch information
matt-ramotar committed Jul 14, 2024
1 parent 5f00da8 commit 5771dcc
Show file tree
Hide file tree
Showing 5 changed files with 202 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,4 @@ class RealPagingLogger(
Logger.v("storex/paging") { message }
}
}
}

class TestPagingLogger : PagingLogger {
override fun verbose(message: String) {
println(message)
}

override fun debug(message: String) {
println(message)
}

override fun error(message: String, error: Throwable) {
println(message)
println(error)
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -75,38 +75,44 @@ internal class DefaultLoadingHandler<Id : Identifier<Id>, K : Comparable<K>, V :

updateLoadingState(loadParams.direction)

try {

// Update max and min request so far for both append and prepend
// This ensures we're tracking the full range of requests regardless of sort order
// These methods don't necessarily update the fetching state, but simply recompute the max and min
fetchingStateHolder.updateMaxRequestSoFar(modifiedParams.key)
fetchingStateHolder.updateMinRequestSoFar(modifiedParams.key)

loadPage(modifiedParams, loadParams.direction, addNextToQueue)
} catch (pagingError: Throwable) {
logger.error(
"""
// Update max and min request so far for both append and prepend
// This ensures we're tracking the full range of requests regardless of sort order
// These methods don't necessarily update the fetching state, but simply recompute the max and min
fetchingStateHolder.updateMaxRequestSoFar(loadParams.key)
fetchingStateHolder.updateMinRequestSoFar(loadParams.key)

tryLoad(loadParams, addNextToQueue)
}
}
}

// This is extracted from handleLoading because we might need to retry the load
// We can't simply re invoke handleLoading is because it will cause a deadlock
private suspend fun tryLoad(loadParams: PagingSource.LoadParams<K>, addNextToQueue: Boolean) {
try {

loadPage(loadParams, addNextToQueue)
} catch (pagingError: Throwable) {
logger.error(
"""
Caught error
Load params: $loadParams
Direction: ${loadParams.direction}
""".trimIndent(),
pagingError
)
handleError(pagingError, modifiedParams, loadParams.direction)
}
}
pagingError
)
handleError(pagingError, loadParams, addNextToQueue)
}
}

private suspend fun loadPage(loadParams: PagingSource.LoadParams<K>, direction: LoadDirection, addNextToQueue: Boolean) {
private suspend fun loadPage(loadParams: PagingSource.LoadParams<K>, addNextToQueue: Boolean) {
val pageLoadFlow = store.loadPage(loadParams)

// Design decision to use only the latest data for any given params
// Not OK to skip params, but OK to skip stale data
pageLoadFlow.collectLatest { pageLoadState ->
when (pageLoadState) {
is PageLoadState.Success -> handleSuccess(pageLoadState, loadParams.key, direction, addNextToQueue)
is PageLoadState.Success -> handleSuccess(pageLoadState, loadParams.key, loadParams.direction, addNextToQueue)
is PageLoadState.Error -> throw pageLoadState.toThrowable()
else -> { /* Do nothing for other states */
}
Expand Down Expand Up @@ -171,16 +177,16 @@ internal class DefaultLoadingHandler<Id : Identifier<Id>, K : Comparable<K>, V :
queueManager.updateExistingPendingJob(key, inFlight = false, completed = true)
}

private suspend fun passThroughError(error: Throwable, loadParams: PagingSource.LoadParams<K>, direction: LoadDirection) {
private suspend fun passThroughError(error: Throwable, loadParams: PagingSource.LoadParams<K>) {

// Passing the error through
updateErrorState(error, direction)
updateErrorState(error, loadParams.direction)

// Complete the job
completeLoadJobAfterError(loadParams, direction)
completeLoadJobAfterError(loadParams)
}

private suspend fun completeLoadJobAfterError(loadParams: PagingSource.LoadParams<K>, direction: LoadDirection) {
private suspend fun completeLoadJobAfterError(loadParams: PagingSource.LoadParams<K>) {
// Design decision to remove placeholders when not retrying after an error
// Page refreshes are not currently supported
// So the page must currently contain placeholders
Expand All @@ -189,7 +195,7 @@ internal class DefaultLoadingHandler<Id : Identifier<Id>, K : Comparable<K>, V :
// Marking the pending job as completed
queueManager.updateExistingPendingJob(loadParams.key, inFlight = false, completed = true)

when (direction) {
when (loadParams.direction) {
LoadDirection.Prepend -> {
// Removing the params from the prepend queue
queueManager.prependQueue.removeFirst { it.params == loadParams }
Expand All @@ -205,32 +211,32 @@ internal class DefaultLoadingHandler<Id : Identifier<Id>, K : Comparable<K>, V :
retryBookkeeper.resetCount(loadParams)
}

private suspend fun handleError(error: Throwable, loadParams: PagingSource.LoadParams<K>, direction: LoadDirection) {
private suspend fun handleError(error: Throwable, loadParams: PagingSource.LoadParams<K>, addNextToQueue: Boolean) {
when (errorHandlingStrategy) {
ErrorHandlingStrategy.Ignore -> {
logger.error(
"""
Ignoring the error
Load params: $loadParams
Direction: $direction
Direction: ${loadParams.direction}
""".trimIndent(),
error
)

// Complete the job, do nothing else
completeLoadJobAfterError(loadParams, direction)
completeLoadJobAfterError(loadParams)
}

ErrorHandlingStrategy.PassThrough -> {
logger.error(
"""
Passing the error through
Load params: $loadParams
Direction: $direction
Direction: $loadParams.direction
""".trimIndent(),
error
)
passThroughError(error, loadParams, direction)
passThroughError(error, loadParams)
}

is ErrorHandlingStrategy.RetryLast -> {
Expand All @@ -241,7 +247,7 @@ internal class DefaultLoadingHandler<Id : Identifier<Id>, K : Comparable<K>, V :
"""
Determining whether to retry
Load params: $loadParams
Direction: $direction
Direction: ${loadParams.direction}
Retry count: $retryCount
Max retries: ${errorHandlingStrategy.maxRetries}
""".trimIndent()
Expand All @@ -254,22 +260,22 @@ internal class DefaultLoadingHandler<Id : Identifier<Id>, K : Comparable<K>, V :

// Retrying with backoff
exponentialBackoff.execute(retryCount) {
when (direction) {
when (loadParams.direction) {
LoadDirection.Prepend -> {
logger.verbose("Retrying prepend load for: $loadParams")
handlePrependLoading(loadParams)
tryLoad(loadParams, addNextToQueue)
}

LoadDirection.Append -> {
logger.verbose("Retrying append load for: $loadParams")
handleAppendLoading(loadParams)
tryLoad(loadParams, addNextToQueue)
}
}
}

} else {
logger.verbose("At maximum retries, passing the error through")
passThroughError(error, loadParams, direction)
passThroughError(error, loadParams)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,21 @@
package org.mobilenativefoundation.storex.paging.runtime

import dev.mokkery.annotations.DelicateMokkeryApi
import dev.mokkery.answering.Answer
import dev.mokkery.answering.returns
import dev.mokkery.every
import dev.mokkery.everySuspend
import dev.mokkery.matcher.any
import dev.mokkery.matcher.eq
import dev.mokkery.mock
import dev.mokkery.spy
import dev.mokkery.verify.VerifyMode.Companion.exactly
import dev.mokkery.verifySuspend
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.flowOf
import kotlinx.coroutines.test.runTest
import kotlinx.datetime.Clock
import org.mobilenativefoundation.storex.paging.custom.Middleware
import org.mobilenativefoundation.storex.paging.runtime.internal.logger.impl.TestPagingLogger
import org.mobilenativefoundation.storex.paging.runtime.internal.pager.api.ExponentialBackoff
import org.mobilenativefoundation.storex.paging.runtime.internal.pager.api.FetchingStateHolder
import org.mobilenativefoundation.storex.paging.runtime.internal.pager.api.LoadParamsQueue
Expand All @@ -26,10 +28,13 @@ import org.mobilenativefoundation.storex.paging.runtime.internal.store.api.Norma
import org.mobilenativefoundation.storex.paging.runtime.internal.store.api.PageLoadState
import org.mobilenativefoundation.storex.paging.testUtils.CursorIdentifier
import org.mobilenativefoundation.storex.paging.testUtils.Post
import org.mobilenativefoundation.storex.paging.testUtils.TestExponentialBackoff
import org.mobilenativefoundation.storex.paging.testUtils.TestPagingLogger
import org.mobilenativefoundation.storex.paging.testUtils.TimelineRequest
import kotlin.test.BeforeTest
import kotlin.test.Test

@OptIn(DelicateMokkeryApi::class)
class DefaultLoadingHandlerTest {

private val appendQueue = mock<LoadParamsQueue<TimelineRequest>>()
Expand All @@ -52,9 +57,11 @@ class DefaultLoadingHandlerTest {
private val errorHandlingStrategy = ErrorHandlingStrategy.Ignore
private val middleware = emptyList<Middleware<TimelineRequest>>()
private val operationApplier = mock<OperationApplier<CursorIdentifier, TimelineRequest, Post>>()
private val retryBookkeeper = mock<RetryBookkeeper<CursorIdentifier, TimelineRequest>>()
private val retryBookkeeper = mock<RetryBookkeeper<CursorIdentifier, TimelineRequest>> {
everySuspend { getCount(any()) } returns 0
}
private val logger = TestPagingLogger()
private val exponentialBackoff = mock<ExponentialBackoff>()
private val exponentialBackoff = spy<ExponentialBackoff>(TestExponentialBackoff())

private lateinit var loadingHandler: DefaultLoadingHandler<CursorIdentifier, TimelineRequest, Post>

Expand Down Expand Up @@ -359,4 +366,128 @@ class DefaultLoadingHandlerTest {
verifySuspend(exactly(0)) { pagingStateManager.updateWithPrependError(any()) }
verifySuspend(exactly(1)) { queueManager.updateExistingPendingJob(eq(loadParams.key), eq(false), eq(true)) }
}

@Test
fun handleAppendLoading_givenRetryLastStrategy_whenPersistentError_thenShouldPassThroughAfterHittingLimit() = runTest {
// Given
val cursor = "1"
val size = 20
val key = TimelineRequest(cursor, size)
val strategy = LoadStrategy.SkipCache
val direction = LoadDirection.Append
val loadParams = PagingSource.LoadParams(key, strategy, direction)
val expectedPosts = List(size) { createFakePost(it + 1) }
val errorHandlingStrategy = ErrorHandlingStrategy.RetryLast(1)
var retryCount = 0

fun incrementRetryCount() {
retryCount++
}

fun getRetryCount() = retryCount
val retryBookkeeper = mock<RetryBookkeeper<CursorIdentifier, TimelineRequest>> {
everySuspend { getCount(any()) }.answers(Answer.Block { getRetryCount() })
everySuspend { incrementCount(any()) }.answers(Answer.Block { incrementRetryCount() })
}

loadingHandler = DefaultLoadingHandler(
store,
pagingStateManager,
queueManager,
fetchingStateHolder,
errorHandlingStrategy,
middleware,
operationApplier,
retryBookkeeper,
logger,
exponentialBackoff
)

val expectedStoreFlow = flowOf(
PageLoadState.Error.Message<CursorIdentifier, TimelineRequest, Post>("", true)
)

everySuspend { store.loadPage(eq(loadParams)) }.returns(expectedStoreFlow)
everySuspend { operationApplier.applyOperations(any(), any(), any(), any()) } returns ItemSnapshotList(expectedPosts)

// When
loadingHandler.handleAppendLoading(loadParams)

// Then
verifySuspend(exactly(2)) { store.loadPage(eq(loadParams)) }
verifySuspend(exactly(1)) { fetchingStateHolder.updateMaxRequestSoFar(eq(key)) }
verifySuspend(exactly(1)) { fetchingStateHolder.updateMinRequestSoFar(eq(key)) }
verifySuspend(exactly(2)) { retryBookkeeper.getCount(eq(loadParams)) }
verifySuspend(exactly(1)) { retryBookkeeper.incrementCount(eq(loadParams)) }
verifySuspend(exactly(1)) { exponentialBackoff.execute(eq(0), any()) }

verifySuspend(exactly(1)) { pagingStateManager.updateWithAppendError(any()) }

verifySuspend(exactly(1)) { store.clearPage(eq(loadParams.key)) }
verifySuspend(exactly(1)) { queueManager.updateExistingPendingJob(eq(loadParams.key), eq(false), eq(true)) }
verifySuspend(exactly(1)) { appendQueue.removeFirst(any()) }
verifySuspend(exactly(1)) { retryBookkeeper.resetCount(eq(loadParams)) }
}

@Test
fun handlePrependLoading_givenRetryLastStrategy_whenPersistentError_thenShouldPassThroughAfterHittingLimit() = runTest {
// Given
val cursor = "1"
val size = 20
val key = TimelineRequest(cursor, size)
val strategy = LoadStrategy.SkipCache
val direction = LoadDirection.Prepend
val loadParams = PagingSource.LoadParams(key, strategy, direction)
val expectedPosts = List(size) { createFakePost(it + 1) }
val errorHandlingStrategy = ErrorHandlingStrategy.RetryLast(1)
var retryCount = 0

fun incrementRetryCount() {
retryCount++
}

fun getRetryCount() = retryCount
val retryBookkeeper = mock<RetryBookkeeper<CursorIdentifier, TimelineRequest>> {
everySuspend { getCount(any()) }.answers(Answer.Block { getRetryCount() })
everySuspend { incrementCount(any()) }.answers(Answer.Block { incrementRetryCount() })
}

loadingHandler = DefaultLoadingHandler(
store,
pagingStateManager,
queueManager,
fetchingStateHolder,
errorHandlingStrategy,
middleware,
operationApplier,
retryBookkeeper,
logger,
exponentialBackoff
)

val expectedStoreFlow = flowOf(
PageLoadState.Error.Message<CursorIdentifier, TimelineRequest, Post>("", true)
)

everySuspend { store.loadPage(eq(loadParams)) }.returns(expectedStoreFlow)
everySuspend { operationApplier.applyOperations(any(), any(), any(), any()) } returns ItemSnapshotList(expectedPosts)

// When
loadingHandler.handlePrependLoading(loadParams)

// Then
verifySuspend(exactly(2)) { store.loadPage(eq(loadParams)) }
verifySuspend(exactly(1)) { fetchingStateHolder.updateMaxRequestSoFar(eq(key)) }
verifySuspend(exactly(1)) { fetchingStateHolder.updateMinRequestSoFar(eq(key)) }
verifySuspend(exactly(2)) { retryBookkeeper.getCount(eq(loadParams)) }
verifySuspend(exactly(1)) { retryBookkeeper.incrementCount(eq(loadParams)) }
verifySuspend(exactly(1)) { exponentialBackoff.execute(eq(0), any()) }

verifySuspend(exactly(1)) { pagingStateManager.updateWithPrependError(any()) }

verifySuspend(exactly(1)) { store.clearPage(eq(loadParams.key)) }
verifySuspend(exactly(1)) { queueManager.updateExistingPendingJob(eq(loadParams.key), eq(false), eq(true)) }
verifySuspend(exactly(1)) { prependQueue.removeFirst(any()) }
verifySuspend(exactly(1)) { retryBookkeeper.resetCount(eq(loadParams)) }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package org.mobilenativefoundation.storex.paging.testUtils

import org.mobilenativefoundation.storex.paging.runtime.internal.pager.api.ExponentialBackoff

internal class TestExponentialBackoff : ExponentialBackoff {
override suspend fun execute(retryCount: Int, block: suspend () -> Unit) {
block()
}
}
Loading

0 comments on commit 5771dcc

Please sign in to comment.