Skip to content

Don't drop retry/refresh events before uiReceiver is set #754

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

Closed
evant wants to merge 1 commit into from
Closed

Don't drop retry/refresh events before uiReceiver is set #754

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
Don't drop retry/refresh events before uiReceiver is set 
Because uiReceiver is set asynchronously it's possible to race with a
call to retry or refresh on the PagingDataPresenter.

Changed the implementation to record calling these methods and then
forward the event when the uiReceiver is set.

Test: ./gradlew test connectedCheck
Fixes: b/330725007
  • Loading branch information
@evant
evant committed Jan 24, 2025
commit b2603e5bfe3c4307f4d3141cfc84c3e37e5cb099
35 changes: 31 additions & 4 deletions paging/paging-common/src/commonMain/kotlin/androidx/paging/PagingDataPresenter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public abstract class PagingDataPresenter<T : Any>(
cachedPagingData: PagingData<T>? = null,
) {
private var hintReceiver: HintReceiver? = null
private var uiReceiver: UiReceiver? = null
private var uiReceiver: UiReceiver = InitialUiReceiver()
private var pageStore: PageStore<T> = PageStore.initial(cachedPagingData?.cachedEvent())
private val combinedLoadStatesCollection =
MutableCombinedLoadStateCollection().apply {
Expand Down Expand Up @@ -114,7 +114,7 @@ public abstract class PagingDataPresenter<T : Any>(

public suspend fun collectFrom(pagingData: PagingData<T>): @JvmSuppressWildcards Unit {
collectFromRunner.runInIsolation {
uiReceiver = pagingData.uiReceiver
setUiReceiver(pagingData.uiReceiver)
pagingData.flow.collect { event ->
log(VERBOSE) { "Collected $event" }
withContext(mainContext) {
Expand Down Expand Up @@ -309,7 +309,7 @@ public abstract class PagingDataPresenter<T : Any>(
*/
public fun retry() {
log(DEBUG) { "Retry signal received" }
uiReceiver?.retry()
uiReceiver.retry()
}

/**
Expand All @@ -329,7 +329,7 @@ public abstract class PagingDataPresenter<T : Any>(
*/
public fun refresh() {
log(DEBUG) { "Refresh signal received" }
uiReceiver?.refresh()
uiReceiver.refresh()
}

/** @return Total number of presented items, including placeholders. */
Expand Down Expand Up @@ -500,6 +500,33 @@ public abstract class PagingDataPresenter<T : Any>(
hintReceiver?.accessHint(newPageStore.initializeHint())
}
}

// Holds on to retry/refresh requests to deliver them when the real UiReceiver is attached.
private class InitialUiReceiver : UiReceiver {
var retry = false
var refresh = false

override fun retry() {
retry = true
}

override fun refresh() {
refresh = true
}
}

private fun setUiReceiver(receiver: UiReceiver) {
val oldReceiver = this.uiReceiver
this.uiReceiver = receiver
if (oldReceiver is InitialUiReceiver) {
if (oldReceiver.retry) {
receiver.retry()
}
if (oldReceiver.refresh) {
receiver.refresh()
}
}
}
}

/**
Expand Down
31 changes: 30 additions & 1 deletion paging/paging-common/src/commonTest/kotlin/androidx/paging/PagingDataPresenterTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,36 @@ class PagingDataPresenterTest {
job.cancel()
}

@Test
fun retrySentBeforeCollection() =
testScope.run {
val presenter = SimplePresenter()
val receiver = UiReceiverFake()

presenter.retry()

val job = launch { presenter.collectFrom(infinitelySuspendingPagingData(receiver)) }

assertEquals(1, receiver.retryEvents.size)

job.cancel()
}

@Test
fun refreshSentBeforeCollection() =
testScope.runTest {
val presenter = SimplePresenter()
val receiver = UiReceiverFake()

presenter.refresh()

val job = launch { presenter.collectFrom(infinitelySuspendingPagingData(receiver)) }

assertEquals(1, receiver.refreshEvents.size)

job.cancel()
}

@Test
fun uiReceiverSetImmediately() =
testScope.runTest {
Expand Down Expand Up @@ -2471,7 +2501,6 @@ class PagingDataPresenterTest {
val job1 = launch {
presenter.collectFrom(PagingData(flow, uiReceiver2, dummyHintReceiver))
}
presenter.refresh()
assertThat(uiReceiver.refreshEvents).hasSize(0)
assertThat(uiReceiver2.refreshEvents).hasSize(1)
job1.cancel()
Expand Down
Loading