Throw IllegalStateException on re-using keys
PageFetcherSnapshot now throws if it detects key re-use on attempts to
load distinct pages. This means re-using keys for loading a dropped page
is allowed and that key-reuse state is not carried over between
generations.
Adds a property to PagingSource to turn off this behavior, for
intentional use cases. In particular, this check was added to cover the
common case of infinitely looping empty loads as some item-keyed sources
and paging2 datasource marks end of load with an empty page, but an
empty load is not a terminal state in paging3.
Fixes: 156110590
Test: ./gradlew paging:paging-common:test
Change-Id: I0229d259f3ed4b8cf2bb1dda5cc2cd6014f59cbc
diff --git a/paging/common/api/3.0.0-alpha01.txt b/paging/common/api/3.0.0-alpha01.txt
index 4745aeb..91f5ad3 100644
--- a/paging/common/api/3.0.0-alpha01.txt
+++ b/paging/common/api/3.0.0-alpha01.txt
@@ -267,6 +267,7 @@
ctor public PagingSource();
method public final boolean getInvalid();
method public boolean getJumpingSupported();
+ method public boolean getKeyReuseSupported();
method @androidx.paging.ExperimentalPagingApi public Key? getRefreshKey(androidx.paging.PagingState<Key,Value> state);
method public void invalidate();
method public abstract suspend Object? load(androidx.paging.PagingSource.LoadParams<Key> params, kotlin.coroutines.Continuation<? super androidx.paging.PagingSource.LoadResult<Key,Value>> p);
@@ -274,6 +275,7 @@
method public final void unregisterInvalidatedCallback(kotlin.jvm.functions.Function0<kotlin.Unit> onInvalidatedCallback);
property public final boolean invalid;
property public boolean jumpingSupported;
+ property public boolean keyReuseSupported;
}
public abstract static sealed class PagingSource.LoadParams<Key> {
diff --git a/paging/common/api/current.txt b/paging/common/api/current.txt
index 4745aeb..91f5ad3 100644
--- a/paging/common/api/current.txt
+++ b/paging/common/api/current.txt
@@ -267,6 +267,7 @@
ctor public PagingSource();
method public final boolean getInvalid();
method public boolean getJumpingSupported();
+ method public boolean getKeyReuseSupported();
method @androidx.paging.ExperimentalPagingApi public Key? getRefreshKey(androidx.paging.PagingState<Key,Value> state);
method public void invalidate();
method public abstract suspend Object? load(androidx.paging.PagingSource.LoadParams<Key> params, kotlin.coroutines.Continuation<? super androidx.paging.PagingSource.LoadResult<Key,Value>> p);
@@ -274,6 +275,7 @@
method public final void unregisterInvalidatedCallback(kotlin.jvm.functions.Function0<kotlin.Unit> onInvalidatedCallback);
property public final boolean invalid;
property public boolean jumpingSupported;
+ property public boolean keyReuseSupported;
}
public abstract static sealed class PagingSource.LoadParams<Key> {
diff --git a/paging/common/api/public_plus_experimental_3.0.0-alpha01.txt b/paging/common/api/public_plus_experimental_3.0.0-alpha01.txt
index 4745aeb..91f5ad3 100644
--- a/paging/common/api/public_plus_experimental_3.0.0-alpha01.txt
+++ b/paging/common/api/public_plus_experimental_3.0.0-alpha01.txt
@@ -267,6 +267,7 @@
ctor public PagingSource();
method public final boolean getInvalid();
method public boolean getJumpingSupported();
+ method public boolean getKeyReuseSupported();
method @androidx.paging.ExperimentalPagingApi public Key? getRefreshKey(androidx.paging.PagingState<Key,Value> state);
method public void invalidate();
method public abstract suspend Object? load(androidx.paging.PagingSource.LoadParams<Key> params, kotlin.coroutines.Continuation<? super androidx.paging.PagingSource.LoadResult<Key,Value>> p);
@@ -274,6 +275,7 @@
method public final void unregisterInvalidatedCallback(kotlin.jvm.functions.Function0<kotlin.Unit> onInvalidatedCallback);
property public final boolean invalid;
property public boolean jumpingSupported;
+ property public boolean keyReuseSupported;
}
public abstract static sealed class PagingSource.LoadParams<Key> {
diff --git a/paging/common/api/public_plus_experimental_current.txt b/paging/common/api/public_plus_experimental_current.txt
index 4745aeb..91f5ad3 100644
--- a/paging/common/api/public_plus_experimental_current.txt
+++ b/paging/common/api/public_plus_experimental_current.txt
@@ -267,6 +267,7 @@
ctor public PagingSource();
method public final boolean getInvalid();
method public boolean getJumpingSupported();
+ method public boolean getKeyReuseSupported();
method @androidx.paging.ExperimentalPagingApi public Key? getRefreshKey(androidx.paging.PagingState<Key,Value> state);
method public void invalidate();
method public abstract suspend Object? load(androidx.paging.PagingSource.LoadParams<Key> params, kotlin.coroutines.Continuation<? super androidx.paging.PagingSource.LoadResult<Key,Value>> p);
@@ -274,6 +275,7 @@
method public final void unregisterInvalidatedCallback(kotlin.jvm.functions.Function0<kotlin.Unit> onInvalidatedCallback);
property public final boolean invalid;
property public boolean jumpingSupported;
+ property public boolean keyReuseSupported;
}
public abstract static sealed class PagingSource.LoadParams<Key> {
diff --git a/paging/common/api/restricted_3.0.0-alpha01.txt b/paging/common/api/restricted_3.0.0-alpha01.txt
index 4745aeb..91f5ad3 100644
--- a/paging/common/api/restricted_3.0.0-alpha01.txt
+++ b/paging/common/api/restricted_3.0.0-alpha01.txt
@@ -267,6 +267,7 @@
ctor public PagingSource();
method public final boolean getInvalid();
method public boolean getJumpingSupported();
+ method public boolean getKeyReuseSupported();
method @androidx.paging.ExperimentalPagingApi public Key? getRefreshKey(androidx.paging.PagingState<Key,Value> state);
method public void invalidate();
method public abstract suspend Object? load(androidx.paging.PagingSource.LoadParams<Key> params, kotlin.coroutines.Continuation<? super androidx.paging.PagingSource.LoadResult<Key,Value>> p);
@@ -274,6 +275,7 @@
method public final void unregisterInvalidatedCallback(kotlin.jvm.functions.Function0<kotlin.Unit> onInvalidatedCallback);
property public final boolean invalid;
property public boolean jumpingSupported;
+ property public boolean keyReuseSupported;
}
public abstract static sealed class PagingSource.LoadParams<Key> {
diff --git a/paging/common/api/restricted_current.txt b/paging/common/api/restricted_current.txt
index 4745aeb..91f5ad3 100644
--- a/paging/common/api/restricted_current.txt
+++ b/paging/common/api/restricted_current.txt
@@ -267,6 +267,7 @@
ctor public PagingSource();
method public final boolean getInvalid();
method public boolean getJumpingSupported();
+ method public boolean getKeyReuseSupported();
method @androidx.paging.ExperimentalPagingApi public Key? getRefreshKey(androidx.paging.PagingState<Key,Value> state);
method public void invalidate();
method public abstract suspend Object? load(androidx.paging.PagingSource.LoadParams<Key> params, kotlin.coroutines.Continuation<? super androidx.paging.PagingSource.LoadResult<Key,Value>> p);
@@ -274,6 +275,7 @@
method public final void unregisterInvalidatedCallback(kotlin.jvm.functions.Function0<kotlin.Unit> onInvalidatedCallback);
property public final boolean invalid;
property public boolean jumpingSupported;
+ property public boolean keyReuseSupported;
}
public abstract static sealed class PagingSource.LoadParams<Key> {
diff --git a/paging/common/src/main/kotlin/androidx/paging/PageFetcherSnapshot.kt b/paging/common/src/main/kotlin/androidx/paging/PageFetcherSnapshot.kt
index df28f60..dc2f293 100644
--- a/paging/common/src/main/kotlin/androidx/paging/PageFetcherSnapshot.kt
+++ b/paging/common/src/main/kotlin/androidx/paging/PageFetcherSnapshot.kt
@@ -373,6 +373,27 @@
// this load loop terminates due to fulfilling prefetchDistance.
var endOfPaginationReached = false
loop@ while (loadKey != null) {
+ // Check for common error case where the same key is re-used to load
+ // new pages, often resulting in infinite loops.
+ stateLock.withLock {
+ val previousKey = when (loadType) {
+ PREPEND -> state.pages.first().prevKey
+ APPEND -> state.pages.last().nextKey
+ else -> throw IllegalArgumentException(
+ "Use doInitialLoad for LoadType == REFRESH"
+ )
+ }
+
+ check(
+ pagingSource.keyReuseSupported || loadKey != previousKey
+ ) {
+ """A load key, $loadKey, was re-used to load two distinct pages with the same
+ | loadType, $loadType. Re-using load keys in PagingSource is often an error, and
+ | must be explicitly enabled by overriding PagingSource.keyReuseSupported.
+ """.trimMargin()
+ }
+ }
+
val params = loadParams(loadType, loadKey)
val result: LoadResult<Key, Value> = pagingSource.load(params)
when (result) {
diff --git a/paging/common/src/main/kotlin/androidx/paging/PagingSource.kt b/paging/common/src/main/kotlin/androidx/paging/PagingSource.kt
index 71e8175..00ca3766 100644
--- a/paging/common/src/main/kotlin/androidx/paging/PagingSource.kt
+++ b/paging/common/src/main/kotlin/androidx/paging/PagingSource.kt
@@ -275,6 +275,13 @@
get() = false
/**
+ * `true` if this [PagingSource] expects to re-use keys to load distinct pages
+ * without a call to [invalidate], `false` otherwise.
+ */
+ open val keyReuseSupported: Boolean
+ get() = false
+
+ /**
* Request a refresh key given the current [PagingState] of the associated [PagingData] used to
* present loaded data from this [PagingSource].
*
diff --git a/paging/common/src/test/kotlin/androidx/paging/CachingTest.kt b/paging/common/src/test/kotlin/androidx/paging/CachingTest.kt
index 30dedd4..5a9ac8e 100644
--- a/paging/common/src/test/kotlin/androidx/paging/CachingTest.kt
+++ b/paging/common/src/test/kotlin/androidx/paging/CachingTest.kt
@@ -370,6 +370,9 @@
) : PagingSource<Int, Item>() {
private var generation = -1
+ override val keyReuseSupported: Boolean
+ get() = true
+
override suspend fun load(params: LoadParams<Int>): LoadResult<Int, Item> {
when (params) {
is LoadParams.Refresh -> {
diff --git a/paging/common/src/test/kotlin/androidx/paging/PageFetcherSnapshotTest.kt b/paging/common/src/test/kotlin/androidx/paging/PageFetcherSnapshotTest.kt
index d9de10f..0011300 100644
--- a/paging/common/src/test/kotlin/androidx/paging/PageFetcherSnapshotTest.kt
+++ b/paging/common/src/test/kotlin/androidx/paging/PageFetcherSnapshotTest.kt
@@ -1558,6 +1558,89 @@
)
}
}
+
+ @Test
+ fun keyReuse_unsupported() = testScope.runBlockingTest {
+ pauseDispatcher {
+ val pager = PageFetcherSnapshot(
+ initialKey = 50,
+ pagingSource = object : PagingSource<Int, Int>() {
+ override val keyReuseSupported: Boolean
+ get() = false
+
+ override suspend fun load(params: LoadParams<Int>) = when (params) {
+ is LoadParams.Refresh -> Page(listOf(0), 0, 0)
+ else -> Page<Int, Int>(listOf(), 0, 0)
+ }
+ },
+ config = config,
+ retryFlow = retryCh.asFlow()
+ )
+
+ // Trigger collection on flow.
+ launch {
+ // Assert second prepend re-using key = 0 leads to IllegalStateException
+ assertFailsWith<IllegalStateException> {
+ pager.pageEventFlow.collect { }
+ }
+ }
+
+ advanceUntilIdle()
+
+ // Trigger first prepend with key = 0
+ pager.addHint(ViewportHint(0, 0))
+ advanceUntilIdle()
+
+ // Trigger second prepend with key = 0
+ pager.addHint(ViewportHint(0, 0))
+ advanceUntilIdle()
+ }
+ }
+
+ @Test
+ fun keyReuse_supported() = testScope.runBlockingTest {
+ pauseDispatcher {
+ val pager = PageFetcherSnapshot(
+ initialKey = 50,
+ pagingSource = object : PagingSource<Int, Int>() {
+ var loads = 0
+
+ override val keyReuseSupported: Boolean
+ get() = true
+
+ override suspend fun load(params: LoadParams<Int>) = when (params) {
+ is LoadParams.Refresh -> Page(listOf(0), 0, 0)
+ else -> Page<Int, Int>(
+ listOf(),
+ if (loads < 3) 0 else null,
+ if (loads < 3) 0 else null
+ )
+ }.also {
+ loads++
+ }
+ },
+ config = config,
+ retryFlow = retryCh.asFlow()
+ )
+
+ // Trigger collection on flow.
+ val job = launch {
+ pager.pageEventFlow.collect { }
+ }
+
+ advanceUntilIdle()
+
+ // Trigger first prepend with key = 0
+ pager.addHint(ViewportHint(0, 0))
+ advanceUntilIdle()
+
+ // Trigger second prepend with key = 0
+ pager.addHint(ViewportHint(0, 0))
+ advanceUntilIdle()
+
+ job.cancel()
+ }
+ }
}
@Suppress("SuspendFunctionOnCoroutineScope")
diff --git a/testutils/testutils-paging/src/main/java/androidx/paging/TestPagingSource.kt b/testutils/testutils-paging/src/main/java/androidx/paging/TestPagingSource.kt
index c918b7f..91f88a0 100644
--- a/testutils/testutils-paging/src/main/java/androidx/paging/TestPagingSource.kt
+++ b/testutils/testutils-paging/src/main/java/androidx/paging/TestPagingSource.kt
@@ -41,6 +41,9 @@
}
}
+ override val keyReuseSupported: Boolean
+ get() = true
+
override suspend fun load(params: LoadParams<Int>): LoadResult<Int, Int> {
val key = params.key ?: 0